public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Alexander Dahl <ada@thorsis.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] ARM: at91: sama5d2: Wrap cpu detection to fix macb driver
Date: Mon, 25 Mar 2019 10:17:47 +0100	[thread overview]
Message-ID: <23906734.aZzzLUK5Xj@ada> (raw)
In-Reply-To: <20190322132554.5848-1-ada@thorsis.com>

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

  parent reply	other threads:[~2019-03-25  9:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-22 13:25 [U-Boot] [PATCH] ARM: at91: sama5d2: Wrap cpu detection to fix macb driver Alexander Dahl
2019-03-23  8:46 ` Alexander Dahl
2019-03-24  9:01   ` Bin Meng
2019-03-25  9:17 ` Alexander Dahl [this message]
2019-03-25  9:46   ` Eugen.Hristev at microchip.com
2019-03-25 10:08     ` Alexander Dahl
2019-03-28  7:27 ` Eugen.Hristev at microchip.com

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=23906734.aZzzLUK5Xj@ada \
    --to=ada@thorsis.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox