linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/7] mux controller abstraction and iio/i2c muxes
@ 2016-11-17 21:48 Peter Rosin
  2016-11-17 21:48 ` [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio Peter Rosin
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Peter Rosin @ 2016-11-17 21:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c, devicetree, linux-iio

Hi!

This is work in progress, I'm asking for early feedback.
The code depends on the _available work in iio which can
be found in linux-next.

v1 -> v2 changes
- fixup export of mux_control_put reported by kbuild
- drop devicetree iio-ext-info property as noted by Lars-Peter,
  and replace the functionality by exposing all ext_info
  attributes of the parent channel for each of the muxed
  channels. A cache on top of that and each muxed channel
  gets its own view of the ext_info of the parent channel.
- implement idle-state for muxes
- clear out the cache on failure in order to force a mux
  update on the following use
- cleanup the probe of i2c-mux-simple driver
- fix a bug in the i2c-mux-simple driver, where failure in
  the selection of the mux caused a deadlock when the mux
  was later unconditionally deselected.

I have a piece of hardware that is using the same 3 GPIO pins
to control four 8-way muxes. Three of them control ADC lines
to an ADS1015 chip with an iio driver, and the last one
controls the SDA line of an i2c bus. We have some deployed
code to handle this, but you do not want to see it or ever
hear about it. I'm not sure why I even mention it. Anyway,
the situation has nagged me to no end for quite some time.

So, after first getting more intimate with the i2c muxing code
and later discovering the drivers/iio/inkern.c file and
writing a couple of drivers making use of it, I came up with
what I think is an acceptable solution; add a generic mux
controller driver that is shared between all instances, and
combine that with an iio mux driver and a new generic i2c mux
driver. The new i2c mux I called "simple" since it is only
hooking the i2c muxing and the new mux controller (much like
the alsa simple card driver does for ASoC).

My initial (private) version didn't add "mux" as a new bus,
but I couldn't get decent type-checking and nice devicetree
integration with that. It does however feel a bit rich to
add a new bus for something as small as mux controllers?

One thing that I would like to do, but don't see a solution
for, is to move the mux control code that is present in
various drivers in drivers/i2c/muxes to this new minimalistic
muxing subsystem, thus converting all present i2c muxes (but
perhaps not gates and arbitrators) to be i2c-mux-simple muxes.

I'm using an rwsem to lock a mux, but that isn't really a
perfect fit. Is there a better locking primitive that I don't
know about that fits better? I had a mutex at one point, but
that didn't allow any concurrent accesses at all. At least
the rwsem allows concurrent access as long as all users
agree on the mux state, but I suspect that the rwsem will
degrade to the mutex situation pretty quickly if there is
any contention.

Also, the "mux" name feels a bit ambitious, there are many muxes
in the world, and this tiny bit of code is probably not good
enough to be a nice fit for all...

This is all very fresh code and only lightly tested, but it
feels very promising! Now, go ahead and rip this to pieces...

Cheers,
Peter

Peter Rosin (7):
  dt-bindings: document devicetree bindings for mux-gpio
  misc: minimal mux subsystem and gpio-based mux controller
  iio: inkern: api for manipulating ext_info of iio channels
  dt-bindings: iio: iio-mux: document iio-mux bindings
  iio: multiplexer: new iio category and iio-mux driver
  dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings
  i2c: i2c-mux-simple: new driver

 .../devicetree/bindings/i2c/i2c-mux-simple.txt     |  91 +++++
 .../bindings/iio/multiplexer/iio-mux.txt           |  49 +++
 .../devicetree/bindings/misc/mux-gpio.txt          |  79 ++++
 drivers/i2c/muxes/Kconfig                          |  12 +
 drivers/i2c/muxes/Makefile                         |   1 +
 drivers/i2c/muxes/i2c-mux-simple.c                 | 168 ++++++++
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/inkern.c                               |  55 +++
 drivers/iio/multiplexer/Kconfig                    |  17 +
 drivers/iio/multiplexer/Makefile                   |   6 +
 drivers/iio/multiplexer/iio-mux.c                  | 444 +++++++++++++++++++++
 drivers/misc/Kconfig                               |   6 +
 drivers/misc/Makefile                              |   2 +
 drivers/misc/mux-core.c                            | 299 ++++++++++++++
 drivers/misc/mux-gpio.c                            | 115 ++++++
 include/linux/iio/consumer.h                       |   6 +
 include/linux/mux.h                                |  53 +++
 18 files changed, 1405 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
 create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
 create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt
 create mode 100644 drivers/i2c/muxes/i2c-mux-simple.c
 create mode 100644 drivers/iio/multiplexer/Kconfig
 create mode 100644 drivers/iio/multiplexer/Makefile
 create mode 100644 drivers/iio/multiplexer/iio-mux.c
 create mode 100644 drivers/misc/mux-core.c
 create mode 100644 drivers/misc/mux-gpio.c
 create mode 100644 include/linux/mux.h

-- 
2.1.4

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio
  2016-11-17 21:48 [RFC PATCH v2 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
@ 2016-11-17 21:48 ` Peter Rosin
  2016-11-18 15:35   ` Rob Herring
  2016-11-17 21:48 ` [RFC PATCH v2 2/7] misc: minimal mux subsystem and gpio-based mux controller Peter Rosin
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-11-17 21:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c, devicetree, linux-iio

---
 .../devicetree/bindings/misc/mux-gpio.txt          | 79 ++++++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt

diff --git a/Documentation/devicetree/bindings/misc/mux-gpio.txt b/Documentation/devicetree/bindings/misc/mux-gpio.txt
new file mode 100644
index 000000000000..73699a37824f
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/mux-gpio.txt
@@ -0,0 +1,79 @@
+GPIO-based multiplexer controller bindings
+
+Define what GPIO pins are used to control a multiplexer. Or several
+multiplexers, if the same pins control more than one multiplexer.
+
+Required properties:
+- compatible : "mux-gpio"
+- mux-gpios : list of gpios used to control the multiplexer, least
+	      significant bit first.
+
+Optional properties:
+- idle-state : if present, the state the mux will have when idle.
+
+Example:
+	control_mux: control-adc-mux {
+		compatible = "mux-gpio";
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "iio-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		control-muxes = <&control_mux>;
+		control-mux-names = "mux";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		sync-1@0 {
+			reg = <0>;
+		};
+
+		in@1 {
+			reg = <1>;
+		};
+
+		out@2 {
+			reg = <2>;
+		};
+
+		sync-2@3 {
+			reg = <3>;
+		};
+	};
+
+	i2c-mux {
+		compatible = "i2c-mux-simple,mux-locked";
+		i2c-parent = <&i2c1>;
+
+		control-muxes = <&control_mux>;
+		control-mux-names = "mux";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ssd1307: oled@3c {
+				/* ... */
+			};
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555@20 {
+				/* ... */
+			};
+		};
+	};
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 2/7] misc: minimal mux subsystem and gpio-based mux controller
  2016-11-17 21:48 [RFC PATCH v2 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
  2016-11-17 21:48 ` [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio Peter Rosin
@ 2016-11-17 21:48 ` Peter Rosin
  2016-11-19 15:34   ` Jonathan Cameron
  2016-11-17 21:48 ` [RFC PATCH v2 3/7] iio: inkern: api for manipulating ext_info of iio channels Peter Rosin
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-11-17 21:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c, devicetree, linux-iio

When both the iio subsystem and the i2c subsystem wants to update
the same mux, there needs to be some coordination. Invent a new
minimal "mux" subsystem that handles this.

Add a single backend driver for this new subsystem that can
control gpio based multiplexers.
---
 drivers/misc/Kconfig    |   6 +
 drivers/misc/Makefile   |   2 +
 drivers/misc/mux-core.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/misc/mux-gpio.c | 115 +++++++++++++++++++
 include/linux/mux.h     |  53 +++++++++
 5 files changed, 475 insertions(+)
 create mode 100644 drivers/misc/mux-core.c
 create mode 100644 drivers/misc/mux-gpio.c
 create mode 100644 include/linux/mux.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 64971baf11fa..9e119bb78d82 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -766,6 +766,12 @@ config PANEL_BOOT_MESSAGE
 	  An empty message will only clear the display at driver init time. Any other
 	  printf()-formatted message is valid with newline and escape codes.
 
+config MUX_GPIO
+	tristate "GPIO-controlled MUX controller"
+	depends on OF
+	help
+	  GPIO-controlled MUX controller
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 31983366090a..92b547bcbac1 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -53,6 +53,8 @@ obj-$(CONFIG_ECHO)		+= echo/
 obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
 obj-$(CONFIG_CXL_BASE)		+= cxl/
 obj-$(CONFIG_PANEL)             += panel.o
+obj-$(CONFIG_MUX_GPIO)          += mux-core.o
+obj-$(CONFIG_MUX_GPIO)          += mux-gpio.o
 
 lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
 lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
diff --git a/drivers/misc/mux-core.c b/drivers/misc/mux-core.c
new file mode 100644
index 000000000000..7a8bf003a92c
--- /dev/null
+++ b/drivers/misc/mux-core.c
@@ -0,0 +1,299 @@
+/*
+ * Multiplexer subsystem
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "mux-core: " fmt
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/idr.h>
+#include <linux/module.h>
+#include <linux/mux.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+static struct bus_type mux_bus_type = {
+	.name = "mux",
+};
+
+static int __init mux_init(void)
+{
+	return bus_register(&mux_bus_type);
+}
+
+static void __exit mux_exit(void)
+{
+	bus_unregister(&mux_bus_type);
+}
+
+static DEFINE_IDA(mux_ida);
+
+static void mux_control_release(struct device *dev)
+{
+	struct mux_control *mux = to_mux_control(dev);
+
+	ida_simple_remove(&mux_ida, mux->id);
+	kfree(mux);
+}
+
+static struct device_type mux_control_type = {
+	.name = "mux-control",
+	.release = mux_control_release,
+};
+
+/*
+ * Allocate a mux-control, plus an extra memory area for private use
+ * by the caller.
+ */
+struct mux_control *mux_control_alloc(size_t sizeof_priv)
+{
+	struct mux_control *mux;
+
+	mux = kzalloc(sizeof(*mux) + sizeof_priv, GFP_KERNEL);
+	if (!mux)
+		return NULL;
+
+	mux->dev.bus = &mux_bus_type;
+	mux->dev.type = &mux_control_type;
+	device_initialize(&mux->dev);
+	dev_set_drvdata(&mux->dev, mux);
+
+	init_rwsem(&mux->lock);
+	mux->priv = mux + 1;
+
+	mux->id = ida_simple_get(&mux_ida, 0, 0, GFP_KERNEL);
+	if (mux->id < 0) {
+		pr_err("mux-controlX failed to get device id\n");
+		kfree(mux);
+		return NULL;
+	}
+	dev_set_name(&mux->dev, "mux:control%d", mux->id);
+
+	mux->cached_state = -1;
+	mux->idle_state = -1;
+
+	return mux;
+}
+EXPORT_SYMBOL_GPL(mux_control_alloc);
+
+/*
+ * Register the mux-control, thus readying it for use.
+ */
+int mux_control_register(struct mux_control *mux)
+{
+	/* If the calling driver did not initialize of_node, do it here */
+	if (!mux->dev.of_node && mux->dev.parent)
+		mux->dev.of_node = mux->dev.parent->of_node;
+
+	return device_add(&mux->dev);
+}
+EXPORT_SYMBOL_GPL(mux_control_register);
+
+/*
+ * Take the mux-control off-line.
+ */
+void mux_control_unregister(struct mux_control *mux)
+{
+	device_del(&mux->dev);
+}
+EXPORT_SYMBOL_GPL(mux_control_unregister);
+
+/*
+ * Put away the mux-control for good.
+ */
+void mux_control_put(struct mux_control *mux)
+{
+	if (!mux)
+		return;
+	put_device(&mux->dev);
+}
+EXPORT_SYMBOL_GPL(mux_control_put);
+
+static int mux_control_set(struct mux_control *mux, int state)
+{
+	int ret = mux->ops->set(mux, state);
+
+	mux->cached_state = ret < 0 ? -1 : state;
+
+	return ret;
+}
+
+/*
+ * Select the given multiplexer channel. Call mux_control_deselect()
+ * when the operation is complete on the multiplexer channel, and the
+ * multiplexer is free for others to use.
+ */
+int mux_control_select(struct mux_control *mux, int state)
+{
+	int ret;
+
+	if (down_read_trylock(&mux->lock)) {
+		if (mux->cached_state == state)
+			return 0;
+
+		/* Sigh, the mux needs updating... */
+		up_read(&mux->lock);
+	}
+
+	/* ...or it's just contended. */
+	down_write(&mux->lock);
+
+	if (mux->cached_state == state) {
+		/*
+		 * Hmmm, someone else changed the mux to my liking.
+		 * That makes me wonder how long I waited for nothing...
+		 */
+		downgrade_write(&mux->lock);
+		return 0;
+	}
+
+	ret = mux_control_set(mux, state);
+	if (ret < 0) {
+		if (mux->idle_state != -1)
+			mux_control_set(mux, mux->idle_state);
+
+		up_write(&mux->lock);
+		return ret;
+	}
+
+	downgrade_write(&mux->lock);
+
+	return 1;
+}
+EXPORT_SYMBOL_GPL(mux_control_select);
+
+/*
+ * Deselect the previously selected multiplexer channel.
+ */
+int mux_control_deselect(struct mux_control *mux)
+{
+	int ret = 0;
+
+	if (mux->idle_state != -1 && mux->cached_state != mux->idle_state)
+		ret = mux_control_set(mux, mux->idle_state);
+
+	up_read(&mux->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mux_control_deselect);
+
+static int of_dev_node_match(struct device *dev, void *data)
+{
+	return dev->of_node == data;
+}
+
+static struct mux_control *of_find_mux_by_node(struct device_node *np)
+{
+	struct device *dev;
+
+	dev = bus_find_device(&mux_bus_type, NULL, np, of_dev_node_match);
+
+	return dev ? to_mux_control(dev) : NULL;
+}
+
+static struct mux_control *of_mux_control_get(struct device_node *np, int index)
+{
+	struct device_node *mux_np;
+	struct mux_control *mux;
+
+	mux_np = of_parse_phandle(np, "control-muxes", index);
+	if (!mux_np)
+		return NULL;
+
+	mux = of_find_mux_by_node(mux_np);
+	of_node_put(mux_np);
+
+	return mux;
+}
+
+/*
+ * Get a named mux.
+ */
+struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+{
+	struct device_node *np = dev->of_node;
+	struct mux_control *mux;
+	int index;
+
+	index = of_property_match_string(np, "control-mux-names", mux_name);
+	if (index < 0) {
+		dev_err(dev, "failed to get control-mux %s:%s(%i)\n",
+			np->full_name, mux_name ?: "", index);
+		return ERR_PTR(index);
+	}
+
+	mux = of_mux_control_get(np, index);
+	if (!mux)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return mux;
+}
+EXPORT_SYMBOL_GPL(mux_control_get);
+
+static void devm_mux_control_free(struct device *dev, void *res)
+{
+	struct mux_control *mux = *(struct mux_control **)res;
+
+	mux_control_put(mux);
+}
+
+/*
+ * Get a named mux, with resource management.
+ */
+struct mux_control *devm_mux_control_get(struct device *dev,
+					 const char *mux_name)
+{
+	struct mux_control **ptr, *mux;
+
+	ptr = devres_alloc(devm_mux_control_free, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	mux = mux_control_get(dev, mux_name);
+	if (IS_ERR(mux)) {
+		devres_free(ptr);
+		return mux;
+	}
+
+	*ptr = mux;
+	devres_add(dev, ptr);
+
+	return mux;
+}
+EXPORT_SYMBOL_GPL(devm_mux_control_get);
+
+static int devm_mux_control_match(struct device *dev, void *res, void *data)
+{
+	struct mux_control **r = res;
+
+	if (!r || !*r) {
+		WARN_ON(!r || !*r);
+		return 0;
+	}
+
+	return *r == data;
+}
+
+/*
+ * Resource-managed version mux_control_put.
+ */
+void devm_mux_control_put(struct device *dev, struct mux_control *mux)
+{
+	WARN_ON(devres_release(dev, devm_mux_control_free,
+			       devm_mux_control_match, mux));
+}
+EXPORT_SYMBOL_GPL(devm_mux_control_put);
+
+subsys_initcall(mux_init);
+module_exit(mux_exit);
+
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
+MODULE_DESCRIPTION("MUX subsystem");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/misc/mux-gpio.c b/drivers/misc/mux-gpio.c
new file mode 100644
index 000000000000..2ddd7fb24078
--- /dev/null
+++ b/drivers/misc/mux-gpio.c
@@ -0,0 +1,115 @@
+/*
+ * GPIO-controlled multiplexer driver
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/mux.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+struct mux_gpio {
+	struct gpio_descs *gpios;
+};
+
+static int mux_gpio_set(struct mux_control *mux, int val)
+{
+	struct mux_gpio *mux_gpio = mux->priv;
+	int i;
+
+	for (i = 0; i < mux_gpio->gpios->ndescs; i++)
+		gpiod_set_value_cansleep(mux_gpio->gpios->desc[i],
+					 val & (1 << i));
+
+	return 0;
+}
+
+static const struct mux_control_ops mux_gpio_ops = {
+	.set = mux_gpio_set,
+};
+
+static const struct of_device_id mux_gpio_dt_ids[] = {
+	{ .compatible = "mux-gpio", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
+
+static int mux_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	struct mux_control *mux;
+	struct mux_gpio *mux_gpio;
+	u32 idle_state;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	mux = mux_control_alloc(sizeof(*mux_gpio));
+	if (!mux)
+		return -ENOMEM;
+	mux_gpio = mux->priv;
+	mux->dev.parent = dev;
+	mux->ops = &mux_gpio_ops;
+
+	platform_set_drvdata(pdev, mux);
+
+	mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
+	if (IS_ERR(mux_gpio->gpios)) {
+		if (PTR_ERR(mux_gpio->gpios) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get gpios\n");
+		mux_control_put(mux);
+		return PTR_ERR(mux_gpio->gpios);
+	}
+
+	ret = of_property_read_u32(np, "idle-state", &idle_state);
+	if (ret >= 0) {
+		if (idle_state >= (1 << mux_gpio->gpios->ndescs)) {
+			dev_err(dev, "invalid idle-state %u\n", idle_state);
+			return -EINVAL;
+		}
+		mux->idle_state = idle_state;
+	}
+
+	ret = mux_control_register(mux);
+	if (ret < 0) {
+		dev_err(dev, "failed to register mux_control\n");
+		mux_control_put(mux);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int mux_gpio_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mux_control *mux = to_mux_control(dev);
+
+	mux_control_unregister(mux);
+	mux_control_put(mux);
+	return 0;
+}
+
+static struct platform_driver mux_gpio_driver = {
+	.driver = {
+		.name = "mux-gpio",
+		.of_match_table	= of_match_ptr(mux_gpio_dt_ids),
+	},
+	.probe = mux_gpio_probe,
+	.remove = mux_gpio_remove,
+};
+module_platform_driver(mux_gpio_driver);
+
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
+MODULE_DESCRIPTION("GPIO-controlled multiplexer driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mux.h b/include/linux/mux.h
new file mode 100644
index 000000000000..5b21b8184056
--- /dev/null
+++ b/include/linux/mux.h
@@ -0,0 +1,53 @@
+/*
+ * mux.h - definitions for the multiplexer interface
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _LINUX_MUX_H
+#define _LINUX_MUX_H
+
+#include <linux/device.h>
+#include <linux/rwsem.h>
+
+struct mux_control;
+
+struct mux_control_ops {
+	int (*set)(struct mux_control *mux, int reg);
+};
+
+struct mux_control {
+	struct rw_semaphore lock; /* protects the state of the mux */
+
+	struct device dev;
+	int id;
+
+	int cached_state;
+	int idle_state;
+
+	const struct mux_control_ops *ops;
+
+	void *priv;
+};
+
+#define to_mux_control(x) container_of((x), struct mux_control, dev)
+
+struct mux_control *mux_control_alloc(size_t sizeof_priv);
+int mux_control_register(struct mux_control *mux);
+void mux_control_unregister(struct mux_control *mux);
+void mux_control_put(struct mux_control *mux);
+
+int mux_control_select(struct mux_control *mux, int state);
+int mux_control_deselect(struct mux_control *mux);
+
+struct mux_control *mux_control_get(struct device *dev,
+				    const char *mux_name);
+struct mux_control *devm_mux_control_get(struct device *dev,
+					 const char *mux_name);
+void devm_mux_control_put(struct device *dev, struct mux_control *mux);
+
+#endif /* _LINUX_MUX_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 3/7] iio: inkern: api for manipulating ext_info of iio channels
  2016-11-17 21:48 [RFC PATCH v2 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
  2016-11-17 21:48 ` [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio Peter Rosin
  2016-11-17 21:48 ` [RFC PATCH v2 2/7] misc: minimal mux subsystem and gpio-based mux controller Peter Rosin
@ 2016-11-17 21:48 ` Peter Rosin
  2016-11-19 15:38   ` Jonathan Cameron
  2016-11-17 21:48 ` [RFC PATCH v2 4/7] dt-bindings: iio: iio-mux: document iio-mux bindings Peter Rosin
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-11-17 21:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c, devicetree, linux-iio

Extend the inkern api with functions for reading and writing ext_info
of iio channels.
---
 drivers/iio/inkern.c         | 55 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/consumer.h |  6 +++++
 2 files changed, 61 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index cfca17ba2535..a8099b164222 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -850,3 +850,58 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(iio_write_channel_raw);
+
+int iio_get_channel_ext_info_count(struct iio_channel *chan)
+{
+	const struct iio_chan_spec_ext_info *ext_info;
+	unsigned int i = 0;
+
+	if (!chan->channel->ext_info)
+		return i;
+
+	for (ext_info = chan->channel->ext_info; ext_info->name; ext_info++)
+		++i;
+
+	return i;
+}
+EXPORT_SYMBOL_GPL(iio_get_channel_ext_info_count);
+
+ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
+				  const char *attr, char *buf)
+{
+	const struct iio_chan_spec_ext_info *ext_info;
+
+	if (!chan->channel->ext_info)
+		return -EINVAL;
+
+	for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
+		if (strcmp(attr, ext_info->name))
+			continue;
+
+		return ext_info->read(chan->indio_dev, ext_info->private,
+				      chan->channel, buf);
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(iio_read_channel_ext_info);
+
+ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
+				   const char *buf, size_t len)
+{
+	const struct iio_chan_spec_ext_info *ext_info;
+
+	if (!chan->channel->ext_info)
+		return -EINVAL;
+
+	for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
+		if (strcmp(attr, ext_info->name))
+			continue;
+
+		return ext_info->write(chan->indio_dev, ext_info->private,
+				       chan->channel, buf, len);
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(iio_write_channel_ext_info);
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 9a4f336d8b4a..471dece8729a 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -299,4 +299,10 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val,
 int iio_convert_raw_to_processed(struct iio_channel *chan, int raw,
 	int *processed, unsigned int scale);
 
+int iio_get_channel_ext_info_count(struct iio_channel *chan);
+ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
+				  const char *attr, char *buf);
+ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
+				   const char *buf, size_t len);
+
 #endif
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 4/7] dt-bindings: iio: iio-mux: document iio-mux bindings
  2016-11-17 21:48 [RFC PATCH v2 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (2 preceding siblings ...)
  2016-11-17 21:48 ` [RFC PATCH v2 3/7] iio: inkern: api for manipulating ext_info of iio channels Peter Rosin
@ 2016-11-17 21:48 ` Peter Rosin
  2016-11-17 21:48 ` [RFC PATCH v2 5/7] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-11-17 21:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c, devicetree, linux-iio

---
 .../bindings/iio/multiplexer/iio-mux.txt           | 49 ++++++++++++++++++++++
 1 file changed, 49 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt

diff --git a/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
new file mode 100644
index 000000000000..df6e4d5cc45e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
@@ -0,0 +1,49 @@
+IIO multiplexer bindings
+
+If a multiplexer is used to select when hardware signal is fed to
+e.g. an ADC channel, these bindings describe that situation.
+
+Required properties:
+- compatible : "iio-mux"
+- io-channels : Channel node of the parent channel that has multiplexed
+		input.
+- io-channel-names : Should be "parent".
+- control-muxes : Node of the multiplexer that controls the input signal.
+- control-mux-names : Should be "mux".
+- #address-cells = <1>;
+- #size-cells = <0>;
+
+Required properties for iio-mux child nodes:
+- reg : The multiplexer number.
+
+Example:
+	control_adc_mux: control-adc-mux {
+		compatible = "mux-gpio";
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "iio-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		control-muxes = <&control_adc_mux>;
+		control-mux-names = "mux";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		sync@0 {
+			reg = <0>;
+		};
+
+		in@1 {
+			reg = <1>;
+		};
+
+		system-regulator@2 {
+			reg = <2>;
+		};
+	};
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 5/7] iio: multiplexer: new iio category and iio-mux driver
  2016-11-17 21:48 [RFC PATCH v2 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (3 preceding siblings ...)
  2016-11-17 21:48 ` [RFC PATCH v2 4/7] dt-bindings: iio: iio-mux: document iio-mux bindings Peter Rosin
@ 2016-11-17 21:48 ` Peter Rosin
  2016-11-19 15:49   ` Jonathan Cameron
  2016-11-17 21:48 ` [RFC PATCH v2 6/7] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings Peter Rosin
  2016-11-17 21:48 ` [RFC PATCH v2 7/7] i2c: i2c-mux-simple: new driver Peter Rosin
  6 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-11-17 21:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c, devicetree, linux-iio

When a multiplexer changes how an iio device behaves (for example
by feeding different signals to an ADC), this driver can be used
create one virtual iio channel for each multiplexer state.

Depends on the generic multiplexer driver.
---
 drivers/iio/Kconfig               |   1 +
 drivers/iio/Makefile              |   1 +
 drivers/iio/multiplexer/Kconfig   |  17 ++
 drivers/iio/multiplexer/Makefile  |   6 +
 drivers/iio/multiplexer/iio-mux.c | 444 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 469 insertions(+)
 create mode 100644 drivers/iio/multiplexer/Kconfig
 create mode 100644 drivers/iio/multiplexer/Makefile
 create mode 100644 drivers/iio/multiplexer/iio-mux.c

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 6743b18194fb..dcb541d0d70e 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -82,6 +82,7 @@ source "drivers/iio/humidity/Kconfig"
 source "drivers/iio/imu/Kconfig"
 source "drivers/iio/light/Kconfig"
 source "drivers/iio/magnetometer/Kconfig"
+source "drivers/iio/multiplexer/Kconfig"
 source "drivers/iio/orientation/Kconfig"
 if IIO_TRIGGER
    source "drivers/iio/trigger/Kconfig"
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 87e4c4369e2f..f9879c29cf6f 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -27,6 +27,7 @@ obj-y += humidity/
 obj-y += imu/
 obj-y += light/
 obj-y += magnetometer/
+obj-y += multiplexer/
 obj-y += orientation/
 obj-y += potentiometer/
 obj-y += pressure/
diff --git a/drivers/iio/multiplexer/Kconfig b/drivers/iio/multiplexer/Kconfig
new file mode 100644
index 000000000000..31cbe4509366
--- /dev/null
+++ b/drivers/iio/multiplexer/Kconfig
@@ -0,0 +1,17 @@
+#
+# Multiplexer drivers
+#
+# When adding new entries keep the list in alphabetical order
+
+menu "Multiplexers"
+
+config IIO_MUX
+	tristate "IIO multiplexer driver"
+	depends on OF && MUX_GPIO
+	help
+	  Say yes here to build support for the IIO multiplexer.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called iio-mux.
+
+endmenu
diff --git a/drivers/iio/multiplexer/Makefile b/drivers/iio/multiplexer/Makefile
new file mode 100644
index 000000000000..68be3c4abd07
--- /dev/null
+++ b/drivers/iio/multiplexer/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for industrial I/O multiplexer drivers
+#
+
+# When adding new entries keep the list in alphabetical order
+obj-$(CONFIG_IIO_MUX) += iio-mux.o
diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
new file mode 100644
index 000000000000..2a8d00da990b
--- /dev/null
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -0,0 +1,444 @@
+/*
+ * IIO multiplexer driver
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/mux.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+struct mux_ext_info_cache {
+	char *data;
+	size_t size;
+};
+
+struct mux_child {
+	u32 state;
+
+	struct mux_ext_info_cache *ext_info_cache;
+};
+
+struct mux {
+	u32 cached_state;
+	struct mux_control *control;
+	struct iio_channel *parent;
+	struct iio_dev *indio_dev;
+	struct iio_chan_spec *c;
+	struct iio_chan_spec_ext_info *ext_info;
+	struct mux_child *child;
+};
+
+static int iio_mux_select(struct mux *mux, int idx)
+{
+	struct mux_child *child = &mux->child[idx];
+	int ret;
+	int i;
+
+	ret = mux_control_select(mux->control, child->state);
+	if (ret < 0) {
+		mux->cached_state = -1;
+		return ret;
+	}
+
+	if (mux->cached_state == child->state)
+		return 0;
+
+	if (mux->c[idx].ext_info) {
+		for (i = 0; mux->c[idx].ext_info[i].name; ++i) {
+			const char *attr = mux->c[idx].ext_info[i].name;
+			struct mux_ext_info_cache *cache;
+
+			cache = &child->ext_info_cache[i];
+
+			if (cache->size < 0)
+				continue;
+
+			ret = iio_write_channel_ext_info(mux->parent, attr,
+							 cache->data,
+							 cache->size);
+
+			if (ret < 0) {
+				mux_control_deselect(mux->control);
+				mux->cached_state = -1;
+				return ret;
+			}
+		}
+	}
+	mux->cached_state = child->state;
+
+	return 0;
+}
+
+static void iio_mux_deselect(struct mux *mux)
+{
+	mux_control_deselect(mux->control);
+}
+
+static int mux_read_raw(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan,
+			int *val, int *val2, long mask)
+{
+	struct mux *mux = iio_priv(indio_dev);
+	int idx = chan - mux->c;
+	int ret;
+
+	ret = iio_mux_select(mux, idx);
+	if (ret < 0)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_read_channel_raw(mux->parent, val);
+		break;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = iio_read_channel_scale(mux->parent, val, val2);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	iio_mux_deselect(mux);
+
+	return ret;
+}
+
+static int mux_read_avail(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan,
+			  const int **vals, int *type, int *length,
+			  long mask)
+{
+	struct mux *mux = iio_priv(indio_dev);
+	int idx = chan - mux->c;
+	int ret;
+
+	ret = iio_mux_select(mux, idx);
+	if (ret < 0)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*type = IIO_VAL_INT;
+		ret = iio_read_avail_channel_raw(mux->parent, vals, length);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	iio_mux_deselect(mux);
+
+	return ret;
+}
+
+static int mux_write_raw(struct iio_dev *indio_dev,
+			 struct iio_chan_spec const *chan,
+			 int val, int val2, long mask)
+{
+	struct mux *mux = iio_priv(indio_dev);
+	int idx = chan - mux->c;
+	int ret;
+
+	ret = iio_mux_select(mux, idx);
+	if (ret < 0)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_write_channel_raw(mux->parent, val);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	iio_mux_deselect(mux);
+
+	return ret;
+}
+
+static const struct iio_info mux_info = {
+	.read_raw = mux_read_raw,
+	.read_avail = mux_read_avail,
+	.write_raw = mux_write_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static ssize_t mux_read_ext_info(struct iio_dev *indio_dev, uintptr_t private,
+				 struct iio_chan_spec const *chan, char *buf)
+{
+	struct mux *mux = iio_priv(indio_dev);
+	int idx = chan - mux->c;
+	ssize_t ret;
+
+	ret = iio_mux_select(mux, idx);
+	if (ret < 0)
+		return ret;
+
+	ret = iio_read_channel_ext_info(mux->parent,
+					mux->ext_info[private].name,
+					buf);
+
+	iio_mux_deselect(mux);
+
+	return ret;
+}
+
+static ssize_t mux_write_ext_info(struct iio_dev *indio_dev, uintptr_t private,
+				  struct iio_chan_spec const *chan,
+				  const char *buf, size_t len)
+{
+	struct device *dev = indio_dev->dev.parent;
+	struct mux *mux = iio_priv(indio_dev);
+	int idx = chan - mux->c;
+	char *new;
+	ssize_t ret;
+
+	ret = iio_mux_select(mux, idx);
+	if (ret < 0)
+		return ret;
+
+	new = devm_kmemdup(dev, buf, len + 1, GFP_KERNEL);
+	if (!new) {
+		iio_mux_deselect(mux);
+		return -ENOMEM;
+	}
+
+	new[len] = 0;
+
+	ret = iio_write_channel_ext_info(mux->parent,
+					 mux->ext_info[private].name,
+					 buf, len);
+	if (ret < 0) {
+		iio_mux_deselect(mux);
+		devm_kfree(dev, new);
+		return ret;
+	}
+
+	devm_kfree(dev, mux->child[idx].ext_info_cache[private].data);
+	mux->child[idx].ext_info_cache[private].data = new;
+	mux->child[idx].ext_info_cache[private].size = len;
+
+	iio_mux_deselect(mux);
+
+	return ret;
+}
+
+static int mux_configure_channel(struct device *dev, struct mux *mux,
+				 struct device_node *child_np, int idx)
+{
+	struct mux_child *child = &mux->child[idx];
+	struct iio_chan_spec *c = &mux->c[idx];
+	const struct iio_chan_spec *pc = mux->parent->channel;
+	char *page;
+	int num_ext_info;
+	int i;
+	int ret;
+
+	c->indexed = 1;
+	c->channel = idx;
+	c->output = pc->output;
+	c->datasheet_name = child_np->name;
+	c->ext_info = mux->ext_info;
+
+	ret = iio_get_channel_type(mux->parent, &c->type);
+	if (ret < 0) {
+		dev_err(dev, "failed to get parent channel type\n");
+		return ret;
+	}
+
+	if (iio_channel_has_info(pc, IIO_CHAN_INFO_RAW))
+		c->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW);
+	if (iio_channel_has_info(pc, IIO_CHAN_INFO_SCALE))
+		c->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE);
+
+	if (iio_channel_has_available(pc, IIO_CHAN_INFO_RAW))
+		c->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
+
+	ret = of_property_read_u32(child_np, "reg", &child->state);
+	if (ret < 0) {
+		dev_err(dev, "no reg property for node '%s'\n", child_np->name);
+		return ret;
+	}
+
+	for (i = 0; i < idx; ++i) {
+		if (mux->child[i].state == child->state) {
+			dev_err(dev, "double use of reg %d\n", child->state);
+			return -EINVAL;
+		}
+	}
+
+	num_ext_info = iio_get_channel_ext_info_count(mux->parent);
+	if (num_ext_info) {
+		page = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL);
+		if (!page)
+			return -ENOMEM;
+	}
+	child->ext_info_cache = devm_kzalloc(dev,
+					     sizeof(*child->ext_info_cache) *
+					     num_ext_info, GFP_KERNEL);
+	for (i = 0; i < num_ext_info; ++i) {
+		child->ext_info_cache[i].size = -1;
+
+		if (!pc->ext_info[i].write)
+			continue;
+		if (!pc->ext_info[i].read)
+			continue;
+
+		ret = iio_read_channel_ext_info(mux->parent,
+						mux->ext_info[i].name,
+						page);
+		if (ret < 0) {
+			dev_err(dev, "failed to get ext_info '%s'\n",
+				pc->ext_info[i].name);
+			return ret;
+		}
+		if (ret >= PAGE_SIZE) {
+			dev_err(dev, "too large ext_info '%s'\n",
+				pc->ext_info[i].name);
+			return -EINVAL;
+		}
+
+		child->ext_info_cache[i].data = devm_kmemdup(dev, page, ret + 1,
+							     GFP_KERNEL);
+		child->ext_info_cache[i].data[ret] = 0;
+		child->ext_info_cache[i].size = ret;
+	}
+
+	if (num_ext_info)
+		devm_kfree(dev, page);
+
+	return 0;
+}
+
+static int mux_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child_np;
+	struct iio_dev *indio_dev;
+	struct iio_channel *parent;
+	struct mux *mux;
+	int sizeof_ext_info;
+	int children;
+	int sizeof_priv;
+	int i;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	parent = devm_iio_channel_get(dev, "parent");
+	if (IS_ERR(parent)) {
+		if (PTR_ERR(parent) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get parent channel\n");
+		return PTR_ERR(parent);
+	}
+
+	sizeof_ext_info = iio_get_channel_ext_info_count(parent);
+	if (sizeof_ext_info) {
+		sizeof_ext_info += 1; /* one extra entry for the sentinel */
+		sizeof_ext_info *= sizeof(*mux->ext_info);
+	}
+
+	children = of_get_child_count(np);
+	if (children <= 0) {
+		dev_err(dev, "not even a single child\n");
+		return -EINVAL;
+	}
+
+	sizeof_priv = sizeof(*mux);
+	sizeof_priv += sizeof(*mux->child) * children;
+	sizeof_priv += sizeof(*mux->c) * children;
+	sizeof_priv += sizeof_ext_info;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof_priv);
+	if (!indio_dev)
+		return -ENOMEM;
+
+	mux = iio_priv(indio_dev);
+	mux->child = (struct mux_child *)(mux + 1);
+	mux->c = (struct iio_chan_spec *)(mux->child + children);
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	mux->parent = parent;
+	mux->cached_state = -1;
+
+	indio_dev->name = dev_name(dev);
+	indio_dev->dev.parent = dev;
+	indio_dev->info = &mux_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = mux->c;
+	indio_dev->num_channels = children;
+	if (sizeof_ext_info) {
+		mux->ext_info = devm_kmemdup(dev,
+					     parent->channel->ext_info,
+					     sizeof_ext_info, GFP_KERNEL);
+		if (!mux->ext_info)
+			return -ENOMEM;
+
+		for (i = 0; mux->ext_info[i].name; ++i) {
+			if (parent->channel->ext_info[i].read)
+				mux->ext_info[i].read = mux_read_ext_info;
+			if (parent->channel->ext_info[i].write)
+				mux->ext_info[i].write = mux_write_ext_info;
+			mux->ext_info[i].private = i;
+		}
+	}
+
+	i = 0;
+	for_each_child_of_node(np, child_np) {
+		ret = mux_configure_channel(dev, mux, child_np, i);
+		if (ret < 0)
+			return ret;
+		i++;
+	}
+
+	mux->control = devm_mux_control_get(dev, "mux");
+	if (IS_ERR(mux->control)) {
+		if (PTR_ERR(mux->control) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get control-mux\n");
+		return PTR_ERR(mux->control);
+	}
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret) {
+		dev_err(dev, "failed to register iio device\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id mux_match[] = {
+	{ .compatible = "iio-mux" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mux_match);
+
+static struct platform_driver mux_driver = {
+	.probe = mux_probe,
+	.driver = {
+		.name = "iio-mux",
+		.of_match_table = mux_match,
+	},
+};
+module_platform_driver(mux_driver);
+
+MODULE_DESCRIPTION("IIO multiplexer driver");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 6/7] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings
  2016-11-17 21:48 [RFC PATCH v2 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (4 preceding siblings ...)
  2016-11-17 21:48 ` [RFC PATCH v2 5/7] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
@ 2016-11-17 21:48 ` Peter Rosin
  2016-11-17 21:48 ` [RFC PATCH v2 7/7] i2c: i2c-mux-simple: new driver Peter Rosin
  6 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-11-17 21:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c, devicetree, linux-iio

---
 .../devicetree/bindings/i2c/i2c-mux-simple.txt     | 91 ++++++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
new file mode 100644
index 000000000000..fec1ab39ad64
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
@@ -0,0 +1,91 @@
+Simple I2C Bus Mux
+
+This binding describes an I2C bus multiplexer that uses GPIOs to
+route the I2C signals.
+
+                                  .-----.  .-----.
+                                  | dev |  | dev |
+    .------------.                '-----'  '-----'
+    | SoC        |                   |        |
+    |            |          .--------+--------'
+    |   .------. |  .------+    child bus A, on MUX value set to 0
+    |   | I2C  |-|--| Mux  |
+    |   '------' |  '--+---+    child bus B, on MUX value set to 1
+    |   .------. |     |    '----------+--------+--------.
+    |   | MUX- | |     |               |        |        |
+    |   | Ctrl |-|-----+            .-----.  .-----.  .-----.
+    |   '------' |                  | dev |  | dev |  | dev |
+    '------------'                  '-----'  '-----'  '-----'
+
+Required properties:
+- compatible: i2c-mux-simple,mux-locked or i2c-mux-simple,parent-locked
+- i2c-parent: The phandle of the I2C bus that this multiplexer's master-side
+  port is connected to.
+- control-muxes : Node of the multiplexer that controls "Mux".
+- control-mux-names : Should be "mux".
+* Standard I2C mux properties. See i2c-mux.txt in this directory.
+* I2C child bus nodes. See i2c-mux.txt in this directory.
+
+Optional properties:
+- idle-state: value to set the muxer to when idle. When no value is
+  given, it defaults to the last value used.
+
+For each i2c child node, an I2C child bus will be created. They will
+be numbered based on their order in the device tree.
+
+Whenever an access is made to a device on a child bus, the value set
+in the relevant node's reg property will be fed to the mux controller.
+
+If an idle state is defined, using the idle-state (optional) property,
+whenever an access is not being made to a device on a child bus, the
+mux controller will be set according to the idle value.
+
+If an idle state is not defined, the most recently used value will be
+left programmed into hardware whenever no access is being made to a
+device on a child bus.
+
+Example:
+	control_i2c_mux: control-i2c-mux {
+		compatible = "mux-gpio";
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	i2c-mux {
+		compatible = "i2c-mux-simple,mux-locked";
+		i2c-parent = <&i2c1>;
+
+		control-muxes = <&control_i2c_mux>;
+		control-mux-names = "mux";
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ssd1307: oled@3c {
+				compatible = "solomon,ssd1307fb-i2c";
+				reg = <0x3c>;
+				pwms = <&pwm 4 3000>;
+				reset-gpios = <&gpio2 7 1>;
+				reset-active-low;
+			};
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555@20 {
+				compatible = "nxp,pca9555";
+				gpio-controller;
+				#gpio-cells = <2>;
+				reg = <0x20>;
+			};
+		};
+	};
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 7/7] i2c: i2c-mux-simple: new driver
  2016-11-17 21:48 [RFC PATCH v2 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (5 preceding siblings ...)
  2016-11-17 21:48 ` [RFC PATCH v2 6/7] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings Peter Rosin
@ 2016-11-17 21:48 ` Peter Rosin
  6 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-11-17 21:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c, devicetree, linux-iio

This is a generic simple i2c mux that uses the generic multiplexer
subsystem to do the muxing.

The user can select if the mux is to be mux-locked and parent-locked
as described in Documentation/i2c/i2c-topology.
---
 drivers/i2c/muxes/Kconfig          |  12 +++
 drivers/i2c/muxes/Makefile         |   1 +
 drivers/i2c/muxes/i2c-mux-simple.c | 168 +++++++++++++++++++++++++++++++++++++
 3 files changed, 181 insertions(+)
 create mode 100644 drivers/i2c/muxes/i2c-mux-simple.c

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index e280c8ecc0b5..45e80ad2d4ab 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -72,6 +72,18 @@ config I2C_MUX_REG
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mux-reg.
 
+config I2C_MUX_SIMPLE
+	tristate "Simple I2C multiplexer"
+	depends on OF
+	help
+	  If you say yes to this option, support will be included for a
+	  simple generic I2C multiplexer. This driver provides access to
+	  I2C busses connected through a MUX, which is controlled
+	  by a generic MUX controller.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-mux-simple.
+
 config I2C_DEMUX_PINCTRL
 	tristate "pinctrl-based I2C demultiplexer"
 	depends on PINCTRL && OF
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index 7c267c29b191..eea1fb9e6466 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -10,5 +10,6 @@ obj-$(CONFIG_I2C_MUX_PCA9541)	+= i2c-mux-pca9541.o
 obj-$(CONFIG_I2C_MUX_PCA954x)	+= i2c-mux-pca954x.o
 obj-$(CONFIG_I2C_MUX_PINCTRL)	+= i2c-mux-pinctrl.o
 obj-$(CONFIG_I2C_MUX_REG)	+= i2c-mux-reg.o
+obj-$(CONFIG_I2C_MUX_SIMPLE)	+= i2c-mux-simple.o
 
 ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/muxes/i2c-mux-simple.c b/drivers/i2c/muxes/i2c-mux-simple.c
new file mode 100644
index 000000000000..6d8ddbc94ecc
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-simple.c
@@ -0,0 +1,168 @@
+/*
+ * Generic simple I2C multiplexer
+ *
+ * Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+#include <linux/module.h>
+#include <linux/mux.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+struct mux {
+	struct mux_control *control;
+
+	bool do_not_deselect;
+};
+
+static int i2c_mux_select(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct mux *mux = i2c_mux_priv(muxc);
+	int ret;
+
+	ret = mux_control_select(mux->control, chan);
+	mux->do_not_deselect = ret < 0;
+
+	return ret;
+}
+
+static int i2c_mux_deselect(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct mux *mux = i2c_mux_priv(muxc);
+
+	if (mux->do_not_deselect)
+		return 0;
+
+	return mux_control_deselect(mux->control);
+}
+
+static struct i2c_adapter *mux_parent_adapter(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *parent_np;
+	struct i2c_adapter *parent;
+
+	parent_np = of_parse_phandle(np, "i2c-parent", 0);
+	if (!parent_np) {
+		dev_err(dev, "Cannot parse i2c-parent\n");
+		return ERR_PTR(-ENODEV);
+	}
+	parent = of_find_i2c_adapter_by_node(parent_np);
+	of_node_put(parent_np);
+	if (!parent)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return parent;
+}
+
+static const struct of_device_id i2c_mux_of_match[] = {
+	{ .compatible = "i2c-mux-simple,parent-locked",
+	  .data = (void *)0, },
+	{ .compatible = "i2c-mux-simple,mux-locked",
+	  .data = (void *)1, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, i2c_mux_of_match);
+
+static int i2c_mux_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct device_node *child;
+	const struct of_device_id *match;
+	struct i2c_mux_core *muxc;
+	struct mux *mux;
+	struct i2c_adapter *parent;
+	int children;
+	int ret;
+
+	if (!np)
+		return -ENODEV;
+
+	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		return -ENOMEM;
+
+	mux->control = devm_mux_control_get(dev, "mux");
+	if (IS_ERR(mux->control)) {
+		if (PTR_ERR(mux->control) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get control-mux\n");
+		return PTR_ERR(mux->control);
+	}
+
+	parent = mux_parent_adapter(dev);
+	if (IS_ERR(parent)) {
+		if (PTR_ERR(parent) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get i2c-parent adapter\n");
+		return PTR_ERR(parent);
+	}
+
+	children = of_get_child_count(np);
+
+	muxc = i2c_mux_alloc(parent, dev, children, 0, 0,
+			     i2c_mux_select, i2c_mux_deselect);
+	if (!muxc) {
+		ret = -ENOMEM;
+		goto err_parent;
+	}
+	muxc->priv = mux;
+
+	platform_set_drvdata(pdev, muxc);
+
+	match = of_match_device(of_match_ptr(i2c_mux_of_match), dev);
+	if (match)
+		muxc->mux_locked = !!of_device_get_match_data(dev);
+
+	for_each_child_of_node(np, child) {
+		u32 chan;
+
+		ret = of_property_read_u32(child, "reg", &chan);
+		if (ret < 0)
+			goto err_children;
+
+		ret = i2c_mux_add_adapter(muxc, 0, chan, 0);
+		if (ret)
+			goto err_children;
+	}
+
+	dev_info(dev, "%d-port mux on %s adapter\n", children, parent->name);
+
+	return 0;
+
+err_children:
+	i2c_mux_del_adapters(muxc);
+err_parent:
+	i2c_put_adapter(parent);
+
+	return ret;
+}
+
+static int i2c_mux_remove(struct platform_device *pdev)
+{
+	struct i2c_mux_core *muxc = platform_get_drvdata(pdev);
+
+	i2c_mux_del_adapters(muxc);
+	i2c_put_adapter(muxc->parent);
+
+	return 0;
+}
+
+static struct platform_driver i2c_mux_driver = {
+	.probe	= i2c_mux_probe,
+	.remove	= i2c_mux_remove,
+	.driver	= {
+		.name	= "i2c-mux-simple",
+		.of_match_table = i2c_mux_of_match,
+	},
+};
+module_platform_driver(i2c_mux_driver);
+
+MODULE_DESCRIPTION("Simple I2C multiplexer driver");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio
  2016-11-17 21:48 ` [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio Peter Rosin
@ 2016-11-18 15:35   ` Rob Herring
  2016-11-18 16:59     ` Peter Rosin
  0 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2016-11-18 15:35 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Wolfram Sang, Mark Rutland, Jonathan Cameron,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Arnd Bergmann, Greg Kroah-Hartman, linux-i2c, devicetree,
	linux-iio

On Thu, Nov 17, 2016 at 10:48:03PM +0100, Peter Rosin wrote:
> ---
>  .../devicetree/bindings/misc/mux-gpio.txt          | 79 ++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/mux-gpio.txt b/Documentation/devicetree/bindings/misc/mux-gpio.txt
> new file mode 100644
> index 000000000000..73699a37824f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/mux-gpio.txt
> @@ -0,0 +1,79 @@
> +GPIO-based multiplexer controller bindings
> +
> +Define what GPIO pins are used to control a multiplexer. Or several
> +multiplexers, if the same pins control more than one multiplexer.

I think this makes sense in your case, but I think it is too complicated 
for a non-shared case. Perhaps mux-gpios should be used directly (i.e. 
in the adc-mux node) and control-muxes only used for the shared case.

Part of me feels like you are working around in DT the GPIO subsystem 
limitation that it can't share GPIO lines. Either this could be fixed 
in some way in the GPIO subsystem, or the mux subsys could deal with it. 
You just have to look up if you already have a mux registered for the 
same GPIOs. Of course, that may make the mux subsys pretty much GPIO 
only, but I'm having a hard time thinking how you would have shared 
muxes that are not GPIO controlled. Any other control would be 
integrated into the mux itself.

> +
> +Required properties:
> +- compatible : "mux-gpio"
> +- mux-gpios : list of gpios used to control the multiplexer, least
> +	      significant bit first.
> +
> +Optional properties:
> +- idle-state : if present, the state the mux will have when idle.

Needs some detail on what the value is. One bit per gpio? One cell per 
gpio?

> +
> +Example:
> +	control_mux: control-adc-mux {
> +		compatible = "mux-gpio";
> +
> +		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> +			    <&pioA 1 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	adc-mux {
> +		compatible = "iio-mux";
> +		io-channels = <&adc 0>;
> +		io-channel-names = "parent";
> +
> +		control-muxes = <&control_mux>;

This is a common name? It should be in a common mux binding doc.

> +		control-mux-names = "mux";

I think this can be dropped at least until you have more than 1.

> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		sync-1@0 {
> +			reg = <0>;
> +		};
> +
> +		in@1 {
> +			reg = <1>;
> +		};
> +
> +		out@2 {
> +			reg = <2>;
> +		};
> +
> +		sync-2@3 {
> +			reg = <3>;
> +		};
> +	};
> +
> +	i2c-mux {
> +		compatible = "i2c-mux-simple,mux-locked";
> +		i2c-parent = <&i2c1>;
> +
> +		control-muxes = <&control_mux>;
> +		control-mux-names = "mux";
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		i2c@0 {
> +			reg = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			ssd1307: oled@3c {
> +				/* ... */
> +			};
> +		};
> +
> +		i2c@3 {
> +			reg = <3>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			pca9555: pca9555@20 {
> +				/* ... */
> +			};
> +		};
> +	};
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio
  2016-11-18 15:35   ` Rob Herring
@ 2016-11-18 16:59     ` Peter Rosin
  2016-11-19 15:21       ` Jonathan Cameron
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-11-18 16:59 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, Wolfram Sang, Mark Rutland, Jonathan Cameron,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Arnd Bergmann, Greg Kroah-Hartman, linux-i2c, devicetree,
	linux-iio

On 2016-11-18 16:35, Rob Herring wrote:
> On Thu, Nov 17, 2016 at 10:48:03PM +0100, Peter Rosin wrote:
>> ---
>>  .../devicetree/bindings/misc/mux-gpio.txt          | 79 ++++++++++++++++++++++
>>  1 file changed, 79 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt
>>
>> diff --git a/Documentation/devicetree/bindings/misc/mux-gpio.txt b/Documentation/devicetree/bindings/misc/mux-gpio.txt
>> new file mode 100644
>> index 000000000000..73699a37824f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/mux-gpio.txt
>> @@ -0,0 +1,79 @@
>> +GPIO-based multiplexer controller bindings
>> +
>> +Define what GPIO pins are used to control a multiplexer. Or several
>> +multiplexers, if the same pins control more than one multiplexer.
> 
> I think this makes sense in your case, but I think it is too complicated 
> for a non-shared case. Perhaps mux-gpios should be used directly (i.e. 
> in the adc-mux node) and control-muxes only used for the shared case.
> 
> Part of me feels like you are working around in DT the GPIO subsystem 
> limitation that it can't share GPIO lines. Either this could be fixed 
> in some way in the GPIO subsystem, or the mux subsys could deal with it. 
> You just have to look up if you already have a mux registered for the 
> same GPIOs. Of course, that may make the mux subsys pretty much GPIO 
> only, but I'm having a hard time thinking how you would have shared 
> muxes that are not GPIO controlled. Any other control would be 
> integrated into the mux itself.

But if someone wants to mux an adc line with a mux that is some kind of
integrated i2c device, you'd have to add extra code to the iio muxer
driver to handle that case. Or fork it. Or build something like the
i2c muxer infrastructure and separate out the mux control in small
drivers and handle the generic iio muxing centrally. But then someone
else uses that i2c device to instead mux an i2c bus, and you'd end up
with code duplication when that same muxer control code is added under
drivers/i2c/muxes.

With the proposed solution, this is unified.

I'd just hate to see drivers for muxers added under drivers/i2c/muxes
that do little more that control a mux that happens to be used to mux
an i2c bus, but are generic muxers that could equally well mux something
else. Even if the control is integrated into the mux, what the mux is
actually used for should perhaps not determine where its driver should
live.

Anyway, I don't know what to make with your suggestion, I just don't
see the path forward (not enough experience with the kernel and/or gpio
code). And it would be a limited solution (GPIO only,a s you say) so it
doesn't feel right.

Is there perhaps some way to keep the complicated shared case work as
is (or equivalently, the exact details are not important), and also
provide a simpler in-node thingy to glue a mux control to a consumer
w/o pointing to it with a phandle, but still have the same mux driver
handle both cases? No, I'm not a devicetree guru, so I don't see a
solution for that either, but maybe someone else does?

Perhaps the consumer could look for the mux control in first the
phandle, as in my proposal. If not found, it could also look for
a mux provider bound to child node.

	adc-mux {
		compatible = "iio-mux";
		io-channels = <&adc 0>;
		io-channel-names = "parent";

		mux-control {
			compatible = "mux-gpio";
			mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
				    <&pioA 1 GPIO_ACTIVE_HIGH>;
		};

		#address-cells = <1>;
		#size-cells = <0>;

		sync-1@0 {
			reg = <0>;
		};
		/* ... */
	};

Or perhaps look in a parent node:

	mux-control {
		compatible = "mux-gpio";
		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
			    <&pioA 1 GPIO_ACTIVE_HIGH>;

		adc-mux {
			compatible = "iio-mux";
			io-channels = <&adc 0>;
			io-channel-names = "parent";

			#address-cells = <1>;
			#size-cells = <0>;

			sync-1@0 {
				reg = <0>;
			};
			/* ... */
		};
	};

With the last suggestion, you could have multiple children of the
mux-control node for the complicated case where it controls more
than one mux. Not too bad? Hmm, what does the driver for the
mux-control node have to do to have drivers tied to its children?

Maybe this last layout should be the only thing supported? Good
enough for me anyway...

Cheers,
Peter

PS. I will take care of the other comments for the next round.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio
  2016-11-18 16:59     ` Peter Rosin
@ 2016-11-19 15:21       ` Jonathan Cameron
  0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2016-11-19 15:21 UTC (permalink / raw)
  To: Peter Rosin, Rob Herring
  Cc: linux-kernel, Wolfram Sang, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio

On 18/11/16 16:59, Peter Rosin wrote:
> On 2016-11-18 16:35, Rob Herring wrote:
>> On Thu, Nov 17, 2016 at 10:48:03PM +0100, Peter Rosin wrote:
>>> ---
>>>  .../devicetree/bindings/misc/mux-gpio.txt          | 79 ++++++++++++++++++++++
>>>  1 file changed, 79 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/misc/mux-gpio.txt b/Documentation/devicetree/bindings/misc/mux-gpio.txt
>>> new file mode 100644
>>> index 000000000000..73699a37824f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/mux-gpio.txt
>>> @@ -0,0 +1,79 @@
>>> +GPIO-based multiplexer controller bindings
>>> +
>>> +Define what GPIO pins are used to control a multiplexer. Or several
>>> +multiplexers, if the same pins control more than one multiplexer.
>>
>> I think this makes sense in your case, but I think it is too complicated 
>> for a non-shared case. Perhaps mux-gpios should be used directly (i.e. 
>> in the adc-mux node) and control-muxes only used for the shared case.
>>
>> Part of me feels like you are working around in DT the GPIO subsystem 
>> limitation that it can't share GPIO lines. Either this could be fixed 
>> in some way in the GPIO subsystem, or the mux subsys could deal with it. 
>> You just have to look up if you already have a mux registered for the 
>> same GPIOs. Of course, that may make the mux subsys pretty much GPIO 
>> only, but I'm having a hard time thinking how you would have shared 
>> muxes that are not GPIO controlled. Any other control would be 
>> integrated into the mux itself.
> 
> But if someone wants to mux an adc line with a mux that is some kind of
> integrated i2c device, you'd have to add extra code to the iio muxer
> driver to handle that case. Or fork it. Or build something like the
> i2c muxer infrastructure and separate out the mux control in small
> drivers and handle the generic iio muxing centrally. But then someone
> else uses that i2c device to instead mux an i2c bus, and you'd end up
> with code duplication when that same muxer control code is added under
> drivers/i2c/muxes.
> 
> With the proposed solution, this is unified.
> 
> I'd just hate to see drivers for muxers added under drivers/i2c/muxes
> that do little more that control a mux that happens to be used to mux
> an i2c bus, but are generic muxers that could equally well mux something
> else. Even if the control is integrated into the mux, what the mux is
> actually used for should perhaps not determine where its driver should
> live.
> 
> Anyway, I don't know what to make with your suggestion, I just don't
> see the path forward (not enough experience with the kernel and/or gpio
> code). And it would be a limited solution (GPIO only,a s you say) so it
> doesn't feel right.
Also worth pointing out here the possibility of multi pole muxes...
Relays are ultimately muxes as well (be it slow ones ;)

A quick google fed me:
TI SN74LS153 for example.  This one is digital only though...

Analog option (in both senses) is:
http://www.analog.com/media/en/technical-documentation/data-sheets/ADG888.pdf

So these 'look' the same as two single muxes wired to the same GPIOs.

> 
> Is there perhaps some way to keep the complicated shared case work as
> is (or equivalently, the exact details are not important), and also
> provide a simpler in-node thingy to glue a mux control to a consumer
> w/o pointing to it with a phandle, but still have the same mux driver
> handle both cases? No, I'm not a devicetree guru, so I don't see a
> solution for that either, but maybe someone else does?
> 
> Perhaps the consumer could look for the mux control in first the
> phandle, as in my proposal. If not found, it could also look for
> a mux provider bound to child node.
> 
> 	adc-mux {
> 		compatible = "iio-mux";
> 		io-channels = <&adc 0>;
> 		io-channel-names = "parent";
> 
> 		mux-control {
> 			compatible = "mux-gpio";
> 			mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> 				    <&pioA 1 GPIO_ACTIVE_HIGH>;
> 		};
> 
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		sync-1@0 {
> 			reg = <0>;
> 		};
> 		/* ... */
> 	};
> 
> Or perhaps look in a parent node:
> 
> 	mux-control {
> 		compatible = "mux-gpio";
> 		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> 			    <&pioA 1 GPIO_ACTIVE_HIGH>;
> 
> 		adc-mux {
> 			compatible = "iio-mux";
> 			io-channels = <&adc 0>;
> 			io-channel-names = "parent";
> 
> 			#address-cells = <1>;
> 			#size-cells = <0>;
> 
> 			sync-1@0 {
> 				reg = <0>;
> 			};
> 			/* ... */
> 		};
> 	};
> 
> With the last suggestion, you could have multiple children of the
> mux-control node for the complicated case where it controls more
> than one mux. Not too bad? Hmm, what does the driver for the
> mux-control node have to do to have drivers tied to its children?
> 
> Maybe this last layout should be the only thing supported? Good
> enough for me anyway...
> 
> Cheers,
> Peter
> 
> PS. I will take care of the other comments for the next round.
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 2/7] misc: minimal mux subsystem and gpio-based mux controller
  2016-11-17 21:48 ` [RFC PATCH v2 2/7] misc: minimal mux subsystem and gpio-based mux controller Peter Rosin
@ 2016-11-19 15:34   ` Jonathan Cameron
  2016-11-21 13:03     ` Peter Rosin
  0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2016-11-19 15:34 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio

On 17/11/16 21:48, Peter Rosin wrote:
> When both the iio subsystem and the i2c subsystem wants to update
> the same mux, there needs to be some coordination. Invent a new
> minimal "mux" subsystem that handles this.
I'd probably put something more general in the description. Lots of things
may need the same infrastructure.  This is just an example.

Few bits inline.

Also, I suspect you will fairly rapidly have a need for a strobe signal
as well.  A lot of mux chips that are more than 2 way seem to have them to
allow multiple chips to be synchronized.
> 
> Add a single backend driver for this new subsystem that can
> control gpio based multiplexers.
> ---
>  drivers/misc/Kconfig    |   6 +
>  drivers/misc/Makefile   |   2 +
>  drivers/misc/mux-core.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/misc/mux-gpio.c | 115 +++++++++++++++++++
>  include/linux/mux.h     |  53 +++++++++
>  5 files changed, 475 insertions(+)
>  create mode 100644 drivers/misc/mux-core.c
>  create mode 100644 drivers/misc/mux-gpio.c
>  create mode 100644 include/linux/mux.h
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 64971baf11fa..9e119bb78d82 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -766,6 +766,12 @@ config PANEL_BOOT_MESSAGE
>  	  An empty message will only clear the display at driver init time. Any other
>  	  printf()-formatted message is valid with newline and escape codes.
>  
> +config MUX_GPIO
> +	tristate "GPIO-controlled MUX controller"
> +	depends on OF
> +	help
> +	  GPIO-controlled MUX controller
> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 31983366090a..92b547bcbac1 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -53,6 +53,8 @@ obj-$(CONFIG_ECHO)		+= echo/
>  obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
>  obj-$(CONFIG_CXL_BASE)		+= cxl/
>  obj-$(CONFIG_PANEL)             += panel.o
> +obj-$(CONFIG_MUX_GPIO)          += mux-core.o
> +obj-$(CONFIG_MUX_GPIO)          += mux-gpio.o
>  
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
> diff --git a/drivers/misc/mux-core.c b/drivers/misc/mux-core.c
> new file mode 100644
> index 000000000000..7a8bf003a92c
> --- /dev/null
> +++ b/drivers/misc/mux-core.c
> @@ -0,0 +1,299 @@
> +/*
> + * Multiplexer subsystem
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "mux-core: " fmt
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/idr.h>
> +#include <linux/module.h>
> +#include <linux/mux.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +
> +static struct bus_type mux_bus_type = {
> +	.name = "mux",
> +};
> +
> +static int __init mux_init(void)
> +{
> +	return bus_register(&mux_bus_type);
> +}
> +
> +static void __exit mux_exit(void)
> +{
> +	bus_unregister(&mux_bus_type);
> +}
> +
> +static DEFINE_IDA(mux_ida);
> +
> +static void mux_control_release(struct device *dev)
> +{
> +	struct mux_control *mux = to_mux_control(dev);
> +
> +	ida_simple_remove(&mux_ida, mux->id);
> +	kfree(mux);
> +}
> +
> +static struct device_type mux_control_type = {
> +	.name = "mux-control",
> +	.release = mux_control_release,
> +};
> +
> +/*
> + * Allocate a mux-control, plus an extra memory area for private use
> + * by the caller.
> + */
> +struct mux_control *mux_control_alloc(size_t sizeof_priv)
> +{
> +	struct mux_control *mux;
> +
Worth planning ahead for spi controlled muxes and others that need their
structures to be carefully aligned to avoid dma cacheline fun?
Easy enough to add later I guess.
> +	mux = kzalloc(sizeof(*mux) + sizeof_priv, GFP_KERNEL);
> +	if (!mux)
> +		return NULL;
> +
> +	mux->dev.bus = &mux_bus_type;
> +	mux->dev.type = &mux_control_type;
> +	device_initialize(&mux->dev);
> +	dev_set_drvdata(&mux->dev, mux);
> +
> +	init_rwsem(&mux->lock);
> +	mux->priv = mux + 1;
Needed?  Or just do it with a bit of pointer math where the access is needed?
> +
> +	mux->id = ida_simple_get(&mux_ida, 0, 0, GFP_KERNEL);
> +	if (mux->id < 0) {
> +		pr_err("mux-controlX failed to get device id\n");
> +		kfree(mux);
> +		return NULL;
> +	}
> +	dev_set_name(&mux->dev, "mux:control%d", mux->id);
> +
> +	mux->cached_state = -1;
> +	mux->idle_state = -1;
> +
> +	return mux;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_alloc);
> +
> +/*
> + * Register the mux-control, thus readying it for use.
Either single line comment style - or perhaps kernel doc the lot...
> + */
> +int mux_control_register(struct mux_control *mux)
> +{
> +	/* If the calling driver did not initialize of_node, do it here */
> +	if (!mux->dev.of_node && mux->dev.parent)
> +		mux->dev.of_node = mux->dev.parent->of_node;
> +
> +	return device_add(&mux->dev);
> +}
> +EXPORT_SYMBOL_GPL(mux_control_register);
> +
> +/*
> + * Take the mux-control off-line.
> + */
> +void mux_control_unregister(struct mux_control *mux)
> +{
> +	device_del(&mux->dev);
> +}
> +EXPORT_SYMBOL_GPL(mux_control_unregister);
> +
> +/*
> + * Put away the mux-control for good.
> + */
> +void mux_control_put(struct mux_control *mux)
> +{
> +	if (!mux)
> +		return;
> +	put_device(&mux->dev);
> +}
> +EXPORT_SYMBOL_GPL(mux_control_put);
> +
> +static int mux_control_set(struct mux_control *mux, int state)
> +{
> +	int ret = mux->ops->set(mux, state);
> +
> +	mux->cached_state = ret < 0 ? -1 : state;
> +
> +	return ret;
> +}
> +
> +/*
> + * Select the given multiplexer channel. Call mux_control_deselect()
> + * when the operation is complete on the multiplexer channel, and the
> + * multiplexer is free for others to use.
> + */
> +int mux_control_select(struct mux_control *mux, int state)
> +{
> +	int ret;
> +
> +	if (down_read_trylock(&mux->lock)) {
> +		if (mux->cached_state == state)
> +			return 0;
> +
> +		/* Sigh, the mux needs updating... */
> +		up_read(&mux->lock);
> +	}
> +
> +	/* ...or it's just contended. */
> +	down_write(&mux->lock);
> +
> +	if (mux->cached_state == state) {
> +		/*
> +		 * Hmmm, someone else changed the mux to my liking.
> +		 * That makes me wonder how long I waited for nothing...
> +		 */
> +		downgrade_write(&mux->lock);
> +		return 0;
> +	}
> +
> +	ret = mux_control_set(mux, state);
> +	if (ret < 0) {
> +		if (mux->idle_state != -1)
> +			mux_control_set(mux, mux->idle_state);
> +
> +		up_write(&mux->lock);
> +		return ret;
> +	}
> +
> +	downgrade_write(&mux->lock);
> +
> +	return 1;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_select);
> +
> +/*
> + * Deselect the previously selected multiplexer channel.
> + */
> +int mux_control_deselect(struct mux_control *mux)
> +{
> +	int ret = 0;
> +
> +	if (mux->idle_state != -1 && mux->cached_state != mux->idle_state)
> +		ret = mux_control_set(mux, mux->idle_state);
> +
> +	up_read(&mux->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_deselect);
> +
> +static int of_dev_node_match(struct device *dev, void *data)
> +{
> +	return dev->of_node == data;
> +}
> +
> +static struct mux_control *of_find_mux_by_node(struct device_node *np)
> +{
> +	struct device *dev;
> +
> +	dev = bus_find_device(&mux_bus_type, NULL, np, of_dev_node_match);
> +
> +	return dev ? to_mux_control(dev) : NULL;
> +}
> +
> +static struct mux_control *of_mux_control_get(struct device_node *np, int index)
> +{
> +	struct device_node *mux_np;
> +	struct mux_control *mux;
> +
> +	mux_np = of_parse_phandle(np, "control-muxes", index);
> +	if (!mux_np)
> +		return NULL;
> +
> +	mux = of_find_mux_by_node(mux_np);
> +	of_node_put(mux_np);
> +
> +	return mux;
> +}
> +
> +/*
> + * Get a named mux.
> + */
> +struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct mux_control *mux;
> +	int index;
> +
> +	index = of_property_match_string(np, "control-mux-names", mux_name);
> +	if (index < 0) {
> +		dev_err(dev, "failed to get control-mux %s:%s(%i)\n",
> +			np->full_name, mux_name ?: "", index);
> +		return ERR_PTR(index);
> +	}
> +
> +	mux = of_mux_control_get(np, index);
> +	if (!mux)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	return mux;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_get);
> +
> +static void devm_mux_control_free(struct device *dev, void *res)
> +{
> +	struct mux_control *mux = *(struct mux_control **)res;
> +
> +	mux_control_put(mux);
> +}
> +
> +/*
> + * Get a named mux, with resource management.
> + */
Guess it might be elsewhere in patch set but remember to add this to
the global list of devm interfaces (in Documentation somewhere.. IIRC)
> +struct mux_control *devm_mux_control_get(struct device *dev,
> +					 const char *mux_name)
> +{
> +	struct mux_control **ptr, *mux;
> +
> +	ptr = devres_alloc(devm_mux_control_free, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	mux = mux_control_get(dev, mux_name);
> +	if (IS_ERR(mux)) {
> +		devres_free(ptr);
> +		return mux;
> +	}
> +
> +	*ptr = mux;
> +	devres_add(dev, ptr);
> +
> +	return mux;
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_control_get);
> +
> +static int devm_mux_control_match(struct device *dev, void *res, void *data)
> +{
> +	struct mux_control **r = res;
> +
> +	if (!r || !*r) {
> +		WARN_ON(!r || !*r);
> +		return 0;
> +	}
> +
> +	return *r == data;
> +}
> +
> +/*
> + * Resource-managed version mux_control_put.
> + */
> +void devm_mux_control_put(struct device *dev, struct mux_control *mux)
> +{
> +	WARN_ON(devres_release(dev, devm_mux_control_free,
> +			       devm_mux_control_match, mux));
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_control_put);
> +
> +subsys_initcall(mux_init);
> +module_exit(mux_exit);
> +
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
> +MODULE_DESCRIPTION("MUX subsystem");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/misc/mux-gpio.c b/drivers/misc/mux-gpio.c
> new file mode 100644
> index 000000000000..2ddd7fb24078
> --- /dev/null
> +++ b/drivers/misc/mux-gpio.c
> @@ -0,0 +1,115 @@
> +/*
> + * GPIO-controlled multiplexer driver
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/mux.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +struct mux_gpio {
> +	struct gpio_descs *gpios;
> +};
> +
> +static int mux_gpio_set(struct mux_control *mux, int val)
> +{
> +	struct mux_gpio *mux_gpio = mux->priv;
> +	int i;
> +
> +	for (i = 0; i < mux_gpio->gpios->ndescs; i++)
> +		gpiod_set_value_cansleep(mux_gpio->gpios->desc[i],
> +					 val & (1 << i));
> +
> +	return 0;
> +}
> +
> +static const struct mux_control_ops mux_gpio_ops = {
> +	.set = mux_gpio_set,
> +};
> +
> +static const struct of_device_id mux_gpio_dt_ids[] = {
> +	{ .compatible = "mux-gpio", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
> +
> +static int mux_gpio_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct mux_control *mux;
> +	struct mux_gpio *mux_gpio;
> +	u32 idle_state;
> +	int ret;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	mux = mux_control_alloc(sizeof(*mux_gpio));
> +	if (!mux)
> +		return -ENOMEM;
> +	mux_gpio = mux->priv;
> +	mux->dev.parent = dev;
> +	mux->ops = &mux_gpio_ops;
> +
> +	platform_set_drvdata(pdev, mux);
> +
> +	mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
> +	if (IS_ERR(mux_gpio->gpios)) {
> +		if (PTR_ERR(mux_gpio->gpios) != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get gpios\n");
> +		mux_control_put(mux);
> +		return PTR_ERR(mux_gpio->gpios);
> +	}
> +
> +	ret = of_property_read_u32(np, "idle-state", &idle_state);
> +	if (ret >= 0) {
> +		if (idle_state >= (1 << mux_gpio->gpios->ndescs)) {
> +			dev_err(dev, "invalid idle-state %u\n", idle_state);
> +			return -EINVAL;
> +		}
> +		mux->idle_state = idle_state;
> +	}
> +
> +	ret = mux_control_register(mux);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to register mux_control\n");
> +		mux_control_put(mux);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int mux_gpio_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mux_control *mux = to_mux_control(dev);
> +
> +	mux_control_unregister(mux);
> +	mux_control_put(mux);
> +	return 0;
> +}
> +
> +static struct platform_driver mux_gpio_driver = {
> +	.driver = {
> +		.name = "mux-gpio",
> +		.of_match_table	= of_match_ptr(mux_gpio_dt_ids),
> +	},
> +	.probe = mux_gpio_probe,
> +	.remove = mux_gpio_remove,
> +};
> +module_platform_driver(mux_gpio_driver);
> +
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
> +MODULE_DESCRIPTION("GPIO-controlled multiplexer driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mux.h b/include/linux/mux.h
> new file mode 100644
> index 000000000000..5b21b8184056
> --- /dev/null
> +++ b/include/linux/mux.h
> @@ -0,0 +1,53 @@
> +/*
> + * mux.h - definitions for the multiplexer interface
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _LINUX_MUX_H
> +#define _LINUX_MUX_H
> +
> +#include <linux/device.h>
> +#include <linux/rwsem.h>
> +
> +struct mux_control;
> +
> +struct mux_control_ops {
> +	int (*set)(struct mux_control *mux, int reg);
> +};
> +
> +struct mux_control {
> +	struct rw_semaphore lock; /* protects the state of the mux */
> +
> +	struct device dev;
> +	int id;
> +
> +	int cached_state;
> +	int idle_state;
> +
> +	const struct mux_control_ops *ops;
> +
> +	void *priv;
> +};
> +
> +#define to_mux_control(x) container_of((x), struct mux_control, dev)
> +
> +struct mux_control *mux_control_alloc(size_t sizeof_priv);
> +int mux_control_register(struct mux_control *mux);
> +void mux_control_unregister(struct mux_control *mux);
> +void mux_control_put(struct mux_control *mux);
> +
> +int mux_control_select(struct mux_control *mux, int state);
> +int mux_control_deselect(struct mux_control *mux);
> +
> +struct mux_control *mux_control_get(struct device *dev,
> +				    const char *mux_name);
> +struct mux_control *devm_mux_control_get(struct device *dev,
> +					 const char *mux_name);
> +void devm_mux_control_put(struct device *dev, struct mux_control *mux);
> +
> +#endif /* _LINUX_MUX_H */
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 3/7] iio: inkern: api for manipulating ext_info of iio channels
  2016-11-17 21:48 ` [RFC PATCH v2 3/7] iio: inkern: api for manipulating ext_info of iio channels Peter Rosin
@ 2016-11-19 15:38   ` Jonathan Cameron
  2016-11-21 15:45     ` Lars-Peter Clausen
  0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2016-11-19 15:38 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio

On 17/11/16 21:48, Peter Rosin wrote:
> Extend the inkern api with functions for reading and writing ext_info
> of iio channels.
I'd like Lars' feedback on this one.

Superficially looks fine to me but I am not as familiar with this interface
as Lars is ;) (he wrote it IIRC:)
> ---
>  drivers/iio/inkern.c         | 55 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/consumer.h |  6 +++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index cfca17ba2535..a8099b164222 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -850,3 +850,58 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(iio_write_channel_raw);
> +
> +int iio_get_channel_ext_info_count(struct iio_channel *chan)
> +{
> +	const struct iio_chan_spec_ext_info *ext_info;
> +	unsigned int i = 0;
> +
> +	if (!chan->channel->ext_info)
> +		return i;
> +
> +	for (ext_info = chan->channel->ext_info; ext_info->name; ext_info++)
> +		++i;
> +
> +	return i;
> +}
> +EXPORT_SYMBOL_GPL(iio_get_channel_ext_info_count);
> +
> +ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
> +				  const char *attr, char *buf)
> +{
> +	const struct iio_chan_spec_ext_info *ext_info;
> +
> +	if (!chan->channel->ext_info)
> +		return -EINVAL;
> +
> +	for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
> +		if (strcmp(attr, ext_info->name))
> +			continue;
> +
> +		return ext_info->read(chan->indio_dev, ext_info->private,
> +				      chan->channel, buf);
> +	}
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(iio_read_channel_ext_info);
> +
> +ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
> +				   const char *buf, size_t len)
> +{
> +	const struct iio_chan_spec_ext_info *ext_info;
> +
> +	if (!chan->channel->ext_info)
> +		return -EINVAL;
> +
> +	for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
> +		if (strcmp(attr, ext_info->name))
> +			continue;
> +
> +		return ext_info->write(chan->indio_dev, ext_info->private,
> +				       chan->channel, buf, len);
> +	}
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(iio_write_channel_ext_info);
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 9a4f336d8b4a..471dece8729a 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -299,4 +299,10 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val,
>  int iio_convert_raw_to_processed(struct iio_channel *chan, int raw,
>  	int *processed, unsigned int scale);
>  
> +int iio_get_channel_ext_info_count(struct iio_channel *chan);
> +ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
> +				  const char *attr, char *buf);
> +ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
> +				   const char *buf, size_t len);
> +
>  #endif
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 5/7] iio: multiplexer: new iio category and iio-mux driver
  2016-11-17 21:48 ` [RFC PATCH v2 5/7] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
@ 2016-11-19 15:49   ` Jonathan Cameron
  2016-11-19 22:08     ` Peter Rosin
  0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2016-11-19 15:49 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio

On 17/11/16 21:48, Peter Rosin wrote:
> When a multiplexer changes how an iio device behaves (for example
> by feeding different signals to an ADC), this driver can be used
> create one virtual iio channel for each multiplexer state.
> 
> Depends on the generic multiplexer driver.
I'm not really following what all the ext info stuff in here is about.
Could you add a little more description of that?

Perhaps an example of how it is used and what the resulting interface
looks like?

Thanks,

Jonathan
> ---
>  drivers/iio/Kconfig               |   1 +
>  drivers/iio/Makefile              |   1 +
>  drivers/iio/multiplexer/Kconfig   |  17 ++
>  drivers/iio/multiplexer/Makefile  |   6 +
>  drivers/iio/multiplexer/iio-mux.c | 444 ++++++++++++++++++++++++++++++++++++++
>  5 files changed, 469 insertions(+)
>  create mode 100644 drivers/iio/multiplexer/Kconfig
>  create mode 100644 drivers/iio/multiplexer/Makefile
>  create mode 100644 drivers/iio/multiplexer/iio-mux.c
> 
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 6743b18194fb..dcb541d0d70e 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -82,6 +82,7 @@ source "drivers/iio/humidity/Kconfig"
>  source "drivers/iio/imu/Kconfig"
>  source "drivers/iio/light/Kconfig"
>  source "drivers/iio/magnetometer/Kconfig"
> +source "drivers/iio/multiplexer/Kconfig"
>  source "drivers/iio/orientation/Kconfig"
>  if IIO_TRIGGER
>     source "drivers/iio/trigger/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 87e4c4369e2f..f9879c29cf6f 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -27,6 +27,7 @@ obj-y += humidity/
>  obj-y += imu/
>  obj-y += light/
>  obj-y += magnetometer/
> +obj-y += multiplexer/
>  obj-y += orientation/
>  obj-y += potentiometer/
>  obj-y += pressure/
> diff --git a/drivers/iio/multiplexer/Kconfig b/drivers/iio/multiplexer/Kconfig
> new file mode 100644
> index 000000000000..31cbe4509366
> --- /dev/null
> +++ b/drivers/iio/multiplexer/Kconfig
> @@ -0,0 +1,17 @@
> +#
> +# Multiplexer drivers
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +menu "Multiplexers"
> +
> +config IIO_MUX
> +	tristate "IIO multiplexer driver"
> +	depends on OF && MUX_GPIO
> +	help
> +	  Say yes here to build support for the IIO multiplexer.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called iio-mux.
> +
> +endmenu
> diff --git a/drivers/iio/multiplexer/Makefile b/drivers/iio/multiplexer/Makefile
> new file mode 100644
> index 000000000000..68be3c4abd07
> --- /dev/null
> +++ b/drivers/iio/multiplexer/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for industrial I/O multiplexer drivers
> +#
> +
> +# When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_IIO_MUX) += iio-mux.o
> diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
> new file mode 100644
> index 000000000000..2a8d00da990b
> --- /dev/null
> +++ b/drivers/iio/multiplexer/iio-mux.c
> @@ -0,0 +1,444 @@
> +/*
> + * IIO multiplexer driver
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda@axentia.se>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/mux.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +struct mux_ext_info_cache {
> +	char *data;
> +	size_t size;
> +};
> +
> +struct mux_child {
> +	u32 state;
> +
> +	struct mux_ext_info_cache *ext_info_cache;
> +};
> +
> +struct mux {
> +	u32 cached_state;
> +	struct mux_control *control;
> +	struct iio_channel *parent;
> +	struct iio_dev *indio_dev;
> +	struct iio_chan_spec *c;
> +	struct iio_chan_spec_ext_info *ext_info;
> +	struct mux_child *child;
> +};
> +
> +static int iio_mux_select(struct mux *mux, int idx)
> +{
> +	struct mux_child *child = &mux->child[idx];
> +	int ret;
> +	int i;
> +
> +	ret = mux_control_select(mux->control, child->state);
> +	if (ret < 0) {
> +		mux->cached_state = -1;
> +		return ret;
> +	}
> +
> +	if (mux->cached_state == child->state)
> +		return 0;
> +
I don't follow what is going on here..  Perhaps you could explain
futher?
> +	if (mux->c[idx].ext_info) {
> +		for (i = 0; mux->c[idx].ext_info[i].name; ++i) {
> +			const char *attr = mux->c[idx].ext_info[i].name;
> +			struct mux_ext_info_cache *cache;
> +
> +			cache = &child->ext_info_cache[i];
> +
> +			if (cache->size < 0)
> +				continue;
> +
> +			ret = iio_write_channel_ext_info(mux->parent, attr,
> +							 cache->data,
> +							 cache->size);
> +
> +			if (ret < 0) {
> +				mux_control_deselect(mux->control);
> +				mux->cached_state = -1;
> +				return ret;
> +			}
> +		}
> +	}
> +	mux->cached_state = child->state;
> +
> +	return 0;
> +}
> +
> +static void iio_mux_deselect(struct mux *mux)
> +{
> +	mux_control_deselect(mux->control);
> +}
> +
> +static int mux_read_raw(struct iio_dev *indio_dev,
> +			struct iio_chan_spec const *chan,
> +			int *val, int *val2, long mask)
> +{
> +	struct mux *mux = iio_priv(indio_dev);
> +	int idx = chan - mux->c;
> +	int ret;
> +
> +	ret = iio_mux_select(mux, idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = iio_read_channel_raw(mux->parent, val);
> +		break;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = iio_read_channel_scale(mux->parent, val, val2);
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	iio_mux_deselect(mux);
> +
> +	return ret;
> +}
> +
> +static int mux_read_avail(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan,
> +			  const int **vals, int *type, int *length,
> +			  long mask)
> +{
> +	struct mux *mux = iio_priv(indio_dev);
> +	int idx = chan - mux->c;
> +	int ret;
> +
> +	ret = iio_mux_select(mux, idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*type = IIO_VAL_INT;
> +		ret = iio_read_avail_channel_raw(mux->parent, vals, length);
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	iio_mux_deselect(mux);
> +
> +	return ret;
> +}
> +
> +static int mux_write_raw(struct iio_dev *indio_dev,
> +			 struct iio_chan_spec const *chan,
> +			 int val, int val2, long mask)
> +{
> +	struct mux *mux = iio_priv(indio_dev);
> +	int idx = chan - mux->c;
> +	int ret;
> +
> +	ret = iio_mux_select(mux, idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = iio_write_channel_raw(mux->parent, val);
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	iio_mux_deselect(mux);
> +
> +	return ret;
> +}
> +
> +static const struct iio_info mux_info = {
> +	.read_raw = mux_read_raw,
> +	.read_avail = mux_read_avail,
> +	.write_raw = mux_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static ssize_t mux_read_ext_info(struct iio_dev *indio_dev, uintptr_t private,
> +				 struct iio_chan_spec const *chan, char *buf)
> +{
> +	struct mux *mux = iio_priv(indio_dev);
> +	int idx = chan - mux->c;
> +	ssize_t ret;
> +
> +	ret = iio_mux_select(mux, idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = iio_read_channel_ext_info(mux->parent,
> +					mux->ext_info[private].name,
> +					buf);
> +
> +	iio_mux_deselect(mux);
> +
> +	return ret;
> +}
> +
> +static ssize_t mux_write_ext_info(struct iio_dev *indio_dev, uintptr_t private,
> +				  struct iio_chan_spec const *chan,
> +				  const char *buf, size_t len)
> +{
> +	struct device *dev = indio_dev->dev.parent;
> +	struct mux *mux = iio_priv(indio_dev);
> +	int idx = chan - mux->c;
> +	char *new;
> +	ssize_t ret;
> +
> +	ret = iio_mux_select(mux, idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	new = devm_kmemdup(dev, buf, len + 1, GFP_KERNEL);
> +	if (!new) {
> +		iio_mux_deselect(mux);
> +		return -ENOMEM;
> +	}
> +
> +	new[len] = 0;
> +
> +	ret = iio_write_channel_ext_info(mux->parent,
> +					 mux->ext_info[private].name,
> +					 buf, len);
> +	if (ret < 0) {
> +		iio_mux_deselect(mux);
> +		devm_kfree(dev, new);
> +		return ret;
> +	}
> +
> +	devm_kfree(dev, mux->child[idx].ext_info_cache[private].data);
> +	mux->child[idx].ext_info_cache[private].data = new;
> +	mux->child[idx].ext_info_cache[private].size = len;
> +
> +	iio_mux_deselect(mux);
> +
> +	return ret;
> +}
> +
> +static int mux_configure_channel(struct device *dev, struct mux *mux,
> +				 struct device_node *child_np, int idx)
> +{
> +	struct mux_child *child = &mux->child[idx];
> +	struct iio_chan_spec *c = &mux->c[idx];
> +	const struct iio_chan_spec *pc = mux->parent->channel;
> +	char *page;
> +	int num_ext_info;
> +	int i;
> +	int ret;
> +
> +	c->indexed = 1;
> +	c->channel = idx;
> +	c->output = pc->output;
> +	c->datasheet_name = child_np->name;
> +	c->ext_info = mux->ext_info;
> +
> +	ret = iio_get_channel_type(mux->parent, &c->type);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to get parent channel type\n");
> +		return ret;
> +	}
> +
> +	if (iio_channel_has_info(pc, IIO_CHAN_INFO_RAW))
> +		c->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW);
> +	if (iio_channel_has_info(pc, IIO_CHAN_INFO_SCALE))
> +		c->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE);
> +
> +	if (iio_channel_has_available(pc, IIO_CHAN_INFO_RAW))
> +		c->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
> +
> +	ret = of_property_read_u32(child_np, "reg", &child->state);
> +	if (ret < 0) {
> +		dev_err(dev, "no reg property for node '%s'\n", child_np->name);
> +		return ret;
> +	}
> +
> +	for (i = 0; i < idx; ++i) {
> +		if (mux->child[i].state == child->state) {
> +			dev_err(dev, "double use of reg %d\n", child->state);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	num_ext_info = iio_get_channel_ext_info_count(mux->parent);
> +	if (num_ext_info) {
> +		page = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL);
> +		if (!page)
> +			return -ENOMEM;
> +	}
> +	child->ext_info_cache = devm_kzalloc(dev,
> +					     sizeof(*child->ext_info_cache) *
> +					     num_ext_info, GFP_KERNEL);
> +	for (i = 0; i < num_ext_info; ++i) {
> +		child->ext_info_cache[i].size = -1;
> +
> +		if (!pc->ext_info[i].write)
> +			continue;
> +		if (!pc->ext_info[i].read)
> +			continue;
> +
> +		ret = iio_read_channel_ext_info(mux->parent,
> +						mux->ext_info[i].name,
> +						page);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to get ext_info '%s'\n",
> +				pc->ext_info[i].name);
> +			return ret;
> +		}
> +		if (ret >= PAGE_SIZE) {
> +			dev_err(dev, "too large ext_info '%s'\n",
> +				pc->ext_info[i].name);
> +			return -EINVAL;
> +		}
> +
> +		child->ext_info_cache[i].data = devm_kmemdup(dev, page, ret + 1,
> +							     GFP_KERNEL);
> +		child->ext_info_cache[i].data[ret] = 0;
> +		child->ext_info_cache[i].size = ret;
> +	}
> +
> +	if (num_ext_info)
> +		devm_kfree(dev, page);
> +
> +	return 0;
> +}
> +
> +static int mux_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *child_np;
> +	struct iio_dev *indio_dev;
> +	struct iio_channel *parent;
> +	struct mux *mux;
> +	int sizeof_ext_info;
> +	int children;
> +	int sizeof_priv;
> +	int i;
> +	int ret;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	parent = devm_iio_channel_get(dev, "parent");
> +	if (IS_ERR(parent)) {
> +		if (PTR_ERR(parent) != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get parent channel\n");
> +		return PTR_ERR(parent);
> +	}
> +
> +	sizeof_ext_info = iio_get_channel_ext_info_count(parent);
> +	if (sizeof_ext_info) {
> +		sizeof_ext_info += 1; /* one extra entry for the sentinel */
> +		sizeof_ext_info *= sizeof(*mux->ext_info);
> +	}
> +
> +	children = of_get_child_count(np);
> +	if (children <= 0) {
> +		dev_err(dev, "not even a single child\n");
> +		return -EINVAL;
> +	}
> +
> +	sizeof_priv = sizeof(*mux);
> +	sizeof_priv += sizeof(*mux->child) * children;
> +	sizeof_priv += sizeof(*mux->c) * children;
> +	sizeof_priv += sizeof_ext_info;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof_priv);
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	mux = iio_priv(indio_dev);
> +	mux->child = (struct mux_child *)(mux + 1);
> +	mux->c = (struct iio_chan_spec *)(mux->child + children);
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	mux->parent = parent;
> +	mux->cached_state = -1;
> +
> +	indio_dev->name = dev_name(dev);
> +	indio_dev->dev.parent = dev;
> +	indio_dev->info = &mux_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = mux->c;
> +	indio_dev->num_channels = children;
> +	if (sizeof_ext_info) {
> +		mux->ext_info = devm_kmemdup(dev,
> +					     parent->channel->ext_info,
> +					     sizeof_ext_info, GFP_KERNEL);
> +		if (!mux->ext_info)
> +			return -ENOMEM;
> +
> +		for (i = 0; mux->ext_info[i].name; ++i) {
> +			if (parent->channel->ext_info[i].read)
> +				mux->ext_info[i].read = mux_read_ext_info;
> +			if (parent->channel->ext_info[i].write)
> +				mux->ext_info[i].write = mux_write_ext_info;
> +			mux->ext_info[i].private = i;
> +		}
> +	}
> +
> +	i = 0;
> +	for_each_child_of_node(np, child_np) {
> +		ret = mux_configure_channel(dev, mux, child_np, i);
> +		if (ret < 0)
> +			return ret;
> +		i++;
> +	}
> +
> +	mux->control = devm_mux_control_get(dev, "mux");
> +	if (IS_ERR(mux->control)) {
> +		if (PTR_ERR(mux->control) != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get control-mux\n");
> +		return PTR_ERR(mux->control);
> +	}
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret) {
> +		dev_err(dev, "failed to register iio device\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id mux_match[] = {
> +	{ .compatible = "iio-mux" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mux_match);
> +
> +static struct platform_driver mux_driver = {
> +	.probe = mux_probe,
> +	.driver = {
> +		.name = "iio-mux",
> +		.of_match_table = mux_match,
> +	},
> +};
> +module_platform_driver(mux_driver);
> +
> +MODULE_DESCRIPTION("IIO multiplexer driver");
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
> +MODULE_LICENSE("GPL v2");
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 5/7] iio: multiplexer: new iio category and iio-mux driver
  2016-11-19 15:49   ` Jonathan Cameron
@ 2016-11-19 22:08     ` Peter Rosin
  2016-11-27 11:42       ` Jonathan Cameron
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-11-19 22:08 UTC (permalink / raw)
  To: Jonathan Cameron, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio

On 2016-11-19 16:49, Jonathan Cameron wrote:
> On 17/11/16 21:48, Peter Rosin wrote:
>> When a multiplexer changes how an iio device behaves (for example
>> by feeding different signals to an ADC), this driver can be used
>> create one virtual iio channel for each multiplexer state.
>>
>> Depends on the generic multiplexer driver.
> I'm not really following what all the ext info stuff in here is about.
> Could you add a little more description of that?

Certainly. I have two needs for this series. The first one is simple
when it comes to the iio part and complex because the mux is shared
between three 8-way muxes on three of the inputs to an ADS-1015 ADC.
The forth ADC line to the ADS-1015 is not muxed. Those three muxes
are of course GPIO-controlled and share GPIO pins. And the GPIO
pins also control an i2c bus that is muxed 8-ways as well. There are
eight (possible) batteries, and we monitor voltage/current/temp with
the 3 muxed ADC lines, and 8 chargers sit on i2c behind the i2c mux.
I guess it felt natural for the HW designer to select battery with
the GPIO lines, but that do not fit too well with the code as it
is without this series...

For this first need, the iio mux does not need ext_info.

The second need is simple in the mux part but worse in the iio
department. It's another 8-way mux that simply muxes an ADC line,
so that is simple. However, the ADC line is the envelope detector
that just got added to linux-next, and it has two ext_info
attributes that needs to be set differently for different mux
states. Two of the states need "invert" to be false, the rest need
"invert" to be true. And it is also preferable to have different
values for "compare_interval" for different mux states since the
signals on the diffrent mux states have the different frequency
characteristics.

True, I could have the ext-info attributes go straight through
the mux, and just start by writing values to "invert"
and "compare_interval", and only then read a sample. But then I
would need to lock out other users during the duration of this
transaction. I believe that the best place to put that lock is
in the iio mux (when it locks its control-mux) and not leave it
to userspace to solve this in some brittle cooperative manner.

> Perhaps an example of how it is used and what the resulting interface
> looks like?

The resulting interface is just a copy of the (ext_info) interface
exposed by the parent channel (with a cache that is rewritten to
the parent on every iio mux state change). I have plans to add code
to not rewrite ext_info attributes that have never been changed in
any mux state.

Below I have an example file listing.

device0 is a dpot (mcp4561).
device1 is a dac  (dpot-dac, wrapping the above dpot).
device2 is an adc (envelope-detector, wrapping the above dac)
device3 is a mux  (iio-mux, wrapping the above adc)

The 8-way iio-mux have no signals attached on mux states 0 and 1, which
is why the first channel for device 3 is in_altvoltage2.

Ultimately, I would like some knob to hide devices 0, 1 and 2 from
userspace. They need/should only be visible to in-kernel users. Or
is there such a knob already?

Cheers,
Peter

$ ls /sys/bus/iio/devices/iio\:device*
/sys/bus/iio/devices/iio:device0:
dev                           out_resistance_raw_available
name                          out_resistance_scale
of_node                       power
out_resistance0_raw           subsystem
out_resistance1_raw           uevent

/sys/bus/iio/devices/iio:device1:
dev                         out_voltage0_scale
name                        power
of_node                     subsystem
out_voltage0_raw            uevent
out_voltage0_raw_available

/sys/bus/iio/devices/iio:device2:
dev                              name
in_altvoltage0_compare_interval  of_node
in_altvoltage0_invert            power
in_altvoltage0_raw               subsystem
in_altvoltage0_scale             uevent

/sys/bus/iio/devices/iio:device3:
dev                              in_altvoltage5_raw
in_altvoltage2_compare_interval  in_altvoltage5_scale
in_altvoltage2_invert            in_altvoltage6_compare_interval
in_altvoltage2_raw               in_altvoltage6_invert
in_altvoltage2_scale             in_altvoltage6_raw
in_altvoltage3_compare_interval  in_altvoltage6_scale
in_altvoltage3_invert            in_altvoltage7_compare_interval
in_altvoltage3_raw               in_altvoltage7_invert
in_altvoltage3_scale             in_altvoltage7_raw
in_altvoltage4_compare_interval  in_altvoltage7_scale
in_altvoltage4_invert            name
in_altvoltage4_raw               of_node
in_altvoltage4_scale             power
in_altvoltage5_compare_interval  subsystem
in_altvoltage5_invert            uevent

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 2/7] misc: minimal mux subsystem and gpio-based mux controller
  2016-11-19 15:34   ` Jonathan Cameron
@ 2016-11-21 13:03     ` Peter Rosin
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-11-21 13:03 UTC (permalink / raw)
  To: Jonathan Cameron, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio

On 2016-11-19 16:34, Jonathan Cameron wrote:
> On 17/11/16 21:48, Peter Rosin wrote:
>> When both the iio subsystem and the i2c subsystem wants to update
>> the same mux, there needs to be some coordination. Invent a new
>> minimal "mux" subsystem that handles this.
> I'd probably put something more general in the description. Lots of things
> may need the same infrastructure.  This is just an example.

I'll make it more general for the next round.

> Few bits inline.
> 
> Also, I suspect you will fairly rapidly have a need for a strobe signal
> as well.  A lot of mux chips that are more than 2 way seem to have them to
> allow multiple chips to be synchronized.

I think that can be easily added later, when/if it's needed.

>>
>> Add a single backend driver for this new subsystem that can
>> control gpio based multiplexers.
>> ---
>>  drivers/misc/Kconfig    |   6 +
>>  drivers/misc/Makefile   |   2 +
>>  drivers/misc/mux-core.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/misc/mux-gpio.c | 115 +++++++++++++++++++
>>  include/linux/mux.h     |  53 +++++++++
>>  5 files changed, 475 insertions(+)
>>  create mode 100644 drivers/misc/mux-core.c
>>  create mode 100644 drivers/misc/mux-gpio.c
>>  create mode 100644 include/linux/mux.h
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 64971baf11fa..9e119bb78d82 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -766,6 +766,12 @@ config PANEL_BOOT_MESSAGE
>>  	  An empty message will only clear the display at driver init time. Any other
>>  	  printf()-formatted message is valid with newline and escape codes.
>>  
>> +config MUX_GPIO
>> +	tristate "GPIO-controlled MUX controller"
>> +	depends on OF
>> +	help
>> +	  GPIO-controlled MUX controller
>> +
>>  source "drivers/misc/c2port/Kconfig"
>>  source "drivers/misc/eeprom/Kconfig"
>>  source "drivers/misc/cb710/Kconfig"
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 31983366090a..92b547bcbac1 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -53,6 +53,8 @@ obj-$(CONFIG_ECHO)		+= echo/
>>  obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
>>  obj-$(CONFIG_CXL_BASE)		+= cxl/
>>  obj-$(CONFIG_PANEL)             += panel.o
>> +obj-$(CONFIG_MUX_GPIO)          += mux-core.o
>> +obj-$(CONFIG_MUX_GPIO)          += mux-gpio.o
>>  
>>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
>> diff --git a/drivers/misc/mux-core.c b/drivers/misc/mux-core.c
>> new file mode 100644
>> index 000000000000..7a8bf003a92c
>> --- /dev/null
>> +++ b/drivers/misc/mux-core.c
>> @@ -0,0 +1,299 @@
>> +/*
>> + * Multiplexer subsystem
>> + *
>> + * Copyright (C) 2016 Axentia Technologies AB
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#define pr_fmt(fmt) "mux-core: " fmt
>> +
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/idr.h>
>> +#include <linux/module.h>
>> +#include <linux/mux.h>
>> +#include <linux/of.h>
>> +#include <linux/slab.h>
>> +
>> +static struct bus_type mux_bus_type = {
>> +	.name = "mux",
>> +};
>> +
>> +static int __init mux_init(void)
>> +{
>> +	return bus_register(&mux_bus_type);
>> +}
>> +
>> +static void __exit mux_exit(void)
>> +{
>> +	bus_unregister(&mux_bus_type);
>> +}
>> +
>> +static DEFINE_IDA(mux_ida);
>> +
>> +static void mux_control_release(struct device *dev)
>> +{
>> +	struct mux_control *mux = to_mux_control(dev);
>> +
>> +	ida_simple_remove(&mux_ida, mux->id);
>> +	kfree(mux);
>> +}
>> +
>> +static struct device_type mux_control_type = {
>> +	.name = "mux-control",
>> +	.release = mux_control_release,
>> +};
>> +
>> +/*
>> + * Allocate a mux-control, plus an extra memory area for private use
>> + * by the caller.
>> + */
>> +struct mux_control *mux_control_alloc(size_t sizeof_priv)
>> +{
>> +	struct mux_control *mux;
>> +
> Worth planning ahead for spi controlled muxes and others that need their
> structures to be carefully aligned to avoid dma cacheline fun?
> Easy enough to add later I guess.

Right, I'll leave it for later when/if it's needed.

>> +	mux = kzalloc(sizeof(*mux) + sizeof_priv, GFP_KERNEL);
>> +	if (!mux)
>> +		return NULL;
>> +
>> +	mux->dev.bus = &mux_bus_type;
>> +	mux->dev.type = &mux_control_type;
>> +	device_initialize(&mux->dev);
>> +	dev_set_drvdata(&mux->dev, mux);
>> +
>> +	init_rwsem(&mux->lock);
>> +	mux->priv = mux + 1;
> Needed?  Or just do it with a bit of pointer math where the access is needed?

Good suggestion, I'll add an inline function and drop the priv member.

>> +
>> +	mux->id = ida_simple_get(&mux_ida, 0, 0, GFP_KERNEL);
>> +	if (mux->id < 0) {
>> +		pr_err("mux-controlX failed to get device id\n");
>> +		kfree(mux);
>> +		return NULL;
>> +	}
>> +	dev_set_name(&mux->dev, "mux:control%d", mux->id);
>> +
>> +	mux->cached_state = -1;
>> +	mux->idle_state = -1;
>> +
>> +	return mux;
>> +}
>> +EXPORT_SYMBOL_GPL(mux_control_alloc);
>> +
>> +/*
>> + * Register the mux-control, thus readying it for use.
> Either single line comment style - or perhaps kernel doc the lot...

Kernel doc was the plan from the start, coming up...

>> + */
>> +int mux_control_register(struct mux_control *mux)
>> +{
>> +	/* If the calling driver did not initialize of_node, do it here */
>> +	if (!mux->dev.of_node && mux->dev.parent)
>> +		mux->dev.of_node = mux->dev.parent->of_node;
>> +
>> +	return device_add(&mux->dev);
>> +}
>> +EXPORT_SYMBOL_GPL(mux_control_register);
>> +
>> +/*
>> + * Take the mux-control off-line.
>> + */
>> +void mux_control_unregister(struct mux_control *mux)
>> +{
>> +	device_del(&mux->dev);
>> +}
>> +EXPORT_SYMBOL_GPL(mux_control_unregister);
>> +
>> +/*
>> + * Put away the mux-control for good.
>> + */
>> +void mux_control_put(struct mux_control *mux)
>> +{
>> +	if (!mux)
>> +		return;
>> +	put_device(&mux->dev);
>> +}
>> +EXPORT_SYMBOL_GPL(mux_control_put);
>> +
>> +static int mux_control_set(struct mux_control *mux, int state)
>> +{
>> +	int ret = mux->ops->set(mux, state);
>> +
>> +	mux->cached_state = ret < 0 ? -1 : state;
>> +
>> +	return ret;
>> +}
>> +
>> +/*
>> + * Select the given multiplexer channel. Call mux_control_deselect()
>> + * when the operation is complete on the multiplexer channel, and the
>> + * multiplexer is free for others to use.
>> + */
>> +int mux_control_select(struct mux_control *mux, int state)
>> +{
>> +	int ret;
>> +
>> +	if (down_read_trylock(&mux->lock)) {
>> +		if (mux->cached_state == state)
>> +			return 0;
>> +
>> +		/* Sigh, the mux needs updating... */
>> +		up_read(&mux->lock);
>> +	}
>> +
>> +	/* ...or it's just contended. */
>> +	down_write(&mux->lock);
>> +
>> +	if (mux->cached_state == state) {
>> +		/*
>> +		 * Hmmm, someone else changed the mux to my liking.
>> +		 * That makes me wonder how long I waited for nothing...
>> +		 */
>> +		downgrade_write(&mux->lock);
>> +		return 0;
>> +	}
>> +
>> +	ret = mux_control_set(mux, state);
>> +	if (ret < 0) {
>> +		if (mux->idle_state != -1)
>> +			mux_control_set(mux, mux->idle_state);
>> +
>> +		up_write(&mux->lock);
>> +		return ret;
>> +	}
>> +
>> +	downgrade_write(&mux->lock);
>> +
>> +	return 1;
>> +}
>> +EXPORT_SYMBOL_GPL(mux_control_select);
>> +
>> +/*
>> + * Deselect the previously selected multiplexer channel.
>> + */
>> +int mux_control_deselect(struct mux_control *mux)
>> +{
>> +	int ret = 0;
>> +
>> +	if (mux->idle_state != -1 && mux->cached_state != mux->idle_state)
>> +		ret = mux_control_set(mux, mux->idle_state);
>> +
>> +	up_read(&mux->lock);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(mux_control_deselect);
>> +
>> +static int of_dev_node_match(struct device *dev, void *data)
>> +{
>> +	return dev->of_node == data;
>> +}
>> +
>> +static struct mux_control *of_find_mux_by_node(struct device_node *np)
>> +{
>> +	struct device *dev;
>> +
>> +	dev = bus_find_device(&mux_bus_type, NULL, np, of_dev_node_match);
>> +
>> +	return dev ? to_mux_control(dev) : NULL;
>> +}
>> +
>> +static struct mux_control *of_mux_control_get(struct device_node *np, int index)
>> +{
>> +	struct device_node *mux_np;
>> +	struct mux_control *mux;
>> +
>> +	mux_np = of_parse_phandle(np, "control-muxes", index);
>> +	if (!mux_np)
>> +		return NULL;
>> +
>> +	mux = of_find_mux_by_node(mux_np);
>> +	of_node_put(mux_np);
>> +
>> +	return mux;
>> +}
>> +
>> +/*
>> + * Get a named mux.
>> + */
>> +struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
>> +{
>> +	struct device_node *np = dev->of_node;
>> +	struct mux_control *mux;
>> +	int index;
>> +
>> +	index = of_property_match_string(np, "control-mux-names", mux_name);
>> +	if (index < 0) {
>> +		dev_err(dev, "failed to get control-mux %s:%s(%i)\n",
>> +			np->full_name, mux_name ?: "", index);
>> +		return ERR_PTR(index);
>> +	}
>> +
>> +	mux = of_mux_control_get(np, index);
>> +	if (!mux)
>> +		return ERR_PTR(-EPROBE_DEFER);
>> +
>> +	return mux;
>> +}
>> +EXPORT_SYMBOL_GPL(mux_control_get);
>> +
>> +static void devm_mux_control_free(struct device *dev, void *res)
>> +{
>> +	struct mux_control *mux = *(struct mux_control **)res;
>> +
>> +	mux_control_put(mux);
>> +}
>> +
>> +/*
>> + * Get a named mux, with resource management.
>> + */
> Guess it might be elsewhere in patch set but remember to add this to
> the global list of devm interfaces (in Documentation somewhere.. IIRC)

Right, now that you mention it, I remember having seen that document
at some point. I'll look it up.

Thanks for all comments!

Cheers,
Peter

>> +struct mux_control *devm_mux_control_get(struct device *dev,
>> +					 const char *mux_name)
>> +{
>> +	struct mux_control **ptr, *mux;
>> +
>> +	ptr = devres_alloc(devm_mux_control_free, sizeof(*ptr), GFP_KERNEL);
>> +	if (!ptr)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	mux = mux_control_get(dev, mux_name);
>> +	if (IS_ERR(mux)) {
>> +		devres_free(ptr);
>> +		return mux;
>> +	}
>> +
>> +	*ptr = mux;
>> +	devres_add(dev, ptr);
>> +
>> +	return mux;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_mux_control_get);
>> +
>> +static int devm_mux_control_match(struct device *dev, void *res, void *data)
>> +{
>> +	struct mux_control **r = res;
>> +
>> +	if (!r || !*r) {
>> +		WARN_ON(!r || !*r);
>> +		return 0;
>> +	}
>> +
>> +	return *r == data;
>> +}
>> +
>> +/*
>> + * Resource-managed version mux_control_put.
>> + */
>> +void devm_mux_control_put(struct device *dev, struct mux_control *mux)
>> +{
>> +	WARN_ON(devres_release(dev, devm_mux_control_free,
>> +			       devm_mux_control_match, mux));
>> +}
>> +EXPORT_SYMBOL_GPL(devm_mux_control_put);
>> +
>> +subsys_initcall(mux_init);
>> +module_exit(mux_exit);
>> +
>> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
>> +MODULE_DESCRIPTION("MUX subsystem");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/misc/mux-gpio.c b/drivers/misc/mux-gpio.c
>> new file mode 100644
>> index 000000000000..2ddd7fb24078
>> --- /dev/null
>> +++ b/drivers/misc/mux-gpio.c
>> @@ -0,0 +1,115 @@
>> +/*
>> + * GPIO-controlled multiplexer driver
>> + *
>> + * Copyright (C) 2016 Axentia Technologies AB
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/module.h>
>> +#include <linux/mux.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +
>> +struct mux_gpio {
>> +	struct gpio_descs *gpios;
>> +};
>> +
>> +static int mux_gpio_set(struct mux_control *mux, int val)
>> +{
>> +	struct mux_gpio *mux_gpio = mux->priv;
>> +	int i;
>> +
>> +	for (i = 0; i < mux_gpio->gpios->ndescs; i++)
>> +		gpiod_set_value_cansleep(mux_gpio->gpios->desc[i],
>> +					 val & (1 << i));
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct mux_control_ops mux_gpio_ops = {
>> +	.set = mux_gpio_set,
>> +};
>> +
>> +static const struct of_device_id mux_gpio_dt_ids[] = {
>> +	{ .compatible = "mux-gpio", },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
>> +
>> +static int mux_gpio_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct device_node *np = pdev->dev.of_node;
>> +	struct mux_control *mux;
>> +	struct mux_gpio *mux_gpio;
>> +	u32 idle_state;
>> +	int ret;
>> +
>> +	if (!np)
>> +		return -ENODEV;
>> +
>> +	mux = mux_control_alloc(sizeof(*mux_gpio));
>> +	if (!mux)
>> +		return -ENOMEM;
>> +	mux_gpio = mux->priv;
>> +	mux->dev.parent = dev;
>> +	mux->ops = &mux_gpio_ops;
>> +
>> +	platform_set_drvdata(pdev, mux);
>> +
>> +	mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
>> +	if (IS_ERR(mux_gpio->gpios)) {
>> +		if (PTR_ERR(mux_gpio->gpios) != -EPROBE_DEFER)
>> +			dev_err(dev, "failed to get gpios\n");
>> +		mux_control_put(mux);
>> +		return PTR_ERR(mux_gpio->gpios);
>> +	}
>> +
>> +	ret = of_property_read_u32(np, "idle-state", &idle_state);
>> +	if (ret >= 0) {
>> +		if (idle_state >= (1 << mux_gpio->gpios->ndescs)) {
>> +			dev_err(dev, "invalid idle-state %u\n", idle_state);
>> +			return -EINVAL;
>> +		}
>> +		mux->idle_state = idle_state;
>> +	}
>> +
>> +	ret = mux_control_register(mux);
>> +	if (ret < 0) {
>> +		dev_err(dev, "failed to register mux_control\n");
>> +		mux_control_put(mux);
>> +		return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int mux_gpio_remove(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct mux_control *mux = to_mux_control(dev);
>> +
>> +	mux_control_unregister(mux);
>> +	mux_control_put(mux);
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver mux_gpio_driver = {
>> +	.driver = {
>> +		.name = "mux-gpio",
>> +		.of_match_table	= of_match_ptr(mux_gpio_dt_ids),
>> +	},
>> +	.probe = mux_gpio_probe,
>> +	.remove = mux_gpio_remove,
>> +};
>> +module_platform_driver(mux_gpio_driver);
>> +
>> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
>> +MODULE_DESCRIPTION("GPIO-controlled multiplexer driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/include/linux/mux.h b/include/linux/mux.h
>> new file mode 100644
>> index 000000000000..5b21b8184056
>> --- /dev/null
>> +++ b/include/linux/mux.h
>> @@ -0,0 +1,53 @@
>> +/*
>> + * mux.h - definitions for the multiplexer interface
>> + *
>> + * Copyright (C) 2016 Axentia Technologies AB
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef _LINUX_MUX_H
>> +#define _LINUX_MUX_H
>> +
>> +#include <linux/device.h>
>> +#include <linux/rwsem.h>
>> +
>> +struct mux_control;
>> +
>> +struct mux_control_ops {
>> +	int (*set)(struct mux_control *mux, int reg);
>> +};
>> +
>> +struct mux_control {
>> +	struct rw_semaphore lock; /* protects the state of the mux */
>> +
>> +	struct device dev;
>> +	int id;
>> +
>> +	int cached_state;
>> +	int idle_state;
>> +
>> +	const struct mux_control_ops *ops;
>> +
>> +	void *priv;
>> +};
>> +
>> +#define to_mux_control(x) container_of((x), struct mux_control, dev)
>> +
>> +struct mux_control *mux_control_alloc(size_t sizeof_priv);
>> +int mux_control_register(struct mux_control *mux);
>> +void mux_control_unregister(struct mux_control *mux);
>> +void mux_control_put(struct mux_control *mux);
>> +
>> +int mux_control_select(struct mux_control *mux, int state);
>> +int mux_control_deselect(struct mux_control *mux);
>> +
>> +struct mux_control *mux_control_get(struct device *dev,
>> +				    const char *mux_name);
>> +struct mux_control *devm_mux_control_get(struct device *dev,
>> +					 const char *mux_name);
>> +void devm_mux_control_put(struct device *dev, struct mux_control *mux);
>> +
>> +#endif /* _LINUX_MUX_H */
>>
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 3/7] iio: inkern: api for manipulating ext_info of iio channels
  2016-11-19 15:38   ` Jonathan Cameron
@ 2016-11-21 15:45     ` Lars-Peter Clausen
  2016-11-21 16:07       ` Peter Rosin
  0 siblings, 1 reply; 19+ messages in thread
From: Lars-Peter Clausen @ 2016-11-21 15:45 UTC (permalink / raw)
  To: Jonathan Cameron, Peter Rosin, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Peter Meerwald-Stadler, Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c, devicetree, linux-iio

On 11/19/2016 04:38 PM, Jonathan Cameron wrote:
> On 17/11/16 21:48, Peter Rosin wrote:
>> Extend the inkern api with functions for reading and writing ext_info
>> of iio channels.
> I'd like Lars' feedback on this one.
> 
> Superficially looks fine to me but I am not as familiar with this interface
> as Lars is ;) (he wrote it IIRC:)

The implementation looks OK. I'm not necessarily convinced about the concept
though, but the code is manageable so I guess it is OK.

The final version should add kernel API documentation for the new functions.

>> ---
>>  drivers/iio/inkern.c         | 55 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/iio/consumer.h |  6 +++++
>>  2 files changed, 61 insertions(+)
>>
>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>> index cfca17ba2535..a8099b164222 100644
>> --- a/drivers/iio/inkern.c
>> +++ b/drivers/iio/inkern.c
>> @@ -850,3 +850,58 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
>>  	return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(iio_write_channel_raw);
>> +
>> +int iio_get_channel_ext_info_count(struct iio_channel *chan)

should be unsigned.

>> +{
>> +	const struct iio_chan_spec_ext_info *ext_info;
>> +	unsigned int i = 0;
>> +
>> +	if (!chan->channel->ext_info)
>> +		return i;
>> +
>> +	for (ext_info = chan->channel->ext_info; ext_info->name; ext_info++)
>> +		++i;
>> +
>> +	return i;
>> +}
>> +EXPORT_SYMBOL_GPL(iio_get_channel_ext_info_count);
>> +
>> +ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
>> +				  const char *attr, char *buf)
>> +{
>> +	const struct iio_chan_spec_ext_info *ext_info;
>> +
>> +	if (!chan->channel->ext_info)
>> +		return -EINVAL;
>> +
>> +	for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
>> +		if (strcmp(attr, ext_info->name))
>> +			continue;

You could factor the lookup out into a helper function that is used for both
read and write. And also stop searching once a match was found.

>> +
>> +		return ext_info->read(chan->indio_dev, ext_info->private,
>> +				      chan->channel, buf);
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(iio_read_channel_ext_info);
>> +
>> +ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
>> +				   const char *buf, size_t len)
>> +{
>> +	const struct iio_chan_spec_ext_info *ext_info;
>> +
>> +	if (!chan->channel->ext_info)
>> +		return -EINVAL;
>> +
>> +	for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
>> +		if (strcmp(attr, ext_info->name))
>> +			continue;
>> +
>> +		return ext_info->write(chan->indio_dev, ext_info->private,
>> +				       chan->channel, buf, len);
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +EXPORT_SYMBOL_GPL(iio_write_channel_ext_info);
>> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
>> index 9a4f336d8b4a..471dece8729a 100644
>> --- a/include/linux/iio/consumer.h
>> +++ b/include/linux/iio/consumer.h
>> @@ -299,4 +299,10 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val,
>>  int iio_convert_raw_to_processed(struct iio_channel *chan, int raw,
>>  	int *processed, unsigned int scale);
>>  
>> +int iio_get_channel_ext_info_count(struct iio_channel *chan);
>> +ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
>> +				  const char *attr, char *buf);
>> +ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
>> +				   const char *buf, size_t len);
>> +
>>  #endif
>>
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 3/7] iio: inkern: api for manipulating ext_info of iio channels
  2016-11-21 15:45     ` Lars-Peter Clausen
@ 2016-11-21 16:07       ` Peter Rosin
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-11-21 16:07 UTC (permalink / raw)
  To: Lars-Peter Clausen, Jonathan Cameron, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Peter Meerwald-Stadler, Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c, devicetree, linux-iio

On 2016-11-21 16:45, Lars-Peter Clausen wrote:
> On 11/19/2016 04:38 PM, Jonathan Cameron wrote:
>> On 17/11/16 21:48, Peter Rosin wrote:
>>> Extend the inkern api with functions for reading and writing ext_info
>>> of iio channels.
>> I'd like Lars' feedback on this one.
>>
>> Superficially looks fine to me but I am not as familiar with this interface
>> as Lars is ;) (he wrote it IIRC:)
> 
> The implementation looks OK. I'm not necessarily convinced about the concept
> though, but the code is manageable so I guess it is OK.
> 
> The final version should add kernel API documentation for the new functions.

I added that in v3 that I sent earlier today.

>>> ---
>>>  drivers/iio/inkern.c         | 55 ++++++++++++++++++++++++++++++++++++++++++++
>>>  include/linux/iio/consumer.h |  6 +++++
>>>  2 files changed, 61 insertions(+)
>>>
>>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>>> index cfca17ba2535..a8099b164222 100644
>>> --- a/drivers/iio/inkern.c
>>> +++ b/drivers/iio/inkern.c
>>> @@ -850,3 +850,58 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
>>>  	return ret;
>>>  }
>>>  EXPORT_SYMBOL_GPL(iio_write_channel_raw);
>>> +
>>> +int iio_get_channel_ext_info_count(struct iio_channel *chan)
> 
> should be unsigned.

Correct, I had a plan to do that change, but it slipped my mind. Thanks
for catching it!

>>> +{
>>> +	const struct iio_chan_spec_ext_info *ext_info;
>>> +	unsigned int i = 0;
>>> +
>>> +	if (!chan->channel->ext_info)
>>> +		return i;
>>> +
>>> +	for (ext_info = chan->channel->ext_info; ext_info->name; ext_info++)
>>> +		++i;
>>> +
>>> +	return i;
>>> +}
>>> +EXPORT_SYMBOL_GPL(iio_get_channel_ext_info_count);
>>> +
>>> +ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
>>> +				  const char *attr, char *buf)
>>> +{
>>> +	const struct iio_chan_spec_ext_info *ext_info;
>>> +
>>> +	if (!chan->channel->ext_info)
>>> +		return -EINVAL;
>>> +
>>> +	for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
>>> +		if (strcmp(attr, ext_info->name))
>>> +			continue;
> 
> You could factor the lookup out into a helper function that is used for both
> read and write. And also stop searching once a match was found.

I'll do a lookup helper for v4. But the stop-searching-thing is already
done with the return statements at the end of the for-loops.

Thanks for looking!

Cheers,
Peter

>>> +
>>> +		return ext_info->read(chan->indio_dev, ext_info->private,
>>> +				      chan->channel, buf);
>>> +	}
>>> +
>>> +	return -EINVAL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(iio_read_channel_ext_info);
>>> +
>>> +ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
>>> +				   const char *buf, size_t len)
>>> +{
>>> +	const struct iio_chan_spec_ext_info *ext_info;
>>> +
>>> +	if (!chan->channel->ext_info)
>>> +		return -EINVAL;
>>> +
>>> +	for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
>>> +		if (strcmp(attr, ext_info->name))
>>> +			continue;
>>> +
>>> +		return ext_info->write(chan->indio_dev, ext_info->private,
>>> +				       chan->channel, buf, len);
>>> +	}
>>> +
>>> +	return -EINVAL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(iio_write_channel_ext_info);
>>> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
>>> index 9a4f336d8b4a..471dece8729a 100644
>>> --- a/include/linux/iio/consumer.h
>>> +++ b/include/linux/iio/consumer.h
>>> @@ -299,4 +299,10 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val,
>>>  int iio_convert_raw_to_processed(struct iio_channel *chan, int raw,
>>>  	int *processed, unsigned int scale);
>>>  
>>> +int iio_get_channel_ext_info_count(struct iio_channel *chan);
>>> +ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
>>> +				  const char *attr, char *buf);
>>> +ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
>>> +				   const char *buf, size_t len);
>>> +
>>>  #endif
>>>
>>
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v2 5/7] iio: multiplexer: new iio category and iio-mux driver
  2016-11-19 22:08     ` Peter Rosin
@ 2016-11-27 11:42       ` Jonathan Cameron
  0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2016-11-27 11:42 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio

On 19/11/16 22:08, Peter Rosin wrote:
> On 2016-11-19 16:49, Jonathan Cameron wrote:
>> On 17/11/16 21:48, Peter Rosin wrote:
>>> When a multiplexer changes how an iio device behaves (for example
>>> by feeding different signals to an ADC), this driver can be used
>>> create one virtual iio channel for each multiplexer state.
>>>
>>> Depends on the generic multiplexer driver.
>> I'm not really following what all the ext info stuff in here is about.
>> Could you add a little more description of that?
> 
> Certainly. I have two needs for this series. The first one is simple
> when it comes to the iio part and complex because the mux is shared
> between three 8-way muxes on three of the inputs to an ADS-1015 ADC.
> The forth ADC line to the ADS-1015 is not muxed. Those three muxes
> are of course GPIO-controlled and share GPIO pins. And the GPIO
> pins also control an i2c bus that is muxed 8-ways as well. There are
> eight (possible) batteries, and we monitor voltage/current/temp with
> the 3 muxed ADC lines, and 8 chargers sit on i2c behind the i2c mux.
> I guess it felt natural for the HW designer to select battery with
> the GPIO lines, but that do not fit too well with the code as it
> is without this series...
> 
> For this first need, the iio mux does not need ext_info.
> 
> The second need is simple in the mux part but worse in the iio
> department. It's another 8-way mux that simply muxes an ADC line,
> so that is simple. However, the ADC line is the envelope detector
> that just got added to linux-next, and it has two ext_info
> attributes that needs to be set differently for different mux
> states. Two of the states need "invert" to be false, the rest need
> "invert" to be true. And it is also preferable to have different
> values for "compare_interval" for different mux states since the
> signals on the diffrent mux states have the different frequency
> characteristics.
> 
> True, I could have the ext-info attributes go straight through
> the mux, and just start by writing values to "invert"
> and "compare_interval", and only then read a sample. But then I
> would need to lock out other users during the duration of this
> transaction. I believe that the best place to put that lock is
> in the iio mux (when it locks its control-mux) and not leave it
> to userspace to solve this in some brittle cooperative manner.
> 
>> Perhaps an example of how it is used and what the resulting interface
>> looks like?
> 
> The resulting interface is just a copy of the (ext_info) interface
> exposed by the parent channel (with a cache that is rewritten to
> the parent on every iio mux state change). I have plans to add code
> to not rewrite ext_info attributes that have never been changed in
> any mux state.
> 
> Below I have an example file listing.
> 
> device0 is a dpot (mcp4561).
> device1 is a dac  (dpot-dac, wrapping the above dpot).
> device2 is an adc (envelope-detector, wrapping the above dac)
> device3 is a mux  (iio-mux, wrapping the above adc)
> 
> The 8-way iio-mux have no signals attached on mux states 0 and 1, which
> is why the first channel for device 3 is in_altvoltage2.
> 
> Ultimately, I would like some knob to hide devices 0, 1 and 2 from
> userspace. They need/should only be visible to in-kernel users. Or
> is there such a knob already?
> 
There isn't and this feeds into what Lars was suggesting with a fabric
driver.  The complexity of the description in device tree is getting really
very nasty indeed, perhaps we are better off allowing for complex cases
to have a driver that directly hooks into all the relevant elements and
can do magic channel remapping etc.

That could then register the 3 muxes and acquire all the chanenls required
to build a single many channel device that hides all the complexity from userspace.

I've considered working out how to do an IIO multiplexer before in software which would
allow us to make 'fake' devices that wrap multiple physical devices.  In the general
case it has always felt to complex as we'd want it to handle triggered and buffered
data flows.  That brings all sorts of nasty data alignment problems with it as there
is no guarantee the devices are producing aligned data at all.

In the specific case though a driver can bundle up everything it needs to create
a pseudo device (for triggered flows we'd probably need a little magic to hold off the
trigger but that's not hard).  With sysfs only access it would be simple to do but
hard to describe.  Hence bringing us back to fabric drivers.

Thanks for the description. Good to understand what you are trying to handle.

Jonathan
> Cheers,
> Peter
> 
> $ ls /sys/bus/iio/devices/iio\:device*
> /sys/bus/iio/devices/iio:device0:
> dev                           out_resistance_raw_available
> name                          out_resistance_scale
> of_node                       power
> out_resistance0_raw           subsystem
> out_resistance1_raw           uevent
> 
> /sys/bus/iio/devices/iio:device1:
> dev                         out_voltage0_scale
> name                        power
> of_node                     subsystem
> out_voltage0_raw            uevent
> out_voltage0_raw_available
> 
> /sys/bus/iio/devices/iio:device2:
> dev                              name
> in_altvoltage0_compare_interval  of_node
> in_altvoltage0_invert            power
> in_altvoltage0_raw               subsystem
> in_altvoltage0_scale             uevent
> 
> /sys/bus/iio/devices/iio:device3:
> dev                              in_altvoltage5_raw
> in_altvoltage2_compare_interval  in_altvoltage5_scale
> in_altvoltage2_invert            in_altvoltage6_compare_interval
> in_altvoltage2_raw               in_altvoltage6_invert
> in_altvoltage2_scale             in_altvoltage6_raw
> in_altvoltage3_compare_interval  in_altvoltage6_scale
> in_altvoltage3_invert            in_altvoltage7_compare_interval
> in_altvoltage3_raw               in_altvoltage7_invert
> in_altvoltage3_scale             in_altvoltage7_raw
> in_altvoltage4_compare_interval  in_altvoltage7_scale
> in_altvoltage4_invert            name
> in_altvoltage4_raw               of_node
> in_altvoltage4_scale             power
> in_altvoltage5_compare_interval  subsystem
> in_altvoltage5_invert            uevent
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-11-27 11:42 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-17 21:48 [RFC PATCH v2 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
2016-11-17 21:48 ` [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio Peter Rosin
2016-11-18 15:35   ` Rob Herring
2016-11-18 16:59     ` Peter Rosin
2016-11-19 15:21       ` Jonathan Cameron
2016-11-17 21:48 ` [RFC PATCH v2 2/7] misc: minimal mux subsystem and gpio-based mux controller Peter Rosin
2016-11-19 15:34   ` Jonathan Cameron
2016-11-21 13:03     ` Peter Rosin
2016-11-17 21:48 ` [RFC PATCH v2 3/7] iio: inkern: api for manipulating ext_info of iio channels Peter Rosin
2016-11-19 15:38   ` Jonathan Cameron
2016-11-21 15:45     ` Lars-Peter Clausen
2016-11-21 16:07       ` Peter Rosin
2016-11-17 21:48 ` [RFC PATCH v2 4/7] dt-bindings: iio: iio-mux: document iio-mux bindings Peter Rosin
2016-11-17 21:48 ` [RFC PATCH v2 5/7] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
2016-11-19 15:49   ` Jonathan Cameron
2016-11-19 22:08     ` Peter Rosin
2016-11-27 11:42       ` Jonathan Cameron
2016-11-17 21:48 ` [RFC PATCH v2 6/7] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings Peter Rosin
2016-11-17 21:48 ` [RFC PATCH v2 7/7] i2c: i2c-mux-simple: new driver Peter Rosin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).