On 02.09.24 08:06, Greg Kroah-Hartman wrote:
> 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?
>
I don't know. Also, the last downstream driver Prolific posted [1]
didn't point to this specialty.
I opened a few of those adapter cables (some received from [2], others
from a different source), and the chips were not labeled (in contrast to
the picture from [2]). So I'm suspecting clones to be in play. Reminds
me of the quest for sane WIFI USB adapters.
>>
>> - 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?
I was copying from serial_break() which now returns -ENOTTY if the
handler is NULL. If you prefer -EINVAL, I can change. Just looking for a
consistent code.
Jan
[1]
https://prolificusa.com/wp-content/uploads/2021/01/PL2303G_Linux_Driver_v1.0.6.zip
[2]
https://www.berrybase.de/en/usb-ttl/uart/rs232-adapter-mit-pl2303hx-chipsatz
--
Siemens AG, Technology
Linux Expert Center
