* [PATCH v5 36/44] usb: add HAS_IOPORT dependencies [not found] <20230522105049.1467313-1-schnelle@linux.ibm.com> @ 2023-05-22 10:50 ` Niklas Schnelle 2023-05-22 10:50 ` [PATCH v5 37/44] usb: uhci: handle " Niklas Schnelle 2023-05-22 10:50 ` [PATCH v5 38/44] usb: pci-quirks: " Niklas Schnelle 2 siblings, 0 replies; 5+ messages in thread From: Niklas Schnelle @ 2023-05-22 10:50 UTC (permalink / raw) To: Arnd Bergmann, Greg Kroah-Hartman Cc: Bjorn Helgaas, Uwe Kleine-König, Mauro Carvalho Chehab, Alan Stern, Rafael J. Wysocki, Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-kernel, linux-arch, linux-pci, Arnd Bergmann, linux-usb In a future patch HAS_IOPORT=n will result in inb()/outb() and friends not being declared. We thus need to add HAS_IOPORT as dependency for those drivers using them. 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> --- drivers/usb/host/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index c170672f847e..4448d0ab06f0 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -376,7 +376,7 @@ config USB_ISP116X_HCD config USB_ISP1362_HCD tristate "ISP1362 HCD support" - depends on HAS_IOMEM + depends on HAS_IOPORT depends on COMPILE_TEST # nothing uses this help Supports the Philips ISP1362 chip as a host controller @@ -578,7 +578,7 @@ endif # USB_OHCI_HCD config USB_UHCI_HCD tristate "UHCI HCD (most Intel and VIA) support" - depends on USB_PCI || USB_UHCI_SUPPORT_NON_PCI_HC + depends on (USB_PCI && HAS_IOPORT) || USB_UHCI_SUPPORT_NON_PCI_HC help The Universal Host Controller Interface is a standard by Intel for accessing the USB hardware in the PC (which is also called the USB -- 2.39.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 37/44] usb: uhci: handle HAS_IOPORT dependencies [not found] <20230522105049.1467313-1-schnelle@linux.ibm.com> 2023-05-22 10:50 ` [PATCH v5 36/44] usb: add HAS_IOPORT dependencies Niklas Schnelle @ 2023-05-22 10:50 ` Niklas Schnelle 2023-05-22 13:57 ` Alan Stern 2023-05-22 10:50 ` [PATCH v5 38/44] usb: pci-quirks: " Niklas Schnelle 2 siblings, 1 reply; 5+ messages in thread From: Niklas Schnelle @ 2023-05-22 10:50 UTC (permalink / raw) To: Arnd Bergmann, Alan Stern, Greg Kroah-Hartman Cc: Bjorn Helgaas, Uwe Kleine-König, Mauro Carvalho Chehab, Rafael J. Wysocki, Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-kernel, linux-arch, linux-pci, Arnd Bergmann, linux-usb 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> --- drivers/usb/host/uhci-hcd.c | 2 +- drivers/usb/host/uhci-hcd.h | 24 +++++++++++++++++------- 2 files changed, 18 insertions(+), 8 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..13ee2a6144b2 100644 --- a/drivers/usb/host/uhci-hcd.h +++ b/drivers/usb/host/uhci-hcd.h @@ -505,6 +505,14 @@ 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) do { } while (0) +#endif + #ifndef CONFIG_USB_UHCI_SUPPORT_NON_PCI_HC /* Support PCI only */ static inline u32 uhci_readl(const struct uhci_hcd *uhci, int reg) @@ -539,7 +547,7 @@ static inline void uhci_writeb(const struct uhci_hcd *uhci, u8 val, int 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. -- 2.39.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 37/44] usb: uhci: handle HAS_IOPORT dependencies 2023-05-22 10:50 ` [PATCH v5 37/44] usb: uhci: handle " Niklas Schnelle @ 2023-05-22 13:57 ` Alan Stern 0 siblings, 0 replies; 5+ messages in thread From: Alan Stern @ 2023-05-22 13:57 UTC (permalink / raw) To: Niklas Schnelle Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Arnd Bergmann, linux-usb On Mon, May 22, 2023 at 12:50:42PM +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> > --- Reviewed-by: Alan Stern <stern@rowland.harvard.edu> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v5 38/44] usb: pci-quirks: handle HAS_IOPORT dependencies [not found] <20230522105049.1467313-1-schnelle@linux.ibm.com> 2023-05-22 10:50 ` [PATCH v5 36/44] usb: add HAS_IOPORT dependencies Niklas Schnelle 2023-05-22 10:50 ` [PATCH v5 37/44] usb: uhci: handle " Niklas Schnelle @ 2023-05-22 10:50 ` Niklas Schnelle 2023-05-29 14:34 ` Greg Kroah-Hartman 2 siblings, 1 reply; 5+ messages in thread From: Niklas Schnelle @ 2023-05-22 10:50 UTC (permalink / raw) To: Arnd Bergmann, Greg Kroah-Hartman, Mathias Nyman Cc: Bjorn Helgaas, Uwe Kleine-König, Mauro Carvalho Chehab, Alan Stern, Rafael J. Wysocki, Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-kernel, linux-arch, linux-pci, Arnd Bergmann, linux-usb In a future patch HAS_IOPORT=n will result in inb()/outb() and friends not being declared. In the pci-quirks case the I/O port acceses are used in the quirks for several AMD south bridges. Move unrelated ASMEDIA quirks out of the way and introduce an additional config option for the AMD quirks that depends on HAS_IOPORT. 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> --- drivers/usb/Kconfig | 10 +++ drivers/usb/core/hcd-pci.c | 2 + drivers/usb/host/pci-quirks.c | 125 ++++++++++++++++++---------------- drivers/usb/host/pci-quirks.h | 30 ++++++-- 4 files changed, 101 insertions(+), 66 deletions(-) diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig index 7f33bcc315f2..abf8c6cdea9e 100644 --- a/drivers/usb/Kconfig +++ b/drivers/usb/Kconfig @@ -91,6 +91,16 @@ config USB_PCI If you have such a device you may say N here and PCI related code will not be built in the USB driver. +config USB_PCI_AMD + bool "AMD PCI USB host support" + depends on USB_PCI && HAS_IOPORT + default X86 || MACH_LOONGSON64 || PPC_PASEMI + help + Enable workarounds for USB implementation quirks in SB600/SB700/SB800 + and later south bridge implementations. These are common on x86 PCs + with AMD CPUs but rarely used elsewhere, with the exception of a few + powerpc and mips desktop machines. + if USB source "drivers/usb/core/Kconfig" diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c index ab2f3737764e..85a0aeae85cd 100644 --- a/drivers/usb/core/hcd-pci.c +++ b/drivers/usb/core/hcd-pci.c @@ -206,8 +206,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct hc_driver *driver) goto free_irq_vectors; } +#ifdef CONFIG_USB_PCI_AMD hcd->amd_resume_bug = (usb_hcd_amd_remote_wakeup_quirk(dev) && driver->flags & (HCD_USB11 | HCD_USB3)) ? 1 : 0; +#endif /* CONFIG_USB_PCI_AMD */ if (driver->flags & HCD_MEMORY) { /* EHCI, OHCI */ diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c index 2665832f9add..e0612f909fad 100644 --- a/drivers/usb/host/pci-quirks.c +++ b/drivers/usb/host/pci-quirks.c @@ -60,6 +60,23 @@ #define EHCI_USBLEGCTLSTS 4 /* legacy control/status */ #define EHCI_USBLEGCTLSTS_SOOE (1 << 13) /* SMI on ownership change */ +/* ASMEDIA quirk use */ +#define ASMT_DATA_WRITE0_REG 0xF8 +#define ASMT_DATA_WRITE1_REG 0xFC +#define ASMT_CONTROL_REG 0xE0 +#define ASMT_CONTROL_WRITE_BIT 0x02 +#define ASMT_WRITEREG_CMD 0x10423 +#define ASMT_FLOWCTL_ADDR 0xFA30 +#define ASMT_FLOWCTL_DATA 0xBA +#define ASMT_PSEUDO_DATA 0 + +/* Intel quirk use */ +#define USB_INTEL_XUSB2PR 0xD0 +#define USB_INTEL_USB2PRM 0xD4 +#define USB_INTEL_USB3_PSSEN 0xD8 +#define USB_INTEL_USB3PRM 0xDC + +#ifdef CONFIG_USB_PCI_AMD /* AMD quirk use */ #define AB_REG_BAR_LOW 0xe0 #define AB_REG_BAR_HIGH 0xe1 @@ -93,21 +110,6 @@ #define NB_PIF0_PWRDOWN_0 0x01100012 #define NB_PIF0_PWRDOWN_1 0x01100013 -#define USB_INTEL_XUSB2PR 0xD0 -#define USB_INTEL_USB2PRM 0xD4 -#define USB_INTEL_USB3_PSSEN 0xD8 -#define USB_INTEL_USB3PRM 0xDC - -/* ASMEDIA quirk use */ -#define ASMT_DATA_WRITE0_REG 0xF8 -#define ASMT_DATA_WRITE1_REG 0xFC -#define ASMT_CONTROL_REG 0xE0 -#define ASMT_CONTROL_WRITE_BIT 0x02 -#define ASMT_WRITEREG_CMD 0x10423 -#define ASMT_FLOWCTL_ADDR 0xFA30 -#define ASMT_FLOWCTL_DATA 0xBA -#define ASMT_PSEUDO_DATA 0 - /* * amd_chipset_gen values represent AMD different chipset generations */ @@ -458,50 +460,6 @@ void usb_amd_quirk_pll_disable(void) } EXPORT_SYMBOL_GPL(usb_amd_quirk_pll_disable); -static int usb_asmedia_wait_write(struct pci_dev *pdev) -{ - unsigned long retry_count; - unsigned char value; - - for (retry_count = 1000; retry_count > 0; --retry_count) { - - pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value); - - if (value == 0xff) { - dev_err(&pdev->dev, "%s: check_ready ERROR", __func__); - return -EIO; - } - - if ((value & ASMT_CONTROL_WRITE_BIT) == 0) - return 0; - - udelay(50); - } - - dev_warn(&pdev->dev, "%s: check_write_ready timeout", __func__); - return -ETIMEDOUT; -} - -void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev) -{ - if (usb_asmedia_wait_write(pdev) != 0) - return; - - /* send command and address to device */ - pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_WRITEREG_CMD); - pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_FLOWCTL_ADDR); - pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT); - - if (usb_asmedia_wait_write(pdev) != 0) - return; - - /* send data to device */ - pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_FLOWCTL_DATA); - pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_PSEUDO_DATA); - pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT); -} -EXPORT_SYMBOL_GPL(usb_asmedia_modifyflowcontrol); - void usb_amd_quirk_pll_enable(void) { usb_amd_quirk_pll(0); @@ -630,7 +588,53 @@ bool usb_amd_pt_check_port(struct device *device, int port) return !(value & BIT(port_shift)); } EXPORT_SYMBOL_GPL(usb_amd_pt_check_port); +#endif /* CONFIG_USB_PCI_AMD */ +static int usb_asmedia_wait_write(struct pci_dev *pdev) +{ + unsigned long retry_count; + unsigned char value; + + for (retry_count = 1000; retry_count > 0; --retry_count) { + + pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value); + + if (value == 0xff) { + dev_err(&pdev->dev, "%s: check_ready ERROR", __func__); + return -EIO; + } + + if ((value & ASMT_CONTROL_WRITE_BIT) == 0) + return 0; + + udelay(50); + } + + dev_warn(&pdev->dev, "%s: check_write_ready timeout", __func__); + return -ETIMEDOUT; +} + +void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev) +{ + if (usb_asmedia_wait_write(pdev) != 0) + return; + + /* send command and address to device */ + pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_WRITEREG_CMD); + pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_FLOWCTL_ADDR); + pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT); + + if (usb_asmedia_wait_write(pdev) != 0) + return; + + /* send data to device */ + pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_FLOWCTL_DATA); + pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_PSEUDO_DATA); + pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT); +} +EXPORT_SYMBOL_GPL(usb_asmedia_modifyflowcontrol); + +#if defined(CONFIG_HAS_IOPORT) && defined(CONFIG_USB_UHCI_HCD) /* * Make sure the controller is completely inactive, unable to * generate interrupts or do DMA. @@ -711,6 +715,7 @@ int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base) return 1; } EXPORT_SYMBOL_GPL(uhci_check_and_reset_hc); +#endif /* defined(CONFIG_HAS_IOPORT && defined(CONFIG_USB_UHCI_HCD) */ static inline int io_type_enabled(struct pci_dev *pdev, unsigned int mask) { @@ -723,6 +728,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 CONFIG_HAS_IOPORT unsigned long base = 0; int i; @@ -737,6 +743,7 @@ static void quirk_usb_handoff_uhci(struct pci_dev *pdev) if (base) uhci_check_and_reset_hc(pdev, base); +#endif /* CONFIG_HAS_IOPORT */ } static int mmio_resource_enabled(struct pci_dev *pdev, int idx) diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h index e729de21fad7..8c87505f0abc 100644 --- a/drivers/usb/host/pci-quirks.h +++ b/drivers/usb/host/pci-quirks.h @@ -2,9 +2,10 @@ #ifndef __LINUX_USB_PCI_QUIRKS_H #define __LINUX_USB_PCI_QUIRKS_H -#ifdef CONFIG_USB_PCI void uhci_reset_hc(struct pci_dev *pdev, unsigned long base); int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base); + +#ifdef CONFIG_USB_PCI_AMD int usb_hcd_amd_remote_wakeup_quirk(struct pci_dev *pdev); bool usb_amd_hang_symptom_quirk(void); bool usb_amd_prefetch_quirk(void); @@ -12,23 +13,38 @@ void usb_amd_dev_put(void); bool usb_amd_quirk_pll_check(void); void usb_amd_quirk_pll_disable(void); void usb_amd_quirk_pll_enable(void); -void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev); -void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev); -void usb_disable_xhci_ports(struct pci_dev *xhci_pdev); void sb800_prefetch(struct device *dev, int on); bool usb_amd_pt_check_port(struct device *device, int port); #else -struct pci_dev; +static inline bool usb_amd_hang_symptom_quirk(void) +{ + return false; +}; +static inline bool usb_amd_prefetch_quirk(void) +{ + return false; +} +static inline bool usb_amd_quirk_pll_check(void) +{ + return false; +} static inline void usb_amd_quirk_pll_disable(void) {} static inline void usb_amd_quirk_pll_enable(void) {} -static inline void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev) {} static inline void usb_amd_dev_put(void) {} -static inline void usb_disable_xhci_ports(struct pci_dev *xhci_pdev) {} static inline void sb800_prefetch(struct device *dev, int on) {} static inline bool usb_amd_pt_check_port(struct device *device, int port) { return false; } +#endif /* CONFIG_USB_PCI_AMD */ + +#ifdef CONFIG_USB_PCI +void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev); +void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev); +void usb_disable_xhci_ports(struct pci_dev *xhci_pdev); +#else +static inline void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev) {} +static inline void usb_disable_xhci_ports(struct pci_dev *xhci_pdev) {} #endif /* CONFIG_USB_PCI */ #endif /* __LINUX_USB_PCI_QUIRKS_H */ -- 2.39.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 38/44] usb: pci-quirks: handle HAS_IOPORT dependencies 2023-05-22 10:50 ` [PATCH v5 38/44] usb: pci-quirks: " Niklas Schnelle @ 2023-05-29 14:34 ` Greg Kroah-Hartman 0 siblings, 0 replies; 5+ messages in thread From: Greg Kroah-Hartman @ 2023-05-29 14:34 UTC (permalink / raw) To: Niklas Schnelle Cc: Arnd Bergmann, Mathias Nyman, Bjorn Helgaas, Uwe Kleine-König, Mauro Carvalho Chehab, Alan Stern, Rafael J. Wysocki, Geert Uytterhoeven, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-kernel, linux-arch, linux-pci, Arnd Bergmann, linux-usb On Mon, May 22, 2023 at 12:50:43PM +0200, Niklas Schnelle wrote: > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends > not being declared. In the pci-quirks case the I/O port acceses are > used in the quirks for several AMD south bridges. Move unrelated > ASMEDIA quirks out of the way and introduce an additional config option > for the AMD quirks that depends on HAS_IOPORT. This is really messy and hard to review in one commit. I know I got lost a bunch, and it's still messy: > > 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> > --- > drivers/usb/Kconfig | 10 +++ > drivers/usb/core/hcd-pci.c | 2 + > drivers/usb/host/pci-quirks.c | 125 ++++++++++++++++++---------------- > drivers/usb/host/pci-quirks.h | 30 ++++++-- > 4 files changed, 101 insertions(+), 66 deletions(-) > > diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig > index 7f33bcc315f2..abf8c6cdea9e 100644 > --- a/drivers/usb/Kconfig > +++ b/drivers/usb/Kconfig > @@ -91,6 +91,16 @@ config USB_PCI > If you have such a device you may say N here and PCI related code > will not be built in the USB driver. > > +config USB_PCI_AMD > + bool "AMD PCI USB host support" > + depends on USB_PCI && HAS_IOPORT > + default X86 || MACH_LOONGSON64 || PPC_PASEMI > + help > + Enable workarounds for USB implementation quirks in SB600/SB700/SB800 > + and later south bridge implementations. These are common on x86 PCs > + with AMD CPUs but rarely used elsewhere, with the exception of a few > + powerpc and mips desktop machines. This is fine, make one commit for this, and then we can argue if you really need it. :) > + > if USB > > source "drivers/usb/core/Kconfig" > diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c > index ab2f3737764e..85a0aeae85cd 100644 > --- a/drivers/usb/core/hcd-pci.c > +++ b/drivers/usb/core/hcd-pci.c > @@ -206,8 +206,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct hc_driver *driver) > goto free_irq_vectors; > } > > +#ifdef CONFIG_USB_PCI_AMD > hcd->amd_resume_bug = (usb_hcd_amd_remote_wakeup_quirk(dev) && > driver->flags & (HCD_USB11 | HCD_USB3)) ? 1 : 0; > +#endif /* CONFIG_USB_PCI_AMD */ No #ifdef in .c files if at all possible please. Why is this needed? And I hate ? : logic, please spell it out. > > if (driver->flags & HCD_MEMORY) { > /* EHCI, OHCI */ > diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c > index 2665832f9add..e0612f909fad 100644 > --- a/drivers/usb/host/pci-quirks.c > +++ b/drivers/usb/host/pci-quirks.c > @@ -60,6 +60,23 @@ > #define EHCI_USBLEGCTLSTS 4 /* legacy control/status */ > #define EHCI_USBLEGCTLSTS_SOOE (1 << 13) /* SMI on ownership change */ > > +/* ASMEDIA quirk use */ > +#define ASMT_DATA_WRITE0_REG 0xF8 > +#define ASMT_DATA_WRITE1_REG 0xFC > +#define ASMT_CONTROL_REG 0xE0 > +#define ASMT_CONTROL_WRITE_BIT 0x02 > +#define ASMT_WRITEREG_CMD 0x10423 > +#define ASMT_FLOWCTL_ADDR 0xFA30 > +#define ASMT_FLOWCTL_DATA 0xBA > +#define ASMT_PSEUDO_DATA 0 > + > +/* Intel quirk use */ > +#define USB_INTEL_XUSB2PR 0xD0 > +#define USB_INTEL_USB2PRM 0xD4 > +#define USB_INTEL_USB3_PSSEN 0xD8 > +#define USB_INTEL_USB3PRM 0xDC > + > +#ifdef CONFIG_USB_PCI_AMD > /* AMD quirk use */ > #define AB_REG_BAR_LOW 0xe0 > #define AB_REG_BAR_HIGH 0xe1 > @@ -93,21 +110,6 @@ > #define NB_PIF0_PWRDOWN_0 0x01100012 > #define NB_PIF0_PWRDOWN_1 0x01100013 > > -#define USB_INTEL_XUSB2PR 0xD0 > -#define USB_INTEL_USB2PRM 0xD4 > -#define USB_INTEL_USB3_PSSEN 0xD8 > -#define USB_INTEL_USB3PRM 0xDC > - > -/* ASMEDIA quirk use */ > -#define ASMT_DATA_WRITE0_REG 0xF8 > -#define ASMT_DATA_WRITE1_REG 0xFC > -#define ASMT_CONTROL_REG 0xE0 > -#define ASMT_CONTROL_WRITE_BIT 0x02 > -#define ASMT_WRITEREG_CMD 0x10423 > -#define ASMT_FLOWCTL_ADDR 0xFA30 > -#define ASMT_FLOWCTL_DATA 0xBA > -#define ASMT_PSEUDO_DATA 0 > - > /* > * amd_chipset_gen values represent AMD different chipset generations > */ > @@ -458,50 +460,6 @@ void usb_amd_quirk_pll_disable(void) > } > EXPORT_SYMBOL_GPL(usb_amd_quirk_pll_disable); > > -static int usb_asmedia_wait_write(struct pci_dev *pdev) Moving these is odd, why? They are just doing pci accesses. > -{ > - unsigned long retry_count; > - unsigned char value; > - > - for (retry_count = 1000; retry_count > 0; --retry_count) { > - > - pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value); > - > - if (value == 0xff) { > - dev_err(&pdev->dev, "%s: check_ready ERROR", __func__); > - return -EIO; > - } > - > - if ((value & ASMT_CONTROL_WRITE_BIT) == 0) > - return 0; > - > - udelay(50); > - } > - > - dev_warn(&pdev->dev, "%s: check_write_ready timeout", __func__); > - return -ETIMEDOUT; > -} > - > -void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev) > -{ > - if (usb_asmedia_wait_write(pdev) != 0) > - return; > - > - /* send command and address to device */ > - pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_WRITEREG_CMD); > - pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_FLOWCTL_ADDR); > - pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT); > - > - if (usb_asmedia_wait_write(pdev) != 0) > - return; > - > - /* send data to device */ > - pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_FLOWCTL_DATA); > - pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_PSEUDO_DATA); > - pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT); > -} > -EXPORT_SYMBOL_GPL(usb_asmedia_modifyflowcontrol); > - > void usb_amd_quirk_pll_enable(void) > { > usb_amd_quirk_pll(0); > @@ -630,7 +588,53 @@ bool usb_amd_pt_check_port(struct device *device, int port) > return !(value & BIT(port_shift)); > } > EXPORT_SYMBOL_GPL(usb_amd_pt_check_port); > +#endif /* CONFIG_USB_PCI_AMD */ > > +static int usb_asmedia_wait_write(struct pci_dev *pdev) > +{ > + unsigned long retry_count; > + unsigned char value; > + > + for (retry_count = 1000; retry_count > 0; --retry_count) { > + > + pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value); > + > + if (value == 0xff) { > + dev_err(&pdev->dev, "%s: check_ready ERROR", __func__); > + return -EIO; > + } > + > + if ((value & ASMT_CONTROL_WRITE_BIT) == 0) > + return 0; > + > + udelay(50); > + } > + > + dev_warn(&pdev->dev, "%s: check_write_ready timeout", __func__); > + return -ETIMEDOUT; > +} > + > +void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev) > +{ > + if (usb_asmedia_wait_write(pdev) != 0) > + return; > + > + /* send command and address to device */ > + pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_WRITEREG_CMD); > + pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_FLOWCTL_ADDR); > + pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT); > + > + if (usb_asmedia_wait_write(pdev) != 0) > + return; > + > + /* send data to device */ > + pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_FLOWCTL_DATA); > + pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_PSEUDO_DATA); > + pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT); > +} > +EXPORT_SYMBOL_GPL(usb_asmedia_modifyflowcontrol); > + > +#if defined(CONFIG_HAS_IOPORT) && defined(CONFIG_USB_UHCI_HCD) Is this really needed? This feels wrong, why is ioport odd like this? > /* > * Make sure the controller is completely inactive, unable to > * generate interrupts or do DMA. > @@ -711,6 +715,7 @@ int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base) > return 1; > } > EXPORT_SYMBOL_GPL(uhci_check_and_reset_hc); > +#endif /* defined(CONFIG_HAS_IOPORT && defined(CONFIG_USB_UHCI_HCD) */ > > static inline int io_type_enabled(struct pci_dev *pdev, unsigned int mask) > { > @@ -723,6 +728,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 CONFIG_HAS_IOPORT > unsigned long base = 0; > int i; > > @@ -737,6 +743,7 @@ static void quirk_usb_handoff_uhci(struct pci_dev *pdev) > > if (base) > uhci_check_and_reset_hc(pdev, base); > +#endif /* CONFIG_HAS_IOPORT */ > } > > static int mmio_resource_enabled(struct pci_dev *pdev, int idx) > diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h > index e729de21fad7..8c87505f0abc 100644 > --- a/drivers/usb/host/pci-quirks.h > +++ b/drivers/usb/host/pci-quirks.h > @@ -2,9 +2,10 @@ > #ifndef __LINUX_USB_PCI_QUIRKS_H > #define __LINUX_USB_PCI_QUIRKS_H > > -#ifdef CONFIG_USB_PCI > void uhci_reset_hc(struct pci_dev *pdev, unsigned long base); > int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base); > + > +#ifdef CONFIG_USB_PCI_AMD > int usb_hcd_amd_remote_wakeup_quirk(struct pci_dev *pdev); > bool usb_amd_hang_symptom_quirk(void); > bool usb_amd_prefetch_quirk(void); > @@ -12,23 +13,38 @@ void usb_amd_dev_put(void); > bool usb_amd_quirk_pll_check(void); > void usb_amd_quirk_pll_disable(void); > void usb_amd_quirk_pll_enable(void); > -void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev); > -void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev); > -void usb_disable_xhci_ports(struct pci_dev *xhci_pdev); > void sb800_prefetch(struct device *dev, int on); > bool usb_amd_pt_check_port(struct device *device, int port); > #else > -struct pci_dev; > +static inline bool usb_amd_hang_symptom_quirk(void) > +{ > + return false; > +}; > +static inline bool usb_amd_prefetch_quirk(void) > +{ > + return false; > +} > +static inline bool usb_amd_quirk_pll_check(void) > +{ > + return false; > +} > static inline void usb_amd_quirk_pll_disable(void) {} > static inline void usb_amd_quirk_pll_enable(void) {} > -static inline void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev) {} > static inline void usb_amd_dev_put(void) {} > -static inline void usb_disable_xhci_ports(struct pci_dev *xhci_pdev) {} > static inline void sb800_prefetch(struct device *dev, int on) {} > static inline bool usb_amd_pt_check_port(struct device *device, int port) > { > return false; > } > +#endif /* CONFIG_USB_PCI_AMD */ > + > +#ifdef CONFIG_USB_PCI > +void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev); > +void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev); > +void usb_disable_xhci_ports(struct pci_dev *xhci_pdev); > +#else > +static inline void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev) {} > +static inline void usb_disable_xhci_ports(struct pci_dev *xhci_pdev) {} > #endif /* CONFIG_USB_PCI */ Again, the changes here are hard to follow, why? Please break up into smaller pieces. thanks, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-29 14:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20230522105049.1467313-1-schnelle@linux.ibm.com>
2023-05-22 10:50 ` [PATCH v5 36/44] usb: add HAS_IOPORT dependencies Niklas Schnelle
2023-05-22 10:50 ` [PATCH v5 37/44] usb: uhci: handle " Niklas Schnelle
2023-05-22 13:57 ` Alan Stern
2023-05-22 10:50 ` [PATCH v5 38/44] usb: pci-quirks: " Niklas Schnelle
2023-05-29 14:34 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox