From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Denk Date: Mon, 11 Aug 2008 12:06:55 +0200 Subject: [U-Boot] [PATCH 5/7 v6] serial: add S3C64XX serial driver In-Reply-To: References: <20080809091323.GD18040@game.jcrosoft.org> Message-ID: <20080811100655.B23AD248BF@gemini.denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Guennadi Liakhovetski, In message you wrote: > > > > + u32 reg; > > > + u32 pclk_ratio = get_PCLK() / gd->baudrate; > > > + int i; > > > + > > IMHO it's still obscur > > > + /* PCLK / (16 * baudrate) - 1 */ > > > + reg = pclk_ratio / 16 - 1; I don't see any baudrate in this calculation. From the comment I would expect to see reg = pclk_ratio / (16 * gd->baudrate) - 1; or similar. For the reader the comment is misleading as it is difficult to figure out that "PCLK" and "pclk_ratio" are not the same thing. > > > + /* i = pclk_ratio % 16 */ > > > + i = pclk_ratio - (reg + 1) * 16; I don't see any % operation in this calculation. From the comment I would expect to see i = pclk_ratio % 16; or similar. Either the comment or the code or both are wrong. > Sorry, I don't understand how this can be made clearer yet. If you have > any specific ideas, please let me know now, before v7 appears, because I > would really like to make it final. Well, maybe the code and the comments should be in sync. Actually, if the comment just says the same as the code, it is redundant and should be omitted. If the comment tries to describe what the code implements in a different way that should be made clear, too. Probably it doesn't make much sense to try to factor out common expressions when it makes thew code more difficult to read - the compiler is pretty clever about such things, too. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "Though a program be but three lines long, someday it will have to be maintained." - The Tao of Programming