From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joakim Tjernlund Date: Wed, 2 Sep 2015 15:06:30 +0000 Subject: [U-Boot] [PATCH] Fix FSL DDR clock adjust calculation. In-Reply-To: <55E70CF4.80000@freescale.com> References: <1441194061-28527-1-git-send-email-joakim.tjernlund@transmode.se> <55E6FB19.6090900@freescale.com> <1441204296.3349.211.camel@transmode.se> <55E70906.7010608@freescale.com> <1441204943.3349.217.camel@transmode.se> <55E70CF4.80000@freescale.com> Message-ID: <1441206390.3349.233.camel@transmode.se> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > > > > > --- > > > > > > 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