public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Joakim Tjernlund <joakim.tjernlund@transmode.se>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation.
Date: Wed, 2 Sep 2015 15:06:30 +0000	[thread overview]
Message-ID: <1441206390.3349.233.camel@transmode.se> (raw)
In-Reply-To: <55E70CF4.80000@freescale.com>

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

      reply	other threads:[~2015-09-02 15:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1441206390.3349.233.camel@transmode.se \
    --to=joakim.tjernlund@transmode.se \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox