[PATCH 05/14] at91: use structure to store the current soc

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:08 Tue 26 Apr     , Ryan Mallon wrote:
> On 04/26/2011 06:31 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > instead of reading the registers everytime
> 
> Hi Jean, a couple of comments below.
> 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj at jcrosoft.com>
> > Cc: Nicolas Ferre <nicolas.ferre at atmel.com>
> > Cc: Patrice Vilchez <patrice.vilchez at atmel.com>
> > ---
> >  arch/arm/mach-at91/at91rm9200.c       |    8 -
> >  arch/arm/mach-at91/at91sam9260.c      |    1 +
> >  arch/arm/mach-at91/at91sam9rl.c       |    1 +
> >  arch/arm/mach-at91/cpu.h              |  181 +++++++++++++++++
> >  arch/arm/mach-at91/include/mach/cpu.h |  355 ++++++++++++++-------------------
> >  arch/arm/mach-at91/soc.c              |   66 +++++-
> >  6 files changed, 391 insertions(+), 221 deletions(-)
> >  create mode 100644 arch/arm/mach-at91/cpu.h
> >  rewrite arch/arm/mach-at91/include/mach/cpu.h (71%)
> > 
> > diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
> > index abc4cc9..afb29b9 100644
> > --- a/arch/arm/mach-at91/at91rm9200.c
> > +++ b/arch/arm/mach-at91/at91rm9200.c
> > @@ -263,14 +263,6 @@ static void at91rm9200_reset(void)
> >  	at91_sys_write(AT91_ST_CR, AT91_ST_WDRST);
> >  }
> >  
> > -int rm9200_type;
> > -EXPORT_SYMBOL(rm9200_type);
> > -
> > -void __init at91rm9200_set_type(int type)
> > -{
> > -	rm9200_type = type;
> > -}
> > -
> 
> This only got introduce a couple of patches ago. I'm aware its a
> stop-gap solution, but is it possible to rework/reorder the patches so
> that this doesn't need to be temporarily introduced?
the is to do not touch the other soc init api so I keep the changes local
and prepare the soc to have the same init API so we can factorize them

I prefer to do small changes to be able to bisect them if needed
> 
> <snip>
> 
> > +
> > +#ifdef CONFIG_ARCH_AT91RM9200
> > +#define __cpu_is_at91rm9200()	(at91_cpu_identify() == ARCH_ID_AT91RM9200)
> > +#else
> > +#define __cpu_is_at91rm9200()	(0)
> > +#endif
> 
> I haven't looked at the subsequent patches yet to see if this patch
> simplifies things later but it seems that this just adds more lines of
> code by having two copies of the each of the cpu_is_xxx macros (the
> __cpu_is_xxx version which reads the register, and the cpu_is_xxx
> version which reads the shadow value)?
> 
> What is the reasoning behind shadowing the cpu type rather than reading
> the registers? The cpu detection macros are mostly used at
> initialisation and device probe time, so they don't necessarily need to
> be fast.
> 
> If this eventually reduces code size then I think it is useful, but
> otherwise I'm not sure I see the point?
It's on purpose as the dbgu physical address is not at the same place
so read the other register really does not impact the chip but if we do it
later duting the boot or the life to the kernel it's an other story

so the split between __cpu_is and cpu_is is necessarly

all of this work is in preparation to allow multiple soc in the same kernel
that's also why I map the system controller the same way on all at91 arm9

Best Regards,
J.