On Tue, Mar 14, 2023, at 13:12, 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. Similarly
> drivers requiring these functions need to depend on HAS_IOPORT.
>
> Co-developed-by: Arnd Bergmann <arnd@xxxxxxxxxx>
> Signed-off-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
I would suggest splitting this patch up into separate bits for the AMD
quirks and the UHCI driver, possibly more if there are other parts
unrelated to that.
> +#ifdef CONFIG_HAS_IOPORT
> /*
> * Make sure the controller is completely inactive, unable to
> * generate interrupts or do DMA.
Maybe check for both HAS_IOPORT and USB_UHCI_HCD ?
> static inline int io_type_enabled(struct pci_dev *pdev, unsigned int
> mask)
> {
> @@ -725,6 +730,7 @@ static inline int io_type_enabled(struct pci_dev
> *pdev, unsigned int mask)
>
> static void quirk_usb_handoff_uhci(struct pci_dev *pdev)
> {
> +#ifdef HAS_IOPORT
> unsigned long base = 0;
> int i;
>
> @@ -739,6 +745,7 @@ static void quirk_usb_handoff_uhci(struct pci_dev *pdev)
>
> if (base)
> uhci_check_and_reset_hc(pdev, base);
> +#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));
> }
>
This looks a bit ugly, though I can't think of a version I
really like here though. Possibly merging this together with the
generic version would result in something better, like
#if defined(CONFIG_USB_UHCI_PCI) && defined(CONFIG_USB_UHCI_SUPPORT_NON_PCI_HC)
/* Support PCI and non-PCI host controllers */
#define uhci_has_pci_registers(u) ((u)->io_addr != 0)
#elif defined(CONFIG_USB_UHCI_PCI)
#define uhci_has_pci_registers(u) 1
#else
/* Support non-PCI host controllers only */
#define uhci_has_pci_registers(u) 0
#endif
static inline u32 uhci_readl(const struct uhci_hcd *uhci, int reg)
{
#ifdef CONfIG_USB_UHCI_PCI
if (uhci_has_pci_registers(uhci))
return inl(uhci->io_addr + reg);
else
#endif
if (uhci_is_aspeed(uhci))
return readl(uhci->regs + uhci_aspeed_reg(reg));
else
#ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
if (uhci_big_endian_mmio(uhci))
return readl_be(uhci->regs + reg);
else
#endif
return readl(uhci->regs + reg);
}
Obviously still ugly, not sure if anyone can come up with a better
version.
Arnd
