[U-Boot] [PATCH] ARM: at91: sama5d2: Wrap cpu detection to fix macb driver

Alexander Dahl ada at thorsis.com
Mon Mar 25 09:17:47 UTC 2019


Hei hei,

one thing still puzzled me, why did it work on SAMA5D27-SOM1-EK1 board, but 
not on our custom board? I think I know the reason now. 

As Nicolas Ferre replied on my confusion on that MID register, on SAMA5D2 that 
register reads 0x00020203, so the value extracted for determining if that mac 
hardware block is of type GEM or not reads 2 on this SoC. Without the fix 
fbcaa260e5cdaeb0cc153c823e034076ac6a6902 the macb driver wouldn't recognize 
the SAMA5D2 as having the GEM variant of the mac block and that must have been 
the case here. 

My test setup for the EK board was based on U-Boot v2019.01, which does not 
have that fix (it's in U-Boot master with v2019.04-rc3), but I backported it 
to my tree for our custom board. ¯\_(ツ)_/¯

I would propose to add an additional Fixes line, see below.

Greets
Alex

Am Freitag, 22. März 2019, 14:25:54 CET schrieb Alexander Dahl:
> When introducing the SAMA5D27 SoCs, the SAMA5D2 series got an additional
> chip id. The check if the cpu is sama5d2 was changed from a preprocessor
> definition (inlining a call to 'get_chip_id()') to a C function,
> probably to not call get_chip_id twice?
> 
> That however broke a check in the macb ethernet driver. That driver is
> more generic and also used for other platforms. I suppose this solution
> was implemented to use it in 'gem_is_gigabit_capable()', without having
> to stricly depend on the at91 platform:
> 
> 	#ifndef cpu_is_sama5d2
> 	#define cpu_is_sama5d2() 0
> 	#endif
> 
> That only works as long as cpu_is_sama5d2 is a preprocessor definition.
> (The same is still true for sama5d4 by the way.) So this is a straight
> forward fix for the workaround.
> 
> The not working check on the SAMA5D2 CPU lead to an issue on a custom
> board with a LAN8720A ethernet phy connected to the SoC:
> 
> 	=> dhcp
> 	ethernet at f8008000: PHY present at 1
> 	ethernet at f8008000: Starting autonegotiation...
> 	ethernet at f8008000: Autonegotiation complete
> 	ethernet at f8008000: link up, 1000Mbps full-duplex (lpa: 0xffff)
> 	BOOTP broadcast 1
> 	BOOTP broadcast 2
> 	BOOTP broadcast 3
> 	BOOTP broadcast 4
> 	BOOTP broadcast 5
> 	BOOTP broadcast 6
> 	BOOTP broadcast 7
> 	BOOTP broadcast 8
> 	BOOTP broadcast 9
> 	BOOTP broadcast 10
> 	BOOTP broadcast 11
> 	BOOTP broadcast 12
> 	BOOTP broadcast 13
> 	BOOTP broadcast 14
> 	BOOTP broadcast 15
> 	BOOTP broadcast 16
> 	BOOTP broadcast 17
> 
> 	Retry time exceeded; starting again
> 
> Notice the wrong reported link speed, although both SoC and phy only
> support 100 MBit/s!
> 
> The real issue on reliably detecting the features of that cadence
> ethernet mac IP block, is probably more complicated, though.
> 
> Fixes: 245cbc583db7c1ca52aa32428b8e86f3449d4af2
> Signed-off-by: Alexander Dahl <ada at thorsis.com>

Fixes: fbcaa260e5cdaeb0cc153c823e034076ac6a6902

> ---
>  arch/arm/mach-at91/armv7/sama5d2_devices.c | 2 +-
>  arch/arm/mach-at91/include/mach/sama5d2.h  | 5 ++++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/armv7/sama5d2_devices.c
> b/arch/arm/mach-at91/armv7/sama5d2_devices.c index 6432f66c82..59a0c44913
> 100644
> --- a/arch/arm/mach-at91/armv7/sama5d2_devices.c
> +++ b/arch/arm/mach-at91/armv7/sama5d2_devices.c
> @@ -9,7 +9,7 @@
>  #include <asm/arch/clk.h>
>  #include <asm/arch/sama5d2.h>
> 
> -int cpu_is_sama5d2(void)
> +int _cpu_is_sama5d2(void)
>  {
>  	unsigned int chip_id = get_chip_id();
> 
> diff --git a/arch/arm/mach-at91/include/mach/sama5d2.h
> b/arch/arm/mach-at91/include/mach/sama5d2.h index 37806cbf34..c7d9bb5ad3
> 100644
> --- a/arch/arm/mach-at91/include/mach/sama5d2.h
> +++ b/arch/arm/mach-at91/include/mach/sama5d2.h
> @@ -222,6 +222,9 @@
>  #define ARCH_EXID_SAMA5D27C_D1G		0x00000033
>  #define ARCH_EXID_SAMA5D28C_D1G		0x00000013
> 
> +/* Checked if defined in ethernet driver macb */
> +#define cpu_is_sama5d2	_cpu_is_sama5d2
> +
>  /* PIT Timer(PIT_PIIR) */
>  #define CONFIG_SYS_TIMER_COUNTER	0xf804803c
> 
> @@ -231,7 +234,7 @@
>  #ifndef __ASSEMBLY__
>  unsigned int get_chip_id(void);
>  unsigned int get_extension_chip_id(void);
> -int cpu_is_sama5d2(void);
> +int _cpu_is_sama5d2(void);
>  unsigned int has_lcdc(void);
>  char *get_cpu_name(void);
>  #endif




More information about the U-Boot mailing list