Message ID | 20220711112900.61363-3-shreeya.patel@collabora.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Add LTRF216A Driver | expand |
On Mon, Jul 11, 2022 at 1:30 PM Shreeya Patel <shreeya.patel@collabora.com> wrote: > > From: Zhigang Shi <Zhigang.Shi@liteon.com> > > Add initial support for ltrf216a ambient light sensor. > > Datasheet: https://gitlab.steamos.cloud/shreeya/iio/-/blob/main/LTRF216A.pdf > Co-developed-by: Shreeya Patel <shreeya.patel@collabora.com> > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> > Signed-off-by: Zhigang Shi <Zhigang.Shi@liteon.com> Submitter's SoB always has to be last among SoBs in the proposed change. https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by ... > +static int ltrf216a_set_power_state(struct ltrf216a_data *data, bool on) > +{ > + struct device *dev = &data->client->dev; > + int ret; > + > + if (on) { > + ret = pm_runtime_resume_and_get(dev); > + if (ret < 0) { > + dev_err(dev, "Failed to resume runtime PM: %d\n", ret); > + return ret; > + } > + Unneeded blank line. > + } else { > + pm_runtime_mark_last_busy(dev); > + ret = pm_runtime_put_autosuspend(dev); > + } > + > + return ret; > +} ... > + ret = regmap_read_poll_timeout(data->regmap, LTRF216A_MAIN_STATUS, > + val, val & LTRF216A_ALS_DATA_STATUS, > + LTRF216A_ALS_READ_DATA_DELAY_US, > + LTRF216A_ALS_READ_DATA_DELAY_US * 50); > + if (ret) { > + dev_err(dev, "Timed out waiting for valid data from LTRF216A_MAIN_STATUS reg: %d\n", > + ret); THe message is a bit misleading. The loop might be broken by the I/O error. > + return ret; > + } > + > + ret = regmap_bulk_read(data->regmap, addr, buf, sizeof(buf)); > + if (ret < 0) { > + dev_err(dev, "Error reading measurement data: %d\n", ret); > + return ret; > + } ... > +static const struct regmap_config ltrf216a_regmap_config = { > + .name = LTRF216A_DRV_NAME, > + .reg_bits = 8, > + .val_bits = 8, > + .max_register = LTRF216A_MAX_REG, Why do you use regmap locking? What for? > +}; ... > + data->regmap = devm_regmap_init_i2c(client, <rf216a_regmap_config); > + if (IS_ERR(data->regmap)) { > + dev_err(&client->dev, "Regmap initialization failed.\n"); > + return PTR_ERR(data->regmap); return dev_err_probe(...); > + } ... > + ret = devm_pm_runtime_enable(&client->dev); > + if (ret < 0) { > + dev_err_probe(&client->dev, ret, "Failed to enable runtime PM\n"); > + return ret; Ditto. > + } ... > + ret = ltrf216a_init(indio_dev); > + if (ret) { > + dev_err_probe(&client->dev, ret, "Failed to enable the sensor\n"); > + return ret; Ditto. > + } ... > + if (ret < 0) For all these ' < 0', please explain what positive return value means there, if any, and why it's being ignored. ... > +static const struct i2c_device_id ltrf216a_id[] = { > + { LTRF216A_DRV_NAME, 0 }, Please, use the string literal directly since it's kinda an ABI, defining above for potential changes is not a good idea. Also you may drop the ', 0' part. > + {} > +}; ... > +static struct i2c_driver ltrf216a_driver = { > + .driver = { > + .name = LTRF216A_DRV_NAME, Ditto. > + .pm = pm_ptr(<rf216a_pm_ops), > + .of_match_table = ltrf216a_of_match, > + }, > + .probe_new = ltrf216a_probe, > + .id_table = ltrf216a_id, > +};
On 11/07/22 18:36, Andy Shevchenko wrote: > On Mon, Jul 11, 2022 at 1:30 PM Shreeya Patel > <shreeya.patel@collabora.com> wrote: >> From: Zhigang Shi <Zhigang.Shi@liteon.com> >> >> Add initial support for ltrf216a ambient light sensor. >> >> Datasheet: https://gitlab.steamos.cloud/shreeya/iio/-/blob/main/LTRF216A.pdf >> Co-developed-by: Shreeya Patel <shreeya.patel@collabora.com> >> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> >> Signed-off-by: Zhigang Shi <Zhigang.Shi@liteon.com> > Submitter's SoB always has to be last among SoBs in the proposed change. > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin > https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by > > ... > >> +static int ltrf216a_set_power_state(struct ltrf216a_data *data, bool on) >> +{ >> + struct device *dev = &data->client->dev; >> + int ret; >> + >> + if (on) { >> + ret = pm_runtime_resume_and_get(dev); >> + if (ret < 0) { >> + dev_err(dev, "Failed to resume runtime PM: %d\n", ret); >> + return ret; >> + } >> + > Unneeded blank line. > >> + } else { >> + pm_runtime_mark_last_busy(dev); >> + ret = pm_runtime_put_autosuspend(dev); >> + } >> + >> + return ret; >> +} > ... > >> + ret = regmap_read_poll_timeout(data->regmap, LTRF216A_MAIN_STATUS, >> + val, val & LTRF216A_ALS_DATA_STATUS, >> + LTRF216A_ALS_READ_DATA_DELAY_US, >> + LTRF216A_ALS_READ_DATA_DELAY_US * 50); >> + if (ret) { >> + dev_err(dev, "Timed out waiting for valid data from LTRF216A_MAIN_STATUS reg: %d\n", >> + ret); > THe message is a bit misleading. The loop might be broken by the I/O error. > >> + return ret; >> + } >> + >> + ret = regmap_bulk_read(data->regmap, addr, buf, sizeof(buf)); >> + if (ret < 0) { >> + dev_err(dev, "Error reading measurement data: %d\n", ret); >> + return ret; >> + } > ... > >> +static const struct regmap_config ltrf216a_regmap_config = { >> + .name = LTRF216A_DRV_NAME, >> + .reg_bits = 8, >> + .val_bits = 8, >> + .max_register = LTRF216A_MAX_REG, > Why do you use regmap locking? What for? > Hi Andy, Why do we want to skip the internal locking if it doesn't bring any benefits? >> +}; > ... > >> + data->regmap = devm_regmap_init_i2c(client, <rf216a_regmap_config); >> + if (IS_ERR(data->regmap)) { >> + dev_err(&client->dev, "Regmap initialization failed.\n"); >> + return PTR_ERR(data->regmap); > return dev_err_probe(...); > >> + } > ... > >> + ret = devm_pm_runtime_enable(&client->dev); >> + if (ret < 0) { >> + dev_err_probe(&client->dev, ret, "Failed to enable runtime PM\n"); >> + return ret; > Ditto. > >> + } > ... > >> + ret = ltrf216a_init(indio_dev); >> + if (ret) { >> + dev_err_probe(&client->dev, ret, "Failed to enable the sensor\n"); >> + return ret; > Ditto. > >> + } > ... > >> + if (ret < 0) > For all these ' < 0', please explain what positive return value means > there, if any, and why it's being ignored. > > ... > >> +static const struct i2c_device_id ltrf216a_id[] = { >> + { LTRF216A_DRV_NAME, 0 }, > Please, use the string literal directly since it's kinda an ABI, > defining above for potential changes is not a good idea. Also you may > drop the ', 0' part. > >> + {} >> +}; > ... > >> +static struct i2c_driver ltrf216a_driver = { >> + .driver = { >> + .name = LTRF216A_DRV_NAME, > Ditto. > >> + .pm = pm_ptr(<rf216a_pm_ops), >> + .of_match_table = ltrf216a_of_match, >> + }, >> + .probe_new = ltrf216a_probe, >> + .id_table = ltrf216a_id, >> +};
On Mon, Jul 11, 2022 at 3:39 PM Shreeya Patel <shreeya.patel@collabora.com> wrote: > On 11/07/22 18:36, Andy Shevchenko wrote: > > On Mon, Jul 11, 2022 at 1:30 PM Shreeya Patel Please, remove unneeded context when replying! ... > >> +static const struct regmap_config ltrf216a_regmap_config = { > >> + .name = LTRF216A_DRV_NAME, > >> + .reg_bits = 8, > >> + .val_bits = 8, > >> + .max_register = LTRF216A_MAX_REG, > > Why do you use regmap locking? What for? > > Why do we want to skip the internal locking if it doesn't bring any > benefits? Can you elaborate on the "no benefits" part, please?
On 7/11/22 16:41, Andy Shevchenko wrote: > On Mon, Jul 11, 2022 at 3:39 PM Shreeya Patel > <shreeya.patel@collabora.com> wrote: >> On 11/07/22 18:36, Andy Shevchenko wrote: >>> On Mon, Jul 11, 2022 at 1:30 PM Shreeya Patel > > Please, remove unneeded context when replying! > > ... > >>>> +static const struct regmap_config ltrf216a_regmap_config = { >>>> + .name = LTRF216A_DRV_NAME, >>>> + .reg_bits = 8, >>>> + .val_bits = 8, >>>> + .max_register = LTRF216A_MAX_REG, >>> Why do you use regmap locking? What for? >> >> Why do we want to skip the internal locking if it doesn't bring any >> benefits? > > Can you elaborate on the "no benefits" part, please? Since the regmap's lock will never be contended, thus it's free to keep using it. If later on we will need to change the driver's code such that the lock will become needed, then we won't need to bother with re-enabling it. The comment to the driver's mutex states clearly that it's intended to protect the cached value. Hence what is point in disabling the regmap's lock? There are very few drivers that disable the regmap's lock and most of them do that for the good reason.
On Mon, Jul 11, 2022 at 4:04 PM Dmitry Osipenko <dmitry.osipenko@collabora.com> wrote: > On 7/11/22 16:41, Andy Shevchenko wrote: > > On Mon, Jul 11, 2022 at 3:39 PM Shreeya Patel > > <shreeya.patel@collabora.com> wrote: > >> On 11/07/22 18:36, Andy Shevchenko wrote: > >>> On Mon, Jul 11, 2022 at 1:30 PM Shreeya Patel ... > >>>> +static const struct regmap_config ltrf216a_regmap_config = { > >>>> + .name = LTRF216A_DRV_NAME, > >>>> + .reg_bits = 8, > >>>> + .val_bits = 8, > >>>> + .max_register = LTRF216A_MAX_REG, > >>> Why do you use regmap locking? What for? > >> > >> Why do we want to skip the internal locking if it doesn't bring any > >> benefits? > > > > Can you elaborate on the "no benefits" part, please? > > Since the regmap's lock will never be contended, thus it's free to keep I'm skeptical about "free" here. My concerns are: 1) grosser code base; 2) slower code flow (even nop takes time to process). > using it. If later on we will need to change the driver's code such that > the lock will become needed, then we won't need to bother with > re-enabling it. The comment to the driver's mutex states clearly that > it's intended to protect the cached value. > > Hence what is point in disabling the regmap's lock? There are very few > drivers that disable the regmap's lock and most of them do that for the > good reason. Most of the drivers that have its own lock _and_ regmap lock took the locking scheme wrong. It is 101 when writing a driver to have a clear picture of what lock protects what data (or I/O). Even if lock is _almost_ free, it's still required to provide understanding of how each of the locks is being used in the code. That said, the main point of my review comment is to make the author think about it, or just elaborate if it has been thought through already.
On 7/11/22 17:22, Andy Shevchenko wrote: > On Mon, Jul 11, 2022 at 4:04 PM Dmitry Osipenko > <dmitry.osipenko@collabora.com> wrote: >> On 7/11/22 16:41, Andy Shevchenko wrote: >>> On Mon, Jul 11, 2022 at 3:39 PM Shreeya Patel >>> <shreeya.patel@collabora.com> wrote: >>>> On 11/07/22 18:36, Andy Shevchenko wrote: >>>>> On Mon, Jul 11, 2022 at 1:30 PM Shreeya Patel > > ... > >>>>>> +static const struct regmap_config ltrf216a_regmap_config = { >>>>>> + .name = LTRF216A_DRV_NAME, >>>>>> + .reg_bits = 8, >>>>>> + .val_bits = 8, >>>>>> + .max_register = LTRF216A_MAX_REG, >>>>> Why do you use regmap locking? What for? >>>> >>>> Why do we want to skip the internal locking if it doesn't bring any >>>> benefits? >>> >>> Can you elaborate on the "no benefits" part, please? >> >> Since the regmap's lock will never be contended, thus it's free to keep > > I'm skeptical about "free" here. My concerns are: > 1) grosser code base; > 2) slower code flow (even nop takes time to process). > >> using it. If later on we will need to change the driver's code such that >> the lock will become needed, then we won't need to bother with >> re-enabling it. The comment to the driver's mutex states clearly that >> it's intended to protect the cached value. >> >> Hence what is point in disabling the regmap's lock? There are very few >> drivers that disable the regmap's lock and most of them do that for the >> good reason. > > Most of the drivers that have its own lock _and_ regmap lock took the > locking scheme wrong. It is 101 when writing a driver to have a clear > picture of what lock protects what data (or I/O). > > Even if lock is _almost_ free, it's still required to provide > understanding of how each of the locks is being used in the code. > > That said, the main point of my review comment is to make the author > think about it, or just elaborate if it has been thought through > already. Alright, thank you for clarifying yours point! I helped Shreeya a tad with preparing the v7 and disabling the regmap's lock wasn't under question. Since this driver isn't about performance and a nanosecond improvement isn't worthwhile in comparison to the the I2C transfer time, should be cleaner to keep the regmap's lock in place, IMO. I'll let Shreeya to answer next time, sorry for jumping in.
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig index 8537e88f02e3..7cf6e8490123 100644 --- a/drivers/iio/light/Kconfig +++ b/drivers/iio/light/Kconfig @@ -331,6 +331,17 @@ config LTR501 This driver can also be built as a module. If so, the module will be called ltr501. +config LTRF216A + tristate "Liteon LTRF216A Light Sensor" + depends on I2C + select REGMAP_I2C + help + If you say Y or M here, you get support for Liteon LTRF216A + Ambient Light Sensor. + + If built as a dynamically linked module, it will be called + ltrf216a. + config LV0104CS tristate "LV0104CS Ambient Light Sensor" depends on I2C diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile index d10912faf964..6f23817fae6f 100644 --- a/drivers/iio/light/Makefile +++ b/drivers/iio/light/Makefile @@ -31,6 +31,7 @@ obj-$(CONFIG_ISL29125) += isl29125.o obj-$(CONFIG_JSA1212) += jsa1212.o obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o obj-$(CONFIG_LTR501) += ltr501.o +obj-$(CONFIG_LTRF216A) += ltrf216a.o obj-$(CONFIG_LV0104CS) += lv0104cs.o obj-$(CONFIG_MAX44000) += max44000.o obj-$(CONFIG_MAX44009) += max44009.o diff --git a/drivers/iio/light/ltrf216a.c b/drivers/iio/light/ltrf216a.c new file mode 100644 index 000000000000..085d0ff229b8 --- /dev/null +++ b/drivers/iio/light/ltrf216a.c @@ -0,0 +1,423 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * LTRF216A Ambient Light Sensor + * + * Copyright (C) 2021 Lite-On Technology Corp (Singapore) + * Author: Shi Zhigang <Zhigang.Shi@liteon.com> + * + * IIO driver for LTRF216A (7-bit I2C slave address 0x53). + */ + +#include <linux/bitfield.h> +#include <linux/bits.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/iopoll.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/pm.h> +#include <linux/pm_runtime.h> +#include <linux/regmap.h> + +#include <linux/iio/iio.h> + +#include <asm/unaligned.h> + +#define LTRF216A_DRV_NAME "ltrf216a" + +#define LTRF216A_ALS_DATA_STATUS BIT(3) +#define LTRF216A_ALS_ENABLE_MASK BIT(1) +#define LTRF216A_MAIN_CTRL 0x00 +#define LTRF216A_ALS_MEAS_RES 0x04 +#define LTRF216A_MAIN_STATUS 0x07 +#define LTRF216A_ALS_DATA_0 0x0D +#define LTRF216A_ALS_RESET 0x10 +#define LTRF216A_MAX_REG 0x26 +#define LTRF216A_ALS_READ_DATA_DELAY_US 20000 + +static const int ltrf216a_int_time_available[][2] = { + {0, 400000}, + {0, 200000}, + {0, 100000}, + {0, 50000}, + {0, 25000}, +}; + +static const int ltrf216a_int_time_reg[][2] = { + {400, 0x03}, + {200, 0x13}, + {100, 0x22}, + {50, 0x31}, + {25, 0x40}, +}; + +/* + * Window Factor is needed when the device is under Window glass + * with coated tinted ink. This is to compensate for the light loss + * due to the lower transmission rate of the window glass and helps + * in calculating lux. + */ +#define LTRF216A_WIN_FAC 1 + +struct ltrf216a_data { + struct regmap *regmap; + struct i2c_client *client; + u32 int_time; + u16 int_time_fac; + u8 als_gain_fac; + /* + * Ensure cached value of integration time is consistent + * with hardware setting and remains constant during a + * measurement of Lux. + */ + struct mutex lock; +}; + +static const struct iio_chan_spec ltrf216a_channels[] = { + { + .type = IIO_LIGHT, + .info_mask_separate = + BIT(IIO_CHAN_INFO_PROCESSED) | + BIT(IIO_CHAN_INFO_INT_TIME), + .info_mask_separate_available = + BIT(IIO_CHAN_INFO_INT_TIME), + }, +}; + +static int ltrf216a_init(struct iio_dev *indio_dev) +{ + struct ltrf216a_data *data = iio_priv(indio_dev); + struct device *dev = &data->client->dev; + int ret; + + /* enable sensor */ + ret = regmap_set_bits(data->regmap, LTRF216A_MAIN_CTRL, LTRF216A_ALS_ENABLE_MASK); + if (ret < 0) + dev_err(dev, "Error writing to LTRF216A_MAIN_CTRL while enabling the sensor: %d\n", + ret); + + return ret; +} + +static void ltrf216a_reset(struct iio_dev *indio_dev) +{ + struct ltrf216a_data *data = iio_priv(indio_dev); + + /* reset sensor, chip fails to respond to this, so ignore any errors */ + regmap_write(data->regmap, LTRF216A_MAIN_CTRL, LTRF216A_ALS_RESET); + + /* reset time */ + usleep_range(1000, 2000); +} + +static int ltrf216a_disable(struct iio_dev *indio_dev) +{ + struct ltrf216a_data *data = iio_priv(indio_dev); + struct device *dev = &data->client->dev; + int ret; + + ret = regmap_write(data->regmap, LTRF216A_MAIN_CTRL, 0); + if (ret < 0) + dev_err(dev, "Error writing to LTRF216A_MAIN_CTRL while disabling the sensor: %d\n", + ret); + + return ret; +} + +static void als_ltrf216a_disable(void *data) +{ + struct iio_dev *indio_dev = data; + + ltrf216a_disable(indio_dev); +} + +static int ltrf216a_set_int_time(struct ltrf216a_data *data, int itime) +{ + struct device *dev = &data->client->dev; + unsigned int i; + u8 reg_val; + int ret; + + for (i = 0; i < ARRAY_SIZE(ltrf216a_int_time_available); i++) { + if (ltrf216a_int_time_available[i][1] == itime) + break; + } + if (i == ARRAY_SIZE(ltrf216a_int_time_available)) + return -EINVAL; + + reg_val = ltrf216a_int_time_reg[i][1]; + + ret = regmap_write(data->regmap, LTRF216A_ALS_MEAS_RES, reg_val); + if (ret < 0) { + dev_err(dev, "Error writing to LTRF216A_ALS_MEAS_RES: %d\n", ret); + return ret; + } + + data->int_time_fac = ltrf216a_int_time_reg[i][0]; + data->int_time = itime; + + return 0; +} + +static int ltrf216a_get_int_time(struct ltrf216a_data *data, int *val, int *val2) +{ + *val = 0; + *val2 = data->int_time; + return IIO_VAL_INT_PLUS_MICRO; +} + +static int ltrf216a_set_power_state(struct ltrf216a_data *data, bool on) +{ + struct device *dev = &data->client->dev; + int ret; + + if (on) { + ret = pm_runtime_resume_and_get(dev); + if (ret < 0) { + dev_err(dev, "Failed to resume runtime PM: %d\n", ret); + return ret; + } + + } else { + pm_runtime_mark_last_busy(dev); + ret = pm_runtime_put_autosuspend(dev); + } + + return ret; +} + +static int ltrf216a_read_data(struct ltrf216a_data *data, u8 addr) +{ + struct device *dev = &data->client->dev; + int ret, val; + u8 buf[3]; + + ret = regmap_read_poll_timeout(data->regmap, LTRF216A_MAIN_STATUS, + val, val & LTRF216A_ALS_DATA_STATUS, + LTRF216A_ALS_READ_DATA_DELAY_US, + LTRF216A_ALS_READ_DATA_DELAY_US * 50); + if (ret) { + dev_err(dev, "Timed out waiting for valid data from LTRF216A_MAIN_STATUS reg: %d\n", + ret); + return ret; + } + + ret = regmap_bulk_read(data->regmap, addr, buf, sizeof(buf)); + if (ret < 0) { + dev_err(dev, "Error reading measurement data: %d\n", ret); + return ret; + } + + return get_unaligned_le24(&buf[0]); +} + +static int ltrf216a_get_lux(struct ltrf216a_data *data) +{ + int ret, greendata; + u64 lux, div; + + ret = ltrf216a_set_power_state(data, true); + if (ret < 0) + return ret; + + greendata = ltrf216a_read_data(data, LTRF216A_ALS_DATA_0); + if (greendata < 0) + return greendata; + + ret = ltrf216a_set_power_state(data, false); + if (ret < 0) + return ret; + + lux = greendata * 45 * LTRF216A_WIN_FAC * 100; + div = data->als_gain_fac * data->int_time_fac * 100; + + return div_u64(lux, div); +} + +static int ltrf216a_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int *val, + int *val2, long mask) +{ + struct ltrf216a_data *data = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_PROCESSED: + mutex_lock(&data->lock); + ret = ltrf216a_get_lux(data); + mutex_unlock(&data->lock); + if (ret < 0) + return ret; + *val = ret; + return IIO_VAL_INT; + case IIO_CHAN_INFO_INT_TIME: + mutex_lock(&data->lock); + ret = ltrf216a_get_int_time(data, val, val2); + mutex_unlock(&data->lock); + return ret; + default: + return -EINVAL; + } +} + +static int ltrf216a_write_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, int val, + int val2, long mask) +{ + struct ltrf216a_data *data = iio_priv(indio_dev); + int ret; + + switch (mask) { + case IIO_CHAN_INFO_INT_TIME: + if (val != 0) + return -EINVAL; + mutex_lock(&data->lock); + ret = ltrf216a_set_int_time(data, val2); + mutex_unlock(&data->lock); + return ret; + default: + return -EINVAL; + } +} + +static int ltrf216a_read_available(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: + *length = ARRAY_SIZE(ltrf216a_int_time_available) * 2; + *vals = (const int *)ltrf216a_int_time_available; + *type = IIO_VAL_INT_PLUS_MICRO; + return IIO_AVAIL_LIST; + default: + return -EINVAL; + } +} + +static const struct iio_info ltrf216a_info = { + .read_raw = ltrf216a_read_raw, + .write_raw = ltrf216a_write_raw, + .read_avail = ltrf216a_read_available, +}; + +static const struct regmap_config ltrf216a_regmap_config = { + .name = LTRF216A_DRV_NAME, + .reg_bits = 8, + .val_bits = 8, + .max_register = LTRF216A_MAX_REG, +}; + +static int ltrf216a_probe(struct i2c_client *client) +{ + struct ltrf216a_data *data; + struct iio_dev *indio_dev; + int ret; + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + + data->regmap = devm_regmap_init_i2c(client, <rf216a_regmap_config); + if (IS_ERR(data->regmap)) { + dev_err(&client->dev, "Regmap initialization failed.\n"); + return PTR_ERR(data->regmap); + } + + i2c_set_clientdata(client, indio_dev); + data->client = client; + + mutex_init(&data->lock); + + indio_dev->info = <rf216a_info; + indio_dev->name = LTRF216A_DRV_NAME; + indio_dev->channels = ltrf216a_channels; + indio_dev->num_channels = ARRAY_SIZE(ltrf216a_channels); + indio_dev->modes = INDIO_DIRECT_MODE; + + /* reset sensor, chip fails to respond to this, so ignore any errors */ + ltrf216a_reset(indio_dev); + + ret = devm_pm_runtime_enable(&client->dev); + if (ret < 0) { + dev_err_probe(&client->dev, ret, "Failed to enable runtime PM\n"); + return ret; + } + pm_runtime_set_autosuspend_delay(&client->dev, 1000); + pm_runtime_use_autosuspend(&client->dev); + + if (!IS_ENABLED(CONFIG_PM)) { + ret = ltrf216a_init(indio_dev); + if (ret) { + dev_err_probe(&client->dev, ret, "Failed to enable the sensor\n"); + return ret; + } + } + + data->int_time = 100000; + data->int_time_fac = 100; + data->als_gain_fac = 3; + + ret = devm_add_action_or_reset(&client->dev, als_ltrf216a_disable, indio_dev); + if (ret < 0) + return ret; + + return devm_iio_device_register(&client->dev, indio_dev); +} + +static int ltrf216a_runtime_suspend(struct device *dev) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); + + return ltrf216a_disable(indio_dev); +} + +static int ltrf216a_runtime_resume(struct device *dev) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); + int ret; + + ret = ltrf216a_init(indio_dev); + + /* Sleep for one integration cycle after resuming the device. */ + msleep(ltrf216a_int_time_reg[0][0]); + + return ret; +} + +static DEFINE_RUNTIME_DEV_PM_OPS(ltrf216a_pm_ops, ltrf216a_runtime_suspend, + ltrf216a_runtime_resume, NULL); + +static const struct i2c_device_id ltrf216a_id[] = { + { LTRF216A_DRV_NAME, 0 }, + {} +}; +MODULE_DEVICE_TABLE(i2c, ltrf216a_id); + +static const struct of_device_id ltrf216a_of_match[] = { + { .compatible = "liteon,ltrf216a", }, + { .compatible = "ltr,ltrf216a", }, + {} +}; +MODULE_DEVICE_TABLE(of, ltrf216a_of_match); + +static struct i2c_driver ltrf216a_driver = { + .driver = { + .name = LTRF216A_DRV_NAME, + .pm = pm_ptr(<rf216a_pm_ops), + .of_match_table = ltrf216a_of_match, + }, + .probe_new = ltrf216a_probe, + .id_table = ltrf216a_id, +}; +module_i2c_driver(ltrf216a_driver); + +MODULE_AUTHOR("Shi Zhigang <Zhigang.Shi@liteon.com>"); +MODULE_AUTHOR("Shreeya Patel <shreeya.patel@collabora.com>"); +MODULE_DESCRIPTION("LTRF216A ambient light sensor driver"); +MODULE_LICENSE("GPL");