| Message ID | 1338356593-6897-1-git-send-email-marex@denx.de |
|---|---|
| State | Accepted |
| Commit | e3ddc646031551627c4eeae4598f8862251a3f94 |
| Delegated to: | Stefano Babic |
| Headers | show |
Hi Marek, On Wed, May 30, 2012 at 2:43 AM, Marek Vasut <marex@denx.de> wrote: > This function can be overridden at run-time and allows implementors > of new boards based on the i.MX28 chip to fine-tune the memory params. > It is possible to write into the dram_vals array because when the SPL > runs, it is located SRAM. Therefore the location is writable. There is > no possibility of these data to be read-only. This still does not fix the mx28evk booting issue on u-boot-imx tree. Have you tried to boot m28evk with the following commit reverted? commit 19a2066b578f826c7bcb2b96a90434382e8ec8f3 Author: Marek Vasut <marex@denx.de> Date: Thu May 3 05:47:19 2012 +0000 M28: Scan only first 512 MB of DRAM to avoid memory wraparound
Dear Fabio Estevam, > Hi Marek, > > On Wed, May 30, 2012 at 2:43 AM, Marek Vasut <marex@denx.de> wrote: > > This function can be overridden at run-time and allows implementors > > of new boards based on the i.MX28 chip to fine-tune the memory params. > > It is possible to write into the dram_vals array because when the SPL > > runs, it is located SRAM. Therefore the location is writable. There is > > no possibility of these data to be read-only. > > This still does not fix the mx28evk booting issue on u-boot-imx tree. > > Have you tried to boot m28evk with the following commit reverted? It's broken, that's why the following patch was applied ;-) Call this or similar in your spl_boot.c to adjust the DRAM configuration. Actually I think even the following value should fix your problem: void mx28_adjust_memory_params(uint32_t *dram_vals) { dram_vals[0x74 >> 2] = 0x0f02010a; } > > commit 19a2066b578f826c7bcb2b96a90434382e8ec8f3 > Author: Marek Vasut <marex@denx.de> > Date: Thu May 3 05:47:19 2012 +0000 > > M28: Scan only first 512 MB of DRAM to avoid memory wraparound Best regards, Marek Vasut
Hi Marek, On Thu, May 31, 2012 at 3:45 PM, Marek Vasut <marex@denx.de> wrote: > Call this or similar in your spl_boot.c to adjust the DRAM configuration. > Actually I think even the following value should fix your problem: > > void mx28_adjust_memory_params(uint32_t *dram_vals) > { > dram_vals[0x74 >> 2] = 0x0f02010a; > } I finally had a chance to try this and my mx28evk can boot again with your patch + the code above. How do we handle this? Would you send a v2 with this additional code or should I do it myself after your original patch in this thread gets applied? Also, please explain to the mortal folks what 'dram_vals[0x74 >> 2] = 0x0f02010a;' means. It is not something very trivial to figure out :-) Thanks, Fabio Estevam
Dear Fabio Estevam, > Hi Marek, > > On Thu, May 31, 2012 at 3:45 PM, Marek Vasut <marex@denx.de> wrote: > > Call this or similar in your spl_boot.c to adjust the DRAM configuration. > > Actually I think even the following value should fix your problem: > > > > void mx28_adjust_memory_params(uint32_t *dram_vals) > > { > > dram_vals[0x74 >> 2] = 0x0f02010a; > > } > > I finally had a chance to try this and my mx28evk can boot again with > your patch + the code above. > > How do we handle this? Would you send a v2 with this additional code > or should I do it myself after your original patch in this thread gets > applied? Stefano, can you please apply this? So Fabio can apply his on top of it? > Also, please explain to the mortal folks what 'dram_vals[0x74 >> 2] = > 0x0f02010a;' means. Well, check that particular register (memory configuration register at +0x74 offset) in the datasheet. It's basically adjusting the number of enabled address row and columns. > It is not something very trivial to figure out :-) It actually is, see above ;-) > Thanks, > > Fabio Estevam Best regards, Marek Vasut
On 18/06/2012 16:23, Marek Vasut wrote: > Dear Fabio Estevam, > >> Hi Marek, >> >> On Thu, May 31, 2012 at 3:45 PM, Marek Vasut <marex@denx.de> wrote: >>> Call this or similar in your spl_boot.c to adjust the DRAM configuration. >>> Actually I think even the following value should fix your problem: >>> >>> void mx28_adjust_memory_params(uint32_t *dram_vals) >>> { >>> dram_vals[0x74 >> 2] = 0x0f02010a; >>> } >> >> I finally had a chance to try this and my mx28evk can boot again with >> your patch + the code above. >> >> How do we handle this? Would you send a v2 with this additional code >> or should I do it myself after your original patch in this thread gets >> applied? > > Stefano, can you please apply this? So Fabio can apply his on top of it? I do it. > >> Also, please explain to the mortal folks what 'dram_vals[0x74 >> 2] = >> 0x0f02010a;' means. > > Well, check that particular register (memory configuration register at +0x74 > offset) in the datasheet. It's basically adjusting the number of enabled address > row and columns. > >> It is not something very trivial to figure out :-) > > It actually is, see above ;-) I see, but I will never apply a patch programmed in hexadecimal instead of plain C ;-) Stefano
Dear Stefano Babic, > On 18/06/2012 16:23, Marek Vasut wrote: > > Dear Fabio Estevam, > > > >> Hi Marek, > >> > >> On Thu, May 31, 2012 at 3:45 PM, Marek Vasut <marex@denx.de> wrote: > >>> Call this or similar in your spl_boot.c to adjust the DRAM > >>> configuration. Actually I think even the following value should fix > >>> your problem: > >>> > >>> void mx28_adjust_memory_params(uint32_t *dram_vals) > >>> { > >>> > >>> dram_vals[0x74 >> 2] = 0x0f02010a; > >>> > >>> } > >> > >> I finally had a chance to try this and my mx28evk can boot again with > >> your patch + the code above. > >> > >> How do we handle this? Would you send a v2 with this additional code > >> or should I do it myself after your original patch in this thread gets > >> applied? > > > > Stefano, can you please apply this? So Fabio can apply his on top of it? > > I do it. > > >> Also, please explain to the mortal folks what 'dram_vals[0x74 >> 2] = > >> 0x0f02010a;' means. > > > > Well, check that particular register (memory configuration register at > > +0x74 offset) in the datasheet. It's basically adjusting the number of > > enabled address row and columns. > > > >> It is not something very trivial to figure out :-) > > > > It actually is, see above ;-) > > I see, but I will never apply a patch programmed in hexadecimal instead > of plain C ;-) So ... what do you expect me to do? The register is called HW_DRAM_CTL29 (very descriptive) ... do you want me to create a file full of HW_DRAM_CTLnn ? > > Stefano Best regards, Marek Vasut
Dear Stefano Babic, > > On 18/06/2012 16:23, Marek Vasut wrote: > > > Dear Fabio Estevam, > > > > > >> Hi Marek, > > >> > > >> On Thu, May 31, 2012 at 3:45 PM, Marek Vasut <marex@denx.de> wrote: > > >>> Call this or similar in your spl_boot.c to adjust the DRAM > > >>> configuration. Actually I think even the following value should fix > > >>> your problem: > > >>> > > >>> void mx28_adjust_memory_params(uint32_t *dram_vals) > > >>> { > > >>> > > >>> dram_vals[0x74 >> 2] = 0x0f02010a; > > >>> > > >>> } > > >> > > >> I finally had a chance to try this and my mx28evk can boot again with > > >> your patch + the code above. > > >> > > >> How do we handle this? Would you send a v2 with this additional code > > >> or should I do it myself after your original patch in this thread gets > > >> applied? > > > > > > Stefano, can you please apply this? So Fabio can apply his on top of > > > it? > > > > I do it. > > > > >> Also, please explain to the mortal folks what 'dram_vals[0x74 >> 2] = > > >> 0x0f02010a;' means. > > > > > > Well, check that particular register (memory configuration register at > > > +0x74 offset) in the datasheet. It's basically adjusting the number of > > > enabled address row and columns. > > > > > >> It is not something very trivial to figure out :-) > > > > > > It actually is, see above ;-) > > > > I see, but I will never apply a patch programmed in hexadecimal instead > > of plain C ;-) > > So ... what do you expect me to do? The register is called HW_DRAM_CTL29 > (very descriptive) ... do you want me to create a file full of > HW_DRAM_CTLnn ? Would adding such comment be enough to satisfy you? (In slightly more polite manner I mean ... damn I'm loosing it, 2 days until exam :'-( ). Sorry for being rough, Stefano. > > Stefano Best regards, Marek Vasut
On 18/06/2012 17:43, Marek Vasut wrote: >> >> So ... what do you expect me to do? The register is called HW_DRAM_CTL29 >> (very descriptive) ... do you want me to create a file full of >> HW_DRAM_CTLnn ? > > Would adding such comment be enough to satisfy you? (In slightly more polite > manner I mean ... damn I'm loosing it, 2 days until exam :'-( ). Sorry for being > rough, Stefano. Do not worry, it is normal before exam ;-) Yes, of course comments will help ! Best regards, Stefano
Dear Stefano Babic, > On 18/06/2012 17:43, Marek Vasut wrote: > >> So ... what do you expect me to do? The register is called HW_DRAM_CTL29 > >> (very descriptive) ... do you want me to create a file full of > >> HW_DRAM_CTLnn ? > > > > Would adding such comment be enough to satisfy you? (In slightly more > > polite manner I mean ... damn I'm loosing it, 2 days until exam :'-( ). > > Sorry for being rough, Stefano. > > Do not worry, it is normal before exam ;-) This ... is not exam. This ... is HELL!!! > Yes, of course comments will help ! Wait ... it isn't my fault, I didn't publish any code that uses the function yet, that encodes stuff in hexadecimal. So this patch can be applied as is. It's Fabio's patch that's at fault here (and I hereby dump all responsibility on Fabio, keh keh :-) ). But anyway ... we're gonna have this used a lot and I'm not quite sure how to handle it. So if comment is enough (aka. I'm hereby adjusting HW_DRAM_CTL29 in order to set enabled rows and cols differently, refer to "14.8.27 in mx28 TRM"), let's go with it. Later, I think we should rework the mem init part. It's there from bootlets and I think it's all shitty because of that. Well ... the whole SPL is shitty because it's from bootlets, but that's a longer run, out of scope of this discussion. > Best regards, > Stefano Best regards, Marek Vasut
On 30/05/2012 07:43, Marek Vasut wrote: > This function can be overridden at run-time and allows implementors > of new boards based on the i.MX28 chip to fine-tune the memory params. > It is possible to write into the dram_vals array because when the SPL > runs, it is located SRAM. Therefore the location is writable. There is > no possibility of these data to be read-only. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Wolfgang Denk <wd@denx.de> > Cc: Detlev Zundel <dzu@denx.de> > Cc: Stefano Babic <sbabic@denx.de> > Cc: Fabio Estevam <festevam@gmail.com> > --- Applied to u-boot-imx, thanks. Best regards, Stefano Babic
diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c index 9fa5d29..e17a4d7 100644 --- a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c +++ b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c @@ -82,10 +82,18 @@ uint32_t dram_vals[] = { 0x00000000, 0x00010001 }; +void __mx28_adjust_memory_params(uint32_t *dram_vals) +{ +} +void mx28_adjust_memory_params(uint32_t *dram_vals) + __attribute__((weak, alias("__mx28_adjust_memory_params"))); + void init_m28_200mhz_ddr2(void) { int i; + mx28_adjust_memory_params(dram_vals); + for (i = 0; i < ARRAY_SIZE(dram_vals); i++) writel(dram_vals[i], MXS_DRAM_BASE + (4 * i)); }
This function can be overridden at run-time and allows implementors of new boards based on the i.MX28 chip to fine-tune the memory params. It is possible to write into the dram_vals array because when the SPL runs, it is located SRAM. Therefore the location is writable. There is no possibility of these data to be read-only. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Wolfgang Denk <wd@denx.de> Cc: Detlev Zundel <dzu@denx.de> Cc: Stefano Babic <sbabic@denx.de> Cc: Fabio Estevam <festevam@gmail.com> --- arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c | 8 ++++++++ 1 file changed, 8 insertions(+)