diff mbox

I2C: BCM2835: Linking platform nodes to adapter nodes

Message ID 527CB3A1.7050808@koalo.de (mailing list archive)
State New, archived
Headers show

Commit Message

Florian Meier Nov. 8, 2013, 9:49 a.m. UTC
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.

Signed-off-by: Florian Meier <florian.meier@koalo.de>
---

I don't know if this is really necessary, but for me it does not work
without this line.

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

Comments

Stephen Warren Nov. 8, 2013, 4:59 p.m. UTC | #1
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.
Stephen Warren Nov. 26, 2013, 3:31 a.m. UTC | #2
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@vger.kernel.org
Charles Keepax Nov. 26, 2013, 1:05 p.m. UTC | #3
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
Mark Brown Nov. 28, 2013, 5:13 p.m. UTC | #4
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.
diff mbox

Patch

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);