Re: [PATCH] usb: misc: cypress_cy7c63: check control transfer status

[Oliver Neukum <oneukum@suse.com>]

On 10.07.26 01:30, Keshav Verma wrote:
> read_port() currently ignores errors from vendor_command() and always
> returns the cached port value. This can report stale data when the USB
> control transfer fails or returns a short response.
> 
> Convert vendor_command() to usb_control_msg_recv(), which returns 0 on
> success and a negative errno on failure, and propagate transport errors
> from read_port() instead of returning cached data.

Hi,

Thank you for making a patch for this issue.
I am sorry, but this approach mixes issues that are real with something
that is not a problem.
  
> Fixes: 9189bfc2df0f ("[PATCH] USB: rename Cypress CY7C63xxx driver to
> proper name and fix up some tiny things")
> Signed-off-by: Keshav Verma <iganschel@gmail.com>
> ---
> v2:

> -       retval = usb_control_msg(dev->udev, pipe, request,
> -                                USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_OTHER,
> -                                address, data, iobuf, CYPRESS_MAX_REQSIZE,
> -                                USB_CTRL_GET_TIMEOUT);

This does return errors.

> -       /* we must not process garbage */
> -       if (retval < 2)
> +       dev_dbg(&dev->udev->dev, "Sending usb_control_msg_recv (data:
> %d)\n", data);
> +
> +       retval = usb_control_msg_recv(dev->udev, 0, request,

There is no point in this change. You just allocate yet another
buffer for no gain.

	Regards
		Oliver