Sergei Shtylyov wrote: > Hello. Hi!
> On 1/27/2015 8:53 AM, Wenyou Yang wrote: > > > From: Peter Rosin <p...@axentia.se> > > > The DDRSDR controller 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, but that is > > untested. > > > Signed-off-by: Peter Rosin <p...@axentia.se> > > Acked-by: Nicolas Ferre <nicolas.fe...@atmel.com> > > --- > > 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 e2bfaf5..1155217 100644 > > --- a/arch/arm/mach-at91/pm_slowclock.S > > +++ b/arch/arm/mach-at91/pm_slowclock.S > [...] > > @@ -108,14 +118,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 > > Didn't you forget ~? Either that, or ~ above is not needed, I think. The code is correct, the first bic with ~ clears bits not in the relevant field in order to compare if LPDDR mode is active. The second bic(eq) w/o ~ clears the field, to make way for the bits in the below orreq when actually changing the register content into DDR2 mode. > > + 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 > > Didn't you forget ~? And isn't it 3-operand instruction (as seen in the > above > code)? The logic for the LPR register is from the old code, the "only" thing I did to it was changing the instruction sequence to not have the ???ne form, i.e. ldrne, strne, bicne, orrne became ldr, str, bic, orr with a jump around it instead. So, the original code also had a two argument bic(ne), which indeed is strange, and I don't know why there is no warning from the assembler. Since there is no warning, my guess is that the assembler somehow mends it? Or does the patch actually break the second controller? It would be a surprise it the assembler handles 2-operand bicne differently from a 2-operand bic, no? > > + orr tmp2, #AT91_DDRSDRC_LPCB_SELF_REFRESH > > Only 2 operands? Same argument as above. I didn't touch it (sort of...) Should I update the patch and fix this collateral 2-operand problem as well? To me, it feels like a separate patch, no? > [...] > > WBR, Sergei Cheers, Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
