Re: [PATCH 8/9] dev-serial: fix FTDI_GET_MDM_ST response

[Samuel Thibault <samuel.thibault@gnu.org>]

Mark Cave-Ayland, le lun. 26 oct. 2020 13:40:05 +0000, a ecrit:
> On 26/10/2020 13:00, Jason Andryuk wrote:
> > On Mon, Oct 26, 2020 at 7:21 AM Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > > Aurelien, you introduced the "| 1" in
> > > 
> > > commit abb8a13918ecc1e8160aa78582de9d5224ea70df
> > > Author: Aurelien Jarno <aurelien@aurel32.net>
> > > Date:   Wed Aug 13 04:23:17 2008 +0000
> > > 
> > >      usb-serial: add support for modem lines
> > > 
> > > [...]
> > > @@ -357,9 +393,9 @@ static int usb_serial_handle_control(USBDevice *dev, int request, int value,
> > >           /* TODO: TX ON/OFF */
> > >           break;
> > >       case DeviceInVendor | FTDI_GET_MDM_ST:
> > > -        /* TODO: return modem status */
> > > -        data[0] = 0;
> > > -        ret = 1;
> > > +        data[0] = usb_get_modem_lines(s) | 1;
> > > +        data[1] = 0;
> > > +        ret = 2;
> > >           break;
> > 
> > I'm not particularly familiar with the FTDI USB serial devices.  I
> > found setting FTDI_THRE | FTDI_TEMT by comparing with real hardware.
> > 
> > A little searching found this:
> > https://elixir.bootlin.com/linux/latest/source/drivers/usb/serial/ftdi_sio.h#L541
> > 
> > That shows "B0   Reserved - must be 1", so maybe that is why "| 1" was added?
> 
> Right - that's for the modem status returned as part of the first 2 status
> bytes for incoming data which is slightly different from modem status
> returned directly from FTDI_SIO_GET_MODEM_STATUS: https://elixir.bootlin.com/linux/latest/source/drivers/usb/serial/ftdi_sio.h#L423.
> 
> It is the latter which this patch changes and appears to match what I see on
> my Chipi-X hardware here.

Aurelien, do you remember the reason for the addition of 1 here? It does
look like the confusion between the incoming data bytes and the modem
status bytes.

Samuel