On Wed, 12 Jun 2024 16:20:47 +0200 Nuno Sa <nuno.sa@xxxxxxxxxx> wrote: > Hi Jonathan, > > In [1], you suggested for an iterator for the active channels (so driver > don't directly access masklength). This RFC showcases that iterator and > goes one step further by giving an accessors for masklength so that > drivers can read that variable (we have drivers doing that). The > accessors uses ACCESS_PRIVATE() so it will warn us if some driver > directly access the variable making it more difficult to mess with it > (like changing it's value) without being noticed during review (or the > auto builders). > > Anyways, before jumping in changing all the drivers using this, I guess > the questions are: > > 1) Is the iterator useful enough to add one (kind of like it and save a > line of code :))? > 2) Do we care about going with the work of marking masklength private? > > If we go ahead the plan would be: > > 1) Add the helpers macros; > 2) Convert all drivers that directly access 'masklength'; > 3) Annotate it as private. > > [1]: https://lore.kernel.org/linux-iio/20240428142343.5067c898@jic23-huawei/ Cute. I'd not seen the __private bit before. Looks good to me. I think we should spin it a little differently. 1. Add macro and a dummy #define iio_dev_mask_length(indio_dev) (indio_dev)->mask_length 2. Convert drivers 3. What you have + the ACCESS_PRIVATE change. that accessor still lets people change it rather than making it strictly private. I wonder if we need a little more complicated static inline int iio_dev_mask_length(struct iio_dev *indio_dev) { return ACCESS_PRIVATE()... } or can just review for anyone doing iio_dev_mask_length(indio_dev) = 4; > > --- > Nuno Sa (3): > iio: core: add new helper to iterate active channels > iio: imu: adis16475: make use of iio_for_each_active_channel() > iio: core annotate masklength as private > > drivers/iio/imu/adis16475.c | 3 +-- > include/linux/iio/iio.h | 8 +++++++- > 2 files changed, 8 insertions(+), 3 deletions(-) > --- > base-commit: cc1ce839526a65620778617da0b022bd88e8a139 > change-id: 20240612-dev-iio-scan-private-86f4a0fd288f > -- > > Thanks! > - Nuno Sá >
