* [PATCH v6 0/2] Add LTRF216A Driver @ 2022-06-15 13:51 Shreeya Patel 2022-06-15 13:51 ` [PATCH v6 1/2] dt-bindings: Document ltrf216a light sensor bindings Shreeya Patel 2022-06-15 13:51 ` [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor Shreeya Patel 0 siblings, 2 replies; 14+ messages in thread From: Shreeya Patel @ 2022-06-15 13:51 UTC (permalink / raw) To: jic23, lars, robh+dt, Zhigang.Shi, krisman Cc: linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx, Shreeya Patel This patchset adds support for ltrf216a Ambient Light Sensor and documents the DT bindings for the same. Changes in v6 - Fix some errors reported by kernel test robot. - Add protocol details for the datasheet link. - Remove useless assignments. - Add unit details for read data delay macro. - Use pm_sleep_ptr(). Changes in v5 - Add power management support. - Add reset functionality. - Use readx_poll_timeout() to get data. - Cleanup some of the redundant code. - Update int_time_fac after I2C write is successful. - Rename mutex to lock. - Use Reverse Xmas tree pattern for all variable definitions. - Improve error handling messages and add error codes. - Add one more MODULE_AUTHOR. - Remove cleardata which was reading data for infrared light. - Remove patch for deprecated vendor prefix [PATCH v4 3/3]. - Remove deprecated string from DT binding document. Changes in v4 - Add more descriptive comment for mutex lock - Fix mutex locking in read_raw() - Use i2c_smbus_read_i2c_block_data() 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 - Fix indentation in the bindings file. 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 - Add interrupt and power supply property in DT bindings - Add vendor prefix name as per the alphabetical order. Shreeya Patel (2): dt-bindings: Document ltrf216a light sensor bindings iio: light: Add support for ltrf216a sensor .../bindings/iio/light/liteon,ltrf216a.yaml | 50 ++ drivers/iio/light/Kconfig | 10 + drivers/iio/light/Makefile | 1 + drivers/iio/light/ltrf216a.c | 429 ++++++++++++++++++ 4 files changed, 490 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml create mode 100644 drivers/iio/light/ltrf216a.c -- 2.30.2 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v6 1/2] dt-bindings: Document ltrf216a light sensor bindings 2022-06-15 13:51 [PATCH v6 0/2] Add LTRF216A Driver Shreeya Patel @ 2022-06-15 13:51 ` Shreeya Patel 2022-06-15 17:20 ` Rob Herring 2022-06-19 12:34 ` Jonathan Cameron 2022-06-15 13:51 ` [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor Shreeya Patel 1 sibling, 2 replies; 14+ messages in thread From: Shreeya Patel @ 2022-06-15 13:51 UTC (permalink / raw) To: jic23, lars, robh+dt, Zhigang.Shi, krisman Cc: linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx, Shreeya Patel Add devicetree bindings for ltrf216a ambient light sensor. Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> --- Changes in v5 - Remove deprecated string 'ltr' from the bindings. Changes in v3 - Fix indentation in the example section Changes in v2 - Take over the maintainership for the bindings - Add interrupt and power supply property in DT bindings .../bindings/iio/light/liteon,ltrf216a.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml diff --git a/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml b/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml new file mode 100644 index 000000000000..f256ff2e744c --- /dev/null +++ b/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml @@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/light/liteon,ltrf216a.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: LTRF216A Ambient Light Sensor + +maintainers: + - Shreeya Patel <shreeya.patel@collabora.com> + +description: + Ambient light sensing with an i2c interface. + +properties: + compatible: + const: + - liteon,ltrf216a + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + vdd-supply: + description: Regulator that provides power to the sensor. + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + light-sensor@53 { + compatible = "liteon,ltrf216a"; + reg = <0x53>; + vdd-supply = <&vdd_regulator>; + interrupt-parent = <&gpio0>; + interrupts = <5 IRQ_TYPE_LEVEL_LOW>; + }; + }; -- 2.30.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v6 1/2] dt-bindings: Document ltrf216a light sensor bindings 2022-06-15 13:51 ` [PATCH v6 1/2] dt-bindings: Document ltrf216a light sensor bindings Shreeya Patel @ 2022-06-15 17:20 ` Rob Herring 2022-06-19 12:34 ` Jonathan Cameron 1 sibling, 0 replies; 14+ messages in thread From: Rob Herring @ 2022-06-15 17:20 UTC (permalink / raw) To: Shreeya Patel Cc: kernel, krisman, Zhigang.Shi, jic23, andy.shevchenko, robh+dt, devicetree, digetx, lars, linux-kernel, linux-iio, alvaro.soliverez On Wed, 15 Jun 2022 19:21:29 +0530, Shreeya Patel wrote: > Add devicetree bindings for ltrf216a ambient light sensor. > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> > --- > Changes in v5 > - Remove deprecated string 'ltr' from the bindings. > > Changes in v3 > - Fix indentation in the example section > > Changes in v2 > - Take over the maintainership for the bindings > - Add interrupt and power supply property in DT bindings > > .../bindings/iio/light/liteon,ltrf216a.yaml | 50 +++++++++++++++++++ > 1 file changed, 50 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml: properties:compatible:const: ['liteon,ltrf216a'] is not of type 'string' from schema $id: http://devicetree.org/meta-schemas/string-array.yaml# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml: ignoring, error in schema: properties: compatible: const Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.example.dtb:0:0: /example-0/i2c/light-sensor@53: failed to match any schema with compatible: ['liteon,ltrf216a'] doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 1/2] dt-bindings: Document ltrf216a light sensor bindings 2022-06-15 13:51 ` [PATCH v6 1/2] dt-bindings: Document ltrf216a light sensor bindings Shreeya Patel 2022-06-15 17:20 ` Rob Herring @ 2022-06-19 12:34 ` Jonathan Cameron 2022-06-19 12:57 ` Shreeya Patel 1 sibling, 1 reply; 14+ messages in thread From: Jonathan Cameron @ 2022-06-19 12:34 UTC (permalink / raw) To: Shreeya Patel Cc: lars, robh+dt, Zhigang.Shi, krisman, linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx On Wed, 15 Jun 2022 19:21:29 +0530 Shreeya Patel <shreeya.patel@collabora.com> wrote: > Add devicetree bindings for ltrf216a ambient light sensor. > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> > --- > Changes in v5 > - Remove deprecated string 'ltr' from the bindings. > > Changes in v3 > - Fix indentation in the example section > > Changes in v2 > - Take over the maintainership for the bindings > - Add interrupt and power supply property in DT bindings > > .../bindings/iio/light/liteon,ltrf216a.yaml | 50 +++++++++++++++++++ > 1 file changed, 50 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml > > diff --git a/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml b/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml > new file mode 100644 > index 000000000000..f256ff2e744c > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml > @@ -0,0 +1,50 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/light/liteon,ltrf216a.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: LTRF216A Ambient Light Sensor > + > +maintainers: > + - Shreeya Patel <shreeya.patel@collabora.com> > + > +description: > + Ambient light sensing with an i2c interface. > + > +properties: > + compatible: > + const: > + - liteon,ltrf216a I assume you figured this out from the build bot error. const: liteon,ltrf216a Please make sure to do what that message from Rob's bot says and test your bindings before sending v7. > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + vdd-supply: > + description: Regulator that provides power to the sensor. > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + light-sensor@53 { > + compatible = "liteon,ltrf216a"; > + reg = <0x53>; > + vdd-supply = <&vdd_regulator>; > + interrupt-parent = <&gpio0>; > + interrupts = <5 IRQ_TYPE_LEVEL_LOW>; > + }; > + }; ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 1/2] dt-bindings: Document ltrf216a light sensor bindings 2022-06-19 12:34 ` Jonathan Cameron @ 2022-06-19 12:57 ` Shreeya Patel 0 siblings, 0 replies; 14+ messages in thread From: Shreeya Patel @ 2022-06-19 12:57 UTC (permalink / raw) To: Jonathan Cameron Cc: lars, robh+dt, Zhigang.Shi, krisman, linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx On 19/06/22 18:04, Jonathan Cameron wrote: > On Wed, 15 Jun 2022 19:21:29 +0530 > Shreeya Patel <shreeya.patel@collabora.com> wrote: > >> Add devicetree bindings for ltrf216a ambient light sensor. >> >> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> >> --- >> Changes in v5 >> - Remove deprecated string 'ltr' from the bindings. >> >> Changes in v3 >> - Fix indentation in the example section >> >> Changes in v2 >> - Take over the maintainership for the bindings >> - Add interrupt and power supply property in DT bindings >> >> .../bindings/iio/light/liteon,ltrf216a.yaml | 50 +++++++++++++++++++ >> 1 file changed, 50 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml >> >> diff --git a/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml b/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml >> new file mode 100644 >> index 000000000000..f256ff2e744c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/iio/light/liteon,ltrf216a.yaml >> @@ -0,0 +1,50 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/iio/light/liteon,ltrf216a.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: LTRF216A Ambient Light Sensor >> + >> +maintainers: >> + - Shreeya Patel <shreeya.patel@collabora.com> >> + >> +description: >> + Ambient light sensing with an i2c interface. >> + >> +properties: >> + compatible: >> + const: >> + - liteon,ltrf216a > I assume you figured this out from the build bot error. > > const: liteon,ltrf216a > > Please make sure to do what that message from Rob's bot says and test your bindings > before sending v7. Hi Jonathan, Sorry for the noise, I wasn't seeing this error before when I tested it on my machine. I just did a make clean and was able to see the error after that. I have fixed it now for v7. Thanks, Shreeya Patel > >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + vdd-supply: >> + description: Regulator that provides power to the sensor. >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/irq.h> >> + >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + light-sensor@53 { >> + compatible = "liteon,ltrf216a"; >> + reg = <0x53>; >> + vdd-supply = <&vdd_regulator>; >> + interrupt-parent = <&gpio0>; >> + interrupts = <5 IRQ_TYPE_LEVEL_LOW>; >> + }; >> + }; ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor 2022-06-15 13:51 [PATCH v6 0/2] Add LTRF216A Driver Shreeya Patel 2022-06-15 13:51 ` [PATCH v6 1/2] dt-bindings: Document ltrf216a light sensor bindings Shreeya Patel @ 2022-06-15 13:51 ` Shreeya Patel 2022-06-15 16:32 ` Andy Shevchenko ` (5 more replies) 1 sibling, 6 replies; 14+ messages in thread From: Shreeya Patel @ 2022-06-15 13:51 UTC (permalink / raw) To: jic23, lars, robh+dt, Zhigang.Shi, krisman Cc: linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx, Shreeya Patel, kernel test robot From: Zhigang Shi <Zhigang.Shi@liteon.com> Add initial support for ltrf216a ambient light sensor. Datasheet: https://bit.ly/3MRTYwY Reported-by: kernel test robot <lkp@intel.com> 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> --- Note :- This patch generates the below mentioned warnings due to not documenting the 'ltr' string in vendors-prefix.yaml and liteon,ltrf216a.yaml files. The thread for the discussion of not documenting 'ltr' as deprecated prefix can be found here. https://lore.kernel.org/lkml/20220511094024.175994-2-shreeya.patel@collabora.com/ There are released devices which uses ltr216a light sensor and exposes the vendor prefix name as 'ltr' through ACPI. Hence, we would like to add this string under compatible property which would help probe the light sensor driver. WARNING: DT compatible string "ltr,ltrf216a" appears un-documented -- check ./Documentation/devicetree/bindings/ #474: FILE: drivers/iio/light/ltrf216a.c:421: + { .compatible = "ltr,ltrf216a", }, WARNING: DT compatible string vendor "ltr" appears un-documented -- check ./Documentation/devicetree/bindings/vendor-prefixes.yaml #474: FILE: drivers/iio/light/ltrf216a.c:421: + { .compatible = "ltr,ltrf216a", }, Changes in v6 - Fix some errors reported by kernel test robot. - Add protocol details for the datasheet link. - Remove useless assignments. - Add unit details for read data delay macro. - Use pm_sleep_ptr(). Changes in v5 - Add power management support. - Add reset functionality. - Use readx_poll_timeout() to get data. - Cleanup some of the redundant code. - Update int_time_fac after I2C write is successful. - Rename mutex to lock. - Use Reverse Xmas tree pattern for all variable definitions. - Improve error handling messages and add error codes. - Add one more MODULE_AUTHOR. - Remove cleardata which was reading data for infrared light. Changes in v4 - Add more descriptive comment for mutex lock - Fix mutex locking in read_raw() - Use i2c_smbus_read_i2c_block_data() 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 | 429 +++++++++++++++++++++++++++++++++++ 3 files changed, 440 insertions(+) create mode 100644 drivers/iio/light/ltrf216a.c diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig index a62c7b4b8678..6c95431d12f6 100644 --- a/drivers/iio/light/Kconfig +++ b/drivers/iio/light/Kconfig @@ -332,6 +332,16 @@ 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 + 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..67165963dc9a --- /dev/null +++ b/drivers/iio/light/ltrf216a.c @@ -0,0 +1,429 @@ +// 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/iio/iio.h> +#include <asm/unaligned.h> + +#define LTRF216A_DRV_NAME "ltrf216a" + +#define LTRF216A_ALS_RESET_MASK BIT(4) +#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_CLEAR_DATA_0 0x0A +#define LTRF216A_ALS_DATA_0 0x0D +#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 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 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) +{ + int regval = FIELD_PREP(LTRF216A_ALS_ENABLE_MASK, 1); + struct ltrf216a_data *data = iio_priv(indio_dev); + struct device *dev = &data->client->dev; + int ret; + + /* enable sensor */ + ret = i2c_smbus_write_byte_data(data->client, LTRF216A_MAIN_CTRL, regval); + 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) +{ + int regval = FIELD_PREP(LTRF216A_ALS_RESET_MASK, 1); + struct ltrf216a_data *data = iio_priv(indio_dev); + + /* reset sensor, chip fails to respond to this, so ignore any errors */ + i2c_smbus_write_byte_data(data->client, LTRF216A_MAIN_CTRL, regval); + + /* 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 = i2c_smbus_write_byte_data(data->client, 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 = i2c_smbus_write_byte_data(data->client, 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, suspended; + + if (on) { + suspended = pm_runtime_suspended(dev); + ret = pm_runtime_get_sync(dev); + + /* Allow one integration cycle before allowing a reading */ + if (suspended) + msleep(ltrf216a_int_time_reg[0][0]); + } else { + pm_runtime_mark_last_busy(dev); + ret = pm_runtime_put_autosuspend(dev); + } + + return ret; +} + +static int ltrf216a_check_for_data(struct i2c_client *client) +{ + int ret; + + ret = i2c_smbus_read_byte_data(client, LTRF216A_MAIN_STATUS); + if (ret < 0) + dev_err(&client->dev, "Failed to read LTRF216A_MAIN_STATUS register: %d\n", ret); + + return ret; +} + +static int ltrf216a_read_data(struct ltrf216a_data *data, u8 addr) +{ + int ret, val; + u8 buf[3]; + + ret = readx_poll_timeout(ltrf216a_check_for_data, data->client, val, + val & LTRF216A_ALS_DATA_STATUS, LTRF216A_ALS_READ_DATA_DELAY_US, + LTRF216A_ALS_READ_DATA_DELAY_US * 25); + if (ret) { + dev_err(&data->client->dev, "Timed out waiting for valid data: %d\n", ret); + return ret; + } + + ret = i2c_smbus_read_i2c_block_data(data->client, addr, sizeof(buf), buf); + if (ret < 0) { + dev_err(&data->client->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 greendata; + u64 lux, div; + + ltrf216a_set_power_state(data, true); + + greendata = ltrf216a_read_data(data, LTRF216A_ALS_DATA_0); + if (greendata < 0) + return greendata; + + ltrf216a_set_power_state(data, false); + + 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; + + mutex_lock(&data->lock); + + switch (mask) { + case IIO_CHAN_INFO_PROCESSED: + ret = ltrf216a_get_lux(data); + if (ret < 0) + break; + *val = ret; + ret = IIO_VAL_INT; + break; + case IIO_CHAN_INFO_INT_TIME: + ret = ltrf216a_get_int_time(data, val, val2); + break; + default: + ret = -EINVAL; + break; + } + + mutex_unlock(&data->lock); + + return ret; +} + +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 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); + 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 = pm_runtime_set_active(&client->dev); + if (ret) + goto error_power_down; + + pm_runtime_enable(&client->dev); + pm_runtime_set_autosuspend_delay(&client->dev, 5000); + pm_runtime_use_autosuspend(&client->dev); + + ltrf216a_set_power_state(data, true); + + ret = ltrf216a_init(indio_dev); + if (ret < 0) { + dev_err_probe(&client->dev, ret, "ltrf216a chip init failed\n"); + goto error_power_down; + } + + 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) + goto error_power_down; + + ret = iio_device_register(indio_dev); + if (ret) + goto error_power_down; + +error_power_down: + ltrf216a_set_power_state(data, false); + + return ret; +} + +static int ltrf216a_remove(struct i2c_client *client) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(client); + + iio_device_unregister(indio_dev); + pm_runtime_disable(&client->dev); + pm_runtime_set_suspended(&client->dev); + ltrf216a_disable(indio_dev); + + return 0; +} + +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)); + + return ltrf216a_init(indio_dev); +} + +static DEFINE_SIMPLE_DEV_PM_OPS(ltrf216a_pm_ops, ltrf216a_runtime_suspend, + ltrf216a_runtime_resume); + +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_sleep_ptr(<rf216a_pm_ops), + .of_match_table = ltrf216a_of_match, + }, + .probe_new = ltrf216a_probe, + .remove = ltrf216a_remove, + .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"); -- 2.30.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor 2022-06-15 13:51 ` [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor Shreeya Patel @ 2022-06-15 16:32 ` Andy Shevchenko 2022-06-19 12:44 ` Jonathan Cameron 2022-07-06 20:52 ` Dmitry Osipenko ` (4 subsequent siblings) 5 siblings, 1 reply; 14+ messages in thread From: Andy Shevchenko @ 2022-06-15 16:32 UTC (permalink / raw) To: Shreeya Patel Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Zhigang.Shi, krisman, linux-iio, devicetree, Linux Kernel Mailing List, Collabora Kernel ML, alvaro.soliverez, Dmitry Osipenko, kernel test robot On Wed, Jun 15, 2022 at 3:52 PM Shreeya Patel <shreeya.patel@collabora.com> wrote: > > From: Zhigang Shi <Zhigang.Shi@liteon.com> > > Add initial support for ltrf216a ambient light sensor. This doesn't clarify why regmap API for SMBus can't be used. ... > Datasheet: https://bit.ly/3MRTYwY These kinds of links tend to disappear, please use the real link. ... > Reported-by: kernel test robot <lkp@intel.com> No, the new feature may not be reported. ... > +#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> + blank line > +#include <linux/iio/iio.h> + blank line > +#include <asm/unaligned.h> ... > +/* > + * Window Factor is needed when the device is under Window glass > + * with coated tinted ink. This is to compensate the light loss compensate for the > + * due to the lower transmission rate of the window glass and helps > + * in calculating lux. > + */ ... > +static int ltrf216a_set_power_state(struct ltrf216a_data *data, bool on) > +{ > + struct device *dev = &data->client->dev; > + int ret, suspended; > + > + if (on) { > + suspended = pm_runtime_suspended(dev); > + ret = pm_runtime_get_sync(dev); > + /* Allow one integration cycle before allowing a reading */ > + if (suspended) > + msleep(ltrf216a_int_time_reg[0][0]); Even if the get_sync() failed? Also, how do you take care about reference count in the case of failed get_sync90? > + } else { > + pm_runtime_mark_last_busy(dev); > + ret = pm_runtime_put_autosuspend(dev); > + } > + > + return ret; > +} ... > +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; > + > + mutex_lock(&data->lock); > + > + switch (mask) { > + case IIO_CHAN_INFO_PROCESSED: > + ret = ltrf216a_get_lux(data); > + if (ret < 0) > + break; > + *val = ret; > + ret = IIO_VAL_INT; > + break; > + case IIO_CHAN_INFO_INT_TIME: > + ret = ltrf216a_get_int_time(data, val, val2); > + break; > + default: > + ret = -EINVAL; > + break; > + } > + > + mutex_unlock(&data->lock); > + > + return ret; > +} You can refactor this function to call mutex lock/unlock as many times as cases you have and return directly. ... > + /* reset sensor, chip fails to respond to this, so ignore any errors */ > + ltrf216a_reset(indio_dev); > + > + ret = pm_runtime_set_active(&client->dev); > + if (ret) > + goto error_power_down; Why do you need to power down here? > + pm_runtime_enable(&client->dev); > + pm_runtime_set_autosuspend_delay(&client->dev, 5000); > + pm_runtime_use_autosuspend(&client->dev); > + > + ltrf216a_set_power_state(data, true); The below code suggests that you are mixing badly devm_ with non-devm_ APIs, don't do this. You have to group devm_ first followed by non-devm_ calls. ... > +static int ltrf216a_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + > + iio_device_unregister(indio_dev); > + pm_runtime_disable(&client->dev); > + pm_runtime_set_suspended(&client->dev); > + ltrf216a_disable(indio_dev); > + > + return 0; I believe the ordering of freeing resources and reverting state is not in reverse. See above why. > +} ... > +static DEFINE_SIMPLE_DEV_PM_OPS(ltrf216a_pm_ops, ltrf216a_runtime_suspend, > + ltrf216a_runtime_resume); Are you sure you are using proper macro? SIMPLE is for system sleep, while the function names suggest that this is about runtime PM. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor 2022-06-15 16:32 ` Andy Shevchenko @ 2022-06-19 12:44 ` Jonathan Cameron 0 siblings, 0 replies; 14+ messages in thread From: Jonathan Cameron @ 2022-06-19 12:44 UTC (permalink / raw) To: Andy Shevchenko Cc: Shreeya Patel, Lars-Peter Clausen, Rob Herring, Zhigang.Shi, krisman, linux-iio, devicetree, Linux Kernel Mailing List, Collabora Kernel ML, alvaro.soliverez, Dmitry Osipenko, kernel test robot > > > + /* reset sensor, chip fails to respond to this, so ignore any errors */ > > + ltrf216a_reset(indio_dev); > > + > > + ret = pm_runtime_set_active(&client->dev); > > + if (ret) > > + goto error_power_down; > > Why do you need to power down here? > > > + pm_runtime_enable(&client->dev); We now have devm_pm_runtime_enable() which will also deal with disabling use_autosuspend for you and should help you sort out some of the ordering. > > + pm_runtime_set_autosuspend_delay(&client->dev, 5000); > > + pm_runtime_use_autosuspend(&client->dev); > > + > > + ltrf216a_set_power_state(data, true); > > The below code suggests that you are mixing badly devm_ with non-devm_ > APIs, don't do this. You have to group devm_ first followed by > non-devm_ calls. > > ... > > > +static int ltrf216a_remove(struct i2c_client *client) > > +{ > > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > > + > > + iio_device_unregister(indio_dev); > > + pm_runtime_disable(&client->dev); > > + pm_runtime_set_suspended(&client->dev); > > + ltrf216a_disable(indio_dev); > > + > > + return 0; > > I believe the ordering of freeing resources and reverting state is not > in reverse. See above why. In particular you are calling disable there which is already handled by devm_ unwinding. > > > +} > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor 2022-06-15 13:51 ` [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor Shreeya Patel 2022-06-15 16:32 ` Andy Shevchenko @ 2022-07-06 20:52 ` Dmitry Osipenko 2022-07-06 20:52 ` Dmitry Osipenko ` (3 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Dmitry Osipenko @ 2022-07-06 20:52 UTC (permalink / raw) To: Shreeya Patel, jic23, lars, robh+dt, Zhigang.Shi, krisman Cc: linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx, kernel test robot Hello Shreeya, On 6/15/22 16:51, Shreeya Patel wrote: > +static int ltrf216a_set_power_state(struct ltrf216a_data *data, bool on) > +{ > + struct device *dev = &data->client->dev; > + int ret, suspended; > + > + if (on) { > + suspended = pm_runtime_suspended(dev); > + ret = pm_runtime_get_sync(dev); The pm_runtime_get_sync() returns 1 if device was already resumed and as Andy said there is no reason not to handle the RPM error. Hence we can rewrite it like this: if (on) { ret = pm_runtime_resume_and_get(dev); if (ret < 0) { dev_err(&data->client->dev, "failed to resume runtime PM: %d\n", ret); return ret; } /* Allow one integration cycle before allowing a reading */ if (!ret) msleep(ltrf216a_int_time_reg[0][0]); } else { pm_runtime_mark_last_busy(dev); ret = pm_runtime_put_autosuspend(dev); } -- Best regards, Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor 2022-06-15 13:51 ` [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor Shreeya Patel 2022-06-15 16:32 ` Andy Shevchenko 2022-07-06 20:52 ` Dmitry Osipenko @ 2022-07-06 20:52 ` Dmitry Osipenko 2022-07-06 20:59 ` Dmitry Osipenko ` (2 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Dmitry Osipenko @ 2022-07-06 20:52 UTC (permalink / raw) To: Shreeya Patel, jic23, lars, robh+dt, Zhigang.Shi, krisman Cc: linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx, kernel test robot On 6/15/22 16:51, Shreeya Patel wrote: > +static int ltrf216a_get_lux(struct ltrf216a_data *data) > +{ > + int greendata; > + u64 lux, div; > + > + ltrf216a_set_power_state(data, true); All the power-on errors should be handled. -- Best regards, Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor 2022-06-15 13:51 ` [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor Shreeya Patel ` (2 preceding siblings ...) 2022-07-06 20:52 ` Dmitry Osipenko @ 2022-07-06 20:59 ` Dmitry Osipenko 2022-07-06 21:00 ` Dmitry Osipenko 2022-07-06 21:09 ` Dmitry Osipenko 5 siblings, 0 replies; 14+ messages in thread From: Dmitry Osipenko @ 2022-07-06 20:59 UTC (permalink / raw) To: Shreeya Patel, jic23, lars, robh+dt, Zhigang.Shi, krisman Cc: linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx, kernel test robot On 6/15/22 16:51, Shreeya Patel wrote: > +static DEFINE_SIMPLE_DEV_PM_OPS(ltrf216a_pm_ops, ltrf216a_runtime_suspend, > + ltrf216a_runtime_resume); static DEFINE_RUNTIME_DEV_PM_OPS(ltrf216a_pm_ops, ltrf216a_runtime_suspend, ltrf216a_runtime_resume, NULL); -- Best regards, Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor 2022-06-15 13:51 ` [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor Shreeya Patel ` (3 preceding siblings ...) 2022-07-06 20:59 ` Dmitry Osipenko @ 2022-07-06 21:00 ` Dmitry Osipenko 2022-07-06 21:09 ` Dmitry Osipenko 5 siblings, 0 replies; 14+ messages in thread From: Dmitry Osipenko @ 2022-07-06 21:00 UTC (permalink / raw) To: Shreeya Patel, jic23, lars, robh+dt, Zhigang.Shi, krisman Cc: linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx, kernel test robot On 6/15/22 16:51, Shreeya Patel wrote: > +static int ltrf216a_remove(struct i2c_client *client) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(client); > + > + iio_device_unregister(indio_dev); > + pm_runtime_disable(&client->dev); > + pm_runtime_set_suspended(&client->dev); > + ltrf216a_disable(indio_dev); > + > + return 0; > +} As Jonathan said, there is no need to disable sensor in the remove() since you're using devm_add_action_or_reset(). If you're going to use devm_pm_runtime_enable(), then let's also use devm_iio_device_register() and in this case the ltrf216a_remove() is not needed anymore at all since the removal will be handled by the driver core entirely, i.e. you'll need to drop ltrf216a_remove(). -- Best regards, Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor 2022-06-15 13:51 ` [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor Shreeya Patel ` (4 preceding siblings ...) 2022-07-06 21:00 ` Dmitry Osipenko @ 2022-07-06 21:09 ` Dmitry Osipenko 2022-07-06 22:48 ` Shreeya Patel 5 siblings, 1 reply; 14+ messages in thread From: Dmitry Osipenko @ 2022-07-06 21:09 UTC (permalink / raw) To: Shreeya Patel, jic23, lars, robh+dt, Zhigang.Shi, krisman Cc: linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx, kernel test robot On 6/15/22 16:51, Shreeya Patel wrote: > +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); > + 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); Shouldn't SW resetting be done after enabling sensor? Perhaps that's why it fails to respond? -- Best regards, Dmitry ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor 2022-07-06 21:09 ` Dmitry Osipenko @ 2022-07-06 22:48 ` Shreeya Patel 0 siblings, 0 replies; 14+ messages in thread From: Shreeya Patel @ 2022-07-06 22:48 UTC (permalink / raw) To: Dmitry Osipenko, jic23, lars, robh+dt, Zhigang.Shi, krisman Cc: linux-iio, devicetree, linux-kernel, kernel, alvaro.soliverez, andy.shevchenko, digetx, kernel test robot On 07/07/22 02:39, Dmitry Osipenko wrote: > On 6/15/22 16:51, Shreeya Patel wrote: >> +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); >> + 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); > Shouldn't SW resetting be done after enabling sensor? Perhaps that's why > it fails to respond? We tried to reset the device through i2c-tool to see if that works :- (root@steamdeck ~)# i2cset -f 0 0x53 0x00 0x10 warning! This program can confuse your i2c bus, cause data loss and worse! Dangerous! Writing to a serial eeprom on a memory dimm may render your memory useless and make your system unbootable! I will write to device file /dev/i2c-0, chip address 0x53, data address 0x00, data 0x10, mode byte. Continue? [y/n] y error: write failed (1)(b+)(root@steamdeck ~)# But the problem here is that the light sensor resets itself instantaneously while the i2c transaction is still in progress. So it never replies with the proper stop bit that is expected at the end of a transaction. Hence, we decided to ignore the error. Thanks, Shreeya Patel > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-07-06 22:48 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-15 13:51 [PATCH v6 0/2] Add LTRF216A Driver Shreeya Patel 2022-06-15 13:51 ` [PATCH v6 1/2] dt-bindings: Document ltrf216a light sensor bindings Shreeya Patel 2022-06-15 17:20 ` Rob Herring 2022-06-19 12:34 ` Jonathan Cameron 2022-06-19 12:57 ` Shreeya Patel 2022-06-15 13:51 ` [PATCH v6 2/2] iio: light: Add support for ltrf216a sensor Shreeya Patel 2022-06-15 16:32 ` Andy Shevchenko 2022-06-19 12:44 ` Jonathan Cameron 2022-07-06 20:52 ` Dmitry Osipenko 2022-07-06 20:52 ` Dmitry Osipenko 2022-07-06 20:59 ` Dmitry Osipenko 2022-07-06 21:00 ` Dmitry Osipenko 2022-07-06 21:09 ` Dmitry Osipenko 2022-07-06 22:48 ` Shreeya Patel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).