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

[<Victor.Duicu@microchip.com>]

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@microchip.com wrote:
> 
> > From: Victor Duicu <victor.duicu@microchip.com>
> > 
> > This is the driver for Microchip MCP998X/33 and MCP998XD/33D
> > Multichannel Automotive Temperature Monitor Family.
> > 
> > Signed-off-by: Victor Duicu <victor.duicu@microchip.com>
> Hi Victor,
> 
Hi Jonathan,

...

> 
> > +
> > +What:                /sys/bus/iio/devices/iio:deviceX/in_beta1
> > +KernelVersion:       6.17
> > +Contact:     linux-iio@vger.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.

With Kind Regards,
Victor Duicu