Devicetree
 help / color / mirror / Atom feed
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: "lianfeng.ouyang" <lianfeng.ouyang@starfivetech.com>
Cc: 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
Subject: Re: [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support
Date: Wed, 27 May 2026 14:36:20 +0200	[thread overview]
Message-ID: <20260527123620.GD3102@black.igk.intel.com> (raw)
In-Reply-To: <20260527085039.44435-1-lianfeng.ouyang@starfivetech.com>

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.

  parent reply	other threads:[~2026-05-27 12:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-27  8:50 [PATCH v2 0/3] i2c: Add Starfive JHB100 I2C master/slave support lianfeng.ouyang
2026-05-27  8:50 ` [PATCH v2 1/3] dt-bindings: i2c: Add StarFive JHB100 I2C lianfeng.ouyang
2026-05-27 13:04   ` Krzysztof Kozlowski
2026-05-28  6:57     ` 回复: " Lianfeng Ouyang
2026-05-28  7:47       ` Krzysztof Kozlowski
2026-05-28  8:30         ` 回复: " Lianfeng Ouyang
2026-05-27  8:50 ` [PATCH v2 2/3] i2c: designware: Export symbols and add __weak for Starfive I2C driver lianfeng.ouyang
2026-05-27 13:07   ` Krzysztof Kozlowski
2026-05-28  2:36     ` 回复: " Lianfeng Ouyang
2026-05-28  7:46       ` Krzysztof Kozlowski
2026-05-28  8:28         ` 回复: " Lianfeng Ouyang
2026-05-27  8:50 ` [PATCH v2 3/3] i2c: starfive: Add StarFive JHB100 I2C master/slave support lianfeng.ouyang
2026-05-27 12:36 ` Mika Westerberg [this message]
2026-05-28  2:41   ` 回复: [PATCH v2 0/3] i2c: Add Starfive " Lianfeng Ouyang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260527123620.GD3102@black.igk.intel.com \
    --to=mika.westerberg@linux.intel.com \
    --cc=andi.shyti@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lianfeng.ouyang@starfivetech.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox