Re: [PATCH v5 04/17] iio: adc: at91-sama5d2_adc: rework temp calibration layout handling

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The calibration data tag is defined and its layout index is configured, but it is never checked against the NVMEM data.
--

--- Patch [4]: [PATCH v5 04/17] iio: adc: at91-sama5d2_adc: rework temp calibration layout handling ---
commit 49b1bddcd8cc36f48960bba2752d5b3937516cd8
Author: Varshini Rajendran <[email protected]>

iio: adc: at91-sama5d2_adc: rework temp calibration layout handling

Extend support to handle different temperature calibration layouts. Add a
temperature calibration data layout structure to describe indexes of the
factors P1, P4, P6, tag, minimum length of the packet and the scaling
factors for P1 (p1_scale) which are SoC-specific instead of the older non
scalable id structure.

> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index 09390f082fc48..82db8f2f4db03 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
[ ... ]
> @@ -444,6 +445,27 @@ static const struct at91_adc_reg_layout sama7g5_layout = {
>  #define at91_adc_writel(st, reg, val)					\
>  	writel_relaxed(val, (st)->base + (st)->soc_info.platform->layout->reg)
>  
> +/* Temperature calibration tag "ACST" in ASCII */
> +#define AT91_TEMP_CALIB_TAG_ACST	0x41435354
> +
> +/**
> + * struct at91_adc_temp_calib_layout - temperature calibration packet layout
> + * @tag_idx:	index of Packet tag in the NVMEM cell buffer
> + * @p1_idx:	index of FT1_TEMP, equivalent to P1 in the NVMEM cell buffer
> + * @p4_idx:	index of FT1_VPAT, equivalent to P4 in the NVMEM cell buffer
> + * @p6_idx:	index of FT2_VBG, equivalent to P6 in the NVMEM cell buffer
> + * @min_len:	minimum number of u32 words expected in the NVMEM cell buffer
> + * @p1_scale:	scaling factor applied to P1 to convert to millicelcius
> + */
> +struct at91_adc_temp_calib_layout {
> +	unsigned int tag_idx;
> +	unsigned int p1_idx;
> +	unsigned int p4_idx;
> +	unsigned int p6_idx;
> +	unsigned int min_len;
> +	struct u32_fract p1_scale;
> +};
> +
[ ... ]
> @@ -2270,20 +2297,23 @@ static int at91_adc_temp_sensor_init(struct at91_adc_state *st,
>  		return dev_err_probe(dev, PTR_ERR(buf),
>  				     "Failed to read calibration data!\n");
>  
> -	if (len < AT91_ADC_TS_CLB_IDX_MAX * sizeof(*buf)) {
> +	if (len < layout->min_len * sizeof(*buf)) {

[Severity: Medium]
Should there be a check here to validate the packet tag?

The commit defines AT91_TEMP_CALIB_TAG_ACST and adds tag_idx to the layout
struct, but they do not appear to be used to validate the NVMEM buffer
content before it is parsed.

>  		dev_err(dev, "Invalid calibration data!\n");
>  		return -EINVAL;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4