On Wed, Nov 25, 2020 at 10:47 AM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxxx>
> Cc: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
> + }
> attrn = attrcount_orig;
> /* Add all elements from the list. */
> list_for_each_entry(p, &iio_dev_opaque->channel_attr_list, l)
> --
> 2.28.0
>
