Re: [PATCH] I2C: BCM2835: Linking platform nodes to adapter nodes

Re: [PATCH] I2C: BCM2835: Linking platform nodes to adapter nodes

[Stephen Warren]

On 11/08/2013 02:49 AM, Florian Meier wrote:
> In order to find I2C devices in the device tree, the platform nodes
> have to be known by the I2C core. Analogous to the i2c-omap driver
> this requires setting the dev.of_node parameter of the adapter.

(CCing the I2C maintainers...)

> diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c

> @@ -299,6 +299,7 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
>  	strlcpy(adap->name, "bcm2835 I2C adapter", sizeof(adap->name));
>  	adap->algo = &bcm2835_i2c_algo;
>  	adap->dev.parent = &pdev->dev;
> +	adap->dev.of_node = pdev->dev.of_node;

Ah, that makes sense. Thinking about it now, I'd only ever used i2cget
etc. to access I2C devices, rather than instantiating drivers from DT.

That all said, I wonder if the I2C core shouldn't do something like the
following inside i2c_add_adapter():

if (!adap->dev.of_node && adap->dev.parent)
	adap->dev.of_node = adap->dev.parent->of_node;

That would save every single I2C driver from having to set up this field
manually.

Re: [PATCH] I2C: BCM2835: Linking platform nodes to adapter nodes

[Stephen Warren]

On 11/08/2013 09:59 AM, Stephen Warren wrote:
> On 11/08/2013 02:49 AM, Florian Meier wrote:
>> In order to find I2C devices in the device tree, the platform nodes
>> have to be known by the I2C core. Analogous to the i2c-omap driver
>> this requires setting the dev.of_node parameter of the adapter.
> 
> (CCing the I2C maintainers...)
> 
>> diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
> 
>> @@ -299,6 +299,7 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
>>  	strlcpy(adap->name, "bcm2835 I2C adapter", sizeof(adap->name));
>>  	adap->algo = &bcm2835_i2c_algo;
>>  	adap->dev.parent = &pdev->dev;
>> +	adap->dev.of_node = pdev->dev.of_node;
> 
> Ah, that makes sense. Thinking about it now, I'd only ever used i2cget
> etc. to access I2C devices, rather than instantiating drivers from DT.
> 
> That all said, I wonder if the I2C core shouldn't do something like the
> following inside i2c_add_adapter():
> 
> if (!adap->dev.of_node && adap->dev.parent)
> 	adap->dev.of_node = adap->dev.parent->of_node;
> 
> That would save every single I2C driver from having to set up this field
> manually.

BTW, this should probably be:
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Re: [PATCH] I2C: BCM2835: Linking platform nodes to adapter nodes

[Charles Keepax]

On Fri, Nov 08, 2013 at 09:59:28AM -0700, Stephen Warren wrote:
> On 11/08/2013 02:49 AM, Florian Meier wrote:
> > In order to find I2C devices in the device tree, the platform nodes
> > have to be known by the I2C core. Analogous to the i2c-omap driver
> > this requires setting the dev.of_node parameter of the adapter.
> 
> (CCing the I2C maintainers...)
> 
> > diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
> 
> > @@ -299,6 +299,7 @@ static int bcm2835_i2c_probe(struct platform_device *pdev)
> >  	strlcpy(adap->name, "bcm2835 I2C adapter", sizeof(adap->name));
> >  	adap->algo = &bcm2835_i2c_algo;
> >  	adap->dev.parent = &pdev->dev;
> > +	adap->dev.of_node = pdev->dev.of_node;
> 
> Ah, that makes sense. Thinking about it now, I'd only ever used i2cget
> etc. to access I2C devices, rather than instantiating drivers from DT.
> 
> That all said, I wonder if the I2C core shouldn't do something like the
> following inside i2c_add_adapter():
> 
> if (!adap->dev.of_node && adap->dev.parent)
> 	adap->dev.of_node = adap->dev.parent->of_node;

Should this not also have an of_node_get to increment the ref
count on the node?

Thanks,
Charles

Re: [PATCH] I2C: BCM2835: Linking platform nodes to adapter nodes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

On Tue, Nov 26, 2013 at 01:05:53PM +0000, Charles Keepax wrote:
> On Fri, Nov 08, 2013 at 09:59:28AM -0700, Stephen Warren wrote:

> > That all said, I wonder if the I2C core shouldn't do something like the
> > following inside i2c_add_adapter():
> > 
> > if (!adap->dev.of_node && adap->dev.parent)
> > 	adap->dev.of_node = adap->dev.parent->of_node;

> Should this not also have an of_node_get to increment the ref
> count on the node?

Given that it's pointing to the device that registered the I2C adaptor
it should be able to assume that there's a reference already.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] I2C: BCM2835: Linking platform nodes to adapter nodes

[Florian Meier]

In order to find I2C devices in the device tree, the platform nodes
have to be known by the I2C core. This requires setting the
dev.of_node parameter of the adapter.

Signed-off-by: Florian Meier <florian.meier-oZ8rN/sblLk@public.gmane.org>
---

Since the general approach is not easy enough
(see [PATCH] i2c: Fallback to of_node of parent),
this patch adds the required line to the bcm2835 bus.

 drivers/i2c/busses/i2c-bcm2835.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-bcm2835.c
b/drivers/i2c/busses/i2c-bcm2835.c
index ea4b08f..8beecfa 100644
--- a/drivers/i2c/busses/i2c-bcm2835.c
+++ b/drivers/i2c/busses/i2c-bcm2835.c
@@ -299,6 +299,7 @@ static int bcm2835_i2c_probe(struct platform_device
*pdev)
 	strlcpy(adap->name, "bcm2835 I2C adapter", sizeof(adap->name));
 	adap->algo = &bcm2835_i2c_algo;
 	adap->dev.parent = &pdev->dev;
+	adap->dev.of_node = pdev->dev.of_node;

 	bcm2835_i2c_writel(i2c_dev, BCM2835_I2C_C, 0);

-- 1.7.9.5

Re: [PATCH] I2C: BCM2835: Linking platform nodes to adapter nodes

[Stephen Warren]

On 11/25/2013 01:01 AM, Florian Meier wrote:
> In order to find I2C devices in the device tree, the platform nodes
> have to be known by the I2C core. This requires setting the
> dev.of_node parameter of the adapter.

Acked-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Tested-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

(Interestingly, I just attached an I2C light sensor to my Pi a couple
days back, so ended up needing this commit, and the patches someone else
had very recently sent to add DT support to the sensor driver. My timing
was impeccable:-)

Re: [PATCH] I2C: BCM2835: Linking platform nodes to adapter nodes

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

On Mon, Nov 25, 2013 at 09:01:50AM +0100, Florian Meier wrote:
> In order to find I2C devices in the device tree, the platform nodes
> have to be known by the I2C core. This requires setting the
> dev.of_node parameter of the adapter.
> 
> Signed-off-by: Florian Meier <florian.meier-oZ8rN/sblLk@public.gmane.org>

Malformed patch, please resend.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] I2C: BCM2835: Linking platform nodes to adapter nodes

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

On Mon, Nov 25, 2013 at 09:01:50AM +0100, Florian Meier wrote:
> In order to find I2C devices in the device tree, the platform nodes
> have to be known by the I2C core. This requires setting the
> dev.of_node parameter of the adapter.
> 
> Signed-off-by: Florian Meier <florian.meier-oZ8rN/sblLk@public.gmane.org>

Fixed up for you and applied to for-current. But PLEASE fix your mailer!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] I2C: BCM2835: Linking platform nodes to adapter nodes

[Florian Meier]

Thank you very much!
I am sorry and will try to make it better next time.

On 11/28/2013 09:50 AM, Wolfram Sang wrote:
> On Mon, Nov 25, 2013 at 09:01:50AM +0100, Florian Meier wrote:
>> In order to find I2C devices in the device tree, the platform nodes
>> have to be known by the I2C core. This requires setting the
>> dev.of_node parameter of the adapter.
>>
>> Signed-off-by: Florian Meier <florian.meier@koalo.de>
> 
> Fixed up for you and applied to for-current. But PLEASE fix your mailer!
>