[RFC PATCH] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

[RFC PATCH] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

[Umesh Kumar]

DesignWare controllers synthesised with the Multiple SAR feature (e.g.
Marvell CN20K) only acknowledge traffic to the primary slave address
(IC_SAR) when IC_ENABLE[19] (SAR_EN) is set. The slave enable path
writes only IC_ENABLE[0], so on such controllers the target never ACKs
its address and a remote master cannot reach it.

Set SAR_EN together with ENABLE when entering slave mode. The bit is
reserved (and ignored) on single-SAR controllers, so existing users are
unaffected.

Signed-off-by: Umesh Kumar <ukumar@marvell.com>
---
RFC note:

On our multi-SAR CN20K the slave only ACKs with IC_ENABLE[19] set.
Existing single-SAR users work without it, and the bit is reserved
there, so I've written it unconditionally. Would you prefer this
gated behind a per-device capability flag instead? The CN20K PCI
device enablement (which would set such a flag) can follow as a
separate series.


 drivers/i2c/busses/i2c-designware-common.c | 9 ++++++++-
 drivers/i2c/busses/i2c-designware-core.h   | 6 ++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4dc57fd56170..b6f3d71cf268 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -374,7 +374,14 @@ static void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
 		regmap_write(dev->map, DW_IC_CON, dev->slave_cfg);
 		regmap_write(dev->map, DW_IC_SAR, dev->slave->addr);
 		regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_SLAVE_MASK);
-		__i2c_dw_enable(dev);
+		/*
+		 * Controllers synthesised with the Multiple SAR feature only
+		 * acknowledge traffic to IC_SAR when SAR_EN is set; a plain
+		 * enable is not enough. The bit is reserved (ignored) on
+		 * single-SAR controllers.
+		 */
+		regmap_write(dev->map, DW_IC_ENABLE,
+			     DW_IC_ENABLE_ENABLE | DW_IC_ENABLE_SAR_EN);
 		break;
 	default:
 		WARN(1, "Invalid mode %d\n", mode);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 9d8d104cc391..35eaf2d7404f 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -125,6 +125,12 @@
 
 #define DW_IC_ENABLE_ENABLE			BIT(0)
 #define DW_IC_ENABLE_ABORT			BIT(1)
+/*
+ * On controllers synthesised with the Multiple SAR feature (e.g. Marvell
+ * CN20K), the primary slave address register (IC_SAR) only acknowledges bus
+ * traffic when this bit is set. It is reserved on single-SAR controllers.
+ */
+#define DW_IC_ENABLE_SAR_EN			BIT(19)
 
 #define DW_IC_STATUS_ACTIVITY			BIT(0)
 #define DW_IC_STATUS_TFE			BIT(2)
-- 
2.43.0

Re: [RFC PATCH] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

[Mika Westerberg]

Hi,

On Tue, Aug 04, 2026 at 09:48:42AM +0530, Umesh Kumar wrote:
> DesignWare controllers synthesised with the Multiple SAR feature (e.g.
> Marvell CN20K) only acknowledge traffic to the primary slave address
> (IC_SAR) when IC_ENABLE[19] (SAR_EN) is set. The slave enable path
> writes only IC_ENABLE[0], so on such controllers the target never ACKs
> its address and a remote master cannot reach it.
> 
> Set SAR_EN together with ENABLE when entering slave mode. The bit is
> reserved (and ignored) on single-SAR controllers, so existing users are
> unaffected.
> 
> Signed-off-by: Umesh Kumar <ukumar@marvell.com>
> ---
> RFC note:
> 
> On our multi-SAR CN20K the slave only ACKs with IC_ENABLE[19] set.
> Existing single-SAR users work without it, and the bit is reserved
> there, so I've written it unconditionally. Would you prefer this
> gated behind a per-device capability flag instead? The CN20K PCI
> device enablement (which would set such a flag) can follow as a
> separate series.

No I think what you do here is fine,

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

[PATCH v2] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

[Umesh Kumar]

DesignWare controllers synthesised with the Multiple SAR feature (e.g.
Marvell CN20K) only acknowledge traffic to the primary slave address
(IC_SAR) when IC_ENABLE[19] (SAR_EN) is set. The slave enable path
writes only IC_ENABLE[0], so on such controllers the target never ACKs
its address and a remote master cannot reach it.

Set SAR_EN together with ENABLE when entering slave mode. The bit is
reserved (and ignored) on single-SAR controllers, so existing users are
unaffected.

Signed-off-by: Umesh Kumar <ukumar@marvell.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Changes in v2:
 - No functional change; dropped the RFC tag.
 - Added Mika's Acked-by.

 drivers/i2c/busses/i2c-designware-common.c | 9 ++++++++-
 drivers/i2c/busses/i2c-designware-core.h   | 6 ++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4dc57fd56170..b6f3d71cf268 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -374,7 +374,14 @@ static void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
 		regmap_write(dev->map, DW_IC_CON, dev->slave_cfg);
 		regmap_write(dev->map, DW_IC_SAR, dev->slave->addr);
 		regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_SLAVE_MASK);
-		__i2c_dw_enable(dev);
+		/*
+		 * Controllers synthesised with the Multiple SAR feature only
+		 * acknowledge traffic to IC_SAR when SAR_EN is set; a plain
+		 * enable is not enough. The bit is reserved (ignored) on
+		 * single-SAR controllers.
+		 */
+		regmap_write(dev->map, DW_IC_ENABLE,
+			     DW_IC_ENABLE_ENABLE | DW_IC_ENABLE_SAR_EN);
 		break;
 	default:
 		WARN(1, "Invalid mode %d\n", mode);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 9d8d104cc391..35eaf2d7404f 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -125,6 +125,12 @@
 
 #define DW_IC_ENABLE_ENABLE			BIT(0)
 #define DW_IC_ENABLE_ABORT			BIT(1)
+/*
+ * On controllers synthesised with the Multiple SAR feature (e.g. Marvell
+ * CN20K), the primary slave address register (IC_SAR) only acknowledges bus
+ * traffic when this bit is set. It is reserved on single-SAR controllers.
+ */
+#define DW_IC_ENABLE_SAR_EN			BIT(19)
 
 #define DW_IC_STATUS_ACTIVITY			BIT(0)
 #define DW_IC_STATUS_TFE			BIT(2)
-- 
2.43.0

Re: [PATCH v2] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

[Andy Shevchenko]

On Wed, Aug 12, 2026 at 09:10:17AM +0530, Umesh Kumar wrote:
> DesignWare controllers synthesised with the Multiple SAR feature (e.g.
> Marvell CN20K) only acknowledge traffic to the primary slave address
> (IC_SAR) when IC_ENABLE[19] (SAR_EN) is set. The slave enable path
> writes only IC_ENABLE[0], so on such controllers the target never ACKs
> its address and a remote master cannot reach it.
> 
> Set SAR_EN together with ENABLE when entering slave mode. The bit is
> reserved (and ignored) on single-SAR controllers, so existing users are
> unaffected.

In the commit message and in the code comment below, can we refer to Synopsys
DataBook for I²C where this bit is described? Or is it Marvell custom bit?

-- 
With Best Regards,
Andy Shevchenko

RE: [EXTERNAL] Re: [PATCH v2] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

[Umesh Kumar]

Hi Andy , 

It's not a Marvell-custom bit — it's the Synopsys DW_apb_i2c IC_ENABLE.SAR_EN field (bit 19), 
part of the Multiple SAR feature described in the DataBook. I'll add the DataBook reference to 
both the commit message and the code comment in v3.

Regards ,
Umesh Kumar

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> 
Sent: Wednesday, August 12, 2026 12:46 PM
To: Umesh Kumar <ukumar@marvell.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>; Andi Shyti <andi.shyti@kernel.org>; Bharat Bhushan <bbhushan2@marvell.com>; linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: [EXTERNAL] Re: [PATCH v2] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

On Wed, Aug 12, 2026 at 09: 10: 17AM +0530, Umesh Kumar wrote: > DesignWare controllers synthesised with the Multiple SAR feature (e. g. > Marvell CN20K) only acknowledge traffic to the primary slave address > (IC_SAR) when IC_ENABLE[19]
ZjQcmQRYFpfptBannerStart
Prioritize security for external emails: 
Confirm sender and content safety before clicking links or opening attachments 
  https://us-phishalarm-ewt.proofpoint.com/EWT/v1/CRVmXkqW!tO3Tvje-Kz8VPqSfGZOxYl-uMGrg7-9lypliHTNEbNmWK_PXdbfXl-N_bBprf9om_uJP2owocFewSpgRPQ$  ‌ 


ZjQcmQRYFpfptBannerEnd
On Wed, Aug 12, 2026 at 09:10:17AM +0530, Umesh Kumar wrote:
> DesignWare controllers synthesised with the Multiple SAR feature (e.g.
> Marvell CN20K) only acknowledge traffic to the primary slave address
> (IC_SAR) when IC_ENABLE[19] (SAR_EN) is set. The slave enable path
> writes only IC_ENABLE[0], so on such controllers the target never ACKs
> its address and a remote master cannot reach it.
> 
> Set SAR_EN together with ENABLE when entering slave mode. The bit is
> reserved (and ignored) on single-SAR controllers, so existing users are
> unaffected.

In the commit message and in the code comment below, can we refer to Synopsys
DataBook for I²C where this bit is described? Or is it Marvell custom bit?

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

[Umesh Kumar]

DesignWare controllers synthesised with the Multiple SAR feature (e.g.
Marvell CN20K) gate the primary slave address behind IC_ENABLE.SAR_EN
(bit 19). Per the Synopsys DW_apb_i2c DataBook (Multiple SAR feature),
when this bit is set the core acknowledges the address matching IC_SAR;
otherwise the address is NACKed. The slave enable path writes only
IC_ENABLE.ENABLE (bit 0), so on such controllers the target never ACKs
its address and a remote master cannot reach it.

Set SAR_EN together with ENABLE when entering slave mode. The bit is
reserved (and ignored) on single-SAR controllers, so existing users are
unaffected.

Signed-off-by: Umesh Kumar <ukumar@marvell.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-common.c | 10 +++++++++-
 drivers/i2c/busses/i2c-designware-core.h   |  8 ++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4dc57fd56170..8ed0b588d67b 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -374,7 +374,15 @@ static void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
 		regmap_write(dev->map, DW_IC_CON, dev->slave_cfg);
 		regmap_write(dev->map, DW_IC_SAR, dev->slave->addr);
 		regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_SLAVE_MASK);
-		__i2c_dw_enable(dev);
+		/*
+		 * Per the Synopsys DW_apb_i2c DataBook (Multiple SAR feature),
+		 * controllers synthesised with multiple SARs only acknowledge
+		 * traffic to IC_SAR when IC_ENABLE.SAR_EN is set; a plain enable
+		 * is not enough. The bit is reserved (ignored) on single-SAR
+		 * controllers.
+		 */
+		regmap_write(dev->map, DW_IC_ENABLE,
+			     DW_IC_ENABLE_ENABLE | DW_IC_ENABLE_SAR_EN);
 		break;
 	default:
 		WARN(1, "Invalid mode %d\n", mode);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 9d8d104cc391..b237d093c562 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -125,6 +125,14 @@
 
 #define DW_IC_ENABLE_ENABLE			BIT(0)
 #define DW_IC_ENABLE_ABORT			BIT(1)
+/*
+ * IC_ENABLE.SAR_EN, per the Synopsys DW_apb_i2c DataBook (Multiple SAR
+ * feature): on controllers synthesised with multiple SARs (e.g. Marvell
+ * CN20K) the primary slave address (IC_SAR) is only acknowledged when this
+ * bit is set. Must be programmed while IC_ENABLE.ENABLE is 0. Reserved on
+ * single-SAR controllers.
+ */
+#define DW_IC_ENABLE_SAR_EN			BIT(19)
 
 #define DW_IC_STATUS_ACTIVITY			BIT(0)
 #define DW_IC_STATUS_TFE			BIT(2)
-- 
2.43.0