| Message ID | 525667C4.3080309@nsn.com |
|---|---|
| State | Superseded |
| Headers | show |
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@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)); > + } There should be a wrapper for that in the gpio-subsystem IMO. Adding linux-gpio to cc.
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) > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@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. > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)
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(-)