ARM: dts: aspeed-g6: add AST2600 I3C nodes and bindings
Subsystems (3)
v3 cover ARM: dts: aspeed-g6: add AST2600 I3C nodes and bindings
This series reworks and resubmits AST2600 I3C DTS updates that were originally posted in 2024, but stalled without further upstream progress.[1] The series was rebased onto the current tree and merge conflicts were resolved.
The patches first move i2c nodes under the APB simple-bus for layout consistency, then add AST2600 I3C controller nodes in aspeed-g6.dtsi and finally add the missing dt-binding for AST2600 I3C global registers. Jeremy agreed in a separate email thread that I can continue this series under my authorship.
Dawid Glazik (3):
ARM: dts: aspeed-g6: move i2c controllers directly into apb node
ARM: dts: aspeed-g6: Add nodes for i3c controllers
dt-bindings: i3c: Add AST2600 I3C global registers
.../i3c/aspeed,ast2600-i3c-global.yaml | 55 ++ arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 549 ++++++++++-------- 2 files changed, 377 insertions(+), 227 deletions(-) create mode 100644 Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
Signature · 1 line
2.43.0
PATCH v3 1/3 ARM: dts: aspeed-g6: move i2c controllers directly into apb node
We currently have the apb's mapping of the i2c controller space as a labelled mostly-empty node:
apb {
i2c: bus@1e78a000 {
ranges = <...>;
};
}... and then define the contents of the i2c block later:
i2c: {
i2c0: i2c-bus@80 {
reg = <0x80 0x80>;
};
i2c1: i2c-bus@100 {
reg = <0x100 0x80>;
};
}Krzysztof mentions[1] that isn't convention though, with the top-level simple-bus being empty and linked via the label. So, drop the label usage and move the i2c bus definition into the simple-bus node directly under the apb:
apb {
bus@1e78a000 {
ranges = <...>; i2c0: i2c-bus@80 {
reg = <0x80 0x80>;
};
i2c1: i2c-bus@100 {
reg = <0x100 0x80>;
};
};}
This will allow us to be consistent when we add new definitions for the i3c nodes, which would require the latter format.
---
v3:
- wasn't sure about target tree - picked the one pointed in
https://docs.kernel.org/process/maintainer-soc.html
- pick up series after two years
- rebase on top of latest tree and solve conflicts
- as agreed with Jeremy off-list, he said I can take authorship of this going forward
v2:
- new patch: reorganise i2c nodes before adding new-format i3c nodes
--- arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 452 ++++++++++++------------ 1 file changed, 225 insertions(+), 227 deletions(-)
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi +225 -227
@@ -835,11 +835,235 @@ uart9: serial@1e790300 { status = "disabled"; }; - i2c: bus@1e78a000 { + bus@1e78a000 { compatible = "simple-bus"; #address-cells = <1>; #size-cells = <1>; ranges = <0 0x1e78a000 0x1000>; + + i2c0: i2c@80 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x80 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c1_default>; + status = "disabled"; + }; + + i2c1: i2c@100 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x100 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c2_default>; + status = "disabled"; + }; + + i2c2: i2c@180 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x180 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c3_default>; + status = "disabled"; + }; + + i2c3: i2c@200 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x200 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c4_default>; + status = "disabled"; + }; + + i2c4: i2c@280 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x280 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c5_default>; + status = "disabled"; + }; + + i2c5: i2c@300 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x300 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c6_default>; + status = "disabled"; + }; + + i2c6: i2c@380 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x380 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c7_default>; + status = "disabled"; + }; + + i2c7: i2c@400 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x400 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c8_default>; + status = "disabled"; + }; + + i2c8: i2c@480 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x480 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c9_default>; + status = "disabled"; + }; + + i2c9: i2c@500 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x500 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c10_default>; + status = "disabled"; + }; + + i2c10: i2c@580 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x580 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c11_default>; + status = "disabled"; + }; + + i2c11: i2c@600 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x600 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c12_default>; + status = "disabled"; + }; + + i2c12: i2c@680 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x680 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c13_default>; + status = "disabled"; + }; + + i2c13: i2c@700 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x700 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c14_default>; + status = "disabled"; + }; + + i2c14: i2c@780 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x780 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c15_default>; + status = "disabled"; + }; + + i2c15: i2c@800 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0x800 0x80>; + compatible = "aspeed,ast2600-i2c-bus"; + clocks = <&syscon ASPEED_CLK_APB2>; + resets = <&syscon ASPEED_RESET_I2C>; + interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>; + bus-frequency = <100000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c16_default>; + status = "disabled"; + }; }; fsim0: fsi@1e79b000 {
@@ -870,229 +1094,3 @@ fsim1: fsi@1e79b100 { }; #include "aspeed-g6-pinctrl.dtsi" - -&i2c { - i2c0: i2c@80 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x80 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c1_default>; - status = "disabled"; - }; - - i2c1: i2c@100 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x100 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c2_default>; - status = "disabled"; - }; - - i2c2: i2c@180 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x180 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c3_default>; - status = "disabled"; - }; - - i2c3: i2c@200 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x200 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c4_default>; - status = "disabled"; - }; - - i2c4: i2c@280 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x280 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c5_default>; - status = "disabled"; - }; - - i2c5: i2c@300 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x300 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c6_default>; - status = "disabled"; - }; - - i2c6: i2c@380 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x380 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c7_default>; - status = "disabled"; - }; - - i2c7: i2c@400 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x400 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c8_default>; - status = "disabled"; - }; - - i2c8: i2c@480 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x480 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c9_default>; - status = "disabled"; - }; - - i2c9: i2c@500 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x500 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c10_default>; - status = "disabled"; - }; - - i2c10: i2c@580 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x580 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c11_default>; - status = "disabled"; - }; - - i2c11: i2c@600 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x600 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c12_default>; - status = "disabled"; - }; - - i2c12: i2c@680 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x680 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c13_default>; - status = "disabled"; - }; - - i2c13: i2c@700 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x700 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c14_default>; - status = "disabled"; - }; - - i2c14: i2c@780 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x780 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c15_default>; - status = "disabled"; - }; - - i2c15: i2c@800 { - #address-cells = <1>; - #size-cells = <0>; - reg = <0x800 0x80>; - compatible = "aspeed,ast2600-i2c-bus"; - clocks = <&syscon ASPEED_CLK_APB2>; - resets = <&syscon ASPEED_RESET_I2C>; - interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>; - bus-frequency = <100000>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_i2c16_default>; - status = "disabled"; - }; -};
Signature · 1 line
2.43.0
PATCH v3 2/3 ARM: dts: aspeed-g6: Add nodes for i3c controllers
In reply to Dawid GlazikAdd 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.
---
v3:
- add i3c aliases
- rebase on top of latest tree and solve conflicts
- as agreed with Jeremy off-list, he said I can take authorship of this going forward
v2:
- use inline bus representation, without the i3c: label
--- arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 97 +++++++++++++++++++++++++ 1 file changed, 97 insertions(+)
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi +97 -0
@@ -29,6 +29,12 @@ aliases { i2c13 = &i2c13; i2c14 = &i2c14; i2c15 = &i2c15; + i3c0 = &i3c0; + i3c1 = &i3c1; + i3c2 = &i3c2; + i3c3 = &i3c3; + i3c4 = &i3c4; + i3c5 = &i3c5; serial0 = &uart1; serial1 = &uart2; serial2 = &uart3;
@@ -1066,6 +1072,97 @@ i2c15: i2c@800 { }; }; + bus@1e7a0000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x1e7a0000 0x8000>; + + i3c_global: i3c-global@0 { + compatible = "aspeed,ast2600-i3c-global", "syscon"; + 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"; + }; + }; + fsim0: fsi@1e79b000 { #interrupt-cells = <1>; compatible = "aspeed,ast2600-fsi-master";
Signature · 1 line
2.43.0
Re: [PATCH v3 2/3] ARM: dts: aspeed-g6: Add nodes for i3c controllers
Quoted 5 lines · Dawid Glazik
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.
Originally-by Jeremy Kerr <[email protected]>
Don't invent tags. This is not used outside of TIP.
Quoted 17 lines
Signed-off-by Dawid Glazik <[email protected]>---
v3:
- add i3c aliases
- rebase on top of latest tree and solve conflicts
- as agreed with Jeremy off-list, he said I can take authorship of this going forward
v2:
- use inline bus representation, without the i3c: label--- arch/arm/boot/dts/aspeed/aspeed-g6.dtsi | 97 +++++++++++++++++++++++++ 1 file changed, 97 insertions(+)arch/arm/boot/dts/aspeed/aspeed-g6.dtsi +0
-0
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi aliases { · line 37
Quoted 9 lines
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi +6
-0@@ -29,6 +29,12 @@ aliases { i2c13 = &i2c13; i2c14 = &i2c14; i2c15 = &i2c15; + i3c0 = &i3c0; + i3c1 = &i3c1; + i3c2 = &i3c2; + i3c3 = &i3c3; + i3c4 = &i3c4; + i3c5 = &i3c5;
Bus aliases are not properties of SoC DTSI.
Quoted 4 lines
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi +0
-0serial0 = &uart1; serial1 = &uart2; serial2 = &uart3;
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi i2c15: i2c@800 { · line 1081
Quoted 10 lines
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi +7
-0@@ -1066,6 +1072,97 @@ i2c15: i2c@800 { }; }; + bus@1e7a0000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x1e7a0000 0x8000>; + + i3c_global: i3c-global@0 {
Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel sources for similar cases or you can grow the spec (via pull request to DT spec repo).
Best regards,
Krzysztof
Re: [PATCH v3 2/3] ARM: dts: aspeed-g6: Add nodes for i3c controllers
In reply to Dawid Glazikarch/arm/boot/dts/aspeed/aspeed-g6.dtsi i2c15: i2c@800 { · line 1082
Quoted 8 lines · Dawid Glazik
arch/arm/boot/dts/aspeed/aspeed-g6.dtsi +8
-0+ bus@1e7a0000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x1e7a0000 0x8000>; + + i3c_global: i3c-global@0 { + compatible = "aspeed,ast2600-i3c-global", "syscon";
Also undocumented ABI. Apply this patch and run checkpatch.
Best regards,
Krzysztof
PATCH v3 3/3 dt-bindings: i3c: Add AST2600 I3C global registers
In reply to Dawid GlazikIntroduce the device-tree bindings for I3C global registers found on AST2600 SoCs.
---
I wasn't sure if I should add newline at the end of the file or not so I took
https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings/i3c
as an example.
--- .../i3c/aspeed,ast2600-i3c-global.yaml | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml
Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml new file +55 -0
@@ -0,0 +1,55 @@ +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/i3c/aspeed,ast2600-i3c-global.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ASPEED AST2600 I3C Global Registers + +description: | + The AST2600 SoC provides a shared I3C global register block used by all + I3C controller instances. This block contains per-instance global + configuration fields, including controller instance ID and SDA pull-up + configuration. + Each I3C controller references this syscon node through the + aspeed,global-regs property. + +maintainers: + - Dawid Glazik <[email protected]> + +allOf: + - $ref: /schemas/mfd/syscon-common.yaml# + +properties: + compatible: + items: + - const: aspeed,ast2600-i3c-global + - const: syscon + + reg: + maxItems: 1 + + resets: + maxItems: 1 + +required: + - compatible + - reg + +unevaluatedProperties: false + +examples: + - | + bus@1e7a0000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x1e7a0000 0x8000>; + + i3c-global@0 { + compatible = "aspeed,ast2600-i3c-global", "syscon"; + reg = <0x0 0x1000>; + resets = <&syscon ASPEED_RESET_I3C_DMA>; + }; + }; +...
Signature · 1 line
2.43.0
Rob Herring (Arm)
maintainerRe: [PATCH v3 3/3] dt-bindings: i3c: Add AST2600 I3C global registers
Quoted 9 lines · Dawid Glazik
Introduce the device-tree bindings for I3C global registers found on AST2600 SoCs.
Signed-off-by Dawid Glazik <[email protected]>---
I wasn't sure if I should add newline at the end of the file or not so I took
https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings/i3c
as an example.
Quoted 5 lines
--- .../i3c/aspeed,ast2600-i3c-global.yaml | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:./Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml:55:4: [error] no new line character at the end of file (new-line-at-end-of-file)
dtschema/dtc warnings/errors:
Lexical error: Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.example.dts:27.35-55 Unexpected 'ASPEED_RESET_I3C_DMA' FATAL ERROR: Syntax error parsing input tree
make[2]: *** [scripts/Makefile.dtbs:140: Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1597: dt_binding_check] Error 2
make: *** [Makefile:248: __sub-make] Error 2
doc reference errors (make refcheckdocs):
The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH v3 3/3] dt-bindings: i3c: Add AST2600 I3C global registers
In reply to Dawid GlazikQuoted 9 lines · Dawid Glazik
Introduce the device-tree bindings for I3C global registers found on AST2600 SoCs.
Signed-off-by Dawid Glazik <[email protected]>---
I wasn't sure if I should add newline at the end of the file or not so I took
https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings/i3c
as an example.
Answer is: you cannot have patch warnings.
Documentation/devicetree/bindings/i3c does not have patch warning, does it?
Quoted 12 lines
--- .../i3c/aspeed,ast2600-i3c-global.yaml | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yamlDocumentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml new file +1
-0@@ -0,0 +1,55 @@ +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml line 11
Quoted 10 lines
Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml new file +10
-0+%YAML 1.2 +--- +$id: http://devicetree.org/schemas/i3c/aspeed,ast2600-i3c-global.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ASPEED AST2600 I3C Global Registers + +description: | + The AST2600 SoC provides a shared I3C global register block used by all + I3C controller instances. This block contains per-instance global
Why are you adding syscon to I3C controllers? Only I3C controllers go to i3c directory.
syscons go soc.
Quoted 13 lines
Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml new file +13
-0+ configuration fields, including controller instance ID and SDA pull-up + configuration. + Each I3C controller references this syscon node through the + aspeed,global-regs property. + +maintainers: + - Dawid Glazik <[email protected]> + +allOf: + - $ref: /schemas/mfd/syscon-common.yaml# + +properties: + compatible:
Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml line 34
Quoted 10 lines
Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml new file +10
-0+ items: + - const: aspeed,ast2600-i3c-global + - const: syscon + + reg: + maxItems: 1 + + resets: + maxItems: 1 +
I do not see any differences against syscon.yaml binding. Why do you need separate file? Look first how other platforms do it.
Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml line 47
Quoted 13 lines
Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml new file +13
-0+required: + - compatible + - reg + +unevaluatedProperties: false + +examples: + - | + bus@1e7a0000 { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x1e7a0000 0x8000>;
Drop entire node, not needed.
Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml line 55
Quoted 9 lines
Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c-global.yaml new file +8
-0+ + i3c-global@0 { + compatible = "aspeed,ast2600-i3c-global", "syscon"; + reg = <0x0 0x1000>; + resets = <&syscon ASPEED_RESET_I3C_DMA>; + }; + }; +...
Patch warning.
Best regards,
Krzysztof
Re: [PATCH v3 3/3] dt-bindings: i3c: Add AST2600 I3C global registers
Quoted 11 lines · Krzysztof Kozlowski
Quoted 9 lines · Dawid Glazik
Introduce the device-tree bindings for I3C global registers found on AST2600 SoCs.
Signed-off-by Dawid Glazik <[email protected]>---
I wasn't sure if I should add newline at the end of the file or not so I took
https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings/i3c
as an example.
Quoted 4 lines
Answer is: you cannot have patch warnings.
Documentation/devicetree/bindings/i3cdoes not have patch warning, does it?
And if you tested this code with standard tools, you would see that...
Best regards,
Krzysztof
Re: [PATCH v3 3/3] dt-bindings: i3c: Add AST2600 I3C global registers
Quoted 14 lines · Krzysztof Kozlowski
Quoted 13 lines · Krzysztof Kozlowski
Quoted 9 lines · Dawid Glazik
Introduce the device-tree bindings for I3C global registers found on AST2600 SoCs.
Signed-off-by Dawid Glazik <[email protected]>---
I wasn't sure if I should add newline at the end of the file or not so I took
https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings/i3c
as an example.Answer is: you cannot have patch warnings.
Quoted 7 lines
Quoted 2 lines
Documentation/devicetree/bindings/i3cdoes not have patch warning, does it?And if you tested this code with standard tools, you would see that...
Best regards,
Krzysztof
Thank you for the review and feedback. This is my first contribution to Linux kernel so I'm still learning the process and toolchain. I apologize for the rookie mistakes. I will address all the issues you've pointed out and resubmit the series.
Best regards,
Dawid
Re: [PATCH v3 3/3] dt-bindings: i3c: Add AST2600 I3C global registers
Quoted 20 lines · Dawid Glazik
Quoted 19 lines · Krzysztof Kozlowski
Quoted 11 lines · Krzysztof Kozlowski
Quoted 9 lines · Dawid Glazik
Introduce the device-tree bindings for I3C global registers found on AST2600 SoCs.
Signed-off-by Dawid Glazik <[email protected]>---
I wasn't sure if I should add newline at the end of the file or not so I took
https://github.com/torvalds/linux/tree/master/Documentation/devicetree/bindings/i3c
as an example.Quoted 4 lines
Answer is: you cannot have patch warnings.
Documentation/devicetree/bindings/i3cdoes not have patch warning, does it?And if you tested this code with standard tools, you would see that...
Quoted 7 lines
Quoted 2 lines
Best regards,
KrzysztofThank you for the review and feedback. This is my first contribution to Linux kernel so I'm still learning the process and toolchain. I apologize for the rookie mistakes. I will address all the issues you've pointed out and resubmit the series.
So get the patch reviewed by Intel colleagues which would tell you what tools you must run and what warnings are accepted or not (and patch warning is never accepted).
Best regards,
Krzysztof