[PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4

[PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4

[Doug Anderson]

This is similar to a recent commit for exynos5250 titled:
  ARM: EXYNOS: Add aliases for i2c controller

Adding aliases will be useful to prevent warnings in a future
change.  See:
  i2c: s3c2410: Get the i2c bus number from alias id

Signed-off-by: Doug Anderson <dianders@chromium.org>

---
 arch/arm/boot/dts/exynos4.dtsi |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index a26c3dd..824d362 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -28,6 +28,14 @@
 		spi0 = &spi_0;
 		spi1 = &spi_1;
 		spi2 = &spi_2;
+		i2c0 = &i2c_0;
+		i2c1 = &i2c_1;
+		i2c2 = &i2c_2;
+		i2c3 = &i2c_3;
+		i2c4 = &i2c_4;
+		i2c5 = &i2c_5;
+		i2c6 = &i2c_6;
+		i2c7 = &i2c_7;
 	};
 
 	gic:interrupt-controller@10490000 {
@@ -121,7 +129,7 @@
 		status = "disabled";
 	};
 
-	i2c@13860000 {
+	i2c_0: i2c@13860000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -130,7 +138,7 @@
 		status = "disabled";
 	};
 
-	i2c@13870000 {
+	i2c_1: i2c@13870000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -139,7 +147,7 @@
 		status = "disabled";
 	};
 
-	i2c@13880000 {
+	i2c_2: i2c@13880000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -148,7 +156,7 @@
 		status = "disabled";
 	};
 
-	i2c@13890000 {
+	i2c_3: i2c@13890000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -157,7 +165,7 @@
 		status = "disabled";
 	};
 
-	i2c@138A0000 {
+	i2c_4: i2c@138A0000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -166,7 +174,7 @@
 		status = "disabled";
 	};
 
-	i2c@138B0000 {
+	i2c_5: i2c@138B0000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -175,7 +183,7 @@
 		status = "disabled";
 	};
 
-	i2c@138C0000 {
+	i2c_6: i2c@138C0000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -184,7 +192,7 @@
 		status = "disabled";
 	};
 
-	i2c@138D0000 {
+	i2c_7: i2c@138D0000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
-- 
1.7.7.3

[PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4

[Doug Anderson]

This is similar to a recent commit for exynos5250 titled:
  ARM: EXYNOS: Add aliases for i2c controller

Adding aliases will be useful to prevent warnings in a future
change.  See:
  i2c: s3c2410: Get the i2c bus number from alias id

Signed-off-by: Doug Anderson <dianders@chromium.org>

---
 arch/arm/boot/dts/exynos4.dtsi |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index a26c3dd..824d362 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -28,6 +28,14 @@
 		spi0 = &spi_0;
 		spi1 = &spi_1;
 		spi2 = &spi_2;
+		i2c0 = &i2c_0;
+		i2c1 = &i2c_1;
+		i2c2 = &i2c_2;
+		i2c3 = &i2c_3;
+		i2c4 = &i2c_4;
+		i2c5 = &i2c_5;
+		i2c6 = &i2c_6;
+		i2c7 = &i2c_7;
 	};
 
 	gic:interrupt-controller at 10490000 {
@@ -121,7 +129,7 @@
 		status = "disabled";
 	};
 
-	i2c at 13860000 {
+	i2c_0: i2c at 13860000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -130,7 +138,7 @@
 		status = "disabled";
 	};
 
-	i2c at 13870000 {
+	i2c_1: i2c at 13870000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -139,7 +147,7 @@
 		status = "disabled";
 	};
 
-	i2c at 13880000 {
+	i2c_2: i2c at 13880000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -148,7 +156,7 @@
 		status = "disabled";
 	};
 
-	i2c at 13890000 {
+	i2c_3: i2c at 13890000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -157,7 +165,7 @@
 		status = "disabled";
 	};
 
-	i2c at 138A0000 {
+	i2c_4: i2c at 138A0000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -166,7 +174,7 @@
 		status = "disabled";
 	};
 
-	i2c at 138B0000 {
+	i2c_5: i2c at 138B0000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -175,7 +183,7 @@
 		status = "disabled";
 	};
 
-	i2c at 138C0000 {
+	i2c_6: i2c at 138C0000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
@@ -184,7 +192,7 @@
 		status = "disabled";
 	};
 
-	i2c at 138D0000 {
+	i2c_7: i2c at 138D0000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		compatible = "samsung,s3c2440-i2c";
-- 
1.7.7.3

[PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Doug Anderson]

From: Padmavathi Venna <padma.v@samsung.com>

Get the i2c bus number that the device is connected to using the alias
id.  This makes debugging / grokking of kernel messages much easier.

[dianders: slight patch cleanup from Padmavathi's original.]

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/i2c/busses/i2c-s3c2410.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 3e0335f..ca43590 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -899,11 +899,19 @@ static void
 s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
 {
 	struct s3c2410_platform_i2c *pdata = i2c->pdata;
+	int id;
 
 	if (!np)
 		return;
 
-	pdata->bus_num = -1; /* i2c bus number is dynamically assigned */
+	id = of_alias_get_id(np, "i2c");
+	if (id < 0) {
+		dev_warn(i2c->dev, "failed to get alias id:%d\n", id);
+		pdata->bus_num = -1;
+	} else {
+		/* i2c bus number is statically assigned from alias */
+		pdata->bus_num = id;
+	}
 	of_property_read_u32(np, "samsung,i2c-sda-delay", &pdata->sda_delay);
 	of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr);
 	of_property_read_u32(np, "samsung,i2c-max-bus-freq",
-- 
1.7.7.3

[PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Doug Anderson]

From: Padmavathi Venna <padma.v@samsung.com>

Get the i2c bus number that the device is connected to using the alias
id.  This makes debugging / grokking of kernel messages much easier.

[dianders: slight patch cleanup from Padmavathi's original.]

Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/i2c/busses/i2c-s3c2410.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 3e0335f..ca43590 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -899,11 +899,19 @@ static void
 s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
 {
 	struct s3c2410_platform_i2c *pdata = i2c->pdata;
+	int id;
 
 	if (!np)
 		return;
 
-	pdata->bus_num = -1; /* i2c bus number is dynamically assigned */
+	id = of_alias_get_id(np, "i2c");
+	if (id < 0) {
+		dev_warn(i2c->dev, "failed to get alias id:%d\n", id);
+		pdata->bus_num = -1;
+	} else {
+		/* i2c bus number is statically assigned from alias */
+		pdata->bus_num = id;
+	}
 	of_property_read_u32(np, "samsung,i2c-sda-delay", &pdata->sda_delay);
 	of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr);
 	of_property_read_u32(np, "samsung,i2c-max-bus-freq",
-- 
1.7.7.3

Re: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Mark Brown]

On Tue, Nov 20, 2012 at 02:27:04PM -0800, Doug Anderson wrote:
> From: Padmavathi Venna <padma.v@samsung.com>
> 
> Get the i2c bus number that the device is connected to using the alias
> id.  This makes debugging / grokking of kernel messages much easier.

This doesn't look like a s3c2410 specific change - it's a generic device
tree issue.  This suggests that it sohuld be implemented in the
framework so that all I2C controllers with DT can use it.

Re: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Mark Brown]

On Tue, Nov 20, 2012 at 02:27:04PM -0800, Doug Anderson wrote:
> From: Padmavathi Venna <padma.v-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> Get the i2c bus number that the device is connected to using the alias
> id.  This makes debugging / grokking of kernel messages much easier.

This doesn't look like a s3c2410 specific change - it's a generic device
tree issue.  This suggests that it sohuld be implemented in the
framework so that all I2C controllers with DT can use it.

[PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Mark Brown]

On Tue, Nov 20, 2012 at 02:27:04PM -0800, Doug Anderson wrote:
> From: Padmavathi Venna <padma.v@samsung.com>
> 
> Get the i2c bus number that the device is connected to using the alias
> id.  This makes debugging / grokking of kernel messages much easier.

This doesn't look like a s3c2410 specific change - it's a generic device
tree issue.  This suggests that it sohuld be implemented in the
framework so that all I2C controllers with DT can use it.

Re: [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4

[Olof Johansson]

On Tue, Nov 20, 2012 at 02:27:03PM -0800, Doug Anderson wrote:
> This is similar to a recent commit for exynos5250 titled:
>   ARM: EXYNOS: Add aliases for i2c controller
> 
> Adding aliases will be useful to prevent warnings in a future
> change.  See:
>   i2c: s3c2410: Get the i2c bus number from alias id
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>

Acked-by: Olof Johansson <olof@lixom.net>

This can go in independently of the pending comment on the i2c driver change
(that it should be done in the core, which makes sense).


-Olof

Re: [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4

[Olof Johansson]

On Tue, Nov 20, 2012 at 02:27:03PM -0800, Doug Anderson wrote:
> This is similar to a recent commit for exynos5250 titled:
>   ARM: EXYNOS: Add aliases for i2c controller
> 
> Adding aliases will be useful to prevent warnings in a future
> change.  See:
>   i2c: s3c2410: Get the i2c bus number from alias id
> 
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Acked-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>

This can go in independently of the pending comment on the i2c driver change
(that it should be done in the core, which makes sense).


-Olof

[PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4

[Olof Johansson]

On Tue, Nov 20, 2012 at 02:27:03PM -0800, Doug Anderson wrote:
> This is similar to a recent commit for exynos5250 titled:
>   ARM: EXYNOS: Add aliases for i2c controller
> 
> Adding aliases will be useful to prevent warnings in a future
> change.  See:
>   i2c: s3c2410: Get the i2c bus number from alias id
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>

Acked-by: Olof Johansson <olof@lixom.net>

This can go in independently of the pending comment on the i2c driver change
(that it should be done in the core, which makes sense).


-Olof

RE: [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4

[Kukjin Kim]

Doug Anderson wrote:
> 
> This is similar to a recent commit for exynos5250 titled:
>   ARM: EXYNOS: Add aliases for i2c controller
> 
> Adding aliases will be useful to prevent warnings in a future
> change.  See:
>   i2c: s3c2410: Get the i2c bus number from alias id
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> 
> ---
>  arch/arm/boot/dts/exynos4.dtsi |   24 ++++++++++++++++--------
>  1 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos4.dtsi
> b/arch/arm/boot/dts/exynos4.dtsi
> index a26c3dd..824d362 100644
> --- a/arch/arm/boot/dts/exynos4.dtsi
> +++ b/arch/arm/boot/dts/exynos4.dtsi
> @@ -28,6 +28,14 @@
>  		spi0 = &spi_0;
>  		spi1 = &spi_1;
>  		spi2 = &spi_2;
> +		i2c0 = &i2c_0;
> +		i2c1 = &i2c_1;
> +		i2c2 = &i2c_2;
> +		i2c3 = &i2c_3;
> +		i2c4 = &i2c_4;
> +		i2c5 = &i2c_5;
> +		i2c6 = &i2c_6;
> +		i2c7 = &i2c_7;
>  	};
> 
>  	gic:interrupt-controller@10490000 {
> @@ -121,7 +129,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c@13860000 {
> +	i2c_0: i2c@13860000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -130,7 +138,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c@13870000 {
> +	i2c_1: i2c@13870000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -139,7 +147,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c@13880000 {
> +	i2c_2: i2c@13880000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -148,7 +156,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c@13890000 {
> +	i2c_3: i2c@13890000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -157,7 +165,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c@138A0000 {
> +	i2c_4: i2c@138A0000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -166,7 +174,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c@138B0000 {
> +	i2c_5: i2c@138B0000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -175,7 +183,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c@138C0000 {
> +	i2c_6: i2c@138C0000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -184,7 +192,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c@138D0000 {
> +	i2c_7: i2c@138D0000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> --
> 1.7.7.3

Applied, thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4

[Kukjin Kim]

Doug Anderson wrote:
> 
> This is similar to a recent commit for exynos5250 titled:
>   ARM: EXYNOS: Add aliases for i2c controller
> 
> Adding aliases will be useful to prevent warnings in a future
> change.  See:
>   i2c: s3c2410: Get the i2c bus number from alias id
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> 
> ---
>  arch/arm/boot/dts/exynos4.dtsi |   24 ++++++++++++++++--------
>  1 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos4.dtsi
> b/arch/arm/boot/dts/exynos4.dtsi
> index a26c3dd..824d362 100644
> --- a/arch/arm/boot/dts/exynos4.dtsi
> +++ b/arch/arm/boot/dts/exynos4.dtsi
> @@ -28,6 +28,14 @@
>  		spi0 = &spi_0;
>  		spi1 = &spi_1;
>  		spi2 = &spi_2;
> +		i2c0 = &i2c_0;
> +		i2c1 = &i2c_1;
> +		i2c2 = &i2c_2;
> +		i2c3 = &i2c_3;
> +		i2c4 = &i2c_4;
> +		i2c5 = &i2c_5;
> +		i2c6 = &i2c_6;
> +		i2c7 = &i2c_7;
>  	};
> 
>  	gic:interrupt-controller at 10490000 {
> @@ -121,7 +129,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c at 13860000 {
> +	i2c_0: i2c at 13860000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -130,7 +138,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c at 13870000 {
> +	i2c_1: i2c at 13870000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -139,7 +147,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c at 13880000 {
> +	i2c_2: i2c at 13880000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -148,7 +156,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c at 13890000 {
> +	i2c_3: i2c at 13890000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -157,7 +165,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c at 138A0000 {
> +	i2c_4: i2c at 138A0000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -166,7 +174,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c at 138B0000 {
> +	i2c_5: i2c at 138B0000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -175,7 +183,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c at 138C0000 {
> +	i2c_6: i2c at 138C0000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> @@ -184,7 +192,7 @@
>  		status = "disabled";
>  	};
> 
> -	i2c at 138D0000 {
> +	i2c_7: i2c at 138D0000 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  		compatible = "samsung,s3c2440-i2c";
> --
> 1.7.7.3

Applied, thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

RE: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Kukjin Kim]

Doug Anderson wrote:
> 
> From: Padmavathi Venna <padma.v@samsung.com>
> 
> Get the i2c bus number that the device is connected to using the alias
> id.  This makes debugging / grokking of kernel messages much easier.
> 
> [dianders: slight patch cleanup from Padmavathi's original.]
> 
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> Signed-off-by: Doug Anderson <dianders@chromium.org>

Acked-by: Kukjin Kim <kgene.kim@samsung.com>

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

> ---
>  drivers/i2c/busses/i2c-s3c2410.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-
> s3c2410.c
> index 3e0335f..ca43590 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -899,11 +899,19 @@ static void
>  s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
>  {
>  	struct s3c2410_platform_i2c *pdata = i2c->pdata;
> +	int id;
> 
>  	if (!np)
>  		return;
> 
> -	pdata->bus_num = -1; /* i2c bus number is dynamically assigned */
> +	id = of_alias_get_id(np, "i2c");
> +	if (id < 0) {
> +		dev_warn(i2c->dev, "failed to get alias id:%d\n", id);
> +		pdata->bus_num = -1;
> +	} else {
> +		/* i2c bus number is statically assigned from alias */
> +		pdata->bus_num = id;
> +	}
>  	of_property_read_u32(np, "samsung,i2c-sda-delay", &pdata-
> >sda_delay);
>  	of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata-
> >slave_addr);
>  	of_property_read_u32(np, "samsung,i2c-max-bus-freq",
> --
> 1.7.7.3

RE: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Kukjin Kim]

Doug Anderson wrote:
> 
> From: Padmavathi Venna <padma.v-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> Get the i2c bus number that the device is connected to using the alias
> id.  This makes debugging / grokking of kernel messages much easier.
> 
> [dianders: slight patch cleanup from Padmavathi's original.]
> 
> Signed-off-by: Padmavathi Venna <padma.v-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Acked-by: Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

> ---
>  drivers/i2c/busses/i2c-s3c2410.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-
> s3c2410.c
> index 3e0335f..ca43590 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -899,11 +899,19 @@ static void
>  s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
>  {
>  	struct s3c2410_platform_i2c *pdata = i2c->pdata;
> +	int id;
> 
>  	if (!np)
>  		return;
> 
> -	pdata->bus_num = -1; /* i2c bus number is dynamically assigned */
> +	id = of_alias_get_id(np, "i2c");
> +	if (id < 0) {
> +		dev_warn(i2c->dev, "failed to get alias id:%d\n", id);
> +		pdata->bus_num = -1;
> +	} else {
> +		/* i2c bus number is statically assigned from alias */
> +		pdata->bus_num = id;
> +	}
>  	of_property_read_u32(np, "samsung,i2c-sda-delay", &pdata-
> >sda_delay);
>  	of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata-
> >slave_addr);
>  	of_property_read_u32(np, "samsung,i2c-max-bus-freq",
> --
> 1.7.7.3

[PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Kukjin Kim]

Doug Anderson wrote:
> 
> From: Padmavathi Venna <padma.v@samsung.com>
> 
> Get the i2c bus number that the device is connected to using the alias
> id.  This makes debugging / grokking of kernel messages much easier.
> 
> [dianders: slight patch cleanup from Padmavathi's original.]
> 
> Signed-off-by: Padmavathi Venna <padma.v@samsung.com>
> Signed-off-by: Doug Anderson <dianders@chromium.org>

Acked-by: Kukjin Kim <kgene.kim@samsung.com>

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

> ---
>  drivers/i2c/busses/i2c-s3c2410.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-
> s3c2410.c
> index 3e0335f..ca43590 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -899,11 +899,19 @@ static void
>  s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
>  {
>  	struct s3c2410_platform_i2c *pdata = i2c->pdata;
> +	int id;
> 
>  	if (!np)
>  		return;
> 
> -	pdata->bus_num = -1; /* i2c bus number is dynamically assigned */
> +	id = of_alias_get_id(np, "i2c");
> +	if (id < 0) {
> +		dev_warn(i2c->dev, "failed to get alias id:%d\n", id);
> +		pdata->bus_num = -1;
> +	} else {
> +		/* i2c bus number is statically assigned from alias */
> +		pdata->bus_num = id;
> +	}
>  	of_property_read_u32(np, "samsung,i2c-sda-delay", &pdata-
> >sda_delay);
>  	of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata-
> >slave_addr);
>  	of_property_read_u32(np, "samsung,i2c-max-bus-freq",
> --
> 1.7.7.3

[PATCH 0/2] Add automatic bus number support for i2c busses with device tree

[Doug Anderson]

This was suggested by Mark Brown in response to a patch for adding
this functionality only for the s3c2410 bus:
  https://lkml.org/lkml/2012/11/20/681

I have also modified the i2c-pxa driver to use this new functionality
but have no good way to test that patch.  Hopefully someone else can
test and ack.  The patch adding the generic functionality could go in
even if the i2c-pxa patch needs changes.


Doug Anderson (2):
  i2c-core: dt: Pick i2c bus number from i2c alias if present
  i2c: pxa: Use i2c-core to get bus number now

 drivers/i2c/busses/i2c-pxa.c |    8 +---
 drivers/i2c/i2c-core.c       |  105 ++++++++++++++++++++++++++++++-----------
 2 files changed, 78 insertions(+), 35 deletions(-)

-- 
1.7.7.3

[PATCH 0/2] Add automatic bus number support for i2c busses with device tree

[Doug Anderson]

This was suggested by Mark Brown in response to a patch for adding
this functionality only for the s3c2410 bus:
  https://lkml.org/lkml/2012/11/20/681

I have also modified the i2c-pxa driver to use this new functionality
but have no good way to test that patch.  Hopefully someone else can
test and ack.  The patch adding the generic functionality could go in
even if the i2c-pxa patch needs changes.


Doug Anderson (2):
  i2c-core: dt: Pick i2c bus number from i2c alias if present
  i2c: pxa: Use i2c-core to get bus number now

 drivers/i2c/busses/i2c-pxa.c |    8 +---
 drivers/i2c/i2c-core.c       |  105 ++++++++++++++++++++++++++++++-----------
 2 files changed, 78 insertions(+), 35 deletions(-)

-- 
1.7.7.3

[PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

This allows you to get the equivalent functionality of
i2c_add_numbered_adapter() with all data in the device tree and no
special case code in your driver.  This is a common device tree
technique.

For quick reference, the FDT syntax for using an alias to provide an
ID looks like:
  aliases {
    i2c0 = &i2c_0;
    i2c1 = &i2c_1;
  };

Signed-off-by: Doug Anderson <dianders@chromium.org>
CC: Mark Brown <broonie@opensource.wolfsonmicro.com>

---
 drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 77 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index a7edf98..71deb2a2 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -915,13 +915,81 @@ out_list:
 }
 
 /**
+ * i2c_get_number_from_dt - get the adapter number based on dt alias
+ * @adap: the adapter to look at
+ *
+ * Check whether there's an alias in the FDT that gives an ID for this i2c
+ * device.  Use an alias like "i2c<nr>", like:
+ *   aliases {
+ *     i2c0 = &i2c_0;
+ *     i2c1 = &i2c_1;
+ *   };
+ *
+ * Returns the ID if found.  If no alias is found returns -1.
+ */
+static int i2c_get_number_from_dt(struct i2c_adapter *adap)
+{
+	struct device *dev = &adap->dev;
+	int id;
+
+	if (!dev->of_node)
+		return -1;
+
+	id = of_alias_get_id(dev->of_node, "i2c");
+	if (id < 0)
+		return -1;
+	return id;
+}
+
+/**
+ * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
+ * @adap: the adapter to register (with adap->nr initialized)
+ * Context: can sleep
+ *
+ * See i2c_add_numbered_adapter() for details.
+ */
+static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
+{
+	int	id;
+	int	status;
+
+	/* Handled by wrappers */
+	BUG_ON(adap->nr == -1);
+
+	if (adap->nr & ~MAX_IDR_MASK)
+		return -EINVAL;
+
+retry:
+	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
+		return -ENOMEM;
+
+	mutex_lock(&core_lock);
+	/* "above" here means "above or equal to", sigh;
+	 * we need the "equal to" result to force the result
+	 */
+	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
+	if (status == 0 && id != adap->nr) {
+		status = -EBUSY;
+		idr_remove(&i2c_adapter_idr, id);
+	}
+	mutex_unlock(&core_lock);
+	if (status == -EAGAIN)
+		goto retry;
+
+	if (status == 0)
+		status = i2c_register_adapter(adap);
+	return status;
+}
+
+/**
  * i2c_add_adapter - declare i2c adapter, use dynamic bus number
  * @adapter: the adapter to add
  * Context: can sleep
  *
  * This routine is used to declare an I2C adapter when its bus number
- * doesn't matter.  Examples: for I2C adapters dynamically added by
- * USB links or PCI plugin cards.
+ * doesn't matter or when its bus number is specified by an dt alias.
+ * Examples of bases when the bus number doesn't matter: I2C adapters
+ * dynamically added by USB links or PCI plugin cards.
  *
  * When this returns zero, a new bus number was allocated and stored
  * in adap->nr, and the specified adapter became available for clients.
@@ -931,6 +999,12 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
 {
 	int	id, res = 0;
 
+	id = i2c_get_number_from_dt(adapter);
+	if (id >= 0) {
+		adapter->nr = id;
+		return _i2c_add_numbered_adapter(adapter);
+	}
+
 retry:
 	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
 		return -ENOMEM;
@@ -977,34 +1051,9 @@ EXPORT_SYMBOL(i2c_add_adapter);
  */
 int i2c_add_numbered_adapter(struct i2c_adapter *adap)
 {
-	int	id;
-	int	status;
-
 	if (adap->nr == -1) /* -1 means dynamically assign bus id */
 		return i2c_add_adapter(adap);
-	if (adap->nr & ~MAX_IDR_MASK)
-		return -EINVAL;
-
-retry:
-	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
-		return -ENOMEM;
-
-	mutex_lock(&core_lock);
-	/* "above" here means "above or equal to", sigh;
-	 * we need the "equal to" result to force the result
-	 */
-	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
-	if (status == 0 && id != adap->nr) {
-		status = -EBUSY;
-		idr_remove(&i2c_adapter_idr, id);
-	}
-	mutex_unlock(&core_lock);
-	if (status == -EAGAIN)
-		goto retry;
-
-	if (status == 0)
-		status = i2c_register_adapter(adap);
-	return status;
+	return _i2c_add_numbered_adapter(adap);
 }
 EXPORT_SYMBOL_GPL(i2c_add_numbered_adapter);
 
-- 
1.7.7.3

[PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

This allows you to get the equivalent functionality of
i2c_add_numbered_adapter() with all data in the device tree and no
special case code in your driver.  This is a common device tree
technique.

For quick reference, the FDT syntax for using an alias to provide an
ID looks like:
  aliases {
    i2c0 = &i2c_0;
    i2c1 = &i2c_1;
  };

Signed-off-by: Doug Anderson <dianders@chromium.org>
CC: Mark Brown <broonie@opensource.wolfsonmicro.com>

---
 drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 77 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index a7edf98..71deb2a2 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -915,13 +915,81 @@ out_list:
 }
 
 /**
+ * i2c_get_number_from_dt - get the adapter number based on dt alias
+ * @adap: the adapter to look at
+ *
+ * Check whether there's an alias in the FDT that gives an ID for this i2c
+ * device.  Use an alias like "i2c<nr>", like:
+ *   aliases {
+ *     i2c0 = &i2c_0;
+ *     i2c1 = &i2c_1;
+ *   };
+ *
+ * Returns the ID if found.  If no alias is found returns -1.
+ */
+static int i2c_get_number_from_dt(struct i2c_adapter *adap)
+{
+	struct device *dev = &adap->dev;
+	int id;
+
+	if (!dev->of_node)
+		return -1;
+
+	id = of_alias_get_id(dev->of_node, "i2c");
+	if (id < 0)
+		return -1;
+	return id;
+}
+
+/**
+ * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
+ * @adap: the adapter to register (with adap->nr initialized)
+ * Context: can sleep
+ *
+ * See i2c_add_numbered_adapter() for details.
+ */
+static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
+{
+	int	id;
+	int	status;
+
+	/* Handled by wrappers */
+	BUG_ON(adap->nr == -1);
+
+	if (adap->nr & ~MAX_IDR_MASK)
+		return -EINVAL;
+
+retry:
+	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
+		return -ENOMEM;
+
+	mutex_lock(&core_lock);
+	/* "above" here means "above or equal to", sigh;
+	 * we need the "equal to" result to force the result
+	 */
+	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
+	if (status == 0 && id != adap->nr) {
+		status = -EBUSY;
+		idr_remove(&i2c_adapter_idr, id);
+	}
+	mutex_unlock(&core_lock);
+	if (status == -EAGAIN)
+		goto retry;
+
+	if (status == 0)
+		status = i2c_register_adapter(adap);
+	return status;
+}
+
+/**
  * i2c_add_adapter - declare i2c adapter, use dynamic bus number
  * @adapter: the adapter to add
  * Context: can sleep
  *
  * This routine is used to declare an I2C adapter when its bus number
- * doesn't matter.  Examples: for I2C adapters dynamically added by
- * USB links or PCI plugin cards.
+ * doesn't matter or when its bus number is specified by an dt alias.
+ * Examples of bases when the bus number doesn't matter: I2C adapters
+ * dynamically added by USB links or PCI plugin cards.
  *
  * When this returns zero, a new bus number was allocated and stored
  * in adap->nr, and the specified adapter became available for clients.
@@ -931,6 +999,12 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
 {
 	int	id, res = 0;
 
+	id = i2c_get_number_from_dt(adapter);
+	if (id >= 0) {
+		adapter->nr = id;
+		return _i2c_add_numbered_adapter(adapter);
+	}
+
 retry:
 	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
 		return -ENOMEM;
@@ -977,34 +1051,9 @@ EXPORT_SYMBOL(i2c_add_adapter);
  */
 int i2c_add_numbered_adapter(struct i2c_adapter *adap)
 {
-	int	id;
-	int	status;
-
 	if (adap->nr == -1) /* -1 means dynamically assign bus id */
 		return i2c_add_adapter(adap);
-	if (adap->nr & ~MAX_IDR_MASK)
-		return -EINVAL;
-
-retry:
-	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
-		return -ENOMEM;
-
-	mutex_lock(&core_lock);
-	/* "above" here means "above or equal to", sigh;
-	 * we need the "equal to" result to force the result
-	 */
-	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
-	if (status == 0 && id != adap->nr) {
-		status = -EBUSY;
-		idr_remove(&i2c_adapter_idr, id);
-	}
-	mutex_unlock(&core_lock);
-	if (status == -EAGAIN)
-		goto retry;
-
-	if (status == 0)
-		status = i2c_register_adapter(adap);
-	return status;
+	return _i2c_add_numbered_adapter(adap);
 }
 EXPORT_SYMBOL_GPL(i2c_add_numbered_adapter);
 
-- 
1.7.7.3

[PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
present" adds support for automatically picking the bus number based
on the alias ID.  Remove the now unnecessary code from i2c-pxa that
did the same thing.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/i2c/busses/i2c-pxa.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..8ee9fa0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 			of_match_device(i2c_pxa_dt_ids, &pdev->dev);
-	int ret;
 
 	if (!of_id)
 		return 1;
-	ret = of_alias_get_id(np, "i2c");
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
-		return ret;
-	}
-	pdev->id = ret;
+	pdev->id = -1;
 	if (of_get_property(np, "mrvl,i2c-polling", NULL))
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
-- 
1.7.7.3

[PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
present" adds support for automatically picking the bus number based
on the alias ID.  Remove the now unnecessary code from i2c-pxa that
did the same thing.

Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/i2c/busses/i2c-pxa.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..8ee9fa0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 			of_match_device(i2c_pxa_dt_ids, &pdev->dev);
-	int ret;
 
 	if (!of_id)
 		return 1;
-	ret = of_alias_get_id(np, "i2c");
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
-		return ret;
-	}
-	pdev->id = ret;
+	pdev->id = -1;
 	if (of_get_property(np, "mrvl,i2c-polling", NULL))
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
-- 
1.7.7.3

[PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
present" adds support for automatically picking the bus number based
on the alias ID.  Remove the now unnecessary code from i2c-pxa that
did the same thing.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
 drivers/i2c/busses/i2c-pxa.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..8ee9fa0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 			of_match_device(i2c_pxa_dt_ids, &pdev->dev);
-	int ret;
 
 	if (!of_id)
 		return 1;
-	ret = of_alias_get_id(np, "i2c");
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
-		return ret;
-	}
-	pdev->id = ret;
+	pdev->id = -1;
 	if (of_get_property(np, "mrvl,i2c-polling", NULL))
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
-- 
1.7.7.3

Re: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Doug Anderson]

On Tue, Nov 20, 2012 at 8:09 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Nov 20, 2012 at 02:27:04PM -0800, Doug Anderson wrote:
>> From: Padmavathi Venna <padma.v@samsung.com>
>>
>> Get the i2c bus number that the device is connected to using the alias
>> id.  This makes debugging / grokking of kernel messages much easier.
>
> This doesn't look like a s3c2410 specific change - it's a generic device
> tree issue.  This suggests that it sohuld be implemented in the
> framework so that all I2C controllers with DT can use it.

Good suggestion.  I have posted a series with the title "Add automatic
bus number support for i2c busses with device tree".  It contains the
i2c-core patch as well as a patch removing similar code from the pxa
i2c driver.

Kukjin: please consider this patch abandoned and superseded by the new
i2c-core patch.  As Olof said, the patch for adding aliases for
exynos4 should still be fine to apply.


Thanks!

-Doug

[PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Doug Anderson]

On Tue, Nov 20, 2012 at 8:09 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Nov 20, 2012 at 02:27:04PM -0800, Doug Anderson wrote:
>> From: Padmavathi Venna <padma.v@samsung.com>
>>
>> Get the i2c bus number that the device is connected to using the alias
>> id.  This makes debugging / grokking of kernel messages much easier.
>
> This doesn't look like a s3c2410 specific change - it's a generic device
> tree issue.  This suggests that it sohuld be implemented in the
> framework so that all I2C controllers with DT can use it.

Good suggestion.  I have posted a series with the title "Add automatic
bus number support for i2c busses with device tree".  It contains the
i2c-core patch as well as a patch removing similar code from the pxa
i2c driver.

Kukjin: please consider this patch abandoned and superseded by the new
i2c-core patch.  As Olof said, the patch for adding aliases for
exynos4 should still be fine to apply.


Thanks!

-Doug

RE: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Kukjin Kim]

Doug Anderson wrote:
> 
> On Tue, Nov 20, 2012 at 8:09 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > On Tue, Nov 20, 2012 at 02:27:04PM -0800, Doug Anderson wrote:
> >> From: Padmavathi Venna <padma.v@samsung.com>
> >>
> >> Get the i2c bus number that the device is connected to using the alias
> >> id.  This makes debugging / grokking of kernel messages much easier.
> >
> > This doesn't look like a s3c2410 specific change - it's a generic device
> > tree issue.  This suggests that it sohuld be implemented in the
> > framework so that all I2C controllers with DT can use it.
> 
> Good suggestion.  I have posted a series with the title "Add automatic
> bus number support for i2c busses with device tree".  It contains the
> i2c-core patch as well as a patch removing similar code from the pxa
> i2c driver.
> 
> Kukjin: please consider this patch abandoned and superseded by the new
> i2c-core patch.  As Olof said, the patch for adding aliases for
> exynos4 should still be fine to apply.
> 
OK, I see.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

RE: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Kukjin Kim]

Doug Anderson wrote:
> 
> On Tue, Nov 20, 2012 at 8:09 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > On Tue, Nov 20, 2012 at 02:27:04PM -0800, Doug Anderson wrote:
> >> From: Padmavathi Venna <padma.v@samsung.com>
> >>
> >> Get the i2c bus number that the device is connected to using the alias
> >> id.  This makes debugging / grokking of kernel messages much easier.
> >
> > This doesn't look like a s3c2410 specific change - it's a generic device
> > tree issue.  This suggests that it sohuld be implemented in the
> > framework so that all I2C controllers with DT can use it.
> 
> Good suggestion.  I have posted a series with the title "Add automatic
> bus number support for i2c busses with device tree".  It contains the
> i2c-core patch as well as a patch removing similar code from the pxa
> i2c driver.
> 
> Kukjin: please consider this patch abandoned and superseded by the new
> i2c-core patch.  As Olof said, the patch for adding aliases for
> exynos4 should still be fine to apply.
> 
OK, I see.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id

[Kukjin Kim]

Doug Anderson wrote:
> 
> On Tue, Nov 20, 2012 at 8:09 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> > On Tue, Nov 20, 2012 at 02:27:04PM -0800, Doug Anderson wrote:
> >> From: Padmavathi Venna <padma.v@samsung.com>
> >>
> >> Get the i2c bus number that the device is connected to using the alias
> >> id.  This makes debugging / grokking of kernel messages much easier.
> >
> > This doesn't look like a s3c2410 specific change - it's a generic device
> > tree issue.  This suggests that it sohuld be implemented in the
> > framework so that all I2C controllers with DT can use it.
> 
> Good suggestion.  I have posted a series with the title "Add automatic
> bus number support for i2c busses with device tree".  It contains the
> i2c-core patch as well as a patch removing similar code from the pxa
> i2c driver.
> 
> Kukjin: please consider this patch abandoned and superseded by the new
> i2c-core patch.  As Olof said, the patch for adding aliases for
> exynos4 should still be fine to apply.
> 
OK, I see.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Haojian Zhuang]

On Thu, Nov 22, 2012 at 2:26 AM, Doug Anderson <dianders@chromium.org> wrote:
> This allows you to get the equivalent functionality of
> i2c_add_numbered_adapter() with all data in the device tree and no
> special case code in your driver.  This is a common device tree
> technique.
>
> For quick reference, the FDT syntax for using an alias to provide an
> ID looks like:
>   aliases {
>     i2c0 = &i2c_0;
>     i2c1 = &i2c_1;
>   };
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> CC: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> ---
>  drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 77 insertions(+), 28 deletions(-)
>

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

Re: [PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Haojian Zhuang]

On Thu, Nov 22, 2012 at 2:26 AM, Doug Anderson <dianders@chromium.org> wrote:
> This allows you to get the equivalent functionality of
> i2c_add_numbered_adapter() with all data in the device tree and no
> special case code in your driver.  This is a common device tree
> technique.
>
> For quick reference, the FDT syntax for using an alias to provide an
> ID looks like:
>   aliases {
>     i2c0 = &i2c_0;
>     i2c1 = &i2c_1;
>   };
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> CC: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> ---
>  drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 77 insertions(+), 28 deletions(-)
>

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

[PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Haojian Zhuang]

On Thu, Nov 22, 2012 at 2:26 AM, Doug Anderson <dianders@chromium.org> wrote:
> This allows you to get the equivalent functionality of
> i2c_add_numbered_adapter() with all data in the device tree and no
> special case code in your driver.  This is a common device tree
> technique.
>
> For quick reference, the FDT syntax for using an alias to provide an
> ID looks like:
>   aliases {
>     i2c0 = &i2c_0;
>     i2c1 = &i2c_1;
>   };
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> CC: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> ---
>  drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 77 insertions(+), 28 deletions(-)
>

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

Re: [PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Haojian Zhuang]

On Thu, Nov 22, 2012 at 2:26 AM, Doug Anderson <dianders@chromium.org> wrote:
> The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
> present" adds support for automatically picking the bus number based
> on the alias ID.  Remove the now unnecessary code from i2c-pxa that
> did the same thing.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/i2c/busses/i2c-pxa.c |    8 +-------
>  1 files changed, 1 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index 1034d93..8ee9fa0 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
>         struct device_node *np = pdev->dev.of_node;
>         const struct of_device_id *of_id =
>                         of_match_device(i2c_pxa_dt_ids, &pdev->dev);
> -       int ret;
>
>         if (!of_id)
>                 return 1;
> -       ret = of_alias_get_id(np, "i2c");
> -       if (ret < 0) {
> -               dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> -               return ret;
> -       }
> -       pdev->id = ret;
> +       pdev->id = -1;
>         if (of_get_property(np, "mrvl,i2c-polling", NULL))
>                 i2c->use_pio = 1;
>         if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
> --
> 1.7.7.3
>

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

Re: [PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Haojian Zhuang]

On Thu, Nov 22, 2012 at 2:26 AM, Doug Anderson <dianders@chromium.org> wrote:
> The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
> present" adds support for automatically picking the bus number based
> on the alias ID.  Remove the now unnecessary code from i2c-pxa that
> did the same thing.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/i2c/busses/i2c-pxa.c |    8 +-------
>  1 files changed, 1 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index 1034d93..8ee9fa0 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
>         struct device_node *np = pdev->dev.of_node;
>         const struct of_device_id *of_id =
>                         of_match_device(i2c_pxa_dt_ids, &pdev->dev);
> -       int ret;
>
>         if (!of_id)
>                 return 1;
> -       ret = of_alias_get_id(np, "i2c");
> -       if (ret < 0) {
> -               dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> -               return ret;
> -       }
> -       pdev->id = ret;
> +       pdev->id = -1;
>         if (of_get_property(np, "mrvl,i2c-polling", NULL))
>                 i2c->use_pio = 1;
>         if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
> --
> 1.7.7.3
>

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

[PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Haojian Zhuang]

On Thu, Nov 22, 2012 at 2:26 AM, Doug Anderson <dianders@chromium.org> wrote:
> The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
> present" adds support for automatically picking the bus number based
> on the alias ID.  Remove the now unnecessary code from i2c-pxa that
> did the same thing.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/i2c/busses/i2c-pxa.c |    8 +-------
>  1 files changed, 1 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index 1034d93..8ee9fa0 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
>         struct device_node *np = pdev->dev.of_node;
>         const struct of_device_id *of_id =
>                         of_match_device(i2c_pxa_dt_ids, &pdev->dev);
> -       int ret;
>
>         if (!of_id)
>                 return 1;
> -       ret = of_alias_get_id(np, "i2c");
> -       if (ret < 0) {
> -               dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> -               return ret;
> -       }
> -       pdev->id = ret;
> +       pdev->id = -1;
>         if (of_get_property(np, "mrvl,i2c-polling", NULL))
>                 i2c->use_pio = 1;
>         if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
> --
> 1.7.7.3
>

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

Re: [PATCH 0/2] Add automatic bus number support for i2c busses with device tree

[Haojian Zhuang]

On Thu, Nov 22, 2012 at 2:26 AM, Doug Anderson <dianders@chromium.org> wrote:
> This was suggested by Mark Brown in response to a patch for adding
> this functionality only for the s3c2410 bus:
>   https://lkml.org/lkml/2012/11/20/681
>
> I have also modified the i2c-pxa driver to use this new functionality
> but have no good way to test that patch.  Hopefully someone else can
> test and ack.  The patch adding the generic functionality could go in
> even if the i2c-pxa patch needs changes.
>
>
> Doug Anderson (2):
>   i2c-core: dt: Pick i2c bus number from i2c alias if present
>   i2c: pxa: Use i2c-core to get bus number now
>
>  drivers/i2c/busses/i2c-pxa.c |    8 +---
>  drivers/i2c/i2c-core.c       |  105 ++++++++++++++++++++++++++++++-----------
>  2 files changed, 78 insertions(+), 35 deletions(-)
>
> --
> 1.7.7.3
>
>
Acked & Tested

[PATCH 0/2] Add automatic bus number support for i2c busses with device tree

[Haojian Zhuang]

On Thu, Nov 22, 2012 at 2:26 AM, Doug Anderson <dianders@chromium.org> wrote:
> This was suggested by Mark Brown in response to a patch for adding
> this functionality only for the s3c2410 bus:
>   https://lkml.org/lkml/2012/11/20/681
>
> I have also modified the i2c-pxa driver to use this new functionality
> but have no good way to test that patch.  Hopefully someone else can
> test and ack.  The patch adding the generic functionality could go in
> even if the i2c-pxa patch needs changes.
>
>
> Doug Anderson (2):
>   i2c-core: dt: Pick i2c bus number from i2c alias if present
>   i2c: pxa: Use i2c-core to get bus number now
>
>  drivers/i2c/busses/i2c-pxa.c |    8 +---
>  drivers/i2c/i2c-core.c       |  105 ++++++++++++++++++++++++++++++-----------
>  2 files changed, 78 insertions(+), 35 deletions(-)
>
> --
> 1.7.7.3
>
>
Acked & Tested

[REPOST PATCH 0/2] Add automatic bus number support for i2c busses with device tree

[Doug Anderson]

This was suggested by Mark Brown in response to a patch for adding
this functionality only for the s3c2410 bus:
  https://lkml.org/lkml/2012/11/20/681

I have also modified the i2c-pxa driver to use this new functionality.
As Haojian Zhuang has Acked this patch, I believe that this has now
been tested.

Reposting (with Haojian Zhuang's ack) in the hopes that a maintainer
will pick it up.  :)


Doug Anderson (2):
  i2c-core: dt: Pick i2c bus number from i2c alias if present
  i2c: pxa: Use i2c-core to get bus number now

 drivers/i2c/busses/i2c-pxa.c |    8 +---
 drivers/i2c/i2c-core.c       |  105 ++++++++++++++++++++++++++++++-----------
 2 files changed, 78 insertions(+), 35 deletions(-)

-- 
1.7.7.3

[REPOST PATCH 0/2] Add automatic bus number support for i2c busses with device tree

[Doug Anderson]

This was suggested by Mark Brown in response to a patch for adding
this functionality only for the s3c2410 bus:
  https://lkml.org/lkml/2012/11/20/681

I have also modified the i2c-pxa driver to use this new functionality.
As Haojian Zhuang has Acked this patch, I believe that this has now
been tested.

Reposting (with Haojian Zhuang's ack) in the hopes that a maintainer
will pick it up.  :)


Doug Anderson (2):
  i2c-core: dt: Pick i2c bus number from i2c alias if present
  i2c: pxa: Use i2c-core to get bus number now

 drivers/i2c/busses/i2c-pxa.c |    8 +---
 drivers/i2c/i2c-core.c       |  105 ++++++++++++++++++++++++++++++-----------
 2 files changed, 78 insertions(+), 35 deletions(-)

-- 
1.7.7.3

[REPOST PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

This allows you to get the equivalent functionality of
i2c_add_numbered_adapter() with all data in the device tree and no
special case code in your driver.  This is a common device tree
technique.

For quick reference, the FDT syntax for using an alias to provide an
ID looks like:
  aliases {
    i2c0 = &i2c_0;
    i2c1 = &i2c_1;
  };

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 77 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e388590..a60ed6d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -921,13 +921,81 @@ out_list:
 }
 
 /**
+ * i2c_get_number_from_dt - get the adapter number based on dt alias
+ * @adap: the adapter to look at
+ *
+ * Check whether there's an alias in the FDT that gives an ID for this i2c
+ * device.  Use an alias like "i2c<nr>", like:
+ *   aliases {
+ *     i2c0 = &i2c_0;
+ *     i2c1 = &i2c_1;
+ *   };
+ *
+ * Returns the ID if found.  If no alias is found returns -1.
+ */
+static int i2c_get_number_from_dt(struct i2c_adapter *adap)
+{
+	struct device *dev = &adap->dev;
+	int id;
+
+	if (!dev->of_node)
+		return -1;
+
+	id = of_alias_get_id(dev->of_node, "i2c");
+	if (id < 0)
+		return -1;
+	return id;
+}
+
+/**
+ * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
+ * @adap: the adapter to register (with adap->nr initialized)
+ * Context: can sleep
+ *
+ * See i2c_add_numbered_adapter() for details.
+ */
+static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
+{
+	int	id;
+	int	status;
+
+	/* Handled by wrappers */
+	BUG_ON(adap->nr == -1);
+
+	if (adap->nr & ~MAX_IDR_MASK)
+		return -EINVAL;
+
+retry:
+	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
+		return -ENOMEM;
+
+	mutex_lock(&core_lock);
+	/* "above" here means "above or equal to", sigh;
+	 * we need the "equal to" result to force the result
+	 */
+	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
+	if (status == 0 && id != adap->nr) {
+		status = -EBUSY;
+		idr_remove(&i2c_adapter_idr, id);
+	}
+	mutex_unlock(&core_lock);
+	if (status == -EAGAIN)
+		goto retry;
+
+	if (status == 0)
+		status = i2c_register_adapter(adap);
+	return status;
+}
+
+/**
  * i2c_add_adapter - declare i2c adapter, use dynamic bus number
  * @adapter: the adapter to add
  * Context: can sleep
  *
  * This routine is used to declare an I2C adapter when its bus number
- * doesn't matter.  Examples: for I2C adapters dynamically added by
- * USB links or PCI plugin cards.
+ * doesn't matter or when its bus number is specified by an dt alias.
+ * Examples of bases when the bus number doesn't matter: I2C adapters
+ * dynamically added by USB links or PCI plugin cards.
  *
  * When this returns zero, a new bus number was allocated and stored
  * in adap->nr, and the specified adapter became available for clients.
@@ -937,6 +1005,12 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
 {
 	int	id, res = 0;
 
+	id = i2c_get_number_from_dt(adapter);
+	if (id >= 0) {
+		adapter->nr = id;
+		return _i2c_add_numbered_adapter(adapter);
+	}
+
 retry:
 	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
 		return -ENOMEM;
@@ -983,34 +1057,9 @@ EXPORT_SYMBOL(i2c_add_adapter);
  */
 int i2c_add_numbered_adapter(struct i2c_adapter *adap)
 {
-	int	id;
-	int	status;
-
 	if (adap->nr == -1) /* -1 means dynamically assign bus id */
 		return i2c_add_adapter(adap);
-	if (adap->nr & ~MAX_IDR_MASK)
-		return -EINVAL;
-
-retry:
-	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
-		return -ENOMEM;
-
-	mutex_lock(&core_lock);
-	/* "above" here means "above or equal to", sigh;
-	 * we need the "equal to" result to force the result
-	 */
-	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
-	if (status == 0 && id != adap->nr) {
-		status = -EBUSY;
-		idr_remove(&i2c_adapter_idr, id);
-	}
-	mutex_unlock(&core_lock);
-	if (status == -EAGAIN)
-		goto retry;
-
-	if (status == 0)
-		status = i2c_register_adapter(adap);
-	return status;
+	return _i2c_add_numbered_adapter(adap);
 }
 EXPORT_SYMBOL_GPL(i2c_add_numbered_adapter);
 
-- 
1.7.7.3

[REPOST PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

This allows you to get the equivalent functionality of
i2c_add_numbered_adapter() with all data in the device tree and no
special case code in your driver.  This is a common device tree
technique.

For quick reference, the FDT syntax for using an alias to provide an
ID looks like:
  aliases {
    i2c0 = &i2c_0;
    i2c1 = &i2c_1;
  };

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 77 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e388590..a60ed6d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -921,13 +921,81 @@ out_list:
 }
 
 /**
+ * i2c_get_number_from_dt - get the adapter number based on dt alias
+ * @adap: the adapter to look at
+ *
+ * Check whether there's an alias in the FDT that gives an ID for this i2c
+ * device.  Use an alias like "i2c<nr>", like:
+ *   aliases {
+ *     i2c0 = &i2c_0;
+ *     i2c1 = &i2c_1;
+ *   };
+ *
+ * Returns the ID if found.  If no alias is found returns -1.
+ */
+static int i2c_get_number_from_dt(struct i2c_adapter *adap)
+{
+	struct device *dev = &adap->dev;
+	int id;
+
+	if (!dev->of_node)
+		return -1;
+
+	id = of_alias_get_id(dev->of_node, "i2c");
+	if (id < 0)
+		return -1;
+	return id;
+}
+
+/**
+ * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
+ * @adap: the adapter to register (with adap->nr initialized)
+ * Context: can sleep
+ *
+ * See i2c_add_numbered_adapter() for details.
+ */
+static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
+{
+	int	id;
+	int	status;
+
+	/* Handled by wrappers */
+	BUG_ON(adap->nr == -1);
+
+	if (adap->nr & ~MAX_IDR_MASK)
+		return -EINVAL;
+
+retry:
+	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
+		return -ENOMEM;
+
+	mutex_lock(&core_lock);
+	/* "above" here means "above or equal to", sigh;
+	 * we need the "equal to" result to force the result
+	 */
+	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
+	if (status == 0 && id != adap->nr) {
+		status = -EBUSY;
+		idr_remove(&i2c_adapter_idr, id);
+	}
+	mutex_unlock(&core_lock);
+	if (status == -EAGAIN)
+		goto retry;
+
+	if (status == 0)
+		status = i2c_register_adapter(adap);
+	return status;
+}
+
+/**
  * i2c_add_adapter - declare i2c adapter, use dynamic bus number
  * @adapter: the adapter to add
  * Context: can sleep
  *
  * This routine is used to declare an I2C adapter when its bus number
- * doesn't matter.  Examples: for I2C adapters dynamically added by
- * USB links or PCI plugin cards.
+ * doesn't matter or when its bus number is specified by an dt alias.
+ * Examples of bases when the bus number doesn't matter: I2C adapters
+ * dynamically added by USB links or PCI plugin cards.
  *
  * When this returns zero, a new bus number was allocated and stored
  * in adap->nr, and the specified adapter became available for clients.
@@ -937,6 +1005,12 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
 {
 	int	id, res = 0;
 
+	id = i2c_get_number_from_dt(adapter);
+	if (id >= 0) {
+		adapter->nr = id;
+		return _i2c_add_numbered_adapter(adapter);
+	}
+
 retry:
 	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
 		return -ENOMEM;
@@ -983,34 +1057,9 @@ EXPORT_SYMBOL(i2c_add_adapter);
  */
 int i2c_add_numbered_adapter(struct i2c_adapter *adap)
 {
-	int	id;
-	int	status;
-
 	if (adap->nr == -1) /* -1 means dynamically assign bus id */
 		return i2c_add_adapter(adap);
-	if (adap->nr & ~MAX_IDR_MASK)
-		return -EINVAL;
-
-retry:
-	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
-		return -ENOMEM;
-
-	mutex_lock(&core_lock);
-	/* "above" here means "above or equal to", sigh;
-	 * we need the "equal to" result to force the result
-	 */
-	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
-	if (status == 0 && id != adap->nr) {
-		status = -EBUSY;
-		idr_remove(&i2c_adapter_idr, id);
-	}
-	mutex_unlock(&core_lock);
-	if (status == -EAGAIN)
-		goto retry;
-
-	if (status == 0)
-		status = i2c_register_adapter(adap);
-	return status;
+	return _i2c_add_numbered_adapter(adap);
 }
 EXPORT_SYMBOL_GPL(i2c_add_numbered_adapter);
 
-- 
1.7.7.3

[REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
present" adds support for automatically picking the bus number based
on the alias ID.  Remove the now unnecessary code from i2c-pxa that
did the same thing.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/i2c/busses/i2c-pxa.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..8ee9fa0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 			of_match_device(i2c_pxa_dt_ids, &pdev->dev);
-	int ret;
 
 	if (!of_id)
 		return 1;
-	ret = of_alias_get_id(np, "i2c");
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
-		return ret;
-	}
-	pdev->id = ret;
+	pdev->id = -1;
 	if (of_get_property(np, "mrvl,i2c-polling", NULL))
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
-- 
1.7.7.3

[REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
present" adds support for automatically picking the bus number based
on the alias ID.  Remove the now unnecessary code from i2c-pxa that
did the same thing.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/i2c/busses/i2c-pxa.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..8ee9fa0 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 			of_match_device(i2c_pxa_dt_ids, &pdev->dev);
-	int ret;
 
 	if (!of_id)
 		return 1;
-	ret = of_alias_get_id(np, "i2c");
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
-		return ret;
-	}
-	pdev->id = ret;
+	pdev->id = -1;
 	if (of_get_property(np, "mrvl,i2c-polling", NULL))
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
-- 
1.7.7.3

Re: [REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Sylwester Nawrocki]

Hi,

On 01/11/2013 06:57 PM, Doug Anderson wrote:
> The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
> present" adds support for automatically picking the bus number based
> on the alias ID.  Remove the now unnecessary code from i2c-pxa that
> did the same thing.
>
> Signed-off-by: Doug Anderson<dianders@chromium.org>
> Acked-by: Haojian Zhuang<haojian.zhuang@gmail.com>
> ---
>   drivers/i2c/busses/i2c-pxa.c |    8 +-------
>   1 files changed, 1 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index 1034d93..8ee9fa0 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
>   	struct device_node *np = pdev->dev.of_node;
>   	const struct of_device_id *of_id =
>   			of_match_device(i2c_pxa_dt_ids,&pdev->dev);
> -	int ret;
>
>   	if (!of_id)
>   		return 1;
> -	ret = of_alias_get_id(np, "i2c");
> -	if (ret<  0) {
> -		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> -		return ret;
> -	}
> -	pdev->id = ret;
> +	pdev->id = -1;

I think assignments like this are illegal. At this point the device is
already registered and modifying pdev->id can cause issues at the core
code.

It might be better to have the bus number stored in struct pxa_i2c,
initialized with pdev->id value for non-dt and now with -1 for dt case.

I realize it is not something your patch is intended to deal with,
but since you're touching this code maybe it's worth to fix that issue
as well ?

--

Thanks,
Sylwester

[REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Sylwester Nawrocki]

Hi,

On 01/11/2013 06:57 PM, Doug Anderson wrote:
> The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
> present" adds support for automatically picking the bus number based
> on the alias ID.  Remove the now unnecessary code from i2c-pxa that
> did the same thing.
>
> Signed-off-by: Doug Anderson<dianders@chromium.org>
> Acked-by: Haojian Zhuang<haojian.zhuang@gmail.com>
> ---
>   drivers/i2c/busses/i2c-pxa.c |    8 +-------
>   1 files changed, 1 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index 1034d93..8ee9fa0 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -1053,16 +1053,10 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
>   	struct device_node *np = pdev->dev.of_node;
>   	const struct of_device_id *of_id =
>   			of_match_device(i2c_pxa_dt_ids,&pdev->dev);
> -	int ret;
>
>   	if (!of_id)
>   		return 1;
> -	ret = of_alias_get_id(np, "i2c");
> -	if (ret<  0) {
> -		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
> -		return ret;
> -	}
> -	pdev->id = ret;
> +	pdev->id = -1;

I think assignments like this are illegal. At this point the device is
already registered and modifying pdev->id can cause issues at the core
code.

It might be better to have the bus number stored in struct pxa_i2c,
initialized with pdev->id value for non-dt and now with -1 for dt case.

I realize it is not something your patch is intended to deal with,
but since you're touching this code maybe it's worth to fix that issue
as well ?

--

Thanks,
Sylwester

Re: [REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

Sylwester,

Thanks for the review...

On Fri, Jan 11, 2013 at 2:12 PM, Sylwester Nawrocki
<sylvester.nawrocki@gmail.com> wrote:
>> -       ret = of_alias_get_id(np, "i2c");
>> -       if (ret<  0) {
>> -               dev_err(&pdev->dev, "failed to get alias id, errno %d\n",
>> ret);
>> -               return ret;
>> -       }
>> -       pdev->id = ret;
>> +       pdev->id = -1;
>
>
> I think assignments like this are illegal. At this point the device is
> already registered and modifying pdev->id can cause issues at the core
> code.

Good catch.  I think the old code is just as illegal since it was also
assigning to pdev->id, but I'm happy to spin the patch.

> It might be better to have the bus number stored in struct pxa_i2c,
> initialized with pdev->id value for non-dt and now with -1 for dt case.

I'll just init i2c->adap.nr before calling i2c_pxa_probe_dt().  Then
i2c_pxa_probe_dt() can adjust the number.  Hopefully this is OK.

> I realize it is not something your patch is intended to deal with,
> but since you're touching this code maybe it's worth to fix that issue
> as well ?

Sure, I've spun it.  While doing this I realized a few other issues
relating to the ID number.  Specifically:

* We were using the id (as %u) when creating 'i2c->adap.name'.  This
probably gave a very nonsensical ID.  I'm just going to remove the
device number from the adap.name.  That matches what i2c-s3c2410.c
does.  As far as I can tell the adap.name isn't actually used for
much.

* The name was being used in request_irq().  I've changed this to be
like i2c-s3c2410 and use dev_name().  On my current board that means
that the name comes from the table in of_platform_populate().  ...but
that table is supposed to be going away.  If I remove that i2c parts
of that table I get names like '12c60000.i2c', which should be fine
for passing to request_irq().


-Doug

Re: [REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

Sylwester,

Thanks for the review...

On Fri, Jan 11, 2013 at 2:12 PM, Sylwester Nawrocki
<sylvester.nawrocki@gmail.com> wrote:
>> -       ret = of_alias_get_id(np, "i2c");
>> -       if (ret<  0) {
>> -               dev_err(&pdev->dev, "failed to get alias id, errno %d\n",
>> ret);
>> -               return ret;
>> -       }
>> -       pdev->id = ret;
>> +       pdev->id = -1;
>
>
> I think assignments like this are illegal. At this point the device is
> already registered and modifying pdev->id can cause issues at the core
> code.

Good catch.  I think the old code is just as illegal since it was also
assigning to pdev->id, but I'm happy to spin the patch.

> It might be better to have the bus number stored in struct pxa_i2c,
> initialized with pdev->id value for non-dt and now with -1 for dt case.

I'll just init i2c->adap.nr before calling i2c_pxa_probe_dt().  Then
i2c_pxa_probe_dt() can adjust the number.  Hopefully this is OK.

> I realize it is not something your patch is intended to deal with,
> but since you're touching this code maybe it's worth to fix that issue
> as well ?

Sure, I've spun it.  While doing this I realized a few other issues
relating to the ID number.  Specifically:

* We were using the id (as %u) when creating 'i2c->adap.name'.  This
probably gave a very nonsensical ID.  I'm just going to remove the
device number from the adap.name.  That matches what i2c-s3c2410.c
does.  As far as I can tell the adap.name isn't actually used for
much.

* The name was being used in request_irq().  I've changed this to be
like i2c-s3c2410 and use dev_name().  On my current board that means
that the name comes from the table in of_platform_populate().  ...but
that table is supposed to be going away.  If I remove that i2c parts
of that table I get names like '12c60000.i2c', which should be fine
for passing to request_irq().


-Doug

[REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

Sylwester,

Thanks for the review...

On Fri, Jan 11, 2013 at 2:12 PM, Sylwester Nawrocki
<sylvester.nawrocki@gmail.com> wrote:
>> -       ret = of_alias_get_id(np, "i2c");
>> -       if (ret<  0) {
>> -               dev_err(&pdev->dev, "failed to get alias id, errno %d\n",
>> ret);
>> -               return ret;
>> -       }
>> -       pdev->id = ret;
>> +       pdev->id = -1;
>
>
> I think assignments like this are illegal. At this point the device is
> already registered and modifying pdev->id can cause issues at the core
> code.

Good catch.  I think the old code is just as illegal since it was also
assigning to pdev->id, but I'm happy to spin the patch.

> It might be better to have the bus number stored in struct pxa_i2c,
> initialized with pdev->id value for non-dt and now with -1 for dt case.

I'll just init i2c->adap.nr before calling i2c_pxa_probe_dt().  Then
i2c_pxa_probe_dt() can adjust the number.  Hopefully this is OK.

> I realize it is not something your patch is intended to deal with,
> but since you're touching this code maybe it's worth to fix that issue
> as well ?

Sure, I've spun it.  While doing this I realized a few other issues
relating to the ID number.  Specifically:

* We were using the id (as %u) when creating 'i2c->adap.name'.  This
probably gave a very nonsensical ID.  I'm just going to remove the
device number from the adap.name.  That matches what i2c-s3c2410.c
does.  As far as I can tell the adap.name isn't actually used for
much.

* The name was being used in request_irq().  I've changed this to be
like i2c-s3c2410 and use dev_name().  On my current board that means
that the name comes from the table in of_platform_populate().  ...but
that table is supposed to be going away.  If I remove that i2c parts
of that table I get names like '12c60000.i2c', which should be fine
for passing to request_irq().


-Doug

[PATCH v2 0/2] Add automatic bus number support for i2c busses with device tree

[Doug Anderson]

This was suggested by Mark Brown in response to a patch for adding
this functionality only for the s3c2410 bus:
  https://lkml.org/lkml/2012/11/20/681

I have also modified the i2c-pxa driver to use this new functionality.

Changes in v2:
- No longer tweak pdev->id as per Sylwester Nawrocki.
- No longer add the dev ID to the adap.name.  Other drivers don't
  include the device ID here and it doesn't make sense with
  dynamically (or automatically) allocated IDs.
- Use dev_name(&dev->dev) to register for the IRQ; this matches what
  the i2c-s3c2410.c does and handles dynamically allocated IDs.
- This change was only compile-tested (corgi_defconfig), since I don't
  have access to a board that uses this driver.

Doug Anderson (2):
  i2c-core: dt: Pick i2c bus number from i2c alias if present
  i2c: pxa: Use i2c-core to get bus number now

 drivers/i2c/busses/i2c-pxa.c |   20 ++++----
 drivers/i2c/i2c-core.c       |  105 ++++++++++++++++++++++++++++++-----------
 2 files changed, 86 insertions(+), 39 deletions(-)

-- 
1.7.7.3

[PATCH v2 0/2] Add automatic bus number support for i2c busses with device tree

[Doug Anderson]

This was suggested by Mark Brown in response to a patch for adding
this functionality only for the s3c2410 bus:
  https://lkml.org/lkml/2012/11/20/681

I have also modified the i2c-pxa driver to use this new functionality.

Changes in v2:
- No longer tweak pdev->id as per Sylwester Nawrocki.
- No longer add the dev ID to the adap.name.  Other drivers don't
  include the device ID here and it doesn't make sense with
  dynamically (or automatically) allocated IDs.
- Use dev_name(&dev->dev) to register for the IRQ; this matches what
  the i2c-s3c2410.c does and handles dynamically allocated IDs.
- This change was only compile-tested (corgi_defconfig), since I don't
  have access to a board that uses this driver.

Doug Anderson (2):
  i2c-core: dt: Pick i2c bus number from i2c alias if present
  i2c: pxa: Use i2c-core to get bus number now

 drivers/i2c/busses/i2c-pxa.c |   20 ++++----
 drivers/i2c/i2c-core.c       |  105 ++++++++++++++++++++++++++++++-----------
 2 files changed, 86 insertions(+), 39 deletions(-)

-- 
1.7.7.3

[PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

This allows you to get the equivalent functionality of
i2c_add_numbered_adapter() with all data in the device tree and no
special case code in your driver.  This is a common device tree
technique.

For quick reference, the FDT syntax for using an alias to provide an
ID looks like:
  aliases {
    i2c0 = &i2c_0;
    i2c1 = &i2c_1;
  };

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
Changes in v2: None

 drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 77 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e388590..a60ed6d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -921,13 +921,81 @@ out_list:
 }
 
 /**
+ * i2c_get_number_from_dt - get the adapter number based on dt alias
+ * @adap: the adapter to look at
+ *
+ * Check whether there's an alias in the FDT that gives an ID for this i2c
+ * device.  Use an alias like "i2c<nr>", like:
+ *   aliases {
+ *     i2c0 = &i2c_0;
+ *     i2c1 = &i2c_1;
+ *   };
+ *
+ * Returns the ID if found.  If no alias is found returns -1.
+ */
+static int i2c_get_number_from_dt(struct i2c_adapter *adap)
+{
+	struct device *dev = &adap->dev;
+	int id;
+
+	if (!dev->of_node)
+		return -1;
+
+	id = of_alias_get_id(dev->of_node, "i2c");
+	if (id < 0)
+		return -1;
+	return id;
+}
+
+/**
+ * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
+ * @adap: the adapter to register (with adap->nr initialized)
+ * Context: can sleep
+ *
+ * See i2c_add_numbered_adapter() for details.
+ */
+static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
+{
+	int	id;
+	int	status;
+
+	/* Handled by wrappers */
+	BUG_ON(adap->nr == -1);
+
+	if (adap->nr & ~MAX_IDR_MASK)
+		return -EINVAL;
+
+retry:
+	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
+		return -ENOMEM;
+
+	mutex_lock(&core_lock);
+	/* "above" here means "above or equal to", sigh;
+	 * we need the "equal to" result to force the result
+	 */
+	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
+	if (status == 0 && id != adap->nr) {
+		status = -EBUSY;
+		idr_remove(&i2c_adapter_idr, id);
+	}
+	mutex_unlock(&core_lock);
+	if (status == -EAGAIN)
+		goto retry;
+
+	if (status == 0)
+		status = i2c_register_adapter(adap);
+	return status;
+}
+
+/**
  * i2c_add_adapter - declare i2c adapter, use dynamic bus number
  * @adapter: the adapter to add
  * Context: can sleep
  *
  * This routine is used to declare an I2C adapter when its bus number
- * doesn't matter.  Examples: for I2C adapters dynamically added by
- * USB links or PCI plugin cards.
+ * doesn't matter or when its bus number is specified by an dt alias.
+ * Examples of bases when the bus number doesn't matter: I2C adapters
+ * dynamically added by USB links or PCI plugin cards.
  *
  * When this returns zero, a new bus number was allocated and stored
  * in adap->nr, and the specified adapter became available for clients.
@@ -937,6 +1005,12 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
 {
 	int	id, res = 0;
 
+	id = i2c_get_number_from_dt(adapter);
+	if (id >= 0) {
+		adapter->nr = id;
+		return _i2c_add_numbered_adapter(adapter);
+	}
+
 retry:
 	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
 		return -ENOMEM;
@@ -983,34 +1057,9 @@ EXPORT_SYMBOL(i2c_add_adapter);
  */
 int i2c_add_numbered_adapter(struct i2c_adapter *adap)
 {
-	int	id;
-	int	status;
-
 	if (adap->nr == -1) /* -1 means dynamically assign bus id */
 		return i2c_add_adapter(adap);
-	if (adap->nr & ~MAX_IDR_MASK)
-		return -EINVAL;
-
-retry:
-	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
-		return -ENOMEM;
-
-	mutex_lock(&core_lock);
-	/* "above" here means "above or equal to", sigh;
-	 * we need the "equal to" result to force the result
-	 */
-	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
-	if (status == 0 && id != adap->nr) {
-		status = -EBUSY;
-		idr_remove(&i2c_adapter_idr, id);
-	}
-	mutex_unlock(&core_lock);
-	if (status == -EAGAIN)
-		goto retry;
-
-	if (status == 0)
-		status = i2c_register_adapter(adap);
-	return status;
+	return _i2c_add_numbered_adapter(adap);
 }
 EXPORT_SYMBOL_GPL(i2c_add_numbered_adapter);
 
-- 
1.7.7.3

[PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

This allows you to get the equivalent functionality of
i2c_add_numbered_adapter() with all data in the device tree and no
special case code in your driver.  This is a common device tree
technique.

For quick reference, the FDT syntax for using an alias to provide an
ID looks like:
  aliases {
    i2c0 = &i2c_0;
    i2c1 = &i2c_1;
  };

Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
Changes in v2: None

 drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 77 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index e388590..a60ed6d 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -921,13 +921,81 @@ out_list:
 }
 
 /**
+ * i2c_get_number_from_dt - get the adapter number based on dt alias
+ * @adap: the adapter to look at
+ *
+ * Check whether there's an alias in the FDT that gives an ID for this i2c
+ * device.  Use an alias like "i2c<nr>", like:
+ *   aliases {
+ *     i2c0 = &i2c_0;
+ *     i2c1 = &i2c_1;
+ *   };
+ *
+ * Returns the ID if found.  If no alias is found returns -1.
+ */
+static int i2c_get_number_from_dt(struct i2c_adapter *adap)
+{
+	struct device *dev = &adap->dev;
+	int id;
+
+	if (!dev->of_node)
+		return -1;
+
+	id = of_alias_get_id(dev->of_node, "i2c");
+	if (id < 0)
+		return -1;
+	return id;
+}
+
+/**
+ * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
+ * @adap: the adapter to register (with adap->nr initialized)
+ * Context: can sleep
+ *
+ * See i2c_add_numbered_adapter() for details.
+ */
+static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
+{
+	int	id;
+	int	status;
+
+	/* Handled by wrappers */
+	BUG_ON(adap->nr == -1);
+
+	if (adap->nr & ~MAX_IDR_MASK)
+		return -EINVAL;
+
+retry:
+	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
+		return -ENOMEM;
+
+	mutex_lock(&core_lock);
+	/* "above" here means "above or equal to", sigh;
+	 * we need the "equal to" result to force the result
+	 */
+	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
+	if (status == 0 && id != adap->nr) {
+		status = -EBUSY;
+		idr_remove(&i2c_adapter_idr, id);
+	}
+	mutex_unlock(&core_lock);
+	if (status == -EAGAIN)
+		goto retry;
+
+	if (status == 0)
+		status = i2c_register_adapter(adap);
+	return status;
+}
+
+/**
  * i2c_add_adapter - declare i2c adapter, use dynamic bus number
  * @adapter: the adapter to add
  * Context: can sleep
  *
  * This routine is used to declare an I2C adapter when its bus number
- * doesn't matter.  Examples: for I2C adapters dynamically added by
- * USB links or PCI plugin cards.
+ * doesn't matter or when its bus number is specified by an dt alias.
+ * Examples of bases when the bus number doesn't matter: I2C adapters
+ * dynamically added by USB links or PCI plugin cards.
  *
  * When this returns zero, a new bus number was allocated and stored
  * in adap->nr, and the specified adapter became available for clients.
@@ -937,6 +1005,12 @@ int i2c_add_adapter(struct i2c_adapter *adapter)
 {
 	int	id, res = 0;
 
+	id = i2c_get_number_from_dt(adapter);
+	if (id >= 0) {
+		adapter->nr = id;
+		return _i2c_add_numbered_adapter(adapter);
+	}
+
 retry:
 	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
 		return -ENOMEM;
@@ -983,34 +1057,9 @@ EXPORT_SYMBOL(i2c_add_adapter);
  */
 int i2c_add_numbered_adapter(struct i2c_adapter *adap)
 {
-	int	id;
-	int	status;
-
 	if (adap->nr == -1) /* -1 means dynamically assign bus id */
 		return i2c_add_adapter(adap);
-	if (adap->nr & ~MAX_IDR_MASK)
-		return -EINVAL;
-
-retry:
-	if (idr_pre_get(&i2c_adapter_idr, GFP_KERNEL) == 0)
-		return -ENOMEM;
-
-	mutex_lock(&core_lock);
-	/* "above" here means "above or equal to", sigh;
-	 * we need the "equal to" result to force the result
-	 */
-	status = idr_get_new_above(&i2c_adapter_idr, adap, adap->nr, &id);
-	if (status == 0 && id != adap->nr) {
-		status = -EBUSY;
-		idr_remove(&i2c_adapter_idr, id);
-	}
-	mutex_unlock(&core_lock);
-	if (status == -EAGAIN)
-		goto retry;
-
-	if (status == 0)
-		status = i2c_register_adapter(adap);
-	return status;
+	return _i2c_add_numbered_adapter(adap);
 }
 EXPORT_SYMBOL_GPL(i2c_add_numbered_adapter);
 
-- 
1.7.7.3

[PATCH v2 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
present" adds support for automatically picking the bus number based
on the alias ID.  Remove the now unnecessary code from i2c-pxa that
did the same thing.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v2:
- No longer tweak pdev->id as per Sylwester Nawrocki.
- No longer add the dev ID to the adap.name.  Other drivers don't
  include the device ID here and it doesn't make sense with
  dynamically (or automatically) allocated IDs.
- Use dev_name(&dev->dev) to register for the IRQ; this matches what
  the i2c-s3c2410.c does and handles dynamically allocated IDs.
- This change was only compile-tested (corgi_defconfig), since I don't
  have access to a board that uses this driver.

 drivers/i2c/busses/i2c-pxa.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..705cc9f 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1053,16 +1053,13 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 			of_match_device(i2c_pxa_dt_ids, &pdev->dev);
-	int ret;
 
 	if (!of_id)
 		return 1;
-	ret = of_alias_get_id(np, "i2c");
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
-		return ret;
-	}
-	pdev->id = ret;
+
+	/* For device tree we always use the dynamic or alias-assigned ID */
+	i2c->adap.nr = -1;
+
 	if (of_get_property(np, "mrvl,i2c-polling", NULL))
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
@@ -1100,6 +1097,9 @@ static int i2c_pxa_probe(struct platform_device *dev)
 		goto emalloc;
 	}
 
+	/* Default adapter num to device id; i2c_pxa_probe_dt can override. */
+	i2c->adap.nr = dev->id;
+
 	ret = i2c_pxa_probe_dt(dev, i2c, &i2c_type);
 	if (ret > 0)
 		ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
@@ -1124,9 +1124,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	spin_lock_init(&i2c->lock);
 	init_waitqueue_head(&i2c->wait);
 
-	i2c->adap.nr = dev->id;
-	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "pxa_i2c-i2c.%u",
-		 i2c->adap.nr);
+	strlcpy(i2c->adap.name, "pxa_i2c-i2c", sizeof(i2c->adap.name));
 
 	i2c->clk = clk_get(&dev->dev, NULL);
 	if (IS_ERR(i2c->clk)) {
@@ -1169,7 +1167,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	} else {
 		i2c->adap.algo = &i2c_pxa_algorithm;
 		ret = request_irq(irq, i2c_pxa_handler, IRQF_SHARED,
-				  i2c->adap.name, i2c);
+				  dev_name(&dev->dev), i2c);
 		if (ret)
 			goto ereqirq;
 	}
-- 
1.7.7.3

[PATCH v2 2/2] i2c: pxa: Use i2c-core to get bus number now

[Doug Anderson]

The commit: "i2c-core: dt: Pick i2c bus number from i2c alias if
present" adds support for automatically picking the bus number based
on the alias ID.  Remove the now unnecessary code from i2c-pxa that
did the same thing.

Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v2:
- No longer tweak pdev->id as per Sylwester Nawrocki.
- No longer add the dev ID to the adap.name.  Other drivers don't
  include the device ID here and it doesn't make sense with
  dynamically (or automatically) allocated IDs.
- Use dev_name(&dev->dev) to register for the IRQ; this matches what
  the i2c-s3c2410.c does and handles dynamically allocated IDs.
- This change was only compile-tested (corgi_defconfig), since I don't
  have access to a board that uses this driver.

 drivers/i2c/busses/i2c-pxa.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 1034d93..705cc9f 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1053,16 +1053,13 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
 	struct device_node *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 			of_match_device(i2c_pxa_dt_ids, &pdev->dev);
-	int ret;
 
 	if (!of_id)
 		return 1;
-	ret = of_alias_get_id(np, "i2c");
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
-		return ret;
-	}
-	pdev->id = ret;
+
+	/* For device tree we always use the dynamic or alias-assigned ID */
+	i2c->adap.nr = -1;
+
 	if (of_get_property(np, "mrvl,i2c-polling", NULL))
 		i2c->use_pio = 1;
 	if (of_get_property(np, "mrvl,i2c-fast-mode", NULL))
@@ -1100,6 +1097,9 @@ static int i2c_pxa_probe(struct platform_device *dev)
 		goto emalloc;
 	}
 
+	/* Default adapter num to device id; i2c_pxa_probe_dt can override. */
+	i2c->adap.nr = dev->id;
+
 	ret = i2c_pxa_probe_dt(dev, i2c, &i2c_type);
 	if (ret > 0)
 		ret = i2c_pxa_probe_pdata(dev, i2c, &i2c_type);
@@ -1124,9 +1124,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	spin_lock_init(&i2c->lock);
 	init_waitqueue_head(&i2c->wait);
 
-	i2c->adap.nr = dev->id;
-	snprintf(i2c->adap.name, sizeof(i2c->adap.name), "pxa_i2c-i2c.%u",
-		 i2c->adap.nr);
+	strlcpy(i2c->adap.name, "pxa_i2c-i2c", sizeof(i2c->adap.name));
 
 	i2c->clk = clk_get(&dev->dev, NULL);
 	if (IS_ERR(i2c->clk)) {
@@ -1169,7 +1167,7 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	} else {
 		i2c->adap.algo = &i2c_pxa_algorithm;
 		ret = request_irq(irq, i2c_pxa_handler, IRQF_SHARED,
-				  i2c->adap.name, i2c);
+				  dev_name(&dev->dev), i2c);
 		if (ret)
 			goto ereqirq;
 	}
-- 
1.7.7.3

Re: [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Olof Johansson]

On Mon, Jan 14, 2013 at 10:53:21AM -0800, Doug Anderson wrote:
> This allows you to get the equivalent functionality of
> i2c_add_numbered_adapter() with all data in the device tree and no
> special case code in your driver.  This is a common device tree
> technique.
> 
> For quick reference, the FDT syntax for using an alias to provide an
> ID looks like:
>   aliases {
>     i2c0 = &i2c_0;
>     i2c1 = &i2c_1;
>   };
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

The call path for i2c_add_numbered_adapter() with nr == -1 is a little
awkward now, but I don't see much how it can be improved much.

Acked-by: Olof Johansson <olof@lixom.net>


-Olof

Re: [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Olof Johansson]

On Mon, Jan 14, 2013 at 10:53:21AM -0800, Doug Anderson wrote:
> This allows you to get the equivalent functionality of
> i2c_add_numbered_adapter() with all data in the device tree and no
> special case code in your driver.  This is a common device tree
> technique.
> 
> For quick reference, the FDT syntax for using an alias to provide an
> ID looks like:
>   aliases {
>     i2c0 = &i2c_0;
>     i2c1 = &i2c_1;
>   };
> 
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Acked-by: Haojian Zhuang <haojian.zhuang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

The call path for i2c_add_numbered_adapter() with nr == -1 is a little
awkward now, but I don't see much how it can be improved much.

Acked-by: Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>


-Olof

[PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Olof Johansson]

On Mon, Jan 14, 2013 at 10:53:21AM -0800, Doug Anderson wrote:
> This allows you to get the equivalent functionality of
> i2c_add_numbered_adapter() with all data in the device tree and no
> special case code in your driver.  This is a common device tree
> technique.
> 
> For quick reference, the FDT syntax for using an alias to provide an
> ID looks like:
>   aliases {
>     i2c0 = &i2c_0;
>     i2c1 = &i2c_1;
>   };
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

The call path for i2c_add_numbered_adapter() with nr == -1 is a little
awkward now, but I don't see much how it can be improved much.

Acked-by: Olof Johansson <olof@lixom.net>


-Olof

Re: [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Wolfram Sang]

Hi Doug,

On Mon, Jan 14, 2013 at 10:53:21AM -0800, Doug Anderson wrote:
> This allows you to get the equivalent functionality of
> i2c_add_numbered_adapter() with all data in the device tree and no
> special case code in your driver.  This is a common device tree
> technique.
> 
> For quick reference, the FDT syntax for using an alias to provide an
> ID looks like:
>   aliases {
>     i2c0 = &i2c_0;
>     i2c1 = &i2c_1;
>   };
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
> ---
> Changes in v2: None
> 
>  drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 77 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index e388590..a60ed6d 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -921,13 +921,81 @@ out_list:
>  }
>  
>  /**
> + * i2c_get_number_from_dt - get the adapter number based on dt alias
> + * @adap: the adapter to look at
> + *
> + * Check whether there's an alias in the FDT that gives an ID for this i2c
> + * device.  Use an alias like "i2c<nr>", like:
> + *   aliases {
> + *     i2c0 = &i2c_0;
> + *     i2c1 = &i2c_1;
> + *   };
> + *
> + * Returns the ID if found.  If no alias is found returns -1.
> + */
> +static int i2c_get_number_from_dt(struct i2c_adapter *adap)

i2c_get_id_from_dt()?

> +{
> +	struct device *dev = &adap->dev;
> +	int id;
> +
> +	if (!dev->of_node)
> +		return -1;

-ESOMETHING?

> +
> +	id = of_alias_get_id(dev->of_node, "i2c");
> +	if (id < 0)
> +		return -1;
> +	return id;

Simply 'return of_alias_get_id(...)'? Even more, since this function
boils down to calling of_alias_get_id only and is only used once, I'd
think we can implement that directly and drop this function. That
shouldn't hurt readability.

> +}
> +
> +/**
> + * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
> + * @adap: the adapter to register (with adap->nr initialized)
> + * Context: can sleep
> + *
> + * See i2c_add_numbered_adapter() for details.
> + */
> +static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)

All other internal functions are prefixed with '__'.

> +{
> +	int	id;
> +	int	status;
> +
> +	/* Handled by wrappers */
> +	BUG_ON(adap->nr == -1);

Is that a reason to halt the kernel? WARN and bailing out would do IMO.

> +
> +	if (adap->nr & ~MAX_IDR_MASK)
> +		return -EINVAL;

Note the idr-cleanup series from Tejun Heo. Given that his series is
scheduled for v3.9, I'd like to have your patches on top of his.

Thanks,

   Wolfram

Re: [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Wolfram Sang]

Hi Doug,

On Mon, Jan 14, 2013 at 10:53:21AM -0800, Doug Anderson wrote:
> This allows you to get the equivalent functionality of
> i2c_add_numbered_adapter() with all data in the device tree and no
> special case code in your driver.  This is a common device tree
> technique.
> 
> For quick reference, the FDT syntax for using an alias to provide an
> ID looks like:
>   aliases {
>     i2c0 = &i2c_0;
>     i2c1 = &i2c_1;
>   };
> 
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Acked-by: Haojian Zhuang <haojian.zhuang-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> Changes in v2: None
> 
>  drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 77 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index e388590..a60ed6d 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -921,13 +921,81 @@ out_list:
>  }
>  
>  /**
> + * i2c_get_number_from_dt - get the adapter number based on dt alias
> + * @adap: the adapter to look at
> + *
> + * Check whether there's an alias in the FDT that gives an ID for this i2c
> + * device.  Use an alias like "i2c<nr>", like:
> + *   aliases {
> + *     i2c0 = &i2c_0;
> + *     i2c1 = &i2c_1;
> + *   };
> + *
> + * Returns the ID if found.  If no alias is found returns -1.
> + */
> +static int i2c_get_number_from_dt(struct i2c_adapter *adap)

i2c_get_id_from_dt()?

> +{
> +	struct device *dev = &adap->dev;
> +	int id;
> +
> +	if (!dev->of_node)
> +		return -1;

-ESOMETHING?

> +
> +	id = of_alias_get_id(dev->of_node, "i2c");
> +	if (id < 0)
> +		return -1;
> +	return id;

Simply 'return of_alias_get_id(...)'? Even more, since this function
boils down to calling of_alias_get_id only and is only used once, I'd
think we can implement that directly and drop this function. That
shouldn't hurt readability.

> +}
> +
> +/**
> + * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
> + * @adap: the adapter to register (with adap->nr initialized)
> + * Context: can sleep
> + *
> + * See i2c_add_numbered_adapter() for details.
> + */
> +static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)

All other internal functions are prefixed with '__'.

> +{
> +	int	id;
> +	int	status;
> +
> +	/* Handled by wrappers */
> +	BUG_ON(adap->nr == -1);

Is that a reason to halt the kernel? WARN and bailing out would do IMO.

> +
> +	if (adap->nr & ~MAX_IDR_MASK)
> +		return -EINVAL;

Note the idr-cleanup series from Tejun Heo. Given that his series is
scheduled for v3.9, I'd like to have your patches on top of his.

Thanks,

   Wolfram

[PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Wolfram Sang]

Hi Doug,

On Mon, Jan 14, 2013 at 10:53:21AM -0800, Doug Anderson wrote:
> This allows you to get the equivalent functionality of
> i2c_add_numbered_adapter() with all data in the device tree and no
> special case code in your driver.  This is a common device tree
> technique.
> 
> For quick reference, the FDT syntax for using an alias to provide an
> ID looks like:
>   aliases {
>     i2c0 = &i2c_0;
>     i2c1 = &i2c_1;
>   };
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
> ---
> Changes in v2: None
> 
>  drivers/i2c/i2c-core.c |  105 +++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 77 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index e388590..a60ed6d 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -921,13 +921,81 @@ out_list:
>  }
>  
>  /**
> + * i2c_get_number_from_dt - get the adapter number based on dt alias
> + * @adap: the adapter to look at
> + *
> + * Check whether there's an alias in the FDT that gives an ID for this i2c
> + * device.  Use an alias like "i2c<nr>", like:
> + *   aliases {
> + *     i2c0 = &i2c_0;
> + *     i2c1 = &i2c_1;
> + *   };
> + *
> + * Returns the ID if found.  If no alias is found returns -1.
> + */
> +static int i2c_get_number_from_dt(struct i2c_adapter *adap)

i2c_get_id_from_dt()?

> +{
> +	struct device *dev = &adap->dev;
> +	int id;
> +
> +	if (!dev->of_node)
> +		return -1;

-ESOMETHING?

> +
> +	id = of_alias_get_id(dev->of_node, "i2c");
> +	if (id < 0)
> +		return -1;
> +	return id;

Simply 'return of_alias_get_id(...)'? Even more, since this function
boils down to calling of_alias_get_id only and is only used once, I'd
think we can implement that directly and drop this function. That
shouldn't hurt readability.

> +}
> +
> +/**
> + * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
> + * @adap: the adapter to register (with adap->nr initialized)
> + * Context: can sleep
> + *
> + * See i2c_add_numbered_adapter() for details.
> + */
> +static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)

All other internal functions are prefixed with '__'.

> +{
> +	int	id;
> +	int	status;
> +
> +	/* Handled by wrappers */
> +	BUG_ON(adap->nr == -1);

Is that a reason to halt the kernel? WARN and bailing out would do IMO.

> +
> +	if (adap->nr & ~MAX_IDR_MASK)
> +		return -EINVAL;

Note the idr-cleanup series from Tejun Heo. Given that his series is
scheduled for v3.9, I'd like to have your patches on top of his.

Thanks,

   Wolfram

Re: [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

Wolfram,

Thanks for the review.  New patch was just sent.  :)

On Sun, Feb 10, 2013 at 4:19 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> +static int i2c_get_number_from_dt(struct i2c_adapter *adap)
>
> i2c_get_id_from_dt()?

Done.


>> +     if (!dev->of_node)
>> +             return -1;
>
> -ESOMETHING?

Function has been removed, as per below.


>> +
>> +     id = of_alias_get_id(dev->of_node, "i2c");
>> +     if (id < 0)
>> +             return -1;
>> +     return id;
>
> Simply 'return of_alias_get_id(...)'? Even more, since this function
> boils down to calling of_alias_get_id only and is only used once, I'd
> think we can implement that directly and drop this function. That
> shouldn't hurt readability.

Good point.  Done.

>> +/**
>> + * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
>> + * @adap: the adapter to register (with adap->nr initialized)
>> + * Context: can sleep
>> + *
>> + * See i2c_add_numbered_adapter() for details.
>> + */
>> +static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
>
> All other internal functions are prefixed with '__'.

Done.


>> +{
>> +     int     id;
>> +     int     status;
>> +
>> +     /* Handled by wrappers */
>> +     BUG_ON(adap->nr == -1);
>
> Is that a reason to halt the kernel? WARN and bailing out would do IMO.

Done.


>> +
>> +     if (adap->nr & ~MAX_IDR_MASK)
>> +             return -EINVAL;
>
> Note the idr-cleanup series from Tejun Heo. Given that his series is
> scheduled for v3.9, I'd like to have your patches on top of his.

Done.

Re: [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

Wolfram,

Thanks for the review.  New patch was just sent.  :)

On Sun, Feb 10, 2013 at 4:19 AM, Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
>> +static int i2c_get_number_from_dt(struct i2c_adapter *adap)
>
> i2c_get_id_from_dt()?

Done.


>> +     if (!dev->of_node)
>> +             return -1;
>
> -ESOMETHING?

Function has been removed, as per below.


>> +
>> +     id = of_alias_get_id(dev->of_node, "i2c");
>> +     if (id < 0)
>> +             return -1;
>> +     return id;
>
> Simply 'return of_alias_get_id(...)'? Even more, since this function
> boils down to calling of_alias_get_id only and is only used once, I'd
> think we can implement that directly and drop this function. That
> shouldn't hurt readability.

Good point.  Done.

>> +/**
>> + * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
>> + * @adap: the adapter to register (with adap->nr initialized)
>> + * Context: can sleep
>> + *
>> + * See i2c_add_numbered_adapter() for details.
>> + */
>> +static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
>
> All other internal functions are prefixed with '__'.

Done.


>> +{
>> +     int     id;
>> +     int     status;
>> +
>> +     /* Handled by wrappers */
>> +     BUG_ON(adap->nr == -1);
>
> Is that a reason to halt the kernel? WARN and bailing out would do IMO.

Done.


>> +
>> +     if (adap->nr & ~MAX_IDR_MASK)
>> +             return -EINVAL;
>
> Note the idr-cleanup series from Tejun Heo. Given that his series is
> scheduled for v3.9, I'd like to have your patches on top of his.

Done.

[PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present

[Doug Anderson]

Wolfram,

Thanks for the review.  New patch was just sent.  :)

On Sun, Feb 10, 2013 at 4:19 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> +static int i2c_get_number_from_dt(struct i2c_adapter *adap)
>
> i2c_get_id_from_dt()?

Done.


>> +     if (!dev->of_node)
>> +             return -1;
>
> -ESOMETHING?

Function has been removed, as per below.


>> +
>> +     id = of_alias_get_id(dev->of_node, "i2c");
>> +     if (id < 0)
>> +             return -1;
>> +     return id;
>
> Simply 'return of_alias_get_id(...)'? Even more, since this function
> boils down to calling of_alias_get_id only and is only used once, I'd
> think we can implement that directly and drop this function. That
> shouldn't hurt readability.

Good point.  Done.

>> +/**
>> + * _i2c_add_numbered_adapter - i2c_add_numbered_adapter where nr is never -1
>> + * @adap: the adapter to register (with adap->nr initialized)
>> + * Context: can sleep
>> + *
>> + * See i2c_add_numbered_adapter() for details.
>> + */
>> +static int _i2c_add_numbered_adapter(struct i2c_adapter *adap)
>
> All other internal functions are prefixed with '__'.

Done.


>> +{
>> +     int     id;
>> +     int     status;
>> +
>> +     /* Handled by wrappers */
>> +     BUG_ON(adap->nr == -1);
>
> Is that a reason to halt the kernel? WARN and bailing out would do IMO.

Done.


>> +
>> +     if (adap->nr & ~MAX_IDR_MASK)
>> +             return -EINVAL;
>
> Note the idr-cleanup series from Tejun Heo. Given that his series is
> scheduled for v3.9, I'd like to have your patches on top of his.

Done.