All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Roese <sr@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] powerpc: Consolidate bootcount_{store|load} for PowerPC
Date: Wed, 28 Apr 2010 09:14:22 +0200	[thread overview]
Message-ID: <201004280914.22624.sr@denx.de> (raw)
In-Reply-To: <q2l660c0f821004270856wecf43497uaee7283e5cf0c4af@mail.gmail.com>

Hi Michael,

On Tuesday 27 April 2010 17:56:27 Michael Zaidman wrote:
> It makes sense to keep some measure of flexibility by giving to the
> user possibility to override the CONFIG_SYS_BOOTCOUNT_ADDR definition.
> It can be easily achieved by adding the following lines:
> 
> #ifdef CONFIG_SYS_BOOTCOUNT_ADDR
> #define _BOOTCOUNT_ADDR CONFIG_SYS_BOOTCOUNT_ADDR
> #else
> 
> > +#if defined(CONFIG_MPC5xxx)
> > +#define CONFIG_BOOTCOUNT_ADDR  (MPC5XXX_CDM_BRDCRMB)
> > +#define CONFIG_SYS_BOOTCOUNT_USE_32BIT
> 
> Also CONFIG_ as stated in u-boot's README should specify selectable by
> user options. I suggest omitting of all the locally defined CONFIG_ in
> your code.

I'll change CONFIG_BOOTCOUNT_ADDR to CONFIG_SYS_BOOTCOUNT_ADDR. And only 
overwrite/define this value, when not already defined.
 
> > +#endif /* defined(CONFIG_MPC5xxx) */
> > +
> > +#if defined(CONFIG_MPC821) || defined(CONFIG_MPC823) || \
> > +    defined(CONFIG_MPC850) || defined(CONFIG_MPC852T) || \
> > +    defined(CONFIG_MPC855) || \
> > +    defined(CONFIG_MPC860) || defined(CONFIG_MPC866) || \
> > +    defined(CONFIG_MPC885)
> 
> This fragment does not cover all mpc8xx permutations.

I did run MAKEALL 8xx and nothing broke. But you may be right. I'll check this 
again.

> We can cover
> them all by the following code:
> 
> #if	defined(CONFIG_MPC821) || defined(CONFIG_MPC823) || \
> 	defined(CONFIG_MPC855) || defined(CONFIG_MPC855T)|| \
> 	defined(CONFIG_MPC850) || defined(CONFIG_MPC86x)

Some 8xx variants are missing here as well (e.g. CONFIG_MPC852T, 
CONFIG_MPC885).
 
> I just used the CONFIG_MPC86x which has been already constructed by
> common.h and accomplished the list by CPUs that for unknown to me reason
> are not mentioned in common.h Probably, the better way is to add them into
> the common.h...

CONFIG_8xx seems the way to go. I just noticed it and will use it in the next 
patch version.

Thanks.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

  reply	other threads:[~2010-04-28  7:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-26 14:28 [U-Boot] [PATCH] powerpc: Consolidate bootcount_{store|load} for PowerPC Stefan Roese
2010-04-27 14:01 ` Detlev Zundel
2010-04-27 14:11   ` Stefan Roese
2010-04-27 15:56 ` Michael Zaidman
2010-04-28  7:14   ` Stefan Roese [this message]
2010-04-28  8:06     ` Michael Zaidman

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=201004280914.22624.sr@denx.de \
    --to=sr@denx.de \
    --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.