[PATCH 0/3] Add Aspeed AST2600 I3C support

[PATCH 0/3] Add Aspeed AST2600 I3C support

[Dylan Hung]

This patch series introduces the necessary changes to enable I3C support
for the Aspeed AST2600 I3C controller. Specifically, it addresses the
missing pinctrl configuration and reset control for the I3C
functionality.

Dylan Hung (3):
  ARM: dts: pinctrl-aspeed-g6: Add I3C1 and I3C2 control pins
  dt-bindings: i3c: ast2600: Add resets and reset-names
  i3c: ast2600: Add reset deassertion for global registers

 .../devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml  | 12 ++++++++++--
 arch/arm/boot/dts/aspeed/aspeed-g6-pinctrl.dtsi      | 10 ++++++++++
 drivers/i3c/master/ast2600-i3c-master.c              |  9 +++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] ARM: dts: pinctrl-aspeed-g6: Add I3C1 and I3C2 control pins

[Dylan Hung]

Add pinctrl support for the I3C1 and I3C2 pins.

Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
---
 arch/arm/boot/dts/aspeed/aspeed-g6-pinctrl.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6-pinctrl.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6-pinctrl.dtsi
index 7cd4f075e325..289668f051eb 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-g6-pinctrl.dtsi
+++ b/arch/arm/boot/dts/aspeed/aspeed-g6-pinctrl.dtsi
@@ -297,6 +297,16 @@ pinctrl_i2c9_default: i2c9_default {
 		groups = "I2C9";
 	};
 
+	pinctrl_i3c1_default: i3c1_default {
+		function = "I3C1";
+		groups = "I3C1";
+	};
+
+	pinctrl_i3c2_default: i3c2_default {
+		function = "I3C2";
+		groups = "I3C2";
+	};
+
 	pinctrl_i3c3_default: i3c3_default {
 		function = "I3C3";
 		groups = "I3C3";
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] dt-bindings: i3c: ast2600: Add resets and reset-names

[Dylan Hung]

Add two reset lines to the AST2600 I3C controller:
- core_rst: the reset line of the controller itself
- global_rst: the reset line of the I3C global register block. Since all
six I3C controllers in AST2600 share this global register block, the
driver needs to handle this carefully. Generally, this reset line should
only need to be de-asserted.

Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
---
 .../devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml  | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml b/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
index fcc3dbff9c9a..3166d6f3a39c 100644
--- a/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
+++ b/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
@@ -23,7 +23,12 @@ properties:
     maxItems: 1
 
   resets:
-    maxItems: 1
+    maxItems: 2
+
+  reset-names:
+    items:
+      - const: core_rst
+      - const: global_rst
 
   interrupts:
     maxItems: 1
@@ -48,6 +53,8 @@ required:
   - compatible
   - reg
   - clocks
+  - resets
+  - reset-names
   - interrupts
   - aspeed,global-regs
 
@@ -63,7 +70,8 @@ examples:
         #address-cells = <3>;
         #size-cells = <0>;
         clocks = <&syscon 0>;
-        resets = <&syscon 0>;
+        resets = <&syscon 40>, <&syscon 39>;
+        reset-names = "core_rst", "global_rst";
         aspeed,global-regs = <&i3c_global 0>;
         pinctrl-names = "default";
         pinctrl-0 = <&pinctrl_i3c1_default>;
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] dt-bindings: i3c: ast2600: Add resets and reset-names

[Krzysztof Kozlowski]

On 08/08/2023 17:42, Dylan Hung wrote:
> Add two reset lines to the AST2600 I3C controller:
> - core_rst: the reset line of the controller itself
> - global_rst: the reset line of the I3C global register block. Since all
> six I3C controllers in AST2600 share this global register block, the
> driver needs to handle this carefully. Generally, this reset line should
> only need to be de-asserted.

The commit msg does not explain why this is now required.

> 
> Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
> ---
>  .../devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml  | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml b/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
> index fcc3dbff9c9a..3166d6f3a39c 100644
> --- a/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
> +++ b/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml
> @@ -23,7 +23,12 @@ properties:
>      maxItems: 1
>  
>    resets:
> -    maxItems: 1
> +    maxItems: 2
> +
> +  reset-names:
> +    items:
> +      - const: core_rst
> +      - const: global_rst
>  

Drop "_rst" suffixes from both.

>    interrupts:
>      maxItems: 1
> @@ -48,6 +53,8 @@ required:
>    - compatible
>    - reg
>    - clocks
> +  - resets
> +  - reset-names


Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] i3c: ast2600: Add reset deassertion for global registers

[Dylan Hung]

Add missing reset deassertion of the I3C global control registers.

Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
---
 drivers/i3c/master/ast2600-i3c-master.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/i3c/master/ast2600-i3c-master.c b/drivers/i3c/master/ast2600-i3c-master.c
index 09ed19d489e9..5d9d060134e0 100644
--- a/drivers/i3c/master/ast2600-i3c-master.c
+++ b/drivers/i3c/master/ast2600-i3c-master.c
@@ -11,6 +11,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 
 #include "dw-i3c-master.h"
 
@@ -128,6 +129,7 @@ static int ast2600_i3c_probe(struct platform_device *pdev)
 	struct device_node *np = pdev->dev.of_node;
 	struct of_phandle_args gspec;
 	struct ast2600_i3c *i3c;
+	struct reset_control *rst;
 	int rc;
 
 	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
@@ -156,6 +158,13 @@ static int ast2600_i3c_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "invalid sda-pullup value %d\n",
 			i3c->sda_pullup);
 
+	rst = devm_reset_control_get_shared(&pdev->dev, "global_rst");
+	if (IS_ERR(rst)) {
+		dev_err(&pdev->dev, "missing of invalid reset entry");
+		return PTR_ERR(rst);
+	}
+	reset_control_deassert(rst);
+
 	i3c->dw.platform_ops = &ast2600_i3c_ops;
 	i3c->dw.ibi_capable = true;
 	return dw_i3c_common_probe(&i3c->dw, pdev);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] i3c: ast2600: Add reset deassertion for global registers

[Krzysztof Kozlowski]

On 08/08/2023 17:42, Dylan Hung wrote:
> Add missing reset deassertion of the I3C global control registers.
> 
> Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
> ---
>  drivers/i3c/master/ast2600-i3c-master.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/i3c/master/ast2600-i3c-master.c b/drivers/i3c/master/ast2600-i3c-master.c
> index 09ed19d489e9..5d9d060134e0 100644
> --- a/drivers/i3c/master/ast2600-i3c-master.c
> +++ b/drivers/i3c/master/ast2600-i3c-master.c
> @@ -11,6 +11,7 @@
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> +#include <linux/reset.h>
>  
>  #include "dw-i3c-master.h"
>  
> @@ -128,6 +129,7 @@ static int ast2600_i3c_probe(struct platform_device *pdev)
>  	struct device_node *np = pdev->dev.of_node;
>  	struct of_phandle_args gspec;
>  	struct ast2600_i3c *i3c;
> +	struct reset_control *rst;
>  	int rc;
>  
>  	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
> @@ -156,6 +158,13 @@ static int ast2600_i3c_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "invalid sda-pullup value %d\n",
>  			i3c->sda_pullup);
>  
> +	rst = devm_reset_control_get_shared(&pdev->dev, "global_rst");
> +	if (IS_ERR(rst)) {
> +		dev_err(&pdev->dev, "missing of invalid reset entry");
> +		return PTR_ERR(rst);

return dev_err_probe

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] Add Aspeed AST2600 I3C support

[Jeremy Kerr]

Hi Dylan,

> This patch series introduces the necessary changes to enable I3C
> support for the Aspeed AST2600 I3C controller. Specifically, it addresses the
> missing pinctrl configuration and reset control for the I3C
> functionality.

+1 for the pinctrl changes for the I3C1 and I3C2 controllers (I'll
review and ack separately). I have been testing on I3C3 and up, but just
not with the HVI3C on 1 & 2, hence no pinctrl definition there.

However, I don't think the other two are needed.

For 2/3 and 3/3, you're adding a reset control for the global register
block within the per-controller driver, but we can already do that on a
global basis with the existing syscon device. Hence this earlier change:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/mfd/syscon.c?id=7d1e3bd94828ad9fc86f55253cd6fec8edd65394

For this, I have:

    &i3c {
        i3c_global: i3c-global {
            compatible = "aspeed,ast2600-i3c-global", "simple-mfd", "syscon";
            resets = <&syscon ASPEED_RESET_I3C_DMA>;
            reg = <0x0 0x1000>;
        };

        i3c2: i3c-master@4000 {
            compatible = "aspeed,ast2600-i3c";
            reg = <0x4000 0x1000>;
            clocks = <&syscon ASPEED_CLK_GATE_I3C2CLK>;
            pinctrl-names = "default";
            pinctrl-0 = <&pinctrl_i3c3_default>;
            interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
            aspeed,global-regs = <&i3c_global 2>;
            status = "disabled";
        };

        /* ... */
    };

- with no changes needed to any bindings. I haven't needed any other
resets; are there per-controller resets specified in the HW docs you
have?

Does that work for you? If you'd like to test, feel free to use my
sample dts at:

  https://github.com/CodeConstruct/linux/commit/05cac24705fa62d2176ecbbbf15d955cfe86e753

Cheers,


Jeremy

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] Add Aspeed AST2600 I3C support

[Jeremy Kerr]

Hi Dylan,

> - with no changes needed to any bindings. I haven't needed any other
> resets; are there per-controller resets specified in the HW docs you
> have?

... keeping in mind the reset control that the aspeed SCU driver
already provides for you, when you enable the appropriate clock gate:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-aspeed.c#n224

Cheers,


Jeremy

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 0/3] Add Aspeed AST2600 I3C support

[Dylan Hung]

Hi Jeremy,

> -----Original Message-----
> From: Jeremy Kerr [mailto:jk@codeconstruct.com.au]
> Sent: Wednesday, August 9, 2023 8:08 AM
> To: Dylan Hung <dylan_hung@aspeedtech.com>;
> alexandre.belloni@bootlin.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; joel@jms.id.au;
> andrew@aj.id.au; p.zabel@pengutronix.de; linux-i3c@lists.infradead.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Cc: BMC-SW <BMC-SW@aspeedtech.com>; kobedylan@gmail.com
> Subject: Re: [PATCH 0/3] Add Aspeed AST2600 I3C support
> 
> Hi Dylan,
> 
> > This patch series introduces the necessary changes to enable I3C
> > support for the Aspeed AST2600 I3C controller. Specifically, it
> > addresses the missing pinctrl configuration and reset control for the
> > I3C functionality.
> 
> +1 for the pinctrl changes for the I3C1 and I3C2 controllers (I'll
> review and ack separately). I have been testing on I3C3 and up, but just not
> with the HVI3C on 1 & 2, hence no pinctrl definition there.
> 
> However, I don't think the other two are needed.
> 
> For 2/3 and 3/3, you're adding a reset control for the global register block
> within the per-controller driver, but we can already do that on a global basis
> with the existing syscon device. Hence this earlier change:
> 
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/dri
> vers/mfd/syscon.c?id=7d1e3bd94828ad9fc86f55253cd6fec8edd65394
> 
> For this, I have:
> 
>     &i3c {
>         i3c_global: i3c-global {
>             compatible = "aspeed,ast2600-i3c-global", "simple-mfd",
> "syscon";
>             resets = <&syscon ASPEED_RESET_I3C_DMA>;
>             reg = <0x0 0x1000>;
>         };
> 
>         i3c2: i3c-master@4000 {
>             compatible = "aspeed,ast2600-i3c";
>             reg = <0x4000 0x1000>;
>             clocks = <&syscon ASPEED_CLK_GATE_I3C2CLK>;
>             pinctrl-names = "default";
>             pinctrl-0 = <&pinctrl_i3c3_default>;
>             interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
>             aspeed,global-regs = <&i3c_global 2>;
>             status = "disabled";
>         };
> 
>         /* ... */
>     };
> 
> - with no changes needed to any bindings. I haven't needed any other resets;
> are there per-controller resets specified in the HW docs you have?
> 
> Does that work for you? If you'd like to test, feel free to use my sample dts at:
> 
> 
> https://github.com/CodeConstruct/linux/commit/05cac24705fa62d2176ecbb
> bf15d955cfe86e753
> 
> Cheers,
> 
> 
> Jeremy
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] Add Aspeed AST2600 I3C support

[Jeremy Kerr]

Hi Dylan,

> Thank you for your review. I3C1 and I3C2 can only operate in low
> voltage (1.0V/1.2V), which is why there are no HVI3C1 and HVI3C2
> pinctrl definitions.

Yep, and that was config that I hadn't tested (so hadn't proposed
pinctrl definitions for those).

> > For 2/3 and 3/3, you're adding a reset control for the global
> > register block within the per-controller driver, but we can already
> > do that on a global basis with the existing syscon device. Hence
> > this earlier change:
>  
> I followed your recommendation and verified that it worked on my end.

OK, excellent!

> Should I resend the pinctrl patch as a stand-alone submission?

Yes, and feel free to add:

Reviewed-by: Jeremy Kerr <jk@codeconstruct.com.au>

Did your test use my i3c DTS definitions? If so, that's a decent
datapoint that the config works (on something other than my setup), and
so I'll submit upstream. Alternatively, feel free to include it with
your pinctrl change, if you like.

Cheers,


Jeremy

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel