Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-23 Thread David Brownell 
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

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-22 Thread Anton Vorontsov 
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

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-22 Thread Anton Vorontsov 
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

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-22 Thread Anton Vorontsov 
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 linux/kernel.h
+#include linux/slab.h
+#include linux/i2c.h
+#include linux/i2c/pcf857x.h
+#include linux/gpio.h
+#include linux/of.h
+#include linux/of_gpio.h
+
+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: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-22 Thread Anton Vorontsov 
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

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-22 Thread David Brownell 
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



___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-22 Thread Anton Vorontsov 
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

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-22 Thread Benjamin Herrenschmidt 
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.



___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-22 Thread David Brownell 
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.

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-21 Thread Anton Vorontsov 
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

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-21 Thread Benjamin Herrenschmidt 
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.



___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-21 Thread Benjamin Herrenschmidt 
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.



___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-20 Thread David Brownell 
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

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-20 Thread Anton Vorontsov 
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

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-17 Thread David Brownell 
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 */



___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: [i2c] [PATCH 4/7] gpiolib: implement dev_gpiochip_{add, remove} calls

2008-10-17 Thread Anton Vorontsov 
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

___
i2c mailing list
i2c@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/i2c