[U-Boot] [PATCH 01/24] usb: add device connection/disconnection detection

[U-Boot] [PATCH 01/24] usb: add device connection/disconnection detection

[Simon Glass]

From: Vincent Palatin <vpalatin@chromium.org>

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 <vpalatin@chromium.org>

Reviewed-by: Stefan Reinauer <reinauer@chromium.org>
Tested-by: Stefan Reinauer <reinauer@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

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

diff --git a/common/usb.c b/common/usb.c
index a4820d3..bde8f31 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -148,6 +148,32 @@ int usb_stop(void)
 	return 0;
 }
 
+/******************************************************************************
+ * Detect if a USB device has been plugged or unplugged.
+ */
+int usb_detect_change(void)
+{
+	int i, j;
+	int change = 0;
+
+	for (j = 0; j < USB_MAX_DEVICE; j++) {
+		for (i = 0; i < usb_dev[j].maxchild; i++) {
+			struct usb_port_status status;
+
+			if (usb_get_port_status(&usb_dev[j], i + 1,
+						&status) < 0)
+				/* USB request failed */
+				continue;
+
+			if (le16_to_cpu(status.wPortChange) &
+			    USB_PORT_STAT_C_CONNECTION)
+				change++;
+		}
+	}
+
+	return change;
+}
+
 /*
  * disables the asynch behaviour of the control message. This is used for data
  * transfers that uses the exclusiv access to the control and bulk messages.
diff --git a/common/usb_hub.c b/common/usb_hub.c
index c9be530..bbed1c8 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -79,7 +79,7 @@ static int usb_get_hub_status(struct usb_device *dev, void *data)
 			data, sizeof(struct usb_hub_status), USB_CNTL_TIMEOUT);
 }
 
-static int usb_get_port_status(struct usb_device *dev, int port, void *data)
+int usb_get_port_status(struct usb_device *dev, int port, void *data)
 {
 	return usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
 			USB_REQ_GET_STATUS, USB_DIR_IN | USB_RT_PORT, 0, port,
diff --git a/include/usb.h b/include/usb.h
index 1984e8f..25dddb8 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -265,6 +265,7 @@ int usb_kbd_deregister(int force);
 /* routines */
 int usb_init(void); /* initialize the USB Controller */
 int usb_stop(void); /* stop the USB Controller */
+int usb_detect_change(void); /* detect if a USB device has been (un)plugged */
 
 
 int usb_set_protocol(struct usb_device *dev, int ifnum, int protocol);
@@ -290,6 +291,7 @@ int usb_get_class_descriptor(struct usb_device *dev, int ifnum,
 int usb_clear_halt(struct usb_device *dev, int pipe);
 int usb_string(struct usb_device *dev, int index, char *buf, size_t size);
 int usb_set_interface(struct usb_device *dev, int interface, int alternate);
+int usb_get_port_status(struct usb_device *dev, int port, void *data);
 
 /* big endian -> little endian conversion */
 /* some CPUs are already little endian e.g. the ARM920T */
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 02/24] dm: usb: Implement usb_detect_change() for driver model

[Simon Glass]

Support this function with driver model also (CONFIG_DM_USB).

Signed-off-by: Simon Glass <sjg@chromium.org>
---

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

diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index 714bc0e..8445a42 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -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;
+
+	ret = uclass_get(UCLASS_USB_HUB, &uc);
+	if (ret)
+		return ret;
+
+	uclass_foreach_dev(hub, uc) {
+		struct usb_device *udev;
+		struct udevice *dev;
+
+		if (!device_active(hub))
+			continue;
+		for (device_find_first_child(hub, &dev);
+		     dev;
+		     device_find_next_child(&dev)) {
+			struct usb_port_status status;
+
+			if (!device_active(hub))
+				continue;
+
+			udev = dev_get_parentdata(hub);
+			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++;
+		}
+	}
+
+	return change;
+}
+
 int usb_child_post_bind(struct udevice *dev)
 {
 	struct usb_dev_platdata *plat = dev_get_parent_platdata(dev);
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 02/24] dm: usb: Implement usb_detect_change() for driver model

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:30 PM, Simon Glass <sjg@chromium.org> wrote:
> Support this function with driver model also (CONFIG_DM_USB).
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  drivers/usb/host/usb-uclass.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
> index 714bc0e..8445a42 100644
> --- a/drivers/usb/host/usb-uclass.c
> +++ b/drivers/usb/host/usb-uclass.c
> @@ -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;
> +
> +       ret = uclass_get(UCLASS_USB_HUB, &uc);
> +       if (ret)
> +               return ret;
> +
> +       uclass_foreach_dev(hub, uc) {
> +               struct usb_device *udev;
> +               struct udevice *dev;
> +
> +               if (!device_active(hub))
> +                       continue;

You check this here.

> +               for (device_find_first_child(hub, &dev);
> +                    dev;
> +                    device_find_next_child(&dev)) {
> +                       struct usb_port_status status;
> +
> +                       if (!device_active(hub))
> +                               continue;

How can this be true here? Maybe you meant to check if the "dev" was active?

> +
> +                       udev = dev_get_parentdata(hub);

Why parent of the hub? Looking for an upstream hub?

> +                       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++;
> +               }
> +       }
> +
> +       return change;
> +}
> +
>  int usb_child_post_bind(struct udevice *dev)
>  {
>         struct usb_dev_platdata *plat = dev_get_parent_platdata(dev);
> --

Thanks,
-Joe

[U-Boot] [PATCH 03/24] bootstage: Add IDs for SPI flash reading and decompression

[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 <sjg@chromium.org>
---

 include/bootstage.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/bootstage.h b/include/bootstage.h
index be44014..fe30ab6 100644
--- a/include/bootstage.h
+++ b/include/bootstage.h
@@ -195,6 +195,8 @@ enum bootstage_id {
 
 	BOOTSTAGE_ID_ACCUM_LCD,
 	BOOTSTAGE_ID_ACCUM_SCSI,
+	BOOTSTAGE_ID_ACCUM_SPI,
+	BOOTSTAGE_ID_ACCUM_DECOMP,
 
 	/* a few spare for the user, from here */
 	BOOTSTAGE_ID_USER,
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 03/24] bootstage: Add IDs for SPI flash reading and decompression

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:30 PM, Simon Glass <sjg@chromium.org> wrote:
> 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 <sjg@chromium.org>
> ---
Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 03/24] bootstage: Add IDs for SPI flash reading and decompression

[Jagan Teki]

On 5 May 2015 at 02:27, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Mon, May 4, 2015 at 12:30 PM, Simon Glass <sjg@chromium.org> wrote:
>> 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 <sjg@chromium.org>
>> ---
> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

Reviewed-by: Jagan Teki <jteki@openedev.com>

-- 
Jagan Teki,
Openedev.

[U-Boot] [PATCH 03/24] bootstage: Add IDs for SPI flash reading and decompression

[Simon Glass]

On 7 May 2015 at 23:58, Jagan Teki <jteki@openedev.com> wrote:
> On 5 May 2015 at 02:27, Joe Hershberger <joe.hershberger@gmail.com> wrote:
>> Hi Simon,
>>
>> On Mon, May 4, 2015 at 12:30 PM, Simon Glass <sjg@chromium.org> wrote:
>>> 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 <sjg@chromium.org>
>>> ---
>> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
>
> Reviewed-by: Jagan Teki <jteki@openedev.com>

Applied to u-boot-dm.

[U-Boot] [PATCH 04/24] dm: i2c: Implement 'i2c bus' command for driver model

[Simon Glass]

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

Signed-off-by: Simon Glass <sjg@chromium.org>
---

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

diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
index ad38cbf..1bc0db8 100644
--- a/common/cmd_i2c.c
+++ b/common/cmd_i2c.c
@@ -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;
@@ -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"
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 04/24] dm: i2c: Implement 'i2c bus' command for driver model

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:30 PM, Simon Glass <sjg@chromium.org> wrote:
> This command was missed in the conversion. Add it back for driver model.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---

Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

>
>  common/cmd_i2c.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 55 insertions(+), 8 deletions(-)
>
> diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
> index ad38cbf..1bc0db8 100644
> --- a/common/cmd_i2c.c
> +++ b/common/cmd_i2c.c
> @@ -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;
> @@ -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)

It's too bad we are already on a 3rd model for I2C and the 1st is
still kicking around.  We have the same problem on the Ethernet phy
drivers as soon as I add DM support. I intend to address that one at
some point before too long... maybe even before I add DM support.
Maybe we need a deadline for this in I2C like we had for common board?

>         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

Cheers,
-Joe

[U-Boot] [PATCH 04/24] dm: i2c: Implement 'i2c bus' command for driver model

[Heiko Schocher]

Hello Simon,

Am 04.05.2015 19:30, schrieb Simon Glass:
> This command was missed in the conversion. Add it back for driver model.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   common/cmd_i2c.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 55 insertions(+), 8 deletions(-)
>
> diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
> index ad38cbf..1bc0db8 100644
> --- a/common/cmd_i2c.c
> +++ b/common/cmd_i2c.c
> @@ -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;
> +

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 <hs@denx.de>

Thanks!

bye,
Heiko
>   			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"
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH 04/24] dm: i2c: Implement 'i2c bus' command for driver model

[Simon Glass]

Hi Heiko,

On 5 May 2015 at 23:44, Heiko Schocher <hs@denx.de> wrote:
> Hello Simon,
>
>
> Am 04.05.2015 19:30, schrieb Simon Glass:
>>
>> This command was missed in the conversion. Add it back for driver model.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>   common/cmd_i2c.c | 63
>> +++++++++++++++++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 55 insertions(+), 8 deletions(-)
>>
>> diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
>> index ad38cbf..1bc0db8 100644
>> --- a/common/cmd_i2c.c
>> +++ b/common/cmd_i2c.c
>> @@ -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;
>> +
>
>
> 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 <hs@denx.de>

Yes, I probably shouldn't do that.

[snip]

Regards,
Simon

[U-Boot] [PATCH 04/24] dm: i2c: Implement 'i2c bus' command for driver model

[Simon Glass]

On 6 May 2015 at 08:34, Simon Glass <sjg@chromium.org> wrote:
> Hi Heiko,
>
> On 5 May 2015 at 23:44, Heiko Schocher <hs@denx.de> wrote:
>> Hello Simon,
>>
>>
>> Am 04.05.2015 19:30, schrieb Simon Glass:
>>>
>>> This command was missed in the conversion. Add it back for driver model.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>>   common/cmd_i2c.c | 63
>>> +++++++++++++++++++++++++++++++++++++++++++++++++-------
>>>   1 file changed, 55 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
>>> index ad38cbf..1bc0db8 100644
>>> --- a/common/cmd_i2c.c
>>> +++ b/common/cmd_i2c.c
>>> @@ -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;
>>> +
>>
>>
>> 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 <hs@denx.de>
>
> Yes, I probably shouldn't do that.
>
> [snip]
>
> Regards,
> Simon

Actually this is OK because it is not really in the middle of the
code, due to the #ifdef.

Applied to u-boot-dm.

[U-Boot] [PATCH 05/24] dm: i2c: Add a function to find out the chip offset length

[Simon Glass]

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

Signed-off-by: Simon Glass <sjg@chromium.org>
---

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

diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index f2e95c0..2dfb435 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -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);
diff --git a/include/i2c.h b/include/i2c.h
index 6fd73fa..9a9e4f7 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -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
  *
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 05/24] dm: i2c: Add a function to find out the chip offset length

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:30 PM, Simon Glass <sjg@chromium.org> wrote:
> We can currently set this but there is no API function to get it. Add one.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---

Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 05/24] dm: i2c: Add a function to find out the chip offset length

[Heiko Schocher]

Hello Simon,

Am 04.05.2015 19:30, schrieb Simon Glass:
> We can currently set this but there is no API function to get it. Add one.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   drivers/i2c/i2c-uclass.c | 7 +++++++
>   include/i2c.h            | 9 ++++++++-
>   2 files changed, 15 insertions(+), 1 deletion(-)

Thanks!

Acked-by: Heiko Schocher<hs@denx.de>

bye,
Heiko
>
> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
> index f2e95c0..2dfb435 100644
> --- a/drivers/i2c/i2c-uclass.c
> +++ b/drivers/i2c/i2c-uclass.c
> @@ -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);
> diff --git a/include/i2c.h b/include/i2c.h
> index 6fd73fa..9a9e4f7 100644
> --- a/include/i2c.h
> +++ b/include/i2c.h
> @@ -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
>    *
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH 05/24] dm: i2c: Add a function to find out the chip offset length

[Simon Glass]

On 5 May 2015 at 23:37, Heiko Schocher <hs@denx.de> wrote:
> Hello Simon,
>
> Am 04.05.2015 19:30, schrieb Simon Glass:
>>
>> We can currently set this but there is no API function to get it. Add one.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>   drivers/i2c/i2c-uclass.c | 7 +++++++
>>   include/i2c.h            | 9 ++++++++-
>>   2 files changed, 15 insertions(+), 1 deletion(-)
>
>
> Thanks!
>
> Acked-by: Heiko Schocher<hs@denx.de>

Applied to u-boot-dm.

[U-Boot] [PATCH 06/24] tpm: Support using driver model with I2C

[Simon Glass]

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 <sjg@chromium.org>
---

 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(-)

diff --git a/drivers/tpm/tpm.c b/drivers/tpm/tpm.c
index 31761ec..a650892 100644
--- a/drivers/tpm/tpm.c
+++ b/drivers/tpm/tpm.c
@@ -34,6 +34,7 @@
 
 #include <config.h>
 #include <common.h>
+#include <dm.h>
 #include <linux/compiler.h>
 #include <fdtdec.h>
 #include <i2c.h>
@@ -48,10 +49,14 @@ DECLARE_GLOBAL_DATA_PTR;
 
 /* TPM configuration */
 struct tpm {
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+#else
 	int i2c_bus;
 	int slave_addr;
-	char inited;
 	int old_bus;
+#endif
+	char inited;
 } tpm;
 
 /* Global structure for tpm chip data */
@@ -372,7 +377,7 @@ static unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
 
 static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz)
 {
-	ssize_t rc;
+	int rc;
 	u32 count, ordinal;
 	unsigned long start, stop;
 
@@ -391,9 +396,11 @@ static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz)
 		return -E2BIG;
 	}
 
+	debug("Calling send\n");
 	rc = chip->vendor.send(chip, (u8 *)buf, count);
+	debug("   ... done calling send\n");
 	if (rc < 0) {
-		error("tpm_transmit: tpm_send: error %zd\n", rc);
+		error("tpm_transmit: tpm_send: error %d\n", rc);
 		goto out;
 	}
 
@@ -403,7 +410,7 @@ static ssize_t tpm_transmit(const unsigned char *buf, size_t bufsiz)
 	start = get_timer(0);
 	stop = tpm_calc_ordinal_duration(chip, ordinal);
 	do {
-		debug("waiting for status...\n");
+		debug("waiting for status... %ld %ld\n", start, stop);
 		u8 status = chip->vendor.status(chip);
 		if ((status & chip->vendor.req_complete_mask) ==
 		    chip->vendor.req_complete_val) {
@@ -428,15 +435,30 @@ out_recv:
 	debug("out_recv: reading response...\n");
 	rc = chip->vendor.recv(chip, (u8 *)buf, TPM_BUFSIZE);
 	if (rc < 0)
-		error("tpm_transmit: tpm_recv: error %zd\n", rc);
+		error("tpm_transmit: tpm_recv: error %d\n", rc);
 
 out:
 	return rc;
 }
 
+#ifdef CONFIG_DM_I2C
+static int tpm_open_dev(struct udevice *dev)
+{
+	int rc;
+
+	debug("%s: start\n", __func__);
+	if (g_chip.is_open)
+		return -EBUSY;
+	rc = tpm_vendor_init_dev(dev);
+	if (rc < 0)
+		g_chip.is_open = 0;
+	return rc;
+}
+#else
 static int tpm_open(uint32_t dev_addr)
 {
 	int rc;
+
 	if (g_chip.is_open)
 		return -EBUSY;
 	rc = tpm_vendor_init(dev_addr);
@@ -444,7 +466,7 @@ static int tpm_open(uint32_t dev_addr)
 		g_chip.is_open = 0;
 	return rc;
 }
-
+#endif
 static void tpm_close(void)
 {
 	if (g_chip.is_open) {
@@ -455,6 +477,7 @@ static void tpm_close(void)
 
 static int tpm_select(void)
 {
+#ifndef CONFIG_DM_I2C
 	int ret;
 
 	tpm.old_bus = i2c_get_bus_num();
@@ -466,11 +489,13 @@ static int tpm_select(void)
 			return -1;
 		}
 	}
+#endif
 	return 0;
 }
 
 static int tpm_deselect(void)
 {
+#ifndef CONFIG_DM_I2C
 	int ret;
 
 	if (tpm.old_bus != i2c_get_bus_num()) {
@@ -482,6 +507,7 @@ static int tpm_deselect(void)
 		}
 	}
 	tpm.old_bus = -1;
+#endif
 	return 0;
 }
 
@@ -493,10 +519,9 @@ static int tpm_deselect(void)
  */
 static int tpm_decode_config(struct tpm *dev)
 {
-#ifdef CONFIG_OF_CONTROL
 	const void *blob = gd->fdt_blob;
-	int node, parent;
-	int i2c_bus;
+	int parent;
+	int node;
 
 	node = fdtdec_next_compatible(blob, 0, COMPAT_INFINEON_SLB9635_TPM);
 	if (node < 0) {
@@ -512,15 +537,48 @@ static int tpm_decode_config(struct tpm *dev)
 		debug("%s: Cannot find node parent\n", __func__);
 		return -1;
 	}
+#ifdef CONFIG_DM_I2C
+	struct udevice *bus;
+	int chip_addr;
+	int ret;
+
+	/*
+	 * TODO(sjg at chromium.org): Remove this when driver model supports
+	 * TPMs
+	 */
+	ret = uclass_get_device_by_of_offset(UCLASS_I2C, parent, &bus);
+	if (ret) {
+		debug("Cannot find bus for node '%s: ret=%d'\n",
+		      fdt_get_name(blob, parent, NULL), ret);
+		return ret;
+	}
+
+	chip_addr = fdtdec_get_int(blob, node, "reg", -1);
+	if (chip_addr == -1) {
+		debug("Cannot find reg property for node '%s: ret=%d'\n",
+		      fdt_get_name(blob, node, NULL), ret);
+		return ret;
+	}
+	/*
+	 * TODO(sjg@chromium.org): Older TPMs will need to use the older method
+	 * in iic_tpm_read() so the offset length needs to be 0 here.
+	 */
+	ret = i2c_get_chip(bus, chip_addr, 1, &dev->dev);
+	if (ret) {
+		debug("Cannot find device for node '%s: ret=%d'\n",
+		      fdt_get_name(blob, node, NULL), ret);
+		return ret;
+	}
+#else
+	int i2c_bus;
+
 	i2c_bus = i2c_get_bus_num_fdt(parent);
 	if (i2c_bus < 0)
 		return -1;
 	dev->i2c_bus = i2c_bus;
 	dev->slave_addr = fdtdec_get_addr(blob, node, "reg");
-#else
-	dev->i2c_bus = CONFIG_TPM_TIS_I2C_BUS_NUMBER;
-	dev->slave_addr = CONFIG_TPM_TIS_I2C_SLAVE_ADDRESS;
 #endif
+
 	return 0;
 }
 
@@ -547,6 +605,7 @@ int tis_init(void)
 	if (tpm_select())
 		return -1;
 
+#ifndef CONFIG_DM_I2C
 	/*
 	 * Probe TPM twice; the first probing might fail because TPM is asleep,
 	 * and the probing can wake up TPM.
@@ -556,8 +615,10 @@ int tis_init(void)
 		      tpm.slave_addr);
 		return -1;
 	}
+#endif
 
 	tpm_deselect();
+	debug("%s: done\n", __func__);
 
 	tpm.inited = 1;
 
@@ -574,7 +635,11 @@ int tis_open(void)
 	if (tpm_select())
 		return -1;
 
+#ifdef CONFIG_DM_I2C
+	rc = tpm_open_dev(tpm.dev);
+#else
 	rc = tpm_open(tpm.slave_addr);
+#endif
 
 	tpm_deselect();
 
diff --git a/drivers/tpm/tpm_private.h b/drivers/tpm/tpm_private.h
index 888a074..8894c98 100644
--- a/drivers/tpm/tpm_private.h
+++ b/drivers/tpm/tpm_private.h
@@ -131,6 +131,9 @@ struct tpm_chip *tpm_register_hardware(const struct tpm_vendor_specific *);
 
 int tpm_vendor_init(uint32_t dev_addr);
 
+struct udevice;
+int tpm_vendor_init_dev(struct udevice *dev);
+
 void tpm_vendor_cleanup(struct tpm_chip *chip);
 
 
diff --git a/drivers/tpm/tpm_tis_i2c.c b/drivers/tpm/tpm_tis_i2c.c
index c1bbed4..ee4dfea 100644
--- a/drivers/tpm/tpm_tis_i2c.c
+++ b/drivers/tpm/tpm_tis_i2c.c
@@ -37,6 +37,7 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <fdtdec.h>
 #include <linux/compiler.h>
 #include <i2c.h>
@@ -122,13 +123,19 @@ static const char * const chip_name[] = {
 
 /* Structure to store I2C TPM specific stuff */
 struct tpm_dev {
+#ifdef CONFIG_DM_I2C
+	struct udevice *dev;
+#else
 	uint addr;
+#endif
 	u8 buf[TPM_DEV_BUFSIZE + sizeof(u8)];  /* Max buffer size + addr */
 	enum i2c_chip_type chip_type;
 };
 
 static struct tpm_dev tpm_dev = {
+#ifndef CONFIG_DM_I2C
 	.addr = TPM_I2C_ADDR
+#endif
 };
 
 static struct tpm_dev tpm_dev;
@@ -156,8 +163,12 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
 	if ((tpm_dev.chip_type == SLB9635) || (tpm_dev.chip_type == UNKNOWN)) {
 		/* slb9635 protocol should work in both cases */
 		for (count = 0; count < MAX_COUNT; count++) {
+#ifdef CONFIG_DM_I2C
+			rc = dm_i2c_write(tpm_dev.dev, 0, (uchar *)&addrbuf, 1);
+#else
 			rc = i2c_write(tpm_dev.addr, 0, 0,
 				       (uchar *)&addrbuf, 1);
+#endif
 			if (rc == 0)
 				break;  /* Success, break to skip sleep */
 			udelay(SLEEP_DURATION);
@@ -171,7 +182,11 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
 		 */
 		for (count = 0; count < MAX_COUNT; count++) {
 			udelay(SLEEP_DURATION);
+#ifdef CONFIG_DM_I2C
+			rc = dm_i2c_read(tpm_dev.dev, 0, buffer, len);
+#else
 			rc = i2c_read(tpm_dev.addr, 0, 0, buffer, len);
+#endif
 			if (rc == 0)
 				break;  /* success, break to skip sleep */
 		}
@@ -184,7 +199,11 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
 		 * be safe on the safe side.
 		 */
 		for (count = 0; count < MAX_COUNT; count++) {
+#ifdef CONFIG_DM_I2C
+			rc = dm_i2c_read(tpm_dev.dev, addr, buffer, len);
+#else
 			rc = i2c_read(tpm_dev.addr, addr, 1, buffer, len);
+#endif
 			if (rc == 0)
 				break;  /* break here to skip sleep */
 			udelay(SLEEP_DURATION);
@@ -206,18 +225,26 @@ static int iic_tpm_write_generic(u8 addr, u8 *buffer, size_t len,
 	int count;
 
 	/* Prepare send buffer */
+#ifndef CONFIG_DM_I2C
 	tpm_dev.buf[0] = addr;
 	memcpy(&(tpm_dev.buf[1]), buffer, len);
+	buffer = tpm_dev.buf;
+	len++;
+#endif
 
 	for (count = 0; count < max_count; count++) {
-		rc = i2c_write(tpm_dev.addr, 0, 0, tpm_dev.buf, len + 1);
+#ifdef CONFIG_DM_I2C
+		rc = dm_i2c_write(tpm_dev.dev, addr, buffer, len);
+#else
+		rc = i2c_write(tpm_dev.addr, 0, 0, buffer, len);
+#endif
 		if (rc == 0)
 			break;  /* Success, break to skip sleep */
 		udelay(sleep_time);
 	}
 
 	/* take care of 'guard time' */
-	udelay(SLEEP_DURATION);
+	udelay(sleep_time);
 	if (rc)
 		return -rc;
 
@@ -292,11 +319,14 @@ static int request_locality(struct tpm_chip *chip, int loc)
 {
 	unsigned long start, stop;
 	u8 buf = TPM_ACCESS_REQUEST_USE;
+	int rc;
 
 	if (check_locality(chip, loc) >= 0)
 		return loc;  /* We already have the locality */
 
-	iic_tpm_write(TPM_ACCESS(loc), &buf, 1);
+	rc = iic_tpm_write(TPM_ACCESS(loc), &buf, 1);
+	if (rc)
+		return rc;
 
 	/* Wait for burstcount */
 	start = get_timer(0);
@@ -323,10 +353,15 @@ static u8 tpm_tis_i2c_status(struct tpm_chip *chip)
 
 static void tpm_tis_i2c_ready(struct tpm_chip *chip)
 {
+	int rc;
+
 	/* This causes the current command to be aborted */
 	u8 buf = TPM_STS_COMMAND_READY;
 
-	iic_tpm_write_long(TPM_STS(chip->vendor.locality), &buf, 1);
+	debug("%s\n", __func__);
+	rc = iic_tpm_write_long(TPM_STS(chip->vendor.locality), &buf, 1);
+	if (rc)
+		debug("%s: rc=%d\n", __func__, rc);
 }
 
 static ssize_t get_burstcount(struct tpm_chip *chip)
@@ -422,6 +457,8 @@ static int tpm_tis_i2c_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 
 	expected = get_unaligned_be32(buf + TPM_RSP_SIZE_BYTE);
 	if ((size_t)expected > count) {
+		error("Error size=%x, expected=%x, count=%x\n", size, expected,
+		      count);
 		size = -EIO;
 		goto out;
 	}
@@ -456,11 +493,12 @@ out:
 static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
 {
 	int rc, status;
-	ssize_t burstcnt;
+	size_t burstcnt;
 	size_t count = 0;
 	int retry = 0;
 	u8 sts = TPM_STS_GO;
 
+	debug("%s: len=%d\n", __func__, len);
 	if (len > TPM_DEV_BUFSIZE)
 		return -E2BIG;  /* Command is too long for our tpm, sorry */
 
@@ -483,9 +521,10 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	if (burstcnt < 0)
 		return burstcnt;
 
-	while (count < len - 1) {
-		if (burstcnt > len - 1 - count)
-			burstcnt = len - 1 - count;
+	while (count < len) {
+		udelay(300);
+		if (burstcnt > len - count)
+			burstcnt = len - count;
 
 #ifdef CONFIG_TPM_TIS_I2C_BURST_LIMITATION
 		if (retry && burstcnt > CONFIG_TPM_TIS_I2C_BURST_LIMITATION)
@@ -497,9 +536,15 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
 		if (rc == 0)
 			count += burstcnt;
 		else {
-			retry++;
-			wait_for_stat(chip, TPM_STS_VALID,
-				      chip->vendor.timeout_c, &status);
+			debug("%s: error\n", __func__);
+			if (retry++ > 10) {
+				rc = -EIO;
+				goto out_err;
+			}
+			rc = wait_for_stat(chip, TPM_STS_VALID,
+					   chip->vendor.timeout_c, &status);
+			if (rc)
+				goto out_err;
 
 			if ((status & TPM_STS_DATA_EXPECT) == 0) {
 				rc = -EIO;
@@ -508,20 +553,14 @@ static int tpm_tis_i2c_send(struct tpm_chip *chip, u8 *buf, size_t len)
 		}
 	}
 
-	/* Write last byte */
-	iic_tpm_write(TPM_DATA_FIFO(chip->vendor.locality), &(buf[count]), 1);
-	wait_for_stat(chip, TPM_STS_VALID, chip->vendor.timeout_c, &status);
-	if ((status & TPM_STS_DATA_EXPECT) != 0) {
-		rc = -EIO;
-		goto out_err;
-	}
-
 	/* Go and do it */
 	iic_tpm_write(TPM_STS(chip->vendor.locality), &sts, 1);
+	debug("done\n");
 
 	return len;
 
 out_err:
+	debug("%s: out_err\n", __func__);
 	tpm_tis_i2c_ready(chip);
 	/*
 	 * The TPM needs some time to clean up here,
@@ -558,26 +597,17 @@ static enum i2c_chip_type tpm_vendor_chip_type(void)
 	return UNKNOWN;
 }
 
-/* Initialisation of i2c tpm */
-int tpm_vendor_init(uint32_t dev_addr)
+static int tpm_vendor_init_common(void)
 {
+	struct tpm_chip *chip;
 	u32 vendor;
 	u32 expected_did_vid;
-	uint old_addr;
-	int rc = 0;
-	struct tpm_chip *chip;
-
-	old_addr = tpm_dev.addr;
-	if (dev_addr != 0)
-		tpm_dev.addr = dev_addr;
 
 	tpm_dev.chip_type = tpm_vendor_chip_type();
 
 	chip = tpm_register_hardware(&tpm_tis_i2c);
-	if (chip < 0) {
-		rc = -ENODEV;
-		goto out_err;
-	}
+	if (chip < 0)
+		return -ENODEV;
 
 	/* Disable interrupts (not supported) */
 	chip->vendor.irq = 0;
@@ -588,15 +618,13 @@ int tpm_vendor_init(uint32_t dev_addr)
 	chip->vendor.timeout_c = TIS_SHORT_TIMEOUT;
 	chip->vendor.timeout_d = TIS_SHORT_TIMEOUT;
 
-	if (request_locality(chip, 0) < 0) {
-		rc = -ENODEV;
-		goto out_err;
-	}
+	if (request_locality(chip, 0) < 0)
+		return  -ENODEV;
 
 	/* Read four bytes from DID_VID register */
 	if (iic_tpm_read(TPM_DID_VID(0), (uchar *)&vendor, 4) < 0) {
-		rc = -EIO;
-		goto out_release;
+		release_locality(chip, 0, 1);
+		return -EIO;
 	}
 
 	if (tpm_dev.chip_type == SLB9635) {
@@ -609,8 +637,7 @@ int tpm_vendor_init(uint32_t dev_addr)
 
 	if (tpm_dev.chip_type != UNKNOWN && vendor != expected_did_vid) {
 		error("Vendor id did not match! ID was %08x\n", vendor);
-		rc = -ENODEV;
-		goto out_release;
+		return -ENODEV;
 	}
 
 	debug("1.2 TPM (chip type %s device-id 0x%X)\n",
@@ -622,14 +649,33 @@ int tpm_vendor_init(uint32_t dev_addr)
 	 */
 
 	return 0;
+}
+
+#ifdef CONFIG_DM_I2C
+/* Initialisation of i2c tpm */
+int tpm_vendor_init_dev(struct udevice *dev)
+{
+	tpm_dev.dev = dev;
+	return tpm_vendor_init_common();
+}
+#else
+/* Initialisation of i2c tpm */
+int tpm_vendor_init(uint32_t dev_addr)
+{
+	uint old_addr;
+	int rc = 0;
 
-out_release:
-	release_locality(chip, 0, 1);
+	old_addr = tpm_dev.addr;
+	if (dev_addr != 0)
+		tpm_dev.addr = dev_addr;
+
+	rc = tpm_vendor_init_common();
+	if (rc)
+		tpm_dev.addr = old_addr;
 
-out_err:
-	tpm_dev.addr = old_addr;
 	return rc;
 }
+#endif
 
 void tpm_vendor_cleanup(struct tpm_chip *chip)
 {
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 06/24] tpm: Support using driver model with I2C

[Simon Glass]

On 4 May 2015 at 11:30, Simon Glass <sjg@chromium.org> wrote:
> 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 <sjg@chromium.org>
> ---
>
>  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(-)

Applied to u-boot-dm.

[U-Boot] [PATCH 07/24] tpm: Rename Infineon TPM to slb9645tt

[Simon Glass]

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

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 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(-)

diff --git a/arch/arm/dts/exynos5420-peach-pit.dts b/arch/arm/dts/exynos5420-peach-pit.dts
index 7d8fa28..6fe762d 100644
--- a/arch/arm/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/dts/exynos5420-peach-pit.dts
@@ -79,7 +79,7 @@
 	i2c at 12e10000 { /* i2c9 */
 		clock-frequency = <400000>;
                 tpm at 20 {
-                        compatible = "infineon,slb9645-tpm";
+                        compatible = "infineon,slb9645tt";
                         reg = <0x20>;
 		};
 	};
diff --git a/arch/arm/dts/exynos5800-peach-pi.dts b/arch/arm/dts/exynos5800-peach-pi.dts
index 8c1f616..176ce55 100644
--- a/arch/arm/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/dts/exynos5800-peach-pi.dts
@@ -72,7 +72,7 @@
 	i2c at 12e10000 { /* i2c9 */
 		clock-frequency = <400000>;
                 tpm at 20 {
-                        compatible = "infineon,slb9645-tpm";
+                        compatible = "infineon,slb9645tt";
                         reg = <0x20>;
 		};
 	};
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 14984b7..30573c2 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -65,7 +65,7 @@ static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(GENERIC_SPI_FLASH, "spi-flash"),
 	COMPAT(MAXIM_98095_CODEC, "maxim,max98095-codec"),
 	COMPAT(INFINEON_SLB9635_TPM, "infineon,slb9635-tpm"),
-	COMPAT(INFINEON_SLB9645_TPM, "infineon,slb9645-tpm"),
+	COMPAT(INFINEON_SLB9645_TPM, "infineon,slb9645tt"),
 	COMPAT(SAMSUNG_EXYNOS5_I2C, "samsung,exynos5-hsi2c"),
 	COMPAT(SANDBOX_LCD_SDL, "sandbox,lcd-sdl"),
 	COMPAT(TI_TPS65090, "ti,tps65090"),
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 07/24] tpm: Rename Infineon TPM to slb9645tt

[Simon Glass]

On 4 May 2015 at 11:31, Simon Glass <sjg@chromium.org> wrote:
> This name is used in Linux, so use it in U-Boot.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  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(-)

Applied to u-boot-dm.

[U-Boot] [PATCH 08/24] dm: gpio: Fix comment typo in GPIOD_IS_IN

[Simon Glass]

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

Signed-off-by: Simon Glass <sjg@chromium.org>
---

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

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 4752ea4..519bb0b 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -115,7 +115,7 @@ struct gpio_desc {
 	unsigned long flags;
 #define GPIOD_REQUESTED		(1 << 0)	/* Requested/claimed */
 #define GPIOD_IS_OUT		(1 << 1)	/* GPIO is an output */
-#define GPIOD_IS_IN		(1 << 2)	/* GPIO is an output */
+#define GPIOD_IS_IN		(1 << 2)	/* GPIO is an input */
 #define GPIOD_ACTIVE_LOW	(1 << 3)	/* value has active low */
 #define GPIOD_IS_OUT_ACTIVE	(1 << 4)	/* set output active */
 
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 08/24] dm: gpio: Fix comment typo in GPIOD_IS_IN

[Simon Glass]

On 4 May 2015 at 11:31, Simon Glass <sjg@chromium.org> wrote:
> This should say 'in', not 'out'.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  include/asm-generic/gpio.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to u-boot-dm.

[U-Boot] [PATCH 09/24] arm: Include the .got section in the binary

[Simon Glass]

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 <sjg@chromium.org>
---

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

diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index c005ce4..0550225 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -112,7 +112,8 @@ endif
 ifdef CONFIG_ARM64
 OBJCOPYFLAGS += -j .text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
 else
-OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .hash -j .data -j .got.plt -j .u_boot_list -j .rel.dyn
+OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .hash -j .data -j \
+	.got -j .got.plt -j .u_boot_list -j .rel.dyn
 endif
 
 ifdef CONFIG_OF_EMBED
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 09/24] arm: Include the .got section in the binary

[Simon Glass]

On 4 May 2015 at 11:31, Simon Glass <sjg@chromium.org> wrote:
> 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 <sjg@chromium.org>
> ---
>
>  arch/arm/config.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Applied to u-boot-dm.

[U-Boot] [PATCH 10/24] arm: Add a prototype for save_boot_params_ret()

[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 <sjg@chromium.org>
---

 arch/arm/include/asm/system.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 2a5bed2..e584c3b 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -158,6 +158,22 @@ void flush_l3_cache(void);
  * void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3);
  */
 
+/**
+ * save_boot_params_ret() - Return from save_boot_params()
+ *
+ * If you provide save_boot_params(), then you should jump back to this
+ * function when done. Try to preserve all registers.
+ *
+ * If your implementation of save_boot_params() is in C then it is acceptable
+ * to simply call save_boot_params_ret() at the end of your function. Since
+ * there is no link register set up, you cannot just exit the function. U-Boot
+ * will return to the (initialised) value of lr, and likely crash/hang.
+ *
+ * If your implementation of save_boot_params() is in assembler then you
+ * should use 'b' or 'bx' to return to save_boot_params_ret.
+ */
+void save_boot_params_ret(void);
+
 #define isb() __asm__ __volatile__ ("" : : : "memory")
 
 #define nop() __asm__ __volatile__("mov\tr0,r0\t@ nop\n\t");
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 10/24] arm: Add a prototype for save_boot_params_ret()

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
> 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 <sjg@chromium.org>
> ---
Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 10/24] arm: Add a prototype for save_boot_params_ret()

[Simon Glass]

On 4 May 2015 at 15:09, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
>> 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 <sjg@chromium.org>
>> ---
> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot-dm.

[U-Boot] [PATCH 11/24] arm: spl: Enable detecting when U-Boot is started from SPL

[Simon Glass]

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 <sjg@chromium.org>
---

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

diff --git a/include/spl.h b/include/spl.h
index b2e5bf7..cdd63a7 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -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 will normally be true, but if U-Boot jumps to second U-Boot, it will
+ * be false. This should be implemented by board-specific code.
+ *
+ * @return true if U-Boot booted from SPL, else false
+ */
+bool spl_was_boot_source(void);
+
 #endif
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 11/24] arm: spl: Enable detecting when U-Boot is started from SPL

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
> 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 <sjg@chromium.org>
> ---
>
>  include/spl.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Part of this patch seems to be missing. I don't see how these changes
can accomplish what is described in the commit log.

> diff --git a/include/spl.h b/include/spl.h
> index b2e5bf7..cdd63a7 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -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 will normally be true, but if U-Boot jumps to second U-Boot, it will
> + * be false. This should be implemented by board-specific code.
> + *
> + * @return true if U-Boot booted from SPL, else false
> + */
> +bool spl_was_boot_source(void);
> +
>  #endif
> --

Thanks,
-Joe

[U-Boot] [PATCH 12/24] arm: Allow cleanup_before_linux() without disabling caches

[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 <sjg@chromium.org>
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
---

 arch/arm/cpu/armv7/cpu.c | 47 +++++++++++++++++++++++++++++------------------
 include/common.h         | 15 +++++++++++++++
 2 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c
index c56417d..0b0e500 100644
--- a/arch/arm/cpu/armv7/cpu.c
+++ b/arch/arm/cpu/armv7/cpu.c
@@ -24,7 +24,7 @@
 
 void __weak cpu_cache_initialization(void){}
 
-int cleanup_before_linux(void)
+int cleanup_before_linux_select(int flags)
 {
 	/*
 	 * this function is called just before we call linux
@@ -42,24 +42,30 @@ int cleanup_before_linux(void)
 	icache_disable();
 	invalidate_icache_all();
 
-	/*
-	 * turn off D-cache
-	 * dcache_disable() in turn flushes the d-cache and disables MMU
-	 */
-	dcache_disable();
-	v7_outer_cache_disable();
+	if (flags & CBL_DISABLE_CACHES) {
+		/*
+		* turn off D-cache
+		* dcache_disable() in turn flushes the d-cache and disables MMU
+		*/
+		dcache_disable();
+		v7_outer_cache_disable();
 
-	/*
-	 * After D-cache is flushed and before it is disabled there may
-	 * be some new valid entries brought into the cache. We are sure
-	 * that these lines are not dirty and will not affect our execution.
-	 * (because unwinding the call-stack and setting a bit in CP15 SCTLR
-	 * is all we did during this. We have not pushed anything on to the
-	 * stack. Neither have we affected any static data)
-	 * So just invalidate the entire d-cache again to avoid coherency
-	 * problems for kernel
-	 */
-	invalidate_dcache_all();
+		/*
+		* After D-cache is flushed and before it is disabled there may
+		* be some new valid entries brought into the cache. We are
+		* sure that these lines are not dirty and will not affect our
+		* execution. (because unwinding the call-stack and setting a
+		* bit in CP15 SCTRL is all we did during this. We have not
+		* pushed anything on to the stack. Neither have we affected
+		* any static data) So just invalidate the entire d-cache again
+		* to avoid coherency problems for kernel
+		*/
+		invalidate_dcache_all();
+	} else {
+		flush_dcache_all();
+		invalidate_icache_all();
+		icache_enable();
+	}
 
 	/*
 	 * Some CPU need more cache attention before starting the kernel.
@@ -68,3 +74,8 @@ int cleanup_before_linux(void)
 
 	return 0;
 }
+
+int cleanup_before_linux(void)
+{
+	return cleanup_before_linux_select(CBL_ALL);
+}
diff --git a/include/common.h b/include/common.h
index cde3474..0b21599 100644
--- a/include/common.h
+++ b/include/common.h
@@ -728,6 +728,21 @@ void	invalidate_dcache_range(unsigned long start, unsigned long stop);
 void	invalidate_dcache_all(void);
 void	invalidate_icache_all(void);
 
+enum {
+	/* Disable caches (else flush caches but leave them active) */
+	CBL_DISABLE_CACHES		= 1 << 0,
+	CBL_SHOW_BOOTSTAGE_REPORT	= 1 << 1,
+
+	CBL_ALL				= 3,
+};
+
+/**
+ * Clean up ready for linux
+ *
+ * @param flags		Flags to control what is done
+ */
+int cleanup_before_linux_select(int flags);
+
 /* arch/$(ARCH)/lib/ticks.S */
 uint64_t get_ticks(void);
 void	wait_ticks    (unsigned long);
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 12/24] arm: Allow cleanup_before_linux() without disabling caches

[Marek Vasut]

On Monday, May 04, 2015 at 07:31:05 PM, Simon Glass wrote:
> 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 <sjg@chromium.org>
> Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>

In which usecase exactly is this applicable please ? :)

Best regards,
Marek Vasut

[U-Boot] [PATCH 12/24] arm: Allow cleanup_before_linux() without disabling caches

[Simon Glass]

Hi Marek,

On 4 May 2015 at 11:36, Marek Vasut <marex@denx.de> wrote:
> On Monday, May 04, 2015 at 07:31:05 PM, Simon Glass wrote:
>> 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 <sjg@chromium.org>
>> Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
>
> 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.

Regards,
Simon

[U-Boot] [PATCH 12/24] arm: Allow cleanup_before_linux() without disabling caches

[Marek Vasut]

On Monday, May 04, 2015 at 07:49:16 PM, Simon Glass wrote:
> Hi Marek,

Hi Simon,

> On 4 May 2015 at 11:36, Marek Vasut <marex@denx.de> wrote:
> > On Monday, May 04, 2015 at 07:31:05 PM, Simon Glass wrote:
> >> 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 <sjg@chromium.org>
> >> Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
> > 
> > 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.

Sounds good, thanks for clarifying !

Best regards,
Marek Vasut

[U-Boot] [PATCH 13/24] sandbox: Add an implementation for cleanup_before_linux_select()

[Simon Glass]

Support this function so we can use Chrome OS verified boot with sandbox.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/cpu/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index 168f2ef..4646f07 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -55,6 +55,11 @@ int cleanup_before_linux(void)
 	return 0;
 }
 
+int cleanup_before_linux_select(int flags)
+{
+	return 0;
+}
+
 void *map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
 {
 #ifdef CONFIG_PCI
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 14/24] sandbox: Correct error handling in state_read_file()

[Simon Glass]

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

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/cpu/state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sandbox/cpu/state.c b/arch/sandbox/cpu/state.c
index 033958c..cae731c 100644
--- a/arch/sandbox/cpu/state.c
+++ b/arch/sandbox/cpu/state.c
@@ -51,7 +51,7 @@ static int state_read_file(struct sandbox_state *state, const char *fname)
 	ret = os_get_filesize(fname, &size);
 	if (ret < 0) {
 		printf("Cannot find sandbox state file '%s'\n", fname);
-		return ret;
+		return -ENOENT;
 	}
 	state->state_fdt = os_malloc(size);
 	if (!state->state_fdt) {
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 14/24] sandbox: Correct error handling in state_read_file()

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
> This function should return a useful error for U-Boot, rather than -1.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 14/24] sandbox: Correct error handling in state_read_file()

[Simon Glass]

On 4 May 2015 at 15:12, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
>> This function should return a useful error for U-Boot, rather than -1.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot-dm.

[U-Boot] [PATCH 15/24] sandbox: Add missing errno.h includes in a few files

[Simon Glass]

These files use error numbering, so add the include.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

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

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index ec01040..0707256 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -4,6 +4,7 @@
  */
 
 #include <common.h>
+#include <errno.h>
 #include <os.h>
 #include <cli.h>
 #include <malloc.h>
diff --git a/drivers/misc/i2c_eeprom_emul.c b/drivers/misc/i2c_eeprom_emul.c
index 7343445..4410d03 100644
--- a/drivers/misc/i2c_eeprom_emul.c
+++ b/drivers/misc/i2c_eeprom_emul.c
@@ -8,6 +8,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <errno.h>
 #include <fdtdec.h>
 #include <i2c.h>
 #include <malloc.h>
diff --git a/drivers/misc/swap_case.c b/drivers/misc/swap_case.c
index f6028ba..3b8aa48 100644
--- a/drivers/misc/swap_case.c
+++ b/drivers/misc/swap_case.c
@@ -9,6 +9,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <errno.h>
 #include <pci.h>
 #include <asm/test.h>
 #include <linux/ctype.h>
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 15/24] sandbox: Add missing errno.h includes in a few files

[Simon Glass]

On 4 May 2015 at 11:31, Simon Glass <sjg@chromium.org> wrote:
> These files use error numbering, so add the include.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/sandbox/cpu/start.c       | 1 +
>  drivers/misc/i2c_eeprom_emul.c | 1 +
>  drivers/misc/swap_case.c       | 1 +
>  3 files changed, 3 insertions(+)

Applied to u-boot-dm.

[U-Boot] [PATCH 16/24] sandbox: cros_ec: Support EC_CMD_ENTERING_MODE emulation

[Simon Glass]

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

Signed-off-by: Simon Glass <sjg@chromium.org>
---

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

diff --git a/drivers/misc/cros_ec_sandbox.c b/drivers/misc/cros_ec_sandbox.c
index df41e82..7509612 100644
--- a/drivers/misc/cros_ec_sandbox.c
+++ b/drivers/misc/cros_ec_sandbox.c
@@ -459,6 +459,8 @@ static int process_cmd(struct ec_state *ec,
 	case EC_CMD_MKBP_STATE:
 		len = cros_ec_keyscan(ec, resp_data);
 		break;
+	case EC_CMD_ENTERING_MODE:
+		break;
 	default:
 		printf("   ** Unknown EC command %#02x\n", req_hdr->command);
 		return -1;
diff --git a/include/ec_commands.h b/include/ec_commands.h
index 78baab1..7605066 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -1555,6 +1555,21 @@ struct ec_params_sb_wr_block {
 	uint16_t data[32];
 } __packed;
 
+/*
+ * Entering Verified Boot Mode Command
+ * Default mode is VBOOT_MODE_NORMAL if EC did not receive this command.
+ * Valid Modes are: normal, developer, and recovery.
+ */
+#define EC_CMD_ENTERING_MODE 0xb6
+
+struct ec_params_entering_mode {
+	int vboot_mode;
+} __packed;
+
+#define VBOOT_MODE_NORMAL    0
+#define VBOOT_MODE_DEVELOPER 1
+#define VBOOT_MODE_RECOVERY  2
+
 /*****************************************************************************/
 /* System commands */
 
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 16/24] sandbox: cros_ec: Support EC_CMD_ENTERING_MODE emulation

[Simon Glass]

On 4 May 2015 at 11:31, Simon Glass <sjg@chromium.org> wrote:
> Emualate this function which is used with Chrome OS verified boot.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  drivers/misc/cros_ec_sandbox.c |  2 ++
>  include/ec_commands.h          | 15 +++++++++++++++
>  2 files changed, 17 insertions(+)
>

Applied to u-boot-dm.

[U-Boot] [PATCH 17/24] sandbox: spi: Add newline to printf() in sandbox_sf_probe

[Simon Glass]

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

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/mtd/spi/sandbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c
index d576d31..895604d 100644
--- a/drivers/mtd/spi/sandbox.c
+++ b/drivers/mtd/spi/sandbox.c
@@ -129,7 +129,7 @@ static int sandbox_sf_probe(struct udevice *dev)
 		}
 	}
 	if (cs == -1) {
-		printf("Error: Unknown chip select for device '%s'",
+		printf("Error: Unknown chip select for device '%s'\n",
 		       dev->name);
 		return -EINVAL;
 	}
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 17/24] sandbox: spi: Add newline to printf() in sandbox_sf_probe

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
> This printf() should have a newline at the end. Add it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 17/24] sandbox: spi: Add newline to printf() in sandbox_sf_probe

[Jagan Teki]

On 5 May 2015 at 02:36, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
>> This printf() should have a newline at the end. Add it.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

Reviewed-by: Jagan Teki <jteki@openedev.com>

thanks!
-- 
Jagan Teki,
Openedev.

[U-Boot] [PATCH 17/24] sandbox: spi: Add newline to printf() in sandbox_sf_probe

[Simon Glass]

On 7 May 2015 at 23:58, Jagan Teki <jteki@openedev.com> wrote:
> On 5 May 2015 at 02:36, Joe Hershberger <joe.hershberger@gmail.com> wrote:
>> Hi Simon,
>>
>> On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
>>> This printf() should have a newline at the end. Add it.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>
>
> Reviewed-by: Jagan Teki <jteki@openedev.com>
>
> thanks!
> --
> Jagan Teki,
> Openedev.

Applied to u-boot-dm.

[U-Boot] [PATCH 18/24] sandbox: Fix warning in display_options

[Simon Glass]

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

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/display_options.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/display_options.c b/lib/display_options.c
index d5d17b2..495a547 100644
--- a/lib/display_options.c
+++ b/lib/display_options.c
@@ -123,7 +123,7 @@ int print_buffer(ulong addr, const void *data, uint width, uint count,
 			else
 				x = lb.uc[i] = *(volatile uint8_t *)data;
 #ifdef CONFIG_SYS_SUPPORT_64BIT_DATA
-			printf(" %0*" PRIx64, width * 2, x);
+			printf(" %0*llx", width * 2, (long long)x);
 #else
 			printf(" %0*x", width * 2, x);
 #endif
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 18/24] sandbox: Fix warning in display_options

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
> This fixes a warning in the print_buffer() function with some toolchains.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 18/24] sandbox: Fix warning in display_options

[Simon Glass]

On 4 May 2015 at 15:45, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
>> This fixes a warning in the print_buffer() function with some toolchains.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
> Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot-dm.

[U-Boot] [PATCH 19/24] sandbox: Support wide-screen LCD emulation

[Simon Glass]

Increase the LCD size to 1366x768.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/dts/sandbox.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
index efa2097..6bfa735 100644
--- a/arch/sandbox/dts/sandbox.dts
+++ b/arch/sandbox/dts/sandbox.dts
@@ -70,8 +70,8 @@
 
 	lcd {
 		compatible = "sandbox,lcd-sdl";
-		xres = <800>;
-		yres = <600>;
+		xres = <1366>;
+		yres = <768>;
 	};
 
 	gpio_a: gpios at 0 {
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 19/24] sandbox: Support wide-screen LCD emulation

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
> Increase the LCD size to 1366x768.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---

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

Thanks,
-Joe

[U-Boot] [PATCH 19/24] sandbox: Support wide-screen LCD emulation

[Simon Glass]

Hi Joe,

On 4 May 2015 at 15:27, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
>> Increase the LCD size to 1366x768.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>
> 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

[U-Boot] [PATCH 19/24] sandbox: Support wide-screen LCD emulation

[Simon Glass]

On 4 May 2015 at 15:36, Simon Glass <sjg@chromium.org> wrote:
> Hi Joe,
>
> On 4 May 2015 at 15:27, Joe Hershberger <joe.hershberger@gmail.com> wrote:
>> Hi Simon,
>>
>> On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
>>> Increase the LCD size to 1366x768.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>
>> 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

Applied to u-boot-dm.

[U-Boot] [PATCH 20/24] lcd: Support colour lookup table on 16bpp display in BMP images

[Simon Glass]

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 <sjg@chromium.org>
---

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

diff --git a/common/lcd.c b/common/lcd.c
index 055c366..2d11578 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -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);
 				}
 			}
diff --git a/include/bmp_layout.h b/include/bmp_layout.h
index 22b1fbc..47f505c 100644
--- a/include/bmp_layout.h
+++ b/include/bmp_layout.h
@@ -11,12 +11,12 @@
 #ifndef _BMP_H_
 #define _BMP_H_
 
-typedef struct bmp_color_table_entry {
+typedef struct __packed bmp_color_table_entry {
 	__u8	blue;
 	__u8	green;
 	__u8	red;
 	__u8	reserved;
-} __attribute__ ((packed)) bmp_color_table_entry_t;
+} bmp_color_table_entry_t;
 
 /* When accessing these fields, remember that they are stored in little
    endian format, so use linux macros, e.g. le32_to_cpu(width)          */
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 20/24] lcd: Support colour lookup table on 16bpp display in BMP images

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
> 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 <sjg@chromium.org>
> ---
>
>  common/lcd.c         | 23 ++++++++++++++++++++---
>  include/bmp_layout.h |  4 ++--
>  2 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/common/lcd.c b/common/lcd.c
> index 055c366..2d11578 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -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);
>                                 }
>                         }
> diff --git a/include/bmp_layout.h b/include/bmp_layout.h
> index 22b1fbc..47f505c 100644
> --- a/include/bmp_layout.h
> +++ b/include/bmp_layout.h
> @@ -11,12 +11,12 @@
>  #ifndef _BMP_H_
>  #define _BMP_H_
>
> -typedef struct bmp_color_table_entry {
> +typedef struct __packed bmp_color_table_entry {

It would be great to drop the typedef here.

>         __u8    blue;
>         __u8    green;
>         __u8    red;
>         __u8    reserved;
> -} __attribute__ ((packed)) bmp_color_table_entry_t;
> +} bmp_color_table_entry_t;
>
>  /* When accessing these fields, remember that they are stored in little
>     endian format, so use linux macros, e.g. le32_to_cpu(width)          */
> --

Cheers,
-Joe

[U-Boot] [PATCH 21/24] tegra124: Implement spl_was_boot_source()

[Simon Glass]

Add an implementation of this function for Tegra.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/arm/mach-tegra/board.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/mach-tegra/board.c b/arch/arm/mach-tegra/board.c
index 222de6a..77be211 100644
--- a/arch/arm/mach-tegra/board.c
+++ b/arch/arm/mach-tegra/board.c
@@ -6,6 +6,7 @@
  */
 
 #include <common.h>
+#include <spl.h>
 #include <asm/io.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/funcmux.h>
@@ -29,6 +30,21 @@ enum {
 	UART_COUNT = 5,
 };
 
+static bool from_spl __attribute__ ((section(".data")));
+
+#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
+
+bool spl_was_boot_source(void)
+{
+	return from_spl;
+}
+
 #if defined(CONFIG_TEGRA_SUPPORT_NON_SECURE)
 #if !defined(CONFIG_TEGRA124)
 #error tegra_cpu_is_non_secure has only been validated on Tegra124
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 21/24] tegra124: Implement spl_was_boot_source()

[Stephen Warren]

On 05/04/2015 11:31 AM, Simon Glass wrote:
> Add an implementation of this function for Tegra.

> diff --git a/arch/arm/mach-tegra/board.c b/arch/arm/mach-tegra/board.c

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

[U-Boot] [PATCH 21/24] tegra124: Implement spl_was_boot_source()

[Simon Glass]

Hi Stephen,

On 5 May 2015 at 09:54, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/04/2015 11:31 AM, Simon Glass wrote:
>>
>> Add an implementation of this function for Tegra.
>
>
>> diff --git a/arch/arm/mach-tegra/board.c b/arch/arm/mach-tegra/board.c
>
>
>> +#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.

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

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

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

Regards,
Simon

[U-Boot] [PATCH 21/24] tegra124: Implement spl_was_boot_source()

[Stephen Warren]

On 05/05/2015 10:02 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 5 May 2015 at 09:54, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 05/04/2015 11:31 AM, Simon Glass wrote:
>>>
>>> Add an implementation of this function for Tegra.
>>
>>
>>> diff --git a/arch/arm/mach-tegra/board.c b/arch/arm/mach-tegra/board.c
>>
>>
>>> +#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.
>
> 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?

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

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?

[U-Boot] [PATCH 21/24] tegra124: Implement spl_was_boot_source()

[Simon Glass]

Hi Stephen,

On 5 May 2015 at 10:10, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/05/2015 10:02 AM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 5 May 2015 at 09:54, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 05/04/2015 11:31 AM, Simon Glass wrote:
>>>>
>>>>
>>>> Add an implementation of this function for Tegra.
>>>
>>>
>>>
>>>> diff --git a/arch/arm/mach-tegra/board.c b/arch/arm/mach-tegra/board.c
>>>
>>>
>>>
>>>> +#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.
>>
>>
>> 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?

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

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

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

Regards,
Simon

[U-Boot] [PATCH 21/24] tegra124: Implement spl_was_boot_source()

[Stephen Warren]

On 05/05/2015 10:19 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 5 May 2015 at 10:10, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 05/05/2015 10:02 AM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 5 May 2015 at 09:54, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>
>>>> On 05/04/2015 11:31 AM, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Add an implementation of this function for Tegra.
>>>>
>>>>
>>>>
>>>>> diff --git a/arch/arm/mach-tegra/board.c b/arch/arm/mach-tegra/board.c
>>>>
>>>>
>>>>
>>>>> +#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.
>>>
>>>
>>> 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?
>
> I really want to say that it is not chain-loaded from SPL. Maybe
> UBOOT_NOT_LOADED_FROM_SPL?

OK, that highlights that better.

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

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

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

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

[U-Boot] [PATCH 21/24] tegra124: Implement spl_was_boot_source()

[Simon Glass]

Hi Stephen,

On 5 May 2015 at 12:07, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/05/2015 10:19 AM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 5 May 2015 at 10:10, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 05/05/2015 10:02 AM, Simon Glass wrote:
>>>>
>>>>
>>>> Hi Stephen,
>>>>
>>>> On 5 May 2015 at 09:54, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>>
>>>>>
>>>>> On 05/04/2015 11:31 AM, Simon Glass wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Add an implementation of this function for Tegra.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> diff --git a/arch/arm/mach-tegra/board.c b/arch/arm/mach-tegra/board.c
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> +#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.
>>>>
>>>>
>>>>
>>>> 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?
>>
>>
>> I really want to say that it is not chain-loaded from SPL. Maybe
>> UBOOT_NOT_LOADED_FROM_SPL?
>
>
> OK, that highlights that better.
>
>>>>> 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.
>>>>
>>>>> 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.
>>>
>>>
>>>
>>> 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)
>
>
> That's a design decision. There's absolutely no need for that to be true.

At present U-Boot has a single CONFIG for the environment
type/position. Therefore we can't have the same U-Boot behave
differently.

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

That's why we have the load-env device tree option, which uses the
default environment. But since the RO and RW U-Boot then have the same
environment, this doesn't really help.

Yes we could validate it, although it's a pain since it is one more
thing to sign and hash and it's not in the FIT.

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

This seems pretty simple to me.

Regards,
Simon

[U-Boot] [PATCH 22/24] tegra: nyan-big: Allow TPM on I2C

[Simon Glass]

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

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 board/nvidia/nyan-big/pinmux-config-nyan-big.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/board/nvidia/nyan-big/pinmux-config-nyan-big.h b/board/nvidia/nyan-big/pinmux-config-nyan-big.h
index 9c5fbaa..ddf572f 100644
--- a/board/nvidia/nyan-big/pinmux-config-nyan-big.h
+++ b/board/nvidia/nyan-big/pinmux-config-nyan-big.h
@@ -234,8 +234,8 @@ static const struct pmux_pingrp_config nyan_big_pingrps[] = {
 	PINCFG(SDMMC4_DAT6_PAA6,       SDMMC4,      UP,     NORMAL,   INPUT,   DEFAULT, DEFAULT),
 	PINCFG(SDMMC4_DAT7_PAA7,       SDMMC4,      UP,     NORMAL,   INPUT,   DEFAULT, DEFAULT),
 	PINCFG(PBB0,                   VGP6,        DOWN,   TRISTATE, OUTPUT,  DEFAULT, DEFAULT),
-	PINCFG(CAM_I2C_SCL_PBB1,       RSVD3,       DOWN,   TRISTATE, OUTPUT,  DISABLE, DEFAULT),
-	PINCFG(CAM_I2C_SDA_PBB2,       RSVD3,       DOWN,   TRISTATE, OUTPUT,  DISABLE, DEFAULT),
+	PINCFG(CAM_I2C_SCL_PBB1,       I2C3,        NORMAL, NORMAL,   INPUT,   ENABLE,  DEFAULT),
+	PINCFG(CAM_I2C_SDA_PBB2,       I2C3,        NORMAL, NORMAL,   INPUT,   ENABLE,  DEFAULT),
 	PINCFG(PBB3,                   VGP3,        DOWN,   TRISTATE, OUTPUT,  DEFAULT, DEFAULT),
 	PINCFG(PBB4,                   VGP4,        DOWN,   TRISTATE, OUTPUT,  DEFAULT, DEFAULT),
 	PINCFG(PBB5,                   RSVD3,       DOWN,   TRISTATE, OUTPUT,  DEFAULT, DEFAULT),
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 22/24] tegra: nyan-big: Allow TPM on I2C

[Stephen Warren]

On 05/04/2015 11:31 AM, Simon Glass wrote:
> Enable the I2C3 pins so that the TPM can be used.

Can you please update the configuration in tegra-pinmux-scripts and the 
Linux kernel DT too, then?

[U-Boot] [PATCH 23/24] tegra: mmc: Set the removable flag correctly

[Simon Glass]

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 <sjg@chromium.org>
---

 drivers/mmc/tegra_mmc.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
index 2cd8cf1..5c9c54a 100644
--- a/drivers/mmc/tegra_mmc.c
+++ b/drivers/mmc/tegra_mmc.c
@@ -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;
 }
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 23/24] tegra: mmc: Set the removable flag correctly

[Joe Hershberger]

Hi Simon,

On Mon, May 4, 2015 at 12:31 PM, Simon Glass <sjg@chromium.org> wrote:
> 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 <sjg@chromium.org>
> ---

Reviewed-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH 23/24] tegra: mmc: Set the removable flag correctly

[Pantelis Antoniou]

Hi Simon,

> On May 4, 2015, at 20:31 , Simon Glass <sjg@chromium.org> wrote:
> 
> 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 <sjg@chromium.org>
> ---
> 
> drivers/mmc/tegra_mmc.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
> index 2cd8cf1..5c9c54a 100644
> --- a/drivers/mmc/tegra_mmc.c
> +++ b/drivers/mmc/tegra_mmc.c
> @@ -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;
> }
> -- 
> 2.2.0.rc0.207.ga3a616c
> 

Applied, thanks.

? Pantelis

[U-Boot] [PATCH 24/24] tegra124: Expand SPL space by 8KB

[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 <sjg@chromium.org>
---

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

diff --git a/include/configs/tegra124-common.h b/include/configs/tegra124-common.h
index f2b3774..0347132 100644
--- a/include/configs/tegra124-common.h
+++ b/include/configs/tegra124-common.h
@@ -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

[U-Boot] [PATCH 24/24] tegra124: Expand SPL space by 8KB

[Stephen Warren]

On 05/04/2015 11:31 AM, Simon Glass wrote:
> 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.

[U-Boot] [PATCH 24/24] tegra124: Expand SPL space by 8KB

[Simon Glass]

Hi Stephen,

On 5 May 2015 at 09:59, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/04/2015 11:31 AM, Simon Glass wrote:
>>
>> 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.

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

>
> You forgot to CC Tom Warren on this Tegra patch.

Ooops, I should have used a 'tegra' tag.

Regards,
Simon

[U-Boot] [PATCH 24/24] tegra124: Expand SPL space by 8KB

[Stephen Warren]

On 05/05/2015 10:03 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 5 May 2015 at 09:59, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 05/04/2015 11:31 AM, Simon Glass wrote:
>>>
>>> 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.
>
> 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?

[U-Boot] [PATCH 24/24] tegra124: Expand SPL space by 8KB

[Simon Glass]

Hi Stephen,

On 5 May 2015 at 10:12, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/05/2015 10:03 AM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 5 May 2015 at 09:59, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 05/04/2015 11:31 AM, Simon Glass wrote:
>>>>
>>>>
>>>> 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.

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

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

Regards,
Simon

[U-Boot] [PATCH 24/24] tegra124: Expand SPL space by 8KB

[Stephen Warren]

On 05/05/2015 10:26 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 5 May 2015 at 10:12, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 05/05/2015 10:03 AM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 5 May 2015 at 09:59, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>
>>>> On 05/04/2015 11:31 AM, Simon Glass wrote:
>>>>>
>>>>>
>>>>> 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.
>
> We were about 1KB away, mostly due to the gcc garbage collection bug I
> think. The cros compiler seems even worse, not sure why.
>
>>>> 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?
>>
>>
>> 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?

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.

[U-Boot] [PATCH 24/24] tegra124: Expand SPL space by 8KB

[Simon Glass]

Hi Stephen,

On 5 May 2015 at 12:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/05/2015 10:26 AM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 5 May 2015 at 10:12, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 05/05/2015 10:03 AM, Simon Glass wrote:
>>>>
>>>>
>>>> Hi Stephen,
>>>>
>>>> On 5 May 2015 at 09:59, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>>
>>>>>
>>>>> On 05/04/2015 11:31 AM, Simon Glass wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> 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.
>>
>>
>> We were about 1KB away, mostly due to the gcc garbage collection bug I
>> think. The cros compiler seems even worse, not sure why.
>>
>>>>> 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?
>>>
>>>
>>>
>>> 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?
>
>
> 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.

Here I am enabling the TPM in U-Boot. There may or may not be a kernel
TPM driver. Presumably if there is, then something is wrong, because
it has the wrong pinmux and won't work, or someone did update it in
the kernel and not in U-Boot? Actually I'm not sure that I care about
it running in Linux any more than someone working in Linux cares that
the TPM is working in U-Boot.

Why can't this stuff go in the device tree, which is already shared
between U-Boot and Linux?

Regards,
Simon

[U-Boot] [PATCH 24/24] tegra124: Expand SPL space by 8KB

[Simon Glass]

+Tom Warren

On 4 May 2015 at 11:31, Simon Glass <sjg@chromium.org> wrote:
> 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 <sjg@chromium.org>
> ---
>
>  include/configs/tegra124-common.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/configs/tegra124-common.h b/include/configs/tegra124-common.h
> index f2b3774..0347132 100644
> --- a/include/configs/tegra124-common.h
> +++ b/include/configs/tegra124-common.h
> @@ -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
>

[U-Boot] [PATCH 24/24] tegra124: Expand SPL space by 8KB

[Tom Warren]

LGTM, but it s/b 'Add a _little_ more space' in the commit msg.

Should we change the TEXT_BASE for the CPU portion in all Tegra builds?

> -----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
> 
> On 4 May 2015 at 11:31, Simon Glass <sjg@chromium.org> wrote:
> > 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 <sjg@chromium.org>
> > ---
> >
> >  include/configs/tegra124-common.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/configs/tegra124-common.h
> > b/include/configs/tegra124-common.h
> > index f2b3774..0347132 100644
> > --- a/include/configs/tegra124-common.h
> > +++ b/include/configs/tegra124-common.h
> > @@ -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
> >

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[U-Boot] [PATCH 01/24] usb: add device connection/disconnection detection

[Simon Glass]

On 4 May 2015 at 11:30, Simon Glass <sjg@chromium.org> wrote:
>
> From: Vincent Palatin <vpalatin@chromium.org>
>
> 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 <vpalatin@chromium.org>
>
> Reviewed-by: Stefan Reinauer <reinauer@chromium.org>
> Tested-by: Stefan Reinauer <reinauer@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  common/usb.c     | 26 ++++++++++++++++++++++++++
>  common/usb_hub.c |  2 +-
>  include/usb.h    |  2 ++
>  3 files changed, 29 insertions(+), 1 deletion(-)

Applied to u-boot-dm, thanks Vincent.