2023-08-09 01:45:15

by Jeremy Kerr

[permalink] [raw]
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/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


2023-08-09 05:02:38

by Dylan Hung

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

Hi Jeremy,

> -----Original Message-----
> From: Jeremy Kerr [mailto:[email protected]]
> Sent: Wednesday, August 9, 2023 8:08 AM
> To: Dylan Hung <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: BMC-SW <[email protected]>; [email protected]
> 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