* [PATCH] ARM: dts: aspeed-g6: Add nodes for i3c controllers
@ 2024-05-01 3:38 Jeremy Kerr
2024-05-01 10:24 ` Krzysztof Kozlowski
0 siblings, 1 reply; 5+ messages in thread
From: Jeremy Kerr @ 2024-05-01 3:38 UTC (permalink / raw
To: linux-aspeed
Cc: devicetree, Andrew Jeffery, Joel Stanley, Conor Dooley,
Rob Herring, Krzysztof Kozlowski
Add the i3c controller devices to the ast2600 g6 common dts. We add all
6 busses to the common g6 definition, but leave disabled through the
status property, to be enabled per-platform.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 93 +++++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
index 29f94696d8b1..f9d01599a965 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
@@ -866,6 +866,13 @@ i2c: bus@1e78a000 {
ranges = <0 0x1e78a000 0x1000>;
};
+ i3c: bus@1e7a0000 {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x1e7a0000 0x8000>;
+ };
+
fsim0: fsi@1e79b000 {
compatible = "aspeed,ast2600-fsi-master", "fsi-master";
reg = <0x1e79b000 0x94>;
@@ -1125,3 +1132,89 @@ i2c15: i2c-bus@800 {
status = "disabled";
};
};
+
+&i3c {
+ i3c_global: i3c-global {
+ compatible = "aspeed,ast2600-i3c-global", "syscon", "simple-mfd";
+ reg = <0x0 0x1000>;
+ resets = <&syscon ASPEED_RESET_I3C_DMA>;
+ };
+
+ i3c0: i3c@2000 {
+ compatible = "aspeed,ast2600-i3c";
+ reg = <0x2000 0x1000>;
+ #address-cells = <3>;
+ #size-cells = <0>;
+ clocks = <&syscon ASPEED_CLK_GATE_I3C0CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i3c1_default>;
+ interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
+ aspeed,global-regs = <&i3c_global 0>;
+ status = "disabled";
+ };
+
+ i3c1: i3c@3000 {
+ compatible = "aspeed,ast2600-i3c";
+ reg = <0x3000 0x1000>;
+ #address-cells = <3>;
+ #size-cells = <0>;
+ clocks = <&syscon ASPEED_CLK_GATE_I3C1CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i3c2_default>;
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+ aspeed,global-regs = <&i3c_global 1>;
+ status = "disabled";
+ };
+
+ i3c2: i3c@4000 {
+ compatible = "aspeed,ast2600-i3c";
+ reg = <0x4000 0x1000>;
+ #address-cells = <3>;
+ #size-cells = <0>;
+ 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";
+ };
+
+ i3c3: i3c@5000 {
+ compatible = "aspeed,ast2600-i3c";
+ reg = <0x5000 0x1000>;
+ #address-cells = <3>;
+ #size-cells = <0>;
+ clocks = <&syscon ASPEED_CLK_GATE_I3C3CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i3c4_default>;
+ interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+ aspeed,global-regs = <&i3c_global 3>;
+ status = "disabled";
+ };
+
+ i3c4: i3c@6000 {
+ compatible = "aspeed,ast2600-i3c";
+ reg = <0x6000 0x1000>;
+ #address-cells = <3>;
+ #size-cells = <0>;
+ clocks = <&syscon ASPEED_CLK_GATE_I3C4CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i3c5_default>;
+ interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+ aspeed,global-regs = <&i3c_global 4>;
+ status = "disabled";
+ };
+
+ i3c5: i3c@7000 {
+ compatible = "aspeed,ast2600-i3c";
+ reg = <0x7000 0x1000>;
+ #address-cells = <3>;
+ #size-cells = <0>;
+ clocks = <&syscon ASPEED_CLK_GATE_I3C5CLK>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i3c6_default>;
+ interrupts = <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>;
+ aspeed,global-regs = <&i3c_global 5>;
+ status = "disabled";
+ };
+};
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: dts: aspeed-g6: Add nodes for i3c controllers
2024-05-01 3:38 [PATCH] ARM: dts: aspeed-g6: Add nodes for i3c controllers Jeremy Kerr
@ 2024-05-01 10:24 ` Krzysztof Kozlowski
2024-05-01 11:17 ` Jeremy Kerr
0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-01 10:24 UTC (permalink / raw
To: Jeremy Kerr, linux-aspeed
Cc: devicetree, Andrew Jeffery, Joel Stanley, Conor Dooley,
Rob Herring, Krzysztof Kozlowski
On 01/05/2024 05:38, Jeremy Kerr wrote:
> Add the i3c controller devices to the ast2600 g6 common dts. We add all
> 6 busses to the common g6 definition, but leave disabled through the
> status property, to be enabled per-platform.
>
> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
> ---
> arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 93 +++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> index 29f94696d8b1..f9d01599a965 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> @@ -866,6 +866,13 @@ i2c: bus@1e78a000 {
> ranges = <0 0x1e78a000 0x1000>;
> };
>
> + i3c: bus@1e7a0000 {
> + compatible = "simple-bus";
What bus is it? Why is it even needed? If it is i3c, then for sure
compatible is wrong.
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x1e7a0000 0x8000>;
> + };
> +
> fsim0: fsi@1e79b000 {
> compatible = "aspeed,ast2600-fsi-master", "fsi-master";
> reg = <0x1e79b000 0x94>;
> @@ -1125,3 +1132,89 @@ i2c15: i2c-bus@800 {
> status = "disabled";
> };
> };
> +
> +&i3c {
????
That's not how we construct DTS. Overrides/extends of nodes are for
boards, not within DTSI.
Please provide full correct definition IN ONE place. See DTS coding style.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: dts: aspeed-g6: Add nodes for i3c controllers
2024-05-01 10:24 ` Krzysztof Kozlowski
@ 2024-05-01 11:17 ` Jeremy Kerr
2024-05-01 16:45 ` Krzysztof Kozlowski
0 siblings, 1 reply; 5+ messages in thread
From: Jeremy Kerr @ 2024-05-01 11:17 UTC (permalink / raw
To: Krzysztof Kozlowski, linux-aspeed
Cc: devicetree, Andrew Jeffery, Joel Stanley, Conor Dooley,
Rob Herring, Krzysztof Kozlowski
Hi Krzysztof,
> > --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> > +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
> > @@ -866,6 +866,13 @@ i2c: bus@1e78a000 {
> > ranges = <0 0x1e78a000 0x1000>;
> > };
> >
> > + i3c: bus@1e7a0000 {
> > + compatible = "simple-bus";
>
> What bus is it? Why is it even needed? If it is i3c, then for sure
> compatible is wrong.
This is not the i3c bus, it's the MMIO mapping that allows us to specify
the individual i3c controller mappings as sensible offsets into the main
address space. Did you miss the ranges property there?
This is following the existing design for the i2c controllers.
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges = <0 0x1e7a0000 0x8000>;
> > + };
> > +
> > fsim0: fsi@1e79b000 {
> > compatible = "aspeed,ast2600-fsi-master", "fsi-master";
> > reg = <0x1e79b000 0x94>;
> > @@ -1125,3 +1132,89 @@ i2c15: i2c-bus@800 {
> > status = "disabled";
> > };
> > };
> > +
> > +&i3c {
>
> ????
>
> That's not how we construct DTS. Overrides/extends of nodes are for
> boards, not within DTSI.
The overrides are occurring at the &i3cX labels, not &i3c. Platform
level dts just connect at those labels to define overrides for each bus:
&i3c0 {
status = "okay";
mctp-controller;
};
&i3c1 {
status = "okay";
mctp-controller;
};
There is existing precedence for this layout; the i2c and pinctrl
mappings already use dtsi-internal labels. It keeps the bus definitions
more manageable.
Cheers,
Jeremy
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: dts: aspeed-g6: Add nodes for i3c controllers
2024-05-01 11:17 ` Jeremy Kerr
@ 2024-05-01 16:45 ` Krzysztof Kozlowski
2024-05-02 11:10 ` Jeremy Kerr
0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-01 16:45 UTC (permalink / raw
To: Jeremy Kerr, linux-aspeed
Cc: devicetree, Andrew Jeffery, Joel Stanley, Conor Dooley,
Rob Herring, Krzysztof Kozlowski
On 01/05/2024 13:17, Jeremy Kerr wrote:
> Hi Krzysztof,
>
>>> --- a/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
>>> +++ b/arch/arm/boot/dts/aspeed/aspeed-g6.dtsi
>>> @@ -866,6 +866,13 @@ i2c: bus@1e78a000 {
>>> ranges = <0 0x1e78a000 0x1000>;
>>> };
>>>
>>> + i3c: bus@1e7a0000 {
>>> + compatible = "simple-bus";
>>
>> What bus is it? Why is it even needed? If it is i3c, then for sure
>> compatible is wrong.
>
> This is not the i3c bus, it's the MMIO mapping that allows us to specify
> the individual i3c controller mappings as sensible offsets into the main
> address space. Did you miss the ranges property there?
>
> This is following the existing design for the i2c controllers.
>
>>> + #address-cells = <1>;
>>> + #size-cells = <1>;
>>> + ranges = <0 0x1e7a0000 0x8000>;
>>> + };
>>> +
>>> fsim0: fsi@1e79b000 {
>>> compatible = "aspeed,ast2600-fsi-master", "fsi-master";
>>> reg = <0x1e79b000 0x94>;
>>> @@ -1125,3 +1132,89 @@ i2c15: i2c-bus@800 {
>>> status = "disabled";
>>> };
>>> };
>>> +
>>> +&i3c {
>>
>> ????
>>
>> That's not how we construct DTS. Overrides/extends of nodes are for
>> boards, not within DTSI.
>
> The overrides are occurring at the &i3cX labels, not &i3c. Platform
> level dts just connect at those labels to define overrides for each bus:
You miss the point. Look how DTS is constructed, read DTS coding style.
Your first node is empty and that is not readable.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: dts: aspeed-g6: Add nodes for i3c controllers
2024-05-01 16:45 ` Krzysztof Kozlowski
@ 2024-05-02 11:10 ` Jeremy Kerr
0 siblings, 0 replies; 5+ messages in thread
From: Jeremy Kerr @ 2024-05-02 11:10 UTC (permalink / raw
To: Krzysztof Kozlowski, linux-aspeed
Cc: devicetree, Andrew Jeffery, Joel Stanley, Conor Dooley,
Rob Herring, Krzysztof Kozlowski
Hi Krysztof,
> Your first node is empty and that is not readable.
I'd argue that separating the i3c definitions makes the source more
readable (granted, at the cognitive expense of having to dereference a
label), but ok.
I'll send a v2 with the bus definitions inline, but first reworking the
existing i2c definitions to use the same format.
Cheers,
Jeremy
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-05-02 11:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-01 3:38 [PATCH] ARM: dts: aspeed-g6: Add nodes for i3c controllers Jeremy Kerr
2024-05-01 10:24 ` Krzysztof Kozlowski
2024-05-01 11:17 ` Jeremy Kerr
2024-05-01 16:45 ` Krzysztof Kozlowski
2024-05-02 11:10 ` Jeremy Kerr
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.