Re: [PATCH 2/7] iio: adc: qcom-spmi-adc5-gen3: Add support for QCOM PMIC5 Gen4 ADC

[Jishnu Prakash <jishnu.prakash@oss.qualcomm.com>]

Hi Andy,

On 8/9/2026 2:36 AM, Andy Shevchenko wrote:
> On Fri, Jul 31, 2026 at 11:36:15PM +0530, Jishnu Prakash wrote:
>> PMIC5 Gen4 ADC is similar to PMIC5 Gen3 ADC, with several changes made
>> for improved performance, mostly at the hardware level.
>>
>> One significant software change is that ratiometric conversion resolution
>> has been increased from 14 bits to 16 bits, so the maximum value of
>> these measurements needs to be updated for Gen4. Add a new scaling
>> function for thermistor channels which use this type of conversion.
>>
>> In the latest PMIC arbiter version (v8), there can be up to 4 buses
>> under the PMIC arbiter and 32 PMICs under each bus. In order to
>> support communication between ADC on the master PMIC and ADCs on any
>> of the other PMICs, a field of width 2 bits is added for bus index
>> and the bits for SID are extended from 4 to 5 bits, in the SID
>> register. Add support for this.
>>
>> In addition, it is possible that the master PMIC has ADC of one generation
>> and it needs to communicate with another PMIC with ADC of a different
>> generation. Add new DT properties "qcom,adc5-gen3" and "qcom,adc5-gen4",
>> to distinguish Gen3 channels under a Gen4 master and Gen4 channels
>> under a Gen3 master respectively, to ensure that their conversions are handled
>> correctly.
> 
> ...
> 
>> +static const struct adc5_channels adc5_gen4_chans_pmic[ADC5_MAX_CHANNEL] = {
>> +	[ADC5_GEN4_OFFSET_REF]		= ADC5_CHAN_VOLT(0,
>> +						SCALE_HW_CALIB_DEFAULT)
> 
> Always nice to see a macro with the embedded comma...
> 
>> +	[ADC5_GEN4_1P25VREF]		= ADC5_CHAN_VOLT(0,
>> +						SCALE_HW_CALIB_DEFAULT)
>> +	[ADC5_GEN4_VPH_PWR]		= ADC5_CHAN_VOLT(1,
>> +						SCALE_HW_CALIB_DEFAULT)
>> +	[ADC5_GEN4_VBAT_SNS_QBG]	= ADC5_CHAN_VOLT(1,
>> +						SCALE_HW_CALIB_DEFAULT)
>> +	[ADC5_GEN4_DIE_TEMP]		= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_PMIC_THERM_PM7)
>> +	[ADC5_GEN4_AMUX1_THM_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +	[ADC5_GEN4_AMUX2_THM_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +	[ADC5_GEN4_AMUX3_THM_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +	[ADC5_GEN4_AMUX4_THM_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +	[ADC5_GEN4_AMUX5_THM_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +	[ADC5_GEN4_AMUX6_THM_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +	[ADC5_GEN4_AMUX1_GPIO_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +	[ADC5_GEN4_AMUX2_GPIO_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +	[ADC5_GEN4_AMUX3_GPIO_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +	[ADC5_GEN4_AMUX4_GPIO_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +	[ADC5_GEN4_AMUX5_GPIO_100K_PU]	= ADC5_CHAN_TEMP(0,
>> +						SCALE_HW_CALIB_THERM_100K_PU_GEN4)
>> +};
> 
> ...
> 
>>  enum adc5_cal_method {
>>  	ADC5_NO_CAL = 0,
> 
> (here as well, see below)
> 
>>  	ADC5_RATIOMETRIC_CAL,
> 
>>  };
>>  
>> +enum adc_generation {
>> +	ADC5_GEN3 = 0,
> 
> Why is this assignment? What's wrong with the C standard?
> 
>> +	ADC5_GEN4,
> 
> If these are related to HW communication protocol or bit fields, all of them
> must be explicitly assigned (to make code robust against changes in the middle
> of enum).

I see that ADC5_RATIOMETRIC_CAL is used for bit fields, but ADC5_GEN3/ADC5_GEN4
are not used in HW. I'll add a fix patch to explicitly assign all the enum
values directly used in HW and drop the assignment for ADC5_GEN3 from this patch.

Thanks,
Jishnu

> 
>> +};
>