Hi Jonathan
Happy new year!
Friendly ping about this patch so we can change the ABI before the
kernel release happens
On Thu, 19 Dec 2024 at 18:17, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Mon, 16 Dec 2024 10:05:53 +0000
> Ricardo Ribalda <ribalda@xxxxxxxxxxxx> wrote:
>
> > When the driver was originally created, it was decided that
> > sampling_frequency and hysteresis would be shared_per_type instead
> > of shared_by_all (even though it is internally shared by all). Eg:
> > in_proximity_raw
> > in_proximity_sampling_frequency
> >
> > When we introduced support for more channels, we continued with
> > shared_by_type which. Eg:
> > in_proximity0_raw
> > in_proximity1_raw
> > in_proximity_sampling_frequency
> > in_attention_raw
> > in_attention_sampling_frequency
> >
> > Ideally we should change to shared_by_all, but it is not an option,
> > because the current naming has been a stablished ABI by now. Luckily we
> > can use separate instead. That will be more consistent:
> > in_proximity0_raw
> > in_proximity0_sampling_frequency
> > in_proximity1_raw
> > in_proximity1_sampling_frequency
> > in_attention_raw
> > in_attention_sampling_frequency
> >
> > Fixes: 596ef5cf654b ("iio: hid-sensor-prox: Add support for more channels")
> > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
>
> I got lost somewhere in the discussion. This is still an ABI change compared
> to original interface at the top (which is the one that has been there
> quite some time).
>
> However we already had to make one of those to add the index that wasn't there
> for _raw. (I'd missed that in earlier discussion - thanks for laying out the
> steps here!) Srinivas, Jiri, do you think we are better off just assuming users
> of this will be using a library that correctly deals with sharing and just
> jump to
> in_proximity0_raw
> in_proximity1_raw
> in_attention_raw
> (should have indexed that but it may never matter) and
> sampling_frequency
>
> Which is what I think Ricardo originally asked.
>
> Do we have any guarantee the sampling_frequency will be shared across the
> sensor channels? It may be the most common situation but I don't want to
> wall us into a corner if it turns out someone runs separate sensors at
> different rates (no particularly reason they should be one type of sensor
> so this might make sense). If we don't have that guarantee
> then this patch is fine as far as I'm concerned.
>
> Jonathan
>
>
>
> > ---
> > Changes in v2:
> > - Use separate
> > - Link to v1: https://lore.kernel.org/r/20241205-fix-hid-sensor-v1-1-9b789f39c220@xxxxxxxxxxxx
> > ---
> > drivers/iio/light/hid-sensor-prox.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> > index c83acbd78275..71dcef3fbe57 100644
> > --- a/drivers/iio/light/hid-sensor-prox.c
> > +++ b/drivers/iio/light/hid-sensor-prox.c
> > @@ -49,9 +49,10 @@ static const u32 prox_sensitivity_addresses[] = {
> > #define PROX_CHANNEL(_is_proximity, _channel) \
> > {\
> > .type = _is_proximity ? IIO_PROXIMITY : IIO_ATTENTION,\
> > - .info_mask_separate = _is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
> > - BIT(IIO_CHAN_INFO_PROCESSED),\
> > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |\
> > + .info_mask_separate = \
> > + (_is_proximity ? BIT(IIO_CHAN_INFO_RAW) :\
> > + BIT(IIO_CHAN_INFO_PROCESSED)) |\
> > + BIT(IIO_CHAN_INFO_OFFSET) |\
> > BIT(IIO_CHAN_INFO_SCALE) |\
> > BIT(IIO_CHAN_INFO_SAMP_FREQ) |\
> > BIT(IIO_CHAN_INFO_HYSTERESIS),\
> >
> > ---
> > base-commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
> > change-id: 20241203-fix-hid-sensor-62e1979ecd03
> >
> > Best regards,
>
--
Ricardo Ribalda
