public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
From: Olof Johansson <olof@lixom.net>
To: Kevin Hilman <khilman@deeprootsystems.com>
Cc: "Govindraj.R" <govindraj.raja@ti.com>, linux-omap@vger.kernel.org
Subject: Re: [PATCH v5] OMAP UART: Add omap-serial driver support.
Date: Thu, 4 Feb 2010 14:37:45 -0600	[thread overview]
Message-ID: <20100204203744.GC711@lixom.net> (raw)
In-Reply-To: <87iqac51xb.fsf@deeprootsystems.com>

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

  reply	other threads:[~2010-02-04 20:34 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-27  9:56 [PATCH v5] OMAP UART: Add omap-serial driver support Govindraj.R
2010-01-27 17:25 ` Tony Lindgren
2010-01-27 17:49   ` Kevin Hilman
2010-01-27 17:57     ` Tony Lindgren
2010-01-27 18:07       ` Kevin Hilman
2010-01-27 18:18         ` Tony Lindgren
2010-01-27 19:10           ` Kevin Hilman
2010-01-27 19:33             ` Tony Lindgren
2010-02-10 14:39   ` Govindraj
2010-02-10 17:25     ` Tony Lindgren
2010-01-27 18:20 ` Kevin Hilman
2010-01-27 19:37   ` Kevin Hilman
2010-01-28  3:22 ` Olof Johansson
2010-02-04 15:09   ` Govindraj.R
2010-02-04 15:28     ` Olof Johansson
2010-02-04 15:45       ` Govindraj.R
2010-02-04 17:46         ` Kevin Hilman
2010-02-04 20:37           ` Olof Johansson [this message]
2010-02-04 20:33         ` Olof Johansson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100204203744.GC711@lixom.net \
    --to=olof@lixom.net \
    --cc=govindraj.raja@ti.com \
    --cc=khilman@deeprootsystems.com \
    --cc=linux-omap@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox