[PATCH] i2c-mux-gpio: use deferred probing with the device tree
If the i2c-parent bus driver is not loaded, returning
-EINVAL will force people to unload and then reload the
module again to get it working.
Also of_get_named_gpio could return -E_PROBE_DEFER or
another error code. This error should be passed further
instead of being ignored.
Signed-off-by: Ionut Nicu
---
drivers/i2c/muxes/i2c-mux-gpio.c | 12
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 5d4a99b..6370b47 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -66,7 +66,7 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
struct device_node *adapter_np, *child;
struct i2c_adapter *adapter;
unsigned *values, *gpios;
- int i = 0;
+ int i = 0, ret;
if (!np)
return -ENODEV;
@@ -79,7 +79,7 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
adapter = of_find_i2c_adapter_by_node(adapter_np);
if (!adapter) {
dev_err(&pdev->dev, "Cannot find parent bus\n");
- return -ENODEV;
+ return -EPROBE_DEFER;
}
mux->data.parent = i2c_adapter_id(adapter);
put_device(&adapter->dev);
@@ -116,8 +116,12 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
return -ENOMEM;
}
- for (i = 0; i < mux->data.n_gpios; i++)
- gpios[i] = of_get_named_gpio(np, "mux-gpios", i);
+ for (i = 0; i < mux->data.n_gpios; i++) {
+ ret = of_get_named_gpio(np, "mux-gpios", i);
+ if (ret < 0)
+ return ret;
+ gpios[i] = ret;
+ }
mux->data.gpios = gpios;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] i2c-mux-gpio: use deferred probing with the device tree
On 08.10.2013 15:30, ext Peter Korsgaard wrote: >>>>>> "IN" == Ionut Nicu writes: > > IN> If the i2c-parent bus driver is not loaded, returning > IN> -EINVAL will force people to unload and then reload the > IN> module again to get it working. > > IN> Also of_get_named_gpio could return -E_PROBE_DEFER or > IN> another error code. This error should be passed further > IN> instead of being ignored. > > Two different fixes, so should be 2 separate patches. Other that that, > it looks good. > > Acked-by: Peter Korsgaard > Right, I will split it into two patches and re-submit. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] i2c-mux-gpio: use deferred probing with the device tree
If the i2c-parent bus driver is not loaded, returning
-EINVAL will force people to unload and then reload the
module again to get it working.
Signed-off-by: Ionut Nicu
---
drivers/i2c/muxes/i2c-mux-gpio.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 5d4a99b..eb99f04 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -79,7 +79,7 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
adapter = of_find_i2c_adapter_by_node(adapter_np);
if (!adapter) {
dev_err(&pdev->dev, "Cannot find parent bus\n");
- return -ENODEV;
+ return -EPROBE_DEFER;
}
mux->data.parent = i2c_adapter_id(adapter);
put_device(&adapter->dev);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] i2c-mux-gpio: don't ignore of_get_named_gpio errors
of_get_named_gpio could return -E_PROBE_DEFER or another
error code. This error should be passed further instead
of being ignored.
Signed-off-by: Ionut Nicu
---
drivers/i2c/muxes/i2c-mux-gpio.c | 10 +++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index eb99f04..6370b47 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -66,7 +66,7 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
struct device_node *adapter_np, *child;
struct i2c_adapter *adapter;
unsigned *values, *gpios;
- int i = 0;
+ int i = 0, ret;
if (!np)
return -ENODEV;
@@ -116,8 +116,12 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
return -ENOMEM;
}
- for (i = 0; i < mux->data.n_gpios; i++)
- gpios[i] = of_get_named_gpio(np, "mux-gpios", i);
+ for (i = 0; i < mux->data.n_gpios; i++) {
+ ret = of_get_named_gpio(np, "mux-gpios", i);
+ if (ret < 0)
+ return ret;
+ gpios[i] = ret;
+ }
mux->data.gpios = gpios;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c-mux-gpio: use deferred probing with the device tree
On 09.10.2013 08:14, ext Peter Korsgaard wrote: >>>>>> "WS" == Wolfram Sang writes: > > WS> On Tue, Oct 08, 2013 at 03:51:50PM +0200, Ionut Nicu wrote: >>> If the i2c-parent bus driver is not loaded, returning >>> -EINVAL will force people to unload and then reload the >>> module again to get it working. >>> >>> Signed-off-by: Ionut Nicu > > WS> Doesn't the non-DT case need fixing, too? > > Arguably yes. > Yes, and it's also a one line fix. Should I do it in a separate patch or should I change this one and resubmit? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] i2c-mux-gpio: use deferred probing
If the i2c-parent bus driver is not loaded, returning
-ENODEV will force people to unload and then reload the
module again to get it working.
Signed-off-by: Ionut Nicu
---
drivers/i2c/muxes/i2c-mux-gpio.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 5d4a99b..ff184eb 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -79,7 +79,7 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
adapter = of_find_i2c_adapter_by_node(adapter_np);
if (!adapter) {
dev_err(&pdev->dev, "Cannot find parent bus\n");
- return -ENODEV;
+ return -EPROBE_DEFER;
}
mux->data.parent = i2c_adapter_id(adapter);
put_device(&adapter->dev);
@@ -177,7 +177,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
if (!parent) {
dev_err(&pdev->dev, "Parent adapter (%d) not found\n",
mux->data.parent);
- return -ENODEV;
+ return -EPROBE_DEFER;
}
mux->parent = parent;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] i2c-mux-gpio: test if the gpio can sleep
Some gpio chips may have get/set operations that
can sleep. For this type of chips we must use the
_cansleep() version of gpio_set_value.
Signed-off-by: Ionut Nicu
---
drivers/i2c/muxes/i2c-mux-gpio.c | 11 ---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index a764da7..b5f17ef 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -27,11 +27,16 @@ struct gpiomux {
static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
{
+ unsigned gpio;
int i;
- for (i = 0; i < mux->data.n_gpios; i++)
- gpio_set_value(mux->gpio_base + mux->data.gpios[i],
- val & (1 << i));
+ for (i = 0; i < mux->data.n_gpios; i++) {
+ gpio = mux->gpio_base + mux->data.gpios[i];
+ if (gpio_cansleep(gpio))
+ gpio_set_value_cansleep(gpio, val & (1 << i));
+ else
+ gpio_set_value(gpio, val & (1 << i));
+ }
}
static int i2c_mux_gpio_select(struct i2c_adapter *adap, void *data, u32 chan)
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] i2c-mux-gpio: eliminate i2c channel order assumptions
The i2c-mux driver uses the chan_id parameter provided
in i2c_add_mux_adapter as a parameter to the select
and deselect callbacks while the i2c-mux-gpio driver
uses the chan_id as an index in the mux->data.values
array.
A simple example of where this doesn't work is when we
have a device tree like this:
i2cmux {
i2c@1 {
reg = <1>;
...
};
i2c@0 {
reg = <0>;
...
};
};
The mux->data.values array will be { 1, 0 }, but when
the i2-mux driver will try to select channel 0, the
i2c-mux-gpio driver will actually use values[0], hence 1
as the gpio selection value.
Signed-off-by: Ionut Nicu
---
drivers/i2c/muxes/i2c-mux-gpio.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index b5f17ef..3505d0e 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -43,7 +43,7 @@ static int i2c_mux_gpio_select(struct i2c_adapter *adap, void
*data, u32 chan)
{
struct gpiomux *mux = data;
- i2c_mux_gpio_set(mux, mux->data.values[chan]);
+ i2c_mux_gpio_set(mux, chan);
return 0;
}
@@ -233,7 +233,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
unsigned int class = mux->data.classes ? mux->data.classes[i] :
0;
mux->adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr,
- i, class,
+ mux->data.values[i], class,
i2c_mux_gpio_select,
deselect);
if (!mux->adap[i]) {
ret = -ENODEV;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] i2c-mux-gpio: test if the gpio can sleep
On 10.10.2013 10:46, ext Lars-Peter Clausen wrote:
> On 10/10/2013 10:39 AM, Ionut Nicu wrote:
>> Some gpio chips may have get/set operations that
>> can sleep. For this type of chips we must use the
>> _cansleep() version of gpio_set_value.
>>
>> Signed-off-by: Ionut Nicu
>> ---
>> drivers/i2c/muxes/i2c-mux-gpio.c | 11 ---
>> 1 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c
>> b/drivers/i2c/muxes/i2c-mux-gpio.c
>> index a764da7..b5f17ef 100644
>> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
>> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
>> @@ -27,11 +27,16 @@ struct gpiomux {
>>
>> static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
>> {
>> +unsigned gpio;
>> int i;
>>
>> -for (i = 0; i < mux->data.n_gpios; i++)
>> -gpio_set_value(mux->gpio_base + mux->data.gpios[i],
>> - val & (1 << i));
>> +for (i = 0; i < mux->data.n_gpios; i++) {
>> +gpio = mux->gpio_base + mux->data.gpios[i];
>> +if (gpio_cansleep(gpio))
>> +gpio_set_value_cansleep(gpio, val & (1 << i));
>> +else
>> +gpio_set_value(gpio, val & (1 << i));
>
> The proper way to do this is just always use the _cansleep() version.
> gpio_set_value() only works for chips which do not sleep,
> gpio_set_value_cansleep() works for both those who do sleep and those who do
> not.
>
Ok, then I will re-work the patch to do just that. Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] i2c-mux-gpio: eliminate i2c channel order assumptions
Hi,
On 10.10.2013 12:34, Alexander Sverdlin wrote:
> Hi!
>
> On 10/10/2013 10:39 AM, Ionut Nicu wrote:
>> The i2c-mux driver uses the chan_id parameter provided
>> in i2c_add_mux_adapter as a parameter to the select
>> and deselect callbacks while the i2c-mux-gpio driver
>> uses the chan_id as an index in the mux->data.values
>> array.
>>
>> A simple example of where this doesn't work is when we
>> have a device tree like this:
>>
>> i2cmux {
>> i2c@1 {
>> reg = <1>;
>> ...
>> };
>>
>> i2c@0 {
>> reg = <0>;
>> ...
>> };
>> };
>>
>> The mux->data.values array will be { 1, 0 }, but when
>> the i2-mux driver will try to select channel 0, the
>> i2c-mux-gpio driver will actually use values[0], hence 1
>> as the gpio selection value.
>
> The patch itself is correct, but the description is not precise,
> I suppose... i2c-mux-gpio is consistent inside itself, it will
> receive for every child adapter the value it has configured.
> The problem happens inside i2c-mux.c, i2c_add_mux_adapter():
>
> for_each_child_of_node(mux_dev->of_node, child) {
> ret = of_property_read_u32(child, "reg", ®);
> if (ret)
> continue;
> if (chan_id == reg) {
> priv->adap.dev.of_node = child;
>
> Which means, i2c-mux-gpio MUST pass reg, not its logical index inside
> array. Otherwise node will not be correctly assigned and i2c-mux will
> have problems selecting right adapter for the multiplexed devices.
>
>> Signed-off-by: Ionut Nicu
>
> So, for the code itself
>
> Acked-by: Alexander Sverdlin
>
You are right, the patch description is not so good. I will try to change
it so it's clearer for everyone what I'm trying to fix here and after that
I will re-submit the series.
>> ---
>> drivers/i2c/muxes/i2c-mux-gpio.c |4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c
>> b/drivers/i2c/muxes/i2c-mux-gpio.c
>> index b5f17ef..3505d0e 100644
>> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
>> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
>> @@ -43,7 +43,7 @@ static int i2c_mux_gpio_select(struct i2c_adapter *adap,
>> void *data, u32 chan)
>> {
>> struct gpiomux *mux = data;
>>
>> -i2c_mux_gpio_set(mux, mux->data.values[chan]);
>> +i2c_mux_gpio_set(mux, chan);
>>
>> return 0;
>> }
>> @@ -233,7 +233,7 @@ static int i2c_mux_gpio_probe(struct platform_device
>> *pdev)
>> unsigned int class = mux->data.classes ? mux->data.classes[i] :
>> 0;
>>
>> mux->adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr,
>> - i, class,
>> + mux->data.values[i], class,
>> i2c_mux_gpio_select,
>> deselect);
>> if (!mux->adap[i]) {
>> ret = -ENODEV;
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] i2c-mux-gpio: use gpio_set_value_cansleep()
Some gpio chips may have get/set operations that can sleep. gpio_set_value() only works for chips which do not sleep, for the others we will get a kernel warning. Using gpio_set_value_cansleep() will work for both chips that do sleep and those who don't. Signed-off-by: Ionut Nicu --- drivers/i2c/muxes/i2c-mux-gpio.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c index a764da7..550e094 100644 --- a/drivers/i2c/muxes/i2c-mux-gpio.c +++ b/drivers/i2c/muxes/i2c-mux-gpio.c @@ -30,8 +30,8 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val) int i; for (i = 0; i < mux->data.n_gpios; i++) - gpio_set_value(mux->gpio_base + mux->data.gpios[i], - val & (1 << i)); + gpio_set_value_cansleep(mux->gpio_base + mux->data.gpios[i], + val & (1 << i)); } static int i2c_mux_gpio_select(struct i2c_adapter *adap, void *data, u32 chan) -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] i2c-mux-gpio: use reg value for i2c_add_mux_adapter
The i2c-mux driver requires that the chan_id parameter
passed to the i2c_add_mux_adapter() function is equal
to the reg value for that adapter:
for_each_child_of_node(mux_dev->of_node, child) {
ret = of_property_read_u32(child, "reg", ®);
if (ret)
continue;
if (chan_id == reg) {
priv->adap.dev.of_node = child;
break;
}
}
The i2c-mux-gpio driver uses an internal logical index
for chan_id when calling i2c_add_mux_adapter() instead
of using the reg value.
Because of this, there will problems in selecting the
right adapter when the i2c-mux-gpio's index into
mux->data.values doesn't match the reg value.
An example of such a case:
mux->data.values = { 1, 0 }
For chan_id = 0, i2c-mux will bind the adapter to the
of_node with reg = <0>, but when it will call the
select() callback with chan_id set to 0, the i2c-mux-gpio
will use it as an index into mux->data.values and it will
actually select the bus with reg = <1>.
Signed-off-by: Ionut Nicu
---
drivers/i2c/muxes/i2c-mux-gpio.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 550e094..ed3e849 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -38,7 +38,7 @@ static int i2c_mux_gpio_select(struct i2c_adapter *adap, void
*data, u32 chan)
{
struct gpiomux *mux = data;
- i2c_mux_gpio_set(mux, mux->data.values[chan]);
+ i2c_mux_gpio_set(mux, chan);
return 0;
}
@@ -228,7 +228,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
unsigned int class = mux->data.classes ? mux->data.classes[i] :
0;
mux->adap[i] = i2c_add_mux_adapter(parent, &pdev->dev, mux, nr,
- i, class,
+ mux->data.values[i], class,
i2c_mux_gpio_select,
deselect);
if (!mux->adap[i]) {
ret = -ENODEV;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] i2c-mux-gpio: use gpio_set_value_cansleep()
Some gpio chips may have get/set operations that can sleep. gpio_set_value() only works for chips which do not sleep, for the others we will get a kernel warning. Using gpio_set_value_cansleep() will work for both chips that do sleep and those who don't. Signed-off-by: Ionut Nicu --- drivers/i2c/muxes/i2c-mux-gpio.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c index a764da7..4ad9e71 100644 --- a/drivers/i2c/muxes/i2c-mux-gpio.c +++ b/drivers/i2c/muxes/i2c-mux-gpio.c @@ -30,8 +30,8 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val) int i; for (i = 0; i < mux->data.n_gpios; i++) - gpio_set_value(mux->gpio_base + mux->data.gpios[i], - val & (1 << i)); + gpio_set_value_cansleep(mux->gpio_base + mux->data.gpios[i], + val & (1 << i)); } static int i2c_mux_gpio_select(struct i2c_adapter *adap, void *data, u32 chan) -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
