| Message ID | 1309352967-5719-7-git-send-email-aneesh@ti.com |
|---|---|
| State | RFC |
| Headers | show |
Hello Aneesh, Aneesh V wrote: > Signed-off-by: Aneesh V <aneesh@ti.com> > --- > arch/arm/cpu/armv7/omap-common/Makefile | 9 ++-- > arch/arm/cpu/armv7/omap-common/spl.c | 56 ++++++++++++++++++++++++++++ > arch/arm/cpu/armv7/omap-common/spl.lds | 62 +++++++++++++++++++++++++++++++ > 3 files changed, 123 insertions(+), 4 deletions(-) > create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c > create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds > [...] > diff --git a/arch/arm/cpu/armv7/omap-common/spl.c b/arch/arm/cpu/armv7/omap-common/spl.c > new file mode 100644 > index 0000000..b5a5f3c > --- /dev/null > +++ b/arch/arm/cpu/armv7/omap-common/spl.c [...] > @@ -0,0 +1,56 @@ > +void board_init_f(ulong dummy) > +{ > + debug(">>board_init_f()\n"); > + relocate_code(CONFIG_SYS_SPL_STACK, &gdata, CONFIG_SYS_SPL_TEXT_BASE); > + debug("<<board_init_f()\n"); This debug printf will never occur ;-) BTW: Do you really need to relocate code? You could just load the 2nd stage loader to ram from board_init_f, or? bye, Heiko
Hi Heiko, On Thursday 30 June 2011 11:31 AM, Heiko Schocher wrote: > Hello Aneesh, > > Aneesh V wrote: >> Signed-off-by: Aneesh V<aneesh@ti.com> >> --- >> arch/arm/cpu/armv7/omap-common/Makefile | 9 ++-- >> arch/arm/cpu/armv7/omap-common/spl.c | 56 ++++++++++++++++++++++++++++ >> arch/arm/cpu/armv7/omap-common/spl.lds | 62 +++++++++++++++++++++++++++++++ >> 3 files changed, 123 insertions(+), 4 deletions(-) >> create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c >> create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds >> > [...] >> diff --git a/arch/arm/cpu/armv7/omap-common/spl.c b/arch/arm/cpu/armv7/omap-common/spl.c >> new file mode 100644 >> index 0000000..b5a5f3c >> --- /dev/null >> +++ b/arch/arm/cpu/armv7/omap-common/spl.c > [...] >> @@ -0,0 +1,56 @@ >> +void board_init_f(ulong dummy) >> +{ >> + debug(">>board_init_f()\n"); >> + relocate_code(CONFIG_SYS_SPL_STACK,&gdata, CONFIG_SYS_SPL_TEXT_BASE); >> + debug("<<board_init_f()\n"); > > This debug printf will never occur ;-) Indeed. Thanks for pointing out. > > BTW: Do you really need to relocate code? You could just load the 2nd > stage loader to ram from board_init_f, or? I am passing the same address as I am executing from as the target for the relocation, so the relocation will not happen, instead BSS will be initialized. That's what I am calling it for. Initially I had my own routine for clearing BSS. Then I decided to re-use it from start.S BTW, this series is not complete as far as SPL support is concerned. It doesn't even build due to many undefined references. It serves only as a prototype for the framework part. br, Aneesh
Dear Aneesh V, Am 30.06.2011 um 08:12 schrieb Aneesh V: > Hi Heiko, > > On Thursday 30 June 2011 11:31 AM, Heiko Schocher wrote: >> Hello Aneesh, >> >> Aneesh V wrote: >>> Signed-off-by: Aneesh V<aneesh@ti.com> >>> --- >>> arch/arm/cpu/armv7/omap-common/Makefile | 9 ++-- >>> arch/arm/cpu/armv7/omap-common/spl.c | 56 ++++++++++++++++++++++++++++ >>> arch/arm/cpu/armv7/omap-common/spl.lds | 62 +++++++++++++++++++++++++++++++ >>> 3 files changed, 123 insertions(+), 4 deletions(-) >>> create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c >>> create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds >>> >> [...] >>> diff --git a/arch/arm/cpu/armv7/omap-common/spl.c b/arch/arm/cpu/armv7/omap-common/spl.c >>> new file mode 100644 >>> index 0000000..b5a5f3c >>> --- /dev/null >>> +++ b/arch/arm/cpu/armv7/omap-common/spl.c >> [...] >>> @@ -0,0 +1,56 @@ >>> +void board_init_f(ulong dummy) >>> +{ >>> + debug(">>board_init_f()\n"); >>> + relocate_code(CONFIG_SYS_SPL_STACK,&gdata, CONFIG_SYS_SPL_TEXT_BASE); >>> + debug("<<board_init_f()\n"); >> >> This debug printf will never occur ;-) > > Indeed. Thanks for pointing out. > >> >> BTW: Do you really need to relocate code? You could just load the 2nd >> stage loader to ram from board_init_f, or? > > I am passing the same address as I am executing from as the target for > the relocation, so the relocation will not happen, instead BSS will be > initialized. That's what I am calling it for. Initially I had my own > routine for clearing BSS. Then I decided to re-use it from start.S So you could just call clear_bss(void) and skip relocate_code. But I think you need to adopt the __bss_start_ofs, __bss_end_ofs markers, cause your linker skript places them in SDRAM. BTW: I think Simon Schwarz is also working on this, can one comment on his first version of patchset? regards Andreas Bießmann
Hello Aneesh, Aneesh V wrote: > On Thursday 30 June 2011 11:31 AM, Heiko Schocher wrote: >> Hello Aneesh, >> >> Aneesh V wrote: >>> Signed-off-by: Aneesh V<aneesh@ti.com> >>> --- >>> arch/arm/cpu/armv7/omap-common/Makefile | 9 ++-- >>> arch/arm/cpu/armv7/omap-common/spl.c | 56 >>> ++++++++++++++++++++++++++++ >>> arch/arm/cpu/armv7/omap-common/spl.lds | 62 >>> +++++++++++++++++++++++++++++++ >>> 3 files changed, 123 insertions(+), 4 deletions(-) >>> create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c >>> create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds >>> >> [...] >>> diff --git a/arch/arm/cpu/armv7/omap-common/spl.c >>> b/arch/arm/cpu/armv7/omap-common/spl.c >>> new file mode 100644 >>> index 0000000..b5a5f3c >>> --- /dev/null >>> +++ b/arch/arm/cpu/armv7/omap-common/spl.c >> [...] [...] >> >> BTW: Do you really need to relocate code? You could just load the 2nd >> stage loader to ram from board_init_f, or? > > I am passing the same address as I am executing from as the target for > the relocation, so the relocation will not happen, instead BSS will be > initialized. That's what I am calling it for. Initially I had my own > routine for clearing BSS. Then I decided to re-use it from start.S Ah, I see! Maybe you can add a comment here? Thanks! Hmm, I actually porting a dm368 based board with nand_spl support (patches following soon), and there I have an empty bss section, so I can direct copy the u-boot image from nand to ram in board_init_f(). > BTW, this series is not complete as far as SPL support is concerned. > It doesn't even build due to many undefined references. It serves only > as a prototype for the framework part. Ah, Ok ... bye, Heiko
Hi, > Hmm, I actually porting a dm368 based board with nand_spl support > (patches following soon), and there I have an empty bss section, > so I can direct copy the u-boot image from nand to ram in board_init_f(). This is what I'am doing with OMAP3 also. IMHO it would be more readable if relocate_code is not called if there is no relocation and instead the clear_bss()-function is used as Andreas suggested. (Or to just not use clear_bss if bss is empty) Regards Simon
On Thursday 30 June 2011 01:51 PM, Simon Schwarz wrote: > Hi, > >> Hmm, I actually porting a dm368 based board with nand_spl support >> (patches following soon), and there I have an empty bss section, >> so I can direct copy the u-boot image from nand to ram in board_init_f(). > > This is what I'am doing with OMAP3 also. IMHO it would be more > readable if relocate_code is not called if there is no relocation and > instead the clear_bss()-function is used as Andreas suggested. (Or to > just not use clear_bss if bss is empty) In my case there is indeed a BSS section, and a huge one at that - 192KB for the FAT driver. You will need it too if your OMAP3 SPL is going to support MMC FAT boot. br, Aneesh
Hi Simon, Le 30/06/2011 10:21, Simon Schwarz a écrit : > Hi, > >> Hmm, I actually porting a dm368 based board with nand_spl support >> (patches following soon), and there I have an empty bss section, >> so I can direct copy the u-boot image from nand to ram in board_init_f(). > > This is what I'am doing with OMAP3 also. IMHO it would be more > readable if relocate_code is not called if there is no relocation and > instead the clear_bss()-function is used as Andreas suggested. (Or to > just not use clear_bss if bss is empty) IMO, for the sake of rpbustness, the clear_bss code should handle the case where the BSS is empty, and for the sake of simplicity, it should be called always. > Regards > Simon Amicalement,
On Thursday 30 June 2011 04:39 PM, Albert ARIBAUD wrote: > Hi Simon, > > Le 30/06/2011 10:21, Simon Schwarz a écrit : >> Hi, >> >>> Hmm, I actually porting a dm368 based board with nand_spl support >>> (patches following soon), and there I have an empty bss section, >>> so I can direct copy the u-boot image from nand to ram in board_init_f(). >> >> This is what I'am doing with OMAP3 also. IMHO it would be more >> readable if relocate_code is not called if there is no relocation and >> instead the clear_bss()-function is used as Andreas suggested. (Or to >> just not use clear_bss if bss is empty) > > IMO, for the sake of rpbustness, the clear_bss code should handle the > case where the BSS is empty, and for the sake of simplicity, it should > be called always. I shall check the case when BSS is empty and fix it if needed. br, Aneesh
Dear Andreas, On Thursday 30 June 2011 12:38 PM, Andreas Bießmann wrote: > Dear Aneesh V, > > Am 30.06.2011 um 08:12 schrieb Aneesh V: > >> Hi Heiko, >> >> On Thursday 30 June 2011 11:31 AM, Heiko Schocher wrote: >>> Hello Aneesh, >>> >>> Aneesh V wrote: >>>> Signed-off-by: Aneesh V<aneesh@ti.com> >>>> --- >>>> arch/arm/cpu/armv7/omap-common/Makefile | 9 ++-- >>>> arch/arm/cpu/armv7/omap-common/spl.c | 56 ++++++++++++++++++++++++++++ >>>> arch/arm/cpu/armv7/omap-common/spl.lds | 62 +++++++++++++++++++++++++++++++ >>>> 3 files changed, 123 insertions(+), 4 deletions(-) >>>> create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c >>>> create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds >>>> >>> [...] >>>> diff --git a/arch/arm/cpu/armv7/omap-common/spl.c b/arch/arm/cpu/armv7/omap-common/spl.c >>>> new file mode 100644 >>>> index 0000000..b5a5f3c >>>> --- /dev/null >>>> +++ b/arch/arm/cpu/armv7/omap-common/spl.c >>> [...] >>>> @@ -0,0 +1,56 @@ >>>> +void board_init_f(ulong dummy) >>>> +{ >>>> + debug(">>board_init_f()\n"); >>>> + relocate_code(CONFIG_SYS_SPL_STACK,&gdata, CONFIG_SYS_SPL_TEXT_BASE); >>>> + debug("<<board_init_f()\n"); >>> >>> This debug printf will never occur ;-) >> >> Indeed. Thanks for pointing out. >> >>> >>> BTW: Do you really need to relocate code? You could just load the 2nd >>> stage loader to ram from board_init_f, or? >> >> I am passing the same address as I am executing from as the target for >> the relocation, so the relocation will not happen, instead BSS will be >> initialized. That's what I am calling it for. Initially I had my own >> routine for clearing BSS. Then I decided to re-use it from start.S > > So you could just call clear_bss(void) and skip relocate_code. But I think you need to adopt the __bss_start_ofs, __bss_end_ofs markers, cause your linker skript places them in SDRAM. Is that really needed, or is it ok to just comment this fact clearly as Heiko suggested? > > BTW: I think Simon Schwarz is also working on this, can one comment on his first version of patchset? We have decided to co-ordinate our work so that there won't be any duplication of efforts. As per this plan, these parts will be taken care in my OMAP4 MMC spl series and then he will extend it for OMAP3 and NAND. best regards, Aneesh
Dear Aneesh, Am 01.07.2011 11:27, schrieb Aneesh V: > Dear Andreas, > > On Thursday 30 June 2011 12:38 PM, Andreas Bießmann wrote: >> Dear Aneesh V, >> >> Am 30.06.2011 um 08:12 schrieb Aneesh V: >> >>> Hi Heiko, >>> >>> On Thursday 30 June 2011 11:31 AM, Heiko Schocher wrote: >>>> Hello Aneesh, >>>> >>>> Aneesh V wrote: >>>>> Signed-off-by: Aneesh V<aneesh@ti.com> <snip> >>>>> diff --git a/arch/arm/cpu/armv7/omap-common/spl.c >>>>> b/arch/arm/cpu/armv7/omap-common/spl.c >>>>> new file mode 100644 >>>>> index 0000000..b5a5f3c >>>>> --- /dev/null >>>>> +++ b/arch/arm/cpu/armv7/omap-common/spl.c >>>> [...] >>>>> @@ -0,0 +1,56 @@ >>>>> +void board_init_f(ulong dummy) >>>>> +{ >>>>> + debug(">>board_init_f()\n"); >>>>> + relocate_code(CONFIG_SYS_SPL_STACK,&gdata, >>>>> CONFIG_SYS_SPL_TEXT_BASE); >>>>> + debug("<<board_init_f()\n"); <snip> >>>> BTW: Do you really need to relocate code? You could just load the 2nd >>>> stage loader to ram from board_init_f, or? >>> >>> I am passing the same address as I am executing from as the target for >>> the relocation, so the relocation will not happen, instead BSS will be >>> initialized. That's what I am calling it for. Initially I had my own >>> routine for clearing BSS. Then I decided to re-use it from start.S >> >> So you could just call clear_bss(void) and skip relocate_code. But I >> think you need to adopt the __bss_start_ofs, __bss_end_ofs markers, >> cause your linker skript places them in SDRAM. > > Is that really needed, or is it ok to just comment this fact clearly as > Heiko suggested? No, it is not needed to call clear_bss() directly. I think commenting the fact, that passing same source and target address will skip the relocation stuff would be also OK here. But the second part is not clear to me. I saw in your linker, that bss is placed in SDRAM. In start.S the boundaries for clear_bss are calculated at compile time to ---8<--- _bss_start_ofs: .word __bss_start - _start --->8--- Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM addressing is far away (> 4GiB) ... So in you special case it may work, but if this is a blueprint for SPL on arm(v7) we should consider this. >> BTW: I think Simon Schwarz is also working on this, can one comment on >> his first version of patchset? > > We have decided to co-ordinate our work so that there won't be any > duplication of efforts. As per this plan, these parts will be taken > care in my OMAP4 MMC spl series and then he will extend it for OMAP3 > and NAND. I'm fine with that. ;) regards Andreas Bießmann
Dear Andreas, On Friday 01 July 2011 03:25 PM, Andreas Bießmann wrote: > Dear Aneesh, [snip ..] > But the second part is not clear to me. I saw in your linker, that bss > is placed in SDRAM. In start.S the boundaries for clear_bss are > calculated at compile time to > > ---8<--- > _bss_start_ofs: > .word __bss_start - _start > --->8--- > > Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM > addressing is far away (> 4GiB) ... So in you special case it may work, > but if this is a blueprint for SPL on arm(v7) we should consider this. > Nice catch. Actually, in my original OMAP4 series I tried to add support for disjoint bss to support my case. But now I realize that it works only for non-relocation case and that too only when the bss is at higher address compared to .text Basically disjoint bss is not relocation friendly. So here is what I propose: 1. Modify existing clear_bss sub-routine in start.S to take absolute addresses. 2. In regular u-boot, calculate the relocated bss address and pass to this function. 3. In SPL don't try to calculate the relocated address and directly pass the absolute address. If this is fine I will make the necessary changes in start.S in the next revision. best regards, Aneesh
Hi Aneesh, Le 01/07/2011 13:48, Aneesh V a écrit : > Dear Andreas, > > On Friday 01 July 2011 03:25 PM, Andreas Bießmann wrote: >> Dear Aneesh, > [snip ..] >> But the second part is not clear to me. I saw in your linker, that bss >> is placed in SDRAM. In start.S the boundaries for clear_bss are >> calculated at compile time to >> >> ---8<--- >> _bss_start_ofs: >> .word __bss_start - _start >> --->8--- >> >> Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM >> addressing is far away (> 4GiB) ... So in you special case it may work, >> but if this is a blueprint for SPL on arm(v7) we should consider this. >> > > Nice catch. Actually, in my original OMAP4 series I tried to add > support for disjoint bss to support my case. But now I realize that it > works only for non-relocation case and that too only when the bss is at > higher address compared to .text > > Basically disjoint bss is not relocation friendly. So here is what I > propose: > > 1. Modify existing clear_bss sub-routine in start.S to take absolute > addresses. > 2. In regular u-boot, calculate the relocated bss address and pass to > this function. > 3. In SPL don't try to calculate the relocated address and directly > pass the absolute address. > > If this is fine I will make the necessary changes in start.S in the > next revision. So you would compute the BSS location in board_init_f() and pass that to relocate_code()? > best regards, > Aneesh Amicalement,
Hi Albert, On Saturday 02 July 2011 01:21 AM, Albert ARIBAUD wrote: > Hi Aneesh, > > Le 01/07/2011 13:48, Aneesh V a écrit : >> Dear Andreas, >> >> On Friday 01 July 2011 03:25 PM, Andreas Bießmann wrote: >>> Dear Aneesh, >> [snip ..] >>> But the second part is not clear to me. I saw in your linker, that bss >>> is placed in SDRAM. In start.S the boundaries for clear_bss are >>> calculated at compile time to >>> >>> ---8<--- >>> _bss_start_ofs: >>> .word __bss_start - _start >>> --->8--- >>> >>> Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM >>> addressing is far away (> 4GiB) ... So in you special case it may work, >>> but if this is a blueprint for SPL on arm(v7) we should consider this. >>> >> >> Nice catch. Actually, in my original OMAP4 series I tried to add >> support for disjoint bss to support my case. But now I realize that it >> works only for non-relocation case and that too only when the bss is at >> higher address compared to .text >> >> Basically disjoint bss is not relocation friendly. So here is what I >> propose: >> >> 1. Modify existing clear_bss sub-routine in start.S to take absolute >> addresses. >> 2. In regular u-boot, calculate the relocated bss address and pass to >> this function. >> 3. In SPL don't try to calculate the relocated address and directly >> pass the absolute address. >> >> If this is fine I will make the necessary changes in start.S in the >> next revision. > > So you would compute the BSS location in board_init_f() and pass that to > relocate_code()? I was thinking of doing that in start.S itself. I haven't looked at all the details though. BTW, please note that I am not trying to support disjoint BSS in regular u-boot. I think it becomes complex with relocation + it doesn't seem to be worth when all SDRAM is at our disposal. So: 1. #ifdef CONFIG_PRELOADER part in start.s will just pass __bss_start and __bss_end to the clear_bss function(assumes no relocation). 2. #else part of above will assume that bss follows text and data(or at least that __bss_start > _start), so add relocation offset to __bss_start and __bss_end, and pass them to the clear_bss() Does that sound ok? br, Aneesh
Hi Aneesh, Le 03/07/2011 06:47, Aneesh V a écrit : > Hi Albert, > > On Saturday 02 July 2011 01:21 AM, Albert ARIBAUD wrote: >> Hi Aneesh, >> >> Le 01/07/2011 13:48, Aneesh V a écrit : >>> Dear Andreas, >>> >>> On Friday 01 July 2011 03:25 PM, Andreas Bießmann wrote: >>>> Dear Aneesh, >>> [snip ..] >>>> But the second part is not clear to me. I saw in your linker, that bss >>>> is placed in SDRAM. In start.S the boundaries for clear_bss are >>>> calculated at compile time to >>>> >>>> ---8<--- >>>> _bss_start_ofs: >>>> .word __bss_start - _start >>>> --->8--- >>>> >>>> Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM >>>> addressing is far away (> 4GiB) ... So in you special case it may work, >>>> but if this is a blueprint for SPL on arm(v7) we should consider this. >>>> >>> >>> Nice catch. Actually, in my original OMAP4 series I tried to add >>> support for disjoint bss to support my case. But now I realize that it >>> works only for non-relocation case and that too only when the bss is at >>> higher address compared to .text >>> >>> Basically disjoint bss is not relocation friendly. So here is what I >>> propose: >>> >>> 1. Modify existing clear_bss sub-routine in start.S to take absolute >>> addresses. >>> 2. In regular u-boot, calculate the relocated bss address and pass to >>> this function. >>> 3. In SPL don't try to calculate the relocated address and directly >>> pass the absolute address. >>> >>> If this is fine I will make the necessary changes in start.S in the >>> next revision. >> >> So you would compute the BSS location in board_init_f() and pass that to >> relocate_code()? > > I was thinking of doing that in start.S itself. I haven't looked at > all the details though. > > BTW, please note that I am not trying to support disjoint BSS in > regular u-boot. I think it becomes complex with relocation + it doesn't > seem to be worth when all SDRAM is at our disposal. > > So: > 1. #ifdef CONFIG_PRELOADER part in start.s will just pass __bss_start > and __bss_end to the clear_bss function(assumes no relocation). > 2. #else part of above will assume that bss follows text and data(or at > least that __bss_start > _start), so add relocation offset to > __bss_start and __bss_end, and pass them to the clear_bss() > > Does that sound ok? (note clear_bss is not a function in arch/arm/cpu/armv7/start.S, it is only a label) So, considering what is already in arch/arm/cpu/armv7/start.S, you would just add a conditional variant to the BSS clearing code for the preloader case? If so, and considering that you'll pass an offset of 0, why would you need that variant for? With offset=0, the code already does what you want, does it not? > br, > Aneesh Amicalement,
Dear Albert Aribaud, Am 03.07.2011 um 08:56 schrieb Albert ARIBAUD: > Hi Aneesh, > > Le 03/07/2011 06:47, Aneesh V a écrit : >> Hi Albert, >> >> On Saturday 02 July 2011 01:21 AM, Albert ARIBAUD wrote: >>> Hi Aneesh, >>> <snip> >> I was thinking of doing that in start.S itself. I haven't looked at >> all the details though. >> >> BTW, please note that I am not trying to support disjoint BSS in >> regular u-boot. I think it becomes complex with relocation + it doesn't >> seem to be worth when all SDRAM is at our disposal. >> >> So: >> 1. #ifdef CONFIG_PRELOADER part in start.s will just pass __bss_start >> and __bss_end to the clear_bss function(assumes no relocation). >> 2. #else part of above will assume that bss follows text and data(or at >> least that __bss_start > _start), so add relocation offset to >> __bss_start and __bss_end, and pass them to the clear_bss() >> >> Does that sound ok? > > (note clear_bss is not a function in arch/arm/cpu/armv7/start.S, it is only a label) > > So, considering what is already in arch/arm/cpu/armv7/start.S, you would just add a conditional variant to the BSS clearing code for the preloader case? > > If so, and considering that you'll pass an offset of 0, why would you need that variant for? With offset=0, the code already does what you want, does it not? we are looking forward to have relocate_code(), clear_bss(), a.s.o. implemented in c in future (as I understood the discussion around christmas about relocation). Wouldn't it be nice to implement now a SPL version of clear_bss(int start, int size) in c and skip the relocate_code() stuff in start.S completely? As I see the normal way to boot is: -> some start vector -> lowlevel_init -> board_init_f -> relocate_code -> board_init_r for SPL we do not need the steps after board_init_f. board_init_f for SPL should implement the generic way to load data from predefined (compile time) source to predefined target address and branch it. Therefore a simple memset() in board_init_f could be sufficient to clear the bss. regards Andreas Bießmann
Le 03/07/2011 09:31, Andreas Bießmann a écrit : > Dear Albert Aribaud, > > Am 03.07.2011 um 08:56 schrieb Albert ARIBAUD: > >> Hi Aneesh, >> >> Le 03/07/2011 06:47, Aneesh V a écrit : >>> Hi Albert, >>> >>> On Saturday 02 July 2011 01:21 AM, Albert ARIBAUD wrote: >>>> Hi Aneesh, >>>> > > <snip> > >>> I was thinking of doing that in start.S itself. I haven't looked at >>> all the details though. >>> >>> BTW, please note that I am not trying to support disjoint BSS in >>> regular u-boot. I think it becomes complex with relocation + it doesn't >>> seem to be worth when all SDRAM is at our disposal. >>> >>> So: >>> 1. #ifdef CONFIG_PRELOADER part in start.s will just pass __bss_start >>> and __bss_end to the clear_bss function(assumes no relocation). >>> 2. #else part of above will assume that bss follows text and data(or at >>> least that __bss_start> _start), so add relocation offset to >>> __bss_start and __bss_end, and pass them to the clear_bss() >>> >>> Does that sound ok? >> >> (note clear_bss is not a function in arch/arm/cpu/armv7/start.S, it is only a label) >> >> So, considering what is already in arch/arm/cpu/armv7/start.S, you would just add a conditional variant to the BSS clearing code for the preloader case? >> >> If so, and considering that you'll pass an offset of 0, why would you need that variant for? With offset=0, the code already does what you want, does it not? > > we are looking forward to have relocate_code(), clear_bss(), a.s.o. implemented in c in future (as I understood the discussion around christmas about relocation). > Wouldn't it be nice to implement now a SPL version of clear_bss(int start, int size) in c and skip the relocate_code() stuff in start.S completely? > > As I see the normal way to boot is: > > -> some start vector > -> lowlevel_init > -> board_init_f > -> relocate_code > -> board_init_r > > for SPL we do not need the steps after board_init_f. board_init_f > for SPL should implement the generic way to load data from predefined > (compile time) source to predefined target address and branch it. > Therefore a simple memset() in board_init_f could be sufficient to clear > the bss. Granted, clear_bss could become a function and should be optimized according to the context; however, I don't like the idea of calling a C library function at a point where we know the C environment for the caller is by definition not really set up yet. And as a general rule, I think we should not perform changes 'now for the future' so yes, it would be nice to have a clear_bss() function, but we don't need it right now in U-Boot working as it is now; I'd much prefer the separate clear_bss C function to appear within the patch set that will bring this future general move of relocation to C. > regards > > Andreas Bießmann Amicalement,
Hi Albert, On Sunday 03 July 2011 12:26 PM, Albert ARIBAUD wrote: > Hi Aneesh, > > Le 03/07/2011 06:47, Aneesh V a écrit : >> Hi Albert, >> >> On Saturday 02 July 2011 01:21 AM, Albert ARIBAUD wrote: >>> Hi Aneesh, >>> >>> Le 01/07/2011 13:48, Aneesh V a écrit : >>>> Dear Andreas, >>>> >>>> On Friday 01 July 2011 03:25 PM, Andreas Bießmann wrote: >>>>> Dear Aneesh, >>>> [snip ..] >>>>> But the second part is not clear to me. I saw in your linker, that bss >>>>> is placed in SDRAM. In start.S the boundaries for clear_bss are >>>>> calculated at compile time to >>>>> >>>>> ---8<--- >>>>> _bss_start_ofs: >>>>> .word __bss_start - _start >>>>> --->8--- >>>>> >>>>> Will that also work with e.g. SDRAM adress space is before SRAM, SDRAM >>>>> addressing is far away (> 4GiB) ... So in you special case it may >>>>> work, >>>>> but if this is a blueprint for SPL on arm(v7) we should consider this. >>>>> >>>> >>>> Nice catch. Actually, in my original OMAP4 series I tried to add >>>> support for disjoint bss to support my case. But now I realize that it >>>> works only for non-relocation case and that too only when the bss is at >>>> higher address compared to .text >>>> >>>> Basically disjoint bss is not relocation friendly. So here is what I >>>> propose: >>>> >>>> 1. Modify existing clear_bss sub-routine in start.S to take absolute >>>> addresses. >>>> 2. In regular u-boot, calculate the relocated bss address and pass to >>>> this function. >>>> 3. In SPL don't try to calculate the relocated address and directly >>>> pass the absolute address. >>>> >>>> If this is fine I will make the necessary changes in start.S in the >>>> next revision. >>> >>> So you would compute the BSS location in board_init_f() and pass that to >>> relocate_code()? >> >> I was thinking of doing that in start.S itself. I haven't looked at >> all the details though. >> >> BTW, please note that I am not trying to support disjoint BSS in >> regular u-boot. I think it becomes complex with relocation + it doesn't >> seem to be worth when all SDRAM is at our disposal. >> >> So: >> 1. #ifdef CONFIG_PRELOADER part in start.s will just pass __bss_start >> and __bss_end to the clear_bss function(assumes no relocation). >> 2. #else part of above will assume that bss follows text and data(or at >> least that __bss_start > _start), so add relocation offset to >> __bss_start and __bss_end, and pass them to the clear_bss() >> >> Does that sound ok? > > (note clear_bss is not a function in arch/arm/cpu/armv7/start.S, it is > only a label) > > So, considering what is already in arch/arm/cpu/armv7/start.S, you would > just add a conditional variant to the BSS clearing code for the > preloader case? > > If so, and considering that you'll pass an offset of 0, why would you > need that variant for? With offset=0, the code already does what you > want, does it not? > Yes, I think the following is all we need: clear_bss: +#ifdef CONFIG_PRELOADER + /* No relocation for SPL */ + ldr r0, =__bss_start + ldr r1, =__bss_end__ +#else ldr r0, _bss_start_ofs ldr r1, _bss_end_ofs mov r4, r6 /* reloc addr */ add r0, r0, r4 add r1, r1, r4 +#endif I haven't tested this though. br, Aneesh
diff --git a/arch/arm/cpu/armv7/omap-common/Makefile b/arch/arm/cpu/armv7/omap-common/Makefile index dc01ee5..7931303 100644 --- a/arch/arm/cpu/armv7/omap-common/Makefile +++ b/arch/arm/cpu/armv7/omap-common/Makefile @@ -25,12 +25,13 @@ include $(TOPDIR)/config.mk LIB = $(obj)libomap-common.o -SOBJS := reset.o +SOBJS-$(CONFIG_NORMAL_UBOOT) := reset.o -COBJS := timer.o +COBJS-$(CONFIG_NORMAL_UBOOT) := timer.o +COBJS-$(CONFIG_UBOOT_SPL_BUILD) := spl.o -SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) -OBJS := $(addprefix $(obj),$(SOBJS) $(COBJS)) +SRCS := $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c) +OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y)) all: $(obj).depend $(LIB) diff --git a/arch/arm/cpu/armv7/omap-common/spl.c b/arch/arm/cpu/armv7/omap-common/spl.c new file mode 100644 index 0000000..b5a5f3c --- /dev/null +++ b/arch/arm/cpu/armv7/omap-common/spl.c @@ -0,0 +1,56 @@ +/* + * + * Clock initialization for OMAP4 + * + * (C) Copyright 2010 + * Texas Instruments, <www.ti.com> + * + * Aneesh V <aneesh@ti.com> + * + * See file CREDITS for list of people who contributed to this + * project. + * + * 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 <asm/u-boot.h> +#include <asm/arch/sys_proto.h> +#include <mmc.h> +#include <fat.h> +#include <timestamp_autogenerated.h> +#include <version_autogenerated.h> +#include <asm/omap_common.h> +#include <asm/arch/mmc_host_def.h> +#include <i2c.h> + +/* Define global data structure pointer to it*/ +gd_t gdata __attribute__ ((section(".data"))); +bd_t bdata __attribute__ ((section(".data"))); +gd_t *gd = &gdata; + +typedef void (*u_boot_entry_t)(void)__attribute__ ((noreturn)); + +void board_init_f(ulong dummy) +{ + debug(">>board_init_f()\n"); + relocate_code(CONFIG_SYS_SPL_STACK, &gdata, CONFIG_SYS_SPL_TEXT_BASE); + debug("<<board_init_f()\n"); +} + +void board_init_r(gd_t *id, ulong dummy) +{ + /* TODO - loading the 2nd stage payload will happen here */ +} diff --git a/arch/arm/cpu/armv7/omap-common/spl.lds b/arch/arm/cpu/armv7/omap-common/spl.lds new file mode 100644 index 0000000..22fd5da --- /dev/null +++ b/arch/arm/cpu/armv7/omap-common/spl.lds @@ -0,0 +1,62 @@ +/* + * (C) Copyright 2002 + * Gary Jennejohn, DENX Software Engineering, <garyj@denx.de> + * + * (C) Copyright 2010 + * Texas Instruments, <www.ti.com> + * Aneesh V <aneesh@ti.com> + * + * See file CREDITS for list of people who contributed to this + * project. + * + * 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 + */ + +MEMORY { .sram : ORIGIN = CONFIG_SYS_SPL_TEXT_BASE,\ + LENGTH = CONFIG_SYS_SPL_MAX_SIZE } +MEMORY { .sdram : ORIGIN = CONFIG_SYS_SPL_BSS_START_ADDR, \ + LENGTH = CONFIG_SYS_SPL_BSS_MAX_SIZE } + +OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm") +OUTPUT_ARCH(arm) +ENTRY(_start) +SECTIONS +{ + .text : + { + __start = .; + arch/arm/cpu/armv7/start.o (.text) + *(.text*) + } >.sram + + . = ALIGN(4); + .rodata : { *(SORT_BY_ALIGNMENT(.rodata*)) } >.sram + + . = ALIGN(4); + .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram + . = ALIGN(4); + __image_copy_end = .; + __flash_image_end = .; + + .bss : + { + . = ALIGN(4); + __bss_start = .; + *(.bss*) + . = ALIGN(4); + _end = .; + } >.sdram +}
Signed-off-by: Aneesh V <aneesh@ti.com> --- arch/arm/cpu/armv7/omap-common/Makefile | 9 ++-- arch/arm/cpu/armv7/omap-common/spl.c | 56 ++++++++++++++++++++++++++++ arch/arm/cpu/armv7/omap-common/spl.lds | 62 +++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 arch/arm/cpu/armv7/omap-common/spl.c create mode 100644 arch/arm/cpu/armv7/omap-common/spl.lds