| Message ID | 1398716258-8420-6-git-send-email-tharvey@gateworks.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
Hi Tim, On 04/28/2014 01:17 PM, Tim Harvey wrote: > Add a common spl.c file to support boot device functions needed for SPL > such as detecting the boot device. > > <snip> > > diff --git a/arch/arm/imx-common/spl.c b/arch/arm/imx-common/spl.c > new file mode 100644 > index 0000000..5c1667c > --- /dev/null > +++ b/arch/arm/imx-common/spl.c > @@ -0,0 +1,79 @@ > > <snip> > > +u32 spl_boot_device(void) > +{ > + struct src *psrc = (struct src *)SRC_BASE_ADDR; > + unsigned reg = readl(&psrc->sbmr1); > + > + /* BOOT_CFG1[7:4] - see IMX6DQRM Table 8-8 */ > + switch ((reg & 0x000000FF) >> 4) { > > <snip> > > + case 0x3: > + /* BOOT_CFG4[2:0] */ > + switch ((reg & 0x07000000) >> 24) { This made me want a language lawyer, although I do see case ranges used in arch/blackfin/cpu/traps.c > + case 0x0 ... 0x4: > + return BOOT_DEVICE_SPI; > + case 0x5 ... 0x7: > + return BOOT_DEVICE_I2C; > + } Regards, Eric
Hi Tim, On 28/04/2014 22:17, Tim Harvey wrote: > Add a common spl.c file to support boot device functions needed for SPL > such as detecting the boot device. > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > --- > v2: > - re-base on top of Masahiro Yamada's consolidation patch: > http://patchwork.ozlabs.org/patch/341817/ > - moved spl_boot_device() and get_boot_mode() into own common imx spl.c file > - use mem-mapped structure for smbr reg access > --- > arch/arm/imx-common/Makefile | 1 + > arch/arm/imx-common/spl.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 80 insertions(+) > create mode 100644 arch/arm/imx-common/spl.c > > diff --git a/arch/arm/imx-common/Makefile b/arch/arm/imx-common/Makefile > index b04dfbb..c97ea48 100644 > --- a/arch/arm/imx-common/Makefile > +++ b/arch/arm/imx-common/Makefile > @@ -16,6 +16,7 @@ obj-$(CONFIG_SYS_I2C_MXC) += i2c-mxv7.o > endif > ifeq ($(SOC),$(filter $(SOC),mx6 mxs)) > obj-y += misc.o > +obj-$(CONFIG_SPL_BUILD) += spl.o > endif > ifeq ($(SOC),$(filter $(SOC),mx6)) > obj-$(CONFIG_CMD_SATA) += sata.o > diff --git a/arch/arm/imx-common/spl.c b/arch/arm/imx-common/spl.c > new file mode 100644 > index 0000000..5c1667c > --- /dev/null > +++ b/arch/arm/imx-common/spl.c > @@ -0,0 +1,79 @@ > +/* > + * Copyright (C) 2014 Gateworks Corporation > + * Author: Tim Harvey <tharvey@gateworks.com> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + Just a little remark. Do you have written the function on your own or have you ported it from Freescale's 2009.08 ? Apart names, it looks similar. If this is the case, you should also add Freescale's Copyright to the header. > +#include <common.h> > +#include <asm/io.h> > +#include <asm/arch/imx-regs.h> > +#include <asm/spl.h> > +#include <spl.h> > + > +#if defined(CONFIG_MX6) > +/* determine boot device from SRC_SBMR1 register (BOOT_CFG[4:1]) */ > +u32 spl_boot_device(void) > +{ > + struct src *psrc = (struct src *)SRC_BASE_ADDR; > + unsigned reg = readl(&psrc->sbmr1); > + > + /* BOOT_CFG1[7:4] - see IMX6DQRM Table 8-8 */ > + switch ((reg & 0x000000FF) >> 4) { > + /* EIM: See 8.5.1, Table 8-9 */ > + case 0x0: > + /* BOOT_CFG1[3]: NOR/OneNAND Selection */ > + if ((reg & 0x00000008) >> 3) > + return BOOT_DEVICE_ONENAND; > + else > + return BOOT_DEVICE_NOR; > + break; > + /* SATA: See 8.5.4, Table 8-20 */ > + case 0x2: > + return BOOT_DEVICE_SATA; > + /* Serial ROM: See 8.5.5.1, Table 8-22 */ > + case 0x3: > + /* BOOT_CFG4[2:0] */ > + switch ((reg & 0x07000000) >> 24) { > + case 0x0 ... 0x4: > + return BOOT_DEVICE_SPI; > + case 0x5 ... 0x7: > + return BOOT_DEVICE_I2C; > + } > + break; > + /* SD/eSD: 8.5.3, Table 8-15 */ > + case 0x4: > + case 0x5: > + return BOOT_DEVICE_MMC1; > + /* MMC/eMMC: 8.5.3 */ > + case 0x6: > + case 0x7: > + return BOOT_DEVICE_MMC1; > + /* NAND Flash: 8.5.2 */ > + case 0x8 ... 0xf: > + return BOOT_DEVICE_NAND; > + } > + return BOOT_DEVICE_NONE; > +} > +#endif > + > +#if defined(CONFIG_SPL_MMC_SUPPORT) > +/* called from spl_mmc to see type of boot mode for storage (RAW or FAT) */ > +u32 spl_boot_mode(void) > +{ > + switch (spl_boot_device()) { > + /* for MMC return either RAW or FAT mode */ > + case BOOT_DEVICE_MMC1: > + case BOOT_DEVICE_MMC2: > +#ifdef CONFIG_SPL_FAT_SUPPORT > + return MMCSD_MODE_FAT; > +#else > + return MMCSD_MODE_RAW; > +#endif > + break; > + default: > + puts("spl: ERROR: unsupported device\n"); > + hang(); > + } > +} > +#endif > Best regards, Stefano Babic
+cc Edward Lin On Mon, May 5, 2014 at 2:14 AM, Stefano Babic <sbabic@denx.de> wrote: > Hi Tim, > > On 28/04/2014 22:17, Tim Harvey wrote: >> Add a common spl.c file to support boot device functions needed for SPL >> such as detecting the boot device. >> >> Signed-off-by: Tim Harvey <tharvey@gateworks.com> >> --- >> v2: >> - re-base on top of Masahiro Yamada's consolidation patch: >> http://patchwork.ozlabs.org/patch/341817/ >> - moved spl_boot_device() and get_boot_mode() into own common imx spl.c file >> - use mem-mapped structure for smbr reg access >> --- >> arch/arm/imx-common/Makefile | 1 + >> arch/arm/imx-common/spl.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 80 insertions(+) >> create mode 100644 arch/arm/imx-common/spl.c >> >> diff --git a/arch/arm/imx-common/Makefile b/arch/arm/imx-common/Makefile >> index b04dfbb..c97ea48 100644 >> --- a/arch/arm/imx-common/Makefile >> +++ b/arch/arm/imx-common/Makefile >> @@ -16,6 +16,7 @@ obj-$(CONFIG_SYS_I2C_MXC) += i2c-mxv7.o >> endif >> ifeq ($(SOC),$(filter $(SOC),mx6 mxs)) >> obj-y += misc.o >> +obj-$(CONFIG_SPL_BUILD) += spl.o >> endif >> ifeq ($(SOC),$(filter $(SOC),mx6)) >> obj-$(CONFIG_CMD_SATA) += sata.o >> diff --git a/arch/arm/imx-common/spl.c b/arch/arm/imx-common/spl.c >> new file mode 100644 >> index 0000000..5c1667c >> --- /dev/null >> +++ b/arch/arm/imx-common/spl.c >> @@ -0,0 +1,79 @@ >> +/* >> + * Copyright (C) 2014 Gateworks Corporation >> + * Author: Tim Harvey <tharvey@gateworks.com> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + > > Just a little remark. Do you have written the function on your own or > have you ported it from Freescale's 2009.08 ? Apart names, it looks similar. > > If this is the case, you should also add Freescale's Copyright to the > header. Hi Stefano, I've never looked at Freescales 2009.08 code myself (I have an aversion to vendor specific forks of u-boot almost 5 years old heh), but its likely bits of the function came from there originally but my inspiration likely came from Edward Lin's patch a while back. A switch/case seems the logical approach given the bitfield from the IMX6 reference manual with several don't-cares spread around and I felt that was much easier to read than trying to convert it all the shifts and masks. I can add additional copyright's if anyone feels its necessary - I've added Edward to the cc to see if he feels its necessary. Tim > >> +#include <common.h> >> +#include <asm/io.h> >> +#include <asm/arch/imx-regs.h> >> +#include <asm/spl.h> >> +#include <spl.h> >> + >> +#if defined(CONFIG_MX6) >> +/* determine boot device from SRC_SBMR1 register (BOOT_CFG[4:1]) */ >> +u32 spl_boot_device(void) >> +{ >> + struct src *psrc = (struct src *)SRC_BASE_ADDR; >> + unsigned reg = readl(&psrc->sbmr1); >> + >> + /* BOOT_CFG1[7:4] - see IMX6DQRM Table 8-8 */ >> + switch ((reg & 0x000000FF) >> 4) { >> + /* EIM: See 8.5.1, Table 8-9 */ >> + case 0x0: >> + /* BOOT_CFG1[3]: NOR/OneNAND Selection */ >> + if ((reg & 0x00000008) >> 3) >> + return BOOT_DEVICE_ONENAND; >> + else >> + return BOOT_DEVICE_NOR; >> + break; >> + /* SATA: See 8.5.4, Table 8-20 */ >> + case 0x2: >> + return BOOT_DEVICE_SATA; >> + /* Serial ROM: See 8.5.5.1, Table 8-22 */ >> + case 0x3: >> + /* BOOT_CFG4[2:0] */ >> + switch ((reg & 0x07000000) >> 24) { >> + case 0x0 ... 0x4: >> + return BOOT_DEVICE_SPI; >> + case 0x5 ... 0x7: >> + return BOOT_DEVICE_I2C; >> + } >> + break; >> + /* SD/eSD: 8.5.3, Table 8-15 */ >> + case 0x4: >> + case 0x5: >> + return BOOT_DEVICE_MMC1; >> + /* MMC/eMMC: 8.5.3 */ >> + case 0x6: >> + case 0x7: >> + return BOOT_DEVICE_MMC1; >> + /* NAND Flash: 8.5.2 */ >> + case 0x8 ... 0xf: >> + return BOOT_DEVICE_NAND; >> + } >> + return BOOT_DEVICE_NONE; >> +} >> +#endif >> + >> +#if defined(CONFIG_SPL_MMC_SUPPORT) >> +/* called from spl_mmc to see type of boot mode for storage (RAW or FAT) */ >> +u32 spl_boot_mode(void) >> +{ >> + switch (spl_boot_device()) { >> + /* for MMC return either RAW or FAT mode */ >> + case BOOT_DEVICE_MMC1: >> + case BOOT_DEVICE_MMC2: >> +#ifdef CONFIG_SPL_FAT_SUPPORT >> + return MMCSD_MODE_FAT; >> +#else >> + return MMCSD_MODE_RAW; >> +#endif >> + break; >> + default: >> + puts("spl: ERROR: unsupported device\n"); >> + hang(); >> + } >> +} >> +#endif >> > > Best regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > =====================================================================
Hi Tim, On 05/05/2014 17:46, Tim Harvey wrote: >> Just a little remark. Do you have written the function on your own or >> have you ported it from Freescale's 2009.08 ? Apart names, it looks similar. >> >> If this is the case, you should also add Freescale's Copyright to the >> header. > > Hi Stefano, > > I've never looked at Freescales 2009.08 code myself (I have an > aversion to vendor specific forks of u-boot almost 5 years old heh), > but its likely bits of the function came from there originally but my > inspiration likely came from Edward Lin's patch a while back. A > switch/case seems the logical approach given the bitfield from the > IMX6 reference manual with several don't-cares spread around and I > felt that was much easier to read than trying to convert it all the > shifts and masks. This is ok - I want only to be sure that Copyright is maintained if code is taken from somewhere else. > > I can add additional copyright's if anyone feels its necessary - I've > added Edward to the cc to see if he feels its necessary. Ok - if nobody complains, you can leave it as it is now. Best regards, Stefano
On Mon, 5 May 2014 08:46:24 -0700 Tim Harvey <tharvey@gateworks.com> wrote: > +cc Edward Lin > > On Mon, May 5, 2014 at 2:14 AM, Stefano Babic <sbabic@denx.de> wrote: > > Hi Tim, > > ... > >> --- /dev/null > >> +++ b/arch/arm/imx-common/spl.c > >> @@ -0,0 +1,79 @@ > >> +/* > >> + * Copyright (C) 2014 Gateworks Corporation > >> + * Author: Tim Harvey <tharvey@gateworks.com> > >> + * > >> + * SPDX-License-Identifier: GPL-2.0+ > >> + */ > >> + > > > > Just a little remark. Do you have written the function on your own or > > have you ported it from Freescale's 2009.08 ? Apart names, it looks similar. > > > > If this is the case, you should also add Freescale's Copyright to the > > header. > > Hi Stefano, > > I've never looked at Freescales 2009.08 code myself (I have an > aversion to vendor specific forks of u-boot almost 5 years old heh), > but its likely bits of the function came from there originally but my > inspiration likely came from Edward Lin's patch a while back. A > switch/case seems the logical approach given the bitfield from the > IMX6 reference manual with several don't-cares spread around and I > felt that was much easier to read than trying to convert it all the > shifts and masks. > > I can add additional copyright's if anyone feels its necessary - I've > added Edward to the cc to see if he feels its necessary. > > Tim > > > > >> +#include <common.h> > >> +#include <asm/io.h> > >> +#include <asm/arch/imx-regs.h> > >> +#include <asm/spl.h> > >> +#include <spl.h> > >> + > >> +#if defined(CONFIG_MX6) > >> +/* determine boot device from SRC_SBMR1 register (BOOT_CFG[4:1]) */ > >> +u32 spl_boot_device(void) > >> +{ > >> + struct src *psrc = (struct src *)SRC_BASE_ADDR; > >> + unsigned reg = readl(&psrc->sbmr1); > >> + > >> + /* BOOT_CFG1[7:4] - see IMX6DQRM Table 8-8 */ > >> + switch ((reg & 0x000000FF) >> 4) { > >> + /* EIM: See 8.5.1, Table 8-9 */ > >> + case 0x0: > >> + /* BOOT_CFG1[3]: NOR/OneNAND Selection */ > >> + if ((reg & 0x00000008) >> 3) > >> + return BOOT_DEVICE_ONENAND; > >> + else > >> + return BOOT_DEVICE_NOR; > >> + break; > >> + /* SATA: See 8.5.4, Table 8-20 */ > >> + case 0x2: > >> + return BOOT_DEVICE_SATA; > >> + /* Serial ROM: See 8.5.5.1, Table 8-22 */ > >> + case 0x3: > >> + /* BOOT_CFG4[2:0] */ > >> + switch ((reg & 0x07000000) >> 24) { > >> + case 0x0 ... 0x4: > >> + return BOOT_DEVICE_SPI; > >> + case 0x5 ... 0x7: > >> + return BOOT_DEVICE_I2C; > >> + } > >> + break; > >> + /* SD/eSD: 8.5.3, Table 8-15 */ > >> + case 0x4: > >> + case 0x5: > >> + return BOOT_DEVICE_MMC1; > >> + /* MMC/eMMC: 8.5.3 */ > >> + case 0x6: > >> + case 0x7: > >> + return BOOT_DEVICE_MMC1; > >> + /* NAND Flash: 8.5.2 */ > >> + case 0x8 ... 0xf: > >> + return BOOT_DEVICE_NAND; > >> + } > >> + return BOOT_DEVICE_NONE; > >> +} > >> +#endif > >> + > >> +#if defined(CONFIG_SPL_MMC_SUPPORT) > >> +/* called from spl_mmc to see type of boot mode for storage (RAW or FAT) */ > >> +u32 spl_boot_mode(void) > >> +{ > >> + switch (spl_boot_device()) { > >> + /* for MMC return either RAW or FAT mode */ > >> + case BOOT_DEVICE_MMC1: > >> + case BOOT_DEVICE_MMC2: > >> +#ifdef CONFIG_SPL_FAT_SUPPORT > >> + return MMCSD_MODE_FAT; > >> +#else > >> + return MMCSD_MODE_RAW; > >> +#endif > >> + break; > >> + default: > >> + puts("spl: ERROR: unsupported device\n"); > >> + hang(); > >> + } > >> +} > >> +#endif > >> Tim, Stefano, that piece of code indeed originates from Freescale's 2009.08 u-boot. (Edward has changed jobs, so the CC:d address is of no use.) regards, //Tapani ps: Tim, thanks for getting the SPL framework mainlined!
Hi Tim, On 06/05/2014 08:36, Tapani Utriainen wrote: > > Tim, Stefano, > > that piece of code indeed originates from Freescale's 2009.08 u-boot. > (Edward has changed jobs, so the CC:d address is of no use.) I supposed this. Tim, please add Freescale's Copyright, too. Thanks, Stefano Babic
On Tue, May 6, 2014 at 12:55 AM, Stefano Babic <sbabic@denx.de> wrote: > Hi Tim, > > On 06/05/2014 08:36, Tapani Utriainen wrote: > >> >> Tim, Stefano, >> >> that piece of code indeed originates from Freescale's 2009.08 u-boot. >> (Edward has changed jobs, so the CC:d address is of no use.) > > > I supposed this. Tim, please add Freescale's Copyright, too. > > Thanks, > Stefano Babic > ok - will do. Thanks for the heads-up Tapani. Tim
diff --git a/arch/arm/imx-common/Makefile b/arch/arm/imx-common/Makefile index b04dfbb..c97ea48 100644 --- a/arch/arm/imx-common/Makefile +++ b/arch/arm/imx-common/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_SYS_I2C_MXC) += i2c-mxv7.o endif ifeq ($(SOC),$(filter $(SOC),mx6 mxs)) obj-y += misc.o +obj-$(CONFIG_SPL_BUILD) += spl.o endif ifeq ($(SOC),$(filter $(SOC),mx6)) obj-$(CONFIG_CMD_SATA) += sata.o diff --git a/arch/arm/imx-common/spl.c b/arch/arm/imx-common/spl.c new file mode 100644 index 0000000..5c1667c --- /dev/null +++ b/arch/arm/imx-common/spl.c @@ -0,0 +1,79 @@ +/* + * Copyright (C) 2014 Gateworks Corporation + * Author: Tim Harvey <tharvey@gateworks.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <asm/io.h> +#include <asm/arch/imx-regs.h> +#include <asm/spl.h> +#include <spl.h> + +#if defined(CONFIG_MX6) +/* determine boot device from SRC_SBMR1 register (BOOT_CFG[4:1]) */ +u32 spl_boot_device(void) +{ + struct src *psrc = (struct src *)SRC_BASE_ADDR; + unsigned reg = readl(&psrc->sbmr1); + + /* BOOT_CFG1[7:4] - see IMX6DQRM Table 8-8 */ + switch ((reg & 0x000000FF) >> 4) { + /* EIM: See 8.5.1, Table 8-9 */ + case 0x0: + /* BOOT_CFG1[3]: NOR/OneNAND Selection */ + if ((reg & 0x00000008) >> 3) + return BOOT_DEVICE_ONENAND; + else + return BOOT_DEVICE_NOR; + break; + /* SATA: See 8.5.4, Table 8-20 */ + case 0x2: + return BOOT_DEVICE_SATA; + /* Serial ROM: See 8.5.5.1, Table 8-22 */ + case 0x3: + /* BOOT_CFG4[2:0] */ + switch ((reg & 0x07000000) >> 24) { + case 0x0 ... 0x4: + return BOOT_DEVICE_SPI; + case 0x5 ... 0x7: + return BOOT_DEVICE_I2C; + } + break; + /* SD/eSD: 8.5.3, Table 8-15 */ + case 0x4: + case 0x5: + return BOOT_DEVICE_MMC1; + /* MMC/eMMC: 8.5.3 */ + case 0x6: + case 0x7: + return BOOT_DEVICE_MMC1; + /* NAND Flash: 8.5.2 */ + case 0x8 ... 0xf: + return BOOT_DEVICE_NAND; + } + return BOOT_DEVICE_NONE; +} +#endif + +#if defined(CONFIG_SPL_MMC_SUPPORT) +/* called from spl_mmc to see type of boot mode for storage (RAW or FAT) */ +u32 spl_boot_mode(void) +{ + switch (spl_boot_device()) { + /* for MMC return either RAW or FAT mode */ + case BOOT_DEVICE_MMC1: + case BOOT_DEVICE_MMC2: +#ifdef CONFIG_SPL_FAT_SUPPORT + return MMCSD_MODE_FAT; +#else + return MMCSD_MODE_RAW; +#endif + break; + default: + puts("spl: ERROR: unsupported device\n"); + hang(); + } +} +#endif
Add a common spl.c file to support boot device functions needed for SPL such as detecting the boot device. Signed-off-by: Tim Harvey <tharvey@gateworks.com> --- v2: - re-base on top of Masahiro Yamada's consolidation patch: http://patchwork.ozlabs.org/patch/341817/ - moved spl_boot_device() and get_boot_mode() into own common imx spl.c file - use mem-mapped structure for smbr reg access --- arch/arm/imx-common/Makefile | 1 + arch/arm/imx-common/spl.c | 79 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 arch/arm/imx-common/spl.c