* [PATCH 1/1] usb: xhci: avoid VBus glitch during controller reset operation
@ 2019-08-21 3:18 Peter Chen
2019-08-22 11:08 ` Mathias Nyman
0 siblings, 1 reply; 6+ messages in thread
From: Peter Chen @ 2019-08-21 3:18 UTC (permalink / raw)
To: mathias.nyman; +Cc: linux-usb, dl-linux-imx, Peter Chen
According to xHCI 1.1 CH4.19.4 Port Power:
While Chip Hardware Reset or HCRST is asserted,
the value of PP is undefined. If the xHC supports
power switches (PPC = ‘1’) then VBus may be deasserted
during this time. PP (and VBus) shall be enabled immediately
upon exiting the reset condition.
The VBus glitch may cause some USB devices work abnormal, we observe
it at NXP LS1012AFWRY/LS1043ARDB/LX2160AQDS/LS1088ARDB platforms. To
avoid this Vbus glitch, we could set PP as 0 before HCRST, and the PP
will back to 1 after HCRST according to spec.
Reported-by: Ran Wang <ran.wang_1@nxp.com>
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
drivers/usb/host/xhci.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6b34a573c3d9..f5a7b5d63031 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -167,7 +167,8 @@ int xhci_reset(struct xhci_hcd *xhci)
{
u32 command;
u32 state;
- int ret;
+ int ret, i;
+ u32 portsc;
state = readl(&xhci->op_regs->status);
@@ -181,6 +182,18 @@ int xhci_reset(struct xhci_hcd *xhci)
return 0;
}
+ /*
+ * Keep PORTSC.PP as 0 before HCRST to eliminate
+ * Vbus glitch, see CH 4.19.4.
+ */
+ for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
+ __le32 __iomem *port_addr = &xhci->op_regs->port_status_base +
+ NUM_PORT_REGS * i;
+ portsc = readl(port_addr);
+ portsc &= ~PORT_POWER;
+ writel(portsc, port_addr);
+ }
+
xhci_dbg_trace(xhci, trace_xhci_dbg_init, "// Reset the HC");
command = readl(&xhci->op_regs->command);
command |= CMD_RESET;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] usb: xhci: avoid VBus glitch during controller reset operation
2019-08-21 3:18 [PATCH 1/1] usb: xhci: avoid VBus glitch during controller reset operation Peter Chen
@ 2019-08-22 11:08 ` Mathias Nyman
2019-08-23 1:01 ` Peter Chen
0 siblings, 1 reply; 6+ messages in thread
From: Mathias Nyman @ 2019-08-22 11:08 UTC (permalink / raw)
To: Peter Chen, mathias.nyman; +Cc: linux-usb, dl-linux-imx
On 21.8.2019 6.18, Peter Chen wrote:
> According to xHCI 1.1 CH4.19.4 Port Power:
> While Chip Hardware Reset or HCRST is asserted,
> the value of PP is undefined. If the xHC supports
> power switches (PPC = ‘1’) then VBus may be deasserted
> during this time. PP (and VBus) shall be enabled immediately
> upon exiting the reset condition.
>
> The VBus glitch may cause some USB devices work abnormal, we observe
> it at NXP LS1012AFWRY/LS1043ARDB/LX2160AQDS/LS1088ARDB platforms. To
> avoid this Vbus glitch, we could set PP as 0 before HCRST, and the PP
> will back to 1 after HCRST according to spec.
Is this glitch causing issues already at the first xHC reset when we are
loading the xhci driver, or only later resets, like xHC reset at resume?
>
> Reported-by: Ran Wang <ran.wang_1@nxp.com>
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
> drivers/usb/host/xhci.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 6b34a573c3d9..f5a7b5d63031 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -167,7 +167,8 @@ int xhci_reset(struct xhci_hcd *xhci)
> {
> u32 command;
> u32 state;
> - int ret;
> + int ret, i;
> + u32 portsc;
>
> state = readl(&xhci->op_regs->status);
>
> @@ -181,6 +182,18 @@ int xhci_reset(struct xhci_hcd *xhci)
> return 0;
> }
>
> + /*
> + * Keep PORTSC.PP as 0 before HCRST to eliminate
> + * Vbus glitch, see CH 4.19.4.
> + */
> + for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
> + __le32 __iomem *port_addr = &xhci->op_regs->port_status_base +
> + NUM_PORT_REGS * i;
> + portsc = readl(port_addr);
> + portsc &= ~PORT_POWER;
> + writel(portsc, port_addr);
Not all bits read from PORTSC should be written back, you might need
portsc = xhci_port_state_to_neutral(portsc) here.
Normally I'd recommend using the xhci_set_port_power() helper instead, but
if this is is needed at driver load time then port arrays may not be set up yet.
xhci_set_port_power() would take care of possible ACPI method calls, and add some debugging.
Not sure if this will impact some other platforms, maybe it would be better to move this to
a separate function, and call it before xhci_reset() if a quirk is set.
-Mathias
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] usb: xhci: avoid VBus glitch during controller reset operation
2019-08-22 11:08 ` Mathias Nyman
@ 2019-08-23 1:01 ` Peter Chen
2019-08-23 1:59 ` Ran Wang
0 siblings, 1 reply; 6+ messages in thread
From: Peter Chen @ 2019-08-23 1:01 UTC (permalink / raw)
To: Mathias Nyman; +Cc: mathias.nyman, linux-usb, dl-linux-imx, Ran Wang
On 19-08-22 14:08:26, Mathias Nyman wrote:
> On 21.8.2019 6.18, Peter Chen wrote:
> > According to xHCI 1.1 CH4.19.4 Port Power:
> > While Chip Hardware Reset or HCRST is asserted,
> > the value of PP is undefined. If the xHC supports
> > power switches (PPC = ‘1’) then VBus may be deasserted
> > during this time. PP (and VBus) shall be enabled immediately
> > upon exiting the reset condition.
> >
> > The VBus glitch may cause some USB devices work abnormal, we observe
> > it at NXP LS1012AFWRY/LS1043ARDB/LX2160AQDS/LS1088ARDB platforms. To
> > avoid this Vbus glitch, we could set PP as 0 before HCRST, and the PP
> > will back to 1 after HCRST according to spec.
>
> Is this glitch causing issues already at the first xHC reset when we are
> loading the xhci driver, or only later resets, like xHC reset at resume?
The first time, Ran would you please confirm?
>
> >
> > Reported-by: Ran Wang <ran.wang_1@nxp.com>
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > ---
> > drivers/usb/host/xhci.c | 15 ++++++++++++++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index 6b34a573c3d9..f5a7b5d63031 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -167,7 +167,8 @@ int xhci_reset(struct xhci_hcd *xhci)
> > {
> > u32 command;
> > u32 state;
> > - int ret;
> > + int ret, i;
> > + u32 portsc;
> > state = readl(&xhci->op_regs->status);
> > @@ -181,6 +182,18 @@ int xhci_reset(struct xhci_hcd *xhci)
> > return 0;
> > }
> > + /*
> > + * Keep PORTSC.PP as 0 before HCRST to eliminate
> > + * Vbus glitch, see CH 4.19.4.
> > + */
> > + for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
> > + __le32 __iomem *port_addr = &xhci->op_regs->port_status_base +
> > + NUM_PORT_REGS * i;
> > + portsc = readl(port_addr);
> > + portsc &= ~PORT_POWER;
> > + writel(portsc, port_addr);
>
> Not all bits read from PORTSC should be written back, you might need
> portsc = xhci_port_state_to_neutral(portsc) here.
Will change.
>
> Normally I'd recommend using the xhci_set_port_power() helper instead, but
> if this is is needed at driver load time then port arrays may not be set up yet.
> xhci_set_port_power() would take care of possible ACPI method calls, and add some debugging.
>
It is needed at load time, so I did not use port array.
> Not sure if this will impact some other platforms, maybe it would be better to move this to
> a separate function, and call it before xhci_reset() if a quirk is set.
>
It follows spec, not at quirk. We talked with Synopsis engineer
(case: 8001235479), they confirmed this behaviour follows spec.
Besides, I tried at both dwc3 and cadence3 xHCI platforms,
the PORT_POWER will be set again after controller set.
What's potential issue you consider?
Thanks,
Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 1/1] usb: xhci: avoid VBus glitch during controller reset operation
2019-08-23 1:01 ` Peter Chen
@ 2019-08-23 1:59 ` Ran Wang
2019-08-23 3:33 ` Peter Chen
0 siblings, 1 reply; 6+ messages in thread
From: Ran Wang @ 2019-08-23 1:59 UTC (permalink / raw)
To: Peter Chen, Mathias Nyman; +Cc: mathias.nyman, linux-usb, dl-linux-imx
HI Peter, Mathias,
Sorry for the late review.
On Friday, August 23, 2019 09:02, Peter Chen wrote:
>
> On 19-08-22 14:08:26, Mathias Nyman wrote:
> > On 21.8.2019 6.18, Peter Chen wrote:
> > > According to xHCI 1.1 CH4.19.4 Port Power:
> > > While Chip Hardware Reset or HCRST is asserted,
> > > the value of PP is undefined. If the xHC supports
> > > power switches (PPC = ‘1’) then VBus may be deasserted
> > > during this time. PP (and VBus) shall be enabled immediately
> > > upon exiting the reset condition.
> > >
> > > The VBus glitch may cause some USB devices work abnormal, we observe
> > > it at NXP LS1012AFWRY/LS1043ARDB/LX2160AQDS/LS1088ARDB platforms.
> To
> > > avoid this Vbus glitch, we could set PP as 0 before HCRST, and the
> > > PP will back to 1 after HCRST according to spec.
> >
> > Is this glitch causing issues already at the first xHC reset when we
> > are loading the xhci driver, or only later resets, like xHC reset at resume?
>
> The first time, Ran would you please confirm?
My understand is this will happened whenever PP is set to 0, such as xHCI reset.
So I think it might also be observed during resume if xHC do reset.
However, according to my previous testing, it might be too late to
do this port power off in xhci_reset(), actually the VBUS will assert once DWC3 driver
set IP to host mode (before doing xHC reset). So the glitch still can be observed on the scope;
I have more issue description in another patch discussion about this, please see
lore.kernel.org/patchwork/patch/1032425/
Quoted from it:
Actually I have done experiment like what you suggested (in xhci-plat.c), but the timing
seems too late--making VBUS waveform look like a square wave as below:
Here DWC3 enable host mode, VBUS on
|
+5V /---------\ 40ms /---------------------------....
0V ________/ 90ms \______/
| |
| Here do xhci reset, VBUS back to +5V again
Here set all PORTSC[PP] to 0 in xhci_gen_setup()
So I am afraid the solution might have to be added in DWC3 core driver where just after host mode enabling code if want fix this :(
Regards,
Ran
> >
> > >
> > > Reported-by: Ran Wang <ran.wang_1@nxp.com>
> > > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > > ---
> > > drivers/usb/host/xhci.c | 15 ++++++++++++++-
> > > 1 file changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index
> > > 6b34a573c3d9..f5a7b5d63031 100644
> > > --- a/drivers/usb/host/xhci.c
> > > +++ b/drivers/usb/host/xhci.c
> > > @@ -167,7 +167,8 @@ int xhci_reset(struct xhci_hcd *xhci)
> > > {
> > > u32 command;
> > > u32 state;
> > > - int ret;
> > > + int ret, i;
> > > + u32 portsc;
> > > state = readl(&xhci->op_regs->status); @@ -181,6 +182,18 @@ int
> > > xhci_reset(struct xhci_hcd *xhci)
> > > return 0;
> > > }
> > > + /*
> > > + * Keep PORTSC.PP as 0 before HCRST to eliminate
> > > + * Vbus glitch, see CH 4.19.4.
> > > + */
> > > + for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
> > > + __le32 __iomem *port_addr = &xhci->op_regs-
> >port_status_base +
> > > + NUM_PORT_REGS * i;
> > > + portsc = readl(port_addr);
> > > + portsc &= ~PORT_POWER;
> > > + writel(portsc, port_addr);
> >
> > Not all bits read from PORTSC should be written back, you might need
> > portsc = xhci_port_state_to_neutral(portsc) here.
>
> Will change.
>
> >
> > Normally I'd recommend using the xhci_set_port_power() helper instead,
> > but if this is is needed at driver load time then port arrays may not be set up
> yet.
> > xhci_set_port_power() would take care of possible ACPI method calls, and add
> some debugging.
> >
>
> It is needed at load time, so I did not use port array.
>
> > Not sure if this will impact some other platforms, maybe it would be
> > better to move this to a separate function, and call it before xhci_reset() if a
> quirk is set.
> >
>
> It follows spec, not at quirk. We talked with Synopsis engineer
> (case: 8001235479), they confirmed this behaviour follows spec.
> Besides, I tried at both dwc3 and cadence3 xHCI platforms, the PORT_POWER
> will be set again after controller set.
>
> What's potential issue you consider?
>
> Thanks,
> Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] usb: xhci: avoid VBus glitch during controller reset operation
2019-08-23 1:59 ` Ran Wang
@ 2019-08-23 3:33 ` Peter Chen
2019-08-23 3:52 ` Ran Wang
0 siblings, 1 reply; 6+ messages in thread
From: Peter Chen @ 2019-08-23 3:33 UTC (permalink / raw)
To: Ran Wang; +Cc: Mathias Nyman, mathias.nyman, linux-usb, dl-linux-imx
On 19-08-23 01:59:24, Ran Wang wrote:
> HI Peter, Mathias,
>
> Sorry for the late review.
>
> On Friday, August 23, 2019 09:02, Peter Chen wrote:
> >
> > On 19-08-22 14:08:26, Mathias Nyman wrote:
> > > On 21.8.2019 6.18, Peter Chen wrote:
> > > > According to xHCI 1.1 CH4.19.4 Port Power:
> > > > While Chip Hardware Reset or HCRST is asserted,
> > > > the value of PP is undefined. If the xHC supports
> > > > power switches (PPC = ‘1’) then VBus may be deasserted
> > > > during this time. PP (and VBus) shall be enabled immediately
> > > > upon exiting the reset condition.
> > > >
> > > > The VBus glitch may cause some USB devices work abnormal, we observe
> > > > it at NXP LS1012AFWRY/LS1043ARDB/LX2160AQDS/LS1088ARDB platforms.
> > To
> > > > avoid this Vbus glitch, we could set PP as 0 before HCRST, and the
> > > > PP will back to 1 after HCRST according to spec.
> > >
> > > Is this glitch causing issues already at the first xHC reset when we
> > > are loading the xhci driver, or only later resets, like xHC reset at resume?
> >
> > The first time, Ran would you please confirm?
>
> My understand is this will happened whenever PP is set to 0, such as xHCI reset.
> So I think it might also be observed during resume if xHC do reset.
Yes, Vbus glitch should exists whenever we do controller reset, I thought
you only meet this issue during boots up.
>
> However, according to my previous testing, it might be too late to
> do this port power off in xhci_reset(), actually the VBUS will assert once DWC3 driver
> set IP to host mode (before doing xHC reset). So the glitch still can be observed on the scope;
> I have more issue description in another patch discussion about this, please see
>
> lore.kernel.org/patchwork/patch/1032425/
> Quoted from it:
> Actually I have done experiment like what you suggested (in xhci-plat.c), but the timing
> seems too late--making VBUS waveform look like a square wave as below:
>
> Here DWC3 enable host mode, VBUS on
> |
> +5V /---------\ 40ms /---------------------------....
> 0V ________/ 90ms \______/
> | |
> | Here do xhci reset, VBUS back to +5V again
> Here set all PORTSC[PP] to 0 in xhci_gen_setup()
>
> So I am afraid the solution might have to be added in DWC3 core driver where just after host mode enabling code if want fix this :(
>
Per spec 4.19.4 Port Power:
Whether an xHC implementation supports port power switches or not,
it shall automatically enable VBus on all Root Hub ports after a
Chip Hardware Reset or HCRST.
Ran's observation confirmed it, PP is asserted once xHCI is enabled.
From the code, the HCRST will be at boots up and system resume.
So, it seems we can't keep PP always asserted. For xHCI, to avoid
Vbus glitch totally, we may have to control Vbus without PP
(eg, configure PP pin as GPIO).
Peter
>
> Regards,
> Ran
> > >
> > > >
> > > > Reported-by: Ran Wang <ran.wang_1@nxp.com>
> > > > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > > > ---
> > > > drivers/usb/host/xhci.c | 15 ++++++++++++++-
> > > > 1 file changed, 14 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index
> > > > 6b34a573c3d9..f5a7b5d63031 100644
> > > > --- a/drivers/usb/host/xhci.c
> > > > +++ b/drivers/usb/host/xhci.c
> > > > @@ -167,7 +167,8 @@ int xhci_reset(struct xhci_hcd *xhci)
> > > > {
> > > > u32 command;
> > > > u32 state;
> > > > - int ret;
> > > > + int ret, i;
> > > > + u32 portsc;
> > > > state = readl(&xhci->op_regs->status); @@ -181,6 +182,18 @@ int
> > > > xhci_reset(struct xhci_hcd *xhci)
> > > > return 0;
> > > > }
> > > > + /*
> > > > + * Keep PORTSC.PP as 0 before HCRST to eliminate
> > > > + * Vbus glitch, see CH 4.19.4.
> > > > + */
> > > > + for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
> > > > + __le32 __iomem *port_addr = &xhci->op_regs-
> > >port_status_base +
> > > > + NUM_PORT_REGS * i;
> > > > + portsc = readl(port_addr);
> > > > + portsc &= ~PORT_POWER;
> > > > + writel(portsc, port_addr);
> > >
> > > Not all bits read from PORTSC should be written back, you might need
> > > portsc = xhci_port_state_to_neutral(portsc) here.
> >
> > Will change.
> >
> > >
> > > Normally I'd recommend using the xhci_set_port_power() helper instead,
> > > but if this is is needed at driver load time then port arrays may not be set up
> > yet.
> > > xhci_set_port_power() would take care of possible ACPI method calls, and add
> > some debugging.
> > >
> >
> > It is needed at load time, so I did not use port array.
> >
> > > Not sure if this will impact some other platforms, maybe it would be
> > > better to move this to a separate function, and call it before xhci_reset() if a
> > quirk is set.
> > >
> >
> > It follows spec, not at quirk. We talked with Synopsis engineer
> > (case: 8001235479), they confirmed this behaviour follows spec.
> > Besides, I tried at both dwc3 and cadence3 xHCI platforms, the PORT_POWER
> > will be set again after controller set.
> >
> > What's potential issue you consider?
> >
> > Thanks,
> > Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 1/1] usb: xhci: avoid VBus glitch during controller reset operation
2019-08-23 3:33 ` Peter Chen
@ 2019-08-23 3:52 ` Ran Wang
0 siblings, 0 replies; 6+ messages in thread
From: Ran Wang @ 2019-08-23 3:52 UTC (permalink / raw)
To: Peter Chen
Cc: Mathias Nyman, mathias.nyman, linux-usb, dl-linux-imx, Ran Wang
Hi Peter,
On Friday, August 23, 2019 11:34, Peter Chen wrote:
>
> On 19-08-23 01:59:24, Ran Wang wrote:
> > HI Peter, Mathias,
> >
> > Sorry for the late review.
> >
> > On Friday, August 23, 2019 09:02, Peter Chen wrote:
> > >
> > > On 19-08-22 14:08:26, Mathias Nyman wrote:
> > > > On 21.8.2019 6.18, Peter Chen wrote:
> > > > > According to xHCI 1.1 CH4.19.4 Port Power:
> > > > > While Chip Hardware Reset or HCRST is asserted,
> > > > > the value of PP is undefined. If the xHC supports
> > > > > power switches (PPC = ‘1’) then VBus may be deasserted
> > > > > during this time. PP (and VBus) shall be enabled immediately
> > > > > upon exiting the reset condition.
> > > > >
> > > > > The VBus glitch may cause some USB devices work abnormal, we
> > > > > observe it at NXP LS1012AFWRY/LS1043ARDB/LX2160AQDS/LS1088ARDB
> platforms.
> > > To
> > > > > avoid this Vbus glitch, we could set PP as 0 before HCRST, and
> > > > > the PP will back to 1 after HCRST according to spec.
> > > >
> > > > Is this glitch causing issues already at the first xHC reset when
> > > > we are loading the xhci driver, or only later resets, like xHC reset at
> resume?
> > >
> > > The first time, Ran would you please confirm?
> >
> > My understand is this will happened whenever PP is set to 0, such as xHCI reset.
> > So I think it might also be observed during resume if xHC do reset.
>
> Yes, Vbus glitch should exists whenever we do controller reset, I thought you
> only meet this issue during boots up.
>
> >
> > However, according to my previous testing, it might be too late to do
> > this port power off in xhci_reset(), actually the VBUS will assert
> > once DWC3 driver set IP to host mode (before doing xHC reset). So the
> > glitch still can be observed on the scope; I have more issue
> > description in another patch discussion about this, please see
> >
> > lore.kernel.org/patchwork/patch/1032425/
> > Quoted from it:
> > Actually I have done experiment like what you suggested (in
> > xhci-plat.c), but the timing seems too late--making VBUS waveform look like a
> square wave as below:
> >
> > Here DWC3 enable host mode, VBUS on
> > |
> > +5V /---------\ 40ms /---------------------------....
> > 0V ________/ 90ms \______/
> > | |
> > | Here do xhci reset, VBUS back to +5V again
> > Here set all PORTSC[PP] to 0 in
> > xhci_gen_setup()
> >
> > So I am afraid the solution might have to be added in DWC3 core driver
> > where just after host mode enabling code if want fix this :(
> >
>
> Per spec 4.19.4 Port Power:
>
> Whether an xHC implementation supports port power switches or not, it shall
> automatically enable VBus on all Root Hub ports after a Chip Hardware Reset or
> HCRST.
>
> Ran's observation confirmed it, PP is asserted once xHCI is enabled.
> From the code, the HCRST will be at boots up and system resume.
> So, it seems we can't keep PP always asserted. For xHCI, to avoid Vbus glitch
> totally, we may have to control Vbus without PP (eg, configure PP pin as GPIO).
Yes, another option is to design a better VBUS driving circuit on HW side to filter this glitch out.
I have found some earlier version of LS1043ARDB board design is perfect on doing this.
Anyway, for boot, my suggestion is to do it in DWC3 driver once after enabling host mode.
But that solution is ugly, I have to admit.
And for resume, frankly I didn’t notice this before (thanks for remind), would do further survey
if can found a similar solution.
Regards,
Ran
> > > >
> > > > >
> > > > > Reported-by: Ran Wang <ran.wang_1@nxp.com>
> > > > > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > > > > ---
> > > > > drivers/usb/host/xhci.c | 15 ++++++++++++++-
> > > > > 1 file changed, 14 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > > > > index
> > > > > 6b34a573c3d9..f5a7b5d63031 100644
> > > > > --- a/drivers/usb/host/xhci.c
> > > > > +++ b/drivers/usb/host/xhci.c
> > > > > @@ -167,7 +167,8 @@ int xhci_reset(struct xhci_hcd *xhci)
> > > > > {
> > > > > u32 command;
> > > > > u32 state;
> > > > > - int ret;
> > > > > + int ret, i;
> > > > > + u32 portsc;
> > > > > state = readl(&xhci->op_regs->status); @@ -181,6 +182,18 @@
> > > > > int xhci_reset(struct xhci_hcd *xhci)
> > > > > return 0;
> > > > > }
> > > > > + /*
> > > > > + * Keep PORTSC.PP as 0 before HCRST to eliminate
> > > > > + * Vbus glitch, see CH 4.19.4.
> > > > > + */
> > > > > + for (i = 0; i < HCS_MAX_PORTS(xhci->hcs_params1); i++) {
> > > > > + __le32 __iomem *port_addr = &xhci->op_regs-
> > > >port_status_base +
> > > > > + NUM_PORT_REGS * i;
> > > > > + portsc = readl(port_addr);
> > > > > + portsc &= ~PORT_POWER;
> > > > > + writel(portsc, port_addr);
> > > >
> > > > Not all bits read from PORTSC should be written back, you might
> > > > need portsc = xhci_port_state_to_neutral(portsc) here.
> > >
> > > Will change.
> > >
> > > >
> > > > Normally I'd recommend using the xhci_set_port_power() helper
> > > > instead, but if this is is needed at driver load time then port
> > > > arrays may not be set up
> > > yet.
> > > > xhci_set_port_power() would take care of possible ACPI method
> > > > calls, and add
> > > some debugging.
> > > >
> > >
> > > It is needed at load time, so I did not use port array.
> > >
> > > > Not sure if this will impact some other platforms, maybe it would
> > > > be better to move this to a separate function, and call it before
> > > > xhci_reset() if a
> > > quirk is set.
> > > >
> > >
> > > It follows spec, not at quirk. We talked with Synopsis engineer
> > > (case: 8001235479), they confirmed this behaviour follows spec.
> > > Besides, I tried at both dwc3 and cadence3 xHCI platforms, the
> > > PORT_POWER will be set again after controller set.
> > >
> > > What's potential issue you consider?
> > >
> > > Thanks,
> > > Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-23 3:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-21 3:18 [PATCH 1/1] usb: xhci: avoid VBus glitch during controller reset operation Peter Chen
2019-08-22 11:08 ` Mathias Nyman
2019-08-23 1:01 ` Peter Chen
2019-08-23 1:59 ` Ran Wang
2019-08-23 3:33 ` Peter Chen
2019-08-23 3:52 ` Ran Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).