Skip to content

Commit aeb0b22

Browse files
amboarklarasm
amboar
authored and
klarasm
committed
ARM: dts: aspeed: Rework APB nodes
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://patch.msgid.link/r/20240821-dt-warnings-apb-nodes-v1-1-c524923acca5@codeconstruct.com.au Signed-off-by: Andrew Jeffery <[email protected]>
1 parent a8fbb26 commit aeb0b22

File tree

3 files changed

+251
-200
lines changed

3 files changed

+251
-200
lines changed

‎arch/arm/boot/dts/aspeed/aspeed-g4.dtsi

+53-40
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,9 @@
179179
status = "disabled";
180180
};
181181

182-
apb {
182+
apb@1e6e0000 {
183183
compatible = "simple-bus";
184+
reg = <0x1e6e0000 0x00010000>;
184185
#address-cells = <1>;
185186
#size-cells = <1>;
186187
ranges;
@@ -226,52 +227,62 @@
226227
#io-channel-cells = <1>;
227228
status = "disabled";
228229
};
230+
};
229231

230-
sram: sram@1e720000 {
231-
compatible = "mmio-sram";
232-
reg = <0x1e720000 0x8000>; // 32K
233-
ranges;
234-
#address-cells = <1>;
235-
#size-cells = <1>;
236-
};
232+
/* There's another APB mapping at 0x1e6f0000 for 0x00010000 */
233+
234+
sram: sram@1e720000 {
235+
compatible = "mmio-sram";
236+
reg = <0x1e720000 0x8000>; // 32K
237+
ranges;
238+
#address-cells = <1>;
239+
#size-cells = <1>;
240+
};
241+
242+
video: video@1e700000 {
243+
compatible = "aspeed,ast2400-video-engine";
244+
reg = <0x1e700000 0x1000>;
245+
clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
246+
<&syscon ASPEED_CLK_GATE_ECLK>;
247+
clock-names = "vclk", "eclk";
248+
interrupts = <7>;
249+
status = "disabled";
250+
};
251+
252+
sdmmc: sd-controller@1e740000 {
253+
compatible = "aspeed,ast2400-sd-controller";
254+
reg = <0x1e740000 0x100>;
255+
#address-cells = <1>;
256+
#size-cells = <1>;
257+
ranges = <0 0x1e740000 0x10000>;
258+
clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
259+
status = "disabled";
237260

238-
video: video@1e700000 {
239-
compatible = "aspeed,ast2400-video-engine";
240-
reg = <0x1e700000 0x1000>;
241-
clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
242-
<&syscon ASPEED_CLK_GATE_ECLK>;
243-
clock-names = "vclk", "eclk";
244-
interrupts = <7>;
261+
sdhci0: sdhci@100 {
262+
compatible = "aspeed,ast2400-sdhci";
263+
reg = <0x100 0x100>;
264+
interrupts = <26>;
265+
sdhci,auto-cmd12;
266+
clocks = <&syscon ASPEED_CLK_SDIO>;
245267
status = "disabled";
246268
};
247269

248-
sdmmc: sd-controller@1e740000 {
249-
compatible = "aspeed,ast2400-sd-controller";
250-
reg = <0x1e740000 0x100>;
251-
#address-cells = <1>;
252-
#size-cells = <1>;
253-
ranges = <0 0x1e740000 0x10000>;
254-
clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
270+
sdhci1: sdhci@200 {
271+
compatible = "aspeed,ast2400-sdhci";
272+
reg = <0x200 0x100>;
273+
interrupts = <26>;
274+
sdhci,auto-cmd12;
275+
clocks = <&syscon ASPEED_CLK_SDIO>;
255276
status = "disabled";
256-
257-
sdhci0: sdhci@100 {
258-
compatible = "aspeed,ast2400-sdhci";
259-
reg = <0x100 0x100>;
260-
interrupts = <26>;
261-
sdhci,auto-cmd12;
262-
clocks = <&syscon ASPEED_CLK_SDIO>;
263-
status = "disabled";
264-
};
265-
266-
sdhci1: sdhci@200 {
267-
compatible = "aspeed,ast2400-sdhci";
268-
reg = <0x200 0x100>;
269-
interrupts = <26>;
270-
sdhci,auto-cmd12;
271-
clocks = <&syscon ASPEED_CLK_SDIO>;
272-
status = "disabled";
273-
};
274277
};
278+
};
279+
280+
apb@1e780000 {
281+
compatible = "simple-bus";
282+
reg = <0x1e780000 0x00010000>;
283+
#address-cells = <1>;
284+
#size-cells = <1>;
285+
ranges;
275286

276287
gpio: gpio@1e780000 {
277288
#gpio-cells = <2>;
@@ -454,6 +465,8 @@
454465
ranges = <0 0x1e78a000 0x1000>;
455466
};
456467
};
468+
469+
/* There's another APB mapping at 0x1e790000 for 0x00010000 */
457470
};
458471
};
459472

‎arch/arm/boot/dts/aspeed/aspeed-g5.dtsi

+61-48
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,9 @@
207207
status = "disabled";
208208
};
209209

210-
apb {
210+
apb@1e6e0000 {
211211
compatible = "simple-bus";
212+
reg = <0x1e6e0000 0x00010000>;
212213
#address-cells = <1>;
213214
#size-cells = <1>;
214215
ranges;
@@ -289,53 +290,63 @@
289290
#io-channel-cells = <1>;
290291
status = "disabled";
291292
};
293+
};
292294

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

303-
sram: sram@1e720000 {
304-
compatible = "mmio-sram";
305-
reg = <0x1e720000 0x9000>; // 36K
306-
ranges;
307-
#address-cells = <1>;
308-
#size-cells = <1>;
309-
};
297+
video: video@1e700000 {
298+
compatible = "aspeed,ast2500-video-engine";
299+
reg = <0x1e700000 0x1000>;
300+
clocks = <&syscon ASPEED_CLK_GATE_VCLK>,
301+
<&syscon ASPEED_CLK_GATE_ECLK>;
302+
clock-names = "vclk", "eclk";
303+
interrupts = <7>;
304+
status = "disabled";
305+
};
310306

311-
sdmmc: sd-controller@1e740000 {
312-
compatible = "aspeed,ast2500-sd-controller";
313-
reg = <0x1e740000 0x100>;
314-
#address-cells = <1>;
315-
#size-cells = <1>;
316-
ranges = <0 0x1e740000 0x10000>;
317-
clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
318-
status = "disabled";
307+
sram: sram@1e720000 {
308+
compatible = "mmio-sram";
309+
reg = <0x1e720000 0x9000>; // 36K
310+
ranges;
311+
#address-cells = <1>;
312+
#size-cells = <1>;
313+
};
319314

320-
sdhci0: sdhci@100 {
321-
compatible = "aspeed,ast2500-sdhci";
322-
reg = <0x100 0x100>;
323-
interrupts = <26>;
324-
sdhci,auto-cmd12;
325-
clocks = <&syscon ASPEED_CLK_SDIO>;
326-
status = "disabled";
327-
};
315+
sdmmc: sd-controller@1e740000 {
316+
compatible = "aspeed,ast2500-sd-controller";
317+
reg = <0x1e740000 0x100>;
318+
#address-cells = <1>;
319+
#size-cells = <1>;
320+
ranges = <0 0x1e740000 0x10000>;
321+
clocks = <&syscon ASPEED_CLK_GATE_SDCLK>;
322+
status = "disabled";
328323

329-
sdhci1: sdhci@200 {
330-
compatible = "aspeed,ast2500-sdhci";
331-
reg = <0x200 0x100>;
332-
interrupts = <26>;
333-
sdhci,auto-cmd12;
334-
clocks = <&syscon ASPEED_CLK_SDIO>;
335-
status = "disabled";
336-
};
324+
sdhci0: sdhci@100 {
325+
compatible = "aspeed,ast2500-sdhci";
326+
reg = <0x100 0x100>;
327+
interrupts = <26>;
328+
sdhci,auto-cmd12;
329+
clocks = <&syscon ASPEED_CLK_SDIO>;
330+
status = "disabled";
337331
};
338332

333+
sdhci1: sdhci@200 {
334+
compatible = "aspeed,ast2500-sdhci";
335+
reg = <0x200 0x100>;
336+
interrupts = <26>;
337+
sdhci,auto-cmd12;
338+
clocks = <&syscon ASPEED_CLK_SDIO>;
339+
status = "disabled";
340+
};
341+
};
342+
343+
apb@1e780000 {
344+
compatible = "simple-bus";
345+
reg = <0x1e780000 0x00010000>;
346+
#address-cells = <1>;
347+
#size-cells = <1>;
348+
ranges;
349+
339350
gpio: gpio@1e780000 {
340351
#gpio-cells = <2>;
341352
gpio-controller;
@@ -521,6 +532,13 @@
521532
};
522533
};
523534

535+
i2c: bus@1e78a000 {
536+
compatible = "simple-bus";
537+
#address-cells = <1>;
538+
#size-cells = <1>;
539+
ranges = <0 0x1e78a000 0x1000>;
540+
};
541+
524542
peci0: peci-controller@1e78b000 {
525543
compatible = "aspeed,ast2500-peci";
526544
reg = <0x1e78b000 0x60>;
@@ -564,14 +582,9 @@
564582
no-loopback-test;
565583
status = "disabled";
566584
};
567-
568-
i2c: bus@1e78a000 {
569-
compatible = "simple-bus";
570-
#address-cells = <1>;
571-
#size-cells = <1>;
572-
ranges = <0 0x1e78a000 0x1000>;
573-
};
574585
};
586+
587+
/* There's another APB mapping at 0x1e790000 for 0x00010000 */
575588
};
576589
};
577590

0 commit comments

Comments
 (0)