Re: [PATCH v3 2/2] iio: adc: Add Intel Dollar Cove TI PMIC ADC driver

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 31 Jul 2025 12:12:55 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> On Tue, 29 Jul 2025 12:50:36 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > On 7/27/25 4:06 PM, Hans de Goede wrote:
> > 
> > ...
> >   
> > > +static const struct iio_chan_spec dc_ti_adc_channels[] = {
> > > +	{
> > > +		.indexed = 1,
> > > +		.type = IIO_VOLTAGE,
> > > +		.channel = DC_TI_ADC_VBAT,
> > > +		.address = DC_TI_ADC_DATA_REG_CH(0),
> > > +		.datasheet_name = "CH0",
> > > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > > +				      BIT(IIO_CHAN_INFO_SCALE) |
> > > +				      BIT(IIO_CHAN_INFO_PROCESSED),
> > > +	}, {
> > > +		.indexed = 1,
> > > +		.type = IIO_TEMP,
> > > +		.channel = DC_TI_ADC_PMICTEMP,
> > > +		.address = DC_TI_ADC_DATA_REG_CH(1),
> > > +		.datasheet_name = "CH1",
> > > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> > > +	}, {
> > > +		.indexed = 1,
> > > +		.type = IIO_TEMP,
> > > +		.channel = DC_TI_ADC_BATTEMP,
> > > +		.address = DC_TI_ADC_DATA_REG_CH(2),
> > > +		.datasheet_name = "CH2",
> > > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> > > +	}, {
> > > +		.indexed = 1,
> > > +		.type = IIO_TEMP,
> > > +		.channel = DC_TI_ADC_SYSTEMP0,
> > > +		.address = DC_TI_ADC_DATA_REG_CH(3),
> > > +		.datasheet_name = "CH3",
> > > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> > > +	}
> > > +};    
> > 
> > Question for Jonathan: If we don't know the scale on these
> > temperature channels, should we implement IIO_CHAN_INFO_SCALE
> > and just always return an error? My understanding is that
> > by not implementing it, the scale is assumed to be 1, which
> > likely isn't correct in in this case.  
> 
> Good question.   We have quite a bit of precedence of just
> not providing scale in these cases.  The docs for the main block of _scale
> do say
> 
> "If known for a device, scale to be applied to <type>Y[_name]_raw
>  post addition of <type>[Y][_name]_offset in order to obtain the
>  measured value in <type> units as specified in
>  <type>[Y][_name]_raw documentation. "
> 
> Rather than trying to fix all those, shall we tweak the docs
> to say that scale is assumed to be 1 if offset is provided?
> 
> If neither is there, the channel can be _PROCESSED anyway.
> 
> Your suggestion of returning an error would have been somewhat neat
> long ago, but now I think it will just be too confusing for userspace.
> 
> Jonathan
> 
Just a quick note.  I looked through the driver again the other
day and other than the bug (and request for tests) that David had
on patch 1 this looks good to me.

I completely missed that bug so thanks David!

Jonathan


> > 
> >   
> 
>