* [PATCH u-boot v2] Enable FIT image support and FDT loading for AST2400/AST2500
@ 2016-11-02 5:26 Rick Altherr
2016-11-02 6:31 ` Joel Stanley
0 siblings, 1 reply; 6+ messages in thread
From: Rick Altherr @ 2016-11-02 5:26 UTC (permalink / raw)
To: openbmc
FIT is the modern u-boot native image format for kernels, device trees,
and ramdisks. Enabling FIT only compiles in support for the image
format. For these devices, the kernel+dtb and ramdisk are loaded from
separate locations in flash and can be any mix of legacy or FIT images.
When using FIT images, the dtb is stored as a separate entry that
requires CONFIG_OF_LIBFDT to load it into RAM and pass it to the kernel.
Tested under qemu with both legacy and FIT kernel+dtb images for
palmetto and witherspoon.
Signed-off-by: Rick Altherr <raltherr@google.com>
---
Changes since v1:
- Fixed commit message grammar
- Clarified config option needed for FDT loading
---
configs/ast_g4_ncsi_defconfig | 3 +++
configs/ast_g4_phy_defconfig | 3 +++
configs/ast_g5_ncsi_defconfig | 3 +++
configs/ast_g5_phy_defconfig | 3 +++
4 files changed, 12 insertions(+)
diff --git a/configs/ast_g4_ncsi_defconfig b/configs/ast_g4_ncsi_defconfig
index 4ee71c5..70cd3c2 100644
--- a/configs/ast_g4_ncsi_defconfig
+++ b/configs/ast_g4_ncsi_defconfig
@@ -3,5 +3,8 @@ CONFIG_TARGET_AST_G4=y
CONFIG_SYS_PROMPT="ast# "
CONFIG_CMD_DHCP=y
CONFIG_CMD_PING=y
+CONFIG_FIT=y
+CONFIG_FIT_VERBOSE=y
+CONFIG_OF_LIBFDT=y
CONFIG_SPI_FLASH=y
CONFIG_SYS_NS16550=y
diff --git a/configs/ast_g4_phy_defconfig b/configs/ast_g4_phy_defconfig
index 61fd69b..468fbc4 100644
--- a/configs/ast_g4_phy_defconfig
+++ b/configs/ast_g4_phy_defconfig
@@ -4,5 +4,8 @@ CONFIG_ASPEED_NET_PHY=y
CONFIG_SYS_PROMPT="ast# "
CONFIG_CMD_DHCP=y
CONFIG_CMD_PING=y
+CONFIG_FIT=y
+CONFIG_FIT_VERBOSE=y
+CONFIG_OF_LIBFDT=y
CONFIG_SPI_FLASH=y
CONFIG_SYS_NS16550=y
diff --git a/configs/ast_g5_ncsi_defconfig b/configs/ast_g5_ncsi_defconfig
index 6d11afb..8a9c297 100644
--- a/configs/ast_g5_ncsi_defconfig
+++ b/configs/ast_g5_ncsi_defconfig
@@ -3,5 +3,8 @@ CONFIG_TARGET_AST_G5=y
CONFIG_SYS_PROMPT="ast# "
CONFIG_CMD_DHCP=y
CONFIG_CMD_PING=y
+CONFIG_FIT=y
+CONFIG_FIT_VERBOSE=y
+CONFIG_OF_LIBFDT=y
CONFIG_SPI_FLASH=y
CONFIG_SYS_NS16550=y
diff --git a/configs/ast_g5_phy_defconfig b/configs/ast_g5_phy_defconfig
index 20f62e0..fd450b9 100644
--- a/configs/ast_g5_phy_defconfig
+++ b/configs/ast_g5_phy_defconfig
@@ -4,5 +4,8 @@ CONFIG_ASPEED_NET_PHY=y
CONFIG_SYS_PROMPT="ast# "
CONFIG_CMD_DHCP=y
CONFIG_CMD_PING=y
+CONFIG_FIT=y
+CONFIG_FIT_VERBOSE=y
+CONFIG_OF_LIBFDT=y
CONFIG_SPI_FLASH=y
CONFIG_SYS_NS16550=y
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH u-boot v2] Enable FIT image support and FDT loading for AST2400/AST2500
2016-11-02 5:26 [PATCH u-boot v2] Enable FIT image support and FDT loading for AST2400/AST2500 Rick Altherr
@ 2016-11-02 6:31 ` Joel Stanley
2016-11-02 16:55 ` Cédric Le Goater
2016-11-10 0:00 ` Rick Altherr
0 siblings, 2 replies; 6+ messages in thread
From: Joel Stanley @ 2016-11-02 6:31 UTC (permalink / raw)
To: Rick Altherr, Cédric Le Goater; +Cc: OpenBMC Maillist
Hi Rick,
On Wed, Nov 2, 2016 at 3:56 PM, Rick Altherr <raltherr@google.com> wrote:
> FIT is the modern u-boot native image format for kernels, device trees,
> and ramdisks. Enabling FIT only compiles in support for the image
> format. For these devices, the kernel+dtb and ramdisk are loaded from
> separate locations in flash and can be any mix of legacy or FIT images.
> When using FIT images, the dtb is stored as a separate entry that
> requires CONFIG_OF_LIBFDT to load it into RAM and pass it to the kernel.
>
> Tested under qemu with both legacy and FIT kernel+dtb images for
> palmetto and witherspoon.
The changes look good to me.
Is the FIT support in v2016.07 new enough for us? Is there any reason
to move to a v2016.09 base?
It does introduce a warning in a hack we carry for loading the
initramfs, which I think points to a legitimate issue:
common/image.c: In function ‘boot_get_ramdisk’:
common/image.c:1078:4: warning: ‘rd_load’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
memmove ((void *)rd_load, (uchar *)rd_data, rd_len);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This code an Aspeed hack we carry. It is required to copy the
initramfs from flash. Cedric was doing some investigation into why we
need the hack, and it appears to be an upstream bug.
If I read the code correctly, prior to enabling FIT we only accepted
IMAGE_FORMAT_LEGACY. If we move the hack to be part of that case we
should be ok, as we don't need to do the hack when loading from FIT.
Can you try that and add a second patch to your series that moves the hack?
Finally, I took a look at the impact on code size. I used
arm-linux-gnueabi-gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005 from
Ubuntu 16.10.
$ size u-boot
text data bss dec hex filename
151636 4576 61620 217832 352e8 u-boot
222302 7952 62768 293022 4789e u-boot
Wow, the FIT image is fatter. What does FIT support pull in that
increases the code size so much?
The on disk size suggests that size isn't lying:
Unfit: 156012
FIT: 230056
Did we expect the change to be that large?
This is not a blocker, as we currently set aside 384KB for u-boot.
Cheers,
Joel
>
> Signed-off-by: Rick Altherr <raltherr@google.com>
> ---
> Changes since v1:
> - Fixed commit message grammar
> - Clarified config option needed for FDT loading
> ---
> configs/ast_g4_ncsi_defconfig | 3 +++
> configs/ast_g4_phy_defconfig | 3 +++
> configs/ast_g5_ncsi_defconfig | 3 +++
> configs/ast_g5_phy_defconfig | 3 +++
> 4 files changed, 12 insertions(+)
>
> diff --git a/configs/ast_g4_ncsi_defconfig b/configs/ast_g4_ncsi_defconfig
> index 4ee71c5..70cd3c2 100644
> --- a/configs/ast_g4_ncsi_defconfig
> +++ b/configs/ast_g4_ncsi_defconfig
> @@ -3,5 +3,8 @@ CONFIG_TARGET_AST_G4=y
> CONFIG_SYS_PROMPT="ast# "
> CONFIG_CMD_DHCP=y
> CONFIG_CMD_PING=y
> +CONFIG_FIT=y
> +CONFIG_FIT_VERBOSE=y
> +CONFIG_OF_LIBFDT=y
> CONFIG_SPI_FLASH=y
> CONFIG_SYS_NS16550=y
> diff --git a/configs/ast_g4_phy_defconfig b/configs/ast_g4_phy_defconfig
> index 61fd69b..468fbc4 100644
> --- a/configs/ast_g4_phy_defconfig
> +++ b/configs/ast_g4_phy_defconfig
> @@ -4,5 +4,8 @@ CONFIG_ASPEED_NET_PHY=y
> CONFIG_SYS_PROMPT="ast# "
> CONFIG_CMD_DHCP=y
> CONFIG_CMD_PING=y
> +CONFIG_FIT=y
> +CONFIG_FIT_VERBOSE=y
> +CONFIG_OF_LIBFDT=y
> CONFIG_SPI_FLASH=y
> CONFIG_SYS_NS16550=y
> diff --git a/configs/ast_g5_ncsi_defconfig b/configs/ast_g5_ncsi_defconfig
> index 6d11afb..8a9c297 100644
> --- a/configs/ast_g5_ncsi_defconfig
> +++ b/configs/ast_g5_ncsi_defconfig
> @@ -3,5 +3,8 @@ CONFIG_TARGET_AST_G5=y
> CONFIG_SYS_PROMPT="ast# "
> CONFIG_CMD_DHCP=y
> CONFIG_CMD_PING=y
> +CONFIG_FIT=y
> +CONFIG_FIT_VERBOSE=y
> +CONFIG_OF_LIBFDT=y
> CONFIG_SPI_FLASH=y
> CONFIG_SYS_NS16550=y
> diff --git a/configs/ast_g5_phy_defconfig b/configs/ast_g5_phy_defconfig
> index 20f62e0..fd450b9 100644
> --- a/configs/ast_g5_phy_defconfig
> +++ b/configs/ast_g5_phy_defconfig
> @@ -4,5 +4,8 @@ CONFIG_ASPEED_NET_PHY=y
> CONFIG_SYS_PROMPT="ast# "
> CONFIG_CMD_DHCP=y
> CONFIG_CMD_PING=y
> +CONFIG_FIT=y
> +CONFIG_FIT_VERBOSE=y
> +CONFIG_OF_LIBFDT=y
> CONFIG_SPI_FLASH=y
> CONFIG_SYS_NS16550=y
> --
> 2.8.0.rc3.226.g39d4020
>
> _______________________________________________
> openbmc mailing list
> openbmc@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH u-boot v2] Enable FIT image support and FDT loading for AST2400/AST2500
2016-11-02 6:31 ` Joel Stanley
@ 2016-11-02 16:55 ` Cédric Le Goater
2016-11-10 0:00 ` Rick Altherr
1 sibling, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2016-11-02 16:55 UTC (permalink / raw)
To: Joel Stanley, Rick Altherr; +Cc: OpenBMC Maillist
On 11/02/2016 07:31 AM, Joel Stanley wrote:
> Hi Rick,
>
> On Wed, Nov 2, 2016 at 3:56 PM, Rick Altherr <raltherr@google.com> wrote:
>> FIT is the modern u-boot native image format for kernels, device trees,
>> and ramdisks. Enabling FIT only compiles in support for the image
>> format. For these devices, the kernel+dtb and ramdisk are loaded from
>> separate locations in flash and can be any mix of legacy or FIT images.
>> When using FIT images, the dtb is stored as a separate entry that
>> requires CONFIG_OF_LIBFDT to load it into RAM and pass it to the kernel.
>>
>> Tested under qemu with both legacy and FIT kernel+dtb images for
>> palmetto and witherspoon.
>
> The changes look good to me.
>
> Is the FIT support in v2016.07 new enough for us? Is there any reason
> to move to a v2016.09 base?
>
> It does introduce a warning in a hack we carry for loading the
> initramfs, which I think points to a legitimate issue:
>
> common/image.c: In function ‘boot_get_ramdisk’:
> common/image.c:1078:4: warning: ‘rd_load’ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> memmove ((void *)rd_load, (uchar *)rd_data, rd_len);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This code an Aspeed hack we carry. It is required to copy the
> initramfs from flash. Cedric was doing some investigation into why we
> need the hack, and it appears to be an upstream bug.
>
> If I read the code correctly, prior to enabling FIT we only accepted
> IMAGE_FORMAT_LEGACY. If we move the hack to be part of that case we
> should be ok, as we don't need to do the hack when loading from FIT.
>
> Can you try that and add a second patch to your series that moves the hack?
Yes. please try to remove the hack copying the ramdisk in ram or take
that patch :
https://github.com/legoater/u-boot/commit/e503c2f5f5ddc967307e373569c09a0bd0e87038
Thanks,
C.
> Finally, I took a look at the impact on code size. I used
> arm-linux-gnueabi-gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005 from
> Ubuntu 16.10.
>
> $ size u-boot
> text data bss dec hex filename
> 151636 4576 61620 217832 352e8 u-boot
> 222302 7952 62768 293022 4789e u-boot
>
> Wow, the FIT image is fatter. What does FIT support pull in that
> increases the code size so much?
>
> The on disk size suggests that size isn't lying:
>
> Unfit: 156012
> FIT: 230056
>
> Did we expect the change to be that large?
>
> This is not a blocker, as we currently set aside 384KB for u-boot.
>
> Cheers,
>
> Joel
>
>
>>
>> Signed-off-by: Rick Altherr <raltherr@google.com>
>> ---
>> Changes since v1:
>> - Fixed commit message grammar
>> - Clarified config option needed for FDT loading
>> ---
>> configs/ast_g4_ncsi_defconfig | 3 +++
>> configs/ast_g4_phy_defconfig | 3 +++
>> configs/ast_g5_ncsi_defconfig | 3 +++
>> configs/ast_g5_phy_defconfig | 3 +++
>> 4 files changed, 12 insertions(+)
>>
>> diff --git a/configs/ast_g4_ncsi_defconfig b/configs/ast_g4_ncsi_defconfig
>> index 4ee71c5..70cd3c2 100644
>> --- a/configs/ast_g4_ncsi_defconfig
>> +++ b/configs/ast_g4_ncsi_defconfig
>> @@ -3,5 +3,8 @@ CONFIG_TARGET_AST_G4=y
>> CONFIG_SYS_PROMPT="ast# "
>> CONFIG_CMD_DHCP=y
>> CONFIG_CMD_PING=y
>> +CONFIG_FIT=y
>> +CONFIG_FIT_VERBOSE=y
>> +CONFIG_OF_LIBFDT=y
>> CONFIG_SPI_FLASH=y
>> CONFIG_SYS_NS16550=y
>> diff --git a/configs/ast_g4_phy_defconfig b/configs/ast_g4_phy_defconfig
>> index 61fd69b..468fbc4 100644
>> --- a/configs/ast_g4_phy_defconfig
>> +++ b/configs/ast_g4_phy_defconfig
>> @@ -4,5 +4,8 @@ CONFIG_ASPEED_NET_PHY=y
>> CONFIG_SYS_PROMPT="ast# "
>> CONFIG_CMD_DHCP=y
>> CONFIG_CMD_PING=y
>> +CONFIG_FIT=y
>> +CONFIG_FIT_VERBOSE=y
>> +CONFIG_OF_LIBFDT=y
>> CONFIG_SPI_FLASH=y
>> CONFIG_SYS_NS16550=y
>> diff --git a/configs/ast_g5_ncsi_defconfig b/configs/ast_g5_ncsi_defconfig
>> index 6d11afb..8a9c297 100644
>> --- a/configs/ast_g5_ncsi_defconfig
>> +++ b/configs/ast_g5_ncsi_defconfig
>> @@ -3,5 +3,8 @@ CONFIG_TARGET_AST_G5=y
>> CONFIG_SYS_PROMPT="ast# "
>> CONFIG_CMD_DHCP=y
>> CONFIG_CMD_PING=y
>> +CONFIG_FIT=y
>> +CONFIG_FIT_VERBOSE=y
>> +CONFIG_OF_LIBFDT=y
>> CONFIG_SPI_FLASH=y
>> CONFIG_SYS_NS16550=y
>> diff --git a/configs/ast_g5_phy_defconfig b/configs/ast_g5_phy_defconfig
>> index 20f62e0..fd450b9 100644
>> --- a/configs/ast_g5_phy_defconfig
>> +++ b/configs/ast_g5_phy_defconfig
>> @@ -4,5 +4,8 @@ CONFIG_ASPEED_NET_PHY=y
>> CONFIG_SYS_PROMPT="ast# "
>> CONFIG_CMD_DHCP=y
>> CONFIG_CMD_PING=y
>> +CONFIG_FIT=y
>> +CONFIG_FIT_VERBOSE=y
>> +CONFIG_OF_LIBFDT=y
>> CONFIG_SPI_FLASH=y
>> CONFIG_SYS_NS16550=y
>> --
>> 2.8.0.rc3.226.g39d4020
>>
>> _______________________________________________
>> openbmc mailing list
>> openbmc@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/openbmc
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH u-boot v2] Enable FIT image support and FDT loading for AST2400/AST2500
2016-11-02 6:31 ` Joel Stanley
2016-11-02 16:55 ` Cédric Le Goater
@ 2016-11-10 0:00 ` Rick Altherr
2016-11-10 11:55 ` Cédric Le Goater
1 sibling, 1 reply; 6+ messages in thread
From: Rick Altherr @ 2016-11-10 0:00 UTC (permalink / raw)
To: Joel Stanley; +Cc: Cédric Le Goater, OpenBMC Maillist
[-- Attachment #1: Type: text/plain, Size: 5816 bytes --]
On Tue, Nov 1, 2016 at 11:31 PM, Joel Stanley <joel@jms.id.au> wrote:
> Hi Rick,
>
> On Wed, Nov 2, 2016 at 3:56 PM, Rick Altherr <raltherr@google.com> wrote:
> > FIT is the modern u-boot native image format for kernels, device trees,
> > and ramdisks. Enabling FIT only compiles in support for the image
> > format. For these devices, the kernel+dtb and ramdisk are loaded from
> > separate locations in flash and can be any mix of legacy or FIT images.
> > When using FIT images, the dtb is stored as a separate entry that
> > requires CONFIG_OF_LIBFDT to load it into RAM and pass it to the kernel.
> >
> > Tested under qemu with both legacy and FIT kernel+dtb images for
> > palmetto and witherspoon.
>
> The changes look good to me.
>
> Is the FIT support in v2016.07 new enough for us? Is there any reason
> to move to a v2016.09 base?
>
>
AFAIK, v2016.07 is new enough. It certainly includes FIT, FDT loading, and
FIT signature checking.
> It does introduce a warning in a hack we carry for loading the
> initramfs, which I think points to a legitimate issue:
>
> common/image.c: In function ‘boot_get_ramdisk’:
> common/image.c:1078:4: warning: ‘rd_load’ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> memmove ((void *)rd_load, (uchar *)rd_data, rd_len);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This code an Aspeed hack we carry. It is required to copy the
> initramfs from flash. Cedric was doing some investigation into why we
> need the hack, and it appears to be an upstream bug.
>
> If I read the code correctly, prior to enabling FIT we only accepted
> IMAGE_FORMAT_LEGACY. If we move the hack to be part of that case we
> should be ok, as we don't need to do the hack when loading from FIT.
>
> Can you try that and add a second patch to your series that moves the hack?
>
>
Looks like Cedric has a patch that just removes the hack. I'll look into
this more and add it to my series.
> Finally, I took a look at the impact on code size. I used
> arm-linux-gnueabi-gcc (Ubuntu 6.2.0-5ubuntu12) 6.2.0 20161005 from
> Ubuntu 16.10.
>
> $ size u-boot
> text data bss dec hex filename
> 151636 4576 61620 217832 352e8 u-boot
> 222302 7952 62768 293022 4789e u-boot
>
> Wow, the FIT image is fatter. What does FIT support pull in that
> increases the code size so much?
>
> The on disk size suggests that size isn't lying:
>
> Unfit: 156012
> FIT: 230056
>
> Did we expect the change to be that large?
>
> This is not a blocker, as we currently set aside 384KB for u-boot.
>
I tried a few variations to see where the increase comes from. Looks like
it is an even split between the FIT image format support and the FDT
loading support. I expected a sizable increase from libfdt (brought in by
both FIT and FDT options) and additional hash algorithms (sha1, sha256).
Palmetto orig: 165336
Palmetto FIT: 201356
Palmetto FIT w/ verbose: 201720
Palmetto FIT w/ verbose+FDT: 238976
Palmetto FIT w/ verbose+FDT-EFI: 223380
Enabling FDT seems to enable EFI by default as well. Explicitly turning
off EFI loading support saves ~13k. I'm uncertain if it is reasonable to
say that EFI support should be turned off by default. OpenBMC certainly
doesn't need it but the u-boot configuration is ideally more generic.
> Cheers,
>
> Joel
>
>
> >
> > Signed-off-by: Rick Altherr <raltherr@google.com>
> > ---
> > Changes since v1:
> > - Fixed commit message grammar
> > - Clarified config option needed for FDT loading
> > ---
> > configs/ast_g4_ncsi_defconfig | 3 +++
> > configs/ast_g4_phy_defconfig | 3 +++
> > configs/ast_g5_ncsi_defconfig | 3 +++
> > configs/ast_g5_phy_defconfig | 3 +++
> > 4 files changed, 12 insertions(+)
> >
> > diff --git a/configs/ast_g4_ncsi_defconfig
> b/configs/ast_g4_ncsi_defconfig
> > index 4ee71c5..70cd3c2 100644
> > --- a/configs/ast_g4_ncsi_defconfig
> > +++ b/configs/ast_g4_ncsi_defconfig
> > @@ -3,5 +3,8 @@ CONFIG_TARGET_AST_G4=y
> > CONFIG_SYS_PROMPT="ast# "
> > CONFIG_CMD_DHCP=y
> > CONFIG_CMD_PING=y
> > +CONFIG_FIT=y
> > +CONFIG_FIT_VERBOSE=y
> > +CONFIG_OF_LIBFDT=y
> > CONFIG_SPI_FLASH=y
> > CONFIG_SYS_NS16550=y
> > diff --git a/configs/ast_g4_phy_defconfig b/configs/ast_g4_phy_defconfig
> > index 61fd69b..468fbc4 100644
> > --- a/configs/ast_g4_phy_defconfig
> > +++ b/configs/ast_g4_phy_defconfig
> > @@ -4,5 +4,8 @@ CONFIG_ASPEED_NET_PHY=y
> > CONFIG_SYS_PROMPT="ast# "
> > CONFIG_CMD_DHCP=y
> > CONFIG_CMD_PING=y
> > +CONFIG_FIT=y
> > +CONFIG_FIT_VERBOSE=y
> > +CONFIG_OF_LIBFDT=y
> > CONFIG_SPI_FLASH=y
> > CONFIG_SYS_NS16550=y
> > diff --git a/configs/ast_g5_ncsi_defconfig
> b/configs/ast_g5_ncsi_defconfig
> > index 6d11afb..8a9c297 100644
> > --- a/configs/ast_g5_ncsi_defconfig
> > +++ b/configs/ast_g5_ncsi_defconfig
> > @@ -3,5 +3,8 @@ CONFIG_TARGET_AST_G5=y
> > CONFIG_SYS_PROMPT="ast# "
> > CONFIG_CMD_DHCP=y
> > CONFIG_CMD_PING=y
> > +CONFIG_FIT=y
> > +CONFIG_FIT_VERBOSE=y
> > +CONFIG_OF_LIBFDT=y
> > CONFIG_SPI_FLASH=y
> > CONFIG_SYS_NS16550=y
> > diff --git a/configs/ast_g5_phy_defconfig b/configs/ast_g5_phy_defconfig
> > index 20f62e0..fd450b9 100644
> > --- a/configs/ast_g5_phy_defconfig
> > +++ b/configs/ast_g5_phy_defconfig
> > @@ -4,5 +4,8 @@ CONFIG_ASPEED_NET_PHY=y
> > CONFIG_SYS_PROMPT="ast# "
> > CONFIG_CMD_DHCP=y
> > CONFIG_CMD_PING=y
> > +CONFIG_FIT=y
> > +CONFIG_FIT_VERBOSE=y
> > +CONFIG_OF_LIBFDT=y
> > CONFIG_SPI_FLASH=y
> > CONFIG_SYS_NS16550=y
> > --
> > 2.8.0.rc3.226.g39d4020
> >
> > _______________________________________________
> > openbmc mailing list
> > openbmc@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/openbmc
>
[-- Attachment #2: Type: text/html, Size: 8137 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH u-boot v2] Enable FIT image support and FDT loading for AST2400/AST2500
2016-11-10 0:00 ` Rick Altherr
@ 2016-11-10 11:55 ` Cédric Le Goater
2016-11-10 14:32 ` Cédric Le Goater
0 siblings, 1 reply; 6+ messages in thread
From: Cédric Le Goater @ 2016-11-10 11:55 UTC (permalink / raw)
To: Rick Altherr, Joel Stanley; +Cc: OpenBMC Maillist
On 11/10/2016 01:00 AM, Rick Altherr wrote:
>
>
> On Tue, Nov 1, 2016 at 11:31 PM, Joel Stanley <joel@jms.id.au <mailto:joel@jms.id.au>> wrote:
>
> Hi Rick,
>
> On Wed, Nov 2, 2016 at 3:56 PM, Rick Altherr <raltherr@google.com <mailto:raltherr@google.com>> wrote:
> > FIT is the modern u-boot native image format for kernels, device trees,
> > and ramdisks. Enabling FIT only compiles in support for the image
> > format. For these devices, the kernel+dtb and ramdisk are loaded from
> > separate locations in flash and can be any mix of legacy or FIT images.
> > When using FIT images, the dtb is stored as a separate entry that
> > requires CONFIG_OF_LIBFDT to load it into RAM and pass it to the kernel.
> >
> > Tested under qemu with both legacy and FIT kernel+dtb images for
> > palmetto and witherspoon.
>
> The changes look good to me.
>
> Is the FIT support in v2016.07 new enough for us? Is there any reason
> to move to a v2016.09 base?
>
>
> AFAIK, v2016.07 is new enough. It certainly includes FIT, FDT loading, and FIT signature checking.
>
>
> It does introduce a warning in a hack we carry for loading the
> initramfs, which I think points to a legitimate issue:
>
> common/image.c: In function ‘boot_get_ramdisk’:
> common/image.c:1078:4: warning: ‘rd_load’ may be used uninitialized in
> this function [-Wmaybe-uninitialized]
> memmove ((void *)rd_load, (uchar *)rd_data, rd_len);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This code an Aspeed hack we carry. It is required to copy the
> initramfs from flash. Cedric was doing some investigation into why we
> need the hack, and it appears to be an upstream bug.
>
> If I read the code correctly, prior to enabling FIT we only accepted
> IMAGE_FORMAT_LEGACY. If we move the hack to be part of that case we
> should be ok, as we don't need to do the hack when loading from FIT.
>
> Can you try that and add a second patch to your series that moves the hack?
>
> Looks like Cedric has a patch that just removes the hack. I'll look into this more and add it to my series.
Thanks. tell us how it goes. I think we should be able to remove the hack
for the ramdisk if we use fdt.
C.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH u-boot v2] Enable FIT image support and FDT loading for AST2400/AST2500
2016-11-10 11:55 ` Cédric Le Goater
@ 2016-11-10 14:32 ` Cédric Le Goater
0 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2016-11-10 14:32 UTC (permalink / raw)
To: Rick Altherr, Joel Stanley; +Cc: OpenBMC Maillist
>> It does introduce a warning in a hack we carry for loading the
>> initramfs, which I think points to a legitimate issue:
>>
>> common/image.c: In function ‘boot_get_ramdisk’:
>> common/image.c:1078:4: warning: ‘rd_load’ may be used uninitialized in
>> this function [-Wmaybe-uninitialized]
>> memmove ((void *)rd_load, (uchar *)rd_data, rd_len);
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> This code an Aspeed hack we carry. It is required to copy the
>> initramfs from flash. Cedric was doing some investigation into why we
>> need the hack, and it appears to be an upstream bug.
>>
>> If I read the code correctly, prior to enabling FIT we only accepted
>> IMAGE_FORMAT_LEGACY. If we move the hack to be part of that case we
>> should be ok, as we don't need to do the hack when loading from FIT.
>>
>> Can you try that and add a second patch to your series that moves the hack?
>>
>> Looks like Cedric has a patch that just removes the hack. I'll look into this more and add it to my series.
>
> Thanks. tell us how it goes. I think we should be able to remove the hack
> for the ramdisk if we use fdt.
Talking of hacks, I think I have killed this ugly one :
https://github.com/legoater/u-boot/commit/c17a8847ca6c1fa8b00c16bd78a70a61cc1f0569
Here is my branch based on a v2016.11-rc3+
https://github.com/legoater/u-boot/commits/v2016.11-aspeed-openbmc
in which I removed the mmu disablement hack. I only tested it on
a AST2500 evb. Could some one (with a flash programmer) confirm
on a board with the same SoC ?
I think that device tree support will remove the need of ramdisk
hacks. From there, the only (major) problem left is the DRAM
calibration algo which needs to be rewritten in C.
Did a wiki update :
https://github.com/openbmc/u-boot/wiki
Cheers,
C.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-10 15:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-02 5:26 [PATCH u-boot v2] Enable FIT image support and FDT loading for AST2400/AST2500 Rick Altherr
2016-11-02 6:31 ` Joel Stanley
2016-11-02 16:55 ` Cédric Le Goater
2016-11-10 0:00 ` Rick Altherr
2016-11-10 11:55 ` Cédric Le Goater
2016-11-10 14:32 ` Cédric Le Goater
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.