[PATCH 04/20] imx/clock-imx27: use a macro to define registers

[Uwe Kleine-König]

Hello,

On Thu, Dec 17, 2009 at 08:16:34AM +0100, Lothar Waßmann wrote:
> Hi,
> 
> Uwe Kleine-König writes:
> > This way the base address isn't hard coded in each register definition
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig at pengutronix.de>
> > Cc: Sascha Hauer <kernel at pengutronix.de>
> > Cc: Russell King <linux at arm.linux.org.uk>
> > Cc: Rabin Vincent <rabin at rab.in>
> > Cc: Javier Martin <javier.martin at vista-silicon.com>
> > Cc: Valentin Longchamp <valentin.longchamp at epfl.ch>
> > ---
> >  arch/arm/mach-mx2/clock_imx27.c |   30 ++++++++++++++++--------------
> >  1 files changed, 16 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/arm/mach-mx2/clock_imx27.c b/arch/arm/mach-mx2/clock_imx27.c
> > index b010bf9..a533208 100644
> > --- a/arch/arm/mach-mx2/clock_imx27.c
> > +++ b/arch/arm/mach-mx2/clock_imx27.c
> > @@ -29,21 +29,23 @@
> >  #include <mach/common.h>
> >  #include <mach/hardware.h>
> >  
> > +#define IO_ADDR_CCM(off)	(IO_ADDRESS(CCM_BASE_ADDR) + (off))
> I would include the offset in the parameter for the IO_ADDRESS macro,
> so that the macro would check the complete address (including the
> offset) for validity:
> | +#define IO_ADDR_CCM(off)	(IO_ADDRESS((CCM_BASE_ADDR) + (off)))
> Otherwise e.g. a negative offset may lead to generation of bogus
> addresses.
I'd use

	#define IO_ADDR_CCM(off)	(IO_ADDRESS(CCM_BASE_ADDR + (off)))

and change it in a seperate patch.  And with negative addresses all bets
are off anyhow.

Grepping for "IO_ADDRESS\(.*\) *\+" finds some more places that want to
be fixed.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |