[PATCH v2 0/5] i2c: mux: pca9541: extend with support for pca9641

[PATCH v2 0/5] i2c: mux: pca9541: extend with support for pca9641

[Peter Rosin]

Hi!

So, it's been a year or so since this was last visited. Time flies.
At that time, Ken Chen gave up and I didn't want to add untested
code. However, Pradeep Srinivasan asked about PCA9641 and so I have
now rebased the preparatory patches to v5.0 and added the bits Ken
wrote on top of the framework I wrote.

Looking forward to some test results, this has only been build-tested.
The actual code that does anything remotely interesting with the
PCA9641 is all Kens work, and I have no knowledge if it works or not.

Changes since last year   (https://lkml.org/lkml/2018/3/20/205)
- rebased to v5.0
- changed a couple of helper functions to return bool instead of int 0/1
- added dt-bindings patch
- warped Kens patch to fit on top of the preparatory work in patches 1-3

Cheers,
Peter

Peter Rosin (5):
  i2c: mux: pca9541: use the BIT macro
  i2c: mux: pca9541: namespace cleanup
  i2c: mux: pca9541: prepare for PCA9641 support
  dt-bindings: i2c: pca9541: extend with compatible for PCA9641
  i2c: mux: pca9541: add support for PCA9641

 .../devicetree/bindings/i2c/nxp,pca9541.txt        |   6 +-
 drivers/i2c/muxes/Kconfig                          |   6 +-
 drivers/i2c/muxes/i2c-mux-pca9541.c                | 252 +++++++++++++++++----
 3 files changed, 218 insertions(+), 46 deletions(-)

-- 
2.11.0

[PATCH v2 1/5] i2c: mux: pca9541: use the BIT macro

[Peter Rosin]

Because it looks nice!

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/i2c-mux-pca9541.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index 9e75d6b9140b..30cabf482985 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -16,6 +16,7 @@
  * warranty of any kind, whether express or implied.
  */
 
+#include <linux/bitops.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/i2c.h>
@@ -43,20 +44,20 @@
 #define PCA9541_CONTROL		0x01
 #define PCA9541_ISTAT		0x02
 
-#define PCA9541_CTL_MYBUS	(1 << 0)
-#define PCA9541_CTL_NMYBUS	(1 << 1)
-#define PCA9541_CTL_BUSON	(1 << 2)
-#define PCA9541_CTL_NBUSON	(1 << 3)
-#define PCA9541_CTL_BUSINIT	(1 << 4)
-#define PCA9541_CTL_TESTON	(1 << 6)
-#define PCA9541_CTL_NTESTON	(1 << 7)
-
-#define PCA9541_ISTAT_INTIN	(1 << 0)
-#define PCA9541_ISTAT_BUSINIT	(1 << 1)
-#define PCA9541_ISTAT_BUSOK	(1 << 2)
-#define PCA9541_ISTAT_BUSLOST	(1 << 3)
-#define PCA9541_ISTAT_MYTEST	(1 << 6)
-#define PCA9541_ISTAT_NMYTEST	(1 << 7)
+#define PCA9541_CTL_MYBUS	BIT(0)
+#define PCA9541_CTL_NMYBUS	BIT(1)
+#define PCA9541_CTL_BUSON	BIT(2)
+#define PCA9541_CTL_NBUSON	BIT(3)
+#define PCA9541_CTL_BUSINIT	BIT(4)
+#define PCA9541_CTL_TESTON	BIT(6)
+#define PCA9541_CTL_NTESTON	BIT(7)
+
+#define PCA9541_ISTAT_INTIN	BIT(0)
+#define PCA9541_ISTAT_BUSINIT	BIT(1)
+#define PCA9541_ISTAT_BUSOK	BIT(2)
+#define PCA9541_ISTAT_BUSLOST	BIT(3)
+#define PCA9541_ISTAT_MYTEST	BIT(6)
+#define PCA9541_ISTAT_NMYTEST	BIT(7)
 
 #define BUSON		(PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
 #define MYBUS		(PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
-- 
2.11.0

Re: [PATCH v2 1/5] i2c: mux: pca9541: use the BIT macro

[Luca Ceresoli]

Hi,

On 07/03/19 00:15, Peter Rosin wrote:
> Because it looks nice!
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
> Signed-off-by: Peter Rosin <peda@axentia.se>

Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>

-- 
Luca

[PATCH v2 2/5] i2c: mux: pca9541: namespace cleanup

[Peter Rosin]

In preparation for PCA9641 support, convert the mybus and busoff macros
to functions, and in the process prefix them with pca9541_. Also prefix
remaining chip specific macros with PCA9541_.

Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/i2c-mux-pca9541.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index 30cabf482985..28f46450f4b4 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -59,10 +59,8 @@
 #define PCA9541_ISTAT_MYTEST	BIT(6)
 #define PCA9541_ISTAT_NMYTEST	BIT(7)
 
-#define BUSON		(PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
-#define MYBUS		(PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
-#define mybus(x)	(!((x) & MYBUS) || ((x) & MYBUS) == MYBUS)
-#define busoff(x)	(!((x) & BUSON) || ((x) & BUSON) == BUSON)
+#define PCA9541_BUSON	(PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
+#define PCA9541_MYBUS	(PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
 
 /* arbitration timeouts, in jiffies */
 #define ARB_TIMEOUT	(HZ / 8)	/* 125 ms until forcing bus ownership */
@@ -93,6 +91,20 @@ static const struct of_device_id pca9541_of_match[] = {
 MODULE_DEVICE_TABLE(of, pca9541_of_match);
 #endif
 
+static bool pca9541_mybus(int ctl)
+{
+	if (!(ctl & PCA9541_MYBUS))
+		return true;
+	return (ctl & PCA9541_MYBUS) == PCA9541_MYBUS;
+}
+
+static bool pca9541_busoff(int ctl)
+{
+	if (!(ctl & PCA9541_BUSON))
+		return true;
+	return (ctl & PCA9541_BUSON) == PCA9541_BUSON;
+}
+
 /*
  * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
  * as they will try to lock the adapter a second time.
@@ -134,7 +146,7 @@ static void pca9541_release_bus(struct i2c_client *client)
 	int reg;
 
 	reg = pca9541_reg_read(client, PCA9541_CONTROL);
-	if (reg >= 0 && !busoff(reg) && mybus(reg))
+	if (reg >= 0 && !pca9541_busoff(reg) && pca9541_mybus(reg))
 		pca9541_reg_write(client, PCA9541_CONTROL,
 				  (reg & PCA9541_CTL_NBUSON) >> 1);
 }
@@ -186,7 +198,7 @@ static int pca9541_arbitrate(struct i2c_client *client)
 	if (reg < 0)
 		return reg;
 
-	if (busoff(reg)) {
+	if (pca9541_busoff(reg)) {
 		int istat;
 		/*
 		 * Bus is off. Request ownership or turn it on unless
@@ -211,7 +223,7 @@ static int pca9541_arbitrate(struct i2c_client *client)
 			 */
 			data->select_timeout = SELECT_DELAY_LONG * 2;
 		}
-	} else if (mybus(reg)) {
+	} else if (pca9541_mybus(reg)) {
 		/*
 		 * Bus is on, and we own it. We are done with acquisition.
 		 * Reset NTESTON and BUSINIT, then return success.
-- 
2.11.0

[PATCH v2 3/5] i2c: mux: pca9541: prepare for PCA9641 support

[Peter Rosin]

Make the arbitrate and release_bus implementation chip specific.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/i2c-mux-pca9541.c | 62 +++++++++++++++++++++++++++----------
 1 file changed, 45 insertions(+), 17 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index 28f46450f4b4..5eb36e3223d5 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -23,6 +23,7 @@
 #include <linux/i2c-mux.h>
 #include <linux/jiffies.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_data/pca954x.h>
 #include <linux/slab.h>
 
@@ -70,26 +71,22 @@
 #define SELECT_DELAY_SHORT	50
 #define SELECT_DELAY_LONG	1000
 
-struct pca9541 {
-	struct i2c_client *client;
-	unsigned long select_timeout;
-	unsigned long arb_timeout;
+enum chip_name {
+	pca9541,
 };
 
-static const struct i2c_device_id pca9541_id[] = {
-	{"pca9541", 0},
-	{}
+struct chip_desc {
+	int (*arbitrate)(struct i2c_client *client);
+	void (*release_bus)(struct i2c_client *client);
 };
 
-MODULE_DEVICE_TABLE(i2c, pca9541_id);
+struct pca9541 {
+	const struct chip_desc *chip;
 
-#ifdef CONFIG_OF
-static const struct of_device_id pca9541_of_match[] = {
-	{ .compatible = "nxp,pca9541" },
-	{}
+	struct i2c_client *client;
+	unsigned long select_timeout;
+	unsigned long arb_timeout;
 };
-MODULE_DEVICE_TABLE(of, pca9541_of_match);
-#endif
 
 static bool pca9541_mybus(int ctl)
 {
@@ -271,7 +268,7 @@ static int pca9541_select_chan(struct i2c_mux_core *muxc, u32 chan)
 		/* force bus ownership after this time */
 
 	do {
-		ret = pca9541_arbitrate(client);
+		ret = data->chip->arbitrate(client);
 		if (ret)
 			return ret < 0 ? ret : 0;
 
@@ -289,10 +286,32 @@ static int pca9541_release_chan(struct i2c_mux_core *muxc, u32 chan)
 	struct pca9541 *data = i2c_mux_priv(muxc);
 	struct i2c_client *client = data->client;
 
-	pca9541_release_bus(client);
+	data->chip->release_bus(client);
 	return 0;
 }
 
+static const struct chip_desc chips[] = {
+	[pca9541] = {
+		.arbitrate = pca9541_arbitrate,
+		.release_bus = pca9541_release_bus,
+	},
+};
+
+static const struct i2c_device_id pca9541_id[] = {
+	{ "pca9541", pca9541 },
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, pca9541_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id pca9541_of_match[] = {
+	{ .compatible = "nxp,pca9541", .data = &chips[pca9541] },
+	{}
+};
+MODULE_DEVICE_TABLE(of, pca9541_of_match);
+#endif
+
 /*
  * I2C init/probing/exit functions
  */
@@ -301,6 +320,8 @@ static int pca9541_probe(struct i2c_client *client,
 {
 	struct i2c_adapter *adap = client->adapter;
 	struct pca954x_platform_data *pdata = dev_get_platdata(&client->dev);
+	const struct of_device_id *match;
+	const struct chip_desc *chip;
 	struct i2c_mux_core *muxc;
 	struct pca9541 *data;
 	int force;
@@ -309,12 +330,18 @@ static int pca9541_probe(struct i2c_client *client,
 	if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;
 
+	match = of_match_device(of_match_ptr(pca9541_of_match), &client->dev);
+	if (match)
+		chip = of_device_get_match_data(&client->dev);
+	else
+		chip = &chips[id->driver_data];
+
 	/*
 	 * I2C accesses are unprotected here.
 	 * We have to lock the I2C segment before releasing the bus.
 	 */
 	i2c_lock_bus(adap, I2C_LOCK_SEGMENT);
-	pca9541_release_bus(client);
+	chip->release_bus(client);
 	i2c_unlock_bus(adap, I2C_LOCK_SEGMENT);
 
 	/* Create mux adapter */
@@ -329,6 +356,7 @@ static int pca9541_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	data = i2c_mux_priv(muxc);
+	data->chip = chip;
 	data->client = client;
 
 	i2c_set_clientdata(client, muxc);
-- 
2.11.0

[PATCH v2 4/5] dt-bindings: i2c: pca9541: extend with compatible for PCA9641

[Peter Rosin]

The binding is equivalent apart from the compatible.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/devicetree/bindings/i2c/nxp,pca9541.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/nxp,pca9541.txt b/Documentation/devicetree/bindings/i2c/nxp,pca9541.txt
index 42bfc09c8918..17b4cb9d76da 100644
--- a/Documentation/devicetree/bindings/i2c/nxp,pca9541.txt
+++ b/Documentation/devicetree/bindings/i2c/nxp,pca9541.txt
@@ -1,8 +1,10 @@
-* NXP PCA9541 I2C bus master selector
+* NXP PCA9541/PCA9641 I2C bus master selectors
 
 Required Properties:
 
-  - compatible: Must be "nxp,pca9541"
+  - compatible: Must be either of
+		"nxp,pca9541"
+		"nxp,pca9641"
 
   - reg: The I2C address of the device.
 
-- 
2.11.0

Re: [PATCH v2 4/5] dt-bindings: i2c: pca9541: extend with compatible for PCA9641

[Rob Herring]

On Wed, 6 Mar 2019 23:15:50 +0000, Peter Rosin wrote:
> The binding is equivalent apart from the compatible.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  Documentation/devicetree/bindings/i2c/nxp,pca9541.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

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

[PATCH v2 5/5] i2c: mux: pca9541: add support for PCA9641

[Peter Rosin]

Heavily based on code from Ken Chen <chen.kenyy@inventec.com>.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/muxes/Kconfig           |   6 +-
 drivers/i2c/muxes/i2c-mux-pca9541.c | 137 ++++++++++++++++++++++++++++++++++--
 2 files changed, 136 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
index 52a4a922e7e6..8532841de5db 100644
--- a/drivers/i2c/muxes/Kconfig
+++ b/drivers/i2c/muxes/Kconfig
@@ -55,10 +55,10 @@ config I2C_MUX_LTC4306
 	  will be called i2c-mux-ltc4306.
 
 config I2C_MUX_PCA9541
-	tristate "NXP PCA9541 I2C Master Selector"
+	tristate "NXP PCA9541/PCA9641 I2C Master Selectors"
 	help
-	  If you say yes here you get support for the NXP PCA9541
-	  I2C Master Selector.
+	  If you say yes here you get support for the NXP PCA9541/PCA9641
+	  I2C Master Selectors.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-mux-pca9541.
diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
index 5eb36e3223d5..5d4e0c92e978 100644
--- a/drivers/i2c/muxes/i2c-mux-pca9541.c
+++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
@@ -1,5 +1,5 @@
 /*
- * I2C multiplexer driver for PCA9541 bus master selector
+ * I2C multiplexer driver for PCA9541/PCA9641 bus master selectors
  *
  * Copyright (c) 2010 Ericsson AB.
  *
@@ -28,8 +28,8 @@
 #include <linux/slab.h>
 
 /*
- * The PCA9541 is a bus master selector. It supports two I2C masters connected
- * to a single slave bus.
+ * The PCA9541 and PCA9641 are bus master selector. They support two I2C masters
+ * connected to a single slave bus.
  *
  * Before each bus transaction, a master has to acquire bus ownership. After the
  * transaction is complete, bus ownership has to be released. This fits well
@@ -63,6 +63,33 @@
 #define PCA9541_BUSON	(PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
 #define PCA9541_MYBUS	(PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
 
+#define PCA9641_ID			0x00
+#define PCA9641_ID_MAGIC		0x38
+
+#define PCA9641_CONTROL			0x01
+#define PCA9641_STATUS			0x02
+#define PCA9641_TIME			0x03
+
+#define PCA9641_CTL_LOCK_REQ		BIT(0)
+#define PCA9641_CTL_LOCK_GRANT		BIT(1)
+#define PCA9641_CTL_BUS_CONNECT		BIT(2)
+#define PCA9641_CTL_BUS_INIT		BIT(3)
+#define PCA9641_CTL_SMBUS_SWRST		BIT(4)
+#define PCA9641_CTL_IDLE_TIMER_DIS	BIT(5)
+#define PCA9641_CTL_SMBUS_DIS		BIT(6)
+#define PCA9641_CTL_PRIORITY		BIT(7)
+
+#define PCA9641_STS_OTHER_LOCK		BIT(0)
+#define PCA9641_STS_BUS_INIT_FAIL	BIT(1)
+#define PCA9641_STS_BUS_HUNG		BIT(2)
+#define PCA9641_STS_MBOX_EMPTY		BIT(3)
+#define PCA9641_STS_MBOX_FULL		BIT(4)
+#define PCA9641_STS_TEST_INT		BIT(5)
+#define PCA9641_STS_SCL_IO		BIT(6)
+#define PCA9641_STS_SDA_IO		BIT(7)
+
+#define PCA9641_RES_TIME		0x03
+
 /* 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 */
@@ -73,6 +100,7 @@
 
 enum chip_name {
 	pca9541,
+	pca9641,
 };
 
 struct chip_desc {
@@ -102,6 +130,21 @@ static bool pca9541_busoff(int ctl)
 	return (ctl & PCA9541_BUSON) == PCA9541_BUSON;
 }
 
+static bool pca9641_lock_grant(int ctl)
+{
+	return !!(ctl & PCA9641_CTL_LOCK_GRANT);
+}
+
+static bool pca9641_other_lock(int sts)
+{
+	return !!(sts & PCA9641_STS_OTHER_LOCK);
+}
+
+static bool pca9641_busoff(int ctl, int sts)
+{
+	return !pca9641_lock_grant(ctl) && !pca9641_other_lock(sts);
+}
+
 /*
  * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
  * as they will try to lock the adapter a second time.
@@ -256,6 +299,86 @@ static int pca9541_arbitrate(struct i2c_client *client)
 	return 0;
 }
 
+/* Release bus. */
+static void pca9641_release_bus(struct i2c_client *client)
+{
+	pca9541_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 pca9541 *data = i2c_mux_priv(muxc);
+	int reg_ctl, reg_sts;
+
+	reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
+	if (reg_ctl < 0)
+		return reg_ctl;
+	reg_sts = pca9541_reg_read(client, PCA9641_STATUS);
+
+	if (pca9641_busoff(reg_ctl, reg_sts)) {
+		/*
+		 * Bus is off. Request ownership or turn it on unless
+		 * other master requested ownership.
+		 */
+		reg_ctl |= PCA9641_CTL_LOCK_REQ;
+		pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
+		reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
+
+		if (pca9641_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;
+			pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
+			data->select_timeout = SELECT_DELAY_SHORT;
+
+			return 1;
+		}
+
+		/*
+		 * Other master requested ownership.
+		 * Set extra long timeout to give it time to acquire it.
+		 */
+		data->select_timeout = SELECT_DELAY_LONG * 2;
+
+		return 0;
+	}
+
+	if (pca9641_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;
+		pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
+
+		return 1;
+	}
+
+	if (pca9641_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;
+		pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
+	}
+
+	return 0;
+}
+
 static int pca9541_select_chan(struct i2c_mux_core *muxc, u32 chan)
 {
 	struct pca9541 *data = i2c_mux_priv(muxc);
@@ -295,10 +418,15 @@ static const struct chip_desc chips[] = {
 		.arbitrate = pca9541_arbitrate,
 		.release_bus = pca9541_release_bus,
 	},
+	[pca9641] = {
+		.arbitrate = pca9641_arbitrate,
+		.release_bus = pca9641_release_bus,
+	},
 };
 
 static const struct i2c_device_id pca9541_id[] = {
 	{ "pca9541", pca9541 },
+	{ "pca9641", pca9641 },
 	{}
 };
 
@@ -307,6 +435,7 @@ MODULE_DEVICE_TABLE(i2c, pca9541_id);
 #ifdef CONFIG_OF
 static const struct of_device_id pca9541_of_match[] = {
 	{ .compatible = "nxp,pca9541", .data = &chips[pca9541] },
+	{ .compatible = "nxp,pca9641", .data = &chips[pca9641] },
 	{}
 };
 MODULE_DEVICE_TABLE(of, pca9541_of_match);
@@ -392,5 +521,5 @@ static struct i2c_driver pca9541_driver = {
 module_i2c_driver(pca9541_driver);
 
 MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net>");
-MODULE_DESCRIPTION("PCA9541 I2C master selector driver");
+MODULE_DESCRIPTION("PCA9541/PCA9641 I2C master selector driver");
 MODULE_LICENSE("GPL v2");
-- 
2.11.0

Re: [PATCH v2 5/5] i2c: mux: pca9541: add support for PCA9641

[Peter Rosin]

Hi!

I should have read Kens code more carefully, before signing off on it...

Review comments inline...

On 2019-03-07 00:15, Peter Rosin wrote:
> Heavily based on code from Ken Chen <chen.kenyy@inventec.com>.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  drivers/i2c/muxes/Kconfig           |   6 +-
>  drivers/i2c/muxes/i2c-mux-pca9541.c | 137 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 136 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> index 52a4a922e7e6..8532841de5db 100644
> --- a/drivers/i2c/muxes/Kconfig
> +++ b/drivers/i2c/muxes/Kconfig
> @@ -55,10 +55,10 @@ config I2C_MUX_LTC4306
>  	  will be called i2c-mux-ltc4306.
>  
>  config I2C_MUX_PCA9541
> -	tristate "NXP PCA9541 I2C Master Selector"
> +	tristate "NXP PCA9541/PCA9641 I2C Master Selectors"
>  	help
> -	  If you say yes here you get support for the NXP PCA9541
> -	  I2C Master Selector.
> +	  If you say yes here you get support for the NXP PCA9541/PCA9641
> +	  I2C Master Selectors.
>  
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called i2c-mux-pca9541.
> diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
> index 5eb36e3223d5..5d4e0c92e978 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
> @@ -1,5 +1,5 @@
>  /*
> - * I2C multiplexer driver for PCA9541 bus master selector
> + * I2C multiplexer driver for PCA9541/PCA9641 bus master selectors
>   *
>   * Copyright (c) 2010 Ericsson AB.
>   *
> @@ -28,8 +28,8 @@
>  #include <linux/slab.h>
>  
>  /*
> - * The PCA9541 is a bus master selector. It supports two I2C masters connected
> - * to a single slave bus.
> + * The PCA9541 and PCA9641 are bus master selector. They support two I2C masters
> + * connected to a single slave bus.
>   *
>   * Before each bus transaction, a master has to acquire bus ownership. After the
>   * transaction is complete, bus ownership has to be released. This fits well
> @@ -63,6 +63,33 @@
>  #define PCA9541_BUSON	(PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
>  #define PCA9541_MYBUS	(PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
>  
> +#define PCA9641_ID			0x00
> +#define PCA9641_ID_MAGIC		0x38
> +
> +#define PCA9641_CONTROL			0x01
> +#define PCA9641_STATUS			0x02
> +#define PCA9641_TIME			0x03
> +
> +#define PCA9641_CTL_LOCK_REQ		BIT(0)
> +#define PCA9641_CTL_LOCK_GRANT		BIT(1)
> +#define PCA9641_CTL_BUS_CONNECT		BIT(2)
> +#define PCA9641_CTL_BUS_INIT		BIT(3)
> +#define PCA9641_CTL_SMBUS_SWRST		BIT(4)
> +#define PCA9641_CTL_IDLE_TIMER_DIS	BIT(5)
> +#define PCA9641_CTL_SMBUS_DIS		BIT(6)
> +#define PCA9641_CTL_PRIORITY		BIT(7)
> +
> +#define PCA9641_STS_OTHER_LOCK		BIT(0)
> +#define PCA9641_STS_BUS_INIT_FAIL	BIT(1)
> +#define PCA9641_STS_BUS_HUNG		BIT(2)
> +#define PCA9641_STS_MBOX_EMPTY		BIT(3)
> +#define PCA9641_STS_MBOX_FULL		BIT(4)
> +#define PCA9641_STS_TEST_INT		BIT(5)
> +#define PCA9641_STS_SCL_IO		BIT(6)
> +#define PCA9641_STS_SDA_IO		BIT(7)
> +
> +#define PCA9641_RES_TIME		0x03

This appears to be the same thing as PCA9641_TIME above. The
register is called PCA9641_RT in my data sheet.

> +
>  /* 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 */
> @@ -73,6 +100,7 @@
>  
>  enum chip_name {
>  	pca9541,
> +	pca9641,
>  };
>  
>  struct chip_desc {
> @@ -102,6 +130,21 @@ static bool pca9541_busoff(int ctl)
>  	return (ctl & PCA9541_BUSON) == PCA9541_BUSON;
>  }
>  
> +static bool pca9641_lock_grant(int ctl)
> +{
> +	return !!(ctl & PCA9641_CTL_LOCK_GRANT);
> +}
> +
> +static bool pca9641_other_lock(int sts)
> +{
> +	return !!(sts & PCA9641_STS_OTHER_LOCK);
> +}
> +
> +static bool pca9641_busoff(int ctl, int sts)
> +{
> +	return !pca9641_lock_grant(ctl) && !pca9641_other_lock(sts);
> +}
> +
>  /*
>   * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
>   * as they will try to lock the adapter a second time.
> @@ -256,6 +299,86 @@ static int pca9541_arbitrate(struct i2c_client *client)
>  	return 0;
>  }
>  
> +/* Release bus. */
> +static void pca9641_release_bus(struct i2c_client *client)
> +{
> +	pca9541_reg_write(client, PCA9641_CONTROL, 0);

Should this release bus function really "clobber" the control bits
PCA9641_CTL_IDLE_TIMER_DIS, PCA9641_CTL_SMBUS_DIS, PCA9641_CTL_PRIORITY?
Yes yes, the driver never sets these bits so they are likely zero. But
the driver doesn't reset the chip either, so some bootstrap code might
have configured those bits...

Also related to bus release, since the driver does not touch the
reserve time register, and then clears the above bits, the only way
to release the bus is if everything continues to work and the above
pca9641_release_bus is in fact happening. But if the kernel crashes
while hogging the bus, and fails to come up, then the other master
has no way of stealing the ownership. I really feel that the driver
should make use of the timers so that the arbiter releases the bus
automatically on catastrophic failure. But maybe I plain and simple
just misunderstand the datasheet?

> +}
> +
> +/*
> + * 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 pca9541 *data = i2c_mux_priv(muxc);
> +	int reg_ctl, reg_sts;
> +
> +	reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
> +	if (reg_ctl < 0)
> +		return reg_ctl;
> +	reg_sts = pca9541_reg_read(client, PCA9641_STATUS);
> +
> +	if (pca9641_busoff(reg_ctl, reg_sts)) {
> +		/*
> +		 * Bus is off. Request ownership or turn it on unless
> +		 * other master requested ownership.
> +		 */
> +		reg_ctl |= PCA9641_CTL_LOCK_REQ;
> +		pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> +		reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
> +
> +		if (pca9641_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;
> +			pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> +			data->select_timeout = SELECT_DELAY_SHORT;
> +
> +			return 1;
> +		}
> +
> +		/*
> +		 * Other master requested ownership.
> +		 * Set extra long timeout to give it time to acquire it.
> +		 */
> +		data->select_timeout = SELECT_DELAY_LONG * 2;
> +
> +		return 0;
> +	}
> +
> +	if (pca9641_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;
> +		pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> +
> +		return 1;
> +	}
> +
> +	if (pca9641_other_lock(reg_sts)) {
> +		/*
> +		 * Other master owns the bus.
> +		 * If arbitration timeout has expired, force ownership.
> +		 * Otherwise request it.

This comment is stale. Reading the data sheet, I find no way to force
ownership with the PCA9641 (as indicated above in the release_bus
review comment). But I have only browsed the data sheet so I could
easily be mistaken...

[time passes]

Ahhh, wait, it could reset the chip to get a new chance to get ownership.
But that will reset all registers for the other master as well, since I
read it as if the reset is chip-global and not master-local with minimal
effects on the other master. So, a big hammer indeed.

Cheers,
Peter

> +		 */
> +		data->select_timeout = SELECT_DELAY_LONG;
> +		reg_ctl |= PCA9641_CTL_LOCK_REQ;
> +		pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> +	}
> +
> +	return 0;
> +}
> +
>  static int pca9541_select_chan(struct i2c_mux_core *muxc, u32 chan)
>  {
>  	struct pca9541 *data = i2c_mux_priv(muxc);
> @@ -295,10 +418,15 @@ static const struct chip_desc chips[] = {
>  		.arbitrate = pca9541_arbitrate,
>  		.release_bus = pca9541_release_bus,
>  	},
> +	[pca9641] = {
> +		.arbitrate = pca9641_arbitrate,
> +		.release_bus = pca9641_release_bus,
> +	},
>  };
>  
>  static const struct i2c_device_id pca9541_id[] = {
>  	{ "pca9541", pca9541 },
> +	{ "pca9641", pca9641 },
>  	{}
>  };
>  
> @@ -307,6 +435,7 @@ MODULE_DEVICE_TABLE(i2c, pca9541_id);
>  #ifdef CONFIG_OF
>  static const struct of_device_id pca9541_of_match[] = {
>  	{ .compatible = "nxp,pca9541", .data = &chips[pca9541] },
> +	{ .compatible = "nxp,pca9641", .data = &chips[pca9641] },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, pca9541_of_match);
> @@ -392,5 +521,5 @@ static struct i2c_driver pca9541_driver = {
>  module_i2c_driver(pca9541_driver);
>  
>  MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net>");
> -MODULE_DESCRIPTION("PCA9541 I2C master selector driver");
> +MODULE_DESCRIPTION("PCA9541/PCA9641 I2C master selector driver");
>  MODULE_LICENSE("GPL v2");
> 

Re: [PATCH v2 5/5] i2c: mux: pca9541: add support for PCA9641

[Pradeep Srinivasan]

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

I have verified the changes on PCA 9541. May I know how you want the test
results to be shared ? (newbie here; please bear with me)

root@cumulus:/home/cumulus# dmesg| grep "pca9541" | grep -v "pmbus"
[    2.922288] pca9541 1-0070: registered master selector for I2C pca9541

root@cumulus:/home/cumulus# cat /sys/class/i2c-dev/i2c-1/device/1-0070/name
pca9541

I need to do the same on PCA 9641. If the above is sufficient, I will grab
a switch with PCA 9641 and check if the driver works .


Thanks
Pradeep

On Thu, Mar 7, 2019 at 1:16 PM Peter Rosin <peda@axentia.se> wrote:

> Hi!
>
> I should have read Kens code more carefully, before signing off on it...
>
> Review comments inline...
>
> On 2019-03-07 00:15, Peter Rosin wrote:
> > Heavily based on code from Ken Chen <chen.kenyy@inventec.com>.
> >
> > Signed-off-by: Peter Rosin <peda@axentia.se>
> > ---
> >  drivers/i2c/muxes/Kconfig           |   6 +-
> >  drivers/i2c/muxes/i2c-mux-pca9541.c | 137
> ++++++++++++++++++++++++++++++++++--
> >  2 files changed, 136 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> > index 52a4a922e7e6..8532841de5db 100644
> > --- a/drivers/i2c/muxes/Kconfig
> > +++ b/drivers/i2c/muxes/Kconfig
> > @@ -55,10 +55,10 @@ config I2C_MUX_LTC4306
> >         will be called i2c-mux-ltc4306.
> >
> >  config I2C_MUX_PCA9541
> > -     tristate "NXP PCA9541 I2C Master Selector"
> > +     tristate "NXP PCA9541/PCA9641 I2C Master Selectors"
> >       help
> > -       If you say yes here you get support for the NXP PCA9541
> > -       I2C Master Selector.
> > +       If you say yes here you get support for the NXP PCA9541/PCA9641
> > +       I2C Master Selectors.
> >
> >         This driver can also be built as a module.  If so, the module
> >         will be called i2c-mux-pca9541.
> > diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c
> b/drivers/i2c/muxes/i2c-mux-pca9541.c
> > index 5eb36e3223d5..5d4e0c92e978 100644
> > --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
> > +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * I2C multiplexer driver for PCA9541 bus master selector
> > + * I2C multiplexer driver for PCA9541/PCA9641 bus master selectors
> >   *
> >   * Copyright (c) 2010 Ericsson AB.
> >   *
> > @@ -28,8 +28,8 @@
> >  #include <linux/slab.h>
> >
> >  /*
> > - * The PCA9541 is a bus master selector. It supports two I2C masters
> connected
> > - * to a single slave bus.
> > + * The PCA9541 and PCA9641 are bus master selector. They support two
> I2C masters
> > + * connected to a single slave bus.
> >   *
> >   * Before each bus transaction, a master has to acquire bus ownership.
> After the
> >   * transaction is complete, bus ownership has to be released. This fits
> well
> > @@ -63,6 +63,33 @@
> >  #define PCA9541_BUSON        (PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
> >  #define PCA9541_MYBUS        (PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
> >
> > +#define PCA9641_ID                   0x00
> > +#define PCA9641_ID_MAGIC             0x38
> > +
> > +#define PCA9641_CONTROL                      0x01
> > +#define PCA9641_STATUS                       0x02
> > +#define PCA9641_TIME                 0x03
> > +
> > +#define PCA9641_CTL_LOCK_REQ         BIT(0)
> > +#define PCA9641_CTL_LOCK_GRANT               BIT(1)
> > +#define PCA9641_CTL_BUS_CONNECT              BIT(2)
> > +#define PCA9641_CTL_BUS_INIT         BIT(3)
> > +#define PCA9641_CTL_SMBUS_SWRST              BIT(4)
> > +#define PCA9641_CTL_IDLE_TIMER_DIS   BIT(5)
> > +#define PCA9641_CTL_SMBUS_DIS                BIT(6)
> > +#define PCA9641_CTL_PRIORITY         BIT(7)
> > +
> > +#define PCA9641_STS_OTHER_LOCK               BIT(0)
> > +#define PCA9641_STS_BUS_INIT_FAIL    BIT(1)
> > +#define PCA9641_STS_BUS_HUNG         BIT(2)
> > +#define PCA9641_STS_MBOX_EMPTY               BIT(3)
> > +#define PCA9641_STS_MBOX_FULL                BIT(4)
> > +#define PCA9641_STS_TEST_INT         BIT(5)
> > +#define PCA9641_STS_SCL_IO           BIT(6)
> > +#define PCA9641_STS_SDA_IO           BIT(7)
> > +
> > +#define PCA9641_RES_TIME             0x03
>
> This appears to be the same thing as PCA9641_TIME above. The
> register is called PCA9641_RT in my data sheet.
>
> > +
> >  /* 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 */
> > @@ -73,6 +100,7 @@
> >
> >  enum chip_name {
> >       pca9541,
> > +     pca9641,
> >  };
> >
> >  struct chip_desc {
> > @@ -102,6 +130,21 @@ static bool pca9541_busoff(int ctl)
> >       return (ctl & PCA9541_BUSON) == PCA9541_BUSON;
> >  }
> >
> > +static bool pca9641_lock_grant(int ctl)
> > +{
> > +     return !!(ctl & PCA9641_CTL_LOCK_GRANT);
> > +}
> > +
> > +static bool pca9641_other_lock(int sts)
> > +{
> > +     return !!(sts & PCA9641_STS_OTHER_LOCK);
> > +}
> > +
> > +static bool pca9641_busoff(int ctl, int sts)
> > +{
> > +     return !pca9641_lock_grant(ctl) && !pca9641_other_lock(sts);
> > +}
> > +
> >  /*
> >   * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
> >   * as they will try to lock the adapter a second time.
> > @@ -256,6 +299,86 @@ static int pca9541_arbitrate(struct i2c_client
> *client)
> >       return 0;
> >  }
> >
> > +/* Release bus. */
> > +static void pca9641_release_bus(struct i2c_client *client)
> > +{
> > +     pca9541_reg_write(client, PCA9641_CONTROL, 0);
>
> Should this release bus function really "clobber" the control bits
> PCA9641_CTL_IDLE_TIMER_DIS, PCA9641_CTL_SMBUS_DIS, PCA9641_CTL_PRIORITY?
> Yes yes, the driver never sets these bits so they are likely zero. But
> the driver doesn't reset the chip either, so some bootstrap code might
> have configured those bits...
>
> Also related to bus release, since the driver does not touch the
> reserve time register, and then clears the above bits, the only way
> to release the bus is if everything continues to work and the above
> pca9641_release_bus is in fact happening. But if the kernel crashes
> while hogging the bus, and fails to come up, then the other master
> has no way of stealing the ownership. I really feel that the driver
> should make use of the timers so that the arbiter releases the bus
> automatically on catastrophic failure. But maybe I plain and simple
> just misunderstand the datasheet?
>
> > +}
> > +
> > +/*
> > + * 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 pca9541 *data = i2c_mux_priv(muxc);
> > +     int reg_ctl, reg_sts;
> > +
> > +     reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
> > +     if (reg_ctl < 0)
> > +             return reg_ctl;
> > +     reg_sts = pca9541_reg_read(client, PCA9641_STATUS);
> > +
> > +     if (pca9641_busoff(reg_ctl, reg_sts)) {
> > +             /*
> > +              * Bus is off. Request ownership or turn it on unless
> > +              * other master requested ownership.
> > +              */
> > +             reg_ctl |= PCA9641_CTL_LOCK_REQ;
> > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> > +             reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
> > +
> > +             if (pca9641_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;
> > +                     pca9541_reg_write(client, PCA9641_CONTROL,
> reg_ctl);
> > +                     data->select_timeout = SELECT_DELAY_SHORT;
> > +
> > +                     return 1;
> > +             }
> > +
> > +             /*
> > +              * Other master requested ownership.
> > +              * Set extra long timeout to give it time to acquire it.
> > +              */
> > +             data->select_timeout = SELECT_DELAY_LONG * 2;
> > +
> > +             return 0;
> > +     }
> > +
> > +     if (pca9641_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;
> > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> > +
> > +             return 1;
> > +     }
> > +
> > +     if (pca9641_other_lock(reg_sts)) {
> > +             /*
> > +              * Other master owns the bus.
> > +              * If arbitration timeout has expired, force ownership.
> > +              * Otherwise request it.
>
> This comment is stale. Reading the data sheet, I find no way to force
> ownership with the PCA9641 (as indicated above in the release_bus
> review comment). But I have only browsed the data sheet so I could
> easily be mistaken...
>
> [time passes]
>
> Ahhh, wait, it could reset the chip to get a new chance to get ownership.
> But that will reset all registers for the other master as well, since I
> read it as if the reset is chip-global and not master-local with minimal
> effects on the other master. So, a big hammer indeed.
>
> Cheers,
> Peter
>
> > +              */
> > +             data->select_timeout = SELECT_DELAY_LONG;
> > +             reg_ctl |= PCA9641_CTL_LOCK_REQ;
> > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  static int pca9541_select_chan(struct i2c_mux_core *muxc, u32 chan)
> >  {
> >       struct pca9541 *data = i2c_mux_priv(muxc);
> > @@ -295,10 +418,15 @@ static const struct chip_desc chips[] = {
> >               .arbitrate = pca9541_arbitrate,
> >               .release_bus = pca9541_release_bus,
> >       },
> > +     [pca9641] = {
> > +             .arbitrate = pca9641_arbitrate,
> > +             .release_bus = pca9641_release_bus,
> > +     },
> >  };
> >
> >  static const struct i2c_device_id pca9541_id[] = {
> >       { "pca9541", pca9541 },
> > +     { "pca9641", pca9641 },
> >       {}
> >  };
> >
> > @@ -307,6 +435,7 @@ MODULE_DEVICE_TABLE(i2c, pca9541_id);
> >  #ifdef CONFIG_OF
> >  static const struct of_device_id pca9541_of_match[] = {
> >       { .compatible = "nxp,pca9541", .data = &chips[pca9541] },
> > +     { .compatible = "nxp,pca9641", .data = &chips[pca9641] },
> >       {}
> >  };
> >  MODULE_DEVICE_TABLE(of, pca9541_of_match);
> > @@ -392,5 +521,5 @@ static struct i2c_driver pca9541_driver = {
> >  module_i2c_driver(pca9541_driver);
> >
> >  MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net>");
> > -MODULE_DESCRIPTION("PCA9541 I2C master selector driver");
> > +MODULE_DESCRIPTION("PCA9541/PCA9641 I2C master selector driver");
> >  MODULE_LICENSE("GPL v2");
> >
>
>

[-- Attachment #2: Type: text/html, Size: 14174 bytes --]

Re: [PATCH v2 5/5] i2c: mux: pca9541: add support for PCA9641

[Peter Rosin]

On 2019-03-22 20:38, Pradeep Srinivasan wrote:
> I have verified the changes on PCA 9541. May I know how you want the test results to be shared ? (newbie here; please bear with me)
> 
> root@cumulus:/home/cumulus# dmesg| grep "pca9541" | grep -v "pmbus"
> [    2.922288] pca9541 1-0070: registered master selector for I2C pca9541
> 
> root@cumulus:/home/cumulus# cat /sys/class/i2c-dev/i2c-1/device/1-0070/name
> pca9541

This only verifies that the probe works and that the chip is detected properly.
It says nothing about if it works to communicate with whatever is beyond the
PCA9541, and nothing on how the interaction with the "alien" other master
connected to the PCA9541 is working. I don't know how I want this to be tested,
but if you have a setup with a PCA9541 / PCA9641 I would assume that you
have some kind of need for those chips and that you at least could report
if basic xfers through them are working? I don't need to see actual commands
that you have executed, I'm much more interested in some summary of what
you did and what worked (or not).

E.g. if you have an eeprom beyond the master selector, you could read from
it in a loop while doing something else from the alien master and check if
it all works as expected? Perhaps try to verify timing if there are stalls
and/or timeouts etc. Go wild! But if you don't know how or don't have the
time, I'd be happy with a report on basic functionality (but a little bit
more than probe-ok would be nice though), because the code affecting the
PCA9541 is probably not broken subtly, it either works as it did before or
it doesn't work at all. And any problem with the PCA9641 side of things
will not be a regression and therefore not a big problem...

Cheers,
Peter

> I need to do the same on PCA 9641. If the above is sufficient, I will grab a switch with PCA 9641 and check if the driver works .
> 
> 
> Thanks
> Pradeep
> 
> On Thu, Mar 7, 2019 at 1:16 PM Peter Rosin <peda@axentia.se <mailto:peda@axentia.se>> wrote:
> 
>     Hi!
> 
>     I should have read Kens code more carefully, before signing off on it...
> 
>     Review comments inline...
> 
>     On 2019-03-07 00:15, Peter Rosin wrote:
>     > Heavily based on code from Ken Chen <chen.kenyy@inventec.com <mailto:chen.kenyy@inventec.com>>.
>     >
>     > Signed-off-by: Peter Rosin <peda@axentia.se <mailto:peda@axentia.se>>
>     > ---
>     >  drivers/i2c/muxes/Kconfig           |   6 +-
>     >  drivers/i2c/muxes/i2c-mux-pca9541.c | 137 ++++++++++++++++++++++++++++++++++--
>     >  2 files changed, 136 insertions(+), 7 deletions(-)
>     >
>     > diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
>     > index 52a4a922e7e6..8532841de5db 100644
>     > --- a/drivers/i2c/muxes/Kconfig
>     > +++ b/drivers/i2c/muxes/Kconfig
>     > @@ -55,10 +55,10 @@ config I2C_MUX_LTC4306
>     >         will be called i2c-mux-ltc4306.
>     > 
>     >  config I2C_MUX_PCA9541
>     > -     tristate "NXP PCA9541 I2C Master Selector"
>     > +     tristate "NXP PCA9541/PCA9641 I2C Master Selectors"
>     >       help
>     > -       If you say yes here you get support for the NXP PCA9541
>     > -       I2C Master Selector.
>     > +       If you say yes here you get support for the NXP PCA9541/PCA9641
>     > +       I2C Master Selectors.
>     > 
>     >         This driver can also be built as a module.  If so, the module
>     >         will be called i2c-mux-pca9541.
>     > diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
>     > index 5eb36e3223d5..5d4e0c92e978 100644
>     > --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
>     > +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
>     > @@ -1,5 +1,5 @@
>     >  /*
>     > - * I2C multiplexer driver for PCA9541 bus master selector
>     > + * I2C multiplexer driver for PCA9541/PCA9641 bus master selectors
>     >   *
>     >   * Copyright (c) 2010 Ericsson AB.
>     >   *
>     > @@ -28,8 +28,8 @@
>     >  #include <linux/slab.h>
>     > 
>     >  /*
>     > - * The PCA9541 is a bus master selector. It supports two I2C masters connected
>     > - * to a single slave bus.
>     > + * The PCA9541 and PCA9641 are bus master selector. They support two I2C masters
>     > + * connected to a single slave bus.
>     >   *
>     >   * Before each bus transaction, a master has to acquire bus ownership. After the
>     >   * transaction is complete, bus ownership has to be released. This fits well
>     > @@ -63,6 +63,33 @@
>     >  #define PCA9541_BUSON        (PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
>     >  #define PCA9541_MYBUS        (PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
>     > 
>     > +#define PCA9641_ID                   0x00
>     > +#define PCA9641_ID_MAGIC             0x38
>     > +
>     > +#define PCA9641_CONTROL                      0x01
>     > +#define PCA9641_STATUS                       0x02
>     > +#define PCA9641_TIME                 0x03
>     > +
>     > +#define PCA9641_CTL_LOCK_REQ         BIT(0)
>     > +#define PCA9641_CTL_LOCK_GRANT               BIT(1)
>     > +#define PCA9641_CTL_BUS_CONNECT              BIT(2)
>     > +#define PCA9641_CTL_BUS_INIT         BIT(3)
>     > +#define PCA9641_CTL_SMBUS_SWRST              BIT(4)
>     > +#define PCA9641_CTL_IDLE_TIMER_DIS   BIT(5)
>     > +#define PCA9641_CTL_SMBUS_DIS                BIT(6)
>     > +#define PCA9641_CTL_PRIORITY         BIT(7)
>     > +
>     > +#define PCA9641_STS_OTHER_LOCK               BIT(0)
>     > +#define PCA9641_STS_BUS_INIT_FAIL    BIT(1)
>     > +#define PCA9641_STS_BUS_HUNG         BIT(2)
>     > +#define PCA9641_STS_MBOX_EMPTY               BIT(3)
>     > +#define PCA9641_STS_MBOX_FULL                BIT(4)
>     > +#define PCA9641_STS_TEST_INT         BIT(5)
>     > +#define PCA9641_STS_SCL_IO           BIT(6)
>     > +#define PCA9641_STS_SDA_IO           BIT(7)
>     > +
>     > +#define PCA9641_RES_TIME             0x03
> 
>     This appears to be the same thing as PCA9641_TIME above. The
>     register is called PCA9641_RT in my data sheet.
> 
>     > +
>     >  /* 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 */
>     > @@ -73,6 +100,7 @@
>     > 
>     >  enum chip_name {
>     >       pca9541,
>     > +     pca9641,
>     >  };
>     > 
>     >  struct chip_desc {
>     > @@ -102,6 +130,21 @@ static bool pca9541_busoff(int ctl)
>     >       return (ctl & PCA9541_BUSON) == PCA9541_BUSON;
>     >  }
>     > 
>     > +static bool pca9641_lock_grant(int ctl)
>     > +{
>     > +     return !!(ctl & PCA9641_CTL_LOCK_GRANT);
>     > +}
>     > +
>     > +static bool pca9641_other_lock(int sts)
>     > +{
>     > +     return !!(sts & PCA9641_STS_OTHER_LOCK);
>     > +}
>     > +
>     > +static bool pca9641_busoff(int ctl, int sts)
>     > +{
>     > +     return !pca9641_lock_grant(ctl) && !pca9641_other_lock(sts);
>     > +}
>     > +
>     >  /*
>     >   * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
>     >   * as they will try to lock the adapter a second time.
>     > @@ -256,6 +299,86 @@ static int pca9541_arbitrate(struct i2c_client *client)
>     >       return 0;
>     >  }
>     > 
>     > +/* Release bus. */
>     > +static void pca9641_release_bus(struct i2c_client *client)
>     > +{
>     > +     pca9541_reg_write(client, PCA9641_CONTROL, 0);
> 
>     Should this release bus function really "clobber" the control bits
>     PCA9641_CTL_IDLE_TIMER_DIS, PCA9641_CTL_SMBUS_DIS, PCA9641_CTL_PRIORITY?
>     Yes yes, the driver never sets these bits so they are likely zero. But
>     the driver doesn't reset the chip either, so some bootstrap code might
>     have configured those bits...
> 
>     Also related to bus release, since the driver does not touch the
>     reserve time register, and then clears the above bits, the only way
>     to release the bus is if everything continues to work and the above
>     pca9641_release_bus is in fact happening. But if the kernel crashes
>     while hogging the bus, and fails to come up, then the other master
>     has no way of stealing the ownership. I really feel that the driver
>     should make use of the timers so that the arbiter releases the bus
>     automatically on catastrophic failure. But maybe I plain and simple
>     just misunderstand the datasheet?
> 
>     > +}
>     > +
>     > +/*
>     > + * 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 pca9541 *data = i2c_mux_priv(muxc);
>     > +     int reg_ctl, reg_sts;
>     > +
>     > +     reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
>     > +     if (reg_ctl < 0)
>     > +             return reg_ctl;
>     > +     reg_sts = pca9541_reg_read(client, PCA9641_STATUS);
>     > +
>     > +     if (pca9641_busoff(reg_ctl, reg_sts)) {
>     > +             /*
>     > +              * Bus is off. Request ownership or turn it on unless
>     > +              * other master requested ownership.
>     > +              */
>     > +             reg_ctl |= PCA9641_CTL_LOCK_REQ;
>     > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
>     > +             reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
>     > +
>     > +             if (pca9641_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;
>     > +                     pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
>     > +                     data->select_timeout = SELECT_DELAY_SHORT;
>     > +
>     > +                     return 1;
>     > +             }
>     > +
>     > +             /*
>     > +              * Other master requested ownership.
>     > +              * Set extra long timeout to give it time to acquire it.
>     > +              */
>     > +             data->select_timeout = SELECT_DELAY_LONG * 2;
>     > +
>     > +             return 0;
>     > +     }
>     > +
>     > +     if (pca9641_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;
>     > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
>     > +
>     > +             return 1;
>     > +     }
>     > +
>     > +     if (pca9641_other_lock(reg_sts)) {
>     > +             /*
>     > +              * Other master owns the bus.
>     > +              * If arbitration timeout has expired, force ownership.
>     > +              * Otherwise request it.
> 
>     This comment is stale. Reading the data sheet, I find no way to force
>     ownership with the PCA9641 (as indicated above in the release_bus
>     review comment). But I have only browsed the data sheet so I could
>     easily be mistaken...
> 
>     [time passes]
> 
>     Ahhh, wait, it could reset the chip to get a new chance to get ownership.
>     But that will reset all registers for the other master as well, since I
>     read it as if the reset is chip-global and not master-local with minimal
>     effects on the other master. So, a big hammer indeed.
> 
>     Cheers,
>     Peter
> 
>     > +              */
>     > +             data->select_timeout = SELECT_DELAY_LONG;
>     > +             reg_ctl |= PCA9641_CTL_LOCK_REQ;
>     > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
>     > +     }
>     > +
>     > +     return 0;
>     > +}
>     > +
>     >  static int pca9541_select_chan(struct i2c_mux_core *muxc, u32 chan)
>     >  {
>     >       struct pca9541 *data = i2c_mux_priv(muxc);
>     > @@ -295,10 +418,15 @@ static const struct chip_desc chips[] = {
>     >               .arbitrate = pca9541_arbitrate,
>     >               .release_bus = pca9541_release_bus,
>     >       },
>     > +     [pca9641] = {
>     > +             .arbitrate = pca9641_arbitrate,
>     > +             .release_bus = pca9641_release_bus,
>     > +     },
>     >  };
>     > 
>     >  static const struct i2c_device_id pca9541_id[] = {
>     >       { "pca9541", pca9541 },
>     > +     { "pca9641", pca9641 },
>     >       {}
>     >  };
>     > 
>     > @@ -307,6 +435,7 @@ MODULE_DEVICE_TABLE(i2c, pca9541_id);
>     >  #ifdef CONFIG_OF
>     >  static const struct of_device_id pca9541_of_match[] = {
>     >       { .compatible = "nxp,pca9541", .data = &chips[pca9541] },
>     > +     { .compatible = "nxp,pca9641", .data = &chips[pca9641] },
>     >       {}
>     >  };
>     >  MODULE_DEVICE_TABLE(of, pca9541_of_match);
>     > @@ -392,5 +521,5 @@ static struct i2c_driver pca9541_driver = {
>     >  module_i2c_driver(pca9541_driver);
>     > 
>     >  MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>>");
>     > -MODULE_DESCRIPTION("PCA9541 I2C master selector driver");
>     > +MODULE_DESCRIPTION("PCA9541/PCA9641 I2C master selector driver");
>     >  MODULE_LICENSE("GPL v2");
>     >
> 

Re: [PATCH v2 5/5] i2c: mux: pca9541: add support for PCA9641

[Guenter Roeck]

On Mon, Mar 25, 2019 at 03:01:21PM +0000, Peter Rosin wrote:
> On 2019-03-22 20:38, Pradeep Srinivasan wrote:
> > I have verified the changes on PCA 9541. May I know how you want the test results to be shared ? (newbie here; please bear with me)
> > 
> > root@cumulus:/home/cumulus# dmesg| grep "pca9541" | grep -v "pmbus"
> > [    2.922288] pca9541 1-0070: registered master selector for I2C pca9541
> > 
> > root@cumulus:/home/cumulus# cat /sys/class/i2c-dev/i2c-1/device/1-0070/name
> > pca9541
> 
> This only verifies that the probe works and that the chip is detected properly.
> It says nothing about if it works to communicate with whatever is beyond the
> PCA9541, and nothing on how the interaction with the "alien" other master
> connected to the PCA9541 is working. I don't know how I want this to be tested,
> but if you have a setup with a PCA9541 / PCA9641 I would assume that you
> have some kind of need for those chips and that you at least could report
> if basic xfers through them are working? I don't need to see actual commands
> that you have executed, I'm much more interested in some summary of what
> you did and what worked (or not).
> 
> E.g. if you have an eeprom beyond the master selector, you could read from
> it in a loop while doing something else from the alien master and check if
> it all works as expected? Perhaps try to verify timing if there are stalls
> and/or timeouts etc. Go wild! But if you don't know how or don't have the

Something like that is what I did to test the original implementation: Access
all chips behind the mux from both ends continuously. Let that run for an hour
or so and declare it a success if there is no error. Usually, while the code
was still buggy, errors would show up within minutes, if not seconds.

Guenter

> time, I'd be happy with a report on basic functionality (but a little bit
> more than probe-ok would be nice though), because the code affecting the
> PCA9541 is probably not broken subtly, it either works as it did before or
> it doesn't work at all. And any problem with the PCA9641 side of things
> will not be a regression and therefore not a big problem...
> 
> Cheers,
> Peter
> 
> > I need to do the same on PCA 9641. If the above is sufficient, I will grab a switch with PCA 9641 and check if the driver works .
> > 
> > 
> > Thanks
> > Pradeep
> > 
> > On Thu, Mar 7, 2019 at 1:16 PM Peter Rosin <peda@axentia.se <mailto:peda@axentia.se>> wrote:
> > 
> >     Hi!
> > 
> >     I should have read Kens code more carefully, before signing off on it...
> > 
> >     Review comments inline...
> > 
> >     On 2019-03-07 00:15, Peter Rosin wrote:
> >     > Heavily based on code from Ken Chen <chen.kenyy@inventec.com <mailto:chen.kenyy@inventec.com>>.
> >     >
> >     > Signed-off-by: Peter Rosin <peda@axentia.se <mailto:peda@axentia.se>>
> >     > ---
> >     >  drivers/i2c/muxes/Kconfig           |   6 +-
> >     >  drivers/i2c/muxes/i2c-mux-pca9541.c | 137 ++++++++++++++++++++++++++++++++++--
> >     >  2 files changed, 136 insertions(+), 7 deletions(-)
> >     >
> >     > diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> >     > index 52a4a922e7e6..8532841de5db 100644
> >     > --- a/drivers/i2c/muxes/Kconfig
> >     > +++ b/drivers/i2c/muxes/Kconfig
> >     > @@ -55,10 +55,10 @@ config I2C_MUX_LTC4306
> >     >         will be called i2c-mux-ltc4306.
> >     > 
> >     >  config I2C_MUX_PCA9541
> >     > -     tristate "NXP PCA9541 I2C Master Selector"
> >     > +     tristate "NXP PCA9541/PCA9641 I2C Master Selectors"
> >     >       help
> >     > -       If you say yes here you get support for the NXP PCA9541
> >     > -       I2C Master Selector.
> >     > +       If you say yes here you get support for the NXP PCA9541/PCA9641
> >     > +       I2C Master Selectors.
> >     > 
> >     >         This driver can also be built as a module.  If so, the module
> >     >         will be called i2c-mux-pca9541.
> >     > diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
> >     > index 5eb36e3223d5..5d4e0c92e978 100644
> >     > --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
> >     > +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
> >     > @@ -1,5 +1,5 @@
> >     >  /*
> >     > - * I2C multiplexer driver for PCA9541 bus master selector
> >     > + * I2C multiplexer driver for PCA9541/PCA9641 bus master selectors
> >     >   *
> >     >   * Copyright (c) 2010 Ericsson AB.
> >     >   *
> >     > @@ -28,8 +28,8 @@
> >     >  #include <linux/slab.h>
> >     > 
> >     >  /*
> >     > - * The PCA9541 is a bus master selector. It supports two I2C masters connected
> >     > - * to a single slave bus.
> >     > + * The PCA9541 and PCA9641 are bus master selector. They support two I2C masters
> >     > + * connected to a single slave bus.
> >     >   *
> >     >   * Before each bus transaction, a master has to acquire bus ownership. After the
> >     >   * transaction is complete, bus ownership has to be released. This fits well
> >     > @@ -63,6 +63,33 @@
> >     >  #define PCA9541_BUSON        (PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
> >     >  #define PCA9541_MYBUS        (PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
> >     > 
> >     > +#define PCA9641_ID                   0x00
> >     > +#define PCA9641_ID_MAGIC             0x38
> >     > +
> >     > +#define PCA9641_CONTROL                      0x01
> >     > +#define PCA9641_STATUS                       0x02
> >     > +#define PCA9641_TIME                 0x03
> >     > +
> >     > +#define PCA9641_CTL_LOCK_REQ         BIT(0)
> >     > +#define PCA9641_CTL_LOCK_GRANT               BIT(1)
> >     > +#define PCA9641_CTL_BUS_CONNECT              BIT(2)
> >     > +#define PCA9641_CTL_BUS_INIT         BIT(3)
> >     > +#define PCA9641_CTL_SMBUS_SWRST              BIT(4)
> >     > +#define PCA9641_CTL_IDLE_TIMER_DIS   BIT(5)
> >     > +#define PCA9641_CTL_SMBUS_DIS                BIT(6)
> >     > +#define PCA9641_CTL_PRIORITY         BIT(7)
> >     > +
> >     > +#define PCA9641_STS_OTHER_LOCK               BIT(0)
> >     > +#define PCA9641_STS_BUS_INIT_FAIL    BIT(1)
> >     > +#define PCA9641_STS_BUS_HUNG         BIT(2)
> >     > +#define PCA9641_STS_MBOX_EMPTY               BIT(3)
> >     > +#define PCA9641_STS_MBOX_FULL                BIT(4)
> >     > +#define PCA9641_STS_TEST_INT         BIT(5)
> >     > +#define PCA9641_STS_SCL_IO           BIT(6)
> >     > +#define PCA9641_STS_SDA_IO           BIT(7)
> >     > +
> >     > +#define PCA9641_RES_TIME             0x03
> > 
> >     This appears to be the same thing as PCA9641_TIME above. The
> >     register is called PCA9641_RT in my data sheet.
> > 
> >     > +
> >     >  /* 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 */
> >     > @@ -73,6 +100,7 @@
> >     > 
> >     >  enum chip_name {
> >     >       pca9541,
> >     > +     pca9641,
> >     >  };
> >     > 
> >     >  struct chip_desc {
> >     > @@ -102,6 +130,21 @@ static bool pca9541_busoff(int ctl)
> >     >       return (ctl & PCA9541_BUSON) == PCA9541_BUSON;
> >     >  }
> >     > 
> >     > +static bool pca9641_lock_grant(int ctl)
> >     > +{
> >     > +     return !!(ctl & PCA9641_CTL_LOCK_GRANT);
> >     > +}
> >     > +
> >     > +static bool pca9641_other_lock(int sts)
> >     > +{
> >     > +     return !!(sts & PCA9641_STS_OTHER_LOCK);
> >     > +}
> >     > +
> >     > +static bool pca9641_busoff(int ctl, int sts)
> >     > +{
> >     > +     return !pca9641_lock_grant(ctl) && !pca9641_other_lock(sts);
> >     > +}
> >     > +
> >     >  /*
> >     >   * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
> >     >   * as they will try to lock the adapter a second time.
> >     > @@ -256,6 +299,86 @@ static int pca9541_arbitrate(struct i2c_client *client)
> >     >       return 0;
> >     >  }
> >     > 
> >     > +/* Release bus. */
> >     > +static void pca9641_release_bus(struct i2c_client *client)
> >     > +{
> >     > +     pca9541_reg_write(client, PCA9641_CONTROL, 0);
> > 
> >     Should this release bus function really "clobber" the control bits
> >     PCA9641_CTL_IDLE_TIMER_DIS, PCA9641_CTL_SMBUS_DIS, PCA9641_CTL_PRIORITY?
> >     Yes yes, the driver never sets these bits so they are likely zero. But
> >     the driver doesn't reset the chip either, so some bootstrap code might
> >     have configured those bits...
> > 
> >     Also related to bus release, since the driver does not touch the
> >     reserve time register, and then clears the above bits, the only way
> >     to release the bus is if everything continues to work and the above
> >     pca9641_release_bus is in fact happening. But if the kernel crashes
> >     while hogging the bus, and fails to come up, then the other master
> >     has no way of stealing the ownership. I really feel that the driver
> >     should make use of the timers so that the arbiter releases the bus
> >     automatically on catastrophic failure. But maybe I plain and simple
> >     just misunderstand the datasheet?
> > 
> >     > +}
> >     > +
> >     > +/*
> >     > + * 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 pca9541 *data = i2c_mux_priv(muxc);
> >     > +     int reg_ctl, reg_sts;
> >     > +
> >     > +     reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
> >     > +     if (reg_ctl < 0)
> >     > +             return reg_ctl;
> >     > +     reg_sts = pca9541_reg_read(client, PCA9641_STATUS);
> >     > +
> >     > +     if (pca9641_busoff(reg_ctl, reg_sts)) {
> >     > +             /*
> >     > +              * Bus is off. Request ownership or turn it on unless
> >     > +              * other master requested ownership.
> >     > +              */
> >     > +             reg_ctl |= PCA9641_CTL_LOCK_REQ;
> >     > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> >     > +             reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
> >     > +
> >     > +             if (pca9641_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;
> >     > +                     pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> >     > +                     data->select_timeout = SELECT_DELAY_SHORT;
> >     > +
> >     > +                     return 1;
> >     > +             }
> >     > +
> >     > +             /*
> >     > +              * Other master requested ownership.
> >     > +              * Set extra long timeout to give it time to acquire it.
> >     > +              */
> >     > +             data->select_timeout = SELECT_DELAY_LONG * 2;
> >     > +
> >     > +             return 0;
> >     > +     }
> >     > +
> >     > +     if (pca9641_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;
> >     > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> >     > +
> >     > +             return 1;
> >     > +     }
> >     > +
> >     > +     if (pca9641_other_lock(reg_sts)) {
> >     > +             /*
> >     > +              * Other master owns the bus.
> >     > +              * If arbitration timeout has expired, force ownership.
> >     > +              * Otherwise request it.
> > 
> >     This comment is stale. Reading the data sheet, I find no way to force
> >     ownership with the PCA9641 (as indicated above in the release_bus
> >     review comment). But I have only browsed the data sheet so I could
> >     easily be mistaken...
> > 
> >     [time passes]
> > 
> >     Ahhh, wait, it could reset the chip to get a new chance to get ownership.
> >     But that will reset all registers for the other master as well, since I
> >     read it as if the reset is chip-global and not master-local with minimal
> >     effects on the other master. So, a big hammer indeed.
> > 
> >     Cheers,
> >     Peter
> > 
> >     > +              */
> >     > +             data->select_timeout = SELECT_DELAY_LONG;
> >     > +             reg_ctl |= PCA9641_CTL_LOCK_REQ;
> >     > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> >     > +     }
> >     > +
> >     > +     return 0;
> >     > +}
> >     > +
> >     >  static int pca9541_select_chan(struct i2c_mux_core *muxc, u32 chan)
> >     >  {
> >     >       struct pca9541 *data = i2c_mux_priv(muxc);
> >     > @@ -295,10 +418,15 @@ static const struct chip_desc chips[] = {
> >     >               .arbitrate = pca9541_arbitrate,
> >     >               .release_bus = pca9541_release_bus,
> >     >       },
> >     > +     [pca9641] = {
> >     > +             .arbitrate = pca9641_arbitrate,
> >     > +             .release_bus = pca9641_release_bus,
> >     > +     },
> >     >  };
> >     > 
> >     >  static const struct i2c_device_id pca9541_id[] = {
> >     >       { "pca9541", pca9541 },
> >     > +     { "pca9641", pca9641 },
> >     >       {}
> >     >  };
> >     > 
> >     > @@ -307,6 +435,7 @@ MODULE_DEVICE_TABLE(i2c, pca9541_id);
> >     >  #ifdef CONFIG_OF
> >     >  static const struct of_device_id pca9541_of_match[] = {
> >     >       { .compatible = "nxp,pca9541", .data = &chips[pca9541] },
> >     > +     { .compatible = "nxp,pca9641", .data = &chips[pca9641] },
> >     >       {}
> >     >  };
> >     >  MODULE_DEVICE_TABLE(of, pca9541_of_match);
> >     > @@ -392,5 +521,5 @@ static struct i2c_driver pca9541_driver = {
> >     >  module_i2c_driver(pca9541_driver);
> >     > 
> >     >  MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>>");
> >     > -MODULE_DESCRIPTION("PCA9541 I2C master selector driver");
> >     > +MODULE_DESCRIPTION("PCA9541/PCA9641 I2C master selector driver");
> >     >  MODULE_LICENSE("GPL v2");
> >     >
> > 
> 

Re: [PATCH v2 5/5] i2c: mux: pca9541: add support for PCA9641

[Pradeep Srinivasan]

[-- Attachment #1.1: Type: text/plain, Size: 15797 bytes --]

Finally, I got some cycles to test the patches on a Celestica seastone
switch which has the PCA 9541 mux. I have attached a rough sketch of the
I2C mux tree. I tested by accessing the all the devices behind PCA9548 mux.
There are 3 PCA 9548 mux devices behind PCA 9541. The test data is attached
to this email. I was able to read all Fan EEPROM data, lm74 sensor data
through smonctl as well as sensors command.

Thanks
Pradeep

On Mon, Mar 25, 2019 at 9:49 AM Guenter Roeck <linux@roeck-us.net> wrote:

> On Mon, Mar 25, 2019 at 03:01:21PM +0000, Peter Rosin wrote:
> > On 2019-03-22 20:38, Pradeep Srinivasan wrote:
> > > I have verified the changes on PCA 9541. May I know how you want the
> test results to be shared ? (newbie here; please bear with me)
> > >
> > > root@cumulus:/home/cumulus# dmesg| grep "pca9541" | grep -v "pmbus"
> > > [    2.922288] pca9541 1-0070: registered master selector for I2C
> pca9541
> > >
> > > root@cumulus:/home/cumulus# cat
> /sys/class/i2c-dev/i2c-1/device/1-0070/name
> > > pca9541
> >
> > This only verifies that the probe works and that the chip is detected
> properly.
> > It says nothing about if it works to communicate with whatever is beyond
> the
> > PCA9541, and nothing on how the interaction with the "alien" other master
> > connected to the PCA9541 is working. I don't know how I want this to be
> tested,
> > but if you have a setup with a PCA9541 / PCA9641 I would assume that you
> > have some kind of need for those chips and that you at least could report
> > if basic xfers through them are working? I don't need to see actual
> commands
> > that you have executed, I'm much more interested in some summary of what
> > you did and what worked (or not).
> >
> > E.g. if you have an eeprom beyond the master selector, you could read
> from
> > it in a loop while doing something else from the alien master and check
> if
> > it all works as expected? Perhaps try to verify timing if there are
> stalls
> > and/or timeouts etc. Go wild! But if you don't know how or don't have the
>
> Something like that is what I did to test the original implementation:
> Access
> all chips behind the mux from both ends continuously. Let that run for an
> hour
> or so and declare it a success if there is no error. Usually, while the
> code
> was still buggy, errors would show up within minutes, if not seconds.
>
> Guenter
>
> > time, I'd be happy with a report on basic functionality (but a little bit
> > more than probe-ok would be nice though), because the code affecting the
> > PCA9541 is probably not broken subtly, it either works as it did before
> or
> > it doesn't work at all. And any problem with the PCA9641 side of things
> > will not be a regression and therefore not a big problem...
> >
> > Cheers,
> > Peter
> >
> > > I need to do the same on PCA 9641. If the above is sufficient, I will
> grab a switch with PCA 9641 and check if the driver works .
> > >
> > >
> > > Thanks
> > > Pradeep
> > >
> > > On Thu, Mar 7, 2019 at 1:16 PM Peter Rosin <peda@axentia.se <mailto:
> peda@axentia.se>> wrote:
> > >
> > >     Hi!
> > >
> > >     I should have read Kens code more carefully, before signing off on
> it...
> > >
> > >     Review comments inline...
> > >
> > >     On 2019-03-07 00:15, Peter Rosin wrote:
> > >     > Heavily based on code from Ken Chen <chen.kenyy@inventec.com
> <mailto:chen.kenyy@inventec.com>>.
> > >     >
> > >     > Signed-off-by: Peter Rosin <peda@axentia.se <mailto:
> peda@axentia.se>>
> > >     > ---
> > >     >  drivers/i2c/muxes/Kconfig           |   6 +-
> > >     >  drivers/i2c/muxes/i2c-mux-pca9541.c | 137
> ++++++++++++++++++++++++++++++++++--
> > >     >  2 files changed, 136 insertions(+), 7 deletions(-)
> > >     >
> > >     > diff --git a/drivers/i2c/muxes/Kconfig
> b/drivers/i2c/muxes/Kconfig
> > >     > index 52a4a922e7e6..8532841de5db 100644
> > >     > --- a/drivers/i2c/muxes/Kconfig
> > >     > +++ b/drivers/i2c/muxes/Kconfig
> > >     > @@ -55,10 +55,10 @@ config I2C_MUX_LTC4306
> > >     >         will be called i2c-mux-ltc4306.
> > >     >
> > >     >  config I2C_MUX_PCA9541
> > >     > -     tristate "NXP PCA9541 I2C Master Selector"
> > >     > +     tristate "NXP PCA9541/PCA9641 I2C Master Selectors"
> > >     >       help
> > >     > -       If you say yes here you get support for the NXP PCA9541
> > >     > -       I2C Master Selector.
> > >     > +       If you say yes here you get support for the NXP
> PCA9541/PCA9641
> > >     > +       I2C Master Selectors.
> > >     >
> > >     >         This driver can also be built as a module.  If so, the
> module
> > >     >         will be called i2c-mux-pca9541.
> > >     > diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c
> b/drivers/i2c/muxes/i2c-mux-pca9541.c
> > >     > index 5eb36e3223d5..5d4e0c92e978 100644
> > >     > --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
> > >     > +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
> > >     > @@ -1,5 +1,5 @@
> > >     >  /*
> > >     > - * I2C multiplexer driver for PCA9541 bus master selector
> > >     > + * I2C multiplexer driver for PCA9541/PCA9641 bus master
> selectors
> > >     >   *
> > >     >   * Copyright (c) 2010 Ericsson AB.
> > >     >   *
> > >     > @@ -28,8 +28,8 @@
> > >     >  #include <linux/slab.h>
> > >     >
> > >     >  /*
> > >     > - * The PCA9541 is a bus master selector. It supports two I2C
> masters connected
> > >     > - * to a single slave bus.
> > >     > + * The PCA9541 and PCA9641 are bus master selector. They
> support two I2C masters
> > >     > + * connected to a single slave bus.
> > >     >   *
> > >     >   * Before each bus transaction, a master has to acquire bus
> ownership. After the
> > >     >   * transaction is complete, bus ownership has to be released.
> This fits well
> > >     > @@ -63,6 +63,33 @@
> > >     >  #define PCA9541_BUSON        (PCA9541_CTL_BUSON |
> PCA9541_CTL_NBUSON)
> > >     >  #define PCA9541_MYBUS        (PCA9541_CTL_MYBUS |
> PCA9541_CTL_NMYBUS)
> > >     >
> > >     > +#define PCA9641_ID                   0x00
> > >     > +#define PCA9641_ID_MAGIC             0x38
> > >     > +
> > >     > +#define PCA9641_CONTROL                      0x01
> > >     > +#define PCA9641_STATUS                       0x02
> > >     > +#define PCA9641_TIME                 0x03
> > >     > +
> > >     > +#define PCA9641_CTL_LOCK_REQ         BIT(0)
> > >     > +#define PCA9641_CTL_LOCK_GRANT               BIT(1)
> > >     > +#define PCA9641_CTL_BUS_CONNECT              BIT(2)
> > >     > +#define PCA9641_CTL_BUS_INIT         BIT(3)
> > >     > +#define PCA9641_CTL_SMBUS_SWRST              BIT(4)
> > >     > +#define PCA9641_CTL_IDLE_TIMER_DIS   BIT(5)
> > >     > +#define PCA9641_CTL_SMBUS_DIS                BIT(6)
> > >     > +#define PCA9641_CTL_PRIORITY         BIT(7)
> > >     > +
> > >     > +#define PCA9641_STS_OTHER_LOCK               BIT(0)
> > >     > +#define PCA9641_STS_BUS_INIT_FAIL    BIT(1)
> > >     > +#define PCA9641_STS_BUS_HUNG         BIT(2)
> > >     > +#define PCA9641_STS_MBOX_EMPTY               BIT(3)
> > >     > +#define PCA9641_STS_MBOX_FULL                BIT(4)
> > >     > +#define PCA9641_STS_TEST_INT         BIT(5)
> > >     > +#define PCA9641_STS_SCL_IO           BIT(6)
> > >     > +#define PCA9641_STS_SDA_IO           BIT(7)
> > >     > +
> > >     > +#define PCA9641_RES_TIME             0x03
> > >
> > >     This appears to be the same thing as PCA9641_TIME above. The
> > >     register is called PCA9641_RT in my data sheet.
> > >
> > >     > +
> > >     >  /* 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 */
> > >     > @@ -73,6 +100,7 @@
> > >     >
> > >     >  enum chip_name {
> > >     >       pca9541,
> > >     > +     pca9641,
> > >     >  };
> > >     >
> > >     >  struct chip_desc {
> > >     > @@ -102,6 +130,21 @@ static bool pca9541_busoff(int ctl)
> > >     >       return (ctl & PCA9541_BUSON) == PCA9541_BUSON;
> > >     >  }
> > >     >
> > >     > +static bool pca9641_lock_grant(int ctl)
> > >     > +{
> > >     > +     return !!(ctl & PCA9641_CTL_LOCK_GRANT);
> > >     > +}
> > >     > +
> > >     > +static bool pca9641_other_lock(int sts)
> > >     > +{
> > >     > +     return !!(sts & PCA9641_STS_OTHER_LOCK);
> > >     > +}
> > >     > +
> > >     > +static bool pca9641_busoff(int ctl, int sts)
> > >     > +{
> > >     > +     return !pca9641_lock_grant(ctl) &&
> !pca9641_other_lock(sts);
> > >     > +}
> > >     > +
> > >     >  /*
> > >     >   * Write to chip register. Don't use
> i2c_transfer()/i2c_smbus_xfer()
> > >     >   * as they will try to lock the adapter a second time.
> > >     > @@ -256,6 +299,86 @@ static int pca9541_arbitrate(struct
> i2c_client *client)
> > >     >       return 0;
> > >     >  }
> > >     >
> > >     > +/* Release bus. */
> > >     > +static void pca9641_release_bus(struct i2c_client *client)
> > >     > +{
> > >     > +     pca9541_reg_write(client, PCA9641_CONTROL, 0);
> > >
> > >     Should this release bus function really "clobber" the control bits
> > >     PCA9641_CTL_IDLE_TIMER_DIS, PCA9641_CTL_SMBUS_DIS,
> PCA9641_CTL_PRIORITY?
> > >     Yes yes, the driver never sets these bits so they are likely zero.
> But
> > >     the driver doesn't reset the chip either, so some bootstrap code
> might
> > >     have configured those bits...
> > >
> > >     Also related to bus release, since the driver does not touch the
> > >     reserve time register, and then clears the above bits, the only way
> > >     to release the bus is if everything continues to work and the above
> > >     pca9641_release_bus is in fact happening. But if the kernel crashes
> > >     while hogging the bus, and fails to come up, then the other master
> > >     has no way of stealing the ownership. I really feel that the driver
> > >     should make use of the timers so that the arbiter releases the bus
> > >     automatically on catastrophic failure. But maybe I plain and simple
> > >     just misunderstand the datasheet?
> > >
> > >     > +}
> > >     > +
> > >     > +/*
> > >     > + * 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 pca9541 *data = i2c_mux_priv(muxc);
> > >     > +     int reg_ctl, reg_sts;
> > >     > +
> > >     > +     reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
> > >     > +     if (reg_ctl < 0)
> > >     > +             return reg_ctl;
> > >     > +     reg_sts = pca9541_reg_read(client, PCA9641_STATUS);
> > >     > +
> > >     > +     if (pca9641_busoff(reg_ctl, reg_sts)) {
> > >     > +             /*
> > >     > +              * Bus is off. Request ownership or turn it on
> unless
> > >     > +              * other master requested ownership.
> > >     > +              */
> > >     > +             reg_ctl |= PCA9641_CTL_LOCK_REQ;
> > >     > +             pca9541_reg_write(client, PCA9641_CONTROL,
> reg_ctl);
> > >     > +             reg_ctl = pca9541_reg_read(client,
> PCA9641_CONTROL);
> > >     > +
> > >     > +             if (pca9641_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;
> > >     > +                     pca9541_reg_write(client, PCA9641_CONTROL,
> reg_ctl);
> > >     > +                     data->select_timeout = SELECT_DELAY_SHORT;
> > >     > +
> > >     > +                     return 1;
> > >     > +             }
> > >     > +
> > >     > +             /*
> > >     > +              * Other master requested ownership.
> > >     > +              * Set extra long timeout to give it time to
> acquire it.
> > >     > +              */
> > >     > +             data->select_timeout = SELECT_DELAY_LONG * 2;
> > >     > +
> > >     > +             return 0;
> > >     > +     }
> > >     > +
> > >     > +     if (pca9641_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;
> > >     > +             pca9541_reg_write(client, PCA9641_CONTROL,
> reg_ctl);
> > >     > +
> > >     > +             return 1;
> > >     > +     }
> > >     > +
> > >     > +     if (pca9641_other_lock(reg_sts)) {
> > >     > +             /*
> > >     > +              * Other master owns the bus.
> > >     > +              * If arbitration timeout has expired, force
> ownership.
> > >     > +              * Otherwise request it.
> > >
> > >     This comment is stale. Reading the data sheet, I find no way to
> force
> > >     ownership with the PCA9641 (as indicated above in the release_bus
> > >     review comment). But I have only browsed the data sheet so I could
> > >     easily be mistaken...
> > >
> > >     [time passes]
> > >
> > >     Ahhh, wait, it could reset the chip to get a new chance to get
> ownership.
> > >     But that will reset all registers for the other master as well,
> since I
> > >     read it as if the reset is chip-global and not master-local with
> minimal
> > >     effects on the other master. So, a big hammer indeed.
> > >
> > >     Cheers,
> > >     Peter
> > >
> > >     > +              */
> > >     > +             data->select_timeout = SELECT_DELAY_LONG;
> > >     > +             reg_ctl |= PCA9641_CTL_LOCK_REQ;
> > >     > +             pca9541_reg_write(client, PCA9641_CONTROL,
> reg_ctl);
> > >     > +     }
> > >     > +
> > >     > +     return 0;
> > >     > +}
> > >     > +
> > >     >  static int pca9541_select_chan(struct i2c_mux_core *muxc, u32
> chan)
> > >     >  {
> > >     >       struct pca9541 *data = i2c_mux_priv(muxc);
> > >     > @@ -295,10 +418,15 @@ static const struct chip_desc chips[] = {
> > >     >               .arbitrate = pca9541_arbitrate,
> > >     >               .release_bus = pca9541_release_bus,
> > >     >       },
> > >     > +     [pca9641] = {
> > >     > +             .arbitrate = pca9641_arbitrate,
> > >     > +             .release_bus = pca9641_release_bus,
> > >     > +     },
> > >     >  };
> > >     >
> > >     >  static const struct i2c_device_id pca9541_id[] = {
> > >     >       { "pca9541", pca9541 },
> > >     > +     { "pca9641", pca9641 },
> > >     >       {}
> > >     >  };
> > >     >
> > >     > @@ -307,6 +435,7 @@ MODULE_DEVICE_TABLE(i2c, pca9541_id);
> > >     >  #ifdef CONFIG_OF
> > >     >  static const struct of_device_id pca9541_of_match[] = {
> > >     >       { .compatible = "nxp,pca9541", .data = &chips[pca9541] },
> > >     > +     { .compatible = "nxp,pca9641", .data = &chips[pca9641] },
> > >     >       {}
> > >     >  };
> > >     >  MODULE_DEVICE_TABLE(of, pca9541_of_match);
> > >     > @@ -392,5 +521,5 @@ static struct i2c_driver pca9541_driver = {
> > >     >  module_i2c_driver(pca9541_driver);
> > >     >
> > >     >  MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net <mailto:
> linux@roeck-us.net>>");
> > >     > -MODULE_DESCRIPTION("PCA9541 I2C master selector driver");
> > >     > +MODULE_DESCRIPTION("PCA9541/PCA9641 I2C master selector
> driver");
> > >     >  MODULE_LICENSE("GPL v2");
> > >     >
> > >
> >
>

[-- Attachment #1.2: Type: text/html, Size: 21798 bytes --]

[-- Attachment #2: smonctl.txt --]
[-- Type: text/plain, Size: 4022 bytes --]

Fan1In(Fan1 Inlet):  OK
fan:11599 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan1Out(Fan1 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan2In(Fan2 Inlet):  OK
fan:11565 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan2Out(Fan2 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan3In(Fan3 Inlet):  OK
fan:11565 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan3Out(Fan3 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan4In(Fan4 Inlet):  OK
fan:11531 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan4Out(Fan4 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan5In(Fan5 Inlet):  OK
fan:11497 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan5Out(Fan5 Outlet):  OK
fan:11202 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

PSU1Fan(PSU1 Fan):  OK
fan:7488 RPM   (max = 18000 RPM, min = 4700 RPM, limit_variance = 15%)

PSU1:  OK

PSU2Fan(PSU2 Fan):  ABSENT

PSU2:  BAD

PSU1Temp1(PSU1 Inlet Temp Sensor):  OK
temp:36.0 C (lcrit = 0 C, fan_max = 60 C, fan_min = 25 C, min = 5 C, max = 60 C, crit = 100 C)

PSU1Temp2(PSU1 Heatsink Temp Sensor):  OK
temp:46.0 C (lcrit = 0 C, fan_max = 60 C, fan_min = 25 C, min = 5 C, max = 60 C, crit = 100 C)

PSU2Temp1(PSU2 Inlet Temp Sensor):  ABSENT

PSU2Temp2(PSU2 Heatsink Temp Sensor):  ABSENT

Temp1(Intel Core 0 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C, max = 90 C, crit = 98 C)

Temp2(Intel Core 1 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C, max = 90 C, crit = 98 C)

Temp3(Intel Core 2 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C, max = 90 C, crit = 98 C)

Temp4(Intel Core 3 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C, max = 90 C, crit = 98 C)

Temp5(Intel CPU external sensor 1):  OK
temp:30.0 C (lcrit = 0 C, fan_max = 65 C, fan_min = 25 C, min = 5 C, max = 65 C, crit = 70 C)

Temp6(Intel CPU external sensor 2):  OK
temp:31.0 C (lcrit = 0 C, fan_max = 65 C, fan_min = 25 C, min = 5 C, max = 65 C, crit = 70 C)

Temp7(Rear Outlet Air sensor):  OK
temp:32.5 C (lcrit = -15 C, fan_max = 48 C, fan_min = 25 C, min = -10 C, max = 48 C, crit = 60 C)

Temp8(Front Outlet Air sensor):  OK
temp:30.5 C (lcrit = -15 C, fan_max = 48 C, fan_min = 25 C, min = -10 C, max = 48 C, crit = 60 C)

Temp9(Temp Sensor close to Networking ASIC):  OK
temp:37.5 C (lcrit = 0 C, fan_max = 75 C, fan_min = 25 C, min = 5 C, max = 75 C, crit = 80 C)

Temp10(DIMM 1 Temp Sensor):  OK
temp:31.8 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C, max = 90 C, crit = 95 C)

Temp11(Networking ASIC Die Temp Sensor):  OK
temp:54.0 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp12(Networking ASIC Die Temp Sensor):  OK
temp:54.5 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp13(Networking ASIC Die Temp Sensor):  OK
temp:54.5 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp14(Networking ASIC Die Temp Sensor):  OK
temp:55.9 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp15(Networking ASIC Die Temp Sensor):  OK
temp:54.0 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp16(Networking ASIC Die Temp Sensor):  OK
temp:56.4 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp17(Networking ASIC Die Temp Sensor):  OK
temp:55.5 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp18(Networking ASIC Die Temp Sensor):  OK
temp:55.0 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Messages:
PSU2:  status is all_not_ok, installed


[-- Attachment #3: sensors-data.txt --]
[-- Type: text/plain, Size: 2601 bytes --]

emc2305-i2c-13-4d
Adapter: i2c-9-mux (chan_id 3)
fan1:        11266 RPM  (div = 4)
fan2:        11234 RPM  (div = 4)
fan3:        11170 RPM  (div = 4)
fan4:        11331 RPM  (div = 4)
fan5:        11266 RPM  (div = 4)

coretemp-isa-0000
Adapter: ISA adapter
Intel core 0 temp:  +19.0°C  (high = +98.0°C, crit = +98.0°C)
Intel core 1 temp:  +19.0°C  (high = +98.0°C, crit = +98.0°C)
Intel core 2 temp:  +19.0°C  (high = +98.0°C, crit = +98.0°C)
Intel core 3 temp:  +19.0°C  (high = +98.0°C, crit = +98.0°C)

dps460-i2c-11-5b
Adapter: i2c-9-mux (chan_id 1)
vin:          +0.00 V                                    ALARM
vcap:         -0.50 V  
vout1:        +0.00 V  (crit min = +10.40 V, crit max = +14.20 V)
fan1:           0 RPM
temp1:        +28.0°C  (high = +90.0°C)
temp2:        +28.0°C  (high = +90.0°C)
temp3:        +28.0°C  (high = +90.0°C)
pin:           0.00 W  (max =   1.15 kW)
pout1:         0.00 W  (max = 900.00 W, crit =   1.10 kW)
iin:          +0.00 A  (max =  +6.00 A, crit max =  +7.00 A)
iout1:        +0.00 A  (max = +76.00 A, crit max = +80.00 A)

lm75-i2c-24-49
Adapter: i2c-9-mux (chan_id 4)
temp1:        +32.5°C  (high = +48.0°C, hyst = +48.0°C)

lm75-i2c-15-4e
Adapter: i2c-9-mux (chan_id 5)
temp1:        +30.5°C  (high = +48.0°C, hyst = +48.0°C)

jc42-i2c-0-18
Adapter: SMBus I801 adapter at f000
temp1:        +31.5°C  (low  =  +0.0°C)
                       (high = +90.0°C, hyst = +90.0°C)
                       (crit = +95.0°C, hyst = +95.0°C)

dps460-i2c-10-5a
Adapter: i2c-9-mux (chan_id 0)
vin:         +206.50 V  
vout1:       +12.07 V  (crit min = +10.40 V, crit max = +14.20 V)
fan1:        7488 RPM
temp1:        +36.0°C  (high = +90.0°C)
temp2:        +46.0°C  (high = +90.0°C)
temp3:        +42.0°C  (high = +90.0°C)
pin:         147.75 W  (max =   1.15 kW)
pout1:       132.75 W  (max = 900.00 W, crit =   1.10 kW)
                       (cap = -500.00 mW)
iin:          +0.73 A  (max =  +6.00 A, crit max =  +7.00 A)
iout1:       +11.11 A  (max = +76.00 A, crit max = +80.00 A)

emc2305-i2c-13-2e
Adapter: i2c-9-mux (chan_id 3)
fan1:        11464 RPM  (div = 4)
fan2:        11599 RPM  (div = 4)
fan3:        11565 RPM  (div = 4)
fan4:        11464 RPM  (div = 4)
fan5:        11633 RPM  (div = 4)

lm75-i2c-25-4a
Adapter: i2c-9-mux (chan_id 5)
temp1:        +37.0°C  (high = +80.0°C, hyst = +80.0°C)

lm75-i2c-23-48
Adapter: i2c-9-mux (chan_id 3)
temp1:        +30.5°C  (high = +70.0°C, hyst = +70.0°C)

lm75-i2c-14-48
Adapter: i2c-9-mux (chan_id 4)
temp1:        +29.5°C  (high = +70.0°C, hyst = +70.0°C)


[-- Attachment #4: seastone-pca9541-test.txt --]
[-- Type: text/plain, Size: 7427 bytes --]

[    3.374804] pca9541 1-0070: registered master selector for I2C pca9541
[    3.380242] pca954x 9-0073: registered 8 multiplexed busses for I2C switch pca9548
[    3.482770] pca954x 9-0071: registered 8 multiplexed busses for I2C switch pca9548
[    3.488394] pca954x 9-0077: registered 8 multiplexed busses for I2C switch pca9548

root@cel-sea-51:/var/tmp# cat /sys/devices/platform/cel_seastone_cpld.0/cpld1_version
1.2
root@cel-sea-51:/var/tmp# cat /sys/devices/platform/cel_seastone_cpld.0/cpld2_version
2.2
root@cel-sea-51:/var/tmp# cat /sys/devices/platform/cel_seastone_cpld.0/cpld3_version
3.2
root@cel-sea-51:/var/tmp# cat /sys/devices/platform/cel_seastone_cpld.0/cpld4_version
4.2
root@cel-sea-51:/var/tmp# cat /sys/devices/platform/cel_seastone_cpld.0/cpld5_version
5.2

root@cel-sea-51:/var/tmp# cat /sys/devices/platform/cel_seastone_cpld.0/board_type
Seastone

root@cel-sea-51:/var/tmp# cat /sys/devices/platform/cel_seastone_cpld.0/led_psu1
green
root@cel-sea-51:/var/tmp# cat /sys/devices/platform/cel_seastone_cpld.0/led_psu2
off



root@cel-sea-51:/var/tmp# cat /sys/bus/i2c/devices/0-0050/eeprom_dev/eeprom0/label
spd1_eeprom

root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/0-0050/eeprom_dev/eeprom0/device/eeprom
00000000  92 11 0b 08 04 21 02 01  0b 11 01 08 0a 00 fc 00  |.....!..........|
00000010  69 78 69 30 69 11 18 81  20 08 3c 3c 00 f0 83 01  |ixi0i... .<<....|
00000020  80 00 00 00 00 00 00 00  00 88 00 00 00 00 00 00  |................|
00000030  00 00 00 00 00 00 00 00  00 00 00 00 0f 11 22 00  |..............".|
00000040  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000070  00 00 00 00 00 86 f1 02  16 24 00 00 00 00 67 5b  |.........$....g[|
00000080  69 2d 44 49 4d 4d 20 20  20 20 20 20 20 20 20 20  |i-DIMM          |
00000090  20 20 00 10 86 f1 44 43  41 31 2d 31 36 30 36 30  |  ....DCA1-16060|
000000a0  37 30 35 32 00 00 00 00  00 00 00 00 00 00 00 00  |7052............|
000000b0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000100


root@cel-sea-51:/var/tmp# cat /sys/bus/i2c/devices/12-0050/eeprom_dev/eeprom3/label
board_eeprom

root@cel-sea-51:/var/tmp# cat /sys/bus/i2c/devices/30-0050/eeprom_dev/eeprom4/label
fan2_eeprom

root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/30-0050/eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100


root@cel-sea-51:/var/tmp# cat /sys/bus/i2c/devices/31-0050/eeprom_dev/eeprom5/label
fan1_eeprom

root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/31-0050/eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100

root@cel-sea-51:/var/tmp# cat /sys/bus/i2c/devices/32-0050/eeprom_dev/eeprom6/label
fan5_eeprom

root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/32-0050/eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100

root@cel-sea-51:/var/tmp# cat /sys/bus/i2c/devices/33-0050/eeprom_dev/eeprom7/label
fan3_eeprom
root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/33-0050/eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100

root@cel-sea-51:/var/tmp# cat /sys/bus/i2c/devices/34-0050/eeprom_dev/eeprom8/label
fan4_eeprom
root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/34-0050/eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100

Fan1In(Fan1 Inlet):  OK
fan:11599 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan1Out(Fan1 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan2In(Fan2 Inlet):  OK
fan:11565 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan2Out(Fan2 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan3In(Fan3 Inlet):  OK
fan:11565 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan3Out(Fan3 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan4In(Fan4 Inlet):  OK
fan:11531 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan4Out(Fan4 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan5In(Fan5 Inlet):  OK
fan:11497 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan5Out(Fan5 Outlet):  OK
fan:11202 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

PSU1Fan(PSU1 Fan):  OK
fan:7488 RPM   (max = 18000 RPM, min = 4700 RPM, limit_variance = 15%)

PSU1:  OK

PSU2Fan(PSU2 Fan):  ABSENT

PSU2:  BAD

PSU1Temp1(PSU1 Inlet Temp Sensor):  OK
temp:36.0 C (lcrit = 0 C, fan_max = 60 C, fan_min = 25 C, min = 5 C, max = 60 C, crit = 100 C)

PSU1Temp2(PSU1 Heatsink Temp Sensor):  OK
temp:46.0 C (lcrit = 0 C, fan_max = 60 C, fan_min = 25 C, min = 5 C, max = 60 C, crit = 100 C)

PSU2Temp1(PSU2 Inlet Temp Sensor):  ABSENT

PSU2Temp2(PSU2 Heatsink Temp Sensor):  ABSENT

Temp1(Intel Core 0 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C, max = 90 C, crit = 98 C)

Temp2(Intel Core 1 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C, max = 90 C, crit = 98 C)

Temp3(Intel Core 2 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C, max = 90 C, crit = 98 C)

Temp4(Intel Core 3 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C, max = 90 C, crit = 98 C)

Temp5(Intel CPU external sensor 1):  OK
temp:30.0 C (lcrit = 0 C, fan_max = 65 C, fan_min = 25 C, min = 5 C, max = 65 C, crit = 70 C)

Temp6(Intel CPU external sensor 2):  OK
temp:31.0 C (lcrit = 0 C, fan_max = 65 C, fan_min = 25 C, min = 5 C, max = 65 C, crit = 70 C)

Temp7(Rear Outlet Air sensor):  OK
temp:32.5 C (lcrit = -15 C, fan_max = 48 C, fan_min = 25 C, min = -10 C, max = 48 C, crit = 60 C)

Temp8(Front Outlet Air sensor):  OK
temp:30.5 C (lcrit = -15 C, fan_max = 48 C, fan_min = 25 C, min = -10 C, max = 48 C, crit = 60 C)

Temp9(Temp Sensor close to Networking ASIC):  OK
temp:37.5 C (lcrit = 0 C, fan_max = 75 C, fan_min = 25 C, min = 5 C, max = 75 C, crit = 80 C)

Temp10(DIMM 1 Temp Sensor):  OK
temp:31.8 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C, max = 90 C, crit = 95 C)

Temp11(Networking ASIC Die Temp Sensor):  OK
temp:54.0 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp12(Networking ASIC Die Temp Sensor):  OK
temp:54.5 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp13(Networking ASIC Die Temp Sensor):  OK
temp:54.5 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp14(Networking ASIC Die Temp Sensor):  OK
temp:55.9 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp15(Networking ASIC Die Temp Sensor):  OK
temp:54.0 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp16(Networking ASIC Die Temp Sensor):  OK
temp:56.4 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp17(Networking ASIC Die Temp Sensor):  OK
temp:55.5 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Temp18(Networking ASIC Die Temp Sensor):  OK
temp:55.0 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C, max = 105 C, crit = 110 C)

Messages:
PSU2:  status is all_not_ok, installed


[-- Attachment #5: seastone-i2c.txt --]
[-- Type: text/plain, Size: 351 bytes --]

PCA 9541 mux
|---------------------------------|------------------------------|
|                                 |                              |
PCA 9548_1                    PCA 9548_2                    PCA 9548_3

Fan                    PSU1, 2, board EEPROM              EEPROM,
eeproms 1-5            fan controller, lm75               lm75

Re: [PATCH v2 5/5] i2c: mux: pca9541: add support for PCA9641

[Pradeep Srinivasan]

 I got some cycles to test the patches on a Celestica seastone switch
which has the PCA 9541 mux.  A rough sketch of the I2C mux tree on
seastone switch is shown below. I tested by accessing the all the
devices behind PCA9548 mux. There are 3 PCA 9548 mux devices behind
PCA 9541. I was able to read all Fan EEPROM data, lm74 sensor data
through smonctl as well as sensors command. Please let me know if you
need any other information.


Figure describing the PCA 9541 mux and devices behind the mux on seastone switch
-------------------------------------------------------------------------------------------------------------------

PCA 9541 mux
|---------------------------------|------------------------------|
|                                        |                                    |
PCA 9548_1                    PCA 9548_2                    PCA 9548_3

Fan                    PSU1, 2, board EEPROM              EEPROM,
eeproms 1-5            fan controller, lm75               lm75

Log messages and seastone CPLD driver accessing the devices behind PCA 9541 mux

[    3.374804] pca9541 1-0070: registered master selector for I2C pca9541
[    3.380242] pca954x 9-0073: registered 8 multiplexed busses for I2C
switch pca9548
[    3.482770] pca954x 9-0071: registered 8 multiplexed busses for I2C
switch pca9548
[    3.488394] pca954x 9-0077: registered 8 multiplexed busses for I2C
switch pca9548

root@cel-sea-51:/var/tmp# cat
/sys/devices/platform/cel_seastone_cpld.0/cpld1_version
1.2
root@cel-sea-51:/var/tmp# cat
/sys/devices/platform/cel_seastone_cpld.0/cpld2_version
2.2
root@cel-sea-51:/var/tmp# cat
/sys/devices/platform/cel_seastone_cpld.0/cpld3_version
3.2
root@cel-sea-51:/var/tmp# cat
/sys/devices/platform/cel_seastone_cpld.0/cpld4_version
4.2
root@cel-sea-51:/var/tmp# cat
/sys/devices/platform/cel_seastone_cpld.0/cpld5_version
5.2

root@cel-sea-51:/var/tmp# cat
/sys/devices/platform/cel_seastone_cpld.0/board_type
Seastone

root@cel-sea-51:/var/tmp# cat /sys/devices/platform/cel_seastone_cpld.0/led_psu1
green
root@cel-sea-51:/var/tmp# cat /sys/devices/platform/cel_seastone_cpld.0/led_psu2
off



root@cel-sea-51:/var/tmp# cat
/sys/bus/i2c/devices/0-0050/eeprom_dev/eeprom0/label
spd1_eeprom

root@cel-sea-51:/var/tmp# hd
/sys/bus/i2c/devices/0-0050/eeprom_dev/eeprom0/device/eeprom
00000000  92 11 0b 08 04 21 02 01  0b 11 01 08 0a 00 fc 00  |.....!..........|
00000010  69 78 69 30 69 11 18 81  20 08 3c 3c 00 f0 83 01  |ixi0i... .<<....|
00000020  80 00 00 00 00 00 00 00  00 88 00 00 00 00 00 00  |................|
00000030  00 00 00 00 00 00 00 00  00 00 00 00 0f 11 22 00  |..............".|
00000040  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000070  00 00 00 00 00 86 f1 02  16 24 00 00 00 00 67 5b  |.........$....g[|
00000080  69 2d 44 49 4d 4d 20 20  20 20 20 20 20 20 20 20  |i-DIMM          |
00000090  20 20 00 10 86 f1 44 43  41 31 2d 31 36 30 36 30  |  ....DCA1-16060|
000000a0  37 30 35 32 00 00 00 00  00 00 00 00 00 00 00 00  |7052............|
000000b0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000100


root@cel-sea-51:/var/tmp# cat
/sys/bus/i2c/devices/12-0050/eeprom_dev/eeprom3/label
board_eeprom

root@cel-sea-51:/var/tmp# cat
/sys/bus/i2c/devices/30-0050/eeprom_dev/eeprom4/label
fan2_eeprom

root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/30-0050/eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100


root@cel-sea-51:/var/tmp# cat
/sys/bus/i2c/devices/31-0050/eeprom_dev/eeprom5/label
fan1_eeprom

root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/31-0050/eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100

root@cel-sea-51:/var/tmp# cat
/sys/bus/i2c/devices/32-0050/eeprom_dev/eeprom6/label
fan5_eeprom

root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/32-0050/eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100

root@cel-sea-51:/var/tmp# cat
/sys/bus/i2c/devices/33-0050/eeprom_dev/eeprom7/label
fan3_eeprom
root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/33-0050/eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100

root@cel-sea-51:/var/tmp# cat
/sys/bus/i2c/devices/34-0050/eeprom_dev/eeprom8/label
fan4_eeprom
root@cel-sea-51:/var/tmp# hd /sys/bus/i2c/devices/34-0050/eeprom
00000000  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
*
00000100

smonctl -v output
------------------------

Fan1In(Fan1 Inlet):  OK
fan:11599 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan1Out(Fan1 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan2In(Fan2 Inlet):  OK
fan:11565 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan2Out(Fan2 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan3In(Fan3 Inlet):  OK
fan:11565 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan3Out(Fan3 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan4In(Fan4 Inlet):  OK
fan:11531 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan4Out(Fan4 Outlet):  OK
fan:11234 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan5In(Fan5 Inlet):  OK
fan:11497 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

Fan5Out(Fan5 Outlet):  OK
fan:11202 RPM   (max = 23000 RPM, min = 5300 RPM, limit_variance = 15%)

PSU1Fan(PSU1 Fan):  OK
fan:7488 RPM   (max = 18000 RPM, min = 4700 RPM, limit_variance = 15%)

PSU1:  OK

PSU2Fan(PSU2 Fan):  ABSENT

PSU2:  BAD

PSU1Temp1(PSU1 Inlet Temp Sensor):  OK
temp:36.0 C (lcrit = 0 C, fan_max = 60 C, fan_min = 25 C, min = 5 C,
max = 60 C, crit = 100 C)

PSU1Temp2(PSU1 Heatsink Temp Sensor):  OK
temp:46.0 C (lcrit = 0 C, fan_max = 60 C, fan_min = 25 C, min = 5 C,
max = 60 C, crit = 100 C)

PSU2Temp1(PSU2 Inlet Temp Sensor):  ABSENT

PSU2Temp2(PSU2 Heatsink Temp Sensor):  ABSENT

Temp1(Intel Core 0 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C,
max = 90 C, crit = 98 C)

Temp2(Intel Core 1 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C,
max = 90 C, crit = 98 C)

Temp3(Intel Core 2 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C,
max = 90 C, crit = 98 C)

Temp4(Intel Core 3 Temp Sensor):  OK
temp:19.0 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C,
max = 90 C, crit = 98 C)

Temp5(Intel CPU external sensor 1):  OK
temp:30.0 C (lcrit = 0 C, fan_max = 65 C, fan_min = 25 C, min = 5 C,
max = 65 C, crit = 70 C)

Temp6(Intel CPU external sensor 2):  OK
temp:31.0 C (lcrit = 0 C, fan_max = 65 C, fan_min = 25 C, min = 5 C,
max = 65 C, crit = 70 C)

Temp7(Rear Outlet Air sensor):  OK
temp:32.5 C (lcrit = -15 C, fan_max = 48 C, fan_min = 25 C, min = -10
C, max = 48 C, crit = 60 C)

Temp8(Front Outlet Air sensor):  OK
temp:30.5 C (lcrit = -15 C, fan_max = 48 C, fan_min = 25 C, min = -10
C, max = 48 C, crit = 60 C)

Temp9(Temp Sensor close to Networking ASIC):  OK
temp:37.5 C (lcrit = 0 C, fan_max = 75 C, fan_min = 25 C, min = 5 C,
max = 75 C, crit = 80 C)

Temp10(DIMM 1 Temp Sensor):  OK
temp:31.8 C (lcrit = 0 C, fan_max = 90 C, fan_min = 25 C, min = 5 C,
max = 90 C, crit = 95 C)

Temp11(Networking ASIC Die Temp Sensor):  OK
temp:54.0 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C,
max = 105 C, crit = 110 C)

Temp12(Networking ASIC Die Temp Sensor):  OK
temp:54.5 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C,
max = 105 C, crit = 110 C)

Temp13(Networking ASIC Die Temp Sensor):  OK
temp:54.5 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C,
max = 105 C, crit = 110 C)

Temp14(Networking ASIC Die Temp Sensor):  OK
temp:55.9 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C,
max = 105 C, crit = 110 C)

Temp15(Networking ASIC Die Temp Sensor):  OK
temp:54.0 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C,
max = 105 C, crit = 110 C)

Temp16(Networking ASIC Die Temp Sensor):  OK
temp:56.4 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C,
max = 105 C, crit = 110 C)

Temp17(Networking ASIC Die Temp Sensor):  OK
temp:55.5 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C,
max = 105 C, crit = 110 C)

Temp18(Networking ASIC Die Temp Sensor):  OK
temp:55.0 C (lcrit = 0 C, fan_max = 105 C, fan_min = 25 C, min = 5 C,
max = 105 C, crit = 110 C)

Messages:
PSU2:  status is all_not_ok, installed

sensors output
---------------------

 emc2305-i2c-13-4d
Adapter: i2c-9-mux (chan_id 3)
fan1:        11266 RPM  (div = 4)
fan2:        11234 RPM  (div = 4)
fan3:        11170 RPM  (div = 4)
fan4:        11331 RPM  (div = 4)
fan5:        11266 RPM  (div = 4)

coretemp-isa-0000
Adapter: ISA adapter
Intel core 0 temp:  +19.0°C  (high = +98.0°C, crit = +98.0°C)
Intel core 1 temp:  +19.0°C  (high = +98.0°C, crit = +98.0°C)
Intel core 2 temp:  +19.0°C  (high = +98.0°C, crit = +98.0°C)
Intel core 3 temp:  +19.0°C  (high = +98.0°C, crit = +98.0°C)

dps460-i2c-11-5b
Adapter: i2c-9-mux (chan_id 1)
vin:          +0.00 V                                    ALARM
vcap:         -0.50 V
vout1:        +0.00 V  (crit min = +10.40 V, crit max = +14.20 V)
fan1:           0 RPM
temp1:        +28.0°C  (high = +90.0°C)
temp2:        +28.0°C  (high = +90.0°C)
temp3:        +28.0°C  (high = +90.0°C)
pin:           0.00 W  (max =   1.15 kW)
pout1:         0.00 W  (max = 900.00 W, crit =   1.10 kW)
iin:          +0.00 A  (max =  +6.00 A, crit max =  +7.00 A)
iout1:        +0.00 A  (max = +76.00 A, crit max = +80.00 A)

lm75-i2c-24-49
Adapter: i2c-9-mux (chan_id 4)
temp1:        +32.5°C  (high = +48.0°C, hyst = +48.0°C)

lm75-i2c-15-4e
Adapter: i2c-9-mux (chan_id 5)
temp1:        +30.5°C  (high = +48.0°C, hyst = +48.0°C)

jc42-i2c-0-18
Adapter: SMBus I801 adapter at f000
temp1:        +31.5°C  (low  =  +0.0°C)
                       (high = +90.0°C, hyst = +90.0°C)
                       (crit = +95.0°C, hyst = +95.0°C)

dps460-i2c-10-5a
Adapter: i2c-9-mux (chan_id 0)
vin:         +206.50 V
vout1:       +12.07 V  (crit min = +10.40 V, crit max = +14.20 V)
fan1:        7488 RPM
temp1:        +36.0°C  (high = +90.0°C)
temp2:        +46.0°C  (high = +90.0°C)
temp3:        +42.0°C  (high = +90.0°C)
pin:         147.75 W  (max =   1.15 kW)
pout1:       132.75 W  (max = 900.00 W, crit =   1.10 kW)
                       (cap = -500.00 mW)
iin:          +0.73 A  (max =  +6.00 A, crit max =  +7.00 A)
iout1:       +11.11 A  (max = +76.00 A, crit max = +80.00 A)

emc2305-i2c-13-2e
Adapter: i2c-9-mux (chan_id 3)
fan1:        11464 RPM  (div = 4)
fan2:        11599 RPM  (div = 4)
fan3:        11565 RPM  (div = 4)
fan4:        11464 RPM  (div = 4)
fan5:        11633 RPM  (div = 4)

lm75-i2c-25-4a
Adapter: i2c-9-mux (chan_id 5)
temp1:        +37.0°C  (high = +80.0°C, hyst = +80.0°C)

lm75-i2c-23-48
Adapter: i2c-9-mux (chan_id 3)
temp1:        +30.5°C  (high = +70.0°C, hyst = +70.0°C)

lm75-i2c-14-48
Adapter: i2c-9-mux (chan_id 4)
temp1:        +29.5°C  (high = +70.0°C, hyst = +70.0°C)



On Mon, Mar 25, 2019 at 9:49 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Mon, Mar 25, 2019 at 03:01:21PM +0000, Peter Rosin wrote:
> > On 2019-03-22 20:38, Pradeep Srinivasan wrote:
> > > I have verified the changes on PCA 9541. May I know how you want the test results to be shared ? (newbie here; please bear with me)
> > >
> > > root@cumulus:/home/cumulus# dmesg| grep "pca9541" | grep -v "pmbus"
> > > [    2.922288] pca9541 1-0070: registered master selector for I2C pca9541
> > >
> > > root@cumulus:/home/cumulus# cat /sys/class/i2c-dev/i2c-1/device/1-0070/name
> > > pca9541
> >
> > This only verifies that the probe works and that the chip is detected properly.
> > It says nothing about if it works to communicate with whatever is beyond the
> > PCA9541, and nothing on how the interaction with the "alien" other master
> > connected to the PCA9541 is working. I don't know how I want this to be tested,
> > but if you have a setup with a PCA9541 / PCA9641 I would assume that you
> > have some kind of need for those chips and that you at least could report
> > if basic xfers through them are working? I don't need to see actual commands
> > that you have executed, I'm much more interested in some summary of what
> > you did and what worked (or not).
> >
> > E.g. if you have an eeprom beyond the master selector, you could read from
> > it in a loop while doing something else from the alien master and check if
> > it all works as expected? Perhaps try to verify timing if there are stalls
> > and/or timeouts etc. Go wild! But if you don't know how or don't have the
>
> Something like that is what I did to test the original implementation: Access
> all chips behind the mux from both ends continuously. Let that run for an hour
> or so and declare it a success if there is no error. Usually, while the code
> was still buggy, errors would show up within minutes, if not seconds.
>
> Guenter
>
> > time, I'd be happy with a report on basic functionality (but a little bit
> > more than probe-ok would be nice though), because the code affecting the
> > PCA9541 is probably not broken subtly, it either works as it did before or
> > it doesn't work at all. And any problem with the PCA9641 side of things
> > will not be a regression and therefore not a big problem...
> >
> > Cheers,
> > Peter
> >
> > > I need to do the same on PCA 9641. If the above is sufficient, I will grab a switch with PCA 9641 and check if the driver works .
> > >
> > >
> > > Thanks
> > > Pradeep
> > >
> > > On Thu, Mar 7, 2019 at 1:16 PM Peter Rosin <peda@axentia.se <mailto:peda@axentia.se>> wrote:
> > >
> > >     Hi!
> > >
> > >     I should have read Kens code more carefully, before signing off on it...
> > >
> > >     Review comments inline...
> > >
> > >     On 2019-03-07 00:15, Peter Rosin wrote:
> > >     > Heavily based on code from Ken Chen <chen.kenyy@inventec.com <mailto:chen.kenyy@inventec.com>>.
> > >     >
> > >     > Signed-off-by: Peter Rosin <peda@axentia.se <mailto:peda@axentia.se>>
> > >     > ---
> > >     >  drivers/i2c/muxes/Kconfig           |   6 +-
> > >     >  drivers/i2c/muxes/i2c-mux-pca9541.c | 137 ++++++++++++++++++++++++++++++++++--
> > >     >  2 files changed, 136 insertions(+), 7 deletions(-)
> > >     >
> > >     > diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> > >     > index 52a4a922e7e6..8532841de5db 100644
> > >     > --- a/drivers/i2c/muxes/Kconfig
> > >     > +++ b/drivers/i2c/muxes/Kconfig
> > >     > @@ -55,10 +55,10 @@ config I2C_MUX_LTC4306
> > >     >         will be called i2c-mux-ltc4306.
> > >     >
> > >     >  config I2C_MUX_PCA9541
> > >     > -     tristate "NXP PCA9541 I2C Master Selector"
> > >     > +     tristate "NXP PCA9541/PCA9641 I2C Master Selectors"
> > >     >       help
> > >     > -       If you say yes here you get support for the NXP PCA9541
> > >     > -       I2C Master Selector.
> > >     > +       If you say yes here you get support for the NXP PCA9541/PCA9641
> > >     > +       I2C Master Selectors.
> > >     >
> > >     >         This driver can also be built as a module.  If so, the module
> > >     >         will be called i2c-mux-pca9541.
> > >     > diff --git a/drivers/i2c/muxes/i2c-mux-pca9541.c b/drivers/i2c/muxes/i2c-mux-pca9541.c
> > >     > index 5eb36e3223d5..5d4e0c92e978 100644
> > >     > --- a/drivers/i2c/muxes/i2c-mux-pca9541.c
> > >     > +++ b/drivers/i2c/muxes/i2c-mux-pca9541.c
> > >     > @@ -1,5 +1,5 @@
> > >     >  /*
> > >     > - * I2C multiplexer driver for PCA9541 bus master selector
> > >     > + * I2C multiplexer driver for PCA9541/PCA9641 bus master selectors
> > >     >   *
> > >     >   * Copyright (c) 2010 Ericsson AB.
> > >     >   *
> > >     > @@ -28,8 +28,8 @@
> > >     >  #include <linux/slab.h>
> > >     >
> > >     >  /*
> > >     > - * The PCA9541 is a bus master selector. It supports two I2C masters connected
> > >     > - * to a single slave bus.
> > >     > + * The PCA9541 and PCA9641 are bus master selector. They support two I2C masters
> > >     > + * connected to a single slave bus.
> > >     >   *
> > >     >   * Before each bus transaction, a master has to acquire bus ownership. After the
> > >     >   * transaction is complete, bus ownership has to be released. This fits well
> > >     > @@ -63,6 +63,33 @@
> > >     >  #define PCA9541_BUSON        (PCA9541_CTL_BUSON | PCA9541_CTL_NBUSON)
> > >     >  #define PCA9541_MYBUS        (PCA9541_CTL_MYBUS | PCA9541_CTL_NMYBUS)
> > >     >
> > >     > +#define PCA9641_ID                   0x00
> > >     > +#define PCA9641_ID_MAGIC             0x38
> > >     > +
> > >     > +#define PCA9641_CONTROL                      0x01
> > >     > +#define PCA9641_STATUS                       0x02
> > >     > +#define PCA9641_TIME                 0x03
> > >     > +
> > >     > +#define PCA9641_CTL_LOCK_REQ         BIT(0)
> > >     > +#define PCA9641_CTL_LOCK_GRANT               BIT(1)
> > >     > +#define PCA9641_CTL_BUS_CONNECT              BIT(2)
> > >     > +#define PCA9641_CTL_BUS_INIT         BIT(3)
> > >     > +#define PCA9641_CTL_SMBUS_SWRST              BIT(4)
> > >     > +#define PCA9641_CTL_IDLE_TIMER_DIS   BIT(5)
> > >     > +#define PCA9641_CTL_SMBUS_DIS                BIT(6)
> > >     > +#define PCA9641_CTL_PRIORITY         BIT(7)
> > >     > +
> > >     > +#define PCA9641_STS_OTHER_LOCK               BIT(0)
> > >     > +#define PCA9641_STS_BUS_INIT_FAIL    BIT(1)
> > >     > +#define PCA9641_STS_BUS_HUNG         BIT(2)
> > >     > +#define PCA9641_STS_MBOX_EMPTY               BIT(3)
> > >     > +#define PCA9641_STS_MBOX_FULL                BIT(4)
> > >     > +#define PCA9641_STS_TEST_INT         BIT(5)
> > >     > +#define PCA9641_STS_SCL_IO           BIT(6)
> > >     > +#define PCA9641_STS_SDA_IO           BIT(7)
> > >     > +
> > >     > +#define PCA9641_RES_TIME             0x03
> > >
> > >     This appears to be the same thing as PCA9641_TIME above. The
> > >     register is called PCA9641_RT in my data sheet.
> > >
> > >     > +
> > >     >  /* 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 */
> > >     > @@ -73,6 +100,7 @@
> > >     >
> > >     >  enum chip_name {
> > >     >       pca9541,
> > >     > +     pca9641,
> > >     >  };
> > >     >
> > >     >  struct chip_desc {
> > >     > @@ -102,6 +130,21 @@ static bool pca9541_busoff(int ctl)
> > >     >       return (ctl & PCA9541_BUSON) == PCA9541_BUSON;
> > >     >  }
> > >     >
> > >     > +static bool pca9641_lock_grant(int ctl)
> > >     > +{
> > >     > +     return !!(ctl & PCA9641_CTL_LOCK_GRANT);
> > >     > +}
> > >     > +
> > >     > +static bool pca9641_other_lock(int sts)
> > >     > +{
> > >     > +     return !!(sts & PCA9641_STS_OTHER_LOCK);
> > >     > +}
> > >     > +
> > >     > +static bool pca9641_busoff(int ctl, int sts)
> > >     > +{
> > >     > +     return !pca9641_lock_grant(ctl) && !pca9641_other_lock(sts);
> > >     > +}
> > >     > +
> > >     >  /*
> > >     >   * Write to chip register. Don't use i2c_transfer()/i2c_smbus_xfer()
> > >     >   * as they will try to lock the adapter a second time.
> > >     > @@ -256,6 +299,86 @@ static int pca9541_arbitrate(struct i2c_client *client)
> > >     >       return 0;
> > >     >  }
> > >     >
> > >     > +/* Release bus. */
> > >     > +static void pca9641_release_bus(struct i2c_client *client)
> > >     > +{
> > >     > +     pca9541_reg_write(client, PCA9641_CONTROL, 0);
> > >
> > >     Should this release bus function really "clobber" the control bits
> > >     PCA9641_CTL_IDLE_TIMER_DIS, PCA9641_CTL_SMBUS_DIS, PCA9641_CTL_PRIORITY?
> > >     Yes yes, the driver never sets these bits so they are likely zero. But
> > >     the driver doesn't reset the chip either, so some bootstrap code might
> > >     have configured those bits...
> > >
> > >     Also related to bus release, since the driver does not touch the
> > >     reserve time register, and then clears the above bits, the only way
> > >     to release the bus is if everything continues to work and the above
> > >     pca9641_release_bus is in fact happening. But if the kernel crashes
> > >     while hogging the bus, and fails to come up, then the other master
> > >     has no way of stealing the ownership. I really feel that the driver
> > >     should make use of the timers so that the arbiter releases the bus
> > >     automatically on catastrophic failure. But maybe I plain and simple
> > >     just misunderstand the datasheet?
> > >
> > >     > +}
> > >     > +
> > >     > +/*
> > >     > + * 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 pca9541 *data = i2c_mux_priv(muxc);
> > >     > +     int reg_ctl, reg_sts;
> > >     > +
> > >     > +     reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
> > >     > +     if (reg_ctl < 0)
> > >     > +             return reg_ctl;
> > >     > +     reg_sts = pca9541_reg_read(client, PCA9641_STATUS);
> > >     > +
> > >     > +     if (pca9641_busoff(reg_ctl, reg_sts)) {
> > >     > +             /*
> > >     > +              * Bus is off. Request ownership or turn it on unless
> > >     > +              * other master requested ownership.
> > >     > +              */
> > >     > +             reg_ctl |= PCA9641_CTL_LOCK_REQ;
> > >     > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> > >     > +             reg_ctl = pca9541_reg_read(client, PCA9641_CONTROL);
> > >     > +
> > >     > +             if (pca9641_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;
> > >     > +                     pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> > >     > +                     data->select_timeout = SELECT_DELAY_SHORT;
> > >     > +
> > >     > +                     return 1;
> > >     > +             }
> > >     > +
> > >     > +             /*
> > >     > +              * Other master requested ownership.
> > >     > +              * Set extra long timeout to give it time to acquire it.
> > >     > +              */
> > >     > +             data->select_timeout = SELECT_DELAY_LONG * 2;
> > >     > +
> > >     > +             return 0;
> > >     > +     }
> > >     > +
> > >     > +     if (pca9641_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;
> > >     > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> > >     > +
> > >     > +             return 1;
> > >     > +     }
> > >     > +
> > >     > +     if (pca9641_other_lock(reg_sts)) {
> > >     > +             /*
> > >     > +              * Other master owns the bus.
> > >     > +              * If arbitration timeout has expired, force ownership.
> > >     > +              * Otherwise request it.
> > >
> > >     This comment is stale. Reading the data sheet, I find no way to force
> > >     ownership with the PCA9641 (as indicated above in the release_bus
> > >     review comment). But I have only browsed the data sheet so I could
> > >     easily be mistaken...
> > >
> > >     [time passes]
> > >
> > >     Ahhh, wait, it could reset the chip to get a new chance to get ownership.
> > >     But that will reset all registers for the other master as well, since I
> > >     read it as if the reset is chip-global and not master-local with minimal
> > >     effects on the other master. So, a big hammer indeed.
> > >
> > >     Cheers,
> > >     Peter
> > >
> > >     > +              */
> > >     > +             data->select_timeout = SELECT_DELAY_LONG;
> > >     > +             reg_ctl |= PCA9641_CTL_LOCK_REQ;
> > >     > +             pca9541_reg_write(client, PCA9641_CONTROL, reg_ctl);
> > >     > +     }
> > >     > +
> > >     > +     return 0;
> > >     > +}
> > >     > +
> > >     >  static int pca9541_select_chan(struct i2c_mux_core *muxc, u32 chan)
> > >     >  {
> > >     >       struct pca9541 *data = i2c_mux_priv(muxc);
> > >     > @@ -295,10 +418,15 @@ static const struct chip_desc chips[] = {
> > >     >               .arbitrate = pca9541_arbitrate,
> > >     >               .release_bus = pca9541_release_bus,
> > >     >       },
> > >     > +     [pca9641] = {
> > >     > +             .arbitrate = pca9641_arbitrate,
> > >     > +             .release_bus = pca9641_release_bus,
> > >     > +     },
> > >     >  };
> > >     >
> > >     >  static const struct i2c_device_id pca9541_id[] = {
> > >     >       { "pca9541", pca9541 },
> > >     > +     { "pca9641", pca9641 },
> > >     >       {}
> > >     >  };
> > >     >
> > >     > @@ -307,6 +435,7 @@ MODULE_DEVICE_TABLE(i2c, pca9541_id);
> > >     >  #ifdef CONFIG_OF
> > >     >  static const struct of_device_id pca9541_of_match[] = {
> > >     >       { .compatible = "nxp,pca9541", .data = &chips[pca9541] },
> > >     > +     { .compatible = "nxp,pca9641", .data = &chips[pca9641] },
> > >     >       {}
> > >     >  };
> > >     >  MODULE_DEVICE_TABLE(of, pca9541_of_match);
> > >     > @@ -392,5 +521,5 @@ static struct i2c_driver pca9541_driver = {
> > >     >  module_i2c_driver(pca9541_driver);
> > >     >
> > >     >  MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net <mailto:linux@roeck-us.net>>");
> > >     > -MODULE_DESCRIPTION("PCA9541 I2C master selector driver");
> > >     > +MODULE_DESCRIPTION("PCA9541/PCA9641 I2C master selector driver");
> > >     >  MODULE_LICENSE("GPL v2");
> > >     >
> > >
> >