| Message ID | 20210430152419.261757-2-i.mikhaylov@yadro.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | add periodic mode, threshold options and hwmon | expand |
On Fri, Apr 30, 2021 at 6:17 PM Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote: > > Add the possibility to run proximity sensor in periodic measurement > mode. ... > +static int vcnl3020_config_threshold(struct iio_dev *indio_dev, bool state) > +{ > + struct vcnl3020_data *data = iio_priv(indio_dev); > + int rc; > + int icr; > + int cmd; > + int isr; > + > + if (state) { > + rc = iio_device_claim_direct_mode(indio_dev); > + if (rc) > + return rc; > + > + /* Enable periodic measurement of proximity data. */ > + cmd = VCNL_PS_EN | VCNL_PS_SELFTIMED_EN; > + > + /* > + * Enable interrupts on threshold, for proximity data by > + * default. > + */ > + icr = VCNL_ICR_THRES_EN; > + } else { > + if (!vcnl3020_is_thr_enabled(data)) > + return 0; > + > + cmd = 0; > + icr = 0; > + isr = 0; > + } > + > + rc = regmap_write(data->regmap, VCNL_COMMAND, cmd); > + if (rc) > + goto end; > + > + rc = regmap_write(data->regmap, VCNL_PS_ICR, icr); > + if (rc) > + goto end; > + > + if (!state) > + /* Clear interrupts */ > + rc = regmap_write(data->regmap, VCNL_ISR, isr); > + > +end: > + if (state) > + iio_device_release_direct_mode(indio_dev); > + > + return rc; > +} The code will benefit in case you split above to two helpers, i.e. _on() and _off(). It will gain better readability.
On Fri, 30 Apr 2021 18:24:16 +0300 Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote: > Add the possibility to run proximity sensor in periodic measurement > mode. Without an interrupt? Unusual and perhaps best left to userspace. > > Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com> > --- > drivers/iio/proximity/vcnl3020.c | 138 +++++++++++++++++++++++++++++++ > 1 file changed, 138 insertions(+) > > diff --git a/drivers/iio/proximity/vcnl3020.c b/drivers/iio/proximity/vcnl3020.c > index 43817f6b3086..25c6bdba3ede 100644 > --- a/drivers/iio/proximity/vcnl3020.c > +++ b/drivers/iio/proximity/vcnl3020.c > @@ -36,6 +36,21 @@ > #define VCNL_PS_OD BIT(3) /* start on-demand proximity > * measurement > */ > +#define VCNL_PS_EN BIT(1) /* Enables periodic proximity > + * measurement > + */ > +#define VCNL_PS_SELFTIMED_EN BIT(0) /* Enables state machine and LP > + * oscillator for self timed > + * measurements This rather suggests that you should just put comments on their own lines as it will involve less wrapping and ultimately be more compact and readable! > + */ > + > +/* Bit masks for ICR */ > +#define VCNL_ICR_THRES_EN BIT(1) /* Enable interrupts on low or high > + * thresholds */ > + > +/* Bit masks for ISR */ > +#define VCNL_INT_TH_HI BIT(0) /* High threshold hit */ > +#define VCNL_INT_TH_LOW BIT(1) /* Low threshold hit */ > > #define VCNL_ON_DEMAND_TIMEOUT_US 100000 > #define VCNL_POLL_US 20000 > @@ -215,12 +230,124 @@ static int vcnl3020_write_proxy_samp_freq(struct vcnl3020_data *data, int val, > return regmap_write(data->regmap, VCNL_PROXIMITY_RATE, index); > } > > +static bool vcnl3020_is_in_periodic_mode(struct vcnl3020_data *data) > +{ > + int rc; > + unsigned int cmd; > + > + rc = regmap_read(data->regmap, VCNL_COMMAND, &cmd); > + if (rc) > + return false; > + > + return !!(cmd & VCNL_PS_SELFTIMED_EN); > +} > + > +static bool vcnl3020_is_thr_enabled(struct vcnl3020_data *data) > +{ > + int rc; > + unsigned int icr; > + > + rc = regmap_read(data->regmap, VCNL_PS_ICR, &icr); > + if (rc) > + return false; > + > + return !!(icr & VCNL_ICR_THRES_EN); > +} > + > +static int vcnl3020_config_threshold(struct iio_dev *indio_dev, bool state) > +{ > + struct vcnl3020_data *data = iio_priv(indio_dev); > + int rc; > + int icr; > + int cmd; > + int isr; > + > + if (state) { > + rc = iio_device_claim_direct_mode(indio_dev); Device doesn't support buffered mode, so this is a lock as in patch 3. Don't do that as that isn't the intended use > + if (rc) > + return rc; > + > + /* Enable periodic measurement of proximity data. */ > + cmd = VCNL_PS_EN | VCNL_PS_SELFTIMED_EN; > + > + /* > + * Enable interrupts on threshold, for proximity data by > + * default. > + */ > + icr = VCNL_ICR_THRES_EN; > + } else { > + if (!vcnl3020_is_thr_enabled(data)) > + return 0; > + > + cmd = 0; > + icr = 0; > + isr = 0; > + } > + > + rc = regmap_write(data->regmap, VCNL_COMMAND, cmd); > + if (rc) > + goto end; > + > + rc = regmap_write(data->regmap, VCNL_PS_ICR, icr); > + if (rc) > + goto end; > + > + if (!state) > + /* Clear interrupts */ Given you don't seem to have an interrupt. I guess this is clearing a status flag? > + rc = regmap_write(data->regmap, VCNL_ISR, isr); > + > +end: > + if (state) > + iio_device_release_direct_mode(indio_dev); I would just allow for the small amount of repeated code and do only one condition on if (state) in this function. > + > + return rc; > +} > + > +static int vcnl3020_write_event_config(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir, > + int state) > +{ > + switch (chan->type) { > + case IIO_PROXIMITY: > + return vcnl3020_config_threshold(indio_dev, state); > + default: > + return -EINVAL; > + } > +} > + > +static int vcnl3020_read_event_config(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, > + enum iio_event_type type, > + enum iio_event_direction dir) > +{ > + struct vcnl3020_data *data = iio_priv(indio_dev); > + > + switch (chan->type) { > + case IIO_PROXIMITY: > + return vcnl3020_is_thr_enabled(data); > + default: > + return -EINVAL; > + } > +} > + > +static const struct iio_event_spec vcnl3020_event_spec[] = { > + { > + .type = IIO_EV_TYPE_THRESH, > + .dir = IIO_EV_DIR_EITHER, > + .mask_separate = BIT(IIO_EV_INFO_ENABLE), > + }, > +}; > + > static const struct iio_chan_spec vcnl3020_channels[] = { > { > .type = IIO_PROXIMITY, > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > BIT(IIO_CHAN_INFO_SAMP_FREQ), > .info_mask_separate_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), > + .event_spec = vcnl3020_event_spec, > + .num_event_specs = ARRAY_SIZE(vcnl3020_event_spec), > }, > }; > > @@ -233,6 +360,11 @@ static int vcnl3020_read_raw(struct iio_dev *indio_dev, > > switch (mask) { > case IIO_CHAN_INFO_RAW: > + > + /* Protect against event capture. */ > + if (vcnl3020_is_in_periodic_mode(data)) > + return -EBUSY; > + > rc = vcnl3020_measure_proximity(data, val); > if (rc) > return rc; > @@ -254,6 +386,10 @@ static int vcnl3020_write_raw(struct iio_dev *indio_dev, > int rc; > struct vcnl3020_data *data = iio_priv(indio_dev); > > + /* Protect against event capture. */ > + if (vcnl3020_is_in_periodic_mode(data)) > + return -EBUSY; > + > switch (mask) { > case IIO_CHAN_INFO_SAMP_FREQ: > rc = iio_device_claim_direct_mode(indio_dev); > @@ -287,6 +423,8 @@ static const struct iio_info vcnl3020_info = { > .read_raw = vcnl3020_read_raw, > .write_raw = vcnl3020_write_raw, > .read_avail = vcnl3020_read_avail, > + .read_event_config = vcnl3020_read_event_config, > + .write_event_config = vcnl3020_write_event_config, > }; > > static const struct regmap_config vcnl3020_regmap_config = {
On Sun, 2021-05-02 at 19:00 +0100, Jonathan Cameron wrote: > On Fri, 30 Apr 2021 18:24:16 +0300 > Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote: > > > Add the possibility to run proximity sensor in periodic measurement > > mode. > > Without an interrupt? Unusual and perhaps best left to userspace. Do you mean without interrupt handler in driver for this particular interrupt? If it's need to be added here, I can add it. In this patch I just added trigger to enable/disable periodic measurement mode without interrupt handler. > > > + if (rc) > > + return rc; > > + > > + /* Enable periodic measurement of proximity data. */ > > + cmd = VCNL_PS_EN | VCNL_PS_SELFTIMED_EN; > > + > > + /* > > + * Enable interrupts on threshold, for proximity data by > > + * default. > > + */ > > + icr = VCNL_ICR_THRES_EN; > > + } else { > > + if (!vcnl3020_is_thr_enabled(data)) > > + return 0; > > + > > + cmd = 0; > > + icr = 0; > > + isr = 0; > > + } > > + > > + rc = regmap_write(data->regmap, VCNL_COMMAND, cmd); > > + if (rc) > > + goto end; > > + > > + rc = regmap_write(data->regmap, VCNL_PS_ICR, icr); > > + if (rc) > > + goto end; > > + > > + if (!state) > > + /* Clear interrupts */ > > Given you don't seem to have an interrupt. I guess this is clearing > a status flag? Yes, it is clearing flag in interrupt status register. > > > + rc = regmap_write(data->regmap, VCNL_ISR, isr); > > + > > +end: > > + if (state) > > + iio_device_release_direct_mode(indio_dev); >
On Tue, 4 May 2021 22:07:53 +0300 Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote: > On Sun, 2021-05-02 at 19:00 +0100, Jonathan Cameron wrote: > > On Fri, 30 Apr 2021 18:24:16 +0300 > > Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote: > > > > > Add the possibility to run proximity sensor in periodic measurement > > > mode. > > > > Without an interrupt? Unusual and perhaps best left to userspace. > > Do you mean without interrupt handler in driver for this particular interrupt? > If it's need to be added here, I can add it. In this patch I just added trigger > to enable/disable periodic measurement mode without interrupt handler. The model for events in IIO is that they are 'pushed' to userspace. So it is possible to do this with a polling thread, but on a device with an interrupt line tied to the event, it makes much more sense to do it that way. We don't have read on demand event as there has been little real demand for them (and they can be implemented in userspase). Jonathan > > > > > > + if (rc) > > > + return rc; > > > + > > > + /* Enable periodic measurement of proximity data. */ > > > + cmd = VCNL_PS_EN | VCNL_PS_SELFTIMED_EN; > > > + > > > + /* > > > + * Enable interrupts on threshold, for proximity data by > > > + * default. > > > + */ > > > + icr = VCNL_ICR_THRES_EN; > > > + } else { > > > + if (!vcnl3020_is_thr_enabled(data)) > > > + return 0; > > > + > > > + cmd = 0; > > > + icr = 0; > > > + isr = 0; > > > + } > > > + > > > + rc = regmap_write(data->regmap, VCNL_COMMAND, cmd); > > > + if (rc) > > > + goto end; > > > + > > > + rc = regmap_write(data->regmap, VCNL_PS_ICR, icr); > > > + if (rc) > > > + goto end; > > > + > > > + if (!state) > > > + /* Clear interrupts */ > > > > Given you don't seem to have an interrupt. I guess this is clearing > > a status flag? > > Yes, it is clearing flag in interrupt status register. > > > > > > + rc = regmap_write(data->regmap, VCNL_ISR, isr); > > > + > > > +end: > > > + if (state) > > > + iio_device_release_direct_mode(indio_dev); > > > >
diff --git a/drivers/iio/proximity/vcnl3020.c b/drivers/iio/proximity/vcnl3020.c index 43817f6b3086..25c6bdba3ede 100644 --- a/drivers/iio/proximity/vcnl3020.c +++ b/drivers/iio/proximity/vcnl3020.c @@ -36,6 +36,21 @@ #define VCNL_PS_OD BIT(3) /* start on-demand proximity * measurement */ +#define VCNL_PS_EN BIT(1) /* Enables periodic proximity + * measurement + */ +#define VCNL_PS_SELFTIMED_EN BIT(0) /* Enables state machine and LP + * oscillator for self timed + * measurements + */ + +/* Bit masks for ICR */ +#define VCNL_ICR_THRES_EN BIT(1) /* Enable interrupts on low or high + * thresholds */ + +/* Bit masks for ISR */ +#define VCNL_INT_TH_HI BIT(0) /* High threshold hit */ +#define VCNL_INT_TH_LOW BIT(1) /* Low threshold hit */ #define VCNL_ON_DEMAND_TIMEOUT_US 100000 #define VCNL_POLL_US 20000 @@ -215,12 +230,124 @@ static int vcnl3020_write_proxy_samp_freq(struct vcnl3020_data *data, int val, return regmap_write(data->regmap, VCNL_PROXIMITY_RATE, index); } +static bool vcnl3020_is_in_periodic_mode(struct vcnl3020_data *data) +{ + int rc; + unsigned int cmd; + + rc = regmap_read(data->regmap, VCNL_COMMAND, &cmd); + if (rc) + return false; + + return !!(cmd & VCNL_PS_SELFTIMED_EN); +} + +static bool vcnl3020_is_thr_enabled(struct vcnl3020_data *data) +{ + int rc; + unsigned int icr; + + rc = regmap_read(data->regmap, VCNL_PS_ICR, &icr); + if (rc) + return false; + + return !!(icr & VCNL_ICR_THRES_EN); +} + +static int vcnl3020_config_threshold(struct iio_dev *indio_dev, bool state) +{ + struct vcnl3020_data *data = iio_priv(indio_dev); + int rc; + int icr; + int cmd; + int isr; + + if (state) { + rc = iio_device_claim_direct_mode(indio_dev); + if (rc) + return rc; + + /* Enable periodic measurement of proximity data. */ + cmd = VCNL_PS_EN | VCNL_PS_SELFTIMED_EN; + + /* + * Enable interrupts on threshold, for proximity data by + * default. + */ + icr = VCNL_ICR_THRES_EN; + } else { + if (!vcnl3020_is_thr_enabled(data)) + return 0; + + cmd = 0; + icr = 0; + isr = 0; + } + + rc = regmap_write(data->regmap, VCNL_COMMAND, cmd); + if (rc) + goto end; + + rc = regmap_write(data->regmap, VCNL_PS_ICR, icr); + if (rc) + goto end; + + if (!state) + /* Clear interrupts */ + rc = regmap_write(data->regmap, VCNL_ISR, isr); + +end: + if (state) + iio_device_release_direct_mode(indio_dev); + + return rc; +} + +static int vcnl3020_write_event_config(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir, + int state) +{ + switch (chan->type) { + case IIO_PROXIMITY: + return vcnl3020_config_threshold(indio_dev, state); + default: + return -EINVAL; + } +} + +static int vcnl3020_read_event_config(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, + enum iio_event_type type, + enum iio_event_direction dir) +{ + struct vcnl3020_data *data = iio_priv(indio_dev); + + switch (chan->type) { + case IIO_PROXIMITY: + return vcnl3020_is_thr_enabled(data); + default: + return -EINVAL; + } +} + +static const struct iio_event_spec vcnl3020_event_spec[] = { + { + .type = IIO_EV_TYPE_THRESH, + .dir = IIO_EV_DIR_EITHER, + .mask_separate = BIT(IIO_EV_INFO_ENABLE), + }, +}; + static const struct iio_chan_spec vcnl3020_channels[] = { { .type = IIO_PROXIMITY, .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SAMP_FREQ), .info_mask_separate_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), + .event_spec = vcnl3020_event_spec, + .num_event_specs = ARRAY_SIZE(vcnl3020_event_spec), }, }; @@ -233,6 +360,11 @@ static int vcnl3020_read_raw(struct iio_dev *indio_dev, switch (mask) { case IIO_CHAN_INFO_RAW: + + /* Protect against event capture. */ + if (vcnl3020_is_in_periodic_mode(data)) + return -EBUSY; + rc = vcnl3020_measure_proximity(data, val); if (rc) return rc; @@ -254,6 +386,10 @@ static int vcnl3020_write_raw(struct iio_dev *indio_dev, int rc; struct vcnl3020_data *data = iio_priv(indio_dev); + /* Protect against event capture. */ + if (vcnl3020_is_in_periodic_mode(data)) + return -EBUSY; + switch (mask) { case IIO_CHAN_INFO_SAMP_FREQ: rc = iio_device_claim_direct_mode(indio_dev); @@ -287,6 +423,8 @@ static const struct iio_info vcnl3020_info = { .read_raw = vcnl3020_read_raw, .write_raw = vcnl3020_write_raw, .read_avail = vcnl3020_read_avail, + .read_event_config = vcnl3020_read_event_config, + .write_event_config = vcnl3020_write_event_config, }; static const struct regmap_config vcnl3020_regmap_config = {
Add the possibility to run proximity sensor in periodic measurement mode. Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com> --- drivers/iio/proximity/vcnl3020.c | 138 +++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+)