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 Thu, Aug 25, 2011 at 01:39:25PM -0400, Alan Stern wrote:
> On Thu, 25 Aug 2011, Felipe Balbi wrote:
> 
> > > With your plan, I don't see how you can hope to make a single driver
> > > source work for all the different IP blocks.  Passing the resources via
> > > devicetree will help, but the different hardware implementations will
> > > still require vendor-specific code to run at various times, right?  
> > > Otherwise all the ehci-*.c files would resemble each other more
> > > closely.
> > 
> > Don't you think they are ? Other than the very intial HW configuration
> > all they do is provide struct hc_driver with the exact same fields.
> > Maybe a few differences here an there, but most of them, just reuse the
> > functions exported by ehci-hcd.
> 
> Then why not start making modifications to ehci-hcd by uniting all
> those different platform drivers into a single source file?  You'd have
> to do that anyway; might as well begin now since it's
> non-controversial.
> 
> If that can be made to work well, it will be a very strong argument for 
> making PCI use a platform device too.

Yes, that can be done. Sounds to me, though, like we will have a
duplication of effort. To see how things look like when we have a PCI
device bridging to a platform device, you just have to look into Greg's
for-next branch. The DesignWare USB3 driver which I wrote with
Sebastian follow that approach and we have PCI and OMAP glue layers.

PCI is used for the FPGA platform and OMAP is used, well, for OMAP
ASICs.

See that, as of now, PM code is just a NOP because we don't have real HW
to test against. But after we _do_ have HW, patches will come
implementing all of those.

> > > If your main concern is getting rid of the #ifdefs in ehci-hcd.c (and
> > > preventing the equivalent from being added to xhci.c), there's a
> > > simpler way to accomplish it: Use conditional linking rather than
> > > conditional compilation.  All that complexity would be moved out of the
> > > C source files and into the Makefile instead.  That is, instead of
> > > 
> > > obj-$(CONFIG_USB_EHCI_HCD)	+= ehci-hcd.o
> > > 
> > > which is what we currently have, we could have
> > > 
> > > obj-$(CONFIG_PCI)		+= ehci-pci.o ehci-hcd.o
> > > obj-$(CONFIG_USB_EHCI_FSL)	+= ehci-fsl.o ehci-hcd.o
> > > obj-$(CONFIG_USB_EHCI_HCD_OMAP)	+= ehci-omap.o ehci-hcd.o
> > > etc.
> > 
> > I remember proposing that long ago and Dave was against it because it
> > would make "copies" of the entire ehci stack right ? So if we had an
> > OMAP board with a PCIe controller, we would have two big ehci drivers.
> > My proposal was to reuse ehci-hcd.ko without having to link it to every
> > single ehci-<arch>.o
> 
> It wouldn't have to be linked to every ehci-<arch>.o; only the one 
> that the kernel is configured for.  Right now you can't build more than 
> one of them, right?  Otherwise there would be multiple definitions for 
> the PLATFORM_DRIVER symbol.

You don't seem to get the big picture. At the end of the day we want to
have a distro-like kernel for embedded too. And that would mean e.g.
building all ehci-<arch>.o which are ARM-based plus the PCI since few
ARM-based board have PCI.

> For that matter, why does ehci-hcd.c have separate code for registering 
> OF_PLATFORM_DRIVER and XILINX_OF_PLATFORM_DRIVER?  Why don't they use 
> the same old PLATFORM_DRIVER symbol as everything else?

That's a good question, but why do you think I'm the one to answer that?
I believe it would be better to send that question straight to the
authors/maintainers of those files ? Maybe even Cc yourself :-p

> > > The one disadvantage would be that you couldn't have a single driver
> > > module to handle both PCI and a platform device; they would have to be
> > > separate drivers.
> > 
> > true.
> 
> There's also the PS3 system bus thingy -- yet another bus.

So we _do_ agree that it's time to unify all of those, that's great. We
just need to agree on the implementation details.

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