> > /*--------------------------------------------------------------------
> > -----*/
> > +#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC 0x0939
> > +static inline bool is_intel_quark_x1000(struct pci_dev *pdev) {
> > + return pdev->vendor == PCI_VENDOR_ID_INTEL &&
> > + pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
> > +}
>
> Why not just put this check inline into ehci_pci_reinit()?
Alan Stern said it is not a problem, I think so also since it just a inline subroutine.
>
> > +
> > +/*
> > + * The offset of in/out threshold register is 0x84.
> > + * And it is the register of 'hostpc'
> > + * in memory-mapped EHCI controller.
> > +*/
>
> The preferred multi-line kernel style is this:
>
> /*
> * bla
> * bla
> */
I will improve it.
> > +#define intel_quark_x1000_insnreg01 hostpc
> > +
> > +/* The maximal ehci packet buffer size is 512 bytes */
>
> s/ehci/EHCI/
>
> > +#define INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE 512
> > +
> > +/* The threshold value set the register is in DWORD */
> > +#define INTEL_QUARK_X1000_EHCI_THRESHOLD(size) ((size)/4u)
> > +#define INTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT 16
> > +#define INTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT 0
> > +
> >
>
> Too many empty lines...
>
> > /* called after powerup, by probe or system-pm "wakeup" */
> > static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
> > {
> > int retval;
> > + u32 val;
> > + u32 thr;
>
> Why not declare these where they are used?
All will be removed as Alan Stern's suggestion.
> > + /* Reset the threshold limit */
> > + if (is_intel_quark_x1000(pdev)) {
> > + /*
> > + * In order to support the isochronous/interrupt
> > + * transactions, 508 bytes should be used as
> > + * max threshold values to maximize the
> > + * performance
> > + */
>
> Same comment about the comment style...
>
> > + thr = INTEL_QUARK_X1000_EHCI_THRESHOLD(
> > + INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE - 4
> > + );
> > + val = thr<<INTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT |
> > + thr<<INTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT;
>
> Please surround << with spaces for consistency.
The above code will be removed as Alan Stern's suggestion.
>
--
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
