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

[Pierre Ossman]

On Mon, 2 Jun 2008 16:53:37 +0400
Anton Vorontsov <avorontsov@xxxxxxxxxxxxx> wrote:

> 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@xxxxxxxxxxxxx> 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);
> 

I'm not necessarily against that idea. What I don't like is using a
function for different things. So you'd need to restructure things a
bit, and not just export the probe function.

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

platform_get_attr("ocr") perhaps? I'm afraid I have not dealt enough
with the embedded parts of the kernel to give a complete suggestion as
to how this might be solved.

My concern is building something OF specific right now, only to have to
restructure things when the next firmware interface shows up and having
to deal with all the fallout caused by API changes.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/