[PATCH] Enable a way to provide the reason for "being here"

[PATCH] Enable a way to provide the reason for "being here"

[Juergen Beisert]

Many architectures support a way to detect why the bootloader is running.
This patch adds a global variable to be able to use the cause in some kind of
shell code to do special things on demand. For example to do an emergency boot,
when the last boot fails and the watchdog reactivates the hanging system.

Comments are welcome.

Juergen

The following changes since commit 2761ef4d4ae401887f0832d1fd496f26cdf11f20:

  Merge branch 'for-next/resource-size' into next (2012-06-20 10:30:40 +0200)

are available in the git repository at:


  git://git.pengutronix.de/git/jbe/barebox.git next_provide_reset_source

for you to fetch changes up to ca97f3afb35740ac742f67a070101decd729c848:

  Add two architectures which can detect the reset source (2012-06-20 16:22:58 +0200)

----------------------------------------------------------------
Juergen Beisert (2):
      Enable a way to provide the reason for "being here"
      Add two architectures which can detect the reset source

 arch/arm/mach-imx/Makefile           |    1 +
 arch/arm/mach-imx/reset_source.c     |   59 ++++++++++++++++++++++++++++++++
 arch/arm/mach-samsung/Makefile       |    1 +
 arch/arm/mach-samsung/reset_source.c |   56 ++++++++++++++++++++++++++++++
 common/Makefile                      |    2 +-
 common/reset_source.c                |   62 ++++++++++++++++++++++++++++++++++
 include/reset_source.h               |   26 ++++++++++++++
 7 files changed, 206 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-imx/reset_source.c
 create mode 100644 arch/arm/mach-samsung/reset_source.c
 create mode 100644 common/reset_source.c
 create mode 100644 include/reset_source.h


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] Enable a way to provide the reason for "being here"

[Juergen Beisert]

Many architectures support a way to detect why the bootloader is running.
This patch adds a global variable to be able to use the cause in some kind of
shell code to do special things on demand. For example to do an emergency boot,
when the last boot fails and the watchdog reactivate the hanging system.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
---
 common/Makefile        |    2 +-
 common/reset_source.c  |   62 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/reset_source.h |   26 ++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 common/reset_source.c
 create mode 100644 include/reset_source.h

diff --git a/common/Makefile b/common/Makefile
index a1926d3..61eeb6b 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -29,7 +29,7 @@ obj-$(CONFIG_CMD_BOOTM) += uimage.o
 obj-y += startup.o
 obj-y += misc.o
 obj-y += memsize.o
-obj-$(CONFIG_GLOBALVAR) += globalvar.o
+obj-$(CONFIG_GLOBALVAR) += globalvar.o reset_source.o
 obj-$(CONFIG_FILETYPE) += filetype.o
 obj-y += resource.o
 obj-$(CONFIG_MENU) += menu.o
diff --git a/common/reset_source.c b/common/reset_source.c
new file mode 100644
index 0000000..19015b2
--- /dev/null
+++ b/common/reset_source.c
@@ -0,0 +1,62 @@
+/*
+ * (C) Copyright 2012 Juergen Beisert - <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <init.h>
+#include <environment.h>
+#include <globalvar.h>
+#include <reset_source.h>
+
+static const char name[] = "global.system.reset";
+static const char unknown_reset[] = "unknown";
+static const char power_on_reset[] = "POR";
+static const char manual_reset[] = "RST";
+static const char watchdog[] = "WDG";
+static const char wake[] = "WKE";
+static const char jtag[] = "JTAG";
+
+void set_reset_source(unsigned source)
+{
+	switch (source) {
+	case RESET_UKWN:
+		setenv(name, unknown_reset);
+		break;
+	case RESET_POR:
+		setenv(name, power_on_reset);
+		break;
+	case RESET_RST:
+		setenv(name, manual_reset);
+		break;
+	case RESET_WDG:
+		setenv(name, watchdog);
+		break;
+	case RESET_WKE:
+		setenv(name, wake);
+		break;
+	case RESET_JTAG:
+		setenv(name, jtag);
+		break;
+	}
+}
+EXPORT_SYMBOL(set_reset_source);
+
+/* ensure this runs after the 'global' device is already registerd */
+static int init_reset_source(void)
+{
+	globalvar_add_simple(&name[7]);
+	set_reset_source(RESET_UKWN);
+	return 0;
+}
+
+coredevice_initcall(init_reset_source);
diff --git a/include/reset_source.h b/include/reset_source.h
new file mode 100644
index 0000000..bc7e736
--- /dev/null
+++ b/include/reset_source.h
@@ -0,0 +1,26 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __INCLUDE_RESET_SOURCE_H
+# define __INCLUDE_RESET_SOURCE_H
+
+/* possible parameters to set_reset_source() */
+#define RESET_UKWN 0
+#define RESET_POR 1	/* Power On Reset */
+#define RESET_RST 2	/* generic ReST */
+#define RESET_WDG 3	/* watchdog */
+#define RESET_WKE 4	/* wake */
+#define RESET_JTAG 5
+
+extern void set_reset_source(unsigned);
+
+#endif /* __INCLUDE_RESET_SOURCE_H */
-- 
1.7.10


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] Add two architectures which can detect the reset source

[Juergen Beisert]

These are examples how to provide the reset source. Not really tested on
the corresponding hardware yet.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
---
 arch/arm/mach-imx/Makefile           |    1 +
 arch/arm/mach-imx/reset_source.c     |   59 ++++++++++++++++++++++++++++++++++
 arch/arm/mach-samsung/Makefile       |    1 +
 arch/arm/mach-samsung/reset_source.c |   56 ++++++++++++++++++++++++++++++++
 4 files changed, 117 insertions(+)
 create mode 100644 arch/arm/mach-imx/reset_source.c
 create mode 100644 arch/arm/mach-samsung/reset_source.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 03e2421..7da8cec 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -1,4 +1,5 @@
 obj-y += clocksource.o gpio.o
+obj-$(CONFIG_GLOBALVAR) += reset_source.o
 obj-$(CONFIG_ARCH_IMX1)  += speed-imx1.o  imx1.o  iomux-v1.o
 obj-$(CONFIG_ARCH_IMX25) += speed-imx25.o imx25.o iomux-v3.o
 obj-$(CONFIG_ARCH_IMX21) += speed-imx21.o imx21.o iomux-v1.o
diff --git a/arch/arm/mach-imx/reset_source.c b/arch/arm/mach-imx/reset_source.c
new file mode 100644
index 0000000..79e5337
--- /dev/null
+++ b/arch/arm/mach-imx/reset_source.c
@@ -0,0 +1,59 @@
+/*
+ * (C) Copyright 2012 Juergen Beisert - <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <init.h>
+#include <io.h>
+#include <reset_source.h>
+#include <mach/imx-regs.h>
+
+#ifdef CONFIG_ARCH_IMX1
+# define WDOG_WRSR 0x08
+#  define WDOG_WRSR_TOUT (1 << 0)
+/* let the compiler sort out useless code on this arch */
+#  define WDOG_WRSR_SFTW 0
+#  define WDOG_WRSR_EXT 0
+#  define WDOG_WRSR_PWR 0
+#else
+# define WDOG_WRSR 0x04
+#  define WDOG_WRSR_SFTW (1 << 0)
+#  define WDOG_WRSR_TOUT (1 << 1)
+#  define WDOG_WRSR_EXT (1 << 2)
+#  define WDOG_WRSR_PWR (1 << 3)
+#endif
+
+static int imx_detect_reset_source(void)
+{
+	u16 reg = readw(IMX_WDT_BASE + WDOG_WRSR);
+
+	if (reg & WDOG_WRSR_PWR) {
+		set_reset_source(RESET_POR);
+		return 0;
+	}
+
+	if (reg & (WDOG_WRSR_EXT | WDOG_WRSR_SFTW)) {
+		set_reset_source(RESET_RST);
+		return 0;
+	}
+
+	if (reg & WDOG_WRSR_TOUT) {
+		set_reset_source(RESET_WDG);
+		return 0;
+	}
+
+	/* else keep the default 'unknown' state */
+	return 0;
+}
+
+device_initcall(imx_detect_reset_source);
diff --git a/arch/arm/mach-samsung/Makefile b/arch/arm/mach-samsung/Makefile
index d7344c8..74799c5 100644
--- a/arch/arm/mach-samsung/Makefile
+++ b/arch/arm/mach-samsung/Makefile
@@ -1,4 +1,5 @@
 obj-y += s3c-timer.o generic.o
+obj-$(CONFIG_GLOBALVAR) += reset_source.o
 obj-lowlevel-$(CONFIG_ARCH_S3C24xx) += lowlevel-s3c24x0.o
 obj-lowlevel-$(CONFIG_ARCH_S5PCxx) += lowlevel-s5pcxx.o
 obj-$(CONFIG_ARCH_S3C24xx) += gpio-s3c24x0.o s3c24xx-clocks.o mem-s3c24x0.o
diff --git a/arch/arm/mach-samsung/reset_source.c b/arch/arm/mach-samsung/reset_source.c
new file mode 100644
index 0000000..2456e3f
--- /dev/null
+++ b/arch/arm/mach-samsung/reset_source.c
@@ -0,0 +1,56 @@
+/*
+ * (C) Copyright 2012 Juergen Beisert - <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <init.h>
+#include <io.h>
+#include <reset_source.h>
+#include <mach/s3c-iomap.h>
+
+/* S3C2440 relevant */
+#define S3C2440_GSTATUS2 0xb4
+# define S3C2440_GSTATUS2_PWRST (1 << 0)
+# define S3C2440_GSTATUS2_SLEEPRST (1 << 1)
+# define S3C2440_GSTATUS2_WDRST (1 << 2)
+
+static int s3c_detect_reset_source(void)
+{
+	u32 reg = readl(S3C_GPIO_BASE + S3C2440_GSTATUS2);
+
+	if (reg & S3C2440_GSTATUS2_PWRST) {
+		set_reset_source(RESET_POR);
+		writel(S3C2440_GSTATUS2_PWRST,
+					S3C_GPIO_BASE + S3C2440_GSTATUS2);
+		return 0;
+	}
+
+	if (reg & S3C2440_GSTATUS2_SLEEPRST) {
+		set_reset_source(RESET_WKE);
+		writel(S3C2440_GSTATUS2_SLEEPRST,
+					S3C_GPIO_BASE + S3C2440_GSTATUS2);
+		return 0;
+	}
+
+	if (reg & S3C2440_GSTATUS2_WDRST) {
+		set_reset_source(RESET_WDG);
+		writel(S3C2440_GSTATUS2_WDRST,
+					S3C_GPIO_BASE + S3C2440_GSTATUS2);
+		return 0;
+	}
+
+	/* else keep the default 'unknown' state */
+	return 0;
+}
+
+device_initcall(s3c_detect_reset_source);
-- 
1.7.10


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] Enable a way to provide the reason for "being here"

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 5076 bytes --]

On 06/20/2012 04:32 PM, Juergen Beisert wrote:
> Many architectures support a way to detect why the bootloader is running.
> This patch adds a global variable to be able to use the cause in some kind of
> shell code to do special things on demand. For example to do an emergency boot,
> when the last boot fails and the watchdog reactivate the hanging system.

Good idea, See comments inline.

Marc

> Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
> ---
>  common/Makefile        |    2 +-
>  common/reset_source.c  |   62 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/reset_source.h |   26 ++++++++++++++++++++
>  3 files changed, 89 insertions(+), 1 deletion(-)
>  create mode 100644 common/reset_source.c
>  create mode 100644 include/reset_source.h
> 
> diff --git a/common/Makefile b/common/Makefile
> index a1926d3..61eeb6b 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -29,7 +29,7 @@ obj-$(CONFIG_CMD_BOOTM) += uimage.o
>  obj-y += startup.o
>  obj-y += misc.o
>  obj-y += memsize.o
> -obj-$(CONFIG_GLOBALVAR) += globalvar.o
> +obj-$(CONFIG_GLOBALVAR) += globalvar.o reset_source.o
>  obj-$(CONFIG_FILETYPE) += filetype.o
>  obj-y += resource.o
>  obj-$(CONFIG_MENU) += menu.o
> diff --git a/common/reset_source.c b/common/reset_source.c
> new file mode 100644
> index 0000000..19015b2
> --- /dev/null
> +++ b/common/reset_source.c
> @@ -0,0 +1,62 @@
> +/*
> + * (C) Copyright 2012 Juergen Beisert - <kernel@pengutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <common.h>
> +#include <init.h>
> +#include <environment.h>
> +#include <globalvar.h>
> +#include <reset_source.h>
> +
> +static const char name[] = "global.system.reset";
> +static const char unknown_reset[] = "unknown";
> +static const char power_on_reset[] = "POR";
> +static const char manual_reset[] = "RST";
> +static const char watchdog[] = "WDG";
> +static const char wake[] = "WKE";
> +static const char jtag[] = "JTAG";

what about using an array

static cost char *reset_reason_array[] = {
	[RESET_UKWN] = "unknown",
	...
};

> +
> +void set_reset_source(unsigned source)
                         ^^^^^^^^
enum (see below)

> +{
> +	switch (source) {
> +	case RESET_UKWN:
> +		setenv(name, unknown_reset);
> +		break;
> +	case RESET_POR:
> +		setenv(name, power_on_reset);
> +		break;
> +	case RESET_RST:
> +		setenv(name, manual_reset);
> +		break;
> +	case RESET_WDG:
> +		setenv(name, watchdog);
> +		break;
> +	case RESET_WKE:
> +		setenv(name, wake);
> +		break;
> +	case RESET_JTAG:
> +		setenv(name, jtag);
> +		break;

	if (source >= ARRAY_SIZE(reset_reason_array)
		return;

	setenv("global.system.reset", reset_reason_array[source]);

> +	}
> +}
> +EXPORT_SYMBOL(set_reset_source);
> +
> +/* ensure this runs after the 'global' device is already registerd */
> +static int init_reset_source(void)
> +{
> +	globalvar_add_simple(&name[7]);
> +	set_reset_source(RESET_UKWN);
> +	return 0;
> +}
> +
> +coredevice_initcall(init_reset_source);
> diff --git a/include/reset_source.h b/include/reset_source.h
> new file mode 100644
> index 0000000..bc7e736
> --- /dev/null
> +++ b/include/reset_source.h
> @@ -0,0 +1,26 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __INCLUDE_RESET_SOURCE_H
> +# define __INCLUDE_RESET_SOURCE_H
> +
> +/* possible parameters to set_reset_source() */
> +#define RESET_UKWN 0
> +#define RESET_POR 1	/* Power On Reset */
> +#define RESET_RST 2	/* generic ReST */
> +#define RESET_WDG 3	/* watchdog */
> +#define RESET_WKE 4	/* wake */
> +#define RESET_JTAG 5

use enum here

> +
> +extern void set_reset_source(unsigned);
> +
> +#endif /* __INCLUDE_RESET_SOURCE_H */


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] Add two architectures which can detect the reset source

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 6286 bytes --]

On 06/20/2012 04:32 PM, Juergen Beisert wrote:
> These are examples how to provide the reset source. Not really tested on
> the corresponding hardware yet.
> 
> Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
> ---
>  arch/arm/mach-imx/Makefile           |    1 +
>  arch/arm/mach-imx/reset_source.c     |   59 ++++++++++++++++++++++++++++++++++
>  arch/arm/mach-samsung/Makefile       |    1 +
>  arch/arm/mach-samsung/reset_source.c |   56 ++++++++++++++++++++++++++++++++
>  4 files changed, 117 insertions(+)
>  create mode 100644 arch/arm/mach-imx/reset_source.c
>  create mode 100644 arch/arm/mach-samsung/reset_source.c
> 
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index 03e2421..7da8cec 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> @@ -1,4 +1,5 @@
>  obj-y += clocksource.o gpio.o
> +obj-$(CONFIG_GLOBALVAR) += reset_source.o
>  obj-$(CONFIG_ARCH_IMX1)  += speed-imx1.o  imx1.o  iomux-v1.o
>  obj-$(CONFIG_ARCH_IMX25) += speed-imx25.o imx25.o iomux-v3.o
>  obj-$(CONFIG_ARCH_IMX21) += speed-imx21.o imx21.o iomux-v1.o
> diff --git a/arch/arm/mach-imx/reset_source.c b/arch/arm/mach-imx/reset_source.c
> new file mode 100644
> index 0000000..79e5337
> --- /dev/null
> +++ b/arch/arm/mach-imx/reset_source.c
> @@ -0,0 +1,59 @@
> +/*
> + * (C) Copyright 2012 Juergen Beisert - <kernel@pengutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <common.h>
> +#include <init.h>
> +#include <io.h>
> +#include <reset_source.h>
> +#include <mach/imx-regs.h>
> +
> +#ifdef CONFIG_ARCH_IMX1
> +# define WDOG_WRSR 0x08
> +#  define WDOG_WRSR_TOUT (1 << 0)
> +/* let the compiler sort out useless code on this arch */
> +#  define WDOG_WRSR_SFTW 0
> +#  define WDOG_WRSR_EXT 0
> +#  define WDOG_WRSR_PWR 0
> +#else
> +# define WDOG_WRSR 0x04
> +#  define WDOG_WRSR_SFTW (1 << 0)
> +#  define WDOG_WRSR_TOUT (1 << 1)
> +#  define WDOG_WRSR_EXT (1 << 2)
> +#  define WDOG_WRSR_PWR (1 << 3)
> +#endif
> +
> +static int imx_detect_reset_source(void)
> +{
> +	u16 reg = readw(IMX_WDT_BASE + WDOG_WRSR);
> +
> +	if (reg & WDOG_WRSR_PWR) {
> +		set_reset_source(RESET_POR);
> +		return 0;
> +	}
> +
> +	if (reg & (WDOG_WRSR_EXT | WDOG_WRSR_SFTW)) {
> +		set_reset_source(RESET_RST);
> +		return 0;
> +	}
> +
> +	if (reg & WDOG_WRSR_TOUT) {
> +		set_reset_source(RESET_WDG);
> +		return 0;
> +	}
> +
> +	/* else keep the default 'unknown' state */
> +	return 0;
> +}
> +
> +device_initcall(imx_detect_reset_source);
> diff --git a/arch/arm/mach-samsung/Makefile b/arch/arm/mach-samsung/Makefile
> index d7344c8..74799c5 100644
> --- a/arch/arm/mach-samsung/Makefile
> +++ b/arch/arm/mach-samsung/Makefile
> @@ -1,4 +1,5 @@
>  obj-y += s3c-timer.o generic.o
> +obj-$(CONFIG_GLOBALVAR) += reset_source.o
>  obj-lowlevel-$(CONFIG_ARCH_S3C24xx) += lowlevel-s3c24x0.o
>  obj-lowlevel-$(CONFIG_ARCH_S5PCxx) += lowlevel-s5pcxx.o
>  obj-$(CONFIG_ARCH_S3C24xx) += gpio-s3c24x0.o s3c24xx-clocks.o mem-s3c24x0.o
> diff --git a/arch/arm/mach-samsung/reset_source.c b/arch/arm/mach-samsung/reset_source.c
> new file mode 100644
> index 0000000..2456e3f
> --- /dev/null
> +++ b/arch/arm/mach-samsung/reset_source.c
> @@ -0,0 +1,56 @@
> +/*
> + * (C) Copyright 2012 Juergen Beisert - <kernel@pengutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <common.h>
> +#include <init.h>
> +#include <io.h>
> +#include <reset_source.h>
> +#include <mach/s3c-iomap.h>
> +
> +/* S3C2440 relevant */
> +#define S3C2440_GSTATUS2 0xb4
> +# define S3C2440_GSTATUS2_PWRST (1 << 0)
> +# define S3C2440_GSTATUS2_SLEEPRST (1 << 1)
> +# define S3C2440_GSTATUS2_WDRST (1 << 2)
> +
> +static int s3c_detect_reset_source(void)
> +{
> +	u32 reg = readl(S3C_GPIO_BASE + S3C2440_GSTATUS2);
> +
> +	if (reg & S3C2440_GSTATUS2_PWRST) {
> +		set_reset_source(RESET_POR);
> +		writel(S3C2440_GSTATUS2_PWRST,
> +					S3C_GPIO_BASE + S3C2440_GSTATUS2);
> +		return 0;
> +	}
> +
> +	if (reg & S3C2440_GSTATUS2_SLEEPRST) {
> +		set_reset_source(RESET_WKE);
> +		writel(S3C2440_GSTATUS2_SLEEPRST,
> +					S3C_GPIO_BASE + S3C2440_GSTATUS2);
> +		return 0;
> +	}
> +
> +	if (reg & S3C2440_GSTATUS2_WDRST) {
> +		set_reset_source(RESET_WDG);
> +		writel(S3C2440_GSTATUS2_WDRST,
> +					S3C_GPIO_BASE + S3C2440_GSTATUS2);
> +		return 0;
> +	}

That "writel(S3C2440_GSTATUS2_WDRST...)" is the same in each line, isn't it?

What about this:

	if (reg & S3C2440_GSTATUS2_PWRST)
		set_reset_source(RESET_POR);
	else if (...)
	else if (..)
	else
		return 0;

	writel(S3C2440_GSTATUS2_WDRST,
	       S3C_GPIO_BASE +S3C2440_GSTATUS2);

	return 0;
}

(If you convert the imx into "if else if" the function will be a
shorter, because you don't need any return 0 and "{ }")

> +
> +	/* else keep the default 'unknown' state */
> +	return 0;
> +}
> +
> +device_initcall(s3c_detect_reset_source);

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] Add two architectures which can detect the reset source

[Juergen Beisert]

Marc Kleine-Budde wrote:
> [...]
> > +static int s3c_detect_reset_source(void)
> > +{
> > +	u32 reg = readl(S3C_GPIO_BASE + S3C2440_GSTATUS2);
> > +
> > +	if (reg & S3C2440_GSTATUS2_PWRST) {
> > +		set_reset_source(RESET_POR);
> > +		writel(S3C2440_GSTATUS2_PWRST,
                                        ^^^^^
> > +					S3C_GPIO_BASE + S3C2440_GSTATUS2);
> > +		return 0;
> > +	}
> > +
> > +	if (reg & S3C2440_GSTATUS2_SLEEPRST) {
> > +		set_reset_source(RESET_WKE);
> > +		writel(S3C2440_GSTATUS2_SLEEPRST,
                                        ^^^^^^^^
> > +					S3C_GPIO_BASE + S3C2440_GSTATUS2);
> > +		return 0;
> > +	}
> > +
> > +	if (reg & S3C2440_GSTATUS2_WDRST) {
> > +		set_reset_source(RESET_WDG);
> > +		writel(S3C2440_GSTATUS2_WDRST,
                                        ^^^^^
> > +					S3C_GPIO_BASE + S3C2440_GSTATUS2);
> > +		return 0;
> > +	}
>
> That "writel(S3C2440_GSTATUS2_WDRST...)" is the same in each line, isn't
> it?

Take a closer look. ;)

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] Enable a way to provide the reason for "being here"

[Juergen Beisert]

Marc Kleine-Budde wrote:
> [...]
> > +
> > +#include <common.h>
> > +#include <init.h>
> > +#include <environment.h>
> > +#include <globalvar.h>
> > +#include <reset_source.h>
> > +
> > +static const char name[] = "global.system.reset";
> > +static const char unknown_reset[] = "unknown";
> > +static const char power_on_reset[] = "POR";
> > +static const char manual_reset[] = "RST";
> > +static const char watchdog[] = "WDG";
> > +static const char wake[] = "WKE";
> > +static const char jtag[] = "JTAG";
>
> what about using an array
>
> static cost char *reset_reason_array[] = {
> 	[RESET_UKWN] = "unknown",
> 	...
> };

The result is not the same. The strings are no longer "const". But it should 
not make a difference from the point of view of a possible reader. Will 
change it.

> [...]

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] Add two architectures which can detect the reset source

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 1370 bytes --]

On 06/20/2012 05:05 PM, Juergen Beisert wrote:
> Marc Kleine-Budde wrote:
>> [...]
>>> +static int s3c_detect_reset_source(void)
>>> +{
>>> +	u32 reg = readl(S3C_GPIO_BASE + S3C2440_GSTATUS2);
>>> +
>>> +	if (reg & S3C2440_GSTATUS2_PWRST) {
>>> +		set_reset_source(RESET_POR);
>>> +		writel(S3C2440_GSTATUS2_PWRST,
>                                         ^^^^^
>>> +					S3C_GPIO_BASE + S3C2440_GSTATUS2);
>>> +		return 0;
>>> +	}
>>> +
>>> +	if (reg & S3C2440_GSTATUS2_SLEEPRST) {
>>> +		set_reset_source(RESET_WKE);
>>> +		writel(S3C2440_GSTATUS2_SLEEPRST,
>                                         ^^^^^^^^
>>> +					S3C_GPIO_BASE + S3C2440_GSTATUS2);
>>> +		return 0;
>>> +	}
>>> +
>>> +	if (reg & S3C2440_GSTATUS2_WDRST) {
>>> +		set_reset_source(RESET_WDG);
>>> +		writel(S3C2440_GSTATUS2_WDRST,
>                                         ^^^^^
>>> +					S3C_GPIO_BASE + S3C2440_GSTATUS2);
>>> +		return 0;
>>> +	}
>>
>> That "writel(S3C2440_GSTATUS2_WDRST...)" is the same in each line, isn't
>> it?
> 
> Take a closer look. ;)

tnx - you're rifht

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] Enable a way to provide the reason for "being here"

[Uwe Kleine-König]

On Wed, Jun 20, 2012 at 05:08:52PM +0200, Juergen Beisert wrote:
> Marc Kleine-Budde wrote:
> > [...]
> > > +
> > > +#include <common.h>
> > > +#include <init.h>
> > > +#include <environment.h>
> > > +#include <globalvar.h>
> > > +#include <reset_source.h>
> > > +
> > > +static const char name[] = "global.system.reset";
> > > +static const char unknown_reset[] = "unknown";
> > > +static const char power_on_reset[] = "POR";
> > > +static const char manual_reset[] = "RST";
> > > +static const char watchdog[] = "WDG";
> > > +static const char wake[] = "WKE";
> > > +static const char jtag[] = "JTAG";
> >
> > what about using an array
> >
> > static cost char *reset_reason_array[] = {
> > 	[RESET_UKWN] = "unknown",
> > 	...
> > };
> 
> The result is not the same. The strings are no longer "const". But it should 
<kidding>Of course they are not "const" if you write "cost".</kidding>
Using

	static const char * const reset_reason_array[] = {

should do the trick.
Having said that, I wonder what is the difference between

	static const char wake[] = "WKE";
	... use wake here ...

and

	... just use "WKE" ...

.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] Enable a way to provide the reason for "being here"

[Juergen Beisert]

Hi Uwe,

Uwe Kleine-König wrote:
> On Wed, Jun 20, 2012 at 05:08:52PM +0200, Juergen Beisert wrote:
> > Marc Kleine-Budde wrote:
> > > [...]
> > >
> > > > +
> > > > +#include <common.h>
> > > > +#include <init.h>
> > > > +#include <environment.h>
> > > > +#include <globalvar.h>
> > > > +#include <reset_source.h>
> > > > +
> > > > +static const char name[] = "global.system.reset";
> > > > +static const char unknown_reset[] = "unknown";
> > > > +static const char power_on_reset[] = "POR";
> > > > +static const char manual_reset[] = "RST";
> > > > +static const char watchdog[] = "WDG";
> > > > +static const char wake[] = "WKE";
> > > > +static const char jtag[] = "JTAG";
> > >
> > > what about using an array
> > >
> > > static cost char *reset_reason_array[] = {
> > > 	[RESET_UKWN] = "unknown",
> > > 	...
> > > };
> >
> > The result is not the same. The strings are no longer "const". But it
> > should
>
> <kidding>Of course they are not "const" if you write "cost".</kidding>
> Using
>
> 	static const char * const reset_reason_array[] = {
>
> should do the trick.

No, it doesn't. Only this would:

static const bla[] = "this is a really constant string";
static const char * const reset_reason_array[] = {
	[0] = bla,
[...]
};

But its ugly. So I will prefer Marc's suggestion.

> Having said that, I wonder what is the difference between
>
> 	static const char wake[] = "WKE";
> 	... use wake here ...
>
> and
>
> 	... just use "WKE" ...

Just based on how U-Boot does it.

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-5121-206917-5128 |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] Enable a way to provide the reason for "being here"

[Uwe Kleine-König]

On Wed, Jun 20, 2012 at 09:09:53PM +0200, Juergen Beisert wrote:
> Hi Uwe,
> 
> Uwe Kleine-König wrote:
> > On Wed, Jun 20, 2012 at 05:08:52PM +0200, Juergen Beisert wrote:
> > > Marc Kleine-Budde wrote:
> > > > [...]
> > > >
> > > > > +
> > > > > +#include <common.h>
> > > > > +#include <init.h>
> > > > > +#include <environment.h>
> > > > > +#include <globalvar.h>
> > > > > +#include <reset_source.h>
> > > > > +
> > > > > +static const char name[] = "global.system.reset";
> > > > > +static const char unknown_reset[] = "unknown";
> > > > > +static const char power_on_reset[] = "POR";
> > > > > +static const char manual_reset[] = "RST";
> > > > > +static const char watchdog[] = "WDG";
> > > > > +static const char wake[] = "WKE";
> > > > > +static const char jtag[] = "JTAG";
> > > >
> > > > what about using an array
> > > >
> > > > static cost char *reset_reason_array[] = {
> > > > 	[RESET_UKWN] = "unknown",
> > > > 	...
> > > > };
> > >
> > > The result is not the same. The strings are no longer "const". But it
> > > should
> >
> > <kidding>Of course they are not "const" if you write "cost".</kidding>
> > Using
> >
> > 	static const char * const reset_reason_array[] = {
> >
> > should do the trick.
> 
> No, it doesn't. Only this would:
> 
> static const bla[] = "this is a really constant string";
> static const char * const reset_reason_array[] = {
> 	[0] = bla,
> [...]
> };
What makes you think it's not const. I think it is, look:

	ukleinek@perseus:~/tmp$ cat test.c
	static const char * const const_reset_reason_array[] = {
		"jtag",
		"por",
	};

	static const char *reset_reason_array[] = {
		"jtag",
		"por",
	};

	void somefunc(void)
	{
		char *reason = const_reset_reason_array[0];
	}
	ukleinek@perseus:~/tmp$ gcc -Wall -c test.c
	test.c: In function ‘somefunc’:
	test.c:13:17: warning: initialization discards ‘const’ qualifier from pointer target type [enabled by default]
	test.c:13:8: warning: unused variable ‘reason’ [-Wunused-variable]
	test.c: At top level:
	test.c:6:20: warning: ‘reset_reason_array’ defined but not used [-Wunused-variable]

	ukleinek@perseus:~/tmp$ objdump -D -j .rodata -j .data test.o

	test.o:     file format elf64-x86-64


	Disassembly of section .data:

	0000000000000000 <reset_reason_array>:
		...

	Disassembly of section .rodata:

	0000000000000000 <const_reset_reason_array-0x10>:
	   0:	6a 74                	pushq  $0x74
	   2:	61                   	(bad)  
	   3:	67 00 70 6f          	add    %dh,0x6f(%eax)
	   7:	72 00                	jb     9 <const_reset_reason_array-0x7>
	   9:	00 00                	add    %al,(%rax)
	   b:	00 00                	add    %al,(%rax)
	   d:	00 00                	add    %al,(%rax)
		...

	0000000000000010 <const_reset_reason_array>:
		...

so const_reset_reason_array lives in .rodata and assinging
const_reset_reason_array[0] to a char * results in a warning.

What am I missing?

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] Enable a way to provide the reason for "being here"

[Juergen Beisert]

Uwe Kleine-König wrote:
> On Wed, Jun 20, 2012 at 09:09:53PM +0200, Juergen Beisert wrote:
> > Hi Uwe,
> >
> > Uwe Kleine-König wrote:
> > > On Wed, Jun 20, 2012 at 05:08:52PM +0200, Juergen Beisert wrote:
> > > > Marc Kleine-Budde wrote:
> > > > > [...]
> > > > >
> > > > > > +
> > > > > > +#include <common.h>
> > > > > > +#include <init.h>
> > > > > > +#include <environment.h>
> > > > > > +#include <globalvar.h>
> > > > > > +#include <reset_source.h>
> > > > > > +
> > > > > > +static const char name[] = "global.system.reset";
> > > > > > +static const char unknown_reset[] = "unknown";
> > > > > > +static const char power_on_reset[] = "POR";
> > > > > > +static const char manual_reset[] = "RST";
> > > > > > +static const char watchdog[] = "WDG";
> > > > > > +static const char wake[] = "WKE";
> > > > > > +static const char jtag[] = "JTAG";
> > > > >
> > > > > what about using an array
> > > > >
> > > > > static cost char *reset_reason_array[] = {
> > > > > 	[RESET_UKWN] = "unknown",
> > > > > 	...
> > > > > };
> > > >
> > > > The result is not the same. The strings are no longer "const". But it
> > > > should
> > >
> > > <kidding>Of course they are not "const" if you write "cost".</kidding>
> > > Using
> > >
> > > 	static const char * const reset_reason_array[] = {
> > >
> > > should do the trick.
> >
> > No, it doesn't. Only this would:
> >
> > static const bla[] = "this is a really constant string";
> > static const char * const reset_reason_array[] = {
> > 	[0] = bla,
> > [...]
> > };
>
> What makes you think it's not const. I think it is, look:
>
> 	ukleinek@perseus:~/tmp$ cat test.c
> 	static const char * const const_reset_reason_array[] = {
> 		"jtag",
> 		"por",
> 	};
>
> 	static const char *reset_reason_array[] = {
> 		"jtag",
> 		"por",
> 	};
>
> 	void somefunc(void)
> 	{
> 		char *reason = const_reset_reason_array[0];
> 	}
> 	ukleinek@perseus:~/tmp$ gcc -Wall -c test.c
> 	test.c: In function ‘somefunc’:
> 	test.c:13:17: warning: initialization discards ‘const’ qualifier from
> pointer target type [enabled by default] test.c:13:8: warning: unused
> variable ‘reason’ [-Wunused-variable] test.c: At top level:
> 	test.c:6:20: warning: ‘reset_reason_array’ defined but not used
> [-Wunused-variable]
>
> 	ukleinek@perseus:~/tmp$ objdump -D -j .rodata -j .data test.o
>
> 	test.o:     file format elf64-x86-64
>
>
> 	Disassembly of section .data:
>
> 	0000000000000000 <reset_reason_array>:
> 		...
>
> 	Disassembly of section .rodata:
>
> 	0000000000000000 <const_reset_reason_array-0x10>:
> 	   0:	6a 74                	pushq  $0x74
> 	   2:	61                   	(bad)
> 	   3:	67 00 70 6f          	add    %dh,0x6f(%eax)
> 	   7:	72 00                	jb     9 <const_reset_reason_array-0x7>
> 	   9:	00 00                	add    %al,(%rax)
> 	   b:	00 00                	add    %al,(%rax)
> 	   d:	00 00                	add    %al,(%rax)
> 		...
>
> 	0000000000000010 <const_reset_reason_array>:
> 		...
>
> so const_reset_reason_array lives in .rodata and assinging
> const_reset_reason_array[0] to a char * results in a warning.
>
> What am I missing?

const char friesel[] = "this is really a constant string";
const char *frasel = "this is not really a constant string";

The compiler will place the "this is really a constant string" into 
the .rodata, but "this is not really a constant string" will be still 
in .data (writeable). This difference is very important to know, if you want 
the compiler to place the strings into the flash of a microcontroller instead 
of the RAM (which is most of the time very small).

Refer also section "2.4 Choosing the Right Type" in Ulrich Drepper's "How To 
Write Shared Libraries".

But for Barebox this difference is not very important.

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-5121-206917-5128 |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] Enable a way to provide the reason for "being here"

[Uwe Kleine-König]

On Wed, Jun 20, 2012 at 09:59:14PM +0200, Juergen Beisert wrote:
> Uwe Kleine-König wrote:
> > On Wed, Jun 20, 2012 at 09:09:53PM +0200, Juergen Beisert wrote:
> > > Hi Uwe,
> > >
> > > Uwe Kleine-König wrote:
> > > > On Wed, Jun 20, 2012 at 05:08:52PM +0200, Juergen Beisert wrote:
> > > > > Marc Kleine-Budde wrote:
> > > > > > [...]
> > > > > >
> > > > > > > +
> > > > > > > +#include <common.h>
> > > > > > > +#include <init.h>
> > > > > > > +#include <environment.h>
> > > > > > > +#include <globalvar.h>
> > > > > > > +#include <reset_source.h>
> > > > > > > +
> > > > > > > +static const char name[] = "global.system.reset";
> > > > > > > +static const char unknown_reset[] = "unknown";
> > > > > > > +static const char power_on_reset[] = "POR";
> > > > > > > +static const char manual_reset[] = "RST";
> > > > > > > +static const char watchdog[] = "WDG";
> > > > > > > +static const char wake[] = "WKE";
> > > > > > > +static const char jtag[] = "JTAG";
> > > > > >
> > > > > > what about using an array
> > > > > >
> > > > > > static cost char *reset_reason_array[] = {
> > > > > > 	[RESET_UKWN] = "unknown",
> > > > > > 	...
> > > > > > };
> > > > >
> > > > > The result is not the same. The strings are no longer "const". But it
> > > > > should
> > > >
> > > > <kidding>Of course they are not "const" if you write "cost".</kidding>
> > > > Using
> > > >
> > > > 	static const char * const reset_reason_array[] = {
> > > >
> > > > should do the trick.
> > >
> > > No, it doesn't. Only this would:
> > >
> > > static const bla[] = "this is a really constant string";
> > > static const char * const reset_reason_array[] = {
> > > 	[0] = bla,
> > > [...]
> > > };
> >
> > What makes you think it's not const. I think it is, look:
> >
> > 	ukleinek@perseus:~/tmp$ cat test.c
> > 	static const char * const const_reset_reason_array[] = {
> > 		"jtag",
> > 		"por",
> > 	};
> >
> > 	static const char *reset_reason_array[] = {
> > 		"jtag",
> > 		"por",
> > 	};
> >
> > 	void somefunc(void)
> > 	{
> > 		char *reason = const_reset_reason_array[0];
> > 	}
> > 	ukleinek@perseus:~/tmp$ gcc -Wall -c test.c
> > 	test.c: In function ‘somefunc’:
> > 	test.c:13:17: warning: initialization discards ‘const’ qualifier from
> > pointer target type [enabled by default] test.c:13:8: warning: unused
> > variable ‘reason’ [-Wunused-variable] test.c: At top level:
> > 	test.c:6:20: warning: ‘reset_reason_array’ defined but not used
> > [-Wunused-variable]
> >
> > 	ukleinek@perseus:~/tmp$ objdump -D -j .rodata -j .data test.o
> >
> > 	test.o:     file format elf64-x86-64
> >
> >
> > 	Disassembly of section .data:
> >
> > 	0000000000000000 <reset_reason_array>:
> > 		...
> >
> > 	Disassembly of section .rodata:
> >
> > 	0000000000000000 <const_reset_reason_array-0x10>:
> > 	   0:	6a 74                	pushq  $0x74
> > 	   2:	61                   	(bad)
> > 	   3:	67 00 70 6f          	add    %dh,0x6f(%eax)
> > 	   7:	72 00                	jb     9 <const_reset_reason_array-0x7>
> > 	   9:	00 00                	add    %al,(%rax)
> > 	   b:	00 00                	add    %al,(%rax)
> > 	   d:	00 00                	add    %al,(%rax)
> > 		...
> >
> > 	0000000000000010 <const_reset_reason_array>:
> > 		...
> >
> > so const_reset_reason_array lives in .rodata and assinging
> > const_reset_reason_array[0] to a char * results in a warning.
> >
> > What am I missing?
> 
> const char friesel[] = "this is really a constant string";
> const char *frasel = "this is not really a constant string";
> 
> The compiler will place the "this is really a constant string" into 
> the .rodata, but "this is not really a constant string" will be still 
> in .data (writeable). This difference is very important to know, if you want 
> the compiler to place the strings into the flash of a microcontroller instead 
> of the RAM (which is most of the time very small).
Right, and if you say:

	const char *const frosel = "this is really a constant string, too";

it will end in .rodata, too. And that is the construct I suggested to
use.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/2] Enable a way to provide the reason for "being here"

[Juergen Beisert]

Uwe Kleine-König wrote:
> On Wed, Jun 20, 2012 at 09:59:14PM +0200, Juergen Beisert wrote:
> > Uwe Kleine-König wrote:
> > > On Wed, Jun 20, 2012 at 09:09:53PM +0200, Juergen Beisert wrote:
> > > > Hi Uwe,
> > > >
> > > > Uwe Kleine-König wrote:
> > > > > On Wed, Jun 20, 2012 at 05:08:52PM +0200, Juergen Beisert wrote:
> > > > > > Marc Kleine-Budde wrote:
> > > > > > > [...]
> > > > > > >
> > > > > > > > +
> > > > > > > > +#include <common.h>
> > > > > > > > +#include <init.h>
> > > > > > > > +#include <environment.h>
> > > > > > > > +#include <globalvar.h>
> > > > > > > > +#include <reset_source.h>
> > > > > > > > +
> > > > > > > > +static const char name[] = "global.system.reset";
> > > > > > > > +static const char unknown_reset[] = "unknown";
> > > > > > > > +static const char power_on_reset[] = "POR";
> > > > > > > > +static const char manual_reset[] = "RST";
> > > > > > > > +static const char watchdog[] = "WDG";
> > > > > > > > +static const char wake[] = "WKE";
> > > > > > > > +static const char jtag[] = "JTAG";
> > > > > > >
> > > > > > > what about using an array
> > > > > > >
> > > > > > > static cost char *reset_reason_array[] = {
> > > > > > > 	[RESET_UKWN] = "unknown",
> > > > > > > 	...
> > > > > > > };
> > > > > >
> > > > > > The result is not the same. The strings are no longer "const".
> > > > > > But it should
> > > > >
> > > > > <kidding>Of course they are not "const" if you write
> > > > > "cost".</kidding> Using
> > > > >
> > > > > 	static const char * const reset_reason_array[] = {
> > > > >
> > > > > should do the trick.
> > > >
> > > > No, it doesn't. Only this would:
> > > >
> > > > static const bla[] = "this is a really constant string";
> > > > static const char * const reset_reason_array[] = {
> > > > 	[0] = bla,
> > > > [...]
> > > > };
> > >
> > > What makes you think it's not const. I think it is, look:
> > >
> > > 	ukleinek@perseus:~/tmp$ cat test.c
> > > 	static const char * const const_reset_reason_array[] = {
> > > 		"jtag",
> > > 		"por",
> > > 	};
> > >
> > > 	static const char *reset_reason_array[] = {
> > > 		"jtag",
> > > 		"por",
> > > 	};
> > >
> > > 	void somefunc(void)
> > > 	{
> > > 		char *reason = const_reset_reason_array[0];
> > > 	}
> > > 	ukleinek@perseus:~/tmp$ gcc -Wall -c test.c
> > > 	test.c: In function ‘somefunc’:
> > > 	test.c:13:17: warning: initialization discards ‘const’ qualifier from
> > > pointer target type [enabled by default] test.c:13:8: warning: unused
> > > variable ‘reason’ [-Wunused-variable] test.c: At top level:
> > > 	test.c:6:20: warning: ‘reset_reason_array’ defined but not used
> > > [-Wunused-variable]
> > >
> > > 	ukleinek@perseus:~/tmp$ objdump -D -j .rodata -j .data test.o
> > >
> > > 	test.o:     file format elf64-x86-64
> > >
> > >
> > > 	Disassembly of section .data:
> > >
> > > 	0000000000000000 <reset_reason_array>:
> > > 		...
> > >
> > > 	Disassembly of section .rodata:
> > >
> > > 	0000000000000000 <const_reset_reason_array-0x10>:
> > > 	   0:	6a 74                	pushq  $0x74
> > > 	   2:	61                   	(bad)
> > > 	   3:	67 00 70 6f          	add    %dh,0x6f(%eax)
> > > 	   7:	72 00                	jb     9 <const_reset_reason_array-0x7>
> > > 	   9:	00 00                	add    %al,(%rax)
> > > 	   b:	00 00                	add    %al,(%rax)
> > > 	   d:	00 00                	add    %al,(%rax)
> > > 		...
> > >
> > > 	0000000000000010 <const_reset_reason_array>:
> > > 		...
> > >
> > > so const_reset_reason_array lives in .rodata and assinging
> > > const_reset_reason_array[0] to a char * results in a warning.
> > >
> > > What am I missing?
> >
> > const char friesel[] = "this is really a constant string";
> > const char *frasel = "this is not really a constant string";
> >
> > The compiler will place the "this is really a constant string" into
> > the .rodata, but "this is not really a constant string" will be still
> > in .data (writeable). This difference is very important to know, if you
> > want the compiler to place the strings into the flash of a
> > microcontroller instead of the RAM (which is most of the time very
> > small).
>
> Right, and if you say:
>
> 	const char *const frosel = "this is really a constant string, too";
>
> it will end in .rodata, too. And that is the construct I suggested to
> use.

Okay, you are right.

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-5121-206917-5128 |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] Enable a way to provide the reason for "being here"

[Juergen Beisert]

Many architectures support a way to detect why the bootloader is running.
This patch adds a global variable to be able to use the cause in some kind of
shell code to do special things on demand. For example to do an emergency boot,
when the last boot fails and the watchdog reactivate the hanging system.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
---
 common/Makefile        |    2 +-
 common/reset_source.c  |   44 ++++++++++++++++++++++++++++++++++++++++++++
 include/reset_source.h |   27 +++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 1 deletion(-)
 create mode 100644 common/reset_source.c
 create mode 100644 include/reset_source.h

diff --git a/common/Makefile b/common/Makefile
index a1926d3..61eeb6b 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -29,7 +29,7 @@ obj-$(CONFIG_CMD_BOOTM) += uimage.o
 obj-y += startup.o
 obj-y += misc.o
 obj-y += memsize.o
-obj-$(CONFIG_GLOBALVAR) += globalvar.o
+obj-$(CONFIG_GLOBALVAR) += globalvar.o reset_source.o
 obj-$(CONFIG_FILETYPE) += filetype.o
 obj-y += resource.o
 obj-$(CONFIG_MENU) += menu.o
diff --git a/common/reset_source.c b/common/reset_source.c
new file mode 100644
index 0000000..2a7f9ff
--- /dev/null
+++ b/common/reset_source.c
@@ -0,0 +1,44 @@
+/*
+ * (C) Copyright 2012 Juergen Beisert - <kernel@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <common.h>
+#include <init.h>
+#include <environment.h>
+#include <globalvar.h>
+#include <reset_source.h>
+
+static const char * const reset_src_names[] = {
+	[RESET_UKWN] = "unknown",
+	[RESET_POR] = "POR",
+	[RESET_RST] = "RST",
+	[RESET_WDG] = "WDG",
+	[RESET_WKE] = "WKE",
+	[RESET_JTAG] = "JTAG",
+};
+
+void set_reset_source(enum reset_src_type st)
+{
+	setenv("global.system.reset", reset_src_names[st]);
+}
+EXPORT_SYMBOL(set_reset_source);
+
+/* ensure this runs after the 'global' device is already registerd */
+static int init_reset_source(void)
+{
+	globalvar_add_simple("system.reset");
+	set_reset_source(RESET_UKWN);
+	return 0;
+}
+
+coredevice_initcall(init_reset_source);
diff --git a/include/reset_source.h b/include/reset_source.h
new file mode 100644
index 0000000..8bb366c
--- /dev/null
+++ b/include/reset_source.h
@@ -0,0 +1,27 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __INCLUDE_RESET_SOURCE_H
+# define __INCLUDE_RESET_SOURCE_H
+
+enum reset_src_type {
+	RESET_UKWN,	/* maybe the SoC cannot detect the reset source */
+	RESET_POR,	/* Power On Reset (cold start) */
+	RESET_RST,	/* generic ReSeT (warm start) */
+	RESET_WDG,	/* watchdog */
+	RESET_WKE,	/* wake-up (some SoCs can handle this) */
+	RESET_JTAG,	/* JTAG reset */
+};
+
+extern void set_reset_source(enum reset_src_type);
+
+#endif /* __INCLUDE_RESET_SOURCE_H */
-- 
1.7.10


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox