[PATCH v3 3/3] Add imx93-var-som support

Fabio Estevam festevam at gmail.com
Wed Jan 3 20:31:34 CET 2024


Hi Mathieu,

On Fri, Dec 29, 2023 at 8:17 AM Mathieu Othacehe <othacehe at gnu.org> wrote:
>
> From: Mathieu Othacehe <m.othacehe at gmail.com>
>
> Add support for the Variscite VAR-SOM-IMX93 evaluation kit. The SoM
> consists of an NXP iMX93 dual A55 CPU. The SoM is mounted on a Variscite
> Symphony SBC.
>
> Signed-off-by: Mathieu Othacehe <m.othacehe at gmail.com>

I applied this patch against U-Boot's next branch.

I had to manually add:

--- a/board/variscite/imx93_var_som/spl.c
+++ b/board/variscite/imx93_var_som/spl.c
@@ -21,6 +21,7 @@
 #include <asm/mach-imx/mxc_i2c.h>
 #include <asm/arch-mx7ulp/gpio.h>
 #include <asm/mach-imx/syscounter.h>
+#include <asm/sections.h>
 #include <dm/uclass.h>
 #include <dm/device.h>
 #include <dm/uclass-internal.h>

so that it can be built.

Also noticed some checkpatch warnings:

ERROR: Do not add common.h to files
#802: FILE: board/variscite/common/eth.c:5:
+#include <common.h>

ERROR: Do not add common.h to files
#886: FILE: board/variscite/common/imx9_eeprom.c:6:
+#include <common.h>

ERROR: Do not add common.h to files
#1172: FILE: board/variscite/common/mmc.c:7:
+#include <common.h>

ERROR: Do not add common.h to files
#1296: FILE: board/variscite/imx93_var_som/imx93_var_som.c:7:
+#include <common.h>

ERROR: Do not add common.h to files
#3050: FILE: board/variscite/imx93_var_som/spl.c:7:
+#include <common.h>

and also got a warning when applying it:

$ git am ~/Downloads/Add-imx93-var-som-support.patch
Applying: spl: binman: Disable u_boot_any symbols for i.MX93 boards
Applying: mach-imx: Add i.MX93 binman support.
Applying: Add imx93-var-som support
.git/rebase-apply/patch:3025: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

> --- /dev/null
> +++ b/arch/arm/dts/imx93-var-som-symphony-u-boot.dtsi
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2022 NXP
> + * Copyright 2023 Variscite Ltd.
> + */
> +
> +#include "imx93-u-boot.dtsi"

This file should be included by default. No need to manually include it.

> diff --git a/board/variscite/common/eth.c b/board/variscite/common/eth.c
> new file mode 100644
> index 00000000000..9e05dee51c2
> --- /dev/null
> +++ b/board/variscite/common/eth.c
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2023 Variscite Ltd.
> + */
> +#include <common.h>
> +#include <net.h>
> +#include <miiphy.h>
> +#include <env.h>
> +#include "../common/imx9_eeprom.h"
> +
> +#define CHAR_BIT 8
> +
> +static u64 mac2int(const u8 hwaddr[])
> +{
> +       s8 i;

Why does i need to be signed?

> +       u64 ret = 0;
> +       const u8 *p = hwaddr;
> +
> +       for (i = 5; i >= 0; i--)
> +               ret |= (u64)*p++ << (CHAR_BIT * i);
> +
> +       return ret;
> +}
> +
> +static void int2mac(const u64 mac, u8 *hwaddr)
> +{
> +       s8 i;

Ditto.

> +int var_carrier_eeprom_read(const char *bus_name, int addr, struct var_carrier_eeprom *ep)
> +{
> +       int ret;
> +       struct udevice *bus;
> +       struct udevice *dev;
> +
> +       ret = uclass_get_device_by_name(UCLASS_I2C, bus_name, &bus);
> +       if (ret) {
> +               debug("%s: No bus '%s'\n", __func__, bus_name);
> +               return ret;
> +       }
> +
> +       ret = dm_i2c_probe(bus, addr, 0, &dev);
> +       if (ret) {
> +               debug("%s: Carrier EEPROM I2C probe failed\n", __func__);
> +               return ret;
> +       }
> +
> +       /* Read EEPROM to memory */
> +       ret = dm_i2c_read(dev, 0, (void *)ep, sizeof(*ep));
> +       if (ret) {
> +               debug("%s: Carrier EEPROM read failed, ret=%d\n", __func__, ret);
> +               return ret;

Better print all these error messages.


> +       return 0;
> +}
> +
> +int var_carrier_eeprom_is_valid(struct var_carrier_eeprom *ep)
> +{
> +       u32 crc, crc_offset = offsetof(struct var_carrier_eeprom, crc);
> +
> +       if (htons(ep->magic) != VAR_CARRIER_EEPROM_MAGIC) {
> +               debug("Invalid carrier EEPROM magic 0x%x, expected 0x%x\n",

Same here.

> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2022 NXP
> + * Copyright 2023 Variscite Ltd.
> + */
> +
> +#include <common.h>
> +#include <env.h>
> +#include <init.h>
> +#include <miiphy.h>
> +#include <netdev.h>
> +#include <asm/global_data.h>
> +#include <asm/arch-imx9/ccm_regs.h>
> +#include <asm/arch/sys_proto.h>
> +#include <asm/arch-imx9/imx93_pins.h>
> +#include <asm/arch/clock.h>
> +#include <power/pmic.h>
> +#include <dm/device.h>
> +#include <dm/uclass.h>
> +#include <usb.h>
> +#include <dwc3-uboot.h>

These USB-related header files could be removed.

> +       /* SDRAM ENV */
> +       snprintf(sdram_size_str, SDRAM_SIZE_STR_LEN, "%d",
> +                (int)(gd->ram_size / 1024 / 1024));
> +       env_set("sdram_size", sdram_size_str);
> +
> +       /* Carrier Board ENV */
> +       var_carrier_eeprom_read(VAR_CARRIER_EEPROM_I2C_NAME, CARRIER_EEPROM_ADDR, &carrier_eeprom);

No error checking?

> +       var_carrier_eeprom_get_revision(&carrier_eeprom, carrier_rev, sizeof(carrier_rev));

No error checking?

> diff --git a/board/variscite/imx93_var_som/imx93_var_som.env b/board/variscite/imx93_var_som/imx93_var_som.env
> new file mode 100644
> index 00000000000..84814e6d935
> --- /dev/null
> +++ b/board/variscite/imx93_var_som/imx93_var_som.env
> @@ -0,0 +1,104 @@
> +initrd_addr=0x83800000
> +emmc_dev=0

Not used anywhere. Please remove it.

> +sd_dev=1

Not used anywhere. Please remove it.

> +scriptaddr=0x83500000

Not used anywhere. Please remove it.

> +kernel_addr_r= __stringify(CONFIG_SYS_LOAD_ADDR)

Not used anywhere. Please remove it.

> +image=Image.gz
> +img_addr=0x82000000
> +splashimage=0x90000000

There is no splash screen support. Please remove it.

> +console=ttyLP0,115200 earlycon

earlycon is only useful for development. Please remove it.

> +#include <linux/kernel.h>
> +#include <asm/arch/ddr.h>
> +
> +struct dram_cfg_param ddr_ddrc_cfg[] = {

Please make it 'static'.

> +/* ddr phy trained csr */
> +struct dram_cfg_param ddr_ddrphy_trained_csr[] = {

Please make it 'static'.

> +struct dram_fsp_msg ddr_dram_fsp_msg[] = {
Please make it 'static'.

> +#include <common.h>
> +#include <command.h>
> +#include <cpu_func.h>
> +#include <hang.h>

Is this needed?

> +Get and Build the ARM Trusted firmware
> +--------------------------------------
> +
> +Note: srctree is U-Boot source directory
> +Get ATF from: https://github.com/varigit/imx-atf
> +branch: lf_v2.8_var02

Please use a more generic repo, such as NXP https://github.com/nxp-imx/imx-atf/
branch: lf_v2.8

> +/* Using ULP WDOG for reset */
> +#define WDOG_BASE_ADDR          WDG3_BASE_ADDR

Not needed to be done in this series, but in the future, it would be
nice to convert this wdog driver to DM.


More information about the U-Boot mailing list