On Tue, Mar 03, 2015 at 02:08:45PM -0500, Peter Hurley wrote:
> On 03/03/2015 05:11 AM, James Hogan wrote:
> > Hi Peter,
> >
> > On Mon, Mar 02, 2015 at 01:37:07PM -0500, Peter Hurley wrote:
> >> [ +cc common clock maintainers ]
> >>
> >> On 02/26/2015 12:34 PM, James Hogan wrote:
> >>> Hi Peter,
> >>>
> >>> On 26/02/15 16:54, Peter Hurley wrote:
> >>>> On 02/26/2015 09:44 AM, James Hogan wrote:
> >>>>> On 26/02/15 13:45, Peter Hurley wrote:
> >>>>>> Hi James,
> >>>>>>
> >>>>>> On 02/26/2015 06:25 AM, James Hogan wrote:
> >>>>>>> Hi Sebastian,
> >>>>>>>
> >>>>>>> On 26/02/15 08:51, Sebastian Andrzej Siewior wrote:
> >>>>>>>> On 02/26/2015 01:21 AM, James Hogan wrote:
> >>>>>>>>> When the UART clock is set slightly under 1.8432MHz, the 8250 driver
> >>>>>>>>> core doesn't permit the 115200 baud rate since it calculates the maximum
> >>>>>>>>> frequency to pass to uart_get_baud_rate by simply dividing the uart
> >>>>>>>>> clock by 16 which yields a value slightly under 115200, even though the
> >>>>>>>>> frequency is close enough for the UART to operate reliably.
> >>>>>>>>>
> >>>>>>>>> Therefore add some tolerance in the calculation of the maximum baud
> >>>>>>>>> rate, specifically +1/128th (~0.8%), to allow the 115200 baud rate to
> >>>>>>>>> be closen for a UART clock as low as 1.8149MHz. For an external divider
> >>>>>>>>> set as close as possible to 1.8432MHz, this should cover every possible
> >>>>>>>>> input frequency above 118.9MHz.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx>
> >>>>>>>>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> >>>>>>>>> Cc: Jiri Slaby <jslaby@xxxxxxx>
> >>>>>>>>> Cc: Anton Vorontsov <avorontsov@xxxxxxxxxxxxx>
> >>>>>>>>> Cc: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
> >>>>>>>>> Cc: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> >>>>>>>>> Cc: linux-serial@xxxxxxxxxxxxxxx
> >>>>>>>>> ---
> >>>>>>>>> As far as I can tell from reading the link below, this tolerance should
> >>>>>>>>> be okay, and it definitely covers the range of input frequencies I
> >>>>>>>>> expect for my particular hardware. I'm open to better ways of handling
> >>>>>>>>> it though.
> >>>>>>>>
> >>>>>>>> What is your hardware?
> >>>>>>>
> >>>>>>> SoC is TZ1090 (metag based). UART clock is best derived from system
> >>>>>>> clock which would normally be somewhere between 180MHz and 240MHz, using
> >>>>>>> an integer divider.
> >>>>>>>
> >>>>>>>> (1814900 + 1814900 / 128) / 16
> >>>>>>>> 114317
> >>>>>>>> It is not 115200 but close enough to get you the 115200 baud rate. It
> >>>>>>>> might make sense to increase the tolerance rather than increasing the
> >>>>>>>> the clock but it is up to Peter / Alan to decide.
> >>>>>>>
> >>>>>>> One random example configuration I have here is:
> >>>>>>>
> >>>>>>> # cat /sys/kernel/debug/clk/clk_summary
> >>>>>>> clock enable_cnt prepare_cnt rate
> >>>>>>> --------------------------------------------------------------------
> >>>>>>> xtal1 2 2 24576000
> >>>>>>> sys_sw 1 1 24576000
> >>>>>>> sys_pll 1 1 719965090
> >>>>>>> sys_div 1 1 719965090
> >>>>>>> sys_x2_undeleted 1 1 719965090
> >>>>>>> sys_undeleted 2 2 179991273
> >>>>>>> uart_sw 1 1 179991273
> >>>>>>> uart_en 1 1 179991273
> >>>>>>> uart 1 1 1836646
> >>>>>>>
> >>>>>>> sys_undeleted divide uart baud error
> >>>>>>> 179991273 97 1855580 115973 774 = 0.67%
> >>>>>>> 179991273 98 1836646 114790 -410 = -0.36% (chosen)
> >>>>>>>
> >>>>>>> At this sys_undeleted frequency, the difference between two consecutive
> >>>>>>> divider values is about 1183 Hz.
> >>>>>>> For round to closest, the max error is about 591.5 Hz = 0.513%.
> >>>>>>> For round up (i.e. increasing the clock), the max error is about 1183 Hz
> >>>>>>> = 1.03%.
> >>>>>>>
> >>>>>>> So my reasoning is that rounding to closest and fixing 8250 driver to
> >>>>>>> accept it and getting as accurate a baud rate as possible is better than
> >>>>>>> having a less accurate baud rate and having to add a new rounding mode
> >>>>>>> to common clock muxes and dividers.
> >>>>>>>
> >>>>>>> Note that AFAICT for faster input clocks the 8250 driver would accept a
> >>>>>>> 115200 baud rate regardless of how inaccurate the resulting baud rate
> >>>>>>> would be after division, so arguably the tolerance could be much higher.
> >>>>>>>
> >>>>>>> The serial core also calculates the divider with DIV_ROUND_CLOSEST, so
> >>>>>>> there is precedent for using that rounding mode right up through the
> >>>>>>> clock tree.
> >>>>>>
> >>>>>> Why clock the uart so low?
> >>>>>
> >>>>> I tried forcing uart = sys_undeleted / 1 a couple of years ago when
> >>>>> switching to common clock & DT, relying instead on the divider internal
> >>>>> to the UART, but it didn't work for everybody. It was presumed at the
> >>>>> time that the UART input was effectively being overclocked between the
> >>>>> divider and the uart so wasn't reliable at higher frequencies.
> >>>>
> >>>> 180 Mhz uart clock :)
> >>>>
> >>>>> The hardware manual lists Fmax/ideal as 25MHz, but I didn't want to set
> >>>>> it to a specific value such as 25MHz as the combination of the 2
> >>>>> dividers (the one in the uart block and the external one in the SoC)
> >>>>> will reduce the precision that can be attained.
> >>>>>
> >>>>> So that really only leaves the option of aiming for the right uart
> >>>>> frequency with the external divider and using an internal divide by 1.
> >>>>
> >>>> Dividing down the sys clock by 14 and the uart clock by 7 yields an
> >>>> identical baud rate of 114790 (as does dividing the sys clock by 7
> >>>> but that sets the uart clock to 25.713 Mhz which may be too high for
> >>>> the IP block).
> >>>>
> >>>> Also, dividing sys clock by 49 and uart clock by 2 = 114790
> >>>
> >>> Sure, but it doesn't generalise very easily. The system/core/DDR clocks
> >>> are statically configured by the bootloader depending on a whole bunch
> >>> of board specifics (particularly for the DDR). I.e. it can be virtually
> >>> anything (including prime number multiples of 115200*16). Linux then
> >>> configures the rest of the clocks as it needs, using the common clock
> >>> framework. This would require adding knowledge of max intermediate clock
> >>> rates to common clock,
> >>
> >> The common clock framework will need to support this anyway, because a
> >> 2-level clock tree can't possibly work with UART, ADC, GPU, etc. off a
> >> single common clock without independent intermediate stages (maybe for
> >> your limited application it does, but it doesn't generalize to other
> >> designs).
> >>
> >>> and adding the UART internal divider into the decision making,
> >>
> >> Constrained dividers are already supported by many clock drivers.
> >>
> >>> i.e. the UART driver trying to set a bunch of rates,
> >>> multiples of 115200*16 for each internal divider value, and seeing which
> >>> gives the best baud rate, which would be even more complicated, and its
> >>> all to appease an arguably nonsensical constraint in the driver.
> >>
> >> The true constraint you're trying to workaround is that the common clock
> >> framework doesn't adequately support your use-case yet. But that's where
> >> your requirement needs to be solved.
> >>
> >> ISTM that the clock api should provide helpers so the intermediate parent
> >> clock can compute the best-fit clock rate given the constraints of
> >> the input sys clock rate, the {min,max} intermediate clock rate range,
> >> and the desired sample rate multiple of the clock consumer.
> >
> > Even with those other constraints fully implemented, is it really a
> > correct constraint that the uart input clock must be strictly >=
> > 115200*16? A value under this may give the closest baud rate even taking
> > all other constraints into account.
>
> I spent some time last night experimenting with a simple brute-force
> solver.
>
> And while there are input clock rates for which the best-fit output clock is
> < 115200 * 16 (for that target rate or its multiples), for the same input
> clock rate there always seems to be faster multiples within some degree
> of error, although not as low as you initially proposed.
>
> For example, an input clock = 152785600 has a best-fit output clock =
> 1840790 (div=83, baud=115049, error=0.13%). That same input clock rate
> has fastest multiples with div=82 and div=41 with a error=1.3%
>
> However, the error limit you initially proposed (as you noted in the
> commit log) is only suitable for input clock rates above 118Mhz; lower
> clock rates would require larger error limits.
>
> I plan to experiment further on the actual effects of a slow uart clock
> and a suitable error limit, but any further information you can provide
> wrt sample error might be helpful.
My only other comment is that the overall error tolerance is shared
with the other end. The following article suggests overall 2% error
would be pushing it for a "nasty" case of only sampling accurately 50%
of the bit time (e.g. a long capacitive connection, though I don't know
how realistic that case is). So if one end is 1.3% fast as in your
example, the other end only needs to be 0.7% slow for it to be a
problem.
http://www.maximintegrated.com/en/app-notes/index.mvp/id/2141
This was my rationale for wanting round to closest throughout the
calculation.
Thanks
James
>
> Regards,
> Peter Hurley
>
>
> >>> At the moment I just need this patch and the following added in the uart
> >>> DT node, and it should basically work for all system clock rates I care
> >>> about:
> >>>
> >>> assigned-clocks = <&top_clks CLK_TOP_UART>;
> >>> assigned-clock-rates = <1843200>;
> >>>
> >>> Cheers
> >>> James
> >>>
> >>
>
Attachment:
signature.asc
Description: Digital signature
