usb: add device connection/disconnection detection

From: Vincent Palatin <[email protected]>

Provide a function to detect USB device insertion/removal in order to avoid having to do USB enumeration in a tight loop when trying to detect peripheral hotplugging.

Signed-off-by Vincent Palatin <[email protected]>

Reviewed-by Stefan Reinauer <[email protected]>

Tested-by Stefan Reinauer <[email protected]>

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

---

 common/usb.c     | 26 ++++++++++++++++++++++++++
 common/usb_hub.c |  2 +-
 include/usb.h    |  2 ++
 3 files changed, 29 insertions(+), 1 deletion(-)

Support this function with driver model also (CONFIG_DM_USB).

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

---

 drivers/usb/host/usb-uclass.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

drivers/usb/host/usb-uclass.c +10 -0

index 714bc0e..8445a42 100644
@@ -555,6 +555,49 @@ int usb_scan_device(struct udevice *parent, int port,
        return 0;
 }
 
+/*
+ * Detect if a USB device has been plugged or unplugged.
+ */
+int usb_detect_change(void)
+{
+       struct udevice *hub;
+       struct uclass *uc;
+       int change = 0;
+       int ret;
+

drivers/usb/host/usb-uclass.c +8 -0

+                       if (usb_get_port_status(udev, udev->portnr, &status)
+                                       < 0)
+                               /* USB request failed */
+                               continue;
+
+                       if (le16_to_cpu(status.wPortChange) &
+                           USB_PORT_STAT_C_CONNECTION)
+                               change++;

We maintain an accumulator for time spent reading from SPI flash, since this can be significant on some platforms. Also add one for decompression time.

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

---

Hi Simon,

Quoted 6 lines · Simon Glass

We maintain an accumulator for time spent reading from SPI flash, since this can be significant on some platforms. Also add one for decompression time.

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

---

Reviewed-by Joe Hershberger <[email protected]>

Quoted 7 lines · Joe Hershberger

Hi Simon,

Quoted 4 lines · Simon Glass

We maintain an accumulator for time spent reading from SPI flash, since this can be significant on some platforms. Also add one for decompression time.

Quoted 3 lines

Quoted 2 lines

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

---

Reviewed-by Joe Hershberger <[email protected]>

Reviewed-by Jagan Teki <[email protected]>

This command was missed in the conversion. Add it back for driver model.

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

---

 common/cmd_i2c.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 55 insertions(+), 8 deletions(-)

common/cmd_i2c.c +53 -6

index ad38cbf..1bc0db8 100644
@@ -1623,6 +1623,27 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 }
 #endif /* CONFIG_I2C_EDID */
 
+#ifdef CONFIG_DM_I2C
+static void show_bus(struct udevice *bus)
+{
+       struct udevice *dev;
+
+       printf("Bus %d:\t%s", bus->req_seq, bus->name);
+       if (device_active(bus))
+               printf("  (active %d)", bus->seq);
+       printf("\n");
+       for (device_find_first_child(bus, &dev);
+            dev;
+            device_find_next_child(&dev)) {
+               struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
+
+               printf("   %02x: %s, offset len %x, flags %x\n",
+                      chip->chip_addr, dev->name, chip->offset_len,
+                      chip->flags);
+       }
+}
+#endif
+
 /**
  * do_i2c_show_bus() - Handle the "i2c bus" command-line command
  * @cmdtp:     Command data struct pointer

@@ -1632,20 +1653,30 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
  *
  * Returns zero always.
  */
-#if defined(CONFIG_SYS_I2C)
+#if defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C)
 static int do_i2c_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
                                char * const argv[])
 {
-       int     i;
-#ifndef CONFIG_SYS_I2C_DIRECT_BUS
-       int     j;
-#endif
-
        if (argc == 1) {
                /* show all busses */
+#ifdef CONFIG_DM_I2C
+               struct udevice *bus;
+               struct uclass *uc;
+               int ret;
+
+               ret = uclass_get(UCLASS_I2C, &uc);
+               if (ret)
+                       return CMD_RET_FAILURE;
+               uclass_foreach_dev(bus, uc)
+                       show_bus(bus);
+#else
+               int i;
+
                for (i = 0; i < CONFIG_SYS_NUM_I2C_BUSES; i++) {
                        printf("Bus %d:\t%s", i, I2C_ADAP_NR(i)->name);
 #ifndef CONFIG_SYS_I2C_DIRECT_BUS
+                       int j;
+
                        for (j = 0; j < CONFIG_SYS_I2C_MAX_HOPS; j++) {
                                if (i2c_bus[i].next_hop[j].chip == 0)
                                        break;

@@ -1657,15 +1688,30 @@ static int do_i2c_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
 #endif
                        printf("\n");
                }
+#endif
        } else {
+               int i;
+
                /* show specific bus */
                i = simple_strtoul(argv[1], NULL, 10);
+#ifdef CONFIG_DM_I2C
+               struct udevice *bus;
+               int ret;
+
+               ret = uclass_get_device_by_seq(UCLASS_I2C, i, &bus);
+               if (ret) {
+                       printf("Invalid bus %d: err=%d\n", i, ret);
+                       return CMD_RET_FAILURE;
+               }
+               show_bus(bus);
+#else
                if (i >= CONFIG_SYS_NUM_I2C_BUSES) {
                        printf("Invalid bus %d\n", i);
                        return -1;
                }
                printf("Bus %d:\t%s", i, I2C_ADAP_NR(i)->name);
 #ifndef CONFIG_SYS_I2C_DIRECT_BUS
+                       int j;
                        for (j = 0; j < CONFIG_SYS_I2C_MAX_HOPS; j++) {
                                if (i2c_bus[i].next_hop[j].chip == 0)
                                        break;

@@ -1676,6 +1722,7 @@ static int do_i2c_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
                        }
 #endif
                printf("\n");
+#endif
        }
 
        return 0;

common/cmd_i2c.c +1 -2

        U_BOOT_CMD_MKENT(bus, 1, 1, do_i2c_show_bus, "", ""),
 #endif
        U_BOOT_CMD_MKENT(crc32, 3, 1, do_i2c_crc, "", ""),

@@ -1902,7 +1949,7 @@ static int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
 /***************************************************/
 #ifdef CONFIG_SYS_LONGHELP
 static char i2c_help_text[] =
-#if defined(CONFIG_SYS_I2C)
+#if defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C)
        "bus [muxtype:muxaddr:muxchannel] - show I2C bus info\n"
 #endif
        "crc32 chip address[.0, .1, .2] count - compute CRC32 checksum\n"
--

2.2.0.rc0.207.ga3a616c

This command was missed in the conversion. Add it back for driver model.

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

---

  common/cmd_i2c.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-------
  1 file changed, 55 insertions(+), 8 deletions(-)

common/cmd_i2c.c +30 -6

index ad38cbf..1bc0db8 100644
@@ -1623,6 +1623,27 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
  }
  #endif /* CONFIG_I2C_EDID */
 
+#ifdef CONFIG_DM_I2C
+static void show_bus(struct udevice *bus)
+{
+	struct udevice *dev;
+
+	printf("Bus %d:\t%s", bus->req_seq, bus->name);
+	if (device_active(bus))
+		printf("  (active %d)", bus->seq);
+	printf("\n");
+	for (device_find_first_child(bus, &dev);
+	     dev;
+	     device_find_next_child(&dev)) {
+		struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
+
+		printf("   %02x: %s, offset len %x, flags %x\n",
+		       chip->chip_addr, dev->name, chip->offset_len,
+		       chip->flags);
+	}
+}
+#endif
+
  /**
   * do_i2c_show_bus() - Handle the "i2c bus" command-line command
   * @cmdtp:	Command data struct pointer

@@ -1632,20 +1653,30 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
   *
   * Returns zero always.
   */
-#if defined(CONFIG_SYS_I2C)
+#if defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C)
  static int do_i2c_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
  				char * const argv[])
  {
-	int	i;
-#ifndef CONFIG_SYS_I2C_DIRECT_BUS
-	int	j;
-#endif
-
  	if (argc == 1) {
  		/* show all busses */
+#ifdef CONFIG_DM_I2C
+		struct udevice *bus;
+		struct uclass *uc;
+		int ret;
+
+		ret = uclass_get(UCLASS_I2C, &uc);
+		if (ret)
+			return CMD_RET_FAILURE;

common/cmd_i2c.c +18 -2

  			for (j = 0; j < CONFIG_SYS_I2C_MAX_HOPS; j++) {
  				if (i2c_bus[i].next_hop[j].chip == 0)
  					break;

@@ -1657,15 +1688,30 @@ static int do_i2c_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
  #endif
  			printf("\n");
  		}
+#endif
  	} else {
+		int i;
+
  		/* show specific bus */
  		i = simple_strtoul(argv[1], NULL, 10);
+#ifdef CONFIG_DM_I2C
+		struct udevice *bus;
+		int ret;
+
+		ret = uclass_get_device_by_seq(UCLASS_I2C, i, &bus);
+		if (ret) {
+			printf("Invalid bus %d: err=%d\n", i, ret);
+			return CMD_RET_FAILURE;
+		}
+		show_bus(bus);
+#else
  		if (i >= CONFIG_SYS_NUM_I2C_BUSES) {
  			printf("Invalid bus %d\n", i);
  			return -1;
  		}
  		printf("Bus %d:\t%s", i, I2C_ADAP_NR(i)->name);
  #ifndef CONFIG_SYS_I2C_DIRECT_BUS
+			int j;
  			for (j = 0; j < CONFIG_SYS_I2C_MAX_HOPS; j++) {
  				if (i2c_bus[i].next_hop[j].chip == 0)
  					break;

@@ -1676,6 +1722,7 @@ static int do_i2c_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
  			}
  #endif
  		printf("\n");
+#endif
  	}
 
  	return 0;

@@ -1835,7 +1882,7 @@ static int do_i2c_reset(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv
  }
 
  static cmd_tbl_t cmd_i2c_sub[] = {
-#if defined(CONFIG_SYS_I2C)
+#if defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C)
  	U_BOOT_CMD_MKENT(bus, 1, 1, do_i2c_show_bus, "", ""),
  #endif
  	U_BOOT_CMD_MKENT(crc32, 3, 1, do_i2c_crc, "", ""),

@@ -1902,7 +1949,7 @@ static int do_i2c(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
  /***************************************************/
  #ifdef CONFIG_SYS_LONGHELP
  static char i2c_help_text[] =
-#if defined(CONFIG_SYS_I2C)
+#if defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C)
  	"bus [muxtype:muxaddr:muxchannel] - show I2C bus info\n"
  #endif
  	"crc32 chip address[.0, .1, .2] count - compute CRC32 checksum\n"

Hello Simon,

Am 04.05.2015 19:30, schrieb Simon Glass:

Quoted 68 lines

This command was missed in the conversion. Add it back for driver model.

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

---

  common/cmd_i2c.c | 63
+++++++++++++++++++++++++++++++++++++++++++++++++-------
  1 file changed, 55 insertions(+), 8 deletions(-)

common/cmd_i2c.c +21 -0

index ad38cbf..1bc0db8 100644
@@ -1623,6 +1623,27 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc,
 char *const argv[])
  }
  #endif /* CONFIG_I2C_EDID */
 
+#ifdef CONFIG_DM_I2C
+static void show_bus(struct udevice *bus)
+{
+       struct udevice *dev;
+
+       printf("Bus %d:\t%s", bus->req_seq, bus->name);
+       if (device_active(bus))
+               printf("  (active %d)", bus->seq);
+       printf("\n");
+       for (device_find_first_child(bus, &dev);
+            dev;
+            device_find_next_child(&dev)) {
+               struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
+
+               printf("   %02x: %s, offset len %x, flags %x\n",
+                      chip->chip_addr, dev->name, chip->offset_len,
+                      chip->flags);
+       }
+}
+#endif
+
  /**
   * do_i2c_show_bus() - Handle the "i2c bus" command-line command

* @cmdtp: Command data struct pointer

common/cmd_i2c.c +9 -6

@@ -1632,20 +1653,30 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc,
 char *const argv[])
   *
   * Returns zero always.
   */
-#if defined(CONFIG_SYS_I2C)
+#if defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C)
  static int do_i2c_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
                                char * const argv[])
  {
-       int     i;
-#ifndef CONFIG_SYS_I2C_DIRECT_BUS
-       int     j;
-#endif
-
        if (argc == 1) {
                /* show all busses */
+#ifdef CONFIG_DM_I2C
+               struct udevice *bus;
+               struct uclass *uc;
+               int ret;
+
+               ret = uclass_get(UCLASS_I2C, &uc);
+               if (ret)
+                       return CMD_RET_FAILURE;

common/cmd_i2c.c int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, · line 1680

Quoted 10 lines

common/cmd_i2c.c +7 -0

+               uclass_foreach_dev(bus, uc)
+                       show_bus(bus);
+#else
+               int i;
+
                for (i = 0; i < CONFIG_SYS_NUM_I2C_BUSES; i++) {
                        printf("Bus %d:\t%s", i, I2C_ADAP_NR(i)->name);
  #ifndef CONFIG_SYS_I2C_DIRECT_BUS
+                       int j;
+

I thought this drops an error, because var j is declared in the middle of code, but just tried it with gcc 4.7.2 and gcc 4.8.1 vor the mgcoge board, drops no warning, so:

Acked-by Heiko Schocher <[email protected]>

Hi Heiko,

Quoted 88 lines · Heiko Schocher

Hello Simon,

Am 04.05.2015 19:30, schrieb Simon Glass:

Quoted 68 lines

This command was missed in the conversion. Add it back for driver model.

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

---

  common/cmd_i2c.c | 63
+++++++++++++++++++++++++++++++++++++++++++++++++-------
  1 file changed, 55 insertions(+), 8 deletions(-)

common/cmd_i2c.c +21 -0

index ad38cbf..1bc0db8 100644
@@ -1623,6 +1623,27 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc,
 char *const argv[])
  }
  #endif /* CONFIG_I2C_EDID */
 
+#ifdef CONFIG_DM_I2C
+static void show_bus(struct udevice *bus)
+{
+       struct udevice *dev;
+
+       printf("Bus %d:\t%s", bus->req_seq, bus->name);
+       if (device_active(bus))
+               printf("  (active %d)", bus->seq);
+       printf("\n");
+       for (device_find_first_child(bus, &dev);
+            dev;
+            device_find_next_child(&dev)) {
+               struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
+
+               printf("   %02x: %s, offset len %x, flags %x\n",
+                      chip->chip_addr, dev->name, chip->offset_len,
+                      chip->flags);
+       }
+}
+#endif
+
  /**
   * do_i2c_show_bus() - Handle the "i2c bus" command-line command

* @cmdtp: Command data struct pointer

common/cmd_i2c.c +9 -6

@@ -1632,20 +1653,30 @@ int do_edid(cmd_tbl_t *cmdtp, int flag, int argc,
 char *const argv[])
   *
   * Returns zero always.
   */
-#if defined(CONFIG_SYS_I2C)
+#if defined(CONFIG_SYS_I2C) || defined(CONFIG_DM_I2C)
  static int do_i2c_show_bus(cmd_tbl_t *cmdtp, int flag, int argc,
                                char * const argv[])
  {
-       int     i;
-#ifndef CONFIG_SYS_I2C_DIRECT_BUS
-       int     j;
-#endif
-
        if (argc == 1) {
                /* show all busses */
+#ifdef CONFIG_DM_I2C
+               struct udevice *bus;
+               struct uclass *uc;
+               int ret;
+
+               ret = uclass_get(UCLASS_I2C, &uc);
+               if (ret)
+                       return CMD_RET_FAILURE;

common/cmd_i2c.c int do_edid(cmd_tbl_t *cmdtp, int flag, int argc, · line 1680

Quoted 10 lines

common/cmd_i2c.c +7 -0

+               uclass_foreach_dev(bus, uc)
+                       show_bus(bus);
+#else
+               int i;
+
                for (i = 0; i < CONFIG_SYS_NUM_I2C_BUSES; i++) {
                        printf("Bus %d:\t%s", i, I2C_ADAP_NR(i)->name);
  #ifndef CONFIG_SYS_I2C_DIRECT_BUS
+                       int j;
+

I thought this drops an error, because var j is declared in the middle of code, but just tried it with gcc 4.7.2 and gcc 4.8.1 vor the mgcoge board, drops no warning, so:

Quoted 1 line

Acked-by Heiko Schocher <[email protected]>

Yes, I probably shouldn't do that.

[snip]

Regards,
Simon

We can currently set this but there is no API function to get it. Add one.

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

---

We can currently set this but there is no API function to get it. Add one.

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

---

  drivers/i2c/i2c-uclass.c | 7 +++++++
  include/i2c.h            | 9 ++++++++-
  2 files changed, 15 insertions(+), 1 deletion(-)

drivers/i2c/i2c-uclass.c +7 -0

index f2e95c0..2dfb435 100644
@@ -396,6 +396,13 @@ int i2c_set_chip_offset_len(struct udevice *dev, uint offset_len)
  	return 0;
  }
 
+int i2c_get_chip_offset_len(struct udevice *dev)
+{
+	struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
+
+	return chip->offset_len;
+}
+
  int i2c_deblock(struct udevice *bus)
  {
  	struct dm_i2c_ops *ops = i2c_get_ops(bus);

include/i2c.h +8 -1

index 6fd73fa..9a9e4f7 100644
@@ -171,8 +171,15 @@ int i2c_get_chip_flags(struct udevice *dev, uint *flagsp);
   *
   * @offset_len:	New offset length value (typically 1 or 2)
   */
-
  int i2c_set_chip_offset_len(struct udevice *dev, uint offset_len);
+
+/**
+ * i2c_get_offset_len() - get the offset length for a chip
+ *
+ * @return:	Current offset length value (typically 1 or 2)
+ */
+int i2c_get_chip_offset_len(struct udevice *dev);
+
  /**
   * i2c_deblock() - recover a bus that is in an unknown state
   *

Hello Simon,

Am 04.05.2015 19:30, schrieb Simon Glass:

Quoted 6 lines

We can currently set this but there is no API function to get it. Add one.

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

---

Quoted 3 lines

  drivers/i2c/i2c-uclass.c | 7 +++++++
  include/i2c.h            | 9 ++++++++-
  2 files changed, 15 insertions(+), 1 deletion(-)

Thanks!

Acked-by Heiko Schocher<[email protected]>

As a first step towards converting the TPM system to driver model, allow it to work with CONFIG_DM_I2C.

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

---

 drivers/tpm/tpm.c         |  89 +++++++++++++++++++++++++-----
 drivers/tpm/tpm_private.h |   3 ++
 drivers/tpm/tpm_tis_i2c.c | 134 +++++++++++++++++++++++++++++++---------------
 3 files changed, 170 insertions(+), 56 deletions(-)

This name is used in Linux, so use it in U-Boot.

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

---

 arch/arm/dts/exynos5420-peach-pit.dts | 2 +-
 arch/arm/dts/exynos5800-peach-pi.dts  | 2 +-
 lib/fdtdec.c                          | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

This should say 'in', not 'out'.

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

---

 include/asm-generic/gpio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Commit 47ed5dd0 dropped the .got section from U-Boot binaries. This is needed for some relocations, and causes failures if missing. Add it back.

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

---

 arch/arm/config.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

It is convenient for some boards to implement save_boot_params() in C rather than assembler. Provide a way to return in this case.

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

---

Hi Simon,

Quoted 5 lines · Simon Glass

It is convenient for some boards to implement save_boot_params() in C rather than assembler. Provide a way to return in this case.

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

---

Reviewed-by Joe Hershberger <[email protected]>

For secure boot systems it is common to have a read-only U-Boot which starts the machine and jumps to a read-write U-Boot for actual booting the OS. This allows the read-write U-Boot to be upgraded without risk of permanently bricking the machine. In the event that the read-write U-Boot is corrupted, the read-only U-Boot can detect this with a checksum and boot into a recovery flow.

To support this, add a way to detect when U-Boot is run from SPL as opposed to some other method, such as booted directly (no SPL) or started from another source (e.g. a primary U-Boot). This works by putting a special value in r0.

For now we rely on board-specific code to actually check the register and set a flag. At some point this could be generalised, perhaps by using a spare register and passing a flag to _main and/or board_init_f().

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

---

 include/spl.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

include/spl.h +5 -0

index b2e5bf7..cdd63a7 100644
@@ -11,6 +11,8 @@
 #include <linux/compiler.h>
 #include <asm/spl.h>
 
+/* Value in r0 indicates we booted from U-Boot */
+#define SPL_RUNNING_FROM_UBOOT 0x13578642
 
 /* Boot type */
 #define MMCSD_MODE_UNDEFINED   0

@@ -82,4 +84,15 @@ int spl_load_image_ext_os(block_dev_desc_t *block_dev, int partition);
 #ifdef CONFIG_SPL_BOARD_INIT
 void spl_board_init(void);
 #endif
+
+/**
+ * spl_was_boot_source() - check if U-Boot booted from SPL

This function is used before jumping to U-Boot, but in that case we don't always want to disable caches.

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

Signed-off-by Vadim Bendebury <[email protected]>

Quoted 5 lines · Simon Glass

This function is used before jumping to U-Boot, but in that case we don't always want to disable caches.

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

Signed-off-by Vadim Bendebury <[email protected]>

In which usecase exactly is this applicable please ? :)

Hi Marek,

Quoted 8 lines · Simon Glass

Quoted 5 lines

This function is used before jumping to U-Boot, but in that case we don't always want to disable caches.

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

Signed-off-by Vadim Bendebury <[email protected]>

In which usecase exactly is this applicable please ? :)

This is sort-of related to "Enable detecting when U-Boot is started from SPL" so there are some details there.

I was able to get SPL to enable the cache and leave it on all the way through the verified boot flow (read-only and read-write U-Boot) and it saved about 600ms. So in short this refactor allows you to jump into U-Boot with the cache still enabled.

This function should return a useful error for U-Boot, rather than -1.

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

---

Hi Simon,

Quoted 4 lines · Simon Glass

This function should return a useful error for U-Boot, rather than -1.

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

---

Reviewed-by Joe Hershberger <[email protected]>

These files use error numbering, so add the include.

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

---

 arch/sandbox/cpu/start.c       | 1 +
 drivers/misc/i2c_eeprom_emul.c | 1 +
 drivers/misc/swap_case.c       | 1 +
 3 files changed, 3 insertions(+)

Emualate this function which is used with Chrome OS verified boot.

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

---

 drivers/misc/cros_ec_sandbox.c |  2 ++
 include/ec_commands.h          | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

This printf() should have a newline at the end. Add it.

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

---

Hi Simon,

Quoted 4 lines · Simon Glass

This printf() should have a newline at the end. Add it.

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

---

Reviewed-by Joe Hershberger <[email protected]>

Quoted 7 lines · Joe Hershberger

Hi Simon,

Quoted 4 lines · Simon Glass

This printf() should have a newline at the end. Add it.

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

---

Quoted 1 line

Reviewed-by Joe Hershberger <[email protected]>

Reviewed-by Jagan Teki <[email protected]>

thanks!
--
Jagan Teki,
Openedev.

This fixes a warning in the print_buffer() function with some toolchains.

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

---

Hi Simon,

Quoted 4 lines · Simon Glass

This fixes a warning in the print_buffer() function with some toolchains.

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

---

Reviewed-by Joe Hershberger <[email protected]>

Increase the LCD size to 1366x768.

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

---

Hi Simon,

Quoted 4 lines · Simon Glass

Increase the LCD size to 1366x768.

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

---

Is this actually emulated somehow? Maybe I just don't see it since I'm SSH'ed into my build machine?

Hi Joe,

Quoted 8 lines · Joe Hershberger

Hi Simon,

Quoted 4 lines · Simon Glass

Increase the LCD size to 1366x768.

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

---

Quoted 2 lines

Is this actually emulated somehow? Maybe I just don't see it since I'm SSH'ed into my build machine?

Yes, plus you need the -l option.

Regards,
Simon

For 16-bit-per-pixel displays it is useful to support 8 bit-per-pixel images to reduce image size. Add support for this when drawing BMP images.

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

---

 common/lcd.c         | 23 ++++++++++++++++++++---
 include/bmp_layout.h |  4 ++--
 2 files changed, 22 insertions(+), 5 deletions(-)

common/lcd.c +20 -3

index 055c366..2d11578 100644
@@ -578,6 +578,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
        unsigned long width, height, byte_width;
        unsigned long pwidth = panel_info.vl_col;
        unsigned colors, bpix, bmp_bpix;
+       int hdr_size;
+       bmp_color_table_entry_t *palette = bmp->color_table;
 
        if (!bmp || !(bmp->header.signature[0] == 'B' &&
                bmp->header.signature[1] == 'M')) {

@@ -589,6 +591,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
        width = get_unaligned_le32(&bmp->header.width);
        height = get_unaligned_le32(&bmp->header.height);
        bmp_bpix = get_unaligned_le16(&bmp->header.bit_count);
+       hdr_size = get_unaligned_le16(&bmp->header.size);
+       debug("hdr_size=%d, bmp_bpix=%d\n", hdr_size, bmp_bpix);
 
        colors = 1 << bmp_bpix;
 

@@ -613,8 +617,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
                return 1;
        }
 
-       debug("Display-bmp: %d x %d  with %d colors\n",
-               (int)width, (int)height, (int)colors);
+       debug("Display-bmp: %d x %d  with %d colors, display %d\n",
+             (int)width, (int)height, (int)colors, 1 << bpix);
 
        if (bmp_bpix == 8)
                lcd_set_cmap(bmp, colors);

@@ -641,6 +645,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
                cmap_base = configuration_get_cmap();
 #ifdef CONFIG_LCD_BMP_RLE8
                u32 compression = get_unaligned_le32(&bmp->header.compression);
+               debug("compressed %d %d\n", compression, BMP_BI_RLE8);
                if (compression == BMP_BI_RLE8) {
                        if (bpix != 16) {
                                /* TODO implement render code for bpix != 16 */

@@ -663,7 +668,19 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)
                                if (bpix != 16) {
                                        fb_put_byte(&fb, &bmap);
                                } else {
-                                       *(uint16_t *)fb = cmap_base[*(bmap++)];
+                                       struct bmp_color_table_entry *entry;
+                                       uint val;
+
+                                       if (cmap_base) {
+                                               val = cmap_base[*bmap];
+                                       } else {
+                                               entry = &palette[*bmap];
+                                               val = entry->blue >> 3 |
+                                                       entry->green >> 2 << 5 |
+                                                       entry->red >> 3 << 11;
+                                       }
+                                       *(uint16_t *)fb = val;
+                                       bmap++;
                                        fb += sizeof(uint16_t) / sizeof(*fb);
                                }
                        }

include/bmp_layout.h +0 -0

index 22b1fbc..47f505c 100644

Quoted 5 lines · Simon Glass

Add an implementation of this function for Tegra.

arch/arm/mach-tegra/board.c enum { · line 41

Quoted 9 lines

arch/arm/mach-tegra/board.c +7 -0

 
 
+#ifndef CONFIG_SPL_BUILD
+void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3)
+{
+       from_spl = r0 != SPL_RUNNING_FROM_UBOOT;
+       save_boot_params_ret();
+}
+#endif

(Using terminology from:
https://patchwork.ozlabs.org/patch/467771/
arm: spl: Enable detecting when U-Boot is started from SPL )

That doesn't look right. Surely (at least on Tegra), if the r/o U-Boot chain-loads to the r/w U-Boot, then the chain-loaded U-Boot has no SPL and is just the main CPU build of U-Boot.

Hence, "SPL_RUNNING_FROM_UBOOT" seems incorrectly named, since the r/o U-Boot doesn't chain to SPL but rather to U-Boot.

This approach sounds a little brittle; what happens if r0 just happens to have that value. Won't the code get confused?

Why does U-Boot care whether it's been chain-loaded? Shouldn't it always behave identically in all cases, so it's independent of what caused it to run?

Hi Stephen,

Quoted 26 lines · Stephen Warren

Quoted 5 lines · Simon Glass

Add an implementation of this function for Tegra.

arch/arm/mach-tegra/board.c enum { · line 41

Quoted 9 lines

arch/arm/mach-tegra/board.c +7 -0

 
 
+#ifndef CONFIG_SPL_BUILD
+void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3)
+{
+       from_spl = r0 != SPL_RUNNING_FROM_UBOOT;
+       save_boot_params_ret();
+}
+#endif

(Using terminology from:
https://patchwork.ozlabs.org/patch/467771/
arm: spl: Enable detecting when U-Boot is started from SPL )

That doesn't look right. Surely (at least on Tegra), if the r/o U-Boot chain-loads to the r/w U-Boot, then the chain-loaded U-Boot has no SPL and is just the main CPU build of U-Boot.

Quoted 2 lines

Hence, "SPL_RUNNING_FROM_UBOOT" seems incorrectly named, since the r/o U-Boot doesn't chain to SPL but rather to U-Boot.

What name do you suggest? I was trying to add a prefix indicating that it relates to non-SPL start-up of U-Boot.

Quoted 2 lines

This approach sounds a little brittle; what happens if r0 just happens to have that value. Won't the code get confused?

Yes, but SPL does not set that value in r0, and we have control over this.

Quoted 3 lines

Why does U-Boot care whether it's been chain-loaded? Shouldn't it always behave identically in all cases, so it's independent of what caused it to run?

In the case of read-only U-Boot it must find the read-write one to jump to. In the case of read-write U-Boot it must boot a kernel.

Quoted 40 lines · Simon Glass

Hi Stephen,

Quoted 34 lines · Stephen Warren

Quoted 7 lines · Simon Glass

Add an implementation of this function for Tegra.

arch/arm/mach-tegra/board.c enum { · line 41

Quoted 10 lines

arch/arm/mach-tegra/board.c +7 -0

 
 
 
+#ifndef CONFIG_SPL_BUILD
+void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3)
+{
+       from_spl = r0 != SPL_RUNNING_FROM_UBOOT;
+       save_boot_params_ret();
+}
+#endif

(Using terminology from:
https://patchwork.ozlabs.org/patch/467771/
arm: spl: Enable detecting when U-Boot is started from SPL )

That doesn't look right. Surely (at least on Tegra), if the r/o U-Boot chain-loads to the r/w U-Boot, then the chain-loaded U-Boot has no SPL and
is just the main CPU build of U-Boot.

Hence, "SPL_RUNNING_FROM_UBOOT" seems incorrectly named, since the r/o U-Boot doesn't chain to SPL but rather to U-Boot.

Quoted 2 lines

What name do you suggest? I was trying to add a prefix indicating that it relates to non-SPL start-up of U-Boot.

Well, that name specifically states that it's SPL that's running, whereas the exact opposite is true.

Perhaps UBOOT_CHAIN_LOADED_FROM_UBOOT?

Quoted 12 lines

Quoted 3 lines

This approach sounds a little brittle; what happens if r0 just happens to have that value. Won't the code get confused?

Yes, but SPL does not set that value in r0, and we have control over this.

Quoted 3 lines

Why does U-Boot care whether it's been chain-loaded? Shouldn't it always behave identically in all cases, so it's independent of what caused it to run?

Quoted 2 lines

In the case of read-only U-Boot it must find the read-write one to jump to. In the case of read-write U-Boot it must boot a kernel.

Surely that should be taken care of by placing the correct boot scripts into the U-Boot environment, rather than hard-coding specific boot behaviour into the U-Boot binary?

This feature seems really use-case-specific; I wonder if it's useful/generic-enough to upstream even?

Hi Stephen,

Quoted 45 lines · Stephen Warren

Quoted 42 lines · Simon Glass

Hi Stephen,

Quoted 27 lines · Stephen Warren

Quoted 7 lines · Simon Glass

Add an implementation of this function for Tegra.

arch/arm/mach-tegra/board.c enum { · line 41

Quoted 10 lines

arch/arm/mach-tegra/board.c +7 -0

 
 
 
+#ifndef CONFIG_SPL_BUILD
+void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3)
+{
+       from_spl = r0 != SPL_RUNNING_FROM_UBOOT;
+       save_boot_params_ret();
+}
+#endif

(Using terminology from:
https://patchwork.ozlabs.org/patch/467771/
arm: spl: Enable detecting when U-Boot is started from SPL )

Quoted 7 lines

That doesn't look right. Surely (at least on Tegra), if the r/o U-Boot chain-loads to the r/w U-Boot, then the chain-loaded U-Boot has no SPL and
is just the main CPU build of U-Boot.

Hence, "SPL_RUNNING_FROM_UBOOT" seems incorrectly named, since the r/o U-Boot doesn't chain to SPL but rather to U-Boot.

What name do you suggest? I was trying to add a prefix indicating that it relates to non-SPL start-up of U-Boot.

Quoted 4 lines

Well, that name specifically states that it's SPL that's running, whereas the exact opposite is true.

Perhaps UBOOT_CHAIN_LOADED_FROM_UBOOT?

I really want to say that it is not chain-loaded from SPL. Maybe UBOOT_NOT_LOADED_FROM_SPL?

Quoted 15 lines

Quoted 13 lines

Quoted 2 lines

This approach sounds a little brittle; what happens if r0 just happens to have that value. Won't the code get confused?

Yes, but SPL does not set that value in r0, and we have control over this.

Quoted 3 lines

Why does U-Boot care whether it's been chain-loaded? Shouldn't it always behave identically in all cases, so it's independent of what caused it to run?

In the case of read-only U-Boot it must find the read-write one to jump to. In the case of read-write U-Boot it must boot a kernel.

Quoted 3 lines

Surely that should be taken care of by placing the correct boot scripts into the U-Boot environment, rather than hard-coding specific boot behaviour into the U-Boot binary?

Two problems here:

1. The two U-Boot will use the same environment (as they are identical after all)

2. Loading the environment is a security risk (since anyone can change it in Linux, for example) so cannot be loaded.

Quoted 2 lines

This feature seems really use-case-specific; I wonder if it's useful/generic-enough to upstream even?

I am keen to upstream this use case (upgrading U-Boot in a secure way) as I think it has wide application.

Quoted 58 lines · Simon Glass

Hi Stephen,

Quoted 54 lines · Stephen Warren

Quoted 49 lines · Simon Glass

Hi Stephen,

Quoted 32 lines · Stephen Warren

Quoted 10 lines · Simon Glass

Add an implementation of this function for Tegra.

arch/arm/mach-tegra/board.c +0 -0

arch/arm/mach-tegra/board.c enum { · line 41

Quoted 10 lines

arch/arm/mach-tegra/board.c +7 -0

 
 
 
+#ifndef CONFIG_SPL_BUILD
+void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3)
+{
+       from_spl = r0 != SPL_RUNNING_FROM_UBOOT;
+       save_boot_params_ret();
+}
+#endif

(Using terminology from:
https://patchwork.ozlabs.org/patch/467771/
arm: spl: Enable detecting when U-Boot is started from SPL )

Quoted 7 lines

That doesn't look right. Surely (at least on Tegra), if the r/o U-Boot chain-loads to the r/w U-Boot, then the chain-loaded U-Boot has no SPL and
is just the main CPU build of U-Boot.

Hence, "SPL_RUNNING_FROM_UBOOT" seems incorrectly named, since the r/o U-Boot doesn't chain to SPL but rather to U-Boot.

What name do you suggest? I was trying to add a prefix indicating that it relates to non-SPL start-up of U-Boot.

Quoted 8 lines

Quoted 4 lines

Well, that name specifically states that it's SPL that's running, whereas the exact opposite is true.

Perhaps UBOOT_CHAIN_LOADED_FROM_UBOOT?

I really want to say that it is not chain-loaded from SPL. Maybe UBOOT_NOT_LOADED_FROM_SPL?

OK, that highlights that better.

Quoted 29 lines

Quoted 27 lines

Quoted 17 lines

Quoted 3 lines

This approach sounds a little brittle; what happens if r0 just happens to
have that value. Won't the code get confused?

Yes, but SPL does not set that value in r0, and we have control over this.

Quoted 5 lines

Why does U-Boot care whether it's been chain-loaded? Shouldn't it always
behave identically in all cases, so it's independent of what caused it to
run?

Quoted 2 lines

In the case of read-only U-Boot it must find the read-write one to jump to. In the case of read-write U-Boot it must boot a kernel.

Surely that should be taken care of by placing the correct boot scripts into
the U-Boot environment, rather than hard-coding specific boot behaviour into
the U-Boot binary?

Quoted 4 lines

Two problems here:

1. The two U-Boot will use the same environment (as they are identical after all)

That's a design decision. There's absolutely no need for that to be true.

Quoted 3 lines

2. Loading the environment is a security risk (since anyone can change it in Linux, for example) so cannot be loaded.

Well, the environment could be the default/built-in environment and hence validated as part of the validation of the U-Boot binary. Or, even if loaded separately, could also be validated in the same way (but perhaps there's not much point in that, since a fall-back to the built-in environment would be required in case the external environment validation failed).

Quoted 7 lines

Quoted 3 lines

This feature seems really use-case-specific; I wonder if it's useful/generic-enough to upstream even?

I am keen to upstream this use case (upgrading U-Boot in a secure way) as I think it has wide application.

OK. I worry that there are many many possible ways of doing that, and the selection of the best option depends on the system use-cases, security model, and environments. We might not want to lock people into a specific method. So long as the existence of this code doesn't prevent doing things some other way if they need, or upstreaming support for other methods, nor make the code too complex, then it's probably fine.

Enable the I2C3 pins so that the TPM can be used.

If the mmc device is non-removable (as indicated by the device tree), set the flag so that users of the device know.

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

---

If the mmc device is non-removable (as indicated by the device tree), set the flag so that users of the device know.

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

---

drivers/mmc/tegra_mmc.c | 12 ++++++++----

1 file changed, 8 insertions(+), 4 deletions(-)

drivers/mmc/tegra_mmc.c +8 -4

index 2cd8cf1..5c9c54a 100644
@@ -528,7 +528,7 @@ static const struct mmc_ops tegra_mmc_ops = {
 	.getcd		= tegra_mmc_getcd,
 };
 
-static int do_mmc_init(int dev_index)
+static int do_mmc_init(int dev_index, bool removable)
 {
 	struct mmc_host *host;
 	struct mmc *mmc;

@@ -573,6 +573,7 @@ static int do_mmc_init(int dev_index)
 	host->cfg.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
 
 	mmc = mmc_create(&host->cfg, host);
+	mmc->block_dev.removable = removable;
 	if (mmc == NULL)
 		return -1;
 

@@ -586,7 +587,8 @@ static int do_mmc_init(int dev_index)
 * @param node		Device index (0-3)
 * @param host		Structure to fill in (reg, width, mmc_id)
 */
-static int mmc_get_config(const void *blob, int node, struct mmc_host *host)
+static int mmc_get_config(const void *blob, int node, struct mmc_host *host,
+			  bool *removablep)
 {
 	debug("%s: node = %d\n", __func__, node);
 

@@ -619,6 +621,7 @@ static int mmc_get_config(const void *blob, int node, struct mmc_host *host)
 				   GPIOD_IS_IN);
 	gpio_request_by_name_nodev(blob, node, "power-gpios", 0,
 				   &host->pwr_gpio, GPIOD_IS_OUT);
+	*removablep = !fdtdec_get_bool(blob, node, "non-removable");
 
 	debug("%s: found controller at %p, width = %d, periph_id = %d\n",
 		__func__, host->reg, host->width, host->mmc_id);

@@ -636,6 +639,7 @@ static int mmc_get_config(const void *blob, int node, struct mmc_host *host)
 static int process_nodes(const void *blob, int node_list[], int count)
 {
 	struct mmc_host *host;
+	bool removable;
 	int i, node;
 
 	debug("%s: count = %d\n", __func__, count);

@@ -649,11 +653,11 @@ static int process_nodes(const void *blob, int node_list[], int count)
 		host = &mmc_host[i];
 		host->id = i;
 
-		if (mmc_get_config(blob, node, host)) {
+		if (mmc_get_config(blob, node, host, &removable)) {
 			printf("%s: failed to decode dev %d\n",	__func__, i);
 			return -1;
 		}
-		do_mmc_init(i);
+		do_mmc_init(i, removable);
 	}
 	return 0;
 }

Signature · 1 line

2.2.0.rc0.207.ga3a616c

We are getting very close to running out of space in SPL, and with the currently Chrome OS gcc 4.9 we exceed the limit. Add a litle more space.

Quoted 3 lines · Simon Glass

We are getting very close to running out of space in SPL, and with the currently Chrome OS gcc 4.9 we exceed the limit. Add a litle more space.

8K is quite a bump given we only had 24K allocated before. Why is gcc-4.9 less efficient that earlier compilers I wonder? Were we extremely close to the limit ebfore? Still, I guess this is fine.

Did you validate whether tegra-uboot-flasher still works with this change? I think it will since it only cares about the SPL TEXT_BASE and not the main U-Boot TEXT_BASE, but double-checking would be nice.

You forgot to CC Tom Warren on this Tegra patch.

Hi Stephen,

Quoted 10 lines · Stephen Warren

Quoted 3 lines · Simon Glass

We are getting very close to running out of space in SPL, and with the currently Chrome OS gcc 4.9 we exceed the limit. Add a litle more space.

8K is quite a bump given we only had 24K allocated before. Why is gcc-4.9 less efficient that earlier compilers I wonder? Were we extremely close to the limit ebfore? Still, I guess this is fine.

Quoted 3 lines

Did you validate whether tegra-uboot-flasher still works with this change? I think it will since it only cares about the SPL TEXT_BASE and not the main U-Boot TEXT_BASE, but double-checking would be nice.

I tested this with USB A-A (tegrarcm). Is that what you mean?

Quoted 13 lines · Simon Glass

Hi Stephen,

Quoted 9 lines · Stephen Warren

Quoted 4 lines · Simon Glass

We are getting very close to running out of space in SPL, and with the currently Chrome OS gcc 4.9 we exceed the limit. Add a litle more space.

Quoted 4 lines

Quoted 4 lines

8K is quite a bump given we only had 24K allocated before. Why is gcc-4.9 less efficient that earlier compilers I wonder? Were we extremely close to
the limit ebfore? Still, I guess this is fine.

Quoted 8 lines

Quoted 6 lines

Did you validate whether tegra-uboot-flasher still works with this change? I
think it will since it only cares about the SPL TEXT_BASE and not the main
U-Boot TEXT_BASE, but double-checking would be nice.

Quoted 1 line

I tested this with USB A-A (tegrarcm). Is that what you mean?

Did you just use tegrarcm, or tegra-uboot-flasher? The latter is a wrapper on top of tegrarcm, and is a bit more involved. See the various READMEs at
https://github.com/NVIDIA/tegra-uboot-flasher-scripts. This doesn't support any nyan-derived boards yet, but does support Jetson TK1 which I think you have?

Hi Stephen,

Quoted 14 lines · Stephen Warren

Quoted 13 lines · Simon Glass

Hi Stephen,

Quoted 9 lines · Stephen Warren

Quoted 4 lines · Simon Glass

We are getting very close to running out of space in SPL, and with the currently Chrome OS gcc 4.9 we exceed the limit. Add a litle more space.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

8K is quite a bump given we only had 24K allocated before. Why is gcc-4.9 less efficient that earlier compilers I wonder? Were we extremely close to
the limit ebfore? Still, I guess this is fine.

We were about 1KB away, mostly due to the gcc garbage collection bug I think. The cros compiler seems even worse, not sure why.

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

Did you validate whether tegra-uboot-flasher still works with this change? I
think it will since it only cares about the SPL TEXT_BASE and not the main
U-Boot TEXT_BASE, but double-checking would be nice.

Quoted 8 lines

Quoted 1 line

I tested this with USB A-A (tegrarcm). Is that what you mean?

Did you just use tegrarcm, or tegra-uboot-flasher? The latter is a wrapper on top of tegrarcm, and is a bit more involved. See the various READMEs at
https://github.com/NVIDIA/tegra-uboot-flasher-scripts. This doesn't support any nyan-derived boards yet, but does support Jetson TK1 which I think you have?

Ah, OK. The script at:

https://github.com/NVIDIA/tegra-uboot-flasher-scripts/blob/master/tegra-uboot-flasher

looks OK to me. It does not have anything hard-coded that I can see. But I have not used it myself. It's kind-of painful that we have all this out-of-tree Tegra stuff (pin mux also) that might break when we change U-Boot. Is there any way to improve this?

Quoted 60 lines · Simon Glass

Hi Stephen,

Quoted 20 lines · Stephen Warren

Quoted 18 lines · Simon Glass

Hi Stephen,

Quoted 13 lines · Stephen Warren

Quoted 6 lines · Simon Glass

We are getting very close to running out of space in SPL, and with the currently Chrome OS gcc 4.9 we exceed the limit. Add a litle more space.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

8K is quite a bump given we only had 24K allocated before. Why is gcc-4.9
less efficient that earlier compilers I wonder? Were we extremely close to
the limit ebfore? Still, I guess this is fine.

We were about 1KB away, mostly due to the gcc garbage collection bug I think. The cros compiler seems even worse, not sure why.

Quoted 12 lines

Quoted 9 lines

Quoted 5 lines

Did you validate whether tegra-uboot-flasher still works with this change? I
think it will since it only cares about the SPL TEXT_BASE and not the main
U-Boot TEXT_BASE, but double-checking would be nice.

I tested this with USB A-A (tegrarcm). Is that what you mean?

Quoted 9 lines

Did you just use tegrarcm, or tegra-uboot-flasher? The latter is a wrapper
on top of tegrarcm, and is a bit more involved. See the various READMEs at
https://github.com/NVIDIA/tegra-uboot-flasher-scripts. This doesn't support
any nyan-derived boards yet, but does support Jetson TK1 which I think you
have?

Ah, OK. The script at:

Quoted 6 lines

https://github.com/NVIDIA/tegra-uboot-flasher-scripts/blob/master/tegra-uboot-flasher

looks OK to me. It does not have anything hard-coded that I can see. But I have not used it myself. It's kind-of painful that we have all this out-of-tree Tegra stuff (pin mux also) that might break when we change U-Boot. Is there any way to improve this?

I don't think there's anything to improve; nothing is wrong.

If something changes about the way U-Boot works or must be used, then that requires changes elsewhere no matter what. Whether the externals changes are:

* Updates to documentation of how to build/use/... U-Boot.
* Updates to the command-lines/techniques people know and use.
* Updates to code/tools that interface with U-Boot.

... then in all cases, a change to U-Boot requires a change to something else. I would argue that updating an external tool is actually the easiest of the cases, since it's entirely clear what that tool does, what change to make to it, and how. Conversely, updating the command-lines/techniques that people use is much harder to propagate to all people. The update-a-document case may also be about as easy, presuming you know which documents to update (there are probably more docs than tools at least at present).

In the pinmux case, please consider the system-level implications of a change to the pinmux. There is a single correct pinmux configuration for a given HW configuration, and that's driven purely by the HW design[1]. If the pinmux that U-Boot programs is wrong and needs to be changed, the same change should be propagated to all SW to fix the bug everywhere. The existence of the external Tegra pinmux tools makes that easier; update the pinmux tool, regenerate derived files for all SW stacks, and apply the changes everywhere. Sure it's more work to do that than just updating only U-Boot. However, the tool doesn't add that extra work, rather the change to the desired pinmux makes the work. The only alternative (which does indeed reduce overall work) is to update e.g. only U-Boot's pinmux programming, and leave everything else buggy. That's certainly not a desirable situation. I introduced tegra-pinmux-scripts specifically to avoid that kind of situation.

[1] For boards with expansion headers where the end-user/... can use different pins for different things, I consider the HW configuration to include the definition of whatever the user has plugged into the expansion header.

We are getting very close to running out of space in SPL, and with the currently Chrome OS gcc 4.9 we exceed the limit. Add a litle more space.

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

---

 include/configs/tegra124-common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/configs/tegra124-common.h +1 -2

index f2b3774..0347132 100644
@@ -30,7 +30,7 @@
 /*-----------------------------------------------------------------------
  * Physical Memory Map
  */
-#define CONFIG_SYS_TEXT_BASE   0x8010E000
+#define CONFIG_SYS_TEXT_BASE   0x80110000
 
 /*
  * Memory layout for where various images get loaded by boot scripts:
--

2.2.0.rc0.207.ga3a616c

-----Original Message-----
From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
Sent: Tuesday, May 05, 2015 9:04 AM
To: U-Boot Mailing List
Cc: Simon Glass; Heiko Schocher; Tom Rini; Tom Warren
Subject: Re: [PATCH 24/24] tegra124: Expand SPL space by 8KB

+Tom Warren

Quoted 21 lines · Simon Glass

We are getting very close to running out of space in SPL, and with the currently Chrome OS gcc 4.9 we exceed the limit. Add a litle more space.

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

---

 include/configs/tegra124-common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/configs/tegra124-common.h +1 -1

index f2b3774..0347132 100644
@@ -30,7 +30,7 @@
 /*-----------------------------------------------------------------------
  * Physical Memory Map
  */
-#define CONFIG_SYS_TEXT_BASE   0x8010E000
+#define CONFIG_SYS_TEXT_BASE   0x80110000
 

Quoted 5 lines

include/configs/tegra124-common.h +0 -1

 /*
  * Memory layout for where various images get loaded by boot scripts:
--

2.2.0.rc0.207.ga3a616c