From: Alan Stern <stern@rowland.harvard.edu>
To: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cc: Hamish Martin <hamish.martin@alliedtelesis.co.nz>,
gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] usb: ohci: Default to per-port over-current protection
Date: Sat, 9 Jan 2021 16:26:08 -0500 [thread overview]
Message-ID: <20210109212608.GB1136657@rowland.harvard.edu> (raw)
In-Reply-To: <X+huemxT9XOeDi5E@aptenodytes>
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
next prev parent reply other threads:[~2021-01-09 21:26 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210109212608.GB1136657@rowland.harvard.edu \
--to=stern@rowland.harvard.edu \
--cc=gregkh@linuxfoundation.org \
--cc=hamish.martin@alliedtelesis.co.nz \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=paul.kocialkowski@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.