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

lianfeng.ouyang posted 3 patches 1 week, 5 days ago
.../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
[PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support
Posted by lianfeng.ouyang 1 week, 5 days ago
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
Re: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support
Posted by Andy Shevchenko 5 days, 19 hours ago
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
Re: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support
Posted by Mika Westerberg 1 week, 5 days ago
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.