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

Josua Mayer posted 6 patches 2 months ago
There is a newer version of this series
[PATCH v3 2/6] mux: Add helper functions for getting optional and selected mux-state
Posted by Josua Mayer 2 months ago
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>
---
 drivers/mux/core.c           | 67 +++++++++++++++++++++++++++++++++++++++-----
 include/linux/mux/consumer.h |  4 +++
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index a3840fe0995fe..130ca47a8be37 100644
--- 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)
+			dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
+				np, state ? "state" : "control",
+				mux_name ?: "", index);
 		return ERR_PTR(ret);
 	}
 
@@ -745,6 +744,60 @@ struct mux_state *devm_mux_state_get(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_mux_state_get);
 
+/**
+ * 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, or an ERR_PTR with a negative errno.
+ */
+struct mux_state *devm_mux_state_get_optional(struct device *dev,
+					      const char *mux_name)
+{
+	struct mux_state *mux_state = devm_mux_state_get(dev, mux_name);
+
+	if (IS_ERR(mux_state) && PTR_ERR(mux_state) == -ENOENT)
+		return NULL;
+
+	return mux_state;
+}
+EXPORT_SYMBOL_GPL(devm_mux_state_get_optional);
+
+/**
+ * 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, or an ERR_PTR with a negative errno.
+ *
+ * The returned mux-state (if valid) is already selected.
+ */
+struct mux_state *devm_mux_state_get_optional_selected(struct device *dev,
+						       const char *mux_name)
+{
+	struct mux_state *mux_state;
+	int ret;
+
+	mux_state = devm_mux_state_get_optional(dev, mux_name);
+	if (IS_ERR_OR_NULL(mux_state))
+		return mux_state;
+
+	ret = mux_state_select(mux_state);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to select mux-state %s: %d\n",
+				mux_name ?: "", ret);
+
+		mux_state_put(mux_state);
+		return ERR_PTR(ret);
+	}
+
+	return mux_state;
+}
+EXPORT_SYMBOL_GPL(devm_mux_state_get_optional_selected);
+
 /*
  * Using subsys_initcall instead of module_init here to try to ensure - for
  * the non-modular case - that the subsystem is initialized when mux consumers
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index 2e25c838f8312..a5da2e33a45c0 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -60,5 +60,9 @@ 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_optional_selected(struct device *dev,
+						       const char *mux_name);
 
 #endif /* _LINUX_MUX_CONSUMER_H */

-- 
2.51.0
Re: [PATCH v3 2/6] mux: Add helper functions for getting optional and selected mux-state
Posted by Geert Uytterhoeven 1 month, 2 weeks ago
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
Re: [PATCH v3 2/6] mux: Add helper functions for getting optional and selected mux-state
Posted by Ulf Hansson 1 month, 3 weeks ago
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>
> ---
>  drivers/mux/core.c           | 67 +++++++++++++++++++++++++++++++++++++++-----
>  include/linux/mux/consumer.h |  4 +++
>  2 files changed, 64 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index a3840fe0995fe..130ca47a8be37 100644
> --- 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)
> +                       dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
> +                               np, state ? "state" : "control",
> +                               mux_name ?: "", index);
>                 return ERR_PTR(ret);
>         }
>
> @@ -745,6 +744,60 @@ struct mux_state *devm_mux_state_get(struct device *dev,
>  }
>  EXPORT_SYMBOL_GPL(devm_mux_state_get);
>
> +/**
> + * 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, or an ERR_PTR with a negative errno.
> + */
> +struct mux_state *devm_mux_state_get_optional(struct device *dev,
> +                                             const char *mux_name)
> +{
> +       struct mux_state *mux_state = devm_mux_state_get(dev, mux_name);
> +
> +       if (IS_ERR(mux_state) && PTR_ERR(mux_state) == -ENOENT)
> +               return NULL;
> +
> +       return mux_state;
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_state_get_optional);
> +
> +/**
> + * 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, or an ERR_PTR with a negative errno.
> + *
> + * The returned mux-state (if valid) is already selected.
> + */
> +struct mux_state *devm_mux_state_get_optional_selected(struct device *dev,
> +                                                      const char *mux_name)
> +{
> +       struct mux_state *mux_state;
> +       int ret;
> +
> +       mux_state = devm_mux_state_get_optional(dev, mux_name);
> +       if (IS_ERR_OR_NULL(mux_state))
> +               return mux_state;
> +
> +       ret = mux_state_select(mux_state);
> +       if (ret) {
> +               if (ret != -EPROBE_DEFER)
> +                       dev_err(dev, "failed to select mux-state %s: %d\n",
> +                               mux_name ?: "", ret);
> +
> +               mux_state_put(mux_state);
> +               return ERR_PTR(ret);
> +       }
> +
> +       return mux_state;
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_state_get_optional_selected);
> +
>  /*
>   * Using subsys_initcall instead of module_init here to try to ensure - for
>   * the non-modular case - that the subsystem is initialized when mux consumers
> diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
> index 2e25c838f8312..a5da2e33a45c0 100644
> --- a/include/linux/mux/consumer.h
> +++ b/include/linux/mux/consumer.h
> @@ -60,5 +60,9 @@ 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_optional_selected(struct device *dev,
> +                                                      const char *mux_name);

Seems like we need stub-functions of these too. Otherwise
subsystems/drivers need to have a "depends on MULTIPLEXER" in their
Kconfigs.

Kind regards
Uffe