diff mbox series

usb: eth: add print of device name on usb_ether registration

Message ID 20210325165639.20876-1-tharvey@gateworks.com
State Changes Requested
Delegated to: Marek Vasut
Headers show
Series usb: eth: add print of device name on usb_ether registration | expand

Commit Message

Tim Harvey March 25, 2021, 4:56 p.m. UTC
When a USB Ethernet device is detected and registered there is no
way to know the device-name in order to use it with the ethact
env variable.

Add a print to display the device name that is registered.

Example using an AX88x72A USB Ethernet device:

GW6404-B> usb start
starting USB...
Bus xhci_pci: Register 2000140 NbrPorts 2
Starting the controller
USB XHCI 1.00
Bus xhci_pci: Register 2000140 NbrPorts 2
Starting the controller
USB XHCI 1.00
scanning bus xhci_pci for devices...
       found ethernet device: asix_eth

GW6404-B> setenv ethact asix_eth # select usb ethernet

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 drivers/usb/eth/usb_ether.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Marek Vasut March 25, 2021, 5:06 p.m. UTC | #1
On 3/25/21 5:56 PM, Tim Harvey wrote:
> When a USB Ethernet device is detected and registered there is no
> way to know the device-name in order to use it with the ethact
> env variable.
> 
> Add a print to display the device name that is registered.
> 
> Example using an AX88x72A USB Ethernet device:
> 
> GW6404-B> usb start
> starting USB...
> Bus xhci_pci: Register 2000140 NbrPorts 2
> Starting the controller
> USB XHCI 1.00
> Bus xhci_pci: Register 2000140 NbrPorts 2
> Starting the controller
> USB XHCI 1.00
> scanning bus xhci_pci for devices...
>         found ethernet device: asix_eth
> 
> GW6404-B> setenv ethact asix_eth # select usb ethernet
> 
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>   drivers/usb/eth/usb_ether.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/eth/usb_ether.c b/drivers/usb/eth/usb_ether.c
> index e368ecda0d..916c4e1c85 100644
> --- a/drivers/usb/eth/usb_ether.c
> +++ b/drivers/usb/eth/usb_ether.c
> @@ -86,6 +86,7 @@ int usb_ether_register(struct udevice *dev, struct ueth_data *ueth, int rxsize)
>   		return ret;
>   	}
>   	ueth->pusb_dev = udev;
> +	printf("\n       found ethernet device: %s\n", dev->name);

What if I have 2 asix_eth plugged in ? Do I get two identical prints ?

btw. it might be useful to have some "net list" command, like "mmc list" 
which would list all ethernet devices in the system.
Tim Harvey March 25, 2021, 5:38 p.m. UTC | #2
On Thu, Mar 25, 2021 at 10:06 AM Marek Vasut <marex@denx.de> wrote:
>
> On 3/25/21 5:56 PM, Tim Harvey wrote:
> > When a USB Ethernet device is detected and registered there is no
> > way to know the device-name in order to use it with the ethact
> > env variable.
> >
> > Add a print to display the device name that is registered.
> >
> > Example using an AX88x72A USB Ethernet device:
> >
> > GW6404-B> usb start
> > starting USB...
> > Bus xhci_pci: Register 2000140 NbrPorts 2
> > Starting the controller
> > USB XHCI 1.00
> > Bus xhci_pci: Register 2000140 NbrPorts 2
> > Starting the controller
> > USB XHCI 1.00
> > scanning bus xhci_pci for devices...
> >         found ethernet device: asix_eth
> >
> > GW6404-B> setenv ethact asix_eth # select usb ethernet
> >
> > Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> > ---
> >   drivers/usb/eth/usb_ether.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/usb/eth/usb_ether.c b/drivers/usb/eth/usb_ether.c
> > index e368ecda0d..916c4e1c85 100644
> > --- a/drivers/usb/eth/usb_ether.c
> > +++ b/drivers/usb/eth/usb_ether.c
> > @@ -86,6 +86,7 @@ int usb_ether_register(struct udevice *dev, struct ueth_data *ueth, int rxsize)
> >               return ret;
> >       }
> >       ueth->pusb_dev = udev;
> > +     printf("\n       found ethernet device: %s\n", dev->name);
>
> What if I have 2 asix_eth plugged in ? Do I get two identical prints ?

Marek,

Yes, two identical prints in this case which is not good.

Which makes me wonder why 'setenv asix_eth' even works here as
drivers/usb/eth/asix.c's get_info function creates a unique instance
name.

>
> btw. it might be useful to have some "net list" command, like "mmc list"
> which would list all ethernet devices in the system.

That's what I was wondering as well. There are the following
subcommands in the usb cmd:
usb storage - show details of USB storage devices
usb dev [dev] - show or set current USB storage device
usb part [dev] - print partition table of one or all USB storage    devices

So I was wondering if we should add a:
usb net - show details of USB network devices

by the way, here are a few other devices I have on hand showing the
vid:pid, ethact name, driver Kconfig, and device:
// vid:pid ethact uboot-driver device-name
0bda:8153 r8152_eth USB_ETHER_RTL8152 CableCreation CD0026: USB3 GbE
0b95:1790 ax88179_eth USB_ETHER_ASIX88179 plugable USB3-E1000: USB 3.0
10/100/1000mbps
0b95:7720 asix_eth USB_ETHER_ASIX plugable USB2-E100: USB2.0 10/100
ASIX AX88772A

Best regards,

Tim
Marek Vasut March 25, 2021, 5:40 p.m. UTC | #3
On 3/25/21 6:38 PM, Tim Harvey wrote:
> On Thu, Mar 25, 2021 at 10:06 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/25/21 5:56 PM, Tim Harvey wrote:
>>> When a USB Ethernet device is detected and registered there is no
>>> way to know the device-name in order to use it with the ethact
>>> env variable.
>>>
>>> Add a print to display the device name that is registered.
>>>
>>> Example using an AX88x72A USB Ethernet device:
>>>
>>> GW6404-B> usb start
>>> starting USB...
>>> Bus xhci_pci: Register 2000140 NbrPorts 2
>>> Starting the controller
>>> USB XHCI 1.00
>>> Bus xhci_pci: Register 2000140 NbrPorts 2
>>> Starting the controller
>>> USB XHCI 1.00
>>> scanning bus xhci_pci for devices...
>>>          found ethernet device: asix_eth
>>>
>>> GW6404-B> setenv ethact asix_eth # select usb ethernet
>>>
>>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>>> ---
>>>    drivers/usb/eth/usb_ether.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/usb/eth/usb_ether.c b/drivers/usb/eth/usb_ether.c
>>> index e368ecda0d..916c4e1c85 100644
>>> --- a/drivers/usb/eth/usb_ether.c
>>> +++ b/drivers/usb/eth/usb_ether.c
>>> @@ -86,6 +86,7 @@ int usb_ether_register(struct udevice *dev, struct ueth_data *ueth, int rxsize)
>>>                return ret;
>>>        }
>>>        ueth->pusb_dev = udev;
>>> +     printf("\n       found ethernet device: %s\n", dev->name);
>>
>> What if I have 2 asix_eth plugged in ? Do I get two identical prints ?
> 
> Marek,
> 
> Yes, two identical prints in this case which is not good.
> 
> Which makes me wonder why 'setenv asix_eth' even works here as
> drivers/usb/eth/asix.c's get_info function creates a unique instance
> name.

Seems a bit further investigation into this might be necessary.

>> btw. it might be useful to have some "net list" command, like "mmc list"
>> which would list all ethernet devices in the system.
> 
> That's what I was wondering as well. There are the following
> subcommands in the usb cmd:
> usb storage - show details of USB storage devices
> usb dev [dev] - show or set current USB storage device
> usb part [dev] - print partition table of one or all USB storage    devices
> 
> So I was wondering if we should add a:
> usb net - show details of USB network devices

I don't think such a command should be limited to USB network devices, 
it should be something applicable to all network devices.

> by the way, here are a few other devices I have on hand showing the
> vid:pid, ethact name, driver Kconfig, and device:
> // vid:pid ethact uboot-driver device-name
> 0bda:8153 r8152_eth USB_ETHER_RTL8152 CableCreation CD0026: USB3 GbE
> 0b95:1790 ax88179_eth USB_ETHER_ASIX88179 plugable USB3-E1000: USB 3.0
> 10/100/1000mbps
> 0b95:7720 asix_eth USB_ETHER_ASIX plugable USB2-E100: USB2.0 10/100
> ASIX AX88772A
Tim Harvey March 25, 2021, 10:38 p.m. UTC | #4
On Thu, Mar 25, 2021 at 10:40 AM Marek Vasut <marex@denx.de> wrote:
>
> On 3/25/21 6:38 PM, Tim Harvey wrote:
> > On Thu, Mar 25, 2021 at 10:06 AM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 3/25/21 5:56 PM, Tim Harvey wrote:
> >>> When a USB Ethernet device is detected and registered there is no
> >>> way to know the device-name in order to use it with the ethact
> >>> env variable.
> >>>
> >>> Add a print to display the device name that is registered.
> >>>
> >>> Example using an AX88x72A USB Ethernet device:
> >>>
> >>> GW6404-B> usb start
> >>> starting USB...
> >>> Bus xhci_pci: Register 2000140 NbrPorts 2
> >>> Starting the controller
> >>> USB XHCI 1.00
> >>> Bus xhci_pci: Register 2000140 NbrPorts 2
> >>> Starting the controller
> >>> USB XHCI 1.00
> >>> scanning bus xhci_pci for devices...
> >>>          found ethernet device: asix_eth
> >>>
> >>> GW6404-B> setenv ethact asix_eth # select usb ethernet
> >>>
> >>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> >>> ---
> >>>    drivers/usb/eth/usb_ether.c | 1 +
> >>>    1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/drivers/usb/eth/usb_ether.c b/drivers/usb/eth/usb_ether.c
> >>> index e368ecda0d..916c4e1c85 100644
> >>> --- a/drivers/usb/eth/usb_ether.c
> >>> +++ b/drivers/usb/eth/usb_ether.c
> >>> @@ -86,6 +86,7 @@ int usb_ether_register(struct udevice *dev, struct ueth_data *ueth, int rxsize)
> >>>                return ret;
> >>>        }
> >>>        ueth->pusb_dev = udev;
> >>> +     printf("\n       found ethernet device: %s\n", dev->name);
> >>
> >> What if I have 2 asix_eth plugged in ? Do I get two identical prints ?
> >
> > Marek,
> >
> > Yes, two identical prints in this case which is not good.
> >
> > Which makes me wonder why 'setenv asix_eth' even works here as
> > drivers/usb/eth/asix.c's get_info function creates a unique instance
> > name.
>
> Seems a bit further investigation into this might be necessary.

It looks to me like the eth dev names are uniquely indexed for the non
DM_ETH case where each driver has a 'get_info' function that returns
an indexed name but for the DM_ETH case the eth_dev name is simply the
udevice name which is not unique.

>
> >> btw. it might be useful to have some "net list" command, like "mmc list"
> >> which would list all ethernet devices in the system.
> >
> > That's what I was wondering as well. There are the following
> > subcommands in the usb cmd:
> > usb storage - show details of USB storage devices
> > usb dev [dev] - show or set current USB storage device
> > usb part [dev] - print partition table of one or all USB storage    devices
> >
> > So I was wondering if we should add a:
> > usb net - show details of USB network devices
>
> I don't think such a command should be limited to USB network devices,
> it should be something applicable to all network devices.
>

Yes, I agree.

I'll look into adding this in the near future.

Thanks,

Tim
Stefan Roese March 26, 2021, 6:25 a.m. UTC | #5
On 25.03.21 17:56, Tim Harvey wrote:
> When a USB Ethernet device is detected and registered there is no
> way to know the device-name in order to use it with the ethact
> env variable.
> 
> Add a print to display the device name that is registered.

Nice. I've also stumbled upon this problem a few times but always
failed to implement such a small "fix".

> Example using an AX88x72A USB Ethernet device:
> 
> GW6404-B> usb start
> starting USB...
> Bus xhci_pci: Register 2000140 NbrPorts 2
> Starting the controller
> USB XHCI 1.00
> Bus xhci_pci: Register 2000140 NbrPorts 2
> Starting the controller
> USB XHCI 1.00
> scanning bus xhci_pci for devices...
>         found ethernet device: asix_eth
> 
> GW6404-B> setenv ethact asix_eth # select usb ethernet
> 
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/usb/eth/usb_ether.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/eth/usb_ether.c b/drivers/usb/eth/usb_ether.c
> index e368ecda0d..916c4e1c85 100644
> --- a/drivers/usb/eth/usb_ether.c
> +++ b/drivers/usb/eth/usb_ether.c
> @@ -86,6 +86,7 @@ int usb_ether_register(struct udevice *dev, struct ueth_data *ueth, int rxsize)
>   		return ret;
>   	}
>   	ueth->pusb_dev = udev;
> +	printf("\n       found ethernet device: %s\n", dev->name);
>   
>   	return 0;
>   }
> 


Viele Grüße,
Stefan
diff mbox series

Patch

diff --git a/drivers/usb/eth/usb_ether.c b/drivers/usb/eth/usb_ether.c
index e368ecda0d..916c4e1c85 100644
--- a/drivers/usb/eth/usb_ether.c
+++ b/drivers/usb/eth/usb_ether.c
@@ -86,6 +86,7 @@  int usb_ether_register(struct udevice *dev, struct ueth_data *ueth, int rxsize)
 		return ret;
 	}
 	ueth->pusb_dev = udev;
+	printf("\n       found ethernet device: %s\n", dev->name);
 
 	return 0;
 }