Skip to content

Commit

Permalink
ARM: dts: aspeed: Rework APB nodes
Browse files Browse the repository at this point in the history 
The way the APB nodes are currently described causes excessive output
from `make dtbs_check` (approximately 30KB per devicetree). This stems
from nesting the apb nodes under the top-level ahb nodes, while the
simple-bus binding requires that the ahb subnode names contain a unit
address[1].

In the process of cleaning this up, it became apparent that both the
APB descriptions in the devicetree and datasheet were pretty murky.
I followed up with Troy Lee and Ryan Chen, and received the following
from Ryan:

> Sorry, I double confirm with designer.
> AST2400/AST2500/AST2600: 1e6exxxx, 1e6fxxxx, 1e78xxxx, 1e79xxxx : APB,
> others is AHB

As a result, update the Aspeed DTSIs to describe one APB node per
mapping listed in Ryan's response, and lift all controllers that are not
in the described ranges out of the APB nodes to the AHB node.

This change may impact OpenBMC userspace applications that use
devicetree paths in sysfs to identify hardware components. However,
these uses of sysfs were previously identified as incorrect[2][3][4].
Its expected that any affected applications will reworked so they are
not sensitive to node renames.

Cc: Andrew Geissler <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Rob Herring (Arm) <[email protected]>
Cc: Ryan Chen <[email protected]>
Cc: Troy Lee <[email protected]>
Link: https://github.com/devicetree-org/dt-schema/blob/c51125d571cac9596048e888a856d70650e400e0/dtschema/schemas/simple-bus.yaml#L35-L36 [1]
Link: openbmc/phosphor-state-manager#27 [2]
Link: https://lore.kernel.org/all/[email protected]/ [3]
Link: https://lore.kernel.org/all/[email protected]/ [4]
Link: https://lore.kernel.org/r/20240821-dt-warnings-apb-nodes-v1-1-c524923acca5@codeconstruct.com.au
Signed-off-by: Andrew Jeffery <[email protected]>
  • Loading branch information
@amboar
amboar committed Sep 19, 2024
1 parent 4a39ac5 commit 7512cd4
Show file tree
Hide file tree
Showing 3 changed files with 251 additions and 200 deletions.
93 changes: 53 additions & 40 deletions arch/arm/boot/dts/aspeed/aspeed-g4.dtsi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,9 @@
status = "disabled";
};

apb {
apb@1e6e0000 {
compatible = "simple-bus";
reg = <0x1e6e0000 0x00010000>;
#address-cells = <1>;
#size-cells = <1>;
ranges;
Expand Down Expand Up @@ -226,52 +227,62 @@
#io-channel-cells = <1>;
status = "disabled";
};
};

sram: sram@1e720000 {
compatible = "mmio-sram";
reg = <0x1e720000 0x8000>; // 32K
ranges;
#address-cells = <1>;
#size-cells = <1>;
};
/* There's another APB mapping at 0x1e6f0000 for 0x00010000 */

sram: sram@1e720000 {
compatible = "mmio-sram";
reg = <0x1e720000 0x8000>; // 32K
ranges;
#address-cells = <1>;
#size-cells = <1>;
};

video: video@1e700000 {
compatible = "aspeed,ast2400-video-engine";
reg = <0x1e700000 0x1000>;
clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
<&syscon ASPEED_CLK_GATE_ECLK>;
clock-names = "vclk", "eclk";
interrupts = <7>;
status = "disabled";
};

sdmmc: sd-controller@1e740000 {
compatible = "aspeed,ast2400-sd-controller";
reg = <0x1e740000 0x100>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x1e740000 0x10000>;
clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
status = "disabled";

video: video@1e700000 {
compatible = "aspeed,ast2400-video-engine";
reg = <0x1e700000 0x1000>;
clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
<&syscon ASPEED_CLK_GATE_ECLK>;
clock-names = "vclk", "eclk";
interrupts = <7>;
sdhci0: sdhci@100 {
compatible = "aspeed,ast2400-sdhci";
reg = <0x100 0x100>;
interrupts = <26>;
sdhci,auto-cmd12;
clocks = <&syscon ASPEED_CLK_SDIO>;
status = "disabled";
};

sdmmc: sd-controller@1e740000 {
compatible = "aspeed,ast2400-sd-controller";
reg = <0x1e740000 0x100>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x1e740000 0x10000>;
clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
sdhci1: sdhci@200 {
compatible = "aspeed,ast2400-sdhci";
reg = <0x200 0x100>;
interrupts = <26>;
sdhci,auto-cmd12;
clocks = <&syscon ASPEED_CLK_SDIO>;
status = "disabled";

sdhci0: sdhci@100 {
compatible = "aspeed,ast2400-sdhci";
reg = <0x100 0x100>;
interrupts = <26>;
sdhci,auto-cmd12;
clocks = <&syscon ASPEED_CLK_SDIO>;
status = "disabled";
};

sdhci1: sdhci@200 {
compatible = "aspeed,ast2400-sdhci";
reg = <0x200 0x100>;
interrupts = <26>;
sdhci,auto-cmd12;
clocks = <&syscon ASPEED_CLK_SDIO>;
status = "disabled";
};
};
};

apb@1e780000 {
compatible = "simple-bus";
reg = <0x1e780000 0x00010000>;
#address-cells = <1>;
#size-cells = <1>;
ranges;

gpio: gpio@1e780000 {
#gpio-cells = <2>;
Expand Down Expand Up @@ -454,6 +465,8 @@
ranges = <0 0x1e78a000 0x1000>;
};
};

/* There's another APB mapping at 0x1e790000 for 0x00010000 */
};
};

Expand Down
109 changes: 61 additions & 48 deletions arch/arm/boot/dts/aspeed/aspeed-g5.dtsi
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,9 @@
status = "disabled";
};

apb {
apb@1e6e0000 {
compatible = "simple-bus";
reg = <0x1e6e0000 0x00010000>;
#address-cells = <1>;
#size-cells = <1>;
ranges;
Expand Down Expand Up @@ -289,53 +290,63 @@
#io-channel-cells = <1>;
status = "disabled";
};
};

video: video@1e700000 {
compatible = "aspeed,ast2500-video-engine";
reg = <0x1e700000 0x1000>;
clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
<&syscon ASPEED_CLK_GATE_ECLK>;
clock-names = "vclk", "eclk";
interrupts = <7>;
status = "disabled";
};
/* There's another APB mapping at 0x1e6f0000 for 0x00010000 */

sram: sram@1e720000 {
compatible = "mmio-sram";
reg = <0x1e720000 0x9000>; // 36K
ranges;
#address-cells = <1>;
#size-cells = <1>;
};
video: video@1e700000 {
compatible = "aspeed,ast2500-video-engine";
reg = <0x1e700000 0x1000>;
clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
<&syscon ASPEED_CLK_GATE_ECLK>;
clock-names = "vclk", "eclk";
interrupts = <7>;
status = "disabled";
};

sdmmc: sd-controller@1e740000 {
compatible = "aspeed,ast2500-sd-controller";
reg = <0x1e740000 0x100>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x1e740000 0x10000>;
clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
status = "disabled";
sram: sram@1e720000 {
compatible = "mmio-sram";
reg = <0x1e720000 0x9000>; // 36K
ranges;
#address-cells = <1>;
#size-cells = <1>;
};

sdhci0: sdhci@100 {
compatible = "aspeed,ast2500-sdhci";
reg = <0x100 0x100>;
interrupts = <26>;
sdhci,auto-cmd12;
clocks = <&syscon ASPEED_CLK_SDIO>;
status = "disabled";
};
sdmmc: sd-controller@1e740000 {
compatible = "aspeed,ast2500-sd-controller";
reg = <0x1e740000 0x100>;
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x1e740000 0x10000>;
clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
status = "disabled";

sdhci1: sdhci@200 {
compatible = "aspeed,ast2500-sdhci";
reg = <0x200 0x100>;
interrupts = <26>;
sdhci,auto-cmd12;
clocks = <&syscon ASPEED_CLK_SDIO>;
status = "disabled";
};
sdhci0: sdhci@100 {
compatible = "aspeed,ast2500-sdhci";
reg = <0x100 0x100>;
interrupts = <26>;
sdhci,auto-cmd12;
clocks = <&syscon ASPEED_CLK_SDIO>;
status = "disabled";
};

sdhci1: sdhci@200 {
compatible = "aspeed,ast2500-sdhci";
reg = <0x200 0x100>;
interrupts = <26>;
sdhci,auto-cmd12;
clocks = <&syscon ASPEED_CLK_SDIO>;
status = "disabled";
};
};

apb@1e780000 {
compatible = "simple-bus";
reg = <0x1e780000 0x00010000>;
#address-cells = <1>;
#size-cells = <1>;
ranges;

gpio: gpio@1e780000 {
#gpio-cells = <2>;
gpio-controller;
Expand Down Expand Up @@ -521,6 +532,13 @@
};
};

i2c: bus@1e78a000 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x1e78a000 0x1000>;
};

peci0: peci-controller@1e78b000 {
compatible = "aspeed,ast2500-peci";
reg = <0x1e78b000 0x60>;
Expand Down Expand Up @@ -564,14 +582,9 @@
no-loopback-test;
status = "disabled";
};

i2c: bus@1e78a000 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 0x1e78a000 0x1000>;
};
};

/* There's another APB mapping at 0x1e790000 for 0x00010000 */
};
};

Expand Down
Loading

0 comments on commit 7512cd4

Please sign in to comment.