Re: [PATCH 1/5] iio: temperature: mcp9600: set channel2 member

[Dimitri Fedrau <dima.fedrau@gmail.com>]

Am Sun, May 05, 2024 at 11:15:41AM +0100 schrieb Jonathan Cameron:
> On Tue, 30 Apr 2024 14:21:57 +0200
> Dimitri Fedrau <dima.fedrau@gmail.com> wrote:
> 
> > Am Tue, Apr 30, 2024 at 01:11:02PM +0100 schrieb Jonathan Cameron:
> > > On Tue, 30 Apr 2024 14:05:31 +0200
> > > Dimitri Fedrau <dima.fedrau@gmail.com> wrote:
> > >   
> > > > Set channel2 member of channel 0 to IIO_MOD_TEMP_OBJECT and set modified
> > > > member to 1.  
> > > This an ABI change, so needs a strong argument + must be a fix 
> > > rather than an improvement.  So why does this need to change?
> > >  
> > Hi Jonathan,
> > 
> > I don't know if it is an valid argument but when using tool "iio_info"
> > the temp_object wasn't displayed at all. After adding these two lines
> > the temp_object is displayed. Don't know if it is a problem with the
> > userspace tools.
> 
> Just to check, it displayed not temperature channel for this?
>
It did it correctly when reading from /sys/bus/iio/devices/iio\:device0
but when using "iio_info" it didn't. See below.

> If you could send the file listing of the appropriate
> /sys/bus/iio/devices/iio\:deviceX/ directory that would be great.
> 

root@raspberrypi3-64:~# ls -al /sys/bus/iio/devices/iio\:device0/
drwxr-xr-x    3 root     root             0 May  4 20:11 .
drwxr-xr-x    4 root     root             0 May  4 20:11 ..
-rw-r--r--    1 root     root          4096 May  4 20:11 in_temp_ambient_raw
-rw-r--r--    1 root     root          4096 May  4 20:11 in_temp_ambient_scale
-rw-r--r--    1 root     root          4096 May  4 20:11 in_temp_raw
-rw-r--r--    1 root     root          4096 May  4 20:11 in_temp_scale
-r--r--r--    1 root     root          4096 May  4 20:11 name
lrwxrwxrwx    1 root     root             0 May  4 20:11 of_node -> ../../../../../../../firmware/devicetree/base/soc/i2c@7e804000/temperature-sensor@67
drwxr-xr-x    2 root     root             0 May  4 20:11 power
lrwxrwxrwx    1 root     root             0 May  4 20:11 subsystem -> ../../../../../../../bus/iio
-rw-r--r--    1 root     root          4096 May  4 20:11 uevent
-r--r--r--    1 root     root          4096 May  4 20:11 waiting_for_supplier

> It is possible the tools don't cope with a mixture of modified and unmodified
> channels (without index).  Whilst the ABI docs don't say you can't do this
> it is a rather obscure corner case.
> 

I think you are right, below are my findings. When doing a cat from
in_temp_ambient_raw or in_temp_raw the results are correct, when using
iio_info there is only a single channel displayed with four attributes.

root@raspberrypi3-64:~# cat /sys/bus/iio/devices/iio\:device0/in_temp_ambient_raw; cat /sys/bus/iio/devices/iio\:device0/in_temp_raw; iio_info
314
540
iio_info version: 0.25 (git tag:b6028fde)
Libiio version: 0.25 (git tag: b6028fd) backends: local xml ip usb serial
IIO context created with local backend.
Backend version: 0.25 (git tag: b6028fd)
Backend description string: Linux raspberrypi3-64 6.9.0-rc1-g7584c270afae-dirty #239 SMP PREEMPT Thu May  9 20:11:36 CEST 2024 aarch64
IIO context has 2 attributes:
        local,kernel: 6.9.0-rc1-g7584c270afae-dirty
        uri: local:
IIO context has 3 devices:
        hwmon0: cpu_thermal
                1 channels found:
                        temp1:  (input)
                        1 channel-specific attributes found:
                                attr  0: input value: 42932
                No trigger on this device
        hwmon1: rpi_volt
                1 channels found:
                        in0:  (input)
                        1 channel-specific attributes found:
                                attr  0: lcrit_alarm value: 0
                No trigger on this device
        iio:device0: mcp9600
                1 channels found:
                        temp_ambient:  (input)
                        4 channel-specific attributes found:
                                attr  0: raw value: 314
                                attr  1: raw value: 314
                                attr  2: scale value: 62.500000
                                attr  3: scale value: 62.500000
                1 device-specific attributes found:
                                attr  0: waiting_for_supplier value: 0
                No trigger on this device




With the patch setting modified and channel2:


root@raspberrypi3-64:~# ls -al /sys/bus/iio/devices/iio\:device0/
drwxr-xr-x    3 root     root             0 May  9 18:21 .
drwxr-xr-x    4 root     root             0 May  9 18:21 ..
-rw-r--r--    1 root     root          4096 May  9 18:22 in_temp_ambient_raw
-rw-r--r--    1 root     root          4096 May  9 18:22 in_temp_ambient_scale
-rw-r--r--    1 root     root          4096 May  9 18:22 in_temp_object_raw
-rw-r--r--    1 root     root          4096 May  9 18:22 in_temp_object_scale
-r--r--r--    1 root     root          4096 May  9 18:21 name
lrwxrwxrwx    1 root     root             0 May  9 18:22 of_node -> ../../../../../../../firmware/devicetree/base/soc/i2c@7e804000/temperature-sensor@67
drwxr-xr-x    2 root     root             0 May  9 18:22 power
lrwxrwxrwx    1 root     root             0 May  9 18:22 subsystem -> ../../../../../../../bus/iio
-rw-r--r--    1 root     root          4096 May  9 18:22 uevent
-r--r--r--    1 root     root          4096 May  9 18:22 waiting_for_supplier

root@raspberrypi3-64:~# cat /sys/bus/iio/devices/iio\:device0/in_temp_ambient_raw; cat /sys/bus/iio/devices/iio\:device0/in_temp_object_raw; iio_info                                                              
318
523
iio_info version: 0.25 (git tag:b6028fde)
Libiio version: 0.25 (git tag: b6028fd) backends: local xml ip usb serial
IIO context created with local backend.
Backend version: 0.25 (git tag: b6028fd)
Backend description string: Linux raspberrypi3-64 6.9.0-rc1-g51f9ab5c4102-dirty #240 SMP PREEMPT Thu May  9 20:21:32 CEST 2024 aarch64
IIO context has 2 attributes:
        local,kernel: 6.9.0-rc1-g51f9ab5c4102-dirty
        uri: local:
IIO context has 3 devices:
        hwmon0: cpu_thermal
                1 channels found:
                        temp1:  (input)
                        1 channel-specific attributes found:
                                attr  0: input value: 45084
                No trigger on this device
        hwmon1: rpi_volt
                1 channels found:
                        in0:  (input)
                        1 channel-specific attributes found:
                                attr  0: lcrit_alarm value: 0
                No trigger on this device
        iio:device0: mcp9600
                2 channels found:
                        temp_ambient:  (input)
                        2 channel-specific attributes found:
                                attr  0: raw value: 317
                                attr  1: scale value: 62.500000
                        temp_object:  (input)
                        2 channel-specific attributes found:
                                attr  0: raw value: 532
                                attr  1: scale value: 62.500000
                1 device-specific attributes found:
                                attr  0: waiting_for_supplier value: 0
                No trigger on this device

> The maping from hotjunction to object isn't totally clear to me.
> Mind you neither is the mapping from cold junction to ambient (that one is
> a bit stronger as the datasheet tables assume
> Cold Junction Temperature == Ambient Temperature.
> 
> Example of why I don't like this is object is no obvious if the hotjunction
> is in a gas or liquid.  The object defintion was I think added for infrared
> temperature sensors where you get nothing meaningful without an object to
> emit the infrared.
> 
Thanks for the explanation, somehow I was satisfied with the mapping
from hotjunction to object. :) If the object is gas or liquid ?!

> An alternative would be to provide an index for both channels. Also an ABI
> change, but avoids the object / hot junction issue and I would assume works
> fine with iio_info.
> 
I will have a look into it and come up with a patch eventually. Don't
know if its worth changing the ABI. What do you think ?

Dimitri

...