[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 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 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.

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

[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

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

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

[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 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

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

[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 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

[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.