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

[Peng Fan]

Hi Stefano,

On Thu, Aug 06, 2015 at 03:54:22PM +0200, Stefano Babic wrote:
>Hi Peng,
>
>On 06/08/2015 06:41, Peng Fan wrote:
>> Hi Nikolay,
>> On Wed, Aug 05, 2015 at 05:31:27PM +0300, Nikolay Dimitrov wrote:
>>> Hi Peng,
>>>
>>> On 08/03/2015 01:06 PM, Peng Fan wrote:
>>>> The MIB RAM and FIFO receive start register does not exist on
>>>> i.MX6UL. Accessing these register will cause enet not work well.
>>>>
>>>> Signed-off-by: Peng Fan <Peng.Fan at freescale.com>
>>>> Signed-off-by: Fugang Duan <B38611 at freescale.com>
>>>> Cc: Joe Hershberger <joe.hershberger at ni.com>
>>>> ---
>>>>  drivers/net/fec_mxc.c | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
>>>> index c5dcbbb..7fb1d5f 100644
>>>> --- a/drivers/net/fec_mxc.c
>>>> +++ b/drivers/net/fec_mxc.c
>>>> @@ -520,8 +520,10 @@ static int fec_open(struct eth_device *edev)
>>>>  static int fec_init(struct eth_device *dev, bd_t* bd)
>>>>  {
>>>>  	struct fec_priv *fec = (struct fec_priv *)dev->priv;
>>>> +#if !defined(CONFIG_MX6UL)
>>>>  	uint32_t mib_ptr = (uint32_t)&fec->eth->rmon_t_drop;
>>>>  	int i;
>>>> +#endif
>>>>
>>>>  	/* Initialize MAC address */
>>>>  	fec_set_hwaddr(dev);
>>>> @@ -551,12 +553,14 @@ static int fec_init(struct eth_device *dev, bd_t* bd)
>>>>  	writel(0x00000000, &fec->eth->gaddr2);
>>>>
>>>>
>>>> +#if !defined(CONFIG_MX6UL)
>>>>  	/* clear MIB RAM */
>>>>  	for (i = mib_ptr; i <= mib_ptr + 0xfc; i += 4)
>>>>  		writel(0, i);
>>>>
>>>>  	/* FIFO receive start register */
>>>>  	writel(0x520, &fec->eth->r_fstart);
>>>> +#endif
>>>>
>>>>  	/* size and address of each buffer */
>>>>  	writel(FEC_MAX_PKT_SIZE, &fec->eth->emrbr);
>>>>
>>>
>>> Is it possible to do runtime check for the SoC type, instead of ifdefs?
>> 
>> This driver is used by i.MX7 and i.MX6, but i.MX7 patchset has not been
>> upstreamed now.
>
>Wait...this driver is used by all i.MXes. We have to take this in mind
>to avoid to break some SOCs - even if this is not the case.
>
>Anyway, the rule is to push patches based on mainline, without supposing
>what happens with patches that are not yet applied. i.MX7 should be then
>adjusted for this.
>
>> 
>> 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. 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?

I am not familar with SoCs prior to i.MX6, not sure whether this 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.

Regards,
Peng.

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

--