在 2021/9/29 下午10:59, Alan Stern 写道:
On Wed, Sep 29, 2021 at 06:09:27PM +0800, Yinbo Zhu wrote:
The pm states of ohci controller include UsbOperational, UsbReset, UsbSuspend
> Those aren't really PM states. The specification calls them "USB
> states".
I had replace "PM states" with "USB states" in v3 version patch
, and UsbResume. Among them, only the UsbOperational state supports launching
--^
> This comma should come directly after the word "launching", with no
> space in between.
okay, I got it.
the start frame for host controller according the ohci protocol spec, but in
S3/S4 press test procedure, it may happen that the start frame was launched
> What is the S3/S4 press test? I don't recall hearing of it before.
S3 test is that suspend to memory, S4 test is that system suspend to disk.
in other pm states and cause ohci works abnormally then kernel will allways
report rcu CallTrace. This patch was to add check for start frame in host
controller functional states for fixing above issue.
> The patch doesn't check for start of frames, that is, it doesn't check
> the INTR_SF bit in the intrstatus register. Instead it checks whether
> controller is in the UsbOperational state. And the patch also sets
> INTR_SF in the intrdisable register -- you do not mention this in the
> description.
okay, I got it, and I had made a appropriate commit changes that according to what you advice in v3 version patch.
Signed-off-by: Yinbo Zhu <zhuyinbo@xxxxxxxxxxx>
---
Change in v2:
Revise the punctuation.
drivers/usb/host/ohci-hcd.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 1f5e693..f0aeae5 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -881,6 +881,13 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd)
struct ohci_regs __iomem *regs = ohci->regs;
int ints;
+ ints = ohci_readl(ohci, ®s->control);
+
+ if ((ints & OHCI_CTRL_HCFS) != OHCI_USB_OPER) {
+ ohci_writel(ohci, OHCI_INTR_SF, ®s->intrdisable);
+ (void)ohci_readl(ohci, ®s->intrdisable);
+ }
> The driver is already supposed to prevent this problem by writing the
> OHCI_INTR_SF flag to the intrdisable register when start-of-frame
> interrupts aren't needed. Maybe what you should do is change this code
> lower down in ohci_irq():
> if ((ints & OHCI_INTR_SF) != 0 && !ohci->ed_rm_list
> && ohci->rh_state == OHCI_RH_RUNNING)
> ohci_writel (ohci, OHCI_INTR_SF, ®s->intrdisable);
> by getting rid of the test for OHCI_RH_RUNNING.
> Alan Stern
Hi Alan Stern,
Above code condition that the key point is ohci->ed_rm_list is
NULL, but my target of my patch is to disable SoF interrupt when hc isn't
UsbOperation state and it doesn't matter with that ohci->ed_rm_list
whether is NULL. In addition the ohci->rh_state is to describe root hub
state that include halt, suspend,run and it isn't exactly the same as
hc state.
following code is the v3 version patch, please you check.
ohci_work(ohci);
- if ((ints & OHCI_INTR_SF) != 0 && !ohci->ed_rm_list
- && ohci->rh_state == OHCI_RH_RUNNING)
+
+ ctl = ohci_readl(ohci, ®s->control);
+
+ if (((ints & OHCI_INTR_SF) != 0 && !ohci->ed_rm_list
+ && ohci->rh_state == OHCI_RH_RUNNING) ||
+ ((ctl & OHCI_CTRL_HCFS) != OHCI_USB_OPER)) {
ohci_writel (ohci, OHCI_INTR_SF, ®s->intrdisable);
+ (void)ohci_readl(ohci, ®s->intrdisable);
+ }
+
/* Read interrupt status (and flush pending writes). We ignore the
* optimization of checking the LSB of hcca->done_head; it doesn't
* work on all systems (edge triggering for OHCI can be a factor).
--
1.8.3.1
