Re: [PATCH 10/11] staging: iio: ad2s1200: Replace angle channel with inclination channel

[David Julian Veenstra <davidjulianveenstra@gmail.com>]

On 23, March 2018 14:27, Jonathan Cameron wrote:

> On Sun, 18 Mar 2018 14:37:04 +0100
> David Veenstra <davidjulianveenstra@gmail.com> wrote:
>
>> The angle channel is not defined in sysfs iio ABI. So it is replaced
>> with an inclination channel, because it is defined in the ABI, and has the
>> semantics of an angle.
>> 
>> In addition, a fractional scaling factor of 360 / (2^12 -1) is added,
>> to scale the 12-bits angular position to degrees, conform the ABI.
>> 
>> Signed-off-by: David Veenstra <davidjulianveenstra@gmail.com>
> No, please do not blugeon a device into the existing documented ABI.
>
> A resolver does not measure something that can be remotely
> sensibly mapped to inclination.  Inclination is relative to 'down'.
> A resolver doesn't care about down.
>
> Add an angle type to the ABI docs. It simply hasn't been documented
> before because there were no drivers outside staging that use it.

I'm sorry, I misunderstood our previous discussing on this topic
(https://marc.info/?l=linux-iio&m=152087432322446&w=2) as saying:
"there already exists another channel type that is a good enough match".

The in_incli and in_rot_from_* family all use degrees as their unit.
For V2 I'll change it back to an angle channel and use angle as its
unit.

Best regards,
David Veenstra

>
> Jonathan
>
>> ---
>>  drivers/staging/iio/resolver/ad2s1200.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/resolver/ad2s1200.c b/drivers/staging/iio/resolver/ad2s1200.c
>> index 4b97a106012c..937676bcc0d4 100644
>> --- a/drivers/staging/iio/resolver/ad2s1200.c
>> +++ b/drivers/staging/iio/resolver/ad2s1200.c
>> @@ -64,6 +64,10 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  	switch (m) {
>>  	case IIO_CHAN_INFO_SCALE:
>>  		switch (chan->type) {
>> +		case IIO_INCLI:
>> +			*val = 360;
>> +			*val2 = 0xFFF;
>> +			return IIO_VAL_FRACTIONAL;
>>  		case IIO_ANGL_VEL:
>>  			/*
>>  			 * 2 * Pi is almost equal to
>> @@ -91,7 +95,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  		/* delay (6 * AD2S1200_TSCLK + 20) nano seconds */
>>  		udelay(1);
>>  		gpiod_set_value(st->sample, 1);
>> -		gpiod_set_value(st->rdvel, !!(chan->type == IIO_ANGL));
>> +		gpiod_set_value(st->rdvel, !!(chan->type == IIO_INCLI));
>>  
>>  		ret = spi_read(st->sdev, st->rx, 2);
>>  		if (ret < 0) {
>> @@ -101,7 +105,7 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  
>>  		vel = be16_to_cpup((__be16 *)st->rx);
>>  		switch (chan->type) {
>> -		case IIO_ANGL:
>> +		case IIO_INCLI:
>>  			*val = vel >> 4;
>>  			ret = IIO_VAL_INT;
>>  			break;
>> @@ -128,10 +132,11 @@ static int ad2s1200_read_raw(struct iio_dev *indio_dev,
>>  
>>  static const struct iio_chan_spec ad2s1200_channels[] = {
>>  	{
>> -		.type = IIO_ANGL,
>> +		.type = IIO_INCLI,
>>  		.indexed = 1,
>>  		.channel = 0,
>>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>>  	}, {
>>  		.type = IIO_ANGL_VEL,
>>  		.indexed = 1,