Re: [PATCH v2] mux: gpio-mux: add support for 4:1 2-channels mux

[Srinivas Kandagatla]

On 6/30/26 4:08 PM, Andrea Tomassetti wrote:
>>> +
>>> +	mux_chip = devm_mux_chip_alloc(dev, cells + 1, sizeof(*mux_gpio));
>> We are now allocating n mux_controls, however mux_gpio_set is not
>> protected against multiple controllers accessing the same gpio resource.
>>
>>
> This is a great point. I see two different type of "race conditions" here:
>     1. serialize accesses in mux_gpio_set
serializing is not the major issue. The end result is.
>     2. "logical race condition" due to how the upper locking mechanism is
> handled by mux/core.c

> 
> The first issue can be easily solved by adding a mutex inside struct mux_gpio
> and use it to serialize accesses in mux_gpio_set.
> 
> OTOH, the second issue is more nasty and depends on how the upper locking
> mechanism is handled by mux/core.c. Please, correct me if I'm wrong, but to me
> it looks like that the core's locking model assumes each mux_control is an
> independently lockable resource. While this is generally true, it doesn't apply
> in the case when there's only one controller that jointly controls two or more
> gpio-muxes. Like in this case.

Yes, we have locking per mux control for a reason, Mux resources are
protected in that control context to make sure the mux is always held in
requested state.

Now with this patch we are using shared resource for multiple mux, that
means an underlying resource can be updated by another mux control
without of each other knowing about it.

Two questions.
1. Why a single resource is split into different mux controls? (sorry In
case I missed previous discussions)

2. Does the hardware allow you to set two different values for the same
resource, respecting both values at any given time. If its gpio, then am
not sure it does.


--srini


> 
> So, adding a mutex at gpio-mux level would fix the race condition of two
> consumers changing the state at the same time (issue 1). But, to my
> understanding, it won't fix issue 2: mutual exclusion on the write makes each
> write atomic, but consumer A's selection can still be immediately hijacked by
> consumer B's, leaving A believing it is reading what it just set when it's
> actually reading whatever B last selected. This happens because the lock used in
> mux/core.c is allocated _per controller_ and not _per chip_. Do you agree with
> my analysis?
> 
> If you agree, I can fix issue 1 but I'd lean towards treating issue 2 as out of
> scope for this patch. Let me know if you see it differently.
> 
> Br,