On Mon, 26 May 2008 17:10:09 +0400
Anton Vorontsov <[email protected]> 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. usb_add_hcd() is designed specifically to be
called by other, real probe functions. mmc_spi_probe() _is_ a probe
function. Also exporting it as a library function is very confusing.
>
> 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");.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
rdesktop, core developer http://www.rdesktop.org
On Sun, Jun 01, 2008 at 12:18:41PM +0200, Pierre Ossman wrote:
> On Mon, 26 May 2008 17:10:09 +0400
> Anton Vorontsov <[email protected]> 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: [email protected]
irc://irc.freenode.net/bd2
On Mon, 2 Jun 2008 16:53:37 +0400
Anton Vorontsov <[email protected]> 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 <[email protected]> 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