| Message ID | 20230516110038.2413224-36-schnelle@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | treewide: Remove I/O port accessors for HAS_IOPORT=n | expand |
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
On Tue, May 16, 2023, at 18:29, Greg Kroah-Hartman wrote: > On Tue, May 16, 2023 at 01:00:31PM +0200, Niklas Schnelle wrote: >> #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? If I remember correctly, the problem here is the lack of abstractions in the uhci driver, it instead supports all combinations of on-chip non-PCI devices using readb()/writeb() and PCI devices using inb()/outb() in a shared codebase. A particularly tricky combination is a kernel that supports on-chip UHCI as well as CONFIG_USB_PCI (for EHCI/XHCI) but does not support I/O ports because of platform limitations. The trick is to come up with a set of changes that doesn't have to rewrite the entire logic but also doesn't add an obscene number of #ifdef checks. That said, there is a minor problem with the empty definition +#define UHCI_OUT(x) I think this should be "do { } while (0)" to avoid warnings about empty if/else blocks. Arnd
On Tue, May 16, 2023 at 06:44:34PM +0200, Arnd Bergmann wrote: > On Tue, May 16, 2023, at 18:29, Greg Kroah-Hartman wrote: > > On Tue, May 16, 2023 at 01:00:31PM +0200, Niklas Schnelle wrote: > > >> #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? > > If I remember correctly, the problem here is the lack of > abstractions in the uhci driver, it instead supports all > combinations of on-chip non-PCI devices using readb()/writeb() > and PCI devices using inb()/outb() in a shared codebase. Isn't that an abstraction? A single set of operations (uhci_readl(), uhci_writel(), etc.) that always does the right sort of I/O even when talking to different buses? So I'm not sure what you mean by "the lack of abstractions". > A particularly tricky combination is a kernel that supports on-chip > UHCI as well as CONFIG_USB_PCI (for EHCI/XHCI) but does not support > I/O ports because of platform limitations. The trick is to come up > with a set of changes that doesn't have to rewrite the entire logic > but also doesn't add an obscene number of #ifdef checks. Indeed, in a kernel supporting that tricky combination the no-op code would be generated. But it would never execute at runtime because the uhci_has_pci_registers(uhci) test would always return 0, and so the driver wouldn't fail. > That said, there is a minor problem with the empty definition > > +#define UHCI_OUT(x) > > I think this should be "do { } while (0)" to avoid warnings > about empty if/else blocks. I'm sure Niklas wouldn't mind making such a change. But do we really get such warnings? Does the compiler really think that this kind of (macro-expanded) code: if (uhci_has_pci_registers(uhci)) ; else if (uhci_is_aspeed(uhci)) writel(val, uhci->regs + uhci_aspeed_reg(reg)); deserves a warning? I write stuff like that fairly often; it's a good way to showcase a high-probability do-nothing pathway at the start of a series of conditional cases. And I haven't noticed any complaints from the compiler. Alan Stern
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: > > 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? 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. 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.) Alan Stern
On Tue, 2023-05-16 at 15:51 -0400, Alan Stern wrote: > On Tue, May 16, 2023 at 06:44:34PM +0200, Arnd Bergmann wrote: > > On Tue, May 16, 2023, at 18:29, Greg Kroah-Hartman wrote: > > > On Tue, May 16, 2023 at 01:00:31PM +0200, Niklas Schnelle wrote: > > > > > > #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? The driver supports multiple access methods in several functions similar to the following: static inline void uhci_writel(const struct uhci_hcd *uhci, u32 val, int reg) { if (uhci_has_pci_registers(uhci)) 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 else if (uhci_big_endian_mmio(uhci)) writel_be(val, uhci->regs + reg); #endif else writel(val, uhci->regs + reg); } Instead of adding more #ifdefs Alan Stern suggested to just stub out both uhci_has_pci_registers() and the access itself. So with a half way optimizing compiler this shouldn't even leave no-ops in the binary. > > > That said, there is a minor problem with the empty definition > > > > +#define UHCI_OUT(x) > > > > I think this should be "do { } while (0)" to avoid warnings > > about empty if/else blocks. > > I'm sure Niklas wouldn't mind making such a change. But do we really > get such warnings? Does the compiler really think that this kind of > (macro-expanded) code: > > if (uhci_has_pci_registers(uhci)) > ; > else if (uhci_is_aspeed(uhci)) > writel(val, uhci->regs + uhci_aspeed_reg(reg)); > > deserves a warning? I write stuff like that fairly often; it's a good > way to showcase a high-probability do-nothing pathway at the start of a > series of conditional cases. And I haven't noticed any complaints from > the compiler. > > Alan Stern I changed it to "do {} while (0)" for v5 but agree I haven't seen warnings for this either. Still doesn't hurt. Thanks, Niklas
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
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 --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.