Patchwork [v1] Add support for getting device platform data to I2C device

login
register
mail settings
Submitter Mingkai Hu
Date Dec. 18, 2008, 2:14 a.m.
Message ID <1229566451-29411-1-git-send-email-Mingkai.hu@freescale.com>
Download mbox | patch
Permalink /patch/14613/
State Changes Requested, archived
Headers show

Comments

Mingkai Hu - Dec. 18, 2008, 2:14 a.m.
There is no standard way of getting device platform data from
dts file to the I2C device when it's probed. This patch adds
a function pointer to the of_modalias_table to get such bindings
properties into device's platform data.

Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
---
Thanks to Scott Wood's suggestions.

 drivers/of/base.c   |    9 ++++++++-
 drivers/of/of_i2c.c |    3 ++-
 drivers/of/of_spi.c |    2 +-
 include/linux/of.h  |    3 ++-
 4 files changed, 13 insertions(+), 4 deletions(-)
Anton Vorontsov - Dec. 18, 2008, 4:59 p.m.
On Thu, Dec 18, 2008 at 10:14:11AM +0800, Mingkai Hu wrote:
> There is no standard way of getting device platform data from
> dts file to the I2C device when it's probed. This patch adds
> a function pointer to the of_modalias_table to get such bindings
> properties into device's platform data.

Unfortunately this approach leads to inability to make the bindings
modular.

Note that distributions compile kernels with allmodconfig*, so the
kernel builds with all modules, but _all_ these bindings will be
kernel's built-ins. Given that these bindings are needed for every
not-so-trivial I2C (or SPI) driver, the approach doesn't seem to
scale good enough.

You can find more discussion on this matter in this huge thread:
http://lkml.org/lkml/2008/10/16/248

And the solution that everybody seem to agree with (SPI driver example):
http://lkml.org/lkml/2008/10/30/393

* Well, this is not quite true, but they're very close to allmodconfig.
Scott Wood - Dec. 18, 2008, 5:20 p.m.
Anton Vorontsov wrote:
> On Thu, Dec 18, 2008 at 10:14:11AM +0800, Mingkai Hu wrote:
>> There is no standard way of getting device platform data from
>> dts file to the I2C device when it's probed. This patch adds
>> a function pointer to the of_modalias_table to get such bindings
>> properties into device's platform data.
> 
> Unfortunately this approach leads to inability to make the bindings
> modular.

True, but I think it's fine for now while a better mechanism is worked 
out -- especially for things that are mainly used in embedded situations 
where modular builds aren't as necessary.

> And the solution that everybody seem to agree with (SPI driver example):
> http://lkml.org/lkml/2008/10/30/393

Hmm, that doesn't seem to allow for any binding mechanism other than 
internal and OF, and it seems like it could be done more 
straightforwardly by having the modalias point to the OF wrapper, which 
then instantiates the core driver.

But the above approach has the distinct advantage of having already been 
implemented. :-)

-Scott
Anton Vorontsov - Dec. 18, 2008, 5:49 p.m.
On Thu, Dec 18, 2008 at 11:20:03AM -0600, Scott Wood wrote:
> Anton Vorontsov wrote:
>> On Thu, Dec 18, 2008 at 10:14:11AM +0800, Mingkai Hu wrote:
>>> There is no standard way of getting device platform data from
>>> dts file to the I2C device when it's probed. This patch adds
>>> a function pointer to the of_modalias_table to get such bindings
>>> properties into device's platform data.
>>
>> Unfortunately this approach leads to inability to make the bindings
>> modular.
>
> True, but I think it's fine for now while a better mechanism is worked  
> out -- especially for things that are mainly used in embedded situations  
> where modular builds aren't as necessary.

Exactly, this matters for non-embedded case. Why would I want totally
unneeded I2C bindings built-in into my iBook kernel? ;-)

>> And the solution that everybody seem to agree with (SPI driver example):
>> http://lkml.org/lkml/2008/10/30/393
>
> Hmm, that doesn't seem to allow for any binding mechanism other than  
> internal and OF

Yeah, not without hacks (though, we can do some sort of chained pdata
handlers, thus we can allow other bindings mechanisms). But so far we
don't have anything other than OF and "board files"/raw bindings (I can't
actually imagine any other option).

Both approaches have their cons, sure. The difference is: for the
$subject approach we'll see the cons immediately, while in my approach
the cons are theoretical.
Benjamin Herrenschmidt - Jan. 7, 2009, 12:55 a.m.
On Thu, 2008-12-18 at 20:49 +0300, Anton Vorontsov wrote:

> Exactly, this matters for non-embedded case. Why would I want totally
> unneeded I2C bindings built-in into my iBook kernel? ;-)
> 
> >> And the solution that everybody seem to agree with (SPI driver example):
> >> http://lkml.org/lkml/2008/10/30/393
> >
> > Hmm, that doesn't seem to allow for any binding mechanism other than  
> > internal and OF
> 
> Yeah, not without hacks (though, we can do some sort of chained pdata
> handlers, thus we can allow other bindings mechanisms). But so far we
> don't have anything other than OF and "board files"/raw bindings (I can't
> actually imagine any other option).
> 
> Both approaches have their cons, sure. The difference is: for the
> $subject approach we'll see the cons immediately, while in my approach
> the cons are theoretical.

So what's the situation with this patch ? In general, who should be
considered as "in charge" of the OF i2c stuff btw ?

Cheers,
Ben.

Patch

diff --git a/drivers/of/base.c b/drivers/of/base.c
index ad8ac1a..38ab2a2 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -408,6 +408,7 @@  EXPORT_SYMBOL(of_find_matching_node);
 struct of_modalias_table {
 	char *of_device;
 	char *modalias;
+	const void *(*binder)(struct device_node *node);
 };
 static struct of_modalias_table of_modalias_table[] = {
 	/* Empty for now; add entries as needed */
@@ -418,6 +419,7 @@  static struct of_modalias_table of_modalias_table[] = {
  * @node:	pointer to a device tree node
  * @modalias:	Pointer to buffer that modalias value will be copied into
  * @len:	Length of modalias value
+ * @platform_data: pointer to device's platform specific data
  *
  * Based on the value of the compatible property, this routine will determine
  * an appropriate modalias value for a particular device tree node.  Three
@@ -430,7 +432,8 @@  static struct of_modalias_table of_modalias_table[] = {
  *
  * This routine returns 0 on success
  */
-int of_modalias_node(struct device_node *node, char *modalias, int len)
+int of_modalias_node(struct device_node *node, char *modalias, int len,
+	const void **platform_data)
 {
 	int i, cplen;
 	const char *compatible;
@@ -442,6 +445,10 @@  int of_modalias_node(struct device_node *node, char *modalias, int len)
 		if (!of_device_is_compatible(node, compatible))
 			continue;
 		strlcpy(modalias, of_modalias_table[i].modalias, len);
+
+		if (platform_data && of_modalias_table[i].binder)
+			*platform_data = of_modalias_table[i].binder(node);
+
 		return 0;
 	}
 
diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
index 6a98dc8..38d9faf 100644
--- a/drivers/of/of_i2c.c
+++ b/drivers/of/of_i2c.c
@@ -27,7 +27,8 @@  void of_register_i2c_devices(struct i2c_adapter *adap,
 		const u32 *addr;
 		int len;
 
-		if (of_modalias_node(node, info.type, sizeof(info.type)) < 0)
+		if (of_modalias_node(node, info.type, sizeof(info.type),
+			(const void **)&info.platform_data) < 0)
 			continue;
 
 		addr = of_get_property(node, "reg", &len);
diff --git a/drivers/of/of_spi.c b/drivers/of/of_spi.c
index b01eec0..0dcd52c 100644
--- a/drivers/of/of_spi.c
+++ b/drivers/of/of_spi.c
@@ -39,7 +39,7 @@  void of_register_spi_devices(struct spi_master *master, struct device_node *np)
 
 		/* Select device driver */
 		if (of_modalias_node(nc, spi->modalias,
-				     sizeof(spi->modalias)) < 0) {
+				     sizeof(spi->modalias), NULL) < 0) {
 			dev_err(&master->dev, "cannot find modalias for %s\n",
 				nc->full_name);
 			spi_dev_put(spi);
diff --git a/include/linux/of.h b/include/linux/of.h
index 79886ad..ed4c269 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -70,6 +70,7 @@  extern int of_n_addr_cells(struct device_node *np);
 extern int of_n_size_cells(struct device_node *np);
 extern const struct of_device_id *of_match_node(
 	const struct of_device_id *matches, const struct device_node *node);
-extern int of_modalias_node(struct device_node *node, char *modalias, int len);
+extern int of_modalias_node(struct device_node *node, char *modalias, int len,
+	const void **platform_data);
 
 #endif /* _LINUX_OF_H */