Re: [PATCH 1/1] pinctrl: pinctrl-generic-mux: use mux_state_try_select()

[Frank Li <Frank.li@oss.nxp.com>]

On Wed, Jul 15, 2026 at 07:51:43PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Replacing `mux_state_select()` with `mux_state_try_select()` causes silent pin configuration failures during driver probe.
>
> Pre-existing issues:
> - [High] Concurrent radix tree insertions and data races due to missing lock in `mux_pinmux_dt_node_to_map`.
> --
>
> --- Patch [1]: [PATCH 1/1] pinctrl: pinctrl-generic-mux: use mux_state_try_select() ---
> commit 8b69728953c0e1594fa409568136b389105c9d0b
> Author: Frank Li <Frank.Li@nxp.com>
> pinctrl: pinctrl-generic-mux: use mux_state_try_select()
>
> This commit updates the generic mux driver to use mux_state_try_select()
> instead of mux_state_select(). This prevents the consumer driver from
> blocking during probe when the mux state is already selected.
>
> > diff --git a/drivers/pinctrl/pinctrl-generic-mux.c b/drivers/pinctrl/pinctrl-generic-mux.c
> > index da5a5ec015833..202b72351efb7 100644
> > --- a/drivers/pinctrl/pinctrl-generic-mux.c
> > +++ b/drivers/pinctrl/pinctrl-generic-mux.c
> > @@ -93,7 +93,7 @@ static int mux_pinmux_set_mux(struct pinctrl_dev *pctldev,
> >  	function = pinmux_generic_get_function(pctldev, func_selector);
> >  	func = function->data;
> >
> > -	ret = mux_state_select(func->mux_state);
> > +	ret = mux_state_try_select(func->mux_state);
>
> [Severity: High]
> Does returning -EBUSY here lead to silent pin configuration failures?
>
> When mux_pinmux_set_mux() returns -EBUSY on contention during a consumer's
> probe, the error propagates up to the pinctrl core in pinctrl_bind_pins().
>
> Because the device core intentionally ignores most errors from
> pinctrl_bind_pins() (only propagating -EPROBE_DEFER and -EINVAL), it
> swallows the -EBUSY error. As a result, the probe silently succeeds while
> the device's pins remain unconfigured.

It is wrong, consumer driver return failure.

fsl_lpspi 5a030000.spi: error -EBUSY: Error applying setting, reverse things back

Frank

>
> >  	if (ret)
> >  		return ret;
> >
>
> [Severity: High]
> This is a pre-existing issue, but is it safe to add functions to the radix
> tree without locking?
>
> When multiple consumer devices probe concurrently, their execution paths
> can run in parallel and invoke pinctrl_dt_to_map(), which calls
> mux_pinmux_dt_node_to_map() on the shared generic mux:
>
> drivers/pinctrl/pinctrl-generic-mux.c:mux_pinmux_dt_node_to_map() {
>     ...
> 	ret = pinmux_generic_add_function(pctldev, np_config->name, group_names,
> 					  1, function);
>     ...
> }
>
> Because pinmux_generic_add_function() modifies pctldev->pin_function_tree
> and increments pctldev->num_functions without protection, concurrent
> insertions can structurally corrupt the radix tree.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260715193546.2852108-1-Frank.Li@oss.nxp.com?part=1