[PATCH v2] imx: Save boot source into env during init

[PATCH v2] imx: Save boot source into env during init

[Marc Reilly]

Hi,

This series cleans up the imx_boot_save_loc function and changes the values
of the exported $boot_loc variable to be more consistent with the IC docs, to 
avoid potential confusion (when say booting off an SPI eeprom, $boot_loc is
now "expansion" rather than "mmc")

Cheers,
Marc


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

[PATCH v2 1/3] Init env context a bit earlier in initalization sequence.

[Marc Reilly]

Ensures subsequent init functions to export environment variables during
init without having to rely on linking order being correct.

Signed-off-by: Marc Reilly <marc@cpdesign.com.au>
---
 common/env.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/common/env.c b/common/env.c
index f81bd46..b72b5d1 100644
--- a/common/env.c
+++ b/common/env.c
@@ -81,7 +81,7 @@ int env_push_context(void)
 	return 0;
 }
 
-late_initcall(env_push_context);
+device_initcall(env_push_context);
 
 /**
  * FIXME
-- 
1.7.1


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

[PATCH v2 2/3] imx: add reg definitions for imx25 and imx35 boot source.

[Marc Reilly]

Signed-off-by: Marc Reilly <marc@cpdesign.com.au>
---
 arch/arm/mach-imx/include/mach/imx25-regs.h |    3 +++
 arch/arm/mach-imx/include/mach/imx35-regs.h |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-imx/include/mach/imx25-regs.h b/arch/arm/mach-imx/include/mach/imx25-regs.h
index ac590d9..78ec0cd 100644
--- a/arch/arm/mach-imx/include/mach/imx25-regs.h
+++ b/arch/arm/mach-imx/include/mach/imx25-regs.h
@@ -79,6 +79,9 @@
 #define PDR0_AUTO_CON		(1 << 0)
 #define PDR0_PER_SEL		(1 << 26)
 
+#define CCM_RCSR_MEM_CTRL_SHIFT		30
+#define CCM_RCSR_MEM_TYPE_SHIFT		28
+
 /*
  * Adresses and ranges of the external chip select lines
  */
diff --git a/arch/arm/mach-imx/include/mach/imx35-regs.h b/arch/arm/mach-imx/include/mach/imx35-regs.h
index 75825e5..280d075 100644
--- a/arch/arm/mach-imx/include/mach/imx35-regs.h
+++ b/arch/arm/mach-imx/include/mach/imx35-regs.h
@@ -79,6 +79,9 @@
 #define CCM_CGR1_SDHC1_SHIFT	26
 #define CCM_CGR2_USB_SHIFT	22
 
+#define CCM_RCSR_MEM_CTRL_SHIFT		25
+#define CCM_RCSR_MEM_TYPE_SHIFT		23
+
 #define PDR0_AUTO_MUX_DIV(x)	(((x) & 0x7) << 9)
 #define PDR0_CCM_PER_AHB(x)	(((x) & 0x7) << 12)
 #define PDR0_CON_MUX_DIV(x)	(((x) & 0xf) << 16)
-- 
1.7.1


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

[PATCH v2 3/3] imx(25,35): save boot location into $boot_loc env.

[Marc Reilly]

Saves the boot source into an environment variable so env scripts
can more easily use boot source information. We could also
look at the extra information in the MEM_TYPE bits if needed in future,
but this is good enough for my purposes.

Note only tested on imx35. I haven't added support for any other variants
because I'm not familiar with them. (And can't test them anyway).

Signed-off-by: Marc Reilly <marc@cpdesign.com.au>
---
 arch/arm/mach-imx/Makefile |    1 +
 arch/arm/mach-imx/boot.c   |   61 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-imx/boot.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index d000683..3b2deaf 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_IMX_IIM)	+= iim.o
 obj-$(CONFIG_NAND_IMX) += nand.o
 obj-y += speed.o
 obj-y += devices.o
+obj-y += boot.o
diff --git a/arch/arm/mach-imx/boot.c b/arch/arm/mach-imx/boot.c
new file mode 100644
index 0000000..560fc76
--- /dev/null
+++ b/arch/arm/mach-imx/boot.c
@@ -0,0 +1,61 @@
+/*
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <environment.h>
+#include <init.h>
+
+#include <asm/io.h>
+#include <mach/imx-regs.h>
+
+#if defined(CONFIG_ARCH_IMX25) || defined(CONFIG_ARCH_IMX35)
+static int imx_boot_save_loc(void)
+{
+	const char *bootloc = NULL;
+	uint32_t reg;
+
+	reg = readl(IMX_CCM_BASE + CCM_RCSR);
+
+	switch ((reg >> CCM_RCSR_MEM_CTRL_SHIFT) & 0x3) {
+	case 0x03:		/* SD/MMC, I2C, SPI is the source */
+		bootloc = "expansion";
+		break;
+	case 0x01:		/* NAND is the source */
+		bootloc = "nand";
+		break;
+	case 0x00:		/* NOR, OneNand is the source */
+		bootloc = "weim";
+		break;
+	default:
+		break;
+	}
+
+	if (bootloc) {
+		setenv("boot_loc", bootloc);
+		export("boot_loc");
+	}
+
+	return 0;
+}
+
+/*
+ * This can only be called after env_push_context() has been called
+ * so it is a late_initcall.
+ */
+late_initcall(imx_boot_save_loc);
+
+#endif
-- 
1.7.1


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

Re: [PATCH v2 3/3] imx(25,35): save boot location into $boot_loc env.

[Sascha Hauer]

On Tue, Jan 11, 2011 at 10:40:16AM +1100, Marc Reilly wrote:
> Saves the boot source into an environment variable so env scripts
> can more easily use boot source information. We could also
> look at the extra information in the MEM_TYPE bits if needed in future,
> but this is good enough for my purposes.
> 
> Note only tested on imx35. I haven't added support for any other variants
> because I'm not familiar with them. (And can't test them anyway).
> 
> Signed-off-by: Marc Reilly <marc@cpdesign.com.au>
> ---
>  arch/arm/mach-imx/Makefile |    1 +
>  arch/arm/mach-imx/boot.c   |   61 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-imx/boot.c
> 
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index d000683..3b2deaf 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_IMX_IIM)	+= iim.o
>  obj-$(CONFIG_NAND_IMX) += nand.o
>  obj-y += speed.o
>  obj-y += devices.o
> +obj-y += boot.o
> diff --git a/arch/arm/mach-imx/boot.c b/arch/arm/mach-imx/boot.c
> new file mode 100644
> index 0000000..560fc76
> --- /dev/null
> +++ b/arch/arm/mach-imx/boot.c
> @@ -0,0 +1,61 @@
> +/*
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <environment.h>
> +#include <init.h>
> +
> +#include <asm/io.h>
> +#include <mach/imx-regs.h>
> +
> +#if defined(CONFIG_ARCH_IMX25) || defined(CONFIG_ARCH_IMX35)
> +static int imx_boot_save_loc(void)
> +{
> +	const char *bootloc = NULL;
> +	uint32_t reg;
> +
> +	reg = readl(IMX_CCM_BASE + CCM_RCSR);
> +
> +	switch ((reg >> CCM_RCSR_MEM_CTRL_SHIFT) & 0x3) {
> +	case 0x03:		/* SD/MMC, I2C, SPI is the source */
> +		bootloc = "expansion";
> +		break;
> +	case 0x01:		/* NAND is the source */
> +		bootloc = "nand";
> +		break;
> +	case 0x00:		/* NOR, OneNand is the source */
> +		bootloc = "weim";
> +		break;
> +	default:
> +		break;
> +	}

This is not exactly what I meant. Your original "nor" and "mmc" strings
were ok. The boards in the tree do not support booting from eeproms (am
I wrong?), so bootloc has the correct value for the boards in the tree.
Now with this patch we have a situation that when somebody adds a board which
can boot from eeprom, he will update this function and "expansion" will
become "mmc" or "eeprom" breaking your environment scripts.

> +
> +	if (bootloc) {
> +		setenv("boot_loc", bootloc);
> +		export("boot_loc");
> +	}

I just asked two collegues about the meaning of a boot_location variable
and they both said this would be the variable where Linux should be
booted from. Maybe barebox_location is a better name?

I already took the first two patches, so you don't have to resend them.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

Re: [PATCH v2 3/3] imx(25, 35): save boot location into $boot_loc env.

[Marc Reilly]

Hi Sascha,


> > +
> > +#if defined(CONFIG_ARCH_IMX25) || defined(CONFIG_ARCH_IMX35)
> > +static int imx_boot_save_loc(void)
> > +{
> > +	const char *bootloc = NULL;
> > +	uint32_t reg;
> > +
> > +	reg = readl(IMX_CCM_BASE + CCM_RCSR);
> > +
> > +	switch ((reg >> CCM_RCSR_MEM_CTRL_SHIFT) & 0x3) {
> > +	case 0x03:		/* SD/MMC, I2C, SPI is the source */
> > +		bootloc = "expansion";
> > +		break;
> > +	case 0x01:		/* NAND is the source */
> > +		bootloc = "nand";
> > +		break;
> > +	case 0x00:		/* NOR, OneNand is the source */
> > +		bootloc = "weim";
> > +		break;
> > +	default:
> > +		break;
> > +	}
> 
> This is not exactly what I meant. Your original "nor" and "mmc" strings
> were ok. The boards in the tree do not support booting from eeproms (am
> I wrong?), 

I don't think any existing boards have the hardware, although I think barebox 
could handle it from a SW point of view. From memory the only source not 
copied by out the boot rom is NOR.

> so bootloc has the correct value for the boards in the tree.
> Now with this patch we have a situation that when somebody adds a board
> which can boot from eeprom, he will update this function and "expansion"
> will become "mmc" or "eeprom" breaking your environment scripts.

I'll do another round where I handle all of them.. (like I should have done 
before :) 

nor onenand nand mmc prom_i2c prom_spi -- do these names sound ok for 
everyone? 

> 
> > +
> > +	if (bootloc) {
> > +		setenv("boot_loc", bootloc);
> > +		export("boot_loc");
> > +	}
> 
> I just asked two collegues about the meaning of a boot_location variable
> and they both said this would be the variable where Linux should be
> booted from. 

Isn't that already kernel_loc?

> Maybe barebox_location is a better name?

Then how about boot_source? (or boot_src). Or boot_media? These seem to me to 
be a better fit, but I don't have any colleagues to bounce them off :) 
I'm also happy with barebox_loc, so no dramas.

> 
> I already took the first two patches, so you don't have to resend them.

Just to check - did you take from v1 or v2? in v2 For the second patch in V2 I 
also added the MEM_TYPE_SHIFT defines..

Cheers
Marc

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

Re: [PATCH v2 3/3] imx(25, 35): save boot location into $boot_loc env.

[Eric Bénard]

Hi Marc,

On 11/01/2011 11:34, Marc Reilly wrote:
> nor onenand nand mmc prom_i2c prom_spi -- do these names sound ok for
> everyone?
>
I think "i2c" & "spi" are enough, no need of "prom_" prefix.

Eric

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

Re: [PATCH v2 3/3] imx(25,35): save boot location into $boot_loc env.

[Sascha Hauer]

On Tue, Jan 11, 2011 at 09:34:35PM +1100, Marc Reilly wrote:
> Hi Sascha,
> 
> 
> > > +
> > > +#if defined(CONFIG_ARCH_IMX25) || defined(CONFIG_ARCH_IMX35)
> > > +static int imx_boot_save_loc(void)
> > > +{
> > > +	const char *bootloc = NULL;
> > > +	uint32_t reg;
> > > +
> > > +	reg = readl(IMX_CCM_BASE + CCM_RCSR);
> > > +
> > > +	switch ((reg >> CCM_RCSR_MEM_CTRL_SHIFT) & 0x3) {
> > > +	case 0x03:		/* SD/MMC, I2C, SPI is the source */
> > > +		bootloc = "expansion";
> > > +		break;
> > > +	case 0x01:		/* NAND is the source */
> > > +		bootloc = "nand";
> > > +		break;
> > > +	case 0x00:		/* NOR, OneNand is the source */
> > > +		bootloc = "weim";
> > > +		break;
> > > +	default:
> > > +		break;
> > > +	}
> > 
> > This is not exactly what I meant. Your original "nor" and "mmc" strings
> > were ok. The boards in the tree do not support booting from eeproms (am
> > I wrong?), 
> 
> I don't think any existing boards have the hardware, although I think barebox 
> could handle it from a SW point of view. From memory the only source not 
> copied by out the boot rom is NOR.
> 
> > so bootloc has the correct value for the boards in the tree.
> > Now with this patch we have a situation that when somebody adds a board
> > which can boot from eeprom, he will update this function and "expansion"
> > will become "mmc" or "eeprom" breaking your environment scripts.
> 
> I'll do another round where I handle all of them.. (like I should have done 
> before :)

Great. That's a good way to end this discussion with everyone being
satisfied ;)

> 
> nor onenand nand mmc prom_i2c prom_spi -- do these names sound ok for 
> everyone?

I agree with Eric about the prefixes.

> 
> > 
> > > +
> > > +	if (bootloc) {
> > > +		setenv("boot_loc", bootloc);
> > > +		export("boot_loc");
> > > +	}
> > 
> > I just asked two collegues about the meaning of a boot_location variable
> > and they both said this would be the variable where Linux should be
> > booted from. 
> 
> Isn't that already kernel_loc?
> 
> > Maybe barebox_location is a better name?
> 
> Then how about boot_source? (or boot_src). Or boot_media? These seem to me to 
> be a better fit, but I don't have any colleagues to bounce them off :)

boot_source was my first idea aswell and I already asked my collegues
about it. For them boot_source and boot_location have the same meaning.

> I'm also happy with barebox_loc, so no dramas.

ok, then lets go for it.

> 
> > 
> > I already took the first two patches, so you don't have to resend them.
> 
> Just to check - did you take from v1 or v2? in v2 For the second patch in V2 I 
> also added the MEM_TYPE_SHIFT defines..

Oh, I took v1 I guess. I didn't realize they differ.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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