Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Thursday 16 October 2008, Anton Vorontsov wrote:
> +/*
> + * Platforms can define their own __dev_ versions to glue gpio_chips with the
> + * architecture-specific code.
> + */
> +#ifndef __dev_gpiochip_add
> +#define __dev_gpiochip_add __dev_gpiochip_add
> +static inline int __dev_gpiochip_add(struct device *dev,
> + struct gpio_chip *chip)
> +{
> + chip->dev = dev;
> + return gpiochip_add(chip);
> +}
> +#endif /* __dev_gpiochip_add */
This is pretty ugly, especially the implication that *EVERY* gpio_chip
provider needs modification to use these calls.
Surely it would be a lot simpler to just add platform-specific hooks
to gpiochip_{add,remove}(), so that no providers need to be changed??
> +#ifndef __dev_gpiochip_remove
> +#define __dev_gpiochip_remove __dev_gpiochip_remove
> +static inline int __dev_gpiochip_remove(struct device *dev,
> + struct gpio_chip *chip)
> +{
> + return gpiochip_remove(chip);
> +}
> +#endif /* __dev_gpiochip_remove */
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Fri, Oct 17, 2008 at 01:24:42PM -0700, David Brownell wrote:
> On Thursday 16 October 2008, Anton Vorontsov wrote:
> > +/*
> > + * Platforms can define their own __dev_ versions to glue gpio_chips with
> > the
> > + * architecture-specific code.
> > + */
> > +#ifndef __dev_gpiochip_add
> > +#define __dev_gpiochip_add __dev_gpiochip_add
> > +static inline int __dev_gpiochip_add(struct device *dev,
> > + struct gpio_chip *chip)
> > +{
> > + chip->dev = dev;
> > + return gpiochip_add(chip);
> > +}
> > +#endif /* __dev_gpiochip_add */
>
> This is pretty ugly, especially the implication that *EVERY* gpio_chip
> provider needs modification to use these calls.
Anyway most of them need some modifications to work with OF...
> Surely it would be a lot simpler to just add platform-specific hooks
> to gpiochip_{add,remove}(), [...]
We have printk and dev_printk. kzalloc and devm_kzalloc (though I
aware that devm_ are different than just dev_). So I thought that
dev_gpiochip_* would be logical order of things...
If you don't like it, I can readily implement hooks for
gpiochip_{add,remove}().
Thanks for the comments,
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Friday 17 October 2008, Anton Vorontsov wrote:
> On Fri, Oct 17, 2008 at 01:24:42PM -0700, David Brownell wrote:
> > On Thursday 16 October 2008, Anton Vorontsov wrote:
> > > +/*
> > > + * Platforms can define their own __dev_ versions to glue gpio_chips
> > > with the
> > > + * architecture-specific code.
> > > + */
> > > +#ifndef __dev_gpiochip_add
> > > +#define __dev_gpiochip_add __dev_gpiochip_add
> > > +static inline int __dev_gpiochip_add(struct device *dev,
> > > + struct gpio_chip *chip)
> > > +{
> > > + chip->dev = dev;
> > > + return gpiochip_add(chip);
> > > +}
> > > +#endif /* __dev_gpiochip_add */
> >
> > This is pretty ugly, especially the implication that *EVERY* gpio_chip
> > provider needs modification to use these calls.
>
> Anyway most of them need some modifications to work with OF...
The changes I saw were just to cope with not having
the system-specific platform_data provided: don't
fail if that pointer is NULL, and arrange for dynamic
allocation of some GPIO numbers.
With OpenFirmware, presumably the implication is that
the relevant data is in the OF device tree...
I think that it *barely* makes sense to allow the chips
to bind to drivers without platform data when there's
not even OF in the environment. ONLY in the case where
the GPIOs are exported through sysfs, in fact, since
otherwise there's no way for other system components
to know those GPIOs even exist!! And even that seems
pretty marginal to me...
> > Surely it would be a lot simpler to just add platform-specific hooks
> > to gpiochip_{add,remove}(), [...]
>
> We have printk and dev_printk. kzalloc and devm_kzalloc (though I
> aware that devm_ are different than just dev_). So I thought that
> dev_gpiochip_* would be logical order of things...
Those aren't platform hook mechanisms though, and
there's no need to modify every driver to use them
in order to work *at all* on OpenFirmware systems.
> If you don't like it, I can readily implement hooks for
> gpiochip_{add,remove}().
It seems a better way to a clean solution, IMO. For
example, the OF hook for adding a gpio_chip might
know that it's got to stuff chip->base with a number
other than "-1" (say, "42") since that was stored in
some property of the device's OF shadow, and other
devices have properties associating them with GPIO
numbers derived from that (3rd gpio on that chip,
42 + 3 == 45) and so forth.
That said ... there's a LOT of configuration that
doesn't seem to me like it can be generic. Pullups,
pulldowns, default values, polarity inversion,
what devices depend on those GPIOs being available
before they can come up (GPIO leds and power switches
come quickly to mind), all kinds of chip-specific
details, and more.
Did you look at providing chip-aware OF glue drivers
for this stuff? Doing stuff like just turn the OF
device properties into the right platform_data, and
maybe runing FORTH bytecodes to do other configuration
magic needed...
- Dave
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Mon, Oct 20, 2008 at 12:29:57AM -0700, David Brownell wrote:
[...]
> > Anyway most of them need some modifications to work with OF...
>
> The changes I saw were just to cope with not having
> the system-specific platform_data provided: don't
> fail if that pointer is NULL, and arrange for dynamic
> allocation of some GPIO numbers.
>
> With OpenFirmware, presumably the implication is that
> the relevant data is in the OF device tree...
Yes. Some data is in the device tree.
> I think that it *barely* makes sense to allow the chips
> to bind to drivers without platform data when there's
> not even OF in the environment. ONLY in the case where
> the GPIOs are exported through sysfs, in fact, since
> otherwise there's no way for other system components
> to know those GPIOs even exist!! And even that seems
> pretty marginal to me...
Platform data is a completely different story. And yes, we can't
handle it properly with the device tree. By "properly" I mean without
adding an explicit OF stuff to the drivers, i.e. we should handle the
pdata transparently to the existing drivers.
I quite like the bus notifiers approach:
http://lkml.org/lkml/2008/6/5/209 (mmc_spi example)
But it doesn't work as a module (i.e. OF-specific bits should be
always in-kernel).
[...]
> > If you don't like it, I can readily implement hooks for
> > gpiochip_{add,remove}().
>
> It seems a better way to a clean solution, IMO.
Ok. I will do it.
[...]
> Did you look at providing chip-aware OF glue drivers
> for this stuff? Doing stuff like just turn the OF
> device properties into the right platform_data, and
> maybe runing FORTH bytecodes to do other configuration
> magic needed...
Yes. Few times already. To make the glue, every driver needs
some modifications, and it is always triggers huge discussions
about how to exactly refactor the driver to make it work with
the OF.
http://lkml.org/lkml/2008/5/23/297 (again mmc_spi example).
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
> But it doesn't work as a module (i.e. OF-specific bits should be > always in-kernel). Why not ? Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, Oct 22, 2008 at 11:29:20AM +1100, Benjamin Herrenschmidt wrote: > > > But it doesn't work as a module (i.e. OF-specific bits should be > > always in-kernel). > > Why not ? If say "X" driver loads prior to bus-notifier module (where we fill the platform data), then X.0 device will try to probe w/o platform data and will fail. The only way to re-probe things is to rmmod X && insmod of_pdata_filler_X && insmod X. So things depend on the module load order. The obvious solution is to link the OF stuff into the module, but this also won't work, since modules have only one entry (and exit) point. So there is no way* to hook our OF helpers into the module. * Well, there is one solution to this problem. We can implement arch-specific init_module and cleanup_module entry/exit points, where we can load/unload the OF hooks. This is quite easy, but may look ugly. I could show the drafts. -- Anton Vorontsov email: [EMAIL PROTECTED] irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, Oct 22, 2008 at 05:03:47AM +0400, Anton Vorontsov wrote: > On Wed, Oct 22, 2008 at 11:29:20AM +1100, Benjamin Herrenschmidt wrote: > > > > > But it doesn't work as a module (i.e. OF-specific bits should be > > > always in-kernel). > > > > Why not ? > > If say "X" driver loads prior to bus-notifier module (where we fill > the platform data), then X.0 device will try to probe w/o platform > data and will fail. The only way to re-probe things is to rmmod X && > insmod of_pdata_filler_X && insmod X. So things depend on the module > load order. Thinking about it more, I started recalling other issues. The bus notifier chain doesn't replay previous events, so we also have to register the notifier before the _devices_ are registered. And this ruins the whole approach. :-/ Yeah, that's why I abandoned the bus notifier idea. -- Anton Vorontsov email: [EMAIL PROTECTED] irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, 2008-10-22 at 05:03 +0400, Anton Vorontsov wrote: > If say "X" driver loads prior to bus-notifier module (where we fill > the platform data), then X.0 device will try to probe w/o platform > data and will fail. The only way to re-probe things is to rmmod X && > insmod of_pdata_filler_X && insmod X. So things depend on the module > load order. > > The obvious solution is to link the OF stuff into the module, but > this also won't work, since modules have only one entry (and exit) > point. So there is no way* to hook our OF helpers into the module. Well, right, we need the bus notifier to be registered before any device gets added ... which mean from the same module_init that registers the bus itself. A bit annoying ... > * Well, there is one solution to this problem. We can implement > arch-specific init_module and cleanup_module entry/exit points, > where we can load/unload the OF hooks. This is quite easy, but > may look ugly. I could show the drafts. Yuck :-) Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, 2008-10-22 at 05:42 +0400, Anton Vorontsov wrote: > > Thinking about it more, I started recalling other issues. The bus > notifier chain doesn't replay previous events, so we also have to > register the notifier before the _devices_ are registered. And this > ruins the whole approach. :-/ Yeah, that's why I abandoned the bus > notifier idea. The notifier can be registered before the devices, though it's a little bit fishy and fragile. Easier I suppose to just have OF specific hooks in the bus code. Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, Oct 22, 2008 at 01:28:17PM +1100, Benjamin Herrenschmidt wrote: > On Wed, 2008-10-22 at 05:42 +0400, Anton Vorontsov wrote: > > > > Thinking about it more, I started recalling other issues. The bus > > notifier chain doesn't replay previous events, so we also have to > > register the notifier before the _devices_ are registered. And this > > ruins the whole approach. :-/ Yeah, that's why I abandoned the bus > > notifier idea. > > The notifier can be registered before the devices, though it's a little > bit fishy and fragile. > > Easier I suppose to just have OF specific hooks in the bus code. Every time I think about the problem, this is the conclusion that I come to. Either have OF specific hooks in the probe/remove functions; or have separate probe/remove for OF created instances of a device. g. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Tuesday 21 October 2008, Benjamin Herrenschmidt wrote: > The notifier can be registered before the devices, though it's a little > bit fishy and fragile. > > Easier I suppose to just have OF specific hooks in the bus code. Like what I suggested: "chip-aware OF glue drivers". The relevant bus code being the "of_platform_bus_type" infrastructure. Example: instead of Anton's patch #6 modifying the existing pca953x driver, an of_pca953x driver that knows how to poke around in the OF device attributes to (a) create the pca953x_platform_data, (b) call i2c_register_board_info() to make that available later, and then finally (c) vanish, since it's not needed any longer. Better that than either the $SUBJECT patch, or modifying gpiolib to grow OF-specific hooks ... hooks that can at best solve *one* of the problems: which GPIO numbers to use with this chip. The platform data does solve other problems(*) like: (i) how to initialize the polarity inversion register, (ii) arranging to set up other devices only after their GPIOs are ready, (iii) initializing things that device drivers won't always know about, or which may need to be set up before such drivers are available. - Dave (*) A trivial example of (ii) would be LEDs driven by those GPIOs. A less trivial example: see arch/arm/mach-davinci/board-evm.c in current GIT. There are three pcf8574 I2C expanders used for various things ... LEDs, audio PLL, device power supplies, reset lines for external devices, more. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Tue, Oct 21, 2008 at 09:22:48PM -0700, David Brownell wrote: > On Tuesday 21 October 2008, Benjamin Herrenschmidt wrote: > > The notifier can be registered before the devices, though it's a little > > bit fishy and fragile. > > > > Easier I suppose to just have OF specific hooks in the bus code. > > Like what I suggested: "chip-aware OF glue drivers". The relevant > bus code being the "of_platform_bus_type" infrastructure. > > Example: instead of Anton's patch #6 modifying the existing pca953x > driver, an of_pca953x driver that knows how to poke around in the OF > device attributes to (a) create the pca953x_platform_data, (b) call > i2c_register_board_info() to make that available later, and then > finally (c) vanish, since it's not needed any longer. Heh. You tell me my first approach: http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056730.html (mmc_spi) The OF people didn't like the patch which was used to support this approach: http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056728.html The board info has another problem though. We can't remove it, thus we can't implement module_exit() for the 'OF glue'. -- Anton Vorontsov email: [EMAIL PROTECTED] irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, Oct 22, 2008 at 02:36:41PM +0400, Anton Vorontsov wrote:
> On Tue, Oct 21, 2008 at 09:22:48PM -0700, David Brownell wrote:
> > On Tuesday 21 October 2008, Benjamin Herrenschmidt wrote:
> > > The notifier can be registered before the devices, though it's a little
> > > bit fishy and fragile.
> > >
> > > Easier I suppose to just have OF specific hooks in the bus code.
> >
> > Like what I suggested: "chip-aware OF glue drivers". The relevant
> > bus code being the "of_platform_bus_type" infrastructure.
> >
> > Example: instead of Anton's patch #6 modifying the existing pca953x
> > driver, an of_pca953x driver that knows how to poke around in the OF
> > device attributes to (a) create the pca953x_platform_data, (b) call
> > i2c_register_board_info() to make that available later, and then
> > finally (c) vanish, since it's not needed any longer.
>
> Heh. You tell me my first approach:
>
> http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056730.html (mmc_spi)
>
> The OF people didn't like the patch which was used to support this
> approach:
> http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056728.html
Though, I think I'll able to persuade Grant that two registration paths
are inevitable (i.e. for simple devices we should use
drivers/of/of_{i2c,spi}.c and for complex cases we'll have to have
another method of registration).
> The board info has another problem though. We can't remove it, thus
> we can't implement module_exit() for the 'OF glue'.
And try to solve this problem... maybe then things will begin to
move forward.
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, Oct 22, 2008 at 02:46:06PM +0400, Anton Vorontsov wrote:
> On Wed, Oct 22, 2008 at 02:36:41PM +0400, Anton Vorontsov wrote:
> > On Tue, Oct 21, 2008 at 09:22:48PM -0700, David Brownell wrote:
> > > On Tuesday 21 October 2008, Benjamin Herrenschmidt wrote:
> > > > The notifier can be registered before the devices, though it's a little
> > > > bit fishy and fragile.
> > > >
> > > > Easier I suppose to just have OF specific hooks in the bus code.
> > >
> > > Like what I suggested: "chip-aware OF glue drivers". The relevant
> > > bus code being the "of_platform_bus_type" infrastructure.
> > >
> > > Example: instead of Anton's patch #6 modifying the existing pca953x
> > > driver, an of_pca953x driver that knows how to poke around in the OF
> > > device attributes to (a) create the pca953x_platform_data, (b) call
> > > i2c_register_board_info() to make that available later, and then
> > > finally (c) vanish, since it's not needed any longer.
> >
> > Heh. You tell me my first approach:
> >
> > http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056730.html (mmc_spi)
> >
> > The OF people didn't like the patch which was used to support this
> > approach:
> > http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056728.html
>
> Though, I think I'll able to persuade Grant that two registration paths
> are inevitable (i.e. for simple devices we should use
> drivers/of/of_{i2c,spi}.c and for complex cases we'll have to have
> another method of registration).
>
> > The board info has another problem though. We can't remove it, thus
> > we can't implement module_exit() for the 'OF glue'.
>
> And try to solve this problem... maybe then things will begin to
> move forward.
There is another problem: board infos are scanned at the controller
registration time only. So if we register the board infos after
the controller registered, then nobody will probe the board infos.
This is all solvable by hacking the i2c core code though. I started
it, but it turned out to be ugly. I'll finish it though, just to show
it someday.
But now I'm not sure if it worth the efforts. Maybe we could just
modify the drivers to do something like this?
This is not exactly "transparently" to the drivers, but well..
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 01b4bbd..b1dfa7b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -9,4 +9,7 @@ obj-$(CONFIG_GPIO_MAX732X) += max732x.o
obj-$(CONFIG_GPIO_MCP23S08)+= mcp23s08.o
obj-$(CONFIG_GPIO_PCA953X) += pca953x.o
obj-$(CONFIG_GPIO_PCF857X) += pcf857x.o
+ifeq ($(CONFIG_OF),y)
+obj-$(CONFIG_GPIO_PCF857X) += pcf857x_of.o
+endif
obj-$(CONFIG_GPIO_BT8XX) += bt8xxgpio.o
diff --git a/drivers/gpio/pcf857x.c b/drivers/gpio/pcf857x.c
index 4bc2070..f8057d2 100644
--- a/drivers/gpio/pcf857x.c
+++ b/drivers/gpio/pcf857x.c
@@ -187,7 +187,7 @@ static int pcf857x_probe(struct i2c_client *client,
struct pcf857x *gpio;
int status;
- pdata = client->dev.platform_data;
+ pdata = pcf857x_get_pdata(client);
if (!pdata)
return -ENODEV;
@@ -314,7 +314,7 @@ fail:
static int pcf857x_remove(struct i2c_client *client)
{
- struct pcf857x_platform_data*pdata = client->dev.platform_data;
+ struct pcf857x_platform_data*pdata = pcf857x_get_pdata(client);
struct pcf857x *gpio = i2c_get_clientdata(client);
int status = 0;
@@ -334,6 +334,8 @@ static int pcf857x_remove(struct i2c_client *client)
kfree(gpio);
else
dev_err(&client->dev, "%s --> %d\n", "remove", status);
+
+ pcf857x_put_pdata(client);
return status;
}
diff --git a/drivers/gpio/pcf857x_of.c b/drivers/gpio/pcf857x_of.c
new file mode 100644
index 000..414943b
--- /dev/null
+++ b/drivers/gpio/pcf857x_of.c
@@ -0,0 +1,36 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct pcf857x_platform_data *pcf857x_get_pdata(struct i2c_client *client)
+{
+ struct pcf857x_platform_data *pdata = client->dev.platform_data;
+
+ if (pdata)
+ return pdata;
+
+ pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return NULL;
+
+ /*
+* Do the OF-specific setup here.
+*/
+
+ client->dev.platform_data = pdata;
+}
+
+void pcf857x_put_pdata(struct i2c_client *client)
+{
+ struct pcf857x_platform_data *pdata = client->dev.platform_data;
+
+ /*
+* Do the OF-specific cleanup here.
+*/
+
+ kfree(pdata);
+}
diff --git a/include/linux/i2c/pcf857x.h b/include/linux/i2c/pcf857x.h
index 0767a2a..bdc1aba 100644
--- a/include/linux/i2c/pcf857x.h
+++ b/include/linux/i2c/pcf857x.h
@@ -1,6 +1,8 @@
#ifndef __LINUX_PCF857X_H
#define __LINUX_PCF857X_H
+struct i2c_client;
+
/**
* struct pcf857x_platform_data - data to set up pcf857x
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wednesday 22 October 2008, Anton Vorontsov wrote: > > > > The board info has another problem though. We can't remove it, thus > > > we can't implement module_exit() for the 'OF glue'. That's not a problem. Why would you want to remove it? > > And try to solve this problem... maybe then things will begin to > > move forward. > > There is another problem: board infos are scanned at the controller > registration time only. Right. Such board description data should be made available early in boot. As a rule: before arch_initcall() finishes, so that subsys_initcall() code can use the associated GPIOs. (It's fairly well acknowledged that init dependency handling has a lot of problems. Until that's fixed ... for GPIOs, the general advice is to make sure everything is available by subsys_initcall time, so the subsystems which rely on GPIOs to initialize -- power switches, resets, etc -- can initialize. That can require i2c adapter drivers to use subsys_initcall, for example.) > So if we register the board infos after > the controller registered, then nobody will probe the board infos. See above. If you're doing it right, there's no problem. That is, scan the OF tables early. Just like PNP tables get scanned early, for example. - Dave ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, Oct 22, 2008 at 02:04:52PM -0700, David Brownell wrote: > On Wednesday 22 October 2008, Anton Vorontsov wrote: > > > > > > The board info has another problem though. We can't remove it, thus > > > > we can't implement module_exit() for the 'OF glue'. > > That's not a problem. Why would you want to remove it? > > > > > And try to solve this problem... maybe then things will begin to > > > move forward. > > > > There is another problem: board infos are scanned at the controller > > registration time only. > > Right. Such board description data should be made available > early in boot. As a rule: before arch_initcall() finishes, > so that subsys_initcall() code can use the associated GPIOs. > > (It's fairly well acknowledged that init dependency handling > has a lot of problems. Until that's fixed ... for GPIOs, the > general advice is to make sure everything is available by > subsys_initcall time, so the subsystems which rely on GPIOs > to initialize -- power switches, resets, etc -- can initialize. > That can require i2c adapter drivers to use subsys_initcall, > for example.) > > > > So if we register the board infos after > > the controller registered, then nobody will probe the board infos. > > See above. If you're doing it right, there's no problem. > That is, scan the OF tables early. Just like PNP tables > get scanned early, for example. Heh. If we don't want to be able to make the OF-parsing code be a module then there is no problem at all. I can use the bus notifiers. And it is most straightforward solution then. But I quite dislike to bloat the kernel image with maybe-never-used-on-this-board code. My aim was to make the OF-parsing part be a module too. Because in the long run we need the OF-parsing stuff for _every_ driver that needs platform data. It's quite expensive to have it always built-in, don't you think? -- Anton Vorontsov email: [EMAIL PROTECTED] irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wednesday 22 October 2008, Anton Vorontsov wrote: > > > > > So if we register the board infos after > > > the controller registered, then nobody will probe the board infos. > > > > See above. If you're doing it right, there's no problem. > > That is, scan the OF tables early. Just like PNP tables > > get scanned early, for example. > > Heh. If we don't want to be able to make the OF-parsing code > be a module then there is no problem at all. I can use the bus > notifiers. And it is most straightforward solution then. > > But I quite dislike to bloat the kernel image with > maybe-never-used-on-this-board code. So have it live in the __init text section. If you're building a kernel with support for several boards, you know it's necessarily going to be larger than it would be if only one board were supported. But you can shrink kernel size by judicious use of __init sections.. > My aim was to make the > OF-parsing part be a module too. Because in the long run we > need the OF-parsing stuff for _every_ driver that needs > platform data. It's quite expensive to have it always built-in, > don't you think? If it's discarded early, after translating the data from OF format into what the drivers need, there will be no RAM footprint. - Dave ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, Oct 22, 2008 at 02:52:46PM -0700, David Brownell wrote: > On Wednesday 22 October 2008, Anton Vorontsov wrote: > > > > > > > So if we register the board infos after > > > > the controller registered, then nobody will probe the board infos. > > > > > > See above. If you're doing it right, there's no problem. > > > That is, scan the OF tables early. Just like PNP tables > > > get scanned early, for example. > > > > Heh. If we don't want to be able to make the OF-parsing code > > be a module then there is no problem at all. I can use the bus > > notifiers. And it is most straightforward solution then. > > > > But I quite dislike to bloat the kernel image with > > maybe-never-used-on-this-board code. > > So have it live in the __init text section. If you're > building a kernel with support for several boards, you > know it's necessarily going to be larger than it would > be if only one board were supported. But you can shrink > kernel size by judicious use of __init sections.. Won't work, unfortunately. I2C devices are created by the i2c controllers, via drivers/of_i2c.c of_register_i2c_devices(). There is a good reason to do so, the code needs to know controller's OF node to walk down and register the child nodes (devices). See drivers/i2c/busses/i2c-mpc.c -- it calls of_register_i2c_devices() at the end of the probe(). Since we can't call __init stuff from non-__init, the scheme you purpose won't work. The same is for SPI (drivers/of_spi.c of_register_spi_devices()). > > My aim was to make the > > OF-parsing part be a module too. Because in the long run we > > need the OF-parsing stuff for _every_ driver that needs > > platform data. It's quite expensive to have it always built-in, > > don't you think? > > If it's discarded early, after translating the data from > OF format into what the drivers need, there will be no > RAM footprint. There is also kernel image size that matters... -- Anton Vorontsov email: [EMAIL PROTECTED] irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, 2008-10-22 at 14:04 -0700, David Brownell wrote: > > So if we register the board infos after > > the controller registered, then nobody will probe the board infos. > > See above. If you're doing it right, there's no problem. > That is, scan the OF tables early. Just like PNP tables > get scanned early, for example. It's still pretty yucky in that case to scan the device-tree to convert it into some kind of fugly board info ... I'd rather have the end drivers that actually use those GPIOs scan the device-tree directly. But then, I'm not a believer in generic drivers for things like GPIOs, i2c devices, etc.. :-) Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wednesday 22 October 2008, Anton Vorontsov wrote: > > > > So have it live in the __init text section... > > Won't work, unfortunately. I2C devices are created by the > i2c controllers, via drivers/of_i2c.c of_register_i2c_devices(). And I'm pointing out a way to have the normal I2C core code flow do that creation. OF shouldn't need to be so much of a special case. > There is a good reason to do so, the code needs to know > controller's OF node to walk down and register the child nodes > (devices). See drivers/i2c/busses/i2c-mpc.c -- it calls > of_register_i2c_devices() at the end of the probe(). I don't get it. (But then, so much of the OF support seems needlessly convoluted to me ... on top of seeming to be insufficient for configuring board-specific details.) There's an OF device tree, distinct from the Linux driver model tree. Why should there be any obstacle to accessing records from that tree before the relevant driver model nodes have been created? Remember that the various board_info structs get registered before the driver model nodes for which they are templates. Just translate the OF tree data to those templates(*), then register them. I understand that it's currently structured differetnly than that ... consulting the OF tree "late" not early. But that's still newish, and from what I've heard so far it doesn't seem like the best structure either... nothing seems to plug in smoothly. - Dave (*) The role of the board_info structs is very similar to the role of OF device attributes. As is the role of the platform_data ... except that's more specific to the chip involved (and its driver), and expects any callbacks to be in C code not FORTH. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wednesday 22 October 2008, Benjamin Herrenschmidt wrote: > On Wed, 2008-10-22 at 14:04 -0700, David Brownell wrote: > > > So if we register the board infos after > > > the controller registered, then nobody will probe the board infos. > > > > See above. If you're doing it right, there's no problem. > > That is, scan the OF tables early. Just like PNP tables > > get scanned early, for example. > > It's still pretty yucky in that case to scan the device-tree to convert > it into some kind of fugly board info ... I'd rather have the end > drivers that actually use those GPIOs scan the device-tree directly. Keep in mind that these problems are not specific to GPIOs. And, very important!!, most of the drivers run without OF... Pretty much any little device that needs board-specific customization has the same class of problems: drivers will need a variety of parameters that may are often not sharable with other devices, with idiosyncratic usage. And they hook up to other drivers in arbitrary ways. When PCs have such issues, ACPI hides them from Linux. If those parameters -- potentially including callbacks that escape to FORTH -- are stored in the OF device tree, so be it. But "fugly" sounds like part of that problem domain, so it's no surprise that it maps onto the solution space too... Specifically with respect to GPIOs ... what do you mean by "end driver" though? I previously pointed at one example (Davinci EVM) where one bank of GPIOs is used by about six different drivers ... none of which have any reason to know they're using a pcf8574a vs any other kind of GPIO. Is the "end driver" the IDE/CF driver? The USB OTG driver? The driver sitting the next layer above of one of those? *Some* of the drivers need to touch the GPIOs. Others don't. > But then, I'm not a believer in generic drivers for things > like GPIOs, i2c devices, etc.. :-) I kind of like being able to re-use code myself. ;) It's a win to have *one* pcf8574/5 driver that can be reused -- with a bit of care configuring it into the system -- instead of having every board contribute yet another board-specific hack in drivers/i2c/chips ... And I think such stuff can be done even with OF. - Dave ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wednesday 22 October 2008, Anton Vorontsov wrote: > --- a/drivers/gpio/pcf857x.c > +++ b/drivers/gpio/pcf857x.c > @@ -187,7 +187,7 @@ static int pcf857x_probe(struct i2c_client *client, > struct pcf857x *gpio; > int status; > > - pdata = client->dev.platform_data; > + pdata = pcf857x_get_pdata(client); > if (!pdata) > return -ENODEV; > I suppose that can work. Regardless, some OF-specific code will need to map device tree state into a generic format that's fully decoupled from OF. (And there's something to be said to having that mapping sit in the same directory as the driver needing it.) ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 0/6 RFC] OF-glue devices for I2C/SPI (was: Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
On Wed, Oct 22, 2008 at 02:46:06PM +0400, Anton Vorontsov wrote:
[...]
> > > Like what I suggested: "chip-aware OF glue drivers". The relevant
> > > bus code being the "of_platform_bus_type" infrastructure.
> > >
> > > Example: instead of Anton's patch #6 modifying the existing pca953x
> > > driver, an of_pca953x driver that knows how to poke around in the OF
> > > device attributes to (a) create the pca953x_platform_data, (b) call
> > > i2c_register_board_info() to make that available later, and then
> > > finally (c) vanish, since it's not needed any longer.
> >
> > Heh. You tell me my first approach:
> >
> > http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056730.html (mmc_spi)
> >
> > The OF people didn't like the patch which was used to support this
> > approach:
> > http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056728.html
>
> Though, I think I'll able to persuade Grant that two registration paths
> are inevitable (i.e. for simple devices we should use
> drivers/of/of_{i2c,spi}.c and for complex cases we'll have to have
> another method of registration).
Ok, here it is.
I don't like this approach because:
1. It feels like an overhead to create an of_device for each i2c
device that needs platform data.
2. We have to do ugly of_should_create_pdev() in the i2c code,
and duplicate lists of supported devices.
Could anybody convince me that this isn't a big deal? ;-)
Otherwise I'll stick with this approach:
http://lkml.org/lkml/2008/10/22/471
Thanks,
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 0/6 RFC] OF-glue devices for I2C/SPI (was: Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls
On Tue, Oct 28, 2008 at 11:45 AM, Anton Vorontsov
<[EMAIL PROTECTED]> wrote:
> On Wed, Oct 22, 2008 at 02:46:06PM +0400, Anton Vorontsov wrote:
> [...]
>> > > Like what I suggested: "chip-aware OF glue drivers". The relevant
>> > > bus code being the "of_platform_bus_type" infrastructure.
>> > >
>> > > Example: instead of Anton's patch #6 modifying the existing pca953x
>> > > driver, an of_pca953x driver that knows how to poke around in the OF
>> > > device attributes to (a) create the pca953x_platform_data, (b) call
>> > > i2c_register_board_info() to make that available later, and then
>> > > finally (c) vanish, since it's not needed any longer.
>> >
>> > Heh. You tell me my first approach:
>> >
>> > http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056730.html (mmc_spi)
>> >
>> > The OF people didn't like the patch which was used to support this
>> > approach:
>> > http://ozlabs.org/pipermail/linuxppc-dev/2008-May/056728.html
>>
>> Though, I think I'll able to persuade Grant that two registration paths
>> are inevitable (i.e. for simple devices we should use
>> drivers/of/of_{i2c,spi}.c and for complex cases we'll have to have
>> another method of registration).
>
> Ok, here it is.
>
> I don't like this approach because:
>
> 1. It feels like an overhead to create an of_device for each i2c
> device that needs platform data.
>
> 2. We have to do ugly of_should_create_pdev() in the i2c code,
> and duplicate lists of supported devices.
>
> Could anybody convince me that this isn't a big deal? ;-)
I really don't like this approach either. I think it is a big deal.
It greatly increases the complexity of the probe path for the device.
Adapting the OF data to pdata is entirely driver specific and it
doesn't make sense to break it out into a separate of_device or use
the board_info mechanism to disassociate the adapter code from the
driver. Each instance of adapter code will never be associated with a
different driver and I think it is entirely reasonable to have a
driver specific hook in the drivers probe path to build pdata from the
device tree.
>
> Otherwise I'll stick with this approach:
> http://lkml.org/lkml/2008/10/22/471
I like this approach.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
