[PATCH v3 0/4] i2c: thunderx: Marvell thunderx i2c changes

[PATCH v3 0/4] i2c: thunderx: Marvell thunderx i2c changes

[Piyush Malgujar]

The changes are for Marvell OcteonTX2 SOC family:

- Handling clock divisor logic using subsytem ID
- Support for high speed mode
- Handle watchdog timeout
- Added ioclk support

Changes since V2:
- Respinning the series, no functional change
- Added Marvell member in MAINTAINERS file
- Added macro OTX2_REF_FREQ_DEFAULT for 100 MHz

Changes since V1:
- Addressed comments, added defines as required
- Removed unnecessary code
- Added a patch to support ioclk if sclk not present in ACPI table

Piyush Malgujar (1):
  i2c: thunderx: Adding ioclk support

Suneel Garapati (3):
  i2c: thunderx: Clock divisor logic changes
  i2c: thunderx: Add support for High speed mode
  i2c: octeon: Handle watchdog timeout

 MAINTAINERS                              |  1 +
 drivers/i2c/busses/i2c-octeon-core.c     | 96 ++++++++++++++++++------
 drivers/i2c/busses/i2c-octeon-core.h     | 27 +++++++
 drivers/i2c/busses/i2c-thunderx-pcidrv.c | 23 ++++--
 4 files changed, 117 insertions(+), 30 deletions(-)

-- 
2.42.0

[PATCH v3 1/4] i2c: thunderx: Clock divisor logic changes

[Piyush Malgujar]

From: Suneel Garapati <sgarapati@marvell.com>

Handle changes to clock divisor logic for OcteonTX2 SoC family using
subsystem ID and using default reference clock source as 100MHz.

Signed-off-by: Suneel Garapati <sgarapati@marvell.com>
Signed-off-by: Piyush Malgujar <pmalgujar@marvell.com>
Acked-by: Andi Shyti <andi.shyti@kernel.org>
---
 MAINTAINERS                              |  1 +
 drivers/i2c/busses/i2c-octeon-core.c     | 29 ++++++++++++++++++++----
 drivers/i2c/busses/i2c-octeon-core.h     | 17 ++++++++++++++
 drivers/i2c/busses/i2c-thunderx-pcidrv.c |  7 ++++++
 4 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 960512bec42885c0f1632a7c90851c3d32fbf20e..92b0a55c36e41cf54c7cbf52576d5424b591aa31 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4725,6 +4725,7 @@ F:	drivers/net/wireless/ath/carl9170/
 
 CAVIUM I2C DRIVER
 M:	Robert Richter <rric@kernel.org>
+M:	Suneel Garapati <sgarapati@marvell.com>
 S:	Odd Fixes
 W:	http://www.marvell.com
 F:	drivers/i2c/busses/i2c-octeon*
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 845eda70b8cab52a0453c9f4cb545010fba4305d..1d8e1f4ad859dc44c08629637530842a0ed50bc4 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -17,6 +17,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/pci.h>
 
 #include "i2c-octeon-core.h"
 
@@ -658,31 +659,51 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 void octeon_i2c_set_clock(struct octeon_i2c *i2c)
 {
 	int tclk, thp_base, inc, thp_idx, mdiv_idx, ndiv_idx, foscl, diff;
-	int thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = 1000000;
+	int mdiv_min = 2;
+	/* starting value on search for lowest diff */
+	const int huge_delta = INITIAL_DELTA_HZ;
+	/*
+	 * Find divisors to produce target frequency, start with large delta
+	 * to cover wider range of divisors, note thp = TCLK half period.
+	 */
+	unsigned int thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta;
+
+	if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) {
+		thp = 0x3;
+		mdiv_min = 0;
+	}
 
 	for (ndiv_idx = 0; ndiv_idx < 8 && delta_hz != 0; ndiv_idx++) {
 		/*
 		 * An mdiv value of less than 2 seems to not work well
 		 * with ds1337 RTCs, so we constrain it to larger values.
 		 */
-		for (mdiv_idx = 15; mdiv_idx >= 2 && delta_hz != 0; mdiv_idx--) {
+		for (mdiv_idx = 15; mdiv_idx >= mdiv_min && delta_hz != 0; mdiv_idx--) {
 			/*
 			 * For given ndiv and mdiv values check the
 			 * two closest thp values.
 			 */
 			tclk = i2c->twsi_freq * (mdiv_idx + 1) * 10;
 			tclk *= (1 << ndiv_idx);
-			thp_base = (i2c->sys_freq / (tclk * 2)) - 1;
+			if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev)))
+				thp_base = (i2c->sys_freq / tclk) - 2;
+			else
+				thp_base = (i2c->sys_freq / (tclk * 2)) - 1;
 
 			for (inc = 0; inc <= 1; inc++) {
 				thp_idx = thp_base + inc;
 				if (thp_idx < 5 || thp_idx > 0xff)
 					continue;
 
-				foscl = i2c->sys_freq / (2 * (thp_idx + 1));
+				if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev)))
+					foscl = i2c->sys_freq / (thp_idx + 2);
+				else
+					foscl = i2c->sys_freq /
+						(2 * (thp_idx + 1));
 				foscl = foscl / (1 << ndiv_idx);
 				foscl = foscl / (mdiv_idx + 1) / 10;
 				diff = abs(foscl - i2c->twsi_freq);
+				/* Use it if smaller diff from target */
 				if (diff < delta_hz) {
 					delta_hz = diff;
 					thp = thp_idx;
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 9bb9f64fdda0392364638ecbaafe3fab5612baf6..694c24cecb7b144c1021549d1661b040c21bb998 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -7,6 +7,7 @@
 #include <linux/i2c-smbus.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/pci.h>
 
 /* Controller command patterns */
 #define SW_TWSI_V		BIT_ULL(63)	/* Valid bit */
@@ -98,6 +99,8 @@ struct octeon_i2c_reg_offset {
 #define TWSI_INT(x)	(x->roff.twsi_int)
 #define SW_TWSI_EXT(x)	(x->roff.sw_twsi_ext)
 
+#define INITIAL_DELTA_HZ	1000000
+
 struct octeon_i2c {
 	wait_queue_head_t queue;
 	struct i2c_adapter adap;
@@ -211,6 +214,20 @@ static inline void octeon_i2c_write_int(struct octeon_i2c *i2c, u64 data)
 	octeon_i2c_writeq_flush(data, i2c->twsi_base + TWSI_INT(i2c));
 }
 
+#define PCI_SUBSYS_DEVID_9XXX 0xB
+/**
+ * octeon_i2c_is_otx2 - check for chip ID
+ * @pdev: PCI dev structure
+ *
+ * Returns TRUE if OcteonTX2, FALSE otherwise.
+ */
+static inline bool octeon_i2c_is_otx2(struct pci_dev *pdev)
+{
+	u32 chip_id = (pdev->subsystem_device >> 12) & 0xF;
+
+	return (chip_id == PCI_SUBSYS_DEVID_9XXX);
+}
+
 /* Prototypes */
 irqreturn_t octeon_i2c_isr(int irq, void *dev_id);
 int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num);
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index a77cd86fe75ed7401bc041b27c651b9fedf67285..75569774003857dc984e8540ef8f4d1bb084cfb0 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -28,6 +28,7 @@
 #define PCI_DEVICE_ID_THUNDER_TWSI	0xa012
 
 #define SYS_FREQ_DEFAULT		700000000
+#define OTX2_REF_FREQ_DEFAULT		100000000
 
 #define TWSI_INT_ENA_W1C		0x1028
 #define TWSI_INT_ENA_W1S		0x1030
@@ -205,6 +206,12 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
 	if (ret)
 		goto error;
 
+	/*
+	 * For OcteonTX2 chips, set reference frequency to 100MHz
+	 * as refclk_src in TWSI_MODE register defaults to 100MHz.
+	 */
+	if (octeon_i2c_is_otx2(pdev))
+		i2c->sys_freq = OTX2_REF_FREQ_DEFAULT;
 	octeon_i2c_set_clock(i2c);
 
 	i2c->adap = thunderx_i2c_ops;
-- 
2.42.0

[PATCH v3 2/4] i2c: thunderx: Add support for High speed mode

[Piyush Malgujar]

From: Suneel Garapati <sgarapati@marvell.com>

Support High speed mode clock setup for OcteonTX2 platforms.

Signed-off-by: Suneel Garapati <sgarapati@marvell.com>
Signed-off-by: Piyush Malgujar <pmalgujar@marvell.com>
---
 drivers/i2c/busses/i2c-octeon-core.c     | 61 +++++++++++++++---------
 drivers/i2c/busses/i2c-octeon-core.h     |  6 +++
 drivers/i2c/busses/i2c-thunderx-pcidrv.c |  3 +-
 3 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 1d8e1f4ad859dc44c08629637530842a0ed50bc4..6636719ca8f005056230620e2cee19de7154e024 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -608,25 +608,27 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	struct octeon_i2c *i2c = i2c_get_adapdata(adap);
 	int i, ret = 0;
 
-	if (num == 1) {
-		if (msgs[0].len > 0 && msgs[0].len <= 8) {
-			if (msgs[0].flags & I2C_M_RD)
-				ret = octeon_i2c_hlc_read(i2c, msgs);
-			else
-				ret = octeon_i2c_hlc_write(i2c, msgs);
-			goto out;
-		}
-	} else if (num == 2) {
-		if ((msgs[0].flags & I2C_M_RD) == 0 &&
-		    (msgs[1].flags & I2C_M_RECV_LEN) == 0 &&
-		    msgs[0].len > 0 && msgs[0].len <= 2 &&
-		    msgs[1].len > 0 && msgs[1].len <= 8 &&
-		    msgs[0].addr == msgs[1].addr) {
-			if (msgs[1].flags & I2C_M_RD)
-				ret = octeon_i2c_hlc_comp_read(i2c, msgs);
-			else
-				ret = octeon_i2c_hlc_comp_write(i2c, msgs);
-			goto out;
+	if (IS_LS_FREQ(i2c->twsi_freq)) {
+		if (num == 1) {
+			if (msgs[0].len > 0 && msgs[0].len <= 8) {
+				if (msgs[0].flags & I2C_M_RD)
+					ret = octeon_i2c_hlc_read(i2c, msgs);
+				else
+					ret = octeon_i2c_hlc_write(i2c, msgs);
+				goto out;
+			}
+		} else if (num == 2) {
+			if ((msgs[0].flags & I2C_M_RD) == 0 &&
+			    (msgs[1].flags & I2C_M_RECV_LEN) == 0 &&
+			    msgs[0].len > 0 && msgs[0].len <= 2 &&
+			    msgs[1].len > 0 && msgs[1].len <= 8 &&
+			    msgs[0].addr == msgs[1].addr) {
+				if (msgs[1].flags & I2C_M_RD)
+					ret = octeon_i2c_hlc_comp_read(i2c, msgs);
+				else
+					ret = octeon_i2c_hlc_comp_write(i2c, msgs);
+				goto out;
+			}
 		}
 	}
 
@@ -666,11 +668,13 @@ void octeon_i2c_set_clock(struct octeon_i2c *i2c)
 	 * Find divisors to produce target frequency, start with large delta
 	 * to cover wider range of divisors, note thp = TCLK half period.
 	 */
-	unsigned int thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta;
+	unsigned int ds = 10, thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta;
 
 	if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) {
 		thp = 0x3;
 		mdiv_min = 0;
+		if (!IS_LS_FREQ(i2c->twsi_freq))
+			ds = 15;
 	}
 
 	for (ndiv_idx = 0; ndiv_idx < 8 && delta_hz != 0; ndiv_idx++) {
@@ -683,7 +687,7 @@ void octeon_i2c_set_clock(struct octeon_i2c *i2c)
 			 * For given ndiv and mdiv values check the
 			 * two closest thp values.
 			 */
-			tclk = i2c->twsi_freq * (mdiv_idx + 1) * 10;
+			tclk = i2c->twsi_freq * (mdiv_idx + 1) * ds;
 			tclk *= (1 << ndiv_idx);
 			if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev)))
 				thp_base = (i2c->sys_freq / tclk) - 2;
@@ -701,7 +705,9 @@ void octeon_i2c_set_clock(struct octeon_i2c *i2c)
 					foscl = i2c->sys_freq /
 						(2 * (thp_idx + 1));
 				foscl = foscl / (1 << ndiv_idx);
-				foscl = foscl / (mdiv_idx + 1) / 10;
+				foscl = foscl / (mdiv_idx + 1) / ds;
+				if (foscl > i2c->twsi_freq)
+					continue;
 				diff = abs(foscl - i2c->twsi_freq);
 				/* Use it if smaller diff from target */
 				if (diff < delta_hz) {
@@ -715,6 +721,17 @@ void octeon_i2c_set_clock(struct octeon_i2c *i2c)
 	}
 	octeon_i2c_reg_write(i2c, SW_TWSI_OP_TWSI_CLK, thp);
 	octeon_i2c_reg_write(i2c, SW_TWSI_EOP_TWSI_CLKCTL, (mdiv << 3) | ndiv);
+	if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) {
+		u64 mode;
+
+		mode = __raw_readq(i2c->twsi_base + MODE(i2c));
+		/* Set REFCLK_SRC and HS_MODE in TWSX_MODE register */
+		if (!IS_LS_FREQ(i2c->twsi_freq))
+			mode |= TWSX_MODE_HS_MASK;
+		else
+			mode &= ~TWSX_MODE_HS_MASK;
+		octeon_i2c_writeq_flush(mode, i2c->twsi_base + MODE(i2c));
+	}
 }
 
 int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 694c24cecb7b144c1021549d1661b040c21bb998..e89f041550ace5f7cbcdd94146d0193abe51d466 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -93,14 +93,19 @@ struct octeon_i2c_reg_offset {
 	unsigned int sw_twsi;
 	unsigned int twsi_int;
 	unsigned int sw_twsi_ext;
+	unsigned int mode;
 };
 
 #define SW_TWSI(x)	(x->roff.sw_twsi)
 #define TWSI_INT(x)	(x->roff.twsi_int)
 #define SW_TWSI_EXT(x)	(x->roff.sw_twsi_ext)
+#define MODE(x)		(x->roff.mode)
 
 #define INITIAL_DELTA_HZ	1000000
 
+/* Set REFCLK_SRC and HS_MODE in TWSX_MODE register */
+#define TWSX_MODE_HS_MASK	(BIT(4) | BIT(0))
+
 struct octeon_i2c {
 	wait_queue_head_t queue;
 	struct i2c_adapter adap;
@@ -214,6 +219,7 @@ static inline void octeon_i2c_write_int(struct octeon_i2c *i2c, u64 data)
 	octeon_i2c_writeq_flush(data, i2c->twsi_base + TWSI_INT(i2c));
 }
 
+#define IS_LS_FREQ(twsi_freq) ((twsi_freq) <= 400000)
 #define PCI_SUBSYS_DEVID_9XXX 0xB
 /**
  * octeon_i2c_is_otx2 - check for chip ID
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index 75569774003857dc984e8540ef8f4d1bb084cfb0..31f11b77ab663626967c86086a03213876bf4a07 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -166,6 +166,7 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
 	i2c->roff.sw_twsi = 0x1000;
 	i2c->roff.twsi_int = 0x1010;
 	i2c->roff.sw_twsi_ext = 0x1018;
+	i2c->roff.mode = 0x1038;
 
 	i2c->dev = dev;
 	pci_set_drvdata(pdev, i2c);
@@ -210,7 +211,7 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
 	 * For OcteonTX2 chips, set reference frequency to 100MHz
 	 * as refclk_src in TWSI_MODE register defaults to 100MHz.
 	 */
-	if (octeon_i2c_is_otx2(pdev))
+	if (octeon_i2c_is_otx2(pdev) && IS_LS_FREQ(i2c->twsi_freq))
 		i2c->sys_freq = OTX2_REF_FREQ_DEFAULT;
 	octeon_i2c_set_clock(i2c);
 
-- 
2.42.0

[PATCH v3 3/4] i2c: octeon: Handle watchdog timeout

[Piyush Malgujar]

From: Suneel Garapati <sgarapati@marvell.com>

Status code 0xF0 refers to expiry of TWSI controller
access watchdog and needs bus monitor reset using MODE
register.

Signed-off-by: Suneel Garapati <sgarapati@marvell.com>
Signed-off-by: Piyush Malgujar <pmalgujar@marvell.com>
Acked-by: Andi Shyti <andi.shyti@kernel.org>
---
 drivers/i2c/busses/i2c-octeon-core.c | 8 ++++++++
 drivers/i2c/busses/i2c-octeon-core.h | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 6636719ca8f005056230620e2cee19de7154e024..0c89d8d640424356f1ea4f7da11d528631ae7efd 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -178,6 +178,7 @@ static int octeon_i2c_hlc_wait(struct octeon_i2c *i2c)
 static int octeon_i2c_check_status(struct octeon_i2c *i2c, int final_read)
 {
 	u8 stat;
+	u64 mode;
 
 	/*
 	 * This is ugly... in HLC mode the status is not in the status register
@@ -240,6 +241,13 @@ static int octeon_i2c_check_status(struct octeon_i2c *i2c, int final_read)
 	case STAT_RXADDR_NAK:
 	case STAT_AD2W_NAK:
 		return -ENXIO;
+
+	case STAT_WDOG_TOUT:
+		mode = __raw_readq(i2c->twsi_base + MODE(i2c));
+		/* Set BUS_MON_RST to reset bus monitor */
+		mode |= BUS_MON_RST_MASK;
+		octeon_i2c_writeq_flush(mode, i2c->twsi_base + MODE(i2c));
+		return -EIO;
 	default:
 		dev_err(i2c->dev, "unhandled state: %d\n", stat);
 		return -EIO;
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index e89f041550ace5f7cbcdd94146d0193abe51d466..e53fe60a41b7feb7ccc081cc671cec7be00c5a97 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -72,6 +72,7 @@
 #define STAT_SLAVE_ACK		0xC8
 #define STAT_AD2W_ACK		0xD0
 #define STAT_AD2W_NAK		0xD8
+#define STAT_WDOG_TOUT		0xF0
 #define STAT_IDLE		0xF8
 
 /* TWSI_INT values */
@@ -106,6 +107,9 @@ struct octeon_i2c_reg_offset {
 /* Set REFCLK_SRC and HS_MODE in TWSX_MODE register */
 #define TWSX_MODE_HS_MASK	(BIT(4) | BIT(0))
 
+/* Set BUS_MON_RST to reset bus monitor */
+#define BUS_MON_RST_MASK	BIT(3)
+
 struct octeon_i2c {
 	wait_queue_head_t queue;
 	struct i2c_adapter adap;
-- 
2.42.0

[PATCH v3 4/4] i2c: thunderx: Adding ioclk support

[Piyush Malgujar]

Adding support to use ioclk as reference clock if sclk not
present to make it SOC agnostic.
In case, it's not defined in dts/acpi table, use 800MHz as
default clock.

Signed-off-by: Piyush Malgujar <pmalgujar@marvell.com>
---
 drivers/i2c/busses/i2c-thunderx-pcidrv.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index 31f11b77ab663626967c86086a03213876bf4a07..15cf794a776533d1b0dbb08597fc0d9acf791b44 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -27,7 +27,7 @@
 
 #define PCI_DEVICE_ID_THUNDER_TWSI	0xa012
 
-#define SYS_FREQ_DEFAULT		700000000
+#define SYS_FREQ_DEFAULT		800000000
 #define OTX2_REF_FREQ_DEFAULT		100000000
 
 #define TWSI_INT_ENA_W1C		0x1028
@@ -100,7 +100,8 @@ static void thunder_i2c_clock_enable(struct device *dev, struct octeon_i2c *i2c)
 		i2c->sys_freq = clk_get_rate(i2c->clk);
 	} else {
 		/* ACPI */
-		device_property_read_u32(dev, "sclk", &i2c->sys_freq);
+		if (device_property_read_u32(dev, "sclk", &i2c->sys_freq))
+			device_property_read_u32(dev, "ioclk", &i2c->sys_freq);
 	}
 
 skip:
@@ -182,7 +183,6 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
 	if (!i2c->twsi_base)
 		return -EINVAL;
 
-	thunder_i2c_clock_enable(dev, i2c);
 	ret = device_property_read_u32(dev, "clock-frequency", &i2c->twsi_freq);
 	if (ret)
 		i2c->twsi_freq = I2C_MAX_STANDARD_MODE_FREQ;
@@ -196,12 +196,12 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
 
 	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
 	if (ret < 0)
-		goto error;
+		return ret;
 
 	ret = devm_request_irq(dev, pci_irq_vector(pdev, 0), octeon_i2c_isr, 0,
 			       DRV_NAME, i2c);
 	if (ret)
-		goto error;
+		return ret;
 
 	ret = octeon_i2c_init_lowlevel(i2c);
 	if (ret)
@@ -213,6 +213,9 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
 	 */
 	if (octeon_i2c_is_otx2(pdev) && IS_LS_FREQ(i2c->twsi_freq))
 		i2c->sys_freq = OTX2_REF_FREQ_DEFAULT;
+	else
+		thunder_i2c_clock_enable(dev, i2c);
+
 	octeon_i2c_set_clock(i2c);
 
 	i2c->adap = thunderx_i2c_ops;
@@ -240,6 +243,8 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
 
 error:
 	thunder_i2c_clock_disable(dev, i2c->clk);
+	if (!IS_LS_FREQ(i2c->twsi_freq))
+		thunder_i2c_clock_disable(dev, i2c->clk);
 	return ret;
 }
 
-- 
2.42.0

Re: [PATCH v3 1/4] i2c: thunderx: Clock divisor logic changes

[Andi Shyti]

Hi Piyush,

On Tue, Feb 06, 2024 at 03:43:46AM -0800, Piyush Malgujar wrote:
> From: Suneel Garapati <sgarapati@marvell.com>
> 
> Handle changes to clock divisor logic for OcteonTX2 SoC family using
> subsystem ID and using default reference clock source as 100MHz.
> 
> Signed-off-by: Suneel Garapati <sgarapati@marvell.com>
> Signed-off-by: Piyush Malgujar <pmalgujar@marvell.com>
> Acked-by: Andi Shyti <andi.shyti@kernel.org>
> ---
>  MAINTAINERS                              |  1 +
>  drivers/i2c/busses/i2c-octeon-core.c     | 29 ++++++++++++++++++++----
>  drivers/i2c/busses/i2c-octeon-core.h     | 17 ++++++++++++++
>  drivers/i2c/busses/i2c-thunderx-pcidrv.c |  7 ++++++
>  4 files changed, 50 insertions(+), 4 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 960512bec42885c0f1632a7c90851c3d32fbf20e..92b0a55c36e41cf54c7cbf52576d5424b591aa31 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4725,6 +4725,7 @@ F:	drivers/net/wireless/ath/carl9170/
>  
>  CAVIUM I2C DRIVER
>  M:	Robert Richter <rric@kernel.org>
> +M:	Suneel Garapati <sgarapati@marvell.com>

Suneel doesn't have much of a history here, though. Can I have an
ack from Robert?

In any case, can you please put this change outside from the
series as it would take a different path.

Thanks,
Andi

Re: [PATCH v3 1/4] i2c: thunderx: Clock divisor logic changes

[Andi Shyti]

Hi Suneel and Piyush,

...

>  void octeon_i2c_set_clock(struct octeon_i2c *i2c)
>  {
>  	int tclk, thp_base, inc, thp_idx, mdiv_idx, ndiv_idx, foscl, diff;
> -	int thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = 1000000;
> +	int mdiv_min = 2;

why is this int while all the other variables moved to be
"unsigned int"?

> +	/* starting value on search for lowest diff */
> +	const int huge_delta = INITIAL_DELTA_HZ;

this is not needed.

> +	/*
> +	 * Find divisors to produce target frequency, start with large delta
> +	 * to cover wider range of divisors, note thp = TCLK half period.
> +	 */
> +	unsigned int thp = 0x18, mdiv = 2, ndiv = 0, delta_hz = huge_delta;
> +
> +	if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev))) {

you could save this in a boolean at the beginning and keep using
it.

> +		thp = 0x3;

as you are here, can we give a meaningful define to 0x18 and 0x3?

> +		mdiv_min = 0;
> +	}
>  
>  	for (ndiv_idx = 0; ndiv_idx < 8 && delta_hz != 0; ndiv_idx++) {
>  		/*
>  		 * An mdiv value of less than 2 seems to not work well
>  		 * with ds1337 RTCs, so we constrain it to larger values.
>  		 */
> -		for (mdiv_idx = 15; mdiv_idx >= 2 && delta_hz != 0; mdiv_idx--) {
> +		for (mdiv_idx = 15; mdiv_idx >= mdiv_min && delta_hz != 0; mdiv_idx--) {
>  			/*
>  			 * For given ndiv and mdiv values check the
>  			 * two closest thp values.
>  			 */
>  			tclk = i2c->twsi_freq * (mdiv_idx + 1) * 10;
>  			tclk *= (1 << ndiv_idx);
> -			thp_base = (i2c->sys_freq / (tclk * 2)) - 1;
> +			if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev)))
> +				thp_base = (i2c->sys_freq / tclk) - 2;
> +			else
> +				thp_base = (i2c->sys_freq / (tclk * 2)) - 1;
>  
>  			for (inc = 0; inc <= 1; inc++) {
>  				thp_idx = thp_base + inc;
>  				if (thp_idx < 5 || thp_idx > 0xff)
>  					continue;
>  
> -				foscl = i2c->sys_freq / (2 * (thp_idx + 1));
> +				if (octeon_i2c_is_otx2(to_pci_dev(i2c->dev)))
> +					foscl = i2c->sys_freq / (thp_idx + 2);
> +				else
> +					foscl = i2c->sys_freq /
> +						(2 * (thp_idx + 1));

I need to trust you on this.

>  				foscl = foscl / (1 << ndiv_idx);
>  				foscl = foscl / (mdiv_idx + 1) / 10;
>  				diff = abs(foscl - i2c->twsi_freq);
> +				/* Use it if smaller diff from target */

can you please expand?

>  				if (diff < delta_hz) {
>  					delta_hz = diff;
>  					thp = thp_idx;
> diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
> index 9bb9f64fdda0392364638ecbaafe3fab5612baf6..694c24cecb7b144c1021549d1661b040c21bb998 100644
> --- a/drivers/i2c/busses/i2c-octeon-core.h
> +++ b/drivers/i2c/busses/i2c-octeon-core.h
> @@ -7,6 +7,7 @@
>  #include <linux/i2c-smbus.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
> +#include <linux/pci.h>
>  
>  /* Controller command patterns */
>  #define SW_TWSI_V		BIT_ULL(63)	/* Valid bit */
> @@ -98,6 +99,8 @@ struct octeon_i2c_reg_offset {
>  #define TWSI_INT(x)	(x->roff.twsi_int)
>  #define SW_TWSI_EXT(x)	(x->roff.sw_twsi_ext)
>  
> +#define INITIAL_DELTA_HZ	1000000

This define doesn't need to be in the header, right? Can we move
it in the c file?

Andi

Re: [PATCH v3 2/4] i2c: thunderx: Add support for High speed mode

[Andi Shyti]

On Tue, Feb 06, 2024 at 03:43:47AM -0800, Piyush Malgujar wrote:
> From: Suneel Garapati <sgarapati@marvell.com>
> 
> Support High speed mode clock setup for OcteonTX2 platforms.

How are you supporting the high speed mode clock setup? Can you
please give more details on the patch?

We don't have the specs and would love to understand more :-)

Thanks,
Andi

Re: [PATCH v3 3/4] i2c: octeon: Handle watchdog timeout

[Andi Shyti]

Hi,

On Tue, Feb 06, 2024 at 03:43:48AM -0800, Piyush Malgujar wrote:
> From: Suneel Garapati <sgarapati@marvell.com>
> 
> Status code 0xF0 refers to expiry of TWSI controller
> access watchdog and needs bus monitor reset using MODE
> register.

Please describe what you did, why you did it and how you did it.

Thanks,
Andi

Re: [PATCH v3 4/4] i2c: thunderx: Adding ioclk support

[Andi Shyti]

Hi Piyush,

On Tue, Feb 06, 2024 at 03:43:49AM -0800, Piyush Malgujar wrote:
> Adding support to use ioclk as reference clock if sclk not
> present to make it SOC agnostic.

Please use the imperative form to describe what you did. Did you
add support or what or "adding support..." what?

Andi