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

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

[Hans de Goede]

Not all LJCA chips implement SPI and on chips without LJCA reading
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 */
 	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;
 
 	/* check firmware response */
 	desc = (struct ljca_spi_descriptor *)buf;
-- 
2.41.0

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

[Sergei Shtylyov]

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?

> 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?

>  	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.

[...]

MBR, Sergey

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

[Hans de Goede]

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

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

[Greg Kroah-Hartman]

On Thu, Nov 02, 2023 at 07:05:33PM +0100, Hans de Goede wrote:
> > 
> >>  	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.

I agree, please leave it in.

thanks,

greg k-h