Hi Stefano,
On 3/11/2011 10:33 AM, Stefano Babic wrote:
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.
Ok.
+{
- 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.
Ok.
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.
Ok.
+++ b/arch/arm/include/asm/imx_soc_revision.h @@ -0,0 +1,42 @@ +/*
- Copyright (C) 2011 Freescale Semiconductor, Inc.
- Fabio Estevam fabio.estevam@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 ?
Yes, the idea is that other i.MX processors can reuse this file. We currently use this same approach in the kernel.
Will post v2 with your recommendations.
Regards,
Fabio Estevam