[PATCH 0/3] i2c: spacemit: add reset support

[PATCH 0/3] i2c: spacemit: add reset support

[Encrow Thorne]

Add reset control to SpacemiT K1 I2C.

Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
---
Encrow Thorne (3):
      dt-bindings: i2c: spacemit: add optional resets
      i2c: k1: add reset support
      riscv: dts: spacemit: add reset property

 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml |  3 +++
 arch/riscv/boot/dts/spacemit/k1.dtsi                       |  2 ++
 drivers/i2c/busses/i2c-k1.c                                | 11 +++++++++++
 3 files changed, 16 insertions(+)
---
base-commit: 8fec172c82c2b5f6f8e47ab837c1dc91ee3d1b87
change-id: 20251104-i2c-k1_reset-support-12784e17bf83

Best regards,
-- 
Encrow Thorne <jyc0019@gmail.com>

[PATCH 1/3] dt-bindings: i2c: spacemit: add optional resets

[Encrow Thorne]

Add optional reset property for the SpacemiT K1 I2C.

Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
---
 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml b/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
index b7220fff2235..3d4faec41abe 100644
--- a/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
@@ -27,6 +27,9 @@ properties:
       - description: I2C Functional Clock
       - description: APB Bus Clock
 
+  resets:
+    maxItems: 1
+
   clock-names:
     items:
       - const: func

-- 
2.25.1

Re: [PATCH 1/3] dt-bindings: i2c: spacemit: add optional resets

[Troy Mitchell]

On Wed, Nov 19, 2025 at 07:46:43PM +0800, Encrow Thorne wrote:
> Add optional reset property for the SpacemiT K1 I2C.
> 
> Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
>
Thanks.

Acked-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>

Re: [PATCH 1/3] dt-bindings: i2c: spacemit: add optional resets

[Krzysztof Kozlowski]

On Wed, Nov 19, 2025 at 07:46:43PM +0800, Encrow Thorne wrote:
> Add optional reset property for the SpacemiT K1 I2C.

Why? You just repeated diff and we can read the diff. Please explain WHY
you are doing things in terms of hardware.

> 
> Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
> ---
>  Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml b/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
> index b7220fff2235..3d4faec41abe 100644
> --- a/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
> @@ -27,6 +27,9 @@ properties:
>        - description: I2C Functional Clock
>        - description: APB Bus Clock
>  
> +  resets:
> +    maxItems: 1
> +

Completely misplaced. There are never random properties between xxx and
xxx-names.

Best regards,
Krzysztof

[PATCH 2/3] i2c: k1: add reset support

[Encrow Thorne]

Add reset control handling to the K1 I2C driver.

Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
---
 drivers/i2c/busses/i2c-k1.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
index 6b918770e612..64d817d8315d 100644
--- a/drivers/i2c/busses/i2c-k1.c
+++ b/drivers/i2c/busses/i2c-k1.c
@@ -10,6 +10,7 @@
  #include <linux/module.h>
  #include <linux/of_address.h>
  #include <linux/platform_device.h>
+ #include <linux/reset.h>
 
 /* spacemit i2c registers */
 #define SPACEMIT_ICR		 0x0		/* Control register */
@@ -113,6 +114,7 @@ struct spacemit_i2c_dev {
 	void __iomem *base;
 	int irq;
 	u32 clock_freq;
+	struct reset_control *resets;
 
 	struct i2c_msg *msgs;
 	u32 msg_num;
@@ -571,6 +573,15 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(clk))
 		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock");
 
+	i2c->resets = devm_reset_control_get_optional(dev, NULL);
+	if (IS_ERR(i2c->resets))
+		return dev_err_probe(dev, PTR_ERR(i2c->resets),
+				    "failed to get reset\n");
+
+	reset_control_assert(i2c->resets);
+	udelay(2);
+	reset_control_deassert(i2c->resets);
+
 	spacemit_i2c_reset(i2c);
 
 	i2c_set_adapdata(&i2c->adapt, i2c);

-- 
2.25.1

Re: [PATCH 2/3] i2c: k1: add reset support

[Troy Mitchell]

Hi Encrow,

On Wed, Nov 19, 2025 at 07:46:44PM +0800, Encrow Thorne wrote:
> Add reset control handling to the K1 I2C driver.
> 
> Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
> ---
>  drivers/i2c/busses/i2c-k1.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
> index 6b918770e612..64d817d8315d 100644
> --- a/drivers/i2c/busses/i2c-k1.c
> +++ b/drivers/i2c/busses/i2c-k1.c
> @@ -10,6 +10,7 @@
>   #include <linux/module.h>
>   #include <linux/of_address.h>
>   #include <linux/platform_device.h>
> + #include <linux/reset.h>
>  
>  /* spacemit i2c registers */
>  #define SPACEMIT_ICR		 0x0		/* Control register */
> @@ -113,6 +114,7 @@ struct spacemit_i2c_dev {
>  	void __iomem *base;
>  	int irq;
>  	u32 clock_freq;
> +	struct reset_control *resets;
>  
>  	struct i2c_msg *msgs;
>  	u32 msg_num;
> @@ -571,6 +573,15 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
>  	if (IS_ERR(clk))
>  		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock");
>  
> +	i2c->resets = devm_reset_control_get_optional(dev, NULL);
> +	if (IS_ERR(i2c->resets))
> +		return dev_err_probe(dev, PTR_ERR(i2c->resets),
> +				    "failed to get reset\n");
Please align.

> +
> +	reset_control_assert(i2c->resets);
> +	udelay(2);
This seems to be a very small value. If this
has been verified multiple times?

                        - Troy
> +	reset_control_deassert(i2c->resets);
> +
>  	spacemit_i2c_reset(i2c);
>  
>  	i2c_set_adapdata(&i2c->adapt, i2c);
> 
> -- 
> 2.25.1
> 
> 

Re: [PATCH 2/3] i2c: k1: add reset support

[Guodong Xu]

On Wed, Nov 19, 2025 at 07:46:44PM +0800, Encrow Thorne wrote:
> Add reset control handling to the K1 I2C driver.
>
> Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
> ---
>  drivers/i2c/busses/i2c-k1.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
> index 6b918770e612..64d817d8315d 100644
> --- a/drivers/i2c/busses/i2c-k1.c
> +++ b/drivers/i2c/busses/i2c-k1.c
> @@ -10,6 +10,7 @@
>   #include <linux/module.h>
>   #include <linux/of_address.h>
>   #include <linux/platform_device.h>
> + #include <linux/reset.h>
>
>  /* spacemit i2c registers */
>  #define SPACEMIT_ICR		 0x0		/* Control register */
> @@ -113,6 +114,7 @@ struct spacemit_i2c_dev {
>  	void __iomem *base;
>  	int irq;
>  	u32 clock_freq;
> +	struct reset_control *resets;

Thanks for the patch! A few suggestions to simplify this:

The reset_control structure doesn't need to be stored in the device
struct since the devm API automatically manages the resource lifecycle.

>
>  	struct i2c_msg *msgs;
>  	u32 msg_num;
> @@ -571,6 +573,15 @@ static int spacemit_i2c_probe(struct platform_device *pdev)

Please use a local variable in the probe function instead:

       struct reset_control *rst;

>  	if (IS_ERR(clk))
>  		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock");
>
> +	i2c->resets = devm_reset_control_get_optional(dev, NULL);

There is a new API, I would suggest this:
       rst = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);


> +	if (IS_ERR(i2c->resets))
> +		return dev_err_probe(dev, PTR_ERR(i2c->resets),
> +				    "failed to get reset\n");
> +
> +	reset_control_assert(i2c->resets);
> +	udelay(2);
> +	reset_control_deassert(i2c->resets);
> +

Why you need to call assert() then deassert()?
Wouldn't the single API fit?

Best regards,
Guodong

>  	spacemit_i2c_reset(i2c);
>
>  	i2c_set_adapdata(&i2c->adapt, i2c);
>

Re: [PATCH 2/3] i2c: k1: add reset support

[Encrow Thorne]

On Thu, Dec 11, 2025 at 03:35:26PM +0800, Guodong Xu wrote:
> On Wed, Nov 19, 2025 at 07:46:44PM +0800, Encrow Thorne wrote:
> > Add reset control handling to the K1 I2C driver.
> >
> > Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
> > ---
> >  drivers/i2c/busses/i2c-k1.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
> > index 6b918770e612..64d817d8315d 100644
> > --- a/drivers/i2c/busses/i2c-k1.c
> > +++ b/drivers/i2c/busses/i2c-k1.c
> > @@ -10,6 +10,7 @@
> >   #include <linux/module.h>
> >   #include <linux/of_address.h>
> >   #include <linux/platform_device.h>
> > + #include <linux/reset.h>
> >
> >  /* spacemit i2c registers */
> >  #define SPACEMIT_ICR		 0x0		/* Control register */
> > @@ -113,6 +114,7 @@ struct spacemit_i2c_dev {
> >  	void __iomem *base;
> >  	int irq;
> >  	u32 clock_freq;
> > +	struct reset_control *resets;
> 
> Thanks for the patch! A few suggestions to simplify this:
> 
> The reset_control structure doesn't need to be stored in the device
> struct since the devm API automatically manages the resource lifecycle.
> 
> >
> >  	struct i2c_msg *msgs;
> >  	u32 msg_num;
> > @@ -571,6 +573,15 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
> 
> Please use a local variable in the probe function instead:
> 
>        struct reset_control *rst;
> 
> >  	if (IS_ERR(clk))
> >  		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock");
> >
> > +	i2c->resets = devm_reset_control_get_optional(dev, NULL);
>
  
> There is a new API, I would suggest this:
>        rst = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);
> 
> 
> > +	if (IS_ERR(i2c->resets))
> > +		return dev_err_probe(dev, PTR_ERR(i2c->resets),
> > +				    "failed to get reset\n");
> > +
> > +	reset_control_assert(i2c->resets);
> > +	udelay(2);
> > +	reset_control_deassert(i2c->resets);
> > +
> 
> Why you need to call assert() then deassert()?
> Wouldn't the single API fit?
> 
 Good point.
 
 Thank you for your suggestions, I will fix them in v2.

 				- Encrow
> Best regards,
> Guodong
> 
> >  	spacemit_i2c_reset(i2c);
> >
> >  	i2c_set_adapdata(&i2c->adapt, i2c);
> >

[PATCH 3/3] riscv: dts: spacemit: add reset property

[Encrow Thorne]

Add resets property to K1 I2C node.

Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
---
 arch/riscv/boot/dts/spacemit/k1.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
index af35f9cd6435..2b84dc276ace 100644
--- a/arch/riscv/boot/dts/spacemit/k1.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
@@ -468,6 +468,7 @@ i2c2: i2c@d4012000 {
 				 <&syscon_apbc CLK_TWSI2_BUS>;
 			clock-names = "func", "bus";
 			clock-frequency = <400000>;
+			resets = <&syscon_apbc RESET_TWSI2>;
 			interrupts = <38>;
 			status = "disabled";
 		};
@@ -481,6 +482,7 @@ i2c8: i2c@d401d800 {
 				 <&syscon_apbc CLK_TWSI8_BUS>;
 			clock-names = "func", "bus";
 			clock-frequency = <400000>;
+			resets = <&syscon_apbc RESET_TWSI8>;
 			interrupts = <19>;
 			status = "disabled";
 		};

-- 
2.25.1

Re: [PATCH 3/3] riscv: dts: spacemit: add reset property

[Troy Mitchell]

On Wed, Nov 19, 2025 at 07:46:45PM +0800, Encrow Thorne wrote:
> Add resets property to K1 I2C node.
> 
> Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
> ---
>  arch/riscv/boot/dts/spacemit/k1.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index af35f9cd6435..2b84dc276ace 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -468,6 +468,7 @@ i2c2: i2c@d4012000 {
>  				 <&syscon_apbc CLK_TWSI2_BUS>;
>  			clock-names = "func", "bus";
>  			clock-frequency = <400000>;
> +			resets = <&syscon_apbc RESET_TWSI2>;
>  			interrupts = <38>;
>  			status = "disabled";
>  		};
> @@ -481,6 +482,7 @@ i2c8: i2c@d401d800 {
>  				 <&syscon_apbc CLK_TWSI8_BUS>;
>  			clock-names = "func", "bus";
>  			clock-frequency = <400000>;
> +			resets = <&syscon_apbc RESET_TWSI8>;
>  			interrupts = <19>;
>  			status = "disabled";
>  		};
Since this patch has been merged [1], you can add resets property in all
i2c node.

                                    - Troy

Link: https://lore.kernel.org/all/176294559674.467363.5417599191009488468.b4-ty@gentoo.org/ [1]
> 
> -- 
> 2.25.1
> 
> 

[PATCH v3 0/3] i2c: spacemit: add reset support

[Encrow Thorne]

Add reset support for the K1 I2C driver. A reset ensures that the
controller starts in a clean and known state.

Reset ensures that the I2C hardware is in a clean state. We cannot assume
that no program used I2C before the kernel booted.

Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
---
Changes in v3:
- Move reset property in dt-bindings.
- Use devm_reset_control_get_optional_exclusive_deasserted() instead.
- Rebase to v6.19-rc1.
- Link to v2: https://lore.kernel.org/r/20251219-i2c-reset-v2-0-ad201a602e74@gmail.com

Changes in v2:
- Replace reset property in dt-bindings.
- Use devm_reset_control_get_optional_exclusive_deasserted() instead.
- Rebase to v6.19-rc1.
- Link to v1: https://lore.kernel.org/r/20251119-i2c-k1_reset-support-v1-0-0e9e82bf9b65@gmail.com

---
Encrow Thorne (3):
      dt-bindings: i2c: spacemit: add optional resets
      i2c: k1: add reset support
      riscv: dts: spacemit: add reset property

 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml | 3 +++
 arch/riscv/boot/dts/spacemit/k1.dtsi                       | 8 ++++++++
 drivers/i2c/busses/i2c-k1.c                                | 7 +++++++
 3 files changed, 18 insertions(+)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251218-i2c-reset-a7be139213de

Best regards,
-- 
Encrow Thorne <jyc0019@gmail.com>

[PATCH 1/3] dt-bindings: i2c: spacemit: add optional resets

[Encrow Thorne]

The I2C controller requires a reset to ensure it starts from a clean state.

Add the 'resets' property to support this hardware requirement.

Reviewed-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
---
 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml b/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
index b7220fff2235..5896fb120501 100644
--- a/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
@@ -41,6 +41,9 @@ properties:
     default: 400000
     maximum: 3300000
 
+  resets:
+    maxItems: 1
+
 required:
   - compatible
   - reg
-- 
2.25.1

[PATCH 2/3] i2c: k1: add reset support

[Encrow Thorne]

The K1 I2C controller provides a reset line that needs to be deasserted
before the controller can be accessed.

Add reset support to the driver to ensure the controller starts in the
required state.

Reviewed-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
Signed-off-by: Encrow Thorne <jyc0019@gmail.com>
---
 drivers/i2c/busses/i2c-k1.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/i2c/busses/i2c-k1.c b/drivers/i2c/busses/i2c-k1.c
index d42c03ef5db5..23661c7ddb67 100644
--- a/drivers/i2c/busses/i2c-k1.c
+++ b/drivers/i2c/busses/i2c-k1.c
@@ -10,6 +10,7 @@
  #include <linux/module.h>
  #include <linux/of_address.h>
  #include <linux/platform_device.h>
+ #include <linux/reset.h>
 
 /* spacemit i2c registers */
 #define SPACEMIT_ICR		 0x0		/* Control register */
@@ -534,6 +535,7 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *of_node = pdev->dev.of_node;
 	struct spacemit_i2c_dev *i2c;
+	struct reset_control *rst;
 	int ret;
 
 	i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
@@ -578,6 +580,11 @@ static int spacemit_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(clk))
 		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock");
 
+	rst = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);
+	if (IS_ERR(rst))
+		return dev_err_probe(dev, PTR_ERR(rst),
+				     "failed to acquire deasserted reset\n");
+
 	spacemit_i2c_reset(i2c);
 
 	i2c_set_adapdata(&i2c->adapt, i2c);
-- 
2.25.1