Re: [RFC PATCH 1/2] mmc_spi: export probe and remove functions

[Anton Vorontsov <avorontsov@ru.mvista.com>]

On Sun, Jun 01, 2008 at 12:18:41PM +0200, Pierre Ossman wrote:
> On Mon, 26 May 2008 17:10:09 +0400
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> 
> > 
> > Btw, this isn't actually drivers encapsulating. This is about making
> > mmc_spi export some "library" function which could be used by other
> > bindings.
> > 
> > Think of usb_add_hcd() used by various drivers' bindings for e.g.
> > drivers/usb/host/ehci-*.c. Though usb_add_hcd() is more generic
> > than just "EHCI" bindings, but only because there is nothing to
> > share between them. (for MMC over SPI bindings all we want to do is fill
> > the platform data).
> > 
> 
> There's a big difference.

This depends on the perception. :-)

> usb_add_hcd() is designed specifically to be called by other, real probe
> functions.

Yes, by convention (or better, by design).

> mmc_spi_probe() _is_ a probe function. 

Yes, so far.

> Also exporting it as a library function is very confusing.

No, if designed/documented properly.

Just imagine this (100% similarity to USB code):

mmc_spi_create_hcd(&mmc_spi_driver, dev, dev->bus_id);
mmc_spi_add_hcd(dev, irq, irqflags);

> > Maybe something like this? I don't like it so much, but given that
> > you don't like to export functions from mmc_spi, we'll have to place
> > some calls into the driver itself. :-/ And there is no easy way to do
> > generic callbacks, since that way we'll have implement "mmc_spi
> > callbacks subsystem". :-)
> 
> That's not a callback, but an explicit call to another module.
> 
> 
> All of this work looks a bit like trying to wedge a square piece into a
> round hole. It looks to me that the kernel needs a bit of restructuring
> to handle it. You can't really export every probe function of every
> platform device so that you can add OF hooks to it.
> 
> From what I can tell, the OF stuff behaves very much like the PNP
> system on PCs. The information relayed is a bit more versatile though.
> Perhaps what is needed is a more advanced "platform" bus that is
> modeled after the PNP bus, but with the extra ability of handling the
> stuff currently crammed into the platform structures. mmc_spi would
> then be extended to be driver for the "platform" bus and we could have
> generic calls like platform_get_pin(dev, "ro");.

platform_get_pin()? Um, maybe platform_get_gpio(), as _irq()? Yes,
this is doable (and someday this should be done). But this way we can
pass only GPIOs and then teach mmc_spi to work with them directly
(in addition to callbacks).

But this is not enough, there is still no way to pass real platform data,
such as: caps and ocr_mask. Any idea how to deliver these?


Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2