Hi,
On Wed, Aug 24, 2011 at 10:49:59AM -0400, Alan Stern wrote:
> > > So what? Sure, every PCI device has such an entity. Does that mean
> > > every pci_device structure needs to have a platform_device child?
> >
> > why not ? Then we split the Integration logic (PCI, OMAP, FreeScale,
> > Marvel, etc) from the IP driver.
>
> Good luck trying to sell that idea to the PCI maintainers. They'll
> laugh at you.
And I just remembered another example: If you look into the AHCI driver,
there are two versions of it: ahci.c and ahci_platform.c. Both drivers
do the exact same thing. The only difference is the PCI and platform
bus.
Now, I didn't dig into both drivers closely enough, but I'm pretty sure
some features available on one driver won't be available on the other
and vice-versa. One driver will have a certain bug somewhere and the
other will have a certain bug on another location.
So IMHO it's just plain silly duplication of effort. If you just split
the PCI to a parent driver so that ahci.c and ahci_platform.c are
combined into one single source code, bug fixing effort will not be
scattered on two different files.
As of today, if I implement pm runtime support to ahci_platform.c,
ahci.c (the PCI version) will lack such support. If they were one single
file with a PCI bridge driver to the ahci platform_driver, when I
implemented pm runtime to ahci_platform, the PCI version would get that
support for free.
So if the PCI maintainers want to laugh, let them laugh but I'm sure in
the long run they'll thank not to have two different drivers to maintain
and bugfix.
another argument is that the more users we have for a certain piece of
code, the more likely we are to find bugs and the more likely to get new
features implemented. Such features and bugfixes would be useful for x86
and all other Archs. As of today, you would need double the amount of
work to implement pm_runtime or fix a bug.
IMHO, having, as you call it, a fake platform_device is a very small
price for the benefits we get from such approach. Just start thinking
about the whole picture wrt e.g. PM:
xhci.c -> dev_pm_ops:
static xhci_suspend()
{
clear_run_stop_bit();
clear_command_rung();
save_context();
set_css_flag();
disable_irq();
}
xhci-pci.c -> dev_pm_ops:
static xhci_pci_suspend()
{
for_each_msi() {
synchronize_irq();
}
pci_disable();
}
meaning that PCI-related is kept on the PCI glue (and xhci-core doesn't
need an exported function neither does xhci-pci.c) and XHCI-specific
part is kept on xhci.c. No one need to call into each other, no
non-static functions.
--
balbi
Attachment:
signature.asc
Description: Digital signature
