On 2018-10-25 10:17, Heikki Krogerus wrote:
> Hi,
>
> On Tue, Oct 23, 2018 at 06:56:59PM +0000, Ajay Gupta wrote:
>>>> + /* i2c adapter (ccgx-ucsi) can read 4 byte max */
>>>
>>> By "i2c adapter" do you mean this Cypress CCGx controller, or the NVIDIA I2C
>>> host adapter?
>> It mean NVIDIA I2C host adapter with name "ccgx-ucsi"
>>
>>>> + while (rem_len > 0) {
>>>> + msgs[1].buf = &data[len - rem_len];
>>>> + rlen = min_t(u16, rem_len, 4);
>>>
>>> I guess this is where you check for that 4 bytes.
>>>
>>> I'm guessing this limitation is for the NVIDIA I2C host adapter.
>> Correct
>>
>>> If that is the case than this driver really should not care about it.
>> I got your point but need to handle this case gracefully.
>>
>> I think best way to handle this is to add a runtime check to find
>> I2C adapter's quirk and use quirks->max_read_len of the adapter.
>> How does below look?
>>
>> @@ -247,6 +247,7 @@ struct ucsi_ccg {
>> static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
>> {
>> struct i2c_client *client = uc->client;
>> + const struct i2c_adapter_quirks *quirks = client->adapter->quirks;
>> unsigned char buf[2];
>> struct i2c_msg msgs[] = {
>> {
>> @@ -261,13 +262,16 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
>> .buf = data,
>> },
>> };
>> - u32 rlen, rem_len = len;
>> + u32 rlen, rem_len = len, max_read_len = len;
>> int status;
>>
>> - /* i2c adapter (ccgx-ucsi) can read 4 byte max */
>> + /* check any max_read_len limitation on i2c adapter */
>> + if (quirks && quirks->max_read_len)
>> + max_read_len = quirks->max_read_len;
>> +
>> while (rem_len > 0) {
>> msgs[1].buf = &data[len - rem_len];
>> - rlen = min_t(u16, rem_len, 4);
>> + rlen = min_t(u16, rem_len, max_read_len);
>> msgs[1].len = rlen;
>> put_unaligned_le16(rab, buf);
>> status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
>>
>>> We most likely need to use this driver on other platforms as well
>>> where the I2C host is something else.
>> Correct and above solution would not impact other I2C host.
>
> I still didn't understand why can't this just be taken care of in your
> I2C host driver? Why can't you just read 4 bytes at a time in your
> master_xfer hook until you have received as much as the message is
> asking, and only after that return?
The I2C host hardware *cannot* read more than 4 bytes in any one xfer
according to the HW designers. Seriously broken crap, that piece of
hardware... (If that assertion from the HW designers is indeed true?
I suspect that it can be made to work, but the docs are closed and I
don't have HW to experiment with, so it remains a suspicion...)
And the I2C host driver *cannot* be expected to know exactly how any
one client device needs to split xfers into many when the 4 byte limit
is getting in the way, and neither can the I2C core. Because I bet
there are devices where it's not even possible to split xfers while
keeping semantics equivalent...
So, every client driver will need to adjust to this quirk if they are
to operate with this worthless I2C host (or others with similar
limitations, if there are any?). Fortunately, most client drivers don't
read in bulk. Unfortunately, many do...
Cheers,
Peter
