| Message ID | 20201125084606.11404-2-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: core: Copy iio_info.attrs->is_visible into iio_dev_opaque.chan_attr_group.is_visible | expand |
On Wed, Nov 25, 2020 at 10:47 AM Hans de Goede <hdegoede@redhat.com> wrote: > > The iio-core extends the attr_group provided by the driver with its > own attributes. To be able to do this it: > > 1. Has its own (non const) io_dev_opaque.chan_attr_group attr_group struct > 2. It allocates a new attrs array with room for both the drivers and its > own attributes > 3. It copies over the driver provided attributes into the newly allocated > attrs array. > > But the drivers attr_group may contain more then just the attrs array, it > may also contain an is_visible callback and at least the adi-axi-adc.c > is currently defining such a callback. > > Change the attr_group copying code to also copy over the is_visible > callback, so that drivers can define one and have it workins as is > normal for attr_group-s all over the kernel. > > Note that the is_visible callback takes an index into the array as > argument, so that indices of the driver's attributes must not change, > this is not a problem as the driver's own attributes are added first > to the newly allocated attrs array and the attributes handled by the > core are appended after the driver's attributes. > Sorry for missing this earlier. I'm terrible with tracking emails sometimes. > Cc: Michael Hennerich <michael.hennerich@analog.com> > Cc: Alexandru Ardelean <alexandru.ardelean@analog.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/iio/industrialio-core.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index 261d3b17edc9..7943d0545b61 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -1466,11 +1466,14 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev) > goto error_clear_attrs; > } > /* Copy across original attributes */ > - if (indio_dev->info->attrs) > + if (indio_dev->info->attrs) { > memcpy(iio_dev_opaque->chan_attr_group.attrs, > indio_dev->info->attrs->attrs, > sizeof(iio_dev_opaque->chan_attr_group.attrs[0]) > *attrcount_orig); > + iio_dev_opaque->chan_attr_group.is_visible = > + indio_dev->info->attrs->is_visible; So, I think I was pretty silly at the time, and did not fully know how IIO handles attributes. But I can see the bug now [in adi-axi-adc]. Thanks for identifying it :) As an initial handling, this looks good. I think this also means that we should check and see where else we may need to do this. Maybe, we should do a utility that handles this atribute_group copying. But for this one as-is: Acked-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > + } > attrn = attrcount_orig; > /* Add all elements from the list. */ > list_for_each_entry(p, &iio_dev_opaque->channel_attr_list, l) > -- > 2.28.0 >
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index 261d3b17edc9..7943d0545b61 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -1466,11 +1466,14 @@ static int iio_device_register_sysfs(struct iio_dev *indio_dev) goto error_clear_attrs; } /* Copy across original attributes */ - if (indio_dev->info->attrs) + if (indio_dev->info->attrs) { memcpy(iio_dev_opaque->chan_attr_group.attrs, indio_dev->info->attrs->attrs, sizeof(iio_dev_opaque->chan_attr_group.attrs[0]) *attrcount_orig); + iio_dev_opaque->chan_attr_group.is_visible = + indio_dev->info->attrs->is_visible; + } attrn = attrcount_orig; /* Add all elements from the list. */ list_for_each_entry(p, &iio_dev_opaque->channel_attr_list, l)
The iio-core extends the attr_group provided by the driver with its own attributes. To be able to do this it: 1. Has its own (non const) io_dev_opaque.chan_attr_group attr_group struct 2. It allocates a new attrs array with room for both the drivers and its own attributes 3. It copies over the driver provided attributes into the newly allocated attrs array. But the drivers attr_group may contain more then just the attrs array, it may also contain an is_visible callback and at least the adi-axi-adc.c is currently defining such a callback. Change the attr_group copying code to also copy over the is_visible callback, so that drivers can define one and have it workins as is normal for attr_group-s all over the kernel. Note that the is_visible callback takes an index into the array as argument, so that indices of the driver's attributes must not change, this is not a problem as the driver's own attributes are added first to the newly allocated attrs array and the attributes handled by the core are appended after the driver's attributes. Cc: Michael Hennerich <michael.hennerich@analog.com> Cc: Alexandru Ardelean <alexandru.ardelean@analog.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/iio/industrialio-core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)