| Message ID | 1460417838-22343-13-git-send-email-d-allred@ti.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 11, 2016 at 06:37:14PM -0500, Daniel Allred wrote: > Update the CPU string output so that the device > type is now included as part of the CPU string that > is printed as the SPL or u-boot comes up. This update > adds a suffix of the form "-GP" or "-HS" for production > devices, so that general purpose (GP) and high security > (HS) can be distiguished. Applies to all OMAP5 variants. When I'm building for AM437x HS and running on the device I don't see that output. It seems like there is something funny going on with CONFIG_SPL_DISPLAY_PRINT. Even though this definition is activated in ti_omap4_common.h and ti_omap5_common.h it is not seen by preloader_console_init() in spl.c, hence the function that prints the chip-type/rev specifics never gets invoked. Madan, can you please confirm that behavior on your end? Thanks, -- Andreas Dannenberg Texas Instruments Inc > > Signed-off-by: Daniel Allred <d-allred@ti.com> > Signed-off-by: Madan Srinivas <madans@ti.com> > --- > arch/arm/cpu/armv7/omap-common/hwinit-common.c | 22 ++++++++++++++++++++-- > arch/arm/include/asm/arch-omap4/cpu.h | 7 +++++++ > arch/arm/include/asm/arch-omap5/cpu.h | 7 +++++++ > 3 files changed, 34 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/cpu/armv7/omap-common/hwinit-common.c b/arch/arm/cpu/armv7/omap-common/hwinit-common.c > index 01c2d57..078bdd8 100644 > --- a/arch/arm/cpu/armv7/omap-common/hwinit-common.c > +++ b/arch/arm/cpu/armv7/omap-common/hwinit-common.c > @@ -65,12 +65,30 @@ static void omap_rev_string(void) > u32 major_rev = (omap_rev & 0x00000F00) >> 8; > u32 minor_rev = (omap_rev & 0x000000F0) >> 4; > > + const char *sec_s; > + > + switch (get_device_type()) { > + case TST_DEVICE: > + sec_s = "TST"; > + break; > + case EMU_DEVICE: > + sec_s = "EMU"; > + break; > + case HS_DEVICE: > + sec_s = "HS"; > + break; > + case GP_DEVICE: > + sec_s = "GP"; > + break; > + default: > + sec_s = "?"; > + } > + > if (soc_variant) > printf("OMAP"); > else > printf("DRA"); > - printf("%x ES%x.%x\n", omap_variant, major_rev, > - minor_rev); > + printf("%x-%s ES%x.%x\n", omap_variant, sec_s, major_rev, minor_rev); > } > > #ifdef CONFIG_SPL_BUILD > diff --git a/arch/arm/include/asm/arch-omap4/cpu.h b/arch/arm/include/asm/arch-omap4/cpu.h > index f7595ae..34609b9 100644 > --- a/arch/arm/include/asm/arch-omap4/cpu.h > +++ b/arch/arm/include/asm/arch-omap4/cpu.h > @@ -59,6 +59,13 @@ struct watchdog { > #define TCLR_AR (0x1 << 1) > #define TCLR_PRE (0x1 << 5) > > +/* device type */ > +#define DEVICE_MASK (BIT(8) | BIT(9) | BIT(10)) > +#define TST_DEVICE 0x0 > +#define EMU_DEVICE 0x1 > +#define HS_DEVICE 0x2 > +#define GP_DEVICE 0x3 > + > /* I2C base */ > #define I2C_BASE1 (OMAP44XX_L4_PER_BASE + 0x70000) > #define I2C_BASE2 (OMAP44XX_L4_PER_BASE + 0x72000) > diff --git a/arch/arm/include/asm/arch-omap5/cpu.h b/arch/arm/include/asm/arch-omap5/cpu.h > index b1513e9..1d53da6 100644 > --- a/arch/arm/include/asm/arch-omap5/cpu.h > +++ b/arch/arm/include/asm/arch-omap5/cpu.h > @@ -63,6 +63,13 @@ struct watchdog { > #define TCLR_AR (0x1 << 1) > #define TCLR_PRE (0x1 << 5) > > +/* device type */ > +#define DEVICE_MASK (BIT(8) | BIT(9) | BIT(10)) > +#define TST_DEVICE 0x0 > +#define EMU_DEVICE 0x1 > +#define HS_DEVICE 0x2 > +#define GP_DEVICE 0x3 > + > /* I2C base */ > #define I2C_BASE1 (OMAP54XX_L4_PER_BASE + 0x70000) > #define I2C_BASE2 (OMAP54XX_L4_PER_BASE + 0x72000) > -- > 1.9.1 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Tue, Apr 19, 2016 at 11:26:30AM -0500, Andreas Dannenberg wrote: > On Mon, Apr 11, 2016 at 06:37:14PM -0500, Daniel Allred wrote: > > Update the CPU string output so that the device > > type is now included as part of the CPU string that > > is printed as the SPL or u-boot comes up. This update > > adds a suffix of the form "-GP" or "-HS" for production > > devices, so that general purpose (GP) and high security > > (HS) can be distiguished. Applies to all OMAP5 variants. > > When I'm building for AM437x HS and running on the device I don't see > that output. It seems like there is something funny going on with > CONFIG_SPL_DISPLAY_PRINT. Even though this definition is activated in > ti_omap4_common.h and ti_omap5_common.h it is not seen by > preloader_console_init() in spl.c, hence the function that prints the > chip-type/rev specifics never gets invoked. So when I run the patches on actual DRA72x HS and DRA74x HS hardware I'll get the device name/type output by SPL as expected so that piece works. However this patch's commit message implies the same should also work on AM437x HS which it doesn't. I don't have AM437x non-secure hardware at my desk but I looked at some boot logs from our test farms and I also don't see the device ID output by SPL so that may be just how it currently is implemented generally for AM437* and has nothing to do with the patch discussed here. Madan, please comment/chime-in as needed. Tested-by: Andreas Dannenberg <dannenberg@ti.com> -- Andreas Dannenberg Texas Instruments Inc _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 4/21/2016 12:55 PM, Andreas Dannenberg wrote: > On Tue, Apr 19, 2016 at 11:26:30AM -0500, Andreas Dannenberg wrote: >> On Mon, Apr 11, 2016 at 06:37:14PM -0500, Daniel Allred wrote: >>> Update the CPU string output so that the device >>> type is now included as part of the CPU string that >>> is printed as the SPL or u-boot comes up. This update >>> adds a suffix of the form "-GP" or "-HS" for production >>> devices, so that general purpose (GP) and high security >>> (HS) can be distiguished. Applies to all OMAP5 variants. >> >> When I'm building for AM437x HS and running on the device I don't see >> that output. It seems like there is something funny going on with >> CONFIG_SPL_DISPLAY_PRINT. Even though this definition is activated in >> ti_omap4_common.h and ti_omap5_common.h it is not seen by >> preloader_console_init() in spl.c, hence the function that prints the >> chip-type/rev specifics never gets invoked. > > So when I run the patches on actual DRA72x HS and DRA74x HS hardware > I'll get the device name/type output by SPL as expected so that piece > works. However this patch's commit message implies the same should also > work on AM437x HS which it doesn't. I don't have AM437x non-secure > hardware at my desk but I looked at some boot logs from our test farms > and I also don't see the device ID output by SPL so that may be just how > it currently is implemented generally for AM437* and has nothing to do > with the patch discussed here. This hwinit-common.c is not used by the AM335x/AM437x parts, hence the statement "Applies to all OMAP5 variants" in the commit message. The omap4/5 use in the commit header is because the omap4 cpu.h header file had to be updated in order to not break omap4 builds (because those builds DO use this hwinit-common.c). Daniel > > Madan, please comment/chime-in as needed. > > > Tested-by: Andreas Dannenberg <dannenberg@ti.com> > > -- > Andreas Dannenberg > Texas Instruments Inc > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Thu, Apr 21, 2016 at 01:01:43PM -0500, Allred, Daniel wrote: > On 4/21/2016 12:55 PM, Andreas Dannenberg wrote: > >On Tue, Apr 19, 2016 at 11:26:30AM -0500, Andreas Dannenberg wrote: > >>On Mon, Apr 11, 2016 at 06:37:14PM -0500, Daniel Allred wrote: > >>>Update the CPU string output so that the device > >>>type is now included as part of the CPU string that > >>>is printed as the SPL or u-boot comes up. This update > >>>adds a suffix of the form "-GP" or "-HS" for production > >>>devices, so that general purpose (GP) and high security > >>>(HS) can be distiguished. Applies to all OMAP5 variants. > >> > >>When I'm building for AM437x HS and running on the device I don't see > >>that output. It seems like there is something funny going on with > >>CONFIG_SPL_DISPLAY_PRINT. Even though this definition is activated in > >>ti_omap4_common.h and ti_omap5_common.h it is not seen by > >>preloader_console_init() in spl.c, hence the function that prints the > >>chip-type/rev specifics never gets invoked. > > > >So when I run the patches on actual DRA72x HS and DRA74x HS hardware > >I'll get the device name/type output by SPL as expected so that piece > >works. However this patch's commit message implies the same should also > >work on AM437x HS which it doesn't. I don't have AM437x non-secure > >hardware at my desk but I looked at some boot logs from our test farms > >and I also don't see the device ID output by SPL so that may be just how > >it currently is implemented generally for AM437* and has nothing to do > >with the patch discussed here. > This hwinit-common.c is not used by the AM335x/AM437x parts, hence the > statement "Applies to all OMAP5 variants" in the commit message. The omap4/5 > use in the commit header is because the omap4 cpu.h header file had to be > updated in order to not break omap4 builds (because those builds DO use this > hwinit-common.c). Daniel, thanks for clarifying/confirming my suspicion. Then I'm okay with this patch. Regards, -- Andreas Dannenberg Texas Instruments Inc _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Thu, Apr 21, 2016 at 04:27:42PM -0400, Tom Rini wrote: > On Thu, Apr 21, 2016 at 01:59:48PM -0500, Andreas Dannenberg wrote: > > On Thu, Apr 21, 2016 at 01:01:43PM -0500, Allred, Daniel wrote: > > > On 4/21/2016 12:55 PM, Andreas Dannenberg wrote: > > > >On Tue, Apr 19, 2016 at 11:26:30AM -0500, Andreas Dannenberg wrote: > > > >>On Mon, Apr 11, 2016 at 06:37:14PM -0500, Daniel Allred wrote: > > > >>>Update the CPU string output so that the device > > > >>>type is now included as part of the CPU string that > > > >>>is printed as the SPL or u-boot comes up. This update > > > >>>adds a suffix of the form "-GP" or "-HS" for production > > > >>>devices, so that general purpose (GP) and high security > > > >>>(HS) can be distiguished. Applies to all OMAP5 variants. > > > >> > > > >>When I'm building for AM437x HS and running on the device I don't see > > > >>that output. It seems like there is something funny going on with > > > >>CONFIG_SPL_DISPLAY_PRINT. Even though this definition is activated in > > > >>ti_omap4_common.h and ti_omap5_common.h it is not seen by > > > >>preloader_console_init() in spl.c, hence the function that prints the > > > >>chip-type/rev specifics never gets invoked. > > > > > > > >So when I run the patches on actual DRA72x HS and DRA74x HS hardware > > > >I'll get the device name/type output by SPL as expected so that piece > > > >works. However this patch's commit message implies the same should also > > > >work on AM437x HS which it doesn't. I don't have AM437x non-secure > > > >hardware at my desk but I looked at some boot logs from our test farms > > > >and I also don't see the device ID output by SPL so that may be just how > > > >it currently is implemented generally for AM437* and has nothing to do > > > >with the patch discussed here. > > > This hwinit-common.c is not used by the AM335x/AM437x parts, hence the > > > statement "Applies to all OMAP5 variants" in the commit message. The omap4/5 > > > use in the commit header is because the omap4 cpu.h header file had to be > > > updated in order to not break omap4 builds (because those builds DO use this > > > hwinit-common.c). > > > > Daniel, > > thanks for clarifying/confirming my suspicion. Then I'm okay with this patch. > > Can we do a follow-up that moves this otherwise common code into the > rest of the families? Hi Tom, just to make sure I understand your suggestion correctly, this is about a behind the scenes optimization to remove the code duplication we currently have in .../asm/arch-omap(4|5)/cpu.h, rather than making the CPU string output as part of SPL work on all of our (TI) platforms, yes? Thanks and Regards, -- Andreas Dannenberg Texas Instruments Inc _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On Thu, Apr 21, 2016 at 07:38:17PM -0400, Tom Rini wrote: > On Thu, Apr 21, 2016 at 05:56:11PM -0500, Andreas Dannenberg wrote: > > On Thu, Apr 21, 2016 at 04:27:42PM -0400, Tom Rini wrote: > > > On Thu, Apr 21, 2016 at 01:59:48PM -0500, Andreas Dannenberg wrote: > > > > On Thu, Apr 21, 2016 at 01:01:43PM -0500, Allred, Daniel wrote: > > > > > On 4/21/2016 12:55 PM, Andreas Dannenberg wrote: > > > > > >On Tue, Apr 19, 2016 at 11:26:30AM -0500, Andreas Dannenberg wrote: > > > > > >>On Mon, Apr 11, 2016 at 06:37:14PM -0500, Daniel Allred wrote: > > > > > >>>Update the CPU string output so that the device > > > > > >>>type is now included as part of the CPU string that > > > > > >>>is printed as the SPL or u-boot comes up. This update > > > > > >>>adds a suffix of the form "-GP" or "-HS" for production > > > > > >>>devices, so that general purpose (GP) and high security > > > > > >>>(HS) can be distiguished. Applies to all OMAP5 variants. > > > > > >> > > > > > >>When I'm building for AM437x HS and running on the device I don't see > > > > > >>that output. It seems like there is something funny going on with > > > > > >>CONFIG_SPL_DISPLAY_PRINT. Even though this definition is activated in > > > > > >>ti_omap4_common.h and ti_omap5_common.h it is not seen by > > > > > >>preloader_console_init() in spl.c, hence the function that prints the > > > > > >>chip-type/rev specifics never gets invoked. > > > > > > > > > > > >So when I run the patches on actual DRA72x HS and DRA74x HS hardware > > > > > >I'll get the device name/type output by SPL as expected so that piece > > > > > >works. However this patch's commit message implies the same should also > > > > > >work on AM437x HS which it doesn't. I don't have AM437x non-secure > > > > > >hardware at my desk but I looked at some boot logs from our test farms > > > > > >and I also don't see the device ID output by SPL so that may be just how > > > > > >it currently is implemented generally for AM437* and has nothing to do > > > > > >with the patch discussed here. > > > > > This hwinit-common.c is not used by the AM335x/AM437x parts, hence the > > > > > statement "Applies to all OMAP5 variants" in the commit message. The omap4/5 > > > > > use in the commit header is because the omap4 cpu.h header file had to be > > > > > updated in order to not break omap4 builds (because those builds DO use this > > > > > hwinit-common.c). > > > > > > > > Daniel, > > > > thanks for clarifying/confirming my suspicion. Then I'm okay with this patch. > > > > > > Can we do a follow-up that moves this otherwise common code into the > > > rest of the families? > > > > Hi Tom, > > just to make sure I understand your suggestion correctly, this is about > > a behind the scenes optimization to remove the code duplication we > > currently have in .../asm/arch-omap(4|5)/cpu.h, rather than making the > > CPU string output as part of SPL work on all of our (TI) platforms, yes? > > I want as much consolidate and consistency of output as is both feasible > and practical. Agreed. Consistency and consolidation would make sense here. I just added an item to our internal issue tracker to capture this suggestion but I can't yet comment on when we'll get to it. Thanks and Regards, Andreas _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
diff --git a/arch/arm/cpu/armv7/omap-common/hwinit-common.c b/arch/arm/cpu/armv7/omap-common/hwinit-common.c index 01c2d57..078bdd8 100644 --- a/arch/arm/cpu/armv7/omap-common/hwinit-common.c +++ b/arch/arm/cpu/armv7/omap-common/hwinit-common.c @@ -65,12 +65,30 @@ static void omap_rev_string(void) u32 major_rev = (omap_rev & 0x00000F00) >> 8; u32 minor_rev = (omap_rev & 0x000000F0) >> 4; + const char *sec_s; + + switch (get_device_type()) { + case TST_DEVICE: + sec_s = "TST"; + break; + case EMU_DEVICE: + sec_s = "EMU"; + break; + case HS_DEVICE: + sec_s = "HS"; + break; + case GP_DEVICE: + sec_s = "GP"; + break; + default: + sec_s = "?"; + } + if (soc_variant) printf("OMAP"); else printf("DRA"); - printf("%x ES%x.%x\n", omap_variant, major_rev, - minor_rev); + printf("%x-%s ES%x.%x\n", omap_variant, sec_s, major_rev, minor_rev); } #ifdef CONFIG_SPL_BUILD diff --git a/arch/arm/include/asm/arch-omap4/cpu.h b/arch/arm/include/asm/arch-omap4/cpu.h index f7595ae..34609b9 100644 --- a/arch/arm/include/asm/arch-omap4/cpu.h +++ b/arch/arm/include/asm/arch-omap4/cpu.h @@ -59,6 +59,13 @@ struct watchdog { #define TCLR_AR (0x1 << 1) #define TCLR_PRE (0x1 << 5) +/* device type */ +#define DEVICE_MASK (BIT(8) | BIT(9) | BIT(10)) +#define TST_DEVICE 0x0 +#define EMU_DEVICE 0x1 +#define HS_DEVICE 0x2 +#define GP_DEVICE 0x3 + /* I2C base */ #define I2C_BASE1 (OMAP44XX_L4_PER_BASE + 0x70000) #define I2C_BASE2 (OMAP44XX_L4_PER_BASE + 0x72000) diff --git a/arch/arm/include/asm/arch-omap5/cpu.h b/arch/arm/include/asm/arch-omap5/cpu.h index b1513e9..1d53da6 100644 --- a/arch/arm/include/asm/arch-omap5/cpu.h +++ b/arch/arm/include/asm/arch-omap5/cpu.h @@ -63,6 +63,13 @@ struct watchdog { #define TCLR_AR (0x1 << 1) #define TCLR_PRE (0x1 << 5) +/* device type */ +#define DEVICE_MASK (BIT(8) | BIT(9) | BIT(10)) +#define TST_DEVICE 0x0 +#define EMU_DEVICE 0x1 +#define HS_DEVICE 0x2 +#define GP_DEVICE 0x3 + /* I2C base */ #define I2C_BASE1 (OMAP54XX_L4_PER_BASE + 0x70000) #define I2C_BASE2 (OMAP54XX_L4_PER_BASE + 0x72000)