| Message ID | 20250203120026.2010567-1-nb@tipi-net.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | usb: xhci: Restore Renesas uPD72020x support in xhci-pci | expand |
On Mon, Feb 03, 2025 at 01:00:26PM +0100, nb@tipi-net.de wrote: > From: Nicolai Buchwitz <nb@tipi-net.de> > > Before commit 25f51b76f90f1 ("xhci-pci: Make xhci-pci-renesas a proper > modular driver"), the xhci-pci driver handled the Renesas uPD72020x USB3 > PHY and only utilized features of xhci-pci-renesas when no external > firmware EEPROM was attached. This allowed devices with a valid firmware > stored in EEPROM to function without requiring xhci-pci-renesas. > > That commit changed the behavior, making xhci-pci-renesas responsible for > handling these devices entirely, even when firmware was already present > in EEPROM. As a result, unnecessary warnings about missing firmware files > appeared, and more critically, USB functionality broke whens > CONFIG_USB_XHCI_PCI_RENESAS was not enabled—despite previously workings > without it. > > Fix this by ensuring that devices are only handed over to xhci-pci-renesas > if the config option is enabled. Otherwise, restore the original behavior > and handle them as standard xhci-pci devices. > > Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de> > Fixes: 25f51b76f90f ("xhci-pci: Make xhci-pci-renesas a proper modular driver") > --- > drivers/usb/host/xhci-pci.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > index 2d1e205c14c60..4ce80d8ac603e 100644 > --- a/drivers/usb/host/xhci-pci.c > +++ b/drivers/usb/host/xhci-pci.c > @@ -654,9 +654,11 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id) > EXPORT_SYMBOL_NS_GPL(xhci_pci_common_probe, "xhci"); > > static const struct pci_device_id pci_ids_reject[] = { > +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) > /* handled by xhci-pci-renesas */ > { PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0014) }, > { PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0015) }, > +#endif > { /* end: all zeroes */ } > }; Have you seen: https://lore.kernel.org/r/20250128104529.58a79bfc@foxbook ? Which one is correct? thanks, greg k-h
On 3.2.2025 13:46, Greg Kroah-Hartman wrote: > On Mon, Feb 03, 2025 at 01:00:26PM +0100, nb@tipi-net.de wrote: >> From: Nicolai Buchwitz <nb@tipi-net.de> >> >> Before commit 25f51b76f90f1 ("xhci-pci: Make xhci-pci-renesas a proper >> modular driver"), the xhci-pci driver handled the Renesas uPD72020x >> USB3 >> PHY and only utilized features of xhci-pci-renesas when no external >> firmware EEPROM was attached. This allowed devices with a valid >> firmware >> stored in EEPROM to function without requiring xhci-pci-renesas. >> >> That commit changed the behavior, making xhci-pci-renesas responsible >> for >> handling these devices entirely, even when firmware was already >> present >> in EEPROM. As a result, unnecessary warnings about missing firmware >> files >> appeared, and more critically, USB functionality broke whens >> CONFIG_USB_XHCI_PCI_RENESAS was not enabled—despite previously >> workings >> without it. >> >> Fix this by ensuring that devices are only handed over to >> xhci-pci-renesas >> if the config option is enabled. Otherwise, restore the original >> behavior >> and handle them as standard xhci-pci devices. >> >> Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de> >> Fixes: 25f51b76f90f ("xhci-pci: Make xhci-pci-renesas a proper modular >> driver") >> --- >> drivers/usb/host/xhci-pci.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c >> index 2d1e205c14c60..4ce80d8ac603e 100644 >> --- a/drivers/usb/host/xhci-pci.c >> +++ b/drivers/usb/host/xhci-pci.c >> @@ -654,9 +654,11 @@ int xhci_pci_common_probe(struct pci_dev *dev, >> const struct pci_device_id *id) >> EXPORT_SYMBOL_NS_GPL(xhci_pci_common_probe, "xhci"); >> >> static const struct pci_device_id pci_ids_reject[] = { >> +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) >> /* handled by xhci-pci-renesas */ >> { PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0014) }, >> { PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0015) }, >> +#endif >> { /* end: all zeroes */ } >> }; > > Have you seen: > https://lore.kernel.org/r/20250128104529.58a79bfc@foxbook > ? Hi Greg. Thanks, I must have overlooked Michal's patch when I initially stumbled over the issue. > > Which one is correct? I guess both, as Michal is implementing the same slightly different. My approach was to to keep the changes less invasive as possible and thus make it possible to use pci_ids_reject[] for further exceptions in the xhci-pci driver. In Michael's patch the list is specifically used for blacklisting the Renesas devices and cannot easily be expanded for other controllers. Either approach is fine with me, so lets move the discussion to the patch which came first. > > thanks, > > greg k-h best, Nicolai
On Mon, Feb 03, 2025 at 02:00:55PM +0100, nb@tipi-net.de wrote: > On 3.2.2025 13:46, Greg Kroah-Hartman wrote: > > On Mon, Feb 03, 2025 at 01:00:26PM +0100, nb@tipi-net.de wrote: > > > From: Nicolai Buchwitz <nb@tipi-net.de> > > > > > > Before commit 25f51b76f90f1 ("xhci-pci: Make xhci-pci-renesas a proper > > > modular driver"), the xhci-pci driver handled the Renesas uPD72020x > > > USB3 > > > PHY and only utilized features of xhci-pci-renesas when no external > > > firmware EEPROM was attached. This allowed devices with a valid > > > firmware > > > stored in EEPROM to function without requiring xhci-pci-renesas. > > > > > > That commit changed the behavior, making xhci-pci-renesas > > > responsible for > > > handling these devices entirely, even when firmware was already > > > present > > > in EEPROM. As a result, unnecessary warnings about missing firmware > > > files > > > appeared, and more critically, USB functionality broke whens > > > CONFIG_USB_XHCI_PCI_RENESAS was not enabled—despite previously > > > workings > > > without it. > > > > > > Fix this by ensuring that devices are only handed over to > > > xhci-pci-renesas > > > if the config option is enabled. Otherwise, restore the original > > > behavior > > > and handle them as standard xhci-pci devices. > > > > > > Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de> > > > Fixes: 25f51b76f90f ("xhci-pci: Make xhci-pci-renesas a proper > > > modular driver") > > > --- > > > drivers/usb/host/xhci-pci.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > > > index 2d1e205c14c60..4ce80d8ac603e 100644 > > > --- a/drivers/usb/host/xhci-pci.c > > > +++ b/drivers/usb/host/xhci-pci.c > > > @@ -654,9 +654,11 @@ int xhci_pci_common_probe(struct pci_dev *dev, > > > const struct pci_device_id *id) > > > EXPORT_SYMBOL_NS_GPL(xhci_pci_common_probe, "xhci"); > > > > > > static const struct pci_device_id pci_ids_reject[] = { > > > +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) > > > /* handled by xhci-pci-renesas */ > > > { PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0014) }, > > > { PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0015) }, > > > +#endif > > > { /* end: all zeroes */ } > > > }; > > > > Have you seen: > > https://lore.kernel.org/r/20250128104529.58a79bfc@foxbook > > ? > Hi Greg. > > Thanks, I must have overlooked Michal's patch when I initially stumbled over > the issue. > > > > Which one is correct? > > I guess both, as Michal is implementing the same slightly different. > > My approach was to to keep the changes less invasive as possible and thus > make it possible to use pci_ids_reject[] for further exceptions in the > xhci-pci driver. In Michael's patch the list is specifically used for > blacklisting the Renesas devices and cannot easily be expanded for other > controllers. Either approach is fine with me, so lets move the discussion to > the patch which came first. Ok, can you test Michael's patch and respond with a tested-by if it works for you? thanks, greg k-h
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 2d1e205c14c60..4ce80d8ac603e 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -654,9 +654,11 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id) EXPORT_SYMBOL_NS_GPL(xhci_pci_common_probe, "xhci"); static const struct pci_device_id pci_ids_reject[] = { +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) /* handled by xhci-pci-renesas */ { PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0014) }, { PCI_DEVICE(PCI_VENDOR_ID_RENESAS, 0x0015) }, +#endif { /* end: all zeroes */ } };