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
