LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510
@ 2015-02-17 18:52 Sebastian Hesselbarth
  2015-02-17 18:52 ` [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
                   ` (9 more replies)
  0 siblings, 10 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 18:52 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, Stephen Warren, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel

This patch set improves current mainline support for the Compulab
CM-A510 System-on-Module (SoM) and its default Compulab SBC-A510
base board. Thanks to Gabriel Dobato who agreed to remote debug and
test the provided DT changes.

On the way to proper support, we
- Rework i2c-mux-pinctrl to honor disabled sub-bus nodes
- Add missing "compulab" vendor prefix
- Fix broken uart[23] reg properties
- Beautify Dove's dtsi files by adding gpio/irq includes, node
  labels for pcie, and some additional pinctrl settings
- Add a node for the internal i2c mux mechanism on Dove SoCs

And finally add a DT include for the Compulab CM-A510 SoM and a DT
board file for the SBC-A510 base board.

Patches are based on stable v3.19 and are indended for the next
merge window (either v3.21 or v4.1). Compulab related changes have
been tested by Gabriel Dobato, I tested on SolidRun CuBox that it
does not break existing Dove boards.

For the i2c-mux-pinctrl, a Tested-by from any user of Tegra20
Seaboard, Tamonten, or Ventana would be nice to see if it is fully
compatible.

I have added MVEBU maintainers to all patches, Wolfram for i2c and
Stephen as the i2c-mux-pinctrl author i2c-mux-pinctrl related patches,
and corresponding lists. As there is no important DT work in here,
I decided to not explicitly add each of the DT maintainers except for
the vendor prefix patch.

Sebastian Hesselbarth (8):
  i2c: mux-pinctrl: Rework to honor disabled child nodes
  devicetree: vendor-prefixes: Add CompuLab to known vendors
  ARM: dts: dove: Fix uart[23] reg property
  ARM: dts: dove: Always include gpio and interrupt-controller headers
  ARM: dts: dove: Add node labels for PCIe ports 0 and 1
  ARM: dts: dove: Add some more common pinctrl settings
  ARM: dts: dove: Add internal i2c multiplexer node
  ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510

 .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt    |  28 +--
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/arm/boot/dts/Makefile                         |   5 +-
 arch/arm/boot/dts/dove-cm-a510.dts                 |  38 ----
 arch/arm/boot/dts/dove-cm-a510.dtsi                | 195 +++++++++++++++++++++
 arch/arm/boot/dts/dove-sbc-a510.dts                | 182 +++++++++++++++++++
 arch/arm/boot/dts/dove.dtsi                        | 103 ++++++++++-
 drivers/i2c/muxes/i2c-mux-pinctrl.c                |  70 +++++---
 8 files changed, 537 insertions(+), 85 deletions(-)
 delete mode 100644 arch/arm/boot/dts/dove-cm-a510.dts
 create mode 100644 arch/arm/boot/dts/dove-cm-a510.dtsi
 create mode 100644 arch/arm/boot/dts/dove-sbc-a510.dts

---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
-- 
2.1.0


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

* [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
@ 2015-02-17 18:52 ` Sebastian Hesselbarth
  2015-02-17 20:46   ` Stephen Warren
  2015-02-26 21:46   ` Stephen Warren
  2015-02-17 18:52 ` [PATCH 2/8] devicetree: vendor-prefixes: Add CompuLab to known vendors Sebastian Hesselbarth
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 18:52 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, Stephen Warren, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel

I2C mux pinctrl driver currently determines the number of sub-busses by
counting available pinctrl-names. Unfortunately, this requires each
incarnation of the devicetree node with different available sub-busses
to be rewritten.

This patch reworks i2c-mux-pinctrl driver to count the number of
available sub-nodes instead. The rework should be compatible to the old
way of probing for sub-busses and additionally allows to disable unused
sub-busses with standard DT property status = "disabled".

This also amends the corresponding devicetree binding documentation to
reflect the new functionality to disable unused sub-nodes. While at it,
also fix two references to binding documentation files that miss an "i2c-"
prefix.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt    | 28 ++++-----
 drivers/i2c/muxes/i2c-mux-pinctrl.c                | 70 ++++++++++++++--------
 2 files changed, 59 insertions(+), 39 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
index ae8af1694e95..24b9fdef8850 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
@@ -28,27 +28,24 @@ Also required are:
 * Standard pinctrl properties that specify the pin mux state for each child
   bus. See ../pinctrl/pinctrl-bindings.txt.
 
-* Standard I2C mux properties. See mux.txt in this directory.
+* Standard I2C mux properties. See i2c-mux.txt in this directory.
 
-* I2C child bus nodes. See mux.txt in this directory.
+* I2C child bus nodes. See i2c-mux.txt in this directory.
 
-For each named state defined in the pinctrl-names property, an I2C child bus
-will be created. I2C child bus numbers are assigned based on the index into
-the pinctrl-names property.
+For each child node that is not disabled by a status != "okay", an I2C
+child bus will be created. I2C child bus numbers are assigned based on the
+order of child nodes.
 
-The only exception is that no bus will be created for a state named "idle". If
-such a state is defined, it must be the last entry in pinctrl-names. For
-example:
-
-	pinctrl-names = "ddc", "pta", "idle"  ->  ddc = bus 0, pta = bus 1
-	pinctrl-names = "ddc", "idle", "pta"  ->  Invalid ("idle" not last)
-	pinctrl-names = "idle", "ddc", "pta"  ->  Invalid ("idle" not last)
+There must be a corresponding pinctrl-names entry for each enabled child
+node at the position of the child node's "reg" property.
 
 Whenever an access is made to a device on a child bus, the relevant pinctrl
 state will be programmed into hardware.
 
-If an idle state is defined, whenever an access is not being made to a device
-on a child bus, the idle pinctrl state will be programmed into hardware.
+Also, there can be an idle pinctrl state defined at the end of possible
+pinctrl states. If an idle state is defined, whenever an access is not being
+made to a device on a child bus, the idle pinctrl state will be programmed
+into hardware.
 
 If an idle state is not defined, the most recently used pinctrl state will be
 left programmed into hardware whenever no access is being made of a device on
@@ -68,6 +65,7 @@ Example:
 		pinctrl-1 = <&state_i2cmux_pta>;
 		pinctrl-2 = <&state_i2cmux_idle>;
 
+		/* Enabled child bus 0 */
 		i2c@0 {
 			reg = <0>;
 			#address-cells = <1>;
@@ -79,10 +77,12 @@ Example:
 			};
 		};
 
+		/* Disabled child bus 1 */
 		i2c@1 {
 			reg = <1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			status = "disabled";
 
 			eeprom {
 				compatible = "eeprom";
diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
index b48378c4b40d..033dacfabfdf 100644
--- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
@@ -56,9 +56,12 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
 				struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	int num_names, i, ret;
+	struct device_node *child;
+	struct property *prop;
+	int num_names, num_children, ret;
 	struct device_node *adapter_np;
 	struct i2c_adapter *adapter;
+	const char *state;
 
 	if (!np)
 		return 0;
@@ -77,32 +80,16 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
 		return num_names;
 	}
 
-	mux->pdata->pinctrl_states = devm_kzalloc(&pdev->dev,
-		sizeof(*mux->pdata->pinctrl_states) * num_names,
-		GFP_KERNEL);
-	if (!mux->pdata->pinctrl_states) {
-		dev_err(mux->dev, "Cannot allocate pinctrl_states\n");
-		return -ENOMEM;
+	num_children = of_get_available_child_count(np);
+	if (num_children < 0) {
+		dev_err(mux->dev, "Unable to count available children: %d\n",
+			num_children);
+		return num_children;
 	}
 
-	for (i = 0; i < num_names; i++) {
-		ret = of_property_read_string_index(np, "pinctrl-names", i,
-			&mux->pdata->pinctrl_states[mux->pdata->bus_count]);
-		if (ret < 0) {
-			dev_err(mux->dev, "Cannot parse pinctrl-names: %d\n",
-				ret);
-			return ret;
-		}
-		if (!strcmp(mux->pdata->pinctrl_states[mux->pdata->bus_count],
-			    "idle")) {
-			if (i != num_names - 1) {
-				dev_err(mux->dev, "idle state must be last\n");
-				return -EINVAL;
-			}
-			mux->pdata->pinctrl_state_idle = "idle";
-		} else {
-			mux->pdata->bus_count++;
-		}
+	if (num_names < num_children) {
+		dev_err(mux->dev, "Found less pinctrl states than children\n");
+		return -EINVAL;
 	}
 
 	adapter_np = of_parse_phandle(np, "i2c-parent", 0);
@@ -118,6 +105,39 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
 	mux->pdata->parent_bus_num = i2c_adapter_id(adapter);
 	put_device(&adapter->dev);
 
+	mux->pdata->pinctrl_states = devm_kzalloc(&pdev->dev,
+		sizeof(*mux->pdata->pinctrl_states) * num_children,
+		GFP_KERNEL);
+	if (!mux->pdata->pinctrl_states) {
+		dev_err(mux->dev, "Cannot allocate pinctrl_states\n");
+		return -ENOMEM;
+	}
+
+	of_property_for_each_string(np, "pinctrl-names", prop, state)
+		if (!strcmp(state, "idle"))
+			mux->pdata->pinctrl_state_idle = "idle";
+
+	for_each_available_child_of_node(np, child) {
+		u32 reg;
+
+		ret = of_property_read_u32(child, "reg", &reg);
+		if (ret < 0) {
+			dev_err(mux->dev, "Missing reg property for child node: %d\n",
+				ret);
+			return ret;
+		}
+
+		ret = of_property_read_string_index(np,
+				    "pinctrl-names", reg, &state);
+		if (ret < 0) {
+			dev_err(mux->dev, "Cannot parse pinctrl-names for mux %d: %d\n",
+				reg, ret);
+			return ret;
+		}
+
+		mux->pdata->pinctrl_states[mux->pdata->bus_count++] = state;
+	}
+
 	return 0;
 }
 #else
-- 
2.1.0


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

* [PATCH 2/8] devicetree: vendor-prefixes: Add CompuLab to known vendors
  2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
  2015-02-17 18:52 ` [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
@ 2015-02-17 18:52 ` Sebastian Hesselbarth
  2015-02-17 19:37   ` Rob Herring
  2015-02-17 18:52 ` [PATCH 3/8] ARM: dts: dove: Fix uart[23] reg property Sebastian Hesselbarth
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 18:52 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree, linux-arm-kernel, linux-kernel

This adds "compulab" as a vendor-prefix for CompuLab (compulab.co.il),
an Israeli company that builds ARM-based SoMs and CoMs.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d443279c95dc..2de628532d4b 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -35,6 +35,7 @@ chrp	Common Hardware Reference Platform
 chunghwa	Chunghwa Picture Tubes Ltd.
 cirrus	Cirrus Logic, Inc.
 cnm	Chips&Media, Inc.
+compulab	CompuLab
 cortina	Cortina Systems, Inc.
 crystalfontz	Crystalfontz America, Inc.
 dallas	Maxim Integrated Products (formerly Dallas Semiconductor)
-- 
2.1.0


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

* [PATCH 3/8] ARM: dts: dove: Fix uart[23] reg property
  2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
  2015-02-17 18:52 ` [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
  2015-02-17 18:52 ` [PATCH 2/8] devicetree: vendor-prefixes: Add CompuLab to known vendors Sebastian Hesselbarth
@ 2015-02-17 18:52 ` Sebastian Hesselbarth
  2015-02-23 14:54   ` Gregory CLEMENT
  2015-02-17 18:52 ` [PATCH 4/8] ARM: dts: dove: Always include gpio and interrupt-controller headers Sebastian Hesselbarth
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 18:52 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	linux-arm-kernel, linux-kernel

Fix Dove's register addresses of uart2 and uart3 nodes that seem to
be broken since ages due to a copy-and-paste error.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Please consider for stable back to v3.7

Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/dove.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index a5441d5482a6..3cc8b8320345 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -154,7 +154,7 @@
 
 			uart2: serial@12200 {
 				compatible = "ns16550a";
-				reg = <0x12000 0x100>;
+				reg = <0x12200 0x100>;
 				reg-shift = <2>;
 				interrupts = <9>;
 				clocks = <&core_clk 0>;
@@ -163,7 +163,7 @@
 
 			uart3: serial@12300 {
 				compatible = "ns16550a";
-				reg = <0x12100 0x100>;
+				reg = <0x12300 0x100>;
 				reg-shift = <2>;
 				interrupts = <10>;
 				clocks = <&core_clk 0>;
-- 
2.1.0


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

* [PATCH 4/8] ARM: dts: dove: Always include gpio and interrupt-controller headers
  2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
                   ` (2 preceding siblings ...)
  2015-02-17 18:52 ` [PATCH 3/8] ARM: dts: dove: Fix uart[23] reg property Sebastian Hesselbarth
@ 2015-02-17 18:52 ` Sebastian Hesselbarth
  2015-02-23 14:54   ` Gregory CLEMENT
  2015-02-17 18:52 ` [PATCH 5/8] ARM: dts: dove: Add node labels for PCIe ports 0 and 1 Sebastian Hesselbarth
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 18:52 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	linux-arm-kernel, linux-kernel

We want to enforce the use of named flags in GPIO and interrupt
specifiers, include the corresponding headers to Dove's SoC dtsi.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/dove.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 3cc8b8320345..cae865435762 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -1,5 +1,8 @@
 /include/ "skeleton.dtsi"
 
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+
 #define MBUS_ID(target,attributes) (((target) << 24) | ((attributes) << 16))
 
 / {
-- 
2.1.0


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

* [PATCH 5/8] ARM: dts: dove: Add node labels for PCIe ports 0 and 1
  2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
                   ` (3 preceding siblings ...)
  2015-02-17 18:52 ` [PATCH 4/8] ARM: dts: dove: Always include gpio and interrupt-controller headers Sebastian Hesselbarth
@ 2015-02-17 18:52 ` Sebastian Hesselbarth
  2015-02-23 14:55   ` Gregory CLEMENT
  2015-02-17 18:52 ` [PATCH 6/8] ARM: dts: dove: Add some more common pinctrl settings Sebastian Hesselbarth
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 18:52 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	linux-arm-kernel, linux-kernel

Add pcie[01] node labels to allow to reference them easily from
board level.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/dove.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index cae865435762..81209bd9525a 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -64,7 +64,7 @@
 				  0x82000000 0x2 0x0 MBUS_ID(0x08, 0xe8) 0 1 0   /* Port 1.0 Mem */
 				  0x81000000 0x2 0x0 MBUS_ID(0x08, 0xe0) 0 1 0>; /* Port 1.0 I/O */
 
-			pcie-port@0 {
+			pcie0: pcie-port@0 {
 				device_type = "pci";
 				status = "disabled";
 				assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
@@ -82,7 +82,7 @@
 				interrupt-map = <0 0 0 0 &intc 16>;
 			};
 
-			pcie-port@1 {
+			pcie1: pcie-port@1 {
 				device_type = "pci";
 				status = "disabled";
 				assigned-addresses = <0x82002800 0 0x80000 0 0x2000>;
-- 
2.1.0


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

* [PATCH 6/8] ARM: dts: dove: Add some more common pinctrl settings
  2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
                   ` (4 preceding siblings ...)
  2015-02-17 18:52 ` [PATCH 5/8] ARM: dts: dove: Add node labels for PCIe ports 0 and 1 Sebastian Hesselbarth
@ 2015-02-17 18:52 ` Sebastian Hesselbarth
  2015-02-23 14:56   ` Gregory CLEMENT
  2015-02-17 18:52 ` [PATCH 7/8] ARM: dts: dove: Add internal i2c multiplexer node Sebastian Hesselbarth
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 18:52 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	linux-arm-kernel, linux-kernel

This add common pinctrl settings for pcie[01]_clkreq, spi1, i2c[23],
and internal i2c mux. These settings have either one or two options
only, so put them into the SoC dtsi instead of repeating them on
board level.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/dove.dtsi | 52 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 81209bd9525a..9ad829523a13 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -451,6 +451,11 @@
 					marvell,function = "gpio";
 				};
 
+				pmx_pcie1_clkreq: pmx-pcie1-clkreq {
+					marvell,pins = "mpp9";
+					marvell,function = "pex1";
+				};
+
 				pmx_gpio_10: pmx-gpio-10 {
 					marvell,pins = "mpp10";
 					marvell,function = "gpio";
@@ -461,6 +466,11 @@
 					marvell,function = "gpio";
 				};
 
+				pmx_pcie0_clkreq: pmx-pcie0-clkreq {
+					marvell,pins = "mpp11";
+					marvell,function = "pex0";
+				};
+
 				pmx_gpio_12: pmx-gpio-12 {
 					marvell,pins = "mpp12";
 					marvell,function = "gpio";
@@ -566,6 +576,18 @@
 					marvell,function = "gpio";
 				};
 
+				pmx_spi1_4_7: pmx-spi1-4-7 {
+					marvell,pins = "mpp4", "mpp5",
+						"mpp6", "mpp7";
+					marvell,function = "spi1";
+				};
+
+				pmx_spi1_20_23: pmx-spi1-20-23 {
+					marvell,pins = "mpp20", "mpp21",
+						"mpp22", "mpp23";
+					marvell,function = "spi1";
+				};
+
 				pmx_uart1: pmx-uart1 {
 					marvell,pins = "mpp_uart1";
 					marvell,function = "uart1";
@@ -585,6 +607,36 @@
 					marvell,pins = "mpp_nand";
 					marvell,function = "gpo";
 				};
+
+				pmx_i2c1: pmx-i2c1 {
+					marvell,pins = "mpp17", "mpp19";
+					marvell,function = "twsi";
+				};
+
+				pmx_i2c2: pmx-i2c2 {
+					marvell,pins = "mpp_audio1";
+					marvell,function = "twsi";
+				};
+
+				pmx_ssp_i2c2: pmx-ssp-i2c2 {
+					marvell,pins = "mpp_audio1";
+					marvell,function = "ssp/twsi";
+				};
+
+				pmx_i2cmux_0: pmx-i2cmux-0 {
+					marvell,pins = "twsi";
+					marvell,function = "twsi-opt1";
+				};
+
+				pmx_i2cmux_1: pmx-i2cmux-1 {
+					marvell,pins = "twsi";
+					marvell,function = "twsi-opt2";
+				};
+
+				pmx_i2cmux_2: pmx-i2cmux-2 {
+					marvell,pins = "twsi";
+					marvell,function = "twsi-opt3";
+				};
 			};
 
 			core_clk: core-clocks@d0214 {
-- 
2.1.0


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

* [PATCH 7/8] ARM: dts: dove: Add internal i2c multiplexer node
  2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
                   ` (5 preceding siblings ...)
  2015-02-17 18:52 ` [PATCH 6/8] ARM: dts: dove: Add some more common pinctrl settings Sebastian Hesselbarth
@ 2015-02-17 18:52 ` Sebastian Hesselbarth
  2015-02-23 15:07   ` Gregory CLEMENT
  2015-02-17 18:52 ` [PATCH 8/8] ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 18:52 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, Stephen Warren, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel

This adds a i2c-mux-pinctrl node to dove.dtsi for the internal i2c
mux found on Dove SoCs. Up to now, we had no board using any of the
two additional i2c busses, so make sure the change does not break
any existing boards.

Therefore, we rename the i2c-controller node label to "i2c" and
enable it by default. Also, the dedicated sub-bus (now "i2c0") is
enabled by default. The two optional sub-busses require additional
external pin-muxing, so disable them by default.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
Wolfram,

Actually, I was hoping that default pin hog mechanism
(pinctrl-names = "default") could also be used from i2c mux nodes
and devices. Anyway, I had a look at i2c-core/mux code and failed
how to achieve that easily. Instead I decided, it would also be ok
to put the pin hog into the i2c controller node where pins will be
bound by standard platform device code.

Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/dove.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 9ad829523a13..b3340e862b0e 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -28,6 +28,42 @@
 		};
 	};
 
+	i2c-mux {
+		compatible = "i2c-mux-pinctrl";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c-parent = <&i2c>;
+
+		pinctrl-names = "i2c0", "i2c1", "i2c2";
+		pinctrl-0 = <&pmx_i2cmux_0>;
+		pinctrl-1 = <&pmx_i2cmux_1>;
+		pinctrl-2 = <&pmx_i2cmux_2>;
+
+		i2c0: i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "okay";
+		};
+
+		i2c1: i2c@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			/* Requires pmx_i2c1 on i2c controller node */
+			status = "disabled";
+		};
+
+		i2c2: i2c@2 {
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			/* Requires pmx_i2c2 on i2c controller node */
+			status = "disabled";
+		};
+	};
+
 	l2: l2-cache {
 		compatible = "marvell,tauros2-cache";
 		marvell,tauros2-cache-features = <0>;
@@ -123,7 +159,7 @@
 				status = "disabled";
 			};
 
-			i2c0: i2c-ctrl@11000 {
+			i2c: i2c-ctrl@11000 {
 				compatible = "marvell,mv64xxx-i2c";
 				reg = <0x11000 0x20>;
 				#address-cells = <1>;
@@ -132,7 +168,7 @@
 				clock-frequency = <400000>;
 				timeout-ms = <1000>;
 				clocks = <&core_clk 0>;
-				status = "disabled";
+				status = "okay";
 			};
 
 			uart0: serial@12000 {
-- 
2.1.0


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

* [PATCH 8/8] ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510
  2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
                   ` (6 preceding siblings ...)
  2015-02-17 18:52 ` [PATCH 7/8] ARM: dts: dove: Add internal i2c multiplexer node Sebastian Hesselbarth
@ 2015-02-17 18:52 ` Sebastian Hesselbarth
  2015-02-26 17:55 ` [PATCH 0/8] " Gregory CLEMENT
  2015-02-27 12:24 ` [PATCH v2 0/4] " Sebastian Hesselbarth
  9 siblings, 0 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 18:52 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	linux-arm-kernel, linux-kernel

Existing dts file for Compulab CM-A510 was very limited due to missing
hardware. Now that we actually found somebody with that board, properly
rework it to provide a CoM/SoM include and a board file for Compulab's
SBC-A510.

Both the CM-A510 SoM and the SBC-A510 can be configured with different
options, so we only enable a minimum set of options. The actual board
configuration will have to be set by either the bootloader or user.

Although functionally not required, repeat even disabled nodes again
to increse their visibility in the dtsi/dts files.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Tested-by: Gabriel Dobato <dobatog@gmail.com>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/Makefile          |   5 +-
 arch/arm/boot/dts/dove-cm-a510.dts  |  38 -------
 arch/arm/boot/dts/dove-cm-a510.dtsi | 195 ++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/dove-sbc-a510.dts | 182 +++++++++++++++++++++++++++++++++
 4 files changed, 380 insertions(+), 40 deletions(-)
 delete mode 100644 arch/arm/boot/dts/dove-cm-a510.dts
 create mode 100644 arch/arm/boot/dts/dove-cm-a510.dtsi
 create mode 100644 arch/arm/boot/dts/dove-sbc-a510.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 91bd5bd62857..0b6d201e5ae7 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -545,12 +545,13 @@ dtb-$(CONFIG_MACH_ARMADA_XP) += \
 	armada-xp-netgear-rn2120.dtb \
 	armada-xp-openblocks-ax3-4.dtb \
 	armada-xp-synology-ds414.dtb
-dtb-$(CONFIG_MACH_DOVE) += dove-cm-a510.dtb \
+dtb-$(CONFIG_MACH_DOVE) += \
 	dove-cubox.dtb \
 	dove-cubox-es.dtb \
 	dove-d2plug.dtb \
 	dove-d3plug.dtb \
-	dove-dove-db.dtb
+	dove-dove-db.dtb \
+	dove-sbc-a510.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6589-aquaris5.dtb \
 	mt6592-evb.dtb \
 	mt8127-moose.dtb \
diff --git a/arch/arm/boot/dts/dove-cm-a510.dts b/arch/arm/boot/dts/dove-cm-a510.dts
deleted file mode 100644
index 50c0d6904497..000000000000
--- a/arch/arm/boot/dts/dove-cm-a510.dts
+++ /dev/null
@@ -1,38 +0,0 @@
-/dts-v1/;
-
-#include "dove.dtsi"
-
-/ {
-	model = "Compulab CM-A510";
-	compatible = "compulab,cm-a510", "marvell,dove";
-
-	memory {
-		device_type = "memory";
-		reg = <0x00000000 0x40000000>;
-	};
-
-	chosen {
-		bootargs = "console=ttyS0,115200n8 earlyprintk";
-	};
-};
-
-&uart0 { status = "okay"; };
-&uart1 { status = "okay"; };
-&sdio0 { status = "okay"; };
-&sdio1 { status = "okay"; };
-&sata0 { status = "okay"; };
-
-&spi0 {
-	status = "okay";
-
-	/* spi0.0: 4M Flash Winbond W25Q32BV */
-	spi-flash@0 {
-		compatible = "st,w25q32";
-		spi-max-frequency = <20000000>;
-		reg = <0>;
-	};
-};
-
-&i2c0 {
-	  status = "okay";
-};
diff --git a/arch/arm/boot/dts/dove-cm-a510.dtsi b/arch/arm/boot/dts/dove-cm-a510.dtsi
new file mode 100644
index 000000000000..ef670bd040d3
--- /dev/null
+++ b/arch/arm/boot/dts/dove-cm-a510.dtsi
@@ -0,0 +1,195 @@
+/*
+ * Device Tree include for Compulab CM-A510 System-on-Module
+ *
+ * Copyright (C) 2015, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/*
+ * The CM-A510 comes with several optional components:
+ *
+ * Memory options:
+ *  D512: 512M
+ *  D1024: 1G
+ *
+ * NAND options:
+ *  N512: 512M NAND
+ *
+ * Ethernet options:
+ *  E1: PHY RTL8211D on internal GbE (SMI address 0x03)
+ *  E2: Additional ethernet NIC RTL8111D on PCIe1
+ *
+ * Audio options:
+ *  A: TI TLV320AIC23b audio codec (I2C address 0x1a)
+ *
+ * Touchscreen options:
+ *  I: TI TSC2046 touchscreen controller (on SPI1)
+ *
+ * USB options:
+ *  U2: 2 dual-role USB2.0 ports
+ *  U4: 2 additional USB2.0 host ports (via USB1)
+ *
+ * WiFi options:
+ *  W: Broadcom BCM4319 802.11b/g/n (USI WM-N-BM-01 on SDIO1)
+ *
+ * GPIOs used on CM-A510:
+ *   1 GbE PHY reset (active low)
+ *   3 WakeUp
+ *   8 PowerOff (active low)
+ *  13 Touchscreen pen irq (active low)
+ *  65 System LED (active high)
+ *  69 USB Hub reset (active low)
+ *  70 WLAN reset (active low)
+ *  71 WLAN regulator (active high)
+ */
+
+#include "dove.dtsi"
+
+/ {
+	model = "Compulab CM-A510";
+	compatible = "compulab,cm-a510", "marvell,dove";
+
+	/*
+	 * Set the minimum memory size here and let the
+	 * bootloader set the real size.
+	 */
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x20000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+		/* Set upper NAND data bit to GPO */
+		pinctrl-0 = <&pmx_nand_gpo>;
+		pinctrl-names = "default";
+
+		system {
+			label = "cm-a510:system:green";
+			gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
+			default-state = "keep";
+		};
+	};
+
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		wifi_power: regulator@1 {
+			compatible = "regulator-fixed";
+			regulator-name = "WiFi Power";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			gpio = <&gpio2 7 GPIO_ACTIVE_HIGH>;
+		};
+	};
+};
+
+/* Optional RTL8211D GbE PHY on SMI address 0x03 */
+&ethphy {
+	reg = <3>;
+	status = "disabled";
+};
+
+&i2c0 {
+	/* Optional TI TLV320AIC23b audio codec */
+	opt_audio: audio@1a {
+		compatible = "ti,tlv320aic23";
+		reg = <0x1a>;
+		status = "disabled";
+	};
+};
+
+/* Optional RTL8111D GbE NIC on PCIe1 */
+&pcie { status = "disabled"; };
+
+&pcie1 {
+	pinctrl-0 = <&pmx_pcie1_clkreq>;
+	pinctrl-names = "default";
+	status = "disabled";
+};
+
+&pinctrl {
+	pmx_uart2: pmx-uart2 {
+		marvell,pins = "mpp14", "mpp15";
+		marvell,function = "uart2";
+	};
+};
+
+/* Optional Broadcom BCM4319 802.11b/g/n WiFi module */
+&sdio1 {
+	non-removable;
+	vmmc-supply = <&wifi_power>;
+	reset-gpio = <&gpio2 6 GPIO_ACTIVE_LOW>;
+	status = "disabled";
+};
+
+&spi0 {
+	status = "okay";
+
+	/* 1M Flash Winbond W25Q80BL */
+	flash@0 {
+		compatible = "winbond,w25q80";
+		spi-max-frequency = <80000000>;
+		reg = <0>;
+	};
+};
+
+&spi1 {
+	pinctrl-0 = <&pmx_spi1_20_23>;
+	pinctrl-names = "default";
+	status = "disabled";
+
+	/* Optional TI TSC2046 touchscreen controller */
+	opt_touch: touchscreen@0 {
+		compatible = "ti,tsc2046";
+		spi-max-frequency = <2500000>;
+		reg = <0>;
+		pinctrl-0 = <&pmx_gpio_13>;
+		pinctrl-names = "default";
+		interrupts-extended = <&gpio0 13 IRQ_TYPE_EDGE_FALLING>;
+	};
+};
+
+&uart2 {
+	pinctrl-0 = <&pmx_uart2>;
+	pinctrl-names = "default";
+};
diff --git a/arch/arm/boot/dts/dove-sbc-a510.dts b/arch/arm/boot/dts/dove-sbc-a510.dts
new file mode 100644
index 000000000000..38096259ecce
--- /dev/null
+++ b/arch/arm/boot/dts/dove-sbc-a510.dts
@@ -0,0 +1,182 @@
+/*
+ * Device Tree file for Compulab SBC-A510 Single Board Computer
+ *
+ * Copyright (C) 2015, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/*
+ * SBC-A510 comprises a PCA9555 I2C GPIO expander its GPIO lines connected to
+ *
+ * 0.0 USB0 VBUS_EN (active high)
+ * 0.1 USB0 VBUS_GOOD
+ * 0.2 DVI transmitter TI TFP410 MSEN
+ * 0.3 DVI transmitter TI TFP410 PD# (active low power down)
+ * 0.4 LVDS transmitter DS90C365 PD# (active low power down)
+ * 0.5 LCD nRST (active low reset)
+ * 0.6 PCIe0 nRST (active low reset)
+ * 0.7 mini-PCIe slot W_DISABLE#
+ *
+ * 1.0 MMC WP
+ * 1.1 Camera Input FPC FLASH_STB and P21.5
+ * 1.2 Camera Input FPC WE        and P21.22
+ * 1.3 MMC VCC_EN (active high)   and P21.7
+ * 1.4 Camera Input FPC AFTR_RST  and P21.17
+ * 1.5 Camera Input FPC OE        and P21.19
+ * 1.6 Camera Input FPC SNPSHT    and P21.6
+ * 1.7 Camera Input FPC SHTR      and P21.10
+ */
+
+/dts-v1/;
+
+#include "dove-cm-a510.dtsi"
+
+/ {
+	model = "Compulab SBC-A510";
+	compatible = "compulab,sbc-a510", "compulab,cm-a510", "marvell,dove";
+
+	chosen {
+		stdout-path = &uart0;
+	};
+
+	regulators {
+		usb0_power: regulator@2 {
+			compatible = "regulator-fixed";
+			regulator-name = "USB Power";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			gpio = <&gpio_ext 0 GPIO_ACTIVE_HIGH>;
+		};
+
+		mmc_power: regulator@3 {
+			compatible = "regulator-fixed";
+			regulator-name = "MMC Power";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			gpio = <&gpio_ext 13 GPIO_ACTIVE_HIGH>;
+		};
+	};
+};
+
+/* Ethernet0 depends on CM-A510 option E1 */
+&mdio { status = "disabled"; };
+&eth { status = "disabled"; };
+&ethphy { status = "disabled"; };
+
+/*
+ * USB port 0 can be powered and monitored by I2C GPIO expander:
+ *  VBUS_ENABLE on GPIO0, VBUS_GOOD on GPIO1
+ */
+&ehci0 {
+	status = "okay";
+	vbus-supply = <&usb0_power>;
+};
+
+/* USB port 1 (and ports 2, 3 if CM-A510 has U4 option) */
+&ehci1 { status = "okay"; };
+
+/*
+ * I2C bus layout:
+ * i2c0:
+ *  - Audio Codec, 0x1a (option from CM-A510)
+ *  - DVI transmitter TI TFP410, 0x39
+ *  - HDMI/DVI DDC channel
+ * i2c1:
+ *  - GPIO expander, NXP PCA9555, 0x20
+ *  - VGA DDC channel
+ */
+&i2c {
+	pinctrl-0 = <&pmx_i2c1>;
+	pinctrl-names = "default";
+};
+
+&i2c0 {
+	/* TI TFP410 DVI transmitter */
+	dvi: video@39 {
+		compatible = "ti,tfp410";
+		reg = <0x39>;
+		powerdown-gpio = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&i2c1 {
+	status = "okay";
+
+	/* NXP PCA9555 GPIO expander */
+	gpio_ext: gpio@20 {
+		compatible = "nxp,pca9555";
+		reg = <0x20>;
+		#gpio-cells = <2>;
+	};
+};
+
+&pcie { status = "okay"; };
+
+/*
+ * PCIe0 can be configured by Jumper E1 to be either connected to
+ * a mini-PCIe slot or a Pericom PI7C9X111 PCIe-to-PCI bridge.
+ */
+&pcie0 {
+	status = "okay";
+	pinctrl-0 = <&pmx_pcie0_clkreq>;
+	pinctrl-names = "default";
+	reset-gpios = <&gpio_ext 6 GPIO_ACTIVE_LOW>;
+};
+
+/* Ethernet1 depends on CM-A510 option E2 */
+&pcie1 { status = "disabled"; };
+
+/* SATA connector */
+&sata0 { status = "okay"; };
+
+/*
+ * SDIO0 is connected to a MMC/SD/SDIO socket, I2C GPIO expander has
+ *  VCC_MMC_ENABLE on GPIO13, MMC_WP on GPIO10
+ */
+&sdio0 {
+	vmmc-supply = <&mmc_power>;
+	wp-gpios = <&gpio_ext 10 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};
+
+/* UART0 on RS232 mini-connector */
+&uart0 { status = "okay"; };
+/* UART2 on pin headers */
+&uart2 { status = "okay"; };
-- 
2.1.0


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

* Re: [PATCH 2/8] devicetree: vendor-prefixes: Add CompuLab to known vendors
  2015-02-17 18:52 ` [PATCH 2/8] devicetree: vendor-prefixes: Add CompuLab to known vendors Sebastian Hesselbarth
@ 2015-02-17 19:37   ` Rob Herring
  0 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2015-02-17 19:37 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree, linux-arm-kernel, linux-kernel

On Tue, Feb 17, 2015 at 12:52 PM, Sebastian Hesselbarth
<sebastian.hesselbarth@gmail.com> wrote:
> This adds "compulab" as a vendor-prefix for CompuLab (compulab.co.il),
> an Israeli company that builds ARM-based SoMs and CoMs.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

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

> ---
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Gabriel Dobato <dobatog@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index d443279c95dc..2de628532d4b 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -35,6 +35,7 @@ chrp  Common Hardware Reference Platform
>  chunghwa       Chunghwa Picture Tubes Ltd.
>  cirrus Cirrus Logic, Inc.
>  cnm    Chips&Media, Inc.
> +compulab       CompuLab
>  cortina        Cortina Systems, Inc.
>  crystalfontz   Crystalfontz America, Inc.
>  dallas Maxim Integrated Products (formerly Dallas Semiconductor)
> --
> 2.1.0
>

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

* Re: [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-02-17 18:52 ` [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
@ 2015-02-17 20:46   ` Stephen Warren
  2015-02-17 21:08     ` Sebastian Hesselbarth
  2015-02-26 21:46   ` Stephen Warren
  1 sibling, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2015-02-17 20:46 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 02/17/2015 11:52 AM, Sebastian Hesselbarth wrote:
> I2C mux pinctrl driver currently determines the number of sub-busses by
> counting available pinctrl-names. Unfortunately, this requires each
> incarnation of the devicetree node with different available sub-busses
> to be rewritten.

Can you be more explicit about the problem here? Why does anything need 
to be re-written if a child node is disabled; presumably there's no need 
for the child bus numbers to be contiguous. In other words, with the 
example in the existing DT binding doc:

         i2cmux {
                 compatible = "i2c-mux-pinctrl";
...
                 pinctrl-names = "ddc", "pta", "idle";
                 pinctrl-0 = <&state_i2cmux_ddc>;
                 pinctrl-1 = <&state_i2cmux_pta>;
                 pinctrl-2 = <&state_i2cmux_idle>;

                 i2c@0 {
                         reg = <0>;
...
                 i2c@1 {
                         reg = <1>;
...

That would generate child busses 0 and 1. If I was to disable the i2c@0 
node, then there would still be definitions for child busses 0 and 1 in 
the DT, it's just that child bus 0 wouldn't actually exist at run-time. 
I don't see what part of DT needs to be re-written to accomodate this?

> This patch reworks i2c-mux-pinctrl driver to count the number of
> available sub-nodes instead. The rework should be compatible to the old
> way of probing for sub-busses and additionally allows to disable unused
> sub-busses with standard DT property status = "disabled".
>
> This also amends the corresponding devicetree binding documentation to
> reflect the new functionality to disable unused sub-nodes. While at it,
> also fix two references to binding documentation files that miss an "i2c-"
> prefix.

> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt

> -For each named state defined in the pinctrl-names property, an I2C child bus
> -will be created. I2C child bus numbers are assigned based on the index into
> -the pinctrl-names property.
> +For each child node that is not disabled by a status != "okay", an I2C
> +child bus will be created. I2C child bus numbers are assigned based on the
> +order of child nodes.

I would have assumed that disabled sub-nodes was a global concept within 
DT, and so wouldn't be mentioned in the binding. It would just be a bug 
in the driver if it didn't ignore disabled sub-nodes.




>
> -The only exception is that no bus will be created for a state named "idle". If
> -such a state is defined, it must be the last entry in pinctrl-names. For
> -example:
> -
> -	pinctrl-names = "ddc", "pta", "idle"  ->  ddc = bus 0, pta = bus 1
> -	pinctrl-names = "ddc", "idle", "pta"  ->  Invalid ("idle" not last)
> -	pinctrl-names = "idle", "ddc", "pta"  ->  Invalid ("idle" not last)
> +There must be a corresponding pinctrl-names entry for each enabled child
> +node at the position of the child node's "reg" property.

The addition there seems fine, but the existing text re: the idle state 
seems clearer in the original text.
>
>   Whenever an access is made to a device on a child bus, the relevant pinctrl
>   state will be programmed into hardware.
>
> -If an idle state is defined, whenever an access is not being made to a device
> -on a child bus, the idle pinctrl state will be programmed into hardware.
> +Also, there can be an idle pinctrl state defined at the end of possible
> +pinctrl states. If an idle state is defined, whenever an access is not being
> +made to a device on a child bus, the idle pinctrl state will be programmed
> +into hardware.


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

* Re: [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-02-17 20:46   ` Stephen Warren
@ 2015-02-17 21:08     ` Sebastian Hesselbarth
  2015-02-17 21:15       ` Stephen Warren
  0 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 21:08 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 17.02.2015 21:46, Stephen Warren wrote:
> On 02/17/2015 11:52 AM, Sebastian Hesselbarth wrote:
>> I2C mux pinctrl driver currently determines the number of sub-busses by
>> counting available pinctrl-names. Unfortunately, this requires each
>> incarnation of the devicetree node with different available sub-busses
>> to be rewritten.
>
> Can you be more explicit about the problem here? Why does anything need
> to be re-written if a child node is disabled; presumably there's no need
> for the child bus numbers to be contiguous. In other words, with the
> example in the existing DT binding doc:
>
>          i2cmux {
>                  compatible = "i2c-mux-pinctrl";
> ...
>                  pinctrl-names = "ddc", "pta", "idle";
>                  pinctrl-0 = <&state_i2cmux_ddc>;
>                  pinctrl-1 = <&state_i2cmux_pta>;
>                  pinctrl-2 = <&state_i2cmux_idle>;
>
>                  i2c@0 {
>                          reg = <0>;
> ...
>                  i2c@1 {
>                          reg = <1>;
> ...
>
> That would generate child busses 0 and 1. If I was to disable the i2c@0
> node, then there would still be definitions for child busses 0 and 1 in
> the DT, it's just that child bus 0 wouldn't actually exist at run-time.
> I don't see what part of DT needs to be re-written to accomodate this?

The way the current driver works, to disable i2c@0 you'd have to remove
the pinctrl-0 state, pinctrl-names string at position 0, and the node
itself.

So, on Dove SoC there is three sub-busses, now consider one board A with
i2c0 and i2c1 enabled but board B with i2c0 and i2c2 enabled:

board-A.dts:

i2cmux {
	pinctrl-names = "i2c0", "i2c1", "idle";
	pinctrl-0 = <&state_for_i2c0>;
	pinctrl-1 = <&state_for_i2c1>;
};

but

board-B.dts:

i2cmux {
	pinctrl-names = "i2c0", "i2c2", "idle";
	pinctrl-0 = <&state_for_i2c0>;
	pinctrl-1 = <&state_for_i2c2>;
	/* Note that this ^^^ is state_for_i2c2 */
};

while the approach with status = "disabled" allows all properties for
both board remain the same - except you'll enable either i2c1 or i2c2
sub-node on board level:

i2cmux {
	pinctrl-names = "i2c0", "i2c1", "i2c2", "idle";
	pinctrl-0 = <&state_for_i2c0>;
	pinctrl-1 = <&state_for_i2c1>;
	pinctrl-2 = <&state_for_i2c2>;
};

board-A.dts:

i2cmux {
	i2c@0 { status = "okay"; };
	i2c@1 { status = "okay"; };
};

and

board-B.dts:

i2cmux {
	i2c@0 { status = "okay"; };
	i2c@2 { status = "okay"; };
};

In general, it is less about the binding but how the driver is written:
Number of sub-busses is determined by elements in pinctrl-names not
available (enabled) sub-nodes.

>> This patch reworks i2c-mux-pinctrl driver to count the number of
>> available sub-nodes instead. The rework should be compatible to the old
>> way of probing for sub-busses and additionally allows to disable unused
>> sub-busses with standard DT property status = "disabled".
>>
>> This also amends the corresponding devicetree binding documentation to
>> reflect the new functionality to disable unused sub-nodes. While at it,
>> also fix two references to binding documentation files that miss an
>> "i2c-"
>> prefix.
>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
>> b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
>
>> -For each named state defined in the pinctrl-names property, an I2C
>> child bus
>> -will be created. I2C child bus numbers are assigned based on the
>> index into
>> -the pinctrl-names property.
>> +For each child node that is not disabled by a status != "okay", an I2C
>> +child bus will be created. I2C child bus numbers are assigned based
>> on the
>> +order of child nodes.
>
> I would have assumed that disabled sub-nodes was a global concept within
> DT, and so wouldn't be mentioned in the binding. It would just be a bug
> in the driver if it didn't ignore disabled sub-nodes.

Yep, the concept is very global. It is about the current driver and this
binding changes are just to make it a little more clear that the driver
should behave different, i.e. get rid of anything that implies that
pinctrl-names has any effect on the number of sub-busses registered.

>> -The only exception is that no bus will be created for a state named
>> "idle". If
>> -such a state is defined, it must be the last entry in pinctrl-names. For
>> -example:
>> -
>> -    pinctrl-names = "ddc", "pta", "idle"  ->  ddc = bus 0, pta = bus 1
>> -    pinctrl-names = "ddc", "idle", "pta"  ->  Invalid ("idle" not last)
>> -    pinctrl-names = "idle", "ddc", "pta"  ->  Invalid ("idle" not last)
>> +There must be a corresponding pinctrl-names entry for each enabled child
>> +node at the position of the child node's "reg" property.
>
> The addition there seems fine, but the existing text re: the idle state
> seems clearer in the original text.

Ok, I'll have a look at how to preserve this section better.

Do you still have one of the current boards available for testing?

Sebastian

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

* Re: [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-02-17 21:08     ` Sebastian Hesselbarth
@ 2015-02-17 21:15       ` Stephen Warren
  2015-02-17 21:19         ` Sebastian Hesselbarth
  0 siblings, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2015-02-17 21:15 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 02/17/2015 02:08 PM, Sebastian Hesselbarth wrote:
> On 17.02.2015 21:46, Stephen Warren wrote:
>> On 02/17/2015 11:52 AM, Sebastian Hesselbarth wrote:
>>> I2C mux pinctrl driver currently determines the number of sub-busses by
>>> counting available pinctrl-names. Unfortunately, this requires each
>>> incarnation of the devicetree node with different available sub-busses
>>> to be rewritten.
>>
>> Can you be more explicit about the problem here? Why does anything need
>> to be re-written if a child node is disabled; presumably there's no need
>> for the child bus numbers to be contiguous. In other words, with the
>> example in the existing DT binding doc:
>>
>>          i2cmux {
>>                  compatible = "i2c-mux-pinctrl";
>> ...
>>                  pinctrl-names = "ddc", "pta", "idle";
>>                  pinctrl-0 = <&state_i2cmux_ddc>;
>>                  pinctrl-1 = <&state_i2cmux_pta>;
>>                  pinctrl-2 = <&state_i2cmux_idle>;
>>
>>                  i2c@0 {
>>                          reg = <0>;
>> ...
>>                  i2c@1 {
>>                          reg = <1>;
>> ...
>>
>> That would generate child busses 0 and 1. If I was to disable the i2c@0
>> node, then there would still be definitions for child busses 0 and 1 in
>> the DT, it's just that child bus 0 wouldn't actually exist at run-time.
>> I don't see what part of DT needs to be re-written to accomodate this?
>
> The way the current driver works, to disable i2c@0 you'd have to remove
> the pinctrl-0 state, pinctrl-names string at position 0, and the node
> itself.
>
> So, on Dove SoC there is three sub-busses, now consider one board A with
> i2c0 and i2c1 enabled but board B with i2c0 and i2c2 enabled:
>
> board-A.dts:
>
> i2cmux {
>      pinctrl-names = "i2c0", "i2c1", "idle";
>      pinctrl-0 = <&state_for_i2c0>;
>      pinctrl-1 = <&state_for_i2c1>;
> };
>
> but
>
> board-B.dts:
>
> i2cmux {
>      pinctrl-names = "i2c0", "i2c2", "idle";
>      pinctrl-0 = <&state_for_i2c0>;
>      pinctrl-1 = <&state_for_i2c2>;
>      /* Note that this ^^^ is state_for_i2c2 */
> };
>
> while the approach with status = "disabled" allows all properties for
> both board remain the same - except you'll enable either i2c1 or i2c2
> sub-node on board level:
>
> i2cmux {
>      pinctrl-names = "i2c0", "i2c1", "i2c2", "idle";
>      pinctrl-0 = <&state_for_i2c0>;
>      pinctrl-1 = <&state_for_i2c1>;
>      pinctrl-2 = <&state_for_i2c2>;
> };
>
> board-A.dts:
>
> i2cmux {
>      i2c@0 { status = "okay"; };
>      i2c@1 { status = "okay"; };
> };
>
> and
>
> board-B.dts:
>
> i2cmux {
>      i2c@0 { status = "okay"; };
>      i2c@2 { status = "okay"; };
> };

OK, that all makes sense, but I don't think there's any change at all to 
the binding; this can all be fixed in the driver without affecting the 
definition of the binding at all. At most all that's needed in the 
binding is a note to the effect that if a particular child node is 
disabled, then this has no effect at all on the requirements for the 
pinctrl properties.

> In general, it is less about the binding but how the driver is written:
> Number of sub-busses is determined by elements in pinctrl-names not
> available (enabled) sub-nodes.
>
>>> This patch reworks i2c-mux-pinctrl driver to count the number of
>>> available sub-nodes instead. The rework should be compatible to the old
>>> way of probing for sub-busses and additionally allows to disable unused
>>> sub-busses with standard DT property status = "disabled".
>>>
>>> This also amends the corresponding devicetree binding documentation to
>>> reflect the new functionality to disable unused sub-nodes. While at it,
>>> also fix two references to binding documentation files that miss an
>>> "i2c-"
>>> prefix.
>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
>>> b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
>>
>>> -For each named state defined in the pinctrl-names property, an I2C
>>> child bus
>>> -will be created. I2C child bus numbers are assigned based on the
>>> index into
>>> -the pinctrl-names property.
>>> +For each child node that is not disabled by a status != "okay", an I2C
>>> +child bus will be created. I2C child bus numbers are assigned based
>>> on the
>>> +order of child nodes.
>>
>> I would have assumed that disabled sub-nodes was a global concept within
>> DT, and so wouldn't be mentioned in the binding. It would just be a bug
>> in the driver if it didn't ignore disabled sub-nodes.
>
> Yep, the concept is very global. It is about the current driver and this
> binding changes are just to make it a little more clear that the driver
> should behave different, i.e. get rid of anything that implies that
> pinctrl-names has any effect on the number of sub-busses registered.
>
>>> -The only exception is that no bus will be created for a state named
>>> "idle". If
>>> -such a state is defined, it must be the last entry in pinctrl-names.
>>> For
>>> -example:
>>> -
>>> -    pinctrl-names = "ddc", "pta", "idle"  ->  ddc = bus 0, pta = bus 1
>>> -    pinctrl-names = "ddc", "idle", "pta"  ->  Invalid ("idle" not last)
>>> -    pinctrl-names = "idle", "ddc", "pta"  ->  Invalid ("idle" not last)
>>> +There must be a corresponding pinctrl-names entry for each enabled
>>> child
>>> +node at the position of the child node's "reg" property.
>>
>> The addition there seems fine, but the existing text re: the idle state
>> seems clearer in the original text.
>
> Ok, I'll have a look at how to preserve this section better.
>
> Do you still have one of the current boards available for testing?

Yes, I have both Seaboard and Ventana still (the two Tegra boards that 
use this driver). I haven't used them in a while; I hope they still work:-)

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

* Re: [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-02-17 21:15       ` Stephen Warren
@ 2015-02-17 21:19         ` Sebastian Hesselbarth
  0 siblings, 0 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-17 21:19 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 17.02.2015 22:15, Stephen Warren wrote:
> On 02/17/2015 02:08 PM, Sebastian Hesselbarth wrote:
>> On 17.02.2015 21:46, Stephen Warren wrote:
>>> Can you be more explicit about the problem here? Why does anything need
>>> to be re-written if a child node is disabled; presumably there's no need
>>> for the child bus numbers to be contiguous. In other words, with the
>>> example in the existing DT binding doc:
[...]
>>
>> The way the current driver works, to disable i2c@0 you'd have to remove
>> the pinctrl-0 state, pinctrl-names string at position 0, and the node
>> itself.
[...]
> OK, that all makes sense, but I don't think there's any change at all to
> the binding; this can all be fixed in the driver without affecting the
> definition of the binding at all. At most all that's needed in the
> binding is a note to the effect that if a particular child node is
> disabled, then this has no effect at all on the requirements for the
> pinctrl properties.

I totally agree that the binding is not affected at all. I was under the
impression that the current binding doc justifies the way the driver is
currently parsing the properties. So this was an attempt to reword it
to make it more clear what properties should influence the way
sub-busses are registered.

I'll have another look at the binding doc when the driver is fine.

[...]
>> Do you still have one of the current boards available for testing?
>
> Yes, I have both Seaboard and Ventana still (the two Tegra boards that
> use this driver). I haven't used them in a while; I hope they still work:-)

Ok, I cross my fingers too and expect a Tested-by :)

Sebastian

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

* Re: [PATCH 3/8] ARM: dts: dove: Fix uart[23] reg property
  2015-02-17 18:52 ` [PATCH 3/8] ARM: dts: dove: Fix uart[23] reg property Sebastian Hesselbarth
@ 2015-02-23 14:54   ` Gregory CLEMENT
  0 siblings, 0 replies; 50+ messages in thread
From: Gregory CLEMENT @ 2015-02-23 14:54 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gabriel Dobato, linux-arm-kernel,
	linux-kernel

Hi Sebastian,

On 17/02/2015 19:52, Sebastian Hesselbarth wrote:
> Fix Dove's register addresses of uart2 and uart3 nodes that seem to
> be broken since ages due to a copy-and-paste error.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>


> ---
> Please consider for stable back to v3.7

If nobody complains about it, I will add the Cc tag for
the stable team when applying this patch to the mvebu tree


Thanks,

Gregory

> 
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Gabriel Dobato <dobatog@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/arm/boot/dts/dove.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
> index a5441d5482a6..3cc8b8320345 100644
> --- a/arch/arm/boot/dts/dove.dtsi
> +++ b/arch/arm/boot/dts/dove.dtsi
> @@ -154,7 +154,7 @@
>  
>  			uart2: serial@12200 {
>  				compatible = "ns16550a";
> -				reg = <0x12000 0x100>;
> +				reg = <0x12200 0x100>;
>  				reg-shift = <2>;
>  				interrupts = <9>;
>  				clocks = <&core_clk 0>;
> @@ -163,7 +163,7 @@
>  
>  			uart3: serial@12300 {
>  				compatible = "ns16550a";
> -				reg = <0x12100 0x100>;
> +				reg = <0x12300 0x100>;
>  				reg-shift = <2>;
>  				interrupts = <10>;
>  				clocks = <&core_clk 0>;
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCH 4/8] ARM: dts: dove: Always include gpio and interrupt-controller headers
  2015-02-17 18:52 ` [PATCH 4/8] ARM: dts: dove: Always include gpio and interrupt-controller headers Sebastian Hesselbarth
@ 2015-02-23 14:54   ` Gregory CLEMENT
  0 siblings, 0 replies; 50+ messages in thread
From: Gregory CLEMENT @ 2015-02-23 14:54 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gabriel Dobato, linux-arm-kernel,
	linux-kernel

Hi Sebastian,

On 17/02/2015 19:52, Sebastian Hesselbarth wrote:
> We want to enforce the use of named flags in GPIO and interrupt
> specifiers, include the corresponding headers to Dove's SoC dtsi.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>


Thanks,

Gregory

> ---
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Gabriel Dobato <dobatog@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/arm/boot/dts/dove.dtsi | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
> index 3cc8b8320345..cae865435762 100644
> --- a/arch/arm/boot/dts/dove.dtsi
> +++ b/arch/arm/boot/dts/dove.dtsi
> @@ -1,5 +1,8 @@
>  /include/ "skeleton.dtsi"
>  
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
>  #define MBUS_ID(target,attributes) (((target) << 24) | ((attributes) << 16))
>  
>  / {
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCH 5/8] ARM: dts: dove: Add node labels for PCIe ports 0 and 1
  2015-02-17 18:52 ` [PATCH 5/8] ARM: dts: dove: Add node labels for PCIe ports 0 and 1 Sebastian Hesselbarth
@ 2015-02-23 14:55   ` Gregory CLEMENT
  0 siblings, 0 replies; 50+ messages in thread
From: Gregory CLEMENT @ 2015-02-23 14:55 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gabriel Dobato, linux-arm-kernel,
	linux-kernel

Hi Sebastian,


On 17/02/2015 19:52, Sebastian Hesselbarth wrote:
> Add pcie[01] node labels to allow to reference them easily from
> board level.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>


Thanks,

Gregory

> ---
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Gabriel Dobato <dobatog@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/arm/boot/dts/dove.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
> index cae865435762..81209bd9525a 100644
> --- a/arch/arm/boot/dts/dove.dtsi
> +++ b/arch/arm/boot/dts/dove.dtsi
> @@ -64,7 +64,7 @@
>  				  0x82000000 0x2 0x0 MBUS_ID(0x08, 0xe8) 0 1 0   /* Port 1.0 Mem */
>  				  0x81000000 0x2 0x0 MBUS_ID(0x08, 0xe0) 0 1 0>; /* Port 1.0 I/O */
>  
> -			pcie-port@0 {
> +			pcie0: pcie-port@0 {
>  				device_type = "pci";
>  				status = "disabled";
>  				assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> @@ -82,7 +82,7 @@
>  				interrupt-map = <0 0 0 0 &intc 16>;
>  			};
>  
> -			pcie-port@1 {
> +			pcie1: pcie-port@1 {
>  				device_type = "pci";
>  				status = "disabled";
>  				assigned-addresses = <0x82002800 0 0x80000 0 0x2000>;
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCH 6/8] ARM: dts: dove: Add some more common pinctrl settings
  2015-02-17 18:52 ` [PATCH 6/8] ARM: dts: dove: Add some more common pinctrl settings Sebastian Hesselbarth
@ 2015-02-23 14:56   ` Gregory CLEMENT
  0 siblings, 0 replies; 50+ messages in thread
From: Gregory CLEMENT @ 2015-02-23 14:56 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gabriel Dobato, linux-arm-kernel,
	linux-kernel

Hi Sebastian,

On 17/02/2015 19:52, Sebastian Hesselbarth wrote:
> This add common pinctrl settings for pcie[01]_clkreq, spi1, i2c[23],
> and internal i2c mux. These settings have either one or two options
> only, so put them into the SoC dtsi instead of repeating them on
> board level.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>


Thanks,

Gregory

> ---
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Gabriel Dobato <dobatog@gmail.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/arm/boot/dts/dove.dtsi | 52 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
> index 81209bd9525a..9ad829523a13 100644
> --- a/arch/arm/boot/dts/dove.dtsi
> +++ b/arch/arm/boot/dts/dove.dtsi
> @@ -451,6 +451,11 @@
>  					marvell,function = "gpio";
>  				};
>  
> +				pmx_pcie1_clkreq: pmx-pcie1-clkreq {
> +					marvell,pins = "mpp9";
> +					marvell,function = "pex1";
> +				};
> +
>  				pmx_gpio_10: pmx-gpio-10 {
>  					marvell,pins = "mpp10";
>  					marvell,function = "gpio";
> @@ -461,6 +466,11 @@
>  					marvell,function = "gpio";
>  				};
>  
> +				pmx_pcie0_clkreq: pmx-pcie0-clkreq {
> +					marvell,pins = "mpp11";
> +					marvell,function = "pex0";
> +				};
> +
>  				pmx_gpio_12: pmx-gpio-12 {
>  					marvell,pins = "mpp12";
>  					marvell,function = "gpio";
> @@ -566,6 +576,18 @@
>  					marvell,function = "gpio";
>  				};
>  
> +				pmx_spi1_4_7: pmx-spi1-4-7 {
> +					marvell,pins = "mpp4", "mpp5",
> +						"mpp6", "mpp7";
> +					marvell,function = "spi1";
> +				};
> +
> +				pmx_spi1_20_23: pmx-spi1-20-23 {
> +					marvell,pins = "mpp20", "mpp21",
> +						"mpp22", "mpp23";
> +					marvell,function = "spi1";
> +				};
> +
>  				pmx_uart1: pmx-uart1 {
>  					marvell,pins = "mpp_uart1";
>  					marvell,function = "uart1";
> @@ -585,6 +607,36 @@
>  					marvell,pins = "mpp_nand";
>  					marvell,function = "gpo";
>  				};
> +
> +				pmx_i2c1: pmx-i2c1 {
> +					marvell,pins = "mpp17", "mpp19";
> +					marvell,function = "twsi";
> +				};
> +
> +				pmx_i2c2: pmx-i2c2 {
> +					marvell,pins = "mpp_audio1";
> +					marvell,function = "twsi";
> +				};
> +
> +				pmx_ssp_i2c2: pmx-ssp-i2c2 {
> +					marvell,pins = "mpp_audio1";
> +					marvell,function = "ssp/twsi";
> +				};
> +
> +				pmx_i2cmux_0: pmx-i2cmux-0 {
> +					marvell,pins = "twsi";
> +					marvell,function = "twsi-opt1";
> +				};
> +
> +				pmx_i2cmux_1: pmx-i2cmux-1 {
> +					marvell,pins = "twsi";
> +					marvell,function = "twsi-opt2";
> +				};
> +
> +				pmx_i2cmux_2: pmx-i2cmux-2 {
> +					marvell,pins = "twsi";
> +					marvell,function = "twsi-opt3";
> +				};
>  			};
>  
>  			core_clk: core-clocks@d0214 {
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCH 7/8] ARM: dts: dove: Add internal i2c multiplexer node
  2015-02-17 18:52 ` [PATCH 7/8] ARM: dts: dove: Add internal i2c multiplexer node Sebastian Hesselbarth
@ 2015-02-23 15:07   ` Gregory CLEMENT
  0 siblings, 0 replies; 50+ messages in thread
From: Gregory CLEMENT @ 2015-02-23 15:07 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gabriel Dobato, Wolfram Sang,
	Stephen Warren, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

Hi Sebastian,

On 17/02/2015 19:52, Sebastian Hesselbarth wrote:
> This adds a i2c-mux-pinctrl node to dove.dtsi for the internal i2c
> mux found on Dove SoCs. Up to now, we had no board using any of the
> two additional i2c busses, so make sure the change does not break
> any existing boards.
> 
> Therefore, we rename the i2c-controller node label to "i2c" and
> enable it by default. Also, the dedicated sub-bus (now "i2c0") is
> enabled by default. The two optional sub-busses require additional
> external pin-muxing, so disable them by default.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>


Thanks,

Gregory

> ---
> Wolfram,
> 
> Actually, I was hoping that default pin hog mechanism
> (pinctrl-names = "default") could also be used from i2c mux nodes
> and devices. Anyway, I had a look at i2c-core/mux code and failed
> how to achieve that easily. Instead I decided, it would also be ok
> to put the pin hog into the i2c controller node where pins will be
> bound by standard platform device code.
> 
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Gabriel Dobato <dobatog@gmail.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: linux-i2c@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  arch/arm/boot/dts/dove.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
> index 9ad829523a13..b3340e862b0e 100644
> --- a/arch/arm/boot/dts/dove.dtsi
> +++ b/arch/arm/boot/dts/dove.dtsi
> @@ -28,6 +28,42 @@
>  		};
>  	};
>  
> +	i2c-mux {
> +		compatible = "i2c-mux-pinctrl";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		i2c-parent = <&i2c>;
> +
> +		pinctrl-names = "i2c0", "i2c1", "i2c2";
> +		pinctrl-0 = <&pmx_i2cmux_0>;
> +		pinctrl-1 = <&pmx_i2cmux_1>;
> +		pinctrl-2 = <&pmx_i2cmux_2>;
> +
> +		i2c0: i2c@0 {
> +			reg = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "okay";
> +		};
> +
> +		i2c1: i2c@1 {
> +			reg = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			/* Requires pmx_i2c1 on i2c controller node */
> +			status = "disabled";
> +		};
> +
> +		i2c2: i2c@2 {
> +			reg = <2>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			/* Requires pmx_i2c2 on i2c controller node */
> +			status = "disabled";
> +		};
> +	};
> +
>  	l2: l2-cache {
>  		compatible = "marvell,tauros2-cache";
>  		marvell,tauros2-cache-features = <0>;
> @@ -123,7 +159,7 @@
>  				status = "disabled";
>  			};
>  
> -			i2c0: i2c-ctrl@11000 {
> +			i2c: i2c-ctrl@11000 {
>  				compatible = "marvell,mv64xxx-i2c";
>  				reg = <0x11000 0x20>;
>  				#address-cells = <1>;
> @@ -132,7 +168,7 @@
>  				clock-frequency = <400000>;
>  				timeout-ms = <1000>;
>  				clocks = <&core_clk 0>;
> -				status = "disabled";
> +				status = "okay";
>  			};
>  
>  			uart0: serial@12000 {
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510
  2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
                   ` (7 preceding siblings ...)
  2015-02-17 18:52 ` [PATCH 8/8] ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
@ 2015-02-26 17:55 ` Gregory CLEMENT
  2015-02-26 19:39   ` Sebastian Hesselbarth
  2015-02-27 12:24 ` [PATCH v2 0/4] " Sebastian Hesselbarth
  9 siblings, 1 reply; 50+ messages in thread
From: Gregory CLEMENT @ 2015-02-26 17:55 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gabriel Dobato, Wolfram Sang,
	Stephen Warren, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

Hi Sebastian,

On 17/02/2015 19:52, Sebastian Hesselbarth wrote:
> This patch set improves current mainline support for the Compulab
> CM-A510 System-on-Module (SoM) and its default Compulab SBC-A510
> base board. Thanks to Gabriel Dobato who agreed to remote debug and
> test the provided DT changes.
> 
> On the way to proper support, we
> - Rework i2c-mux-pinctrl to honor disabled sub-bus nodes
> - Add missing "compulab" vendor prefix
> - Fix broken uart[23] reg properties
> - Beautify Dove's dtsi files by adding gpio/irq includes, node
>   labels for pcie, and some additional pinctrl settings
> - Add a node for the internal i2c mux mechanism on Dove SoCs
> 
> And finally add a DT include for the Compulab CM-A510 SoM and a DT
> board file for the SBC-A510 base board.
> 
> Patches are based on stable v3.19 and are indended for the next
> merge window (either v3.21 or v4.1). Compulab related changes have
> been tested by Gabriel Dobato, I tested on SolidRun CuBox that it
> does not break existing Dove boards.
> 
> For the i2c-mux-pinctrl, a Tested-by from any user of Tegra20
> Seaboard, Tamonten, or Ventana would be nice to see if it is fully
> compatible.
> 
> I have added MVEBU maintainers to all patches, Wolfram for i2c and
> Stephen as the i2c-mux-pinctrl author i2c-mux-pinctrl related patches,
> and corresponding lists. As there is no important DT work in here,
> I decided to not explicitly add each of the DT maintainers except for
> the vendor prefix patch.
> 
> Sebastian Hesselbarth (8):
>   i2c: mux-pinctrl: Rework to honor disabled child nodes
>   devicetree: vendor-prefixes: Add CompuLab to known vendors
>   ARM: dts: dove: Fix uart[23] reg property
>   ARM: dts: dove: Always include gpio and interrupt-controller headers
>   ARM: dts: dove: Add node labels for PCIe ports 0 and 1
>   ARM: dts: dove: Add some more common pinctrl settings
>   ARM: dts: dove: Add internal i2c multiplexer node
>   ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510

Patches 3 to 6 applied on mvebu/dt

If I understood well patches 7 and 8 depend on patch 1 which had not been
taken yet.


Thanks,

Gregory


> 
>  .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt    |  28 +--
>  .../devicetree/bindings/vendor-prefixes.txt        |   1 +
>  arch/arm/boot/dts/Makefile                         |   5 +-
>  arch/arm/boot/dts/dove-cm-a510.dts                 |  38 ----
>  arch/arm/boot/dts/dove-cm-a510.dtsi                | 195 +++++++++++++++++++++
>  arch/arm/boot/dts/dove-sbc-a510.dts                | 182 +++++++++++++++++++
>  arch/arm/boot/dts/dove.dtsi                        | 103 ++++++++++-
>  drivers/i2c/muxes/i2c-mux-pinctrl.c                |  70 +++++---
>  8 files changed, 537 insertions(+), 85 deletions(-)
>  delete mode 100644 arch/arm/boot/dts/dove-cm-a510.dts
>  create mode 100644 arch/arm/boot/dts/dove-cm-a510.dtsi
>  create mode 100644 arch/arm/boot/dts/dove-sbc-a510.dts
> 
> ---
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Gabriel Dobato <dobatog@gmail.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: linux-i2c@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

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

* Re: [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510
  2015-02-26 17:55 ` [PATCH 0/8] " Gregory CLEMENT
@ 2015-02-26 19:39   ` Sebastian Hesselbarth
  2015-02-26 20:01     ` Stephen Warren
  0 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-26 19:39 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Jason Cooper, Andrew Lunn, Gabriel Dobato, Wolfram Sang,
	Stephen Warren, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 26.02.2015 18:55, Gregory CLEMENT wrote:
> On 17/02/2015 19:52, Sebastian Hesselbarth wrote:
>> This patch set improves current mainline support for the Compulab
>> CM-A510 System-on-Module (SoM) and its default Compulab SBC-A510
>> base board. Thanks to Gabriel Dobato who agreed to remote debug and
>> test the provided DT changes.
[...]
>> Sebastian Hesselbarth (8):
>>    i2c: mux-pinctrl: Rework to honor disabled child nodes
>>    devicetree: vendor-prefixes: Add CompuLab to known vendors
>>    ARM: dts: dove: Fix uart[23] reg property
>>    ARM: dts: dove: Always include gpio and interrupt-controller headers
>>    ARM: dts: dove: Add node labels for PCIe ports 0 and 1
>>    ARM: dts: dove: Add some more common pinctrl settings
>>    ARM: dts: dove: Add internal i2c multiplexer node
>>    ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510
>
> Patches 3 to 6 applied on mvebu/dt
>
> If I understood well patches 7 and 8 depend on patch 1 which had not been
> taken yet.

Correct. Once I get a Tested-by from Stephen, I'll resend the 4
remaining patches.

Thanks,
   Sebastian

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

* Re: [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510
  2015-02-26 19:39   ` Sebastian Hesselbarth
@ 2015-02-26 20:01     ` Stephen Warren
  2015-02-26 20:35       ` Sebastian Hesselbarth
  0 siblings, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2015-02-26 20:01 UTC (permalink / raw)
  To: Sebastian Hesselbarth, Gregory CLEMENT
  Cc: Jason Cooper, Andrew Lunn, Gabriel Dobato, Wolfram Sang,
	linux-i2c, devicetree, linux-arm-kernel, linux-kernel

On 02/26/2015 12:39 PM, Sebastian Hesselbarth wrote:
> On 26.02.2015 18:55, Gregory CLEMENT wrote:
>> On 17/02/2015 19:52, Sebastian Hesselbarth wrote:
>>> This patch set improves current mainline support for the Compulab
>>> CM-A510 System-on-Module (SoM) and its default Compulab SBC-A510
>>> base board. Thanks to Gabriel Dobato who agreed to remote debug and
>>> test the provided DT changes.
> [...]
>>> Sebastian Hesselbarth (8):
>>>    i2c: mux-pinctrl: Rework to honor disabled child nodes
>>>    devicetree: vendor-prefixes: Add CompuLab to known vendors
>>>    ARM: dts: dove: Fix uart[23] reg property
>>>    ARM: dts: dove: Always include gpio and interrupt-controller headers
>>>    ARM: dts: dove: Add node labels for PCIe ports 0 and 1
>>>    ARM: dts: dove: Add some more common pinctrl settings
>>>    ARM: dts: dove: Add internal i2c multiplexer node
>>>    ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510
>>
>> Patches 3 to 6 applied on mvebu/dt
>>
>> If I understood well patches 7 and 8 depend on patch 1 which had not been
>> taken yet.
>
> Correct. Once I get a Tested-by from Stephen, I'll resend the 4
> remaining patches.

Me? For which patch? I thought there was going to be a v2 of the i2c mux 
driver patch given the comments on it, but perhaps I'm misremembering?

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

* Re: [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510
  2015-02-26 20:01     ` Stephen Warren
@ 2015-02-26 20:35       ` Sebastian Hesselbarth
  0 siblings, 0 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-26 20:35 UTC (permalink / raw)
  To: Stephen Warren, Gregory CLEMENT
  Cc: Jason Cooper, Andrew Lunn, Gabriel Dobato, Wolfram Sang,
	linux-i2c, devicetree, linux-arm-kernel, linux-kernel

On 26.02.2015 21:01, Stephen Warren wrote:
> On 02/26/2015 12:39 PM, Sebastian Hesselbarth wrote:
>> On 26.02.2015 18:55, Gregory CLEMENT wrote:
>>> On 17/02/2015 19:52, Sebastian Hesselbarth wrote:
>>>> This patch set improves current mainline support for the Compulab
>>>> CM-A510 System-on-Module (SoM) and its default Compulab SBC-A510
>>>> base board. Thanks to Gabriel Dobato who agreed to remote debug and
>>>> test the provided DT changes.
>> [...]
>>>> Sebastian Hesselbarth (8):
>>>>    i2c: mux-pinctrl: Rework to honor disabled child nodes
>>>>    devicetree: vendor-prefixes: Add CompuLab to known vendors
>>>>    ARM: dts: dove: Fix uart[23] reg property
>>>>    ARM: dts: dove: Always include gpio and interrupt-controller headers
>>>>    ARM: dts: dove: Add node labels for PCIe ports 0 and 1
>>>>    ARM: dts: dove: Add some more common pinctrl settings
>>>>    ARM: dts: dove: Add internal i2c multiplexer node
>>>>    ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510
>>>
>>> Patches 3 to 6 applied on mvebu/dt
>>>
>>> If I understood well patches 7 and 8 depend on patch 1 which had not
>>> been
>>> taken yet.
>>
>> Correct. Once I get a Tested-by from Stephen, I'll resend the 4
>> remaining patches.
>
> Me? For which patch? I thought there was going to be a v2 of the i2c mux
> driver patch given the comments on it, but perhaps I'm misremembering?

AFAIKS, your concerns were addressed to the wording of the binding doc
changes. You don't need that ones to be taken care of before you can
give the i2c-mux-pinctrl driver a functional test, do you? ;)

FWIW, it would be great if you can give patch 1 a test run on one of
the tegra boards using i2c-mux-pinctrl. I'll address the wording
comments if there is no issues with the driver changes.

Sebastian


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

* Re: [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-02-17 18:52 ` [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
  2015-02-17 20:46   ` Stephen Warren
@ 2015-02-26 21:46   ` Stephen Warren
  1 sibling, 0 replies; 50+ messages in thread
From: Stephen Warren @ 2015-02-26 21:46 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 02/17/2015 11:52 AM, Sebastian Hesselbarth wrote:
> I2C mux pinctrl driver currently determines the number of sub-busses by
> counting available pinctrl-names. Unfortunately, this requires each
> incarnation of the devicetree node with different available sub-busses
> to be rewritten.
>
> This patch reworks i2c-mux-pinctrl driver to count the number of
> available sub-nodes instead. The rework should be compatible to the old
> way of probing for sub-busses and additionally allows to disable unused
> sub-busses with standard DT property status = "disabled".
>
> This also amends the corresponding devicetree binding documentation to
> reflect the new functionality to disable unused sub-nodes. While at it,
> also fix two references to binding documentation files that miss an "i2c-"
> prefix.

Tested-by: Stephen Warren <swarren@nvidia.com>

(Both the HDMI and LCD panel DDC busses on NVIDIA Tegra 
Springbank/Seaboard, running next-20150224)

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

* [PATCH v2 0/4] Add proper support for Compulab CM-A510/SBC-A510
  2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
                   ` (8 preceding siblings ...)
  2015-02-26 17:55 ` [PATCH 0/8] " Gregory CLEMENT
@ 2015-02-27 12:24 ` Sebastian Hesselbarth
  2015-02-27 12:24   ` [PATCH v2 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
                     ` (3 more replies)
  9 siblings, 4 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-27 12:24 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, Stephen Warren, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel

This is v2 of the patch set to improve current mainline support for the
Compulab CM-A510 System-on-Module (SoM) and its default Compulab SBC-A510
base board. 

Compared to v1 there have been the following changes:
- removed preparing Dove patches that already have been taken by MVEBU
  maintainers.
- added a Tested-by for i2c-mux-pinctrl changes from Stepen Warren.
- reworded i2c-mux-pinctrl devicetree doc changes
  (Suggested by Stephen Warren).
- Added an Acked-by for the vendor-prefix patch from Rob Herring.
- Added an Acked-by for the addition of i2c mux node to dove.dtsi from
  Gregory Clement.
- Rebased on top of v4.0-rc1 with preparing patches applied.

Patches are based on v4.0-rc1 with preparing patches for Dove applied
and are intended for v4.1.

Sebastian Hesselbarth (4):
  i2c: mux-pinctrl: Rework to honor disabled child nodes
  devicetree: vendor-prefixes: Add CompuLab to known vendors
  ARM: dts: dove: Add internal i2c multiplexer node
  ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510

 .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt    |  25 +--
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/arm/boot/dts/Makefile                         |   4 +-
 arch/arm/boot/dts/dove-cm-a510.dts                 |  38 ----
 arch/arm/boot/dts/dove-cm-a510.dtsi                | 195 +++++++++++++++++++++
 arch/arm/boot/dts/dove-sbc-a510.dts                | 182 +++++++++++++++++++
 arch/arm/boot/dts/dove.dtsi                        |  40 ++++-
 drivers/i2c/muxes/i2c-mux-pinctrl.c                |  70 +++++---
 8 files changed, 477 insertions(+), 78 deletions(-)
 delete mode 100644 arch/arm/boot/dts/dove-cm-a510.dts
 create mode 100644 arch/arm/boot/dts/dove-cm-a510.dtsi
 create mode 100644 arch/arm/boot/dts/dove-sbc-a510.dts

---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
-- 
2.1.0


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

* [PATCH v2 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-02-27 12:24 ` [PATCH v2 0/4] " Sebastian Hesselbarth
@ 2015-02-27 12:24   ` Sebastian Hesselbarth
  2015-03-02 20:01     ` Stephen Warren
  2015-03-09 12:21     ` [PATCH v3 " Sebastian Hesselbarth
  2015-02-27 12:24   ` [PATCH v2 2/4] devicetree: vendor-prefixes: Add CompuLab to known vendors Sebastian Hesselbarth
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-27 12:24 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, Stephen Warren, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel

I2C mux pinctrl driver currently determines the number of sub-busses by
counting available pinctrl-names. Unfortunately, this requires each
incarnation of the devicetree node with different available sub-busses
to be rewritten.

This patch reworks i2c-mux-pinctrl driver to count the number of
available sub-nodes instead. The rework should be compatible to the old
way of probing for sub-busses and additionally allows to disable unused
sub-busses with standard DT property status = "disabled".

This also amends the corresponding devicetree binding documentation to
reflect the new functionality to disable unused sub-nodes. While at it,
also fix two references to binding documentation files that miss an "i2c-"
prefix.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt    | 25 ++++----
 drivers/i2c/muxes/i2c-mux-pinctrl.c                | 70 ++++++++++++++--------
 2 files changed, 59 insertions(+), 36 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
index ae8af1694e95..40cf6d86f935 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
@@ -28,21 +28,21 @@ Also required are:
 * Standard pinctrl properties that specify the pin mux state for each child
   bus. See ../pinctrl/pinctrl-bindings.txt.
 
-* Standard I2C mux properties. See mux.txt in this directory.
+* Standard I2C mux properties. See i2c-mux.txt in this directory.
 
-* I2C child bus nodes. See mux.txt in this directory.
+* I2C child bus nodes. See i2c-mux.txt in this directory.
 
-For each named state defined in the pinctrl-names property, an I2C child bus
-will be created. I2C child bus numbers are assigned based on the index into
-the pinctrl-names property.
+For each enabled child node an I2C child bus will be created. I2C child bus
+numbers are assigned based on the order of child nodes.
 
-The only exception is that no bus will be created for a state named "idle". If
-such a state is defined, it must be the last entry in pinctrl-names. For
-example:
+There must be a corresponding pinctrl-names entry for each enabled child
+node at the position of the child node's "reg" property. Also, there can be
+an idle pinctrl state defined at the end of possible pinctrl states. If such
+a state is defined, it must be the last entry in pinctrl-names. For example:
 
-	pinctrl-names = "ddc", "pta", "idle"  ->  ddc = bus 0, pta = bus 1
-	pinctrl-names = "ddc", "idle", "pta"  ->  Invalid ("idle" not last)
-	pinctrl-names = "idle", "ddc", "pta"  ->  Invalid ("idle" not last)
+       pinctrl-names = "ddc", "pta", "idle"  ->  ddc = bus 0, pta = bus 1
+       pinctrl-names = "ddc", "idle", "pta"  ->  Invalid ("idle" not last)
+       pinctrl-names = "idle", "ddc", "pta"  ->  Invalid ("idle" not last)
 
 Whenever an access is made to a device on a child bus, the relevant pinctrl
 state will be programmed into hardware.
@@ -68,6 +68,7 @@ Example:
 		pinctrl-1 = <&state_i2cmux_pta>;
 		pinctrl-2 = <&state_i2cmux_idle>;
 
+		/* Enabled child bus 0 */
 		i2c@0 {
 			reg = <0>;
 			#address-cells = <1>;
@@ -79,10 +80,12 @@ Example:
 			};
 		};
 
+		/* Disabled child bus 1 */
 		i2c@1 {
 			reg = <1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			status = "disabled";
 
 			eeprom {
 				compatible = "eeprom";
diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
index b48378c4b40d..033dacfabfdf 100644
--- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
@@ -56,9 +56,12 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
 				struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	int num_names, i, ret;
+	struct device_node *child;
+	struct property *prop;
+	int num_names, num_children, ret;
 	struct device_node *adapter_np;
 	struct i2c_adapter *adapter;
+	const char *state;
 
 	if (!np)
 		return 0;
@@ -77,32 +80,16 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
 		return num_names;
 	}
 
-	mux->pdata->pinctrl_states = devm_kzalloc(&pdev->dev,
-		sizeof(*mux->pdata->pinctrl_states) * num_names,
-		GFP_KERNEL);
-	if (!mux->pdata->pinctrl_states) {
-		dev_err(mux->dev, "Cannot allocate pinctrl_states\n");
-		return -ENOMEM;
+	num_children = of_get_available_child_count(np);
+	if (num_children < 0) {
+		dev_err(mux->dev, "Unable to count available children: %d\n",
+			num_children);
+		return num_children;
 	}
 
-	for (i = 0; i < num_names; i++) {
-		ret = of_property_read_string_index(np, "pinctrl-names", i,
-			&mux->pdata->pinctrl_states[mux->pdata->bus_count]);
-		if (ret < 0) {
-			dev_err(mux->dev, "Cannot parse pinctrl-names: %d\n",
-				ret);
-			return ret;
-		}
-		if (!strcmp(mux->pdata->pinctrl_states[mux->pdata->bus_count],
-			    "idle")) {
-			if (i != num_names - 1) {
-				dev_err(mux->dev, "idle state must be last\n");
-				return -EINVAL;
-			}
-			mux->pdata->pinctrl_state_idle = "idle";
-		} else {
-			mux->pdata->bus_count++;
-		}
+	if (num_names < num_children) {
+		dev_err(mux->dev, "Found less pinctrl states than children\n");
+		return -EINVAL;
 	}
 
 	adapter_np = of_parse_phandle(np, "i2c-parent", 0);
@@ -118,6 +105,39 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
 	mux->pdata->parent_bus_num = i2c_adapter_id(adapter);
 	put_device(&adapter->dev);
 
+	mux->pdata->pinctrl_states = devm_kzalloc(&pdev->dev,
+		sizeof(*mux->pdata->pinctrl_states) * num_children,
+		GFP_KERNEL);
+	if (!mux->pdata->pinctrl_states) {
+		dev_err(mux->dev, "Cannot allocate pinctrl_states\n");
+		return -ENOMEM;
+	}
+
+	of_property_for_each_string(np, "pinctrl-names", prop, state)
+		if (!strcmp(state, "idle"))
+			mux->pdata->pinctrl_state_idle = "idle";
+
+	for_each_available_child_of_node(np, child) {
+		u32 reg;
+
+		ret = of_property_read_u32(child, "reg", &reg);
+		if (ret < 0) {
+			dev_err(mux->dev, "Missing reg property for child node: %d\n",
+				ret);
+			return ret;
+		}
+
+		ret = of_property_read_string_index(np,
+				    "pinctrl-names", reg, &state);
+		if (ret < 0) {
+			dev_err(mux->dev, "Cannot parse pinctrl-names for mux %d: %d\n",
+				reg, ret);
+			return ret;
+		}
+
+		mux->pdata->pinctrl_states[mux->pdata->bus_count++] = state;
+	}
+
 	return 0;
 }
 #else
-- 
2.1.0


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

* [PATCH v2 2/4] devicetree: vendor-prefixes: Add CompuLab to known vendors
  2015-02-27 12:24 ` [PATCH v2 0/4] " Sebastian Hesselbarth
  2015-02-27 12:24   ` [PATCH v2 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
@ 2015-02-27 12:24   ` Sebastian Hesselbarth
  2015-02-27 12:24   ` [PATCH v2 3/4] ARM: dts: dove: Add internal i2c multiplexer node Sebastian Hesselbarth
  2015-02-27 12:24   ` [PATCH v2 4/4] ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
  3 siblings, 0 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-27 12:24 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree, linux-arm-kernel, linux-kernel

This adds "compulab" as a vendor-prefix for CompuLab (compulab.co.il),
an Israeli company that builds ARM-based SoMs and CoMs.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 389ca1347a77..03d139c1174b 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -42,6 +42,7 @@ cirrus	Cirrus Logic, Inc.
 cloudengines	Cloud Engines, Inc.
 cnm	Chips&Media, Inc.
 cnxt	Conexant Systems, Inc.
+compulab	CompuLab
 cortina	Cortina Systems, Inc.
 cosmic	Cosmic Circuits
 crystalfontz	Crystalfontz America, Inc.
-- 
2.1.0


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

* [PATCH v2 3/4] ARM: dts: dove: Add internal i2c multiplexer node
  2015-02-27 12:24 ` [PATCH v2 0/4] " Sebastian Hesselbarth
  2015-02-27 12:24   ` [PATCH v2 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
  2015-02-27 12:24   ` [PATCH v2 2/4] devicetree: vendor-prefixes: Add CompuLab to known vendors Sebastian Hesselbarth
@ 2015-02-27 12:24   ` Sebastian Hesselbarth
  2015-02-27 12:24   ` [PATCH v2 4/4] ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
  3 siblings, 0 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-27 12:24 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, Stephen Warren, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel

This adds a i2c-mux-pinctrl node to dove.dtsi for the internal i2c
mux found on Dove SoCs. Up to now, we had no board using any of the
two additional i2c busses, so make sure the change does not break
any existing boards.

Therefore, we rename the i2c-controller node label to "i2c" and
enable it by default. Also, the dedicated sub-bus (now "i2c0") is
enabled by default. The two optional sub-busses require additional
external pin-muxing, so disable them by default.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Acked-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/dove.dtsi | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 9ad829523a13..b3340e862b0e 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -28,6 +28,42 @@
 		};
 	};
 
+	i2c-mux {
+		compatible = "i2c-mux-pinctrl";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c-parent = <&i2c>;
+
+		pinctrl-names = "i2c0", "i2c1", "i2c2";
+		pinctrl-0 = <&pmx_i2cmux_0>;
+		pinctrl-1 = <&pmx_i2cmux_1>;
+		pinctrl-2 = <&pmx_i2cmux_2>;
+
+		i2c0: i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "okay";
+		};
+
+		i2c1: i2c@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			/* Requires pmx_i2c1 on i2c controller node */
+			status = "disabled";
+		};
+
+		i2c2: i2c@2 {
+			reg = <2>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			/* Requires pmx_i2c2 on i2c controller node */
+			status = "disabled";
+		};
+	};
+
 	l2: l2-cache {
 		compatible = "marvell,tauros2-cache";
 		marvell,tauros2-cache-features = <0>;
@@ -123,7 +159,7 @@
 				status = "disabled";
 			};
 
-			i2c0: i2c-ctrl@11000 {
+			i2c: i2c-ctrl@11000 {
 				compatible = "marvell,mv64xxx-i2c";
 				reg = <0x11000 0x20>;
 				#address-cells = <1>;
@@ -132,7 +168,7 @@
 				clock-frequency = <400000>;
 				timeout-ms = <1000>;
 				clocks = <&core_clk 0>;
-				status = "disabled";
+				status = "okay";
 			};
 
 			uart0: serial@12000 {
-- 
2.1.0


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

* [PATCH v2 4/4] ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510
  2015-02-27 12:24 ` [PATCH v2 0/4] " Sebastian Hesselbarth
                     ` (2 preceding siblings ...)
  2015-02-27 12:24   ` [PATCH v2 3/4] ARM: dts: dove: Add internal i2c multiplexer node Sebastian Hesselbarth
@ 2015-02-27 12:24   ` Sebastian Hesselbarth
  3 siblings, 0 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-02-27 12:24 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	linux-arm-kernel, linux-kernel

Existing dts file for Compulab CM-A510 was very limited due to missing
hardware. Now that we actually found somebody with that board, properly
rework it to provide a CoM/SoM include and a board file for Compulab's
SBC-A510.

Both the CM-A510 SoM and the SBC-A510 can be configured with different
options, so we only enable a minimum set of options. The actual board
configuration will have to be set by either the bootloader or user.

Although functionally not required, repeat even disabled nodes again
to increse their visibility in the dtsi/dts files.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Tested-by: Gabriel Dobato <dobatog@gmail.com>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/boot/dts/Makefile          |   4 +-
 arch/arm/boot/dts/dove-cm-a510.dts  |  38 -------
 arch/arm/boot/dts/dove-cm-a510.dtsi | 195 ++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/dove-sbc-a510.dts | 182 +++++++++++++++++++++++++++++++++
 4 files changed, 379 insertions(+), 40 deletions(-)
 delete mode 100644 arch/arm/boot/dts/dove-cm-a510.dts
 create mode 100644 arch/arm/boot/dts/dove-cm-a510.dtsi
 create mode 100644 arch/arm/boot/dts/dove-sbc-a510.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index a1c776b8dcec..aedac13a3304 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -634,12 +634,12 @@ dtb-$(CONFIG_MACH_ARMADA_XP) += \
 	armada-xp-openblocks-ax3-4.dtb \
 	armada-xp-synology-ds414.dtb
 dtb-$(CONFIG_MACH_DOVE) += \
-	dove-cm-a510.dtb \
 	dove-cubox.dtb \
 	dove-cubox-es.dtb \
 	dove-d2plug.dtb \
 	dove-d3plug.dtb \
-	dove-dove-db.dtb
+	dove-dove-db.dtb \
+	dove-sbc-a510.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += \
 	mt6589-aquaris5.dtb \
 	mt6592-evb.dtb \
diff --git a/arch/arm/boot/dts/dove-cm-a510.dts b/arch/arm/boot/dts/dove-cm-a510.dts
deleted file mode 100644
index 50c0d6904497..000000000000
--- a/arch/arm/boot/dts/dove-cm-a510.dts
+++ /dev/null
@@ -1,38 +0,0 @@
-/dts-v1/;
-
-#include "dove.dtsi"
-
-/ {
-	model = "Compulab CM-A510";
-	compatible = "compulab,cm-a510", "marvell,dove";
-
-	memory {
-		device_type = "memory";
-		reg = <0x00000000 0x40000000>;
-	};
-
-	chosen {
-		bootargs = "console=ttyS0,115200n8 earlyprintk";
-	};
-};
-
-&uart0 { status = "okay"; };
-&uart1 { status = "okay"; };
-&sdio0 { status = "okay"; };
-&sdio1 { status = "okay"; };
-&sata0 { status = "okay"; };
-
-&spi0 {
-	status = "okay";
-
-	/* spi0.0: 4M Flash Winbond W25Q32BV */
-	spi-flash@0 {
-		compatible = "st,w25q32";
-		spi-max-frequency = <20000000>;
-		reg = <0>;
-	};
-};
-
-&i2c0 {
-	  status = "okay";
-};
diff --git a/arch/arm/boot/dts/dove-cm-a510.dtsi b/arch/arm/boot/dts/dove-cm-a510.dtsi
new file mode 100644
index 000000000000..ef670bd040d3
--- /dev/null
+++ b/arch/arm/boot/dts/dove-cm-a510.dtsi
@@ -0,0 +1,195 @@
+/*
+ * Device Tree include for Compulab CM-A510 System-on-Module
+ *
+ * Copyright (C) 2015, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/*
+ * The CM-A510 comes with several optional components:
+ *
+ * Memory options:
+ *  D512: 512M
+ *  D1024: 1G
+ *
+ * NAND options:
+ *  N512: 512M NAND
+ *
+ * Ethernet options:
+ *  E1: PHY RTL8211D on internal GbE (SMI address 0x03)
+ *  E2: Additional ethernet NIC RTL8111D on PCIe1
+ *
+ * Audio options:
+ *  A: TI TLV320AIC23b audio codec (I2C address 0x1a)
+ *
+ * Touchscreen options:
+ *  I: TI TSC2046 touchscreen controller (on SPI1)
+ *
+ * USB options:
+ *  U2: 2 dual-role USB2.0 ports
+ *  U4: 2 additional USB2.0 host ports (via USB1)
+ *
+ * WiFi options:
+ *  W: Broadcom BCM4319 802.11b/g/n (USI WM-N-BM-01 on SDIO1)
+ *
+ * GPIOs used on CM-A510:
+ *   1 GbE PHY reset (active low)
+ *   3 WakeUp
+ *   8 PowerOff (active low)
+ *  13 Touchscreen pen irq (active low)
+ *  65 System LED (active high)
+ *  69 USB Hub reset (active low)
+ *  70 WLAN reset (active low)
+ *  71 WLAN regulator (active high)
+ */
+
+#include "dove.dtsi"
+
+/ {
+	model = "Compulab CM-A510";
+	compatible = "compulab,cm-a510", "marvell,dove";
+
+	/*
+	 * Set the minimum memory size here and let the
+	 * bootloader set the real size.
+	 */
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x20000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+		/* Set upper NAND data bit to GPO */
+		pinctrl-0 = <&pmx_nand_gpo>;
+		pinctrl-names = "default";
+
+		system {
+			label = "cm-a510:system:green";
+			gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
+			default-state = "keep";
+		};
+	};
+
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		wifi_power: regulator@1 {
+			compatible = "regulator-fixed";
+			regulator-name = "WiFi Power";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			gpio = <&gpio2 7 GPIO_ACTIVE_HIGH>;
+		};
+	};
+};
+
+/* Optional RTL8211D GbE PHY on SMI address 0x03 */
+&ethphy {
+	reg = <3>;
+	status = "disabled";
+};
+
+&i2c0 {
+	/* Optional TI TLV320AIC23b audio codec */
+	opt_audio: audio@1a {
+		compatible = "ti,tlv320aic23";
+		reg = <0x1a>;
+		status = "disabled";
+	};
+};
+
+/* Optional RTL8111D GbE NIC on PCIe1 */
+&pcie { status = "disabled"; };
+
+&pcie1 {
+	pinctrl-0 = <&pmx_pcie1_clkreq>;
+	pinctrl-names = "default";
+	status = "disabled";
+};
+
+&pinctrl {
+	pmx_uart2: pmx-uart2 {
+		marvell,pins = "mpp14", "mpp15";
+		marvell,function = "uart2";
+	};
+};
+
+/* Optional Broadcom BCM4319 802.11b/g/n WiFi module */
+&sdio1 {
+	non-removable;
+	vmmc-supply = <&wifi_power>;
+	reset-gpio = <&gpio2 6 GPIO_ACTIVE_LOW>;
+	status = "disabled";
+};
+
+&spi0 {
+	status = "okay";
+
+	/* 1M Flash Winbond W25Q80BL */
+	flash@0 {
+		compatible = "winbond,w25q80";
+		spi-max-frequency = <80000000>;
+		reg = <0>;
+	};
+};
+
+&spi1 {
+	pinctrl-0 = <&pmx_spi1_20_23>;
+	pinctrl-names = "default";
+	status = "disabled";
+
+	/* Optional TI TSC2046 touchscreen controller */
+	opt_touch: touchscreen@0 {
+		compatible = "ti,tsc2046";
+		spi-max-frequency = <2500000>;
+		reg = <0>;
+		pinctrl-0 = <&pmx_gpio_13>;
+		pinctrl-names = "default";
+		interrupts-extended = <&gpio0 13 IRQ_TYPE_EDGE_FALLING>;
+	};
+};
+
+&uart2 {
+	pinctrl-0 = <&pmx_uart2>;
+	pinctrl-names = "default";
+};
diff --git a/arch/arm/boot/dts/dove-sbc-a510.dts b/arch/arm/boot/dts/dove-sbc-a510.dts
new file mode 100644
index 000000000000..38096259ecce
--- /dev/null
+++ b/arch/arm/boot/dts/dove-sbc-a510.dts
@@ -0,0 +1,182 @@
+/*
+ * Device Tree file for Compulab SBC-A510 Single Board Computer
+ *
+ * Copyright (C) 2015, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/*
+ * SBC-A510 comprises a PCA9555 I2C GPIO expander its GPIO lines connected to
+ *
+ * 0.0 USB0 VBUS_EN (active high)
+ * 0.1 USB0 VBUS_GOOD
+ * 0.2 DVI transmitter TI TFP410 MSEN
+ * 0.3 DVI transmitter TI TFP410 PD# (active low power down)
+ * 0.4 LVDS transmitter DS90C365 PD# (active low power down)
+ * 0.5 LCD nRST (active low reset)
+ * 0.6 PCIe0 nRST (active low reset)
+ * 0.7 mini-PCIe slot W_DISABLE#
+ *
+ * 1.0 MMC WP
+ * 1.1 Camera Input FPC FLASH_STB and P21.5
+ * 1.2 Camera Input FPC WE        and P21.22
+ * 1.3 MMC VCC_EN (active high)   and P21.7
+ * 1.4 Camera Input FPC AFTR_RST  and P21.17
+ * 1.5 Camera Input FPC OE        and P21.19
+ * 1.6 Camera Input FPC SNPSHT    and P21.6
+ * 1.7 Camera Input FPC SHTR      and P21.10
+ */
+
+/dts-v1/;
+
+#include "dove-cm-a510.dtsi"
+
+/ {
+	model = "Compulab SBC-A510";
+	compatible = "compulab,sbc-a510", "compulab,cm-a510", "marvell,dove";
+
+	chosen {
+		stdout-path = &uart0;
+	};
+
+	regulators {
+		usb0_power: regulator@2 {
+			compatible = "regulator-fixed";
+			regulator-name = "USB Power";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			gpio = <&gpio_ext 0 GPIO_ACTIVE_HIGH>;
+		};
+
+		mmc_power: regulator@3 {
+			compatible = "regulator-fixed";
+			regulator-name = "MMC Power";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			gpio = <&gpio_ext 13 GPIO_ACTIVE_HIGH>;
+		};
+	};
+};
+
+/* Ethernet0 depends on CM-A510 option E1 */
+&mdio { status = "disabled"; };
+&eth { status = "disabled"; };
+&ethphy { status = "disabled"; };
+
+/*
+ * USB port 0 can be powered and monitored by I2C GPIO expander:
+ *  VBUS_ENABLE on GPIO0, VBUS_GOOD on GPIO1
+ */
+&ehci0 {
+	status = "okay";
+	vbus-supply = <&usb0_power>;
+};
+
+/* USB port 1 (and ports 2, 3 if CM-A510 has U4 option) */
+&ehci1 { status = "okay"; };
+
+/*
+ * I2C bus layout:
+ * i2c0:
+ *  - Audio Codec, 0x1a (option from CM-A510)
+ *  - DVI transmitter TI TFP410, 0x39
+ *  - HDMI/DVI DDC channel
+ * i2c1:
+ *  - GPIO expander, NXP PCA9555, 0x20
+ *  - VGA DDC channel
+ */
+&i2c {
+	pinctrl-0 = <&pmx_i2c1>;
+	pinctrl-names = "default";
+};
+
+&i2c0 {
+	/* TI TFP410 DVI transmitter */
+	dvi: video@39 {
+		compatible = "ti,tfp410";
+		reg = <0x39>;
+		powerdown-gpio = <&gpio_ext 3 GPIO_ACTIVE_LOW>;
+	};
+};
+
+&i2c1 {
+	status = "okay";
+
+	/* NXP PCA9555 GPIO expander */
+	gpio_ext: gpio@20 {
+		compatible = "nxp,pca9555";
+		reg = <0x20>;
+		#gpio-cells = <2>;
+	};
+};
+
+&pcie { status = "okay"; };
+
+/*
+ * PCIe0 can be configured by Jumper E1 to be either connected to
+ * a mini-PCIe slot or a Pericom PI7C9X111 PCIe-to-PCI bridge.
+ */
+&pcie0 {
+	status = "okay";
+	pinctrl-0 = <&pmx_pcie0_clkreq>;
+	pinctrl-names = "default";
+	reset-gpios = <&gpio_ext 6 GPIO_ACTIVE_LOW>;
+};
+
+/* Ethernet1 depends on CM-A510 option E2 */
+&pcie1 { status = "disabled"; };
+
+/* SATA connector */
+&sata0 { status = "okay"; };
+
+/*
+ * SDIO0 is connected to a MMC/SD/SDIO socket, I2C GPIO expander has
+ *  VCC_MMC_ENABLE on GPIO13, MMC_WP on GPIO10
+ */
+&sdio0 {
+	vmmc-supply = <&mmc_power>;
+	wp-gpios = <&gpio_ext 10 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};
+
+/* UART0 on RS232 mini-connector */
+&uart0 { status = "okay"; };
+/* UART2 on pin headers */
+&uart2 { status = "okay"; };
-- 
2.1.0


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

* Re: [PATCH v2 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-02-27 12:24   ` [PATCH v2 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
@ 2015-03-02 20:01     ` Stephen Warren
  2015-03-04  9:10       ` Sebastian Hesselbarth
  2015-03-09 12:21     ` [PATCH v3 " Sebastian Hesselbarth
  1 sibling, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2015-03-02 20:01 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 02/27/2015 05:24 AM, Sebastian Hesselbarth wrote:
> I2C mux pinctrl driver currently determines the number of sub-busses by
> counting available pinctrl-names. Unfortunately, this requires each
> incarnation of the devicetree node with different available sub-busses
> to be rewritten.
>
> This patch reworks i2c-mux-pinctrl driver to count the number of
> available sub-nodes instead. The rework should be compatible to the old
> way of probing for sub-busses and additionally allows to disable unused
> sub-busses with standard DT property status = "disabled".
>
> This also amends the corresponding devicetree binding documentation to
> reflect the new functionality to disable unused sub-nodes. While at it,
> also fix two references to binding documentation files that miss an "i2c-"
> prefix.

> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt

> -For each named state defined in the pinctrl-names property, an I2C child bus
> -will be created. I2C child bus numbers are assigned based on the index into
> -the pinctrl-names property.
> +For each enabled child node an I2C child bus will be created. I2C child bus
> +numbers are assigned based on the order of child nodes.

I think that I2C bus numbers are an internal concept for the OS. As 
such, we should probably remove any mention re: the bus numbers from the 
binding.

> -The only exception is that no bus will be created for a state named "idle". If
> -such a state is defined, it must be the last entry in pinctrl-names. For
> -example:
> +There must be a corresponding pinctrl-names entry for each enabled child
> +node at the position of the child node's "reg" property. Also, there can be
> +an idle pinctrl state defined at the end of possible pinctrl states. If such
> +a state is defined, it must be the last entry in pinctrl-names. For example:

What about gaps in the numbering sequence? IIRC, in a situation with 5 
nodes with reg 0, 1, 2, 3, 4 but where only the nodes with reg of 1, 3 
enabled, we only want 2 entries in pinctrl-names? If so, "at the 
position of the child node's "reg" property" isn't correct, since "at 
the position" implies there must be gaps in pinctrl-names. "In the same 
order as the reg property values for enabled subnodes" might be a better 
description.

Perhaps I'm misremembering and you explicitly didn't want to remove 
entries from pinctrl-names if child nodes were disabled? If so, then 
surely then in the text above, "for each enabled child" should be 
replaced with "for each child"?

> @@ -68,6 +68,7 @@ Example:
>   		pinctrl-1 = <&state_i2cmux_pta>;
>   		pinctrl-2 = <&state_i2cmux_idle>;
>
> +		/* Enabled child bus 0 */
>   		i2c@0 {
>   			reg = <0>;
>   			#address-cells = <1>;
> @@ -79,10 +80,12 @@ Example:
>   			};
>   		};
>
> +		/* Disabled child bus 1 */
>   		i2c@1 {
>   			reg = <1>;
>   			#address-cells = <1>;
>   			#size-cells = <0>;
> +			status = "disabled";

To make the example cover more cases, perhaps make child node i2c@0 
disabled and i2c@1 enabled. Then, the example would show what happens to 
pinctrl-names when there are gaps in the reg property numbering space of 
enabled children?

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

* Re: [PATCH v2 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-02 20:01     ` Stephen Warren
@ 2015-03-04  9:10       ` Sebastian Hesselbarth
  0 siblings, 0 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-03-04  9:10 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 02.03.2015 21:01, Stephen Warren wrote:
> On 02/27/2015 05:24 AM, Sebastian Hesselbarth wrote:
>> I2C mux pinctrl driver currently determines the number of sub-busses by
>> counting available pinctrl-names. Unfortunately, this requires each
>> incarnation of the devicetree node with different available sub-busses
>> to be rewritten.
>>
>> This patch reworks i2c-mux-pinctrl driver to count the number of
>> available sub-nodes instead. The rework should be compatible to the old
>> way of probing for sub-busses and additionally allows to disable unused
>> sub-busses with standard DT property status = "disabled".
>>
>> This also amends the corresponding devicetree binding documentation to
>> reflect the new functionality to disable unused sub-nodes. While at it,
>> also fix two references to binding documentation files that miss an
>> "i2c-"
>> prefix.
>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
>> b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
>
>> -For each named state defined in the pinctrl-names property, an I2C
>> child bus
>> -will be created. I2C child bus numbers are assigned based on the
>> index into
>> -the pinctrl-names property.
>> +For each enabled child node an I2C child bus will be created. I2C
>> child bus
>> +numbers are assigned based on the order of child nodes.
>
> I think that I2C bus numbers are an internal concept for the OS. As
> such, we should probably remove any mention re: the bus numbers from the
> binding.

Stephen,

yeah as you can see I am struggling to find a good documentation. I
agree that we should get rid of the bus number thing above.

>> -The only exception is that no bus will be created for a state named
>> "idle". If
>> -such a state is defined, it must be the last entry in pinctrl-names. For
>> -example:
>> +There must be a corresponding pinctrl-names entry for each enabled child
>> +node at the position of the child node's "reg" property. Also, there
>> can be
>> +an idle pinctrl state defined at the end of possible pinctrl states.
>> If such
>> +a state is defined, it must be the last entry in pinctrl-names. For
>> example:
>
> What about gaps in the numbering sequence? IIRC, in a situation with 5
> nodes with reg 0, 1, 2, 3, 4 but where only the nodes with reg of 1, 3
> enabled, we only want 2 entries in pinctrl-names? If so, "at the
> position of the child node's "reg" property" isn't correct, since "at
> the position" implies there must be gaps in pinctrl-names. "In the same
> order as the reg property values for enabled subnodes" might be a better
> description.

Good point. The idea was to _have_ gaps in pinctrl-names to allow to
configure the current mux layout by status properties only. The existing
implementation (and docu) suggested you have to amend pinctrl-names to
achieve a specific setup.

> Perhaps I'm misremembering and you explicitly didn't want to remove
> entries from pinctrl-names if child nodes were disabled? If so, then
> surely then in the text above, "for each enabled child" should be
> replaced with "for each child"?

True.

>> @@ -68,6 +68,7 @@ Example:
>>           pinctrl-1 = <&state_i2cmux_pta>;
>>           pinctrl-2 = <&state_i2cmux_idle>;
>>
>> +        /* Enabled child bus 0 */
>>           i2c@0 {
>>               reg = <0>;
>>               #address-cells = <1>;
>> @@ -79,10 +80,12 @@ Example:
>>               };
>>           };
>>
>> +        /* Disabled child bus 1 */
>>           i2c@1 {
>>               reg = <1>;
>>               #address-cells = <1>;
>>               #size-cells = <0>;
>> +            status = "disabled";
>
> To make the example cover more cases, perhaps make child node i2c@0
> disabled and i2c@1 enabled. Then, the example would show what happens to
> pinctrl-names when there are gaps in the reg property numbering space of
> enabled children?

The idea was to make nothing happen to pinctrl-names if you enable/
disable any of the children. But I can move the disabled status to
i2c@0 to make it more clear that there should still be a pinctrl-names
cell for it.

Sebastian

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

* [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-02-27 12:24   ` [PATCH v2 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
  2015-03-02 20:01     ` Stephen Warren
@ 2015-03-09 12:21     ` Sebastian Hesselbarth
  2015-03-10 16:28       ` Stephen Warren
  2015-03-18 12:30       ` Wolfram Sang
  1 sibling, 2 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-03-09 12:21 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, Stephen Warren, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel

I2C mux pinctrl driver currently determines the number of sub-busses by
counting available pinctrl-names. Unfortunately, this requires each
incarnation of the devicetree node with different available sub-busses
to be rewritten.

This patch reworks i2c-mux-pinctrl driver to count the number of
available sub-nodes instead. The rework should be compatible to the old
way of probing for sub-busses and additionally allows to disable unused
sub-busses with standard DT property status = "disabled".

This also amends the corresponding devicetree binding documentation to
reflect the new functionality to disable unused sub-nodes. While at it,
also fix two references to binding documentation files that miss an "i2c-"
prefix.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Changelog:

v2->v3:
- remove mention of "I2C bus numbers" (Suggested by Stephen Warren)
- require pinctrl-names property for "each child" instead of
  "each enabled child" (Suggested by Stephen Warren)
- swap enabled/disabled child nodes (Suggested by Stephen Warren)

v1->v2:
- added a Tested-by for i2c-mux-pinctrl changes from Stepen Warren.
- reworded i2c-mux-pinctrl devicetree doc changes
  (Suggested by Stephen Warren).

Patches 2/4 - 4/4 remain unchanged.

Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Gabriel Dobato <dobatog@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt    | 21 ++++---
 drivers/i2c/muxes/i2c-mux-pinctrl.c                | 70 ++++++++++++++--------
 2 files changed, 58 insertions(+), 33 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
index ae8af1694e95..cd94a0f64d76 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
@@ -28,17 +28,18 @@ Also required are:
 * Standard pinctrl properties that specify the pin mux state for each child
   bus. See ../pinctrl/pinctrl-bindings.txt.
 
-* Standard I2C mux properties. See mux.txt in this directory.
+* Standard I2C mux properties. See i2c-mux.txt in this directory.
 
-* I2C child bus nodes. See mux.txt in this directory.
+* I2C child bus nodes. See i2c-mux.txt in this directory.
 
-For each named state defined in the pinctrl-names property, an I2C child bus
-will be created. I2C child bus numbers are assigned based on the index into
-the pinctrl-names property.
+For each enabled child node an I2C child bus will be created.
 
-The only exception is that no bus will be created for a state named "idle". If
-such a state is defined, it must be the last entry in pinctrl-names. For
-example:
+There must be a corresponding pinctrl-names entry for each child node at the
+position of the child node's "reg" property.
+
+Also, there can be an idle pinctrl state defined at the end of possible pinctrl
+states. If such a state is defined, it must be the last entry in pinctrl-names.
+For example:
 
 	pinctrl-names = "ddc", "pta", "idle"  ->  ddc = bus 0, pta = bus 1
 	pinctrl-names = "ddc", "idle", "pta"  ->  Invalid ("idle" not last)
@@ -68,10 +69,12 @@ Example:
 		pinctrl-1 = <&state_i2cmux_pta>;
 		pinctrl-2 = <&state_i2cmux_idle>;
 
+		/* Disabled child bus 0 */
 		i2c@0 {
 			reg = <0>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			status = "disabled";
 
 			eeprom {
 				compatible = "eeprom";
@@ -79,10 +82,12 @@ Example:
 			};
 		};
 
+		/* Enabled child bus 1 */
 		i2c@1 {
 			reg = <1>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			status = "okay";
 
 			eeprom {
 				compatible = "eeprom";
diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
index b48378c4b40d..033dacfabfdf 100644
--- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
+++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
@@ -56,9 +56,12 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
 				struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
-	int num_names, i, ret;
+	struct device_node *child;
+	struct property *prop;
+	int num_names, num_children, ret;
 	struct device_node *adapter_np;
 	struct i2c_adapter *adapter;
+	const char *state;
 
 	if (!np)
 		return 0;
@@ -77,32 +80,16 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
 		return num_names;
 	}
 
-	mux->pdata->pinctrl_states = devm_kzalloc(&pdev->dev,
-		sizeof(*mux->pdata->pinctrl_states) * num_names,
-		GFP_KERNEL);
-	if (!mux->pdata->pinctrl_states) {
-		dev_err(mux->dev, "Cannot allocate pinctrl_states\n");
-		return -ENOMEM;
+	num_children = of_get_available_child_count(np);
+	if (num_children < 0) {
+		dev_err(mux->dev, "Unable to count available children: %d\n",
+			num_children);
+		return num_children;
 	}
 
-	for (i = 0; i < num_names; i++) {
-		ret = of_property_read_string_index(np, "pinctrl-names", i,
-			&mux->pdata->pinctrl_states[mux->pdata->bus_count]);
-		if (ret < 0) {
-			dev_err(mux->dev, "Cannot parse pinctrl-names: %d\n",
-				ret);
-			return ret;
-		}
-		if (!strcmp(mux->pdata->pinctrl_states[mux->pdata->bus_count],
-			    "idle")) {
-			if (i != num_names - 1) {
-				dev_err(mux->dev, "idle state must be last\n");
-				return -EINVAL;
-			}
-			mux->pdata->pinctrl_state_idle = "idle";
-		} else {
-			mux->pdata->bus_count++;
-		}
+	if (num_names < num_children) {
+		dev_err(mux->dev, "Found less pinctrl states than children\n");
+		return -EINVAL;
 	}
 
 	adapter_np = of_parse_phandle(np, "i2c-parent", 0);
@@ -118,6 +105,39 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
 	mux->pdata->parent_bus_num = i2c_adapter_id(adapter);
 	put_device(&adapter->dev);
 
+	mux->pdata->pinctrl_states = devm_kzalloc(&pdev->dev,
+		sizeof(*mux->pdata->pinctrl_states) * num_children,
+		GFP_KERNEL);
+	if (!mux->pdata->pinctrl_states) {
+		dev_err(mux->dev, "Cannot allocate pinctrl_states\n");
+		return -ENOMEM;
+	}
+
+	of_property_for_each_string(np, "pinctrl-names", prop, state)
+		if (!strcmp(state, "idle"))
+			mux->pdata->pinctrl_state_idle = "idle";
+
+	for_each_available_child_of_node(np, child) {
+		u32 reg;
+
+		ret = of_property_read_u32(child, "reg", &reg);
+		if (ret < 0) {
+			dev_err(mux->dev, "Missing reg property for child node: %d\n",
+				ret);
+			return ret;
+		}
+
+		ret = of_property_read_string_index(np,
+				    "pinctrl-names", reg, &state);
+		if (ret < 0) {
+			dev_err(mux->dev, "Cannot parse pinctrl-names for mux %d: %d\n",
+				reg, ret);
+			return ret;
+		}
+
+		mux->pdata->pinctrl_states[mux->pdata->bus_count++] = state;
+	}
+
 	return 0;
 }
 #else
-- 
2.1.0


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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-09 12:21     ` [PATCH v3 " Sebastian Hesselbarth
@ 2015-03-10 16:28       ` Stephen Warren
  2015-03-16 20:15         ` Sebastian Hesselbarth
  2015-03-18 12:30       ` Wolfram Sang
  1 sibling, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2015-03-10 16:28 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 03/09/2015 06:21 AM, Sebastian Hesselbarth wrote:
> I2C mux pinctrl driver currently determines the number of sub-busses by
> counting available pinctrl-names. Unfortunately, this requires each
> incarnation of the devicetree node with different available sub-busses
> to be rewritten.
>
> This patch reworks i2c-mux-pinctrl driver to count the number of
> available sub-nodes instead. The rework should be compatible to the old
> way of probing for sub-busses and additionally allows to disable unused
> sub-busses with standard DT property status = "disabled".
>
> This also amends the corresponding devicetree binding documentation to
> reflect the new functionality to disable unused sub-nodes. While at it,
> also fix two references to binding documentation files that miss an "i2c-"
> prefix.

The DT binding changes at least,
Acked-by: Stephen Warren <swarren@nvidia.com>

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-10 16:28       ` Stephen Warren
@ 2015-03-16 20:15         ` Sebastian Hesselbarth
  0 siblings, 0 replies; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-03-16 20:15 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Wolfram Sang, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 10.03.2015 17:28, Stephen Warren wrote:
> On 03/09/2015 06:21 AM, Sebastian Hesselbarth wrote:
>> I2C mux pinctrl driver currently determines the number of sub-busses by
>> counting available pinctrl-names. Unfortunately, this requires each
>> incarnation of the devicetree node with different available sub-busses
>> to be rewritten.
>>
>> This patch reworks i2c-mux-pinctrl driver to count the number of
>> available sub-nodes instead. The rework should be compatible to the old
>> way of probing for sub-busses and additionally allows to disable unused
>> sub-busses with standard DT property status = "disabled".
>>
>> This also amends the corresponding devicetree binding documentation to
>> reflect the new functionality to disable unused sub-nodes. While at it,
>> also fix two references to binding documentation files that miss an
>> "i2c-"
>> prefix.
>
> The DT binding changes at least,
> Acked-by: Stephen Warren <swarren@nvidia.com>

Wolfram,

are you going to pick this patch through your tree now that Stephen
ack'ed the binding documentation change, too?

I can also split the patch up into driver/doc changes if you prefer.

Sebastian

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-09 12:21     ` [PATCH v3 " Sebastian Hesselbarth
  2015-03-10 16:28       ` Stephen Warren
@ 2015-03-18 12:30       ` Wolfram Sang
  2015-03-18 13:23         ` Sebastian Hesselbarth
  1 sibling, 1 reply; 50+ messages in thread
From: Wolfram Sang @ 2015-03-18 12:30 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Stephen Warren, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

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

On Mon, Mar 09, 2015 at 01:21:05PM +0100, Sebastian Hesselbarth wrote:
> I2C mux pinctrl driver currently determines the number of sub-busses by
> counting available pinctrl-names. Unfortunately, this requires each
> incarnation of the devicetree node with different available sub-busses
> to be rewritten.
> 
> This patch reworks i2c-mux-pinctrl driver to count the number of
> available sub-nodes instead. The rework should be compatible to the old
> way of probing for sub-busses and additionally allows to disable unused
> sub-busses with standard DT property status = "disabled".

Not sure about this change. With DYNAMIC_OF these days, you can't rely
that 'disabled' stays disabled all the time. My gut feeling tells me
that people will want to use this someday.

> 
> This also amends the corresponding devicetree binding documentation to
> reflect the new functionality to disable unused sub-nodes. While at it,
> also fix two references to binding documentation files that miss an "i2c-"
> prefix.
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
> Changelog:
> 
> v2->v3:
> - remove mention of "I2C bus numbers" (Suggested by Stephen Warren)
> - require pinctrl-names property for "each child" instead of
>   "each enabled child" (Suggested by Stephen Warren)
> - swap enabled/disabled child nodes (Suggested by Stephen Warren)
> 
> v1->v2:
> - added a Tested-by for i2c-mux-pinctrl changes from Stepen Warren.
> - reworded i2c-mux-pinctrl devicetree doc changes
>   (Suggested by Stephen Warren).
> 
> Patches 2/4 - 4/4 remain unchanged.
> 
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Gabriel Dobato <dobatog@gmail.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: linux-i2c@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  .../devicetree/bindings/i2c/i2c-mux-pinctrl.txt    | 21 ++++---
>  drivers/i2c/muxes/i2c-mux-pinctrl.c                | 70 ++++++++++++++--------
>  2 files changed, 58 insertions(+), 33 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
> index ae8af1694e95..cd94a0f64d76 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pinctrl.txt
> @@ -28,17 +28,18 @@ Also required are:
>  * Standard pinctrl properties that specify the pin mux state for each child
>    bus. See ../pinctrl/pinctrl-bindings.txt.
>  
> -* Standard I2C mux properties. See mux.txt in this directory.
> +* Standard I2C mux properties. See i2c-mux.txt in this directory.
>  
> -* I2C child bus nodes. See mux.txt in this directory.
> +* I2C child bus nodes. See i2c-mux.txt in this directory.
>  
> -For each named state defined in the pinctrl-names property, an I2C child bus
> -will be created. I2C child bus numbers are assigned based on the index into
> -the pinctrl-names property.
> +For each enabled child node an I2C child bus will be created.
>  
> -The only exception is that no bus will be created for a state named "idle". If
> -such a state is defined, it must be the last entry in pinctrl-names. For
> -example:
> +There must be a corresponding pinctrl-names entry for each child node at the
> +position of the child node's "reg" property.
> +
> +Also, there can be an idle pinctrl state defined at the end of possible pinctrl
> +states. If such a state is defined, it must be the last entry in pinctrl-names.
> +For example:
>  
>  	pinctrl-names = "ddc", "pta", "idle"  ->  ddc = bus 0, pta = bus 1
>  	pinctrl-names = "ddc", "idle", "pta"  ->  Invalid ("idle" not last)
> @@ -68,10 +69,12 @@ Example:
>  		pinctrl-1 = <&state_i2cmux_pta>;
>  		pinctrl-2 = <&state_i2cmux_idle>;
>  
> +		/* Disabled child bus 0 */
>  		i2c@0 {
>  			reg = <0>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> +			status = "disabled";
>  
>  			eeprom {
>  				compatible = "eeprom";
> @@ -79,10 +82,12 @@ Example:
>  			};
>  		};
>  
> +		/* Enabled child bus 1 */
>  		i2c@1 {
>  			reg = <1>;
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> +			status = "okay";
>  
>  			eeprom {
>  				compatible = "eeprom";
> diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
> index b48378c4b40d..033dacfabfdf 100644
> --- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
> +++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
> @@ -56,9 +56,12 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
>  				struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
> -	int num_names, i, ret;
> +	struct device_node *child;
> +	struct property *prop;
> +	int num_names, num_children, ret;
>  	struct device_node *adapter_np;
>  	struct i2c_adapter *adapter;
> +	const char *state;
>  
>  	if (!np)
>  		return 0;
> @@ -77,32 +80,16 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
>  		return num_names;
>  	}
>  
> -	mux->pdata->pinctrl_states = devm_kzalloc(&pdev->dev,
> -		sizeof(*mux->pdata->pinctrl_states) * num_names,
> -		GFP_KERNEL);
> -	if (!mux->pdata->pinctrl_states) {
> -		dev_err(mux->dev, "Cannot allocate pinctrl_states\n");
> -		return -ENOMEM;
> +	num_children = of_get_available_child_count(np);
> +	if (num_children < 0) {
> +		dev_err(mux->dev, "Unable to count available children: %d\n",
> +			num_children);
> +		return num_children;
>  	}
>  
> -	for (i = 0; i < num_names; i++) {
> -		ret = of_property_read_string_index(np, "pinctrl-names", i,
> -			&mux->pdata->pinctrl_states[mux->pdata->bus_count]);
> -		if (ret < 0) {
> -			dev_err(mux->dev, "Cannot parse pinctrl-names: %d\n",
> -				ret);
> -			return ret;
> -		}
> -		if (!strcmp(mux->pdata->pinctrl_states[mux->pdata->bus_count],
> -			    "idle")) {
> -			if (i != num_names - 1) {
> -				dev_err(mux->dev, "idle state must be last\n");
> -				return -EINVAL;
> -			}
> -			mux->pdata->pinctrl_state_idle = "idle";
> -		} else {
> -			mux->pdata->bus_count++;
> -		}
> +	if (num_names < num_children) {
> +		dev_err(mux->dev, "Found less pinctrl states than children\n");
> +		return -EINVAL;
>  	}
>  
>  	adapter_np = of_parse_phandle(np, "i2c-parent", 0);
> @@ -118,6 +105,39 @@ static int i2c_mux_pinctrl_parse_dt(struct i2c_mux_pinctrl *mux,
>  	mux->pdata->parent_bus_num = i2c_adapter_id(adapter);
>  	put_device(&adapter->dev);
>  
> +	mux->pdata->pinctrl_states = devm_kzalloc(&pdev->dev,
> +		sizeof(*mux->pdata->pinctrl_states) * num_children,
> +		GFP_KERNEL);
> +	if (!mux->pdata->pinctrl_states) {
> +		dev_err(mux->dev, "Cannot allocate pinctrl_states\n");
> +		return -ENOMEM;
> +	}
> +
> +	of_property_for_each_string(np, "pinctrl-names", prop, state)
> +		if (!strcmp(state, "idle"))
> +			mux->pdata->pinctrl_state_idle = "idle";
> +
> +	for_each_available_child_of_node(np, child) {
> +		u32 reg;
> +
> +		ret = of_property_read_u32(child, "reg", &reg);
> +		if (ret < 0) {
> +			dev_err(mux->dev, "Missing reg property for child node: %d\n",
> +				ret);
> +			return ret;
> +		}
> +
> +		ret = of_property_read_string_index(np,
> +				    "pinctrl-names", reg, &state);
> +		if (ret < 0) {
> +			dev_err(mux->dev, "Cannot parse pinctrl-names for mux %d: %d\n",
> +				reg, ret);
> +			return ret;
> +		}
> +
> +		mux->pdata->pinctrl_states[mux->pdata->bus_count++] = state;
> +	}
> +
>  	return 0;
>  }
>  #else
> -- 
> 2.1.0
> 

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

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-18 12:30       ` Wolfram Sang
@ 2015-03-18 13:23         ` Sebastian Hesselbarth
  2015-03-18 14:00           ` Wolfram Sang
  0 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-03-18 13:23 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Stephen Warren, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 18.03.2015 13:30, Wolfram Sang wrote:
> On Mon, Mar 09, 2015 at 01:21:05PM +0100, Sebastian Hesselbarth wrote:
>> I2C mux pinctrl driver currently determines the number of sub-busses by
>> counting available pinctrl-names. Unfortunately, this requires each
>> incarnation of the devicetree node with different available sub-busses
>> to be rewritten.
>>
>> This patch reworks i2c-mux-pinctrl driver to count the number of
>> available sub-nodes instead. The rework should be compatible to the old
>> way of probing for sub-busses and additionally allows to disable unused
>> sub-busses with standard DT property status = "disabled".
>
> Not sure about this change. With DYNAMIC_OF these days, you can't rely
> that 'disabled' stays disabled all the time. My gut feeling tells me
> that people will want to use this someday.

Possible. But this change just makes i2c-mux-pinctrl honor status
property at all. There is no functional change except it now allows
you to disable any of the sub-busses.

I agree that this driver still does not cope well with DYNAMIC_OF but
neither did the former implementation. How about we settle this driver
to this implementation now and wait for any maniac that wants to use it
the way you are suggesting above?

The other option would be to leave the driver as is - but at least on
Dove where the muxing-options are not used often, it will always create
4 i2c-busses (controller plus the three muxing options) on any board
even though the pins are not accessible at all. I think that will just
create massive confusion from the user point-of-view?

BTW, I have received a patchwork update notification - it may be
unrelated but I prefer the Dove dts/dtsi changes to go through mvebu
tree.

Sebastian


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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-18 13:23         ` Sebastian Hesselbarth
@ 2015-03-18 14:00           ` Wolfram Sang
  2015-03-18 23:10             ` Sebastian Hesselbarth
  0 siblings, 1 reply; 50+ messages in thread
From: Wolfram Sang @ 2015-03-18 14:00 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Stephen Warren, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

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

On Wed, Mar 18, 2015 at 02:23:18PM +0100, Sebastian Hesselbarth wrote:
> On 18.03.2015 13:30, Wolfram Sang wrote:
> >On Mon, Mar 09, 2015 at 01:21:05PM +0100, Sebastian Hesselbarth wrote:
> >>I2C mux pinctrl driver currently determines the number of sub-busses by
> >>counting available pinctrl-names. Unfortunately, this requires each
> >>incarnation of the devicetree node with different available sub-busses
> >>to be rewritten.
> >>
> >>This patch reworks i2c-mux-pinctrl driver to count the number of
> >>available sub-nodes instead. The rework should be compatible to the old
> >>way of probing for sub-busses and additionally allows to disable unused
> >>sub-busses with standard DT property status = "disabled".
> >
> >Not sure about this change. With DYNAMIC_OF these days, you can't rely
> >that 'disabled' stays disabled all the time. My gut feeling tells me
> >that people will want to use this someday.
> 
> Possible. But this change just makes i2c-mux-pinctrl honor status
> property at all. There is no functional change except it now allows
> you to disable any of the sub-busses.

Actually, this is the feature I like. However, I wonder if we shouldn't
have that in the core, say in of_i2c_register_devices()?

> I agree that this driver still does not cope well with DYNAMIC_OF but
> neither did the former implementation. How about we settle this driver
> to this implementation now and wait for any maniac that wants to use it
> the way you are suggesting above?

Sure. I don't want you to make this driver OF_DYNAMIC compatible. I just
thought it makes it harder, though, e.g. you allocate memory for the
number of active busses not the number of possibilities, so that would
have to be reverted by the "maniac". I am still at the glimpse level,
but what if we let the mux-pinctrl parse all the data (even for disabled
busses), but only the enabled ones will get a muxed adapter because this
is handled in of_i2c_register_devices()?

> BTW, I have received a patchwork update notification - it may be
> unrelated but I prefer the Dove dts/dtsi changes to go through mvebu
> tree.

Yes, I only take dts patches in rare cases.


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

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-18 14:00           ` Wolfram Sang
@ 2015-03-18 23:10             ` Sebastian Hesselbarth
  2015-03-19 10:09               ` Wolfram Sang
  0 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-03-18 23:10 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Stephen Warren, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 18.03.2015 15:00, Wolfram Sang wrote:
> On Wed, Mar 18, 2015 at 02:23:18PM +0100, Sebastian Hesselbarth wrote:
>> Possible. But this change just makes i2c-mux-pinctrl honor status
>> property at all. There is no functional change except it now allows
>> you to disable any of the sub-busses.
>
> Actually, this is the feature I like. However, I wonder if we shouldn't
> have that in the core, say in of_i2c_register_devices()?

Hmm, looking at of_i2c_register_devices():

	for_each_available_child_of_node(adap->dev.of_node, node)
		of_i2c_register_device(adap, node);

already honors status properties by using for_each_available_foo.
Therefore, i2c-core will also skip i2c device nodes disabled by
status property.

>> I agree that this driver still does not cope well with DYNAMIC_OF but
>> neither did the former implementation. How about we settle this driver
>> to this implementation now and wait for any maniac that wants to use it
>> the way you are suggesting above?
>
> Sure. I don't want you to make this driver OF_DYNAMIC compatible. I just
> thought it makes it harder, though, e.g. you allocate memory for the
> number of active busses not the number of possibilities, so that would
> have to be reverted by the "maniac". I am still at the glimpse level,
> but what if we let the mux-pinctrl parse all the data (even for disabled
> busses), but only the enabled ones will get a muxed adapter because this
> is handled in of_i2c_register_devices()?

I am not too deep into i2c-core, but AFAIKS i2c-mux-pinctrl is not an
i2c device but an i2c mux that is dealt with differently? It is not
probed with of_i2c_register_devices() but as a separate platform_device
with a reference to the parent i2c bus.

About the memory allocation for the maximum potential number of muxes:
We would need some way to distinguish disabled from enabled muxes in
i2c-mux-pinctrl's platform_data.

i2c_mux_pinctrl_probe() is basically DT-agnostic and it should
definitely stay that way. Currently, each mux within pd->bus_count
requires a non-NULL pd->pinctrl_states[i] otherwise _probe() will bail
out for all sub-busses.

We could rework it to

(a) deal with each sub-bus individually with respect to 
pinctrl_lookup_state() and i2c_add_mux_adapter()

and

(b) allow (and skip) muxes with pinctrl_states[i] == NULL for now and
let the "maniac" deal with storing/re-probing the corresponding
pinctrl_state name once it gets dynamically enabled.

I am still not too eager working on it but if you insist, I can see
what I can do as long as Stephen sticks with testing it on Tegra. ;)

Sebastian

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-18 23:10             ` Sebastian Hesselbarth
@ 2015-03-19 10:09               ` Wolfram Sang
  2015-03-19 10:48                 ` Wolfram Sang
  2015-03-19 15:47                 ` Stephen Warren
  0 siblings, 2 replies; 50+ messages in thread
From: Wolfram Sang @ 2015-03-19 10:09 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Stephen Warren, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

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


> >>Possible. But this change just makes i2c-mux-pinctrl honor status
> >>property at all. There is no functional change except it now allows
> >>you to disable any of the sub-busses.
> >
> >Actually, this is the feature I like. However, I wonder if we shouldn't
> >have that in the core, say in of_i2c_register_devices()?
> 
> Hmm, looking at of_i2c_register_devices():
> 
> 	for_each_available_child_of_node(adap->dev.of_node, node)
> 		of_i2c_register_device(adap, node);
> 
> already honors status properties by using for_each_available_foo.
> Therefore, i2c-core will also skip i2c device nodes disabled by
> status property.

Yes, but only child nodes, not the complete bus. Here is an RFC of what
I mean:

From: Wolfram Sang <wsa@the-dreams.de>
Subject: [RFC] i2c: of: always check if busses are enabled

Allow all busses to have a "status" property which allows busses to not
be probed when set to "disabled". Needed for DTS overlays with i2c mux
scenarios.

Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
 drivers/i2c/i2c-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index fe80f85896e267..d9a3ad2149332e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1305,8 +1305,8 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
 {
 	struct device_node *node;
 
-	/* Only register child devices if the adapter has a node pointer set */
-	if (!adap->dev.of_node)
+	/* Only register childs if adapter has a node pointer with enabled status */
+	if (!adap->dev.of_node || !of_device_is_available(adap->dev.of_node))
 		return;
 
 	dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");

> I am not too deep into i2c-core, but AFAIKS i2c-mux-pinctrl is not an
> i2c device but an i2c mux that is dealt with differently? It is not
> probed with of_i2c_register_devices() but as a separate platform_device
> with a reference to the parent i2c bus.

Yes, but the busses have seperate nodes. Those nodes with the 'reg =
<0>' properties. And those are matched with of_i2c_register_devices when
the reg-property and the chan_id match.

> About the memory allocation for the maximum potential number of muxes:
> We would need some way to distinguish disabled from enabled muxes in
> i2c-mux-pinctrl's platform_data.

Do we? Can't we claim that every described bus needs a pinctrl entry.
Still, the disabled busses won't be probed? So, we could also think
about putting the above code snippet into i2c-mux when registering
busses, so not only the childs will be skipped but also the whole
bus will not be created.

> i2c_mux_pinctrl_probe() is basically DT-agnostic and it should
> definitely stay that way. Currently, each mux within pd->bus_count

I agree.

> requires a non-NULL pd->pinctrl_states[i] otherwise _probe() will bail
> out for all sub-busses.

And I think that makes sense.

> (b) allow (and skip) muxes with pinctrl_states[i] == NULL for now and
> let the "maniac" deal with storing/re-probing the corresponding
> pinctrl_state name once it gets dynamically enabled.

Why do you need the NULL?

> I am still not too eager working on it but if you insist, I can see
> what I can do as long as Stephen sticks with testing it on Tegra. ;)

Please decide if you want to work on it. Remember, I am not short of
patches to deal with.


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

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-19 10:09               ` Wolfram Sang
@ 2015-03-19 10:48                 ` Wolfram Sang
  2015-03-19 15:47                 ` Stephen Warren
  1 sibling, 0 replies; 50+ messages in thread
From: Wolfram Sang @ 2015-03-19 10:48 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	Stephen Warren, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

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

> > I am still not too eager working on it but if you insist, I can see
> > what I can do as long as Stephen sticks with testing it on Tegra. ;)
> 
> Please decide if you want to work on it. Remember, I am not short of
> patches to deal with.

Maybe sounds more harsh than it was meant. I just want to know if I need
to schedule time for this issue.


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

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-19 10:09               ` Wolfram Sang
  2015-03-19 10:48                 ` Wolfram Sang
@ 2015-03-19 15:47                 ` Stephen Warren
  2015-03-19 16:02                   ` Wolfram Sang
  1 sibling, 1 reply; 50+ messages in thread
From: Stephen Warren @ 2015-03-19 15:47 UTC (permalink / raw)
  To: Wolfram Sang, Sebastian Hesselbarth
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	linux-i2c, devicetree, linux-arm-kernel, linux-kernel

On 03/19/2015 04:09 AM, Wolfram Sang wrote:
>
>>>> Possible. But this change just makes i2c-mux-pinctrl honor status
>>>> property at all. There is no functional change except it now allows
>>>> you to disable any of the sub-busses.
>>>
>>> Actually, this is the feature I like. However, I wonder if we shouldn't
>>> have that in the core, say in of_i2c_register_devices()?
>>
>> Hmm, looking at of_i2c_register_devices():
>>
>> 	for_each_available_child_of_node(adap->dev.of_node, node)
>> 		of_i2c_register_device(adap, node);
>>
>> already honors status properties by using for_each_available_foo.
>> Therefore, i2c-core will also skip i2c device nodes disabled by
>> status property.
>
> Yes, but only child nodes, not the complete bus. Here is an RFC of what
> I mean:
>
> From: Wolfram Sang <wsa@the-dreams.de>
> Subject: [RFC] i2c: of: always check if busses are enabled
>
> Allow all busses to have a "status" property which allows busses to not
> be probed when set to "disabled". Needed for DTS overlays with i2c mux
> scenarios.

> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c

> @@ -1305,8 +1305,8 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
>   {
>   	struct device_node *node;
>
> -	/* Only register child devices if the adapter has a node pointer set */
> -	if (!adap->dev.of_node)
> +	/* Only register childs if adapter has a node pointer with enabled status */
> +	if (!adap->dev.of_node || !of_device_is_available(adap->dev.of_node))
>   		return;

That feels a bit odd to me. For a regular non-mux I2C controller, that 
extra case would never trigger if the controller node was disabled, 
since the device core would never probe the controller device itself. 
So, we'd end up with inconsistent paths through the I2C core for regular 
controllers and muxes.

Perhaps better would be to have a mux-specific function to iterate over 
a mux's child nodes and instantiate buses for those. That function would 
check whether each bus node was disabled or not. That'd isolate the 
special case into the place where it was relevant.

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-19 15:47                 ` Stephen Warren
@ 2015-03-19 16:02                   ` Wolfram Sang
  2015-03-19 16:49                     ` Stephen Warren
  2015-03-19 20:52                     ` Sebastian Hesselbarth
  0 siblings, 2 replies; 50+ messages in thread
From: Wolfram Sang @ 2015-03-19 16:02 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Sebastian Hesselbarth, Jason Cooper, Andrew Lunn,
	Gregory Clement, Gabriel Dobato, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel

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

> >-	/* Only register child devices if the adapter has a node pointer set */
> >-	if (!adap->dev.of_node)
> >+	/* Only register childs if adapter has a node pointer with enabled status */
> >+	if (!adap->dev.of_node || !of_device_is_available(adap->dev.of_node))
> >  		return;
> 
> That feels a bit odd to me. For a regular non-mux I2C controller, that extra
> case would never trigger if the controller node was disabled, since the
> device core would never probe the controller device itself. So, we'd end up
> with inconsistent paths through the I2C core for regular controllers and
> muxes.

I first thought the no-op for the non-mux case wouldn't hurt, but I
agree about the consistent code path. I mentioned in my previous mail
that i2c-mux might be a better place for this...

> Perhaps better would be to have a mux-specific function to iterate over a
> mux's child nodes and instantiate buses for those. That function would check
> whether each bus node was disabled or not. That'd isolate the special case
> into the place where it was relevant.

... so I wonder what you think about putting the
of_device_is_available() check into i2c_add_mux_adapter() once the
reg-property and chan_id have been matched?


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

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-19 16:02                   ` Wolfram Sang
@ 2015-03-19 16:49                     ` Stephen Warren
  2015-03-19 20:52                     ` Sebastian Hesselbarth
  1 sibling, 0 replies; 50+ messages in thread
From: Stephen Warren @ 2015-03-19 16:49 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Sebastian Hesselbarth, Jason Cooper, Andrew Lunn,
	Gregory Clement, Gabriel Dobato, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel

On 03/19/2015 10:02 AM, Wolfram Sang wrote:
>>> -	/* Only register child devices if the adapter has a node pointer set */
>>> -	if (!adap->dev.of_node)
>>> +	/* Only register childs if adapter has a node pointer with enabled status */
>>> +	if (!adap->dev.of_node || !of_device_is_available(adap->dev.of_node))
>>>   		return;
>>
>> That feels a bit odd to me. For a regular non-mux I2C controller, that extra
>> case would never trigger if the controller node was disabled, since the
>> device core would never probe the controller device itself. So, we'd end up
>> with inconsistent paths through the I2C core for regular controllers and
>> muxes.
>
> I first thought the no-op for the non-mux case wouldn't hurt, but I
> agree about the consistent code path. I mentioned in my previous mail
> that i2c-mux might be a better place for this...
>
>> Perhaps better would be to have a mux-specific function to iterate over a
>> mux's child nodes and instantiate buses for those. That function would check
>> whether each bus node was disabled or not. That'd isolate the special case
>> into the place where it was relevant.
>
> ... so I wonder what you think about putting the
> of_device_is_available() check into i2c_add_mux_adapter() once the
> reg-property and chan_id have been matched?

I think that looks like a good place, yes.

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-19 16:02                   ` Wolfram Sang
  2015-03-19 16:49                     ` Stephen Warren
@ 2015-03-19 20:52                     ` Sebastian Hesselbarth
  2015-03-20 10:19                       ` Wolfram Sang
  1 sibling, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-03-19 20:52 UTC (permalink / raw)
  To: Wolfram Sang, Stephen Warren
  Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Gabriel Dobato,
	linux-i2c, devicetree, linux-arm-kernel, linux-kernel

On 19.03.2015 17:02, Wolfram Sang wrote:
>> Perhaps better would be to have a mux-specific function to iterate over a
>> mux's child nodes and instantiate buses for those. That function would check
>> whether each bus node was disabled or not. That'd isolate the special case
>> into the place where it was relevant.
>
> ... so I wonder what you think about putting the
> of_device_is_available() check into i2c_add_mux_adapter() once the
> reg-property and chan_id have been matched?
>

Ok, I see what you mean. I had a look at the place in question and
wonder what to return from i2c_add_mux_adapter() in the disabled
case so that i2c-mux-pinctrl is still happy with the returned value.

I guess what you want to have is that i2c_add_adapter() is not called
for the disabled case, right?

Is the i2c_adapter struct prepared in i2c_mux_add_adapter() still valid
if i2c_add_adapter() is not called?

Sorry, I am not too deep into i2c subsystem, I just reworked i2c-mux-
pinctrl to make it work on Dove. If you are fine with giving me some
guidance how you prefer to have it done, I can try to free some spare
time. Unfortunately there is already little of it, so please don't
expect a quick tested patch.

Sebastian

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-19 20:52                     ` Sebastian Hesselbarth
@ 2015-03-20 10:19                       ` Wolfram Sang
  2015-03-21 21:00                         ` Wolfram Sang
  0 siblings, 1 reply; 50+ messages in thread
From: Wolfram Sang @ 2015-03-20 10:19 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Stephen Warren, Jason Cooper, Andrew Lunn, Gregory Clement,
	Gabriel Dobato, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

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


> Ok, I see what you mean. I had a look at the place in question and
> wonder what to return from i2c_add_mux_adapter() in the disabled
> case so that i2c-mux-pinctrl is still happy with the returned value.

Ouch, you are right. The crux of interfaces returning NULL instead of an
ERR_PTR :( I'll have a look, I maybe started to fix this somewhen.

> I guess what you want to have is that i2c_add_adapter() is not called
> for the disabled case, right?

I think that makes sense.

> Is the i2c_adapter struct prepared in i2c_mux_add_adapter() still valid
> if i2c_add_adapter() is not called?

I will have a closer look to the issue this weekend.

> Sorry, I am not too deep into i2c subsystem, I just reworked i2c-mux-
> pinctrl to make it work on Dove. If you are fine with giving me some
> guidance how you prefer to have it done, I can try to free some spare
> time.

Cool, thanks. Learning by doing is a good way to get such knowledge :)

> Unfortunately there is already little of it, so please don't
> expect a quick tested patch.

I understand.

Thanks,

   Wolfram


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

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-20 10:19                       ` Wolfram Sang
@ 2015-03-21 21:00                         ` Wolfram Sang
  2015-03-22 13:03                           ` Sebastian Hesselbarth
  0 siblings, 1 reply; 50+ messages in thread
From: Wolfram Sang @ 2015-03-21 21:00 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Stephen Warren, Jason Cooper, Andrew Lunn, Gregory Clement,
	Gabriel Dobato, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

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


> > I guess what you want to have is that i2c_add_adapter() is not called
> > for the disabled case, right?
> 
> I think that makes sense.

But maybe we should just start simple and keep calling i2c_add_adapter()
for the disabled case. We will just skip probing devices on the bus.
Would that help the issue you were originally trying to solve?



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

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-21 21:00                         ` Wolfram Sang
@ 2015-03-22 13:03                           ` Sebastian Hesselbarth
  2015-03-23 18:32                             ` Wolfram Sang
  0 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-03-22 13:03 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Stephen Warren, Jason Cooper, Andrew Lunn, Gregory Clement,
	Gabriel Dobato, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 21.03.2015 22:00, Wolfram Sang wrote:
>>> I guess what you want to have is that i2c_add_adapter() is not called
>>> for the disabled case, right?
>>
>> I think that makes sense.
>
> But maybe we should just start simple and keep calling i2c_add_adapter()
> for the disabled case. We will just skip probing devices on the bus.
> Would that help the issue you were originally trying to solve?

It is not about probing devices on the mux sub-busses, I'd expect no
devices on the optional sub-busses in DT on boards where those pins
are not used as i2c.

The idea was to hide those busses completely in particular from
userspace on boards where they'll never be available.

If modifying i2c-mux-pinctrl to respect the sub-bus status property is
such a big issue, I'd rather leave the driver as is and expose all
sub-busses to userspace.

Sebastian


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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-22 13:03                           ` Sebastian Hesselbarth
@ 2015-03-23 18:32                             ` Wolfram Sang
  2015-03-27 21:08                               ` Sebastian Hesselbarth
  0 siblings, 1 reply; 50+ messages in thread
From: Wolfram Sang @ 2015-03-23 18:32 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Stephen Warren, Jason Cooper, Andrew Lunn, Gregory Clement,
	Gabriel Dobato, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel


> If modifying i2c-mux-pinctrl to respect the sub-bus status property is
> such a big issue, I'd rather leave the driver as is and expose all
> sub-busses to userspace.

Well, dunno what 'big issue' is in your book :) What definately needs to
be done is:

* handle "status" at mux-core level, not mux-driver
* that probably needs us to convert i2c_add_mux_adapter() to return
  ERR_PTRs instead of NULL, so we can distinguish the "disabled" case
* that would mean to fix all existing users

That's all not groundbreaking stuff, but needs caution and thoroughness.
There might be some gory details left, though...

Regards,

   Wolfram

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-23 18:32                             ` Wolfram Sang
@ 2015-03-27 21:08                               ` Sebastian Hesselbarth
  2015-04-03 18:17                                 ` Wolfram Sang
  0 siblings, 1 reply; 50+ messages in thread
From: Sebastian Hesselbarth @ 2015-03-27 21:08 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Stephen Warren, Jason Cooper, Andrew Lunn, Gregory Clement,
	Gabriel Dobato, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

On 23.03.2015 19:32, Wolfram Sang wrote:
>> If modifying i2c-mux-pinctrl to respect the sub-bus status property is
>> such a big issue, I'd rather leave the driver as is and expose all
>> sub-busses to userspace.
>
> Well, dunno what 'big issue' is in your book :) What definately needs to
> be done is:

Wolfram,

I had a look at the code in question again and prepared some patches
to return ERR_PTRs from i2c_add_mux_adapter(), rework users to check
for IS_ERR() and finally skip i2c_add_adapter for the OF disabled case.

I have them compile-tested but I don't have any hardware available
right now.

Anyway, I still have some questions.

> * handle "status" at mux-core level, not mux-driver

I get that.. but:

> * that probably needs us to convert i2c_add_mux_adapter() to return
>    ERR_PTRs instead of NULL, so we can distinguish the "disabled" case

What do you want to return from i2c_add_mux_adapter() if you find an OF
disabled child node?

AFAIKS, there is no explicit errno for a disabled device (node) so all
I can imagine here is to return either the &priv->adap before actually
calling i2c_add_adapter on it or NULL now that we explicitly check for
errors.

> * that would mean to fix all existing users

If we choose to return NULL, those users who can deal with a
missing/disabled sub-bus on the mux can check with IS_ERR()
the others should check with IS_ERR_OR_NULL().

We could also choose to return some other errno (-ENODEV maybe)
but still we'd have to double-check that return value on i2c-mux-pinctrl
and the others too if we don't care that i2c_add_adapter() wasn't
called for a mux.

> That's all not groundbreaking stuff, but needs caution and thoroughness.
> There might be some gory details left, though...

As I said before, the intention was not disable a possible i2c bus that
can be dynamically added/removed on that specific Dove SBC/CM-A510
board but to have a single i2c-mux-pinctrl in the SoC dtsi just because
the SoC has the optional i2c bus muxing.

While thinking about it (and I still think of it as a 'big issue'
compared to the intention of the initial patch) I came to the
conclusion that I should maybe just go for a board-specific
i2c-mux-pinctrl node instead of adding it to the SoC dtsi. That will
also avoid doubled i2c busses on boards with just the default i2c
option.

Sebastian

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

* Re: [PATCH v3 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes
  2015-03-27 21:08                               ` Sebastian Hesselbarth
@ 2015-04-03 18:17                                 ` Wolfram Sang
  0 siblings, 0 replies; 50+ messages in thread
From: Wolfram Sang @ 2015-04-03 18:17 UTC (permalink / raw)
  To: Sebastian Hesselbarth
  Cc: Stephen Warren, Jason Cooper, Andrew Lunn, Gregory Clement,
	Gabriel Dobato, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel

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


> While thinking about it (and I still think of it as a 'big issue'
> compared to the intention of the initial patch) I came to the
> conclusion that I should maybe just go for a board-specific
> i2c-mux-pinctrl node instead of adding it to the SoC dtsi. That will
> also avoid doubled i2c busses on boards with just the default i2c
> option.

Ehrm, then please let me know what you decided on. If you chose the
above road, then I don't need to think about the other questions :)


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

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

end of thread, other threads:[~2015-04-03 18:17 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-17 18:52 [PATCH 0/8] Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
2015-02-17 18:52 ` [PATCH 1/8] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
2015-02-17 20:46   ` Stephen Warren
2015-02-17 21:08     ` Sebastian Hesselbarth
2015-02-17 21:15       ` Stephen Warren
2015-02-17 21:19         ` Sebastian Hesselbarth
2015-02-26 21:46   ` Stephen Warren
2015-02-17 18:52 ` [PATCH 2/8] devicetree: vendor-prefixes: Add CompuLab to known vendors Sebastian Hesselbarth
2015-02-17 19:37   ` Rob Herring
2015-02-17 18:52 ` [PATCH 3/8] ARM: dts: dove: Fix uart[23] reg property Sebastian Hesselbarth
2015-02-23 14:54   ` Gregory CLEMENT
2015-02-17 18:52 ` [PATCH 4/8] ARM: dts: dove: Always include gpio and interrupt-controller headers Sebastian Hesselbarth
2015-02-23 14:54   ` Gregory CLEMENT
2015-02-17 18:52 ` [PATCH 5/8] ARM: dts: dove: Add node labels for PCIe ports 0 and 1 Sebastian Hesselbarth
2015-02-23 14:55   ` Gregory CLEMENT
2015-02-17 18:52 ` [PATCH 6/8] ARM: dts: dove: Add some more common pinctrl settings Sebastian Hesselbarth
2015-02-23 14:56   ` Gregory CLEMENT
2015-02-17 18:52 ` [PATCH 7/8] ARM: dts: dove: Add internal i2c multiplexer node Sebastian Hesselbarth
2015-02-23 15:07   ` Gregory CLEMENT
2015-02-17 18:52 ` [PATCH 8/8] ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth
2015-02-26 17:55 ` [PATCH 0/8] " Gregory CLEMENT
2015-02-26 19:39   ` Sebastian Hesselbarth
2015-02-26 20:01     ` Stephen Warren
2015-02-26 20:35       ` Sebastian Hesselbarth
2015-02-27 12:24 ` [PATCH v2 0/4] " Sebastian Hesselbarth
2015-02-27 12:24   ` [PATCH v2 1/4] i2c: mux-pinctrl: Rework to honor disabled child nodes Sebastian Hesselbarth
2015-03-02 20:01     ` Stephen Warren
2015-03-04  9:10       ` Sebastian Hesselbarth
2015-03-09 12:21     ` [PATCH v3 " Sebastian Hesselbarth
2015-03-10 16:28       ` Stephen Warren
2015-03-16 20:15         ` Sebastian Hesselbarth
2015-03-18 12:30       ` Wolfram Sang
2015-03-18 13:23         ` Sebastian Hesselbarth
2015-03-18 14:00           ` Wolfram Sang
2015-03-18 23:10             ` Sebastian Hesselbarth
2015-03-19 10:09               ` Wolfram Sang
2015-03-19 10:48                 ` Wolfram Sang
2015-03-19 15:47                 ` Stephen Warren
2015-03-19 16:02                   ` Wolfram Sang
2015-03-19 16:49                     ` Stephen Warren
2015-03-19 20:52                     ` Sebastian Hesselbarth
2015-03-20 10:19                       ` Wolfram Sang
2015-03-21 21:00                         ` Wolfram Sang
2015-03-22 13:03                           ` Sebastian Hesselbarth
2015-03-23 18:32                             ` Wolfram Sang
2015-03-27 21:08                               ` Sebastian Hesselbarth
2015-04-03 18:17                                 ` Wolfram Sang
2015-02-27 12:24   ` [PATCH v2 2/4] devicetree: vendor-prefixes: Add CompuLab to known vendors Sebastian Hesselbarth
2015-02-27 12:24   ` [PATCH v2 3/4] ARM: dts: dove: Add internal i2c multiplexer node Sebastian Hesselbarth
2015-02-27 12:24   ` [PATCH v2 4/4] ARM: dts: dove: Add proper support for Compulab CM-A510/SBC-A510 Sebastian Hesselbarth

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