On 03/10/2011 08:26 PM, Fabio Estevam wrote:

> +void mx31_read_cpu_rev(void)

Generally, for exported function, I would prefer to remove the processor
name. For other i.MX processors we use the convention
mxc_<function_name>, as we can get rid of nasty #ifdef inside the
drivers. You can see a lot of examples in code.

> +{
> +     u32 i, srev;
> +
> +     /* read SREV register from IIM module */
> +     srev = __raw_readl(MX31_IIM_BASE_ADDR + MXC_IIMSREV);

We have already used the IIM registers on other i.MX processors, you can
see for i.MX35/MX51/MX53. You should set a structure for the iim
registers and use it, instead of using offset.

I know the i.MX31, as it was the first i.MX31, does not follow this
rule, but it means it should be clean up.

> diff --git a/arch/arm/include/asm/arch-mx31/mx31-regs.h 
> b/arch/arm/include/asm/arch-mx31/mx31-regs.h
> index 37337f2..cc0ffc8 100644
> --- a/arch/arm/include/asm/arch-mx31/mx31-regs.h
> +++ b/arch/arm/include/asm/arch-mx31/mx31-regs.h
> @@ -480,6 +480,10 @@ enum iomux_pins {
>  #define CCMR_FPM     (1 << 1)
>  #define CCMR_CKIH    (2 << 1)
>  
> +#define MX31_SPBA0_BASE_ADDR 0x50000000
> +#define MX31_IIM_BASE_ADDR   (MX31_SPBA0_BASE_ADDR + 0x1c000)
> +#define MXC_IIMSREV             0x0024

As I said, replace them with a structure.

> +++ b/arch/arm/include/asm/imx_soc_revision.h
> @@ -0,0 +1,42 @@
> +/*
> + * Copyright (C) 2011 Freescale Semiconductor, Inc.
> + *
> + * Fabio Estevam <fabio.este...@freescale.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation;
> + */
> +
> +#define IMX_CHIP_REVISION_1_0                0x10
> +#define IMX_CHIP_REVISION_1_1                0x11
> +#define IMX_CHIP_REVISION_1_2                0x12
> +#define IMX_CHIP_REVISION_1_3                0x13
> +#define IMX_CHIP_REVISION_2_0                0x20
> +#define IMX_CHIP_REVISION_2_1                0x21
> +#define IMX_CHIP_REVISION_2_2                0x22
> +#define IMX_CHIP_REVISION_2_3                0x23
> +#define IMX_CHIP_REVISION_3_0                0x30
> +#define IMX_CHIP_REVISION_3_1                0x31
> +#define IMX_CHIP_REVISION_3_2                0x32
> +#define IMX_CHIP_REVISION_3_3                0x33
> +#define IMX_CHIP_REVISION_UNKNOWN    0xff

Is there a good reason to add a further file and not put them inside
inside the mx31-regs.h file ?

Best regards,
Stefano

-- 
=====================================================================
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: off...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to