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

[Lianfeng Ouyang]

Hi, Mika

Thank for the comments

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

Let's first discuss using i2c StarFive

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

i2c designware:
DesignWare DW_apb_i2c Databook
1.16a
October 2011

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

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

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

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

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

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

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

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