On Sun, 3 Nov 2013, Peter Chen wrote:
> > > diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
> > > index a06d501..bf405fd 100644
> > > --- a/drivers/usb/host/ehci-fsl.c
> > > +++ b/drivers/usb/host/ehci-fsl.c
> > > @@ -139,6 +139,7 @@ static int usb_hcd_fsl_probe(const struct hc_driver *driver,
> > > if (retval != 0)
> > > goto err4;
> > >
> > > + device_wakeup_enable(hcd->self.controller);
> > > #ifdef CONFIG_USB_OTG
> > > if (pdata->operating_mode == FSL_USB2_DR_OTG) {
> > > struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> >
> > Here and in some other places, the patch displays a bad sense of style.
> > The device_wakeup_enable() call belongs along with the other
> > hcd-related statements; it has nothing to do with CONFIG_USB_OTG.
> > Therefore you should put a blank line before the "#ifdef".
> >
> > (You could also consider removing the blank line that precedes
> > device_wakeup_enable().)
>
> So the basic code stype is: before comment and MACRO, there is
> a blank line.
No, the basic coding style is: You should prefer to put blank lines
between unrelated lines, not between related lines.
This can be stated more precisely: If there are three lines of code, A
B C, and if B is more closely related to A than to C, then they should
not be written like this:
A
B
C
because then the blank line indicates that B is more closely related to
C than to A, and people reading the code will be confused.
> But after if (), keep a blank line will let code look like clean, do
> you think so?
That's up to you. Either way is okay with me.
> For pci device, do you want hcd-pci to control wakeup setting together?
> or let each pci(xhci/ehci/ohci/uhci-pci) driver controller it?
For now, put it in hcd-pci. We can move it later if we need to.
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
