[U-Boot] [PATCH 0/20] tegra: Expand Nyan-big support
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
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 sjg@chromium.org ---
drivers/spi/spi-uclass.c | 9 ++++++--- include/spi.h | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index 63a6217..34cab34 100644 --- a/drivers/spi/spi-uclass.c +++ b/drivers/spi/spi-uclass.c @@ -63,9 +63,12 @@ int spi_claim_bus(struct spi_slave *slave) } if (!speed) speed = 100000; - ret = spi_set_speed_mode(bus, speed, slave->mode); - if (ret) - return ret; + if (speed != slave->speed) { + ret = spi_set_speed_mode(bus, speed, slave->mode); + if (ret) + return ret; + slave->speed = speed; + }
return ops->claim_bus ? ops->claim_bus(bus) : 0; } diff --git a/include/spi.h b/include/spi.h index c58e453..0d9ae97 100644 --- a/include/spi.h +++ b/include/spi.h @@ -99,6 +99,8 @@ struct dm_spi_slave_platdata { * @dev: SPI slave device * @max_hz: Maximum speed for this slave * @mode: SPI mode to use for this slave (see SPI mode flags) + * @speed: Current bus speed. This is 0 until the bus is first + * claimed. * @bus: ID of the bus that the slave is attached to. For * driver model this is the sequence number of the SPI * bus (bus->seq) so does not need to be stored @@ -116,6 +118,7 @@ struct spi_slave { #ifdef CONFIG_DM_SPI struct udevice *dev; /* struct spi_slave is dev->parentdata */ uint max_hz; + uint speed; uint mode; #else unsigned int bus;
On 17 February 2015 at 15:29, Simon Glass sjg@chromium.org wrote:
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 sjg@chromium.org
drivers/spi/spi-uclass.c | 9 ++++++--- include/spi.h | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-)
Applied to u-boot-dm.
When starting up, show the protocol version that has been negotiated with the EC.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/misc/cros_ec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/cros_ec.c b/drivers/misc/cros_ec.c index 5846e76..962d1dd 100644 --- a/drivers/misc/cros_ec.c +++ b/drivers/misc/cros_ec.c @@ -1109,7 +1109,8 @@ int cros_ec_register(struct udevice *dev) }
/* Remember this device for use by the cros_ec command */ - debug("Google Chrome EC CROS-EC driver ready, id '%s'\n", id); + debug("Google Chrome EC v%d CROS-EC driver ready, id '%s'\n", + cdev->protocol_version, id);
return 0; }
On 17 February 2015 at 15:29, Simon Glass sjg@chromium.org wrote:
When starting up, show the protocol version that has been negotiated with the EC.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/misc/cros_ec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Applied to u-boot-dm.
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 sjg@chromium.org ---
drivers/misc/cros_ec_spi.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/cros_ec_spi.c b/drivers/misc/cros_ec_spi.c index 9359c56..5f37947 100644 --- a/drivers/misc/cros_ec_spi.c +++ b/drivers/misc/cros_ec_spi.c @@ -25,6 +25,8 @@ int cros_ec_spi_packet(struct udevice *udev, int out_bytes, int in_bytes) { struct cros_ec_dev *dev = udev->uclass_priv; struct spi_slave *slave = dev_get_parentdata(dev->dev); + ulong start; + uint8_t byte; int rv;
/* Do the transfer */ @@ -33,10 +35,25 @@ int cros_ec_spi_packet(struct udevice *udev, int out_bytes, int in_bytes) return -1; }
- rv = spi_xfer(slave, max(out_bytes, in_bytes) * 8, - dev->dout, dev->din, - SPI_XFER_BEGIN | SPI_XFER_END); + rv = spi_xfer(slave, out_bytes * 8, dev->dout, NULL, SPI_XFER_BEGIN); + if (rv) + goto done; + start = get_timer(0); + while (1) { + rv = spi_xfer(slave, 8, NULL, &byte, 0); + if (byte == SPI_PREAMBLE_END_BYTE) + break; + if (rv) + goto done; + if (get_timer(start) > 100) { + rv = -ETIMEDOUT; + goto done; + } + }
+ rv = spi_xfer(slave, in_bytes * 8, NULL, dev->din, 0); +done: + spi_xfer(slave, 0, NULL, NULL, SPI_XFER_END); spi_release_bus(slave);
if (rv) {
On 17 February 2015 at 15:29, Simon Glass sjg@chromium.org wrote:
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 sjg@chromium.org
drivers/misc/cros_ec_spi.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
Applied to u-boot-dm.
Add an implementation of the timer functions for tegra, so that timing is more accurate. Tegra has a 1 microsecond timer for this purpose.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/cpu/tegra-common/Makefile | 1 + arch/arm/cpu/tegra-common/timer.c | 87 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 arch/arm/cpu/tegra-common/timer.c
diff --git a/arch/arm/cpu/tegra-common/Makefile b/arch/arm/cpu/tegra-common/Makefile index a78869e..3dbe97f 100644 --- a/arch/arm/cpu/tegra-common/Makefile +++ b/arch/arm/cpu/tegra-common/Makefile @@ -14,6 +14,7 @@ obj-y += clock.o obj-y += lowlevel_init.o obj-y += pinmux-common.o obj-y += powergate.o +obj-y += timer.o obj-y += xusb-padctl.o obj-$(CONFIG_DISPLAY_CPUINFO) += sys_info.o obj-$(CONFIG_TEGRA124) += vpr.o diff --git a/arch/arm/cpu/tegra-common/timer.c b/arch/arm/cpu/tegra-common/timer.c new file mode 100644 index 0000000..c7f80a5 --- /dev/null +++ b/arch/arm/cpu/tegra-common/timer.c @@ -0,0 +1,87 @@ +/* + * (C) Copyright 2015 Googoe, Inc + * (C) Copyright 2010,2011 + * NVIDIA Corporation <www.nvidia.com> + * + * (C) Copyright 2008 + * Texas Instruments + * + * Richard Woodruff r-woodruff2@ti.com + * Syed Moahmmed Khasim khasim@ti.com + * + * (C) Copyright 2002 + * Sysgo Real-Time Solutions, GmbH <www.elinos.com> + * Marius Groeger mgroeger@sysgo.de + * Alex Zuepke azu@sysgo.de + * + * (C) Copyright 2002 + * Gary Jennejohn, DENX Software Engineering, garyj@denx.de + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <asm/io.h> +#include <asm/arch/tegra.h> +#include <asm/arch-tegra/timer.h> + +DECLARE_GLOBAL_DATA_PTR; + +/* counter runs at 1MHz */ +#define TIMER_CLK 1000000 +#define TIMER_LOAD_VAL 0xffffffff + +/* timer without interrupts */ +ulong get_timer(ulong base) +{ + return get_timer_masked() - base; +} + +/* delay x useconds */ +void __udelay(unsigned long usec) +{ + long tmo = usec * (TIMER_CLK / 1000) / 1000; + unsigned long now, last = timer_get_us(); + + while (tmo > 0) { + now = timer_get_us(); + if (last > now) /* count up timer overflow */ + tmo -= TIMER_LOAD_VAL - last + now; + else + tmo -= now - last; + last = now; + } +} + +ulong get_timer_masked(void) +{ + ulong now; + + /* current tick value */ + now = timer_get_us() / (TIMER_CLK / CONFIG_SYS_HZ); + + if (now >= gd->arch.lastinc) /* normal mode (non roll) */ + /* move stamp forward with absolute diff ticks */ + gd->arch.tbl += (now - gd->arch.lastinc); + else /* we have rollover of incrementer */ + gd->arch.tbl += ((TIMER_LOAD_VAL / (TIMER_CLK / CONFIG_SYS_HZ)) + - gd->arch.lastinc) + now; + gd->arch.lastinc = now; + return gd->arch.tbl; +} + +/* + * This function is derived from PowerPC code (read timebase as long long). + * On ARM it just returns the timer value. + */ +unsigned long long get_ticks(void) +{ + return get_timer(0); +} + +unsigned long timer_get_us(void) +{ + struct timerus *timer_base = (struct timerus *)NV_PA_TMRUS_BASE; + + return readl(&timer_base->cntr_1us); +}
On 02/17/2015 03:29 PM, Simon Glass wrote:
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?
Hi Stephen,
On 25 February 2015 at 16:10, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
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?
Yes you are right. I couldn't find out where this was being done and was seeing strange behaviour with udelay(). I'm not very keen on the lldiv() in lib/time.c when it is a no-op. But I suppose all it can do is slow things down a bit.
Anyway, we can drop this patch.
Regards, Simon
This requires a change to stdin to include the 'cros-ec-keyb' input device. Put this in the common file, enabled by the relevant CONFIG.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/configs/tegra-common-post.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h index 46a155d..2de5d36 100644 --- a/include/configs/tegra-common-post.h +++ b/include/configs/tegra-common-post.h @@ -40,8 +40,14 @@ #define STDOUT_LCD "" #endif
+#ifdef CONFIG_CROS_EC_KEYB +#define STDOUT_CROS_EC ",cros-ec-keyb" +#else +#define STDOUT_CROS_EC "" +#endif + #define TEGRA_DEVICE_SETTINGS \ - "stdin=serial" STDIN_KBD_KBC STDIN_KBD_USB "\0" \ + "stdin=serial" STDIN_KBD_KBC STDIN_KBD_USB STDOUT_CROS_EC "\0" \ "stdout=serial" STDOUT_LCD "\0" \ "stderr=serial" STDOUT_LCD "\0" \ ""
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.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/spi/tegra114_spi.c | 39 ++++++++------------------------------- 1 file changed, 8 insertions(+), 31 deletions(-)
diff --git a/drivers/spi/tegra114_spi.c b/drivers/spi/tegra114_spi.c index 53ff9ea..a73c6b0 100644 --- a/drivers/spi/tegra114_spi.c +++ b/drivers/spi/tegra114_spi.c @@ -150,34 +150,12 @@ static int tegra114_spi_probe(struct udevice *bus) priv->freq = plat->frequency; priv->periph_id = plat->periph_id;
- return 0; -} - -static int tegra114_spi_claim_bus(struct udevice *bus) -{ - struct tegra114_spi_priv *priv = dev_get_priv(bus); - struct spi_regs *regs = priv->regs; - /* Change SPI clock to correct frequency, PLLP_OUT0 source */ - clock_start_periph_pll(priv->periph_id, CLOCK_ID_PERIPH, priv->freq); - - /* Clear stale status here */ - setbits_le32(®s->fifo_status, - SPI_FIFO_STS_ERR | - SPI_FIFO_STS_TX_FIFO_OVF | - SPI_FIFO_STS_TX_FIFO_UNR | - SPI_FIFO_STS_RX_FIFO_OVF | - SPI_FIFO_STS_RX_FIFO_UNR | - SPI_FIFO_STS_TX_FIFO_FULL | - SPI_FIFO_STS_TX_FIFO_EMPTY | - SPI_FIFO_STS_RX_FIFO_FULL | - SPI_FIFO_STS_RX_FIFO_EMPTY); - debug("%s: FIFO STATUS = %08x\n", __func__, readl(®s->fifo_status)); - - /* Set master mode and sw controlled CS */ - setbits_le32(®s->command1, SPI_CMD1_M_S | SPI_CMD1_CS_SW_HW | - (priv->mode << SPI_CMD1_MODE_SHIFT)); - debug("%s: COMMAND1 = %08x\n", __func__, readl(®s->command1)); + clock_start_periph_pll(priv->periph_id, CLOCK_ID_PERIPH, + priv->freq); + + 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);
return 0; } @@ -248,6 +226,9 @@ static int tegra114_spi_xfer(struct udevice *dev, unsigned int bitlen,
ret = 0;
+ if (flags & SPI_XFER_BEGIN) + spi_cs_activate(dev); + /* clear all error status bits */ reg = readl(®s->fifo_status); writel(reg, ®s->fifo_status); @@ -259,9 +240,6 @@ static int tegra114_spi_xfer(struct udevice *dev, unsigned int bitlen, /* set xfer size to 1 block (32 bits) */ writel(0, ®s->dma_blk);
- if (flags & SPI_XFER_BEGIN) - spi_cs_activate(dev); - /* handle data in 32-bit chunks */ while (num_bytes > 0) { int bytes; @@ -384,7 +362,6 @@ static int tegra114_spi_set_mode(struct udevice *bus, uint mode) }
static const struct dm_spi_ops tegra114_spi_ops = { - .claim_bus = tegra114_spi_claim_bus, .xfer = tegra114_spi_xfer, .set_speed = tegra114_spi_set_speed, .set_mode = tegra114_spi_set_mode,
On 02/17/2015 03:29 PM, Simon Glass wrote:
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.
diff --git a/drivers/spi/tegra114_spi.c b/drivers/spi/tegra114_spi.c
- setbits_le32(®s->command1, SPI_CMD1_M_S | SPI_CMD1_CS_SW_HW |
(priv->mode << SPI_CMD1_MODE_SHIFT));
...
- 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?
Hi Stephen,
On 25 February 2015 at 16:14, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
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.
diff --git a/drivers/spi/tegra114_spi.c b/drivers/spi/tegra114_spi.c
setbits_le32(®s->command1, SPI_CMD1_M_S | SPI_CMD1_CS_SW_HW |(priv->mode << SPI_CMD1_MODE_SHIFT));...
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?
Yes. To me it seems that we should set CS high when probing. Otherwise the bus may sit low (enabled) until we do our first activate().
Regards, Simon
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.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/dts/tegra124-nyan-big.dts | 6 ++++-- configs/nyan-big_defconfig | 5 +++++ include/configs/nyan-big.h | 2 ++ 3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/arm/dts/tegra124-nyan-big.dts b/arch/arm/dts/tegra124-nyan-big.dts index 8ac770b..f063afd 100644 --- a/arch/arm/dts/tegra124-nyan-big.dts +++ b/arch/arm/dts/tegra124-nyan-big.dts @@ -163,12 +163,14 @@
spi@7000d400 { status = "okay"; + spi-deactivate-delay = <200>; + spi-max-frequency = <3000000>;
cros_ec: cros-ec@0 { - compatible = "google,cros-ec-spi"; - spi-max-frequency = <3000000>; + compatible = "google,cros-ec"; interrupt-parent = <&gpio>; interrupts = <TEGRA_GPIO(C, 7) IRQ_TYPE_LEVEL_LOW>; + ec-interrupt = <&gpio TEGRA_GPIO(C, 7) GPIO_ACTIVE_LOW>; reg = <0>;
google,cros-ec-spi-msg-delay = <2000>; diff --git a/configs/nyan-big_defconfig b/configs/nyan-big_defconfig index 41de8e6..dae82cb 100644 --- a/configs/nyan-big_defconfig +++ b/configs/nyan-big_defconfig @@ -5,3 +5,8 @@ CONFIG_DEFAULT_DEVICE_TREE="tegra124-nyan-big" CONFIG_DISPLAY_PORT=y CONFIG_VIDEO_TEGRA124=y +CONFIG_DM_CROS_EC=y +CONFIG_CROS_EC=y +CONFIG_CROS_EC_SPI=y +CONFIG_CROS_EC_KEYB=y +CONFIG_CMD_CROS_EC=y diff --git a/include/configs/nyan-big.h b/include/configs/nyan-big.h index f46f8dc..0b7d4d6 100644 --- a/include/configs/nyan-big.h +++ b/include/configs/nyan-big.h @@ -81,6 +81,8 @@ #define CONFIG_FIT #define CONFIG_OF_LIBFDT
+#define CONFIG_KEYBOARD + #include "tegra-common-usb-gadget.h" #include "tegra-common-post.h"
On 02/17/2015 03:29 PM, Simon Glass wrote:
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.
diff --git a/arch/arm/dts/tegra124-nyan-big.dts b/arch/arm/dts/tegra124-nyan-big.dts
spi@7000d400 { status = "okay";
spi-deactivate-delay = <200>;spi-max-frequency = <3000000>;cros_ec: cros-ec@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?
Hi Stephen,
On 25 February 2015 at 16:15, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
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.
diff --git a/arch/arm/dts/tegra124-nyan-big.dts b/arch/arm/dts/tegra124-nyan-big.dts
spi@7000d400 { status = "okay";
spi-deactivate-delay = <200>;spi-max-frequency = <3000000>; cros_ec: cros-ec@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?
It looks like it needs to be in the slave so I'll add it back there. I don't see it in the bus in the Linux version of the SPI bus node, so presumably it uses a default speed, perhaps higher than 3MHz. Probably best to have it in both places.
Regards, Simon
This has a prototype but no implementation. It returns the global GPIO number given a gpio_desc. It is useful for debugging in some cases.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/gpio/gpio-uclass.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 6e05c54..f316fc1 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -721,6 +721,18 @@ static int gpio_renumber(struct udevice *removed_dev) return 0; }
+int gpio_get_number(struct gpio_desc *desc) +{ + struct udevice *dev = desc->dev; + struct gpio_dev_priv *uc_priv; + + if (!dev) + return -1; + uc_priv = dev->uclass_priv; + + return uc_priv->gpio_base + desc->offset; +} + static int gpio_post_probe(struct udevice *dev) { struct gpio_dev_priv *uc_priv = dev->uclass_priv;
Use the oscillator as the source clock when we cannot achieve a low-enough speed with the peripheral clock. This happens when we request 3MHz on a SPI clock, for example.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/spi/tegra114_spi.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/tegra114_spi.c b/drivers/spi/tegra114_spi.c index a73c6b0..1c5368b 100644 --- a/drivers/spi/tegra114_spi.c +++ b/drivers/spi/tegra114_spi.c @@ -143,6 +143,7 @@ static int tegra114_spi_probe(struct udevice *bus) { struct tegra_spi_platdata *plat = dev_get_platdata(bus); struct tegra114_spi_priv *priv = dev_get_priv(bus); + ulong rate;
priv->regs = (struct spi_regs *)plat->base;
@@ -150,9 +151,20 @@ static int tegra114_spi_probe(struct udevice *bus) priv->freq = plat->frequency; priv->periph_id = plat->periph_id;
- /* Change SPI clock to correct frequency, PLLP_OUT0 source */ - clock_start_periph_pll(priv->periph_id, CLOCK_ID_PERIPH, - priv->freq); + /* + * Change SPI clock to correct frequency, PLLP_OUT0 source, falling + * back to the oscillator if that is too fast. + */ + rate = clock_start_periph_pll(priv->periph_id, CLOCK_ID_PERIPH, + priv->freq); + if (rate > priv->freq + 100000) { + rate = clock_start_periph_pll(priv->periph_id, CLOCK_ID_OSC, + priv->freq); + if (rate != priv->freq) { + printf("Warning: SPI '%s' requested clock %u, actual clock %lu\n", + bus->name, priv->freq, rate); + } + }
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);
Add a simple function to enable external clocks.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/cpu/tegra-common/clock.c | 17 +++++++++++++++++ arch/arm/include/asm/arch-tegra/clock.h | 8 ++++++++ 2 files changed, 25 insertions(+)
diff --git a/arch/arm/cpu/tegra-common/clock.c b/arch/arm/cpu/tegra-common/clock.c index 0d36b02..5d53444 100644 --- a/arch/arm/cpu/tegra-common/clock.c +++ b/arch/arm/cpu/tegra-common/clock.c @@ -17,10 +17,12 @@ /* Tegra SoC common clock control functions */
#include <common.h> +#include <errno.h> #include <asm/io.h> #include <asm/arch/clock.h> #include <asm/arch/tegra.h> #include <asm/arch-tegra/clk_rst.h> +#include <asm/arch-tegra/pmc.h> #include <asm/arch-tegra/timer.h> #include <div64.h> #include <fdtdec.h> @@ -698,3 +700,18 @@ void tegra30_set_up_pllp(void)
set_avp_clock_source(SCLK_SOURCE_PLLP_OUT4); } + +int clock_external_output(int clk_id) +{ + struct pmc_ctlr *pmc = (struct pmc_ctlr *)NV_PA_PMC_BASE; + + if (clk_id >= 1 && clk_id <= 3) { + setbits_le32(&pmc->pmc_clk_out_cntrl, + 1 << (2 + (clk_id - 1) * 8)); + } else { + printf("%s: Unknown output clock id %d\n", __func__, clk_id); + return -EINVAL; + } + + return 0; +} diff --git a/arch/arm/include/asm/arch-tegra/clock.h b/arch/arm/include/asm/arch-tegra/clock.h index 04011ae..f9dd3c8 100644 --- a/arch/arm/include/asm/arch-tegra/clock.h +++ b/arch/arm/include/asm/arch-tegra/clock.h @@ -336,4 +336,12 @@ void arch_timer_init(void);
void tegra30_set_up_pllp(void);
+/** + * Enable output clock for external peripherals + * + * @param clk_id Clock ID to output (1, 2 or 3) + * @return 0 if OK. -ve on error + */ +int clock_external_output(int clk_id); + #endif /* _TEGRA_CLOCK_H_ */
A harmless but confusing warning is displayed when looking up the DisplayPort PLL. Correct this.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/cpu/tegra-common/clock.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/tegra-common/clock.c b/arch/arm/cpu/tegra-common/clock.c index 5d53444..636be85 100644 --- a/arch/arm/cpu/tegra-common/clock.c +++ b/arch/arm/cpu/tegra-common/clock.c @@ -83,7 +83,7 @@ static struct clk_pll *get_pll(enum clock_id clkid)
assert(clock_id_is_pll(clkid)); if (clkid >= (enum clock_id)TEGRA_CLK_PLLS) { - debug("%s: Invalid PLL\n", __func__); + debug("%s: Invalid PLL %d\n", __func__, clkid); return NULL; } return &clkrst->crc_pll[clkid]; @@ -119,9 +119,12 @@ int clock_ll_read_pll(enum clock_id clkid, u32 *divm, u32 *divn, unsigned long clock_start_pll(enum clock_id clkid, u32 divm, u32 divn, u32 divp, u32 cpcon, u32 lfcon) { - struct clk_pll *pll = get_pll(clkid); + struct clk_pll *pll = NULL; u32 misc_data, data;
+ if (clkid < (enum clock_id)TEGRA_CLK_PLLS) + pll = get_pll(clkid); + /* * We cheat by treating all PLL (except PLLU) in the same fashion. * This works only because:
This is required in order to avoid instability when running from caches after the kernel starts.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/cpu/tegra-common/powergate.c | 20 +++++++++++++++++++- arch/arm/include/asm/arch-tegra124/flow.h | 12 ++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/tegra-common/powergate.c b/arch/arm/cpu/tegra-common/powergate.c index 439cff3..7db348e 100644 --- a/arch/arm/cpu/tegra-common/powergate.c +++ b/arch/arm/cpu/tegra-common/powergate.c @@ -9,7 +9,7 @@
#include <asm/io.h> #include <asm/types.h> - +#include <asm/arch/flow.h> #include <asm/arch/powergate.h> #include <asm/arch/tegra.h>
@@ -75,11 +75,29 @@ static int tegra_powergate_remove_clamping(enum tegra_powergate id) return 0; }
+static void tegra_powergate_ram_repair(void) +{ +#ifdef CONFIG_TEGRA124 + struct flow_ctlr *flow = (struct flow_ctlr *)NV_PA_FLOW_BASE; + + /* Request RAM repair for cluster 0 and wait until complete */ + setbits_le32(&flow->ram_repair, RAM_REPAIR_REQ); + while (!(readl(&flow->ram_repair) & RAM_REPAIR_STS)) + ; + + /* Same for cluster 1 */ + setbits_le32(&flow->ram_repair_cluster1, RAM_REPAIR_REQ); + while (!(readl(&flow->ram_repair_cluster1) & RAM_REPAIR_STS)) + ; +#endif +} + int tegra_powergate_sequence_power_up(enum tegra_powergate id, enum periph_id periph) { int err;
+ tegra_powergate_ram_repair(); reset_set_enable(periph, 1);
err = tegra_powergate_power_on(id); diff --git a/arch/arm/include/asm/arch-tegra124/flow.h b/arch/arm/include/asm/arch-tegra124/flow.h index 0db1881..ab5cd78 100644 --- a/arch/arm/include/asm/arch-tegra124/flow.h +++ b/arch/arm/include/asm/arch-tegra124/flow.h @@ -26,6 +26,12 @@ struct flow_ctlr { u32 cpu_pwr_csr; /* offset 0x38 */ u32 mpid; /* offset 0x3c */ u32 ram_repair; /* offset 0x40 */ + u32 flow_dbg_sel; /* offset 0x44 */ + u32 flow_dbg_cnt0; /* offset 0x48 */ + u32 flow_dbg_cnt1; /* offset 0x4c */ + u32 flow_dbg_qual; /* offset 0x50 */ + u32 flow_ctlr_spare; /* offset 0x54 */ + u32 ram_repair_cluster1;/* offset 0x58 */ };
/* HALT_COP_EVENTS_0, 0x04 */ @@ -37,4 +43,10 @@ struct flow_ctlr { /* FLOW_CTLR_CLUSTER_CONTROL_0 0x2c */ #define ACTIVE_LP (1 << 0)
+/* RAM_REPAIR, 0x40, 0x58 */ +enum { + RAM_REPAIR_REQ = 0x1 << 0, + RAM_REPAIR_STS = 0x1 << 1, +}; + #endif /* _TEGRA124_FLOW_H_ */
There are some missing entries in the tables. Add them.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/cpu/tegra124-common/clock.c | 2 +- arch/arm/include/asm/arch-tegra124/clock-tables.h | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm/cpu/tegra124-common/clock.c b/arch/arm/cpu/tegra124-common/clock.c index 2d17550..b955848 100644 --- a/arch/arm/cpu/tegra124-common/clock.c +++ b/arch/arm/cpu/tegra124-common/clock.c @@ -475,7 +475,7 @@ static s8 periph_id_to_internal_id[PERIPH_ID_COUNT] = { PERIPHC_ACTMON,
/* 120 */ - NONE(EXTPERIPH1), + PERIPHC_EXTPERIPH1, NONE(EXTPERIPH2), NONE(EXTPERIPH3), NONE(OOB), diff --git a/arch/arm/include/asm/arch-tegra124/clock-tables.h b/arch/arm/include/asm/arch-tegra124/clock-tables.h index 7005855..3c67e72 100644 --- a/arch/arm/include/asm/arch-tegra124/clock-tables.h +++ b/arch/arm/include/asm/arch-tegra124/clock-tables.h @@ -285,12 +285,12 @@ enum periph_id { /* 184 */ PERIPH_ID_GPU, PERIPH_ID_AMX1, - PERIPH_ID_X_RESERVED26, - PERIPH_ID_X_RESERVED27, - PERIPH_ID_X_RESERVED28, - PERIPH_ID_X_RESERVED29, - PERIPH_ID_X_RESERVED30, - PERIPH_ID_X_RESERVED31, + PERIPH_ID_AFC5, + PERIPH_ID_AFC4, + PERIPH_ID_AFC3, + PERIPH_ID_AFC2, + PERIPH_ID_AFC1, + PERIPH_ID_AFC0,
PERIPH_ID_COUNT, PERIPH_ID_NONE = -1,
When setting up large environment variables we can exceed 16 arguemnts. Increase this to avoid problems.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/configs/tegra-common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h index 005fc6a..14cc366 100644 --- a/include/configs/tegra-common.h +++ b/include/configs/tegra-common.h @@ -103,7 +103,7 @@ /* Print Buffer Size */ #define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + \ sizeof(CONFIG_SYS_PROMPT) + 16) -#define CONFIG_SYS_MAXARGS 16 /* max number of command args */ +#define CONFIG_SYS_MAXARGS 32 /* max number of command args */ /* Boot Argument Buffer Size */ #define CONFIG_SYS_BARGSIZE (CONFIG_SYS_CBSIZE)
Use the correct function for clock init.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/video/tegra124/tegra124-lcd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/video/tegra124/tegra124-lcd.c b/drivers/video/tegra124/tegra124-lcd.c index 55f63b1..d5b301d 100644 --- a/drivers/video/tegra124/tegra124-lcd.c +++ b/drivers/video/tegra124/tegra124-lcd.c @@ -50,15 +50,13 @@ static int tegra124_lcd_init(void *lcdbase) struct display_timing timing;
clock_set_up_plldp(); - clock_adjust_periph_pll_div(PERIPH_ID_HOST1X, CLOCK_ID_PERIPH, - 408000000, NULL); + clock_start_periph_pll(PERIPH_ID_HOST1X, CLOCK_ID_PERIPH, 408000000);
clock_enable(PERIPH_ID_HOST1X); clock_enable(PERIPH_ID_DISP1); clock_enable(PERIPH_ID_PWM); clock_enable(PERIPH_ID_DPAUX); clock_enable(PERIPH_ID_SOR0); - udelay(2);
reset_set_enable(PERIPH_ID_HOST1X, 0);
Add a hook to allows boards to add their own init to board_init().
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/include/asm/arch-tegra/sys_proto.h | 7 +++++++ board/nvidia/common/board.c | 8 ++++++-- 2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/arch-tegra/sys_proto.h b/arch/arm/include/asm/arch-tegra/sys_proto.h index 83f9f47..b64f9d8 100644 --- a/arch/arm/include/asm/arch-tegra/sys_proto.h +++ b/arch/arm/include/asm/arch-tegra/sys_proto.h @@ -25,4 +25,11 @@ int tegra_board_id(void); */ int tegra_lcd_pmic_init(int board_id);
+/** + * nvidia_board_init() - perform any board-specific init + * + * @return 0 if OK, -ve on error + */ +int nvidia_board_init(void); + #endif diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c index 18e1709..7563c4c 100644 --- a/board/nvidia/common/board.c +++ b/board/nvidia/common/board.c @@ -106,6 +106,11 @@ __weak int tegra_lcd_pmic_init(int board_it) return 0; }
+__weak int nvidia_board_init(void) +{ + return 0; +} + /* * Routine: board_init * Description: Early hardware init. @@ -179,8 +184,7 @@ int board_init(void) /* prepare the WB code to LP0 location */ warmboot_prepare_code(TEGRA_LP0_ADDR, TEGRA_LP0_SIZE); #endif - - return 0; + return nvidia_board_init(); }
#ifdef CONFIG_BOARD_EARLY_INIT_F
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.
Signed-off-by: Simon Glass sjg@chromium.org ---
board/nvidia/nyan-big/nyan-big.c | 76 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+)
diff --git a/board/nvidia/nyan-big/nyan-big.c b/board/nvidia/nyan-big/nyan-big.c index ae8874b..987ec7c 100644 --- a/board/nvidia/nyan-big/nyan-big.c +++ b/board/nvidia/nyan-big/nyan-big.c @@ -8,7 +8,12 @@ #include <common.h> #include <errno.h> #include <asm/gpio.h> +#include <asm/io.h> #include <asm/arch/pinmux.h> +#include <asm/arch/clock.h> +#include <asm/arch/mc.h> +#include <asm/arch-tegra/clk_rst.h> +#include <asm/arch-tegra/pmc.h> #include <power/as3722.h> #include <power/pmic.h> #include "pinmux-config-nyan-big.h" @@ -57,3 +62,74 @@ int tegra_lcd_pmic_init(int board_id)
return 0; } + +/* Setup required information for Linux kernel */ +static void setup_kernel_info(void) +{ + struct pmc_ctlr *pmc = (void *)NV_PA_PMC_BASE; + struct mc_ctlr *mc = (void *)NV_PA_MC_BASE; + + /* + * pmc.odmdata: [18:19]: console type, [15:17]: UART id + * TODO: Derive this from the current settings + */ + writel(0x80080000, &pmc->pmc_scratch20); + + /* The kernel graphics driver needs this region locked down */ + writel(0, &mc->mc_video_protect_bom); + writel(0, &mc->mc_video_protect_size_mb); + writel(1, &mc->mc_video_protect_reg_ctrl); +} + +/* + * We need to take ALL audio devices conntected to AHUB (AUDIO, APBIF, + * I2S, DAM, AMX, ADX, SPDIF, AFC) out of reset and enable the clocks. + * Otherwise reading AHUB devices will hang when the kernel boots. + */ +static void enable_required_clocks(void) +{ + static enum periph_id ids[] = { + PERIPH_ID_I2S0, + PERIPH_ID_I2S1, + PERIPH_ID_I2S2, + PERIPH_ID_I2S3, + PERIPH_ID_I2S4, + PERIPH_ID_AUDIO, + PERIPH_ID_APBIF, + PERIPH_ID_DAM0, + PERIPH_ID_DAM1, + PERIPH_ID_DAM2, + PERIPH_ID_AMX0, + PERIPH_ID_AMX1, + PERIPH_ID_ADX0, + PERIPH_ID_ADX1, + PERIPH_ID_SPDIF, + PERIPH_ID_AFC0, + PERIPH_ID_AFC1, + PERIPH_ID_AFC2, + PERIPH_ID_AFC3, + PERIPH_ID_AFC4, + PERIPH_ID_AFC5, + PERIPH_ID_EXTPERIPH1 + }; + int i; + + for (i = 0; i < ARRAY_SIZE(ids); i++) + clock_enable(ids[i]); + udelay(2); + for (i = 0; i < ARRAY_SIZE(ids); i++) + reset_set_enable(ids[i], 0); +} + +int nvidia_board_init(void) +{ + clock_start_periph_pll(PERIPH_ID_EXTPERIPH1, CLOCK_ID_OSC, 12000000); + clock_start_periph_pll(PERIPH_ID_I2S1, CLOCK_ID_OSC, 1500000); + + /* For external MAX98090 audio codec */ + clock_external_output(1); + setup_kernel_info(); + enable_required_clocks(); + + return 0; +}
On 02/17/2015 03:29 PM, Simon Glass wrote:
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.
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?
Hi Stephen,
On 25 February 2015 at 16:23, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
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.
OK I'll adjust the commit message. At some point perhaps this problem will go away. Chrome OS is running kernel v3.10 on this device.
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.
OK, so perhaps I can just drop this code. It might just be a hack in Coreboot.
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.
No, there really isn't any point in running U-Boot as a Coreboot payload, since U-Boot can do all the init itself. This series is for running U-Boot stand-alone on Nyan-big.
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?
See above... if we can get the display stuff merged then I could do this also. But without a display it's missing a big piece.
Regards, Simon
From: Doug Anderson dianders@chromium.org
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.
Signed-off-by: Doug Anderson dianders@chromium.org Signed-off-by: Simon Glass sjg@chromium.org ---
include/configs/chromeos.h | 464 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 464 insertions(+) create mode 100644 include/configs/chromeos.h
diff --git a/include/configs/chromeos.h b/include/configs/chromeos.h new file mode 100644 index 0000000..554ffba --- /dev/null +++ b/include/configs/chromeos.h @@ -0,0 +1,464 @@ +/* + * Copyright (c) 2015 Google, Inc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef __configs_chromeos_h__ +#define __configs_chromeos_h__ + +/* + * In anticipation of implementing early firmware selection, these defines + * signal the features that are required in U-Boot. + * + * The defines munging is thus kept to one Chrome OS-specific file. The + * upstream boards will define all features as normal for their platform. + * + * It is possible that these will be too broad for some platforms - e.g. we + * may have a platform which wants to use MMC in CONFIG_CROS_RO. However, + * we can deal with additional needs as we come to them. + * + * While it is something of an inconvenience, it may improve boot time on + * some platforms also. + * + * CONFIG_CROS_LARGE + * - Full version as now with factory defined, all features enabled + * - This will operate as factory build, or verified boot + * + * CONFIG_CROS_SMALL + * - Minimized for use only with verified boot + * - No command line, filesystems, LCD console, etc. + * - Still has all drivers enabled and can perform verified boot + * + * CONFIG_CROS_RO + * - Requires CONFIG_CROS_SMALL. Will only support running RO firmware + * - Set up for running VbLoadFirmware() only + * - Minimal RAM, no display, no USB, no mass storage (SPI flash only) + * - Intended for running in SPL + * + * CONFIG_CROS_RW + * - Requires CONFIG_CROS_SMALL. Will only support running RW firmware + * - Set up for running VbSelectAndLoadKernel() only + */ + +/* + * This config file defines platform-independent settings that a verified boot + * firmware must have. + */ + +/* Stringify a token */ +#ifndef STRINGIFY +#define _STRINGIFY(x) #x +#define STRINGIFY(x) _STRINGIFY(x) +#endif + +#define CONFIG_CROS_FULL + +/* Enable verified boot */ +#define CONFIG_CHROMEOS + +/* Enable test codes */ +#ifdef CONFIG_CROS_FULL +#define CONFIG_CHROMEOS_TEST +#endif /* VBOOT_DEBUG */ + +/* Support constant vboot flag from fdt */ +#define CONFIG_CHROMEOS_CONST_FLAG + +/* Enable vboot twostop with SPI flash */ +#define CONFIG_CHROMEOS_SPI + +#define CONFIG_VBOOT_REGION_READ + +#ifndef CONFIG_CROS_RO +#define CONFIG_CHROMEOS_DISPLAY + +/* Enable legacy vboot_twostop - crosbug.com/p/21810 */ +#define CONFIG_CROS_LEGACY_VBOOT +#endif + +#ifndef CONFIG_CROS_FULL +#undef CONFIG_CMDLINE +#undef CONFIG_SYS_LONGHELP +#undef CONFIG_SYS_CONSOLE_IS_IN_ENV +#undef CONFIG_SYS_STDIO_DEREGISTER +#undef CONFIG_SYS_HUSH_PARSER +#undef CONFIG_CBMEM_CONSOLE +#undef CONFIG_CMDLINE_EDITING +#undef CONFIG_COMMAND_HISTORY +#undef CONFIG_AUTOCOMPLETE +#undef CONFIG_CONSOLE_MUX +#undef CONFIG_SHOW_BOOT_PROGRESS + +#undef CONFIG_I8042_KBD +#define CONFIG_VGA_AS_SINGLE_DEVICE +#undef CONFIG_VIDEO_SW_CURSOR + +#undef CONFIG_SUPPORT_VFAT +#undef CONFIG_ATAPI +#undef CONFIG_EFI_PARTITION +#undef CONFIG_DOS_PARTITION +#undef CONFIG_MAC_PARTITION +#undef CONFIG_ISO_PARTITION +#undef CONFIG_PARTITION_UUIDS + +#undef CONFIG_CMD_PART +#undef CONFIG_CMD_CBFS +#undef CONFIG_CMD_EXT4 +#undef CONFIG_CMD_EXT4_WRITE +#undef CONFIG_CMD_NET +#undef CONFIG_CMD_CRC32 +#undef CONFIG_CMD_CROS_EC + +#undef CONFIG_USB_HOST_ETHER +#undef CONFIG_USB_ETHER_ASIX +#undef CONFIG_USB_ETHER_SMSC95XX + +#undef CONFIG_GENERIC_MMC +#undef CONFIG_MMC + +#define DYNAMIC_CRC_TABLE +#undef CONFIG_BOOTDELAY + +#endif + +/* Enable graphics display */ +#ifdef CONFIG_CHROMEOS_DISPLAY +#define CONFIG_LCD_BMP_RLE8 +#define CONFIG_LZMA +#define CONFIG_VIDEO_NO_TEXT +#else +#undef CONFIG_LCD +#undef CONFIG_EXYNOS_FB +#undef CONFIG_EXYNOS_DP +#undef CONFIG_VIDEO_ANALOGIX +#undef CONFIG_VIDEO_PARADE +#undef CONFIG_CMD_BMP +#endif + +#ifdef CONFIG_CROS_RO +#undef CONFIG_USB_EHCI +#undef CONFIG_USB_EHCI_PCI +#undef CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS +#undef CONFIG_USB_MAX_CONTROLLER_COUNT +#undef CONFIG_USB_STORAGE +#undef CONFIG_USB_KEYBOARD +#undef CONFIG_SYS_USB_EVENT_POLL + +#undef CONFIG_CMD_USB +#undef CONFIG_CMD_SOUND +#undef CONFIG_SOUND_INTEL_HDA +#undef CONFIG_CRC32_VERIFY +#undef CONFIG_TPM +#undef CONFIG_FIT +#define CONFIG_CRC32 +#undef CONFIG_LZO + +/* Limited memory so use a smaller recorded console */ +#undef CONFIG_RECORDED_CONSOLE_SIZE +#define CONFIG_RECORDED_CONSOLE_SIZE 3000 + +#else +/* USB is used in recovery mode */ +#define CONFIG_CHROMEOS_USB +#endif + +/* + * Enable this feature to embed crossystem data into device tree before booting + * the kernel. We add quite a few things to the FDT, including a 16KB binary + * blob. + * #define CONFIG_OF_BOARD_SETUP + */ +#define CONFIG_SYS_FDT_PAD 0x8000 + +/* Make sure that the safe version of printf() is compiled in. */ +#define CONFIG_SYS_VSNPRINTF + +/* + * This is the default kernel command line to a Chrome OS kernel. An ending + * space character helps us concatenate more arguments. + */ +#ifndef CONFIG_BOOTARGS +#define CONFIG_BOOTARGS +#endif +#define CHROMEOS_BOOTARGS "cros_secure " CONFIG_BOOTARGS " " + +#ifndef CONFIG_DIRECT_BOOTARGS +#define CONFIG_DIRECT_BOOTARGS +#endif +#ifndef CONFIG_EXTRA_BOOTARGS +#define CONFIG_EXTRA_BOOTARGS +#endif + +/******************************************************************************* + * Non-verified boot script * + ******************************************************************************/ + +/* + * Defines the regen_all variable, which is used by other commands + * defined in this file. Usage is to override one or more of the environment + * variables and then run regen_all to regenerate the environment. + * + * Args from other scipts in this file: + * bootdev_bootargs: Filled in by other commands below based on the boot + * device. + * + * Args: + * common_bootargs: A copy of the default bootargs so we can run regen_all + * more than once. + * dev_extras: Placeholder space for developers to put their own boot args. + * extra_bootargs: Filled in by update_firmware_vars.py script in some cases. + */ +#define CONFIG_REGEN_ALL_SETTINGS \ + "common_bootargs=cros_legacy " CONFIG_DIRECT_BOOTARGS "\0" \ + \ + "dev_extras=\0" \ + "extra_bootargs=" \ + CONFIG_EXTRA_BOOTARGS "\0" \ + "bootdev_bootargs=\0" \ + \ + "regen_all=" \ + "setenv bootargs " \ + "${common_bootargs} " \ + "${dev_extras} " \ + "${extra_bootargs} " \ + "${bootdev_bootargs}\0" + +/* + * Defines ext2_boot and run_disk_boot_script. + * + * The run_disk_boot_script runs a u-boot script on the boot disk. At the + * moment this is used to allow the boot disk to choose a partion to boot from, + * but could theoretically be used for more complicated things. + * + * The ext2_boot script boots from an ext2 device. + * + * Args from other scipts in this file: + * devtype: The device type we're booting from, like "usb" or "mmc" + * devnum: The device number (depends on devtype). If we're booting from + * extranal MMC (for instance), this would be 1 + * devname: The linux device name that will be assigned, like "sda" or + * mmcblk0p + * + * Args expected to be set by the u-boot script in /u-boot/boot.scr.uimg: + * rootpart: The root filesystem partion; we default to 3 in case there are + * problems reading the boot script. + * cros_bootfile: The name of the kernel in the root partition; we default to + * "/boot/vmlinux.uimg" + * + * Other args: + * script_part: The FAT partion we'll look for a boot script in. + * script_img: The name of the u-boot script. + * + * When we boot from an ext2 device, we will look at partion 12 (0x0c) to find + * a u-boot script (as /u-boot/boot.scr.uimg). That script is expected to + * override "rootpart" and "cros_bootfile" as needed to select which partition + * to boot from. + * + * USB download support: + * + * Once we have loaded the kernel from the selected device successfully, + * we check whether a kernel has in fact been provided through the USB + * download feature. In that case the kernaddr environment variable will + * be set. It might seem strange that we load the original kernel and + * then ignore it, but we try to load the kernel from a number of different + * places. If the USB disk fails (because there is no disk inserted or + * it is invalid) we don't want to pull in the kernaddr kernel and boot it + * with USB as the root disk. So allow the normal boot failover to occur, + * and only insert the kernaddr kernel when we actually have decided + * what to boot from. + */ +#define CONFIG_EXT2_BOOT_HELPER_SETTINGS \ + "rootpart=3\0" \ + "cros_bootfile=/boot/vmlinux.uimg\0" \ + \ + "script_part=c\0" \ + "script_img=/u-boot/boot.scr.uimg\0" \ + \ + "run_disk_boot_script=" \ + "if fatload ${devtype} ${devnum}:${script_part} " \ + "${loadaddr} ${script_img}; then " \ + "source ${loadaddr}; " \ + "echo done; " \ + "fi\0" \ + \ + "regen_ext2_bootargs=" \ + "setenv bootdev_bootargs root=${devname} rootwait ro; " \ + "run regen_all\0" \ + \ + "ext2_boot=" \ + "run regen_ext2_bootargs; " \ + "if ext2load ${devtype} ${devnum}:${rootpart} " \ + "${loadaddr} ${cros_bootfile}; then " \ + "if test "${kernaddr}" != ""; then "\ + "echo "Using bundled kernel"; "\ + "bootm ${kernaddr};" \ + "fi; "\ + "bootm ${loadaddr};" \ + "fi\0" + +/* + * Network-boot related settings. + * + * At the moment, we support: + * - initramfs factory install (tftp kernel with factory installer initramfs) + * - full network root booting (tftp kernel and initial ramdisk) + * - nfs booting (tftp kernel and point root to NFS) + * + * Network booting is enabled if you have an ethernet adapter plugged in at boot + * and also have set tftpserverip/nfsserverip to something other than 0.0.0.0. + * For full network booting or initramfs factory install you just need + * tftpserverip. To choose full network booting over initramfs factory intsall, + * you have to set has_initrd=1. For full NFS root you neet to set both + * tftpserverip and nfsserverip. + */ +#define CONFIG_NETBOOT_SETTINGS \ + "tftpserverip=0.0.0.0\0" \ + "nfsserverip=0.0.0.0\0" \ + "has_initrd=0\0" \ + \ + "rootaddr=" STRINGIFY(CONFIG_INITRD_ADDRESS) "\0" \ + "initrd_high=0xffffffff\0" \ + \ + "regen_nfsroot_bootargs=" \ + "setenv bootdev_bootargs " \ + "dev=/dev/nfs4 rw nfsroot=${nfsserverip}:${rootpath} " \ + "ip=dhcp noinitrd; " \ + "run regen_all\0" \ + "regen_initrdroot_bootargs=" \ + "setenv bootdev_bootargs " \ + "rw root=/dev/ram0 ramdisk_size=512000 cros_netboot; " \ + "run regen_all\0" \ + "regen_initramfs_install_bootargs=" \ + "setenv bootdev_bootargs " \ + "lsm.module_locking=0 cros_netboot_ramfs " \ + "cros_factory_install cros_secure; " \ + "run regen_all\0" \ + \ + "tftp_setup=" \ + "setenv tftpkernelpath " \ + "/tftpboot/vmlinux.uimg; " \ + "setenv tftprootpath " \ + "/tftpboot/initrd.uimg; " \ + "setenv rootpath " \ + "/export/nfsroot; " \ + "setenv autoload n\0" \ + "initrdroot_boot=" \ + "run tftp_setup; " \ + "run regen_initrdroot_bootargs; " \ + "bootp; " \ + "if tftpboot ${rootaddr} ${tftpserverip}:${tftprootpath} && " \ + " tftpboot ${loadaddr} ${tftpserverip}:${tftpkernelpath}; " \ + "then " \ + "bootm ${loadaddr} ${rootaddr}; " \ + "else " \ + "echo 'ERROR: Could not load root/kernel from TFTP'; " \ + "exit; " \ + "fi\0" \ + "initramfs_boot=" \ + "run tftp_setup; "\ + "run regen_initramfs_install_bootargs; "\ + "bootp; " \ + "if tftpboot ${loadaddr} ${tftpserverip}:${tftpkernelpath}; " \ + "then " \ + "bootm ${loadaddr}; "\ + "else " \ + "echo 'ERROR: Could not load kernel from TFTP'; " \ + "exit; " \ + "fi\0" \ + "tftp_ext2_boot=" \ + "run tftp_setup; " \ + "run regen_ext2_bootargs; " \ + "bootp; " \ + "if tftpboot ${loadaddr} ${tftpserverip}:${tftpkernelpath}; " \ + "then " \ + "bootm ${loadaddr}; " \ + "else " \ + "echo 'ERROR: Could not load kernel from TFTP'; " \ + "exit; " \ + "fi\0" \ + "nfsroot_boot=" \ + "run tftp_setup; " \ + "run regen_nfsroot_bootargs; " \ + "bootp; " \ + "if tftpboot ${loadaddr} ${tftpserverip}:${tftpkernelpath}; " \ + "then " \ + "bootm ${loadaddr}; " \ + "else " \ + "echo 'ERROR: Could not load kernel from TFTP'; " \ + "exit; " \ + "fi\0" \ + \ + "net_boot=" \ + "if test ${ethact} != ""; then " \ + "if test ${tftpserverip} != "0.0.0.0"; then " \ + "if test ${nfsserverip} != "0.0.0.0"; then " \ + "run nfsroot_boot; " \ + "fi; " \ + "if test ${has_initrd} != "0"; then " \ + "run initrdroot_boot; " \ + "else " \ + "run initramfs_boot; " \ + "fi; " \ + "fi; " \ + "fi\0" \ + +/* + * Our full set of extra enviornment variables. + * + * A few notes: + * - Right now, we can only boot from one USB device. Need to fix this once + * usb works better. + * - We define "non_verified_boot", which is the normal boot command unless + * it is overridden in the FDT. + * - When we're running securely, the FDT will specify to call vboot_twostop + * directly. + */ + +#ifdef CONFIG_CROS_RO +#define CONFIG_CHROMEOS_EXTRA_ENV_SETTINGS +#else +#define CONFIG_CHROMEOS_EXTRA_ENV_SETTINGS \ + CONFIG_REGEN_ALL_SETTINGS \ + CONFIG_EXT2_BOOT_HELPER_SETTINGS \ + CONFIG_NETBOOT_SETTINGS \ + \ + "set_devname=" \ + "part uuid ${devtype} ${devnum}:${rootpart} rootuuid; " \ + "setenv devname PARTUUID=${rootuuid}\0" \ + \ + "usb_boot=setenv devtype usb; " \ + "setenv devnum 0; " \ + "run run_disk_boot_script;" \ + "run set_devname; " \ + "run ext2_boot\0" \ + \ + "mmc_setup=" \ + "mmc dev ${devnum}; " \ + "mmc rescan; " \ + "setenv devtype mmc\0" \ + "mmc_boot=" \ + "run mmc_setup; " \ + "run run_disk_boot_script;" \ + "run set_devname; " \ + "run ext2_boot\0" \ + "mmc0_boot=setenv devnum 0; " \ + "run mmc_boot\0" \ + "mmc1_boot=setenv devnum 1; " \ + "run mmc_boot\0" \ + "mmc0_tftpboot=setenv devnum 0; " \ + "run mmc_setup; " \ + "run tftp_ext2_boot\0" \ + \ + "nvboot=" \ + "usb start; " \ + "run net_boot; " \ + "run usb_boot; " \ + \ + "run mmc1_boot; " \ + "run mmc0_boot\0" +#endif + +#define CONFIG_NON_VERIFIED_BOOTCOMMAND "run nvboot" + +#endif /* __configs_chromeos_h__ */
On 02/17/2015 03:29 PM, Simon Glass wrote:
From: Doug Anderson dianders@chromium.org
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.
diff --git a/include/configs/chromeos.h b/include/configs/chromeos.h
+/* 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?
+#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?
Hello Stephen,
diff --git a/include/configs/chromeos.h b/include/configs/chromeos.h
+/* 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?
U-Boot has already __stringify(), so this should not be necessary at all.
Best Regards, Thomas
Hi Stephen,
On 25 February 2015 at 16:28, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
From: Doug Anderson dianders@chromium.org
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.
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.
diff --git a/include/configs/chromeos.h b/include/configs/chromeos.h
+/* 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?
Yes I'll drop it since U-Boot has its own now.
+#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. + * + * The defines munging is thus kept to one Chrome OS-specific file. The + * upstream boards will define all features as normal for their platform. + * + * It is possible that these will be too broad for some platforms - e.g. we + * may have a platform which wants to use MMC in CONFIG_CROS_RO. However, + * we can deal with additional needs as we come to them. + * + * While it is something of an inconvenience, it may improve boot time on + * some platforms also. + * + * CONFIG_CROS_LARGE + * - Full version as now with factory defined, all features enabled + * - This will operate as factory build, or verified boot + * + * CONFIG_CROS_SMALL + * - Minimized for use only with verified boot + * - No command line, filesystems, LCD console, etc. + * - Still has all drivers enabled and can perform verified boot + * + * CONFIG_CROS_RO + * - Requires CONFIG_CROS_SMALL. Will only support running RO firmware + * - Set up for running VbLoadFirmware() only + * - Minimal RAM, no display, no USB, no mass storage (SPI flash only) + * - Intended for running in SPL + * + * CONFIG_CROS_RW + * - Requires CONFIG_CROS_SMALL. Will only support running RW firmware + * - Set up for running VbSelectAndLoadKernel() only
Regards, Simon
On 05/13/2015 07:19 AM, Simon Glass wrote:
Hi Stephen,
On 25 February 2015 at 16:28, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
From: Doug Anderson dianders@chromium.org
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.
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.
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.
+#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.
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?
Hi Stephen,
On 15 May 2015 at 09:27, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/13/2015 07:19 AM, Simon Glass wrote:
Hi Stephen,
On 25 February 2015 at 16:28, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
From: Doug Anderson dianders@chromium.org
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.
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.
OK I see.
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.
The command line really should be read from the storage. But chromeos.h uses a hard-coded command line. This is good enough to boot.
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.
OK. Well if we apply the other patches we at least get the code in. We won't be able to boot Chrome OS, but it's close.
+#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.
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?
What is missing here is the verified boot implementation. There are several patches that can sit on top of this patch to add full verified boot. The ones you identify are used by that code. I'm quite keen to move them to Kconfig but that work has not done yet.
My objective is to allow Chrome OS verified boot to be applied as a small number of patches on mainline.
Regards, Simon
Bring these in if they are provided by the board.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/configs/tegra-common-post.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h index 2de5d36..a004b0b 100644 --- a/include/configs/tegra-common-post.h +++ b/include/configs/tegra-common-post.h @@ -56,13 +56,18 @@ #define BOARD_EXTRA_ENV_SETTINGS #endif
+#ifndef CONFIG_CHROMEOS_EXTRA_ENV_SETTINGS +#define CONFIG_CHROMEOS_EXTRA_ENV_SETTINGS +#endif + #define CONFIG_EXTRA_ENV_SETTINGS \ TEGRA_DEVICE_SETTINGS \ MEM_LAYOUT_ENV_SETTINGS \ "fdt_high=ffffffff\0" \ "initrd_high=ffffffff\0" \ BOOTENV \ - BOARD_EXTRA_ENV_SETTINGS + BOARD_EXTRA_ENV_SETTINGS \ + CONFIG_CHROMEOS_EXTRA_ENV_SETTINGS
#if defined(CONFIG_TEGRA20_SFLASH) || defined(CONFIG_TEGRA20_SLINK) || defined(CONFIG_TEGRA114_SPI) #define CONFIG_TEGRA_SPI
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.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/configs/nyan-big.h | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/include/configs/nyan-big.h b/include/configs/nyan-big.h index 0b7d4d6..3f27756 100644 --- a/include/configs/nyan-big.h +++ b/include/configs/nyan-big.h @@ -12,6 +12,8 @@
#include "tegra124-common.h"
+#include <configs/chromeos.h> + /* High-level configuration options */ #define V_PROMPT "Tegra124 (Nyan-big) # " #define CONFIG_TEGRA_BOARD_STRING "Google/NVIDIA Nyan-big" @@ -79,10 +81,14 @@ #define CONFIG_CMD_DHCP
#define CONFIG_FIT +#define CONFIG_FIT_BEST_MATCH #define CONFIG_OF_LIBFDT
#define CONFIG_KEYBOARD
+#undef CONFIG_LOADADDR +#define CONFIG_LOADADDR 0x82408000 + #include "tegra-common-usb-gadget.h" #include "tegra-common-post.h"
On 02/17/2015 03:29 PM, Simon Glass wrote:
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,
On 25 February 2015 at 16:31, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
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?
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?
Regards, Simon
On 05/13/2015 07:56 AM, Simon Glass wrote:
Hi Stephen,
On 25 February 2015 at 16:31, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
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?
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,
On 15 May 2015 at 09:34, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/13/2015 07:56 AM, Simon Glass wrote:
Hi Stephen,
On 25 February 2015 at 16:31, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
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?
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.
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?
Regards, Simon
On 05/18/2015 03:33 PM, Simon Glass wrote:
Hi Stephen,
On 15 May 2015 at 09:34, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/13/2015 07:56 AM, Simon Glass wrote:
Hi Stephen,
On 25 February 2015 at 16:31, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
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?
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.
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!
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.
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".
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,
On 19 May 2015 at 09:41, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/18/2015 03:33 PM, Simon Glass wrote:
Hi Stephen,
On 15 May 2015 at 09:34, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/13/2015 07:56 AM, Simon Glass wrote:
Hi Stephen,
On 25 February 2015 at 16:31, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote:
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?
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.
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!
Do we need to adjust the mechanism? The only difference I see is that FIT brings the files together.
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.
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.
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.
Regards, Simon
On 05/19/2015 12:01 PM, Simon Glass wrote:
Hi Stephen,
On 19 May 2015 at 09:41, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/18/2015 03:33 PM, Simon Glass wrote:
Hi Stephen,
On 15 May 2015 at 09:34, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/13/2015 07:56 AM, Simon Glass wrote:
Hi Stephen,
On 25 February 2015 at 16:31, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/17/2015 03:29 PM, Simon Glass wrote: > > > > 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?
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.
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!
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.
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.
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.
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?
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.
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.
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,
On 19 May 2015 at 15:36, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/19/2015 12:01 PM, Simon Glass wrote:
Hi Stephen,
On 19 May 2015 at 09:41, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/18/2015 03:33 PM, Simon Glass wrote:
Hi Stephen,
On 15 May 2015 at 09:34, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/13/2015 07:56 AM, Simon Glass wrote:
Hi Stephen,
On 25 February 2015 at 16:31, Stephen Warren swarren@wwwdotorg.org wrote: > > > > On 02/17/2015 03:29 PM, Simon Glass wrote: >> >> >> >> >> 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?
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.
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!
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.
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.
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.
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.
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?
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.
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.
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.
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...
Regards, Simon
On 05/19/2015 05:27 PM, Simon Glass wrote:
+Tom Rini in case he is interested...
Hi Stephen,
On 19 May 2015 at 15:36, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/19/2015 12:01 PM, Simon Glass wrote:
Hi Stephen,
On 19 May 2015 at 09:41, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/18/2015 03:33 PM, Simon Glass wrote:
Hi Stephen,
On 15 May 2015 at 09:34, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/13/2015 07:56 AM, Simon Glass wrote: > > > > Hi Stephen, > > On 25 February 2015 at 16:31, Stephen Warren swarren@wwwdotorg.org > wrote: >> >> >> >> On 02/17/2015 03:29 PM, Simon Glass wrote: >>> >>> >>> >>> >>> 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? > > > > > 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.
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!
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.
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.
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.
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.
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?
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?
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.
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.
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.
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.
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.
Hi Stephen,
On 19 May 2015 at 19:44, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/19/2015 05:27 PM, Simon Glass wrote:
+Tom Rini in case he is interested...
Hi Stephen,
On 19 May 2015 at 15:36, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/19/2015 12:01 PM, Simon Glass wrote:
Hi Stephen,
On 19 May 2015 at 09:41, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/18/2015 03:33 PM, Simon Glass wrote:
Hi Stephen,
On 15 May 2015 at 09:34, Stephen Warren swarren@wwwdotorg.org wrote: > > > On 05/13/2015 07:56 AM, Simon Glass wrote: >> >> >> >> Hi Stephen, >> >> On 25 February 2015 at 16:31, Stephen Warren swarren@wwwdotorg.org >> wrote: >>> >>> >>> >>> On 02/17/2015 03:29 PM, Simon Glass wrote: >>>> >>>> >>>> >>>> >>>> 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? >> >> >> >> >> 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.
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!
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.
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.
Yes I understand that, but what are you getting at? FIT would work the same way if implemented there.
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.
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.
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?
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?
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.
My reading of the code is that the kernel is always compiled to run at c0008000 and that the early code is relocatable so that it can be run anywhere. That doesn't seem to have changed in years. This seems to be different from x86 where in general the code seems to be loaded at a particular place.
Given what you say above, Linux should be able to be loaded almost anywhere in the first 128MB of physical memory. It then decompresses to 32KB above the start of physical memory and jumps to itself there.
We should also be able to load the uncompressed kernel 'Image' (arch/arm/boot/Image) at any place which is an offset 0x8000 from the start of a 1MB boundary. This used to work but I have not tried it recently.
Given that the boot loader has to load the kernel into the bottom of physical memory, it doesn't seem to be to be any harder for the boot loader to load the kernel to an address 32KB above a 1MB boundary. Effectively the kernel relies on being loaded to a sensible address. In no sense is the boot loader deprived of the need to know where to put the kernel by this scheme.
And from what I can tell the kernel itself (i.e. without the 1300 lines of head.S) doesn't care where it is loaded, within reason.
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.
OK I see what you are saying. It's a bit ugly. We want the compressed kernel to be loaded in a place from which we can infer, through some convention, the start of physical RAM. The incoming PC is effectively a parameter. Another way would be to make that an explicit parameter to the kernel, or perhaps use the device tree.
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.
OK so perhaps that is one option. Although I wonder whether we should just allow U-Boot to place the kernel at some offset from the start of RAM and that would be good enough for any platform? Why do we need this kernel address env variable?
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.
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.
OK I see.
How does the kernel deal with multiple device tree files and the boot loader selecting the correct one?
Regards, Simon
Hi Simon,
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!
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.
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.
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.
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.
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.
Peter
Hi Peter,
On 20 May 2015 at 04:21, Peter Robinson pbrobinson@gmail.com wrote:
Hi Simon,
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!
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.
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.
U-Boot has a mechanism to select the correct DT based on the model of the board it is running on.
Then I asked if we could go a step further and use FIT compression (it can specify an algorithm to use) instead of bzImage so that all the 'loading' happens in the boot loader. The problem of the kernel load address came up, as below. I believe that (ARM-specific problem?) can be solved by perhaps placing the kernel at address 0x8000 in physical memory, for example, something the boot loader can do. But that is yet to be tested and might be an addition to 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.
I disagree that FIT is a good idea, but that's a separate topic.
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.
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.
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.
It does support that. I believe that was one of the design goals, albeit for PowerPC at the time. There is also CONFIG_FIT_BEST_MATCH in U-Boot which can select the correct DT for the board.
You could try this - the verified boot part of it is documented also:
http://git.denx.de/?p=u-boot.git;a=tree;f=doc/uImage.FIT;hb=HEAD
While you are here, what boot loaders do people mostly use with Fedora on ARM?
Regards, Simon
On 05/20/2015 07:40 AM, Simon Glass wrote:
Hi Peter,
On 20 May 2015 at 04:21, Peter Robinson pbrobinson@gmail.com wrote:
Hi Simon,
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!
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.
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.
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?
Hi Stephen,
On 20 May 2015 at 08:04, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/20/2015 07:40 AM, Simon Glass wrote:
Hi Peter,
On 20 May 2015 at 04:21, Peter Robinson pbrobinson@gmail.com wrote:
Hi Simon,
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!
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.
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.
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?
Chrome OS keeps its script in a funny place, or we can use the A/B flag directly with a lot more code. But to boot partition A is not too far away - I had it finding and starting the kernel (but with the wrong bootargs). It needs more work though.
Regards, Simon
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)
On Tue, 2015-02-17 at 15:29 -0700, Simon Glass wrote:
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
Hi Sjoerd,
On 25 February 2015 at 17:12, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
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)
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.
Regards, Simon
On Fri, 2015-02-27 at 22:11 -0700, Simon Glass wrote:
Hi Sjoerd,
On 25 February 2015 at 17:12, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
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)
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).
Hi Sjoerd,
On 30 March 2015 at 02:14, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
On Fri, 2015-02-27 at 22:11 -0700, Simon Glass wrote:
Hi Sjoerd,
On 25 February 2015 at 17:12, Sjoerd Simons sjoerd.simons@collabora.co.uk wrote:
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)
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).
Nice work!
I have the data cache enabled. USB does not work. MMC works OK (I can boot a kernel from the internal eMMC).
I'm not sure about the cache, it seems odd. I wonder if there is something broken in the cache set, and U-Boot needs to fix it?
Regards, Simon
participants (5)
-
Peter Robinson -
Simon Glass -
Sjoerd Simons -
Stephen Warren -
thomas.langer＠lantiq.com