linux-kernel - Re: [PATCH v2 2/2] iio: temperature: add support for MCP998X

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]

Message-ID: <20250607182813.64230171@jic23-huawei>
Date: Sat, 7 Jun 2025 18:28:13 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: <Victor.Duicu@...rochip.com>
Cc: <Jonathan.Cameron@...wei.com>, <dlechner@...libre.com>,
 <nuno.sa@...log.com>, <linux-iio@...r.kernel.org>,
 <devicetree@...r.kernel.org>, <robh@...nel.org>, <andy@...nel.org>,
 <krzk+dt@...nel.org>, <linux-kernel@...r.kernel.org>,
 <Marius.Cristea@...rochip.com>, <conor+dt@...nel.org>
Subject: Re: [PATCH v2 2/2] iio: temperature: add support for MCP998X

On Mon, 2 Jun 2025 14:49:29 +0000
<Victor.Duicu@...rochip.com> wrote:

> On Fri, 2025-05-30 at 17:53 +0100, Jonathan Cameron wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > On Thu, 29 May 2025 12:36:28 +0300
> > victor.duicu@...rochip.com wrote:
> >   
> > > From: Victor Duicu <victor.duicu@...rochip.com>
> > > 
> > > This is the driver for Microchip MCP998X/33 and MCP998XD/33D
> > > Multichannel Automotive Temperature Monitor Family.
> > > 
> > > Signed-off-by: Victor Duicu <victor.duicu@...rochip.com>  
> > Hi Victor,
> >   
> Hi Jonathan,
> 
> ...
> 
> >   
> > > +
> > > +What:                /sys/bus/iio/devices/iio:deviceX/in_beta1
> > > +KernelVersion:       6.17
> > > +Contact:     linux-iio@...r.kernel.org
> > > +Description:
> > > +             This attribute controls the value of beta correction
> > > +             for channel 1.  
> > 
> > Is this something that we'd normally expect to manually update? What
> > is
> > it a characteristic of?  If it is expected to the be related to the
> > diodes attached, that's a problem for firmware/dt, not sysfs
> > interfaces.
> >   
> 
> Beta is a characteristic of the diode/transistor placed on the setup.
> Different diodes require different betas.
> So yes, beta value should be in devicetree.
> 
> ...
> > > 
> > > +
> > > +             priv->iio_chan[iio_idx++] = (struct
> > > iio_chan_spec)MCP9982_CHAN(reg_nr, reg_nr,
> > > +                                                     
> > > MCP9982_INT_VALUE_ADDR(reg_nr));  
> > 
> > Seems very likely that the (struct iio_chan_spec) should be in the
> > macro definition.
> >   
> 
> In version 1 the macro used to define the channels was:
> 
> >   
> #define MCP9982_CHAN(index, si, __address) ({ \
> 	struct iio_chan_spec __chan = { \
> 		.type = IIO_TEMP, \
> 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> 		.info_mask_shared_by_all_available =
> BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> 		.info_mask_shared_by_all =
> BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> 		.channel = index, \
> 		.address = __address, \
> 		.scan_index = si, \
> 		.scan_type = { \
> 			.sign = 'u', \
> 			.realbits = 8, \
> 			.storagebits = 8, \
> 			.endianness = IIO_CPU \
> 		}, \
> 		.indexed = 1, \
> 	}; \
> 	__chan; \
> })
> 
> which contains struct iio_chan_spec inside.
> Andy suggested to use compound literal, so I switched to version 2.
> I still use a macro because most of the parameters are common among the
> channels.

I think we can add the (struct iio_chan_spec) to the macro definition. 

> 
> With Kind Regards,
> Victor Duicu