diff mbox series

[V2] USB: OHCI: Add quirk for LS7A OHCI controller (rev 0x02)

Message ID 20250327044840.3179796-1-chenhuacai@loongson.cn (mailing list archive)
State Superseded
Headers show
Series [V2] USB: OHCI: Add quirk for LS7A OHCI controller (rev 0x02) | expand

Commit Message

Huacai Chen March 27, 2025, 4:48 a.m. UTC 
The OHCI controller (rev 0x02) under LS7A PCI host has a hardware flaw.
MMIO register with offset 0x60/0x64 is treated as legacy PS2-compatible
keyboard/mouse interface, which confuse the OHCI controller. Since OHCI
only use a 4KB BAR resource indeed, the LS7A OHCI controller's 32KB BAR
is wrapped around (the second 4KB BAR space is the same as the first 4KB
internally). So we can add an 4KB offset (0x1000) to the OHCI registers
(from the PCI BAR resource) as a quirk.

Cc: stable@vger.kernel.org
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Mingcong Bai <baimingcong@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
V2: add a comment explaining why the quirk is needed and how it fixes.

 drivers/usb/host/ohci-pci.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Alan Stern March 27, 2025, 2:13 p.m. UTC | #1 
On Thu, Mar 27, 2025 at 12:48:40PM +0800, Huacai Chen wrote:
> The OHCI controller (rev 0x02) under LS7A PCI host has a hardware flaw.
> MMIO register with offset 0x60/0x64 is treated as legacy PS2-compatible
> keyboard/mouse interface, which confuse the OHCI controller. Since OHCI
> only use a 4KB BAR resource indeed, the LS7A OHCI controller's 32KB BAR
> is wrapped around (the second 4KB BAR space is the same as the first 4KB
> internally). So we can add an 4KB offset (0x1000) to the OHCI registers
> (from the PCI BAR resource) as a quirk.
> 
> Cc: stable@vger.kernel.org
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Tested-by: Mingcong Bai <baimingcong@loongson.cn>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> V2: add a comment explaining why the quirk is needed and how it fixes.
> 
>  drivers/usb/host/ohci-pci.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
> index 900ea0d368e0..bd90b2fed51b 100644
> --- a/drivers/usb/host/ohci-pci.c
> +++ b/drivers/usb/host/ohci-pci.c
> @@ -165,6 +165,24 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd)
>  	return 0;
>  }
>  
> +static int ohci_quirk_loongson(struct usb_hcd *hcd)
> +{
> +	struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
> +
> +	/*
> +	 * Loongson's LS7A OHCI controller (rev 0x02) has a
> +	 * flaw. MMIO register with offset 0x60/64 is treated
> +	 * as legacy PS2-compatible keyboard/mouse interface.
> +	 * Since OHCI only use 4KB BAR resource, LS7A OHCI's
> +	 * 32KB BAR is wrapped around (the 2nd 4KB BAR space
> +	 * is the same as the 1st 4KB internally). So add 4KB
> +	 * offset (0x1000) to the OHCI registers as a quirk.
> +	 */
> +	hcd->regs += (pdev->revision == 0x2) ? 0x1000 : 0x0;

I'm sorry, I should have mentioned this previously but I only noticed it 
now.  This would be a lot easier for people to read if you wrote it as a 
simple "if" statement:

	if (pdev->revision == 0x02)
		hcd->regs += 0x1000;

Otherwise the patch looks fine.  If you make this change, you can 
resubmit it with:

Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern

> +
> +	return 0;
> +}
> +
>  static int ohci_quirk_qemu(struct usb_hcd *hcd)
>  {
>  	struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> @@ -224,6 +242,10 @@ static const struct pci_device_id ohci_pci_quirks[] = {
>  		PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4399),
>  		.driver_data = (unsigned long)ohci_quirk_amd700,
>  	},
> +	{
> +		PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a24),
> +		.driver_data = (unsigned long)ohci_quirk_loongson,
> +	},
>  	{
>  		.vendor		= PCI_VENDOR_ID_APPLE,
>  		.device		= 0x003f,
Huacai Chen March 29, 2025, 8:40 a.m. UTC | #2 
On Thu, Mar 27, 2025 at 10:13 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Thu, Mar 27, 2025 at 12:48:40PM +0800, Huacai Chen wrote:
> > The OHCI controller (rev 0x02) under LS7A PCI host has a hardware flaw.
> > MMIO register with offset 0x60/0x64 is treated as legacy PS2-compatible
> > keyboard/mouse interface, which confuse the OHCI controller. Since OHCI
> > only use a 4KB BAR resource indeed, the LS7A OHCI controller's 32KB BAR
> > is wrapped around (the second 4KB BAR space is the same as the first 4KB
> > internally). So we can add an 4KB offset (0x1000) to the OHCI registers
> > (from the PCI BAR resource) as a quirk.
> >
> > Cc: stable@vger.kernel.org
> > Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> > Tested-by: Mingcong Bai <baimingcong@loongson.cn>
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > V2: add a comment explaining why the quirk is needed and how it fixes.
> >
> >  drivers/usb/host/ohci-pci.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
> > index 900ea0d368e0..bd90b2fed51b 100644
> > --- a/drivers/usb/host/ohci-pci.c
> > +++ b/drivers/usb/host/ohci-pci.c
> > @@ -165,6 +165,24 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd)
> >       return 0;
> >  }
> >
> > +static int ohci_quirk_loongson(struct usb_hcd *hcd)
> > +{
> > +     struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
> > +
> > +     /*
> > +      * Loongson's LS7A OHCI controller (rev 0x02) has a
> > +      * flaw. MMIO register with offset 0x60/64 is treated
> > +      * as legacy PS2-compatible keyboard/mouse interface.
> > +      * Since OHCI only use 4KB BAR resource, LS7A OHCI's
> > +      * 32KB BAR is wrapped around (the 2nd 4KB BAR space
> > +      * is the same as the 1st 4KB internally). So add 4KB
> > +      * offset (0x1000) to the OHCI registers as a quirk.
> > +      */
> > +     hcd->regs += (pdev->revision == 0x2) ? 0x1000 : 0x0;
>
> I'm sorry, I should have mentioned this previously but I only noticed it
> now.  This would be a lot easier for people to read if you wrote it as a
> simple "if" statement:
>
>         if (pdev->revision == 0x02)
>                 hcd->regs += 0x1000;
>
> Otherwise the patch looks fine.  If you make this change, you can
> resubmit it with:
>
> Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
Thanks, V3 is sent:
https://lore.kernel.org/linux-usb/20250328040059.3672979-1-chenhuacai@loongson.cn/T/#u

Huacai

>
> Alan Stern
>
> > +
> > +     return 0;
> > +}
> > +
> >  static int ohci_quirk_qemu(struct usb_hcd *hcd)
> >  {
> >       struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> > @@ -224,6 +242,10 @@ static const struct pci_device_id ohci_pci_quirks[] = {
> >               PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4399),
> >               .driver_data = (unsigned long)ohci_quirk_amd700,
> >       },
> > +     {
> > +             PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a24),
> > +             .driver_data = (unsigned long)ohci_quirk_loongson,
> > +     },
> >       {
> >               .vendor         = PCI_VENDOR_ID_APPLE,
> >               .device         = 0x003f,
Alan Stern March 29, 2025, 2:32 p.m. UTC | #3 
On Sat, Mar 29, 2025 at 04:40:59PM +0800, Huacai Chen wrote:
> Thanks, V3 is sent:
> https://lore.kernel.org/linux-usb/20250328040059.3672979-1-chenhuacai@loongson.cn/T/#u

Okay, it looks good.

Alan Stern
diff mbox series

Patch

diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index 900ea0d368e0..bd90b2fed51b 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -165,6 +165,24 @@  static int ohci_quirk_amd700(struct usb_hcd *hcd)
 	return 0;
 }
 
+static int ohci_quirk_loongson(struct usb_hcd *hcd)
+{
+	struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
+
+	/*
+	 * Loongson's LS7A OHCI controller (rev 0x02) has a
+	 * flaw. MMIO register with offset 0x60/64 is treated
+	 * as legacy PS2-compatible keyboard/mouse interface.
+	 * Since OHCI only use 4KB BAR resource, LS7A OHCI's
+	 * 32KB BAR is wrapped around (the 2nd 4KB BAR space
+	 * is the same as the 1st 4KB internally). So add 4KB
+	 * offset (0x1000) to the OHCI registers as a quirk.
+	 */
+	hcd->regs += (pdev->revision == 0x2) ? 0x1000 : 0x0;
+
+	return 0;
+}
+
 static int ohci_quirk_qemu(struct usb_hcd *hcd)
 {
 	struct ohci_hcd *ohci = hcd_to_ohci(hcd);
@@ -224,6 +242,10 @@  static const struct pci_device_id ohci_pci_quirks[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_ATI, 0x4399),
 		.driver_data = (unsigned long)ohci_quirk_amd700,
 	},
+	{
+		PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a24),
+		.driver_data = (unsigned long)ohci_quirk_loongson,
+	},
 	{
 		.vendor		= PCI_VENDOR_ID_APPLE,
 		.device		= 0x003f,