[RFC 0/3] switch I2C IP cores at runtime

[RFC 0/3] switch I2C IP cores at runtime

[Wolfram Sang]

This series allows an I2C bus to switch between multiple masters. 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 use case.

It works by using OF_DYNAMIC and en-/disabling the devices as needed. See patch
2 for the implementation and more documentation.


Changes since alpha v2 (only sent to sh-devel):

* don't use the i2c-mux infrastructure but create own adapter
  This makes the DT binding proper. Otherwise, they were nested
  another level deeper only for a useless reg = <0>;
  It makes the approach also less I2C specific.

* the bus can now be named
  I like this and consider moving this out of this series and
  implement it in the i2c-core.

* pinctrl states to be used are now named as the bus
  "active" was just a too random name IMO.

* bugfixes (removed two OOPSes)

* more error checking

* beautified the sysfs-readout to make distinction of adapters
  easier

So, this not only a brush up since the last version, rather a new revision.
I couldn't send this to the public with these obvious points left open :)

PS: Since these patches depend on two others, I pushed out a branch here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/ip-core-switch-v2-on-4.1-experimental


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

 .../devicetree/bindings/i2c/i2c-demux-pinctrl.txt  |  44 ++++
 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              | 266 +++++++++++++++++++++
 drivers/of/of_private.h                            |   1 -
 include/linux/of.h                                 |   2 +
 7 files changed, 411 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

[RFC 1/3] of: make of_mutex public

[Wolfram Sang]

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. If OF maintainers don't like exporting, we can
maybe create accessor functions that we can use?

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 b871ff9d81d720..f0464efcbdc5aa 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

[RFC 1/3] of: make of_mutex public

[Rob Herring]

+Pantelis

On Tue, Jun 30, 2015 at 4:44 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. If OF maintainers don't like exporting, we can
> maybe create accessor functions that we can use?

It looks like you are just taking the mutex around various
of_changeset_* calls. We should either split the mutex into of_mutex
for core and changesets and an overlay mutex for overlays, or we need
to do the typical pattern of having changeset functions having locked
and unlocked versions for external and internal (overlays) use.

Rob

> 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 b871ff9d81d720..f0464efcbdc5aa 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 1/3] of: make of_mutex public

[Pantelis Antoniou]

+Grant

Hi Rob,

> On Jul 3, 2015, at 07:43 , Rob Herring <robherring2@gmail.com> wrote:
> 
> +Pantelis
> 
> On Tue, Jun 30, 2015 at 4:44 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. If OF maintainers don't like exporting, we can
>> maybe create accessor functions that we can use?
> 
> It looks like you are just taking the mutex around various
> of_changeset_* calls. We should either split the mutex into of_mutex
> for core and changesets and an overlay mutex for overlays, or we need
> to do the typical pattern of having changeset functions having locked
> and unlocked versions for external and internal (overlays) use.
> 

IIRC we had an argument with Grant just about that. The original
overlays patches did use a two level locking scheme, a changeset specific 
mutex, and a global transaction mutex.

They were dropped after Grant?s request.

>> +
>> +/**
>> + * struct of_transaction - transaction tracker structure
>> + *
>> + * @lock:	lock for accessing the te_list and state entries
>> + * @te_list:	list_head for the transaction entries
>> + * @state:	State of the transaction
>> + *
>> + * Transactions are a convenient way to apply bulk changes to the
>> + * live tree. In case of an error, changes are rolled-back.
>> + * Transactions live on after initial application, and if not
>> + * destroyed after use, they can be reverted in one single call.
>> + */
>> +struct of_transaction {
>> +	struct mutex lock;
> 
> What's the lock for? There will only ever by a single transaction in
> flight at any given time, and when a transaction is /not/ in flight, it
> is completely owned by the caller. I see no condition where there needs
> to be a lock just for the of_transaction structure.

What is it that we?re trying to do here? What is the use case?

> Rob
> 

Regards

? Pantelis

>> 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 b871ff9d81d720..f0464efcbdc5aa 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
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 1/3] of: make of_mutex public

[Wolfram Sang]

> What is it that we?re trying to do here? What is the use case?

http://www.spinics.net/lists/devicetree/msg85462.html

I'll bounce the mails to you as well.

Thanks,

   Wolfram
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150703/d690b338/attachment.sig>

[RFC 2/3] i2c: mux: demux-pinctrl: add driver

[Wolfram Sang]

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

This driver allows an I2C bus to switch between multiple masters. 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 use case.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 .../devicetree/bindings/i2c/i2c-demux-pinctrl.txt  |  44 ++++
 drivers/i2c/muxes/Kconfig                          |   9 +
 drivers/i2c/muxes/Makefile                         |   2 +
 drivers/i2c/muxes/i2c-demux-pinctrl.c              | 266 +++++++++++++++++++++
 4 files changed, 321 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
 create mode 100644 drivers/i2c/muxes/i2c-demux-pinctrl.c

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..2211f6acbf8c0c
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
@@ -0,0 +1,44 @@
+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.
+
+    +------------------------------+
+    | SoC                          |
+    |                              |   +-----+  +-----+
+    |   +------------+             |   | dev |  | dev |
+    |   |I2C IP Core1|--\          |   +-----+  +-----+
+    |   +------------+   \------+  |      |        |
+    |                    |Pinmux|--|------+--------+
+    |   +------------+   +------+  |
+    |   |I2C IP Core2|--/          |
+    |   +------------+             |
+    |                              |
+    +------------------------------+
+
+Required properties:
+- compatible: "i2c-demux-pinctrl"
+- i2c-parent: List of phandles of I2C cores to be selected
+- i2c-bus-name: the name of this bus
+
+Also I2C mux properties and child nodes. See mux.txt in this directory.
+
+Also required:
+
+- 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.
+
+//FIXME: add example
+
+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.
diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index f6d313e528de34..b99cc15d683a50 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -60,4 +60,13 @@ config I2C_MUX_PINCTRL
 	  This driver can also be built as a module. If so, the module will be
 	  called pinctrl-i2cmux.
 
+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 465778b5d5dc86..4d14cb88d83438 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..5444c65de76c55
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
@@ -0,0 +1,266 @@
+/*
+ * Pinctrl based I2C DeMultiplexer V2 PROTOTYPE!
+ *
+ * Copyright (C) 2015 by Wolfram Sang, Sang Engineering <wsa@sang-engineering.com>
+ * Copyright (C) 2015 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)
+		return ret;
+
+	adap = of_find_i2c_adapter_by_node(priv->chan[new_chan].parent_np);
+	if (!adap)
+		return -EPROBE_DEFER;
+
+	p = devm_pinctrl_get_select(adap->dev.parent, priv->bus_name);
+	if (IS_ERR(p)) {
+		put_device(&adap->dev);
+		return PTR_ERR(p);
+	}
+
+	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) {
+		dev_err(priv->dev, "failed to add demux-adapter (%d)\n", ret);
+		priv->cur_chan = -EINVAL;
+		return ret;
+	}
+
+	return 0;
+}
+
+static int i2c_demux_deactivate_master(struct i2c_demux_pinctrl_priv *priv)
+{
+	int ret;
+
+	i2c_del_adapter(&priv->cur_adap);
+	put_device(&priv->chan[priv->cur_chan].parent_adap->dev);
+
+	mutex_lock(&of_mutex);
+	ret = of_changeset_revert(&priv->chan[priv->cur_chan].chgset);
+	mutex_unlock(&of_mutex);
+
+	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; i++)
+		count += sprintf(buf + count, "%c %d - %s\n",
+				 i == priv->cur_chan ? '*' : ' ', i,
+				 priv->chan[i].parent_np->full_name);
+	//FIXME: Check count > PAGE_SIZE
+
+	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

[RFC 2/3] i2c: mux: demux-pinctrl: add driver

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Jun 30, 2015 at 11:44 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-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 use case.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  .../devicetree/bindings/i2c/i2c-demux-pinctrl.txt  |  44 ++++
>  drivers/i2c/muxes/Kconfig                          |   9 +
>  drivers/i2c/muxes/Makefile                         |   2 +
>  drivers/i2c/muxes/i2c-demux-pinctrl.c              | 266 +++++++++++++++++++++
>  4 files changed, 321 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt
>  create mode 100644 drivers/i2c/muxes/i2c-demux-pinctrl.c
>
> 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..2211f6acbf8c0c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-demux-pinctrl.txt

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

This paragraph doesn't belong in the DT binding doc, but somewhere
under Documentation/i2c/

> diff --git a/drivers/i2c/muxes/i2c-demux-pinctrl.c b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> new file mode 100644
> index 00000000000000..5444c65de76c55
> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-demux-pinctrl.c
> @@ -0,0 +1,266 @@
> +/*
> + * Pinctrl based I2C DeMultiplexer V2 PROTOTYPE!
> + *
> + * Copyright (C) 2015 by Wolfram Sang, Sang Engineering <wsa@sang-engineering.com>
> + * Copyright (C) 2015 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;

unsigned int

> +       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" };

"okay" or "ok"?

> +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;

unsigned int i

> +
> +       for (i = 0; i < priv->num_chan; i++)
> +               count += sprintf(buf + count, "%c %d - %s\n",
> +                                i == priv->cur_chan ? '*' : ' ', i,
> +                                priv->chan[i].parent_np->full_name);
> +       //FIXME: Check count > PAGE_SIZE

Can you use seq_printf() for device attributes?

> +
> +       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;

unsigned int

> +       int ret;
> +
> +       ret = kstrtoul(buf, 0, &val);

kstrtouint()

> +       if (ret < 0)
> +               return ret;
> +
> +       if (val >= priv->num_chan)
> +               return -EINVAL;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

[RFC 2/3] i2c: mux: demux-pinctrl: add driver

[Wolfram Sang]

> > +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.
> 
> This paragraph doesn't belong in the DT binding doc, but somewhere
> under Documentation/i2c/

Yes.

> > +struct i2c_demux_pinctrl_priv {
> > +       int cur_chan;
> > +       int num_chan;
> 
> unsigned int

When comparing variables, I prefer to have them the same signedness.

> > +static struct property status_okay = { .name = "status", .length = 3, .value = "ok" };
> 
> "okay" or "ok"?

Both are valid, I took the shorter one.

> > +       struct i2c_demux_pinctrl_priv *priv = dev_get_drvdata(dev);
> > +       int count = 0, i;
> 
> unsigned int i

Same as above.

> 
> > +
> > +       for (i = 0; i < priv->num_chan; i++)
> > +               count += sprintf(buf + count, "%c %d - %s\n",
> > +                                i == priv->cur_chan ? '*' : ' ', i,
> > +                                priv->chan[i].parent_np->full_name);
> > +       //FIXME: Check count > PAGE_SIZE
> 
> Can you use seq_printf() for device attributes?

I can't find a reference to that.


> > +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;
> 
> unsigned int
> 
> > +       int ret;
> > +
> > +       ret = kstrtoul(buf, 0, &val);
> 
> kstrtouint()

Yes, I agree to the last two.

Thanks for the review,

   Wolfram

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150701/fa07ecde/attachment.sig>

[RFC 3/3] ARM: shmobile: r8a7790: rework dts to use i2c demuxer

[Wolfram Sang]

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 aaa4f258e279cc..c695728b451506 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 {
@@ -245,6 +247,79 @@
 		#clock-cells = <0>;
 		clock-frequency = <148500000>;
 	};
+
+
+	gpioi2c: i2c at 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 at 8 {
+
+		compatible = "i2c-demux-pinctrl";
+		i2c-parent = <&gpioi2c>, <&iic2>, <&i2c2>;
+		i2c-bus-name = "i2c-hdmi";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ak4643: sound-codec at 12 {
+			compatible = "asahi-kasei,ak4643";
+
+			#sound-dai-cells = <0>;
+			reg = <0x12>;
+		};
+
+		composite-in at 20 {
+			compatible = "adi,adv7180";
+			reg = <0x20>;
+			remote = <&vin1>;
+
+			port {
+				adv7180: endpoint {
+					bus-width = <8>;
+					remote-endpoint = <&vin1ep0>;
+				};
+			};
+		};
+
+		hdmi at 39 {
+			compatible = "adi,adv7511w";
+			reg = <0x39>;
+			interrupt-parent = <&gpio1>;
+			interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
+
+			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 at 0 {
+					reg = <0>;
+					adv7511_in: endpoint {
+						remote-endpoint = <&du_out_lvds0>;
+					};
+				};
+
+				port at 1 {
+					reg = <1>;
+					adv7511_out: endpoint {
+						remote-endpoint = <&hdmi_con>;
+					};
+				};
+			};
+		};
+	};
 };
 
 &du {
@@ -340,6 +415,11 @@
 		renesas,function = "iic1";
 	};
 
+	i2c2_pins: i2c2 {
+		renesas,groups = "i2c2";
+		renesas,function = "i2c2";
+	};
+
 	iic2_pins: iic2 {
 		renesas,groups = "iic2";
 		renesas,function = "iic2";
@@ -518,63 +598,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: sound-codec at 12 {
-		compatible = "asahi-kasei,ak4643";
-		#sound-dai-cells = <0>;
-		reg = <0x12>;
-	};
-
-	composite-in at 20 {
-		compatible = "adi,adv7180";
-		reg = <0x20>;
-		remote = <&vin1>;
-
-		port {
-			adv7180: endpoint {
-				bus-width = <8>;
-				remote-endpoint = <&vin1ep0>;
-			};
-		};
-	};
-
-	hdmi at 39 {
-		compatible = "adi,adv7511w";
-		reg = <0x39>;
-		interrupt-parent = <&gpio1>;
-		interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
-
-		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 at 0 {
-				reg = <0>;
-				adv7511_in: endpoint {
-					remote-endpoint = <&du_out_lvds0>;
-				};
-			};
+&iic2	{
+	pinctrl-0 = <&iic2_pins>;
+	pinctrl-names = "i2c-hdmi";
 
-			port at 1 {
-				reg = <1>;
-				adv7511_out: endpoint {
-					remote-endpoint = <&hdmi_con>;
-				};
-			};
-		};
-	};
+	clock-frequency = <100000>;
 };
 
 &iic3 {
-- 
2.1.4

[RFC 3/3] ARM: shmobile: r8a7790: rework dts to use i2c demuxer

[Geert Uytterhoeven]

On Tue, Jun 30, 2015 at 11:44 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> --- 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;

"i2c<suffix>" or "<prefix>i2c"?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

[RFC 3/3] ARM: shmobile: r8a7790: rework dts to use i2c demuxer

[Wolfram Sang]

On Wed, Jul 01, 2015 at 09:10:23AM +0200, Geert Uytterhoeven wrote:
> On Tue, Jun 30, 2015 at 11:44 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> > --- 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;
> 
> "i2c<suffix>" or "<prefix>i2c"?

i2c<suffix> describes the bus here. <prefix>i2c another master. Could be
made more obvious, probably.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150701/e786482a/attachment.sig>