[PATCH v1 0/3] i2c: dwc: Add I2C DWC master/slave support for StarFive JHB100

[PATCH v1 0/3] i2c: dwc: Add I2C DWC master/slave support for StarFive JHB100

[lianfeng.ouyang]

From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

The Synopsys DesignWare Core (DWC) I2C controller is a variant of the
widely-used DesignWare I2C IP, with a distinct register layout and
enhanced features such as SMBus Alert and programmable FIFO depths.

This patch series introduces support for this controller as implemented
on the StarFive JHB100 platform, which utilizes it for both master and
slave operations (e.g., for MCTP over I2C).

The series is structured as follows:
1.  Adds the device tree binding document for the snps,dwc-i2c compatible.
2.  Prepares the existing i2c-designware-core by exporting and making
    certain key functions overridable, allowing code reuse.
3.  Introduces the new i2c-dwc-core driver, with separate modules for
    master and slave functionality, based on the 2023-07 revision of the
    Synopsys IP manual.

Key differences from the Existing i2c-designware Driver
1.  The DWC IP's offsets for all key registers are redefined. The driver
    maps to the correct addresses by overriding macros from the core
    header file in a new header (i2c-dwc-core.h).
2.  The host and slave of DWC IP need to perform probe callbacks
    separately, so they cannot be directly set through i2c_dew_set_mode
3.  Interrupts are cleared by writing​ to the corresponding bits in the
    INTR_CLRregister (write-1-to-clear).
4.  The DWC controller's IC_ENABLEregister contains an additional
    TX_CMD_BLOCKcontrol bit. When enabling the controller, the driver must
    ensure this bit is cleared. When disabling, only the ENABLEbit is
    cleared, preserving other configurations.

Lianfeng Ouyang (3):
  dt-bindings: i2c: snps,dwc-i2c: Add StarFive JHB100 bindings
  i2c: designware: Export symbols and add __weak for DWC I2C driver
  i2c: dwc: Add StarFive JHB100 I2C master/slave support

 .../devicetree/bindings/i2c/snps,dwc-i2c.yaml | 120 +++++
 MAINTAINERS                                   |   7 +
 drivers/i2c/busses/Kconfig                    |  34 ++
 drivers/i2c/busses/Makefile                   |   3 +
 drivers/i2c/busses/i2c-designware-common.c    |  57 ++-
 drivers/i2c/busses/i2c-designware-core.h      |  25 +
 drivers/i2c/busses/i2c-designware-master.c    |  14 +-
 drivers/i2c/busses/i2c-designware-platdrv.c   |   6 +
 drivers/i2c/busses/i2c-designware-slave.c     |   4 +-
 drivers/i2c/busses/i2c-dwc-core.h             | 192 ++++++++
 drivers/i2c/busses/i2c-dwc-master.c           | 441 ++++++++++++++++++
 drivers/i2c/busses/i2c-dwc-slave.c            | 180 +++++++
 12 files changed, 1068 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
 create mode 100644 drivers/i2c/busses/i2c-dwc-core.h
 create mode 100644 drivers/i2c/busses/i2c-dwc-master.c
 create mode 100644 drivers/i2c/busses/i2c-dwc-slave.c

--
2.43.0

[PATCH v1 1/3] dt-bindings: i2c: snps,dwc-i2c: Add StarFive JHB100 bindings

[lianfeng.ouyang]

From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

Add device tree bindings for the Synopsys DesignWare Core (DWC) I2C
controller and its StarFive JHB100 implementation

The binding introduces a new compatible string: "snps,dwc-i2c", intended
for the generic IP. It also defines two platform-specific compatibles
for the StarFive JHB100 implementation:
- "starfive,jhb100-dwc-i2c-master"
- "starfive,jhb100-dwc-i2c-slave"

The controller supports standard I2C and SMBus protocols, programmable
FIFO depths, and optional SMBus Alert routing. The binding documents
the necessary clocks, resets, and timing properties.

Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
---
 .../devicetree/bindings/i2c/snps,dwc-i2c.yaml | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
new file mode 100644
index 000000000000..7227f24f7cbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2024 StarFive Technology Co., Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/snps,dwc-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DWC I2C Controller
+
+maintainers:
+  - Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - description: Generic Synopsys DWC I2C controller
+        const: snps,dwc-i2c
+      - description: StarFive JHB100 I2C master controller
+        items:
+          - const: starfive,jhb100-dwc-i2c-master
+          - const: snps,dwc-i2c
+      - description: StarFive JHB100 I2C slave controller
+        items:
+          - const: starfive,jhb100-dwc-i2c-slave
+          - const: snps,dwc-i2c
+
+  reg:
+    description: DWC I2C controller memory mapped registers
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+    items:
+      - description: I2C controller reference clock source
+      - description: APB interface clock source
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: ref
+      - const: pclk
+
+  resets:
+    maxItems: 1
+
+  clock-frequency:
+    description: Desired I2C bus clock frequency in Hz
+    enum: [100000, 400000, 1000000, 3400000]
+    default: 400000
+
+  i2c-sda-hold-time-ns:
+    description: |
+      The property should contain the SDA hold time in nanoseconds.
+      This value is used to compute value written into DW_IC_SDA_HOLD register.
+
+  i2c-scl-falling-time-ns:
+    description: |
+      The property should contain the SCL falling time in nanoseconds.
+      This value is used to compute the tLOW period.
+    default: 300
+
+  i2c-sda-falling-time-ns:
+    description: |
+      The property should contain the SDA falling time in nanoseconds.
+      This value is used to compute the tHIGH period.
+    default: 300
+
+  starfive,mctp-i2c-ms:
+    description: |
+      The property should contain reference to the master node associated with the slave.
+      This value is only used in slave mode, especially for MCTP application.
+
+  dwc-i2c-tx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      The property describes the tx fifo depth.
+    default: 8
+
+  dwc-i2c-rx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      The property describes the rx fifo depth.
+    default: 8
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+examples:
+  - |
+    i2c@f0000 {
+      compatible = "snps,dwc-i2c";
+      reg = <0xf0000 0x1000>;
+      interrupts = <11>;
+      clock-frequency = <400000>;
+    };
+  - |
+    i2c@2000 {
+      compatible = "snps,dwc-i2c";
+      reg = <0x2000 0x100>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      clock-frequency = <400000>;
+      clocks = <&i2cclk>;
+      interrupts = <0>;
+
+      eeprom@64 {
+        compatible = "atmel,24c02";
+        reg = <0x64>;
+      };
+    };
+...
-- 
2.43.0

[PATCH v1 2/3] i2c: designware: Export symbols and add __weak for DWC I2C driver

[lianfeng.ouyang]

From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

The upcoming StarFive DWC I2C driver is based on the DesignWare I2C
core but requires its own probe and configuration routines due to
register layout differences.

Export several key functions (i2c_dw_probe_master, i2c_dw_init,
i2c_dw_xfer_init, i2c_dw_read_clear_intrbits, etc.) and mark them as
__weak. This allows the DWC driver to reuse the common infrastructure
while overriding the implementations where needed, promoting code
sharing without sacrificing flexibility for the DWC variant.

Additionally, extend the register map configuration and introduce the
MODEL_STARFIVE flag to accommodate the DWC IP's different register
space.

Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
---
 drivers/i2c/busses/i2c-designware-common.c  | 57 ++++++++++++++++++---
 drivers/i2c/busses/i2c-designware-core.h    | 25 +++++++++
 drivers/i2c/busses/i2c-designware-master.c  | 14 +++--
 drivers/i2c/busses/i2c-designware-platdrv.c |  6 +++
 drivers/i2c/busses/i2c-designware-slave.c   |  4 +-
 5 files changed, 91 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4dc57fd56170..cfeec5d338bb 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -167,6 +167,11 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
 	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
 		map_cfg.max_register = AMD_UCSI_INTR_REG;
 
+#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
+	if ((dev->flags & MODEL_MASK) == MODEL_STARFIVE)
+		map_cfg.max_register = DWC_IC_SMBUS_INTR_CLR;
+#endif
+
 	if (reg == swab32(DW_IC_COMP_TYPE_VALUE)) {
 		map_cfg.reg_read = dw_reg_read_swab;
 		map_cfg.reg_write = dw_reg_write_swab;
@@ -411,7 +416,7 @@ static void i2c_dw_write_timings(struct dw_i2c_dev *dev)
  *
  * The controller must be disabled before this function is called.
  */
-void i2c_dw_set_mode(struct dw_i2c_dev *dev, int mode)
+__weak void i2c_dw_set_mode(struct dw_i2c_dev *dev, int mode)
 {
 	if (mode == DW_IC_SLAVE && !dev->slave)
 		mode = DW_IC_MASTER;
@@ -430,7 +435,7 @@ void i2c_dw_set_mode(struct dw_i2c_dev *dev, int mode)
  *
  * Return: 0 on success, or negative errno otherwise.
  */
-int i2c_dw_init(struct dw_i2c_dev *dev)
+__weak int i2c_dw_init(struct dw_i2c_dev *dev)
 {
 	int ret;
 
@@ -806,10 +811,25 @@ static int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
 	if (ret)
 		return ret;
 
+#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
+	u32 tx_fifo_cfg = 8, rx_fifo_cfg = 8;
+
+#ifdef CONFIG_OF
+	ret = of_property_read_u32(dev->dev->of_node, "dwc-i2c-tx-fifo-depth", &tx_fifo_cfg);
+	if (!ret && (tx_fifo_cfg < 2 || tx_fifo_cfg > 256))
+		tx_fifo_cfg = 8;
+
+	ret = of_property_read_u32(dev->dev->of_node, "dwc-i2c-rx-fifo-depth", &rx_fifo_cfg);
+	if (!ret && (rx_fifo_cfg < 2 || rx_fifo_cfg > 256))
+		rx_fifo_cfg = 8;
+#endif
+	param = rx_fifo_cfg << 8 | tx_fifo_cfg << 16;
+#else
 	ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, &param);
 	i2c_dw_release_lock(dev);
 	if (ret)
 		return ret;
+#endif
 
 	tx_fifo_depth = FIELD_GET(DW_IC_FIFO_TX_FIELD, param) + 1;
 	rx_fifo_depth = FIELD_GET(DW_IC_FIFO_RX_FIELD, param) + 1;
@@ -835,7 +855,9 @@ u32 i2c_dw_func(struct i2c_adapter *adap)
 
 void i2c_dw_disable(struct dw_i2c_dev *dev)
 {
+#if !IS_ENABLED(CONFIG_I2C_DWC_CORE)
 	unsigned int dummy;
+#endif
 	int ret;
 
 	ret = i2c_dw_acquire_lock(dev);
@@ -847,7 +869,12 @@ void i2c_dw_disable(struct dw_i2c_dev *dev)
 
 	/* Disable all interrupts */
 	__i2c_dw_write_intr_mask(dev, 0);
+
+#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
+	regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_CLR_INTR);
+#else
 	regmap_read(dev->map, DW_IC_CLR_INTR, &dummy);
+#endif
 
 	i2c_dw_release_lock(dev);
 }
@@ -896,6 +923,12 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
 	if (ret)
 		return ret;
 
+#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
+	if (dev->mode == DW_IC_SLAVE)
+		i2c_dw_probe_slave(dev);
+	else
+		i2c_dw_probe_master(dev);
+#else
 	ret = i2c_dw_probe_master(dev);
 	if (ret)
 		return ret;
@@ -906,10 +939,16 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
 
 	if (!adap->name[0])
 		strscpy(adap->name, "Synopsys DesignWare I2C adapter");
+#endif
 
 	adap->retries = 3;
 	adap->algo = &i2c_dw_algo;
+#if IS_ENABLED(CONFIG_I2C_DWC_SLAVE)
+	if (dev->mode == DW_IC_SLAVE)
+		adap->algo = &i2c_dw_slave_algo;
+#else
 	adap->quirks = &i2c_dw_quirks;
+#endif
 	adap->dev.parent = dev->dev;
 	i2c_set_adapdata(adap, dev);
 
@@ -938,16 +977,18 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
 	if (!dev->emptyfifo_hold_master)
 		irq_flags |= IRQF_NO_THREAD;
 
-	ret = i2c_dw_acquire_lock(dev);
-	if (ret)
-		return ret;
+	if (!IS_ENABLED(CONFIG_I2C_DWC_CORE) || dev->mode == DW_IC_MASTER) {
+		ret = i2c_dw_acquire_lock(dev);
+		if (ret)
+			return ret;
 
-	__i2c_dw_write_intr_mask(dev, 0);
-	i2c_dw_release_lock(dev);
+		__i2c_dw_write_intr_mask(dev, 0);
+		i2c_dw_release_lock(dev);
+	}
 
 	if (!(dev->flags & ACCESS_POLLING)) {
 		ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr,
-				       irq_flags, dev_name(dev->dev), dev);
+				irq_flags, dev_name(dev->dev), dev);
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 9d8d104cc391..263bff23dd3b 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -321,6 +321,11 @@ struct dw_i2c_dev {
 	u32			bus_capacitance_pF;
 	bool			clk_freq_optimized;
 	bool			emptyfifo_hold_master;
+#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
+	u16			scl_hcnt;
+	u16			scl_lcnt;
+	struct i2c_adapter	*ms_adapter; /* Bind another I2C master controller */
+#endif
 };
 
 #define ACCESS_INTR_MASK			BIT(0)
@@ -328,6 +333,7 @@ struct dw_i2c_dev {
 #define ARBITRATION_SEMAPHORE			BIT(2)
 #define ACCESS_POLLING				BIT(3)
 
+#define MODEL_STARFIVE				BIT(9)
 #define MODEL_AMD_NAVI_GPU			BIT(10)
 #define MODEL_WANGXUN_SP			BIT(11)
 #define MODEL_MASK				GENMASK(11, 8)
@@ -359,8 +365,16 @@ int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev);
 u32 i2c_dw_func(struct i2c_adapter *adap);
 irqreturn_t i2c_dw_isr_master(struct dw_i2c_dev *dev);
 
+void i2c_dw_xfer_init(struct dw_i2c_dev *dev);
+int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev);
+u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev);
+u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev);
+
 extern const struct dev_pm_ops i2c_dw_dev_pm_ops;
 
+#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
+#include "i2c-dwc-core.h"
+#else
 static inline void __i2c_dw_enable(struct dw_i2c_dev *dev)
 {
 	dev->status |= STATUS_ACTIVE;
@@ -372,6 +386,7 @@ static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
 	regmap_write(dev->map, DW_IC_ENABLE, 0);
 	dev->status &= ~STATUS_ACTIVE;
 }
+#endif
 
 static inline void __i2c_dw_write_intr_mask(struct dw_i2c_dev *dev,
 					    unsigned int intr_mask)
@@ -409,11 +424,21 @@ static inline void i2c_dw_configure_slave(struct dw_i2c_dev *dev) { }
 static inline irqreturn_t i2c_dw_isr_slave(struct dw_i2c_dev *dev) { return IRQ_NONE; }
 #endif
 
+#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
+static inline void i2c_dw_configure(struct dw_i2c_dev *dev)
+{
+	if (device_is_compatible(dev->dev, "starfive,jhb100-dwc-i2c-slave"))
+		i2c_dw_configure_slave(dev);
+	else
+		i2c_dw_configure_master(dev);
+}
+#else
 static inline void i2c_dw_configure(struct dw_i2c_dev *dev)
 {
 	i2c_dw_configure_slave(dev);
 	i2c_dw_configure_master(dev);
 }
+#endif
 
 int i2c_dw_probe(struct dw_i2c_dev *dev);
 int i2c_dw_init(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index de929b91d5ea..ef15f590ac5c 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -185,7 +185,7 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
 	return 0;
 }
 
-static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
+__weak void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 {
 	struct i2c_msg *msgs = dev->msgs;
 	u32 ic_con = 0, ic_tar = 0;
@@ -397,8 +397,12 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 			 * IC_RESTART_EN are set, we must manually
 			 * set restart bit between messages.
 			 */
+#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
+			if (dev->msg_write_idx > 0)
+#else
 			if ((dev->master_cfg & DW_IC_CON_RESTART_EN) &&
 					(dev->msg_write_idx > 0))
+#endif
 				need_restart = true;
 		}
 
@@ -570,7 +574,7 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 	}
 }
 
-static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
+__weak u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
 {
 	unsigned int stat, dummy;
 
@@ -921,7 +925,7 @@ int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	return i2c_dw_xfer_common(dev, msgs, num);
 }
 
-void i2c_dw_configure_master(struct dw_i2c_dev *dev)
+__weak void i2c_dw_configure_master(struct dw_i2c_dev *dev)
 {
 	struct i2c_timings *t = &dev->timings;
 
@@ -967,7 +971,7 @@ static void i2c_dw_unprepare_recovery(struct i2c_adapter *adap)
 	i2c_dw_init(dev);
 }
 
-static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
+int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
 {
 	struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
 	struct i2c_adapter *adap = &dev->adapter;
@@ -1006,7 +1010,7 @@ static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev)
 	return 0;
 }
 
-int i2c_dw_probe_master(struct dw_i2c_dev *dev)
+__weak int i2c_dw_probe_master(struct dw_i2c_dev *dev)
 {
 	unsigned int ic_con;
 	int ret;
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 426ffec06e22..a637c5ab0ea4 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -151,6 +151,10 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (device_property_present(device, "wx,i2c-snps-model"))
 		flags = MODEL_WANGXUN_SP | ACCESS_POLLING;
 
+	if (device_is_compatible(device, "starfive,jhb100-dwc-i2c-master") ||
+	    device_is_compatible(device, "starfive,jhb100-dwc-i2c-slave"))
+		flags |= MODEL_STARFIVE;
+
 	dev->dev = device;
 	dev->irq = irq;
 	dev->flags = flags;
@@ -255,6 +259,8 @@ static const struct of_device_id dw_i2c_of_match[] = {
 	{ .compatible = "mobileye,eyeq6lplus-i2c" },
 	{ .compatible = "mscc,ocelot-i2c" },
 	{ .compatible = "snps,designware-i2c" },
+	{ .compatible = "starfive,jhb100-dwc-i2c-master" },
+	{ .compatible = "starfive,jhb100-dwc-i2c-slave" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index ad0d5fbfa6d5..7b03dc88286a 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -61,7 +61,7 @@ int i2c_dw_unreg_slave(struct i2c_client *slave)
 	return 0;
 }
 
-static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
+__weak u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
 {
 	unsigned int stat, dummy;
 
@@ -176,7 +176,7 @@ irqreturn_t i2c_dw_isr_slave(struct dw_i2c_dev *dev)
 	return IRQ_HANDLED;
 }
 
-void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
+__weak void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
 {
 	if (dev->flags & ACCESS_POLLING)
 		return;
-- 
2.43.0

[PATCH v1 3/3] i2c: dwc: Add StarFive JHB100 I2C master/slave support

[lianfeng.ouyang]

From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

Add support for the Synopsys DesignWare Core (DWC) I2C controller
found on the StarFive JHB100 platform. The controller supports both
master and slave modes and uses a different register layout compared
to the standard DesignWare I2C IP.

Key differences handled by this driver:
1.  All key registers have different offsets. The driver redefines
    these offsets in i2c-dwc-core.h.
2.  Interrupts are cleared by writing to a separate INTR_CLR register
    (write-1-to-clear), unlike the read-to-clear scheme of the
    standard IP.
3.  The IC_ENABLE register has an additional TX_CMD_BLOCK control bit
    which must be managed during enable/disable sequences.
4.  Master and slave require separate probe callbacks and cannot rely
    solely on the runtime mode switching provided by i2c_dw_set_mode().

The driver leverages the common i2c-designware-core infrastructure
exported by the previous patch, while implementing its own timing
calculation, FIFO configuration, and interrupt handling to accommodate
the DWC register offsets and bit definitions.

Signed-off-by: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
---
 MAINTAINERS                         |   7 +
 drivers/i2c/busses/Kconfig          |  34 +++
 drivers/i2c/busses/Makefile         |   3 +
 drivers/i2c/busses/i2c-dwc-core.h   | 184 ++++++++++++
 drivers/i2c/busses/i2c-dwc-master.c | 441 ++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-dwc-slave.c  | 180 ++++++++++++
 6 files changed, 849 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-dwc-core.h
 create mode 100644 drivers/i2c/busses/i2c-dwc-master.c
 create mode 100644 drivers/i2c/busses/i2c-dwc-slave.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d3a6b3f6b6a0..730388b94e56 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25677,6 +25677,13 @@ L:	linux-i2c@vger.kernel.org
 S:	Supported
 F:	drivers/i2c/busses/i2c-designware-*
 
+SYNOPSYS DWC I2C DRIVER
+M:	Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
+L:	linux-i2c@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
+F:	drivers/i2c/busses/i2c-dwc-*
+
 SYNOPSYS DESIGNWARE I2C DRIVER - AMDISP
 M:	Nirujogi Pratap <pratap.nirujogi@amd.com>
 M:	Bin Du <bin.du@amd.com>
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 7cb6b9b864a7..7775b4f40d88 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -640,6 +640,40 @@ config I2C_DESIGNWARE_PCI
 
 endif
 
+config I2C_DWC_CORE
+	tristate "Synopsys DWC I2C adapter"
+	select REGMAP
+	help
+	  This is a driver for the Synopsys DesignWare CORE (DWC) I2C controller,
+	  a variant of the DesignWare I2C IP with a different register layout.
+	  It shares common infrastructure with the standard DesignWare I2C
+	  driver (I2C_DESIGNWARE_CORE)
+
+if I2C_DWC_CORE
+
+config I2C_DWC_SLAVE
+	bool "Synopsys DWC Slave"
+	default I2C_DWC_CORE
+	select I2C_SLAVE
+	help
+	  If you say yes to this option, support will be included for the
+	  Synopsys DWC I2C slave adapter.
+
+	  This is not a standalone module, this module compiles together with
+	  i2c-dwc-core.
+
+config I2C_DWC_PLATFORM
+	tristate "Synopsys DWC Platform"
+	default I2C_DWC_CORE
+	help
+	  If you say yes to this option, support will be included for the
+	  Synopsys DWC I2C adapter.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i2c-dwc-platform.
+
+endif
+
 config I2C_DIGICOLOR
 	tristate "Conexant Digicolor I2C driver"
 	depends on ARCH_DIGICOLOR || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 547123ab351f..3ed593011578 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -61,6 +61,9 @@ i2c-designware-platform-$(CONFIG_I2C_DESIGNWARE_BAYTRAIL) += i2c-designware-bayt
 obj-$(CONFIG_I2C_DESIGNWARE_AMDISP) += i2c-designware-amdisp.o
 obj-$(CONFIG_I2C_DESIGNWARE_PCI)			+= i2c-designware-pci.o
 i2c-designware-pci-y					:= i2c-designware-pcidrv.o
+obj-$(CONFIG_I2C_DWC_CORE) 				+= i2c-dwc-core.o
+i2c-dwc-core-y                                     	+= i2c-dwc-master.o
+i2c-dwc-core-$(CONFIG_I2C_DWC_SLAVE) 			+= i2c-dwc-slave.o
 obj-$(CONFIG_I2C_DIGICOLOR)	+= i2c-digicolor.o
 obj-$(CONFIG_I2C_EG20T)		+= i2c-eg20t.o
 obj-$(CONFIG_I2C_EMEV2)		+= i2c-emev2.o
diff --git a/drivers/i2c/busses/i2c-dwc-core.h b/drivers/i2c/busses/i2c-dwc-core.h
new file mode 100644
index 000000000000..eba2eaf116c9
--- /dev/null
+++ b/drivers/i2c/busses/i2c-dwc-core.h
@@ -0,0 +1,184 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Synopsys DWC I2C adapter driver.
+ *
+ * Based on the TI DAVINCI I2C adapter driver.
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software Inc.
+ * Copyright (C) 2009 Provigent Ltd.
+ */
+
+#include <linux/bits.h>
+#include <linux/compiler_types.h>
+#include <linux/completion.h>
+#include <linux/dev_printk.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#define IC_DFLT_OPERATION_REG_OFFSET		0x00
+#define IC_DFLT_I2C_REG_OFFSET			0x20
+#define IC_DFLT_SMBUS_REG_OFFSET		0xFC
+
+#define DW_IC_DEFAULT_FUNCTIONALITY		(I2C_FUNC_I2C | \
+						 I2C_FUNC_SMBUS_BYTE | \
+						 I2C_FUNC_SMBUS_BYTE_DATA | \
+						 I2C_FUNC_SMBUS_WORD_DATA | \
+						 I2C_FUNC_SMBUS_BLOCK_DATA | \
+						 I2C_FUNC_SMBUS_I2C_BLOCK)
+
+#define DWC_IC_CTRL_OP_MODE			BIT(0)
+#define DWC_IC_CTRL_SPEED_STD			BIT(4)
+#define DWC_IC_CTRL_SPEED_FAST			(2 << 4)
+#define DWC_IC_CTRL_SPEED_HIGH			(3 << 4)
+#define DWC_IC_CTRL_SPEED_MASK			GENMASK(5, 4)
+#define DWC_IC_CTRL_10BITADDR_TGT		BIT(8)
+#define DWC_IC_CTRL_10BITADDR_CTRLR		BIT(9)
+#define DWC_IC_CTRL_STOP_DET_IFADDRESSED	BIT(10)
+#define DWC_IC_CTRL_TX_EMPTY_CTRL		BIT(11)
+#define DWC_IC_CTRL_RX_FIFO_FULL_HLD_CTRL	BIT(12)
+#define DWC_IC_CTRL_BUS_CLEAR_CTRL		BIT(14)
+
+#define DWC_IC_DATA_CMD_DAT			GENMASK(7, 0)
+#define DWC_IC_DATA_CMD_FIRST_DATA_BYTE		BIT(11)
+#define DWC_IC_DATA_CMD_STOP			BIT(9)
+#define DWC_IC_DATA_CMD_CMD			BIT(8)
+
+/*
+ * Registers offset
+ */
+#undef DW_IC_ENABLE
+#define DW_IC_ENABLE				(IC_DFLT_OPERATION_REG_OFFSET + 0x04)
+#define DWC_IC_CAPABILITIES			(IC_DFLT_OPERATION_REG_OFFSET + 0x0c)
+#define DWC_IC_SMBUS_CAPABILITIES		(IC_DFLT_OPERATION_REG_OFFSET + 0x18)
+
+#define DWC_IC_CTRL				(IC_DFLT_I2C_REG_OFFSET + 0x04)
+#undef DW_IC_TAR
+#define DW_IC_TAR				(IC_DFLT_I2C_REG_OFFSET + 0x08)
+#define DWC_IC_DAR				(IC_DFLT_I2C_REG_OFFSET + 0x0C)
+#define DWC_IC_SCL_HCNT				(IC_DFLT_I2C_REG_OFFSET + 0x24)
+#define DWC_IC_SCL_LCNT				(IC_DFLT_I2C_REG_OFFSET + 0x28)
+#define DWC_IC_HS_SCL_HCNT			(IC_DFLT_I2C_REG_OFFSET + 0x2c)
+#define DWC_IC_HS_SCL_LCNT			(IC_DFLT_I2C_REG_OFFSET + 0x30)
+#undef DW_IC_SDA_HOLD
+#define DW_IC_SDA_HOLD				(IC_DFLT_I2C_REG_OFFSET + 0x34)
+#define DWC_IC_SPKLEN				(IC_DFLT_I2C_REG_OFFSET + 0x3c)
+#define DWC_IC_HS_SPKLEN			(IC_DFLT_I2C_REG_OFFSET + 0x40)
+#define DWC_IC_SCL_STUCK_AT_LOW_TIMEOUT		(IC_DFLT_I2C_REG_OFFSET + 0x44)
+#define DWC_IC_SCL_STUCK_AT_LOW_TIMEOUT_MAX	(IC_DFLT_I2C_REG_OFFSET + 0x48)
+#define DWC_IC_SDA_STUCK_AT_LOW_TIMEOUT		(IC_DFLT_I2C_REG_OFFSET + 0x4c)
+#undef DW_IC_DATA_CMD
+#define DW_IC_DATA_CMD				(IC_DFLT_I2C_REG_OFFSET + 0x58)
+#define DWC_IC_RX_TL				(IC_DFLT_I2C_REG_OFFSET + 0x5c)
+#define DWC_IC_TX_TL				(IC_DFLT_I2C_REG_OFFSET + 0x60)
+#undef DW_IC_INTR_STAT
+#define DW_IC_INTR_STAT				(IC_DFLT_I2C_REG_OFFSET + 0x74)
+#undef DW_IC_INTR_MASK
+#define DW_IC_INTR_MASK				(IC_DFLT_I2C_REG_OFFSET + 0x78)
+#undef DW_IC_RAW_INTR_STAT
+#define DW_IC_RAW_INTR_STAT			(IC_DFLT_I2C_REG_OFFSET + 0x7c)
+#define DWC_IC_INTR_CLR				(IC_DFLT_I2C_REG_OFFSET + 0x80)
+#define DWC_CLR_INTR				BIT(0)
+#define DWC_IC_CLR_RX_UNDER			BIT(1)
+#define DWC_IC_CLR_RX_OVER			BIT(2)
+#define DWC_IC_CLR_TX_OVER			BIT(3)
+#define DWC_IC_CLR_RD_REQ			BIT(4)
+#define DWC_IC_CLR_TX_ABRT			BIT(5)
+#define DWC_IC_CLR_RX_DONE			BIT(6)
+#define DWC_IC_CLR_ACTIVITY			BIT(7)
+#define DWC_IC_CLR_STOP_DET			BIT(8)
+#define DWC_IC_CLR_START_DET			BIT(9)
+#define DWC_IC_CLR_GEN_CALL			BIT(10)
+#define DWC_IC_CLR_RESTART_DET			BIT(11)
+#define DWC_IC_CLR_SCL_STUCK_DET		BIT(12)
+#undef DW_IC_ENABLE_STATUS
+#define DW_IC_ENABLE_STATUS			(IC_DFLT_I2C_REG_OFFSET + 0x84)
+#define DWC_IC_TX_TRMNT_SOURCE			(IC_DFLT_I2C_REG_OFFSET + 0x88)
+#undef DW_IC_STATUS
+#define DW_IC_STATUS				(IC_DFLT_I2C_REG_OFFSET + 0x8c)
+#undef DW_IC_TXFLR
+#define DW_IC_TXFLR				(IC_DFLT_I2C_REG_OFFSET + 0x90)
+#undef DW_IC_RXFLR
+#define DW_IC_RXFLR				(IC_DFLT_I2C_REG_OFFSET + 0x94)
+#define DW_IC_COMP_PARAM_1			0xf4
+#undef DW_IC_COMP_VERSION
+#define DW_IC_COMP_VERSION			(IC_DFLT_I2C_REG_OFFSET + 0xa4)
+#define DWC_IC_SDA_HOLD_MIN_VERS		0x3131312A /* "111*" == v1.11* */
+
+#undef DW_IC_COMP_TYPE
+#define DW_IC_COMP_TYPE				(IC_DFLT_I2C_REG_OFFSET + 0xa8)
+#define DWC_IC_INTR_SCL_STUCK_AT_LOW		BIT(14)
+
+#define DWC_IC_STATUS_ACTIVITY			BIT(0)
+#define DWC_IC_STATUS_TFE			BIT(2)
+#define DWC_IC_STATUS_RFNE			BIT(3)
+#define DWC_IC_STATUS_CTRLR_ACTIVITY		BIT(5)
+#define DWC_IC_STATUS_TGT_ACTIVITY		BIT(6)
+
+#define DW_IC_SDA_HOLD_TX_SHIFT			0
+#define DW_IC_SDA_HOLD_TX_MASK			GENMASK(15, 0)
+
+#define DW_IC_ERR_TX_ABRT			0x1
+
+#define DWC_IC_TAR_SMBUS_QUICK_CMD		BIT(16)
+#define DWC_IC_TAR_SPECIAL			BIT(11)
+
+#define DWC_IC_COMP_PARAM_1_SPEED_MODE_HIGH	(BIT(4) | BIT(5))
+#define DWC_IC_COMP_PARAM_1_SPEED_MODE_MASK	GENMASK(5, 4)
+
+#define DWC_IC_SMBUS_ARP_CTRL			(IC_DFLT_SMBUS_REG_OFFSET + 0x8)
+#define DWC_IC_SMBUS_INTR_STAT			(IC_DFLT_SMBUS_REG_OFFSET + 0x28)
+#define DWC_IC_SMBUS_INTR_CLR			(IC_DFLT_SMBUS_REG_OFFSET + 0x34)
+#define DWC_CLR_SMBUS_ALERT_DET			BIT(10)
+#define DWC_R_SMBUS_ALERT_DET			BIT(10)
+
+#define DWC_IC_DAR4_EN				19
+#define DWC_IC_DAR3_EN				18
+#define DWC_IC_DAR2_EN				17
+#define DWC_IC_DAR_EN				16
+
+#define DWC_IC_ENABLE_DAR_EN			BIT(DWC_IC_DAR_EN)
+#define DWC_IC_ENABLE_DAR2_EN			BIT(DWC_IC_DAR2_EN)
+#define DWC_IC_ENABLE_DAR3_EN			BIT(DWC_IC_DAR3_EN)
+#define DWC_IC_ENABLE_DAR4_EN			BIT(DWC_IC_DAR4_EN)
+
+#define DW_IC_ENABLE_TX_CMD_BLOCK		BIT(2)
+
+#define DWC_IC_SMBUS				10
+
+#define DWC_IC_CAPABILITIES_IC_SMBUS		BIT(DWC_IC_SMBUS)
+
+#define DWC_IC_SMBUS_ARP			2
+
+#define DWC_IC_SMBUS_CAPABILITIES_SMBUS_ARP	BIT(DWC_IC_SMBUS_ARP)
+
+#define DWC_IC_SMBUS_ARP_CTRL_NARP_DEVICE_TYPE	0
+
+#undef TXGBE_RX_FIFO_DEPTH
+#define TXGBE_RX_FIFO_DEPTH			0
+
+extern const struct i2c_algorithm i2c_dw_slave_algo;
+int i2c_dw_probe_slave(struct dw_i2c_dev *dev);
+int i2c_dw_probe_master(struct dw_i2c_dev *dev);
+void i2c_dw_read_clear_intrbits_common(struct dw_i2c_dev *dev);
+
+static inline void __i2c_dw_enable(struct dw_i2c_dev *dev)
+{
+	int val;
+
+	dev->status |= STATUS_ACTIVE;
+	regmap_read(dev->map, DW_IC_ENABLE, &val);
+	regmap_write(dev->map, DW_IC_ENABLE,
+		     ((val & ~DW_IC_ENABLE_TX_CMD_BLOCK) | DW_IC_ENABLE_ENABLE));
+}
+
+static inline void __i2c_dw_disable_nowait(struct dw_i2c_dev *dev)
+{
+	int val;
+
+	regmap_read(dev->map, DW_IC_ENABLE, &val);
+	regmap_write(dev->map, DW_IC_ENABLE, val & ~DW_IC_ENABLE_ENABLE);
+	dev->status &= ~STATUS_ACTIVE;
+}
diff --git a/drivers/i2c/busses/i2c-dwc-master.c b/drivers/i2c/busses/i2c-dwc-master.c
new file mode 100644
index 000000000000..6e8d1828f99e
--- /dev/null
+++ b/drivers/i2c/busses/i2c-dwc-master.c
@@ -0,0 +1,441 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Synopsys DWC I2C adapter driver (master only).
+ *
+ * Based on the TI DAVINCI I2C adapter driver.
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software Inc.
+ * Copyright (C) 2009 Provigent Ltd.
+ */
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/units.h>
+
+#include "i2c-designware-core.h"
+
+#define AMD_TIMEOUT_MIN_US	25
+#define AMD_TIMEOUT_MAX_US	250
+#define AMD_MASTERCFG_MASK	GENMASK(15, 0)
+
+/**
+ * i2c_dwc_scl_hcnt() -  Calculate SCL HCNT
+ * @ic_clk: Input clock in kHz
+ * @thigh: Duration in ns of logic 1 to generate
+ * @tr: SCL rise time in ns
+ * @spk_cnt: Spike count
+ */
+static u32 i2c_dwc_scl_hcnt(u32 ic_clk, u32 thigh, u32 tr, u32 spk_cnt)
+{
+	u64 min_thigh_cnt, rise_cnt;
+
+	/* Formula: cnt = f_kHz * t_ns * 10^(-6) */
+	min_thigh_cnt = DIV_ROUND_CLOSEST_ULL((u64)ic_clk * thigh, MICRO);
+	rise_cnt = DIV_ROUND_CLOSEST_ULL((u64)ic_clk * tr, MICRO);
+
+	return max(5, min_thigh_cnt + rise_cnt - spk_cnt - 3);
+}
+
+/**
+ * i2c_dwc_scl_lcnt() -  Calculate SCL LCNT
+ * @ic_clk: Input clock in kHz
+ * @tlow: Duration in ns of logic 0 to generate
+ * @tf: SCL fall time in ns
+ */
+static u32 i2c_dwc_scl_lcnt(u32 ic_clk, u32 tlow, u32 tf)
+{
+	u64 min_tlow_cnt, fall_cnt;
+
+	/* Formula: cnt = f_kHz * t_ns * 10^(-6) */
+	min_tlow_cnt = DIV_ROUND_CLOSEST_ULL((u64)ic_clk * tlow, MICRO);
+	fall_cnt = DIV_ROUND_CLOSEST_ULL((u64)ic_clk * tf, MICRO);
+
+	return max(6, min_tlow_cnt + fall_cnt);
+}
+
+static void i2c_dwc_configure_fifo_master(struct dw_i2c_dev *dev)
+{
+	/* Configure Tx/Rx FIFO threshold levels */
+	regmap_write(dev->map, DWC_IC_TX_TL, dev->tx_fifo_depth / 2);
+	regmap_write(dev->map, DWC_IC_RX_TL, 0);
+
+	/* Configure the I2C master */
+	regmap_write(dev->map, DWC_IC_CTRL, dev->master_cfg);
+}
+
+static int i2c_dwc_set_timings_master(struct dw_i2c_dev *dev)
+{
+	u32 scl_falling_time = 0, scl_rising_time = 0;
+	u32 scl_high_time = 0, scl_low_time = 0;
+	struct i2c_timings *t = &dev->timings;
+	unsigned int comp_param1;
+	u32 ic_clk, spk_cnt;
+	int ret;
+
+	ret = i2c_dw_acquire_lock(dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(dev->map, DWC_IC_CTRL, &comp_param1);
+	i2c_dw_release_lock(dev);
+	if (ret)
+		return ret;
+
+	ic_clk = i2c_dw_clk_rate(dev);
+
+	/* 50ns maximum spike */
+	spk_cnt = DIV_ROUND_CLOSEST_ULL((u64)ic_clk * 50, MICRO);
+
+	regmap_write(dev->map, DWC_IC_HS_SPKLEN, spk_cnt);
+	regmap_write(dev->map, DWC_IC_SPKLEN, spk_cnt);
+
+	/* Parse user defined rise time and fall time*/
+	if (t->scl_rise_ns)
+		scl_rising_time = t->scl_rise_ns;
+
+	if (t->scl_fall_ns)
+		scl_falling_time = t->scl_fall_ns;
+
+	/* Ensure the rise time and fall time should not lower than t_rise_max
+	 * and t_fall_max specification, else it would run faster than expected
+	 * frequency
+	 */
+	switch (t->bus_freq_hz) {
+	case I2C_MAX_STANDARD_MODE_FREQ:
+		scl_rising_time = max(scl_rising_time, 1000);
+		scl_falling_time = max(scl_falling_time, 300);
+		scl_high_time = 4000;	/* tHIGH_min = 4.0 us */
+		scl_low_time = 4700;	/* tLOW_min = 4.7 us */
+		break;
+	case I2C_MAX_FAST_MODE_FREQ:
+		scl_rising_time = max(scl_rising_time, 300);
+		scl_falling_time = max(scl_falling_time, 300);
+		scl_high_time = 600;	/* tHIGH_min = 600 ns */
+		scl_low_time = 1300;	/* tLOW_min = 1.3 us */
+		break;
+	case I2C_MAX_FAST_MODE_PLUS_FREQ:
+		scl_rising_time = max(scl_rising_time, 120);
+		scl_falling_time = max(scl_falling_time, 120);
+		scl_high_time = 260;	/* tHIGH_min = 260 ns */
+		scl_low_time = 500;	/* tLOW_min = 500 ns */
+		break;
+	case I2C_MAX_HIGH_SPEED_MODE_FREQ:
+		scl_rising_time = max(scl_rising_time, 40);
+		scl_falling_time = max(scl_falling_time, 40);
+		scl_high_time = 60;	/* tHIGH_min = 60 ns */
+		scl_low_time = 160;	/* tLOW_min = 160 ns */
+		break;
+	default:
+		scl_rising_time = max(scl_rising_time, 1000);
+		scl_falling_time = max(scl_falling_time, 300);
+		scl_high_time = 4000;	/* tHIGH_min = 4.0 us */
+		scl_low_time = 4700;	/* tLOW_min = 4.7 us */
+		break;
+	}
+
+	ic_clk = i2c_dw_clk_rate(dev);
+
+	if (!dev->scl_hcnt || !dev->scl_lcnt) {
+		dev->scl_hcnt = i2c_dwc_scl_hcnt(ic_clk, scl_high_time,
+						 scl_rising_time, spk_cnt);
+		dev->scl_lcnt = i2c_dwc_scl_lcnt(ic_clk, scl_low_time,
+						 scl_falling_time);
+	}
+
+	dev_dbg(dev->dev, "Bus speed: %s\n", i2c_freq_mode_string(t->bus_freq_hz));
+
+	/* Check is high speed possible and fall back to fast mode if not */
+	if ((dev->master_cfg & DWC_IC_CTRL_SPEED_MASK) == DWC_IC_CTRL_SPEED_HIGH) {
+		if ((comp_param1 & DWC_IC_COMP_PARAM_1_SPEED_MODE_MASK)
+			!= DWC_IC_COMP_PARAM_1_SPEED_MODE_HIGH) {
+			dev_err(dev->dev, "High Speed not supported!\n");
+			t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ;
+			dev->master_cfg &= ~DWC_IC_CTRL_SPEED_MASK;
+			dev->master_cfg |= DWC_IC_CTRL_SPEED_FAST;
+			dev->hs_hcnt = 0;
+			dev->hs_lcnt = 0;
+
+			/* Replace with I2C_MAX_FAST_MODE_PLUS_FREQ */
+			scl_rising_time = max(scl_rising_time, 120);
+			scl_falling_time = max(scl_falling_time, 120);
+			scl_high_time = 260;	/* tHIGH_min = 260 ns */
+			scl_low_time = 500;	/* tLOW_min = 500 ns */
+
+			dev->scl_hcnt = i2c_dwc_scl_hcnt(ic_clk, scl_high_time,
+							 scl_rising_time, spk_cnt);
+			dev->scl_lcnt = i2c_dwc_scl_lcnt(ic_clk, scl_low_time,
+							 scl_falling_time);
+		} else if (!dev->hs_hcnt || !dev->hs_lcnt) {
+			dev->hs_hcnt = dev->scl_hcnt;
+			dev->hs_lcnt = dev->scl_lcnt;
+		}
+		dev_dbg(dev->dev, "High Speed Mode HCNT:LCNT = %d:%d\n",
+			dev->hs_hcnt, dev->hs_lcnt);
+	} else {
+		dev_dbg(dev->dev, "HCNT:LCNT: %d:%d\n", dev->scl_hcnt, dev->scl_lcnt);
+	}
+
+	return 0;
+}
+
+int i2c_dw_init(struct dw_i2c_dev *dev)
+{
+	int ret;
+
+	ret = i2c_dw_acquire_lock(dev);
+	if (ret)
+		return ret;
+
+	/* Disable the adapter */
+	__i2c_dw_disable(dev);
+
+	/* Write standard speed timing parameters */
+	regmap_write(dev->map, DWC_IC_SCL_HCNT, dev->scl_hcnt);
+	regmap_write(dev->map, DWC_IC_SCL_LCNT, dev->scl_lcnt);
+
+	/* Write high speed timing parameters if supported */
+	if (dev->hs_hcnt && dev->hs_lcnt) {
+		regmap_write(dev->map, DWC_IC_HS_SCL_HCNT, dev->hs_hcnt);
+		regmap_write(dev->map, DWC_IC_HS_SCL_LCNT, dev->hs_lcnt);
+	}
+
+	/* Write SDA hold time if supported */
+	if (dev->sda_hold_time)
+		regmap_write(dev->map, DW_IC_SDA_HOLD, dev->sda_hold_time);
+
+	i2c_dwc_configure_fifo_master(dev);
+	i2c_dw_release_lock(dev);
+
+	return 0;
+}
+
+static int i2c_dwc_set_sda_hold_time(struct dw_i2c_dev *dev)
+{
+	int ret = regmap_write_bits(dev->map,
+			DW_IC_SDA_HOLD,
+			DW_IC_SDA_HOLD_TX_MASK,
+			dev->sda_hold_time);
+	return ret;
+}
+
+void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
+{
+	struct i2c_msg *msgs = dev->msgs;
+	u32 ic_ctrl = 0, ic_tar = 0;
+	unsigned int dummy;
+
+	/* Disable the adapter */
+	__i2c_dw_disable(dev);
+
+	/* If the slave address is ten bit address, enable 10BITADDR */
+	if (msgs[dev->msg_write_idx].flags & I2C_M_TEN) {
+		ic_ctrl = DWC_IC_CTRL_10BITADDR_CTRLR;
+		/*
+		 * If I2C_DYNAMIC_TAR_UPDATE is set, the 10-bit addressing
+		 * mode has to be enabled via bit 12 of IC_TAR register.
+		 * We set it always as I2C_DYNAMIC_TAR_UPDATE can't be
+		 * detected from registers.
+		 */
+		ic_tar = DW_IC_TAR_10BITADDR_MASTER;
+	}
+
+	/* From i2c-core-smbus.c, I2C_SMBUS_QUICK intentionally has msg[0].len = 0 */
+	if (dev->msgs[0].len == 0)
+		ic_tar |= DWC_IC_TAR_SMBUS_QUICK_CMD | DWC_IC_TAR_SPECIAL;
+
+	regmap_update_bits(dev->map, DWC_IC_CTRL, DWC_IC_CTRL_10BITADDR_CTRLR,
+			   ic_ctrl);
+
+	/*
+	 * Set the slave (target) address and enable 10-bit addressing mode
+	 * if applicable.
+	 */
+	regmap_write(dev->map, DW_IC_TAR,
+		     msgs[dev->msg_write_idx].addr | ic_tar);
+
+	/* Enforce disabled interrupts (due to HW issues) */
+	regmap_write(dev->map, DW_IC_INTR_MASK, 0);
+
+	/* Enable the adapter */
+	__i2c_dw_enable(dev);
+
+	/* Dummy read to avoid the register getting stuck on Bay Trail */
+	regmap_read(dev->map, DW_IC_ENABLE_STATUS, &dummy);
+
+	/* Clear and enable interrupts */
+	regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_CLR_INTR);
+	regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_MASTER_MASK);
+}
+
+void i2c_dw_read_clear_intrbits_common(struct dw_i2c_dev *dev)
+{
+	unsigned int stat;
+
+	/*
+	 * The IC_INTR_STAT register just indicates "enabled" interrupts.
+	 * The unmasked raw version of interrupt status bits is available
+	 * in the IC_RAW_INTR_STAT register.
+	 *
+	 * That is,
+	 *   stat = readl(IC_INTR_STAT);
+	 * equals to,
+	 *   stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
+	 *
+	 * The raw version might be useful for debugging purposes.
+	 */
+	regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
+	/*
+	 * Do not use the IC_CLR_INTR register to clear interrupts, or
+	 * you'll miss some interrupts, triggered during the period from
+	 * readl(IC_INTR_STAT) to readl(IC_CLR_INTR).
+	 *
+	 * Instead, use the separately-prepared IC_CLR_* registers.
+	 */
+	if (stat & DW_IC_INTR_RX_UNDER)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_RX_UNDER);
+	if (stat & DW_IC_INTR_RX_OVER)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_RX_OVER);
+	if (stat & DW_IC_INTR_TX_OVER)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_TX_OVER);
+	if (stat & DW_IC_INTR_RD_REQ)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_RD_REQ);
+	if (stat & DW_IC_INTR_RX_DONE)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_RX_DONE);
+	if (stat & DW_IC_INTR_ACTIVITY)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_ACTIVITY);
+	if (stat & DW_IC_INTR_START_DET)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_START_DET);
+	if (stat & DW_IC_INTR_GEN_CALL)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_GEN_CALL);
+}
+EXPORT_SYMBOL_GPL(i2c_dw_read_clear_intrbits_common);
+
+u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
+{
+	unsigned int stat;
+
+	/*
+	 * The IC_INTR_STAT register just indicates "enabled" interrupts.
+	 * The unmasked raw version of interrupt status bits is available
+	 * in the IC_RAW_INTR_STAT register.
+	 *
+	 * That is,
+	 *   stat = readl(IC_INTR_STAT);
+	 * equals to,
+	 *   stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
+	 *
+	 * The raw version might be useful for debugging purposes.
+	 */
+	regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
+
+	if (stat & DW_IC_INTR_TX_ABRT) {
+		/*
+		 * The IC_TX_ABRT_SOURCE register is cleared whenever
+		 * the IC_CLR_TX_ABRT is read.  Preserve it beforehand.
+		 */
+		regmap_read(dev->map, DWC_IC_TX_TRMNT_SOURCE, &dev->abort_source);
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_TX_ABRT);
+	}
+	if ((stat & DW_IC_INTR_STOP_DET) &&
+	    (dev->rx_outstanding == 0 || (stat & DW_IC_INTR_RX_FULL)))
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_STOP_DET);
+	if (stat & DW_IC_INTR_RESTART_DET)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_RESTART_DET);
+	if (stat & DWC_IC_INTR_SCL_STUCK_AT_LOW)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_SCL_STUCK_DET);
+
+	i2c_dw_read_clear_intrbits_common(dev);
+
+	return stat;
+}
+
+void i2c_dw_configure_master(struct dw_i2c_dev *dev)
+{
+	struct i2c_timings *t = &dev->timings;
+
+	dev->functionality = I2C_FUNC_10BIT_ADDR | DW_IC_DEFAULT_FUNCTIONALITY |
+		I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_PEC;
+
+	dev->master_cfg = DWC_IC_CTRL_OP_MODE;
+
+	dev->mode = DW_IC_MASTER;
+
+	switch (t->bus_freq_hz) {
+	case I2C_MAX_STANDARD_MODE_FREQ:
+		dev->master_cfg |= DWC_IC_CTRL_SPEED_STD;
+		break;
+	case I2C_MAX_FAST_MODE_FREQ:
+		dev->master_cfg |= DWC_IC_CTRL_SPEED_FAST;
+		break;
+	case I2C_MAX_FAST_MODE_PLUS_FREQ:
+		dev->master_cfg |= DWC_IC_CTRL_SPEED_FAST;
+		break;
+	case I2C_MAX_HIGH_SPEED_MODE_FREQ:
+		dev->master_cfg |= DWC_IC_CTRL_SPEED_HIGH;
+		break;
+	default:
+		dev->master_cfg |= DWC_IC_CTRL_SPEED_FAST;
+	}
+
+	dev->set_sda_hold_time = i2c_dwc_set_sda_hold_time;
+}
+
+int i2c_dw_probe_master(struct dw_i2c_dev *dev)
+{
+	struct i2c_adapter *adap = &dev->adapter;
+	unsigned int ic_ctrl;
+	int ret;
+
+	init_completion(&dev->cmd_complete);
+
+	ret = i2c_dwc_set_timings_master(dev);
+	if (ret)
+		return ret;
+
+	/* Lock the bus for accessing DWC_IC_CTRL */
+	ret = i2c_dw_acquire_lock(dev);
+	if (ret)
+		return ret;
+
+	/*
+	 * On AMD platforms BIOS advertises the bus clear feature
+	 * and enables the SCL/SDA stuck low. SMU FW does the
+	 * bus recovery process. Driver should not ignore this BIOS
+	 * advertisement of bus clear feature.
+	 */
+	ret = regmap_read(dev->map, DWC_IC_CTRL, &ic_ctrl);
+	i2c_dw_release_lock(dev);
+	if (ret)
+		return ret;
+
+	if (ic_ctrl & DWC_IC_CTRL_BUS_CLEAR_CTRL)
+		dev->master_cfg |= DWC_IC_CTRL_BUS_CLEAR_CTRL;
+
+	ret = i2c_dw_init(dev);
+	if (ret)
+		return ret;
+
+	snprintf(adap->name, sizeof(adap->name),
+		 "Synopsys DWC I2C adapter");
+
+	ret = i2c_dw_init_recovery_info(dev);
+	if (ret)
+		return ret;
+
+	return ret;
+}
+
+MODULE_AUTHOR("Sankarshan shukla <sashukla@synopsys.com>");
+MODULE_DESCRIPTION("Synopsys DWC I2C bus master adapter");
+MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-dwc-slave.c b/drivers/i2c/busses/i2c-dwc-slave.c
new file mode 100644
index 000000000000..5369eb881238
--- /dev/null
+++ b/drivers/i2c/busses/i2c-dwc-slave.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Synopsys DWC I2C adapter driver (slave only).
+ *
+ * Based on the Synopsys DWC I2C adapter driver (master).
+ *
+ * Copyright (C) 2016 Synopsys Inc.
+ */
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#include "i2c-designware-core.h"
+
+void i2c_dw_set_mode(struct dw_i2c_dev *dev, int mode)
+{
+	if (dev->mode == DW_IC_SLAVE && mode == DW_IC_SLAVE) {
+		regmap_write(dev->map, DWC_IC_DAR, dev->slave->addr);
+		__i2c_dw_enable(dev);
+		dev->status = 0;
+	}
+}
+
+static void i2c_dwc_configure_fifo_slave(struct dw_i2c_dev *dev)
+{
+	/* Configure the I2C slave. */
+	regmap_write(dev->map, DWC_IC_CTRL, 0);
+	regmap_write(dev->map, DWC_IC_CTRL, dev->slave_cfg);
+	regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_SLAVE_MASK);
+}
+
+/**
+ * i2c_dwc_init_slave() - Initialize the DWC i2c slave hardware
+ * @dev: device private data
+ *
+ * This function configures and enables the I2C in slave mode.
+ * This function is called during I2C init function, and in case of timeout at
+ * run time.
+ */
+static int i2c_dwc_init_slave(struct dw_i2c_dev *dev)
+{
+	int ret;
+	int r_value;
+
+	ret = i2c_dw_acquire_lock(dev);
+	if (ret)
+		return ret;
+
+	/* Disable the adapter. */
+	__i2c_dw_disable(dev);
+
+	/* Write SDA hold time if supported */
+	if (dev->sda_hold_time)
+		regmap_write(dev->map, DW_IC_SDA_HOLD, dev->sda_hold_time);
+
+	regmap_read(dev->map, DWC_IC_CAPABILITIES, &r_value);
+	if (r_value & DWC_IC_CAPABILITIES_IC_SMBUS) {
+		regmap_read(dev->map, DWC_IC_SMBUS_CAPABILITIES, &r_value);
+		if (r_value & DWC_IC_SMBUS_CAPABILITIES_SMBUS_ARP) {
+			regmap_write(dev->map, DW_IC_ENABLE, DWC_IC_ENABLE_DAR_EN);
+			regmap_write(dev->map, DWC_IC_SMBUS_ARP_CTRL,
+				     DWC_IC_SMBUS_ARP_CTRL_NARP_DEVICE_TYPE);
+		}
+	}
+
+	i2c_dwc_configure_fifo_slave(dev);
+	i2c_dw_release_lock(dev);
+
+	return 0;
+}
+
+u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
+{
+	unsigned int stat;
+	unsigned int smbus_stat;
+
+	/*
+	 * The IC_INTR_STAT register just indicates "enabled" interrupts.
+	 * The unmasked raw version of interrupt status bits is available
+	 * in the IC_RAW_INTR_STAT register.
+	 *
+	 * That is,
+	 *   stat = readl(IC_INTR_STAT);
+	 * equals to,
+	 *   stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
+	 *
+	 * The raw version might be useful for debugging purposes.
+	 */
+	regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
+	regmap_read(dev->map, DWC_IC_SMBUS_INTR_STAT, &smbus_stat);
+
+	/*
+	 * Do not use the IC_CLR_INTR register to clear interrupts, or
+	 * you'll miss some interrupts, triggered during the period from
+	 * readl(IC_INTR_STAT) to readl(IC_CLR_INTR).
+	 *
+	 * Instead, use the separately-prepared IC_CLR_* registers.
+	 */
+
+	if (stat & DW_IC_INTR_TX_ABRT)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_TX_ABRT);
+	if (stat & DW_IC_INTR_STOP_DET)
+		regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_IC_CLR_STOP_DET);
+	if (smbus_stat & DWC_R_SMBUS_ALERT_DET)
+		regmap_write(dev->map, DWC_IC_SMBUS_INTR_CLR, DWC_CLR_SMBUS_ALERT_DET);
+
+	i2c_dw_read_clear_intrbits_common(dev);
+
+	return stat;
+}
+
+#if IS_ENABLED(CONFIG_I2C_DWC_SLAVE)
+static int i2c_dwc_xfer_ms(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
+
+	if (!dev->ms_adapter)
+		return -ENXIO;
+
+	return i2c_dw_xfer(dev->ms_adapter, msgs, num);
+}
+#endif
+
+const struct i2c_algorithm i2c_dw_slave_algo = {
+	.master_xfer = i2c_dwc_xfer_ms,
+	.functionality = i2c_dw_func,
+	.reg_slave = i2c_dw_reg_slave,
+	.unreg_slave = i2c_dw_unreg_slave,
+};
+
+void i2c_dw_configure_slave(struct dw_i2c_dev *dev)
+{
+	dev->functionality = I2C_FUNC_SLAVE | DW_IC_DEFAULT_FUNCTIONALITY;
+
+	dev->slave_cfg = DWC_IC_CTRL_RX_FIFO_FULL_HLD_CTRL |
+			 DWC_IC_CTRL_STOP_DET_IFADDRESSED;
+
+	dev->mode = DW_IC_SLAVE;
+}
+
+int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
+{
+	struct i2c_adapter *adap = &dev->adapter;
+	struct device_node *ms_node;
+	int ret;
+
+	ret = i2c_dwc_init_slave(dev);
+	if (ret)
+		return ret;
+
+	snprintf(adap->name, sizeof(adap->name), "Synopsys DWC I2C Slave adapter");
+
+	/* Get the specified I2C master adapter from DTS for MCTP */
+	ms_node = of_parse_phandle(dev->dev->of_node, "starfive,mctp-i2c-ms", 0);
+	if (ms_node) {
+		struct platform_device *ms_pdev = of_find_device_by_node(ms_node);
+
+		if (ms_pdev) {
+			struct dw_i2c_dev *ms_dev = platform_get_drvdata(ms_pdev);
+
+			if (ms_dev)
+				dev->ms_adapter = &ms_dev->adapter;
+		}
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_probe_slave);
+
+MODULE_AUTHOR("Sankarshan shukla <sashukla@synopsys.com>");
+MODULE_DESCRIPTION("Synopsys DWC I2C bus slave adapter");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH v1 1/3] dt-bindings: i2c: snps,dwc-i2c: Add StarFive JHB100 bindings

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The patch introduces a redundant device tree binding schema (`snps,dwc-i2c.yaml`) and a duplicate generic compatible string (`snps,dwc-i2c`) for the Synopsys DesignWare I2C controller.
- [Medium] Compatible strings are incorrectly used to encode the operational mode (master vs. slave) instead of describing the hardware IP variant.
- [Medium] FIFO depth properties (`dwc-i2c-tx-fifo-depth` and `dwc-i2c-rx-fifo-depth`) use an invalid and unregistered vendor prefix.
- [Medium] The custom property `starfive,mctp-i2c-ms` improperly uses the standard `-ms` suffix for a `phandle` reference and lacks an explicit type definition.
- [Medium] Missing constraints for the slave-only property `starfive,mctp-i2c-ms`.
--

commit 7e9b926c17cf1f417236a77e427787e7ba5331b1
Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

dt-bindings: i2c: snps,dwc-i2c: Add StarFive JHB100 bindings

Add device tree bindings for the Synopsys DesignWare Core (DWC) I2C
controller and its StarFive JHB100 implementation. The binding introduces
a new compatible string for the generic IP and two platform-specific
compatibles.

> diff --git a/Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
> new file mode 100644
> index 0000000000000..7227f24f7cbe0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
> @@ -0,0 +1,120 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2024 StarFive Technology Co., Ltd.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/snps,dwc-i2c.yaml#

[Severity: High]
Does this introduce a redundant binding schema?

The Synopsys DesignWare I2C controller is already documented under
Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml with the
generic compatible string snps,designware-i2c.

Creating a parallel schema for the same hardware block might fracture the
hardware description and bypass existing constraints. Could the StarFive
JHB100 variants be added to the existing designware-i2c schema instead?

[ ... ]

> +properties:
> +  compatible:
> +    oneOf:
> +      - description: Generic Synopsys DWC I2C controller
> +        const: snps,dwc-i2c
> +      - description: StarFive JHB100 I2C master controller
> +        items:
> +          - const: starfive,jhb100-dwc-i2c-master
> +          - const: snps,dwc-i2c
> +      - description: StarFive JHB100 I2C slave controller
> +        items:
> +          - const: starfive,jhb100-dwc-i2c-slave
> +          - const: snps,dwc-i2c

[Severity: Medium]
Are these compatible strings encoding the software operational mode rather
than describing the hardware IP variant?

Since the hardware natively supports both modes, shouldn't the operating
mode be dynamically determined by the framework or described using standard
subsystem properties like an i2c-slave sub-node?

[ ... ]

> +  starfive,mctp-i2c-ms:
> +    description: |
> +      The property should contain reference to the master node associated with the slave.
> +      This value is only used in slave mode, especially for MCTP application.

[Severity: Medium]
Does the -ms suffix conflict with dt-schema evaluation? Properties ending
with -ms are strictly evaluated as a time unit in milliseconds, but the
description indicates this is a phandle reference to a master node.

Also, is there an explicit type definition missing here? Custom properties
intended as a phandle typically require an explicit
$ref: /schemas/types.yaml#/definitions/phandle.

[Severity: Medium]
Furthermore, since the description states this value is only used in slave
mode, should there be an allOf/if-then block added to enforce this
restriction based on the compatible string?

> +  dwc-i2c-tx-fifo-depth:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      The property describes the tx fifo depth.
> +    default: 8
> +
> +  dwc-i2c-rx-fifo-depth:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      The property describes the rx fifo depth.

[Severity: Medium]
Do these properties use an invalid vendor prefix?

Custom device tree properties must begin with a recognized vendor prefix
followed by a comma. Using dwc-i2c- without a comma violates core device
tree naming constraints. Should these use a registered prefix like snps,
or starfive, instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521034340.27837-1-lianfeng.ouyang@starfivetech.com?part=1

Re: [PATCH v1 2/3] i2c: designware: Export symbols and add __weak for DWC I2C driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Exporting core driver functions using `__weak` allows global overrides, completely breaking generic DesignWare I2C hardware in multi-platform kernels.
- [High] Compile-time `#if IS_ENABLED(CONFIG_I2C_DWC_CORE)` checks unconditionally replace generic hardware logic with StarFive DWC-specific logic.
- [High] The function `i2c_dw_set_fifo_size` acquires the bus lock but fails to release it if `CONFIG_I2C_DWC_CORE` is enabled.
- [High] Return values of probe functions are ignored, and standard adapter initialization is skipped when `CONFIG_I2C_DWC_CORE` is enabled.
- [High] The `adap->quirks` field is completely uninitialized when `CONFIG_I2C_DWC_SLAVE` is enabled.
- [High] Incorrect packing of FIFO depths leads to bitfield overlap and severely corrupted hardware configurations.
- [High] Skipped interrupt mask initialization for DWC I2C slaves opens the system to unhandled interrupt storms.
--

commit 49b74b77ebfe8943f63b3e20e42d019503ae6640
Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
Subject: i2c: designware: Export symbols and add __weak for DWC I2C driver

The upcoming StarFive DWC I2C driver is based on the DesignWare I2C core but
requires its own probe and configuration routines due to register layout
differences. Export several key functions and mark them as __weak to allow
the DWC driver to override the implementations.

> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index 4dc57fd561703..cfeec5d338bba 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
[ ... ]
> @@ -411,7 +416,7 @@ static void i2c_dw_write_timings(struct dw_i2c_dev *dev)
>   *
>   * The controller must be disabled before this function is called.
>   */
> -void i2c_dw_set_mode(struct dw_i2c_dev *dev, int mode)
> +__weak void i2c_dw_set_mode(struct dw_i2c_dev *dev, int mode)

[Severity: High]
Does marking generic driver functions like i2c_dw_set_mode() as __weak risk
breaking generic DesignWare I2C hardware in multi-platform kernels?

If both the standard DesignWare and StarFive drivers are enabled, strong
symbols from the StarFive driver could globally override these functions for
all controllers. Could hardware-specific variants be implemented using an
operations structure with function pointers instead?

[ ... ]
> @@ -806,10 +811,25 @@ static int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
>  	if (ret)
>  		return ret;
>  
> +#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
> +	u32 tx_fifo_cfg = 8, rx_fifo_cfg = 8;
> +
> +#ifdef CONFIG_OF
> +	ret = of_property_read_u32(dev->dev->of_node, "dwc-i2c-tx-fifo-depth", &tx_fifo_cfg);
> +	if (!ret && (tx_fifo_cfg < 2 || tx_fifo_cfg > 256))
> +		tx_fifo_cfg = 8;
> +
> +	ret = of_property_read_u32(dev->dev->of_node, "dwc-i2c-rx-fifo-depth", &rx_fifo_cfg);
> +	if (!ret && (rx_fifo_cfg < 2 || rx_fifo_cfg > 256))
> +		rx_fifo_cfg = 8;
> +#endif
> +	param = rx_fifo_cfg << 8 | tx_fifo_cfg << 16;

[Severity: High]
Can this bitwise packing corrupt the hardware configuration if rx_fifo_cfg is
set to 256?

Since 256 requires 9 bits, shifting rx_fifo_cfg by 8 yields 0x10000, which
sets bit 16 and overflows into the tx_fifo_cfg field. This causes FIELD_GET
to return 0 later on, incorrectly setting the FIFO depths to 1.

> +#else
>  	ret = regmap_read(dev->map, DW_IC_COMP_PARAM_1, &param);
>  	i2c_dw_release_lock(dev);
>  	if (ret)
>  		return ret;
> +#endif

[Severity: High]
Does this code leak the bus lock when CONFIG_I2C_DWC_CORE is enabled?

i2c_dw_acquire_lock(dev) is called earlier in this function, but the matching
i2c_dw_release_lock(dev) call is placed in the #else block. When the DWC core
is enabled, the function returns without releasing the lock, which can cause
a deadlock during the next lock acquisition.

[ ... ]
> @@ -835,7 +855,9 @@ u32 i2c_dw_func(struct i2c_adapter *adap)
>  
>  void i2c_dw_disable(struct dw_i2c_dev *dev)
>  {
> +#if !IS_ENABLED(CONFIG_I2C_DWC_CORE)
>  	unsigned int dummy;
> +#endif
>  	int ret;
>  
>  	ret = i2c_dw_acquire_lock(dev);
> @@ -847,7 +869,12 @@ void i2c_dw_disable(struct dw_i2c_dev *dev)
>  
>  	/* Disable all interrupts */
>  	__i2c_dw_write_intr_mask(dev, 0);
> +
> +#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
> +	regmap_write(dev->map, DWC_IC_INTR_CLR, DWC_CLR_INTR);
> +#else
>  	regmap_read(dev->map, DW_IC_CLR_INTR, &dummy);
> +#endif

[Severity: High]
Will this compile-time check break standard DesignWare I2C controllers in
multi-platform kernels?

By using #if IS_ENABLED(), the mandatory read from DW_IC_CLR_INTR is replaced
globally by a write to DWC_IC_INTR_CLR when the DWC core is compiled in,
causing generic controllers to execute incorrect logic.

[ ... ]
> @@ -896,6 +923,12 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
>  	if (ret)
>  		return ret;
>  
> +#if IS_ENABLED(CONFIG_I2C_DWC_CORE)
> +	if (dev->mode == DW_IC_SLAVE)
> +		i2c_dw_probe_slave(dev);
> +	else
> +		i2c_dw_probe_master(dev);

[Severity: High]
Does this code ignore the return values of the probe functions?

If initialization fails inside i2c_dw_probe_slave() or i2c_dw_probe_master(),
the error is silently ignored and the driver continues probing.

> +#else
>  	ret = i2c_dw_probe_master(dev);
>  	if (ret)
>  		return ret;
> @@ -906,10 +939,16 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
>  
>  	if (!adap->name[0])
>  		strscpy(adap->name, "Synopsys DesignWare I2C adapter");
> +#endif

[Severity: High]
Are we skipping generic initialization steps here?

Because the calls to i2c_dw_init(dev) and initializing adap->name are enclosed
in the #else block, they are entirely skipped when CONFIG_I2C_DWC_CORE is
enabled, which could break standard adapter initialization.

>  	adap->retries = 3;
>  	adap->algo = &i2c_dw_algo;
> +#if IS_ENABLED(CONFIG_I2C_DWC_SLAVE)
> +	if (dev->mode == DW_IC_SLAVE)
> +		adap->algo = &i2c_dw_slave_algo;
> +#else
>  	adap->quirks = &i2c_dw_quirks;
> +#endif

[Severity: High]
Will adap->quirks remain uninitialized when CONFIG_I2C_DWC_SLAVE is enabled?

The assignment to adap->quirks is in the #else block. If CONFIG_I2C_DWC_SLAVE
is enabled, adap->quirks is never initialized for any device mode, including
master. This removes quirks like I2C_AQ_NO_ZERO_LEN, which could allow
invalid zero-length transfers to hang standard controllers indefinitely.

>  	adap->dev.parent = dev->dev;
>  	i2c_set_adapdata(adap, dev);
>  
> @@ -938,16 +977,18 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
>  	if (!dev->emptyfifo_hold_master)
>  		irq_flags |= IRQF_NO_THREAD;
>  
> -	ret = i2c_dw_acquire_lock(dev);
> -	if (ret)
> -		return ret;
> +	if (!IS_ENABLED(CONFIG_I2C_DWC_CORE) || dev->mode == DW_IC_MASTER) {
> +		ret = i2c_dw_acquire_lock(dev);
> +		if (ret)
> +			return ret;
>  
> -	__i2c_dw_write_intr_mask(dev, 0);
> -	i2c_dw_release_lock(dev);
> +		__i2c_dw_write_intr_mask(dev, 0);
> +		i2c_dw_release_lock(dev);
> +	}
>  
>  	if (!(dev->flags & ACCESS_POLLING)) {
>  		ret = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr,

[Severity: High]
Could skipping the interrupt mask clearing for DWC slaves cause unhandled
interrupt storms?

If the unmasked hardware has pending interrupts, devm_request_irq() will
immediately trigger i2c_dw_isr_slave(). Because dev->slave is not yet
registered during probe, the ISR will return IRQ_NONE without clearing the
hardware interrupt, leading to a persistent storm.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521034340.27837-1-lianfeng.ouyang@starfivetech.com?part=2

Re: [PATCH v1 0/3] i2c: dwc: Add I2C DWC master/slave support for StarFive JHB100

[Mika Westerberg]

Hi,

On Thu, May 21, 2026 at 11:43:37AM +0800, lianfeng.ouyang wrote:
> From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> 
> The Synopsys DesignWare Core (DWC) I2C controller is a variant of the
> widely-used DesignWare I2C IP, with a distinct register layout and
> enhanced features such as SMBus Alert and programmable FIFO depths.

I wonder why they did shuffle the registers... :(

> This patch series introduces support for this controller as implemented
> on the StarFive JHB100 platform, which utilizes it for both master and
> slave operations (e.g., for MCTP over I2C).
> 
> The series is structured as follows:
> 1.  Adds the device tree binding document for the snps,dwc-i2c compatible.
> 2.  Prepares the existing i2c-designware-core by exporting and making
>     certain key functions overridable, allowing code reuse.
> 3.  Introduces the new i2c-dwc-core driver, with separate modules for
>     master and slave functionality, based on the 2023-07 revision of the
>     Synopsys IP manual.
> 
> Key differences from the Existing i2c-designware Driver
> 1.  The DWC IP's offsets for all key registers are redefined. The driver
>     maps to the correct addresses by overriding macros from the core
>     header file in a new header (i2c-dwc-core.h).

Instead of this, can you provide a regmap that internally maps to these
shuffled registers?

> 2.  The host and slave of DWC IP need to perform probe callbacks
>     separately, so they cannot be directly set through i2c_dew_set_mode
> 3.  Interrupts are cleared by writing​ to the corresponding bits in the
>     INTR_CLRregister (write-1-to-clear).
> 4.  The DWC controller's IC_ENABLEregister contains an additional
>     TX_CMD_BLOCKcontrol bit. When enabling the controller, the driver must
>     ensure this bit is cleared. When disabling, only the ENABLEbit is
>     cleared, preserving other configurations.
> 
> Lianfeng Ouyang (3):
>   dt-bindings: i2c: snps,dwc-i2c: Add StarFive JHB100 bindings
>   i2c: designware: Export symbols and add __weak for DWC I2C driver
>   i2c: dwc: Add StarFive JHB100 I2C master/slave support
> 
>  .../devicetree/bindings/i2c/snps,dwc-i2c.yaml | 120 +++++
>  MAINTAINERS                                   |   7 +
>  drivers/i2c/busses/Kconfig                    |  34 ++
>  drivers/i2c/busses/Makefile                   |   3 +
>  drivers/i2c/busses/i2c-designware-common.c    |  57 ++-
>  drivers/i2c/busses/i2c-designware-core.h      |  25 +
>  drivers/i2c/busses/i2c-designware-master.c    |  14 +-
>  drivers/i2c/busses/i2c-designware-platdrv.c   |   6 +
>  drivers/i2c/busses/i2c-designware-slave.c     |   4 +-
>  drivers/i2c/busses/i2c-dwc-core.h             | 192 ++++++++
>  drivers/i2c/busses/i2c-dwc-master.c           | 441 ++++++++++++++++++
>  drivers/i2c/busses/i2c-dwc-slave.c            | 180 +++++++

Also the naming is confusing so if you need any glue code I recommend
calling it i2c-starfive-* instead.

>  12 files changed, 1068 insertions(+), 15 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
>  create mode 100644 drivers/i2c/busses/i2c-dwc-core.h
>  create mode 100644 drivers/i2c/busses/i2c-dwc-master.c
>  create mode 100644 drivers/i2c/busses/i2c-dwc-slave.c
> 
> --
> 2.43.0

Re: [PATCH v1 3/3] i2c: dwc: Add StarFive JHB100 I2C master/slave support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] Global symbol namespace collisions and overriding of core driver functions break the generic DesignWare I2C driver.
- [High] Device tree references are leaked and a dangling pointer to the master adapter creates a Use-After-Free risk.
- [High] Unsigned arithmetic underflow in the SCL High Count (HCNT) calculation corrupts hardware timing.
- [High] Hardware timing registers are programmed locklessly, out of sequence, and lost on resume.
- [High] Unconditional read of the SMBus interrupt register on potentially unsupported hardware.
- [Medium] Incorrect register read invalidates the hardware High Speed capability check.
- [Medium] Slave mode initialization clobbers the interrupt state machine.
- [Medium] Mismatch between bus frequency fallback flag and actual timing values.
- [Low] Missing inclusion of 'i2c-dwc-core.h' causes compilation to fail due to undeclared identifiers.
- [Low] Missing EXPORT_SYMBOL for i2c_dw_slave_algo creates cross-module linking failures.
--

commit 30ef7c6d2faa5af3ac90a02dd939e51c945c393c
Author: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>

i2c: dwc: Add StarFive JHB100 I2C master/slave support

Add support for the Synopsys DesignWare Core (DWC) I2C controller
found on the StarFive JHB100 platform. The controller supports both
master and slave modes and uses a different register layout compared
to the standard DesignWare I2C IP.

> diff --git a/drivers/i2c/busses/i2c-dwc-master.c b/drivers/i2c/busses/i2c-dwc-master.c
> new file mode 100644
> index 0000000000000..6e8d1828f99e6
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-dwc-master.c
> @@ -0,0 +1,441 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Synopsys DWC I2C adapter driver (master only).
> + *
> + * Based on the TI DAVINCI I2C adapter driver.
> + *
> + * Copyright (C) 2006 Texas Instruments.
> + * Copyright (C) 2007 MontaVista Software Inc.
> + * Copyright (C) 2009 Provigent Ltd.
> + */
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/export.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/units.h>
> +
> +#include "i2c-designware-core.h"

[Severity: Low]
Is there a missing include for "i2c-dwc-core.h" here? It looks like macros
like DWC_IC_CTRL are used in this file but not declared.

[ ... ]

> +/**
> + * i2c_dwc_scl_hcnt() -  Calculate SCL HCNT
> + * @ic_clk: Input clock in kHz
> + * @thigh: Duration in ns of logic 1 to generate
> + * @tr: SCL rise time in ns
> + * @spk_cnt: Spike count
> + */
> +static u32 i2c_dwc_scl_hcnt(u32 ic_clk, u32 thigh, u32 tr, u32 spk_cnt)
> +{
> +	u64 min_thigh_cnt, rise_cnt;
> +
> +	/* Formula: cnt = f_kHz * t_ns * 10^(-6) */
> +	min_thigh_cnt = DIV_ROUND_CLOSEST_ULL((u64)ic_clk * thigh, MICRO);
> +	rise_cnt = DIV_ROUND_CLOSEST_ULL((u64)ic_clk * tr, MICRO);
> +
> +	return max(5, min_thigh_cnt + rise_cnt - spk_cnt - 3);

[Severity: High]
Could the subtraction here cause an unsigned arithmetic underflow?

If the input clock frequency is low and the controller is in High Speed mode,
spk_cnt + 3 could exceed min_thigh_cnt + rise_cnt, which would result in a
huge positive value being written to the hardware register.

[ ... ]

> +static int i2c_dwc_set_timings_master(struct dw_i2c_dev *dev)
> +{
> +	u32 scl_falling_time = 0, scl_rising_time = 0;
> +	u32 scl_high_time = 0, scl_low_time = 0;
> +	struct i2c_timings *t = &dev->timings;
> +	unsigned int comp_param1;
> +	u32 ic_clk, spk_cnt;
> +	int ret;
> +
> +	ret = i2c_dw_acquire_lock(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(dev->map, DWC_IC_CTRL, &comp_param1);

[Severity: Medium]
Should this register read be from DW_IC_COMP_PARAM_1 instead of DWC_IC_CTRL?

The later code checks for DWC_IC_COMP_PARAM_1_SPEED_MODE_HIGH in
comp_param1, but DWC_IC_CTRL is at offset 0x24 while DW_IC_COMP_PARAM_1
is at 0xf4.

> +	i2c_dw_release_lock(dev);
> +	if (ret)
> +		return ret;
> +
> +	ic_clk = i2c_dw_clk_rate(dev);
> +
> +	/* 50ns maximum spike */
> +	spk_cnt = DIV_ROUND_CLOSEST_ULL((u64)ic_clk * 50, MICRO);
> +
> +	regmap_write(dev->map, DWC_IC_HS_SPKLEN, spk_cnt);
> +	regmap_write(dev->map, DWC_IC_SPKLEN, spk_cnt);

[Severity: High]
Are these registers being programmed locklessly and out of sequence?

The bus lock was released above, and this is happening before the controller
is disabled by __i2c_dw_disable(). Also, because this configuration is done
during probe, will the SPKLEN settings be lost upon resuming from suspend?

[ ... ]

> +	/* Check is high speed possible and fall back to fast mode if not */
> +	if ((dev->master_cfg & DWC_IC_CTRL_SPEED_MASK) == DWC_IC_CTRL_SPEED_HIGH) {
> +		if ((comp_param1 & DWC_IC_COMP_PARAM_1_SPEED_MODE_MASK)
> +			!= DWC_IC_COMP_PARAM_1_SPEED_MODE_HIGH) {
> +			dev_err(dev->dev, "High Speed not supported!\n");
> +			t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ;
> +			dev->master_cfg &= ~DWC_IC_CTRL_SPEED_MASK;
> +			dev->master_cfg |= DWC_IC_CTRL_SPEED_FAST;
> +			dev->hs_hcnt = 0;
> +			dev->hs_lcnt = 0;
> +
> +			/* Replace with I2C_MAX_FAST_MODE_PLUS_FREQ */
> +			scl_rising_time = max(scl_rising_time, 120);
> +			scl_falling_time = max(scl_falling_time, 120);
> +			scl_high_time = 260;	/* tHIGH_min = 260 ns */
> +			scl_low_time = 500;	/* tLOW_min = 500 ns */

[Severity: Medium]
Is there a mismatch here between the fallback bus frequency and the timing
values?

The bus frequency is set to I2C_MAX_FAST_MODE_FREQ (400kHz), but the
hardcoded timing values correspond to I2C_MAX_FAST_MODE_PLUS_FREQ (1MHz).

[ ... ]

> +int i2c_dw_init(struct dw_i2c_dev *dev)

[Severity: High]
Does defining a strong global function i2c_dw_init() here break the generic
DesignWare I2C driver?

If CONFIG_I2C_DWC_CORE is enabled, all standard DesignWare controllers might
execute this DWC-specific logic instead of their own, accessing incorrect
register offsets.

> +{
> +	int ret;

[ ... ]

> +void i2c_dw_configure_master(struct dw_i2c_dev *dev)

[Severity: High]
Like i2c_dw_init() above, will defining i2c_dw_configure_master() here also
override the generic DesignWare implementation and break other generic hardware?

> +{
> +	struct i2c_timings *t = &dev->timings;

[ ... ]

> +int i2c_dw_probe_master(struct dw_i2c_dev *dev)

[Severity: High]
Will defining i2c_dw_probe_master() here cause namespace collisions and override
the generic i2c-designware-core driver implementation?

> +{
> +	struct i2c_adapter *adap = &dev->adapter;
> +	unsigned int ic_ctrl;
> +	int ret;

[ ... ]

> diff --git a/drivers/i2c/busses/i2c-dwc-slave.c b/drivers/i2c/busses/i2c-dwc-slave.c
> new file mode 100644
> index 0000000000000..5369eb8812382
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-dwc-slave.c
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Synopsys DWC I2C adapter driver (slave only).
> + *
> + * Based on the Synopsys DWC I2C adapter driver (master).
> + *
> + * Copyright (C) 2016 Synopsys Inc.
> + */
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +
> +#include "i2c-designware-core.h"

[Severity: Low]
Should "i2c-dwc-core.h" be included here to prevent build failures from
undeclared identifiers like DWC_IC_DAR?

> +
> +void i2c_dw_set_mode(struct dw_i2c_dev *dev, int mode)
> +{
> +	if (dev->mode == DW_IC_SLAVE && mode == DW_IC_SLAVE) {
> +		regmap_write(dev->map, DWC_IC_DAR, dev->slave->addr);
> +		__i2c_dw_enable(dev);
> +		dev->status = 0;

[Severity: Medium]
Could clearing dev->status unconditionally here clobber the interrupt state
machine?

Because the hardware is active and interrupts are already unmasked, an
interrupt might fire and update dev->status concurrently.

> +	}
> +}

[ ... ]

> +u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
> +{
> +	unsigned int stat;
> +	unsigned int smbus_stat;
> +
> +	/*
> +	 * The IC_INTR_STAT register just indicates "enabled" interrupts.
> +	 * The unmasked raw version of interrupt status bits is available
> +	 * in the IC_RAW_INTR_STAT register.
> +	 *
> +	 * That is,
> +	 *   stat = readl(IC_INTR_STAT);
> +	 * equals to,
> +	 *   stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
> +	 *
> +	 * The raw version might be useful for debugging purposes.
> +	 */
> +	regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
> +	regmap_read(dev->map, DWC_IC_SMBUS_INTR_STAT, &smbus_stat);

[Severity: High]
Is it safe to unconditionally read DWC_IC_SMBUS_INTR_STAT here?

The SMBus block is optional, and accessing unmapped or disabled IP block
registers might cause imprecise external aborts or bus faults on some
architectures. Should this check DWC_IC_CAPABILITIES_IC_SMBUS first?

[ ... ]

> +const struct i2c_algorithm i2c_dw_slave_algo = {
> +	.master_xfer = i2c_dwc_xfer_ms,
> +	.functionality = i2c_dw_func,
> +	.reg_slave = i2c_dw_reg_slave,
> +	.unreg_slave = i2c_dw_unreg_slave,
> +};

[Severity: Low]
Does i2c_dw_slave_algo need an EXPORT_SYMBOL_GPL()?

It is referenced by i2c-designware-common.c, which could cause a cross-module
linking failure if the drivers are built as separate modules.

[ ... ]

> +int i2c_dw_probe_slave(struct dw_i2c_dev *dev)

[Severity: High]
Similar to the master driver, will defining a strong i2c_dw_probe_slave()
function here override the generic DesignWare implementation and break other
I2C controllers?

> +{
> +	struct i2c_adapter *adap = &dev->adapter;
> +	struct device_node *ms_node;
> +	int ret;
> +
> +	ret = i2c_dwc_init_slave(dev);
> +	if (ret)
> +		return ret;
> +
> +	snprintf(adap->name, sizeof(adap->name), "Synopsys DWC I2C Slave adapter");
> +
> +	/* Get the specified I2C master adapter from DTS for MCTP */
> +	ms_node = of_parse_phandle(dev->dev->of_node, "starfive,mctp-i2c-ms", 0);
> +	if (ms_node) {
> +		struct platform_device *ms_pdev = of_find_device_by_node(ms_node);
> +
> +		if (ms_pdev) {
> +			struct dw_i2c_dev *ms_dev = platform_get_drvdata(ms_pdev);
> +
> +			if (ms_dev)
> +				dev->ms_adapter = &ms_dev->adapter;

[Severity: High]
Are we leaking device tree references and risking a Use-After-Free here?

of_parse_phandle() and of_find_device_by_node() increment reference counts,
but of_node_put() and put_device() are never called. Also, a raw pointer to
the master adapter is stored in dev->ms_adapter without taking a proper
reference. If the master device is unbound, dev->ms_adapter could become a
dangling pointer.

> +		}
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(i2c_dw_probe_slave);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521034340.27837-1-lianfeng.ouyang@starfivetech.com?part=3

回复: [PATCH v1 0/3] i2c: dwc: Add I2C DWC master/slave support for StarFive JHB100

[Lianfeng Ouyang]

Hi, Mika

Thank for the comments

> -----邮件原件-----
> 发件人: Mika Westerberg <mika.westerberg@linux.intel.com>
> 发送时间: 2026年5月21日 12:55
> 收件人: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> 抄送: Andi Shyti <andi.shyti@kernel.org>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>; Jan Dabros <jsd@semihalf.com>;
> linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 主题: Re: [PATCH v1 0/3] i2c: dwc: Add I2C DWC master/slave support for
> StarFive JHB100
> 
> Hi,
> 
> On Thu, May 21, 2026 at 11:43:37AM +0800, lianfeng.ouyang wrote:
> > From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> >
> > The Synopsys DesignWare Core (DWC) I2C controller is a variant of the
> > widely-used DesignWare I2C IP, with a distinct register layout and
> > enhanced features such as SMBus Alert and programmable FIFO depths.
> 
> I wonder why they did shuffle the registers... :(

Let's first discuss using i2c StarFive

the colleague who was responsible for the i2c starfive driver earlier cannot be reached, 
so I compared the databook of i2c designware and i2c starfive, and it seems that the versions are different
The homepage information of the two databooks is as follows

i2c designware:
DesignWare DW_apb_i2c Databook
1.16a
October 2011

i2c starfive:
DesignWare  ®  Cores Advanced I2C/SMBus Controller and Target Device Databook
Version 1.01a-lca00
July 2023

> 
> > This patch series introduces support for this controller as implemented
> > on the StarFive JHB100 platform, which utilizes it for both master and
> > slave operations (e.g., for MCTP over I2C).
> >
> > The series is structured as follows:
> > 1.  Adds the device tree binding document for the snps,dwc-i2c compatible.
> > 2.  Prepares the existing i2c-designware-core by exporting and making
> >     certain key functions overridable, allowing code reuse.
> > 3.  Introduces the new i2c-dwc-core driver, with separate modules for
> >     master and slave functionality, based on the 2023-07 revision of the
> >     Synopsys IP manual.
> >
> > Key differences from the Existing i2c-designware Driver
> > 1.  The DWC IP's offsets for all key registers are redefined. The driver
> >     maps to the correct addresses by overriding macros from the core
> >     header file in a new header (i2c-dwc-core.h).
> 
> Instead of this, can you provide a regmap that internally maps to these
> shuffled registers?

It seems that regmap cannot solve this difference completely, because
1. The i2c designware register and i2c starfive register are not offset by the same amount, and even have different orders, for example
	offset		I2c designware 	i2c starfive
DW_IC_DATA_CMD 	0x10 				0x78
DW_IC_ENABLE   		0x6c 				0x4
......

2. I2c starfive has some registers with new bit definitions, which i2c designware does not have,
resulting in the inability to directly use i2c designware functions when accessing these registers, such as
DW_IC_ENABLE, i2c designware only defines bit0, while i2c starfive defines bit0~bit19
......

Difference point 1 should be solved by defining a register conversion table from i2c designware to i2c starfive, 
and then using this conversion table for reg_read and regw_write callback. 
However, difference point 2 seems to be solved only by overwriting weak functions?

> 
> > 2.  The host and slave of DWC IP need to perform probe callbacks
> >     separately, so they cannot be directly set through i2c_dew_set_mode
> > 3.  Interrupts are cleared by writing​ to the corresponding bits in the
> >     INTR_CLRregister (write-1-to-clear).
> > 4.  The DWC controller's IC_ENABLEregister contains an additional
> >     TX_CMD_BLOCKcontrol bit. When enabling the controller, the driver
> must
> >     ensure this bit is cleared. When disabling, only the ENABLEbit is
> >     cleared, preserving other configurations.
> >
> > Lianfeng Ouyang (3):
> >   dt-bindings: i2c: snps,dwc-i2c: Add StarFive JHB100 bindings
> >   i2c: designware: Export symbols and add __weak for DWC I2C driver
> >   i2c: dwc: Add StarFive JHB100 I2C master/slave support
> >
> >  .../devicetree/bindings/i2c/snps,dwc-i2c.yaml | 120 +++++
> >  MAINTAINERS                                   |   7 +
> >  drivers/i2c/busses/Kconfig                    |  34 ++
> >  drivers/i2c/busses/Makefile                   |   3 +
> >  drivers/i2c/busses/i2c-designware-common.c    |  57 ++-
> >  drivers/i2c/busses/i2c-designware-core.h      |  25 +
> >  drivers/i2c/busses/i2c-designware-master.c    |  14 +-
> >  drivers/i2c/busses/i2c-designware-platdrv.c   |   6 +
> >  drivers/i2c/busses/i2c-designware-slave.c     |   4 +-
> >  drivers/i2c/busses/i2c-dwc-core.h             | 192 ++++++++
> >  drivers/i2c/busses/i2c-dwc-master.c           | 441
> ++++++++++++++++++
> >  drivers/i2c/busses/i2c-dwc-slave.c            | 180 +++++++
> 
> Also the naming is confusing so if you need any glue code I recommend
> calling it i2c-starfive-* instead.

Considering that the IP was designed by Synopsys instead of StarFive, i2c DWC was used. 
If there are any requirements, I will change it to i2c StarFive in the next version

> 
> >  12 files changed, 1068 insertions(+), 15 deletions(-)
> >  create mode 100644
> Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
> >  create mode 100644 drivers/i2c/busses/i2c-dwc-core.h
> >  create mode 100644 drivers/i2c/busses/i2c-dwc-master.c
> >  create mode 100644 drivers/i2c/busses/i2c-dwc-slave.c
> >
> > --
> > 2.43.0

Re: 回复: [PATCH v1 0/3] i2c: dwc: Add I2C DWC master/slave support for StarFive JHB100

[Mika Westerberg]

Hi,

On Thu, May 21, 2026 at 07:01:08AM +0000, Lianfeng Ouyang wrote:
> 
> Hi, Mika
> 
> Thank for the comments
> 
> > -----邮件原件-----
> > 发件人: Mika Westerberg <mika.westerberg@linux.intel.com>
> > 发送时间: 2026年5月21日 12:55
> > 收件人: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> > 抄送: Andi Shyti <andi.shyti@kernel.org>; Rob Herring <robh@kernel.org>;
> > Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> > <conor+dt@kernel.org>; Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com>; Jan Dabros <jsd@semihalf.com>;
> > linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> > linux-kernel@vger.kernel.org
> > 主题: Re: [PATCH v1 0/3] i2c: dwc: Add I2C DWC master/slave support for
> > StarFive JHB100
> > 
> > Hi,
> > 
> > On Thu, May 21, 2026 at 11:43:37AM +0800, lianfeng.ouyang wrote:
> > > From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> > >
> > > The Synopsys DesignWare Core (DWC) I2C controller is a variant of the
> > > widely-used DesignWare I2C IP, with a distinct register layout and
> > > enhanced features such as SMBus Alert and programmable FIFO depths.
> > 
> > I wonder why they did shuffle the registers... :(
> 
> Let's first discuss using i2c StarFive
> 
> the colleague who was responsible for the i2c starfive driver earlier cannot be reached, 
> so I compared the databook of i2c designware and i2c starfive, and it seems that the versions are different
> The homepage information of the two databooks is as follows
> 
> i2c designware:
> DesignWare DW_apb_i2c Databook
> 1.16a
> October 2011

This has evolved. My databook is 2.03a which also includes additional
features such as SMBus alert and the like. I suggest at least to get a
recent version from Synopsys.

> i2c starfive:
> DesignWare  ®  Cores Advanced I2C/SMBus Controller and Target Device Databook
> Version 1.01a-lca00
> July 2023
> 
> > 
> > > This patch series introduces support for this controller as implemented
> > > on the StarFive JHB100 platform, which utilizes it for both master and
> > > slave operations (e.g., for MCTP over I2C).
> > >
> > > The series is structured as follows:
> > > 1.  Adds the device tree binding document for the snps,dwc-i2c compatible.
> > > 2.  Prepares the existing i2c-designware-core by exporting and making
> > >     certain key functions overridable, allowing code reuse.
> > > 3.  Introduces the new i2c-dwc-core driver, with separate modules for
> > >     master and slave functionality, based on the 2023-07 revision of the
> > >     Synopsys IP manual.
> > >
> > > Key differences from the Existing i2c-designware Driver
> > > 1.  The DWC IP's offsets for all key registers are redefined. The driver
> > >     maps to the correct addresses by overriding macros from the core
> > >     header file in a new header (i2c-dwc-core.h).
> > 
> > Instead of this, can you provide a regmap that internally maps to these
> > shuffled registers?
> 
> It seems that regmap cannot solve this difference completely, because
> 1. The i2c designware register and i2c starfive register are not offset by the same amount, and even have different orders, for example
> 	offset		I2c designware 	i2c starfive
> DW_IC_DATA_CMD 	0x10 				0x78
> DW_IC_ENABLE   		0x6c 				0x4

This is fine, internally you should be able to map DW_IC_ENABLE to the
correct offset.

Of course if the content is different then this may not work.

> ......
> 
> 2. I2c starfive has some registers with new bit definitions, which i2c designware does not have,
> resulting in the inability to directly use i2c designware functions when accessing these registers, such as
> DW_IC_ENABLE, i2c designware only defines bit0, while i2c starfive defines bit0~bit19

My databook defines DW_IC_ENABLE (offset 0x6c) bits from 0 to 22.

> ......
> 
> Difference point 1 should be solved by defining a register conversion table from i2c designware to i2c starfive, 
> and then using this conversion table for reg_read and regw_write callback. 
> However, difference point 2 seems to be solved only by overwriting weak functions?

We have regmap for that so if possible at all that should be used instead.

> > 
> > > 2.  The host and slave of DWC IP need to perform probe callbacks
> > >     separately, so they cannot be directly set through i2c_dew_set_mode
> > > 3.  Interrupts are cleared by writing​ to the corresponding bits in the
> > >     INTR_CLRregister (write-1-to-clear).
> > > 4.  The DWC controller's IC_ENABLEregister contains an additional
> > >     TX_CMD_BLOCKcontrol bit. When enabling the controller, the driver
> > must
> > >     ensure this bit is cleared. When disabling, only the ENABLEbit is
> > >     cleared, preserving other configurations.
> > >
> > > Lianfeng Ouyang (3):
> > >   dt-bindings: i2c: snps,dwc-i2c: Add StarFive JHB100 bindings
> > >   i2c: designware: Export symbols and add __weak for DWC I2C driver
> > >   i2c: dwc: Add StarFive JHB100 I2C master/slave support
> > >
> > >  .../devicetree/bindings/i2c/snps,dwc-i2c.yaml | 120 +++++
> > >  MAINTAINERS                                   |   7 +
> > >  drivers/i2c/busses/Kconfig                    |  34 ++
> > >  drivers/i2c/busses/Makefile                   |   3 +
> > >  drivers/i2c/busses/i2c-designware-common.c    |  57 ++-
> > >  drivers/i2c/busses/i2c-designware-core.h      |  25 +
> > >  drivers/i2c/busses/i2c-designware-master.c    |  14 +-
> > >  drivers/i2c/busses/i2c-designware-platdrv.c   |   6 +
> > >  drivers/i2c/busses/i2c-designware-slave.c     |   4 +-
> > >  drivers/i2c/busses/i2c-dwc-core.h             | 192 ++++++++
> > >  drivers/i2c/busses/i2c-dwc-master.c           | 441
> > ++++++++++++++++++
> > >  drivers/i2c/busses/i2c-dwc-slave.c            | 180 +++++++
> > 
> > Also the naming is confusing so if you need any glue code I recommend
> > calling it i2c-starfive-* instead.
> 
> Considering that the IP was designed by Synopsys instead of StarFive, i2c DWC was used. 
> If there are any requirements, I will change it to i2c StarFive in the next version

Yes but i2c-dwc- is confusing to say the least. If StarFive is the first
one to use this IMHO this can be called i2c-starfive-platdrv.c which then
provides that regmap and calls into i2c-designwware- library where needed.

I don't have strong opinnion about this though, just my 2c.

> > 
> > >  12 files changed, 1068 insertions(+), 15 deletions(-)
> > >  create mode 100644
> > Documentation/devicetree/bindings/i2c/snps,dwc-i2c.yaml
> > >  create mode 100644 drivers/i2c/busses/i2c-dwc-core.h
> > >  create mode 100644 drivers/i2c/busses/i2c-dwc-master.c
> > >  create mode 100644 drivers/i2c/busses/i2c-dwc-slave.c
> > >
> > > --
> > > 2.43.0