| Message ID | 20221126035825.6991-1-alexh@vpitech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | USB: serial: cp210x: map B0 to B9600 | expand |
On Fri, Nov 25, 2022 at 08:58:25PM -0700, Alex Henrie wrote: > When a baud rate of 0 is requested, both the 8250 driver and the FTDI > driver reset the baud rate to the default of 9600 (see the comment above > the uart_get_baud_rate function). Some old versions of the NXP blhost > utility depend on this behavior. However, the CP210x driver resets the > baud rate to the minimum supported rate of 300. Special-case B0 so that > it returns the baud rate to the more sensible default of 9600. > > Signed-off-by: Alex Henrie <alexh@vpitech.com> > --- > drivers/usb/serial/cp210x.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c > index 3bcec419f463..2c910550dca8 100644 > --- a/drivers/usb/serial/cp210x.c > +++ b/drivers/usb/serial/cp210x.c > @@ -1051,9 +1051,14 @@ static void cp210x_change_speed(struct tty_struct *tty, > * This maps the requested rate to the actual rate, a valid rate on > * cp2102 or cp2103, or to an arbitrary rate in [1M, max_speed]. > * > - * NOTE: B0 is not implemented. > + * NOTE: B0 is not implemented, apart from returning the baud rate to > + * the default of B9600. > */ > - baud = clamp(tty->termios.c_ospeed, priv->min_speed, priv->max_speed); > + if (tty->termios.c_ospeed) { > + baud = clamp(tty->termios.c_ospeed, priv->min_speed, priv->max_speed); > + } else { > + baud = 9600; > + } No need for { } here (checkpatch.pl should have warned about this.) thanks, greg k-h
Hello! On 11/26/22 6:58 AM, Alex Henrie wrote: > When a baud rate of 0 is requested, both the 8250 driver and the FTDI > driver reset the baud rate to the default of 9600 (see the comment above > the uart_get_baud_rate function). Some old versions of the NXP blhost > utility depend on this behavior. However, the CP210x driver resets the > baud rate to the minimum supported rate of 300. Special-case B0 so that > it returns the baud rate to the more sensible default of 9600. > > Signed-off-by: Alex Henrie <alexh@vpitech.com> > --- > drivers/usb/serial/cp210x.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c > index 3bcec419f463..2c910550dca8 100644 > --- a/drivers/usb/serial/cp210x.c > +++ b/drivers/usb/serial/cp210x.c > @@ -1051,9 +1051,14 @@ static void cp210x_change_speed(struct tty_struct *tty, > * This maps the requested rate to the actual rate, a valid rate on > * cp2102 or cp2103, or to an arbitrary rate in [1M, max_speed]. > * > - * NOTE: B0 is not implemented. > + * NOTE: B0 is not implemented, apart from returning the baud rate to > + * the default of B9600. > */ > - baud = clamp(tty->termios.c_ospeed, priv->min_speed, priv->max_speed); > + if (tty->termios.c_ospeed) { > + baud = clamp(tty->termios.c_ospeed, priv->min_speed, priv->max_speed); > + } else { > + baud = 9600; > + } {} not needed at all... [...] MBR, Sergey
On Fri, Nov 25, 2022 at 08:58:25PM -0700, Alex Henrie wrote: > When a baud rate of 0 is requested, both the 8250 driver and the FTDI > driver reset the baud rate to the default of 9600 (see the comment above > the uart_get_baud_rate function). Some old versions of the NXP blhost > utility depend on this behavior. What exactly do you mean by "depend on" here? Setting B0 is supposed to hang up a modem connection by deasserting the modem control lines, but there's nothing mandating any particular line speed to be set in hardware. Why would that even matter? If the user space tool is thrown off by the fact that B0 isn't implemented, perhaps that's what should be addressed. Johan
On Saturday, November 26, 2022 12:10:07 AM MST Greg KH wrote: > No need for { } here (checkpatch.pl should have warned about this.) On Saturday, November 26, 2022 3:34:57 AM MST Sergey Shtylyov wrote: > {} not needed at all... Thanks for the feedback. I will send a new version after running checkpatch.pl, and in the future I'll try to remember to run checkpatch.pl over my patches before sending them. -Alex
On Monday, November 28, 2022 7:41:59 AM MST Johan Hovold wrote: > On Fri, Nov 25, 2022 at 08:58:25PM -0700, Alex Henrie wrote: > > When a baud rate of 0 is requested, both the 8250 driver and the FTDI > > driver reset the baud rate to the default of 9600 (see the comment above > > the uart_get_baud_rate function). Some old versions of the NXP blhost > > utility depend on this behavior. > > What exactly do you mean by "depend on" here? Setting B0 is supposed to > hang up a modem connection by deasserting the modem control lines, but > there's nothing mandating any particular line speed to be set in > hardware. Why would that even matter? > > If the user space tool is thrown off by the fact that B0 isn't > implemented, perhaps that's what should be addressed. Oh, it's definitely a bug in blhost. The program sets the baud rate to 0, then tries to communicate over the UART assuming that the baud rate is 9600. It's been fixed in the latest version of blhost, but I'm stuck on an old version and there's nothing I can do about that. I don't know why the 8250 and FTDI drivers map B0 to B9600, however it's very old behavior that must have had a purpose. Maybe Russell knows? Alternatively, leaving the baud rate unchanged seems like reasonable behavior and would also work with the old blhost. But mapping B0 to B300 makes even less sense than mapping it to B9600. -Alex
On Mon, Nov 28, 2022 at 11:08:25AM -0700, Alex Henrie wrote: > On Monday, November 28, 2022 7:41:59 AM MST Johan Hovold wrote: > > On Fri, Nov 25, 2022 at 08:58:25PM -0700, Alex Henrie wrote: > > > When a baud rate of 0 is requested, both the 8250 driver and the FTDI > > > driver reset the baud rate to the default of 9600 (see the comment above > > > the uart_get_baud_rate function). Some old versions of the NXP blhost > > > utility depend on this behavior. > > > > What exactly do you mean by "depend on" here? Setting B0 is supposed to > > hang up a modem connection by deasserting the modem control lines, but > > there's nothing mandating any particular line speed to be set in > > hardware. Why would that even matter? > > > > If the user space tool is thrown off by the fact that B0 isn't > > implemented, perhaps that's what should be addressed. > > Oh, it's definitely a bug in blhost. The program sets the baud rate to 0, then > tries to communicate over the UART assuming that the baud rate is 9600. It's > been fixed in the latest version of blhost, but I'm stuck on an old version and > there's nothing I can do about that. > > I don't know why the 8250 and FTDI drivers map B0 to B9600, however it's very > old behavior that must have had a purpose. Maybe Russell knows? What exactly do you think should be done when a baud rate of zero is requested? The fact of the matter is that at hardware level, the UART takes a clock, and divides that down. To get to a baud rate of zero, one would need an infinitely large divisor, which (a) is impossible to program into the hardware and (b) would trigger a divide-by-zero error in the kernel. So, we have to choose something. That decision was made before my time, when Ted Ts'o was maintaining what was the original serial.c 8250-based driver, and the behaviour he adopted was to set a baud rate of 9600 when B0 was requested, which is reasonable - 9600 baud is the default setting. POSIX (which is what we use to define the behaviour of the TTY layer, or at least what we _used_ to) doesn't specify the behaviour of B0 as the output rate, other than it shall cause the model control lines to be deasserted. What baud rate you get on the line is unspecified, and thus left up to the implementation. So basically, 9600 baud for B0 is the behaviour of the 8250 driver going back to the very early Linux versions and that has become the standard Linux implementation shared by a great many serial drivers. In effect, it's almost a de-facto standard.
On Monday, November 28, 2022 11:20:41 AM MST Russell King (Oracle) wrote: > What exactly do you think should be done when a baud rate of zero is > requested? I see two reasonable options: Leave the baud rate alone, or reset it to the default (i.e. 9600). In my opinion, either of those options is just fine. > The fact of the matter is that at hardware level, the UART takes a > clock, and divides that down. To get to a baud rate of zero, one > would need an infinitely large divisor, which (a) is impossible to > program into the hardware and (b) would trigger a divide-by-zero > error in the kernel. So, we have to choose something. > > That decision was made before my time, when Ted Ts'o was maintaining > what was the original serial.c 8250-based driver, and the behaviour > he adopted was to set a baud rate of 9600 when B0 was requested, > which is reasonable - 9600 baud is the default setting. > > POSIX (which is what we use to define the behaviour of the TTY layer, > or at least what we _used_ to) doesn't specify the behaviour of B0 > as the output rate, other than it shall cause the model control lines > to be deasserted. What baud rate you get on the line is unspecified, > and thus left up to the implementation. > > So basically, 9600 baud for B0 is the behaviour of the 8250 driver > going back to the very early Linux versions and that has become the > standard Linux implementation shared by a great many serial drivers. > In effect, it's almost a de-facto standard. That is really interesting, thanks for the explanation! I like the idea of having consistent behavior across the Linux serial drivers, so it seems to me that mapping B0 to B9600 in all drivers is the way to go. -Alex
On Mon, Nov 28, 2022 at 11:08:25AM -0700, Alex Henrie wrote: > On Monday, November 28, 2022 7:41:59 AM MST Johan Hovold wrote: > > On Fri, Nov 25, 2022 at 08:58:25PM -0700, Alex Henrie wrote: > > > When a baud rate of 0 is requested, both the 8250 driver and the FTDI > > > driver reset the baud rate to the default of 9600 (see the comment above > > > the uart_get_baud_rate function). Some old versions of the NXP blhost > > > utility depend on this behavior. > > > > What exactly do you mean by "depend on" here? Setting B0 is supposed to > > hang up a modem connection by deasserting the modem control lines, but > > there's nothing mandating any particular line speed to be set in > > hardware. Why would that even matter? > > > > If the user space tool is thrown off by the fact that B0 isn't > > implemented, perhaps that's what should be addressed. > > Oh, it's definitely a bug in blhost. The program sets the baud rate to 0, then > tries to communicate over the UART assuming that the baud rate is 9600. It's > been fixed in the latest version of blhost, but I'm stuck on an old version and > there's nothing I can do about that. > > I don't know why the 8250 and FTDI drivers map B0 to B9600, however it's very > old behavior that must have had a purpose. Heh, not everything has a purpose even if it's old. For implementation and protocol reasons a driver may need to pick some arbitrary speed to use for B0, but this is generally not needed for USB serial devices and only about half of the drivers do so (and then tend to pick 9600). Also note that the FTDI driver does in fact leave the line speed unchanged when B0 is requested (that zero-baud check in get_ftdi_divisor() is only used for ASYNC_SPD_CUST). > Maybe Russell knows? Alternatively, > leaving the baud rate unchanged seems like reasonable behavior and would also > work with the old blhost. But mapping B0 to B300 makes even less sense than > mapping it to B9600. Your application really should not depend on any particular hardware setting after requesting B0. That said, I've implemented support for B0 in cp210x which leaves the current settings unchanged (and which incidentally allows you to use the buggy tool). Johan
On Tuesday, November 29, 2022 7:15:08 AM MST Johan Hovold wrote: > Also note that the FTDI driver does in fact leave the line speed > unchanged when B0 is requested (that zero-baud check in > get_ftdi_divisor() is only used for ASYNC_SPD_CUST). I tested it empirically today, and you're right, the FTDI driver simply does not change the baud rate. Thank you for correcting my misunderstanding. > Your application really should not depend on any particular hardware > setting after requesting B0. Understood. > That said, I've implemented support for B0 in cp210x which leaves the > current settings unchanged (and which incidentally allows you to > use the buggy tool). Excellent! Many thanks! -Alex
diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c index 3bcec419f463..2c910550dca8 100644 --- a/drivers/usb/serial/cp210x.c +++ b/drivers/usb/serial/cp210x.c @@ -1051,9 +1051,14 @@ static void cp210x_change_speed(struct tty_struct *tty, * This maps the requested rate to the actual rate, a valid rate on * cp2102 or cp2103, or to an arbitrary rate in [1M, max_speed]. * - * NOTE: B0 is not implemented. + * NOTE: B0 is not implemented, apart from returning the baud rate to + * the default of B9600. */ - baud = clamp(tty->termios.c_ospeed, priv->min_speed, priv->max_speed); + if (tty->termios.c_ospeed) { + baud = clamp(tty->termios.c_ospeed, priv->min_speed, priv->max_speed); + } else { + baud = 9600; + } if (priv->use_actual_rate) baud = cp210x_get_actual_rate(baud); @@ -1069,7 +1074,8 @@ static void cp210x_change_speed(struct tty_struct *tty, baud = 9600; } - tty_encode_baud_rate(tty, baud, baud); + if (tty->termios.c_ospeed) + tty_encode_baud_rate(tty, baud, baud); } static void cp210x_enable_event_mode(struct usb_serial_port *port)
When a baud rate of 0 is requested, both the 8250 driver and the FTDI driver reset the baud rate to the default of 9600 (see the comment above the uart_get_baud_rate function). Some old versions of the NXP blhost utility depend on this behavior. However, the CP210x driver resets the baud rate to the minimum supported rate of 300. Special-case B0 so that it returns the baud rate to the more sensible default of 9600. Signed-off-by: Alex Henrie <alexh@vpitech.com> --- drivers/usb/serial/cp210x.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)