Messages in this thread Patch in this message | | | Date | Fri, 22 Jul 2022 09:52:34 +0100 | From | Paul Cercueil <> | Subject | Re: [PATCH 3/4] iio: afe/rescale: Add support for converting scale avail table |
| |
Hi Peter,
Le ven., juil. 22 2022 at 00:16:31 +0200, Peter Rosin <peda@axentia.se> a écrit : > Hi! > > 2022-07-21 at 21:15, Paul Cercueil wrote: >> When the IIO channel has a scale_available attribute, we want the >> values >> contained to be properly converted the same way the scale value is. >> >> Since rescale_process_scale() may change the encoding type, we must >> convert the IIO_AVAIL_LIST to a IIO_AVAIL_LIST_WITH_TYPE. >> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> --- >> drivers/iio/afe/iio-rescale.c | 85 >> +++++++++++++++++++++++++++++++++ >> include/linux/iio/afe/rescale.h | 2 + >> 2 files changed, 87 insertions(+) >> >> diff --git a/drivers/iio/afe/iio-rescale.c >> b/drivers/iio/afe/iio-rescale.c >> index 6949d2151025..5c9970b93384 100644 >> --- a/drivers/iio/afe/iio-rescale.c >> +++ b/drivers/iio/afe/iio-rescale.c >> @@ -232,6 +232,18 @@ static int rescale_read_avail(struct iio_dev >> *indio_dev, >> *type = IIO_VAL_INT; >> return iio_read_avail_channel_raw(rescale->source, >> vals, length); >> + case IIO_CHAN_INFO_SCALE: >> + if (rescale->chan_processed) { > > I think it is wrong to simply feed the info-scale to the source > channel if it > happens to be processed. It still needs the inverse rescale. But see > below.
Yes, when I started working on that patchset, processed channels weren't a thing, and I don't think I understood what they are about.
> >> + return iio_read_avail_channel_attribute(rescale->source, >> + vals, type, >> + length, >> + IIO_CHAN_INFO_SCALE); >> + } else if (rescale->scale_len) { >> + *length = rescale->scale_len; >> + *vals = rescale->scale_data; >> + return IIO_AVAIL_LIST_WITH_TYPE; >> + } >> + fallthrough; >> default: >> return -EINVAL; >> } >> @@ -266,11 +278,74 @@ static ssize_t rescale_write_ext_info(struct >> iio_dev *indio_dev, >> buf, len); >> } >> >> +static int rescale_init_scale_avail(struct device *dev, struct >> rescale *rescale) >> +{ >> + int ret, type, length, *data; >> + const int *scale_raw; >> + unsigned int i; >> + size_t out_len; >> + >> + ret = iio_read_avail_channel_attribute(rescale->source, >> &scale_raw, >> + &type, &length, >> + IIO_CHAN_INFO_SCALE); >> + if (ret < 0) >> + return ret; >> + >> + switch (ret) { >> + case IIO_AVAIL_LIST_WITH_TYPE: >> + out_len = length; >> + break; >> + case IIO_AVAIL_LIST: >> + if (type == IIO_VAL_INT) >> + out_len = length * 3 / 1; >> + else >> + out_len = length * 3 / 2; >> + break; >> + default: >> + /* TODO: Support IIO_AVAIL_RANGE */ >> + return -EOPNOTSUPP; >> + } >> + >> + data = devm_kzalloc(dev, sizeof(*data) * out_len, GFP_KERNEL); >> + if (!data) >> + return -ENOMEM; >> + >> + if (ret == IIO_AVAIL_LIST_WITH_TYPE) { >> + memcpy(data, scale_raw, sizeof(*scale_raw) * length); >> + } else if (type == IIO_VAL_INT) { >> + for (i = 0; i < length; i++) { >> + data[i * 3 + 0] = scale_raw[i]; >> + data[i * 3 + 2] = IIO_VAL_INT; >> + } >> + } else { >> + for (i = 0; i < length / 2; i++) { >> + data[i * 3 + 0] = scale_raw[i * 2]; >> + data[i * 3 + 1] = scale_raw[i * 2 + 1]; >> + data[i * 3 + 2] = type; >> + } >> + } >> + >> + for (i = 0; i < out_len; i += 3) { >> + ret = rescale_process_scale(rescale, data[i + 2], >> + &data[i], &data[i + 1]); >> + if (ret < 0) >> + return ret; >> + >> + data[i + 2] = ret; >> + } >> + >> + rescale->scale_len = out_len; >> + rescale->scale_data = data; >> + >> + return 0; >> +} >> + >> static int rescale_configure_channel(struct device *dev, >> struct rescale *rescale) >> { >> struct iio_chan_spec *chan = &rescale->chan; >> struct iio_chan_spec const *schan = rescale->source->channel; >> + int ret; >> >> chan->indexed = 1; >> chan->output = schan->output; >> @@ -303,6 +378,16 @@ static int rescale_configure_channel(struct >> device *dev, >> !rescale->chan_processed) >> chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW); >> >> + if (iio_channel_has_available(schan, IIO_CHAN_INFO_SCALE)) { >> + chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_SCALE); >> + >> + if (!rescale->chan_processed) { >> + ret = rescale_init_scale_avail(dev, rescale); >> + if (ret) >> + return ret; >> + } >> + } >> + > > Does a (sane) processed channel really have a scale? That seems a bit > fringe. > Wouldn't it be better to conditionally publish availability of > info-scale so > that it isn't visible for processed channels and dodge that > rabbit-hole? In > either case, the above commented implementation of info-scale for > rescaled > processed channels is wrong (I think...).
I could set the IIO_CHAN_INFO_SCALE only for non-processed channels, since this is what I can test with.
Cheers, -Paul
>> return 0; >> } >> >> diff --git a/include/linux/iio/afe/rescale.h >> b/include/linux/iio/afe/rescale.h >> index 6eecb435488f..74de2962f864 100644 >> --- a/include/linux/iio/afe/rescale.h >> +++ b/include/linux/iio/afe/rescale.h >> @@ -26,6 +26,8 @@ struct rescale { >> s32 numerator; >> s32 denominator; >> s32 offset; >> + int scale_len; >> + int *scale_data; >> }; >> >> int rescale_process_scale(struct rescale *rescale, int scale_type,
| |