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 Fri, 26 Aug 2011, Felipe Balbi wrote:

> > to see how it looks when somebody handles a bunch of platforms in a 
> > single source file.  For example, is it possible to merge ehci-fsl.c, 
> > ehci-xilinx-of.c, ehci-sh.c, and ehci-omap.c into a single file without 
> > getting something that looks like Frankenstein's monster?
> 
> I find it very unlikely, but what's the point ? That's really not what
> we want.

I thought that was exactly what you wanted.  Your scheme is to register
a platform device and make the ?hci-hcd module that device's driver,
right?  Which means a single source file will have to contain the bus
glue for fsl, xilinx, sh, omap, and all the other EHCI platform
implementations.

Unless you register multiple platform drivers within the same module.  
Was that your plan?  I thought you intended to have just a single 
platform driver that would work with everything.  If not -- if you're 
going to have multiple drivers in the same module -- then what's the 
reason for adding a "glue" platform device below a PCI controller?

>  What's common should definitely go to a common file - like when
> I moved the hc_driver from xhci-pci.c to xhci.c -, on the other hand, if
> it's platform-specific code, why do we want to combine them ?
> 
> When I talk about common code, I mean the duplicated hc_driver on
> ehci-<arch>.c files. That could all go into a common source
> (ehci-hcd.c?) and the arch files become smaller and easier to maintain
> (less code == less trouble).

But the hc_driver structure _does_ contain some platform-specific 
fields: .product_desc and .reset for sure (although maybe the reset 
routines can be factored better), and several other fields in a few of 
the drivers.


> > The end result is that if you plug in a standard mass-storage device,
> > the main usb-storage module gets loaded and acts as the driver.  If you
> > plug in one of the non-standard devices then the appropriate submodule
> > acts as a driver, and the main module is also loaded in order to
> > satisfy the export dependencies.
> 
> Generally I like to avoid exported symbols where possible. That's why
> using the device hierarchy from driver core came to mind.

The purpose of the device hierarchy isn't to allow programmers to avoid
exporting symbols.  Its purpose is to provide a software model for the
actual hardware.

> > xhci-hcd could be made to work similarly, except that there would be no
> > "standard" driver -- there would just be a submodule driver for PCI
> > plus one or more for various platform devices.  The main module would
> > contain only the xHCI core routines.
> > 
> > I realize this involves an extra layer of software compared to what you
> > want, but it might end up being cleaner -- especially if other vendors
> > design their own USB-3 IP blocks that are not sufficiently compatible
> > with your DesignWare code.  We could handle them simply by adding
> > another IP-specific platform-driver submodule, whereas your scheme
> > really couldn't handle them without great difficulty.
> 
> I didn't get this. DesignWare is one IP. Any licensee from Synopsys will
> get that IP, configure the way they want wrt FIFO sizes, number of
> endpoints, number of interrupters, number of RAMs, etc etc etc. But the
> IP itself (and the programming model) remains the same.

What will happen when some other company, not Synopsys, creates their
own XHCI IP?

> So if SoC Vendor XYZ decides to license the IP from Synopsys, all they
> have to do is provide the parent device and allocate the core
> platform_device. If they need something a bit more complex, they can
> pass in platform_data or use the driver_data depending on the case.

And what if vendor XYZ decides to license the non-Synopsys IP?

> Now, with regards to xhci, this will become very similar. No matter
> where xhci is being used, other than Silicon bugs/quirks and compliance
> to XHCI 0.96 or 1.0, the programming model is the same. Meaning that
> struct hc_driver for xhci will be the same thing throughout. We can
> already see what I'm talking about with ehci-<arch>.c files. Take a look
> at their struct hc_driver structure and you'll see that they are just
> using ehci_* functions exported by ehci-core code.

They are not.  At least, not all of them.  Look at ehci-tegra.c for 
example.

> What I'm trying to say is that no matter where [ouex]hci is being used,
> the standard is the same, correct ? So ehci_urb_enqueue, ehci_irq,
> ehci_urb_dequeue and all their friends really don't need to be exported.
> If [ouex]hci have their own platform_device/platform_driver, those could
> be made static and [ouex]hci-<arch>.c can, at least, remove the
> hc_driver definition, since that's just a big pile of duplicated code.
> 
> Now, I apologize if I didn't get your point and I ask you to describe
> further what you meant ;-)

Maybe you weren't aware of all the little special-purpose changes lying 
hidden in the various <arch>.c files.  They could become a substantial
hindrance to your plan.

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