diff mbox

[U-Boot,v2,2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets

Message ID 1359304956-6540-2-git-send-email-otavio@ossystems.com.br
State Not Applicable
Headers show

Commit Message

Otavio Salvador Jan. 27, 2013, 4:42 p.m. UTC
This enables the 'Fast Auto Pre-Charge' found in the memory chip.

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
Changes in v2:
- Improve commit message

 arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marek Vasut Jan. 27, 2013, 5:16 p.m. UTC | #1
Dear Otavio Salvador,

> This enables the 'Fast Auto Pre-Charge' found in the memory chip.

The datasheet claims not all chips support this.

> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> ---
> Changes in v2:
> - Improve commit message
> 
>  arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 836e636..a9efd87 100644
> --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> @@ -90,7 +90,7 @@ static uint32_t dram_vals[] = {
>  #elif defined(CONFIG_MX23)
>  	0x01010001, 0x00010100, 0x01000101, 0x00000001,
>  	0x00000101, 0x00000000, 0x00010000, 0x01000001,
> -	0x00000000, 0x00000001, 0x07000200, 0x00070202,
> +	0x01000000, 0x00000001, 0x07000200, 0x00070202,
>  	0x02020000, 0x04040a01, 0x00000201, 0x02040000,
>  	0x02000000, 0x19000f08, 0x0d0d0000, 0x02021313,
>  	0x02061521, 0x0000000a, 0x00080008, 0x00200020,

Best regards,
Marek Vasut
Otavio Salvador Jan. 27, 2013, 5:25 p.m. UTC | #2
On Sun, Jan 27, 2013 at 3:16 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Otavio Salvador,
>
>> This enables the 'Fast Auto Pre-Charge' found in the memory chip.
>
> The datasheet claims not all chips support this.

By 'all chips' you mean memory chips, right?

It seems both used chips (mx23evk and olinuxlino) does. So I think
this can be the default and we change it in board if need.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Marek Vasut Jan. 29, 2013, 2:33 a.m. UTC | #3
Dear Otavio Salvador,

> This enables the 'Fast Auto Pre-Charge' found in the memory chip.
> 
> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> ---
> Changes in v2:
> - Improve commit message
> 
>  arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 836e636..a9efd87 100644
> --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> @@ -90,7 +90,7 @@ static uint32_t dram_vals[] = {
>  #elif defined(CONFIG_MX23)
>  	0x01010001, 0x00010100, 0x01000101, 0x00000001,
>  	0x00000101, 0x00000000, 0x00010000, 0x01000001,
> -	0x00000000, 0x00000001, 0x07000200, 0x00070202,
> +	0x01000000, 0x00000001, 0x07000200, 0x00070202,
>  	0x02020000, 0x04040a01, 0x00000201, 0x02040000,
>  	0x02000000, 0x19000f08, 0x0d0d0000, 0x02021313,
>  	0x02061521, 0x0000000a, 0x00080008, 0x00200020,

I went through the u-boot mem init and detected you apparently added the 
following undocumented portion of code (the writel((1 << 24 ...) already:

112 static void initialize_dram_values(void)
113 {
114         int i;
115 
116         mxs_adjust_memory_params(dram_vals);
117 
118         for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
119                 writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
120 
121 #ifdef CONFIG_MX23
122         writel((1 << 24), MXS_DRAM_BASE + (4 * 8));
123 #endif
124 }

It does enable the TRANS_LOCKOUT. So what the heck is going on here? Are you 
coding this stuff at random now ? WHAT THE FUCK IS HAPPENING HERE ?!

Best regards,
Marek Vasut
Marek Vasut Jan. 29, 2013, 3:14 a.m. UTC | #4
Dear Marek Vasut,

> Dear Otavio Salvador,
> 
> > This enables the 'Fast Auto Pre-Charge' found in the memory chip.
> > 
> > Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> > ---
> > Changes in v2:
> > - Improve commit message
> > 
> >  arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> > b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 836e636..a9efd87 100644
> > --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> > +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> > @@ -90,7 +90,7 @@ static uint32_t dram_vals[] = {
> > 
> >  #elif defined(CONFIG_MX23)
> >  
> >  	0x01010001, 0x00010100, 0x01000101, 0x00000001,
> >  	0x00000101, 0x00000000, 0x00010000, 0x01000001,
> > 
> > -	0x00000000, 0x00000001, 0x07000200, 0x00070202,
> > +	0x01000000, 0x00000001, 0x07000200, 0x00070202,
> > 
> >  	0x02020000, 0x04040a01, 0x00000201, 0x02040000,
> >  	0x02000000, 0x19000f08, 0x0d0d0000, 0x02021313,
> >  	0x02061521, 0x0000000a, 0x00080008, 0x00200020,
> 
> I went through the u-boot mem init and detected you apparently added the
> following undocumented portion of code (the writel((1 << 24 ...) already:
> 
> 112 static void initialize_dram_values(void)
> 113 {
> 114         int i;
> 115
> 116         mxs_adjust_memory_params(dram_vals);
> 117
> 118         for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
> 119                 writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
> 120
> 121 #ifdef CONFIG_MX23
> 122         writel((1 << 24), MXS_DRAM_BASE + (4 * 8));
> 123 #endif
> 124 }
[...]

Sorry about me blowing. Anyway, I better put down the message I would like to 
relay. Otavio, please follow these steps:

* Work in proper sequence -- patches must apply one after another. The same way 
you cannot build house from the roof to the ground, you can not apply patches in 
anachronistic order against their dependencies.
* Prove why your patch fixes issues -- apply proper reasoning. Do a proper 
research, there's no time-limit for sending a patch. There is no deadline, take 
your time.
* Step back and slow down -- please do not roll one patch after another, wait 
for more reviews. This does put a great deal of strain on everyone in the ML, so 
please be considerate ; you are flooding the mailing list for no reason ; you 
are also pushing too much work on the reviewers. Thus, wait for some reviews, 
then fix the issues and repost.
* Focus on the changes you make -- look at the stuff above, you need to properly 
study the code instead of rolling out random patches. Properly focus on a single 
task, finish it, then move on to the other task.

Hacking is not a race, it's an art .

Best regards,
Marek Vasut
Otavio Salvador Jan. 29, 2013, 10:10 a.m. UTC | #5
On Tue, Jan 29, 2013 at 12:33 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dear Otavio Salvador,
>
>> This enables the 'Fast Auto Pre-Charge' found in the memory chip.
>>
>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>> ---
>> Changes in v2:
>> - Improve commit message
>>
>>  arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
>> b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 836e636..a9efd87 100644
>> --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
>> +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
>> @@ -90,7 +90,7 @@ static uint32_t dram_vals[] = {
>>  #elif defined(CONFIG_MX23)
>>       0x01010001, 0x00010100, 0x01000101, 0x00000001,
>>       0x00000101, 0x00000000, 0x00010000, 0x01000001,
>> -     0x00000000, 0x00000001, 0x07000200, 0x00070202,
>> +     0x01000000, 0x00000001, 0x07000200, 0x00070202,
>>       0x02020000, 0x04040a01, 0x00000201, 0x02040000,
>>       0x02000000, 0x19000f08, 0x0d0d0000, 0x02021313,
>>       0x02061521, 0x0000000a, 0x00080008, 0x00200020,
>
> I went through the u-boot mem init and detected you apparently added the
> following undocumented portion of code (the writel((1 << 24 ...) already:
>
> 112 static void initialize_dram_values(void)
> 113 {
> 114         int i;
> 115
> 116         mxs_adjust_memory_params(dram_vals);
> 117
> 118         for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
> 119                 writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
> 120
> 121 #ifdef CONFIG_MX23
> 122         writel((1 << 24), MXS_DRAM_BASE + (4 * 8));
> 123 #endif
> 124 }
>
> It does enable the TRANS_LOCKOUT. So what the heck is going on here? Are you
> coding this stuff at random now ? WHAT THE FUCK IS HAPPENING HERE ?!

What your tone, please.

Indeed, it does it. It has been done loooong time ago when we started
looking at MX23 and it was not obvious for me it.

I will send a patch reverting it and adding a comment explaning it
there so it is documented.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Otavio Salvador Jan. 29, 2013, 10:14 a.m. UTC | #6
On Tue, Jan 29, 2013 at 1:14 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Marek Vasut,
>
>> Dear Otavio Salvador,
>>
>> > This enables the 'Fast Auto Pre-Charge' found in the memory chip.
>> >
>> > Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>> > ---
>> > Changes in v2:
>> > - Improve commit message
>> >
>> >  arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
>> > b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 836e636..a9efd87 100644
>> > --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
>> > +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
>> > @@ -90,7 +90,7 @@ static uint32_t dram_vals[] = {
>> >
>> >  #elif defined(CONFIG_MX23)
>> >
>> >     0x01010001, 0x00010100, 0x01000101, 0x00000001,
>> >     0x00000101, 0x00000000, 0x00010000, 0x01000001,
>> >
>> > -   0x00000000, 0x00000001, 0x07000200, 0x00070202,
>> > +   0x01000000, 0x00000001, 0x07000200, 0x00070202,
>> >
>> >     0x02020000, 0x04040a01, 0x00000201, 0x02040000,
>> >     0x02000000, 0x19000f08, 0x0d0d0000, 0x02021313,
>> >     0x02061521, 0x0000000a, 0x00080008, 0x00200020,
>>
>> I went through the u-boot mem init and detected you apparently added the
>> following undocumented portion of code (the writel((1 << 24 ...) already:
>>
>> 112 static void initialize_dram_values(void)
>> 113 {
>> 114         int i;
>> 115
>> 116         mxs_adjust_memory_params(dram_vals);
>> 117
>> 118         for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
>> 119                 writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
>> 120
>> 121 #ifdef CONFIG_MX23
>> 122         writel((1 << 24), MXS_DRAM_BASE + (4 * 8));
>> 123 #endif
>> 124 }
> [...]
>
> Sorry about me blowing. Anyway, I better put down the message I would like to
> relay. Otavio, please follow these steps:
>
> * Work in proper sequence -- patches must apply one after another. The same way
> you cannot build house from the roof to the ground, you can not apply patches in
> anachronistic order against their dependencies.
> * Prove why your patch fixes issues -- apply proper reasoning. Do a proper
> research, there's no time-limit for sending a patch. There is no deadline, take
> your time.
> * Step back and slow down -- please do not roll one patch after another, wait
> for more reviews. This does put a great deal of strain on everyone in the ML, so
> please be considerate ; you are flooding the mailing list for no reason ; you
> are also pushing too much work on the reviewers. Thus, wait for some reviews,
> then fix the issues and repost.
> * Focus on the changes you make -- look at the stuff above, you need to properly
> study the code instead of rolling out random patches. Properly focus on a single
> task, finish it, then move on to the other task.
>
> Hacking is not a race, it's an art .

I've been trying to be cooperative and to improve on the way. I (as
other humans) do mistakes but I try to fix them as soon as possible.

I've been trying to get MX23 in basic working state (for users) and do
wish to have it for next release and that's why I am trying to make
things fast; I am sorry by the patch but it is normal to make mistakes
on the way.

I am glad you noticed it but not so glad by the way you communicated it.

Regards,


--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Marek Vasut Jan. 29, 2013, 12:13 p.m. UTC | #7
Dear Otavio Salvador,

> On Tue, Jan 29, 2013 at 1:14 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Marek Vasut,
> > 
> >> Dear Otavio Salvador,
> >> 
> >> > This enables the 'Fast Auto Pre-Charge' found in the memory chip.
> >> > 
> >> > Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> >> > ---
> >> > Changes in v2:
> >> > - Improve commit message
> >> > 
> >> >  arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> > 
> >> > diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> >> > b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 836e636..a9efd87
> >> > 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> >> > +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> >> > @@ -90,7 +90,7 @@ static uint32_t dram_vals[] = {
> >> > 
> >> >  #elif defined(CONFIG_MX23)
> >> >  
> >> >     0x01010001, 0x00010100, 0x01000101, 0x00000001,
> >> >     0x00000101, 0x00000000, 0x00010000, 0x01000001,
> >> > 
> >> > -   0x00000000, 0x00000001, 0x07000200, 0x00070202,
> >> > +   0x01000000, 0x00000001, 0x07000200, 0x00070202,
> >> > 
> >> >     0x02020000, 0x04040a01, 0x00000201, 0x02040000,
> >> >     0x02000000, 0x19000f08, 0x0d0d0000, 0x02021313,
> >> >     0x02061521, 0x0000000a, 0x00080008, 0x00200020,
> >> 
> >> I went through the u-boot mem init and detected you apparently added the
> >> following undocumented portion of code (the writel((1 << 24 ...)
> >> already:
> >> 
> >> 112 static void initialize_dram_values(void)
> >> 113 {
> >> 114         int i;
> >> 115
> >> 116         mxs_adjust_memory_params(dram_vals);
> >> 117
> >> 118         for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
> >> 119                 writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
> >> 120
> >> 121 #ifdef CONFIG_MX23
> >> 122         writel((1 << 24), MXS_DRAM_BASE + (4 * 8));
> >> 123 #endif
> >> 124 }
> > 
> > [...]
> > 
> > Sorry about me blowing. Anyway, I better put down the message I would
> > like to relay. Otavio, please follow these steps:
> > 
> > * Work in proper sequence -- patches must apply one after another. The
> > same way you cannot build house from the roof to the ground, you can not
> > apply patches in anachronistic order against their dependencies.
> > * Prove why your patch fixes issues -- apply proper reasoning. Do a
> > proper research, there's no time-limit for sending a patch. There is no
> > deadline, take your time.
> > * Step back and slow down -- please do not roll one patch after another,
> > wait for more reviews. This does put a great deal of strain on everyone
> > in the ML, so please be considerate ; you are flooding the mailing list
> > for no reason ; you are also pushing too much work on the reviewers.
> > Thus, wait for some reviews, then fix the issues and repost.
> > * Focus on the changes you make -- look at the stuff above, you need to
> > properly study the code instead of rolling out random patches. Properly
> > focus on a single task, finish it, then move on to the other task.
> > 
> > Hacking is not a race, it's an art .
> 
> I've been trying to be cooperative and to improve on the way. I (as
> other humans) do mistakes but I try to fix them as soon as possible.
> 
> I've been trying to get MX23 in basic working state (for users) and do
> wish to have it for next release and that's why I am trying to make
> things fast; I am sorry by the patch but it is normal to make mistakes
> on the way.
> 
> I am glad you noticed it but not so glad by the way you communicated it.

Slow down already. We can apply minor things after the MW. If you go at this 
pace, it helps noone really. So just take it easy, carefully review the patches 
etc.

Best regards,
Marek Vasut
Marek Vasut Jan. 29, 2013, 12:26 p.m. UTC | #8
Dear Otavio Salvador,

> On Tue, Jan 29, 2013 at 12:33 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dear Otavio Salvador,
> > 
> >> This enables the 'Fast Auto Pre-Charge' found in the memory chip.
> >> 
> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> >> ---
> >> Changes in v2:
> >> - Improve commit message
> >> 
> >>  arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> >> b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 836e636..a9efd87
> >> 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> >> +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> >> @@ -90,7 +90,7 @@ static uint32_t dram_vals[] = {
> >> 
> >>  #elif defined(CONFIG_MX23)
> >>  
> >>       0x01010001, 0x00010100, 0x01000101, 0x00000001,
> >>       0x00000101, 0x00000000, 0x00010000, 0x01000001,
> >> 
> >> -     0x00000000, 0x00000001, 0x07000200, 0x00070202,
> >> +     0x01000000, 0x00000001, 0x07000200, 0x00070202,
> >> 
> >>       0x02020000, 0x04040a01, 0x00000201, 0x02040000,
> >>       0x02000000, 0x19000f08, 0x0d0d0000, 0x02021313,
> >>       0x02061521, 0x0000000a, 0x00080008, 0x00200020,
> > 
> > I went through the u-boot mem init and detected you apparently added the
> > following undocumented portion of code (the writel((1 << 24 ...) already:
> > 
> > 112 static void initialize_dram_values(void)
> > 113 {
> > 114         int i;
> > 115
> > 116         mxs_adjust_memory_params(dram_vals);
> > 117
> > 118         for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
> > 119                 writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
> > 120
> > 121 #ifdef CONFIG_MX23
> > 122         writel((1 << 24), MXS_DRAM_BASE + (4 * 8));
> > 123 #endif
> > 124 }
> > 
> > It does enable the TRANS_LOCKOUT. So what the heck is going on here? Are
> > you coding this stuff at random now ? WHAT THE FUCK IS HAPPENING HERE ?!
> 
> What your tone, please.

My tone reflects my frustration here.

> Indeed, it does it. It has been done loooong time ago when we started
> looking at MX23 and it was not obvious for me it.
> 
> I will send a patch reverting it and adding a comment explaning it
> there so it is documented.

No! Again, you are charging forward without thinking first!

Why was this code written like that in the first place? Why was this bit set 
later instead of during the register programming in the first place?

Best regards,
Marek Vasut
Otavio Salvador Jan. 29, 2013, 12:28 p.m. UTC | #9
On Tue, Jan 29, 2013 at 10:26 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Dear Otavio Salvador,
>
>> On Tue, Jan 29, 2013 at 12:33 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> > Dear Otavio Salvador,
>> >
>> >> This enables the 'Fast Auto Pre-Charge' found in the memory chip.
>> >>
>> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>> >> ---
>> >> Changes in v2:
>> >> - Improve commit message
>> >>
>> >>  arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
>> >> b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 836e636..a9efd87
>> >> 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
>> >> +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
>> >> @@ -90,7 +90,7 @@ static uint32_t dram_vals[] = {
>> >>
>> >>  #elif defined(CONFIG_MX23)
>> >>
>> >>       0x01010001, 0x00010100, 0x01000101, 0x00000001,
>> >>       0x00000101, 0x00000000, 0x00010000, 0x01000001,
>> >>
>> >> -     0x00000000, 0x00000001, 0x07000200, 0x00070202,
>> >> +     0x01000000, 0x00000001, 0x07000200, 0x00070202,
>> >>
>> >>       0x02020000, 0x04040a01, 0x00000201, 0x02040000,
>> >>       0x02000000, 0x19000f08, 0x0d0d0000, 0x02021313,
>> >>       0x02061521, 0x0000000a, 0x00080008, 0x00200020,
>> >
>> > I went through the u-boot mem init and detected you apparently added the
>> > following undocumented portion of code (the writel((1 << 24 ...) already:
>> >
>> > 112 static void initialize_dram_values(void)
>> > 113 {
>> > 114         int i;
>> > 115
>> > 116         mxs_adjust_memory_params(dram_vals);
>> > 117
>> > 118         for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
>> > 119                 writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
>> > 120
>> > 121 #ifdef CONFIG_MX23
>> > 122         writel((1 << 24), MXS_DRAM_BASE + (4 * 8));
>> > 123 #endif
>> > 124 }
>> >
>> > It does enable the TRANS_LOCKOUT. So what the heck is going on here? Are
>> > you coding this stuff at random now ? WHAT THE FUCK IS HAPPENING HERE ?!
>>
>> What your tone, please.
>
> My tone reflects my frustration here.

This does not give you the right to be hard at someone, specially when
someone does not make  a mistake by propose.

>> Indeed, it does it. It has been done loooong time ago when we started
>> looking at MX23 and it was not obvious for me it.
>>
>> I will send a patch reverting it and adding a comment explaning it
>> there so it is documented.
>
> No! Again, you are charging forward without thinking first!
>
> Why was this code written like that in the first place? Why was this bit set
> later instead of during the register programming in the first place?

Check the patchset I sent before to shoot, please.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
diff mbox

Patch

diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
index 836e636..a9efd87 100644
--- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
+++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
@@ -90,7 +90,7 @@  static uint32_t dram_vals[] = {
 #elif defined(CONFIG_MX23)
 	0x01010001, 0x00010100, 0x01000101, 0x00000001,
 	0x00000101, 0x00000000, 0x00010000, 0x01000001,
-	0x00000000, 0x00000001, 0x07000200, 0x00070202,
+	0x01000000, 0x00000001, 0x07000200, 0x00070202,
 	0x02020000, 0x04040a01, 0x00000201, 0x02040000,
 	0x02000000, 0x19000f08, 0x0d0d0000, 0x02021313,
 	0x02061521, 0x0000000a, 0x00080008, 0x00200020,