Re: [PATCH] serial: 8250_dw: verify clock rate in dw8250_set_termios

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Jan 11, 2022 at 05:28:47PM +0400, asheplyakov@basealt.ru wrote:
> From: Alexey Sheplyakov <asheplyakov@basealt.ru>

Thanks for your patch, my comments below.

> Refuse to change the clock rate if clk_round_rate() returns
> a rate which is way too off (i.e. by more than 1/16 from the one
> necessary for a given baud rate). In particular this happens if
> the requested rate is below the minimum supported by the clock.
> 
> Fixes the UART console on BE-M1000 SoC. Without this patch the
> console gets garbled immediately after loading the driver.
> dw8250_set_termios tries to configure the baud rate (115200),
> and calls clk_round_rate to figure out the supported rate closest
> to 1843200 Hz (which is 115200 * 16). However the (SoC-specific)
> clock driver returns 4705882 Hz. This frequency is way too off,
> hence after setting it the console gets garbled.

So, the root cause is to understand _why_ the clock provider is uncapable
to fulfil the request. Any investigation has been conducted?

> Signed-off-by: Alexey Sheplyakov <asheplyakov@basealt.ru>
> Signed-off-by: Vadim V. Vlasov <vadim.vlasov@elpitech.ru>

> 

Should be no blank lines in the tag block, but...

> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>

...I recommend to use --cc parameter to git send-email instead of polluting
commit message.

...

> -	if (rate > 0) {
> +	if (rate > 0 && rate >= baud * 15 && rate <= baud * 17) {

It doesn't fell like a correct fix.

-- 
With Best Regards,
Andy Shevchenko