Re: [Upstream] [PATCH 2/3] board: phytec: imx93: Add eeprom-based hardware introspection

[Wadim Egorov <w.egorov@phytec.de>]

Hi Christoph,

Am 04.11.24 um 11:41 schrieb Christoph Stoidner:
> The phyCORE-i.MX 93 is available in various variants. Relevant variant
> options for the spl/u-boot are:
> - with or without HS400 support for the eMMC
> - with 1GB ram chip, or 2GB ram chip
> 
> The phyCORE's eeprom contains all information about the existing variant
> options. Add evaluation of the eeprom data to the spl/u-boot to
> enable/disable HS400 and to select the appropriate ram configuration at
> startup.
> 
> Signed-off-by: Christoph Stoidner <c.stoidner@phytec.de>

<snip>

> --- a/board/phytec/phycore_imx93/phycore-imx93.c
> +++ b/board/phytec/phycore_imx93/phycore-imx93.c
> @@ -3,6 +3,7 @@
>    * Copyright (C) 2023 PHYTEC Messtechnik GmbH
>    * Author: Christoph Stoidner <c.stoidner@phytec.de>
>    * Copyright (C) 2024 Mathieu Othacehe <m.othacehe@gmail.com>
> + * Copyright (C) 2024 PHYTEC Messtechnik GmbH
>    */
>   
>   #include <asm/arch-imx9/ccm_regs.h>
> @@ -12,11 +13,21 @@
>   #include <asm/global_data.h>
>   #include <asm/mach-imx/boot_mode.h>
>   #include <env.h>
> +#include <fdt_support.h>
> +
> +#include "../common/imx93_som_detection.h"
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> +#define EEPROM_ADDR            0x50
> +
>   int board_init(void)
>   {
> +	int ret = phytec_eeprom_data_setup(NULL, 2, EEPROM_ADDR);
> +
> +	if (ret)
> +		printf("%s: EEPROM data init failed\n", __func__);
> +
>   	return 0;
>   }
>   
> @@ -40,3 +51,44 @@ int board_late_init(void)
>   
>   	return 0;
>   }
> +
> +static void emmc_fixup(void *blob, struct phytec_eeprom_data *data)
> +{
> +	u8 option = phytec_imx93_get_opt(data, PHYTEC_IMX93_OPT_FEAT);

I am thinking about future SoMs using similar variants.



> +	int offset;
> +
> +	if (option == PHYTEC_EEPROM_INVAL)
> +		goto err;
> +
> +	/* Check "IO Voltage 1v8" flag is set */
> +	if (option & 0x01) {
If you abstract away the option details into an own function, e.g. 
phytec_get_imx93_io_volt(), you could reduce the amount of code in your 
board files. Also, if SoMs change the option layouts for a specific 
detail (for whatever reason), you would have an abstraction layer to 
details with it.


> +		offset = fdt_node_offset_by_compat_reg(blob, "fsl,imx93-usdhc",
> +						       0x42850000);
> +		if (offset)
> +			fdt_delprop(blob, offset, "no-1-8-v");
> +		else
> +			goto err;
> +	}
> +
> +	return;
> +err:
> +	printf("Could not detect eMMC VDD-IO. Fall back to default.\n");
> +}
> +
> +int board_fix_fdt(void *blob)
> +{
> +	struct phytec_eeprom_data data;
> +
> +	phytec_eeprom_data_setup(&data, 2, EEPROM_ADDR);
> +
> +	emmc_fixup(blob, &data);
> +
> +	return 0;
> +}
> +
> +int ft_board_setup(void *blob, struct bd_info *bd)
> +{
> +	emmc_fixup(blob, NULL);
> +
> +	return 0;
> +}
> diff --git a/board/phytec/phycore_imx93/spl.c b/board/phytec/phycore_imx93/spl.c
> index 17a8736c73..cf9c94eb45 100644
> --- a/board/phytec/phycore_imx93/spl.c
> +++ b/board/phytec/phycore_imx93/spl.c
> @@ -3,6 +3,7 @@
>    * Copyright (C) 2023 PHYTEC Messtechnik GmbH
>    * Author: Christoph Stoidner <c.stoidner@phytec.de>
>    * Copyright (C) 2024 Mathieu Othacehe <m.othacehe@gmail.com>
> + * Copyright (C) 2024 PHYTEC Messtechnik GmbH
>    */
>   
>   #include <asm/arch/clock.h>
> @@ -20,6 +21,8 @@
>   #include <power/pca9450.h>
>   #include <spl.h>
>   
> +#include "../common/imx93_som_detection.h"
> +
>   DECLARE_GLOBAL_DATA_PTR;
>   
>   /*
> @@ -27,6 +30,13 @@ DECLARE_GLOBAL_DATA_PTR;
>    * when pca9451a support is added.
>    */
>   #define PCA9450_REG_PWRCTRL_TOFF_DEB    BIT(5)
> +#define EEPROM_ADDR            0x50
> +
> +/*
> + * Prototypes of automatically generated ram config file
> + */
> +void set_dram_timings_2gb_lpddr4x(void);
> +void set_dram_timings_1gb_lpddr4x_900mhz(void);
>   
>   int spl_board_boot_device(enum boot_device boot_dev_spl)
>   {
> @@ -44,8 +54,56 @@ void spl_board_init(void)
>   	puts("Normal Boot\n");
>   }
>   
> +enum phytec_imx93_ddr_eeprom_code {
> +	INVALID = PHYTEC_EEPROM_INVAL,
> +	PHYTEC_IMX93_LPDDR4X_512MB = 0,
> +	PHYTEC_IMX93_LPDDR4X_1GB = 1,
> +	PHYTEC_IMX93_LPDDR4X_2GB = 2,
> +	PHYTEC_IMX93_LPDDR4_512MB = 3,
> +	PHYTEC_IMX93_LPDDR4_1GB = 4,
> +	PHYTEC_IMX93_LPDDR4_2GB = 5,
> +};
^ IMO this belongs into imx93_som_detection.h file

Regards,
Wadim
> +
>   void spl_dram_init(void)
>   {
> +	int ret;
> +	enum phytec_imx93_ddr_eeprom_code ddr_opt = INVALID;
> +
> +	/* NOTE: In SPL lpi2c3 is mapped to bus 0 */
> +	ret = phytec_eeprom_data_setup(NULL, 0, EEPROM_ADDR);
> +	if (ret && !IS_ENABLED(CONFIG_PHYCORE_IMX93_RAM_TYPE_FIX))
> +		goto out;
> +
> +	ret = phytec_imx93_detect(NULL);
> +	if (!ret)
> +		phytec_print_som_info(NULL);
> +
> +	if (IS_ENABLED(CONFIG_PHYCORE_IMX93_RAM_TYPE_FIX)) {
> +		if (IS_ENABLED(CONFIG_PHYCORE_IMX93_RAM_TYPE_LPDDR4X_1GB))
> +			ddr_opt = PHYTEC_IMX93_LPDDR4X_1GB;
> +		else if (IS_ENABLED(CONFIG_PHYCORE_IMX93_RAM_TYPE_LPDDR4X_2GB))
> +			ddr_opt = PHYTEC_IMX93_LPDDR4X_2GB;
> +	} else {
> +		ddr_opt = phytec_imx93_get_opt(NULL, PHYTEC_IMX93_OPT_DDR);
> +	}
> +
> +	switch (ddr_opt) {
> +	case PHYTEC_IMX93_LPDDR4X_1GB:
> +		if (is_voltage_mode(VOLT_LOW_DRIVE))
> +			set_dram_timings_1gb_lpddr4x_900mhz();
> +		break;
> +	case PHYTEC_IMX93_LPDDR4X_2GB:
> +		set_dram_timings_2gb_lpddr4x();
> +		break;
> +	default:
> +		goto out;
> +	}
> +	ddr_init(&dram_timing);
> +	return;
> +out:
> +	puts("Could not detect correct RAM type and size. Fall back to default.\n");
> +	if (is_voltage_mode(VOLT_LOW_DRIVE))
> +		set_dram_timings_1gb_lpddr4x_900mhz();
>   	ddr_init(&dram_timing);
>   }
>