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

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

[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