Re: [PATCH 3/3] omap3evm: musb: Update power capability for OMAP3EVM (Rev >= E)

[Felipe Balbi <felipe.balbi@nokia.com>]

Hi,

On Thu, Oct 29, 2009 at 06:56:03AM +0100, ext Gupta, Ajay Kumar wrote:
> Felipe,
> > -----Original Message-----
> > From: Menon, Nishanth
> > Sent: Thursday, October 29, 2009 11:06 AM
> > To: Gupta, Ajay Kumar; Gadiyar, Anand; linux-omap@vger.kernel.org
> > Cc: felipe.balbi@nokia.com; tony@atomide.com
> > Subject: RE: [PATCH 3/3] omap3evm: musb: Update power capability for
> > OMAP3EVM (Rev >= E)
> > 
> > > From: Gupta, Ajay Kumar
> > > Sent: Thursday, October 29, 2009 7:10 AM
> > > >
> > > > > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > > > > > owner@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar
> > > > > > Sent: Wednesday, October 28, 2009 5:13 PM
> > > > > >
> > > > > [...]
> > > > > > diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-
> > > omap2/usb-
> > > > > > musb.c
> > > > > > index a80441d..529e2b1 100644
> > > > > > --- a/arch/arm/mach-omap2/usb-musb.c
> > > > > > +++ b/arch/arm/mach-omap2/usb-musb.c
> > > > > > @@ -148,10 +148,14 @@ static struct platform_device musb_device =
> > {
> > > > > >
> > > > > >  void __init usb_musb_init(void)
> > > > > >  {
> > > > > > -     if (cpu_is_omap243x())
> > > > > > +     if (cpu_is_omap243x()) {
> > > > > >               musb_resources[0].start = OMAP243X_HS_BASE;
> > > > > > -     else
> > > > > > +     } else {
> > > > > Do you need {} for a one liner? Checkpatch should have warned you I
> > > > think..
> > > > >
> > > >
> > > > Er, CodingStyle says you need to do this, if the else clause
> > > > has brackets.
> > >
> > > Correct and also checkpatch didn't give any warning for this.
> > Thanks..
> > 
> > >
> > > >
> > > > > >               musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE;
> > > > > > +             /* OMAP3EVM Rev >= E can source 500mA */
> > > > > > +             if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2)
> > > > > > +                     musb_plat.power = 250;
> > > > > > +     }
> 
> Felipe,
> 
> How about moving the platform specific data (musb_hdrc_platform_data) moved to board-x.c file from usb-musb.c (same as ehci). This would avoid using omap3evm specific check in usb-musb.c files.

I was thinking on adding a musb_hdrc_board_data which would group
board-specific data such as this one.

Musb's init phase is quite messy as of today so we would need to clean
that up. Anyways, the main idea is:

board will call usb_musb_init() with a musb_hdrc_board_data * as
parameter. usb-musb would still hold a static struct
musb_hdrc_platform_data for the (in our case) OMAP-specific init, those
would be passed down to driver and init phase would be done as
following:

musb_init()
	-> musb_platform_init()
		-> musb_board_init()

we could also have board_ops and platform_ops structures for the
function pointers to be passed to musb_core.c Then all init could be
done there.

What do you say ???

-- 
balbi