| Message ID | 20201210140313.258739-3-damien.lemoal@wdc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | None | expand |
On Thu, 10 Dec 2020 06:02:53 PST (-0800), Damien Le Moal wrote: > Setup the port uartclk in sifive_serial_probe() so that the base baud > rate is correctly printed during device probe instead of always showing > "0". I.e. the probe message is changed from > > 38000000.serial: ttySIF0 at MMIO 0x38000000 (irq = 1, > base_baud = 0) is a SiFive UART v0 > > to the correct: > > 38000000.serial: ttySIF0 at MMIO 0x38000000 (irq = 1, > base_baud = 115200) is a SiFive UART v0 > > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > --- > drivers/tty/serial/sifive.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c > index 13eadcb8aec4..214bf3086c68 100644 > --- a/drivers/tty/serial/sifive.c > +++ b/drivers/tty/serial/sifive.c > @@ -999,6 +999,7 @@ static int sifive_serial_probe(struct platform_device *pdev) > /* Set up clock divider */ > ssp->clkin_rate = clk_get_rate(ssp->clk); > ssp->baud_rate = SIFIVE_DEFAULT_BAUD_RATE; > + ssp->port.uartclk = ssp->baud_rate * 16; > __ssp_update_div(ssp); > > platform_set_drvdata(pdev, ssp); Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com> As an aside: is this an intrinsic property of being a serial port, or specific to the SiFive driver? We seem to multiply/divide by 16 quite a bit to convert between baud rates and clocks, so if it's specific to SiFive then it seems reasonable to name that constant in this driver. If it's a serial thing then I guess we should just do whatever everyone else is doing. Don't think that blocks this patch, though, as it's all over the place.
On Thu, 2020-12-10 at 18:09 -0800, Palmer Dabbelt wrote: > On Thu, 10 Dec 2020 06:02:53 PST (-0800), Damien Le Moal wrote: > > Setup the port uartclk in sifive_serial_probe() so that the base baud > > rate is correctly printed during device probe instead of always showing > > "0". I.e. the probe message is changed from > > > > 38000000.serial: ttySIF0 at MMIO 0x38000000 (irq = 1, > > base_baud = 0) is a SiFive UART v0 > > > > to the correct: > > > > 38000000.serial: ttySIF0 at MMIO 0x38000000 (irq = 1, > > base_baud = 115200) is a SiFive UART v0 > > > > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > > --- > > drivers/tty/serial/sifive.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c > > index 13eadcb8aec4..214bf3086c68 100644 > > --- a/drivers/tty/serial/sifive.c > > +++ b/drivers/tty/serial/sifive.c > > @@ -999,6 +999,7 @@ static int sifive_serial_probe(struct platform_device *pdev) > > /* Set up clock divider */ > > ssp->clkin_rate = clk_get_rate(ssp->clk); > > ssp->baud_rate = SIFIVE_DEFAULT_BAUD_RATE; > > + ssp->port.uartclk = ssp->baud_rate * 16; > > __ssp_update_div(ssp); > > > > platform_set_drvdata(pdev, ssp); > > Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com> > Acked-by: Palmer Dabbelt <palmerdabbelt@google.com> > > As an aside: is this an intrinsic property of being a serial port, or specific > to the SiFive driver? We seem to multiply/divide by 16 quite a bit to convert > between baud rates and clocks, so if it's specific to SiFive then it seems > reasonable to name that constant in this driver. If it's a serial thing then I > guess we should just do whatever everyone else is doing. That port.uartclk field is not specific to the sifive driver. And setting its value correctly lead to the correct message being printed. That message is also generic and not special to the SiFive serial driver. > Don't think that blocks this patch, though, as it's all over the place. I did not dig too far in that driver, I only wanted to fix the message so that meaningful information is printed. But yes, I also thought that a little cleanup would be good. The DT binding doc is also incomplete as it is missing the "sifive,fu540-c000-uart0" compatible string. -- Damien Le Moal Western Digital
diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c index 13eadcb8aec4..214bf3086c68 100644 --- a/drivers/tty/serial/sifive.c +++ b/drivers/tty/serial/sifive.c @@ -999,6 +999,7 @@ static int sifive_serial_probe(struct platform_device *pdev) /* Set up clock divider */ ssp->clkin_rate = clk_get_rate(ssp->clk); ssp->baud_rate = SIFIVE_DEFAULT_BAUD_RATE; + ssp->port.uartclk = ssp->baud_rate * 16; __ssp_update_div(ssp); platform_set_drvdata(pdev, ssp);
Setup the port uartclk in sifive_serial_probe() so that the base baud rate is correctly printed during device probe instead of always showing "0". I.e. the probe message is changed from 38000000.serial: ttySIF0 at MMIO 0x38000000 (irq = 1, base_baud = 0) is a SiFive UART v0 to the correct: 38000000.serial: ttySIF0 at MMIO 0x38000000 (irq = 1, base_baud = 115200) is a SiFive UART v0 Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> --- drivers/tty/serial/sifive.c | 1 + 1 file changed, 1 insertion(+)