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

[Stefano Babic]

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.

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

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