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

[Andrea Tomassetti]

Hi Linus,
thank you very much for taking the time to review and reply.

On 5/5/26 10:23, Linus Walleij wrote:
> **Warning** This email is from an external address. Please take care to proceed.
> 
> 
> Hi Andrea,
> 
> thanks for your patch!
> 
> On Thu, Apr 30, 2026 at 4:13 PM Andrea Tomassetti
> <andrea.tomassetti@xxxxxxxxxxx> wrote:
> 
>> Some gpio multiplexers, like TMUX1209, offer differential 4:1
>> or dual 4:1 single-ended channels. Similarly to what already done by
>> the adg792a driver, the gpio-mux driver has to take into account
>> the #mux-control-cells property and allocate as many controllers
>> as advised by it.
>>
>> So, in the DTS you can now define:
>>
>>         tmux1209: mux-controller {
>>                 compatible = "gpio-mux";
>>                 #mux-control-cells = <1>;
>>
>>                 mux-gpios = <&gpio_expander 01 GPIO_ACTIVE_HIGH>,
>>                                         <&gpio_expander 02 GPIO_ACTIVE_HIGH>;
>>         };
>>
>>         adcmux30: adcmux30 {
>>                 compatible = "io-channel-mux";
>>                 io-channels = <&adc1 4>;
>>                 io-channel-names = "parent";
>>                 #io-channel-cells = <1>;
>>                 mux-controls = <&tmux1209 0>;
>>
>>                 channels = "S1A", "S2A", "S3A", "S4A";
>>         };
>>
>>         adcmux31: adcmux31 {
>>                 compatible = "io-channel-mux";
>>                 io-channels = <&adc1 5>;
>>                 io-channel-names = "parent";
>>                 #io-channel-cells = <1>;
>>                 mux-controls = <&tmux1209 1>;
>>
>>                 channels = "S1B", "S2B", "S3B", "S4B";
>>         };
>>
>> Signed-off-by: Andrea Tomassetti <andrea.tomassetti@xxxxxxxxxxx>
> 
> The mux controller binding looks like this:
> 
> properties:
>   '#mux-control-cells':
>     enum: [ 0, 1 ]
> 
> So you do not patch the bindings, you actually implement this
> for the case when #mux-control-cells is 1.
> 
> Please detail this in the commit.
> 
Exactly, I'm not patching the bindings, there's no need. I tried to detail this
in my commit message:

    [...] the gpio-mux driver has to take into account
    the #mux-control-cells property and allocate as many controllers
    as advised by it.

but it looks like is not as clear as I thought. Maybe I can add the fact that
"patching the binding is not needed because the binding already supports
#mux-control-cells 0 and 1". What do you think?


>> -       mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_gpio));
>> +       ret = device_property_read_u32(dev, "#mux-control-cells", &cells);
>> +       if (ret < 0)
>> +               cells = 0;
>> +
>> +       if (cells >= 2) {
>> +               dev_err(dev, "invalid control-cells %u\n", cells);
>> +               return -EINVAL;
>> +       }
> 
> Maybe put in a comment that the bindings only allow 0 or 1 cell.
> 
Noted it!

>> +
>> +       mux_chip = devm_mux_chip_alloc(dev, cells + 1, sizeof(*mux_gpio));
> 
> Otherwise looks correct to me.
> 
>> -       mux_chip->mux->states = BIT(pins);
>> +
>> +       for (i = 0; i < mux_chip->controllers; ++i)
>> +               mux_chip->mux[i].states = BIT(pins);
> 
> Is the mux core handling any other specifics? (I guess so...)
> 
I guess so too but if there's something specific I'm missing, please let me know.

Moreover, I just realized that I missed two lines that should be probably
adjusted as well:

@@ -78,7 +91,8 @@ static int mux_gpio_probe(struct platform_device *pdev)
                        return -EINVAL;
                }

-               mux_chip->mux->idle_state = idle_state;
+               for (i = 0; i < mux_chip->controllers; ++i)
+                       mux_chip->mux[i].idle_state = idle_state;
        }

        ret = devm_regulator_get_enable_optional(dev, "mux");
@@ -89,8 +103,8 @@ static int mux_gpio_probe(struct platform_device *pdev)
        if (ret < 0)
                return ret;

-       dev_info(dev, "%u-way mux-controller registered\n",
-                mux_chip->mux->states);
+       dev_info(dev, "%u-way mux-controller registered, %u controllers\n",
+                mux_chip->mux->states, mux_chip->controllers);

        return 0;
 }

Should I integrate these modifications on the next version of the patch?

Thank you very much,
Andrea

> With the above added comments, details:
> Reviewed-by: Linus Walleij <linusw@xxxxxxxxxx>
> 
> Thanks!
> Linus Walleij
>