[U-Boot] [PATCH] Move ICS CLK chip frequenty calculation code into a common board library

Timur Tabi
2010-05-21T19:58:23Z

Wolfgang Denk wrote:

>> So here's a better version of that function that rounds to the nearest
>> MHz and is of a proper coding style:
>
> Why do we need that?

Um, because you complained about it?

>> > +static unsigned long ics307_clk_freq(unsigned char cw0, unsigned char cw1,
>> > + unsigned char cw2)
>> > +{
>> > + const unsigned long InputFrequency = CONFIG_ICS307_REFCLK_HZ;
>> > + unsigned long VDW = ((cw1 << 1) & 0x1FE) + ((cw2 >> 7) & 1);
>> > + unsigned long RDW = cw2 & 0x7F;
>> > + unsigned long OD = ics307_S_to_OD[cw0 & 0x7];
>> > + unsigned long freq;

> Please do not use any CamelCase or UPPER CASE identifiers.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Also, because this is silly:

Clock Configuration:
CPU0:799.992 MHz, CPU1:799.992 MHz,
CCB:399.996 MHz,
DDR:299.997 MHz (599.994 MT/s data rate) (Asynchronous), LBC:25 MHz

Why display 799.992 MHZ when 800 MHz makes more sense?

>> Clock Configuration:
>> CPU0:800 MHz, CPU1:800 MHz,
>> CCB:400 MHz,
>> DDR:300 MHz (600 MT/s data rate) (Asynchronous), LBC:25 MHz
>
> The result looks ugly (why do we have double spaces after the
> numbers?, why do the numbers not align vertically?).

> This makes me wonder why you use a "%-4s" format in
> arch/powerpc/cpu/mpc8?xx/cpu.c - may I recommend changing this into
> "%s" (if you don't care about vertical alignment), or something like
> "%4s" else?

I'm okay with that.

--
Timur Tabi
Linux kernel developer at Freescale