[U-Boot] [PATCH v2 5/6] ARM64: hikey: hi6220: Add u-boot support for the 96boards CE HiKey board.

[Rob Herring <robherring2@gmail.com>]

On Wed, Jul 15, 2015 at 7:41 PM, Peter Griffin <peter.griffin@linaro.org> wrote:
> Hi Rob,
>
> On Fri, 10 Jul 2015, Rob Herring wrote:
>
>> On Wed, Jul 8, 2015 at 10:57 AM, Peter Griffin <peter.griffin@linaro.org> wrote:
>> > HiKey is the first 96boards consumer edition compliant board. It features a hi6220
>> > SoC which has eight ARM A53 cpu's.
>> >

[...]

>> > +#define CONFIG_BAUDRATE                        115200
>> > +#define CONFIG_SYS_SERIAL0             0xF8015000
>>
>> Just do:
>>
>> #define CONFIG_PL01x_PORTS             {(void *)0xf8015000}
>
> Ok, fixed in V3
>
>>
>> You are probably going to want to setup multiple serial consoles
>> (debug + LS header). That can come later, but I've figured out how to
>> enable that if you are interested.
>
> Yes I'm interested, please do let me know :)

See this commit:

https://git.linaro.org/people/rob.herring/u-boot.git/commitdiff/f1d0aef06ae7fe09793d46589bd94fa36c45bbc0

This may be 8250 specific and require more work for pl011 driver. The
mixture of 0 and 1 based indexing makes it fun too.


>> > +#define CONFIG_EXTRA_ENV_SETTINGS      \
>> > +                               "kernel_name=Image\0"   \
>> > +                               "kernel_addr=0x0000000\0" \
>>
>> Shouldn't this be 0x80000 to avoid copying from 0x0 to 0x80000.
>
> I've updated this. Kernel boot time is much reduced with this and also the
> icache being enabled.

Also, this should be kernel_addr_r

>
>>
>> > +                               "fdt_name=hi6220-hikey.dtb\0" \
>> > +                               "fdt_addr=0x0300000\0" \

and fdt_addr_r

>> > +                               "max_fdt=0x100000\0" \
>>
>> I don't think this is needed.
>
> Removed in V3
>>
>> > +                               "fdt_high=0xffffffffffffffff\0" \
>> > +                               "initrd_high=0xffffffffffffffff\0" \
>> > +
>> > +/* Assume we boot with root on the first partition of a USB stick */
>> > +#define CONFIG_BOOTARGS                "console=ttyAMA0,115200n8 /dev/mmcblk0p7 rw "
>>
>> /dev/mmcblk0p7 doesn't look right. You mean "root=/dev/..."?
>
> Good spot, yes your right. Plus now you highlight it the comment above also needs updating.
>
> Will fix in V3.
>
>>
>> > +
>> > +/* Copy the kernel and FDT to DRAM memory and boot */
>> > +#define CONFIG_BOOTCOMMAND     "booti $kernel_addr_r - $fdt_addr_r"
>>
>> Don't you need to set these variables?
>>
>> Also, don't you need to load the kernel and dtb first?
>
> Yes, but I'm not sure quite what to make the default here. My personal
> workflow is: -
>
>  "usb start; dhcp; tftp $kernel_addr $kernel_name; tftp $fdt_addr $fdt_name;
>    booti $kernel_addr - $fdt_addr"
>
> So I could use that unless you have a better idea?

Not really as everyone has their own preferences. I have some thing like this:

#define CONFIG_BOOTCOMMAND \
"while true; do " \
"mmc read ${fdt_addr_r} 0x10000 0x1000; " \
"fastboot; " \
"mmc read ${fdt_addr_r} 0x10000 0x1000; " \
"mmc read ${kernel_addr_r} 0x8000 0x8000 && " \
"bootm ${kernel_addr_r} ${kernel_addr_r} ${fdt_addr_r};" \
"done"

This relies on fastboot doing USB cable detection and it exits if no
USB connection.

USB ethernet is as good a default as any. Otherwise reading Image and
dtb from the 1st or bootable partition (the default) would be
reasonable.


>> > +/* Preserve enviroment onto sd card */
>> > +#define CONFIG_ENV_IS_IN_MMC
>> > +#define CONFIG_SYS_MMC_ENV_DEV         1
>> > +#define CONFIG_SYS_MMC_ENV_PART                0
>>
>> Don't you have these reversed? The first MMC device is 0 and I think
>> partition numbering starts at 1.
>
> Having CONFIG_SYS_MMC_ENV_DEV 1 was deliberate, as the first device is eMMC, and
> I don't have a "official" partition to save the u-boot enviroment in.
> So as not to corrupt anything folks may have flashed into eMMC from the official
> builds I opted to save the u-boot env to SD card which is device 1.

Okay, but don't you have spare space in the partition with u-boot? I
have a single bootloader partition 1MB in size and the last 8? KB is
the env.

> However that seems to have been working by luck with ENC_PART being 0, and it was
> actually corrupting the partition table of the SD card. Looking more closely I think
> what I should of used is
>
> #define CONFIG_ENV_IS_IN_FAT
> #define FAT_ENV_INTERFACE               "mmc"
> #define FAT_ENV_DEVICE_AND_PART         "1:1"
> #define FAT_ENV_FILE                    "uboot.env"
>
> This then saves the enviroment on a fat formatted SD card with the filename
> u-boot.env. This is what I plan on using for v3.
>
> Maybe I should additionally request some space in the official eMMC parition
> table and then we could switch over to using that.
>
>>
>> > +#define CONFIG_ENV_OFFSET               0x0
>> > +#define CONFIG_ENV_SIZE                        0x1000
>> > +#define CONFIG_ENV_OFFSET_REDUND        (CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE)
>> > +#define CONFIG_SYS_REDUNDAND_ENVIRONMENT
>>
>> Is redundant env necessary? It seems like this was more for raw NAND
>> and shouldn't really be needed for MMC.
>
> README file documents it as being valid for CONFIG_ENV_IS_IN_MMC, and a bunch of boards
> declare it with their CONFIG_ENV_IS_IN_MMC such as omap5_uevm.h, dra7xx_evm.h,
> am335x_evm.h. Whilst using managed NAND should be more reliable, I think it
> is still used in case there is a power failure whilst issuing 'saveenv'.

Perhaps a bunch of cut and paste. I'd guess there are many more
platforms that use MMC and don't enable redundant.

> Anyways with moving to CONFIG_ENV_IS_IN_FAT I won't need it anymore so it will be
> removed in V3.

Storing in FAT probably only increases your chance of failure from
power failure. :)

Rob