RE: [PATCH] USB: ehci-pci: USB host controller support for Intel Quark X1000

From: Chen, Alvin
Date: Thu Jun 26 2014 - 01:24:48 EST


> > This patch is to enable USB host controller for Intel Quark X1000. Add
>> pci quirks to adjust the packet buffer in/out threshold value, and
> >ensure EHCI packet buffer i/o threshold value is reconfigured to half
>
>
> What is the packet buffer in/out threshold value and why does it need to be
> reconfigured to half?
>
I go through the hardware specification carefully again. The EHCI buffer packet depth can be 512 bytes, and the in/out threshold is programmable and can be programmed to 512 bytes (0x80 DWord) only when isochronous/interrupt transactions are not initiated by the host controller. The default threshold value for Quark X1000 is 0x20 DWord, so this patch try to program it as maximal as possible, and it is 0x7F Dword since the host controller initiates isochronous/interrupt transactions .

I will update the description in PATCH v2.

> > + val = readl(op_reg_base + EHCI_INSNREG01);
> > + dev_printk(KERN_INFO, &pdev->dev, "INSNREG01 is 0x%08x\n", val);
> > +
> > + val = EHCI_INSNREG01_THRESH;
> > +
> > + writel(val, op_reg_base + EHCI_INSNREG01);
> > +
> > + val = readl(op_reg_base + EHCI_INSNREG01);
> > + dev_printk(KERN_INFO, &pdev->dev, "INSNREG01 is 0x%08x\n", val);
>
> What good will these log messages do anybody? Is there any reason not to
> make them debug messages? Or even leave them out entirely, since you
> pretty much know beforehand what they're going to say?
>
These messages are not necessary, I will remove them.

> > +
> > + iounmap(base);
> > +
> > +}
> > +EXPORT_SYMBOL_GPL(usb_set_qrk_bulk_thresh);
>
> None of this material belongs in pci-quirks.c. Please move it into ehci-pci.c.
>

OK. I will move them to ehci-pci.c, since these two functions will not be called by other modules.

> Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/