Re: [PATCH v2 07/14] iio: adc: stm32-adc: add support for stm32mp25

[Fabrice Gasnier]

On 9/24/26 23:01, Andy Shevchenko wrote:
> On Wed, Sep 23, 2026 at 05:39:10PM +0200, Fabrice Gasnier wrote:
>> Add support for ADC on STM32MP25 SoC. It has 3 ADCs, split into two blocks:
>> - ADC1 & ADC2 are tightly coupled.
>> - ADC3 is managed independently.
>>
>> Trigger list slightly changes between these blocks (ADC1 & ADC2). Other
>> differences are found on channels interconnects (similar between ADC2
>> and ADC3):
>> - ADC1 is connected to 18 external channels + 2 internal channels
>> - ADC2 is connected to 14 external channels + 6 internal channels
>> - ADC3 is connected to 14 external channels + 6 internal channels
>>
>> Each ADC is a 12-bits successive approximation analog-to-digital converter,
>> with up to 20 multiplexed channels that can be configured as single ended
>> or differential. ADC resolution ranges from 6 to 12 bits.
>>
>> It introduces diversity regarding IRQs, clocks, software calibration
>> procedure, internal voltage channels, sampling time (prescaler) and
>> trigger list. Most of the architecture, and the driver engine remains
>> similar. So, handle the differences w.r.t. other STM32 ADCs family with
>> a dedicated compatible and compatible data.
> 
> ...
> 
>>  		.compatible = "st,stm32mp13-adc-core",
>>  		.data = (void *)&stm32mp13_adc_priv_cfg
>>  	}, {
>> -	},
>> +		.compatible = "st,stm32mp25-adc-core",
>> +		.data = (void *)&stm32mp25_adc_priv_cfg
>> +	}, {
>> +	}
> 
> Same issue and now it's a regression from maintenance perspective: you added an
> unnedeed churn that has to be handled from now on... TL;DR: do add trailing
> commas to the non-terminator entries and remove trailing commas in the
> terminator entries.

Hi Andy,

Do you wish I add a separate cleanup patch for existing match table ?
This can also fit into 100 chars line now:

static const struct of_device_id stm32_adc_of_match[] = {
	{ .compatible = "st,stm32f4-adc-core", .data = (void
*)&stm32f4_adc_priv_cfg, },
	{ .compatible = "st,stm32h7-adc-core", .data = (void
*)&stm32h7_adc_priv_cfg, },
	{ .compatible = "st,stm32mp1-adc-core",	.data = (void
*)&stm32mp1_adc_priv_cfg, },
	{ .compatible = "st,stm32mp13-adc-core", .data = (void
*)&stm32mp13_adc_priv_cfg, },
	{ }
};

> 
>>  };
> 
> ...
> 
>> +	STM32_EXT23,
>> +	STM32_EXT24,
>> +	STM32_EXT25,
>> +	STM32_EXT26,
>> +	STM32_EXT27,
>> +	STM32_EXT28
> 
> Same issue and so on...

Ack,

> 
>>  };
> 
> ...
> 
> Are you doing patches with an assistance? LLMs might have a problem with the
> style issues.

My apologies, that was just me between the keyboard and the chair.

> 
> ...
> 
>> +retry:
>> +	/* Clears or set CALADDOS (also clear old calibration data if any) */
>> +	stm32_adc_writel(adc, STM32MP25_ADC_CALFACT,
>> +			 FIELD_PREP(STM32MP25_CALFACT_CALADDOS, *add_offset));
>> +
>> +	ret = stm32mp25_adc_calib_get_average_data(indio_dev, &average);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Add offset and retry single-ended calibration if the averaged data is zero */
>> +	if (!average && !*add_offset) {
>> +		*add_offset = true;
>> +		goto retry;
>> +	}
> 
> Refactor to avoid a label. It's possible to achieve.

Ok, I'll refactor in v3.

Thanks for reviewing,
BR,
Fabrice