Re: [PATCH v1 1/1] mux: Add new API to get mux_control ref by device name.

From: sathyanarayanan kuppuswamy
Date: Mon Jul 10 2017 - 17:38:05 EST

Hi Peter,

Thanks for the comments.

On 07/10/2017 03:07 AM, Peter Rosin wrote:
On 2017-07-09 01:24, Kuppuswamy, Sathyanarayanan wrote:
Hi Peter,

On 7/8/2017 2:12 PM, Peter Rosin wrote:
On 2017-07-08 00:03, sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx wrote:
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>

Currently this driver only provides a single API, mux_control_get() to
get mux_control reference based on mux_name, and also this API has tight
dependency on device tree node. For devices, that does not use device
tree, it makes it difficult to use this API. This patch adds new
API to access mux_control reference based on device name, chip index and
controller index value.
I assume this is for the Intel USB Multiplexer that you sent a driver for
a month or so ago? If so, you still have not answered these questions:
I am not planning to merge the Intel USB MUX driver any more. I agree
with Hans comments
and decided not to proceed further on this approach.

But I created these helper functions to get my driver working with MUX
framework. Since these
helper functions can be useful for any non-dt drivers who wants to use
MUX framework, I thought
to submit these changes for review.
Is any other consumer in the charts at all? Can this existing consumer
ever make use of some other mux? If the answer to both those questions
are 'no', then I do not see much point in involving the mux subsystem at
all. The Broxton USB PHY driver could just as well write to the register
all by itself, no?

that I asked in

What is the point of that driver?

Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
drivers/mux/mux-core.c | 114 +++++++++++++++++++++++++++++++++++++++++++
include/linux/mux/consumer.h | 6 ++-
2 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/drivers/mux/mux-core.c b/drivers/mux/mux-core.c
index 90b8995..f8796b9 100644
--- a/drivers/mux/mux-core.c
+++ b/drivers/mux/mux-core.c
@@ -422,6 +422,87 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
return dev ? to_mux_chip(dev) : NULL;
+static int dev_parent_name_match(struct device *dev, const void *data)
+ const char *devname = dev_name(dev->parent);
+ unsigned int i;
+ if (!devname || !data)
+ return 0;
+ for (i = 0; i < strlen(devname); i++) {
+ if (devname[i] == '.')
+ break;
+ }
+ return !strncmp(devname, data, i-1);
Ouch, strlen as a termination test is wasteful, you want to remove the loop
and do something like this

return !strncmp(devname, data, strcspn(devname, "."));
will fix it in next version.
+ * mux_chip_get_by_index() - Get the mux-chip associated with give device.
+ * @devname: Name of the device which registered the mux-chip.
+ * @index: Index of the mux chip.
+ *
+ * Return: A pointer to the mux-chip, or an ERR_PTR with a negative errno.
+ */
+static struct mux_chip *mux_chip_get_by_index(const char *devname, int index)
+ struct device *dev;
+ int found = -1;
+ if (!devname)
+ return ERR_PTR(-EINVAL);
+ do {
+ dev = class_find_device(&mux_class, NULL, devname,
+ dev_parent_name_match);
+ if (dev != NULL)
+ found++;
+ if (found >= index)
+ break;
+ } while (dev != NULL);
This loop is broken. class_find_device will always return the same device.
Good catch. I did not test the case with multiple chips. So I failed to
notice this.
Also, if you fix the loop, why is the ordering stable and something to rely
You failed to comment on this very important point. Sorry for not putting
more emphasis on it. So, before you waste more time on the indexed approach,
have a look at e.g. the pwm core with its pwm_get (which takes a name) and
its *deprecated* pwm_request (which takes an index).

I think having a lookup table (like pwm) is closer to what the mux core
should do. Or something like that.
Let me go through it and get back to you.


Sathyanarayanan Kuppuswamy
Linux kernel developer