| Message ID | 20241031-iio-fix-write-event-config-signature-v2-13-2bcacbb517a2@baylibre.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iio: fix write_event_config signature | expand |
On Thu, 31 Oct 2024 16:27:08 +0100 Julien Stephan <jstephan@baylibre.com> wrote: > Simplifies the regmap_wite if branch in apds9306_write_event_config. Applied. > > Signed-off-by: Julien Stephan <jstephan@baylibre.com> > --- > drivers/iio/light/apds9306.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/iio/light/apds9306.c b/drivers/iio/light/apds9306.c > index 8adc74040db2bddf93fbb773e3519abcc726b9a6..9c08e7c3ad0c17519689a630d42fe9b90438dfe8 100644 > --- a/drivers/iio/light/apds9306.c > +++ b/drivers/iio/light/apds9306.c > @@ -1125,10 +1125,7 @@ static int apds9306_write_event_config(struct iio_dev *indio_dev, > } > } > case IIO_EV_TYPE_THRESH_ADAPTIVE: > - if (state) > - return regmap_field_write(rf->int_thresh_var_en, 1); > - else > - return regmap_field_write(rf->int_thresh_var_en, 0); > + return regmap_field_write(rf->int_thresh_var_en, state); > default: > return -EINVAL; > } >
On 2/11/24 03:11, Jonathan Cameron wrote: > On Thu, 31 Oct 2024 16:27:08 +0100 > Julien Stephan <jstephan@baylibre.com> wrote: > >> Simplifies the regmap_wite if branch in apds9306_write_event_config. > Applied. >> >> Signed-off-by: Julien Stephan <jstephan@baylibre.com> >> --- >> drivers/iio/light/apds9306.c | 5 +---- >> 1 file changed, 1 insertion(+), 4 deletions(-) >> >> diff --git a/drivers/iio/light/apds9306.c b/drivers/iio/light/apds9306.c >> index 8adc74040db2bddf93fbb773e3519abcc726b9a6..9c08e7c3ad0c17519689a630d42fe9b90438dfe8 100644 >> --- a/drivers/iio/light/apds9306.c >> +++ b/drivers/iio/light/apds9306.c >> @@ -1125,10 +1125,7 @@ static int apds9306_write_event_config(struct iio_dev *indio_dev, >> } >> } >> case IIO_EV_TYPE_THRESH_ADAPTIVE: >> - if (state) >> - return regmap_field_write(rf->int_thresh_var_en, 1); >> - else >> - return regmap_field_write(rf->int_thresh_var_en, 0); >> + return regmap_field_write(rf->int_thresh_var_en, state); >> default: >> return -EINVAL; >> } >> > Hi Julien, Sorry for the delay. "int_thresh_var_en" corresponds to APDS9306_INT_CFG_REG bit 3 (Datasheet: INT_CFG, Address: 0x19) which is a single bit value only. If user does "echo 4 > /sys/bus/iio/devices/iio:device0/events/thresh_adaptive_either_en", which trickles down to the "state" variable, do we really want to write any other value except "0" or "1"? Correct me if I am wrong here. Regards, Subhajit Ghosh
Le sam. 2 nov. 2024 à 14:21, Subhajit Ghosh <subhajit.ghosh@tweaklogic.com> a écrit : > > On 2/11/24 03:11, Jonathan Cameron wrote: > > On Thu, 31 Oct 2024 16:27:08 +0100 > > Julien Stephan <jstephan@baylibre.com> wrote: > > > >> Simplifies the regmap_wite if branch in apds9306_write_event_config. > > Applied. > >> > >> Signed-off-by: Julien Stephan <jstephan@baylibre.com> > >> --- > >> drivers/iio/light/apds9306.c | 5 +---- > >> 1 file changed, 1 insertion(+), 4 deletions(-) > >> > >> diff --git a/drivers/iio/light/apds9306.c b/drivers/iio/light/apds9306.c > >> index 8adc74040db2bddf93fbb773e3519abcc726b9a6..9c08e7c3ad0c17519689a630d42fe9b90438dfe8 100644 > >> --- a/drivers/iio/light/apds9306.c > >> +++ b/drivers/iio/light/apds9306.c > >> @@ -1125,10 +1125,7 @@ static int apds9306_write_event_config(struct iio_dev *indio_dev, > >> } > >> } > >> case IIO_EV_TYPE_THRESH_ADAPTIVE: > >> - if (state) > >> - return regmap_field_write(rf->int_thresh_var_en, 1); > >> - else > >> - return regmap_field_write(rf->int_thresh_var_en, 0); > >> + return regmap_field_write(rf->int_thresh_var_en, state); > >> default: > >> return -EINVAL; > >> } > >> > > > Hi Julien, > Sorry for the delay. > "int_thresh_var_en" corresponds to APDS9306_INT_CFG_REG bit 3 (Datasheet: INT_CFG, Address: 0x19) which > is a single bit value only. If user does "echo 4 > /sys/bus/iio/devices/iio:device0/events/thresh_adaptive_either_en", > which trickles down to the "state" variable, do we really want to write any other value except "0" or "1"? > Correct me if I am wrong here. Hi Subhajit, In drivers/iio/industrialio-event.c, iio_ev_state_store is actually using kstrtobool to check user input, then gives the converted boolean value to write_event_config. So state is a boolean. By the way the purpose of this series is to clean up code and use a bool instead of int for the state parameter. So new code is equivalent to what was there before. Cheers Julien > Regards, > Subhajit Ghosh
On 3/11/24 00:59, Julien Stephan wrote: > Le sam. 2 nov. 2024 à 14:21, Subhajit Ghosh > <subhajit.ghosh@tweaklogic.com> a écrit : >> >> On 2/11/24 03:11, Jonathan Cameron wrote: >>> On Thu, 31 Oct 2024 16:27:08 +0100 >>> Julien Stephan <jstephan@baylibre.com> wrote: >>> >>>> Simplifies the regmap_wite if branch in apds9306_write_event_config. >>> Applied. >>>> >>>> Signed-off-by: Julien Stephan <jstephan@baylibre.com> >>>> --- >>>> drivers/iio/light/apds9306.c | 5 +---- >>>> 1 file changed, 1 insertion(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/iio/light/apds9306.c b/drivers/iio/light/apds9306.c >>>> index 8adc74040db2bddf93fbb773e3519abcc726b9a6..9c08e7c3ad0c17519689a630d42fe9b90438dfe8 100644 >>>> --- a/drivers/iio/light/apds9306.c >>>> +++ b/drivers/iio/light/apds9306.c >>>> @@ -1125,10 +1125,7 @@ static int apds9306_write_event_config(struct iio_dev *indio_dev, >>>> } >>>> } >>>> case IIO_EV_TYPE_THRESH_ADAPTIVE: >>>> - if (state) >>>> - return regmap_field_write(rf->int_thresh_var_en, 1); >>>> - else >>>> - return regmap_field_write(rf->int_thresh_var_en, 0); >>>> + return regmap_field_write(rf->int_thresh_var_en, state); >>>> default: >>>> return -EINVAL; >>>> } >>>> >>> >> Hi Julien, >> Sorry for the delay. >> "int_thresh_var_en" corresponds to APDS9306_INT_CFG_REG bit 3 (Datasheet: INT_CFG, Address: 0x19) which >> is a single bit value only. If user does "echo 4 > /sys/bus/iio/devices/iio:device0/events/thresh_adaptive_either_en", >> which trickles down to the "state" variable, do we really want to write any other value except "0" or "1"? >> Correct me if I am wrong here. > > Hi Subhajit, > In drivers/iio/industrialio-event.c, iio_ev_state_store is actually > using kstrtobool to check user input, then gives the converted boolean > value to write_event_configOhh ok, that's handy. > So state is a boolean. By the way the purpose of this series is to > clean up code and use a bool instead of int for the state parameter. Thank you Julien for your efforts. > > So new code is equivalent to what was there before. > Cheers > Julien > Since it is already applied, nothing much for me to do here. Regards, Subhajit Ghosh
diff --git a/drivers/iio/light/apds9306.c b/drivers/iio/light/apds9306.c index 8adc74040db2bddf93fbb773e3519abcc726b9a6..9c08e7c3ad0c17519689a630d42fe9b90438dfe8 100644 --- a/drivers/iio/light/apds9306.c +++ b/drivers/iio/light/apds9306.c @@ -1125,10 +1125,7 @@ static int apds9306_write_event_config(struct iio_dev *indio_dev, } } case IIO_EV_TYPE_THRESH_ADAPTIVE: - if (state) - return regmap_field_write(rf->int_thresh_var_en, 1); - else - return regmap_field_write(rf->int_thresh_var_en, 0); + return regmap_field_write(rf->int_thresh_var_en, state); default: return -EINVAL; }
Simplifies the regmap_wite if branch in apds9306_write_event_config. Signed-off-by: Julien Stephan <jstephan@baylibre.com> --- drivers/iio/light/apds9306.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)