| Message ID | 1408536865-6132-1-git-send-email-crosthwaite.peter@gmail.com |
|---|---|
| State | Deferred |
| Delegated to: | Michal Simek |
| Headers | show |
Ping! On Wed, Aug 20, 2014 at 10:14 PM, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote: > To better enable debug of u-boot itself (in particular SPL). To debug > u-boot you want to put your board in JTAG boot mode for quick recompile > and elf downloads via the JTAG debugger. Yet you may still want to boot > from a persistent storage media (SD or QSPI). Allow override of the > boot mode pins to facilitate this. > > Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> > --- > arch/arm/cpu/armv7/zynq/slcr.c | 4 ++++ > doc/README.zynq | 6 ++++++ > 2 files changed, 10 insertions(+) > > diff --git a/arch/arm/cpu/armv7/zynq/slcr.c b/arch/arm/cpu/armv7/zynq/slcr.c > index 934ccc3..26e02b8 100644 > --- a/arch/arm/cpu/armv7/zynq/slcr.c > +++ b/arch/arm/cpu/armv7/zynq/slcr.c > @@ -155,8 +155,12 @@ void zynq_slcr_devcfg_enable(void) > > u32 zynq_slcr_get_boot_mode(void) > { > +#ifdef CONFIG_ZYNQ_BM_FORCE > + return CONFIG_ZYNQ_BM_FORCE; > +#else > /* Get the bootmode register value */ > return readl(&slcr_base->boot_mode); > +#endif > } > > u32 zynq_slcr_get_idcode(void) > diff --git a/doc/README.zynq b/doc/README.zynq > index 043c970..70be7ae 100644 > --- a/doc/README.zynq > +++ b/doc/README.zynq > @@ -52,6 +52,12 @@ board_late_init() will read the bootmode values using slcr bootmode register > at runtime and assign the modeboot variable to specific bootmode string which > is intern used in autoboot. > > +This value can be overridden at compile time with the define > +CONFIG_ZYNQ_BM_FORCE. This is useful for debugging a u-boot elf downloaded > +over JTAG. E.g. The board will be jumpered for ZYNQ_BM_JTAG, but you can > +force u-boot to attempt an SD or QSPI boot should that be what you want to > +debug. > + > SLCR bootmode register Bit[3:0] values > #define ZYNQ_BM_NOR 0x02 > #define ZYNQ_BM_SD 0x05 > -- > 1.9.1 >
On 08/28/2014 01:02 PM, Peter Crosthwaite wrote: > Ping! > > On Wed, Aug 20, 2014 at 10:14 PM, Peter Crosthwaite > <crosthwaitepeter@gmail.com> wrote: >> To better enable debug of u-boot itself (in particular SPL). To debug >> u-boot you want to put your board in JTAG boot mode for quick recompile >> and elf downloads via the JTAG debugger. Yet you may still want to boot >> from a persistent storage media (SD or QSPI). Allow override of the >> boot mode pins to facilitate this. >> >> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> >> --- >> arch/arm/cpu/armv7/zynq/slcr.c | 4 ++++ >> doc/README.zynq | 6 ++++++ >> 2 files changed, 10 insertions(+) >> >> diff --git a/arch/arm/cpu/armv7/zynq/slcr.c b/arch/arm/cpu/armv7/zynq/slcr.c >> index 934ccc3..26e02b8 100644 >> --- a/arch/arm/cpu/armv7/zynq/slcr.c >> +++ b/arch/arm/cpu/armv7/zynq/slcr.c >> @@ -155,8 +155,12 @@ void zynq_slcr_devcfg_enable(void) >> >> u32 zynq_slcr_get_boot_mode(void) >> { >> +#ifdef CONFIG_ZYNQ_BM_FORCE >> + return CONFIG_ZYNQ_BM_FORCE; >> +#else >> /* Get the bootmode register value */ >> return readl(&slcr_base->boot_mode); >> +#endif >> } >> >> u32 zynq_slcr_get_idcode(void) >> diff --git a/doc/README.zynq b/doc/README.zynq >> index 043c970..70be7ae 100644 >> --- a/doc/README.zynq >> +++ b/doc/README.zynq >> @@ -52,6 +52,12 @@ board_late_init() will read the bootmode values using slcr bootmode register >> at runtime and assign the modeboot variable to specific bootmode string which >> is intern used in autoboot. >> >> +This value can be overridden at compile time with the define >> +CONFIG_ZYNQ_BM_FORCE. This is useful for debugging a u-boot elf downloaded >> +over JTAG. E.g. The board will be jumpered for ZYNQ_BM_JTAG, but you can >> +force u-boot to attempt an SD or QSPI boot should that be what you want to >> +debug. >> + I understand the purpose of this patch but I don't think it is useful to add and support this option. Someone else could come with reading GPIO for this purpose, etc which is just the same case. I am happy to add the patch which shows bootmode register content or any ? : logic around in debug() print but adding new CONFIG_ option is just too much. (Also every new config option should be properly documented). The second option is to setup spl_boot_device in arch zynq as weak function and overwrite it. Then you can setup whatever you like in your board file. Thanks, Michal
On 28 August 2014 17:28, Michal Simek <michal.simek@xilinx.com> wrote: > On 08/28/2014 01:02 PM, Peter Crosthwaite wrote: >> Ping! >> >> On Wed, Aug 20, 2014 at 10:14 PM, Peter Crosthwaite >> <crosthwaitepeter@gmail.com> wrote: >>> To better enable debug of u-boot itself (in particular SPL). To debug >>> u-boot you want to put your board in JTAG boot mode for quick recompile >>> and elf downloads via the JTAG debugger. Yet you may still want to boot >>> from a persistent storage media (SD or QSPI). Allow override of the >>> boot mode pins to facilitate this. >>> >>> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> >>> --- >>> arch/arm/cpu/armv7/zynq/slcr.c | 4 ++++ >>> doc/README.zynq | 6 ++++++ >>> 2 files changed, 10 insertions(+) >>> >>> diff --git a/arch/arm/cpu/armv7/zynq/slcr.c b/arch/arm/cpu/armv7/zynq/slcr.c >>> index 934ccc3..26e02b8 100644 >>> --- a/arch/arm/cpu/armv7/zynq/slcr.c >>> +++ b/arch/arm/cpu/armv7/zynq/slcr.c >>> @@ -155,8 +155,12 @@ void zynq_slcr_devcfg_enable(void) >>> >>> u32 zynq_slcr_get_boot_mode(void) >>> { >>> +#ifdef CONFIG_ZYNQ_BM_FORCE >>> + return CONFIG_ZYNQ_BM_FORCE; >>> +#else >>> /* Get the bootmode register value */ >>> return readl(&slcr_base->boot_mode); >>> +#endif >>> } >>> >>> u32 zynq_slcr_get_idcode(void) >>> diff --git a/doc/README.zynq b/doc/README.zynq >>> index 043c970..70be7ae 100644 >>> --- a/doc/README.zynq >>> +++ b/doc/README.zynq >>> @@ -52,6 +52,12 @@ board_late_init() will read the bootmode values using slcr bootmode register >>> at runtime and assign the modeboot variable to specific bootmode string which >>> is intern used in autoboot. >>> >>> +This value can be overridden at compile time with the define >>> +CONFIG_ZYNQ_BM_FORCE. This is useful for debugging a u-boot elf downloaded >>> +over JTAG. E.g. The board will be jumpered for ZYNQ_BM_JTAG, but you can >>> +force u-boot to attempt an SD or QSPI boot should that be what you want to >>> +debug. >>> + > > I understand the purpose of this patch but I don't think it is useful to add and support this option. > Someone else could come with reading GPIO for this purpose, etc which is just the same case. > > I am happy to add the patch which shows bootmode register content or any ? : logic around > in debug() print but adding new CONFIG_ option is just too much. > (Also every new config option should be properly documented). > > The second option is to setup spl_boot_device in arch zynq as weak function and overwrite it. > Then you can setup whatever you like in your board file. Agreed with Michal, this looks hack to macro, where u can get different elf for with and without that macro. May be we can add one env like "zynq_bm_force" on ENV_SETTINGS of our board file for selecting the respective bootmode at runtime. thanks!
On Thu, Aug 28, 2014 at 9:58 PM, Michal Simek <michal.simek@xilinx.com> wrote: > On 08/28/2014 01:02 PM, Peter Crosthwaite wrote: >> Ping! >> >> On Wed, Aug 20, 2014 at 10:14 PM, Peter Crosthwaite >> <crosthwaitepeter@gmail.com> wrote: >>> To better enable debug of u-boot itself (in particular SPL). To debug >>> u-boot you want to put your board in JTAG boot mode for quick recompile >>> and elf downloads via the JTAG debugger. Yet you may still want to boot >>> from a persistent storage media (SD or QSPI). Allow override of the >>> boot mode pins to facilitate this. >>> >>> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> >>> --- >>> arch/arm/cpu/armv7/zynq/slcr.c | 4 ++++ >>> doc/README.zynq | 6 ++++++ >>> 2 files changed, 10 insertions(+) >>> >>> diff --git a/arch/arm/cpu/armv7/zynq/slcr.c b/arch/arm/cpu/armv7/zynq/slcr.c >>> index 934ccc3..26e02b8 100644 >>> --- a/arch/arm/cpu/armv7/zynq/slcr.c >>> +++ b/arch/arm/cpu/armv7/zynq/slcr.c >>> @@ -155,8 +155,12 @@ void zynq_slcr_devcfg_enable(void) >>> >>> u32 zynq_slcr_get_boot_mode(void) >>> { >>> +#ifdef CONFIG_ZYNQ_BM_FORCE >>> + return CONFIG_ZYNQ_BM_FORCE; >>> +#else >>> /* Get the bootmode register value */ >>> return readl(&slcr_base->boot_mode); >>> +#endif >>> } >>> >>> u32 zynq_slcr_get_idcode(void) >>> diff --git a/doc/README.zynq b/doc/README.zynq >>> index 043c970..70be7ae 100644 >>> --- a/doc/README.zynq >>> +++ b/doc/README.zynq >>> @@ -52,6 +52,12 @@ board_late_init() will read the bootmode values using slcr bootmode register >>> at runtime and assign the modeboot variable to specific bootmode string which >>> is intern used in autoboot. >>> >>> +This value can be overridden at compile time with the define >>> +CONFIG_ZYNQ_BM_FORCE. This is useful for debugging a u-boot elf downloaded >>> +over JTAG. E.g. The board will be jumpered for ZYNQ_BM_JTAG, but you can >>> +force u-boot to attempt an SD or QSPI boot should that be what you want to >>> +debug. >>> + > > I understand the purpose of this patch but I don't think it is useful to add and support this option. > Someone else could come with reading GPIO for this purpose, etc which is just the same case. > > I am happy to add the patch which shows bootmode register content or any ? : logic around > in debug() print but adding new CONFIG_ option is just too much. > (Also every new config option should be properly documented). > I'm not sure what this adds? Where does the debug print get involved? > The second option is to setup spl_boot_device in arch zynq as weak function and overwrite it. > Then you can setup whatever you like in your board file. > Sounds like the winning compromise. Regards, Peter > Thanks, > Michal >
On Thu, Aug 28, 2014 at 10:05 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote: > On 28 August 2014 17:28, Michal Simek <michal.simek@xilinx.com> wrote: >> On 08/28/2014 01:02 PM, Peter Crosthwaite wrote: >>> Ping! >>> >>> On Wed, Aug 20, 2014 at 10:14 PM, Peter Crosthwaite >>> <crosthwaitepeter@gmail.com> wrote: >>>> To better enable debug of u-boot itself (in particular SPL). To debug >>>> u-boot you want to put your board in JTAG boot mode for quick recompile >>>> and elf downloads via the JTAG debugger. Yet you may still want to boot >>>> from a persistent storage media (SD or QSPI). Allow override of the >>>> boot mode pins to facilitate this. >>>> >>>> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> >>>> --- >>>> arch/arm/cpu/armv7/zynq/slcr.c | 4 ++++ >>>> doc/README.zynq | 6 ++++++ >>>> 2 files changed, 10 insertions(+) >>>> >>>> diff --git a/arch/arm/cpu/armv7/zynq/slcr.c b/arch/arm/cpu/armv7/zynq/slcr.c >>>> index 934ccc3..26e02b8 100644 >>>> --- a/arch/arm/cpu/armv7/zynq/slcr.c >>>> +++ b/arch/arm/cpu/armv7/zynq/slcr.c >>>> @@ -155,8 +155,12 @@ void zynq_slcr_devcfg_enable(void) >>>> >>>> u32 zynq_slcr_get_boot_mode(void) >>>> { >>>> +#ifdef CONFIG_ZYNQ_BM_FORCE >>>> + return CONFIG_ZYNQ_BM_FORCE; >>>> +#else >>>> /* Get the bootmode register value */ >>>> return readl(&slcr_base->boot_mode); >>>> +#endif >>>> } >>>> >>>> u32 zynq_slcr_get_idcode(void) >>>> diff --git a/doc/README.zynq b/doc/README.zynq >>>> index 043c970..70be7ae 100644 >>>> --- a/doc/README.zynq >>>> +++ b/doc/README.zynq >>>> @@ -52,6 +52,12 @@ board_late_init() will read the bootmode values using slcr bootmode register >>>> at runtime and assign the modeboot variable to specific bootmode string which >>>> is intern used in autoboot. >>>> >>>> +This value can be overridden at compile time with the define >>>> +CONFIG_ZYNQ_BM_FORCE. This is useful for debugging a u-boot elf downloaded >>>> +over JTAG. E.g. The board will be jumpered for ZYNQ_BM_JTAG, but you can >>>> +force u-boot to attempt an SD or QSPI boot should that be what you want to >>>> +debug. >>>> + >> >> I understand the purpose of this patch but I don't think it is useful to add and support this option. >> Someone else could come with reading GPIO for this purpose, etc which is just the same case. >> >> I am happy to add the patch which shows bootmode register content or any ? : logic around >> in debug() print but adding new CONFIG_ option is just too much. >> (Also every new config option should be properly documented). >> >> The second option is to setup spl_boot_device in arch zynq as weak function and overwrite it. >> Then you can setup whatever you like in your board file. > > Agreed with Michal, this looks hack to macro, where u can get > different elf for with and without that macro. > > May be we can add one env like "zynq_bm_force" on ENV_SETTINGS of our > board file for > selecting the respective bootmode at runtime. > That might help for the fuller versions of u-boot but im trying for a super lightweight SPL (less than 64k for all sections, hopefully 32k) so the goal was to have all env parsing code paths removed. Regards, Peter > thanks! > -- > Jagan.
On 08/28/2014 02:51 PM, Peter Crosthwaite wrote: > On Thu, Aug 28, 2014 at 9:58 PM, Michal Simek <michal.simek@xilinx.com> wrote: >> On 08/28/2014 01:02 PM, Peter Crosthwaite wrote: >>> Ping! >>> >>> On Wed, Aug 20, 2014 at 10:14 PM, Peter Crosthwaite >>> <crosthwaitepeter@gmail.com> wrote: >>>> To better enable debug of u-boot itself (in particular SPL). To debug >>>> u-boot you want to put your board in JTAG boot mode for quick recompile >>>> and elf downloads via the JTAG debugger. Yet you may still want to boot >>>> from a persistent storage media (SD or QSPI). Allow override of the >>>> boot mode pins to facilitate this. >>>> >>>> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> >>>> --- >>>> arch/arm/cpu/armv7/zynq/slcr.c | 4 ++++ >>>> doc/README.zynq | 6 ++++++ >>>> 2 files changed, 10 insertions(+) >>>> >>>> diff --git a/arch/arm/cpu/armv7/zynq/slcr.c b/arch/arm/cpu/armv7/zynq/slcr.c >>>> index 934ccc3..26e02b8 100644 >>>> --- a/arch/arm/cpu/armv7/zynq/slcr.c >>>> +++ b/arch/arm/cpu/armv7/zynq/slcr.c >>>> @@ -155,8 +155,12 @@ void zynq_slcr_devcfg_enable(void) >>>> >>>> u32 zynq_slcr_get_boot_mode(void) >>>> { >>>> +#ifdef CONFIG_ZYNQ_BM_FORCE >>>> + return CONFIG_ZYNQ_BM_FORCE; >>>> +#else >>>> /* Get the bootmode register value */ >>>> return readl(&slcr_base->boot_mode); >>>> +#endif >>>> } >>>> >>>> u32 zynq_slcr_get_idcode(void) >>>> diff --git a/doc/README.zynq b/doc/README.zynq >>>> index 043c970..70be7ae 100644 >>>> --- a/doc/README.zynq >>>> +++ b/doc/README.zynq >>>> @@ -52,6 +52,12 @@ board_late_init() will read the bootmode values using slcr bootmode register >>>> at runtime and assign the modeboot variable to specific bootmode string which >>>> is intern used in autoboot. >>>> >>>> +This value can be overridden at compile time with the define >>>> +CONFIG_ZYNQ_BM_FORCE. This is useful for debugging a u-boot elf downloaded >>>> +over JTAG. E.g. The board will be jumpered for ZYNQ_BM_JTAG, but you can >>>> +force u-boot to attempt an SD or QSPI boot should that be what you want to >>>> +debug. >>>> + >> >> I understand the purpose of this patch but I don't think it is useful to add and support this option. >> Someone else could come with reading GPIO for this purpose, etc which is just the same case. >> >> I am happy to add the patch which shows bootmode register content or any ? : logic around >> in debug() print but adding new CONFIG_ option is just too much. >> (Also every new config option should be properly documented). >> > > I'm not sure what this adds? Where does the debug print get involved? I expect when you start to debug things the first thing you do is #define DEBUG which convert all debug() to printf() and you can see more verbose logs. From that logs you will see what SPL does and having one log about choosing boot mode can be helpful. > >> The second option is to setup spl_boot_device in arch zynq as weak function and overwrite it. >> Then you can setup whatever you like in your board file. >> > > Sounds like the winning compromise. Great. Please send the patch for it. Thanks, Michal
diff --git a/arch/arm/cpu/armv7/zynq/slcr.c b/arch/arm/cpu/armv7/zynq/slcr.c index 934ccc3..26e02b8 100644 --- a/arch/arm/cpu/armv7/zynq/slcr.c +++ b/arch/arm/cpu/armv7/zynq/slcr.c @@ -155,8 +155,12 @@ void zynq_slcr_devcfg_enable(void) u32 zynq_slcr_get_boot_mode(void) { +#ifdef CONFIG_ZYNQ_BM_FORCE + return CONFIG_ZYNQ_BM_FORCE; +#else /* Get the bootmode register value */ return readl(&slcr_base->boot_mode); +#endif } u32 zynq_slcr_get_idcode(void) diff --git a/doc/README.zynq b/doc/README.zynq index 043c970..70be7ae 100644 --- a/doc/README.zynq +++ b/doc/README.zynq @@ -52,6 +52,12 @@ board_late_init() will read the bootmode values using slcr bootmode register at runtime and assign the modeboot variable to specific bootmode string which is intern used in autoboot. +This value can be overridden at compile time with the define +CONFIG_ZYNQ_BM_FORCE. This is useful for debugging a u-boot elf downloaded +over JTAG. E.g. The board will be jumpered for ZYNQ_BM_JTAG, but you can +force u-boot to attempt an SD or QSPI boot should that be what you want to +debug. + SLCR bootmode register Bit[3:0] values #define ZYNQ_BM_NOR 0x02 #define ZYNQ_BM_SD 0x05
To better enable debug of u-boot itself (in particular SPL). To debug u-boot you want to put your board in JTAG boot mode for quick recompile and elf downloads via the JTAG debugger. Yet you may still want to boot from a persistent storage media (SD or QSPI). Allow override of the boot mode pins to facilitate this. Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com> --- arch/arm/cpu/armv7/zynq/slcr.c | 4 ++++ doc/README.zynq | 6 ++++++ 2 files changed, 10 insertions(+)