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

[Liviu Stan <liviu.stan@analog.com>]

On Mon, 18 May 2026 14:58:02 +0100 Jonathan Cameron <jic23@kernel.org> wrote:
...
> > > > +		    !(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 :)

Fair enough :)

> 
> 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)

I checked internally and we could make the table required for leak detectors. For
copper traces, sub-ohms variants cannot have one, but we could make it required
for > 1ohm ones.

This means we could remove the LTC2983_SENSOR_LEAK_DETECTOR from the if condition,
and have something like this in ltc2983_setup:

if (st->sensors[chan]->type == LTC2983_SENSOR_COPPER_TRACE) {
    if (st->sensors[chan]->n_iio_chan == 1) {
        st->iio_chan[iio_idx++] =
            LTC2983_CHAN(IIO_RESISTANCE, iio_chan_r++, chan);
        continue;
    }
}
+ the n_iio_chan == 2 check at the end

or drop the n_iio_chan == 2 check and do something like:

if (st->sensors[chan]->type == LTC2983_SENSOR_COPPER_TRACE) {
    if (st->sensors[chan]->n_iio_chan == 1) {
        st->iio_chan[iio_idx++] =
            LTC2983_CHAN(IIO_RESISTANCE, iio_chan_r++, chan);
    } else {
        st->iio_chan[iio_idx++] =
            LTC2983_CHAN(IIO_TEMP, iio_chan_t++, chan);
        st->iio_chan[iio_idx++] =
            LTC2983_CHAN(IIO_RESISTANCE, iio_chan_r++, chan);
    }
    continue;
}

if (st->sensors[chan]->type == LTC2983_SENSOR_LEAK_DETECTOR) {
    st->iio_chan[iio_idx++] =
        LTC2983_CHAN(IIO_COVERAGE_PERCENT, iio_chan_c++, chan);
    st->iio_chan[iio_idx++] =
        LTC2983_CHAN(IIO_RESISTANCE, iio_chan_r++, chan);
    continue;
}

or use a switch case:

switch (st->sensors[chan]->type) {
case LTC2983_SENSOR_COPPER_TRACE:
    if (st->sensors[chan]->n_iio_chan == 1) {
        st->iio_chan[iio_idx++] =
            LTC2983_CHAN(IIO_RESISTANCE, iio_chan_r++, chan);
    } else {
        st->iio_chan[iio_idx++] =
            LTC2983_CHAN(IIO_TEMP, iio_chan_t++, chan);
        st->iio_chan[iio_idx++] =
            LTC2983_CHAN(IIO_RESISTANCE, iio_chan_r++, chan);
    }
    continue;
case LTC2983_SENSOR_LEAK_DETECTOR:
    st->iio_chan[iio_idx++] =
        LTC2983_CHAN(IIO_COVERAGE_PERCENT, iio_chan_c++, chan);
    st->iio_chan[iio_idx++] =
        LTC2983_CHAN(IIO_RESISTANCE, iio_chan_r++, chan);
    continue;
case LTC2983_SENSOR_DIRECT_ADC:
    chan_type = IIO_VOLTAGE;
    iio_chan = &iio_chan_v;
    break;
default:
    chan_type = IIO_TEMP;
    iio_chan = &iio_chan_t;
    break;
}
st->iio_chan[iio_idx++] = LTC2983_CHAN(chan_type, (*iio_chan)++, chan);

What do you think?

Thanks,
Liviu