* [PATCH v2 0/2] Add ps_it attributes for vcnl4040 @ 2022-09-23 11:40 Mårten Lindahl 2022-09-23 11:40 ` [PATCH v2 1/2] iio: light: vcnl4000: Preserve conf bits when toggle power Mårten Lindahl 2022-09-23 11:40 ` [PATCH v2 2/2] iio: light: vcnl4000: Add ps_it attributes for vcnl4040 Mårten Lindahl 0 siblings, 2 replies; 7+ messages in thread From: Mårten Lindahl @ 2022-09-23 11:40 UTC (permalink / raw) To: Jonathan Cameron, Lars-Peter Clausen Cc: Paul Cercueil, Uwe Kleine-König, linux-iio, kernel, Mårten Lindahl Currently there is no way for userspace to make any configuration of the VCNL4040 sensors, but only the sensor readings are exported in sysfs. To support configuration for proximity integration time value, sysfs attributes for this needs to be exported. To begin with the runtime power management turns both sensors (ALS, and PS) on before reading the sensor register values and then switches them off again. But when doing so it writes the whole register instead of just switching the power on/off bit. This needs to be fixed in order to make other persistent configurations. Kind regards Mårten Lindahl Mårten Lindahl (2): iio: light: vcnl4000: Preserve conf bits when toggle power iio: light: vcnl4000: Add ps_it attributes for vcnl4040 drivers/iio/light/vcnl4000.c | 183 +++++++++++++++++++++++++++++++++-- 1 file changed, 177 insertions(+), 6 deletions(-) -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] iio: light: vcnl4000: Preserve conf bits when toggle power 2022-09-23 11:40 [PATCH v2 0/2] Add ps_it attributes for vcnl4040 Mårten Lindahl @ 2022-09-23 11:40 ` Mårten Lindahl 2022-09-24 16:37 ` Jonathan Cameron 2022-09-23 11:40 ` [PATCH v2 2/2] iio: light: vcnl4000: Add ps_it attributes for vcnl4040 Mårten Lindahl 1 sibling, 1 reply; 7+ messages in thread From: Mårten Lindahl @ 2022-09-23 11:40 UTC (permalink / raw) To: Jonathan Cameron, Lars-Peter Clausen Cc: Paul Cercueil, Uwe Kleine-König, linux-iio, kernel, Mårten Lindahl As the vcnl4040 and vcnl4200 chip uses runtime power management for turning the ambient light and proximity sensors on/off, it overwrites the entire register each time. In ALS_CONF register bit fields ALS_IT, ALS_PERS, ALS_INT_EN are overwritten. In PS_CONF1 register bit fields PS_DUTY, PS_PERS, PS_IT, PS_HD, and PS_INT are overwritten. Add functions for preserving the affected bit fields when changing power state. Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> --- drivers/iio/light/vcnl4000.c | 54 ++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c index 3db4e26731bb..b2ecf8af1aa5 100644 --- a/drivers/iio/light/vcnl4000.c +++ b/drivers/iio/light/vcnl4000.c @@ -74,6 +74,9 @@ #define VCNL4000_PROX_EN BIT(1) /* start proximity measurement */ #define VCNL4000_SELF_TIMED_EN BIT(0) /* start self-timed measurement */ +#define VCNL4040_ALS_CONF_ALS_SD BIT(0) /* Enable ambient light sensor */ +#define VCNL4040_PS_CONF1_PS_SD BIT(0) /* Enable proximity sensor */ + /* Bit masks for interrupt registers. */ #define VCNL4010_INT_THR_SEL BIT(0) /* Select threshold interrupt source */ #define VCNL4010_INT_THR_EN BIT(1) /* Threshold interrupt type */ @@ -188,16 +191,61 @@ static int vcnl4000_init(struct vcnl4000_data *data) return data->chip_spec->set_power_state(data, true); }; +static ssize_t vcnl4000_write_als_enable(struct vcnl4000_data *data, bool en) +{ + int ret; + + mutex_lock(&data->vcnl4000_lock); + + ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF); + if (ret < 0) + goto out; + + if (en) + ret &= ~VCNL4040_ALS_CONF_ALS_SD; + else + ret |= VCNL4040_ALS_CONF_ALS_SD; + + ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, ret); + +out: + mutex_unlock(&data->vcnl4000_lock); + + return ret; +} + +static ssize_t vcnl4000_write_ps_enable(struct vcnl4000_data *data, bool en) +{ + int ret; + + mutex_lock(&data->vcnl4000_lock); + + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1); + if (ret < 0) + goto out; + + if (en) + ret &= ~VCNL4040_PS_CONF1_PS_SD; + else + ret |= VCNL4040_PS_CONF1_PS_SD; + + ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, ret); + +out: + mutex_unlock(&data->vcnl4000_lock); + + return ret; +} + static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on) { - u16 val = on ? 0 /* power on */ : 1 /* shut down */; int ret; - ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, val); + ret = vcnl4000_write_als_enable(data, on); if (ret < 0) return ret; - ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, val); + ret = vcnl4000_write_ps_enable(data, on); if (ret < 0) return ret; -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] iio: light: vcnl4000: Preserve conf bits when toggle power 2022-09-23 11:40 ` [PATCH v2 1/2] iio: light: vcnl4000: Preserve conf bits when toggle power Mårten Lindahl @ 2022-09-24 16:37 ` Jonathan Cameron 2022-09-26 8:22 ` Marten Lindahl 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2022-09-24 16:37 UTC (permalink / raw) To: Mårten Lindahl Cc: Lars-Peter Clausen, Paul Cercueil, Uwe Kleine-König, linux-iio, kernel On Fri, 23 Sep 2022 13:40:30 +0200 Mårten Lindahl <marten.lindahl@axis.com> wrote: > As the vcnl4040 and vcnl4200 chip uses runtime power management for > turning the ambient light and proximity sensors on/off, it overwrites > the entire register each time. In ALS_CONF register bit fields ALS_IT, > ALS_PERS, ALS_INT_EN are overwritten. In PS_CONF1 register bit fields > PS_DUTY, PS_PERS, PS_IT, PS_HD, and PS_INT are overwritten. > > Add functions for preserving the affected bit fields when changing power > state. > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> See inline. Otherwise looks good to me. Jonathan > --- > drivers/iio/light/vcnl4000.c | 54 ++++++++++++++++++++++++++++++++++-- > 1 file changed, 51 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c > index 3db4e26731bb..b2ecf8af1aa5 100644 > --- a/drivers/iio/light/vcnl4000.c > +++ b/drivers/iio/light/vcnl4000.c > @@ -74,6 +74,9 @@ > #define VCNL4000_PROX_EN BIT(1) /* start proximity measurement */ > #define VCNL4000_SELF_TIMED_EN BIT(0) /* start self-timed measurement */ > > +#define VCNL4040_ALS_CONF_ALS_SD BIT(0) /* Enable ambient light sensor */ Comment seems inverted. Bit being set is 'shut down' I would expand the name to VCNL4040_ALS_CONF_ALS_SHUTDOWN then drop the comment as the name is self explanatory > +#define VCNL4040_PS_CONF1_PS_SD BIT(0) /* Enable proximity sensor */ > + > /* Bit masks for interrupt registers. */ > #define VCNL4010_INT_THR_SEL BIT(0) /* Select threshold interrupt source */ > #define VCNL4010_INT_THR_EN BIT(1) /* Threshold interrupt type */ > @@ -188,16 +191,61 @@ static int vcnl4000_init(struct vcnl4000_data *data) > return data->chip_spec->set_power_state(data, true); > }; > > +static ssize_t vcnl4000_write_als_enable(struct vcnl4000_data *data, bool en) > +{ > + int ret; > + > + mutex_lock(&data->vcnl4000_lock); > + > + ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF); > + if (ret < 0) > + goto out; > + > + if (en) > + ret &= ~VCNL4040_ALS_CONF_ALS_SD; > + else > + ret |= VCNL4040_ALS_CONF_ALS_SD; > + > + ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, ret); > + > +out: > + mutex_unlock(&data->vcnl4000_lock); > + > + return ret; > +} > + > +static ssize_t vcnl4000_write_ps_enable(struct vcnl4000_data *data, bool en) > +{ > + int ret; > + > + mutex_lock(&data->vcnl4000_lock); > + > + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1); > + if (ret < 0) > + goto out; > + > + if (en) > + ret &= ~VCNL4040_PS_CONF1_PS_SD; > + else > + ret |= VCNL4040_PS_CONF1_PS_SD; > + > + ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, ret); > + > +out: > + mutex_unlock(&data->vcnl4000_lock); > + > + return ret; > +} > + > static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on) > { > - u16 val = on ? 0 /* power on */ : 1 /* shut down */; > int ret; > > - ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, val); > + ret = vcnl4000_write_als_enable(data, on); > if (ret < 0) > return ret; > > - ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, val); > + ret = vcnl4000_write_ps_enable(data, on); > if (ret < 0) > return ret; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] iio: light: vcnl4000: Preserve conf bits when toggle power 2022-09-24 16:37 ` Jonathan Cameron @ 2022-09-26 8:22 ` Marten Lindahl 0 siblings, 0 replies; 7+ messages in thread From: Marten Lindahl @ 2022-09-26 8:22 UTC (permalink / raw) To: Jonathan Cameron Cc: Mårten Lindahl, Lars-Peter Clausen, Paul Cercueil, Uwe Kleine-König, linux-iio, kernel On Sat, Sep 24, 2022 at 06:37:24PM +0200, Jonathan Cameron wrote: > On Fri, 23 Sep 2022 13:40:30 +0200 > Mårten Lindahl <marten.lindahl@axis.com> wrote: > > > As the vcnl4040 and vcnl4200 chip uses runtime power management for > > turning the ambient light and proximity sensors on/off, it overwrites > > the entire register each time. In ALS_CONF register bit fields ALS_IT, > > ALS_PERS, ALS_INT_EN are overwritten. In PS_CONF1 register bit fields > > PS_DUTY, PS_PERS, PS_IT, PS_HD, and PS_INT are overwritten. > > > > Add functions for preserving the affected bit fields when changing power > > state. > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > See inline. > Otherwise looks good to me. > > Jonathan > > > --- > > drivers/iio/light/vcnl4000.c | 54 ++++++++++++++++++++++++++++++++++-- > > 1 file changed, 51 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c > > index 3db4e26731bb..b2ecf8af1aa5 100644 > > --- a/drivers/iio/light/vcnl4000.c > > +++ b/drivers/iio/light/vcnl4000.c > > @@ -74,6 +74,9 @@ > > #define VCNL4000_PROX_EN BIT(1) /* start proximity measurement */ > > #define VCNL4000_SELF_TIMED_EN BIT(0) /* start self-timed measurement */ > > > > +#define VCNL4040_ALS_CONF_ALS_SD BIT(0) /* Enable ambient light sensor */ > Hi Jonathan! > Comment seems inverted. Bit being set is 'shut down' I would expand the > name to > VCNL4040_ALS_CONF_ALS_SHUTDOWN > then drop the comment as the name is self explanatory Ok, I'll do so. Thanks! Kind regards Mårten > > > +#define VCNL4040_PS_CONF1_PS_SD BIT(0) /* Enable proximity sensor */ > > + > > /* Bit masks for interrupt registers. */ > > #define VCNL4010_INT_THR_SEL BIT(0) /* Select threshold interrupt source */ > > #define VCNL4010_INT_THR_EN BIT(1) /* Threshold interrupt type */ > > @@ -188,16 +191,61 @@ static int vcnl4000_init(struct vcnl4000_data *data) > > return data->chip_spec->set_power_state(data, true); > > }; > > > > +static ssize_t vcnl4000_write_als_enable(struct vcnl4000_data *data, bool en) > > +{ > > + int ret; > > + > > + mutex_lock(&data->vcnl4000_lock); > > + > > + ret = i2c_smbus_read_word_data(data->client, VCNL4200_AL_CONF); > > + if (ret < 0) > > + goto out; > > + > > + if (en) > > + ret &= ~VCNL4040_ALS_CONF_ALS_SD; > > + else > > + ret |= VCNL4040_ALS_CONF_ALS_SD; > > + > > + ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, ret); > > + > > +out: > > + mutex_unlock(&data->vcnl4000_lock); > > + > > + return ret; > > +} > > + > > +static ssize_t vcnl4000_write_ps_enable(struct vcnl4000_data *data, bool en) > > +{ > > + int ret; > > + > > + mutex_lock(&data->vcnl4000_lock); > > + > > + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1); > > + if (ret < 0) > > + goto out; > > + > > + if (en) > > + ret &= ~VCNL4040_PS_CONF1_PS_SD; > > + else > > + ret |= VCNL4040_PS_CONF1_PS_SD; > > + > > + ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, ret); > > + > > +out: > > + mutex_unlock(&data->vcnl4000_lock); > > + > > + return ret; > > +} > > + > > static int vcnl4200_set_power_state(struct vcnl4000_data *data, bool on) > > { > > - u16 val = on ? 0 /* power on */ : 1 /* shut down */; > > int ret; > > > > - ret = i2c_smbus_write_word_data(data->client, VCNL4200_AL_CONF, val); > > + ret = vcnl4000_write_als_enable(data, on); > > if (ret < 0) > > return ret; > > > > - ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, val); > > + ret = vcnl4000_write_ps_enable(data, on); > > if (ret < 0) > > return ret; > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] iio: light: vcnl4000: Add ps_it attributes for vcnl4040 2022-09-23 11:40 [PATCH v2 0/2] Add ps_it attributes for vcnl4040 Mårten Lindahl 2022-09-23 11:40 ` [PATCH v2 1/2] iio: light: vcnl4000: Preserve conf bits when toggle power Mårten Lindahl @ 2022-09-23 11:40 ` Mårten Lindahl 2022-09-24 16:40 ` Jonathan Cameron 1 sibling, 1 reply; 7+ messages in thread From: Mårten Lindahl @ 2022-09-23 11:40 UTC (permalink / raw) To: Jonathan Cameron, Lars-Peter Clausen Cc: Paul Cercueil, Uwe Kleine-König, linux-iio, kernel, Mårten Lindahl Add read/write attribute for proximity integration time, and read attribute for available proximity integration times for the vcnl4040 chip. Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> --- drivers/iio/light/vcnl4000.c | 129 ++++++++++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 3 deletions(-) diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c index b2ecf8af1aa5..056079b592c6 100644 --- a/drivers/iio/light/vcnl4000.c +++ b/drivers/iio/light/vcnl4000.c @@ -17,6 +17,7 @@ * interrupts (VCNL4040, VCNL4200) */ +#include <linux/bitfield.h> #include <linux/module.h> #include <linux/i2c.h> #include <linux/err.h> @@ -76,6 +77,7 @@ #define VCNL4040_ALS_CONF_ALS_SD BIT(0) /* Enable ambient light sensor */ #define VCNL4040_PS_CONF1_PS_SD BIT(0) /* Enable proximity sensor */ +#define VCNL4040_PS_CONF2_PS_IT GENMASK(3, 1) /* Proximity integration time */ /* Bit masks for interrupt registers. */ #define VCNL4010_INT_THR_SEL BIT(0) /* Select threshold interrupt source */ @@ -104,6 +106,17 @@ static const int vcnl4010_prox_sampling_frequency[][2] = { {250, 0}, }; +static const int vcnl4040_ps_it_times[][2] = { + {0, 100}, + {0, 150}, + {0, 200}, + {0, 250}, + {0, 300}, + {0, 350}, + {0, 400}, + {0, 800}, +}; + #define VCNL4000_SLEEP_DELAY_MS 2000 /* before we enter pm_runtime_suspend */ enum vcnl4000_device_ids { @@ -470,6 +483,55 @@ static int vcnl4000_set_pm_runtime_state(struct vcnl4000_data *data, bool on) return ret; } +static int vcnl4040_read_ps_it(struct vcnl4000_data *data, int *val, int *val2) +{ + int ret; + + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1); + if (ret < 0) + return ret; + + ret = FIELD_GET(VCNL4040_PS_CONF2_PS_IT, ret); + + if (ret >= ARRAY_SIZE(vcnl4040_ps_it_times)) + return -EINVAL; + + *val = vcnl4040_ps_it_times[ret][0]; + *val2 = vcnl4040_ps_it_times[ret][1]; + + return 0; +} + +static ssize_t vcnl4040_write_ps_it(struct vcnl4000_data *data, int val) +{ + unsigned int i; + int ret, index = -1; + + for (i = 0; i < ARRAY_SIZE(vcnl4040_ps_it_times); i++) { + if (val == vcnl4040_ps_it_times[i][1]) { + index = i; + break; + } + } + + if (index < 0) + return -EINVAL; + + mutex_lock(&data->vcnl4000_lock); + + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1); + if (ret < 0) + goto out; + + ret = (ret & ~VCNL4040_PS_CONF2_PS_IT) | + FIELD_PREP(VCNL4040_PS_CONF2_PS_IT, index); + ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, ret); + +out: + mutex_unlock(&data->vcnl4000_lock); + return ret; +} + static int vcnl4000_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long mask) @@ -506,6 +568,47 @@ static int vcnl4000_read_raw(struct iio_dev *indio_dev, *val = 0; *val2 = data->al_scale; return IIO_VAL_INT_PLUS_MICRO; + case IIO_CHAN_INFO_INT_TIME: + if (chan->type != IIO_PROXIMITY) + return -EINVAL; + ret = vcnl4040_read_ps_it(data, val, val2); + if (ret < 0) + return ret; + return IIO_VAL_INT_PLUS_MICRO; + default: + return -EINVAL; + } +} + +static int vcnl4040_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + int val, int val2, long mask) +{ + struct vcnl4000_data *data = iio_priv(indio_dev); + + switch (mask) { + case IIO_CHAN_INFO_INT_TIME: + if (val != 0) + return -EINVAL; + if (chan->type != IIO_PROXIMITY) + return -EINVAL; + return vcnl4040_write_ps_it(data, val2); + default: + return -EINVAL; + } +} + +static int vcnl4040_read_avail(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + const int **vals, int *type, int *length, + long mask) +{ + switch (mask) { + case IIO_CHAN_INFO_INT_TIME: + *vals = (int *)vcnl4040_ps_it_times; + *type = IIO_VAL_INT_PLUS_MICRO; + *length = 2 * ARRAY_SIZE(vcnl4040_ps_it_times); + return IIO_AVAIL_LIST; default: return -EINVAL; } @@ -844,6 +947,20 @@ static const struct iio_chan_spec vcnl4010_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(1), }; +static const struct iio_chan_spec vcnl4040_channels[] = { + { + .type = IIO_LIGHT, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE), + }, { + .type = IIO_PROXIMITY, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_INT_TIME), + .info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME), + .ext_info = vcnl4000_ext_info, + } +}; + static const struct iio_info vcnl4000_info = { .read_raw = vcnl4000_read_raw, }; @@ -858,6 +975,12 @@ static const struct iio_info vcnl4010_info = { .write_event_config = vcnl4010_write_event_config, }; +static const struct iio_info vcnl4040_info = { + .read_raw = vcnl4000_read_raw, + .write_raw = vcnl4040_write_raw, + .read_avail = vcnl4040_read_avail, +}; + static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = { [VCNL4000] = { .prod = "VCNL4000", @@ -887,9 +1010,9 @@ static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = { .measure_light = vcnl4200_measure_light, .measure_proximity = vcnl4200_measure_proximity, .set_power_state = vcnl4200_set_power_state, - .channels = vcnl4000_channels, - .num_channels = ARRAY_SIZE(vcnl4000_channels), - .info = &vcnl4000_info, + .channels = vcnl4040_channels, + .num_channels = ARRAY_SIZE(vcnl4040_channels), + .info = &vcnl4040_info, .irq_support = false, }, [VCNL4200] = { -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] iio: light: vcnl4000: Add ps_it attributes for vcnl4040 2022-09-23 11:40 ` [PATCH v2 2/2] iio: light: vcnl4000: Add ps_it attributes for vcnl4040 Mårten Lindahl @ 2022-09-24 16:40 ` Jonathan Cameron 2022-09-26 8:25 ` Marten Lindahl 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2022-09-24 16:40 UTC (permalink / raw) To: Mårten Lindahl Cc: Lars-Peter Clausen, Paul Cercueil, Uwe Kleine-König, linux-iio, kernel On Fri, 23 Sep 2022 13:40:31 +0200 Mårten Lindahl <marten.lindahl@axis.com> wrote: > Add read/write attribute for proximity integration time, and read > attribute for available proximity integration times for the vcnl4040 > chip. > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> Hi Mårten, One minor comment inline given I've asked for changes that mean you'll probably be doing a v3 anyway. Thanks, Jonathan > --- > drivers/iio/light/vcnl4000.c | 129 ++++++++++++++++++++++++++++++++++- > 1 file changed, 126 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c > index b2ecf8af1aa5..056079b592c6 100644 > --- a/drivers/iio/light/vcnl4000.c > +++ b/drivers/iio/light/vcnl4000.c > @@ -17,6 +17,7 @@ > * interrupts (VCNL4040, VCNL4200) > */ > > +#include <linux/bitfield.h> > #include <linux/module.h> > #include <linux/i2c.h> > #include <linux/err.h> > @@ -76,6 +77,7 @@ > > #define VCNL4040_ALS_CONF_ALS_SD BIT(0) /* Enable ambient light sensor */ > #define VCNL4040_PS_CONF1_PS_SD BIT(0) /* Enable proximity sensor */ > +#define VCNL4040_PS_CONF2_PS_IT GENMASK(3, 1) /* Proximity integration time */ > > /* Bit masks for interrupt registers. */ > #define VCNL4010_INT_THR_SEL BIT(0) /* Select threshold interrupt source */ > @@ -104,6 +106,17 @@ static const int vcnl4010_prox_sampling_frequency[][2] = { > {250, 0}, > }; > > +static const int vcnl4040_ps_it_times[][2] = { > + {0, 100}, > + {0, 150}, > + {0, 200}, > + {0, 250}, > + {0, 300}, > + {0, 350}, > + {0, 400}, > + {0, 800}, > +}; > + > #define VCNL4000_SLEEP_DELAY_MS 2000 /* before we enter pm_runtime_suspend */ > > enum vcnl4000_device_ids { > @@ -470,6 +483,55 @@ static int vcnl4000_set_pm_runtime_state(struct vcnl4000_data *data, bool on) > return ret; > } > > +static int vcnl4040_read_ps_it(struct vcnl4000_data *data, int *val, int *val2) > +{ > + int ret; > + > + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1); > + if (ret < 0) > + return ret; > + > + ret = FIELD_GET(VCNL4040_PS_CONF2_PS_IT, ret); > + > + if (ret >= ARRAY_SIZE(vcnl4040_ps_it_times)) > + return -EINVAL; > + > + *val = vcnl4040_ps_it_times[ret][0]; > + *val2 = vcnl4040_ps_it_times[ret][1]; > + > + return 0; > +} > + > +static ssize_t vcnl4040_write_ps_it(struct vcnl4000_data *data, int val) > +{ > + unsigned int i; > + int ret, index = -1; > + > + for (i = 0; i < ARRAY_SIZE(vcnl4040_ps_it_times); i++) { > + if (val == vcnl4040_ps_it_times[i][1]) { > + index = i; > + break; > + } > + } > + > + if (index < 0) > + return -EINVAL; > + > + mutex_lock(&data->vcnl4000_lock); > + > + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1); > + if (ret < 0) > + goto out; > + > + ret = (ret & ~VCNL4040_PS_CONF2_PS_IT) | > + FIELD_PREP(VCNL4040_PS_CONF2_PS_IT, index); It can be confusing to read ret both as a temporary to build value ad for the return code. I would introduce a u16 val and build the value in that. > + ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, ret); > + > +out: > + mutex_unlock(&data->vcnl4000_lock); > + return ret; > +} > + ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] iio: light: vcnl4000: Add ps_it attributes for vcnl4040 2022-09-24 16:40 ` Jonathan Cameron @ 2022-09-26 8:25 ` Marten Lindahl 0 siblings, 0 replies; 7+ messages in thread From: Marten Lindahl @ 2022-09-26 8:25 UTC (permalink / raw) To: Jonathan Cameron Cc: Mårten Lindahl, Lars-Peter Clausen, Paul Cercueil, Uwe Kleine-König, linux-iio, kernel On Sat, Sep 24, 2022 at 06:40:44PM +0200, Jonathan Cameron wrote: > On Fri, 23 Sep 2022 13:40:31 +0200 > Mårten Lindahl <marten.lindahl@axis.com> wrote: > > > Add read/write attribute for proximity integration time, and read > > attribute for available proximity integration times for the vcnl4040 > > chip. > > > > Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com> > Hi Mårten, > > One minor comment inline given I've asked for changes that mean you'll > probably be doing a v3 anyway. > > Thanks, > > Jonathan > > > --- > > drivers/iio/light/vcnl4000.c | 129 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 126 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c > > index b2ecf8af1aa5..056079b592c6 100644 > > --- a/drivers/iio/light/vcnl4000.c > > +++ b/drivers/iio/light/vcnl4000.c > > @@ -17,6 +17,7 @@ > > * interrupts (VCNL4040, VCNL4200) > > */ > > > > +#include <linux/bitfield.h> > > #include <linux/module.h> > > #include <linux/i2c.h> > > #include <linux/err.h> > > @@ -76,6 +77,7 @@ > > > > #define VCNL4040_ALS_CONF_ALS_SD BIT(0) /* Enable ambient light sensor */ > > #define VCNL4040_PS_CONF1_PS_SD BIT(0) /* Enable proximity sensor */ > > +#define VCNL4040_PS_CONF2_PS_IT GENMASK(3, 1) /* Proximity integration time */ > > > > /* Bit masks for interrupt registers. */ > > #define VCNL4010_INT_THR_SEL BIT(0) /* Select threshold interrupt source */ > > @@ -104,6 +106,17 @@ static const int vcnl4010_prox_sampling_frequency[][2] = { > > {250, 0}, > > }; > > > > +static const int vcnl4040_ps_it_times[][2] = { > > + {0, 100}, > > + {0, 150}, > > + {0, 200}, > > + {0, 250}, > > + {0, 300}, > > + {0, 350}, > > + {0, 400}, > > + {0, 800}, > > +}; > > + > > #define VCNL4000_SLEEP_DELAY_MS 2000 /* before we enter pm_runtime_suspend */ > > > > enum vcnl4000_device_ids { > > @@ -470,6 +483,55 @@ static int vcnl4000_set_pm_runtime_state(struct vcnl4000_data *data, bool on) > > return ret; > > } > > > > +static int vcnl4040_read_ps_it(struct vcnl4000_data *data, int *val, int *val2) > > +{ > > + int ret; > > + > > + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1); > > + if (ret < 0) > > + return ret; > > + > > + ret = FIELD_GET(VCNL4040_PS_CONF2_PS_IT, ret); > > + > > + if (ret >= ARRAY_SIZE(vcnl4040_ps_it_times)) > > + return -EINVAL; > > + > > + *val = vcnl4040_ps_it_times[ret][0]; > > + *val2 = vcnl4040_ps_it_times[ret][1]; > > + > > + return 0; > > +} > > + > > +static ssize_t vcnl4040_write_ps_it(struct vcnl4000_data *data, int val) > > +{ > > + unsigned int i; > > + int ret, index = -1; > > + > > + for (i = 0; i < ARRAY_SIZE(vcnl4040_ps_it_times); i++) { > > + if (val == vcnl4040_ps_it_times[i][1]) { > > + index = i; > > + break; > > + } > > + } > > + > > + if (index < 0) > > + return -EINVAL; > > + > > + mutex_lock(&data->vcnl4000_lock); > > + > > + ret = i2c_smbus_read_word_data(data->client, VCNL4200_PS_CONF1); > > + if (ret < 0) > > + goto out; > > + > > + ret = (ret & ~VCNL4040_PS_CONF2_PS_IT) | > > + FIELD_PREP(VCNL4040_PS_CONF2_PS_IT, index); > > It can be confusing to read ret both as a temporary to build value ad for the > return code. I would introduce a > u16 val > and build the value in that. Hi Jonathan! Thanks. I'll do so. As there already is a 'val' I will call it 'regval'. Kind regards Mårten > > > + ret = i2c_smbus_write_word_data(data->client, VCNL4200_PS_CONF1, ret); > > + > > +out: > > + mutex_unlock(&data->vcnl4000_lock); > > + return ret; > > +} > > + ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-09-26 8:25 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-23 11:40 [PATCH v2 0/2] Add ps_it attributes for vcnl4040 Mårten Lindahl 2022-09-23 11:40 ` [PATCH v2 1/2] iio: light: vcnl4000: Preserve conf bits when toggle power Mårten Lindahl 2022-09-24 16:37 ` Jonathan Cameron 2022-09-26 8:22 ` Marten Lindahl 2022-09-23 11:40 ` [PATCH v2 2/2] iio: light: vcnl4000: Add ps_it attributes for vcnl4040 Mårten Lindahl 2022-09-24 16:40 ` Jonathan Cameron 2022-09-26 8:25 ` Marten Lindahl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).