Patchwork Add the of_find_i2c_device_by_node function

login
register
mail settings
Submitter Jon Smirl
Date Oct. 19, 2008, 2 p.m.
Message ID <20081019140040.32281.65209.stgit@terra>
Download mbox | patch
Permalink /patch/5033/
State Superseded
Headers show

Comments

Jon Smirl - Oct. 19, 2008, 2 p.m.
Add the of_find_i2c_device_by_node function. This allows you to follow
a reference in the device tree to an i2c device node and then locate
the linux device instantiated by the device tree. Example use, an i2s
codec controlled by i2c. Depends on patch exporting i2c root bus symbol.

Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
---
 drivers/of/of_i2c.c |   28 ++++++++++++++++++++++++----
 1 files changed, 24 insertions(+), 4 deletions(-)
Anton Vorontsov - Oct. 19, 2008, 9:20 p.m.
Hi Jon,

On Sun, Oct 19, 2008 at 10:00:40AM -0400, Jon Smirl wrote:
> Add the of_find_i2c_device_by_node function. This allows you to follow
> a reference in the device tree to an i2c device node and then locate
> the linux device instantiated by the device tree. Example use, an i2s
> codec controlled by i2c. Depends on patch exporting i2c root bus symbol.
> 
> Signed-off-by: Jon Smirl <jonsmirl@gmail.com>

Few comments are below.

> ---
>  drivers/of/of_i2c.c |   28 ++++++++++++++++++++++++----
>  1 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
> index 6a98dc8..ba7b394 100644
> --- a/drivers/of/of_i2c.c
> +++ b/drivers/of/of_i2c.c
> @@ -19,7 +19,7 @@
>  void of_register_i2c_devices(struct i2c_adapter *adap,
>  			     struct device_node *adap_node)
>  {
> -	void *result;
> +	struct i2c_client *i2c_dev;
>  	struct device_node *node;
>  
>  	for_each_child_of_node(adap_node, node) {
> @@ -41,18 +41,38 @@ void of_register_i2c_devices(struct i2c_adapter *adap,
>  
>  		info.addr = *addr;
>  
> -		request_module(info.type);
> +		request_module("%s", info.type);

Patch description doesn't mention this change.

>  
> -		result = i2c_new_device(adap, &info);
> -		if (result == NULL) {
> +		i2c_dev = i2c_new_device(adap, &info);
> +		if (i2c_dev == NULL) {
>  			printk(KERN_ERR
>  			       "of-i2c: Failed to load driver for %s\n",
>  			       info.type);
>  			irq_dispose_mapping(info.irq);
>  			continue;
>  		}
> +		
> +		i2c_dev->dev.archdata.of_node = of_node_get(node);

Would break sparc build. Plus setting this after i2c_new_device() isn't
right... Recently I sent few patches to deal with the archdata, could
you please rebase your patch against these three patches?

http://lkml.org/lkml/2008/10/16/250
http://lkml.org/lkml/2008/10/16/251
http://lkml.org/lkml/2008/10/16/252

>  	}
>  }
>  EXPORT_SYMBOL(of_register_i2c_devices);
>  
> +static int of_dev_node_match(struct device *dev, void *data)
> +{
> +        return dev->archdata.of_node == data;
> +}
> +
> +struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)

This should be documented. Especially the fact that every time you
call this function, you must call device_put() when you're done with
the returned i2c_client.

> +{
> +	struct device *dev;
> +	
> +	dev = bus_find_device(&i2c_bus_type, NULL, node, 
> +					 of_dev_node_match);
> +	if (!dev)
> +		return NULL;
> +		
> +	return to_i2c_client(dev);
> +}
> +EXPORT_SYMBOL(of_find_i2c_device_by_node);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/include/linux/of_i2c.h b/include/linux/of_i2c.h
> index bd2a870..17d5897 100644
> --- a/include/linux/of_i2c.h
> +++ b/include/linux/of_i2c.h
> @@ -16,5 +16,7 @@
>  
>  void of_register_i2c_devices(struct i2c_adapter *adap,
>  			     struct device_node *adap_node);
> +struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
> +			     
>  
>  #endif /* __LINUX_OF_I2C_H */

Thanks,
Jon Smirl - Oct. 19, 2008, 9:50 p.m.
On Sun, Oct 19, 2008 at 5:20 PM, Anton Vorontsov
<avorontsov@ru.mvista.com> wrote:
> Hi Jon,
>
> On Sun, Oct 19, 2008 at 10:00:40AM -0400, Jon Smirl wrote:
>> Add the of_find_i2c_device_by_node function. This allows you to follow
>> a reference in the device tree to an i2c device node and then locate
>> the linux device instantiated by the device tree. Example use, an i2s
>> codec controlled by i2c. Depends on patch exporting i2c root bus symbol.
>>
>> Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
>
> Few comments are below.
>
>> ---
>>  drivers/of/of_i2c.c |   28 ++++++++++++++++++++++++----
>>  1 files changed, 24 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
>> index 6a98dc8..ba7b394 100644
>> --- a/drivers/of/of_i2c.c
>> +++ b/drivers/of/of_i2c.c
>> @@ -19,7 +19,7 @@
>>  void of_register_i2c_devices(struct i2c_adapter *adap,
>>                            struct device_node *adap_node)
>>  {
>> -     void *result;
>> +     struct i2c_client *i2c_dev;
>>       struct device_node *node;
>>
>>       for_each_child_of_node(adap_node, node) {
>> @@ -41,18 +41,38 @@ void of_register_i2c_devices(struct i2c_adapter *adap,
>>
>>               info.addr = *addr;
>>
>> -             request_module(info.type);
>> +             request_module("%s", info.type);
>
> Patch description doesn't mention this change.

Patches for this have been posted before by other people and they
aren't making it in.

This is the original mail....
http://lkml.org/lkml/2008/6/13/290
http://lkml.org/lkml/2008/6/12/8
I can't find the ones patching i2c.


>
>>
>> -             result = i2c_new_device(adap, &info);
>> -             if (result == NULL) {
>> +             i2c_dev = i2c_new_device(adap, &info);
>> +             if (i2c_dev == NULL) {
>>                       printk(KERN_ERR
>>                              "of-i2c: Failed to load driver for %s\n",
>>                              info.type);
>>                       irq_dispose_mapping(info.irq);
>>                       continue;
>>               }
>> +
>> +             i2c_dev->dev.archdata.of_node = of_node_get(node);
>
> Would break sparc build. Plus setting this after i2c_new_device() isn't
> right... Recently I sent few patches to deal with the archdata, could
> you please rebase your patch against these three patches?
>
> http://lkml.org/lkml/2008/10/16/250
> http://lkml.org/lkml/2008/10/16/251
> http://lkml.org/lkml/2008/10/16/252
>
>>       }
>>  }
>>  EXPORT_SYMBOL(of_register_i2c_devices);
>>
>> +static int of_dev_node_match(struct device *dev, void *data)
>> +{
>> +        return dev->archdata.of_node == data;
>> +}
>> +
>> +struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
>
> This should be documented. Especially the fact that every time you
> call this function, you must call device_put() when you're done with
> the returned i2c_client.
>
>> +{
>> +     struct device *dev;
>> +
>> +     dev = bus_find_device(&i2c_bus_type, NULL, node,
>> +                                      of_dev_node_match);
>> +     if (!dev)
>> +             return NULL;
>> +
>> +     return to_i2c_client(dev);
>> +}
>> +EXPORT_SYMBOL(of_find_i2c_device_by_node);
>> +
>>  MODULE_LICENSE("GPL");
>> diff --git a/include/linux/of_i2c.h b/include/linux/of_i2c.h
>> index bd2a870..17d5897 100644
>> --- a/include/linux/of_i2c.h
>> +++ b/include/linux/of_i2c.h
>> @@ -16,5 +16,7 @@
>>
>>  void of_register_i2c_devices(struct i2c_adapter *adap,
>>                            struct device_node *adap_node);
>> +struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
>> +
>>
>>  #endif /* __LINUX_OF_I2C_H */
>
> Thanks,
>
> --
> Anton Vorontsov
> email: cbouatmailru@gmail.com
> irc://irc.freenode.net/bd2
>
Anton Vorontsov - Oct. 19, 2008, 10:03 p.m.
On Sun, Oct 19, 2008 at 05:50:15PM -0400, Jon Smirl wrote:
> On Sun, Oct 19, 2008 at 5:20 PM, Anton Vorontsov
> <avorontsov@ru.mvista.com> wrote:
> > Hi Jon,
> >
> > On Sun, Oct 19, 2008 at 10:00:40AM -0400, Jon Smirl wrote:
> >> Add the of_find_i2c_device_by_node function. This allows you to follow
> >> a reference in the device tree to an i2c device node and then locate
> >> the linux device instantiated by the device tree. Example use, an i2s
> >> codec controlled by i2c. Depends on patch exporting i2c root bus symbol.
> >>
> >> Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
> >
> > Few comments are below.
> >
> >> ---
> >>  drivers/of/of_i2c.c |   28 ++++++++++++++++++++++++----
> >>  1 files changed, 24 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
> >> index 6a98dc8..ba7b394 100644
> >> --- a/drivers/of/of_i2c.c
> >> +++ b/drivers/of/of_i2c.c
> >> @@ -19,7 +19,7 @@
> >>  void of_register_i2c_devices(struct i2c_adapter *adap,
> >>                            struct device_node *adap_node)
> >>  {
> >> -     void *result;
> >> +     struct i2c_client *i2c_dev;
> >>       struct device_node *node;
> >>
> >>       for_each_child_of_node(adap_node, node) {
> >> @@ -41,18 +41,38 @@ void of_register_i2c_devices(struct i2c_adapter *adap,
> >>
> >>               info.addr = *addr;
> >>
> >> -             request_module(info.type);
> >> +             request_module("%s", info.type);
> >
> > Patch description doesn't mention this change.
> 
> Patches for this have been posted before by other people and they
> aren't making it in.
> 
> This is the original mail....
> http://lkml.org/lkml/2008/6/13/290
> http://lkml.org/lkml/2008/6/12/8
> I can't find the ones patching i2c.

Well, I didn't disagree with the change. I think this is good change
(and good catch, btw).

Just mention it in the patch description (or better yet, you could send
a separate patch for this particular issue, it seems quite serious).


Thanks,
Benjamin Herrenschmidt - Oct. 20, 2008, 5:19 a.m.
On Mon, 2008-10-20 at 02:03 +0400, Anton Vorontsov wrote:
> > This is the original mail....
> > http://lkml.org/lkml/2008/6/13/290
> > http://lkml.org/lkml/2008/6/12/8
> > I can't find the ones patching i2c.
> 
> Well, I didn't disagree with the change. I think this is good change
> (and good catch, btw).
> 
> Just mention it in the patch description (or better yet, you could send
> a separate patch for this particular issue, it seems quite serious).

Yes, it should most definitely be a separate patch.

Cheers,
Ben.

Patch

diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
index 6a98dc8..ba7b394 100644
--- a/drivers/of/of_i2c.c
+++ b/drivers/of/of_i2c.c
@@ -19,7 +19,7 @@ 
 void of_register_i2c_devices(struct i2c_adapter *adap,
 			     struct device_node *adap_node)
 {
-	void *result;
+	struct i2c_client *i2c_dev;
 	struct device_node *node;
 
 	for_each_child_of_node(adap_node, node) {
@@ -41,18 +41,38 @@  void of_register_i2c_devices(struct i2c_adapter *adap,
 
 		info.addr = *addr;
 
-		request_module(info.type);
+		request_module("%s", info.type);
 
-		result = i2c_new_device(adap, &info);
-		if (result == NULL) {
+		i2c_dev = i2c_new_device(adap, &info);
+		if (i2c_dev == NULL) {
 			printk(KERN_ERR
 			       "of-i2c: Failed to load driver for %s\n",
 			       info.type);
 			irq_dispose_mapping(info.irq);
 			continue;
 		}
+		
+		i2c_dev->dev.archdata.of_node = of_node_get(node);
 	}
 }
 EXPORT_SYMBOL(of_register_i2c_devices);
 
+static int of_dev_node_match(struct device *dev, void *data)
+{
+        return dev->archdata.of_node == data;
+}
+
+struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
+{
+	struct device *dev;
+	
+	dev = bus_find_device(&i2c_bus_type, NULL, node, 
+					 of_dev_node_match);
+	if (!dev)
+		return NULL;
+		
+	return to_i2c_client(dev);
+}
+EXPORT_SYMBOL(of_find_i2c_device_by_node);
+
 MODULE_LICENSE("GPL");
diff --git a/include/linux/of_i2c.h b/include/linux/of_i2c.h
index bd2a870..17d5897 100644
--- a/include/linux/of_i2c.h
+++ b/include/linux/of_i2c.h
@@ -16,5 +16,7 @@ 
 
 void of_register_i2c_devices(struct i2c_adapter *adap,
 			     struct device_node *adap_node);
+struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
+			     
 
 #endif /* __LINUX_OF_I2C_H */