All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH linux dev-4.13] drivers: i2c: master arbiter: create driver
@ 2018-02-23  5:22 Ken Chen
  2018-03-08  3:02 ` Joel Stanley
  0 siblings, 1 reply; 6+ messages in thread
From: Ken Chen @ 2018-02-23  5:22 UTC (permalink / raw)
  To: openbmc; +Cc: Ken Chen, joel

Initial PCA9641 2 chennel I2C bus master arbiter

Signed-off-by: Ken Chen <chen.kenyy@inventec.com>
---
 drivers/i2c/muxes/Kconfig           |   9 +
 drivers/i2c/muxes/Makefile          |   1 +
 drivers/i2c/muxes/i2c-mux-pca9641.c | 372 ++++++++++++++++++++++++++++++++++++
 3 files changed, 382 insertions(+)
 create mode 100644 drivers/i2c/muxes/i2c-mux-pca9641.c

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 1712132..1cd1ad3 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -73,6 +73,15 @@ config I2C_MUX_PCA954x
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mux-pca954x.
 
+config I2C_MUX_PCA9641
+	tristate "NXP PCA9641 I2C Master demultiplexer"
+	help
+	  If you say yes here you get support for the NXP PCA9641
+	  I2C Master demultiplexer with an arbiter function.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-mux-pca9641.
+
 config I2C_MUX_PINCTRL
 	tristate "pinctrl-based I2C multiplexer"
 	depends on PINCTRL
diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
index 4a67d31..f95d5f5 100644
--- a/drivers/i2c/muxes/Makefile
+++ b/drivers/i2c/muxes/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_I2C_MUX_LTC4306)	+= i2c-mux-ltc4306.o
 obj-$(CONFIG_I2C_MUX_MLXCPLD)	+= i2c-mux-mlxcpld.o
 obj-$(CONFIG_I2C_MUX_PCA9541)	+= i2c-mux-pca9541.o
 obj-$(CONFIG_I2C_MUX_PCA954x)	+= i2c-mux-pca954x.o
+obj-$(CONFIG_I2C_MUX_PCA9641)	+= i2c-mux-pca9641.o
 obj-$(CONFIG_I2C_MUX_PINCTRL)	+= i2c-mux-pinctrl.o
 obj-$(CONFIG_I2C_MUX_REG)	+= i2c-mux-reg.o
 
diff --git a/drivers/i2c/muxes/i2c-mux-pca9641.c b/drivers/i2c/muxes/i2c-mux-pca9641.c
new file mode 100644
index 0000000..ca7b816
--- /dev/null
+++ b/drivers/i2c/muxes/i2c-mux-pca9641.c
@@ -0,0 +1,372 @@
+/*
+ * I2C demultiplexer driver for PCA9641 bus master demultiplexer
+ *
+ * Copyright (c) 2010 Ericsson AB.
+ *
+ * Author: Ken Chen <chen.kenyy@inventec.com>
+ *
+ * Derived from:
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/jiffies.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/i2c-mux.h>
+
+#include <linux/i2c/pca954x.h>
+
+/*
+ * The PCA9641 is two I2C bus masters demultiplexer. It supports two I2C masters 
+ * connected to a single slave bus.
+ *
+ * It is designed for high reliability dual master I2C bus applications where
+ * correct system operation is required, even when two I2C master issue their
+ * commands at the same time. The arbiter will select a winner and let it work
+ * uninterrupted, and the losing master will take control of the I2C bus after
+ * the winnter has finished. The arbiter also allows for queued requests where
+ * a master requests the downstream bus while the other master has control.
+ *
+ */
+
+#define PCA9641_ID		0x01
+#define PCA9641_ID_MAGIC	0x38
+
+#define PCA9641_CONTROL		0x01
+#define PCA9641_STATUS		0x02
+#define PCA9641_TIME		0x03
+
+#define PCA9641_CTL_LOCK_REQ            (1 << 0)
+#define PCA9641_CTL_LOCK_GRANT          (1 << 1)
+#define PCA9641_CTL_BUS_CONNECT         (1 << 2)
+#define PCA9641_CTL_BUS_INIT            (1 << 3)
+#define PCA9641_CTL_SMBUS_SWRST         (1 << 4)
+#define PCA9641_CTL_IDLE_TIMER_DIS      (1 << 5)
+#define PCA9641_CTL_SMBUS_DIS           (1 << 6)
+#define PCA9641_CTL_PRIORITY            (1 << 7)
+
+#define PCA9641_STS_OTHER_LOCK          (1 << 0)
+#define PCA9641_STS_BUS_INIT_FAIL       (1 << 1)
+#define PCA9641_STS_BUS_HUNG            (1 << 2)
+#define PCA9641_STS_MBOX_EMPTY          (1 << 3)
+#define PCA9641_STS_MBOX_FULL           (1 << 4)
+#define PCA9641_STS_TEST_INT            (1 << 5)
+#define PCA9641_STS_SCL_IO              (1 << 6)
+#define PCA9641_STS_SDA_IO              (1 << 7)
+
+#define PCA9641_RES_TIME	0x03
+
+#define other_lock(x)  ((x) & PCA9641_STS_OTHER_LOCK)
+#define lock_grant(x)  ((x) & PCA9641_CTL_LOCK_GRANT)
+
+/* arbitration timeouts, in jiffies */
+#define ARB_TIMEOUT	(HZ / 8)	/* 125 ms until forcing bus ownership */
+#define ARB2_TIMEOUT	(HZ / 4)	/* 250 ms until acquisition failure */
+
+/* arbitration retry delays, in us */
+#define SELECT_DELAY_SHORT	50
+#define SELECT_DELAY_LONG	1000
+
+struct pca9641 {
+	struct i2c_client *client;
+	unsigned long select_timeout;
+	unsigned long arb_timeout;
+};
+
+static const struct i2c_device_id pca9641_id[] = {
+	{"pca9641", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, pca9641_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id pca9641_of_match[] = {
+	{ .compatible = "nxp,pca9641" },
+	{}
+};
+#endif
+
+/*
+ * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
+ * as they will try to lock the adapter a second time.
+ */
+static int pca9641_reg_write(struct i2c_client *client, u8 command, u8 val)
+{
+	struct i2c_adapter *adap = client->adapter;
+	int ret;
+
+	if (adap->algo->master_xfer) {
+		struct i2c_msg msg;
+		char buf[2];
+
+		msg.addr = client->addr;
+		msg.flags = 0;
+		msg.len = 2;
+		buf[0] = command;
+		buf[1] = val;
+		msg.buf = buf;
+		ret = __i2c_transfer(adap, &msg, 1);
+	} else {
+		union i2c_smbus_data data;
+
+		data.byte = val;
+		ret = adap->algo->smbus_xfer(adap, client->addr,
+					     client->flags,
+					     I2C_SMBUS_WRITE,
+					     command,
+					     I2C_SMBUS_BYTE_DATA, &data);
+	}
+
+	return ret;
+}
+
+/*
+ * Read from chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
+ * as they will try to lock adapter a second time.
+ */
+static int pca9641_reg_read(struct i2c_client *client, u8 command)
+{
+	struct i2c_adapter *adap = client->adapter;
+	int ret;
+	u8 val;
+
+	if (adap->algo->master_xfer) {
+		struct i2c_msg msg[2] = {
+			{
+				.addr = client->addr,
+				.flags = 0,
+				.len = 1,
+				.buf = &command
+			},
+			{
+				.addr = client->addr,
+				.flags = I2C_M_RD,
+				.len = 1,
+				.buf = &val
+			}
+		};
+		ret = __i2c_transfer(adap, msg, 2);
+		if (ret == 2)
+			ret = val;
+		else if (ret >= 0)
+			ret = -EIO;
+	} else {
+		union i2c_smbus_data data;
+
+		ret = adap->algo->smbus_xfer(adap, client->addr,
+					     client->flags,
+					     I2C_SMBUS_READ,
+					     command,
+					     I2C_SMBUS_BYTE_DATA, &data);
+		if (!ret)
+			ret = data.byte;
+	}
+	return ret;
+}
+
+/*
+ * Arbitration management functions
+ */
+static void pca9641_release_bus(struct i2c_client *client)
+{
+	int reg;
+
+	pca9641_reg_write(client, PCA9641_CONTROL, 0);
+}
+
+/*
+ * Channel arbitration
+ *
+ * Return values:
+ *  <0: error
+ *  0 : bus not acquired
+ *  1 : bus acquired
+ */
+static int pca9641_arbitrate(struct i2c_client *client)
+{
+	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+	struct pca9641 *data = i2c_mux_priv(muxc);
+	int reg_ctl, reg_sts;
+
+	reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
+	if (reg_ctl < 0)
+		return reg_ctl;
+	reg_sts = pca9641_reg_read(client, PCA9641_STATUS);
+
+	if (!other_lock(reg_sts) && !lock_grant(reg_ctl)) {
+		/*
+		 * Bus is off. Request ownership or turn it on unless
+		 * other master requested ownership.
+		 */
+                reg_ctl |= PCA9641_CTL_LOCK_REQ;
+                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
+
+                udelay(100);
+
+		reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
+
+                if (lock_grant(reg_ctl)) {
+                        /*
+                         * Other master did not request ownership,
+                         * or arbitration timeout expired. Take the bus.
+                         */
+                        reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
+                        pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
+                        data->select_timeout = SELECT_DELAY_SHORT;
+
+                        return 1;
+                } else {
+                        /*
+                         * Other master requested ownership.
+                         * Set extra long timeout to give it time to acquire it.
+                         */
+                        data->select_timeout = SELECT_DELAY_LONG * 2;
+                }
+	} else if (lock_grant(reg_ctl)) {
+                /*
+                 * Bus is on, and we own it. We are done with acquisition.
+                 */
+                reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
+                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
+
+                return 1;
+	} else if (other_lock(reg_sts)) {
+                /*
+                 * Other master owns the bus.
+                 * If arbitration timeout has expired, force ownership.
+                 * Otherwise request it.
+                 */
+                data->select_timeout = SELECT_DELAY_LONG;
+                reg_ctl |= PCA9641_CTL_LOCK_REQ;
+                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
+
+                /*
+                 * if (time_is_before_eq_jiffies(data->arb_timeout)) {
+                 * TODO:Time is up, take the bus and reset it.
+                 *
+                 *} else {
+                 * TODO: Request bus ownership if needed
+                 *
+                 *}
+                 */
+        }
+	return 0;
+}
+
+static int pca9641_select_chan(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct pca9641 *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
+	int ret;
+	unsigned long timeout = jiffies + ARB2_TIMEOUT;
+		/* give up after this time */
+
+	data->arb_timeout = jiffies + ARB_TIMEOUT;
+		/* force bus ownership after this time */
+
+	do {
+		ret = pca9641_arbitrate(client);
+		if (ret)
+			return ret < 0 ? ret : 0;
+
+		if (data->select_timeout == SELECT_DELAY_SHORT)
+			udelay(data->select_timeout);
+		else
+			msleep(data->select_timeout / 1000);
+	} while (time_is_after_eq_jiffies(timeout));
+
+	return -ETIMEDOUT;
+}
+
+static int pca9641_release_chan(struct i2c_mux_core *muxc, u32 chan)
+{
+	struct pca9641 *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
+
+	pca9641_release_bus(client);
+	return 0;
+}
+
+/*
+ * I2C init/probing/exit functions
+ */
+static int pca9641_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct i2c_adapter *adap = client->adapter;
+	struct pca954x_platform_data *pdata = dev_get_platdata(&client->dev);
+	struct i2c_mux_core *muxc;
+	struct pca9641 *data;
+	int force;
+	int ret;
+
+	if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -ENODEV;
+
+	/*
+	 * I2C accesses are unprotected here.
+	 * We have to lock the adapter before releasing the bus.
+	 */
+	i2c_lock_adapter(adap);
+	pca9641_release_bus(client);
+	i2c_unlock_adapter(adap);
+
+	/* Create mux adapter */
+
+	force = 0;
+	if (pdata)
+		force = pdata->modes[0].adap_id;
+	muxc = i2c_mux_alloc(adap, &client->dev, 8, sizeof(*data),
+			     I2C_MUX_ARBITRATOR,
+			     pca9641_select_chan, pca9641_release_chan);
+	if (!muxc)
+		return -ENOMEM;
+
+	data = i2c_mux_priv(muxc);
+	data->client = client;
+
+	i2c_set_clientdata(client, muxc);
+
+
+	ret = i2c_mux_add_adapter(muxc, force, 0, 0);
+	if (ret) {
+		dev_err(&client->dev, "failed to register master demultiplexer\n");
+		return ret;
+	}
+
+	dev_info(&client->dev, "registered master demultiplexer for I2C %s\n",
+		 client->name);
+
+	return 0;
+}
+
+static int pca9641_remove(struct i2c_client *client)
+{
+	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+
+	i2c_mux_del_adapters(muxc);
+	return 0;
+}
+
+static struct i2c_driver pca9641_driver = {
+	.driver = {
+		   .name = "pca9641",
+		   .of_match_table = of_match_ptr(pca9641_of_match),
+		   },
+	.probe = pca9641_probe,
+	.remove = pca9641_remove,
+	.id_table = pca9641_id,
+};
+
+module_i2c_driver(pca9641_driver);
+
+MODULE_AUTHOR("Ken Chen <chen.kenyy@inventec.com>");
+MODULE_DESCRIPTION("PCA9641 I2C master demultiplexer driver");
+MODULE_LICENSE("GPL v2");
-- 
2.9.3

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

* Re: [PATCH linux dev-4.13] drivers: i2c: master arbiter: create driver
  2018-02-23  5:22 [PATCH linux dev-4.13] drivers: i2c: master arbiter: create driver Ken Chen
@ 2018-03-08  3:02 ` Joel Stanley
  2018-03-08  5:06   ` ChenKenYY 陳永營 TAO
  0 siblings, 1 reply; 6+ messages in thread
From: Joel Stanley @ 2018-03-08  3:02 UTC (permalink / raw)
  To: Ken Chen; +Cc: OpenBMC Maillist

Hello Ken,

On Fri, Feb 23, 2018 at 3:52 PM, Ken Chen <chen.kenyy@inventec.com> wrote:
> Initial PCA9641 2 chennel I2C bus master arbiter
>
> Signed-off-by: Ken Chen <chen.kenyy@inventec.com>

The code here looks good. I did some searching, and found that someone
submitted a driver for this part about a year ago:

 https://patchwork.ozlabs.org/patch/726491/

Unfortunately the submitter did not send another version after it was
reviewed. I suggest we take up the patch that was submitted, make the
changes suggested, and submit it upstream. Are you able to take on
that work?

Cheers,

Joel

> ---
>  drivers/i2c/muxes/Kconfig           |   9 +
>  drivers/i2c/muxes/Makefile          |   1 +
>  drivers/i2c/muxes/i2c-mux-pca9641.c | 372 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 382 insertions(+)
>  create mode 100644 drivers/i2c/muxes/i2c-mux-pca9641.c
>
> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> index 1712132..1cd1ad3 100644
> --- a/drivers/i2c/muxes/Kconfig
> +++ b/drivers/i2c/muxes/Kconfig
> @@ -73,6 +73,15 @@ config I2C_MUX_PCA954x
>           This driver can also be built as a module.  If so, the module
>           will be called i2c-mux-pca954x.
>
> +config I2C_MUX_PCA9641
> +       tristate "NXP PCA9641 I2C Master demultiplexer"
> +       help
> +         If you say yes here you get support for the NXP PCA9641
> +         I2C Master demultiplexer with an arbiter function.
> +
> +         This driver can also be built as a module.  If so, the module
> +         will be called i2c-mux-pca9641.
> +
>  config I2C_MUX_PINCTRL
>         tristate "pinctrl-based I2C multiplexer"
>         depends on PINCTRL
> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
> index 4a67d31..f95d5f5 100644
> --- a/drivers/i2c/muxes/Makefile
> +++ b/drivers/i2c/muxes/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_I2C_MUX_LTC4306) += i2c-mux-ltc4306.o
>  obj-$(CONFIG_I2C_MUX_MLXCPLD)  += i2c-mux-mlxcpld.o
>  obj-$(CONFIG_I2C_MUX_PCA9541)  += i2c-mux-pca9541.o
>  obj-$(CONFIG_I2C_MUX_PCA954x)  += i2c-mux-pca954x.o
> +obj-$(CONFIG_I2C_MUX_PCA9641)  += i2c-mux-pca9641.o
>  obj-$(CONFIG_I2C_MUX_PINCTRL)  += i2c-mux-pinctrl.o
>  obj-$(CONFIG_I2C_MUX_REG)      += i2c-mux-reg.o
>
> diff --git a/drivers/i2c/muxes/i2c-mux-pca9641.c b/drivers/i2c/muxes/i2c-mux-pca9641.c
> new file mode 100644
> index 0000000..ca7b816
> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-mux-pca9641.c
> @@ -0,0 +1,372 @@
> +/*
> + * I2C demultiplexer driver for PCA9641 bus master demultiplexer
> + *
> + * Copyright (c) 2010 Ericsson AB.
> + *
> + * Author: Ken Chen <chen.kenyy@inventec.com>
> + *
> + * Derived from:
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/jiffies.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-mux.h>
> +
> +#include <linux/i2c/pca954x.h>
> +
> +/*
> + * The PCA9641 is two I2C bus masters demultiplexer. It supports two I2C masters
> + * connected to a single slave bus.
> + *
> + * It is designed for high reliability dual master I2C bus applications where
> + * correct system operation is required, even when two I2C master issue their
> + * commands at the same time. The arbiter will select a winner and let it work
> + * uninterrupted, and the losing master will take control of the I2C bus after
> + * the winnter has finished. The arbiter also allows for queued requests where
> + * a master requests the downstream bus while the other master has control.
> + *
> + */
> +
> +#define PCA9641_ID             0x01
> +#define PCA9641_ID_MAGIC       0x38
> +
> +#define PCA9641_CONTROL                0x01
> +#define PCA9641_STATUS         0x02
> +#define PCA9641_TIME           0x03
> +
> +#define PCA9641_CTL_LOCK_REQ            (1 << 0)
> +#define PCA9641_CTL_LOCK_GRANT          (1 << 1)
> +#define PCA9641_CTL_BUS_CONNECT         (1 << 2)
> +#define PCA9641_CTL_BUS_INIT            (1 << 3)
> +#define PCA9641_CTL_SMBUS_SWRST         (1 << 4)
> +#define PCA9641_CTL_IDLE_TIMER_DIS      (1 << 5)
> +#define PCA9641_CTL_SMBUS_DIS           (1 << 6)
> +#define PCA9641_CTL_PRIORITY            (1 << 7)
> +
> +#define PCA9641_STS_OTHER_LOCK          (1 << 0)
> +#define PCA9641_STS_BUS_INIT_FAIL       (1 << 1)
> +#define PCA9641_STS_BUS_HUNG            (1 << 2)
> +#define PCA9641_STS_MBOX_EMPTY          (1 << 3)
> +#define PCA9641_STS_MBOX_FULL           (1 << 4)
> +#define PCA9641_STS_TEST_INT            (1 << 5)
> +#define PCA9641_STS_SCL_IO              (1 << 6)
> +#define PCA9641_STS_SDA_IO              (1 << 7)
> +
> +#define PCA9641_RES_TIME       0x03
> +
> +#define other_lock(x)  ((x) & PCA9641_STS_OTHER_LOCK)
> +#define lock_grant(x)  ((x) & PCA9641_CTL_LOCK_GRANT)
> +
> +/* arbitration timeouts, in jiffies */
> +#define ARB_TIMEOUT    (HZ / 8)        /* 125 ms until forcing bus ownership */
> +#define ARB2_TIMEOUT   (HZ / 4)        /* 250 ms until acquisition failure */
> +
> +/* arbitration retry delays, in us */
> +#define SELECT_DELAY_SHORT     50
> +#define SELECT_DELAY_LONG      1000
> +
> +struct pca9641 {
> +       struct i2c_client *client;
> +       unsigned long select_timeout;
> +       unsigned long arb_timeout;
> +};
> +
> +static const struct i2c_device_id pca9641_id[] = {
> +       {"pca9641", 0},
> +       {}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, pca9641_id);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id pca9641_of_match[] = {
> +       { .compatible = "nxp,pca9641" },
> +       {}
> +};
> +#endif
> +
> +/*
> + * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
> + * as they will try to lock the adapter a second time.
> + */
> +static int pca9641_reg_write(struct i2c_client *client, u8 command, u8 val)
> +{
> +       struct i2c_adapter *adap = client->adapter;
> +       int ret;
> +
> +       if (adap->algo->master_xfer) {
> +               struct i2c_msg msg;
> +               char buf[2];
> +
> +               msg.addr = client->addr;
> +               msg.flags = 0;
> +               msg.len = 2;
> +               buf[0] = command;
> +               buf[1] = val;
> +               msg.buf = buf;
> +               ret = __i2c_transfer(adap, &msg, 1);
> +       } else {
> +               union i2c_smbus_data data;
> +
> +               data.byte = val;
> +               ret = adap->algo->smbus_xfer(adap, client->addr,
> +                                            client->flags,
> +                                            I2C_SMBUS_WRITE,
> +                                            command,
> +                                            I2C_SMBUS_BYTE_DATA, &data);
> +       }
> +
> +       return ret;
> +}
> +
> +/*
> + * Read from chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
> + * as they will try to lock adapter a second time.
> + */
> +static int pca9641_reg_read(struct i2c_client *client, u8 command)
> +{
> +       struct i2c_adapter *adap = client->adapter;
> +       int ret;
> +       u8 val;
> +
> +       if (adap->algo->master_xfer) {
> +               struct i2c_msg msg[2] = {
> +                       {
> +                               .addr = client->addr,
> +                               .flags = 0,
> +                               .len = 1,
> +                               .buf = &command
> +                       },
> +                       {
> +                               .addr = client->addr,
> +                               .flags = I2C_M_RD,
> +                               .len = 1,
> +                               .buf = &val
> +                       }
> +               };
> +               ret = __i2c_transfer(adap, msg, 2);
> +               if (ret == 2)
> +                       ret = val;
> +               else if (ret >= 0)
> +                       ret = -EIO;
> +       } else {
> +               union i2c_smbus_data data;
> +
> +               ret = adap->algo->smbus_xfer(adap, client->addr,
> +                                            client->flags,
> +                                            I2C_SMBUS_READ,
> +                                            command,
> +                                            I2C_SMBUS_BYTE_DATA, &data);
> +               if (!ret)
> +                       ret = data.byte;
> +       }
> +       return ret;
> +}
> +
> +/*
> + * Arbitration management functions
> + */
> +static void pca9641_release_bus(struct i2c_client *client)
> +{
> +       int reg;
> +
> +       pca9641_reg_write(client, PCA9641_CONTROL, 0);
> +}
> +
> +/*
> + * Channel arbitration
> + *
> + * Return values:
> + *  <0: error
> + *  0 : bus not acquired
> + *  1 : bus acquired
> + */
> +static int pca9641_arbitrate(struct i2c_client *client)
> +{
> +       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
> +       struct pca9641 *data = i2c_mux_priv(muxc);
> +       int reg_ctl, reg_sts;
> +
> +       reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
> +       if (reg_ctl < 0)
> +               return reg_ctl;
> +       reg_sts = pca9641_reg_read(client, PCA9641_STATUS);
> +
> +       if (!other_lock(reg_sts) && !lock_grant(reg_ctl)) {
> +               /*
> +                * Bus is off. Request ownership or turn it on unless
> +                * other master requested ownership.
> +                */
> +                reg_ctl |= PCA9641_CTL_LOCK_REQ;
> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
> +
> +                udelay(100);
> +
> +               reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
> +
> +                if (lock_grant(reg_ctl)) {
> +                        /*
> +                         * Other master did not request ownership,
> +                         * or arbitration timeout expired. Take the bus.
> +                         */
> +                        reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
> +                        pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
> +                        data->select_timeout = SELECT_DELAY_SHORT;
> +
> +                        return 1;
> +                } else {
> +                        /*
> +                         * Other master requested ownership.
> +                         * Set extra long timeout to give it time to acquire it.
> +                         */
> +                        data->select_timeout = SELECT_DELAY_LONG * 2;
> +                }
> +       } else if (lock_grant(reg_ctl)) {
> +                /*
> +                 * Bus is on, and we own it. We are done with acquisition.
> +                 */
> +                reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
> +
> +                return 1;
> +       } else if (other_lock(reg_sts)) {
> +                /*
> +                 * Other master owns the bus.
> +                 * If arbitration timeout has expired, force ownership.
> +                 * Otherwise request it.
> +                 */
> +                data->select_timeout = SELECT_DELAY_LONG;
> +                reg_ctl |= PCA9641_CTL_LOCK_REQ;
> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
> +
> +                /*
> +                 * if (time_is_before_eq_jiffies(data->arb_timeout)) {
> +                 * TODO:Time is up, take the bus and reset it.
> +                 *
> +                 *} else {
> +                 * TODO: Request bus ownership if needed
> +                 *
> +                 *}
> +                 */
> +        }
> +       return 0;
> +}
> +
> +static int pca9641_select_chan(struct i2c_mux_core *muxc, u32 chan)
> +{
> +       struct pca9641 *data = i2c_mux_priv(muxc);
> +       struct i2c_client *client = data->client;
> +       int ret;
> +       unsigned long timeout = jiffies + ARB2_TIMEOUT;
> +               /* give up after this time */
> +
> +       data->arb_timeout = jiffies + ARB_TIMEOUT;
> +               /* force bus ownership after this time */
> +
> +       do {
> +               ret = pca9641_arbitrate(client);
> +               if (ret)
> +                       return ret < 0 ? ret : 0;
> +
> +               if (data->select_timeout == SELECT_DELAY_SHORT)
> +                       udelay(data->select_timeout);
> +               else
> +                       msleep(data->select_timeout / 1000);
> +       } while (time_is_after_eq_jiffies(timeout));
> +
> +       return -ETIMEDOUT;
> +}
> +
> +static int pca9641_release_chan(struct i2c_mux_core *muxc, u32 chan)
> +{
> +       struct pca9641 *data = i2c_mux_priv(muxc);
> +       struct i2c_client *client = data->client;
> +
> +       pca9641_release_bus(client);
> +       return 0;
> +}
> +
> +/*
> + * I2C init/probing/exit functions
> + */
> +static int pca9641_probe(struct i2c_client *client,
> +                        const struct i2c_device_id *id)
> +{
> +       struct i2c_adapter *adap = client->adapter;
> +       struct pca954x_platform_data *pdata = dev_get_platdata(&client->dev);
> +       struct i2c_mux_core *muxc;
> +       struct pca9641 *data;
> +       int force;
> +       int ret;
> +
> +       if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE_DATA))
> +               return -ENODEV;
> +
> +       /*
> +        * I2C accesses are unprotected here.
> +        * We have to lock the adapter before releasing the bus.
> +        */
> +       i2c_lock_adapter(adap);
> +       pca9641_release_bus(client);
> +       i2c_unlock_adapter(adap);
> +
> +       /* Create mux adapter */
> +
> +       force = 0;
> +       if (pdata)
> +               force = pdata->modes[0].adap_id;
> +       muxc = i2c_mux_alloc(adap, &client->dev, 8, sizeof(*data),
> +                            I2C_MUX_ARBITRATOR,
> +                            pca9641_select_chan, pca9641_release_chan);
> +       if (!muxc)
> +               return -ENOMEM;
> +
> +       data = i2c_mux_priv(muxc);
> +       data->client = client;
> +
> +       i2c_set_clientdata(client, muxc);
> +
> +
> +       ret = i2c_mux_add_adapter(muxc, force, 0, 0);
> +       if (ret) {
> +               dev_err(&client->dev, "failed to register master demultiplexer\n");
> +               return ret;
> +       }
> +
> +       dev_info(&client->dev, "registered master demultiplexer for I2C %s\n",
> +                client->name);
> +
> +       return 0;
> +}
> +
> +static int pca9641_remove(struct i2c_client *client)
> +{
> +       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
> +
> +       i2c_mux_del_adapters(muxc);
> +       return 0;
> +}
> +
> +static struct i2c_driver pca9641_driver = {
> +       .driver = {
> +                  .name = "pca9641",
> +                  .of_match_table = of_match_ptr(pca9641_of_match),
> +                  },
> +       .probe = pca9641_probe,
> +       .remove = pca9641_remove,
> +       .id_table = pca9641_id,
> +};
> +
> +module_i2c_driver(pca9641_driver);
> +
> +MODULE_AUTHOR("Ken Chen <chen.kenyy@inventec.com>");
> +MODULE_DESCRIPTION("PCA9641 I2C master demultiplexer driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.9.3
>

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

* Re: [PATCH linux dev-4.13] drivers: i2c: master arbiter: create driver
  2018-03-08  3:02 ` Joel Stanley
@ 2018-03-08  5:06   ` ChenKenYY 陳永營 TAO
  2018-03-08  5:34     ` Joel Stanley
  0 siblings, 1 reply; 6+ messages in thread
From: ChenKenYY 陳永營 TAO @ 2018-03-08  5:06 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist

Hi Joel,

Sure, I can take it, but I need some instructions on the next action.

Thanks,
Ken

2018-03-08 11:02 GMT+08:00 Joel Stanley <joel@jms.id.au>:
> Hello Ken,
>
> On Fri, Feb 23, 2018 at 3:52 PM, Ken Chen <chen.kenyy@inventec.com> wrote:
>> Initial PCA9641 2 chennel I2C bus master arbiter
>>
>> Signed-off-by: Ken Chen <chen.kenyy@inventec.com>
>
> The code here looks good. I did some searching, and found that someone
> submitted a driver for this part about a year ago:
>
>  https://patchwork.ozlabs.org/patch/726491/
>
> Unfortunately the submitter did not send another version after it was
> reviewed. I suggest we take up the patch that was submitted, make the
> changes suggested, and submit it upstream. Are you able to take on
> that work?
>
> Cheers,
>
> Joel
>
>> ---
>>  drivers/i2c/muxes/Kconfig           |   9 +
>>  drivers/i2c/muxes/Makefile          |   1 +
>>  drivers/i2c/muxes/i2c-mux-pca9641.c | 372 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 382 insertions(+)
>>  create mode 100644 drivers/i2c/muxes/i2c-mux-pca9641.c
>>
>> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
>> index 1712132..1cd1ad3 100644
>> --- a/drivers/i2c/muxes/Kconfig
>> +++ b/drivers/i2c/muxes/Kconfig
>> @@ -73,6 +73,15 @@ config I2C_MUX_PCA954x
>>           This driver can also be built as a module.  If so, the module
>>           will be called i2c-mux-pca954x.
>>
>> +config I2C_MUX_PCA9641
>> +       tristate "NXP PCA9641 I2C Master demultiplexer"
>> +       help
>> +         If you say yes here you get support for the NXP PCA9641
>> +         I2C Master demultiplexer with an arbiter function.
>> +
>> +         This driver can also be built as a module.  If so, the module
>> +         will be called i2c-mux-pca9641.
>> +
>>  config I2C_MUX_PINCTRL
>>         tristate "pinctrl-based I2C multiplexer"
>>         depends on PINCTRL
>> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
>> index 4a67d31..f95d5f5 100644
>> --- a/drivers/i2c/muxes/Makefile
>> +++ b/drivers/i2c/muxes/Makefile
>> @@ -11,6 +11,7 @@ obj-$(CONFIG_I2C_MUX_LTC4306) += i2c-mux-ltc4306.o
>>  obj-$(CONFIG_I2C_MUX_MLXCPLD)  += i2c-mux-mlxcpld.o
>>  obj-$(CONFIG_I2C_MUX_PCA9541)  += i2c-mux-pca9541.o
>>  obj-$(CONFIG_I2C_MUX_PCA954x)  += i2c-mux-pca954x.o
>> +obj-$(CONFIG_I2C_MUX_PCA9641)  += i2c-mux-pca9641.o
>>  obj-$(CONFIG_I2C_MUX_PINCTRL)  += i2c-mux-pinctrl.o
>>  obj-$(CONFIG_I2C_MUX_REG)      += i2c-mux-reg.o
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-pca9641.c b/drivers/i2c/muxes/i2c-mux-pca9641.c
>> new file mode 100644
>> index 0000000..ca7b816
>> --- /dev/null
>> +++ b/drivers/i2c/muxes/i2c-mux-pca9641.c
>> @@ -0,0 +1,372 @@
>> +/*
>> + * I2C demultiplexer driver for PCA9641 bus master demultiplexer
>> + *
>> + * Copyright (c) 2010 Ericsson AB.
>> + *
>> + * Author: Ken Chen <chen.kenyy@inventec.com>
>> + *
>> + * Derived from:
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/delay.h>
>> +#include <linux/slab.h>
>> +#include <linux/device.h>
>> +#include <linux/i2c.h>
>> +#include <linux/i2c-mux.h>
>> +
>> +#include <linux/i2c/pca954x.h>
>> +
>> +/*
>> + * The PCA9641 is two I2C bus masters demultiplexer. It supports two I2C masters
>> + * connected to a single slave bus.
>> + *
>> + * It is designed for high reliability dual master I2C bus applications where
>> + * correct system operation is required, even when two I2C master issue their
>> + * commands at the same time. The arbiter will select a winner and let it work
>> + * uninterrupted, and the losing master will take control of the I2C bus after
>> + * the winnter has finished. The arbiter also allows for queued requests where
>> + * a master requests the downstream bus while the other master has control.
>> + *
>> + */
>> +
>> +#define PCA9641_ID             0x01
>> +#define PCA9641_ID_MAGIC       0x38
>> +
>> +#define PCA9641_CONTROL                0x01
>> +#define PCA9641_STATUS         0x02
>> +#define PCA9641_TIME           0x03
>> +
>> +#define PCA9641_CTL_LOCK_REQ            (1 << 0)
>> +#define PCA9641_CTL_LOCK_GRANT          (1 << 1)
>> +#define PCA9641_CTL_BUS_CONNECT         (1 << 2)
>> +#define PCA9641_CTL_BUS_INIT            (1 << 3)
>> +#define PCA9641_CTL_SMBUS_SWRST         (1 << 4)
>> +#define PCA9641_CTL_IDLE_TIMER_DIS      (1 << 5)
>> +#define PCA9641_CTL_SMBUS_DIS           (1 << 6)
>> +#define PCA9641_CTL_PRIORITY            (1 << 7)
>> +
>> +#define PCA9641_STS_OTHER_LOCK          (1 << 0)
>> +#define PCA9641_STS_BUS_INIT_FAIL       (1 << 1)
>> +#define PCA9641_STS_BUS_HUNG            (1 << 2)
>> +#define PCA9641_STS_MBOX_EMPTY          (1 << 3)
>> +#define PCA9641_STS_MBOX_FULL           (1 << 4)
>> +#define PCA9641_STS_TEST_INT            (1 << 5)
>> +#define PCA9641_STS_SCL_IO              (1 << 6)
>> +#define PCA9641_STS_SDA_IO              (1 << 7)
>> +
>> +#define PCA9641_RES_TIME       0x03
>> +
>> +#define other_lock(x)  ((x) & PCA9641_STS_OTHER_LOCK)
>> +#define lock_grant(x)  ((x) & PCA9641_CTL_LOCK_GRANT)
>> +
>> +/* arbitration timeouts, in jiffies */
>> +#define ARB_TIMEOUT    (HZ / 8)        /* 125 ms until forcing bus ownership */
>> +#define ARB2_TIMEOUT   (HZ / 4)        /* 250 ms until acquisition failure */
>> +
>> +/* arbitration retry delays, in us */
>> +#define SELECT_DELAY_SHORT     50
>> +#define SELECT_DELAY_LONG      1000
>> +
>> +struct pca9641 {
>> +       struct i2c_client *client;
>> +       unsigned long select_timeout;
>> +       unsigned long arb_timeout;
>> +};
>> +
>> +static const struct i2c_device_id pca9641_id[] = {
>> +       {"pca9641", 0},
>> +       {}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(i2c, pca9641_id);
>> +
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id pca9641_of_match[] = {
>> +       { .compatible = "nxp,pca9641" },
>> +       {}
>> +};
>> +#endif
>> +
>> +/*
>> + * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
>> + * as they will try to lock the adapter a second time.
>> + */
>> +static int pca9641_reg_write(struct i2c_client *client, u8 command, u8 val)
>> +{
>> +       struct i2c_adapter *adap = client->adapter;
>> +       int ret;
>> +
>> +       if (adap->algo->master_xfer) {
>> +               struct i2c_msg msg;
>> +               char buf[2];
>> +
>> +               msg.addr = client->addr;
>> +               msg.flags = 0;
>> +               msg.len = 2;
>> +               buf[0] = command;
>> +               buf[1] = val;
>> +               msg.buf = buf;
>> +               ret = __i2c_transfer(adap, &msg, 1);
>> +       } else {
>> +               union i2c_smbus_data data;
>> +
>> +               data.byte = val;
>> +               ret = adap->algo->smbus_xfer(adap, client->addr,
>> +                                            client->flags,
>> +                                            I2C_SMBUS_WRITE,
>> +                                            command,
>> +                                            I2C_SMBUS_BYTE_DATA, &data);
>> +       }
>> +
>> +       return ret;
>> +}
>> +
>> +/*
>> + * Read from chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
>> + * as they will try to lock adapter a second time.
>> + */
>> +static int pca9641_reg_read(struct i2c_client *client, u8 command)
>> +{
>> +       struct i2c_adapter *adap = client->adapter;
>> +       int ret;
>> +       u8 val;
>> +
>> +       if (adap->algo->master_xfer) {
>> +               struct i2c_msg msg[2] = {
>> +                       {
>> +                               .addr = client->addr,
>> +                               .flags = 0,
>> +                               .len = 1,
>> +                               .buf = &command
>> +                       },
>> +                       {
>> +                               .addr = client->addr,
>> +                               .flags = I2C_M_RD,
>> +                               .len = 1,
>> +                               .buf = &val
>> +                       }
>> +               };
>> +               ret = __i2c_transfer(adap, msg, 2);
>> +               if (ret == 2)
>> +                       ret = val;
>> +               else if (ret >= 0)
>> +                       ret = -EIO;
>> +       } else {
>> +               union i2c_smbus_data data;
>> +
>> +               ret = adap->algo->smbus_xfer(adap, client->addr,
>> +                                            client->flags,
>> +                                            I2C_SMBUS_READ,
>> +                                            command,
>> +                                            I2C_SMBUS_BYTE_DATA, &data);
>> +               if (!ret)
>> +                       ret = data.byte;
>> +       }
>> +       return ret;
>> +}
>> +
>> +/*
>> + * Arbitration management functions
>> + */
>> +static void pca9641_release_bus(struct i2c_client *client)
>> +{
>> +       int reg;
>> +
>> +       pca9641_reg_write(client, PCA9641_CONTROL, 0);
>> +}
>> +
>> +/*
>> + * Channel arbitration
>> + *
>> + * Return values:
>> + *  <0: error
>> + *  0 : bus not acquired
>> + *  1 : bus acquired
>> + */
>> +static int pca9641_arbitrate(struct i2c_client *client)
>> +{
>> +       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>> +       int reg_ctl, reg_sts;
>> +
>> +       reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
>> +       if (reg_ctl < 0)
>> +               return reg_ctl;
>> +       reg_sts = pca9641_reg_read(client, PCA9641_STATUS);
>> +
>> +       if (!other_lock(reg_sts) && !lock_grant(reg_ctl)) {
>> +               /*
>> +                * Bus is off. Request ownership or turn it on unless
>> +                * other master requested ownership.
>> +                */
>> +                reg_ctl |= PCA9641_CTL_LOCK_REQ;
>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>> +
>> +                udelay(100);
>> +
>> +               reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
>> +
>> +                if (lock_grant(reg_ctl)) {
>> +                        /*
>> +                         * Other master did not request ownership,
>> +                         * or arbitration timeout expired. Take the bus.
>> +                         */
>> +                        reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
>> +                        pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>> +                        data->select_timeout = SELECT_DELAY_SHORT;
>> +
>> +                        return 1;
>> +                } else {
>> +                        /*
>> +                         * Other master requested ownership.
>> +                         * Set extra long timeout to give it time to acquire it.
>> +                         */
>> +                        data->select_timeout = SELECT_DELAY_LONG * 2;
>> +                }
>> +       } else if (lock_grant(reg_ctl)) {
>> +                /*
>> +                 * Bus is on, and we own it. We are done with acquisition.
>> +                 */
>> +                reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>> +
>> +                return 1;
>> +       } else if (other_lock(reg_sts)) {
>> +                /*
>> +                 * Other master owns the bus.
>> +                 * If arbitration timeout has expired, force ownership.
>> +                 * Otherwise request it.
>> +                 */
>> +                data->select_timeout = SELECT_DELAY_LONG;
>> +                reg_ctl |= PCA9641_CTL_LOCK_REQ;
>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>> +
>> +                /*
>> +                 * if (time_is_before_eq_jiffies(data->arb_timeout)) {
>> +                 * TODO:Time is up, take the bus and reset it.
>> +                 *
>> +                 *} else {
>> +                 * TODO: Request bus ownership if needed
>> +                 *
>> +                 *}
>> +                 */
>> +        }
>> +       return 0;
>> +}
>> +
>> +static int pca9641_select_chan(struct i2c_mux_core *muxc, u32 chan)
>> +{
>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>> +       struct i2c_client *client = data->client;
>> +       int ret;
>> +       unsigned long timeout = jiffies + ARB2_TIMEOUT;
>> +               /* give up after this time */
>> +
>> +       data->arb_timeout = jiffies + ARB_TIMEOUT;
>> +               /* force bus ownership after this time */
>> +
>> +       do {
>> +               ret = pca9641_arbitrate(client);
>> +               if (ret)
>> +                       return ret < 0 ? ret : 0;
>> +
>> +               if (data->select_timeout == SELECT_DELAY_SHORT)
>> +                       udelay(data->select_timeout);
>> +               else
>> +                       msleep(data->select_timeout / 1000);
>> +       } while (time_is_after_eq_jiffies(timeout));
>> +
>> +       return -ETIMEDOUT;
>> +}
>> +
>> +static int pca9641_release_chan(struct i2c_mux_core *muxc, u32 chan)
>> +{
>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>> +       struct i2c_client *client = data->client;
>> +
>> +       pca9641_release_bus(client);
>> +       return 0;
>> +}
>> +
>> +/*
>> + * I2C init/probing/exit functions
>> + */
>> +static int pca9641_probe(struct i2c_client *client,
>> +                        const struct i2c_device_id *id)
>> +{
>> +       struct i2c_adapter *adap = client->adapter;
>> +       struct pca954x_platform_data *pdata = dev_get_platdata(&client->dev);
>> +       struct i2c_mux_core *muxc;
>> +       struct pca9641 *data;
>> +       int force;
>> +       int ret;
>> +
>> +       if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE_DATA))
>> +               return -ENODEV;
>> +
>> +       /*
>> +        * I2C accesses are unprotected here.
>> +        * We have to lock the adapter before releasing the bus.
>> +        */
>> +       i2c_lock_adapter(adap);
>> +       pca9641_release_bus(client);
>> +       i2c_unlock_adapter(adap);
>> +
>> +       /* Create mux adapter */
>> +
>> +       force = 0;
>> +       if (pdata)
>> +               force = pdata->modes[0].adap_id;
>> +       muxc = i2c_mux_alloc(adap, &client->dev, 8, sizeof(*data),
>> +                            I2C_MUX_ARBITRATOR,
>> +                            pca9641_select_chan, pca9641_release_chan);
>> +       if (!muxc)
>> +               return -ENOMEM;
>> +
>> +       data = i2c_mux_priv(muxc);
>> +       data->client = client;
>> +
>> +       i2c_set_clientdata(client, muxc);
>> +
>> +
>> +       ret = i2c_mux_add_adapter(muxc, force, 0, 0);
>> +       if (ret) {
>> +               dev_err(&client->dev, "failed to register master demultiplexer\n");
>> +               return ret;
>> +       }
>> +
>> +       dev_info(&client->dev, "registered master demultiplexer for I2C %s\n",
>> +                client->name);
>> +
>> +       return 0;
>> +}
>> +
>> +static int pca9641_remove(struct i2c_client *client)
>> +{
>> +       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>> +
>> +       i2c_mux_del_adapters(muxc);
>> +       return 0;
>> +}
>> +
>> +static struct i2c_driver pca9641_driver = {
>> +       .driver = {
>> +                  .name = "pca9641",
>> +                  .of_match_table = of_match_ptr(pca9641_of_match),
>> +                  },
>> +       .probe = pca9641_probe,
>> +       .remove = pca9641_remove,
>> +       .id_table = pca9641_id,
>> +};
>> +
>> +module_i2c_driver(pca9641_driver);
>> +
>> +MODULE_AUTHOR("Ken Chen <chen.kenyy@inventec.com>");
>> +MODULE_DESCRIPTION("PCA9641 I2C master demultiplexer driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.9.3
>>

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

* Re: [PATCH linux dev-4.13] drivers: i2c: master arbiter: create driver
  2018-03-08  5:06   ` ChenKenYY 陳永營 TAO
@ 2018-03-08  5:34     ` Joel Stanley
  2018-03-12  6:40       ` ChenKenYY 陳永營 TAO
  0 siblings, 1 reply; 6+ messages in thread
From: Joel Stanley @ 2018-03-08  5:34 UTC (permalink / raw)
  To: ChenKenYY 陳永營 TAO; +Cc: OpenBMC Maillist

On Thu, Mar 8, 2018 at 3:36 PM, ChenKenYY 陳永營 TAO
<chen.kenyy@inventec.com> wrote:
> Hi Joel,
>
> Sure, I can take it, but I need some instructions on the next action.

Okay. I suggest you download the patch from patchwork and apply it to
the latest kernel release, 4.16-rc4:

https://patchwork.ozlabs.org/patch/726491/

You can download it by clicking the 'mbox' link at the top right of the page.

Now you will have a file called
RFC-i2c-mux-Driver-for-PCA9641-I2C-Master-Arbiter.patch.

$ git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
$ cd linux
$ git checkout -b pca9641-mux v4.16-rc4
$ git am ~/Downloads/RFC-i2c-mux-Driver-for-PCA9641-I2C-Master-Arbiter.patch


From there, take a look at the comments on the two reviews that Vidya had:

 https://patchwork.ozlabs.org/patch/726491/#1580479
 https://patchwork.ozlabs.org/patch/726491/#1582916

Fix those issues, and add your signed off by:

$ git add drivers/i2c/muxes/i2c-mux-pca9541.c
$ git commit --amend -s

Then test this driver on your board. To do this, modify the device
tree. You should be able to do this by modifying the ast2500-evb
device tree, and boot that on your system.

Once this is working, use the checkpatch.pl tool to check for common mistakes:

$ ./scripts/checkpatch.pl drivers/i2c/muxes/i2c-mux-pca9541.c

Then use get_maintainer.pl to get a list of people you should send the patch to:

$ ./scripts/get_maintainer.pl -f drivers/i2c/muxes/i2c-mux-pca9541.c
Guenter Roeck <linux@roeck-us.net> (maintainer:PCA9541 I2C BUS MASTER
SELECTOR DRIVER)
Peter Rosin <peda@axentia.se> (maintainer:I2C MUXES)
Wolfram Sang <wsa@the-dreams.de> (maintainer:I2C SUBSYSTEM)
linux-i2c@vger.kernel.org (open list:PCA9541 I2C BUS MASTER SELECTOR DRIVER)
linux-kernel@vger.kernel.org (open list)

In addition, please add me to this list so I can help. Then send the
patch out, with a comment in the commit message (below the ---) that
you have continued this work from the previous author, and have tested
it on your system.

Cheers,

Joel

>
> Thanks,
> Ken
>
> 2018-03-08 11:02 GMT+08:00 Joel Stanley <joel@jms.id.au>:
>> Hello Ken,
>>
>> On Fri, Feb 23, 2018 at 3:52 PM, Ken Chen <chen.kenyy@inventec.com> wrote:
>>> Initial PCA9641 2 chennel I2C bus master arbiter
>>>
>>> Signed-off-by: Ken Chen <chen.kenyy@inventec.com>
>>
>> The code here looks good. I did some searching, and found that someone
>> submitted a driver for this part about a year ago:
>>
>>  https://patchwork.ozlabs.org/patch/726491/
>>
>> Unfortunately the submitter did not send another version after it was
>> reviewed. I suggest we take up the patch that was submitted, make the
>> changes suggested, and submit it upstream. Are you able to take on
>> that work?
>>
>> Cheers,
>>
>> Joel
>>
>>> ---
>>>  drivers/i2c/muxes/Kconfig           |   9 +
>>>  drivers/i2c/muxes/Makefile          |   1 +
>>>  drivers/i2c/muxes/i2c-mux-pca9641.c | 372 ++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 382 insertions(+)
>>>  create mode 100644 drivers/i2c/muxes/i2c-mux-pca9641.c
>>>
>>> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
>>> index 1712132..1cd1ad3 100644
>>> --- a/drivers/i2c/muxes/Kconfig
>>> +++ b/drivers/i2c/muxes/Kconfig
>>> @@ -73,6 +73,15 @@ config I2C_MUX_PCA954x
>>>           This driver can also be built as a module.  If so, the module
>>>           will be called i2c-mux-pca954x.
>>>
>>> +config I2C_MUX_PCA9641
>>> +       tristate "NXP PCA9641 I2C Master demultiplexer"
>>> +       help
>>> +         If you say yes here you get support for the NXP PCA9641
>>> +         I2C Master demultiplexer with an arbiter function.
>>> +
>>> +         This driver can also be built as a module.  If so, the module
>>> +         will be called i2c-mux-pca9641.
>>> +
>>>  config I2C_MUX_PINCTRL
>>>         tristate "pinctrl-based I2C multiplexer"
>>>         depends on PINCTRL
>>> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
>>> index 4a67d31..f95d5f5 100644
>>> --- a/drivers/i2c/muxes/Makefile
>>> +++ b/drivers/i2c/muxes/Makefile
>>> @@ -11,6 +11,7 @@ obj-$(CONFIG_I2C_MUX_LTC4306) += i2c-mux-ltc4306.o
>>>  obj-$(CONFIG_I2C_MUX_MLXCPLD)  += i2c-mux-mlxcpld.o
>>>  obj-$(CONFIG_I2C_MUX_PCA9541)  += i2c-mux-pca9541.o
>>>  obj-$(CONFIG_I2C_MUX_PCA954x)  += i2c-mux-pca954x.o
>>> +obj-$(CONFIG_I2C_MUX_PCA9641)  += i2c-mux-pca9641.o
>>>  obj-$(CONFIG_I2C_MUX_PINCTRL)  += i2c-mux-pinctrl.o
>>>  obj-$(CONFIG_I2C_MUX_REG)      += i2c-mux-reg.o
>>>
>>> diff --git a/drivers/i2c/muxes/i2c-mux-pca9641.c b/drivers/i2c/muxes/i2c-mux-pca9641.c
>>> new file mode 100644
>>> index 0000000..ca7b816
>>> --- /dev/null
>>> +++ b/drivers/i2c/muxes/i2c-mux-pca9641.c
>>> @@ -0,0 +1,372 @@
>>> +/*
>>> + * I2C demultiplexer driver for PCA9641 bus master demultiplexer
>>> + *
>>> + * Copyright (c) 2010 Ericsson AB.
>>> + *
>>> + * Author: Ken Chen <chen.kenyy@inventec.com>
>>> + *
>>> + * Derived from:
>>> + *
>>> + * This file is licensed under the terms of the GNU General Public
>>> + * License version 2. This program is licensed "as is" without any
>>> + * warranty of any kind, whether express or implied.
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/jiffies.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/device.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/i2c-mux.h>
>>> +
>>> +#include <linux/i2c/pca954x.h>
>>> +
>>> +/*
>>> + * The PCA9641 is two I2C bus masters demultiplexer. It supports two I2C masters
>>> + * connected to a single slave bus.
>>> + *
>>> + * It is designed for high reliability dual master I2C bus applications where
>>> + * correct system operation is required, even when two I2C master issue their
>>> + * commands at the same time. The arbiter will select a winner and let it work
>>> + * uninterrupted, and the losing master will take control of the I2C bus after
>>> + * the winnter has finished. The arbiter also allows for queued requests where
>>> + * a master requests the downstream bus while the other master has control.
>>> + *
>>> + */
>>> +
>>> +#define PCA9641_ID             0x01
>>> +#define PCA9641_ID_MAGIC       0x38
>>> +
>>> +#define PCA9641_CONTROL                0x01
>>> +#define PCA9641_STATUS         0x02
>>> +#define PCA9641_TIME           0x03
>>> +
>>> +#define PCA9641_CTL_LOCK_REQ            (1 << 0)
>>> +#define PCA9641_CTL_LOCK_GRANT          (1 << 1)
>>> +#define PCA9641_CTL_BUS_CONNECT         (1 << 2)
>>> +#define PCA9641_CTL_BUS_INIT            (1 << 3)
>>> +#define PCA9641_CTL_SMBUS_SWRST         (1 << 4)
>>> +#define PCA9641_CTL_IDLE_TIMER_DIS      (1 << 5)
>>> +#define PCA9641_CTL_SMBUS_DIS           (1 << 6)
>>> +#define PCA9641_CTL_PRIORITY            (1 << 7)
>>> +
>>> +#define PCA9641_STS_OTHER_LOCK          (1 << 0)
>>> +#define PCA9641_STS_BUS_INIT_FAIL       (1 << 1)
>>> +#define PCA9641_STS_BUS_HUNG            (1 << 2)
>>> +#define PCA9641_STS_MBOX_EMPTY          (1 << 3)
>>> +#define PCA9641_STS_MBOX_FULL           (1 << 4)
>>> +#define PCA9641_STS_TEST_INT            (1 << 5)
>>> +#define PCA9641_STS_SCL_IO              (1 << 6)
>>> +#define PCA9641_STS_SDA_IO              (1 << 7)
>>> +
>>> +#define PCA9641_RES_TIME       0x03
>>> +
>>> +#define other_lock(x)  ((x) & PCA9641_STS_OTHER_LOCK)
>>> +#define lock_grant(x)  ((x) & PCA9641_CTL_LOCK_GRANT)
>>> +
>>> +/* arbitration timeouts, in jiffies */
>>> +#define ARB_TIMEOUT    (HZ / 8)        /* 125 ms until forcing bus ownership */
>>> +#define ARB2_TIMEOUT   (HZ / 4)        /* 250 ms until acquisition failure */
>>> +
>>> +/* arbitration retry delays, in us */
>>> +#define SELECT_DELAY_SHORT     50
>>> +#define SELECT_DELAY_LONG      1000
>>> +
>>> +struct pca9641 {
>>> +       struct i2c_client *client;
>>> +       unsigned long select_timeout;
>>> +       unsigned long arb_timeout;
>>> +};
>>> +
>>> +static const struct i2c_device_id pca9641_id[] = {
>>> +       {"pca9641", 0},
>>> +       {}
>>> +};
>>> +
>>> +MODULE_DEVICE_TABLE(i2c, pca9641_id);
>>> +
>>> +#ifdef CONFIG_OF
>>> +static const struct of_device_id pca9641_of_match[] = {
>>> +       { .compatible = "nxp,pca9641" },
>>> +       {}
>>> +};
>>> +#endif
>>> +
>>> +/*
>>> + * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
>>> + * as they will try to lock the adapter a second time.
>>> + */
>>> +static int pca9641_reg_write(struct i2c_client *client, u8 command, u8 val)
>>> +{
>>> +       struct i2c_adapter *adap = client->adapter;
>>> +       int ret;
>>> +
>>> +       if (adap->algo->master_xfer) {
>>> +               struct i2c_msg msg;
>>> +               char buf[2];
>>> +
>>> +               msg.addr = client->addr;
>>> +               msg.flags = 0;
>>> +               msg.len = 2;
>>> +               buf[0] = command;
>>> +               buf[1] = val;
>>> +               msg.buf = buf;
>>> +               ret = __i2c_transfer(adap, &msg, 1);
>>> +       } else {
>>> +               union i2c_smbus_data data;
>>> +
>>> +               data.byte = val;
>>> +               ret = adap->algo->smbus_xfer(adap, client->addr,
>>> +                                            client->flags,
>>> +                                            I2C_SMBUS_WRITE,
>>> +                                            command,
>>> +                                            I2C_SMBUS_BYTE_DATA, &data);
>>> +       }
>>> +
>>> +       return ret;
>>> +}
>>> +
>>> +/*
>>> + * Read from chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
>>> + * as they will try to lock adapter a second time.
>>> + */
>>> +static int pca9641_reg_read(struct i2c_client *client, u8 command)
>>> +{
>>> +       struct i2c_adapter *adap = client->adapter;
>>> +       int ret;
>>> +       u8 val;
>>> +
>>> +       if (adap->algo->master_xfer) {
>>> +               struct i2c_msg msg[2] = {
>>> +                       {
>>> +                               .addr = client->addr,
>>> +                               .flags = 0,
>>> +                               .len = 1,
>>> +                               .buf = &command
>>> +                       },
>>> +                       {
>>> +                               .addr = client->addr,
>>> +                               .flags = I2C_M_RD,
>>> +                               .len = 1,
>>> +                               .buf = &val
>>> +                       }
>>> +               };
>>> +               ret = __i2c_transfer(adap, msg, 2);
>>> +               if (ret == 2)
>>> +                       ret = val;
>>> +               else if (ret >= 0)
>>> +                       ret = -EIO;
>>> +       } else {
>>> +               union i2c_smbus_data data;
>>> +
>>> +               ret = adap->algo->smbus_xfer(adap, client->addr,
>>> +                                            client->flags,
>>> +                                            I2C_SMBUS_READ,
>>> +                                            command,
>>> +                                            I2C_SMBUS_BYTE_DATA, &data);
>>> +               if (!ret)
>>> +                       ret = data.byte;
>>> +       }
>>> +       return ret;
>>> +}
>>> +
>>> +/*
>>> + * Arbitration management functions
>>> + */
>>> +static void pca9641_release_bus(struct i2c_client *client)
>>> +{
>>> +       int reg;
>>> +
>>> +       pca9641_reg_write(client, PCA9641_CONTROL, 0);
>>> +}
>>> +
>>> +/*
>>> + * Channel arbitration
>>> + *
>>> + * Return values:
>>> + *  <0: error
>>> + *  0 : bus not acquired
>>> + *  1 : bus acquired
>>> + */
>>> +static int pca9641_arbitrate(struct i2c_client *client)
>>> +{
>>> +       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>>> +       int reg_ctl, reg_sts;
>>> +
>>> +       reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
>>> +       if (reg_ctl < 0)
>>> +               return reg_ctl;
>>> +       reg_sts = pca9641_reg_read(client, PCA9641_STATUS);
>>> +
>>> +       if (!other_lock(reg_sts) && !lock_grant(reg_ctl)) {
>>> +               /*
>>> +                * Bus is off. Request ownership or turn it on unless
>>> +                * other master requested ownership.
>>> +                */
>>> +                reg_ctl |= PCA9641_CTL_LOCK_REQ;
>>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>> +
>>> +                udelay(100);
>>> +
>>> +               reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
>>> +
>>> +                if (lock_grant(reg_ctl)) {
>>> +                        /*
>>> +                         * Other master did not request ownership,
>>> +                         * or arbitration timeout expired. Take the bus.
>>> +                         */
>>> +                        reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
>>> +                        pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>> +                        data->select_timeout = SELECT_DELAY_SHORT;
>>> +
>>> +                        return 1;
>>> +                } else {
>>> +                        /*
>>> +                         * Other master requested ownership.
>>> +                         * Set extra long timeout to give it time to acquire it.
>>> +                         */
>>> +                        data->select_timeout = SELECT_DELAY_LONG * 2;
>>> +                }
>>> +       } else if (lock_grant(reg_ctl)) {
>>> +                /*
>>> +                 * Bus is on, and we own it. We are done with acquisition.
>>> +                 */
>>> +                reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
>>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>> +
>>> +                return 1;
>>> +       } else if (other_lock(reg_sts)) {
>>> +                /*
>>> +                 * Other master owns the bus.
>>> +                 * If arbitration timeout has expired, force ownership.
>>> +                 * Otherwise request it.
>>> +                 */
>>> +                data->select_timeout = SELECT_DELAY_LONG;
>>> +                reg_ctl |= PCA9641_CTL_LOCK_REQ;
>>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>> +
>>> +                /*
>>> +                 * if (time_is_before_eq_jiffies(data->arb_timeout)) {
>>> +                 * TODO:Time is up, take the bus and reset it.
>>> +                 *
>>> +                 *} else {
>>> +                 * TODO: Request bus ownership if needed
>>> +                 *
>>> +                 *}
>>> +                 */
>>> +        }
>>> +       return 0;
>>> +}
>>> +
>>> +static int pca9641_select_chan(struct i2c_mux_core *muxc, u32 chan)
>>> +{
>>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>>> +       struct i2c_client *client = data->client;
>>> +       int ret;
>>> +       unsigned long timeout = jiffies + ARB2_TIMEOUT;
>>> +               /* give up after this time */
>>> +
>>> +       data->arb_timeout = jiffies + ARB_TIMEOUT;
>>> +               /* force bus ownership after this time */
>>> +
>>> +       do {
>>> +               ret = pca9641_arbitrate(client);
>>> +               if (ret)
>>> +                       return ret < 0 ? ret : 0;
>>> +
>>> +               if (data->select_timeout == SELECT_DELAY_SHORT)
>>> +                       udelay(data->select_timeout);
>>> +               else
>>> +                       msleep(data->select_timeout / 1000);
>>> +       } while (time_is_after_eq_jiffies(timeout));
>>> +
>>> +       return -ETIMEDOUT;
>>> +}
>>> +
>>> +static int pca9641_release_chan(struct i2c_mux_core *muxc, u32 chan)
>>> +{
>>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>>> +       struct i2c_client *client = data->client;
>>> +
>>> +       pca9641_release_bus(client);
>>> +       return 0;
>>> +}
>>> +
>>> +/*
>>> + * I2C init/probing/exit functions
>>> + */
>>> +static int pca9641_probe(struct i2c_client *client,
>>> +                        const struct i2c_device_id *id)
>>> +{
>>> +       struct i2c_adapter *adap = client->adapter;
>>> +       struct pca954x_platform_data *pdata = dev_get_platdata(&client->dev);
>>> +       struct i2c_mux_core *muxc;
>>> +       struct pca9641 *data;
>>> +       int force;
>>> +       int ret;
>>> +
>>> +       if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE_DATA))
>>> +               return -ENODEV;
>>> +
>>> +       /*
>>> +        * I2C accesses are unprotected here.
>>> +        * We have to lock the adapter before releasing the bus.
>>> +        */
>>> +       i2c_lock_adapter(adap);
>>> +       pca9641_release_bus(client);
>>> +       i2c_unlock_adapter(adap);
>>> +
>>> +       /* Create mux adapter */
>>> +
>>> +       force = 0;
>>> +       if (pdata)
>>> +               force = pdata->modes[0].adap_id;
>>> +       muxc = i2c_mux_alloc(adap, &client->dev, 8, sizeof(*data),
>>> +                            I2C_MUX_ARBITRATOR,
>>> +                            pca9641_select_chan, pca9641_release_chan);
>>> +       if (!muxc)
>>> +               return -ENOMEM;
>>> +
>>> +       data = i2c_mux_priv(muxc);
>>> +       data->client = client;
>>> +
>>> +       i2c_set_clientdata(client, muxc);
>>> +
>>> +
>>> +       ret = i2c_mux_add_adapter(muxc, force, 0, 0);
>>> +       if (ret) {
>>> +               dev_err(&client->dev, "failed to register master demultiplexer\n");
>>> +               return ret;
>>> +       }
>>> +
>>> +       dev_info(&client->dev, "registered master demultiplexer for I2C %s\n",
>>> +                client->name);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int pca9641_remove(struct i2c_client *client)
>>> +{
>>> +       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>>> +
>>> +       i2c_mux_del_adapters(muxc);
>>> +       return 0;
>>> +}
>>> +
>>> +static struct i2c_driver pca9641_driver = {
>>> +       .driver = {
>>> +                  .name = "pca9641",
>>> +                  .of_match_table = of_match_ptr(pca9641_of_match),
>>> +                  },
>>> +       .probe = pca9641_probe,
>>> +       .remove = pca9641_remove,
>>> +       .id_table = pca9641_id,
>>> +};
>>> +
>>> +module_i2c_driver(pca9641_driver);
>>> +
>>> +MODULE_AUTHOR("Ken Chen <chen.kenyy@inventec.com>");
>>> +MODULE_DESCRIPTION("PCA9641 I2C master demultiplexer driver");
>>> +MODULE_LICENSE("GPL v2");
>>> --
>>> 2.9.3
>>>

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

* Re: [PATCH linux dev-4.13] drivers: i2c: master arbiter: create driver
  2018-03-08  5:34     ` Joel Stanley
@ 2018-03-12  6:40       ` ChenKenYY 陳永營 TAO
  2018-03-13  3:52         ` Joel Stanley
  0 siblings, 1 reply; 6+ messages in thread
From: ChenKenYY 陳永營 TAO @ 2018-03-12  6:40 UTC (permalink / raw)
  To: Joel Stanley; +Cc: OpenBMC Maillist

Hi Joel,

Could I use an independent .c file(PCA9641) to upstream this driver?
Because the reference code has probe issue and sperate the difference
device behavior between PCA9541 and PCA9641 by function.
So I think It would be better using an independent .c file.


Best Regards,
Ken

2018-03-08 13:34 GMT+08:00 Joel Stanley <joel@jms.id.au>:
> On Thu, Mar 8, 2018 at 3:36 PM, ChenKenYY 陳永營 TAO
> <chen.kenyy@inventec.com> wrote:
>> Hi Joel,
>>
>> Sure, I can take it, but I need some instructions on the next action.
>
> Okay. I suggest you download the patch from patchwork and apply it to
> the latest kernel release, 4.16-rc4:
>
> https://patchwork.ozlabs.org/patch/726491/
>
> You can download it by clicking the 'mbox' link at the top right of the page.
>
> Now you will have a file called
> RFC-i2c-mux-Driver-for-PCA9641-I2C-Master-Arbiter.patch.
>
> $ git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> $ cd linux
> $ git checkout -b pca9641-mux v4.16-rc4
> $ git am ~/Downloads/RFC-i2c-mux-Driver-for-PCA9641-I2C-Master-Arbiter.patch
>
>
> From there, take a look at the comments on the two reviews that Vidya had:
>
>  https://patchwork.ozlabs.org/patch/726491/#1580479
>  https://patchwork.ozlabs.org/patch/726491/#1582916
>
> Fix those issues, and add your signed off by:
>
> $ git add drivers/i2c/muxes/i2c-mux-pca9541.c
> $ git commit --amend -s
>
> Then test this driver on your board. To do this, modify the device
> tree. You should be able to do this by modifying the ast2500-evb
> device tree, and boot that on your system.
>
> Once this is working, use the checkpatch.pl tool to check for common mistakes:
>
> $ ./scripts/checkpatch.pl drivers/i2c/muxes/i2c-mux-pca9541.c
>
> Then use get_maintainer.pl to get a list of people you should send the patch to:
>
> $ ./scripts/get_maintainer.pl -f drivers/i2c/muxes/i2c-mux-pca9541.c
> Guenter Roeck <linux@roeck-us.net> (maintainer:PCA9541 I2C BUS MASTER
> SELECTOR DRIVER)
> Peter Rosin <peda@axentia.se> (maintainer:I2C MUXES)
> Wolfram Sang <wsa@the-dreams.de> (maintainer:I2C SUBSYSTEM)
> linux-i2c@vger.kernel.org (open list:PCA9541 I2C BUS MASTER SELECTOR DRIVER)
> linux-kernel@vger.kernel.org (open list)
>
> In addition, please add me to this list so I can help. Then send the
> patch out, with a comment in the commit message (below the ---) that
> you have continued this work from the previous author, and have tested
> it on your system.
>
> Cheers,
>
> Joel
>
>>
>> Thanks,
>> Ken
>>
>> 2018-03-08 11:02 GMT+08:00 Joel Stanley <joel@jms.id.au>:
>>> Hello Ken,
>>>
>>> On Fri, Feb 23, 2018 at 3:52 PM, Ken Chen <chen.kenyy@inventec.com> wrote:
>>>> Initial PCA9641 2 chennel I2C bus master arbiter
>>>>
>>>> Signed-off-by: Ken Chen <chen.kenyy@inventec.com>
>>>
>>> The code here looks good. I did some searching, and found that someone
>>> submitted a driver for this part about a year ago:
>>>
>>>  https://patchwork.ozlabs.org/patch/726491/
>>>
>>> Unfortunately the submitter did not send another version after it was
>>> reviewed. I suggest we take up the patch that was submitted, make the
>>> changes suggested, and submit it upstream. Are you able to take on
>>> that work?
>>>
>>> Cheers,
>>>
>>> Joel
>>>
>>>> ---
>>>>  drivers/i2c/muxes/Kconfig           |   9 +
>>>>  drivers/i2c/muxes/Makefile          |   1 +
>>>>  drivers/i2c/muxes/i2c-mux-pca9641.c | 372 ++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 382 insertions(+)
>>>>  create mode 100644 drivers/i2c/muxes/i2c-mux-pca9641.c
>>>>
>>>> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
>>>> index 1712132..1cd1ad3 100644
>>>> --- a/drivers/i2c/muxes/Kconfig
>>>> +++ b/drivers/i2c/muxes/Kconfig
>>>> @@ -73,6 +73,15 @@ config I2C_MUX_PCA954x
>>>>           This driver can also be built as a module.  If so, the module
>>>>           will be called i2c-mux-pca954x.
>>>>
>>>> +config I2C_MUX_PCA9641
>>>> +       tristate "NXP PCA9641 I2C Master demultiplexer"
>>>> +       help
>>>> +         If you say yes here you get support for the NXP PCA9641
>>>> +         I2C Master demultiplexer with an arbiter function.
>>>> +
>>>> +         This driver can also be built as a module.  If so, the module
>>>> +         will be called i2c-mux-pca9641.
>>>> +
>>>>  config I2C_MUX_PINCTRL
>>>>         tristate "pinctrl-based I2C multiplexer"
>>>>         depends on PINCTRL
>>>> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
>>>> index 4a67d31..f95d5f5 100644
>>>> --- a/drivers/i2c/muxes/Makefile
>>>> +++ b/drivers/i2c/muxes/Makefile
>>>> @@ -11,6 +11,7 @@ obj-$(CONFIG_I2C_MUX_LTC4306) += i2c-mux-ltc4306.o
>>>>  obj-$(CONFIG_I2C_MUX_MLXCPLD)  += i2c-mux-mlxcpld.o
>>>>  obj-$(CONFIG_I2C_MUX_PCA9541)  += i2c-mux-pca9541.o
>>>>  obj-$(CONFIG_I2C_MUX_PCA954x)  += i2c-mux-pca954x.o
>>>> +obj-$(CONFIG_I2C_MUX_PCA9641)  += i2c-mux-pca9641.o
>>>>  obj-$(CONFIG_I2C_MUX_PINCTRL)  += i2c-mux-pinctrl.o
>>>>  obj-$(CONFIG_I2C_MUX_REG)      += i2c-mux-reg.o
>>>>
>>>> diff --git a/drivers/i2c/muxes/i2c-mux-pca9641.c b/drivers/i2c/muxes/i2c-mux-pca9641.c
>>>> new file mode 100644
>>>> index 0000000..ca7b816
>>>> --- /dev/null
>>>> +++ b/drivers/i2c/muxes/i2c-mux-pca9641.c
>>>> @@ -0,0 +1,372 @@
>>>> +/*
>>>> + * I2C demultiplexer driver for PCA9641 bus master demultiplexer
>>>> + *
>>>> + * Copyright (c) 2010 Ericsson AB.
>>>> + *
>>>> + * Author: Ken Chen <chen.kenyy@inventec.com>
>>>> + *
>>>> + * Derived from:
>>>> + *
>>>> + * This file is licensed under the terms of the GNU General Public
>>>> + * License version 2. This program is licensed "as is" without any
>>>> + * warranty of any kind, whether express or implied.
>>>> + */
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include <linux/jiffies.h>
>>>> +#include <linux/delay.h>
>>>> +#include <linux/slab.h>
>>>> +#include <linux/device.h>
>>>> +#include <linux/i2c.h>
>>>> +#include <linux/i2c-mux.h>
>>>> +
>>>> +#include <linux/i2c/pca954x.h>
>>>> +
>>>> +/*
>>>> + * The PCA9641 is two I2C bus masters demultiplexer. It supports two I2C masters
>>>> + * connected to a single slave bus.
>>>> + *
>>>> + * It is designed for high reliability dual master I2C bus applications where
>>>> + * correct system operation is required, even when two I2C master issue their
>>>> + * commands at the same time. The arbiter will select a winner and let it work
>>>> + * uninterrupted, and the losing master will take control of the I2C bus after
>>>> + * the winnter has finished. The arbiter also allows for queued requests where
>>>> + * a master requests the downstream bus while the other master has control.
>>>> + *
>>>> + */
>>>> +
>>>> +#define PCA9641_ID             0x01
>>>> +#define PCA9641_ID_MAGIC       0x38
>>>> +
>>>> +#define PCA9641_CONTROL                0x01
>>>> +#define PCA9641_STATUS         0x02
>>>> +#define PCA9641_TIME           0x03
>>>> +
>>>> +#define PCA9641_CTL_LOCK_REQ            (1 << 0)
>>>> +#define PCA9641_CTL_LOCK_GRANT          (1 << 1)
>>>> +#define PCA9641_CTL_BUS_CONNECT         (1 << 2)
>>>> +#define PCA9641_CTL_BUS_INIT            (1 << 3)
>>>> +#define PCA9641_CTL_SMBUS_SWRST         (1 << 4)
>>>> +#define PCA9641_CTL_IDLE_TIMER_DIS      (1 << 5)
>>>> +#define PCA9641_CTL_SMBUS_DIS           (1 << 6)
>>>> +#define PCA9641_CTL_PRIORITY            (1 << 7)
>>>> +
>>>> +#define PCA9641_STS_OTHER_LOCK          (1 << 0)
>>>> +#define PCA9641_STS_BUS_INIT_FAIL       (1 << 1)
>>>> +#define PCA9641_STS_BUS_HUNG            (1 << 2)
>>>> +#define PCA9641_STS_MBOX_EMPTY          (1 << 3)
>>>> +#define PCA9641_STS_MBOX_FULL           (1 << 4)
>>>> +#define PCA9641_STS_TEST_INT            (1 << 5)
>>>> +#define PCA9641_STS_SCL_IO              (1 << 6)
>>>> +#define PCA9641_STS_SDA_IO              (1 << 7)
>>>> +
>>>> +#define PCA9641_RES_TIME       0x03
>>>> +
>>>> +#define other_lock(x)  ((x) & PCA9641_STS_OTHER_LOCK)
>>>> +#define lock_grant(x)  ((x) & PCA9641_CTL_LOCK_GRANT)
>>>> +
>>>> +/* arbitration timeouts, in jiffies */
>>>> +#define ARB_TIMEOUT    (HZ / 8)        /* 125 ms until forcing bus ownership */
>>>> +#define ARB2_TIMEOUT   (HZ / 4)        /* 250 ms until acquisition failure */
>>>> +
>>>> +/* arbitration retry delays, in us */
>>>> +#define SELECT_DELAY_SHORT     50
>>>> +#define SELECT_DELAY_LONG      1000
>>>> +
>>>> +struct pca9641 {
>>>> +       struct i2c_client *client;
>>>> +       unsigned long select_timeout;
>>>> +       unsigned long arb_timeout;
>>>> +};
>>>> +
>>>> +static const struct i2c_device_id pca9641_id[] = {
>>>> +       {"pca9641", 0},
>>>> +       {}
>>>> +};
>>>> +
>>>> +MODULE_DEVICE_TABLE(i2c, pca9641_id);
>>>> +
>>>> +#ifdef CONFIG_OF
>>>> +static const struct of_device_id pca9641_of_match[] = {
>>>> +       { .compatible = "nxp,pca9641" },
>>>> +       {}
>>>> +};
>>>> +#endif
>>>> +
>>>> +/*
>>>> + * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
>>>> + * as they will try to lock the adapter a second time.
>>>> + */
>>>> +static int pca9641_reg_write(struct i2c_client *client, u8 command, u8 val)
>>>> +{
>>>> +       struct i2c_adapter *adap = client->adapter;
>>>> +       int ret;
>>>> +
>>>> +       if (adap->algo->master_xfer) {
>>>> +               struct i2c_msg msg;
>>>> +               char buf[2];
>>>> +
>>>> +               msg.addr = client->addr;
>>>> +               msg.flags = 0;
>>>> +               msg.len = 2;
>>>> +               buf[0] = command;
>>>> +               buf[1] = val;
>>>> +               msg.buf = buf;
>>>> +               ret = __i2c_transfer(adap, &msg, 1);
>>>> +       } else {
>>>> +               union i2c_smbus_data data;
>>>> +
>>>> +               data.byte = val;
>>>> +               ret = adap->algo->smbus_xfer(adap, client->addr,
>>>> +                                            client->flags,
>>>> +                                            I2C_SMBUS_WRITE,
>>>> +                                            command,
>>>> +                                            I2C_SMBUS_BYTE_DATA, &data);
>>>> +       }
>>>> +
>>>> +       return ret;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Read from chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
>>>> + * as they will try to lock adapter a second time.
>>>> + */
>>>> +static int pca9641_reg_read(struct i2c_client *client, u8 command)
>>>> +{
>>>> +       struct i2c_adapter *adap = client->adapter;
>>>> +       int ret;
>>>> +       u8 val;
>>>> +
>>>> +       if (adap->algo->master_xfer) {
>>>> +               struct i2c_msg msg[2] = {
>>>> +                       {
>>>> +                               .addr = client->addr,
>>>> +                               .flags = 0,
>>>> +                               .len = 1,
>>>> +                               .buf = &command
>>>> +                       },
>>>> +                       {
>>>> +                               .addr = client->addr,
>>>> +                               .flags = I2C_M_RD,
>>>> +                               .len = 1,
>>>> +                               .buf = &val
>>>> +                       }
>>>> +               };
>>>> +               ret = __i2c_transfer(adap, msg, 2);
>>>> +               if (ret == 2)
>>>> +                       ret = val;
>>>> +               else if (ret >= 0)
>>>> +                       ret = -EIO;
>>>> +       } else {
>>>> +               union i2c_smbus_data data;
>>>> +
>>>> +               ret = adap->algo->smbus_xfer(adap, client->addr,
>>>> +                                            client->flags,
>>>> +                                            I2C_SMBUS_READ,
>>>> +                                            command,
>>>> +                                            I2C_SMBUS_BYTE_DATA, &data);
>>>> +               if (!ret)
>>>> +                       ret = data.byte;
>>>> +       }
>>>> +       return ret;
>>>> +}
>>>> +
>>>> +/*
>>>> + * Arbitration management functions
>>>> + */
>>>> +static void pca9641_release_bus(struct i2c_client *client)
>>>> +{
>>>> +       int reg;
>>>> +
>>>> +       pca9641_reg_write(client, PCA9641_CONTROL, 0);
>>>> +}
>>>> +
>>>> +/*
>>>> + * Channel arbitration
>>>> + *
>>>> + * Return values:
>>>> + *  <0: error
>>>> + *  0 : bus not acquired
>>>> + *  1 : bus acquired
>>>> + */
>>>> +static int pca9641_arbitrate(struct i2c_client *client)
>>>> +{
>>>> +       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>>>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>>>> +       int reg_ctl, reg_sts;
>>>> +
>>>> +       reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
>>>> +       if (reg_ctl < 0)
>>>> +               return reg_ctl;
>>>> +       reg_sts = pca9641_reg_read(client, PCA9641_STATUS);
>>>> +
>>>> +       if (!other_lock(reg_sts) && !lock_grant(reg_ctl)) {
>>>> +               /*
>>>> +                * Bus is off. Request ownership or turn it on unless
>>>> +                * other master requested ownership.
>>>> +                */
>>>> +                reg_ctl |= PCA9641_CTL_LOCK_REQ;
>>>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>>> +
>>>> +                udelay(100);
>>>> +
>>>> +               reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
>>>> +
>>>> +                if (lock_grant(reg_ctl)) {
>>>> +                        /*
>>>> +                         * Other master did not request ownership,
>>>> +                         * or arbitration timeout expired. Take the bus.
>>>> +                         */
>>>> +                        reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
>>>> +                        pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>>> +                        data->select_timeout = SELECT_DELAY_SHORT;
>>>> +
>>>> +                        return 1;
>>>> +                } else {
>>>> +                        /*
>>>> +                         * Other master requested ownership.
>>>> +                         * Set extra long timeout to give it time to acquire it.
>>>> +                         */
>>>> +                        data->select_timeout = SELECT_DELAY_LONG * 2;
>>>> +                }
>>>> +       } else if (lock_grant(reg_ctl)) {
>>>> +                /*
>>>> +                 * Bus is on, and we own it. We are done with acquisition.
>>>> +                 */
>>>> +                reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
>>>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>>> +
>>>> +                return 1;
>>>> +       } else if (other_lock(reg_sts)) {
>>>> +                /*
>>>> +                 * Other master owns the bus.
>>>> +                 * If arbitration timeout has expired, force ownership.
>>>> +                 * Otherwise request it.
>>>> +                 */
>>>> +                data->select_timeout = SELECT_DELAY_LONG;
>>>> +                reg_ctl |= PCA9641_CTL_LOCK_REQ;
>>>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>>> +
>>>> +                /*
>>>> +                 * if (time_is_before_eq_jiffies(data->arb_timeout)) {
>>>> +                 * TODO:Time is up, take the bus and reset it.
>>>> +                 *
>>>> +                 *} else {
>>>> +                 * TODO: Request bus ownership if needed
>>>> +                 *
>>>> +                 *}
>>>> +                 */
>>>> +        }
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int pca9641_select_chan(struct i2c_mux_core *muxc, u32 chan)
>>>> +{
>>>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>>>> +       struct i2c_client *client = data->client;
>>>> +       int ret;
>>>> +       unsigned long timeout = jiffies + ARB2_TIMEOUT;
>>>> +               /* give up after this time */
>>>> +
>>>> +       data->arb_timeout = jiffies + ARB_TIMEOUT;
>>>> +               /* force bus ownership after this time */
>>>> +
>>>> +       do {
>>>> +               ret = pca9641_arbitrate(client);
>>>> +               if (ret)
>>>> +                       return ret < 0 ? ret : 0;
>>>> +
>>>> +               if (data->select_timeout == SELECT_DELAY_SHORT)
>>>> +                       udelay(data->select_timeout);
>>>> +               else
>>>> +                       msleep(data->select_timeout / 1000);
>>>> +       } while (time_is_after_eq_jiffies(timeout));
>>>> +
>>>> +       return -ETIMEDOUT;
>>>> +}
>>>> +
>>>> +static int pca9641_release_chan(struct i2c_mux_core *muxc, u32 chan)
>>>> +{
>>>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>>>> +       struct i2c_client *client = data->client;
>>>> +
>>>> +       pca9641_release_bus(client);
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +/*
>>>> + * I2C init/probing/exit functions
>>>> + */
>>>> +static int pca9641_probe(struct i2c_client *client,
>>>> +                        const struct i2c_device_id *id)
>>>> +{
>>>> +       struct i2c_adapter *adap = client->adapter;
>>>> +       struct pca954x_platform_data *pdata = dev_get_platdata(&client->dev);
>>>> +       struct i2c_mux_core *muxc;
>>>> +       struct pca9641 *data;
>>>> +       int force;
>>>> +       int ret;
>>>> +
>>>> +       if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE_DATA))
>>>> +               return -ENODEV;
>>>> +
>>>> +       /*
>>>> +        * I2C accesses are unprotected here.
>>>> +        * We have to lock the adapter before releasing the bus.
>>>> +        */
>>>> +       i2c_lock_adapter(adap);
>>>> +       pca9641_release_bus(client);
>>>> +       i2c_unlock_adapter(adap);
>>>> +
>>>> +       /* Create mux adapter */
>>>> +
>>>> +       force = 0;
>>>> +       if (pdata)
>>>> +               force = pdata->modes[0].adap_id;
>>>> +       muxc = i2c_mux_alloc(adap, &client->dev, 8, sizeof(*data),
>>>> +                            I2C_MUX_ARBITRATOR,
>>>> +                            pca9641_select_chan, pca9641_release_chan);
>>>> +       if (!muxc)
>>>> +               return -ENOMEM;
>>>> +
>>>> +       data = i2c_mux_priv(muxc);
>>>> +       data->client = client;
>>>> +
>>>> +       i2c_set_clientdata(client, muxc);
>>>> +
>>>> +
>>>> +       ret = i2c_mux_add_adapter(muxc, force, 0, 0);
>>>> +       if (ret) {
>>>> +               dev_err(&client->dev, "failed to register master demultiplexer\n");
>>>> +               return ret;
>>>> +       }
>>>> +
>>>> +       dev_info(&client->dev, "registered master demultiplexer for I2C %s\n",
>>>> +                client->name);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int pca9641_remove(struct i2c_client *client)
>>>> +{
>>>> +       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>>>> +
>>>> +       i2c_mux_del_adapters(muxc);
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static struct i2c_driver pca9641_driver = {
>>>> +       .driver = {
>>>> +                  .name = "pca9641",
>>>> +                  .of_match_table = of_match_ptr(pca9641_of_match),
>>>> +                  },
>>>> +       .probe = pca9641_probe,
>>>> +       .remove = pca9641_remove,
>>>> +       .id_table = pca9641_id,
>>>> +};
>>>> +
>>>> +module_i2c_driver(pca9641_driver);
>>>> +
>>>> +MODULE_AUTHOR("Ken Chen <chen.kenyy@inventec.com>");
>>>> +MODULE_DESCRIPTION("PCA9641 I2C master demultiplexer driver");
>>>> +MODULE_LICENSE("GPL v2");
>>>> --
>>>> 2.9.3
>>>>

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

* Re: [PATCH linux dev-4.13] drivers: i2c: master arbiter: create driver
  2018-03-12  6:40       ` ChenKenYY 陳永營 TAO
@ 2018-03-13  3:52         ` Joel Stanley
  0 siblings, 0 replies; 6+ messages in thread
From: Joel Stanley @ 2018-03-13  3:52 UTC (permalink / raw)
  To: ChenKenYY 陳永營 TAO; +Cc: OpenBMC Maillist

On Mon, Mar 12, 2018 at 5:10 PM, ChenKenYY 陳永營 TAO
<chen.kenyy@inventec.com> wrote:
> Hi Joel,
>
> Could I use an independent .c file(PCA9641) to upstream this driver?
> Because the reference code has probe issue and sperate the difference
> device behavior between PCA9541 and PCA9641 by function.
> So I think It would be better using an independent .c file.

If you think that there is a good reason to do a separate
implementation, then we should submit your version upstream and add
that information to your commit message.

Cheers,

Joel

>
>
> Best Regards,
> Ken
>
> 2018-03-08 13:34 GMT+08:00 Joel Stanley <joel@jms.id.au>:
>> On Thu, Mar 8, 2018 at 3:36 PM, ChenKenYY 陳永營 TAO
>> <chen.kenyy@inventec.com> wrote:
>>> Hi Joel,
>>>
>>> Sure, I can take it, but I need some instructions on the next action.
>>
>> Okay. I suggest you download the patch from patchwork and apply it to
>> the latest kernel release, 4.16-rc4:
>>
>> https://patchwork.ozlabs.org/patch/726491/
>>
>> You can download it by clicking the 'mbox' link at the top right of the page.
>>
>> Now you will have a file called
>> RFC-i2c-mux-Driver-for-PCA9641-I2C-Master-Arbiter.patch.
>>
>> $ git clone https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>> $ cd linux
>> $ git checkout -b pca9641-mux v4.16-rc4
>> $ git am ~/Downloads/RFC-i2c-mux-Driver-for-PCA9641-I2C-Master-Arbiter.patch
>>
>>
>> From there, take a look at the comments on the two reviews that Vidya had:
>>
>>  https://patchwork.ozlabs.org/patch/726491/#1580479
>>  https://patchwork.ozlabs.org/patch/726491/#1582916
>>
>> Fix those issues, and add your signed off by:
>>
>> $ git add drivers/i2c/muxes/i2c-mux-pca9541.c
>> $ git commit --amend -s
>>
>> Then test this driver on your board. To do this, modify the device
>> tree. You should be able to do this by modifying the ast2500-evb
>> device tree, and boot that on your system.
>>
>> Once this is working, use the checkpatch.pl tool to check for common mistakes:
>>
>> $ ./scripts/checkpatch.pl drivers/i2c/muxes/i2c-mux-pca9541.c
>>
>> Then use get_maintainer.pl to get a list of people you should send the patch to:
>>
>> $ ./scripts/get_maintainer.pl -f drivers/i2c/muxes/i2c-mux-pca9541.c
>> Guenter Roeck <linux@roeck-us.net> (maintainer:PCA9541 I2C BUS MASTER
>> SELECTOR DRIVER)
>> Peter Rosin <peda@axentia.se> (maintainer:I2C MUXES)
>> Wolfram Sang <wsa@the-dreams.de> (maintainer:I2C SUBSYSTEM)
>> linux-i2c@vger.kernel.org (open list:PCA9541 I2C BUS MASTER SELECTOR DRIVER)
>> linux-kernel@vger.kernel.org (open list)
>>
>> In addition, please add me to this list so I can help. Then send the
>> patch out, with a comment in the commit message (below the ---) that
>> you have continued this work from the previous author, and have tested
>> it on your system.
>>
>> Cheers,
>>
>> Joel
>>
>>>
>>> Thanks,
>>> Ken
>>>
>>> 2018-03-08 11:02 GMT+08:00 Joel Stanley <joel@jms.id.au>:
>>>> Hello Ken,
>>>>
>>>> On Fri, Feb 23, 2018 at 3:52 PM, Ken Chen <chen.kenyy@inventec.com> wrote:
>>>>> Initial PCA9641 2 chennel I2C bus master arbiter
>>>>>
>>>>> Signed-off-by: Ken Chen <chen.kenyy@inventec.com>
>>>>
>>>> The code here looks good. I did some searching, and found that someone
>>>> submitted a driver for this part about a year ago:
>>>>
>>>>  https://patchwork.ozlabs.org/patch/726491/
>>>>
>>>> Unfortunately the submitter did not send another version after it was
>>>> reviewed. I suggest we take up the patch that was submitted, make the
>>>> changes suggested, and submit it upstream. Are you able to take on
>>>> that work?
>>>>
>>>> Cheers,
>>>>
>>>> Joel
>>>>
>>>>> ---
>>>>>  drivers/i2c/muxes/Kconfig           |   9 +
>>>>>  drivers/i2c/muxes/Makefile          |   1 +
>>>>>  drivers/i2c/muxes/i2c-mux-pca9641.c | 372 ++++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 382 insertions(+)
>>>>>  create mode 100644 drivers/i2c/muxes/i2c-mux-pca9641.c
>>>>>
>>>>> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
>>>>> index 1712132..1cd1ad3 100644
>>>>> --- a/drivers/i2c/muxes/Kconfig
>>>>> +++ b/drivers/i2c/muxes/Kconfig
>>>>> @@ -73,6 +73,15 @@ config I2C_MUX_PCA954x
>>>>>           This driver can also be built as a module.  If so, the module
>>>>>           will be called i2c-mux-pca954x.
>>>>>
>>>>> +config I2C_MUX_PCA9641
>>>>> +       tristate "NXP PCA9641 I2C Master demultiplexer"
>>>>> +       help
>>>>> +         If you say yes here you get support for the NXP PCA9641
>>>>> +         I2C Master demultiplexer with an arbiter function.
>>>>> +
>>>>> +         This driver can also be built as a module.  If so, the module
>>>>> +         will be called i2c-mux-pca9641.
>>>>> +
>>>>>  config I2C_MUX_PINCTRL
>>>>>         tristate "pinctrl-based I2C multiplexer"
>>>>>         depends on PINCTRL
>>>>> diff --git a/drivers/i2c/muxes/Makefile b/drivers/i2c/muxes/Makefile
>>>>> index 4a67d31..f95d5f5 100644
>>>>> --- a/drivers/i2c/muxes/Makefile
>>>>> +++ b/drivers/i2c/muxes/Makefile
>>>>> @@ -11,6 +11,7 @@ obj-$(CONFIG_I2C_MUX_LTC4306) += i2c-mux-ltc4306.o
>>>>>  obj-$(CONFIG_I2C_MUX_MLXCPLD)  += i2c-mux-mlxcpld.o
>>>>>  obj-$(CONFIG_I2C_MUX_PCA9541)  += i2c-mux-pca9541.o
>>>>>  obj-$(CONFIG_I2C_MUX_PCA954x)  += i2c-mux-pca954x.o
>>>>> +obj-$(CONFIG_I2C_MUX_PCA9641)  += i2c-mux-pca9641.o
>>>>>  obj-$(CONFIG_I2C_MUX_PINCTRL)  += i2c-mux-pinctrl.o
>>>>>  obj-$(CONFIG_I2C_MUX_REG)      += i2c-mux-reg.o
>>>>>
>>>>> diff --git a/drivers/i2c/muxes/i2c-mux-pca9641.c b/drivers/i2c/muxes/i2c-mux-pca9641.c
>>>>> new file mode 100644
>>>>> index 0000000..ca7b816
>>>>> --- /dev/null
>>>>> +++ b/drivers/i2c/muxes/i2c-mux-pca9641.c
>>>>> @@ -0,0 +1,372 @@
>>>>> +/*
>>>>> + * I2C demultiplexer driver for PCA9641 bus master demultiplexer
>>>>> + *
>>>>> + * Copyright (c) 2010 Ericsson AB.
>>>>> + *
>>>>> + * Author: Ken Chen <chen.kenyy@inventec.com>
>>>>> + *
>>>>> + * Derived from:
>>>>> + *
>>>>> + * This file is licensed under the terms of the GNU General Public
>>>>> + * License version 2. This program is licensed "as is" without any
>>>>> + * warranty of any kind, whether express or implied.
>>>>> + */
>>>>> +
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/jiffies.h>
>>>>> +#include <linux/delay.h>
>>>>> +#include <linux/slab.h>
>>>>> +#include <linux/device.h>
>>>>> +#include <linux/i2c.h>
>>>>> +#include <linux/i2c-mux.h>
>>>>> +
>>>>> +#include <linux/i2c/pca954x.h>
>>>>> +
>>>>> +/*
>>>>> + * The PCA9641 is two I2C bus masters demultiplexer. It supports two I2C masters
>>>>> + * connected to a single slave bus.
>>>>> + *
>>>>> + * It is designed for high reliability dual master I2C bus applications where
>>>>> + * correct system operation is required, even when two I2C master issue their
>>>>> + * commands at the same time. The arbiter will select a winner and let it work
>>>>> + * uninterrupted, and the losing master will take control of the I2C bus after
>>>>> + * the winnter has finished. The arbiter also allows for queued requests where
>>>>> + * a master requests the downstream bus while the other master has control.
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +#define PCA9641_ID             0x01
>>>>> +#define PCA9641_ID_MAGIC       0x38
>>>>> +
>>>>> +#define PCA9641_CONTROL                0x01
>>>>> +#define PCA9641_STATUS         0x02
>>>>> +#define PCA9641_TIME           0x03
>>>>> +
>>>>> +#define PCA9641_CTL_LOCK_REQ            (1 << 0)
>>>>> +#define PCA9641_CTL_LOCK_GRANT          (1 << 1)
>>>>> +#define PCA9641_CTL_BUS_CONNECT         (1 << 2)
>>>>> +#define PCA9641_CTL_BUS_INIT            (1 << 3)
>>>>> +#define PCA9641_CTL_SMBUS_SWRST         (1 << 4)
>>>>> +#define PCA9641_CTL_IDLE_TIMER_DIS      (1 << 5)
>>>>> +#define PCA9641_CTL_SMBUS_DIS           (1 << 6)
>>>>> +#define PCA9641_CTL_PRIORITY            (1 << 7)
>>>>> +
>>>>> +#define PCA9641_STS_OTHER_LOCK          (1 << 0)
>>>>> +#define PCA9641_STS_BUS_INIT_FAIL       (1 << 1)
>>>>> +#define PCA9641_STS_BUS_HUNG            (1 << 2)
>>>>> +#define PCA9641_STS_MBOX_EMPTY          (1 << 3)
>>>>> +#define PCA9641_STS_MBOX_FULL           (1 << 4)
>>>>> +#define PCA9641_STS_TEST_INT            (1 << 5)
>>>>> +#define PCA9641_STS_SCL_IO              (1 << 6)
>>>>> +#define PCA9641_STS_SDA_IO              (1 << 7)
>>>>> +
>>>>> +#define PCA9641_RES_TIME       0x03
>>>>> +
>>>>> +#define other_lock(x)  ((x) & PCA9641_STS_OTHER_LOCK)
>>>>> +#define lock_grant(x)  ((x) & PCA9641_CTL_LOCK_GRANT)
>>>>> +
>>>>> +/* arbitration timeouts, in jiffies */
>>>>> +#define ARB_TIMEOUT    (HZ / 8)        /* 125 ms until forcing bus ownership */
>>>>> +#define ARB2_TIMEOUT   (HZ / 4)        /* 250 ms until acquisition failure */
>>>>> +
>>>>> +/* arbitration retry delays, in us */
>>>>> +#define SELECT_DELAY_SHORT     50
>>>>> +#define SELECT_DELAY_LONG      1000
>>>>> +
>>>>> +struct pca9641 {
>>>>> +       struct i2c_client *client;
>>>>> +       unsigned long select_timeout;
>>>>> +       unsigned long arb_timeout;
>>>>> +};
>>>>> +
>>>>> +static const struct i2c_device_id pca9641_id[] = {
>>>>> +       {"pca9641", 0},
>>>>> +       {}
>>>>> +};
>>>>> +
>>>>> +MODULE_DEVICE_TABLE(i2c, pca9641_id);
>>>>> +
>>>>> +#ifdef CONFIG_OF
>>>>> +static const struct of_device_id pca9641_of_match[] = {
>>>>> +       { .compatible = "nxp,pca9641" },
>>>>> +       {}
>>>>> +};
>>>>> +#endif
>>>>> +
>>>>> +/*
>>>>> + * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
>>>>> + * as they will try to lock the adapter a second time.
>>>>> + */
>>>>> +static int pca9641_reg_write(struct i2c_client *client, u8 command, u8 val)
>>>>> +{
>>>>> +       struct i2c_adapter *adap = client->adapter;
>>>>> +       int ret;
>>>>> +
>>>>> +       if (adap->algo->master_xfer) {
>>>>> +               struct i2c_msg msg;
>>>>> +               char buf[2];
>>>>> +
>>>>> +               msg.addr = client->addr;
>>>>> +               msg.flags = 0;
>>>>> +               msg.len = 2;
>>>>> +               buf[0] = command;
>>>>> +               buf[1] = val;
>>>>> +               msg.buf = buf;
>>>>> +               ret = __i2c_transfer(adap, &msg, 1);
>>>>> +       } else {
>>>>> +               union i2c_smbus_data data;
>>>>> +
>>>>> +               data.byte = val;
>>>>> +               ret = adap->algo->smbus_xfer(adap, client->addr,
>>>>> +                                            client->flags,
>>>>> +                                            I2C_SMBUS_WRITE,
>>>>> +                                            command,
>>>>> +                                            I2C_SMBUS_BYTE_DATA, &data);
>>>>> +       }
>>>>> +
>>>>> +       return ret;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Read from chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
>>>>> + * as they will try to lock adapter a second time.
>>>>> + */
>>>>> +static int pca9641_reg_read(struct i2c_client *client, u8 command)
>>>>> +{
>>>>> +       struct i2c_adapter *adap = client->adapter;
>>>>> +       int ret;
>>>>> +       u8 val;
>>>>> +
>>>>> +       if (adap->algo->master_xfer) {
>>>>> +               struct i2c_msg msg[2] = {
>>>>> +                       {
>>>>> +                               .addr = client->addr,
>>>>> +                               .flags = 0,
>>>>> +                               .len = 1,
>>>>> +                               .buf = &command
>>>>> +                       },
>>>>> +                       {
>>>>> +                               .addr = client->addr,
>>>>> +                               .flags = I2C_M_RD,
>>>>> +                               .len = 1,
>>>>> +                               .buf = &val
>>>>> +                       }
>>>>> +               };
>>>>> +               ret = __i2c_transfer(adap, msg, 2);
>>>>> +               if (ret == 2)
>>>>> +                       ret = val;
>>>>> +               else if (ret >= 0)
>>>>> +                       ret = -EIO;
>>>>> +       } else {
>>>>> +               union i2c_smbus_data data;
>>>>> +
>>>>> +               ret = adap->algo->smbus_xfer(adap, client->addr,
>>>>> +                                            client->flags,
>>>>> +                                            I2C_SMBUS_READ,
>>>>> +                                            command,
>>>>> +                                            I2C_SMBUS_BYTE_DATA, &data);
>>>>> +               if (!ret)
>>>>> +                       ret = data.byte;
>>>>> +       }
>>>>> +       return ret;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Arbitration management functions
>>>>> + */
>>>>> +static void pca9641_release_bus(struct i2c_client *client)
>>>>> +{
>>>>> +       int reg;
>>>>> +
>>>>> +       pca9641_reg_write(client, PCA9641_CONTROL, 0);
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Channel arbitration
>>>>> + *
>>>>> + * Return values:
>>>>> + *  <0: error
>>>>> + *  0 : bus not acquired
>>>>> + *  1 : bus acquired
>>>>> + */
>>>>> +static int pca9641_arbitrate(struct i2c_client *client)
>>>>> +{
>>>>> +       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>>>>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>>>>> +       int reg_ctl, reg_sts;
>>>>> +
>>>>> +       reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
>>>>> +       if (reg_ctl < 0)
>>>>> +               return reg_ctl;
>>>>> +       reg_sts = pca9641_reg_read(client, PCA9641_STATUS);
>>>>> +
>>>>> +       if (!other_lock(reg_sts) && !lock_grant(reg_ctl)) {
>>>>> +               /*
>>>>> +                * Bus is off. Request ownership or turn it on unless
>>>>> +                * other master requested ownership.
>>>>> +                */
>>>>> +                reg_ctl |= PCA9641_CTL_LOCK_REQ;
>>>>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>>>> +
>>>>> +                udelay(100);
>>>>> +
>>>>> +               reg_ctl = pca9641_reg_read(client, PCA9641_CONTROL);
>>>>> +
>>>>> +                if (lock_grant(reg_ctl)) {
>>>>> +                        /*
>>>>> +                         * Other master did not request ownership,
>>>>> +                         * or arbitration timeout expired. Take the bus.
>>>>> +                         */
>>>>> +                        reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
>>>>> +                        pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>>>> +                        data->select_timeout = SELECT_DELAY_SHORT;
>>>>> +
>>>>> +                        return 1;
>>>>> +                } else {
>>>>> +                        /*
>>>>> +                         * Other master requested ownership.
>>>>> +                         * Set extra long timeout to give it time to acquire it.
>>>>> +                         */
>>>>> +                        data->select_timeout = SELECT_DELAY_LONG * 2;
>>>>> +                }
>>>>> +       } else if (lock_grant(reg_ctl)) {
>>>>> +                /*
>>>>> +                 * Bus is on, and we own it. We are done with acquisition.
>>>>> +                 */
>>>>> +                reg_ctl |= PCA9641_CTL_BUS_CONNECT | PCA9641_CTL_LOCK_REQ;
>>>>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>>>> +
>>>>> +                return 1;
>>>>> +       } else if (other_lock(reg_sts)) {
>>>>> +                /*
>>>>> +                 * Other master owns the bus.
>>>>> +                 * If arbitration timeout has expired, force ownership.
>>>>> +                 * Otherwise request it.
>>>>> +                 */
>>>>> +                data->select_timeout = SELECT_DELAY_LONG;
>>>>> +                reg_ctl |= PCA9641_CTL_LOCK_REQ;
>>>>> +                pca9641_reg_write(client, PCA9641_CONTROL, reg_ctl);
>>>>> +
>>>>> +                /*
>>>>> +                 * if (time_is_before_eq_jiffies(data->arb_timeout)) {
>>>>> +                 * TODO:Time is up, take the bus and reset it.
>>>>> +                 *
>>>>> +                 *} else {
>>>>> +                 * TODO: Request bus ownership if needed
>>>>> +                 *
>>>>> +                 *}
>>>>> +                 */
>>>>> +        }
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>> +static int pca9641_select_chan(struct i2c_mux_core *muxc, u32 chan)
>>>>> +{
>>>>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>>>>> +       struct i2c_client *client = data->client;
>>>>> +       int ret;
>>>>> +       unsigned long timeout = jiffies + ARB2_TIMEOUT;
>>>>> +               /* give up after this time */
>>>>> +
>>>>> +       data->arb_timeout = jiffies + ARB_TIMEOUT;
>>>>> +               /* force bus ownership after this time */
>>>>> +
>>>>> +       do {
>>>>> +               ret = pca9641_arbitrate(client);
>>>>> +               if (ret)
>>>>> +                       return ret < 0 ? ret : 0;
>>>>> +
>>>>> +               if (data->select_timeout == SELECT_DELAY_SHORT)
>>>>> +                       udelay(data->select_timeout);
>>>>> +               else
>>>>> +                       msleep(data->select_timeout / 1000);
>>>>> +       } while (time_is_after_eq_jiffies(timeout));
>>>>> +
>>>>> +       return -ETIMEDOUT;
>>>>> +}
>>>>> +
>>>>> +static int pca9641_release_chan(struct i2c_mux_core *muxc, u32 chan)
>>>>> +{
>>>>> +       struct pca9641 *data = i2c_mux_priv(muxc);
>>>>> +       struct i2c_client *client = data->client;
>>>>> +
>>>>> +       pca9641_release_bus(client);
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * I2C init/probing/exit functions
>>>>> + */
>>>>> +static int pca9641_probe(struct i2c_client *client,
>>>>> +                        const struct i2c_device_id *id)
>>>>> +{
>>>>> +       struct i2c_adapter *adap = client->adapter;
>>>>> +       struct pca954x_platform_data *pdata = dev_get_platdata(&client->dev);
>>>>> +       struct i2c_mux_core *muxc;
>>>>> +       struct pca9641 *data;
>>>>> +       int force;
>>>>> +       int ret;
>>>>> +
>>>>> +       if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE_DATA))
>>>>> +               return -ENODEV;
>>>>> +
>>>>> +       /*
>>>>> +        * I2C accesses are unprotected here.
>>>>> +        * We have to lock the adapter before releasing the bus.
>>>>> +        */
>>>>> +       i2c_lock_adapter(adap);
>>>>> +       pca9641_release_bus(client);
>>>>> +       i2c_unlock_adapter(adap);
>>>>> +
>>>>> +       /* Create mux adapter */
>>>>> +
>>>>> +       force = 0;
>>>>> +       if (pdata)
>>>>> +               force = pdata->modes[0].adap_id;
>>>>> +       muxc = i2c_mux_alloc(adap, &client->dev, 8, sizeof(*data),
>>>>> +                            I2C_MUX_ARBITRATOR,
>>>>> +                            pca9641_select_chan, pca9641_release_chan);
>>>>> +       if (!muxc)
>>>>> +               return -ENOMEM;
>>>>> +
>>>>> +       data = i2c_mux_priv(muxc);
>>>>> +       data->client = client;
>>>>> +
>>>>> +       i2c_set_clientdata(client, muxc);
>>>>> +
>>>>> +
>>>>> +       ret = i2c_mux_add_adapter(muxc, force, 0, 0);
>>>>> +       if (ret) {
>>>>> +               dev_err(&client->dev, "failed to register master demultiplexer\n");
>>>>> +               return ret;
>>>>> +       }
>>>>> +
>>>>> +       dev_info(&client->dev, "registered master demultiplexer for I2C %s\n",
>>>>> +                client->name);
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>> +static int pca9641_remove(struct i2c_client *client)
>>>>> +{
>>>>> +       struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>>>>> +
>>>>> +       i2c_mux_del_adapters(muxc);
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>> +static struct i2c_driver pca9641_driver = {
>>>>> +       .driver = {
>>>>> +                  .name = "pca9641",
>>>>> +                  .of_match_table = of_match_ptr(pca9641_of_match),
>>>>> +                  },
>>>>> +       .probe = pca9641_probe,
>>>>> +       .remove = pca9641_remove,
>>>>> +       .id_table = pca9641_id,
>>>>> +};
>>>>> +
>>>>> +module_i2c_driver(pca9641_driver);
>>>>> +
>>>>> +MODULE_AUTHOR("Ken Chen <chen.kenyy@inventec.com>");
>>>>> +MODULE_DESCRIPTION("PCA9641 I2C master demultiplexer driver");
>>>>> +MODULE_LICENSE("GPL v2");
>>>>> --
>>>>> 2.9.3
>>>>>

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

end of thread, other threads:[~2018-03-13  3:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-23  5:22 [PATCH linux dev-4.13] drivers: i2c: master arbiter: create driver Ken Chen
2018-03-08  3:02 ` Joel Stanley
2018-03-08  5:06   ` ChenKenYY 陳永營 TAO
2018-03-08  5:34     ` Joel Stanley
2018-03-12  6:40       ` ChenKenYY 陳永營 TAO
2018-03-13  3:52         ` Joel Stanley

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.