[PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[lianfeng.ouyang]

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

The Starfive JHB100 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.

The series is structured as follows:
1.  Adds the device tree binding document for the starfive,jhb100-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-starfive-* driver, with separate modules for
        master and slave functionality, based on the 2023-07 revision of
	the Synopsys IP manual.

Currently, due to the following differences, i2c designware cannot be
fully reused
1.  For high and low level counting settings at different rates, i2c
        starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
2.  Interrupt clearing is achieved by writing 1 to the corresponding
        bit of INTR_CLR, while designware reads different clearing
	registers
3.  Master and slave require separate probe callbacks and cannot rely
        solely on the runtime mode switching provided by i2c_dw_set_mode()
4.  The value of FIFO depth is not obtained through registers, but
        written through DTS

I have written some poorly styled code to reduce changes to i2c designware
and reuse its functions by keeping aa always true, for example
1.  the implementation of i2c-d w_probe_master() differs only for the two
        IPs in i2c_dw_set_timits_master(). In order to reuse
	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared as
	__weak. A better approach is to use a callback function, but using
	a callback function requires changing more i2c designware files.
	I don't know what the attitude of the community is
2.  For the operation of clearing interrupt flags, i2c designware reads
        and i2c starfive writes. Therefore, in order not to modify the
	relevant logic of i2c designware, I added a write operation to
	sf_reg_read()
So I think this version of the code is not allowed to merge, but I don't
know how to handle this situation because if i2c designware is not changed
 at all, we will have to write code that is similar to i2c designware.
 Will this type of IP not be allowed to merge?

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

 .../bindings/i2c/starfive,jhb100-i2c.yaml     | 128 +++++
 MAINTAINERS                                   |   7 +
 drivers/i2c/busses/Kconfig                    |  24 +
 drivers/i2c/busses/Makefile                   |   3 +
 drivers/i2c/busses/i2c-designware-common.c    |  72 ++-
 drivers/i2c/busses/i2c-designware-core.h      |  20 +
 drivers/i2c/busses/i2c-designware-master.c    |   4 +-
 drivers/i2c/busses/i2c-designware-platdrv.c   |   6 +
 drivers/i2c/busses/i2c-designware-slave.c     |   2 +-
 drivers/i2c/busses/i2c-starfive-core.h        | 164 ++++++
 drivers/i2c/busses/i2c-starfive-master.c      | 470 ++++++++++++++++++
 drivers/i2c/busses/i2c-starfive-slave.c       | 180 +++++++
 12 files changed, 1060 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
 create mode 100644 drivers/i2c/busses/i2c-starfive-core.h
 create mode 100644 drivers/i2c/busses/i2c-starfive-master.c
 create mode 100644 drivers/i2c/busses/i2c-starfive-slave.c

--
2.43.0

[PATCH v2 1/3] dt-bindings: i2c: Add StarFive JHB100 I2C

[lianfeng.ouyang]

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

Add device tree bindings for the Starfive I2C controller
and its implementation

The binding defines two platform-specific compatibles for the StarFive
JHB100 implementation:
- "starfive,jhb100-i2c-master"
- "starfive,jhb100-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>
---
 .../bindings/i2c/starfive,jhb100-i2c.yaml     | 128 ++++++++++++++++++
 1 file changed, 128 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml

diff --git a/Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml b/Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
new file mode 100644
index 000000000000..c8631348121c
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
@@ -0,0 +1,128 @@
+# 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/starfive,jhb100-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive jhb100 I2C Controller
+
+maintainers:
+  - Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
+
+allOf:
+  - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+  compatible:
+    description: |
+      Must be one of:
+      - "starfive,jhb100-i2c-master" for master mode controller
+      - "starfive,jhb100-i2c-slave" for slave mode controller
+    enum:
+      - starfive,jhb100-i2c-master
+      - starfive,jhb100-i2c-slave
+
+  reg:
+    maxItems: 1
+    description: StarFive I2C controller memory mapped registers
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 2
+    maxItems: 2
+    items:
+      - description: I2C controller reference clock source
+      - description: APB interface clock source
+
+  clock-names:
+    minItems: 2
+    maxItems: 2
+    items:
+      - const: ref
+      - const: pclk
+
+  resets:
+    maxItems: 1
+    description: Phandle to the reset controller for the I2C controller
+
+  clock-frequency:
+    description: Desired I2C bus clock frequency in Hz
+    enum: [100000, 400000, 1000000, 3400000]
+    default: 400000
+
+  i2c-sda-hold-time-ns:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      SDA hold time in nanoseconds, used to compute the value written into
+      the DW_IC_SDA_HOLD register.
+    default: 0
+
+  i2c-scl-falling-time-ns:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      SCL falling time in nanoseconds, used to compute the tLOW period.
+    default: 300
+
+  i2c-sda-falling-time-ns:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      SDA falling time in nanoseconds, used to compute the tHIGH period.
+    default: 300
+
+  starfive,mctp-i2c-ms:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      Phandle to the master I2C node associated with this slave controller.
+      Only used in slave mode, typically for MCTP applications.
+
+  starfive,i2c-tx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      TX FIFO depth of the I2C controller.
+    default: 8
+
+  starfive,i2c-rx-fifo-depth:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      RX FIFO depth of the I2C controller.
+    default: 8
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+
+examples:
+  - |
+    i2c@2000 {
+      compatible = "starfive,jhb100-i2c-master";
+      reg = <0x2000 0x100>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      clock-frequency = <400000>;
+      clocks = <&i2c_refclk>, <&apb_pclk>;
+      clock-names = "ref", "pclk";
+      interrupts = <0>;
+
+      eeprom@64 {
+        compatible = "atmel,24c02";
+        reg = <0x64>;
+      };
+    };
+    |
+    i2c@3000 {
+      compatible = "starfive,jhb100-i2c-slave";
+      reg = <0x3000 0x100>;
+      clocks = <&i2c_refclk>, <&apb_pclk>;
+      clock-names = "ref", "pclk";
+      interrupts = <1>;
+      starfive,mctp-i2c-ms = <&i2c_master>;
+    };
+...
-- 
2.43.0

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

[lianfeng.ouyang]

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

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 i2c starfive driver to reuse the common
infrastructure while overriding the implementations where needed.

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

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

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 4dc57fd56170..034f6a446ab6 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -70,6 +70,7 @@ static const char *const abort_sources[] = {
 		"incorrect slave-transmitter mode configuration",
 };
 
+#if !IS_ENABLED(CONFIG_I2C_STARFIVE)
 static int dw_reg_read(void *context, unsigned int reg, unsigned int *val)
 {
 	struct dw_i2c_dev *dev = context;
@@ -87,6 +88,7 @@ static int dw_reg_write(void *context, unsigned int reg, unsigned int val)
 
 	return 0;
 }
+#endif
 
 static int dw_reg_read_swab(void *context, unsigned int reg, unsigned int *val)
 {
@@ -143,9 +145,15 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
 		.val_bits = 32,
 		.reg_stride = 4,
 		.disable_locking = true,
+#if IS_ENABLED(CONFIG_I2C_STARFIVE)
+		.reg_read = sf_reg_read,
+		.reg_write = sf_reg_write,
+		.max_register = SF_IC_SMBUS_INTR_CLR,
+#else
 		.reg_read = dw_reg_read,
 		.reg_write = dw_reg_write,
 		.max_register = DW_IC_COMP_TYPE,
+#endif
 	};
 	u32 reg;
 	int ret;
@@ -162,6 +170,10 @@ static int i2c_dw_init_regmap(struct dw_i2c_dev *dev)
 		return ret;
 
 	reg = readl(dev->base + DW_IC_COMP_TYPE);
+
+	if ((dev->flags & MODEL_MASK) == MODEL_STARFIVE)
+		reg = readl(dev->base + (SF_IC_COMP_TYPE & (~SF_REG_FLAG)));
+
 	i2c_dw_release_lock(dev);
 
 	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
@@ -359,7 +371,7 @@ static inline u32 i2c_dw_acpi_round_bus_speed(struct device *device) { return 0;
 
 #endif	/* CONFIG_ACPI */
 
-static void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
+__weak void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
 {
 	switch (mode) {
 	case DW_IC_MASTER:
@@ -382,7 +394,7 @@ static void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
 	}
 }
 
-static void i2c_dw_write_timings(struct dw_i2c_dev *dev)
+__weak void i2c_dw_write_timings(struct dw_i2c_dev *dev)
 {
 	/* Write standard speed timing parameters */
 	regmap_write(dev->map, DW_IC_SS_SCL_HCNT, dev->ss_hcnt);
@@ -411,7 +423,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;
@@ -806,10 +818,25 @@ static int i2c_dw_set_fifo_size(struct dw_i2c_dev *dev)
 	if (ret)
 		return ret;
 
+#if IS_ENABLED(CONFIG_I2C_STARFIVE)
+	u32 tx_fifo_cfg = 8, rx_fifo_cfg = 8;
+
+#ifdef CONFIG_OF
+	ret = of_property_read_u32(dev->dev->of_node, "starfive,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, "starfive,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;
@@ -896,20 +923,29 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
 	if (ret)
 		return ret;
 
-	ret = i2c_dw_probe_master(dev);
-	if (ret)
-		return ret;
+	if (dev->mode == DW_IC_SLAVE) {
+		ret = i2c_sf_probe_slave(dev);
+	} else {
+		ret = i2c_dw_probe_master(dev);
+		if (ret)
+			return ret;
 
-	ret = i2c_dw_init(dev);
-	if (ret)
-		return ret;
+		ret = i2c_dw_init(dev);
+		if (ret)
+			return ret;
 
-	if (!adap->name[0])
-		strscpy(adap->name, "Synopsys DesignWare I2C adapter");
+		if (!adap->name[0])
+			strscpy(adap->name, "Synopsys DesignWare I2C adapter");
+	}
 
 	adap->retries = 3;
 	adap->algo = &i2c_dw_algo;
+#if IS_ENABLED(CONFIG_I2C_STARFIVE_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 +974,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_STARFIVE) || 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..87223f7e28aa 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_STARFIVE)
+	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,14 @@ 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);
 
+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_STARFIVE)
+#include "i2c-starfive-core.h"
+#else
 static inline void __i2c_dw_enable(struct dw_i2c_dev *dev)
 {
 	dev->status |= STATUS_ACTIVE;
@@ -372,6 +384,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)
@@ -411,8 +424,15 @@ static inline irqreturn_t i2c_dw_isr_slave(struct dw_i2c_dev *dev) { return IRQ_
 
 static inline void i2c_dw_configure(struct dw_i2c_dev *dev)
 {
+#if IS_ENABLED(CONFIG_I2C_STARFIVE)
+	if (device_is_compatible(dev->dev, "starfive,jhb100-i2c-slave"))
+		i2c_sf_configure_slave(dev);
+	else
+		i2c_sf_configure_master(dev);
+#else
 	i2c_dw_configure_slave(dev);
 	i2c_dw_configure_master(dev);
+#endif
 }
 
 int i2c_dw_probe(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..f27c0fc761da 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -31,7 +31,7 @@
 #define AMD_TIMEOUT_MAX_US	250
 #define AMD_MASTERCFG_MASK	GENMASK(15, 0)
 
-static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
+__weak int i2c_dw_set_timings_master(struct dw_i2c_dev *dev)
 {
 	unsigned int comp_param1;
 	u32 sda_falling_time, scl_falling_time;
@@ -570,7 +570,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;
 
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 426ffec06e22..8c0f5c39e4ff 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-i2c-master") ||
+	    device_is_compatible(device, "starfive,jhb100-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-i2c-master" },
+	{ .compatible = "starfive,jhb100-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..1b94e4fbaad1 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;
 
-- 
2.43.0

[PATCH v2 3/3] i2c: starfive: Add StarFive JHB100 I2C master/slave support

[lianfeng.ouyang]

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

Add support for the Starfive 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.

Currently, due to the following differences, i2c designware cannot be
fully reused
1.  For high and low level counting settings at different rates, i2c
    starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
2.  Interrupt clearing is achieved by writing 1 to the corresponding
    bit of INTR_CLR, while designware reads different clearing registers
3.  Master and slave require separate probe callbacks and cannot rely
    solely on the runtime mode switching provided by i2c_dw_set_mode().
4.  The value of FIFO depth is not obtained through registers, but
written through DTS

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 register offsets and bit definitions.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index d3a6b3f6b6a0..7a6bc33b1118 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25319,6 +25319,13 @@ L:	linux-riscv@lists.infradead.org
 S:	Maintained
 F:	arch/riscv/boot/dts/starfive/jhb100*
 
+Starfive JHB100 I2C DRIVER
+M:	Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
+L:	linux-i2c@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
+F:	drivers/i2c/busses/i2c-starfive-*
+
 STARFIVE JHB100 PINCTRL DRIVERS
 M:	Changhuang Liang <changhuang.liang@starfivetech.com>
 M:	Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 7cb6b9b864a7..7630b19e706c 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -640,6 +640,30 @@ config I2C_DESIGNWARE_PCI
 
 endif
 
+config I2C_STARFIVE
+	tristate "Starfive i2c adapter"
+	select REGMAP
+	help
+	  This is a driver for the Starfive 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_STARFIVE
+
+config I2C_STARFIVE_SLAVE
+	bool "Starfive i2c slave"
+	default I2C_STARFIVE
+	select I2C_SLAVE
+	help
+	  If you say yes to this option, support will be included for the
+	  Starfive i2c slave adapter.
+
+	  This is not a standalone module, this module compiles together with
+	  i2c-starfive-core.
+
+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..1f6f56394bc9 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_STARFIVE) 				+= i2c-starfive.o
+i2c-starfive-y						+= i2c-starfive-master.o
+i2c-starfive-$(CONFIG_I2C_STARFIVE_SLAVE) 		+= i2c-starfive-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-starfive-core.h b/drivers/i2c/busses/i2c-starfive-core.h
new file mode 100644
index 000000000000..eb38d7215208
--- /dev/null
+++ b/drivers/i2c/busses/i2c-starfive-core.h
@@ -0,0 +1,164 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Starfive JHB100 I2C adapter driver.
+ *
+ * Based on the Starfive JHB100 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 SF_REG_FLAG				BIT(31)
+
+#define IC_DFLT_OPERATION_REG_OFFSET		(SF_REG_FLAG | 0x00)
+#define IC_DFLT_I2C_REG_OFFSET			(SF_REG_FLAG | 0x20)
+#define IC_DFLT_SMBUS_REG_OFFSET		(SF_REG_FLAG | 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)
+
+/*
+ * Registers offset
+ */
+#define SF_IC_ENABLE				(IC_DFLT_OPERATION_REG_OFFSET + 0x04)
+#define DW_IC_ENABLE_TX_CMD_BLOCK		BIT(2)
+#define SF_IC_ENABLE_DAR_EN			BIT(16)
+#define SF_IC_ENABLE_DAR2_EN			BIT(17)
+#define SF_IC_ENABLE_DAR3_EN			BIT(18)
+#define SF_IC_ENABLE_DAR4_EN			BIT(19)
+#define SF_IC_CAPABILITIES			(IC_DFLT_OPERATION_REG_OFFSET + 0x0c)
+#define SF_IC_CAPABILITIES_IC_SMBUS		BIT(10)
+#define SF_IC_SMBUS_CAPABILITIES		(IC_DFLT_OPERATION_REG_OFFSET + 0x18)
+#define SF_IC_SMBUS_CAPABILITIES_SMBUS_ARP	BIT(2)
+
+#define SF_IC_CTRL				(IC_DFLT_I2C_REG_OFFSET + 0x04)
+#define SF_IC_CTRL_OP_MODE			BIT(0)
+#define SF_IC_CTRL_SPEED_STD			BIT(4)
+#define SF_IC_CTRL_SPEED_FAST			(2 << 4)
+#define SF_IC_CTRL_SPEED_HIGH			(3 << 4)
+#define SF_IC_CTRL_SPEED_MASK			GENMASK(5, 4)
+#define SF_IC_CTRL_10BITADDR_TGT		BIT(8)
+#define SF_IC_CTRL_10BITADDR_CTRLR		BIT(9)
+#undef DW_IC_CON_10BITADDR_MASTER
+#define DW_IC_CON_10BITADDR_MASTER		BIT(9)
+#define SF_IC_CTRL_STOP_DET_IFADDRESSED		BIT(10)
+#define SF_IC_CTRL_TX_EMPTY_CTRL		BIT(11)
+#define SF_IC_CTRL_RX_FIFO_FULL_HLD_CTRL	BIT(12)
+#define SF_IC_CTRL_BUS_CLEAR_CTRL		BIT(14)
+#undef DW_IC_CON_BUS_CLEAR_CTRL
+#define DW_IC_CON_BUS_CLEAR_CTRL		BIT(14)
+#define SF_IC_TAR				(IC_DFLT_I2C_REG_OFFSET + 0x08)
+#define SF_IC_TAR_SPECIAL			BIT(11)
+#define SF_IC_TAR_SMBUS_QUICK_CMD		BIT(16)
+#define SF_IC_DAR				(IC_DFLT_I2C_REG_OFFSET + 0x0C)
+#define SF_IC_SCL_HCNT				(IC_DFLT_I2C_REG_OFFSET + 0x24)
+#define SF_IC_SCL_LCNT				(IC_DFLT_I2C_REG_OFFSET + 0x28)
+#define SF_IC_HS_SCL_HCNT			(IC_DFLT_I2C_REG_OFFSET + 0x2c)
+#define SF_IC_HS_SCL_LCNT			(IC_DFLT_I2C_REG_OFFSET + 0x30)
+#define SF_IC_SDA_HOLD				(IC_DFLT_I2C_REG_OFFSET + 0x34)
+#define DW_IC_SDA_HOLD_TX_SHIFT			0
+#define DW_IC_SDA_HOLD_TX_MASK			GENMASK(15, 0)
+#define SF_IC_SPKLEN				(IC_DFLT_I2C_REG_OFFSET + 0x3c)
+#define SF_IC_HS_SPKLEN				(IC_DFLT_I2C_REG_OFFSET + 0x40)
+#define SF_IC_SCL_STUCK_AT_LOW_TIMEOUT		(IC_DFLT_I2C_REG_OFFSET + 0x44)
+#define SF_IC_SCL_STUCK_AT_LOW_TIMEOUT_MAX	(IC_DFLT_I2C_REG_OFFSET + 0x48)
+#define SF_IC_SDA_STUCK_AT_LOW_TIMEOUT		(IC_DFLT_I2C_REG_OFFSET + 0x4c)
+#define SF_IC_DATA_CMD				(IC_DFLT_I2C_REG_OFFSET + 0x58)
+#define SF_IC_DATA_CMD_DAT			GENMASK(7, 0)
+#define SF_IC_DATA_CMD_CMD			BIT(8)
+#define SF_IC_DATA_CMD_STOP			BIT(9)
+#define SF_IC_DATA_CMD_FIRST_DATA_BYTE		BIT(11)
+#define SF_IC_RX_TL				(IC_DFLT_I2C_REG_OFFSET + 0x5c)
+#define SF_IC_TX_TL				(IC_DFLT_I2C_REG_OFFSET + 0x60)
+#define SF_IC_INTR_STAT				(IC_DFLT_I2C_REG_OFFSET + 0x74)
+#define SF_IC_INTR_SCL_STUCK_AT_LOW		BIT(14)
+#define SF_IC_INTR_MASK				(IC_DFLT_I2C_REG_OFFSET + 0x78)
+#define SF_IC_RAW_INTR_STAT			(IC_DFLT_I2C_REG_OFFSET + 0x7c)
+#define SF_IC_INTR_CLR				(IC_DFLT_I2C_REG_OFFSET + 0x80)
+#define SF_IC_CLR_INTR				BIT(0)
+#define SF_IC_CLR_RX_UNDER			BIT(1)
+#define SF_IC_CLR_RX_OVER			BIT(2)
+#define SF_IC_CLR_TX_OVER			BIT(3)
+#define SF_IC_CLR_RD_REQ			BIT(4)
+#define SF_IC_CLR_TX_ABRT			BIT(5)
+#define SF_IC_CLR_RX_DONE			BIT(6)
+#define SF_IC_CLR_ACTIVITY			BIT(7)
+#define SF_IC_CLR_STOP_DET			BIT(8)
+#define SF_IC_CLR_START_DET			BIT(9)
+#define SF_IC_CLR_GEN_CALL			BIT(10)
+#define SF_IC_CLR_RESTART_DET			BIT(11)
+#define SF_IC_CLR_SCL_STUCK_DET			BIT(12)
+#define SF_IC_ENABLE_STATUS			(IC_DFLT_I2C_REG_OFFSET + 0x84)
+#define SF_IC_TX_TRMNT_SOURCE			(IC_DFLT_I2C_REG_OFFSET + 0x88)
+#define SF_IC_STATUS				(IC_DFLT_I2C_REG_OFFSET + 0x8c)
+#define SF_IC_STATUS_ACTIVITY			BIT(0)
+#define SF_IC_STATUS_TFE			BIT(2)
+#define SF_IC_STATUS_RFNE			BIT(3)
+#define SF_IC_STATUS_CTRLR_ACTIVITY		BIT(5)
+#define SF_IC_STATUS_TGT_ACTIVITY		BIT(6)
+#define SF_IC_TXFLR				(IC_DFLT_I2C_REG_OFFSET + 0x90)
+#define SF_IC_RXFLR				(IC_DFLT_I2C_REG_OFFSET + 0x94)
+#define SF_IC_COMP_VERSION			(IC_DFLT_I2C_REG_OFFSET + 0xa4)
+#define SF_IC_COMP_TYPE				(IC_DFLT_I2C_REG_OFFSET + 0xa8)
+
+#define SF_IC_SMBUS_ARP_CTRL			(IC_DFLT_SMBUS_REG_OFFSET + 0x8)
+#define SF_IC_SMBUS_ARP_CTRL_NARP_DEVICE_TYPE	0
+#define SF_IC_SMBUS_INTR_STAT			(IC_DFLT_SMBUS_REG_OFFSET + 0x28)
+#define SF_IC_R_SMBUS_ALERT_DET			BIT(10)
+#define SF_IC_SMBUS_INTR_MASK			(IC_DFLT_SMBUS_REG_OFFSET + 0x2c)
+#define SF_IC_SMBUS_INTR_CLR			(IC_DFLT_SMBUS_REG_OFFSET + 0x34)
+#define SF_IC_CLR_SMBUS_ALERT_DET		BIT(10)
+
+/* error code */
+#define DW_IC_ERR_TX_ABRT			0x1
+
+#undef TXGBE_RX_FIFO_DEPTH
+#define TXGBE_RX_FIFO_DEPTH			0
+
+/* make (dev->master_cfg & DW_IC_CON_RESTART_EN) always true*/
+#undef DW_IC_CON_RESTART_EN
+#define DW_IC_CON_RESTART_EN			0xffffffff
+
+extern const struct i2c_algorithm i2c_dw_slave_algo;
+int sf_reg_read(void *context, unsigned int reg, unsigned int *val);
+int sf_reg_write(void *context, unsigned int reg, unsigned int val);
+int i2c_sf_probe_slave(struct dw_i2c_dev *dev);
+void i2c_dw_read_clear_intrbits_common(struct dw_i2c_dev *dev);
+void i2c_dw_write_timings(struct dw_i2c_dev *dev);
+void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode);
+int i2c_dw_set_timings_master(struct dw_i2c_dev *dev);
+void i2c_sf_configure_master(struct dw_i2c_dev *dev);
+void i2c_sf_configure_slave(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-starfive-master.c b/drivers/i2c/busses/i2c-starfive-master.c
new file mode 100644
index 000000000000..94fe041cf46d
--- /dev/null
+++ b/drivers/i2c/busses/i2c-starfive-master.c
@@ -0,0 +1,470 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Starfive JHB100 I2C adapter driver (master only).
+ * *
+ * 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)
+
+#define REG_MAP_NONE		0xffff
+
+struct starfive_reg_desc {
+	u32 sf_addr;
+	u32 dw_addr;
+};
+
+static const struct starfive_reg_desc sf_dw_reg_map[] = {
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_CTRL},		/* 0x0 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_TAR},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = SF_IC_CAPABILITIES,	.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_DATA_CMD},	/* 0x10 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = SF_IC_SMBUS_CAPABILITIES,	.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},	/* 0x20 */
+	{ .sf_addr = SF_IC_CTRL,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = SF_IC_DAR,			.dw_addr = SF_IC_INTR_STAT},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_INTR_MASK},	/* 0x30 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_RAW_INTR_STAT},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_INTR_CLR},	/* 0x40 */
+	{ .sf_addr = SF_IC_SCL_HCNT,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = SF_IC_SCL_LCNT,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = SF_IC_HS_SCL_HCNT,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = SF_IC_HS_SCL_LCNT,		.dw_addr = REG_MAP_NONE},	/* 0x50 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = SF_IC_SPKLEN,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = SF_IC_HS_SPKLEN,		.dw_addr = REG_MAP_NONE},	/* 0x60 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_ENABLE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_STATUS},	/* 0x70 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_TXFLR},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_RXFLR},
+	{ .sf_addr = SF_IC_RX_TL,		.dw_addr = SF_IC_SDA_HOLD},
+	{ .sf_addr = SF_IC_TX_TL,		.dw_addr = SF_IC_TX_TRMNT_SOURCE},	/* 0x80 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},	/* 0x90 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_ENABLE_STATUS},
+	{ .sf_addr = SF_IC_INTR_CLR,		.dw_addr = REG_MAP_NONE},	/* 0xA0 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},	/* 0xB0 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},	/* 0xC0 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_SMBUS_INTR_MASK},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},	/* 0xD0 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},	/* 0xE0 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},	/* 0xF0 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = SF_IC_COMP_VERSION},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},	/* 0x100 */
+	{ .sf_addr = SF_IC_SMBUS_ARP_CTRL,	.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},	/* 0x110 */
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},	/* 0x120 */
+	{ .sf_addr = SF_IC_SMBUS_INTR_STAT,	.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = REG_MAP_NONE,		.dw_addr = REG_MAP_NONE},
+	{ .sf_addr = SF_IC_SMBUS_INTR_CLR,	.dw_addr = REG_MAP_NONE},	/* 0x130 */
+};
+
+int sf_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct dw_i2c_dev *dev = context;
+	unsigned int reg_v, reg_nof, idx;
+
+	reg_nof = reg & ~SF_REG_FLAG;
+	idx = reg_nof >> 2;
+
+	if (idx >= ARRAY_SIZE(sf_dw_reg_map))
+		return -EINVAL;
+
+	const struct starfive_reg_desc *desc = &sf_dw_reg_map[idx];
+
+	if (reg & SF_REG_FLAG)
+		reg_v = desc->sf_addr;
+	else
+		reg_v = desc->dw_addr;
+
+	if (reg_v == 0xff) {
+		dev_err(dev->dev, "sf reg read 0x%x error!\n", reg_nof);
+		return -EINVAL;
+	}
+
+	if (reg_v == SF_IC_INTR_CLR)
+		writel(SF_IC_CLR_INTR, dev->base + (reg_v & ~SF_REG_FLAG));
+	else
+		*val = readl(dev->base + (reg_v & ~SF_REG_FLAG));
+
+	return 0;
+}
+
+int sf_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct dw_i2c_dev *dev = context;
+	unsigned int reg_v, reg_nof, idx;
+
+	reg_nof = reg & ~SF_REG_FLAG;
+	idx = reg_nof >> 2;
+
+	if (idx >= ARRAY_SIZE(sf_dw_reg_map))
+		return -EINVAL;
+
+	const struct starfive_reg_desc *desc = &sf_dw_reg_map[idx];
+
+	if (reg & SF_REG_FLAG)
+		reg_v = desc->sf_addr;
+	else
+		reg_v = desc->dw_addr;
+
+	if (reg_v == 0xff) {
+		dev_err(dev->dev, "sf reg read 0x%x error!\n", reg_nof);
+		return -EINVAL;
+	}
+
+	writel(val, dev->base + (reg_v & ~SF_REG_FLAG));
+
+	return 0;
+}
+
+/**
+ * i2c_sf_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_sf_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_sf_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_sf_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);
+}
+
+int i2c_dw_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, SF_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, SF_IC_HS_SPKLEN, spk_cnt);
+	regmap_write(dev->map, SF_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_sf_scl_hcnt(ic_clk, scl_high_time, scl_rising_time, spk_cnt);
+		dev->scl_lcnt = i2c_sf_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 & SF_IC_CTRL_SPEED_MASK) == SF_IC_CTRL_SPEED_HIGH) {
+		if ((comp_param1 & SF_IC_CTRL_SPEED_MASK) != SF_IC_CTRL_SPEED_HIGH) {
+			dev_err(dev->dev, "High Speed not supported!\n");
+			t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ;
+			dev->master_cfg &= ~SF_IC_CTRL_SPEED_MASK;
+			dev->master_cfg |= SF_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_sf_scl_hcnt(ic_clk, scl_high_time,
+							scl_rising_time, spk_cnt);
+			dev->scl_lcnt = i2c_sf_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;
+}
+
+void i2c_dw_write_timings(struct dw_i2c_dev *dev)
+{
+	/* Write standard speed timing parameters */
+	regmap_write(dev->map, SF_IC_SCL_HCNT, dev->scl_hcnt);
+	regmap_write(dev->map, SF_IC_SCL_LCNT, dev->scl_lcnt);
+
+	/* Write high speed timing parameters if supported */
+	if (dev->hs_hcnt && dev->hs_lcnt) {
+		regmap_write(dev->map, SF_IC_HS_SCL_HCNT, dev->hs_hcnt);
+		regmap_write(dev->map, SF_IC_HS_SCL_LCNT, dev->hs_lcnt);
+	}
+}
+
+void i2c_dw_configure_mode(struct dw_i2c_dev *dev, int mode)
+{
+	/* Configure Tx/Rx FIFO threshold levels */
+	regmap_write(dev->map, SF_IC_TX_TL, dev->tx_fifo_depth / 2);
+	regmap_write(dev->map, SF_IC_RX_TL, 0);
+
+	/* Configure the I2C master */
+	regmap_write(dev->map, SF_IC_CTRL, dev->master_cfg);
+}
+
+static int i2c_sf_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_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, SF_IC_INTR_CLR, SF_IC_CLR_RX_UNDER);
+	if (stat & DW_IC_INTR_RX_OVER)
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_IC_CLR_RX_OVER);
+	if (stat & DW_IC_INTR_TX_OVER)
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_IC_CLR_TX_OVER);
+	if (stat & DW_IC_INTR_RD_REQ)
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_IC_CLR_RD_REQ);
+	if (stat & DW_IC_INTR_RX_DONE)
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_IC_CLR_RX_DONE);
+	if (stat & DW_IC_INTR_ACTIVITY)
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_IC_CLR_ACTIVITY);
+	if (stat & DW_IC_INTR_START_DET)
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_IC_CLR_START_DET);
+	if (stat & DW_IC_INTR_GEN_CALL)
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_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, DW_IC_TX_ABRT_SOURCE, &dev->abort_source);
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_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, SF_IC_INTR_CLR, SF_IC_CLR_STOP_DET);
+	if (stat & DW_IC_INTR_RESTART_DET)
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_IC_CLR_RESTART_DET);
+	if (stat & SF_IC_INTR_SCL_STUCK_AT_LOW)
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_IC_CLR_SCL_STUCK_DET);
+
+	i2c_dw_read_clear_intrbits_common(dev);
+
+	return stat;
+}
+
+void i2c_sf_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 = SF_IC_CTRL_OP_MODE;
+
+	dev->mode = DW_IC_MASTER;
+
+	switch (t->bus_freq_hz) {
+	case I2C_MAX_STANDARD_MODE_FREQ:
+		dev->master_cfg |= SF_IC_CTRL_SPEED_STD;
+		break;
+	case I2C_MAX_FAST_MODE_FREQ:
+		dev->master_cfg |= SF_IC_CTRL_SPEED_FAST;
+		break;
+	case I2C_MAX_FAST_MODE_PLUS_FREQ:
+		dev->master_cfg |= SF_IC_CTRL_SPEED_FAST;
+		break;
+	case I2C_MAX_HIGH_SPEED_MODE_FREQ:
+		dev->master_cfg |= SF_IC_CTRL_SPEED_HIGH;
+		break;
+	default:
+		dev->master_cfg |= SF_IC_CTRL_SPEED_FAST;
+	}
+
+	dev->set_sda_hold_time = i2c_sf_set_sda_hold_time;
+}
+
+MODULE_AUTHOR("Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>");
+MODULE_DESCRIPTION("Starfive JHB100 I2C bus master adapter");
+MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-starfive-slave.c b/drivers/i2c/busses/i2c-starfive-slave.c
new file mode 100644
index 000000000000..06c3ead35841
--- /dev/null
+++ b/drivers/i2c/busses/i2c-starfive-slave.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Starfive JHB100 I2C adapter driver (slave only).
+ *
+ * Based on the Starfive JHB100 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, SF_IC_DAR, dev->slave->addr);
+		__i2c_dw_enable(dev);
+		dev->status = 0;
+	}
+}
+
+static void i2c_sf_configure_fifo_slave(struct dw_i2c_dev *dev)
+{
+	/* Configure the I2C slave. */
+	regmap_write(dev->map, SF_IC_CTRL, 0);
+	regmap_write(dev->map, SF_IC_CTRL, dev->slave_cfg);
+	regmap_write(dev->map, DW_IC_INTR_MASK, DW_IC_INTR_SLAVE_MASK);
+}
+
+/**
+ * i2c_sf_init_slave() - Initialize the Starfive 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_sf_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, SF_IC_CAPABILITIES, &r_value);
+	if (r_value & SF_IC_CAPABILITIES_IC_SMBUS) {
+		regmap_read(dev->map, SF_IC_SMBUS_CAPABILITIES, &r_value);
+		if (r_value & SF_IC_SMBUS_CAPABILITIES_SMBUS_ARP) {
+			regmap_write(dev->map, DW_IC_ENABLE, SF_IC_ENABLE_DAR_EN);
+			regmap_write(dev->map, SF_IC_SMBUS_ARP_CTRL,
+				     SF_IC_SMBUS_ARP_CTRL_NARP_DEVICE_TYPE);
+		}
+	}
+
+	i2c_sf_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, SF_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, SF_IC_INTR_CLR, SF_IC_CLR_TX_ABRT);
+	if (stat & DW_IC_INTR_STOP_DET)
+		regmap_write(dev->map, SF_IC_INTR_CLR, SF_IC_CLR_STOP_DET);
+	if (smbus_stat & SF_IC_R_SMBUS_ALERT_DET)
+		regmap_write(dev->map, SF_IC_SMBUS_INTR_CLR, SF_IC_CLR_SMBUS_ALERT_DET);
+
+	i2c_dw_read_clear_intrbits_common(dev);
+
+	return stat;
+}
+
+#if IS_ENABLED(CONFIG_I2C_STARFIVE_SLAVE)
+static int i2c_sf_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_sf_xfer_ms,
+	.functionality = i2c_dw_func,
+	.reg_slave = i2c_dw_reg_slave,
+	.unreg_slave = i2c_dw_unreg_slave,
+};
+
+void i2c_sf_configure_slave(struct dw_i2c_dev *dev)
+{
+	dev->functionality = I2C_FUNC_SLAVE | DW_IC_DEFAULT_FUNCTIONALITY;
+
+	dev->slave_cfg = SF_IC_CTRL_RX_FIFO_FULL_HLD_CTRL |
+			 SF_IC_CTRL_STOP_DET_IFADDRESSED;
+
+	dev->mode = DW_IC_SLAVE;
+}
+
+int i2c_sf_probe_slave(struct dw_i2c_dev *dev)
+{
+	struct i2c_adapter *adap = &dev->adapter;
+	struct device_node *ms_node;
+	int ret;
+
+	ret = i2c_sf_init_slave(dev);
+	if (ret)
+		return ret;
+
+	snprintf(adap->name, sizeof(adap->name), "Starfive JHB100 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_sf_probe_slave);
+
+MODULE_AUTHOR("Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>");
+MODULE_DESCRIPTION("Starfive JHB100 I2C bus slave adapter");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[Mika Westerberg]

Hi,

On Wed, May 27, 2026 at 04:50:36PM +0800, lianfeng.ouyang wrote:
> From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> 
> The Starfive JHB100 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.
> 
> The series is structured as follows:
> 1.  Adds the device tree binding document for the starfive,jhb100-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-starfive-* driver, with separate modules for
>         master and slave functionality, based on the 2023-07 revision of
> 	the Synopsys IP manual.
> 
> Currently, due to the following differences, i2c designware cannot be
> fully reused
> 1.  For high and low level counting settings at different rates, i2c
>         starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
> 2.  Interrupt clearing is achieved by writing 1 to the corresponding
>         bit of INTR_CLR, while designware reads different clearing
> 	registers
> 3.  Master and slave require separate probe callbacks and cannot rely
>         solely on the runtime mode switching provided by i2c_dw_set_mode()
> 4.  The value of FIFO depth is not obtained through registers, but
>         written through DTS
> 
> I have written some poorly styled code to reduce changes to i2c designware
> and reuse its functions by keeping aa always true, for example
> 1.  the implementation of i2c-d w_probe_master() differs only for the two
>         IPs in i2c_dw_set_timits_master(). In order to reuse
> 	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared as
> 	__weak. A better approach is to use a callback function, but using
> 	a callback function requires changing more i2c designware files.
> 	I don't know what the attitude of the community is
> 2.  For the operation of clearing interrupt flags, i2c designware reads
>         and i2c starfive writes. Therefore, in order not to modify the
> 	relevant logic of i2c designware, I added a write operation to
> 	sf_reg_read()
> So I think this version of the code is not allowed to merge, but I don't
> know how to handle this situation because if i2c designware is not changed
>  at all, we will have to write code that is similar to i2c designware.
>  Will this type of IP not be allowed to merge?

It's not that you are not allowed to change the existing driver. It is
definitely encouraged to avoid unnecessary duplication. If possible think
of the core parts as library that your glue driver can call and if
something is missing you can always add (or modify).

Also while there, let's start by adding support for the master operations
first and then later on you can add the slave bits.

Hope this helps.

Re: [PATCH v2 1/3] dt-bindings: i2c: Add StarFive JHB100 I2C

[Krzysztof Kozlowski]

On 27/05/2026 10:50, lianfeng.ouyang wrote:
> From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> 
> Add device tree bindings for the Starfive I2C controller
> and its implementation
> 
> The binding defines two platform-specific compatibles for the StarFive
> JHB100 implementation:
> - "starfive,jhb100-i2c-master"
> - "starfive,jhb100-i2c-slave"

You might get the same questions as v1, till you solve them. Why do you
add device role to the compatible?

Do not explain  WHAT you did here. Explain the background and why you
did that way.

Also, use undeprecated naming, not master/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>
> ---
>  .../bindings/i2c/starfive,jhb100-i2c.yaml     | 128 ++++++++++++++++++
>  1 file changed, 128 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml b/Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
> new file mode 100644
> index 000000000000..c8631348121c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
> @@ -0,0 +1,128 @@
> +# 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/starfive,jhb100-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive jhb100 I2C Controller
> +
> +maintainers:
> +  - Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> +
> +allOf:
> +  - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +properties:
> +  compatible:
> +    description: |
> +      Must be one of:
> +      - "starfive,jhb100-i2c-master" for master mode controller
> +      - "starfive,jhb100-i2c-slave" for slave mode controller

Drop description, redundant. Schema tells that.

> +    enum:
> +      - starfive,jhb100-i2c-master
> +      - starfive,jhb100-i2c-slave
> +
> +  reg:
> +    maxItems: 1
> +    description: StarFive I2C controller memory mapped registers

Drop description, redundant.

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 2

Drop both, redundant.

> +    items:
> +      - description: I2C controller reference clock source
> +      - description: APB interface clock source
> +
> +  clock-names:
> +    minItems: 2
> +    maxItems: 2

Same. From where did you take such syntax...

> +    items:
> +      - const: ref
> +      - const: pclk
> +
> +  resets:
> +    maxItems: 1
> +    description: Phandle to the reset controller for the I2C controller

Drop description. Or say something useful.

> +
> +  clock-frequency:
> +    description: Desired I2C bus clock frequency in Hz
> +    enum: [100000, 400000, 1000000, 3400000]

Aren't you duplicating constraints from dtschema?

> +    default: 400000
> +
> +  i2c-sda-hold-time-ns:

So you added a generic property - where is it documented? Generic
properties must be in common schema or dtschema.

And please prove that none of the generic properties are suitable.

> +    $ref: /schemas/types.yaml#/definitions/uint32

I don't think you tested it. And this concludes my review. I finished
here. Please do not send untested bindings.


Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 27/05/2026 10:50, lianfeng.ouyang wrote:
>  	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-i2c-master" },
> +	{ .compatible = "starfive,jhb100-i2c-slave" },

So both devices are the same... Or you miss proper entry in ID table.
Don't sprinkle compatibles around the code - ID table match data is for
variant customization.

Best regards,
Krzysztof

回复: [PATCH v2 2/3] i2c: designware: Export symbols and add __weak for Starfive I2C driver

[Lianfeng Ouyang]

Hi, Krzysztof
Thanks for the review.

> -----邮件原件-----
> 发件人: Krzysztof Kozlowski <krzk@kernel.org>
> 发送时间: 2026年5月27日 21:07
> 收件人: 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>; Mika
> Westerberg <mika.westerberg@linux.intel.com>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>
> 抄送: linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 主题: Re: [PATCH v2 2/3] i2c: designware: Export symbols and add __weak for
> Starfive I2C driver
> 
> On 27/05/2026 10:50, lianfeng.ouyang wrote:
> >  	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-i2c-master" },
> > +	{ .compatible = "starfive,jhb100-i2c-slave" },
> 
> So both devices are the same... Or you miss proper entry in ID table.
> Don't sprinkle compatibles around the code - ID table match data is for
> variant customization.
> 
> Best regards,
> Krzysztof

Ok, Master and slave have different base addresses and interrupt numbers, but 
the register layout is the same, so in the next version, I will only use the same 
compatible ”starfive, jhb100-i2c“, and then add starfive, i2c-slave to the dts node 
of the slave to distinguish them

Best Regards,
Lianfeng Ouyang

回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: Mika Westerberg <mika.westerberg@linux.intel.com>
> 发送时间: 2026年5月27日 20:36
> 收件人: 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>; linux-i2c@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
> 主题: Re: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support
> 
> Hi,
> 
> On Wed, May 27, 2026 at 04:50:36PM +0800, lianfeng.ouyang wrote:
> > From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> >
> > The Starfive JHB100 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.
> >
> > The series is structured as follows:
> > 1.  Adds the device tree binding document for the starfive,jhb100-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-starfive-* driver, with separate modules for
> >         master and slave functionality, based on the 2023-07 revision of
> > 	the Synopsys IP manual.
> >
> > Currently, due to the following differences, i2c designware cannot be
> > fully reused
> > 1.  For high and low level counting settings at different rates, i2c
> >         starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
> > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> >         bit of INTR_CLR, while designware reads different clearing
> > 	registers
> > 3.  Master and slave require separate probe callbacks and cannot rely
> >         solely on the runtime mode switching provided by
> i2c_dw_set_mode()
> > 4.  The value of FIFO depth is not obtained through registers, but
> >         written through DTS
> >
> > I have written some poorly styled code to reduce changes to i2c designware
> > and reuse its functions by keeping aa always true, for example
> > 1.  the implementation of i2c-d w_probe_master() differs only for the two
> >         IPs in i2c_dw_set_timits_master(). In order to reuse
> > 	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared as
> > 	__weak. A better approach is to use a callback function, but using
> > 	a callback function requires changing more i2c designware files.
> > 	I don't know what the attitude of the community is
> > 2.  For the operation of clearing interrupt flags, i2c designware reads
> >         and i2c starfive writes. Therefore, in order not to modify the
> > 	relevant logic of i2c designware, I added a write operation to
> > 	sf_reg_read()
> > So I think this version of the code is not allowed to merge, but I don't
> > know how to handle this situation because if i2c designware is not changed
> >  at all, we will have to write code that is similar to i2c designware.
> >  Will this type of IP not be allowed to merge?
> 
> It's not that you are not allowed to change the existing driver. It is
> definitely encouraged to avoid unnecessary duplication. If possible think
> of the core parts as library that your glue driver can call and if
> something is missing you can always add (or modify).
> 
> Also while there, let's start by adding support for the master operations
> first and then later on you can add the slave bits.
> 
> Hope this helps.

Thanks for your reply. I have a general understanding. First, I will complete the 
i2c Starfive Master driver and appropriately modify the i2c designware related 
functions into callback form

Best Regards,
Lianfeng Ouyang

回复: [PATCH v2 1/3] dt-bindings: i2c: Add StarFive JHB100 I2C

[Lianfeng Ouyang]

Hi, Krzysztof
Thanks for the review.

> -----邮件原件-----
> 发件人: Krzysztof Kozlowski <krzk@kernel.org>
> 发送时间: 2026年5月27日 21:05
> 收件人: 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>; Mika
> Westerberg <mika.westerberg@linux.intel.com>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>
> 抄送: linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 主题: Re: [PATCH v2 1/3] dt-bindings: i2c: Add StarFive JHB100 I2C
> 
> On 27/05/2026 10:50, lianfeng.ouyang wrote:
> > From: Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> >
> > Add device tree bindings for the Starfive I2C controller
> > and its implementation
> >
> > The binding defines two platform-specific compatibles for the StarFive
> > JHB100 implementation:
> > - "starfive,jhb100-i2c-master"
> > - "starfive,jhb100-i2c-slave"
> 
> You might get the same questions as v1, till you solve them. Why do you
> add device role to the compatible?
> 
> Do not explain  WHAT you did here. Explain the background and why you
> did that way.
> 
> Also, use undeprecated naming, not master/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>
> > ---
> >  .../bindings/i2c/starfive,jhb100-i2c.yaml     | 128 ++++++++++++++++++
> >  1 file changed, 128 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
> b/Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
> > new file mode 100644
> > index 000000000000..c8631348121c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/starfive,jhb100-i2c.yaml
> > @@ -0,0 +1,128 @@
> > +# 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/starfive,jhb100-i2c.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: StarFive jhb100 I2C Controller
> > +
> > +maintainers:
> > +  - Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
> > +
> > +allOf:
> > +  - $ref: /schemas/i2c/i2c-controller.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    description: |
> > +      Must be one of:
> > +      - "starfive,jhb100-i2c-master" for master mode controller
> > +      - "starfive,jhb100-i2c-slave" for slave mode controller
> 
> Drop description, redundant. Schema tells that.
> 
> > +    enum:
> > +      - starfive,jhb100-i2c-master
> > +      - starfive,jhb100-i2c-slave
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: StarFive I2C controller memory mapped registers
> 
> Drop description, redundant.
> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    minItems: 2
> > +    maxItems: 2
> 
> Drop both, redundant.
> 
> > +    items:
> > +      - description: I2C controller reference clock source
> > +      - description: APB interface clock source
> > +
> > +  clock-names:
> > +    minItems: 2
> > +    maxItems: 2
> 
> Same. From where did you take such syntax...
> 
> > +    items:
> > +      - const: ref
> > +      - const: pclk
> > +
> > +  resets:
> > +    maxItems: 1
> > +    description: Phandle to the reset controller for the I2C controller
> 
> Drop description. Or say something useful.
> 
> > +
> > +  clock-frequency:
> > +    description: Desired I2C bus clock frequency in Hz
> > +    enum: [100000, 400000, 1000000, 3400000]
> 
> Aren't you duplicating constraints from dtschema?
> 
> > +    default: 400000
> > +
> > +  i2c-sda-hold-time-ns:
> 
> So you added a generic property - where is it documented? Generic
> properties must be in common schema or dtschema.
> 
> And please prove that none of the generic properties are suitable.
> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> 
> I don't think you tested it. And this concludes my review. I finished
> here. Please do not send untested bindings.
> 
> 
> Best regards,
> Krzysztof

Sorry, I didn't know I needed to run dt-binding_check before, but now I have 
set up the environment. Starting from the next version, I will run dt-binding_check 
and modify it according to your reply. Then, I will remove the description of 
the required system properties and the common i2c properties

Best Regards,
Lianfeng Ouyang

Re: 回复: [PATCH v2 2/3] i2c: designware: Export symbols and add __weak for Starfive I2C driver

[Krzysztof Kozlowski]

On 28/05/2026 04:36, Lianfeng Ouyang wrote:
>>>  	{ .compatible = "mobileye,eyeq6lplus-i2c" },
>>>  	{ .compatible = "mscc,ocelot-i2c" },
>>>  	{ .compatible = "snps,designware-i2c" },
>>> +	{ .compatible = "starfive,jhb100-i2c-master" },
>>> +	{ .compatible = "starfive,jhb100-i2c-slave" },
>>
>> So both devices are the same... Or you miss proper entry in ID table.
>> Don't sprinkle compatibles around the code - ID table match data is for
>> variant customization.
>>
>> Best regards,
>> Krzysztof
> 
> Ok, Master and slave have different base addresses and interrupt numbers, but 
> the register layout is the same, so in the next version, I will only use the same 
> compatible ”starfive, jhb100-i2c“, and then add starfive, i2c-slave to the dts node 
> of the slave to distinguish them

You do not need to distinguish same devices usually, but I still do not
understand why do you need all these different compatibles and your
commit msgs or binding text do not help, so I don't know what to advise.


Best regards,
Krzysztof

Re: 回复: [PATCH v2 1/3] dt-bindings: i2c: Add StarFive JHB100 I2C

[Krzysztof Kozlowski]

On 28/05/2026 08:57, Lianfeng Ouyang wrote:
>>> +    default: 400000
>>> +
>>> +  i2c-sda-hold-time-ns:
>>
>> So you added a generic property - where is it documented? Generic
>> properties must be in common schema or dtschema.
>>
>> And please prove that none of the generic properties are suitable.
>>
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>
>> I don't think you tested it. And this concludes my review. I finished
>> here. Please do not send untested bindings.
>>
>>
>> Best regards,
>> Krzysztof
> 
> Sorry, I didn't know I needed to run dt-binding_check before, but now I have 

Every doc asks that. DTS101 slides or my earlier talks as well... Other
people talks as well... and you received that request last time at v1.
Please do not repeat the same mistakes because then we feel like our
feedback is wasted.

Best regards,
Krzysztof

回复: 回复: [PATCH v2 2/3] i2c: designware: Export symbols and add __weak for Starfive I2C driver

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: Krzysztof Kozlowski <krzk@kernel.org>
> 发送时间: 2026年5月28日 15:46
> 收件人: 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>; Mika
> Westerberg <mika.westerberg@linux.intel.com>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>
> 抄送: linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 主题: Re: 回复: [PATCH v2 2/3] i2c: designware: Export symbols and add
> __weak for Starfive I2C driver
> 
> On 28/05/2026 04:36, Lianfeng Ouyang wrote:
> >>>  	{ .compatible = "mobileye,eyeq6lplus-i2c" },
> >>>  	{ .compatible = "mscc,ocelot-i2c" },
> >>>  	{ .compatible = "snps,designware-i2c" },
> >>> +	{ .compatible = "starfive,jhb100-i2c-master" },
> >>> +	{ .compatible = "starfive,jhb100-i2c-slave" },
> >>
> >> So both devices are the same... Or you miss proper entry in ID table.
> >> Don't sprinkle compatibles around the code - ID table match data is for
> >> variant customization.
> >>
> >> Best regards,
> >> Krzysztof
> >
> > Ok, Master and slave have different base addresses and interrupt numbers, but
> > the register layout is the same, so in the next version, I will only use the same
> > compatible ”starfive, jhb100-i2c“, and then add starfive, i2c-slave to the dts
> node
> > of the slave to distinguish them
> 
> You do not need to distinguish same devices usually, but I still do not
> understand why do you need all these different compatibles and your
> commit msgs or binding text do not help, so I don't know what to advise.
> 
> 
> Best regards,
> Krzysztof

The IP corresponding to the i2c driver submitted this time is quite unique, 
so it should be distinguished. The master-slave of i2c designware are at the
same base address and switch according to the scene, so only one is initialized
in probe(). But i2c starfive has master-slave at different base addresses,
and it can work simultaneously. Therefore, both roles need to be initialized in 
their respective probes. If we don't distinguish, we don't know if this probe is 
initializing the master or the slave

Best Regards,
Lianfeng Ouyang

回复: 回复: [PATCH v2 1/3] dt-bindings: i2c: Add StarFive JHB100 I2C

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: Krzysztof Kozlowski <krzk@kernel.org>
> 发送时间: 2026年5月28日 15:48
> 收件人: 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>; Mika
> Westerberg <mika.westerberg@linux.intel.com>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>
> 抄送: linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 主题: Re: 回复: [PATCH v2 1/3] dt-bindings: i2c: Add StarFive JHB100 I2C
> 
> On 28/05/2026 08:57, Lianfeng Ouyang wrote:
> >>> +    default: 400000
> >>> +
> >>> +  i2c-sda-hold-time-ns:
> >>
> >> So you added a generic property - where is it documented? Generic
> >> properties must be in common schema or dtschema.
> >>
> >> And please prove that none of the generic properties are suitable.
> >>
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>
> >> I don't think you tested it. And this concludes my review. I finished
> >> here. Please do not send untested bindings.
> >>
> >>
> >> Best regards,
> >> Krzysztof
> >
> > Sorry, I didn't know I needed to run dt-binding_check before, but now I have
> 
> Every doc asks that. DTS101 slides or my earlier talks as well... Other
> people talks as well... and you received that request last time at v1.
> Please do not repeat the same mistakes because then we feel like our
> feedback is wasted.
> 
> Best regards,
> Krzysztof

I'm very sorry, I know that language is not strong at this moment. In order to 
prevent your feedback from being wasted, I will correct the relevant errors 
and run tests in the next version. Thank you

Best Regards,
Lianfeng Ouyang

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

[Andy Shevchenko]

On Wed, May 27, 2026 at 04:50:38PM +0800, lianfeng.ouyang wrote:

> 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 i2c starfive driver to reuse the common
> infrastructure while overriding the implementations where needed.
> 
> Additionally, extend the register map configuration and introduce the
> MODEL_STARFIVE flag to accommodate the starfive i2c  IP's different
> register space.

...

> +#endif

My gosh, no.


>  		.val_bits = 32,
>  		.reg_stride = 4,
>  		.disable_locking = true,
> +#if IS_ENABLED(CONFIG_I2C_STARFIVE)
> +		.reg_read = sf_reg_read,
> +		.reg_write = sf_reg_write,
> +		.max_register = SF_IC_SMBUS_INTR_CLR,
> +#else

No.

Just find a different way. See how Baikal T1 support was implemented
(in v6.18, for example).

>  		.reg_read = dw_reg_read,
>  		.reg_write = dw_reg_write,
>  		.max_register = DW_IC_COMP_TYPE,
> +#endif

...

> +#ifdef CONFIG_OF

Definitely no.

> +	ret = of_property_read_u32(dev->dev->of_node, "starfive,i2c-tx-fifo-depth", &tx_fifo_cfg);

No. Why?!

Also do not use OF-centric APIs in the driver that uses fwnode.

> +	if (!ret && (tx_fifo_cfg < 2 || tx_fifo_cfg > 256))
> +		tx_fifo_cfg = 8;
> +
> +	ret = of_property_read_u32(dev->dev->of_node, "starfive,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

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[Andy Shevchenko]

On Wed, May 27, 2026 at 04:50:36PM +0800, lianfeng.ouyang wrote:

> The Starfive JHB100 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.
> 
> The series is structured as follows:
> 1.  Adds the device tree binding document for the starfive,jhb100-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-starfive-* driver, with separate modules for
>         master and slave functionality, based on the 2023-07 revision of
> 	the Synopsys IP manual.
> 
> Currently, due to the following differences, i2c designware cannot be
> fully reused
> 1.  For high and low level counting settings at different rates, i2c
>         starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
> 2.  Interrupt clearing is achieved by writing 1 to the corresponding
>         bit of INTR_CLR, while designware reads different clearing
> 	registers
> 3.  Master and slave require separate probe callbacks and cannot rely
>         solely on the runtime mode switching provided by i2c_dw_set_mode()
> 4.  The value of FIFO depth is not obtained through registers, but
>         written through DTS

NAK in this form. We well discourage code duplication and ugly ifdeffery with
full of __weak annotations that may not be present in the regular driver. There
is not even a tiny bit of justification for this nonsense.

TL;DR: this series needs much more work.

> I have written some poorly styled code to reduce changes to i2c designware
> and reuse its functions by keeping aa always true, for example
> 1.  the implementation of i2c-d w_probe_master() differs only for the two
>         IPs in i2c_dw_set_timits_master(). In order to reuse
> 	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared as
> 	__weak. A better approach is to use a callback function, but using
> 	a callback function requires changing more i2c designware files.
> 	I don't know what the attitude of the community is
> 2.  For the operation of clearing interrupt flags, i2c designware reads
>         and i2c starfive writes. Therefore, in order not to modify the
> 	relevant logic of i2c designware, I added a write operation to
> 	sf_reg_read()
> So I think this version of the code is not allowed to merge, but I don't
> know how to handle this situation because if i2c designware is not changed
>  at all, we will have to write code that is similar to i2c designware.
>  Will this type of IP not be allowed to merge?

-- 
With Best Regards,
Andy Shevchenko

回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 发送时间: 2026年6月3日 6:27
> 收件人: 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>; Mika Westerberg <mika.westerberg@linux.intel.com>;
> linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 主题: Re: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support
> 
> On Wed, May 27, 2026 at 04:50:36PM +0800, lianfeng.ouyang wrote:
> 
> > The Starfive JHB100 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.
> >
> > The series is structured as follows:
> > 1.  Adds the device tree binding document for the starfive,jhb100-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-starfive-* driver, with separate modules for
> >         master and slave functionality, based on the 2023-07 revision of
> > 	the Synopsys IP manual.
> >
> > Currently, due to the following differences, i2c designware cannot be
> > fully reused
> > 1.  For high and low level counting settings at different rates, i2c
> >         starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
> > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> >         bit of INTR_CLR, while designware reads different clearing
> > 	registers
> > 3.  Master and slave require separate probe callbacks and cannot rely
> >         solely on the runtime mode switching provided by
> i2c_dw_set_mode()
> > 4.  The value of FIFO depth is not obtained through registers, but
> >         written through DTS
> 
> NAK in this form. We well discourage code duplication and ugly ifdeffery with
> full of __weak annotations that may not be present in the regular driver. There
> is not even a tiny bit of justification for this nonsense.
> 
> TL;DR: this series needs much more work.
> 
> > I have written some poorly styled code to reduce changes to i2c designware
> > and reuse its functions by keeping aa always true, for example
> > 1.  the implementation of i2c-d w_probe_master() differs only for the two
> >         IPs in i2c_dw_set_timits_master(). In order to reuse
> > 	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared as
> > 	__weak. A better approach is to use a callback function, but using
> > 	a callback function requires changing more i2c designware files.
> > 	I don't know what the attitude of the community is
> > 2.  For the operation of clearing interrupt flags, i2c designware reads
> >         and i2c starfive writes. Therefore, in order not to modify the
> > 	relevant logic of i2c designware, I added a write operation to
> > 	sf_reg_read()
> > So I think this version of the code is not allowed to merge, but I don't
> > know how to handle this situation because if i2c designware is not changed
> >  at all, we will have to write code that is similar to i2c designware.
> >  Will this type of IP not be allowed to merge?
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Thanks for the review.

In the future, the designware will be changed to the form of callback functions, 
and then callback functions will be passed in i2c starry - * and implemented 
using designware as a library

Best Regards,
Lianfeng Ouyang

Re: 回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[Andy Shevchenko]

On Wed, Jun 03, 2026 at 05:31:38AM +0000, Lianfeng Ouyang wrote:
> > -----邮件原件-----
> > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 发送时间: 2026年6月3日 6:27
> > On Wed, May 27, 2026 at 04:50:36PM +0800, lianfeng.ouyang wrote:

> > > The Starfive JHB100 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.
> > >
> > > The series is structured as follows:
> > > 1.  Adds the device tree binding document for the starfive,jhb100-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-starfive-* driver, with separate modules for
> > >         master and slave functionality, based on the 2023-07 revision of
> > > 	the Synopsys IP manual.
> > >
> > > Currently, due to the following differences, i2c designware cannot be
> > > fully reused
> > > 1.  For high and low level counting settings at different rates, i2c
> > >         starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
> > > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> > >         bit of INTR_CLR, while designware reads different clearing
> > > 	registers
> > > 3.  Master and slave require separate probe callbacks and cannot rely
> > >         solely on the runtime mode switching provided by
> > i2c_dw_set_mode()
> > > 4.  The value of FIFO depth is not obtained through registers, but
> > >         written through DTS
> > 
> > NAK in this form. We well discourage code duplication and ugly ifdeffery with
> > full of __weak annotations that may not be present in the regular driver. There
> > is not even a tiny bit of justification for this nonsense.
> > 
> > TL;DR: this series needs much more work.
> > 
> > > I have written some poorly styled code to reduce changes to i2c designware
> > > and reuse its functions by keeping aa always true, for example
> > > 1.  the implementation of i2c-d w_probe_master() differs only for the two
> > >         IPs in i2c_dw_set_timits_master(). In order to reuse
> > > 	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared as
> > > 	__weak. A better approach is to use a callback function, but using
> > > 	a callback function requires changing more i2c designware files.
> > > 	I don't know what the attitude of the community is
> > > 2.  For the operation of clearing interrupt flags, i2c designware reads
> > >         and i2c starfive writes. Therefore, in order not to modify the
> > > 	relevant logic of i2c designware, I added a write operation to
> > > 	sf_reg_read()
> > > So I think this version of the code is not allowed to merge, but I don't
> > > know how to handle this situation because if i2c designware is not changed
> > >  at all, we will have to write code that is similar to i2c designware.
> > >  Will this type of IP not be allowed to merge?
> 
> Thanks for the review.
> 
> In the future, the designware will be changed to the form of callback functions, 
> and then callback functions will be passed in i2c starry - * and implemented 
> using designware as a library

Why you can't specify your own regmap as it was done in Baikal case? What are
the obstacles to achieve that?

-- 
With Best Regards,
Andy Shevchenko

回复: 回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 发送时间: 2026年6月3日 13:49
> 收件人: 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>; Mika Westerberg <mika.westerberg@linux.intel.com>;
> linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 主题: Re: 回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave
> support
> 
> On Wed, Jun 03, 2026 at 05:31:38AM +0000, Lianfeng Ouyang wrote:
> > > -----邮件原件-----
> > > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 发送时间: 2026年6月3日 6:27
> > > On Wed, May 27, 2026 at 04:50:36PM +0800, lianfeng.ouyang wrote:
> 
> > > > The Starfive JHB100 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.
> > > >
> > > > The series is structured as follows:
> > > > 1.  Adds the device tree binding document for the starfive,jhb100-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-starfive-* driver, with separate modules for
> > > >         master and slave functionality, based on the 2023-07 revision of
> > > > 	the Synopsys IP manual.
> > > >
> > > > Currently, due to the following differences, i2c designware cannot be
> > > > fully reused
> > > > 1.  For high and low level counting settings at different rates, i2c
> > > >         starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
> > > > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> > > >         bit of INTR_CLR, while designware reads different clearing
> > > > 	registers
> > > > 3.  Master and slave require separate probe callbacks and cannot rely
> > > >         solely on the runtime mode switching provided by
> > > i2c_dw_set_mode()
> > > > 4.  The value of FIFO depth is not obtained through registers, but
> > > >         written through DTS
> > >
> > > NAK in this form. We well discourage code duplication and ugly ifdeffery with
> > > full of __weak annotations that may not be present in the regular driver.
> There
> > > is not even a tiny bit of justification for this nonsense.
> > >
> > > TL;DR: this series needs much more work.
> > >
> > > > I have written some poorly styled code to reduce changes to i2c
> designware
> > > > and reuse its functions by keeping aa always true, for example
> > > > 1.  the implementation of i2c-d w_probe_master() differs only for the two
> > > >         IPs in i2c_dw_set_timits_master(). In order to reuse
> > > > 	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared as
> > > > 	__weak. A better approach is to use a callback function, but using
> > > > 	a callback function requires changing more i2c designware files.
> > > > 	I don't know what the attitude of the community is
> > > > 2.  For the operation of clearing interrupt flags, i2c designware reads
> > > >         and i2c starfive writes. Therefore, in order not to modify the
> > > > 	relevant logic of i2c designware, I added a write operation to
> > > > 	sf_reg_read()
> > > > So I think this version of the code is not allowed to merge, but I don't
> > > > know how to handle this situation because if i2c designware is not changed
> > > >  at all, we will have to write code that is similar to i2c designware.
> > > >  Will this type of IP not be allowed to merge?
> >
> > Thanks for the review.
> >
> > In the future, the designware will be changed to the form of callback functions,
> > and then callback functions will be passed in i2c starry - * and implemented
> > using designware as a library
> 
> Why you can't specify your own regmap as it was done in Baikal case? What are
> the obstacles to achieve that?
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Hi Andy,

The main reasons are as follows
1.  For high and low level counting settings at different rates, i2c
starfive just use IC_SCL_H/LCNT to set SS, FM, FM+, UFM, 
====> Therefore, it is not possible to directly use the i2c_dew_set_timits_master() 
      of designware, Because the definition of registers has changed

2.  Interrupt clearing is achieved by writing 1 to the corresponding
bit of INTR_CLR, while designware reads different clearing
registers
====> The way of operating registers is different, so it cannot be 
      distinguished solely by address or offset, and can only be adapted 
      to operations belonging to i2c starfive through callback functions

3.  Master and slave require separate probe callbacks and cannot rely
solely on the runtime mode switching provided by i2c_dw_set_mode()
====> There are host and slave IP addresses separately, unlike designware 
      where one IP supports two roles, because the logic of switching roles 
      cannot be distinguished by address and offset

4.  The value of FIFO depth is not obtained through registers, but
written through DTS
====> The register does not have information on the depth of FIFO, 
      and manual writing of register settings is required

In addition to the above four points, the initialization, transmission, and interrupts 
of i2c Starfive are basically the same as i2c Desirware. Therefore, we hope to reuse 
i2c designware instead of re implementing the i2c Starfive driver, otherwise the code repetition will be high

Hope I can answer your question, thank you

Re: 回复: 回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[Andy Shevchenko]

On Wed, Jun 03, 2026 at 06:09:24AM +0000, Lianfeng Ouyang wrote:
> > -----邮件原件-----
> > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 发送时间: 2026年6月3日 13:49
> > On Wed, Jun 03, 2026 at 05:31:38AM +0000, Lianfeng Ouyang wrote:
> > > > -----邮件原件-----
> > > > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > 发送时间: 2026年6月3日 6:27
> > > > On Wed, May 27, 2026 at 04:50:36PM +0800, lianfeng.ouyang wrote:

> > > > > The Starfive JHB100 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.
> > > > >
> > > > > The series is structured as follows:
> > > > > 1.  Adds the device tree binding document for the starfive,jhb100-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-starfive-* driver, with separate modules for
> > > > >         master and slave functionality, based on the 2023-07 revision of
> > > > > 	the Synopsys IP manual.
> > > > >
> > > > > Currently, due to the following differences, i2c designware cannot be
> > > > > fully reused
> > > > > 1.  For high and low level counting settings at different rates, i2c
> > > > >         starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
> > > > > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> > > > >         bit of INTR_CLR, while designware reads different clearing
> > > > > 	registers
> > > > > 3.  Master and slave require separate probe callbacks and cannot rely
> > > > >         solely on the runtime mode switching provided by
> > > > i2c_dw_set_mode()
> > > > > 4.  The value of FIFO depth is not obtained through registers, but
> > > > >         written through DTS
> > > >
> > > > NAK in this form. We well discourage code duplication and ugly ifdeffery with
> > > > full of __weak annotations that may not be present in the regular driver.
> > There
> > > > is not even a tiny bit of justification for this nonsense.
> > > >
> > > > TL;DR: this series needs much more work.
> > > >
> > > > > I have written some poorly styled code to reduce changes to i2c
> > designware
> > > > > and reuse its functions by keeping aa always true, for example
> > > > > 1.  the implementation of i2c-d w_probe_master() differs only for the two
> > > > >         IPs in i2c_dw_set_timits_master(). In order to reuse
> > > > > 	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared as
> > > > > 	__weak. A better approach is to use a callback function, but using
> > > > > 	a callback function requires changing more i2c designware files.
> > > > > 	I don't know what the attitude of the community is
> > > > > 2.  For the operation of clearing interrupt flags, i2c designware reads
> > > > >         and i2c starfive writes. Therefore, in order not to modify the
> > > > > 	relevant logic of i2c designware, I added a write operation to
> > > > > 	sf_reg_read()
> > > > > So I think this version of the code is not allowed to merge, but I don't
> > > > > know how to handle this situation because if i2c designware is not changed
> > > > >  at all, we will have to write code that is similar to i2c designware.
> > > > >  Will this type of IP not be allowed to merge?
> > >
> > > Thanks for the review.
> > >
> > > In the future, the designware will be changed to the form of callback functions,
> > > and then callback functions will be passed in i2c starry - * and implemented
> > > using designware as a library
> > 
> > Why you can't specify your own regmap as it was done in Baikal case? What are
> > the obstacles to achieve that?
> 
> The main reasons are as follows
> 1.  For high and low level counting settings at different rates, i2c
> starfive just use IC_SCL_H/LCNT to set SS, FM, FM+, UFM, 
> ====> Therefore, it is not possible to directly use the i2c_dew_set_timits_master() 
>       of designware, Because the definition of registers has changed

So, it's not a DW per se? What the DW databook reflects the register layout and
other bits? I.o.w. which version of DW i2c IP is this?

> 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> bit of INTR_CLR, while designware reads different clearing
> registers
> ====> The way of operating registers is different, so it cannot be 
>       distinguished solely by address or offset, and can only be adapted 
>       to operations belonging to i2c starfive through callback functions

This can be done in the custom regmap functions. You know that you may
redefine the regmap IO accessors and do whatever you want there based on
the registers. Yes, what you are probably talking about is a workflow.

But we basically need to see a patch-per-workflow change for each case
to understand the differences better. I.o.w. if you need, for instance,
separate or special IRQ handling part, create a patch to move existing
code to a callback with explanation why it's needed. All the same for
the timings and so on. It would be nice to have a datasheet at hand to
actually read and see the differences. Without that it's even hard to
propose better solutions. But current state is for sure no go.

> 3.  Master and slave require separate probe callbacks and cannot rely
> solely on the runtime mode switching provided by i2c_dw_set_mode()
> ====> There are host and slave IP addresses separately, unlike designware 
>       where one IP supports two roles, because the logic of switching roles 
>       cannot be distinguished by address and offset

This feature should be left at last, but altogether it smells like either
a completely new i2c DW design (like they did for DMA and SSI at some
point) and most likely will need a brand new driver explaining all of this
with a link to a datasheet.

> 4.  The value of FIFO depth is not obtained through registers, but
> written through DTS
> ====> The register does not have information on the depth of FIFO, 
>       and manual writing of register settings is required
> 
> In addition to the above four points, the initialization, transmission, and interrupts 
> of i2c Starfive are basically the same as i2c Desirware. Therefore, we hope to reuse 
> i2c designware instead of re implementing the i2c Starfive driver, otherwise the code repetition will be high

These sounds like candidates for i2c-designware-lib.c, indeed.
But the rest most likely shouldn't be intervened at all. I.o.w.
please provide a detailed analysis (something like [1]) in
the cover letter, or just send an RFC on the driver design
before even starting considering actual coding.

> Hope I can answer your question, thank you

[1]: e539f435cb9c ("spi: dw: Add support for DesignWare DWC_ssi")


-- 
With Best Regards,
Andy Shevchenko

Re: 回复: 回复: [PATCH v2 2/3] i2c: designware: Export symbols and add __weak for Starfive I2C driver

[Krzysztof Kozlowski]

On 28/05/2026 10:28, Lianfeng Ouyang wrote:
> 
> 
>> -----邮件原件-----
>> 发件人: Krzysztof Kozlowski <krzk@kernel.org>
>> 发送时间: 2026年5月28日 15:46
>> 收件人: 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>; Mika
>> Westerberg <mika.westerberg@linux.intel.com>; Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com>
>> 抄送: linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
>> linux-kernel@vger.kernel.org
>> 主题: Re: 回复: [PATCH v2 2/3] i2c: designware: Export symbols and add
>> __weak for Starfive I2C driver
>>
>> On 28/05/2026 04:36, Lianfeng Ouyang wrote:
>>>>>  	{ .compatible = "mobileye,eyeq6lplus-i2c" },
>>>>>  	{ .compatible = "mscc,ocelot-i2c" },
>>>>>  	{ .compatible = "snps,designware-i2c" },
>>>>> +	{ .compatible = "starfive,jhb100-i2c-master" },
>>>>> +	{ .compatible = "starfive,jhb100-i2c-slave" },
>>>>
>>>> So both devices are the same... Or you miss proper entry in ID table.
>>>> Don't sprinkle compatibles around the code - ID table match data is for
>>>> variant customization.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>
>>> Ok, Master and slave have different base addresses and interrupt numbers, but
>>> the register layout is the same, so in the next version, I will only use the same
>>> compatible ”starfive, jhb100-i2c“, and then add starfive, i2c-slave to the dts
>> node
>>> of the slave to distinguish them
>>
>> You do not need to distinguish same devices usually, but I still do not
>> understand why do you need all these different compatibles and your
>> commit msgs or binding text do not help, so I don't know what to advise.
>>
>>
>> Best regards,
>> Krzysztof
> 
> The IP corresponding to the i2c driver submitted this time is quite unique, 
> so it should be distinguished. The master-slave of i2c designware are at the
> same base address and switch according to the scene, so only one is initialized
> in probe(). But i2c starfive has master-slave at different base addresses,
> and it can work simultaneously. Therefore, both roles need to be initialized in 
> their respective probes. If we don't distinguish, we don't know if this probe is 
> initializing the master or the slave
> 

I don't think you read the comments. Again, you claim devices are
incompatible but have exactly the same programming interface.

Best regards,
Krzysztof

回复: 回复: 回复: [PATCH v2 2/3] i2c: designware: Export symbols and add __weak for Starfive I2C driver

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: Krzysztof Kozlowski <krzk@kernel.org>
> 发送时间: 2026年6月3日 22:34
> 收件人: 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>; Mika
> Westerberg <mika.westerberg@linux.intel.com>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>
> 抄送: linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 主题: Re: 回复: 回复: [PATCH v2 2/3] i2c: designware: Export symbols and add
> __weak for Starfive I2C driver
> 
> On 28/05/2026 10:28, Lianfeng Ouyang wrote:
> >
> >
> >> -----邮件原件-----
> >> 发件人: Krzysztof Kozlowski <krzk@kernel.org>
> >> 发送时间: 2026年5月28日 15:46
> >> 收件人: 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>; Mika
> >> Westerberg <mika.westerberg@linux.intel.com>; Andy Shevchenko
> >> <andriy.shevchenko@linux.intel.com>
> >> 抄送: linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> >> linux-kernel@vger.kernel.org
> >> 主题: Re: 回复: [PATCH v2 2/3] i2c: designware: Export symbols and add
> >> __weak for Starfive I2C driver
> >>
> >> On 28/05/2026 04:36, Lianfeng Ouyang wrote:
> >>>>>  	{ .compatible = "mobileye,eyeq6lplus-i2c" },
> >>>>>  	{ .compatible = "mscc,ocelot-i2c" },
> >>>>>  	{ .compatible = "snps,designware-i2c" },
> >>>>> +	{ .compatible = "starfive,jhb100-i2c-master" },
> >>>>> +	{ .compatible = "starfive,jhb100-i2c-slave" },
> >>>>
> >>>> So both devices are the same... Or you miss proper entry in ID table.
> >>>> Don't sprinkle compatibles around the code - ID table match data is for
> >>>> variant customization.
> >>>>
> >>>> Best regards,
> >>>> Krzysztof
> >>>
> >>> Ok, Master and slave have different base addresses and interrupt numbers,
> but
> >>> the register layout is the same, so in the next version, I will only use the
> same
> >>> compatible ”starfive, jhb100-i2c“, and then add starfive, i2c-slave to the dts
> >> node
> >>> of the slave to distinguish them
> >>
> >> You do not need to distinguish same devices usually, but I still do not
> >> understand why do you need all these different compatibles and your
> >> commit msgs or binding text do not help, so I don't know what to advise.
> >>
> >>
> >> Best regards,
> >> Krzysztof
> >
> > The IP corresponding to the i2c driver submitted this time is quite unique,
> > so it should be distinguished. The master-slave of i2c designware are at the
> > same base address and switch according to the scene, so only one is initialized
> > in probe(). But i2c starfive has master-slave at different base addresses,
> > and it can work simultaneously. Therefore, both roles need to be initialized in
> > their respective probes. If we don't distinguish, we don't know if this probe is
> > initializing the master or the slave
> >
> 
> I don't think you read the comments. Again, you claim devices are
> incompatible but have exactly the same programming interface.
> 
> Best regards,
> Krzysztof

Hi Krzysztof , 
Thanks for your review.

Now i2c has been changed to use the same compatible and use 
.data to distinguish between master and slave

Best Regards,
Lianfeng Ouyang

回复: 回复: 回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 发送时间: 2026年6月3日 14:40
> 收件人: 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>; Mika Westerberg <mika.westerberg@linux.intel.com>;
> linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 主题: Re: 回复: 回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C
> master/slave support
> 
> On Wed, Jun 03, 2026 at 06:09:24AM +0000, Lianfeng Ouyang wrote:
> > > -----邮件原件-----
> > > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 发送时间: 2026年6月3日 13:49
> > > On Wed, Jun 03, 2026 at 05:31:38AM +0000, Lianfeng Ouyang wrote:
> > > > > -----邮件原件-----
> > > > > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > > 发送时间: 2026年6月3日 6:27
> > > > > On Wed, May 27, 2026 at 04:50:36PM +0800, lianfeng.ouyang wrote:
> 
> > > > > > The Starfive JHB100 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.
> > > > > >
> > > > > > The series is structured as follows:
> > > > > > 1.  Adds the device tree binding document for the starfive,jhb100-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-starfive-* driver, with separate modules for
> > > > > >         master and slave functionality, based on the 2023-07 revision
> of
> > > > > > 	the Synopsys IP manual.
> > > > > >
> > > > > > Currently, due to the following differences, i2c designware cannot be
> > > > > > fully reused
> > > > > > 1.  For high and low level counting settings at different rates, i2c
> > > > > >         starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
> > > > > > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> > > > > >         bit of INTR_CLR, while designware reads different clearing
> > > > > > 	registers
> > > > > > 3.  Master and slave require separate probe callbacks and cannot rely
> > > > > >         solely on the runtime mode switching provided by
> > > > > i2c_dw_set_mode()
> > > > > > 4.  The value of FIFO depth is not obtained through registers, but
> > > > > >         written through DTS
> > > > >
> > > > > NAK in this form. We well discourage code duplication and ugly ifdeffery
> with
> > > > > full of __weak annotations that may not be present in the regular driver.
> > > There
> > > > > is not even a tiny bit of justification for this nonsense.
> > > > >
> > > > > TL;DR: this series needs much more work.
> > > > >
> > > > > > I have written some poorly styled code to reduce changes to i2c
> > > designware
> > > > > > and reuse its functions by keeping aa always true, for example
> > > > > > 1.  the implementation of i2c-d w_probe_master() differs only for the
> two
> > > > > >         IPs in i2c_dw_set_timits_master(). In order to reuse
> > > > > > 	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared as
> > > > > > 	__weak. A better approach is to use a callback function, but using
> > > > > > 	a callback function requires changing more i2c designware files.
> > > > > > 	I don't know what the attitude of the community is
> > > > > > 2.  For the operation of clearing interrupt flags, i2c designware reads
> > > > > >         and i2c starfive writes. Therefore, in order not to modify the
> > > > > > 	relevant logic of i2c designware, I added a write operation to
> > > > > > 	sf_reg_read()
> > > > > > So I think this version of the code is not allowed to merge, but I don't
> > > > > > know how to handle this situation because if i2c designware is not
> changed
> > > > > >  at all, we will have to write code that is similar to i2c designware.
> > > > > >  Will this type of IP not be allowed to merge?
> > > >
> > > > Thanks for the review.
> > > >
> > > > In the future, the designware will be changed to the form of callback
> functions,
> > > > and then callback functions will be passed in i2c starry - * and implemented
> > > > using designware as a library
> > >
> > > Why you can't specify your own regmap as it was done in Baikal case? What
> are
> > > the obstacles to achieve that?
> >
> > The main reasons are as follows
> > 1.  For high and low level counting settings at different rates, i2c
> > starfive just use IC_SCL_H/LCNT to set SS, FM, FM+, UFM,
> > ====> Therefore, it is not possible to directly use the
> i2c_dew_set_timits_master()
> >       of designware, Because the definition of registers has changed
> 
> So, it's not a DW per se? What the DW databook reflects the register layout and
> other bits? I.o.w. which version of DW i2c IP is this?
> 
> > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> > bit of INTR_CLR, while designware reads different clearing
> > registers
> > ====> The way of operating registers is different, so it cannot be
> >       distinguished solely by address or offset, and can only be adapted
> >       to operations belonging to i2c starfive through callback functions
> 
> This can be done in the custom regmap functions. You know that you may
> redefine the regmap IO accessors and do whatever you want there based on
> the registers. Yes, what you are probably talking about is a workflow.
> 
> But we basically need to see a patch-per-workflow change for each case
> to understand the differences better. I.o.w. if you need, for instance,
> separate or special IRQ handling part, create a patch to move existing
> code to a callback with explanation why it's needed. All the same for
> the timings and so on. It would be nice to have a datasheet at hand to
> actually read and see the differences. Without that it's even hard to
> propose better solutions. But current state is for sure no go.
> 
> > 3.  Master and slave require separate probe callbacks and cannot rely
> > solely on the runtime mode switching provided by i2c_dw_set_mode()
> > ====> There are host and slave IP addresses separately, unlike designware
> >       where one IP supports two roles, because the logic of switching roles
> >       cannot be distinguished by address and offset
> 
> This feature should be left at last, but altogether it smells like either
> a completely new i2c DW design (like they did for DMA and SSI at some
> point) and most likely will need a brand new driver explaining all of this
> with a link to a datasheet.
> 
> > 4.  The value of FIFO depth is not obtained through registers, but
> > written through DTS
> > ====> The register does not have information on the depth of FIFO,
> >       and manual writing of register settings is required
> >
> > In addition to the above four points, the initialization, transmission, and
> interrupts
> > of i2c Starfive are basically the same as i2c Desirware. Therefore, we hope to
> reuse
> > i2c designware instead of re implementing the i2c Starfive driver, otherwise
> the code repetition will be high
> 
> These sounds like candidates for i2c-designware-lib.c, indeed.
> But the rest most likely shouldn't be intervened at all. I.o.w.
> please provide a detailed analysis (something like [1]) in
> the cover letter, or just send an RFC on the driver design
> before even starting considering actual coding.
> 
> > Hope I can answer your question, thank you
> 
> [1]: e539f435cb9c ("spi: dw: Add support for DesignWare DWC_ssi")
> 
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Okay, based on the format of e539f435cb9c, I have compiled the differences in 
register offsets and the differences between registers IC_CTRL and IC-INTR_CLR.
Looking forward to your reply

DWC_ADV_i2c is the enhanced version of DW_apb_i2c (corresponding to existing i2c designware driver), 
Most of their registers are similar, but the bit fields of key registers 
(e.g. IC_CTRL/IC_CON, IC_TAR, IC_ENABLE) are different, and DWC_ADV_i2c has 
additional registers for mission critical features and newer protocol version support.

DWC_ADV_i2c has additional features compared to DW_apb_i2c. Major enhancements
in DWC_ADV_i2c are:
- Support for I2C Bus Specification V7.0 (October 2021) and SMBus Specification V3.2 (January 2022),
  while DW_apb_i2c supports I2C V6.0 (April 2014) and SMBus V3.1 (March 2018)
- Support for SMBus Non-ARP Command Code Decoding and Advanced Block Read/Write
- Support for dynamic target address update (IC_DYNAMIC_TAR_UPDATE=1)
- Support for Ultra-Fast Mode (up to 5 MB/s) with dedicated timing registers
- Both DWC_ADV_i2c and DW_apb_i2c are APB slave devices, supporting APB2/APB3/APB4
  interfaces with configurable data width (8/16/32 bits).

Register offset (base address 0x0 for both, DWC_ADV_i2c uses separate address blocks for
operational, I2C and SMBus registers, DW_apb_i2c uses a unified register map)
                        DWC_ADV_i2c          DW_apb_i2c
IC_HCI_VERSION            0x00                    N/A
IC_ENABLE                 0x04                    0x6c
IC_RESET_CTRL             0x08                    N/A
IC_CAPABILITIES             0x0c                    N/A
IC_I2C_CAPABILITIES         0x10                    N/A
IC_SMBUS_CAPABILITIES      0x18                    0x9c
IC_SMBUS_BLOCK_OFFSET     0x1c                    0xa0
IC_CTRL                   0x04 (I2C block)         0x00 (IC_CON)
IC_TAR                    0x08 (I2C block)        0x04
IC_DAR                    0x0c (I2C block)        0x08 (IC_SAR)
IC_HS_CADDR               0x10 (I2C block)        0x0c (IC_HS_MADDR)
IC_DATA_CMD               0x58 (I2C block)        0x10
IC_UFM_TBUF_CNT           0x20 (I2C block)        N/A
IC_SCL_HCNT               0x24 (I2C block)        0x1c (IC_FS_SCL_HCNT)
IC_SCL_LCNT               0x28 (I2C block)        0x20 (IC_FS_SCL_LCNT)
IC_HS_SCL_HCNT            0x2c (I2C block)        0x24
IC_HS_SCL_LCNT            0x30 (I2C block)        0x28
IC_SDA_HOLD               0x34 (I2C block)        0x7c
IC_SDA_SETUP              0x38 (I2C block)        0x94
IC_SPKLEN                 0x3c (I2C block)        0xa0 (IC_FS_SPKLEN)
IC_HS_SPKLEN              0x40 (I2C block)        0xa4
IC_DMA_RDLR               0x6c (I2C block)        0x90
IC_INTR_STAT              0x74 (I2C block)         0x2c
IC_INTR_MASK              0x78 (I2C block)        0x30
IC_RAW_INTR_STAT          0x7c (I2C block)        0x34
IC_DAR4                   0xb8 (I2C block)        0x108 (IC_SAR4)
IC_SFTY_CTRL              0xc0 (I2C block)         N/A
IC_SMBUS_CTRL             0x04 (SMBus block)      N/A
IC_SMBUS_ARP_CTRL         0x08 (SMBus block)      N/A
IC_SMBUS_INTR_STAT        0x28 (SMBus block)      0xc8
IC_SMBUS_INTR_MASK        0x2c (SMBus block)      0xcc
IC_SMBUS_INTR_RAW_STAT    0x30 (SMBus block)      0xd0

Register configuration - IC_CTRL (DWC_ADV_i2c) vs IC_CON (DW_apb_i2c)
                        DWC_Adv_i2c          DW_apb_i2c
  IC_OP_MODE               bit[0]            bit[0] (MASTER_MODE)
  IC_SPEED                  bit[5:4]           bit[2:1] (SPEED)
  IC_10BITADDR_TGT          bit[8]            bit[3] (IC_10BITADDR_SLAVE)
  IC_10BITADDR_CTRLR        bit[9]            bit[4] (IC_10BITADDR_MASTER)
  BUS_CLEAR_FEATURE_CTRL    bit[14]           bit[11]

Register configuration - IC_INTR_CLR
                        DWC_ADV_i2c              DW_apb_i2c
  CLR_INTR               bit[0] (write)      IC_INTR_CLR       bit[0] (read)
  CLR_RX_UNDER          bit[1] (write)      IC_CLR_RX_UNDER  bit[0] (read)
  CLR_RX_OVER           bit[2] (write)      IC_CLR_RX_OVER    bit[0] (read)
  CLR_TX_OVER            bit[3] (write)      IC_CLR_TX_OVER   bit[0] (read)
  CLR_RD_REQ             bit[4] (write)      IC_CLR_RD_REQ    bit[0] (read)
  ......

The documents used are
[1] DesignWare ® Cores Advanced I2C/SMBus Controller and Target Device Databook, Product Code H464-0, I2C Bus Spec V7.0, 
[2] DesignWare ® DW_apb_i2c Databook, Version 2.03a, December 2020, I2C Bus Spec V6.0

Best Regards,
Lianfeng Ouyang

Re: 回复: 回复: 回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[Andy Shevchenko]

On Thu, Jun 04, 2026 at 03:00:13AM +0000, Lianfeng Ouyang wrote:
> > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 发送时间: 2026年6月3日 14:40
> > On Wed, Jun 03, 2026 at 06:09:24AM +0000, Lianfeng Ouyang wrote:
> > > > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > 发送时间: 2026年6月3日 13:49
> > > > On Wed, Jun 03, 2026 at 05:31:38AM +0000, Lianfeng Ouyang wrote:
> > > > > > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > > > 发送时间: 2026年6月3日 6:27
> > > > > > On Wed, May 27, 2026 at 04:50:36PM +0800, lianfeng.ouyang wrote:
> > 
> > > > > > > The Starfive JHB100 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.
> > > > > > >
> > > > > > > The series is structured as follows:
> > > > > > > 1.  Adds the device tree binding document for the starfive,jhb100-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-starfive-* driver, with separate modules for
> > > > > > >         master and slave functionality, based on the 2023-07 revision
> > of
> > > > > > > 	the Synopsys IP manual.
> > > > > > >
> > > > > > > Currently, due to the following differences, i2c designware cannot be
> > > > > > > fully reused
> > > > > > > 1.  For high and low level counting settings at different rates, i2c
> > > > > > >         starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
> > > > > > > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> > > > > > >         bit of INTR_CLR, while designware reads different clearing
> > > > > > > 	registers
> > > > > > > 3.  Master and slave require separate probe callbacks and cannot rely
> > > > > > >         solely on the runtime mode switching provided by
> > > > > > i2c_dw_set_mode()
> > > > > > > 4.  The value of FIFO depth is not obtained through registers, but
> > > > > > >         written through DTS
> > > > > >
> > > > > > NAK in this form. We well discourage code duplication and ugly ifdeffery
> > with
> > > > > > full of __weak annotations that may not be present in the regular driver.
> > > > There
> > > > > > is not even a tiny bit of justification for this nonsense.
> > > > > >
> > > > > > TL;DR: this series needs much more work.
> > > > > >
> > > > > > > I have written some poorly styled code to reduce changes to i2c
> > > > designware
> > > > > > > and reuse its functions by keeping aa always true, for example
> > > > > > > 1.  the implementation of i2c-d w_probe_master() differs only for the
> > two
> > > > > > >         IPs in i2c_dw_set_timits_master(). In order to reuse
> > > > > > > 	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared as
> > > > > > > 	__weak. A better approach is to use a callback function, but using
> > > > > > > 	a callback function requires changing more i2c designware files.
> > > > > > > 	I don't know what the attitude of the community is
> > > > > > > 2.  For the operation of clearing interrupt flags, i2c designware reads
> > > > > > >         and i2c starfive writes. Therefore, in order not to modify the
> > > > > > > 	relevant logic of i2c designware, I added a write operation to
> > > > > > > 	sf_reg_read()
> > > > > > > So I think this version of the code is not allowed to merge, but I don't
> > > > > > > know how to handle this situation because if i2c designware is not
> > changed
> > > > > > >  at all, we will have to write code that is similar to i2c designware.
> > > > > > >  Will this type of IP not be allowed to merge?
> > > > >
> > > > > Thanks for the review.
> > > > >
> > > > > In the future, the designware will be changed to the form of callback
> > functions,
> > > > > and then callback functions will be passed in i2c starry - * and implemented
> > > > > using designware as a library
> > > >
> > > > Why you can't specify your own regmap as it was done in Baikal case? What
> > are
> > > > the obstacles to achieve that?
> > >
> > > The main reasons are as follows
> > > 1.  For high and low level counting settings at different rates, i2c
> > > starfive just use IC_SCL_H/LCNT to set SS, FM, FM+, UFM,
> > > ====> Therefore, it is not possible to directly use the
> > i2c_dew_set_timits_master()
> > >       of designware, Because the definition of registers has changed
> > 
> > So, it's not a DW per se? What the DW databook reflects the register layout and
> > other bits? I.o.w. which version of DW i2c IP is this?
> > 
> > > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> > > bit of INTR_CLR, while designware reads different clearing
> > > registers
> > > ====> The way of operating registers is different, so it cannot be
> > >       distinguished solely by address or offset, and can only be adapted
> > >       to operations belonging to i2c starfive through callback functions
> > 
> > This can be done in the custom regmap functions. You know that you may
> > redefine the regmap IO accessors and do whatever you want there based on
> > the registers. Yes, what you are probably talking about is a workflow.
> > 
> > But we basically need to see a patch-per-workflow change for each case
> > to understand the differences better. I.o.w. if you need, for instance,
> > separate or special IRQ handling part, create a patch to move existing
> > code to a callback with explanation why it's needed. All the same for
> > the timings and so on. It would be nice to have a datasheet at hand to
> > actually read and see the differences. Without that it's even hard to
> > propose better solutions. But current state is for sure no go.
> > 
> > > 3.  Master and slave require separate probe callbacks and cannot rely
> > > solely on the runtime mode switching provided by i2c_dw_set_mode()
> > > ====> There are host and slave IP addresses separately, unlike designware
> > >       where one IP supports two roles, because the logic of switching roles
> > >       cannot be distinguished by address and offset
> > 
> > This feature should be left at last, but altogether it smells like either
> > a completely new i2c DW design (like they did for DMA and SSI at some
> > point) and most likely will need a brand new driver explaining all of this
> > with a link to a datasheet.
> > 
> > > 4.  The value of FIFO depth is not obtained through registers, but
> > > written through DTS
> > > ====> The register does not have information on the depth of FIFO,
> > >       and manual writing of register settings is required
> > >
> > > In addition to the above four points, the initialization, transmission, and
> > interrupts
> > > of i2c Starfive are basically the same as i2c Desirware. Therefore, we hope to
> > reuse
> > > i2c designware instead of re implementing the i2c Starfive driver, otherwise
> > the code repetition will be high
> > 
> > These sounds like candidates for i2c-designware-lib.c, indeed.
> > But the rest most likely shouldn't be intervened at all. I.o.w.
> > please provide a detailed analysis (something like [1]) in
> > the cover letter, or just send an RFC on the driver design
> > before even starting considering actual coding.
> > 
> > > Hope I can answer your question, thank you
> > 
> > [1]: e539f435cb9c ("spi: dw: Add support for DesignWare DWC_ssi")
> 
> Okay, based on the format of e539f435cb9c, I have compiled the differences in 
> register offsets and the differences between registers IC_CTRL and IC-INTR_CLR.
> Looking forward to your reply

Thank you for this information, it makes much more easier to see the big picture.
With the given information I think you most likely need a brand new driver.
No callbacks for sure. The only thing that may be done is to split really common
code and data structures in i2c-designware-lib.[ch] between two. This will
require quite a bit of refactoring (as you most likely don't want to inherit
all quirked and hacked fields from struct dw_i2c_dev, meaning you want to
have something just very basic and common between two there. means that
the current struct dw_i2c_dev will be morphed to something like

drivers/i2c/busses/i2c-designware-lib.h:

	struct dw_i2c_common {
		...
	};

drivers/i2c/busses/i2c-designware-core.h:

	struct dw_i2c_dev {
		struct dw_i2c_common icom;
		...
	};

and in your new driver

	struct dw_adv_i2c_dev {
		struct dw_i2c_common icom;
		...
	};

And in the drivers/i2c/busses/i2c-designware-lib.c:

	... dw_i2c_common_...(struct dw_i2c_common *icom, ...)
	{
		...
	}

for the shared APIs.

With that I predict a series out of ~10 patches (or more).

The brand new driver is also a solution, but to go there you need to at least
have a PoC of the above to justify that the shared code makes little sense.

> DWC_ADV_i2c is the enhanced version of DW_apb_i2c (corresponding to existing i2c designware driver), 
> Most of their registers are similar, but the bit fields of key registers 
> (e.g. IC_CTRL/IC_CON, IC_TAR, IC_ENABLE) are different, and DWC_ADV_i2c has 
> additional registers for mission critical features and newer protocol version support.
> 
> DWC_ADV_i2c has additional features compared to DW_apb_i2c. Major enhancements
> in DWC_ADV_i2c are:
> - Support for I2C Bus Specification V7.0 (October 2021) and SMBus Specification V3.2 (January 2022),
>   while DW_apb_i2c supports I2C V6.0 (April 2014) and SMBus V3.1 (March 2018)
> - Support for SMBus Non-ARP Command Code Decoding and Advanced Block Read/Write
> - Support for dynamic target address update (IC_DYNAMIC_TAR_UPDATE=1)
> - Support for Ultra-Fast Mode (up to 5 MB/s) with dedicated timing registers
> - Both DWC_ADV_i2c and DW_apb_i2c are APB slave devices, supporting APB2/APB3/APB4
>   interfaces with configurable data width (8/16/32 bits).
> 
> Register offset (base address 0x0 for both, DWC_ADV_i2c uses separate address blocks for
> operational, I2C and SMBus registers, DW_apb_i2c uses a unified register map)
>                         DWC_ADV_i2c          DW_apb_i2c
> IC_HCI_VERSION            0x00                    N/A
> IC_ENABLE                 0x04                    0x6c
> IC_RESET_CTRL             0x08                    N/A
> IC_CAPABILITIES             0x0c                    N/A
> IC_I2C_CAPABILITIES         0x10                    N/A
> IC_SMBUS_CAPABILITIES      0x18                    0x9c
> IC_SMBUS_BLOCK_OFFSET     0x1c                    0xa0
> IC_CTRL                   0x04 (I2C block)         0x00 (IC_CON)
> IC_TAR                    0x08 (I2C block)        0x04
> IC_DAR                    0x0c (I2C block)        0x08 (IC_SAR)
> IC_HS_CADDR               0x10 (I2C block)        0x0c (IC_HS_MADDR)
> IC_DATA_CMD               0x58 (I2C block)        0x10
> IC_UFM_TBUF_CNT           0x20 (I2C block)        N/A
> IC_SCL_HCNT               0x24 (I2C block)        0x1c (IC_FS_SCL_HCNT)
> IC_SCL_LCNT               0x28 (I2C block)        0x20 (IC_FS_SCL_LCNT)
> IC_HS_SCL_HCNT            0x2c (I2C block)        0x24
> IC_HS_SCL_LCNT            0x30 (I2C block)        0x28
> IC_SDA_HOLD               0x34 (I2C block)        0x7c
> IC_SDA_SETUP              0x38 (I2C block)        0x94
> IC_SPKLEN                 0x3c (I2C block)        0xa0 (IC_FS_SPKLEN)
> IC_HS_SPKLEN              0x40 (I2C block)        0xa4
> IC_DMA_RDLR               0x6c (I2C block)        0x90
> IC_INTR_STAT              0x74 (I2C block)         0x2c
> IC_INTR_MASK              0x78 (I2C block)        0x30
> IC_RAW_INTR_STAT          0x7c (I2C block)        0x34
> IC_DAR4                   0xb8 (I2C block)        0x108 (IC_SAR4)
> IC_SFTY_CTRL              0xc0 (I2C block)         N/A
> IC_SMBUS_CTRL             0x04 (SMBus block)      N/A
> IC_SMBUS_ARP_CTRL         0x08 (SMBus block)      N/A
> IC_SMBUS_INTR_STAT        0x28 (SMBus block)      0xc8
> IC_SMBUS_INTR_MASK        0x2c (SMBus block)      0xcc
> IC_SMBUS_INTR_RAW_STAT    0x30 (SMBus block)      0xd0
> 
> Register configuration - IC_CTRL (DWC_ADV_i2c) vs IC_CON (DW_apb_i2c)
>                         DWC_Adv_i2c          DW_apb_i2c
>   IC_OP_MODE               bit[0]            bit[0] (MASTER_MODE)
>   IC_SPEED                  bit[5:4]           bit[2:1] (SPEED)
>   IC_10BITADDR_TGT          bit[8]            bit[3] (IC_10BITADDR_SLAVE)
>   IC_10BITADDR_CTRLR        bit[9]            bit[4] (IC_10BITADDR_MASTER)
>   BUS_CLEAR_FEATURE_CTRL    bit[14]           bit[11]
> 
> Register configuration - IC_INTR_CLR
>                         DWC_ADV_i2c              DW_apb_i2c
>   CLR_INTR               bit[0] (write)      IC_INTR_CLR       bit[0] (read)
>   CLR_RX_UNDER          bit[1] (write)      IC_CLR_RX_UNDER  bit[0] (read)
>   CLR_RX_OVER           bit[2] (write)      IC_CLR_RX_OVER    bit[0] (read)
>   CLR_TX_OVER            bit[3] (write)      IC_CLR_TX_OVER   bit[0] (read)
>   CLR_RD_REQ             bit[4] (write)      IC_CLR_RD_REQ    bit[0] (read)
>   ......
> 
> The documents used are
> [1] DesignWare ® Cores Advanced I2C/SMBus Controller and Target Device Databook, Product Code H464-0, I2C Bus Spec V7.0, 
> [2] DesignWare ® DW_apb_i2c Databook, Version 2.03a, December 2020, I2C Bus Spec V6.0

-- 
With Best Regards,
Andy Shevchenko

回复: 回复: 回复: 回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support

[Lianfeng Ouyang]

> -----邮件原件-----
> 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 发送时间: 2026年6月5日 2:09
> 收件人: 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>; Mika Westerberg <mika.westerberg@linux.intel.com>;
> linux-i2c@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> 主题: Re: 回复: 回复: 回复: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C
> master/slave support
> 
> On Thu, Jun 04, 2026 at 03:00:13AM +0000, Lianfeng Ouyang wrote:
> > > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > 发送时间: 2026年6月3日 14:40
> > > On Wed, Jun 03, 2026 at 06:09:24AM +0000, Lianfeng Ouyang wrote:
> > > > > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > > 发送时间: 2026年6月3日 13:49
> > > > > On Wed, Jun 03, 2026 at 05:31:38AM +0000, Lianfeng Ouyang wrote:
> > > > > > > 发件人: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > > > > 发送时间: 2026年6月3日 6:27
> > > > > > > On Wed, May 27, 2026 at 04:50:36PM +0800, lianfeng.ouyang wrote:
> > >
> > > > > > > > The Starfive JHB100 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.
> > > > > > > >
> > > > > > > > The series is structured as follows:
> > > > > > > > 1.  Adds the device tree binding document for the
> starfive,jhb100-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-starfive-* driver, with separate modules
> for
> > > > > > > >         master and slave functionality, based on the 2023-07
> revision
> > > of
> > > > > > > > 	the Synopsys IP manual.
> > > > > > > >
> > > > > > > > Currently, due to the following differences, i2c designware cannot
> be
> > > > > > > > fully reused
> > > > > > > > 1.  For high and low level counting settings at different rates, i2c
> > > > > > > >         starfive can use IC_SCL-H/LCNT to set SS, FM, FM+, UFM
> > > > > > > > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> > > > > > > >         bit of INTR_CLR, while designware reads different
> clearing
> > > > > > > > 	registers
> > > > > > > > 3.  Master and slave require separate probe callbacks and cannot
> rely
> > > > > > > >         solely on the runtime mode switching provided by
> > > > > > > i2c_dw_set_mode()
> > > > > > > > 4.  The value of FIFO depth is not obtained through registers, but
> > > > > > > >         written through DTS
> > > > > > >
> > > > > > > NAK in this form. We well discourage code duplication and ugly
> ifdeffery
> > > with
> > > > > > > full of __weak annotations that may not be present in the regular
> driver.
> > > > > There
> > > > > > > is not even a tiny bit of justification for this nonsense.
> > > > > > >
> > > > > > > TL;DR: this series needs much more work.
> > > > > > >
> > > > > > > > I have written some poorly styled code to reduce changes to i2c
> > > > > designware
> > > > > > > > and reuse its functions by keeping aa always true, for example
> > > > > > > > 1.  the implementation of i2c-d w_probe_master() differs only for
> the
> > > two
> > > > > > > >         IPs in i2c_dw_set_timits_master(). In order to reuse
> > > > > > > > 	i2c_dw_probe_master(), i2c_dw_set_timits_master is declared
> as
> > > > > > > > 	__weak. A better approach is to use a callback function, but using
> > > > > > > > 	a callback function requires changing more i2c designware files.
> > > > > > > > 	I don't know what the attitude of the community is
> > > > > > > > 2.  For the operation of clearing interrupt flags, i2c designware
> reads
> > > > > > > >         and i2c starfive writes. Therefore, in order not to modify
> the
> > > > > > > > 	relevant logic of i2c designware, I added a write operation to
> > > > > > > > 	sf_reg_read()
> > > > > > > > So I think this version of the code is not allowed to merge, but I
> don't
> > > > > > > > know how to handle this situation because if i2c designware is not
> > > changed
> > > > > > > >  at all, we will have to write code that is similar to i2c designware.
> > > > > > > >  Will this type of IP not be allowed to merge?
> > > > > >
> > > > > > Thanks for the review.
> > > > > >
> > > > > > In the future, the designware will be changed to the form of callback
> > > functions,
> > > > > > and then callback functions will be passed in i2c starry - * and
> implemented
> > > > > > using designware as a library
> > > > >
> > > > > Why you can't specify your own regmap as it was done in Baikal case?
> What
> > > are
> > > > > the obstacles to achieve that?
> > > >
> > > > The main reasons are as follows
> > > > 1.  For high and low level counting settings at different rates, i2c
> > > > starfive just use IC_SCL_H/LCNT to set SS, FM, FM+, UFM,
> > > > ====> Therefore, it is not possible to directly use the
> > > i2c_dew_set_timits_master()
> > > >       of designware, Because the definition of registers has changed
> > >
> > > So, it's not a DW per se? What the DW databook reflects the register layout
> and
> > > other bits? I.o.w. which version of DW i2c IP is this?
> > >
> > > > 2.  Interrupt clearing is achieved by writing 1 to the corresponding
> > > > bit of INTR_CLR, while designware reads different clearing
> > > > registers
> > > > ====> The way of operating registers is different, so it cannot be
> > > >       distinguished solely by address or offset, and can only be adapted
> > > >       to operations belonging to i2c starfive through callback functions
> > >
> > > This can be done in the custom regmap functions. You know that you may
> > > redefine the regmap IO accessors and do whatever you want there based on
> > > the registers. Yes, what you are probably talking about is a workflow.
> > >
> > > But we basically need to see a patch-per-workflow change for each case
> > > to understand the differences better. I.o.w. if you need, for instance,
> > > separate or special IRQ handling part, create a patch to move existing
> > > code to a callback with explanation why it's needed. All the same for
> > > the timings and so on. It would be nice to have a datasheet at hand to
> > > actually read and see the differences. Without that it's even hard to
> > > propose better solutions. But current state is for sure no go.
> > >
> > > > 3.  Master and slave require separate probe callbacks and cannot rely
> > > > solely on the runtime mode switching provided by i2c_dw_set_mode()
> > > > ====> There are host and slave IP addresses separately, unlike designware
> > > >       where one IP supports two roles, because the logic of switching
> roles
> > > >       cannot be distinguished by address and offset
> > >
> > > This feature should be left at last, but altogether it smells like either
> > > a completely new i2c DW design (like they did for DMA and SSI at some
> > > point) and most likely will need a brand new driver explaining all of this
> > > with a link to a datasheet.
> > >
> > > > 4.  The value of FIFO depth is not obtained through registers, but
> > > > written through DTS
> > > > ====> The register does not have information on the depth of FIFO,
> > > >       and manual writing of register settings is required
> > > >
> > > > In addition to the above four points, the initialization, transmission, and
> > > interrupts
> > > > of i2c Starfive are basically the same as i2c Desirware. Therefore, we hope
> to
> > > reuse
> > > > i2c designware instead of re implementing the i2c Starfive driver,
> otherwise
> > > the code repetition will be high
> > >
> > > These sounds like candidates for i2c-designware-lib.c, indeed.
> > > But the rest most likely shouldn't be intervened at all. I.o.w.
> > > please provide a detailed analysis (something like [1]) in
> > > the cover letter, or just send an RFC on the driver design
> > > before even starting considering actual coding.
> > >
> > > > Hope I can answer your question, thank you
> > >
> > > [1]: e539f435cb9c ("spi: dw: Add support for DesignWare DWC_ssi")
> >
> > Okay, based on the format of e539f435cb9c, I have compiled the differences in
> > register offsets and the differences between registers IC_CTRL and
> IC-INTR_CLR.
> > Looking forward to your reply
> 
> Thank you for this information, it makes much more easier to see the big
> picture.
> With the given information I think you most likely need a brand new driver.
> No callbacks for sure. The only thing that may be done is to split really common
> code and data structures in i2c-designware-lib.[ch] between two. This will
> require quite a bit of refactoring (as you most likely don't want to inherit
> all quirked and hacked fields from struct dw_i2c_dev, meaning you want to
> have something just very basic and common between two there. means that
> the current struct dw_i2c_dev will be morphed to something like
> 
> drivers/i2c/busses/i2c-designware-lib.h:
> 
> 	struct dw_i2c_common {
> 		...
> 	};
> 
> drivers/i2c/busses/i2c-designware-core.h:
> 
> 	struct dw_i2c_dev {
> 		struct dw_i2c_common icom;
> 		...
> 	};
> 
> and in your new driver
> 
> 	struct dw_adv_i2c_dev {
> 		struct dw_i2c_common icom;
> 		...
> 	};
> 
> And in the drivers/i2c/busses/i2c-designware-lib.c:
> 
> 	... dw_i2c_common_...(struct dw_i2c_common *icom, ...)
> 	{
> 		...
> 	}
> 
> for the shared APIs.
> 
> With that I predict a series out of ~10 patches (or more).
> 
> The brand new driver is also a solution, but to go there you need to at least
> have a PoC of the above to justify that the shared code makes little sense.
> 

Thank you for your idea. I will modify my code in this direction

Best Regards,
Lianfeng Ouyang

> > DWC_ADV_i2c is the enhanced version of DW_apb_i2c (corresponding to
> existing i2c designware driver),
> > Most of their registers are similar, but the bit fields of key registers
> > (e.g. IC_CTRL/IC_CON, IC_TAR, IC_ENABLE) are different, and DWC_ADV_i2c
> has
> > additional registers for mission critical features and newer protocol version
> support.
> >
> > DWC_ADV_i2c has additional features compared to DW_apb_i2c. Major
> enhancements
> > in DWC_ADV_i2c are:
> > - Support for I2C Bus Specification V7.0 (October 2021) and SMBus
> Specification V3.2 (January 2022),
> >   while DW_apb_i2c supports I2C V6.0 (April 2014) and SMBus V3.1 (March
> 2018)
> > - Support for SMBus Non-ARP Command Code Decoding and Advanced Block
> Read/Write
> > - Support for dynamic target address update (IC_DYNAMIC_TAR_UPDATE=1)
> > - Support for Ultra-Fast Mode (up to 5 MB/s) with dedicated timing registers
> > - Both DWC_ADV_i2c and DW_apb_i2c are APB slave devices, supporting
> APB2/APB3/APB4
> >   interfaces with configurable data width (8/16/32 bits).
> >
> > Register offset (base address 0x0 for both, DWC_ADV_i2c uses separate
> address blocks for
> > operational, I2C and SMBus registers, DW_apb_i2c uses a unified register
> map)
> >                         DWC_ADV_i2c          DW_apb_i2c
> > IC_HCI_VERSION            0x00                    N/A
> > IC_ENABLE                 0x04                    0x6c
> > IC_RESET_CTRL             0x08                    N/A
> > IC_CAPABILITIES             0x0c                    N/A
> > IC_I2C_CAPABILITIES         0x10                    N/A
> > IC_SMBUS_CAPABILITIES      0x18                    0x9c
> > IC_SMBUS_BLOCK_OFFSET     0x1c                    0xa0
> > IC_CTRL                   0x04 (I2C block)         0x00 (IC_CON)
> > IC_TAR                    0x08 (I2C block)        0x04
> > IC_DAR                    0x0c (I2C block)        0x08 (IC_SAR)
> > IC_HS_CADDR               0x10 (I2C block)        0x0c
> (IC_HS_MADDR)
> > IC_DATA_CMD               0x58 (I2C block)        0x10
> > IC_UFM_TBUF_CNT           0x20 (I2C block)        N/A
> > IC_SCL_HCNT               0x24 (I2C block)        0x1c
> (IC_FS_SCL_HCNT)
> > IC_SCL_LCNT               0x28 (I2C block)        0x20
> (IC_FS_SCL_LCNT)
> > IC_HS_SCL_HCNT            0x2c (I2C block)        0x24
> > IC_HS_SCL_LCNT            0x30 (I2C block)        0x28
> > IC_SDA_HOLD               0x34 (I2C block)        0x7c
> > IC_SDA_SETUP              0x38 (I2C block)        0x94
> > IC_SPKLEN                 0x3c (I2C block)        0xa0 (IC_FS_SPKLEN)
> > IC_HS_SPKLEN              0x40 (I2C block)        0xa4
> > IC_DMA_RDLR               0x6c (I2C block)        0x90
> > IC_INTR_STAT              0x74 (I2C block)         0x2c
> > IC_INTR_MASK              0x78 (I2C block)        0x30
> > IC_RAW_INTR_STAT          0x7c (I2C block)        0x34
> > IC_DAR4                   0xb8 (I2C block)        0x108 (IC_SAR4)
> > IC_SFTY_CTRL              0xc0 (I2C block)         N/A
> > IC_SMBUS_CTRL             0x04 (SMBus block)      N/A
> > IC_SMBUS_ARP_CTRL         0x08 (SMBus block)      N/A
> > IC_SMBUS_INTR_STAT        0x28 (SMBus block)      0xc8
> > IC_SMBUS_INTR_MASK        0x2c (SMBus block)      0xcc
> > IC_SMBUS_INTR_RAW_STAT    0x30 (SMBus block)      0xd0
> >
> > Register configuration - IC_CTRL (DWC_ADV_i2c) vs IC_CON (DW_apb_i2c)
> >                         DWC_Adv_i2c          DW_apb_i2c
> >   IC_OP_MODE               bit[0]            bit[0]
> (MASTER_MODE)
> >   IC_SPEED                  bit[5:4]           bit[2:1] (SPEED)
> >   IC_10BITADDR_TGT          bit[8]            bit[3]
> (IC_10BITADDR_SLAVE)
> >   IC_10BITADDR_CTRLR        bit[9]            bit[4]
> (IC_10BITADDR_MASTER)
> >   BUS_CLEAR_FEATURE_CTRL    bit[14]           bit[11]
> >
> > Register configuration - IC_INTR_CLR
> >                         DWC_ADV_i2c              DW_apb_i2c
> >   CLR_INTR               bit[0] (write)      IC_INTR_CLR
> bit[0] (read)
> >   CLR_RX_UNDER          bit[1] (write)      IC_CLR_RX_UNDER
> bit[0] (read)
> >   CLR_RX_OVER           bit[2] (write)      IC_CLR_RX_OVER
> bit[0] (read)
> >   CLR_TX_OVER            bit[3] (write)      IC_CLR_TX_OVER
> bit[0] (read)
> >   CLR_RD_REQ             bit[4] (write)      IC_CLR_RD_REQ
> bit[0] (read)
> >   ......
> >
> > The documents used are
> > [1] DesignWare ® Cores Advanced I2C/SMBus Controller and Target Device
> Databook, Product Code H464-0, I2C Bus Spec V7.0,
> > [2] DesignWare ® DW_apb_i2c Databook, Version 2.03a, December 2020, I2C
> Bus Spec V6.0
> 
> --
> With Best Regards,
> Andy Shevchenko
>