Am 2021-02-24 19:14, schrieb Vladimir Oltean:
On Wed, Feb 24, 2021 at 06:21:33PM +0100, Michael Walle wrote:
Am 2021-01-19 21:40, schrieb Tom Rini:
On Tue, Jan 19, 2021 at 03:01:38PM -0500, Tom Rini wrote:
On Fri, Jan 08, 2021 at 10:53:05AM +0800, David Wu wrote:
dev_read_alias_seq() used uc_drv->name compared to alias stem string, Ethernet's alias stem uses "ethernet", which does not match the eth-uclass driver name "eth", can not get the correct index of ethernet alias namer. So it seems change uclass driver name to match the alias stem is a more reasonable way.
Signed-off-by: David Wu david.wu@rock-chips.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
I'm reverting this change as it breaks a number of tests that need to be updated to match on the new name.
David, are you planning to submit a new version? If I'm not mistaken, the following changes should be enought to make the tests pass again:
--- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -14,9 +14,9 @@
aliases { console = &uart0;
eth0 = "/eth@10002000";eth3 = ð_3;eth5 = ð_5;
ethernet0 = "/eth@10002000";ethernet3 = ð_3;ethernet5 = ð_5; gpio1 = &gpio_a; gpio2 = &gpio_b; gpio3 = &gpio_c;In commit cc32fd911aa9 ("arm: dts: ls1028a: Add Ethernet switch node and dependencies") there was recently an additon to a board which actually uses these aliases. So this patch should change them too.
I was actually under the impression that the alias was "ethernetN" and added them to my board, see arch/arm/fsl-ls1028a-kontron*u-boot.dtsi. Seems like I wasn't the first one which was mistaken:
$ grep "ethernet. =" arch/**/*u-boot.dtsi arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi: ethernet2 = &enetc2; arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi: ethernet3 = &enetc6; arch/arm/dts/fsl-ls1028a-kontron-sl28-var1-u-boot.dtsi: ethernet0 = &enetc1; arch/arm/dts/fsl-ls1028a-kontron-sl28-var3-u-boot.dtsi: ethernet0 = &enetc0; arch/arm/dts/fsl-ls1028a-kontron-sl28-var4-u-boot.dtsi: ethernet0 = &enetc0; arch/arm/dts/fsl-ls1028a-kontron-sl28-var4-u-boot.dtsi: ethernet1 = &enetc1; arch/arm/dts/k3-am654-base-board-u-boot.dtsi: ethernet0 = &cpsw_port1; arch/arm/dts/k3-j7200-common-proc-board-u-boot.dtsi: ethernet0 = &cpsw_port1; arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi: ethernet0 = &cpsw_port1; arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi: ethernet1 = &ksz8851;
So at the moment I'm not sure if I should fix my dtsi files to use the ethN aliases or if I should just wait because this patch will make it into u-boot soon.
I could also pick up this patch, amend it and resubmit it myself.
-michael
Sorry, not sure that I understand. We are talking about the following code path, right?
device_bind_common -> dev_read_alias_seq -> of_alias_get_id -> strcmp(app->stem, stem)
Why on earth is it called "stem" if strcmp is used?
I'm not sure if there's any C standard library for string prefix comparison, but at least I know iproute2 uses a custom function (copy-pasted below):
I'm not sure if it should really be just a prefix match but a full string compare.
Wasn't the intention of David's patch, in fact, to have the new uclass name also match on "eth" aliases? If I'm correct, doesn't this mean we'll have to replace the strcmp with an actual stem check?
I guess it was intended the other way around, to rename the "ethN" aliases to "ethernetN". The latter are used way more in u-boot's device trees and linux' device trees just use ethernetN aliases, though I'm not sure where they are used (can't find any of_alias_get_id("ethernet") in linux). So this makes sense, no?
I would not, under any circumstance, break compatibility with "eth" alias names.
Assuming that ethN is not used expect in u-boot device trees, how would we break backwards compatibility? Also it seems that the only users of the ethN aliases are the sandbox and u-boot's DSA.
-michael