LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Anton Vorontsov <avorontsov@ru.mvista.com>
To: David Brownell <david-b@pacbell.net>
Cc: benh@kernel.crashing.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@ozlabs.org, i2c@lm-sensors.org,
	Jean Delvare <khali@linux-fr.org>,
	David Miller <davem@davemloft.net>
Subject: Re: [PATCH 4/7] gpiolib: implement dev_gpiochip_{add,remove} calls
Date: Wed, 22 Oct 2008 22:32:18 +0400	[thread overview]
Message-ID: <20081022183218.GA19025@oksana.dev.rtsoft.ru> (raw)
In-Reply-To: <20081022104606.GA510@oksana.dev.rtsoft.ru>

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 0000000..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 driver
  * @gpio_base: number of the chip's first GPIO
@@ -41,4 +43,13 @@ struct pcf857x_platform_data {
 	void		*context;
 };
 
+#ifdef CONFIG_OF
+extern struct pcf857x_platform_data *
+	pcf857x_get_pdata(struct i2c_client *client);
+extern void pcf857x_put_pdata(struct i2c_client *client);
+#else
+#define pcf857x_get_pdata(client) ((client)->dev.platform_data)
+#define pcf857x_put_pdata(client)
+#endif
+
 #endif /* __LINUX_PCF857X_H */

  reply	other threads:[~2008-10-22 18:32 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
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 [this message]
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=20081022183218.GA19025@oksana.dev.rtsoft.ru \
    --to=avorontsov@ru.mvista.com \
    --cc=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=david-b@pacbell.net \
    --cc=i2c@lm-sensors.org \
    --cc=khali@linux-fr.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --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).