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