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]

 



On Thu, 1 Sep 2011, Felipe Balbi wrote:

> > 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.

To some extent, yes.  I don't see anything wrong with that.  You're
proposing a significant change to part of the USB stack; it ought to
affect the existing drivers.

> 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

Your diagram is not a good description of the existing stack.  We
_already_ have an intermediate glue layer; the true picture before and
after looks like this:

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

The main differences are renaming the structure and embedding a struct
device within what used to called struct usb_hcd.

What you're talking about, by contrast, would look like this (using 
OMAP as an example):

  usbcore <--> usb_hcd <--> MUSB <--> architecture
- usbcore <--> usb_hcd <--> ehci_omap <--> architecture
+ usbcore <--> usb_hcd <--> ehci_platform <--> ehci_omap <--> architecture

This would be both inconsistent and more complicated.

> Such change should not affect the non-standard host.

Why not?  A host controller driver is a host controller driver, whether
it obeys some standard or not.  In the non-standard case, where there
is a one-to-one relationship between the hardware driver and the
usb_host driver (they would both belong to the same module), we can
directly specify the usb_host's driver at the time it is registered
instead of relying on the driver core to probe for the appropriate 
driver.  But that's a minor difference.

>  I believe you still
> didn't understand the problem which I'm trying to solve.

Partly this is because you've never written a single, complete
description of the problem.  In various emails you have described
various parts of it, but there was never a full picture including all
the details and their relative importance.

>  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.

Even here you haven't given a complete description of what you want.  I 
know this because earlier you said that you also want to be able to 
build all the platform drivers in a single distribution.

Now, there's nothing wrong with any of this.  I agree with your goals
(insofar as I am aware of them).  But none of what you're saying is
inconsistent with expanding the usb_hcd structure into its own device
layer.

> 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'm not against adding another driver layer to ehci-hcd; I'm just
against making it a platform_driver.  The platform bus is a sort of
catch-all for devices that don't have any suitable bus of their own.  
But we do already have a perfectly good bus for these new devices to
use.  So why involve the platform layer?

>  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,

Believe it or not, I do understand all of this, having read it several
times in your earlier messages.

>  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).

They already are there, although the code path is a little convoluted.  
It might be possible to refactor this as you suggest; I'm not sure.  
Consider xHCI as an example.  To what extent does it make sense to 
suspend/resume the low/full/high-speed bus independently of the 
SuperSpeed bus?  I don't know enough about xHCI to say.

> 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.

Why not?  They have integration contexts just as much as EHCI does.  

Anyway, the changes I'm talking about are minimal.  Just how minimal
it's hard to say without doing some serious thinking about the proposed
new software design, but I bet they could be made pretty small.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

  Powered by Linux