[PATCH v1 0/2] iio: light: ltr501: Add LTR-659PS-01 prox sensor support

[PATCH v1 0/2] iio: light: ltr501: Add LTR-659PS-01 prox sensor support

[Svyatoslav Ryhel]

Add support for the Lite-On LTR-659PS-01 proximity sensor. This device is
nearly identical to the LTR-559ALS-01, but omits all elements related to
ambient light sensing.

Svyatoslav Ryhel (2):
  dt-bindings: iio: light: ltr501: Document LTR659 proximity sensor
  iio: light: ltr501: Add support for the LTR-659PS-01 proximity sensor

 .../bindings/iio/light/liteon,ltr501.yaml     |  1 +
 drivers/iio/light/ltr501.c                    | 58 +++++++++++++++++++
 2 files changed, 59 insertions(+)

-- 
2.53.0

[PATCH v1 1/2] dt-bindings: iio: light: ltr501: Document LTR659 proximity sensor

[Svyatoslav Ryhel]

The LTR-659PS-01 is a proximity sensor similar to the LTR-559ALS-01, but
without ambient light sensing support.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.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 20abd7da080d6..85ac3b540c206 100644
--- a/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
+++ b/Documentation/devicetree/bindings/iio/light/liteon,ltr501.yaml
@@ -17,6 +17,7 @@ properties:
       - liteon,ltr329
       - liteon,ltr501
       - liteon,ltr559
+      - liteon,ltr659
 
   reg:
     maxItems: 1
-- 
2.53.0

[PATCH v1 2/2] iio: light: ltr501: Add support for the LTR-659PS-01 proximity sensor

[Svyatoslav Ryhel]

Add support for the Lite-On LTR-659PS-01 proximity sensor. This device is
nearly identical to the LTR-559ALS-01, but omits all elements related to
ambient light sensing (Even the regmap is identical, but ALS registers are
marked as reserved and interactions with them are discouraged).

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/iio/light/ltr501.c | 58 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
index 302be2fc314c5..5cc1141322ad5 100644
--- a/drivers/iio/light/ltr501.c
+++ b/drivers/iio/light/ltr501.c
@@ -96,6 +96,7 @@ enum {
 	ltr329,
 	ltr501,
 	ltr559,
+	ltr659,
 };
 
 struct ltr501_gain {
@@ -649,6 +650,26 @@ static const struct iio_chan_spec ltr301_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(2),
 };
 
+static const struct iio_chan_spec ltr659_channels[] = {
+	{
+		.type = IIO_PROXIMITY,
+		.address = LTR501_PS_DATA,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 11,
+			.storagebits = 16,
+			.endianness = IIO_CPU,
+		},
+		.event_spec = ltr501_pxs_event_spec,
+		.num_event_specs = ARRAY_SIZE(ltr501_pxs_event_spec),
+		.ext_info = ltr501_ext_info,
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
 static int ltr501_read_info_raw(struct ltr501_data *data,
 				struct iio_chan_spec const *chan,
 				int *val)
@@ -1178,6 +1199,12 @@ static struct attribute *ltr301_attributes[] = {
 	NULL
 };
 
+static struct attribute *ltr659_attributes[] = {
+	&iio_dev_attr_in_proximity_scale_available.dev_attr.attr,
+	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	NULL
+};
+
 static const struct attribute_group ltr501_attribute_group = {
 	.attrs = ltr501_attributes,
 };
@@ -1186,6 +1213,10 @@ static const struct attribute_group ltr301_attribute_group = {
 	.attrs = ltr301_attributes,
 };
 
+static const struct attribute_group ltr659_attribute_group = {
+	.attrs = ltr659_attributes,
+};
+
 static const struct iio_info ltr501_info_no_irq = {
 	.read_raw = ltr501_read_raw,
 	.write_raw = ltr501_write_raw,
@@ -1218,6 +1249,22 @@ static const struct iio_info ltr301_info = {
 	.write_event_config	= &ltr501_write_event_config,
 };
 
+static const struct iio_info ltr659_info_no_irq = {
+	.read_raw = ltr501_read_raw,
+	.write_raw = ltr501_write_raw,
+	.attrs = &ltr659_attribute_group,
+};
+
+static const struct iio_info ltr659_info = {
+	.read_raw = ltr501_read_raw,
+	.write_raw = ltr501_write_raw,
+	.attrs = &ltr659_attribute_group,
+	.read_event_value	= &ltr501_read_event,
+	.write_event_value	= &ltr501_write_event,
+	.read_event_config	= &ltr501_read_event_config,
+	.write_event_config	= &ltr501_write_event_config,
+};
+
 static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
 	[ltr301] = {
 		.partid = 0x08,
@@ -1283,6 +1330,15 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
 		.channels = ltr501_channels,
 		.no_channels = ARRAY_SIZE(ltr501_channels),
 	},
+	[ltr659] = {
+		.partid = 0x09,
+		.ps_gain = ltr559_ps_gain_tbl,
+		.ps_gain_tbl_size = ARRAY_SIZE(ltr559_ps_gain_tbl),
+		.info = &ltr659_info,
+		.info_no_irq = &ltr659_info_no_irq,
+		.channels = ltr659_channels,
+		.no_channels = ARRAY_SIZE(ltr659_channels),
+	},
 };
 
 static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)
@@ -1635,6 +1691,7 @@ static const struct i2c_device_id ltr501_id[] = {
 	{ .name = "ltr329", .driver_data = ltr329 },
 	{ .name = "ltr501", .driver_data = ltr501 },
 	{ .name = "ltr559", .driver_data = ltr559 },
+	{ .name = "ltr659", .driver_data = ltr659 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ltr501_id);
@@ -1645,6 +1702,7 @@ static const struct of_device_id ltr501_of_match[] = {
 	{ .compatible = "liteon,ltr329", },
 	{ .compatible = "liteon,ltr501", },
 	{ .compatible = "liteon,ltr559", },
+	{ .compatible = "liteon,ltr659", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ltr501_of_match);
-- 
2.53.0

Re: [PATCH v1 2/2] iio: light: ltr501: Add support for the LTR-659PS-01 proximity sensor

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] iio: light: ltr501: scan_index mismatch breaks ltr501_trigger_handler()
- [Medium] iio: light: ltr501: missing IIO_CHAN_INFO_SAMP_FREQ in ltr659_channels
- [Medium] iio: light: ltr501: unconditional ALS register accesses in ltr501_init()
--

commit c2f0d77abdcaeb0f7ea57d758703f90ae8398569
Author: Svyatoslav Ryhel <clamor95@gmail.com>

iio: light: ltr501: Add support for the LTR-659PS-01 proximity sensor

This commit adds support for the Lite-On LTR-659PS-01 proximity-only sensor
to the ltr501 IIO driver.

> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 302be2fc314c5..5cc1141322ad5 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c

[ ... ]

> @@ -649,6 +650,26 @@ static const struct iio_chan_spec ltr301_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(2),
>  };
>  
> +static const struct iio_chan_spec ltr659_channels[] = {
> +	{
> +		.type = IIO_PROXIMITY,
> +		.address = LTR501_PS_DATA,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = 0,

[Severity: High]
Does assigning scan_index 0 here break triggered buffer capture for ltr659?

In ltr501_trigger_handler(), active_scan_mask bits 0 and 1 are hardcoded for
the ALS channels, while only bit 2 is treated as the proximity channel:

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

	ret = ltr501_drdy(data, mask);
	if (ret < 0)
		goto done;

	if (mask & LTR501_STATUS_ALS_RDY) {
		ret = regmap_bulk_read(data->regmap, LTR501_ALS_DATA1,
				       als_buf, sizeof(als_buf));
		if (ret < 0)
			goto done;
		if (test_bit(0, indio_dev->active_scan_mask))
			scan.channels[j++] = le16_to_cpu(als_buf[1]);

When buffered capture is enabled on ltr659, bit 0 is set in active_scan_mask
and bit 2 is never set, so ltr501_trigger_handler() waits on
LTR501_STATUS_ALS_RDY and reads LTR501_ALS_DATA1 instead of reading
LTR501_PS_DATA.

> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 11,
> +			.storagebits = 16,
> +			.endianness = IIO_CPU,
> +		},
> +		.event_spec = ltr501_pxs_event_spec,
> +		.num_event_specs = ARRAY_SIZE(ltr501_pxs_event_spec),
> +		.ext_info = ltr501_ext_info,
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +};
> +
>  static int ltr501_read_info_raw(struct ltr501_data *data,
>  				struct iio_chan_spec const *chan,
>  				int *val)
> @@ -1178,6 +1199,12 @@ static struct attribute *ltr301_attributes[] = {
>  	NULL
>  };
>  
> +static struct attribute *ltr659_attributes[] = {
> +	&iio_dev_attr_in_proximity_scale_available.dev_attr.attr,
> +	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	NULL
> +};

[Severity: Medium]
Should ltr659_channels also set BIT(IIO_CHAN_INFO_SAMP_FREQ) on the
IIO_PROXIMITY channel?

ltr659_attributes exposes sampling_frequency_available in sysfs, and
ltr501_read_raw() and __ltr501_write_raw() support IIO_CHAN_INFO_SAMP_FREQ for
IIO_PROXIMITY, but ltr659_channels only sets BIT(IIO_CHAN_INFO_RAW) and
BIT(IIO_CHAN_INFO_SCALE). Without BIT(IIO_CHAN_INFO_SAMP_FREQ) in the channel
spec, no sampling_frequency sysfs attribute is created for ltr659.

[ ... ]

> @@ -1283,6 +1330,15 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
>  		.channels = ltr501_channels,
>  		.no_channels = ARRAY_SIZE(ltr501_channels),
>  	},
> +	[ltr659] = {
> +		.partid = 0x09,
> +		.ps_gain = ltr559_ps_gain_tbl,
> +		.ps_gain_tbl_size = ARRAY_SIZE(ltr559_ps_gain_tbl),
> +		.info = &ltr659_info,
> +		.info_no_irq = &ltr659_info_no_irq,
> +		.channels = ltr659_channels,
> +		.no_channels = ARRAY_SIZE(ltr659_channels),
> +	},
>  };
>  
>  static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)

[Severity: Medium]
The commit message notes that ALS registers are marked as reserved on the
LTR-659PS-01 and interactions with them are discouraged, but won't
ltr501_init() and ltr501_write_contr() still read and write those registers?

During probe, ltr501_init() unconditionally reads LTR501_ALS_CONTR, calls
ltr501_read_intr_prst(data, IIO_INTENSITY, &data->als_period) which reads the
ALS persistence field and LTR501_ALS_MEAS_RATE, and then calls
ltr501_write_contr():

static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)
{
	int ret;

	ret = regmap_write(data->regmap, LTR501_ALS_CONTR, als_val);
	if (ret < 0)
		return ret;

	return regmap_write(data->regmap, LTR501_PS_CONTR, ps_val);
}

ltr501_write_contr() also writes LTR501_ALS_CONTR during ltr501_powerdown()
and ltr501_resume(). Should these ALS register accesses be skipped when
chip_info->als_gain is NULL?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101141507.119186-1-clamor95@gmail.com?part=2

Re: [PATCH v1 2/2] iio: light: ltr501: Add support for the LTR-659PS-01 proximity sensor

[Jonathan Cameron]

On Sun,  1 Nov 2026 16:15:07 +0200
Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> Add support for the Lite-On LTR-659PS-01 proximity sensor. This device is
> nearly identical to the LTR-559ALS-01, but omits all elements related to
> ambient light sensing (Even the regmap is identical, but ALS registers are
> marked as reserved and interactions with them are discouraged).
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
Hi Svyatoslav,

Various more general comments on the driver in here. Only real request
is to not copy the rather ugly existing indentation for new code.

If nothing else comes up in review, I can just tidy that up whilst applying.
For now I'm going to leave this on list to give others time to take a
look.

Thanks,

Jonathan

> ---
>  drivers/iio/light/ltr501.c | 58 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 302be2fc314c5..5cc1141322ad5 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -96,6 +96,7 @@ enum {
>  	ltr329,
>  	ltr501,
>  	ltr559,
> +	ltr659,
>  };
>  
>  struct ltr501_gain {
> @@ -649,6 +650,26 @@ static const struct iio_chan_spec ltr301_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(2),
>  };
>  
> +static const struct iio_chan_spec ltr659_channels[] = {
> +	{
> +		.type = IIO_PROXIMITY,
> +		.address = LTR501_PS_DATA,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = 11,
> +			.storagebits = 16,
> +			.endianness = IIO_CPU,
> +		},
> +		.event_spec = ltr501_pxs_event_spec,
> +		.num_event_specs = ARRAY_SIZE(ltr501_pxs_event_spec),
> +		.ext_info = ltr501_ext_info,
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +};
> +
>  static int ltr501_read_info_raw(struct ltr501_data *data,
>  				struct iio_chan_spec const *chan,
>  				int *val)
> @@ -1178,6 +1199,12 @@ static struct attribute *ltr301_attributes[] = {
>  	NULL
>  };
>  
> +static struct attribute *ltr659_attributes[] = {
> +	&iio_dev_attr_in_proximity_scale_available.dev_attr.attr,
> +	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	NULL
> +};
Not something I'm going to ask for as part of this series, but
if anyone working with this driver has time it would be good to
move the available handling over to the read_avail() callback
and masks.  This is an old driver and so far not been converted
to that newer infrastructure.

> +
>  static const struct attribute_group ltr501_attribute_group = {
>  	.attrs = ltr501_attributes,
>  };
> @@ -1186,6 +1213,10 @@ static const struct attribute_group ltr301_attribute_group = {
>  	.attrs = ltr301_attributes,
>  };
>  
> +static const struct attribute_group ltr659_attribute_group = {
> +	.attrs = ltr659_attributes,
> +};
> +
>  static const struct iio_info ltr501_info_no_irq = {
>  	.read_raw = ltr501_read_raw,
>  	.write_raw = ltr501_write_raw,
> @@ -1218,6 +1249,22 @@ static const struct iio_info ltr301_info = {
>  	.write_event_config	= &ltr501_write_event_config,
>  };
>  
> +static const struct iio_info ltr659_info_no_irq = {
> +	.read_raw = ltr501_read_raw,
> +	.write_raw = ltr501_write_raw,
> +	.attrs = &ltr659_attribute_group,
> +};
> +
> +static const struct iio_info ltr659_info = {
> +	.read_raw = ltr501_read_raw,
> +	.write_raw = ltr501_write_raw,
> +	.attrs = &ltr659_attribute_group,
> +	.read_event_value	= &ltr501_read_event,
> +	.write_event_value	= &ltr501_write_event,
> +	.read_event_config	= &ltr501_read_event_config,
> +	.write_event_config	= &ltr501_write_event_config,

The indenting of the existing structure is weird and could do with a tidy up.
I'd suggest just dropping any attempt to force alignment.  As is shown
here, doing that tends to lead to inconsistency over time as a driver
evolves.

> +};
> +
>  static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
>  	[ltr301] = {
>  		.partid = 0x08,
> @@ -1283,6 +1330,15 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
>  		.channels = ltr501_channels,
>  		.no_channels = ARRAY_SIZE(ltr501_channels),
>  	},

Another nice to have but not a request for this series.
Would be good to move away from handling these as an array with the enum
value in the firmware tables. That scales badly and runs into NULL check
problems when we use the generic firmware accessors (which we should really
be using here). i2c_get_match_data().  The standard way to do this
today is to use separate named structures and put pointers to those in the
tables.

> +	[ltr659] = {
> +		.partid = 0x09,
> +		.ps_gain = ltr559_ps_gain_tbl,
> +		.ps_gain_tbl_size = ARRAY_SIZE(ltr559_ps_gain_tbl),
> +		.info = &ltr659_info,
> +		.info_no_irq = &ltr659_info_no_irq,
> +		.channels = ltr659_channels,
> +		.no_channels = ARRAY_SIZE(ltr659_channels),
> +	},
>  };
>  
>  static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)
> @@ -1635,6 +1691,7 @@ static const struct i2c_device_id ltr501_id[] = {
>  	{ .name = "ltr329", .driver_data = ltr329 },
>  	{ .name = "ltr501", .driver_data = ltr501 },
>  	{ .name = "ltr559", .driver_data = ltr559 },
> +	{ .name = "ltr659", .driver_data = ltr659 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, ltr501_id);
> @@ -1645,6 +1702,7 @@ static const struct of_device_id ltr501_of_match[] = {
>  	{ .compatible = "liteon,ltr329", },
>  	{ .compatible = "liteon,ltr501", },
>  	{ .compatible = "liteon,ltr559", },
> +	{ .compatible = "liteon,ltr659", },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, ltr501_of_match);

Re: [PATCH v1 2/2] iio: light: ltr501: Add support for the LTR-659PS-01 proximity sensor

[Svyatoslav Ryhel]

нд, 1 лист. 2026 р. о 19:41 Jonathan Cameron <jic23@kernel.org> пише:
>
> On Sun,  1 Nov 2026 16:15:07 +0200
> Svyatoslav Ryhel <clamor95@gmail.com> wrote:
>
> > Add support for the Lite-On LTR-659PS-01 proximity sensor. This device is
> > nearly identical to the LTR-559ALS-01, but omits all elements related to
> > ambient light sensing (Even the regmap is identical, but ALS registers are
> > marked as reserved and interactions with them are discouraged).
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Hi Svyatoslav,
>
> Various more general comments on the driver in here. Only real request
> is to not copy the rather ugly existing indentation for new code.
>
> If nothing else comes up in review, I can just tidy that up whilst applying.
> For now I'm going to leave this on list to give others time to take a
> look.
>

I can address some of your suggestions in this series. I would like to
send v2 anyway since Sashiko found a few legit places needing a fix.

> Thanks,
>
> Jonathan
>
> > ---
> >  drivers/iio/light/ltr501.c | 58 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >
> > diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> > index 302be2fc314c5..5cc1141322ad5 100644
> > --- a/drivers/iio/light/ltr501.c
> > +++ b/drivers/iio/light/ltr501.c
> > @@ -96,6 +96,7 @@ enum {
> >       ltr329,
> >       ltr501,
> >       ltr559,
> > +     ltr659,
> >  };
> >
> >  struct ltr501_gain {
> > @@ -649,6 +650,26 @@ static const struct iio_chan_spec ltr301_channels[] = {
> >       IIO_CHAN_SOFT_TIMESTAMP(2),
> >  };
> >
> > +static const struct iio_chan_spec ltr659_channels[] = {
> > +     {
> > +             .type = IIO_PROXIMITY,
> > +             .address = LTR501_PS_DATA,
> > +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +                                   BIT(IIO_CHAN_INFO_SCALE),
> > +             .scan_index = 0,
> > +             .scan_type = {
> > +                     .sign = 'u',
> > +                     .realbits = 11,
> > +                     .storagebits = 16,
> > +                     .endianness = IIO_CPU,
> > +             },
> > +             .event_spec = ltr501_pxs_event_spec,
> > +             .num_event_specs = ARRAY_SIZE(ltr501_pxs_event_spec),
> > +             .ext_info = ltr501_ext_info,
> > +     },
> > +     IIO_CHAN_SOFT_TIMESTAMP(1),
> > +};
> > +
> >  static int ltr501_read_info_raw(struct ltr501_data *data,
> >                               struct iio_chan_spec const *chan,
> >                               int *val)
> > @@ -1178,6 +1199,12 @@ static struct attribute *ltr301_attributes[] = {
> >       NULL
> >  };
> >
> > +static struct attribute *ltr659_attributes[] = {
> > +     &iio_dev_attr_in_proximity_scale_available.dev_attr.attr,
> > +     &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> > +     NULL
> > +};
> Not something I'm going to ask for as part of this series, but
> if anyone working with this driver has time it would be good to
> move the available handling over to the read_avail() callback
> and masks.  This is an old driver and so far not been converted
> to that newer infrastructure.
>

This I don't promise, I will see what I can do. Maybe you have a good example?

> > +
> >  static const struct attribute_group ltr501_attribute_group = {
> >       .attrs = ltr501_attributes,
> >  };
> > @@ -1186,6 +1213,10 @@ static const struct attribute_group ltr301_attribute_group = {
> >       .attrs = ltr301_attributes,
> >  };
> >
> > +static const struct attribute_group ltr659_attribute_group = {
> > +     .attrs = ltr659_attributes,
> > +};
> > +
> >  static const struct iio_info ltr501_info_no_irq = {
> >       .read_raw = ltr501_read_raw,
> >       .write_raw = ltr501_write_raw,
> > @@ -1218,6 +1249,22 @@ static const struct iio_info ltr301_info = {
> >       .write_event_config     = &ltr501_write_event_config,
> >  };
> >
> > +static const struct iio_info ltr659_info_no_irq = {
> > +     .read_raw = ltr501_read_raw,
> > +     .write_raw = ltr501_write_raw,
> > +     .attrs = &ltr659_attribute_group,
> > +};
> > +
> > +static const struct iio_info ltr659_info = {
> > +     .read_raw = ltr501_read_raw,
> > +     .write_raw = ltr501_write_raw,
> > +     .attrs = &ltr659_attribute_group,
> > +     .read_event_value       = &ltr501_read_event,
> > +     .write_event_value      = &ltr501_write_event,
> > +     .read_event_config      = &ltr501_read_event_config,
> > +     .write_event_config     = &ltr501_write_event_config,
>
> The indenting of the existing structure is weird and could do with a tidy up.
> I'd suggest just dropping any attempt to force alignment.  As is shown
> here, doing that tends to lead to inconsistency over time as a driver
> evolves.
>

This I will address as a separate commit before adding ltr659

> > +};
> > +
> >  static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
> >       [ltr301] = {
> >               .partid = 0x08,
> > @@ -1283,6 +1330,15 @@ static const struct ltr501_chip_info ltr501_chip_info_tbl[] = {
> >               .channels = ltr501_channels,
> >               .no_channels = ARRAY_SIZE(ltr501_channels),
> >       },
>
> Another nice to have but not a request for this series.
> Would be good to move away from handling these as an array with the enum
> value in the firmware tables. That scales badly and runs into NULL check
> problems when we use the generic firmware accessors (which we should really
> be using here). i2c_get_match_data().  The standard way to do this
> today is to use separate named structures and put pointers to those in the
> tables.
>

This I will address as well.

> > +     [ltr659] = {
> > +             .partid = 0x09,
> > +             .ps_gain = ltr559_ps_gain_tbl,
> > +             .ps_gain_tbl_size = ARRAY_SIZE(ltr559_ps_gain_tbl),
> > +             .info = &ltr659_info,
> > +             .info_no_irq = &ltr659_info_no_irq,
> > +             .channels = ltr659_channels,
> > +             .no_channels = ARRAY_SIZE(ltr659_channels),
> > +     },
> >  };
> >
> >  static int ltr501_write_contr(struct ltr501_data *data, u8 als_val, u8 ps_val)
> > @@ -1635,6 +1691,7 @@ static const struct i2c_device_id ltr501_id[] = {
> >       { .name = "ltr329", .driver_data = ltr329 },
> >       { .name = "ltr501", .driver_data = ltr501 },
> >       { .name = "ltr559", .driver_data = ltr559 },
> > +     { .name = "ltr659", .driver_data = ltr659 },
> >       { }
> >  };
> >  MODULE_DEVICE_TABLE(i2c, ltr501_id);
> > @@ -1645,6 +1702,7 @@ static const struct of_device_id ltr501_of_match[] = {
> >       { .compatible = "liteon,ltr329", },
> >       { .compatible = "liteon,ltr501", },
> >       { .compatible = "liteon,ltr559", },
> > +     { .compatible = "liteon,ltr659", },
> >       { }
> >  };
> >  MODULE_DEVICE_TABLE(of, ltr501_of_match);
>