On Fri, 2018-01-19 at 16:40 +0000, Ed Blake wrote:
> On 19/01/18 16:02, Andy Shevchenko wrote:
> > The commit
> >
> > de9e33bdfa22 ("serial: 8250_dw: Improve clock rate setting")
> >
> > obviously tries to cure symptoms, and not a root cause.
> >
> > The root cause is the non-flexible rate calculation inside the
> > corresponding clock driver.
>
> The issue is the clock generating hardware in this particular platform
> is only capable of generating finite clock rates from a fixed
> divider.
Which platform? Is it in upstream? What the driver handles clock for it?
(If it's not yet upstream, it's one more point to revert, btw.)
> So when dw8250_set_termios() requests baud * 16 it may get something
> quite different - e.g. it requests 1.8432MHz for a baud rate of
> 115,200
> and gets back 3MHz from clk_round_rate() as this is the nearest
> achievable. Before de9e33bdfa22, dw8250_set_termios() then sets the
> clock to this rate which obviously doesn't work.
I understand that and agree this is an issue, but I object the approach
how you "fixed" it.
> > What we need is to provide maximum UART
> > divisor value to the clock driver to allow it do the job
> > transparently
> > to the caller.
>
> So the clock driver should know that this clock is attached to a UART
> with a fixed divider, and when 1.8432MHz is requested in the above
> example it returns say 24MHz from clk_round_rate() because it knows
> that
> the UART can divide this down to something within an acceptable range
> of
> the requested rate for correct operation?
Roughly speaking, AFAIU situation, you need your clock driver be based
on clk-divider with appropriate input / platform data.
If clk-divider is not good enough, then, derive something based on it,
or expand existing one.
--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
