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