All of lore.kernel.org
 help / color / mirror / Atom feed
From: Timur Tabi <timur@freescale.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] Move ICS CLK chip frequenty calculation code into a common board library
Date: Fri, 21 May 2010 14:58:23 -0500	[thread overview]
Message-ID: <4BF6E5DF.5020306@freescale.com> (raw)
In-Reply-To: <20100521195233.A5156CCF026@gemini.denx.de>

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

  reply	other threads:[~2010-05-21 19:58 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-21  9:18 [U-Boot] [PATCH] Move ICS CLK chip frequenty calculation code into a common board library Kumar Gala
2010-05-21  9:18 ` [U-Boot] [PATCH] ppc/85xx: Convert MPC8572DS to using board common ICS307 code Kumar Gala
2010-05-21  9:18   ` [U-Boot] [PATCH] ppc/85xx: Convert MPC8536DS " Kumar Gala
2010-05-21 15:57 ` [U-Boot] [PATCH] Move ICS CLK chip frequenty calculation code into a common board library Timur Tabi
2010-05-21 22:19   ` Kumar Gala
2010-05-21 22:29     ` Wolfgang Denk
2010-05-21 22:22   ` Kumar Gala
2010-05-21 22:35     ` Wolfgang Denk
2010-05-21 18:42 ` Timur Tabi
2010-05-22 22:41   ` Kumar Gala
2010-05-21 18:59 ` Timur Tabi
2010-05-21 19:35   ` Timur Tabi
2010-05-21 19:52     ` Wolfgang Denk
2010-05-21 19:58       ` Timur Tabi [this message]
2010-05-21 20:16         ` Wolfgang Denk
2010-05-21 20:28           ` Timur Tabi
2010-05-21 21:03             ` Wolfgang Denk
2010-05-21 21:09               ` Timur Tabi
2010-05-22 22:39           ` Kumar Gala
2010-05-22 22:36 ` [U-Boot] [PATCH v2] " Kumar Gala
2010-05-24 14:52   ` Timur Tabi
2010-05-24 18:57     ` Kumar Gala
2010-05-24 19:06       ` Timur Tabi
2010-05-24 20:09 ` [U-Boot] [PATCH v3] " Kumar Gala
2010-05-24 20:12   ` Timur Tabi
2010-06-30  9:52   ` Kumar Gala
2010-07-14 19:33     ` Wolfgang Denk
2010-07-14 21:21     ` [U-Boot] [PATCH v4] Move ICS CLK chip frequency " Kumar Gala

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=4BF6E5DF.5020306@freescale.com \
    --to=timur@freescale.com \
    --cc=u-boot@lists.denx.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.