Message ID | 1290538131-12383-7-git-send-email-Joakim.Tjernlund@transmode.se |
---|---|
State | RFC |
Headers | show |
On Tue, 23 Nov 2010 19:48:51 +0100 Joakim Tjernlund <Joakim.Tjernlund@transmode.se> wrote: > Only these 2 call sites depends on fixups for my mpc8321 based > board. > --- > arch/powerpc/cpu/mpc83xx/cpu_init.c | 2 +- > arch/powerpc/lib/board.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c > index f3b67ae..0437b49 100644 > --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c > +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c > @@ -534,7 +534,7 @@ int prt_83xx_rsr(void) > sep = " "; > for (i = 0; i < n; i++) > if (rsr & bits[i].mask) { > - printf("%s%s", sep, bits[i].desc); > + printf("%s%s", sep, LINK_OFF(bits[i].desc)); > sep = ", "; > } > puts("\n"); > diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c > index 7b09fb5..9fa99dc 100644 > --- a/arch/powerpc/lib/board.c > +++ b/arch/powerpc/lib/board.c > @@ -386,7 +386,7 @@ void board_init_f (ulong bootflag) > #endif > > for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) { > - if ((*init_fnc_ptr) () != 0) { > + if ((LINK_OFF(*init_fnc_ptr)) () != 0) { > hang (); > } > } "Only these" that you've found so far, for the board you've tried -- is it worth adding another pre-relocation landmine to shrink the image by 5%? I don't miss the manual fixups we had to do under the old relocation scheme. Please document the specific circumstances in which one would need to use this (any data-segment pointer before relocation?). Is a missing LINK_OFF likely to result in a crash, or silent bad behavior? Will LINK_OFF be a no-op after relocation? -Scott
Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 21:46:51: > On Tue, 23 Nov 2010 19:48:51 +0100 > Joakim Tjernlund <Joakim.Tjernlund@transmode.se> wrote: > > > Only these 2 call sites depends on fixups for my mpc8321 based > > board. > > --- > > arch/powerpc/cpu/mpc83xx/cpu_init.c | 2 +- > > arch/powerpc/lib/board.c | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c > > index f3b67ae..0437b49 100644 > > --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c > > +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c > > @@ -534,7 +534,7 @@ int prt_83xx_rsr(void) > > sep = " "; > > for (i = 0; i < n; i++) > > if (rsr & bits[i].mask) { > > - printf("%s%s", sep, bits[i].desc); > > + printf("%s%s", sep, LINK_OFF(bits[i].desc)); > > sep = ", "; > > } > > puts("\n"); > > diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c > > index 7b09fb5..9fa99dc 100644 > > --- a/arch/powerpc/lib/board.c > > +++ b/arch/powerpc/lib/board.c > > @@ -386,7 +386,7 @@ void board_init_f (ulong bootflag) > > #endif > > > > for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) { > > - if ((*init_fnc_ptr) () != 0) { > > + if ((LINK_OFF(*init_fnc_ptr)) () != 0) { > > hang (); > > } > > } > > "Only these" that you've found so far, for the board you've tried -- Yes, that is what I wrote. The previous try had LINK_OFF calls all over the place just for my board. > is it worth adding another pre-relocation landmine to shrink the image > by 5%? I don't miss the manual fixups we had to do under the old > relocation scheme. -msingle-pic-base does not require LINK_OFF per se. It will shrink the image significantly for free. It enables impl. of true PIC and if you want to have that too you need LINK_OFF. > Please document the specific circumstances in which one would need > to use this (any data-segment pointer before relocation?). Any ptr needing fixups: char *sptr = "hi"; Same for static initialized function ptrs too. > Is a missing LINK_OFF likely to result in a crash, or silent bad > behavior? Will LINK_OFF be a no-op after relocation? Either crash or garbage printed on the port. LINK_OFF is a NOP after relocation. It will be a NOP if link address == load address too. Jocke
> > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 21:46:51: > > On Tue, 23 Nov 2010 19:48:51 +0100 > > Joakim Tjernlund <Joakim.Tjernlund@transmode.se> wrote: > > Please document the specific circumstances in which one would need > > to use this (any data-segment pointer before relocation?). > > Any ptr needing fixups: > char *sptr = "hi"; > Same for static initialized function ptrs too. Clarification, the above is for ptrs used before relocation to RAM. Jocke
On Tue, 23 Nov 2010 22:03:36 +0100 Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 21:46:51: > > "Only these" that you've found so far, for the board you've tried -- > > Yes, that is what I wrote. Yes, I was just emphasizing that. :-) > > is it worth adding another pre-relocation landmine to shrink the image > > by 5%? I don't miss the manual fixups we had to do under the old > > relocation scheme. > > -msingle-pic-base does not require LINK_OFF per se. It will shrink the image > significantly for free. It enables impl. of true PIC and if you want to have > that too you need LINK_OFF. What is "true PIC" versus the rest of what -msingle-pic-base does, and what are the advantages? > > Is a missing LINK_OFF likely to result in a crash, or silent bad > > behavior? Will LINK_OFF be a no-op after relocation? > > Either crash or garbage printed on the port. Garbage possibly being empty strings that aren't noticed? Or possibly bad values changing program flow in nonobvious ways? > LINK_OFF is a NOP after relocation. It will be a NOP > if link address == load address too. "load address" being pre-relocation? Currently these must be equal (which doesn't seem particularly burdensome). Seems like there would be a high risk for a developer of board A to add new code that affects board B, and needs a manual relocation, but board A doesn't use this or loads at the link address, so it doesn't show up. Seems like a maintenance headache. -Scott
Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 22:20:58: > > On Tue, 23 Nov 2010 22:03:36 +0100 > Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > > > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 21:46:51: > > > "Only these" that you've found so far, for the board you've tried -- > > > > Yes, that is what I wrote. > > Yes, I was just emphasizing that. :-) > > > > is it worth adding another pre-relocation landmine to shrink the image > > > by 5%? I don't miss the manual fixups we had to do under the old > > > relocation scheme. > > > > -msingle-pic-base does not require LINK_OFF per se. It will shrink the image > > significantly for free. It enables impl. of true PIC and if you want to have > > that too you need LINK_OFF. > > What is "true PIC" versus the rest of what -msingle-pic-base does, and > what are the advantages? Being able to load the same image to any address. msingle-pic-base simple skips the PIC prologue and assumes the R30 is already setup correctly. I a self contained env. like uboot this is true for -fpic. I use that property to copy the GOT to dcache, relocate it there and point R30 to the GOT in dcache. > > > > Is a missing LINK_OFF likely to result in a crash, or silent bad > > > behavior? Will LINK_OFF be a no-op after relocation? > > > > Either crash or garbage printed on the port. > > Garbage possibly being empty strings that aren't noticed? > > Or possibly bad values changing program flow in nonobvious ways? Impossible to say, I guess anything can happen. > > > LINK_OFF is a NOP after relocation. It will be a NOP > > if link address == load address too. > > "load address" being pre-relocation? Currently these must be equal > (which doesn't seem particularly burdensome). Yes, but in our case we update the boot in the field and we want to make that safer by having two uboot areas but we don't want to carry around two u-boot images. I think Wolfgang had some other uses in mind too. > > Seems like there would be a high risk for a developer of board A to add > new code that affects board B, and needs a manual relocation, but board > A doesn't use this or loads at the link address, so it doesn't show up. > Seems like a maintenance headache. yes, it is a risk. One that can be minimized the more uses this function.
On Tue, 23 Nov 2010 23:14:06 +0100 Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 22:20:58: > > "load address" being pre-relocation? Currently these must be equal > > (which doesn't seem particularly burdensome). > > Yes, but in our case we update the boot in the field and we want to > make that safer by having two uboot areas but we don't want to carry around > two u-boot images. How about playing with BATs before entering C code, so that the image always appears at the same effective address? In fact, we could probably do something similar when copying to RAM as well, and not need any sort of link relocation. -Scott
Dear Scott Wood, In message <20101123163204.4f843e61@udp111988uds.am.freescale.net> you wrote: > > How about playing with BATs before entering C code, so that the image > always appears at the same effective address? Not all systems have BATs. Best regards, Wolfgang Denk
On Wed, 24 Nov 2010 00:03:13 +0100 Wolfgang Denk <wd@denx.de> wrote: > Dear Scott Wood, > > In message <20101123163204.4f843e61@udp111988uds.am.freescale.net> you wrote: > > > > How about playing with BATs before entering C code, so that the image > > always appears at the same effective address? > > Not all systems have BATs. I was speaking in the context of what he wanted to do with an 83xx board -- but the concept applies to any hardware with an MMU that isn't too painful to set up that early. If someone wants to do this kind of thing on hardware that doesn't meet that description, that's another matter -- if the the hardware doesn't provide a nicer bank switching mechanism (e.g. p4080ds lets you rotate the flash banks' physical addresses, rather than change the reset vector), or an SRAM that U-Boot (or an SPL) could copy itself to before C code, etc. -Scott
On Wed, Nov 24, 2010 at 10:24 AM, Scott Wood <scottwood@freescale.com> wrote: > On Wed, 24 Nov 2010 00:03:13 +0100 > Wolfgang Denk <wd@denx.de> wrote: > >> Dear Scott Wood, >> >> In message <20101123163204.4f843e61@udp111988uds.am.freescale.net> you wrote: >> > >> > How about playing with BATs before entering C code, so that the image >> > always appears at the same effective address? >> >> Not all systems have BATs. > > I was speaking in the context of what he wanted to do with an 83xx board > -- but the concept applies to any hardware with an MMU that isn't too > painful to set up that early. > > If someone wants to do this kind of thing on hardware that doesn't > meet that description, that's another matter -- if the the hardware > doesn't provide a nicer bank switching mechanism (e.g. p4080ds lets you > rotate the flash banks' physical addresses, rather than change the > reset vector), or an SRAM that U-Boot (or an SPL) could copy itself to > before C code, etc. It seems to me that we are applying at the architecture level a 'nice to have' which may belong at the board level. How many vendors are going to do a fancy 'two U-Boot images' trickery? Will it be (nearly) every 83xx board? I agree with Scott - If you want to do something that fancy, provide support for it in your board hardware. I don't like the idea of diverging the core feature-set available at the architecture level if those features belong more at the SOC or Board level. Regards, Graeme
Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 23:32:04: > > On Tue, 23 Nov 2010 23:14:06 +0100 > Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > > > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 22:20:58: > > > "load address" being pre-relocation? Currently these must be equal > > > (which doesn't seem particularly burdensome). > > > > Yes, but in our case we update the boot in the field and we want to > > make that safer by having two uboot areas but we don't want to carry around > > two u-boot images. > > How about playing with BATs before entering C code, so that the image > always appears at the same effective address? hmm, never thought of that. The extra bonus would be that LINK_OFF should not be needed either. Jocke
> > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 23:32:04: > > > > On Tue, 23 Nov 2010 23:14:06 +0100 > > Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > > > > > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 22:20:58: > > > > "load address" being pre-relocation? Currently these must be equal > > > > (which doesn't seem particularly burdensome). > > > > > > Yes, but in our case we update the boot in the field and we want to > > > make that safer by having two uboot areas but we don't want to carry around > > > two u-boot images. > > > > How about playing with BATs before entering C code, so that the image > > always appears at the same effective address? > > hmm, never thought of that. The extra bonus would be that LINK_OFF should > not be needed either. After sleeping on it I realize that all direct accesses to the flash such as getting the env. will need to be adjusted instead. Jocke
On Wed, 24 Nov 2010 12:04:15 +0100 Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > > > > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 23:32:04: > > > > > > On Tue, 23 Nov 2010 23:14:06 +0100 > > > Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > > > > > > > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 22:20:58: > > > > > "load address" being pre-relocation? Currently these must be equal > > > > > (which doesn't seem particularly burdensome). > > > > > > > > Yes, but in our case we update the boot in the field and we want to > > > > make that safer by having two uboot areas but we don't want to carry around > > > > two u-boot images. > > > > > > How about playing with BATs before entering C code, so that the image > > > always appears at the same effective address? > > > > hmm, never thought of that. The extra bonus would be that LINK_OFF should > > not be needed either. > > After sleeping on it I realize that all direct accesses to the flash > such as getting the env. will need to be adjusted instead. You could have one small mapping for the U-Boot image, and another larger unchanging mapping that covers the whole flash. -Scott
Scott Wood <scottwood@freescale.com> wrote on 2010/11/24 18:16:56: > > On Wed, 24 Nov 2010 12:04:15 +0100 > Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > > > > > > > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 23:32:04: > > > > > > > > On Tue, 23 Nov 2010 23:14:06 +0100 > > > > Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > > > > > > > > > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 22:20:58: > > > > > > "load address" being pre-relocation? Currently these must be equal > > > > > > (which doesn't seem particularly burdensome). > > > > > > > > > > Yes, but in our case we update the boot in the field and we want to > > > > > make that safer by having two uboot areas but we don't want to carry around > > > > > two u-boot images. > > > > > > > > How about playing with BATs before entering C code, so that the image > > > > always appears at the same effective address? > > > > > > hmm, never thought of that. The extra bonus would be that LINK_OFF should > > > not be needed either. > > > > After sleeping on it I realize that all direct accesses to the flash > > such as getting the env. will need to be adjusted instead. > > You could have one small mapping for the U-Boot image, and another > larger unchanging mapping that covers the whole flash. Played a little with this but it seems like two BATs cannot overlap?
On Wed, 24 Nov 2010 22:36:27 +0100 Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > Scott Wood <scottwood@freescale.com> wrote on 2010/11/24 18:16:56: > > > > On Wed, 24 Nov 2010 12:04:15 +0100 > > Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > > > > > > > > > > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 23:32:04: > > > > > > > > > > On Tue, 23 Nov 2010 23:14:06 +0100 > > > > > Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > > > > > > > > > > > Scott Wood <scottwood@freescale.com> wrote on 2010/11/23 22:20:58: > > > > > > > "load address" being pre-relocation? Currently these must be equal > > > > > > > (which doesn't seem particularly burdensome). > > > > > > > > > > > > Yes, but in our case we update the boot in the field and we want to > > > > > > make that safer by having two uboot areas but we don't want to carry around > > > > > > two u-boot images. > > > > > > > > > > How about playing with BATs before entering C code, so that the image > > > > > always appears at the same effective address? > > > > > > > > hmm, never thought of that. The extra bonus would be that LINK_OFF should > > > > not be needed either. > > > > > > After sleeping on it I realize that all direct accesses to the flash > > > such as getting the env. will need to be adjusted instead. > > > > You could have one small mapping for the U-Boot image, and another > > larger unchanging mapping that covers the whole flash. > > Played a little with this but it seems like two BATs cannot overlap? They can't overlap in effective address space. They can have overlapping physical addresses (make sure the WIMG bits are the same). -Scott
Dear Joakim Tjernlund, In message <OF3286D177.B2B7747D-ONC12577E5.00769A1F-C12577E5.0076B18F@transmode.se> you wrote: > > Played a little with this but it seems like two BATs cannot overlap? IIRC they can, but the first (lower register numbers) mapping wins. Best regards, Wolfgang Denk
Scott Wood <scottwood@freescale.com> wrote on 2010/11/24 22:41:19: > > > > > > How about playing with BATs before entering C code, so that the image > > > > > > always appears at the same effective address? > > > > > > > > > > hmm, never thought of that. The extra bonus would be that LINK_OFF should > > > > > not be needed either. > > > > > > > > After sleeping on it I realize that all direct accesses to the flash > > > > such as getting the env. will need to be adjusted instead. > > > > > > You could have one small mapping for the U-Boot image, and another > > > larger unchanging mapping that covers the whole flash. > > > > Played a little with this but it seems like two BATs cannot overlap? > > They can't overlap in effective address space. They can have > overlapping physical addresses (make sure the WIMG bits are the same). hmm, now its starting to get a bit messy, I have flash over and under my boot area. In general it may be hard to match any random layout.
Wolfgang Denk <wd@denx.de> wrote on 2010/11/24 22:45:11: > > Dear Joakim Tjernlund, > > In message <OF3286D177.B2B7747D-ONC12577E5.00769A1F-C12577E5.0076B18F@transmode.se> you wrote: > > > > Played a little with this but it seems like two BATs cannot overlap? > > IIRC they can, but the first (lower register numbers) mapping wins. This text from PPC PEM suggests otherwise: If a BAT entry is not valid for a given access, it does not participate in address translation for that access. Two BAT entries may not map an overlapping effective address range and be valid at the same time. Entries that have complementary settings of V[s] and V[p] may map overlapping effective address blocks. Complementary settings would be as follows: BAT entry A: Vs = 1, Vp = 0 BAT entry B: Vs = 0, Vp = 1 > > Best regards, > > Wolfgang Denk PS. Are you planning to apply some of my earlier patches(not this RFC series) or are you waiting for Freescale to Ack/pick up? So far nobody has said anything. Jocke
Dear Joakim Tjernlund, In message <OF14AC2061.9896D2D3-ONC12577E5.0077F975-C12577E5.00785AA9@transmode.se> you wrote: > > Are you planning to apply some of my earlier patches(not this RFC series) > or are you waiting for Freescale to Ack/pick up? So far nobody > has said anything. I think these are all on Freescale's (Kim Phillips') list. Best regards, Wolfgang Denk
On Wed, 24 Nov 2010 22:50:21 +0100 Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote: > Scott Wood <scottwood@freescale.com> wrote on 2010/11/24 22:41:19: > > > > > > > How about playing with BATs before entering C code, so that the image > > > > > > > always appears at the same effective address? > > > > > > > > > > > > hmm, never thought of that. The extra bonus would be that LINK_OFF should > > > > > > not be needed either. > > > > > > > > > > After sleeping on it I realize that all direct accesses to the flash > > > > > such as getting the env. will need to be adjusted instead. > > > > > > > > You could have one small mapping for the U-Boot image, and another > > > > larger unchanging mapping that covers the whole flash. > > > > > > Played a little with this but it seems like two BATs cannot overlap? > > > > They can't overlap in effective address space. They can have > > overlapping physical addresses (make sure the WIMG bits are the same). > > hmm, now its starting to get a bit messy, I have flash over and under > my boot area. In general it may be hard to match any random layout. It shouldn't matter where in flash the boot area is. E.g. if you have 64 MiB flash, with a 256 KiB boot image possibly residing at offset zero or 0x3f00000, and the flash physical address (after boot code is done with BR0/OR0 games) is 0xfc000000. You could have a 64 MiB mapping at, say, 0xe0000000 always poing to 0xfc000000, and a 256 KiB mapping at 0xfff00000 pointing at either 0xfc000000 or 0xfff00000 depending on which you booted from. If somehow you could also boot from offset 0x2480000, then have 0xfff00000 point to 0xfe480000. The mapping at 0xe0000000 is independent and always points to 64 MiB at 0xfc000000. -Scott
On Wed, 24 Nov 2010 22:59:55 +0100 Wolfgang Denk <wd@denx.de> wrote: > Dear Joakim Tjernlund, > > In message <OF14AC2061.9896D2D3-ONC12577E5.0077F975-C12577E5.00785AA9@transmode.se> you wrote: > > > > Are you planning to apply some of my earlier patches(not this RFC series) > > or are you waiting for Freescale to Ack/pick up? So far nobody > > has said anything. > > I think these are all on Freescale's (Kim Phillips') list. outside of the PIC ones, which I believe are still outstanding, I have: Thu Nov 4 15:45:12 2010 +0100 "mpc83xx: Make it boot again" -- which looks like it has comments that haven't been addressed and: Tue Nov 23 19:33:43 2010 +0100 "mpc83xx: remove some dead code, saving space" -- which I've applied. Please let me know what else is needed, if anything. Thanks, Kim
Kim Phillips <kim.phillips@freescale.com> wrote on 2010/11/28 16:12:21: > > On Wed, 24 Nov 2010 22:59:55 +0100 > Wolfgang Denk <wd@denx.de> wrote: > > > Dear Joakim Tjernlund, > > > > In message <OF14AC2061.9896D2D3-ONC12577E5.0077F975-C12577E5.00785AA9@transmode.se> you wrote: > > > > > > Are you planning to apply some of my earlier patches(not this RFC series) > > > or are you waiting for Freescale to Ack/pick up? So far nobody > > > has said anything. > > > > I think these are all on Freescale's (Kim Phillips') list. > > outside of the PIC ones, which I believe are still outstanding, I have: > > Thu Nov 4 15:45:12 2010 +0100 "mpc83xx: Make it boot again" > > -- which looks like it has comments that haven't been addressed What comments would that be? There are none. Jocke PS. The last version has this fix for the problem in case you are looking at an old one. + /* Wait for HW to catch up */ + lwz r4, LBLAWAR1(r3) + twi 0,r4,0 + isync
diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c index f3b67ae..0437b49 100644 --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c @@ -534,7 +534,7 @@ int prt_83xx_rsr(void) sep = " "; for (i = 0; i < n; i++) if (rsr & bits[i].mask) { - printf("%s%s", sep, bits[i].desc); + printf("%s%s", sep, LINK_OFF(bits[i].desc)); sep = ", "; } puts("\n"); diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c index 7b09fb5..9fa99dc 100644 --- a/arch/powerpc/lib/board.c +++ b/arch/powerpc/lib/board.c @@ -386,7 +386,7 @@ void board_init_f (ulong bootflag) #endif for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) { - if ((*init_fnc_ptr) () != 0) { + if ((LINK_OFF(*init_fnc_ptr)) () != 0) { hang (); } }