[PATCH 2/3] board: Add support for Sielaff i.MX6 Solo board

[Frieder Schrempf]

Hi Fabio,

thanks for reviewing!

On 10.02.24 19:12, Fabio Estevam wrote:
> Hi Frieder,
> 
> On Thu, Feb 8, 2024 at 1:59 PM Frieder Schrempf <frieder at fris.de> wrote:
> 
>> +/ {
>> +       binman: binman {
>> +               filename = "flash.bin";
>> +               pad-byte = <0x00>;
>> +
>> +               spl: blob-ext at 1 {
>> +                       offset = <0x0>;
>> +                       filename = "SPL";
>> +               };
>> +
>> +               uboot: blob-ext at 2 {
>> +                       offset = <0x11000>;
>> +                       filename = "u-boot.img";
>> +               };
>> +       };
> 
> As mentioned in patch 3/3, please remove the binman entry and use
> u-boot-with-spl.imx instead.

I understand the reasoning and I can change that if you insist. But I'm
not really happy about it as we already use the flash.bin as common name
for the binary across all platforms downstream. Also in our upstream
config for the i.MX6UL/ULL SoMs we already use this pattern (see [1]).

> 
>> +config TARGET_MX6S_SIELAFF
>> +       bool "Sielaff i.MX6 Solo Board"
>> +       depends on MX6S
>> +       select BINMAN
> 
> Please drop it.
> 
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +iomux_v3_cfg_t nfc_pads[] = {
> 
> Make it static.

OK

> 
>> --- /dev/null
>> +++ b/board/sielaff/imx6dl-sielaff/imx6dl-sielaff.env
>> @@ -0,0 +1,115 @@
>> +blkloadfdt=fatload ${device} ${devnum}:${partnum} ${fdt_addr} ${load_fdt_file}
>> +blkloadimage=fatload ${device} ${devnum}:${partnum} ${loadaddr} ${load_image}
>> +boot_devices=usb mmc ubi
>> +bootargs_base=vt.global_cursor_default=0 consoleblank=0 cma=200M fbcon=rotate:1
>> +bootdelay=3
>> +bootdir=
>> +console=ttymxc1,115200
>> +ethact=FEC0
>> +fdt_addr=0x18000000
>> +fdt_file_legacy=imx6dl_sielaff.dtb
>> +fdt_file=imx6dl-sielaff.dtb
>> +fdt_high=0xffffffff
>> +hostname=mx6s-siline-hmi
> 
> Please remove it.

OK

> 
>> +void reset_cpu(void)
>> +{
>> +}
> 
> Is this needed?

No, I will drop it.

> Does the 'reset' command work on this board?

No, I will fix it.

>> +++ b/configs/imx6dl_sielaff_defconfig
>> @@ -0,0 +1,117 @@
>> +CONFIG_ARM=y
>> +CONFIG_SPL_SYS_ICACHE_OFF=y
>> +CONFIG_SPL_SYS_DCACHE_OFF=y
> 
> These options should be dropped.

OK

> 
>> +#include <asm/arch/imx-regs.h>
>> +#include <linux/sizes.h>
>> +#include "mx6_common.h"
>> +
>> +#define CFG_MXC_UART_BASE              UART2_BASE
> 
> As you use DM_SERIAL, this can be dropped.

This is still needed as SPL runs without DM due to internal RAM size
constraints.

Thanks
Frieder

[1]
https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/imx6ul-kontron-bl-common-u-boot.dtsi#L10