Hi Jonathan, Just one comment inline related to your previous review. On 03/05/22 20:13, Shreeya Patel wrote:
From: Zhigang Shi <Zhigang.Shi@xxxxxxxxxx> Add initial support for ltrf216a ambient light sensor. Datasheet: gitlab.steamos.cloud/shreeya/iio/-/blob/main/LTRF216A.pdf Co-developed-by: Shreeya Patel <shreeya.patel@xxxxxxxxxxxxx> Signed-off-by: Shreeya Patel <shreeya.patel@xxxxxxxxxxxxx> Signed-off-by: Zhigang Shi <Zhigang.Shi@xxxxxxxxxx> --- Changes in v3 - Use u16 instead of u8 for int_time_fac - Reorder headers in ltrf216a.c file - Remove int_time_mapping table and use int_time_available Changes in v2 - Add support for 25ms and 50ms integration time. - Rename some of the macros as per names given in datasheet - Add a comment for the mutex lock - Use read_avail callback instead of attributes and set the appropriate _available bit. - Use FIELD_PREP() at appropriate places. - Add a constant lookup table for integration time and reg val - Use BIT() macro for magic numbers. - Improve error handling at few places. - Use get_unaligned_le24() and div_u64() - Use probe_new() callback and devm functions - Return errors in probe using dev_err_probe() - Use DEFINE_SIMPLE_DEV_PM_OPS() - Correct the formula for lux to use 0.45 instead of 0.8 drivers/iio/light/Kconfig | 10 + drivers/iio/light/Makefile | 1 + drivers/iio/light/ltrf216a.c | 343 +++++++++++++++++++++++++++++++++++ 3 files changed, 354 insertions(+) create mode 100644 drivers/iio/light/ltrf216a.c diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig index a62c7b4b8678..33d2b24ba1da 100644 --- a/drivers/iio/light/Kconfig +++ b/drivers/iio/light/Kconfig @@ -318,6 +318,16 @@ config SENSORS_LM3533 changes. The ALS-control output values can be set per zone for the three current output channels.+config LTRF216A+ tristate "Liteon LTRF216A Light Sensor" + depends on 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 LTR501 tristate "LTR-501ALS-01 light sensor" depends on I2C diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile index d10912faf964..8fa91b9fe5b6 100644 --- a/drivers/iio/light/Makefile +++ b/drivers/iio/light/Makefile @@ -30,6 +30,7 @@ obj-$(CONFIG_SENSORS_ISL29028) += isl29028.o obj-$(CONFIG_ISL29125) += isl29125.o obj-$(CONFIG_JSA1212) += jsa1212.o obj-$(CONFIG_SENSORS_LM3533) += lm3533-als.o +obj-$(CONFIG_LTRF216A) += ltrf216a.o obj-$(CONFIG_LTR501) += ltr501.o obj-$(CONFIG_LV0104CS) += lv0104cs.o obj-$(CONFIG_MAX44000) += max44000.o diff --git a/drivers/iio/light/ltrf216a.c b/drivers/iio/light/ltrf216a.c new file mode 100644 index 000000000000..1ad1eb4a4c6d --- /dev/null +++ b/drivers/iio/light/ltrf216a.c @@ -0,0 +1,343 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * LTRF216A Ambient Light Sensor + * + * Copyright (C) 2021 Lite-On Technology Corp (Singapore) + * Author: Shi Zhigang <Zhigang.Shi@xxxxxxxxxx> + * + * IIO driver for LTRF216A (7-bit I2C slave address 0x53). + */ + +#include <linux/bitfield.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/pm.h> +#include <linux/iio/iio.h> +#include <asm/unaligned.h> + +#define LTRF216A_DRV_NAME "ltrf216a" + +#define LTRF216A_MAIN_CTRL 0x00 + +#define LTRF216A_ALS_DATA_STATUS BIT(3) +#define LTRF216A_ALS_ENABLE_MASK BIT(1) + +#define LTRF216A_ALS_MEAS_RES 0x04 +#define LTRF216A_MAIN_STATUS 0x07 +#define LTRF216A_CLEAR_DATA_0 0x0A + +#define LTRF216A_ALS_DATA_0 0x0D + +static const int ltrf216a_int_time_available[5][2] = { + {0, 400000}, + {0, 200000}, + {0, 100000}, + {0, 50000}, + {0, 25000}, +}; + +static const int ltrf216a_int_time_reg[5][2] = { + {400, 0x03}, + {200, 0x13}, + {100, 0x22}, + {50, 0x31}, + {25, 0x40}, +}; + +struct ltrf216a_data { + struct i2c_client *client; + u32 int_time; + u16 int_time_fac; + u8 als_gain_fac; + struct mutex mutex; /* Protect read and write operations */
I wasn't really sure about your comment related to the lock description here.
I see we are using these locks in read_raw and write_raw functions only and hence I've added the above comment. Thanks, Shreeya Patel
