[PATCH 1/2] i2c-mux-gpio: test if the gpio can sleep

[PATCH 1/2] i2c-mux-gpio: test if the gpio can sleep

[Ionut Nicu]

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 <ioan.nicu.ext-OYasijW0DpE@public.gmane.org>
---
 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

Re: [PATCH 1/2] i2c-mux-gpio: test if the gpio can sleep

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]

On Thu, Oct 10, 2013 at 10:39:32AM +0200, 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 <ioan.nicu.ext-OYasijW0DpE@public.gmane.org>
> ---
>  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));
> +	}

There should be a wrapper for that in the gpio-subsystem IMO.
Adding linux-gpio to cc.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] i2c-mux-gpio: test if the gpio can sleep

[Lars-Peter Clausen]

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 <ioan.nicu.ext@nsn.com>
> ---
>  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.

> +	}
>  }
>  
>  static int i2c_mux_gpio_select(struct i2c_adapter *adap, void *data, u32 chan)
> 

Re: [PATCH 1/2] i2c-mux-gpio: test if the gpio can sleep

[Ionut Nicu]

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 <ioan.nicu.ext-OYasijW0DpE@public.gmane.org>
>> ---
>>  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!

Re: [PATCH 1/2] i2c-mux-gpio: test if the gpio can sleep

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

On Thu, Oct 10, 2013 at 10:46:41AM +0200, Lars-Peter Clausen wrote:

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

To the gpio-list: Has it been considered to have sth. like
gpio_set_value and gpio_set_value_nosleep? I'd think it makes more sense
to have the specific function have the specific name.

Regards,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] i2c-mux-gpio: test if the gpio can sleep

[Lars-Peter Clausen]

On 10/10/2013 09:43 PM, Wolfram Sang wrote:
> On Thu, Oct 10, 2013 at 10:46:41AM +0200, Lars-Peter Clausen wrote:
>
>>> +		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.
>
> To the gpio-list: Has it been considered to have sth. like
> gpio_set_value and gpio_set_value_nosleep? I'd think it makes more sense
> to have the specific function have the specific name.

It has been a few times, but I think the conclusion has always been that it is 
now too late to invert the semantics of gpio_set_value(). If you want to look 
up the discussions the keyword is gpio_set_value_atomic().

- Lars