[PATCH 0/4] Use just one DTS file for all Espressobin variants

[PATCH 0/4] Use just one DTS file for all Espressobin variants

[Pali Rohár]

This patch series change Espressobin code to use in U-Boot just one DTS
file for all Espressobin variants. Therefore DT compatible string
globalscale,espressobin-emmc is not used anymore as it is not needed.

It means that setup and compilation of U-Boot for Espressobin is less
complicated and more simple. As there is no need to check for HW details
and just one U-Boot binary would work for all Espressobin variants.

First two patches just revert previous eMMC support and next two patches
add support for eMMC in way that just one DTS file is used and fdtfile
env variable is correctly set for any Espressobin variant.

We have tested that fdtfile env variable is correctly set on Espressobin
variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 RAM.
Also that eMMC is working on Espressobin variant with eMMC.

Pali Roh?r (4):
  Revert "arm64: dts: armada-3720-espressobin: split common parts to
    .dtsi"
  Revert "arm64: dts: a3720: add support for espressobin with populated
    emmc"
  arm: mvebu: Espressobin: Add support for emmc into dts file
  arm: mvebu: Espressobin: Detect presence of emmc at runtime

 arch/arm/dts/Makefile                         |   1 -
 arch/arm/dts/armada-3720-espressobin-emmc.dts |  44 -----
 arch/arm/dts/armada-3720-espressobin.dts      | 186 +++++++++++++++++-
 arch/arm/dts/armada-3720-espressobin.dtsi     | 167 ----------------
 board/Marvell/mvebu_armada-37xx/board.c       |   6 +-
 doc/README.marvell                            |   7 +-
 6 files changed, 186 insertions(+), 225 deletions(-)
 delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
 delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi

-- 
2.20.1

[PATCH 1/4] Revert "arm64: dts: armada-3720-espressobin: split common parts to .dtsi"

[Pali Rohár]

This reverts commit 03bb6a9b1ed7085794c5f167307273d15c99d3f0.
---
 arch/arm/dts/armada-3720-espressobin.dts  | 164 ++++++++++++++++++++-
 arch/arm/dts/armada-3720-espressobin.dtsi | 167 ----------------------
 2 files changed, 157 insertions(+), 174 deletions(-)
 delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi

diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
index 1542d836c0..be67a45870 100644
--- a/arch/arm/dts/armada-3720-espressobin.dts
+++ b/arch/arm/dts/armada-3720-espressobin.dts
@@ -1,20 +1,170 @@
-// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
 /*
- * Device Tree file for Globalscale Marvell ESPRESSOBin Board
+ * Device Tree file for Marvell Armada 3720 community board
+ * (ESPRESSOBin)
  * Copyright (C) 2016 Marvell
  *
- * Romain Perier <romain.perier@free-electrons.com>
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ * Konstantin Porotchkin <kostap@marvell.com>
  *
- */
-/*
- * Schematic available at http://espressobin.net/wp-content/uploads/2017/08/ESPRESSObin_V5_Schematics.pdf
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
  */
 
 /dts-v1/;
 
-#include "armada-3720-espressobin.dtsi"
+#include "armada-372x.dtsi"
 
 / {
 	model = "Globalscale Marvell ESPRESSOBin Board";
 	compatible = "globalscale,espressobin", "marvell,armada3720", "marvell,armada3710";
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	aliases {
+		ethernet0 = &eth0;
+		i2c0 = &i2c0;
+		spi0 = &spi0;
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x00000000 0x00000000 0x20000000>;
+	};
+
+	vcc_sd_reg0: regulator at 0 {
+		compatible = "regulator-gpio";
+		regulator-name = "vcc_sd0";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-type = "voltage";
+		states = <1800000 0x1
+			  3300000 0x0>;
+		gpios = <&gpionb 4 GPIO_ACTIVE_HIGH>;
+	};
+};
+
+&comphy {
+	max-lanes = <3>;
+	phy0 {
+		phy-type = <PHY_TYPE_USB3_HOST0>;
+		phy-speed = <PHY_SPEED_5G>;
+	};
+
+	phy1 {
+		phy-type = <PHY_TYPE_PEX0>;
+		phy-speed = <PHY_SPEED_2_5G>;
+	};
+
+	phy2 {
+		phy-type = <PHY_TYPE_SATA0>;
+		phy-speed = <PHY_SPEED_5G>;
+	};
+};
+
+&eth0 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&rgmii_pins>, <&smi_pins>;
+	phy-mode = "rgmii";
+	phy_addr = <0x1>;
+	fixed-link {
+		speed = <1000>;
+		full-duplex;
+	};
+};
+
+&i2c0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins>;
+	status = "okay";
+};
+
+/* CON3 */
+&sata {
+	status = "okay";
+};
+
+&sdhci0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sdio_pins>;
+	bus-width = <4>;
+	cd-gpios = <&gpionb 3 GPIO_ACTIVE_LOW>;
+	vqmmc-supply = <&vcc_sd_reg0>;
+	status = "okay";
+};
+
+&spi0 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&spi_quad_pins>;
+
+	spi-flash at 0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "st,m25p128", "jedec,spi-nor";
+		reg = <0>; /* Chip select 0 */
+		spi-max-frequency = <50000000>;
+		m25p,fast-read;
+	};
+};
+
+/* Exported on the micro USB connector CON32 through an FTDI */
+&uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>;
+	status = "okay";
+};
+
+/* CON29 */
+&usb2 {
+	status = "okay";
+};
+
+/* CON31 */
+&usb3 {
+	status = "okay";
+};
+
+&pcie0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pcie_pins>;
+	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
+	status = "okay";
 };
diff --git a/arch/arm/dts/armada-3720-espressobin.dtsi b/arch/arm/dts/armada-3720-espressobin.dtsi
deleted file mode 100644
index 05dec89834..0000000000
--- a/arch/arm/dts/armada-3720-espressobin.dtsi
+++ /dev/null
@@ -1,167 +0,0 @@
-/*
- * Device Tree file for Marvell Armada 3720 community board
- * (ESPRESSOBin)
- * Copyright (C) 2016 Marvell
- *
- * Gregory CLEMENT <gregory.clement@free-electrons.com>
- * Konstantin Porotchkin <kostap@marvell.com>
- *
- * This file is dual-licensed: you can use it either under the terms
- * of the GPL or the X11 license, at your option. Note that this dual
- * licensing only applies to this file, and not this project as a
- * whole.
- *
- *  a) This file is free software; you can redistribute it and/or
- *     modify it under the terms of the GNU General Public License as
- *     published by the Free Software Foundation; either version 2 of the
- *     License, or (at your option) any later version.
- *
- *     This file is distributed in the hope that it will be useful
- *     but WITHOUT ANY WARRANTY; without even the implied warranty of
- *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *     GNU General Public License for more details.
- *
- * Or, alternatively
- *
- *  b) Permission is hereby granted, free of charge, to any person
- *     obtaining a copy of this software and associated documentation
- *     files (the "Software"), to deal in the Software without
- *     restriction, including without limitation the rights to use
- *     copy, modify, merge, publish, distribute, sublicense, and/or
- *     sell copies of the Software, and to permit persons to whom the
- *     Software is furnished to do so, subject to the following
- *     conditions:
- *
- *     The above copyright notice and this permission notice shall be
- *     included in all copies or substantial portions of the Software.
- *
- *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
- *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
- *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
- *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
- *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
- *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- *     OTHER DEALINGS IN THE SOFTWARE.
- */
-
-/dts-v1/;
-
-#include "armada-372x.dtsi"
-
-/ {
-	chosen {
-		stdout-path = "serial0:115200n8";
-	};
-
-	aliases {
-		ethernet0 = &eth0;
-		i2c0 = &i2c0;
-		spi0 = &spi0;
-	};
-
-	memory {
-		device_type = "memory";
-		reg = <0x00000000 0x00000000 0x00000000 0x20000000>;
-	};
-
-	vcc_sd_reg0: regulator at 0 {
-		compatible = "regulator-gpio";
-		regulator-name = "vcc_sd0";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <3300000>;
-		regulator-type = "voltage";
-		states = <1800000 0x1
-			  3300000 0x0>;
-		gpios = <&gpionb 4 GPIO_ACTIVE_HIGH>;
-	};
-};
-
-&comphy {
-	max-lanes = <3>;
-	phy0 {
-		phy-type = <PHY_TYPE_USB3_HOST0>;
-		phy-speed = <PHY_SPEED_5G>;
-	};
-
-	phy1 {
-		phy-type = <PHY_TYPE_PEX0>;
-		phy-speed = <PHY_SPEED_2_5G>;
-	};
-
-	phy2 {
-		phy-type = <PHY_TYPE_SATA0>;
-		phy-speed = <PHY_SPEED_5G>;
-	};
-};
-
-&eth0 {
-	status = "okay";
-	pinctrl-names = "default";
-	pinctrl-0 = <&rgmii_pins>, <&smi_pins>;
-	phy-mode = "rgmii";
-	phy_addr = <0x1>;
-	fixed-link {
-		speed = <1000>;
-		full-duplex;
-	};
-};
-
-&i2c0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&i2c1_pins>;
-	status = "okay";
-};
-
-/* CON3 */
-&sata {
-	status = "okay";
-};
-
-&sdhci0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&sdio_pins>;
-	bus-width = <4>;
-	cd-gpios = <&gpionb 3 GPIO_ACTIVE_LOW>;
-	vqmmc-supply = <&vcc_sd_reg0>;
-	status = "okay";
-};
-
-&spi0 {
-	status = "okay";
-	pinctrl-names = "default";
-	pinctrl-0 = <&spi_quad_pins>;
-
-	spi-flash at 0 {
-		#address-cells = <1>;
-		#size-cells = <1>;
-		compatible = "st,m25p128", "jedec,spi-nor";
-		reg = <0>; /* Chip select 0 */
-		spi-max-frequency = <50000000>;
-		m25p,fast-read;
-	};
-};
-
-/* Exported on the micro USB connector CON32 through an FTDI */
-&uart0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&uart1_pins>;
-	status = "okay";
-};
-
-/* CON29 */
-&usb2 {
-	status = "okay";
-};
-
-/* CON31 */
-&usb3 {
-	status = "okay";
-};
-
-&pcie0 {
-	pinctrl-names = "default";
-	pinctrl-0 = <&pcie_pins>;
-	reset-gpios = <&gpiosb 3 GPIO_ACTIVE_LOW>;
-	status = "okay";
-};
-- 
2.20.1

[PATCH 2/4] Revert "arm64: dts: a3720: add support for espressobin with populated emmc"

[Pali Rohár]

This reverts commit f1a43c84a960265309fa8365759de271a70c5a7e.
---
 arch/arm/dts/Makefile                         |  1 -
 arch/arm/dts/armada-3720-espressobin-emmc.dts | 44 -------------------
 doc/README.marvell                            |  7 +--
 3 files changed, 2 insertions(+), 50 deletions(-)
 delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 7d1a369845..3f0e01f0e5 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -204,7 +204,6 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \
 dtb-$(CONFIG_ARCH_MVEBU) +=			\
 	armada-3720-db.dtb			\
 	armada-3720-espressobin.dtb		\
-	armada-3720-espressobin-emmc.dtb	\
 	armada-3720-turris-mox.dtb		\
 	armada-3720-uDPU.dtb			\
 	armada-375-db.dtb			\
diff --git a/arch/arm/dts/armada-3720-espressobin-emmc.dts b/arch/arm/dts/armada-3720-espressobin-emmc.dts
deleted file mode 100644
index 29ccb6a573..0000000000
--- a/arch/arm/dts/armada-3720-espressobin-emmc.dts
+++ /dev/null
@@ -1,44 +0,0 @@
-// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
-/*
- * Device Tree file for Globalscale Marvell ESPRESSOBin Board with eMMC
- * Copyright (C) 2018 Marvell
- *
- * Romain Perier <romain.perier@free-electrons.com>
- * Konstantin Porotchkin <kostap@marvell.com>
- *
- */
-/*
- * Schematic available at http://espressobin.net/wp-content/uploads/2017/08/ESPRESSObin_V5_Schematics.pdf
- */
-
-/dts-v1/;
-
-#include "armada-3720-espressobin.dtsi"
-
-/ {
-	model = "Globalscale Marvell ESPRESSOBin Board (eMMC)";
-	compatible = "globalscale,espressobin-emmc", "globalscale,espressobin",
-		     "marvell,armada3720", "marvell,armada3710";
-};
-
-/* U11 */
-&sdhci1 {
-	non-removable;
-	bus-width = <8>;
-	mmc-ddr-1_8v;
-	mmc-hs400-1_8v;
-	marvell,xenon-emmc;
-	marvell,xenon-tun-count = <9>;
-	marvell,pad-type = "fixed-1-8v";
-
-	pinctrl-names = "default";
-	pinctrl-0 = <&mmc_pins>;
-	status = "okay";
-
-	#address-cells = <1>;
-	#size-cells = <0>;
-	mmccard: mmccard at 0 {
-		compatible = "mmc-card";
-		reg = <0>;
-	};
-};
diff --git a/doc/README.marvell b/doc/README.marvell
index 6f05ad4cb1..6fc5ac8a40 100644
--- a/doc/README.marvell
+++ b/doc/README.marvell
@@ -43,11 +43,8 @@ Build Procedure
         In order to prevent this, the required device-tree MUST be set during compilation.
         All device-tree files are located in ./arch/arm/dts/ folder.
 
-	For the EspressoBin board with populated eMMC device use
-		# make DEVICE_TREE=armada-3720-espressobin-emmc
-
-	For other DB boards (MacchiatoBin, EspressoBin without soldered eMMC and 3700 DB board)
-	compile u-boot with just default device-tree from defconfig using:
+	For other DB boards (MacchiatoBin, EspressoBin and 3700 DB board) compile u-boot with
+	just default device-tree from defconfig using:
 
 		# make
 
-- 
2.20.1

[PATCH 3/4] arm: mvebu: Espressobin: Add support for emmc into dts file

[Pali Rohár]

To simplify setup, configuration and compilation of u-boot, define emmc
node for all Espressobin boards. Espressobin boards without populated emmc
works correctly, just detection and initialization of emmc obviously fails.

Code for emmc is extracted from commit f1a43c84a960 ("arm64: dts: a3720:
add support for espressobin with populated emmc").

Signed-off-by: Pali Roh?r <pali@kernel.org>
Tested-by: G?rald Kerma <gerald@gk2.net>
---
 arch/arm/dts/armada-3720-espressobin.dts | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
index be67a45870..96a4b3d95b 100644
--- a/arch/arm/dts/armada-3720-espressobin.dts
+++ b/arch/arm/dts/armada-3720-espressobin.dts
@@ -130,6 +130,28 @@
 	status = "okay";
 };
 
+/* U11 */
+&sdhci1 {
+	non-removable;
+	bus-width = <8>;
+	mmc-ddr-1_8v;
+	mmc-hs400-1_8v;
+	marvell,xenon-emmc;
+	marvell,xenon-tun-count = <9>;
+	marvell,pad-type = "fixed-1-8v";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc_pins>;
+	status = "okay";
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+	mmccard: mmccard at 0 {
+		compatible = "mmc-card";
+		reg = <0>;
+	};
+};
+
 &spi0 {
 	status = "okay";
 	pinctrl-names = "default";
-- 
2.20.1

[PATCH 4/4] arm: mvebu: Espressobin: Detect presence of emmc at runtime

[Pali Rohár]

Try to initialize emmc in board_late_init() and if it fails then we know
that emmc device is not connected.

This allows to use in U-Boot just one DTS file for all Espressobin variants
and also to correctly set fdtfile env variable for Linux kernel.

Signed-off-by: Pali Roh?r <pali@kernel.org>
Tested-by: G?rald Kerma <gerald@gk2.net>
---
 board/Marvell/mvebu_armada-37xx/board.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
index 73d69e0388..f67b04b78c 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -8,6 +8,7 @@
 #include <env.h>
 #include <i2c.h>
 #include <init.h>
+#include <mmc.h>
 #include <phy.h>
 #include <asm/io.h>
 #include <asm/arch/cpu.h>
@@ -83,6 +84,7 @@ int board_init(void)
 #ifdef CONFIG_BOARD_LATE_INIT
 int board_late_init(void)
 {
+	struct mmc *mmc_dev;
 	bool ddr4, emmc;
 
 	if (env_get("fdtfile"))
@@ -95,7 +97,9 @@ int board_late_init(void)
 	ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
 		& A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
 
-	emmc = of_machine_is_compatible("globalscale,espressobin-emmc");
+	/* eMMC is mmc dev num 1 */
+	mmc_dev = find_mmc_device(1);
+	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
 
 	if (ddr4 && emmc)
 		env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
-- 
2.20.1

[PATCH 4/4] arm: mvebu: Espressobin: Detect presence of emmc at runtime

[Andre Heider]

On 25/11/2020 19:20, Pali Roh?r wrote:
> Try to initialize emmc in board_late_init() and if it fails then we know
> that emmc device is not connected.
> 
> This allows to use in U-Boot just one DTS file for all Espressobin variants
> and also to correctly set fdtfile env variable for Linux kernel.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
> Tested-by: G?rald Kerma <gerald@gk2.net>

Small nit below, with that:
Reviewed-by: Andre Heider <a.heider@gmail.com>

> ---
>   board/Marvell/mvebu_armada-37xx/board.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
> index 73d69e0388..f67b04b78c 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -8,6 +8,7 @@
>   #include <env.h>
>   #include <i2c.h>
>   #include <init.h>
> +#include <mmc.h>
>   #include <phy.h>
>   #include <asm/io.h>
>   #include <asm/arch/cpu.h>
> @@ -83,6 +84,7 @@ int board_init(void)
>   #ifdef CONFIG_BOARD_LATE_INIT
>   int board_late_init(void)
>   {
> +	struct mmc *mmc_dev;
>   	bool ddr4, emmc;
>   
>   	if (env_get("fdtfile"))
> @@ -95,7 +97,9 @@ int board_late_init(void)
>   	ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
>   		& A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
>   
> -	emmc = of_machine_is_compatible("globalscale,espressobin-emmc");
> +	/* eMMC is mmc dev num 1 */
> +	mmc_dev = find_mmc_device(1);
> +	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);

I think you meant "mmc_dev && (mmc_init(mmc_dev) == 0);"?

>   
>   	if (ddr4 && emmc)
>   		env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
> 

[PATCH 4/4] arm: mvebu: Espressobin: Detect presence of emmc at runtime

[Pali Rohár]

On Wednesday 02 December 2020 15:35:08 Andre Heider wrote:
> On 25/11/2020 19:20, Pali Roh?r wrote:
> > Try to initialize emmc in board_late_init() and if it fails then we know
> > that emmc device is not connected.
> > 
> > This allows to use in U-Boot just one DTS file for all Espressobin variants
> > and also to correctly set fdtfile env variable for Linux kernel.
> > 
> > Signed-off-by: Pali Roh?r <pali@kernel.org>
> > Tested-by: G?rald Kerma <gerald@gk2.net>
> 
> Small nit below, with that:
> Reviewed-by: Andre Heider <a.heider@gmail.com>
> 
> > ---
> >   board/Marvell/mvebu_armada-37xx/board.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
> > index 73d69e0388..f67b04b78c 100644
> > --- a/board/Marvell/mvebu_armada-37xx/board.c
> > +++ b/board/Marvell/mvebu_armada-37xx/board.c
> > @@ -8,6 +8,7 @@
> >   #include <env.h>
> >   #include <i2c.h>
> >   #include <init.h>
> > +#include <mmc.h>
> >   #include <phy.h>
> >   #include <asm/io.h>
> >   #include <asm/arch/cpu.h>
> > @@ -83,6 +84,7 @@ int board_init(void)
> >   #ifdef CONFIG_BOARD_LATE_INIT
> >   int board_late_init(void)
> >   {
> > +	struct mmc *mmc_dev;
> >   	bool ddr4, emmc;
> >   	if (env_get("fdtfile"))
> > @@ -95,7 +97,9 @@ int board_late_init(void)
> >   	ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
> >   		& A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
> > -	emmc = of_machine_is_compatible("globalscale,espressobin-emmc");
> > +	/* eMMC is mmc dev num 1 */
> > +	mmc_dev = find_mmc_device(1);
> > +	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
> 
> I think you meant "mmc_dev && (mmc_init(mmc_dev) == 0);"?

Hm... no... I usually do not put parenthesis around || and && operators.

But both variants are same right?

If U-Boot coding style prefers second (your) variant, I do not have
a problem to change it.

Stefan, do you need to change this styling?

> >   	if (ddr4 && emmc)
> >   		env_set("fdtfile", "marvell/armada-3720-espressobin-v7-emmc.dtb");
> > 
> 

[PATCH 4/4] arm: mvebu: Espressobin: Detect presence of emmc at runtime

[Stefan Roese]

On 03.12.20 16:56, Pali Roh?r wrote:
> On Wednesday 02 December 2020 15:35:08 Andre Heider wrote:
>> On 25/11/2020 19:20, Pali Roh?r wrote:
>>> Try to initialize emmc in board_late_init() and if it fails then we know
>>> that emmc device is not connected.
>>>
>>> This allows to use in U-Boot just one DTS file for all Espressobin variants
>>> and also to correctly set fdtfile env variable for Linux kernel.
>>>
>>> Signed-off-by: Pali Roh?r <pali@kernel.org>
>>> Tested-by: G?rald Kerma <gerald@gk2.net>
>>
>> Small nit below, with that:
>> Reviewed-by: Andre Heider <a.heider@gmail.com>
>>
>>> ---
>>>    board/Marvell/mvebu_armada-37xx/board.c | 6 +++++-
>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
>>> index 73d69e0388..f67b04b78c 100644
>>> --- a/board/Marvell/mvebu_armada-37xx/board.c
>>> +++ b/board/Marvell/mvebu_armada-37xx/board.c
>>> @@ -8,6 +8,7 @@
>>>    #include <env.h>
>>>    #include <i2c.h>
>>>    #include <init.h>
>>> +#include <mmc.h>
>>>    #include <phy.h>
>>>    #include <asm/io.h>
>>>    #include <asm/arch/cpu.h>
>>> @@ -83,6 +84,7 @@ int board_init(void)
>>>    #ifdef CONFIG_BOARD_LATE_INIT
>>>    int board_late_init(void)
>>>    {
>>> +	struct mmc *mmc_dev;
>>>    	bool ddr4, emmc;
>>>    	if (env_get("fdtfile"))
>>> @@ -95,7 +97,9 @@ int board_late_init(void)
>>>    	ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
>>>    		& A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
>>> -	emmc = of_machine_is_compatible("globalscale,espressobin-emmc");
>>> +	/* eMMC is mmc dev num 1 */
>>> +	mmc_dev = find_mmc_device(1);
>>> +	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
>>
>> I think you meant "mmc_dev && (mmc_init(mmc_dev) == 0);"?
> 
> Hm... no... I usually do not put parenthesis around || and && operators.
> 
> But both variants are same right?

Yours still has parenthesis around the (a && b) part, which is not
needed. But this is nitpicking AFAIAC.

> If U-Boot coding style prefers second (your) variant, I do not have
> a problem to change it.

checkpatch should complain if something does not match the coding style.

> Stefan, do you need to change this styling?

No, not from my point of view.

Thanks,
Stefan

[PATCH 4/4] arm: mvebu: Espressobin: Detect presence of emmc at runtime

[Pali Rohár]

On Thursday 03 December 2020 17:30:44 Stefan Roese wrote:
> On 03.12.20 16:56, Pali Roh?r wrote:
> > On Wednesday 02 December 2020 15:35:08 Andre Heider wrote:
> > > On 25/11/2020 19:20, Pali Roh?r wrote:
> > > > Try to initialize emmc in board_late_init() and if it fails then we know
> > > > that emmc device is not connected.
> > > > 
> > > > This allows to use in U-Boot just one DTS file for all Espressobin variants
> > > > and also to correctly set fdtfile env variable for Linux kernel.
> > > > 
> > > > Signed-off-by: Pali Roh?r <pali@kernel.org>
> > > > Tested-by: G?rald Kerma <gerald@gk2.net>
> > > 
> > > Small nit below, with that:
> > > Reviewed-by: Andre Heider <a.heider@gmail.com>
> > > 
> > > > ---
> > > >    board/Marvell/mvebu_armada-37xx/board.c | 6 +++++-
> > > >    1 file changed, 5 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
> > > > index 73d69e0388..f67b04b78c 100644
> > > > --- a/board/Marvell/mvebu_armada-37xx/board.c
> > > > +++ b/board/Marvell/mvebu_armada-37xx/board.c
> > > > @@ -8,6 +8,7 @@
> > > >    #include <env.h>
> > > >    #include <i2c.h>
> > > >    #include <init.h>
> > > > +#include <mmc.h>
> > > >    #include <phy.h>
> > > >    #include <asm/io.h>
> > > >    #include <asm/arch/cpu.h>
> > > > @@ -83,6 +84,7 @@ int board_init(void)
> > > >    #ifdef CONFIG_BOARD_LATE_INIT
> > > >    int board_late_init(void)
> > > >    {
> > > > +	struct mmc *mmc_dev;
> > > >    	bool ddr4, emmc;
> > > >    	if (env_get("fdtfile"))
> > > > @@ -95,7 +97,9 @@ int board_late_init(void)
> > > >    	ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
> > > >    		& A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
> > > > -	emmc = of_machine_is_compatible("globalscale,espressobin-emmc");
> > > > +	/* eMMC is mmc dev num 1 */
> > > > +	mmc_dev = find_mmc_device(1);
> > > > +	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
> > > 
> > > I think you meant "mmc_dev && (mmc_init(mmc_dev) == 0);"?
> > 
> > Hm... no... I usually do not put parenthesis around || and && operators.
> > 
> > But both variants are same right?
> 
> Yours still has parenthesis around the (a && b) part, which is not
> needed. But this is nitpicking AFAIAC.
> 
> > If U-Boot coding style prefers second (your) variant, I do not have
> > a problem to change it.
> 
> checkpatch should complain if something does not match the coding style.
> 
> > Stefan, do you need to change this styling?
> 
> No, not from my point of view.

Ok! So based on Philipp and Andre replies, would you be sending these
patches to 2021.01 release? As for this release is scheduled usage of
"globalscale,espressobin-emmc" compatible string in U-Boot, these
patches are removing them and previous versions do not used them.

> Thanks,
> Stefan

[PATCH 4/4] arm: mvebu: Espressobin: Detect presence of emmc at runtime

[Stefan Roese]

On 05.12.20 13:44, Pali Roh?r wrote:
> On Thursday 03 December 2020 17:30:44 Stefan Roese wrote:
>> On 03.12.20 16:56, Pali Roh?r wrote:
>>> On Wednesday 02 December 2020 15:35:08 Andre Heider wrote:
>>>> On 25/11/2020 19:20, Pali Roh?r wrote:
>>>>> Try to initialize emmc in board_late_init() and if it fails then we know
>>>>> that emmc device is not connected.
>>>>>
>>>>> This allows to use in U-Boot just one DTS file for all Espressobin variants
>>>>> and also to correctly set fdtfile env variable for Linux kernel.
>>>>>
>>>>> Signed-off-by: Pali Roh?r <pali@kernel.org>
>>>>> Tested-by: G?rald Kerma <gerald@gk2.net>
>>>>
>>>> Small nit below, with that:
>>>> Reviewed-by: Andre Heider <a.heider@gmail.com>
>>>>
>>>>> ---
>>>>>     board/Marvell/mvebu_armada-37xx/board.c | 6 +++++-
>>>>>     1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
>>>>> index 73d69e0388..f67b04b78c 100644
>>>>> --- a/board/Marvell/mvebu_armada-37xx/board.c
>>>>> +++ b/board/Marvell/mvebu_armada-37xx/board.c
>>>>> @@ -8,6 +8,7 @@
>>>>>     #include <env.h>
>>>>>     #include <i2c.h>
>>>>>     #include <init.h>
>>>>> +#include <mmc.h>
>>>>>     #include <phy.h>
>>>>>     #include <asm/io.h>
>>>>>     #include <asm/arch/cpu.h>
>>>>> @@ -83,6 +84,7 @@ int board_init(void)
>>>>>     #ifdef CONFIG_BOARD_LATE_INIT
>>>>>     int board_late_init(void)
>>>>>     {
>>>>> +	struct mmc *mmc_dev;
>>>>>     	bool ddr4, emmc;
>>>>>     	if (env_get("fdtfile"))
>>>>> @@ -95,7 +97,9 @@ int board_late_init(void)
>>>>>     	ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
>>>>>     		& A3700_MC_CTRL2_SDRAM_TYPE_MASK) == A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
>>>>> -	emmc = of_machine_is_compatible("globalscale,espressobin-emmc");
>>>>> +	/* eMMC is mmc dev num 1 */
>>>>> +	mmc_dev = find_mmc_device(1);
>>>>> +	emmc = (mmc_dev && mmc_init(mmc_dev) == 0);
>>>>
>>>> I think you meant "mmc_dev && (mmc_init(mmc_dev) == 0);"?
>>>
>>> Hm... no... I usually do not put parenthesis around || and && operators.
>>>
>>> But both variants are same right?
>>
>> Yours still has parenthesis around the (a && b) part, which is not
>> needed. But this is nitpicking AFAIAC.
>>
>>> If U-Boot coding style prefers second (your) variant, I do not have
>>> a problem to change it.
>>
>> checkpatch should complain if something does not match the coding style.
>>
>>> Stefan, do you need to change this styling?
>>
>> No, not from my point of view.
> 
> Ok! So based on Philipp and Andre replies, would you be sending these
> patches to 2021.01 release? As for this release is scheduled usage of
> "globalscale,espressobin-emmc" compatible string in U-Boot, these
> patches are removing them and previous versions do not used them.

Yes, I'm preparing a pull request just now.

Thanks,
Stefan

[PATCH 0/4] Use just one DTS file for all Espressobin variants

[Peter Robinson]

> This patch series change Espressobin code to use in U-Boot just one DTS
> file for all Espressobin variants. Therefore DT compatible string
> globalscale,espressobin-emmc is not used anymore as it is not needed.

Does this work if this DT provided to Linux for booting on the
different variants?

> It means that setup and compilation of U-Boot for Espressobin is less
> complicated and more simple. As there is no need to check for HW details
> and just one U-Boot binary would work for all Espressobin variants.
>
> First two patches just revert previous eMMC support and next two patches
> add support for eMMC in way that just one DTS file is used and fdtfile
> env variable is correctly set for any Espressobin variant.
>
> We have tested that fdtfile env variable is correctly set on Espressobin
> variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 RAM.
> Also that eMMC is working on Espressobin variant with eMMC.
>
> Pali Roh?r (4):
>   Revert "arm64: dts: armada-3720-espressobin: split common parts to
>     .dtsi"
>   Revert "arm64: dts: a3720: add support for espressobin with populated
>     emmc"
>   arm: mvebu: Espressobin: Add support for emmc into dts file
>   arm: mvebu: Espressobin: Detect presence of emmc at runtime
>
>  arch/arm/dts/Makefile                         |   1 -
>  arch/arm/dts/armada-3720-espressobin-emmc.dts |  44 -----
>  arch/arm/dts/armada-3720-espressobin.dts      | 186 +++++++++++++++++-
>  arch/arm/dts/armada-3720-espressobin.dtsi     | 167 ----------------
>  board/Marvell/mvebu_armada-37xx/board.c       |   6 +-
>  doc/README.marvell                            |   7 +-
>  6 files changed, 186 insertions(+), 225 deletions(-)
>  delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
>  delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
>
> --
> 2.20.1
>

[PATCH 0/4] Use just one DTS file for all Espressobin variants

[Pali Rohár]

On Wednesday 25 November 2020 18:38:06 Peter Robinson wrote:
> > This patch series change Espressobin code to use in U-Boot just one DTS
> > file for all Espressobin variants. Therefore DT compatible string
> > globalscale,espressobin-emmc is not used anymore as it is not needed.
> 
> Does this work if this DT provided to Linux for booting on the
> different variants?

U-Boot DT is incompatible for booting Linux kernel. It is because comphy
driver in U-Boot is different as in Linux kernel and needs different DT
nodes. But there is a work to port Linux kernel comphy driver to U-Boot.

> > It means that setup and compilation of U-Boot for Espressobin is less
> > complicated and more simple. As there is no need to check for HW details
> > and just one U-Boot binary would work for all Espressobin variants.
> >
> > First two patches just revert previous eMMC support and next two patches
> > add support for eMMC in way that just one DTS file is used and fdtfile
> > env variable is correctly set for any Espressobin variant.
> >
> > We have tested that fdtfile env variable is correctly set on Espressobin
> > variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 RAM.
> > Also that eMMC is working on Espressobin variant with eMMC.
> >
> > Pali Roh?r (4):
> >   Revert "arm64: dts: armada-3720-espressobin: split common parts to
> >     .dtsi"
> >   Revert "arm64: dts: a3720: add support for espressobin with populated
> >     emmc"
> >   arm: mvebu: Espressobin: Add support for emmc into dts file
> >   arm: mvebu: Espressobin: Detect presence of emmc at runtime
> >
> >  arch/arm/dts/Makefile                         |   1 -
> >  arch/arm/dts/armada-3720-espressobin-emmc.dts |  44 -----
> >  arch/arm/dts/armada-3720-espressobin.dts      | 186 +++++++++++++++++-
> >  arch/arm/dts/armada-3720-espressobin.dtsi     | 167 ----------------
> >  board/Marvell/mvebu_armada-37xx/board.c       |   6 +-
> >  doc/README.marvell                            |   7 +-
> >  6 files changed, 186 insertions(+), 225 deletions(-)
> >  delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
> >  delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
> >
> > --
> > 2.20.1
> >

[PATCH 0/4] Use just one DTS file for all Espressobin variants

[Pali Rohár]

On Wednesday 25 November 2020 19:20:06 Pali Roh?r wrote:
> This patch series change Espressobin code to use in U-Boot just one DTS
> file for all Espressobin variants. Therefore DT compatible string
> globalscale,espressobin-emmc is not used anymore as it is not needed.
> 
> It means that setup and compilation of U-Boot for Espressobin is less
> complicated and more simple. As there is no need to check for HW details
> and just one U-Boot binary would work for all Espressobin variants.
> 
> First two patches just revert previous eMMC support and next two patches
> add support for eMMC in way that just one DTS file is used and fdtfile
> env variable is correctly set for any Espressobin variant.
> 
> We have tested that fdtfile env variable is correctly set on Espressobin
> variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 RAM.
> Also that eMMC is working on Espressobin variant with eMMC.

Stefan, could you please review this patch series?

Andre, are you fine with these changes? I would like to get your
acknowledgment or review comment what needs to be changed or improved as
this patch series basically rework your code (which is first reverted
and them implemented in different way).

> Pali Roh?r (4):
>   Revert "arm64: dts: armada-3720-espressobin: split common parts to
>     .dtsi"
>   Revert "arm64: dts: a3720: add support for espressobin with populated
>     emmc"
>   arm: mvebu: Espressobin: Add support for emmc into dts file
>   arm: mvebu: Espressobin: Detect presence of emmc at runtime
> 
>  arch/arm/dts/Makefile                         |   1 -
>  arch/arm/dts/armada-3720-espressobin-emmc.dts |  44 -----
>  arch/arm/dts/armada-3720-espressobin.dts      | 186 +++++++++++++++++-
>  arch/arm/dts/armada-3720-espressobin.dtsi     | 167 ----------------
>  board/Marvell/mvebu_armada-37xx/board.c       |   6 +-
>  doc/README.marvell                            |   7 +-
>  6 files changed, 186 insertions(+), 225 deletions(-)
>  delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
>  delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
> 
> -- 
> 2.20.1
> 

[PATCH 0/4] Use just one DTS file for all Espressobin variants

[Stefan Roese]

On 02.12.20 01:33, Pali Roh?r wrote:
> On Wednesday 25 November 2020 19:20:06 Pali Roh?r wrote:
>> This patch series change Espressobin code to use in U-Boot just one DTS
>> file for all Espressobin variants. Therefore DT compatible string
>> globalscale,espressobin-emmc is not used anymore as it is not needed.
>>
>> It means that setup and compilation of U-Boot for Espressobin is less
>> complicated and more simple. As there is no need to check for HW details
>> and just one U-Boot binary would work for all Espressobin variants.
>>
>> First two patches just revert previous eMMC support and next two patches
>> add support for eMMC in way that just one DTS file is used and fdtfile
>> env variable is correctly set for any Espressobin variant.
>>
>> We have tested that fdtfile env variable is correctly set on Espressobin
>> variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 RAM.
>> Also that eMMC is working on Espressobin variant with eMMC.
> 
> Stefan, could you please review this patch series?

I like the approach in general to simplify things. One comment though:

AFAICT, Linux uses multiple dts/dtsi files for espressobin. So your
approach to move to one single file contradicts the (planned after
comphy conversion) move to the Linux dts/dtsi files.

> Andre, are you fine with these changes? I would like to get your
> acknowledgment or review comment what needs to be changed or improved as
> this patch series basically rework your code (which is first reverted
> and them implemented in different way).

Yes. Andre please also comment on this.

Thanks,
Stefan

>> Pali Roh?r (4):
>>    Revert "arm64: dts: armada-3720-espressobin: split common parts to
>>      .dtsi"
>>    Revert "arm64: dts: a3720: add support for espressobin with populated
>>      emmc"
>>    arm: mvebu: Espressobin: Add support for emmc into dts file
>>    arm: mvebu: Espressobin: Detect presence of emmc at runtime
>>
>>   arch/arm/dts/Makefile                         |   1 -
>>   arch/arm/dts/armada-3720-espressobin-emmc.dts |  44 -----
>>   arch/arm/dts/armada-3720-espressobin.dts      | 186 +++++++++++++++++-
>>   arch/arm/dts/armada-3720-espressobin.dtsi     | 167 ----------------
>>   board/Marvell/mvebu_armada-37xx/board.c       |   6 +-
>>   doc/README.marvell                            |   7 +-
>>   6 files changed, 186 insertions(+), 225 deletions(-)
>>   delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
>>   delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
>>
>> -- 
>> 2.20.1
>>


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 0/4] Use just one DTS file for all Espressobin variants

[Pali Rohár]

On Wednesday 02 December 2020 09:09:15 Stefan Roese wrote:
> On 02.12.20 01:33, Pali Roh?r wrote:
> > On Wednesday 25 November 2020 19:20:06 Pali Roh?r wrote:
> > > This patch series change Espressobin code to use in U-Boot just one DTS
> > > file for all Espressobin variants. Therefore DT compatible string
> > > globalscale,espressobin-emmc is not used anymore as it is not needed.
> > > 
> > > It means that setup and compilation of U-Boot for Espressobin is less
> > > complicated and more simple. As there is no need to check for HW details
> > > and just one U-Boot binary would work for all Espressobin variants.
> > > 
> > > First two patches just revert previous eMMC support and next two patches
> > > add support for eMMC in way that just one DTS file is used and fdtfile
> > > env variable is correctly set for any Espressobin variant.
> > > 
> > > We have tested that fdtfile env variable is correctly set on Espressobin
> > > variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 RAM.
> > > Also that eMMC is working on Espressobin variant with eMMC.
> > 
> > Stefan, could you please review this patch series?
> 
> I like the approach in general to simplify things. One comment though:
> 
> AFAICT, Linux uses multiple dts/dtsi files for espressobin. So your
> approach to move to one single file contradicts the (planned after
> comphy conversion) move to the Linux dts/dtsi files.

After comphy conversion we can use e.g. Linux dtsi file and create one
main U-Boot dts file which would contain all nodes enabled and in U-Boot
code disable nodes which are not present/relevant. This patch series
allows to detect all variants v5, v7, with emmc, without emmc; so we can
reconstruct dts file at U-Boot runtime. In Linux we also simplified dts
files as much as possible, so all options are in common dtsi file and
only variant relevant changes (enable/disable nodes) are in dts files.

> > Andre, are you fine with these changes? I would like to get your
> > acknowledgment or review comment what needs to be changed or improved as
> > this patch series basically rework your code (which is first reverted
> > and them implemented in different way).
> 
> Yes. Andre please also comment on this.
> 
> Thanks,
> Stefan
> 
> > > Pali Roh?r (4):
> > >    Revert "arm64: dts: armada-3720-espressobin: split common parts to
> > >      .dtsi"
> > >    Revert "arm64: dts: a3720: add support for espressobin with populated
> > >      emmc"
> > >    arm: mvebu: Espressobin: Add support for emmc into dts file
> > >    arm: mvebu: Espressobin: Detect presence of emmc at runtime
> > > 
> > >   arch/arm/dts/Makefile                         |   1 -
> > >   arch/arm/dts/armada-3720-espressobin-emmc.dts |  44 -----
> > >   arch/arm/dts/armada-3720-espressobin.dts      | 186 +++++++++++++++++-
> > >   arch/arm/dts/armada-3720-espressobin.dtsi     | 167 ----------------
> > >   board/Marvell/mvebu_armada-37xx/board.c       |   6 +-
> > >   doc/README.marvell                            |   7 +-
> > >   6 files changed, 186 insertions(+), 225 deletions(-)
> > >   delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
> > >   delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
> > > 
> > > -- 
> > > 2.20.1
> > > 
> 
> 
> Viele Gr??e,
> Stefan
> 
> -- 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 0/4] Use just one DTS file for all Espressobin variants

[Stefan Roese]

On 02.12.20 10:12, Pali Roh?r wrote:
> On Wednesday 02 December 2020 09:09:15 Stefan Roese wrote:
>> On 02.12.20 01:33, Pali Roh?r wrote:
>>> On Wednesday 25 November 2020 19:20:06 Pali Roh?r wrote:
>>>> This patch series change Espressobin code to use in U-Boot just one DTS
>>>> file for all Espressobin variants. Therefore DT compatible string
>>>> globalscale,espressobin-emmc is not used anymore as it is not needed.
>>>>
>>>> It means that setup and compilation of U-Boot for Espressobin is less
>>>> complicated and more simple. As there is no need to check for HW details
>>>> and just one U-Boot binary would work for all Espressobin variants.
>>>>
>>>> First two patches just revert previous eMMC support and next two patches
>>>> add support for eMMC in way that just one DTS file is used and fdtfile
>>>> env variable is correctly set for any Espressobin variant.
>>>>
>>>> We have tested that fdtfile env variable is correctly set on Espressobin
>>>> variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 RAM.
>>>> Also that eMMC is working on Espressobin variant with eMMC.
>>>
>>> Stefan, could you please review this patch series?
>>
>> I like the approach in general to simplify things. One comment though:
>>
>> AFAICT, Linux uses multiple dts/dtsi files for espressobin. So your
>> approach to move to one single file contradicts the (planned after
>> comphy conversion) move to the Linux dts/dtsi files.
> 
> After comphy conversion we can use e.g. Linux dtsi file and create one
> main U-Boot dts file which would contain all nodes enabled and in U-Boot
> code disable nodes which are not present/relevant. This patch series
> allows to detect all variants v5, v7, with emmc, without emmc; so we can
> reconstruct dts file at U-Boot runtime. In Linux we also simplified dts
> files as much as possible, so all options are in common dtsi file and
> only variant relevant changes (enable/disable nodes) are in dts files.

I see. So let's please wait for Andre, if he has some comments on this.

Thanks,
Stefan

>>> Andre, are you fine with these changes? I would like to get your
>>> acknowledgment or review comment what needs to be changed or improved as
>>> this patch series basically rework your code (which is first reverted
>>> and them implemented in different way).
>>
>> Yes. Andre please also comment on this.
>>
>> Thanks,
>> Stefan
>>
>>>> Pali Roh?r (4):
>>>>     Revert "arm64: dts: armada-3720-espressobin: split common parts to
>>>>       .dtsi"
>>>>     Revert "arm64: dts: a3720: add support for espressobin with populated
>>>>       emmc"
>>>>     arm: mvebu: Espressobin: Add support for emmc into dts file
>>>>     arm: mvebu: Espressobin: Detect presence of emmc at runtime
>>>>
>>>>    arch/arm/dts/Makefile                         |   1 -
>>>>    arch/arm/dts/armada-3720-espressobin-emmc.dts |  44 -----
>>>>    arch/arm/dts/armada-3720-espressobin.dts      | 186 +++++++++++++++++-
>>>>    arch/arm/dts/armada-3720-espressobin.dtsi     | 167 ----------------
>>>>    board/Marvell/mvebu_armada-37xx/board.c       |   6 +-
>>>>    doc/README.marvell                            |   7 +-
>>>>    6 files changed, 186 insertions(+), 225 deletions(-)
>>>>    delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
>>>>    delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
>>>>
>>>> -- 
>>>> 2.20.1
>>>>
>>
>>
>> Viele Gr??e,
>> Stefan
>>
>> -- 
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH 0/4] Use just one DTS file for all Espressobin variants

[Andre Heider]

On 02/12/2020 11:35, Stefan Roese wrote:
> On 02.12.20 10:12, Pali Roh?r wrote:
>> On Wednesday 02 December 2020 09:09:15 Stefan Roese wrote:
>>> On 02.12.20 01:33, Pali Roh?r wrote:
>>>> On Wednesday 25 November 2020 19:20:06 Pali Roh?r wrote:
>>>>> This patch series change Espressobin code to use in U-Boot just one 
>>>>> DTS
>>>>> file for all Espressobin variants. Therefore DT compatible string
>>>>> globalscale,espressobin-emmc is not used anymore as it is not needed.
>>>>>
>>>>> It means that setup and compilation of U-Boot for Espressobin is less
>>>>> complicated and more simple. As there is no need to check for HW 
>>>>> details
>>>>> and just one U-Boot binary would work for all Espressobin variants.
>>>>>
>>>>> First two patches just revert previous eMMC support and next two 
>>>>> patches
>>>>> add support for eMMC in way that just one DTS file is used and fdtfile
>>>>> env variable is correctly set for any Espressobin variant.
>>>>>
>>>>> We have tested that fdtfile env variable is correctly set on 
>>>>> Espressobin
>>>>> variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 
>>>>> RAM.
>>>>> Also that eMMC is working on Espressobin variant with eMMC.
>>>>
>>>> Stefan, could you please review this patch series?
>>>
>>> I like the approach in general to simplify things. One comment though:
>>>
>>> AFAICT, Linux uses multiple dts/dtsi files for espressobin. So your
>>> approach to move to one single file contradicts the (planned after
>>> comphy conversion) move to the Linux dts/dtsi files.
>>
>> After comphy conversion we can use e.g. Linux dtsi file and create one
>> main U-Boot dts file which would contain all nodes enabled and in U-Boot
>> code disable nodes which are not present/relevant. This patch series
>> allows to detect all variants v5, v7, with emmc, without emmc; so we can
>> reconstruct dts file at U-Boot runtime. In Linux we also simplified dts
>> files as much as possible, so all options are in common dtsi file and
>> only variant relevant changes (enable/disable nodes) are in dts files.
> 
> I see. So let's please wait for Andre, if he has some comments on this.
> 
> Thanks,
> Stefan
> 
>>>> Andre, are you fine with these changes? I would like to get your
>>>> acknowledgment or review comment what needs to be changed or 
>>>> improved as
>>>> this patch series basically rework your code (which is first reverted
>>>> and them implemented in different way).
>>>
>>> Yes. Andre please also comment on this.

This is a nice simplification. Pali had this idea before, and I 
hesitated because of two questions I don't have the answer to:
- Can we just try to detect an emmc chip on an unpopulated board without 
any drawbacks?
- What about power consumption? Does this unnecessarily waste power on 
non-emmc boards because something is still powered up?

But if noone sees an issue with that I don't have any objections.

I think this set could be simplified though. Instead of the first two 
reverts, just remove the non-emmc dts and rename the emmc dts. That's 
less churn and keeps the dtsi for future syncing with linux.

Thanks,
Andre

>>>
>>> Thanks,
>>> Stefan
>>>
>>>>> Pali Roh?r (4):
>>>>> ??? Revert "arm64: dts: armada-3720-espressobin: split common parts to
>>>>> ????? .dtsi"
>>>>> ??? Revert "arm64: dts: a3720: add support for espressobin with 
>>>>> populated
>>>>> ????? emmc"
>>>>> ??? arm: mvebu: Espressobin: Add support for emmc into dts file
>>>>> ??? arm: mvebu: Espressobin: Detect presence of emmc at runtime
>>>>>
>>>>> ?? arch/arm/dts/Makefile???????????????????????? |?? 1 -
>>>>> ?? arch/arm/dts/armada-3720-espressobin-emmc.dts |? 44 -----
>>>>> ?? arch/arm/dts/armada-3720-espressobin.dts????? | 186 
>>>>> +++++++++++++++++-
>>>>> ?? arch/arm/dts/armada-3720-espressobin.dtsi???? | 167 
>>>>> ----------------
>>>>> ?? board/Marvell/mvebu_armada-37xx/board.c?????? |?? 6 +-
>>>>> ?? doc/README.marvell??????????????????????????? |?? 7 +-
>>>>> ?? 6 files changed, 186 insertions(+), 225 deletions(-)
>>>>> ?? delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
>>>>> ?? delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
>>>>>
>>>>> -- 
>>>>> 2.20.1
>>>>>
>>>
>>>
>>> Viele Gr??e,
>>> Stefan
>>>
>>> -- 
>>> DENX Software Engineering GmbH,????? Managing Director: Wolfgang Denk
>>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
> 
> 
> Viele Gr??e,
> Stefan
> 

[PATCH 0/4] Use just one DTS file for all Espressobin variants

[Philipp Tomsich]

> On 02.12.2020, at 13:37, Andre Heider <a.heider@gmail.com> wrote:
> 
> On 02/12/2020 11:35, Stefan Roese wrote:
>> On 02.12.20 10:12, Pali Roh?r wrote:
>>> On Wednesday 02 December 2020 09:09:15 Stefan Roese wrote:
>>>> On 02.12.20 01:33, Pali Roh?r wrote:
>>>>> On Wednesday 25 November 2020 19:20:06 Pali Roh?r wrote:
>>>>>> This patch series change Espressobin code to use in U-Boot just one DTS
>>>>>> file for all Espressobin variants. Therefore DT compatible string
>>>>>> globalscale,espressobin-emmc is not used anymore as it is not needed.
>>>>>> 
>>>>>> It means that setup and compilation of U-Boot for Espressobin is less
>>>>>> complicated and more simple. As there is no need to check for HW details
>>>>>> and just one U-Boot binary would work for all Espressobin variants.
>>>>>> 
>>>>>> First two patches just revert previous eMMC support and next two patches
>>>>>> add support for eMMC in way that just one DTS file is used and fdtfile
>>>>>> env variable is correctly set for any Espressobin variant.
>>>>>> 
>>>>>> We have tested that fdtfile env variable is correctly set on Espressobin
>>>>>> variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 RAM.
>>>>>> Also that eMMC is working on Espressobin variant with eMMC.
>>>>> 
>>>>> Stefan, could you please review this patch series?
>>>> 
>>>> I like the approach in general to simplify things. One comment though:
>>>> 
>>>> AFAICT, Linux uses multiple dts/dtsi files for espressobin. So your
>>>> approach to move to one single file contradicts the (planned after
>>>> comphy conversion) move to the Linux dts/dtsi files.
>>> 
>>> After comphy conversion we can use e.g. Linux dtsi file and create one
>>> main U-Boot dts file which would contain all nodes enabled and in U-Boot
>>> code disable nodes which are not present/relevant. This patch series
>>> allows to detect all variants v5, v7, with emmc, without emmc; so we can
>>> reconstruct dts file at U-Boot runtime. In Linux we also simplified dts
>>> files as much as possible, so all options are in common dtsi file and
>>> only variant relevant changes (enable/disable nodes) are in dts files.
>> I see. So let's please wait for Andre, if he has some comments on this.
>> Thanks,
>> Stefan
>>>>> Andre, are you fine with these changes? I would like to get your
>>>>> acknowledgment or review comment what needs to be changed or improved as
>>>>> this patch series basically rework your code (which is first reverted
>>>>> and them implemented in different way).
>>>> 
>>>> Yes. Andre please also comment on this.
> 
> This is a nice simplification. Pali had this idea before, and I hesitated because of two questions I don't have the answer to:
> - Can we just try to detect an emmc chip on an unpopulated board without any drawbacks?

All eMMC host controllers I know, will eventually return a timeout on the first command(s) to the eMMC.
So the main drawback should be wasted runtime that will slow down the boot sequence.

> - What about power consumption? Does this unnecessarily waste power on non-emmc boards because something is still powered up?
> 
> But if noone sees an issue with that I don't have any objections.
> 
> I think this set could be simplified though. Instead of the first two reverts, just remove the non-emmc dts and rename the emmc dts. That's less churn and keeps the dtsi for future syncing with linux.
> 
> Thanks,
> Andre
> 
>>>> 
>>>> Thanks,
>>>> Stefan
>>>> 
>>>>>> Pali Roh?r (4):
>>>>>>     Revert "arm64: dts: armada-3720-espressobin: split common parts to
>>>>>>       .dtsi"
>>>>>>     Revert "arm64: dts: a3720: add support for espressobin with populated
>>>>>>       emmc"
>>>>>>     arm: mvebu: Espressobin: Add support for emmc into dts file
>>>>>>     arm: mvebu: Espressobin: Detect presence of emmc at runtime
>>>>>> 
>>>>>>    arch/arm/dts/Makefile                         |   1 -
>>>>>>    arch/arm/dts/armada-3720-espressobin-emmc.dts |  44 -----
>>>>>>    arch/arm/dts/armada-3720-espressobin.dts      | 186 +++++++++++++++++-
>>>>>>    arch/arm/dts/armada-3720-espressobin.dtsi     | 167 ----------------
>>>>>>    board/Marvell/mvebu_armada-37xx/board.c       |   6 +-
>>>>>>    doc/README.marvell                            |   7 +-
>>>>>>    6 files changed, 186 insertions(+), 225 deletions(-)
>>>>>>    delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
>>>>>>    delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi
>>>>>> 
>>>>>> -- 
>>>>>> 2.20.1
>>>>>> 
>>>> 
>>>> 
>>>> Viele Gr??e,
>>>> Stefan
>>>> 
>>>> -- 
>>>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>>>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>>>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
>> Viele Gr??e,
>> Stefan

[PATCH 0/4] Use just one DTS file for all Espressobin variants

[Stefan Roese]

On 25.11.20 19:20, Pali Roh?r wrote:
> This patch series change Espressobin code to use in U-Boot just one DTS
> file for all Espressobin variants. Therefore DT compatible string
> globalscale,espressobin-emmc is not used anymore as it is not needed.
> 
> It means that setup and compilation of U-Boot for Espressobin is less
> complicated and more simple. As there is no need to check for HW details
> and just one U-Boot binary would work for all Espressobin variants.
> 
> First two patches just revert previous eMMC support and next two patches
> add support for eMMC in way that just one DTS file is used and fdtfile
> env variable is correctly set for any Espressobin variant.
> 
> We have tested that fdtfile env variable is correctly set on Espressobin
> variants with eMMC, without eMMC, with DDR3 RAM and also with DDR4 RAM.
> Also that eMMC is working on Espressobin variant with eMMC.
> 
> Pali Roh?r (4):
>    Revert "arm64: dts: armada-3720-espressobin: split common parts to
>      .dtsi"
>    Revert "arm64: dts: a3720: add support for espressobin with populated
>      emmc"
>    arm: mvebu: Espressobin: Add support for emmc into dts file
>    arm: mvebu: Espressobin: Detect presence of emmc at runtime
> 
>   arch/arm/dts/Makefile                         |   1 -
>   arch/arm/dts/armada-3720-espressobin-emmc.dts |  44 -----
>   arch/arm/dts/armada-3720-espressobin.dts      | 186 +++++++++++++++++-
>   arch/arm/dts/armada-3720-espressobin.dtsi     | 167 ----------------
>   board/Marvell/mvebu_armada-37xx/board.c       |   6 +-
>   doc/README.marvell                            |   7 +-
>   6 files changed, 186 insertions(+), 225 deletions(-)
>   delete mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
>   delete mode 100644 arch/arm/dts/armada-3720-espressobin.dtsi

Applied to u-boot-marvell/master

Thanks,
Stefan