On Mon, May 22, 2017 at 06:43:25PM +1000, Benjamin Herrenschmidt wrote:
> The Aspeed 2400/2500 families have a variant of UHCI which requires
> some quirks to the driver to work:
A UHCI driver? New hardware? That's crazy, what decade is this? :)
>
> - The register offsets are different. We add a remapping helper.
>
> - All accesses have to be done via 32-bit loads and stores. We
> force all accessors to use readl/writel. This is of no consequence
> for reads as we never read "in the middle" of a register. For writes
> it also works fine as the registers only actually implement the bits
> we try to write (16-bit for the registers accessed with writew and
> 8-bit for the register accessed with writeb), so always using a
> 32-bit write will have no negative effect. We never do partial writes.
>
> - The resume detect interrupt is broken
>
> - The number of ports is (optionally) provided via the device-tree
>
> - The device comes up with stale status bits causing spurrious
> detection of a "dead" HC on first plug. We need to clear the status
> bits when starting the root hub.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> ---
>
> The device-tree updates will come separately. To work this also needs
> some pinmux & clock driver updates that are coming separately as well.
>
> drivers/usb/host/Kconfig | 6 +++-
> drivers/usb/host/uhci-hcd.c | 13 +++++++++
> drivers/usb/host/uhci-hcd.h | 61 ++++++++++++++++++++++++++++++++++++++++
> drivers/usb/host/uhci-platform.c | 24 +++++++++++++++-
> 4 files changed, 102 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 6361fc7..dab8a93 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -627,7 +627,11 @@ config USB_UHCI_SUPPORT_NON_PCI_HC
>
> config USB_UHCI_PLATFORM
> bool
> - default y if ARCH_VT8500
> + default y if (ARCH_VT8500 || ARCH_ASPEED)
> +
> +config USB_UHCI_ASPEED
> + bool
> + default y if ARCH_ASPEED
>
> config USB_UHCI_BIG_ENDIAN_MMIO
> bool
> diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
> index 683098afa..9a41195 100644
> --- a/drivers/usb/host/uhci-hcd.c
> +++ b/drivers/usb/host/uhci-hcd.c
> @@ -269,6 +269,10 @@ static int resume_detect_interrupts_are_broken(struct uhci_hcd *uhci)
> * we can't depend on resume-detect interrupts. */
> if (ignore_oc)
> return 1;
> +#ifdef CONFIG_USB_UHCI_ASPEED
> + if (uhci->is_aspeed)
> + return 1;
> +#endif
Ick, you know better, don't put #ifdefs everywhere, just always check
for this, no need to clutter up the code please. In looking over this
code, I don't think you need any #ifdef at all.
thanks,
greg k-h
>
> return uhci->resume_detect_interrupts_are_broken ?
> uhci->resume_detect_interrupts_are_broken(uhci) : 0;
> @@ -384,6 +388,15 @@ static void start_rh(struct uhci_hcd *uhci)
> {
> uhci->is_stopped = 0;
>
> +#ifdef CONFIG_USB_UHCI_ASPEED
> + /*
> + * Clear stale status bits on Aspeed as we get a stale HCH
> + * which causes problems later on
> + */
> + if (uhci->is_aspeed)
> + uhci_writew(uhci, uhci_readw(uhci, USBSTS), USBSTS);
> +#endif
> +
> /* Mark it configured and running with a 64-byte max packet.
> * All interrupts are enabled, even though RESUME won't do anything.
> */
> diff --git a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
> index 6f986d8..d2347f7 100644
> --- a/drivers/usb/host/uhci-hcd.h
> +++ b/drivers/usb/host/uhci-hcd.h
> @@ -48,6 +48,8 @@
> /* USB port status and control registers */
> #define USBPORTSC1 16
> #define USBPORTSC2 18
> +#define USBPORTSC3 20
> +#define USBPORTSC4 22
> #define USBPORTSC_CCS 0x0001 /* Current Connect Status
> * ("device present") */
> #define USBPORTSC_CSC 0x0002 /* Connect Status Change */
> @@ -427,6 +429,9 @@ struct uhci_hcd {
> unsigned int wait_for_hp:1; /* Wait for HP port reset */
> unsigned int big_endian_mmio:1; /* Big endian registers */
> unsigned int big_endian_desc:1; /* Big endian descriptors */
> +#ifdef CONFIG_USB_UHCI_ASPEED
> + unsigned int is_aspeed:1; /* Aspeed impl. workarounds */
> +#endif
>
> /* Support for port suspend/resume/reset */
> unsigned long port_c_suspend; /* Bit-arrays of ports */
> @@ -545,10 +550,46 @@ static inline void uhci_writeb(const struct uhci_hcd *uhci, u8 val, int reg)
> #define uhci_big_endian_mmio(u) 0
> #endif
>
> +#ifdef CONFIG_USB_UHCI_ASPEED
> +static inline int uhci_aspeed_reg(unsigned int reg)
> +{
> + switch (reg) {
> + case USBCMD:
> + return 00;
> + case USBSTS:
> + return 0x04;
> + case USBINTR:
> + return 0x08;
> + case USBFRNUM:
> + return 0x80;
> + case USBFLBASEADD:
> + return 0x0c;
> + case USBSOF:
> + return 0x84;
> + case USBPORTSC1:
> + return 0x88;
> + case USBPORTSC2:
> + return 0x8c;
> + case USBPORTSC3:
> + return 0x90;
> + case USBPORTSC4:
> + return 0x94;
> + default:
> + pr_warn("UHCI: Unsupported register 0x%02x on Aspeed\n", reg);
> + /* Return an unimplemented register */
> + return 0x10;
> + }
> +}
> +#endif /* CONFIG_USB_UHCI_ASPEED */
> +
> static inline u32 uhci_readl(const struct uhci_hcd *uhci, int reg)
> {
> if (uhci_has_pci_registers(uhci))
> return inl(uhci->io_addr + reg);
> +#ifdef CONFIG_USB_UHCI_ASPEED
> + else if (uhci->is_aspeed)
> + return readl(uhci->regs + uhci_aspeed_reg(reg));
> +#endif
> #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
> else if (uhci_big_endian_mmio(uhci))
> return readl_be(uhci->regs + reg);
> @@ -561,6 +602,10 @@ 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);
> +#ifdef CONFIG_USB_UHCI_ASPEED
> + else if (uhci->is_aspeed)
> + writel(val, uhci->regs + uhci_aspeed_reg(reg));
> +#endif
> #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
> else if (uhci_big_endian_mmio(uhci))
> writel_be(val, uhci->regs + reg);
> @@ -573,6 +618,10 @@ static inline u16 uhci_readw(const struct uhci_hcd *uhci, int reg)
> {
> if (uhci_has_pci_registers(uhci))
> return inw(uhci->io_addr + reg);
> +#ifdef CONFIG_USB_UHCI_ASPEED
> + else if (uhci->is_aspeed) /* 32-bit access only */
> + return readl(uhci->regs + uhci_aspeed_reg(reg));
> +#endif
> #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
> else if (uhci_big_endian_mmio(uhci))
> return readw_be(uhci->regs + reg);
> @@ -585,6 +634,10 @@ 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);
> +#ifdef CONFIG_USB_UHCI_ASPEED
> + else if (uhci->is_aspeed) /* 32-bit access only */
> + writel(val, uhci->regs + uhci_aspeed_reg(reg));
> +#endif
> #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
> else if (uhci_big_endian_mmio(uhci))
> writew_be(val, uhci->regs + reg);
> @@ -597,6 +650,10 @@ static inline u8 uhci_readb(const struct uhci_hcd *uhci, int reg)
> {
> if (uhci_has_pci_registers(uhci))
> return inb(uhci->io_addr + reg);
> +#ifdef CONFIG_USB_UHCI_ASPEED
> + else if (uhci->is_aspeed) /* 32-bit access only */
> + return readl(uhci->regs + uhci_aspeed_reg(reg));
> +#endif
> #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
> else if (uhci_big_endian_mmio(uhci))
> return readb_be(uhci->regs + reg);
> @@ -609,6 +666,10 @@ 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);
> +#ifdef CONFIG_USB_UHCI_ASPEED
> + else if (uhci->is_aspeed) /* 32-bit access only (only used for SOF) */
> + writel(val, uhci->regs + uhci_aspeed_reg(reg));
> +#endif
> #ifdef CONFIG_USB_UHCI_BIG_ENDIAN_MMIO
> else if (uhci_big_endian_mmio(uhci))
> writeb_be(val, uhci->regs + reg);
> diff --git a/drivers/usb/host/uhci-platform.c b/drivers/usb/host/uhci-platform.c
> index 32a6f3d..62efa34 100644
> --- a/drivers/usb/host/uhci-platform.c
> +++ b/drivers/usb/host/uhci-platform.c
> @@ -15,7 +15,9 @@ static int uhci_platform_init(struct usb_hcd *hcd)
> {
> struct uhci_hcd *uhci = hcd_to_uhci(hcd);
>
> - uhci->rh_numports = uhci_count_ports(hcd);
> + /* Probe number of ports if not already provided by DT */
> + if (!uhci->rh_numports)
> + uhci->rh_numports = uhci_count_ports(hcd);
>
> /* Set up pointers to to generic functions */
> uhci->reset_hc = uhci_generic_reset_hc;
> @@ -63,6 +65,7 @@ static const struct hc_driver uhci_platform_hc_driver = {
>
> static int uhci_hcd_platform_probe(struct platform_device *pdev)
> {
> + struct device_node *np = pdev->dev.of_node;
> struct usb_hcd *hcd;
> struct uhci_hcd *uhci;
> struct resource *res;
> @@ -98,6 +101,25 @@ static int uhci_hcd_platform_probe(struct platform_device *pdev)
>
> uhci->regs = hcd->regs;
>
> + /* Grab some things from the device-tree */
> + if (np) {
> + u32 num_ports;
> +
> + if (of_property_read_u32(np, "#ports", &num_ports) == 0) {
> + uhci->rh_numports = num_ports;
> + dev_info(&pdev->dev,
> + "Detected %d ports from device-tree\n",
> + num_ports);
> + }
> +#ifdef CONFIG_USB_UHCI_ASPEED
> + if (of_device_is_compatible(np, "aspeed,ast2400-uhci") ||
> + of_device_is_compatible(np, "aspeed,ast2500-uhci")) {
> + uhci->is_aspeed = 1;
> + dev_info(&pdev->dev,
> + "Enabled Aspeed implementation workarounds\n");
> + }
> +#endif /* CONFIG_USB_UHCI_ASPEED */
> + }
> ret = usb_add_hcd(hcd, pdev->resource[1].start, IRQF_SHARED);
> if (ret)
> goto err_rmr;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
