[U-Boot] [PATCH 1/2] mx6: soc: Add ENET2 mac address support

[Ye Li]

Hi Stefano,

On 1/24/2016 6:35 PM, Stefano Babic wrote:
> Hi Ye,
>
> On 19/01/2016 14:16, Ye Li wrote:
>> The i.MX6SX and i.MX6UL has two ENET controllers, add support for reading
>> MAC address from fuse for ENET2.
>>
>> Signed-off-by: Ye Li <ye.li at nxp.com>
>> ---
>>  arch/arm/cpu/armv7/mx6/soc.c             |   32 +++++++++++++++++++++--------
>>  arch/arm/include/asm/arch-mx6/imx-regs.h |   19 +----------------
>>  2 files changed, 24 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c
>> index bf5ae8c..e521bf2 100644
>> --- a/arch/arm/cpu/armv7/mx6/soc.c
>> +++ b/arch/arm/cpu/armv7/mx6/soc.c
>> @@ -364,15 +364,29 @@ void imx_get_mac_from_fuse(int dev_id, unsigned char *mac)
>>  	struct fuse_bank4_regs *fuse =
>>  			(struct fuse_bank4_regs *)bank->fuse_regs;
>>  
>> -	u32 value = readl(&fuse->mac_addr_high);
>> -	mac[0] = (value >> 8);
>> -	mac[1] = value ;
>> -
>> -	value = readl(&fuse->mac_addr_low);
>> -	mac[2] = value >> 24 ;
>> -	mac[3] = value >> 16 ;
>> -	mac[4] = value >> 8 ;
>> -	mac[5] = value ;
> I beg your pardon, but I need your help to better understand it with the
> manual. References are Application Processor manual for MX6Q and MX6X.
>
> Currently, we support loading the MAC address from fuses (all other
> i.MX6 with just one FEC) reading the fuses MAC_ADDRESS[31:0] and
> MAC_ADDRESS[47:32] as stated, for example, in Table 5-19 for i.MX6Q.
>
> In i.MX6SX manual, there is no an additional entry for the second MAC
> address, and in my understanding GP1[31:0] is used. This is ok if we
> agree on that, but it should be documented in doc/README.imx6, that
> already explains the case with a single controller
The fuse map table seems not update that fuse in i.MX6SX manual. But in OCOTP chapter,  I can find the new OCOTP_MAC2. It is at the offset 0x640, not the GP1[31:0].
I agree the README.imx6 needs updating for second controller. I will add it in patch v2.

>> +	if ((is_cpu_type(MXC_CPU_MX6SX) || is_cpu_type(MXC_CPU_MX6UL)) && 
>> +		1 == dev_id) {
>> +		u32 value = readl(&fuse->mac_addr2);
>> +		mac[0] = value >> 24 ;
>> +		mac[1] = value >> 16 ;
>> +		mac[2] = value >> 8 ;
>> +		mac[3] = value ;
>> +
>> +		value = readl(&fuse->mac_addr_high);
>> +		mac[4] = value >> 24 ;
>> +		mac[5] = value >> 16 ;
>> +		
> I admit I have not understood why the order is exchanged. Should be
> mac_addr_high not be identical to the two controllers, and just the
> lower part different ? Here mac_addr_high goes into mac[4,5], while for
> id = 0 it goes into mac[0,1]. Do I miss something ? Or is the manual not
> updated ?
>
The layout for MAC2 address is low bytes 4 and 5 come from the mac_addr_high and other high bytes from the mac_addr2. The name "mac_addr_high" only means to MAC1.

>> +	} else {
>> +		u32 value = readl(&fuse->mac_addr_high);
>> +		mac[0] = (value >> 8);
>> +		mac[1] = value ;
>> +
>> +		value = readl(&fuse->mac_addr_low);
>> +		mac[2] = value >> 24 ;
>> +		mac[3] = value >> 16 ;
>> +		mac[4] = value >> 8 ;
>> +		mac[5] = value ;
>> +	}
>
>>  
>>  }
>>  #endif
>> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h
>> index f24525e..d0324a0 100644
>> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
>> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
>> @@ -715,7 +715,6 @@ struct fuse_bank1_regs {
>>  	u32	rsvd7[3];
>>  };
>>  
>> -#if (defined(CONFIG_MX6SX) || defined(CONFIG_MX6UL))
>>  struct fuse_bank4_regs {
>>  	u32 sjc_resp_low;
>>  	u32 rsvd0[3];
>> @@ -725,29 +724,13 @@ struct fuse_bank4_regs {
>>  	u32 rsvd2[3];
>>  	u32 mac_addr_high;
>>  	u32 rsvd3[3];
>> -	u32 mac_addr2;
>> +	u32 mac_addr2; /*For i.MX6SX and i.MX6UL*/
>>  	u32 rsvd4[7];
>>  	u32 gp1;
>>  	u32 rsvd5[3];
>>  	u32 gp2;
>>  	u32 rsvd6[3];
>>  };
>> -#else
>> -struct fuse_bank4_regs {
>> -	u32	sjc_resp_low;
>> -	u32     rsvd0[3];
>> -	u32     sjc_resp_high;
>> -	u32     rsvd1[3];
>> -	u32	mac_addr_low;
>> -	u32     rsvd2[3];
>> -	u32     mac_addr_high;
>> -	u32	rsvd3[0xb];
>> -	u32	gp1;
>> -	u32	rsvd4[3];
>> -	u32	gp2;
>> -	u32	rsvd5[3];
>> -};
>> -#endif
>>  
>>  struct aipstz_regs {
>>  	u32	mprot0;
>>
> Best regards,
> Stefano Babic
>

Best regards,
Ye Li