Re: [PATCH v3 2/5] powerpc: apm82181: create shared dtsi for APM bluestone
On Tue, Sep 22, 2020 at 9:14 PM Rob Herring wrote:
>
> On Sat, Sep 19, 2020 at 2:23 PM Christian Lamparter
> wrote:
> >
> > On 2020-09-15 03:05, Rob Herring wrote:
> > > On Sun, Sep 06, 2020 at 12:06:12AM +0200, Christian Lamparter wrote:
> > >> This patch adds an DTSI-File that can be used by various device-tree
> > >> files for APM82181-based devices.
> > >>
> > >> Some of the nodes (like UART, PCIE, SATA) are used by the uboot and
> > >> need to stick with the naming-conventions of the old times'.
> > >> I've added comments whenever this was the case.
> > >>
> > >> Signed-off-by: Chris Blake
> > >> Signed-off-by: Christian Lamparter
> > >> ---
> > >> rfc v1 -> v2:
> > >> - removed PKA (this CryptoPU will need driver)
> > >> - stick with compatibles, nodes, ... from either
> > >>Bluestone (APM82181) or Canyonlands (PPC460EX).
> > >> - add labels for NAND and NOR to help with access.
> > >> v2 -> v3:
> > >> - nodename of pciex@d was changed to pcie@d..
> > >>due to upstream patch.
> > >> - use simple-bus on the ebc, opb and plb nodes
> > >> ---
> > >> arch/powerpc/boot/dts/apm82181.dtsi | 466
> > >> 1 file changed, 466 insertions(+)
> > >> create mode 100644 arch/powerpc/boot/dts/apm82181.dtsi
> > >>
> > >> diff --git a/arch/powerpc/boot/dts/apm82181.dtsi
> > >> b/arch/powerpc/boot/dts/apm82181.dtsi
> > >> new file mode 100644
> > >> index ..60283430978d
> > >> --- /dev/null
> > >> +++ b/arch/powerpc/boot/dts/apm82181.dtsi
> > >> @@ -0,0 +1,466 @@
> > >> +// SPDX-License-Identifier: GPL-2.0-or-later
> > >> +/*
> > >> + * Device Tree template include for various APM82181 boards.
> > >> + *
> > >> + * The SoC is an evolution of the PPC460EX predecessor.
> > >> + * This is why dt-nodes from the canyonlands EBC, OPB, USB,
> > >> + * DMA, SATA, EMAC, ... ended up in here.
> > >> + *
> > >> + * Copyright (c) 2010, Applied Micro Circuits Corporation
> > >> + * Author: Tirumala R Marri ,
> > >> + * Christian Lamparter ,
> > >> + * Chris Blake
> > >> + */
> > >> +
> > >> +#include
> > >> +#include
> > >> +#include
> > >> +#include
> > >> +
> > >> +/ {
> > >> +#address-cells = <2>;
> > >> +#size-cells = <1>;
> > >> +dcr-parent = <&{/cpus/cpu@0}>;
> > >> +
> > >> +aliases {
> > >> +ethernet0 = /* needed for BSP u-boot */
> > >> +};
> > >> +
> > >> +cpus {
> > >> +#address-cells = <1>;
> > >> +#size-cells = <0>;
> > >> +
> > >> +CPU0: cpu@0 {
> > >> +device_type = "cpu";
> > >> +model = "PowerPC,apm82181";
> > >
> > > This doesn't match the existing bluestone dts file.
> > >
> > > Please separate any restructuring from changes.
> >
> >
> > "I see" (I'm including your comment of the dt-binding as well).
> >
> > I'm getting the vibe that I better should not touch that bluestone.dts.
>
> I don't know about that.
k, understood.
>
> > An honestly, looking at the series and patches that the APM-engineers
> > posted back in the day, I can see why this well is so poisoned... and
> > stuff like SATA/AHBDMA/USB/GPIO/CPM/... was missing.
> >
> > As for the devices. In the spirit of Arnd Bergmann's post of
> >
> >
> > |It would be nice to move over the the bluestone .dts to the apm82181.dtsi
> > file
> > |when that gets added, if only to ensure they use the same description for
> > each
> > |node, but that shouldn't stop the addition of the new file if that is
> > needed for
> > |distros to make use of a popular device.
> > |I see a couple of additional files in openwrt.
> >
> > I mean I don't have the bluestone dev board, just the consumer devices.
>
> This stuff is old enough, I'd guess no one cares about a dev board.
> But we should figure that out and document that with any changes.
>
> > Would it be possible to support those? I can start from a "skeleton"
> > apm82181.dtsi
> > This would just include CPU, Memory (SD-RAM+L2C+OCM), UIC
> > (Interrupt-Controller),
> > the PLB+OBP+EBC Busses and UART. Just enough to make a board "boot from
> > ram".
>
> This skeleton would be chunks moved over or duplicated? I don't think
> we want 2 of the same thing.
My Idea was copying the working apm82181.dtsi we have in OpenWrt
and stripping the nodes we added for SATA, USB, GPIO and the likes.
so the remaining nodes would be very close to what bluestone.dts had.
The main differences would be:
- It's a bit smaller since I made a separate patch for the NOR/NAND on the EBC.
Reason being that the SoC uses glue-logic for mapping NOR/NAND (and other
external peripherals like the GPIOs on the WD) into the memory and I thought
this needed some explanation as to why this weird thing works.
- it would already use the dt-bindings/interrupt-controller/irq.h macros
for LEVEL/EDGE cell values
- it contains valuable comments about the uboot. Because ethernet0 alias
and the
Re: [PATCH v3 2/5] powerpc: apm82181: create shared dtsi for APM bluestone
On Sat, Sep 19, 2020 at 2:23 PM Christian Lamparter wrote:
>
> On 2020-09-15 03:05, Rob Herring wrote:
> > On Sun, Sep 06, 2020 at 12:06:12AM +0200, Christian Lamparter wrote:
> >> This patch adds an DTSI-File that can be used by various device-tree
> >> files for APM82181-based devices.
> >>
> >> Some of the nodes (like UART, PCIE, SATA) are used by the uboot and
> >> need to stick with the naming-conventions of the old times'.
> >> I've added comments whenever this was the case.
> >>
> >> Signed-off-by: Chris Blake
> >> Signed-off-by: Christian Lamparter
> >> ---
> >> rfc v1 -> v2:
> >> - removed PKA (this CryptoPU will need driver)
> >> - stick with compatibles, nodes, ... from either
> >>Bluestone (APM82181) or Canyonlands (PPC460EX).
> >> - add labels for NAND and NOR to help with access.
> >> v2 -> v3:
> >> - nodename of pciex@d was changed to pcie@d..
> >>due to upstream patch.
> >> - use simple-bus on the ebc, opb and plb nodes
> >> ---
> >> arch/powerpc/boot/dts/apm82181.dtsi | 466
> >> 1 file changed, 466 insertions(+)
> >> create mode 100644 arch/powerpc/boot/dts/apm82181.dtsi
> >>
> >> diff --git a/arch/powerpc/boot/dts/apm82181.dtsi
> >> b/arch/powerpc/boot/dts/apm82181.dtsi
> >> new file mode 100644
> >> index ..60283430978d
> >> --- /dev/null
> >> +++ b/arch/powerpc/boot/dts/apm82181.dtsi
> >> @@ -0,0 +1,466 @@
> >> +// SPDX-License-Identifier: GPL-2.0-or-later
> >> +/*
> >> + * Device Tree template include for various APM82181 boards.
> >> + *
> >> + * The SoC is an evolution of the PPC460EX predecessor.
> >> + * This is why dt-nodes from the canyonlands EBC, OPB, USB,
> >> + * DMA, SATA, EMAC, ... ended up in here.
> >> + *
> >> + * Copyright (c) 2010, Applied Micro Circuits Corporation
> >> + * Author: Tirumala R Marri ,
> >> + * Christian Lamparter ,
> >> + * Chris Blake
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +/ {
> >> +#address-cells = <2>;
> >> +#size-cells = <1>;
> >> +dcr-parent = <&{/cpus/cpu@0}>;
> >> +
> >> +aliases {
> >> +ethernet0 = /* needed for BSP u-boot */
> >> +};
> >> +
> >> +cpus {
> >> +#address-cells = <1>;
> >> +#size-cells = <0>;
> >> +
> >> +CPU0: cpu@0 {
> >> +device_type = "cpu";
> >> +model = "PowerPC,apm82181";
> >
> > This doesn't match the existing bluestone dts file.
> >
> > Please separate any restructuring from changes.
>
>
> "I see" (I'm including your comment of the dt-binding as well).
>
> I'm getting the vibe that I better should not touch that bluestone.dts.
I don't know about that.
> An honestly, looking at the series and patches that the APM-engineers
> posted back in the day, I can see why this well is so poisoned... and
> stuff like SATA/AHBDMA/USB/GPIO/CPM/... was missing.
>
> As for the devices. In the spirit of Arnd Bergmann's post of
>
>
> |It would be nice to move over the the bluestone .dts to the apm82181.dtsi
> file
> |when that gets added, if only to ensure they use the same description for
> each
> |node, but that shouldn't stop the addition of the new file if that is needed
> for
> |distros to make use of a popular device.
> |I see a couple of additional files in openwrt.
>
> I mean I don't have the bluestone dev board, just the consumer devices.
This stuff is old enough, I'd guess no one cares about a dev board.
But we should figure that out and document that with any changes.
> Would it be possible to support those? I can start from a "skeleton"
> apm82181.dtsi
> This would just include CPU, Memory (SD-RAM+L2C+OCM), UIC
> (Interrupt-Controller),
> the PLB+OBP+EBC Busses and UART. Just enough to make a board "boot from ram".
This skeleton would be chunks moved over or duplicated? I don't think
we want 2 of the same thing.
The order I would go is split into an apm82181.dtsi with 0 changes to
the built dtb(s). Then make changes/additions you need. As far as
changes to existing bindings, it's only an ABI if someone notices.
Rob
Re: [PATCH v3 2/5] powerpc: apm82181: create shared dtsi for APM bluestone
On 2020-09-15 03:05, Rob Herring wrote:
On Sun, Sep 06, 2020 at 12:06:12AM +0200, Christian Lamparter wrote:
This patch adds an DTSI-File that can be used by various device-tree
files for APM82181-based devices.
Some of the nodes (like UART, PCIE, SATA) are used by the uboot and
need to stick with the naming-conventions of the old times'.
I've added comments whenever this was the case.
Signed-off-by: Chris Blake
Signed-off-by: Christian Lamparter
---
rfc v1 -> v2:
- removed PKA (this CryptoPU will need driver)
- stick with compatibles, nodes, ... from either
Bluestone (APM82181) or Canyonlands (PPC460EX).
- add labels for NAND and NOR to help with access.
v2 -> v3:
- nodename of pciex@d was changed to pcie@d..
due to upstream patch.
- use simple-bus on the ebc, opb and plb nodes
---
arch/powerpc/boot/dts/apm82181.dtsi | 466
1 file changed, 466 insertions(+)
create mode 100644 arch/powerpc/boot/dts/apm82181.dtsi
diff --git a/arch/powerpc/boot/dts/apm82181.dtsi
b/arch/powerpc/boot/dts/apm82181.dtsi
new file mode 100644
index ..60283430978d
--- /dev/null
+++ b/arch/powerpc/boot/dts/apm82181.dtsi
@@ -0,0 +1,466 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Device Tree template include for various APM82181 boards.
+ *
+ * The SoC is an evolution of the PPC460EX predecessor.
+ * This is why dt-nodes from the canyonlands EBC, OPB, USB,
+ * DMA, SATA, EMAC, ... ended up in here.
+ *
+ * Copyright (c) 2010, Applied Micro Circuits Corporation
+ * Author: Tirumala R Marri ,
+ *Christian Lamparter ,
+ *Chris Blake
+ */
+
+#include
+#include
+#include
+#include
+
+/ {
+ #address-cells = <2>;
+ #size-cells = <1>;
+ dcr-parent = <&{/cpus/cpu@0}>;
+
+ aliases {
+ ethernet0 = /* needed for BSP u-boot */
+ };
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ CPU0: cpu@0 {
+ device_type = "cpu";
+ model = "PowerPC,apm82181";
This doesn't match the existing bluestone dts file.
Please separate any restructuring from changes.
"I see" (I'm including your comment of the dt-binding as well).
I'm getting the vibe that I better should not touch that bluestone.dts.
An honestly, looking at the series and patches that the APM-engineers
posted back in the day, I can see why this well is so poisoned... and
stuff like SATA/AHBDMA/USB/GPIO/CPM/... was missing.
As for the devices. In the spirit of Arnd Bergmann's post of
|It would be nice to move over the the bluestone .dts to the apm82181.dtsi file
|when that gets added, if only to ensure they use the same description for each
|node, but that shouldn't stop the addition of the new file if that is needed
for
|distros to make use of a popular device.
|I see a couple of additional files in openwrt.
I mean I don't have the bluestone dev board, just the consumer devices.
Would it be possible to support those? I can start from a "skeleton"
apm82181.dtsi
This would just include CPU, Memory (SD-RAM+L2C+OCM), UIC
(Interrupt-Controller),
the PLB+OBP+EBC Busses and UART. Just enough to make a board "boot from ram".
And then add nodes for PCIE+MSI, AHBDMA+SATA, I2C, Ethernet, NAND+NOR and
finally
the Crypto each in separate patches.
Re: [PATCH v3 2/5] powerpc: apm82181: create shared dtsi for APM bluestone
On Sun, Sep 06, 2020 at 12:06:12AM +0200, Christian Lamparter wrote:
> This patch adds an DTSI-File that can be used by various device-tree
> files for APM82181-based devices.
>
> Some of the nodes (like UART, PCIE, SATA) are used by the uboot and
> need to stick with the naming-conventions of the old times'.
> I've added comments whenever this was the case.
>
> Signed-off-by: Chris Blake
> Signed-off-by: Christian Lamparter
> ---
> rfc v1 -> v2:
> - removed PKA (this CryptoPU will need driver)
> - stick with compatibles, nodes, ... from either
> Bluestone (APM82181) or Canyonlands (PPC460EX).
> - add labels for NAND and NOR to help with access.
> v2 -> v3:
> - nodename of pciex@d was changed to pcie@d..
> due to upstream patch.
> - use simple-bus on the ebc, opb and plb nodes
> ---
> arch/powerpc/boot/dts/apm82181.dtsi | 466
> 1 file changed, 466 insertions(+)
> create mode 100644 arch/powerpc/boot/dts/apm82181.dtsi
>
> diff --git a/arch/powerpc/boot/dts/apm82181.dtsi
> b/arch/powerpc/boot/dts/apm82181.dtsi
> new file mode 100644
> index ..60283430978d
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/apm82181.dtsi
> @@ -0,0 +1,466 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Device Tree template include for various APM82181 boards.
> + *
> + * The SoC is an evolution of the PPC460EX predecessor.
> + * This is why dt-nodes from the canyonlands EBC, OPB, USB,
> + * DMA, SATA, EMAC, ... ended up in here.
> + *
> + * Copyright (c) 2010, Applied Micro Circuits Corporation
> + * Author: Tirumala R Marri ,
> + * Christian Lamparter ,
> + * Chris Blake
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +/ {
> + #address-cells = <2>;
> + #size-cells = <1>;
> + dcr-parent = <&{/cpus/cpu@0}>;
> +
> + aliases {
> + ethernet0 = /* needed for BSP u-boot */
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + CPU0: cpu@0 {
> + device_type = "cpu";
> + model = "PowerPC,apm82181";
This doesn't match the existing bluestone dts file.
Please separate any restructuring from changes.
[PATCH v3 2/5] powerpc: apm82181: create shared dtsi for APM bluestone
This patch adds an DTSI-File that can be used by various device-tree
files for APM82181-based devices.
Some of the nodes (like UART, PCIE, SATA) are used by the uboot and
need to stick with the naming-conventions of the old times'.
I've added comments whenever this was the case.
Signed-off-by: Chris Blake
Signed-off-by: Christian Lamparter
---
rfc v1 -> v2:
- removed PKA (this CryptoPU will need driver)
- stick with compatibles, nodes, ... from either
Bluestone (APM82181) or Canyonlands (PPC460EX).
- add labels for NAND and NOR to help with access.
v2 -> v3:
- nodename of pciex@d was changed to pcie@d..
due to upstream patch.
- use simple-bus on the ebc, opb and plb nodes
---
arch/powerpc/boot/dts/apm82181.dtsi | 466
1 file changed, 466 insertions(+)
create mode 100644 arch/powerpc/boot/dts/apm82181.dtsi
diff --git a/arch/powerpc/boot/dts/apm82181.dtsi
b/arch/powerpc/boot/dts/apm82181.dtsi
new file mode 100644
index ..60283430978d
--- /dev/null
+++ b/arch/powerpc/boot/dts/apm82181.dtsi
@@ -0,0 +1,466 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Device Tree template include for various APM82181 boards.
+ *
+ * The SoC is an evolution of the PPC460EX predecessor.
+ * This is why dt-nodes from the canyonlands EBC, OPB, USB,
+ * DMA, SATA, EMAC, ... ended up in here.
+ *
+ * Copyright (c) 2010, Applied Micro Circuits Corporation
+ * Author: Tirumala R Marri ,
+ *Christian Lamparter ,
+ *Chris Blake
+ */
+
+#include
+#include
+#include
+#include
+
+/ {
+ #address-cells = <2>;
+ #size-cells = <1>;
+ dcr-parent = <&{/cpus/cpu@0}>;
+
+ aliases {
+ ethernet0 = /* needed for BSP u-boot */
+ };
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ CPU0: cpu@0 {
+ device_type = "cpu";
+ model = "PowerPC,apm82181";
+ reg = <0x>;
+ clock-frequency = <0>; /* Filled in by U-Boot */
+ timebase-frequency = <0>; /* Filled in by U-Boot */
+ i-cache-line-size = <32>;
+ d-cache-line-size = <32>;
+ i-cache-size = <32768>;
+ d-cache-size = <32768>;
+ dcr-controller;
+ dcr-access-method = "native";
+ next-level-cache = <>;
+ };
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0x 0x 0x>; /* Filled in by
U-Boot */
+ };
+
+ UIC0: interrupt-controller0 {
+ compatible = "apm,uic-apm82181", "ibm,uic";
+ interrupt-controller;
+ cell-index = <0>;
+ dcr-reg = <0x0c0 0x009>;
+ #address-cells = <0>;
+ #size-cells = <0>;
+ #interrupt-cells = <2>;
+ };
+
+ UIC1: interrupt-controller1 {
+ compatible = "apm,uic-apm82181", "ibm,uic";
+ interrupt-controller;
+ cell-index = <1>;
+ dcr-reg = <0x0d0 0x009>;
+ #address-cells = <0>;
+ #size-cells = <0>;
+ #interrupt-cells = <2>;
+ interrupts = <0x1e IRQ_TYPE_LEVEL_HIGH>,
+<0x1f IRQ_TYPE_LEVEL_HIGH>; /* cascade */
+ interrupt-parent = <>;
+ };
+
+ UIC2: interrupt-controller2 {
+ compatible = "apm,uic-apm82181", "ibm,uic";
+ interrupt-controller;
+ cell-index = <2>;
+ dcr-reg = <0x0e0 0x009>;
+ #address-cells = <0>;
+ #size-cells = <0>;
+ #interrupt-cells = <2>;
+ interrupts = <0x0a IRQ_TYPE_LEVEL_HIGH>,
+<0x0b IRQ_TYPE_LEVEL_HIGH>; /* cascade */
+ interrupt-parent = <>;
+ };
+
+ UIC3: interrupt-controller3 {
+ compatible = "apm,uic-apm82181","ibm,uic";
+ interrupt-controller;
+ cell-index = <3>;
+ dcr-reg = <0x0f0 0x009>;
+ #address-cells = <0>;
+ #size-cells = <0>;
+ #interrupt-cells = <2>;
+ interrupts = <0x10 IRQ_TYPE_LEVEL_HIGH>,
+<0x11 IRQ_TYPE_LEVEL_HIGH>; /* cascade */
+ interrupt-parent = <>;
+ };
+
+ OCM: ocm@40004 {
+ compatible = "ibm,ocm";
+ status = "okay";
+ cell-index = <1>;
+ /* configured in U-Boot */
+ reg = <4 0x0004 0x8000>; /* 32K */
+ };
+
+ SDR0: sdr {
+ compatible = "apm,sdr-apm821xx";
+ dcr-reg = <0x00e 0x002>;
+ };
+
+ CPR0: cpr {
+
