All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] pinctrl: pinctrl-generic-mux: use mux_state_try_select()
@ 2026-07-15 19:35 Frank.Li
  2026-07-15 19:51 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Frank.Li @ 2026-07-15 19:35 UTC (permalink / raw)
  To: Linus Walleij, open list:PIN CONTROL SUBSYSTEM, open list; +Cc: imx, Frank Li

From: Frank Li <Frank.Li@nxp.com>

Use mux_state_try_select() instead of mux_state_select() so that the
consumer driver does not block during probe when the mux state has
already been selected.

mux_state_try_select() returns -EBUSY if the requested state is already
selected, allowing the driver to handle the condition without waiting.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Previous earily version pinctrl-generic-mux manage this resouce to avoid
call mux_state_select(). But review or respin patch, remove these check.
anyways mux_state_try_select() better method.
---
 drivers/pinctrl/pinctrl-generic-mux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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);
 	if (ret)
 		return ret;
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] pinctrl: pinctrl-generic-mux: use mux_state_try_select()
  2026-07-15 19:35 [PATCH 1/1] pinctrl: pinctrl-generic-mux: use mux_state_try_select() Frank.Li
@ 2026-07-15 19:51 ` sashiko-bot
  2026-07-15 20:12   ` Frank Li
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-07-15 19:51 UTC (permalink / raw)
  To: Frank.Li; +Cc: Frank.Li, imx

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.

>  	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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] pinctrl: pinctrl-generic-mux: use mux_state_try_select()
  2026-07-15 19:51 ` sashiko-bot
@ 2026-07-15 20:12   ` Frank Li
  0 siblings, 0 replies; 3+ messages in thread
From: Frank Li @ 2026-07-15 20:12 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Frank.Li, imx

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-15 20:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 19:35 [PATCH 1/1] pinctrl: pinctrl-generic-mux: use mux_state_try_select() Frank.Li
2026-07-15 19:51 ` sashiko-bot
2026-07-15 20:12   ` Frank Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.