[PATCH 0/2] i2c: Don't assume bus nr 0 if none was specified

[PATCH 0/2] i2c: Don't assume bus nr 0 if none was specified

[Karol Lewandowski]

Hi,

i2c controller drivers used to assume bus number 0 when none (-1) was specified.

This worked on non-device tree systems, where one could explicitly specify
bus number via platform data.  On DT-enabled systems bus number is always -1.

Some drivers assume bus number 0 when -1 is specified.  This patchset kills
this logic and switches to dynamic bus allocation (default when -1 is provided[1]).

[ I've found out this when I've tried to instantiate more than one i2c-gpio
  instance on DT-enabled system. I've, hopefully, fixed few other drivers too. ]

[1] Introduced by Grant Likely in 488bf314b ("i2c: Allow i2c_add_numbered_adapter()
    to assign a bus id")

Karol Lewandowski (2):
  i2c-pxa: Drop leftover comment
  i2c: Dynamically assign adapter id if it wasn't explictly specified

 drivers/i2c/busses/i2c-gpio.c         |    7 +------
 drivers/i2c/busses/i2c-octeon.c       |    2 +-
 drivers/i2c/busses/i2c-pca-platform.c |    2 +-
 drivers/i2c/busses/i2c-pxa.c          |    5 -----
 drivers/i2c/busses/i2c-versatile.c    |    9 ++-------
 5 files changed, 5 insertions(+), 20 deletions(-)

-- 
1.7.9

[PATCH 1/2] i2c-pxa: Drop leftover comment

[Karol Lewandowski]

Commit 488bf314b ("i2c: Allow i2c_add_numbered_adapter() to assign a
bus id") reworked i2c-pxa driver leaving obsolete comment.

This commit simply drops it.

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/i2c/busses/i2c-pxa.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index d603646..b81fd10 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1074,11 +1074,6 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	spin_lock_init(&i2c->lock);
 	init_waitqueue_head(&i2c->wait);
 
-	/*
-	 * If "dev->id" is negative we consider it as zero.
-	 * The reason to do so is to avoid sysfs names that only make
-	 * sense when there are multiple adapters.
-	 */
 	i2c->adap.nr = dev->id;
 	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "pxa_i2c-i2c.%u",
 		 i2c->adap.nr);
-- 
1.7.9

[PATCH 2/2] i2c: Dynamically assign adapter id if it wasn't explictly specified

[Karol Lewandowski]

Commit 488bf314b ("i2c: Allow i2c_add_numbered_adapter() to assign a
bus id") reworked i2c_add_numbered_adapter() to call i2c_add_adapter()
if requested bus was -1.

This allows to simplify driver's initialization procedure as it allows
static and dynamic adapter id registration using one function.

This patch updates few more drivers (missed out in original patch) to
use this functionality.

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/i2c/busses/i2c-gpio.c         |    7 +------
 drivers/i2c/busses/i2c-octeon.c       |    2 +-
 drivers/i2c/busses/i2c-pca-platform.c |    2 +-
 drivers/i2c/busses/i2c-versatile.c    |    9 ++-------
 4 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index a651779..50a2a94 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -144,12 +144,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	adap->dev.parent = &pdev->dev;
 
-	/*
-	 * If "dev->id" is negative we consider it as zero.
-	 * The reason to do so is to avoid sysfs names that only make
-	 * sense when there are multiple adapters.
-	 */
-	adap->nr = (pdev->id != -1) ? pdev->id : 0;
+	adap->nr = pdev->id;
 	ret = i2c_bit_add_numbered_bus(adap);
 	if (ret)
 		goto err_add_bus;
diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c
index ee139a5..8470232 100644
--- a/drivers/i2c/busses/i2c-octeon.c
+++ b/drivers/i2c/busses/i2c-octeon.c
@@ -581,7 +581,7 @@ static int __devinit octeon_i2c_probe(struct platform_device *pdev)
 
 	i2c->adap = octeon_i2c_ops;
 	i2c->adap.dev.parent = &pdev->dev;
-	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
+	i2c->adap.nr = pdev->id;
 	i2c_set_adapdata(&i2c->adap, i2c);
 	platform_set_drvdata(pdev, i2c);
 
diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c
index 2adbf1a..675878f 100644
--- a/drivers/i2c/busses/i2c-pca-platform.c
+++ b/drivers/i2c/busses/i2c-pca-platform.c
@@ -171,7 +171,7 @@ static int __devinit i2c_pca_pf_probe(struct platform_device *pdev)
 	i2c->io_size = resource_size(res);
 	i2c->irq = irq;
 
-	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
+	i2c->adap.nr = pdev->id;
 	i2c->adap.owner = THIS_MODULE;
 	snprintf(i2c->adap.name, sizeof(i2c->adap.name),
 		 "PCA9564/PCA9665 at 0x%08lx",
diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
index 6055601..9458568 100644
--- a/drivers/i2c/busses/i2c-versatile.c
+++ b/drivers/i2c/busses/i2c-versatile.c
@@ -102,13 +102,8 @@ static int i2c_versatile_probe(struct platform_device *dev)
 	i2c->algo = i2c_versatile_algo;
 	i2c->algo.data = i2c;
 
-	if (dev->id >= 0) {
-		/* static bus numbering */
-		i2c->adap.nr = dev->id;
-		ret = i2c_bit_add_numbered_bus(&i2c->adap);
-	} else
-		/* dynamic bus numbering */
-		ret = i2c_bit_add_bus(&i2c->adap);
+	i2c->adap.nr = dev->id;
+	ret = i2c_bit_add_numbered_bus(&i2c->adap);
 	if (ret >= 0) {
 		platform_set_drvdata(dev, i2c);
 		return 0;
-- 
1.7.9

Re: [PATCH 2/2] i2c: Dynamically assign adapter id if it wasn't explictly specified

[Karol Lewandowski]

On 16.03.2012 13:19, Karol Lewandowski wrote:

>> Commit 488bf314b ("i2c: Allow i2c_add_numbered_adapter() to assign a
>> bus id") reworked i2c_add_numbered_adapter() to call i2c_add_adapter()
>> if requested bus was -1.

>> This allows to simplify driver's initialization procedure as it allows

>> static and dynamic adapter id registration using one function.
>> 
>> This patch updates few more drivers (missed out in original patch) to
>> use this functionality.
>

> [ I must have lost actual problem description while rewording
>  message itself... ]
>
> Problem arises when multiple drivers (or multiple instances
> of one driver) try to assume the same fixed bus number (0).
>
> This simply causes i2c_add_numbered_bus() to fail.
> Leaving -1 works perfectly, as registration function switches
> to dynamic id registration.

>> Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  drivers/i2c/busses/i2c-gpio.c         |    7 +------
>>  drivers/i2c/busses/i2c-octeon.c       |    2 +-
>>  drivers/i2c/busses/i2c-pca-platform.c |    2 +-
>>  drivers/i2c/busses/i2c-versatile.c    |    9 ++-------
>>  4 files changed, 5 insertions(+), 15 deletions(-)


Dear Haavard, Rade, Wolfram and Russel

Could you review following changes for gpio, octeon, pca-platform
and versatile i2c controller drivers (for which you are, or were,
maintainers)?

Grant requested explicit Ack to get this merged.

Thanks!

> 
> diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> index a651779..50a2a94 100644
> --- a/drivers/i2c/busses/i2c-gpio.c
> +++ b/drivers/i2c/busses/i2c-gpio.c
> @@ -144,12 +144,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
>  	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
>  	adap->dev.parent = &pdev->dev;
>  
> -	/*
> -	 * If "dev->id" is negative we consider it as zero.
> -	 * The reason to do so is to avoid sysfs names that only make
> -	 * sense when there are multiple adapters.
> -	 */
> -	adap->nr = (pdev->id != -1) ? pdev->id : 0;
> +	adap->nr = pdev->id;
>  	ret = i2c_bit_add_numbered_bus(adap);
>  	if (ret)
>  		goto err_add_bus;
> diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c
> index ee139a5..8470232 100644
> --- a/drivers/i2c/busses/i2c-octeon.c
> +++ b/drivers/i2c/busses/i2c-octeon.c
> @@ -581,7 +581,7 @@ static int __devinit octeon_i2c_probe(struct platform_device *pdev)
>  
>  	i2c->adap = octeon_i2c_ops;
>  	i2c->adap.dev.parent = &pdev->dev;
> -	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
> +	i2c->adap.nr = pdev->id;
>  	i2c_set_adapdata(&i2c->adap, i2c);
>  	platform_set_drvdata(pdev, i2c);
>  
> diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c
> index 2adbf1a..675878f 100644
> --- a/drivers/i2c/busses/i2c-pca-platform.c
> +++ b/drivers/i2c/busses/i2c-pca-platform.c
> @@ -171,7 +171,7 @@ static int __devinit i2c_pca_pf_probe(struct platform_device *pdev)
>  	i2c->io_size = resource_size(res);
>  	i2c->irq = irq;
>  
> -	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
> +	i2c->adap.nr = pdev->id;
>  	i2c->adap.owner = THIS_MODULE;
>  	snprintf(i2c->adap.name, sizeof(i2c->adap.name),
>  		 "PCA9564/PCA9665 at 0x%08lx",
> diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
> index 6055601..9458568 100644
> --- a/drivers/i2c/busses/i2c-versatile.c
> +++ b/drivers/i2c/busses/i2c-versatile.c
> @@ -102,13 +102,8 @@ static int i2c_versatile_probe(struct platform_device *dev)
>  	i2c->algo = i2c_versatile_algo;
>  	i2c->algo.data = i2c;
>  
> -	if (dev->id >= 0) {
> -		/* static bus numbering */
> -		i2c->adap.nr = dev->id;
> -		ret = i2c_bit_add_numbered_bus(&i2c->adap);
> -	} else
> -		/* dynamic bus numbering */
> -		ret = i2c_bit_add_bus(&i2c->adap);
> +	i2c->adap.nr = dev->id;
> +	ret = i2c_bit_add_numbered_bus(&i2c->adap);
>  	if (ret >= 0) {
>  		platform_set_drvdata(dev, i2c);
>  		return 0;



-- 
Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform

Re: [PATCH 2/2] i2c: Dynamically assign adapter id if it wasn't explictly specified

[Russell King - ARM Linux]

On Thu, Mar 22, 2012 at 04:28:50PM +0100, Karol Lewandowski wrote:
> Dear Haavard, Rade, Wolfram and Russel
> 
> Could you review following changes for gpio, octeon, pca-platform
> and versatile i2c controller drivers (for which you are, or were,
> maintainers)?
> 
> Grant requested explicit Ack to get this merged.

All I have is the patch below, which doesn't make sense on its own.  Where
is patch 1, which is presumably the core changes ?

> 
> Thanks!
> 
> > 
> > diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> > index a651779..50a2a94 100644
> > --- a/drivers/i2c/busses/i2c-gpio.c
> > +++ b/drivers/i2c/busses/i2c-gpio.c
> > @@ -144,12 +144,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
> >  	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
> >  	adap->dev.parent = &pdev->dev;
> >  
> > -	/*
> > -	 * If "dev->id" is negative we consider it as zero.
> > -	 * The reason to do so is to avoid sysfs names that only make
> > -	 * sense when there are multiple adapters.
> > -	 */
> > -	adap->nr = (pdev->id != -1) ? pdev->id : 0;
> > +	adap->nr = pdev->id;
> >  	ret = i2c_bit_add_numbered_bus(adap);
> >  	if (ret)
> >  		goto err_add_bus;
> > diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c
> > index ee139a5..8470232 100644
> > --- a/drivers/i2c/busses/i2c-octeon.c
> > +++ b/drivers/i2c/busses/i2c-octeon.c
> > @@ -581,7 +581,7 @@ static int __devinit octeon_i2c_probe(struct platform_device *pdev)
> >  
> >  	i2c->adap = octeon_i2c_ops;
> >  	i2c->adap.dev.parent = &pdev->dev;
> > -	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
> > +	i2c->adap.nr = pdev->id;
> >  	i2c_set_adapdata(&i2c->adap, i2c);
> >  	platform_set_drvdata(pdev, i2c);
> >  
> > diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c
> > index 2adbf1a..675878f 100644
> > --- a/drivers/i2c/busses/i2c-pca-platform.c
> > +++ b/drivers/i2c/busses/i2c-pca-platform.c
> > @@ -171,7 +171,7 @@ static int __devinit i2c_pca_pf_probe(struct platform_device *pdev)
> >  	i2c->io_size = resource_size(res);
> >  	i2c->irq = irq;
> >  
> > -	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
> > +	i2c->adap.nr = pdev->id;
> >  	i2c->adap.owner = THIS_MODULE;
> >  	snprintf(i2c->adap.name, sizeof(i2c->adap.name),
> >  		 "PCA9564/PCA9665 at 0x%08lx",
> > diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
> > index 6055601..9458568 100644
> > --- a/drivers/i2c/busses/i2c-versatile.c
> > +++ b/drivers/i2c/busses/i2c-versatile.c
> > @@ -102,13 +102,8 @@ static int i2c_versatile_probe(struct platform_device *dev)
> >  	i2c->algo = i2c_versatile_algo;
> >  	i2c->algo.data = i2c;
> >  
> > -	if (dev->id >= 0) {
> > -		/* static bus numbering */
> > -		i2c->adap.nr = dev->id;
> > -		ret = i2c_bit_add_numbered_bus(&i2c->adap);
> > -	} else
> > -		/* dynamic bus numbering */
> > -		ret = i2c_bit_add_bus(&i2c->adap);
> > +	i2c->adap.nr = dev->id;
> > +	ret = i2c_bit_add_numbered_bus(&i2c->adap);
> >  	if (ret >= 0) {
> >  		platform_set_drvdata(dev, i2c);
> >  		return 0;
> 
> 
> 
> -- 
> Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform

Re: [PATCH 2/2] i2c: Dynamically assign adapter id if it wasn't explictly specified

[Karol Lewandowski]

On 22.03.2012 17:11, Russell King - ARM Linux wrote:

> On Thu, Mar 22, 2012 at 04:28:50PM +0100, Karol Lewandowski wrote:
>> Dear Haavard, Rade, Wolfram and Russel
>>
>> Could you review following changes for gpio, octeon, pca-platform
>> and versatile i2c controller drivers (for which you are, or were,
>> maintainers)?
>>
>> Grant requested explicit Ack to get this merged.
> 
> All I have is the patch below, which doesn't make sense on its own.  Where
> is patch 1, which is presumably the core changes ?


Grant's (core) patch has been merged into v3.1-rc1 - the one I've cited:

  Commit 488bf314b ("i2c: Allow i2c_add_numbered_adapter() to assign a
  bus id") reworked i2c_add_numbered_adapter() to call i2c_add_adapter()
  if requested bus was -1.

This patchset just cleans up the drivers after it.

[ All what patch 1 does is just to drop no longer applicable comment:

  http://permalink.gmane.org/gmane.linux.drivers.i2c/10349 ]

Thanks

>>> diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
>>> index a651779..50a2a94 100644
>>> --- a/drivers/i2c/busses/i2c-gpio.c
>>> +++ b/drivers/i2c/busses/i2c-gpio.c
>>> @@ -144,12 +144,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
>>>  	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
>>>  	adap->dev.parent = &pdev->dev;
>>>  
>>> -	/*
>>> -	 * If "dev->id" is negative we consider it as zero.
>>> -	 * The reason to do so is to avoid sysfs names that only make
>>> -	 * sense when there are multiple adapters.
>>> -	 */
>>> -	adap->nr = (pdev->id != -1) ? pdev->id : 0;
>>> +	adap->nr = pdev->id;
>>>  	ret = i2c_bit_add_numbered_bus(adap);
>>>  	if (ret)
>>>  		goto err_add_bus;
>>> diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c
>>> index ee139a5..8470232 100644
>>> --- a/drivers/i2c/busses/i2c-octeon.c
>>> +++ b/drivers/i2c/busses/i2c-octeon.c
>>> @@ -581,7 +581,7 @@ static int __devinit octeon_i2c_probe(struct platform_device *pdev)
>>>  
>>>  	i2c->adap = octeon_i2c_ops;
>>>  	i2c->adap.dev.parent = &pdev->dev;
>>> -	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
>>> +	i2c->adap.nr = pdev->id;
>>>  	i2c_set_adapdata(&i2c->adap, i2c);
>>>  	platform_set_drvdata(pdev, i2c);
>>>  
>>> diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c
>>> index 2adbf1a..675878f 100644
>>> --- a/drivers/i2c/busses/i2c-pca-platform.c
>>> +++ b/drivers/i2c/busses/i2c-pca-platform.c
>>> @@ -171,7 +171,7 @@ static int __devinit i2c_pca_pf_probe(struct platform_device *pdev)
>>>  	i2c->io_size = resource_size(res);
>>>  	i2c->irq = irq;
>>>  
>>> -	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
>>> +	i2c->adap.nr = pdev->id;
>>>  	i2c->adap.owner = THIS_MODULE;
>>>  	snprintf(i2c->adap.name, sizeof(i2c->adap.name),
>>>  		 "PCA9564/PCA9665 at 0x%08lx",
>>> diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
>>> index 6055601..9458568 100644
>>> --- a/drivers/i2c/busses/i2c-versatile.c
>>> +++ b/drivers/i2c/busses/i2c-versatile.c
>>> @@ -102,13 +102,8 @@ static int i2c_versatile_probe(struct platform_device *dev)
>>>  	i2c->algo = i2c_versatile_algo;
>>>  	i2c->algo.data = i2c;
>>>  
>>> -	if (dev->id >= 0) {
>>> -		/* static bus numbering */
>>> -		i2c->adap.nr = dev->id;
>>> -		ret = i2c_bit_add_numbered_bus(&i2c->adap);
>>> -	} else
>>> -		/* dynamic bus numbering */
>>> -		ret = i2c_bit_add_bus(&i2c->adap);
>>> +	i2c->adap.nr = dev->id;
>>> +	ret = i2c_bit_add_numbered_bus(&i2c->adap);
>>>  	if (ret >= 0) {
>>>  		platform_set_drvdata(dev, i2c);
>>>  		return 0;
>>
>>
>>
>> -- 
>> Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform
> 



-- 
Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform

Re: [PATCH 2/2] i2c: Dynamically assign adapter id if it wasn't explictly specified

[David Daney]

On 03/22/2012 08:28 AM, Karol Lewandowski wrote:
> On 16.03.2012 13:19, Karol Lewandowski wrote:
>
>>> Commit 488bf314b ("i2c: Allow i2c_add_numbered_adapter() to assign a
>>> bus id") reworked i2c_add_numbered_adapter() to call i2c_add_adapter()
>>> if requested bus was -1.
>
>>> This allows to simplify driver's initialization procedure as it allows
>
>>> static and dynamic adapter id registration using one function.
>>>
>>> This patch updates few more drivers (missed out in original patch) to
>>> use this functionality.
>>
>
>> [ I must have lost actual problem description while rewording
>>   message itself... ]
>>
>> Problem arises when multiple drivers (or multiple instances
>> of one driver) try to assume the same fixed bus number (0).
>>
>> This simply causes i2c_add_numbered_bus() to fail.
>> Leaving -1 works perfectly, as registration function switches
>> to dynamic id registration.
>
>>> Signed-off-by: Karol Lewandowski<k.lewandowsk@samsung.com>
>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>> ---
>>>   drivers/i2c/busses/i2c-gpio.c         |    7 +------
>>>   drivers/i2c/busses/i2c-octeon.c       |    2 +-
>>>   drivers/i2c/busses/i2c-pca-platform.c |    2 +-
>>>   drivers/i2c/busses/i2c-versatile.c    |    9 ++-------
>>>   4 files changed, 5 insertions(+), 15 deletions(-)
>
>
> Dear Haavard, Rade, Wolfram and Russel
>
> Could you review following changes for gpio, octeon, pca-platform
> and versatile i2c controller drivers (for which you are, or were,
> maintainers)?
>
> Grant requested explicit Ack to get this merged.
>
> Thanks!
>
>>
>> diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
>> index a651779..50a2a94 100644
>> --- a/drivers/i2c/busses/i2c-gpio.c
>> +++ b/drivers/i2c/busses/i2c-gpio.c
>> @@ -144,12 +144,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
>>   	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
>>   	adap->dev.parent =&pdev->dev;
>>
>> -	/*
>> -	 * If "dev->id" is negative we consider it as zero.
>> -	 * The reason to do so is to avoid sysfs names that only make
>> -	 * sense when there are multiple adapters.
>> -	 */
>> -	adap->nr = (pdev->id != -1) ? pdev->id : 0;
>> +	adap->nr = pdev->id;
>>   	ret = i2c_bit_add_numbered_bus(adap);
>>   	if (ret)
>>   		goto err_add_bus;
>> diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c
>> index ee139a5..8470232 100644
>> --- a/drivers/i2c/busses/i2c-octeon.c
>> +++ b/drivers/i2c/busses/i2c-octeon.c
>> @@ -581,7 +581,7 @@ static int __devinit octeon_i2c_probe(struct platform_device *pdev)
>>
>>   	i2c->adap = octeon_i2c_ops;
>>   	i2c->adap.dev.parent =&pdev->dev;
>> -	i2c->adap.nr = pdev->id>= 0 ? pdev->id : 0;
>> +	i2c->adap.nr = pdev->id;

I guess the OCTEON bit seems sane enough.  I don't fully understand why 
this needs changing, because OCTEON platform code always passes a 
non-negative pdev->id.  But since you asked for it:

Acked-by: David Daney <david.daney@cavium.com>

>>   	i2c_set_adapdata(&i2c->adap, i2c);
>>   	platform_set_drvdata(pdev, i2c);
>>
>> diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c
>> index 2adbf1a..675878f 100644
>> --- a/drivers/i2c/busses/i2c-pca-platform.c
>> +++ b/drivers/i2c/busses/i2c-pca-platform.c
>> @@ -171,7 +171,7 @@ static int __devinit i2c_pca_pf_probe(struct platform_device *pdev)
>>   	i2c->io_size = resource_size(res);
>>   	i2c->irq = irq;
>>
>> -	i2c->adap.nr = pdev->id>= 0 ? pdev->id : 0;
>> +	i2c->adap.nr = pdev->id;
>>   	i2c->adap.owner = THIS_MODULE;
>>   	snprintf(i2c->adap.name, sizeof(i2c->adap.name),
>>   		 "PCA9564/PCA9665 at 0x%08lx",
>> diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
>> index 6055601..9458568 100644
>> --- a/drivers/i2c/busses/i2c-versatile.c
>> +++ b/drivers/i2c/busses/i2c-versatile.c
>> @@ -102,13 +102,8 @@ static int i2c_versatile_probe(struct platform_device *dev)
>>   	i2c->algo = i2c_versatile_algo;
>>   	i2c->algo.data = i2c;
>>
>> -	if (dev->id>= 0) {
>> -		/* static bus numbering */
>> -		i2c->adap.nr = dev->id;
>> -		ret = i2c_bit_add_numbered_bus(&i2c->adap);
>> -	} else
>> -		/* dynamic bus numbering */
>> -		ret = i2c_bit_add_bus(&i2c->adap);
>> +	i2c->adap.nr = dev->id;
>> +	ret = i2c_bit_add_numbered_bus(&i2c->adap);
>>   	if (ret>= 0) {
>>   		platform_set_drvdata(dev, i2c);
>>   		return 0;
>
>
>

Re: [PATCH 2/2] i2c: Dynamically assign adapter id if it wasn't explictly specified

[Karol Lewandowski]

On 22.03.2012 17:58, David Daney wrote:

> On 03/22/2012 08:28 AM, Karol Lewandowski wrote:
>> On 16.03.2012 13:19, Karol Lewandowski wrote:
>>
>>>> Commit 488bf314b ("i2c: Allow i2c_add_numbered_adapter() to assign a
>>>> bus id") reworked i2c_add_numbered_adapter() to call i2c_add_adapter()
>>>> if requested bus was -1.
>>
>>>> This allows to simplify driver's initialization procedure as it allows
>>
>>>> static and dynamic adapter id registration using one function.
>>>>
>>>> This patch updates few more drivers (missed out in original patch) to
>>>> use this functionality.
>>>
>>
>>> [ I must have lost actual problem description while rewording
>>>   message itself... ]
>>>
>>> Problem arises when multiple drivers (or multiple instances
>>> of one driver) try to assume the same fixed bus number (0).
>>>
>>> This simply causes i2c_add_numbered_bus() to fail.
>>> Leaving -1 works perfectly, as registration function switches
>>> to dynamic id registration.
>>
>>>> Signed-off-by: Karol Lewandowski<k.lewandowsk@samsung.com>
>>>> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
>>>> ---
>>>>   drivers/i2c/busses/i2c-gpio.c         |    7 +------
>>>>   drivers/i2c/busses/i2c-octeon.c       |    2 +-
>>>>   drivers/i2c/busses/i2c-pca-platform.c |    2 +-
>>>>   drivers/i2c/busses/i2c-versatile.c    |    9 ++-------
>>>>   4 files changed, 5 insertions(+), 15 deletions(-)
>>
>>
>> Dear Haavard, Rade, Wolfram and Russel
>>
>> Could you review following changes for gpio, octeon, pca-platform
>> and versatile i2c controller drivers (for which you are, or were,
>> maintainers)?
>>
>> Grant requested explicit Ack to get this merged.
>>
>> Thanks!
>>
>>>
>>> diff --git a/drivers/i2c/busses/i2c-gpio.c
>>> b/drivers/i2c/busses/i2c-gpio.c
>>> index a651779..50a2a94 100644
>>> --- a/drivers/i2c/busses/i2c-gpio.c
>>> +++ b/drivers/i2c/busses/i2c-gpio.c
>>> @@ -144,12 +144,7 @@ static int __devinit i2c_gpio_probe(struct
>>> platform_device *pdev)
>>>       adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
>>>       adap->dev.parent =&pdev->dev;
>>>
>>> -    /*
>>> -     * If "dev->id" is negative we consider it as zero.
>>> -     * The reason to do so is to avoid sysfs names that only make
>>> -     * sense when there are multiple adapters.
>>> -     */
>>> -    adap->nr = (pdev->id != -1) ? pdev->id : 0;
>>> +    adap->nr = pdev->id;
>>>       ret = i2c_bit_add_numbered_bus(adap);
>>>       if (ret)
>>>           goto err_add_bus;
>>> diff --git a/drivers/i2c/busses/i2c-octeon.c
>>> b/drivers/i2c/busses/i2c-octeon.c
>>> index ee139a5..8470232 100644
>>> --- a/drivers/i2c/busses/i2c-octeon.c
>>> +++ b/drivers/i2c/busses/i2c-octeon.c
>>> @@ -581,7 +581,7 @@ static int __devinit octeon_i2c_probe(struct
>>> platform_device *pdev)
>>>
>>>       i2c->adap = octeon_i2c_ops;
>>>       i2c->adap.dev.parent =&pdev->dev;
>>> -    i2c->adap.nr = pdev->id>= 0 ? pdev->id : 0;
>>> +    i2c->adap.nr = pdev->id;
> 
> I guess the OCTEON bit seems sane enough.  I don't fully understand why
> this needs changing, because OCTEON platform code always passes a
> non-negative pdev->id.


i2c controllers instantiated from device tree seem to have -1 as id.
Thus, trying to register more than one controller will fail as both
will try to register on bus 0.

However, I've just found that you got rid of this line altogether and
switched to dynamic id allocation (i2c_add_adapter() instead of
_numbered_ variant) in "MIPS: Octeon: Use Device Tree." RFC.
Found here:

  http://thread.gmane.org/gmane.linux.kernel/1104062

In the light of above my (octeon-)fixup becomes redundant.

Shall I repost this patch without octeon changes or is ok anyway?

Thanks!

>  But since you asked for it:
> 
> Acked-by: David Daney <david.daney@cavium.com>
> 
>>>       i2c_set_adapdata(&i2c->adap, i2c);
>>>       platform_set_drvdata(pdev, i2c);
>>>
>>> diff --git a/drivers/i2c/busses/i2c-pca-platform.c
>>> b/drivers/i2c/busses/i2c-pca-platform.c
>>> index 2adbf1a..675878f 100644
>>> --- a/drivers/i2c/busses/i2c-pca-platform.c
>>> +++ b/drivers/i2c/busses/i2c-pca-platform.c
>>> @@ -171,7 +171,7 @@ static int __devinit i2c_pca_pf_probe(struct
>>> platform_device *pdev)
>>>       i2c->io_size = resource_size(res);
>>>       i2c->irq = irq;
>>>
>>> -    i2c->adap.nr = pdev->id>= 0 ? pdev->id : 0;
>>> +    i2c->adap.nr = pdev->id;
>>>       i2c->adap.owner = THIS_MODULE;
>>>       snprintf(i2c->adap.name, sizeof(i2c->adap.name),
>>>            "PCA9564/PCA9665 at 0x%08lx",
>>> diff --git a/drivers/i2c/busses/i2c-versatile.c
>>> b/drivers/i2c/busses/i2c-versatile.c
>>> index 6055601..9458568 100644
>>> --- a/drivers/i2c/busses/i2c-versatile.c
>>> +++ b/drivers/i2c/busses/i2c-versatile.c
>>> @@ -102,13 +102,8 @@ static int i2c_versatile_probe(struct
>>> platform_device *dev)
>>>       i2c->algo = i2c_versatile_algo;
>>>       i2c->algo.data = i2c;
>>>
>>> -    if (dev->id>= 0) {
>>> -        /* static bus numbering */
>>> -        i2c->adap.nr = dev->id;
>>> -        ret = i2c_bit_add_numbered_bus(&i2c->adap);
>>> -    } else
>>> -        /* dynamic bus numbering */
>>> -        ret = i2c_bit_add_bus(&i2c->adap);
>>> +    i2c->adap.nr = dev->id;
>>> +    ret = i2c_bit_add_numbered_bus(&i2c->adap);
>>>       if (ret>= 0) {
>>>           platform_set_drvdata(dev, i2c);
>>>           return 0;
>>
>>
>>
> 
> 



-- 
Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform

Re: [PATCH 2/2] i2c: Dynamically assign adapter id if it wasn't explictly specified

[David Daney]

On 03/22/2012 10:48 AM, Karol Lewandowski wrote:
> On 22.03.2012 17:58, David Daney wrote:
>
[...]

>>>> diff --git a/drivers/i2c/busses/i2c-octeon.c
>>>> b/drivers/i2c/busses/i2c-octeon.c
>>>> index ee139a5..8470232 100644
>>>> --- a/drivers/i2c/busses/i2c-octeon.c
>>>> +++ b/drivers/i2c/busses/i2c-octeon.c
>>>> @@ -581,7 +581,7 @@ static int __devinit octeon_i2c_probe(struct
>>>> platform_device *pdev)
>>>>
>>>>        i2c->adap = octeon_i2c_ops;
>>>>        i2c->adap.dev.parent =&pdev->dev;
>>>> -    i2c->adap.nr = pdev->id>= 0 ? pdev->id : 0;
>>>> +    i2c->adap.nr = pdev->id;
>>
>> I guess the OCTEON bit seems sane enough.  I don't fully understand why
>> this needs changing, because OCTEON platform code always passes a
>> non-negative pdev->id.
>
>
> i2c controllers instantiated from device tree seem to have -1 as id.
> Thus, trying to register more than one controller will fail as both
> will try to register on bus 0.
>
> However, I've just found that you got rid of this line altogether and
> switched to dynamic id allocation (i2c_add_adapter() instead of
> _numbered_ variant) in "MIPS: Octeon: Use Device Tree." RFC.
> Found here:
>
>    http://thread.gmane.org/gmane.linux.kernel/1104062
>
> In the light of above my (octeon-)fixup becomes redundant.
>
> Shall I repost this patch without octeon changes or is ok anyway?

My preference would be to omit the OCTEON portion from your patch.  As 
you noted, I plan to blow all that code away in the very near future, 
and the patch is not needed for correctness as far as I can see.

David Daney


>
> Thanks!
>
>>   But since you asked for it:
>>
>> Acked-by: David Daney<david.daney@cavium.com>
>>

Re: [PATCH 2/2] i2c: Dynamically assign adapter id if it wasn't explictly specified

[Wolfram Sang]

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

> Dear Haavard, Rade, Wolfram and Russel
> 
> Could you review following changes for gpio, octeon, pca-platform
> and versatile i2c controller drivers (for which you are, or were,
> maintainers)?
> 
> Grant requested explicit Ack to get this merged.

? Since those are I2C patches, I was assuming that I pick them. After the merge
window, that is. Did I miss something?

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 2/2] i2c: Dynamically assign adapter id if it wasn't explictly specified

[Karol Lewandowski]

On 22.03.2012 22:47, Wolfram Sang wrote:

>> Dear Haavard, Rade, Wolfram and Russel
>>
>> Could you review following changes for gpio, octeon, pca-platform
>> and versatile i2c controller drivers (for which you are, or were,
>> maintainers)?
>>
>> Grant requested explicit Ack to get this merged.
> 
> ? Since those are I2C patches, I was assuming that I pick them. After the merge
> window, that is. Did I miss something?

That's all fine by me. I just wanted confirmation from respective
maintainers that changes I'm doing are ok for them.

... and it looks changes to i2c-octeon are redundant in light of
the work maintainer has done. I'll drop this part and repost patch
again.

Thanks.
-- 
Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform

Re: [PATCH 0/2] i2c: Don't assume bus nr 0 if none was specified

[Karol Lewandowski]

On 16.03.2012 13:19, Karol Lewandowski wrote:

> Hi,
> 
> i2c controller drivers used to assume bus number 0 when none (-1) was specified.
> 
> This worked on non-device tree systems, where one could explicitly specify
> bus number via platform data.  On DT-enabled systems bus number is always -1.
> 
> Some drivers assume bus number 0 when -1 is specified.  This patchset kills
> this logic and switches to dynamic bus allocation (default when -1 is provided[1]).


[ I must have lost actual problem description while rewording
  message itself... ]

Problem arises when multiple drivers (or multiple instances
of one driver) try to assume the same fixed bus number (0).

This causes simply causes i2c_add_numbered_bus() to fail.
Leaving -1 works perfectly, as registration function switches
to dynamic id registration.

> 
> [ I've found out this when I've tried to instantiate more than one i2c-gpio
>   instance on DT-enabled system. I've, hopefully, fixed few other drivers too. ]
> 
> [1] Introduced by Grant Likely in 488bf314b ("i2c: Allow i2c_add_numbered_adapter()
>     to assign a bus id")
> 
> Karol Lewandowski (2):
>   i2c-pxa: Drop leftover comment
>   i2c: Dynamically assign adapter id if it wasn't explictly specified
> 
>  drivers/i2c/busses/i2c-gpio.c         |    7 +------
>  drivers/i2c/busses/i2c-octeon.c       |    2 +-
>  drivers/i2c/busses/i2c-pca-platform.c |    2 +-
>  drivers/i2c/busses/i2c-pxa.c          |    5 -----
>  drivers/i2c/busses/i2c-versatile.c    |    9 ++-------
>  5 files changed, 5 insertions(+), 20 deletions(-)
> 



-- 
Karol Lewandowski | Samsung Poland R&D Center | Linux/Platform

Re: [PATCH 0/2] i2c: Don't assume bus nr 0 if none was specified

[Grant Likely]

On Fri, 16 Mar 2012 13:27:27 +0100, Karol Lewandowski <k.lewandowsk@samsung.com> wrote:
> On 16.03.2012 13:19, Karol Lewandowski wrote:
> 
> > Hi,
> > 
> > i2c controller drivers used to assume bus number 0 when none (-1) was specified.
> > 
> > This worked on non-device tree systems, where one could explicitly specify
> > bus number via platform data.  On DT-enabled systems bus number is always -1.
> > 
> > Some drivers assume bus number 0 when -1 is specified.  This patchset kills
> > this logic and switches to dynamic bus allocation (default when -1 is provided[1]).
> 
> 
> [ I must have lost actual problem description while rewording
>   message itself... ]
> 
> Problem arises when multiple drivers (or multiple instances
> of one driver) try to assume the same fixed bus number (0).
> 
> This causes simply causes i2c_add_numbered_bus() to fail.
> Leaving -1 works perfectly, as registration function switches
> to dynamic id registration.

Patch series looks good to me.  You'll need acks from the affected users.

g.

[PATCH v2 0/2] i2c: Don't assume bus nr 0 if none was specified

[Karol Lewandowski]

Changes since v1:
 - Dropped reduntant i2c-octeon change

i2c controller drivers used to assume bus number 0 when none (-1) was
specified.

This worked on non-device tree systems, where one could explicitly
specify bus number via platform data.  On DT-enabled systems bus
number is always -1.

This patchset reworks few remaining drivers to use dynamic bus
allocation when no id has been provided.

[ I've found out this when I've tried to instantiate more than one
  i2c-gpio instance on DT-enabled system. I've, hopefully, fixed few
  other drivers too. ]

Karol Lewandowski (2):
  i2c-pxa: Drop leftover comment
  i2c: Dynamically assign adapter id if it wasn't explictly specified

 drivers/i2c/busses/i2c-gpio.c         |    7 +------
 drivers/i2c/busses/i2c-pca-platform.c |    2 +-
 drivers/i2c/busses/i2c-pxa.c          |    5 -----
 drivers/i2c/busses/i2c-versatile.c    |    9 ++-------
 4 files changed, 4 insertions(+), 19 deletions(-)

-- 
1.7.9.1

[PATCH 2/2] i2c: Dynamically assign adapter id if it wasn't explictly specified

[Karol Lewandowski]

Commit 488bf314b ("i2c: Allow i2c_add_numbered_adapter() to assign a
bus id") reworked i2c_add_numbered_adapter() to call i2c_add_adapter()
if requested bus was -1.

This allows to simplify driver's initialization procedure by using
just one function for static and dynamic adapter id registration.

This patch updates few more drivers (missed out in original patch)
to use this functionality.

Signed-off-by: Karol Lewandowski <k.lewandowsk@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/i2c/busses/i2c-gpio.c         |    7 +------
 drivers/i2c/busses/i2c-pca-platform.c |    2 +-
 drivers/i2c/busses/i2c-versatile.c    |    9 ++-------
 3 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index a651779..50a2a94 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -144,12 +144,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 	adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	adap->dev.parent = &pdev->dev;
 
-	/*
-	 * If "dev->id" is negative we consider it as zero.
-	 * The reason to do so is to avoid sysfs names that only make
-	 * sense when there are multiple adapters.
-	 */
-	adap->nr = (pdev->id != -1) ? pdev->id : 0;
+	adap->nr = pdev->id;
 	ret = i2c_bit_add_numbered_bus(adap);
 	if (ret)
 		goto err_add_bus;
diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c
index 2adbf1a..675878f 100644
--- a/drivers/i2c/busses/i2c-pca-platform.c
+++ b/drivers/i2c/busses/i2c-pca-platform.c
@@ -171,7 +171,7 @@ static int __devinit i2c_pca_pf_probe(struct platform_device *pdev)
 	i2c->io_size = resource_size(res);
 	i2c->irq = irq;
 
-	i2c->adap.nr = pdev->id >= 0 ? pdev->id : 0;
+	i2c->adap.nr = pdev->id;
 	i2c->adap.owner = THIS_MODULE;
 	snprintf(i2c->adap.name, sizeof(i2c->adap.name),
 		 "PCA9564/PCA9665 at 0x%08lx",
diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
index 6055601..9458568 100644
--- a/drivers/i2c/busses/i2c-versatile.c
+++ b/drivers/i2c/busses/i2c-versatile.c
@@ -102,13 +102,8 @@ static int i2c_versatile_probe(struct platform_device *dev)
 	i2c->algo = i2c_versatile_algo;
 	i2c->algo.data = i2c;
 
-	if (dev->id >= 0) {
-		/* static bus numbering */
-		i2c->adap.nr = dev->id;
-		ret = i2c_bit_add_numbered_bus(&i2c->adap);
-	} else
-		/* dynamic bus numbering */
-		ret = i2c_bit_add_bus(&i2c->adap);
+	i2c->adap.nr = dev->id;
+	ret = i2c_bit_add_numbered_bus(&i2c->adap);
 	if (ret >= 0) {
 		platform_set_drvdata(dev, i2c);
 		return 0;
-- 
1.7.9.1