Re: [PATCH] pinctrl: stm32: use a raw spinlock regmap to program the EXTI mux

On 8/3/26 2:34 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Bypassing standard syscon regmap initialization leaves the syscfg clock disabled, leading to hardware bus faults on platforms where the syscfg node requires a clock.
> - [High] If another driver sharing the syscon node probes before pinctrl, the generic syscon layer will create a default regmap without `use_raw_spinlock`, causing the PREEMPT_RT sleeping-in-atomic bug to persist.

> [Severity: High]
> Does bypassing the standard syscon regmap initialization leave the syscfg
> clock disabled?
> 
> The generic syscon handling normally calls of_clk_get() and 
> regmap_mmio_attach_clk(). Since this manual initialization skips that, the
> clock is never attached to the regmap.
> 
> On platforms like STM32F4/F7/H7 where the syscfg node explicitly requires a 
> clock, could writing to the EXTI mux registers in stm32_gpio_domain_activate()
> with the clock disabled trigger an imprecise external abort (bus fault)?

That's right for STM32F4/F7, I have checked related dts files and the syscfg
node does require a clock.

>> +
>> +	/*
>> +	 * A regmap is already registered for that node, most likely by the
>> +	 * other pinctrl instance sharing it. Drop ours and use that one.
>> +	 */
>> +	if (of_syscon_register_regmap(syscfg_np, regmap)) {
>> +		regmap_exit(regmap);
>> +		iounmap(base);
>> +	}
> 
> [Severity: High]
> If another driver sharing the st,syscfg node (like Ethernet MAC, I2C, or USB)
> probes before pinctrl, won't the generic syscon layer have already
> instantiated a default regmap without use_raw_spinlock?
> 
> In that scenario, of_syscon_register_regmap() would return an error and the
> driver silently discards its raw regmap. Since the driver then falls back to
> the already registered sleeping regmap, does this cause the PREEMPT_RT 
> sleeping-in-atomic bug to persist when stm32_gpio_domain_activate() writes to
> the regmap under the raw desc->lock?
> 


True again for STM32F4/H7. But the syscfg node is shared between pinctrl and
pinctrl_z only on STM32MP series SoC, and that's the platform on which I tested
this patch.

Thanks sashiko-bot :P, I will send a V2 later.

>> +
>> +out_put:
>> +	of_node_put(syscfg_np);
>> +}
>