Re: [PATCH 06/20] usb: hcd-pci: introduce pm-ops for platform-pci devs

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

 



Hi,

On Wed, Aug 31, 2011 at 04:46:58PM -0400, Alan Stern wrote:
> > > > While it might possible to merge them. I think we should take smaller
> > > > steps at a time. Those files are already there anyway. So at first we
> > > > find a way to clean up the ifdeferry and allow all of those to be built
> > > > on all archs (it's definitely possible)
> > > 
> > > Just how did you want all these different drivers to be built?  You
> > > said you don't want a separate module containing an entire copy of the
> > > ehci core for each arch; you want the core to be shared.  That means
> > > one of two things:
> > > 
> > > 	Build a single monolithic module containing all the drivers.
> > > 	This is quite likely to require a large bunch of #ifdefs.
> > > 
> > > 	Build a minimal driver module for each arch, and have the ehci
> > > 	core in a single separate module that gets loaded for
> > > 	dependency resolution.  This is likely to require a bunch of
> > > 	exported symbols.
> > > 
> > > Which approach are you advocating?
> > 
> > let udev do its thing:
> > 
> > - arch code will add a platform_device to the driver model, e.g.:
> >   platform_device_register(&omap_ehci_device);
> > 
> > - based on the device add uevent, udev will load the correct module (in
> >   this case ehci-omap.ko)
> > 
> > - ehci-omap.ko instantiates and add EHCI-core platform_device:
> >   ehci = platform_device_alloc("ehci", -1);
> >   platform_device_add(ehci);
> > 
> > - based on the device add uevent, udev will load the correct module (in
> >   this case ehci-hcd.ko)
> > 
> > So, there aren't only two approaches for this, can't you see ? There's
> > no need for exported symbols, or a monolithic driver.
> 
> Okay, I completely misunderstood what you wanted to do.  Now I've got 
> it.  I didn't realize that you wanted to add a new platform device 
> child beneath the arch-specific platform device.
> 
> I'm willing to go along with the idea of adding an extra device layer
> between the controller and the root hub.  But I'm not so keen about
> making it a platform_device.  What we need here is a general usb_host
> device that can be a new device_type on the USB bus, like we discussed 
> earlier.

I don't think this will be a good approach. You see, this will affect
the non-standard hosts too.

Hosts like MUSB are completely non-standard, so they must provide their
own hc_driver. But they talk directly to usbcore.

In the case of the standard hosts what we have is, for example:

usbcore <--> EHCI Stack <--> Architecture

What I'm proposing is to split the EHCI stack to its own
platform_driver/device. If you add a struct usb_host kinda thing, that
will affect MUSB too:

- usbcore <--> MUSB <--> Architcture
+ usbcore <--> usb_host <--> MUSB <--> architecture

Such change should not affect the non-standard host. I believe you still
didn't understand the problem which I'm trying to solve. It's not about
how EHCI talks to usbcore, it's how architecture talks to EHCI.

Currently, we have lots of duplicated code on all archs (hc_driver,
reuse of ehci_* symbols, allocation of usb_hcd + ehci structures, etc
etc). And what I want is to pack all things which are mandated by the
EHCI specification into its own self-sufficient driver. Then
architecture code, handles only architecture-specific things.

Like I have said before, no matter if it's PCI, OMAP, Tegra or whatever,
the EHCI specification remains the same. Meaning that the way you
enqueue an urb (except on quirky HW) is the same throughout all
IPs. Similarly, the way you suspend/resume the bus, control a port, get
a frame number, enable/disable an endpoint and so on is the same thing.
So if you know those things are all the same, why do you even bother
exporting that symbol ? It will be reused as is by most implementations.

I really don't understand your reluctance to add a
platform_driver/device to ehci-hcd. I guess you really don't see the
benefits. It's not only about adding "static" to a bunch of functions,
it will also allow for a object-oriented approach, where Architecture
code doesn't need to know about EHCI and EHCI-code doesn't need to know
about architecture details, it will also allow for re-factoring of bus
suspend/resume (we could probably implement those with pm_runtime; put
bus suspend on ->runtime_suspend and bus resume on ->runtime_resume).
There are other benefits which I could describe but, again as I said
before, those can be seen from MUSB (a bit) and DWC3 drivers.

On top of all that, you add the fact the HW _does__have_ two entities in
order to build a functional EHCI controller (one entity will pack EHCI
rules and the other entity will pack integration context - be it PCI or
anything else). In short we _do_ have two "devices" which we could be
modelling in a better way, IMHO.

> Ideally this could be done in a way that allows relatively easy
> conversion of the existing [eoux]hci drivers while also allowing all
> the other special-purpose host controller drivers to continue as they
> are with only minimal changes.

there should be no changes needed on the non-standard hosts.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux