Re: [PATCH v3 1/5] iio: adc: qcom-spmi-adc5: Add battery thermal channels

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] iio: adc: qcom-vadc: unit mismatch in
  qcom_vadc_scale_hw_calib_therm_volt()
--

commit e0ceb603a8d45a55cad2b26a93189eb6391e4af4
Author: Luca Weiss <luca.weiss@fairphone.com>

iio: adc: qcom-spmi-adc5: Add battery thermal channels

This commit adds ADC5_BAT_THERM_30K_PU, ADC5_BAT_THERM_100K_PU, and
ADC5_BAT_THERM_400K_PU voltage channels and the SCALE_HW_CALIB_THERM_VOLT
scaling type for reading battery thermistor voltages.

> diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
> index d57ba8526e0c5..24ac845805705 100644
> --- a/drivers/iio/adc/qcom-spmi-adc5.c
> +++ b/drivers/iio/adc/qcom-spmi-adc5.c
> @@ -542,6 +542,12 @@ static const struct adc5_channels adc5_chans_pmic[ADC5_MAX_CHANNEL] = {
>  					SCALE_HW_CALIB_DEFAULT)
>  	[ADC5_XO_THERM_100K_PU]	= ADC5_CHAN_TEMP("xo_therm", 0,
>  					SCALE_HW_CALIB_XOTHERM)
> +	[ADC5_BAT_THERM_30K_PU]	= ADC5_CHAN_VOLT("bat_therm_30k_pu", 0,
> +					SCALE_HW_CALIB_THERM_VOLT)
> +	[ADC5_BAT_THERM_100K_PU] = ADC5_CHAN_VOLT("bat_therm_100k_pu", 0,
> +					SCALE_HW_CALIB_THERM_VOLT)
> +	[ADC5_BAT_THERM_400K_PU] = ADC5_CHAN_VOLT("bat_therm_400k_pu", 0,
> +					SCALE_HW_CALIB_THERM_VOLT)
>  	[ADC5_BAT_ID_100K_PU]	= ADC5_CHAN_TEMP("bat_id", 0,
>  					SCALE_HW_CALIB_DEFAULT)
>  	[ADC5_AMUX_THM1_100K_PU] = ADC5_CHAN_TEMP("amux_thm1_100k_pu", 0,
> diff --git a/drivers/iio/adc/qcom-vadc-common.c b/drivers/iio/adc/qcom-vadc-common.c
> index b03cf584b1657..09a13e2b0ef43 100644
> --- a/drivers/iio/adc/qcom-vadc-common.c
> +++ b/drivers/iio/adc/qcom-vadc-common.c
> @@ -309,6 +309,10 @@ static int qcom_vadc_scale_hw_calib_therm(
>  				const struct u32_fract *prescale,
>  				const struct adc5_data *data,
>  				u16 adc_code, int *result_mdec);
> +static int qcom_vadc_scale_hw_calib_therm_volt(
> +				const struct u32_fract *prescale,
> +				const struct adc5_data *data,
> +				u16 adc_code, int *result_mdec);
>  static int qcom_vadc7_scale_hw_calib_therm(
>  				const struct u32_fract *prescale,
>  				const struct adc5_data *data,
> @@ -333,6 +337,7 @@ static int qcom_vadc7_scale_hw_calib_die_temp(
>  static const struct qcom_adc5_scale_type scale_adc5_fn[] = {
>  	[SCALE_HW_CALIB_DEFAULT] = {qcom_vadc_scale_hw_calib_volt},
>  	[SCALE_HW_CALIB_THERM_100K_PULLUP] = {qcom_vadc_scale_hw_calib_therm},
> +	[SCALE_HW_CALIB_THERM_VOLT] = {qcom_vadc_scale_hw_calib_therm_volt},
>  	[SCALE_HW_CALIB_XOTHERM] = {qcom_vadc_scale_hw_calib_therm},
>  	[SCALE_HW_CALIB_THERM_100K_PU_PM7] = {
>  					qcom_vadc7_scale_hw_calib_therm},
> @@ -583,6 +588,17 @@ static int qcom_vadc_scale_hw_calib_therm(
>  				 voltage, result_mdec);
>  }
>  
> +static int qcom_vadc_scale_hw_calib_therm_volt(
> +				const struct u32_fract *prescale,
> +				const struct adc5_data *data,
> +				u16 adc_code, int *result_uv)
> +{
> +	*result_uv = qcom_vadc_scale_code_voltage_factor(adc_code,
> +				prescale, data, 1000);
> +
> +	return 0;
> +}
> +
>  static int qcom_vadc_scale_hw_calib_die_temp(
>  				const struct u32_fract *prescale,
>  				const struct adc5_data *data,
> diff --git a/include/linux/iio/adc/qcom-vadc-common.h b/include/linux/iio/adc/qcom-vadc-common.h
> index 3bf4c49726a73..ce037994b0197 100644
> --- a/include/linux/iio/adc/qcom-vadc-common.h
> +++ b/include/linux/iio/adc/qcom-vadc-common.h
> @@ -93,6 +93,8 @@ struct vadc_linear_graph {
>   *	voltage (uV) with hardware applied offset/slope values to adc code.
>   * @SCALE_HW_CALIB_THERM_100K_PULLUP: Returns temperature in millidegC using
>   *	lookup table. The hardware applies offset/slope to adc code.
> + * @SCALE_HW_CALIB_THERM_VOLT: Returns voltage in uV of a temperature channel.
> + *	The hardware applies offset/slope to adc code.

[Severity: Low]
Is there a unit mismatch in qcom_vadc_scale_hw_calib_therm_volt() and the
new ADC5_CHAN_VOLT channels?

qcom_vadc_scale_code_voltage_factor() computes microvolts and divides by
factor, so passing factor = 1000 returns integer millivolts rather than
microvolts:

drivers/iio/adc/qcom-vadc-common.c:qcom_vadc_scale_code_voltage_factor() {
    ...
	/* (ADC code * vref_vadc (1.875V)) / full_scale_code */
	voltage = (s64) adc_code * adc_vdd_ref_mv * 1000;
	voltage = div64_s64(voltage, data->full_scale_code_volt);
	if (voltage > 0) {
		voltage *= prescale->denominator;
		temp = prescale->numerator * factor;
		voltage = div64_s64(voltage, temp);
    ...
}

However, the kernel-doc for SCALE_HW_CALIB_THERM_VOLT and the definition of
qcom_vadc_scale_hw_calib_therm_volt() state that the result is in uV
(result_uv), while its forward declaration names the parameter result_mdec.
In addition, all other ADC5_CHAN_VOLT channels in adc5_chans_pmic use
SCALE_HW_CALIB_DEFAULT (qcom_vadc_scale_hw_calib_volt(), which passes
factor = 1) and return microvolts, so returning integer millivolts here
creates a 1000x unit inconsistency across ADC5_CHAN_VOLT channels and
truncates sub-millivolt precision.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-bat-temp-adc-v3-0-743979a8fd90@fairphone.com?part=1