On Sat, 24 May 2014, Dr. Werner Fink wrote:
> On Wed, May 14, 2014 at 10:32:21AM -0400, Alan Stern wrote:
> > On Tue, 13 May 2014, Dr. Werner Fink wrote:
> >
> > > On Tue, May 13, 2014 at 06:10:46PM +0200, Dr. Werner Fink wrote:
> > > > On Tue, May 13, 2014 at 11:59:38AM -0400, Alan Stern wrote:
> > > > >
> > > > > What about something like this instead?
> > > > >
> > > > > masked_status = status & (INTR_MASK | STS_FLR);
> > > > >
> > > > > /* Shared IRQ? */
> > > > > if (!masked_status || unlikely(ehci->rh_state == EHCI_RH_HALTED)) {
> > > > > + if (status & STS_RECL) {
> > > > > + ehci_writel(ehci, STS_RECL, &ehci->regs->status);
> > > > > + ehci_readl(ehci, &ehci->regs->status);
> > > > > + }
> > > > > spin_unlock_irqrestore(&ehci->lock, flags);
> > > > > return IRQ_NONE;
> > > > > }
> > > > >
> > > > > That should do about the same thing as your change, except for
> > > > > returning IRQ_NONE rather than IRQ_HANDLED.
> > > >
> > > > I'll forward this to my home to try it out. It will take some time
> > > > as my wife is also working on this system. Nevertheless I'm curious
> > > > about it.
> > >
> > > The code is active now:
> >
> > ...
> >
> > > it works.
> >
> > Does it still work if you omit the ehci_readl() call?
> >
> > If you want to submit a patch containing this fix, please add a quirk
> > flag for it. And a comment explaining why the ehci_writel() call is
> > needed.
>
> During debugging and rebooting I've seen the problem again:
What patch were you using when you saw the problem again?
> Sidemark ... I've tried to check the Vendor/Product indentifier but run into
> the problem that I'd like to see the hrdware and not the driver vendor and product. That is I'd like to check for the added lines:
>
> drivers/usb/core/quirks.c:
>
> /* INTEL Hub */
> { USB_DEVICE(0x8087, 0x0024), .driver_info = USB_QUIRK_STATUS_RECL },
>
> include/linux/usb/quirks.h:
>
> /* device may cause the STS_RECL status register bit in IRQ if it is
> not used. This is a readonly bit, which is used to detect an empty
> asynchronous schedule. */
> #define USB_QUIRK_STATUS_RECL 0x00000080
The quirk information does not belong in those files.
drivers/usb/core/quirks.c and include/linux/usb/quirks.h are for USB
devices, not USB host controllers.
PCI vendor and device IDs are stored in include/linux/pci_ids.h. But
if you're going to use the ID in only one place (which should be
drivers/usb/host/ehci-pci.c), there's no reason to add it to the header
file. The quirk flag should be added to drivers/usb/host/ehci.h.
> in drivers/usb/host/ehci-hcd.c : ehci_irq() where I've added
>
> struct usb_device *hub;
>
> hub = hcd->self.root_hub;
> if (hub->quirks & USB_QUIRK_STATUS_RECL)
> printk_once(KERN_WARNING FW_BUG "QUIRK: INTEL hub found!");
> printk_once(KERN_WARNING FW_BUG "EHCI Hub: Vendor %x Product %x",
> le16_to_cpu(hub->descriptor.idVendor),
> le16_to_cpu(hub->descriptor.idProduct));
>
> but then seen the Linux Foundation as Vendor and not the hardware hub.
Don't print this information in ehci_irq(); print it in ehci-pci.c.
And by the way, the problem is in the Intel host controller, not in an
Intel hub.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
