[PATCH v2 0/4] iio: light: ltr501: Add ltr329 support

[PATCH v2 0/4] iio: light: ltr501: Add ltr329 support

[Esben Haabendal]

The LiteON LTR-329ALS-01 chip is similar to the LTR-303ALS-01, except for
interrupt support and related registers, which LTR-329ALS-01 does not have.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
Changes in v2:
- Fixed error handling when irq is defined for a chip that does not support
  irq, powering the chip down again.
- Added simlar fix to error handling of devm_request_threaded_irq() error
  handling, powering the chip down again on failure.
- Added explicit #include <linux/array_size.h>.
- Link to v1: https://patch.msgid.link/20260715-liteon-ltr329-v1-0-31f027051594@geanix.com

To: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
To: Nuno Sá <nuno.sa@analog.com>
To: Andy Shevchenko <andy@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Nikita Travkin <nikita@trvn.ru>
To: Maslov Dmitry <maslovdmitry@seeed.cc>
Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: linux-iio@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Esben Haabendal (4):
      dt-bindings: iio: light: ltr501: Add missing ltr303 compatible
      dt-bindings: iio: light: ltr501: Add ltr329 compatible
      iio: light: ltr501: Add ltr329 driver support
      iio: light: ltr501: Power down chip if request irq fails

 .../bindings/iio/light/liteon,ltr501.yaml          |  2 ++
 drivers/iio/light/ltr501.c                         | 35 +++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)
---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260711-liteon-ltr329-a3eb12414866

Best regards,
--  
Esben Haabendal <esben@geanix.com>

[PATCH v2 1/4] dt-bindings: iio: light: ltr501: Add missing ltr303 compatible

[Esben Haabendal]

When support for ltr303 was back in 2021, the bindings documentation was
not updated accordingly.

Fixes: 7d71d289e1ba ("iio: light: ltr501: Added ltr303 driver support")
Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml b/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
index c8074f180a79..ab9bb68dd736 100644
--- a/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
+++ b/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
@@ -18,6 +18,7 @@ properties:
       - liteon,ltr501
       - liteon,ltr559
       - liteon,ltr301
+      - liteon,ltr303
 
   reg:
     maxItems: 1

-- 
2.55.0

[PATCH v2 2/4] dt-bindings: iio: light: ltr501: Add ltr329 compatible

[Esben Haabendal]

LTR-329ALS-01 is an ambient light sensor similar to LTR-303ALS-01, but
without interrupt support.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml b/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
index ab9bb68dd736..76e81eba7ea4 100644
--- a/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
+++ b/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
@@ -19,6 +19,7 @@ properties:
       - liteon,ltr559
       - liteon,ltr301
       - liteon,ltr303
+      - liteon,ltr329
 
   reg:
     maxItems: 1

-- 
2.55.0

[PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support

[Esben Haabendal]

This adds support for the LTR-329ALS-01 chip, which is similar to
LTR-303ALS-01, except for interrupt, which LTR-329ALS-01 chip does not
have.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/iio/light/ltr501.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 7d045be78c6d..379e57ac5f5b 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/array_size.h> // for ARRAY_SIZE
 
 #include <linux/iio/iio.h>
 #include <linux/iio/events.h>
@@ -94,6 +95,7 @@ enum {
 	ltr559,
 	ltr301,
 	ltr303,
+	ltr329,
 };
 
 struct ltr501_gain {
@@ -178,6 +180,11 @@ static const struct ltr501_samp_table ltr501_ps_samp_table[] = {
 			{500000, 2000000}
 };
 
+static bool ltr501_has_irq_support(const struct ltr501_chip_info *chip_info)
+{
+	return chip_info->info != chip_info->info_no_irq;
+}
+
 static int ltr501_match_samp_freq(const struct ltr501_samp_table *tab,
 					   int len, int val, int val2)
 {
@@ -428,6 +435,9 @@ static int ltr501_read_intr_prst(const struct ltr501_data *data,
 {
 	int ret, samp_period, prst;
 
+	if (!ltr501_has_irq_support(data->chip_info))
+		return 0;
+
 	switch (type) {
 	case IIO_INTENSITY:
 		ret = regmap_field_read(data->reg_als_prst, &prst);
@@ -466,6 +476,9 @@ static int ltr501_write_intr_prst(struct ltr501_data *data,
 	int ret, samp_period, new_val;
 	unsigned long period;
 
+	if (!ltr501_has_irq_support(data->chip_info))
+		return 0;
+
 	if (val < 0 || val2 < 0)
 		return -EINVAL;
 
@@ -1257,6 +1270,18 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
 		.channels = ltr301_channels,
 		.no_channels = ARRAY_SIZE(ltr301_channels),
 	},
+	[ltr329] = {
+		.partid = 0x0A,
+		.als_gain = ltr559_als_gain_tbl,
+		.als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl),
+		.als_mode_active = BIT(0),
+		.als_gain_mask = BIT(2) | BIT(3) | BIT(4),
+		.als_gain_shift = 2,
+		.info = &ltr301_info_no_irq,
+		.info_no_irq = &ltr301_info_no_irq,
+		.channels = ltr301_channels,
+		.no_channels = ARRAY_SIZE(ltr301_channels),
+	},
 };
 
 static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)
@@ -1531,6 +1556,12 @@ static int ltr501_probe(struct i2c_client *client)
 		return ret;
 
 	if (client->irq > 0) {
+		if (!ltr501_has_irq_support(data->chip_info)) {
+			dev_err(&client->dev, "chip does not support irq\n");
+			ret = -EINVAL;
+			goto powerdown_on_error;
+		}
+
 		ret = devm_request_threaded_irq(&client->dev, client->irq,
 						NULL, ltr501_interrupt_handler,
 						IRQF_TRIGGER_FALLING |
@@ -1604,6 +1635,7 @@ static const struct i2c_device_id ltr501_id[] = {
 	{ .name = "ltr559", .driver_data = ltr559 },
 	{ .name = "ltr301", .driver_data = ltr301 },
 	{ .name = "ltr303", .driver_data = ltr303 },
+	{ .name = "ltr329", .driver_data = ltr329 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ltr501_id);
@@ -1613,6 +1645,7 @@ static const struct of_device_id ltr501_of_match[] = {
 	{ .compatible = "liteon,ltr559", },
 	{ .compatible = "liteon,ltr301", },
 	{ .compatible = "liteon,ltr303", },
+	{ .compatible = "liteon,ltr329", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ltr501_of_match);

-- 
2.55.0

[PATCH v2 4/4] iio: light: ltr501: Power down chip if request irq fails

[Esben Haabendal]

In case request irq failed, we were leaving the chip powered instead of
powering it down again.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 drivers/iio/light/ltr501.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 379e57ac5f5b..9d4de798e5ab 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -1571,7 +1571,7 @@ static int ltr501_probe(struct i2c_client *client)
 		if (ret) {
 			dev_err(&client->dev, "request irq (%d) failed\n",
 				client->irq);
-			return ret;
+			goto powerdown_on_error;
 		}
 	} else {
 		indio_dev->info = data->chip_info->info_no_irq;

-- 
2.55.0

Re: [PATCH v2 1/4] dt-bindings: iio: light: ltr501: Add missing ltr303 compatible

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Triggered buffer reads will always time out and fail for ltr303 (and ltr301) when the IIO timestamp channel is enabled.
--

--- Patch [1]: [PATCH v2 1/4] dt-bindings: iio: light: ltr501: Add missing ltr303 compatible ---
commit 0ab668b54a0d483bbb8920d0f07259a66c763c55
Author: Esben Haabendal <esben@geanix.com>

dt-bindings: iio: light: ltr501: Add missing ltr303 compatible

This commit updates the device tree bindings for the ltr501 driver to
include the liteon,ltr303 compatible string, which was missed when driver
support was added in 2021.

> diff --git a/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml b/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
> index c8074f180a793..ab9bb68dd7363 100644
> --- a/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
> +++ b/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
> @@ -18,6 +18,7 @@ properties:
>        - liteon,ltr501
>        - liteon,ltr559
>        - liteon,ltr301
> +      - liteon,ltr303

[Severity: High]
This isn't a bug introduced by this patch, but could making this compatible
string valid expose a pre-existing timeout issue in the driver?

Looking at ltr501_trigger_handler() in drivers/iio/light/ltr501.c, the code
hardcodes a check for bit 2 in the scan mask to wait for proximity data:

if (test_bit(2, indio_dev->active_scan_mask))
        mask |= LTR501_STATUS_PS_RDY;

Since ltr303 (and ltr301) are ALS-only devices without a proximity sensor,
bit 2 actually maps to the IIO timestamp channel. If a user enables the
timestamp channel, will the driver wait for LTR501_STATUS_PS_RDY?

Because the hardware never sets this ready bit for an ALS-only variant, it
appears ltr501_drdy() would hit its 2.5-second timeout on every trigger
and return -EIO. Is a fix needed in the driver to properly handle the
timestamp channel for these variants?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-liteon-ltr329-v2-0-d18af55edab5@geanix.com?part=1

Re: [PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free of IIO event interface due to mixing devm_request_threaded_irq with manual iio_device_unregister
- [High] Kernel stack memory leak and logic failure on big-endian systems due to reading 2 bytes into an uninitialized 4-byte integer
--

--- Patch [3]: [PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support ---
commit 8ae95bcf91bc6043da3fe9505d6fab5f356128f7
Author: Esben Haabendal <esben@geanix.com>

iio: light: ltr501: Add ltr329 driver support
    
This adds support for the LTR-329ALS-01 chip, which is similar to
LTR-303ALS-01, except for interrupt, which LTR-329ALS-01 chip does not
have.

> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 7d045be78c6df..379e57ac5f5b2 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c

[ ... ]

> @@ -1531,6 +1556,12 @@ static int ltr501_probe(struct i2c_client *client)
>  		return ret;
>  
>  	if (client->irq > 0) {
> +		if (!ltr501_has_irq_support(data->chip_info)) {
> +			dev_err(&client->dev, "chip does not support irq\n");
> +			ret = -EINVAL;
> +			goto powerdown_on_error;
> +		}
> +
>  		ret = devm_request_threaded_irq(&client->dev, client->irq,

[Severity: High]
This is a pre-existing issue, but does using devres for the IRQ while
manually unregistering the IIO device cause a use-after-free?

In ltr501_remove(), iio_device_unregister() is called manually, which
frees the event interface. However, because the IRQ is allocated with
devm_request_threaded_irq(), it remains active until after
ltr501_remove() returns.

If an interrupt fires during this window, ltr501_interrupt_handler()
could call iio_push_event(), which accesses the freed event interface.


[Severity: High]
This is also a pre-existing issue, but is there a risk of leaking kernel
stack memory on big-endian systems in ltr501_trigger_handler()?

In ltr501_trigger_handler(), regmap_bulk_read() reads 2 bytes into an
uninitialized 4-byte integer:

ltr501_trigger_handler() {
    ...
    int psdata;
    ...
    ret = regmap_bulk_read(data->regmap, LTR501_PS_DATA, &psdata, 2);
    ...
}

On big-endian architectures, these 2 bytes overwrite the most
significant bytes of psdata, leaving the least significant bytes
uninitialized.

The driver then applies a mask:

    scan.channels[j++] = psdata & LTR501_PS_DATA_MASK;

Could this extract uninitialized kernel stack data and push it to the
IIO ring buffer, exposing it to userspace?

>  						NULL, ltr501_interrupt_handler,
>  						IRQF_TRIGGER_FALLING |

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-liteon-ltr329-v2-0-d18af55edab5@geanix.com?part=3

Re: [PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support

[Joshua Crofts]

On Wed, 15 Jul 2026 14:27:25 +0200
Esben Haabendal <esben@geanix.com> wrote:

> This adds support for the LTR-329ALS-01 chip, which is similar to
> LTR-303ALS-01, except for interrupt, which LTR-329ALS-01 chip does not
> have.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---
>  drivers/iio/light/ltr501.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 7d045be78c6d..379e57ac5f5b 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -15,6 +15,7 @@
>  #include <linux/delay.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/array_size.h> // for ARRAY_SIZE

Just a small nit, even though the list isn't exactly ordered, please
try to add the new include approximately where it would be if ordered,
i.e. array_size.h goes to the top. Additionally, you don't need to add
a comment.

Everything else seems fine otherwise.

-- 
Kind regards

CJD

Re: [PATCH v2 0/4] iio: light: ltr501: Add ltr329 support

[Joshua Crofts]

On Wed, 15 Jul 2026 14:27:22 +0200
Esben Haabendal <esben@geanix.com> wrote:

> The LiteON LTR-329ALS-01 chip is similar to the LTR-303ALS-01, except for
> interrupt support and related registers, which LTR-329ALS-01 does not have.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---
> Changes in v2:
> - Fixed error handling when irq is defined for a chip that does not support
>   irq, powering the chip down again.
> - Added simlar fix to error handling of devm_request_threaded_irq() error
>   handling, powering the chip down again on failure.
> - Added explicit #include <linux/array_size.h>.
> - Link to v1: https://patch.msgid.link/20260715-liteon-ltr329-v1-0-31f027051594@geanix.com
> 

Quick process thing, please wait at least 24 hours before sending
a new version - let it sit on the mailing list so other reviewers
can send feedback! Larger series should wait a couple of days.

-- 
Kind regards

CJD

Re: [PATCH v2 4/4] iio: light: ltr501: Power down chip if request irq fails

[Joshua Crofts]

On Wed, 15 Jul 2026 14:27:26 +0200
Esben Haabendal <esben@geanix.com> wrote:

> In case request irq failed, we were leaving the chip powered instead of

devm_request_threaded_irq(), please mention functions and structs by their
full name to prevent ambiguity.

> powering it down again.

+ a very short explanation of what you did, 1 sentence

> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---
>  drivers/iio/light/ltr501.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 379e57ac5f5b..9d4de798e5ab 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -1571,7 +1571,7 @@ static int ltr501_probe(struct i2c_client *client)
>  		if (ret) {
>  			dev_err(&client->dev, "request irq (%d) failed\n",
>  				client->irq);

While you're at it, you can remove the dev_err() call as devm_request_threaded_irq
already calls dev_err_probe on failure, creating duplicate messages.

> -			return ret;
> +			goto powerdown_on_error;
>  		}
>  	} else {
>  		indio_dev->info = data->chip_info->info_no_irq;
> 

-- 
Kind regards

CJD

Re: [PATCH v2 4/4] iio: light: ltr501: Power down chip if request irq fails

[Nuno Sá]

On Wed, Jul 15, 2026 at 02:27:26PM +0200, Esben Haabendal wrote:
> In case request irq failed, we were leaving the chip powered instead of
> powering it down again.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---
>  drivers/iio/light/ltr501.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 379e57ac5f5b..9d4de798e5ab 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -1571,7 +1571,7 @@ static int ltr501_probe(struct i2c_client *client)
>  		if (ret) {
>  			dev_err(&client->dev, "request irq (%d) failed\n",
>  				client->irq);
> -			return ret;
> +			goto powerdown_on_error;

This looks like a fix so we should have a Fixes: tag. And being it a fix
it should be the first patch in the series (can come after bindings
though). Reason is for backports.

- Nuno Sá
>  		}
>  	} else {
>  		indio_dev->info = data->chip_info->info_no_irq;
> 
> -- 
> 2.55.0
> 

Re: [PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support

[Nuno Sá]

On Wed, Jul 15, 2026 at 02:27:25PM +0200, Esben Haabendal wrote:
> This adds support for the LTR-329ALS-01 chip, which is similar to
> LTR-303ALS-01, except for interrupt, which LTR-329ALS-01 chip does not
> have.
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> ---

Hi, I have a small not below. Kind of personal preference though. But
what Joshua mentioned should be addressed. With that:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/light/ltr501.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 7d045be78c6d..379e57ac5f5b 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -15,6 +15,7 @@
>  #include <linux/delay.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/array_size.h> // for ARRAY_SIZE
>  

...

>  
> +	if (!ltr501_has_irq_support(data->chip_info))
> +		return 0;
> +
>  	if (val < 0 || val2 < 0)
>  		return -EINVAL;
>  
> @@ -1257,6 +1270,18 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
>  		.channels = ltr301_channels,
>  		.no_channels = ARRAY_SIZE(ltr301_channels),
>  	},
> +	[ltr329] = {
> +		.partid = 0x0A,
> +		.als_gain = ltr559_als_gain_tbl,
> +		.als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl),
> +		.als_mode_active = BIT(0),
> +		.als_gain_mask = BIT(2) | BIT(3) | BIT(4),
> +		.als_gain_shift = 2,
> +		.info = &ltr301_info_no_irq,
> +		.info_no_irq = &ltr301_info_no_irq,
> +		.channels = ltr301_channels,
> +		.no_channels = ARRAY_SIZE(ltr301_channels),

Instead of playing the above game with info vs info_no_irq, an explicit
has_no_irq would probably be better. I mean conceptually if the pointers
are the same, it could also mean that both are with IRQ support. With
it, I think it would be safe to leave the .info pointer as NULL as it
would be always overwritten.

Having said the above, so strong feelings about it so up to you :)

- Nuno Sá

> +	},
>  };
>  
>  static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)
> @@ -1531,6 +1556,12 @@ static int ltr501_probe(struct i2c_client *client)
>  		return ret;
>  
>  	if (client->irq > 0) {
> +		if (!ltr501_has_irq_support(data->chip_info)) {
> +			dev_err(&client->dev, "chip does not support irq\n");
> +			ret = -EINVAL;
> +			goto powerdown_on_error;
> +		}
> +
>  		ret = devm_request_threaded_irq(&client->dev, client->irq,
>  						NULL, ltr501_interrupt_handler,
>  						IRQF_TRIGGER_FALLING |
> @@ -1604,6 +1635,7 @@ static const struct i2c_device_id ltr501_id[] = {
>  	{ .name = "ltr559", .driver_data = ltr559 },
>  	{ .name = "ltr301", .driver_data = ltr301 },
>  	{ .name = "ltr303", .driver_data = ltr303 },
> +	{ .name = "ltr329", .driver_data = ltr329 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, ltr501_id);
> @@ -1613,6 +1645,7 @@ static const struct of_device_id ltr501_of_match[] = {
>  	{ .compatible = "liteon,ltr559", },
>  	{ .compatible = "liteon,ltr301", },
>  	{ .compatible = "liteon,ltr303", },
> +	{ .compatible = "liteon,ltr329", },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, ltr501_of_match);
> 
> -- 
> 2.55.0
> 

Re: [PATCH v2 0/4] iio: light: ltr501: Add ltr329 support

[Nuno Sá]

On Wed, Jul 15, 2026 at 02:55:46PM +0200, Joshua Crofts wrote:
> On Wed, 15 Jul 2026 14:27:22 +0200
> Esben Haabendal <esben@geanix.com> wrote:
> 
> > The LiteON LTR-329ALS-01 chip is similar to the LTR-303ALS-01, except for
> > interrupt support and related registers, which LTR-329ALS-01 does not have.
> > 
> > Signed-off-by: Esben Haabendal <esben@geanix.com>
> > ---
> > Changes in v2:
> > - Fixed error handling when irq is defined for a chip that does not support
> >   irq, powering the chip down again.
> > - Added simlar fix to error handling of devm_request_threaded_irq() error
> >   handling, powering the chip down again on failure.
> > - Added explicit #include <linux/array_size.h>.
> > - Link to v1: https://patch.msgid.link/20260715-liteon-ltr329-v1-0-31f027051594@geanix.com
> > 
> 
> Quick process thing, please wait at least 24 hours before sending
> a new version - let it sit on the mailing list so other reviewers
> can send feedback! Larger series should wait a couple of days.

Yeah, as a nice rule of thumb at least a couple of days.

- Nuno Sá
> 
> -- 
> Kind regards
> 
> CJD

Re: [PATCH v2 4/4] iio: light: ltr501: Power down chip if request irq fails

[Esben Haabendal]

Nuno Sá <noname.nuno@gmail.com> writes:

> On Wed, Jul 15, 2026 at 02:27:26PM +0200, Esben Haabendal wrote:
>> In case request irq failed, we were leaving the chip powered instead of
>> powering it down again.
>>
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> ---
>>  drivers/iio/light/ltr501.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
>> index 379e57ac5f5b..9d4de798e5ab 100644
>> --- a/drivers/iio/light/ltr501.c
>> +++ b/drivers/iio/light/ltr501.c
>> @@ -1571,7 +1571,7 @@ static int ltr501_probe(struct i2c_client *client)
>>  		if (ret) {
>>  			dev_err(&client->dev, "request irq (%d) failed\n",
>>  				client->irq);
>> -			return ret;
>> +			goto powerdown_on_error;
>
> This looks like a fix so we should have a Fixes: tag. And being it a fix
> it should be the first patch in the series (can come after bindings
> though). Reason is for backports.

Sure.

/Esben

Re: [PATCH v2 4/4] iio: light: ltr501: Power down chip if request irq fails

[Esben Haabendal]

"Joshua Crofts" <joshua.crofts1@gmail.com> writes:

> On Wed, 15 Jul 2026 14:27:26 +0200
> Esben Haabendal <esben@geanix.com> wrote:
>
>> In case request irq failed, we were leaving the chip powered instead of
>
> devm_request_threaded_irq(), please mention functions and structs by their
> full name to prevent ambiguity.

Ok.

>> powering it down again.
>
> + a very short explanation of what you did, 1 sentence

I will do what I can.

>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> ---
>>  drivers/iio/light/ltr501.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
>> index 379e57ac5f5b..9d4de798e5ab 100644
>> --- a/drivers/iio/light/ltr501.c
>> +++ b/drivers/iio/light/ltr501.c
>> @@ -1571,7 +1571,7 @@ static int ltr501_probe(struct i2c_client *client)
>>  		if (ret) {
>>  			dev_err(&client->dev, "request irq (%d) failed\n",
>>  				client->irq);
>
> While you're at it, you can remove the dev_err() call as devm_request_threaded_irq
> already calls dev_err_probe on failure, creating duplicate messages.

Consider it done.

>
>> -			return ret;
>> +			goto powerdown_on_error;
>>  		}
>>  	} else {
>>  		indio_dev->info = data->chip_info->info_no_irq;
>>
>
> --

/Esben

Re: [PATCH v2 0/4] iio: light: ltr501: Add ltr329 support

[Esben Haabendal]

Nuno Sá <noname.nuno@gmail.com> writes:

> On Wed, Jul 15, 2026 at 02:55:46PM +0200, Joshua Crofts wrote:
>> On Wed, 15 Jul 2026 14:27:22 +0200
>> Esben Haabendal <esben@geanix.com> wrote:
>>
>> > The LiteON LTR-329ALS-01 chip is similar to the LTR-303ALS-01, except for
>> > interrupt support and related registers, which LTR-329ALS-01 does not have.
>> >
>> > Signed-off-by: Esben Haabendal <esben@geanix.com>
>> > ---
>> > Changes in v2:
>> > - Fixed error handling when irq is defined for a chip that does not support
>> >   irq, powering the chip down again.
>> > - Added simlar fix to error handling of devm_request_threaded_irq() error
>> >   handling, powering the chip down again on failure.
>> > - Added explicit #include <linux/array_size.h>.
>> > - Link to v1: https://patch.msgid.link/20260715-liteon-ltr329-v1-0-31f027051594@geanix.com
>> >
>>
>> Quick process thing, please wait at least 24 hours before sending
>> a new version - let it sit on the mailing list so other reviewers
>> can send feedback! Larger series should wait a couple of days.
>
> Yeah, as a nice rule of thumb at least a couple of days.

Fair enough.

/Esben

Re: [PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support

[Esben Haabendal]

Nuno Sá <noname.nuno@gmail.com> writes:

> On Wed, Jul 15, 2026 at 02:27:25PM +0200, Esben Haabendal wrote:
>> This adds support for the LTR-329ALS-01 chip, which is similar to
>> LTR-303ALS-01, except for interrupt, which LTR-329ALS-01 chip does not
>> have.
>>
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> ---
>
> Hi, I have a small not below. Kind of personal preference though. But
> what Joshua mentioned should be addressed. With that:
>
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
>
>>  drivers/iio/light/ltr501.c | 33 +++++++++++++++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>
>> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
>> index 7d045be78c6d..379e57ac5f5b 100644
>> --- a/drivers/iio/light/ltr501.c
>> +++ b/drivers/iio/light/ltr501.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/delay.h>
>>  #include <linux/regmap.h>
>>  #include <linux/regulator/consumer.h>
>> +#include <linux/array_size.h> // for ARRAY_SIZE
>>
>
> ...
>
>>
>> +	if (!ltr501_has_irq_support(data->chip_info))
>> +		return 0;
>> +
>>  	if (val < 0 || val2 < 0)
>>  		return -EINVAL;
>>
>> @@ -1257,6 +1270,18 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
>>  		.channels = ltr301_channels,
>>  		.no_channels = ARRAY_SIZE(ltr301_channels),
>>  	},
>> +	[ltr329] = {
>> +		.partid = 0x0A,
>> +		.als_gain = ltr559_als_gain_tbl,
>> +		.als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl),
>> +		.als_mode_active = BIT(0),
>> +		.als_gain_mask = BIT(2) | BIT(3) | BIT(4),
>> +		.als_gain_shift = 2,
>> +		.info = &ltr301_info_no_irq,
>> +		.info_no_irq = &ltr301_info_no_irq,
>> +		.channels = ltr301_channels,
>> +		.no_channels = ARRAY_SIZE(ltr301_channels),
>
> Instead of playing the above game with info vs info_no_irq, an explicit
> has_no_irq would probably be better. I mean conceptually if the pointers
> are the same, it could also mean that both are with IRQ support. With
> it, I think it would be safe to leave the .info pointer as NULL as it
> would be always overwritten.
>
> Having said the above, so strong feelings about it so up to you :)

Calling it has_irq would avoid double negation. But we would then have
to set it to true in most of the entries (all except ltr329 for now).

I will give it a spin.

/Esben

Re: [PATCH v2 4/4] iio: light: ltr501: Power down chip if request irq fails

[Esben Haabendal]

Nuno Sá <noname.nuno@gmail.com> writes:

> On Wed, Jul 15, 2026 at 02:27:26PM +0200, Esben Haabendal wrote:
>> In case request irq failed, we were leaving the chip powered instead of
>> powering it down again.
>>
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> ---
>>  drivers/iio/light/ltr501.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
>> index 379e57ac5f5b..9d4de798e5ab 100644
>> --- a/drivers/iio/light/ltr501.c
>> +++ b/drivers/iio/light/ltr501.c
>> @@ -1571,7 +1571,7 @@ static int ltr501_probe(struct i2c_client *client)
>>  		if (ret) {
>>  			dev_err(&client->dev, "request irq (%d) failed\n",
>>  				client->irq);
>> -			return ret;
>> +			goto powerdown_on_error;
>
> This looks like a fix so we should have a Fixes: tag. And being it a fix
> it should be the first patch in the series (can come after bindings
> though). Reason is for backports.

Added for next version.

/Esben

Re: [PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support

[Esben Haabendal]

"Joshua Crofts" <joshua.crofts1@gmail.com> writes:

> On Wed, 15 Jul 2026 14:27:25 +0200
> Esben Haabendal <esben@geanix.com> wrote:
>
>> This adds support for the LTR-329ALS-01 chip, which is similar to
>> LTR-303ALS-01, except for interrupt, which LTR-329ALS-01 chip does not
>> have.
>>
>> Signed-off-by: Esben Haabendal <esben@geanix.com>
>> ---
>>  drivers/iio/light/ltr501.c | 33 +++++++++++++++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>
>> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
>> index 7d045be78c6d..379e57ac5f5b 100644
>> --- a/drivers/iio/light/ltr501.c
>> +++ b/drivers/iio/light/ltr501.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/delay.h>
>>  #include <linux/regmap.h>
>>  #include <linux/regulator/consumer.h>
>> +#include <linux/array_size.h> // for ARRAY_SIZE
>
> Just a small nit, even though the list isn't exactly ordered, please
> try to add the new include approximately where it would be if ordered,
> i.e. array_size.h goes to the top. Additionally, you don't need to add
> a comment.

So alphabetically sorted? And even before linux/module.h?

Comment is going away...

> Everything else seems fine otherwise.

/Esben

Re: [PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support

[Joshua Crofts]

On Wed, 15 Jul 2026 15:55:48 +0200
Esben Haabendal <esben@geanix.com> wrote:

> "Joshua Crofts" <joshua.crofts1@gmail.com> writes:
> > Just a small nit, even though the list isn't exactly ordered, please
> > try to add the new include approximately where it would be if ordered,
> > i.e. array_size.h goes to the top. Additionally, you don't need to add
> > a comment.  
> 
> So alphabetically sorted? And even before linux/module.h?

Yes, includes should be sorted alphabetically (or at least as much
as possible), not randomly or by importance.

-- 
Kind regards

CJD

Re: [PATCH v2 3/4] iio: light: ltr501: Add ltr329 driver support

[Nuno Sá]

On Wed, Jul 15, 2026 at 03:43:10PM +0200, Esben Haabendal wrote:
> Nuno Sá <noname.nuno@gmail.com> writes:
> 
> > On Wed, Jul 15, 2026 at 02:27:25PM +0200, Esben Haabendal wrote:
> >> This adds support for the LTR-329ALS-01 chip, which is similar to
> >> LTR-303ALS-01, except for interrupt, which LTR-329ALS-01 chip does not
> >> have.
> >>
> >> Signed-off-by: Esben Haabendal <esben@geanix.com>
> >> ---
> >
> > Hi, I have a small not below. Kind of personal preference though. But
> > what Joshua mentioned should be addressed. With that:
> >
> > Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> >
> >>  drivers/iio/light/ltr501.c | 33 +++++++++++++++++++++++++++++++++
> >>  1 file changed, 33 insertions(+)
> >>
> >> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> >> index 7d045be78c6d..379e57ac5f5b 100644
> >> --- a/drivers/iio/light/ltr501.c
> >> +++ b/drivers/iio/light/ltr501.c
> >> @@ -15,6 +15,7 @@
> >>  #include <linux/delay.h>
> >>  #include <linux/regmap.h>
> >>  #include <linux/regulator/consumer.h>
> >> +#include <linux/array_size.h> // for ARRAY_SIZE
> >>
> >
> > ...
> >
> >>
> >> +	if (!ltr501_has_irq_support(data->chip_info))
> >> +		return 0;
> >> +
> >>  	if (val < 0 || val2 < 0)
> >>  		return -EINVAL;
> >>
> >> @@ -1257,6 +1270,18 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
> >>  		.channels = ltr301_channels,
> >>  		.no_channels = ARRAY_SIZE(ltr301_channels),
> >>  	},
> >> +	[ltr329] = {
> >> +		.partid = 0x0A,
> >> +		.als_gain = ltr559_als_gain_tbl,
> >> +		.als_gain_tbl_size = ARRAY_SIZE(ltr559_als_gain_tbl),
> >> +		.als_mode_active = BIT(0),
> >> +		.als_gain_mask = BIT(2) | BIT(3) | BIT(4),
> >> +		.als_gain_shift = 2,
> >> +		.info = &ltr301_info_no_irq,
> >> +		.info_no_irq = &ltr301_info_no_irq,
> >> +		.channels = ltr301_channels,
> >> +		.no_channels = ARRAY_SIZE(ltr301_channels),
> >
> > Instead of playing the above game with info vs info_no_irq, an explicit
> > has_no_irq would probably be better. I mean conceptually if the pointers
> > are the same, it could also mean that both are with IRQ support. With
> > it, I think it would be safe to leave the .info pointer as NULL as it
> > would be always overwritten.
> >
> > Having said the above, so strong feelings about it so up to you :)
> 
> Calling it has_irq would avoid double negation. But we would then have
> to set it to true in most of the entries (all except ltr329 for now).

Yeps, that´s is why I proposed has_no_* :)

- Nuno Sá

> 
> I will give it a spin.
> 
> /Esben