diff mbox series

[RESEND,u-boot,v2019.04-aspeed-openbmc,v5,1/2] arm/dts: Add IBM Genesis3 board

Message ID 20220630110745.345705-2-patrick.rudolph@9elements.com
State New
Headers show
Series Add support for IBM Genesis3 | expand

Commit Message

Patrick Rudolph June 30, 2022, 11:07 a.m. UTC 
Add devicetree source file. It uses the evb-ast2500 board files.

Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-by: Zev Weiss <zweiss@equinix.com>
---
 arch/arm/dts/Makefile             |  1 +
 arch/arm/dts/ast2500-genesis3.dts | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100644 arch/arm/dts/ast2500-genesis3.dts

Comments

Andrew Jeffery Aug. 10, 2022, 4:22 a.m. UTC | #1 
On Thu, 30 Jun 2022, at 20:37, Patrick Rudolph wrote:
> Add devicetree source file. It uses the evb-ast2500 board files.
>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> Reviewed-by: Zev Weiss <zweiss@equinix.com>
> ---
>  arch/arm/dts/Makefile             |  1 +
>  arch/arm/dts/ast2500-genesis3.dts | 28 ++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
>  create mode 100644 arch/arm/dts/ast2500-genesis3.dts
>
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 3515100c65..ed9714d832 100755
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -678,6 +678,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
>  	ast2400-evb.dtb \
>  	ast2400-ahe-50dc.dtb \
>  	ast2500-evb.dtb \
> +	ast2500-genesis3.dtb \
>  	ast2600a0-evb.dtb \
>  	ast2600a1-evb.dtb \
>  	ast2600-bletchley.dtb \
> diff --git a/arch/arm/dts/ast2500-genesis3.dts 
> b/arch/arm/dts/ast2500-genesis3.dts
> new file mode 100644
> index 0000000000..544758c5b8
> --- /dev/null
> +++ b/arch/arm/dts/ast2500-genesis3.dts
> @@ -0,0 +1,28 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * IBM Genesis3
> + *
> + * Copyright (C) 2022 9elements GmbH
> + */
> +
> +#include "ast2500-evb.dts"
> +
> +/ {
> +	model = "IBM Genesis3";
> +	compatible = "ibm,genesis3-bmc", "aspeed,ast2500";
> +};
> +
> +&spi1 {
> +	status = "disabled";
> +};
> +
> +&fmc {
> +	flash@0 {
> +		compatible = "spansion,s25fl256l", "spi-flash";
> +	};
> +
> +	flash@1 {
> +		compatible = "spansion,s25fl256l", "spi-flash";
> +	};
> +};

So looking at ast2500-evb.dts, you still have both the SD card slots,
both MACs (in RGMII mode) and i2c{3,7} enabled. Is this intended?

Andrew
Patrick Rudolph Aug. 11, 2022, 6 a.m. UTC | #2 
There's no SD card slot, there are two MACs in RGMII mode and nothing
that's required to boot on i2c bus.
In the defconfig used I disabled SD card and I2C support, so this is
not an issue.
Should I still disable it in the devicetree, just for reference?

Regards,
Patrick Rudolph


On Wed, Aug 10, 2022 at 6:23 AM Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Thu, 30 Jun 2022, at 20:37, Patrick Rudolph wrote:
> > Add devicetree source file. It uses the evb-ast2500 board files.
> >
> > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > Reviewed-by: Zev Weiss <zweiss@equinix.com>
> > ---
> >  arch/arm/dts/Makefile             |  1 +
> >  arch/arm/dts/ast2500-genesis3.dts | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> >  create mode 100644 arch/arm/dts/ast2500-genesis3.dts
> >
> > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > index 3515100c65..ed9714d832 100755
> > --- a/arch/arm/dts/Makefile
> > +++ b/arch/arm/dts/Makefile
> > @@ -678,6 +678,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \
> >       ast2400-evb.dtb \
> >       ast2400-ahe-50dc.dtb \
> >       ast2500-evb.dtb \
> > +     ast2500-genesis3.dtb \
> >       ast2600a0-evb.dtb \
> >       ast2600a1-evb.dtb \
> >       ast2600-bletchley.dtb \
> > diff --git a/arch/arm/dts/ast2500-genesis3.dts
> > b/arch/arm/dts/ast2500-genesis3.dts
> > new file mode 100644
> > index 0000000000..544758c5b8
> > --- /dev/null
> > +++ b/arch/arm/dts/ast2500-genesis3.dts
> > @@ -0,0 +1,28 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * IBM Genesis3
> > + *
> > + * Copyright (C) 2022 9elements GmbH
> > + */
> > +
> > +#include "ast2500-evb.dts"
> > +
> > +/ {
> > +     model = "IBM Genesis3";
> > +     compatible = "ibm,genesis3-bmc", "aspeed,ast2500";
> > +};
> > +
> > +&spi1 {
> > +     status = "disabled";
> > +};
> > +
> > +&fmc {
> > +     flash@0 {
> > +             compatible = "spansion,s25fl256l", "spi-flash";
> > +     };
> > +
> > +     flash@1 {
> > +             compatible = "spansion,s25fl256l", "spi-flash";
> > +     };
> > +};
>
> So looking at ast2500-evb.dts, you still have both the SD card slots,
> both MACs (in RGMII mode) and i2c{3,7} enabled. Is this intended?
>
> Andrew
Andrew Jeffery Aug. 11, 2022, 6:16 a.m. UTC | #3 
On Thu, 11 Aug 2022, at 15:30, Patrick Rudolph wrote:
> There's no SD card slot, there are two MACs in RGMII mode and nothing
> that's required to boot on i2c bus.
> In the defconfig used I disabled SD card and I2C support, so this is
> not an issue.
> Should I still disable it in the devicetree, just for reference?

It's at least more idiomatic to `#include "ast2500.dtsi"` and then
enable the controllers you require, rather than #including the dts for a
board not entirely related to the design you're bringing up.

That would be my preference. Is the result much more verbose? It feels 
like it shouldn't be if you only need networking and SPI flash.

Andrew
Patrick Rudolph Aug. 15, 2022, 5:41 a.m. UTC | #4 
Hi Andrew,
the first and second version of this patch were including ast2500.dtsi
and enabled the devices.
Joel and Zev recommended including the EVB dts to have less copy&paste code.

Please advise how to continue. Both versions are working and have been
send to the ML.

Regards,
Patrick


On Thu, Aug 11, 2022 at 8:17 AM Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Thu, 11 Aug 2022, at 15:30, Patrick Rudolph wrote:
> > There's no SD card slot, there are two MACs in RGMII mode and nothing
> > that's required to boot on i2c bus.
> > In the defconfig used I disabled SD card and I2C support, so this is
> > not an issue.
> > Should I still disable it in the devicetree, just for reference?
>
> It's at least more idiomatic to `#include "ast2500.dtsi"` and then
> enable the controllers you require, rather than #including the dts for a
> board not entirely related to the design you're bringing up.
>
> That would be my preference. Is the result much more verbose? It feels
> like it shouldn't be if you only need networking and SPI flash.
>
> Andrew
Andrew Jeffery Aug. 15, 2022, 6:22 a.m. UTC | #5 
On Mon, 15 Aug 2022, at 15:11, Patrick Rudolph wrote:
> Hi Andrew,
> the first and second version of this patch were including ast2500.dtsi
> and enabled the devices.
> Joel and Zev recommended including the EVB dts to have less copy&paste code.

Ah okay, maybe I should have tracked down the earlier revisions for context.

I'll defer to Joel as he'll be merging the patches.

Andrew
diff mbox series

Patch

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 3515100c65..ed9714d832 100755
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -678,6 +678,7 @@  dtb-$(CONFIG_ARCH_ASPEED) += \
 	ast2400-evb.dtb \
 	ast2400-ahe-50dc.dtb \
 	ast2500-evb.dtb \
+	ast2500-genesis3.dtb \
 	ast2600a0-evb.dtb \
 	ast2600a1-evb.dtb \
 	ast2600-bletchley.dtb \
diff --git a/arch/arm/dts/ast2500-genesis3.dts b/arch/arm/dts/ast2500-genesis3.dts
new file mode 100644
index 0000000000..544758c5b8
--- /dev/null
+++ b/arch/arm/dts/ast2500-genesis3.dts
@@ -0,0 +1,28 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IBM Genesis3
+ *
+ * Copyright (C) 2022 9elements GmbH
+ */
+
+#include "ast2500-evb.dts"
+
+/ {
+	model = "IBM Genesis3";
+	compatible = "ibm,genesis3-bmc", "aspeed,ast2500";
+};
+
+&spi1 {
+	status = "disabled";
+};
+
+&fmc {
+	flash@0 {
+		compatible = "spansion,s25fl256l", "spi-flash";
+	};
+
+	flash@1 {
+		compatible = "spansion,s25fl256l", "spi-flash";
+	};
+};
+