Dear Guennadi Liakhovetski,
In message Pine.LNX.4.64.0808111042400.30591@axis700.grange 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