On Mon, 27 Jan 2025 18:29:52 +0100, Greg Kroah-Hartman wrote: > On Mon, Jan 27, 2025 at 03:10:51PM +0100, Michal Pecio wrote: > > Some Renesas HCs require firmware upload to work, this is handled > > by the xhci_pci_renesas driver. Other variants of those chips load > > firmware from onboard flash and can work with xhci_pci alone. > > > > A refactor merged in v6.12 broke the latter configuration and users > > are finding their hardware not working after upgrading and need to > > enable the firmware loader which isn't really necessary on their > > systems. > > Why isn't it necessary? Those chips want firmware to be uploaded, and > if the driver doesn't need it, it should be able to handle it if it is > not present. Because their chips have already loaded themselves from attached SPI flash, so they are ready to use with xhci_pci like any other HC. The FW isn't optional, we just usually don't need to load it. And FW for those chips is obscure and not redistributable AFAIK, so people have little reason to enable xhci_pci_renesas if their board has ROM and works without it. It is 'N' by default. > No, please don't put #ifdef in .c files if at all preventable. Hmm, I suppose I could replace the #ifdef with 'if (IS_ENABLED(...))' and leave the rest to dead code elimination if that's preferred. Annoyingly, the code to be knocked out is in xhci-pci and it has to stay there to avoid runtime dependency on xhci-pci-renesas, which was the whole point of the refactor. > If the firmware isn't there, then the driver should be fixed to keep > on working if it determines loading it is not needed. This type of > check should not be needed in the xhci-pci.c file. This is a check for kernel configuration, not for firmware. If the loader is disabled, we don't want to reject those chips expecting them to be taken care of by the loader, because that is not going to happen. Regards, Michal
