| Message ID | 20220924102718.2984-6-pali@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ftdi_sio driver improvements | expand |
On Sat, Sep 24, 2022 at 12:27:16PM +0200, Pali Rohár wrote: > To compute more accurate baud rate when user uses ASYNC_SPD_CUST API, > use DIV_ROUND_CLOSEST() instead of just rounding down. > > Rationale: > Application uses old API, so it computes divisor D for baud rate B. s/old/deprecated/ > The driver then tries to compute back the requested baud rate B, but > rounds down in the division. > > Using rounding to closest value instead should increate accuracy here. typo: increase > Signed-off-by: Pali Rohár <pali@kernel.org> > Tested-by: Marek Behún <kabel@kernel.org> > Signed-off-by: Marek Behún <kabel@kernel.org> > --- > drivers/usb/serial/ftdi_sio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c > index 1ab6bf48516f..718c86db2900 100644 > --- a/drivers/usb/serial/ftdi_sio.c > +++ b/drivers/usb/serial/ftdi_sio.c > @@ -1333,7 +1333,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, > if (baud == 38400 && > ((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST) && > (priv->custom_divisor)) { > - baud = priv->baud_base / priv->custom_divisor; > + baud = DIV_ROUND_CLOSEST(priv->baud_base, priv->custom_divisor); > dev_dbg(dev, "%s - custom divisor %d sets baud rate to %d\n", > __func__, priv->custom_divisor, baud); > } I'm having second thoughts about this one. The SPD_CUST hack should not be used anymore, but it was supposed to be used to set the hardware divisor directly. Someone was creative and reinterpreted this for for the FTDI driver to mean software divisor instead. So instead of understanding how the hardware determines rates from divisors, you know needed knowledge of how the FTDI driver happens to work, including that it rounds down. And now you're changing that again. Perhaps we should just leave this as is. This interface has been deprecated for decades and comes with a deprecated warning since several years. Would be nice to drop it completely instead. Johan
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index 1ab6bf48516f..718c86db2900 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -1333,7 +1333,7 @@ static u32 get_ftdi_divisor(struct tty_struct *tty, if (baud == 38400 && ((priv->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST) && (priv->custom_divisor)) { - baud = priv->baud_base / priv->custom_divisor; + baud = DIV_ROUND_CLOSEST(priv->baud_base, priv->custom_divisor); dev_dbg(dev, "%s - custom divisor %d sets baud rate to %d\n", __func__, priv->custom_divisor, baud); }