| Message ID | 20200909035734.22463-1-hamish.martin@alliedtelesis.co.nz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: ohci: Default to per-port over-current protection | expand |
On Wed, 2020-09-09 at 10:05 -0400, Alan Stern wrote: > On Wed, Sep 09, 2020 at 03:57:34PM +1200, Hamish Martin wrote: > > Some integrated OHCI controller hubs do not expose all ports of the > > hub > > to pins on the SoC. In some cases the unconnected ports generate > > spurious over-current events. For example the Broadcom 56060/Ranger > > 2 SoC > > contains a nominally 3 port hub but only the first port is wired. > > > > Default behaviour for ohci-platform driver is to use global over- > > current > > protection mode (AKA "ganged"). This leads to the spurious over- > > current > > events affecting all ports in the hub. > > > > We now alter the default to use per-port over-current protection. > > > > This patch results in the following configuration changes depending > > on quirks: > > - For quirk OHCI_QUIRK_SUPERIO no changes. These systems remain set > > up > > for ganged power switching and no over-current protection. > > How about changing the quirk name to something more meaningful, such > as > OHCI_QUIRK_GANGED_POWER_NO_OVERCURRENT? I'll look at that in a separate patch later. The code I'm modifying seems to be cloned in various drivers so perhaps that needs to be centralised as the next step? See refs to OHCI_QUIRK_SUPERIO in u132- hcd.c and ftdi-elan.c. > > > - For quirk OHCI_QUIRK_AMD756 or OHCI_QUIRK_HUB_POWER power > > switching > > remains at none, while over-current protection is now guaranteed > > to be > > set to per-port rather than the previous behaviour where it was > > either > > none or global over-current protection depending on the value at > > function entry. > > Also consider renaming OHCI_QUIRK_HUB_POWER to something like > OHCI_QUIRK_PORT_POWER_ALWAYS_ON. Ditto the above comment. > > > Suggested-by: Alan Stern <stern@rowland.harvard.edu> > > Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz> > > --- > > drivers/usb/host/ohci-hcd.c | 17 +++++++++++------ > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci- > > hcd.c > > index dd37e77dae00..8ab81f6ab150 100644 > > --- a/drivers/usb/host/ohci-hcd.c > > +++ b/drivers/usb/host/ohci-hcd.c > > @@ -673,20 +673,25 @@ static int ohci_run (struct ohci_hcd *ohci) > > > > /* handle root hub init quirks ... */ > > val = roothub_a (ohci); > > - val &= ~(RH_A_PSM | RH_A_OCPM); > > + /* Configure for per-port over-current protection by default */ > > + val &= ~RH_A_NOCP; > > + val |= RH_A_OCPM; > > if (ohci->flags & OHCI_QUIRK_SUPERIO) { > > - /* NSC 87560 and maybe others */ > > + /* NSC 87560 and maybe others. > > + * Ganged power switching, no over-current protection. > > + */ > > val |= RH_A_NOCP; > > - val &= ~(RH_A_POTPGT | RH_A_NPS); > > - ohci_writel (ohci, val, &ohci->regs->roothub.a); > > + val &= ~(RH_A_POTPGT | RH_A_NPS | RH_A_PSM | > > RH_A_OCPM); > > } else if ((ohci->flags & OHCI_QUIRK_AMD756) || > > (ohci->flags & OHCI_QUIRK_HUB_POWER)) { > > /* hub power always on; required for AMD-756 and some > > - * Mac platforms. ganged overcurrent reporting, if > > any. > > + * Mac platforms. > > */ > > + val &= ~RH_A_PSM; > > val |= RH_A_NPS; > > PSM is ignored when NPS is on. You needn't bother to set it. OK, I'll remove that. > > > - ohci_writel (ohci, val, &ohci->regs->roothub.a); > > } > > + ohci_writel(ohci, val, &ohci->regs->roothub.a); > > + > > ohci_writel (ohci, RH_HS_LPSC, &ohci->regs->roothub.status); > > ohci_writel (ohci, (val & RH_A_NPS) ? 0 : RH_B_PPCM, > > &ohci->regs- > > >roothub.b); > > You didn't actually change the default: distrust_firmware is still > initialized to true. Yes. I didn't need to for my system but I understand you want to do the change. I'll make that a second patch in this series to keep the logical changes quite separate in case of regression. v2 (3?) series coming soon. Thanks, Hamish M. > > Alan Stern
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index dd37e77dae00..8ab81f6ab150 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -673,20 +673,25 @@ static int ohci_run (struct ohci_hcd *ohci) /* handle root hub init quirks ... */ val = roothub_a (ohci); - val &= ~(RH_A_PSM | RH_A_OCPM); + /* Configure for per-port over-current protection by default */ + val &= ~RH_A_NOCP; + val |= RH_A_OCPM; if (ohci->flags & OHCI_QUIRK_SUPERIO) { - /* NSC 87560 and maybe others */ + /* NSC 87560 and maybe others. + * Ganged power switching, no over-current protection. + */ val |= RH_A_NOCP; - val &= ~(RH_A_POTPGT | RH_A_NPS); - ohci_writel (ohci, val, &ohci->regs->roothub.a); + val &= ~(RH_A_POTPGT | RH_A_NPS | RH_A_PSM | RH_A_OCPM); } else if ((ohci->flags & OHCI_QUIRK_AMD756) || (ohci->flags & OHCI_QUIRK_HUB_POWER)) { /* hub power always on; required for AMD-756 and some - * Mac platforms. ganged overcurrent reporting, if any. + * Mac platforms. */ + val &= ~RH_A_PSM; val |= RH_A_NPS; - ohci_writel (ohci, val, &ohci->regs->roothub.a); } + ohci_writel(ohci, val, &ohci->regs->roothub.a); + ohci_writel (ohci, RH_HS_LPSC, &ohci->regs->roothub.status); ohci_writel (ohci, (val & RH_A_NPS) ? 0 : RH_B_PPCM, &ohci->regs->roothub.b);
Some integrated OHCI controller hubs do not expose all ports of the hub to pins on the SoC. In some cases the unconnected ports generate spurious over-current events. For example the Broadcom 56060/Ranger 2 SoC contains a nominally 3 port hub but only the first port is wired. Default behaviour for ohci-platform driver is to use global over-current protection mode (AKA "ganged"). This leads to the spurious over-current events affecting all ports in the hub. We now alter the default to use per-port over-current protection. This patch results in the following configuration changes depending on quirks: - For quirk OHCI_QUIRK_SUPERIO no changes. These systems remain set up for ganged power switching and no over-current protection. - For quirk OHCI_QUIRK_AMD756 or OHCI_QUIRK_HUB_POWER power switching remains at none, while over-current protection is now guaranteed to be set to per-port rather than the previous behaviour where it was either none or global over-current protection depending on the value at function entry. Suggested-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz> --- drivers/usb/host/ohci-hcd.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)