[PATCH] USB: gadget driver for LPC32xx

Arnd Bergmann arnd at arndb.de
Tue Mar 20 09:01:49 EDT 2012


On Monday 19 March 2012, Roland Stigge wrote:
> Hi Arnd,
> 
> On 19/03/12 22:30, Arnd Bergmann wrote:
> > There is already a driver for the isp1301 otg part in the kernel, I don't
> > think we want to add another one.
> > 
> > From what I can tell, this shares a common ancestry with the omap version
> > but has diverged quite a bit. The best solution would really be to
> > bring the two back together and let them share a common base driver,
> > with the lpc32xx and omap specific bits in another file.
> 
> Yes, it's a good idea to share code where possible and consolidate into
> one driver.
> 
> Please consider:
> 
> The LPC32xx driver is actually using only 3 functions via isp1301:
> 
> isp1301_udc_configure()
> isp1301_set_powerstate()
> isp1301_pullup_set()
> 
> The first of those is LPC32xx specific. The power setting function is
> also done differently in isp1301_omap's power_up()/power_down() ("board
> specific"). For the pullups, there is not (yet?) a dedicated API in the
> OMAP driver, but it's really only two small I2C commands.
> 
> Are you still sure it's worth it to use a common driver when there is
> hardly shared code?

No, I guess I should have looked closed. I found a few common functions
and assumed that this would be the same driver.

> Maybe the right thing is a common low-level isp1301 interface defining
> all the registers and providing low-level (I2C) access functions,
> leaving all the "higher level"/"board specific" functions up to the
> existing drivers?
> 
> (I guess you meant drivers/usb/otg/isp1301_omap.c ?)

Yes, that sounds like a very good idea. Can you describe which part of
the function isp1301 does? It's not clear how the layering between
that and the platform driver should end up.

	Arnd



More information about the linux-arm-kernel mailing list