tegra: Expand Nyan-big support

Only set the speed if it has changed from last time. Since the speed will be 0 when the device is probed it will always be changed on the first transfer after the device is probed.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/spi/spi-uclass.c | 9 ++++++---
 include/spi.h            | 3 +++
 2 files changed, 9 insertions(+), 3 deletions(-)

When starting up, show the protocol version that has been negotiated with the EC.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/misc/cros_ec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

With several chips using the SPI protocol it seems better to put the single duplex functionality in the EC rather than the SPI driver.

Signed-off-by Simon Glass <[email protected]>

---

 drivers/misc/cros_ec_spi.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Add an implementation of the timer functions for tegra, so that timing is more accurate. Tegra has a 1 microsecond timer for this purpose.

Quoted 3 lines · Simon Glass

Add an implementation of the timer functions for tegra, so that timing is more accurate. Tegra has a 1 microsecond timer for this purpose.

I'm a bit confused about this:

include/configs/tegra-common.h:32:#define CONFIG_SYS_TIMER_COUNTER NV_PA_TMRUS_BASE

lib/time.c:

#ifdef CONFIG_SYS_TIMER_COUNTER
unsigned long notrace timer_read_counter(void)
{

#ifdef CONFIG_SYS_TIMER_COUNTS_DOWN
        return ~readl(CONFIG_SYS_TIMER_COUNTER);
#else
        return readl(CONFIG_SYS_TIMER_COUNTER);

#endif
}

Doesn't that provide the same set of features, without requiring Tegra-specific code?

At present the driver does not properly honour the requested SPI CS deactivation delay since the SPI bus is changed in the claim_bus() method.

Everything the claim_bus() method does can be done when the device is probed (setting the speed and mode) and at the start of a new transfer (where the fifo_status is already cleared). So drop this method.

Also, until the delay is complete, we should not touch the bus, so make sure that spi_cs_activate() is called before other things are done in the xfer() method.

drivers/spi/tegra114_spi.c +0 -2

 
-	setbits_le32(&regs->command1, SPI_CMD1_M_S | SPI_CMD1_CS_SW_HW |
-		     (priv->mode << SPI_CMD1_MODE_SHIFT));

Quoted 21 lines · Simon Glass

At present the driver does not properly honour the requested SPI CS deactivation delay since the SPI bus is changed in the claim_bus() method.

Everything the claim_bus() method does can be done when the device is probed
(setting the speed and mode) and at the start of a new transfer (where the fifo_status is already cleared). So drop this method.

Also, until the delay is complete, we should not touch the bus, so make sure
that spi_cs_activate() is called before other things are done in the xfer()
method.

drivers/spi/tegra114_spi.c +0 -2

 
 
-       setbits_le32(&regs->command1, SPI_CMD1_M_S | SPI_CMD1_CS_SW_HW |
-                    (priv->mode << SPI_CMD1_MODE_SHIFT));

...

Quoted 1 line

Quoted 4 lines

+ setbits_le32(&priv->regs->command1, SPI_CMD1_M_S |
SPI_CMD1_CS_SW_HW |
+ (priv->mode << SPI_CMD1_MODE_SHIFT) |
SPI_CMD1_CS_SW_VAL);

Is that addition of SPI_CMD1_CS_SW_VAL there relative to the old code intended?

Enable the EC and keyboard, using the SPI bus.

The EC driver requires a particular format and a deactivation delay. Also U-Boot does not support interrupts.

For now, adjust the device tree to comply. At some point we should tidy this up to support interrupts and make tegra and exynos use the same setup.

Quoted 14 lines · Simon Glass

Enable the EC and keyboard, using the SPI bus.

The EC driver requires a particular format and a deactivation delay. Also U-Boot does not support interrupts.

For now, adjust the device tree to comply. At some point we should tidy this up to support interrupts and make tegra and exynos use the same setup.

arch/arm/dts/tegra124-nyan-big.dts +0 -0

arch/arm/dts/tegra124-nyan-big.dts line 170

Quoted 10 lines

arch/arm/dts/tegra124-nyan-big.dts +3 -2

 
        spi at 7000d400 {
                status = "okay";
+               spi-deactivate-delay = <200>;
+               spi-max-frequency = <3000000>;
 
                cros_ec: cros-ec at 0 {
-                       compatible = "google,cros-ec-spi";
-                       spi-max-frequency = <3000000>;
+                       compatible = "google,cros-ec";

I recall some discussion that implied both the SPI bus/controller node and the SPI device node both needed an spi-max-frequency property. Can you double-check this change conforms with the DT binding, and whatever Linux has in its DT files?

We need to turn on all audio-related clocks for the kernel to boot. Otherwise it will hang when trying to enable audio.

Also for Linux set up the ODMDATA and graphics driver video protection.

Quoted 3 lines · Simon Glass

We need to turn on all audio-related clocks for the kernel to boot. Otherwise it will hang when trying to enable audio.

This certainly isn't true for the upstream kernel; is this some bug in the ChromeOS kernel? If so, we should explicitly call this out in the commit description.

Quoted 2 lines

Also for Linux set up the ODMDATA and graphics driver video protection.

Why doesn't ODMDATA come from the BCT? The way this is suppose to work is that the boot ROM copies the BCT into IRAM, and U-Boot (or indeed any bootloader) copies the ODMDATA field from the BCT in IRAM into the PMC scratch20 register. This logic is already all in place in U-Boot, and indeed any NVIDIA-authored bootloader AFAIK.

Is this U-Boot port intended to run as a Coreboot payload rather than natively, and Coreboot is somehow corrupting the copy of the BCT in IRAM? If so, we should explicitly call this out in the commit description.

I would personally want to (be able to) make my SPI flash r/w and replace Coreboot with U-Boot. Perhaps we need different board names for those two use-cases; something like nyan-big for the Coreboot payload, and nyan-big-native for the version you'd write directly into SPI?

From: Doug Anderson <[email protected]>

This header includes useful scripts which can be used with any board that can boot Chrome OS.

In particular, 'run nvboot' will boot a board without verified boot enabled.

+#define CONFIG_CROS_FULL

include/configs/chromeos.h line 53

Quoted 7 lines

include/configs/chromeos.h +5 -0

 
+/* Stringify a token */
+#ifndef STRINGIFY
+#define _STRINGIFY(x)	#x
+#define STRINGIFY(x)	_STRINGIFY(x)
+#endif

Shouldn't that be in some common header so it isn't ever duplicated?

Quoted 3 lines · Simon Glass

From: Doug Anderson <[email protected]>

Quoted 4 lines

This header includes useful scripts which can be used with any board that can boot Chrome OS.

In particular, 'run nvboot' will boot a board without verified boot enabled.

I think this needs a bit of minification for an upstream U-Boot. In particular, many of the environment variables overlap semantically or by name with those from include/config_distro_*.h, and I'd like to see any U-Boot for the Tegra Chromebooks (at least) support booting both ChromeOS kernels and arbitrary distros using the environment from config_distro_bootcmd.h.

Quoted 2 lines

include/configs/chromeos.h +0 -0

include/configs/chromeos.h line 53

Quoted 5 lines

include/configs/chromeos.h +5 -0

+/* Stringify a token */
+#ifndef STRINGIFY
+#define _STRINGIFY(x)  #x
+#define STRINGIFY(x)   _STRINGIFY(x)
+#endif

Shouldn't that be in some common header so it isn't ever duplicated?

Quoted 2 lines

+#define CONFIG_CROS_FULL

There are a ton of macros in here without much in the way of explanation. Shouldn't everything be documented in the README?

Hi Stephen,

Quoted 8 lines · Stephen Warren

Quoted 6 lines · Simon Glass

From: Doug Anderson <[email protected]>

This header includes useful scripts which can be used with any board that can boot Chrome OS.

Quoted 8 lines

Quoted 1 line

In particular, 'run nvboot' will boot a board without verified boot enabled.

I think this needs a bit of minification for an upstream U-Boot. In particular, many of the environment variables overlap semantically or by name with those from include/config_distro_*.h, and I'd like to see any U-Boot for the Tegra Chromebooks (at least) support booting both ChromeOS kernels and arbitrary distros using the environment from config_distro_bootcmd.h.

That would be good. I think it is possible also, although I haven't been able to make it work so far. I can get it to find the kernel, although not the one that it needs and the U-Boot scripts are in a different place on the disk in Chrome OS. Also the command line args are wrong.

So I think for now we should either apply this as it is (since it works) or wait until someone has time to rationalise things a bit and figure out how to support Chrome OS booting in the distro stuff.

Quoted 5 lines

Quoted 1 line

+#define CONFIG_CROS_FULL

There are a ton of macros in here without much in the way of explanation. Shouldn't everything be documented in the README?

The documentation for these is at the top of the file:

+ * In anticipation of implementing early firmware selection, these defines
+ * signal the features that are required in U-Boot.

Quoted 29 lines · Simon Glass

Hi Stephen,

Quoted 15 lines · Stephen Warren

Quoted 10 lines · Simon Glass

From: Doug Anderson <[email protected]>

This header includes useful scripts which can be used with any board that
can boot Chrome OS.

In particular, 'run nvboot' will boot a board without verified boot enabled.

Quoted 7 lines

I think this needs a bit of minification for an upstream U-Boot. In

particular, many of the environment variables overlap semantically or by name with those from include/config_distro_*.h, and I'd like to see any U-Boot for the Tegra Chromebooks (at least) support booting both ChromeOS kernels and arbitrary distros using the environment from config_distro_bootcmd.h.

Quoted 5 lines

That would be good. I think it is possible also, although I haven't been able to make it work so far. I can get it to find the kernel, although not the one that it needs and the U-Boot scripts are in a different place on the disk in Chrome OS. Also the command line args are wrong.

I would not expect the scripts from config_distro_bootcmd.h to just work for ChromeOS; I'd expect it to be an alternate set of scripts. I'm mainly looking for the ChromeOS scripts to use the same variables where they mean the same thing, to avoid having two completely different sets.

Re: the command-line; wouldn't the ChromeOS scripts just set bootargs from scratch whenever they were used? The scripts in config_distro_bootcmd.h expect the complete command-line to come from the extlinux.conf or boot.scr file that they load; the scripts in config_distro_bootcmd.h shouldn't be providing a command-line.

Quoted 4 lines

So I think for now we should either apply this as it is (since it works) or wait until someone has time to rationalise things a bit and figure out how to support Chrome OS booting in the distro stuff.

I really think this should be cleaned up before it's applied. As soon as it's applied, it'll serve as an example and other boards will pick up the same code. That'll make the eventual required cleanup more involved.

Quoted 7 lines

Quoted 6 lines

Quoted 2 lines

+#define CONFIG_CROS_FULL

There are a ton of macros in here without much in the way of explanation.

Quoted 8 lines

Quoted 1 line

Shouldn't everything be documented in the README?

The documentation for these is at the top of the file:

+ * In anticipation of implementing early firmware selection, these defines
+ * signal the features that are required in U-Boot.

Not all of them by far; I mean things like CONFIG_CHROMEOS, CONFIG_CHROMEOS_TEST, CONFIG_CHROMEOS_CONST_FLAG, CONFIG_VBOOT_REGION_READ, CONFIG_CROS_RO, CONFIG_CHROMEOS_DISPLAY, CONFIG_CROS_LEGACY_VBOOT, CONFIG_CHROMEOS_USB, and probably a variety of others I didn't pick up in a quick scan. I don't see any of those in u-boot/master as of a few days ago either, so I think they're all introduced new in this patch?

We need to match the device tree in the FIT with the U-Boot model so we can automatically select the right device tree. Also adjust the load address so that the device tree is not in the way when a zImage kernel tries to extract itself.

Quoted 6 lines · Simon Glass

We need to match the device tree in the FIT with the U-Boot model so we can automatically select the right device tree. Also adjust the load address
so that the device tree is not in the way when a zImage kernel tries to extract itself.

We don't tend to use LOADADDR in any of the default boot scripts any more. Rather, we explicitly load files to a "semantic" location indicated by one of the following variables in tegra124-common.h:

#define MEM_LAYOUT_ENV_SETTINGS \
        "scriptaddr=0x90000000\0" \
        "pxefile_addr_r=0x90100000\0" \
        "kernel_addr_r=0x81000000\0" \
        "fdt_addr_r=0x82000000\0" \
        "ramdisk_addr_r=0x82100000\0"

Perhaps the ChromeOS boot scripts could be adjusted to use one/some of those variables?

If the value of CONFIG_LOADADDR isn't appropriate, perhaps we should fix it for all Tegra SoCs/boards?

Hi Stephen,

Quoted 20 lines · Stephen Warren

Quoted 6 lines · Simon Glass

We need to match the device tree in the FIT with the U-Boot model so we can automatically select the right device tree. Also adjust the load address
so that the device tree is not in the way when a zImage kernel tries to extract itself.

We don't tend to use LOADADDR in any of the default boot scripts any more. Rather, we explicitly load files to a "semantic" location indicated by one of the following variables in tegra124-common.h:

#define MEM_LAYOUT_ENV_SETTINGS \
         "scriptaddr=0x90000000\0" \
         "pxefile_addr_r=0x90100000\0" \
         "kernel_addr_r=0x81000000\0" \
         "fdt_addr_r=0x82000000\0" \
         "ramdisk_addr_r=0x82100000\0"

Quoted 5 lines

Perhaps the ChromeOS boot scripts could be adjusted to use one/some of those variables?

If the value of CONFIG_LOADADDR isn't appropriate, perhaps we should fix it for all Tegra SoCs/boards?

I forgot about this comment sorry. I had problems with the image overwriting itself. It is a bzImage inside a FIT so doesn't use the proper FIT decompression.

Anyway I'd like to clarify what is meant by kernel_addr_r. Is that where the FIT is loaded or where the kernel will decompress to, or something else?

Quoted 39 lines · Simon Glass

Hi Stephen,

Quoted 25 lines · Stephen Warren

Quoted 7 lines · Simon Glass

We need to match the device tree in the FIT with the U-Boot model so we can automatically select the right device tree. Also adjust the load address
so that the device tree is not in the way when a zImage kernel tries to extract itself.

We don't tend to use LOADADDR in any of the default boot scripts any more.
Rather, we explicitly load files to a "semantic" location indicated by one
of the following variables in tegra124-common.h:

#define MEM_LAYOUT_ENV_SETTINGS \
         "scriptaddr=0x90000000\0" \
         "pxefile_addr_r=0x90100000\0" \
         "kernel_addr_r=0x81000000\0" \
         "fdt_addr_r=0x82000000\0" \
         "ramdisk_addr_r=0x82100000\0"

Quoted 7 lines

Perhaps the ChromeOS boot scripts could be adjusted to use one/some of those
variables?

If the value of CONFIG_LOADADDR isn't appropriate, perhaps we should fix it
for all Tegra SoCs/boards?

Quoted 7 lines

I forgot about this comment sorry. I had problems with the image overwriting itself. It is a bzImage inside a FIT so doesn't use the proper FIT decompression.

Anyway I'd like to clarify what is meant by kernel_addr_r. Is that where the FIT is loaded or where the kernel will decompress to, or something else?

kernel_addr_r is the address in RAM at which a kernel zImage is loaded by config_distro_bootcmd.h scripts (and likely other scripts too). It's usually deliberately chosen to be a fair way into RAM, so that when the zImage decompresses (to approx the start of RAM), the decompressed image doesn't overlap the compressed image at kernel_addr_r. This avoids the kernel decompressor having to move/copy the zImage somewhere else before copying to avoid any overlap during decompression.

config_distro_bootcmd.h doesn't support FIT, so I haven't tried out loading FIT images. That said, if U-Boot simply jumps to the location of the kernel in the FIT image itself without any copying, I would expect everything to work fine if you loaded a FIT image to kernel_addr_r. However, if the processing of the FIT image requires the kernel to be copied out of the FIT image to some other location, you'd have to carefully choose that location so it didn't overlap anything. I'd strongly recommend avoiding any unnecessary copies like that though, if at all possible, from the perspective of both pointlessly wasted execution time and simplicity of the boot process. Perhaps storing a a kernel_noload image type inside the FIT rather than a kernel image type might help there? Perhaps you want to introduce a new standard variable that defines where FIT images are loaded.

Hi Stephen,

Quoted 57 lines · Stephen Warren

Quoted 39 lines · Simon Glass

Hi Stephen,

Quoted 25 lines · Stephen Warren

Quoted 7 lines · Simon Glass

We need to match the device tree in the FIT with the U-Boot model so we can automatically select the right device tree. Also adjust the load address
so that the device tree is not in the way when a zImage kernel tries to extract itself.

We don't tend to use LOADADDR in any of the default boot scripts any more.
Rather, we explicitly load files to a "semantic" location indicated by one
of the following variables in tegra124-common.h:

#define MEM_LAYOUT_ENV_SETTINGS \
          "scriptaddr=0x90000000\0" \
          "pxefile_addr_r=0x90100000\0" \
          "kernel_addr_r=0x81000000\0" \
          "fdt_addr_r=0x82000000\0" \
          "ramdisk_addr_r=0x82100000\0"

Quoted 7 lines

Perhaps the ChromeOS boot scripts could be adjusted to use one/some of those
variables?

If the value of CONFIG_LOADADDR isn't appropriate, perhaps we should fix it
for all Tegra SoCs/boards?

Quoted 7 lines

I forgot about this comment sorry. I had problems with the image overwriting itself. It is a bzImage inside a FIT so doesn't use the proper FIT decompression.

Anyway I'd like to clarify what is meant by kernel_addr_r. Is that where the FIT is loaded or where the kernel will decompress to, or something else?

kernel_addr_r is the address in RAM at which a kernel zImage is loaded by config_distro_bootcmd.h scripts (and likely other scripts too). It's usually deliberately chosen to be a fair way into RAM, so that when the zImage decompresses (to approx the start of RAM), the decompressed image doesn't overlap the compressed image at kernel_addr_r. This avoids the kernel decompressor having to move/copy the zImage somewhere else before copying to avoid any overlap during decompression.

Quoted 12 lines

config_distro_bootcmd.h doesn't support FIT, so I haven't tried out loading FIT images. That said, if U-Boot simply jumps to the location of the kernel in the FIT image itself without any copying, I would expect everything to work fine if you loaded a FIT image to kernel_addr_r. However, if the processing of the FIT image requires the kernel to be copied out of the FIT image to some other location, you'd have to carefully choose that location so it didn't overlap anything. I'd strongly recommend avoiding any unnecessary copies like that though, if at all possible, from the perspective of both pointlessly wasted execution time and simplicity of the boot process. Perhaps storing a a kernel_noload image type inside the FIT rather than a kernel image type might help there? Perhaps you want to introduce a new standard variable that defines where FIT images are loaded.

I wonder what would be involved in adjusting config_distro_bootcmd to support FIT?

Would it make it simpler or harder? To me, we should be moving to using FIT with U-Boot as it is much more flexible and better designed from the ground up to provide the required features.

Anyway, normally FIT has a compressed kernel (e.g. with LZO), not a bzImage.

So it seems like we need an additional decompression address. I suppose we could always use malloc() instead... But perhaps a FIT load address makes sense. But then we don't really need a kernel load address do we? Shouldn't that be specified in the FIT itself?

Quoted 93 lines · Simon Glass

Hi Stephen,

Quoted 79 lines · Stephen Warren

Quoted 47 lines · Simon Glass

Hi Stephen,

Quoted 30 lines · Stephen Warren

Quoted 10 lines · Simon Glass

We need to match the device tree in the FIT with the U-Boot model so we
can automatically select the right device tree. Also adjust the load address
so that the device tree is not in the way when a zImage kernel tries to
extract itself.

We don't tend to use LOADADDR in any of the default boot scripts any more.
Rather, we explicitly load files to a "semantic" location indicated by one
of the following variables in tegra124-common.h:

#define MEM_LAYOUT_ENV_SETTINGS \
          "scriptaddr=0x90000000\0" \
          "pxefile_addr_r=0x90100000\0" \
          "kernel_addr_r=0x81000000\0" \
          "fdt_addr_r=0x82000000\0" \
          "ramdisk_addr_r=0x82100000\0"

Quoted 8 lines

Perhaps the ChromeOS boot scripts could be adjusted to use one/some of those
variables?

If the value of CONFIG_LOADADDR isn't appropriate, perhaps we should fix
it
for all Tegra SoCs/boards?

Quoted 7 lines

I forgot about this comment sorry. I had problems with the image overwriting itself. It is a bzImage inside a FIT so doesn't use the proper FIT decompression.

Anyway I'd like to clarify what is meant by kernel_addr_r. Is that where the FIT is loaded or where the kernel will decompress to, or something else?

kernel_addr_r is the address in RAM at which a kernel zImage is loaded by config_distro_bootcmd.h scripts (and likely other scripts too). It's usually
deliberately chosen to be a fair way into RAM, so that when the zImage decompresses (to approx the start of RAM), the decompressed image doesn't overlap the compressed image at kernel_addr_r. This avoids the kernel decompressor having to move/copy the zImage somewhere else before copying to
avoid any overlap during decompression.

config_distro_bootcmd.h doesn't support FIT, so I haven't tried out loading
FIT images. That said, if U-Boot simply jumps to the location of the kernel
in the FIT image itself without any copying, I would expect everything to work fine if you loaded a FIT image to kernel_addr_r. However, if the processing of the FIT image requires the kernel to be copied out of the FIT
image to some other location, you'd have to carefully choose that location

Quoted 8 lines

so it didn't overlap anything. I'd strongly recommend avoiding any unnecessary copies like that though, if at all possible, from the perspective of both pointlessly wasted execution time and simplicity of the
boot process. Perhaps storing a a kernel_noload image type inside the FIT rather than a kernel image type might help there? Perhaps you want to introduce a new standard variable that defines where FIT images are loaded.

Quoted 2 lines

I wonder what would be involved in adjusting config_distro_bootcmd to support FIT?

Well, it goes against the very idea of config_distro_bootcmd, which is to provide a single standard mechanism that doesn't rely on any bootloader-specific file formats etc. That mechanism is a raw zImage, a raw initrd, and a plain text extlinux.cfg file to specify things like file paths/names, command-line, etc.

The boot.scr support there is legacy, and not something that should be built upon going forward. So, that's not an argument for adding support for a third mechanism!

Quoted 4 lines

Would it make it simpler or harder? To me, we should be moving to using FIT with U-Boot as it is much more flexible and better designed from the ground up to provide the required features.

I disagree that FIT is a good idea, but that's a separate topic.

Quoted 2 lines

Anyway, normally FIT has a compressed kernel (e.g. with LZO), not a bzImage.

Do you mean FIT normally contains an originally uncompressed kernel (i.e. an Image file in ARM land at least), but then compresses it itself as part of FIT image generation? A bzImage is also a "compressed kernel".

Quoted 6 lines

So it seems like we need an additional decompression address.

I suppose we could always use malloc() instead... But perhaps a FIT load address makes sense. But then we don't really need a kernel load address do we? Shouldn't that be specified in the FIT itself?

A FIT load address does sound like it makes sense.

If U-Boot copies the kernel image out of the FIT image to somewhere else, the FIT image shouldn't specify the address to copy it to. This varies per SoC, so if this address is hard-coded into FIT, it means the FIT image can't be used on different SoCs (or perhaps even different boards, depending on how differing RAM sizes work on various HW). This is why with config_distro_bootcmd, all the addresses come from the U-Boot environment, not hard-coded into a file on the disk. That way, the files are all generic and can be used on various different systems that require different addresses. It possibly makes sense for kernel_addr_r to be the destination of that copy?

Hi Stephen,

Quoted 104 lines · Stephen Warren

Quoted 93 lines · Simon Glass

Hi Stephen,

Quoted 79 lines · Stephen Warren

Quoted 47 lines · Simon Glass

Hi Stephen,

Quoted 30 lines · Stephen Warren

Quoted 10 lines · Simon Glass

We need to match the device tree in the FIT with the U-Boot model so we
can automatically select the right device tree. Also adjust the load address
so that the device tree is not in the way when a zImage kernel tries to
extract itself.

We don't tend to use LOADADDR in any of the default boot scripts any more.
Rather, we explicitly load files to a "semantic" location indicated by one
of the following variables in tegra124-common.h:

#define MEM_LAYOUT_ENV_SETTINGS \
           "scriptaddr=0x90000000\0" \
           "pxefile_addr_r=0x90100000\0" \
           "kernel_addr_r=0x81000000\0" \
           "fdt_addr_r=0x82000000\0" \
           "ramdisk_addr_r=0x82100000\0"

Quoted 8 lines

Perhaps the ChromeOS boot scripts could be adjusted to use one/some of those
variables?

If the value of CONFIG_LOADADDR isn't appropriate, perhaps we should fix
it
for all Tegra SoCs/boards?

Quoted 7 lines

I forgot about this comment sorry. I had problems with the image overwriting itself. It is a bzImage inside a FIT so doesn't use the proper FIT decompression.

Anyway I'd like to clarify what is meant by kernel_addr_r. Is that where the FIT is loaded or where the kernel will decompress to, or something else?

kernel_addr_r is the address in RAM at which a kernel zImage is loaded by config_distro_bootcmd.h scripts (and likely other scripts too). It's usually
deliberately chosen to be a fair way into RAM, so that when the zImage decompresses (to approx the start of RAM), the decompressed image doesn't overlap the compressed image at kernel_addr_r. This avoids the kernel decompressor having to move/copy the zImage somewhere else before copying to
avoid any overlap during decompression.

config_distro_bootcmd.h doesn't support FIT, so I haven't tried out loading
FIT images. That said, if U-Boot simply jumps to the location of the kernel
in the FIT image itself without any copying, I would expect everything to work fine if you loaded a FIT image to kernel_addr_r. However, if the processing of the FIT image requires the kernel to be copied out of the FIT
image to some other location, you'd have to carefully choose that location

Quoted 8 lines

so it didn't overlap anything. I'd strongly recommend avoiding any unnecessary copies like that though, if at all possible, from the perspective of both pointlessly wasted execution time and simplicity of the
boot process. Perhaps storing a a kernel_noload image type inside the FIT rather than a kernel image type might help there? Perhaps you want to introduce a new standard variable that defines where FIT images are loaded.

Quoted 2 lines

I wonder what would be involved in adjusting config_distro_bootcmd to support FIT?

Well, it goes against the very idea of config_distro_bootcmd, which is to provide a single standard mechanism that doesn't rely on any bootloader-specific file formats etc. That mechanism is a raw zImage, a raw initrd, and a plain text extlinux.cfg file to specify things like file paths/names, command-line, etc.

Quoted 3 lines

The boot.scr support there is legacy, and not something that should be built upon going forward. So, that's not an argument for adding support for a third mechanism!

Do we need to adjust the mechanism? The only difference I see is that FIT brings the files together.

Quoted 7 lines

Quoted 3 lines

Would it make it simpler or harder? To me, we should be moving to using FIT with U-Boot as it is much more flexible and better designed from the ground up to provide the required features.

I disagree that FIT is a good idea, but that's a separate topic.

Quoted 7 lines

Quoted 2 lines

Anyway, normally FIT has a compressed kernel (e.g. with LZO), not a bzImage.

Do you mean FIT normally contains an originally uncompressed kernel (i.e. an Image file in ARM land at least), but then compresses it itself as part of FIT image generation? A bzImage is also a "compressed kernel".

That's right, that's what I mean.

Quoted 10 lines

Quoted 6 lines

So it seems like we need an additional decompression address.

I suppose we could always use malloc() instead... But perhaps a FIT load address makes sense. But then we don't really need a kernel load address do we? Shouldn't that be specified in the FIT itself?

A FIT load address does sound like it makes sense.

Quoted 10 lines

If U-Boot copies the kernel image out of the FIT image to somewhere else, the FIT image shouldn't specify the address to copy it to. This varies per SoC, so if this address is hard-coded into FIT, it means the FIT image can't be used on different SoCs (or perhaps even different boards, depending on how differing RAM sizes work on various HW). This is why with config_distro_bootcmd, all the addresses come from the U-Boot environment, not hard-coded into a file on the disk. That way, the files are all generic and can be used on various different systems that require different addresses. It possibly makes sense for kernel_addr_r to be the destination of that copy?

Doesn't the kernel know where the kernel needs to be loaded / copied as part of the bzImage stuff? From what I see at present this is hard-coded into the code in the kernel. So we need to put this correct address in the FIT, is that right?

Are you thinking we should allow
FIT to take an environment variable as a load address?

If so, I feel it would make a lot of sense for the kernel to package up the FIT to avoid these issues.

Quoted 127 lines · Simon Glass

Hi Stephen,

Quoted 123 lines · Stephen Warren

Quoted 109 lines · Simon Glass

Hi Stephen,

Quoted 93 lines · Stephen Warren

Quoted 54 lines · Simon Glass

Hi Stephen,

Quoted 39 lines · Stephen Warren

Quoted 11 lines · Simon Glass

We need to match the device tree in the FIT with the U-Boot model so we
can automatically select the right device tree. Also adjust the load address
so that the device tree is not in the way when a zImage kernel tries to
extract itself.

We don't tend to use LOADADDR in any of the default boot scripts any more.
Rather, we explicitly load files to a "semantic" location indicated by
one
of the following variables in tegra124-common.h:

#define MEM_LAYOUT_ENV_SETTINGS \
           "scriptaddr=0x90000000\0" \
           "pxefile_addr_r=0x90100000\0" \
           "kernel_addr_r=0x81000000\0" \
           "fdt_addr_r=0x82000000\0" \
           "ramdisk_addr_r=0x82100000\0"

Perhaps the ChromeOS boot scripts could be adjusted to use one/some of
those
variables?

Quoted 4 lines

If the value of CONFIG_LOADADDR isn't appropriate, perhaps we should fix
it
for all Tegra SoCs/boards?

Quoted 7 lines

I forgot about this comment sorry. I had problems with the image overwriting itself. It is a bzImage inside a FIT so doesn't use the proper FIT decompression.

Anyway I'd like to clarify what is meant by kernel_addr_r. Is that where the FIT is loaded or where the kernel will decompress to, or something else?

kernel_addr_r is the address in RAM at which a kernel zImage is loaded by
config_distro_bootcmd.h scripts (and likely other scripts too). It's usually
deliberately chosen to be a fair way into RAM, so that when the zImage decompresses (to approx the start of RAM), the decompressed image doesn't
overlap the compressed image at kernel_addr_r. This avoids the kernel decompressor having to move/copy the zImage somewhere else before copying
to
avoid any overlap during decompression.

config_distro_bootcmd.h doesn't support FIT, so I haven't tried out loading
FIT images. That said, if U-Boot simply jumps to the location of the kernel
in the FIT image itself without any copying, I would expect everything to
work fine if you loaded a FIT image to kernel_addr_r. However, if the processing of the FIT image requires the kernel to be copied out of the FIT
image to some other location, you'd have to carefully choose that location
so it didn't overlap anything. I'd strongly recommend avoiding any

Quoted 8 lines

unnecessary copies like that though, if at all possible, from the perspective of both pointlessly wasted execution time and simplicity of the
boot process. Perhaps storing a a kernel_noload image type inside the FIT
rather than a kernel image type might help there? Perhaps you want to introduce a new standard variable that defines where FIT images are loaded.

Quoted 2 lines

I wonder what would be involved in adjusting config_distro_bootcmd to support FIT?

Well, it goes against the very idea of config_distro_bootcmd, which is to provide a single standard mechanism that doesn't rely on any bootloader-specific file formats etc. That mechanism is a raw zImage, a raw
initrd, and a plain text extlinux.cfg file to specify things like file paths/names, command-line, etc.

Quoted 8 lines

Quoted 4 lines

The boot.scr support there is legacy, and not something that should be built
upon going forward. So, that's not an argument for adding support for a third mechanism!

Do we need to adjust the mechanism? The only difference I see is that FIT brings the files together.

No, it's just fine as it is.

The benefit of the existing mechanism is precisely that nobody has to package the zImage/initrd/... together, whereas that appears to be precisely the purpose of FIT. The two schemes are by definition opposites of each-other.

Quoted 52 lines

Quoted 15 lines

Quoted 5 lines

Would it make it simpler or harder? To me, we should be moving to using FIT with U-Boot as it is much more flexible and better designed from the ground up to provide the required features.

I disagree that FIT is a good idea, but that's a separate topic.

Quoted 2 lines

Anyway, normally FIT has a compressed kernel (e.g. with LZO), not a bzImage.

Quoted 5 lines

Do you mean FIT normally contains an originally uncompressed kernel (i.e. an
Image file in ARM land at least), but then compresses it itself as part of
FIT image generation? A bzImage is also a "compressed kernel".

That's right, that's what I mean.

Quoted 11 lines

Quoted 6 lines

So it seems like we need an additional decompression address.

I suppose we could always use malloc() instead... But perhaps a FIT load address makes sense. But then we don't really need a kernel load address do we? Shouldn't that be specified in the FIT itself?

A FIT load address does sound like it makes sense.

Quoted 15 lines

If U-Boot copies the kernel image out of the FIT image to somewhere else, the FIT image shouldn't specify the address to copy it to. This varies per
SoC, so if this address is hard-coded into FIT, it means the FIT image can't
be used on different SoCs (or perhaps even different boards, depending on how differing RAM sizes work on various HW). This is why with config_distro_bootcmd, all the addresses come from the U-Boot environment,
not hard-coded into a file on the disk. That way, the files are all generic
and can be used on various different systems that require different addresses. It possibly makes sense for kernel_addr_r to be the destination
of that copy?

Quoted 4 lines

Doesn't the kernel know where the kernel needs to be loaded / copied as part of the bzImage stuff? From what I see at present this is hard-coded into the code in the kernel. So we need to put this correct address in the FIT, is that right?

No. The address is dynamically calculated by the kernel decompressor for any kernel with CONFIG_AUTO_ZRELADDR enabled. Any distro-provided kernel will have that option enabled, so that the kernel doesn't have to hard-code any addresses, and so that the same kernel image can run on multiple SoCs.

For kernels without CONFIG_AUTO_ZRELADDR enabled, everything should still work too, provided the decompression target address the kernel has hard-coded into it doesn't overlap stuff like the DTB or initrd.

Quoted 3 lines

Are you thinking we should allow
FIT to take an environment variable as a load address?

That would likely be required for it to be useful with CONFIG_AUTO_ZRELADDR, yes.

Quoted 3 lines

If so, I feel it would make a lot of sense for the kernel to package up the FIT to avoid these issues.

The kernel community has already specifically rejected (multiple times IIRC) generating bootloader-specific image formats in the kernel build system.

+Tom Rini in case he is interested...

Hi Stephen,

Quoted 140 lines · Stephen Warren

Quoted 127 lines · Simon Glass

Hi Stephen,

Quoted 123 lines · Stephen Warren

Quoted 109 lines · Simon Glass

Hi Stephen,

Quoted 93 lines · Stephen Warren

Quoted 54 lines · Simon Glass

Hi Stephen,

Quoted 39 lines · Stephen Warren

Quoted 11 lines · Simon Glass

We need to match the device tree in the FIT with the U-Boot model so we
can automatically select the right device tree. Also adjust the load address
so that the device tree is not in the way when a zImage kernel tries to
extract itself.

We don't tend to use LOADADDR in any of the default boot scripts any more.
Rather, we explicitly load files to a "semantic" location indicated by
one
of the following variables in tegra124-common.h:

#define MEM_LAYOUT_ENV_SETTINGS \
           "scriptaddr=0x90000000\0" \
           "pxefile_addr_r=0x90100000\0" \
           "kernel_addr_r=0x81000000\0" \
           "fdt_addr_r=0x82000000\0" \
           "ramdisk_addr_r=0x82100000\0"

Perhaps the ChromeOS boot scripts could be adjusted to use one/some of
those
variables?

Quoted 4 lines

If the value of CONFIG_LOADADDR isn't appropriate, perhaps we should fix
it
for all Tegra SoCs/boards?

Quoted 7 lines

I forgot about this comment sorry. I had problems with the image overwriting itself. It is a bzImage inside a FIT so doesn't use the proper FIT decompression.

Anyway I'd like to clarify what is meant by kernel_addr_r. Is that where the FIT is loaded or where the kernel will decompress to, or something else?

kernel_addr_r is the address in RAM at which a kernel zImage is loaded by
config_distro_bootcmd.h scripts (and likely other scripts too). It's usually
deliberately chosen to be a fair way into RAM, so that when the zImage decompresses (to approx the start of RAM), the decompressed image doesn't
overlap the compressed image at kernel_addr_r. This avoids the kernel decompressor having to move/copy the zImage somewhere else before copying
to
avoid any overlap during decompression.

config_distro_bootcmd.h doesn't support FIT, so I haven't tried out loading
FIT images. That said, if U-Boot simply jumps to the location of the kernel
in the FIT image itself without any copying, I would expect everything to
work fine if you loaded a FIT image to kernel_addr_r. However, if the processing of the FIT image requires the kernel to be copied out of the FIT
image to some other location, you'd have to carefully choose that location
so it didn't overlap anything. I'd strongly recommend avoiding any

Quoted 8 lines

unnecessary copies like that though, if at all possible, from the perspective of both pointlessly wasted execution time and simplicity of the
boot process. Perhaps storing a a kernel_noload image type inside the FIT
rather than a kernel image type might help there? Perhaps you want to introduce a new standard variable that defines where FIT images are loaded.

Quoted 2 lines

I wonder what would be involved in adjusting config_distro_bootcmd to support FIT?

Well, it goes against the very idea of config_distro_bootcmd, which is to provide a single standard mechanism that doesn't rely on any bootloader-specific file formats etc. That mechanism is a raw zImage, a raw
initrd, and a plain text extlinux.cfg file to specify things like file paths/names, command-line, etc.

Quoted 8 lines

Quoted 4 lines

The boot.scr support there is legacy, and not something that should be built
upon going forward. So, that's not an argument for adding support for a third mechanism!

Do we need to adjust the mechanism? The only difference I see is that FIT brings the files together.

No, it's just fine as it is.

Quoted 4 lines

The benefit of the existing mechanism is precisely that nobody has to package the zImage/initrd/... together, whereas that appears to be precisely the purpose of FIT. The two schemes are by definition opposites of each-other.

Really? Well zImage is a packaged kernel isn't it? That part of it is definitely opposite. But other than that I don't see why a group of separate files is so different from packaging them together.

Quoted 56 lines

Quoted 50 lines

Quoted 13 lines

Quoted 3 lines

Would it make it simpler or harder? To me, we should be moving to using FIT with U-Boot as it is much more flexible and better designed from the ground up to provide the required features.

I disagree that FIT is a good idea, but that's a separate topic.

Quoted 2 lines

Anyway, normally FIT has a compressed kernel (e.g. with LZO), not a bzImage.

Quoted 5 lines

Do you mean FIT normally contains an originally uncompressed kernel (i.e. an
Image file in ARM land at least), but then compresses it itself as part of
FIT image generation? A bzImage is also a "compressed kernel".

That's right, that's what I mean.

Quoted 11 lines

Quoted 6 lines

So it seems like we need an additional decompression address.

I suppose we could always use malloc() instead... But perhaps a FIT load address makes sense. But then we don't really need a kernel load address do we? Shouldn't that be specified in the FIT itself?

A FIT load address does sound like it makes sense.

Quoted 15 lines

If U-Boot copies the kernel image out of the FIT image to somewhere else, the FIT image shouldn't specify the address to copy it to. This varies per
SoC, so if this address is hard-coded into FIT, it means the FIT image can't
be used on different SoCs (or perhaps even different boards, depending on how differing RAM sizes work on various HW). This is why with config_distro_bootcmd, all the addresses come from the U-Boot environment,
not hard-coded into a file on the disk. That way, the files are all generic
and can be used on various different systems that require different addresses. It possibly makes sense for kernel_addr_r to be the destination
of that copy?

Quoted 4 lines

Doesn't the kernel know where the kernel needs to be loaded / copied as part of the bzImage stuff? From what I see at present this is hard-coded into the code in the kernel. So we need to put this correct address in the FIT, is that right?

Quoted 8 lines

No. The address is dynamically calculated by the kernel decompressor for any kernel with CONFIG_AUTO_ZRELADDR enabled. Any distro-provided kernel will have that option enabled, so that the kernel doesn't have to hard-code any addresses, and so that the same kernel image can run on multiple SoCs.

For kernels without CONFIG_AUTO_ZRELADDR enabled, everything should still work too, provided the decompression target address the kernel has hard-coded into it doesn't overlap stuff like the DTB or initrd.

Form the help:

"ZRELADDR is the physical address where the decompressed kernel image will be placed. If AUTO_ZRELADDR is selected, the address will be determined at run-time by masking the current IP with 0xf8000000. This assumes the zImage being placed in the first 128MB from start of memory."

It is dynamically calculated based on the kernel being loaded to a reasonable place. But presumably the kernel itself actually ends up at a fixed TEXTBASE. Unless you are using ASLR iwc you have ELF relocations anyway. Or do I have this wrong?

So what I am saying is that the kernel could make a FIT with TEXTBASE set correctly.

Quoted 6 lines

Quoted 2 lines

Are you thinking we should allow
FIT to take an environment variable as a load address?

That would likely be required for it to be useful with CONFIG_AUTO_ZRELADDR, yes.

Will await your thoughts on the above before commenting further.

Quoted 5 lines

Quoted 3 lines

If so, I feel it would make a lot of sense for the kernel to package up the FIT to avoid these issues.

Quoted 2 lines

The kernel community has already specifically rejected (multiple times IIRC) generating bootloader-specific image formats in the kernel build system.

Doesn't' the kernel still support 'make uImage'? Anyway, if the information is available we don't need the kernel to support it itself. It would just be easier...

Quoted 145 lines · Simon Glass

+Tom Rini in case he is interested...

Hi Stephen,

Quoted 140 lines · Stephen Warren

Quoted 127 lines · Simon Glass

Hi Stephen,

Quoted 123 lines · Stephen Warren

Quoted 109 lines · Simon Glass

Hi Stephen,

Quoted 93 lines · Stephen Warren

Quoted 54 lines · Simon Glass

Hi Stephen,

Quoted 39 lines · Stephen Warren

Quoted 11 lines · Simon Glass

We need to match the device tree in the FIT with the U-Boot model so we
can automatically select the right device tree. Also adjust the load address
so that the device tree is not in the way when a zImage kernel tries to
extract itself.

We don't tend to use LOADADDR in any of the default boot scripts any more.
Rather, we explicitly load files to a "semantic" location indicated by
one
of the following variables in tegra124-common.h:

#define MEM_LAYOUT_ENV_SETTINGS \
           "scriptaddr=0x90000000\0" \
           "pxefile_addr_r=0x90100000\0" \
           "kernel_addr_r=0x81000000\0" \
           "fdt_addr_r=0x82000000\0" \
           "ramdisk_addr_r=0x82100000\0"

Perhaps the ChromeOS boot scripts could be adjusted to use one/some of
those
variables?

Quoted 4 lines

If the value of CONFIG_LOADADDR isn't appropriate, perhaps we should fix
it
for all Tegra SoCs/boards?

Quoted 7 lines

I forgot about this comment sorry. I had problems with the image overwriting itself. It is a bzImage inside a FIT so doesn't use the proper FIT decompression.

Anyway I'd like to clarify what is meant by kernel_addr_r. Is that where the FIT is loaded or where the kernel will decompress to, or something else?

kernel_addr_r is the address in RAM at which a kernel zImage is loaded by
config_distro_bootcmd.h scripts (and likely other scripts too). It's usually
deliberately chosen to be a fair way into RAM, so that when the zImage decompresses (to approx the start of RAM), the decompressed image doesn't
overlap the compressed image at kernel_addr_r. This avoids the kernel decompressor having to move/copy the zImage somewhere else before copying
to
avoid any overlap during decompression.

config_distro_bootcmd.h doesn't support FIT, so I haven't tried out loading
FIT images. That said, if U-Boot simply jumps to the location of the kernel
in the FIT image itself without any copying, I would expect everything to
work fine if you loaded a FIT image to kernel_addr_r. However, if the processing of the FIT image requires the kernel to be copied out of the FIT
image to some other location, you'd have to carefully choose that location
so it didn't overlap anything. I'd strongly recommend avoiding any

Quoted 8 lines

unnecessary copies like that though, if at all possible, from the perspective of both pointlessly wasted execution time and simplicity of the
boot process. Perhaps storing a a kernel_noload image type inside the FIT
rather than a kernel image type might help there? Perhaps you want to introduce a new standard variable that defines where FIT images are loaded.

Quoted 2 lines

I wonder what would be involved in adjusting config_distro_bootcmd to support FIT?

Well, it goes against the very idea of config_distro_bootcmd, which is to provide a single standard mechanism that doesn't rely on any bootloader-specific file formats etc. That mechanism is a raw zImage, a raw
initrd, and a plain text extlinux.cfg file to specify things like file paths/names, command-line, etc.

Quoted 8 lines

Quoted 4 lines

The boot.scr support there is legacy, and not something that should be built
upon going forward. So, that's not an argument for adding support for a third mechanism!

Do we need to adjust the mechanism? The only difference I see is that FIT brings the files together.

No, it's just fine as it is.

Quoted 8 lines

Quoted 4 lines

The benefit of the existing mechanism is precisely that nobody has to package the zImage/initrd/... together, whereas that appears to be precisely the purpose of FIT. The two schemes are by definition opposites of each-other.

Really? Well zImage is a packaged kernel isn't it? That part of it is definitely opposite. But other than that I don't see why a group of separate files is so different from packaging them together.

The zImage packaging is a kernel-internal function implemented (both compression and decompression) fully as part of the kernel build process. Code/... that consumes the kernel doesn't even know that such packaging happens; you can just run make in the kernel tree, and you get a zImage.

However, if you want a FIT or uImage, you need to run additional tools beyond a plain "make" in the kernel tree.

Quoted 74 lines

Quoted 57 lines

Quoted 51 lines

Quoted 14 lines

Quoted 4 lines

Would it make it simpler or harder? To me, we should be moving to using FIT with U-Boot as it is much more flexible and better designed from the ground up to provide the required features.

I disagree that FIT is a good idea, but that's a separate topic.

Quoted 2 lines

Anyway, normally FIT has a compressed kernel (e.g. with LZO), not a bzImage.

Quoted 5 lines

Do you mean FIT normally contains an originally uncompressed kernel (i.e. an
Image file in ARM land at least), but then compresses it itself as part of
FIT image generation? A bzImage is also a "compressed kernel".

That's right, that's what I mean.

Quoted 11 lines

Quoted 6 lines

So it seems like we need an additional decompression address.

I suppose we could always use malloc() instead... But perhaps a FIT load address makes sense. But then we don't really need a kernel load address do we? Shouldn't that be specified in the FIT itself?

A FIT load address does sound like it makes sense.

Quoted 15 lines

If U-Boot copies the kernel image out of the FIT image to somewhere else, the FIT image shouldn't specify the address to copy it to. This varies per
SoC, so if this address is hard-coded into FIT, it means the FIT image can't
be used on different SoCs (or perhaps even different boards, depending on how differing RAM sizes work on various HW). This is why with config_distro_bootcmd, all the addresses come from the U-Boot environment,
not hard-coded into a file on the disk. That way, the files are all generic
and can be used on various different systems that require different addresses. It possibly makes sense for kernel_addr_r to be the destination
of that copy?

Quoted 4 lines

Doesn't the kernel know where the kernel needs to be loaded / copied as part of the bzImage stuff? From what I see at present this is hard-coded into the code in the kernel. So we need to put this correct address in the FIT, is that right?

Quoted 8 lines

No. The address is dynamically calculated by the kernel decompressor for any kernel with CONFIG_AUTO_ZRELADDR enabled. Any distro-provided kernel will have that option enabled, so that the kernel doesn't have to hard-code any addresses, and so that the same kernel image can run on multiple SoCs.

For kernels without CONFIG_AUTO_ZRELADDR enabled, everything should still work too, provided the decompression target address the kernel has hard-coded into it doesn't overlap stuff like the DTB or initrd.

Form the help:

"ZRELADDR is the physical address where the decompressed kernel image will be placed. If AUTO_ZRELADDR is selected, the address will be determined at run-time by masking the current IP with 0xf8000000. This assumes the zImage being placed in the first 128MB from start of memory."

Quoted 4 lines

It is dynamically calculated based on the kernel being loaded to a reasonable place. But presumably the kernel itself actually ends up at a fixed TEXTBASE. Unless you are using ASLR iwc you have ELF relocations anyway. Or do I have this wrong?

The location the kernel is decompressed to is dynamic; it isn't fixed to TEXTBASE. AUTO_ZRELADDR calculates this location. IIRC (it's been a while since I looked at how this is implemented) the kernel code is either position-independent (early boot code prior to MMU setup) or runs in a virtual address space with the MMU configuration hiding the dynamic nature of the run-time-generated physical address.

Quoted 3 lines

So what I am saying is that the kernel could make a FIT with TEXTBASE set correctly.

I don't believe so. Well, for a single platform or set of similar platforms, yes it could. For a completely generic multi-platform kernel it could not.

Quoted 9 lines

Quoted 7 lines

Quoted 3 lines

Are you thinking we should allow
FIT to take an environment variable as a load address?

That would likely be required for it to be useful with CONFIG_AUTO_ZRELADDR, yes.

Will await your thoughts on the above before commenting further.

Quoted 6 lines

Quoted 6 lines

Quoted 4 lines

If so, I feel it would make a lot of sense for the kernel to package up the FIT to avoid these issues.

Quoted 6 lines

Quoted 2 lines

The kernel community has already specifically rejected (multiple times IIRC) generating bootloader-specific image formats in the kernel build system.

Doesn't' the kernel still support 'make uImage'? Anyway, if the information is available we don't need the kernel to support it itself. It would just be easier...

That target exists *but* since it runs "mkimage -T kernel" (rather than e.g. -T kernel_noload), you must specify a fixed address for the uImage to be unpacked into. This makes the build target useless for multi-platform kernels, since the set of legal "load" addresses differs between platforms. Multi-plaform kernels are pretty much all anyone cares about these days at least for ARM Linux. IIRC there were some discussions and patches proposed to solve some of this, but they were rejected because the kernel maintainers didn't want the kernel build process to support external bootloader-specific packaging steps. I believe it was even proposed to remove the build target, but that doesn't seem to have happened.

Quoted 10 lines

Quoted 2 lines

I wonder what would be involved in adjusting config_distro_bootcmd to support FIT?

Well, it goes against the very idea of config_distro_bootcmd, which is to provide a single standard mechanism that doesn't rely on any bootloader-specific file formats etc. That mechanism is a raw zImage, a raw initrd, and a plain text extlinux.cfg file to specify things like file paths/names, command-line, etc.

Quoted 3 lines

The boot.scr support there is legacy, and not something that should be built upon going forward. So, that's not an argument for adding support for a third mechanism!

Do we need to adjust the mechanism? The only difference I see is that FIT brings the files together.

Quoted 11 lines

Quoted 3 lines

Would it make it simpler or harder? To me, we should be moving to using FIT with U-Boot as it is much more flexible and better designed from the ground up to provide the required features.

I disagree that FIT is a good idea, but that's a separate topic.

Quoted 2 lines

Anyway, normally FIT has a compressed kernel (e.g. with LZO), not a bzImage.

Quoted 3 lines

Do you mean FIT normally contains an originally uncompressed kernel (i.e. an Image file in ARM land at least), but then compresses it itself as part of FIT image generation? A bzImage is also a "compressed kernel".

That's right, that's what I mean.

Quoted 20 lines

Quoted 6 lines

So it seems like we need an additional decompression address.

I suppose we could always use malloc() instead... But perhaps a FIT load address makes sense. But then we don't really need a kernel load address do we? Shouldn't that be specified in the FIT itself?

A FIT load address does sound like it makes sense.

If U-Boot copies the kernel image out of the FIT image to somewhere else, the FIT image shouldn't specify the address to copy it to. This varies per SoC, so if this address is hard-coded into FIT, it means the FIT image can't be used on different SoCs (or perhaps even different boards, depending on how differing RAM sizes work on various HW). This is why with config_distro_bootcmd, all the addresses come from the U-Boot environment, not hard-coded into a file on the disk. That way, the files are all generic and can be used on various different systems that require different addresses. It possibly makes sense for kernel_addr_r to be the destination of that copy?

Doesn't the kernel know where the kernel needs to be loaded / copied as part of the bzImage stuff? From what I see at present this is hard-coded into the code in the kernel. So we need to put this correct address in the FIT, is that right? Are you thinking we should allow FIT to take an environment variable as a load address?

If so, I feel it would make a lot of sense for the kernel to package up the FIT to avoid these issues.

Hi Simon,

Quoted 10 lines

Quoted 10 lines

Quoted 2 lines

I wonder what would be involved in adjusting config_distro_bootcmd to support FIT?

Well, it goes against the very idea of config_distro_bootcmd, which is to provide a single standard mechanism that doesn't rely on any bootloader-specific file formats etc. That mechanism is a raw zImage, a raw initrd, and a plain text extlinux.cfg file to specify things like file paths/names, command-line, etc.

Quoted 6 lines

Quoted 3 lines

The boot.scr support there is legacy, and not something that should be built upon going forward. So, that's not an argument for adding support for a third mechanism!

Do we need to adjust the mechanism? The only difference I see is that FIT brings the files together.

Speaking as one of the ARM maintainers that's not what we want. We want to be able to use the standard kernel, initrd and then a DT so we can boot a single image across any device that the kernel supports.

In Fedora at the moment we can boot around a 100 odd devices off a single kernel by specifying the DT separately. I've not looked at FIT closely but I don't believe it provides us that.

Quoted 12 lines

Quoted 12 lines

Quoted 4 lines

Would it make it simpler or harder? To me, we should be moving to using FIT with U-Boot as it is much more flexible and better designed from the ground up to provide the required features.

I disagree that FIT is a good idea, but that's a separate topic.

Quoted 2 lines

Anyway, normally FIT has a compressed kernel (e.g. with LZO), not a bzImage.

Quoted 26 lines

Quoted 3 lines

Do you mean FIT normally contains an originally uncompressed kernel (i.e. an Image file in ARM land at least), but then compresses it itself as part of FIT image generation? A bzImage is also a "compressed kernel".

That's right, that's what I mean.

Quoted 20 lines

Quoted 6 lines

So it seems like we need an additional decompression address.

I suppose we could always use malloc() instead... But perhaps a FIT load address makes sense. But then we don't really need a kernel load address do we? Shouldn't that be specified in the FIT itself?

A FIT load address does sound like it makes sense.

If U-Boot copies the kernel image out of the FIT image to somewhere else, the FIT image shouldn't specify the address to copy it to. This varies per SoC, so if this address is hard-coded into FIT, it means the FIT image can't be used on different SoCs (or perhaps even different boards, depending on how differing RAM sizes work on various HW). This is why with config_distro_bootcmd, all the addresses come from the U-Boot environment, not hard-coded into a file on the disk. That way, the files are all generic and can be used on various different systems that require different addresses. It possibly makes sense for kernel_addr_r to be the destination of that copy?

The above it what Fedora does, this is why we assisted in the implementation of config_distro_bootcmd so we could have a single kernel for all devices.

Quoted 8 lines

Doesn't the kernel know where the kernel needs to be loaded / copied as part of the bzImage stuff? From what I see at present this is hard-coded into the code in the kernel. So we need to put this correct address in the FIT, is that right? Are you thinking we should allow FIT to take an environment variable as a load address?

If so, I feel it would make a lot of sense for the kernel to package up the FIT to avoid these issues.

Is there an overview of a FIT image somewhere, does it only contain a single DT or does it package up a single one for the device? Currently in Fedora we're shipping 283 DTs for a single 4.0.3 multiplatform kernel. For the images we ship we can for example just plug a generic image into a Jetson TK1 board and have it boot without any device specific configuration.

Hi Peter,

Quoted 19 lines · Peter Robinson

Hi Simon,

Quoted 10 lines

Quoted 10 lines

Quoted 2 lines

I wonder what would be involved in adjusting config_distro_bootcmd to support FIT?

Well, it goes against the very idea of config_distro_bootcmd, which is to provide a single standard mechanism that doesn't rely on any bootloader-specific file formats etc. That mechanism is a raw zImage, a raw initrd, and a plain text extlinux.cfg file to specify things like file paths/names, command-line, etc.

Quoted 6 lines

Quoted 3 lines

The boot.scr support there is legacy, and not something that should be built upon going forward. So, that's not an argument for adding support for a third mechanism!

Do we need to adjust the mechanism? The only difference I see is that FIT brings the files together.

Quoted 7 lines

Speaking as one of the ARM maintainers that's not what we want. We want to be able to use the standard kernel, initrd and then a DT so we can boot a single image across any device that the kernel supports.

In Fedora at the moment we can boot around a 100 odd devices off a single kernel by specifying the DT separately. I've not looked at FIT closely but I don't believe it provides us that.

My comment was not to adjust the standard mechanism, but to adjust the internal details of how U-Boot implements it such that FIT could be supported. I reviewed the U-Boot config_distro implementation at the time - I was careful to confirm that the mechanism itself was defined separately from U-Boot's implementation.

From my understanding we could package the bzImage kernel and all the

DTs/ramdisk into the FIT and make it work. This is what Chrome OS does, for example. Actually this all came up after Stephen asked how to make U-Boot's Chrome OS boot scripts look more like config_distro.

It may actually be simpler for U-Boot to implement I think, since it
would be using pre-wired feature. But that needs to be looked at with config_distro.

Quoted 39 lines · Simon Glass

Hi Peter,

Quoted 26 lines · Peter Robinson

Hi Simon,

Quoted 13 lines

Quoted 13 lines

Quoted 2 lines

I wonder what would be involved in adjusting config_distro_bootcmd to support FIT?

Well, it goes against the very idea of config_distro_bootcmd, which is to
provide a single standard mechanism that doesn't rely on any bootloader-specific file formats etc. That mechanism is a raw zImage, a raw
initrd, and a plain text extlinux.cfg file to specify things like file paths/names, command-line, etc.

Quoted 8 lines

Quoted 4 lines

The boot.scr support there is legacy, and not something that should be built
upon going forward. So, that's not an argument for adding support for a third mechanism!

Do we need to adjust the mechanism? The only difference I see is that FIT brings the files together.

Quoted 7 lines

Speaking as one of the ARM maintainers that's not what we want. We want to be able to use the standard kernel, initrd and then a DT so we can boot a single image across any device that the kernel supports.

In Fedora at the moment we can boot around a 100 odd devices off a single kernel by specifying the DT separately. I've not looked at FIT closely but I don't believe it provides us that.

Quoted 7 lines

My comment was not to adjust the standard mechanism, but to adjust the internal details of how U-Boot implements it such that FIT could be supported. I reviewed the U-Boot config_distro implementation at the time - I was careful to confirm that the mechanism itself was defined separately from U-Boot's implementation.

From my understanding we could package the bzImage kernel and all the

Nit: On ARM there's an Image (I think that's the uncompressed kernel build output) and a zImage (that is the compressed kernel build output). bzImage is x86-specific.

Quoted 7 lines

DTs/ramdisk into the FIT and make it work. This is what Chrome OS does, for example. Actually this all came up after Stephen asked how to make U-Boot's Chrome OS boot scripts look more like config_distro.

It may actually be simpler for U-Boot to implement I think, since it
would be using pre-wired feature. But that needs to be looked at with config_distro.

The main thing I asked was for the ChromeOS stuff to re-use existing environment variables rather than re-inventing its own for the same purpose.

I'm not sure whether there could be much more unification than that, since the model that ChromeOS and config_distro_bootcmd use to select the boot device and partition probably vary quite a bit?

This series expands Nyan-big support:

- Enable Chrome OS EC, so that the keyboard works
- Add some extra clock and pre-kernel init required for reliable operation
- Add Chrome OS environment variables, including 'run nvboot' to allow booting Chrome OS more easily

Still missing are audio and USB.

Doug Anderson (1):
Add Chrome OS config header

Simon Glass (19):
dm: spi: Avoid setting the speed with every transfer
cros_ec: Show the protocol version in the debug message
cros_ec: Handle the single duplex requirement in cros_ec
tegra: Provide more accurate microsecond time
tegra: cros_ec: Add tegra support for Chrome OS EC
tegra: spi: Drop the claim_bus() method to correct delays
dm: tegra: cros_ec: Enable Chrome OS EC on Nyan-big
dm: gpio: Add an implementation for gpio_get_number()
tegra: spi: Support slow SPI rates
tegra: clock: Support enabling external clocks
tegra: clock: Adjust PLL access to avoid a warning
tegra: Introduce SRAM repair on tegra124
tegra: Add missing tegra124 peripherals
tegra: Increase maximum arguments to 32
tegra: lcd: Tidy up clock init
tegra: Allow board-specific init
tegra: nyan-big: Add additional clock and kernel init
tegra: config: Allow Chrome OS environment settings to be included
tegra: config: nyan-big: Add options required by Chrome OS boot

 arch/arm/cpu/tegra-common/Makefile                |   1 +
 arch/arm/cpu/tegra-common/clock.c                 |  24 +-
 arch/arm/cpu/tegra-common/powergate.c             |  20 +-
 arch/arm/cpu/tegra-common/timer.c                 |  87 ++++
 arch/arm/cpu/tegra124-common/clock.c              |   2 +-
 arch/arm/dts/tegra124-nyan-big.dts                |   6 +-
 arch/arm/include/asm/arch-tegra/clock.h           |   8 +
 arch/arm/include/asm/arch-tegra/sys_proto.h       |   7 +
 arch/arm/include/asm/arch-tegra124/clock-tables.h |  12 +-
 arch/arm/include/asm/arch-tegra124/flow.h         |  12 +
 board/nvidia/common/board.c                       |   8 +-
 board/nvidia/nyan-big/nyan-big.c                  |  76 ++++
 configs/nyan-big_defconfig                        |   5 +
 drivers/gpio/gpio-uclass.c                        |  12 +
 drivers/misc/cros_ec.c                            |   3 +-
 drivers/misc/cros_ec_spi.c                        |  23 +-
 drivers/spi/spi-uclass.c                          |   9 +-
 drivers/spi/tegra114_spi.c                        |  51 +--
 drivers/video/tegra124/tegra124-lcd.c             |   4 +-

 include/configs/chromeos.h                        | 464 ++++++++++++++++++++++
 include/configs/nyan-big.h                        |   8 +
 include/configs/tegra-common-post.h               |  15 +-
 include/configs/tegra-common.h                    |   2 +-
 include/spi.h                                     |   3 +
 24 files changed, 804 insertions(+), 58 deletions(-)
 create mode 100644 arch/arm/cpu/tegra-common/timer.c
 create mode 100644 include/configs/chromeos.h

Hey Simon,

Incidentally i got acces to a Nyan big and wanted to start testing u-boot on it. Unfortunately putting a uImage in a vboot signed blob to chainload it from the primary bootloader like on the exynos based chromebooks seemed not to work.

Do you have any good pointers how to use u-boot on nyan? (Ideally without having to re-flash coreboot, as i would like to create images people can easily test on a vanilla chromebook)

Hi Sjoerd,

Quoted 7 lines · Sjoerd Simons

Hey Simon,

Incidentally i got acces to a Nyan big and wanted to start testing u-boot on it. Unfortunately putting a uImage in a vboot signed blob to chainload it from the primary bootloader like on the exynos based chromebooks seemed not to work.

Quoted 3 lines

Do you have any good pointers how to use u-boot on nyan? (Ideally without having to re-flash coreboot, as i would like to create images people can easily test on a vanilla chromebook)

No I don't sorry. I suppose in dev mode it should boot a signed image so if you put U-Boot in a FIT as with snow/pit it should work. But I don't have instructions...if you figure it out it would be good to put this info somewhere.

Quoted 11 lines · Simon Glass

Hi Sjoerd,

Quoted 7 lines · Sjoerd Simons

Hey Simon,

Incidentally i got acces to a Nyan big and wanted to start testing u-boot on it. Unfortunately putting a uImage in a vboot signed blob to chainload it from the primary bootloader like on the exynos based chromebooks seemed not to work.

Quoted 8 lines

Quoted 3 lines

Do you have any good pointers how to use u-boot on nyan? (Ideally without having to re-flash coreboot, as i would like to create images people can easily test on a vanilla chromebook)

No I don't sorry. I suppose in dev mode it should boot a signed image so if you put U-Boot in a FIT as with snow/pit it should work. But I don't have instructions...if you figure it out it would be good to put this info somewhere.

Finally got time to play a bit with this. On the snow/peach boards the approach is to put u-boot in a legacy u-boot image (not a FIT image), which has the nice side-effect of re-locating u-boot before jumping to it.

Unfortunately the depthcharge on the nyan boards appears not to support legacy images, only FIT images, which don't get relocated before jumping into the kernel blob. Long story short, on the nyan boards the FIT image gets loaded at 0x81000000 and the FIT images i created put the u-boot blob at an offset of 0xCC. After re-configuring CONFIG_SYS_TEXT_BASE to match i got u-boot starting \o/ ;)

Unfortunately the board seems to hard hang when it tries to enable/use the data cache. Enabling CONFIG_SYS_DCACHE_OFF makes it get to the u-boot prompt. Unfortunately neither MMC (didn't detect card) nor USB (failed to get descriptor from my usb network dongle) worked so i couldn't try booting a kernel just, but it's start.

Simon, does data cache/MMC/USB work properly when booting u-boot "natively" rather then chainloading from coreboot or does it have similar issues?

Fwiw, I've attached the output of u-boot running onthe board (with CONFIG_SYS_DCACHE_OFF enabled).