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

[Richard Genoud]

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.