All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v2 0/4] i2c/of: switch I2C IP cores at runtime via OF_DYNAMIC
@ 2016-01-06 13:51 ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-06 13:51 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

I know this is gonna be a controversial series, but we have a usecase for this :)

This series allows an I2C bus to switch between multiple masters, i.e. a
n-to-1-demuxer. This is not hot-switching because connected I2C slaves will be
re-instantiated. It is meant to select the best I2C core at runtime once the
task is known. Example: Prefer i2c-gpio over another I2C core because of HW
errata affecting your current runtime configuration.

It works by using OF_DYNAMIC and en-/disabling the i2c parent as needed. See
the binding docs for more details. Because this is largely using OF_DYNAMIC, I
decided to post the whole series to the dt list.

Changes since RFC v1:

* gracefully handle if i2c adapters are not present at runtime
  (driver not loaded yet)
* added more documentation and examples
* properly put the i2c adapters after use
* respect PAGE_SIZE in sysfs_show

This has been tested on a Renesas Lager board switching between i2c-gpio and
two different IP cores (i2c-rcar and i2c-sh_mobile). The rebinding seems to be
working as expected. However, in practice, I couldn't use the HDMI i2c slaves
with another controller yet, because the rcar-du driver OOPSes when unbinding.
This seems unrelated to this series because it can also be triggered via
sysfs-unbind. soc-camera also had problems properly cleaning up on unbind, a
patch for this already has been sent. So, this series is for sure a good test
for the unbind path of drivers.

Let me know what you think.

Thanks,

   Wolfram


Wolfram Sang (4):
  of: make of_mutex public
  dt-bindings: i2c: mux: demux-pinctrl: add bindings
  i2c: mux: demux-pinctrl: add driver
  ARM: shmobile: r8a7790: rework dts to use i2c demuxer

 .../devicetree/bindings/i2c/i2c-demux-pinctrl.txt  | 155 ++++++++++++
 arch/arm/boot/dts/r8a7790-lager.dts                | 141 +++++++----
 drivers/i2c/muxes/Kconfig                          |   9 +
 drivers/i2c/muxes/Makefile                         |   2 +
 drivers/i2c/muxes/i2c-demux-pinctrl.c              | 276 +++++++++++++++++++++
 drivers/of/of_private.h                            |   1 -
 include/linux/of.h                                 |   2 +
 7 files changed, 532 insertions(+), 54 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
 create mode 100644 drivers/i2c/muxes/i2c-demux-pinctrl.c

-- 
2.1.4


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

* [RFC v2 0/4] i2c/of: switch I2C IP cores at runtime via OF_DYNAMIC
@ 2016-01-06 13:51 ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-06 13:51 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

I know this is gonna be a controversial series, but we have a usecase for this :)

This series allows an I2C bus to switch between multiple masters, i.e. a
n-to-1-demuxer. This is not hot-switching because connected I2C slaves will be
re-instantiated. It is meant to select the best I2C core at runtime once the
task is known. Example: Prefer i2c-gpio over another I2C core because of HW
errata affecting your current runtime configuration.

It works by using OF_DYNAMIC and en-/disabling the i2c parent as needed. See
the binding docs for more details. Because this is largely using OF_DYNAMIC, I
decided to post the whole series to the dt list.

Changes since RFC v1:

* gracefully handle if i2c adapters are not present at runtime
  (driver not loaded yet)
* added more documentation and examples
* properly put the i2c adapters after use
* respect PAGE_SIZE in sysfs_show

This has been tested on a Renesas Lager board switching between i2c-gpio and
two different IP cores (i2c-rcar and i2c-sh_mobile). The rebinding seems to be
working as expected. However, in practice, I couldn't use the HDMI i2c slaves
with another controller yet, because the rcar-du driver OOPSes when unbinding.
This seems unrelated to this series because it can also be triggered via
sysfs-unbind. soc-camera also had problems properly cleaning up on unbind, a
patch for this already has been sent. So, this series is for sure a good test
for the unbind path of drivers.

Let me know what you think.

Thanks,

   Wolfram


Wolfram Sang (4):
  of: make of_mutex public
  dt-bindings: i2c: mux: demux-pinctrl: add bindings
  i2c: mux: demux-pinctrl: add driver
  ARM: shmobile: r8a7790: rework dts to use i2c demuxer

 .../devicetree/bindings/i2c/i2c-demux-pinctrl.txt  | 155 ++++++++++++
 arch/arm/boot/dts/r8a7790-lager.dts                | 141 +++++++----
 drivers/i2c/muxes/Kconfig                          |   9 +
 drivers/i2c/muxes/Makefile                         |   2 +
 drivers/i2c/muxes/i2c-demux-pinctrl.c              | 276 +++++++++++++++++++++
 drivers/of/of_private.h                            |   1 -
 include/linux/of.h                                 |   2 +
 7 files changed, 532 insertions(+), 54 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
 create mode 100644 drivers/i2c/muxes/i2c-demux-pinctrl.c

-- 
2.1.4


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

* [RFC v2 1/4] of: make of_mutex public
  2016-01-06 13:51 ` Wolfram Sang
@ 2016-01-06 13:51   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-06 13:51 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

If we want to use OF_DYNAMIC features outside the of framework, we need
to access this lock.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/of/of_private.h | 1 -
 include/linux/of.h      | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 8e882e706cd8c6..f92ec41efb5dfd 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -31,7 +31,6 @@ struct alias_prop {
 	char stem[0];
 };
 
-extern struct mutex of_mutex;
 extern struct list_head aliases_lookup;
 extern struct kset *of_kset;
 
diff --git a/include/linux/of.h b/include/linux/of.h
index dd10626a615fb1..acac979063f067 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -32,6 +32,8 @@
 typedef u32 phandle;
 typedef u32 ihandle;
 
+extern struct mutex of_mutex;
+
 struct property {
 	char	*name;
 	int	length;
-- 
2.1.4


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

* [RFC v2 1/4] of: make of_mutex public
@ 2016-01-06 13:51   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-06 13:51 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

If we want to use OF_DYNAMIC features outside the of framework, we need
to access this lock.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/of/of_private.h | 1 -
 include/linux/of.h      | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 8e882e706cd8c6..f92ec41efb5dfd 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -31,7 +31,6 @@ struct alias_prop {
 	char stem[0];
 };
 
-extern struct mutex of_mutex;
 extern struct list_head aliases_lookup;
 extern struct kset *of_kset;
 
diff --git a/include/linux/of.h b/include/linux/of.h
index dd10626a615fb1..acac979063f067 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -32,6 +32,8 @@
 typedef u32 phandle;
 typedef u32 ihandle;
 
+extern struct mutex of_mutex;
+
 struct property {
 	char	*name;
 	int	length;
-- 
2.1.4


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

* [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
  2016-01-06 13:51 ` Wolfram Sang
@ 2016-01-06 13:51   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-06 13:51 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

These bindings allow an I2C bus to switch between multiple masters. This
is not hot-swichting because connected I2C slaves will be
re-instantiated. It is meant to select the best I2C core at runtime once
the task is known. Example: Prefer i2c-gpio over another I2C core
because of HW errata affetcing your use case.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 .../devicetree/bindings/i2c/i2c-demux-pinctrl.txt  | 155 +++++++++++++++++++++
 1 file changed, 155 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
new file mode 100644
index 00000000000000..de571be68f4754
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
@@ -0,0 +1,155 @@
+Pinctrl-based I2C Bus DeMux
+
+This binding describes an I2C bus demultiplexer that uses pin multiplexing to
+route the I2C signals, and represents the pin multiplexing configuration using
+the pinctrl device tree bindings. This may be used to select one I2C IP core at
+runtime which may have a better feature set for a given task than another I2C
+IP core on the SoC. The most simple example is to fall back to GPIO bitbanging
+if your current runtime configuration hits an errata of the internal IP core.
+
+    +-------------------------------+
+    | SoC                           |
+    |                               |   +-----+  +-----+
+    |   +------------+              |   | dev |  | dev |
+    |   |I2C IP Core1|--\           |   +-----+  +-----+
+    |   +------------+   \-------+  |      |        |
+    |                    |Pinctrl|--|------+--------+
+    |   +------------+   +-------+  |
+    |   |I2C IP Core2|--/           |
+    |   +------------+              |
+    |                               |
+    +-------------------------------+
+
+Required properties:
+- compatible: "i2c-demux-pinctrl"
+- i2c-parent: List of phandles of I2C masters available for selection. The first
+	      one will be used as default.
+- i2c-bus-name: The name of this bus. Also needed as pinctrl-name for the I2C
+		parents.
+
+Furthermore, I2C mux properties and child nodes. See mux.txt in this directory.
+
+Example:
+
+Here is a snipplet for a bus to be demuxed. It contains various i2c clients for
+HDMI, so the bus is named "i2c-hdmi":
+
+	i2chdmi: i2c@8 {
+
+		compatible = "i2c-demux-pinctrl";
+		i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
+		i2c-bus-name = "i2c-hdmi";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ak4643: sound-codec@12 {
+			compatible = "asahi-kasei,ak4643";
+
+			#sound-dai-cells = <0>;
+			reg = <0x12>;
+		};
+
+		composite-in@20 {
+			compatible = "adi,adv7180";
+			reg = <0x20>;
+			remote = <&vin1>;
+
+			port {
+				adv7180: endpoint {
+					bus-width = <8>;
+					remote-endpoint = <&vin1ep0>;
+				};
+			};
+		};
+
+		hdmi@39 {
+			compatible = "adi,adv7511w";
+			reg = <0x39>;
+			interrupt-parent = <&gpio1>;
+			interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
+
+			adi,input-depth = <8>;
+			adi,input-colorspace = "rgb";
+			adi,input-clock = "1x";
+			adi,input-style = <1>;
+			adi,input-justification = "evenly";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					adv7511_in: endpoint {
+						remote-endpoint = <&du_out_lvds0>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					adv7511_out: endpoint {
+						remote-endpoint = <&hdmi_con>;
+					};
+				};
+			};
+		};
+	};
+
+And for clarification, here are the snipplets for the i2c-parents:
+
+	gpioi2c: i2c@9 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "i2c-gpio";
+		status = "disabled";
+		gpios = <&gpio5 6 GPIO_ACTIVE_HIGH /* sda */
+			 &gpio5 5 GPIO_ACTIVE_HIGH /* scl */
+			>;
+		i2c-gpio,delay-us = <5>;
+	};
+
+...
+
+&i2c2	{
+	pinctrl-0 = <&i2c2_pins>;
+	pinctrl-names = "i2c-hdmi";
+
+	clock-frequency = <100000>;
+};
+
+...
+
+&iic2	{
+	pinctrl-0 = <&iic2_pins>;
+	pinctrl-names = "i2c-hdmi";
+
+	clock-frequency = <100000>;
+};
+
+Please note:
+
+- pinctrl properties for the parent I2C controllers need a pinctrl state
+  with the same name as i2c-bus-name, not "default"!
+
+- the i2c masters must have their status "disabled". This driver will
+  enable them at runtime when needed.
+
+Changing I2C controllers:
+
+The created mux-device will have a file "cur_master" in its sysfs-entry. Write
+0 there for the first master listed in the "i2c-parent" property, 1 for the
+second etc. Reading the file will give you a list with the active master
+marked. Example from a Renesas Lager board:
+
+root@Lager:~# cat /sys/bus/platform/devices/i2c@8/cur_master
+* 0 - /i2c@9
+  1 - /i2c@e6520000
+  2 - /i2c@e6530000
+
+root@Lager:~# echo 2 > /sys/bus/platform/devices/i2c@8/cur_master
+
+root@Lager:~# cat /sys/bus/platform/devices/i2c@8/cur_master
+  0 - /i2c@9
+  1 - /i2c@e6520000
+* 2 - /i2c@e6530000
+
-- 
2.1.4


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

* [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
@ 2016-01-06 13:51   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-06 13:51 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

These bindings allow an I2C bus to switch between multiple masters. This
is not hot-swichting because connected I2C slaves will be
re-instantiated. It is meant to select the best I2C core at runtime once
the task is known. Example: Prefer i2c-gpio over another I2C core
because of HW errata affetcing your use case.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 .../devicetree/bindings/i2c/i2c-demux-pinctrl.txt  | 155 +++++++++++++++++++++
 1 file changed, 155 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
new file mode 100644
index 00000000000000..de571be68f4754
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
@@ -0,0 +1,155 @@
+Pinctrl-based I2C Bus DeMux
+
+This binding describes an I2C bus demultiplexer that uses pin multiplexing to
+route the I2C signals, and represents the pin multiplexing configuration using
+the pinctrl device tree bindings. This may be used to select one I2C IP core at
+runtime which may have a better feature set for a given task than another I2C
+IP core on the SoC. The most simple example is to fall back to GPIO bitbanging
+if your current runtime configuration hits an errata of the internal IP core.
+
+    +-------------------------------+
+    | SoC                           |
+    |                               |   +-----+  +-----+
+    |   +------------+              |   | dev |  | dev |
+    |   |I2C IP Core1|--\           |   +-----+  +-----+
+    |   +------------+   \-------+  |      |        |
+    |                    |Pinctrl|--|------+--------+
+    |   +------------+   +-------+  |
+    |   |I2C IP Core2|--/           |
+    |   +------------+              |
+    |                               |
+    +-------------------------------+
+
+Required properties:
+- compatible: "i2c-demux-pinctrl"
+- i2c-parent: List of phandles of I2C masters available for selection. The first
+	      one will be used as default.
+- i2c-bus-name: The name of this bus. Also needed as pinctrl-name for the I2C
+		parents.
+
+Furthermore, I2C mux properties and child nodes. See mux.txt in this directory.
+
+Example:
+
+Here is a snipplet for a bus to be demuxed. It contains various i2c clients for
+HDMI, so the bus is named "i2c-hdmi":
+
+	i2chdmi: i2c@8 {
+
+		compatible = "i2c-demux-pinctrl";
+		i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
+		i2c-bus-name = "i2c-hdmi";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ak4643: sound-codec@12 {
+			compatible = "asahi-kasei,ak4643";
+
+			#sound-dai-cells = <0>;
+			reg = <0x12>;
+		};
+
+		composite-in@20 {
+			compatible = "adi,adv7180";
+			reg = <0x20>;
+			remote = <&vin1>;
+
+			port {
+				adv7180: endpoint {
+					bus-width = <8>;
+					remote-endpoint = <&vin1ep0>;
+				};
+			};
+		};
+
+		hdmi@39 {
+			compatible = "adi,adv7511w";
+			reg = <0x39>;
+			interrupt-parent = <&gpio1>;
+			interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
+
+			adi,input-depth = <8>;
+			adi,input-colorspace = "rgb";
+			adi,input-clock = "1x";
+			adi,input-style = <1>;
+			adi,input-justification = "evenly";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					adv7511_in: endpoint {
+						remote-endpoint = <&du_out_lvds0>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					adv7511_out: endpoint {
+						remote-endpoint = <&hdmi_con>;
+					};
+				};
+			};
+		};
+	};
+
+And for clarification, here are the snipplets for the i2c-parents:
+
+	gpioi2c: i2c@9 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "i2c-gpio";
+		status = "disabled";
+		gpios = <&gpio5 6 GPIO_ACTIVE_HIGH /* sda */
+			 &gpio5 5 GPIO_ACTIVE_HIGH /* scl */
+			>;
+		i2c-gpio,delay-us = <5>;
+	};
+
+...
+
+&i2c2	{
+	pinctrl-0 = <&i2c2_pins>;
+	pinctrl-names = "i2c-hdmi";
+
+	clock-frequency = <100000>;
+};
+
+...
+
+&iic2	{
+	pinctrl-0 = <&iic2_pins>;
+	pinctrl-names = "i2c-hdmi";
+
+	clock-frequency = <100000>;
+};
+
+Please note:
+
+- pinctrl properties for the parent I2C controllers need a pinctrl state
+  with the same name as i2c-bus-name, not "default"!
+
+- the i2c masters must have their status "disabled". This driver will
+  enable them at runtime when needed.
+
+Changing I2C controllers:
+
+The created mux-device will have a file "cur_master" in its sysfs-entry. Write
+0 there for the first master listed in the "i2c-parent" property, 1 for the
+second etc. Reading the file will give you a list with the active master
+marked. Example from a Renesas Lager board:
+
+root@Lager:~# cat /sys/bus/platform/devices/i2c@8/cur_master
+* 0 - /i2c@9
+  1 - /i2c@e6520000
+  2 - /i2c@e6530000
+
+root@Lager:~# echo 2 > /sys/bus/platform/devices/i2c@8/cur_master
+
+root@Lager:~# cat /sys/bus/platform/devices/i2c@8/cur_master
+  0 - /i2c@9
+  1 - /i2c@e6520000
+* 2 - /i2c@e6530000
+
-- 
2.1.4


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

* [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
  2016-01-06 13:51 ` Wolfram Sang
@ 2016-01-06 13:51   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-06 13:51 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

This driver allows an I2C bus to switch between multiple masters. This
is not hot-swichting because connected I2C slaves will be
re-instantiated. It is meant to select the best I2C core at runtime once
the task is known. Example: Prefer i2c-gpio over another I2C core
because of HW errata affetcing your use case.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/muxes/Kconfig             |   9 ++
 drivers/i2c/muxes/Makefile            |   2 +
 drivers/i2c/muxes/i2c-demux-pinctrl.c | 276 ++++++++++++++++++++++++++++++++++
 3 files changed, 287 insertions(+)
 create mode 100644 drivers/i2c/muxes/i2c-demux-pinctrl.c

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index f06b0e24673b87..7b5da5ff7b16f9 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -72,4 +72,13 @@ config I2C_MUX_REG
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mux-reg.
 
+config I2C_DEMUX_PINCTRL
+	tristate "pinctrl-based I2C demultiplexer"
+	depends on PINCTRL
+	select OF_DYNAMIC
+	help
+	  If you say yes to this option, support will be included for an I2C
+	  demultiplexer that uses the pinctrl subsystem. This is useful if you
+	  want to change the I2C master at run-time depending on features.
+
 endmenu
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index e89799b76a9280..7c267c29b19196 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -3,6 +3,8 @@
 
 obj-$(CONFIG_I2C_ARB_GPIO_CHALLENGE)	+= i2c-arb-gpio-challenge.o
 
+obj-$(CONFIG_I2C_DEMUX_PINCTRL)		+= i2c-demux-pinctrl.o
+
 obj-$(CONFIG_I2C_MUX_GPIO)	+= i2c-mux-gpio.o
 obj-$(CONFIG_I2C_MUX_PCA9541)	+= i2c-mux-pca9541.o
 obj-$(CONFIG_I2C_MUX_PCA954x)	+= i2c-mux-pca954x.o
diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c
new file mode 100644
index 00000000000000..5ec2779b176432
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
@@ -0,0 +1,276 @@
+/*
+ * Pinctrl based I2C DeMultiplexer
+ *
+ * Copyright (C) 2015-16 by Wolfram Sang, Sang Engineering <wsa@sang-engineering.com>
+ * Copyright (C) 2015-16 by Renesas Electronics Corporation
+ *
+ * This program 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; version 2 of the License.
+ *
+ * See the bindings doc for DTS setup.
+ */
+
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+
+struct i2c_demux_pinctrl_chan {
+	struct device_node *parent_np;
+	struct i2c_adapter *parent_adap;
+	struct of_changeset chgset;
+};
+
+struct i2c_demux_pinctrl_priv {
+	int cur_chan;
+	int num_chan;
+	struct device *dev;
+	const char *bus_name;
+	struct i2c_adapter cur_adap;
+	struct i2c_algorithm algo;
+	struct i2c_demux_pinctrl_chan chan[];
+};
+
+static struct property status_okay = { .name = "status", .length = 3, .value = "ok" };
+
+static int i2c_demux_master_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
+{
+	struct i2c_demux_pinctrl_priv *priv = adap->algo_data;
+	struct i2c_adapter *parent = priv->chan[priv->cur_chan].parent_adap;
+
+	return __i2c_transfer(parent, msgs, num);
+}
+
+static u32 i2c_demux_functionality(struct i2c_adapter *adap)
+{
+	struct i2c_demux_pinctrl_priv *priv = adap->algo_data;
+	struct i2c_adapter *parent = priv->chan[priv->cur_chan].parent_adap;
+
+	return parent->algo->functionality(parent);
+}
+
+static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 new_chan)
+{
+	struct i2c_adapter *adap;
+	struct pinctrl *p;
+	int ret;
+
+	mutex_lock(&of_mutex);
+	ret = of_changeset_apply(&priv->chan[new_chan].chgset);
+	mutex_unlock(&of_mutex);
+	if (ret)
+		goto err;
+
+	adap = of_find_i2c_adapter_by_node(priv->chan[new_chan].parent_np);
+	if (!adap) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	p = devm_pinctrl_get_select(adap->dev.parent, priv->bus_name);
+	if (IS_ERR(p)) {
+		ret = PTR_ERR(p);
+		goto err_with_put;
+	}
+
+	priv->chan[new_chan].parent_adap = adap;
+	priv->cur_chan = new_chan;
+
+	/* Now fill out current adapter structure. cur_chan must be up to date */
+	priv->algo.master_xfer = i2c_demux_master_xfer;
+	priv->algo.functionality = i2c_demux_functionality;
+
+	snprintf(priv->cur_adap.name, sizeof(priv->cur_adap.name),
+		 "i2c-demux (master i2c-%d)", i2c_adapter_id(adap));
+	priv->cur_adap.owner = THIS_MODULE;
+	priv->cur_adap.algo = &priv->algo;
+	priv->cur_adap.algo_data = priv;
+	priv->cur_adap.dev.parent = priv->dev;
+	priv->cur_adap.class = adap->class;
+	priv->cur_adap.retries = adap->retries;
+	priv->cur_adap.timeout = adap->timeout;
+	priv->cur_adap.quirks = adap->quirks;
+	priv->cur_adap.dev.of_node = priv->dev->of_node;
+	ret = i2c_add_adapter(&priv->cur_adap);
+	if (ret < 0)
+		goto err_with_put;
+
+	return 0;
+
+ err_with_put:
+	i2c_put_adapter(adap);
+ err:
+	dev_err(priv->dev, "failed to setup demux-adapter %d (%d)\n", new_chan, ret);
+	return ret;
+}
+
+static int i2c_demux_deactivate_master(struct i2c_demux_pinctrl_priv *priv)
+{
+	int ret, cur = priv->cur_chan;
+
+	if (cur < 0)
+		return 0;
+
+	i2c_del_adapter(&priv->cur_adap);
+	i2c_put_adapter(priv->chan[cur].parent_adap);
+
+	mutex_lock(&of_mutex);
+	ret = of_changeset_revert(&priv->chan[cur].chgset);
+	mutex_unlock(&of_mutex);
+
+	priv->chan[cur].parent_adap = NULL;
+	priv->cur_chan = -EINVAL;
+
+	return ret;
+}
+
+static int i2c_demux_change_master(struct i2c_demux_pinctrl_priv *priv, u32 new_chan)
+{
+	int ret;
+
+	if (new_chan = priv->cur_chan)
+		return 0;
+
+	ret = i2c_demux_deactivate_master(priv);
+	if (ret)
+		return ret;
+
+	return i2c_demux_activate_master(priv, new_chan);
+}
+
+static ssize_t cur_master_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct i2c_demux_pinctrl_priv *priv = dev_get_drvdata(dev);
+	int count = 0, i;
+
+	for (i = 0; i < priv->num_chan && count < PAGE_SIZE; i++)
+		count += scnprintf(buf + count, PAGE_SIZE - count, "%c %d - %s\n",
+				 i = priv->cur_chan ? '*' : ' ', i,
+				 priv->chan[i].parent_np->full_name);
+
+	return count;
+}
+
+static ssize_t cur_master_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	struct i2c_demux_pinctrl_priv *priv = dev_get_drvdata(dev);
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret < 0)
+		return ret;
+
+	if (val >= priv->num_chan)
+		return -EINVAL;
+
+	ret = i2c_demux_change_master(priv, val);
+
+	return ret < 0 ? ret : count;
+}
+static DEVICE_ATTR_RW(cur_master);
+
+static int i2c_demux_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct i2c_demux_pinctrl_priv *priv;
+	int num_chan, i, j, err;
+
+	num_chan = of_count_phandle_with_args(np, "i2c-parent", NULL);
+	if (num_chan < 2) {
+		dev_err(&pdev->dev, "Need at least two I2C masters to switch\n");
+		return -EINVAL;
+	}
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv)
+			   + num_chan * sizeof(struct i2c_demux_pinctrl_chan), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	err = of_property_read_string(np, "i2c-bus-name", &priv->bus_name);
+	if (err)
+		return err;
+
+	for (i = 0; i < num_chan; i++) {
+		struct device_node *adap_np;
+
+		adap_np = of_parse_phandle(np, "i2c-parent", i);
+		if (!adap_np) {
+			dev_err(&pdev->dev, "can't get phandle for parent %d\n", i);
+			err = -ENOENT;
+			goto err_rollback;
+		}
+		priv->chan[i].parent_np = adap_np;
+
+		of_changeset_init(&priv->chan[i].chgset);
+		of_changeset_update_property(&priv->chan[i].chgset, adap_np, &status_okay);
+	}
+
+	priv->num_chan = num_chan;
+	priv->dev = &pdev->dev;
+
+	platform_set_drvdata(pdev, priv);
+
+	/* switch to first parent as active master */
+	i2c_demux_activate_master(priv, 0);
+
+	err = device_create_file(&pdev->dev, &dev_attr_cur_master);
+	if (err)
+		goto err_rollback;
+
+	return 0;
+
+err_rollback:
+	for (j = 0; j < i; j++) {
+		of_node_put(priv->chan[j].parent_np);
+		of_changeset_destroy(&priv->chan[j].chgset);
+	}
+
+	return err;
+}
+
+static int i2c_demux_pinctrl_remove(struct platform_device *pdev)
+{
+	struct i2c_demux_pinctrl_priv *priv = platform_get_drvdata(pdev);
+	int i;
+
+	device_remove_file(&pdev->dev, &dev_attr_cur_master);
+
+	i2c_demux_deactivate_master(priv);
+
+	for (i = 0; i < priv->num_chan; i++) {
+		of_node_put(priv->chan[i].parent_np);
+		of_changeset_destroy(&priv->chan[i].chgset);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id i2c_demux_pinctrl_of_match[] = {
+	{ .compatible = "i2c-demux-pinctrl", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, i2c_demux_pinctrl_of_match);
+
+static struct platform_driver i2c_demux_pinctrl_driver = {
+	.driver	= {
+		.name = "i2c-demux-pinctrl",
+		.of_match_table = i2c_demux_pinctrl_of_match,
+	},
+	.probe	= i2c_demux_pinctrl_probe,
+	.remove	= i2c_demux_pinctrl_remove,
+};
+module_platform_driver(i2c_demux_pinctrl_driver);
+
+MODULE_DESCRIPTION("pinctrl-based I2C demux driver");
+MODULE_AUTHOR("Wolfram Sang <wsa@sang-engineering.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:i2c-demux-pinctrl");
-- 
2.1.4


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

* [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
@ 2016-01-06 13:51   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-06 13:51 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

This driver allows an I2C bus to switch between multiple masters. This
is not hot-swichting because connected I2C slaves will be
re-instantiated. It is meant to select the best I2C core at runtime once
the task is known. Example: Prefer i2c-gpio over another I2C core
because of HW errata affetcing your use case.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/muxes/Kconfig             |   9 ++
 drivers/i2c/muxes/Makefile            |   2 +
 drivers/i2c/muxes/i2c-demux-pinctrl.c | 276 ++++++++++++++++++++++++++++++++++
 3 files changed, 287 insertions(+)
 create mode 100644 drivers/i2c/muxes/i2c-demux-pinctrl.c

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index f06b0e24673b87..7b5da5ff7b16f9 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -72,4 +72,13 @@ config I2C_MUX_REG
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mux-reg.
 
+config I2C_DEMUX_PINCTRL
+	tristate "pinctrl-based I2C demultiplexer"
+	depends on PINCTRL
+	select OF_DYNAMIC
+	help
+	  If you say yes to this option, support will be included for an I2C
+	  demultiplexer that uses the pinctrl subsystem. This is useful if you
+	  want to change the I2C master at run-time depending on features.
+
 endmenu
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index e89799b76a9280..7c267c29b19196 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -3,6 +3,8 @@
 
 obj-$(CONFIG_I2C_ARB_GPIO_CHALLENGE)	+= i2c-arb-gpio-challenge.o
 
+obj-$(CONFIG_I2C_DEMUX_PINCTRL)		+= i2c-demux-pinctrl.o
+
 obj-$(CONFIG_I2C_MUX_GPIO)	+= i2c-mux-gpio.o
 obj-$(CONFIG_I2C_MUX_PCA9541)	+= i2c-mux-pca9541.o
 obj-$(CONFIG_I2C_MUX_PCA954x)	+= i2c-mux-pca954x.o
diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c
new file mode 100644
index 00000000000000..5ec2779b176432
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
@@ -0,0 +1,276 @@
+/*
+ * Pinctrl based I2C DeMultiplexer
+ *
+ * Copyright (C) 2015-16 by Wolfram Sang, Sang Engineering <wsa@sang-engineering.com>
+ * Copyright (C) 2015-16 by Renesas Electronics Corporation
+ *
+ * This program 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; version 2 of the License.
+ *
+ * See the bindings doc for DTS setup.
+ */
+
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+
+struct i2c_demux_pinctrl_chan {
+	struct device_node *parent_np;
+	struct i2c_adapter *parent_adap;
+	struct of_changeset chgset;
+};
+
+struct i2c_demux_pinctrl_priv {
+	int cur_chan;
+	int num_chan;
+	struct device *dev;
+	const char *bus_name;
+	struct i2c_adapter cur_adap;
+	struct i2c_algorithm algo;
+	struct i2c_demux_pinctrl_chan chan[];
+};
+
+static struct property status_okay = { .name = "status", .length = 3, .value = "ok" };
+
+static int i2c_demux_master_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
+{
+	struct i2c_demux_pinctrl_priv *priv = adap->algo_data;
+	struct i2c_adapter *parent = priv->chan[priv->cur_chan].parent_adap;
+
+	return __i2c_transfer(parent, msgs, num);
+}
+
+static u32 i2c_demux_functionality(struct i2c_adapter *adap)
+{
+	struct i2c_demux_pinctrl_priv *priv = adap->algo_data;
+	struct i2c_adapter *parent = priv->chan[priv->cur_chan].parent_adap;
+
+	return parent->algo->functionality(parent);
+}
+
+static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 new_chan)
+{
+	struct i2c_adapter *adap;
+	struct pinctrl *p;
+	int ret;
+
+	mutex_lock(&of_mutex);
+	ret = of_changeset_apply(&priv->chan[new_chan].chgset);
+	mutex_unlock(&of_mutex);
+	if (ret)
+		goto err;
+
+	adap = of_find_i2c_adapter_by_node(priv->chan[new_chan].parent_np);
+	if (!adap) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	p = devm_pinctrl_get_select(adap->dev.parent, priv->bus_name);
+	if (IS_ERR(p)) {
+		ret = PTR_ERR(p);
+		goto err_with_put;
+	}
+
+	priv->chan[new_chan].parent_adap = adap;
+	priv->cur_chan = new_chan;
+
+	/* Now fill out current adapter structure. cur_chan must be up to date */
+	priv->algo.master_xfer = i2c_demux_master_xfer;
+	priv->algo.functionality = i2c_demux_functionality;
+
+	snprintf(priv->cur_adap.name, sizeof(priv->cur_adap.name),
+		 "i2c-demux (master i2c-%d)", i2c_adapter_id(adap));
+	priv->cur_adap.owner = THIS_MODULE;
+	priv->cur_adap.algo = &priv->algo;
+	priv->cur_adap.algo_data = priv;
+	priv->cur_adap.dev.parent = priv->dev;
+	priv->cur_adap.class = adap->class;
+	priv->cur_adap.retries = adap->retries;
+	priv->cur_adap.timeout = adap->timeout;
+	priv->cur_adap.quirks = adap->quirks;
+	priv->cur_adap.dev.of_node = priv->dev->of_node;
+	ret = i2c_add_adapter(&priv->cur_adap);
+	if (ret < 0)
+		goto err_with_put;
+
+	return 0;
+
+ err_with_put:
+	i2c_put_adapter(adap);
+ err:
+	dev_err(priv->dev, "failed to setup demux-adapter %d (%d)\n", new_chan, ret);
+	return ret;
+}
+
+static int i2c_demux_deactivate_master(struct i2c_demux_pinctrl_priv *priv)
+{
+	int ret, cur = priv->cur_chan;
+
+	if (cur < 0)
+		return 0;
+
+	i2c_del_adapter(&priv->cur_adap);
+	i2c_put_adapter(priv->chan[cur].parent_adap);
+
+	mutex_lock(&of_mutex);
+	ret = of_changeset_revert(&priv->chan[cur].chgset);
+	mutex_unlock(&of_mutex);
+
+	priv->chan[cur].parent_adap = NULL;
+	priv->cur_chan = -EINVAL;
+
+	return ret;
+}
+
+static int i2c_demux_change_master(struct i2c_demux_pinctrl_priv *priv, u32 new_chan)
+{
+	int ret;
+
+	if (new_chan == priv->cur_chan)
+		return 0;
+
+	ret = i2c_demux_deactivate_master(priv);
+	if (ret)
+		return ret;
+
+	return i2c_demux_activate_master(priv, new_chan);
+}
+
+static ssize_t cur_master_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
+{
+	struct i2c_demux_pinctrl_priv *priv = dev_get_drvdata(dev);
+	int count = 0, i;
+
+	for (i = 0; i < priv->num_chan && count < PAGE_SIZE; i++)
+		count += scnprintf(buf + count, PAGE_SIZE - count, "%c %d - %s\n",
+				 i == priv->cur_chan ? '*' : ' ', i,
+				 priv->chan[i].parent_np->full_name);
+
+	return count;
+}
+
+static ssize_t cur_master_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	struct i2c_demux_pinctrl_priv *priv = dev_get_drvdata(dev);
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret < 0)
+		return ret;
+
+	if (val >= priv->num_chan)
+		return -EINVAL;
+
+	ret = i2c_demux_change_master(priv, val);
+
+	return ret < 0 ? ret : count;
+}
+static DEVICE_ATTR_RW(cur_master);
+
+static int i2c_demux_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct i2c_demux_pinctrl_priv *priv;
+	int num_chan, i, j, err;
+
+	num_chan = of_count_phandle_with_args(np, "i2c-parent", NULL);
+	if (num_chan < 2) {
+		dev_err(&pdev->dev, "Need at least two I2C masters to switch\n");
+		return -EINVAL;
+	}
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv)
+			   + num_chan * sizeof(struct i2c_demux_pinctrl_chan), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	err = of_property_read_string(np, "i2c-bus-name", &priv->bus_name);
+	if (err)
+		return err;
+
+	for (i = 0; i < num_chan; i++) {
+		struct device_node *adap_np;
+
+		adap_np = of_parse_phandle(np, "i2c-parent", i);
+		if (!adap_np) {
+			dev_err(&pdev->dev, "can't get phandle for parent %d\n", i);
+			err = -ENOENT;
+			goto err_rollback;
+		}
+		priv->chan[i].parent_np = adap_np;
+
+		of_changeset_init(&priv->chan[i].chgset);
+		of_changeset_update_property(&priv->chan[i].chgset, adap_np, &status_okay);
+	}
+
+	priv->num_chan = num_chan;
+	priv->dev = &pdev->dev;
+
+	platform_set_drvdata(pdev, priv);
+
+	/* switch to first parent as active master */
+	i2c_demux_activate_master(priv, 0);
+
+	err = device_create_file(&pdev->dev, &dev_attr_cur_master);
+	if (err)
+		goto err_rollback;
+
+	return 0;
+
+err_rollback:
+	for (j = 0; j < i; j++) {
+		of_node_put(priv->chan[j].parent_np);
+		of_changeset_destroy(&priv->chan[j].chgset);
+	}
+
+	return err;
+}
+
+static int i2c_demux_pinctrl_remove(struct platform_device *pdev)
+{
+	struct i2c_demux_pinctrl_priv *priv = platform_get_drvdata(pdev);
+	int i;
+
+	device_remove_file(&pdev->dev, &dev_attr_cur_master);
+
+	i2c_demux_deactivate_master(priv);
+
+	for (i = 0; i < priv->num_chan; i++) {
+		of_node_put(priv->chan[i].parent_np);
+		of_changeset_destroy(&priv->chan[i].chgset);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id i2c_demux_pinctrl_of_match[] = {
+	{ .compatible = "i2c-demux-pinctrl", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, i2c_demux_pinctrl_of_match);
+
+static struct platform_driver i2c_demux_pinctrl_driver = {
+	.driver	= {
+		.name = "i2c-demux-pinctrl",
+		.of_match_table = i2c_demux_pinctrl_of_match,
+	},
+	.probe	= i2c_demux_pinctrl_probe,
+	.remove	= i2c_demux_pinctrl_remove,
+};
+module_platform_driver(i2c_demux_pinctrl_driver);
+
+MODULE_DESCRIPTION("pinctrl-based I2C demux driver");
+MODULE_AUTHOR("Wolfram Sang <wsa@sang-engineering.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:i2c-demux-pinctrl");
-- 
2.1.4


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

* [RFC v2 4/4] ARM: shmobile: r8a7790: rework dts to use i2c demuxer
  2016-01-06 13:51 ` Wolfram Sang
@ 2016-01-06 13:51   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-06 13:51 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Create a seperate bus for HDMI related I2C slaves and assign it
to a i2c-gpio master. It can be switched to the i2c-rcar or
i2c-sh_mobile core at runtime.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/boot/dts/r8a7790-lager.dts | 141 ++++++++++++++++++++++--------------
 1 file changed, 88 insertions(+), 53 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index c553abd711eeb3..d8f0ca8e094dad 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -49,6 +49,8 @@
 	aliases {
 		serial0 = &scifa0;
 		serial1 = &scifa1;
+		i2c8 = &i2chdmi;
+		i2c9 = &gpioi2c;
 	};
 
 	chosen {
@@ -252,6 +254,79 @@
 		#clock-cells = <0>;
 		clock-frequency = <148500000>;
 	};
+
+
+	gpioi2c: i2c@9 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "i2c-gpio";
+		status = "disabled";
+		gpios = <&gpio5 6 GPIO_ACTIVE_HIGH /* sda */
+			 &gpio5 5 GPIO_ACTIVE_HIGH /* scl */
+			>;
+		i2c-gpio,delay-us = <5>;
+	};
+
+	i2chdmi: i2c@8 {
+
+		compatible = "i2c-demux-pinctrl";
+		i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
+		i2c-bus-name = "i2c-hdmi";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ak4643: sound-codec@12 {
+			compatible = "asahi-kasei,ak4643";
+
+			#sound-dai-cells = <0>;
+			reg = <0x12>;
+		};
+
+		composite-in@20 {
+			compatible = "adi,adv7180";
+			reg = <0x20>;
+			remote = <&vin1>;
+
+			port {
+				adv7180: endpoint {
+					bus-width = <8>;
+					remote-endpoint = <&vin1ep0>;
+				};
+			};
+		};
+
+		hdmi@39 {
+			compatible = "adi,adv7511w";
+			reg = <0x39>;
+			interrupt-parent = <&gpio1>;
+			interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
+
+			adi,input-depth = <8>;
+			adi,input-colorspace = "rgb";
+			adi,input-clock = "1x";
+			adi,input-style = <1>;
+			adi,input-justification = "evenly";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					adv7511_in: endpoint {
+						remote-endpoint = <&du_out_lvds0>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					adv7511_out: endpoint {
+						remote-endpoint = <&hdmi_con>;
+					};
+				};
+			};
+		};
+	};
 };
 
 &du {
@@ -352,6 +427,11 @@
 		renesas,function = "iic1";
 	};
 
+	i2c2_pins: i2c2 {
+		renesas,groups = "i2c2";
+		renesas,function = "i2c2";
+	};
+
 	iic2_pins: iic2 {
 		renesas,groups = "iic2";
 		renesas,function = "iic2";
@@ -532,63 +612,18 @@
 	pinctrl-names = "default";
 };
 
-&iic2	{
-	status = "okay";
-	pinctrl-0 = <&iic2_pins>;
-	pinctrl-names = "default";
+&i2c2	{
+	pinctrl-0 = <&i2c2_pins>;
+	pinctrl-names = "i2c-hdmi";
 
 	clock-frequency = <100000>;
+};
 
-	ak4643: codec@12 {
-		compatible = "asahi-kasei,ak4643";
-		#sound-dai-cells = <0>;
-		reg = <0x12>;
-	};
-
-	composite-in@20 {
-		compatible = "adi,adv7180";
-		reg = <0x20>;
-		remote = <&vin1>;
-
-		port {
-			adv7180: endpoint {
-				bus-width = <8>;
-				remote-endpoint = <&vin1ep0>;
-			};
-		};
-	};
-
-	hdmi@39 {
-		compatible = "adi,adv7511w";
-		reg = <0x39>;
-		interrupt-parent = <&gpio1>;
-		interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
-
-		adi,input-depth = <8>;
-		adi,input-colorspace = "rgb";
-		adi,input-clock = "1x";
-		adi,input-style = <1>;
-		adi,input-justification = "evenly";
-
-		ports {
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			port@0 {
-				reg = <0>;
-				adv7511_in: endpoint {
-					remote-endpoint = <&du_out_lvds0>;
-				};
-			};
+&iic2	{
+	pinctrl-0 = <&iic2_pins>;
+	pinctrl-names = "i2c-hdmi";
 
-			port@1 {
-				reg = <1>;
-				adv7511_out: endpoint {
-					remote-endpoint = <&hdmi_con>;
-				};
-			};
-		};
-	};
+	clock-frequency = <100000>;
 };
 
 &iic3 {
-- 
2.1.4


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

* [RFC v2 4/4] ARM: shmobile: r8a7790: rework dts to use i2c demuxer
@ 2016-01-06 13:51   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-06 13:51 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Create a seperate bus for HDMI related I2C slaves and assign it
to a i2c-gpio master. It can be switched to the i2c-rcar or
i2c-sh_mobile core at runtime.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm/boot/dts/r8a7790-lager.dts | 141 ++++++++++++++++++++++--------------
 1 file changed, 88 insertions(+), 53 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index c553abd711eeb3..d8f0ca8e094dad 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -49,6 +49,8 @@
 	aliases {
 		serial0 = &scifa0;
 		serial1 = &scifa1;
+		i2c8 = &i2chdmi;
+		i2c9 = &gpioi2c;
 	};
 
 	chosen {
@@ -252,6 +254,79 @@
 		#clock-cells = <0>;
 		clock-frequency = <148500000>;
 	};
+
+
+	gpioi2c: i2c@9 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "i2c-gpio";
+		status = "disabled";
+		gpios = <&gpio5 6 GPIO_ACTIVE_HIGH /* sda */
+			 &gpio5 5 GPIO_ACTIVE_HIGH /* scl */
+			>;
+		i2c-gpio,delay-us = <5>;
+	};
+
+	i2chdmi: i2c@8 {
+
+		compatible = "i2c-demux-pinctrl";
+		i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
+		i2c-bus-name = "i2c-hdmi";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ak4643: sound-codec@12 {
+			compatible = "asahi-kasei,ak4643";
+
+			#sound-dai-cells = <0>;
+			reg = <0x12>;
+		};
+
+		composite-in@20 {
+			compatible = "adi,adv7180";
+			reg = <0x20>;
+			remote = <&vin1>;
+
+			port {
+				adv7180: endpoint {
+					bus-width = <8>;
+					remote-endpoint = <&vin1ep0>;
+				};
+			};
+		};
+
+		hdmi@39 {
+			compatible = "adi,adv7511w";
+			reg = <0x39>;
+			interrupt-parent = <&gpio1>;
+			interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
+
+			adi,input-depth = <8>;
+			adi,input-colorspace = "rgb";
+			adi,input-clock = "1x";
+			adi,input-style = <1>;
+			adi,input-justification = "evenly";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					adv7511_in: endpoint {
+						remote-endpoint = <&du_out_lvds0>;
+					};
+				};
+
+				port@1 {
+					reg = <1>;
+					adv7511_out: endpoint {
+						remote-endpoint = <&hdmi_con>;
+					};
+				};
+			};
+		};
+	};
 };
 
 &du {
@@ -352,6 +427,11 @@
 		renesas,function = "iic1";
 	};
 
+	i2c2_pins: i2c2 {
+		renesas,groups = "i2c2";
+		renesas,function = "i2c2";
+	};
+
 	iic2_pins: iic2 {
 		renesas,groups = "iic2";
 		renesas,function = "iic2";
@@ -532,63 +612,18 @@
 	pinctrl-names = "default";
 };
 
-&iic2	{
-	status = "okay";
-	pinctrl-0 = <&iic2_pins>;
-	pinctrl-names = "default";
+&i2c2	{
+	pinctrl-0 = <&i2c2_pins>;
+	pinctrl-names = "i2c-hdmi";
 
 	clock-frequency = <100000>;
+};
 
-	ak4643: codec@12 {
-		compatible = "asahi-kasei,ak4643";
-		#sound-dai-cells = <0>;
-		reg = <0x12>;
-	};
-
-	composite-in@20 {
-		compatible = "adi,adv7180";
-		reg = <0x20>;
-		remote = <&vin1>;
-
-		port {
-			adv7180: endpoint {
-				bus-width = <8>;
-				remote-endpoint = <&vin1ep0>;
-			};
-		};
-	};
-
-	hdmi@39 {
-		compatible = "adi,adv7511w";
-		reg = <0x39>;
-		interrupt-parent = <&gpio1>;
-		interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
-
-		adi,input-depth = <8>;
-		adi,input-colorspace = "rgb";
-		adi,input-clock = "1x";
-		adi,input-style = <1>;
-		adi,input-justification = "evenly";
-
-		ports {
-			#address-cells = <1>;
-			#size-cells = <0>;
-
-			port@0 {
-				reg = <0>;
-				adv7511_in: endpoint {
-					remote-endpoint = <&du_out_lvds0>;
-				};
-			};
+&iic2	{
+	pinctrl-0 = <&iic2_pins>;
+	pinctrl-names = "i2c-hdmi";
 
-			port@1 {
-				reg = <1>;
-				adv7511_out: endpoint {
-					remote-endpoint = <&hdmi_con>;
-				};
-			};
-		};
-	};
+	clock-frequency = <100000>;
 };
 
 &iic3 {
-- 
2.1.4


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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
  2016-01-06 13:51   ` Wolfram Sang
  (?)
@ 2016-01-06 14:20       ` Geert Uytterhoeven
  -1 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-06 14:20 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou,
	Jan Luebbe

Hi Wolfram,

On Wed, Jan 6, 2016 at 2:51 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This driver allows an I2C bus to switch between multiple masters. This
> is not hot-swichting because connected I2C slaves will be

switching

> re-instantiated. It is meant to select the best I2C core at runtime once
> the task is known. Example: Prefer i2c-gpio over another I2C core
> because of HW errata affetcing your use case.

affecting

> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> @@ -0,0 +1,276 @@
> +/*
> + * Pinctrl based I2C DeMultiplexer
> + *
> + * Copyright (C) 2015-16 by Wolfram Sang, Sang Engineering <wsa@sang-engineering.com>
> + * Copyright (C) 2015-16 by Renesas Electronics Corporation
> + *
> + * This program 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; version 2 of the License.
> + *
> + * See the bindings doc for DTS setup.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +
> +struct i2c_demux_pinctrl_chan {
> +       struct device_node *parent_np;
> +       struct i2c_adapter *parent_adap;
> +       struct of_changeset chgset;
> +};
> +
> +struct i2c_demux_pinctrl_priv {
> +       int cur_chan;
> +       int num_chan;

This is always positive, so you can make num_chan and a few loop counters
unsigned.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
@ 2016-01-06 14:20       ` Geert Uytterhoeven
  0 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-06 14:20 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

Hi Wolfram,

On Wed, Jan 6, 2016 at 2:51 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This driver allows an I2C bus to switch between multiple masters. This
> is not hot-swichting because connected I2C slaves will be

switching

> re-instantiated. It is meant to select the best I2C core at runtime once
> the task is known. Example: Prefer i2c-gpio over another I2C core
> because of HW errata affetcing your use case.

affecting

> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> @@ -0,0 +1,276 @@
> +/*
> + * Pinctrl based I2C DeMultiplexer
> + *
> + * Copyright (C) 2015-16 by Wolfram Sang, Sang Engineering <wsa@sang-engineering.com>
> + * Copyright (C) 2015-16 by Renesas Electronics Corporation
> + *
> + * This program 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; version 2 of the License.
> + *
> + * See the bindings doc for DTS setup.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +
> +struct i2c_demux_pinctrl_chan {
> +       struct device_node *parent_np;
> +       struct i2c_adapter *parent_adap;
> +       struct of_changeset chgset;
> +};
> +
> +struct i2c_demux_pinctrl_priv {
> +       int cur_chan;
> +       int num_chan;

This is always positive, so you can make num_chan and a few loop counters
unsigned.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
@ 2016-01-06 14:20       ` Geert Uytterhoeven
  0 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-06 14:20 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou,
	Jan Luebbe

Hi Wolfram,

On Wed, Jan 6, 2016 at 2:51 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> From: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>
>
> This driver allows an I2C bus to switch between multiple masters. This
> is not hot-swichting because connected I2C slaves will be

switching

> re-instantiated. It is meant to select the best I2C core at runtime once
> the task is known. Example: Prefer i2c-gpio over another I2C core
> because of HW errata affetcing your use case.

affecting

> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> @@ -0,0 +1,276 @@
> +/*
> + * Pinctrl based I2C DeMultiplexer
> + *
> + * Copyright (C) 2015-16 by Wolfram Sang, Sang Engineering <wsa-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>
> + * Copyright (C) 2015-16 by Renesas Electronics Corporation
> + *
> + * This program 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; version 2 of the License.
> + *
> + * See the bindings doc for DTS setup.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +
> +struct i2c_demux_pinctrl_chan {
> +       struct device_node *parent_np;
> +       struct i2c_adapter *parent_adap;
> +       struct of_changeset chgset;
> +};
> +
> +struct i2c_demux_pinctrl_priv {
> +       int cur_chan;
> +       int num_chan;

This is always positive, so you can make num_chan and a few loop counters
unsigned.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [RFC v2 1/4] of: make of_mutex public
  2016-01-06 13:51   ` Wolfram Sang
@ 2016-01-06 14:26     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-06 14:26 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

Hi Wolfram,

On Wed, Jan 6, 2016 at 2:51 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> If we want to use OF_DYNAMIC features outside the of framework, we need
> to access this lock.

As I2C_DEMUX_PINCTRL is tristate, you want to add an EXPORT_SYMBOL_GPL(), too.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC v2 1/4] of: make of_mutex public
@ 2016-01-06 14:26     ` Geert Uytterhoeven
  0 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-06 14:26 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

Hi Wolfram,

On Wed, Jan 6, 2016 at 2:51 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> If we want to use OF_DYNAMIC features outside the of framework, we need
> to access this lock.

As I2C_DEMUX_PINCTRL is tristate, you want to add an EXPORT_SYMBOL_GPL(), too.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
  2016-01-06 13:51   ` Wolfram Sang
@ 2016-01-06 14:30     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-06 14:30 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

Hi Wolfram,

On Wed, Jan 6, 2016 at 2:51 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> These bindings allow an I2C bus to switch between multiple masters. This
> is not hot-swichting because connected I2C slaves will be
> re-instantiated. It is meant to select the best I2C core at runtime once
> the task is known. Example: Prefer i2c-gpio over another I2C core
> because of HW errata affetcing your use case.

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
> @@ -0,0 +1,155 @@

> +Here is a snipplet for a bus to be demuxed. It contains various i2c clients for

snippet

> +HDMI, so the bus is named "i2c-hdmi":
> +
> +       i2chdmi: i2c@8 {

Would it make sense to call the node "i2c-bus@8"?
> +
> +               compatible = "i2c-demux-pinctrl";
> +               i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
@ 2016-01-06 14:30     ` Geert Uytterhoeven
  0 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-06 14:30 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

Hi Wolfram,

On Wed, Jan 6, 2016 at 2:51 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> These bindings allow an I2C bus to switch between multiple masters. This
> is not hot-swichting because connected I2C slaves will be
> re-instantiated. It is meant to select the best I2C core at runtime once
> the task is known. Example: Prefer i2c-gpio over another I2C core
> because of HW errata affetcing your use case.

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
> @@ -0,0 +1,155 @@

> +Here is a snipplet for a bus to be demuxed. It contains various i2c clients for

snippet

> +HDMI, so the bus is named "i2c-hdmi":
> +
> +       i2chdmi: i2c@8 {

Would it make sense to call the node "i2c-bus@8"?
> +
> +               compatible = "i2c-demux-pinctrl";
> +               i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
  2016-01-06 13:51   ` Wolfram Sang
@ 2016-01-06 17:08     ` Rob Herring
  -1 siblings, 0 replies; 38+ messages in thread
From: Rob Herring @ 2016-01-06 17:08 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, SH-Linux, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

On Wed, Jan 6, 2016 at 7:51 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This driver allows an I2C bus to switch between multiple masters. This
> is not hot-swichting because connected I2C slaves will be
> re-instantiated. It is meant to select the best I2C core at runtime once
> the task is known. Example: Prefer i2c-gpio over another I2C core
> because of HW errata affetcing your use case.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

[...]

> +static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 new_chan)
> +{
> +       struct i2c_adapter *adap;
> +       struct pinctrl *p;
> +       int ret;
> +
> +       mutex_lock(&of_mutex);
> +       ret = of_changeset_apply(&priv->chan[new_chan].chgset);
> +       mutex_unlock(&of_mutex);

Looks like you need this patch[1] rather than exposing of_mutex.

Rob

[1] https://patchwork.ozlabs.org/patch/539938/

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
@ 2016-01-06 17:08     ` Rob Herring
  0 siblings, 0 replies; 38+ messages in thread
From: Rob Herring @ 2016-01-06 17:08 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, SH-Linux, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

On Wed, Jan 6, 2016 at 7:51 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> This driver allows an I2C bus to switch between multiple masters. This
> is not hot-swichting because connected I2C slaves will be
> re-instantiated. It is meant to select the best I2C core at runtime once
> the task is known. Example: Prefer i2c-gpio over another I2C core
> because of HW errata affetcing your use case.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

[...]

> +static int i2c_demux_activate_master(struct i2c_demux_pinctrl_priv *priv, u32 new_chan)
> +{
> +       struct i2c_adapter *adap;
> +       struct pinctrl *p;
> +       int ret;
> +
> +       mutex_lock(&of_mutex);
> +       ret = of_changeset_apply(&priv->chan[new_chan].chgset);
> +       mutex_unlock(&of_mutex);

Looks like you need this patch[1] rather than exposing of_mutex.

Rob

[1] https://patchwork.ozlabs.org/patch/539938/

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
  2016-01-06 13:51   ` Wolfram Sang
@ 2016-01-07 16:56     ` Jan Lübbe
  -1 siblings, 0 replies; 38+ messages in thread
From: Jan Lübbe @ 2016-01-07 16:56 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou

Hi Wolfram,

On Mi, 2016-01-06 at 14:51 +0100, Wolfram Sang wrote:
[...]
> +Required properties:
> +- compatible: "i2c-demux-pinctrl"
> +- i2c-parent: List of phandles of I2C masters available for selection. The first
> +	      one will be used as default.
> +- i2c-bus-name: The name of this bus. Also needed as pinctrl-name for the I2C
> +		parents.
[...]
> +	i2chdmi: i2c@8 {
> +		compatible = "i2c-demux-pinctrl";
> +		i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
> +		i2c-bus-name = "i2c-hdmi";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
[...]
> +	gpioi2c: i2c@9 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "i2c-gpio";
[...]
> +&i2c2	{
> +	pinctrl-0 = <&i2c2_pins>;
> +	pinctrl-names = "i2c-hdmi";
> +
> +	clock-frequency = <100000>;
> +};
[...]
> +&iic2	{
> +	pinctrl-0 = <&iic2_pins>;
> +	pinctrl-names = "i2c-hdmi";
> +
> +	clock-frequency = <100000>;
> +};
[...]

It seems that the demux-pinctrl driver reconfigures the pinctrl settings
for the parent devices. I would have expected to have alternative
pinctrl state on the demux node support switching the same external pins
between the different controllers. Wouldn't it be possible to have
pinctrl conflicts between &i2c2_pins and &iic2_pins otherwise?

Regards,
Jan
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
@ 2016-01-07 16:56     ` Jan Lübbe
  0 siblings, 0 replies; 38+ messages in thread
From: Jan Lübbe @ 2016-01-07 16:56 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou

Hi Wolfram,

On Mi, 2016-01-06 at 14:51 +0100, Wolfram Sang wrote:
[...]
> +Required properties:
> +- compatible: "i2c-demux-pinctrl"
> +- i2c-parent: List of phandles of I2C masters available for selection. The first
> +	      one will be used as default.
> +- i2c-bus-name: The name of this bus. Also needed as pinctrl-name for the I2C
> +		parents.
[...]
> +	i2chdmi: i2c@8 {
> +		compatible = "i2c-demux-pinctrl";
> +		i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
> +		i2c-bus-name = "i2c-hdmi";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
[...]
> +	gpioi2c: i2c@9 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "i2c-gpio";
[...]
> +&i2c2	{
> +	pinctrl-0 = <&i2c2_pins>;
> +	pinctrl-names = "i2c-hdmi";
> +
> +	clock-frequency = <100000>;
> +};
[...]
> +&iic2	{
> +	pinctrl-0 = <&iic2_pins>;
> +	pinctrl-names = "i2c-hdmi";
> +
> +	clock-frequency = <100000>;
> +};
[...]

It seems that the demux-pinctrl driver reconfigures the pinctrl settings
for the parent devices. I would have expected to have alternative
pinctrl state on the demux node support switching the same external pins
between the different controllers. Wouldn't it be possible to have
pinctrl conflicts between &i2c2_pins and &iic2_pins otherwise?

Regards,
Jan
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
  2016-01-06 13:51   ` Wolfram Sang
@ 2016-01-11  2:52     ` Rob Herring
  -1 siblings, 0 replies; 38+ messages in thread
From: Rob Herring @ 2016-01-11  2:52 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

On Wed, Jan 06, 2016 at 02:51:23PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> These bindings allow an I2C bus to switch between multiple masters. This
> is not hot-swichting because connected I2C slaves will be

s/swichting/switching/

> re-instantiated. It is meant to select the best I2C core at runtime once
> the task is known. Example: Prefer i2c-gpio over another I2C core
> because of HW errata affetcing your use case.

This seems okay to me. I don't like not having the i2c devices under the 
i2c controller, but not really much we can do about that.

I have another usecase which might end up needing a similar structure 
as this. That is the whole question of how to deal with capes, hats, 
shields and various daughterboards. For I2C devices on a daughterboard, 
we need to be able to have a DT overlay for the daughterboard described 
in terms of the connector(s) and independent of the host. Then the 
base DT needs to define the mapping from the connector to the host 
controller. A complicating factor would be having devices on the same 
bus but split across the main and daughter boards. I don't see anything 
specific to change here, but just want to throw that out.

[...]

> +
> +Changing I2C controllers:
> +
> +The created mux-device will have a file "cur_master" in its sysfs-entry. Write
> +0 there for the first master listed in the "i2c-parent" property, 1 for the
> +second etc. Reading the file will give you a list with the active master
> +marked. Example from a Renesas Lager board:

This is Linux specific and should not be in the binding. You also need 
sysfs docs for these sysfs files.

Rob

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
@ 2016-01-11  2:52     ` Rob Herring
  0 siblings, 0 replies; 38+ messages in thread
From: Rob Herring @ 2016-01-11  2:52 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

On Wed, Jan 06, 2016 at 02:51:23PM +0100, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> These bindings allow an I2C bus to switch between multiple masters. This
> is not hot-swichting because connected I2C slaves will be

s/swichting/switching/

> re-instantiated. It is meant to select the best I2C core at runtime once
> the task is known. Example: Prefer i2c-gpio over another I2C core
> because of HW errata affetcing your use case.

This seems okay to me. I don't like not having the i2c devices under the 
i2c controller, but not really much we can do about that.

I have another usecase which might end up needing a similar structure 
as this. That is the whole question of how to deal with capes, hats, 
shields and various daughterboards. For I2C devices on a daughterboard, 
we need to be able to have a DT overlay for the daughterboard described 
in terms of the connector(s) and independent of the host. Then the 
base DT needs to define the mapping from the connector to the host 
controller. A complicating factor would be having devices on the same 
bus but split across the main and daughter boards. I don't see anything 
specific to change here, but just want to throw that out.

[...]

> +
> +Changing I2C controllers:
> +
> +The created mux-device will have a file "cur_master" in its sysfs-entry. Write
> +0 there for the first master listed in the "i2c-parent" property, 1 for the
> +second etc. Reading the file will give you a list with the active master
> +marked. Example from a Renesas Lager board:

This is Linux specific and should not be in the binding. You also need 
sysfs docs for these sysfs files.

Rob

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
  2016-01-11  2:52     ` Rob Herring
@ 2016-01-11  8:01       ` Geert Uytterhoeven
  -1 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-11  8:01 UTC (permalink / raw)
  To: Rob Herring
  Cc: Wolfram Sang, Linux I2C, Linux-sh list, Magnus Damm, devicetree,
	linux-kernel, Pantelis Antoniou, Jan Luebbe

Hi Rob,

On Mon, Jan 11, 2016 at 3:52 AM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Jan 06, 2016 at 02:51:23PM +0100, Wolfram Sang wrote:
>> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>
>> These bindings allow an I2C bus to switch between multiple masters. This
>> is not hot-swichting because connected I2C slaves will be
>
> s/swichting/switching/
>
>> re-instantiated. It is meant to select the best I2C core at runtime once
>> the task is known. Example: Prefer i2c-gpio over another I2C core
>> because of HW errata affetcing your use case.
>
> This seems okay to me. I don't like not having the i2c devices under the
> i2c controller, but not really much we can do about that.
>
> I have another usecase which might end up needing a similar structure
> as this. That is the whole question of how to deal with capes, hats,
> shields and various daughterboards. For I2C devices on a daughterboard,
> we need to be able to have a DT overlay for the daughterboard described
> in terms of the connector(s) and independent of the host. Then the
> base DT needs to define the mapping from the connector to the host

Indeed. Currently an i2c bus is described in DT as tied to an i2c controller,
while physically it's tied to a bunch of pins. Tying the bus to a controller
is done through pinctrl.

> controller. A complicating factor would be having devices on the same
> bus but split across the main and daughter boards. I don't see anything
> specific to change here, but just want to throw that out.

Can't the daughter board DTS just add more child devices to the mainboard bus?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
@ 2016-01-11  8:01       ` Geert Uytterhoeven
  0 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-11  8:01 UTC (permalink / raw)
  To: Rob Herring
  Cc: Wolfram Sang, Linux I2C, Linux-sh list, Magnus Damm, devicetree,
	linux-kernel, Pantelis Antoniou, Jan Luebbe

Hi Rob,

On Mon, Jan 11, 2016 at 3:52 AM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Jan 06, 2016 at 02:51:23PM +0100, Wolfram Sang wrote:
>> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>
>> These bindings allow an I2C bus to switch between multiple masters. This
>> is not hot-swichting because connected I2C slaves will be
>
> s/swichting/switching/
>
>> re-instantiated. It is meant to select the best I2C core at runtime once
>> the task is known. Example: Prefer i2c-gpio over another I2C core
>> because of HW errata affetcing your use case.
>
> This seems okay to me. I don't like not having the i2c devices under the
> i2c controller, but not really much we can do about that.
>
> I have another usecase which might end up needing a similar structure
> as this. That is the whole question of how to deal with capes, hats,
> shields and various daughterboards. For I2C devices on a daughterboard,
> we need to be able to have a DT overlay for the daughterboard described
> in terms of the connector(s) and independent of the host. Then the
> base DT needs to define the mapping from the connector to the host

Indeed. Currently an i2c bus is described in DT as tied to an i2c controller,
while physically it's tied to a bunch of pins. Tying the bus to a controller
is done through pinctrl.

> controller. A complicating factor would be having devices on the same
> bus but split across the main and daughter boards. I don't see anything
> specific to change here, but just want to throw that out.

Can't the daughter board DTS just add more child devices to the mainboard bus?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
  2016-01-06 14:30     ` Geert Uytterhoeven
@ 2016-01-13 12:52       ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-13 12:52 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe, Sergei Shtylyov

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

On Wed, Jan 06, 2016 at 03:30:39PM +0100, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Wed, Jan 6, 2016 at 2:51 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> > These bindings allow an I2C bus to switch between multiple masters. This
> > is not hot-swichting because connected I2C slaves will be
> > re-instantiated. It is meant to select the best I2C core at runtime once
> > the task is known. Example: Prefer i2c-gpio over another I2C core
> > because of HW errata affetcing your use case.
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
> > @@ -0,0 +1,155 @@
> 
> > +Here is a snipplet for a bus to be demuxed. It contains various i2c clients for
> 
> snippet
> 
> > +HDMI, so the bus is named "i2c-hdmi":
> > +
> > +       i2chdmi: i2c@8 {
> 
> Would it make sense to call the node "i2c-bus@8"?

That sounds like a question for our EPapr expert Sergei (CCed) :)

> > +
> > +               compatible = "i2c-demux-pinctrl";
> > +               i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

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

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
@ 2016-01-13 12:52       ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-13 12:52 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe, Sergei Shtylyov

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

On Wed, Jan 06, 2016 at 03:30:39PM +0100, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Wed, Jan 6, 2016 at 2:51 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> > These bindings allow an I2C bus to switch between multiple masters. This
> > is not hot-swichting because connected I2C slaves will be
> > re-instantiated. It is meant to select the best I2C core at runtime once
> > the task is known. Example: Prefer i2c-gpio over another I2C core
> > because of HW errata affetcing your use case.
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
> > @@ -0,0 +1,155 @@
> 
> > +Here is a snipplet for a bus to be demuxed. It contains various i2c clients for
> 
> snippet
> 
> > +HDMI, so the bus is named "i2c-hdmi":
> > +
> > +       i2chdmi: i2c@8 {
> 
> Would it make sense to call the node "i2c-bus@8"?

That sounds like a question for our EPapr expert Sergei (CCed) :)

> > +
> > +               compatible = "i2c-demux-pinctrl";
> > +               i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

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

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
  2016-01-06 14:20       ` Geert Uytterhoeven
@ 2016-01-13 12:55         ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-13 12:55 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

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


> > +struct i2c_demux_pinctrl_priv {
> > +       int cur_chan;
> > +       int num_chan;
> 
> This is always positive, so you can make num_chan and a few loop counters
> unsigned.

Nope, sadly not. cur_chan can get a -ERRNO, and the type of num_chan is
defined by the return type of of_count_phandle_with_args().


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

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
@ 2016-01-13 12:55         ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-13 12:55 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

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


> > +struct i2c_demux_pinctrl_priv {
> > +       int cur_chan;
> > +       int num_chan;
> 
> This is always positive, so you can make num_chan and a few loop counters
> unsigned.

Nope, sadly not. cur_chan can get a -ERRNO, and the type of num_chan is
defined by the return type of of_count_phandle_with_args().


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

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
  2016-01-06 17:08     ` Rob Herring
@ 2016-01-13 12:55       ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-13 12:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-i2c, SH-Linux, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

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


> > +       mutex_lock(&of_mutex);
> > +       ret = of_changeset_apply(&priv->chan[new_chan].chgset);
> > +       mutex_unlock(&of_mutex);
> 
> Looks like you need this patch[1] rather than exposing of_mutex.

Tasty! Thanks for the pointer, Rob.


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

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
@ 2016-01-13 12:55       ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-13 12:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-i2c, SH-Linux, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

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


> > +       mutex_lock(&of_mutex);
> > +       ret = of_changeset_apply(&priv->chan[new_chan].chgset);
> > +       mutex_unlock(&of_mutex);
> 
> Looks like you need this patch[1] rather than exposing of_mutex.

Tasty! Thanks for the pointer, Rob.


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

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
  2016-01-07 16:56     ` Jan Lübbe
@ 2016-01-13 13:02       ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-13 13:02 UTC (permalink / raw)
  To: Jan Lübbe
  Cc: linux-i2c, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou

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

Hi Jan,

> > +Required properties:
> > +- compatible: "i2c-demux-pinctrl"
> > +- i2c-parent: List of phandles of I2C masters available for selection. The first
> > +	      one will be used as default.
> > +- i2c-bus-name: The name of this bus. Also needed as pinctrl-name for the I2C
> > +		parents.
> [...]
> > +	i2chdmi: i2c@8 {
> > +		compatible = "i2c-demux-pinctrl";
> > +		i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
> > +		i2c-bus-name = "i2c-hdmi";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> [...]
> > +	gpioi2c: i2c@9 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "i2c-gpio";
> [...]
> > +&i2c2	{
> > +	pinctrl-0 = <&i2c2_pins>;
> > +	pinctrl-names = "i2c-hdmi";
> > +
> > +	clock-frequency = <100000>;
> > +};
> [...]
> > +&iic2	{
> > +	pinctrl-0 = <&iic2_pins>;
> > +	pinctrl-names = "i2c-hdmi";
> > +
> > +	clock-frequency = <100000>;
> > +};
> [...]
> 
> It seems that the demux-pinctrl driver reconfigures the pinctrl settings
> for the parent devices. I would have expected to have alternative
> pinctrl state on the demux node support switching the same external pins
> between the different controllers. Wouldn't it be possible to have
> pinctrl conflicts between &i2c2_pins and &iic2_pins otherwise?

I don't think so. Before i2c2 is enabled, iic2 gets disabled (status =
"disabled" via OF_DYNAMIC) and thus the pins get free. Doing it this
way, you could even have something like this:

	pinctrl-0 = <&iic2_pins>, <&iic2_c_pins>;
	pinctrl-names = "i2c-hdmi", "i2c-sensors";

which allows you to demux this controller to this or that bus. I haven't
tested this, though.

   Wolfram


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

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
@ 2016-01-13 13:02       ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2016-01-13 13:02 UTC (permalink / raw)
  To: Jan Lübbe
  Cc: linux-i2c, linux-sh, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou

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

Hi Jan,

> > +Required properties:
> > +- compatible: "i2c-demux-pinctrl"
> > +- i2c-parent: List of phandles of I2C masters available for selection. The first
> > +	      one will be used as default.
> > +- i2c-bus-name: The name of this bus. Also needed as pinctrl-name for the I2C
> > +		parents.
> [...]
> > +	i2chdmi: i2c@8 {
> > +		compatible = "i2c-demux-pinctrl";
> > +		i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
> > +		i2c-bus-name = "i2c-hdmi";
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> [...]
> > +	gpioi2c: i2c@9 {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +		compatible = "i2c-gpio";
> [...]
> > +&i2c2	{
> > +	pinctrl-0 = <&i2c2_pins>;
> > +	pinctrl-names = "i2c-hdmi";
> > +
> > +	clock-frequency = <100000>;
> > +};
> [...]
> > +&iic2	{
> > +	pinctrl-0 = <&iic2_pins>;
> > +	pinctrl-names = "i2c-hdmi";
> > +
> > +	clock-frequency = <100000>;
> > +};
> [...]
> 
> It seems that the demux-pinctrl driver reconfigures the pinctrl settings
> for the parent devices. I would have expected to have alternative
> pinctrl state on the demux node support switching the same external pins
> between the different controllers. Wouldn't it be possible to have
> pinctrl conflicts between &i2c2_pins and &iic2_pins otherwise?

I don't think so. Before i2c2 is enabled, iic2 gets disabled (status =
"disabled" via OF_DYNAMIC) and thus the pins get free. Doing it this
way, you could even have something like this:

	pinctrl-0 = <&iic2_pins>, <&iic2_c_pins>;
	pinctrl-names = "i2c-hdmi", "i2c-sensors";

which allows you to demux this controller to this or that bus. I haven't
tested this, though.

   Wolfram


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

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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
  2016-01-13 12:52       ` Wolfram Sang
@ 2016-01-13 14:07         ` Sergei Shtylyov
  -1 siblings, 0 replies; 38+ messages in thread
From: Sergei Shtylyov @ 2016-01-13 14:07 UTC (permalink / raw)
  To: Wolfram Sang, Geert Uytterhoeven
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

Hello.

On 01/13/2016 03:52 PM, Wolfram Sang wrote:

>>> These bindings allow an I2C bus to switch between multiple masters. This
>>> is not hot-swichting because connected I2C slaves will be
>>> re-instantiated. It is meant to select the best I2C core at runtime once
>>> the task is known. Example: Prefer i2c-gpio over another I2C core
>>> because of HW errata affetcing your use case.
>>
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
>>> @@ -0,0 +1,155 @@
>>
>>> +Here is a snipplet for a bus to be demuxed. It contains various i2c clients for
>>
>> snippet
>>
>>> +HDMI, so the bus is named "i2c-hdmi":
>>> +
>>> +       i2chdmi: i2c@8 {
>>
>> Would it make sense to call the node "i2c-bus@8"?
>
> That sounds like a question for our EPapr expert Sergei (CCed) :)

    ePAPR only mentions "i2c". I'm not sure where the numeric part of the name 
comes from in this case...

[...]

MBR, Sergei


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

* Re: [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings
@ 2016-01-13 14:07         ` Sergei Shtylyov
  0 siblings, 0 replies; 38+ messages in thread
From: Sergei Shtylyov @ 2016-01-13 14:07 UTC (permalink / raw)
  To: Wolfram Sang, Geert Uytterhoeven
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

Hello.

On 01/13/2016 03:52 PM, Wolfram Sang wrote:

>>> These bindings allow an I2C bus to switch between multiple masters. This
>>> is not hot-swichting because connected I2C slaves will be
>>> re-instantiated. It is meant to select the best I2C core at runtime once
>>> the task is known. Example: Prefer i2c-gpio over another I2C core
>>> because of HW errata affetcing your use case.
>>
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
>>> @@ -0,0 +1,155 @@
>>
>>> +Here is a snipplet for a bus to be demuxed. It contains various i2c clients for
>>
>> snippet
>>
>>> +HDMI, so the bus is named "i2c-hdmi":
>>> +
>>> +       i2chdmi: i2c@8 {
>>
>> Would it make sense to call the node "i2c-bus@8"?
>
> That sounds like a question for our EPapr expert Sergei (CCed) :)

    ePAPR only mentions "i2c". I'm not sure where the numeric part of the name 
comes from in this case...

[...]

MBR, Sergei

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
  2016-01-13 12:55         ` Wolfram Sang
  (?)
@ 2016-01-13 16:16           ` Geert Uytterhoeven
  -1 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-13 16:16 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou,
	Jan Luebbe

Hi Wolfram,

On Wed, Jan 13, 2016 at 1:55 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> > +struct i2c_demux_pinctrl_priv {
>> > +       int cur_chan;
>> > +       int num_chan;
>>
>> This is always positive, so you can make num_chan and a few loop counters
>> unsigned.
>
> Nope, sadly not. cur_chan can get a -ERRNO, and the type of num_chan is

Sorry for the bad quoting, my comment applied to num_chan only.

> defined by the return type of of_count_phandle_with_args().

But you will never store negative error codes returned by that function, as you
won't get to the storing part...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
@ 2016-01-13 16:16           ` Geert Uytterhoeven
  0 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-13 16:16 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm, devicetree, linux-kernel,
	Pantelis Antoniou, Jan Luebbe

Hi Wolfram,

On Wed, Jan 13, 2016 at 1:55 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> > +struct i2c_demux_pinctrl_priv {
>> > +       int cur_chan;
>> > +       int num_chan;
>>
>> This is always positive, so you can make num_chan and a few loop counters
>> unsigned.
>
> Nope, sadly not. cur_chan can get a -ERRNO, and the type of num_chan is

Sorry for the bad quoting, my comment applied to num_chan only.

> defined by the return type of of_count_phandle_with_args().

But you will never store negative error codes returned by that function, as you
won't get to the storing part...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver
@ 2016-01-13 16:16           ` Geert Uytterhoeven
  0 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2016-01-13 16:16 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou,
	Jan Luebbe

Hi Wolfram,

On Wed, Jan 13, 2016 at 1:55 PM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>> > +struct i2c_demux_pinctrl_priv {
>> > +       int cur_chan;
>> > +       int num_chan;
>>
>> This is always positive, so you can make num_chan and a few loop counters
>> unsigned.
>
> Nope, sadly not. cur_chan can get a -ERRNO, and the type of num_chan is

Sorry for the bad quoting, my comment applied to num_chan only.

> defined by the return type of of_count_phandle_with_args().

But you will never store negative error codes returned by that function, as you
won't get to the storing part...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2016-01-13 16:16 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-06 13:51 [RFC v2 0/4] i2c/of: switch I2C IP cores at runtime via OF_DYNAMIC Wolfram Sang
2016-01-06 13:51 ` Wolfram Sang
2016-01-06 13:51 ` [RFC v2 1/4] of: make of_mutex public Wolfram Sang
2016-01-06 13:51   ` Wolfram Sang
2016-01-06 14:26   ` Geert Uytterhoeven
2016-01-06 14:26     ` Geert Uytterhoeven
2016-01-06 13:51 ` [RFC v2 2/4] dt-bindings: i2c: mux: demux-pinctrl: add bindings Wolfram Sang
2016-01-06 13:51   ` Wolfram Sang
2016-01-06 14:30   ` Geert Uytterhoeven
2016-01-06 14:30     ` Geert Uytterhoeven
2016-01-13 12:52     ` Wolfram Sang
2016-01-13 12:52       ` Wolfram Sang
2016-01-13 14:07       ` Sergei Shtylyov
2016-01-13 14:07         ` Sergei Shtylyov
2016-01-07 16:56   ` Jan Lübbe
2016-01-07 16:56     ` Jan Lübbe
2016-01-13 13:02     ` Wolfram Sang
2016-01-13 13:02       ` Wolfram Sang
2016-01-11  2:52   ` Rob Herring
2016-01-11  2:52     ` Rob Herring
2016-01-11  8:01     ` Geert Uytterhoeven
2016-01-11  8:01       ` Geert Uytterhoeven
2016-01-06 13:51 ` [RFC v2 3/4] i2c: mux: demux-pinctrl: add driver Wolfram Sang
2016-01-06 13:51   ` Wolfram Sang
     [not found]   ` <1452088285-6427-4-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
2016-01-06 14:20     ` Geert Uytterhoeven
2016-01-06 14:20       ` Geert Uytterhoeven
2016-01-06 14:20       ` Geert Uytterhoeven
2016-01-13 12:55       ` Wolfram Sang
2016-01-13 12:55         ` Wolfram Sang
2016-01-13 16:16         ` Geert Uytterhoeven
2016-01-13 16:16           ` Geert Uytterhoeven
2016-01-13 16:16           ` Geert Uytterhoeven
2016-01-06 17:08   ` Rob Herring
2016-01-06 17:08     ` Rob Herring
2016-01-13 12:55     ` Wolfram Sang
2016-01-13 12:55       ` Wolfram Sang
2016-01-06 13:51 ` [RFC v2 4/4] ARM: shmobile: r8a7790: rework dts to use i2c demuxer Wolfram Sang
2016-01-06 13:51   ` Wolfram Sang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.