Re: [RFC PATCH] usb: makefile cleanup

[matt mooney <mfm@muteddisk.com>]

On 14:01 Wed 06 Oct     , Michal Marek wrote:
> On 6.10.2010 09:51, matt mooney wrote:
> > For all modules, change <module>-objs to <module>-y; remove
> > if-statements and replace with lists using the kbuild idiom; move
> > flags to the top of the file; and fix alignment while trying to
> > maintain the original scheme in each file.
> > 
> > None of the dependencies are modified.
> > 
> > Signed-off-by: matt mooney <mfm@muteddisk.com>
> > ---
> > 
> > So here is a sample cleanup patch; I am not posting it to greg-kh or
> > the rest of the necessary usb guys yet because I would like to know
> > what you guys think first.
> 
> The elimination of conditionals in Makefiles is definitely worth it. Not
> sure about pure whitespace fixes, if the USB developers don't show
> interest, you can try pushing these through trivial@kernel.org. I have
> only one remark below:

I questioned this myself, but I concluded that whitespace fixes help with
readability (of course this is purely subjective).

> > diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile
> > index 4fd29f8..b1f79ae 100644
> > --- a/drivers/usb/musb/Makefile
> > +++ b/drivers/usb/musb/Makefile
> > [...]
> >  # the kconfig must guarantee that only one of the
> >  # possible I/O schemes will be enabled at a time ...
> > @@ -54,24 +27,17 @@ endif
> >  ifneq ($(CONFIG_MUSB_PIO_ONLY),y)
> >  
> >    ifeq ($(CONFIG_USB_INVENTRA_DMA),y)
> > -    musb_hdrc-objs		+= musbhsdma.o
> > +    musb_hdrc-y			+= musbhsdma.o
> >  
> >    else
> >      ifeq ($(CONFIG_USB_TI_CPPI_DMA),y)
> > -      musb_hdrc-objs		+= cppi_dma.o
> > +      musb_hdrc-y		+= cppi_dma.o
> >  
> >      else
> >        ifeq ($(CONFIG_USB_TUSB_OMAP_DMA),y)
> > -        musb_hdrc-objs		+= tusb6010_omap.o
> > +	musb_hdrc-y		+= tusb6010_omap.o
> >  
> >        endif
> >      endif
> >    endif
> >  endif
> 
> So this wasn't exactly elegant before and you are only changing *-objs
> to *-y. Looking at drivers/usb/musb/Kconfig, al the three USB_*_DMA
> depend on !MUSB_PIO_ONLY, so the outermost if statement can go away.
> Furthermore, the intent seems to be to only enable one of the four modes
> (MUSB_PIO_ONLY, USB_INVENTRA_DMA, USB_TI_CPPI_DMA or USB_TUSB_OMAP_DMA),
> so it is a perfect candidate for a "choice" group. Have a look e.g. at
> "PCI access mode" in arch/x86/Kconfig. Then, the Makefile part can be
> reduced to three lines without any ifs.

I thought this section was more of a Kconfig issue although I am not all that
familiar with Kconfig yet. After I look into it a little more and better
understand your advice, I will send a separate patch to fix this.

Thanks,
mfm