[PATCH v5 0/8] i2c: i2c-mlxbf.c: bug fixes and new feature support

[PATCH v5 0/8] i2c: i2c-mlxbf.c: bug fixes and new feature support

[Asmaa Mnebhi]

This is a series of patches fixing several bugs and implementing
new features.

Bug fixes:
1) Fix the frequency calculation
2) Fix incorrect base address passed during io write
3) prevent stack overflow in mlxbf_i2c_smbus_start_transaction()
4) Support lock mechanism to avoid race condition between entities
   using the i2c bus

Cleanup:
5) remove IRQF_ONESHOT flag as it is no longer needed.

Features:
6) Support multi slave functionality
7) Support BlueField-3 SoC
8) Update binding devicetree

What has changed from v4->v5:
Fix build error in the mellanox i2c device tree documentation

Asmaa Mnebhi (8):
  i2c: i2c-mlxbf.c: Fix frequency calculation
  i2c: i2c-mlxbf.c: remove IRQF_ONESHOT
  i2c: i2c-mlxbf.c: incorrect base address passed during io write
  i2c: i2c-mlxbf: prevent stack overflow in
    mlxbf_i2c_smbus_start_transaction()
  i2c: i2c-mlxbf.c: support lock mechanism
  i2c: i2c-mlxbf: add multi slave functionality
  i2c: i2c-mlxbf.c: support BlueField-3 SoC
  i2c: i2c-mlxbf.c: Update binding devicetree

 .../bindings/i2c/mellanox,i2c-mlxbf.yaml      |  48 +-
 MAINTAINERS                                   |   1 +
 drivers/i2c/busses/i2c-mlxbf.c                | 862 ++++++++++--------
 3 files changed, 521 insertions(+), 390 deletions(-)

-- 
2.30.1

[PATCH v5 1/8] i2c: i2c-mlxbf.c: Fix frequency calculation

[Asmaa Mnebhi]

The i2c-mlxbf.c driver is currently broken because there is a bug
in the calculation of the frequency. core_f, core_r and core_od
are components read from hardware registers and are used to
compute the frequency used to compute different timing parameters.
The shifting mechanism used to get core_f, core_r and core_od is
wrong. Use FIELD_GET to mask and shift the bitfields properly.

Fixes: b5b5b32081cd206b (i2c: mlxbf: I2C SMBus driver for Mellanox BlueField SoC)
Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 drivers/i2c/busses/i2c-mlxbf.c | 63 +++++++++++++---------------------
 1 file changed, 23 insertions(+), 40 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mlxbf.c b/drivers/i2c/busses/i2c-mlxbf.c
index 8716032f030a..ca8850241fa5 100644
--- a/drivers/i2c/busses/i2c-mlxbf.c
+++ b/drivers/i2c/busses/i2c-mlxbf.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/acpi.h>
+#include <linux/bitfield.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/interrupt.h>
@@ -63,13 +64,14 @@
  */
 #define MLXBF_I2C_TYU_PLL_OUT_FREQ  (400 * 1000 * 1000)
 /* Reference clock for Bluefield - 156 MHz. */
-#define MLXBF_I2C_PLL_IN_FREQ       (156 * 1000 * 1000)
+#define MLXBF_I2C_PLL_IN_FREQ       156250000ULL
 
 /* Constant used to determine the PLL frequency. */
-#define MLNXBF_I2C_COREPLL_CONST    16384
+#define MLNXBF_I2C_COREPLL_CONST    16384ULL
+
+#define MLXBF_I2C_FREQUENCY_1GHZ  1000000000ULL
 
 /* PLL registers. */
-#define MLXBF_I2C_CORE_PLL_REG0         0x0
 #define MLXBF_I2C_CORE_PLL_REG1         0x4
 #define MLXBF_I2C_CORE_PLL_REG2         0x8
 
@@ -181,22 +183,15 @@
 #define MLXBF_I2C_COREPLL_FREQ          MLXBF_I2C_TYU_PLL_OUT_FREQ
 
 /* Core PLL TYU configuration. */
-#define MLXBF_I2C_COREPLL_CORE_F_TYU_MASK   GENMASK(12, 0)
-#define MLXBF_I2C_COREPLL_CORE_OD_TYU_MASK  GENMASK(3, 0)
-#define MLXBF_I2C_COREPLL_CORE_R_TYU_MASK   GENMASK(5, 0)
-
-#define MLXBF_I2C_COREPLL_CORE_F_TYU_SHIFT  3
-#define MLXBF_I2C_COREPLL_CORE_OD_TYU_SHIFT 16
-#define MLXBF_I2C_COREPLL_CORE_R_TYU_SHIFT  20
+#define MLXBF_I2C_COREPLL_CORE_F_TYU_MASK   GENMASK(15, 3)
+#define MLXBF_I2C_COREPLL_CORE_OD_TYU_MASK  GENMASK(19, 16)
+#define MLXBF_I2C_COREPLL_CORE_R_TYU_MASK   GENMASK(25, 20)
 
 /* Core PLL YU configuration. */
 #define MLXBF_I2C_COREPLL_CORE_F_YU_MASK    GENMASK(25, 0)
 #define MLXBF_I2C_COREPLL_CORE_OD_YU_MASK   GENMASK(3, 0)
-#define MLXBF_I2C_COREPLL_CORE_R_YU_MASK    GENMASK(5, 0)
+#define MLXBF_I2C_COREPLL_CORE_R_YU_MASK    GENMASK(31, 26)
 
-#define MLXBF_I2C_COREPLL_CORE_F_YU_SHIFT   0
-#define MLXBF_I2C_COREPLL_CORE_OD_YU_SHIFT  1
-#define MLXBF_I2C_COREPLL_CORE_R_YU_SHIFT   26
 
 /* Core PLL frequency. */
 static u64 mlxbf_i2c_corepll_frequency;
@@ -479,8 +474,6 @@ static struct mutex mlxbf_i2c_bus_lock;
 #define MLXBF_I2C_MASK_8    GENMASK(7, 0)
 #define MLXBF_I2C_MASK_16   GENMASK(15, 0)
 
-#define MLXBF_I2C_FREQUENCY_1GHZ  1000000000
-
 /*
  * Function to poll a set of bits at a specific address; it checks whether
  * the bits are equal to zero when eq_zero is set to 'true', and not equal
@@ -1407,24 +1400,19 @@ static int mlxbf_i2c_init_master(struct platform_device *pdev,
 	return 0;
 }
 
-static u64 mlxbf_calculate_freq_from_tyu(struct mlxbf_i2c_resource *corepll_res)
+static u64 mlxbf_i2c_calculate_freq_from_tyu(struct mlxbf_i2c_resource *corepll_res)
 {
-	u64 core_frequency, pad_frequency;
+	u64 core_frequency;
 	u8 core_od, core_r;
 	u32 corepll_val;
 	u16 core_f;
 
-	pad_frequency = MLXBF_I2C_PLL_IN_FREQ;
-
 	corepll_val = readl(corepll_res->io + MLXBF_I2C_CORE_PLL_REG1);
 
 	/* Get Core PLL configuration bits. */
-	core_f = rol32(corepll_val, MLXBF_I2C_COREPLL_CORE_F_TYU_SHIFT) &
-			MLXBF_I2C_COREPLL_CORE_F_TYU_MASK;
-	core_od = rol32(corepll_val, MLXBF_I2C_COREPLL_CORE_OD_TYU_SHIFT) &
-			MLXBF_I2C_COREPLL_CORE_OD_TYU_MASK;
-	core_r = rol32(corepll_val, MLXBF_I2C_COREPLL_CORE_R_TYU_SHIFT) &
-			MLXBF_I2C_COREPLL_CORE_R_TYU_MASK;
+	core_f = FIELD_GET(MLXBF_I2C_COREPLL_CORE_F_TYU_MASK, corepll_val);
+	core_od = FIELD_GET(MLXBF_I2C_COREPLL_CORE_OD_TYU_MASK, corepll_val);
+	core_r = FIELD_GET(MLXBF_I2C_COREPLL_CORE_R_TYU_MASK, corepll_val);
 
 	/*
 	 * Compute PLL output frequency as follow:
@@ -1436,31 +1424,26 @@ static u64 mlxbf_calculate_freq_from_tyu(struct mlxbf_i2c_resource *corepll_res)
 	 * Where PLL_OUT_FREQ and PLL_IN_FREQ refer to CoreFrequency
 	 * and PadFrequency, respectively.
 	 */
-	core_frequency = pad_frequency * (++core_f);
+	core_frequency = MLXBF_I2C_PLL_IN_FREQ * (++core_f);
 	core_frequency /= (++core_r) * (++core_od);
 
 	return core_frequency;
 }
 
-static u64 mlxbf_calculate_freq_from_yu(struct mlxbf_i2c_resource *corepll_res)
+static u64 mlxbf_i2c_calculate_freq_from_yu(struct mlxbf_i2c_resource *corepll_res)
 {
 	u32 corepll_reg1_val, corepll_reg2_val;
-	u64 corepll_frequency, pad_frequency;
+	u64 corepll_frequency;
 	u8 core_od, core_r;
 	u32 core_f;
 
-	pad_frequency = MLXBF_I2C_PLL_IN_FREQ;
-
 	corepll_reg1_val = readl(corepll_res->io + MLXBF_I2C_CORE_PLL_REG1);
 	corepll_reg2_val = readl(corepll_res->io + MLXBF_I2C_CORE_PLL_REG2);
 
 	/* Get Core PLL configuration bits */
-	core_f = rol32(corepll_reg1_val, MLXBF_I2C_COREPLL_CORE_F_YU_SHIFT) &
-			MLXBF_I2C_COREPLL_CORE_F_YU_MASK;
-	core_r = rol32(corepll_reg1_val, MLXBF_I2C_COREPLL_CORE_R_YU_SHIFT) &
-			MLXBF_I2C_COREPLL_CORE_R_YU_MASK;
-	core_od = rol32(corepll_reg2_val,  MLXBF_I2C_COREPLL_CORE_OD_YU_SHIFT) &
-			MLXBF_I2C_COREPLL_CORE_OD_YU_MASK;
+	core_f = FIELD_GET(MLXBF_I2C_COREPLL_CORE_F_YU_MASK, corepll_reg1_val);
+	core_r = FIELD_GET(MLXBF_I2C_COREPLL_CORE_R_YU_MASK, corepll_reg1_val);
+	core_od = FIELD_GET(MLXBF_I2C_COREPLL_CORE_OD_YU_MASK, corepll_reg2_val);
 
 	/*
 	 * Compute PLL output frequency as follow:
@@ -1472,7 +1455,7 @@ static u64 mlxbf_calculate_freq_from_yu(struct mlxbf_i2c_resource *corepll_res)
 	 * Where PLL_OUT_FREQ and PLL_IN_FREQ refer to CoreFrequency
 	 * and PadFrequency, respectively.
 	 */
-	corepll_frequency = (pad_frequency * core_f) / MLNXBF_I2C_COREPLL_CONST;
+	corepll_frequency = (MLXBF_I2C_PLL_IN_FREQ * core_f) / MLNXBF_I2C_COREPLL_CONST;
 	corepll_frequency /= (++core_r) * (++core_od);
 
 	return corepll_frequency;
@@ -2180,14 +2163,14 @@ static struct mlxbf_i2c_chip_info mlxbf_i2c_chip[] = {
 			[1] = &mlxbf_i2c_corepll_res[MLXBF_I2C_CHIP_TYPE_1],
 			[2] = &mlxbf_i2c_gpio_res[MLXBF_I2C_CHIP_TYPE_1]
 		},
-		.calculate_freq = mlxbf_calculate_freq_from_tyu
+		.calculate_freq = mlxbf_i2c_calculate_freq_from_tyu
 	},
 	[MLXBF_I2C_CHIP_TYPE_2] = {
 		.type = MLXBF_I2C_CHIP_TYPE_2,
 		.shared_res = {
 			[0] = &mlxbf_i2c_corepll_res[MLXBF_I2C_CHIP_TYPE_2]
 		},
-		.calculate_freq = mlxbf_calculate_freq_from_yu
+		.calculate_freq = mlxbf_i2c_calculate_freq_from_yu
 	}
 };
 
-- 
2.30.1

[PATCH v5 2/8] i2c: i2c-mlxbf.c: remove IRQF_ONESHOT

[Asmaa Mnebhi]

IRQF_ONESHOT is not needed so remove it.

Fixes: b5b5b32081cd206b (i2c: mlxbf: I2C SMBus driver for Mellanox BlueField SoC)
Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 drivers/i2c/busses/i2c-mlxbf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mlxbf.c b/drivers/i2c/busses/i2c-mlxbf.c
index ca8850241fa5..8b8b7c8f7b19 100644
--- a/drivers/i2c/busses/i2c-mlxbf.c
+++ b/drivers/i2c/busses/i2c-mlxbf.c
@@ -2356,7 +2356,7 @@ static int mlxbf_i2c_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 	ret = devm_request_irq(dev, irq, mlxbf_smbus_irq,
-			       IRQF_ONESHOT | IRQF_SHARED | IRQF_PROBE_SHARED,
+			       IRQF_SHARED | IRQF_PROBE_SHARED,
 			       dev_name(dev), priv);
 	if (ret < 0) {
 		dev_err(dev, "Cannot get irq %d\n", irq);
-- 
2.30.1

[PATCH v5 3/8] i2c: i2c-mlxbf.c: incorrect base address passed during io write

[Asmaa Mnebhi]

Correct the base address used during io write.
This bug had no impact over the overall functionality of the read and write
transactions. MLXBF_I2C_CAUSE_OR_CLEAR=0x18 so writing to (smbus->io + 0x18)
corresponds to sc_low_timeout register which just sets the timeout value
before a read/write aborts.

Fixes: b5b5b32081cd206b (i2c: mlxbf: I2C SMBus driver for Mellanox BlueField SoC)
Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 drivers/i2c/busses/i2c-mlxbf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mlxbf.c b/drivers/i2c/busses/i2c-mlxbf.c
index 8b8b7c8f7b19..8a6a0ccc1c39 100644
--- a/drivers/i2c/busses/i2c-mlxbf.c
+++ b/drivers/i2c/busses/i2c-mlxbf.c
@@ -662,7 +662,7 @@ static int mlxbf_i2c_smbus_enable(struct mlxbf_i2c_priv *priv, u8 slave,
 	/* Clear status bits. */
 	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_STATUS);
 	/* Set the cause data. */
-	writel(~0x0, priv->smbus->io + MLXBF_I2C_CAUSE_OR_CLEAR);
+	writel(~0x0, priv->mst_cause->io + MLXBF_I2C_CAUSE_OR_CLEAR);
 	/* Zero PEC byte. */
 	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_PEC);
 	/* Zero byte count. */
-- 
2.30.1

[PATCH v5 4/8] i2c: i2c-mlxbf: prevent stack overflow in mlxbf_i2c_smbus_start_transaction()

[Asmaa Mnebhi]

memcpy() is called in a loop while 'operation->length' upper bound
is not checked and 'data_idx' also increments.

Fixes: b5b5b32081cd206b ("i2c: mlxbf: I2C SMBus driver for Mellanox BlueField SoC")
Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 drivers/i2c/busses/i2c-mlxbf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-mlxbf.c b/drivers/i2c/busses/i2c-mlxbf.c
index 8a6a0ccc1c39..1a0fc9640c23 100644
--- a/drivers/i2c/busses/i2c-mlxbf.c
+++ b/drivers/i2c/busses/i2c-mlxbf.c
@@ -731,6 +731,9 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
 		if (flags & MLXBF_I2C_F_WRITE) {
 			write_en = 1;
 			write_len += operation->length;
+			if (data_idx + operation->length >
+					MLXBF_I2C_MASTER_DATA_DESC_SIZE)
+				return -ENOBUFS;
 			memcpy(data_desc + data_idx,
 			       operation->buffer, operation->length);
 			data_idx += operation->length;
-- 
2.30.1

[PATCH v5 5/8] i2c: i2c-mlxbf.c: support lock mechanism

[Asmaa Mnebhi]

Linux is not the only entity using the BlueField I2C busses so
support a lock mechanism provided by hardware to avoid issues
when multiple entities are trying to access the same bus.

The lock is acquired whenever written explicitely or the lock
register is read. So make sure it is always released at the end
of a successful or failed transaction.

Fixes: b5b5b32081cd206b (i2c: mlxbf: I2C SMBus driver for Mellanox BlueField SoC)
Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 drivers/i2c/busses/i2c-mlxbf.c | 44 ++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mlxbf.c b/drivers/i2c/busses/i2c-mlxbf.c
index 1a0fc9640c23..78b2bc9b0a34 100644
--- a/drivers/i2c/busses/i2c-mlxbf.c
+++ b/drivers/i2c/busses/i2c-mlxbf.c
@@ -306,6 +306,7 @@ static u64 mlxbf_i2c_corepll_frequency;
  * exact.
  */
 #define MLXBF_I2C_SMBUS_TIMEOUT   (300 * 1000) /* 300ms */
+#define MLXBF_I2C_SMBUS_LOCK_POLL_TIMEOUT (300 * 1000) /* 300ms */
 
 /* Encapsulates timing parameters. */
 struct mlxbf_i2c_timings {
@@ -514,6 +515,25 @@ static bool mlxbf_smbus_master_wait_for_idle(struct mlxbf_i2c_priv *priv)
 	return false;
 }
 
+/*
+ * wait for the lock to be released before acquiring it.
+ */
+static bool mlxbf_i2c_smbus_master_lock(struct mlxbf_i2c_priv *priv)
+{
+	if (mlxbf_smbus_poll(priv->smbus->io, MLXBF_I2C_SMBUS_MASTER_GW,
+			   MLXBF_I2C_MASTER_LOCK_BIT, true,
+			   MLXBF_I2C_SMBUS_LOCK_POLL_TIMEOUT))
+		return true;
+
+	return false;
+}
+
+static void mlxbf_i2c_smbus_master_unlock(struct mlxbf_i2c_priv *priv)
+{
+	/* Clear the gw to clear the lock */
+	writel(0, priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_GW);
+}
+
 static bool mlxbf_i2c_smbus_transaction_success(u32 master_status,
 						u32 cause_status)
 {
@@ -705,10 +725,19 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
 	slave = request->slave & GENMASK(6, 0);
 	addr = slave << 1;
 
-	/* First of all, check whether the HW is idle. */
-	if (WARN_ON(!mlxbf_smbus_master_wait_for_idle(priv)))
+	/*
+	 * Try to acquire the smbus gw lock before any reads of the GW register since
+	 * a read sets the lock.
+	 */
+	if (WARN_ON(!mlxbf_i2c_smbus_master_lock(priv)))
 		return -EBUSY;
 
+	/* Check whether the HW is idle */
+	if (WARN_ON(!mlxbf_smbus_master_wait_for_idle(priv))) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
 	/* Set first byte. */
 	data_desc[data_idx++] = addr;
 
@@ -732,8 +761,10 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
 			write_en = 1;
 			write_len += operation->length;
 			if (data_idx + operation->length >
-					MLXBF_I2C_MASTER_DATA_DESC_SIZE)
-				return -ENOBUFS;
+					MLXBF_I2C_MASTER_DATA_DESC_SIZE) {
+				ret = -ENOBUFS;
+				goto out_unlock;
+			}
 			memcpy(data_desc + data_idx,
 			       operation->buffer, operation->length);
 			data_idx += operation->length;
@@ -765,7 +796,7 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
 		ret = mlxbf_i2c_smbus_enable(priv, slave, write_len, block_en,
 					 pec_en, 0);
 		if (ret)
-			return ret;
+			goto out_unlock;
 	}
 
 	if (read_en) {
@@ -792,6 +823,9 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
 			priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_FSM);
 	}
 
+out_unlock:
+	mlxbf_i2c_smbus_master_unlock(priv);
+
 	return ret;
 }
 
-- 
2.30.1

[PATCH v5 6/8] i2c: i2c-mlxbf: add multi slave functionality

[Asmaa Mnebhi]

Support the multi slave functionality which enables the BlueField
to be registered at up to 16 i2c slave addresses.

Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 drivers/i2c/busses/i2c-mlxbf.c | 320 +++++++++++++++------------------
 1 file changed, 149 insertions(+), 171 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mlxbf.c b/drivers/i2c/busses/i2c-mlxbf.c
index 78b2bc9b0a34..e9e700012136 100644
--- a/drivers/i2c/busses/i2c-mlxbf.c
+++ b/drivers/i2c/busses/i2c-mlxbf.c
@@ -298,9 +298,6 @@ static u64 mlxbf_i2c_corepll_frequency;
 #define MLXBF_I2C_SMBUS_SLAVE_ADDR_EN_BIT     7
 #define MLXBF_I2C_SMBUS_SLAVE_ADDR_MASK       GENMASK(6, 0)
 
-#define MLXBF_I2C_SLAVE_ADDR_ENABLED(addr) \
-	((addr) & (1 << MLXBF_I2C_SMBUS_SLAVE_ADDR_EN_BIT))
-
 /*
  * Timeout is given in microsends. Note also that timeout handling is not
  * exact.
@@ -426,7 +423,7 @@ struct mlxbf_i2c_priv {
 	u64 frequency; /* Core frequency in Hz. */
 	int bus; /* Physical bus identifier. */
 	int irq;
-	struct i2c_client *slave;
+	struct i2c_client *slave[MLXBF_I2C_SMBUS_SLAVE_ADDR_CNT];
 };
 
 static struct mlxbf_i2c_resource mlxbf_i2c_coalesce_res[] = {
@@ -1543,25 +1540,23 @@ static int mlxbf_i2c_calculate_corepll_freq(struct platform_device *pdev,
 	return 0;
 }
 
-static int mlxbf_slave_enable(struct mlxbf_i2c_priv *priv, u8 addr)
+static int mlxbf_i2c_slave_enable(struct mlxbf_i2c_priv *priv,
+			      struct i2c_client *slave)
 {
-	u32 slave_reg, slave_reg_tmp, slave_reg_avail, slave_addr_mask;
-	u8 reg, reg_cnt, byte, addr_tmp, reg_avail, byte_avail;
-	bool avail, disabled;
-
-	disabled = false;
-	avail = false;
+	u8 reg, reg_cnt, byte, addr_tmp;
+	u32 slave_reg, slave_reg_tmp;
 
 	if (!priv)
 		return -EPERM;
 
 	reg_cnt = MLXBF_I2C_SMBUS_SLAVE_ADDR_CNT >> 2;
-	slave_addr_mask = MLXBF_I2C_SMBUS_SLAVE_ADDR_MASK;
 
 	/*
 	 * Read the slave registers. There are 4 * 32-bit slave registers.
-	 * Each slave register can hold up to 4 * 8-bit slave configuration
-	 * (7-bit address, 1 status bit (1 if enabled, 0 if not)).
+	 * Each slave register can hold up to 4 * 8-bit slave configuration:
+	 * 1) A 7-bit address
+	 * 2) And a status bit (1 if enabled, 0 if not).
+	 * Look for the next available slave register slot.
 	 */
 	for (reg = 0; reg < reg_cnt; reg++) {
 		slave_reg = readl(priv->smbus->io +
@@ -1576,121 +1571,87 @@ static int mlxbf_slave_enable(struct mlxbf_i2c_priv *priv, u8 addr)
 			addr_tmp = slave_reg_tmp & GENMASK(7, 0);
 
 			/*
-			 * Mark the first available slave address slot, i.e. its
-			 * enabled bit should be unset. This slot might be used
-			 * later on to register our slave.
-			 */
-			if (!avail && !MLXBF_I2C_SLAVE_ADDR_ENABLED(addr_tmp)) {
-				avail = true;
-				reg_avail = reg;
-				byte_avail = byte;
-				slave_reg_avail = slave_reg;
-			}
-
-			/*
-			 * Parse slave address bytes and check whether the
-			 * slave address already exists and it's enabled,
-			 * i.e. most significant bit is set.
+			 * If an enable bit is not set in the
+			 * MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG register, then the
+			 * slave address slot associated with that bit is
+			 * free. So set the enable bit and write the
+			 * slave address bits.
 			 */
-			if ((addr_tmp & slave_addr_mask) == addr) {
-				if (MLXBF_I2C_SLAVE_ADDR_ENABLED(addr_tmp))
-					return 0;
-				disabled = true;
-				break;
+			if (!(addr_tmp & MLXBF_I2C_SMBUS_SLAVE_ADDR_EN_BIT)) {
+				slave_reg &= ~(MLXBF_I2C_SMBUS_SLAVE_ADDR_MASK << (byte * 8));
+				slave_reg |= (slave->addr << (byte * 8));
+				slave_reg |= MLXBF_I2C_SMBUS_SLAVE_ADDR_EN_BIT << (byte * 8);
+				writel(slave_reg, priv->smbus->io +
+					MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG +
+					(reg * 0x4));
+
+				/*
+				 * Set the slave at the corresponding index.
+				 */
+				priv->slave[(reg * 4) + byte] = slave;
+
+				return 0;
 			}
 
 			/* Parse next byte. */
 			slave_reg_tmp >>= 8;
 		}
-
-		/* Exit the loop if the slave address is found. */
-		if (disabled)
-			break;
 	}
 
-	if (!avail && !disabled)
-		return -EINVAL; /* No room for a new slave address. */
-
-	if (avail && !disabled) {
-		reg = reg_avail;
-		byte = byte_avail;
-		/* Set the slave address. */
-		slave_reg_avail &= ~(slave_addr_mask << (byte * 8));
-		slave_reg_avail |= addr << (byte * 8);
-		slave_reg = slave_reg_avail;
-	}
-
-	/* Enable the slave address and update the register. */
-	slave_reg |= (1 << MLXBF_I2C_SMBUS_SLAVE_ADDR_EN_BIT) << (byte * 8);
-	writel(slave_reg, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG +
-		reg * 0x4);
-
-	return 0;
+	return -EBUSY;
 }
 
-static int mlxbf_slave_disable(struct mlxbf_i2c_priv *priv)
+static int mlxbf_i2c_slave_disable(struct mlxbf_i2c_priv *priv, u8 addr)
 {
-	u32 slave_reg, slave_reg_tmp, slave_addr_mask;
-	u8 addr, addr_tmp, reg, reg_cnt, slave_byte;
-	struct i2c_client *client = priv->slave;
-	bool exist;
+	u8 addr_tmp, reg, reg_cnt, byte;
+	u32 slave_reg, slave_reg_tmp;
 
-	exist = false;
-
-	addr = client->addr;
 	reg_cnt = MLXBF_I2C_SMBUS_SLAVE_ADDR_CNT >> 2;
-	slave_addr_mask = MLXBF_I2C_SMBUS_SLAVE_ADDR_MASK;
 
 	/*
 	 * Read the slave registers. There are 4 * 32-bit slave registers.
-	 * Each slave register can hold up to 4 * 8-bit slave configuration
-	 * (7-bit address, 1 status bit (1 if enabled, 0 if not)).
+	 * Each slave register can hold up to 4 * 8-bit slave configuration:
+	 * 1) A 7-bit address
+	 * 2) And a status bit (1 if enabled, 0 if not).
+	 * Check if addr is present in the registers.
 	 */
 	for (reg = 0; reg < reg_cnt; reg++) {
 		slave_reg = readl(priv->smbus->io +
 				MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG + reg * 0x4);
 
 		/* Check whether the address slots are empty. */
-		if (slave_reg == 0)
+		if (!slave_reg)
 			continue;
 
 		/*
-		 * Each register holds 4 slave addresses. So, we have to keep
-		 * the byte order consistent with the value read in order to
-		 * update the register correctly, if needed.
+		 * Check if addr matches any of the 4 slave addresses
+		 * in the register.
 		 */
 		slave_reg_tmp = slave_reg;
-		slave_byte = 0;
-		while (slave_reg_tmp != 0) {
-			addr_tmp = slave_reg_tmp & slave_addr_mask;
+		for (byte = 0; byte < 4; byte++) {
+			addr_tmp = slave_reg_tmp & MLXBF_I2C_SMBUS_SLAVE_ADDR_MASK;
 			/*
 			 * Parse slave address bytes and check whether the
 			 * slave address already exists.
 			 */
 			if (addr_tmp == addr) {
-				exist = true;
-				break;
+				/* Clear the slave address slot. */
+				slave_reg &= ~(GENMASK(7, 0) << (byte * 8));
+				writel(slave_reg, priv->smbus->io +
+					MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG +
+					(reg * 0x4));
+				/* Free slave at the corresponding index */
+				priv->slave[(reg * 4) + byte] = NULL;
+
+				return 0;
 			}
 
 			/* Parse next byte. */
 			slave_reg_tmp >>= 8;
-			slave_byte += 1;
 		}
-
-		/* Exit the loop if the slave address is found. */
-		if (exist)
-			break;
 	}
 
-	if (!exist)
-		return 0; /* Slave is not registered, nothing to do. */
-
-	/* Cleanup the slave address slot. */
-	slave_reg &= ~(GENMASK(7, 0) << (slave_byte * 8));
-	writel(slave_reg, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG +
-		reg * 0x4);
-
-	return 0;
+	return -ENXIO;
 }
 
 static int mlxbf_i2c_init_coalesce(struct platform_device *pdev,
@@ -1852,72 +1813,81 @@ static bool mlxbf_smbus_slave_wait_for_idle(struct mlxbf_i2c_priv *priv,
 	return false;
 }
 
-/* Send byte to 'external' smbus master. */
-static int mlxbf_smbus_irq_send(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
+static struct i2c_client *mlxbf_i2c_get_slave_from_addr(
+			struct mlxbf_i2c_priv *priv, u8 addr)
 {
-	u8 data_desc[MLXBF_I2C_SLAVE_DATA_DESC_SIZE] = { 0 };
-	u8 write_size, pec_en, addr, byte, value, byte_cnt, desc_size;
-	struct i2c_client *slave = priv->slave;
-	u32 control32, data32;
-	int ret;
+	int i;
 
-	if (!slave)
-		return -EINVAL;
+	for (i = 0; i < MLXBF_I2C_SMBUS_SLAVE_ADDR_CNT; i++) {
+		if (!priv->slave[i])
+			continue;
 
-	addr = 0;
-	byte = 0;
-	desc_size = MLXBF_I2C_SLAVE_DATA_DESC_SIZE;
+		if (priv->slave[i]->addr == addr)
+			return priv->slave[i];
+	}
+
+	return NULL;
+}
+
+/*
+ * Send byte to 'external' smbus master. This function is executed when
+ * an external smbus master wants to read data from the BlueField.
+ */
+static int mlxbf_i2c_irq_send(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
+{
+	u8 data_desc[MLXBF_I2C_SLAVE_DATA_DESC_SIZE] = { 0 };
+	u8 write_size, pec_en, addr, value, byte_cnt;
+	struct i2c_client *slave;
+	u32 control32, data32;
+	int ret = 0;
 
 	/*
-	 * Read bytes received from the external master. These bytes should
-	 * be located in the first data descriptor register of the slave GW.
-	 * These bytes are the slave address byte and the internal register
-	 * address, if supplied.
+	 * Read the first byte received from the external master to
+	 * determine the slave address. This byte is located in the
+	 * first data descriptor register of the slave GW.
 	 */
-	if (recv_bytes > 0) {
-		data32 = ioread32be(priv->smbus->io +
-					MLXBF_I2C_SLAVE_DATA_DESC_ADDR);
-
-		/* Parse the received bytes. */
-		switch (recv_bytes) {
-		case 2:
-			byte = (data32 >> 8) & GENMASK(7, 0);
-			fallthrough;
-		case 1:
-			addr = (data32 & GENMASK(7, 0)) >> 1;
-		}
+	data32 = ioread32be(priv->smbus->io +
+				MLXBF_I2C_SLAVE_DATA_DESC_ADDR);
+	addr = (data32 & GENMASK(7, 0)) >> 1;
 
-		/* Check whether it's our slave address. */
-		if (slave->addr != addr)
-			return -EINVAL;
+	/*
+	 * Check if the slave address received in the data descriptor register
+	 * matches any of the slave addresses registered. If there is a match,
+	 * set the slave.
+	 */
+	slave = mlxbf_i2c_get_slave_from_addr(priv, addr);
+	if (!slave) {
+		ret = -ENXIO;
+		goto clear_csr;
 	}
 
 	/*
-	 * I2C read transactions may start by a WRITE followed by a READ.
-	 * Indeed, most slave devices would expect the internal address
-	 * following the slave address byte. So, write that byte first,
-	 * and then, send the requested data bytes to the master.
+	 * An I2C read can consist of a WRITE bit transaction followed by
+	 * a READ bit transaction. Indeed, slave devices often expect
+	 * the slave address to be followed by the internal address.
+	 * So, write the internal address byte first, and then, send the
+	 * requested data to the master.
 	 */
 	if (recv_bytes > 1) {
 		i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
-		value = byte;
+		value = (data32 >> 8) & GENMASK(7, 0);
 		ret = i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED,
 				      &value);
 		i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
 
 		if (ret < 0)
-			return ret;
+			goto clear_csr;
 	}
 
 	/*
-	 * Now, send data to the master; currently, the driver supports
-	 * READ_BYTE, READ_WORD and BLOCK READ protocols. Note that the
-	 * hardware can send up to 128 bytes per transfer. That is the
-	 * size of its data registers.
+	 * Send data to the master. Currently, the driver supports
+	 * READ_BYTE, READ_WORD and BLOCK READ protocols. The
+	 * hardware can send up to 128 bytes per transfer which is
+	 * the total size of the data registers.
 	 */
 	i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
 
-	for (byte_cnt = 0; byte_cnt < desc_size; byte_cnt++) {
+	for (byte_cnt = 0; byte_cnt < MLXBF_I2C_SLAVE_DATA_DESC_SIZE; byte_cnt++) {
 		data_desc[byte_cnt] = value;
 		i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
 	}
@@ -1925,8 +1895,6 @@ static int mlxbf_smbus_irq_send(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
 	/* Send a stop condition to the backend. */
 	i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
 
-	/* Handle the actual transfer. */
-
 	/* Set the number of bytes to write to master. */
 	write_size = (byte_cnt - 1) & 0x7f;
 
@@ -1949,38 +1917,44 @@ static int mlxbf_smbus_irq_send(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
 	 */
 	mlxbf_smbus_slave_wait_for_idle(priv, MLXBF_I2C_SMBUS_TIMEOUT);
 
+clear_csr:
 	/* Release the Slave GW. */
 	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_RS_MASTER_BYTES);
 	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_PEC);
 	writel(0x1, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_READY);
 
-	return 0;
+	return ret;
 }
 
-/* Receive bytes from 'external' smbus master. */
-static int mlxbf_smbus_irq_recv(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
+/*
+ * Receive bytes from 'external' smbus master. This function is executed when
+ * an external smbus master wants to write data to the BlueField.
+ */
+static int mlxbf_i2c_irq_recv(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
 {
 	u8 data_desc[MLXBF_I2C_SLAVE_DATA_DESC_SIZE] = { 0 };
-	struct i2c_client *slave = priv->slave;
+	struct i2c_client *slave;
 	u8 value, byte, addr;
 	int ret = 0;
 
-	if (!slave)
-		return -EINVAL;
-
 	/* Read data from Slave GW data descriptor. */
 	mlxbf_i2c_smbus_read_data(priv, data_desc, recv_bytes,
 				  MLXBF_I2C_SLAVE_DATA_DESC_ADDR);
-
-	/* Check whether its our slave address. */
 	addr = data_desc[0] >> 1;
-	if (slave->addr != addr)
-		return -EINVAL;
 
 	/*
-	 * Notify the slave backend; another I2C master wants to write data
-	 * to us. This event is sent once the slave address and the write bit
-	 * is detected.
+	 * Check if the slave address received in the data descriptor register
+	 * matches any of the slave addresses registered.
+	 */
+	slave = mlxbf_i2c_get_slave_from_addr(priv, addr);
+	if (!slave) {
+		ret = -EINVAL;
+		goto clear_csr;
+	}
+
+	/*
+	 * Notify the slave backend that an smbus master wants to write data
+	 * to the BlueField.
 	 */
 	i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
 
@@ -1993,9 +1967,13 @@ static int mlxbf_smbus_irq_recv(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
 			break;
 	}
 
-	/* Send a stop condition to the backend. */
+	/*
+	 * Send a stop event to the slave backend, to signal
+	 * the end of the write transactions.
+	 */
 	i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
 
+clear_csr:
 	/* Release the Slave GW. */
 	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_RS_MASTER_BYTES);
 	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_PEC);
@@ -2004,7 +1982,7 @@ static int mlxbf_smbus_irq_recv(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
 	return ret;
 }
 
-static irqreturn_t mlxbf_smbus_irq(int irq, void *ptr)
+static irqreturn_t mlxbf_i2c_irq(int irq, void *ptr)
 {
 	struct mlxbf_i2c_priv *priv = ptr;
 	bool read, write, irq_is_set;
@@ -2052,9 +2030,9 @@ static irqreturn_t mlxbf_smbus_irq(int irq, void *ptr)
 		MLXBF_I2C_SLAVE_DATA_DESC_SIZE : recv_bytes;
 
 	if (read)
-		mlxbf_smbus_irq_send(priv, recv_bytes);
+		mlxbf_i2c_irq_send(priv, recv_bytes);
 	else
-		mlxbf_smbus_irq_recv(priv, recv_bytes);
+		mlxbf_i2c_irq_recv(priv, recv_bytes);
 
 	return IRQ_HANDLED;
 }
@@ -2149,23 +2127,21 @@ static s32 mlxbf_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr,
 static int mlxbf_i2c_reg_slave(struct i2c_client *slave)
 {
 	struct mlxbf_i2c_priv *priv = i2c_get_adapdata(slave->adapter);
+	struct device *dev = &slave->dev;
 	int ret;
 
-	if (priv->slave)
-		return -EBUSY;
-
 	/*
 	 * Do not support ten bit chip address and do not use Packet Error
 	 * Checking (PEC).
 	 */
-	if (slave->flags & (I2C_CLIENT_TEN | I2C_CLIENT_PEC))
+	if (slave->flags & (I2C_CLIENT_TEN | I2C_CLIENT_PEC)) {
+		dev_err(dev, "SMBus PEC and 10 bit address not supported\n");
 		return -EAFNOSUPPORT;
+	}
 
-	ret = mlxbf_slave_enable(priv, slave->addr);
-	if (ret < 0)
-		return ret;
-
-	priv->slave = slave;
+	ret = mlxbf_i2c_slave_enable(priv, slave);
+	if (ret)
+		dev_err(dev, "Surpassed max number of registered slaves allowed\n");
 
 	return 0;
 }
@@ -2173,18 +2149,19 @@ static int mlxbf_i2c_reg_slave(struct i2c_client *slave)
 static int mlxbf_i2c_unreg_slave(struct i2c_client *slave)
 {
 	struct mlxbf_i2c_priv *priv = i2c_get_adapdata(slave->adapter);
+	struct device *dev = &slave->dev;
 	int ret;
 
-	WARN_ON(!priv->slave);
-
-	/* Unregister slave, i.e. disable the slave address in hardware. */
-	ret = mlxbf_slave_disable(priv);
-	if (ret < 0)
-		return ret;
-
-	priv->slave = NULL;
+	/*
+	 * Unregister slave by:
+	 * 1) Disabling the slave address in hardware
+	 * 2) Freeing priv->slave at the corresponding index
+	 */
+	ret = mlxbf_i2c_slave_disable(priv, slave->addr);
+	if (ret)
+		dev_err(dev, "Unable to find slave 0x%x\n", slave->addr);
 
-	return 0;
+	return ret;
 }
 
 static u32 mlxbf_i2c_functionality(struct i2c_adapter *adap)
@@ -2392,7 +2369,7 @@ static int mlxbf_i2c_probe(struct platform_device *pdev)
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
 		return irq;
-	ret = devm_request_irq(dev, irq, mlxbf_smbus_irq,
+	ret = devm_request_irq(dev, irq, mlxbf_i2c_irq,
 			       IRQF_SHARED | IRQF_PROBE_SHARED,
 			       dev_name(dev), priv);
 	if (ret < 0) {
@@ -2487,4 +2464,5 @@ module_exit(mlxbf_i2c_exit);
 
 MODULE_DESCRIPTION("Mellanox BlueField I2C bus driver");
 MODULE_AUTHOR("Khalil Blaiech <kblaiech@nvidia.com>");
+MODULE_AUTHOR("Asmaa Mnebhi <asmaa@nvidia.com>");
 MODULE_LICENSE("GPL v2");
-- 
2.30.1

[PATCH v5 7/8] i2c: i2c-mlxbf.c: support BlueField-3 SoC

[Asmaa Mnebhi]

BlueField-3 SoC has the same I2C IP logic as previous
BlueField-1 and 2 SoCs but it has different registers' addresses.
This is an effort to keep this driver generic accross all
BlueField generations.
This patch breaks down the "smbus" resource into 3 separate
resources to enable us to use common registers' offsets for all
BlueField SoCs:
struct mlxbf_i2c_resource *timer;
struct mlxbf_i2c_resource *mst;
struct mlxbf_i2c_resource *slv;

Of course, all offsets had to be adjusted accordingly, and we took
this chance to reorganize the macros depending on the register block
they target.

There are only 2 registers' offsets that do not fit within this
schema so their offsets are passed as SoC-specific parameters:
smbus_master_rs_bytes_off
smbus_master_fsm_off

Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 MAINTAINERS                    |   1 +
 drivers/i2c/busses/i2c-mlxbf.c | 450 ++++++++++++++++++++-------------
 2 files changed, 280 insertions(+), 171 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f512b430c7cb..9f03c945bedd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12978,6 +12978,7 @@ F:	drivers/input/touchscreen/melfas_mip4.c
 
 MELLANOX BLUEFIELD I2C DRIVER
 M:	Khalil Blaiech <kblaiech@nvidia.com>
+M:	Asmaa Mnebhi <asmaa@nvidia.com>
 L:	linux-i2c@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.yaml
diff --git a/drivers/i2c/busses/i2c-mlxbf.c b/drivers/i2c/busses/i2c-mlxbf.c
index e9e700012136..30e6ba1f412c 100644
--- a/drivers/i2c/busses/i2c-mlxbf.c
+++ b/drivers/i2c/busses/i2c-mlxbf.c
@@ -32,8 +32,6 @@
 	(MLXBF_I2C_FUNC_SMBUS_DEFAULT | MLXBF_I2C_FUNC_SMBUS_BLOCK | \
 	 I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SLAVE)
 
-#define MLXBF_I2C_SMBUS_MAX        3
-
 /* Shared resources info in BlueField platforms. */
 
 #define MLXBF_I2C_COALESCE_TYU_ADDR    0x02801300
@@ -48,6 +46,9 @@
 #define MLXBF_I2C_COREPLL_YU_ADDR      0x02800c30
 #define MLXBF_I2C_COREPLL_YU_SIZE      0x00c
 
+#define MLXBF_I2C_COREPLL_RSH_YU_ADDR  0x13409824
+#define MLXBF_I2C_COREPLL_RSH_YU_SIZE  0x00c
+
 #define MLXBF_I2C_SHARED_RES_MAX       3
 
 /*
@@ -131,14 +132,10 @@
 /* Slave busy bit reset. */
 #define MLXBF_I2C_CAUSE_S_GW_BUSY_FALL        BIT(18)
 
-#define MLXBF_I2C_CAUSE_SLAVE_ARBITER_BITS_MASK     GENMASK(20, 0)
-
 /* Cause coalesce registers. */
 #define MLXBF_I2C_CAUSE_COALESCE_0        0x00
-#define MLXBF_I2C_CAUSE_COALESCE_1        0x04
-#define MLXBF_I2C_CAUSE_COALESCE_2        0x08
 
-#define MLXBF_I2C_CAUSE_TYU_SLAVE_BIT   MLXBF_I2C_SMBUS_MAX
+#define MLXBF_I2C_CAUSE_TYU_SLAVE_BIT   3
 #define MLXBF_I2C_CAUSE_YU_SLAVE_BIT    1
 
 /* Functional enable register. */
@@ -165,15 +162,6 @@
 #define MLXBF_I2C_GPIO_SMBUS_GW_ASSERT_PINS(num, val) \
 	((val) | (0x3 << MLXBF_I2C_GPIO_SMBUS_GW_PINS(num)))
 
-/* SMBus timing parameters. */
-#define MLXBF_I2C_SMBUS_TIMER_SCL_LOW_SCL_HIGH    0x00
-#define MLXBF_I2C_SMBUS_TIMER_FALL_RISE_SPIKE     0x04
-#define MLXBF_I2C_SMBUS_TIMER_THOLD               0x08
-#define MLXBF_I2C_SMBUS_TIMER_TSETUP_START_STOP   0x0c
-#define MLXBF_I2C_SMBUS_TIMER_TSETUP_DATA         0x10
-#define MLXBF_I2C_SMBUS_THIGH_MAX_TBUF            0x14
-#define MLXBF_I2C_SMBUS_SCL_LOW_TIMEOUT           0x18
-
 /*
  * Defines SMBus operating frequency and core clock frequency.
  * According to ADB files, default values are compliant to 100KHz SMBus
@@ -192,26 +180,37 @@
 #define MLXBF_I2C_COREPLL_CORE_OD_YU_MASK   GENMASK(3, 0)
 #define MLXBF_I2C_COREPLL_CORE_R_YU_MASK    GENMASK(31, 26)
 
+/* SMBus timing parameters. */
+#define MLXBF_I2C_SMBUS_TIMER_SCL_LOW_SCL_HIGH    0x00
+#define MLXBF_I2C_SMBUS_TIMER_FALL_RISE_SPIKE     0x04
+#define MLXBF_I2C_SMBUS_TIMER_THOLD               0x08
+#define MLXBF_I2C_SMBUS_TIMER_TSETUP_START_STOP   0x0c
+#define MLXBF_I2C_SMBUS_TIMER_TSETUP_DATA         0x10
+#define MLXBF_I2C_SMBUS_THIGH_MAX_TBUF            0x14
+#define MLXBF_I2C_SMBUS_SCL_LOW_TIMEOUT           0x18
 
-/* Core PLL frequency. */
-static u64 mlxbf_i2c_corepll_frequency;
+#define MLXBF_I2C_SHIFT_0   0
+#define MLXBF_I2C_SHIFT_8   8
+#define MLXBF_I2C_SHIFT_16  16
+#define MLXBF_I2C_SHIFT_24  24
+
+#define MLXBF_I2C_MASK_8    GENMASK(7, 0)
+#define MLXBF_I2C_MASK_16   GENMASK(15, 0)
+
+#define MLXBF_I2C_MST_ADDR_OFFSET         0x200
 
 /* SMBus Master GW. */
-#define MLXBF_I2C_SMBUS_MASTER_GW     0x200
+#define MLXBF_I2C_SMBUS_MASTER_GW         0x0
 /* Number of bytes received and sent. */
-#define MLXBF_I2C_SMBUS_RS_BYTES      0x300
+#define MLXBF_I2C_YU_SMBUS_RS_BYTES       0x100
+#define MLXBF_I2C_RSH_YU_SMBUS_RS_BYTES   0x10c
 /* Packet error check (PEC) value. */
-#define MLXBF_I2C_SMBUS_MASTER_PEC    0x304
+#define MLXBF_I2C_SMBUS_MASTER_PEC        0x104
 /* Status bits (ACK/NACK/FW Timeout). */
-#define MLXBF_I2C_SMBUS_MASTER_STATUS 0x308
+#define MLXBF_I2C_SMBUS_MASTER_STATUS     0x108
 /* SMbus Master Finite State Machine. */
-#define MLXBF_I2C_SMBUS_MASTER_FSM    0x310
-
-/*
- * When enabled, the master will issue a stop condition in case of
- * timeout while waiting for FW response.
- */
-#define MLXBF_I2C_SMBUS_EN_FW_TIMEOUT 0x31c
+#define MLXBF_I2C_YU_SMBUS_MASTER_FSM     0x110
+#define MLXBF_I2C_RSH_YU_SMBUS_MASTER_FSM 0x100
 
 /* SMBus master GW control bits offset in MLXBF_I2C_SMBUS_MASTER_GW[31:3]. */
 #define MLXBF_I2C_MASTER_LOCK_BIT         BIT(31) /* Lock bit. */
@@ -231,14 +230,14 @@ static u64 mlxbf_i2c_corepll_frequency;
 #define MLXBF_I2C_MASTER_ENABLE_READ \
 	(MLXBF_I2C_MASTER_ENABLE | MLXBF_I2C_MASTER_CTL_READ_BIT)
 
-#define MLXBF_I2C_MASTER_SLV_ADDR_SHIFT   12 /* Slave address shift. */
-#define MLXBF_I2C_MASTER_WRITE_SHIFT      21 /* Control write bytes shift. */
-#define MLXBF_I2C_MASTER_SEND_PEC_SHIFT   20 /* Send PEC byte shift. */
-#define MLXBF_I2C_MASTER_PARSE_EXP_SHIFT  11 /* Parse expected bytes shift. */
-#define MLXBF_I2C_MASTER_READ_SHIFT       4  /* Control read bytes shift. */
+#define MLXBF_I2C_MASTER_WRITE_SHIFT      21 /* Control write bytes */
+#define MLXBF_I2C_MASTER_SEND_PEC_SHIFT   20 /* Send PEC byte when set to 1 */
+#define MLXBF_I2C_MASTER_PARSE_EXP_SHIFT  11 /* Control parse expected bytes */
+#define MLXBF_I2C_MASTER_SLV_ADDR_SHIFT   12 /* Slave address */
+#define MLXBF_I2C_MASTER_READ_SHIFT       4  /* Control read bytes */
 
 /* SMBus master GW Data descriptor. */
-#define MLXBF_I2C_MASTER_DATA_DESC_ADDR   0x280
+#define MLXBF_I2C_MASTER_DATA_DESC_ADDR   0x80
 #define MLXBF_I2C_MASTER_DATA_DESC_SIZE   0x80 /* Size in bytes. */
 
 /* Maximum bytes to read/write per SMBus transaction. */
@@ -264,19 +263,21 @@ static u64 mlxbf_i2c_corepll_frequency;
 #define MLXBF_I2C_SMBUS_MASTER_FSM_STOP_MASK      BIT(31)
 #define MLXBF_I2C_SMBUS_MASTER_FSM_PS_STATE_MASK  BIT(15)
 
+#define MLXBF_I2C_SLV_ADDR_OFFSET             0x400
+
 /* SMBus slave GW. */
-#define MLXBF_I2C_SMBUS_SLAVE_GW              0x400
+#define MLXBF_I2C_SMBUS_SLAVE_GW              0x0
 /* Number of bytes received and sent from/to master. */
-#define MLXBF_I2C_SMBUS_SLAVE_RS_MASTER_BYTES 0x500
+#define MLXBF_I2C_SMBUS_SLAVE_RS_MASTER_BYTES 0x100
 /* Packet error check (PEC) value. */
-#define MLXBF_I2C_SMBUS_SLAVE_PEC             0x504
+#define MLXBF_I2C_SMBUS_SLAVE_PEC             0x104
 /* SMBus slave Finite State Machine (FSM). */
-#define MLXBF_I2C_SMBUS_SLAVE_FSM             0x510
+#define MLXBF_I2C_SMBUS_SLAVE_FSM             0x110
 /*
  * Should be set when all raised causes handled, and cleared by HW on
  * every new cause.
  */
-#define MLXBF_I2C_SMBUS_SLAVE_READY           0x52c
+#define MLXBF_I2C_SMBUS_SLAVE_READY           0x12c
 
 /* SMBus slave GW control bits offset in MLXBF_I2C_SMBUS_SLAVE_GW[31:19]. */
 #define MLXBF_I2C_SLAVE_BUSY_BIT         BIT(30) /* Busy bit. */
@@ -289,13 +290,13 @@ static u64 mlxbf_i2c_corepll_frequency;
 #define MLXBF_I2C_SLAVE_SEND_PEC_SHIFT    21 /* Send PEC byte shift. */
 
 /* SMBus slave GW Data descriptor. */
-#define MLXBF_I2C_SLAVE_DATA_DESC_ADDR   0x480
+#define MLXBF_I2C_SLAVE_DATA_DESC_ADDR   0x80
 #define MLXBF_I2C_SLAVE_DATA_DESC_SIZE   0x80 /* Size in bytes. */
 
 /* SMbus slave configuration registers. */
-#define MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG        0x514
+#define MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG        0x114
 #define MLXBF_I2C_SMBUS_SLAVE_ADDR_CNT        16
-#define MLXBF_I2C_SMBUS_SLAVE_ADDR_EN_BIT     7
+#define MLXBF_I2C_SMBUS_SLAVE_ADDR_EN_BIT     BIT(7)
 #define MLXBF_I2C_SMBUS_SLAVE_ADDR_MASK       GENMASK(6, 0)
 
 /*
@@ -305,6 +306,59 @@ static u64 mlxbf_i2c_corepll_frequency;
 #define MLXBF_I2C_SMBUS_TIMEOUT   (300 * 1000) /* 300ms */
 #define MLXBF_I2C_SMBUS_LOCK_POLL_TIMEOUT (300 * 1000) /* 300ms */
 
+/* Polling frequency in microseconds. */
+#define MLXBF_I2C_POLL_FREQ_IN_USEC        200
+
+#define MLXBF_I2C_SMBUS_OP_CNT_1   1
+#define MLXBF_I2C_SMBUS_OP_CNT_2   2
+#define MLXBF_I2C_SMBUS_OP_CNT_3   3
+#define MLXBF_I2C_SMBUS_MAX_OP_CNT MLXBF_I2C_SMBUS_OP_CNT_3
+
+/* Helper macro to define an I2C resource parameters. */
+#define MLXBF_I2C_RES_PARAMS(addr, size, str) \
+	{ \
+		.start = (addr), \
+		.end = (addr) + (size) - 1, \
+		.name = (str) \
+	}
+
+enum {
+	MLXBF_I2C_TIMING_100KHZ = 100000,
+	MLXBF_I2C_TIMING_400KHZ = 400000,
+	MLXBF_I2C_TIMING_1000KHZ = 1000000,
+};
+
+enum {
+	MLXBF_I2C_F_READ = BIT(0),
+	MLXBF_I2C_F_WRITE = BIT(1),
+	MLXBF_I2C_F_NORESTART = BIT(3),
+	MLXBF_I2C_F_SMBUS_OPERATION = BIT(4),
+	MLXBF_I2C_F_SMBUS_BLOCK = BIT(5),
+	MLXBF_I2C_F_SMBUS_PEC = BIT(6),
+	MLXBF_I2C_F_SMBUS_PROCESS_CALL = BIT(7),
+};
+
+/* Mellanox BlueField chip type. */
+enum mlxbf_i2c_chip_type {
+	MLXBF_I2C_CHIP_TYPE_1, /* Mellanox BlueField-1 chip. */
+	MLXBF_I2C_CHIP_TYPE_2, /* Mellanox BlueField-2 chip. */
+	MLXBF_I2C_CHIP_TYPE_3 /* Mellanox BlueField-3 chip. */
+};
+
+/* List of chip resources that are being accessed by the driver. */
+enum {
+	MLXBF_I2C_SMBUS_RES,
+	MLXBF_I2C_MST_CAUSE_RES,
+	MLXBF_I2C_SLV_CAUSE_RES,
+	MLXBF_I2C_COALESCE_RES,
+	MLXBF_I2C_SMBUS_TIMER_RES,
+	MLXBF_I2C_SMBUS_MST_RES,
+	MLXBF_I2C_SMBUS_SLV_RES,
+	MLXBF_I2C_COREPLL_RES,
+	MLXBF_I2C_GPIO_RES,
+	MLXBF_I2C_END_RES
+};
+
 /* Encapsulates timing parameters. */
 struct mlxbf_i2c_timings {
 	u16 scl_high;		/* Clock high period. */
@@ -324,27 +378,12 @@ struct mlxbf_i2c_timings {
 	u32 timeout;		/* Detect clock low timeout. */
 };
 
-enum {
-	MLXBF_I2C_F_READ = BIT(0),
-	MLXBF_I2C_F_WRITE = BIT(1),
-	MLXBF_I2C_F_NORESTART = BIT(3),
-	MLXBF_I2C_F_SMBUS_OPERATION = BIT(4),
-	MLXBF_I2C_F_SMBUS_BLOCK = BIT(5),
-	MLXBF_I2C_F_SMBUS_PEC = BIT(6),
-	MLXBF_I2C_F_SMBUS_PROCESS_CALL = BIT(7),
-};
-
 struct mlxbf_i2c_smbus_operation {
 	u32 flags;
 	u32 length; /* Buffer length in bytes. */
 	u8 *buffer;
 };
 
-#define MLXBF_I2C_SMBUS_OP_CNT_1	1
-#define MLXBF_I2C_SMBUS_OP_CNT_2	2
-#define MLXBF_I2C_SMBUS_OP_CNT_3	3
-#define MLXBF_I2C_SMBUS_MAX_OP_CNT	MLXBF_I2C_SMBUS_OP_CNT_3
-
 struct mlxbf_i2c_smbus_request {
 	u8 slave;
 	u8 operation_cnt;
@@ -358,24 +397,38 @@ struct mlxbf_i2c_resource {
 	u8 type;
 };
 
-/* List of chip resources that are being accessed by the driver. */
-enum {
-	MLXBF_I2C_SMBUS_RES,
-	MLXBF_I2C_MST_CAUSE_RES,
-	MLXBF_I2C_SLV_CAUSE_RES,
-	MLXBF_I2C_COALESCE_RES,
-	MLXBF_I2C_COREPLL_RES,
-	MLXBF_I2C_GPIO_RES,
-	MLXBF_I2C_END_RES,
+struct mlxbf_i2c_chip_info {
+	enum mlxbf_i2c_chip_type type;
+	/* Chip shared resources that are being used by the I2C controller. */
+	struct mlxbf_i2c_resource *shared_res[MLXBF_I2C_SHARED_RES_MAX];
+
+	/* Callback to calculate the core PLL frequency. */
+	u64 (*calculate_freq)(struct mlxbf_i2c_resource *corepll_res);
+
+	/* Registers' address offset */
+	u32 smbus_master_rs_bytes_off;
+	u32 smbus_master_fsm_off;
 };
 
-/* Helper macro to define an I2C resource parameters. */
-#define MLXBF_I2C_RES_PARAMS(addr, size, str) \
-	{ \
-		.start = (addr), \
-		.end = (addr) + (size) - 1, \
-		.name = (str) \
-	}
+struct mlxbf_i2c_priv {
+	const struct mlxbf_i2c_chip_info *chip;
+	struct i2c_adapter adap;
+	struct mlxbf_i2c_resource *smbus;
+	struct mlxbf_i2c_resource *timer;
+	struct mlxbf_i2c_resource *mst;
+	struct mlxbf_i2c_resource *slv;
+	struct mlxbf_i2c_resource *mst_cause;
+	struct mlxbf_i2c_resource *slv_cause;
+	struct mlxbf_i2c_resource *coalesce;
+	u64 frequency; /* Core frequency in Hz. */
+	int bus; /* Physical bus identifier. */
+	int irq;
+	struct i2c_client *slave[MLXBF_I2C_SMBUS_SLAVE_ADDR_CNT];
+	u32 resource_version;
+};
+
+/* Core PLL frequency. */
+static u64 mlxbf_i2c_corepll_frequency;
 
 static struct resource mlxbf_i2c_coalesce_tyu_params =
 		MLXBF_I2C_RES_PARAMS(MLXBF_I2C_COALESCE_TYU_ADDR,
@@ -389,6 +442,10 @@ static struct resource mlxbf_i2c_corepll_yu_params =
 		MLXBF_I2C_RES_PARAMS(MLXBF_I2C_COREPLL_YU_ADDR,
 				     MLXBF_I2C_COREPLL_YU_SIZE,
 				     "COREPLL_MEM");
+static struct resource mlxbf_i2c_corepll_rsh_yu_params =
+		MLXBF_I2C_RES_PARAMS(MLXBF_I2C_COREPLL_RSH_YU_ADDR,
+				     MLXBF_I2C_COREPLL_RSH_YU_SIZE,
+				     "COREPLL_MEM");
 static struct resource mlxbf_i2c_gpio_tyu_params =
 		MLXBF_I2C_RES_PARAMS(MLXBF_I2C_GPIO_TYU_ADDR,
 				     MLXBF_I2C_GPIO_TYU_SIZE,
@@ -398,34 +455,6 @@ static struct mutex mlxbf_i2c_coalesce_lock;
 static struct mutex mlxbf_i2c_corepll_lock;
 static struct mutex mlxbf_i2c_gpio_lock;
 
-/* Mellanox BlueField chip type. */
-enum mlxbf_i2c_chip_type {
-	MLXBF_I2C_CHIP_TYPE_1, /* Mellanox BlueField-1 chip. */
-	MLXBF_I2C_CHIP_TYPE_2, /* Mallanox BlueField-2 chip. */
-};
-
-struct mlxbf_i2c_chip_info {
-	enum mlxbf_i2c_chip_type type;
-	/* Chip shared resources that are being used by the I2C controller. */
-	struct mlxbf_i2c_resource *shared_res[MLXBF_I2C_SHARED_RES_MAX];
-
-	/* Callback to calculate the core PLL frequency. */
-	u64 (*calculate_freq)(struct mlxbf_i2c_resource *corepll_res);
-};
-
-struct mlxbf_i2c_priv {
-	const struct mlxbf_i2c_chip_info *chip;
-	struct i2c_adapter adap;
-	struct mlxbf_i2c_resource *smbus;
-	struct mlxbf_i2c_resource *mst_cause;
-	struct mlxbf_i2c_resource *slv_cause;
-	struct mlxbf_i2c_resource *coalesce;
-	u64 frequency; /* Core frequency in Hz. */
-	int bus; /* Physical bus identifier. */
-	int irq;
-	struct i2c_client *slave[MLXBF_I2C_SMBUS_SLAVE_ADDR_CNT];
-};
-
 static struct mlxbf_i2c_resource mlxbf_i2c_coalesce_res[] = {
 	[MLXBF_I2C_CHIP_TYPE_1] = {
 		.params = &mlxbf_i2c_coalesce_tyu_params,
@@ -445,6 +474,11 @@ static struct mlxbf_i2c_resource mlxbf_i2c_corepll_res[] = {
 		.params = &mlxbf_i2c_corepll_yu_params,
 		.lock = &mlxbf_i2c_corepll_lock,
 		.type = MLXBF_I2C_COREPLL_RES,
+	},
+	[MLXBF_I2C_CHIP_TYPE_3] = {
+		.params = &mlxbf_i2c_corepll_rsh_yu_params,
+		.lock = &mlxbf_i2c_corepll_lock,
+		.type = MLXBF_I2C_COREPLL_RES,
 	}
 };
 
@@ -461,24 +495,13 @@ static u8 mlxbf_i2c_bus_count;
 
 static struct mutex mlxbf_i2c_bus_lock;
 
-/* Polling frequency in microseconds. */
-#define MLXBF_I2C_POLL_FREQ_IN_USEC        200
-
-#define MLXBF_I2C_SHIFT_0   0
-#define MLXBF_I2C_SHIFT_8   8
-#define MLXBF_I2C_SHIFT_16  16
-#define MLXBF_I2C_SHIFT_24  24
-
-#define MLXBF_I2C_MASK_8    GENMASK(7, 0)
-#define MLXBF_I2C_MASK_16   GENMASK(15, 0)
-
 /*
  * Function to poll a set of bits at a specific address; it checks whether
  * the bits are equal to zero when eq_zero is set to 'true', and not equal
  * to zero when eq_zero is set to 'false'.
  * Note that the timeout is given in microseconds.
  */
-static u32 mlxbf_smbus_poll(void __iomem *io, u32 addr, u32 mask,
+static u32 mlxbf_i2c_poll(void __iomem *io, u32 addr, u32 mask,
 			    bool eq_zero, u32  timeout)
 {
 	u32 bits;
@@ -500,13 +523,13 @@ static u32 mlxbf_smbus_poll(void __iomem *io, u32 addr, u32 mask,
  * a transaction. Accordingly, this function polls the Master FSM stop
  * bit; it returns false when the bit is asserted, true if not.
  */
-static bool mlxbf_smbus_master_wait_for_idle(struct mlxbf_i2c_priv *priv)
+static bool mlxbf_i2c_smbus_master_wait_for_idle(struct mlxbf_i2c_priv *priv)
 {
 	u32 mask = MLXBF_I2C_SMBUS_MASTER_FSM_STOP_MASK;
-	u32 addr = MLXBF_I2C_SMBUS_MASTER_FSM;
+	u32 addr = priv->chip->smbus_master_fsm_off;
 	u32 timeout = MLXBF_I2C_SMBUS_TIMEOUT;
 
-	if (mlxbf_smbus_poll(priv->smbus->io, addr, mask, true, timeout))
+	if (mlxbf_i2c_poll(priv->mst->io, addr, mask, true, timeout))
 		return true;
 
 	return false;
@@ -517,7 +540,7 @@ static bool mlxbf_smbus_master_wait_for_idle(struct mlxbf_i2c_priv *priv)
  */
 static bool mlxbf_i2c_smbus_master_lock(struct mlxbf_i2c_priv *priv)
 {
-	if (mlxbf_smbus_poll(priv->smbus->io, MLXBF_I2C_SMBUS_MASTER_GW,
+	if (mlxbf_i2c_poll(priv->mst->io, MLXBF_I2C_SMBUS_MASTER_GW,
 			   MLXBF_I2C_MASTER_LOCK_BIT, true,
 			   MLXBF_I2C_SMBUS_LOCK_POLL_TIMEOUT))
 		return true;
@@ -528,7 +551,7 @@ static bool mlxbf_i2c_smbus_master_lock(struct mlxbf_i2c_priv *priv)
 static void mlxbf_i2c_smbus_master_unlock(struct mlxbf_i2c_priv *priv)
 {
 	/* Clear the gw to clear the lock */
-	writel(0, priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_GW);
+	writel(0, priv->mst->io + MLXBF_I2C_SMBUS_MASTER_GW);
 }
 
 static bool mlxbf_i2c_smbus_transaction_success(u32 master_status,
@@ -568,7 +591,7 @@ static int mlxbf_i2c_smbus_check_status(struct mlxbf_i2c_priv *priv)
 	 * then read the cause and master status bits to determine if
 	 * errors occurred during the transaction.
 	 */
-	mlxbf_smbus_poll(priv->smbus->io, MLXBF_I2C_SMBUS_MASTER_GW,
+	mlxbf_i2c_poll(priv->mst->io, MLXBF_I2C_SMBUS_MASTER_GW,
 			 MLXBF_I2C_MASTER_BUSY_BIT, true,
 			 MLXBF_I2C_SMBUS_TIMEOUT);
 
@@ -581,7 +604,7 @@ static int mlxbf_i2c_smbus_check_status(struct mlxbf_i2c_priv *priv)
 	 * Parse both Cause and Master GW bits, then return transaction status.
 	 */
 
-	master_status_bits = readl(priv->smbus->io +
+	master_status_bits = readl(priv->mst->io +
 					MLXBF_I2C_SMBUS_MASTER_STATUS);
 	master_status_bits &= MLXBF_I2C_SMBUS_MASTER_STATUS_MASK;
 
@@ -606,7 +629,8 @@ static int mlxbf_i2c_smbus_check_status(struct mlxbf_i2c_priv *priv)
 }
 
 static void mlxbf_i2c_smbus_write_data(struct mlxbf_i2c_priv *priv,
-				       const u8 *data, u8 length, u32 addr)
+				       const u8 *data, u8 length, u32 addr,
+				       bool is_master)
 {
 	u8 offset, aligned_length;
 	u32 data32;
@@ -623,12 +647,16 @@ static void mlxbf_i2c_smbus_write_data(struct mlxbf_i2c_priv *priv,
 	 */
 	for (offset = 0; offset < aligned_length; offset += sizeof(u32)) {
 		data32 = *((u32 *)(data + offset));
-		iowrite32be(data32, priv->smbus->io + addr + offset);
+		if (is_master)
+			iowrite32be(data32, priv->mst->io + addr + offset);
+		else
+			iowrite32be(data32, priv->slv->io + addr + offset);
 	}
 }
 
 static void mlxbf_i2c_smbus_read_data(struct mlxbf_i2c_priv *priv,
-				      u8 *data, u8 length, u32 addr)
+				      u8 *data, u8 length, u32 addr,
+				      bool is_master)
 {
 	u32 data32, mask;
 	u8 byte, offset;
@@ -644,14 +672,20 @@ static void mlxbf_i2c_smbus_read_data(struct mlxbf_i2c_priv *priv,
 	 */
 
 	for (offset = 0; offset < (length & ~mask); offset += sizeof(u32)) {
-		data32 = ioread32be(priv->smbus->io + addr + offset);
+		if (is_master)
+			data32 = ioread32be(priv->mst->io + addr + offset);
+		else
+			data32 = ioread32be(priv->slv->io + addr + offset);
 		*((u32 *)(data + offset)) = data32;
 	}
 
 	if (!(length & mask))
 		return;
 
-	data32 = ioread32be(priv->smbus->io + addr + offset);
+	if (is_master)
+		data32 = ioread32be(priv->mst->io + addr + offset);
+	else
+		data32 = ioread32be(priv->slv->io + addr + offset);
 
 	for (byte = 0; byte < (length & mask); byte++) {
 		data[offset + byte] = data32 & GENMASK(7, 0);
@@ -677,16 +711,16 @@ static int mlxbf_i2c_smbus_enable(struct mlxbf_i2c_priv *priv, u8 slave,
 	command |= rol32(pec_en, MLXBF_I2C_MASTER_SEND_PEC_SHIFT);
 
 	/* Clear status bits. */
-	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_STATUS);
+	writel(0x0, priv->mst->io + MLXBF_I2C_SMBUS_MASTER_STATUS);
 	/* Set the cause data. */
 	writel(~0x0, priv->mst_cause->io + MLXBF_I2C_CAUSE_OR_CLEAR);
 	/* Zero PEC byte. */
-	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_PEC);
+	writel(0x0, priv->mst->io + MLXBF_I2C_SMBUS_MASTER_PEC);
 	/* Zero byte count. */
-	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_RS_BYTES);
+	writel(0x0, priv->mst->io + priv->chip->smbus_master_rs_bytes_off);
 
 	/* GW activation. */
-	writel(command, priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_GW);
+	writel(command, priv->mst->io + MLXBF_I2C_SMBUS_MASTER_GW);
 
 	/*
 	 * Poll master status and check status bits. An ACK is sent when
@@ -730,7 +764,7 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
 		return -EBUSY;
 
 	/* Check whether the HW is idle */
-	if (WARN_ON(!mlxbf_smbus_master_wait_for_idle(priv))) {
+	if (WARN_ON(!mlxbf_i2c_smbus_master_wait_for_idle(priv))) {
 		ret = -EBUSY;
 		goto out_unlock;
 	}
@@ -787,7 +821,7 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
 	 * must be written to the data registers.
 	 */
 	mlxbf_i2c_smbus_write_data(priv, (const u8 *)data_desc, data_len,
-				   MLXBF_I2C_MASTER_DATA_DESC_ADDR);
+				   MLXBF_I2C_MASTER_DATA_DESC_ADDR, true);
 
 	if (write_en) {
 		ret = mlxbf_i2c_smbus_enable(priv, slave, write_len, block_en,
@@ -799,13 +833,13 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
 	if (read_en) {
 		/* Write slave address to Master GW data descriptor. */
 		mlxbf_i2c_smbus_write_data(priv, (const u8 *)&addr, 1,
-					   MLXBF_I2C_MASTER_DATA_DESC_ADDR);
+					   MLXBF_I2C_MASTER_DATA_DESC_ADDR, true);
 		ret = mlxbf_i2c_smbus_enable(priv, slave, read_len, block_en,
 					 pec_en, 1);
 		if (!ret) {
 			/* Get Master GW data descriptor. */
 			mlxbf_i2c_smbus_read_data(priv, data_desc, read_len + 1,
-					     MLXBF_I2C_MASTER_DATA_DESC_ADDR);
+					     MLXBF_I2C_MASTER_DATA_DESC_ADDR, true);
 
 			/* Get data from Master GW data descriptor. */
 			memcpy(read_buf, data_desc, read_len + 1);
@@ -817,7 +851,7 @@ mlxbf_i2c_smbus_start_transaction(struct mlxbf_i2c_priv *priv,
 		 * next tag integration.
 		 */
 		writel(MLXBF_I2C_SMBUS_MASTER_FSM_PS_STATE_MASK,
-			priv->smbus->io + MLXBF_I2C_SMBUS_MASTER_FSM);
+			priv->mst->io + priv->chip->smbus_master_fsm_off);
 	}
 
 out_unlock:
@@ -1109,7 +1143,7 @@ static void mlxbf_i2c_set_timings(struct mlxbf_i2c_priv *priv,
 	timer |= mlxbf_i2c_set_timer(priv, timings->scl_low,
 				     false, MLXBF_I2C_MASK_16,
 				     MLXBF_I2C_SHIFT_16);
-	writel(timer, priv->smbus->io +
+	writel(timer, priv->timer->io +
 		MLXBF_I2C_SMBUS_TIMER_SCL_LOW_SCL_HIGH);
 
 	timer = mlxbf_i2c_set_timer(priv, timings->sda_rise, false,
@@ -1120,34 +1154,34 @@ static void mlxbf_i2c_set_timings(struct mlxbf_i2c_priv *priv,
 				     MLXBF_I2C_MASK_8, MLXBF_I2C_SHIFT_16);
 	timer |= mlxbf_i2c_set_timer(priv, timings->scl_fall, false,
 				     MLXBF_I2C_MASK_8, MLXBF_I2C_SHIFT_24);
-	writel(timer, priv->smbus->io +
+	writel(timer, priv->timer->io +
 		MLXBF_I2C_SMBUS_TIMER_FALL_RISE_SPIKE);
 
 	timer = mlxbf_i2c_set_timer(priv, timings->hold_start, true,
 				    MLXBF_I2C_MASK_16, MLXBF_I2C_SHIFT_0);
 	timer |= mlxbf_i2c_set_timer(priv, timings->hold_data, true,
 				     MLXBF_I2C_MASK_16, MLXBF_I2C_SHIFT_16);
-	writel(timer, priv->smbus->io + MLXBF_I2C_SMBUS_TIMER_THOLD);
+	writel(timer, priv->timer->io + MLXBF_I2C_SMBUS_TIMER_THOLD);
 
 	timer = mlxbf_i2c_set_timer(priv, timings->setup_start, true,
 				    MLXBF_I2C_MASK_16, MLXBF_I2C_SHIFT_0);
 	timer |= mlxbf_i2c_set_timer(priv, timings->setup_stop, true,
 				     MLXBF_I2C_MASK_16, MLXBF_I2C_SHIFT_16);
-	writel(timer, priv->smbus->io +
+	writel(timer, priv->timer->io +
 		MLXBF_I2C_SMBUS_TIMER_TSETUP_START_STOP);
 
 	timer = mlxbf_i2c_set_timer(priv, timings->setup_data, true,
 				    MLXBF_I2C_MASK_16, MLXBF_I2C_SHIFT_0);
-	writel(timer, priv->smbus->io + MLXBF_I2C_SMBUS_TIMER_TSETUP_DATA);
+	writel(timer, priv->timer->io + MLXBF_I2C_SMBUS_TIMER_TSETUP_DATA);
 
 	timer = mlxbf_i2c_set_timer(priv, timings->buf, false,
 				    MLXBF_I2C_MASK_16, MLXBF_I2C_SHIFT_0);
 	timer |= mlxbf_i2c_set_timer(priv, timings->thigh_max, false,
 				     MLXBF_I2C_MASK_16, MLXBF_I2C_SHIFT_16);
-	writel(timer, priv->smbus->io + MLXBF_I2C_SMBUS_THIGH_MAX_TBUF);
+	writel(timer, priv->timer->io + MLXBF_I2C_SMBUS_THIGH_MAX_TBUF);
 
 	timer = timings->timeout;
-	writel(timer, priv->smbus->io + MLXBF_I2C_SMBUS_SCL_LOW_TIMEOUT);
+	writel(timer, priv->timer->io + MLXBF_I2C_SMBUS_SCL_LOW_TIMEOUT);
 }
 
 enum mlxbf_i2c_timings_config {
@@ -1559,7 +1593,7 @@ static int mlxbf_i2c_slave_enable(struct mlxbf_i2c_priv *priv,
 	 * Look for the next available slave register slot.
 	 */
 	for (reg = 0; reg < reg_cnt; reg++) {
-		slave_reg = readl(priv->smbus->io +
+		slave_reg = readl(priv->slv->io +
 				MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG + reg * 0x4);
 		/*
 		 * Each register holds 4 slave addresses. So, we have to keep
@@ -1581,7 +1615,7 @@ static int mlxbf_i2c_slave_enable(struct mlxbf_i2c_priv *priv,
 				slave_reg &= ~(MLXBF_I2C_SMBUS_SLAVE_ADDR_MASK << (byte * 8));
 				slave_reg |= (slave->addr << (byte * 8));
 				slave_reg |= MLXBF_I2C_SMBUS_SLAVE_ADDR_EN_BIT << (byte * 8);
-				writel(slave_reg, priv->smbus->io +
+				writel(slave_reg, priv->slv->io +
 					MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG +
 					(reg * 0x4));
 
@@ -1616,7 +1650,7 @@ static int mlxbf_i2c_slave_disable(struct mlxbf_i2c_priv *priv, u8 addr)
 	 * Check if addr is present in the registers.
 	 */
 	for (reg = 0; reg < reg_cnt; reg++) {
-		slave_reg = readl(priv->smbus->io +
+		slave_reg = readl(priv->slv->io +
 				MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG + reg * 0x4);
 
 		/* Check whether the address slots are empty. */
@@ -1637,7 +1671,7 @@ static int mlxbf_i2c_slave_disable(struct mlxbf_i2c_priv *priv, u8 addr)
 			if (addr_tmp == addr) {
 				/* Clear the slave address slot. */
 				slave_reg &= ~(GENMASK(7, 0) << (byte * 8));
-				writel(slave_reg, priv->smbus->io +
+				writel(slave_reg, priv->slv->io +
 					MLXBF_I2C_SMBUS_SLAVE_ADDR_CFG +
 					(reg * 0x4));
 				/* Free slave at the corresponding index */
@@ -1741,7 +1775,7 @@ static int mlxbf_i2c_init_slave(struct platform_device *pdev,
 	int ret;
 
 	/* Reset FSM. */
-	writel(0, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_FSM);
+	writel(0, priv->slv->io + MLXBF_I2C_SMBUS_SLAVE_FSM);
 
 	/*
 	 * Enable slave cause interrupt bits. Drive
@@ -1756,7 +1790,7 @@ static int mlxbf_i2c_init_slave(struct platform_device *pdev,
 	writel(int_reg, priv->slv_cause->io + MLXBF_I2C_CAUSE_OR_EVTEN0);
 
 	/* Finally, set the 'ready' bit to start handling transactions. */
-	writel(0x1, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_READY);
+	writel(0x1, priv->slv->io + MLXBF_I2C_SMBUS_SLAVE_READY);
 
 	/* Initialize the cause coalesce resource. */
 	ret = mlxbf_i2c_init_coalesce(pdev, priv);
@@ -1801,13 +1835,13 @@ static bool mlxbf_i2c_has_coalesce(struct mlxbf_i2c_priv *priv, bool *read,
 	return true;
 }
 
-static bool mlxbf_smbus_slave_wait_for_idle(struct mlxbf_i2c_priv *priv,
+static bool mlxbf_i2c_slave_wait_for_idle(struct mlxbf_i2c_priv *priv,
 					    u32 timeout)
 {
 	u32 mask = MLXBF_I2C_CAUSE_S_GW_BUSY_FALL;
 	u32 addr = MLXBF_I2C_CAUSE_ARBITER;
 
-	if (mlxbf_smbus_poll(priv->slv_cause->io, addr, mask, false, timeout))
+	if (mlxbf_i2c_poll(priv->slv_cause->io, addr, mask, false, timeout))
 		return true;
 
 	return false;
@@ -1846,7 +1880,7 @@ static int mlxbf_i2c_irq_send(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
 	 * determine the slave address. This byte is located in the
 	 * first data descriptor register of the slave GW.
 	 */
-	data32 = ioread32be(priv->smbus->io +
+	data32 = ioread32be(priv->slv->io +
 				MLXBF_I2C_SLAVE_DATA_DESC_ADDR);
 	addr = (data32 & GENMASK(7, 0)) >> 1;
 
@@ -1900,7 +1934,7 @@ static int mlxbf_i2c_irq_send(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
 
 	/* Write data to Slave GW data descriptor. */
 	mlxbf_i2c_smbus_write_data(priv, data_desc, byte_cnt,
-				   MLXBF_I2C_SLAVE_DATA_DESC_ADDR);
+				   MLXBF_I2C_SLAVE_DATA_DESC_ADDR, false);
 
 	pec_en = 0; /* Disable PEC since it is not supported. */
 
@@ -1909,19 +1943,19 @@ static int mlxbf_i2c_irq_send(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
 	control32 |= rol32(write_size, MLXBF_I2C_SLAVE_WRITE_BYTES_SHIFT);
 	control32 |= rol32(pec_en, MLXBF_I2C_SLAVE_SEND_PEC_SHIFT);
 
-	writel(control32, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_GW);
+	writel(control32, priv->slv->io + MLXBF_I2C_SMBUS_SLAVE_GW);
 
 	/*
 	 * Wait until the transfer is completed; the driver will wait
 	 * until the GW is idle, a cause will rise on fall of GW busy.
 	 */
-	mlxbf_smbus_slave_wait_for_idle(priv, MLXBF_I2C_SMBUS_TIMEOUT);
+	mlxbf_i2c_slave_wait_for_idle(priv, MLXBF_I2C_SMBUS_TIMEOUT);
 
 clear_csr:
 	/* Release the Slave GW. */
-	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_RS_MASTER_BYTES);
-	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_PEC);
-	writel(0x1, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_READY);
+	writel(0x0, priv->slv->io + MLXBF_I2C_SMBUS_SLAVE_RS_MASTER_BYTES);
+	writel(0x0, priv->slv->io + MLXBF_I2C_SMBUS_SLAVE_PEC);
+	writel(0x1, priv->slv->io + MLXBF_I2C_SMBUS_SLAVE_READY);
 
 	return ret;
 }
@@ -1939,7 +1973,7 @@ static int mlxbf_i2c_irq_recv(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
 
 	/* Read data from Slave GW data descriptor. */
 	mlxbf_i2c_smbus_read_data(priv, data_desc, recv_bytes,
-				  MLXBF_I2C_SLAVE_DATA_DESC_ADDR);
+				  MLXBF_I2C_SLAVE_DATA_DESC_ADDR, false);
 	addr = data_desc[0] >> 1;
 
 	/*
@@ -1975,9 +2009,9 @@ static int mlxbf_i2c_irq_recv(struct mlxbf_i2c_priv *priv, u8 recv_bytes)
 
 clear_csr:
 	/* Release the Slave GW. */
-	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_RS_MASTER_BYTES);
-	writel(0x0, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_PEC);
-	writel(0x1, priv->smbus->io + MLXBF_I2C_SMBUS_SLAVE_READY);
+	writel(0x0, priv->slv->io + MLXBF_I2C_SMBUS_SLAVE_RS_MASTER_BYTES);
+	writel(0x0, priv->slv->io + MLXBF_I2C_SMBUS_SLAVE_PEC);
+	writel(0x1, priv->slv->io + MLXBF_I2C_SMBUS_SLAVE_READY);
 
 	return ret;
 }
@@ -2012,7 +2046,7 @@ static irqreturn_t mlxbf_i2c_irq(int irq, void *ptr)
 	 * slave, if the higher 8 bits are sent then the slave expect N bytes
 	 * from the master.
 	 */
-	rw_bytes_reg = readl(priv->smbus->io +
+	rw_bytes_reg = readl(priv->slv->io +
 				MLXBF_I2C_SMBUS_SLAVE_RS_MASTER_BYTES);
 	recv_bytes = (rw_bytes_reg >> 8) & GENMASK(7, 0);
 
@@ -2177,14 +2211,27 @@ static struct mlxbf_i2c_chip_info mlxbf_i2c_chip[] = {
 			[1] = &mlxbf_i2c_corepll_res[MLXBF_I2C_CHIP_TYPE_1],
 			[2] = &mlxbf_i2c_gpio_res[MLXBF_I2C_CHIP_TYPE_1]
 		},
-		.calculate_freq = mlxbf_i2c_calculate_freq_from_tyu
+		.calculate_freq = mlxbf_i2c_calculate_freq_from_tyu,
+		.smbus_master_rs_bytes_off = MLXBF_I2C_YU_SMBUS_RS_BYTES,
+		.smbus_master_fsm_off = MLXBF_I2C_YU_SMBUS_MASTER_FSM
 	},
 	[MLXBF_I2C_CHIP_TYPE_2] = {
 		.type = MLXBF_I2C_CHIP_TYPE_2,
 		.shared_res = {
 			[0] = &mlxbf_i2c_corepll_res[MLXBF_I2C_CHIP_TYPE_2]
 		},
-		.calculate_freq = mlxbf_i2c_calculate_freq_from_yu
+		.calculate_freq = mlxbf_i2c_calculate_freq_from_yu,
+		.smbus_master_rs_bytes_off = MLXBF_I2C_YU_SMBUS_RS_BYTES,
+		.smbus_master_fsm_off = MLXBF_I2C_YU_SMBUS_MASTER_FSM
+	},
+	[MLXBF_I2C_CHIP_TYPE_3] = {
+		.type = MLXBF_I2C_CHIP_TYPE_3,
+		.shared_res = {
+			[0] = &mlxbf_i2c_corepll_res[MLXBF_I2C_CHIP_TYPE_3]
+		},
+		.calculate_freq = mlxbf_i2c_calculate_freq_from_yu,
+		.smbus_master_rs_bytes_off = MLXBF_I2C_RSH_YU_SMBUS_RS_BYTES,
+		.smbus_master_fsm_off = MLXBF_I2C_RSH_YU_SMBUS_MASTER_FSM
 	}
 };
 
@@ -2209,6 +2256,10 @@ static const struct of_device_id mlxbf_i2c_dt_ids[] = {
 		.compatible = "mellanox,i2c-mlxbf2",
 		.data = &mlxbf_i2c_chip[MLXBF_I2C_CHIP_TYPE_2]
 	},
+	{
+		.compatible = "mellanox,i2c-mlxbf3",
+		.data = &mlxbf_i2c_chip[MLXBF_I2C_CHIP_TYPE_3]
+	},
 	{},
 };
 
@@ -2218,6 +2269,7 @@ MODULE_DEVICE_TABLE(of, mlxbf_i2c_dt_ids);
 static const struct acpi_device_id mlxbf_i2c_acpi_ids[] = {
 	{ "MLNXBF03", (kernel_ulong_t)&mlxbf_i2c_chip[MLXBF_I2C_CHIP_TYPE_1] },
 	{ "MLNXBF23", (kernel_ulong_t)&mlxbf_i2c_chip[MLXBF_I2C_CHIP_TYPE_2] },
+	{ "MLNXBF31", (kernel_ulong_t)&mlxbf_i2c_chip[MLXBF_I2C_CHIP_TYPE_3] },
 	{},
 };
 
@@ -2293,6 +2345,7 @@ static int mlxbf_i2c_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct mlxbf_i2c_priv *priv;
 	struct i2c_adapter *adap;
+	u32 resource_version;
 	int irq, ret;
 
 	priv = devm_kzalloc(dev, sizeof(struct mlxbf_i2c_priv), GFP_KERNEL);
@@ -2306,11 +2359,55 @@ static int mlxbf_i2c_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	ret = mlxbf_i2c_init_resource(pdev, &priv->smbus,
-				      MLXBF_I2C_SMBUS_RES);
-	if (ret < 0) {
-		dev_err(dev, "Cannot fetch smbus resource info");
-		return ret;
+	/* This property allows the driver to stay backward compatible with older
+	 * ACPI table and device trees versions.
+	 * Starting BlueField-3 SoC, the "smbus" resource was broken down into 3
+	 * separate resources "timer", "master" and "slave".
+	 */
+	if (device_property_read_u32(dev, "resource_version", &resource_version))
+		resource_version = 0;
+
+	priv->resource_version = resource_version;
+
+	if (priv->chip->type < MLXBF_I2C_CHIP_TYPE_3 && resource_version == 0) {
+		priv->timer = devm_kzalloc(dev, sizeof(struct mlxbf_i2c_resource), GFP_KERNEL);
+		if (!priv->timer)
+			return -ENOMEM;
+
+		priv->mst = devm_kzalloc(dev, sizeof(struct mlxbf_i2c_resource), GFP_KERNEL);
+		if (!priv->mst)
+			return -ENOMEM;
+
+		priv->slv = devm_kzalloc(dev, sizeof(struct mlxbf_i2c_resource), GFP_KERNEL);
+		if (!priv->slv)
+			return -ENOMEM;
+
+		ret = mlxbf_i2c_init_resource(pdev, &priv->smbus,
+					      MLXBF_I2C_SMBUS_RES);
+		priv->timer->io = priv->smbus->io;
+		priv->mst->io = priv->smbus->io + MLXBF_I2C_MST_ADDR_OFFSET;
+		priv->slv->io = priv->smbus->io + MLXBF_I2C_SLV_ADDR_OFFSET;
+	} else {
+		ret = mlxbf_i2c_init_resource(pdev, &priv->timer,
+					      MLXBF_I2C_SMBUS_TIMER_RES);
+		if (ret < 0) {
+			dev_err(dev, "Cannot fetch timer resource info");
+			return ret;
+		}
+
+		ret = mlxbf_i2c_init_resource(pdev, &priv->mst,
+					      MLXBF_I2C_SMBUS_MST_RES);
+		if (ret < 0) {
+			dev_err(dev, "Cannot fetch master resource info");
+			return ret;
+		}
+
+		ret = mlxbf_i2c_init_resource(pdev, &priv->slv,
+					      MLXBF_I2C_SMBUS_SLV_RES);
+		if (ret < 0) {
+			dev_err(dev, "Cannot fetch slave resource info");
+			return ret;
+		}
 	}
 
 	ret = mlxbf_i2c_init_resource(pdev, &priv->mst_cause,
@@ -2398,8 +2495,19 @@ static int mlxbf_i2c_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource *params;
 
-	params = priv->smbus->params;
-	devm_release_mem_region(dev, params->start, resource_size(params));
+	if (priv->chip->type < MLXBF_I2C_CHIP_TYPE_3 && priv->resource_version == 0) {
+		params = priv->smbus->params;
+		devm_release_mem_region(dev, params->start, resource_size(params));
+	} else {
+		params = priv->timer->params;
+		devm_release_mem_region(dev, params->start, resource_size(params));
+
+		params = priv->mst->params;
+		devm_release_mem_region(dev, params->start, resource_size(params));
+
+		params = priv->slv->params;
+		devm_release_mem_region(dev, params->start, resource_size(params));
+	}
 
 	params = priv->mst_cause->params;
 	devm_release_mem_region(dev, params->start, resource_size(params));
-- 
2.30.1

[PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree

[Asmaa Mnebhi]

In the latest version of the i2c-mlxbf.c driver, the "Smbus block"
resource was broken down to 3 separate resources "Smbus timer",
"Smbus master" and "Smbus slave" to accommodate for BlueField-3
SoC registers' changes.

Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
---
 .../bindings/i2c/mellanox,i2c-mlxbf.yaml      | 48 ++++++++++++++-----
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.yaml b/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.yaml
index 93198d5d43a6..24ab70c987fe 100644
--- a/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.yaml
+++ b/Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.yaml
@@ -8,6 +8,7 @@ title: Mellanox I2C SMBus on BlueField SoCs
 
 maintainers:
   - Khalil Blaiech <kblaiech@nvidia.com>
+  - Asmaa Mnebhi <asmaa@nvidia.com>
 
 allOf:
   - $ref: /schemas/i2c/i2c-controller.yaml#
@@ -17,6 +18,7 @@ properties:
     enum:
       - mellanox,i2c-mlxbf1
       - mellanox,i2c-mlxbf2
+      - mellanox,i2c-mlxbf3
 
   reg:
     minItems: 3
@@ -25,6 +27,9 @@ properties:
       - description: Cause master registers
       - description: Cause slave registers
       - description: Cause coalesce registers
+      - description: Smbus timer registers
+      - description: Smbus master registers
+      - description: Smbus slave registers
 
   interrupts:
     maxItems: 1
@@ -35,6 +40,13 @@ properties:
       bus frequency used to configure timing registers;
       The frequency is expressed in Hz. Default is 100000.
 
+  resource_version:
+    enum: [ 0, 1 ]
+    description:
+      Version of the device tree. resource_version = 0 when the driver uses
+      Smbus block resource. resource_version = 1 when the driver uses Smbus
+      timer, Smbus master and Smbus slave resources.
+
 required:
   - compatible
   - reg
@@ -42,18 +54,6 @@ required:
 
 unevaluatedProperties: false
 
-if:
-  properties:
-    compatible:
-      contains:
-        enum:
-          - mellanox,i2c-mlxbf1
-
-then:
-  properties:
-    reg:
-      maxItems: 3
-
 examples:
   - |
     i2c@2804000 {
@@ -61,8 +61,13 @@ examples:
         reg = <0x02804000 0x800>,
               <0x02801200 0x020>,
               <0x02801260 0x020>;
+              <0x00000001 0x1>;
+              <0x02804000 0x40>,
+              <0x02804200 0x200>,
+              <0x02804400 0x200>,
         interrupts = <57>;
         clock-frequency = <100000>;
+        resource_version = <1>;
     };
 
   - |
@@ -72,6 +77,25 @@ examples:
               <0x02808e00 0x020>,
               <0x02808e20 0x020>,
               <0x02808e40 0x010>;
+              <0x02808800 0x040>;
+              <0x02808a00 0x200>,
+              <0x02808c00 0x200>,
         interrupts = <57>;
         clock-frequency = <400000>;
+        resource_version = <1>;
+    };
+
+  - |
+    i2c@2808800 {
+        compatible = "mellanox,i2c-mlxbf3";
+        reg = <0x00000001 0x1>,
+              <0x13404400 0x020>,
+              <0x13404420 0x020>,
+              <0x13404440 0x010>;
+              <0x13404480 0x40>,
+              <0x13404200 0x200>,
+              <0x13404000 0x200>,
+        interrupts = <35>;
+        clock-frequency = <400000>;
+        resource_version = <1>;
     };
-- 
2.30.1

Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree

[Krzysztof Kozlowski]

On Tue, 20 Sep 2022 13:47:36 -0400, Asmaa Mnebhi wrote:
> In the latest version of the i2c-mlxbf.c driver, the "Smbus block"
> resource was broken down to 3 separate resources "Smbus timer",
> "Smbus master" and "Smbus slave" to accommodate for BlueField-3
> SoC registers' changes.
> 
> Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> ---
>  .../bindings/i2c/mellanox,i2c-mlxbf.yaml      | 48 ++++++++++++++-----
>  1 file changed, 36 insertions(+), 12 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.example.dts:26.19-20 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:384: Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1420: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

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

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree

[Krzysztof Kozlowski]

On 20/09/2022 19:47, Asmaa Mnebhi wrote:
> In the latest version of the i2c-mlxbf.c driver, the "Smbus block"
> resource was broken down to 3 separate resources "Smbus timer",
> "Smbus master" and "Smbus slave" to accommodate for BlueField-3
> SoC registers' changes.
> 
> Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>

Use scripts/get_maintainers.pl to CC all maintainers and relevant
mailing lists.

You keep cc-ing other addresses or you rebased your patch on some old
tree (like 1 year old...). If this is the second case, please be sure it
is rebased on LATEST kernel, maintainer's tree or linux-next.

By not-ccing people, you will not get reviews from maintainers.

Best regards,
Krzysztof

Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree

[Krzysztof Kozlowski]

On 20/09/2022 19:47, Asmaa Mnebhi wrote:
> In the latest version of the i2c-mlxbf.c driver, the "Smbus block"
> resource was broken down to 3 separate resources "Smbus timer",
> "Smbus master" and "Smbus slave" to accommodate for BlueField-3
> SoC registers' changes.
> 
> Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> ---


>    reg:
>      minItems: 3
> @@ -25,6 +27,9 @@ properties:
>        - description: Cause master registers
>        - description: Cause slave registers
>        - description: Cause coalesce registers
> +      - description: Smbus timer registers
> +      - description: Smbus master registers
> +      - description: Smbus slave registers
>  
>    interrupts:
>      maxItems: 1
> @@ -35,6 +40,13 @@ properties:
>        bus frequency used to configure timing registers;
>        The frequency is expressed in Hz. Default is 100000.
>  
> +  resource_version:

No underscores in names.

> +    enum: [ 0, 1 ]
> +    description:
> +      Version of the device tree. resource_version = 0 when the driver uses
> +      Smbus block resource. resource_version = 1 when the driver uses Smbus
> +      timer, Smbus master and Smbus slave resources.

No way. That's not a DT property.

> +
>  required:
>    - compatible
>    - reg
> @@ -42,18 +54,6 @@ required:
>  
>  unevaluatedProperties: false
>  
> -if:
> -  properties:
> -    compatible:
> -      contains:
> -        enum:
> -          - mellanox,i2c-mlxbf1
> -
> -then:
> -  properties:
> -    reg:
> -      maxItems: 3

Why?

> -
>  examples:
>    - |
>      i2c@2804000 {
> @@ -61,8 +61,13 @@ examples:
>          reg = <0x02804000 0x800>,
>                <0x02801200 0x020>,
>                <0x02801260 0x020>;
> +              <0x00000001 0x1>;
> +              <0x02804000 0x40>,
> +              <0x02804200 0x200>,
> +              <0x02804400 0x200>,
>          interrupts = <57>;
>          clock-frequency = <100000>;
> +        resource_version = <1>;
>      };
>  
>    - |
> @@ -72,6 +77,25 @@ examples:
>                <0x02808e00 0x020>,
>                <0x02808e20 0x020>,
>                <0x02808e40 0x010>;
> +              <0x02808800 0x040>;
> +              <0x02808a00 0x200>,
> +              <0x02808c00 0x200>,
>          interrupts = <57>;
>          clock-frequency = <400000>;
> +        resource_version = <1>;
> +    };
> +
> +  - |
> +    i2c@2808800 {
> +        compatible = "mellanox,i2c-mlxbf3";
> +        reg = <0x00000001 0x1>,
> +              <0x13404400 0x020>,
> +              <0x13404420 0x020>,
> +              <0x13404440 0x010>;
> +              <0x13404480 0x40>,
> +              <0x13404200 0x200>,
> +              <0x13404000 0x200>,

No need for the same example.

Best regards,
Krzysztof

RE: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree

[Asmaa Mnebhi]

I have a question for you and Wolfram, we don’t use device trees and are not planning to use device trees; we only use ACPI tables. But I think when Khalil submitted the first version of the i2c-mlxbf.c driver, it was requested from him to add devicetree support. Do you know why? Is it possible to remove the device tree support and so this doc? or is devicetree support a requirement regardless of the actual implementation? 

-----Original Message-----
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 
Sent: Wednesday, September 21, 2022 2:55 AM
To: Asmaa Mnebhi <asmaa@nvidia.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>; linux-kernel@vger.kernel.org; robh@kernel.org; devicetree@vger.kernel.org; linux-i2c@vger.kernel.org; Khalil Blaiech <kblaiech@nvidia.com>
Subject: Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree

On Tue, 20 Sep 2022 13:47:36 -0400, Asmaa Mnebhi wrote:
> In the latest version of the i2c-mlxbf.c driver, the "Smbus block"
> resource was broken down to 3 separate resources "Smbus timer", "Smbus 
> master" and "Smbus slave" to accommodate for BlueField-3 SoC 
> registers' changes.
> 
> Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>
> ---
>  .../bindings/i2c/mellanox,i2c-mlxbf.yaml      | 48 ++++++++++++++-----
>  1 file changed, 36 insertions(+), 12 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.example.dts:26.19-20 syntax error FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:384: Documentation/devicetree/bindings/i2c/mellanox,i2c-mlxbf.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1420: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

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

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree

[Krzysztof Kozlowski]

On 21/09/2022 15:12, Asmaa Mnebhi wrote:
> I have a question for you and Wolfram, we don’t use device trees and are not planning to use device trees; we only use ACPI tables. But I think when Khalil submitted the first version of the i2c-mlxbf.c driver, it was requested from him to add devicetree support. Do you know why? Is it possible to remove the device tree support and so this doc? or is devicetree support a requirement regardless of the actual implementation? 

I don't think I am the right person to answer your question. I don't
know why you do things and did things in your driver. I also I do not
have any interest in your driver supporting anything. However if you do
support DT, I have interest in its correctness.

Best regards,
Krzysztof

RE: [PATCH v5 0/8] i2c: i2c-mlxbf.c: bug fixes and new feature support

[Asmaa Mnebhi]

Hi Wolfram,

Is there any requirement to support DTs in i2c drivers?
I would like to remove it in the v6 series because we don't support device tree, only ACPI.

Thank you,
Asmaa

-----Original Message-----
From: Asmaa Mnebhi <asmaa@nvidia.com> 
Sent: Tuesday, September 20, 2022 1:47 PM
To: Wolfram Sang <wsa+renesas@sang-engineering.com>; robh@kernel.org; linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
Cc: Asmaa Mnebhi <asmaa@nvidia.com>
Subject: [PATCH v5 0/8] i2c: i2c-mlxbf.c: bug fixes and new feature support

This is a series of patches fixing several bugs and implementing new features.

Bug fixes:
1) Fix the frequency calculation
2) Fix incorrect base address passed during io write
3) prevent stack overflow in mlxbf_i2c_smbus_start_transaction()
4) Support lock mechanism to avoid race condition between entities
   using the i2c bus

Cleanup:
5) remove IRQF_ONESHOT flag as it is no longer needed.

Features:
6) Support multi slave functionality
7) Support BlueField-3 SoC
8) Update binding devicetree

What has changed from v4->v5:
Fix build error in the mellanox i2c device tree documentation

Asmaa Mnebhi (8):
  i2c: i2c-mlxbf.c: Fix frequency calculation
  i2c: i2c-mlxbf.c: remove IRQF_ONESHOT
  i2c: i2c-mlxbf.c: incorrect base address passed during io write
  i2c: i2c-mlxbf: prevent stack overflow in
    mlxbf_i2c_smbus_start_transaction()
  i2c: i2c-mlxbf.c: support lock mechanism
  i2c: i2c-mlxbf: add multi slave functionality
  i2c: i2c-mlxbf.c: support BlueField-3 SoC
  i2c: i2c-mlxbf.c: Update binding devicetree

 .../bindings/i2c/mellanox,i2c-mlxbf.yaml      |  48 +-
 MAINTAINERS                                   |   1 +
 drivers/i2c/busses/i2c-mlxbf.c                | 862 ++++++++++--------
 3 files changed, 521 insertions(+), 390 deletions(-)

--
2.30.1

Re: [PATCH v5 1/8] i2c: i2c-mlxbf.c: Fix frequency calculation

[Wolfram Sang]

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

On Tue, Sep 20, 2022 at 01:47:29PM -0400, Asmaa Mnebhi wrote:
> The i2c-mlxbf.c driver is currently broken because there is a bug
> in the calculation of the frequency. core_f, core_r and core_od
> are components read from hardware registers and are used to
> compute the frequency used to compute different timing parameters.
> The shifting mechanism used to get core_f, core_r and core_od is
> wrong. Use FIELD_GET to mask and shift the bitfields properly.
> 
> Fixes: b5b5b32081cd206b (i2c: mlxbf: I2C SMBus driver for Mellanox BlueField SoC)
> Reviewed-by: Khalil Blaiech <kblaiech@nvidia.com>
> Signed-off-by: Asmaa Mnebhi <asmaa@nvidia.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

How to remove DT support from a driver? (was Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree)

[Wolfram Sang]

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

Hi,

> I have a question for you and Wolfram, we don’t use device trees and
> are not planning to use device trees; we only use ACPI tables. But I
> think when Khalil submitted the first version of the i2c-mlxbf.c
> driver, it was requested from him to add devicetree support. Do you
> know why? Is it possible to remove the device tree support and so this
> doc? or is devicetree support a requirement regardless of the actual
> implementation? 

The first version sent from Khalil to the public I2C mailing list already
had DT bindings [1]. I don't see a sign of someone of the public list
requesting DT bindings. Maybe it was company internal?

Technically, there is no requirement to support DT, especially since you
have working ACPI. I don't know the process, though, of removing DT
support. You would basically need to be sure that no user made use of
the DT bindings introduced before. I don't know to what degree you can
assume that.

Maybe the DT list has more to add here?

Happy hacking,

   Wolfram

[1] http://patchwork.ozlabs.org/project/linux-i2c/list/?series=73827&state=*


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: How to remove DT support from a driver? (was Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree)

[Asmaa Mnebhi]

Thanks for your reply Wolfram. All customers using BlueField hardware need to install our internal Firmware (proprietary) before booting any customized OS so they will always use ACPI tables. So I think it is safe to remove it. Any feedback from the DT list would be greatly appreciated! 

-----Original Message-----
From: Wolfram Sang <wsa+renesas@sang-engineering.com> 
Sent: Wednesday, September 21, 2022 4:02 PM
To: Asmaa Mnebhi <asmaa@nvidia.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>; linux-kernel@vger.kernel.org; robh@kernel.org; devicetree@vger.kernel.org; linux-i2c@vger.kernel.org; Khalil Blaiech <kblaiech@nvidia.com>
Subject: How to remove DT support from a driver? (was Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree)

Hi,

> I have a question for you and Wolfram, we don’t use device trees and 
> are not planning to use device trees; we only use ACPI tables. But I 
> think when Khalil submitted the first version of the i2c-mlxbf.c 
> driver, it was requested from him to add devicetree support. Do you 
> know why? Is it possible to remove the device tree support and so this 
> doc? or is devicetree support a requirement regardless of the actual 
> implementation?

The first version sent from Khalil to the public I2C mailing list already had DT bindings [1]. I don't see a sign of someone of the public list requesting DT bindings. Maybe it was company internal?

Technically, there is no requirement to support DT, especially since you have working ACPI. I don't know the process, though, of removing DT support. You would basically need to be sure that no user made use of the DT bindings introduced before. I don't know to what degree you can assume that.

Maybe the DT list has more to add here?

Happy hacking,

   Wolfram

[1] http://patchwork.ozlabs.org/project/linux-i2c/list/?series=73827&state=*

Re: How to remove DT support from a driver? (was Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree)

[Rob Herring]

On Wed, Sep 21, 2022 at 10:01:59PM +0200, Wolfram Sang wrote:
> Hi,
> 
> > I have a question for you and Wolfram, we don’t use device trees and
> > are not planning to use device trees; we only use ACPI tables. But I
> > think when Khalil submitted the first version of the i2c-mlxbf.c
> > driver, it was requested from him to add devicetree support. Do you
> > know why? Is it possible to remove the device tree support and so this
> > doc? or is devicetree support a requirement regardless of the actual
> > implementation? 
> 
> The first version sent from Khalil to the public I2C mailing list already
> had DT bindings [1]. I don't see a sign of someone of the public list
> requesting DT bindings. Maybe it was company internal?
> 
> Technically, there is no requirement to support DT, especially since you
> have working ACPI. I don't know the process, though, of removing DT
> support. You would basically need to be sure that no user made use of
> the DT bindings introduced before. I don't know to what degree you can
> assume that.

There's the whole using DT bindings in ACPI bindings thing, but I have 
little interest (or time) in supporting that. Maybe that's what's 
happening here? I haven't looked. The whole concept is flawed IMO. It 
may work for simple cases of key/value device properties, but the ACPI 
model is quite different in how resources are described and managed.

Rob

RE: How to remove DT support from a driver? (was Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree)

[Asmaa Mnebhi]

There's the whole using DT bindings in ACPI bindings thing, but I have little interest (or time) in supporting that. Maybe that's what's happening here? I haven't looked. The whole concept is flawed IMO. It may work for simple cases of key/value device properties, but the ACPI model is quite different in how resources are described and managed.

Hi Rob,
I chatted with Khalil and he confirmed that the reason he added the device tree support was to follow the example of existing upstreamed i2c drivers (even if we have no support for it and all our customers have to use our firmware including UEFI ACPI tables). So it is unrelated to DT bindings in ACPI bindings. He agrees that it is better to just remove the device tree support (from the driver itself and from the documentation). So if you don't have any objections, I will remove it.

Re: How to remove DT support from a driver? (was Re: [PATCH v5 8/8] i2c: i2c-mlxbf.c: Update binding devicetree)

[Wolfram Sang]

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


> i2c drivers (even if we have no support for it and all our customers
> have to use our firmware including UEFI ACPI tables). So it is

Because of the "custom firmware is needed and it is only ACPI" fact, I
think this is one of the rare cases where we can actually remove DT
support.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]