LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Brownell <david-b@pacbell.net>
To: avorontsov@ru.mvista.com
Cc: linux-kernel@vger.kernel.org,
	David Brownell <dbrownell@users.sourceforge.net>,
	"Steven A. Falco" <sfalco@harris.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	Jean Delvare <khali@linux-fr.org>,
	David Miller <davem@davemloft.net>,
	i2c@lm-sensors.org, linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
Date: Mon, 20 Oct 2008 00:29:57 -0700	[thread overview]
Message-ID: <200810200029.58312.david-b@pacbell.net> (raw)
In-Reply-To: <20081017212942.GA1919@oksana.dev.rtsoft.ru>

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

  reply	other threads:[~2008-10-20  7:30 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-16 17:12 [PATCH 0/7 RFC] Handle I2C GPIO controllers with the OF (was: pca9539 I2C gpio expander) Anton Vorontsov
2008-10-16 17:12 ` [PATCH 1/7] powerpc and sparc: introduce dev_archdata node accessors Anton Vorontsov
2008-10-16 22:36   ` David Miller
2008-10-16 23:02     ` Grant Likely
2008-10-16 17:12 ` [PATCH 2/7] i2c: add info->archdata field Anton Vorontsov
2008-10-17  9:21   ` Jean Delvare
2008-10-22  0:27     ` Benjamin Herrenschmidt
2008-10-22  6:50       ` Jean Delvare
2008-10-22  7:37         ` Benjamin Herrenschmidt
2008-10-22 10:08           ` Anton Vorontsov
2008-10-22 11:07             ` Jean Delvare
2008-10-22 12:50               ` Anton Vorontsov
2008-10-16 17:12 ` [PATCH 3/7] of: fill the archdata for I2C devices Anton Vorontsov
2008-10-22  4:14   ` Grant Likely
2008-10-16 17:12 ` [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls Anton Vorontsov
2008-10-17 20:24   ` David Brownell
2008-10-17 21:29     ` Anton Vorontsov
2008-10-20  7:29       ` David Brownell [this message]
2008-10-20 15:48         ` Anton Vorontsov
2008-10-22  0:29           ` Benjamin Herrenschmidt
2008-10-22  1:03             ` Anton Vorontsov
2008-10-22  1:42               ` Anton Vorontsov
2008-10-22  2:28                 ` Benjamin Herrenschmidt
2008-10-22  4:20                   ` Grant Likely
2008-10-22  4:22                   ` David Brownell
2008-10-22 10:36                     ` Anton Vorontsov
2008-10-22 10:46                       ` Anton Vorontsov
2008-10-22 18:32                         ` Anton Vorontsov
2008-10-22 21:04                           ` David Brownell
2008-10-22 21:22                             ` Anton Vorontsov
2008-10-22 21:52                               ` David Brownell
2008-10-22 22:29                                 ` Anton Vorontsov
2008-10-23  5:19                                   ` David Brownell
2008-10-23  4:45                             ` Benjamin Herrenschmidt
2008-10-23  6:06                               ` David Brownell
2008-10-23  6:15                           ` David Brownell
2008-10-28 17:45                         ` [PATCH 0/6 RFC] OF-glue devices for I2C/SPI (was: " Anton Vorontsov
2008-10-28 17:46                           ` [PATCH 1/6] of/base: Add new helper of_should_create_pdev() Anton Vorontsov
2008-10-28 17:46                           ` [PATCH 2/6] of/of_i2c: implement of_{,un}register_i2c_device Anton Vorontsov
2008-10-28 17:46                           ` [PATCH 3/6] of/of_i2c: add support for dedicated OF I2C devices Anton Vorontsov
2008-10-28 18:41                             ` David Miller
2008-10-28 17:46                           ` [PATCH 4/6] of/gpio: add support for two-stage registration for the of_gpio_chips Anton Vorontsov
2008-10-28 17:46                           ` [PATCH 5/6] gpio/pca953x: pass gpio_chip pointer to the setup/teardown callbacks Anton Vorontsov
2008-10-28 17:46                           ` [PATCH 6/6] gpio: OpenFirmware bindings for the pca953x Anton Vorontsov
2008-10-28 17:53                           ` [PATCH 0/6 RFC] OF-glue devices for I2C/SPI (was: Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls Grant Likely
2008-10-22  2:27               ` Benjamin Herrenschmidt
2008-10-16 17:13 ` [PATCH 5/7] of/gpio: implement of_dev_gpiochip_{add,remove} calls Anton Vorontsov
2008-10-17 20:25   ` David Brownell
2008-10-17 21:13     ` Anton Vorontsov
2008-10-16 17:13 ` [PATCH 6/7] gpio/pca953x: convert to dev_gpiochip_add and make it work with the OF Anton Vorontsov
2008-10-16 17:13 ` [PATCH 7/7] i2c/mcu_mpc8349emitx: convert to the new I2C/OF/GPIO infrastructure Anton Vorontsov
2008-10-17 16:07 ` [PATCH 0/7 RFC] Handle I2C GPIO controllers with the OF (was: pca9539 I2C gpio expander) Steven A. Falco

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200810200029.58312.david-b@pacbell.net \
    --to=david-b@pacbell.net \
    --cc=avorontsov@ru.mvista.com \
    --cc=davem@davemloft.net \
    --cc=dbrownell@users.sourceforge.net \
    --cc=grant.likely@secretlab.ca \
    --cc=i2c@lm-sensors.org \
    --cc=khali@linux-fr.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=sfalco@harris.com \
    --subject='Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).