Re: [PATCH 6/7] arm: mvebu: Add RD-AC5X board

Pali Rohár
2022-09-16T10:58:16Z

On Friday 16 September 2022 16:54:22 Chris Packham wrote:
> +&spi0 {
> + status = "okay";
> +
> + spiflash0: flash@0 {
> + compatible = "jedec,spi-nor";
> + spi-max-frequency = <50000000>;
> + spi-tx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
> + spi-rx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
> + reg = <0>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition@0 {
> + label = "spi_flash_part0";

I guess such label is useless. There is something stored, so it would be
a wise idea to put there correct label. I do not know how it is on this
platform, but on A3720 at offset 0 is "firmware", which consist of CM3
part, A53 part and U-Boot. It is not even U-Boot itself.

> + reg = <0x0 0x800000>;
> + };
> +
> + parition@1 {
> + label = "spi_flash_part1";
> + reg = <0x800000 0x700000>;
> + };
> +
> + parition@2 {
> + label = "spi_flash_part2";
> + reg = <0xF00000 0x100000>;
> + };
> + };
> +};

...

> +/* Default Env vars */
> +#define CONFIG_IPADDR 0.0.0.0 /* In order to cause an error */
> +#define CONFIG_SERVERIP 0.0.0.0 /* In order to cause an error */
> +#define CONFIG_NETMASK 255.255.255.0
> +#define CONFIG_GATEWAYIP 0.0.0.0
> +#define CONFIG_ETHPRIME "eth0"
> +#define CONFIG_ROOTPATH "/srv/nfs/" /* Default Dir for NFS */
> +#define CONFIG_ENV_OVERWRITE /* ethaddr can be reprogrammed */
> +#define CONFIG_EXTRA_ENV_SETTINGS "bootcmd=run get_images; " \
> + "run set_bootargs; " \
> + "booti $kernel_addr_r " \
> + "$ramdisk_addr_r " \
> + "$fdt_addr_r\0" \
> + "extra_params=pci=pcie_bus_safe\0" \
> + "kernel_addr_r=0x202000000\0" \
> + "initrd_addr=0x206000000\0" \
> + "initrd_size=0x2000000\0" \
> + "fdt_addr_r=0x201000000\0" \
> + "loadaddr=0x202000000\0" \
> + "hostname=marvell\0" \
> + "ramdisk_addr_r=0x206000000\0" \
> + "ramfs_name=-\0" \
> + "cpuidle=cpuidle.off=1\0" \
> + "fdt_name=fdt.dtb\0" \
> + "netdev=eth0\0" \
> + "ethaddr=00:51:82:11:22:00\0" \
> + "eth1addr=00:51:82:11:22:01\0" \
> + "image_name=Image\0" \
> + "get_ramfs=if test \"${ramfs_name}\"" \
> + " != \"-\"; then setenv " \
> + "ramdisk_addr_r 0x8000000; " \
> + "tftpboot $ramdisk_addr_r " \
> + "$ramfs_name; else setenv " \
> + "ramdisk_addr_r -;fi\0" \
> + "get_images=tftpboot $kernel_addr_r " \
> + "$image_name; tftpboot " \
> + "$fdt_addr_r $fdt_name; " \
> + "run get_ramfs\0" \
> + "console=" "console=ttyS0,115200 "\
> + "earlycon=uart8250,mmio32,0xf0512000\0"\
> + "root=root=/dev/nfs rw\0" \
> + "set_bootargs=setenv bootargs $console"\
> + " $root ip=$ipaddr:$serverip:" \
> + "$gatewayip:$netmask:$hostname"\
> + ":$netdev:none nfsroot="\
> + "$serverip:$rootpath,tcp,v3 " \
> + "$extra_params " \
> + "$cpuidle"

This is hard to read, where is the variable name, where there is its
value, etc...

And it would be better to use distroboot instead of manually written
boot script.