linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes
@ 2016-11-21 13:17 Peter Rosin
  2016-11-21 13:17 ` [PATCH v3 1/7] dt-bindings: document devicetree bindings for mux-controllers and mux-gpio Peter Rosin
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Peter Rosin @ 2016-11-21 13:17 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, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

Hi!

The code depends on the _available work in iio which can
be found in linux-next.

v2 -> v3 changes

- have the mux-controller in the parent node of any mux-controller consumer,
  to hopefully satisfy complaint from Rob about dt complexity.
- improve commit message of the mux subsystem commit, making it more
  general, as requested by Jonathan.
- remove priv member from struct mux_control and calculate it on the
  fly. /Jonathan.
- make the function comments in mux-core.c kernel doc. /Jonathan
- add devm_mux_control_* to Documentation/driver.model/devres.txt. /Jonathan
- add common dt bindings for mux-controllers, refer to them from the
  mux-gpio bindings.
- clarify how the gpio pins map to the mux state.
- separate CONFIG_ variables for the mux core and the mux gpio driver.
- improve Kconfig help texts.
- make CONFIG_MUX_GPIO depend on CONFIG_GPIOLIB.
- keep track of the number of mux states in the mux core.
- since the iio channel number is used as mux state, it was possible
  to drop the state member from the mux_child struct.
- cleanup dt bindings for i2c-mux-simple, it had some of copy-paste
  problems from ots origin (i2c-mux-gpio).
- select the mux control subsystem in config for the i2c-mux-simple driver.
- add entries to MAINTAINERS and my sign-off, I'm now satisfied and know
  nothing in this to be ashamed of.

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 (and subsystem) 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...

Cheers,
Peter

Peter Rosin (7):
  dt-bindings: document devicetree bindings for mux-controllers and
    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     |  76 ++++
 .../bindings/iio/multiplexer/iio-mux.txt           |  47 +++
 .../devicetree/bindings/misc/mux-controller.txt    |  76 ++++
 .../devicetree/bindings/misc/mux-gpio.txt          |  78 ++++
 Documentation/driver-model/devres.txt              |   6 +-
 MAINTAINERS                                        |  14 +
 drivers/i2c/muxes/Kconfig                          |  13 +
 drivers/i2c/muxes/Makefile                         |   1 +
 drivers/i2c/muxes/i2c-mux-simple.c                 | 177 ++++++++
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/inkern.c                               |  55 +++
 drivers/iio/multiplexer/Kconfig                    |  18 +
 drivers/iio/multiplexer/Makefile                   |   6 +
 drivers/iio/multiplexer/iio-mux.c                  | 450 +++++++++++++++++++++
 drivers/misc/Kconfig                               |  23 ++
 drivers/misc/Makefile                              |   2 +
 drivers/misc/mux-core.c                            | 325 +++++++++++++++
 drivers/misc/mux-gpio.c                            | 116 ++++++
 include/linux/iio/consumer.h                       |  37 ++
 include/linux/mux.h                                |  55 +++
 21 files changed, 1576 insertions(+), 1 deletion(-)
 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-controller.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] 14+ messages in thread

* [PATCH v3 1/7] dt-bindings: document devicetree bindings for mux-controllers and mux-gpio
  2016-11-21 13:17 [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
@ 2016-11-21 13:17 ` Peter Rosin
  2016-11-21 13:17 ` [PATCH v3 2/7] misc: minimal mux subsystem and gpio-based mux controller Peter Rosin
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Peter Rosin @ 2016-11-21 13:17 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, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/misc/mux-controller.txt    | 76 +++++++++++++++++++++
 .../devicetree/bindings/misc/mux-gpio.txt          | 78 ++++++++++++++++++++++
 MAINTAINERS                                        |  5 ++
 3 files changed, 159 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/mux-controller.txt
 create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt

diff --git a/Documentation/devicetree/bindings/misc/mux-controller.txt b/Documentation/devicetree/bindings/misc/mux-controller.txt
new file mode 100644
index 000000000000..6946b5428546
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/mux-controller.txt
@@ -0,0 +1,76 @@
+Common multiplexer controller bindings
+
+A mux controller will have one, or several, child devices that uses (or
+consumes) a mux controller. A mux controller can possibly control several
+parallel multiplexers. The node for a mux controller will have one child
+node for each multiplexer controller consumer.
+
+A mux controller provides a number of states to its consumers, and the
+state space is a simple zero-based enumeration. I.e. 0-1 for a 2-way
+multiplexer, 0-7 for an 8-way multiplexer, etc.
+
+Example:
+
+	/*
+	 * Two consumers (one for an ADC line and one for an i2c bus) of
+	 * parallel 4-way multiplexers controlled by the same two GPIO-lines.
+	 */
+	mux {
+		compatible = "mux-gpio";
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+
+		adc {
+			compatible = "iio-mux";
+			io-channels = <&adc 0>;
+			io-channel-names = "parent";
+
+			#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>;
+
+			#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 {
+					/* ... */
+				};
+			};
+		};
+	};
diff --git a/Documentation/devicetree/bindings/misc/mux-gpio.txt b/Documentation/devicetree/bindings/misc/mux-gpio.txt
new file mode 100644
index 000000000000..23b87913f4b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/mux-gpio.txt
@@ -0,0 +1,78 @@
+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.
+* Standard mux-controller bindings as decribed in mux-controller.txt
+
+Optional properties:
+- idle-state : if present, the state the mux will have when idle.
+
+The multiplexer state is defined as the number represented by the
+multiplexer GPIO pins, where the first pin is the least significant
+bit. And active pin is a binary 1, an inactive pin is a binary 0.
+
+Example:
+	mux {
+		compatible = "mux-gpio";
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+
+		adc {
+			compatible = "iio-mux";
+			io-channels = <&adc 0>;
+			io-channel-names = "parent";
+
+			#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>;
+
+			#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 {
+					/* ... */
+				};
+			};
+		};
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index ad9b965e5e44..7d797f087822 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8244,6 +8244,11 @@ S:	Orphan
 F:	drivers/mmc/host/mmc_spi.c
 F:	include/linux/spi/mmc_spi.h
 
+MULTIPLEXER SUBSYSTEM
+M:	Peter Rosin <peda@axentia.se>
+S:	Maintained
+F:	Documentation/devicetree/bindings/misc/mux-*
+
 MULTISOUND SOUND DRIVER
 M:	Andrew Veliath <andrewtv@usa.net>
 S:	Maintained
-- 
2.1.4

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

* [PATCH v3 2/7] misc: minimal mux subsystem and gpio-based mux controller
  2016-11-21 13:17 [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
  2016-11-21 13:17 ` [PATCH v3 1/7] dt-bindings: document devicetree bindings for mux-controllers and mux-gpio Peter Rosin
@ 2016-11-21 13:17 ` Peter Rosin
  2016-11-21 13:17 ` [PATCH v3 3/7] iio: inkern: api for manipulating ext_info of iio channels Peter Rosin
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Peter Rosin @ 2016-11-21 13:17 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, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

Add a new minimalistic subsystem that handles multiplexer controllers.
When multiplexers are used in various places in the kernel, and the
same multiplexer controller can be used for several independent things,
there should be one place to implement support for said multiplexer
controller.

A single multiplexer controller can also be used to control several
parallel multiplexers, that are in turn used by different subsystems
in the kernel, leading to a need to coordinate multiplexer accesses.
The multiplexer subsystem handles this coordination.

This new mux controller subsystem comes with a single backend driver
that controls gpio based multiplexers.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/driver-model/devres.txt |   6 +-
 MAINTAINERS                           |   2 +
 drivers/misc/Kconfig                  |  23 +++
 drivers/misc/Makefile                 |   2 +
 drivers/misc/mux-core.c               | 325 ++++++++++++++++++++++++++++++++++
 drivers/misc/mux-gpio.c               | 116 ++++++++++++
 include/linux/mux.h                   |  55 ++++++
 7 files changed, 528 insertions(+), 1 deletion(-)
 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/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index 167070895498..947bfcfac9ae 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -330,7 +330,11 @@ MEM
   devm_kzalloc()
 
 MFD
- devm_mfd_add_devices()
+  devm_mfd_add_devices()
+
+MUX
+  devm_mux_control_get()
+  devm_mux_control_put()
 
 PCI
   pcim_enable_device()	: after success, all PCI ops become managed
diff --git a/MAINTAINERS b/MAINTAINERS
index 7d797f087822..37974aeee750 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8248,6 +8248,8 @@ MULTIPLEXER SUBSYSTEM
 M:	Peter Rosin <peda@axentia.se>
 S:	Maintained
 F:	Documentation/devicetree/bindings/misc/mux-*
+F:	include/linux/mux.h
+F:	drivers/misc/mux-*
 
 MULTISOUND SOUND DRIVER
 M:	Andrew Veliath <andrewtv@usa.net>
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 64971baf11fa..a3ca79e082c7 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -766,6 +766,29 @@ 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 MULTIPLEXER
+	tristate "Multiplexer subsystem"
+	help
+	  Multiplexer controller subsystem. Multiplexers are used in a
+	  variety of settings, and this subsystem abstracts their use
+	  so that the rest of the kernel sees a common interface. When
+	  multiple parallel multiplexers are controlled by one single
+	  multiplexer controller, this subsystem also coordinates the
+	  multiplexer accesses.
+
+if MULTIPLEXER
+
+config MUX_GPIO
+	tristate "GPIO-controlled MUX controller"
+	depends on OF && GPIOLIB
+	help
+	  GPIO-controlled MUX controller.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called mux-gpio.
+
+endif
+
 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..0befa2bba762 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_MULTIPLEXER)      	+= 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..5142caa19236
--- /dev/null
+++ b/drivers/misc/mux-core.c
@@ -0,0 +1,325 @@
+/*
+ * 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/of_platform.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,
+};
+
+/**
+ * mux_control_alloc - allocate a mux-control
+ * @sizeof_priv: Size of extra memory area for private use by the caller.
+ *
+ * Returns the new mux-control.
+ */
+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->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);
+
+/**
+ * mux_control_register - register a mux-control, thus readying it for use
+ * @mux: The mux-control to register.
+ *
+ * Returns zero on success or a negative errno on error.
+ */
+int mux_control_register(struct mux_control *mux)
+{
+	int ret;
+
+	/* 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;
+
+	ret = device_add(&mux->dev);
+	if (ret < 0)
+		return ret;
+
+	ret = of_platform_populate(mux->dev.of_node, NULL, NULL, &mux->dev);
+	if (ret < 0)
+		device_del(&mux->dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(mux_control_register);
+
+/**
+ * mux_control_unregister - take the mux-control off-line, reversing the
+ *			    effexts of mux_control_register
+ * @mux: the mux-control to unregister.
+ */
+void mux_control_unregister(struct mux_control *mux)
+{
+	of_platform_depopulate(&mux->dev);
+	device_del(&mux->dev);
+}
+EXPORT_SYMBOL_GPL(mux_control_unregister);
+
+/**
+ * mux_control_put - put away the mux-control for good, reversing the
+ *		     effects of either mux_control_alloc or mux_control_get
+ * @mux: The mux-control to put away.
+ */
+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;
+}
+
+/**
+ * mux_control_select - select the given multiplexer state
+ * @mux: The mux-control to request a change of state from.
+ * @state: The new requested state.
+ *
+ * Returns 0 if the requested state was already active, or 1 it the
+ * mux-control state was changed to the requested state. Or a negavive
+ * errno on error.
+ * Note that the difference in return value of zero or one is of
+ * questionable value; especially if the mux-control has several independent
+ * consumers, which is something the consumers should not be making
+ * assumptions about.
+ *
+ * Make sure to call mux_control_deselect when the operation is complete and
+ * the mux-control is free for others to use, but do not call
+ * mux_control_deselect if mux_control_select fails.
+ */
+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);
+
+/**
+ * mux_control_deselect - deselect the previously selected multiplexer state
+ * @mux: The mux-control to deselect.
+ *
+ * Returns 0 on success and a negative errno on error. An error can only
+ * occur if the mux has an idle state. Note that even if an error occurs, the
+ * mux-control is unlocked for others to access.
+ */
+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;
+}
+
+/**
+ * mux_control_get - get the mux-control for a device
+ * @dev: The device that needs a mux-control.
+ *
+ * Returns the mux-control.
+ */
+struct mux_control *mux_control_get(struct device *dev)
+{
+	struct mux_control *mux;
+
+	if (!dev->of_node)
+		return ERR_PTR(-ENODEV);
+
+	mux = of_find_mux_by_node(dev->of_node->parent);
+	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);
+}
+
+/**
+ * devm_mux_control_get - get the mux-control for a device, with resource
+ *			  management
+ * @dev: The device that needs a mux-control.
+ *
+ * Returns the mux-control.
+ */
+struct mux_control *devm_mux_control_get(struct device *dev)
+{
+	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);
+	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;
+}
+
+/**
+ * devm_mux_control_put - resource-managed version mux_control_put
+ * @dev: The device that originally got the mux-control.
+ * @mux: The mux-control to put away.
+ *
+ * Note that you do not normally need to call this function.
+ */
+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..e9baca35f400
--- /dev/null
+++ b/drivers/misc/mux-gpio.c
@@ -0,0 +1,116 @@
+/*
+ * 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_control_priv(mux);
+	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_control_priv(mux);
+	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);
+	}
+	mux->states = 1 << mux_gpio->gpios->ndescs;
+
+	ret = of_property_read_u32(np, "idle-state", &idle_state);
+	if (ret >= 0) {
+		if (idle_state >= mux->states) {
+			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..2f1472192d6d
--- /dev/null
+++ b/include/linux/mux.h
@@ -0,0 +1,55 @@
+/*
+ * 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;
+
+	unsigned int states;
+	int cached_state;
+	int idle_state;
+
+	const struct mux_control_ops *ops;
+};
+
+#define to_mux_control(x) container_of((x), struct mux_control, dev)
+
+static inline void *mux_control_priv(struct mux_control *mux)
+{
+	return mux + 1;
+}
+
+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);
+struct mux_control *devm_mux_control_get(struct device *dev);
+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] 14+ messages in thread

* [PATCH v3 3/7] iio: inkern: api for manipulating ext_info of iio channels
  2016-11-21 13:17 [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
  2016-11-21 13:17 ` [PATCH v3 1/7] dt-bindings: document devicetree bindings for mux-controllers and mux-gpio Peter Rosin
  2016-11-21 13:17 ` [PATCH v3 2/7] misc: minimal mux subsystem and gpio-based mux controller Peter Rosin
@ 2016-11-21 13:17 ` Peter Rosin
  2016-11-21 13:17 ` [PATCH v3 4/7] dt-bindings: iio: iio-mux: document iio-mux bindings Peter Rosin
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Peter Rosin @ 2016-11-21 13:17 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, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

Extend the inkern api with functions for reading and writing ext_info
of iio channels.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/iio/inkern.c         | 55 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/consumer.h | 37 +++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c4757e6367e7..efe418282cc3 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -746,3 +746,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 9edccfba1ffb..1e908ccb11f0 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -271,4 +271,41 @@ 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);
 
+/**
+ * iio_get_channel_ext_info_count() - get number of ext_info attributes
+ *				      connected to the channel.
+ * @chan:		The channel being queried
+ *
+ * Returns the number of ext_info attributes
+ */
+int iio_get_channel_ext_info_count(struct iio_channel *chan);
+
+/**
+ * iio_read_channel_ext_info() - read ext_info attribute from a given channel
+ * @chan:		The channel being queried.
+ * @attr:		The ext_info attribute to read.
+ * @buf:		Where to store the attribute value. Assumed to hold
+ *			at least PAGE_SIZE bytes.
+ *
+ * Returns the number of bytes written to buf (perhaps w/o zero termination;
+ * it need not even be a string), or an error code.
+ */
+ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
+				  const char *attr, char *buf);
+
+/**
+ * iio_write_channel_ext_info() - write ext_info attribute from a given channel
+ * @chan:		The channel being queried.
+ * @attr:		The ext_info attribute to read.
+ * @buf:		The new attribute value. Strings needs to be zero-
+ *			terminated, but the terminator should not be included
+ *			in the below len.
+ * @len:		The size of the new attribute value.
+ *
+ * Returns the number of accepted bytes, which should be the same as len.
+ * An error code can also be returned.
+ */
+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] 14+ messages in thread

* [PATCH v3 4/7] dt-bindings: iio: iio-mux: document iio-mux bindings
  2016-11-21 13:17 [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (2 preceding siblings ...)
  2016-11-21 13:17 ` [PATCH v3 3/7] iio: inkern: api for manipulating ext_info of iio channels Peter Rosin
@ 2016-11-21 13:17 ` Peter Rosin
  2016-11-21 13:17 ` [PATCH v3 5/7] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Peter Rosin @ 2016-11-21 13:17 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, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../bindings/iio/multiplexer/iio-mux.txt           | 47 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 +++
 2 files changed, 53 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..2807333ccc86
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
@@ -0,0 +1,47 @@
+IIO multiplexer bindings
+
+If a multiplexer is used to select which 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".
+- #address-cells = <1>;
+- #size-cells = <0>;
+
+Required properties for iio-mux child nodes:
+- reg : The multiplexer state as described in ../misc/mux-controller.txt
+
+For each iio-mux child, an iio channel will be created whose number will
+match the mux controller state.
+
+Example:
+	mux {
+		compatible = "mux-gpio";
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+
+		adc {
+			compatible = "iio-mux";
+			io-channels = <&adc 0>;
+			io-channel-names = "parent";
+
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			sync@0 {
+				reg = <0>;
+			};
+
+			in@1 {
+				reg = <1>;
+			};
+
+			system-regulator@2 {
+				reg = <2>;
+			};
+		};
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 37974aeee750..3fc0dbbc04ea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6124,6 +6124,12 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	drivers/media/rc/iguanair.c
 
+IIO MULTIPLEXER
+M:	Peter Rosin <peda@axentia.se>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
+
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23@kernel.org>
 R:	Hartmut Knaack <knaack.h@gmx.de>
-- 
2.1.4

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

* [PATCH v3 5/7] iio: multiplexer: new iio category and iio-mux driver
  2016-11-21 13:17 [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (3 preceding siblings ...)
  2016-11-21 13:17 ` [PATCH v3 4/7] dt-bindings: iio: iio-mux: document iio-mux bindings Peter Rosin
@ 2016-11-21 13:17 ` Peter Rosin
  2016-11-21 14:33   ` kbuild test robot
  2016-11-21 13:17 ` [PATCH v3 6/7] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings Peter Rosin
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Peter Rosin @ 2016-11-21 13:17 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, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

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 subsystem.

Cache any ext_info values from the parent iio channel, creating a private
copy of the ext_info attributes for each multiplexer state/channel.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 MAINTAINERS                       |   1 +
 drivers/iio/Kconfig               |   1 +
 drivers/iio/Makefile              |   1 +
 drivers/iio/multiplexer/Kconfig   |  18 ++
 drivers/iio/multiplexer/Makefile  |   6 +
 drivers/iio/multiplexer/iio-mux.c | 450 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 477 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/MAINTAINERS b/MAINTAINERS
index 3fc0dbbc04ea..02d9072aae16 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6129,6 +6129,7 @@ M:	Peter Rosin <peda@axentia.se>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
+F:	drivers/iio/multiplexer/iio-mux.c
 
 IIO SUBSYSTEM AND DRIVERS
 M:	Jonathan Cameron <jic23@kernel.org>
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..70a044510686
--- /dev/null
+++ b/drivers/iio/multiplexer/Kconfig
@@ -0,0 +1,18 @@
+#
+# Multiplexer drivers
+#
+# When adding new entries keep the list in alphabetical order
+
+menu "Multiplexers"
+
+config IIO_MUX
+	tristate "IIO multiplexer driver"
+	select MULTIPLEXER
+	depends on OF
+	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..0b9d04914241
--- /dev/null
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -0,0 +1,450 @@
+/*
+ * 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 {
+	struct mux_ext_info_cache *ext_info_cache;
+};
+
+struct mux {
+	int cached_state;
+	struct mux_control *control;
+	struct iio_channel *parent;
+	struct iio_dev *indio_dev;
+	struct iio_chan_spec *chan;
+	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];
+	struct iio_chan_spec const *chan = &mux->chan[idx];
+	int ret;
+	int i;
+
+	ret = mux_control_select(mux->control, chan->channel);
+	if (ret < 0) {
+		mux->cached_state = -1;
+		return ret;
+	}
+
+	if (mux->cached_state == chan->channel)
+		return 0;
+
+	if (chan->ext_info) {
+		for (i = 0; chan->ext_info[i].name; ++i) {
+			const char *attr = chan->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 = chan->channel;
+
+	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->chan;
+	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->chan;
+	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->chan;
+	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->chan;
+	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->chan;
+	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 *chan = &mux->chan[idx];
+	struct iio_chan_spec const *pchan = mux->parent->channel;
+	u32 state;
+	char *page = NULL;
+	int num_ext_info;
+	int i;
+	int ret;
+
+	chan->indexed = 1;
+	chan->output = pchan->output;
+	chan->datasheet_name = child_np->name;
+	chan->ext_info = mux->ext_info;
+
+	ret = iio_get_channel_type(mux->parent, &chan->type);
+	if (ret < 0) {
+		dev_err(dev, "failed to get parent channel type\n");
+		return ret;
+	}
+
+	if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW))
+		chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW);
+	if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE))
+		chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE);
+
+	if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW))
+		chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
+
+	ret = of_property_read_u32(child_np, "reg", &state);
+	if (ret < 0) {
+		dev_err(dev, "no reg property for node '%s'\n", child_np->name);
+		return ret;
+	}
+
+	if (state >= mux->control->states) {
+		dev_err(dev, "invalid reg %u\n", state);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < idx; ++i) {
+		if (mux->chan[i].channel == state) {
+			dev_err(dev, "double use of reg %u\n", state);
+			return -EINVAL;
+		}
+	}
+
+	chan->channel = state;
+
+	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 (!pchan->ext_info[i].write)
+			continue;
+		if (!pchan->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",
+				pchan->ext_info[i].name);
+			return ret;
+		}
+		if (ret >= PAGE_SIZE) {
+			dev_err(dev, "too large ext_info '%s'\n",
+				pchan->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 (page)
+		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->chan) * 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->chan = (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->chan;
+	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;
+		}
+	}
+
+	mux->control = devm_mux_control_get(dev);
+	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);
+	}
+
+	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++;
+	}
+
+	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] 14+ messages in thread

* [PATCH v3 6/7] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings
  2016-11-21 13:17 [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (4 preceding siblings ...)
  2016-11-21 13:17 ` [PATCH v3 5/7] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
@ 2016-11-21 13:17 ` Peter Rosin
  2016-11-21 13:17 ` [PATCH v3 7/7] i2c: i2c-mux-simple: new driver Peter Rosin
  2016-11-22 20:58 ` [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Lars-Peter Clausen
  7 siblings, 0 replies; 14+ messages in thread
From: Peter Rosin @ 2016-11-21 13:17 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, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/i2c/i2c-mux-simple.txt     | 76 ++++++++++++++++++++++
 1 file changed, 76 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..decfd742b297
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
@@ -0,0 +1,76 @@
+Simple I2C Bus Mux
+
+This binding describes an I2C bus multiplexer that uses a mux controller
+from the mux subsystem 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.
+* Standard I2C mux properties. See i2c-mux.txt in this directory.
+* I2C child bus nodes. See i2c-mux.txt in this directory. The sub-bus number
+  is also the mux-controller state described in ../misc/mux-controller.txt
+
+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 set as the state in the
+mux controller.
+
+Example:
+	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>;
+
+			#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] 14+ messages in thread

* [PATCH v3 7/7] i2c: i2c-mux-simple: new driver
  2016-11-21 13:17 [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (5 preceding siblings ...)
  2016-11-21 13:17 ` [PATCH v3 6/7] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings Peter Rosin
@ 2016-11-21 13:17 ` Peter Rosin
  2016-11-22 20:58 ` [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Lars-Peter Clausen
  7 siblings, 0 replies; 14+ messages in thread
From: Peter Rosin @ 2016-11-21 13:17 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, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

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.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/Kconfig          |  13 +++
 drivers/i2c/muxes/Makefile         |   1 +
 drivers/i2c/muxes/i2c-mux-simple.c | 177 +++++++++++++++++++++++++++++++++++++
 3 files changed, 191 insertions(+)
 create mode 100644 drivers/i2c/muxes/i2c-mux-simple.c

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 96de9ce5669b..fb4f587d822a 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -73,6 +73,19 @@ 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"
+	select 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..a2f1dc8a3295
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-simple.c
@@ -0,0 +1,177 @@
+/*
+ * 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);
+	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) {
+			dev_err(dev, "no reg property for node '%s'\n",
+				child->name);
+			goto err_children;
+		}
+
+		if (chan >= mux->control->states) {
+			dev_err(dev, "invalid reg %u\n", chan);
+			ret = -EINVAL;
+			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] 14+ messages in thread

* Re: [PATCH v3 5/7] iio: multiplexer: new iio category and iio-mux driver
  2016-11-21 13:17 ` [PATCH v3 5/7] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
@ 2016-11-21 14:33   ` kbuild test robot
  0 siblings, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2016-11-21 14:33 UTC (permalink / raw)
  To: Peter Rosin
  Cc: kbuild-all, linux-kernel, Peter Rosin, Wolfram Sang, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	Arnd Bergmann, Greg Kroah-Hartman, linux-i2c, devicetree,
	linux-iio, linux-doc

[-- Attachment #1: Type: text/plain, Size: 6412 bytes --]

Hi Peter,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.9-rc6]
[cannot apply to next-20161117]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Peter-Rosin/mux-controller-abstraction-and-iio-i2c-muxes/20161121-215311
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/iio/multiplexer/iio-mux.c: In function 'mux_read_avail':
>> drivers/iio/multiplexer/iio-mux.c:134:9: error: implicit declaration of function 'iio_read_avail_channel_raw' [-Werror=implicit-function-declaration]
      ret = iio_read_avail_channel_raw(mux->parent, vals, length);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/multiplexer/iio-mux.c: At top level:
>> drivers/iio/multiplexer/iio-mux.c:174:2: error: unknown field 'read_avail' specified in initializer
     .read_avail = mux_read_avail,
     ^
>> drivers/iio/multiplexer/iio-mux.c:174:16: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
     .read_avail = mux_read_avail,
                   ^~~~~~~~~~~~~~
   drivers/iio/multiplexer/iio-mux.c:174:16: note: (near initialization for 'mux_info.read_raw_multi')
   drivers/iio/multiplexer/iio-mux.c: In function 'mux_configure_channel':
>> drivers/iio/multiplexer/iio-mux.c:267:6: error: implicit declaration of function 'iio_channel_has_available' [-Werror=implicit-function-declaration]
     if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW))
         ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iio/multiplexer/iio-mux.c:268:7: error: 'struct iio_chan_spec' has no member named 'info_mask_separate_available'; did you mean 'info_mask_separate'?
      chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
          ^~
   cc1: some warnings being treated as errors

vim +/iio_read_avail_channel_raw +134 drivers/iio/multiplexer/iio-mux.c

   128		if (ret < 0)
   129			return ret;
   130	
   131		switch (mask) {
   132		case IIO_CHAN_INFO_RAW:
   133			*type = IIO_VAL_INT;
 > 134			ret = iio_read_avail_channel_raw(mux->parent, vals, length);
   135			break;
   136	
   137		default:
   138			ret = -EINVAL;
   139		}
   140	
   141		iio_mux_deselect(mux);
   142	
   143		return ret;
   144	}
   145	
   146	static int mux_write_raw(struct iio_dev *indio_dev,
   147				 struct iio_chan_spec const *chan,
   148				 int val, int val2, long mask)
   149	{
   150		struct mux *mux = iio_priv(indio_dev);
   151		int idx = chan - mux->chan;
   152		int ret;
   153	
   154		ret = iio_mux_select(mux, idx);
   155		if (ret < 0)
   156			return ret;
   157	
   158		switch (mask) {
   159		case IIO_CHAN_INFO_RAW:
   160			ret = iio_write_channel_raw(mux->parent, val);
   161			break;
   162	
   163		default:
   164			ret = -EINVAL;
   165		}
   166	
   167		iio_mux_deselect(mux);
   168	
   169		return ret;
   170	}
   171	
   172	static const struct iio_info mux_info = {
   173		.read_raw = mux_read_raw,
 > 174		.read_avail = mux_read_avail,
   175		.write_raw = mux_write_raw,
   176		.driver_module = THIS_MODULE,
   177	};
   178	
   179	static ssize_t mux_read_ext_info(struct iio_dev *indio_dev, uintptr_t private,
   180					 struct iio_chan_spec const *chan, char *buf)
   181	{
   182		struct mux *mux = iio_priv(indio_dev);
   183		int idx = chan - mux->chan;
   184		ssize_t ret;
   185	
   186		ret = iio_mux_select(mux, idx);
   187		if (ret < 0)
   188			return ret;
   189	
   190		ret = iio_read_channel_ext_info(mux->parent,
   191						mux->ext_info[private].name,
   192						buf);
   193	
   194		iio_mux_deselect(mux);
   195	
   196		return ret;
   197	}
   198	
   199	static ssize_t mux_write_ext_info(struct iio_dev *indio_dev, uintptr_t private,
   200					  struct iio_chan_spec const *chan,
   201					  const char *buf, size_t len)
   202	{
   203		struct device *dev = indio_dev->dev.parent;
   204		struct mux *mux = iio_priv(indio_dev);
   205		int idx = chan - mux->chan;
   206		char *new;
   207		ssize_t ret;
   208	
   209		ret = iio_mux_select(mux, idx);
   210		if (ret < 0)
   211			return ret;
   212	
   213		new = devm_kmemdup(dev, buf, len + 1, GFP_KERNEL);
   214		if (!new) {
   215			iio_mux_deselect(mux);
   216			return -ENOMEM;
   217		}
   218	
   219		new[len] = 0;
   220	
   221		ret = iio_write_channel_ext_info(mux->parent,
   222						 mux->ext_info[private].name,
   223						 buf, len);
   224		if (ret < 0) {
   225			iio_mux_deselect(mux);
   226			devm_kfree(dev, new);
   227			return ret;
   228		}
   229	
   230		devm_kfree(dev, mux->child[idx].ext_info_cache[private].data);
   231		mux->child[idx].ext_info_cache[private].data = new;
   232		mux->child[idx].ext_info_cache[private].size = len;
   233	
   234		iio_mux_deselect(mux);
   235	
   236		return ret;
   237	}
   238	
   239	static int mux_configure_channel(struct device *dev, struct mux *mux,
   240					 struct device_node *child_np, int idx)
   241	{
   242		struct mux_child *child = &mux->child[idx];
   243		struct iio_chan_spec *chan = &mux->chan[idx];
   244		struct iio_chan_spec const *pchan = mux->parent->channel;
   245		u32 state;
   246		char *page = NULL;
   247		int num_ext_info;
   248		int i;
   249		int ret;
   250	
   251		chan->indexed = 1;
   252		chan->output = pchan->output;
   253		chan->datasheet_name = child_np->name;
   254		chan->ext_info = mux->ext_info;
   255	
   256		ret = iio_get_channel_type(mux->parent, &chan->type);
   257		if (ret < 0) {
   258			dev_err(dev, "failed to get parent channel type\n");
   259			return ret;
   260		}
   261	
   262		if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW))
   263			chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW);
   264		if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE))
   265			chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE);
   266	
 > 267		if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW))
 > 268			chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
   269	
   270		ret = of_property_read_u32(child_np, "reg", &state);
   271		if (ret < 0) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56692 bytes --]

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

* Re: [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes
  2016-11-21 13:17 [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
                   ` (6 preceding siblings ...)
  2016-11-21 13:17 ` [PATCH v3 7/7] i2c: i2c-mux-simple: new driver Peter Rosin
@ 2016-11-22 20:58 ` Lars-Peter Clausen
  2016-11-23 11:47   ` Peter Rosin
  7 siblings, 1 reply; 14+ messages in thread
From: Lars-Peter Clausen @ 2016-11-22 20:58 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, Jonathan Corbet,
	Arnd Bergmann, Greg Kroah-Hartman, linux-i2c, devicetree,
	linux-iio, linux-doc

On 11/21/2016 02:17 PM, Peter Rosin wrote:
[...]
> 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 (and subsystem) 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).

While abstracting this properly is all nice and good and the way it should
be done, but it also adds a lot of complexity and the devicetree adds a lot
of restrictions on what can actually be represented.

There is a certain point where the fabric on a PCB becomes so complex that
it deserves to be a device on its own (like the audio fabric drivers).
Especially when the hardware is built with a certain application in mind and
the driver is supposed to impose policy which reflects this application. The
latter can often not properly be described with the primitives the
devicetree can offer.

And I think your setup is very borderline what can be done in a declarative
way only and it adds a lot of complexity over a more imperative solution in
form of a driver. I think it is worth investigating about having a driver
that is specific to your fabric and handles the interdependencies of the
discrete components.

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

* Re: [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes
  2016-11-22 20:58 ` [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Lars-Peter Clausen
@ 2016-11-23 11:47   ` Peter Rosin
  2016-11-27 12:00     ` Jonathan Cameron
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Rosin @ 2016-11-23 11:47 UTC (permalink / raw)
  To: Lars-Peter Clausen, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, Jonathan Corbet,
	Arnd Bergmann, Greg Kroah-Hartman, linux-i2c, devicetree,
	linux-iio, linux-doc

On 2016-11-22 21:58, Lars-Peter Clausen wrote:
> On 11/21/2016 02:17 PM, Peter Rosin wrote:
> [...]
>> 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 (and subsystem) 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).
> 
> While abstracting this properly is all nice and good and the way it should
> be done, but it also adds a lot of complexity and the devicetree adds a lot
> of restrictions on what can actually be represented.

This is a characterization without any specifics. But is the
characterization true? You have two complaints, complexity
and restrictions with bindings.

> There is a certain point where the fabric on a PCB becomes so complex that
> it deserves to be a device on its own (like the audio fabric drivers).
> Especially when the hardware is built with a certain application in mind and
> the driver is supposed to impose policy which reflects this application. The
> latter can often not properly be described with the primitives the
> devicetree can offer.
> 
> And I think your setup is very borderline what can be done in a declarative
> way only and it adds a lot of complexity over a more imperative solution in
> form of a driver. I think it is worth investigating about having a driver
> that is specific to your fabric and handles the interdependencies of the
> discrete components.

So, there are three "new" concepts:

1. Sticking a mux in front of an AD-converter. That's not all that
novel, nor complex. Quite the opposite, I'd say. In fact, I find it
a bit amazing that there is no in-kernel support for it.

2. Reusing the same GPIO-pins to drive different muxes. There are
obviously chips that work this way (as Jonathan pointed out) and
these will at some point get used in Linux devices. I guess they
already are used, but that people handle them in userspace. Or
something? If this is complex, which I question, it will still need
support at some point. At least that's what I believe.

3. Using the same GPIO pins to mux things handled by different
subsystems. Right, this is a bit crazy, and I'd rather not have this
requirement, but this HW is what it is so I'll need to handle it in
some way. It is also what stops me from falling back to a userspace
solution, which is probably connected to why #1 and #2 is not supported
by the kernel; everybody probably does muxing in userspace. Which is
not necessarily a good idea, nor how it's supposed to be done...

So, the only thing that's out of the ordinary (as I see it), is #3.
The question that then needs an answer is how the in-kernel solution
for #1 and #2 would look if we do not consider #3.

And I claim that the desired solution to #1 and #2 is pretty close
to my proposal.

A. You do not want mux-controller drivers in every subsystem that
needs them.

B. You do not want every mux-consumer to know the specifics of how to
operate every kind of mux; there are muxes that are not controlled
with GPIO pins...

C. When you implement muxing in a subsystem, there will in some cases
be a need to handle parallel muxing, where there is a need to share
mux-controllers.

It just feels right to separate out the mux-controller and refer to
it from where a mux is needed. It solves #1 and #2. And, of course,
as a bonus #3 is also solved. But my bias is obvious.

And that leads us to the restrictions with the bindings. And the same
thing happens; the solution for #2 also solves #3.

So how do you refer to a mux-controller from where it's needed? My
first proposal used a dt phandle, for the second round I put them in
the parent node. It would be super if it was possible for the mux-
consumer to create the mux-controller device from the same dt
node that is already bound to the mux-consumer. The problem is that
the mux-consumer should not hard-code which mux-controller device it
should create. The best I can think of is some kind of 'mux-compatible'
attribute, that works like the standard 'compatible' attribute. That
would simplify the bindings for the normal case (#1) where the mux-
controller isn't shared (#2 and #3). Maybe it's possible to fix this
issue somehow? I simply don't know?

Cheers,
Peter

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

* Re: [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes
  2016-11-23 11:47   ` Peter Rosin
@ 2016-11-27 12:00     ` Jonathan Cameron
  2016-11-29 16:02       ` Peter Rosin
  0 siblings, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2016-11-27 12:00 UTC (permalink / raw)
  To: Peter Rosin, Lars-Peter Clausen, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

On 23/11/16 11:47, Peter Rosin wrote:
> On 2016-11-22 21:58, Lars-Peter Clausen wrote:
>> On 11/21/2016 02:17 PM, Peter Rosin wrote:
>> [...]
>>> 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 (and subsystem) 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).
>>
>> While abstracting this properly is all nice and good and the way it should
>> be done, but it also adds a lot of complexity and the devicetree adds a lot
>> of restrictions on what can actually be represented.
> 
> This is a characterization without any specifics. But is the
> characterization true? You have two complaints, complexity
> and restrictions with bindings.
> 
>> There is a certain point where the fabric on a PCB becomes so complex that
>> it deserves to be a device on its own (like the audio fabric drivers).
>> Especially when the hardware is built with a certain application in mind and
>> the driver is supposed to impose policy which reflects this application. The
>> latter can often not properly be described with the primitives the
>> devicetree can offer.
>>
>> And I think your setup is very borderline what can be done in a declarative
>> way only and it adds a lot of complexity over a more imperative solution in
>> form of a driver. I think it is worth investigating about having a driver
>> that is specific to your fabric and handles the interdependencies of the
>> discrete components.
> 
> So, there are three "new" concepts:
> 
> 1. Sticking a mux in front of an AD-converter. That's not all that
> novel, nor complex. Quite the opposite, I'd say. In fact, I find it
> a bit amazing that there is no in-kernel support for it.
As ever first person who needs it and has the skills to write it gets to do it ;)
Congratulations Peter ;)
> 
> 2. Reusing the same GPIO-pins to drive different muxes. There are
> obviously chips that work this way (as Jonathan pointed out) and
> these will at some point get used in Linux devices. I guess they
> already are used, but that people handle them in userspace. Or
> something? If this is complex, which I question, it will still need
> support at some point. At least that's what I believe.
> 
> 3. Using the same GPIO pins to mux things handled by different
> subsystems. Right, this is a bit crazy, and I'd rather not have this
> requirement, but this HW is what it is so I'll need to handle it in
> some way. It is also what stops me from falling back to a userspace
> solution, which is probably connected to why #1 and #2 is not supported
> by the kernel; everybody probably does muxing in userspace. Which is
> not necessarily a good idea, nor how it's supposed to be done...
> 
> So, the only thing that's out of the ordinary (as I see it), is #3.
> The question that then needs an answer is how the in-kernel solution
> for #1 and #2 would look if we do not consider #3.
> 
> And I claim that the desired solution to #1 and #2 is pretty close
> to my proposal.
> 
> A. You do not want mux-controller drivers in every subsystem that
> needs them.
Agreed.
> 
> B. You do not want every mux-consumer to know the specifics of how to
> operate every kind of mux; there are muxes that are not controlled
> with GPIO pins...
> 
> C. When you implement muxing in a subsystem, there will in some cases
> be a need to handle parallel muxing, where there is a need to share
> mux-controllers.
> 
> It just feels right to separate out the mux-controller and refer to
> it from where a mux is needed. It solves #1 and #2. And, of course,
> as a bonus #3 is also solved. But my bias is obvious.
> 
> And that leads us to the restrictions with the bindings. And the same
> thing happens; the solution for #2 also solves #3.
> 
> So how do you refer to a mux-controller from where it's needed? My
> first proposal used a dt phandle, for the second round I put them in
> the parent node. It would be super if it was possible for the mux-
> consumer to create the mux-controller device from the same dt
> node that is already bound to the mux-consumer. The problem is that
> the mux-consumer should not hard-code which mux-controller device it
> should create. The best I can think of is some kind of 'mux-compatible'
> attribute, that works like the standard 'compatible' attribute. That
> would simplify the bindings for the normal case (#1) where the mux-
> controller isn't shared (#2 and #3). Maybe it's possible to fix this
> issue somehow? I simply don't know?
As Lars stated, it's marginal.  The question is not at what point do we
'have to' bother with a fabric driver, but rather at what point does it
make a our lives easier.

Take you nastiest mux case described earlier.
The ideal would be to represent the ADC and 3 muxes as (approximately) a
single ADC to userspace that just happens to have somewhere near 23 inputs.

To do that in device tree we need to describe

1 The adc
2 The three muxes
3 The software representation to pull all of these back into a single device.

That last part to my mind trips the balance to the point where a fabric driver
would make sense.  It's not complex.  Just a few lines of code tying all the
elements together without ending up with a fairly fiendish setup to describe in
device tree.

Also just wait until we have muxes stacked on muxes, with cross overs occuring.
Some of the ASoC parts can actually have effective loops if you try all the mux
combinations.

So question is do we have a 'simple case description' in device tree or force
fabric drivers everywhere?  I think I'm in favour of the simple case - which handles
one of your two uses nicely.  The second one to do the the recombining of channels after
the muxes, ends up looking to me like it needs a fabric driver.

Note we are only talking about bindings vs code based description here. I agree
entirely with the concept of a generic mux subsystem.

Jonathan

> 
> Cheers,
> Peter
> 

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

* Re: [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes
  2016-11-27 12:00     ` Jonathan Cameron
@ 2016-11-29 16:02       ` Peter Rosin
  2016-12-03 10:11         ` Jonathan Cameron
  0 siblings, 1 reply; 14+ messages in thread
From: Peter Rosin @ 2016-11-29 16:02 UTC (permalink / raw)
  To: Jonathan Cameron, Lars-Peter Clausen, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

On 2016-11-27 13:00, Jonathan Cameron wrote:
> On 23/11/16 11:47, Peter Rosin wrote:
>> On 2016-11-22 21:58, Lars-Peter Clausen wrote:
>>> On 11/21/2016 02:17 PM, Peter Rosin wrote:
>>> [...]
>>>> 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 (and subsystem) 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).
>>>
>>> While abstracting this properly is all nice and good and the way it should
>>> be done, but it also adds a lot of complexity and the devicetree adds a lot
>>> of restrictions on what can actually be represented.
>>
>> This is a characterization without any specifics. But is the
>> characterization true? You have two complaints, complexity
>> and restrictions with bindings.
>>
>>> There is a certain point where the fabric on a PCB becomes so complex that
>>> it deserves to be a device on its own (like the audio fabric drivers).
>>> Especially when the hardware is built with a certain application in mind and
>>> the driver is supposed to impose policy which reflects this application. The
>>> latter can often not properly be described with the primitives the
>>> devicetree can offer.
>>>
>>> And I think your setup is very borderline what can be done in a declarative
>>> way only and it adds a lot of complexity over a more imperative solution in
>>> form of a driver. I think it is worth investigating about having a driver
>>> that is specific to your fabric and handles the interdependencies of the
>>> discrete components.
>>
>> So, there are three "new" concepts:
>>
>> 1. Sticking a mux in front of an AD-converter. That's not all that
>> novel, nor complex. Quite the opposite, I'd say. In fact, I find it
>> a bit amazing that there is no in-kernel support for it.
> As ever first person who needs it and has the skills to write it gets to do it ;)
> Congratulations Peter ;)
>>
>> 2. Reusing the same GPIO-pins to drive different muxes. There are
>> obviously chips that work this way (as Jonathan pointed out) and
>> these will at some point get used in Linux devices. I guess they
>> already are used, but that people handle them in userspace. Or
>> something? If this is complex, which I question, it will still need
>> support at some point. At least that's what I believe.
>>
>> 3. Using the same GPIO pins to mux things handled by different
>> subsystems. Right, this is a bit crazy, and I'd rather not have this
>> requirement, but this HW is what it is so I'll need to handle it in
>> some way. It is also what stops me from falling back to a userspace
>> solution, which is probably connected to why #1 and #2 is not supported
>> by the kernel; everybody probably does muxing in userspace. Which is
>> not necessarily a good idea, nor how it's supposed to be done...
>>
>> So, the only thing that's out of the ordinary (as I see it), is #3.
>> The question that then needs an answer is how the in-kernel solution
>> for #1 and #2 would look if we do not consider #3.
>>
>> And I claim that the desired solution to #1 and #2 is pretty close
>> to my proposal.
>>
>> A. You do not want mux-controller drivers in every subsystem that
>> needs them.
> Agreed.
>>
>> B. You do not want every mux-consumer to know the specifics of how to
>> operate every kind of mux; there are muxes that are not controlled
>> with GPIO pins...
>>
>> C. When you implement muxing in a subsystem, there will in some cases
>> be a need to handle parallel muxing, where there is a need to share
>> mux-controllers.
>>
>> It just feels right to separate out the mux-controller and refer to
>> it from where a mux is needed. It solves #1 and #2. And, of course,
>> as a bonus #3 is also solved. But my bias is obvious.
>>
>> And that leads us to the restrictions with the bindings. And the same
>> thing happens; the solution for #2 also solves #3.
>>
>> So how do you refer to a mux-controller from where it's needed? My
>> first proposal used a dt phandle, for the second round I put them in
>> the parent node. It would be super if it was possible for the mux-
>> consumer to create the mux-controller device from the same dt
>> node that is already bound to the mux-consumer. The problem is that
>> the mux-consumer should not hard-code which mux-controller device it
>> should create. The best I can think of is some kind of 'mux-compatible'
>> attribute, that works like the standard 'compatible' attribute. That
>> would simplify the bindings for the normal case (#1) where the mux-
>> controller isn't shared (#2 and #3). Maybe it's possible to fix this
>> issue somehow? I simply don't know?
> As Lars stated, it's marginal.  The question is not at what point do we
> 'have to' bother with a fabric driver, but rather at what point does it
> make a our lives easier.
> 
> Take you nastiest mux case described earlier.
> The ideal would be to represent the ADC and 3 muxes as (approximately) a
> single ADC to userspace that just happens to have somewhere near 23 inputs.
> 
> To do that in device tree we need to describe
> 
> 1 The adc
> 2 The three muxes
> 3 The software representation to pull all of these back into a single device.
> 
> That last part to my mind trips the balance to the point where a fabric driver
> would make sense.  It's not complex.  Just a few lines of code tying all the
> elements together without ending up with a fairly fiendish setup to describe in
> device tree.
> 
> Also just wait until we have muxes stacked on muxes, with cross overs occuring.
> Some of the ASoC parts can actually have effective loops if you try all the mux
> combinations.
> 
> So question is do we have a 'simple case description' in device tree or force
> fabric drivers everywhere?  I think I'm in favour of the simple case - which handles
> one of your two uses nicely.  The second one to do the the recombining of channels after
> the muxes, ends up looking to me like it needs a fabric driver.
> 
> Note we are only talking about bindings vs code based description here. I agree
> entirely with the concept of a generic mux subsystem.

Ok, take the simple case - an adc with a mux in front of it.

We do not want to build a whole new iio-mux subsystem like the one under
i2c, so from iio we want to refer to the actual mux controller driver
which lives under the mux subsystem. Getting this far is what solves my
"second need" [2] from the v2 thread.

Agreed, doing this w/o a fabric driver spills the guts and it might be
cleaner to build a fabric driver that takes a reference to the dpot and
the mux controller and just knows the rest. In the end this fabric driver
requires two things to actually make a difference; some way to instantiate
drivers without the help of device-tree and some way make those drivers
only provide in-kernel access (and preferably it should hide the dpot from
userspace too, while at it).

But doing all that in a fabric driver is not going to change the fact that
the iio-mux driver is useful on its own. I bet someone else is going to
reuse it somewhere down the line. Which means that a fabric driver would
perhaps be nice for my "second need", but not critical, it works pretty
well to just piggyback on the general solution .

Over to my "first need" [1] from the v2 thread.

The above iio-mux driver handles the three muxed adc lines beautifully,
just refer all three iio-muxes to the same mux controller. Agreed, with
a fabric driver I could get one device with 25 channels instead of three
devices with 8 channels each plus one unmuxed line directly from the adc
device. However, that doesn't bother me at all, I may even think it is
preferable because otherwise I'd have to come up with some way to
identify which channel is which in that big 25-channel jungle. Another
drawback with having a fabric driver here is that it would need to be an
i2c-mux driver, because one of the key points of doing a fabric driver
for my first need was to hide the shared mux, right? Instead, I have the
new i2c-mux-simple driver that builds an i2c-mux using any mux controller
from the mux subsystem (something that may prove useful to others in the
future), which in my case is the same mux controller that also muxes the
three adc lines.

In short, doing fabric drivers buys me almost nothing besides having a
slightly more distinct device tree. All the components used to describe
this are either already accepted drivers or usable by others (at least
the way I see it).

Cheers,
Peter

[1] three adc lines and an i2c bus muxed with the same gpio pins
[2] mcp4561 dpot -> dpot-dac -> envelope-detector-adc -> iio-mux

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

* Re: [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes
  2016-11-29 16:02       ` Peter Rosin
@ 2016-12-03 10:11         ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2016-12-03 10:11 UTC (permalink / raw)
  To: Peter Rosin, Lars-Peter Clausen, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
	Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc

On 29/11/16 16:02, Peter Rosin wrote:
> On 2016-11-27 13:00, Jonathan Cameron wrote:
>> On 23/11/16 11:47, Peter Rosin wrote:
>>> On 2016-11-22 21:58, Lars-Peter Clausen wrote:
>>>> On 11/21/2016 02:17 PM, Peter Rosin wrote:
>>>> [...]
>>>>> 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 (and subsystem) 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).
>>>>
>>>> While abstracting this properly is all nice and good and the way it should
>>>> be done, but it also adds a lot of complexity and the devicetree adds a lot
>>>> of restrictions on what can actually be represented.
>>>
>>> This is a characterization without any specifics. But is the
>>> characterization true? You have two complaints, complexity
>>> and restrictions with bindings.
>>>
>>>> There is a certain point where the fabric on a PCB becomes so complex that
>>>> it deserves to be a device on its own (like the audio fabric drivers).
>>>> Especially when the hardware is built with a certain application in mind and
>>>> the driver is supposed to impose policy which reflects this application. The
>>>> latter can often not properly be described with the primitives the
>>>> devicetree can offer.
>>>>
>>>> And I think your setup is very borderline what can be done in a declarative
>>>> way only and it adds a lot of complexity over a more imperative solution in
>>>> form of a driver. I think it is worth investigating about having a driver
>>>> that is specific to your fabric and handles the interdependencies of the
>>>> discrete components.
>>>
>>> So, there are three "new" concepts:
>>>
>>> 1. Sticking a mux in front of an AD-converter. That's not all that
>>> novel, nor complex. Quite the opposite, I'd say. In fact, I find it
>>> a bit amazing that there is no in-kernel support for it.
>> As ever first person who needs it and has the skills to write it gets to do it ;)
>> Congratulations Peter ;)
>>>
>>> 2. Reusing the same GPIO-pins to drive different muxes. There are
>>> obviously chips that work this way (as Jonathan pointed out) and
>>> these will at some point get used in Linux devices. I guess they
>>> already are used, but that people handle them in userspace. Or
>>> something? If this is complex, which I question, it will still need
>>> support at some point. At least that's what I believe.
>>>
>>> 3. Using the same GPIO pins to mux things handled by different
>>> subsystems. Right, this is a bit crazy, and I'd rather not have this
>>> requirement, but this HW is what it is so I'll need to handle it in
>>> some way. It is also what stops me from falling back to a userspace
>>> solution, which is probably connected to why #1 and #2 is not supported
>>> by the kernel; everybody probably does muxing in userspace. Which is
>>> not necessarily a good idea, nor how it's supposed to be done...
>>>
>>> So, the only thing that's out of the ordinary (as I see it), is #3.
>>> The question that then needs an answer is how the in-kernel solution
>>> for #1 and #2 would look if we do not consider #3.
>>>
>>> And I claim that the desired solution to #1 and #2 is pretty close
>>> to my proposal.
>>>
>>> A. You do not want mux-controller drivers in every subsystem that
>>> needs them.
>> Agreed.
>>>
>>> B. You do not want every mux-consumer to know the specifics of how to
>>> operate every kind of mux; there are muxes that are not controlled
>>> with GPIO pins...
>>>
>>> C. When you implement muxing in a subsystem, there will in some cases
>>> be a need to handle parallel muxing, where there is a need to share
>>> mux-controllers.
>>>
>>> It just feels right to separate out the mux-controller and refer to
>>> it from where a mux is needed. It solves #1 and #2. And, of course,
>>> as a bonus #3 is also solved. But my bias is obvious.
>>>
>>> And that leads us to the restrictions with the bindings. And the same
>>> thing happens; the solution for #2 also solves #3.
>>>
>>> So how do you refer to a mux-controller from where it's needed? My
>>> first proposal used a dt phandle, for the second round I put them in
>>> the parent node. It would be super if it was possible for the mux-
>>> consumer to create the mux-controller device from the same dt
>>> node that is already bound to the mux-consumer. The problem is that
>>> the mux-consumer should not hard-code which mux-controller device it
>>> should create. The best I can think of is some kind of 'mux-compatible'
>>> attribute, that works like the standard 'compatible' attribute. That
>>> would simplify the bindings for the normal case (#1) where the mux-
>>> controller isn't shared (#2 and #3). Maybe it's possible to fix this
>>> issue somehow? I simply don't know?
>> As Lars stated, it's marginal.  The question is not at what point do we
>> 'have to' bother with a fabric driver, but rather at what point does it
>> make a our lives easier.
>>
>> Take you nastiest mux case described earlier.
>> The ideal would be to represent the ADC and 3 muxes as (approximately) a
>> single ADC to userspace that just happens to have somewhere near 23 inputs.
>>
>> To do that in device tree we need to describe
>>
>> 1 The adc
>> 2 The three muxes
>> 3 The software representation to pull all of these back into a single device.
>>
>> That last part to my mind trips the balance to the point where a fabric driver
>> would make sense.  It's not complex.  Just a few lines of code tying all the
>> elements together without ending up with a fairly fiendish setup to describe in
>> device tree.
>>
>> Also just wait until we have muxes stacked on muxes, with cross overs occuring.
>> Some of the ASoC parts can actually have effective loops if you try all the mux
>> combinations.
>>
>> So question is do we have a 'simple case description' in device tree or force
>> fabric drivers everywhere?  I think I'm in favour of the simple case - which handles
>> one of your two uses nicely.  The second one to do the the recombining of channels after
>> the muxes, ends up looking to me like it needs a fabric driver.
>>
>> Note we are only talking about bindings vs code based description here. I agree
>> entirely with the concept of a generic mux subsystem.
> 
> Ok, take the simple case - an adc with a mux in front of it.
> 
> We do not want to build a whole new iio-mux subsystem like the one under
> i2c, so from iio we want to refer to the actual mux controller driver
> which lives under the mux subsystem. Getting this far is what solves my
> "second need" [2] from the v2 thread.
> 
> Agreed, doing this w/o a fabric driver spills the guts and it might be
> cleaner to build a fabric driver that takes a reference to the dpot and
> the mux controller and just knows the rest. In the end this fabric driver
> requires two things to actually make a difference; some way to instantiate
> drivers without the help of device-tree and some way make those drivers
> only provide in-kernel access (and preferably it should hide the dpot from
> userspace too, while at it).
> 
> But doing all that in a fabric driver is not going to change the fact that
> the iio-mux driver is useful on its own. I bet someone else is going to
> reuse it somewhere down the line. Which means that a fabric driver would
> perhaps be nice for my "second need", but not critical, it works pretty
> well to just piggyback on the general solution .
Absolutely. No denying the usefulness of having a nice little mux subsystem
with the resulting iio-mux driver.
> 
> Over to my "first need" [1] from the v2 thread.
> 
> The above iio-mux driver handles the three muxed adc lines beautifully,
> just refer all three iio-muxes to the same mux controller. Agreed, with
> a fabric driver I could get one device with 25 channels instead of three
> devices with 8 channels each plus one unmuxed line directly from the adc
> device. However, that doesn't bother me at all, I may even think it is
> preferable because otherwise I'd have to come up with some way to
> identify which channel is which in that big 25-channel jungle. Another
> drawback with having a fabric driver here is that it would need to be an
> i2c-mux driver, because one of the key points of doing a fabric driver
> for my first need was to hide the shared mux, right? Instead, I have the
> new i2c-mux-simple driver that builds an i2c-mux using any mux controller
> from the mux subsystem (something that may prove useful to others in the
> future), which in my case is the same mux controller that also muxes the
> three adc lines.
> 
> In short, doing fabric drivers buys me almost nothing besides having a
> slightly more distinct device tree. All the components used to describe
> this are either already accepted drivers or usable by others (at least
> the way I see it).
Fair enough.  Perhaps it's not worthwhile in this case, but lets keep
the concept in mind as we move forward and see if anyone else has
more complex hardware than you do ;)
(there is always something out there!)
> 
> Cheers,
> Peter
> 
> [1] three adc lines and an i2c bus muxed with the same gpio pins
> [2] mcp4561 dpot -> dpot-dac -> envelope-detector-adc -> iio-mux
> 

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

end of thread, other threads:[~2016-12-03 14:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-21 13:17 [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Peter Rosin
2016-11-21 13:17 ` [PATCH v3 1/7] dt-bindings: document devicetree bindings for mux-controllers and mux-gpio Peter Rosin
2016-11-21 13:17 ` [PATCH v3 2/7] misc: minimal mux subsystem and gpio-based mux controller Peter Rosin
2016-11-21 13:17 ` [PATCH v3 3/7] iio: inkern: api for manipulating ext_info of iio channels Peter Rosin
2016-11-21 13:17 ` [PATCH v3 4/7] dt-bindings: iio: iio-mux: document iio-mux bindings Peter Rosin
2016-11-21 13:17 ` [PATCH v3 5/7] iio: multiplexer: new iio category and iio-mux driver Peter Rosin
2016-11-21 14:33   ` kbuild test robot
2016-11-21 13:17 ` [PATCH v3 6/7] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings Peter Rosin
2016-11-21 13:17 ` [PATCH v3 7/7] i2c: i2c-mux-simple: new driver Peter Rosin
2016-11-22 20:58 ` [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes Lars-Peter Clausen
2016-11-23 11:47   ` Peter Rosin
2016-11-27 12:00     ` Jonathan Cameron
2016-11-29 16:02       ` Peter Rosin
2016-12-03 10:11         ` Jonathan Cameron

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).