Hi, Ryan,
On 10.02.2025 23:13, Ryan.Wanner@microchip.com wrote:
> From: Ryan Wanner <Ryan.Wanner@microchip.com>
>
> Add config check that enables Backup mode for SAMA7D65 SoC.
>
> Add SHDWC_SR read to clear the status bits once finished exiting low
> power modes.
Can you please also explain why? From [1]:
"The text should be written in such detail so that when read weeks, months
or even years later, it can give the reader the needed details to grasp the
reasoning for **why** the patch was created."
[1] https://www.kernel.org/doc/html/v6.13/process/submitting-patches.html
> This is only for SAMA7D65 SoCs.
>
> Signed-off-by: Ryan Wanner <Ryan.Wanner@microchip.com>
> ---
> arch/arm/mach-at91/pm.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 1eec68e92f8d8..55cab31ce1ecb 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -707,6 +707,9 @@ static int at91_pm_enter(suspend_state_t state)
> static void at91_pm_end(void)
> {
> at91_pm_config_ws(soc_pm.data.mode, false);
> +
> + if (IS_ENABLED(CONFIG_SOC_SAMA7D65))
> + readl(soc_pm.data.shdwc + 0x08);
Can you please add a comment near explaining what 0x08 offset means (search
for "SHDWC.MR" in this file for an example)?
Is this cleanup needed only for backup mode or for all of them. If only for
backup you can move it in at91_pm_suspend() after fncpy().
Thank you,
Claudiu
> }
>
>
> @@ -1065,7 +1068,8 @@ static int __init at91_pm_backup_init(void)
> int ret = -ENODEV, located = 0;
>
> if (!IS_ENABLED(CONFIG_SOC_SAMA5D2) &&
> - !IS_ENABLED(CONFIG_SOC_SAMA7G5))
> + !IS_ENABLED(CONFIG_SOC_SAMA7G5) &&
> + !IS_ENABLED(CONFIG_SOC_SAMA7D65))
> return -EPERM;
>
> if (!at91_is_pm_mode_active(AT91_PM_BACKUP))