[U-Boot] [PATCH 2/4] net: fec: do not access reserved register for i.MX6UL

[Stefano Babic]

Hi Peng,

On 07/08/2015 03:08, Peng Fan wrote:

>>> I considered using "if (!is_cpu_type(MXC_CPU_MX6UL))", but this will cause
>>> i.MX7 fail to complile successfully, because i.MX7 does not support the macro
>>> MXC_CPU_MX6UL.
>>
>> It looks like we have a problem in design - we have to move this macros
>> to make available to all i.MXes.
>>
>> In fact, it is even plausible that MX35 code runs
>> "is_cpu_type(MXC_CPU_MX6UL)", and the macros must return false. Having
>> these checks working for some SOCs vanifies the goal: check at runtime
>> if a SOC is of a certain type.
> 
> I checked related code for i.MX25/28/31/35/5x/6x.
> 
> We can add following define in imx-common/xxx.h
> #define MXC_CPU_MX35 0x35
> #define MXC_CPU_MX31 0x31
> #define MXC_CPU_MX25 0x25
> #define MXC_CPU_MX23 xxxx
> #define MXC_CPU_MX28 yyyy
> About i.mx23/28, I am not sure, since they have different get_cpu_rev
> implementation.

As far as I understand, the chip id is retrieved and then converted as
"23" or "28" (see get_cpu_type). I think it is plausible to define them
in this way as well as using the waqy in get_cpu_type() to identify the SOC.

get_cpu_rev() in MX23/MX28 is also not compliant with the rest of SOCs.
It returns a string intead of a u32.

> Also they have different chipid layout.
> 
> To i.MX31, we can do following change:
> return mx31_cpu_type[i].v | 0x31000; to replace return mx31_cpu_type[i].v;
> 
> Then we can use:
> #define is_cpu_type(xxx) (((get_cpu_rev() & 0xFF000) >> 12) == xxx)
> 
> To i.MX23/28, they have different get_cpu_rev() prototype, maybe need to
> rewrite the function?

Agree - it is an exception in U-Boot for i.MXEs, and it defines a
different prototype for the function.

> 
> I am not familar with SoCs prior to i.MX6, not sure whether this ok.
> 

IMHO it looks ok ;-)

>>
>>>
>>> The way I can think out is to refactor the code to support DM or FDT,
>>> using compatible string to figure out which SoC. But now I do not have
>>> much time to refactor the driver. So I just use the "#if !defined" way
>>> which is not good solution.
>>
>> It is not - anyway, making macros commonly available to all i.MXes could
>> be done with a smaller effort. Currently, macros are available only to
>> mx6 (define in sys_proto.h). We have to move them in a header in
>> arch/arm/include/asm/imx-common/, that is accessible by all SOCs.
> 
> Maybe need to add sys_proto.h in arch/arm/include/asm/imx-common.

+1

Right - we have several sys_proto.h, iun most cases they have prototypes
for the same functions.

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================