
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.