[u-boot][master][PATCH 1/3] pico-imx7d: add support for 2GB memory SoMs

From: Benjamin Szőke egyszeregy@freemail.hu
Take over codes from Techenxion to support SoMs with 2GB DDR3.
Signed-off-by: Benjamin Szőke egyszeregy@freemail.hu --- board/technexion/pico-imx7d/Makefile | 2 +- .../pico-imx7d/{spl.c => pico-imx7d_spl.c} | 30 +++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) rename board/technexion/pico-imx7d/{spl.c => pico-imx7d_spl.c} (83%)
diff --git a/board/technexion/pico-imx7d/Makefile b/board/technexion/pico-imx7d/Makefile index 4ae3d606b5..b45b127884 100644 --- a/board/technexion/pico-imx7d/Makefile +++ b/board/technexion/pico-imx7d/Makefile @@ -1,4 +1,4 @@ # SPDX-License-Identifier: GPL-2.0+ # (C) Copyright 2017 NXP Semiconductors
-obj-y := pico-imx7d.o spl.o +obj-y := pico-imx7d.o pico-imx7d_spl.o diff --git a/board/technexion/pico-imx7d/spl.c b/board/technexion/pico-imx7d/pico-imx7d_spl.c similarity index 83% rename from board/technexion/pico-imx7d/spl.c rename to board/technexion/pico-imx7d/pico-imx7d_spl.c index df5f058577..0009c55022 100644 --- a/board/technexion/pico-imx7d/spl.c +++ b/board/technexion/pico-imx7d/pico-imx7d_spl.c @@ -61,6 +61,8 @@ static struct ddrc ddrc_regs_val = { .dramtmg0 = 0x09081109, .addrmap0 = 0x0000001f, .addrmap1 = 0x00080808, + .addrmap2 = 0x00000000, + .addrmap3 = 0x00000000, .addrmap4 = 0x00000f0f, .addrmap5 = 0x07070707, .addrmap6 = 0x0f0f0707, @@ -100,17 +102,39 @@ static void gpr_init(void) writel(0x4F400005, &gpr_regs->gpr[1]); }
+/********************************************** +* Revision Detection +* +* DDR_TYPE_DET_1 DDR_TYPE_DET_2 +* GPIO_1 GPIO_2 +* 0 1 2GB DDR3 +* 0 0 1GB DDR3 +* 1 0 512MB DDR3 +***********************************************/ static bool is_1g(void) { gpio_direction_input(IMX_GPIO_NR(1, 12)); return !gpio_get_value(IMX_GPIO_NR(1, 12)); }
-static void ddr_init(void) +static bool is_2g(void) { - if (is_1g()) - ddrc_regs_val.addrmap6 = 0x0f070707; + gpio_direction_input(IMX_GPIO_NR(1, 13)); + return gpio_get_value(IMX_GPIO_NR(1, 13)); +}
+static void ddr_init(void) +{ + if (is_1g()) { + if (is_2g()) { + ddrc_regs_val.addrmap0 = 0x0000001f; + ddrc_regs_val.addrmap1 = 0x00181818; + ddrc_regs_val.addrmap4 = 0x00000f0f; + ddrc_regs_val.addrmap5 = 0x04040404; + ddrc_regs_val.addrmap6 = 0x04040404; + } else + ddrc_regs_val.addrmap6 = 0x0f070707; + } mx7_dram_cfg(&ddrc_regs_val, &ddrc_mp_val, &ddr_phy_regs_val, &calib_param); }

From: Benjamin Szőke egyszeregy@freemail.hu
Take over codes from Techenxion to support mmc autodetect boot for pico-imx7d.
Signed-off-by: Benjamin Szőke egyszeregy@freemail.hu --- board/technexion/pico-imx7d/pico-imx7d.c | 82 +++++++++++++++++ board/technexion/pico-imx7d/pico-imx7d_spl.c | 92 ++++++++++++++++++-- include/configs/pico-imx7d.h | 4 +- 3 files changed, 170 insertions(+), 8 deletions(-)
diff --git a/board/technexion/pico-imx7d/pico-imx7d.c b/board/technexion/pico-imx7d/pico-imx7d.c index 7db34abcb1..7e47f15e71 100644 --- a/board/technexion/pico-imx7d/pico-imx7d.c +++ b/board/technexion/pico-imx7d/pico-imx7d.c @@ -21,6 +21,8 @@ #include <power/pmic.h> #include <power/pfuze3000_pmic.h> #include "../../freescale/common/pfuze.h" +#include <mmc.h> +#include <asm/mach-imx/boot_mode.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -159,6 +161,53 @@ int board_phy_config(struct phy_device *phydev) } #endif
+#if CONFIG_IS_ENABLED(FSL_ESDHC_IMX) +int check_mmc_autodetect(void) +{ + char *autodetect_str = env_get("mmcautodetect"); + + if ((autodetect_str != NULL) && + (strcmp(autodetect_str, "yes") == 0)) { + return 1; + } + + return 0; +} + +void board_late_mmc_init(void) +{ + int dev_no = 0; + char cmd[32]; + char mmcblk[32]; + + if (!check_mmc_autodetect()) + return; + + switch (get_boot_device()) { + case SD3_BOOT: + case MMC3_BOOT: + env_set("bootdev", "MMC3"); + dev_no = 2; + break; + case SD1_BOOT: + env_set("bootdev", "SD1"); + dev_no = 0; + break; + default: + printf("Wrong boot device!"); + } + + env_set_ulong("mmcdev", dev_no); + + /* Set mmcblk env */ + sprintf(mmcblk, "/dev/mmcblk%dp2 rootwait rw", dev_no); + env_set("mmcroot", mmcblk); + + sprintf(cmd, "mmc dev %d", dev_no); + run_command(cmd, 0); +} +#endif + static void setup_iomux_uart(void) { imx_iomux_v3_setup_multiple_pads(uart5_pads, ARRAY_SIZE(uart5_pads)); @@ -210,6 +259,10 @@ int board_late_init(void)
set_wdog_reset(wdog);
+#if defined(CONFIG_ENV_IS_IN_MMC) || defined(CONFIG_ENV_IS_NOWHERE) + board_late_mmc_init(); +#endif /* CONFIG_ENV_IS_IN_MMC or CONFIG_ENV_IS_NOWHERE */ + /* * Do not assert internal WDOG_RESET_B_DEB(controlled by bit 4), * since we use PMIC_PWRON to reset the board. @@ -219,6 +272,29 @@ int board_late_init(void) return 0; }
+#ifdef CONFIG_OF_BOARD_SETUP +int ft_board_setup(void *blob, struct bd_info *bd) +{ + const int *cell; + int offs; + uint32_t cma_size; + unsigned int dram_size; + + dram_size = imx_ddr_size() / 1024 / 1024; + offs = fdt_path_offset(blob, "/reserved-memory/linux,cma"); + cell = fdt_getprop(blob, offs, "size", NULL); + cma_size = fdt32_to_cpu(cell[0]); + if (dram_size == 512) { + /* CMA is aligned by 32MB on i.mx8mq, + so CMA size can only be multiple of 32MB */ + cma_size = env_get_ulong("cma_size", 10, (6 * 32) * 1024 * 1024); + fdt_setprop_u32(blob, offs, "size", (uint64_t)cma_size); + } + + return 0; +} +#endif + int checkboard(void) { puts("Board: i.MX7D PICOSOM\n"); @@ -244,3 +320,9 @@ int board_ehci_hcd_init(int port) } return 0; } + +/* This should be defined for each board */ +__weak int mmc_map_to_kernel_blk(int dev_no) +{ + return dev_no; +} diff --git a/board/technexion/pico-imx7d/pico-imx7d_spl.c b/board/technexion/pico-imx7d/pico-imx7d_spl.c index 0009c55022..499e4a49e0 100644 --- a/board/technexion/pico-imx7d/pico-imx7d_spl.c +++ b/board/technexion/pico-imx7d/pico-imx7d_spl.c @@ -18,6 +18,8 @@ #include <asm/gpio.h> #include <fsl_esdhc_imx.h> #include <spl.h> +#include <command.h> +#include <asm/mach-imx/boot_mode.h>
#if defined(CONFIG_SPL_BUILD)
@@ -158,7 +160,20 @@ void reset_cpu(void) #define USDHC_PAD_CTRL (PAD_CTL_DSE_3P3V_32OHM | PAD_CTL_SRE_SLOW | \ PAD_CTL_HYS | PAD_CTL_PUE | PAD_CTL_PUS_PU47KOHM)
-static iomux_v3_cfg_t const usdhc3_pads[] = { +#define USDHC1_CD_GPIO IMX_GPIO_NR(5, 0) +/* EMMC/SD */ +static iomux_v3_cfg_t const usdhc1_pads[] = { + MX7D_PAD_SD1_CLK__SD1_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX7D_PAD_SD1_CMD__SD1_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX7D_PAD_SD1_DATA0__SD1_DATA0 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX7D_PAD_SD1_DATA1__SD1_DATA1 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX7D_PAD_SD1_DATA2__SD1_DATA2 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX7D_PAD_SD1_DATA3__SD1_DATA3 | MUX_PAD_CTRL(USDHC_PAD_CTRL), + MX7D_PAD_SD1_CD_B__GPIO5_IO0 | MUX_PAD_CTRL(USDHC_PAD_CTRL), +}; + +#define USDHC3_CD_GPIO IMX_GPIO_NR(1, 14) +static iomux_v3_cfg_t const usdhc3_emmc_pads[] = { MX7D_PAD_SD3_CLK__SD3_CLK | MUX_PAD_CTRL(USDHC_PAD_CTRL), MX7D_PAD_SD3_CMD__SD3_CMD | MUX_PAD_CTRL(USDHC_PAD_CTRL), MX7D_PAD_SD3_DATA0__SD3_DATA0 | MUX_PAD_CTRL(USDHC_PAD_CTRL), @@ -172,20 +187,83 @@ static iomux_v3_cfg_t const usdhc3_pads[] = { MX7D_PAD_GPIO1_IO14__GPIO1_IO14 | MUX_PAD_CTRL(USDHC_PAD_CTRL), };
-static struct fsl_esdhc_cfg usdhc_cfg[1] = { +static struct fsl_esdhc_cfg usdhc_cfg[2] = { {USDHC3_BASE_ADDR}, + {USDHC1_BASE_ADDR}, };
int board_mmc_getcd(struct mmc *mmc) { - /* Assume uSDHC3 emmc is always present */ - return 1; + struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv; + int ret = 0; + + switch (cfg->esdhc_base) { + case USDHC1_BASE_ADDR: + ret = !gpio_get_value(USDHC1_CD_GPIO); /* Assume uSDHC1 sd is always present */ + break; + case USDHC3_BASE_ADDR: + ret = !gpio_get_value(USDHC3_CD_GPIO); /* Assume uSDHC3 emmc is always present */ + break; + } + + return ret; }
int board_mmc_init(struct bd_info *bis) { - imx_iomux_v3_setup_multiple_pads(usdhc3_pads, ARRAY_SIZE(usdhc3_pads)); - usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); - return fsl_esdhc_initialize(bis, &usdhc_cfg[0]); + int ret; + u32 index = 0; + + /* + * Following map is done: + * (USDHC) (Physical Port) + * usdhc3 SOM MicroSD/MMC + * usdhc1 Carrier board MicroSD + * Always set boot USDHC as mmc0 + */ + + imx_iomux_v3_setup_multiple_pads( + usdhc3_emmc_pads, ARRAY_SIZE(usdhc3_emmc_pads)); + gpio_direction_input(USDHC3_CD_GPIO); + + imx_iomux_v3_setup_multiple_pads( + usdhc1_pads, ARRAY_SIZE(usdhc1_pads)); + gpio_direction_input(USDHC1_CD_GPIO); + + switch (get_boot_device()) { + case SD1_BOOT: + usdhc_cfg[0].esdhc_base = USDHC1_BASE_ADDR; + usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK); + usdhc_cfg[0].max_bus_width = 4; + usdhc_cfg[1].esdhc_base = USDHC3_BASE_ADDR; + usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); + usdhc_cfg[1].max_bus_width = 4; + break; + case MMC3_BOOT: + usdhc_cfg[0].esdhc_base = USDHC3_BASE_ADDR; + usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); + usdhc_cfg[0].max_bus_width = 8; + usdhc_cfg[1].esdhc_base = USDHC1_BASE_ADDR; + usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK); + usdhc_cfg[1].max_bus_width = 4; + break; + case SD3_BOOT: + default: + usdhc_cfg[0].esdhc_base = USDHC3_BASE_ADDR; + usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); + usdhc_cfg[0].max_bus_width = 4; + usdhc_cfg[1].esdhc_base = USDHC1_BASE_ADDR; + usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK); + usdhc_cfg[1].max_bus_width = 4; + break; + } + + for (index = 0; index < CONFIG_SYS_FSL_USDHC_NUM; ++index) { + ret = fsl_esdhc_initialize(bis, &usdhc_cfg[index]); + if (ret) + return ret; + } + + return 0; } #endif diff --git a/include/configs/pico-imx7d.h b/include/configs/pico-imx7d.h index 159bf4c68c..3a647cde97 100644 --- a/include/configs/pico-imx7d.h +++ b/include/configs/pico-imx7d.h @@ -21,7 +21,7 @@ #define CONFIG_MXC_UART_BASE UART5_IPS_BASE_ADDR
/* MMC Config */ -#define CFG_SYS_FSL_ESDHC_ADDR 0 +#define CFG_SYS_FSL_ESDHC_ADDR USDHC3_BASE_ADDR
#define CONFIG_DFU_ENV_SETTINGS \ "dfu_alt_info=" \ @@ -87,9 +87,11 @@ "name=rootfs,size=0,uuid=${uuid_gpt_rootfs}\0" \ "fastboot_partition_alias_system=rootfs\0" \ "setup_emmc=mmc dev 0; gpt write mmc 0 $partitions; reset;\0" \ + "mmcautodetect=yes\0" \ PICO_BOOT_ENV
#define BOOT_TARGET_DEVICES(func) \ + func(MMC, mmc, 3) \ func(MMC, mmc, 0) \ func(USB, usb, 0) \ func(PXE, pxe, na) \

On Sun, Dec 18, 2022 at 9:53 AM egyszeregy@freemail.hu wrote:
for (index = 0; index < CONFIG_SYS_FSL_USDHC_NUM; ++index) {
ret = fsl_esdhc_initialize(bis, &usdhc_cfg[index]);
if (ret)
return ret;
In patch 3/3, CONFIG_SPL_DM_MMC=y, so can't the driver do the MMC init instead of doing this manually?
#define BOOT_TARGET_DEVICES(func) \
func(MMC, mmc, 3) \
Shouldn't the index 2 instead?

To use fsl_esdhc_initialize() is must or esdhc drivers object will be not initialized. It was made by Technexion.In practice, it turned out that 3 is good.

From: Benjamin Szőke egyszeregy@freemail.hu
Update defconfigs of pico-imx7d baseboards. In freescale community default CONFIG_SPL_FS_LOAD_PAYLOAD_NAME is "u-boot-dtb.img".
Signed-off-by: Benjamin Szőke egyszeregy@freemail.hu --- configs/pico-dwarf-imx7d_defconfig | 6 ++++++ configs/pico-hobbit-imx7d_defconfig | 6 ++++++ configs/pico-imx7d_bl33_defconfig | 6 ++++++ configs/pico-imx7d_defconfig | 6 ++++++ configs/pico-nymph-imx7d_defconfig | 6 ++++++ configs/pico-pi-imx7d_defconfig | 6 ++++++ 6 files changed, 36 insertions(+)
diff --git a/configs/pico-dwarf-imx7d_defconfig b/configs/pico-dwarf-imx7d_defconfig index 821357827a..9e82380419 100644 --- a/configs/pico-dwarf-imx7d_defconfig +++ b/configs/pico-dwarf-imx7d_defconfig @@ -15,8 +15,14 @@ CONFIG_DM_GPIO=y CONFIG_DEFAULT_DEVICE_TREE="imx7d-pico-pi" CONFIG_TARGET_PICO_IMX7D=y CONFIG_SPL_MMC=y +CONFIG_SPL_DM_MMC=y +CONFIG_SPL_MMC_BOOT=y +CONFIG_MMC_VERBOSE=y CONFIG_SPL_SERIAL=y CONFIG_SPL=y +CONFIG_SUPPORT_SPL=y +CONFIG_SPL_FS_FAT=y +CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot-dtb.img" CONFIG_ARMV7_BOOT_SEC_DEFAULT=y CONFIG_IMX_RDC=y CONFIG_IMX_BOOTAUX=y diff --git a/configs/pico-hobbit-imx7d_defconfig b/configs/pico-hobbit-imx7d_defconfig index 759866ce11..f896352cad 100644 --- a/configs/pico-hobbit-imx7d_defconfig +++ b/configs/pico-hobbit-imx7d_defconfig @@ -15,8 +15,14 @@ CONFIG_DM_GPIO=y CONFIG_DEFAULT_DEVICE_TREE="imx7d-pico-pi" CONFIG_TARGET_PICO_IMX7D=y CONFIG_SPL_MMC=y +CONFIG_SPL_DM_MMC=y +CONFIG_SPL_MMC_BOOT=y +CONFIG_MMC_VERBOSE=y CONFIG_SPL_SERIAL=y CONFIG_SPL=y +CONFIG_SUPPORT_SPL=y +CONFIG_SPL_FS_FAT=y +CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot-dtb.img" CONFIG_ARMV7_BOOT_SEC_DEFAULT=y CONFIG_IMX_RDC=y CONFIG_IMX_BOOTAUX=y diff --git a/configs/pico-imx7d_bl33_defconfig b/configs/pico-imx7d_bl33_defconfig index 8631f81f33..a7f25a8d94 100644 --- a/configs/pico-imx7d_bl33_defconfig +++ b/configs/pico-imx7d_bl33_defconfig @@ -14,8 +14,14 @@ CONFIG_DM_GPIO=y CONFIG_DEFAULT_DEVICE_TREE="imx7d-pico-pi" CONFIG_TARGET_PICO_IMX7D=y CONFIG_SPL_MMC=y +CONFIG_SPL_DM_MMC=y +CONFIG_SPL_MMC_BOOT=y +CONFIG_MMC_VERBOSE=y CONFIG_SPL_SERIAL=y CONFIG_SPL=y +CONFIG_SUPPORT_SPL=y +CONFIG_SPL_FS_FAT=y +CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot-dtb.img" CONFIG_ARMV7_BOOT_SEC_DEFAULT=y CONFIG_SYS_MEMTEST_START=0x80000000 CONFIG_SYS_MEMTEST_END=0xa0000000 diff --git a/configs/pico-imx7d_defconfig b/configs/pico-imx7d_defconfig index a84954d22f..7b3fb4c686 100644 --- a/configs/pico-imx7d_defconfig +++ b/configs/pico-imx7d_defconfig @@ -15,8 +15,14 @@ CONFIG_DM_GPIO=y CONFIG_DEFAULT_DEVICE_TREE="imx7d-pico-pi" CONFIG_TARGET_PICO_IMX7D=y CONFIG_SPL_MMC=y +CONFIG_SPL_DM_MMC=y +CONFIG_SPL_MMC_BOOT=y +CONFIG_MMC_VERBOSE=y CONFIG_SPL_SERIAL=y CONFIG_SPL=y +CONFIG_SUPPORT_SPL=y +CONFIG_SPL_FS_FAT=y +CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot-dtb.img" CONFIG_ARMV7_BOOT_SEC_DEFAULT=y CONFIG_IMX_RDC=y CONFIG_IMX_BOOTAUX=y diff --git a/configs/pico-nymph-imx7d_defconfig b/configs/pico-nymph-imx7d_defconfig index 821357827a..9e82380419 100644 --- a/configs/pico-nymph-imx7d_defconfig +++ b/configs/pico-nymph-imx7d_defconfig @@ -15,8 +15,14 @@ CONFIG_DM_GPIO=y CONFIG_DEFAULT_DEVICE_TREE="imx7d-pico-pi" CONFIG_TARGET_PICO_IMX7D=y CONFIG_SPL_MMC=y +CONFIG_SPL_DM_MMC=y +CONFIG_SPL_MMC_BOOT=y +CONFIG_MMC_VERBOSE=y CONFIG_SPL_SERIAL=y CONFIG_SPL=y +CONFIG_SUPPORT_SPL=y +CONFIG_SPL_FS_FAT=y +CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot-dtb.img" CONFIG_ARMV7_BOOT_SEC_DEFAULT=y CONFIG_IMX_RDC=y CONFIG_IMX_BOOTAUX=y diff --git a/configs/pico-pi-imx7d_defconfig b/configs/pico-pi-imx7d_defconfig index dec4280974..077fd3e71c 100644 --- a/configs/pico-pi-imx7d_defconfig +++ b/configs/pico-pi-imx7d_defconfig @@ -15,8 +15,14 @@ CONFIG_DM_GPIO=y CONFIG_DEFAULT_DEVICE_TREE="imx7d-pico-pi" CONFIG_TARGET_PICO_IMX7D=y CONFIG_SPL_MMC=y +CONFIG_SPL_DM_MMC=y +CONFIG_SPL_MMC_BOOT=y +CONFIG_MMC_VERBOSE=y CONFIG_SPL_SERIAL=y CONFIG_SPL=y +CONFIG_SUPPORT_SPL=y +CONFIG_SPL_FS_FAT=y +CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot-dtb.img" CONFIG_ARMV7_BOOT_SEC_DEFAULT=y CONFIG_IMX_RDC=y CONFIG_IMX_BOOTAUX=y

On Sun, Dec 18, 2022 at 9:52 AM egyszeregy@freemail.hu wrote:
+static void ddr_init(void) +{
if (is_1g()) {
if (is_2g()) {
This logic is confusing.
It would be better to read the two GPIOs and then do a switch case to handle the possible values.

All codes come from official u-boot codes from Technexion may they knows better why it is optimal in this form.https://github.com/TechNexion/u-boot-tn-imx/tree/tn-imx_v2021.04_5.10.72_2.2...

On Sun, Dec 18, 2022 at 9:52 AM egyszeregy@freemail.hu wrote:
From: Benjamin Szőke egyszeregy@freemail.hu
Take over codes from Techenxion to support SoMs with 2GB DDR3.
Signed-off-by: Benjamin Szőke egyszeregy@freemail.hu
board/technexion/pico-imx7d/Makefile | 2 +- .../pico-imx7d/{spl.c => pico-imx7d_spl.c} | 30 +++++++++++++++++--
Also, what is the purpose of this file rename?
I don't think it is needed.

Technexion did that renaming, it is better to keep in synchronized.https://github.com/TechNexion/u-boot-tn-imx/tree/tn-imx_v2021.04_5.10.72_2.2...

On Mon, Dec 19, 2022 at 9:12 PM Szőke Kálmán Benjamin egyszeregy@freemail.hu wrote:
Technexion did that renaming, it is better to keep in synchronized. https://github.com/TechNexion/u-boot-tn-imx/tree/tn-imx_v2021.04_5.10.72_2.2...
Sorry, that's not a good justification. Just keep the original file name.

I will not keep, most of Technexion SoM codes are broken or limited in functionalities (like 2GB RAM support missing, dual boot missing), because there is no any active maintenancing and as i see nobody cares about it from freescale community.What is the point of renaming and rewriting these codes, which they have already made for themselves once and work well, now? My opinion is, need to migrate only these codes, and do not need to change.

Hi Benjamin,
On Sun, Dec 18, 2022 at 9:52 AM egyszeregy@freemail.hu wrote:
From: Benjamin Szőke egyszeregy@freemail.hu
Take over codes from Techenxion to support SoMs with 2GB DDR3.
Signed-off-by: Benjamin Szőke egyszeregy@freemail.hu
board/technexion/pico-imx7d/Makefile | 2 +- .../pico-imx7d/{spl.c => pico-imx7d_spl.c} | 30 +++++++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) rename board/technexion/pico-imx7d/{spl.c => pico-imx7d_spl.c} (83%)
Could you please test whether the attached patch works for you?
I don't have a 2GB board variant here to test it myself.
You also need to apply the following patch to fix the boot against U-Boot top of tree: https://lore.kernel.org/u-boot/20221231162514.334303-1-festevam@denx.de/T/#u

Original code was a nested if statement for checking 1g() and 2g().https://github.com/TechNexion/u-boot-tn-imx/blob/tn-imx_v2021.04_5.10.72_2.2... you have any information about the PCB layout of the different i.MX7 pico SoM revisions?Question is, what is the electrical states of GPIO1_12 and GPIO1_13 in real for these variants? My fear is that maybe one of them is not connected to any VDD (high) or GND (low). Question can be, what is the default logical state for it high or low if one of them is not connected neither to GND or VDD? I totaly do not understand why you need to completly refactoring the code if you have no posibbilites to test it with all revision of SoMs. I rather prefere to keep the original if statement code from Technexion, because that should works 100% and tested. I have no any 2GB varians SoMs yet, also i can not test it yet.So i am prefering more to apply my last (copy pasted from Technexion) patch, to support 2GB SoMs in u-boot mainline.https://lists.denx.de/pipermail/u-boot/2022-December/502465.html-------- Eredeti levél --------Feladó: Fabio Estevam festevam@gmail.comDátum: 2022 december 31 17:31:01Tárgy: Re: [u-boot][master][PATCH 1/3] pico-imx7d: add support for 2GB memory SoMsCímzett: egyszeregy@freemail.huHi Benjamin, On Sun, Dec 18, 2022 at 9:52 AM egyszeregy@freemail.hu wrote: > > From: Benjamin Szőke egyszeregy@freemail.hu > > Take over codes from Techenxion to support SoMs with 2GB DDR3. > > Signed-off-by: Benjamin Szőke egyszeregy@freemail.hu > --- > board/technexion/pico-imx7d/Makefile | 2 +- > .../pico-imx7d/{spl.c => pico-imx7d_spl.c} | 30 +++++++++++++++++-- > 2 files changed, 28 insertions(+), 4 deletions(-) > rename board/technexion/pico-imx7d/{spl.c => pico-imx7d_spl.c} (83%) Could you please test whether the attached patch works for you? I don't have a 2GB board variant here to test it myself. You also need to apply the following patch to fix the boot against U-Boot top of tree: https://lore.kernel.org/u-boot/20221231162514.334303-1-festevam@denx.de/T/#u

Hi Benjamin,
On Sat, Jan 7, 2023 at 8:22 AM Szőke Kálmán Benjamin egyszeregy@freemail.hu wrote:
Original code was a nested if statement for checking 1g() and 2g(). https://github.com/TechNexion/u-boot-tn-imx/blob/tn-imx_v2021.04_5.10.72_2.2...
Yes, I saw that, but it doesn't look good. We can't blindly take the vendor code as-is and throw it to mainline.
When I added the is_1g() function there was only the 512MB or 1GB variants, so the function name was meaningful.
After adding the 2GB variant, the is_1g() function no longer means that the board has 1GB of RAM.
If you want to keep TechNexion logic, at least rename is_1g() to is_gpio1_12_low() and is_2g() to is_gpio1_13_high().
Do you have any information about the PCB layout of the different i.MX7 pico SoM revisions?Question is, what is the electrical states of GPIO1_12 and GPIO1_13 in real for these variants? My fear is that maybe one of them is not connected to any VDD (high) or GND (low). Question can be, what is the default logical state for it high or low if one of them is not connected neither to GND or VDD?
I totaly do not understand why you need to completly refactoring the code if you have no posibbilites to test it with all revision of SoMs. I rather prefere to keep the original if statement code from Technexion, because that should works 100% and tested. I have no any 2GB varians SoMs yet, also i can not test it yet.
The code below really bothers me:
static void ddr_init(void) { if (is_1g()) { if (is_2g()) {
"If the board has 1GB of RAM, then check if it has 2GB of RAM"
If you want to replace it with is_gpio1_12_low() and is_gpio1_13_high(), as suggested above, I would accept it.
Also, you submitted the patch without even testing it. That's not good.
The pico-imx7d board was not even booting at all. I fixed it recently by:
https://source.denx.de/u-boot/u-boot/-/commit/f8548ce0e09385926574283b17af6c...
and now you say that you also don't have the 2GB variant.
Please make sure to test your patches against U-Boot mainline.

My last three patches were tested in my company's custom i.MX7D pico SoMs carrier board, it was workd well with 512MB and 1 GB SoM variants in U-boot v2022.10. I have no any 2GB variants but i can trust in Technexion that it was tested in their house, bedore.But it can be better if you take contact with them and request a confirmation about your swtich case logic, is it will be fine or not for them.https://github.com/richard-huhttps://github.com/JoeZhang-tn@Tom RiniIs there any plan to improve the patches, pull-requests, technical review/threads processes to use a better web-technnolgies instead of to use infinity long-long ping-pong mailing in a legacy mailing lists? In 2023, It's time to use something better, for example to use a normal pull request features on GitHub/GitLab website for U-boot as every modern open-source project use it. It could be more traceabla and faster in ask a question for a developer, moreover the code reviewing is much much better in this modern way.https://github.com/u-boot/u-boot/pulls-------- Eredeti levél --------Feladó: Fabio Estevam festevam@gmail.comDátum: 2023 január 7 14:37:47Tárgy: Re: [u-boot][master][PATCH 1/3] pico-imx7d: add support for 2GB memory SoMsCímzett: Szőke Kálmán Benjamin egyszeregy@freemail.huHi Benjamin, On Sat, Jan 7, 2023 at 8:22 AM Szőke Kálmán Benjamin egyszeregy@freemail.hu wrote: > > Original code was a nested if statement for checking 1g() and 2g(). > https://github.com/TechNexion/u-boot-tn-imx/blob/tn-imx_v2021.04_5.10.72_2.2... Yes, I saw that, but it doesn't look good. We can't blindly take the vendor code as-is and throw it to mainline. When I added the is_1g() function there was only the 512MB or 1GB variants, so the function name was meaningful. After adding the 2GB variant, the is_1g() function no longer means that the board has 1GB of RAM. If you want to keep TechNexion logic, at least rename is_1g() to is_gpio1_12_low() and is_2g() to is_gpio1_13_high(). > Do you have any information about the PCB layout of the different i.MX7 pico SoM revisions?Question is, what is the electrical states of GPIO1_12 and GPIO1_13 in real for these variants? My fear is that maybe one of them is not connected to any VDD (high) or GND (low). Question can be, what is the default logical state for it high or low if one of them is not connected neither to GND or VDD? > > I totaly do not understand why you need to completly refactoring the code if you have no posibbilites to test it with all revision of SoMs. I rather prefere to keep the original if statement code from Technexion, because that should works 100% and tested. I have no any 2GB varians SoMs yet, also i can not test it yet. The code below really bothers me: static void ddr_init(void) { if (is_1g()) { if (is_2g()) { "If the board has 1GB of RAM, then check if it has 2GB of RAM" If you want to replace it with is_gpio1_12_low() and is_gpio1_13_high(), as suggested above, I would accept it. Also, you submitted the patch without even testing it. That's not good. The pico-imx7d board was not even booting at all. I fixed it recently by: https://source.denx.de/u-boot/u-boot/-/commit/f8548ce0e09385926574283b17af6c... and now you say that you also don't have the 2GB variant. Please make sure to test your patches against U-Boot mainline.

Hi Benjamin,
Please don't top post.
On Sat, Jan 7, 2023 at 2:15 PM Szőke Kálmán Benjamin egyszeregy@freemail.hu wrote:
My last three patches were tested in my company's custom i.MX7D pico SoMs carrier board, it was workd well with 512MB and 1 GB SoM variants in U-boot v2022.10. I have no any 2GB variants but i can trust in Technexion that it was tested in their house, before.
pico-imx7d_defconfig does not boot in v2022.10 either.
2f96d4dd95f8 ("imx7s/d: synchronise device trees with linux") landed in v2022.10-rc4.
Anyway, please always generate and test the patches against top of tree U-Boot.

On Sat, Jan 07, 2023 at 04:53:16PM +0000, Szőke Kálmán Benjamin wrote:
[snip]
@Tom Rini Is there any plan to improve the patches, pull-requests, technical review/threads processes to use a better web-technnolgies instead of to use infinity long-long ping-pong mailing in a legacy mailing lists? In 2023, It's time to use something better, for example to use a normal pull request features on GitHub/GitLab website for U-boot as every modern open-source project use it. It could be more traceabla and faster in ask a question for a developer, moreover the code reviewing is much much better in this modern way.
If you have specific plans and resources you're going to be able to dedicate to such a project, I'd be happy to have your plan discussed on the custodians and board maintainer mailing lists. Otherwise, we should probably try and leverage some of the more recent posts / videos that try and explain and making emailing patches for the linux kernel be linked in our documentation as they apply here too.
participants (4)
-
egyszeregy@freemail.hu
-
Fabio Estevam
-
Szőke Kálmán Benjamin
-
Tom Rini