Re: [PATCH v2 3/3] ARM: at91: pm: preload base address of controllers in tlb

[Nicolas Ferre <nicolas.ferre@microchip.com>]

On 30/09/2021 at 17:42, Claudiu Beznea wrote:
> In suspend/resume procedure for AT91 architecture different controllers
> (PMC, SHDWC, RAM, RAM PHY, SFRBU) are accessed to do the proper settings
> for power saving. Commit f0bbf17958e8 ("ARM: at91: pm: add self-refresh
> support for sama7g5") introduced the access to RAMC PHY controller for
> SAMA7G5. The access to this controller is done after RAMC ports are
> closed, thus any TLB walk necessary for RAMC PHY virtual address will
> fail. In the development branch this was not encountered. However, on
> current kernel the issue is reproducible.
> 
> To solve the issue the previous mechanism of pre-loading the TLB with
> the RAMC PHY virtual address has been used. However, only the addition
> of this new pre-load breaks the functionality for ARMv5 based
> devices (SAM9X60). This behavior has been encountered previously
> while debugging this code and using the same mechanism for pre-loading
> address for different controllers (e.g. pin controller, the assumption
> being that other requested translations are replaced from TLB).
> 
> To solve this new issue the TLB flush + the extension of pre-loading
> the rest of controllers to TLB (e.g. PMC, RAMC) has been added. The
> rest of the controllers should have been pre-loaded previously, anyway.
> 
> Fixes: f0bbf17958e8 ("ARM: at91: pm: add self-refresh support for sama7g5")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
> 
> Hi Nicolas,
> 
> Please link this patch with the previous one in this series ("ARM: at91:
> pm: group constants and addresses loading") with a Depends-on tag as this
> will fail to apply if backported to older kernel versions.

Done.

For whole series:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Whole series added to at91-fixes. To be sent as a second fixes tag to 
arm-soc.

Best regards,
   Nicolas


>   arch/arm/mach-at91/pm_suspend.S | 25 ++++++++++++++++++++++++-
>   1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
> index 34f251fdb743..fdb4f63ecde4 100644
> --- a/arch/arm/mach-at91/pm_suspend.S
> +++ b/arch/arm/mach-at91/pm_suspend.S
> @@ -1014,6 +1014,10 @@ ENTRY(at91_pm_suspend_in_sram)
>   	mov	tmp1, #0
>   	mcr	p15, 0, tmp1, c7, c10, 4
>   
> +	/* Flush tlb. */
> +	mov	r4, #0
> +	mcr	p15, 0, r4, c8, c7, 0
> +
>   	ldr	tmp1, [r0, #PM_DATA_PMC_MCKR_OFFSET]
>   	str	tmp1, .mckr_offset
>   	ldr	tmp1, [r0, #PM_DATA_PMC_VERSION]
> @@ -1023,23 +1027,42 @@ ENTRY(at91_pm_suspend_in_sram)
>   	ldr	tmp1, [r0, #PM_DATA_MODE]
>   	str	tmp1, .pm_mode
>   
> +	/*
> +	 * ldrne below are here to preload their address in the TLB as access
> +	 * to RAM may be limited while in self-refresh.
> +	 */
>   	ldr	tmp1, [r0, #PM_DATA_PMC]
>   	str	tmp1, .pmc_base
> +	cmp	tmp1, #0
> +	ldrne	tmp2, [tmp1, #0]
> +
>   	ldr	tmp1, [r0, #PM_DATA_RAMC0]
>   	str	tmp1, .sramc_base
> +	cmp	tmp1, #0
> +	ldrne	tmp2, [tmp1, #0]
> +
>   	ldr	tmp1, [r0, #PM_DATA_RAMC1]
>   	str	tmp1, .sramc1_base
> +	cmp	tmp1, #0
> +	ldrne	tmp2, [tmp1, #0]
> +
> +#ifndef CONFIG_SOC_SAM_V4_V5
> +	/* ldrne below are here to preload their address in the TLB */
>   	ldr	tmp1, [r0, #PM_DATA_RAMC_PHY]
>   	str	tmp1, .sramc_phy_base
> -	/* Both ldrne below are here to preload their address in the TLB */
> +	cmp	tmp1, #0
> +	ldrne	tmp2, [tmp1, #0]
> +
>   	ldr	tmp1, [r0, #PM_DATA_SHDWC]
>   	str	tmp1, .shdwc
>   	cmp	tmp1, #0
>   	ldrne	tmp2, [tmp1, #0]
> +
>   	ldr	tmp1, [r0, #PM_DATA_SFRBU]
>   	str	tmp1, .sfrbu
>   	cmp	tmp1, #0
>   	ldrne	tmp2, [tmp1, #0x10]
> +#endif
>   
>   	/* Active the self-refresh mode */
>   	at91_sramc_self_refresh_ena
> 


-- 
Nicolas Ferre