diff mbox series

[v4,35/41] usb: uhci: handle HAS_IOPORT dependencies

Message ID 20230516110038.2413224-36-schnelle@linux.ibm.com
State Superseded
Headers show
Series None | expand

Commit Message

Niklas Schnelle May 16, 2023, 11 a.m. UTC 
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(-)

Comments

Niklas Schnelle May 19, 2023, 11:31 a.m. UTC | #1 
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
diff mbox series

Patch

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));
 }
 
 static inline u16 uhci_readw(const struct uhci_hcd *uhci, int reg)
 {
-	return inw(uhci->io_addr + reg);
+	return UHCI_IN(inw(uhci->io_addr + reg));
 }
 
 static inline void uhci_writew(const struct uhci_hcd *uhci, u16 val, int reg)
 {
-	outw(val, uhci->io_addr + reg);
+	UHCI_OUT(outw(val, uhci->io_addr + reg));
 }
 
 static inline u8 uhci_readb(const struct uhci_hcd *uhci, int reg)
 {
-	return inb(uhci->io_addr + reg);
+	return UHCI_IN(inb(uhci->io_addr + reg));
 }
 
 static inline void uhci_writeb(const struct uhci_hcd *uhci, u8 val, int reg)
 {
-	outb(val, uhci->io_addr + reg);
+	UHCI_OUT(outb(val, uhci->io_addr + reg));
 }
 
 #else
 /* Support non-PCI host controllers */
-#ifdef CONFIG_USB_PCI
+#if defined(CONFIG_USB_PCI) && defined(HAS_IOPORT)
 /* Support PCI and non-PCI host controllers */
 #define uhci_has_pci_registers(u)	((u)->io_addr != 0)
 #else
@@ -587,7 +595,7 @@  static inline int uhci_aspeed_reg(unsigned int reg)
 static inline u32 uhci_readl(const struct uhci_hcd *uhci, int reg)
 {
 	if (uhci_has_pci_registers(uhci))
-		return inl(uhci->io_addr + reg);
+		return UHCI_IN(inl(uhci->io_addr + reg));
 	else if (uhci_is_aspeed(uhci))
 		return readl(uhci->regs + uhci_aspeed_reg(reg));
 #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
@@ -601,7 +609,7 @@  static inline u32 uhci_readl(const struct uhci_hcd *uhci, int reg)
 static inline void uhci_writel(const struct uhci_hcd *uhci, u32 val, int reg)
 {
 	if (uhci_has_pci_registers(uhci))
-		outl(val, uhci->io_addr + reg);
+		UHCI_OUT(outl(val, uhci->io_addr + reg));
 	else if (uhci_is_aspeed(uhci))
 		writel(val, uhci->regs + uhci_aspeed_reg(reg));
 #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
@@ -615,7 +623,7 @@  static inline void uhci_writel(const struct uhci_hcd *uhci, u32 val, int reg)
 static inline u16 uhci_readw(const struct uhci_hcd *uhci, int reg)
 {
 	if (uhci_has_pci_registers(uhci))
-		return inw(uhci->io_addr + reg);
+		return UHCI_IN(inw(uhci->io_addr + reg));
 	else if (uhci_is_aspeed(uhci))
 		return readl(uhci->regs + uhci_aspeed_reg(reg));
 #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
@@ -629,7 +637,7 @@  static inline u16 uhci_readw(const struct uhci_hcd *uhci, int reg)
 static inline void uhci_writew(const struct uhci_hcd *uhci, u16 val, int reg)
 {
 	if (uhci_has_pci_registers(uhci))
-		outw(val, uhci->io_addr + reg);
+		UHCI_OUT(outw(val, uhci->io_addr + reg));
 	else if (uhci_is_aspeed(uhci))
 		writel(val, uhci->regs + uhci_aspeed_reg(reg));
 #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
@@ -643,7 +651,7 @@  static inline void uhci_writew(const struct uhci_hcd *uhci, u16 val, int reg)
 static inline u8 uhci_readb(const struct uhci_hcd *uhci, int reg)
 {
 	if (uhci_has_pci_registers(uhci))
-		return inb(uhci->io_addr + reg);
+		return UHCI_IN(inb(uhci->io_addr + reg));
 	else if (uhci_is_aspeed(uhci))
 		return readl(uhci->regs + uhci_aspeed_reg(reg));
 #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
@@ -657,7 +665,7 @@  static inline u8 uhci_readb(const struct uhci_hcd *uhci, int reg)
 static inline void uhci_writeb(const struct uhci_hcd *uhci, u8 val, int reg)
 {
 	if (uhci_has_pci_registers(uhci))
-		outb(val, uhci->io_addr + reg);
+		UHCI_OUT(outb(val, uhci->io_addr + reg));
 	else if (uhci_is_aspeed(uhci))
 		writel(val, uhci->regs + uhci_aspeed_reg(reg));
 #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
@@ -668,6 +676,8 @@  static inline void uhci_writeb(const struct uhci_hcd *uhci, u8 val, int reg)
 		writeb(val, uhci->regs + reg);
 }
 #endif /* CONFIG_USB_UHCI_SUPPORT_NON_PCI_HC */
+#undef UHCI_IN
+#undef UHCI_OUT
 
 /*
  * The GRLIB GRUSBHC controller can use big endian format for its descriptors.