[U-Boot] [PATCH] ARM: mx31: Print the silicon version

[Fabio Estevam]

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 at 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