Re: [PATCH] usb: misc: ljca: Fix enumeration error on Dell Latitude 9420

[Hans de Goede <hdegoede@redhat.com>]

Hi Sergei,

On 11/2/23 18:25, Sergei Shtylyov wrote:
> On 11/2/23 7:48 PM, Hans de Goede wrote:
> 
>> Not all LJCA chips implement SPI and on chips without LJCA reading
> 
>    LJCA chips without LJCA?

Heh, will fix for v2.

> 
>> the SPI descriptors will timeout.
>>
>> On laptop models like the Dell Latitude 9420, this is expected behavior
>> and not an error.
>>
>> Modify the driver to continue without instantiating a SPI auxbus child,
>> instead of failing to probe() the whole LJCA chip.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/usb/misc/usb-ljca.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/misc/usb-ljca.c b/drivers/usb/misc/usb-ljca.c
>> index c9decd0396d4..24f781782c56 100644
>> --- a/drivers/usb/misc/usb-ljca.c
>> +++ b/drivers/usb/misc/usb-ljca.c
>> @@ -656,10 +656,11 @@ static int ljca_enumerate_spi(struct ljca_adapter *adap)
>>  	unsigned int i;
>>  	int ret;
>>  
>> +	/* Not all LJCA chips implement SPI a timeout reading the descriptors is normal */
> 
>    Comma missing after SPI?

1 line comments typically don't have any punctuation, also notice
the missing period at the end. With that said I'm happy to add
a comma for v2.

> 
>>  	ret = ljca_send(adap, LJCA_CLIENT_MNG, LJCA_MNG_ENUM_SPI, NULL, 0, buf,
>>  			sizeof(buf), true, LJCA_ENUM_CLIENT_TIMEOUT_MS);
>>  	if (ret < 0)
>> -		return ret;
>> +		return (ret == -ETIMEDOUT) ? 0 : ret;
> 
>    I don't think the parens are necessary.

They are not strictly necessary, but IMHO the code is more readable with
the parens.

Regards,

Hans