Skip to contentSkip to conversation

i2c: i2c-gpio: fix issue when DT node greater than 1

PATCH New Bo Shen 4 participants 10 messages Sep 18, 2012 – Oct 15, 2012

Subsystems (3)
Device tree bindings I2C subsystem host drivers Generic GPIO I2C driver
lore
Share & tools
lore

Bo Shen

author
#0 lore raw

PATCH 1/1 i2c: i2c-gpio: fix issue when DT node greater than 1

When i2c-gpio node number is greater than 1, the second can not sucessfully regiter

Using alias method to get the pdev->id, or else the second device will use the same id which will cause fail to register

Signed-off-by Bo Shen <[email protected]>
---
 Documentation/devicetree/bindings/i2c/gpio-i2c.txt |   46 +++++++++++++++++++-
 drivers/i2c/busses/i2c-gpio.c                      |    1 +
 2 files changed, 46 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/i2c/gpio-i2c.txt +45 -1
index 4f8ec94..e76dd00 100644
@@ -1,4 +1,7 @@
-Device-Tree bindings for i2c gpio driver
+* Device-Tree bindings for i2c gpio driver
+
+Optional alias:
+	- i2c-gpio: used to get id, when more than one i2c-gpio in dts file
 
 Required properties:
 	- compatible = "i2c-gpio";

@@ -30,3 +33,44 @@ i2c at 0 {
 		reg = <0x56>;
 	};
 };
+
+
+Example nodes with alias:
+
+aliases {
+	i2c-gpio0 = &i2c0;
+	i2c-gpio1 = &i2c1;
+};
+
+i2c0: i2c at 0 {
+	compatible = "i2c-gpio";
+	gpios = <&pioA 23 0 /* sda */
+		 &pioA 24 0 /* scl */
+		>;
+	i2c-gpio,sda-open-drain;
+	i2c-gpio,scl-open-drain;
+	i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	rv3029c2 at 56 {
+		compatible = "rv3029c2";
+		reg = <0x56>;
+	};
+};
+
+i2c1: i2c at 0 {
+	compatible = "i2c-gpio";
+	gpios = <&pioA 30 0 /* sda */
+		 &pioA 31 0 /* scl */
+		>;
+	i2c-gpio,sda-open-drain;
+	i2c-gpio,scl-open-drain;
+	i2c-gpio,delay-us = <2>;	/* ~100 kHz */
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	rv3029c2 at 56 {
+		compatible = "rv3029c2";
+		reg = <0x56>;
+	};
drivers/i2c/busses/i2c-gpio.c +1 -0
index e62d2d9..051fbb3 100644
@@ -136,6 +136,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 		ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
 		if (ret)
 			return ret;
+		pdev->id = of_alias_get_id(pdev->dev.of_node, "i2c-gpio");
 	} else {
 		if (!pdev->dev.platform_data)
 			return -ENXIO;
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 i2c: i2c-gpio: fix issue when DT node greater than 1

Quoted 5 lines · Bo Shen

When i2c-gpio node number is greater than 1, the second can not sucessfully regiter

Using alias method to get the pdev->id, or else the second device will use the same id which will cause fail to register

pdev->id shouldn't be used at all with device tree. Why does registration fail without this change?

Quoted 13 lines
drivers/i2c/busses/i2c-gpio.c +1 -0
index e62d2d9..051fbb3 100644
@@ -136,6 +136,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
 		ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
 		if (ret)
 			return ret;
+		pdev->id = of_alias_get_id(pdev->dev.of_node, "i2c-gpio");
 	} else {
 		if (!pdev->dev.platform_data)
 			return -ENXIO;
author
#2 lore raw

PATCH 1/1 i2c: i2c-gpio: fix issue when DT node greater than 1

Hi Stephen Warren,

Quoted 9 lines · Stephen Warren
Quoted 5 lines · Bo Shen

When i2c-gpio node number is greater than 1, the second can not sucessfully regiter

Using alias method to get the pdev->id, or else the second device will use the same id which will cause fail to register

pdev->id shouldn't be used at all with device tree. Why does registration fail without this change?

I add the debug info, and it give the following error without this patch.

--<8----------------------------------
adap->name = i2c-gpio-1
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio-1
i2c-gpio: probe of i2c.3 failed with error -16
-->8----------------------------------

With this patch, it successfully registered.
--<8----------------------------------
adap->name = i2c-gpio0
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio1
i2c-gpio i2c.3: using pins 90 (SDA) and 91 (SCL)
-->8----------------------------------

Quoted 16 lines
Quoted 14 lines
drivers/i2c/busses/i2c-gpio.c +1 -0
index e62d2d9..051fbb3 100644
@@ -136,6 +136,7 @@ static int __devinit i2c_gpio_probe(struct platform_device *pdev)
  		ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
  		if (ret)
  			return ret;
+		pdev->id = of_alias_get_id(pdev->dev.of_node, "i2c-gpio");
  	} else {
  		if (!pdev->dev.platform_data)
  			return -ENXIO;
#3 lore raw

PATCH 1/1 i2c: i2c-gpio: fix issue when DT node greater than 1

Quoted 24 lines · Bo Shen

Hi Stephen Warren,

Quoted 11 lines · Stephen Warren
Quoted 7 lines · Bo Shen

When i2c-gpio node number is greater than 1, the second can not sucessfully
regiter

Using alias method to get the pdev->id, or else the second device will use
the same id which will cause fail to register

pdev->id shouldn't be used at all with device tree. Why does registration fail without this change?

I add the debug info, and it give the following error without this patch.

--<8----------------------------------
adap->name = i2c-gpio-1
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio-1
i2c-gpio: probe of i2c.3 failed with error -16
-->8----------------------------------

Quoted 7 lines

With this patch, it successfully registered.
--<8----------------------------------
adap->name = i2c-gpio0
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio1
i2c-gpio i2c.3: using pins 90 (SDA) and 91 (SCL)
-->8----------------------------------

Yes, that explains why the registration fails, but not why this patch is the correct solution to the problem.

The problematic code appears to be:

snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

Instead, I think that should be something more like:

adap->name = dev_name(&pdev->dev);

or perhaps:

if (pdev->dev.of_node)
/* named will be based on DT node name */
adap->name = dev_name(&pdev->dev);
else
snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

?

#4 lore raw

PATCH 1/1 i2c: i2c-gpio: fix issue when DT node greater than 1

Quoted 21 lines · Stephen Warren
Quoted 9 lines · Bo Shen

I add the debug info, and it give the following error without this patch.

--<8----------------------------------
adap->name = i2c-gpio-1
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio-1
i2c-gpio: probe of i2c.3 failed with error -16
-->8----------------------------------

Quoted 7 lines

With this patch, it successfully registered.
--<8----------------------------------
adap->name = i2c-gpio0
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio1
i2c-gpio i2c.3: using pins 90 (SDA) and 91 (SCL)
-->8----------------------------------

Yes, that explains why the registration fails, but not why this patch is the correct solution to the problem.

Quoted 7 lines

The problematic code appears to be:

snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

Instead, I think that should be something more like:

adap->name = dev_name(&pdev->dev);

strlcpy(adap->name, dev_name(&pdev->dev), sizeof(adap->name))

if anything, as adap->name is a buffer, not a pointer.

Quoted 9 lines

or perhaps:

if (pdev->dev.of_node)
/* named will be based on DT node name */
adap->name = dev_name(&pdev->dev);
else
snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

?

Signature · 1 line
Jean Delvare
author
#5 lore raw

PATCH 1/1 i2c: i2c-gpio: fix issue when DT node greater than 1

Hi Jean Delvare,

Quoted 24 lines · Jean Delvare
Quoted 23 lines · Stephen Warren
Quoted 9 lines · Bo Shen

I add the debug info, and it give the following error without this patch.

--<8----------------------------------
adap->name = i2c-gpio-1
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio-1
i2c-gpio: probe of i2c.3 failed with error -16
-->8----------------------------------

Quoted 7 lines

With this patch, it successfully registered.
--<8----------------------------------
adap->name = i2c-gpio0
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio1
i2c-gpio i2c.3: using pins 90 (SDA) and 91 (SCL)
-->8----------------------------------

Yes, that explains why the registration fails, but not why this patch is the correct solution to the problem.

The problematic code appears to be:

Quoted 7 lines
Quoted 5 lines

snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

Instead, I think that should be something more like:

adap->name = dev_name(&pdev->dev);

strlcpy(adap->name, dev_name(&pdev->dev), sizeof(adap->name))

I am sorry for late. I have tested with DT, it works.

Please send this patch to fix the issue.
Thanks.

Quoted 12 lines

if anything, as adap->name is a buffer, not a pointer.

Quoted 9 lines

or perhaps:

if (pdev->dev.of_node)
/* named will be based on DT node name */
adap->name = dev_name(&pdev->dev);
else
snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

?

author
#6 lore raw

PATCH 1/1 i2c: i2c-gpio: fix issue when DT node greater than 1

Hi Jean Delvare,

Quoted 35 lines · Bo Shen

Hi Jean Delvare,

Quoted 31 lines · Jean Delvare
Quoted 29 lines · Stephen Warren
Quoted 10 lines · Bo Shen

I add the debug info, and it give the following error without this patch.

--<8----------------------------------
adap->name = i2c-gpio-1
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio-1
i2c-gpio: probe of i2c.3 failed with error -16
-->8----------------------------------

Quoted 7 lines

With this patch, it successfully registered.
--<8----------------------------------
adap->name = i2c-gpio0
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio1
i2c-gpio i2c.3: using pins 90 (SDA) and 91 (SCL)
-->8----------------------------------

Yes, that explains why the registration fails, but not why this patch is the correct solution to the problem.

The problematic code appears to be:

snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

Instead, I think that should be something more like:

adap->name = dev_name(&pdev->dev);

Quoted 7 lines
Quoted 2 lines

strlcpy(adap->name, dev_name(&pdev->dev), sizeof(adap->name)) if anything, as adap->name is a buffer, not a pointer.

I am sorry for late. I have tested with DT, it works.

Please send this patch to fix the issue.
Thanks.

Ping?

Will you send this patch? Or, I send this patch and add your S.O.B, which do you prefer?

BRs,
Bo Shen

Quoted 12 lines
Quoted 11 lines
Quoted 10 lines

or perhaps:

if (pdev->dev.of_node)
/* named will be based on DT node name */
adap->name = dev_name(&pdev->dev);
else
snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

?

#7 lore raw

PATCH 1/1 i2c: i2c-gpio: fix issue when DT node greater than 1

Quoted 43 lines · Bo Shen

Hi Jean Delvare,

Quoted 40 lines · Bo Shen

Hi Jean Delvare,

Quoted 25 lines · Jean Delvare
Quoted 24 lines · Stephen Warren
Quoted 10 lines · Bo Shen

I add the debug info, and it give the following error without this patch.

--<8----------------------------------
adap->name = i2c-gpio-1
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio-1
i2c-gpio: probe of i2c.3 failed with error -16
-->8----------------------------------

Quoted 7 lines

With this patch, it successfully registered.
--<8----------------------------------
adap->name = i2c-gpio0
i2c-gpio i2c.2: using pins 30 (SDA) and 31 (SCL)
adap->name = i2c-gpio1
i2c-gpio i2c.3: using pins 90 (SDA) and 91 (SCL)
-->8----------------------------------

Yes, that explains why the registration fails, but not why this patch is the correct solution to the problem.

The problematic code appears to be:

Quoted 8 lines
Quoted 5 lines

snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

Instead, I think that should be something more like:

adap->name = dev_name(&pdev->dev);

strlcpy(adap->name, dev_name(&pdev->dev), sizeof(adap->name)) if anything, as adap->name is a buffer, not a pointer.

I am sorry for late. I have tested with DT, it works.

Quoted 7 lines
Quoted 2 lines

Please send this patch to fix the issue.
Thanks.

Ping?

Will you send this patch? Or, I send this patch and add your S.O.B, which do you prefer?

do it

Best Regards,
J.

#8 lore raw

PATCH 1/1 i2c: i2c-gpio: fix issue when DT node greater than 1

In reply to Bo Shen
Quoted 14 lines · Bo Shen
Quoted 13 lines · Bo Shen
Quoted 11 lines · Jean Delvare
Quoted 7 lines · Stephen Warren

The problematic code appears to be:

snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

Instead, I think that should be something more like:

adap->name = dev_name(&pdev->dev);

strlcpy(adap->name, dev_name(&pdev->dev), sizeof(adap->name)) if anything, as adap->name is a buffer, not a pointer.

Quoted 8 lines
Quoted 4 lines

I am sorry for late. I have tested with DT, it works.

Please send this patch to fix the issue.
Thanks.

Ping?

Will you send this patch?

Which patch, please? On September 18th, you sent a first version. Stephen Warren asked you for a change, but you never sent version two of the patch including the requested change. So don't be surprised if your patch never got applied.

Quoted 1 line

Or, I send this patch and add your S.O.B, which do you prefer?

I prefer that you send patch v2 and let the interested parties review and apply it. Do not add S-o-B for anyone else, ever.

Note that i2c-gpio is normally handled by Wolfram Sang, not me.

Signature · 1 line
Jean Delvare
author
#9 lore raw

PATCH 1/1 i2c: i2c-gpio: fix issue when DT node greater than 1

Hi Jean Delvare,

Quoted 20 lines · Jean Delvare
Quoted 19 lines · Bo Shen
Quoted 17 lines · Bo Shen
Quoted 11 lines · Jean Delvare
Quoted 7 lines · Stephen Warren

The problematic code appears to be:

snprintf(adap->name, sizeof(adap->name), "i2c-gpio%d", pdev->id);

Instead, I think that should be something more like:

adap->name = dev_name(&pdev->dev);

strlcpy(adap->name, dev_name(&pdev->dev), sizeof(adap->name)) if anything, as adap->name is a buffer, not a pointer.

I am sorry for late. I have tested with DT, it works.

Please send this patch to fix the issue.
Thanks.

Quoted 9 lines
Quoted 3 lines

Ping?

Will you send this patch?

Which patch, please? On September 18th, you sent a first version. Stephen Warren asked you for a change, but you never sent version two of the patch including the requested change. So don't be surprised if your patch never got applied.

I am sorry. I take a long vacation, so, I didnot send out version two.

Quoted 4 lines
Quoted 1 line

Or, I send this patch and add your S.O.B, which do you prefer?

I prefer that you send patch v2 and let the interested parties review and apply it. Do not add S-o-B for anyone else, ever.

OK. I will send out v2 as soon as possible.

Quoted 2 lines

Note that i2c-gpio is normally handled by Wolfram Sang, not me.

Thanks for your information.

BRs,
Bo Shen