Re: [PATCH v2 7/7] iio: temperature: ltc2983: Add support for ADT7604

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 18 May 2026 11:07:17 +0300
Liviu Stan <liviu.stan@analog.com> wrote:

> On Sat, 16 May 2026 18:12:50 +0100 Jonathan Cameron <jic23@kernel.org> wrote:
> > > 
> > >  drivers/iio/temperature/ltc2983.c | 401 ++++++++++++++++++++++++++++--
> > >  1 file changed, 386 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> > > index bf435e965c6d..acd043ed62f5 100644
> > > --- a/drivers/iio/temperature/ltc2983.c
> > > +++ b/drivers/iio/temperature/ltc2983.c
> > > @@ -28,6 +28,8 @@
> > >  #define LTC2983_STATUS_REG			0x0000
> > >  #define LTC2983_TEMP_RES_START_REG		0x0010
> > >  #define LTC2983_TEMP_RES_END_REG		0x005F
> > > +#define ADT7604_RES_RES_START_REG		0x0060
> > > +#define ADT7604_RES_RES_END_REG			0x00AF
> > >  #define LTC2983_EEPROM_KEY_REG			0x00B0
> > >  #define LTC2983_EEPROM_READ_STATUS_REG		0x00D0
> > >  #define LTC2983_GLOBAL_CONFIG_REG		0x00F0
> > > @@ -186,17 +188,43 @@ enum {
> > >  	LTC2983_SENSOR_SENSE_RESISTOR = 29,
> > >  	LTC2983_SENSOR_DIRECT_ADC = 30,
> > >  	LTC2983_SENSOR_ACTIVE_TEMP = 31,
> > > +	/* Sensor types for some parts only; map to RTD_CUSTOM/THERMISTOR_CUSTOM in HW */
> > > +	LTC2983_SENSOR_COPPER_TRACE = 32,
> > > +	LTC2983_SENSOR_LEAK_DETECTOR = 33,  
> > Given you care about being in range of this I'd add
> > 	LTC2983_SENSOR_NUM  
> > >  };  
> >   
> > > @@ -1329,7 +1649,7 @@ static int ltc2983_parse_fw(struct ltc2983_data *st)
> > >  	if (!st->sensors)
> > >  		return -ENOMEM;
> > >  
> > > -	st->iio_channels = st->num_channels;
> > > +	st->iio_channels = 0;
> > >  	device_for_each_child_node_scoped(dev, child) {
> > >  		struct ltc2983_sensor sensor;
> > >  
> > > @@ -1357,7 +1677,13 @@ static int ltc2983_parse_fw(struct ltc2983_data *st)
> > >  			return dev_err_probe(dev, ret,
> > >  				"adi,sensor-type property must given for child nodes\n");
> > >  
> > > -		dev_dbg(dev, "Create new sensor, type %u, chann %u",
> > > +		if (sensor.type > LTC2983_SENSOR_LEAK_DETECTOR ||  
> > 
> > To make it easier to extend in future, perhaps add the NUM entry I mention
> > above then >= to it here.
> >   
> 
> This makes sense. I will change in v3. Thanks!
> 
> > > +		    !(st->info->supported_sensors & BIT_ULL(sensor.type)))
> > > +			return dev_err_probe(dev, -EINVAL,
> > > +					     "sensor type %d not supported on %s\n",
> > > +					     sensor.type, st->info->name);
> > > +
> > > +		dev_dbg(dev, "Create new sensor, type %u, channel %u",
> > >  			sensor.type, sensor.chan);
> > >    
> >   
> > > @@ -1445,8 +1782,9 @@ static int ltc2983_eeprom_cmd(struct ltc2983_data *st, unsigned int cmd,
> > >  
> > >  static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio)
> > >  {
> > > -	u32 iio_chan_t = 0, iio_chan_v = 0, chan, iio_idx = 0, status;
> > >  	struct device *dev = &st->spi->dev;
> > > +	u32 iio_chan_t = 0, iio_chan_v = 0, iio_chan_r = 0, iio_chan_c = 0;
> > > +	u32 chan, iio_idx = 0, status;
> > >  	int ret;
> > >  
> > >  	/* make sure the device is up: start bit (7) is 0 and done bit (6) is 1 */
> > > @@ -1493,8 +1831,26 @@ static int ltc2983_setup(struct ltc2983_data *st, bool assign_iio)
> > >  		    !assign_iio)
> > >  			continue;
> > >  
> > > +		/*
> > > +		 * Copper trace and leak detector sensors without a custom table
> > > +		 * produce only a resistance result; the chip does not populate
> > > +		 * the temperature result register. Emit only an IIO_RESISTANCE
> > > +		 * channel in this case.  
> > 
> > Do we care?  That is are they useful without the table? We could just make it
> > required in the binding.
> >   
> 
> The datasheet specifies the table is optional. But more practically, in order to
> be able to add accurate values to the custom table, the users first need to measure
> the sensor's resistance at multiple known conditions, so I think the resistance-only
> output is useful during that characterization phase, before the table exists. Making
> it required would force users to provide placeholder values just to get the driver
> to probe.
Who cares of datasheet is crazy :)

The initial case could I think be handled by an 'identity' table. 
If it's useful in more general cases maybe we should always put out the resistance
channels? This would be a bit like we often do for ambient light sensors, where
we have a computed illuminance channel (IIO_LIGHT) + the data it comes from
(IIO_INTENSITY)

Jonathan

> 
> Thanks,
> Liviu
>