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

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

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

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