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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 5 May 2024 11:15:41 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Dimitri Fedrau <dima.fedrau@...il.com>
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>, Lars-Peter Clausen
 <lars@...afoo.de>, Andrew Hepp <andrew.hepp@...pp.dev>, Marcelo Schmitt
 <marcelo.schmitt1@...il.com>, linux-iio@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] iio: temperature: mcp9600: set channel2 member

On Tue, 30 Apr 2024 14:21:57 +0200
Dimitri Fedrau <dima.fedrau@...il.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@...il.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?

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

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.

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.

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.

Jonathan

> 
> iio_info version: 0.25 (git tag:b6028fde)
> Libiio version: 0.25 (git tag: b6028fd) backends: local xml ip usb serial
> 
> Besides that it eases distinction between the two channels in the last
> patch, but I think this argument is not strong enough. :)
> 
> Best regards,
> Dimitri
> 
> >   
> > > 
> > > Signed-off-by: Dimitri Fedrau <dima.fedrau@...il.com>
> > > ---
> > >  drivers/iio/temperature/mcp9600.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
> > > index 7a3eef5d5e75..e277edb4ae4b 100644
> > > --- a/drivers/iio/temperature/mcp9600.c
> > > +++ b/drivers/iio/temperature/mcp9600.c
> > > @@ -26,6 +26,8 @@ static const struct iio_chan_spec mcp9600_channels[] = {
> > >  	{
> > >  		.type = IIO_TEMP,
> > >  		.address = MCP9600_HOT_JUNCTION,
> > > +		.channel2 = IIO_MOD_TEMP_OBJECT,
> > > +		.modified = 1,
> > >  		.info_mask_separate =
> > >  			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> > >  	},  
> >   
>