[U-Boot] [PATCH] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password

[U-Boot] [PATCH] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password

[Stefan Roese]

This patch adds the feature to only stop the autobooting, and therefor
boot into the U-Boot prompt, when the input string / password matches
a values that is encypted via a SHA256 hash and saved in the environment.

This feature is enabled by defined these config options:
     CONFIG_AUTOBOOT_KEYED
     CONFIG_AUTOBOOT_STOP_STR_SHA256

Signed-off-by: Stefan Roese <sr@denx.de>
---
 common/autoboot.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/common/autoboot.c b/common/autoboot.c
index c27cc2c..4635551 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -12,6 +12,7 @@
 #include <fdtdec.h>
 #include <menu.h>
 #include <post.h>
+#include <u-boot/sha256.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -35,6 +36,11 @@ static int abortboot_keyed(int bootdelay)
 {
 	int abort = 0;
 	uint64_t etime = endtick(bootdelay);
+#if defined(CONFIG_AUTOBOOT_STOP_STR_SHA256)
+	const char *sha_env_str = getenv("bootstopkeysha256");
+	u8 sha_env[SHA256_SUM_LEN];
+	u8 sha[SHA256_SUM_LEN];
+#else
 	struct {
 		char *str;
 		u_int len;
@@ -46,10 +52,11 @@ static int abortboot_keyed(int bootdelay)
 		{ .str = getenv("bootstopkey"),   .retry = 0 },
 		{ .str = getenv("bootstopkey2"),  .retry = 0 },
 	};
+	u_int presskey_max = 0;
+#endif
 
 	char presskey[MAX_DELAY_STOP_STR];
 	u_int presskey_len = 0;
-	u_int presskey_max = 0;
 	u_int i;
 
 #ifndef CONFIG_ZERO_BOOTDELAY_CHECK
@@ -61,6 +68,41 @@ static int abortboot_keyed(int bootdelay)
 	printf(CONFIG_AUTOBOOT_PROMPT);
 #  endif
 
+#if defined(CONFIG_AUTOBOOT_STOP_STR_SHA256)
+	if (sha_env_str == NULL)
+		sha_env_str = CONFIG_AUTOBOOT_STOP_STR_SHA256;
+
+	/*
+	 * Generate the binary value from the environment hash value
+	 * so that we can compare this value with the computed hash
+	 * from the user input
+	 */
+	for (i = 0; i < SHA256_SUM_LEN; i++) {
+		char chr[3];
+
+		strncpy(chr, &sha_env_str[i * 2], 2);
+		sha_env[i] = simple_strtoul(chr, NULL, 16);
+	}
+
+	/*
+	 * We don't know how long the stop-string is, so we need to
+	 * generate the sha256 hash upon each input character and
+	 * compare the value with the one saved in the environment
+	 */
+	do {
+		if (tstc()) {
+			presskey[presskey_len++] = getc();
+
+			/* Calculate sha256 upon each new char */
+			sha256_csum_wd((unsigned char *)presskey, presskey_len,
+				       sha, CHUNKSZ_SHA256);
+
+			/* And check if sha matches saved value in env */
+			if (memcmp(sha, sha_env, SHA256_SUM_LEN) == 0)
+				abort = 1;
+		}
+	} while (!abort && get_ticks() <= etime);
+#else
 #  ifdef CONFIG_AUTOBOOT_DELAY_STR
 	if (delaykey[0].str == NULL)
 		delaykey[0].str = CONFIG_AUTOBOOT_DELAY_STR;
@@ -124,6 +166,7 @@ static int abortboot_keyed(int bootdelay)
 			}
 		}
 	} while (!abort && get_ticks() <= etime);
+#endif
 
 	if (!abort)
 		debug_bootkeys("key timeout\n");
-- 
2.3.2

[U-Boot] [PATCH] bootcount: Add dcache flush to bootcount_store()

[Stefan Roese]

Without this dcache_flush the updated bootcounter may not be saved to
its location.

This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
as bootcounter storage area. And issuing "reset" from within U-Boot
cause the bootcounter to stay on its initial value.

Signed-off-by: Stefan Roese <sr@denx.de>
---
 drivers/bootcount/bootcount.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bootcount/bootcount.c b/drivers/bootcount/bootcount.c
index e0343f7..b3e79de 100644
--- a/drivers/bootcount/bootcount.c
+++ b/drivers/bootcount/bootcount.c
@@ -57,9 +57,11 @@ __weak void bootcount_store(ulong a)
 
 #if defined(CONFIG_SYS_BOOTCOUNT_SINGLEWORD)
 	raw_bootcount_store(reg, (BOOTCOUNT_MAGIC & 0xffff0000) | a);
+	flush_dcache_range((u32)reg, (u32)reg + 4);
 #else
 	raw_bootcount_store(reg, a);
 	raw_bootcount_store(reg + 4, BOOTCOUNT_MAGIC);
+	flush_dcache_range((u32)reg, (u32)reg + 8);
 #endif
 }
 
-- 
2.3.2

[U-Boot] [PATCH] cmd_led: Extend led command to support blinking and more leds

[Stefan Roese]

This patch extends the U-Boot "led" command to support automatic blinking
by setting a blink frequency in milliseconds. Additionally the number of
supported LEDs is increased to 6 (0...5).

This will be used by the PCA9551 LED driver.

Signed-off-by: Stefan Roese <sr@denx.de>
---
 common/cmd_led.c          | 48 +++++++++++++++++++++++++++++++++++++----------
 drivers/misc/status_led.c | 14 ++++++++++++++
 include/status_led.h      |  1 +
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/common/cmd_led.c b/common/cmd_led.c
index 172bc30..b0f1a61 100644
--- a/common/cmd_led.c
+++ b/common/cmd_led.c
@@ -39,6 +39,12 @@ static const led_tbl_t led_commands[] = {
 #ifdef STATUS_LED_BIT3
 	{ "3", STATUS_LED_BIT3, NULL, NULL, NULL },
 #endif
+#ifdef STATUS_LED_BIT4
+	{ "4", STATUS_LED_BIT4, NULL, NULL, NULL },
+#endif
+#ifdef STATUS_LED_BIT5
+	{ "5", STATUS_LED_BIT5, NULL, NULL, NULL },
+#endif
 #endif
 #ifdef STATUS_LED_GREEN
 	{ "green", STATUS_LED_GREEN, green_led_off, green_led_on, NULL },
@@ -55,30 +61,39 @@ static const led_tbl_t led_commands[] = {
 	{ NULL, 0, NULL, NULL, NULL }
 };
 
-enum led_cmd { LED_ON, LED_OFF, LED_TOGGLE };
+enum led_cmd { LED_ON, LED_OFF, LED_TOGGLE, LED_BLINK };
 
 enum led_cmd get_led_cmd(char *var)
 {
-	if (strcmp(var, "off") == 0) {
+	if (strcmp(var, "off") == 0)
 		return LED_OFF;
-	}
-	if (strcmp(var, "on") == 0) {
+	if (strcmp(var, "on") == 0)
 		return LED_ON;
-	}
 	if (strcmp(var, "toggle") == 0)
 		return LED_TOGGLE;
+	if (strcmp(var, "blink") == 0)
+		return LED_BLINK;
+
 	return -1;
 }
 
+/*
+ * LED drivers providing a blinking LED functionality, like the
+ * PCA9551, can override this empty weak function
+ */
+void __weak __led_blink(led_id_t mask, int freq)
+{
+}
+
 int do_led (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	int i, match = 0;
 	enum led_cmd cmd;
+	int freq;
 
 	/* Validate arguments */
-	if ((argc != 3)) {
+	if ((argc < 3) || (argc > 4))
 		return CMD_RET_USAGE;
-	}
 
 	cmd = get_led_cmd(argv[2]);
 	if (cmd < 0) {
@@ -109,6 +124,13 @@ int do_led (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 					led_commands[i].toggle();
 				else
 					__led_toggle(led_commands[i].mask);
+				break;
+			case LED_BLINK:
+				if (argc != 4)
+					return CMD_RET_USAGE;
+
+				freq = simple_strtoul(argv[3], NULL, 10);
+				__led_blink(led_commands[i].mask, freq);
 			}
 			/* Need to set only 1 led if led_name wasn't 'all' */
 			if (strcmp("all", argv[1]) != 0)
@@ -125,7 +147,7 @@ int do_led (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 }
 
 U_BOOT_CMD(
-	led, 3, 1, do_led,
+	led, 4, 1, do_led,
 	"["
 #ifdef CONFIG_BOARD_SPECIFIC_LED
 #ifdef STATUS_LED_BIT
@@ -140,6 +162,12 @@ U_BOOT_CMD(
 #ifdef STATUS_LED_BIT3
 	"3|"
 #endif
+#ifdef STATUS_LED_BIT4
+	"4|"
+#endif
+#ifdef STATUS_LED_BIT5
+	"5|"
+#endif
 #endif
 #ifdef STATUS_LED_GREEN
 	"green|"
@@ -153,6 +181,6 @@ U_BOOT_CMD(
 #ifdef STATUS_LED_BLUE
 	"blue|"
 #endif
-	"all] [on|off|toggle]",
-	"[led_name] [on|off|toggle] sets or clears led(s)"
+	"all] [on|off|toggle|blink] [blink-freq in ms]",
+	"[led_name] [on|off|toggle|blink] sets or clears led(s)"
 );
diff --git a/drivers/misc/status_led.c b/drivers/misc/status_led.c
index ed9adb2..9869d98 100644
--- a/drivers/misc/status_led.c
+++ b/drivers/misc/status_led.c
@@ -53,6 +53,20 @@ led_dev_t led_dev[] = {
 	0,
     },
 #endif
+#if defined(STATUS_LED_BIT4)
+    {	STATUS_LED_BIT4,
+	STATUS_LED_STATE4,
+	STATUS_LED_PERIOD4,
+	0,
+    },
+#endif
+#if defined(STATUS_LED_BIT5)
+    {	STATUS_LED_BIT5,
+	STATUS_LED_STATE5,
+	STATUS_LED_PERIOD5,
+	0,
+    },
+#endif
 };
 
 #define MAX_LED_DEV	(sizeof(led_dev)/sizeof(led_dev_t))
diff --git a/include/status_led.h b/include/status_led.h
index 27f4bdf..a5e35df 100644
--- a/include/status_led.h
+++ b/include/status_led.h
@@ -105,6 +105,7 @@ typedef unsigned long led_id_t;
 extern void __led_toggle (led_id_t mask);
 extern void __led_init (led_id_t mask, int state);
 extern void __led_set (led_id_t mask, int state);
+void __led_blink(led_id_t mask, int freq);
 #else
 # error Status LED configuration missing
 #endif
-- 
2.3.2

[U-Boot] [PATCH] misc: led: Add PCA9551 LED driver

[Stefan Roese]

This patch adds a driver for the PCA9551 LED controller.

Originated-by: Timo Herbrecher <t.herbrecher@gateware.de>
Signed-off-by: Stefan Roese <sr@denx.de>
---
 drivers/misc/Kconfig       |  14 +++++
 drivers/misc/Makefile      |   1 +
 drivers/misc/pca9551_led.c | 147 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+)
 create mode 100644 drivers/misc/pca9551_led.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 36a8f0d..b8884ec 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -61,3 +61,17 @@ config CONFIG_FSL_SEC_MON
 	  system states.
 	  Security Monitor can be transitioned on any security failures,
 	  like software violations or hardware security violations.
+
+config PCA9551_LED
+	bool "Enable PCA9551 LED driver"
+	depends on TARGET_TQMA6
+	help
+	  Enable driver for PCA9551 LED controller. This controller
+	  is connected via I2C. So I2C needs to be enabled.
+
+config PCA9551_I2C_ADDR
+	hex "I2C address of PCA9551 LED controller"
+	depends on PCA9551_LED
+	default 0x60
+	help
+	  The I2C address of the PCA9551 LED controller.
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 6028cd4..149e686 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -29,3 +29,4 @@ obj-$(CONFIG_STATUS_LED) += status_led.o
 obj-$(CONFIG_TWL4030_LED) += twl4030_led.o
 obj-$(CONFIG_FSL_IFC) += fsl_ifc.o
 obj-$(CONFIG_FSL_SEC_MON) += fsl_sec_mon.o
+obj-$(CONFIG_PCA9551_LED) += pca9551_led.o
diff --git a/drivers/misc/pca9551_led.c b/drivers/misc/pca9551_led.c
new file mode 100644
index 0000000..273cc53
--- /dev/null
+++ b/drivers/misc/pca9551_led.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (C) 2015 Stefan Roese <sr@denx.de>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <i2c.h>
+
+#ifndef CONFIG_PCA9551_I2C_ADDR
+#error "CONFIG_PCA9551_I2C_ADDR not defined!"
+#endif
+
+#define PCA9551_REG_INPUT	0x00	/* Input register (read only) */
+#define PCA9551_REG_PSC0	0x01	/* Frequency prescaler 0 */
+#define PCA9551_REG_PWM0	0x02	/* PWM0 */
+#define PCA9551_REG_PSC1	0x03	/* Frequency prescaler 1 */
+#define PCA9551_REG_PWM1	0x04	/* PWM1 */
+#define PCA9551_REG_LS0		0x05	/* LED0 to LED3 selector */
+#define PCA9551_REG_LS1		0x06	/* LED4 to LED7 selector */
+
+#define PCA9551_CTRL_AI		(1 << 4)	/* Auto-increment flag */
+
+#define PCA9551_LED_STATE_ON		0x00
+#define PCA9551_LED_STATE_OFF		0x01
+#define PCA9551_LED_STATE_BLINK0	0x02
+#define PCA9551_LED_STATE_BLINK1	0x03
+
+struct pca9551_blink_rate {
+	u8 psc;	/* Frequency preescaler, see PCA9551_7.pdf p. 6 */
+	u8 pwm;	/* Pulse width modulation, see PCA9551_7.pdf p. 6 */
+} __packed;
+
+static int freq0, freq1;
+
+static int pca9551_led_get_state(int led, int *state)
+{
+	unsigned int reg;
+	u8 shift, buf;
+
+	if (led < 0 || led > 7) {
+		return -1;
+	} else if (led < 4) {
+		reg = PCA9551_REG_LS0;
+		shift = led << 1;
+	} else {
+		reg = PCA9551_REG_LS1;
+		shift = (led - 4) << 1;
+	}
+
+	if (i2c_read(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1))
+		return -1;
+
+	*state = (buf >> shift) & 0x03;
+	return 0;
+}
+
+static int pca9551_led_set_state(int led, int state)
+{
+	unsigned int reg;
+	u8 shift, buf, mask;
+
+	if (led < 0 || led > 7) {
+		return -1;
+	} else if (led < 4) {
+		reg = PCA9551_REG_LS0;
+		shift = led << 1;
+	} else {
+		reg = PCA9551_REG_LS1;
+		shift = (led - 4) << 1;
+	}
+	mask = 0x03 << shift;
+
+	if (i2c_read(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1))
+		return -1;
+
+	buf = (buf & ~mask) | ((state & 0x03) << shift);
+
+	if (i2c_write(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1))
+		return -1;
+
+	return 0;
+}
+
+static int pca9551_led_set_blink_rate(int idx, struct pca9551_blink_rate rate)
+{
+	unsigned int reg;
+
+	switch (idx) {
+	case 0:
+		reg = PCA9551_REG_PSC0;
+		break;
+	case 1:
+		reg = PCA9551_REG_PSC1;
+		break;
+	default:
+		return -1;
+	}
+	reg |= PCA9551_CTRL_AI;
+
+	if (i2c_write(CONFIG_PCA9551_I2C_ADDR, reg, 1, (u8 *)&rate, 2))
+		return -1;
+
+	return 0;
+}
+
+/*
+ * Functions referenced by cmd_led.c
+ */
+void __led_set(led_id_t mask, int state)
+{
+	if (state == STATUS_LED_OFF)
+		pca9551_led_set_state(mask, PCA9551_LED_STATE_OFF);
+	else
+		pca9551_led_set_state(mask, PCA9551_LED_STATE_ON);
+}
+
+void __led_toggle(led_id_t mask)
+{
+	int state = 0;
+
+	pca9551_led_get_state(mask, &state);
+	pca9551_led_set_state(mask, !state);
+}
+
+void __led_blink(led_id_t mask, int freq)
+{
+	struct pca9551_blink_rate rate;
+	int mode;
+	int blink;
+
+	if ((freq0 == 0) || (freq == freq0)) {
+		blink = 0;
+		mode = PCA9551_LED_STATE_BLINK0;
+		freq0 = freq;
+	} else {
+		blink = 1;
+		mode = PCA9551_LED_STATE_BLINK1;
+		freq1 = freq;
+	}
+
+	rate.psc = ((freq * 38) / 1000) - 1;
+	rate.pwm = 128;		/* 50% duty cycle */
+
+	pca9551_led_set_blink_rate(blink, rate);
+	pca9551_led_set_state(mask, mode);
+}
-- 
2.3.2

[U-Boot] [PATCH] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password

[Tom Rini]

On Wed, Mar 11, 2015 at 09:51:37AM +0100, Stefan Roese wrote:

> This patch adds the feature to only stop the autobooting, and therefor
> boot into the U-Boot prompt, when the input string / password matches
> a values that is encypted via a SHA256 hash and saved in the environment.
> 
> This feature is enabled by defined these config options:
>      CONFIG_AUTOBOOT_KEYED
>      CONFIG_AUTOBOOT_STOP_STR_SHA256
> 
> Signed-off-by: Stefan Roese <sr@denx.de>

This is certainly interesting but I think brings us back to a point
Simon made a long while back about needing to factor out this code
better.  Especially since this adds big long #if-#else-#endif blocks.
Can we re-do this so at least have some functions be called out instead?
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150311/2ae2733d/attachment.sig>

[U-Boot] [PATCH] cmd_led: Extend led command to support blinking and more leds

[Tom Rini]

On Wed, Mar 11, 2015 at 09:51:39AM +0100, Stefan Roese wrote:

> This patch extends the U-Boot "led" command to support automatic blinking
> by setting a blink frequency in milliseconds. Additionally the number of
> supported LEDs is increased to 6 (0...5).
> 
> This will be used by the PCA9551 LED driver.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150311/81d198d7/attachment.sig>

[U-Boot] [PATCH] bootcount: Add dcache flush to bootcount_store()

[Tom Rini]

On Wed, Mar 11, 2015 at 09:51:38AM +0100, Stefan Roese wrote:

> Without this dcache_flush the updated bootcounter may not be saved to
> its location.
> 
> This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
> as bootcounter storage area. And issuing "reset" from within U-Boot
> cause the bootcounter to stay on its initial value.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150311/0722137a/attachment.sig>

[U-Boot] [PATCH] misc: led: Add PCA9551 LED driver

[Tom Rini]

On Wed, Mar 11, 2015 at 09:51:40AM +0100, Stefan Roese wrote:

> This patch adds a driver for the PCA9551 LED controller.
> 
> Originated-by: Timo Herbrecher <t.herbrecher@gateware.de>
> Signed-off-by: Stefan Roese <sr@denx.de>

Reviewed-by: Tom Rini <trini@konsulko.com>

But:

> +struct pca9551_blink_rate {
> +	u8 psc;	/* Frequency preescaler, see PCA9551_7.pdf p. 6 */
> +	u8 pwm;	/* Pulse width modulation, see PCA9551_7.pdf p. 6 */
> +} __packed;

Yes, really, this must be packed because otherwise we get unwanted
padding in between these u8s?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150311/7aaa2f11/attachment.sig>

[U-Boot] [PATCH] misc: led: Add PCA9551 LED driver

[Fabio Estevam]

Hi Stefan,

On Wed, Mar 11, 2015 at 5:51 AM, Stefan Roese <sr@denx.de> wrote:

> +
> +config PCA9551_LED
> +       bool "Enable PCA9551 LED driver"
> +       depends on TARGET_TQMA6

Is this 'depends' correct? We should be able to use this driver with
other boards.

> +static int pca9551_led_get_state(int led, int *state)
> +{
> +       unsigned int reg;
> +       u8 shift, buf;
> +
> +       if (led < 0 || led > 7) {
> +               return -1;
> +       } else if (led < 4) {
> +               reg = PCA9551_REG_LS0;
> +               shift = led << 1;
> +       } else {
> +               reg = PCA9551_REG_LS1;
> +               shift = (led - 4) << 1;
> +       }
> +
> +       if (i2c_read(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1))
> +               return -1;

what about:

ret = i2c_read(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1);
if (ret)
    return ret;

> +
> +       *state = (buf >> shift) & 0x03;
> +       return 0;
> +}
> +
> +static int pca9551_led_set_state(int led, int state)
> +{
> +       unsigned int reg;
> +       u8 shift, buf, mask;
> +
> +       if (led < 0 || led > 7) {
> +               return -1;

return -EINVAL;

> +       } else if (led < 4) {
> +               reg = PCA9551_REG_LS0;
> +               shift = led << 1;
> +       } else {
> +               reg = PCA9551_REG_LS1;
> +               shift = (led - 4) << 1;
> +       }
> +       mask = 0x03 << shift;
> +
> +       if (i2c_read(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1))
> +               return -1;

Same here.

> +
> +       buf = (buf & ~mask) | ((state & 0x03) << shift);
> +
> +       if (i2c_write(CONFIG_PCA9551_I2C_ADDR, reg, 1, &buf, 1))
> +               return -1;

Same here.

> +
> +       return 0;
> +}
> +
> +static int pca9551_led_set_blink_rate(int idx, struct pca9551_blink_rate rate)
> +{
> +       unsigned int reg;
> +
> +       switch (idx) {
> +       case 0:
> +               reg = PCA9551_REG_PSC0;
> +               break;
> +       case 1:
> +               reg = PCA9551_REG_PSC1;
> +               break;
> +       default:
> +               return -1;

return -EINVAL;

[U-Boot] [PATCH] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password

[Stefan Roese]

Hi Tom,

On 11.03.2015 15:36, Tom Rini wrote:
> On Wed, Mar 11, 2015 at 09:51:37AM +0100, Stefan Roese wrote:
>
>> This patch adds the feature to only stop the autobooting, and therefor
>> boot into the U-Boot prompt, when the input string / password matches
>> a values that is encypted via a SHA256 hash and saved in the environment.
>>
>> This feature is enabled by defined these config options:
>>       CONFIG_AUTOBOOT_KEYED
>>       CONFIG_AUTOBOOT_STOP_STR_SHA256
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>
> This is certainly interesting but I think brings us back to a point
> Simon made a long while back about needing to factor out this code
> better.  Especially since this adds big long #if-#else-#endif blocks.
> Can we re-do this so at least have some functions be called out instead?

Yes, I'll try to rework this patch a bit to make this feature 
integration less ugly.

Thanks,
Stefan

[U-Boot] [PATCH] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password

[Simon Glass]

Hi,

On 11 March 2015 at 08:36, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Mar 11, 2015 at 09:51:37AM +0100, Stefan Roese wrote:
>
> > This patch adds the feature to only stop the autobooting, and therefor
> > boot into the U-Boot prompt, when the input string / password matches
> > a values that is encypted via a SHA256 hash and saved in the environment.
> >
> > This feature is enabled by defined these config options:
> >      CONFIG_AUTOBOOT_KEYED
> >      CONFIG_AUTOBOOT_STOP_STR_SHA256
> >
> > Signed-off-by: Stefan Roese <sr@denx.de>
>
> This is certainly interesting but I think brings us back to a point
> Simon made a long while back about needing to factor out this code
> better.  Especially since this adds big long #if-#else-#endif blocks.
> Can we re-do this so at least have some functions be called out instead?
> Thanks!
>

Also if these CONFIG options are in Kconfig (as they should be) then we can use

if (IS_ENABLED(CONFIG_AUTOBOOT_STOP_STR_SHA256))

instead of #ifdef which may improve the code.

Regards,
Simon

[U-Boot] [PATCH] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password

[Stefan Roese]

Hi Simon,

On 13.03.2015 03:48, Simon Glass wrote:
>>> This patch adds the feature to only stop the autobooting, and therefor
>>> boot into the U-Boot prompt, when the input string / password matches
>>> a values that is encypted via a SHA256 hash and saved in the environment.
>>>
>>> This feature is enabled by defined these config options:
>>>       CONFIG_AUTOBOOT_KEYED
>>>       CONFIG_AUTOBOOT_STOP_STR_SHA256
>>>
>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>
>> This is certainly interesting but I think brings us back to a point
>> Simon made a long while back about needing to factor out this code
>> better.  Especially since this adds big long #if-#else-#endif blocks.
>> Can we re-do this so at least have some functions be called out instead?
>> Thanks!
>>
>
> Also if these CONFIG options are in Kconfig (as they should be) then we can use
>
> if (IS_ENABLED(CONFIG_AUTOBOOT_STOP_STR_SHA256))
>
> instead of #ifdef which may improve the code.

Right. I also thought about this. But the resulting code has all the 
functionality extracted into 2 functions:

#if defined(CONFIG_AUTOBOOT_STOP_STR_SHA256)
static int passwd_abort(uint64_t etime)
{
	const char *sha_env_str = getenv("bootstopkeysha256");
	...
}
#else
static int passwd_abort(uint64_t etime)
{
	int abort = 0;
	...
}
#endif

And this function is now called unconditionally:

	...
	abort = passwd_abort(etime);

So there is nothing here that could be simplified by using IS_ENABLED().

I could of course just add this new config option to Kconfig. But with 
all the other related options not in Kconfig (CONFIG_AUTOBOOT_KEYED, 
CONFIG_AUTOBOOT_DELAY_STR...), this doesn't make much sense. So at some 
point all those config options should be moved to Kconfig. Unfortunately 
I don't have the time for this right now. But I'll add it to my list to 
do this at a later time.

Thanks,
Stefan

[U-Boot] bootcount: Add dcache flush to bootcount_store()

[Tom Rini]

On Wed, Mar 11, 2015 at 09:51:38AM +0100, Stefan Roese wrote:

> Without this dcache_flush the updated bootcounter may not be saved to
> its location.
> 
> This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
> as bootcounter storage area. And issuing "reset" from within U-Boot
> cause the bootcounter to stay on its initial value.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Tom Rini <trini@konsulko.com>

OK, this breaks some platforms:
   powerpc:  +   TQM850L
+(TQM850L) drivers/built-in.o: In function `bootcount_store':
+(TQM850L) build/../drivers/bootcount/bootcount.c:64: undefined reference to `flush_dca che_range'
+(TQM850L) make[1]: *** [u-boot] Error 1
+(TQM850L) make: *** [sub-make] Error 2

We'll see how many others have the same problem soon and then I'll
decide on nuking the old platforms of holding off on this change.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150313/4d658a71/attachment.sig>

[U-Boot] bootcount: Add dcache flush to bootcount_store()

[Tom Rini]

On Fri, Mar 13, 2015 at 09:48:56AM -0400, Tom Rini wrote:
> On Wed, Mar 11, 2015 at 09:51:38AM +0100, Stefan Roese wrote:
> 
> > Without this dcache_flush the updated bootcounter may not be saved to
> > its location.
> > 
> > This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
> > as bootcounter storage area. And issuing "reset" from within U-Boot
> > cause the bootcounter to stay on its initial value.
> > 
> > Signed-off-by: Stefan Roese <sr@denx.de>
> > Reviewed-by: Tom Rini <trini@konsulko.com>
> 
> OK, this breaks some platforms:
>    powerpc:  +   TQM850L
> +(TQM850L) drivers/built-in.o: In function `bootcount_store':
> +(TQM850L) build/../drivers/bootcount/bootcount.c:64: undefined reference to `flush_dcache_range'
> +(TQM850L) make[1]: *** [u-boot] Error 1
> +(TQM850L) make: *** [sub-make] Error 2
> 
> We'll see how many others have the same problem soon and then I'll
> decide on nuking the old platforms of holding off on this change.

Aside from the TQM 8xx family that Wolfgang owns we have mgcoge and
mgcoge3ne also breaking from this
(http://patchwork.ozlabs.org/patch/448849/) change.  Wolfgang, Holger,
how do you want to proceed?  We either need cache operations or dropping
bootcount from the platforms or dropping the platforms.

Frankly, after looking at most of arch/powerpc/cpu/*/cache.c I suspect
8xx and 83xx just need the dummy files copied over.

Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150313/af439248/attachment.sig>

[U-Boot] bootcount: Add dcache flush to bootcount_store()

[Tom Rini]

... Add a few more PowerPC people.

On Fri, Mar 13, 2015 at 10:34:03AM -0400, Tom Rini wrote:
> On Fri, Mar 13, 2015 at 09:48:56AM -0400, Tom Rini wrote:
> > On Wed, Mar 11, 2015 at 09:51:38AM +0100, Stefan Roese wrote:
> > 
> > > Without this dcache_flush the updated bootcounter may not be saved to
> > > its location.
> > > 
> > > This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
> > > as bootcounter storage area. And issuing "reset" from within U-Boot
> > > cause the bootcounter to stay on its initial value.
> > > 
> > > Signed-off-by: Stefan Roese <sr@denx.de>
> > > Reviewed-by: Tom Rini <trini@konsulko.com>
> > 
> > OK, this breaks some platforms:
> >    powerpc:  +   TQM850L
> > +(TQM850L) drivers/built-in.o: In function `bootcount_store':
> > +(TQM850L) build/../drivers/bootcount/bootcount.c:64: undefined reference to `flush_dcache_range'
> > +(TQM850L) make[1]: *** [u-boot] Error 1
> > +(TQM850L) make: *** [sub-make] Error 2
> > 
> > We'll see how many others have the same problem soon and then I'll
> > decide on nuking the old platforms of holding off on this change.
> 
> Aside from the TQM 8xx family that Wolfgang owns we have mgcoge and
> mgcoge3ne also breaking from this
> (http://patchwork.ozlabs.org/patch/448849/) change.  Wolfgang, Holger,
> how do you want to proceed?  We either need cache operations or dropping
> bootcount from the platforms or dropping the platforms.
> 
> Frankly, after looking at most of arch/powerpc/cpu/*/cache.c I suspect
> 8xx and 83xx just need the dummy files copied over.

OK, I went and poked at going one direction on this and then dug into
the higher level problem more.  PowerPC _needs_ the current kernel's
arch/powerpc/kernel/misc_32.S relevant cache functions ported over for
everyone and we should kill the dummy functions we have today.  To try
and encourage some folks to do this I'm going to drop this bootcount
patch for now.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150315/b841309b/attachment.sig>

[U-Boot] bootcount: Add dcache flush to bootcount_store()

[York Sun]

Tom,

On 03/15/2015 11:30 AM, Tom Rini wrote:
> ... Add a few more PowerPC people.
> 
> On Fri, Mar 13, 2015 at 10:34:03AM -0400, Tom Rini wrote:
>> On Fri, Mar 13, 2015 at 09:48:56AM -0400, Tom Rini wrote:
>>> On Wed, Mar 11, 2015 at 09:51:38AM +0100, Stefan Roese wrote:
>>>
>>>> Without this dcache_flush the updated bootcounter may not be saved to
>>>> its location.
>>>>
>>>> This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
>>>> as bootcounter storage area. And issuing "reset" from within U-Boot
>>>> cause the bootcounter to stay on its initial value.
>>>>
>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>>
>>> OK, this breaks some platforms:
>>>    powerpc:  +   TQM850L
>>> +(TQM850L) drivers/built-in.o: In function `bootcount_store':
>>> +(TQM850L) build/../drivers/bootcount/bootcount.c:64: undefined reference to `flush_dcache_range'
>>> +(TQM850L) make[1]: *** [u-boot] Error 1
>>> +(TQM850L) make: *** [sub-make] Error 2
>>>
>>> We'll see how many others have the same problem soon and then I'll
>>> decide on nuking the old platforms of holding off on this change.
>>
>> Aside from the TQM 8xx family that Wolfgang owns we have mgcoge and
>> mgcoge3ne also breaking from this
>> (http://patchwork.ozlabs.org/patch/448849/) change.  Wolfgang, Holger,
>> how do you want to proceed?  We either need cache operations or dropping
>> bootcount from the platforms or dropping the platforms.
>>
>> Frankly, after looking at most of arch/powerpc/cpu/*/cache.c I suspect
>> 8xx and 83xx just need the dummy files copied over.
> 
> OK, I went and poked at going one direction on this and then dug into
> the higher level problem more.  PowerPC _needs_ the current kernel's
> arch/powerpc/kernel/misc_32.S relevant cache functions ported over for
> everyone and we should kill the dummy functions we have today.  To try
> and encourage some folks to do this I'm going to drop this bootcount
> patch for now.
> 

FWIW, powerpc mpc83xx, mpc85xx, mpc86xx all have flush_dcache_range() function
defined.

York

[U-Boot] bootcount: Add dcache flush to bootcount_store()

[Tom Rini]

On Mon, Mar 16, 2015 at 08:57:02AM -0700, York Sun wrote:
> Tom,
> 
> On 03/15/2015 11:30 AM, Tom Rini wrote:
> > ... Add a few more PowerPC people.
> > 
> > On Fri, Mar 13, 2015 at 10:34:03AM -0400, Tom Rini wrote:
> >> On Fri, Mar 13, 2015 at 09:48:56AM -0400, Tom Rini wrote:
> >>> On Wed, Mar 11, 2015 at 09:51:38AM +0100, Stefan Roese wrote:
> >>>
> >>>> Without this dcache_flush the updated bootcounter may not be saved to
> >>>> its location.
> >>>>
> >>>> This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
> >>>> as bootcounter storage area. And issuing "reset" from within U-Boot
> >>>> cause the bootcounter to stay on its initial value.
> >>>>
> >>>> Signed-off-by: Stefan Roese <sr@denx.de>
> >>>> Reviewed-by: Tom Rini <trini@konsulko.com>
> >>>
> >>> OK, this breaks some platforms:
> >>>    powerpc:  +   TQM850L
> >>> +(TQM850L) drivers/built-in.o: In function `bootcount_store':
> >>> +(TQM850L) build/../drivers/bootcount/bootcount.c:64: undefined reference to `flush_dcache_range'
> >>> +(TQM850L) make[1]: *** [u-boot] Error 1
> >>> +(TQM850L) make: *** [sub-make] Error 2
> >>>
> >>> We'll see how many others have the same problem soon and then I'll
> >>> decide on nuking the old platforms of holding off on this change.
> >>
> >> Aside from the TQM 8xx family that Wolfgang owns we have mgcoge and
> >> mgcoge3ne also breaking from this
> >> (http://patchwork.ozlabs.org/patch/448849/) change.  Wolfgang, Holger,
> >> how do you want to proceed?  We either need cache operations or dropping
> >> bootcount from the platforms or dropping the platforms.
> >>
> >> Frankly, after looking at most of arch/powerpc/cpu/*/cache.c I suspect
> >> 8xx and 83xx just need the dummy files copied over.
> > 
> > OK, I went and poked at going one direction on this and then dug into
> > the higher level problem more.  PowerPC _needs_ the current kernel's
> > arch/powerpc/kernel/misc_32.S relevant cache functions ported over for
> > everyone and we should kill the dummy functions we have today.  To try
> > and encourage some folks to do this I'm going to drop this bootcount
> > patch for now.
> > 
> 
> FWIW, powerpc mpc83xx, mpc85xx, mpc86xx all have flush_dcache_range() function
> defined.

Yes and no:
$ git grep -l flush_dcache_range arch/powerpc/
arch/powerpc/cpu/mpc512x/cache.c
arch/powerpc/cpu/mpc5xxx/cache.c
arch/powerpc/cpu/mpc83xx/cache.c
arch/powerpc/cpu/mpc85xx/cache.c
arch/powerpc/cpu/mpc86xx/cache.S
arch/powerpc/cpu/ppc4xx/cache.S

Of these only ppc4xx and mpc86xx are real functions, borrowed from the
kernel long long ago.  The rest are dummy functions.  And we should
instead make everyone have the same real functions the kernel does :)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150316/6300d4b8/attachment.sig>

[U-Boot] bootcount: Add dcache flush to bootcount_store()

[York Sun]

On 03/16/2015 10:05 AM, Tom Rini wrote:
>>>> 8xx and 83xx just need the dummy files copied over.
>>>
>>> OK, I went and poked at going one direction on this and then dug into
>>> the higher level problem more.  PowerPC _needs_ the current kernel's
>>> arch/powerpc/kernel/misc_32.S relevant cache functions ported over for
>>> everyone and we should kill the dummy functions we have today.  To try
>>> and encourage some folks to do this I'm going to drop this bootcount
>>> patch for now.
>>>
>>
>> FWIW, powerpc mpc83xx, mpc85xx, mpc86xx all have flush_dcache_range() function
>> defined.
> 
> Yes and no:
> $ git grep -l flush_dcache_range arch/powerpc/
> arch/powerpc/cpu/mpc512x/cache.c
> arch/powerpc/cpu/mpc5xxx/cache.c
> arch/powerpc/cpu/mpc83xx/cache.c
> arch/powerpc/cpu/mpc85xx/cache.c
> arch/powerpc/cpu/mpc86xx/cache.S
> arch/powerpc/cpu/ppc4xx/cache.S
> 
> Of these only ppc4xx and mpc86xx are real functions, borrowed from the
> kernel long long ago.  The rest are dummy functions.  And we should
> instead make everyone have the same real functions the kernel does :)
> 

Ah! I was under the impression we use it. But we actually used flush_cache()
function.

York

[U-Boot] bootcount: Add dcache flush to bootcount_store()

[Tom Rini]

On Mon, Mar 16, 2015 at 10:11:17AM -0700, York Sun wrote:
> 
> 
> On 03/16/2015 10:05 AM, Tom Rini wrote:
> >>>> 8xx and 83xx just need the dummy files copied over.
> >>>
> >>> OK, I went and poked at going one direction on this and then dug into
> >>> the higher level problem more.  PowerPC _needs_ the current kernel's
> >>> arch/powerpc/kernel/misc_32.S relevant cache functions ported over for
> >>> everyone and we should kill the dummy functions we have today.  To try
> >>> and encourage some folks to do this I'm going to drop this bootcount
> >>> patch for now.
> >>>
> >>
> >> FWIW, powerpc mpc83xx, mpc85xx, mpc86xx all have flush_dcache_range() function
> >> defined.
> > 
> > Yes and no:
> > $ git grep -l flush_dcache_range arch/powerpc/
> > arch/powerpc/cpu/mpc512x/cache.c
> > arch/powerpc/cpu/mpc5xxx/cache.c
> > arch/powerpc/cpu/mpc83xx/cache.c
> > arch/powerpc/cpu/mpc85xx/cache.c
> > arch/powerpc/cpu/mpc86xx/cache.S
> > arch/powerpc/cpu/ppc4xx/cache.S
> > 
> > Of these only ppc4xx and mpc86xx are real functions, borrowed from the
> > kernel long long ago.  The rest are dummy functions.  And we should
> > instead make everyone have the same real functions the kernel does :)
> 
> Ah! I was under the impression we use it. But we actually used flush_cache()
> function.

Yeah, there's some related cleanup that should be done too :)

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150316/29f32fa5/attachment.sig>

[U-Boot] bootcount: Add dcache flush to bootcount_store()

[Holger Brunck]

Hi Tom,

On 03/13/2015 03:34 PM, Tom Rini wrote:
> On Fri, Mar 13, 2015 at 09:48:56AM -0400, Tom Rini wrote:
>> On Wed, Mar 11, 2015 at 09:51:38AM +0100, Stefan Roese wrote:
>>
>>> Without this dcache_flush the updated bootcounter may not be saved to
>>> its location.
>>>
>>> This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
>>> as bootcounter storage area. And issuing "reset" from within U-Boot
>>> cause the bootcounter to stay on its initial value.
>>>
>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>
>> OK, this breaks some platforms:
>>    powerpc:  +   TQM850L
>> +(TQM850L) drivers/built-in.o: In function `bootcount_store':
>> +(TQM850L) build/../drivers/bootcount/bootcount.c:64: undefined reference to `flush_dcache_range'
>> +(TQM850L) make[1]: *** [u-boot] Error 1
>> +(TQM850L) make: *** [sub-make] Error 2
>>
>> We'll see how many others have the same problem soon and then I'll
>> decide on nuking the old platforms of holding off on this change.
> 
> Aside from the TQM 8xx family that Wolfgang owns we have mgcoge and
> mgcoge3ne also breaking from this
> (http://patchwork.ozlabs.org/patch/448849/) change.  Wolfgang, Holger,
> how do you want to proceed?  We either need cache operations or dropping
> bootcount from the platforms or dropping the platforms.
> 

we still would like to keep mgcoge and mgcoge3ne support. These boards are still
in maintenance. Unfortunately this week we are very busy. Next week Valentin or
myself have planned to find some time to look at this.

Regards
Holger

[U-Boot] [PATCH] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password

[Simon Glass]

Hi Stefan,

On 13 March 2015 at 01:15, Stefan Roese <sr@denx.de> wrote:
> Hi Simon,
>
> On 13.03.2015 03:48, Simon Glass wrote:
>>>>
>>>> This patch adds the feature to only stop the autobooting, and therefor
>>>> boot into the U-Boot prompt, when the input string / password matches
>>>> a values that is encypted via a SHA256 hash and saved in the
>>>> environment.
>>>>
>>>> This feature is enabled by defined these config options:
>>>>       CONFIG_AUTOBOOT_KEYED
>>>>       CONFIG_AUTOBOOT_STOP_STR_SHA256
>>>>
>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>
>>>
>>> This is certainly interesting but I think brings us back to a point
>>> Simon made a long while back about needing to factor out this code
>>> better.  Especially since this adds big long #if-#else-#endif blocks.
>>> Can we re-do this so at least have some functions be called out instead?
>>> Thanks!
>>>
>>
>> Also if these CONFIG options are in Kconfig (as they should be) then we
>> can use
>>
>> if (IS_ENABLED(CONFIG_AUTOBOOT_STOP_STR_SHA256))
>>
>> instead of #ifdef which may improve the code.
>
>
> Right. I also thought about this. But the resulting code has all the
> functionality extracted into 2 functions:
>
> #if defined(CONFIG_AUTOBOOT_STOP_STR_SHA256)
> static int passwd_abort(uint64_t etime)
> {
>         const char *sha_env_str = getenv("bootstopkeysha256");
>         ...
> }
> #else
> static int passwd_abort(uint64_t etime)
> {
>         int abort = 0;
>         ...
> }
> #endif
>
> And this function is now called unconditionally:
>
>         ...
>         abort = passwd_abort(etime);
>
> So there is nothing here that could be simplified by using IS_ENABLED().
>
> I could of course just add this new config option to Kconfig. But with all
> the other related options not in Kconfig (CONFIG_AUTOBOOT_KEYED,
> CONFIG_AUTOBOOT_DELAY_STR...), this doesn't make much sense. So at some
> point all those config options should be moved to Kconfig. Unfortunately I
> don't have the time for this right now. But I'll add it to my list to do
> this at a later time.

Well rather than adding more options, perhaps we should wait until we
get this moved to Kconfig? It's not going to get any easier :-)

Regards,
Simon

[U-Boot] bootcount: Add dcache flush to bootcount_store()

[Stefan Roese]

Hi!

(added a few more people to the Cc list)

On 17.03.2015 10:00, Holger Brunck wrote:
> Hi Tom,
> 
> On 03/13/2015 03:34 PM, Tom Rini wrote:
>> On Fri, Mar 13, 2015 at 09:48:56AM -0400, Tom Rini wrote:
>>> On Wed, Mar 11, 2015 at 09:51:38AM +0100, Stefan Roese wrote:
>>>
>>>> Without this dcache_flush the updated bootcounter may not be saved to
>>>> its location.
>>>>
>>>> This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
>>>> as bootcounter storage area. And issuing "reset" from within U-Boot
>>>> cause the bootcounter to stay on its initial value.
>>>>
>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>>
>>> OK, this breaks some platforms:
>>>     powerpc:  +   TQM850L
>>> +(TQM850L) drivers/built-in.o: In function `bootcount_store':
>>> +(TQM850L) build/../drivers/bootcount/bootcount.c:64: undefined reference to `flush_dcache_range'
>>> +(TQM850L) make[1]: *** [u-boot] Error 1
>>> +(TQM850L) make: *** [sub-make] Error 2
>>>
>>> We'll see how many others have the same problem soon and then I'll
>>> decide on nuking the old platforms of holding off on this change.
>>
>> Aside from the TQM 8xx family that Wolfgang owns we have mgcoge and
>> mgcoge3ne also breaking from this
>> (http://patchwork.ozlabs.org/patch/448849/) change.  Wolfgang, Holger,
>> how do you want to proceed?  We either need cache operations or dropping
>> bootcount from the platforms or dropping the platforms.
>>
> 
> we still would like to keep mgcoge and mgcoge3ne support. These boards are still
> in maintenance. Unfortunately this week we are very busy. Next week Valentin or
> myself have planned to find some time to look at this.

I just yesterday noticed this code for mx6:

------------------------- arch/arm/cpu/armv7/mx6/soc.c -------------------------
void enable_caches(void)
{
	...
	/* Enable caching on OCRAM and ROM */
	mmu_set_region_dcache_behaviour(ROMCP_ARB_BASE_ADDR,
					ROMCP_ARB_END_ADDR,
					option);
	mmu_set_region_dcache_behaviour(IRAM_BASE_ADDR,
					IRAM_SIZE,
					option);

So we definitely have the dcache enabled on mx6 in the OCRAM. And this
of course explains, why I need the cache flush operations in the
bootcounter code.

I'm not really sure if we want this area to be cached though. This got
introduced with this patch:

Author: Nitin Garg <nitin.garg@freescale.com>  2014-09-16 20:33:25
Committer: Stefano Babic <sbabic@denx.de>  2014-09-22 16:21:04

    imx: Support i.MX6 High Assurance Boot authentication
    
    When CONFIG_SECURE_BOOT is enabled, the signed images
    like kernel and dtb can be authenticated using iMX6 CAAM.
    The added command hab_auth_img can be used for HAB
    authentication of images. The command takes the image
    DDR location, IVT (Image Vector Table) offset inside
    image as parameters. Detailed info about signing images
    can be found in Freescale AppNote AN4581.
    
    Signed-off-by: Nitin Garg <nitin.garg@freescale.com>

The cache stuff is not mentioned here in this commit log.
Nitin, why did you enable the cache here? Performance reason
only?

If the OCRAM was uncached (again), we could really drop my patch
(this mail thread) with those flush calls. So how should we proceed?
Make this OCRAM area uncached again?

Comments welcome...

Thanks,
Stefan

BTW: I'm on vacation now until 8th April, so please don't expect
any replies in this time.

[U-Boot] bootcount: Add dcache flush to bootcount_store()

[Nitin Garg]

Hi Stefan,
On 03/27/2015 08:18 AM, Stefan Roese wrote:
> Hi!
> 
> (added a few more people to the Cc list)
> 
> On 17.03.2015 10:00, Holger Brunck wrote:
>> Hi Tom,
>>
>> On 03/13/2015 03:34 PM, Tom Rini wrote:
>>> On Fri, Mar 13, 2015 at 09:48:56AM -0400, Tom Rini wrote:
>>>> On Wed, Mar 11, 2015 at 09:51:38AM +0100, Stefan Roese wrote:
>>>>
>>>>> Without this dcache_flush the updated bootcounter may not be saved to
>>>>> its location.
>>>>>
>>>>> This was detected on an iMX.6 platform using the OCRAM (internal SRAM)
>>>>> as bootcounter storage area. And issuing "reset" from within U-Boot
>>>>> cause the bootcounter to stay on its initial value.
>>>>>
>>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>>>
>>>> OK, this breaks some platforms:
>>>>     powerpc:  +   TQM850L
>>>> +(TQM850L) drivers/built-in.o: In function `bootcount_store':
>>>> +(TQM850L) build/../drivers/bootcount/bootcount.c:64: undefined reference to `flush_dcache_range'
>>>> +(TQM850L) make[1]: *** [u-boot] Error 1
>>>> +(TQM850L) make: *** [sub-make] Error 2
>>>>
>>>> We'll see how many others have the same problem soon and then I'll
>>>> decide on nuking the old platforms of holding off on this change.
>>>
>>> Aside from the TQM 8xx family that Wolfgang owns we have mgcoge and
>>> mgcoge3ne also breaking from this
>>> (http://patchwork.ozlabs.org/patch/448849/) change.  Wolfgang, Holger,
>>> how do you want to proceed?  We either need cache operations or dropping
>>> bootcount from the platforms or dropping the platforms.
>>>
>>
>> we still would like to keep mgcoge and mgcoge3ne support. These boards are still
>> in maintenance. Unfortunately this week we are very busy. Next week Valentin or
>> myself have planned to find some time to look at this.
> 
> I just yesterday noticed this code for mx6:
> 
> ------------------------- arch/arm/cpu/armv7/mx6/soc.c -------------------------
> void enable_caches(void)
> {
> 	...
> 	/* Enable caching on OCRAM and ROM */
> 	mmu_set_region_dcache_behaviour(ROMCP_ARB_BASE_ADDR,
> 					ROMCP_ARB_END_ADDR,
> 					option);
> 	mmu_set_region_dcache_behaviour(IRAM_BASE_ADDR,
> 					IRAM_SIZE,
> 					option);
> 
> So we definitely have the dcache enabled on mx6 in the OCRAM. And this
> of course explains, why I need the cache flush operations in the
> bootcounter code.
> 
> I'm not really sure if we want this area to be cached though. This got
> introduced with this patch:
> 
> Author: Nitin Garg <nitin.garg@freescale.com>  2014-09-16 20:33:25
> Committer: Stefano Babic <sbabic@denx.de>  2014-09-22 16:21:04
> 
>     imx: Support i.MX6 High Assurance Boot authentication
>     
>     When CONFIG_SECURE_BOOT is enabled, the signed images
>     like kernel and dtb can be authenticated using iMX6 CAAM.
>     The added command hab_auth_img can be used for HAB
>     authentication of images. The command takes the image
>     DDR location, IVT (Image Vector Table) offset inside
>     image as parameters. Detailed info about signing images
>     can be found in Freescale AppNote AN4581.
>     
>     Signed-off-by: Nitin Garg <nitin.garg@freescale.com>
> 
> The cache stuff is not mentioned here in this commit log.
> Nitin, why did you enable the cache here? Performance reason
> only?
Seems like I missed mentioning this in the commit log, sorry.
We enabled the OCRAM d-cache since it helped HAB to
execute faster when authenticating OS image. The time taken 
to complete HAB improved by 3 times with d-cache enabled
for ROM and OCRAM. This is because the HAB uses various
regions of the OCRAM for data.

> 
> If the OCRAM was uncached (again), we could really drop my patch
> (this mail thread) with those flush calls. So how should we proceed?
> Make this OCRAM area uncached again?
> 
> Comments welcome...
> 
> Thanks,
> Stefan
> 
> BTW: I'm on vacation now until 8th April, so please don't expect
> any replies in this time.
> 
Regards,
Nitin

[U-Boot] [RFC] powerpc: add 2 common dcache assembly functions

[Valentin Longchamp]

This patch defines the 2 flush_dcache_range and invalidate_dcache_range
functions for all the powerpc architecture. Their implementation is
borrowed from the kernel's misc_32.S file and replace the ones from
mpc86xx and ppc4xx since they were equivalent.

This is a fix for the problem introduced by this patch:
http://patchwork.ozlabs.org/patch/448849/

Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
---
 arch/powerpc/cpu/mpc512x/Makefile |  3 ---
 arch/powerpc/cpu/mpc512x/cache.c  | 17 --------------
 arch/powerpc/cpu/mpc5xxx/Makefile |  1 -
 arch/powerpc/cpu/mpc5xxx/cache.c  | 15 ------------
 arch/powerpc/cpu/mpc83xx/Makefile |  3 ---
 arch/powerpc/cpu/mpc83xx/cache.c  | 17 --------------
 arch/powerpc/cpu/mpc85xx/Makefile |  3 ---
 arch/powerpc/cpu/mpc85xx/cache.c  | 17 --------------
 arch/powerpc/cpu/mpc86xx/cache.S  | 45 ------------------------------------
 arch/powerpc/cpu/ppc4xx/cache.S   | 43 -----------------------------------
 arch/powerpc/lib/ppccache.S       | 48 +++++++++++++++++++++++++++++++++++++++
 11 files changed, 48 insertions(+), 164 deletions(-)
 delete mode 100644 arch/powerpc/cpu/mpc512x/cache.c
 delete mode 100644 arch/powerpc/cpu/mpc5xxx/cache.c
 delete mode 100644 arch/powerpc/cpu/mpc83xx/cache.c
 delete mode 100644 arch/powerpc/cpu/mpc85xx/cache.c

diff --git a/arch/powerpc/cpu/mpc512x/Makefile b/arch/powerpc/cpu/mpc512x/Makefile
index a4934ef..98991c6 100644
--- a/arch/powerpc/cpu/mpc512x/Makefile
+++ b/arch/powerpc/cpu/mpc512x/Makefile
@@ -17,6 +17,3 @@ obj-y += speed.o
 obj-$(CONFIG_FSL_DIU_FB) += diu.o
 obj-$(CONFIG_CMD_IDE) += ide.o
 obj-$(CONFIG_PCI) += pci.o
-
-# Stub implementations of cache management functions for USB
-obj-$(CONFIG_USB_EHCI) += cache.o
diff --git a/arch/powerpc/cpu/mpc512x/cache.c b/arch/powerpc/cpu/mpc512x/cache.c
deleted file mode 100644
index 66384f9..0000000
--- a/arch/powerpc/cpu/mpc512x/cache.c
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * Copyright (C) 2012 Marek Vasut <marex@denx.de>
- *
- * This file contains stub implementation of
- *   invalidate_dcache_range()
- *   flush_dcache_range()
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-void invalidate_dcache_range(unsigned long start, unsigned long stop)
-{
-}
-
-void flush_dcache_range(unsigned long start, unsigned long stop)
-{
-}
diff --git a/arch/powerpc/cpu/mpc5xxx/Makefile b/arch/powerpc/cpu/mpc5xxx/Makefile
index d122b29..5c67e1d 100644
--- a/arch/powerpc/cpu/mpc5xxx/Makefile
+++ b/arch/powerpc/cpu/mpc5xxx/Makefile
@@ -7,7 +7,6 @@
 
 extra-y	= start.o
 extra-y += traps.o
-obj-y  += cache.o
 obj-y  += io.o
 obj-y  += firmware_sc_task_bestcomm.impl.o
 obj-y += i2c.o
diff --git a/arch/powerpc/cpu/mpc5xxx/cache.c b/arch/powerpc/cpu/mpc5xxx/cache.c
deleted file mode 100644
index 5d674bc..0000000
--- a/arch/powerpc/cpu/mpc5xxx/cache.c
+++ /dev/null
@@ -1,15 +0,0 @@
-/*
- * This file contains stub implementation of
- *   invalidate_dcache_range()
- *   flush_dcache_range()
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-void invalidate_dcache_range(unsigned long start, unsigned long stop)
-{
-}
-
-void flush_dcache_range(unsigned long start, unsigned long stop)
-{
-}
diff --git a/arch/powerpc/cpu/mpc83xx/Makefile b/arch/powerpc/cpu/mpc83xx/Makefile
index cf91162..a93cf13 100644
--- a/arch/powerpc/cpu/mpc83xx/Makefile
+++ b/arch/powerpc/cpu/mpc83xx/Makefile
@@ -35,9 +35,6 @@ obj-$(CONFIG_PCI) += pci.o
 obj-$(CONFIG_PCIE) += pcie.o
 obj-$(CONFIG_OF_LIBFDT) += fdt.o
 
-# Stub implementations of cache management functions for USB
-obj-y += cache.o
-
 ifndef CONFIG_SYS_FSL_DDRC_GEN2
 obj-y += spd_sdram.o
 endif
diff --git a/arch/powerpc/cpu/mpc83xx/cache.c b/arch/powerpc/cpu/mpc83xx/cache.c
deleted file mode 100644
index 66384f9..0000000
--- a/arch/powerpc/cpu/mpc83xx/cache.c
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * Copyright (C) 2012 Marek Vasut <marex@denx.de>
- *
- * This file contains stub implementation of
- *   invalidate_dcache_range()
- *   flush_dcache_range()
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-void invalidate_dcache_range(unsigned long start, unsigned long stop)
-{
-}
-
-void flush_dcache_range(unsigned long start, unsigned long stop)
-{
-}
diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile
index b93158b..65c26c0 100644
--- a/arch/powerpc/cpu/mpc85xx/Makefile
+++ b/arch/powerpc/cpu/mpc85xx/Makefile
@@ -114,7 +114,4 @@ endif
 obj-y	+= tlb.o
 obj-y	+= traps.o
 
-# Stub implementations of cache management functions for USB
-obj-y += cache.o
-
 endif # not minimal
diff --git a/arch/powerpc/cpu/mpc85xx/cache.c b/arch/powerpc/cpu/mpc85xx/cache.c
deleted file mode 100644
index 66384f9..0000000
--- a/arch/powerpc/cpu/mpc85xx/cache.c
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * Copyright (C) 2012 Marek Vasut <marex@denx.de>
- *
- * This file contains stub implementation of
- *   invalidate_dcache_range()
- *   flush_dcache_range()
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-void invalidate_dcache_range(unsigned long start, unsigned long stop)
-{
-}
-
-void flush_dcache_range(unsigned long start, unsigned long stop)
-{
-}
diff --git a/arch/powerpc/cpu/mpc86xx/cache.S b/arch/powerpc/cpu/mpc86xx/cache.S
index 536d9b9..34968c6 100644
--- a/arch/powerpc/cpu/mpc86xx/cache.S
+++ b/arch/powerpc/cpu/mpc86xx/cache.S
@@ -115,51 +115,6 @@ _GLOBAL(clean_dcache_range)
 	blr
 
 /*
- * Write any modified data cache blocks out to memory
- * and invalidate the corresponding instruction cache blocks.
- *
- * flush_dcache_range(unsigned long start, unsigned long stop)
- */
-_GLOBAL(flush_dcache_range)
-	li	r5,CACHE_LINE_SIZE-1
-	andc	r3,r3,r5
-	subf	r4,r3,r4
-	add	r4,r4,r5
-	srwi.	r4,r4,LG_CACHE_LINE_SIZE
-	beqlr
-	mtctr	r4
-
-	sync
-1:	dcbf	0,r3
-	addi	r3,r3,CACHE_LINE_SIZE
-	bdnz	1b
-	sync				/* wait for dcbf's to get to ram */
-	blr
-
-/*
- * Like above, but invalidate the D-cache.  This is used by the 8xx
- * to invalidate the cache so the PPC core doesn't get stale data
- * from the CPM (no cache snooping here :-).
- *
- * invalidate_dcache_range(unsigned long start, unsigned long stop)
- */
-_GLOBAL(invalidate_dcache_range)
-	li	r5,CACHE_LINE_SIZE-1
-	andc	r3,r3,r5
-	subf	r4,r3,r4
-	add	r4,r4,r5
-	srwi.	r4,r4,LG_CACHE_LINE_SIZE
-	beqlr
-	mtctr	r4
-
-	sync
-1:	dcbi	0,r3
-	addi	r3,r3,CACHE_LINE_SIZE
-	bdnz	1b
-	sync				/* wait for dcbi's to get to ram */
-	blr
-
-/*
  * Flush a particular page from the data cache to RAM.
  * Note: this is necessary because the instruction cache does *not*
  * snoop from the data cache.
diff --git a/arch/powerpc/cpu/ppc4xx/cache.S b/arch/powerpc/cpu/ppc4xx/cache.S
index 2714c2f..93e8366 100644
--- a/arch/powerpc/cpu/ppc4xx/cache.S
+++ b/arch/powerpc/cpu/ppc4xx/cache.S
@@ -74,49 +74,6 @@ _GLOBAL(clean_dcache_range)
 	blr
 
 /*
- * Write any modified data cache blocks out to memory and invalidate them.
- * Does not invalidate the corresponding instruction cache blocks.
- *
- * flush_dcache_range(unsigned long start, unsigned long stop)
- */
-_GLOBAL(flush_dcache_range)
-	li	r5,L1_CACHE_BYTES-1
-	andc	r3,r3,r5
-	subf	r4,r3,r4
-	add	r4,r4,r5
-	srwi.	r4,r4,L1_CACHE_SHIFT
-	beqlr
-	mtctr	r4
-
-1:	dcbf	0,r3
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	1b
-	sync				/* wait for dcbst's to get to ram */
-	blr
-
-/*
- * Like above, but invalidate the D-cache.  This is used by the 8xx
- * to invalidate the cache so the PPC core doesn't get stale data
- * from the CPM (no cache snooping here :-).
- *
- * invalidate_dcache_range(unsigned long start, unsigned long stop)
- */
-_GLOBAL(invalidate_dcache_range)
-	li	r5,L1_CACHE_BYTES-1
-	andc	r3,r3,r5
-	subf	r4,r3,r4
-	add	r4,r4,r5
-	srwi.	r4,r4,L1_CACHE_SHIFT
-	beqlr
-	mtctr	r4
-
-1:	dcbi	0,r3
-	addi	r3,r3,L1_CACHE_BYTES
-	bdnz	1b
-	sync				/* wait for dcbi's to get to ram */
-	blr
-
-/*
  * 40x cores have 8K or 16K dcache and 32 byte line size.
  * 44x has a 32K dcache and 32 byte line size.
  * 8xx has 1, 2, 4, 8K variants.
diff --git a/arch/powerpc/lib/ppccache.S b/arch/powerpc/lib/ppccache.S
index 349a1c1..b96dbc6 100644
--- a/arch/powerpc/lib/ppccache.S
+++ b/arch/powerpc/lib/ppccache.S
@@ -9,6 +9,9 @@
 
 #include <config.h>
 #include <ppc_asm.tmpl>
+#include <ppc_defs.h>
+
+#include <asm/cache.h>
 
 /*------------------------------------------------------------------------------- */
 /* Function:	 ppcDcbf */
@@ -54,3 +57,48 @@ ppcDcbz:
 ppcSync:
 	sync
 	blr
+
+/*
+ * Write any modified data cache blocks out to memory and invalidate them.
+ * Does not invalidate the corresponding instruction cache blocks.
+ *
+ * flush_dcache_range(unsigned long start, unsigned long stop)
+ */
+_GLOBAL(flush_dcache_range)
+	li	r5,L1_CACHE_BYTES-1
+	andc	r3,r3,r5
+	subf	r4,r3,r4
+	add	r4,r4,r5
+	srwi.	r4,r4,L1_CACHE_SHIFT
+	beqlr
+	mtctr	r4
+
+1:	dcbf	0,r3
+	addi	r3,r3,L1_CACHE_BYTES
+	bdnz	1b
+	sync				/* wait for dcbst's to get to ram */
+	blr
+
+/*
+ * Like above, but invalidate the D-cache.  This is used by the 8xx
+ * to invalidate the cache so the PPC core doesn't get stale data
+ * from the CPM (no cache snooping here :-).
+ *
+ * invalidate_dcache_range(unsigned long start, unsigned long stop)
+ */
+_GLOBAL(invalidate_dcache_range)
+	li	r5,L1_CACHE_BYTES-1
+	andc	r3,r3,r5
+	subf	r4,r3,r4
+	add	r4,r4,r5
+	srwi.	r4,r4,L1_CACHE_SHIFT
+	beqlr
+	mtctr	r4
+
+	sync
+1:	dcbi	0,r3
+	addi	r3,r3,L1_CACHE_BYTES
+	bdnz	1b
+	sync				/* wait for dcbi's to get to ram */
+	blr
+
-- 
1.8.0.1

[U-Boot] [RFC] powerpc: add 2 common dcache assembly functions

[Tom Rini]

On Fri, Mar 27, 2015 at 04:07:32PM +0100, Valentin Longchamp wrote:

> This patch defines the 2 flush_dcache_range and invalidate_dcache_range
> functions for all the powerpc architecture. Their implementation is
> borrowed from the kernel's misc_32.S file and replace the ones from
> mpc86xx and ppc4xx since they were equivalent.
> 
> This is a fix for the problem introduced by this patch:
> http://patchwork.ozlabs.org/patch/448849/
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150328/6756e2e1/attachment.sig>

[U-Boot] cmd_led: Extend led command to support blinking and more leds

[Tom Rini]

On Wed, Mar 11, 2015 at 09:51:39AM +0100, Stefan Roese wrote:

> This patch extends the U-Boot "led" command to support automatic blinking
> by setting a blink frequency in milliseconds. Additionally the number of
> supported LEDs is increased to 6 (0...5).
> 
> This will be used by the PCA9551 LED driver.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150423/23ded757/attachment.sig>

[U-Boot] [PATCH] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password

[Stefan Roese]

Hi Simon,

On 23.03.2015 21:28, Simon Glass wrote:
> Hi Stefan,
> 
> On 13 March 2015 at 01:15, Stefan Roese <sr@denx.de> wrote:
>> Hi Simon,
>>
>> On 13.03.2015 03:48, Simon Glass wrote:
>>>>>
>>>>> This patch adds the feature to only stop the autobooting, and therefor
>>>>> boot into the U-Boot prompt, when the input string / password matches
>>>>> a values that is encypted via a SHA256 hash and saved in the
>>>>> environment.
>>>>>
>>>>> This feature is enabled by defined these config options:
>>>>>        CONFIG_AUTOBOOT_KEYED
>>>>>        CONFIG_AUTOBOOT_STOP_STR_SHA256
>>>>>
>>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>>
>>>>
>>>> This is certainly interesting but I think brings us back to a point
>>>> Simon made a long while back about needing to factor out this code
>>>> better.  Especially since this adds big long #if-#else-#endif blocks.
>>>> Can we re-do this so at least have some functions be called out instead?
>>>> Thanks!
>>>>
>>>
>>> Also if these CONFIG options are in Kconfig (as they should be) then we
>>> can use
>>>
>>> if (IS_ENABLED(CONFIG_AUTOBOOT_STOP_STR_SHA256))
>>>
>>> instead of #ifdef which may improve the code.
>>
>>
>> Right. I also thought about this. But the resulting code has all the
>> functionality extracted into 2 functions:
>>
>> #if defined(CONFIG_AUTOBOOT_STOP_STR_SHA256)
>> static int passwd_abort(uint64_t etime)
>> {
>>          const char *sha_env_str = getenv("bootstopkeysha256");
>>          ...
>> }
>> #else
>> static int passwd_abort(uint64_t etime)
>> {
>>          int abort = 0;
>>          ...
>> }
>> #endif
>>
>> And this function is now called unconditionally:
>>
>>          ...
>>          abort = passwd_abort(etime);
>>
>> So there is nothing here that could be simplified by using IS_ENABLED().
>>
>> I could of course just add this new config option to Kconfig. But with all
>> the other related options not in Kconfig (CONFIG_AUTOBOOT_KEYED,
>> CONFIG_AUTOBOOT_DELAY_STR...), this doesn't make much sense. So at some
>> point all those config options should be moved to Kconfig. Unfortunately I
>> don't have the time for this right now. But I'll add it to my list to do
>> this at a later time.
> 
> Well rather than adding more options, perhaps we should wait until we
> get this moved to Kconfig? It's not going to get any easier :-)

Right. And now I'm finally back at this task. To get this encrypted
password support into mainline. With Kconfig support of course this
time. ;)

Unfortunately I'm hitting a problem while moving some of the "old"
macros to Kconfig. Especially some strings like CONFIG_AUTOBOOT_PROMPT.
Here how this looks in some config headers:

#define CONFIG_AUTOBOOT_PROMPT         "autoboot in %d seconds\n", bootdelay

This does not work, as Kconfig truncates the string after the 2nd
'"'. Escaping this '"' using '\' also doesn't seem to work. Do you
or Masahiro have some experience with this kind of Kconfig macro
transition?

Thanks,
Stefan

[U-Boot] [PATCH] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password

[Simon Glass]

Hi Stefan,

On 5 May 2015 at 09:06, Stefan Roese <sr@denx.de> wrote:
> Hi Simon,
>
> On 23.03.2015 21:28, Simon Glass wrote:
>> Hi Stefan,
>>
>> On 13 March 2015 at 01:15, Stefan Roese <sr@denx.de> wrote:
>>> Hi Simon,
>>>
>>> On 13.03.2015 03:48, Simon Glass wrote:
>>>>>>
>>>>>> This patch adds the feature to only stop the autobooting, and therefor
>>>>>> boot into the U-Boot prompt, when the input string / password matches
>>>>>> a values that is encypted via a SHA256 hash and saved in the
>>>>>> environment.
>>>>>>
>>>>>> This feature is enabled by defined these config options:
>>>>>>        CONFIG_AUTOBOOT_KEYED
>>>>>>        CONFIG_AUTOBOOT_STOP_STR_SHA256
>>>>>>
>>>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>>>
>>>>>
>>>>> This is certainly interesting but I think brings us back to a point
>>>>> Simon made a long while back about needing to factor out this code
>>>>> better.  Especially since this adds big long #if-#else-#endif blocks.
>>>>> Can we re-do this so at least have some functions be called out instead?
>>>>> Thanks!
>>>>>
>>>>
>>>> Also if these CONFIG options are in Kconfig (as they should be) then we
>>>> can use
>>>>
>>>> if (IS_ENABLED(CONFIG_AUTOBOOT_STOP_STR_SHA256))
>>>>
>>>> instead of #ifdef which may improve the code.
>>>
>>>
>>> Right. I also thought about this. But the resulting code has all the
>>> functionality extracted into 2 functions:
>>>
>>> #if defined(CONFIG_AUTOBOOT_STOP_STR_SHA256)
>>> static int passwd_abort(uint64_t etime)
>>> {
>>>          const char *sha_env_str = getenv("bootstopkeysha256");
>>>          ...
>>> }
>>> #else
>>> static int passwd_abort(uint64_t etime)
>>> {
>>>          int abort = 0;
>>>          ...
>>> }
>>> #endif
>>>
>>> And this function is now called unconditionally:
>>>
>>>          ...
>>>          abort = passwd_abort(etime);
>>>
>>> So there is nothing here that could be simplified by using IS_ENABLED().
>>>
>>> I could of course just add this new config option to Kconfig. But with all
>>> the other related options not in Kconfig (CONFIG_AUTOBOOT_KEYED,
>>> CONFIG_AUTOBOOT_DELAY_STR...), this doesn't make much sense. So at some
>>> point all those config options should be moved to Kconfig. Unfortunately I
>>> don't have the time for this right now. But I'll add it to my list to do
>>> this at a later time.
>>
>> Well rather than adding more options, perhaps we should wait until we
>> get this moved to Kconfig? It's not going to get any easier :-)
>
> Right. And now I'm finally back at this task. To get this encrypted
> password support into mainline. With Kconfig support of course this
> time. ;)
>
> Unfortunately I'm hitting a problem while moving some of the "old"
> macros to Kconfig. Especially some strings like CONFIG_AUTOBOOT_PROMPT.
> Here how this looks in some config headers:
>
> #define CONFIG_AUTOBOOT_PROMPT         "autoboot in %d seconds\n", bootdelay
>
> This does not work, as Kconfig truncates the string after the 2nd
> '"'. Escaping this '"' using '\' also doesn't seem to work. Do you
> or Masahiro have some experience with this kind of Kconfig macro
> transition?

Not me. I noticed this when refactoring the code. IMO it is broken -
we should not be doing things like that.

From what I can see we only ever pass bootdelay as a parameter. So
perhaps you can drop the ", bootdelay" part and adjust the code in
from common/autoboot.c from:

    printf(CONFIG_AUTOBOOT_PROMPT);

to:

    printf(CONFIG_AUTOBOOT_PROMPT, bootdelay);

Of course that will create printf() warnings for a few boards but it
should be possible to turn them off at that call site.

Regards,
Simon

[U-Boot] [U-Boot, RFC] powerpc: add 2 common dcache assembly functions

[York Sun]

On 03/27/2015 08:07 AM, Valentin Longchamp wrote:
> This patch defines the 2 flush_dcache_range and invalidate_dcache_range
> functions for all the powerpc architecture. Their implementation is
> borrowed from the kernel's misc_32.S file and replace the ones from
> mpc86xx and ppc4xx since they were equivalent.
> 
> This is a fix for the problem introduced by this patch:
> http://patchwork.ozlabs.org/patch/448849/
> 
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> ---

Applied to u-boot-mpc85xx master. Awaiting upstream.

York