Hi Peter,

On 30/11/21 11:14 am, Aswath Govindraju wrote:

Hi Peter,

On 25/11/21 7:22 pm, Peter Rosin wrote:

quoted

Hi!

On 2021-11-23 09:12, Aswath Govindraju wrote:

quoted

In some cases, we might need to provide the state of the mux to be set for
the operation of a given peripheral. Therefore, pass this information using
the second argument of the mux-controls property.

Signed-off-by: Aswath Govindraju <redacted>
---
 drivers/mux/core.c           | 146 ++++++++++++++++++++++++++++++++++-
 include/linux/mux/consumer.h |  19 ++++-
 include/linux/mux/driver.h   |  13 ++++
 3 files changed, 173 insertions(+), 5 deletions(-)

diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 22f4709768d1..9622b98f9818 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c

@@ -370,6 +370,29 @@ int mux_control_select_delay(struct mux_control *mux, unsigned int state,
 }
 EXPORT_SYMBOL_GPL(mux_control_select_delay);
 

[...]

quoted

quoted

 }
 
 /**
- * mux_control_get() - Get the mux-control for a device.
+ * 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.
+ * @enable_state: The variable to store the enable state for the requested device
  *
  * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
  */
-struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+static struct mux_control *mux_get(struct device *dev, const char *mux_name,
+				   unsigned int *enable_state)

s/enable_state/state/ (goes for all of the patch).

quoted

 {
 	struct device_node *np = dev->of_node;
 	struct of_phandle_args args;

@@ -481,8 +545,7 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
 	if (!mux_chip)
 		return ERR_PTR(-EPROBE_DEFER);
 
-	if (args.args_count > 1 ||

It is inconsistent to allow more than 2 args, but then only allow
digging out the state from the 2nd arg if the count is exactly 2.

quoted

-	    (!args.args_count && (mux_chip->controllers > 1))) {
+	if (!args.args_count && mux_chip->controllers > 1) {
 		dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
 			np, args.np);
 		put_device(&mux_chip->dev);

@@ -500,8 +563,25 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
 		return ERR_PTR(-EINVAL);
 	}
 
+	if (args.args_count == 2)
+		*enable_state = args.args[1];
+

With the suggested binding in my comment for patch 1/4, you'd need to do
either

	ret = of_parse_phandle_with_args(np,
					 "mux-controls", "#mux-control-cells",
					 index, &args);

or

	ret = of_parse_phandle_with_args(np,
					 "mux-states", "#mux-state-cells",
					 index, &args);

depending on if the mux_get helper gets a NULL (enable_)state pointer or a "real"
address, and then...

Sorry, while trying to implement the above method, I came across one
more question. So, in a given consumer DT node we will be either having
only mux-states or mux-controls right? How would we take care of the
condition when we would want to set the state of a given line in the
controller. Especially when a single mux chip is used by multiple
consumers each using a different line. Wouldn't we require both
mux-controls and mux-states in that case? So, shouldn't the
implementation be such that we need to first read mux-controls and then
based whether the enable_state is NULL, we read mux-states?

Just to add more clarity, if we go about this method then we would have
both mux-controls and mux-states in the consumer DT node when we want to
specify the state.

I now understood the implementation that you described. mux-states will
be similar to the mux-controls after this patch is applied. mux-states
can have 2 arguments(mux line and state) whereas mux-controls can have
only one argument(mux line).

Sorry, for the inconvenience.

Thanks,
Aswath

Thanks,
Aswath

quoted

quoted

 	return &mux_chip->mux[controller];
 }
+
+/**
+ * mux_control_get() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+{

[...]