Re: [PATCH][4/4] usb: use single ohci_start()

On Jan 16, 2008 12:52 PM, David Brownell <[email protected]> wrote:
> On Sunday 13 January 2008, Magnus Damm wrote:
> > usb: use single ohci_start()
> >
> > This patch adds adds an ohci_start() function that easily can be shared
> > between many ohci drivers. This patch only changes drivers to ohci_start()
> > when the driver specific implementation exactly match ohci_start(). It also
> > seems popular to sometimes do ohci_init() in .reset instead of .start, and if
> > those drivers can be rewritten then it may be possible to use ohci_start()
> > in even more drivers.
>
> Actually, the drivers that you made use ohci_start() are the problem ones...
> you might get a hint of that because the PCI glue isn't structured that way,
> yet that code gets bugfixed quicker than anything else.
>
> Thing is, the reset() entry point is where all the one-time init *should*
> happen, and that includes calling ohci_init().  Its name is a historical
> goof; it doesn't just reset the hardware any more, that's a side effect.
>
> And the start() entry point is what should be moving from the initialized
> but inactive state into the operational state -- what ohci_run() does,
> and maybe a bit more.
>
> Whereas, ohci_start() idioms are calling ohci_init() *AND* ohci_run(), but
> they should really only be doing the latter as start() entry points.
>
> The situation got bad because of lots of copy'n'paste that was mostly
> harmless on embedded hardware that often didn't suffer from the stranger
> nuances of HCD lifecycle.  And because some ancient 2.2/2.4 init sequence
> code didn't get overhauled for a very long time.  (In fact it's still not
> exactly done...)
>
> So, NAK on this one, sorry.  The idea of code reuse is great, but this
> is the wrong code to re-use.

No worries, I understand your point. So what about the following:

Move ohci_hcd_init() into ohci_init():
ohci_hcd_init() seems to be called at probe time today - with the
exception of ohci-pci.c, ohci-ps3.c and ohci-ssb.c. (They are the ones
that do the right thing, right?) It looks like calling ohci_hcd_init()
 from ohci_init() should be ok.

Connect ohci_init() to .reset:
Just change the argument to struct usb_hcd * and connect it to the
driver callbacks.

Connect ohci_start() to .start:
Rename ohci_run() to ohci_start() and change the argument. Connect it
to the driver callbacks.

I'd be happy to hack up the above code if you think it's a good idea...

Thanks for the feedback!

/ magnus

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel