linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates
@ 2016-08-15 13:40 Peter Rosin
  2016-08-15 13:40 ` [PATCH v2 1/8] dt-bindings: i2c: add support for 'i2c-mux' subnode Peter Rosin
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Peter Rosin @ 2016-08-15 13:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Guenter Roeck, David S. Miller, Geert Uytterhoeven,
	Andrew Morton, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-i2c, devicetree, Crestez Dan Leonard

Hi!

The nxp pca9541 chip does not have any devicetree bindings.
When trying to write such bindings matching the implementation,
I stumbled upon something which I did not like. I had to
give the node holding the i2c child bus a 'reg' property that
is left unused and is really odd to explain from a devicetree
point of view. It really is a leftover from the fact that linux
implements i2c arbitrators (and i2c gates) using the i2c mux
code. See this thread for history [1].

This series resolves the issues, and maintains backwards compat
with old device trees (well, one could, in theory, create a
device tree that would break with these changes, but my guess
is that the odds for that happening inadvertedly are high...)

This should also help Crestez Dan Leonard with the mpu6050
series containing the changes for its auxiliary i2c master [2],
especially if you also consider the recent (very similar) changes
from Jon Hunter that adds an optional 'i2c-bus' subnode [3].

If this is ok, I will follow up with patches for other drivers
so that they inform the i2c mux core if they are muxes, arbs or
gates. As stated, they will continue to work with these changes,
so there is no huge rush.

The mux core does not really need to differentiate between
arbitrators and gates. Should they be folded? What to call them
in that case?

Changes since v1:

- Fixes and additions suggested by Rob Herring (3/8 still needs an ack).
- Inserted patch 6/8 so that the two arb drivers support the new more
  compact dt syntax.

Cheers,
Peter

[1] https://lkml.org/lkml/2016/6/27/203
[2] https://lkml.org/lkml/2016/5/18/355
[3] https://patchwork.ozlabs.org/patch/641934/

Peter Rosin (8):
  dt-bindings: i2c: add support for 'i2c-mux' subnode
  dt-bindings: i2c: add support for 'i2c-arb' subnode
  dt-bindings: i2c: add support for 'i2c-gate' subnode
  dt-bindings: i2c: add bindings for nxp,pca9541
  i2c: mux: add support for 'i2c-mux', 'i2c-arb' and 'i2c-gate' DT
    subnodes
  i2c: mux: inform the i2c mux core about how it is used
  i2c: pca9541: add device tree binding
  i2c: pca954x: add device tree binding

 .../bindings/i2c/i2c-arb-gpio-challenge.txt        |  8 +---
 Documentation/devicetree/bindings/i2c/i2c-arb.txt  | 35 ++++++++++++++++
 Documentation/devicetree/bindings/i2c/i2c-gate.txt | 41 +++++++++++++++++++
 Documentation/devicetree/bindings/i2c/i2c-mux.txt  | 23 ++++++++---
 .../devicetree/bindings/i2c/nxp,pca9541.txt        | 29 ++++++++++++++
 MAINTAINERS                                        |  2 +
 drivers/i2c/i2c-mux.c                              | 44 +++++++++++++++++----
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c         |  2 +-
 drivers/i2c/muxes/i2c-mux-pca9541.c                | 11 +++++-
 drivers/i2c/muxes/i2c-mux-pca954x.c                | 46 ++++++++++++++++------
 include/linux/i2c-mux.h                            |  8 +++-
 11 files changed, 214 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-arb.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-gate.txt
 create mode 100644 Documentation/devicetree/bindings/i2c/nxp,pca9541.txt

-- 
2.1.4

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

* [PATCH v2 1/8] dt-bindings: i2c: add support for 'i2c-mux' subnode
  2016-08-15 13:40 [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Peter Rosin
@ 2016-08-15 13:40 ` Peter Rosin
  2016-08-15 13:40 ` [PATCH v2 2/8] dt-bindings: i2c: add support for 'i2c-arb' subnode Peter Rosin
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-08-15 13:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Guenter Roeck, David S. Miller, Geert Uytterhoeven,
	Andrew Morton, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-i2c, devicetree, Crestez Dan Leonard

Similar to the new optional 'i2c-bus' subnode from Jon Hunter, this
adds an optional 'i2c-mux' subnode, for similar reasons. I.e. it is
bad of the i2c mux core to assume that any subnode of an i2c mux device
is a potential (when the 'reg' property matches) i2c-mux child bus,
given that i2c mux devices might do more than mux i2c traffic.

So, if an 'i2c-mux' subnode is present, dictate that all i2c-mux child
buses exist beneath that subnode.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/devicetree/bindings/i2c/i2c-mux.txt | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux.txt b/Documentation/devicetree/bindings/i2c/i2c-mux.txt
index af84cce5cd7b..212e6779dc5c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux.txt
@@ -2,19 +2,32 @@ Common i2c bus multiplexer/switch properties.
 
 An i2c bus multiplexer/switch will have several child busses that are
 numbered uniquely in a device dependent manner.  The nodes for an i2c bus
-multiplexer/switch will have one child node for each child
-bus.
+multiplexer/switch will have one child node for each child bus.
 
-Required properties:
+Optional properties:
+- #address-cells = <1>;
+   This property is required is the i2c-mux child node does not exist.
+
+- #size-cells = <0>;
+   This property is required is the i2c-mux child node does not exist.
+
+- i2c-mux
+   For i2c multiplexers/switches that have child nodes that are a mixture
+   of both i2c child busses and other child nodes, the 'i2c-mux' subnode
+   can be used for populating the i2c child busses.  If an 'i2c-mux'
+   subnode is present, only subnodes of this will be considered as i2c
+   child busses.
+
+Required properties for the i2c-mux child node:
 - #address-cells = <1>;
 - #size-cells = <0>;
 
-Required properties for child nodes:
+Required properties for i2c child bus nodes:
 - #address-cells = <1>;
 - #size-cells = <0>;
 - reg : The sub-bus number.
 
-Optional properties for child nodes:
+Optional properties for i2c child bus nodes:
 - Other properties specific to the multiplexer/switch hardware.
 - Child nodes conforming to i2c bus binding
 
-- 
2.1.4

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

* [PATCH v2 2/8] dt-bindings: i2c: add support for 'i2c-arb' subnode
  2016-08-15 13:40 [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Peter Rosin
  2016-08-15 13:40 ` [PATCH v2 1/8] dt-bindings: i2c: add support for 'i2c-mux' subnode Peter Rosin
@ 2016-08-15 13:40 ` Peter Rosin
  2016-08-15 13:40 ` [PATCH v2 3/8] dt-bindings: i2c: add support for 'i2c-gate' subnode Peter Rosin
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-08-15 13:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Guenter Roeck, David S. Miller, Geert Uytterhoeven,
	Andrew Morton, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-i2c, devicetree, Crestez Dan Leonard

This gets rid of the need for a pointless 'reg' property for i2c
arbitrators.

I.e. this new and more compact style

	some-arbitrator {
		i2c-arb {
			#address-cells = <1>;
			#size-cells = <0>;

			some-i2c-device@50 {
				reg = <0x50>;
			};
		};
	};

instead of the old

	some-arbitrator {
		#address-cells = <1>;
		#size-cells = <0>;

		i2c@0 {
			reg = <0>;

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

			some-i2c-device@50 {
				reg = <0x50>;
			};
		};
	};

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../bindings/i2c/i2c-arb-gpio-challenge.txt        |  8 ++---
 Documentation/devicetree/bindings/i2c/i2c-arb.txt  | 35 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 3 files changed, 38 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-arb.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt b/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt
index 71191ff0e781..248a155414c2 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt
@@ -44,8 +44,7 @@ Required properties:
 - our-claim-gpio: The GPIO that we use to claim the bus.
 - their-claim-gpios: The GPIOs that the other sides use to claim the bus.
   Note that some implementations may only support a single other master.
-- Standard I2C mux properties. See i2c-mux.txt in this directory.
-- Single I2C child bus node at reg 0. See i2c-mux.txt in this directory.
+- I2C arbitration bus node. See i2c-arb.txt in this directory.
 
 Optional properties:
 - slew-delay-us: microseconds to wait for a GPIO to go high. Default is 10 us.
@@ -63,8 +62,6 @@ Example:
 
 	i2c-arbitrator {
 		compatible = "i2c-arb-gpio-challenge";
-		#address-cells = <1>;
-		#size-cells = <0>;
 
 		i2c-parent = <&{/i2c@12CA0000}>;
 
@@ -74,8 +71,7 @@ Example:
 		wait-retry-us = <3000>;
 		wait-free-us = <50000>;
 
-		i2c@0 {
-			reg = <0>;
+		i2c-arb {
 			#address-cells = <1>;
 			#size-cells = <0>;
 
diff --git a/Documentation/devicetree/bindings/i2c/i2c-arb.txt b/Documentation/devicetree/bindings/i2c/i2c-arb.txt
new file mode 100644
index 000000000000..59abf9277bdc
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-arb.txt
@@ -0,0 +1,35 @@
+Common i2c arbitration bus properties.
+
+- i2c-arb child node
+
+Required properties for the i2c-arb child node:
+- #address-cells = <1>;
+- #size-cells = <0>;
+
+Optional properties for i2c-arb child node:
+- Child nodes conforming to i2c bus binding
+
+
+Example :
+
+	/*
+	   An NXP pca9541 I2C bus master selector at address 0x74
+	   with a NXP pca8574 GPIO expander attached.
+	 */
+
+	arb@74 {
+		compatible = "nxp,pca9541";
+		reg = <0x74>;
+
+		i2c-arb {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			gpio@38 {
+				compatible = "nxp,pca8574";
+				reg = <0x38>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+		};
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 20bb1d00098c..1a61e2f5a0a9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5660,6 +5660,7 @@ S:	Maintained
 F:	Documentation/i2c/i2c-topology
 F:	Documentation/i2c/muxes/
 F:	Documentation/devicetree/bindings/i2c/i2c-mux*
+F:	Documentation/devicetree/bindings/i2c/i2c-arb*
 F:	drivers/i2c/i2c-mux.c
 F:	drivers/i2c/muxes/
 F:	include/linux/i2c-mux.h
-- 
2.1.4

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

* [PATCH v2 3/8] dt-bindings: i2c: add support for 'i2c-gate' subnode
  2016-08-15 13:40 [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Peter Rosin
  2016-08-15 13:40 ` [PATCH v2 1/8] dt-bindings: i2c: add support for 'i2c-mux' subnode Peter Rosin
  2016-08-15 13:40 ` [PATCH v2 2/8] dt-bindings: i2c: add support for 'i2c-arb' subnode Peter Rosin
@ 2016-08-15 13:40 ` Peter Rosin
  2016-08-16 13:57   ` Rob Herring
  2016-08-15 13:40 ` [PATCH v2 4/8] dt-bindings: i2c: add bindings for nxp,pca9541 Peter Rosin
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-08-15 13:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Guenter Roeck, David S. Miller, Geert Uytterhoeven,
	Andrew Morton, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-i2c, devicetree, Crestez Dan Leonard

Handle i2c gates similarly to how i2c arbitrators are handled.
This gets rid of a pointless 'reg' property for i2c gates.

I.e. this new and more compact style

        some-gate {
                i2c-gate {
                        #address-cells = <1>;
                        #size-cells = <0>;

                        some-i2c-device@50 {
                                reg = <0x50>;
                        };
                };
        };

instead of the old

        some-gate {
                #address-cells = <1>;
                #size-cells = <0>;

                i2c@0 {
                        reg = <0>;

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

                        some-i2c-device@50 {
                                reg = <0x50>;
                        };
                };
        };

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/devicetree/bindings/i2c/i2c-gate.txt | 41 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-gate.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-gate.txt b/Documentation/devicetree/bindings/i2c/i2c-gate.txt
new file mode 100644
index 000000000000..0b057fb2a15a
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-gate.txt
@@ -0,0 +1,41 @@
+An i2c gate is useful to e.g. reduce the digital noise for RF tuners connected
+to the i2c bus. Gates are similar to arbitrators in that you need to perform
+some kind of operation to access the i2c bus past the arbitrator/gate, but
+there are no competing masters to consider for gates and therefore there is
+no arbitration happening for gates.
+
+Common i2c gate properties.
+
+- i2c-gate child node
+
+Required properties for the i2c-gate child node:
+- #address-cells = <1>;
+- #size-cells = <0>;
+
+Optional properties for i2c-gate child node:
+- Child nodes conforming to i2c bus binding
+
+
+Example :
+
+	/*
+	   An Invensense mpu9150 at address 0x68 featuring an on-chip Asahi
+	   Kasei ak8975 compass behind a gate.
+	 */
+
+	mpu9150@68 {
+		compatible = "invensense,mpu9150";
+		reg = <0x68>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <18 1>;
+
+		i2c-gate {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ax8975@0c {
+				compatible = "ak,ak8975";
+				reg = <0x0c>;
+			};
+		};
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 1a61e2f5a0a9..7444b03a4c28 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5661,6 +5661,7 @@ F:	Documentation/i2c/i2c-topology
 F:	Documentation/i2c/muxes/
 F:	Documentation/devicetree/bindings/i2c/i2c-mux*
 F:	Documentation/devicetree/bindings/i2c/i2c-arb*
+F:	Documentation/devicetree/bindings/i2c/i2c-gate*
 F:	drivers/i2c/i2c-mux.c
 F:	drivers/i2c/muxes/
 F:	include/linux/i2c-mux.h
-- 
2.1.4

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

* [PATCH v2 4/8] dt-bindings: i2c: add bindings for nxp,pca9541
  2016-08-15 13:40 [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Peter Rosin
                   ` (2 preceding siblings ...)
  2016-08-15 13:40 ` [PATCH v2 3/8] dt-bindings: i2c: add support for 'i2c-gate' subnode Peter Rosin
@ 2016-08-15 13:40 ` Peter Rosin
  2016-08-15 13:40 ` [PATCH v2 5/8] i2c: mux: add support for 'i2c-mux', 'i2c-arb' and 'i2c-gate' DT subnodes Peter Rosin
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-08-15 13:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Guenter Roeck, David S. Miller, Geert Uytterhoeven,
	Andrew Morton, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-i2c, devicetree, Crestez Dan Leonard

Fill the gap for this pre-existing driver.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 .../devicetree/bindings/i2c/nxp,pca9541.txt        | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/nxp,pca9541.txt

diff --git a/Documentation/devicetree/bindings/i2c/nxp,pca9541.txt b/Documentation/devicetree/bindings/i2c/nxp,pca9541.txt
new file mode 100644
index 000000000000..0fbbc6970ec5
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/nxp,pca9541.txt
@@ -0,0 +1,29 @@
+* NXP PCA9541 I2C bus master selector
+
+Required Properties:
+
+  - compatible: Must be "nxp,pca9541"
+
+  - reg: The I2C address of the device.
+
+  The following required properties are defined externally:
+
+  - I2C arbitration bus node. See i2c-arb.txt in this directory.
+
+
+Example:
+
+	i2c-arbitrator@74 {
+		compatible = "nxp,pca9541";
+		reg = <0x74>;
+
+		i2c-arb {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			eeprom@54 {
+				compatible = "at,24c08";
+				reg = <0x54>;
+			};
+		};
+	};
-- 
2.1.4

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

* [PATCH v2 5/8] i2c: mux: add support for 'i2c-mux', 'i2c-arb' and 'i2c-gate' DT subnodes
  2016-08-15 13:40 [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Peter Rosin
                   ` (3 preceding siblings ...)
  2016-08-15 13:40 ` [PATCH v2 4/8] dt-bindings: i2c: add bindings for nxp,pca9541 Peter Rosin
@ 2016-08-15 13:40 ` Peter Rosin
  2016-08-15 13:55   ` Vladimir Zapolskiy
  2016-08-15 13:40 ` [PATCH v2 6/8] i2c: mux: inform the i2c mux core about how it is used Peter Rosin
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-08-15 13:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Guenter Roeck, David S. Miller, Geert Uytterhoeven,
	Andrew Morton, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-i2c, devicetree, Crestez Dan Leonard

Backwards compatibility is preserved; the subnodes are in practice
optional.

However, the mux core needs to know what subnode it should examine, so add
a couple of new flags for i2c_mux_alloc for this purpose.

The rule is that if the mux core finds a 'reg' property in the appropriate
subnode, e.g. if 'reg' exists in the 'i2c-mux' subnode, then the mux core
will assume that this is an old style entry and not an i2c-mux subnode
(correspondingly for arbitrators and gates with 'i2c-arb' and 'i2c-gate').

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/i2c-mux.c   | 44 ++++++++++++++++++++++++++++++++++++--------
 include/linux/i2c-mux.h |  8 ++++++--
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 8eee98634cda..c1ae719d1a7a 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -255,6 +255,10 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
 	muxc->dev = dev;
 	if (flags & I2C_MUX_LOCKED)
 		muxc->mux_locked = true;
+	if (flags & I2C_MUX_ARBITRATOR)
+		muxc->arbitrator = true;
+	if (flags & I2C_MUX_GATE)
+		muxc->gate = true;
 	muxc->select = select;
 	muxc->deselect = deselect;
 	muxc->max_adapters = max_adapters;
@@ -335,18 +339,42 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 	 * nothing if !CONFIG_OF.
 	 */
 	if (muxc->dev->of_node) {
-		struct device_node *child;
+		struct device_node *dev_node = muxc->dev->of_node;
+		struct device_node *mux_node, *child = NULL;
 		u32 reg;
 
-		for_each_child_of_node(muxc->dev->of_node, child) {
-			ret = of_property_read_u32(child, "reg", &reg);
-			if (ret)
-				continue;
-			if (chan_id == reg) {
-				priv->adap.dev.of_node = child;
-				break;
+		if (muxc->arbitrator)
+			mux_node = of_get_child_by_name(dev_node, "i2c-arb");
+		else if (muxc->gate)
+			mux_node = of_get_child_by_name(dev_node, "i2c-gate");
+		else
+			mux_node = of_get_child_by_name(dev_node, "i2c-mux");
+
+		if (mux_node) {
+			/* A "reg" property indicates an old-style DT entry */
+			if (!of_property_read_u32(mux_node, "reg", &reg)) {
+				of_node_put(mux_node);
+				mux_node = NULL;
+			}
+		}
+
+		if (!mux_node)
+			mux_node = of_node_get(dev_node);
+		else if (muxc->arbitrator || muxc->gate)
+			child = of_node_get(mux_node);
+
+		if (!child) {
+			for_each_child_of_node(mux_node, child) {
+				ret = of_property_read_u32(child, "reg", &reg);
+				if (ret)
+					continue;
+				if (chan_id == reg)
+					break;
 			}
 		}
+
+		priv->adap.dev.of_node = child;
+		of_node_put(mux_node);
 	}
 
 	/*
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index d4c1d12f900d..bd74d5706f3b 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -32,7 +32,9 @@
 struct i2c_mux_core {
 	struct i2c_adapter *parent;
 	struct device *dev;
-	bool mux_locked;
+	unsigned int mux_locked:1;
+	unsigned int arbitrator:1;
+	unsigned int gate:1;
 
 	void *priv;
 
@@ -51,7 +53,9 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
 				   int (*deselect)(struct i2c_mux_core *, u32));
 
 /* flags for i2c_mux_alloc */
-#define I2C_MUX_LOCKED BIT(0)
+#define I2C_MUX_LOCKED     BIT(0)
+#define I2C_MUX_ARBITRATOR BIT(1)
+#define I2C_MUX_GATE       BIT(2)
 
 static inline void *i2c_mux_priv(struct i2c_mux_core *muxc)
 {
-- 
2.1.4

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

* [PATCH v2 6/8] i2c: mux: inform the i2c mux core about how it is used
  2016-08-15 13:40 [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Peter Rosin
                   ` (4 preceding siblings ...)
  2016-08-15 13:40 ` [PATCH v2 5/8] i2c: mux: add support for 'i2c-mux', 'i2c-arb' and 'i2c-gate' DT subnodes Peter Rosin
@ 2016-08-15 13:40 ` Peter Rosin
  2016-08-25 16:19   ` Wolfram Sang
  2016-08-15 13:40 ` [PATCH v2 7/8] i2c: pca9541: add device tree binding Peter Rosin
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-08-15 13:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Guenter Roeck, David S. Miller, Geert Uytterhoeven,
	Andrew Morton, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-i2c, devicetree, Crestez Dan Leonard

The i2c mux core can then take appropriate action depending on if it is
used for an actual i2c mux, for a gate or for an arbitrator (the last
is the case for these drivers). This adds support for the new clearer
and more compact devicetree bindings that was added recently.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 2 +-
 drivers/i2c/muxes/i2c-mux-pca9541.c        | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
index a90bbc4037dd..86fc2d4c081b 100644
--- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
+++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
@@ -130,7 +130,7 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	muxc = i2c_mux_alloc(NULL, dev, 1, sizeof(*arb), 0,
+	muxc = i2c_mux_alloc(NULL, dev, 1, sizeof(*arb), I2C_MUX_ARBITRATOR,
 			     i2c_arbitrator_select, i2c_arbitrator_deselect);
 	if (!muxc)
 		return -ENOMEM;
diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index 3cb8af635db5..f052c3067791 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -349,7 +349,8 @@ static int pca9541_probe(struct i2c_client *client,
 	force = 0;
 	if (pdata)
 		force = pdata->modes[0].adap_id;
-	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data), 0,
+	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data),
+			     I2C_MUX_ARBITRATOR,
 			     pca9541_select_chan, pca9541_release_chan);
 	if (!muxc)
 		return -ENOMEM;
-- 
2.1.4

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

* [PATCH v2 7/8] i2c: pca9541: add device tree binding
  2016-08-15 13:40 [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Peter Rosin
                   ` (5 preceding siblings ...)
  2016-08-15 13:40 ` [PATCH v2 6/8] i2c: mux: inform the i2c mux core about how it is used Peter Rosin
@ 2016-08-15 13:40 ` Peter Rosin
  2016-08-15 13:40 ` [PATCH v2 8/8] i2c: pca954x: " Peter Rosin
  2016-08-25 16:22 ` [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Wolfram Sang
  8 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-08-15 13:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Guenter Roeck, David S. Miller, Geert Uytterhoeven,
	Andrew Morton, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-i2c, devicetree, Crestez Dan Leonard

No longer rely on the implicit matching with the i2c device name, use
an explicit compatible string instead.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/i2c-mux-pca9541.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index f052c3067791..4ea7e691afc7 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -85,6 +85,13 @@ static const struct i2c_device_id pca9541_id[] = {
 
 MODULE_DEVICE_TABLE(i2c, pca9541_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id pca9541_of_match[] = {
+	{ .compatible = "nxp,pca9541" },
+	{}
+};
+#endif
+
 /*
  * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
  * as they will try to lock the adapter a second time.
@@ -383,6 +390,7 @@ static int pca9541_remove(struct i2c_client *client)
 static struct i2c_driver pca9541_driver = {
 	.driver = {
 		   .name = "pca9541",
+		   .of_match_table = of_match_ptr(pca9541_of_match),
 		   },
 	.probe = pca9541_probe,
 	.remove = pca9541_remove,
-- 
2.1.4

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

* [PATCH v2 8/8] i2c: pca954x: add device tree binding
  2016-08-15 13:40 [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Peter Rosin
                   ` (6 preceding siblings ...)
  2016-08-15 13:40 ` [PATCH v2 7/8] i2c: pca9541: add device tree binding Peter Rosin
@ 2016-08-15 13:40 ` Peter Rosin
  2016-08-25 16:22 ` [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Wolfram Sang
  8 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-08-15 13:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Guenter Roeck, David S. Miller, Geert Uytterhoeven,
	Andrew Morton, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	linux-i2c, devicetree, Crestez Dan Leonard

No longer rely on the implicit matching with the i2c device name, use
an explicit compatible string instead.

Keep a direct pointer to the chip description instead of an index into
the chip description array.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 46 +++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 528e755c468f..6dfd31ba29a0 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -42,6 +42,7 @@
 #include <linux/i2c/pca954x.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
 
@@ -58,14 +59,6 @@ enum pca_type {
 	pca_9548,
 };
 
-struct pca954x {
-	enum pca_type type;
-
-	u8 last_chan;		/* last register value */
-	u8 deselect;
-	struct i2c_client *client;
-};
-
 struct chip_desc {
 	u8 nchans;
 	u8 enable;	/* used for muxes only */
@@ -75,6 +68,14 @@ struct chip_desc {
 	} muxtype;
 };
 
+struct pca954x {
+	const struct chip_desc *chip;
+
+	u8 last_chan;		/* last register value */
+	u8 deselect;
+	struct i2c_client *client;
+};
+
 /* Provide specs for the PCA954x types we know about */
 static const struct chip_desc chips[] = {
 	[pca_9540] = {
@@ -119,6 +120,20 @@ static const struct i2c_device_id pca954x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pca954x_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id pca954x_of_match[] = {
+	{ .compatible = "nxp,pca9540", .data = &chips[pca_9540] },
+	{ .compatible = "nxp,pca9542", .data = &chips[pca_9542] },
+	{ .compatible = "nxp,pca9543", .data = &chips[pca_9543] },
+	{ .compatible = "nxp,pca9544", .data = &chips[pca_9544] },
+	{ .compatible = "nxp,pca9545", .data = &chips[pca_9545] },
+	{ .compatible = "nxp,pca9546", .data = &chips[pca_9546] },
+	{ .compatible = "nxp,pca9547", .data = &chips[pca_9547] },
+	{ .compatible = "nxp,pca9548", .data = &chips[pca_9548] },
+	{}
+};
+#endif
+
 /* Write to mux register. Don't use i2c_transfer()/i2c_smbus_xfer()
    for this as they will try to lock adapter a second time */
 static int pca954x_reg_write(struct i2c_adapter *adap,
@@ -151,7 +166,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
 {
 	struct pca954x *data = i2c_mux_priv(muxc);
 	struct i2c_client *client = data->client;
-	const struct chip_desc *chip = &chips[data->type];
+	const struct chip_desc *chip = data->chip;
 	u8 regval;
 	int ret = 0;
 
@@ -197,6 +212,7 @@ static int pca954x_probe(struct i2c_client *client,
 	int num, force, class;
 	struct i2c_mux_core *muxc;
 	struct pca954x *data;
+	const struct of_device_id *match;
 	int ret;
 
 	if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE))
@@ -226,14 +242,19 @@ static int pca954x_probe(struct i2c_client *client,
 		return -ENODEV;
 	}
 
-	data->type = id->driver_data;
+	match = of_match_device(of_match_ptr(pca954x_of_match), &client->dev);
+	if (match)
+		data->chip = of_device_get_match_data(&client->dev);
+	else
+		data->chip = &chips[id->driver_data];
+
 	data->last_chan = 0;		   /* force the first selection */
 
 	idle_disconnect_dt = of_node &&
 		of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
 
 	/* Now create an adapter for each channel */
-	for (num = 0; num < chips[data->type].nchans; num++) {
+	for (num = 0; num < data->chip->nchans; num++) {
 		bool idle_disconnect_pd = false;
 
 		force = 0;			  /* dynamic adap number */
@@ -263,7 +284,7 @@ static int pca954x_probe(struct i2c_client *client,
 
 	dev_info(&client->dev,
 		 "registered %d multiplexed busses for I2C %s %s\n",
-		 num, chips[data->type].muxtype == pca954x_ismux
+		 num, data->chip->muxtype == pca954x_ismux
 				? "mux" : "switch", client->name);
 
 	return 0;
@@ -299,6 +320,7 @@ static struct i2c_driver pca954x_driver = {
 	.driver		= {
 		.name	= "pca954x",
 		.pm	= &pca954x_pm,
+		.of_match_table = of_match_ptr(pca954x_of_match),
 	},
 	.probe		= pca954x_probe,
 	.remove		= pca954x_remove,
-- 
2.1.4

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

* Re: [PATCH v2 5/8] i2c: mux: add support for 'i2c-mux', 'i2c-arb' and 'i2c-gate' DT subnodes
  2016-08-15 13:40 ` [PATCH v2 5/8] i2c: mux: add support for 'i2c-mux', 'i2c-arb' and 'i2c-gate' DT subnodes Peter Rosin
@ 2016-08-15 13:55   ` Vladimir Zapolskiy
  2016-08-15 14:33     ` Peter Rosin
  0 siblings, 1 reply; 19+ messages in thread
From: Vladimir Zapolskiy @ 2016-08-15 13:55 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Guenter Roeck,
	David S. Miller, Geert Uytterhoeven, Andrew Morton,
	Greg Kroah-Hartman, Mauro Carvalho Chehab, linux-i2c, devicetree,
	Crestez Dan Leonard

Hi Peter,

On 08/15/2016 04:40 PM, Peter Rosin wrote:
> Backwards compatibility is preserved; the subnodes are in practice
> optional.
>
> However, the mux core needs to know what subnode it should examine, so add
> a couple of new flags for i2c_mux_alloc for this purpose.
>
> The rule is that if the mux core finds a 'reg' property in the appropriate
> subnode, e.g. if 'reg' exists in the 'i2c-mux' subnode, then the mux core
> will assume that this is an old style entry and not an i2c-mux subnode
> (correspondingly for arbitrators and gates with 'i2c-arb' and 'i2c-gate').
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/i2c/i2c-mux.c   | 44 ++++++++++++++++++++++++++++++++++++--------
>  include/linux/i2c-mux.h |  8 ++++++--
>  2 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 8eee98634cda..c1ae719d1a7a 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -255,6 +255,10 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
>  	muxc->dev = dev;
>  	if (flags & I2C_MUX_LOCKED)
>  		muxc->mux_locked = true;
> +	if (flags & I2C_MUX_ARBITRATOR)
> +		muxc->arbitrator = true;
> +	if (flags & I2C_MUX_GATE)
> +		muxc->gate = true;
>  	muxc->select = select;
>  	muxc->deselect = deselect;
>  	muxc->max_adapters = max_adapters;
> @@ -335,18 +339,42 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>  	 * nothing if !CONFIG_OF.
>  	 */
>  	if (muxc->dev->of_node) {
> -		struct device_node *child;
> +		struct device_node *dev_node = muxc->dev->of_node;
> +		struct device_node *mux_node, *child = NULL;
>  		u32 reg;
>
> -		for_each_child_of_node(muxc->dev->of_node, child) {
> -			ret = of_property_read_u32(child, "reg", &reg);
> -			if (ret)
> -				continue;
> -			if (chan_id == reg) {
> -				priv->adap.dev.of_node = child;
> -				break;

missing of_node_put(child);

Also adding an empty line would be perfect.

> +		if (muxc->arbitrator)
> +			mux_node = of_get_child_by_name(dev_node, "i2c-arb");
> +		else if (muxc->gate)
> +			mux_node = of_get_child_by_name(dev_node, "i2c-gate");
> +		else
> +			mux_node = of_get_child_by_name(dev_node, "i2c-mux");
> +
> +		if (mux_node) {
> +			/* A "reg" property indicates an old-style DT entry */
> +			if (!of_property_read_u32(mux_node, "reg", &reg)) {
> +				of_node_put(mux_node);
> +				mux_node = NULL;
> +			}
> +		}
> +
> +		if (!mux_node)
> +			mux_node = of_node_get(dev_node);
> +		else if (muxc->arbitrator || muxc->gate)
> +			child = of_node_get(mux_node);
> +
> +		if (!child) {
> +			for_each_child_of_node(mux_node, child) {
> +				ret = of_property_read_u32(child, "reg", &reg);
> +				if (ret)
> +					continue;
> +				if (chan_id == reg)
> +					break;

missing of_node_put(child);

>  			}
>  		}
> +
> +		priv->adap.dev.of_node = child;
> +		of_node_put(mux_node);
>  	}
>
>  	/*
> diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
> index d4c1d12f900d..bd74d5706f3b 100644
> --- a/include/linux/i2c-mux.h
> +++ b/include/linux/i2c-mux.h
> @@ -32,7 +32,9 @@
>  struct i2c_mux_core {
>  	struct i2c_adapter *parent;
>  	struct device *dev;
> -	bool mux_locked;
> +	unsigned int mux_locked:1;
> +	unsigned int arbitrator:1;
> +	unsigned int gate:1;
>
>  	void *priv;
>
> @@ -51,7 +53,9 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
>  				   int (*deselect)(struct i2c_mux_core *, u32));
>
>  /* flags for i2c_mux_alloc */
> -#define I2C_MUX_LOCKED BIT(0)
> +#define I2C_MUX_LOCKED     BIT(0)
> +#define I2C_MUX_ARBITRATOR BIT(1)
> +#define I2C_MUX_GATE       BIT(2)
>
>  static inline void *i2c_mux_priv(struct i2c_mux_core *muxc)
>  {
>

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

* Re: [PATCH v2 5/8] i2c: mux: add support for 'i2c-mux', 'i2c-arb' and 'i2c-gate' DT subnodes
  2016-08-15 13:55   ` Vladimir Zapolskiy
@ 2016-08-15 14:33     ` Peter Rosin
  0 siblings, 0 replies; 19+ messages in thread
From: Peter Rosin @ 2016-08-15 14:33 UTC (permalink / raw)
  To: Vladimir Zapolskiy, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Guenter Roeck,
	David S. Miller, Geert Uytterhoeven, Andrew Morton,
	Greg Kroah-Hartman, Mauro Carvalho Chehab, linux-i2c, devicetree,
	Crestez Dan Leonard

Hi Vladimir,

On 2016-08-15 15:55, Vladimir Zapolskiy wrote:
> Hi Peter,
> 
> On 08/15/2016 04:40 PM, Peter Rosin wrote:
>> Backwards compatibility is preserved; the subnodes are in practice
>> optional.
>>
>> However, the mux core needs to know what subnode it should examine, so add
>> a couple of new flags for i2c_mux_alloc for this purpose.
>>
>> The rule is that if the mux core finds a 'reg' property in the appropriate
>> subnode, e.g. if 'reg' exists in the 'i2c-mux' subnode, then the mux core
>> will assume that this is an old style entry and not an i2c-mux subnode
>> (correspondingly for arbitrators and gates with 'i2c-arb' and 'i2c-gate').
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>  drivers/i2c/i2c-mux.c   | 44 ++++++++++++++++++++++++++++++++++++--------
>>  include/linux/i2c-mux.h |  8 ++++++--
>>  2 files changed, 42 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
>> index 8eee98634cda..c1ae719d1a7a 100644
>> --- a/drivers/i2c/i2c-mux.c
>> +++ b/drivers/i2c/i2c-mux.c
>> @@ -255,6 +255,10 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
>>  	muxc->dev = dev;
>>  	if (flags & I2C_MUX_LOCKED)
>>  		muxc->mux_locked = true;
>> +	if (flags & I2C_MUX_ARBITRATOR)
>> +		muxc->arbitrator = true;
>> +	if (flags & I2C_MUX_GATE)
>> +		muxc->gate = true;
>>  	muxc->select = select;
>>  	muxc->deselect = deselect;
>>  	muxc->max_adapters = max_adapters;
>> @@ -335,18 +339,42 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>>  	 * nothing if !CONFIG_OF.
>>  	 */
>>  	if (muxc->dev->of_node) {
>> -		struct device_node *child;
>> +		struct device_node *dev_node = muxc->dev->of_node;
>> +		struct device_node *mux_node, *child = NULL;
>>  		u32 reg;
>>
>> -		for_each_child_of_node(muxc->dev->of_node, child) {
>> -			ret = of_property_read_u32(child, "reg", &reg);
>> -			if (ret)
>> -				continue;
>> -			if (chan_id == reg) {
>> -				priv->adap.dev.of_node = child;
>> -				break;
> 
> missing of_node_put(child);

Note, you are commenting on the old code that is removed, but I don't think
there was any bug. A reference is stored away for later use, so better not
of_node_put(child) or it might disappear from under us. Or do I have a
fundamental misunderstanding?

> Also adding an empty line would be perfect.

Strange comment for the split between removing and adding lines. What do you
mean? I certainly don't miss any empty lines here...

>> +		if (muxc->arbitrator)
>> +			mux_node = of_get_child_by_name(dev_node, "i2c-arb");
>> +		else if (muxc->gate)
>> +			mux_node = of_get_child_by_name(dev_node, "i2c-gate");
>> +		else
>> +			mux_node = of_get_child_by_name(dev_node, "i2c-mux");
>> +
>> +		if (mux_node) {
>> +			/* A "reg" property indicates an old-style DT entry */
>> +			if (!of_property_read_u32(mux_node, "reg", &reg)) {
>> +				of_node_put(mux_node);
>> +				mux_node = NULL;
>> +			}
>> +		}
>> +
>> +		if (!mux_node)
>> +			mux_node = of_node_get(dev_node);
>> +		else if (muxc->arbitrator || muxc->gate)
>> +			child = of_node_get(mux_node);
>> +
>> +		if (!child) {
>> +			for_each_child_of_node(mux_node, child) {
>> +				ret = of_property_read_u32(child, "reg", &reg);
>> +				if (ret)
>> +					continue;
>> +				if (chan_id == reg)
>> +					break;
> 
> missing of_node_put(child);

Here you are commenting on the new code, but I think the same argument
holds here as well. The reference to the child node is stored away...

>>  			}
>>  		}
>> +
>> +		priv->adap.dev.of_node = child;

...here, for later use (by i2c_add_adapter).

Anyway, IF the of node really should be put, this is orthogonal to what
I'm doing here, and the problem exists in other places as well (just grep
for "dev.of_node = of_node_get". So, that would be fodder for future
patches.

Cheers,
Peter

>> +		of_node_put(mux_node);
>>  	}
>>
>>  	/*
>> diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
>> index d4c1d12f900d..bd74d5706f3b 100644
>> --- a/include/linux/i2c-mux.h
>> +++ b/include/linux/i2c-mux.h
>> @@ -32,7 +32,9 @@
>>  struct i2c_mux_core {
>>  	struct i2c_adapter *parent;
>>  	struct device *dev;
>> -	bool mux_locked;
>> +	unsigned int mux_locked:1;
>> +	unsigned int arbitrator:1;
>> +	unsigned int gate:1;
>>
>>  	void *priv;
>>
>> @@ -51,7 +53,9 @@ struct i2c_mux_core *i2c_mux_alloc(struct i2c_adapter *parent,
>>  				   int (*deselect)(struct i2c_mux_core *, u32));
>>
>>  /* flags for i2c_mux_alloc */
>> -#define I2C_MUX_LOCKED BIT(0)
>> +#define I2C_MUX_LOCKED     BIT(0)
>> +#define I2C_MUX_ARBITRATOR BIT(1)
>> +#define I2C_MUX_GATE       BIT(2)
>>
>>  static inline void *i2c_mux_priv(struct i2c_mux_core *muxc)
>>  {
>>

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

* Re: [PATCH v2 3/8] dt-bindings: i2c: add support for 'i2c-gate' subnode
  2016-08-15 13:40 ` [PATCH v2 3/8] dt-bindings: i2c: add support for 'i2c-gate' subnode Peter Rosin
@ 2016-08-16 13:57   ` Rob Herring
  2016-08-25 16:15     ` Wolfram Sang
  0 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2016-08-16 13:57 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Wolfram Sang, Mark Rutland, Guenter Roeck,
	David S. Miller, Geert Uytterhoeven, Andrew Morton,
	Greg Kroah-Hartman, Mauro Carvalho Chehab, linux-i2c, devicetree,
	Crestez Dan Leonard

On Mon, Aug 15, 2016 at 03:40:26PM +0200, Peter Rosin wrote:
> Handle i2c gates similarly to how i2c arbitrators are handled.
> This gets rid of a pointless 'reg' property for i2c gates.
> 
> I.e. this new and more compact style
> 
>         some-gate {
>                 i2c-gate {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         some-i2c-device@50 {
>                                 reg = <0x50>;
>                         };
>                 };
>         };
> 
> instead of the old
> 
>         some-gate {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> 
>                 i2c@0 {
>                         reg = <0>;
> 
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         some-i2c-device@50 {
>                                 reg = <0x50>;
>                         };
>                 };
>         };
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-gate.txt | 41 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-gate.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-gate.txt b/Documentation/devicetree/bindings/i2c/i2c-gate.txt
> new file mode 100644
> index 000000000000..0b057fb2a15a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-gate.txt
> @@ -0,0 +1,41 @@
> +An i2c gate is useful to e.g. reduce the digital noise for RF tuners connected
> +to the i2c bus. Gates are similar to arbitrators in that you need to perform
> +some kind of operation to access the i2c bus past the arbitrator/gate, but
> +there are no competing masters to consider for gates and therefore there is
> +no arbitration happening for gates.
> +
> +Common i2c gate properties.
> +
> +- i2c-gate child node
> +
> +Required properties for the i2c-gate child node:
> +- #address-cells = <1>;
> +- #size-cells = <0>;
> +
> +Optional properties for i2c-gate child node:
> +- Child nodes conforming to i2c bus binding
> +
> +
> +Example :
> +
> +	/*
> +	   An Invensense mpu9150 at address 0x68 featuring an on-chip Asahi
> +	   Kasei ak8975 compass behind a gate.
> +	 */
> +
> +	mpu9150@68 {
> +		compatible = "invensense,mpu9150";
> +		reg = <0x68>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <18 1>;
> +
> +		i2c-gate {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			ax8975@0c {

Drop the leading 0. With that,

Acked-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v2 3/8] dt-bindings: i2c: add support for 'i2c-gate' subnode
  2016-08-16 13:57   ` Rob Herring
@ 2016-08-25 16:15     ` Wolfram Sang
  0 siblings, 0 replies; 19+ messages in thread
From: Wolfram Sang @ 2016-08-25 16:15 UTC (permalink / raw)
  To: Rob Herring
  Cc: Peter Rosin, linux-kernel, Mark Rutland, Guenter Roeck,
	David S. Miller, Geert Uytterhoeven, Andrew Morton,
	Greg Kroah-Hartman, Mauro Carvalho Chehab, linux-i2c, devicetree,
	Crestez Dan Leonard

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

> > +			ax8975@0c {
> 
> Drop the leading 0. With that,

I fixed that locally.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v2 6/8] i2c: mux: inform the i2c mux core about how it is used
  2016-08-15 13:40 ` [PATCH v2 6/8] i2c: mux: inform the i2c mux core about how it is used Peter Rosin
@ 2016-08-25 16:19   ` Wolfram Sang
  2016-08-25 16:22     ` Peter Rosin
  0 siblings, 1 reply; 19+ messages in thread
From: Wolfram Sang @ 2016-08-25 16:19 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Rob Herring, Mark Rutland, Guenter Roeck,
	David S. Miller, Geert Uytterhoeven, Andrew Morton,
	Greg Kroah-Hartman, Mauro Carvalho Chehab, linux-i2c, devicetree,
	Crestez Dan Leonard

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


> diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
> index 3cb8af635db5..f052c3067791 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
> @@ -349,7 +349,8 @@ static int pca9541_probe(struct i2c_client *client,
>  	force = 0;
>  	if (pdata)
>  		force = pdata->modes[0].adap_id;
> -	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data), 0,
> +	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data),
> +			     I2C_MUX_ARBITRATOR,

Does it make sense to rename the file to i2c-arb-* somewhen then? Just
asking, I'll apply the patch anyhow.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates
  2016-08-15 13:40 [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Peter Rosin
                   ` (7 preceding siblings ...)
  2016-08-15 13:40 ` [PATCH v2 8/8] i2c: pca954x: " Peter Rosin
@ 2016-08-25 16:22 ` Wolfram Sang
  8 siblings, 0 replies; 19+ messages in thread
From: Wolfram Sang @ 2016-08-25 16:22 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Rob Herring, Mark Rutland, Guenter Roeck,
	David S. Miller, Geert Uytterhoeven, Andrew Morton,
	Greg Kroah-Hartman, Mauro Carvalho Chehab, linux-i2c, devicetree,
	Crestez Dan Leonard

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

On Mon, Aug 15, 2016 at 03:40:23PM +0200, Peter Rosin wrote:
> Hi!
> 
> The nxp pca9541 chip does not have any devicetree bindings.
> When trying to write such bindings matching the implementation,
> I stumbled upon something which I did not like. I had to
> give the node holding the i2c child bus a 'reg' property that
> is left unused and is really odd to explain from a devicetree
> point of view. It really is a leftover from the fact that linux
> implements i2c arbitrators (and i2c gates) using the i2c mux
> code. See this thread for history [1].
> 
> This series resolves the issues, and maintains backwards compat
> with old device trees (well, one could, in theory, create a
> device tree that would break with these changes, but my guess
> is that the odds for that happening inadvertedly are high...)
> 
> This should also help Crestez Dan Leonard with the mpu6050
> series containing the changes for its auxiliary i2c master [2],
> especially if you also consider the recent (very similar) changes
> from Jon Hunter that adds an optional 'i2c-bus' subnode [3].
> 
> If this is ok, I will follow up with patches for other drivers
> so that they inform the i2c mux core if they are muxes, arbs or
> gates. As stated, they will continue to work with these changes,
> so there is no huge rush.
> 
> The mux core does not really need to differentiate between
> arbitrators and gates. Should they be folded? What to call them
> in that case?
> 
> Changes since v1:
> 
> - Fixes and additions suggested by Rob Herring (3/8 still needs an ack).
> - Inserted patch 6/8 so that the two arb drivers support the new more
>   compact dt syntax.
> 
> Cheers,
> Peter
> 
> [1] https://lkml.org/lkml/2016/6/27/203
> [2] https://lkml.org/lkml/2016/5/18/355
> [3] https://patchwork.ozlabs.org/patch/641934/
> 
> Peter Rosin (8):
>   dt-bindings: i2c: add support for 'i2c-mux' subnode
>   dt-bindings: i2c: add support for 'i2c-arb' subnode
>   dt-bindings: i2c: add support for 'i2c-gate' subnode
>   dt-bindings: i2c: add bindings for nxp,pca9541
>   i2c: mux: add support for 'i2c-mux', 'i2c-arb' and 'i2c-gate' DT
>     subnodes
>   i2c: mux: inform the i2c mux core about how it is used
>   i2c: pca9541: add device tree binding
>   i2c: pca954x: add device tree binding

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v2 6/8] i2c: mux: inform the i2c mux core about how it is used
  2016-08-25 16:19   ` Wolfram Sang
@ 2016-08-25 16:22     ` Peter Rosin
  2016-08-25 16:24       ` Wolfram Sang
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-08-25 16:22 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel, Rob Herring, Mark Rutland, Guenter Roeck,
	David S. Miller, Geert Uytterhoeven, Andrew Morton,
	Greg Kroah-Hartman, Mauro Carvalho Chehab, linux-i2c, devicetree,
	Crestez Dan Leonard

On 2016-08-25 18:19, Wolfram Sang wrote:
> 
>> diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
>> index 3cb8af635db5..f052c3067791 100644
>> --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
>> +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
>> @@ -349,7 +349,8 @@ static int pca9541_probe(struct i2c_client *client,
>>  	force = 0;
>>  	if (pdata)
>>  		force = pdata->modes[0].adap_id;
>> -	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data), 0,
>> +	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data),
>> +			     I2C_MUX_ARBITRATOR,
> 
> Does it make sense to rename the file to i2c-arb-* somewhen then? Just
> asking, I'll apply the patch anyhow.

There should be a clean branch with /only/ this series that both i2c and
iio can share, to prevent merge problems. I said I'd make such a branch
in the i2c-mux repo, but you are of course also welcome to provide that
branch if you prefer...

Cheers,
Peter

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

* Re: [PATCH v2 6/8] i2c: mux: inform the i2c mux core about how it is used
  2016-08-25 16:22     ` Peter Rosin
@ 2016-08-25 16:24       ` Wolfram Sang
  2016-08-25 16:28         ` Peter Rosin
  0 siblings, 1 reply; 19+ messages in thread
From: Wolfram Sang @ 2016-08-25 16:24 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Rob Herring, Mark Rutland, Guenter Roeck,
	David S. Miller, Geert Uytterhoeven, Andrew Morton,
	Greg Kroah-Hartman, Mauro Carvalho Chehab, linux-i2c, devicetree,
	Crestez Dan Leonard

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

On Thu, Aug 25, 2016 at 06:22:37PM +0200, Peter Rosin wrote:
> On 2016-08-25 18:19, Wolfram Sang wrote:
> > 
> >> diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
> >> index 3cb8af635db5..f052c3067791 100644
> >> --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
> >> +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
> >> @@ -349,7 +349,8 @@ static int pca9541_probe(struct i2c_client *client,
> >>  	force = 0;
> >>  	if (pdata)
> >>  		force = pdata->modes[0].adap_id;
> >> -	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data), 0,
> >> +	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data),
> >> +			     I2C_MUX_ARBITRATOR,
> > 
> > Does it make sense to rename the file to i2c-arb-* somewhen then? Just
> > asking, I'll apply the patch anyhow.
> 
> There should be a clean branch with /only/ this series that both i2c and
> iio can share, to prevent merge problems. I said I'd make such a branch
> in the i2c-mux repo, but you are of course also welcome to provide that
> branch if you prefer...

No, this is fine. Send me a pull request and I'll merge.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v2 6/8] i2c: mux: inform the i2c mux core about how it is used
  2016-08-25 16:24       ` Wolfram Sang
@ 2016-08-25 16:28         ` Peter Rosin
  2016-08-25 16:38           ` Wolfram Sang
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Rosin @ 2016-08-25 16:28 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel, Rob Herring, Mark Rutland, Guenter Roeck,
	David S. Miller, Geert Uytterhoeven, Andrew Morton,
	Greg Kroah-Hartman, Mauro Carvalho Chehab, linux-i2c, devicetree,
	Crestez Dan Leonard

On 2016-08-25 18:24, Wolfram Sang wrote:
> On Thu, Aug 25, 2016 at 06:22:37PM +0200, Peter Rosin wrote:
>> On 2016-08-25 18:19, Wolfram Sang wrote:
>>>
>>>> diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
>>>> index 3cb8af635db5..f052c3067791 100644
>>>> --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
>>>> +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
>>>> @@ -349,7 +349,8 @@ static int pca9541_probe(struct i2c_client *client,
>>>>  	force = 0;
>>>>  	if (pdata)
>>>>  		force = pdata->modes[0].adap_id;
>>>> -	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data), 0,
>>>> +	muxc = i2c_mux_alloc(adap, &client->dev, 1, sizeof(*data),
>>>> +			     I2C_MUX_ARBITRATOR,
>>>
>>> Does it make sense to rename the file to i2c-arb-* somewhen then? Just
>>> asking, I'll apply the patch anyhow.
>>
>> There should be a clean branch with /only/ this series that both i2c and
>> iio can share, to prevent merge problems. I said I'd make such a branch
>> in the i2c-mux repo, but you are of course also welcome to provide that
>> branch if you prefer...
> 
> No, this is fine. Send me a pull request and I'll merge.

Right, I was waiting for some confirmation if v4.8-rc3 was a good base,
but I'm running out of patience...

Anyway, I'll send the pull request no later that tomorrow morning (CEST).

I assume I can add your ack?

Cheers,
Peter

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

* Re: [PATCH v2 6/8] i2c: mux: inform the i2c mux core about how it is used
  2016-08-25 16:28         ` Peter Rosin
@ 2016-08-25 16:38           ` Wolfram Sang
  0 siblings, 0 replies; 19+ messages in thread
From: Wolfram Sang @ 2016-08-25 16:38 UTC (permalink / raw)
  To: Peter Rosin
  Cc: linux-kernel, Rob Herring, Mark Rutland, Guenter Roeck,
	David S. Miller, Geert Uytterhoeven, Andrew Morton,
	Greg Kroah-Hartman, Mauro Carvalho Chehab, linux-i2c, devicetree,
	Crestez Dan Leonard

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


> Right, I was waiting for some confirmation if v4.8-rc3 was a good base,
> but I'm running out of patience...

For me, it is perfect.

> Anyway, I'll send the pull request no later that tomorrow morning (CEST).

Thanks!

> I assume I can add your ack?

Rather this:

Reviewed-by: Wolfram Sang <wsa@the-dreams.de>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

end of thread, other threads:[~2016-08-26  4:04 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-15 13:40 [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Peter Rosin
2016-08-15 13:40 ` [PATCH v2 1/8] dt-bindings: i2c: add support for 'i2c-mux' subnode Peter Rosin
2016-08-15 13:40 ` [PATCH v2 2/8] dt-bindings: i2c: add support for 'i2c-arb' subnode Peter Rosin
2016-08-15 13:40 ` [PATCH v2 3/8] dt-bindings: i2c: add support for 'i2c-gate' subnode Peter Rosin
2016-08-16 13:57   ` Rob Herring
2016-08-25 16:15     ` Wolfram Sang
2016-08-15 13:40 ` [PATCH v2 4/8] dt-bindings: i2c: add bindings for nxp,pca9541 Peter Rosin
2016-08-15 13:40 ` [PATCH v2 5/8] i2c: mux: add support for 'i2c-mux', 'i2c-arb' and 'i2c-gate' DT subnodes Peter Rosin
2016-08-15 13:55   ` Vladimir Zapolskiy
2016-08-15 14:33     ` Peter Rosin
2016-08-15 13:40 ` [PATCH v2 6/8] i2c: mux: inform the i2c mux core about how it is used Peter Rosin
2016-08-25 16:19   ` Wolfram Sang
2016-08-25 16:22     ` Peter Rosin
2016-08-25 16:24       ` Wolfram Sang
2016-08-25 16:28         ` Peter Rosin
2016-08-25 16:38           ` Wolfram Sang
2016-08-15 13:40 ` [PATCH v2 7/8] i2c: pca9541: add device tree binding Peter Rosin
2016-08-15 13:40 ` [PATCH v2 8/8] i2c: pca954x: " Peter Rosin
2016-08-25 16:22 ` [PATCH v2 0/8] devicetree cleanup for i2c muxes/arbs/gates Wolfram Sang

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