On Fri, 2018-01-19 at 17:11 +0000, Ed Blake wrote: > On 19/01/18 17:02, Andy Shevchenko wrote: > > On Fri, 2018-01-19 at 16:40 +0000, Ed Blake wrote: > > > On 19/01/18 16:02, Andy Shevchenko wrote: > > 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.) > > It's not upstream. Oh, please, consider Ack this revert then. > > > 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. > > That's fine, I'm open to other solutions. I would be constructive whenever we would see a real clk driver for that platform. > > > > 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. > > > > But fundamentally you are saying that the clock driver should have > knowledge that this particular clock is connected to a UART which > requires a clock rate that is within a defined range (say +/- 2%) of > an > integer multiple of the requested rate. Nope. Fundamentally I'm saying that the clock driver should take additional parameters, like maximum available chained divider, and it's properties. If you wish, you can also add a margin property to a set. In any case it would have not known what kind of the device is connected, just some of properties of theirs prescaler. > I don't really like this approach as in my view the clock driver > should > only have knowledge of the clock generating hardware and not the > internals of the devices it's connected to. Yep. That's why I'm proposing an approach that would work not only for UART and not only for your very platform. Because we might need on some platforms to use that knowledge as well (SPI comes to my mind as a possible user). So, it might be beneficial for some of the users. -- 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
