Hi Javier, after being back from my extended Christmas holidays, I took a look onto your changes: On Wednesday, 3 January 2024, 13:08:53 CET, Javier Carrasco wrote: > The AMS AS7331 is a UV light sensor with three channels: UVA, UVB and > UVC (also known as deep UV and referenced as DUV in the iio core). > Its internal structure and forming blocks are practically identical to > the ones the AS73211 contains: API, internal DAC, I2C interface and > registers, measurement modes, number of channels and pinout. > > The only difference between them is the photodiodes used to acquire > light, which means that only some modifications are required to add > support for the AS7331 in the existing driver. > > The temperature channel is identical for both devices and only the > channel modifiers of the IIO_INTENSITY channels need to account for the > device type. > > The scale values have been obtained from the chapter "7.5 Transfer > Function" of the official datasheet[1] for the configuration chosen as > basis (Nclk = 1024 and GAIN = 1). Those values keep the units from the > datasheet (nW/cm^2), as opposed to the units used for the AS73211 > (nW/m^2). > > Add a new device-specific data structure to account for the device > differences: channel types and scale of LSB per channel. > > [1] https://ams.com/documents/20143/9106314/AS7331_DS001047_4-00.pdf > > Tested-by: Christian Eggers <ceggers@xxxxxxx> > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> > --- > drivers/iio/light/Kconfig | 5 +- > drivers/iio/light/as73211.c | 141 ++++++++++++++++++++++++++++++++++++-------- > 2 files changed, 118 insertions(+), 28 deletions(-) > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index 143003232d1c..fd5a9879a582 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -87,13 +87,14 @@ config APDS9960 > module will be called apds9960 > > config AS73211 > - tristate "AMS AS73211 XYZ color sensor" > + tristate "AMS AS73211 XYZ color sensor and AMS AS7331 UV sensor" > depends on I2C > select IIO_BUFFER > select IIO_TRIGGERED_BUFFER > help > If you say yes here you get support for the AMS AS73211 > - JENCOLOR(R) Digital XYZ Sensor. > + JENCOLOR(R) Digital XYZ and the AMS AS7331 UVA, UVB and UVC > + ultraviolet sensors. > > For triggered measurements, you will need an additional trigger driver > like IIO_HRTIMER_TRIGGER or IIO_SYSFS_TRIGGER. > diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c > index b4c6f389a292..44daf816ae57 100644 > --- a/drivers/iio/light/as73211.c > +++ b/drivers/iio/light/as73211.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* > - * Support for AMS AS73211 JENCOLOR(R) Digital XYZ Sensor > + * Support for AMS AS73211 JENCOLOR(R) Digital XYZ Sensor and AMS AS7331 > + * UVA, UVB and UVC (DUV) Ultraviolet Sensor > * > * Author: Christian Eggers <ceggers@xxxxxxx> > * > @@ -9,7 +10,9 @@ > * Color light sensor with 16-bit channels for x, y, z and temperature); > * 7-bit I2C slave address 0x74 .. 0x77. > * > - * Datasheet: https://ams.com/documents/20143/36005/AS73211_DS000556_3-01.pdf > + * Datasheets: > + * AS73211: https://ams.com/documents/20143/36005/AS73211_DS000556_3-01.pdf > + * AS7331: https://ams.com/documents/20143/9106314/AS7331_DS001047_4-00.pdf > */ > > #include <linux/bitfield.h> > @@ -84,6 +87,20 @@ static const int as73211_hardwaregain_avail[] = { > 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, > }; > > +struct as73211_data; > + > +/** > + * struct spec_dev_data - device-specific data > + * @intensity_scale: Function to retrieve intensity scale values. > + * @channel: Device channels. > + * @num_channels: Number of channels of the device. > + */ > +struct spec_dev_data { I would call it as73211_spec_dev_data (is the C++ One Definition Rule relevant for the kernel?) > + int (*intensity_scale)(struct as73211_data *data, int chan, int *val, int *val2); > + struct iio_chan_spec const *channel; s/channel/channels/ > + int num_channels; > +}; > + > /** > * struct as73211_data - Instance data for one AS73211 > * @client: I2C client. > @@ -94,6 +111,7 @@ static const int as73211_hardwaregain_avail[] = { > * @mutex: Keeps cached registers in sync with the device. > * @completion: Completion to wait for interrupt. > * @int_time_avail: Available integration times (depend on sampling frequency). > + * @spec_dev: device-specific configuration. > */ > struct as73211_data { > struct i2c_client *client; > @@ -104,6 +122,7 @@ struct as73211_data { > struct mutex mutex; > struct completion completion; > int int_time_avail[AS73211_SAMPLE_TIME_NUM * 2]; > + const struct spec_dev_data *spec_dev; > }; > > #define AS73211_COLOR_CHANNEL(_color, _si, _addr) { \ > @@ -138,6 +157,10 @@ struct as73211_data { > #define AS73211_SCALE_Y 298384270 /* nW/m^2 */ > #define AS73211_SCALE_Z 160241927 /* nW/m^2 */ > > +#define AS7331_SCALE_UVA 340000 /* nW/cm^2 */ > +#define AS7331_SCALE_UVB 378000 /* nW/cm^2 */ > +#define AS7331_SCALE_UVC 166000 /* nW/cm^2 */ > + > /* Channel order MUST match devices result register order */ > #define AS73211_SCAN_INDEX_TEMP 0 > #define AS73211_SCAN_INDEX_X 1 > @@ -176,6 +199,28 @@ static const struct iio_chan_spec as73211_channels[] = { > IIO_CHAN_SOFT_TIMESTAMP(AS73211_SCAN_INDEX_TS), > }; > > +static const struct iio_chan_spec as7331_channels[] = { > + { > + .type = IIO_TEMP, > + .info_mask_separate = > + BIT(IIO_CHAN_INFO_RAW) | > + BIT(IIO_CHAN_INFO_OFFSET) | > + BIT(IIO_CHAN_INFO_SCALE), > + .address = AS73211_OUT_TEMP, > + .scan_index = AS73211_SCAN_INDEX_TEMP, > + .scan_type = { > + .sign = 'u', > + .realbits = 16, > + .storagebits = 16, > + .endianness = IIO_LE, > + } > + }, > + AS73211_COLOR_CHANNEL(LIGHT_UVA, AS73211_SCAN_INDEX_X, AS73211_OUT_MRES1), > + AS73211_COLOR_CHANNEL(LIGHT_UVB, AS73211_SCAN_INDEX_Y, AS73211_OUT_MRES2), > + AS73211_COLOR_CHANNEL(LIGHT_DUV, AS73211_SCAN_INDEX_Z, AS73211_OUT_MRES3), > + IIO_CHAN_SOFT_TIMESTAMP(AS73211_SCAN_INDEX_TS), > +}; > + > static unsigned int as73211_integration_time_1024cyc(struct as73211_data *data) > { > /* > @@ -316,6 +361,48 @@ static int as73211_req_data(struct as73211_data *data) > return 0; > } > > +static int as73211_intensity_scale(struct as73211_data *data, int chan, > + int *val, int *val2) > +{ > + switch (chan) { > + case IIO_MOD_X: > + *val = AS73211_SCALE_X; > + break; > + case IIO_MOD_Y: > + *val = AS73211_SCALE_Y; > + break; > + case IIO_MOD_Z: > + *val = AS73211_SCALE_Z; > + break; > + default: > + return -EINVAL; > + } > + *val2 = as73211_integration_time_1024cyc(data) * as73211_gain(data); > + > + return IIO_VAL_FRACTIONAL; > +} > + > +static int as7331_intensity_scale(struct as73211_data *data, int chan, > + int *val, int *val2) > +{ > + switch (chan) { > + case IIO_MOD_LIGHT_UVA: > + *val = AS7331_SCALE_UVA; > + break; > + case IIO_MOD_LIGHT_UVB: > + *val = AS7331_SCALE_UVB; > + break; > + case IIO_MOD_LIGHT_DUV: > + *val = AS7331_SCALE_UVC; > + break; > + default: > + return -EINVAL; > + } > + *val2 = as73211_integration_time_1024cyc(data) * as73211_gain(data); > + > + return IIO_VAL_FRACTIONAL; > +} > + > static int as73211_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, > int *val, int *val2, long mask) > { > @@ -355,29 +442,13 @@ static int as73211_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec cons > *val2 = AS73211_SCALE_TEMP_MICRO; > return IIO_VAL_INT_PLUS_MICRO; > > - case IIO_INTENSITY: { > - > - switch (chan->channel2) { > - case IIO_MOD_X: > - *val = AS73211_SCALE_X; > - break; > - case IIO_MOD_Y: > - *val = AS73211_SCALE_Y; > - break; > - case IIO_MOD_Z: > - *val = AS73211_SCALE_Z; > - break; > - default: > - return -EINVAL; > - } > - *val2 = as73211_integration_time_1024cyc(data) * > - as73211_gain(data); > - > - return IIO_VAL_FRACTIONAL; > + case IIO_INTENSITY: > + return data->spec_dev->intensity_scale(data, chan->channel2, > + val, val2); > > default: > return -EINVAL; > - }} > + } > > case IIO_CHAN_INFO_SAMP_FREQ: > /* f_samp is configured in CREG3 in powers of 2 (x 1.024 MHz) */ > @@ -675,13 +746,17 @@ static int as73211_probe(struct i2c_client *client) > i2c_set_clientdata(client, indio_dev); > data->client = client; > > + data->spec_dev = i2c_get_match_data(client); > + if (!data->spec_dev) > + return -EINVAL; > + > mutex_init(&data->mutex); > init_completion(&data->completion); > > indio_dev->info = &as73211_info; > indio_dev->name = AS73211_DRV_NAME; > - indio_dev->channels = as73211_channels; > - indio_dev->num_channels = ARRAY_SIZE(as73211_channels); > + indio_dev->channels = data->spec_dev->channel; > + indio_dev->num_channels = data->spec_dev->num_channels; > indio_dev->modes = INDIO_DIRECT_MODE; > > ret = i2c_smbus_read_byte_data(data->client, AS73211_REG_OSR); > @@ -771,14 +846,28 @@ static int as73211_resume(struct device *dev) > static DEFINE_SIMPLE_DEV_PM_OPS(as73211_pm_ops, as73211_suspend, > as73211_resume); > > +static const struct spec_dev_data as73211_spec = { > + .intensity_scale = as73211_intensity_scale, > + .channel = as73211_channels, > + .num_channels = ARRAY_SIZE(as73211_channels), > +}; > + > +static const struct spec_dev_data as7331_spec = { > + .intensity_scale = as7331_intensity_scale, > + .channel = as7331_channels, > + .num_channels = ARRAY_SIZE(as7331_channels), > +}; > + > static const struct of_device_id as73211_of_match[] = { > - { .compatible = "ams,as73211" }, > + { .compatible = "ams,as73211", &as73211_spec }, > + { .compatible = "ams,as7331", &as7331_spec }, > { } > }; > MODULE_DEVICE_TABLE(of, as73211_of_match); > > static const struct i2c_device_id as73211_id[] = { > - { "as73211", 0 }, > + { "as73211", (kernel_ulong_t)&as73211_spec }, > + { "as7331", (kernel_ulong_t)&as7331_spec }, > { } > }; > MODULE_DEVICE_TABLE(i2c, as73211_id); > > Tested-by: Christian Eggers <ceggers@xxxxxxx>
