On Mon, Mar 14, 2022 at 03:35:37PM +0800, WeitaoWang-oc@xxxxxxxxxxx wrote:
> In zhaoxin platform, some ehci projects will latch a wakeup signal
> internal when plug in a device on port during system S0. This wakeup
> signal will turn on when ehci runtime suspend, which will trigger a
> system control interrupt that will resume ehci back to D0. As no
> device connect, ehci will be set to runtime suspend and turn on the
> internal latched wakeup signal again. It will cause a suspend-resume
> loop and generate system control interrupt continuously.
>
> Fixed this issue by clear wakeup signal latched in ehci internal when
> ehci resume callback is called.
>
> Signed-off-by: Weitao Wang <WeitaoWang-oc@xxxxxxxxxxx>
> ---
> drivers/usb/host/ehci-hcd.c | 26 ++++++++++++++++++++++++++
> drivers/usb/host/ehci-pci.c | 4 ++++
> drivers/usb/host/ehci.h | 1 +
> 3 files changed, 31 insertions(+)
>
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 3d82e0b..e4840ef 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -1103,6 +1103,30 @@ static void ehci_remove_device(struct usb_hcd *hcd,
> struct usb_device *udev)
>
> #ifdef CONFIG_PM
>
> +/* Clear wakeup signal locked in zhaoxin platform when device plug in. */
> +static void ehci_zx_wakeup_clear(struct ehci_hcd *ehci)
> +{
> + u32 __iomem *reg = &ehci->regs->port_status[4];
> + u32 t1 = ehci_readl(ehci, reg);
The "t1" in this line should start in the same column as the "*reg" in
the line above, to match the style of other variable declarations in
this file (see ehci_init() as an example).
> +
> + t1 &= (u32)~0xf0000;
> + t1 |= PORT_TEST_FORCE;
> + ehci_writel(ehci, t1, reg);
> + t1 = ehci_readl(ehci, reg);
> + msleep(1);
> + t1 &= (u32)~0xf0000;
> + ehci_writel(ehci, t1, reg);
> + ehci_readl(ehci, reg);
> + msleep(1);
> + t1 = ehci_readl(ehci, reg);
> + ehci_writel(ehci, t1 | PORT_CSC, reg);
> + ehci_readl(ehci, reg);
> + udelay(500);
> + t1 = ehci_readl(ehci, &ehci->regs->status);
> + ehci_writel(ehci, t1 & STS_PCD, &ehci->regs->status);
> + ehci_readl(ehci, &ehci->regs->status);
You should not clear the STS_PCD bit. What if some other port had a
status change at the same time? Then because you cleared the
port-change-detect bit, the system would not realize that the other port
needed to be handled.
Leaving the STS_PCD bit turned on will cause the driver to do a little
extra work, but it shouldn't cause any harm.
> +}
> +
> /* suspend/resume, section 4.3 */
>
> /* These routines handle the generic parts of controller suspend/resume */
> @@ -1154,6 +1178,8 @@ int ehci_resume(struct usb_hcd *hcd, bool force_reset)
> if (ehci->shutdown)
> return 0; /* Controller is dead */
>
> + if (ehci->zx_wakeup_clear == 1)
You don't need to check that the value is equal to 1. Treat this
more like a Boolean flag and just write:
if (ehci->zx_wakeup_clear)
Also, to make the flag's meaning more obvious, you might want to name
it "zx_wakeup_clear_needed" or "zx_clear_latched_wakeup".
Otherwise this patch looks okay. Please submit a revised version,
without the whitespace damage.
Alan Stern
> + ehci_zx_wakeup_clear(ehci);
> /*
> * If CF is still set and reset isn't forced
> * then we maintained suspend power.
> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index e87cf3a..a5e27de 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -222,6 +222,10 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
> ehci->has_synopsys_hc_bug = 1;
> }
> break;
> + case PCI_VENDOR_ID_ZHAOXIN:
> + if (pdev->device == 0x3104 && (pdev->revision & 0xf0) ==
> 0x90)
> + ehci->zx_wakeup_clear = 1;
> + break;
> }
>
> /* optional debug port, normally in the first BAR */
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index fdd073c..712fdd0 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -220,6 +220,7 @@ struct ehci_hcd { /* one per
> controller */
> unsigned imx28_write_fix:1; /* For Freescale i.MX28
> */
> unsigned spurious_oc:1;
> unsigned is_aspeed:1;
> + unsigned zx_wakeup_clear:1;
>
> /* required for usb32 quirk */
> #define OHCI_CTRL_HCFS (3 << 6)
> --
> 2.7.4
