* [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection @ 2020-09-10 21:25 Hamish Martin 2020-09-10 21:25 ` [PATCH v2 2/2] usb: ohci: Make distrust_firmware param default to false Hamish Martin ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Hamish Martin @ 2020-09-10 21:25 UTC (permalink / raw) To: stern, gregkh; +Cc: linux-usb, linux-kernel, Hamish Martin 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> --- Notes: Changes in v2: - remove clearing of RH_A_PSM in OHCI_QUIRK_HUB_POWER block. drivers/usb/host/ohci-hcd.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index dd37e77dae00..2845ea328a06 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -673,20 +673,24 @@ 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_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); -- 2.28.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] usb: ohci: Make distrust_firmware param default to false 2020-09-10 21:25 [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection Hamish Martin @ 2020-09-10 21:25 ` Hamish Martin 2020-09-11 15:17 ` Alan Stern 2020-09-11 15:17 ` [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection Alan Stern 2020-12-27 11:22 ` Paul Kocialkowski 2 siblings, 1 reply; 10+ messages in thread From: Hamish Martin @ 2020-09-10 21:25 UTC (permalink / raw) To: stern, gregkh; +Cc: linux-usb, linux-kernel, Hamish Martin The 'distrust_firmware' module parameter dates from 2004 and the USB subsystem is a lot more mature and reliable now than it was then. Alter the default to false now. Suggested-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz> --- drivers/usb/host/ohci-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index 2845ea328a06..73e13e7c2b46 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -102,7 +102,7 @@ static void io_watchdog_func(struct timer_list *t); /* Some boards misreport power switching/overcurrent */ -static bool distrust_firmware = true; +static bool distrust_firmware; module_param (distrust_firmware, bool, 0); MODULE_PARM_DESC (distrust_firmware, "true to distrust firmware power/overcurrent setup"); -- 2.28.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] usb: ohci: Make distrust_firmware param default to false 2020-09-10 21:25 ` [PATCH v2 2/2] usb: ohci: Make distrust_firmware param default to false Hamish Martin @ 2020-09-11 15:17 ` Alan Stern 0 siblings, 0 replies; 10+ messages in thread From: Alan Stern @ 2020-09-11 15:17 UTC (permalink / raw) To: Hamish Martin; +Cc: gregkh, linux-usb, linux-kernel On Fri, Sep 11, 2020 at 09:25:12AM +1200, Hamish Martin wrote: > The 'distrust_firmware' module parameter dates from 2004 and the USB > subsystem is a lot more mature and reliable now than it was then. > Alter the default to false now. > > Suggested-by: Alan Stern <stern@rowland.harvard.edu> > Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz> > --- > drivers/usb/host/ohci-hcd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c > index 2845ea328a06..73e13e7c2b46 100644 > --- a/drivers/usb/host/ohci-hcd.c > +++ b/drivers/usb/host/ohci-hcd.c > @@ -102,7 +102,7 @@ static void io_watchdog_func(struct timer_list *t); > > > /* Some boards misreport power switching/overcurrent */ > -static bool distrust_firmware = true; > +static bool distrust_firmware; > module_param (distrust_firmware, bool, 0); > MODULE_PARM_DESC (distrust_firmware, > "true to distrust firmware power/overcurrent setup"); > -- > 2.28.0 Acked-by: Alan Stern <stern@rowland.harvard.edu> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection 2020-09-10 21:25 [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection Hamish Martin 2020-09-10 21:25 ` [PATCH v2 2/2] usb: ohci: Make distrust_firmware param default to false Hamish Martin @ 2020-09-11 15:17 ` Alan Stern 2020-12-27 11:22 ` Paul Kocialkowski 2 siblings, 0 replies; 10+ messages in thread From: Alan Stern @ 2020-09-11 15:17 UTC (permalink / raw) To: Hamish Martin; +Cc: gregkh, linux-usb, linux-kernel On Fri, Sep 11, 2020 at 09:25:11AM +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. > - 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> > --- > > Notes: > Changes in v2: > - remove clearing of RH_A_PSM in OHCI_QUIRK_HUB_POWER block. > > drivers/usb/host/ohci-hcd.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c > index dd37e77dae00..2845ea328a06 100644 > --- a/drivers/usb/host/ohci-hcd.c > +++ b/drivers/usb/host/ohci-hcd.c > @@ -673,20 +673,24 @@ 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_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); > -- > 2.28.0 Acked-by: Alan Stern <stern@rowland.harvard.edu> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection 2020-09-10 21:25 [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection Hamish Martin 2020-09-10 21:25 ` [PATCH v2 2/2] usb: ohci: Make distrust_firmware param default to false Hamish Martin 2020-09-11 15:17 ` [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection Alan Stern @ 2020-12-27 11:22 ` Paul Kocialkowski 2021-01-09 21:26 ` Alan Stern 2 siblings, 1 reply; 10+ messages in thread From: Paul Kocialkowski @ 2020-12-27 11:22 UTC (permalink / raw) To: Hamish Martin; +Cc: stern, gregkh, linux-usb, linux-kernel [-- Attachment #1: Type: text/plain, Size: 4490 bytes --] Hi, On Fri 11 Sep 20, 09:25, 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 specific patch lead to breaking OHCI on my mom's laptop (whom was about to buy a new one thinking the hardware had failed). I get no OHCI interrupt at all and no USB 1 device is ever detected. I haven't really found a reasonable explanation about why that is, but here are some notes I was able to collect: - The issue showed up on 5.8,18 and 5.9.15, which don't include the patch from this series that sets distrust_firmware = false; This results in the NPS bit being set via OHCI_QUIRK_HUB_POWER. - Adding val &= ~RH_A_PSM; (as was done before this change) solves the issue which is weird because the bit is supposed to be inactive when NPS is set; - Setting ohci_hcd.distrust_firmware=0 in the cmdline results in not setting the NPS bit and also solves the issue; - The initial value of the register at function entry is 0x1001104 (PSM bit is set, NPS is unset); - The OHCI controller is the following: 00:03.0 USB controller: Silicon Integrated Systems [SiS] USB 1.1 Controller (rev 0f) (prog-if 10 [OHCI]) Subsystem: ASUSTeK Computer Inc. Device 1aa7 Does that make any sense to you? I really wonder what a proper fix could be and here are some suggestions: - Adding a specific quirk to clear the PSM bit for this hardware which seems to consider the bit regardless of NPS; - Adding the patch that sets distrust_firmware = false to stable branches; What do you think? Cheers, Paul > 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> > --- > > Notes: > Changes in v2: > - remove clearing of RH_A_PSM in OHCI_QUIRK_HUB_POWER block. > > drivers/usb/host/ohci-hcd.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c > index dd37e77dae00..2845ea328a06 100644 > --- a/drivers/usb/host/ohci-hcd.c > +++ b/drivers/usb/host/ohci-hcd.c > @@ -673,20 +673,24 @@ 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_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); > -- > 2.28.0 > -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection 2020-12-27 11:22 ` Paul Kocialkowski @ 2021-01-09 21:26 ` Alan Stern 2021-01-19 1:09 ` Hamish Martin 0 siblings, 1 reply; 10+ messages in thread From: Alan Stern @ 2021-01-09 21:26 UTC (permalink / raw) To: Paul Kocialkowski; +Cc: Hamish Martin, gregkh, linux-usb, linux-kernel On Sun, Dec 27, 2020 at 12:22:34PM +0100, Paul Kocialkowski wrote: > Hi, Sorry it has taken so long to respond to this. The holidays intervened, but that's no excuse. > On Fri 11 Sep 20, 09:25, 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 specific patch lead to breaking OHCI on my mom's laptop (whom was about > to buy a new one thinking the hardware had failed). I get no OHCI interrupt at > all and no USB 1 device is ever detected. > > I haven't really found a reasonable explanation about why that is, but here > are some notes I was able to collect: > - The issue showed up on 5.8,18 and 5.9.15, which don't include the patch > from this series that sets distrust_firmware = false; This results in the NPS > bit being set via OHCI_QUIRK_HUB_POWER. > - Adding val &= ~RH_A_PSM; (as was done before this change) solves the issue > which is weird because the bit is supposed to be inactive when NPS is set; > - Setting ohci_hcd.distrust_firmware=0 in the cmdline results in not setting > the NPS bit and also solves the issue; > - The initial value of the register at function entry is 0x1001104 (PSM bit > is set, NPS is unset); > - The OHCI controller is the following: > 00:03.0 USB controller: Silicon Integrated Systems [SiS] USB 1.1 Controller (rev 0f) (prog-if 10 [OHCI]) > Subsystem: ASUSTeK Computer Inc. Device 1aa7 Great reporting -- thanks. > Does that make any sense to you? > > I really wonder what a proper fix could be and here are some suggestions: > - Adding a specific quirk to clear the PSM bit for this hardware which seems to > consider the bit regardless of NPS; We don't need a quirk for this. There shouldn't be anything wrong with _always_ clearing PSM whenever NPS is set, since the controller is supposed to ignore PSM under that condition. Would you like to submit a patch for this? > - Adding the patch that sets distrust_firmware = false to stable branches; That's certainly reasonable. Nobody has reported any problems caused by that patch, so adding it to the stable branches should be safe enough. > What do you think? We could even do both. That would help if, for example, somebody decided to set ohci_hcd.distrust_firmware=true explicitly. Greg, in the meantime can we have commit c4005a8f65ed ("usb: ohci: Make distrust_firmware param default to false") added to all the stable kernels which have back-ported versions of commit b77d2a0a223b? Alan Stern ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection 2021-01-09 21:26 ` Alan Stern @ 2021-01-19 1:09 ` Hamish Martin 2021-01-19 15:51 ` Request to apply commit c4005a8f65ed to the -stable kernels Alan Stern 2021-01-20 10:50 ` [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection paul.kocialkowski 0 siblings, 2 replies; 10+ messages in thread From: Hamish Martin @ 2021-01-19 1:09 UTC (permalink / raw) To: stern, paul.kocialkowski; +Cc: linux-kernel, gregkh, linux-usb On Sat, 2021-01-09 at 16:26 -0500, Alan Stern wrote: > On Sun, Dec 27, 2020 at 12:22:34PM +0100, Paul Kocialkowski wrote: > > Hi, > > Sorry it has taken so long to respond to this. The holidays > intervened, > but that's no excuse. I'm sorry too, same reason/non-excuse. Thanks for your thorough report on the issue my changes caused and pass on my apologies to your Mom! > > > On Fri 11 Sep 20, 09:25, 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 specific patch lead to breaking OHCI on my mom's laptop (whom > > was about > > to buy a new one thinking the hardware had failed). I get no OHCI > > interrupt at > > all and no USB 1 device is ever detected. > > > > I haven't really found a reasonable explanation about why that is, > > but here > > are some notes I was able to collect: > > - The issue showed up on 5.8,18 and 5.9.15, which don't include the > > patch > > from this series that sets distrust_firmware = false; This > > results in the NPS > > bit being set via OHCI_QUIRK_HUB_POWER. > > - Adding val &= ~RH_A_PSM; (as was done before this change) solves > > the issue > > which is weird because the bit is supposed to be inactive when > > NPS is set; > > - Setting ohci_hcd.distrust_firmware=0 in the cmdline results in > > not setting > > the NPS bit and also solves the issue; > > - The initial value of the register at function entry is 0x1001104 > > (PSM bit > > is set, NPS is unset); > > - The OHCI controller is the following: > > 00:03.0 USB controller: Silicon Integrated Systems [SiS] USB 1.1 > > Controller (rev 0f) (prog-if 10 [OHCI]) > > Subsystem: ASUSTeK Computer Inc. Device 1aa7 > > Great reporting -- thanks. > > > Does that make any sense to you? > > > > I really wonder what a proper fix could be and here are some > > suggestions: > > - Adding a specific quirk to clear the PSM bit for this hardware > > which seems to > > consider the bit regardless of NPS; > > We don't need a quirk for this. There shouldn't be anything wrong > with > _always_ clearing PSM whenever NPS is set, since the controller is > supposed to ignore PSM under that condition. > > Would you like to submit a patch for this? Yes, I think that looks reasonable too. > > > - Adding the patch that sets distrust_firmware = false to stable > > branches; > > That's certainly reasonable. Nobody has reported any problems caused > by > that patch, so adding it to the stable branches should be safe > enough. > Yes, that is probably a good idea. I've carried both patches locally for my systems. > > What do you think? > > We could even do both. That would help if, for example, somebody > decided to set ohci_hcd.distrust_firmware=true explicitly. I think both might be best. > > Greg, in the meantime can we have commit c4005a8f65ed ("usb: ohci: > Make > distrust_firmware param default to false") added to all the stable > kernels which have back-ported versions of commit b77d2a0a223b? > > Alan Stern I second that. Thanks, Hamish Martin ^ permalink raw reply [flat|nested] 10+ messages in thread
* Request to apply commit c4005a8f65ed to the -stable kernels 2021-01-19 1:09 ` Hamish Martin @ 2021-01-19 15:51 ` Alan Stern 2021-01-19 18:11 ` Greg KH 2021-01-20 10:50 ` [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection paul.kocialkowski 1 sibling, 1 reply; 10+ messages in thread From: Alan Stern @ 2021-01-19 15:51 UTC (permalink / raw) To: Greg KH; +Cc: Hamish Martin, paul.kocialkowski, stable, linux-usb Greg: Hamish Martin has reported a problem caused by applying commit b77d2a0a223b ("usb: ohci: Default to per-port over-current protection") to the -stable kernel series without also applying commit c4005a8f65ed ("usb: ohci: Make distrust_firmware param default to false"). Can we please queue up commit c4005a8f65ed for all the -stable kernel branches which already merged versions of commit b77d2a0a223b? Thanks, Alan Stern ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Request to apply commit c4005a8f65ed to the -stable kernels 2021-01-19 15:51 ` Request to apply commit c4005a8f65ed to the -stable kernels Alan Stern @ 2021-01-19 18:11 ` Greg KH 0 siblings, 0 replies; 10+ messages in thread From: Greg KH @ 2021-01-19 18:11 UTC (permalink / raw) To: Alan Stern; +Cc: Hamish Martin, paul.kocialkowski, stable, linux-usb On Tue, Jan 19, 2021 at 10:51:45AM -0500, Alan Stern wrote: > Greg: > > Hamish Martin has reported a problem caused by applying commit > b77d2a0a223b ("usb: ohci: Default to per-port over-current protection") > to the -stable kernel series without also applying commit c4005a8f65ed > ("usb: ohci: Make distrust_firmware param default to false"). > > Can we please queue up commit c4005a8f65ed for all the -stable kernel > branches which already merged versions of commit b77d2a0a223b? Now queued up, thanks. greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection 2021-01-19 1:09 ` Hamish Martin 2021-01-19 15:51 ` Request to apply commit c4005a8f65ed to the -stable kernels Alan Stern @ 2021-01-20 10:50 ` paul.kocialkowski 1 sibling, 0 replies; 10+ messages in thread From: paul.kocialkowski @ 2021-01-20 10:50 UTC (permalink / raw) To: Hamish Martin; +Cc: stern, linux-kernel, gregkh, linux-usb [-- Attachment #1: Type: text/plain, Size: 4264 bytes --] Hi, On Tue 19 Jan 21, 01:09, Hamish Martin wrote: > On Sat, 2021-01-09 at 16:26 -0500, Alan Stern wrote: > > On Sun, Dec 27, 2020 at 12:22:34PM +0100, Paul Kocialkowski wrote: > > > Hi, > > > > Sorry it has taken so long to respond to this. The holidays > > intervened, > > but that's no excuse. > I'm sorry too, same reason/non-excuse. Thanks for your thorough report > on the issue my changes caused and pass on my apologies to your Mom! Aaaand sorry for the delay here as well, I've been busy with other things lately. No problem at all :) > > > On Fri 11 Sep 20, 09:25, 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 specific patch lead to breaking OHCI on my mom's laptop (whom > > > was about > > > to buy a new one thinking the hardware had failed). I get no OHCI > > > interrupt at > > > all and no USB 1 device is ever detected. > > > > > > I haven't really found a reasonable explanation about why that is, > > > but here > > > are some notes I was able to collect: > > > - The issue showed up on 5.8,18 and 5.9.15, which don't include the > > > patch > > > from this series that sets distrust_firmware = false; This > > > results in the NPS > > > bit being set via OHCI_QUIRK_HUB_POWER. > > > - Adding val &= ~RH_A_PSM; (as was done before this change) solves > > > the issue > > > which is weird because the bit is supposed to be inactive when > > > NPS is set; > > > - Setting ohci_hcd.distrust_firmware=0 in the cmdline results in > > > not setting > > > the NPS bit and also solves the issue; > > > - The initial value of the register at function entry is 0x1001104 > > > (PSM bit > > > is set, NPS is unset); > > > - The OHCI controller is the following: > > > 00:03.0 USB controller: Silicon Integrated Systems [SiS] USB 1.1 > > > Controller (rev 0f) (prog-if 10 [OHCI]) > > > Subsystem: ASUSTeK Computer Inc. Device 1aa7 > > > > Great reporting -- thanks. > > > > > Does that make any sense to you? > > > > > > I really wonder what a proper fix could be and here are some > > > suggestions: > > > - Adding a specific quirk to clear the PSM bit for this hardware > > > which seems to > > > consider the bit regardless of NPS; > > > > We don't need a quirk for this. There shouldn't be anything wrong > > with > > _always_ clearing PSM whenever NPS is set, since the controller is > > supposed to ignore PSM under that condition. > > > > Would you like to submit a patch for this? > Yes, I think that looks reasonable too. Agreed, I'll craft a patch in this direction and have you CC-ed. > > > - Adding the patch that sets distrust_firmware = false to stable > > > branches; > > > > That's certainly reasonable. Nobody has reported any problems caused > > by > > that patch, so adding it to the stable branches should be safe > > enough. > > > Yes, that is probably a good idea. I've carried both patches locally > for my systems. Thanks for requesting it :) Cheers, Paul > > > What do you think? > > > > We could even do both. That would help if, for example, somebody > > decided to set ohci_hcd.distrust_firmware=true explicitly. > I think both might be best. > > > > > Greg, in the meantime can we have commit c4005a8f65ed ("usb: ohci: > > Make > > distrust_firmware param default to false") added to all the stable > > kernels which have back-ported versions of commit b77d2a0a223b? > > > > Alan Stern > I second that. > > Thanks, > Hamish Martin > -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-01-20 11:29 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-10 21:25 [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection Hamish Martin 2020-09-10 21:25 ` [PATCH v2 2/2] usb: ohci: Make distrust_firmware param default to false Hamish Martin 2020-09-11 15:17 ` Alan Stern 2020-09-11 15:17 ` [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection Alan Stern 2020-12-27 11:22 ` Paul Kocialkowski 2021-01-09 21:26 ` Alan Stern 2021-01-19 1:09 ` Hamish Martin 2021-01-19 15:51 ` Request to apply commit c4005a8f65ed to the -stable kernels Alan Stern 2021-01-19 18:11 ` Greg KH 2021-01-20 10:50 ` [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection paul.kocialkowski
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).