Message ID | 20210702054754.2056918-1-gthelen@google.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | usb: xhci: avoid renesas_usb_fw.mem when it's unusable | expand |
On Thu, Jul 01, 2021 at 10:47:54PM -0700, Greg Thelen wrote: > Commit a66d21d7dba8 ("usb: xhci: Add support for Renesas controller with > memory") added renesas_usb_fw.mem firmware reference to xhci-pci. Thus > modinfo indicates xhci-pci.ko has "firmware: renesas_usb_fw.mem". But > the firmware is only actually used with CONFIG_USB_XHCI_PCI_RENESAS. An > unusable firmware reference can trigger safety checkers which look for > drivers with unmet firmware dependencies. > > Avoid referring to renesas_usb_fw.mem in circumstances when it cannot be > loaded (when CONFIG_USB_XHCI_PCI_RENESAS isn't set). > > Signed-off-by: Greg Thelen <gthelen@google.com> > --- > drivers/usb/host/xhci-pci.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c > index 18c2bbddf080..cb148da7a789 100644 > --- a/drivers/usb/host/xhci-pci.c > +++ b/drivers/usb/host/xhci-pci.c > @@ -618,8 +618,10 @@ static void xhci_pci_shutdown(struct usb_hcd *hcd) > /*-------------------------------------------------------------------------*/ > > static const struct xhci_driver_data reneses_data = { > +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) If this is not enabled, why is the whole structure needed? > .quirks = XHCI_RENESAS_FW_QUIRK, > .firmware = "renesas_usb_fw.mem", > +#endif > }; > > /* PCI driver selection metadata; PCI hotplugging uses this */ > @@ -636,7 +638,13 @@ static const struct pci_device_id pci_ids[] = { > { /* end: all zeroes */ } > }; > MODULE_DEVICE_TABLE(pci, pci_ids); > +/* > + * Without CONFIG_USB_XHCI_PCI_RENESAS renesas_xhci_check_request_fw() won't > + * load firmware, so don't encumber the xhci-pci driver with it. > + */ > +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) > MODULE_FIRMWARE("renesas_usb_fw.mem"); > +#endif Why not just have this one chunk, why is the first chunk needed? And a blank line please before your comment? And can you add a Fixes: line to the changelog text when you resend? thanks, greg k-h
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Thu, Jul 01, 2021 at 10:47:54PM -0700, Greg Thelen wrote: >> Commit a66d21d7dba8 ("usb: xhci: Add support for Renesas controller with >> memory") added renesas_usb_fw.mem firmware reference to xhci-pci. Thus >> modinfo indicates xhci-pci.ko has "firmware: renesas_usb_fw.mem". But >> the firmware is only actually used with CONFIG_USB_XHCI_PCI_RENESAS. An >> unusable firmware reference can trigger safety checkers which look for >> drivers with unmet firmware dependencies. >> >> Avoid referring to renesas_usb_fw.mem in circumstances when it cannot be >> loaded (when CONFIG_USB_XHCI_PCI_RENESAS isn't set). >> >> Signed-off-by: Greg Thelen <gthelen@google.com> >> --- >> drivers/usb/host/xhci-pci.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c >> index 18c2bbddf080..cb148da7a789 100644 >> --- a/drivers/usb/host/xhci-pci.c >> +++ b/drivers/usb/host/xhci-pci.c >> @@ -618,8 +618,10 @@ static void xhci_pci_shutdown(struct usb_hcd *hcd) >> /*-------------------------------------------------------------------------*/ >> >> static const struct xhci_driver_data reneses_data = { >> +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) > > If this is not enabled, why is the whole structure needed? Removing the struct meant 2 more #if guards needed below to initialize pci_ids, which exceeded my taste. >> .quirks = XHCI_RENESAS_FW_QUIRK, >> .firmware = "renesas_usb_fw.mem", >> +#endif >> }; >> >> /* PCI driver selection metadata; PCI hotplugging uses this */ >> @@ -636,7 +638,13 @@ static const struct pci_device_id pci_ids[] = { >> { /* end: all zeroes */ } >> }; >> MODULE_DEVICE_TABLE(pci, pci_ids); >> +/* >> + * Without CONFIG_USB_XHCI_PCI_RENESAS renesas_xhci_check_request_fw() won't >> + * load firmware, so don't encumber the xhci-pci driver with it. >> + */ >> +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) >> MODULE_FIRMWARE("renesas_usb_fw.mem"); >> +#endif > > Why not just have this one chunk, why is the first chunk needed? It seemed cleaner to remove both renesas_usb_fw.mem references rather than leave one unusable one in reneses_data above. But a simpler patch is appealing, especially given the number of #if guards needed to remove reneses_data. Patch V2 will just go with this 1-hunk approach. > And a blank line please before your comment? Will do. > And can you add a Fixes: line to the changelog text when you resend? Will do. > thanks, > > greg k-h
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 18c2bbddf080..cb148da7a789 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -618,8 +618,10 @@ static void xhci_pci_shutdown(struct usb_hcd *hcd) /*-------------------------------------------------------------------------*/ static const struct xhci_driver_data reneses_data = { +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) .quirks = XHCI_RENESAS_FW_QUIRK, .firmware = "renesas_usb_fw.mem", +#endif }; /* PCI driver selection metadata; PCI hotplugging uses this */ @@ -636,7 +638,13 @@ static const struct pci_device_id pci_ids[] = { { /* end: all zeroes */ } }; MODULE_DEVICE_TABLE(pci, pci_ids); +/* + * Without CONFIG_USB_XHCI_PCI_RENESAS renesas_xhci_check_request_fw() won't + * load firmware, so don't encumber the xhci-pci driver with it. + */ +#if IS_ENABLED(CONFIG_USB_XHCI_PCI_RENESAS) MODULE_FIRMWARE("renesas_usb_fw.mem"); +#endif /* pci driver glue; this is a "new style" PCI driver module */ static struct pci_driver xhci_pci_driver = {
Commit a66d21d7dba8 ("usb: xhci: Add support for Renesas controller with memory") added renesas_usb_fw.mem firmware reference to xhci-pci. Thus modinfo indicates xhci-pci.ko has "firmware: renesas_usb_fw.mem". But the firmware is only actually used with CONFIG_USB_XHCI_PCI_RENESAS. An unusable firmware reference can trigger safety checkers which look for drivers with unmet firmware dependencies. Avoid referring to renesas_usb_fw.mem in circumstances when it cannot be loaded (when CONFIG_USB_XHCI_PCI_RENESAS isn't set). Signed-off-by: Greg Thelen <gthelen@google.com> --- drivers/usb/host/xhci-pci.c | 8 ++++++++ 1 file changed, 8 insertions(+)