Re: [PATCH 3/3] arm64: dts: imx8mm: Add imx8mm ddr4 evk board support

[Shawn Guo <shawnguo@kernel.org>]

On Tue, Jul 28, 2020 at 02:44:01PM +0800, Jacky Bai wrote:
> Add the board dts support for i.MX8MM DDR4 EVK board.
> 
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/Makefile        |  1 +
>  .../boot/dts/freescale/imx8mm-ddr4-evk.dts    | 77 +++++++++++++++++++
>  2 files changed, 78 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mm-ddr4-evk.dts
> 
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index a39f0a1723e0..417c552480f2 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -29,6 +29,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-qds.dtb
>  dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-lx2160a-rdb.dtb
>  
>  dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mn-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-ddr4-evk.dts b/arch/arm64/boot/dts/freescale/imx8mm-ddr4-evk.dts
> new file mode 100644
> index 000000000000..9cd89182218e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-ddr4-evk.dts
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2020 NXP
> + */
> +
> +/dts-v1/;
> +
> +#include "imx8mm-evk.dtsi"
> +
> +/ {
> +	model = "FSL i.MX8MM DDR4 EVK with CYW43455 WIFI/BT board";
> +	compatible = "fsl,imx8mm-ddr4-evk", "fsl,imx8mm";
> +
> +	leds {
> +		pinctrl-0 = <&pinctrl_gpio_led_2>;
> +
> +		status {
> +			gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +};
> +
> +&ddrc {
> +	operating-points-v2 = <&ddrc_opp_table>;
> +
> +	ddrc_opp_table: opp-table {
> +		compatible = "operating-points-v2";
> +
> +		opp-25M {
> +			opp-hz = /bits/ 64 <25000000>;
> +		};
> +
> +		opp-100M {
> +			opp-hz = /bits/ 64 <100000000>;
> +		};
> +
> +		opp-600M {
> +			opp-hz = /bits/ 64 <600000000>;
> +		};
> +	};

I'm trying to understand how this ddrc_opp_table is determined.  It's
defined by SoC/DDR controller or board/DDR chip?

> +};
> +
> +&gpmi {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_gpmi_nand_1>;
> +	status = "okay";

Please put 'status' at end of property list.

> +	nand-on-flash-bbt;
> +};
> +
> +&iomuxc {
> +	pinctrl_gpmi_nand_1: gpmi-nand-1 {

The suffix "1" isn't really needed, is it?

Shawn

> +		fsl,pins = <
> +			MX8MM_IOMUXC_NAND_ALE_RAWNAND_ALE		0x00000096
> +			MX8MM_IOMUXC_NAND_CE0_B_RAWNAND_CE0_B		0x00000096
> +			MX8MM_IOMUXC_NAND_CE1_B_RAWNAND_CE1_B		0x00000096
> +			MX8MM_IOMUXC_NAND_CLE_RAWNAND_CLE		0x00000096
> +			MX8MM_IOMUXC_NAND_DATA00_RAWNAND_DATA00		0x00000096
> +			MX8MM_IOMUXC_NAND_DATA01_RAWNAND_DATA01		0x00000096
> +			MX8MM_IOMUXC_NAND_DATA02_RAWNAND_DATA02		0x00000096
> +			MX8MM_IOMUXC_NAND_DATA03_RAWNAND_DATA03		0x00000096
> +			MX8MM_IOMUXC_NAND_DATA04_RAWNAND_DATA04		0x00000096
> +			MX8MM_IOMUXC_NAND_DATA05_RAWNAND_DATA05		0x00000096
> +			MX8MM_IOMUXC_NAND_DATA06_RAWNAND_DATA06		0x00000096
> +			MX8MM_IOMUXC_NAND_DATA07_RAWNAND_DATA07		0x00000096
> +			MX8MM_IOMUXC_NAND_RE_B_RAWNAND_RE_B		0x00000096
> +			MX8MM_IOMUXC_NAND_READY_B_RAWNAND_READY_B	0x00000056
> +			MX8MM_IOMUXC_NAND_WE_B_RAWNAND_WE_B		0x00000096
> +			MX8MM_IOMUXC_NAND_WP_B_RAWNAND_WP_B		0x00000096
> +		>;
> +	};
> +
> +	pinctrl_gpio_led_2: gpioled2grp {
> +		fsl,pins = <
> +			MX8MM_IOMUXC_NAND_CE3_B_GPIO3_IO4	0x19
> +		>;
> +	};
> +};
> -- 
> 2.26.2
>