RE: [PATCH RESEND] pm: at91: Workaround DDRSDRC self-refresh bug with LPDDR1 memories

[Yang, Wenyou]

> -----Original Message-----
> From: Ferre, Nicolas
> Sent: Thursday, January 15, 2015 12:48 AM
> To: Peter Rosin; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Yang, Wenyou
> Cc: Peter Rosin; Andrew Victor; Jean-Christophe Plagniol-Villard; Russell King;
> linux-kernel@xxxxxxxxxxxxxxx; Alexandre Belloni; Boris BREZILLON
> Subject: Re: [PATCH RESEND] pm: at91: Workaround DDRSDRC self-refresh bug
> with LPDDR1 memories
> 
> Le 14/01/2015 14:20, Peter Rosin a écrit :
> > From: Peter Rosin <peda@xxxxxxxxxx>
> >
> > The DDRSDR controller (on the ATSAMA5D31) fails miserably to put
> > LPDDR1 memories in self-refresh. Force the controller to think it has
> > DDR2 memories during the self-refresh period, as the DDR2 self-refresh
> > spec is equivalent to LPDDR1, and is correctly implemented in the controller.
> >
> > Assume that the second controller has the same fault, and that other
> > CPUs in the family has the same problem, but that is untested.
> >
> > Signed-off-by: Peter Rosin <peda@xxxxxxxxxx>
> 
> I've just verified your code and the scope of this issue and your implementation
> makes perfect sense.
> 
> Acked-by: Nicolas Ferre <nicolas.ferre@xxxxxxxxx>
> 
> Peter,
> Thanks for your patch. You will probably see it appearing in 3.20 or 3.21.
> 
> Wenyou,
> Can you please integrate the patch from Peter in your current rework of the PM
> routines (keeping his authorship of course)?
Of course, I will integrate.

> Please tell me if I can help with this.
> 
> Best regards,
> 
> 
> > ---
> >  arch/arm/mach-at91/pm_slowclock.S  |   43
> +++++++++++++++++++++++++++++++-----
> >  include/soc/at91/at91sam9_ddrsdr.h |    2 +-
> >  2 files changed, 39 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm/mach-at91/pm_slowclock.S
> > b/arch/arm/mach-at91/pm_slowclock.S
> > index 20018779bae7..63a9e0b0a17d 100644
> > --- a/arch/arm/mach-at91/pm_slowclock.S
> > +++ b/arch/arm/mach-at91/pm_slowclock.S
> > @@ -143,6 +143,16 @@ ddr_sr_enable:
> >  	cmp	memctrl, #AT91_MEMCTRL_DDRSDR
> >  	bne	sdr_sr_enable
> >
> > +	/* LPDDR1 --> force DDR2 mode during self-refresh */
> > +	ldr	tmp1, [sdramc, #AT91_DDRSDRC_MDR]
> > +	str	tmp1, .saved_sam9_mdr
> > +	bic	tmp1, tmp1, #~AT91_DDRSDRC_MD
> > +	cmp	tmp1, #AT91_DDRSDRC_MD_LOW_POWER_DDR
> > +	ldreq	tmp1, [sdramc, #AT91_DDRSDRC_MDR]
> > +	biceq	tmp1, tmp1, #AT91_DDRSDRC_MD
> > +	orreq	tmp1, tmp1, #AT91_DDRSDRC_MD_DDR2
> > +	streq	tmp1, [sdramc, #AT91_DDRSDRC_MDR]
> > +
> >  	/* prepare for DDRAM self-refresh mode */
> >  	ldr	tmp1, [sdramc, #AT91_DDRSDRC_LPR]
> >  	str	tmp1, .saved_sam9_lpr
> > @@ -151,14 +161,26 @@ ddr_sr_enable:
> >
> >  	/* figure out if we use the second ram controller */
> >  	cmp	ramc1, #0
> > -	ldrne	tmp2, [ramc1, #AT91_DDRSDRC_LPR]
> > -	strne	tmp2, .saved_sam9_lpr1
> > -	bicne	tmp2, #AT91_DDRSDRC_LPCB
> > -	orrne	tmp2, #AT91_DDRSDRC_LPCB_SELF_REFRESH
> > +	beq	ddr_no_2nd_ctrl
> > +
> > +	ldr	tmp2, [ramc1, #AT91_DDRSDRC_MDR]
> > +	str	tmp2, .saved_sam9_mdr1
> > +	bic	tmp2, tmp2, #~AT91_DDRSDRC_MD
> > +	cmp	tmp2, #AT91_DDRSDRC_MD_LOW_POWER_DDR
> > +	ldreq	tmp2, [ramc1, #AT91_DDRSDRC_MDR]
> > +	biceq	tmp2, tmp2, #AT91_DDRSDRC_MD
> > +	orreq	tmp2, tmp2, #AT91_DDRSDRC_MD_DDR2
> > +	streq	tmp2, [ramc1, #AT91_DDRSDRC_MDR]
> > +
> > +	ldr	tmp2, [ramc1, #AT91_DDRSDRC_LPR]
> > +	str	tmp2, .saved_sam9_lpr1
> > +	bic	tmp2, #AT91_DDRSDRC_LPCB
> > +	orr	tmp2, #AT91_DDRSDRC_LPCB_SELF_REFRESH
> >
> >  	/* Enable DDRAM self-refresh mode */
> > +	str	tmp2, [ramc1, #AT91_DDRSDRC_LPR]
> > +ddr_no_2nd_ctrl:
> >  	str	tmp1, [sdramc, #AT91_DDRSDRC_LPR]
> > -	strne	tmp2, [ramc1, #AT91_DDRSDRC_LPR]
> >
> >  	b	sdr_sr_done
> >
> > @@ -289,12 +311,17 @@ sdr_sr_done:
> >  	 */
> >  	cmp	memctrl, #AT91_MEMCTRL_DDRSDR
> >  	bne	sdr_en_restore
> > +	/* Restore MDR in case of LPDDR1 */
> > +	ldr	tmp1, .saved_sam9_mdr
> > +	str	tmp1, [sdramc, #AT91_DDRSDRC_MDR]
> >  	/* Restore LPR on AT91 with DDRAM */
> >  	ldr	tmp1, .saved_sam9_lpr
> >  	str	tmp1, [sdramc, #AT91_DDRSDRC_LPR]
> >
> >  	/* if we use the second ram controller */
> >  	cmp	ramc1, #0
> > +	ldrne	tmp2, .saved_sam9_mdr1
> > +	strne	tmp2, [ramc1, #AT91_DDRSDRC_MDR]
> >  	ldrne	tmp2, .saved_sam9_lpr1
> >  	strne	tmp2, [ramc1, #AT91_DDRSDRC_LPR]
> >
> > @@ -328,5 +355,11 @@ ram_restored:
> >  .saved_sam9_lpr1:
> >  	.word 0
> >
> > +.saved_sam9_mdr:
> > +	.word 0
> > +
> > +.saved_sam9_mdr1:
> > +	.word 0
> > +
> >  ENTRY(at91_slow_clock_sz)
> >  	.word .-at91_slow_clock
> > diff --git a/include/soc/at91/at91sam9_ddrsdr.h
> > b/include/soc/at91/at91sam9_ddrsdr.h
> > index 0210797abf2e..cd2c18787833 100644
> > --- a/include/soc/at91/at91sam9_ddrsdr.h
> > +++ b/include/soc/at91/at91sam9_ddrsdr.h
> > @@ -92,7 +92,7 @@
> >  #define		AT91_DDRSDRC_UPD_MR	(3 << 20)	 /* Update
> load mode register and extended mode register */
> >
> >  #define AT91_DDRSDRC_MDR	0x20	/* Memory Device Register */
> > -#define		AT91_DDRSDRC_MD		(3 << 0)		/* Memory
> Device Type */
> > +#define		AT91_DDRSDRC_MD		(7 << 0)		/* Memory
> Device Type */
> >  #define			AT91_DDRSDRC_MD_SDR		0
> >  #define			AT91_DDRSDRC_MD_LOW_POWER_SDR	1
> >  #define			AT91_DDRSDRC_MD_LOW_POWER_DDR	3
> >
> 
> 
> --
> Nicolas Ferre

Best Regards,
Wenyou Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/