Re: [PATCH v1] net: eth-uclass: Change uclass driver name to ethernet

24 Feb 2021

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 = &eth_3;


          eth5 = &eth_5;




          ethernet0 = "/eth@10002000";


          ethernet3 = &eth_3;


          ethernet5 = &eth_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):
-----------------------------[ cut here ]-----------------------------
#include <stdbool.h>
#include <stdio.h>
#include <string.h>
/* Returns false if 'prefix' is a not empty prefix of 'string'.
 */
bool matches(const char *prefix, const char *string)
{
    if (!*prefix)
    	return true;
    while (*string && *prefix == *string) {
    	prefix++;
    	string++;
    }
return !!*prefix;
}
int main(void)
{
    char *str1 = "eth";
    char *str2 = "ethernet";
printf("strcmp returns %d\n", strcmp(str1, str2));
    printf("matches returns %d\n", matches(str2, str1));
    printf("reverse matches returns %d\n", matches(str1, str2));
return 0;
}
-----------------------------[ cut here ]-----------------------------
strcmp returns -101
matches returns 0
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 would not, under any circumstance, break compatibility with "eth"
alias names.

    

Re: [PATCH v1] net: eth-uclass: Change uclass driver name to ethernet

Vladimir Oltean