From mboxrd@z Thu Jan  1 00:00:00 1970
From: Peter Griffin <peter.griffin@linaro.org>
Date: Wed, 29 Jul 2015 21:59:37 +0100
Subject: [U-Boot] [PATCH v2 5/6] ARM64: hikey: hi6220: Add u-boot
 support for the 96boards CE HiKey board.
In-Reply-To: <CAL_Jsq+uMYozJbw+C6cxNA8RzXObGjf+M+S8cEUksommzD_A7A@mail.gmail.com>
References: <1436371040-26620-1-git-send-email-peter.griffin@linaro.org>
 <1436371040-26620-6-git-send-email-peter.griffin@linaro.org>
 <CAL_Jsq+8b19G6u12A-4Qb3eBVTDvzU8V1r4zv_xip=_269bTNg@mail.gmail.com>
 <20150716004133.GB25886@griffinp-ThinkPad-X1-Carbon-2nd>
 <CAL_Jsq+uMYozJbw+C6cxNA8RzXObGjf+M+S8cEUksommzD_A7A@mail.gmail.com>
Message-ID: <CADrjBPpxHfF9OUqpDGxBS7mtcas87=ORbi4azS9pXRO45dLO+A@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Rob,

On 16 July 2015 at 14:28, Rob Herring <robherring2@gmail.com> wrote:

> 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 at linaro.org> wrote:
> >> > HiKey is the first 96boards consumer edition compliant board. It
> features a hi6220
> >> > SoC which has eight ARM A53 cpu's.
> >> >
>
> [...]
>

<snip>


> >> 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.
>

OK thanks for the pointer :) I'm waiting for one of Grants expansion boards
to be sent to me, so will try it out when it arrives.


> >> > +#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
>

Fixed in v3


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

Fixed in v3


>
> >> > +                               "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.
>

Thanks for sharing your setup, I've updated the bootcmd to be the USB
ethernet by default in the V3 patches.


>> > +/* 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.
>

Good idea, I'd not thought of that and we could most likely do that if we
wanted. However I plan on sticking with u-boot env on the sd card for the
moment
as Tom pointed out it can be a bit more user friendly for a community board.


>
> > 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.
>

Tom answered this one already.


>
> > 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. :)
>
> Ha, yes maybe :)

regards,

Peter.