* Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-10 15:36 [PATCH RESEND] i2c: tegra: Add SMBus block read function Akhil R
@ 2022-02-10 20:25 ` Dmitry Osipenko
2022-02-11 9:11 ` Akhil R
2022-02-16 20:32 ` Dmitry Osipenko
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Dmitry Osipenko @ 2022-02-10 20:25 UTC (permalink / raw)
To: Akhil R, christian.koenig, jonathanh, ldewangan, linux-i2c,
linux-kernel, linux-tegra, mperttunen, p.zabel, sumit.semwal,
thierry.reding
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.
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-10 20:25 ` Dmitry Osipenko
@ 2022-02-11 9:11 ` Akhil R
2022-02-12 16:15 ` Dmitry Osipenko
0 siblings, 1 reply; 13+ messages in thread
From: Akhil R @ 2022-02-11 9:11 UTC (permalink / raw)
To: Dmitry Osipenko, christian.koenig, Jonathan Hunter,
Laxman Dewangan, linux-i2c, linux-kernel, linux-tegra,
Mikko Perttunen, p.zabel, sumit.semwal, thierry.reding
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-11 9:11 ` Akhil R
@ 2022-02-12 16:15 ` Dmitry Osipenko
2022-02-12 16:20 ` Dmitry Osipenko
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Osipenko @ 2022-02-12 16:15 UTC (permalink / raw)
To: Akhil R, christian.koenig, Jonathan Hunter, Laxman Dewangan,
linux-i2c, linux-kernel, linux-tegra, Mikko Perttunen, p.zabel,
sumit.semwal, thierry.reding, Svyatoslav Ryhel,
Michał Mirosław
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-12 16:15 ` Dmitry Osipenko
@ 2022-02-12 16:20 ` Dmitry Osipenko
2022-02-12 20:08 ` Dmitry Osipenko
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Osipenko @ 2022-02-12 16:20 UTC (permalink / raw)
To: Akhil R, christian.koenig, Jonathan Hunter, Laxman Dewangan,
linux-i2c, linux-kernel, linux-tegra, Mikko Perttunen, p.zabel,
sumit.semwal, thierry.reding, Svyatoslav Ryhel,
Michał Mirosław
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.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-12 16:20 ` Dmitry Osipenko
@ 2022-02-12 20:08 ` Dmitry Osipenko
2022-02-14 4:49 ` Akhil R
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Osipenko @ 2022-02-12 20:08 UTC (permalink / raw)
To: Akhil R, christian.koenig, Jonathan Hunter, Laxman Dewangan,
linux-i2c, linux-kernel, linux-tegra, Mikko Perttunen, p.zabel,
sumit.semwal, thierry.reding, Svyatoslav Ryhel,
Michał Mirosław
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().
^ permalink raw reply [flat|nested] 13+ messages in thread* RE: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-12 20:08 ` Dmitry Osipenko
@ 2022-02-14 4:49 ` Akhil R
2022-02-15 21:33 ` Dmitry Osipenko
0 siblings, 1 reply; 13+ messages in thread
From: Akhil R @ 2022-02-14 4:49 UTC (permalink / raw)
To: Dmitry Osipenko, christian.koenig, Jonathan Hunter,
Laxman Dewangan, linux-i2c, linux-kernel, linux-tegra,
Mikko Perttunen, p.zabel, sumit.semwal, thierry.reding,
Svyatoslav Ryhel, Michał Mirosław, Krishna Yarlagadda
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-14 4:49 ` Akhil R
@ 2022-02-15 21:33 ` Dmitry Osipenko
2022-02-16 3:54 ` Akhil R
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Osipenko @ 2022-02-15 21:33 UTC (permalink / raw)
To: Akhil R, christian.koenig, Jonathan Hunter, Laxman Dewangan,
linux-i2c, linux-kernel, linux-tegra, Mikko Perttunen, p.zabel,
sumit.semwal, thierry.reding, Svyatoslav Ryhel,
Michał Mirosław, Krishna Yarlagadda
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-15 21:33 ` Dmitry Osipenko
@ 2022-02-16 3:54 ` Akhil R
2022-02-16 18:50 ` Dmitry Osipenko
0 siblings, 1 reply; 13+ messages in thread
From: Akhil R @ 2022-02-16 3:54 UTC (permalink / raw)
To: Dmitry Osipenko, christian.koenig, Jonathan Hunter,
Laxman Dewangan, linux-i2c, linux-kernel, linux-tegra,
Mikko Perttunen, p.zabel, sumit.semwal, thierry.reding,
Svyatoslav Ryhel, Michał Mirosław, Krishna Yarlagadda
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-16 3:54 ` Akhil R
@ 2022-02-16 18:50 ` Dmitry Osipenko
0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2022-02-16 18:50 UTC (permalink / raw)
To: Akhil R, christian.koenig, Jonathan Hunter, Laxman Dewangan,
linux-i2c, linux-kernel, linux-tegra, Mikko Perttunen, p.zabel,
sumit.semwal, thierry.reding, Svyatoslav Ryhel,
Michał Mirosław, Krishna Yarlagadda
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-10 15:36 [PATCH RESEND] i2c: tegra: Add SMBus block read function Akhil R
2022-02-10 20:25 ` Dmitry Osipenko
@ 2022-02-16 20:32 ` Dmitry Osipenko
2022-02-25 13:03 ` Thierry Reding
2022-03-01 19:37 ` Wolfram Sang
3 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2022-02-16 20:32 UTC (permalink / raw)
To: Akhil R, christian.koenig, jonathanh, ldewangan, linux-i2c,
linux-kernel, linux-tegra, mperttunen, p.zabel, sumit.semwal,
thierry.reding
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-10 15:36 [PATCH RESEND] i2c: tegra: Add SMBus block read function Akhil R
2022-02-10 20:25 ` Dmitry Osipenko
2022-02-16 20:32 ` Dmitry Osipenko
@ 2022-02-25 13:03 ` Thierry Reding
2022-03-01 19:37 ` Wolfram Sang
3 siblings, 0 replies; 13+ messages in thread
From: Thierry Reding @ 2022-02-25 13:03 UTC (permalink / raw)
To: Akhil R
Cc: christian.koenig, digetx, jonathanh, ldewangan, linux-i2c,
linux-kernel, linux-tegra, mperttunen, p.zabel, sumit.semwal
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH RESEND] i2c: tegra: Add SMBus block read function
2022-02-10 15:36 [PATCH RESEND] i2c: tegra: Add SMBus block read function Akhil R
` (2 preceding siblings ...)
2022-02-25 13:03 ` Thierry Reding
@ 2022-03-01 19:37 ` Wolfram Sang
3 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2022-03-01 19:37 UTC (permalink / raw)
To: Akhil R
Cc: christian.koenig, digetx, jonathanh, ldewangan, linux-i2c,
linux-kernel, linux-tegra, mperttunen, p.zabel, sumit.semwal,
thierry.reding
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread