Re: [PATCH 1/3] ARM: OMAP: Add support for USB on OMAP34XX

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> > > > > +#if defined(CONFIG_USB_MUSB_HDRC) || defined(CONFIG_USB_MUSB_HDRC_MODULE)
> > > >
> > > > What about #ifdef CONFIG_USB_MUSB_SOC
> > > >
> > >
> > > How is this relevant? Could you please elaborate?
> >
> > #if defined(*) || defined(*_MODULE) looks a bit awkward.
> > CONFIG_USB_MUSB_SOC is a boolean macro and is true in 2430 and 34XX.
> >
> > Anybody has any comments on this ??
>
> I might end up doing this in other places as well.
> Specifically, when adding EHCI, I might have something along the lines of
>
> #if defined (CONFIG_USB_EHCI_HCD) || defined (CONFIG_USB_EHCI_HCD_MODULE)
>
> Besides, it seems to be common usage anywhere you have something that can
> be built as a module.

It's common usage only when it's the best solution available.

When there's a single boolean symbol available (as in this
case), the simple #ifdef seems clearly better....


> > > > > +extern void __init sdp3430_usb_init(void);
> > > >
> > > > This extern will look better in
> > > > include/asm-arm/arch-omap/board-3430sdp.h
> > >
> > > Yes. But there are other .c files in arch/arm/mach-omap2
> > > that declare externs.
> > > Specifically, board-2430sdp.c also does it this way. I
> > > thought it would be a good idea to retain the same structure.
> > >
> > > I can move it to the .h file if needed. What do you think?
> >
> > Don't know about the others, but I'd prefer seeing this
> > prototype in the header file.
>
> Ok. No problem moving it to the .h file. Anyone else thinks this way?

Yes.  Don't perpetuate bad idioms.  I'm fairly convinced this
one happens out of laziness ... I know that's been the reason
I've done it on occasion, just to avoid a larger rebuild.

Prototypes should be in header files so that there is only one
copy, so that when the signature changes it's easier to catch
all the code that relies on the old version.


> Also what about the other externs in board*.c files?

They're all bugs that should be fixed at a convenient time.
Sooner is better, so fewer folk are tempted to repeat these goofs.

- Dave

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

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux