[PATCH RESEND] i2c: tegra: Add SMBus block read function

[PATCH RESEND] i2c: tegra: Add SMBus block read function

[Akhil R]

Emulate SMBus block read using ContinueXfer to read the length byte

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 03cea102ab76..2941e42aa6a0 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -1233,6 +1233,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 */
+	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);
@@ -1389,6 +1394,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;
@@ -1416,10 +1430,10 @@ 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_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
 
 	if (i2c_dev->hw->has_continue_xfer_support)
-		ret |= I2C_FUNC_NOSTART;
+		ret |= I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
 
 	return ret;
 }
-- 
2.17.1

Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Dmitry Osipenko]

10.02.2022 18:36, Akhil R пишет:
> Emulate SMBus block read using ContinueXfer to read the length byte
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 03cea102ab76..2941e42aa6a0 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -1233,6 +1233,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 */
> +	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);
> @@ -1389,6 +1394,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;
> @@ -1416,10 +1430,10 @@ 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_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
>  
>  	if (i2c_dev->hw->has_continue_xfer_support)
> -		ret |= I2C_FUNC_NOSTART;
> +		ret |= I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
>  
>  	return ret;
>  }

Please describe how this was tested.

RE: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Akhil R]

> 10.02.2022 18:36, Akhil R пишет:
> > Emulate SMBus block read using ContinueXfer to read the length byte
> >
> > Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> > ---
> >  drivers/i2c/busses/i2c-tegra.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> > index 03cea102ab76..2941e42aa6a0 100644
> > --- a/drivers/i2c/busses/i2c-tegra.c
> > +++ b/drivers/i2c/busses/i2c-tegra.c
> > @@ -1233,6 +1233,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 */
> > +     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);
> > @@ -1389,6 +1394,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;
> > @@ -1416,10 +1430,10 @@ 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_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
> >
> >       if (i2c_dev->hw->has_continue_xfer_support)
> > -             ret |= I2C_FUNC_NOSTART;
> > +             ret |= I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
> >
> >       return ret;
> >  }
> 
> Please describe how this was tested.
This is tested using an I2C EEPROM to emulate SMBus block read in which
we read the first byte as the length of bytes to read. This is an expected
feature for NVIDIA Grace chipset where there will be an actual SMBus device.

Thanks,
Akhil

Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Dmitry Osipenko]

11.02.2022 12:11, Akhil R пишет:
>> 10.02.2022 18:36, Akhil R пишет:
>>> Emulate SMBus block read using ContinueXfer to read the length byte
>>>
>>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
>>> ---
>>>  drivers/i2c/busses/i2c-tegra.c | 18 ++++++++++++++++--
>>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>>> index 03cea102ab76..2941e42aa6a0 100644
>>> --- a/drivers/i2c/busses/i2c-tegra.c
>>> +++ b/drivers/i2c/busses/i2c-tegra.c
>>> @@ -1233,6 +1233,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 */
>>> +     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);
>>> @@ -1389,6 +1394,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;
>>> @@ -1416,10 +1430,10 @@ 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_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
>>>
>>>       if (i2c_dev->hw->has_continue_xfer_support)
>>> -             ret |= I2C_FUNC_NOSTART;
>>> +             ret |= I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
>>>
>>>       return ret;
>>>  }
>>
>> Please describe how this was tested.
> This is tested using an I2C EEPROM to emulate SMBus block read in which
> we read the first byte as the length of bytes to read. This is an expected
> feature for NVIDIA Grace chipset where there will be an actual SMBus device.

We have several Tegra30+ tablets that have EC on SMBus. Svyatoslav tried
your I2C patch + [1] on Asus TF201 and reported that it breaks EC. Any
idea why it doesn't work?

[1]
https://github.com/grate-driver/linux/commit/aa8d71f5a960ef40503e5448c622d62d1c53a2c0

Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Dmitry Osipenko]

12.02.2022 19:15, Dmitry Osipenko пишет:
> 11.02.2022 12:11, Akhil R пишет:
>>> 10.02.2022 18:36, Akhil R пишет:
>>>> Emulate SMBus block read using ContinueXfer to read the length byte
>>>>
>>>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
>>>> ---
>>>>  drivers/i2c/busses/i2c-tegra.c | 18 ++++++++++++++++--
>>>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>>>> index 03cea102ab76..2941e42aa6a0 100644
>>>> --- a/drivers/i2c/busses/i2c-tegra.c
>>>> +++ b/drivers/i2c/busses/i2c-tegra.c
>>>> @@ -1233,6 +1233,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 */
>>>> +     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);
>>>> @@ -1389,6 +1394,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;
>>>> @@ -1416,10 +1430,10 @@ 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_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
>>>>
>>>>       if (i2c_dev->hw->has_continue_xfer_support)
>>>> -             ret |= I2C_FUNC_NOSTART;
>>>> +             ret |= I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
>>>>
>>>>       return ret;
>>>>  }
>>>
>>> Please describe how this was tested.
>> This is tested using an I2C EEPROM to emulate SMBus block read in which
>> we read the first byte as the length of bytes to read. This is an expected
>> feature for NVIDIA Grace chipset where there will be an actual SMBus device.
> 
> We have several Tegra30+ tablets that have EC on SMBus. Svyatoslav tried
> your I2C patch + [1] on Asus TF201 and reported that it breaks EC. Any
> idea why it doesn't work?
> 
> [1]
> https://github.com/grate-driver/linux/commit/aa8d71f5a960ef40503e5448c622d62d1c53a2c0

Ah, I see now that I2C_FUNC_SMBUS_WRITE_BLOCK_DATA not supported, we
should check again without the write then.

Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Dmitry Osipenko]

12.02.2022 19:20, Dmitry Osipenko пишет:
> 12.02.2022 19:15, Dmitry Osipenko пишет:
>> 11.02.2022 12:11, Akhil R пишет:
>>>> 10.02.2022 18:36, Akhil R пишет:
>>>>> Emulate SMBus block read using ContinueXfer to read the length byte
>>>>>
>>>>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
>>>>> ---
>>>>>  drivers/i2c/busses/i2c-tegra.c | 18 ++++++++++++++++--
>>>>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>>>>> index 03cea102ab76..2941e42aa6a0 100644
>>>>> --- a/drivers/i2c/busses/i2c-tegra.c
>>>>> +++ b/drivers/i2c/busses/i2c-tegra.c
>>>>> @@ -1233,6 +1233,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 */
>>>>> +     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);
>>>>> @@ -1389,6 +1394,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;
>>>>> @@ -1416,10 +1430,10 @@ 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_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
>>>>>
>>>>>       if (i2c_dev->hw->has_continue_xfer_support)
>>>>> -             ret |= I2C_FUNC_NOSTART;
>>>>> +             ret |= I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
>>>>>
>>>>>       return ret;
>>>>>  }
>>>>
>>>> Please describe how this was tested.
>>> This is tested using an I2C EEPROM to emulate SMBus block read in which
>>> we read the first byte as the length of bytes to read. This is an expected
>>> feature for NVIDIA Grace chipset where there will be an actual SMBus device.
>>
>> We have several Tegra30+ tablets that have EC on SMBus. Svyatoslav tried
>> your I2C patch + [1] on Asus TF201 and reported that it breaks EC. Any
>> idea why it doesn't work?
>>
>> [1]
>> https://github.com/grate-driver/linux/commit/aa8d71f5a960ef40503e5448c622d62d1c53a2c0
> 
> Ah, I see now that I2C_FUNC_SMBUS_WRITE_BLOCK_DATA not supported, we
> should check again without the write then.

We also missed that i2c_smbus_read_i2c_block_data() populates the first
byte of the read data with the transfer size. So
i2c_smbus_read_block_data() actually works properly.

It's unclear to me what's the point of emulating
I2C_FUNC_SMBUS_READ_BLOCK_DATA within the driver if you could use
i2c_smbus_read_i2c_block_data().

RE: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Akhil R]

> 12.02.2022 19:20, Dmitry Osipenko пишет:
> > 12.02.2022 19:15, Dmitry Osipenko пишет:
> >> 11.02.2022 12:11, Akhil R пишет:
> >>>> 10.02.2022 18:36, Akhil R пишет:
> >>>>> Emulate SMBus block read using ContinueXfer to read the length byte
> >>>>>
> >>>>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> >>>>> ---
> >>>>>  drivers/i2c/busses/i2c-tegra.c | 18 ++++++++++++++++--
> >>>>>  1 file changed, 16 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> >>>>> index 03cea102ab76..2941e42aa6a0 100644
> >>>>> --- a/drivers/i2c/busses/i2c-tegra.c
> >>>>> +++ b/drivers/i2c/busses/i2c-tegra.c
> >>>>> @@ -1233,6 +1233,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
> */
> >>>>> +     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);
> >>>>> @@ -1389,6 +1394,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;
> >>>>> @@ -1416,10 +1430,10 @@ 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_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
> >>>>>
> >>>>>       if (i2c_dev->hw->has_continue_xfer_support)
> >>>>> -             ret |= I2C_FUNC_NOSTART;
> >>>>> +             ret |= I2C_FUNC_NOSTART |
> I2C_FUNC_SMBUS_READ_BLOCK_DATA;
> >>>>>
> >>>>>       return ret;
> >>>>>  }
> >>>>
> >>>> Please describe how this was tested.
> >>> This is tested using an I2C EEPROM to emulate SMBus block read in which
> >>> we read the first byte as the length of bytes to read. This is an expected
> >>> feature for NVIDIA Grace chipset where there will be an actual SMBus
> device.
> >>
> >> We have several Tegra30+ tablets that have EC on SMBus. Svyatoslav tried
> >> your I2C patch + [1] on Asus TF201 and reported that it breaks EC. Any
> >> idea why it doesn't work?
> >>
> >> [1]
> >> https://github.com/grate-
> driver/linux/commit/aa8d71f5a960ef40503e5448c622d62d1c53a2c0
> >
> > Ah, I see now that I2C_FUNC_SMBUS_WRITE_BLOCK_DATA not supported,
> we
> > should check again without the write then.
> 
> We also missed that i2c_smbus_read_i2c_block_data() populates the first
> byte of the read data with the transfer size. So
> i2c_smbus_read_block_data() actually works properly.
> 
> It's unclear to me what's the point of emulating
> I2C_FUNC_SMBUS_READ_BLOCK_DATA within the driver if you could use
> i2c_smbus_read_i2c_block_data().

We are looking to support I2C_M_RECV_LEN where the length is read from the
first byte of data. I see that i2c_smbus_read_i2c_block_data() requires the length
to be passed from the client driver.

BTW, I2C_FUNC_SMBUS_WRITE_BLOCK_DATA is also expected to be supported.
It is included in I2C_FUNC_SMBUS_EMUL. I suppose, it doesn't require any additional
change in the driver. The client driver should populate the first byte as the length
of data to be transferred.

Thanks,
Akhil

Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Dmitry Osipenko]

14.02.2022 07:49, Akhil R пишет:
>> It's unclear to me what's the point of emulating
>> I2C_FUNC_SMBUS_READ_BLOCK_DATA within the driver if you could use
>> i2c_smbus_read_i2c_block_data().
> We are looking to support I2C_M_RECV_LEN where the length is read from the
> first byte of data. I see that i2c_smbus_read_i2c_block_data() requires the length
> to be passed from the client driver.
> 
> BTW, I2C_FUNC_SMBUS_WRITE_BLOCK_DATA is also expected to be supported.
> It is included in I2C_FUNC_SMBUS_EMUL. I suppose, it doesn't require any additional
> change in the driver. The client driver should populate the first byte as the length
> of data to be transferred.

Please support both read and write.

RE: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Akhil R]

> 14.02.2022 07:49, Akhil R пишет:
> >> It's unclear to me what's the point of emulating
> >> I2C_FUNC_SMBUS_READ_BLOCK_DATA within the driver if you could use
> >> i2c_smbus_read_i2c_block_data().
> > We are looking to support I2C_M_RECV_LEN where the length is read from
> > the first byte of data. I see that i2c_smbus_read_i2c_block_data()
> > requires the length to be passed from the client driver.
> >
> > BTW, I2C_FUNC_SMBUS_WRITE_BLOCK_DATA is also expected to be
> supported.
> > It is included in I2C_FUNC_SMBUS_EMUL. I suppose, it doesn't require
> > any additional change in the driver. The client driver should populate
> > the first byte as the length of data to be transferred.
> 
> Please support both read and write.
Both read and write are supported. Write doesn't require any additional
change in the driver as far as I understand.

It is actually the same that I mentioned before. Or am I missing something here?

Thanks,
Akhil

Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Dmitry Osipenko]

16.02.2022 06:54, Akhil R пишет:
>> 14.02.2022 07:49, Akhil R пишет:
>>>> It's unclear to me what's the point of emulating
>>>> I2C_FUNC_SMBUS_READ_BLOCK_DATA within the driver if you could use
>>>> i2c_smbus_read_i2c_block_data().
>>> We are looking to support I2C_M_RECV_LEN where the length is read from
>>> the first byte of data. I see that i2c_smbus_read_i2c_block_data()
>>> requires the length to be passed from the client driver.
>>>
>>> BTW, I2C_FUNC_SMBUS_WRITE_BLOCK_DATA is also expected to be
>> supported.
>>> It is included in I2C_FUNC_SMBUS_EMUL. I suppose, it doesn't require
>>> any additional change in the driver. The client driver should populate
>>> the first byte as the length of data to be transferred.
>>
>> Please support both read and write.
> Both read and write are supported. Write doesn't require any additional
> change in the driver as far as I understand.
> 
> It is actually the same that I mentioned before. Or am I missing something here?

I missed that I2C_FUNC_SMBUS_EMUL includes I2C_FUNC_SMBUS_WRITE_BLOCK_DATA.

Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Dmitry Osipenko]

10.02.2022 18:36, Akhil R пишет:
> Emulate SMBus block read using ContinueXfer to read the length byte
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>

Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 352 bytes --]

On Thu, Feb 10, 2022 at 09:06:03PM +0530, Akhil R wrote:
> Emulate SMBus block read using ContinueXfer to read the length byte
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 218 bytes --]

On Thu, Feb 10, 2022 at 09:06:03PM +0530, Akhil R wrote:
> Emulate SMBus block read using ContinueXfer to read the length byte
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]