[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPDyKFrBuL+747QUJJUejtcvjm0V7Lt=vHwjvcMdCt_h4=sFwg@mail.gmail.com>
Date: Thu, 29 Jan 2026 17:06:10 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Josua Mayer <josua@...id-run.com>
Cc: Marc Kleine-Budde <mkl@...gutronix.de>, Vincent Mailhol <mailhol@...nel.org>,
Vinod Koul <vkoul@...nel.org>, Neil Armstrong <neil.armstrong@...aro.org>,
Peter Rosin <peda@...ntia.se>, Aaro Koskinen <aaro.koskinen@....fi>,
Andreas Kemnade <andreas@...nade.info>, Kevin Hilman <khilman@...libre.com>,
Roger Quadros <rogerq@...nel.org>, Tony Lindgren <tony@...mide.com>,
Janusz Krzysztofik <jmkrzyszt@...il.com>, Vignesh R <vigneshr@...com>,
Andi Shyti <andi.shyti@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Geert Uytterhoeven <geert+renesas@...der.be>, Magnus Damm <magnus.damm@...il.com>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
Yazan Shhady <yazan.shhady@...id-run.com>, Jon Nettleton <jon@...id-run.com>,
Mikhail Anikin <mikhail.anikin@...id-run.com>, linux-can@...r.kernel.org,
linux-phy@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-omap@...r.kernel.org, linux-i2c@...r.kernel.org,
linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH v7 2/7] mux: Add helper functions for getting optional and
selected mux-state
On Wed, 28 Jan 2026 at 15:46, Josua Mayer <josua@...id-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:
>
> - mux_control_get_optional:
> Get a mux-control if specified in dt, return NULL otherwise.
> - devm_mux_state_get_optional:
> Get a mux-state if specified in dt, return NULL otherwise.
> - devm_mux_state_get_selected:
> Get and select a mux-state specified in dt, return error 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 take an extra argument
> indicating whether the mux is optional.
> In this case no error is printed, and NULL returned in case of ENOENT.
>
> Calling code is adapted to handle NULL return case, and to pass optional
> argument as required.
>
> To support automatic deselect for _selected helper, a new structure is
> created storing an exit pointer similar to clock core which is called on
> release.
>
> To facilitate code sharing between optional/mandatory/selected helpers,
> a new internal helper function is added to handle quiet (optional) and
> verbose (mandatory) errors, as well as storing the correct callback for
> devm release: __devm_mux_state_get
>
> Due to this structure devm_mux_state_get_*_selected can no longer print
> a useful error message when select fails. Instead callers should print
> errors where needed.
>
> 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. The second error message is based on of_parse_phandle_with_args
> return value.
>
> In optional case no error is printed and NULL is returned.
> This ensures that the new helper functions will not confuse the user
> either.
>
> With the addition of optional helper functions it became clear that
> drivers should compile and link even if CONFIG_MULTIPLEXER was not enabled.
> Add stubs for all symbols exported by mux core.
>
> Signed-off-by: Josua Mayer <josua@...id-run.com>
> ---
> drivers/mux/core.c | 178 ++++++++++++++++++++++++++++++++++++-------
> include/linux/mux/consumer.h | 108 +++++++++++++++++++++++++-
> 2 files changed, 253 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index a3840fe0995f..b01ec126caaf 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
[...]
> static void devm_mux_state_release(struct device *dev, void *res)
> {
> - struct mux_state *mstate = *(struct mux_state **)res;
> + struct devm_mux_state_state *devm_state = res;
>
> - mux_state_put(mstate);
> + if (devm_state->exit)
> + devm_state->exit(devm_state->mstate);
> +
> + mux_state_put(devm_state->mstate);
> }
>
> /**
> - * devm_mux_state_get() - Get the mux-state for a device, with resource
> - * management.
> - * @dev: The device that needs a mux-control.
> - * @mux_name: The name identifying the mux-control.
> + * __devm_mux_state_get() - Get the optional mux-state for a device,
> + * with resource management.
> + * @dev: The device that needs a mux-state.
> + * @mux_name: The name identifying the mux-state.
> + * @optional: Whether to return NULL and silence errors when mux doesn't exist.
> + * @init: Optional function pointer for mux-state object initialisation.
> + * @exit: Optional function pointer for mux-state object cleanup on release.
> *
> * Return: Pointer to the mux-state, or an ERR_PTR with a negative errno.
> */
> -struct mux_state *devm_mux_state_get(struct device *dev,
> - const char *mux_name)
> +static struct mux_state *__devm_mux_state_get(struct device *dev, const char *mux_name,
> + bool optional,
> + int (*init)(struct mux_state *mstate),
> + int (*exit)(struct mux_state *mstate))
> {
> - struct mux_state **ptr, *mstate;
> + struct devm_mux_state_state *devm_state;
> + struct mux_state *mstate;
> + int ret;
>
> - ptr = devres_alloc(devm_mux_state_release, sizeof(*ptr), GFP_KERNEL);
> - if (!ptr)
> + devm_state = devres_alloc(devm_mux_state_release, sizeof(*devm_state), GFP_KERNEL);
> + if (!devm_state)
> return ERR_PTR(-ENOMEM);
>
> - mstate = mux_state_get(dev, mux_name);
> - if (IS_ERR(mstate)) {
> - devres_free(ptr);
> - return mstate;
> + mstate = mux_state_get(dev, mux_name, optional);
> + if (IS_ERR_OR_NULL(mstate)) {
> + ret = PTR_ERR(mstate);
Should this be PTR_ERR_OR_ZERO?
> + goto err_mux_state_get;
> }
>
> - *ptr = mstate;
> - devres_add(dev, ptr);
> + if (init) {
> + ret = init(mstate);
> + if (ret)
> + goto err_mux_state_init;
> + }
> +
> + devm_state->mstate = mstate;
> + devm_state->exit = exit;
> + devres_add(dev, devm_state);
>
> return mstate;
> +
> +err_mux_state_init:
> + mux_state_put(mstate);
> +err_mux_state_get:
> + devres_free(devm_state);
> + return ERR_PTR(ret);
If PTR_ERR_OR_ZERO is used above as I suggested, ret may be 0 here.
Then it is probably clearer to add a check if (!ret) and return NULL
explicitly for that case, I think.
[...]
Kind regards
Uffe
Powered by blists - more mailing lists