[PATCH 1/2] arm: mvebu: Espressobin: Make SPI env offset compatible with Marvell's U-Boot

[PATCH 1/2] arm: mvebu: Espressobin: Make SPI env offset compatible with Marvell's U-Boot

[Pali Rohár]

Espressobin board comes with Marvell's U-Boot version where U-Boot env is
stored in SPI at offset 0x3F0000. This patch changes env offset in
Espressobin defconfig file to match Marvell's U-Boot version.

Users who want to use previous or different env offset can still change it
in .config file when compiling U-Boot.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 configs/mvebu_espressobin-88f3720_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
index 933592af88..afcdd947c1 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -6,7 +6,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_TARGET_MVEBU_ARMADA_37XX=y
 CONFIG_ENV_SIZE=0x10000
-CONFIG_ENV_OFFSET=0x180000
+CONFIG_ENV_OFFSET=0x3F0000
 CONFIG_ENV_SECT_SIZE=0x10000
 CONFIG_DM_GPIO=y
 CONFIG_DEBUG_UART_BASE=0xd0012000
-- 
2.20.1

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Pali Rohár]

Due to different partition layouts in different U-Boot versions, DTS for
Espressobin in Linux does not contain any definition of MTD partitions.
See commit https://git.kernel.org/stable/c/00954566464a4 for more details.

This patch via ft_board_setup() hook fills current partition layout used by
U-Boot, so booted kernel would see correct MTD partitions layout.

U-Boot env partition is calculated from CONFIG_ENV_OFFSET option.

First partition contains secure firmware, ARM trusted firmware and U-Boot
with checksums. So it is not possible to replace just one image (e.g.
U-Boot) without updating other parts where is stored checksum of U-Boot.
Therefore there is no extra partition defined for U-Boot and first
partition is called just 'firmware'.

Signed-off-by: Pali Roh?r <pali@kernel.org>
---
 board/Marvell/mvebu_armada-37xx/board.c     | 100 ++++++++++++++++++++
 configs/mvebu_espressobin-88f3720_defconfig |   1 +
 2 files changed, 101 insertions(+)

diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
index 031de318c6..7b9c3223ed 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -243,3 +243,103 @@ int board_network_enable(struct mii_dev *bus)
 
 	return 0;
 }
+
+#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_ENV_IS_IN_SPI_FLASH)
+int ft_board_setup(void *blob, struct bd_info *bd)
+{
+	int ret;
+	int spi_off;
+	int parts_off;
+	int part_off;
+
+	/* Fill SPI MTD partitions for Linux kernel on Espressobin */
+	if (!of_machine_is_compatible("marvell,armada-3720-espressobin"))
+		return 0;
+
+	spi_off = fdt_node_offset_by_compatible(blob, -1, "jedec,spi-nor");
+	if (spi_off < 0)
+		return 0;
+
+	/* Do not touch partitions if they are already defined */
+	if (fdt_subnode_offset(blob, spi_off, "partitions") >= 0)
+		return 0;
+
+	parts_off = fdt_add_subnode(blob, spi_off, "partitions");
+	if (parts_off < 0) {
+		printf("Can't add partitions node: %s\n", fdt_strerror(parts_off));
+		return 0;
+	}
+
+	ret = fdt_setprop_string(blob, parts_off, "compatible", "fixed-partitions");
+	if (ret < 0) {
+		printf("Can't set compatible property: %s\n", fdt_strerror(ret));
+		return 0;
+	}
+
+	ret = fdt_setprop_u32(blob, parts_off, "#address-cells", 1);
+	if (ret < 0) {
+		printf("Can't set #address-cells property: %s\n", fdt_strerror(ret));
+		return 0;
+	}
+
+	ret = fdt_setprop_u32(blob, parts_off, "#size-cells", 1);
+	if (ret < 0) {
+		printf("Can't set #size-cells property: %s\n", fdt_strerror(ret));
+		return 0;
+	}
+
+	/* Add u-boot-env partition */
+
+	part_off = fdt_add_subnode(blob, parts_off, "partition at u-boot-env");
+	if (part_off < 0) {
+		printf("Can't add partition at u-boot-env node: %s\n", fdt_strerror(part_off));
+		return 0;
+	}
+
+	ret = fdt_setprop_u32(blob, part_off, "reg", CONFIG_ENV_OFFSET);
+	if (ret < 0) {
+		printf("Can't set partition at u-boot-env reg property: %s\n", fdt_strerror(ret));
+		return 0;
+	}
+
+	ret = fdt_appendprop_u32(blob, part_off, "reg", CONFIG_ENV_SIZE);
+	if (ret < 0) {
+		printf("Can't set partition at u-boot-env reg property: %s\n", fdt_strerror(ret));
+		return 0;
+	}
+
+	ret = fdt_setprop_string(blob, part_off, "label", "u-boot-env");
+	if (ret < 0) {
+		printf("Can't set partition at u-boot-env label property: %s\n", fdt_strerror(ret));
+		return 0;
+	}
+
+	/* Add firmware partition */
+
+	part_off = fdt_add_subnode(blob, parts_off, "partition at firmware");
+	if (part_off < 0) {
+		printf("Can't add partition at firmware node: %s\n", fdt_strerror(part_off));
+		return 0;
+	}
+
+	ret = fdt_setprop_u32(blob, part_off, "reg", 0);
+	if (ret < 0) {
+		printf("Can't set partition at firmware reg property: %s\n", fdt_strerror(ret));
+		return 0;
+	}
+
+	ret = fdt_appendprop_u32(blob, part_off, "reg", CONFIG_ENV_OFFSET);
+	if (ret < 0) {
+		printf("Can't set partition at firmware reg property: %s\n", fdt_strerror(ret));
+		return 0;
+	}
+
+	ret = fdt_setprop_string(blob, part_off, "label", "firmware");
+	if (ret < 0) {
+		printf("Can't set partition@firmware label property: %s\n", fdt_strerror(ret));
+		return 0;
+	}
+
+	return 0;
+}
+#endif
diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
index afcdd947c1..0c1c92d4ff 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -82,3 +82,4 @@ CONFIG_SHA1=y
 CONFIG_SHA256=y
 CONFIG_MVNETA=y
 CONFIG_DM_REGULATOR_GPIO=y
+CONFIG_OF_BOARD_SETUP=y
-- 
2.20.1

[EXT] [PATCH 1/2] arm: mvebu: Espressobin: Make SPI env offset compatible with Marvell's U-Boot

[Kostya Porotchkin]

> -----Original Message-----
> From: Pali Roh?r <pali@kernel.org>
> Sent: Wednesday, August 19, 2020 17:24
> To: Stefan Roese <sr@denx.de>; Kostya Porotchkin <kostap@marvell.com>
> Cc: u-boot at lists.denx.de
> Subject: [EXT] [PATCH 1/2] arm: mvebu: Espressobin: Make SPI env offset
> compatible with Marvell's U-Boot
> 
> External Email
> 
> ----------------------------------------------------------------------
> Espressobin board comes with Marvell's U-Boot version where U-Boot env is
> stored in SPI at offset 0x3F0000. This patch changes env offset in Espressobin
> defconfig file to match Marvell's U-Boot version.
> 
> Users who want to use previous or different env offset can still change it in
> .config file when compiling U-Boot.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
Reviewed-by: Konstantin Porotchkin <kostap@marvell.com>

> ---
>  configs/mvebu_espressobin-88f3720_defconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig
> b/configs/mvebu_espressobin-88f3720_defconfig
> index 933592af88..afcdd947c1 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -6,7 +6,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
>  CONFIG_NR_DRAM_BANKS=1
>  CONFIG_TARGET_MVEBU_ARMADA_37XX=y
>  CONFIG_ENV_SIZE=0x10000
> -CONFIG_ENV_OFFSET=0x180000
> +CONFIG_ENV_OFFSET=0x3F0000
>  CONFIG_ENV_SECT_SIZE=0x10000
>  CONFIG_DM_GPIO=y
>  CONFIG_DEBUG_UART_BASE=0xd0012000
> --
> 2.20.1

[EXT] [PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Kostya Porotchkin]

> -----Original Message-----
> From: Pali Roh?r <pali@kernel.org>
> Sent: Wednesday, August 19, 2020 17:24
> To: Stefan Roese <sr@denx.de>; Kostya Porotchkin <kostap@marvell.com>
> Cc: u-boot at lists.denx.de
> Subject: [EXT] [PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions
> when booting kernel
> 
> External Email
> 
> ----------------------------------------------------------------------
> Due to different partition layouts in different U-Boot versions, DTS for
> Espressobin in Linux does not contain any definition of MTD partitions.
> See commit https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__git.kernel.org_stable_c_00954566464a4&d=DwIDaQ&c=nKjWec2b6R0m
> OyPaz7xtfQ&r=-
> N9sN4p5NSr0JGQoQ_2UCOgAqajG99W1EbSOww0WU8o&m=EH2hF8LOBbgZ
> NUxvBwhhmIe2uzWWEa_eJ3vAmWPxqmo&s=ei6YI15fI5FM73Z6537XKz_p9
> p3nx9nT7qiUvg2o_tU&e=  for more details.
> 
> This patch via ft_board_setup() hook fills current partition layout used by U-
> Boot, so booted kernel would see correct MTD partitions layout.
> 
> U-Boot env partition is calculated from CONFIG_ENV_OFFSET option.
> 
> First partition contains secure firmware, ARM trusted firmware and U-Boot
> with checksums. So it is not possible to replace just one image (e.g.
> U-Boot) without updating other parts where is stored checksum of U-Boot.
> Therefore there is no extra partition defined for U-Boot and first partition is
> called just 'firmware'.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
Reviewed-by: Konstantin Porotchkin <kostap@marvell.com>

> ---
>  board/Marvell/mvebu_armada-37xx/board.c     | 100
> ++++++++++++++++++++
>  configs/mvebu_espressobin-88f3720_defconfig |   1 +
>  2 files changed, 101 insertions(+)
> 
> diff --git a/board/Marvell/mvebu_armada-37xx/board.c
> b/board/Marvell/mvebu_armada-37xx/board.c
> index 031de318c6..7b9c3223ed 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -243,3 +243,103 @@ int board_network_enable(struct mii_dev *bus)
> 
>  	return 0;
>  }
> +
> +#if defined(CONFIG_OF_BOARD_SETUP) &&
> +defined(CONFIG_ENV_IS_IN_SPI_FLASH)
> +int ft_board_setup(void *blob, struct bd_info *bd) {
> +	int ret;
> +	int spi_off;
> +	int parts_off;
> +	int part_off;
> +
> +	/* Fill SPI MTD partitions for Linux kernel on Espressobin */
> +	if (!of_machine_is_compatible("marvell,armada-3720-espressobin"))
> +		return 0;
> +
> +	spi_off = fdt_node_offset_by_compatible(blob, -1, "jedec,spi-nor");
> +	if (spi_off < 0)
> +		return 0;
> +
> +	/* Do not touch partitions if they are already defined */
> +	if (fdt_subnode_offset(blob, spi_off, "partitions") >= 0)
> +		return 0;
> +
> +	parts_off = fdt_add_subnode(blob, spi_off, "partitions");
> +	if (parts_off < 0) {
> +		printf("Can't add partitions node: %s\n",
> fdt_strerror(parts_off));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_string(blob, parts_off, "compatible", "fixed-
> partitions");
> +	if (ret < 0) {
> +		printf("Can't set compatible property: %s\n",
> fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, parts_off, "#address-cells", 1);
> +	if (ret < 0) {
> +		printf("Can't set #address-cells property: %s\n",
> fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, parts_off, "#size-cells", 1);
> +	if (ret < 0) {
> +		printf("Can't set #size-cells property: %s\n",
> fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	/* Add u-boot-env partition */
> +
> +	part_off = fdt_add_subnode(blob, parts_off, "partition at u-boot-
> env");
> +	if (part_off < 0) {
> +		printf("Can't add partition at u-boot-env node: %s\n",
> fdt_strerror(part_off));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, part_off, "reg", CONFIG_ENV_OFFSET);
> +	if (ret < 0) {
> +		printf("Can't set partition at u-boot-env reg property: %s\n",
> fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_appendprop_u32(blob, part_off, "reg",
> CONFIG_ENV_SIZE);
> +	if (ret < 0) {
> +		printf("Can't set partition at u-boot-env reg property: %s\n",
> fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_string(blob, part_off, "label", "u-boot-env");
> +	if (ret < 0) {
> +		printf("Can't set partition at u-boot-env label property: %s\n",
> fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	/* Add firmware partition */
> +
> +	part_off = fdt_add_subnode(blob, parts_off, "partition at firmware");
> +	if (part_off < 0) {
> +		printf("Can't add partition at firmware node: %s\n",
> fdt_strerror(part_off));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, part_off, "reg", 0);
> +	if (ret < 0) {
> +		printf("Can't set partition at firmware reg property: %s\n",
> fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_appendprop_u32(blob, part_off, "reg",
> CONFIG_ENV_OFFSET);
> +	if (ret < 0) {
> +		printf("Can't set partition at firmware reg property: %s\n",
> fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_string(blob, part_off, "label", "firmware");
> +	if (ret < 0) {
> +		printf("Can't set partition at firmware label property: %s\n",
> fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	return 0;
> +}
> +#endif
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig
> b/configs/mvebu_espressobin-88f3720_defconfig
> index afcdd947c1..0c1c92d4ff 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -82,3 +82,4 @@ CONFIG_SHA1=y
>  CONFIG_SHA256=y
>  CONFIG_MVNETA=y
>  CONFIG_DM_REGULATOR_GPIO=y
> +CONFIG_OF_BOARD_SETUP=y
> --
> 2.20.1

[PATCH 1/2] arm: mvebu: Espressobin: Make SPI env offset compatible with Marvell's U-Boot

[Stefan Roese]

On 19.08.20 16:24, Pali Roh?r wrote:
> Espressobin board comes with Marvell's U-Boot version where U-Boot env is
> stored in SPI at offset 0x3F0000. This patch changes env offset in
> Espressobin defconfig file to match Marvell's U-Boot version.
> 
> Users who want to use previous or different env offset can still change it
> in .config file when compiling U-Boot.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

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

Thanks,
Stefan

> ---
>   configs/mvebu_espressobin-88f3720_defconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
> index 933592af88..afcdd947c1 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -6,7 +6,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
>   CONFIG_NR_DRAM_BANKS=1
>   CONFIG_TARGET_MVEBU_ARMADA_37XX=y
>   CONFIG_ENV_SIZE=0x10000
> -CONFIG_ENV_OFFSET=0x180000
> +CONFIG_ENV_OFFSET=0x3F0000
>   CONFIG_ENV_SECT_SIZE=0x10000
>   CONFIG_DM_GPIO=y
>   CONFIG_DEBUG_UART_BASE=0xd0012000
> 


Viele Gr??e,
Stefan

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

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Stefan Roese]

On 19.08.20 16:24, Pali Roh?r wrote:
> Due to different partition layouts in different U-Boot versions, DTS for
> Espressobin in Linux does not contain any definition of MTD partitions.
> See commit https://git.kernel.org/stable/c/00954566464a4 for more details.
> 
> This patch via ft_board_setup() hook fills current partition layout used by
> U-Boot, so booted kernel would see correct MTD partitions layout.
> 
> U-Boot env partition is calculated from CONFIG_ENV_OFFSET option.
> 
> First partition contains secure firmware, ARM trusted firmware and U-Boot
> with checksums. So it is not possible to replace just one image (e.g.
> U-Boot) without updating other parts where is stored checksum of U-Boot.
> Therefore there is no extra partition defined for U-Boot and first
> partition is called just 'firmware'.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>
> ---
>   board/Marvell/mvebu_armada-37xx/board.c     | 100 ++++++++++++++++++++
>   configs/mvebu_espressobin-88f3720_defconfig |   1 +
>   2 files changed, 101 insertions(+)
> 
> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
> index 031de318c6..7b9c3223ed 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -243,3 +243,103 @@ int board_network_enable(struct mii_dev *bus)
>   
>   	return 0;
>   }
> +
> +#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_ENV_IS_IN_SPI_FLASH)
> +int ft_board_setup(void *blob, struct bd_info *bd)
> +{
> +	int ret;
> +	int spi_off;
> +	int parts_off;
> +	int part_off;
> +
> +	/* Fill SPI MTD partitions for Linux kernel on Espressobin */
> +	if (!of_machine_is_compatible("marvell,armada-3720-espressobin"))
> +		return 0;
> +
> +	spi_off = fdt_node_offset_by_compatible(blob, -1, "jedec,spi-nor");
> +	if (spi_off < 0)
> +		return 0;
> +
> +	/* Do not touch partitions if they are already defined */
> +	if (fdt_subnode_offset(blob, spi_off, "partitions") >= 0)
> +		return 0;
> +
> +	parts_off = fdt_add_subnode(blob, spi_off, "partitions");
> +	if (parts_off < 0) {
> +		printf("Can't add partitions node: %s\n", fdt_strerror(parts_off));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_string(blob, parts_off, "compatible", "fixed-partitions");
> +	if (ret < 0) {
> +		printf("Can't set compatible property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, parts_off, "#address-cells", 1);
> +	if (ret < 0) {
> +		printf("Can't set #address-cells property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, parts_off, "#size-cells", 1);
> +	if (ret < 0) {
> +		printf("Can't set #size-cells property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	/* Add u-boot-env partition */
> +
> +	part_off = fdt_add_subnode(blob, parts_off, "partition at u-boot-env");
> +	if (part_off < 0) {
> +		printf("Can't add partition at u-boot-env node: %s\n", fdt_strerror(part_off));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, part_off, "reg", CONFIG_ENV_OFFSET);
> +	if (ret < 0) {
> +		printf("Can't set partition at u-boot-env reg property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_appendprop_u32(blob, part_off, "reg", CONFIG_ENV_SIZE);
> +	if (ret < 0) {
> +		printf("Can't set partition at u-boot-env reg property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_string(blob, part_off, "label", "u-boot-env");
> +	if (ret < 0) {
> +		printf("Can't set partition at u-boot-env label property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	/* Add firmware partition */
> +
> +	part_off = fdt_add_subnode(blob, parts_off, "partition at firmware");
> +	if (part_off < 0) {
> +		printf("Can't add partition at firmware node: %s\n", fdt_strerror(part_off));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, part_off, "reg", 0);
> +	if (ret < 0) {
> +		printf("Can't set partition at firmware reg property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_appendprop_u32(blob, part_off, "reg", CONFIG_ENV_OFFSET);
> +	if (ret < 0) {
> +		printf("Can't set partition at firmware reg property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_string(blob, part_off, "label", "firmware");
> +	if (ret < 0) {
> +		printf("Can't set partition at firmware label property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	return 0;
> +}
> +#endif

Can't you just use "mtdparts=" kernel cmdline parameter instead to pass
the MTD layout to the kernel?

Thanks,
Stefan

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Pali Rohár]

On Thursday 20 August 2020 07:02:18 Stefan Roese wrote:
> Can't you just use "mtdparts=" kernel cmdline parameter instead to pass
> the MTD layout to the kernel?

Maybe it is possible too, I have not tried it.

I thought that more common is to update DTS file by uboot when loading
kernel as it is already done e.g. for ethernet MAC address on
Espressobin.

Also I see that uboot has function fdt_fixup_mtdparts() via
CONFIG_FDT_FIXUP_PARTITIONS option which do this, but uses uboot MTD
code which IIRC cannot initialize SPI NOR.

Anyway, updating DTS has advantage that it is not needed to update
existing boot scripts for OS. There are more distributions for
Espressobin which have own boot scripts stored on SD card for loading
kernel. And therefore to use command line parameters it would be needed
to update all of them.

And I see there another problem. For specifying size of mtd partitions
in command line, it is required to know offsets of those partitions. And
e.g. uboot env partition depends on CONFIG_ENV_OFFSET option which is
not available for uboot boot script code.

But if you have other idea, I'm open to also other solutions.

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Stefan Roese]

On 20.08.20 09:40, Pali Roh?r wrote:
> On Thursday 20 August 2020 07:02:18 Stefan Roese wrote:
>> Can't you just use "mtdparts=" kernel cmdline parameter instead to pass
>> the MTD layout to the kernel?
> 
> Maybe it is possible too, I have not tried it.
> 
> I thought that more common is to update DTS file by uboot when loading
> kernel as it is already done e.g. for ethernet MAC address on
> Espressobin.

Updating the MAC address is very common, yes. But passing the
MTD partition layout via the mtdparts= cmdline is also very
common and used very frequently. From my experience, its the
defacto recommended method to pass this info and also easier than
generating these DTS lines via some code.

Please take a look at the mtdparts U-Boot command and its usage
in other targets.

> Also I see that uboot has function fdt_fixup_mtdparts() via
> CONFIG_FDT_FIXUP_PARTITIONS option which do this, but uses uboot MTD
> code which IIRC cannot initialize SPI NOR.

Why not? SPI NOR is also integrated into the U-Boot MTD world as
well. Please see this GARDENA MT7688 target, which uses SPI NOR &
SPI NAND:

=> mtd list
List of MTD devices:
* nor0
   - type: NOR flash
   - block size: 0x1000 bytes
   - min I/O: 0x1 bytes
   - 0x000000000000-0x000000800000 : "nor0"
           - 0x000000000000-0x0000000a0000 : "uboot"
           - 0x0000000a0000-0x0000000b0000 : "uboot_env0"
           - 0x0000000b0000-0x0000000c0000 : "uboot_env1"
           - 0x0000000c0000-0x0000000d0000 : "factory"
           - 0x0000000d0000-0x000000800000 : "unused"
* spi-nand0
   - device: spi-nand at 1
   - parent: spi at b00
   - driver: spi_nand
   - type: NAND flash
   - block size: 0x20000 bytes
   - min I/O: 0x800 bytes
   - OOB size: 128 bytes
   - OOB available: 63 bytes
   - 0x000000000000-0x000008000000 : "spi-nand0"
           - 0x000000000000-0x000008000000 : "nand"
=> mtdparts

device nor0 <spi0.0>, # parts = 5
  #: name                size            offset          mask_flags
  0: uboot               0x000a0000      0x00000000      0
  1: uboot_env0          0x00010000      0x000a0000      0
  2: uboot_env1          0x00010000      0x000b0000      0
  3: factory             0x00010000      0x000c0000      0
  4: unused              0x00730000      0x000d0000      0

device spi-nand0 <spi0.1>, # parts = 1
  #: name                size            offset          mask_flags
  0: nand                0x08000000      0x00000000      0

active partition: nor0,0 - (uboot) 0x000a0000 @ 0x00000000

defaults:
mtdids  : spi-nand0=spi0.1,nor0=spi0.0
mtdparts: 
spi0.0:640k(uboot),64k(uboot_env0),64k(uboot_env1),64k(factory),-(unused);spi0.1:-(nand)

> Anyway, updating DTS has advantage that it is not needed to update
> existing boot scripts for OS. There are more distributions for
> Espressobin which have own boot scripts stored on SD card for loading
> kernel. And therefore to use command line parameters it would be needed
> to update all of them.

I see. This is an argument I understand. But can't you use the common
fdt_fixup_mtdparts() then?

> And I see there another problem. For specifying size of mtd partitions
> in command line, it is required to know offsets of those partitions. And
> e.g. uboot env partition depends on CONFIG_ENV_OFFSET option which is
> not available for uboot boot script code.
> 
> But if you have other idea, I'm open to also other solutions.

I have not investigated a multi-distribution solution here. Perhaps
the common fdt_fixup_mtdparts() is able to handle this?

If not, then I won't stop this current patch. I just wanted to point
out the more common and more elegant solutions to you.

Thanks,
Stefan

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Pali Rohár]

On Thursday 20 August 2020 10:17:55 Stefan Roese wrote:
> On 20.08.20 09:40, Pali Roh?r wrote:
> > On Thursday 20 August 2020 07:02:18 Stefan Roese wrote:
> > > Can't you just use "mtdparts=" kernel cmdline parameter instead to pass
> > > the MTD layout to the kernel?
> > 
> > Maybe it is possible too, I have not tried it.
> > 
> > I thought that more common is to update DTS file by uboot when loading
> > kernel as it is already done e.g. for ethernet MAC address on
> > Espressobin.
> 
> Updating the MAC address is very common, yes. But passing the
> MTD partition layout via the mtdparts= cmdline is also very
> common and used very frequently. From my experience, its the
> defacto recommended method to pass this info and also easier than
> generating these DTS lines via some code.
> 
> Please take a look at the mtdparts U-Boot command and its usage
> in other targets.
> 
> > Also I see that uboot has function fdt_fixup_mtdparts() via
> > CONFIG_FDT_FIXUP_PARTITIONS option which do this, but uses uboot MTD
> > code which IIRC cannot initialize SPI NOR.
> 
> Why not?

I had an impression that U-Boot does not support.

> SPI NOR is also integrated into the U-Boot MTD world as
> well. Please see this GARDENA MT7688 target, which uses SPI NOR &
> SPI NAND:

Ok, I'm going to look at it.

> > Anyway, updating DTS has advantage that it is not needed to update
> > existing boot scripts for OS. There are more distributions for
> > Espressobin which have own boot scripts stored on SD card for loading
> > kernel. And therefore to use command line parameters it would be needed
> > to update all of them.
> 
> I see. This is an argument I understand. But can't you use the common
> fdt_fixup_mtdparts() then?
> 
> > And I see there another problem. For specifying size of mtd partitions
> > in command line, it is required to know offsets of those partitions. And
> > e.g. uboot env partition depends on CONFIG_ENV_OFFSET option which is
> > not available for uboot boot script code.
> > 
> > But if you have other idea, I'm open to also other solutions.
> 
> I have not investigated a multi-distribution solution here. Perhaps
> the common fdt_fixup_mtdparts() is able to handle this?

In case U-Boot would see MTD SPI partitions, then fdt_fixup_mtdparts()
should work.

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Pali Rohár]

On Thursday 20 August 2020 10:51:28 Pali Roh?r wrote:
> On Thursday 20 August 2020 10:17:55 Stefan Roese wrote:
> > On 20.08.20 09:40, Pali Roh?r wrote:
> > > On Thursday 20 August 2020 07:02:18 Stefan Roese wrote:
> > > > Can't you just use "mtdparts=" kernel cmdline parameter instead to pass
> > > > the MTD layout to the kernel?
> > > 
> > > Maybe it is possible too, I have not tried it.
> > > 
> > > I thought that more common is to update DTS file by uboot when loading
> > > kernel as it is already done e.g. for ethernet MAC address on
> > > Espressobin.
> > 
> > Updating the MAC address is very common, yes. But passing the
> > MTD partition layout via the mtdparts= cmdline is also very
> > common and used very frequently. From my experience, its the
> > defacto recommended method to pass this info and also easier than
> > generating these DTS lines via some code.
> > 
> > Please take a look at the mtdparts U-Boot command and its usage
> > in other targets.
> > 
> > > Also I see that uboot has function fdt_fixup_mtdparts() via
> > > CONFIG_FDT_FIXUP_PARTITIONS option which do this, but uses uboot MTD
> > > code which IIRC cannot initialize SPI NOR.
> > 
> > Why not?
> 
> I had an impression that U-Boot does not support.
> 
> > SPI NOR is also integrated into the U-Boot MTD world as
> > well. Please see this GARDENA MT7688 target, which uses SPI NOR &
> > SPI NAND:
> 
> Ok, I'm going to look at it.

With following config

CONFIG_MTD=y
CONFIG_CMD_MTD=y
CONFIG_CMD_MTDPARTS=y
CONFIG_SPI_FLASH_MTD=y
CONFIG_MTDIDS_DEFAULT="nor0=nor0"

I get following result:

=> mtd list
List of MTD devices:
No MTD device found
=> mtdparts 
Device nor0 not found!
=> sf probe
SF: Detected w25q32dw with page size 256 Bytes, erase size 4 KiB, total 4 MiB
=> mtdparts 
=> mtd list 
List of MTD devices:
* nor0
  - type: NOR flash
  - block size: 0x1000 bytes
  - min I/O: 0x1 bytes
  - 0x000000000000-0x000000400000 : "nor0"

So main problem is that MTD does not work until I call 'sf probe' in
uboot command line.

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Stefan Roese]

On 20.08.20 13:12, Pali Roh?r wrote:
> On Thursday 20 August 2020 10:51:28 Pali Roh?r wrote:
>> On Thursday 20 August 2020 10:17:55 Stefan Roese wrote:
>>> On 20.08.20 09:40, Pali Roh?r wrote:
>>>> On Thursday 20 August 2020 07:02:18 Stefan Roese wrote:
>>>>> Can't you just use "mtdparts=" kernel cmdline parameter instead to pass
>>>>> the MTD layout to the kernel?
>>>>
>>>> Maybe it is possible too, I have not tried it.
>>>>
>>>> I thought that more common is to update DTS file by uboot when loading
>>>> kernel as it is already done e.g. for ethernet MAC address on
>>>> Espressobin.
>>>
>>> Updating the MAC address is very common, yes. But passing the
>>> MTD partition layout via the mtdparts= cmdline is also very
>>> common and used very frequently. From my experience, its the
>>> defacto recommended method to pass this info and also easier than
>>> generating these DTS lines via some code.
>>>
>>> Please take a look at the mtdparts U-Boot command and its usage
>>> in other targets.
>>>
>>>> Also I see that uboot has function fdt_fixup_mtdparts() via
>>>> CONFIG_FDT_FIXUP_PARTITIONS option which do this, but uses uboot MTD
>>>> code which IIRC cannot initialize SPI NOR.
>>>
>>> Why not?
>>
>> I had an impression that U-Boot does not support.
>>
>>> SPI NOR is also integrated into the U-Boot MTD world as
>>> well. Please see this GARDENA MT7688 target, which uses SPI NOR &
>>> SPI NAND:
>>
>> Ok, I'm going to look at it.
> 
> With following config
> 
> CONFIG_MTD=y
> CONFIG_CMD_MTD=y
> CONFIG_CMD_MTDPARTS=y
> CONFIG_SPI_FLASH_MTD=y
> CONFIG_MTDIDS_DEFAULT="nor0=nor0"
> 
> I get following result:
> 
> => mtd list
> List of MTD devices:
> No MTD device found
> => mtdparts
> Device nor0 not found!
> => sf probe
> SF: Detected w25q32dw with page size 256 Bytes, erase size 4 KiB, total 4 MiB
> => mtdparts
> => mtd list
> List of MTD devices:
> * nor0
>    - type: NOR flash
>    - block size: 0x1000 bytes
>    - min I/O: 0x1 bytes
>    - 0x000000000000-0x000000400000 : "nor0"
> 
> So main problem is that MTD does not work until I call 'sf probe' in
> uboot command line.

Then a dependency seems to be missing here. On my MT7688 board I get
this:

U-Boot 2020.07-rc5-00075-g0b7d95531c-dirty (Jun 30 2020 - 09:51:39 +0200)

CPU:   MediaTek MT7688A ver:1 eco:2
Boot:  DDR2, SPI-NOR 3-Byte Addr, CPU clock from XTAL
Clock: CPU: 580MHz, Bus: 193MHz, XTAL: 40MHz
Model: GARDENA smart Gateway (MT7688)
DRAM:  128 MiB
WDT:   Started with servicing (60s timeout)
Loading Environment from SPI Flash... SF: Detected XM25QH64A with page 
size 256 Bytes, erase size 4 KiB, total 8 MiB
OK
F-Data:factory-data version 1 detected
Net:   eth0: eth at 10110000
Hit any key to stop autoboot:  0
=> dm tree
  Class     Index  Probed  Driver                Name
-----------------------------------------------------------
  root          0  [ + ]   root_driver           root_driver
...
  spi           0  [ + ]   mt7621_spi            |   |-- spi at b00
  spi_flash     0  [ + ]   spi_flash_std         |   |   |-- spi-flash at 0
  mtd           0  [   ]   spi_nand              |   |   `-- spi-nand at 1
...
=> mtd list
List of MTD devices:
* nor0
   - type: NOR flash
   - block size: 0x1000 bytes
   - min I/O: 0x1 bytes
   - 0x000000000000-0x000000800000 : "nor0"
           - 0x000000000000-0x0000000a0000 : "uboot"
           - 0x0000000a0000-0x0000000b0000 : "uboot_env0"
           - 0x0000000b0000-0x0000000c0000 : "uboot_env1"
           - 0x0000000c0000-0x0000000d0000 : "factory"
           - 0x0000000d0000-0x000000800000 : "unused"
* spi-nand0
   - device: spi-nand at 1
   - parent: spi at b00
   - driver: spi_nand
   - type: NAND flash
   - block size: 0x20000 bytes
   - min I/O: 0x800 bytes
   - OOB size: 128 bytes
   - OOB available: 63 bytes
   - 0x000000000000-0x000008000000 : "spi-nand0"
           - 0x000000000000-0x000008000000 : "nand"
=> dm tree
  Class     Index  Probed  Driver                Name
-----------------------------------------------------------
  root          0  [ + ]   root_driver           root_driver
...
  spi           0  [ + ]   mt7621_spi            |   |-- spi at b00
  spi_flash     0  [ + ]   spi_flash_std         |   |   |-- spi-flash at 0
  mtd           0  [ + ]   spi_nand              |   |   `-- spi-nand at 1
...

As you see, spi_nand is automatically probed by "mtd list". SPI NOR
is most likely already probed since its used for ENV storage.

It would be great, if you could check, if and where such a dependency
is missing in your case.

Thanks,
Stefan

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Pali Rohár]

On Thursday 20 August 2020 13:19:34 Stefan Roese wrote:
> On 20.08.20 13:12, Pali Roh?r wrote:
> > On Thursday 20 August 2020 10:51:28 Pali Roh?r wrote:
> > > On Thursday 20 August 2020 10:17:55 Stefan Roese wrote:
> > > > On 20.08.20 09:40, Pali Roh?r wrote:
> > > > > On Thursday 20 August 2020 07:02:18 Stefan Roese wrote:
> > > > > > Can't you just use "mtdparts=" kernel cmdline parameter instead to pass
> > > > > > the MTD layout to the kernel?
> > > > > 
> > > > > Maybe it is possible too, I have not tried it.
> > > > > 
> > > > > I thought that more common is to update DTS file by uboot when loading
> > > > > kernel as it is already done e.g. for ethernet MAC address on
> > > > > Espressobin.
> > > > 
> > > > Updating the MAC address is very common, yes. But passing the
> > > > MTD partition layout via the mtdparts= cmdline is also very
> > > > common and used very frequently. From my experience, its the
> > > > defacto recommended method to pass this info and also easier than
> > > > generating these DTS lines via some code.
> > > > 
> > > > Please take a look at the mtdparts U-Boot command and its usage
> > > > in other targets.
> > > > 
> > > > > Also I see that uboot has function fdt_fixup_mtdparts() via
> > > > > CONFIG_FDT_FIXUP_PARTITIONS option which do this, but uses uboot MTD
> > > > > code which IIRC cannot initialize SPI NOR.
> > > > 
> > > > Why not?
> > > 
> > > I had an impression that U-Boot does not support.
> > > 
> > > > SPI NOR is also integrated into the U-Boot MTD world as
> > > > well. Please see this GARDENA MT7688 target, which uses SPI NOR &
> > > > SPI NAND:
> > > 
> > > Ok, I'm going to look at it.
> > 
> > With following config
> > 
> > CONFIG_MTD=y
> > CONFIG_CMD_MTD=y
> > CONFIG_CMD_MTDPARTS=y
> > CONFIG_SPI_FLASH_MTD=y
> > CONFIG_MTDIDS_DEFAULT="nor0=nor0"
> > 
> > I get following result:
> > 
> > => mtd list
> > List of MTD devices:
> > No MTD device found
> > => mtdparts
> > Device nor0 not found!
> > => sf probe
> > SF: Detected w25q32dw with page size 256 Bytes, erase size 4 KiB, total 4 MiB
> > => mtdparts
> > => mtd list
> > List of MTD devices:
> > * nor0
> >    - type: NOR flash
> >    - block size: 0x1000 bytes
> >    - min I/O: 0x1 bytes
> >    - 0x000000000000-0x000000400000 : "nor0"
> > 
> > So main problem is that MTD does not work until I call 'sf probe' in
> > uboot command line.
> 
> Then a dependency seems to be missing here. On my MT7688 board I get
> this:
> 
> U-Boot 2020.07-rc5-00075-g0b7d95531c-dirty (Jun 30 2020 - 09:51:39 +0200)
> 
> CPU:   MediaTek MT7688A ver:1 eco:2
> Boot:  DDR2, SPI-NOR 3-Byte Addr, CPU clock from XTAL
> Clock: CPU: 580MHz, Bus: 193MHz, XTAL: 40MHz
> Model: GARDENA smart Gateway (MT7688)
> DRAM:  128 MiB
> WDT:   Started with servicing (60s timeout)
> Loading Environment from SPI Flash... SF: Detected XM25QH64A with page size
> 256 Bytes, erase size 4 KiB, total 8 MiB
> OK
> F-Data:factory-data version 1 detected
> Net:   eth0: eth at 10110000
> Hit any key to stop autoboot:  0
> => dm tree
>  Class     Index  Probed  Driver                Name
> -----------------------------------------------------------
>  root          0  [ + ]   root_driver           root_driver
> ...
>  spi           0  [ + ]   mt7621_spi            |   |-- spi at b00
>  spi_flash     0  [ + ]   spi_flash_std         |   |   |-- spi-flash at 0
>  mtd           0  [   ]   spi_nand              |   |   `-- spi-nand at 1
> ...
> => mtd list
> List of MTD devices:
> * nor0
>   - type: NOR flash
>   - block size: 0x1000 bytes
>   - min I/O: 0x1 bytes
>   - 0x000000000000-0x000000800000 : "nor0"
>           - 0x000000000000-0x0000000a0000 : "uboot"
>           - 0x0000000a0000-0x0000000b0000 : "uboot_env0"
>           - 0x0000000b0000-0x0000000c0000 : "uboot_env1"
>           - 0x0000000c0000-0x0000000d0000 : "factory"
>           - 0x0000000d0000-0x000000800000 : "unused"
> * spi-nand0
>   - device: spi-nand at 1
>   - parent: spi at b00
>   - driver: spi_nand
>   - type: NAND flash
>   - block size: 0x20000 bytes
>   - min I/O: 0x800 bytes
>   - OOB size: 128 bytes
>   - OOB available: 63 bytes
>   - 0x000000000000-0x000008000000 : "spi-nand0"
>           - 0x000000000000-0x000008000000 : "nand"
> => dm tree
>  Class     Index  Probed  Driver                Name
> -----------------------------------------------------------
>  root          0  [ + ]   root_driver           root_driver
> ...
>  spi           0  [ + ]   mt7621_spi            |   |-- spi at b00
>  spi_flash     0  [ + ]   spi_flash_std         |   |   |-- spi-flash at 0
>  mtd           0  [ + ]   spi_nand              |   |   `-- spi-nand at 1
> ...
> 
> As you see, spi_nand is automatically probed by "mtd list". SPI NOR
> is most likely already probed since its used for ENV storage.
> 
> It would be great, if you could check, if and where such a dependency
> is missing in your case.
> 
> Thanks,
> Stefan

Full bootlog is:

U-Boot 2020.10-rc2-00156-gd21f8d2fdf-dirty (Aug 20 2020 - 12:55:39 +0200)

DRAM:  512 MiB
Comphy-0: USB3_HOST0    5 Gbps    
Comphy-1: PEX0          2.5 Gbps  
Comphy-2: SATA0         5 Gbps    
SATA link 0 timeout.
AHCI 0001.0300 32 slots 1 ports 6 Gbps 0x1 impl SATA mode
flags: ncq led only pmp fbss pio slum part sxs 
PCIE-0: Link up
MMC:   sdhci at d0000: 0
Loading Environment from SPIFlash... SF: Detected w25q32dw with page size 256 Bytes, erase size 4 KiB, total 4 MiB
OK
Model: Marvell Armada 3720 Community Board ESPRESSOBin
Net:   eth0: neta at 30000 [PRIME]
Hit any key to stop autoboot:  0

Then I called mtd commands which I wrote in previous email.
Just to note that ENV variables are correctly read from SPI.

In 'dm tree' is:
 spi           0  [ + ]   mvebu_spi             |   |   |-- spi at 10600
 spi_flash     0  [ + ]   jedec_spi_nor         |   |   |   `-- spi-flash at 0

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Pali Rohár]

On Thursday 20 August 2020 10:51:28 Pali Roh?r wrote:
> On Thursday 20 August 2020 10:17:55 Stefan Roese wrote:
> > On 20.08.20 09:40, Pali Roh?r wrote:
> > > Anyway, updating DTS has advantage that it is not needed to update
> > > existing boot scripts for OS. There are more distributions for
> > > Espressobin which have own boot scripts stored on SD card for loading
> > > kernel. And therefore to use command line parameters it would be needed
> > > to update all of them.
> > 
> > I see. This is an argument I understand. But can't you use the common
> > fdt_fixup_mtdparts() then?
> > 
> > > And I see there another problem. For specifying size of mtd partitions
> > > in command line, it is required to know offsets of those partitions. And
> > > e.g. uboot env partition depends on CONFIG_ENV_OFFSET option which is
> > > not available for uboot boot script code.
> > > 
> > > But if you have other idea, I'm open to also other solutions.
> > 
> > I have not investigated a multi-distribution solution here. Perhaps
> > the common fdt_fixup_mtdparts() is able to handle this?
> 
> In case U-Boot would see MTD SPI partitions, then fdt_fixup_mtdparts()
> should work.

It looks like that fdt_fixup_mtdparts() is broken.

I added following config lines

CONFIG_MTDIDS_DEFAULT="nor0=nor0"
CONFIG_MTDPARTS_DEFAULT="nor0:4032k(firmware),64k(u-boot-env)"
CONFIG_FDT_FIXUP_PARTITIONS=y

Then in uboot console I called 'sf probe' and 'boot'.

And Linux kernel thrown following MTD error:

[    1.038352] spi-nor spi0.0: w25q32dw (4096 Kbytes)
[    1.043403] spi0.0: error parsing ofpart partition /soc/internal-regs at d0000000/spi at 10600/flash at 0/partition at 0 (/soc/internal-regs at d0000000/spi at 10600/flash at 0)

So I do not know if it makes sense to continue debugging SF <--> MTD
layer in Uboot when passing uboot MTD partitions via
fdt_fixup_mtdparts() to Linux kernel does not work...

Original patch which in this thread is working fine and kernel correctly
see both defined partitions in ft_board_setup() function.

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Stefan Roese]

On 20.08.20 14:03, Pali Roh?r wrote:
> On Thursday 20 August 2020 10:51:28 Pali Roh?r wrote:
>> On Thursday 20 August 2020 10:17:55 Stefan Roese wrote:
>>> On 20.08.20 09:40, Pali Roh?r wrote:
>>>> Anyway, updating DTS has advantage that it is not needed to update
>>>> existing boot scripts for OS. There are more distributions for
>>>> Espressobin which have own boot scripts stored on SD card for loading
>>>> kernel. And therefore to use command line parameters it would be needed
>>>> to update all of them.
>>>
>>> I see. This is an argument I understand. But can't you use the common
>>> fdt_fixup_mtdparts() then?
>>>
>>>> And I see there another problem. For specifying size of mtd partitions
>>>> in command line, it is required to know offsets of those partitions. And
>>>> e.g. uboot env partition depends on CONFIG_ENV_OFFSET option which is
>>>> not available for uboot boot script code.
>>>>
>>>> But if you have other idea, I'm open to also other solutions.
>>>
>>> I have not investigated a multi-distribution solution here. Perhaps
>>> the common fdt_fixup_mtdparts() is able to handle this?
>>
>> In case U-Boot would see MTD SPI partitions, then fdt_fixup_mtdparts()
>> should work.
> 
> It looks like that fdt_fixup_mtdparts() is broken.
> 
> I added following config lines
> 
> CONFIG_MTDIDS_DEFAULT="nor0=nor0"
> CONFIG_MTDPARTS_DEFAULT="nor0:4032k(firmware),64k(u-boot-env)"
> CONFIG_FDT_FIXUP_PARTITIONS=y
> 
> Then in uboot console I called 'sf probe' and 'boot'.
> 
> And Linux kernel thrown following MTD error:
> 
> [    1.038352] spi-nor spi0.0: w25q32dw (4096 Kbytes)
> [    1.043403] spi0.0: error parsing ofpart partition /soc/internal-regs at d0000000/spi at 10600/flash at 0/partition at 0 (/soc/internal-regs at d0000000/spi at 10600/flash at 0)

I know that passing mtdparts= is working in general. You need the
correct device names though. Perhaps something is wrong here. Hard
to say, without looking deeper. Could you please post the complete
Linux bootlog of this failing boot attempt?

> So I do not know if it makes sense to continue debugging SF <--> MTD
> layer in Uboot when passing uboot MTD partitions via
> fdt_fixup_mtdparts() to Linux kernel does not work...

I understand. You have a working version and it makes not much sense
for you to work on an alternative solution. But the fdt_fixup_mtdparts()
way provides a common way for all (most?) boards and your's needs
to be rewritten for each board. So from this prospective, it definitely
makes sense to continue debugging / testing.

> Original patch which in this thread is working fine and kernel correctly
> see both defined partitions in ft_board_setup() function.

I understand. Thanks for at looking into this anyways.

Thanks,
Stefan

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Pali Rohár]

On Thursday 20 August 2020 14:09:50 Stefan Roese wrote:
> On 20.08.20 14:03, Pali Roh?r wrote:
> > On Thursday 20 August 2020 10:51:28 Pali Roh?r wrote:
> > > On Thursday 20 August 2020 10:17:55 Stefan Roese wrote:
> > > > On 20.08.20 09:40, Pali Roh?r wrote:
> > > > > Anyway, updating DTS has advantage that it is not needed to update
> > > > > existing boot scripts for OS. There are more distributions for
> > > > > Espressobin which have own boot scripts stored on SD card for loading
> > > > > kernel. And therefore to use command line parameters it would be needed
> > > > > to update all of them.
> > > > 
> > > > I see. This is an argument I understand. But can't you use the common
> > > > fdt_fixup_mtdparts() then?
> > > > 
> > > > > And I see there another problem. For specifying size of mtd partitions
> > > > > in command line, it is required to know offsets of those partitions. And
> > > > > e.g. uboot env partition depends on CONFIG_ENV_OFFSET option which is
> > > > > not available for uboot boot script code.
> > > > > 
> > > > > But if you have other idea, I'm open to also other solutions.
> > > > 
> > > > I have not investigated a multi-distribution solution here. Perhaps
> > > > the common fdt_fixup_mtdparts() is able to handle this?
> > > 
> > > In case U-Boot would see MTD SPI partitions, then fdt_fixup_mtdparts()
> > > should work.
> > 
> > It looks like that fdt_fixup_mtdparts() is broken.
> > 
> > I added following config lines
> > 
> > CONFIG_MTDIDS_DEFAULT="nor0=nor0"
> > CONFIG_MTDPARTS_DEFAULT="nor0:4032k(firmware),64k(u-boot-env)"
> > CONFIG_FDT_FIXUP_PARTITIONS=y
> > 
> > Then in uboot console I called 'sf probe' and 'boot'.
> > 
> > And Linux kernel thrown following MTD error:
> > 
> > [    1.038352] spi-nor spi0.0: w25q32dw (4096 Kbytes)
> > [    1.043403] spi0.0: error parsing ofpart partition /soc/internal-regs at d0000000/spi at 10600/flash at 0/partition at 0 (/soc/internal-regs at d0000000/spi at 10600/flash at 0)
> 
> I know that passing mtdparts= is working in general. You need the
> correct device names though. Perhaps something is wrong here. Hard
> to say, without looking deeper. Could you please post the complete
> Linux bootlog of this failing boot attempt?

I sent it to you in private email as log is large.

> > So I do not know if it makes sense to continue debugging SF <--> MTD
> > layer in Uboot when passing uboot MTD partitions via
> > fdt_fixup_mtdparts() to Linux kernel does not work...
> 
> I understand. You have a working version and it makes not much sense
> for you to work on an alternative solution. But the fdt_fixup_mtdparts()
> way provides a common way for all (most?) boards and your's needs
> to be rewritten for each board. So from this prospective, it definitely
> makes sense to continue debugging / testing.

Well, currently I do not have time to debug other components to figure
out what is broken in uboot.

For me it looks like a overkill to enable & configure MTD layer, then
attach SF layer to it and all this would be needed only for usage of
fdt_fixup_mtdparts() function. And once these layers would be fixed and
fully working it would same thing as in this my patch.

> > Original patch which in this thread is working fine and kernel correctly
> > see both defined partitions in ft_board_setup() function.
> 
> I understand. Thanks for at looking into this anyways.
> 
> Thanks,
> Stefan

[PATCH 1/2] arm: mvebu: Espressobin: Make SPI env offset compatible with Marvell's U-Boot

[Andre Heider]

On 19/08/2020 16:24, Pali Roh?r wrote:
> Espressobin board comes with Marvell's U-Boot version where U-Boot env is
> stored in SPI at offset 0x3F0000. This patch changes env offset in
> Espressobin defconfig file to match Marvell's U-Boot version.
> 
> Users who want to use previous or different env offset can still change it
> in .config file when compiling U-Boot.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Tested-by: Andre Heider <a.heider@gmail.com>

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Andre Heider]

On 19/08/2020 16:24, Pali Roh?r wrote:
> Due to different partition layouts in different U-Boot versions, DTS for
> Espressobin in Linux does not contain any definition of MTD partitions.
> See commit https://git.kernel.org/stable/c/00954566464a4 for more details.
> 
> This patch via ft_board_setup() hook fills current partition layout used by
> U-Boot, so booted kernel would see correct MTD partitions layout.
> 
> U-Boot env partition is calculated from CONFIG_ENV_OFFSET option.
> 
> First partition contains secure firmware, ARM trusted firmware and U-Boot
> with checksums. So it is not possible to replace just one image (e.g.
> U-Boot) without updating other parts where is stored checksum of U-Boot.
> Therefore there is no extra partition defined for U-Boot and first
> partition is called just 'firmware'.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Tested-by: Andre Heider <a.heider@gmail.com>

[PATCH 1/2] arm: mvebu: Espressobin: Make SPI env offset compatible with Marvell's U-Boot

[Stefan Roese]

On 19.08.20 16:24, Pali Roh?r wrote:
> Espressobin board comes with Marvell's U-Boot version where U-Boot env is
> stored in SPI at offset 0x3F0000. This patch changes env offset in
> Espressobin defconfig file to match Marvell's U-Boot version.
> 
> Users who want to use previous or different env offset can still change it
> in .config file when compiling U-Boot.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   configs/mvebu_espressobin-88f3720_defconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
> index 933592af88..afcdd947c1 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -6,7 +6,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
>   CONFIG_NR_DRAM_BANKS=1
>   CONFIG_TARGET_MVEBU_ARMADA_37XX=y
>   CONFIG_ENV_SIZE=0x10000
> -CONFIG_ENV_OFFSET=0x180000
> +CONFIG_ENV_OFFSET=0x3F0000
>   CONFIG_ENV_SECT_SIZE=0x10000
>   CONFIG_DM_GPIO=y
>   CONFIG_DEBUG_UART_BASE=0xd0012000
> 


Viele Gr??e,
Stefan

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

[PATCH 2/2] arm: mvebu: Espressobin: Setup MTD partitions when booting kernel

[Stefan Roese]

On 19.08.20 16:24, Pali Roh?r wrote:
> Due to different partition layouts in different U-Boot versions, DTS for
> Espressobin in Linux does not contain any definition of MTD partitions.
> See commit https://git.kernel.org/stable/c/00954566464a4 for more details.
> 
> This patch via ft_board_setup() hook fills current partition layout used by
> U-Boot, so booted kernel would see correct MTD partitions layout.
> 
> U-Boot env partition is calculated from CONFIG_ENV_OFFSET option.
> 
> First partition contains secure firmware, ARM trusted firmware and U-Boot
> with checksums. So it is not possible to replace just one image (e.g.
> U-Boot) without updating other parts where is stored checksum of U-Boot.
> Therefore there is no extra partition defined for U-Boot and first
> partition is called just 'firmware'.
> 
> Signed-off-by: Pali Roh?r <pali@kernel.org>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   board/Marvell/mvebu_armada-37xx/board.c     | 100 ++++++++++++++++++++
>   configs/mvebu_espressobin-88f3720_defconfig |   1 +
>   2 files changed, 101 insertions(+)
> 
> diff --git a/board/Marvell/mvebu_armada-37xx/board.c b/board/Marvell/mvebu_armada-37xx/board.c
> index 031de318c6..7b9c3223ed 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -243,3 +243,103 @@ int board_network_enable(struct mii_dev *bus)
>   
>   	return 0;
>   }
> +
> +#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_ENV_IS_IN_SPI_FLASH)
> +int ft_board_setup(void *blob, struct bd_info *bd)
> +{
> +	int ret;
> +	int spi_off;
> +	int parts_off;
> +	int part_off;
> +
> +	/* Fill SPI MTD partitions for Linux kernel on Espressobin */
> +	if (!of_machine_is_compatible("marvell,armada-3720-espressobin"))
> +		return 0;
> +
> +	spi_off = fdt_node_offset_by_compatible(blob, -1, "jedec,spi-nor");
> +	if (spi_off < 0)
> +		return 0;
> +
> +	/* Do not touch partitions if they are already defined */
> +	if (fdt_subnode_offset(blob, spi_off, "partitions") >= 0)
> +		return 0;
> +
> +	parts_off = fdt_add_subnode(blob, spi_off, "partitions");
> +	if (parts_off < 0) {
> +		printf("Can't add partitions node: %s\n", fdt_strerror(parts_off));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_string(blob, parts_off, "compatible", "fixed-partitions");
> +	if (ret < 0) {
> +		printf("Can't set compatible property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, parts_off, "#address-cells", 1);
> +	if (ret < 0) {
> +		printf("Can't set #address-cells property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, parts_off, "#size-cells", 1);
> +	if (ret < 0) {
> +		printf("Can't set #size-cells property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	/* Add u-boot-env partition */
> +
> +	part_off = fdt_add_subnode(blob, parts_off, "partition at u-boot-env");
> +	if (part_off < 0) {
> +		printf("Can't add partition at u-boot-env node: %s\n", fdt_strerror(part_off));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, part_off, "reg", CONFIG_ENV_OFFSET);
> +	if (ret < 0) {
> +		printf("Can't set partition at u-boot-env reg property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_appendprop_u32(blob, part_off, "reg", CONFIG_ENV_SIZE);
> +	if (ret < 0) {
> +		printf("Can't set partition at u-boot-env reg property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_string(blob, part_off, "label", "u-boot-env");
> +	if (ret < 0) {
> +		printf("Can't set partition at u-boot-env label property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	/* Add firmware partition */
> +
> +	part_off = fdt_add_subnode(blob, parts_off, "partition at firmware");
> +	if (part_off < 0) {
> +		printf("Can't add partition at firmware node: %s\n", fdt_strerror(part_off));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_u32(blob, part_off, "reg", 0);
> +	if (ret < 0) {
> +		printf("Can't set partition at firmware reg property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_appendprop_u32(blob, part_off, "reg", CONFIG_ENV_OFFSET);
> +	if (ret < 0) {
> +		printf("Can't set partition at firmware reg property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	ret = fdt_setprop_string(blob, part_off, "label", "firmware");
> +	if (ret < 0) {
> +		printf("Can't set partition at firmware label property: %s\n", fdt_strerror(ret));
> +		return 0;
> +	}
> +
> +	return 0;
> +}
> +#endif
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
> index afcdd947c1..0c1c92d4ff 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -82,3 +82,4 @@ CONFIG_SHA1=y
>   CONFIG_SHA256=y
>   CONFIG_MVNETA=y
>   CONFIG_DM_REGULATOR_GPIO=y
> +CONFIG_OF_BOARD_SETUP=y
> 


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