Re: [PATCH 6/8] arm64: dts: s32g2: add VNP-EVB and VNP-RDB2 support

["Andreas Färber" <afaerber@suse.de>]

Hi Chester et al.,

On 05.08.21 08:54, Chester Lin wrote:
> Add initial device-trees of NXP S32G2's Evaluation Board (S32G-VNP-EVB)
> and Reference Design 2 Board (S32G-VNP-RDB2).

"Reference Design Board 2"?

> 
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  arch/arm64/boot/dts/freescale/Makefile        |  2 ++
>  .../arm64/boot/dts/freescale/s32g274a-evb.dts | 21 ++++++++++++++++
>  .../boot/dts/freescale/s32g274a-rdb2.dts      | 25 +++++++++++++++++++
>  3 files changed, 48 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-evb.dts
>  create mode 100644 arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> 
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index 4b4785d86324..2886ddd42894 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -67,4 +67,6 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-ai_ml.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-colibri-eval-v3.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
>  
> +dtb-$(CONFIG_ARCH_S32) += s32g274a-evb.dtb
> +dtb-$(CONFIG_ARCH_S32) += s32g274a-rdb2.dtb

@NXP: Note that since there's no distinction between S32V and S32G on
the Kconfig level, we decided not to add a white line here. If you wish
to visually separate them, that could be changed.

For reference:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/Kconfig.platforms

speaks about S32 (without V), so did not need to get updated for S32G2.

>  dtb-$(CONFIG_ARCH_S32) += s32v234-evb.dtb
> diff --git a/arch/arm64/boot/dts/freescale/s32g274a-evb.dts b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> new file mode 100644
> index 000000000000..a1ae5031730a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/s32g274a-evb.dts
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Copyright (c) 2021 SUSE LLC
> + */
> +
> +/dts-v1/;
> +
> +#include "s32g2.dtsi"
> +
> +/ {
> +	model = "NXP S32G2 Evaluation Board (S32G-VNP-EVB)";
> +	compatible = "fsl,s32g274a-evb", "fsl,s32g2";
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +

Is there any marking on the PCB that a /* ... */ comment could reference
here? Right now uart0 is close to stdout-path above, but that will
change once more device nodes get added and enabled alphabetically.

> +&uart0 {
> +	status = "okay";

No clock-frequency or clocks property needed?

> +};
> diff --git a/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> new file mode 100644
> index 000000000000..b2faae306b70
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/s32g274a-rdb2.dts
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Copyright (c) 2021 SUSE LLC
> + */
> +
> +/dts-v1/;
> +
> +#include "s32g2.dtsi"
> +
> +/ {
> +	model = "NXP S32G2 Reference Design 2 (S32G-VNP-RDB2)";

"Board" missing.

> +	compatible = "fsl,s32g274a-rdb2", "fsl,s32g2";
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +

Comment please.

> +&uart0 {
> +	status = "okay";

No clock-frequency or clocks property needed?

> +};
> +
> +&uart1 {
> +	status = "okay";
> +};

What is uart1? Please add a comment. Does it actually work without
clocks property?

Thanks,
Andreas

-- 
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer
HRB 36809 (AG Nürnberg)