* [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4 @ 2012-11-20 22:27 Doug Anderson 2012-11-20 22:27 ` [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id Doug Anderson ` (2 more replies) 0 siblings, 3 replies; 23+ messages in thread From: Doug Anderson @ 2012-11-20 22:27 UTC (permalink / raw) To: linux-samsung-soc Cc: linux-i2c, Olof Johansson, Thomas Abraham, Kukjin Kim, Doug Anderson, Russell King, Tomasz Figa, Kyungmin Park, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id 2012-11-20 22:27 [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4 Doug Anderson @ 2012-11-20 22:27 ` Doug Anderson 2012-11-21 4:09 ` Mark Brown ` (2 more replies) 2012-11-21 7:25 ` [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4 Olof Johansson 2012-11-21 9:43 ` Kukjin Kim 2 siblings, 3 replies; 23+ messages in thread From: Doug Anderson @ 2012-11-20 22:27 UTC (permalink / raw) To: linux-samsung-soc Cc: linux-i2c, Olof Johansson, Thomas Abraham, Kukjin Kim, Padmavathi Venna, Doug Anderson, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id 2012-11-20 22:27 ` [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id Doug Anderson @ 2012-11-21 4:09 ` Mark Brown 2012-11-21 18:33 ` Doug Anderson 2012-11-21 9:43 ` Kukjin Kim [not found] ` <1353522362-25519-1-git-send-email-dianders@chromium.org> 2 siblings, 1 reply; 23+ messages in thread From: Mark Brown @ 2012-11-21 4:09 UTC (permalink / raw) To: Doug Anderson Cc: linux-samsung-soc, linux-i2c, Olof Johansson, Thomas Abraham, Kukjin Kim, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-kernel 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id 2012-11-21 4:09 ` Mark Brown @ 2012-11-21 18:33 ` Doug Anderson 2012-11-22 6:56 ` Kukjin Kim 0 siblings, 1 reply; 23+ messages in thread From: Doug Anderson @ 2012-11-21 18:33 UTC (permalink / raw) To: Mark Brown Cc: linux-samsung-soc, linux-i2c, Olof Johansson, Thomas Abraham, Kukjin Kim, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id 2012-11-21 18:33 ` Doug Anderson @ 2012-11-22 6:56 ` Kukjin Kim 0 siblings, 0 replies; 23+ messages in thread From: Kukjin Kim @ 2012-11-22 6:56 UTC (permalink / raw) To: 'Doug Anderson', 'Mark Brown' Cc: linux-samsung-soc, linux-i2c, 'Olof Johansson', 'Thomas Abraham', 'Padmavathi Venna', 'Ben Dooks', 'Wolfram Sang', linux-arm-kernel, linux-kernel 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id 2012-11-20 22:27 ` [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id Doug Anderson 2012-11-21 4:09 ` Mark Brown @ 2012-11-21 9:43 ` Kukjin Kim [not found] ` <1353522362-25519-1-git-send-email-dianders@chromium.org> 2 siblings, 0 replies; 23+ messages in thread From: Kukjin Kim @ 2012-11-21 9:43 UTC (permalink / raw) To: 'Doug Anderson', linux-samsung-soc Cc: linux-i2c, 'Olof Johansson', 'Thomas Abraham', 'Padmavathi Venna', 'Ben Dooks', 'Wolfram Sang', linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <1353522362-25519-1-git-send-email-dianders@chromium.org>]
* [PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present [not found] ` <1353522362-25519-1-git-send-email-dianders@chromium.org> @ 2012-11-21 18:26 ` Doug Anderson 2012-12-05 1:45 ` Haojian Zhuang 2013-01-11 17:57 ` [REPOST PATCH 0/2] Add automatic bus number support for i2c busses with device tree Doug Anderson 2012-11-21 18:26 ` [PATCH " Doug Anderson 1 sibling, 2 replies; 23+ messages in thread From: Doug Anderson @ 2012-11-21 18:26 UTC (permalink / raw) To: linux-i2c, Mark Brown Cc: Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Doug Anderson, linux-kernel 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present 2012-11-21 18:26 ` [PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson @ 2012-12-05 1:45 ` Haojian Zhuang 2013-01-11 17:57 ` [REPOST PATCH 0/2] Add automatic bus number support for i2c busses with device tree Doug Anderson 1 sibling, 0 replies; 23+ messages in thread From: Haojian Zhuang @ 2012-12-05 1:45 UTC (permalink / raw) To: Doug Anderson Cc: linux-i2c, Mark Brown, Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, linux-kernel 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> ^ permalink raw reply [flat|nested] 23+ messages in thread
* [REPOST PATCH 0/2] Add automatic bus number support for i2c busses with device tree 2012-11-21 18:26 ` [PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson 2012-12-05 1:45 ` Haojian Zhuang @ 2013-01-11 17:57 ` Doug Anderson 2013-01-11 17:57 ` [REPOST PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson 2013-01-11 17:57 ` [REPOST PATCH " Doug Anderson 1 sibling, 2 replies; 23+ messages in thread From: Doug Anderson @ 2013-01-11 17:57 UTC (permalink / raw) To: linux-i2c, Mark Brown Cc: Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Doug Anderson, Karol Lewandowski, linux-kernel, Andrew Morton 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [REPOST PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present 2013-01-11 17:57 ` [REPOST PATCH 0/2] Add automatic bus number support for i2c busses with device tree Doug Anderson @ 2013-01-11 17:57 ` Doug Anderson 2013-01-14 18:53 ` [PATCH v2 0/2] Add automatic bus number support for i2c busses with device tree Doug Anderson 2013-01-11 17:57 ` [REPOST PATCH " Doug Anderson 1 sibling, 1 reply; 23+ messages in thread From: Doug Anderson @ 2013-01-11 17:57 UTC (permalink / raw) To: linux-i2c, Mark Brown Cc: Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Doug Anderson, linux-kernel 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 0/2] Add automatic bus number support for i2c busses with device tree 2013-01-11 17:57 ` [REPOST PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson @ 2013-01-14 18:53 ` Doug Anderson 2013-01-14 18:53 ` [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson 2013-01-14 18:53 ` [PATCH v2 2/2] i2c: pxa: Use i2c-core to get bus number now Doug Anderson 0 siblings, 2 replies; 23+ messages in thread From: Doug Anderson @ 2013-01-14 18:53 UTC (permalink / raw) To: linux-i2c, Mark Brown Cc: Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Sylwester Nawrocki, Doug Anderson, Karol Lewandowski, linux-kernel, Haojian Zhuang, Andrew Morton 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present 2013-01-14 18:53 ` [PATCH v2 0/2] Add automatic bus number support for i2c busses with device tree Doug Anderson @ 2013-01-14 18:53 ` Doug Anderson 2013-01-15 6:34 ` Olof Johansson 2013-02-10 12:19 ` Wolfram Sang 2013-01-14 18:53 ` [PATCH v2 2/2] i2c: pxa: Use i2c-core to get bus number now Doug Anderson 1 sibling, 2 replies; 23+ messages in thread From: Doug Anderson @ 2013-01-14 18:53 UTC (permalink / raw) To: linux-i2c, Mark Brown Cc: Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Sylwester Nawrocki, Doug Anderson, linux-kernel 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present 2013-01-14 18:53 ` [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson @ 2013-01-15 6:34 ` Olof Johansson 2013-02-10 12:19 ` Wolfram Sang 1 sibling, 0 replies; 23+ messages in thread From: Olof Johansson @ 2013-01-15 6:34 UTC (permalink / raw) To: Doug Anderson Cc: linux-i2c, Mark Brown, Kukjin Kim, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Sylwester Nawrocki, linux-kernel 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present 2013-01-14 18:53 ` [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson 2013-01-15 6:34 ` Olof Johansson @ 2013-02-10 12:19 ` Wolfram Sang 2013-02-12 0:48 ` Doug Anderson 1 sibling, 1 reply; 23+ messages in thread From: Wolfram Sang @ 2013-02-10 12:19 UTC (permalink / raw) To: Doug Anderson Cc: linux-i2c, Mark Brown, Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Sylwester Nawrocki, linux-kernel 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present 2013-02-10 12:19 ` Wolfram Sang @ 2013-02-12 0:48 ` Doug Anderson 0 siblings, 0 replies; 23+ messages in thread From: Doug Anderson @ 2013-02-12 0:48 UTC (permalink / raw) To: Wolfram Sang Cc: linux-i2c, Mark Brown, Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Sylwester Nawrocki, linux-kernel 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 2/2] i2c: pxa: Use i2c-core to get bus number now 2013-01-14 18:53 ` [PATCH v2 0/2] Add automatic bus number support for i2c busses with device tree Doug Anderson 2013-01-14 18:53 ` [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson @ 2013-01-14 18:53 ` Doug Anderson 1 sibling, 0 replies; 23+ messages in thread From: Doug Anderson @ 2013-01-14 18:53 UTC (permalink / raw) To: linux-i2c, Mark Brown Cc: Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Sylwester Nawrocki, Doug Anderson, Andrew Morton, Karol Lewandowski, Haojian Zhuang, linux-kernel 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now 2013-01-11 17:57 ` [REPOST PATCH 0/2] Add automatic bus number support for i2c busses with device tree Doug Anderson 2013-01-11 17:57 ` [REPOST PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson @ 2013-01-11 17:57 ` Doug Anderson 2013-01-11 22:12 ` Sylwester Nawrocki 1 sibling, 1 reply; 23+ messages in thread From: Doug Anderson @ 2013-01-11 17:57 UTC (permalink / raw) To: linux-i2c, Mark Brown Cc: Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Doug Anderson, Andrew Morton, Karol Lewandowski, linux-kernel 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now 2013-01-11 17:57 ` [REPOST PATCH " Doug Anderson @ 2013-01-11 22:12 ` Sylwester Nawrocki 2013-01-14 18:52 ` Doug Anderson 0 siblings, 1 reply; 23+ messages in thread From: Sylwester Nawrocki @ 2013-01-11 22:12 UTC (permalink / raw) To: Doug Anderson Cc: linux-i2c, Mark Brown, Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Andrew Morton, Karol Lewandowski, linux-kernel 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [REPOST PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now 2013-01-11 22:12 ` Sylwester Nawrocki @ 2013-01-14 18:52 ` Doug Anderson 0 siblings, 0 replies; 23+ messages in thread From: Doug Anderson @ 2013-01-14 18:52 UTC (permalink / raw) To: Sylwester Nawrocki Cc: linux-i2c, Mark Brown, Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Andrew Morton, Karol Lewandowski, linux-kernel 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now [not found] ` <1353522362-25519-1-git-send-email-dianders@chromium.org> 2012-11-21 18:26 ` [PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson @ 2012-11-21 18:26 ` Doug Anderson 2012-12-05 1:45 ` Haojian Zhuang 1 sibling, 1 reply; 23+ messages in thread From: Doug Anderson @ 2012-11-21 18:26 UTC (permalink / raw) To: linux-i2c, Mark Brown Cc: Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Doug Anderson, Andrew Morton, Karol Lewandowski, linux-kernel 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/2] i2c: pxa: Use i2c-core to get bus number now 2012-11-21 18:26 ` [PATCH " Doug Anderson @ 2012-12-05 1:45 ` Haojian Zhuang 0 siblings, 0 replies; 23+ messages in thread From: Haojian Zhuang @ 2012-12-05 1:45 UTC (permalink / raw) To: Doug Anderson Cc: linux-i2c, Mark Brown, Kukjin Kim, Olof Johansson, Thomas Abraham, Padmavathi Venna, Ben Dooks, Wolfram Sang, linux-arm-kernel, linux-samsung-soc, Haojian Zhuang, Arnd Bergmann, Andrew Morton, Karol Lewandowski, linux-kernel 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> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4 2012-11-20 22:27 [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4 Doug Anderson 2012-11-20 22:27 ` [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id Doug Anderson @ 2012-11-21 7:25 ` Olof Johansson 2012-11-21 9:43 ` Kukjin Kim 2 siblings, 0 replies; 23+ messages in thread From: Olof Johansson @ 2012-11-21 7:25 UTC (permalink / raw) To: Doug Anderson Cc: linux-samsung-soc, linux-i2c, Thomas Abraham, Kukjin Kim, Russell King, Tomasz Figa, Kyungmin Park, linux-arm-kernel, linux-kernel 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4 2012-11-20 22:27 [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4 Doug Anderson 2012-11-20 22:27 ` [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id Doug Anderson 2012-11-21 7:25 ` [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4 Olof Johansson @ 2012-11-21 9:43 ` Kukjin Kim 2 siblings, 0 replies; 23+ messages in thread From: Kukjin Kim @ 2012-11-21 9:43 UTC (permalink / raw) To: 'Doug Anderson', linux-samsung-soc Cc: linux-i2c, 'Olof Johansson', 'Thomas Abraham', 'Russell King', 'Tomasz Figa', 'Kyungmin Park', linux-arm-kernel, linux-kernel 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. ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2013-02-12 0:48 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-11-20 22:27 [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4 Doug Anderson 2012-11-20 22:27 ` [PATCH 2/2] i2c: s3c2410: Get the i2c bus number from alias id Doug Anderson 2012-11-21 4:09 ` Mark Brown 2012-11-21 18:33 ` Doug Anderson 2012-11-22 6:56 ` Kukjin Kim 2012-11-21 9:43 ` Kukjin Kim [not found] ` <1353522362-25519-1-git-send-email-dianders@chromium.org> 2012-11-21 18:26 ` [PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson 2012-12-05 1:45 ` Haojian Zhuang 2013-01-11 17:57 ` [REPOST PATCH 0/2] Add automatic bus number support for i2c busses with device tree Doug Anderson 2013-01-11 17:57 ` [REPOST PATCH 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson 2013-01-14 18:53 ` [PATCH v2 0/2] Add automatic bus number support for i2c busses with device tree Doug Anderson 2013-01-14 18:53 ` [PATCH v2 1/2] i2c-core: dt: Pick i2c bus number from i2c alias if present Doug Anderson 2013-01-15 6:34 ` Olof Johansson 2013-02-10 12:19 ` Wolfram Sang 2013-02-12 0:48 ` Doug Anderson 2013-01-14 18:53 ` [PATCH v2 2/2] i2c: pxa: Use i2c-core to get bus number now Doug Anderson 2013-01-11 17:57 ` [REPOST PATCH " Doug Anderson 2013-01-11 22:12 ` Sylwester Nawrocki 2013-01-14 18:52 ` Doug Anderson 2012-11-21 18:26 ` [PATCH " Doug Anderson 2012-12-05 1:45 ` Haojian Zhuang 2012-11-21 7:25 ` [PATCH 1/2] ARM: EXYNOS: Add aliases for i2c controller for exynos4 Olof Johansson 2012-11-21 9:43 ` Kukjin Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).