* [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation.
@ 2015-09-02 11:41 Joakim Tjernlund
2015-09-02 13:35 ` York Sun
0 siblings, 1 reply; 7+ messages in thread
From: Joakim Tjernlund @ 2015-09-02 11:41 UTC (permalink / raw)
To: u-boot
T1040 RM specifies CLK_ADJUST as 5 bits starting at bit pos 9
in DDR_DDR_SDRAM_CLK_CNTL, update code to match.
Signed-off-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>
---
drivers/ddr/fsl/ctrl_regs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
index 3919257..57077e1 100644
--- a/drivers/ddr/fsl/ctrl_regs.c
+++ b/drivers/ddr/fsl/ctrl_regs.c
@@ -1756,7 +1756,7 @@ static void set_ddr_sdram_clk_cntl(fsl_ddr_cfg_regs_t *ddr,
clk_adjust = popts->clk_adjust;
ddr->ddr_sdram_clk_cntl = (0
| ((ss_en & 0x1) << 31)
- | ((clk_adjust & 0xF) << 23)
+ | ((clk_adjust & 0x1F) << 22)
);
debug("FSLDDR: clk_cntl = 0x%08x\n", ddr->ddr_sdram_clk_cntl);
}
--
2.4.6
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation.
2015-09-02 11:41 [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation Joakim Tjernlund
@ 2015-09-02 13:35 ` York Sun
2015-09-02 14:31 ` Joakim Tjernlund
0 siblings, 1 reply; 7+ messages in thread
From: York Sun @ 2015-09-02 13:35 UTC (permalink / raw)
To: u-boot
On 09/02/2015 06:41 AM, Joakim Tjernlund wrote:
> T1040 RM specifies CLK_ADJUST as 5 bits starting at bit pos 9
> in DDR_DDR_SDRAM_CLK_CNTL, update code to match.
>
> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> ---
> drivers/ddr/fsl/ctrl_regs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
> index 3919257..57077e1 100644
> --- a/drivers/ddr/fsl/ctrl_regs.c
> +++ b/drivers/ddr/fsl/ctrl_regs.c
> @@ -1756,7 +1756,7 @@ static void set_ddr_sdram_clk_cntl(fsl_ddr_cfg_regs_t *ddr,
> clk_adjust = popts->clk_adjust;
> ddr->ddr_sdram_clk_cntl = (0
> | ((ss_en & 0x1) << 31)
> - | ((clk_adjust & 0xF) << 23)
> + | ((clk_adjust & 0x1F) << 22)
> );
> debug("FSLDDR: clk_cntl = 0x%08x\n", ddr->ddr_sdram_clk_cntl);
> }
>
NACK. This is logically correct but it needs to consider all DDR controllers.
Older controllers don't have the same bits for this field. So far we don't have
to use this this fine granularity on clock adjustment.
York
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation.
2015-09-02 13:35 ` York Sun
@ 2015-09-02 14:31 ` Joakim Tjernlund
2015-09-02 14:34 ` York Sun
0 siblings, 1 reply; 7+ messages in thread
From: Joakim Tjernlund @ 2015-09-02 14:31 UTC (permalink / raw)
To: u-boot
On Wed, 2015-09-02 at 08:35 -0500, York Sun wrote:
>
> On 09/02/2015 06:41 AM, Joakim Tjernlund wrote:
> > T1040 RM specifies CLK_ADJUST as 5 bits starting at bit pos 9
> > in DDR_DDR_SDRAM_CLK_CNTL, update code to match.
> >
> > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> > ---
> > drivers/ddr/fsl/ctrl_regs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
> > index 3919257..57077e1 100644
> > --- a/drivers/ddr/fsl/ctrl_regs.c
> > +++ b/drivers/ddr/fsl/ctrl_regs.c
> > @@ -1756,7 +1756,7 @@ static void set_ddr_sdram_clk_cntl(fsl_ddr_cfg_regs_t *ddr,
> > clk_adjust = popts->clk_adjust;
> > ddr->ddr_sdram_clk_cntl = (0
> > | ((ss_en & 0x1) << 31)
> > - | ((clk_adjust & 0xF) << 23)
> > + | ((clk_adjust & 0x1F) << 22)
> > );
> > debug("FSLDDR: clk_cntl = 0x%08x\n", ddr->ddr_sdram_clk_cntl);
> > }
> >
>
> NACK. This is logically correct but it needs to consider all DDR controllers.
> Older controllers don't have the same bits for this field. So far we don't have
> to use this this fine granularity on clock adjustment.
I have no idea what older controllers do with these bits so I leave it to you to find out.
Anyhow, current code is broken as the programmed value is doubled(we see this on our board
which cannot boot linux without this fix)
Jocke
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation.
2015-09-02 14:31 ` Joakim Tjernlund
@ 2015-09-02 14:34 ` York Sun
2015-09-02 14:42 ` Joakim Tjernlund
0 siblings, 1 reply; 7+ messages in thread
From: York Sun @ 2015-09-02 14:34 UTC (permalink / raw)
To: u-boot
On 09/02/2015 09:31 AM, Joakim Tjernlund wrote:
> On Wed, 2015-09-02 at 08:35 -0500, York Sun wrote:
>>
>> On 09/02/2015 06:41 AM, Joakim Tjernlund wrote:
>>> T1040 RM specifies CLK_ADJUST as 5 bits starting at bit pos 9
>>> in DDR_DDR_SDRAM_CLK_CNTL, update code to match.
>>>
>>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>
>>> ---
>>> drivers/ddr/fsl/ctrl_regs.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
>>> index 3919257..57077e1 100644
>>> --- a/drivers/ddr/fsl/ctrl_regs.c
>>> +++ b/drivers/ddr/fsl/ctrl_regs.c
>>> @@ -1756,7 +1756,7 @@ static void set_ddr_sdram_clk_cntl(fsl_ddr_cfg_regs_t *ddr,
>>> clk_adjust = popts->clk_adjust;
>>> ddr->ddr_sdram_clk_cntl = (0
>>> | ((ss_en & 0x1) << 31)
>>> - | ((clk_adjust & 0xF) << 23)
>>> + | ((clk_adjust & 0x1F) << 22)
>>> );
>>> debug("FSLDDR: clk_cntl = 0x%08x\n", ddr->ddr_sdram_clk_cntl);
>>> }
>>>
>>
>> NACK. This is logically correct but it needs to consider all DDR controllers.
>> Older controllers don't have the same bits for this field. So far we don't have
>> to use this this fine granularity on clock adjustment.
>
> I have no idea what older controllers do with these bits so I leave it to you to find out.
>
> Anyhow, current code is broken as the programmed value is doubled(we see this on our board
> which cannot boot linux without this fix)
Jocke,
Can you refer to Freescale board? You may be using correct clk_adjust value on
your board. But when you use this driver, you may have to adjust the value by
shifting 1 bit. You will lose some values but we haven't found it difficult.
This value is important but can also tolerate a wide range.
York
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation.
2015-09-02 14:34 ` York Sun
@ 2015-09-02 14:42 ` Joakim Tjernlund
2015-09-02 14:51 ` York Sun
0 siblings, 1 reply; 7+ messages in thread
From: Joakim Tjernlund @ 2015-09-02 14:42 UTC (permalink / raw)
To: u-boot
On Wed, 2015-09-02 at 09:34 -0500, York Sun wrote:
>
> On 09/02/2015 09:31 AM, Joakim Tjernlund wrote:
> > On Wed, 2015-09-02 at 08:35 -0500, York Sun wrote:
> > >
> > > On 09/02/2015 06:41 AM, Joakim Tjernlund wrote:
> > > > T1040 RM specifies CLK_ADJUST as 5 bits starting at bit pos 9
> > > > in DDR_DDR_SDRAM_CLK_CNTL, update code to match.
> > > >
> > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> > > > ---
> > > > drivers/ddr/fsl/ctrl_regs.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
> > > > index 3919257..57077e1 100644
> > > > --- a/drivers/ddr/fsl/ctrl_regs.c
> > > > +++ b/drivers/ddr/fsl/ctrl_regs.c
> > > > @@ -1756,7 +1756,7 @@ static void set_ddr_sdram_clk_cntl(fsl_ddr_cfg_regs_t *ddr,
> > > > clk_adjust = popts->clk_adjust;
> > > > ddr->ddr_sdram_clk_cntl = (0
> > > > | ((ss_en & 0x1) << 31)
> > > > - | ((clk_adjust & 0xF) << 23)
> > > > + | ((clk_adjust & 0x1F) << 22)
> > > > );
> > > > debug("FSLDDR: clk_cntl = 0x%08x\n", ddr->ddr_sdram_clk_cntl);
> > > > }
> > > >
> > >
> > > NACK. This is logically correct but it needs to consider all DDR controllers.
> > > Older controllers don't have the same bits for this field. So far we don't have
> > > to use this this fine granularity on clock adjustment.
> >
> > I have no idea what older controllers do with these bits so I leave it to you to find out.
> >
> > Anyhow, current code is broken as the programmed value is doubled(we see this on our board
> > which cannot boot linux without this fix)
>
> Jocke,
>
> Can you refer to Freescale board? You may be using correct clk_adjust value on
Custom board based on T1042RDB
> your board. But when you use this driver, you may have to adjust the value by
> shifting 1 bit. You will lose some values but we haven't found it difficult.
> This value is important but can also tolerate a wide range.
Sure, but how should anyone know that beforehand? The clk_adjust was calculated using
Freescales CW tool and added to table with the write lvl data.
Are the values for the T104x boards already compensated like you suggest?
Then they will (possibly) break when this bug is fixed.
Jocke
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation.
2015-09-02 14:42 ` Joakim Tjernlund
@ 2015-09-02 14:51 ` York Sun
2015-09-02 15:06 ` Joakim Tjernlund
0 siblings, 1 reply; 7+ messages in thread
From: York Sun @ 2015-09-02 14:51 UTC (permalink / raw)
To: u-boot
On 09/02/2015 09:42 AM, Joakim Tjernlund wrote:
> On Wed, 2015-09-02 at 09:34 -0500, York Sun wrote:
>>
>> On 09/02/2015 09:31 AM, Joakim Tjernlund wrote:
>>> On Wed, 2015-09-02 at 08:35 -0500, York Sun wrote:
>>>>
>>>> On 09/02/2015 06:41 AM, Joakim Tjernlund wrote:
>>>>> T1040 RM specifies CLK_ADJUST as 5 bits starting at bit pos 9
>>>>> in DDR_DDR_SDRAM_CLK_CNTL, update code to match.
>>>>>
>>>>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>
>>>>> ---
>>>>> drivers/ddr/fsl/ctrl_regs.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
>>>>> index 3919257..57077e1 100644
>>>>> --- a/drivers/ddr/fsl/ctrl_regs.c
>>>>> +++ b/drivers/ddr/fsl/ctrl_regs.c
>>>>> @@ -1756,7 +1756,7 @@ static void set_ddr_sdram_clk_cntl(fsl_ddr_cfg_regs_t *ddr,
>>>>> clk_adjust = popts->clk_adjust;
>>>>> ddr->ddr_sdram_clk_cntl = (0
>>>>> | ((ss_en & 0x1) << 31)
>>>>> - | ((clk_adjust & 0xF) << 23)
>>>>> + | ((clk_adjust & 0x1F) << 22)
>>>>> );
>>>>> debug("FSLDDR: clk_cntl = 0x%08x\n", ddr->ddr_sdram_clk_cntl);
>>>>> }
>>>>>
>>>>
>>>> NACK. This is logically correct but it needs to consider all DDR controllers.
>>>> Older controllers don't have the same bits for this field. So far we don't have
>>>> to use this this fine granularity on clock adjustment.
>>>
>>> I have no idea what older controllers do with these bits so I leave it to you to find out.
>>>
>>> Anyhow, current code is broken as the programmed value is doubled(we see this on our board
>>> which cannot boot linux without this fix)
>>
>> Jocke,
>>
>> Can you refer to Freescale board? You may be using correct clk_adjust value on
> Custom board based on T1042RDB
>
>> your board. But when you use this driver, you may have to adjust the value by
>> shifting 1 bit. You will lose some values but we haven't found it difficult.
>> This value is important but can also tolerate a wide range.
>
> Sure, but how should anyone know that beforehand? The clk_adjust was calculated using
> Freescales CW tool and added to table with the write lvl data.
>
> Are the values for the T104x boards already compensated like you suggest?
> Then they will (possibly) break when this bug is fixed.
Jocke,
I understand the frustration for having tools disconnected.
The CW tool gives:file you a suggestion but you have to put it in u-boot
differently. Freescale boards including T104x have correct values, for example
board/freescale/t104xrdb/ddr.h.
I don't consider it a bug for backward compatibility.
York
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation.
2015-09-02 14:51 ` York Sun
@ 2015-09-02 15:06 ` Joakim Tjernlund
0 siblings, 0 replies; 7+ messages in thread
From: Joakim Tjernlund @ 2015-09-02 15:06 UTC (permalink / raw)
To: u-boot
On Wed, 2015-09-02 at 09:51 -0500, York Sun wrote:
>
> On 09/02/2015 09:42 AM, Joakim Tjernlund wrote:
> > On Wed, 2015-09-02 at 09:34 -0500, York Sun wrote:
> > >
> > > On 09/02/2015 09:31 AM, Joakim Tjernlund wrote:
> > > > On Wed, 2015-09-02 at 08:35 -0500, York Sun wrote:
> > > > >
> > > > > On 09/02/2015 06:41 AM, Joakim Tjernlund wrote:
> > > > > > T1040 RM specifies CLK_ADJUST as 5 bits starting at bit pos 9
> > > > > > in DDR_DDR_SDRAM_CLK_CNTL, update code to match.
> > > > > >
> > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> > > > > > ---
> > > > > > drivers/ddr/fsl/ctrl_regs.c | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
> > > > > > index 3919257..57077e1 100644
> > > > > > --- a/drivers/ddr/fsl/ctrl_regs.c
> > > > > > +++ b/drivers/ddr/fsl/ctrl_regs.c
> > > > > > @@ -1756,7 +1756,7 @@ static void set_ddr_sdram_clk_cntl(fsl_ddr_cfg_regs_t *ddr,
> > > > > > clk_adjust = popts->clk_adjust;
> > > > > > ddr->ddr_sdram_clk_cntl = (0
> > > > > > | ((ss_en & 0x1) << 31)
> > > > > > - | ((clk_adjust & 0xF) << 23)
> > > > > > + | ((clk_adjust & 0x1F) << 22)
> > > > > > );
> > > > > > debug("FSLDDR: clk_cntl = 0x%08x\n", ddr->ddr_sdram_clk_cntl);
> > > > > > }
> > > > > >
> > > > >
> > > > > NACK. This is logically correct but it needs to consider all DDR controllers.
> > > > > Older controllers don't have the same bits for this field. So far we don't have
> > > > > to use this this fine granularity on clock adjustment.
> > > >
> > > > I have no idea what older controllers do with these bits so I leave it to you to find out.
> > > >
> > > > Anyhow, current code is broken as the programmed value is doubled(we see this on our board
> > > > which cannot boot linux without this fix)
> > >
> > > Jocke,
> > >
> > > Can you refer to Freescale board? You may be using correct clk_adjust value on
> > Custom board based on T1042RDB
> >
> > > your board. But when you use this driver, you may have to adjust the value by
> > > shifting 1 bit. You will lose some values but we haven't found it difficult.
> > > This value is important but can also tolerate a wide range.
> >
> > Sure, but how should anyone know that beforehand? The clk_adjust was calculated using
> > Freescales CW tool and added to table with the write lvl data.
> >
> > Are the values for the T104x boards already compensated like you suggest?
> > Then they will (possibly) break when this bug is fixed.
>
> Jocke,
>
> I understand the frustration for having tools disconnected.
> The CW tool gives:file you a suggestion but you have to put it in u-boot
> differently. Freescale boards including T104x have correct values, for example
> board/freescale/t104xrdb/ddr.h.
How would anyone know that these values are real clk_adjust/2 ?
>
> I don't consider it a bug for backward compatibility.
Of course this is a bug that many will keep falling into! Costing lots
of time to find too.
Just fix it and adjust old boards by doubling their values instead as a minimum fix.
I will adjust our value with /2 know that I know this bug and that be done.
Jocke
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-09-02 15:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-02 11:41 [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation Joakim Tjernlund
2015-09-02 13:35 ` York Sun
2015-09-02 14:31 ` Joakim Tjernlund
2015-09-02 14:34 ` York Sun
2015-09-02 14:42 ` Joakim Tjernlund
2015-09-02 14:51 ` York Sun
2015-09-02 15:06 ` Joakim Tjernlund
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox