Re: [PATCH v8 1/2] Added AMS tsl2591 driver implementation

2021-04-18 Thread Joe Sandom
On Sun, Apr 18, 2021 at 10:08:30AM +0100, Jonathan Cameron wrote:
> On Fri, 16 Apr 2021 18:49:01 +0100
> Joe Sandom  wrote:
> 
> > Driver implementation for AMS/TAOS tsl2591 ambient light sensor.
> > 
> > This driver supports configuration via device tree and sysfs.
> > Supported channels for raw infrared light intensity,
> > raw combined light intensity and illuminance in lux.
> > The driver additionally supports iio events on lower and
> > upper thresholds.
> > 
> > This is a very-high sensitivity light-to-digital converter that
> > transforms light intensity into a digital signal.
> > 
> > Datasheet: https://ams.com/tsl25911#tab/documents
> > Signed-off-by: Joe Sandom 
> 
> Hi Joe,
> 
> I was in two minds about whether to just apply this and roll in the below
> suggestions + those Andy made or whether to ask you to do a v9.
> 
> The naming and units things below are complex enough that I'm not 100% sure
> I'd get the right so please take a look and clean up those last few
> corners!
> 
> Thanks,
> 
> Jonathan
> 
Thanks for the comments Jonathan. All good points! happy to clean up the
last few bits in v9. I'll aim to get this out over the next few days.

Thanks,

Joe
> 
> > ---
> > Changes in v8;
> > - tsl2591_write_raw() - goto after tsl2591_set_als_gain_int_time() not 
> > necessary
> > - tsl2591_read_event_value() and tsl2591_write_event_value() - break 
> > instead of goto in default case
> > - Introduced tsl2591_info_no_irq iio_info structure which is used when the 
> > interrupt is disabled. 
> >   This variant doesn't expose anything that can't be used when the 
> > interrupt is disabled
> > - Corrected ordering of includes for 
> > - Renamed TSL2591_FVAL_TO_ATIME to TSL2591_MSEC_TO_ATIME
> > - Renamed TSL2591_ATIME_TO_FVAL to TSL2591_ATIME_TO_MSEC
> > - Changed TSL2591_STS_ALS* defines to all use BIT for consistency
> > - Use (BIT(4) - 1) instead of value from list for 
> > TSL2591_PRST_ALS_INT_CYCLE_MAX
> > - Cleaned up a few blank lines that were unneccesary
> > - Use sysfs_emit() instead of snprintf() in 
> > tsl2591_in_illuminance_period_available_show()
> > - Use err_unlock branch instead of err to be clear on mutex_unlock cases
> > - Tidied up use of goto err_unlock in tsl2591_read_raw() to be consistent 
> > with other functions
> > 
> > NOTE;
> > - tsl2591_set_als_lower_threshold() and tsl2591_set_als_upper_threshold() 
> > forward declaration needed
> >   because tsl2591_set_als_lower_threshold() calls 
> > tsl2591_set_als_upper_threshold() and vice versa
> > 
> >  drivers/iio/light/Kconfig   |   11 +
> >  drivers/iio/light/Makefile  |1 +
> >  drivers/iio/light/tsl2591.c | 1227 +++
> >  3 files changed, 1239 insertions(+)
> >  create mode 100644 drivers/iio/light/tsl2591.c
> > 
> 
> ...
> 
> > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> > index ea376deaca54..d10912faf964 100644
> > --- a/drivers/iio/light/Makefile
> > +++ b/drivers/iio/light/Makefile
> > @@ -48,6 +48,7 @@ obj-$(CONFIG_ST_UVIS25_SPI)   += st_uvis25_spi.o
> >  obj-$(CONFIG_TCS3414)  += tcs3414.o
> >  obj-$(CONFIG_TCS3472)  += tcs3472.o
> >  obj-$(CONFIG_TSL2583)  += tsl2583.o
> > +obj-$(CONFIG_TSL2591)  += tsl2591.o
> >  obj-$(CONFIG_TSL2772)  += tsl2772.o
> >  obj-$(CONFIG_TSL4531)  += tsl4531.o
> >  obj-$(CONFIG_US5182D)  += us5182d.o
> > diff --git a/drivers/iio/light/tsl2591.c b/drivers/iio/light/tsl2591.c
> > new file mode 100644
> > index ..4f9c5e19ee35
> > --- /dev/null
> > +++ b/drivers/iio/light/tsl2591.c
> > @@ -0,0 +1,1227 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2020 Joe Sandom 
> 
> Update perhaps?
> 
> > + *
> > + * Datasheet: https://ams.com/tsl25911#tab/documents
> > + *
> > + * Device driver for the TAOS TSL2591. This is a very-high sensitivity
> > + * light-to-digital converter that transforms light intensity into a 
> > digital
> > + * signal.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* ADC integration time, field value to time in ms */
> 

Re: [PATCH v8 1/2] Added AMS tsl2591 driver implementation

2021-04-18 Thread Joe Sandom
On Sat, Apr 17, 2021 at 03:50:16PM +0300, Andy Shevchenko wrote:
> On Fri, Apr 16, 2021 at 8:49 PM Joe Sandom  wrote:
> >
> > Driver implementation for AMS/TAOS tsl2591 ambient light sensor.
> >
> > This driver supports configuration via device tree and sysfs.
> > Supported channels for raw infrared light intensity,
> > raw combined light intensity and illuminance in lux.
> > The driver additionally supports iio events on lower and
> > upper thresholds.
> >
> > This is a very-high sensitivity light-to-digital converter that
> > transforms light intensity into a digital signal.
> 
> Hmm... It's v8 and the subject line is wrongly formatted.
> Please add the corresponding prefix "iio: light: ..."
> 
Thanks for pointing that out Andy. I'll be sure to correct this in v9.

> Otherwise it's in very good shape.
> 
> ...
> 
> > +/* TSL2591 enable register definitions */
> > +#define TSL2591_PWR_ON  0x01
> > +#define TSL2591_PWR_OFF 0x00
> 
> > +#define TSL2591_ENABLE_ALS  0x02
> > +#define TSL2591_ENABLE_ALS_INT  0x10
> > +#define TSL2591_ENABLE_SLEEP_INT0x40
> > +#define TSL2591_ENABLE_NP_INT   0x80
> 
> Is it a bitfield?
> 
> ...
> 
> > +   als_lower_l = als_lower_threshold;
> 
> >> 0, but it's up to you.
> 
> > +   als_lower_h = als_lower_threshold >> 8;
> 
> ...
> 
> > +   als_upper_l = als_upper_threshold;
> > +   als_upper_h = als_upper_threshold >> 8;
> 
> Ditto.
> 
> -- 
> With Best Regards,
> Andy Shevchenko


[PATCH v8 1/2] Added AMS tsl2591 driver implementation

2021-04-16 Thread Joe Sandom
Driver implementation for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet: https://ams.com/tsl25911#tab/documents
Signed-off-by: Joe Sandom 
---
Changes in v8;
- tsl2591_write_raw() - goto after tsl2591_set_als_gain_int_time() not necessary
- tsl2591_read_event_value() and tsl2591_write_event_value() - break instead of 
goto in default case
- Introduced tsl2591_info_no_irq iio_info structure which is used when the 
interrupt is disabled. 
  This variant doesn't expose anything that can't be used when the interrupt is 
disabled
- Corrected ordering of includes for 
- Renamed TSL2591_FVAL_TO_ATIME to TSL2591_MSEC_TO_ATIME
- Renamed TSL2591_ATIME_TO_FVAL to TSL2591_ATIME_TO_MSEC
- Changed TSL2591_STS_ALS* defines to all use BIT for consistency
- Use (BIT(4) - 1) instead of value from list for TSL2591_PRST_ALS_INT_CYCLE_MAX
- Cleaned up a few blank lines that were unneccesary
- Use sysfs_emit() instead of snprintf() in 
tsl2591_in_illuminance_period_available_show()
- Use err_unlock branch instead of err to be clear on mutex_unlock cases
- Tidied up use of goto err_unlock in tsl2591_read_raw() to be consistent with 
other functions

NOTE;
- tsl2591_set_als_lower_threshold() and tsl2591_set_als_upper_threshold() 
forward declaration needed
  because tsl2591_set_als_lower_threshold() calls 
tsl2591_set_als_upper_threshold() and vice versa

 drivers/iio/light/Kconfig   |   11 +
 drivers/iio/light/Makefile  |1 +
 drivers/iio/light/tsl2591.c | 1227 +++
 3 files changed, 1239 insertions(+)
 create mode 100644 drivers/iio/light/tsl2591.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 33ad4dd0b5c7..6a69a9a3577a 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -501,6 +501,17 @@ config TSL2583
  Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
  Access ALS data via iio, sysfs.
 
+config TSL2591
+tristate "TAOS TSL2591 ambient light sensor"
+depends on I2C
+help
+  Select Y here for support of the AMS/TAOS TSL2591 ambient light 
sensor,
+  featuring channels for combined visible + IR intensity and lux 
illuminance.
+  Access data via iio and sysfs. Supports iio_events.
+
+  To compile this driver as a module, select M: the
+  module will be called tsl2591.
+
 config TSL2772
tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and 
proximity sensors"
depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index ea376deaca54..d10912faf964 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_ST_UVIS25_SPI)   += st_uvis25_spi.o
 obj-$(CONFIG_TCS3414)  += tcs3414.o
 obj-$(CONFIG_TCS3472)  += tcs3472.o
 obj-$(CONFIG_TSL2583)  += tsl2583.o
+obj-$(CONFIG_TSL2591)  += tsl2591.o
 obj-$(CONFIG_TSL2772)  += tsl2772.o
 obj-$(CONFIG_TSL4531)  += tsl4531.o
 obj-$(CONFIG_US5182D)  += us5182d.o
diff --git a/drivers/iio/light/tsl2591.c b/drivers/iio/light/tsl2591.c
new file mode 100644
index ..4f9c5e19ee35
--- /dev/null
+++ b/drivers/iio/light/tsl2591.c
@@ -0,0 +1,1227 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 Joe Sandom 
+ *
+ * Datasheet: https://ams.com/tsl25911#tab/documents
+ *
+ * Device driver for the TAOS TSL2591. This is a very-high sensitivity
+ * light-to-digital converter that transforms light intensity into a digital
+ * signal.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+
+/* ADC integration time, field value to time in ms */
+#define TSL2591_MSEC_TO_ATIME(x) (((x) + 1) * 100)
+/* ADC integration time, time in ms to field value */
+#define TSL2591_ATIME_TO_MSEC(x) (((x) / 100) - 1)
+
+/* TSL2591 register set */
+#define TSL2591_ENABLE  0x00
+#define TSL2591_CONTROL 0x01
+#define TSL2591_AILTL   0x04
+#define TSL2591_AILTH   0x05
+#define TSL2591_AIHTL   0x06
+#define TSL2591_AIHTH   0x07
+#define TSL2591_NP_AILTL0x08
+#define TSL2591_NP_AILTH0x09
+#define TSL2591_NP_AIHTL0x0A
+#define TSL2591_NP_AIHTH0x0B
+#define TSL2591_PERSIST 0x0C
+#define TSL2591_PACKAGE_ID  0x11
+#define TSL2591_DEVICE_ID   0x12
+#define TSL2591_STATUS  0x13
+#define TSL2591_C0_DATAL0x14
+#define TSL2591_C0_DATAH0x15
+#define TSL2591_C1_DATAL0x16
+#define TSL2591_C1_DATAH0x17
+
+/* TSL2591 command 

[PATCH v8 2/2] Added AMS tsl2591 device tree binding

2021-04-16 Thread Joe Sandom
Device tree binding for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Signed-off-by: Joe Sandom 
Reviewed-by: Rob Herring 
---
Changes in v8:
- No changes

Notes:
- Re-submitted to align the version with part 1 of the patch series

 .../bindings/iio/light/amstaos,tsl2591.yaml   | 50 +++
 1 file changed, 50 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml

diff --git a/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml 
b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
new file mode 100644
index ..596a3bc770f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.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/amstaos,tsl2591.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMS/TAOS TSL2591 Ambient Light Sensor (ALS)
+
+maintainers:
+  - Joe Sandom 
+
+description: |
+  AMS/TAOS TSL2591 is a very-high sensitivity
+  light-to-digital converter that transforms light intensity into a digital
+  signal.
+
+properties:
+  compatible:
+const: amstaos,tsl2591
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+description:
+  Interrupt (INT:Pin 2) Active low. Should be set to IRQ_TYPE_EDGE_FALLING.
+  interrupt is used to detect if the light intensity has fallen below
+  or reached above the configured threshold values.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+tsl2591@29 {
+compatible = "amstaos,tsl2591";
+reg = <0x29>;
+interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
+   };
+};
+...
-- 
2.17.1



[PATCH v7 1/2] Added AMS tsl2591 driver implementation

2021-04-01 Thread Joe Sandom
Driver implementation for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet: https://ams.com/tsl25911#tab/documents

Signed-off-by: Joe Sandom 
---

Changes in v7;
- Revert back to using plain numbers in register defines instead of BIT and 
GENMASK
- Changed from pre-increment of 'i' in for loops to post-increment
- Use iopoll.h - readx_poll_timeout macro for periodic poll on ALS status flag 
valid
- Remove the 0xFF masks on u8 types as they are redundant

 drivers/iio/light/Kconfig   |   11 +
 drivers/iio/light/Makefile  |1 +
 drivers/iio/light/tsl2591.c | 1224 +++
 3 files changed, 1236 insertions(+)
 create mode 100644 drivers/iio/light/tsl2591.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 33ad4dd0b5c7..6a69a9a3577a 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -501,6 +501,17 @@ config TSL2583
  Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
  Access ALS data via iio, sysfs.
 
+config TSL2591
+tristate "TAOS TSL2591 ambient light sensor"
+depends on I2C
+help
+  Select Y here for support of the AMS/TAOS TSL2591 ambient light 
sensor,
+  featuring channels for combined visible + IR intensity and lux 
illuminance.
+  Access data via iio and sysfs. Supports iio_events.
+
+  To compile this driver as a module, select M: the
+  module will be called tsl2591.
+
 config TSL2772
tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and 
proximity sensors"
depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index ea376deaca54..d10912faf964 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_ST_UVIS25_SPI)   += st_uvis25_spi.o
 obj-$(CONFIG_TCS3414)  += tcs3414.o
 obj-$(CONFIG_TCS3472)  += tcs3472.o
 obj-$(CONFIG_TSL2583)  += tsl2583.o
+obj-$(CONFIG_TSL2591)  += tsl2591.o
 obj-$(CONFIG_TSL2772)  += tsl2772.o
 obj-$(CONFIG_TSL4531)  += tsl4531.o
 obj-$(CONFIG_US5182D)  += us5182d.o
diff --git a/drivers/iio/light/tsl2591.c b/drivers/iio/light/tsl2591.c
new file mode 100644
index ..14a405a96e3a
--- /dev/null
+++ b/drivers/iio/light/tsl2591.c
@@ -0,0 +1,1224 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 Joe Sandom 
+ *
+ * Datasheet: https://ams.com/tsl25911#tab/documents
+ *
+ * Device driver for the TAOS TSL2591. This is a very-high sensitivity
+ * light-to-digital converter that transforms light intensity into a digital
+ * signal.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/* ADC integration time, field value to time in ms*/
+#define TSL2591_FVAL_TO_ATIME(x) (((x) + 1) * 100)
+/* ADC integration time, time in ms to field value */
+#define TSL2591_ATIME_TO_FVAL(x) (((x) / 100) - 1)
+
+/* TSL2591 register set */
+#define TSL2591_ENABLE  0x00
+#define TSL2591_CONTROL 0x01
+#define TSL2591_AILTL   0x04
+#define TSL2591_AILTH   0x05
+#define TSL2591_AIHTL   0x06
+#define TSL2591_AIHTH   0x07
+#define TSL2591_NP_AILTL0x08
+#define TSL2591_NP_AILTH0x09
+#define TSL2591_NP_AIHTL0x0A
+#define TSL2591_NP_AIHTH0x0B
+#define TSL2591_PERSIST 0x0C
+#define TSL2591_PACKAGE_ID  0x11
+#define TSL2591_DEVICE_ID   0x12
+#define TSL2591_STATUS  0x13
+#define TSL2591_C0_DATAL0x14
+#define TSL2591_C0_DATAH0x15
+#define TSL2591_C1_DATAL0x16
+#define TSL2591_C1_DATAH0x17
+
+/* TSL2591 command register definitions */
+#define TSL2591_CMD_NOP 0xA0
+#define TSL2591_CMD_SF_INTSET   0xE4
+#define TSL2591_CMD_SF_CALS_I   0xE5
+#define TSL2591_CMD_SF_CALS_NPI 0xE7
+#define TSL2591_CMD_SF_CNP_ALSI 0xEA
+
+/* TSL2591 enable register definitions */
+#define TSL2591_PWR_ON  0x01
+#define TSL2591_PWR_OFF 0x00
+#define TSL2591_ENABLE_ALS  0x02
+#define TSL2591_ENABLE_ALS_INT  0x10
+#define TSL2591_ENABLE_SLEEP_INT0x40
+#define TSL2591_ENABLE_NP_INT   0x80
+
+/* TSL2591 control register definitions */
+#define TSL2591_CTRL_ALS_INTEGRATION_100MS  0x00
+#define TSL2591_CTRL_ALS_INTEGRATION_200MS  0x01
+#define TSL2591_CTRL_ALS_INTEGRATION_300MS  0x02
+#define TSL2591_CTRL_ALS_INTEGRATION_400MS  0x03
+#define TSL2591_CTRL_ALS_INTEGRATION_500MS  0x04
+#define TSL2591_CTRL_ALS_INTEGRATION_600MS  0x05
+#define TSL2591_CTRL_ALS_LOW_GAIN

[PATCH v7 2/2] Added AMS tsl2591 device tree binding

2021-04-01 Thread Joe Sandom
Device tree binding for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Signed-off-by: Joe Sandom 
Reviewed-by: Rob Herring 
---
Changes in v7:
- No changes

Notes:
- Re-submitted to align the version with part 1 of the patch series

 .../bindings/iio/light/amstaos,tsl2591.yaml   | 50 +++
 1 file changed, 50 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml

diff --git a/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml 
b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
new file mode 100644
index ..596a3bc770f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.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/amstaos,tsl2591.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMS/TAOS TSL2591 Ambient Light Sensor (ALS)
+
+maintainers:
+  - Joe Sandom 
+
+description: |
+  AMS/TAOS TSL2591 is a very-high sensitivity
+  light-to-digital converter that transforms light intensity into a digital
+  signal.
+
+properties:
+  compatible:
+const: amstaos,tsl2591
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+description:
+  Interrupt (INT:Pin 2) Active low. Should be set to IRQ_TYPE_EDGE_FALLING.
+  interrupt is used to detect if the light intensity has fallen below
+  or reached above the configured threshold values.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+tsl2591@29 {
+compatible = "amstaos,tsl2591";
+reg = <0x29>;
+interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
+   };
+};
+...
-- 
2.17.1



Re: [PATCH v6 1/2] Added AMS tsl2591 driver implementation

2021-04-01 Thread Joe Sandom
On Fri, Mar 26, 2021 at 01:01:57PM +0200, Andy Shevchenko wrote:
> On Fri, Mar 26, 2021 at 12:05 AM Joe Sandom  wrote:
> >
> > Driver implementation for AMS/TAOS tsl2591 ambient light sensor.
> >
> > This driver supports configuration via device tree and sysfs.
> > Supported channels for raw infrared light intensity,
> > raw combined light intensity and illuminance in lux.
> > The driver additionally supports iio events on lower and
> > upper thresholds.
> >
> > This is a very-high sensitivity light-to-digital converter that
> > transforms light intensity into a digital signal.
> 
> I'm under the impression that you ignored at least half of my comments

The majority of your comments were applied in V5 as far as I can see.
Some of them I recognised as optional at the time. I had another sweep
through and have seen value in enforcing a few of the other points you
mentioned. I've added them to V7 and will release shortly. Thanks for
the feedback Andy.

> [1]. Have you seen them?
> 
> [1]: 
> https://lore.kernel.org/linux-iio/cahp75vcsw2xxdh--rxan7xt0ju+qfw9c_va0ggrgpgpbua0...@mail.gmail.com/
> 
> Please. address and come again.
> NAK for this version, sorry.
> 
> -- 
> With Best Regards,
> Andy Shevchenko


[RESEND][PATCH v6 2/2] Added AMS tsl2591 device tree binding

2021-03-25 Thread Joe Sandom
Device tree binding for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet: https://ams.com/tsl25911#tab/documents

Signed-off-by: Joe Sandom 
Reviewed-by: Rob Herring 
---
Changes in v6:
- No changes

Notes:
- Re-submitted to align the version with part 1 of the patch series
 
Reason for resend:
- Correctly pointed out that I forgot to add reviewed-by tag offered by Rob 
Herring

 .../bindings/iio/light/amstaos,tsl2591.yaml   | 50 +++
 1 file changed, 50 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml

diff --git a/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml 
b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
new file mode 100644
index ..596a3bc770f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.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/amstaos,tsl2591.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMS/TAOS TSL2591 Ambient Light Sensor (ALS)
+
+maintainers:
+  - Joe Sandom 
+
+description: |
+  AMS/TAOS TSL2591 is a very-high sensitivity
+  light-to-digital converter that transforms light intensity into a digital
+  signal.
+
+properties:
+  compatible:
+const: amstaos,tsl2591
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+description:
+  Interrupt (INT:Pin 2) Active low. Should be set to IRQ_TYPE_EDGE_FALLING.
+  interrupt is used to detect if the light intensity has fallen below
+  or reached above the configured threshold values.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+tsl2591@29 {
+compatible = "amstaos,tsl2591";
+reg = <0x29>;
+interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
+   };
+};
+...
-- 
2.17.1



Re: [PATCH v6 2/2] Added AMS tsl2591 device tree binding

2021-03-25 Thread Joe Sandom
On Thu, Mar 25, 2021 at 05:43:43PM -0600, Rob Herring wrote:
> On Thu, 25 Mar 2021 22:05:04 +0000, Joe Sandom wrote:
> > Device tree binding for AMS/TAOS tsl2591 ambient light sensor.
> > 
> > This driver supports configuration via device tree and sysfs.
> > Supported channels for raw infrared light intensity,
> > raw combined light intensity and illuminance in lux.
> > The driver additionally supports iio events on lower and
> > upper thresholds.
> > 
> > This is a very-high sensitivity light-to-digital converter that
> > transforms light intensity into a digital signal.
> > 
> > Datasheet: https://ams.com/tsl25911#tab/documents
> > 
> > Signed-off-by: Joe Sandom 
> > ---
> > Changes in v6:
> > - No changes
> > 
> > Notes:
> > - Re-submitted to align the version with part 1 of the patch series
> > 
> >  .../bindings/iio/light/amstaos,tsl2591.yaml   | 50 +++
> >  1 file changed, 50 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
> > 
> 
> 
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
> 
> If a tag was not added on purpose, please state why and what changed.
>
Thanks for pointing that out Rob, will amend that now and resend for
this version.


[PATCH v6 1/2] Added AMS tsl2591 driver implementation

2021-03-25 Thread Joe Sandom
Driver implementation for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet: https://ams.com/tsl25911#tab/documents

Signed-off-by: Joe Sandom 
---

Changes in v6;
- Separated tsl2591_set_als_thresholds into tsl2591_set_als_lower_threshold and
  tsl2591_set_upper_threshold for cleaner handling of forcing values when 
thresholds are met
- Added als persist value as an argument to tsl2591_set_als_persist_cycle to
  assign to chip from within the function. Cleaner approach.
- Cleaned up power_state handling in tsl2591_resume function
- Corrected interrupt handler to return IRQ_NONE in case of spurious interrupt

 drivers/iio/light/Kconfig   |   11 +
 drivers/iio/light/Makefile  |1 +
 drivers/iio/light/tsl2591.c | 1215 +++
 3 files changed, 1227 insertions(+)
 create mode 100644 drivers/iio/light/tsl2591.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 33ad4dd0b5c7..6a69a9a3577a 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -501,6 +501,17 @@ config TSL2583
  Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
  Access ALS data via iio, sysfs.
 
+config TSL2591
+tristate "TAOS TSL2591 ambient light sensor"
+depends on I2C
+help
+  Select Y here for support of the AMS/TAOS TSL2591 ambient light 
sensor,
+  featuring channels for combined visible + IR intensity and lux 
illuminance.
+  Access data via iio and sysfs. Supports iio_events.
+
+  To compile this driver as a module, select M: the
+  module will be called tsl2591.
+
 config TSL2772
tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and 
proximity sensors"
depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index ea376deaca54..d10912faf964 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_ST_UVIS25_SPI)   += st_uvis25_spi.o
 obj-$(CONFIG_TCS3414)  += tcs3414.o
 obj-$(CONFIG_TCS3472)  += tcs3472.o
 obj-$(CONFIG_TSL2583)  += tsl2583.o
+obj-$(CONFIG_TSL2591)  += tsl2591.o
 obj-$(CONFIG_TSL2772)  += tsl2772.o
 obj-$(CONFIG_TSL4531)  += tsl4531.o
 obj-$(CONFIG_US5182D)  += us5182d.o
diff --git a/drivers/iio/light/tsl2591.c b/drivers/iio/light/tsl2591.c
new file mode 100644
index ..d38569ce165e
--- /dev/null
+++ b/drivers/iio/light/tsl2591.c
@@ -0,0 +1,1215 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 Joe Sandom 
+ *
+ * Datasheet: https://ams.com/tsl25911#tab/documents
+ *
+ * Device driver for the TAOS TSL2591. This is a very-high sensitivity
+ * light-to-digital converter that transforms light intensity into a digital
+ * signal.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/* ADC integration time, field value to time in ms*/
+#define TSL2591_FVAL_TO_ATIME(x) (((x) + 1) * 100)
+/* ADC integration time, time in ms to field value */
+#define TSL2591_ATIME_TO_FVAL(x) (((x) / 100) - 1)
+
+/* TSL2591 register set */
+#define TSL2591_ENABLE  0x00
+#define TSL2591_CONTROL 0x01
+#define TSL2591_AILTL   0x04
+#define TSL2591_AILTH   0x05
+#define TSL2591_AIHTL   0x06
+#define TSL2591_AIHTH   0x07
+#define TSL2591_NP_AILTL0x08
+#define TSL2591_NP_AILTH0x09
+#define TSL2591_NP_AIHTL0x0A
+#define TSL2591_NP_AIHTH0x0B
+#define TSL2591_PERSIST 0x0C
+#define TSL2591_PACKAGE_ID  0x11
+#define TSL2591_DEVICE_ID   0x12
+#define TSL2591_STATUS  0x13
+#define TSL2591_C0_DATAL0x14
+#define TSL2591_C0_DATAH0x15
+#define TSL2591_C1_DATAL0x16
+#define TSL2591_C1_DATAH0x17
+
+/* TSL2591 command register definitions */
+#define TSL2591_CMD_NOP (BIT(5) | BIT(7))
+#define TSL2591_CMD_SF  GENMASK(7, 5)
+#define TSL2591_CMD_SF_INTSET   (BIT(2) | TSL2591_CMD_SF)
+#define TSL2591_CMD_SF_CALS_I   (BIT(0) | BIT(2) | TSL2591_CMD_SF)
+#define TSL2591_CMD_SF_CALS_NPI (GENMASK(2, 0) | TSL2591_CMD_SF)
+#define TSL2591_CMD_SF_CNP_ALSI (BIT(1) | BIT(3) | TSL2591_CMD_SF)
+
+/* TSL2591 enable register definitions */
+#define TSL2591_PWR_ON  BIT(0)
+#define TSL2591_PWR_OFF (0 << 0)
+#define TSL2591_ENABLE_ALS  BIT(1)
+#define TSL2591_ENABLE_ALS_INT  BIT(4)
+#define TSL2591_ENABLE_SLEEP_INTBIT(6)
+#define TSL2591_ENABLE_NP_INT   BIT(7)
+
+/* TSL2591 control regist

[PATCH v6 2/2] Added AMS tsl2591 device tree binding

2021-03-25 Thread Joe Sandom
Device tree binding for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet: https://ams.com/tsl25911#tab/documents

Signed-off-by: Joe Sandom 
---
Changes in v6:
- No changes

Notes:
- Re-submitted to align the version with part 1 of the patch series
 
 .../bindings/iio/light/amstaos,tsl2591.yaml   | 50 +++
 1 file changed, 50 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml

diff --git a/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml 
b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
new file mode 100644
index ..596a3bc770f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.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/amstaos,tsl2591.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMS/TAOS TSL2591 Ambient Light Sensor (ALS)
+
+maintainers:
+  - Joe Sandom 
+
+description: |
+  AMS/TAOS TSL2591 is a very-high sensitivity
+  light-to-digital converter that transforms light intensity into a digital
+  signal.
+
+properties:
+  compatible:
+const: amstaos,tsl2591
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+description:
+  Interrupt (INT:Pin 2) Active low. Should be set to IRQ_TYPE_EDGE_FALLING.
+  interrupt is used to detect if the light intensity has fallen below
+  or reached above the configured threshold values.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+tsl2591@29 {
+compatible = "amstaos,tsl2591";
+reg = <0x29>;
+interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
+   };
+};
+...
-- 
2.17.1



Re: [PATCH v5] Added AMS tsl2591 driver implementation

2021-03-18 Thread Joe Sandom
O Sat, Mar 13, 2021 at 07:15:54PM +, Jonathan Cameron wrote:
> On Tue,  9 Mar 2021 01:17:15 +
> Joe Sandom  wrote:
> 
> > Driver implementation for AMS/TAOS tsl2591 ambient light sensor.
> > 
> > This driver supports configuration via device tree and sysfs.
> > Supported channels for raw infrared light intensity,
> > raw combined light intensity and illuminance in lux.
> > The driver additionally supports iio events on lower and
> > upper thresholds.
> > 
> > This is a very-high sensitivity light-to-digital converter that
> > transforms light intensity into a digital signal.
> > 
> > Datasheet: https://ams.com/tsl25911#tab/documents
> > 
> > Signed-off-by: Joe Sandom 
> 
> A few minor details inline.  Very nearly good to go as far as
> I am concerned.
> 
> Jonathan
>
Thanks for the feedback Jonathan, glad to hear that we're nearly ready
to merge :). The changes for V6 are complete now, will give it a couple
more days before I send it out in case Andy or anyone else has any points to 
raise
> 
> > ---
> > 
> > Changes in v5:
> > - Added counterpart macro for integration time in ms to field value
> > - Tidied up some of the GENMASK usage
> > - Removed defines for ALS persist cycle literals, placed inline instead
> > - Removed unnecessary array size check & break in tsl2591_als_time_to_fval()
> > - Removed unnecessary casting and replaced with get_unaligned_le16() in
> >   tsl2591_read_channel_data()
> > - Cleaned up some return code checks on i2c calls
> > - Explicit return codes rather than relying on default value at before
> >   entering switch statements.
> > - Combine exit paths on error labels - following proper kernel idiom
> > - Cleaned up managed exit path for pm runtime
> > - Using reverse xmas tree convention for local vars in functions
> > - Corrected datasheet tag
> > 
> > Acknowledgements:
> > - Thanks to Jonathan and Andy for the constructive feedback!
> > 
> > NOTE:
> > Rob has reviewed the binding document and no changes required it
> > seems. Not sure what the best practice/convention is here; is [PATCH v5]
> > sufficient or do I still need to include the series number despite
> > there being no updates to the binding? please advise. Thank you!
> 
> Always send out the full series and increase version number for all patches.
> For those that haven't changed, just state that in the change log.
> Remember to pick up tags from reviewers though for new versions.
> 
> When everyone is happy I'll use the b4 utility which pulls the latest
> patch series down from lore.kernel.org and figures out any new tags
> for that last version.  It doesn't go scraping earlier versions for
> tags though.
> 
> I might just have picked it up except for the question over spurious
> interrupt handling + giving Andy more time for another look if he
> wants to take one.  We are fairly early in the cycle still though
> so lets do this the slow and steady way of having a v6.
> 
Thanks for this. Will be sure to send the full series out for V6!
> > 
> >  drivers/iio/light/Kconfig   |   11 +
> >  drivers/iio/light/Makefile  |1 +
> >  drivers/iio/light/tsl2591.c | 1193 +++
> >  3 files changed, 1205 insertions(+)
> >  create mode 100644 drivers/iio/light/tsl2591.c
> > 
> > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> > index 33ad4dd0b5c7..6a69a9a3577a 100644
> > --- a/drivers/iio/light/Kconfig
> > +++ b/drivers/iio/light/Kconfig
> > @@ -501,6 +501,17 @@ config TSL2583
> >   Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
> >   Access ALS data via iio, sysfs.
> >  
> > +config TSL2591
> > +tristate "TAOS TSL2591 ambient light sensor"
> > +depends on I2C
> > +help
> > +  Select Y here for support of the AMS/TAOS TSL2591 ambient light 
> > sensor,
> > +  featuring channels for combined visible + IR intensity and lux 
> > illuminance.
> > +  Access data via iio and sysfs. Supports iio_events.
> > +
> > +  To compile this driver as a module, select M: the
> > +  module will be called tsl2591.
> > +
> >  config TSL2772
> > tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and 
> > proximity sensors"
> > depends on I2C
> > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> > index ea376deaca54..d10912faf964 100644
> > --- a/drivers/iio/light/Makefile
> > +++ b/drivers/iio/light/Makefile
> &

[PATCH v5] Added AMS tsl2591 driver implementation

2021-03-08 Thread Joe Sandom
Driver implementation for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet: https://ams.com/tsl25911#tab/documents

Signed-off-by: Joe Sandom 
---

Changes in v5:
- Added counterpart macro for integration time in ms to field value
- Tidied up some of the GENMASK usage
- Removed defines for ALS persist cycle literals, placed inline instead
- Removed unnecessary array size check & break in tsl2591_als_time_to_fval()
- Removed unnecessary casting and replaced with get_unaligned_le16() in
  tsl2591_read_channel_data()
- Cleaned up some return code checks on i2c calls
- Explicit return codes rather than relying on default value at before
  entering switch statements.
- Combine exit paths on error labels - following proper kernel idiom
- Cleaned up managed exit path for pm runtime
- Using reverse xmas tree convention for local vars in functions
- Corrected datasheet tag

Acknowledgements:
- Thanks to Jonathan and Andy for the constructive feedback!

NOTE:
Rob has reviewed the binding document and no changes required it
seems. Not sure what the best practice/convention is here; is [PATCH v5]
sufficient or do I still need to include the series number despite
there being no updates to the binding? please advise. Thank you!

 drivers/iio/light/Kconfig   |   11 +
 drivers/iio/light/Makefile  |1 +
 drivers/iio/light/tsl2591.c | 1193 +++
 3 files changed, 1205 insertions(+)
 create mode 100644 drivers/iio/light/tsl2591.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 33ad4dd0b5c7..6a69a9a3577a 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -501,6 +501,17 @@ config TSL2583
  Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
  Access ALS data via iio, sysfs.
 
+config TSL2591
+tristate "TAOS TSL2591 ambient light sensor"
+depends on I2C
+help
+  Select Y here for support of the AMS/TAOS TSL2591 ambient light 
sensor,
+  featuring channels for combined visible + IR intensity and lux 
illuminance.
+  Access data via iio and sysfs. Supports iio_events.
+
+  To compile this driver as a module, select M: the
+  module will be called tsl2591.
+
 config TSL2772
tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and 
proximity sensors"
depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index ea376deaca54..d10912faf964 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_ST_UVIS25_SPI)   += st_uvis25_spi.o
 obj-$(CONFIG_TCS3414)  += tcs3414.o
 obj-$(CONFIG_TCS3472)  += tcs3472.o
 obj-$(CONFIG_TSL2583)  += tsl2583.o
+obj-$(CONFIG_TSL2591)  += tsl2591.o
 obj-$(CONFIG_TSL2772)  += tsl2772.o
 obj-$(CONFIG_TSL4531)  += tsl4531.o
 obj-$(CONFIG_US5182D)  += us5182d.o
diff --git a/drivers/iio/light/tsl2591.c b/drivers/iio/light/tsl2591.c
new file mode 100644
index ..ddecb32e206d
--- /dev/null
+++ b/drivers/iio/light/tsl2591.c
@@ -0,0 +1,1193 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 Joe Sandom 
+ *
+ * Datasheet: https://ams.com/tsl25911#tab/documents
+ *
+ * Device driver for the TAOS TSL2591. This is a very-high sensitivity
+ * light-to-digital converter that transforms light intensity into a digital
+ * signal.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/* ADC integration time, field value to time in ms*/
+#define TSL2591_FVAL_TO_ATIME(x) (((x) + 1) * 100)
+/* ADC integration time, time in ms to field value */
+#define TSL2591_ATIME_TO_FVAL(x) (((x) / 100) - 1)
+
+/* TSL2591 register set */
+#define TSL2591_ENABLE  0x00
+#define TSL2591_CONTROL 0x01
+#define TSL2591_AILTL   0x04
+#define TSL2591_AILTH   0x05
+#define TSL2591_AIHTL   0x06
+#define TSL2591_AIHTH   0x07
+#define TSL2591_NP_AILTL0x08
+#define TSL2591_NP_AILTH0x09
+#define TSL2591_NP_AIHTL0x0A
+#define TSL2591_NP_AIHTH0x0B
+#define TSL2591_PERSIST 0x0C
+#define TSL2591_PACKAGE_ID  0x11
+#define TSL2591_DEVICE_ID   0x12
+#define TSL2591_STATUS  0x13
+#define TSL2591_C0_DATAL0x14
+#define TSL2591_C0_DATAH0x15
+#define TSL2591_C1_DATAL0x16
+#define TSL2591_C1_DATAH0x17
+
+/* TSL2591 command register definitions */
+#define TSL2591_CMD_NOP (BIT(5) | BIT(7))
+#define TSL2591_CMD_SF  GENMASK(7, 5)
+#define TSL2591_

Re: [RESEND][PATCH v4 1/2] Added AMS tsl2591 driver implementation

2021-03-08 Thread Joe Sandom
Thanks for the feedback Andy, just finishing up V5 with the majority of
your comments on board. Some points of clarification inline.

On Mon, Mar 01, 2021 at 01:49:49PM +0200, Andy Shevchenko wrote:
> On Sat, Feb 27, 2021 at 6:58 PM Jonathan Cameron  wrote:
> > On Mon, 22 Feb 2021 21:23:12 +0000
> > Joe Sandom  wrote:
> 
> I will give my review on top of Jonathan's ones.
> 
> ...
> 
> > > Datasheet Available at: https://ams.com/tsl25911
> 
> Can we use Datasheet tag, please?
> 
> Datasheet: 
> 
> > >
> 
> ...and drop this blank line.
> 
> > > Signed-off-by: Joe Sandom 
> 
> ...
> 
> > > +config TSL2591
> > > +tristate "TAOS TSL2591 ambient light sensor"
> > > +depends on I2C
> > > +help
> > > +  Select Y here for support of the AMS/TAOS TSL2591 ambient 
> > > light sensor,
> > > +  featuring channels for combined visible + IR intensity and lux 
> > > illuminance.
> > > +  Access als data via iio and sysfs. Supports iio_events.
> 
> "Access data via IIO ad sysfs."
> 
> > > +  To compile this driver as a module, select M: the
> > > +  module will be called tsl2591.
> 
> ...
> 
> > > +/* ALS integration time field value to als time*/
> 
> Besides missing space the phrase confuses me (mostly due to "ALS ...
> als..." passage).
> 
> > > +#define TSL2591_FVAL_TO_ATIME(x) (((x) + 1) * 100)
> 
> ...
> 
> > > +/* TSL2591 command register definitions */
> > > +#define TSL2591_CMD_NOP (BIT(5) | BIT(7))
> > > +#define TSL2591_CMD_SF_INTSET   (BIT(2) | GENMASK(7, 5))
> > > +#define TSL2591_CMD_SF_CALS_I   (BIT(0) | BIT(2) | GENMASK(7, 5))
> > > +#define TSL2591_CMD_SF_CALS_NPI (GENMASK(2, 0) | GENMASK(7, 5))
> > > +#define TSL2591_CMD_SF_CNP_ALSI (BIT(1) | BIT(3) | GENMASK(7, 5))
> 
> If it's a bit combination, describe each bit field separately, but my
> guts are telling me that BIT() and GENMASK() use here is wrong.
> 
> ...
> 
> > > +/* TSL2591 enable register definitions */
> > > +#define TSL2591_PWR_ON  BIT(0)
> > > +#define TSL2591_PWR_OFF 0x00
> 
> Similar to above, here you rather use (1 << 0) and (0 << 0), or plain numbers.
> 
> ...
> 
> > > +#define TSL2591_CTRL_ALS_INTEGRATION_100MS  0x00
> > > +#define TSL2591_CTRL_ALS_INTEGRATION_200MS  BIT(0)
> > > +#define TSL2591_CTRL_ALS_INTEGRATION_300MS  BIT(1)
> > > +#define TSL2591_CTRL_ALS_INTEGRATION_400MS  GENMASK(1, 0)
> > > +#define TSL2591_CTRL_ALS_INTEGRATION_500MS  BIT(2)
> > > +#define TSL2591_CTRL_ALS_INTEGRATION_600MS  (BIT(0) | BIT(2))
> 
> Similar to the above. Drop all BIT() / GENMASK() use here and convert
> to plain numbers.
> 
> ...
> 
> > > +#define TSL2591_CTRL_ALS_LOW_GAIN   0x00
> > > +#define TSL2591_CTRL_ALS_MED_GAIN   BIT(4)
> > > +#define TSL2591_CTRL_ALS_HIGH_GAIN  BIT(5)
> 
> Ditto. And so on. Please, revisit all descriptions above and below.
> 
> ...
> 
> > > +#define TSL2591_ALS_MAX_VALUE   65535
> 
> If it's limited by the amount of bits in use, I prefer to spell it as
> (BIT(16) - 1), and we will immediately see this implication.
> 
> ...
> 
> > > +/*
> > > + * Period table is ALS persist cycle x integration time setting
> > > + * Integration times: 100ms, 200ms, 300ms, 400ms, 500ms, 600ms
> > > + * ALS cycles: 1, 2, 3, 5, 10, 20, 25, 30, 35, 40, 45, 50, 55, 60
> > > + */
> > > +static const char * const tsl2591_als_period_list[] = {
> > > + "0.1 0.2 0.3 0.5 1.0 2.0 2.5 3.0 3.5 4.0 4.5 5.0 5.5 6.0",
> > > + "0.2 0.4 0.6 1.0 2.0 4.0 5.0 6.0 7.0 8.0 9.0 10.0 11.0 12.0",
> > > + "0.3 0.6 0.9 1.5 3.0 6.0 7.5 9.0 10.5 12.0 13.5 15.0 16.5 18.0",
> > > + "0.4 0.8 1.2 2.0 4.0 8.0 10.0 12.0 14.0 16.0 18.0 20.0 22.0 24.0",
> > > + "0.5 1.0 1.5 2.5 5.0 10.0 12.5 15.0 17.5 20.0 22.5 25.0 27.5 30.0",
> > > + "0.6 1.2 1.8 3.0 6.0 12.0 15.0 18.0 21.0 24.0 27.0 30.0 33.0 36.0",
> > > +};
> 
> Okay... But it can be generated I guess.
> 
> ...
> 
> > > +static int tsl2591_als_time_to_fval(const u32 als_integration_time)
> > > +{
> > > + int i;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(tsl2591_int_time_available); ++i) {
> > > + if (als_integration_time == tsl2

[RESEND][PATCH v4 2/2] Added AMS tsl2591 device tree binding

2021-02-22 Thread Joe Sandom
Device tree binding for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet Available at: https://ams.com/tsl25911

Signed-off-by: Joe Sandom 
---
Changes in v4:
- Corrected id field to include vendor name in prefix

Notes:
- Previously incorrectly included binding in same patch as driver.
  This was pointed out by the maintainer and has now been changed to
  a patch series. Sorry for the confusion!

Reason for re-send;
- Maintainer email was outlook address, changed to gmail address as this
  is the email the patch is being sent from.
 
 .../bindings/iio/light/amstaos,tsl2591.yaml   | 50 +++
 1 file changed, 50 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml

diff --git a/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml 
b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
new file mode 100644
index ..596a3bc770f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.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/amstaos,tsl2591.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMS/TAOS TSL2591 Ambient Light Sensor (ALS)
+
+maintainers:
+  - Joe Sandom 
+
+description: |
+  AMS/TAOS TSL2591 is a very-high sensitivity
+  light-to-digital converter that transforms light intensity into a digital
+  signal.
+
+properties:
+  compatible:
+const: amstaos,tsl2591
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+description:
+  Interrupt (INT:Pin 2) Active low. Should be set to IRQ_TYPE_EDGE_FALLING.
+  interrupt is used to detect if the light intensity has fallen below
+  or reached above the configured threshold values.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+tsl2591@29 {
+compatible = "amstaos,tsl2591";
+reg = <0x29>;
+interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
+   };
+};
+...
-- 
2.17.1



[RESEND][PATCH v4 1/2] Added AMS tsl2591 driver implementation

2021-02-22 Thread Joe Sandom
Driver implementation for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet Available at: https://ams.com/tsl25911

Signed-off-by: Joe Sandom 
---

Changes in v4:
- Moved binding document to a separate patch and is now formed as a patch series
- Ensure vendor prefix is included in macros and that macros have appropriate 
naming
- Made use of BIT, GENMASK and FIELD_GET where appropriate for improved 
readability
- Corrected data channels terminology to more appropriate data register naming
- Removed tsl2591_als_readings and return data directly depending on the 
channel being read. See tsl2591_read_channel_data.
- Add more detailed comment on mutex definition
- Read als data as a block read instead of 4 separate byte reads
- Reserve *_compatible functions for checking compatibility, not for assigning 
the setting value
- Enforce setting corresponding upper/lower threshold to avoid issues in 
ordering when modifying thresholds
- Remove tsl2591_sysfs_attrs_ctrl and use info_mask to handle sysfs 
configuration where applicable
- Use .read_avail callback for *_available functions
- In .write_event_value, clean up period calculation handling. Removed some 
redundant code.
- Cleaned up some debug prints
- Cleaned up mutex handling for improved readability
- Ensured not to swallow return code in if statements in function calls

Reason for re-send;
- Maintainer email was outlook address, changed to gmail address as this
  is the email the patch is being sent from.

 drivers/iio/light/Kconfig   |   11 +
 drivers/iio/light/Makefile  |1 +
 drivers/iio/light/tsl2591.c | 1217 +++
 3 files changed, 1229 insertions(+)
 create mode 100644 drivers/iio/light/tsl2591.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 33ad4dd0b5c7..07550f1a1783 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -501,6 +501,17 @@ config TSL2583
  Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
  Access ALS data via iio, sysfs.
 
+config TSL2591
+tristate "TAOS TSL2591 ambient light sensor"
+depends on I2C
+help
+  Select Y here for support of the AMS/TAOS TSL2591 ambient light 
sensor,
+  featuring channels for combined visible + IR intensity and lux 
illuminance.
+  Access als data via iio and sysfs. Supports iio_events.
+
+  To compile this driver as a module, select M: the
+  module will be called tsl2591.
+
 config TSL2772
tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and 
proximity sensors"
depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index ea376deaca54..d10912faf964 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_ST_UVIS25_SPI)   += st_uvis25_spi.o
 obj-$(CONFIG_TCS3414)  += tcs3414.o
 obj-$(CONFIG_TCS3472)  += tcs3472.o
 obj-$(CONFIG_TSL2583)  += tsl2583.o
+obj-$(CONFIG_TSL2591)  += tsl2591.o
 obj-$(CONFIG_TSL2772)  += tsl2772.o
 obj-$(CONFIG_TSL4531)  += tsl4531.o
 obj-$(CONFIG_US5182D)  += us5182d.o
diff --git a/drivers/iio/light/tsl2591.c b/drivers/iio/light/tsl2591.c
new file mode 100644
index ..1124e9da5106
--- /dev/null
+++ b/drivers/iio/light/tsl2591.c
@@ -0,0 +1,1217 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 Joe Sandom 
+ *
+ * Datasheet available at: https://ams.com/tsl25911
+ *
+ * Device driver for the TAOS TSL2591. This is a very-high sensitivity
+ * light-to-digital converter that transforms light intensity into a digital
+ * signal.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/* ALS integration time field value to als time*/
+#define TSL2591_FVAL_TO_ATIME(x) (((x) + 1) * 100)
+
+/* TSL2591 register set */
+#define TSL2591_ENABLE  0x00
+#define TSL2591_CONTROL 0x01
+#define TSL2591_AILTL   0x04
+#define TSL2591_AILTH   0x05
+#define TSL2591_AIHTL   0x06
+#define TSL2591_AIHTH   0x07
+#define TSL2591_NP_AILTL0x08
+#define TSL2591_NP_AILTH0x09
+#define TSL2591_NP_AIHTL0x0A
+#define TSL2591_NP_AIHTH0x0B
+#define TSL2591_PERSIST 0x0C
+#define TSL2591_PACKAGE_ID  0x11
+#define TSL2591_DEVICE_ID   0x12
+#define TSL2591_STATUS  0x13
+#define TSL2591_C0_DATAL0x14
+#define TSL2591_C0_DATAH0x15
+#define TSL2591_C1_DATAL0x16
+#define TSL2591_C1_DATAH0x17
+
+/* TSL2591 command register def

[PATCH v4 2/2] Added AMS tsl2591 device tree binding

2021-02-22 Thread Joe Sandom
Device tree binding for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet Available at: https://ams.com/tsl25911

Signed-off-by: Joe Sandom 
---
Changes in v4:
- Corrected id field to include vendor name in prefix

Notes:
- Previously incorrectly included binding in same patch as driver.
  This was pointed out by the maintainer and has now been changed to
  a patch series. Sorry for the confusion!
 
 .../bindings/iio/light/amstaos,tsl2591.yaml   | 50 +++
 1 file changed, 50 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml

diff --git a/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml 
b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
new file mode 100644
index ..596a3bc770f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.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/amstaos,tsl2591.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMS/TAOS TSL2591 Ambient Light Sensor (ALS)
+
+maintainers:
+  - Joe Sandom 
+
+description: |
+  AMS/TAOS TSL2591 is a very-high sensitivity
+  light-to-digital converter that transforms light intensity into a digital
+  signal.
+
+properties:
+  compatible:
+const: amstaos,tsl2591
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+description:
+  Interrupt (INT:Pin 2) Active low. Should be set to IRQ_TYPE_EDGE_FALLING.
+  interrupt is used to detect if the light intensity has fallen below
+  or reached above the configured threshold values.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+tsl2591@29 {
+compatible = "amstaos,tsl2591";
+reg = <0x29>;
+interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
+   };
+};
+...
-- 
2.17.1



[PATCH v4 1/2] Added AMS tsl2591 driver implementation

2021-02-22 Thread Joe Sandom
Driver implementation for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light intensity,
raw combined light intensity and illuminance in lux.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet Available at: https://ams.com/tsl25911

Signed-off-by: Joe Sandom 
---

Changes in v4:
- Moved binding document to a separate patch and is now formed as a patch series
- Ensure vendor prefix is included in macros and that macros have appropriate 
naming
- Made use of BIT, GENMASK and FIELD_GET where appropriate for improved 
readability
- Corrected data channels terminology to more appropriate data register naming
- Removed tsl2591_als_readings and return data directly depending on the 
channel being read. See tsl2591_read_channel_data.
- Add more detailed comment on mutex definition
- Read als data as a block read instead of 4 separate byte reads
- Reserve *_compatible functions for checking compatibility, not for assigning 
the setting value
- Enforce setting corresponding upper/lower threshold to avoid issues in 
ordering when modifying thresholds
- Remove tsl2591_sysfs_attrs_ctrl and use info_mask to handle sysfs 
configuration where applicable
- Use .read_avail callback for *_available functions
- In .write_event_value, clean up period calculation handling. Removed some 
redundant code.
- Cleaned up some debug prints
- Cleaned up mutex handling for improved readability
- Ensured not to swallow return code in if statements in function calls

 drivers/iio/light/Kconfig   |   11 +
 drivers/iio/light/Makefile  |1 +
 drivers/iio/light/tsl2591.c | 1217 +++
 3 files changed, 1229 insertions(+)
 create mode 100644 drivers/iio/light/tsl2591.c

diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 33ad4dd0b5c7..07550f1a1783 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -501,6 +501,17 @@ config TSL2583
  Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
  Access ALS data via iio, sysfs.
 
+config TSL2591
+tristate "TAOS TSL2591 ambient light sensor"
+depends on I2C
+help
+  Select Y here for support of the AMS/TAOS TSL2591 ambient light 
sensor,
+  featuring channels for combined visible + IR intensity and lux 
illuminance.
+  Access als data via iio and sysfs. Supports iio_events.
+
+  To compile this driver as a module, select M: the
+  module will be called tsl2591.
+
 config TSL2772
tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and 
proximity sensors"
depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index ea376deaca54..d10912faf964 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_ST_UVIS25_SPI)   += st_uvis25_spi.o
 obj-$(CONFIG_TCS3414)  += tcs3414.o
 obj-$(CONFIG_TCS3472)  += tcs3472.o
 obj-$(CONFIG_TSL2583)  += tsl2583.o
+obj-$(CONFIG_TSL2591)  += tsl2591.o
 obj-$(CONFIG_TSL2772)  += tsl2772.o
 obj-$(CONFIG_TSL4531)  += tsl4531.o
 obj-$(CONFIG_US5182D)  += us5182d.o
diff --git a/drivers/iio/light/tsl2591.c b/drivers/iio/light/tsl2591.c
new file mode 100644
index ..1124e9da5106
--- /dev/null
+++ b/drivers/iio/light/tsl2591.c
@@ -0,0 +1,1217 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020 Joe Sandom 
+ *
+ * Datasheet available at: https://ams.com/tsl25911
+ *
+ * Device driver for the TAOS TSL2591. This is a very-high sensitivity
+ * light-to-digital converter that transforms light intensity into a digital
+ * signal.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/* ALS integration time field value to als time*/
+#define TSL2591_FVAL_TO_ATIME(x) (((x) + 1) * 100)
+
+/* TSL2591 register set */
+#define TSL2591_ENABLE  0x00
+#define TSL2591_CONTROL 0x01
+#define TSL2591_AILTL   0x04
+#define TSL2591_AILTH   0x05
+#define TSL2591_AIHTL   0x06
+#define TSL2591_AIHTH   0x07
+#define TSL2591_NP_AILTL0x08
+#define TSL2591_NP_AILTH0x09
+#define TSL2591_NP_AIHTL0x0A
+#define TSL2591_NP_AIHTH0x0B
+#define TSL2591_PERSIST 0x0C
+#define TSL2591_PACKAGE_ID  0x11
+#define TSL2591_DEVICE_ID   0x12
+#define TSL2591_STATUS  0x13
+#define TSL2591_C0_DATAL0x14
+#define TSL2591_C0_DATAH0x15
+#define TSL2591_C1_DATAL0x16
+#define TSL2591_C1_DATAH0x17
+
+/* TSL2591 command register definitions */
+#define TSL2591_CMD_NOP (BIT(5) | BIT(7))
+#define TSL2591_CMD_SF_INTSET   (BIT(2) | GENMASK(7, 5))
+#define TSL2591_CMD_

Re: [PATCH v3 RESEND] Added AMS tsl2591 driver implementation

2021-02-14 Thread Joe Sandom
On Sun, Feb 14, 2021 at 12:23:53PM +, Jonathan Cameron wrote:
> On Sat, 13 Feb 2021 13:22:41 +
> Joe Sandom  wrote:
> 
> > Driver implementation for AMS/TAOS tsl2591 ambient light sensor.
> > 
> > This driver supports configuration via device tree and sysfs.
> > Supported channels for raw infrared light, raw visible light,
> > raw combined light and combined lux value.
> > The driver additionally supports iio events on lower and
> > upper thresholds.
> > 
> > This is a very-high sensitivity light-to-digital converter that
> > transforms light intensity into a digital signal.
> > 
> > Datasheet Available at: https://ams.com/tsl25911
> > 
> > Signed-off-by: Joe Sandom 
> 
> Various bits and bobs inline.  Note that before we can apply this
> you will have to figure out how to deal with your email issues.
> gmail will alow smtp I believe, so I'd use that and
> git send-email if possible.
> 
> Jonathan

Hi Jonathan,

Thanks again for the comprehensive review. A lot of valuable points
made! all looks fine now with get send-email + gmail, thanks for that.
I'll work on getting a new revision out this week.

Thanks,
Joe

> 
> > ---
> > Changes in v3:
> > - Cleaned up descriptions in binding file and Kconfig
> > - Changed macros to be uppercase
> > - Cleaned up comment formatting for capitalisation and block comments
> > - Changed tsl2591_settings to tsl2591_als_settings as settings only
> >   related to als
> > - Return -EINVAL directly in default case to save some lines
> > - Consistent use of const in "compatible" check functions
> > - Removed mutex use in _show functions as not necessary
> > - Removed print's which contribute little value/have little meaning
> > 
> > NOTES;
> > - Where spaces are seen at the end of some lines, it looks like gmail
> >   has mangled things slightly.
> Ah.  You need to work out how to fix that unfortunately otherwise it
> won't be possible to apply patches.
> 
> There are some hints in Documentation/process/email-clients.rst
> 
> 
> > - checkpatch.pl --strict remarks that mutex definition should have a
> >   comment above it. I agree it has little meaning, but just added it to
> >   satisfy checkpatch.pl :)
> 
> Hmm. Comment on that below.  It should have a meaningful comment defining
> the scope of the lock.
> 
> > - For sysfs functions e.g. "in_illuminance_*", they're not currently
> >   prefixed with "tsl2591" because I wanted to keep things consistent
> >   with the other light drivers. Is this something we're looking to
> >   change with the other drivers too?
> Most of these shouldn't exist anyway as they are handled by the core
> of IIO via read_raw() and read_avail() callbacks.
> 
> The one remainder is the event _available one as we haven't yet added
> infrastructure to build those in the core.  I'd prefer it was prefixed
> but not enough to suggest we add noise fixing other drivers that do
> it differently.  Key rule of kernel development, never assume another
> driver is right - it may just be we decided it wasn't worth the pain
> of tidying it up!
> 
> > 
> > REASON FOR RESEND;
> > - Mailing lists were rejecting my outlook email, so switched to gmail as
> >   mailing lists seem to accept without issues.
> > 
> >  .../bindings/iio/light/amstaos,tsl2591.yaml   |   50 +
> >  drivers/iio/light/Kconfig |   11 +
> >  drivers/iio/light/Makefile|1 +
> >  drivers/iio/light/tsl2591.c   | 1220 +
> >  4 files changed, 1282 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
> >  create mode 100644 drivers/iio/light/tsl2591.c
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml 
> > b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
> > new file mode 100644
> > index ..29a53e0019dd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
> > @@ -0,0 +1,50 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> 
> Dt bindings should be a in a separate patch titled
> dt-bindings: * 
> to make them easy for Rob and devicetree specialists to spot.
> 
> > +$id: http://devicetree.org/schemas/iio/light/tsl2591.yaml#
> Try building this binding as per instructions in 
> Documentation/devicetree/writing-schema.rst
> 
> The line above needs the vendor prefix.
> 
> > +$schema: http://devicetree.o

[PATCH v3 RESEND] Added AMS tsl2591 driver implementation

2021-02-13 Thread Joe Sandom
Driver implementation for AMS/TAOS tsl2591 ambient light sensor.

This driver supports configuration via device tree and sysfs.
Supported channels for raw infrared light, raw visible light,
raw combined light and combined lux value.
The driver additionally supports iio events on lower and
upper thresholds.

This is a very-high sensitivity light-to-digital converter that
transforms light intensity into a digital signal.

Datasheet Available at: https://ams.com/tsl25911

Signed-off-by: Joe Sandom 
---
Changes in v3:
- Cleaned up descriptions in binding file and Kconfig
- Changed macros to be uppercase
- Cleaned up comment formatting for capitalisation and block comments
- Changed tsl2591_settings to tsl2591_als_settings as settings only
  related to als
- Return -EINVAL directly in default case to save some lines
- Consistent use of const in "compatible" check functions
- Removed mutex use in _show functions as not necessary
- Removed print's which contribute little value/have little meaning

NOTES;
- Where spaces are seen at the end of some lines, it looks like gmail
  has mangled things slightly.
- checkpatch.pl --strict remarks that mutex definition should have a
  comment above it. I agree it has little meaning, but just added it to
  satisfy checkpatch.pl :)
- For sysfs functions e.g. "in_illuminance_*", they're not currently
  prefixed with "tsl2591" because I wanted to keep things consistent
  with the other light drivers. Is this something we're looking to
  change with the other drivers too?

REASON FOR RESEND;
- Mailing lists were rejecting my outlook email, so switched to gmail as
  mailing lists seem to accept without issues.

 .../bindings/iio/light/amstaos,tsl2591.yaml   |   50 +
 drivers/iio/light/Kconfig |   11 +
 drivers/iio/light/Makefile|1 +
 drivers/iio/light/tsl2591.c   | 1220 +
 4 files changed, 1282 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
 create mode 100644 drivers/iio/light/tsl2591.c

diff --git a/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml 
b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.yaml
new file mode 100644
index ..29a53e0019dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/amstaos,tsl2591.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/tsl2591.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMS/TAOS TSL2591 Ambient Light Sensor (ALS)
+
+maintainers:
+  - Joe Sandom 
+
+description: |
+  AMS/TAOS TSL2591 is a very-high sensitivity
+  light-to-digital converter that transforms light intensity into a digital
+  signal.
+
+properties:
+  compatible:
+const: amstaos,tsl2591
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+description:
+  Interrupt (INT:Pin 2) Active low. Should be set to IRQ_TYPE_EDGE_FALLING.
+  interrupt is used to detect if the light intensity has fallen below
+  or reached above the configured threshold values.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+tsl2591@29 {
+compatible = "amstaos,tsl2591";
+reg = <0x29>;
+interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
+   };
+};
+...
diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
index 33ad4dd0b5c7..07550f1a1783 100644
--- a/drivers/iio/light/Kconfig
+++ b/drivers/iio/light/Kconfig
@@ -501,6 +501,17 @@ config TSL2583
  Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
  Access ALS data via iio, sysfs.
 
+config TSL2591
+tristate "TAOS TSL2591 ambient light sensor"
+depends on I2C
+help
+  Select Y here for support of the AMS/TAOS TSL2591 ambient light 
sensor,
+  featuring channels for combined visible + IR intensity and lux 
illuminance.
+  Access als data via iio and sysfs. Supports iio_events.
+
+  To compile this driver as a module, select M: the
+  module will be called tsl2591.
+
 config TSL2772
tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and 
proximity sensors"
depends on I2C
diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
index ea376deaca54..d10912faf964 100644
--- a/drivers/iio/light/Makefile
+++ b/drivers/iio/light/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_ST_UVIS25_SPI)   += st_uvis25_spi.o
 obj-$(CONFIG_TCS3414)  += tcs3414.o
 obj-$(CONFIG_TCS3472)  += tcs3472.o
 obj-$(CONFIG_TSL2583)  += tsl2583.o
+obj-$(CONFIG_TSL2591)  += tsl2591.o
 obj-$(CONFIG_TSL2772)  += tsl2772.o
 obj-$(CONFIG_TSL4531)