[U-Boot] [PATCH] ARM: at91: sama5d2: Wrap cpu detection to fix macb driver
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@f8008000: PHY present at 1 ethernet@f8008000: Starting autonegotiation... ethernet@f8008000: Autonegotiation complete ethernet@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 --- 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
Hei hei,
my use of git send-email messed up my original cc list, so I add Eugen as maintainer for at91 here. I have some additional comments below.
On Fri, Mar 22, 2019 at 02:25:54PM +0100, Alexander Dahl wrote:
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:
I'm not sure if those macros for checking on certain cpu types are a good solution at all, other code parts seem to distinguish things by CONFIG_* variables.
The real issue on reliably detecting the features of that cadence ethernet mac IP block, is probably more complicated, though.
The Linux kernel driver sets cap flags based on the dt compat strings. I consider that a good approach in general, but I don't know if that's a way for the U-Boot driver model, too?
Greets Alex
Greetings Alex and everyone,
Currently I'm trying to block all some PC applications that is appearing as hidden, system files as well as unknown files on my mobile phones, and any related to my laptop. It's blocking me from writing proper commands. And upon every restore, reinstall etc, I will have 4 windows update security files related to arm devices even though I didn't initiate the update exe. I'm guessing its trying to force a update on my device?
And after very clean reinstall, reset pc, or repairing etc, I will still have those hidden files scattered all around each folder. As for any portable devices, mobile phones including sdcards, portable ssd are affected as well when they are plugged in. Antivirus software seemed to allow these files as non virus. McAfee however drops some hints when full scan was applied but not detailed enough but it states something that a device needs update etc.
I'm now at my last attempt to delete all activities that are stored in cloud, e.g. onedrive, windows activities, etc. Or changing another pc pr even try to attempt against a different location where the IP address is different?
Thanks and best regards Bin Meng
-- Sent from: http://u-boot.10912.n7.nabble.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@f8008000: PHY present at 1 ethernet@f8008000: Starting autonegotiation... ethernet@f8008000: Autonegotiation complete ethernet@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
On 25.03.2019 11:17, Alexander Dahl wrote:
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@f8008000: PHY present at 1 ethernet@f8008000: Starting autonegotiation... ethernet@f8008000: Autonegotiation complete ethernet@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
Hi,
Are you sure there is a problem with this commit itself:
net: macb: Fix GEM hardware detection
as it looks , this commit fixes the interpretation of the MID register w.r.t. the gem capability... not an issue there that your patch fixes.
More likely your patch fixes the fact that the sama5d2 soc was not properly defining that macro, which was turned into a function, so your patch fixes the code that changed the macro into a function, which broke things... is my understanding correct ?
Eugen
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
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hello Eugen,
Am Montag, 25. März 2019, 09:46:02 CET schrieb Eugen.Hristev@microchip.com:
Are you sure there is a problem with this commit itself:
net: macb: Fix GEM hardware detection
as it looks , this commit fixes the interpretation of the MID register w.r.t. the gem capability... not an issue there that your patch fixes.
Well, without the changeset "net: macb: Fix GEM hardware detection" the actual problem I had, would not trigger, because the SAMA5D2 is not recognized having the gem capability. That's why I asked if that additional fixes line is correct.
More likely your patch fixes the fact that the sama5d2 soc was not properly defining that macro, which was turned into a function, so your patch fixes the code that changed the macro into a function, which broke things... is my understanding correct ?
My patch fixes that soc detection only. The behaviour on SAMA5D2 however is as follows:
1) Without both patches (as in v2019.01): SAMA5D2 mac is not detected as gem → soc detection in function 'gem_is_gigabit_capable()' is not relevant for gigabit detection -> ethernet works (maybe accidentally, gem actually used like old mac?)
2) With my patch only: mac is not detected as gem AND soc is detected as SAMA5D2 → no gigabit detected → ethernet might work as in 1), not tested though
3) With the gem hw detect fix only (as in v2019.04-rc3 already): mac is detected as gem, soc is not detected as SAMA5D2 → falsely detected gigabit capability → ethernet broken (can you reproduce this?)
4) With both patches: hw recognized as gem, soc recognized as SAMA5D2 → mac used as gem, but without gigabit caps → works for me on custom board
Greets Alex
On 22.03.2019 15:25, Alexander Dahl wrote:
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@f8008000: PHY present at 1 ethernet@f8008000: Starting autonegotiation... ethernet@f8008000: Autonegotiation complete ethernet@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
Applied to u-boot-atmel/next , with a minor tweak on fixes tag Thanks !
participants (4)
-
Alexander Dahl -
Alexander Dahl -
Bin Meng -
Eugen.Hristev＠microchip.com