ARM: dts: aspeed-g5: Add mailbox and LPC Control nodes
Subsystems (2)
PATCH 1/1 ARM: dts: aspeed-g5: Add mailbox and LPC Control nodes
This reserves BMC ram for host to BMC communication required by the LPC control driver.
As both these devices exist on the LPC bus these nodes are children of a new LPC node.
--- arch/arm/boot/dts/aspeed-g5.dtsi | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
arch/arm/boot/dts/aspeed-g5.dtsi +50 -0
@@ -18,6 +18,18 @@ }; }; + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + flash_memory: region@94000000 { + compatible = "aspeed,ast2500-lpc-ctrl"; + no-map; + reg = <0x94000000 0x04000000>; /* 64M */ + }; + }; + ahb { compatible = "simple-bus"; #address-cells = <1>;
@@ -89,6 +101,44 @@ }; }; + lpc: lpc@1e789000 { + compatible = "aspeed,ast2500-lpc", "simple-mfd"; + reg = <0x1e789000 0x1000>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1e789000 0x1000>; + + lpc_bmc: lpc-bmc@0 { + compatible = "aspeed,ast2500-lpc-bmc"; + reg = <0x0 0x80>; + }; + + lpc_host: lpc-host@80 { + compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon"; + reg = <0x80 0x1e0>; + reg-io-width = <4>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x80 0x1e0>; + + lpc-ctrl@0 { + compatible = "aspeed,ast2500-lpc-ctrl"; + memory-region = <&flash_memory>; + flash = <&spi1>; + reg = <0x0 0x80>; + }; + + mbox: mbox@180 { + compatible = "aspeed,ast2500-mbox"; + reg = <0x180 0x5c>; + interrupts = <46>; + #mbox-cells = <1>; + }; + }; + }; + vic: interrupt-controller@1e6c0080 { compatible = "aspeed,ast2400-vic"; interrupt-controller;
Signature · 1 line
2.11.0
Joel Stanley
maintainerQuoted 18 lines · Cyril Bur
This reserves BMC ram for host to BMC communication required by the LPC control driver.
As both these devices exist on the LPC bus these nodes are children of a new LPC node.
Signed-off-by Cyril Bur <[email protected]>--- arch/arm/boot/dts/aspeed-g5.dtsi | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)arch/arm/boot/dts/aspeed-g5.dtsi +0
-0index
d6ff41ee6c58..51c339b46740100644
arch/arm/boot/dts/aspeed-g5.dtsi line 27
Quoted 10 lines
arch/arm/boot/dts/aspeed-g5.dtsi +7
-0@@ -18,6 +18,18 @@ }; }; + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + flash_memory: region@94000000 { + compatible = "aspeed,ast2500-lpc-ctrl";
That doesn't make sense, the RAM isn't a LPC Host Controller.
arch/arm/boot/dts/aspeed-g5.dtsi line 29
Quoted 2 lines
arch/arm/boot/dts/aspeed-g5.dtsi +2
-0+ no-map; + reg = <0x94000000 0x04000000>; /* 64M */
We don't want this in the dtsi, as there are platforms that don't use LPC buses that lose 64 MB of ram. Put this node in the dts. We might decide to have an aspeed-bmc-opp.dtsi with common snippets, but for now cut and paste.
Have you investigated reserving this memory when the driver probes instead of hardcoding it?
Quoted 23 lines
arch/arm/boot/dts/aspeed-g5.dtsi +16
-0+ }; + }; + ahb { compatible = "simple-bus"; #address-cells = <1>;@@ -89,6 +101,44 @@ }; }; + lpc: lpc@1e789000 { + compatible = "aspeed,ast2500-lpc", "simple-mfd"; + reg = <0x1e789000 0x1000>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1e789000 0x1000>; + + lpc_bmc: lpc-bmc@0 { + compatible = "aspeed,ast2500-lpc-bmc"; + reg = <0x0 0x80>; + }; +
arch/arm/boot/dts/aspeed-g5.dtsi line 126
Quoted 10 lines
arch/arm/boot/dts/aspeed-g5.dtsi +10
-0+ lpc_host: lpc-host@80 { + compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon"; + reg = <0x80 0x1e0>; + reg-io-width = <4>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x80 0x1e0>; + + lpc-ctrl@0 {
This needs to have a label.
arch/arm/boot/dts/aspeed-g5.dtsi line 129
Quoted 3 lines
arch/arm/boot/dts/aspeed-g5.dtsi +3
-0+ compatible = "aspeed,ast2500-lpc-ctrl"; + memory-region = <&flash_memory>; + flash = <&spi1>;
Make this node status = "disabled" in the dtsi, and omit the flash phandle.
I also suggest we have your phandle links for memory and flash in the dts too.
arch/arm/boot/dts/aspeed-g5.dtsi line 137
Quoted 8 lines
arch/arm/boot/dts/aspeed-g5.dtsi +8
-0+ reg = <0x0 0x80>; + }; + + mbox: mbox@180 { + compatible = "aspeed,ast2500-mbox"; + reg = <0x180 0x5c>; + interrupts = <46>; + #mbox-cells = <1>;
Make this node status = "disabled" in the dtsi.
Cheers,
Joel
Quoted 10 lines
arch/arm/boot/dts/aspeed-g5.dtsi +4
-1+ }; + }; + }; + vic: interrupt-controller@1e6c0080 { compatible = "aspeed,ast2400-vic"; interrupt-controller; --2.11.0
Quoted 22 lines · Joel Stanley
Quoted 21 lines · Cyril Bur
This reserves BMC ram for host to BMC communication required by the LPC control driver.
As both these devices exist on the LPC bus these nodes are children of a new LPC node.
Signed-off-by Cyril Bur <[email protected]>--- arch/arm/boot/dts/aspeed-g5.dtsi | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)arch/arm/boot/dts/aspeed-g5.dtsi +0
-0index
d6ff41ee6c58..51c339b46740100644arch/arm/boot/dts/aspeed-g5.dtsi +0
-0@@ -18,6 +18,18 @@ }; };
Quoted 10 lines
arch/arm/boot/dts/aspeed-g5.dtsi line 27
Quoted 7 lines
arch/arm/boot/dts/aspeed-g5.dtsi +7
-0+ reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + flash_memory: region@94000000 { + compatible = "aspeed,ast2500-lpc-ctrl";That doesn't make sense, the RAM isn't a LPC Host Controller.
Agreed, I have a feeling that the lpc-ctrl driver didn't probe wihtout that. I don't have either machine that this would work on handy to test. I'm hopeful I'm wrong here because I agree that it doesn't make sense that this should have that compatible property.
Quoted 8 lines
arch/arm/boot/dts/aspeed-g5.dtsi line 29
Quoted 2 lines
arch/arm/boot/dts/aspeed-g5.dtsi +2
-0+ no-map; + reg = <0x94000000 0x04000000>; /* 64M */We don't want this in the dtsi, as there are platforms that don't use LPC buses that lose 64 MB of ram. Put this node in the dts. We might decide to have an
aspeed-bmc-opp.dtsiwith common snippets, but for now cut and paste.
I agree we don't want it here but I have a very strong aversion to large chunks of ctrl+c and ctrl+v.
Quoted 2 lines
Have you investigated reserving this memory when the driver probes instead of hardcoding it?
Yes, this is by far the cleanest way of getting such a large chunk. If we do want to drop our usage to 4k or 64k then there are methods at runtime but for this amount the DT is really the way to go.
This is somthing we can probably add to driver one day. If it doens't find a reserved-memory node then it should allocate at runtime but that
allocation will likely need to be smaller orders of megs
Quoted 54 lines
Quoted 24 lines
arch/arm/boot/dts/aspeed-g5.dtsi +16
-0+ }; + }; + ahb { compatible = "simple-bus"; #address-cells = <1>;@@ -89,6 +101,44 @@ }; }; + lpc: lpc@1e789000 { + compatible = "aspeed,ast2500-lpc", "simple-mfd"; + reg = <0x1e789000 0x1000>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1e789000 0x1000>; + + lpc_bmc: lpc-bmc@0 { + compatible = "aspeed,ast2500-lpc-bmc"; + reg = <0x0 0x80>; + }; +arch/arm/boot/dts/aspeed-g5.dtsi line 126
Quoted 10 lines
arch/arm/boot/dts/aspeed-g5.dtsi +10
-0+ lpc_host: lpc-host@80 { + compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon"; + reg = <0x80 0x1e0>; + reg-io-width = <4>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x80 0x1e0>; + + lpc-ctrl@0 {This needs to have a label.
arch/arm/boot/dts/aspeed-g5.dtsi line 129
Quoted 3 lines
arch/arm/boot/dts/aspeed-g5.dtsi +3
-0+ compatible = "aspeed,ast2500-lpc-ctrl"; + memory-region = <&flash_memory>; + flash = <&spi1>;Make this node status = "disabled" in the dtsi, and omit the flash phandle.
I also suggest we have your phandle links for memory and flash in the dts too.
arch/arm/boot/dts/aspeed-g5.dtsi line 137
Quoted 8 lines
arch/arm/boot/dts/aspeed-g5.dtsi +8
-0+ reg = <0x0 0x80>; + }; + + mbox: mbox@180 { + compatible = "aspeed,ast2500-mbox"; + reg = <0x180 0x5c>; + interrupts = <46>; + #mbox-cells = <1>;
Quoted 2 lines
Make this node status = "disabled" in the dtsi.
Re all those comments above, I think I see what you're getting at.
Thanks,
Cyril
Quoted 14 lines
Cheers,
Joel
Quoted 10 lines
arch/arm/boot/dts/aspeed-g5.dtsi +4
-1+ }; + }; + }; + vic: interrupt-controller@1e6c0080 { compatible = "aspeed,ast2400-vic"; interrupt-controller; --2.11.0
Quoted 96 lines · Joel Stanley
Quoted 18 lines · Cyril Bur
This reserves BMC ram for host to BMC communication required by the LPC control driver.
As both these devices exist on the LPC bus these nodes are children of a new LPC node.
Signed-off-by Cyril Bur <[email protected]>--- arch/arm/boot/dts/aspeed-g5.dtsi | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)arch/arm/boot/dts/aspeed-g5.dtsi +0
-0index
d6ff41ee6c58..51c339b46740100644arch/arm/boot/dts/aspeed-g5.dtsi line 27
Quoted 10 lines
arch/arm/boot/dts/aspeed-g5.dtsi +7
-0@@ -18,6 +18,18 @@ }; }; + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + flash_memory: region@94000000 { + compatible = "aspeed,ast2500-lpc-ctrl";That doesn't make sense, the RAM isn't a LPC Host Controller.
arch/arm/boot/dts/aspeed-g5.dtsi line 29
Quoted 2 lines
arch/arm/boot/dts/aspeed-g5.dtsi +2
-0+ no-map; + reg = <0x94000000 0x04000000>; /* 64M */We don't want this in the dtsi, as there are platforms that don't use LPC buses that lose 64 MB of ram. Put this node in the dts. We might decide to have an
aspeed-bmc-opp.dtsiwith common snippets, but for now cut and paste.Have you investigated reserving this memory when the driver probes instead of hardcoding it?
Quoted 23 lines
arch/arm/boot/dts/aspeed-g5.dtsi +16
-0+ }; + }; + ahb { compatible = "simple-bus"; #address-cells = <1>;@@ -89,6 +101,44 @@ }; }; + lpc: lpc@1e789000 { + compatible = "aspeed,ast2500-lpc", "simple-mfd"; + reg = <0x1e789000 0x1000>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x1e789000 0x1000>; + + lpc_bmc: lpc-bmc@0 { + compatible = "aspeed,ast2500-lpc-bmc"; + reg = <0x0 0x80>; + }; +arch/arm/boot/dts/aspeed-g5.dtsi line 126
Quoted 10 lines
arch/arm/boot/dts/aspeed-g5.dtsi +10
-0+ lpc_host: lpc-host@80 { + compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon"; + reg = <0x80 0x1e0>; + reg-io-width = <4>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x80 0x1e0>; + + lpc-ctrl@0 {This needs to have a label.
arch/arm/boot/dts/aspeed-g5.dtsi line 129
Quoted 3 lines
arch/arm/boot/dts/aspeed-g5.dtsi +3
-0+ compatible = "aspeed,ast2500-lpc-ctrl"; + memory-region = <&flash_memory>; + flash = <&spi1>;Make this node status = "disabled" in the dtsi, and omit the flash phandle.
I also suggest we have your phandle links for memory and flash in the dts too.
arch/arm/boot/dts/aspeed-g5.dtsi line 137
Quoted 8 lines
arch/arm/boot/dts/aspeed-g5.dtsi +8
-0+ reg = <0x0 0x80>; + }; + + mbox: mbox@180 { + compatible = "aspeed,ast2500-mbox"; + reg = <0x180 0x5c>; + interrupts = <46>; + #mbox-cells = <1>;
Quoted 2 lines
Make this node status = "disabled" in the dtsi.
Oops missed this, do we want to make this disabled - it should exist on every ast2500 and it doesn't depend on anything else...
Quoted 14 lines
Cheers,
Joel
Quoted 10 lines
arch/arm/boot/dts/aspeed-g5.dtsi +4
-1+ }; + }; + }; + vic: interrupt-controller@1e6c0080 { compatible = "aspeed,ast2400-vic"; interrupt-controller; --2.11.0