| Message ID | 20240913-veml6035-v1-7-0b09c0c90418@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: light: veml6030: fix issues and add support for veml6035 | expand |
On Fri, 13 Sep 2024 15:19:02 +0200 Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote: > The veml6035 is an ALS that shares most of its functionality with the > veml6030, which allows for some code recycling. > > Some chip-specific properties differ and dedicated functions to get and > set the sensor gain as well as its initialization are required. > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> Mostly a request to first switch to using read_avail() and the relevant bit masks instead of custom attributes. That will require converting the driver to that approach first, but looks straight forward. > --- > drivers/iio/light/veml6030.c | 300 +++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 273 insertions(+), 27 deletions(-) > > diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c > index 2945cc1db599..105f310c4954 100644 > --- a/drivers/iio/light/veml6030.c > +++ b/drivers/iio/light/veml6030.c > @@ -1,13 +1,19 @@ > // SPDX-License-Identifier: GPL-2.0+ > /* > - * VEML6030 Ambient Light Sensor > + * VEML6030 and VMEL6035 Ambient Light Sensors > * > * Copyright (c) 2019, Rishi Gupta <gupt21@gmail.com> > * > + * VEML6030: > * Datasheet: https://www.vishay.com/docs/84366/veml6030.pdf > * Appnote-84367: https://www.vishay.com/docs/84367/designingveml6030.pdf > + * > + * VEML6035: > + * Datasheet: https://www.vishay.com/docs/84889/veml6035.pdf > + * Appnote-84944: https://www.vishay.com/docs/84944/designingveml6035.pdf > */ > > +#include <linux/bitfield.h> > #include <linux/module.h> > #include <linux/i2c.h> > #include <linux/err.h> > @@ -38,16 +44,33 @@ > #define VEML6030_ALS_INT_EN BIT(1) > #define VEML6030_ALS_SD BIT(0) > > +#define VEML6035_GAIN_M GENMASK(12, 10) > +#define VEML6035_GAIN BIT(10) > +#define VEML6035_DG BIT(11) > +#define VEML6035_SENS BIT(12) > +#define VEML6035_INT_CHAN BIT(3) > +#define VEML6035_CHAN_EN BIT(2) > + > +struct veml603x_chip { > + const char *name; > + const struct iio_info *info; > + const struct iio_info *info_no_irq; > + const char * const in_illuminance_scale_avail; For this, better with read_avail() provided and a pointer to an array of values + a size element in here. That way we can get rid of the custom attribute handling. Might end up as similar amount of code, but will be simpler to read. > + int (*hw_init)(struct iio_dev *indio_dev); > + int (*set_als_gain)(struct iio_dev *indio_dev, int val, int val2); > + int (*get_als_gain)(struct iio_dev *indio_dev, int *val, int *val2); > +}; > > /* Integration time available in seconds */ > @@ -63,14 +87,25 @@ static IIO_CONST_ATTR(in_illuminance_integration_time_available, > > /* > * Scale is 1/gain. Value 0.125 is ALS gain x (1/8), 0.25 is > - * ALS gain x (1/4), 1.0 = ALS gain x 1 and 2.0 is ALS gain x 2. > + * ALS gain x (1/4), 0.5 is ALS gain x (1/2), 1.0 is ALS gain x 1, > + * 2.0 is ALS gain x2, and 4.0 is ALS gain x 4. > */ > -static IIO_CONST_ATTR(in_illuminance_scale_available, > +static IIO_CONST_ATTR_NAMED(veml6030_in_illuminance_scale_available, > + in_illuminance_scale_available, > "0.125 0.25 1.0 2.0"); > +static IIO_CONST_ATTR_NAMED(veml6035_in_illuminance_scale_available, > + in_illuminance_scale_available, > + "0.125 0.25 0.5 1.0 2.0 4.0"); > > static struct attribute *veml6030_attributes[] = { > &iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr, > - &iio_const_attr_in_illuminance_scale_available.dev_attr.attr, > + &iio_const_attr_veml6030_in_illuminance_scale_available.dev_attr.attr, > + NULL > +}; > + > +static struct attribute *veml6035_attributes[] = { > + &iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr, > + &iio_const_attr_veml6035_in_illuminance_scale_available.dev_attr.attr, Using get_avail() etc would let you handle these as arrays of numbers rather than strings + get rid of the need for any custom attributes. This should be a very simple conversion so perhaps worth doing before adding the new support. Then you will have pointers to the value arrays + sizes in your chip specific structures that just get looked up directly by read_avail() > NULL > }; > > +/* > + * Set ALS gain to 1/8, integration time to 100 ms, ALS and WHITE > + * channel enabled, ALS channel interrupt, PSM enabled, > + * PSM_WAIT = 0.8 s, persistence to 1 x integration time and the > + * threshold interrupt disabled by default. First shutdown the sensor, > + * update registers and then power on the sensor. > + */ > +static int veml6035_hw_init(struct iio_dev *indio_dev) > +{ > + int ret, val; > + struct veml6030_data *data = iio_priv(indio_dev); > + struct i2c_client *client = data->client; > + > + ret = veml6030_als_shut_down(data); > + if (ret) { > + dev_err(&client->dev, "can't shutdown als %d\n", ret); > + return ret; If this is only ever called from probe() (I think that's true?) can use return dev_err_probe() for all these error cases. Main advantage here being shorter simpler code. > + } > + > + ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, > + VEML6035_SENS | VEML6035_CHAN_EN | VEML6030_ALS_SD); > + if (ret) { > + dev_err(&client->dev, "can't setup als configs %d\n", ret); > + return ret; > + } > + > + ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM, > + VEML6030_PSM | VEML6030_PSM_EN, 0x03); > + if (ret) { > + dev_err(&client->dev, "can't setup default PSM %d\n", ret); > + return ret; > + } > + > + ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF); > + if (ret) { > + dev_err(&client->dev, "can't setup high threshold %d\n", ret); > + return ret; > + } > + > + ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000); > + if (ret) { > + dev_err(&client->dev, "can't setup low threshold %d\n", ret); > + return ret; > + } > + > + ret = veml6030_als_pwr_on(data); > + if (ret) { > + dev_err(&client->dev, "can't poweron als %d\n", ret); > + return ret; > + } > + > + /* Clear stale interrupt status bits if any during start */ > + ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val); > + if (ret < 0) { > + dev_err(&client->dev, > + "can't clear als interrupt status %d\n", ret); > + return ret; It's true of existing code, but I noticed it here. Should we be powering down in this error path? > + } > + > + /* Cache currently active measurement parameters */ > + data->cur_gain = 5; > + data->cur_resolution = 1024; > + data->cur_integration_time = 3; > + > + return 0; > +}
On 14/09/2024 18:03, Jonathan Cameron wrote: > On Fri, 13 Sep 2024 15:19:02 +0200 > Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote: > >> The veml6035 is an ALS that shares most of its functionality with the >> veml6030, which allows for some code recycling. >> >> Some chip-specific properties differ and dedicated functions to get and >> set the sensor gain as well as its initialization are required. >> >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> > Mostly a request to first switch to using read_avail() and the relevant > bit masks instead of custom attributes. That will require converting the > driver to that approach first, but looks straight forward. > >> --- >> drivers/iio/light/veml6030.c | 300 +++++++++++++++++++++++++++++++++++++++---- >> 1 file changed, 273 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c >> index 2945cc1db599..105f310c4954 100644 >> --- a/drivers/iio/light/veml6030.c >> +++ b/drivers/iio/light/veml6030.c >> @@ -1,13 +1,19 @@ >> // SPDX-License-Identifier: GPL-2.0+ >> /* >> - * VEML6030 Ambient Light Sensor >> + * VEML6030 and VMEL6035 Ambient Light Sensors >> * >> * Copyright (c) 2019, Rishi Gupta <gupt21@gmail.com> >> * >> + * VEML6030: >> * Datasheet: https://www.vishay.com/docs/84366/veml6030.pdf >> * Appnote-84367: https://www.vishay.com/docs/84367/designingveml6030.pdf >> + * >> + * VEML6035: >> + * Datasheet: https://www.vishay.com/docs/84889/veml6035.pdf >> + * Appnote-84944: https://www.vishay.com/docs/84944/designingveml6035.pdf >> */ >> >> +#include <linux/bitfield.h> >> #include <linux/module.h> >> #include <linux/i2c.h> >> #include <linux/err.h> >> @@ -38,16 +44,33 @@ >> #define VEML6030_ALS_INT_EN BIT(1) >> #define VEML6030_ALS_SD BIT(0) >> >> +#define VEML6035_GAIN_M GENMASK(12, 10) >> +#define VEML6035_GAIN BIT(10) >> +#define VEML6035_DG BIT(11) >> +#define VEML6035_SENS BIT(12) >> +#define VEML6035_INT_CHAN BIT(3) >> +#define VEML6035_CHAN_EN BIT(2) >> + >> +struct veml603x_chip { >> + const char *name; >> + const struct iio_info *info; >> + const struct iio_info *info_no_irq; >> + const char * const in_illuminance_scale_avail; > > For this, better with read_avail() provided and a pointer to an array of > values + a size element in here. That way we can get rid of the > custom attribute handling. Might end up as similar amount of code, but > will be simpler to read. > >> + int (*hw_init)(struct iio_dev *indio_dev); >> + int (*set_als_gain)(struct iio_dev *indio_dev, int val, int val2); >> + int (*get_als_gain)(struct iio_dev *indio_dev, int *val, int *val2); >> +}; > >> >> /* Integration time available in seconds */ >> @@ -63,14 +87,25 @@ static IIO_CONST_ATTR(in_illuminance_integration_time_available, >> >> /* >> * Scale is 1/gain. Value 0.125 is ALS gain x (1/8), 0.25 is >> - * ALS gain x (1/4), 1.0 = ALS gain x 1 and 2.0 is ALS gain x 2. >> + * ALS gain x (1/4), 0.5 is ALS gain x (1/2), 1.0 is ALS gain x 1, >> + * 2.0 is ALS gain x2, and 4.0 is ALS gain x 4. >> */ >> -static IIO_CONST_ATTR(in_illuminance_scale_available, >> +static IIO_CONST_ATTR_NAMED(veml6030_in_illuminance_scale_available, >> + in_illuminance_scale_available, >> "0.125 0.25 1.0 2.0"); >> +static IIO_CONST_ATTR_NAMED(veml6035_in_illuminance_scale_available, >> + in_illuminance_scale_available, >> + "0.125 0.25 0.5 1.0 2.0 4.0"); >> >> static struct attribute *veml6030_attributes[] = { >> &iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr, >> - &iio_const_attr_in_illuminance_scale_available.dev_attr.attr, >> + &iio_const_attr_veml6030_in_illuminance_scale_available.dev_attr.attr, >> + NULL >> +}; >> + >> +static struct attribute *veml6035_attributes[] = { >> + &iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr, >> + &iio_const_attr_veml6035_in_illuminance_scale_available.dev_attr.attr, > > Using get_avail() etc would let you handle these as arrays of numbers rather than > strings + get rid of the need for any custom attributes. This should be > a very simple conversion so perhaps worth doing before adding the > new support. Then you will have pointers to the value arrays + sizes > in your chip specific structures that just get looked up directly > by read_avail() > > Hi Jonathan, thanks for your review. I will update the update the driver to work that way. >> NULL >> }; > > >> >> +/* >> + * Set ALS gain to 1/8, integration time to 100 ms, ALS and WHITE >> + * channel enabled, ALS channel interrupt, PSM enabled, >> + * PSM_WAIT = 0.8 s, persistence to 1 x integration time and the >> + * threshold interrupt disabled by default. First shutdown the sensor, >> + * update registers and then power on the sensor. >> + */ >> +static int veml6035_hw_init(struct iio_dev *indio_dev) >> +{ >> + int ret, val; >> + struct veml6030_data *data = iio_priv(indio_dev); >> + struct i2c_client *client = data->client; >> + >> + ret = veml6030_als_shut_down(data); >> + if (ret) { >> + dev_err(&client->dev, "can't shutdown als %d\n", ret); >> + return ret; > > If this is only ever called from probe() (I think that's true?) > can use return dev_err_probe() for all these error cases. > Main advantage here being shorter simpler code. > I know, I procrastinated a little bit and I left the dev_err() calls as they are. But that's easy to update, so I will add a patch for it in v2. >> + } >> + >> + ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, >> + VEML6035_SENS | VEML6035_CHAN_EN | VEML6030_ALS_SD); >> + if (ret) { >> + dev_err(&client->dev, "can't setup als configs %d\n", ret); >> + return ret; >> + } >> + >> + ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM, >> + VEML6030_PSM | VEML6030_PSM_EN, 0x03); >> + if (ret) { >> + dev_err(&client->dev, "can't setup default PSM %d\n", ret); >> + return ret; >> + } >> + >> + ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF); >> + if (ret) { >> + dev_err(&client->dev, "can't setup high threshold %d\n", ret); >> + return ret; >> + } >> + >> + ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000); >> + if (ret) { >> + dev_err(&client->dev, "can't setup low threshold %d\n", ret); >> + return ret; >> + } >> + >> + ret = veml6030_als_pwr_on(data); >> + if (ret) { >> + dev_err(&client->dev, "can't poweron als %d\n", ret); >> + return ret; >> + } >> + >> + /* Clear stale interrupt status bits if any during start */ >> + ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val); >> + if (ret < 0) { >> + dev_err(&client->dev, >> + "can't clear als interrupt status %d\n", ret); >> + return ret; > > It's true of existing code, but I noticed it here. > Should we be powering down in this error path? > We could, because this is the only error path where the device is powered on before the power off action gets registered. On the other hand, could we not move the call to devm_add_action_or_reset() a few lines up, so the action gets registered before calling hw_init()? Powering off the device is just writing a bit, so it would not hurt in the error paths where the device is already powered off. Then we would not need an explicit call to power off the device in this error path. >> + } >> + >> + /* Cache currently active measurement parameters */ >> + data->cur_gain = 5; >> + data->cur_resolution = 1024; >> + data->cur_integration_time = 3; >> + >> + return 0; >> +} > Best regards, Javier Carrasco
... > >> > >> +/* > >> + * Set ALS gain to 1/8, integration time to 100 ms, ALS and WHITE > >> + * channel enabled, ALS channel interrupt, PSM enabled, > >> + * PSM_WAIT = 0.8 s, persistence to 1 x integration time and the > >> + * threshold interrupt disabled by default. First shutdown the sensor, > >> + * update registers and then power on the sensor. > >> + */ > >> +static int veml6035_hw_init(struct iio_dev *indio_dev) > >> +{ > >> + int ret, val; > >> + struct veml6030_data *data = iio_priv(indio_dev); > >> + struct i2c_client *client = data->client; > >> + > >> + ret = veml6030_als_shut_down(data); > >> + if (ret) { > >> + dev_err(&client->dev, "can't shutdown als %d\n", ret); > >> + return ret; > > > > If this is only ever called from probe() (I think that's true?) > > can use return dev_err_probe() for all these error cases. > > Main advantage here being shorter simpler code. > > > > I know, I procrastinated a little bit and I left the dev_err() calls as > they are. But that's easy to update, so I will add a patch for it in v2. > >> + } > >> + > >> + ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, > >> + VEML6035_SENS | VEML6035_CHAN_EN | VEML6030_ALS_SD); > >> + if (ret) { > >> + dev_err(&client->dev, "can't setup als configs %d\n", ret); > >> + return ret; > >> + } > >> + > >> + ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM, > >> + VEML6030_PSM | VEML6030_PSM_EN, 0x03); > >> + if (ret) { > >> + dev_err(&client->dev, "can't setup default PSM %d\n", ret); > >> + return ret; > >> + } > >> + > >> + ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF); > >> + if (ret) { > >> + dev_err(&client->dev, "can't setup high threshold %d\n", ret); > >> + return ret; > >> + } > >> + > >> + ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000); > >> + if (ret) { > >> + dev_err(&client->dev, "can't setup low threshold %d\n", ret); > >> + return ret; > >> + } > >> + > >> + ret = veml6030_als_pwr_on(data); > >> + if (ret) { > >> + dev_err(&client->dev, "can't poweron als %d\n", ret); > >> + return ret; > >> + } > >> + > >> + /* Clear stale interrupt status bits if any during start */ > >> + ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val); > >> + if (ret < 0) { > >> + dev_err(&client->dev, > >> + "can't clear als interrupt status %d\n", ret); > >> + return ret; > > > > It's true of existing code, but I noticed it here. > > Should we be powering down in this error path? > > > > We could, because this is the only error path where the device is > powered on before the power off action gets registered. On the other > hand, could we not move the call to devm_add_action_or_reset() a few > lines up, so the action gets registered before calling hw_init()? Move it in here so that you register the powerdown alongside the power up. Doesn't matter that it is inside the hw_init() function so a little less obvious. Jonathan > > Powering off the device is just writing a bit, so it would not hurt in > the error paths where the device is already powered off. Then we would > not need an explicit call to power off the device in this error path. > > >> + } > >> + > >> + /* Cache currently active measurement parameters */ > >> + data->cur_gain = 5; > >> + data->cur_resolution = 1024; > >> + data->cur_integration_time = 3; > >> + > >> + return 0; > >> +} > > > > > Best regards, > Javier Carrasco
diff --git a/drivers/iio/light/veml6030.c b/drivers/iio/light/veml6030.c index 2945cc1db599..105f310c4954 100644 --- a/drivers/iio/light/veml6030.c +++ b/drivers/iio/light/veml6030.c @@ -1,13 +1,19 @@ // SPDX-License-Identifier: GPL-2.0+ /* - * VEML6030 Ambient Light Sensor + * VEML6030 and VMEL6035 Ambient Light Sensors * * Copyright (c) 2019, Rishi Gupta <gupt21@gmail.com> * + * VEML6030: * Datasheet: https://www.vishay.com/docs/84366/veml6030.pdf * Appnote-84367: https://www.vishay.com/docs/84367/designingveml6030.pdf + * + * VEML6035: + * Datasheet: https://www.vishay.com/docs/84889/veml6035.pdf + * Appnote-84944: https://www.vishay.com/docs/84944/designingveml6035.pdf */ +#include <linux/bitfield.h> #include <linux/module.h> #include <linux/i2c.h> #include <linux/err.h> @@ -38,16 +44,33 @@ #define VEML6030_ALS_INT_EN BIT(1) #define VEML6030_ALS_SD BIT(0) +#define VEML6035_GAIN_M GENMASK(12, 10) +#define VEML6035_GAIN BIT(10) +#define VEML6035_DG BIT(11) +#define VEML6035_SENS BIT(12) +#define VEML6035_INT_CHAN BIT(3) +#define VEML6035_CHAN_EN BIT(2) + +struct veml603x_chip { + const char *name; + const struct iio_info *info; + const struct iio_info *info_no_irq; + const char * const in_illuminance_scale_avail; + int (*hw_init)(struct iio_dev *indio_dev); + int (*set_als_gain)(struct iio_dev *indio_dev, int val, int val2); + int (*get_als_gain)(struct iio_dev *indio_dev, int *val, int *val2); +}; + /* * The resolution depends on both gain and integration time. The * cur_resolution stores one of the resolution mentioned in the * table during startup and gets updated whenever integration time * or gain is changed. * - * Table 'resolution and maximum detection range' in appnote 84367 + * Table 'resolution and maximum detection range' in the appnotes * is visualized as a 2D array. The cur_gain stores index of gain - * in this table (0-3) while the cur_integration_time holds index - * of integration time (0-5). + * in this table (0-3 for VEML6030, 0-5 for VEML6035) while the + * cur_integration_time holds index of integration time (0-5). */ struct veml6030_data { struct i2c_client *client; @@ -55,6 +78,7 @@ struct veml6030_data { int cur_resolution; int cur_gain; int cur_integration_time; + const struct veml603x_chip *chip; }; /* Integration time available in seconds */ @@ -63,14 +87,25 @@ static IIO_CONST_ATTR(in_illuminance_integration_time_available, /* * Scale is 1/gain. Value 0.125 is ALS gain x (1/8), 0.25 is - * ALS gain x (1/4), 1.0 = ALS gain x 1 and 2.0 is ALS gain x 2. + * ALS gain x (1/4), 0.5 is ALS gain x (1/2), 1.0 is ALS gain x 1, + * 2.0 is ALS gain x2, and 4.0 is ALS gain x 4. */ -static IIO_CONST_ATTR(in_illuminance_scale_available, +static IIO_CONST_ATTR_NAMED(veml6030_in_illuminance_scale_available, + in_illuminance_scale_available, "0.125 0.25 1.0 2.0"); +static IIO_CONST_ATTR_NAMED(veml6035_in_illuminance_scale_available, + in_illuminance_scale_available, + "0.125 0.25 0.5 1.0 2.0 4.0"); static struct attribute *veml6030_attributes[] = { &iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr, - &iio_const_attr_in_illuminance_scale_available.dev_attr.attr, + &iio_const_attr_veml6030_in_illuminance_scale_available.dev_attr.attr, + NULL +}; + +static struct attribute *veml6035_attributes[] = { + &iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr, + &iio_const_attr_veml6035_in_illuminance_scale_available.dev_attr.attr, NULL }; @@ -78,6 +113,10 @@ static const struct attribute_group veml6030_attr_group = { .attrs = veml6030_attributes, }; +static const struct attribute_group veml6035_attr_group = { + .attrs = veml6035_attributes, +}; + /* * Persistence = 1/2/4/8 x integration time * Minimum time for which light readings must stay above configured @@ -380,6 +419,21 @@ static int veml6030_write_persistence(struct iio_dev *indio_dev, return ret; } +/* + * Cache currently set gain & update resolution. For every + * increase in the gain to next level, resolution is halved + * and vice-versa. + */ +static void veml6030_update_gain_res(struct veml6030_data *data, int gain_idx) +{ + if (data->cur_gain < gain_idx) + data->cur_resolution <<= gain_idx - data->cur_gain; + else if (data->cur_gain > gain_idx) + data->cur_resolution >>= data->cur_gain - gain_idx; + + data->cur_gain = gain_idx; +} + static int veml6030_set_als_gain(struct iio_dev *indio_dev, int val, int val2) { @@ -410,19 +464,49 @@ static int veml6030_set_als_gain(struct iio_dev *indio_dev, return ret; } - /* - * Cache currently set gain & update resolution. For every - * increase in the gain to next level, resolution is halved - * and vice-versa. - */ - if (data->cur_gain < gain_idx) - data->cur_resolution <<= gain_idx - data->cur_gain; - else if (data->cur_gain > gain_idx) - data->cur_resolution >>= data->cur_gain - gain_idx; + veml6030_update_gain_res(data, gain_idx); - data->cur_gain = gain_idx; + return 0; +} - return ret; +static int veml6035_set_als_gain(struct iio_dev *indio_dev, int val, int val2) +{ + int ret, new_gain, gain_idx; + struct veml6030_data *data = iio_priv(indio_dev); + + if (val == 0 && val2 == 125000) { + new_gain = VEML6035_SENS; + gain_idx = 5; + } else if (val == 0 && val2 == 250000) { + new_gain = VEML6035_SENS | VEML6035_GAIN; + gain_idx = 4; + } else if (val == 0 && val2 == 500000) { + new_gain = VEML6035_SENS | VEML6035_GAIN | + VEML6035_DG; + gain_idx = 3; + } else if (val == 1 && val2 == 0) { + new_gain = 0x0000; + gain_idx = 2; + } else if (val == 2 && val2 == 0) { + new_gain = VEML6035_GAIN; + gain_idx = 1; + } else if (val == 4 && val2 == 0) { + new_gain = VEML6035_GAIN | VEML6035_DG; + gain_idx = 0; + } else { + return -EINVAL; + } + + ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_CONF, + VEML6035_GAIN_M, new_gain); + if (ret) { + dev_err(&data->client->dev, "can't set als gain %d\n", ret); + return ret; + } + + veml6030_update_gain_res(data, gain_idx); + + return 0; } static int veml6030_get_als_gain(struct iio_dev *indio_dev, @@ -462,6 +546,52 @@ static int veml6030_get_als_gain(struct iio_dev *indio_dev, return IIO_VAL_INT_PLUS_MICRO; } +static int veml6035_get_als_gain(struct iio_dev *indio_dev, int *val, int *val2) +{ + int ret, reg; + struct veml6030_data *data = iio_priv(indio_dev); + + ret = regmap_read(data->regmap, VEML6030_REG_ALS_CONF, ®); + if (ret) { + dev_err(&data->client->dev, + "can't read als conf register %d\n", ret); + return ret; + } + + switch (FIELD_GET(VEML6035_GAIN_M, reg)) { + case 0: + *val = 1; + *val2 = 0; + break; + case 1: + case 2: + *val = 2; + *val2 = 0; + break; + case 3: + *val = 4; + *val2 = 0; + break; + case 4: + *val = 0; + *val2 = 125000; + break; + case 5: + case 6: + *val = 0; + *val2 = 250000; + break; + case 7: + *val = 0; + *val2 = 500000; + break; + default: + return -EINVAL; + } + + return IIO_VAL_INT_PLUS_MICRO; +} + static int veml6030_read_thresh(struct iio_dev *indio_dev, int *val, int *val2, int dir) { @@ -558,7 +688,7 @@ static int veml6030_read_raw(struct iio_dev *indio_dev, return -EINVAL; case IIO_CHAN_INFO_SCALE: if (chan->type == IIO_LIGHT) - return veml6030_get_als_gain(indio_dev, val, val2); + return data->chip->get_als_gain(indio_dev, val, val2); return -EINVAL; default: return -EINVAL; @@ -569,6 +699,8 @@ static int veml6030_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int val, int val2, long mask) { + struct veml6030_data *data = iio_priv(indio_dev); + switch (mask) { case IIO_CHAN_INFO_INT_TIME: switch (chan->type) { @@ -580,7 +712,7 @@ static int veml6030_write_raw(struct iio_dev *indio_dev, case IIO_CHAN_INFO_SCALE: switch (chan->type) { case IIO_LIGHT: - return veml6030_set_als_gain(indio_dev, val, val2); + return data->chip->set_als_gain(indio_dev, val, val2); default: return -EINVAL; } @@ -691,12 +823,29 @@ static const struct iio_info veml6030_info = { .event_attrs = &veml6030_event_attr_group, }; +static const struct iio_info veml6035_info = { + .read_raw = veml6030_read_raw, + .write_raw = veml6030_write_raw, + .read_event_value = veml6030_read_event_val, + .write_event_value = veml6030_write_event_val, + .read_event_config = veml6030_read_interrupt_config, + .write_event_config = veml6030_write_interrupt_config, + .attrs = &veml6035_attr_group, + .event_attrs = &veml6030_event_attr_group, +}; + static const struct iio_info veml6030_info_no_irq = { .read_raw = veml6030_read_raw, .write_raw = veml6030_write_raw, .attrs = &veml6030_attr_group, }; +static const struct iio_info veml6035_info_no_irq = { + .read_raw = veml6030_read_raw, + .write_raw = veml6030_write_raw, + .attrs = &veml6035_attr_group, +}; + static irqreturn_t veml6030_event_handler(int irq, void *private) { int ret, reg, evtdir; @@ -791,6 +940,73 @@ static int veml6030_hw_init(struct iio_dev *indio_dev) return ret; } +/* + * Set ALS gain to 1/8, integration time to 100 ms, ALS and WHITE + * channel enabled, ALS channel interrupt, PSM enabled, + * PSM_WAIT = 0.8 s, persistence to 1 x integration time and the + * threshold interrupt disabled by default. First shutdown the sensor, + * update registers and then power on the sensor. + */ +static int veml6035_hw_init(struct iio_dev *indio_dev) +{ + int ret, val; + struct veml6030_data *data = iio_priv(indio_dev); + struct i2c_client *client = data->client; + + ret = veml6030_als_shut_down(data); + if (ret) { + dev_err(&client->dev, "can't shutdown als %d\n", ret); + return ret; + } + + ret = regmap_write(data->regmap, VEML6030_REG_ALS_CONF, + VEML6035_SENS | VEML6035_CHAN_EN | VEML6030_ALS_SD); + if (ret) { + dev_err(&client->dev, "can't setup als configs %d\n", ret); + return ret; + } + + ret = regmap_update_bits(data->regmap, VEML6030_REG_ALS_PSM, + VEML6030_PSM | VEML6030_PSM_EN, 0x03); + if (ret) { + dev_err(&client->dev, "can't setup default PSM %d\n", ret); + return ret; + } + + ret = regmap_write(data->regmap, VEML6030_REG_ALS_WH, 0xFFFF); + if (ret) { + dev_err(&client->dev, "can't setup high threshold %d\n", ret); + return ret; + } + + ret = regmap_write(data->regmap, VEML6030_REG_ALS_WL, 0x0000); + if (ret) { + dev_err(&client->dev, "can't setup low threshold %d\n", ret); + return ret; + } + + ret = veml6030_als_pwr_on(data); + if (ret) { + dev_err(&client->dev, "can't poweron als %d\n", ret); + return ret; + } + + /* Clear stale interrupt status bits if any during start */ + ret = regmap_read(data->regmap, VEML6030_REG_ALS_INT, &val); + if (ret < 0) { + dev_err(&client->dev, + "can't clear als interrupt status %d\n", ret); + return ret; + } + + /* Cache currently active measurement parameters */ + data->cur_gain = 5; + data->cur_resolution = 1024; + data->cur_integration_time = 3; + + return 0; +} + static int veml6030_probe(struct i2c_client *client) { int ret; @@ -818,7 +1034,11 @@ static int veml6030_probe(struct i2c_client *client) data->client = client; data->regmap = regmap; - indio_dev->name = "veml6030"; + data->chip = i2c_get_match_data(client); + if (!data->chip) + return -EINVAL; + + indio_dev->name = data->chip->name; indio_dev->channels = veml6030_channels; indio_dev->num_channels = ARRAY_SIZE(veml6030_channels); indio_dev->modes = INDIO_DIRECT_MODE; @@ -827,18 +1047,18 @@ static int veml6030_probe(struct i2c_client *client) ret = devm_request_threaded_irq(&client->dev, client->irq, NULL, veml6030_event_handler, IRQF_TRIGGER_LOW | IRQF_ONESHOT, - "veml6030", indio_dev); + indio_dev->name, indio_dev); if (ret < 0) { dev_err(&client->dev, "irq %d request failed\n", client->irq); return ret; } - indio_dev->info = &veml6030_info; + indio_dev->info = data->chip->info; } else { - indio_dev->info = &veml6030_info_no_irq; + indio_dev->info = data->chip->info_no_irq; } - ret = veml6030_hw_init(indio_dev); + ret = data->chip->hw_init(indio_dev); if (ret < 0) return ret; @@ -879,14 +1099,40 @@ static int veml6030_runtime_resume(struct device *dev) static DEFINE_RUNTIME_DEV_PM_OPS(veml6030_pm_ops, veml6030_runtime_suspend, veml6030_runtime_resume, NULL); +static const struct veml603x_chip veml6030_chip = { + .name = "veml6030", + .info = &veml6030_info, + .info_no_irq = &veml6030_info_no_irq, + .hw_init = veml6030_hw_init, + .set_als_gain = veml6030_set_als_gain, + .get_als_gain = veml6030_get_als_gain, +}; + +static const struct veml603x_chip veml6035_chip = { + .name = "veml6035", + .info = &veml6035_info, + .info_no_irq = &veml6035_info_no_irq, + .hw_init = veml6035_hw_init, + .set_als_gain = veml6035_set_als_gain, + .get_als_gain = veml6035_get_als_gain, +}; + static const struct of_device_id veml6030_of_match[] = { - { .compatible = "vishay,veml6030" }, + { + .compatible = "vishay,veml6030", + .data = &veml6030_chip, + }, + { + .compatible = "vishay,veml6035", + .data = &veml6035_chip, + }, { } }; MODULE_DEVICE_TABLE(of, veml6030_of_match); static const struct i2c_device_id veml6030_id[] = { - { "veml6030" }, + { "veml6030", (kernel_ulong_t)&veml6030_chip}, + { "veml6035", (kernel_ulong_t)&veml6035_chip}, { } }; MODULE_DEVICE_TABLE(i2c, veml6030_id);
The veml6035 is an ALS that shares most of its functionality with the veml6030, which allows for some code recycling. Some chip-specific properties differ and dedicated functions to get and set the sensor gain as well as its initialization are required. Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> --- drivers/iio/light/veml6030.c | 300 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 273 insertions(+), 27 deletions(-)