Re: [PATCH RESEND v8 10/10] iio: adc: qcom-spmi-iadc: Remove regmap R/W wrapper functions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Format Specifier Mismatch in `iadc_version_check`
- [Low] Implicit Signed/Unsigned Arithmetic Conversion in `iadc_rsense_read`
--

commit e3f3f37b20b194577f3177eac52f98e565837579
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

iio: adc: qcom-spmi-iadc: Remove regmap R/W wrapper functions
    
This commit migrates the driver to use the regmap api directly instead of
relying on custom read and write wrapper functions. It simplifies the driver
as the spmi subdevice natively handles the register base address internally.

> diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
> index 55a09c0e2d5cb..503f90b2a5bad 100644
> --- a/drivers/iio/adc/qcom-spmi-iadc.c
> +++ b/drivers/iio/adc/qcom-spmi-iadc.c

[ ... ]

> @@ -392,10 +369,10 @@ static int iadc_update_offset(struct iadc_chip *iadc)
>  
>  static int iadc_version_check(struct iadc_chip *iadc)
>  {
> -	u8 val;
> +	u32 val;

This isn't a bug, but since val is now a u32, should the subsequent
dev_err() format specifiers in this function be updated from %d to %u to
avoid compiler warnings?

>  	int ret;
>  
> -	ret = iadc_read(iadc, IADC_PERPH_TYPE, &val);
> +	ret = regmap_read(iadc->regmap, IADC_PERPH_TYPE, &val);
>  	if (ret < 0)
>  		return ret;

[ ... ]

> @@ -428,7 +405,7 @@ static int iadc_version_check(struct iadc_chip *iadc)
>  static int iadc_rsense_read(struct iadc_chip *iadc, struct device_node *node)
>  {
>  	int ret, sign, int_sense;
> -	u8 deviation;
> +	u32 deviation;

Does changing deviation to u32 cause an implicit unsigned arithmetic
conversion later in this function?

When calculating int_sense, sign can be -1:

int_sense += sign * deviation * IADC_INT_RSENSE_DEVIATION;

Since deviation is unsigned, -1 will be promoted to an unsigned value.
Could this trigger static analysis warnings, and would explicitly casting
deviation to int during the calculation be safer?

>  
>  	ret = of_property_read_u32(node, "qcom,external-resistor-micro-ohms",
>  				   &iadc->rsense[IADC_EXT_RSENSE]);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511100805.121432-1-angelogioacchino.delregno@collabora.com?part=10

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy