Re: [U-Boot] [PATCH] ARM: mx31: Print the silicon version
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_, 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_ADDR0x5000
>> +#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
>> + *
>> + * 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] ARM: mx31: Print the silicon version
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_, 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 0x5000
> +#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
> + *
> + * 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_00x10
> +#define IMX_CHIP_REVISION_1_10x11
> +#define IMX_CHIP_REVISION_1_20x12
> +#define IMX_CHIP_REVISION_1_30x13
> +#define IMX_CHIP_REVISION_2_00x20
> +#define IMX_CHIP_REVISION_2_10x21
> +#define IMX_CHIP_REVISION_2_20x22
> +#define IMX_CHIP_REVISION_2_30x23
> +#define IMX_CHIP_REVISION_3_00x30
> +#define IMX_CHIP_REVISION_3_10x31
> +#define IMX_CHIP_REVISION_3_20x32
> +#define IMX_CHIP_REVISION_3_30x33
> +#define IMX_CHIP_REVISION_UNKNOWN0xff
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
Re: [U-Boot] [PATCH] ARM: mx31: Print the silicon version
Hi, Fabio,
2011/3/11 Fabio Estevam :
> Use the same method of the Linux kernel to print the MX31 silicon version on
> boot.
>
> Tested on a MX31PDK with a 2.0 silicon, where it shows:
>
> CPU: Freescale i.MX31 at 531 MHz
> MX31 silicon rev 2.0
>
> Signed-off-by: Fabio Estevam
> ---
> arch/arm/cpu/arm1136/mx31/generic.c | 18
> arch/arm/include/asm/arch-mx31/mx31-regs.h | 4 ++
> arch/arm/include/asm/imx_soc_revision.h | 42
>
> 3 files changed, 64 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/include/asm/imx_soc_revision.h
>
> diff --git a/arch/arm/cpu/arm1136/mx31/generic.c
> b/arch/arm/cpu/arm1136/mx31/generic.c
> index 8bd23ee..4791449 100644
> --- a/arch/arm/cpu/arm1136/mx31/generic.c
> +++ b/arch/arm/cpu/arm1136/mx31/generic.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
>
> static u32 mx31_decode_pll(u32 reg, u32 infreq)
> {
> @@ -106,11 +107,28 @@ void mx31_set_pad(enum iomux_pins pin, u32 config)
>
> }
>
> +void mx31_read_cpu_rev(void)
> +{
> + u32 i, srev;
> +
> + /* read SREV register from IIM module */
> + srev = __raw_readl(MX31_IIM_BASE_ADDR + MXC_IIMSREV);
Need use struct way to access srev.
[code snip]
> 1.6.0.4
>
>
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
