* [PATCH] i2c-gpio: Add support for deferred probing
@ 2012-07-19 11:19 Jean Delvare
[not found] ` <20120719131924.7672f99e-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Jean Delvare @ 2012-07-19 11:19 UTC (permalink / raw)
To: Linux I2C; +Cc: Haavard Skinnemoen
GPIOs may not be available immediately when i2c-gpio looks for them.
Implement support for deferred probing so that probing can be
attempted again later when GPIO pins are finally available.
Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Haavard Skinnemoen <hskinnemoen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
A little more changes were needed than I initially thought, because we
want to check the pins before we allocate memory. Otherwise we would
have a possibly large number of memory allocation and freeing cycles
until GPIO pins are finally available.
Note: I wrote this patch for someone else, I can't test it, so I would
appreciate if someone could at least test and confirm that I did not
break anything in the standard (non-deferred) case. Thanks!
drivers/i2c/busses/i2c-gpio.c | 76 +++++++++++++++++++++++++++--------------
1 file changed, 51 insertions(+), 25 deletions(-)
--- linux-3.5-rc5.orig/drivers/i2c/busses/i2c-gpio.c 2012-06-05 16:22:59.000000000 +0200
+++ linux-3.5-rc5/drivers/i2c/busses/i2c-gpio.c 2012-07-07 10:53:40.911407245 +0200
@@ -85,23 +85,30 @@ static int i2c_gpio_getscl(void *data)
return gpio_get_value(pdata->scl_pin);
}
-static int __devinit of_i2c_gpio_probe(struct device_node *np,
- struct i2c_gpio_platform_data *pdata)
+static int __devinit of_i2c_gpio_get_pins(struct device_node *np,
+ unsigned int *sda_pin,
+ unsigned int *scl_pin)
{
- u32 reg;
-
if (of_gpio_count(np) < 2)
return -ENODEV;
- pdata->sda_pin = of_get_gpio(np, 0);
- pdata->scl_pin = of_get_gpio(np, 1);
+ *sda_pin = of_get_gpio(np, 0);
+ *scl_pin = of_get_gpio(np, 1);
- if (!gpio_is_valid(pdata->sda_pin) || !gpio_is_valid(pdata->scl_pin)) {
+ if (!gpio_is_valid(*sda_pin) || !gpio_is_valid(*scl_pin)) {
pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
- np->full_name, pdata->sda_pin, pdata->scl_pin);
+ np->full_name, *sda_pin, *scl_pin);
return -ENODEV;
}
+ return 0;
+}
+
+static void __devinit of_i2c_gpio_get_props(struct device_node *np,
+ struct i2c_gpio_platform_data *pdata)
+{
+ u32 reg;
+
of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
if (!of_property_read_u32(np, "i2c-gpio,timeout-ms", ®))
@@ -113,8 +120,6 @@ static int __devinit of_i2c_gpio_probe(s
of_property_read_bool(np, "i2c-gpio,scl-open-drain");
pdata->scl_is_output_only =
of_property_read_bool(np, "i2c-gpio,scl-output-only");
-
- return 0;
}
static int __devinit i2c_gpio_probe(struct platform_device *pdev)
@@ -123,31 +128,52 @@ static int __devinit i2c_gpio_probe(stru
struct i2c_gpio_platform_data *pdata;
struct i2c_algo_bit_data *bit_data;
struct i2c_adapter *adap;
+ unsigned int sda_pin, scl_pin;
int ret;
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
- adap = &priv->adap;
- bit_data = &priv->bit_data;
- pdata = &priv->pdata;
-
+ /* First get the GPIO pins; if it fails, we'll defer the probe. */
if (pdev->dev.of_node) {
- ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
+ ret = of_i2c_gpio_get_pins(pdev->dev.of_node,
+ &sda_pin, &scl_pin);
if (ret)
return ret;
} else {
if (!pdev->dev.platform_data)
return -ENXIO;
- memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+ pdata = pdev->dev.platform_data;
+ sda_pin = pdata->sda_pin;
+ scl_pin = pdata->scl_pin;
}
- ret = gpio_request(pdata->sda_pin, "sda");
- if (ret)
+ ret = gpio_request(sda_pin, "sda");
+ if (ret) {
+ if (ret == -EINVAL)
+ ret = -EPROBE_DEFER; /* Try again later */
goto err_request_sda;
- ret = gpio_request(pdata->scl_pin, "scl");
- if (ret)
+ }
+ ret = gpio_request(scl_pin, "scl");
+ if (ret) {
+ if (ret == -EINVAL)
+ ret = -EPROBE_DEFER; /* Try again later */
goto err_request_scl;
+ }
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ ret = -ENOMEM;
+ goto err_add_bus;
+ }
+ adap = &priv->adap;
+ bit_data = &priv->bit_data;
+ pdata = &priv->pdata;
+
+ if (pdev->dev.of_node) {
+ pdata->sda_pin = sda_pin;
+ pdata->scl_pin = scl_pin;
+ of_i2c_gpio_get_props(pdev->dev.of_node, pdata);
+ } else {
+ memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+ }
if (pdata->sda_is_open_drain) {
gpio_direction_output(pdata->sda_pin, 1);
@@ -207,9 +233,9 @@ static int __devinit i2c_gpio_probe(stru
return 0;
err_add_bus:
- gpio_free(pdata->scl_pin);
+ gpio_free(scl_pin);
err_request_scl:
- gpio_free(pdata->sda_pin);
+ gpio_free(sda_pin);
err_request_sda:
return ret;
}
--
Jean Delvare
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c-gpio: Add support for deferred probing
[not found] ` <20120719131924.7672f99e-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
@ 2012-08-31 12:38 ` Jean Delvare
0 siblings, 0 replies; 10+ messages in thread
From: Jean Delvare @ 2012-08-31 12:38 UTC (permalink / raw)
To: Linux I2C; +Cc: Haavard Skinnemoen
On Thu, 19 Jul 2012 13:19:24 +0200, Jean Delvare wrote:
> GPIOs may not be available immediately when i2c-gpio looks for them.
> Implement support for deferred probing so that probing can be
> attempted again later when GPIO pins are finally available.
>
> Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: Haavard Skinnemoen <hskinnemoen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> A little more changes were needed than I initially thought, because we
> want to check the pins before we allocate memory. Otherwise we would
> have a possibly large number of memory allocation and freeing cycles
> until GPIO pins are finally available.
>
> Note: I wrote this patch for someone else, I can't test it, so I would
> appreciate if someone could at least test and confirm that I did not
> break anything in the standard (non-deferred) case. Thanks!
Anyone, please? Anyone using i2c-gpio, please test this patch and
report how it goes. Thanks.
> drivers/i2c/busses/i2c-gpio.c | 76 +++++++++++++++++++++++++++--------------
> 1 file changed, 51 insertions(+), 25 deletions(-)
>
> --- linux-3.5-rc5.orig/drivers/i2c/busses/i2c-gpio.c 2012-06-05 16:22:59.000000000 +0200
> +++ linux-3.5-rc5/drivers/i2c/busses/i2c-gpio.c 2012-07-07 10:53:40.911407245 +0200
> @@ -85,23 +85,30 @@ static int i2c_gpio_getscl(void *data)
> return gpio_get_value(pdata->scl_pin);
> }
>
> -static int __devinit of_i2c_gpio_probe(struct device_node *np,
> - struct i2c_gpio_platform_data *pdata)
> +static int __devinit of_i2c_gpio_get_pins(struct device_node *np,
> + unsigned int *sda_pin,
> + unsigned int *scl_pin)
> {
> - u32 reg;
> -
> if (of_gpio_count(np) < 2)
> return -ENODEV;
>
> - pdata->sda_pin = of_get_gpio(np, 0);
> - pdata->scl_pin = of_get_gpio(np, 1);
> + *sda_pin = of_get_gpio(np, 0);
> + *scl_pin = of_get_gpio(np, 1);
>
> - if (!gpio_is_valid(pdata->sda_pin) || !gpio_is_valid(pdata->scl_pin)) {
> + if (!gpio_is_valid(*sda_pin) || !gpio_is_valid(*scl_pin)) {
> pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
> - np->full_name, pdata->sda_pin, pdata->scl_pin);
> + np->full_name, *sda_pin, *scl_pin);
> return -ENODEV;
> }
>
> + return 0;
> +}
> +
> +static void __devinit of_i2c_gpio_get_props(struct device_node *np,
> + struct i2c_gpio_platform_data *pdata)
> +{
> + u32 reg;
> +
> of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
>
> if (!of_property_read_u32(np, "i2c-gpio,timeout-ms", ®))
> @@ -113,8 +120,6 @@ static int __devinit of_i2c_gpio_probe(s
> of_property_read_bool(np, "i2c-gpio,scl-open-drain");
> pdata->scl_is_output_only =
> of_property_read_bool(np, "i2c-gpio,scl-output-only");
> -
> - return 0;
> }
>
> static int __devinit i2c_gpio_probe(struct platform_device *pdev)
> @@ -123,31 +128,52 @@ static int __devinit i2c_gpio_probe(stru
> struct i2c_gpio_platform_data *pdata;
> struct i2c_algo_bit_data *bit_data;
> struct i2c_adapter *adap;
> + unsigned int sda_pin, scl_pin;
> int ret;
>
> - priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> - if (!priv)
> - return -ENOMEM;
> - adap = &priv->adap;
> - bit_data = &priv->bit_data;
> - pdata = &priv->pdata;
> -
> + /* First get the GPIO pins; if it fails, we'll defer the probe. */
> if (pdev->dev.of_node) {
> - ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
> + ret = of_i2c_gpio_get_pins(pdev->dev.of_node,
> + &sda_pin, &scl_pin);
> if (ret)
> return ret;
> } else {
> if (!pdev->dev.platform_data)
> return -ENXIO;
> - memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
> + pdata = pdev->dev.platform_data;
> + sda_pin = pdata->sda_pin;
> + scl_pin = pdata->scl_pin;
> }
>
> - ret = gpio_request(pdata->sda_pin, "sda");
> - if (ret)
> + ret = gpio_request(sda_pin, "sda");
> + if (ret) {
> + if (ret == -EINVAL)
> + ret = -EPROBE_DEFER; /* Try again later */
> goto err_request_sda;
> - ret = gpio_request(pdata->scl_pin, "scl");
> - if (ret)
> + }
> + ret = gpio_request(scl_pin, "scl");
> + if (ret) {
> + if (ret == -EINVAL)
> + ret = -EPROBE_DEFER; /* Try again later */
> goto err_request_scl;
> + }
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv) {
> + ret = -ENOMEM;
> + goto err_add_bus;
> + }
> + adap = &priv->adap;
> + bit_data = &priv->bit_data;
> + pdata = &priv->pdata;
> +
> + if (pdev->dev.of_node) {
> + pdata->sda_pin = sda_pin;
> + pdata->scl_pin = scl_pin;
> + of_i2c_gpio_get_props(pdev->dev.of_node, pdata);
> + } else {
> + memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
> + }
>
> if (pdata->sda_is_open_drain) {
> gpio_direction_output(pdata->sda_pin, 1);
> @@ -207,9 +233,9 @@ static int __devinit i2c_gpio_probe(stru
> return 0;
>
> err_add_bus:
> - gpio_free(pdata->scl_pin);
> + gpio_free(scl_pin);
> err_request_scl:
> - gpio_free(pdata->sda_pin);
> + gpio_free(sda_pin);
> err_request_sda:
> return ret;
> }
>
>
--
Jean Delvare
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c-gpio: Add support for deferred probing
[not found] ` <20130228120140.127ebb91-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2013-03-01 3:58 ` Bo Shen
2013-03-22 11:56 ` Wolfram Sang
@ 2013-03-27 8:21 ` Wolfram Sang
2 siblings, 0 replies; 10+ messages in thread
From: Wolfram Sang @ 2013-03-27 8:21 UTC (permalink / raw)
To: Jean Delvare
Cc: Linux I2C, Bo Shen, Karol Lewandowski, Jean-Christophe PLAGNIOL-VILLARD
On Thu, Feb 28, 2013 at 12:01:40PM +0100, Jean Delvare wrote:
> GPIOs may not be available immediately when i2c-gpio looks for them.
> Implement support for deferred probing so that probing can be
> attempted again later when GPIO pins are finally available.
>
> Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: Haavard Skinnemoen <hskinnemoen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Applied to for-next, thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c-gpio: Add support for deferred probing
[not found] ` <20130326110908.GC8553-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
@ 2013-03-26 12:22 ` Jean Delvare
0 siblings, 0 replies; 10+ messages in thread
From: Jean Delvare @ 2013-03-26 12:22 UTC (permalink / raw)
To: Wolfram Sang
Cc: Linux I2C, Bo Shen, Karol Lewandowski, Jean-Christophe PLAGNIOL-VILLARD
Hi Wolfram,
On Tue, 26 Mar 2013 12:09:08 +0100, Wolfram Sang wrote:
> > If you still have a concern about the types used, please clarify and
> > let me know what change you expect.
>
> Leave it. I think the fragile part is gpio_is_valid() but this is truly
> outside the scope of this patch.
> (...)
> > Note that my patch doesn't introduce the gpio_request() calls, they
> > were there before, so this decision is actually independent from my
> > patch, and even if we decide to switch to using gpio_request_array(),
> > I'd rather do it in a separate patch for clarity.
>
> I don't fully get it. Do you want to appl gpio_request() to this patch?
> Otherwise, I'd take it as is.
As I do not understand your question, I'd say you take my patch as is :)
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c-gpio: Add support for deferred probing
[not found] ` <20130324114301.4b5efc34-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
@ 2013-03-26 11:09 ` Wolfram Sang
[not found] ` <20130326110908.GC8553-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Wolfram Sang @ 2013-03-26 11:09 UTC (permalink / raw)
To: Jean Delvare
Cc: Linux I2C, Bo Shen, Karol Lewandowski, Jean-Christophe PLAGNIOL-VILLARD
> What do you mean? In <linux/gpio.h> I see:
>
> struct gpio {
> unsigned gpio;
> (...)
>
> static inline int gpio_get_value(unsigned int gpio)
> {
> return __gpio_get_value(gpio);
> }
>
> And in <linux/i2c-gpio.h>:
>
> struct i2c_gpio_platform_data {
> unsigned int sda_pin;
> unsigned int scl_pin;
> (...)
I remembered this paragraph from Documentation/gpio.txt:
===
If you want to initialize a structure with an invalid GPIO number, use
some negative number (perhaps "-EINVAL"); that will never be valid. To
test if such number from such a structure could reference a GPIO, you
may use this predicate:
int gpio_is_valid(int number);
...
===
Confusingly, I know found that the chapter starts with
===
GPIOs are identified by unsigned integers in the range 0..MAX_INT. That
reserves "negative" numbers for other purposes like marking signals as
"not available on this board", or indicating faults.
===
Meh.
> If you still have a concern about the types used, please clarify and
> let me know what change you expect.
Leave it. I think the fragile part is gpio_is_valid() but this is truly
outside the scope of this patch.
> > > + ret = gpio_request(sda_pin, "sda");
> > > + if (ret) {
> > > + if (ret == -EINVAL)
> > > + ret = -EPROBE_DEFER; /* Try again later */
> >
> > Would gpio_request_array() make the code simpler?
>
> I gave it a try, this indeed makes the code slightly simpler (-4 lines)
> but the resulting binary slightly larger (+40 bytes on x86-64.) I'd say
> it's not worth it?
OK. Then leave it.
> Note that my patch doesn't introduce the gpio_request() calls, they
> were there before, so this decision is actually independent from my
> patch, and even if we decide to switch to using gpio_request_array(),
> I'd rather do it in a separate patch for clarity.
I don't fully get it. Do you want to appl gpio_request() to this patch?
Otherwise, I'd take it as is.
Thanks,
Wolfram
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c-gpio: Add support for deferred probing
[not found] ` <20130322115621.GB24508-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
@ 2013-03-24 10:43 ` Jean Delvare
[not found] ` <20130324114301.4b5efc34-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Jean Delvare @ 2013-03-24 10:43 UTC (permalink / raw)
To: Wolfram Sang
Cc: Linux I2C, Bo Shen, Karol Lewandowski, Jean-Christophe PLAGNIOL-VILLARD
Hi Wolfram,
Thanks for the review.
On Fri, 22 Mar 2013 12:56:21 +0100, Wolfram Sang wrote:
> On Thu, Feb 28, 2013 at 12:01:40PM +0100, Jean Delvare wrote:
> > GPIOs may not be available immediately when i2c-gpio looks for them.
> > Implement support for deferred probing so that probing can be
> > attempted again later when GPIO pins are finally available.
> >
> > Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Cc: Haavard Skinnemoen <hskinnemoen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> > A little more changes were needed than I initially thought, because we
> > want to check the pins before we allocate memory. Otherwise we would
> > have a possibly large number of memory allocation and freeing cycles
> > until GPIO pins are finally available.
> >
> > I wrote this quite some time ago already, but I do not have any system
> > to test it. I would appreciate if one or more users of the i2c-gpio
> > driver could give it a try and confirm it works as intended, or at
> > least doesn't introduce any regression. Thanks.
> >
> > drivers/i2c/busses/i2c-gpio.c | 75 ++++++++++++++++++++++++++++--------------
> > 1 file changed, 50 insertions(+), 25 deletions(-)
> >
> > --- linux-3.8-rc2.orig/drivers/i2c/busses/i2c-gpio.c 2013-01-09 22:26:30.031060312 +0100
> > +++ linux-3.8-rc2/drivers/i2c/busses/i2c-gpio.c 2013-01-09 22:32:59.950308645 +0100
> > @@ -85,23 +85,29 @@ static int i2c_gpio_getscl(void *data)
> > return gpio_get_value(pdata->scl_pin);
> > }
> >
> > -static int of_i2c_gpio_probe(struct device_node *np,
> > - struct i2c_gpio_platform_data *pdata)
> > +static int of_i2c_gpio_get_pins(struct device_node *np,
> > + unsigned int *sda_pin, unsigned int *scl_pin)
>
> GPIOs are expressed via int.
What do you mean? In <linux/gpio.h> I see:
struct gpio {
unsigned gpio;
(...)
static inline int gpio_get_value(unsigned int gpio)
{
return __gpio_get_value(gpio);
}
And in <linux/i2c-gpio.h>:
struct i2c_gpio_platform_data {
unsigned int sda_pin;
unsigned int scl_pin;
(...)
So unsigned int seems to be the right type for gpios. The OF layer
indeed uses int as the return type of of_get_gpio(), presumably to be
able to report errors, but the original code did not handle errors from
these calls so I assumed they couldn't fail and did not bother adding
error handling.
If you still have a concern about the types used, please clarify and
let me know what change you expect.
> > + ret = gpio_request(sda_pin, "sda");
> > + if (ret) {
> > + if (ret == -EINVAL)
> > + ret = -EPROBE_DEFER; /* Try again later */
>
> Would gpio_request_array() make the code simpler?
I gave it a try, this indeed makes the code slightly simpler (-4 lines)
but the resulting binary slightly larger (+40 bytes on x86-64.) I'd say
it's not worth it?
Note that my patch doesn't introduce the gpio_request() calls, they
were there before, so this decision is actually independent from my
patch, and even if we decide to switch to using gpio_request_array(),
I'd rather do it in a separate patch for clarity.
Thanks,
--
Jean Delvare
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c-gpio: Add support for deferred probing
[not found] ` <20130228120140.127ebb91-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2013-03-01 3:58 ` Bo Shen
@ 2013-03-22 11:56 ` Wolfram Sang
[not found] ` <20130322115621.GB24508-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
2013-03-27 8:21 ` Wolfram Sang
2 siblings, 1 reply; 10+ messages in thread
From: Wolfram Sang @ 2013-03-22 11:56 UTC (permalink / raw)
To: Jean Delvare
Cc: Linux I2C, Bo Shen, Karol Lewandowski, Jean-Christophe PLAGNIOL-VILLARD
Hi Jean,
On Thu, Feb 28, 2013 at 12:01:40PM +0100, Jean Delvare wrote:
> GPIOs may not be available immediately when i2c-gpio looks for them.
> Implement support for deferred probing so that probing can be
> attempted again later when GPIO pins are finally available.
>
> Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: Haavard Skinnemoen <hskinnemoen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> A little more changes were needed than I initially thought, because we
> want to check the pins before we allocate memory. Otherwise we would
> have a possibly large number of memory allocation and freeing cycles
> until GPIO pins are finally available.
>
> I wrote this quite some time ago already, but I do not have any system
> to test it. I would appreciate if one or more users of the i2c-gpio
> driver could give it a try and confirm it works as intended, or at
> least doesn't introduce any regression. Thanks.
>
> drivers/i2c/busses/i2c-gpio.c | 75 ++++++++++++++++++++++++++++--------------
> 1 file changed, 50 insertions(+), 25 deletions(-)
>
> --- linux-3.8-rc2.orig/drivers/i2c/busses/i2c-gpio.c 2013-01-09 22:26:30.031060312 +0100
> +++ linux-3.8-rc2/drivers/i2c/busses/i2c-gpio.c 2013-01-09 22:32:59.950308645 +0100
> @@ -85,23 +85,29 @@ static int i2c_gpio_getscl(void *data)
> return gpio_get_value(pdata->scl_pin);
> }
>
> -static int of_i2c_gpio_probe(struct device_node *np,
> - struct i2c_gpio_platform_data *pdata)
> +static int of_i2c_gpio_get_pins(struct device_node *np,
> + unsigned int *sda_pin, unsigned int *scl_pin)
GPIOs are expressed via int.
> + ret = gpio_request(sda_pin, "sda");
> + if (ret) {
> + if (ret == -EINVAL)
> + ret = -EPROBE_DEFER; /* Try again later */
Would gpio_request_array() make the code simpler?
Thanks,
Wolfram
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c-gpio: Add support for deferred probing
[not found] ` <51302765.8030202-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
@ 2013-03-01 7:32 ` Jean Delvare
0 siblings, 0 replies; 10+ messages in thread
From: Jean Delvare @ 2013-03-01 7:32 UTC (permalink / raw)
To: Bo Shen; +Cc: Linux I2C, Karol Lewandowski, Jean-Christophe PLAGNIOL-VILLARD
On Fri, 01 Mar 2013 11:58:29 +0800, Bo Shen wrote:
> Hi Jean,
>
> On 2/28/2013 19:01, Jean Delvare wrote:
> > GPIOs may not be available immediately when i2c-gpio looks for them.
> > Implement support for deferred probing so that probing can be
> > attempted again later when GPIO pins are finally available.
> >
> > Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Cc: Haavard Skinnemoen <hskinnemoen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> > A little more changes were needed than I initially thought, because we
> > want to check the pins before we allocate memory. Otherwise we would
> > have a possibly large number of memory allocation and freeing cycles
> > until GPIO pins are finally available.
> >
> > I wrote this quite some time ago already, but I do not have any system
> > to test it. I would appreciate if one or more users of the i2c-gpio
> > driver could give it a try and confirm it works as intended, or at
> > least doesn't introduce any regression. Thanks.
> >
> > drivers/i2c/busses/i2c-gpio.c | 75 ++++++++++++++++++++++++++++--------------
> > 1 file changed, 50 insertions(+), 25 deletions(-)
>
> Test on at91sam9g20ek_2mmc board, booting with non-dt kernel and
> dt-kernel base on Linux-3.8. Both are OK.
>
> Tested-by: Bo Shen <voice.shen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
Thanks Bo, this is very appreciated!
--
Jean Delvare
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] i2c-gpio: Add support for deferred probing
[not found] ` <20130228120140.127ebb91-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
@ 2013-03-01 3:58 ` Bo Shen
[not found] ` <51302765.8030202-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
2013-03-22 11:56 ` Wolfram Sang
2013-03-27 8:21 ` Wolfram Sang
2 siblings, 1 reply; 10+ messages in thread
From: Bo Shen @ 2013-03-01 3:58 UTC (permalink / raw)
To: Jean Delvare
Cc: Linux I2C, Karol Lewandowski, Jean-Christophe PLAGNIOL-VILLARD
Hi Jean,
On 2/28/2013 19:01, Jean Delvare wrote:
> GPIOs may not be available immediately when i2c-gpio looks for them.
> Implement support for deferred probing so that probing can be
> attempted again later when GPIO pins are finally available.
>
> Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> Cc: Haavard Skinnemoen <hskinnemoen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> A little more changes were needed than I initially thought, because we
> want to check the pins before we allocate memory. Otherwise we would
> have a possibly large number of memory allocation and freeing cycles
> until GPIO pins are finally available.
>
> I wrote this quite some time ago already, but I do not have any system
> to test it. I would appreciate if one or more users of the i2c-gpio
> driver could give it a try and confirm it works as intended, or at
> least doesn't introduce any regression. Thanks.
>
> drivers/i2c/busses/i2c-gpio.c | 75 ++++++++++++++++++++++++++++--------------
> 1 file changed, 50 insertions(+), 25 deletions(-)
Test on at91sam9g20ek_2mmc board, booting with non-dt kernel and
dt-kernel base on Linux-3.8. Both are OK.
Tested-by: Bo Shen <voice.shen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
Best Regards,
Bo Shen
> --- linux-3.8-rc2.orig/drivers/i2c/busses/i2c-gpio.c 2013-01-09 22:26:30.031060312 +0100
> +++ linux-3.8-rc2/drivers/i2c/busses/i2c-gpio.c 2013-01-09 22:32:59.950308645 +0100
> @@ -85,23 +85,29 @@ static int i2c_gpio_getscl(void *data)
> return gpio_get_value(pdata->scl_pin);
> }
>
> -static int of_i2c_gpio_probe(struct device_node *np,
> - struct i2c_gpio_platform_data *pdata)
> +static int of_i2c_gpio_get_pins(struct device_node *np,
> + unsigned int *sda_pin, unsigned int *scl_pin)
> {
> - u32 reg;
> -
> if (of_gpio_count(np) < 2)
> return -ENODEV;
>
> - pdata->sda_pin = of_get_gpio(np, 0);
> - pdata->scl_pin = of_get_gpio(np, 1);
> + *sda_pin = of_get_gpio(np, 0);
> + *scl_pin = of_get_gpio(np, 1);
>
> - if (!gpio_is_valid(pdata->sda_pin) || !gpio_is_valid(pdata->scl_pin)) {
> + if (!gpio_is_valid(*sda_pin) || !gpio_is_valid(*scl_pin)) {
> pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
> - np->full_name, pdata->sda_pin, pdata->scl_pin);
> + np->full_name, *sda_pin, *scl_pin);
> return -ENODEV;
> }
>
> + return 0;
> +}
> +
> +static void of_i2c_gpio_get_props(struct device_node *np,
> + struct i2c_gpio_platform_data *pdata)
> +{
> + u32 reg;
> +
> of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
>
> if (!of_property_read_u32(np, "i2c-gpio,timeout-ms", ®))
> @@ -113,8 +119,6 @@ static int of_i2c_gpio_probe(struct devi
> of_property_read_bool(np, "i2c-gpio,scl-open-drain");
> pdata->scl_is_output_only =
> of_property_read_bool(np, "i2c-gpio,scl-output-only");
> -
> - return 0;
> }
>
> static int i2c_gpio_probe(struct platform_device *pdev)
> @@ -123,31 +127,52 @@ static int i2c_gpio_probe(struct platfor
> struct i2c_gpio_platform_data *pdata;
> struct i2c_algo_bit_data *bit_data;
> struct i2c_adapter *adap;
> + unsigned int sda_pin, scl_pin;
> int ret;
>
> - priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> - if (!priv)
> - return -ENOMEM;
> - adap = &priv->adap;
> - bit_data = &priv->bit_data;
> - pdata = &priv->pdata;
> -
> + /* First get the GPIO pins; if it fails, we'll defer the probe. */
> if (pdev->dev.of_node) {
> - ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
> + ret = of_i2c_gpio_get_pins(pdev->dev.of_node,
> + &sda_pin, &scl_pin);
> if (ret)
> return ret;
> } else {
> if (!pdev->dev.platform_data)
> return -ENXIO;
> - memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
> + pdata = pdev->dev.platform_data;
> + sda_pin = pdata->sda_pin;
> + scl_pin = pdata->scl_pin;
> }
>
> - ret = gpio_request(pdata->sda_pin, "sda");
> - if (ret)
> + ret = gpio_request(sda_pin, "sda");
> + if (ret) {
> + if (ret == -EINVAL)
> + ret = -EPROBE_DEFER; /* Try again later */
> goto err_request_sda;
> - ret = gpio_request(pdata->scl_pin, "scl");
> - if (ret)
> + }
> + ret = gpio_request(scl_pin, "scl");
> + if (ret) {
> + if (ret == -EINVAL)
> + ret = -EPROBE_DEFER; /* Try again later */
> goto err_request_scl;
> + }
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv) {
> + ret = -ENOMEM;
> + goto err_add_bus;
> + }
> + adap = &priv->adap;
> + bit_data = &priv->bit_data;
> + pdata = &priv->pdata;
> +
> + if (pdev->dev.of_node) {
> + pdata->sda_pin = sda_pin;
> + pdata->scl_pin = scl_pin;
> + of_i2c_gpio_get_props(pdev->dev.of_node, pdata);
> + } else {
> + memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
> + }
>
> if (pdata->sda_is_open_drain) {
> gpio_direction_output(pdata->sda_pin, 1);
> @@ -211,9 +236,9 @@ static int i2c_gpio_probe(struct platfor
> return 0;
>
> err_add_bus:
> - gpio_free(pdata->scl_pin);
> + gpio_free(scl_pin);
> err_request_scl:
> - gpio_free(pdata->sda_pin);
> + gpio_free(sda_pin);
> err_request_sda:
> return ret;
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] i2c-gpio: Add support for deferred probing
@ 2013-02-28 11:01 Jean Delvare
[not found] ` <20130228120140.127ebb91-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
0 siblings, 1 reply; 10+ messages in thread
From: Jean Delvare @ 2013-02-28 11:01 UTC (permalink / raw)
To: Linux I2C; +Cc: Bo Shen, Karol Lewandowski, Jean-Christophe PLAGNIOL-VILLARD
GPIOs may not be available immediately when i2c-gpio looks for them.
Implement support for deferred probing so that probing can be
attempted again later when GPIO pins are finally available.
Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Haavard Skinnemoen <hskinnemoen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
A little more changes were needed than I initially thought, because we
want to check the pins before we allocate memory. Otherwise we would
have a possibly large number of memory allocation and freeing cycles
until GPIO pins are finally available.
I wrote this quite some time ago already, but I do not have any system
to test it. I would appreciate if one or more users of the i2c-gpio
driver could give it a try and confirm it works as intended, or at
least doesn't introduce any regression. Thanks.
drivers/i2c/busses/i2c-gpio.c | 75 ++++++++++++++++++++++++++++--------------
1 file changed, 50 insertions(+), 25 deletions(-)
--- linux-3.8-rc2.orig/drivers/i2c/busses/i2c-gpio.c 2013-01-09 22:26:30.031060312 +0100
+++ linux-3.8-rc2/drivers/i2c/busses/i2c-gpio.c 2013-01-09 22:32:59.950308645 +0100
@@ -85,23 +85,29 @@ static int i2c_gpio_getscl(void *data)
return gpio_get_value(pdata->scl_pin);
}
-static int of_i2c_gpio_probe(struct device_node *np,
- struct i2c_gpio_platform_data *pdata)
+static int of_i2c_gpio_get_pins(struct device_node *np,
+ unsigned int *sda_pin, unsigned int *scl_pin)
{
- u32 reg;
-
if (of_gpio_count(np) < 2)
return -ENODEV;
- pdata->sda_pin = of_get_gpio(np, 0);
- pdata->scl_pin = of_get_gpio(np, 1);
+ *sda_pin = of_get_gpio(np, 0);
+ *scl_pin = of_get_gpio(np, 1);
- if (!gpio_is_valid(pdata->sda_pin) || !gpio_is_valid(pdata->scl_pin)) {
+ if (!gpio_is_valid(*sda_pin) || !gpio_is_valid(*scl_pin)) {
pr_err("%s: invalid GPIO pins, sda=%d/scl=%d\n",
- np->full_name, pdata->sda_pin, pdata->scl_pin);
+ np->full_name, *sda_pin, *scl_pin);
return -ENODEV;
}
+ return 0;
+}
+
+static void of_i2c_gpio_get_props(struct device_node *np,
+ struct i2c_gpio_platform_data *pdata)
+{
+ u32 reg;
+
of_property_read_u32(np, "i2c-gpio,delay-us", &pdata->udelay);
if (!of_property_read_u32(np, "i2c-gpio,timeout-ms", ®))
@@ -113,8 +119,6 @@ static int of_i2c_gpio_probe(struct devi
of_property_read_bool(np, "i2c-gpio,scl-open-drain");
pdata->scl_is_output_only =
of_property_read_bool(np, "i2c-gpio,scl-output-only");
-
- return 0;
}
static int i2c_gpio_probe(struct platform_device *pdev)
@@ -123,31 +127,52 @@ static int i2c_gpio_probe(struct platfor
struct i2c_gpio_platform_data *pdata;
struct i2c_algo_bit_data *bit_data;
struct i2c_adapter *adap;
+ unsigned int sda_pin, scl_pin;
int ret;
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
- adap = &priv->adap;
- bit_data = &priv->bit_data;
- pdata = &priv->pdata;
-
+ /* First get the GPIO pins; if it fails, we'll defer the probe. */
if (pdev->dev.of_node) {
- ret = of_i2c_gpio_probe(pdev->dev.of_node, pdata);
+ ret = of_i2c_gpio_get_pins(pdev->dev.of_node,
+ &sda_pin, &scl_pin);
if (ret)
return ret;
} else {
if (!pdev->dev.platform_data)
return -ENXIO;
- memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+ pdata = pdev->dev.platform_data;
+ sda_pin = pdata->sda_pin;
+ scl_pin = pdata->scl_pin;
}
- ret = gpio_request(pdata->sda_pin, "sda");
- if (ret)
+ ret = gpio_request(sda_pin, "sda");
+ if (ret) {
+ if (ret == -EINVAL)
+ ret = -EPROBE_DEFER; /* Try again later */
goto err_request_sda;
- ret = gpio_request(pdata->scl_pin, "scl");
- if (ret)
+ }
+ ret = gpio_request(scl_pin, "scl");
+ if (ret) {
+ if (ret == -EINVAL)
+ ret = -EPROBE_DEFER; /* Try again later */
goto err_request_scl;
+ }
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ ret = -ENOMEM;
+ goto err_add_bus;
+ }
+ adap = &priv->adap;
+ bit_data = &priv->bit_data;
+ pdata = &priv->pdata;
+
+ if (pdev->dev.of_node) {
+ pdata->sda_pin = sda_pin;
+ pdata->scl_pin = scl_pin;
+ of_i2c_gpio_get_props(pdev->dev.of_node, pdata);
+ } else {
+ memcpy(pdata, pdev->dev.platform_data, sizeof(*pdata));
+ }
if (pdata->sda_is_open_drain) {
gpio_direction_output(pdata->sda_pin, 1);
@@ -211,9 +236,9 @@ static int i2c_gpio_probe(struct platfor
return 0;
err_add_bus:
- gpio_free(pdata->scl_pin);
+ gpio_free(scl_pin);
err_request_scl:
- gpio_free(pdata->sda_pin);
+ gpio_free(sda_pin);
err_request_sda:
return ret;
}
--
Jean Delvare
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-03-27 8:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-19 11:19 [PATCH] i2c-gpio: Add support for deferred probing Jean Delvare
[not found] ` <20120719131924.7672f99e-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2012-08-31 12:38 ` Jean Delvare
2013-02-28 11:01 Jean Delvare
[not found] ` <20130228120140.127ebb91-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2013-03-01 3:58 ` Bo Shen
[not found] ` <51302765.8030202-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
2013-03-01 7:32 ` Jean Delvare
2013-03-22 11:56 ` Wolfram Sang
[not found] ` <20130322115621.GB24508-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
2013-03-24 10:43 ` Jean Delvare
[not found] ` <20130324114301.4b5efc34-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2013-03-26 11:09 ` Wolfram Sang
[not found] ` <20130326110908.GC8553-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
2013-03-26 12:22 ` Jean Delvare
2013-03-27 8:21 ` Wolfram Sang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.