* [PATCH 0/1] Adding support for range ignore on vl6180 @ 2020-03-05 14:16 Mike Goeppner 2020-03-05 14:16 ` [PATCH 1/1] iio: light: vl6180: Add support for range ignore Mike Goeppner 0 siblings, 1 reply; 3+ messages in thread From: Mike Goeppner @ 2020-03-05 14:16 UTC (permalink / raw) To: linux-iio; +Cc: pmeerw, manivannanece23, Mike Goeppner Hi, I've been working on getting support for the built in range ignore functionality on the vl6180 working. I've implemented this via some additional dev attrs in sysfs. This functionality is described in [1] on page 29 under the "Range ignore" header. The relevant registers are covered in sections 6.2.25, 6.2.26, and 6.2.28 of the linked datasheer. Would appreciate any feedback -- thanks! Cheers! /mike [1]: https://www.st.com/resource/en/datasheet/vl6180x.pdf Mike Goeppner (1): iio: light: vl6180: Add support for range ignore drivers/iio/light/vl6180.c | 180 ++++++++++++++++++++++++++++++++++++- 1 file changed, 177 insertions(+), 3 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/1] iio: light: vl6180: Add support for range ignore 2020-03-05 14:16 [PATCH 0/1] Adding support for range ignore on vl6180 Mike Goeppner @ 2020-03-05 14:16 ` Mike Goeppner 2020-03-07 15:18 ` Jonathan Cameron 0 siblings, 1 reply; 3+ messages in thread From: Mike Goeppner @ 2020-03-05 14:16 UTC (permalink / raw) To: linux-iio; +Cc: pmeerw, manivannanece23, Mike Goeppner This now makes it possible to configure and use the range ignore setting on the vl6180 sensor via sysfs. In order to do this, three new device files have been added, range_ignore_enable, range_ignore_threshold, and range_ignore_valid_height. range_ignore_threshold and range_ignore_valid_height write to the corresponding registers on the sensor. range_ignore_enable sets the sysrange__range_ignore_enable bit in the SYSRANGE__RANGE_CHECK_ENABLES register. Signed-off-by: Mike Goeppner <mikegoeppner@spotify.com> --- drivers/iio/light/vl6180.c | 180 ++++++++++++++++++++++++++++++++++++- 1 file changed, 177 insertions(+), 3 deletions(-) diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c index 192c77ef3608..4cc308de97f2 100644 --- a/drivers/iio/light/vl6180.c +++ b/drivers/iio/light/vl6180.c @@ -41,10 +41,18 @@ #define VL6180_OUT_OF_RESET 0x016 #define VL6180_HOLD 0x017 #define VL6180_RANGE_START 0x018 +#define VL6180_RANGE_CHECK_ENABLES 0x02d +#define VL6180_RANGE_IGNORE_VALID_HEIGHT 0x025 +#define VL6180_RANGE_IGNORE_THRESHOLD 0x026 #define VL6180_ALS_START 0x038 #define VL6180_ALS_GAIN 0x03f #define VL6180_ALS_IT 0x040 +/* Range check enable configuration */ +#define VL6180_ALS_EARLY_CONVERGENCE_ENABLE (1 << 0) +#define VL6180_RANGE_IGNORE_ENABLE (1 << 1) +#define VL6180_SIGNAL_TO_NOISE_ENABLE (1 << 4) + /* Status registers */ #define VL6180_RANGE_STATUS 0x04d #define VL6180_ALS_STATUS 0x04e @@ -128,7 +136,7 @@ static const struct vl6180_chan_regs vl6180_chan_regs_table[] = { }; static int vl6180_read(struct i2c_client *client, u16 cmd, void *databuf, - u8 len) + u8 len) { __be16 cmdbuf = cpu_to_be16(cmd); struct i2c_msg msgs[2] = { @@ -207,6 +215,76 @@ static int vl6180_write_word(struct i2c_client *client, u16 cmd, u16 val) return 0; } +static ssize_t vl6180_show_register_byte(struct device *dev, u16 cmd, + char *buff) +{ + int ret; + + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); + struct i2c_client *client = data->client; + + ret = vl6180_read_byte(client, cmd); + if (ret < 0) + return ret; + + return sprintf(buff, "%d\n", ret); +} + +static ssize_t vl6180_store_register_byte(struct device *dev, u16 cmd, + const char *buff, size_t len) +{ + int ret; + u8 val; + + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); + struct i2c_client *client = data->client; + + ret = kstrtou8(buff, 0, &val); + if (ret != 0) + return ret; + + ret = vl6180_write_byte(client, cmd, val); + if (ret < 0) + return ret; + + return len; +} + +static ssize_t vl6180_show_register_word(struct device *dev, u16 cmd, + char *buff) +{ + int ret; + + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); + struct i2c_client *client = data->client; + + ret = vl6180_read_word(client, cmd); + if (ret < 0) + return ret; + + return sprintf(buff, "%d\n", ret); +} + +static ssize_t vl6180_store_register_word(struct device *dev, u16 cmd, + const char *buff, size_t len) +{ + int ret; + u16 val; + + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); + struct i2c_client *client = data->client; + + ret = kstrtou16(buff, 0, &val); + if (ret != 0) + return ret; + + ret = vl6180_write_word(client, cmd, val); + if (ret < 0) + return ret; + + return len; +} + static int vl6180_measure(struct vl6180_data *data, int addr) { struct i2c_client *client = data->client; @@ -343,8 +421,104 @@ static int vl6180_read_raw(struct iio_dev *indio_dev, static IIO_CONST_ATTR(als_gain_available, "1 1.25 1.67 2.5 5 10 20 40"); +static ssize_t vl6180_show_range_ignore_threshold(struct device *dev, + struct device_attribute *attr, char *buff) +{ + return vl6180_show_register_word(dev, VL6180_RANGE_IGNORE_THRESHOLD, + buff); +} + +static ssize_t vl6180_store_range_ignore_threshold(struct device *dev, + struct device_attribute *attr, const char *buff, size_t len) +{ + return vl6180_store_register_word(dev, VL6180_RANGE_IGNORE_THRESHOLD, + buff, len); +} + +static IIO_DEVICE_ATTR(range_ignore_threshold, + 0644, + vl6180_show_range_ignore_threshold, + vl6180_store_range_ignore_threshold, + 0); + +static ssize_t vl6180_show_range_ignore_valid_height(struct device *dev, + struct device_attribute *attr, char *buff) +{ + return vl6180_show_register_byte(dev, VL6180_RANGE_IGNORE_VALID_HEIGHT, + buff); +} + +static ssize_t vl6180_store_range_ignore_valid_height(struct device *dev, + struct device_attribute *attr, const char *buff, size_t len) +{ + return vl6180_store_register_byte(dev, VL6180_RANGE_IGNORE_VALID_HEIGHT, + buff, len); +} + +static IIO_DEVICE_ATTR(range_ignore_valid_height, + 0644, + vl6180_show_range_ignore_valid_height, + vl6180_store_range_ignore_valid_height, + 0); + +static ssize_t vl6180_show_range_ignore_enable(struct device *dev, + struct device_attribute *attr, char *buff) +{ + int ret; + + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); + struct i2c_client *client = data->client; + + ret = vl6180_read_byte(client, VL6180_RANGE_CHECK_ENABLES); + if (ret < 0) + return ret; + + return sprintf(buff, "%d\n", !!(ret & VL6180_RANGE_IGNORE_ENABLE)); +} + +static ssize_t vl6180_store_range_ignore_enable(struct device *dev, + struct device_attribute *attr, const char *buff, size_t len) +{ + int ret; + bool val; + + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); + struct i2c_client *client = data->client; + + ret = kstrtobool(buff, &val); + if (ret != 0) + return ret; + + ret = vl6180_read_byte(client, VL6180_RANGE_CHECK_ENABLES); + if (ret < 0) + return ret; + + if (val) + ret = vl6180_write_byte(client, + VL6180_RANGE_CHECK_ENABLES, + ret | VL6180_RANGE_IGNORE_ENABLE); + else + ret = vl6180_write_byte(client, + VL6180_RANGE_CHECK_ENABLES, + ret & ~VL6180_RANGE_IGNORE_ENABLE); + + if (ret < 0) + return ret; + + return len; +} + +static IIO_DEVICE_ATTR(range_ignore_enable, + 0644, + vl6180_show_range_ignore_enable, + vl6180_store_range_ignore_enable, + 0); + static struct attribute *vl6180_attributes[] = { &iio_const_attr_als_gain_available.dev_attr.attr, + &iio_dev_attr_range_ignore_enable.dev_attr.attr, + &iio_dev_attr_range_ignore_threshold.dev_attr.attr, + &iio_dev_attr_range_ignore_valid_height.dev_attr.attr, NULL }; @@ -416,8 +590,8 @@ static int vl6180_set_it(struct vl6180_data *data, int val, int val2) } static int vl6180_write_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int val, int val2, long mask) + struct iio_chan_spec const *chan, + int val, int val2, long mask) { struct vl6180_data *data = iio_priv(indio_dev); -- 2.20.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] iio: light: vl6180: Add support for range ignore 2020-03-05 14:16 ` [PATCH 1/1] iio: light: vl6180: Add support for range ignore Mike Goeppner @ 2020-03-07 15:18 ` Jonathan Cameron 0 siblings, 0 replies; 3+ messages in thread From: Jonathan Cameron @ 2020-03-07 15:18 UTC (permalink / raw) To: Mike Goeppner; +Cc: linux-iio, pmeerw, manivannanece23 On Thu, 5 Mar 2020 15:16:15 +0100 Mike Goeppner <mikegoeppner@spotify.com> wrote: > This now makes it possible to configure and use the range ignore setting > on the vl6180 sensor via sysfs. In order to do this, three new device files > have been added, range_ignore_enable, range_ignore_threshold, and > range_ignore_valid_height. > > range_ignore_threshold and range_ignore_valid_height write to the > corresponding registers on the sensor. range_ignore_enable sets the > sysrange__range_ignore_enable bit in the SYSRANGE__RANGE_CHECK_ENABLES > register. > > Signed-off-by: Mike Goeppner <mikegoeppner@spotify.com> The best way to consider new ABI is not really the code, but rather the ABI documentation. Add some docs to Documentation/ABI/testing/sysfs-bus-iio-light-vl6180 Having taken a quick look at the docs, this seems to be a feature intended to avoid giving a reading from covering glass. If that's so, why do we want to change it at runtime? Seems like something that ought to be coming from device firmware and always be applied. (so put the controls in dt) A few things inline, but I suspect we'll move this to dt and the code will look rather different so feel free to not read them. thanks, Jonathan > --- > drivers/iio/light/vl6180.c | 180 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 177 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c > index 192c77ef3608..4cc308de97f2 100644 > --- a/drivers/iio/light/vl6180.c > +++ b/drivers/iio/light/vl6180.c > @@ -41,10 +41,18 @@ > #define VL6180_OUT_OF_RESET 0x016 > #define VL6180_HOLD 0x017 > #define VL6180_RANGE_START 0x018 > +#define VL6180_RANGE_CHECK_ENABLES 0x02d > +#define VL6180_RANGE_IGNORE_VALID_HEIGHT 0x025 > +#define VL6180_RANGE_IGNORE_THRESHOLD 0x026 > #define VL6180_ALS_START 0x038 > #define VL6180_ALS_GAIN 0x03f > #define VL6180_ALS_IT 0x040 > > +/* Range check enable configuration */ > +#define VL6180_ALS_EARLY_CONVERGENCE_ENABLE (1 << 0) > +#define VL6180_RANGE_IGNORE_ENABLE (1 << 1) > +#define VL6180_SIGNAL_TO_NOISE_ENABLE (1 << 4) > + > /* Status registers */ > #define VL6180_RANGE_STATUS 0x04d > #define VL6180_ALS_STATUS 0x04e > @@ -128,7 +136,7 @@ static const struct vl6180_chan_regs vl6180_chan_regs_table[] = { > }; > > static int vl6180_read(struct i2c_client *client, u16 cmd, void *databuf, > - u8 len) > + u8 len) > { > __be16 cmdbuf = cpu_to_be16(cmd); > struct i2c_msg msgs[2] = { > @@ -207,6 +215,76 @@ static int vl6180_write_word(struct i2c_client *client, u16 cmd, u16 val) > return 0; > } > > +static ssize_t vl6180_show_register_byte(struct device *dev, u16 cmd, > + char *buff) > +{ > + int ret; > + > + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); > + struct i2c_client *client = data->client; > + > + ret = vl6180_read_byte(client, cmd); > + if (ret < 0) > + return ret; > + > + return sprintf(buff, "%d\n", ret); > +} > + > +static ssize_t vl6180_store_register_byte(struct device *dev, u16 cmd, > + const char *buff, size_t len) > +{ > + int ret; > + u8 val; > + > + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); > + struct i2c_client *client = data->client; > + > + ret = kstrtou8(buff, 0, &val); > + if (ret != 0) > + return ret; > + > + ret = vl6180_write_byte(client, cmd, val); > + if (ret < 0) > + return ret; > + > + return len; > +} > + > +static ssize_t vl6180_show_register_word(struct device *dev, u16 cmd, > + char *buff) > +{ > + int ret; > + > + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); > + struct i2c_client *client = data->client; > + > + ret = vl6180_read_word(client, cmd); > + if (ret < 0) > + return ret; > + > + return sprintf(buff, "%d\n", ret); > +} > + > +static ssize_t vl6180_store_register_word(struct device *dev, u16 cmd, > + const char *buff, size_t len) > +{ > + int ret; > + u16 val; > + > + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); > + struct i2c_client *client = data->client; > + > + ret = kstrtou16(buff, 0, &val); > + if (ret != 0) > + return ret; > + > + ret = vl6180_write_word(client, cmd, val); > + if (ret < 0) > + return ret; > + > + return len; > +} > + > static int vl6180_measure(struct vl6180_data *data, int addr) > { > struct i2c_client *client = data->client; > @@ -343,8 +421,104 @@ static int vl6180_read_raw(struct iio_dev *indio_dev, > > static IIO_CONST_ATTR(als_gain_available, "1 1.25 1.67 2.5 5 10 20 40"); > > +static ssize_t vl6180_show_range_ignore_threshold(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + return vl6180_show_register_word(dev, VL6180_RANGE_IGNORE_THRESHOLD, > + buff); > +} > + > +static ssize_t vl6180_store_range_ignore_threshold(struct device *dev, > + struct device_attribute *attr, const char *buff, size_t len) > +{ > + return vl6180_store_register_word(dev, VL6180_RANGE_IGNORE_THRESHOLD, > + buff, len); > +} > + > +static IIO_DEVICE_ATTR(range_ignore_threshold, > + 0644, > + vl6180_show_range_ignore_threshold, > + vl6180_store_range_ignore_threshold, > + 0); > + > +static ssize_t vl6180_show_range_ignore_valid_height(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + return vl6180_show_register_byte(dev, VL6180_RANGE_IGNORE_VALID_HEIGHT, > + buff); > +} > + > +static ssize_t vl6180_store_range_ignore_valid_height(struct device *dev, > + struct device_attribute *attr, const char *buff, size_t len) > +{ > + return vl6180_store_register_byte(dev, VL6180_RANGE_IGNORE_VALID_HEIGHT, > + buff, len); > +} > + > +static IIO_DEVICE_ATTR(range_ignore_valid_height, > + 0644, > + vl6180_show_range_ignore_valid_height, > + vl6180_store_range_ignore_valid_height, > + 0); > + > +static ssize_t vl6180_show_range_ignore_enable(struct device *dev, > + struct device_attribute *attr, char *buff) > +{ > + int ret; > + > + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); > + struct i2c_client *client = data->client; > + > + ret = vl6180_read_byte(client, VL6180_RANGE_CHECK_ENABLES); > + if (ret < 0) > + return ret; > + > + return sprintf(buff, "%d\n", !!(ret & VL6180_RANGE_IGNORE_ENABLE)); > +} > + > +static ssize_t vl6180_store_range_ignore_enable(struct device *dev, > + struct device_attribute *attr, const char *buff, size_t len) > +{ > + int ret; > + bool val; > + > + struct vl6180_data *data = iio_priv(dev_to_iio_dev(dev)); > + struct i2c_client *client = data->client; > + > + ret = kstrtobool(buff, &val); > + if (ret != 0) > + return ret; > + > + ret = vl6180_read_byte(client, VL6180_RANGE_CHECK_ENABLES); > + if (ret < 0) > + return ret; > + It's worth checking if you are actually making a change. Saves on bus transfers which might be better deployed for something else. > + if (val) > + ret = vl6180_write_byte(client, > + VL6180_RANGE_CHECK_ENABLES, > + ret | VL6180_RANGE_IGNORE_ENABLE); > + else > + ret = vl6180_write_byte(client, > + VL6180_RANGE_CHECK_ENABLES, > + ret & ~VL6180_RANGE_IGNORE_ENABLE); > + > + if (ret < 0) > + return ret; > + > + return len; > +} > + > +static IIO_DEVICE_ATTR(range_ignore_enable, > + 0644, > + vl6180_show_range_ignore_enable, > + vl6180_store_range_ignore_enable, > + 0); > + > static struct attribute *vl6180_attributes[] = { > &iio_const_attr_als_gain_available.dev_attr.attr, > + &iio_dev_attr_range_ignore_enable.dev_attr.attr, > + &iio_dev_attr_range_ignore_threshold.dev_attr.attr, > + &iio_dev_attr_range_ignore_valid_height.dev_attr.attr, > NULL > }; > > @@ -416,8 +590,8 @@ static int vl6180_set_it(struct vl6180_data *data, int val, int val2) > } > > static int vl6180_write_raw(struct iio_dev *indio_dev, > - struct iio_chan_spec const *chan, > - int val, int val2, long mask) > + struct iio_chan_spec const *chan, > + int val, int val2, long mask) Make sure to clean out any unrelated white space changes before sending a patch out. If they make sense and you want to do them then do it in a separate patch. In here they are just distracting noise whilst people try to figure out if there were real changes... > { > struct vl6180_data *data = iio_priv(indio_dev); > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-07 15:18 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-03-05 14:16 [PATCH 0/1] Adding support for range ignore on vl6180 Mike Goeppner 2020-03-05 14:16 ` [PATCH 1/1] iio: light: vl6180: Add support for range ignore Mike Goeppner 2020-03-07 15:18 ` Jonathan Cameron
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.