Re: [PATCH v3 2/6] mux: Add helper functions for getting optional and selected mux-state

Geert Uytterhoeven
2025-12-22T10:15:51Z

Hi Josua,

On Wed, 10 Dec 2025 at 18:39, Josua Mayer <josua@solid-run.com> wrote:
> In-tree phy-can-transceiver driver has already implemented a local
> version of devm_mux_state_get_optional.
>
> The omap-i2c driver gets and selects an optional mux in its probe
> function without using any helper.
>
> Add new helper functions covering both aforementioned use-cases:
>
> - devm_mux_state_get_optional:
> Get a mux-state if specified in dt, return NULL otherwise.
> - devm_mux_state_get_optional_selected:
> Get and select a mux-state if specified in dt, return error or NULL.
>
> Existing mux_get helper function is changed to return -ENOENT in case dt
> did not specify a mux-state or -control matching given name (if valid).
> This matches of_parse_phandle_with_args semantics which also returns
> -ENOENT if the property does nto exists, or its value is zero.
>
> The new helper functions check for ENOENT to return NULL for optional
> muxes.
>
> Commit e153fdea9db04 ("phy: can-transceiver: Re-instate "mux-states"
> property presence check") noted that "mux_get() always prints an error
> message in case of an error, including when the property is not present,
> confusing the user."
>
> The first error message covers the case that a mux name is not matched
> in dt. This is removed as the returned error code (-ENOENT) is clear.
>
> The second error message is based on of_parse_phandle_with_args return
> value. In case mux description is missing from DT, it returns -ENOENT.
> Print error message only for other error codes.
>
> This ensures that the new helper functions will not confuse the user
> either.
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Thanks for your patch!

> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -542,11 +542,8 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> else
> index = of_property_match_string(np, "mux-control-names",
> mux_name);
> - if (index < 0) {
> - dev_err(dev, "mux controller '%s' not found\n",
> - mux_name);
> - return ERR_PTR(index);
> - }
> + if (index < 0)
> + return ERR_PTR(-ENOENT);
> }
>
> if (state)
> @@ -558,8 +555,10 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> "mux-controls", "#mux-control-cells",
> index, &args);
> if (ret) {
> - dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
> - np, state ? "state" : "control", mux_name ?: "", index);
> + if (ret != -ENOENT)

I think the non-optional variant should still print an error message in
case of -ENOENT, else this has to be duplicated in all drivers using it.

This is typically handled by having a non-printing core helper function,
and having printing non-optional, and non-printing/ignoring optional wrappers
around the former.

> + dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
> + np, state ? "state" : "control",
> + mux_name ?: "", index);
> return ERR_PTR(ret);
> }
>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds