On Do, 2024-11-14 at 07:30 +0100, Yannic Moog wrote:
Hi Christoph,
On Wed, 2024-11-13 at 17:00 +0100, Christoph Stoidner wrote:
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
Cc: Mathieu Othacehe m.othacehe@gmail.com Cc: Christoph Stoidner c.stoidner@phytec.de Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam festevam@gmail.com Cc: "NXP i.MX U-Boot Team" uboot-imx@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Yannic Moog y.moog@phytec.de Cc: Primoz Fiser primoz.fiser@norik.com Cc: Andrej Picej andrej.picej@norik.com Cc: Wadim Egorov w.egorov@phytec.de
Changes in v2:
- encapsulate handling of feature flag VOLTAGE into own function
- move definition of enum phytec_imx93_ddr_eeprom_code into header
file
arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi | 19 +++ arch/arm/mach-imx/imx9/Kconfig | 2 + arch/arm/mach-imx/imx9/soc.c | 2 +- board/phytec/common/Kconfig | 8 ++ board/phytec/common/Makefile | 1 + board/phytec/common/imx93_som_detection.c | 111 ++++++++++++++++++ board/phytec/common/imx93_som_detection.h | 51 ++++++++ board/phytec/phycore_imx93/Kconfig | 28 +++++ board/phytec/phycore_imx93/MAINTAINERS | 5 +- board/phytec/phycore_imx93/phycore-imx93.c | 51 ++++++++ board/phytec/phycore_imx93/spl.c | 48 ++++++++ 11 files changed, 324 insertions(+), 2 deletions(-) create mode 100644 board/phytec/common/imx93_som_detection.c create mode 100644 board/phytec/common/imx93_som_detection.h
diff --git a/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi b/arch/arm/dts/imx93-phyboard-segin-u- boot.dtsi index 6897c91f4d..25c778bb07 100644 --- a/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi +++ b/arch/arm/dts/imx93-phyboard-segin-u-boot.dtsi @@ -305,4 +305,23 @@ }; }; };
+ eeprom@50 { + bootph-pre-ram; + bootph-some-ram; + compatible = "atmel,24c32"; + reg = <0x50>; + pagesize = <32>; + vcc-supply = <&buck4>; + };
+ eepromid@58 { + bootph-pre-ram; + bootph-some-ram; + compatible = "atmel,24c32"; + pagesize = <32>; + reg = <0x58>; + size = <32>; + vcc-supply = <&buck4>; + }; }; diff --git a/arch/arm/mach-imx/imx9/Kconfig b/arch/arm/mach- imx/imx9/Kconfig index 5c1054138f..2465e31d73 100644 --- a/arch/arm/mach-imx/imx9/Kconfig +++ b/arch/arm/mach-imx/imx9/Kconfig @@ -45,6 +45,8 @@ config TARGET_PHYCORE_IMX93 bool "phycore_imx93" select IMX93 select IMX9_LPDDR4X + select OF_BOARD_FIXUP + select OF_BOARD_SETUP endchoice diff --git a/arch/arm/mach-imx/imx9/soc.c b/arch/arm/mach- imx/imx9/soc.c index 7c28fa39e1..237354f507 100644 --- a/arch/arm/mach-imx/imx9/soc.c +++ b/arch/arm/mach-imx/imx9/soc.c @@ -628,7 +628,7 @@ static int low_drive_freq_update(void *blob) return 0; } -#ifdef CONFIG_OF_BOARD_FIXUP +#if defined(CONFIG_OF_BOARD_FIXUP) && !defined(CONFIG_TARGET_PHYCORE_IMX93)
Why do you do this?
It doesn't look right to me. When you have to modify soc.c to add your own board config, then either there is a problem with the file and it should be fixed properly here or there is a problem with your board and it should be fixed in your board code.
The function board_fix_fdt() is an u-boot callback, that can be implemented by the board-code. It enables the board-code to modify the Device Tree. The modified device tree is then subsequently used by U-Boot.
We need that in our board-code to enable or disable HS400 for the eMMC, depending of what module variant we detected from the information in the eeprom.
Unfortunately, NXP has already implemented that function in their SOC-code, so it is no more available for the board-code. From my opinion, the best solution would be to move board_fix_fdt() from soc.c to all imx9 board codes that need it. Then that function is again available for the board code.
My proposal: To avoid loosig the focus of that patchset we accept it now as it is here. After that, I will then take care to send another patchset to move board_fix_fdt() for all boards accordingly.
Do you agree with that?
#ifndef CONFIG_XPL_BUILD int board_fix_fdt(void *fdt) { diff --git a/board/phytec/common/Kconfig b/board/phytec/common/Kconfig index f394ace786..bc5511707a 100644 --- a/board/phytec/common/Kconfig +++ b/board/phytec/common/Kconfig @@ -19,6 +19,14 @@ config PHYTEC_IMX8M_SOM_DETECTION Support of I2C EEPROM based SoM detection. Supported for PHYTEC i.MX8MM/i.MX8MP boards +config PHYTEC_IMX93_SOM_DETECTION + bool "Support SoM detection for i.MX93 PHYTEC platforms" + depends on ARCH_IMX9 && PHYTEC_SOM_DETECTION + default y + help + Support of I2C EEPROM based SoM detection. Supported + for PHYTEC i.MX93 based boards
config PHYTEC_AM62_SOM_DETECTION bool "Support SoM detection for AM62x PHYTEC platforms" depends on (TARGET_PHYCORE_AM62X_A53 || TARGET_PHYCORE_AM62X_R5) && \ diff --git a/board/phytec/common/Makefile b/board/phytec/common/Makefile index cd78f7686f..8126f7356e 100644 --- a/board/phytec/common/Makefile +++ b/board/phytec/common/Makefile @@ -10,3 +10,4 @@ endif obj-y += phytec_som_detection.o phytec_som_detection_blocks.o obj-$(CONFIG_ARCH_K3) += am6_som_detection.o k3/ obj-$(CONFIG_ARCH_IMX8M) += imx8m_som_detection.o +obj-$(CONFIG_ARCH_IMX9) += imx93_som_detection.o diff --git a/board/phytec/common/imx93_som_detection.c b/board/phytec/common/imx93_som_detection.c new file mode 100644 index 0000000000..a28cdf6b51 --- /dev/null +++ b/board/phytec/common/imx93_som_detection.c @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Copyright (C) 2024 PHYTEC Messtechnik GmbH
- Author: Primoz Fiser primoz.fiser@norik.com
- */
+#include <asm/arch/sys_proto.h> +#include <dm/device.h> +#include <dm/uclass.h> +#include <i2c.h> +#include <u-boot/crc.h>
+#include "imx93_som_detection.h"
+extern struct phytec_eeprom_data eeprom_data;
+#if IS_ENABLED(CONFIG_PHYTEC_IMX93_SOM_DETECTION)
+/* Check if the SoM is actually one of the following products:
- i.MX93
- Returns 0 in case it's a known SoM. Otherwise, returns 1.
- */
+u8 __maybe_unused phytec_imx93_detect(struct phytec_eeprom_data *data) +{ + u8 som;
+ if (!data) + data = &eeprom_data;
+ /* Early API revisions are not supported */ + if (!data->valid || data->payload.api_rev < PHYTEC_API_REV2) + return 1;
+ som = data->payload.data.data_api2.som_no; + debug("%s: som id: %u\n", __func__, som);
+ if (som == PHYTEC_IMX93_SOM && is_imx93()) + return 0;
+ pr_err("%s: SoM ID does not match. Wrong EEPROM data?\n", __func__); + return 1; +}
+/*
- Filter PHYTEC i.MX93 SoM options by option index
- Returns:
- * - option value
- * - PHYTEC_EEPROM_INVAL when the data is invalid
- */
+u8 __maybe_unused phytec_imx93_get_opt(struct phytec_eeprom_data *data, + enum phytec_imx93_option_index idx) +{ + char *opt; + u8 opt_id;
+ if (!data) + data = &eeprom_data;
+ if (!data->valid || data->payload.api_rev < PHYTEC_API_REV2) + return PHYTEC_EEPROM_INVAL;
+ opt = phytec_get_opt(data); + if (opt) + opt_id = PHYTEC_GET_OPTION(opt[idx]); + else + opt_id = PHYTEC_EEPROM_INVAL;
+ debug("%s: opt[%d] id: %u\n", __func__, idx, opt_id); + return opt_id; +}
+/*
- Filter PHYTEC i.MX93 SoM voltage
- Returns:
- * - PHYTEC_IMX93_VOLTAGE_1V8 or PHYTEC_IMX93_VOLTAGE_3V8
- * - PHYTEC_EEPROM_INVAL when the data is invalid
- */
+enum phytec_imx93_voltage __maybe_unused phytec_imx93_get_voltage(struct phytec_eeprom_data *data) +{ + u8 option = phytec_imx93_get_opt(data, PHYTEC_IMX93_OPT_FEAT);
+ if (option == PHYTEC_EEPROM_INVAL) + return PHYTEC_IMX93_VOLTAGE_INVALID; + return (option & 0x01) ? PHYTEC_IMX93_VOLTAGE_1V8 : PHYTEC_IMX93_VOLTAGE_3V3; +}
+#else
+inline u8 __maybe_unused phytec_imx93_detect(struct phytec_eeprom_data *data) +{ + return 1; +}
+inline u8 __maybe_unused phytec_imx93_get_opt(struct phytec_eeprom_data *data, + enum phytec_imx93_option_index idx) +{ + return PHYTEC_EEPROM_INVAL; +}
+inline enum phytec_imx93_voltage __maybe_unused phytec_imx93_get_voltage + (struct phytec_eeprom_data *data) +{ + return PHYTEC_EEPROM_INVAL; +}
+#endif /* IS_ENABLED(CONFIG_PHYTEC_IMX93_SOM_DETECTION) */ diff --git a/board/phytec/common/imx93_som_detection.h b/board/phytec/common/imx93_som_detection.h new file mode 100644 index 0000000000..6d3d9283d8 --- /dev/null +++ b/board/phytec/common/imx93_som_detection.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/*
- Copyright (C) 2024 PHYTEC Messtechnik GmbH
- Author: Primoz Fiser primoz.fiser@norik.com
- */
+#ifndef _PHYTEC_IMX93_SOM_DETECTION_H +#define _PHYTEC_IMX93_SOM_DETECTION_H
+#include "phytec_som_detection.h"
+#define PHYTEC_IMX93_SOM 77
+enum phytec_imx93_option_index { + PHYTEC_IMX93_OPT_DDR = 0, + PHYTEC_IMX93_OPT_EMMC, + PHYTEC_IMX93_OPT_CPU, + PHYTEC_IMX93_OPT_FREQ, + PHYTEC_IMX93_OPT_NPU, + PHYTEC_IMX93_OPT_DISP, + PHYTEC_IMX93_OPT_ETH, + PHYTEC_IMX93_OPT_FEAT, + PHYTEC_IMX93_OPT_TEMP, + PHYTEC_IMX93_OPT_BOOT, + PHYTEC_IMX93_OPT_LED, + PHYTEC_IMX93_OPT_EEPROM, +};
+enum phytec_imx93_voltage { + PHYTEC_IMX93_VOLTAGE_INVALID = PHYTEC_EEPROM_INVAL, + PHYTEC_IMX93_VOLTAGE_3V3 = 0, + PHYTEC_IMX93_VOLTAGE_1V8, +};
+enum phytec_imx93_ddr_eeprom_code { + PHYTEC_IMX93_DDR_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, +};
This is inconsistent within enums. For the ddr, you set each value explicitly, but for the others you don't. Please choose one style.
Yannic
[...]