On Wed, 2023-05-17 at 14:17 +0200, Arnd Bergmann wrote: > On Tue, May 16, 2023, at 22:17, Alan Stern wrote: > > On Tue, May 16, 2023 at 06:29:56PM +0200, Greg Kroah-Hartman wrote: > > > On Tue, May 16, 2023 at 01:00:31PM +0200, Niklas Schnelle wrote: > > > > > I'm confused now. > > > > > > So if CONFIG_HAS_IOPORT is enabled, wonderful, all is good. > > > > > > But if it isn't, then these are just no-ops that do nothing? So then > > > the driver will fail to work? Why have these stubs at all? > > > > > > Why not just not build the driver at all if this option is not enabled? > > > > I should add something to my previous email. This particular section of > > code is protected by: > > > > #ifndef CONFIG_USB_UHCI_SUPPORT_NON_PCI_HC > > /* Support PCI only */ > > > > So it gets used only in cases where the driver supports just a PCI bus > > -- no other sorts of non-PCI on-chip devices. But the preceding patch > > in this series changes the Kconfig file to say: > > > > config USB_UHCI_HCD > > tristate "UHCI HCD (most Intel and VIA) support" > > depends on (USB_PCI && HAS_IOPORT) || USB_UHCI_SUPPORT_NON_PCI_HC > > > > As a result, when the configuration includes support only for PCI > > controllers the driver won't get built unless HAS_IOPORT is set. Thus > > the no-op case (in this part of the code) can't arise. > > Indeed, that makes sense. > > > Which is a long-winded way of saying that you're right; the UHCI_IN() > > and UHCI_OUT() wrappers aren't needed in this part of the driver. I > > guess Niklas put them in either for consistency with the rest of the > > code or because it didn't occur to him that they could be omitted. (And > > I didn't spot it either.) > > It's probably less confusing to leave out the PCI-only part of > the patch then and only modify the generic portion. > > Arnd Yes I agree that way the UHCI_IN/OUT() macro is also only used directly in combination with uhci_has_pci_registers(). I've done this for v5. Thanks, Niklas
