On Wed, Dec 25, 2019 at 09:30:05PM +0800, Charles Yeh wrote:
> PL2303HXN (TYPE_HXN) can program form 1 bps to 12000000 bps and
> support standard & non-standard baud rates (Note 1) are set directly
> It doesn't need complicated baud rate division calculation.
What's the benefit of this over using divisors directly?
If there's something wrong with that algorithm I think we should fix
that instead so that all device types benefit.
> Note 1:
> Standard baud rate:
> 75, 150, 300, 600, 1200, 1800, 2400, 3600, 4800, 7200, 9600,14400,
> 19200, 28800, 38400, 57600, 115200, 230400, 460800,614400, 921600,
> 1228800, 2457600, 3000000, 6000000
>
> Non-standard baud rate (1 ~ 12000000):
> 1, 2, 5, 22, 55, 60, 75, 80, 123, 130, 150, 180, 187, 200, 300, 340,
> 400,..... 115200, 230400, 460800, 474747, 515151, 614400, 921600,
> .. 1000000,.. 7000000,.. 12000000
Which rate does HXN pick if it's not in either of these lists (e.g.
next lower/higher, etc)?
> Signed-off-by: Charles Yeh <charlesyeh522@xxxxxxxxx>
> ---
> ---
> drivers/usb/serial/pl2303.c | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index aab737e1e7b6..63d354a92db9 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -565,17 +565,21 @@ static void pl2303_encode_baud_rate(struct tty_struct *tty,
> if (!baud)
> return;
>
> - if (spriv->type->max_baud_rate)
> - baud = min_t(speed_t, baud, spriv->type->max_baud_rate);
You still need this for HXN I guess.
> - /*
> - * Use direct method for supported baud rates, otherwise use divisors.
> - */
> - baud_sup = pl2303_get_supported_baud_rate(baud);
> -
> - if (baud == baud_sup)
> + if (spriv->type == &pl2303_type_data[TYPE_HXN]) {
> baud = pl2303_encode_baud_rate_direct(buf, baud);
> - else
> - baud = pl2303_encode_baud_rate_divisor(buf, baud);
> + } else {
> + if (spriv->type->max_baud_rate)
> + baud = min_t(speed_t, baud, spriv->type->max_baud_rate);
> + /*
> + * Use direct method for supported baud rates, otherwise use divisors.
> + */
> + baud_sup = pl2303_get_supported_baud_rate(baud);
> +
> + if (baud == baud_sup)
> + baud = pl2303_encode_baud_rate_direct(buf, baud);
> + else
> + baud = pl2303_encode_baud_rate_divisor(buf, baud);
> + }
You need to find a better way to abstract this. We can't just add
conditionals like this throughout the code without reorganising it or it
will become very hard to read and maintain.
>
> /* Save resulting baud rate */
> tty_encode_baud_rate(tty, baud, baud);
Johan
