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

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

Posted by Umesh Kumar 1 month, 4 weeks ago

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

Posted by Mika Westerberg 1 month, 4 weeks ago

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 v6] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

Posted by Umesh Kumar 1 month, 2 weeks ago

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 v2.03a (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>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Changes in v6:
 - Added Andy's Reviewed-by (missed in v5).

Changes in v5:
 - Fixed missing spaces in commit message.

Changes in v4:
 - Added DataBook version (v2.03a); removed duplicate comment from core.h.

Changes in v3:
 - Referenced the Synopsys DW_apb_i2c DataBook (Andy Shevchenko).

Changes in v2:
 - Dropped RFC tag; added Mika's Acked-by.

v1 (RFC): https://lore.kernel.org/linux-i2c/20260804041842.3205356-1-ukumar@marvell.com/

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

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4dc57fd56170..880b9a74b3ff 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 v2.03a (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..1390c8862a68 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -125,6 +125,7 @@
 
 #define DW_IC_ENABLE_ENABLE			BIT(0)
 #define DW_IC_ENABLE_ABORT			BIT(1)
+#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

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

Posted by Umesh Kumar 1 month, 2 weeks ago

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 v2.03a (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>
---

Changes in v5:
 - Fixed missing spaces in commit message (Andy Shevchenko).
Changes in v4:
 - Added DataBook version (v2.03a) to commit message and code comment.
 - Removed duplicate comment from core.h; kept only in the .c file.
 - Added changelog.
Changes in v3:
 - Referenced the Synopsys DW_apb_i2c DataBook (Andy Shevchenko).
Changes in v2:
 - Dropped RFC tag; added Mika's Acked-by.
v1 (RFC): https://lore.kernel.org/linux-i2c/20260804041842.3205356-1-ukumar@marvell.com/

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

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4dc57fd56170..880b9a74b3ff 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 v2.03a (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..1390c8862a68 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -125,6 +125,7 @@
 
 #define DW_IC_ENABLE_ENABLE			BIT(0)
 #define DW_IC_ENABLE_ABORT			BIT(1)
+#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 v5] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

Posted by Andy Shevchenko 1 month, 2 weeks ago

On Mon, Aug 17, 2026 at 09:51:41PM +0530, Umesh Kumar wrote:
> 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 v2.03a (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.

I gave my tag and it is absent here. What's going on?
Do you think the fix is not only that and requires a new round of review?

-- 
With Best Regards,
Andy Shevchenko

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

Posted by Umesh Kumar 1 month, 2 weeks ago

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 v2.03a (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>
---

Changes in v4:
 - Added DataBook version (v2.03a) to commit message and code comment.
 - Removed duplicate comment from core.h; kept only in the .c file.

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

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4dc57fd56170..880b9a74b3ff 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 v2.03a (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..1390c8862a68 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -125,6 +125,7 @@
 
 #define DW_IC_ENABLE_ENABLE			BIT(0)
 #define DW_IC_ENABLE_ABORT			BIT(1)
+#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 v4] i2c: designware: Enable SAR in slave mode for Multiple SAR controllers

Posted by Andy Shevchenko 1 month, 2 weeks ago

On Mon, Aug 17, 2026 at 02:10:23PM +0530, Umesh Kumar wrote:
> 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 v2.03a (Multiple SAR
> feature),when this bit is set the core acknowledges the address matching

Missing space after ).

> IC_SAR;otherwise the address is NACKed. The slave enable path writes only

Same here for ;.

> 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.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

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

Posted by Umesh Kumar 1 month, 2 weeks ago

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

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

Posted by Andy Shevchenko 1 month, 2 weeks ago

On Wed, Aug 12, 2026 at 05:15:14PM +0530, Umesh Kumar wrote:
> 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),

Also provide a version of the DataBook. Something like v2.20c (hypothetical
example).

> 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>
> ---

This is a v3 and no changelog here... What's going on?

Assuming this will be provided
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> +		/*
> +		 * Per the Synopsys DW_apb_i2c DataBook (Multiple SAR feature),

Ditto (mind the version).

> +		 * 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.
> +		 */

...

> +/*
> + * 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.
> + */

Not sure if we need a dup comment as it's also in the code. Perhaps leave only
in the C code where it makes more sense?

> +#define DW_IC_ENABLE_SAR_EN			BIT(19)

-- 
With Best Regards,
Andy Shevchenko

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

Posted by Umesh Kumar 1 month, 3 weeks ago

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

Posted by Andy Shevchenko 1 month, 2 weeks ago

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