Re: [PATCH v5] OMAP UART: Add omap-serial driver support.

Olof Johansson
2010-02-04T20:34:26Z

On Thu, Feb 04, 2010 at 09:46:40AM -0800, Kevin Hilman wrote:
> "Govindraj.R" <govindraj.raja@ti.com> writes:
>
> [...]
>
> >>> >> + * @baud: baudrate for which divisor needs to be calculated.
> >>> >> + *
> >>> >> + * We have written our own function to get the divisor so as to support
> >>> >> + * 13x mode.
> >>> >> + */
> >>> >
> >>> > Again, the why, not the how. Why do you need the 13x divisor? What's
> >>> > magic about 3Mbaud?
> >>> >
> >>>
> >>> Refering to TRM UART chapter 17:
> >>>
> >>> Table 17-1. UART Mode Baud Rates, Divisor Values, and Error Rates
> >>>
> >>> referring to oversampling - divisor value
> >>>
> >>> baudrate 460,800 to 3,686,400 all have divisor 13
> >>>
> >>> except 3,000,000 which has divisor value 16
> >>>
> >>> thus we are checking if baud != 3000000
> >>
> >> Ok. It's always useful to have just a bit of information in the driver
> >> so you don't have to search around the manual when trying to figure out
> >> why something was done.
> >>
> >> Maybe something simple: "3Mbaud is unique in that it requires a divisor
> >> of 13. See the TRM for full details".
> >
> > ok. will add that comments.
>
> Referncing TRM Chapter and Section names (and/or numbers) is very
> helpful too. Usually section names are more helpful since numbers
> change w/TRM versions.

Yep! Rule of thumb: Put yourself in the seat of someone that has to debug this
driver without having memorized the TRM for the module, and try to make their
work as simple as possible (to certain limits).

Which in other words means reducing the number of "Huh? Why is that
there?" questions to a minimum. :)

-Olof