Re: [PATCH] ARM: boards: i.MX93: Add NXP FRDM i.MX93 support

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 5/27/25 14:23, Marco Felsch wrote:
> On 25-05-27, Fabian Pflug wrote:

>> +++ b/arch/arm/boards/nxp-imx93-frdm/board.c
>> @@ -0,0 +1,34 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#define pr_fmt(fmt) "FRDM i.MX93: " fmt
>> +
>> +#include <bootsource.h>
>> +#include <common.h>
>> +#include <init.h>
>> +#include <deep-probe.h>
>> +
>> +static int frdm_imx93_probe(struct device *dev)
>> +{
>> +	if (bootsource_get() == BOOTSOURCE_MMC && bootsource_get_instance() == 2) {
>> +		of_device_enable_path("/choosen/environment-sd");
>> +	} else {
>> +		of_device_enable_path("/choosen/environment-emmc");
>> +	}
>> +	return 0;
> 
> Out of curiosity: could this be handled in a common way, so we can drop
> this board driver completely?

Yes, this should just be dropped and environment should come from the
bootsource via Type UUID. FWIW, this is non-functional anyway;
The device tree added in this series has no barebox,environment
compatible node and if it did, nothing descends into /choosen
(as opposed to /chosen).

>> +BAREBOX_DEEP_PROBE_ENABLE(frdm_imx93_of_match);

This will need to remain. I have a series for deep probe to become the
default, but for now just keep the match struct and the
BAREBOX_DEEP_PROBE_ENABLE and drop the driver code.

>> diff --git a/arch/arm/boards/nxp-imx93-frdm/lowlevel.c b/arch/arm/boards/nxp-imx93-frdm/lowlevel.c
>> new file mode 100644
>> index 0000000000..7fbb7e96c5
>> --- /dev/null
>> +++ b/arch/arm/boards/nxp-imx93-frdm/lowlevel.c
>> @@ -0,0 +1,46 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <common.h>
>> +#include <debug_ll.h>
>> +#include <mach/imx/debug_ll.h>
>> +#include <mach/imx/generic.h>
>> +#include <mach/imx/xload.h>
>> +#include <asm/barebox-arm.h>
>> +#include <soc/imx9/ddr.h>
>> +#include <mach/imx/atf.h>
>> +#include <mach/imx/xload.h>
>> +#include <mach/imx/esdctl.h>
>> +
>> +extern char __dtb_z_imx93_frdm_start[];
>> +extern struct dram_timing_info frdm_imx93_dram_timing;
>> +
>> +static noinline void frdm_imx93_continue(void)
>> +{
>> +	void *base = IOMEM(MX9_UART1_BASE_ADDR);

void __iomem *

>> +	void *muxbase = IOMEM(MX9_IOMUXC_BASE_ADDR);

void __iomem *

>> +
>> +	/* configure LPUART1 TX pin */
>> +	writel(0x0, muxbase + 0x184);
>> +
>> +	imx9_uart_setup(IOMEM(base));

IOMEM here is redundant with above changes.

>> +	pbl_set_putc(lpuart32_putc, base + 0x10);
>> +	putc_ll('>');

putchar(). I think it's useful to print it always.


Cheers,
Ahmad

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |