Re: [PATCH] HID: letsketch: skip string descriptors the firmware does not implement

[Hans de Goede <hansg@kernel.org>]

Hi,

On 13-Nov-26 12:04, Eren Çakar wrote:
> The Starcom GC-422 is a rebranded LetSketch WP9620C with the same USB
> ID (6161:4d15), so it binds to this driver. Its firmware
> ("Vision_V201_180530") does not implement every string descriptor index
> that the raw mode handshake reads. For those indices usb_string() keeps
> returning an empty string or -EPIPE, letsketch_get_string() gives up
> after its retries, and the probe fails:
> 
>   letsketch ...: probe with driver letsketch failed with error -5
> 
> The tablet then does not work at all.
> 
> The handshake only needs the reads to happen, not their contents. Once
> the retries are used up, report an index that still reads as empty or
> STALLs as missing with -ENOENT, and let the probe skip it. Other errors
> still fail the probe as before, and the retries stay in place, so
> firmware that only fails a read now and then behaves as it did.
> 
> With this change the GC-422 completes the handshake, switches to raw
> mode and reports pen position, pressure and the five pad buttons.
> 
> The change and this message were written with the help of an AI coding
> assistant (Claude Code); I reviewed them and tested the result on the
> hardware.
> 
> Fixes: 33a5c2793451 ("HID: Add new Letsketch tablet driver")
> Signed-off-by: Eren Çakar <hey@erencakar.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans



> ---
>  drivers/hid/hid-letsketch.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hid/hid-letsketch.c b/drivers/hid/hid-letsketch.c
> index b52e93a..6a93245 100644
> --- a/drivers/hid/hid-letsketch.c
> +++ b/drivers/hid/hid-letsketch.c
> @@ -213,6 +213,12 @@ static int letsketch_raw_event(struct hid_device *hdev,
>   * string descriptors. But the firmware is buggy and does not like it if
>   * we do this too fast. Even if we go slow sometimes the usb_string() call
>   * fails. Ignore errors and retry it a couple of times if necessary.
> + *
> + * Some firmware revisions (e.g. "Vision_V201_180530", sold as the Starcom
> + * GC-422) do not implement every string descriptor index the handshake
> + * reads. For those indices usb_string() keeps returning an empty string or
> + * -EPIPE (STALL). Report such an index as missing with -ENOENT once the
> + * retries are used up, so that the caller can skip it.
>   */
>  static int letsketch_get_string(struct usb_device *udev, int index, char *buf, int size)
>  {
> @@ -225,9 +231,14 @@ static int letsketch_get_string(struct usb_device *udev, int index, char *buf, i
>  			return 0;
>  	}
>  
> +	if (ret == 0 || ret == -EPIPE) {
> +		dev_dbg(&udev->dev, "String descriptor %d not implemented\n", index);
> +		return -ENOENT;
> +	}
> +
>  	dev_err(&udev->dev, "Max retries (%d) exceeded reading string descriptor %d\n",
>  		LETSKETCH_GET_STRING_RETRIES, index);
> -	return ret ? ret : -EIO;
> +	return ret;
>  }
>  
>  static int letsketch_probe(struct hid_device *hdev, const struct hid_device_id *id)
> @@ -256,6 +267,8 @@ static int letsketch_probe(struct hid_device *hdev, const struct hid_device_id *
>  	 */
>  	for (i = LETSKETCH_INFO_STR_IDX_BEGIN; i <= LETSKETCH_INFO_STR_IDX_END; i++) {
>  		ret = letsketch_get_string(udev, i, buf, sizeof(buf));
> +		if (ret == -ENOENT)
> +			continue;
>  		if (ret)
>  			return ret;
>  
> @@ -264,16 +277,16 @@ static int letsketch_probe(struct hid_device *hdev, const struct hid_device_id *
>  
>  	for (i = 1; i <= 250; i++) {
>  		ret = letsketch_get_string(udev, i, buf, sizeof(buf));
> -		if (ret)
> +		if (ret && ret != -ENOENT)
>  			return ret;
>  	}
>  
>  	ret = letsketch_get_string(udev, 0x64, buf, sizeof(buf));
> -	if (ret)
> +	if (ret && ret != -ENOENT)
>  		return ret;
>  
>  	ret = letsketch_get_string(udev, LETSKETCH_INFO_STR_IDX_BEGIN, buf, sizeof(buf));
> -	if (ret)
> +	if (ret && ret != -ENOENT)
>  		return ret;
>  
>  	/*