On Sun, Sep 01, 2024 at 11:11:29PM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>
> There are apparently incomplete clones of the HXD type chip in use.
> Those return -EPIPE on GET_LINE_REQUEST and BREAK_REQUEST. Avoid
> flooding the kernel log with those errors. Rather use the
> line_settings cache for GET_LINE_REQUEST and signal missing support by
> returning -ENOTTY from pl2303_set_break.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> ---
> drivers/usb/serial/pl2303.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index d93f5d584557..04cafa819390 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -731,12 +731,13 @@ static int pl2303_get_line_request(struct usb_serial_port *port,
> GET_LINE_REQUEST, GET_LINE_REQUEST_TYPE,
> 0, 0, buf, 7, 100);
> if (ret != 7) {
> - dev_err(&port->dev, "%s - failed: %d\n", __func__, ret);
> + struct pl2303_private *priv = usb_get_serial_port_data(port);
>
> - if (ret >= 0)
> - ret = -EIO;
> + dev_dbg(&port->dev, "%s - failed, falling back on cache: %d\n",
> + __func__, ret);
> + memcpy(buf, priv->line_settings, 7);
Ugh, how is this device working in other operating systems?
>
> - return ret;
> + return 0;
> }
>
> dev_dbg(&port->dev, "%s - %7ph\n", __func__, buf);
> @@ -1078,8 +1079,8 @@ static int pl2303_set_break(struct usb_serial_port *port, bool enable)
> BREAK_REQUEST, BREAK_REQUEST_TYPE, state,
> 0, NULL, 0, 100);
> if (result) {
> - dev_err(&port->dev, "error sending break = %d\n", result);
> - return result;
> + dev_dbg(&port->dev, "error sending break = %d\n", result);
> + return -ENOTTY;
Are you sure that ENOTTY is correct here? Why not just send back
-EINVAL or something like that telling userspace that this is not
allowed for this device?
thanks,
greg k-h
