Re: [PATCH v2 1/4] iio: light: ltr501: Switch to read_avail() op from manual attribute registration

[Svyatoslav Ryhel]

сб, 7 лист. 2026 р. о 05:20 Jonathan Cameron <jic23@xxxxxxxxxx> пише:
>
> On Fri,  6 Nov 2026 11:19:40 +0200
> Svyatoslav Ryhel <clamor95@xxxxxxxxx> wrote:
>
> > Switch to the read_avail() operation from manual attribute group
> > registration. This eliminates the need for per-chip attribute_group
> > structures and related ltr501_chip_info iio_info entries.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> Given you were quick off the mark on sashiko on v1, I guess
> you already saw:
> https://sashiko.dev/#/patchset/20261106091943.35783-1-clamor95%40gmail.com
>
> It seems correct that you'll only get one attribute where there were two
> before.  I suspect it's as simple as the mask should be set of
> shared_by_type_available not shared_by_all for scale and shared_by_all
> for only sampling frequency and integration time.
>
> The current code is odd in having per type sampling_freq and integration
> time as both apply to the light channel as well I think.  Not vital
> to fix that though so I'd just split the mask to make it work for both
> as it does today.
>

-       .info_mask_shared_by_all_available = (_shared), \
+       .info_mask_shared_by_type_available = (_avail_type), \
+       .info_mask_shared_by_all_available = (_avail_all), \
...

 static const struct iio_chan_spec ltr501_channels[] = {
        LTR501_LIGHT_CHANNEL(),
-       LTR501_INTENSITY_CHANNEL(0, LTR501_ALS_DATA0, IIO_MOD_LIGHT_BOTH, 0,
+       LTR501_INTENSITY_CHANNEL(0, LTR501_ALS_DATA0,
IIO_MOD_LIGHT_BOTH, 0, 0, 0,
                                 ltr501_als_event_spec,
                                 ARRAY_SIZE(ltr501_als_event_spec)),
        LTR501_INTENSITY_CHANNEL(1, LTR501_ALS_DATA1, IIO_MOD_LIGHT_IR,
                                 BIT(IIO_CHAN_INFO_SCALE) |
                                 BIT(IIO_CHAN_INFO_INT_TIME) |
                                 BIT(IIO_CHAN_INFO_SAMP_FREQ),
-                                NULL, 0),
+                                BIT(IIO_CHAN_INFO_SCALE),
+                                BIT(IIO_CHAN_INFO_INT_TIME) |
+                                BIT(IIO_CHAN_INFO_SAMP_FREQ), NULL, 0),
        {
                .type = IIO_PROXIMITY,
                .address = LTR501_PS_DATA,
                .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
                        BIT(IIO_CHAN_INFO_SCALE),
+               .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
                .scan_index = 2,
                .scan_type = {

IIO_CHAN_INFO_SCALE is by type and IIO_CHAN_INFO_INT_TIME and
IIO_CHAN_INFO_SAMP_FREQ by all

> The current way the two Intensity channels is done is very odd.
> The .info_mask_shared_by_type should be the same for both of them
> (only one set of attrs will be created).
> Ideally fix that as a precursor patch (don't label it as a fix
> though as it's just a code consistency thing rather than actually
> resulting in a change.)
>

No more precursor patches! I am just a random contributor, not a
driver author, not a subsystem maintainer, not even a random code
janitor. Stop using me for random unrelated fixes!

>
> > ---
> >  drivers/iio/light/ltr501.c | 153 +++++++++++++------------------------
> >  1 file changed, 52 insertions(+), 101 deletions(-)
> >
>
> >  static const struct iio_info ltr501_info = {
> >       .read_raw = ltr501_read_raw,
> >       .write_raw = ltr501_write_raw,
> > -     .attrs = &ltr501_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 iio_info ltr301_info_no_irq = {
> > -     .read_raw = ltr501_read_raw,
> > -     .write_raw = ltr501_write_raw,
> > -     .attrs = &ltr301_attribute_group,
> > -};
> > -
> > -static const struct iio_info ltr301_info = {
> > -     .read_raw = ltr501_read_raw,
> > -     .write_raw = ltr501_write_raw,
> > -     .attrs = &ltr301_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,
> > +     .read_avail = ltr501_read_avail,
> > +     .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,
>
> I'd prefer a precursor patch tidying up the indent as that should make
> the diff on this patch easier to read.
>

NO, either this, or I can drop alignment adjustment and you can fix it
on your own whenever you like.

>
> >  };
>