[PATCH net-next v1 0/4] usbnet: add "label" support

[PATCH net-next v1 0/4] usbnet: add "label" support

[Oleksij Rempel]

Add devicetree label property for usbnet devices and related yaml
schema.

Oleksij Rempel (4):
  dt-bindings: net: add schema for ASIX USB Ethernet controllers
  dt-bindings: net: add schema for Microchip/SMSC LAN95xx USB Ethernet
    controllers
  dt-bindings: net: add "label" property for all usbnet Ethernet
    controllers
  usbnet: add support for label from device tree

 .../devicetree/bindings/net/asix,ax88178.yaml | 102 ++++++++++++++++++
 .../bindings/net/microchip,lan95xx.yaml       |  84 +++++++++++++++
 .../devicetree/bindings/net/usbnet.yaml       |  36 +++++++
 drivers/net/usb/usbnet.c                      |  15 +++
 4 files changed, 237 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/asix,ax88178.yaml
 create mode 100644 Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
 create mode 100644 Documentation/devicetree/bindings/net/usbnet.yaml

-- 
2.30.2

[PATCH net-next v1 1/4] dt-bindings: net: add schema for ASIX USB Ethernet controllers

[Oleksij Rempel]

Create initial schema for ASIX USB Ethernet controllers and import all
currently supported USB IDs form drivers/net/usb/asix_devices.c

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../devicetree/bindings/net/asix,ax88178.yaml | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/asix,ax88178.yaml

diff --git a/Documentation/devicetree/bindings/net/asix,ax88178.yaml b/Documentation/devicetree/bindings/net/asix,ax88178.yaml
new file mode 100644
index 000000000000..74b6806006e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/asix,ax88178.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/asix,ax88178.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: The device tree bindings for the USB Ethernet controllers
+
+maintainers:
+  - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: |
+  Device tree properties for hard wired USB Ethernet devices.
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - usb411,3d     # Buffalo LUA-U2-KTX
+          - usb411,6e     # Buffalo LUA-U2-GT 10/100/1000
+          - usb4bb,930    # IO-DATA ETG-US2
+          - usb4f1,3008   # JVC MP-PRX1 Port Replicator
+          - usb50d,5055   # Belkin F5D5055
+          - usb557,2009   # ATEN UC210T
+          - usb5ac,1402   # Apple USB Ethernet Adapter
+          - usb66b,20f9   # USBLINK HG20F9
+          - usb77b,2226   # Linksys USB200M
+          - usb789,160    # Logitec LAN-GTJ/U2A
+          - usb7aa,17     # corega FEther USB2-TX
+          - usb7b8,420a   # Hawking UF200, TrendNet TU2-ET100
+          - usb7d1,3c05   # DLink DUB-E100 H/W Ver B1
+          - usb846,1040   # Netgear FA120
+          - usb8dd,114    # Billionton Systems, GUSB2AM-1G-B
+          - usb8dd,90ff   # Billionton Systems, USB2AR
+          - usbb95,1720   # Intellinet, ST Lab USB Ethernet
+          - usbb95,172a   # ASIX 88172a demo board
+          - usbb95,1780   # ASIX AX88178 10/100/1000
+          - usbb95,7720   # ASIX AX88772 10/100
+          - usbb95,772a   # Cables-to-Go USB Ethernet Adapter
+          - usbb95,772b   # ASIX AX88772B 10/100
+          - usbb95,7e2b   # Asus USB Ethernet Adapter
+          - usbdb0,a877   # ASIX 88772a
+          - usbdf6,56     # Sitecom LN-031
+          - usbdf6,61c    # Sitecom LN-028
+          - usb1189,893   # Surecom EP-1427X-2
+          - usb13b1,18    # Linksys USB200M Rev 2
+          - usb14ea,ab11  # ABOCOM for pci
+          - usb1557,7720  # 0Q0 cable ethernet
+          - usb1631,6200  # goodway corp usb gwusb2e
+          - usb1737,39    # Linksys USB1000
+          - usb17ef,7203  # Lenovo U2L100P 10/100
+          - usb2001,1a00  # DLink DUB-E100
+          - usb2001,1a02  # DLink DUB-E100 H/W Ver C1
+          - usb2001,3c05  # DLink DUB-E100 H/W Ver B1 Alternate
+          - usb6189,182d  # Sitecom LN-029
+
+  reg: true
+  local-mac-address: true
+  mac-address: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    usb@11270000 {
+        reg = <0x11270000 0x1000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethernet@1 {
+            compatible = "usbdb0,a877";
+            reg = <1>;
+            local-mac-address = [00 00 00 00 00 00];
+        };
+    };
+  - |
+    usb@11270000 {
+        reg = <0x11270000 0x1000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        usb1@1 {
+            compatible = "usb1234,5678";
+            reg = <1>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            ethernet@1 {
+               compatible = "usbdb0,a877";
+               reg = <1>;
+            };
+        };
+    };
-- 
2.30.2

Re: [PATCH net-next v1 1/4] dt-bindings: net: add schema for ASIX USB Ethernet controllers

[Greg KH]

On Thu, Jan 27, 2022 at 11:49:02AM +0100, Oleksij Rempel wrote:
> Create initial schema for ASIX USB Ethernet controllers and import all
> currently supported USB IDs form drivers/net/usb/asix_devices.c

Again, you are setting yourself to play a game you are always going to
loose and be behind on.  This is not acceptable, sorry.

greg k-h

Re: [PATCH net-next v1 1/4] dt-bindings: net: add schema for ASIX USB Ethernet controllers

[Oliver Neukum]

On 27.01.22 11:59, Greg KH wrote:
> On Thu, Jan 27, 2022 at 11:49:02AM +0100, Oleksij Rempel wrote:
>> Create initial schema for ASIX USB Ethernet controllers and import all
>> currently supported USB IDs form drivers/net/usb/asix_devices.c
> Again, you are setting yourself to play a game you are always going to
> loose and be behind on.  This is not acceptable, sorry.

Hi,

having checked the DSA subsystem, it looks to me like
the "label" tag is quite common for those setups.
I am afraid we cannot tell the DSA people how to build switches.

The question then becomes whether switches using USB
internally should be different from other switches.
I'd answer that question in the negative.

    Regards
        Oliver

Re: [PATCH net-next v1 1/4] dt-bindings: net: add schema for ASIX USB Ethernet controllers

[Rob Herring]

On Thu, Jan 27, 2022 at 11:59:49AM +0100, Greg KH wrote:
> On Thu, Jan 27, 2022 at 11:49:02AM +0100, Oleksij Rempel wrote:
> > Create initial schema for ASIX USB Ethernet controllers and import all
> > currently supported USB IDs form drivers/net/usb/asix_devices.c
> 
> Again, you are setting yourself to play a game you are always going to
> loose and be behind on.  This is not acceptable, sorry.

I would suggest adding just the IDs you need. There probably aren't many 
which are hardwired. And if they aren't hardwired, what are they doing 
in DT?

Rob

[PATCH net-next v1 2/4] dt-bindings: net: add schema for Microchip/SMSC LAN95xx USB Ethernet controllers

[Oleksij Rempel]

Create initial schema for Microchip/SMSC LAN95xx USB Ethernet controllers and
import all currently supported USB IDs form drivers/net/usb/smsc95xx.c

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../bindings/net/microchip,lan95xx.yaml       | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/microchip,lan95xx.yaml

diff --git a/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml b/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
new file mode 100644
index 000000000000..b185c7068a8a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/microchip,lan95xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: The device tree bindings for the USB Ethernet controllers
+
+maintainers:
+  - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: |
+  Device tree properties for hard wired SMSC95xx compatible USB Ethernet
+  controller.
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - usb424,9500   # SMSC9500 USB Ethernet Device
+          - usb424,9505   # SMSC9505 USB Ethernet Device
+          - usb424,9530   # SMSC LAN9530 USB Ethernet Device
+          - usb424,9730   # SMSC LAN9730 USB Ethernet Device
+          - usb424,9900   # SMSC9500 USB Ethernet Device (SAL10)
+          - usb424,9901   # SMSC9505 USB Ethernet Device (SAL10)
+          - usb424,9902   # SMSC9500A USB Ethernet Device (SAL10)
+          - usb424,9903   # SMSC9505A USB Ethernet Device (SAL10)
+          - usb424,9904   # SMSC9512/9514 USB Hub & Ethernet Device (SAL10)
+          - usb424,9905   # SMSC9500A USB Ethernet Device (HAL)
+          - usb424,9906   # SMSC9505A USB Ethernet Device (HAL)
+          - usb424,9907   # SMSC9500 USB Ethernet Device (Alternate ID)
+          - usb424,9908   # SMSC9500A USB Ethernet Device (Alternate ID)
+          - usb424,9909   # SMSC9512/9514 USB Hub & Ethernet Devic.  ID)
+          - usb424,9e00   # SMSC9500A USB Ethernet Device
+          - usb424,9e01   # SMSC9505A USB Ethernet Device
+          - usb424,9e08   # SMSC LAN89530 USB Ethernet Device
+          - usb424,ec00   # SMSC9512/9514 USB Hub & Ethernet Device
+
+  reg: true
+  local-mac-address: true
+  mac-address: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    usb@11270000 {
+        reg = <0x11270000 0x1000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethernet@1 {
+            compatible = "usb424,ec00";
+            reg = <1>;
+            local-mac-address = [00 00 00 00 00 00];
+        };
+    };
+  - |
+    usb@11270000 {
+        reg = <0x11270000 0x1000>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        usb1@1 {
+            compatible = "usb424,9514";
+            reg = <1>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            ethernet@1 {
+               compatible = "usb424,ec00";
+               reg = <1>;
+            };
+        };
+    };
-- 
2.30.2

Re: [PATCH net-next v1 2/4] dt-bindings: net: add schema for Microchip/SMSC LAN95xx USB Ethernet controllers

[Greg KH]

On Thu, Jan 27, 2022 at 11:49:03AM +0100, Oleksij Rempel wrote:
> Create initial schema for Microchip/SMSC LAN95xx USB Ethernet controllers and
> import all currently supported USB IDs form drivers/net/usb/smsc95xx.c

That is a loosing game to play.  There is a reason that kernel drivers
only require a device id in 1 place, instead of multiple places like
other operating systems.  Please do not go back and make the same
mistakes others have.

Not to mention that I think overall this is a bad idea anyway.  USB
devices are self-describing, don't add them to DT.

thanks,

greg k-h

Re: [PATCH net-next v1 2/4] dt-bindings: net: add schema for Microchip/SMSC LAN95xx USB Ethernet controllers

[Oleksij Rempel]

On Thu, Jan 27, 2022 at 11:59:11AM +0100, Greg KH wrote:
> On Thu, Jan 27, 2022 at 11:49:03AM +0100, Oleksij Rempel wrote:
> > Create initial schema for Microchip/SMSC LAN95xx USB Ethernet controllers and
> > import all currently supported USB IDs form drivers/net/usb/smsc95xx.c
> 
> That is a loosing game to play.  There is a reason that kernel drivers
> only require a device id in 1 place, instead of multiple places like
> other operating systems.  Please do not go back and make the same
> mistakes others have.
> 
> Not to mention that I think overall this is a bad idea anyway.  USB
> devices are self-describing, don't add them to DT.

This patch set is the pre-step before making it even more complicated
with description of external PHYs and DSA switches. I assume, it is
preferable to have schema to be able to automatically validate it.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 2/4] dt-bindings: net: add schema for Microchip/SMSC LAN95xx USB Ethernet controllers

[Rob Herring]

On Thu, Jan 27, 2022 at 11:59:11AM +0100, Greg KH wrote:
> On Thu, Jan 27, 2022 at 11:49:03AM +0100, Oleksij Rempel wrote:
> > Create initial schema for Microchip/SMSC LAN95xx USB Ethernet controllers and
> > import all currently supported USB IDs form drivers/net/usb/smsc95xx.c
> 
> That is a loosing game to play.  There is a reason that kernel drivers
> only require a device id in 1 place, instead of multiple places like
> other operating systems.  Please do not go back and make the same
> mistakes others have.

This instance doesn't look so bad because SMSC devices are chips rather 
than random OEM rebranded devices all with the same underlying chip.

> Not to mention that I think overall this is a bad idea anyway.  USB
> devices are self-describing, don't add them to DT.

Until they are soldered down and the board maker cheaps out on having an 
eeprom to hold the MAC address...

Rob

Re: [PATCH net-next v1 2/4] dt-bindings: net: add schema for Microchip/SMSC LAN95xx USB Ethernet controllers

[Rob Herring]

On Thu, 27 Jan 2022 11:49:03 +0100, Oleksij Rempel wrote:
> Create initial schema for Microchip/SMSC LAN95xx USB Ethernet controllers and
> import all currently supported USB IDs form drivers/net/usb/smsc95xx.c
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../bindings/net/microchip,lan95xx.yaml       | 82 +++++++++++++++++++
>  1 file changed, 82 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/1584951


smsc@2: $nodename:0: 'smsc@2' does not match '^ethernet(@.*)?$'
	arch/arm/boot/dts/tegra30-ouya.dt.yaml

usbether@1: $nodename:0: 'usbether@1' does not match '^ethernet(@.*)?$'
	arch/arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dt.yaml
	arch/arm64/boot/dts/freescale/imx8mm-kontron-n801x-s.dt.yaml
	arch/arm/boot/dts/bcm2835-rpi-b.dt.yaml
	arch/arm/boot/dts/bcm2835-rpi-b-plus.dt.yaml
	arch/arm/boot/dts/bcm2835-rpi-b-rev2.dt.yaml
	arch/arm/boot/dts/bcm2836-rpi-2-b.dt.yaml
	arch/arm/boot/dts/bcm2837-rpi-3-b.dt.yaml
	arch/arm/boot/dts/omap3-beagle-xm-ab.dt.yaml
	arch/arm/boot/dts/omap3-beagle-xm.dt.yaml
	arch/arm/boot/dts/omap4-panda-a4.dt.yaml
	arch/arm/boot/dts/omap4-panda.dt.yaml
	arch/arm/boot/dts/omap4-panda-es.dt.yaml

usbether@3: $nodename:0: 'usbether@3' does not match '^ethernet(@.*)?$'
	arch/arm/boot/dts/omap5-uevm.dt.yaml

[PATCH net-next v1 3/4] dt-bindings: net: add "label" property for all usbnet Ethernet controllers

[Oleksij Rempel]

For hard wired Ethernet controllers it is helpful to assign name related
to port description on the board. Or name, related to the special
internal function, if the USB ethernet controller attached to the CPU
port of some DSA switch.

This patch provides documentation for "label" property, reusable for all
usbnet controllers.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../devicetree/bindings/net/asix,ax88178.yaml |  4 ++-
 .../bindings/net/microchip,lan95xx.yaml       |  4 ++-
 .../devicetree/bindings/net/usbnet.yaml       | 36 +++++++++++++++++++
 3 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/usbnet.yaml

diff --git a/Documentation/devicetree/bindings/net/asix,ax88178.yaml b/Documentation/devicetree/bindings/net/asix,ax88178.yaml
index 74b6806006e3..c8ad767a2e45 100644
--- a/Documentation/devicetree/bindings/net/asix,ax88178.yaml
+++ b/Documentation/devicetree/bindings/net/asix,ax88178.yaml
@@ -13,7 +13,7 @@ description: |
   Device tree properties for hard wired USB Ethernet devices.
 
 allOf:
-  - $ref: ethernet-controller.yaml#
+  - $ref: usbnet.yaml#
 
 properties:
   compatible:
@@ -58,6 +58,7 @@ properties:
           - usb6189,182d  # Sitecom LN-029
 
   reg: true
+  label: true
   local-mac-address: true
   mac-address: true
 
@@ -77,6 +78,7 @@ examples:
         ethernet@1 {
             compatible = "usbdb0,a877";
             reg = <1>;
+            label = "LAN0";
             local-mac-address = [00 00 00 00 00 00];
         };
     };
diff --git a/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml b/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
index b185c7068a8a..259879bba3a0 100644
--- a/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
+++ b/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
@@ -14,7 +14,7 @@ description: |
   controller.
 
 allOf:
-  - $ref: ethernet-controller.yaml#
+  - $ref: usbnet.yaml#
 
 properties:
   compatible:
@@ -40,6 +40,7 @@ properties:
           - usb424,ec00   # SMSC9512/9514 USB Hub & Ethernet Device
 
   reg: true
+  label: true
   local-mac-address: true
   mac-address: true
 
@@ -59,6 +60,7 @@ examples:
         ethernet@1 {
             compatible = "usb424,ec00";
             reg = <1>;
+            label = "LAN0";
             local-mac-address = [00 00 00 00 00 00];
         };
     };
diff --git a/Documentation/devicetree/bindings/net/usbnet.yaml b/Documentation/devicetree/bindings/net/usbnet.yaml
new file mode 100644
index 000000000000..fe0848433263
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/usbnet.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/usbnet.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: The device tree bindings for the USB Ethernet controllers
+
+maintainers:
+  - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: |
+  Device tree properties for hard wired USB Ethernet devices.
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+properties:
+  compatible: true
+
+  reg:
+    description: Port number
+
+  label:
+    description:
+      Describes the label associated with this port, which will become
+      the netdev name
+    $ref: /schemas/types.yaml#/definitions/string
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: true
+
+...
-- 
2.30.2

Re: [PATCH net-next v1 3/4] dt-bindings: net: add "label" property for all usbnet Ethernet controllers

[Rob Herring]

On Thu, Jan 27, 2022 at 11:49:04AM +0100, Oleksij Rempel wrote:
> For hard wired Ethernet controllers it is helpful to assign name related
> to port description on the board. Or name, related to the special
> internal function, if the USB ethernet controller attached to the CPU
> port of some DSA switch.

Yes, so add 'label' to ethernet-controller.yaml.

Then I don't think usbnet.yaml is needed.

> 
> This patch provides documentation for "label" property, reusable for all
> usbnet controllers.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  .../devicetree/bindings/net/asix,ax88178.yaml |  4 ++-
>  .../bindings/net/microchip,lan95xx.yaml       |  4 ++-
>  .../devicetree/bindings/net/usbnet.yaml       | 36 +++++++++++++++++++
>  3 files changed, 42 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/usbnet.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/asix,ax88178.yaml b/Documentation/devicetree/bindings/net/asix,ax88178.yaml
> index 74b6806006e3..c8ad767a2e45 100644
> --- a/Documentation/devicetree/bindings/net/asix,ax88178.yaml
> +++ b/Documentation/devicetree/bindings/net/asix,ax88178.yaml
> @@ -13,7 +13,7 @@ description: |
>    Device tree properties for hard wired USB Ethernet devices.
>  
>  allOf:
> -  - $ref: ethernet-controller.yaml#
> +  - $ref: usbnet.yaml#
>  
>  properties:
>    compatible:
> @@ -58,6 +58,7 @@ properties:
>            - usb6189,182d  # Sitecom LN-029
>  
>    reg: true
> +  label: true
>    local-mac-address: true
>    mac-address: true
>  
> @@ -77,6 +78,7 @@ examples:
>          ethernet@1 {
>              compatible = "usbdb0,a877";
>              reg = <1>;
> +            label = "LAN0";
>              local-mac-address = [00 00 00 00 00 00];
>          };
>      };
> diff --git a/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml b/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
> index b185c7068a8a..259879bba3a0 100644
> --- a/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
> +++ b/Documentation/devicetree/bindings/net/microchip,lan95xx.yaml
> @@ -14,7 +14,7 @@ description: |
>    controller.
>  
>  allOf:
> -  - $ref: ethernet-controller.yaml#
> +  - $ref: usbnet.yaml#
>  
>  properties:
>    compatible:
> @@ -40,6 +40,7 @@ properties:
>            - usb424,ec00   # SMSC9512/9514 USB Hub & Ethernet Device
>  
>    reg: true
> +  label: true
>    local-mac-address: true
>    mac-address: true
>  
> @@ -59,6 +60,7 @@ examples:
>          ethernet@1 {
>              compatible = "usb424,ec00";
>              reg = <1>;
> +            label = "LAN0";
>              local-mac-address = [00 00 00 00 00 00];
>          };
>      };
> diff --git a/Documentation/devicetree/bindings/net/usbnet.yaml b/Documentation/devicetree/bindings/net/usbnet.yaml
> new file mode 100644
> index 000000000000..fe0848433263
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/usbnet.yaml
> @@ -0,0 +1,36 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/usbnet.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: The device tree bindings for the USB Ethernet controllers
> +
> +maintainers:
> +  - Oleksij Rempel <o.rempel@pengutronix.de>
> +
> +description: |
> +  Device tree properties for hard wired USB Ethernet devices.
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#
> +
> +properties:
> +  compatible: true
> +
> +  reg:
> +    description: Port number
> +
> +  label:
> +    description:
> +      Describes the label associated with this port, which will become
> +      the netdev name
> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: true
> +
> +...
> -- 
> 2.30.2
> 
> 

[PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Oleksij Rempel]

Similar to the option to set a netdev name in device tree for switch
ports by using the property "label" in the DSA framework, this patch
adds this functionality to the usbnet infrastructure.

This will help to name the interfaces properly throughout supported
devices. This provides stable interface names which are useful
especially in embedded use cases.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/usb/usbnet.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 9a6450f796dc..3fdca0cfda88 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -33,6 +33,7 @@
 #include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/pm_runtime.h>
+#include <linux/of.h>
 
 /*-------------------------------------------------------------------------*/
 
@@ -1762,6 +1763,20 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
 		if ((dev->driver_info->flags & FLAG_WWAN) != 0)
 			strscpy(net->name, "wwan%d", sizeof(net->name));
 
+		if (IS_ENABLED(CONFIG_OF)) {
+			const char *label = NULL;
+
+			/* try reading label from device tree node */
+			if (xdev->dev.of_node)
+				label = of_get_property(xdev->dev.of_node,
+							"label", NULL);
+			if (label) {
+				strscpy(net->name, label, sizeof(net->name));
+				dev_info(&udev->dev, "netdev name from dt: %s\n",
+					 net->name);
+			}
+		}
+
 		/* devices that cannot do ARP */
 		if ((dev->driver_info->flags & FLAG_NOARP) != 0)
 			net->flags |= IFF_NOARP;
-- 
2.30.2

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Greg KH]

On Thu, Jan 27, 2022 at 11:49:05AM +0100, Oleksij Rempel wrote:
> Similar to the option to set a netdev name in device tree for switch
> ports by using the property "label" in the DSA framework, this patch
> adds this functionality to the usbnet infrastructure.
> 
> This will help to name the interfaces properly throughout supported
> devices. This provides stable interface names which are useful
> especially in embedded use cases.

Stable interface names are for userspace to set, not the kernel.

Why would USB care about this?  If you need something like this, get it
from the USB device itself, not DT, which should have nothing to do with
USB as USB is a dynamic, self-describing, bus.  Unlike DT.

So I do not think this is a good idea.

greg k-h

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Oleksij Rempel]

On Thu, Jan 27, 2022 at 11:57:26AM +0100, Greg KH wrote:
> On Thu, Jan 27, 2022 at 11:49:05AM +0100, Oleksij Rempel wrote:
> > Similar to the option to set a netdev name in device tree for switch
> > ports by using the property "label" in the DSA framework, this patch
> > adds this functionality to the usbnet infrastructure.
> > 
> > This will help to name the interfaces properly throughout supported
> > devices. This provides stable interface names which are useful
> > especially in embedded use cases.
> 
> Stable interface names are for userspace to set, not the kernel.
> 
> Why would USB care about this?  If you need something like this, get it
> from the USB device itself, not DT, which should have nothing to do with
> USB as USB is a dynamic, self-describing, bus.  Unlike DT.
> 
> So I do not think this is a good idea.

This is needed for embedded devices with integrated USB Ethernet
controller. Currently I have following use cases to solve:
- Board with one or multiple USB Ethernet controllers with external PHY.
  The PHY need devicetree to describe IRQ, clock sources, label on board, etc.
- Board with USB Ethernet controller with DSA switch. The USB ethernet
  controller is attached to the CPU port of DSA switch. In this case,
  DSA switch is the sub-node of the USB device. The CPU port should have
  stable name for all device related to this product.

Using user space tools to name interfaces would double the maintenance
of similar information: DT - describing the HW + udev scripts describing
same HW again.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Greg KH]

On Thu, Jan 27, 2022 at 12:23:05PM +0100, Oleksij Rempel wrote:
> On Thu, Jan 27, 2022 at 11:57:26AM +0100, Greg KH wrote:
> > On Thu, Jan 27, 2022 at 11:49:05AM +0100, Oleksij Rempel wrote:
> > > Similar to the option to set a netdev name in device tree for switch
> > > ports by using the property "label" in the DSA framework, this patch
> > > adds this functionality to the usbnet infrastructure.
> > > 
> > > This will help to name the interfaces properly throughout supported
> > > devices. This provides stable interface names which are useful
> > > especially in embedded use cases.
> > 
> > Stable interface names are for userspace to set, not the kernel.
> > 
> > Why would USB care about this?  If you need something like this, get it
> > from the USB device itself, not DT, which should have nothing to do with
> > USB as USB is a dynamic, self-describing, bus.  Unlike DT.
> > 
> > So I do not think this is a good idea.
> 
> This is needed for embedded devices with integrated USB Ethernet
> controller. Currently I have following use cases to solve:
> - Board with one or multiple USB Ethernet controllers with external PHY.
>   The PHY need devicetree to describe IRQ, clock sources, label on board, etc.

The phy is for the USB controller, not the Ethernet controller, right?
If for the ethernet controller, ugh, that's a crazy design and I would
argue a broken one.  But whatever, DT should not be used to describe a
USB device itself.

> - Board with USB Ethernet controller with DSA switch. The USB ethernet
>   controller is attached to the CPU port of DSA switch. In this case,
>   DSA switch is the sub-node of the USB device.

What do you mean exactly by "sub node"?  USB does not have such a term.

>  The CPU port should have
>   stable name for all device related to this product.

name for who to use?  Userspace?  Or within the kernel?

Naming is done by userspace, as USB is NOT determinisitic in numbering /
naming the devices attached to it, by design.  If you need to have a
stable name, do so in userspace please, we have loads of tools that
already do this there today.  Let's not reinvent the wheel.

> Using user space tools to name interfaces would double the maintenance
> of similar information: DT - describing the HW + udev scripts describing
> same HW again.

Not for the network name of the device, that belongs in userspace.

Do not be listing USB device ids in a DT file, that way lies madness.

thanks,

greg k-h

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Oleksij Rempel]

On Thu, Jan 27, 2022 at 12:30:20PM +0100, Greg KH wrote:
> On Thu, Jan 27, 2022 at 12:23:05PM +0100, Oleksij Rempel wrote:
> > On Thu, Jan 27, 2022 at 11:57:26AM +0100, Greg KH wrote:
> > > On Thu, Jan 27, 2022 at 11:49:05AM +0100, Oleksij Rempel wrote:
> > > > Similar to the option to set a netdev name in device tree for switch
> > > > ports by using the property "label" in the DSA framework, this patch
> > > > adds this functionality to the usbnet infrastructure.
> > > > 
> > > > This will help to name the interfaces properly throughout supported
> > > > devices. This provides stable interface names which are useful
> > > > especially in embedded use cases.
> > > 
> > > Stable interface names are for userspace to set, not the kernel.
> > > 
> > > Why would USB care about this?  If you need something like this, get it
> > > from the USB device itself, not DT, which should have nothing to do with
> > > USB as USB is a dynamic, self-describing, bus.  Unlike DT.
> > > 
> > > So I do not think this is a good idea.
> > 
> > This is needed for embedded devices with integrated USB Ethernet
> > controller. Currently I have following use cases to solve:
> > - Board with one or multiple USB Ethernet controllers with external PHY.
> >   The PHY need devicetree to describe IRQ, clock sources, label on board, etc.
> 
> The phy is for the USB controller, not the Ethernet controller, right?
> If for the ethernet controller, ugh, that's a crazy design and I would
> argue a broken one.  But whatever, DT should not be used to describe a
> USB device itself.
> 
> > - Board with USB Ethernet controller with DSA switch. The USB ethernet
> >   controller is attached to the CPU port of DSA switch. In this case,
> >   DSA switch is the sub-node of the USB device.
> 
> What do you mean exactly by "sub node"?  USB does not have such a term.

Here are some examples:

  - |
    usb@11270000 {
        reg = <0x11270000 0x1000>;
        #address-cells = <1>;
        #size-cells = <0>;

        ethernet@1 {
            compatible = "usb424,ec00";
            reg = <1>;
            label = "LAN0";
	    // there is no internal eeprom, so MAC address is taken from
	    // NVMEM of the SoC.
            local-mac-address = [00 00 00 00 00 00];

            mdio {
		ethernet-phy@4 {
			reg = <4>;
			// Interrupt is attached to the SoC or the GPIO
			// controller of the same USB devices.
			interrupts-extended = <&gpio1 28 IRQ_TYPE_LEVEL_LOW>;
			// same about reset. It is attached to the SoC
			// or GPIO controller of the USB device.
			reset-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>;
			reset-assert-us = <10000>;
			reset-deassert-us = <1000>;
			// some external clock provider
			clocks = <&clk>
			qca,smarteee-tw-us-1g = <24>;
			qca,clk-out-frequency = <125000000>;
		};
            };
        };
    };
  - |
    usb@11270000 {
        reg = <0x11270000 0x1000>;
        #address-cells = <1>;
        #size-cells = <0>;

        usb1@1 {
            compatible = "usb424,9514";
            reg = <1>;
            #address-cells = <1>;
            #size-cells = <0>;

            eth0: ethernet@1 {
               compatible = "usb424,ec00";
               reg = <1>;
               label = "cpu0";

               fixed-link {
                   speed = <1000>;
                   full-duplex;
               };

               // managment interface of the switch is attached to the
	       // MDIO bus of this USB device.
               mdio {
                switch@0 {
                    compatible = "microchip,ksz9477";
                    reg = <0>;
		    // reset is controlled by the SoC or by the GPIO
		    // controller of this USB device.
                    reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;

                    ethernet-ports {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        port@0 {
                            reg = <0>;
                            label = "lan1";
                        };
                        port@1 {
                            reg = <1>;
                            label = "lan2";
                        };
                        port@2 {
                            reg = <2>;
                            label = "lan3";
                        };
                        port@3 {
                            reg = <3>;
                            label = "lan4";
                        };
                        port@4 {
                            reg = <4>;
                            label = "lan5";
                        };
                        port@5 {
                            reg = <5>;
                            label = "cpu";
                            ethernet = <&eth0>;
                            fixed-link {
                                speed = <1000>;
                                full-duplex;
                            };
                        };
                    };
                };
               };
            };
        };
    };


> >  The CPU port should have
> >   stable name for all device related to this product.
> 
> name for who to use?  Userspace?  Or within the kernel?
> 
> Naming is done by userspace, as USB is NOT determinisitic in numbering /
> naming the devices attached to it, by design.  If you need to have a
> stable name, do so in userspace please, we have loads of tools that
> already do this there today.  Let's not reinvent the wheel.
> 
> > Using user space tools to name interfaces would double the maintenance
> > of similar information: DT - describing the HW + udev scripts describing
> > same HW again.
> 
> Not for the network name of the device, that belongs in userspace.
> 
> Do not be listing USB device ids in a DT file, that way lies madness.
> 
> thanks,
> 
> greg k-h
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Greg KH]

On Thu, Jan 27, 2022 at 01:00:39PM +0100, Oleksij Rempel wrote:
> On Thu, Jan 27, 2022 at 12:30:20PM +0100, Greg KH wrote:
> > On Thu, Jan 27, 2022 at 12:23:05PM +0100, Oleksij Rempel wrote:
> > > On Thu, Jan 27, 2022 at 11:57:26AM +0100, Greg KH wrote:
> > > > On Thu, Jan 27, 2022 at 11:49:05AM +0100, Oleksij Rempel wrote:
> > > > > Similar to the option to set a netdev name in device tree for switch
> > > > > ports by using the property "label" in the DSA framework, this patch
> > > > > adds this functionality to the usbnet infrastructure.
> > > > > 
> > > > > This will help to name the interfaces properly throughout supported
> > > > > devices. This provides stable interface names which are useful
> > > > > especially in embedded use cases.
> > > > 
> > > > Stable interface names are for userspace to set, not the kernel.
> > > > 
> > > > Why would USB care about this?  If you need something like this, get it
> > > > from the USB device itself, not DT, which should have nothing to do with
> > > > USB as USB is a dynamic, self-describing, bus.  Unlike DT.
> > > > 
> > > > So I do not think this is a good idea.
> > > 
> > > This is needed for embedded devices with integrated USB Ethernet
> > > controller. Currently I have following use cases to solve:
> > > - Board with one or multiple USB Ethernet controllers with external PHY.
> > >   The PHY need devicetree to describe IRQ, clock sources, label on board, etc.
> > 
> > The phy is for the USB controller, not the Ethernet controller, right?
> > If for the ethernet controller, ugh, that's a crazy design and I would
> > argue a broken one.  But whatever, DT should not be used to describe a
> > USB device itself.
> > 
> > > - Board with USB Ethernet controller with DSA switch. The USB ethernet
> > >   controller is attached to the CPU port of DSA switch. In this case,
> > >   DSA switch is the sub-node of the USB device.
> > 
> > What do you mean exactly by "sub node"?  USB does not have such a term.
> 
> Here are some examples:
> 
>   - |
>     usb@11270000 {
>         reg = <0x11270000 0x1000>;

How can a USB device have a register?

And what does 11270000 mean?


>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         ethernet@1 {
>             compatible = "usb424,ec00";
>             reg = <1>;
>             label = "LAN0";

Where did that come from?  That should be added in userspace, not from
the kernel.

> 	    // there is no internal eeprom, so MAC address is taken from
> 	    // NVMEM of the SoC.
>             local-mac-address = [00 00 00 00 00 00];
> 
>             mdio {
> 		ethernet-phy@4 {
> 			reg = <4>;
> 			// Interrupt is attached to the SoC or the GPIO
> 			// controller of the same USB devices.
> 			interrupts-extended = <&gpio1 28 IRQ_TYPE_LEVEL_LOW>;
> 			// same about reset. It is attached to the SoC
> 			// or GPIO controller of the USB device.
> 			reset-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>;
> 			reset-assert-us = <10000>;
> 			reset-deassert-us = <1000>;
> 			// some external clock provider
> 			clocks = <&clk>
> 			qca,smarteee-tw-us-1g = <24>;
> 			qca,clk-out-frequency = <125000000>;

So this device does not follow the spec for this driver in that you have
to get the values for the phy from DT and not the device itself?  Why
not fix the firmware in the device to report this?

Anyway, this feels really wrong, USB should not be involved in DT by
virtue of how the bus was designed.

And again, pick your names in userspace, embedded is not "special" here.
You can do persistant network device names in a very trivial shell
script if needed, we used to do it that way 18 years ago :)

thanks,

greg k-h

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Lucas Stach]

Hi Greg,

Am Donnerstag, dem 27.01.2022 um 14:21 +0100 schrieb Greg KH:
> On Thu, Jan 27, 2022 at 01:00:39PM +0100, Oleksij Rempel wrote:
> > On Thu, Jan 27, 2022 at 12:30:20PM +0100, Greg KH wrote:
> > > On Thu, Jan 27, 2022 at 12:23:05PM +0100, Oleksij Rempel wrote:
> > > > On Thu, Jan 27, 2022 at 11:57:26AM +0100, Greg KH wrote:
> > > > > On Thu, Jan 27, 2022 at 11:49:05AM +0100, Oleksij Rempel wrote:
> > > > > > Similar to the option to set a netdev name in device tree for switch
> > > > > > ports by using the property "label" in the DSA framework, this patch
> > > > > > adds this functionality to the usbnet infrastructure.
> > > > > > 
> > > > > > This will help to name the interfaces properly throughout supported
> > > > > > devices. This provides stable interface names which are useful
> > > > > > especially in embedded use cases.
> > > > > 
> > > > > Stable interface names are for userspace to set, not the kernel.
> > > > > 
> > > > > Why would USB care about this?  If you need something like this, get it
> > > > > from the USB device itself, not DT, which should have nothing to do with
> > > > > USB as USB is a dynamic, self-describing, bus.  Unlike DT.
> > > > > 
> > > > > So I do not think this is a good idea.
> > > > 
> > > > This is needed for embedded devices with integrated USB Ethernet
> > > > controller. Currently I have following use cases to solve:
> > > > - Board with one or multiple USB Ethernet controllers with external PHY.
> > > >   The PHY need devicetree to describe IRQ, clock sources, label on board, etc.
> > > 
> > > The phy is for the USB controller, not the Ethernet controller, right?
> > > If for the ethernet controller, ugh, that's a crazy design and I would
> > > argue a broken one.  But whatever, DT should not be used to describe a
> > > USB device itself.
> > > 
> > > > - Board with USB Ethernet controller with DSA switch. The USB ethernet
> > > >   controller is attached to the CPU port of DSA switch. In this case,
> > > >   DSA switch is the sub-node of the USB device.
> > > 
> > > What do you mean exactly by "sub node"?  USB does not have such a term.
> > 
> > Here are some examples:
> > 
> >   - |
> >     usb@11270000 {
> >         reg = <0x11270000 0x1000>;
> 
> How can a USB device have a register?
> 
> And what does 11270000 mean?
> 
> 
> >         #address-cells = <1>;
> >         #size-cells = <0>;
> > 
> >         ethernet@1 {
> >             compatible = "usb424,ec00";
> >             reg = <1>;
> >             label = "LAN0";
> 
> Where did that come from?  That should be added in userspace, not from
> the kernel.
> 
> > 	    // there is no internal eeprom, so MAC address is taken from
> > 	    // NVMEM of the SoC.
> >             local-mac-address = [00 00 00 00 00 00];
> > 
> >             mdio {
> > 		ethernet-phy@4 {
> > 			reg = <4>;
> > 			// Interrupt is attached to the SoC or the GPIO
> > 			// controller of the same USB devices.
> > 			interrupts-extended = <&gpio1 28 IRQ_TYPE_LEVEL_LOW>;
> > 			// same about reset. It is attached to the SoC
> > 			// or GPIO controller of the USB device.
> > 			reset-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>;
> > 			reset-assert-us = <10000>;
> > 			reset-deassert-us = <1000>;
> > 			// some external clock provider
> > 			clocks = <&clk>
> > 			qca,smarteee-tw-us-1g = <24>;
> > 			qca,clk-out-frequency = <125000000>;
> 
> So this device does not follow the spec for this driver in that you have
> to get the values for the phy from DT and not the device itself?  Why
> not fix the firmware in the device to report this?
> 
> Anyway, this feels really wrong, USB should not be involved in DT by
> virtue of how the bus was designed.

While one can argue about the kind of information provided here, it is
well defined how DT can augment the information about a device on a
runtime discoverable bus like USB. There is even a DT binding that
lists you as the maintainer of this standard:
Documentation/devicetree/bindings/usb/usb-device.yaml

USB is not special here, PCI has the same way for DT to augment runtime
discovered device information and that is even covered by the ancient
IEEE 1275 Open Firmware standard.

Regards,
Lucas

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Sergey Ryazanov]

Hello,

On Fri, Jan 28, 2022 at 5:57 AM Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Donnerstag, dem 27.01.2022 um 14:21 +0100 schrieb Greg KH:
>> On Thu, Jan 27, 2022 at 01:00:39PM +0100, Oleksij Rempel wrote:
>>> On Thu, Jan 27, 2022 at 12:30:20PM +0100, Greg KH wrote:
>>>> On Thu, Jan 27, 2022 at 12:23:05PM +0100, Oleksij Rempel wrote:
>>>>> On Thu, Jan 27, 2022 at 11:57:26AM +0100, Greg KH wrote:
>>>>>> On Thu, Jan 27, 2022 at 11:49:05AM +0100, Oleksij Rempel wrote:
>>>>>>> Similar to the option to set a netdev name in device tree for switch
>>>>>>> ports by using the property "label" in the DSA framework, this patch
>>>>>>> adds this functionality to the usbnet infrastructure.
>>>>>>>
>>>>>>> This will help to name the interfaces properly throughout supported
>>>>>>> devices. This provides stable interface names which are useful
>>>>>>> especially in embedded use cases.
>>>>>>
>>>>>> Stable interface names are for userspace to set, not the kernel.
>>>>>>
>>>>>> Why would USB care about this?  If you need something like this, get it
>>>>>> from the USB device itself, not DT, which should have nothing to do with
>>>>>> USB as USB is a dynamic, self-describing, bus.  Unlike DT.
>>>>>>
>>>>>> So I do not think this is a good idea.
>>>>>
>>>>> This is needed for embedded devices with integrated USB Ethernet
>>>>> controller. Currently I have following use cases to solve:
>>>>> - Board with one or multiple USB Ethernet controllers with external PHY.
>>>>>   The PHY need devicetree to describe IRQ, clock sources, label on board, etc.
>>>>
>>>> The phy is for the USB controller, not the Ethernet controller, right?
>>>> If for the ethernet controller, ugh, that's a crazy design and I would
>>>> argue a broken one.  But whatever, DT should not be used to describe a
>>>> USB device itself.
>>>>
>>>>> - Board with USB Ethernet controller with DSA switch. The USB ethernet
>>>>>   controller is attached to the CPU port of DSA switch. In this case,
>>>>>   DSA switch is the sub-node of the USB device.
>>>>
>>>> What do you mean exactly by "sub node"?  USB does not have such a term.
>>>
>>> Here are some examples:
>>>
>>>   - |
>>>     usb@11270000 {
>>>         reg = <0x11270000 0x1000>;
>>
>> How can a USB device have a register?
>>
>> And what does 11270000 mean?
>>
>>
>>>         #address-cells = <1>;
>>>         #size-cells = <0>;
>>>
>>>         ethernet@1 {
>>>             compatible = "usb424,ec00";
>>>             reg = <1>;
>>>             label = "LAN0";
>>
>> Where did that come from?  That should be added in userspace, not from
>> the kernel.
>>
>>>         // there is no internal eeprom, so MAC address is taken from
>>>         // NVMEM of the SoC.
>>>             local-mac-address = [00 00 00 00 00 00];
>>>
>>>             mdio {
>>>             ethernet-phy@4 {
>>>                     reg = <4>;
>>>                     // Interrupt is attached to the SoC or the GPIO
>>>                     // controller of the same USB devices.
>>>                     interrupts-extended = <&gpio1 28 IRQ_TYPE_LEVEL_LOW>;
>>>                     // same about reset. It is attached to the SoC
>>>                     // or GPIO controller of the USB device.
>>>                     reset-gpios = <&gpio3 31 GPIO_ACTIVE_LOW>;
>>>                     reset-assert-us = <10000>;
>>>                     reset-deassert-us = <1000>;
>>>                     // some external clock provider
>>>                     clocks = <&clk>
>>>                     qca,smarteee-tw-us-1g = <24>;
>>>                     qca,clk-out-frequency = <125000000>;
>>
>> So this device does not follow the spec for this driver in that you have
>> to get the values for the phy from DT and not the device itself?  Why
>> not fix the firmware in the device to report this?
>>
>> Anyway, this feels really wrong, USB should not be involved in DT by
>> virtue of how the bus was designed.
>
> While one can argue about the kind of information provided here, it is
> well defined how DT can augment the information about a device on a
> runtime discoverable bus like USB. There is even a DT binding that
> lists you as the maintainer of this standard:
> Documentation/devicetree/bindings/usb/usb-device.yaml
>
> USB is not special here, PCI has the same way for DT to augment runtime
> discovered device information and that is even covered by the ancient
> IEEE 1275 Open Firmware standard.

Not to defend this particular patch, but to talk about USB in general.

USB is not stable at all in terms of the run-time device number, but
USB is perfectly stable in terms of connection ports. If you traverse
from a host controller through a sequence of connection ports then you
reach the same device. As for PCI or any other bus. The document
referenced by Lucas shows this.

Moreover, USB specification even allows you to mark a device connected
to a particular hub port as non-detachable. What means that such a
device will be just a block on a PCB. And in this case, FDT becomes a
good (only?) option to store a subordinated device configuration.
E.g. as in Oleksij's example with a DSA switch that is connected to an
Ethernet MAC controller with the USB interface.

-- 
BR,
Sergey

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Sergey Ryazanov]

Hello Greg,

if I may be allowed, I would like to make a couple of points about
specifying network interface names in DT. As in previous mail, not to
defend this particular patch, but to talk about names assignment in
general.

I may be totally wrong, so consider my words as a request for
discussion. I have been thinking about an efficient way for network
device names assignment for routers with a fixed configuration and
have always come to a conclusion that DT is a good place for names
storage. Recent DSA capability to assign names from labels and this
patch by Oleksij show that I am not alone.

On Fri, Jan 28, 2022 at 3:34 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> And again, pick your names in userspace, embedded is not "special" here.

Embedded is not a special case, but fixed configuration is.

> You can do persistant network device names in a very trivial shell
> script if needed, we used to do it that way 18 years ago :)

Network device name is not a solely userspace entity. It is part of
the interface between the kernel and userspace software.

Sure, persistent names can be established with a userspace script. But
this implies the device renaming, which is a complex and race prone
task. Once I even found a comment in the kernel code that only network
devices could be renamed and this is a headache. As for userspace, it
is possible to workaround the device renaming issues. But this
requires a lot of code in many programs and sometimes even special
conventions on a programs interaction. E.g. consider a case where a
service would like to bind to a network interface, which is in the
middle of renaming by udev. On the other hand, we have the kernel that
could provide predictable names from the beginning for all software on
a host. So this is a desired option.

As for DT, this is an excellent database with perfectly established
relations to hardware configuration. And if we try to implement a
userspace storage with the network device names, then we will just
duplicate the DT in the userspace, as already was mentioned by
Oleksij. To me, implementation of a names database in userspace looks
more like reinventing the  DT (wheel) than adding device names to the
DT.

To summarize, we (developers of embedded software) have two related needs:
1) the need for persistent names provided by the kernel,
2) using the DT as a source of persistent names for (1).

Greg, what do you think about device names storing in DeviceTree in
the above context? Does it still make no sense?

-- 
Sergey

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Andrew Lunn]

On Thu, Feb 03, 2022 at 05:20:34AM +0300, Sergey Ryazanov wrote:
> Hello Greg,
> 
> if I may be allowed, I would like to make a couple of points about
> specifying network interface names in DT. As in previous mail, not to
> defend this particular patch, but to talk about names assignment in
> general.
> 
> I may be totally wrong, so consider my words as a request for
> discussion. I have been thinking about an efficient way for network
> device names assignment for routers with a fixed configuration and
> have always come to a conclusion that DT is a good place for names
> storage. Recent DSA capability to assign names from labels and this
> patch by Oleksij show that I am not alone.

DSA doing this is not recent. The first patch implementing DSA in 2008
had the ability to set the interface names. This was long before the
idea that userspace should set interface names became the 'correct'
way to do this.

The current thinking for routers which don't make use of the DSA
framework, it to use interface names like swXpY, where X is the switch
number and Y is the port number. udev can make use of for example
/sys/class/net/*/phys_port_name to get the pY bit to give the
interface its full name.

	Andrew

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Greg KH]

On Thu, Feb 03, 2022 at 02:04:28PM +0100, Andrew Lunn wrote:
> On Thu, Feb 03, 2022 at 05:20:34AM +0300, Sergey Ryazanov wrote:
> > Hello Greg,
> > 
> > if I may be allowed, I would like to make a couple of points about
> > specifying network interface names in DT. As in previous mail, not to
> > defend this particular patch, but to talk about names assignment in
> > general.
> > 
> > I may be totally wrong, so consider my words as a request for
> > discussion. I have been thinking about an efficient way for network
> > device names assignment for routers with a fixed configuration and
> > have always come to a conclusion that DT is a good place for names
> > storage. Recent DSA capability to assign names from labels and this
> > patch by Oleksij show that I am not alone.
> 
> DSA doing this is not recent. The first patch implementing DSA in 2008
> had the ability to set the interface names. This was long before the
> idea that userspace should set interface names became the 'correct'
> way to do this.

udev came out in 2003, and we had the goal of having userspace do all of
the device naming way back then, so DSA was late to the game :)

thanks,

greg k-h

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Andrew Lunn]

> - Board with USB Ethernet controller with DSA switch. The USB ethernet
>   controller is attached to the CPU port of DSA switch. In this case,
>   DSA switch is the sub-node of the USB device. The CPU port should have
>   stable name for all device related to this product.

All that DSA requires is a phandle pointing to the MAC. It does not
care what the interface name is. It should not be an issue if udev
changes the name to something your product marketing guys say it
should be.

   Andrew

Re: [PATCH net-next v1 4/4] usbnet: add support for label from device tree

[Rob Herring]

On Thu, Jan 27, 2022 at 11:49:05AM +0100, Oleksij Rempel wrote:
> Similar to the option to set a netdev name in device tree for switch
> ports by using the property "label" in the DSA framework, this patch
> adds this functionality to the usbnet infrastructure.
> 
> This will help to name the interfaces properly throughout supported
> devices. This provides stable interface names which are useful
> especially in embedded use cases.

I'll pile on... The purpose of 'label' is to describe a human readable 
label on a port of a box. It should otherwise be opaque to s/w. Yes, 
there are abuses of that in the kernel, but don't add more.

Rob

Re: [PATCH net-next v1 0/4] usbnet: add "label" support

[Greg KH]

On Thu, Jan 27, 2022 at 11:49:01AM +0100, Oleksij Rempel wrote:
> Add devicetree label property for usbnet devices and related yaml
> schema.

That says _what_ you are doing, but not _why_ you would want to do such
a crazy thing, nor what problem you are attempting to solve here.

thanks,

greg k-h

Re: [PATCH net-next v1 0/4] usbnet: add "label" support

[Oliver Neukum]

On 27.01.22 11:57, Greg KH wrote:
> On Thu, Jan 27, 2022 at 11:49:01AM +0100, Oleksij Rempel wrote:
>> Add devicetree label property for usbnet devices and related yaml
>> schema.
> That says _what_ you are doing, but not _why_ you would want to do such
> a crazy thing, nor what problem you are attempting to solve here.

Hi,

could you at least describe what kind of systems we are talking
about? Is this for a limited set of embedded devices?
Are we talking about devices embedded on a motherboard,
which happen to be connected by USB?

That is, are we talking about another kind of firmware
we are to take information about devices from?
And if so, why are you proposing to solve this on the
USB driver level?
It looks to me like those devices are addressed by
their USB path. But still there is no reason that a USB
driver should actively interpret firmware stuff that
comes from a source that tells us nothing about USB
properties.
In other words it looks to me like you are trying to put
a generic facility for getting device properties into
a specific driver. The question whether device names
should be read out of firmware is not a USB question.

I would suggest you implement a generic facility
in the network layer and if everybody is happy with that
obviously usbnet can pass through a pointer for that
to operate on. Frankly, it looks to me like you are
implementing only a subset of what device tree
could contain for your specific use case.

    Regards
        Oliver

Re: [PATCH net-next v1 0/4] usbnet: add "label" support

[Oleksij Rempel]

On Thu, Feb 03, 2022 at 10:34:25AM +0100, Oliver Neukum wrote:
> 
> On 27.01.22 11:57, Greg KH wrote:
> > On Thu, Jan 27, 2022 at 11:49:01AM +0100, Oleksij Rempel wrote:
> >> Add devicetree label property for usbnet devices and related yaml
> >> schema.
> > That says _what_ you are doing, but not _why_ you would want to do such
> > a crazy thing, nor what problem you are attempting to solve here.
> 
> could you at least describe what kind of systems we are talking
> about? Is this for a limited set of embedded devices?
> Are we talking about devices embedded on a motherboard,
> which happen to be connected by USB?

In this particular use case there is a PCB with a imx6 SoC with hard
wired USB attached USB-Ethernet-MAC adapters. One of these adapters is
connected in the same PCB to an Ethernet switch chip. There is a DSA
driver for the switch, so we want to describe the whole boards in a DT.
Putting a label in the DT that renames the network interface is "nice to
have" but not so important.

As the DT DSA bindings rely on linking a MAC phandle to the switch we
need to describe the USB Ethernet adapter in the DT, this is more
important. See this discussion:

https://lore.kernel.org/all/20220127120039.GE9150@pengutronix.de/

> That is, are we talking about another kind of firmware
> we are to take information about devices from?

There is no other firmware involved. The switch chip is attached via
RGMII to the USB/MAC and with SPI to the CPU for the configuration
interface. (I2C to the CPU or MDIO to the USB/MAC would be another
option for the configuration interface.)

> And if so, why are you proposing to solve this on the
> USB driver level?
> It looks to me like those devices are addressed by
> their USB path. But still there is no reason that a USB
> driver should actively interpret firmware stuff that
> comes from a source that tells us nothing about USB
> properties.
> In other words it looks to me like you are trying to put
> a generic facility for getting device properties into
> a specific driver. The question whether device names
> should be read out of firmware is not a USB question.
> 
> I would suggest you implement a generic facility
> in the network layer and if everybody is happy with that
> obviously usbnet can pass through a pointer for that
> to operate on. Frankly, it looks to me like you are
> implementing only a subset of what device tree
> could contain for your specific use case.

Sounds good, but we'll focus on the DSA use case, as this is more
important. So patches 1 and 2 of this patches set have highest prio for
us.

Regards,
Oleksij & Marc
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v1 0/4] usbnet: add "label" support

[Oliver Neukum]

On 03.02.22 11:27, Oleksij Rempel wrote:
> On Thu, Feb 03, 2022 at 10:34:25AM +0100, Oliver Neukum wrote:
>> On 27.01.22 11:57, Greg KH wrote:
>>> On Thu, Jan 27, 2022 at 11:49:01AM +0100, Oleksij Rempel wrote:
>>>
> In this particular use case there is a PCB with a imx6 SoC with hard
> wired USB attached USB-Ethernet-MAC adapters. One of these adapters is
> connected in the same PCB to an Ethernet switch chip. There is a DSA
> driver for the switch, so we want to describe the whole boards in a DT.
OK, so you are talking about what is technically an embedded
device with a DT as is usual for such devices.
> Putting a label in the DT that renames the network interface is "nice to
> have" but not so important.
Well, this applies to your particular device only, doesn't it?
>
> As the DT DSA bindings rely on linking a MAC phandle to the switch we
> need to describe the USB Ethernet adapter in the DT, this is more
> important. See this discussion:
>
> https://lore.kernel.org/all/20220127120039.GE9150@pengutronix.de/
And this one irks me. The USB list is not the place to talk about
how to build switches. The question here is whether OF and
DSA have features that need support in USB drivers.

I am not ready to discuss the merits of features in OF
>> I would suggest you implement a generic facility
>> in the network layer and if everybody is happy with that
>> obviously usbnet can pass through a pointer for that
>> to operate on. Frankly, it looks to me like you are
>> implementing only a subset of what device tree
>> could contain for your specific use case.
> Sounds good, but we'll focus on the DSA use case, as this is more
> important. So patches 1 and 2 of this patches set have highest prio for
> us.
It looks to me like you want a layering violation for
a special case. Is there any reason for you not to provide
a generic helper in the networking core?

    Regards
        Oliver