All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
@ 2020-08-31  3:34 Andre Heider
  2020-08-31  3:34 ` [PATCH 2/2] arm64: dts: armada-3720-espressobin-emmc: add emmc dts Andre Heider
                   ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: Andre Heider @ 2020-08-31  3:34 UTC (permalink / raw)
  To: u-boot

This adds the disabled eMMC node.

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 arch/arm/dts/armada-3720-espressobin.dts | 63 +++++++++---------------
 1 file changed, 23 insertions(+), 40 deletions(-)

diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
index 4534f5ff29..a66ab814eb 100644
--- a/arch/arm/dts/armada-3720-espressobin.dts
+++ b/arch/arm/dts/armada-3720-espressobin.dts
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0+
 /*
  * Device Tree file for Marvell Armada 3720 community board
  * (ESPRESSOBin)
@@ -5,53 +6,15 @@
  *
  * 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"
 
 / {
 	model = "Marvell Armada 3720 Community Board ESPRESSOBin";
-	compatible = "marvell,armada-3720-espressobin", "marvell,armada3720", "marvell,armada3710";
+	compatible = "marvell,armada-3720-espressobin",
+		     "marvell,armada3720", "marvell,armada3710";
 
 	chosen {
 		stdout-path = "serial0:115200n8";
@@ -121,6 +84,7 @@
 	status = "okay";
 };
 
+/* J1 */
 &sdhci0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&sdio_pins>;
@@ -130,6 +94,25 @@
 	status = "okay";
 };
 
+/* U11 */
+&sdhci1 {
+	non-removable;
+	bus-width = <8>;
+	mmc-ddr-1_8v;
+	mmc-hs400-1_8v;
+	marvell,pad-type = "fixed-1-8v";
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc_pins>;
+	status = "disabled";
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+	mmccard: mmccard at 0 {
+		compatible = "mmc-card";
+		reg = <0>;
+	};
+};
+
 &spi0 {
 	status = "okay";
 	pinctrl-names = "default";
-- 
2.28.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/2] arm64: dts: armada-3720-espressobin-emmc: add emmc dts
  2020-08-31  3:34 [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream Andre Heider
@ 2020-08-31  3:34 ` Andre Heider
  2020-08-31  6:33   ` [PATCH v2 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC Andre Heider
  2020-09-01  7:03   ` [PATCH v3 " Andre Heider
  2020-08-31  7:53 ` [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream Pali Rohár
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 25+ messages in thread
From: Andre Heider @ 2020-08-31  3:34 UTC (permalink / raw)
  To: u-boot

Add the downstream dts for the Espressobin with eMMC.

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 arch/arm/dts/Makefile                         |  1 +
 arch/arm/dts/armada-3720-espressobin-emmc.dts | 19 +++++++++++++++++++
 doc/README.marvell                            |  8 ++++++--
 3 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 5e34192be6..8f1958b5a7 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -202,6 +202,7 @@ 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
new file mode 100644
index 0000000000..0dd59af9c0
--- /dev/null
+++ b/arch/arm/dts/armada-3720-espressobin-emmc.dts
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Marvell International Ltd.
+ */
+
+#include "armada-3720-espressobin.dts"
+
+/ {
+	model = "Marvell Armada 3720 Community Board ESPRESSOBin (eMMC)";
+	compatible = "marvell,armada-3720-espressobin",
+		     "marvell,armada-3720-espressobin-emmc",
+		     "marvell,armada3720", "marvell,armada3710";
+
+};
+
+/* U11 */
+&sdhci1 {
+	status = "okay";
+};
diff --git a/doc/README.marvell b/doc/README.marvell
index 5416bc3035..c5cab2fb38 100644
--- a/doc/README.marvell
+++ b/doc/README.marvell
@@ -43,8 +43,12 @@ 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 other DB boards (MacchiatoBin, EspressoBin and 3700 DB board) compile u-boot with
-	just default device-tree from defconfig using:
+	For the EspressoBin with eMMC compile u-boot and set the required device-tree using:
+
+		# make DEVICE_TREE=armada-3720-espressobin-emmc
+
+	For other DB boards (MacchiatoBin, EspressoBin without eMMC and 3700 DB board) compile
+	u-boot with just default device-tree from defconfig using:
 
 		# make
 
-- 
2.28.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v2 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC
  2020-08-31  3:34 ` [PATCH 2/2] arm64: dts: armada-3720-espressobin-emmc: add emmc dts Andre Heider
@ 2020-08-31  6:33   ` Andre Heider
  2020-08-31  7:55     ` Pali Rohár
  2020-09-01  7:03   ` [PATCH v3 " Andre Heider
  1 sibling, 1 reply; 25+ messages in thread
From: Andre Heider @ 2020-08-31  6:33 UTC (permalink / raw)
  To: u-boot

From: Konstantin Porotchkin <kostap@marvell.com>

Add support for additional EspressoBIN board with installed
eMMC device (U11).
Starting from this patch the DEVICE_TREE= must be added to
"make" in order to distinguish between platforms wth and without
eMMC on board.
Regualr (no eMMC) EspressoBIN builds should use DTS file named
armada-3720-espressobin and build for boards with eMMC installed
the DTS named armada-3720-espressobin-emmc.
The default device tree string is now removed from
mvebu_espressobin-88f3720_defconfig config file.
Update build documentation accordingly.

Change-Id: Id1a4f3ca01a6e52df57bf7279f33f0fe45f8ed18
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Reviewed-on: http://vgitil04.il.marvell.com:8080/61290
Tested-by: iSoC Platform CI <ykjenk@marvell.com>
[a.heider: adapt to mainline]
Signed-off-by: Andre Heider <a.heider@gmail.com>
---
v2: base upon downstream patch

 arch/arm/dts/Makefile                         |  1 +
 arch/arm/dts/armada-3720-espressobin-emmc.dts | 19 +++++++++++++++++++
 configs/mvebu_espressobin-88f3720_defconfig   |  1 -
 doc/README.marvell                            |  7 ++++++-
 4 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 5e34192be6..8f1958b5a7 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -202,6 +202,7 @@ 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
new file mode 100644
index 0000000000..0dd59af9c0
--- /dev/null
+++ b/arch/arm/dts/armada-3720-espressobin-emmc.dts
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Marvell International Ltd.
+ */
+
+#include "armada-3720-espressobin.dts"
+
+/ {
+	model = "Marvell Armada 3720 Community Board ESPRESSOBin (eMMC)";
+	compatible = "marvell,armada-3720-espressobin",
+		     "marvell,armada-3720-espressobin-emmc",
+		     "marvell,armada3720", "marvell,armada3710";
+
+};
+
+/* U11 */
+&sdhci1 {
+	status = "okay";
+};
diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
index 0c1c92d4ff..99f421d841 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -11,7 +11,6 @@ CONFIG_ENV_SECT_SIZE=0x10000
 CONFIG_DM_GPIO=y
 CONFIG_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_CLOCK=25804800
-CONFIG_DEFAULT_DEVICE_TREE="armada-3720-espressobin"
 CONFIG_DEBUG_UART=y
 CONFIG_AHCI=y
 CONFIG_DISTRO_DEFAULTS=y
diff --git a/doc/README.marvell b/doc/README.marvell
index 5416bc3035..ffd0544aef 100644
--- a/doc/README.marvell
+++ b/doc/README.marvell
@@ -43,7 +43,12 @@ 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 other DB boards (MacchiatoBin, EspressoBin and 3700 DB board) compile u-boot with
+	For the EspressoBin board without soldered eMMC device use
+		# make DEVICE_TREE=armada-3720-espressobin
+	For the EspressoBin board with populated eMMC device use
+		# make DEVICE_TREE=armada-3720-espressobin-emmc
+
+	For other DB boards (MacchiatoBin, and 3700 DB board) compile u-boot with
 	just default device-tree from defconfig using:
 
 		# make
-- 
2.28.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-08-31  3:34 [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream Andre Heider
  2020-08-31  3:34 ` [PATCH 2/2] arm64: dts: armada-3720-espressobin-emmc: add emmc dts Andre Heider
@ 2020-08-31  7:53 ` Pali Rohár
  2020-08-31  8:17   ` Andre Heider
  2020-09-04 12:02   ` Stefan Roese
  2020-08-31  8:01 ` Pali Rohár
  2020-09-04  9:01 ` Stefan Roese
  3 siblings, 2 replies; 25+ messages in thread
From: Pali Rohár @ 2020-08-31  7:53 UTC (permalink / raw)
  To: u-boot

On Monday 31 August 2020 05:34:07 Andre Heider wrote:
> This adds the disabled eMMC node.
> 
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
>  arch/arm/dts/armada-3720-espressobin.dts | 63 +++++++++---------------
>  1 file changed, 23 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
> index 4534f5ff29..a66ab814eb 100644
> --- a/arch/arm/dts/armada-3720-espressobin.dts
> +++ b/arch/arm/dts/armada-3720-espressobin.dts
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0+
>  /*
>   * Device Tree file for Marvell Armada 3720 community board
>   * (ESPRESSOBin)
> @@ -5,53 +6,15 @@
>   *
>   * 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.
>   */

You are changing license of whole file. Have all contributors and
copyright holders agreed with it?

> -
>  /dts-v1/;
>  
>  #include "armada-372x.dtsi"
>  
>  / {
>  	model = "Marvell Armada 3720 Community Board ESPRESSOBin";
> -	compatible = "marvell,armada-3720-espressobin", "marvell,armada3720", "marvell,armada3710";
> +	compatible = "marvell,armada-3720-espressobin",
> +		     "marvell,armada3720", "marvell,armada3710";

There is no change on these two lines, right?

>  
>  	chosen {
>  		stdout-path = "serial0:115200n8";
> @@ -121,6 +84,7 @@
>  	status = "okay";
>  };
>  
> +/* J1 */
>  &sdhci0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&sdio_pins>;
> @@ -130,6 +94,25 @@
>  	status = "okay";
>  };
>  
> +/* U11 */
> +&sdhci1 {
> +	non-removable;
> +	bus-width = <8>;
> +	mmc-ddr-1_8v;
> +	mmc-hs400-1_8v;
> +	marvell,pad-type = "fixed-1-8v";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc_pins>;
> +	status = "disabled";
> +
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	mmccard: mmccard at 0 {
> +		compatible = "mmc-card";
> +		reg = <0>;
> +	};
> +};
> +
>  &spi0 {
>  	status = "okay";
>  	pinctrl-names = "default";
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC
  2020-08-31  6:33   ` [PATCH v2 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC Andre Heider
@ 2020-08-31  7:55     ` Pali Rohár
  2020-08-31  8:21       ` Andre Heider
  0 siblings, 1 reply; 25+ messages in thread
From: Pali Rohár @ 2020-08-31  7:55 UTC (permalink / raw)
  To: u-boot

On Monday 31 August 2020 08:33:24 Andre Heider wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
> 
> Add support for additional EspressoBIN board with installed
> eMMC device (U11).
> Starting from this patch the DEVICE_TREE= must be added to

Why? Is not it better to have the most common non-emmc version by
default instead forcing people to specify another compile time option?
IIRC mmc versions are very rare.

> "make" in order to distinguish between platforms wth and without
> eMMC on board.
> Regualr (no eMMC) EspressoBIN builds should use DTS file named
> armada-3720-espressobin and build for boards with eMMC installed
> the DTS named armada-3720-espressobin-emmc.
> The default device tree string is now removed from
> mvebu_espressobin-88f3720_defconfig config file.
> Update build documentation accordingly.
> 
> Change-Id: Id1a4f3ca01a6e52df57bf7279f33f0fe45f8ed18
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Reviewed-on: http://vgitil04.il.marvell.com:8080/61290
> Tested-by: iSoC Platform CI <ykjenk@marvell.com>
> [a.heider: adapt to mainline]
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
> v2: base upon downstream patch
> 
>  arch/arm/dts/Makefile                         |  1 +
>  arch/arm/dts/armada-3720-espressobin-emmc.dts | 19 +++++++++++++++++++
>  configs/mvebu_espressobin-88f3720_defconfig   |  1 -
>  doc/README.marvell                            |  7 ++++++-
>  4 files changed, 26 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
> 
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 5e34192be6..8f1958b5a7 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -202,6 +202,7 @@ 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
> new file mode 100644
> index 0000000000..0dd59af9c0
> --- /dev/null
> +++ b/arch/arm/dts/armada-3720-espressobin-emmc.dts
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Marvell International Ltd.
> + */
> +
> +#include "armada-3720-espressobin.dts"
> +
> +/ {
> +	model = "Marvell Armada 3720 Community Board ESPRESSOBin (eMMC)";
> +	compatible = "marvell,armada-3720-espressobin",
> +		     "marvell,armada-3720-espressobin-emmc",
> +		     "marvell,armada3720", "marvell,armada3710";
> +
> +};
> +
> +/* U11 */
> +&sdhci1 {
> +	status = "okay";
> +};
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
> index 0c1c92d4ff..99f421d841 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -11,7 +11,6 @@ CONFIG_ENV_SECT_SIZE=0x10000
>  CONFIG_DM_GPIO=y
>  CONFIG_DEBUG_UART_BASE=0xd0012000
>  CONFIG_DEBUG_UART_CLOCK=25804800
> -CONFIG_DEFAULT_DEVICE_TREE="armada-3720-espressobin"
>  CONFIG_DEBUG_UART=y
>  CONFIG_AHCI=y
>  CONFIG_DISTRO_DEFAULTS=y
> diff --git a/doc/README.marvell b/doc/README.marvell
> index 5416bc3035..ffd0544aef 100644
> --- a/doc/README.marvell
> +++ b/doc/README.marvell
> @@ -43,7 +43,12 @@ 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 other DB boards (MacchiatoBin, EspressoBin and 3700 DB board) compile u-boot with
> +	For the EspressoBin board without soldered eMMC device use
> +		# make DEVICE_TREE=armada-3720-espressobin
> +	For the EspressoBin board with populated eMMC device use
> +		# make DEVICE_TREE=armada-3720-espressobin-emmc
> +
> +	For other DB boards (MacchiatoBin, and 3700 DB board) compile u-boot with
>  	just default device-tree from defconfig using:
>  
>  		# make
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-08-31  3:34 [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream Andre Heider
  2020-08-31  3:34 ` [PATCH 2/2] arm64: dts: armada-3720-espressobin-emmc: add emmc dts Andre Heider
  2020-08-31  7:53 ` [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream Pali Rohár
@ 2020-08-31  8:01 ` Pali Rohár
  2020-08-31  8:17   ` Andre Heider
  2020-09-04  9:01 ` Stefan Roese
  3 siblings, 1 reply; 25+ messages in thread
From: Pali Rohár @ 2020-08-31  8:01 UTC (permalink / raw)
  To: u-boot

On Monday 31 August 2020 05:34:07 Andre Heider wrote:
> This adds the disabled eMMC node.
> 
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
>  arch/arm/dts/armada-3720-espressobin.dts | 63 +++++++++---------------
>  1 file changed, 23 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
> index 4534f5ff29..a66ab814eb 100644
> --- a/arch/arm/dts/armada-3720-espressobin.dts
> +++ b/arch/arm/dts/armada-3720-espressobin.dts
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0+
>  /*
>   * Device Tree file for Marvell Armada 3720 community board
>   * (ESPRESSOBin)
> @@ -5,53 +6,15 @@
>   *
>   * 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"
>  
>  / {
>  	model = "Marvell Armada 3720 Community Board ESPRESSOBin";
> -	compatible = "marvell,armada-3720-espressobin", "marvell,armada3720", "marvell,armada3710";
> +	compatible = "marvell,armada-3720-espressobin",
> +		     "marvell,armada3720", "marvell,armada3710";
>  
>  	chosen {
>  		stdout-path = "serial0:115200n8";
> @@ -121,6 +84,7 @@
>  	status = "okay";
>  };
>  
> +/* J1 */
>  &sdhci0 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&sdio_pins>;
> @@ -130,6 +94,25 @@
>  	status = "okay";
>  };
>  
> +/* U11 */
> +&sdhci1 {
> +	non-removable;
> +	bus-width = <8>;
> +	mmc-ddr-1_8v;
> +	mmc-hs400-1_8v;
> +	marvell,pad-type = "fixed-1-8v";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc_pins>;
> +	status = "disabled";
> +
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	mmccard: mmccard at 0 {
> +		compatible = "mmc-card";
> +		reg = <0>;
> +	};
> +};

Anyway, Andre, have you tested these Marvell's patches on top of U-Boot
from master git branch? Because espressobin with eMMC is rather rare (do
not know who has this board for testing; do you really have it?) and
from my experience with previous patches I know that Marvell's patches
does not always work on current upstream U-Boot.

> +
>  &spi0 {
>  	status = "okay";
>  	pinctrl-names = "default";
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-08-31  7:53 ` [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream Pali Rohár
@ 2020-08-31  8:17   ` Andre Heider
  2020-09-04 12:02   ` Stefan Roese
  1 sibling, 0 replies; 25+ messages in thread
From: Andre Heider @ 2020-08-31  8:17 UTC (permalink / raw)
  To: u-boot

On 31/08/2020 09:53, Pali Roh?r wrote:
> On Monday 31 August 2020 05:34:07 Andre Heider wrote:
>> This adds the disabled eMMC node.
>>
>> Signed-off-by: Andre Heider <a.heider@gmail.com>
>> ---
>>   arch/arm/dts/armada-3720-espressobin.dts | 63 +++++++++---------------
>>   1 file changed, 23 insertions(+), 40 deletions(-)
>>
>> diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
>> index 4534f5ff29..a66ab814eb 100644
>> --- a/arch/arm/dts/armada-3720-espressobin.dts
>> +++ b/arch/arm/dts/armada-3720-espressobin.dts
>> @@ -1,3 +1,4 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>>   /*
>>    * Device Tree file for Marvell Armada 3720 community board
>>    * (ESPRESSOBin)
>> @@ -5,53 +6,15 @@
>>    *
>>    * 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.
>>    */
> 
> You are changing license of whole file. Have all contributors and
> copyright holders agreed with it?

These are all downstream changes, I haven't added any additional changes!

> 
>> -
>>   /dts-v1/;
>>   
>>   #include "armada-372x.dtsi"
>>   
>>   / {
>>   	model = "Marvell Armada 3720 Community Board ESPRESSOBin";
>> -	compatible = "marvell,armada-3720-espressobin", "marvell,armada3720", "marvell,armada3710";
>> +	compatible = "marvell,armada-3720-espressobin",
>> +		     "marvell,armada3720", "marvell,armada3710";
> 
> There is no change on these two lines, right?

Right, just white spaces.

> 
>>   
>>   	chosen {
>>   		stdout-path = "serial0:115200n8";
>> @@ -121,6 +84,7 @@
>>   	status = "okay";
>>   };
>>   
>> +/* J1 */
>>   &sdhci0 {
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&sdio_pins>;
>> @@ -130,6 +94,25 @@
>>   	status = "okay";
>>   };
>>   
>> +/* U11 */
>> +&sdhci1 {
>> +	non-removable;
>> +	bus-width = <8>;
>> +	mmc-ddr-1_8v;
>> +	mmc-hs400-1_8v;
>> +	marvell,pad-type = "fixed-1-8v";
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&mmc_pins>;
>> +	status = "disabled";
>> +
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	mmccard: mmccard at 0 {
>> +		compatible = "mmc-card";
>> +		reg = <0>;
>> +	};
>> +};
>> +
>>   &spi0 {
>>   	status = "okay";
>>   	pinctrl-names = "default";
>> -- 
>> 2.28.0
>>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-08-31  8:01 ` Pali Rohár
@ 2020-08-31  8:17   ` Andre Heider
  2020-08-31  8:46     ` Pali Rohár
  0 siblings, 1 reply; 25+ messages in thread
From: Andre Heider @ 2020-08-31  8:17 UTC (permalink / raw)
  To: u-boot

On 31/08/2020 10:01, Pali Roh?r wrote:
> On Monday 31 August 2020 05:34:07 Andre Heider wrote:
>> This adds the disabled eMMC node.
>>
>> Signed-off-by: Andre Heider <a.heider@gmail.com>
>> ---
>>   arch/arm/dts/armada-3720-espressobin.dts | 63 +++++++++---------------
>>   1 file changed, 23 insertions(+), 40 deletions(-)
>>
>> diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
>> index 4534f5ff29..a66ab814eb 100644
>> --- a/arch/arm/dts/armada-3720-espressobin.dts
>> +++ b/arch/arm/dts/armada-3720-espressobin.dts
>> @@ -1,3 +1,4 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>>   /*
>>    * Device Tree file for Marvell Armada 3720 community board
>>    * (ESPRESSOBin)
>> @@ -5,53 +6,15 @@
>>    *
>>    * 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"
>>   
>>   / {
>>   	model = "Marvell Armada 3720 Community Board ESPRESSOBin";
>> -	compatible = "marvell,armada-3720-espressobin", "marvell,armada3720", "marvell,armada3710";
>> +	compatible = "marvell,armada-3720-espressobin",
>> +		     "marvell,armada3720", "marvell,armada3710";
>>   
>>   	chosen {
>>   		stdout-path = "serial0:115200n8";
>> @@ -121,6 +84,7 @@
>>   	status = "okay";
>>   };
>>   
>> +/* J1 */
>>   &sdhci0 {
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&sdio_pins>;
>> @@ -130,6 +94,25 @@
>>   	status = "okay";
>>   };
>>   
>> +/* U11 */
>> +&sdhci1 {
>> +	non-removable;
>> +	bus-width = <8>;
>> +	mmc-ddr-1_8v;
>> +	mmc-hs400-1_8v;
>> +	marvell,pad-type = "fixed-1-8v";
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&mmc_pins>;
>> +	status = "disabled";
>> +
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	mmccard: mmccard at 0 {
>> +		compatible = "mmc-card";
>> +		reg = <0>;
>> +	};
>> +};
> 
> Anyway, Andre, have you tested these Marvell's patches on top of U-Boot
> from master git branch? Because espressobin with eMMC is rather rare (do
> not know who has this board for testing; do you really have it?) and
> from my experience with previous patches I know that Marvell's patches
> does not always work on current upstream U-Boot.

I haven't tested it on a espressobin emmc myself, but the node is the 
same as for arch/arm/dts/armada-3720-db.dts and almost the same as for 
arch/arm/dts/armada-3720-uDPU.dts.

Thanks,
Andre

> 
>> +
>>   &spi0 {
>>   	status = "okay";
>>   	pinctrl-names = "default";
>> -- 
>> 2.28.0
>>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC
  2020-08-31  7:55     ` Pali Rohár
@ 2020-08-31  8:21       ` Andre Heider
  2020-08-31  8:27         ` Pali Rohár
  0 siblings, 1 reply; 25+ messages in thread
From: Andre Heider @ 2020-08-31  8:21 UTC (permalink / raw)
  To: u-boot

On 31/08/2020 09:55, Pali Roh?r wrote:
> On Monday 31 August 2020 08:33:24 Andre Heider wrote:
>> From: Konstantin Porotchkin <kostap@marvell.com>
>>
>> Add support for additional EspressoBIN board with installed
>> eMMC device (U11).
>> Starting from this patch the DEVICE_TREE= must be added to
> 
> Why? Is not it better to have the most common non-emmc version by
> default instead forcing people to specify another compile time option?
> IIRC mmc versions are very rare.

That's actually what my v1 did, but other mvebu boards do it as well, as 
does downstream (which is where this patch original comes from). So I 
went ahead and kept it consistent.

But sure, I can change it again.

Thanks,
Andre

>> "make" in order to distinguish between platforms wth and without
>> eMMC on board.
>> Regualr (no eMMC) EspressoBIN builds should use DTS file named
>> armada-3720-espressobin and build for boards with eMMC installed
>> the DTS named armada-3720-espressobin-emmc.
>> The default device tree string is now removed from
>> mvebu_espressobin-88f3720_defconfig config file.
>> Update build documentation accordingly.
>>
>> Change-Id: Id1a4f3ca01a6e52df57bf7279f33f0fe45f8ed18
>> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
>> Reviewed-on: http://vgitil04.il.marvell.com:8080/61290
>> Tested-by: iSoC Platform CI <ykjenk@marvell.com>
>> [a.heider: adapt to mainline]
>> Signed-off-by: Andre Heider <a.heider@gmail.com>
>> ---
>> v2: base upon downstream patch
>>
>>   arch/arm/dts/Makefile                         |  1 +
>>   arch/arm/dts/armada-3720-espressobin-emmc.dts | 19 +++++++++++++++++++
>>   configs/mvebu_espressobin-88f3720_defconfig   |  1 -
>>   doc/README.marvell                            |  7 ++++++-
>>   4 files changed, 26 insertions(+), 2 deletions(-)
>>   create mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
>>
>> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
>> index 5e34192be6..8f1958b5a7 100644
>> --- a/arch/arm/dts/Makefile
>> +++ b/arch/arm/dts/Makefile
>> @@ -202,6 +202,7 @@ 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
>> new file mode 100644
>> index 0000000000..0dd59af9c0
>> --- /dev/null
>> +++ b/arch/arm/dts/armada-3720-espressobin-emmc.dts
>> @@ -0,0 +1,19 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2018 Marvell International Ltd.
>> + */
>> +
>> +#include "armada-3720-espressobin.dts"
>> +
>> +/ {
>> +	model = "Marvell Armada 3720 Community Board ESPRESSOBin (eMMC)";
>> +	compatible = "marvell,armada-3720-espressobin",
>> +		     "marvell,armada-3720-espressobin-emmc",
>> +		     "marvell,armada3720", "marvell,armada3710";
>> +
>> +};
>> +
>> +/* U11 */
>> +&sdhci1 {
>> +	status = "okay";
>> +};
>> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
>> index 0c1c92d4ff..99f421d841 100644
>> --- a/configs/mvebu_espressobin-88f3720_defconfig
>> +++ b/configs/mvebu_espressobin-88f3720_defconfig
>> @@ -11,7 +11,6 @@ CONFIG_ENV_SECT_SIZE=0x10000
>>   CONFIG_DM_GPIO=y
>>   CONFIG_DEBUG_UART_BASE=0xd0012000
>>   CONFIG_DEBUG_UART_CLOCK=25804800
>> -CONFIG_DEFAULT_DEVICE_TREE="armada-3720-espressobin"
>>   CONFIG_DEBUG_UART=y
>>   CONFIG_AHCI=y
>>   CONFIG_DISTRO_DEFAULTS=y
>> diff --git a/doc/README.marvell b/doc/README.marvell
>> index 5416bc3035..ffd0544aef 100644
>> --- a/doc/README.marvell
>> +++ b/doc/README.marvell
>> @@ -43,7 +43,12 @@ 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 other DB boards (MacchiatoBin, EspressoBin and 3700 DB board) compile u-boot with
>> +	For the EspressoBin board without soldered eMMC device use
>> +		# make DEVICE_TREE=armada-3720-espressobin
>> +	For the EspressoBin board with populated eMMC device use
>> +		# make DEVICE_TREE=armada-3720-espressobin-emmc
>> +
>> +	For other DB boards (MacchiatoBin, and 3700 DB board) compile u-boot with
>>   	just default device-tree from defconfig using:
>>   
>>   		# make
>> -- 
>> 2.28.0
>>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC
  2020-08-31  8:21       ` Andre Heider
@ 2020-08-31  8:27         ` Pali Rohár
  2020-08-31  8:41           ` Andre Heider
  0 siblings, 1 reply; 25+ messages in thread
From: Pali Rohár @ 2020-08-31  8:27 UTC (permalink / raw)
  To: u-boot

On Monday 31 August 2020 10:21:46 Andre Heider wrote:
> On 31/08/2020 09:55, Pali Roh?r wrote:
> > On Monday 31 August 2020 08:33:24 Andre Heider wrote:
> > > From: Konstantin Porotchkin <kostap@marvell.com>
> > > 
> > > Add support for additional EspressoBIN board with installed
> > > eMMC device (U11).
> > > Starting from this patch the DEVICE_TREE= must be added to
> > 
> > Why? Is not it better to have the most common non-emmc version by
> > default instead forcing people to specify another compile time option?
> > IIRC mmc versions are very rare.
> 
> That's actually what my v1 did, but other mvebu boards do it as well, as
> does downstream (which is where this patch original comes from). So I went
> ahead and kept it consistent.

If Marvell did something in their 2 years old unmaintained fork it does
not mean that upstream U-Boot must copy+paste whole Marvell code
including suspicious whitespace changes if there is no good reason for
it.

And in my opinion, if we know that eMMC espressobin versions are rare
and DTS file for non-eMMC version is compatible with eMMC espressobin
HW, I do not see reason why not to have DTS file for non-eMMC version
chosen by default. It would really simplify compilation and setup for
most of people. And also it would simplify bisecting git repository for
other Espressobin developers in case of some U-boot failure.

> But sure, I can change it again.
> 
> Thanks,
> Andre
> 
> > > "make" in order to distinguish between platforms wth and without
> > > eMMC on board.
> > > Regualr (no eMMC) EspressoBIN builds should use DTS file named
> > > armada-3720-espressobin and build for boards with eMMC installed
> > > the DTS named armada-3720-espressobin-emmc.
> > > The default device tree string is now removed from
> > > mvebu_espressobin-88f3720_defconfig config file.
> > > Update build documentation accordingly.
> > > 
> > > Change-Id: Id1a4f3ca01a6e52df57bf7279f33f0fe45f8ed18
> > > Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> > > Reviewed-on: http://vgitil04.il.marvell.com:8080/61290
> > > Tested-by: iSoC Platform CI <ykjenk@marvell.com>
> > > [a.heider: adapt to mainline]
> > > Signed-off-by: Andre Heider <a.heider@gmail.com>
> > > ---
> > > v2: base upon downstream patch
> > > 
> > >   arch/arm/dts/Makefile                         |  1 +
> > >   arch/arm/dts/armada-3720-espressobin-emmc.dts | 19 +++++++++++++++++++
> > >   configs/mvebu_espressobin-88f3720_defconfig   |  1 -
> > >   doc/README.marvell                            |  7 ++++++-
> > >   4 files changed, 26 insertions(+), 2 deletions(-)
> > >   create mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
> > > 
> > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > index 5e34192be6..8f1958b5a7 100644
> > > --- a/arch/arm/dts/Makefile
> > > +++ b/arch/arm/dts/Makefile
> > > @@ -202,6 +202,7 @@ 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
> > > new file mode 100644
> > > index 0000000000..0dd59af9c0
> > > --- /dev/null
> > > +++ b/arch/arm/dts/armada-3720-espressobin-emmc.dts
> > > @@ -0,0 +1,19 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2018 Marvell International Ltd.
> > > + */
> > > +
> > > +#include "armada-3720-espressobin.dts"
> > > +
> > > +/ {
> > > +	model = "Marvell Armada 3720 Community Board ESPRESSOBin (eMMC)";
> > > +	compatible = "marvell,armada-3720-espressobin",
> > > +		     "marvell,armada-3720-espressobin-emmc",
> > > +		     "marvell,armada3720", "marvell,armada3710";
> > > +
> > > +};
> > > +
> > > +/* U11 */
> > > +&sdhci1 {
> > > +	status = "okay";
> > > +};
> > > diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
> > > index 0c1c92d4ff..99f421d841 100644
> > > --- a/configs/mvebu_espressobin-88f3720_defconfig
> > > +++ b/configs/mvebu_espressobin-88f3720_defconfig
> > > @@ -11,7 +11,6 @@ CONFIG_ENV_SECT_SIZE=0x10000
> > >   CONFIG_DM_GPIO=y
> > >   CONFIG_DEBUG_UART_BASE=0xd0012000
> > >   CONFIG_DEBUG_UART_CLOCK=25804800
> > > -CONFIG_DEFAULT_DEVICE_TREE="armada-3720-espressobin"
> > >   CONFIG_DEBUG_UART=y
> > >   CONFIG_AHCI=y
> > >   CONFIG_DISTRO_DEFAULTS=y
> > > diff --git a/doc/README.marvell b/doc/README.marvell
> > > index 5416bc3035..ffd0544aef 100644
> > > --- a/doc/README.marvell
> > > +++ b/doc/README.marvell
> > > @@ -43,7 +43,12 @@ 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 other DB boards (MacchiatoBin, EspressoBin and 3700 DB board) compile u-boot with
> > > +	For the EspressoBin board without soldered eMMC device use
> > > +		# make DEVICE_TREE=armada-3720-espressobin
> > > +	For the EspressoBin board with populated eMMC device use
> > > +		# make DEVICE_TREE=armada-3720-espressobin-emmc
> > > +
> > > +	For other DB boards (MacchiatoBin, and 3700 DB board) compile u-boot with
> > >   	just default device-tree from defconfig using:
> > >   		# make
> > > -- 
> > > 2.28.0
> > > 
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC
  2020-08-31  8:27         ` Pali Rohár
@ 2020-08-31  8:41           ` Andre Heider
  0 siblings, 0 replies; 25+ messages in thread
From: Andre Heider @ 2020-08-31  8:41 UTC (permalink / raw)
  To: u-boot

On 31/08/2020 10:27, Pali Roh?r wrote:
> On Monday 31 August 2020 10:21:46 Andre Heider wrote:
>> On 31/08/2020 09:55, Pali Roh?r wrote:
>>> On Monday 31 August 2020 08:33:24 Andre Heider wrote:
>>>> From: Konstantin Porotchkin <kostap@marvell.com>
>>>>
>>>> Add support for additional EspressoBIN board with installed
>>>> eMMC device (U11).
>>>> Starting from this patch the DEVICE_TREE= must be added to
>>>
>>> Why? Is not it better to have the most common non-emmc version by
>>> default instead forcing people to specify another compile time option?
>>> IIRC mmc versions are very rare.
>>
>> That's actually what my v1 did, but other mvebu boards do it as well, as
>> does downstream (which is where this patch original comes from). So I went
>> ahead and kept it consistent.
> 
> If Marvell did something in their 2 years old unmaintained fork it does
> not mean that upstream U-Boot must copy+paste whole Marvell code
> including suspicious whitespace changes if there is no good reason for
> it.

Hehe, of course not, there're changes I didn't carry over :)
And that one whitespace change added a line break to a +80 char line, 
which I opted for.

> And in my opinion, if we know that eMMC espressobin versions are rare
> and DTS file for non-eMMC version is compatible with eMMC espressobin
> HW, I do not see reason why not to have DTS file for non-eMMC version
> chosen by default. It would really simplify compilation and setup for
> most of people. And also it would simplify bisecting git repository for
> other Espressobin developers in case of some U-boot failure.

Those are two good points, thanks. I'll change it!

>> But sure, I can change it again.
>>
>> Thanks,
>> Andre
>>
>>>> "make" in order to distinguish between platforms wth and without
>>>> eMMC on board.
>>>> Regualr (no eMMC) EspressoBIN builds should use DTS file named
>>>> armada-3720-espressobin and build for boards with eMMC installed
>>>> the DTS named armada-3720-espressobin-emmc.
>>>> The default device tree string is now removed from
>>>> mvebu_espressobin-88f3720_defconfig config file.
>>>> Update build documentation accordingly.
>>>>
>>>> Change-Id: Id1a4f3ca01a6e52df57bf7279f33f0fe45f8ed18
>>>> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
>>>> Reviewed-on: http://vgitil04.il.marvell.com:8080/61290
>>>> Tested-by: iSoC Platform CI <ykjenk@marvell.com>
>>>> [a.heider: adapt to mainline]
>>>> Signed-off-by: Andre Heider <a.heider@gmail.com>
>>>> ---
>>>> v2: base upon downstream patch
>>>>
>>>>    arch/arm/dts/Makefile                         |  1 +
>>>>    arch/arm/dts/armada-3720-espressobin-emmc.dts | 19 +++++++++++++++++++
>>>>    configs/mvebu_espressobin-88f3720_defconfig   |  1 -
>>>>    doc/README.marvell                            |  7 ++++++-
>>>>    4 files changed, 26 insertions(+), 2 deletions(-)
>>>>    create mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
>>>>
>>>> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
>>>> index 5e34192be6..8f1958b5a7 100644
>>>> --- a/arch/arm/dts/Makefile
>>>> +++ b/arch/arm/dts/Makefile
>>>> @@ -202,6 +202,7 @@ 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
>>>> new file mode 100644
>>>> index 0000000000..0dd59af9c0
>>>> --- /dev/null
>>>> +++ b/arch/arm/dts/armada-3720-espressobin-emmc.dts
>>>> @@ -0,0 +1,19 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * Copyright (C) 2018 Marvell International Ltd.
>>>> + */
>>>> +
>>>> +#include "armada-3720-espressobin.dts"
>>>> +
>>>> +/ {
>>>> +	model = "Marvell Armada 3720 Community Board ESPRESSOBin (eMMC)";
>>>> +	compatible = "marvell,armada-3720-espressobin",
>>>> +		     "marvell,armada-3720-espressobin-emmc",
>>>> +		     "marvell,armada3720", "marvell,armada3710";
>>>> +
>>>> +};
>>>> +
>>>> +/* U11 */
>>>> +&sdhci1 {
>>>> +	status = "okay";
>>>> +};
>>>> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
>>>> index 0c1c92d4ff..99f421d841 100644
>>>> --- a/configs/mvebu_espressobin-88f3720_defconfig
>>>> +++ b/configs/mvebu_espressobin-88f3720_defconfig
>>>> @@ -11,7 +11,6 @@ CONFIG_ENV_SECT_SIZE=0x10000
>>>>    CONFIG_DM_GPIO=y
>>>>    CONFIG_DEBUG_UART_BASE=0xd0012000
>>>>    CONFIG_DEBUG_UART_CLOCK=25804800
>>>> -CONFIG_DEFAULT_DEVICE_TREE="armada-3720-espressobin"
>>>>    CONFIG_DEBUG_UART=y
>>>>    CONFIG_AHCI=y
>>>>    CONFIG_DISTRO_DEFAULTS=y
>>>> diff --git a/doc/README.marvell b/doc/README.marvell
>>>> index 5416bc3035..ffd0544aef 100644
>>>> --- a/doc/README.marvell
>>>> +++ b/doc/README.marvell
>>>> @@ -43,7 +43,12 @@ 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 other DB boards (MacchiatoBin, EspressoBin and 3700 DB board) compile u-boot with
>>>> +	For the EspressoBin board without soldered eMMC device use
>>>> +		# make DEVICE_TREE=armada-3720-espressobin
>>>> +	For the EspressoBin board with populated eMMC device use
>>>> +		# make DEVICE_TREE=armada-3720-espressobin-emmc
>>>> +
>>>> +	For other DB boards (MacchiatoBin, and 3700 DB board) compile u-boot with
>>>>    	just default device-tree from defconfig using:
>>>>    		# make
>>>> -- 
>>>> 2.28.0
>>>>
>>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-08-31  8:17   ` Andre Heider
@ 2020-08-31  8:46     ` Pali Rohár
  2020-08-31  9:59       ` Andre Heider
  0 siblings, 1 reply; 25+ messages in thread
From: Pali Rohár @ 2020-08-31  8:46 UTC (permalink / raw)
  To: u-boot

On Monday 31 August 2020 10:17:07 Andre Heider wrote:
> On 31/08/2020 10:01, Pali Roh?r wrote:
> > On Monday 31 August 2020 05:34:07 Andre Heider wrote:
> > > +/* U11 */
> > > +&sdhci1 {
> > > +	non-removable;
> > > +	bus-width = <8>;
> > > +	mmc-ddr-1_8v;
> > > +	mmc-hs400-1_8v;
> > > +	marvell,pad-type = "fixed-1-8v";
> > > +	pinctrl-names = "default";
> > > +	pinctrl-0 = <&mmc_pins>;
> > > +	status = "disabled";
> > > +
> > > +	#address-cells = <1>;
> > > +	#size-cells = <0>;
> > > +	mmccard: mmccard at 0 {
> > > +		compatible = "mmc-card";
> > > +		reg = <0>;
> > > +	};
> > > +};
> > 
> > Anyway, Andre, have you tested these Marvell's patches on top of U-Boot
> > from master git branch? Because espressobin with eMMC is rather rare (do
> > not know who has this board for testing; do you really have it?) and
> > from my experience with previous patches I know that Marvell's patches
> > does not always work on current upstream U-Boot.
> 
> I haven't tested it on a espressobin emmc myself, but the node is the same
> as for arch/arm/dts/armada-3720-db.dts and almost the same as for
> arch/arm/dts/armada-3720-uDPU.dts.

I think changes should be tested prior merged them into upstream.
Untested patches can cause other hidden issues.

Also as I wrote, other SD patches for EspressoBin copied+pasted from
Marvell's did not worked as is applied on current upstream U-Boot. And
also argument that "patch is similar as other node" is not reason to not
test changed. As a first thing which I did for SD card support on
EspressoBin was that I copied DTS node from Turris MOX... and on
EspressoBin it did not worked. On MOX was card working fine.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-08-31  8:46     ` Pali Rohár
@ 2020-08-31  9:59       ` Andre Heider
  2020-09-01 20:00         ` Pali Rohár
  0 siblings, 1 reply; 25+ messages in thread
From: Andre Heider @ 2020-08-31  9:59 UTC (permalink / raw)
  To: u-boot

On 31/08/2020 10:46, Pali Roh?r wrote:
> On Monday 31 August 2020 10:17:07 Andre Heider wrote:
>> On 31/08/2020 10:01, Pali Roh?r wrote:
>>> On Monday 31 August 2020 05:34:07 Andre Heider wrote:
>>>> +/* U11 */
>>>> +&sdhci1 {
>>>> +	non-removable;
>>>> +	bus-width = <8>;
>>>> +	mmc-ddr-1_8v;
>>>> +	mmc-hs400-1_8v;
>>>> +	marvell,pad-type = "fixed-1-8v";
>>>> +	pinctrl-names = "default";
>>>> +	pinctrl-0 = <&mmc_pins>;
>>>> +	status = "disabled";
>>>> +
>>>> +	#address-cells = <1>;
>>>> +	#size-cells = <0>;
>>>> +	mmccard: mmccard at 0 {
>>>> +		compatible = "mmc-card";
>>>> +		reg = <0>;
>>>> +	};
>>>> +};
>>>
>>> Anyway, Andre, have you tested these Marvell's patches on top of U-Boot
>>> from master git branch? Because espressobin with eMMC is rather rare (do
>>> not know who has this board for testing; do you really have it?) and
>>> from my experience with previous patches I know that Marvell's patches
>>> does not always work on current upstream U-Boot.
>>
>> I haven't tested it on a espressobin emmc myself, but the node is the same
>> as for arch/arm/dts/armada-3720-db.dts and almost the same as for
>> arch/arm/dts/armada-3720-uDPU.dts.
> 
> I think changes should be tested prior merged them into upstream.
> Untested patches can cause other hidden issues.

Like an utterly broken u-boot for 2 years due to data aborts? :)

> Also as I wrote, other SD patches for EspressoBin copied+pasted from
> Marvell's did not worked as is applied on current upstream U-Boot. And
> also argument that "patch is similar as other node" is not reason to not
> test changed. As a first thing which I did for SD card support on
> EspressoBin was that I copied DTS node from Turris MOX... and on
> EspressoBin it did not worked. On MOX was card working fine.

I would test the patches if I had the hardware, but I just don't.

Maybe someone with an espressobin-emmc reads and tests this.
And I can try to find users on e.g. the openwrt forums.
But I wouldn't hold my breath...

Thanks,
Andre

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC
  2020-08-31  3:34 ` [PATCH 2/2] arm64: dts: armada-3720-espressobin-emmc: add emmc dts Andre Heider
  2020-08-31  6:33   ` [PATCH v2 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC Andre Heider
@ 2020-09-01  7:03   ` Andre Heider
  2020-09-01 20:02     ` Pali Rohár
  2020-09-04  9:02     ` Stefan Roese
  1 sibling, 2 replies; 25+ messages in thread
From: Andre Heider @ 2020-09-01  7:03 UTC (permalink / raw)
  To: u-boot

From: Konstantin Porotchkin <kostap@marvell.com>

Add support for additional EspressoBIN board with installed
eMMC device (U11).
EspressoBIN boards with eMMC installed should use the DTS named
armada-3720-espressobin-emmc.
Update build documentation accordingly.

Change-Id: Id1a4f3ca01a6e52df57bf7279f33f0fe45f8ed18
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Reviewed-on: http://vgitil04.il.marvell.com:8080/61290
Tested-by: iSoC Platform CI <ykjenk@marvell.com>
[a.heider: adapt to mainline]
Signed-off-by: Andre Heider <a.heider@gmail.com>
---
v2: base upon downstream patch
v3: keep CONFIG_DEFAULT_DEVICE_TREE for non-emmc boards

 arch/arm/dts/Makefile                         |  1 +
 arch/arm/dts/armada-3720-espressobin-emmc.dts | 19 +++++++++++++++++++
 doc/README.marvell                            |  7 +++++--
 3 files changed, 25 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 5e34192be6..8f1958b5a7 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -202,6 +202,7 @@ 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
new file mode 100644
index 0000000000..0dd59af9c0
--- /dev/null
+++ b/arch/arm/dts/armada-3720-espressobin-emmc.dts
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Marvell International Ltd.
+ */
+
+#include "armada-3720-espressobin.dts"
+
+/ {
+	model = "Marvell Armada 3720 Community Board ESPRESSOBin (eMMC)";
+	compatible = "marvell,armada-3720-espressobin",
+		     "marvell,armada-3720-espressobin-emmc",
+		     "marvell,armada3720", "marvell,armada3710";
+
+};
+
+/* U11 */
+&sdhci1 {
+	status = "okay";
+};
diff --git a/doc/README.marvell b/doc/README.marvell
index 5416bc3035..be07f31f8c 100644
--- a/doc/README.marvell
+++ b/doc/README.marvell
@@ -43,8 +43,11 @@ 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 other DB boards (MacchiatoBin, EspressoBin and 3700 DB board) compile u-boot with
-	just default device-tree from defconfig using:
+	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:
 
 		# make
 
-- 
2.28.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-08-31  9:59       ` Andre Heider
@ 2020-09-01 20:00         ` Pali Rohár
  0 siblings, 0 replies; 25+ messages in thread
From: Pali Rohár @ 2020-09-01 20:00 UTC (permalink / raw)
  To: u-boot

On Monday 31 August 2020 11:59:28 Andre Heider wrote:
> On 31/08/2020 10:46, Pali Roh?r wrote:
> > On Monday 31 August 2020 10:17:07 Andre Heider wrote:
> > > On 31/08/2020 10:01, Pali Roh?r wrote:
> > > > On Monday 31 August 2020 05:34:07 Andre Heider wrote:
> > > > > +/* U11 */
> > > > > +&sdhci1 {
> > > > > +	non-removable;
> > > > > +	bus-width = <8>;
> > > > > +	mmc-ddr-1_8v;
> > > > > +	mmc-hs400-1_8v;
> > > > > +	marvell,pad-type = "fixed-1-8v";
> > > > > +	pinctrl-names = "default";
> > > > > +	pinctrl-0 = <&mmc_pins>;
> > > > > +	status = "disabled";
> > > > > +
> > > > > +	#address-cells = <1>;
> > > > > +	#size-cells = <0>;
> > > > > +	mmccard: mmccard at 0 {
> > > > > +		compatible = "mmc-card";
> > > > > +		reg = <0>;
> > > > > +	};
> > > > > +};
> > > > 
> > > > Anyway, Andre, have you tested these Marvell's patches on top of U-Boot
> > > > from master git branch? Because espressobin with eMMC is rather rare (do
> > > > not know who has this board for testing; do you really have it?) and
> > > > from my experience with previous patches I know that Marvell's patches
> > > > does not always work on current upstream U-Boot.
> > > 
> > > I haven't tested it on a espressobin emmc myself, but the node is the same
> > > as for arch/arm/dts/armada-3720-db.dts and almost the same as for
> > > arch/arm/dts/armada-3720-uDPU.dts.
> > 
> > I think changes should be tested prior merged them into upstream.
> > Untested patches can cause other hidden issues.
> 
> Like an utterly broken u-boot for 2 years due to data aborts? :)

Exactly!

> > Also as I wrote, other SD patches for EspressoBin copied+pasted from
> > Marvell's did not worked as is applied on current upstream U-Boot. And
> > also argument that "patch is similar as other node" is not reason to not
> > test changed. As a first thing which I did for SD card support on
> > EspressoBin was that I copied DTS node from Turris MOX... and on
> > EspressoBin it did not worked. On MOX was card working fine.
> 
> I would test the patches if I had the hardware, but I just don't.
> 
> Maybe someone with an espressobin-emmc reads and tests this.
> And I can try to find users on e.g. the openwrt forums.
> But I wouldn't hold my breath...

Another option could be to solder eMMC on existing non-eMMC EspressoBin
board, but this is rather very hard without expensive equipment.

Anyway, if there are no users with hardware I think it does not make
sense to try supporting such HW combination.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC
  2020-09-01  7:03   ` [PATCH v3 " Andre Heider
@ 2020-09-01 20:02     ` Pali Rohár
  2020-09-04  9:02     ` Stefan Roese
  1 sibling, 0 replies; 25+ messages in thread
From: Pali Rohár @ 2020-09-01 20:02 UTC (permalink / raw)
  To: u-boot

On Tuesday 01 September 2020 09:03:59 Andre Heider wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
> 
> Add support for additional EspressoBIN board with installed
> eMMC device (U11).
> EspressoBIN boards with eMMC installed should use the DTS named
> armada-3720-espressobin-emmc.
> Update build documentation accordingly.
> 
> Change-Id: Id1a4f3ca01a6e52df57bf7279f33f0fe45f8ed18
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Reviewed-on: http://vgitil04.il.marvell.com:8080/61290
> Tested-by: iSoC Platform CI <ykjenk@marvell.com>
> [a.heider: adapt to mainline]
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
> v2: base upon downstream patch
> v3: keep CONFIG_DEFAULT_DEVICE_TREE for non-emmc boards

Now it looks good for me!

The only missing part is test these changes.

>  arch/arm/dts/Makefile                         |  1 +
>  arch/arm/dts/armada-3720-espressobin-emmc.dts | 19 +++++++++++++++++++
>  doc/README.marvell                            |  7 +++++--
>  3 files changed, 25 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
> 
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 5e34192be6..8f1958b5a7 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -202,6 +202,7 @@ 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
> new file mode 100644
> index 0000000000..0dd59af9c0
> --- /dev/null
> +++ b/arch/arm/dts/armada-3720-espressobin-emmc.dts
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Marvell International Ltd.
> + */
> +
> +#include "armada-3720-espressobin.dts"
> +
> +/ {
> +	model = "Marvell Armada 3720 Community Board ESPRESSOBin (eMMC)";
> +	compatible = "marvell,armada-3720-espressobin",
> +		     "marvell,armada-3720-espressobin-emmc",
> +		     "marvell,armada3720", "marvell,armada3710";
> +
> +};
> +
> +/* U11 */
> +&sdhci1 {
> +	status = "okay";
> +};
> diff --git a/doc/README.marvell b/doc/README.marvell
> index 5416bc3035..be07f31f8c 100644
> --- a/doc/README.marvell
> +++ b/doc/README.marvell
> @@ -43,8 +43,11 @@ 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 other DB boards (MacchiatoBin, EspressoBin and 3700 DB board) compile u-boot with
> -	just default device-tree from defconfig using:
> +	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:
>  
>  		# make
>  
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-08-31  3:34 [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream Andre Heider
                   ` (2 preceding siblings ...)
  2020-08-31  8:01 ` Pali Rohár
@ 2020-09-04  9:01 ` Stefan Roese
  3 siblings, 0 replies; 25+ messages in thread
From: Stefan Roese @ 2020-09-04  9:01 UTC (permalink / raw)
  To: u-boot

On 31.08.20 05:34, Andre Heider wrote:
> This adds the disabled eMMC node.
> 
> Signed-off-by: Andre Heider <a.heider@gmail.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   arch/arm/dts/armada-3720-espressobin.dts | 63 +++++++++---------------
>   1 file changed, 23 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
> index 4534f5ff29..a66ab814eb 100644
> --- a/arch/arm/dts/armada-3720-espressobin.dts
> +++ b/arch/arm/dts/armada-3720-espressobin.dts
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0+
>   /*
>    * Device Tree file for Marvell Armada 3720 community board
>    * (ESPRESSOBin)
> @@ -5,53 +6,15 @@
>    *
>    * 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"
>   
>   / {
>   	model = "Marvell Armada 3720 Community Board ESPRESSOBin";
> -	compatible = "marvell,armada-3720-espressobin", "marvell,armada3720", "marvell,armada3710";
> +	compatible = "marvell,armada-3720-espressobin",
> +		     "marvell,armada3720", "marvell,armada3710";
>   
>   	chosen {
>   		stdout-path = "serial0:115200n8";
> @@ -121,6 +84,7 @@
>   	status = "okay";
>   };
>   
> +/* J1 */
>   &sdhci0 {
>   	pinctrl-names = "default";
>   	pinctrl-0 = <&sdio_pins>;
> @@ -130,6 +94,25 @@
>   	status = "okay";
>   };
>   
> +/* U11 */
> +&sdhci1 {
> +	non-removable;
> +	bus-width = <8>;
> +	mmc-ddr-1_8v;
> +	mmc-hs400-1_8v;
> +	marvell,pad-type = "fixed-1-8v";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mmc_pins>;
> +	status = "disabled";
> +
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	mmccard: mmccard at 0 {
> +		compatible = "mmc-card";
> +		reg = <0>;
> +	};
> +};
> +
>   &spi0 {
>   	status = "okay";
>   	pinctrl-names = "default";
> 


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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC
  2020-09-01  7:03   ` [PATCH v3 " Andre Heider
  2020-09-01 20:02     ` Pali Rohár
@ 2020-09-04  9:02     ` Stefan Roese
  1 sibling, 0 replies; 25+ messages in thread
From: Stefan Roese @ 2020-09-04  9:02 UTC (permalink / raw)
  To: u-boot

On 01.09.20 09:03, Andre Heider wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
> 
> Add support for additional EspressoBIN board with installed
> eMMC device (U11).
> EspressoBIN boards with eMMC installed should use the DTS named
> armada-3720-espressobin-emmc.
> Update build documentation accordingly.
> 
> Change-Id: Id1a4f3ca01a6e52df57bf7279f33f0fe45f8ed18
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Reviewed-on: http://vgitil04.il.marvell.com:8080/61290
> Tested-by: iSoC Platform CI <ykjenk@marvell.com>
> [a.heider: adapt to mainline]
> Signed-off-by: Andre Heider <a.heider@gmail.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> v2: base upon downstream patch
> v3: keep CONFIG_DEFAULT_DEVICE_TREE for non-emmc boards
> 
>   arch/arm/dts/Makefile                         |  1 +
>   arch/arm/dts/armada-3720-espressobin-emmc.dts | 19 +++++++++++++++++++
>   doc/README.marvell                            |  7 +++++--
>   3 files changed, 25 insertions(+), 2 deletions(-)
>   create mode 100644 arch/arm/dts/armada-3720-espressobin-emmc.dts
> 
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index 5e34192be6..8f1958b5a7 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -202,6 +202,7 @@ 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
> new file mode 100644
> index 0000000000..0dd59af9c0
> --- /dev/null
> +++ b/arch/arm/dts/armada-3720-espressobin-emmc.dts
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2018 Marvell International Ltd.
> + */
> +
> +#include "armada-3720-espressobin.dts"
> +
> +/ {
> +	model = "Marvell Armada 3720 Community Board ESPRESSOBin (eMMC)";
> +	compatible = "marvell,armada-3720-espressobin",
> +		     "marvell,armada-3720-espressobin-emmc",
> +		     "marvell,armada3720", "marvell,armada3710";
> +
> +};
> +
> +/* U11 */
> +&sdhci1 {
> +	status = "okay";
> +};
> diff --git a/doc/README.marvell b/doc/README.marvell
> index 5416bc3035..be07f31f8c 100644
> --- a/doc/README.marvell
> +++ b/doc/README.marvell
> @@ -43,8 +43,11 @@ 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 other DB boards (MacchiatoBin, EspressoBin and 3700 DB board) compile u-boot with
> -	just default device-tree from defconfig using:
> +	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:
>   
>   		# make
>   
> 


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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-08-31  7:53 ` [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream Pali Rohár
  2020-08-31  8:17   ` Andre Heider
@ 2020-09-04 12:02   ` Stefan Roese
  2020-09-04 12:35     ` Andre Heider
  1 sibling, 1 reply; 25+ messages in thread
From: Stefan Roese @ 2020-09-04 12:02 UTC (permalink / raw)
  To: u-boot

Hi Andre,

On 31.08.20 09:53, Pali Roh?r wrote:
> On Monday 31 August 2020 05:34:07 Andre Heider wrote:
>> This adds the disabled eMMC node.
>>
>> Signed-off-by: Andre Heider <a.heider@gmail.com>
>> ---
>>   arch/arm/dts/armada-3720-espressobin.dts | 63 +++++++++---------------
>>   1 file changed, 23 insertions(+), 40 deletions(-)
>>
>> diff --git a/arch/arm/dts/armada-3720-espressobin.dts b/arch/arm/dts/armada-3720-espressobin.dts
>> index 4534f5ff29..a66ab814eb 100644
>> --- a/arch/arm/dts/armada-3720-espressobin.dts
>> +++ b/arch/arm/dts/armada-3720-espressobin.dts
>> @@ -1,3 +1,4 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>>   /*
>>    * Device Tree file for Marvell Armada 3720 community board
>>    * (ESPRESSOBin)
>> @@ -5,53 +6,15 @@
>>    *
>>    * 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.
>>    */
> 
> You are changing license of whole file. Have all contributors and
> copyright holders agreed with it?

First I though that you have been syncing the file with the Linux kernel
version. But now I see that its sync'ed with downstream U-Boot most
likely. As for the license of the file: The Linux kernel version has
this SPDX tag:

// SPDX-License-Identifier: (GPL-2.0+ OR MIT)

Changing this to this tag would haven been fine (AFAICT). But using a
diffent one from a downstream U-Boot repository is a bit troublesome
for my taste.

Can't you sync with Linux dts/dtsi files instead at some point?

Thanks,
Stefan

>> -
>>   /dts-v1/;
>>   
>>   #include "armada-372x.dtsi"
>>   
>>   / {
>>   	model = "Marvell Armada 3720 Community Board ESPRESSOBin";
>> -	compatible = "marvell,armada-3720-espressobin", "marvell,armada3720", "marvell,armada3710";
>> +	compatible = "marvell,armada-3720-espressobin",
>> +		     "marvell,armada3720", "marvell,armada3710";
> 
> There is no change on these two lines, right?
> 
>>   
>>   	chosen {
>>   		stdout-path = "serial0:115200n8";
>> @@ -121,6 +84,7 @@
>>   	status = "okay";
>>   };
>>   
>> +/* J1 */
>>   &sdhci0 {
>>   	pinctrl-names = "default";
>>   	pinctrl-0 = <&sdio_pins>;
>> @@ -130,6 +94,25 @@
>>   	status = "okay";
>>   };
>>   
>> +/* U11 */
>> +&sdhci1 {
>> +	non-removable;
>> +	bus-width = <8>;
>> +	mmc-ddr-1_8v;
>> +	mmc-hs400-1_8v;
>> +	marvell,pad-type = "fixed-1-8v";
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&mmc_pins>;
>> +	status = "disabled";
>> +
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	mmccard: mmccard at 0 {
>> +		compatible = "mmc-card";
>> +		reg = <0>;
>> +	};
>> +};
>> +
>>   &spi0 {
>>   	status = "okay";
>>   	pinctrl-names = "default";
>> -- 
>> 2.28.0
>>


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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-09-04 12:02   ` Stefan Roese
@ 2020-09-04 12:35     ` Andre Heider
  2020-09-04 12:40       ` Stefan Roese
  2020-09-06 16:03       ` Peter Robinson
  0 siblings, 2 replies; 25+ messages in thread
From: Andre Heider @ 2020-09-04 12:35 UTC (permalink / raw)
  To: u-boot

Hi Stefan,

On 04/09/2020 14:02, Stefan Roese wrote:
> Hi Andre,
> 
> On 31.08.20 09:53, Pali Roh?r wrote:
>> On Monday 31 August 2020 05:34:07 Andre Heider wrote:
>>> This adds the disabled eMMC node.
>>>
>>> Signed-off-by: Andre Heider <a.heider@gmail.com>
>>> ---
>>> ? arch/arm/dts/armada-3720-espressobin.dts | 63 +++++++++---------------
>>> ? 1 file changed, 23 insertions(+), 40 deletions(-)
>>>
>>> diff --git a/arch/arm/dts/armada-3720-espressobin.dts 
>>> b/arch/arm/dts/armada-3720-espressobin.dts
>>> index 4534f5ff29..a66ab814eb 100644
>>> --- a/arch/arm/dts/armada-3720-espressobin.dts
>>> +++ b/arch/arm/dts/armada-3720-espressobin.dts
>>> @@ -1,3 +1,4 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> ? /*
>>> ?? * Device Tree file for Marvell Armada 3720 community board
>>> ?? * (ESPRESSOBin)
>>> @@ -5,53 +6,15 @@
>>> ?? *
>>> ?? * 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.
>>> ?? */
>>
>> You are changing license of whole file. Have all contributors and
>> copyright holders agreed with it?
> 
> First I though that you have been syncing the file with the Linux kernel
> version. But now I see that its sync'ed with downstream U-Boot most
> likely. As for the license of the file: The Linux kernel version has
> this SPDX tag:
> 
> // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> 
> Changing this to this tag would haven been fine (AFAICT). But using a
> diffent one from a downstream U-Boot repository is a bit troublesome
> for my taste.
> 
> Can't you sync with Linux dts/dtsi files instead at some point?

at some point, yes. As far as I understand that's not yet possible due 
to the comphy driver. Quoting Marek's cover letter "[PATCH 
u-boot-marvell 00/11] Armada 37xx: port comphy to generic-phys (PLEASE 
TEST)" from 19th April:

 > I have ported the COMPHY and UTMI PHY drivers from Linux to U-Boot.
 > With this we are able to abandon the current comphy_a3700 driver, which
 > is incompatible with Linux' device trees. So if we want to have DTS
 > files for A3720 boards identical to Linux', we have to do this.

So unfortunately we can't use Linux' dts files for a3700 yet. Marek said 
he plans to work on that set again in the near future, so hopefully that 
lands rather sooner than later ;)

Thanks,
Andre

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-09-04 12:35     ` Andre Heider
@ 2020-09-04 12:40       ` Stefan Roese
  2020-09-04 15:36         ` Andre Heider
  2020-09-06 16:03       ` Peter Robinson
  1 sibling, 1 reply; 25+ messages in thread
From: Stefan Roese @ 2020-09-04 12:40 UTC (permalink / raw)
  To: u-boot

Hi Andre,

On 04.09.20 14:35, Andre Heider wrote:

<snip>

>> First I though that you have been syncing the file with the Linux kernel
>> version. But now I see that its sync'ed with downstream U-Boot most
>> likely. As for the license of the file: The Linux kernel version has
>> this SPDX tag:
>>
>> // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>
>> Changing this to this tag would haven been fine (AFAICT). But using a
>> diffent one from a downstream U-Boot repository is a bit troublesome
>> for my taste.
>>
>> Can't you sync with Linux dts/dtsi files instead at some point?
> 
> at some point, yes. As far as I understand that's not yet possible due 
> to the comphy driver. Quoting Marek's cover letter "[PATCH 
> u-boot-marvell 00/11] Armada 37xx: port comphy to generic-phys (PLEASE 
> TEST)" from 19th April:
> 
>  > I have ported the COMPHY and UTMI PHY drivers from Linux to U-Boot.
>  > With this we are able to abandon the current comphy_a3700 driver, which
>  > is incompatible with Linux' device trees. So if we want to have DTS
>  > files for A3720 boards identical to Linux', we have to do this.
> 
> So unfortunately we can't use Linux' dts files for a3700 yet. Marek said 
> he plans to work on that set again in the near future, so hopefully that 
> lands rather sooner than later ;)

Ah, thanks. I remember now. ;)

Can you then please drop the sync with the downstream dts and better
only make the necessary changes for eMMC? In a git bisectable way
please?

I know that nobody can test this right now. Perhaps Kostya or somebody
at Marvell be help out here?

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-09-04 12:40       ` Stefan Roese
@ 2020-09-04 15:36         ` Andre Heider
  2020-09-06  9:21           ` Pali Rohár
  0 siblings, 1 reply; 25+ messages in thread
From: Andre Heider @ 2020-09-04 15:36 UTC (permalink / raw)
  To: u-boot

On 04/09/2020 14:40, Stefan Roese wrote:
> Hi Andre,
> 
> On 04.09.20 14:35, Andre Heider wrote:
> 
> <snip>
> 
>>> First I though that you have been syncing the file with the Linux kernel
>>> version. But now I see that its sync'ed with downstream U-Boot most
>>> likely. As for the license of the file: The Linux kernel version has
>>> this SPDX tag:
>>>
>>> // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>
>>> Changing this to this tag would haven been fine (AFAICT). But using a
>>> diffent one from a downstream U-Boot repository is a bit troublesome
>>> for my taste.
>>>
>>> Can't you sync with Linux dts/dtsi files instead at some point?
>>
>> at some point, yes. As far as I understand that's not yet possible due 
>> to the comphy driver. Quoting Marek's cover letter "[PATCH 
>> u-boot-marvell 00/11] Armada 37xx: port comphy to generic-phys (PLEASE 
>> TEST)" from 19th April:
>>
>> ?> I have ported the COMPHY and UTMI PHY drivers from Linux to U-Boot.
>> ?> With this we are able to abandon the current comphy_a3700 driver, 
>> which
>> ?> is incompatible with Linux' device trees. So if we want to have DTS
>> ?> files for A3720 boards identical to Linux', we have to do this.
>>
>> So unfortunately we can't use Linux' dts files for a3700 yet. Marek 
>> said he plans to work on that set again in the near future, so 
>> hopefully that lands rather sooner than later ;)
> 
> Ah, thanks. I remember now. ;)
> 
> Can you then please drop the sync with the downstream dts and better
> only make the necessary changes for eMMC? In a git bisectable way
> please?

Sure, I've just sent v2, completely redone. That also prepares synching 
from Linux, actually already imports two dts from from there.

Regards,
Andre

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-09-04 15:36         ` Andre Heider
@ 2020-09-06  9:21           ` Pali Rohár
  0 siblings, 0 replies; 25+ messages in thread
From: Pali Rohár @ 2020-09-06  9:21 UTC (permalink / raw)
  To: u-boot

On Friday 04 September 2020 17:36:18 Andre Heider wrote:
> On 04/09/2020 14:40, Stefan Roese wrote:
> > Hi Andre,
> > 
> > On 04.09.20 14:35, Andre Heider wrote:
> > 
> > <snip>
> > 
> > > > First I though that you have been syncing the file with the Linux kernel
> > > > version. But now I see that its sync'ed with downstream U-Boot most
> > > > likely. As for the license of the file: The Linux kernel version has
> > > > this SPDX tag:
> > > > 
> > > > // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > > > 
> > > > Changing this to this tag would haven been fine (AFAICT). But using a
> > > > diffent one from a downstream U-Boot repository is a bit troublesome
> > > > for my taste.
> > > > 
> > > > Can't you sync with Linux dts/dtsi files instead at some point?
> > > 
> > > at some point, yes. As far as I understand that's not yet possible
> > > due to the comphy driver. Quoting Marek's cover letter "[PATCH
> > > u-boot-marvell 00/11] Armada 37xx: port comphy to generic-phys
> > > (PLEASE TEST)" from 19th April:
> > > 
> > > ?> I have ported the COMPHY and UTMI PHY drivers from Linux to U-Boot.
> > > ?> With this we are able to abandon the current comphy_a3700 driver,
> > > which
> > > ?> is incompatible with Linux' device trees. So if we want to have DTS
> > > ?> files for A3720 boards identical to Linux', we have to do this.
> > > 
> > > So unfortunately we can't use Linux' dts files for a3700 yet. Marek
> > > said he plans to work on that set again in the near future, so
> > > hopefully that lands rather sooner than later ;)
> > 
> > Ah, thanks. I remember now. ;)
> > 
> > Can you then please drop the sync with the downstream dts and better
> > only make the necessary changes for eMMC? In a git bisectable way
> > please?
> 
> Sure, I've just sent v2, completely redone. That also prepares synching from
> Linux, actually already imports two dts from from there.

So synchronization with Linux DTS files needs to wait until Armada
comphy patches are finished and merged into mainline U-Boot.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-09-04 12:35     ` Andre Heider
  2020-09-04 12:40       ` Stefan Roese
@ 2020-09-06 16:03       ` Peter Robinson
  2020-09-06 16:11         ` Pali Rohár
  1 sibling, 1 reply; 25+ messages in thread
From: Peter Robinson @ 2020-09-06 16:03 UTC (permalink / raw)
  To: u-boot

On Fri, Sep 4, 2020 at 1:35 PM Andre Heider <a.heider@gmail.com> wrote:
>
> Hi Stefan,
>
> On 04/09/2020 14:02, Stefan Roese wrote:
> > Hi Andre,
> >
> > On 31.08.20 09:53, Pali Roh?r wrote:
> >> On Monday 31 August 2020 05:34:07 Andre Heider wrote:
> >>> This adds the disabled eMMC node.
> >>>
> >>> Signed-off-by: Andre Heider <a.heider@gmail.com>
> >>> ---
> >>>   arch/arm/dts/armada-3720-espressobin.dts | 63 +++++++++---------------
> >>>   1 file changed, 23 insertions(+), 40 deletions(-)
> >>>
> >>> diff --git a/arch/arm/dts/armada-3720-espressobin.dts
> >>> b/arch/arm/dts/armada-3720-espressobin.dts
> >>> index 4534f5ff29..a66ab814eb 100644
> >>> --- a/arch/arm/dts/armada-3720-espressobin.dts
> >>> +++ b/arch/arm/dts/armada-3720-espressobin.dts
> >>> @@ -1,3 +1,4 @@
> >>> +// SPDX-License-Identifier: GPL-2.0+
> >>>   /*
> >>>    * Device Tree file for Marvell Armada 3720 community board
> >>>    * (ESPRESSOBin)
> >>> @@ -5,53 +6,15 @@
> >>>    *
> >>>    * 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.
> >>>    */
> >>
> >> You are changing license of whole file. Have all contributors and
> >> copyright holders agreed with it?
> >
> > First I though that you have been syncing the file with the Linux kernel
> > version. But now I see that its sync'ed with downstream U-Boot most
> > likely. As for the license of the file: The Linux kernel version has
> > this SPDX tag:
> >
> > // SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> >
> > Changing this to this tag would haven been fine (AFAICT). But using a
> > diffent one from a downstream U-Boot repository is a bit troublesome
> > for my taste.
> >
> > Can't you sync with Linux dts/dtsi files instead at some point?
>
> at some point, yes. As far as I understand that's not yet possible due
> to the comphy driver. Quoting Marek's cover letter "[PATCH
> u-boot-marvell 00/11] Armada 37xx: port comphy to generic-phys (PLEASE
> TEST)" from 19th April:
>
>  > I have ported the COMPHY and UTMI PHY drivers from Linux to U-Boot.
>  > With this we are able to abandon the current comphy_a3700 driver, which
>  > is incompatible with Linux' device trees. So if we want to have DTS
>  > files for A3720 boards identical to Linux', we have to do this.
>
> So unfortunately we can't use Linux' dts files for a3700 yet. Marek said
> he plans to work on that set again in the near future, so hopefully that
> lands rather sooner than later ;)

The way U-Boot usually deals with this is syncs as much of the Linux
ones as possible and if there's bits not in the Linux one that's
needed to make/keep U-Boot working they add it in a XXX-u-boot.dtsi
file. There's a number of examples in the arch/arm/dts directory.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream
  2020-09-06 16:03       ` Peter Robinson
@ 2020-09-06 16:11         ` Pali Rohár
  0 siblings, 0 replies; 25+ messages in thread
From: Pali Rohár @ 2020-09-06 16:11 UTC (permalink / raw)
  To: u-boot

On Sunday 06 September 2020 17:03:29 Peter Robinson wrote:
> The way U-Boot usually deals with this is syncs as much of the Linux
> ones as possible and if there's bits not in the Linux one that's
> needed to make/keep U-Boot working they add it in a XXX-u-boot.dtsi
> file. There's a number of examples in the arch/arm/dts directory.

Hello Peter! The main issue is that U-Boot has different comphy driver
as Linux kernel and these two different implementations also needs
different DTS files. Marek is working on patches to replace U-Boot
comphy driver from Linux and then U-Boot will also use comphy DTS
definitions from Linux.

I think it does not make sense to spend time on thinking and
implementing how to semi-synchronize DTS files from kernel as after
comphy U-Boot driver changes, DTS files would be again changed.

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2020-09-06 16:11 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-31  3:34 [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream Andre Heider
2020-08-31  3:34 ` [PATCH 2/2] arm64: dts: armada-3720-espressobin-emmc: add emmc dts Andre Heider
2020-08-31  6:33   ` [PATCH v2 2/2] arm64: mvebu: a37xx: Add support for EspressoBIN with eMMC Andre Heider
2020-08-31  7:55     ` Pali Rohár
2020-08-31  8:21       ` Andre Heider
2020-08-31  8:27         ` Pali Rohár
2020-08-31  8:41           ` Andre Heider
2020-09-01  7:03   ` [PATCH v3 " Andre Heider
2020-09-01 20:02     ` Pali Rohár
2020-09-04  9:02     ` Stefan Roese
2020-08-31  7:53 ` [PATCH 1/2] arm64: dts: armada-3720-espressobin: sync with downstream Pali Rohár
2020-08-31  8:17   ` Andre Heider
2020-09-04 12:02   ` Stefan Roese
2020-09-04 12:35     ` Andre Heider
2020-09-04 12:40       ` Stefan Roese
2020-09-04 15:36         ` Andre Heider
2020-09-06  9:21           ` Pali Rohár
2020-09-06 16:03       ` Peter Robinson
2020-09-06 16:11         ` Pali Rohár
2020-08-31  8:01 ` Pali Rohár
2020-08-31  8:17   ` Andre Heider
2020-08-31  8:46     ` Pali Rohár
2020-08-31  9:59       ` Andre Heider
2020-09-01 20:00         ` Pali Rohár
2020-09-04  9:01 ` Stefan Roese

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.