Devicetree
 help / color / mirror / Atom feed
* [PATCH 0/2] i2c: rtl9300: remove IO_MODE configuration
@ 2026-10-04 15:32 Michael Zavertkin
  2026-10-04 15:32 ` [PATCH 1/2] " Michael Zavertkin
  2026-10-04 15:32 ` [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl Michael Zavertkin
  0 siblings, 2 replies; 9+ messages in thread
From: Michael Zavertkin @ 2026-10-04 15:32 UTC (permalink / raw)
  To: linux-i2c
  Cc: Rustam Adilov, Chris Packham, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel,
	Michael Zavertkin

i2c driver for RTL9607C used to configure IO_MODE register. The register
is global and responsible for configuring SoC pins, therefore it should
belong to pinctrl driver.

SoC-specific pinctrl driver is WIP (waiting for these patches in OpenWRT),
but pinctrl-single could be used instead.

Remove scl configuration (used only to modify IO_MODE register) and
reflect these changes in Documentation.

Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
---
Michael Zavertkin (2):
  i2c: rtl9300: remove IO_MODE configuration
  dt-bindings: i2c: rtl9300: add pinctrl

 .../devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml |  7 ++++++-
 drivers/i2c/busses/i2c-rtl9300.c                     | 12 +++---------
 2 files changed, 9 insertions(+), 10 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] i2c: rtl9300: remove IO_MODE configuration
  2026-10-04 15:32 [PATCH 0/2] i2c: rtl9300: remove IO_MODE configuration Michael Zavertkin
@ 2026-10-04 15:32 ` Michael Zavertkin
  2026-10-04 15:32 ` [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl Michael Zavertkin
  1 sibling, 0 replies; 9+ messages in thread
From: Michael Zavertkin @ 2026-10-04 15:32 UTC (permalink / raw)
  To: linux-i2c
  Cc: Rustam Adilov, Chris Packham, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel,
	Michael Zavertkin

IO_MODE is used to route peripheral controller to SoC pins. This is a
job for pinctrl driver.

Since select_scl now have nothing to do, remove it.

Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
---
 drivers/i2c/busses/i2c-rtl9300.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
index 3a8225b0666c..c36c29e1833e 100644
--- a/drivers/i2c/busses/i2c-rtl9300.c
+++ b/drivers/i2c/busses/i2c-rtl9300.c
@@ -130,7 +130,6 @@ struct rtl9300_i2c_xfer {
 #define RTL9310_I2C_MST_DATA_CTRL			0x8
 
 #define RTL9607_I2C_CONFIG				0x22f50
-#define RTL9607_IO_MODE_EN				0x23014
 #define RTL9607_I2C_IND_WD				0x0
 #define RTL9607_I2C_IND_ADR				0x8
 #define RTL9607_I2C_IND_CMD				0x10
@@ -190,7 +189,6 @@ static int rtl9300_i2c_config_chan(struct rtl9300_i2c *i2c, struct rtl9300_i2c_c
 
 static int rtl9607_i2c_config_chan(struct rtl9300_i2c *i2c, struct rtl9300_i2c_chan *chan)
 {
-	const struct rtl9300_i2c_drv_data *drv_data;
 	int ret;
 
 	if (i2c->sda_num == chan->sda_num)
@@ -200,11 +198,6 @@ static int rtl9607_i2c_config_chan(struct rtl9300_i2c *i2c, struct rtl9300_i2c_c
 	if (ret)
 		return ret;
 
-	drv_data = device_get_match_data(i2c->dev);
-	ret = drv_data->select_scl(i2c, i2c->scl_num);
-	if (ret)
-		return ret;
-
 	i2c->sda_num = chan->sda_num;
 	return 0;
 }
@@ -619,7 +612,6 @@ static const struct rtl9300_i2c_drv_data rtl9310_i2c_drv_data = {
 
 static const struct rtl9300_i2c_drv_data rtl9607_i2c_drv_data = {
 	.field_desc = {
-		[F_SCL_SEL]		= GLB_REG_FIELD(RTL9607_IO_MODE_EN, 13, 14),
 		[F_EXT_SCK_5MS]		= MST_REG_FIELD(RTL9607_I2C_CONFIG, 26, 26),
 		[F_DEV_ADDR]		= MST_REG_FIELD(RTL9607_I2C_CONFIG, 14, 20),
 		[F_MEM_ADDR_WIDTH]	= MST_REG_FIELD(RTL9607_I2C_CONFIG, 12, 13),
@@ -631,7 +623,9 @@ static const struct rtl9300_i2c_drv_data rtl9607_i2c_drv_data = {
 		[F_I2C_TRIG]		= MST_REG_FIELD(RTL9607_I2C_IND_CMD, 0, 0),
 		[F_MEM_ADDR]		= MST_REG_FIELD(RTL9607_I2C_IND_ADR, 0, 31),
 	},
-	.select_scl = rtl9310_i2c_select_scl,
+	/*
+	 * No select_scl - single channel available
+	 */
 	.config_chan = rtl9607_i2c_config_chan,
 	.config_clock = rtl9607_i2c_config_clock,
 	.misc_init = rtl9607_i2c_init,
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl
  2026-10-04 15:32 [PATCH 0/2] i2c: rtl9300: remove IO_MODE configuration Michael Zavertkin
  2026-10-04 15:32 ` [PATCH 1/2] " Michael Zavertkin
@ 2026-10-04 15:32 ` Michael Zavertkin
  2026-10-04 15:55   ` sashiko-bot
  2026-10-05 10:38   ` Conor Dooley
  1 sibling, 2 replies; 9+ messages in thread
From: Michael Zavertkin @ 2026-10-04 15:32 UTC (permalink / raw)
  To: linux-i2c
  Cc: Rustam Adilov, Chris Packham, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel,
	Michael Zavertkin

For RTL9607C driver used to configure IO_MODE register. This register is
global and is a scope of pinctrl driver (either pinctrl-single or WIP
driver for SoC).

Drop realtek,scl requirement for that SoC and require pinctrl instead.

Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
---
 .../devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml       | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml b/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
index 5873cfdc5b3e..ad143509aa42 100644
--- a/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
@@ -57,6 +57,10 @@ properties:
   clocks:
     maxItems: 1
 
+  pinctrl-0: true
+
+  pinctrl-names: true
+
 patternProperties:
   '^i2c@[0-9ab]$':
     $ref: /schemas/i2c/i2c-controller.yaml
@@ -94,8 +98,9 @@ allOf:
             const: realtek,rtl9607-i2c
     then:
       required:
-        - realtek,scl
         - clocks
+        - pinctrl-0
+        - pinctrl-names
 
 required:
   - compatible
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl
  2026-10-04 15:32 ` [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl Michael Zavertkin
@ 2026-10-04 15:55   ` sashiko-bot
  2026-10-05 10:38   ` Conor Dooley
  1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-10-04 15:55 UTC (permalink / raw)
  To: Michael Zavertkin; +Cc: robh, conor+dt, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] dt-bindings: ABI breakage by making pinctrl required for rtl9607-i2c
--

commit b162c61d2fc7ab941d4ea49cc274452fca8d089d
Author: Michael Zavertkin <misha.zavertkin@mail.ru>

dt-bindings: i2c: rtl9300: add pinctrl

For RTL9607C driver used to configure IO_MODE register. This register is
global and is a scope of pinctrl driver (either pinctrl-single or WIP
driver for SoC). Drop realtek,scl requirement for that SoC and require
pinctrl instead.

> diff --git a/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml b/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
> index 5873cfdc5b3e0..ad143509aa425 100644
> --- a/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
[ ... ]
> @@ -94,8 +98,9 @@ allOf:
>              const: realtek,rtl9607-i2c
>      then:
>        required:
> -        - realtek,scl
>          - clocks
> +        - pinctrl-0
> +        - pinctrl-names

[Severity: Critical]
Does making these properties strictly required introduce a regression for
existing device trees?

Device tree bindings are considered stable ABI, and changes must be backward
compatible. The realtek,rtl9607-i2c compatible string was already present in
the bindings. Existing valid device trees for this SoC do not contain
pinctrl-0 or pinctrl-names because the previous schema used
additionalProperties: false without defining them.

Adding pinctrl-0 and pinctrl-names to the required list in the allOf block
for rtl9607-i2c means all existing, deployed device trees will now fail
schema validation during dtbs_check.

Could these new properties be made optional to prevent ABI breakage?

>  
>  required:
>    - compatible

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791127538.git.misha.zavertkin@mail.ru?part=2

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl
  2026-10-04 15:32 ` [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl Michael Zavertkin
  2026-10-04 15:55   ` sashiko-bot
@ 2026-10-05 10:38   ` Conor Dooley
  2026-10-05 13:53     ` Michael Zavertkin
  1 sibling, 1 reply; 9+ messages in thread
From: Conor Dooley @ 2026-10-05 10:38 UTC (permalink / raw)
  To: Michael Zavertkin
  Cc: linux-i2c, Rustam Adilov, Chris Packham, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]

On Sun, Oct 04, 2026 at 10:32:35PM +0700, Michael Zavertkin wrote:
> For RTL9607C driver used to configure IO_MODE register. This register is
> global and is a scope of pinctrl driver (either pinctrl-single or WIP
> driver for SoC).
> 
> Drop realtek,scl requirement for that SoC and require pinctrl instead.

This patch itself is probably fine, but the driver changes will cause
regressions because all existing devicetrees are going to have
realtek,scl and not the pinctrl properties. You need to retain the
driver code and make the register setting optional depending on if the
scl is there (or similar).
pw-bot: changes-requested

Thanks,
Conor.

> 
> Signed-off-by: Michael Zavertkin <misha.zavertkin@mail.ru>
> ---
>  .../devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml       | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml b/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
> index 5873cfdc5b3e..ad143509aa42 100644
> --- a/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml
> @@ -57,6 +57,10 @@ properties:
>    clocks:
>      maxItems: 1
>  
> +  pinctrl-0: true
> +
> +  pinctrl-names: true
> +
>  patternProperties:
>    '^i2c@[0-9ab]$':
>      $ref: /schemas/i2c/i2c-controller.yaml
> @@ -94,8 +98,9 @@ allOf:
>              const: realtek,rtl9607-i2c
>      then:
>        required:
> -        - realtek,scl
>          - clocks
> +        - pinctrl-0
> +        - pinctrl-names
>  
>  required:
>    - compatible
> -- 
> 2.54.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl
  2026-10-05 10:38   ` Conor Dooley
@ 2026-10-05 13:53     ` Michael Zavertkin
  2026-10-07 10:12       ` Conor Dooley
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Zavertkin @ 2026-10-05 13:53 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-i2c, devicetree, linux-kernel, Chris Packham, Andi Shyti,
	Rob Herring, Krzysztof Kozlowski, Rustam Adilov

On Mon, Oct 05, 2026 at 11:38:51AM +0100, Conor Dooley wrote:
> On Sun, Oct 04, 2026 at 10:32:35PM +0700, Michael Zavertkin wrote:
> > For RTL9607C driver used to configure IO_MODE register. This register is
> > global and is a scope of pinctrl driver (either pinctrl-single or WIP
> > driver for SoC).
> > 
> > Drop realtek,scl requirement for that SoC and require pinctrl instead.
> 
> This patch itself is probably fine, but the driver changes will cause
> regressions because all existing devicetrees are going to have
> realtek,scl and not the pinctrl properties. You need to retain the
> driver code and make the register setting optional depending on if the
> scl is there (or similar).
> pw-bot: changes-requested
> 
> Thanks,
> Conor.

I understand backward compatibilyti requirement. However all known users
of this driver are here (me and Rustam Adilov). Does compatibility
strictly required for such case?

If so, what would be better - print warning about realtek,scl (only for
RTL9607C) and work as earlier, potentially interfering with pinctrl
driver - or abort probe with error if there is no pinctrl property?
Explicit error or message is better then silent regression, if I
understand you correctly.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl
  2026-10-05 13:53     ` Michael Zavertkin
@ 2026-10-07 10:12       ` Conor Dooley
  2026-10-07 13:33         ` Michael Zavertkin
  0 siblings, 1 reply; 9+ messages in thread
From: Conor Dooley @ 2026-10-07 10:12 UTC (permalink / raw)
  To: Michael Zavertkin
  Cc: linux-i2c, devicetree, linux-kernel, Chris Packham, Andi Shyti,
	Rob Herring, Krzysztof Kozlowski, Rustam Adilov

[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]

On Mon, Oct 05, 2026 at 08:53:05PM +0700, Michael Zavertkin wrote:
> On Mon, Oct 05, 2026 at 11:38:51AM +0100, Conor Dooley wrote:
> > On Sun, Oct 04, 2026 at 10:32:35PM +0700, Michael Zavertkin wrote:
> > > For RTL9607C driver used to configure IO_MODE register. This register is
> > > global and is a scope of pinctrl driver (either pinctrl-single or WIP
> > > driver for SoC).
> > > 
> > > Drop realtek,scl requirement for that SoC and require pinctrl instead.
> > 
> > This patch itself is probably fine, but the driver changes will cause
> > regressions because all existing devicetrees are going to have
> > realtek,scl and not the pinctrl properties. You need to retain the
> > driver code and make the register setting optional depending on if the
> > scl is there (or similar).
> > pw-bot: changes-requested
> > 
> > Thanks,
> > Conor.
> 
> I understand backward compatibilyti requirement. However all known users
> of this driver are here (me and Rustam Adilov). Does compatibility
> strictly required for such case?

How do you know there's only the two of you?

> If so, what would be better - print warning about realtek,scl (only for
> RTL9607C) and work as earlier, potentially interfering with pinctrl
> driver - or abort probe with error if there is no pinctrl property?
> Explicit error or message is better then silent regression, if I
> understand you correctly.

If there's genuinely only two users, then I suppose you can break it.
But two known users doesn't meant two users. Otherwise, use pintrl if
available and use the fallback to the existing property if not. Mark the
property deprecated and probably emit a warning.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl
  2026-10-07 10:12       ` Conor Dooley
@ 2026-10-07 13:33         ` Michael Zavertkin
  2026-10-07 14:01           ` Conor Dooley
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Zavertkin @ 2026-10-07 13:33 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-i2c, devicetree, linux-kernel, Chris Packham, Andi Shyti,
	Rob Herring, Krzysztof Kozlowski, Rustam Adilov

On Wed, Oct 07, 2026 at 11:12:36AM +0100, Conor Dooley wrote:
> > If so, what would be better - print warning about realtek,scl (only for
> > RTL9607C) and work as earlier, potentially interfering with pinctrl
> > driver - or abort probe with error if there is no pinctrl property?
> > Explicit error or message is better then silent regression, if I
> > understand you correctly.
> 
> If there's genuinely only two users, then I suppose you can break it.
> But two known users doesn't meant two users. Otherwise, use pintrl if
> available and use the fallback to the existing property if not. Mark the
> property deprecated and probably emit a warning.

Okay, I'll prepare v2. Support for deprecated property should be removed
somewhere in future, right?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl
  2026-10-07 13:33         ` Michael Zavertkin
@ 2026-10-07 14:01           ` Conor Dooley
  0 siblings, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2026-10-07 14:01 UTC (permalink / raw)
  To: Michael Zavertkin
  Cc: linux-i2c, devicetree, linux-kernel, Chris Packham, Andi Shyti,
	Rob Herring, Krzysztof Kozlowski, Rustam Adilov

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

On Wed, Oct 07, 2026 at 08:33:46PM +0700, Michael Zavertkin wrote:
> On Wed, Oct 07, 2026 at 11:12:36AM +0100, Conor Dooley wrote:
> > > If so, what would be better - print warning about realtek,scl (only for
> > > RTL9607C) and work as earlier, potentially interfering with pinctrl
> > > driver - or abort probe with error if there is no pinctrl property?
> > > Explicit error or message is better then silent regression, if I
> > > understand you correctly.
> > 
> > If there's genuinely only two users, then I suppose you can break it.
> > But two known users doesn't meant two users. Otherwise, use pintrl if
> > available and use the fallback to the existing property if not. Mark the
> > property deprecated and probably emit a warning.
> 
> Okay, I'll prepare v2. Support for deprecated property should be removed
> somewhere in future, right?

Sure

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-10-07 14:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-04 15:32 [PATCH 0/2] i2c: rtl9300: remove IO_MODE configuration Michael Zavertkin
2026-10-04 15:32 ` [PATCH 1/2] " Michael Zavertkin
2026-10-04 15:32 ` [PATCH 2/2] dt-bindings: i2c: rtl9300: add pinctrl Michael Zavertkin
2026-10-04 15:55   ` sashiko-bot
2026-10-05 10:38   ` Conor Dooley
2026-10-05 13:53     ` Michael Zavertkin
2026-10-07 10:12       ` Conor Dooley
2026-10-07 13:33         ` Michael Zavertkin
2026-10-07 14:01           ` Conor Dooley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox