All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] arm: mvebu: clearfog: defconfig and eMMC updates
@ 2023-03-05 11:30 Martin Rowe
  2023-03-05 11:30 ` [PATCH 1/4] arm: mvebu: clearfog: Fix MMC detection Martin Rowe
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Martin Rowe @ 2023-03-05 11:30 UTC (permalink / raw)
  To: pali; +Cc: josua, jon, mibodhi, sr, u-boot, Martin Rowe

Minor defconfig and eMMC updates for SolidRun's A388 Clearfog devices.

Changes since initial discussion:
https://lists.denx.de/pipermail/u-boot/2023-February/510492.html
 - CONFIG_SPL_SPI selected for SPI defconfig
 - Runtime patching of kernel FDT added for eMMC detection

Note that this patch depends on this patch series (has been merged to
u-boot-marvell/next):
https://lists.denx.de/pipermail/u-boot/2023-March/511038.html

Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>

Martin Rowe (4):
  arm: mvebu: clearfog: Fix MMC detection
  arm: mvebu: clearfog: Align defconfigs
  arm: mvebu: clearfog: Add defconfig for SPI booting
  arm: mvebu: clearfog: Detect MMC vs SDHC and fixup fdt

 board/solidrun/clearfog/clearfog.c            | 42 +++++++++++++++++++
 configs/clearfog_defconfig                    |  5 ++-
 configs/clearfog_sata_defconfig               | 12 ++++--
 ...arfog_defconfig => clearfog_spi_defconfig} | 10 ++++-
 4 files changed, 61 insertions(+), 8 deletions(-)
 copy configs/{clearfog_defconfig => clearfog_spi_defconfig} (91%)

-- 
2.39.2


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

* [PATCH 1/4] arm: mvebu: clearfog: Fix MMC detection
  2023-03-05 11:30 [PATCH 0/4] arm: mvebu: clearfog: defconfig and eMMC updates Martin Rowe
@ 2023-03-05 11:30 ` Martin Rowe
  2023-03-05 12:19   ` Pali Rohár
  2023-03-05 11:30 ` [PATCH 2/4] arm: mvebu: clearfog: Align defconfigs Martin Rowe
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Martin Rowe @ 2023-03-05 11:30 UTC (permalink / raw)
  To: pali; +Cc: josua, jon, mibodhi, sr, u-boot, Martin Rowe

A388 Clearfog MMC is either SD Card or eMMC with different behaviour for
both. Setting MMC_BROKEN_CD allows both to correctly detect MMC.

Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
---
 configs/clearfog_defconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
index 8cd35f9f1a..24e7c16ac7 100644
--- a/configs/clearfog_defconfig
+++ b/configs/clearfog_defconfig
@@ -46,7 +46,6 @@ CONFIG_CMD_USB=y
 CONFIG_CMD_TFTPPUT=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
-CONFIG_CMD_MVEBU_BUBT=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_MIN_ENTRIES=128
 CONFIG_ARP_TIMEOUT=200
@@ -59,7 +58,7 @@ CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_MVTWSI=y
 CONFIG_I2C_EEPROM=y
 CONFIG_SPL_I2C_EEPROM=y
-CONFIG_SUPPORT_EMMC_BOOT=y
+CONFIG_MMC_BROKEN_CD=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_MV=y
-- 
2.39.2


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

* [PATCH 2/4] arm: mvebu: clearfog: Align defconfigs
  2023-03-05 11:30 [PATCH 0/4] arm: mvebu: clearfog: defconfig and eMMC updates Martin Rowe
  2023-03-05 11:30 ` [PATCH 1/4] arm: mvebu: clearfog: Fix MMC detection Martin Rowe
@ 2023-03-05 11:30 ` Martin Rowe
  2023-03-05 12:27   ` Pali Rohár
  2023-03-05 11:30 ` [PATCH 3/4] arm: mvebu: clearfog: Add defconfig for SPI booting Martin Rowe
  2023-03-05 11:31 ` [PATCH 4/4] arm: mvebu: clearfog: Detect MMC vs SDHC and fixup fdt Martin Rowe
  3 siblings, 1 reply; 13+ messages in thread
From: Martin Rowe @ 2023-03-05 11:30 UTC (permalink / raw)
  To: pali; +Cc: josua, jon, mibodhi, sr, u-boot, Martin Rowe

Ensure that functionality of generated configs is identical between SATA
and MMC defconfigs for Clearfog boards. The only expected difference is
the boot mode and environment location.

Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
---
 configs/clearfog_defconfig      |  1 +
 configs/clearfog_sata_defconfig | 11 +++++++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
index 24e7c16ac7..f9cfa94e77 100644
--- a/configs/clearfog_defconfig
+++ b/configs/clearfog_defconfig
@@ -46,6 +46,7 @@ CONFIG_CMD_USB=y
 CONFIG_CMD_TFTPPUT=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
+CONFIG_MVEBU_SATA_BOOT=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_MIN_ENTRIES=128
 CONFIG_ARP_TIMEOUT=200
diff --git a/configs/clearfog_sata_defconfig b/configs/clearfog_sata_defconfig
index e9b36150ea..b11d8746b0 100644
--- a/configs/clearfog_sata_defconfig
+++ b/configs/clearfog_sata_defconfig
@@ -3,14 +3,19 @@ CONFIG_ARCH_CPU_INIT=y
 CONFIG_SYS_THUMB_BUILD=y
 CONFIG_ARCH_MVEBU=y
 CONFIG_TEXT_BASE=0x00800000
+CONFIG_SPL_GPIO=y
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff0000
 CONFIG_TARGET_CLEARFOG=y
 CONFIG_MVEBU_SPL_BOOT_DEVICE_SATA=y
 CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
 CONFIG_SPL_TEXT_BASE=0x40000030
+CONFIG_SPL_MMC=y
 CONFIG_SPL_SERIAL=y
+CONFIG_SPL_STACK=0x4002c000
 CONFIG_SPL=y
 CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
@@ -18,8 +23,6 @@ CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
 CONFIG_AHCI=y
 CONFIG_DISTRO_DEFAULTS=y
-CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
-CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff0000
 CONFIG_BOOTDELAY=3
 CONFIG_USE_PREBOOT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
@@ -31,7 +34,6 @@ CONFIG_SPL_BSS_START_ADDR=0x40023000
 CONFIG_SPL_BSS_MAX_SIZE=0x4000
 CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
-CONFIG_SPL_STACK=0x4002c000
 CONFIG_SPL_I2C=y
 CONFIG_SYS_MAXARGS=32
 CONFIG_CMD_TLV_EEPROM=y
@@ -46,7 +48,6 @@ CONFIG_CMD_USB=y
 CONFIG_CMD_TFTPPUT=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
-CONFIG_CMD_MVEBU_BUBT=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_MIN_ENTRIES=128
 CONFIG_ARP_TIMEOUT=200
@@ -59,6 +60,8 @@ CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_MVTWSI=y
 CONFIG_I2C_EEPROM=y
 CONFIG_SPL_I2C_EEPROM=y
+CONFIG_MMC_BROKEN_CD=y
+CONFIG_SPL_DM_MMC=y
 CONFIG_SUPPORT_EMMC_BOOT=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_SDMA=y
-- 
2.39.2


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

* [PATCH 3/4] arm: mvebu: clearfog: Add defconfig for SPI booting
  2023-03-05 11:30 [PATCH 0/4] arm: mvebu: clearfog: defconfig and eMMC updates Martin Rowe
  2023-03-05 11:30 ` [PATCH 1/4] arm: mvebu: clearfog: Fix MMC detection Martin Rowe
  2023-03-05 11:30 ` [PATCH 2/4] arm: mvebu: clearfog: Align defconfigs Martin Rowe
@ 2023-03-05 11:30 ` Martin Rowe
  2023-03-05 12:29   ` Pali Rohár
  2023-03-05 11:31 ` [PATCH 4/4] arm: mvebu: clearfog: Detect MMC vs SDHC and fixup fdt Martin Rowe
  3 siblings, 1 reply; 13+ messages in thread
From: Martin Rowe @ 2023-03-05 11:30 UTC (permalink / raw)
  To: pali; +Cc: josua, jon, mibodhi, sr, u-boot, Martin Rowe

This new clearfog_spi_defconfig file is a copy of existing
clearfog_defconfig file modified to instruct build system to generate
final kwbimage for SPI booting and to store the environment in SPI.

Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
---
 configs/{clearfog_sata_defconfig => clearfog_spi_defconfig} | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
 copy configs/{clearfog_sata_defconfig => clearfog_spi_defconfig} (96%)

diff --git a/configs/clearfog_sata_defconfig b/configs/clearfog_spi_defconfig
similarity index 96%
copy from configs/clearfog_sata_defconfig
copy to configs/clearfog_spi_defconfig
index b11d8746b0..d6695f31e6 100644
--- a/configs/clearfog_sata_defconfig
+++ b/configs/clearfog_spi_defconfig
@@ -10,7 +10,7 @@ CONFIG_NR_DRAM_BANKS=2
 CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
 CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff0000
 CONFIG_TARGET_CLEARFOG=y
-CONFIG_MVEBU_SPL_BOOT_DEVICE_SATA=y
+CONFIG_ENV_SECT_SIZE=0x10000
 CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
 CONFIG_SPL_TEXT_BASE=0x40000030
 CONFIG_SPL_MMC=y
@@ -19,6 +19,7 @@ CONFIG_SPL_STACK=0x4002c000
 CONFIG_SPL=y
 CONFIG_DEBUG_UART_BASE=0xf1012000
 CONFIG_DEBUG_UART_CLOCK=250000000
+CONFIG_SPL_LIBDISK_SUPPORT=y
 CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
 CONFIG_AHCI=y
@@ -48,6 +49,7 @@ CONFIG_CMD_USB=y
 CONFIG_CMD_TFTPPUT=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
+CONFIG_MVEBU_SATA_BOOT=y
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_MIN_ENTRIES=128
 CONFIG_ARP_TIMEOUT=200
-- 
2.39.2


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

* [PATCH 4/4] arm: mvebu: clearfog: Detect MMC vs SDHC and fixup fdt
  2023-03-05 11:30 [PATCH 0/4] arm: mvebu: clearfog: defconfig and eMMC updates Martin Rowe
                   ` (2 preceding siblings ...)
  2023-03-05 11:30 ` [PATCH 3/4] arm: mvebu: clearfog: Add defconfig for SPI booting Martin Rowe
@ 2023-03-05 11:31 ` Martin Rowe
  2023-03-05 12:43   ` Pali Rohár
  3 siblings, 1 reply; 13+ messages in thread
From: Martin Rowe @ 2023-03-05 11:31 UTC (permalink / raw)
  To: pali; +Cc: josua, jon, mibodhi, sr, u-boot, Martin Rowe

[upstream of vendor commit 19a96f7c40a8fc1d0a6546ac2418d966e5840a99]

The Clearfog devices have only one SDHC device. This is either eMMC if
it is populated on the SOM or SDHC if not. The Linux device tree assumes
the SDHC case. Detect if the device is an eMMC and fixup the device-tree
so it will be detected by Linux.

Ported from vendor repo at https://github.com/SolidRun/u-boot

Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
---
 board/solidrun/clearfog/clearfog.c | 42 ++++++++++++++++++++++++++++++
 configs/clearfog_defconfig         |  1 +
 configs/clearfog_sata_defconfig    |  1 +
 configs/clearfog_spi_defconfig     |  1 +
 4 files changed, 45 insertions(+)

diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
index 03adb591d8..cafead3a1a 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -10,6 +10,7 @@
 #include <miiphy.h>
 #include <net.h>
 #include <netdev.h>
+#include <mmc.h>
 #include <asm/global_data.h>
 #include <asm/io.h>
 #include <asm/arch/cpu.h>
@@ -261,3 +262,44 @@ int board_late_init(void)
 
 	return 0;
 }
+
+static bool has_emmc(void)
+{
+	struct mmc *mmc;
+	mmc = find_mmc_device(0);
+	if (!mmc)
+		return 0;
+	return (!mmc_init(mmc) && IS_MMC(mmc)) ? true : false;
+}
+
+/*
+ * The Clearfog devices have only one SDHC device. This is either eMMC
+ * if it is populated on the SOM or SDHC if not. The Linux device tree
+ * assumes the SDHC case. Detect if the device is an eMMC and fixup the
+ * device-tree so it will be detected by Linux.
+ */
+int ft_board_setup(void *blob, struct bd_info *bd)
+{
+	__maybe_unused int offset, rc;
+
+	if (has_emmc()) {
+		puts("Patching FDT so that eMMC is detected by kernel\n");
+		offset = fdt_path_offset(blob, "/soc/internal-regs/sdhci@d8000");
+		if (offset >= 0) {
+			rc = fdt_delprop(blob, offset, "cd-gpios");
+			if (rc == -FDT_ERR_NOTFOUND)
+				return 0; /* Assume that the device tree is already customized for eMMC */
+			if (rc) {
+				printf("Unable to remove cd-gpios for eMMC, err=%s\n", fdt_strerror(rc));
+				return rc;
+			}
+			rc = fdt_setprop_empty(blob, offset, "non-removable");
+			if (rc) {
+				printf("Unable to add non-removable for eMMC, err=%s\n", fdt_strerror(rc));
+				return rc;
+			}
+		}
+	}
+
+	return 0;
+}
diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
index f9cfa94e77..45b093c6d8 100644
--- a/configs/clearfog_defconfig
+++ b/configs/clearfog_defconfig
@@ -21,6 +21,7 @@ CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
 CONFIG_AHCI=y
 CONFIG_DISTRO_DEFAULTS=y
+CONFIG_OF_BOARD_SETUP=y
 CONFIG_BOOTDELAY=3
 CONFIG_USE_PREBOOT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
diff --git a/configs/clearfog_sata_defconfig b/configs/clearfog_sata_defconfig
index b11d8746b0..081ebd7e7a 100644
--- a/configs/clearfog_sata_defconfig
+++ b/configs/clearfog_sata_defconfig
@@ -23,6 +23,7 @@ CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
 CONFIG_AHCI=y
 CONFIG_DISTRO_DEFAULTS=y
+CONFIG_OF_BOARD_SETUP=y
 CONFIG_BOOTDELAY=3
 CONFIG_USE_PREBOOT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
diff --git a/configs/clearfog_spi_defconfig b/configs/clearfog_spi_defconfig
index d6695f31e6..6b9e817f68 100644
--- a/configs/clearfog_spi_defconfig
+++ b/configs/clearfog_spi_defconfig
@@ -24,6 +24,7 @@ CONFIG_SYS_LOAD_ADDR=0x800000
 CONFIG_DEBUG_UART=y
 CONFIG_AHCI=y
 CONFIG_DISTRO_DEFAULTS=y
+CONFIG_OF_BOARD_SETUP=y
 CONFIG_BOOTDELAY=3
 CONFIG_USE_PREBOOT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
-- 
2.39.2


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

* Re: [PATCH 1/4] arm: mvebu: clearfog: Fix MMC detection
  2023-03-05 11:30 ` [PATCH 1/4] arm: mvebu: clearfog: Fix MMC detection Martin Rowe
@ 2023-03-05 12:19   ` Pali Rohár
  2023-03-06  5:35     ` Martin Rowe
  0 siblings, 1 reply; 13+ messages in thread
From: Pali Rohár @ 2023-03-05 12:19 UTC (permalink / raw)
  To: Martin Rowe; +Cc: josua, jon, mibodhi, sr, u-boot

On Sunday 05 March 2023 21:30:57 Martin Rowe wrote:
> A388 Clearfog MMC is either SD Card or eMMC with different behaviour for
> both. Setting MMC_BROKEN_CD allows both to correctly detect MMC.

When you are sending defconfig changes, please do not put there
unrelated canonicalization changes. Canonicalization of defconfigs are
done semi-automatically when Tom regenerates them. So avoid
CONFIG_SUPPORT_EMMC_BOOT and CONFIG_CMD_MVEBU_BUBT changes for eMMC
related change.


Anyway, I have looked at how MMC_BROKEN_CD is working in u-boot. The
relevant part is sdhci_get_cd() function in drivers/mmc/sdhci.c file and
in mmc_of_parse() function in drivers/mmc/mmc-uclass.c file.

So instead of MMC_BROKEN_CD would not it be better the following u-boot
specific patch? cd-gpios is completely ignored by u-boot when
non-removable is set.

diff --git a/arch/arm/dts/armada-388-clearfog-u-boot.dtsi b/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
index 96629294be4b..e725770c1899 100644
--- a/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
+++ b/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
@@ -10,6 +10,7 @@
 
 &sdhci {
 	u-boot,dm-spl;
+	non-removable; /* assume that the card is always present, required for eMMC variant */
 };
 
 &gpio0 {


> Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
> ---
>  configs/clearfog_defconfig | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
> index 8cd35f9f1a..24e7c16ac7 100644
> --- a/configs/clearfog_defconfig
> +++ b/configs/clearfog_defconfig
> @@ -46,7 +46,6 @@ CONFIG_CMD_USB=y
>  CONFIG_CMD_TFTPPUT=y
>  CONFIG_CMD_CACHE=y
>  CONFIG_CMD_TIME=y
> -CONFIG_CMD_MVEBU_BUBT=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_ENV_MIN_ENTRIES=128
>  CONFIG_ARP_TIMEOUT=200
> @@ -59,7 +58,7 @@ CONFIG_DM_I2C=y
>  CONFIG_SYS_I2C_MVTWSI=y
>  CONFIG_I2C_EEPROM=y
>  CONFIG_SPL_I2C_EEPROM=y
> -CONFIG_SUPPORT_EMMC_BOOT=y
> +CONFIG_MMC_BROKEN_CD=y
>  CONFIG_MMC_SDHCI=y
>  CONFIG_MMC_SDHCI_SDMA=y
>  CONFIG_MMC_SDHCI_MV=y
> -- 
> 2.39.2
> 

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

* Re: [PATCH 2/4] arm: mvebu: clearfog: Align defconfigs
  2023-03-05 11:30 ` [PATCH 2/4] arm: mvebu: clearfog: Align defconfigs Martin Rowe
@ 2023-03-05 12:27   ` Pali Rohár
  2023-03-06  5:47     ` Martin Rowe
  0 siblings, 1 reply; 13+ messages in thread
From: Pali Rohár @ 2023-03-05 12:27 UTC (permalink / raw)
  To: Martin Rowe; +Cc: josua, jon, mibodhi, sr, u-boot

On Sunday 05 March 2023 21:30:58 Martin Rowe wrote:
> Ensure that functionality of generated configs is identical between SATA
> and MMC defconfigs for Clearfog boards. The only expected difference is
> the boot mode and environment location.
> 
> Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
> ---
>  configs/clearfog_defconfig      |  1 +
>  configs/clearfog_sata_defconfig | 11 +++++++----
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
> index 24e7c16ac7..f9cfa94e77 100644
> --- a/configs/clearfog_defconfig
> +++ b/configs/clearfog_defconfig
> @@ -46,6 +46,7 @@ CONFIG_CMD_USB=y
>  CONFIG_CMD_TFTPPUT=y
>  CONFIG_CMD_CACHE=y
>  CONFIG_CMD_TIME=y
> +CONFIG_MVEBU_SATA_BOOT=y

MVEBU_*_BOOT is the default target for the bubt command. I guess that
you do not want to set default one to SATA for eMMC/SD boot.

Anyway, when you are doing cleanup of clearfog defconfig files, what
about renaming clearfog_defconfig to clearfog_mmc_defconfig?

>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_ENV_MIN_ENTRIES=128
>  CONFIG_ARP_TIMEOUT=200
> diff --git a/configs/clearfog_sata_defconfig b/configs/clearfog_sata_defconfig
> index e9b36150ea..b11d8746b0 100644
> --- a/configs/clearfog_sata_defconfig
> +++ b/configs/clearfog_sata_defconfig
> @@ -3,14 +3,19 @@ CONFIG_ARCH_CPU_INIT=y
>  CONFIG_SYS_THUMB_BUILD=y
>  CONFIG_ARCH_MVEBU=y
>  CONFIG_TEXT_BASE=0x00800000
> +CONFIG_SPL_GPIO=y
>  CONFIG_SPL_LIBCOMMON_SUPPORT=y
>  CONFIG_SPL_LIBGENERIC_SUPPORT=y
>  CONFIG_NR_DRAM_BANKS=2
> +CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> +CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff0000
>  CONFIG_TARGET_CLEARFOG=y
>  CONFIG_MVEBU_SPL_BOOT_DEVICE_SATA=y
>  CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
>  CONFIG_SPL_TEXT_BASE=0x40000030
> +CONFIG_SPL_MMC=y

SPL contains code just for loading main u-boot. So I think you do not
need MMC support in SPL code (symbols prefixed by CONFIG_SPL) which
loads u-boot from SATA.

>  CONFIG_SPL_SERIAL=y
> +CONFIG_SPL_STACK=0x4002c000
>  CONFIG_SPL=y
>  CONFIG_DEBUG_UART_BASE=0xf1012000
>  CONFIG_DEBUG_UART_CLOCK=250000000
> @@ -18,8 +23,6 @@ CONFIG_SYS_LOAD_ADDR=0x800000
>  CONFIG_DEBUG_UART=y
>  CONFIG_AHCI=y
>  CONFIG_DISTRO_DEFAULTS=y
> -CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> -CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff0000
>  CONFIG_BOOTDELAY=3
>  CONFIG_USE_PREBOOT=y
>  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> @@ -31,7 +34,6 @@ CONFIG_SPL_BSS_START_ADDR=0x40023000
>  CONFIG_SPL_BSS_MAX_SIZE=0x4000
>  CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
> -CONFIG_SPL_STACK=0x4002c000
>  CONFIG_SPL_I2C=y
>  CONFIG_SYS_MAXARGS=32
>  CONFIG_CMD_TLV_EEPROM=y
> @@ -46,7 +48,6 @@ CONFIG_CMD_USB=y
>  CONFIG_CMD_TFTPPUT=y
>  CONFIG_CMD_CACHE=y
>  CONFIG_CMD_TIME=y
> -CONFIG_CMD_MVEBU_BUBT=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_ENV_MIN_ENTRIES=128
>  CONFIG_ARP_TIMEOUT=200
> @@ -59,6 +60,8 @@ CONFIG_DM_I2C=y
>  CONFIG_SYS_I2C_MVTWSI=y
>  CONFIG_I2C_EEPROM=y
>  CONFIG_SPL_I2C_EEPROM=y
> +CONFIG_MMC_BROKEN_CD=y
> +CONFIG_SPL_DM_MMC=y
>  CONFIG_SUPPORT_EMMC_BOOT=y
>  CONFIG_MMC_SDHCI=y
>  CONFIG_MMC_SDHCI_SDMA=y
> -- 
> 2.39.2
> 

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

* Re: [PATCH 3/4] arm: mvebu: clearfog: Add defconfig for SPI booting
  2023-03-05 11:30 ` [PATCH 3/4] arm: mvebu: clearfog: Add defconfig for SPI booting Martin Rowe
@ 2023-03-05 12:29   ` Pali Rohár
  2023-03-06  6:11     ` Martin Rowe
  0 siblings, 1 reply; 13+ messages in thread
From: Pali Rohár @ 2023-03-05 12:29 UTC (permalink / raw)
  To: Martin Rowe; +Cc: josua, jon, mibodhi, sr, u-boot

On Sunday 05 March 2023 21:30:59 Martin Rowe wrote:
> This new clearfog_spi_defconfig file is a copy of existing
> clearfog_defconfig file modified to instruct build system to generate
> final kwbimage for SPI booting and to store the environment in SPI.
> 
> Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
> ---
>  configs/{clearfog_sata_defconfig => clearfog_spi_defconfig} | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>  copy configs/{clearfog_sata_defconfig => clearfog_spi_defconfig} (96%)
> 
> diff --git a/configs/clearfog_sata_defconfig b/configs/clearfog_spi_defconfig
> similarity index 96%
> copy from configs/clearfog_sata_defconfig
> copy to configs/clearfog_spi_defconfig
> index b11d8746b0..d6695f31e6 100644
> --- a/configs/clearfog_sata_defconfig
> +++ b/configs/clearfog_spi_defconfig
> @@ -10,7 +10,7 @@ CONFIG_NR_DRAM_BANKS=2
>  CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
>  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff0000
>  CONFIG_TARGET_CLEARFOG=y
> -CONFIG_MVEBU_SPL_BOOT_DEVICE_SATA=y
> +CONFIG_ENV_SECT_SIZE=0x10000
>  CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
>  CONFIG_SPL_TEXT_BASE=0x40000030
>  CONFIG_SPL_MMC=y
> @@ -19,6 +19,7 @@ CONFIG_SPL_STACK=0x4002c000
>  CONFIG_SPL=y
>  CONFIG_DEBUG_UART_BASE=0xf1012000
>  CONFIG_DEBUG_UART_CLOCK=250000000
> +CONFIG_SPL_LIBDISK_SUPPORT=y

I think you do not need libdisk support in SPL which loads main u-boot
from SPI.

>  CONFIG_SYS_LOAD_ADDR=0x800000
>  CONFIG_DEBUG_UART=y
>  CONFIG_AHCI=y
> @@ -48,6 +49,7 @@ CONFIG_CMD_USB=y
>  CONFIG_CMD_TFTPPUT=y
>  CONFIG_CMD_CACHE=y
>  CONFIG_CMD_TIME=y
> +CONFIG_MVEBU_SATA_BOOT=y

Should not be default bubt location SPI for SPI booting?

>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_ENV_MIN_ENTRIES=128
>  CONFIG_ARP_TIMEOUT=200
> -- 
> 2.39.2
> 

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

* Re: [PATCH 4/4] arm: mvebu: clearfog: Detect MMC vs SDHC and fixup fdt
  2023-03-05 11:31 ` [PATCH 4/4] arm: mvebu: clearfog: Detect MMC vs SDHC and fixup fdt Martin Rowe
@ 2023-03-05 12:43   ` Pali Rohár
  2023-03-06  6:32     ` Martin Rowe
  0 siblings, 1 reply; 13+ messages in thread
From: Pali Rohár @ 2023-03-05 12:43 UTC (permalink / raw)
  To: Martin Rowe; +Cc: josua, jon, mibodhi, sr, u-boot

On Sunday 05 March 2023 21:31:00 Martin Rowe wrote:
> [upstream of vendor commit 19a96f7c40a8fc1d0a6546ac2418d966e5840a99]
> 
> The Clearfog devices have only one SDHC device. This is either eMMC if
> it is populated on the SOM or SDHC if not. The Linux device tree assumes
> the SDHC case. Detect if the device is an eMMC and fixup the device-tree
> so it will be detected by Linux.
> 
> Ported from vendor repo at https://github.com/SolidRun/u-boot
> 
> Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
> ---
>  board/solidrun/clearfog/clearfog.c | 42 ++++++++++++++++++++++++++++++
>  configs/clearfog_defconfig         |  1 +
>  configs/clearfog_sata_defconfig    |  1 +
>  configs/clearfog_spi_defconfig     |  1 +
>  4 files changed, 45 insertions(+)
> 
> diff --git a/board/solidrun/clearfog/clearfog.c b/board/solidrun/clearfog/clearfog.c
> index 03adb591d8..cafead3a1a 100644
> --- a/board/solidrun/clearfog/clearfog.c
> +++ b/board/solidrun/clearfog/clearfog.c
> @@ -10,6 +10,7 @@
>  #include <miiphy.h>
>  #include <net.h>
>  #include <netdev.h>
> +#include <mmc.h>
>  #include <asm/global_data.h>
>  #include <asm/io.h>
>  #include <asm/arch/cpu.h>
> @@ -261,3 +262,44 @@ int board_late_init(void)
>  
>  	return 0;
>  }
> +
> +static bool has_emmc(void)
> +{
> +	struct mmc *mmc;
> +	mmc = find_mmc_device(0);
> +	if (!mmc)
> +		return 0;
> +	return (!mmc_init(mmc) && IS_MMC(mmc)) ? true : false;
> +}
> +
> +/*
> + * The Clearfog devices have only one SDHC device. This is either eMMC
> + * if it is populated on the SOM or SDHC if not. The Linux device tree
> + * assumes the SDHC case. Detect if the device is an eMMC and fixup the
> + * device-tree so it will be detected by Linux.
> + */
> +int ft_board_setup(void *blob, struct bd_info *bd)
> +{
> +	__maybe_unused int offset, rc;
> +
> +	if (has_emmc()) {
> +		puts("Patching FDT so that eMMC is detected by kernel\n");
> +		offset = fdt_path_offset(blob, "/soc/internal-regs/sdhci@d8000");

Do not use fixed DT paths when parsing Linux kernel DT files. Linux
developers often changes DT node names in their DTS files which cause
changing of DT paths and hence this code in bootloader stops working.

I would suggest to find node offset by compatible string
("marvell,armada-380-sdhci"). Look for inspiration what I done in past:
https://source.denx.de/u-boot/u-boot/-/commit/7cd67018dd7f754c66cf08a76397bbee254b32aa

> +		if (offset >= 0) {
> +			rc = fdt_delprop(blob, offset, "cd-gpios");
> +			if (rc == -FDT_ERR_NOTFOUND)
> +				return 0; /* Assume that the device tree is already customized for eMMC */

I would suggest to always add "non-removable" property when it does not
exist.

> +			if (rc) {
> +				printf("Unable to remove cd-gpios for eMMC, err=%s\n", fdt_strerror(rc));
> +				return rc;

Normally it is not a good idea to return non-zero value from
ft_board_setup() function because it "crashes" u-boot and stops booting
kernel.

> +			}
> +			rc = fdt_setprop_empty(blob, offset, "non-removable");
> +			if (rc) {
> +				printf("Unable to add non-removable for eMMC, err=%s\n", fdt_strerror(rc));
> +				return rc;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
> index f9cfa94e77..45b093c6d8 100644
> --- a/configs/clearfog_defconfig
> +++ b/configs/clearfog_defconfig
> @@ -21,6 +21,7 @@ CONFIG_SYS_LOAD_ADDR=0x800000
>  CONFIG_DEBUG_UART=y
>  CONFIG_AHCI=y
>  CONFIG_DISTRO_DEFAULTS=y
> +CONFIG_OF_BOARD_SETUP=y

Hint: To avoid having CONFIG_OF_BOARD_SETUP=y in every defconfig file,
you can add "select OF_BOARD_SETUP" into "config TARGET_CLEARFOG"
section in arch/arm/mach-mvebu/Kconfig file.

>  CONFIG_BOOTDELAY=3
>  CONFIG_USE_PREBOOT=y
>  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> diff --git a/configs/clearfog_sata_defconfig b/configs/clearfog_sata_defconfig
> index b11d8746b0..081ebd7e7a 100644
> --- a/configs/clearfog_sata_defconfig
> +++ b/configs/clearfog_sata_defconfig
> @@ -23,6 +23,7 @@ CONFIG_SYS_LOAD_ADDR=0x800000
>  CONFIG_DEBUG_UART=y
>  CONFIG_AHCI=y
>  CONFIG_DISTRO_DEFAULTS=y
> +CONFIG_OF_BOARD_SETUP=y
>  CONFIG_BOOTDELAY=3
>  CONFIG_USE_PREBOOT=y
>  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> diff --git a/configs/clearfog_spi_defconfig b/configs/clearfog_spi_defconfig
> index d6695f31e6..6b9e817f68 100644
> --- a/configs/clearfog_spi_defconfig
> +++ b/configs/clearfog_spi_defconfig
> @@ -24,6 +24,7 @@ CONFIG_SYS_LOAD_ADDR=0x800000
>  CONFIG_DEBUG_UART=y
>  CONFIG_AHCI=y
>  CONFIG_DISTRO_DEFAULTS=y
> +CONFIG_OF_BOARD_SETUP=y
>  CONFIG_BOOTDELAY=3
>  CONFIG_USE_PREBOOT=y
>  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> -- 
> 2.39.2
> 

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

* Re: [PATCH 1/4] arm: mvebu: clearfog: Fix MMC detection
  2023-03-05 12:19   ` Pali Rohár
@ 2023-03-06  5:35     ` Martin Rowe
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Rowe @ 2023-03-06  5:35 UTC (permalink / raw)
  To: Pali Rohár; +Cc: josua, jon, mibodhi, sr, u-boot

On Sun, 5 Mar 2023 at 12:19, Pali Rohár <pali@kernel.org> wrote:

> On Sunday 05 March 2023 21:30:57 Martin Rowe wrote:
> > A388 Clearfog MMC is either SD Card or eMMC with different behaviour for
> > both. Setting MMC_BROKEN_CD allows both to correctly detect MMC.
>
> When you are sending defconfig changes, please do not put there
> unrelated canonicalization changes. Canonicalization of defconfigs are
> done semi-automatically when Tom regenerates them. So avoid
> CONFIG_SUPPORT_EMMC_BOOT and CONFIG_CMD_MVEBU_BUBT changes for eMMC
> related change.
>

Noted. I'll fix it up.

Anyway, I have looked at how MMC_BROKEN_CD is working in u-boot. The
> relevant part is sdhci_get_cd() function in drivers/mmc/sdhci.c file and
> in mmc_of_parse() function in drivers/mmc/mmc-uclass.c file.
>
> So instead of MMC_BROKEN_CD would not it be better the following u-boot
> specific patch? cd-gpios is completely ignored by u-boot when
> non-removable is set.
>

Both are fine with me. I initially went with MMC_BROKEN_CD since it seemed
like the established way. I've tested the patch below and the only issue is
in the corner case of non-MMC boot while expecting something from MMC, but
the error message is helpful enough ("Card did not respond to voltage
select! : -110"). I'll resubmit with your patch instead.


> diff --git a/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
> b/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
> index 96629294be4b..e725770c1899 100644
> --- a/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
> +++ b/arch/arm/dts/armada-388-clearfog-u-boot.dtsi
> @@ -10,6 +10,7 @@
>
>  &sdhci {
>         u-boot,dm-spl;
> +       non-removable; /* assume that the card is always present, required
> for eMMC variant */
>  };
>
>  &gpio0 {
>
>
> > Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
> > ---
> >  configs/clearfog_defconfig | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
> > index 8cd35f9f1a..24e7c16ac7 100644
> > --- a/configs/clearfog_defconfig
> > +++ b/configs/clearfog_defconfig
> > @@ -46,7 +46,6 @@ CONFIG_CMD_USB=y
> >  CONFIG_CMD_TFTPPUT=y
> >  CONFIG_CMD_CACHE=y
> >  CONFIG_CMD_TIME=y
> > -CONFIG_CMD_MVEBU_BUBT=y
> >  CONFIG_ENV_OVERWRITE=y
> >  CONFIG_ENV_MIN_ENTRIES=128
> >  CONFIG_ARP_TIMEOUT=200
> > @@ -59,7 +58,7 @@ CONFIG_DM_I2C=y
> >  CONFIG_SYS_I2C_MVTWSI=y
> >  CONFIG_I2C_EEPROM=y
> >  CONFIG_SPL_I2C_EEPROM=y
> > -CONFIG_SUPPORT_EMMC_BOOT=y
> > +CONFIG_MMC_BROKEN_CD=y
> >  CONFIG_MMC_SDHCI=y
> >  CONFIG_MMC_SDHCI_SDMA=y
> >  CONFIG_MMC_SDHCI_MV=y
> > --
> > 2.39.2
> >
>

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

* Re: [PATCH 2/4] arm: mvebu: clearfog: Align defconfigs
  2023-03-05 12:27   ` Pali Rohár
@ 2023-03-06  5:47     ` Martin Rowe
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Rowe @ 2023-03-06  5:47 UTC (permalink / raw)
  To: Pali Rohár; +Cc: josua, jon, mibodhi, sr, u-boot

On Sun, 5 Mar 2023 at 12:27, Pali Rohár <pali@kernel.org> wrote:

> On Sunday 05 March 2023 21:30:58 Martin Rowe wrote:
> > Ensure that functionality of generated configs is identical between SATA
> > and MMC defconfigs for Clearfog boards. The only expected difference is
> > the boot mode and environment location.
> >
> > Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
> > ---
> >  configs/clearfog_defconfig      |  1 +
> >  configs/clearfog_sata_defconfig | 11 +++++++----
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
> > index 24e7c16ac7..f9cfa94e77 100644
> > --- a/configs/clearfog_defconfig
> > +++ b/configs/clearfog_defconfig
> > @@ -46,6 +46,7 @@ CONFIG_CMD_USB=y
> >  CONFIG_CMD_TFTPPUT=y
> >  CONFIG_CMD_CACHE=y
> >  CONFIG_CMD_TIME=y
> > +CONFIG_MVEBU_SATA_BOOT=y
>
> MVEBU_*_BOOT is the default target for the bubt command. I guess that
> you do not want to set default one to SATA for eMMC/SD boot.
>

Correct, I'll remove it.

Anyway, when you are doing cleanup of clearfog defconfig files, what
> about renaming clearfog_defconfig to clearfog_mmc_defconfig?
>

I considered this but left it as is since we have several years worth of
support with that original name. It will make bisects easier if we keep it
the same, but I don't feel strongly either way. Happy to rename it if you'd
prefer.


> >  CONFIG_ENV_OVERWRITE=y
> >  CONFIG_ENV_MIN_ENTRIES=128
> >  CONFIG_ARP_TIMEOUT=200
> > diff --git a/configs/clearfog_sata_defconfig
> b/configs/clearfog_sata_defconfig
> > index e9b36150ea..b11d8746b0 100644
> > --- a/configs/clearfog_sata_defconfig
> > +++ b/configs/clearfog_sata_defconfig
> > @@ -3,14 +3,19 @@ CONFIG_ARCH_CPU_INIT=y
> >  CONFIG_SYS_THUMB_BUILD=y
> >  CONFIG_ARCH_MVEBU=y
> >  CONFIG_TEXT_BASE=0x00800000
> > +CONFIG_SPL_GPIO=y
> >  CONFIG_SPL_LIBCOMMON_SUPPORT=y
> >  CONFIG_SPL_LIBGENERIC_SUPPORT=y
> >  CONFIG_NR_DRAM_BANKS=2
> > +CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> > +CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff0000
> >  CONFIG_TARGET_CLEARFOG=y
> >  CONFIG_MVEBU_SPL_BOOT_DEVICE_SATA=y
> >  CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
> >  CONFIG_SPL_TEXT_BASE=0x40000030
> > +CONFIG_SPL_MMC=y
>
> SPL contains code just for loading main u-boot. So I think you do not
> need MMC support in SPL code (symbols prefixed by CONFIG_SPL) which
> loads u-boot from SATA.
>

Correct, I'll remove it.

That's actually all of the changes removed (the rest are canonicalization
or the resolved MMC issue), so I'll drop this patch completely in v2.

>  CONFIG_SPL_SERIAL=y
> > +CONFIG_SPL_STACK=0x4002c000
> >  CONFIG_SPL=y
> >  CONFIG_DEBUG_UART_BASE=0xf1012000
> >  CONFIG_DEBUG_UART_CLOCK=250000000
> > @@ -18,8 +23,6 @@ CONFIG_SYS_LOAD_ADDR=0x800000
> >  CONFIG_DEBUG_UART=y
> >  CONFIG_AHCI=y
> >  CONFIG_DISTRO_DEFAULTS=y
> > -CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> > -CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff0000
> >  CONFIG_BOOTDELAY=3
> >  CONFIG_USE_PREBOOT=y
> >  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> > @@ -31,7 +34,6 @@ CONFIG_SPL_BSS_START_ADDR=0x40023000
> >  CONFIG_SPL_BSS_MAX_SIZE=0x4000
> >  CONFIG_SPL_SYS_MALLOC_SIMPLE=y
> >  # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
> > -CONFIG_SPL_STACK=0x4002c000
> >  CONFIG_SPL_I2C=y
> >  CONFIG_SYS_MAXARGS=32
> >  CONFIG_CMD_TLV_EEPROM=y
> > @@ -46,7 +48,6 @@ CONFIG_CMD_USB=y
> >  CONFIG_CMD_TFTPPUT=y
> >  CONFIG_CMD_CACHE=y
> >  CONFIG_CMD_TIME=y
> > -CONFIG_CMD_MVEBU_BUBT=y
> >  CONFIG_ENV_OVERWRITE=y
> >  CONFIG_ENV_MIN_ENTRIES=128
> >  CONFIG_ARP_TIMEOUT=200
> > @@ -59,6 +60,8 @@ CONFIG_DM_I2C=y
> >  CONFIG_SYS_I2C_MVTWSI=y
> >  CONFIG_I2C_EEPROM=y
> >  CONFIG_SPL_I2C_EEPROM=y
> > +CONFIG_MMC_BROKEN_CD=y
> > +CONFIG_SPL_DM_MMC=y
> >  CONFIG_SUPPORT_EMMC_BOOT=y
> >  CONFIG_MMC_SDHCI=y
> >  CONFIG_MMC_SDHCI_SDMA=y
> > --
> > 2.39.2
> >
>

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

* Re: [PATCH 3/4] arm: mvebu: clearfog: Add defconfig for SPI booting
  2023-03-05 12:29   ` Pali Rohár
@ 2023-03-06  6:11     ` Martin Rowe
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Rowe @ 2023-03-06  6:11 UTC (permalink / raw)
  To: Pali Rohár; +Cc: josua, jon, mibodhi, sr, u-boot

On Sun, 5 Mar 2023 at 12:29, Pali Rohár <pali@kernel.org> wrote:

> On Sunday 05 March 2023 21:30:59 Martin Rowe wrote:
> > This new clearfog_spi_defconfig file is a copy of existing
> > clearfog_defconfig file modified to instruct build system to generate
> > final kwbimage for SPI booting and to store the environment in SPI.
> >
> > Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
> > ---
> >  configs/{clearfog_sata_defconfig => clearfog_spi_defconfig} | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >  copy configs/{clearfog_sata_defconfig => clearfog_spi_defconfig} (96%)
> >
> > diff --git a/configs/clearfog_sata_defconfig
> b/configs/clearfog_spi_defconfig
> > similarity index 96%
> > copy from configs/clearfog_sata_defconfig
> > copy to configs/clearfog_spi_defconfig
> > index b11d8746b0..d6695f31e6 100644
> > --- a/configs/clearfog_sata_defconfig
> > +++ b/configs/clearfog_spi_defconfig
> > @@ -10,7 +10,7 @@ CONFIG_NR_DRAM_BANKS=2
> >  CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> >  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff0000
> >  CONFIG_TARGET_CLEARFOG=y
> > -CONFIG_MVEBU_SPL_BOOT_DEVICE_SATA=y
> > +CONFIG_ENV_SECT_SIZE=0x10000
> >  CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
> >  CONFIG_SPL_TEXT_BASE=0x40000030
> >  CONFIG_SPL_MMC=y
> > @@ -19,6 +19,7 @@ CONFIG_SPL_STACK=0x4002c000
> >  CONFIG_SPL=y
> >  CONFIG_DEBUG_UART_BASE=0xf1012000
> >  CONFIG_DEBUG_UART_CLOCK=250000000
> > +CONFIG_SPL_LIBDISK_SUPPORT=y
>
> I think you do not need libdisk support in SPL which loads main u-boot
> from SPI.
>

Makes sense, I'll remove


> >  CONFIG_SYS_LOAD_ADDR=0x800000
> >  CONFIG_DEBUG_UART=y
> >  CONFIG_AHCI=y
> > @@ -48,6 +49,7 @@ CONFIG_CMD_USB=y
> >  CONFIG_CMD_TFTPPUT=y
> >  CONFIG_CMD_CACHE=y
> >  CONFIG_CMD_TIME=y
> > +CONFIG_MVEBU_SATA_BOOT=y
>
> Should not be default bubt location SPI for SPI booting?
>

Yes, I'll fix

>  CONFIG_ENV_OVERWRITE=y
> >  CONFIG_ENV_MIN_ENTRIES=128
> >  CONFIG_ARP_TIMEOUT=200
> > --
> > 2.39.2
> >
>

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

* Re: [PATCH 4/4] arm: mvebu: clearfog: Detect MMC vs SDHC and fixup fdt
  2023-03-05 12:43   ` Pali Rohár
@ 2023-03-06  6:32     ` Martin Rowe
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Rowe @ 2023-03-06  6:32 UTC (permalink / raw)
  To: Pali Rohár; +Cc: josua, jon, mibodhi, sr, u-boot

On Sun, 5 Mar 2023 at 12:43, Pali Rohár <pali@kernel.org> wrote:

> On Sunday 05 March 2023 21:31:00 Martin Rowe wrote:
> > [upstream of vendor commit 19a96f7c40a8fc1d0a6546ac2418d966e5840a99]
> >
> > The Clearfog devices have only one SDHC device. This is either eMMC if
> > it is populated on the SOM or SDHC if not. The Linux device tree assumes
> > the SDHC case. Detect if the device is an eMMC and fixup the device-tree
> > so it will be detected by Linux.
> >
> > Ported from vendor repo at https://github.com/SolidRun/u-boot
> >
> > Signed-off-by: Martin Rowe <martin.p.rowe@gmail.com>
> > ---
> >  board/solidrun/clearfog/clearfog.c | 42 ++++++++++++++++++++++++++++++
> >  configs/clearfog_defconfig         |  1 +
> >  configs/clearfog_sata_defconfig    |  1 +
> >  configs/clearfog_spi_defconfig     |  1 +
> >  4 files changed, 45 insertions(+)
> >
> > diff --git a/board/solidrun/clearfog/clearfog.c
> b/board/solidrun/clearfog/clearfog.c
> > index 03adb591d8..cafead3a1a 100644
> > --- a/board/solidrun/clearfog/clearfog.c
> > +++ b/board/solidrun/clearfog/clearfog.c
> > @@ -10,6 +10,7 @@
> >  #include <miiphy.h>
> >  #include <net.h>
> >  #include <netdev.h>
> > +#include <mmc.h>
> >  #include <asm/global_data.h>
> >  #include <asm/io.h>
> >  #include <asm/arch/cpu.h>
> > @@ -261,3 +262,44 @@ int board_late_init(void)
> >
> >       return 0;
> >  }
> > +
> > +static bool has_emmc(void)
> > +{
> > +     struct mmc *mmc;
> > +     mmc = find_mmc_device(0);
> > +     if (!mmc)
> > +             return 0;
> > +     return (!mmc_init(mmc) && IS_MMC(mmc)) ? true : false;
> > +}
> > +
> > +/*
> > + * The Clearfog devices have only one SDHC device. This is either eMMC
> > + * if it is populated on the SOM or SDHC if not. The Linux device tree
> > + * assumes the SDHC case. Detect if the device is an eMMC and fixup the
> > + * device-tree so it will be detected by Linux.
> > + */
> > +int ft_board_setup(void *blob, struct bd_info *bd)
> > +{
> > +     __maybe_unused int offset, rc;
> > +
> > +     if (has_emmc()) {
> > +             puts("Patching FDT so that eMMC is detected by kernel\n");
> > +             offset = fdt_path_offset(blob,
> "/soc/internal-regs/sdhci@d8000");
>
> Do not use fixed DT paths when parsing Linux kernel DT files. Linux
> developers often changes DT node names in their DTS files which cause
> changing of DT paths and hence this code in bootloader stops working.
>
> I would suggest to find node offset by compatible string
> ("marvell,armada-380-sdhci"). Look for inspiration what I done in past:
>
> https://source.denx.de/u-boot/u-boot/-/commit/7cd67018dd7f754c66cf08a76397bbee254b32aa
>

Makes sense, I'll rework the patch.


> > +             if (offset >= 0) {
> > +                     rc = fdt_delprop(blob, offset, "cd-gpios");
> > +                     if (rc == -FDT_ERR_NOTFOUND)
> > +                             return 0; /* Assume that the device tree
> is already customized for eMMC */
>
> I would suggest to always add "non-removable" property when it does not
> exist.
>

OK, that makes sense, too. I'll test that out.

> +                     if (rc) {
> > +                             printf("Unable to remove cd-gpios for
> eMMC, err=%s\n", fdt_strerror(rc));
> > +                             return rc;
>
> Normally it is not a good idea to return non-zero value from
> ft_board_setup() function because it "crashes" u-boot and stops booting
> kernel.
>

I think this might be an exception. If the board has eMMC there's a
reasonable chance that it's used for the root device. By the time Linux
tries and fails to mount the root device there are already reams of printks
on the serial console that make it hard to see the root cause all the way
back in u-boot. Failing early with an appropriate error message would be
more beneficial while troubleshooting this problem.

There's a corner case where you have an eMMC board and never use the eMMC
device (like my use case), so you wouldn't want to have it fail for
something that you don't use. I think we'll cause more issues for people
that can't mount their root device than for people that get an error for a
device they don't use.

Given I'm going to rework the logic here anyway it's likely this first
'return rc' will be removed, but I think it is worth retaining the one
below.

> +                     }
> > +                     rc = fdt_setprop_empty(blob, offset,
> "non-removable");
> > +                     if (rc) {
> > +                             printf("Unable to add non-removable for
> eMMC, err=%s\n", fdt_strerror(rc));
> > +                             return rc;
> > +                     }
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
> > index f9cfa94e77..45b093c6d8 100644
> > --- a/configs/clearfog_defconfig
> > +++ b/configs/clearfog_defconfig
> > @@ -21,6 +21,7 @@ CONFIG_SYS_LOAD_ADDR=0x800000
> >  CONFIG_DEBUG_UART=y
> >  CONFIG_AHCI=y
> >  CONFIG_DISTRO_DEFAULTS=y
> > +CONFIG_OF_BOARD_SETUP=y
>
> Hint: To avoid having CONFIG_OF_BOARD_SETUP=y in every defconfig file,
> you can add "select OF_BOARD_SETUP" into "config TARGET_CLEARFOG"
> section in arch/arm/mach-mvebu/Kconfig file.
>

I'll fix that up.


> >  CONFIG_BOOTDELAY=3
> >  CONFIG_USE_PREBOOT=y
> >  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> > diff --git a/configs/clearfog_sata_defconfig
> b/configs/clearfog_sata_defconfig
> > index b11d8746b0..081ebd7e7a 100644
> > --- a/configs/clearfog_sata_defconfig
> > +++ b/configs/clearfog_sata_defconfig
> > @@ -23,6 +23,7 @@ CONFIG_SYS_LOAD_ADDR=0x800000
> >  CONFIG_DEBUG_UART=y
> >  CONFIG_AHCI=y
> >  CONFIG_DISTRO_DEFAULTS=y
> > +CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_BOOTDELAY=3
> >  CONFIG_USE_PREBOOT=y
> >  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> > diff --git a/configs/clearfog_spi_defconfig
> b/configs/clearfog_spi_defconfig
> > index d6695f31e6..6b9e817f68 100644
> > --- a/configs/clearfog_spi_defconfig
> > +++ b/configs/clearfog_spi_defconfig
> > @@ -24,6 +24,7 @@ CONFIG_SYS_LOAD_ADDR=0x800000
> >  CONFIG_DEBUG_UART=y
> >  CONFIG_AHCI=y
> >  CONFIG_DISTRO_DEFAULTS=y
> > +CONFIG_OF_BOARD_SETUP=y
> >  CONFIG_BOOTDELAY=3
> >  CONFIG_USE_PREBOOT=y
> >  CONFIG_SYS_CONSOLE_INFO_QUIET=y
> > --
> > 2.39.2
> >
>

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

end of thread, other threads:[~2023-03-06  6:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-05 11:30 [PATCH 0/4] arm: mvebu: clearfog: defconfig and eMMC updates Martin Rowe
2023-03-05 11:30 ` [PATCH 1/4] arm: mvebu: clearfog: Fix MMC detection Martin Rowe
2023-03-05 12:19   ` Pali Rohár
2023-03-06  5:35     ` Martin Rowe
2023-03-05 11:30 ` [PATCH 2/4] arm: mvebu: clearfog: Align defconfigs Martin Rowe
2023-03-05 12:27   ` Pali Rohár
2023-03-06  5:47     ` Martin Rowe
2023-03-05 11:30 ` [PATCH 3/4] arm: mvebu: clearfog: Add defconfig for SPI booting Martin Rowe
2023-03-05 12:29   ` Pali Rohár
2023-03-06  6:11     ` Martin Rowe
2023-03-05 11:31 ` [PATCH 4/4] arm: mvebu: clearfog: Detect MMC vs SDHC and fixup fdt Martin Rowe
2023-03-05 12:43   ` Pali Rohár
2023-03-06  6:32     ` Martin Rowe

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.