Re: [PATCH v10 3/9] mux: Add helper functions for getting optional and selected mux-state

Josua Mayer
2026-02-26T10:46:55Z

Am 25.02.26 um 12:34 schrieb Josua Mayer:
> In-tree phy-can-transceiver and phy_rcar_gen3_usb2 have already
> implemented local versions 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.
>
> Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
> drivers/mux/core.c | 206 ++++++++++++++++++++++++++++++++++++-------
> include/linux/mux/consumer.h | 108 ++++++++++++++++++++++-
> 2 files changed, 279 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index f09ee8782e3d..6033da0a9e17 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -46,6 +46,16 @@ static const struct class mux_class = {
> .name = "mux",
> };
>
> +/**
> + * struct devm_mux_state_state - Tracks managed resources for mux-state objects.
> + * @mstate: Pointer to a mux state.
> + * @exit: An optional callback to execute before free.
> + */
> +struct devm_mux_state_state {
> + struct mux_state *mstate;
> + int (*exit)(struct mux_state *mstate);
> +};
> +
> static DEFINE_IDA(mux_ida);
>
> static int __init mux_init(void)
> @@ -516,17 +526,19 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
> return dev ? to_mux_chip(dev) : NULL;
> }
>
> -/*
> +/**
> * mux_get() - Get the mux-control for a device.
> * @dev: The device that needs a mux-control.
> * @mux_name: The name identifying the mux-control.
> * @state: Pointer to where the requested state is returned, or NULL when
> * the required multiplexer states are handled by other means.
> + * @optional: Whether to return NULL and silence errors when mux doesn't exist.
> *
> - * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
> + * Return: Pointer to the mux-control on success, an ERR_PTR with a negative errno on error,
> + * or NULL if optional is true and mux doesn't exist.
> */
> static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> - unsigned int *state)
> + unsigned int *state, bool optional)
> {
> struct device_node *np = dev->of_node;
> struct of_phandle_args args;
> @@ -542,7 +554,9 @@ 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) {
> + if (index < 0 && optional) {
> + return NULL;
> + } else if (index < 0) {
> dev_err(dev, "mux controller '%s' not found\n",
> mux_name);
> return ERR_PTR(index);
> @@ -558,8 +572,12 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> "mux-controls", "#mux-control-cells",
> index, &args);
> if (ret) {
> + if (optional && ret == -ENOENT)
> + return NULL;
> +
> dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
> - np, state ? "state" : "control", mux_name ?: "", index);
> + np, state ? "state" : "control",
> + mux_name ?: "", index);
> return ERR_PTR(ret);
> }
>
> @@ -617,10 +635,29 @@ static struct mux_control *mux_get(struct device *dev, const char *mux_name,
> */
> struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> {
> - return mux_get(dev, mux_name, NULL);
> + struct mux_control *mux = mux_get(dev, mux_name, NULL, false);
> +
> + if (!mux)
> + return ERR_PTR(-ENOENT);
> +
> + return mux;
> }
> EXPORT_SYMBOL_GPL(mux_control_get);
>
> +/**
> + * mux_control_get_optional() - Get the optional mux-control for a device.
> + * @dev: The device that needs a mux-control.
> + * @mux_name: The name identifying the mux-control.
> + *
> + * Return: Pointer to the mux-state on success, an ERR_PTR with a negative errno on error,
> + * or NULL if mux doesn't exist.
> + */
> +struct mux_control *mux_control_get_optional(struct device *dev, const char *mux_name)
> +{
> + return mux_get(dev, mux_name, NULL, true);
> +}
> +EXPORT_SYMBOL_GPL(mux_control_get_optional);
> +
> /**
> * mux_control_put() - Put away the mux-control for good.
> * @mux: The mux-control to put away.
> @@ -657,10 +694,13 @@ struct mux_control *devm_mux_control_get(struct device *dev,
> if (!ptr)
> return ERR_PTR(-ENOMEM);
>
> - mux = mux_control_get(dev, mux_name);
> + mux = mux_get(dev, mux_name, NULL, false);
> if (IS_ERR(mux)) {
> devres_free(ptr);
> return mux;
> + } else if (!mux) {
> + devres_free(ptr);
> + return ERR_PTR(-ENOENT);
> }
>
> *ptr = mux;
> @@ -670,14 +710,16 @@ struct mux_control *devm_mux_control_get(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(devm_mux_control_get);
>
> -/*
> +/**
> * mux_state_get() - Get the mux-state for a device.
> * @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.
> *
> - * Return: A pointer to the mux-state, or an ERR_PTR with a negative errno.
> + * Return: Pointer to the mux-state on success, an ERR_PTR with a negative errno on error,
> + * or NULL if optional is true and mux doesn't exist.
> */
> -static struct mux_state *mux_state_get(struct device *dev, const char *mux_name)
> +static struct mux_state *mux_state_get(struct device *dev, const char *mux_name, bool optional)
> {
> struct mux_state *mstate;
>
> @@ -685,12 +727,16 @@ static struct mux_state *mux_state_get(struct device *dev, const char *mux_name)
> if (!mstate)
> return ERR_PTR(-ENOMEM);
>
> - mstate->mux = mux_get(dev, mux_name, &mstate->state);
> + mstate->mux = mux_get(dev, mux_name, &mstate->state, optional);
> if (IS_ERR(mstate->mux)) {
> - int err = PTR_ERR(mstate->mux);
> -
> kfree(mstate);
> - return ERR_PTR(err);
> + return ERR_CAST(mstate->mux);
kernel robot found a use-after-free here which I will resolve for v11.
> + } else if (optional && !mstate->mux) {
> + kfree(mstate);
> + return NULL;
> + } else if (!mstate->mux) {
> + kfree(mstate);
> + return ERR_PTR(-ENOENT);
> }
>
> return mstate;
> @@ -710,9 +756,66 @@ static void mux_state_put(struct mux_state *mstate)
>
> 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;
>
> + if (devm_state->exit)
> + devm_state->exit(devm_state->mstate);
> +
> + mux_state_put(devm_state->mstate);
> +}
> +
> +/**
> + * __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 on success, an ERR_PTR with a negative errno on error,
> + * or NULL if optional is true and mux doesn't exist.
> + */
> +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 devm_mux_state_state *devm_state;
> + struct mux_state *mstate;
> + int ret;
> +
> + mstate = mux_state_get(dev, mux_name, optional);
> + if (IS_ERR(mstate))
> + return ERR_CAST(mstate);
> + else if (optional && !mstate)
> + return NULL;
> + else if (!mstate)
> + return ERR_PTR(-ENOENT);
> +
> + devm_state = devres_alloc(devm_mux_state_release, sizeof(*devm_state), GFP_KERNEL);
> + if (!devm_state) {
> + ret = -ENOMEM;
> + goto err_devres_alloc;
> + }
> +
> + 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:
> + devres_free(devm_state);
> +err_devres_alloc:
> mux_state_put(mstate);
> + return ERR_PTR(ret);
> }
>
> /**
> @@ -722,28 +825,69 @@ static void devm_mux_state_release(struct device *dev, void *res)
> * @mux_name: The name identifying the mux-control.
> *
> * Return: Pointer to the mux-state, or an ERR_PTR with a negative errno.
> + *
> + * The mux-state will automatically be freed on release.
> */
> -struct mux_state *devm_mux_state_get(struct device *dev,
> - const char *mux_name)
> +struct mux_state *devm_mux_state_get(struct device *dev, const char *mux_name)
> {
> - struct mux_state **ptr, *mstate;
> -
> - ptr = devres_alloc(devm_mux_state_release, sizeof(*ptr), GFP_KERNEL);
> - if (!ptr)
> - return ERR_PTR(-ENOMEM);
> + return __devm_mux_state_get(dev, mux_name, false, NULL, NULL);
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_state_get);
>
> - mstate = mux_state_get(dev, mux_name);
> - if (IS_ERR(mstate)) {
> - devres_free(ptr);
> - return mstate;
> - }
> +/**
> + * devm_mux_state_get_optional() - 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.
> + *
> + * Return: Pointer to the mux-state on success, an ERR_PTR with a negative errno on error,
> + * or NULL if mux doesn't exist.
> + *
> + * The mux-state will automatically be freed on release.
> + */
> +struct mux_state *devm_mux_state_get_optional(struct device *dev, const char *mux_name)
> +{
> + return __devm_mux_state_get(dev, mux_name, true, NULL, NULL);
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_state_get_optional);
>
> - *ptr = mstate;
> - devres_add(dev, ptr);
> +/**
> + * devm_mux_state_get_selected() - Get the mux-state for a device, with
> + * resource management.
> + * @dev: The device that needs a mux-state.
> + * @mux_name: The name identifying the mux-state.
> + *
> + * Return: Pointer to the mux-state, or an ERR_PTR with a negative errno.
> + *
> + * The returned mux-state (if valid) is already selected.
> + *
> + * The mux-state will automatically be deselected and freed on release.
> + */
> +struct mux_state *devm_mux_state_get_selected(struct device *dev, const char *mux_name)
> +{
> + return __devm_mux_state_get(dev, mux_name, false, mux_state_select, mux_state_deselect);
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_state_get_selected);
>
> - return mstate;
> +/**
> + * devm_mux_state_get_optional_selected() - 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.
> + *
> + * Return: Pointer to the mux-state on success, an ERR_PTR with a negative errno on error,
> + * or NULL if mux doesn't exist.
> + *
> + * The returned mux-state (if valid) is already selected.
> + *
> + * The mux-state will automatically be deselected and freed on release.
> + */
> +struct mux_state *devm_mux_state_get_optional_selected(struct device *dev,
> + const char *mux_name)
> +{
> + return __devm_mux_state_get(dev, mux_name, true, mux_state_select, mux_state_deselect);
> }
> -EXPORT_SYMBOL_GPL(devm_mux_state_get);
> +EXPORT_SYMBOL_GPL(devm_mux_state_get_optional_selected);
>
> /*
> * Using subsys_initcall instead of module_init here to try to ensure - for
> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
> index 2e25c838f831..a961861a503b 100644
> --- a/include/linux/mux/consumer.h
> +++ b/include/linux/mux/consumer.h
> @@ -16,6 +16,8 @@ struct device;
> struct mux_control;
> struct mux_state;
>
> +#if IS_ENABLED(CONFIG_MULTIPLEXER)
> +
> unsigned int mux_control_states(struct mux_control *mux);
> int __must_check mux_control_select_delay(struct mux_control *mux,
> unsigned int state,
> @@ -54,11 +56,109 @@ int mux_control_deselect(struct mux_control *mux);
> int mux_state_deselect(struct mux_state *mstate);
>
> struct mux_control *mux_control_get(struct device *dev, const char *mux_name);
> +struct mux_control *mux_control_get_optional(struct device *dev, const char *mux_name);
> void mux_control_put(struct mux_control *mux);
>
> -struct mux_control *devm_mux_control_get(struct device *dev,
> - const char *mux_name);
> -struct mux_state *devm_mux_state_get(struct device *dev,
> - const char *mux_name);
> +struct mux_control *devm_mux_control_get(struct device *dev, const char *mux_name);
> +struct mux_state *devm_mux_state_get(struct device *dev, const char *mux_name);
> +struct mux_state *devm_mux_state_get_optional(struct device *dev, const char *mux_name);
> +struct mux_state *devm_mux_state_get_selected(struct device *dev, const char *mux_name);
> +struct mux_state *devm_mux_state_get_optional_selected(struct device *dev, const char *mux_name);
> +
> +#else
> +
> +static inline unsigned int mux_control_states(struct mux_control *mux)
> +{
> + return 0;
> +}
> +static inline int __must_check mux_control_select_delay(struct mux_control *mux,
> + unsigned int state, unsigned int delay_us)
> +{
> + return -EOPNOTSUPP;
> +}
> +static inline int __must_check mux_state_select_delay(struct mux_state *mstate,
> + unsigned int delay_us)
> +{
> + return -EOPNOTSUPP;
> +}
> +static inline int __must_check mux_control_try_select_delay(struct mux_control *mux,
> + unsigned int state,
> + unsigned int delay_us)
> +{
> + return -EOPNOTSUPP;
> +}
> +static inline int __must_check mux_state_try_select_delay(struct mux_state *mstate,
> + unsigned int delay_us)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int __must_check mux_control_select(struct mux_control *mux,
> + unsigned int state)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int __must_check mux_state_select(struct mux_state *mstate)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int __must_check mux_control_try_select(struct mux_control *mux,
> + unsigned int state)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int __must_check mux_state_try_select(struct mux_state *mstate)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline int mux_control_deselect(struct mux_control *mux)
> +{
> + return -EOPNOTSUPP;
> +}
> +static inline int mux_state_deselect(struct mux_state *mstate)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +static inline struct mux_control *mux_control_get_optional(struct device *dev,
> + const char *mux_name)
> +{
> + return NULL;
> +}
> +static inline void mux_control_put(struct mux_control *mux) {}
> +
> +static inline struct mux_control *devm_mux_control_get(struct device *dev, const char *mux_name)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +static inline struct mux_state *devm_mux_state_get(struct device *dev, const char *mux_name)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +static inline struct mux_state *devm_mux_state_get_optional(struct device *dev,
> + const char *mux_name)
> +{
> + return NULL;
> +}
> +static inline struct mux_state *devm_mux_state_get_selected(struct device *dev,
> + const char *mux_name)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +static inline struct mux_state *devm_mux_state_get_optional_selected(struct device *dev,
> + const char *mux_name)
> +{
> + return NULL;
> +}
> +
> +#endif /* CONFIG_MULTIPLEXER */
>
> #endif /* _LINUX_MUX_CONSUMER_H */
>