Re: [PATCH 2/2] power/reset: at91-poweroff: timely shitdown LPDDR memories

From: Richard Genoud
Date: Thu Oct 13 2016 - 08:42:47 EST


2016-10-13 14:27 GMT+02:00 Jean-Jacques Hiblot <jjhiblot@xxxxxxxxxxxxxxx>:
> 2016-10-13 13:03 GMT+02:00 Alexandre Belloni
> <alexandre.belloni@xxxxxxxxxxxxxxxxxx>:
>> On 12/10/2016 at 14:48:27 +0200, Jean-Jacques Hiblot wrote :
>>> > +static void at91_lpddr_poweroff(void)
>>> > +{
>>> > + asm volatile(
>>> > + /* Align to cache lines */
>>> > + ".balign 32\n\t"
>>> > +
>>> > + " ldr r6, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
>>> At first sight, it looks useless. I assume it's used to preload the
>>> TLB before the LPDDR is turned off.
>>> A comment to explain why this line is useful would prevent its removal.
>>
>> Yes, this is the case. I can add a comment.
>>
>> Anyway, I would prefer the whole thing to run from SRAM, as a PIE
>> instead of relying on the cache.
>
> Instead of copying into the SRAM, you can make the cache reliable by
> preloading it, much like the TLB.
> LDI is probably not available for most of atmel's SOC, so the only way
> I can think of, is to execute code from the targeted area. here is an
> example:
> + /*
> + * Jump to the end of the sequence to preload instruction cache
> + * It only works because the sequence is short enough not to
> + * sit accross more than 2 cache lines
> + */
> + " b end_of_sequence\n\t"
> + "start_of_sequence:\n\t"
> +
> /* Power down SDRAM0 */
> " str %1, [%0, #"
> __stringify(AT91_DDRSDRC_LPR) "]\n\t"
> /* Shutdown CPU */
> " str %3, [%2, #" __stringify(AT91_SHDW_CR) "]\n\t"
>
> " b .\n\t"
> +
> + /*
> + * we're now 100% sure that the code to shutdown the LPDDR and
> + * the CPU is in cache, go back to do the actual job
> + */
> + "end_of_sequence:\n\t"
> + " b start_of_sequence\n\t"
> :

My 2c: I think you may want to change your subject :)

Richard.