* [U-Boot] [PATCH v2 1/3] mxs: Rename CONFIG_SPL_MX28_PSWITCH_WAIT to CONFIG_SPL_MXS_PSWITCH_WAIT
@ 2013-01-27 16:42 Otavio Salvador
2013-01-27 16:42 ` [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets Otavio Salvador
2013-01-27 16:42 ` [U-Boot] [PATCH v2 3/3] mx23evk: Adjust DRAM control register to use full 128MB of RAM Otavio Salvador
0 siblings, 2 replies; 12+ messages in thread
From: Otavio Salvador @ 2013-01-27 16:42 UTC (permalink / raw)
To: u-boot
The power switch option is compatible with i.MX23 and i.MX28 so the
configration option needs to reflect it. We choose
'CONFIG_SPL_MXS_PSWITCH_WAIT' for the option name.
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
Changes in v2:
- Rewrite short description as per Marek's request
arch/arm/cpu/arm926ejs/mxs/mxs_init.h | 2 +-
arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mxs/mxs_init.h b/arch/arm/cpu/arm926ejs/mxs/mxs_init.h
index 2ddc5bc..084def5 100644
--- a/arch/arm/cpu/arm926ejs/mxs/mxs_init.h
+++ b/arch/arm/cpu/arm926ejs/mxs/mxs_init.h
@@ -30,7 +30,7 @@ void early_delay(int delay);
void mxs_power_init(void);
-#ifdef CONFIG_SPL_MX28_PSWITCH_WAIT
+#ifdef CONFIG_SPL_MXS_PSWITCH_WAIT
void mxs_power_wait_pswitch(void);
#else
static inline void mxs_power_wait_pswitch(void) { }
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
index e9d6302..287c698 100644
--- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
+++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
@@ -921,7 +921,7 @@ void mxs_power_init(void)
early_delay(1000);
}
-#ifdef CONFIG_SPL_MX28_PSWITCH_WAIT
+#ifdef CONFIG_SPL_MXS_PSWITCH_WAIT
void mxs_power_wait_pswitch(void)
{
struct mxs_power_regs *power_regs =
--
1.8.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets
2013-01-27 16:42 [U-Boot] [PATCH v2 1/3] mxs: Rename CONFIG_SPL_MX28_PSWITCH_WAIT to CONFIG_SPL_MXS_PSWITCH_WAIT Otavio Salvador
@ 2013-01-27 16:42 ` Otavio Salvador
2013-01-27 17:16 ` Marek Vasut
2013-01-29 2:33 ` Marek Vasut
2013-01-27 16:42 ` [U-Boot] [PATCH v2 3/3] mx23evk: Adjust DRAM control register to use full 128MB of RAM Otavio Salvador
1 sibling, 2 replies; 12+ messages in thread
From: Otavio Salvador @ 2013-01-27 16:42 UTC (permalink / raw)
To: u-boot
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,
--
1.8.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 3/3] mx23evk: Adjust DRAM control register to use full 128MB of RAM
2013-01-27 16:42 [U-Boot] [PATCH v2 1/3] mxs: Rename CONFIG_SPL_MX28_PSWITCH_WAIT to CONFIG_SPL_MXS_PSWITCH_WAIT Otavio Salvador
2013-01-27 16:42 ` [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets Otavio Salvador
@ 2013-01-27 16:42 ` Otavio Salvador
1 sibling, 0 replies; 12+ messages in thread
From: Otavio Salvador @ 2013-01-27 16:42 UTC (permalink / raw)
To: u-boot
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
Changes in v2: None
board/freescale/mx23evk/spl_boot.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/board/freescale/mx23evk/spl_boot.c b/board/freescale/mx23evk/spl_boot.c
index 6007433..b6f4e7e 100644
--- a/board/freescale/mx23evk/spl_boot.c
+++ b/board/freescale/mx23evk/spl_boot.c
@@ -98,6 +98,16 @@ const iomux_cfg_t iomux_setup[] = {
(MXS_PAD_4MA | MXS_PAD_3V3 | MXS_PAD_NOPULL),
};
+#define HW_DRAM_CTL14 (0x38 >> 2)
+#define CS_MAP 0x3
+#define INTAREF 0x2
+#define HW_DRAM_CTL14_CONFIG (INTAREF << 8 | CS_MAP)
+
+void mxs_adjust_memory_params(uint32_t *dram_vals)
+{
+ dram_vals[HW_DRAM_CTL14] = HW_DRAM_CTL14_CONFIG;
+}
+
void board_init_ll(void)
{
mxs_common_spl_init(iomux_setup, ARRAY_SIZE(iomux_setup));
--
1.8.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets
2013-01-27 16:42 ` [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets Otavio Salvador
@ 2013-01-27 17:16 ` Marek Vasut
2013-01-27 17:25 ` Otavio Salvador
2013-01-29 2:33 ` Marek Vasut
1 sibling, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2013-01-27 17:16 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets
2013-01-27 17:16 ` Marek Vasut
@ 2013-01-27 17:25 ` Otavio Salvador
0 siblings, 0 replies; 12+ messages in thread
From: Otavio Salvador @ 2013-01-27 17:25 UTC (permalink / raw)
To: u-boot
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 at ossystems.com.br http://www.ossystems.com.br
Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets
2013-01-27 16:42 ` [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets Otavio Salvador
2013-01-27 17:16 ` Marek Vasut
@ 2013-01-29 2:33 ` Marek Vasut
2013-01-29 3:14 ` Marek Vasut
2013-01-29 10:10 ` Otavio Salvador
1 sibling, 2 replies; 12+ messages in thread
From: Marek Vasut @ 2013-01-29 2:33 UTC (permalink / raw)
To: u-boot
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@random now ? WHAT THE FUCK IS HAPPENING HERE ?!
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets
2013-01-29 2:33 ` Marek Vasut
@ 2013-01-29 3:14 ` Marek Vasut
2013-01-29 10:14 ` Otavio Salvador
2013-01-29 10:10 ` Otavio Salvador
1 sibling, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2013-01-29 3:14 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets
2013-01-29 2:33 ` Marek Vasut
2013-01-29 3:14 ` Marek Vasut
@ 2013-01-29 10:10 ` Otavio Salvador
2013-01-29 12:26 ` Marek Vasut
1 sibling, 1 reply; 12+ messages in thread
From: Otavio Salvador @ 2013-01-29 10:10 UTC (permalink / raw)
To: u-boot
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 at ossystems.com.br http://www.ossystems.com.br
Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets
2013-01-29 3:14 ` Marek Vasut
@ 2013-01-29 10:14 ` Otavio Salvador
2013-01-29 12:13 ` Marek Vasut
0 siblings, 1 reply; 12+ messages in thread
From: Otavio Salvador @ 2013-01-29 10:14 UTC (permalink / raw)
To: u-boot
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 at ossystems.com.br http://www.ossystems.com.br
Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets
2013-01-29 10:14 ` Otavio Salvador
@ 2013-01-29 12:13 ` Marek Vasut
0 siblings, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2013-01-29 12:13 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets
2013-01-29 10:10 ` Otavio Salvador
@ 2013-01-29 12:26 ` Marek Vasut
2013-01-29 12:28 ` Otavio Salvador
0 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2013-01-29 12:26 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets
2013-01-29 12:26 ` Marek Vasut
@ 2013-01-29 12:28 ` Otavio Salvador
0 siblings, 0 replies; 12+ messages in thread
From: Otavio Salvador @ 2013-01-29 12:28 UTC (permalink / raw)
To: u-boot
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 at ossystems.com.br http://www.ossystems.com.br
Mobile: +55 53 9981-7854 http://projetos.ossystems.com.br
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-01-29 12:28 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-27 16:42 [U-Boot] [PATCH v2 1/3] mxs: Rename CONFIG_SPL_MX28_PSWITCH_WAIT to CONFIG_SPL_MXS_PSWITCH_WAIT Otavio Salvador
2013-01-27 16:42 ` [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of HW_DRAM_CTL08) as in imx-bootlets Otavio Salvador
2013-01-27 17:16 ` Marek Vasut
2013-01-27 17:25 ` Otavio Salvador
2013-01-29 2:33 ` Marek Vasut
2013-01-29 3:14 ` Marek Vasut
2013-01-29 10:14 ` Otavio Salvador
2013-01-29 12:13 ` Marek Vasut
2013-01-29 10:10 ` Otavio Salvador
2013-01-29 12:26 ` Marek Vasut
2013-01-29 12:28 ` Otavio Salvador
2013-01-27 16:42 ` [U-Boot] [PATCH v2 3/3] mx23evk: Adjust DRAM control register to use full 128MB of RAM Otavio Salvador
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox