Re: [PATCH v4 35/41] usb: uhci: handle HAS_IOPORT dependencies - Greg Kroah-Hartman

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
	"Alan Stern" <stern@rowland.harvard.edu>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Geert Uytterhoeven" <geert@linux-m68k.org>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-pci@vger.kernel.org, "Arnd Bergmann" <arnd@kernel.org>,
	linux-usb@vger.kernel.org
Subject: Re: [PATCH v4 35/41] usb: uhci: handle HAS_IOPORT dependencies
Date: Tue, 16 May 2023 18:29:56 +0200	[thread overview]
Message-ID: <2023051643-overtime-unbridle-7cdd@gregkh> (raw)
In-Reply-To: <20230516110038.2413224-36-schnelle@linux.ibm.com>

On Tue, May 16, 2023 at 01:00:31PM +0200, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> not being declared. We thus need to guard sections of code calling them
> as alternative access methods with CONFIG_HAS_IOPORT checks. For
> uhci-hcd there are a lot of I/O port uses that do have MMIO alternatives
> all selected by uhci_has_pci_registers() so this can be handled by
> UHCI_IN/OUT macros and making uhci_has_pci_registers() constant 0 if
> CONFIG_HAS_IOPORT is unset.
> 
> Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
> Note: The HAS_IOPORT Kconfig option was added in v6.4-rc1 so
>       per-subsystem patches may be applied independently
> 
>  drivers/usb/host/uhci-hcd.c |  2 +-
>  drivers/usb/host/uhci-hcd.h | 36 +++++++++++++++++++++++-------------
>  2 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
> index 7cdc2fa7c28f..fd2408b553cf 100644
> --- a/drivers/usb/host/uhci-hcd.c
> +++ b/drivers/usb/host/uhci-hcd.c
> @@ -841,7 +841,7 @@ static int uhci_count_ports(struct usb_hcd *hcd)
>  
>  static const char hcd_name[] = "uhci_hcd";
>  
> -#ifdef CONFIG_USB_PCI
> +#if defined(CONFIG_USB_PCI) && defined(CONFIG_HAS_IOPORT)
>  #include "uhci-pci.c"
>  #define	PCI_DRIVER		uhci_pci_driver
>  #endif
> diff --git a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
> index 0688c3e5bfe2..c77705d03ed0 100644
> --- a/drivers/usb/host/uhci-hcd.h
> +++ b/drivers/usb/host/uhci-hcd.h
> @@ -505,41 +505,49 @@ static inline bool uhci_is_aspeed(const struct uhci_hcd *uhci)
>   * we use memory mapped registers.
>   */
>  
> +#ifdef CONFIG_HAS_IOPORT
> +#define UHCI_IN(x)	x
> +#define UHCI_OUT(x)	x
> +#else
> +#define UHCI_IN(x)	0
> +#define UHCI_OUT(x)
> +#endif
> +
>  #ifndef CONFIG_USB_UHCI_SUPPORT_NON_PCI_HC
>  /* Support PCI only */
>  static inline u32 uhci_readl(const struct uhci_hcd *uhci, int reg)
>  {
> -	return inl(uhci->io_addr + reg);
> +	return UHCI_IN(inl(uhci->io_addr + reg));
>  }
>  
>  static inline void uhci_writel(const struct uhci_hcd *uhci, u32 val, int reg)
>  {
> -	outl(val, uhci->io_addr + reg);
> +	UHCI_OUT(outl(val, uhci->io_addr + reg));

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?

thanks,

greg k-h