lkml.org 
[lkml]   [2014]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v3] USB: ehci-pci: USB host controller support for Intel Quark X1000
Date
> > /*--------------------------------------------------------------------
> > -----*/
> > +#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.

>



\
 
 \ /
  Last update: 2014-07-01 04:21    [W:0.060 / U:21.752 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site