Re: [PATCH u-boot v2019.04-aspeed-openbmc v3 1/3] arm/dts: Add Genesis3 board

[Zev Weiss <zev@bewilderbeest.net>]

On Thu, May 19, 2022 at 08:07:17AM PDT, Patrick Rudolph wrote:
> Add devicetree source file. It uses the evb-ast2500 board files.
> 
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
>  arch/arm/dts/Makefile                 |  1 +
>  arch/arm/dts/ast2500-ibm-genesis3.dts | 28 +++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+)
>  create mode 100644 arch/arm/dts/ast2500-ibm-genesis3.dts
> 
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index a79f885f54..4b026296f0 100755
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -677,6 +677,7 @@ dtb-$(CONFIG_ARCH_BCM6858) += \
>  dtb-$(CONFIG_ARCH_ASPEED) += \
>  	ast2400-evb.dtb \
>  	ast2500-evb.dtb \
> +	ast2500-ibm-genesis3.dtb \

It looks like (with the exception of ast2600-intel.dts, which is an odd 
one out that I'd think we wouldn't want to emulate) the existing 
convention is to name these as just 'ast2x00-systemname' (without the 
vendor name), so it'd be nice to follow that pattern.

Also, this patch is now a bit out of date w.r.t. the current 
v2019.04-aspeed-openbmc branch and no longer applies cleanly; rebasing 
onto the current upstream branch would make it a bit more convenient 
(not that it's a terribly complex merge conflict).

>  	ast2600a0-evb.dtb \
>  	ast2600a1-evb.dtb \
>  	ast2600-bletchley.dtb \
> diff --git a/arch/arm/dts/ast2500-ibm-genesis3.dts b/arch/arm/dts/ast2500-ibm-genesis3.dts
> new file mode 100644
> index 0000000000..932b598063
> --- /dev/null
> +++ b/arch/arm/dts/ast2500-ibm-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 = "spi-flash", "spansion,s25fl256l";

The usual approach with compatible strings is to go from more specific 
to more generic (so that you get whatever available driver support is 
most appropriately tailored to the precise hardware in question) -- 
these should probably be in the opposite order.

> +	};
> +
> +	flash@1 {
> +		compatible = "spi-flash", "spansion,s25fl256l";

...and likewise here of course.

> +	};
> +};
> +
> -- 
> 2.35.3
>