| Message ID | 20210518111640.243559-1-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xhci: State explicitly when the controller is inaccessible | expand |
On 18.5.2021 14.16, Kai-Heng Feng wrote: > Sometimes the dmesg says "Controller not ready at resume" because CNR is > flagged. But what actually happens is that the whole USBSTS becomes > inaccessible, and the reason could be disabled PCI I/O space or faulty > firmware/hardware. > > So state the reason explicitly to make the message more clear. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/usb/host/xhci.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index ca9385d22f68..0e6fbe1f4fcc 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1117,8 +1117,9 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > retval = xhci_handshake(&xhci->op_regs->status, > STS_CNR, 0, 10 * 1000 * 1000); > if (retval) { > - xhci_warn(xhci, "Controller not ready at resume %d\n", > - retval); > + xhci_warn(xhci, "Controller is %s at resume %d\n", > + retval == -ENODEV ? "inaccessible" : > + "not ready", retval); Old way did print out retval, and was greppable. Not sure this is an improvement -Mathias
On Tue, May 18, 2021 at 8:19 PM Mathias Nyman <mathias.nyman@linux.intel.com> wrote: > > On 18.5.2021 14.16, Kai-Heng Feng wrote: > > Sometimes the dmesg says "Controller not ready at resume" because CNR is > > flagged. But what actually happens is that the whole USBSTS becomes > > inaccessible, and the reason could be disabled PCI I/O space or faulty > > firmware/hardware. > > > > So state the reason explicitly to make the message more clear. > > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > --- > > drivers/usb/host/xhci.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > > index ca9385d22f68..0e6fbe1f4fcc 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -1117,8 +1117,9 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > > retval = xhci_handshake(&xhci->op_regs->status, > > STS_CNR, 0, 10 * 1000 * 1000); > > if (retval) { > > - xhci_warn(xhci, "Controller not ready at resume %d\n", > > - retval); > > + xhci_warn(xhci, "Controller is %s at resume %d\n", > > + retval == -ENODEV ? "inaccessible" : > > + "not ready", retval); > > Old way did print out retval, and was greppable. > Not sure this is an improvement That's true. Maybe it's just me, but I can't directly mapped the value to -ENODEV at first glance. But in essence this is just a cosmetic change. Kai-Heng > > -Mathias
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index ca9385d22f68..0e6fbe1f4fcc 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1117,8 +1117,9 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) retval = xhci_handshake(&xhci->op_regs->status, STS_CNR, 0, 10 * 1000 * 1000); if (retval) { - xhci_warn(xhci, "Controller not ready at resume %d\n", - retval); + xhci_warn(xhci, "Controller is %s at resume %d\n", + retval == -ENODEV ? "inaccessible" : + "not ready", retval); spin_unlock_irq(&xhci->lock); return retval; }
Sometimes the dmesg says "Controller not ready at resume" because CNR is flagged. But what actually happens is that the whole USBSTS becomes inaccessible, and the reason could be disabled PCI I/O space or faulty firmware/hardware. So state the reason explicitly to make the message more clear. Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/usb/host/xhci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)