On Mon, Aug 25, 2014 at 05:10:55PM -0300, Rodrigo Severo wrote:
> On Mon, Aug 25, 2014 at 3:17 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, Aug 22, 2014 at 12:33:10PM -0300, Rodrigo Severo wrote:
> > Renesas uPD720202 USB 3.0 chip needs XHCI_TRUST_TX_LENGTH quirk
> workaround as per below logs
> > produced when using a Diammond video capture dongle:
> >
> > Aug 21 18:07:33 [kernel] handle_tx_event: 67 callbacks suppressed
> > Aug 21 18:07:33 [kernel] xhci_hcd 0000:01:00.0: WARN Successful
> completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk?
> > - Last output repeated 9 times -
> >
> > While at it I took the opportunity to define Renesas uPD720202 device ID.
> >
> > Signed-off-by: Rodrigo Severo <rodrigo@xxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/usb/host/xhci-pci.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > index 47390e3..52df456 100644
> > --- a/drivers/usb/host/xhci-pci.c
> > +++ b/drivers/usb/host/xhci-pci.c
> > @@ -38,6 +38,8 @@
> > #define PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI 0x8c31
> > #define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
> >
> > +#define PCI_DEVICE_ID_RENESAS_UPD720202 0x0015
>
> Minor nit, can you use a tab to line up the value properly?
>
>
> Will do. If I just apply a single tab I can't see much aliging happening.
> Should I apply more tabs?
Do what you need to do to make it line up with the lines above it :)
> Also, please use scripts/get_maintainer.pl to send the patch to the
> proper person, I don't know if the xhci maintainer saw this patch :(
>
>
> That would be a nice reason for the long silence. Will do it. I didn't know
> about scripts/get_maintainer.pl
>
> Thanks for the heads up.
>
>
> > +
> > static const char hcd_name[] = "xhci_hcd";
> >
> > /* called after powerup, by probe or system-pm "wakeup" */
> > @@ -143,10 +145,12 @@ static void xhci_pci_quirks(struct device *dev,
> struct xhci_hcd *xhci)
> > xhci->quirks |= XHCI_TRUST_TX_LENGTH;
> > }
> > if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
> > - pdev->device == 0x0015 &&
> > - pdev->subsystem_vendor == PCI_VENDOR_ID_SAMSUNG &&
> > - pdev->subsystem_device == 0xc0cd)
> > - xhci->quirks |= XHCI_RESET_ON_RESUME;
> > + pdev->device == PCI_DEVICE_ID_RENESAS_UPD720202) {
> > + xhci->quirks |= XHCI_TRUST_TX_LENGTH;
>
> You just added this quirk to devices that previously didn't seem to need
> it, why?
>
>
> Because I got the "WARN Successful completion on short TX: needs
> XHCI_TRUST_TX_LENGTH quirk?" kernel message when using a Video Grabber
> connected to a Renesas USB PCI-e board.
>
> As far as I could understand from my research that's the proper way to deal
> with this situation. Or is there a better course of action?
I'll let the XHCI maintainer speak up here, he would know better than I.
> > + if (pdev->subsystem_vendor == PCI_VENDOR_ID_SAMSUNG &&
> > + pdev->subsystem_device == 0xc0cd)
> > + xhci->quirks |= XHCI_RESET_ON_RESUME;
>
> Can't we just get a table of quirks instead of logic functions to make
> this easier to add new ones?
>
>
> Such a change might be just out of my reach but I can try to do it if you could
> point me to some other code that uses a table as you suggest.
For this patch, it's probably not needed, but for future ones, I'd
recommend it if at all possible. We have "quirk tables" for other
devices in drivers, shouldn't be that hard to find some examples.
> By the way, which git repository should I use as base for my work? I'm
> currently using https://kernel.googlesource.com/pub/scm/linux/kernel/git/mnyman
> /xhci/
>
> Is this the right one?
As that is the XHCI's maintainer's tree, yes, that's probably the best.
thanks,
greg k-h
--
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
