Re: [PATCH 2/2] iio: afe: rescale: Implement write_raw

[Peter Rosin <peda@axentia.se>]

Hi!

Sorry for the delay. Vacation...

On 2020-07-26 14:41, Jonathan Cameron wrote:
> On Tue, 21 Jul 2020 01:16:55 +0200
> Paul Cercueil <paul@crapouillou.net> wrote:
> 
>> Hi Jonathan,
>>
>> Le sam. 15 févr. 2020 à 18:32, Jonathan Cameron <jic23@kernel.org> a 
>> écrit :
>>> On Mon, 10 Feb 2020 19:54:38 -0300
>>> Paul Cercueil <paul@crapouillou.net> wrote:
>>>   
>>>>  Implement write_raw by converting the value if writing the scale, or
>>>>  just calling the managed channel driver's write_raw otherwise.
>>>>
>>>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>>>  ---
>>>>   drivers/iio/afe/iio-rescale.c | 22 ++++++++++++++++++++++
>>>>   1 file changed, 22 insertions(+)
>>>>
>>>>  diff --git a/drivers/iio/afe/iio-rescale.c 
>>>> b/drivers/iio/afe/iio-rescale.c
>>>>  index 95802d9ee25e..a48f6af9316d 100644
>>>>  --- a/drivers/iio/afe/iio-rescale.c
>>>>  +++ b/drivers/iio/afe/iio-rescale.c
>>>>  @@ -35,6 +35,27 @@ struct rescale {
>>>>   	int *scale_data;
>>>>   };
>>>>
>>>>  +static int rescale_write_raw(struct iio_dev *indio_dev,
>>>>  +			     struct iio_chan_spec const *chan,
>>>>  +			     int val, int val2, long mask)
>>>>  +{
>>>>  +	struct rescale *rescale = iio_priv(indio_dev);
>>>>  +	unsigned long long tmp;
>>>>  +
>>>>  +	switch (mask) {
>>>>  +	case IIO_CHAN_INFO_SCALE:
>>>>  +		tmp = val * 1000000000LL;
>>>>  +		do_div(tmp, rescale->numerator);
>>>>  +		tmp *= rescale->denominator;
>>>>  +		do_div(tmp, 1000000000LL);
>>>>  +		return iio_write_channel_attribute(rescale->source, tmp, 0,
>>>>  +						   IIO_CHAN_INFO_SCALE);  
>>>
>>> Why is val2 always 0?  Won't that only work if the backend device
>>> has integer scales?  
>>
>> Sorry, somehow I didn't see your answer.
>>
>> Indeed, this will only work if the backend device has integer scales, 
>> but what should I do? Just pass 'val2' instead of 0? Will the value be 
>> correct if I only apply the scale ratio to 'val'?
> 
> I think you'll need to include it through the calculation. Given you
> premultiply by 1000000000LL it should be easy enough to do.
> Then for the final do_div you can easily work out the val2 part.
> 
> I'm not sure we currently have an inkern interface to get the type
> of the channel attribute value though.  You may need to add one.

Right, I didn't originally add scaled writing as
1. I don't need it.
2. It's a rats nest, IIRC some drivers are picky in what they take and
   you would need some kind of matrix of how to best handle the different
   conversion cases. I just didn't want to go there, and this patch
   feels far too simple to be adequate. But then again, maybe my memory is
   poorer that I thought...

Cheers,
Peter