Re: [PATCH 1/2] net: asix: add 0b95:1790 to AX88179A device list

[Grant Grundler]

On Sat, Feb 10, 2024 at 10:20 AM Grant Grundler <grundler@xxxxxxxxxx> wrote:
>
> On Mon, Feb 5, 2024 at 2:40 AM Christian Hewitt
> <christianshewitt@xxxxxxxxx> wrote:
> >
> > Add a generic AX88179A entry for the 0b95:1790 device id:
> >
> > kernel: usb 2-1: New USB device found, idVendor=0b95, idProduct=1790, bcdDevice= 2.00
> > kernel: usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> > kernel: usb 2-1: Product: AX88179A
> > kernel: usb 2-1: Manufacturer: ASIX
> > kernel: usb 2-1: SerialNumber: 00D24DC0
> > kernel: asix 2-1:1.0 (unnamed net_device) (uninitialized): Failed to read reg index 0x0000: -32
> > kernel: asix: probe of 2-1:1.0 failed with error -32
> > kernel: ax88179_178a 2-1:1.0 (unnamed net_device) (uninitialized): Failed to read reg index 0x0040: -32
> > kernel: ax88179_178a 2-1:1.0 eth1: register 'ax88179_178a' at usb-0000:01:00.0-1, ASIX AX88179 USB 3.0 Gigabit Ethernet, 20:7b:d2:d2:4d:c0
> >
> > Signed-off-by: Christian Hewitt <christianshewitt@xxxxxxxxx>
> > ---
> > The change is tested by a LibreELEC (distro) user who reports the NIC to be working
> > fine (and logs support this) but the "Failed to read reg index 0x0000: -32" errors
> > suggest ax88178_info might not be the correct choice. I'm not a serious coder so I
> > need to "ask the audience" for suggestions on what more might be needed?
> >
> >  drivers/net/usb/asix_devices.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> > index f7cff58fe044..9a7b1136cd98 100644
> > --- a/drivers/net/usb/asix_devices.c
> > +++ b/drivers/net/usb/asix_devices.c
> > @@ -1506,6 +1506,10 @@ static const struct usb_device_id        products [] = {
> >         // ASIX AX88178 10/100/1000
> >         USB_DEVICE (0x0b95, 0x1780),
> >         .driver_info = (unsigned long) &ax88178_info,
> > +}, {
> > +       // ASIX AX88179A 10/100/1000
> > +       USB_DEVICE(0x0b95, 0x1790),
> > +       .driver_info = (unsigned long)&ax88178_info,
> >  }, {
>
> Hi Christian!
> Seems like there are two problems here:
> 1) The USB output is telling you this device is a AX88179A : the
> changes most likely should be in ax88179_178a.c using the
> ax881798_info:
>    https://elixir.bootlin.com/linux/latest/source/drivers/net/usb/ax88179_178a.c#L1690

Christian,
Let me restate this more clearly: ax88179_178a is already claiming
this device. No need to add anything to asix_devices.c

> 2) What Andrew Lunn said. I'll add the asix driver should not be
> probing (or claiming) this device - or at least be quiet about it when
> it does.

That's why Andrew was asking about where 0x40 index is used.... and
it's not obvious to me either right now since I don't see a definition
for register index 0x40 in ax88179_178a.c file.

Since this is happening with both drivers (asix and ax88179_178a) and
it looks like this is happening before SET_NETDEV_DEV() is called, I'm
going to assume this has something to do with usbnet_probe() calling
pm_runtime_enable().

Andrew's advice to add a WARN_ON call to the function printing "Failed
to read reg index" on failure, is really good as it will tell us the
exact call path through "a maze of twisty little passages, all the
same" (to quote "colossal cave adventure" game from 1977 :D).

cheers,
grant

> Thanks for looking into this!
>
> cheers,
> grant
>
> >         // Logitec LAN-GTJ/U2A
> >         USB_DEVICE (0x0789, 0x0160),
> > --
> > 2.34.1
> >