On Thu, Dec 09, 2021 at 08:35:20PM +0530, Akhil R wrote:

Tegra I2C can use a gpio as an smbus-alert. Document the usage of the same.

Signed-off-by: Akhil R akhilrajeev@nvidia.com

Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt index 3f2f990..71ee79b 100644 --- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt +++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt @@ -70,6 +70,10 @@ Required properties:

• rx
• tx

+optional properties: +- smbalert-gpio: Must contain an entry for the gpio to be used as smbus alert.

• It will be used only if optional smbus-alert property is present.

There's already a standard way to do this with interrupts. And GPIOs can be interrupts usually.

Example:

i2c@7000c000 {

2.7.4

09.12.2021 18:05, Akhil R пишет:

Emulate the SMBus block read using ContinueXfer and SMBus using GPIO interrupt.

For SMBus block read, the driver reads the first byte with ContinueXfer set which will help to parse the data count and read the remaining bytes without stop condition in between. SMBus alert is implemented using external gpio interrupt.

Signed-off-by: Akhil R akhilrajeev@nvidia.com

drivers/i2c/busses/i2c-tegra.c | 54 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c index a5be8f0..3b70013 100644 --- a/drivers/i2c/busses/i2c-tegra.c +++ b/drivers/i2c/busses/i2c-tegra.c @@ -14,6 +14,7 @@ #include <linux/dma-mapping.h> #include <linux/err.h> #include <linux/i2c.h> +#include <linux/i2c-smbus.h> #include <linux/init.h> #include <linux/interrupt.h> #include <linux/io.h> @@ -226,6 +227,11 @@ struct tegra_i2c_hw_feature { bool has_interface_timing_reg; };

+struct tegra_i2c_smbalert {

smbalert isn't a word, should be smbus_alert. Same for the GPIO name and other places.

• struct i2c_smbus_alert_setup alert_data;
• struct i2c_client *ara;

What "ara" stands for? Please use meaningful names, like alert_dev for example.

I don't see where this member is used at all, please remove it.

+};

/**

• struct tegra_i2c_dev - per device I2C context
• @dev: device reference for power management

@@ -280,6 +286,8 @@ struct tegra_i2c_dev { int msg_err; u8 *msg_buf;

• struct tegra_i2c_smbalert smbalert;

All properties must have doc comment.

struct completion dma_complete; struct dma_chan *tx_dma_chan; struct dma_chan *rx_dma_chan; @@ -1232,6 +1240,11 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev, return err;

i2c_dev->msg_buf = msg->buf;

• /* The condition true implies smbus block read and len is already read*/

Proper SMBus capitalization in comments. Mussing whitespace in the end of the comment.

• if (msg->flags & I2C_M_RECV_LEN && end_state != MSG_END_CONTINUE)

• i2c_dev->msg_buf = msg->buf + 1;
  

• i2c_dev->msg_buf_remaining = msg->len; i2c_dev->msg_err = I2C_ERR_NONE; i2c_dev->msg_read = !!(msg->flags & I2C_M_RD);

@@ -1388,6 +1401,15 @@ static int tegra_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], else end_type = MSG_END_REPEAT_START; }

• /* If M_RECV_LEN use ContinueXfer to read the first byte */
  

• if (msgs[i].flags & I2C_M_RECV_LEN) {
  

• 	ret = tegra_i2c_xfer_msg(i2c_dev, &msgs[i], MSG_END_CONTINUE);
  

• 	if (ret)
  

• 		break;
  

• 	/* Set the read byte as msg len */
  

• 	msgs[i].len = msgs[i].buf[0];
  

• 	dev_dbg(i2c_dev->dev, "reading %d bytes\n", msgs[i].len);
  

• }
  

  ret = tegra_i2c_xfer_msg(i2c_dev, &msgs[i], end_type); if (ret) break;

@@ -1415,7 +1437,8 @@ static u32 tegra_i2c_func(struct i2c_adapter *adap) { struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(adap); u32 ret = I2C_FUNC_I2C | (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK) |

•   I2C_FUNC_10BIT_ADDR |	I2C_FUNC_PROTOCOL_MANGLING;
  

•   I2C_FUNC_SMBUS_READ_BLOCK_DATA | I2C_FUNC_10BIT_ADDR |
  

•   I2C_FUNC_PROTOCOL_MANGLING;
  
  

  if (i2c_dev->hw->has_continue_xfer_support) ret |= I2C_FUNC_NOSTART;

@@ -1727,6 +1750,29 @@ static int tegra_i2c_init_hardware(struct tegra_i2c_dev *i2c_dev) return ret; }

+static int tegra_i2c_setup_smbalert(struct tegra_i2c_dev *i2c_dev) +{

• struct tegra_i2c_smbalert *smbalert = &i2c_dev->smbalert;
• struct gpio_desc *alert_gpiod;
• struct i2c_client *ara;
• alert_gpiod = devm_gpiod_get(i2c_dev->dev, "smbalert", GPIOD_IN);
• if (IS_ERR(alert_gpiod))

• return PTR_ERR(alert_gpiod);
  

• smbalert->alert_data.irq = gpiod_to_irq(alert_gpiod);
• if (smbalert->alert_data.irq <= 0)

• return smbalert->alert_data.irq;
  

• ara = i2c_new_smbus_alert_device(&i2c_dev->adapter, &smbalert->alert_data);
• if (IS_ERR(ara))

• return PTR_ERR(ara);
  

• smbalert->ara = ara;
• return 0;

+}

static int tegra_i2c_probe(struct platform_device *pdev) { struct tegra_i2c_dev *i2c_dev; @@ -1821,6 +1867,12 @@ static int tegra_i2c_probe(struct platform_device *pdev) if (err) goto release_rpm;

• if (device_property_read_bool(i2c_dev->dev, "smbus-alert")) {

I'd move this device_property_read_bool() inside of tegra_i2c_setup_smbus_alert(), for consistency with the rest of the code in this driver.

Although, you shouldn't need it at all, use devm_gpiod_get_optional().

• err = tegra_i2c_setup_smbalert(i2c_dev);
  

• if (err)
  

• 	dev_warn(&pdev->dev, "smbus-alert setup failed: %d\n", err);
  

GPIO may probe-defer, must be dev_err_probe() here.