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

From: Oleksij Rempel
Date: Thu Jan 27 2022 - 07:00:52 EST

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 |