* [PATCH 0/3] AT91 PM improvements
@ 2020-08-03 7:25 ` Claudiu Beznea
0 siblings, 0 replies; 14+ messages in thread
From: Claudiu Beznea @ 2020-08-03 7:25 UTC (permalink / raw)
To: nicolas.ferre, alexandre.belloni, ludovic.desroches
Cc: linux-arm-kernel, linux-kernel, Claudiu Beznea
Hi,
This series adds ULP0 fast mode intended to reduce the suspend/resume
time in the detriment of power consumption (patch 1/3).
Along with this patch 2/3 adds code to avoid requesting a PM mode
not available on platforms not supporting it.
Patch 3/3 decrements a device_node refcount after its usage.
Thank you,
Claudiu Beznea
Claudiu Beznea (3):
ARM: at91: pm: add support for ulp0 fast mode
ARM: at91: pm: add per soc validation of pm modes
ARM: at91: pm: put node after its usage
arch/arm/mach-at91/at91rm9200.c | 10 +++++-
arch/arm/mach-at91/at91sam9.c | 9 ++++-
arch/arm/mach-at91/generic.h | 20 +++++------
arch/arm/mach-at91/pm.c | 79 +++++++++++++++++++++++++++++++++++------
arch/arm/mach-at91/pm.h | 5 +--
arch/arm/mach-at91/pm_suspend.S | 41 ++++++++++++++++++---
arch/arm/mach-at91/sam9x60.c | 11 +++++-
arch/arm/mach-at91/sama5.c | 21 +++++++++--
8 files changed, 165 insertions(+), 31 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/3] AT91 PM improvements
@ 2020-08-03 7:25 ` Claudiu Beznea
0 siblings, 0 replies; 14+ messages in thread
From: Claudiu Beznea @ 2020-08-03 7:25 UTC (permalink / raw)
To: nicolas.ferre, alexandre.belloni, ludovic.desroches
Cc: linux-kernel, linux-arm-kernel, Claudiu Beznea
Hi,
This series adds ULP0 fast mode intended to reduce the suspend/resume
time in the detriment of power consumption (patch 1/3).
Along with this patch 2/3 adds code to avoid requesting a PM mode
not available on platforms not supporting it.
Patch 3/3 decrements a device_node refcount after its usage.
Thank you,
Claudiu Beznea
Claudiu Beznea (3):
ARM: at91: pm: add support for ulp0 fast mode
ARM: at91: pm: add per soc validation of pm modes
ARM: at91: pm: put node after its usage
arch/arm/mach-at91/at91rm9200.c | 10 +++++-
arch/arm/mach-at91/at91sam9.c | 9 ++++-
arch/arm/mach-at91/generic.h | 20 +++++------
arch/arm/mach-at91/pm.c | 79 +++++++++++++++++++++++++++++++++++------
arch/arm/mach-at91/pm.h | 5 +--
arch/arm/mach-at91/pm_suspend.S | 41 ++++++++++++++++++---
arch/arm/mach-at91/sam9x60.c | 11 +++++-
arch/arm/mach-at91/sama5.c | 21 +++++++++--
8 files changed, 165 insertions(+), 31 deletions(-)
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] ARM: at91: pm: add support for ulp0 fast mode
2020-08-03 7:25 ` Claudiu Beznea
@ 2020-08-03 7:25 ` Claudiu Beznea
-1 siblings, 0 replies; 14+ messages in thread
From: Claudiu Beznea @ 2020-08-03 7:25 UTC (permalink / raw)
To: nicolas.ferre, alexandre.belloni, ludovic.desroches
Cc: linux-arm-kernel, linux-kernel, Claudiu Beznea
ULP0 fast improves suspend/resume time with few milliseconds the drawback
being the power consumption. The mean values measured for suspend/resume
time are as follows (measured on SAMA5D2 Xplained board), ULP0 compared
with ULP0 fast:
- ulp0 fast: suspend time: 169 ms, resume time: 216 ms
- ulp0 : suspend time: 197 ms, resume time: 258 ms
Current consumption while suspended (measured on SAMA5D2 Xplained board):
- ulp0 fast: 730uA
- ulp0 : 270uA
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
arch/arm/mach-at91/pm.c | 9 +++++----
arch/arm/mach-at91/pm.h | 5 +++--
arch/arm/mach-at91/pm_suspend.S | 41 +++++++++++++++++++++++++++++++++++++----
3 files changed, 45 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 074bde64064e..04fdcbd57100 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -51,10 +51,11 @@ static struct at91_soc_pm soc_pm = {
};
static const match_table_t pm_modes __initconst = {
- { AT91_PM_STANDBY, "standby" },
- { AT91_PM_ULP0, "ulp0" },
- { AT91_PM_ULP1, "ulp1" },
- { AT91_PM_BACKUP, "backup" },
+ { AT91_PM_STANDBY, "standby" },
+ { AT91_PM_ULP0, "ulp0" },
+ { AT91_PM_ULP0_FAST, "ulp0-fast" },
+ { AT91_PM_ULP1, "ulp1" },
+ { AT91_PM_BACKUP, "backup" },
{ -1, NULL },
};
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index 218e8d1a30fb..bfb260be371e 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -19,8 +19,9 @@
#define AT91_PM_STANDBY 0x00
#define AT91_PM_ULP0 0x01
-#define AT91_PM_ULP1 0x02
-#define AT91_PM_BACKUP 0x03
+#define AT91_PM_ULP0_FAST 0x02
+#define AT91_PM_ULP1 0x03
+#define AT91_PM_BACKUP 0x04
#ifndef __ASSEMBLY__
struct at91_pm_data {
diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
index be9764e8d3fa..0184de05c1be 100644
--- a/arch/arm/mach-at91/pm_suspend.S
+++ b/arch/arm/mach-at91/pm_suspend.S
@@ -164,7 +164,22 @@ ENDPROC(at91_backup_mode)
.macro at91_pm_ulp0_mode
ldr pmc, .pmc_base
+ ldr tmp2, .pm_mode
+ ldr tmp3, .mckr_offset
+
+ /* Check if ULP0 fast variant has been requested. */
+ cmp tmp2, #AT91_PM_ULP0_FAST
+ bne 0f
+
+ /* Set highest prescaler for power saving */
+ ldr tmp1, [pmc, tmp3]
+ bic tmp1, tmp1, #AT91_PMC_PRES
+ orr tmp1, tmp1, #AT91_PMC_PRES_64
+ str tmp1, [pmc, tmp3]
+ wait_mckrdy
+ b 1f
+0:
/* Turn off the crystal oscillator */
ldr tmp1, [pmc, #AT91_CKGR_MOR]
bic tmp1, tmp1, #AT91_PMC_MOSCEN
@@ -192,7 +207,18 @@ ENDPROC(at91_backup_mode)
/* Wait for interrupt */
1: at91_cpu_idle
- /* Restore RC oscillator state */
+ /* Check if ULP0 fast variant has been requested. */
+ cmp tmp2, #AT91_PM_ULP0_FAST
+ bne 5f
+
+ /* Set lowest prescaler for fast resume. */
+ ldr tmp1, [pmc, tmp3]
+ bic tmp1, tmp1, #AT91_PMC_PRES
+ str tmp1, [pmc, tmp3]
+ wait_mckrdy
+ b 6f
+
+5: /* Restore RC oscillator state */
ldr tmp1, .saved_osc_status
tst tmp1, #AT91_PMC_MOSCRCS
beq 4f
@@ -216,6 +242,7 @@ ENDPROC(at91_backup_mode)
str tmp1, [pmc, #AT91_CKGR_MOR]
wait_moscrdy
+6:
.endm
/**
@@ -473,23 +500,29 @@ ENDPROC(at91_backup_mode)
ENTRY(at91_ulp_mode)
ldr pmc, .pmc_base
ldr tmp2, .mckr_offset
+ ldr tmp3, .pm_mode
/* Save Master clock setting */
ldr tmp1, [pmc, tmp2]
str tmp1, .saved_mckr
/*
- * Set the Master clock source to slow clock
+ * Set master clock source to:
+ * - MAINCK if using ULP0 fast variant
+ * - slow clock, otherwise
*/
bic tmp1, tmp1, #AT91_PMC_CSS
+ cmp tmp3, #AT91_PM_ULP0_FAST
+ bne save_mck
+ orr tmp1, tmp1, #AT91_PMC_CSS_MAIN
+save_mck:
str tmp1, [pmc, tmp2]
wait_mckrdy
at91_plla_disable
- ldr r0, .pm_mode
- cmp r0, #AT91_PM_ULP1
+ cmp tmp3, #AT91_PM_ULP1
beq ulp1_mode
at91_pm_ulp0_mode
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/3] ARM: at91: pm: add support for ulp0 fast mode
@ 2020-08-03 7:25 ` Claudiu Beznea
0 siblings, 0 replies; 14+ messages in thread
From: Claudiu Beznea @ 2020-08-03 7:25 UTC (permalink / raw)
To: nicolas.ferre, alexandre.belloni, ludovic.desroches
Cc: linux-kernel, linux-arm-kernel, Claudiu Beznea
ULP0 fast improves suspend/resume time with few milliseconds the drawback
being the power consumption. The mean values measured for suspend/resume
time are as follows (measured on SAMA5D2 Xplained board), ULP0 compared
with ULP0 fast:
- ulp0 fast: suspend time: 169 ms, resume time: 216 ms
- ulp0 : suspend time: 197 ms, resume time: 258 ms
Current consumption while suspended (measured on SAMA5D2 Xplained board):
- ulp0 fast: 730uA
- ulp0 : 270uA
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
arch/arm/mach-at91/pm.c | 9 +++++----
arch/arm/mach-at91/pm.h | 5 +++--
arch/arm/mach-at91/pm_suspend.S | 41 +++++++++++++++++++++++++++++++++++++----
3 files changed, 45 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 074bde64064e..04fdcbd57100 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -51,10 +51,11 @@ static struct at91_soc_pm soc_pm = {
};
static const match_table_t pm_modes __initconst = {
- { AT91_PM_STANDBY, "standby" },
- { AT91_PM_ULP0, "ulp0" },
- { AT91_PM_ULP1, "ulp1" },
- { AT91_PM_BACKUP, "backup" },
+ { AT91_PM_STANDBY, "standby" },
+ { AT91_PM_ULP0, "ulp0" },
+ { AT91_PM_ULP0_FAST, "ulp0-fast" },
+ { AT91_PM_ULP1, "ulp1" },
+ { AT91_PM_BACKUP, "backup" },
{ -1, NULL },
};
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index 218e8d1a30fb..bfb260be371e 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -19,8 +19,9 @@
#define AT91_PM_STANDBY 0x00
#define AT91_PM_ULP0 0x01
-#define AT91_PM_ULP1 0x02
-#define AT91_PM_BACKUP 0x03
+#define AT91_PM_ULP0_FAST 0x02
+#define AT91_PM_ULP1 0x03
+#define AT91_PM_BACKUP 0x04
#ifndef __ASSEMBLY__
struct at91_pm_data {
diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
index be9764e8d3fa..0184de05c1be 100644
--- a/arch/arm/mach-at91/pm_suspend.S
+++ b/arch/arm/mach-at91/pm_suspend.S
@@ -164,7 +164,22 @@ ENDPROC(at91_backup_mode)
.macro at91_pm_ulp0_mode
ldr pmc, .pmc_base
+ ldr tmp2, .pm_mode
+ ldr tmp3, .mckr_offset
+
+ /* Check if ULP0 fast variant has been requested. */
+ cmp tmp2, #AT91_PM_ULP0_FAST
+ bne 0f
+
+ /* Set highest prescaler for power saving */
+ ldr tmp1, [pmc, tmp3]
+ bic tmp1, tmp1, #AT91_PMC_PRES
+ orr tmp1, tmp1, #AT91_PMC_PRES_64
+ str tmp1, [pmc, tmp3]
+ wait_mckrdy
+ b 1f
+0:
/* Turn off the crystal oscillator */
ldr tmp1, [pmc, #AT91_CKGR_MOR]
bic tmp1, tmp1, #AT91_PMC_MOSCEN
@@ -192,7 +207,18 @@ ENDPROC(at91_backup_mode)
/* Wait for interrupt */
1: at91_cpu_idle
- /* Restore RC oscillator state */
+ /* Check if ULP0 fast variant has been requested. */
+ cmp tmp2, #AT91_PM_ULP0_FAST
+ bne 5f
+
+ /* Set lowest prescaler for fast resume. */
+ ldr tmp1, [pmc, tmp3]
+ bic tmp1, tmp1, #AT91_PMC_PRES
+ str tmp1, [pmc, tmp3]
+ wait_mckrdy
+ b 6f
+
+5: /* Restore RC oscillator state */
ldr tmp1, .saved_osc_status
tst tmp1, #AT91_PMC_MOSCRCS
beq 4f
@@ -216,6 +242,7 @@ ENDPROC(at91_backup_mode)
str tmp1, [pmc, #AT91_CKGR_MOR]
wait_moscrdy
+6:
.endm
/**
@@ -473,23 +500,29 @@ ENDPROC(at91_backup_mode)
ENTRY(at91_ulp_mode)
ldr pmc, .pmc_base
ldr tmp2, .mckr_offset
+ ldr tmp3, .pm_mode
/* Save Master clock setting */
ldr tmp1, [pmc, tmp2]
str tmp1, .saved_mckr
/*
- * Set the Master clock source to slow clock
+ * Set master clock source to:
+ * - MAINCK if using ULP0 fast variant
+ * - slow clock, otherwise
*/
bic tmp1, tmp1, #AT91_PMC_CSS
+ cmp tmp3, #AT91_PM_ULP0_FAST
+ bne save_mck
+ orr tmp1, tmp1, #AT91_PMC_CSS_MAIN
+save_mck:
str tmp1, [pmc, tmp2]
wait_mckrdy
at91_plla_disable
- ldr r0, .pm_mode
- cmp r0, #AT91_PM_ULP1
+ cmp tmp3, #AT91_PM_ULP1
beq ulp1_mode
at91_pm_ulp0_mode
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] ARM: at91: pm: add per soc validation of pm modes
2020-08-03 7:25 ` Claudiu Beznea
@ 2020-08-03 7:25 ` Claudiu Beznea
-1 siblings, 0 replies; 14+ messages in thread
From: Claudiu Beznea @ 2020-08-03 7:25 UTC (permalink / raw)
To: nicolas.ferre, alexandre.belloni, ludovic.desroches
Cc: linux-arm-kernel, linux-kernel, Claudiu Beznea
Not all SoCs supports all the PM mode. User may end up settings,
e.g. backup mode, on a non SAMA5D2 device, but the mode to not be valid.
If backup mode is used on a devices not supporting it there will be no
way of resuming other than rebooting.
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
arch/arm/mach-at91/at91rm9200.c | 10 +++++-
arch/arm/mach-at91/at91sam9.c | 9 +++++-
arch/arm/mach-at91/generic.h | 20 ++++++------
arch/arm/mach-at91/pm.c | 69 +++++++++++++++++++++++++++++++++++++----
arch/arm/mach-at91/sam9x60.c | 11 ++++++-
arch/arm/mach-at91/sama5.c | 21 +++++++++++--
6 files changed, 119 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
index 4f8186211619..7318d8e16797 100644
--- a/arch/arm/mach-at91/at91rm9200.c
+++ b/arch/arm/mach-at91/at91rm9200.c
@@ -13,12 +13,20 @@
#include <asm/mach/arch.h>
#include "generic.h"
+#include "pm.h"
+
+/* AT91RM9200 valid PM modes. */
+static const int at91rm9200_pm_modes[] = {
+ AT91_PM_STANDBY,
+ AT91_PM_ULP0,
+};
static void __init at91rm9200_dt_device_init(void)
{
of_platform_default_populate(NULL, NULL, NULL);
- at91rm9200_pm_init();
+ at91rm9200_pm_init(at91rm9200_pm_modes,
+ ARRAY_SIZE(at91rm9200_pm_modes));
}
static const char *const at91rm9200_dt_board_compat[] __initconst = {
diff --git a/arch/arm/mach-at91/at91sam9.c b/arch/arm/mach-at91/at91sam9.c
index 7e572189a5eb..683f2c35a116 100644
--- a/arch/arm/mach-at91/at91sam9.c
+++ b/arch/arm/mach-at91/at91sam9.c
@@ -13,12 +13,19 @@
#include <asm/system_misc.h>
#include "generic.h"
+#include "pm.h"
+
+/* AT91SAM9 valid PM modes. */
+static const int at91sam9_pm_modes[] = {
+ AT91_PM_STANDBY,
+ AT91_PM_ULP0,
+};
static void __init at91sam9_init(void)
{
of_platform_default_populate(NULL, NULL, NULL);
- at91sam9_pm_init();
+ at91sam9_pm_init(at91sam9_pm_modes, ARRAY_SIZE(at91sam9_pm_modes));
}
static const char *const at91_dt_board_compat[] __initconst = {
diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
index 0a4cdcb4985b..610a12e5a71c 100644
--- a/arch/arm/mach-at91/generic.h
+++ b/arch/arm/mach-at91/generic.h
@@ -9,17 +9,17 @@
#define _AT91_GENERIC_H
#ifdef CONFIG_PM
-extern void __init at91rm9200_pm_init(void);
-extern void __init at91sam9_pm_init(void);
-extern void __init sam9x60_pm_init(void);
-extern void __init sama5_pm_init(void);
-extern void __init sama5d2_pm_init(void);
+extern void __init at91rm9200_pm_init(const int *modes, int len);
+extern void __init at91sam9_pm_init(const int *modes, int len);
+extern void __init sam9x60_pm_init(const int *modes, int len);
+extern void __init sama5_pm_init(const int *modes, int len);
+extern void __init sama5d2_pm_init(const int *modes, int len);
#else
-static inline void __init at91rm9200_pm_init(void) { }
-static inline void __init at91sam9_pm_init(void) { }
-static inline void __init sam9x60_pm_init(void) { }
-static inline void __init sama5_pm_init(void) { }
-static inline void __init sama5d2_pm_init(void) { }
+static inline void __init at91rm9200_pm_init(const int *modes, int len) { }
+static inline void __init at91sam9_pm_init(const int *modes, int len) { }
+static inline void __init sam9x60_pm_init(const int *modes, int len) { }
+static inline void __init sama5_pm_init(const int *modes, int len) { }
+static inline void __init sama5d2_pm_init(const int *modes, int len) { }
#endif
#endif /* _AT91_GENERIC_H */
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 04fdcbd57100..caf87efc7eeb 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -785,6 +785,59 @@ static const struct of_device_id atmel_pmc_ids[] __initconst = {
{ /* sentinel */ },
};
+static void __init at91_pm_modes_validate(const int *modes, int len)
+{
+ u8 i, located = 0;
+ int mode;
+
+#define STANDBY BIT(0)
+#define SUSPEND BIT(1)
+
+ for (i = 0; i < len; i++) {
+ if ((located & STANDBY) && (located & SUSPEND))
+ break;
+
+ if (modes[i] == soc_pm.data.standby_mode &&
+ !(located & STANDBY)) {
+ located |= STANDBY;
+ continue;
+ }
+
+ if (modes[i] == soc_pm.data.suspend_mode &&
+ !(located & SUSPEND)) {
+ located |= SUSPEND;
+ continue;
+ }
+ }
+
+ if (!(located & STANDBY)) {
+ if (soc_pm.data.suspend_mode == AT91_PM_STANDBY)
+ mode = AT91_PM_ULP0;
+ else
+ mode = AT91_PM_STANDBY;
+
+ pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
+ pm_modes[soc_pm.data.standby_mode].pattern,
+ pm_modes[mode].pattern);
+ soc_pm.data.standby_mode = mode;
+ }
+
+ if (!(located & SUSPEND)) {
+ if (soc_pm.data.standby_mode == AT91_PM_ULP0)
+ mode = AT91_PM_STANDBY;
+ else
+ mode = AT91_PM_ULP0;
+
+ pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
+ pm_modes[soc_pm.data.suspend_mode].pattern,
+ pm_modes[mode].pattern);
+ soc_pm.data.suspend_mode = mode;
+ }
+
+#undef STANDBY
+#undef SUSPEND
+}
+
static void __init at91_pm_init(void (*pm_idle)(void))
{
struct device_node *pmc_np;
@@ -821,11 +874,12 @@ static void __init at91_pm_init(void (*pm_idle)(void))
}
}
-void __init at91rm9200_pm_init(void)
+void __init at91rm9200_pm_init(const int *modes, int len)
{
if (!IS_ENABLED(CONFIG_SOC_AT91RM9200))
return;
+ at91_pm_modes_validate(modes, len);
at91_dt_ramc();
/*
@@ -836,11 +890,12 @@ void __init at91rm9200_pm_init(void)
at91_pm_init(at91rm9200_idle);
}
-void __init sam9x60_pm_init(void)
+void __init sam9x60_pm_init(const int *modes, int len)
{
if (!IS_ENABLED(CONFIG_SOC_SAM9X60))
return;
+ at91_pm_modes_validate(modes, len);
at91_pm_modes_init();
at91_dt_ramc();
at91_pm_init(at91sam9x60_idle);
@@ -849,31 +904,33 @@ void __init sam9x60_pm_init(void)
soc_pm.config_pmc_ws = at91_sam9x60_config_pmc_ws;
}
-void __init at91sam9_pm_init(void)
+void __init at91sam9_pm_init(const int *modes, int len)
{
if (!IS_ENABLED(CONFIG_SOC_AT91SAM9))
return;
+ at91_pm_modes_validate(modes, len);
at91_dt_ramc();
at91_pm_init(at91sam9_idle);
}
-void __init sama5_pm_init(void)
+void __init sama5_pm_init(const int *modes, int len)
{
if (!IS_ENABLED(CONFIG_SOC_SAMA5))
return;
+ at91_pm_modes_validate(modes, len);
at91_dt_ramc();
at91_pm_init(NULL);
}
-void __init sama5d2_pm_init(void)
+void __init sama5d2_pm_init(const int *modes, int len)
{
if (!IS_ENABLED(CONFIG_SOC_SAMA5D2))
return;
+ sama5_pm_init(modes, len);
at91_pm_modes_init();
- sama5_pm_init();
soc_pm.ws_ids = sama5d2_ws_ids;
soc_pm.config_shdwc_ws = at91_sama5d2_config_shdwc_ws;
diff --git a/arch/arm/mach-at91/sam9x60.c b/arch/arm/mach-at91/sam9x60.c
index d8c739d25458..d7c869c7b542 100644
--- a/arch/arm/mach-at91/sam9x60.c
+++ b/arch/arm/mach-at91/sam9x60.c
@@ -14,12 +14,21 @@
#include <asm/system_misc.h>
#include "generic.h"
+#include "pm.h"
+
+/* SAM9X60 valid PM modes. */
+static const int sam9x60_pm_modes[] = {
+ AT91_PM_STANDBY,
+ AT91_PM_ULP0,
+ AT91_PM_ULP0_FAST,
+ AT91_PM_ULP1,
+};
static void __init sam9x60_init(void)
{
of_platform_default_populate(NULL, NULL, NULL);
- sam9x60_pm_init();
+ sam9x60_pm_init(sam9x60_pm_modes, ARRAY_SIZE(sam9x60_pm_modes));
}
static const char *const sam9x60_dt_board_compat[] __initconst = {
diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
index 89dab7cf01e8..7eb124612a10 100644
--- a/arch/arm/mach-at91/sama5.c
+++ b/arch/arm/mach-at91/sama5.c
@@ -14,11 +14,19 @@
#include <asm/system_misc.h>
#include "generic.h"
+#include "pm.h"
+
+/* SAMA5 valid PM modes. */
+static const int sama5_pm_modes[] = {
+ AT91_PM_STANDBY,
+ AT91_PM_ULP0,
+ AT91_PM_ULP0_FAST,
+};
static void __init sama5_dt_device_init(void)
{
of_platform_default_populate(NULL, NULL, NULL);
- sama5_pm_init();
+ sama5_pm_init(sama5_pm_modes, ARRAY_SIZE(sama5_pm_modes));
}
static const char *const sama5_dt_board_compat[] __initconst = {
@@ -44,10 +52,19 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
.l2c_aux_mask = ~0UL,
MACHINE_END
+/* sama5d2 PM modes. */
+static const int sama5d2_pm_modes[] = {
+ AT91_PM_STANDBY,
+ AT91_PM_ULP0,
+ AT91_PM_ULP0_FAST,
+ AT91_PM_ULP1,
+ AT91_PM_BACKUP,
+};
+
static void __init sama5d2_init(void)
{
of_platform_default_populate(NULL, NULL, NULL);
- sama5d2_pm_init();
+ sama5d2_pm_init(sama5d2_pm_modes, ARRAY_SIZE(sama5d2_pm_modes));
}
static const char *const sama5d2_compat[] __initconst = {
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] ARM: at91: pm: add per soc validation of pm modes
@ 2020-08-03 7:25 ` Claudiu Beznea
0 siblings, 0 replies; 14+ messages in thread
From: Claudiu Beznea @ 2020-08-03 7:25 UTC (permalink / raw)
To: nicolas.ferre, alexandre.belloni, ludovic.desroches
Cc: linux-kernel, linux-arm-kernel, Claudiu Beznea
Not all SoCs supports all the PM mode. User may end up settings,
e.g. backup mode, on a non SAMA5D2 device, but the mode to not be valid.
If backup mode is used on a devices not supporting it there will be no
way of resuming other than rebooting.
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
arch/arm/mach-at91/at91rm9200.c | 10 +++++-
arch/arm/mach-at91/at91sam9.c | 9 +++++-
arch/arm/mach-at91/generic.h | 20 ++++++------
arch/arm/mach-at91/pm.c | 69 +++++++++++++++++++++++++++++++++++++----
arch/arm/mach-at91/sam9x60.c | 11 ++++++-
arch/arm/mach-at91/sama5.c | 21 +++++++++++--
6 files changed, 119 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
index 4f8186211619..7318d8e16797 100644
--- a/arch/arm/mach-at91/at91rm9200.c
+++ b/arch/arm/mach-at91/at91rm9200.c
@@ -13,12 +13,20 @@
#include <asm/mach/arch.h>
#include "generic.h"
+#include "pm.h"
+
+/* AT91RM9200 valid PM modes. */
+static const int at91rm9200_pm_modes[] = {
+ AT91_PM_STANDBY,
+ AT91_PM_ULP0,
+};
static void __init at91rm9200_dt_device_init(void)
{
of_platform_default_populate(NULL, NULL, NULL);
- at91rm9200_pm_init();
+ at91rm9200_pm_init(at91rm9200_pm_modes,
+ ARRAY_SIZE(at91rm9200_pm_modes));
}
static const char *const at91rm9200_dt_board_compat[] __initconst = {
diff --git a/arch/arm/mach-at91/at91sam9.c b/arch/arm/mach-at91/at91sam9.c
index 7e572189a5eb..683f2c35a116 100644
--- a/arch/arm/mach-at91/at91sam9.c
+++ b/arch/arm/mach-at91/at91sam9.c
@@ -13,12 +13,19 @@
#include <asm/system_misc.h>
#include "generic.h"
+#include "pm.h"
+
+/* AT91SAM9 valid PM modes. */
+static const int at91sam9_pm_modes[] = {
+ AT91_PM_STANDBY,
+ AT91_PM_ULP0,
+};
static void __init at91sam9_init(void)
{
of_platform_default_populate(NULL, NULL, NULL);
- at91sam9_pm_init();
+ at91sam9_pm_init(at91sam9_pm_modes, ARRAY_SIZE(at91sam9_pm_modes));
}
static const char *const at91_dt_board_compat[] __initconst = {
diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
index 0a4cdcb4985b..610a12e5a71c 100644
--- a/arch/arm/mach-at91/generic.h
+++ b/arch/arm/mach-at91/generic.h
@@ -9,17 +9,17 @@
#define _AT91_GENERIC_H
#ifdef CONFIG_PM
-extern void __init at91rm9200_pm_init(void);
-extern void __init at91sam9_pm_init(void);
-extern void __init sam9x60_pm_init(void);
-extern void __init sama5_pm_init(void);
-extern void __init sama5d2_pm_init(void);
+extern void __init at91rm9200_pm_init(const int *modes, int len);
+extern void __init at91sam9_pm_init(const int *modes, int len);
+extern void __init sam9x60_pm_init(const int *modes, int len);
+extern void __init sama5_pm_init(const int *modes, int len);
+extern void __init sama5d2_pm_init(const int *modes, int len);
#else
-static inline void __init at91rm9200_pm_init(void) { }
-static inline void __init at91sam9_pm_init(void) { }
-static inline void __init sam9x60_pm_init(void) { }
-static inline void __init sama5_pm_init(void) { }
-static inline void __init sama5d2_pm_init(void) { }
+static inline void __init at91rm9200_pm_init(const int *modes, int len) { }
+static inline void __init at91sam9_pm_init(const int *modes, int len) { }
+static inline void __init sam9x60_pm_init(const int *modes, int len) { }
+static inline void __init sama5_pm_init(const int *modes, int len) { }
+static inline void __init sama5d2_pm_init(const int *modes, int len) { }
#endif
#endif /* _AT91_GENERIC_H */
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 04fdcbd57100..caf87efc7eeb 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -785,6 +785,59 @@ static const struct of_device_id atmel_pmc_ids[] __initconst = {
{ /* sentinel */ },
};
+static void __init at91_pm_modes_validate(const int *modes, int len)
+{
+ u8 i, located = 0;
+ int mode;
+
+#define STANDBY BIT(0)
+#define SUSPEND BIT(1)
+
+ for (i = 0; i < len; i++) {
+ if ((located & STANDBY) && (located & SUSPEND))
+ break;
+
+ if (modes[i] == soc_pm.data.standby_mode &&
+ !(located & STANDBY)) {
+ located |= STANDBY;
+ continue;
+ }
+
+ if (modes[i] == soc_pm.data.suspend_mode &&
+ !(located & SUSPEND)) {
+ located |= SUSPEND;
+ continue;
+ }
+ }
+
+ if (!(located & STANDBY)) {
+ if (soc_pm.data.suspend_mode == AT91_PM_STANDBY)
+ mode = AT91_PM_ULP0;
+ else
+ mode = AT91_PM_STANDBY;
+
+ pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
+ pm_modes[soc_pm.data.standby_mode].pattern,
+ pm_modes[mode].pattern);
+ soc_pm.data.standby_mode = mode;
+ }
+
+ if (!(located & SUSPEND)) {
+ if (soc_pm.data.standby_mode == AT91_PM_ULP0)
+ mode = AT91_PM_STANDBY;
+ else
+ mode = AT91_PM_ULP0;
+
+ pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
+ pm_modes[soc_pm.data.suspend_mode].pattern,
+ pm_modes[mode].pattern);
+ soc_pm.data.suspend_mode = mode;
+ }
+
+#undef STANDBY
+#undef SUSPEND
+}
+
static void __init at91_pm_init(void (*pm_idle)(void))
{
struct device_node *pmc_np;
@@ -821,11 +874,12 @@ static void __init at91_pm_init(void (*pm_idle)(void))
}
}
-void __init at91rm9200_pm_init(void)
+void __init at91rm9200_pm_init(const int *modes, int len)
{
if (!IS_ENABLED(CONFIG_SOC_AT91RM9200))
return;
+ at91_pm_modes_validate(modes, len);
at91_dt_ramc();
/*
@@ -836,11 +890,12 @@ void __init at91rm9200_pm_init(void)
at91_pm_init(at91rm9200_idle);
}
-void __init sam9x60_pm_init(void)
+void __init sam9x60_pm_init(const int *modes, int len)
{
if (!IS_ENABLED(CONFIG_SOC_SAM9X60))
return;
+ at91_pm_modes_validate(modes, len);
at91_pm_modes_init();
at91_dt_ramc();
at91_pm_init(at91sam9x60_idle);
@@ -849,31 +904,33 @@ void __init sam9x60_pm_init(void)
soc_pm.config_pmc_ws = at91_sam9x60_config_pmc_ws;
}
-void __init at91sam9_pm_init(void)
+void __init at91sam9_pm_init(const int *modes, int len)
{
if (!IS_ENABLED(CONFIG_SOC_AT91SAM9))
return;
+ at91_pm_modes_validate(modes, len);
at91_dt_ramc();
at91_pm_init(at91sam9_idle);
}
-void __init sama5_pm_init(void)
+void __init sama5_pm_init(const int *modes, int len)
{
if (!IS_ENABLED(CONFIG_SOC_SAMA5))
return;
+ at91_pm_modes_validate(modes, len);
at91_dt_ramc();
at91_pm_init(NULL);
}
-void __init sama5d2_pm_init(void)
+void __init sama5d2_pm_init(const int *modes, int len)
{
if (!IS_ENABLED(CONFIG_SOC_SAMA5D2))
return;
+ sama5_pm_init(modes, len);
at91_pm_modes_init();
- sama5_pm_init();
soc_pm.ws_ids = sama5d2_ws_ids;
soc_pm.config_shdwc_ws = at91_sama5d2_config_shdwc_ws;
diff --git a/arch/arm/mach-at91/sam9x60.c b/arch/arm/mach-at91/sam9x60.c
index d8c739d25458..d7c869c7b542 100644
--- a/arch/arm/mach-at91/sam9x60.c
+++ b/arch/arm/mach-at91/sam9x60.c
@@ -14,12 +14,21 @@
#include <asm/system_misc.h>
#include "generic.h"
+#include "pm.h"
+
+/* SAM9X60 valid PM modes. */
+static const int sam9x60_pm_modes[] = {
+ AT91_PM_STANDBY,
+ AT91_PM_ULP0,
+ AT91_PM_ULP0_FAST,
+ AT91_PM_ULP1,
+};
static void __init sam9x60_init(void)
{
of_platform_default_populate(NULL, NULL, NULL);
- sam9x60_pm_init();
+ sam9x60_pm_init(sam9x60_pm_modes, ARRAY_SIZE(sam9x60_pm_modes));
}
static const char *const sam9x60_dt_board_compat[] __initconst = {
diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
index 89dab7cf01e8..7eb124612a10 100644
--- a/arch/arm/mach-at91/sama5.c
+++ b/arch/arm/mach-at91/sama5.c
@@ -14,11 +14,19 @@
#include <asm/system_misc.h>
#include "generic.h"
+#include "pm.h"
+
+/* SAMA5 valid PM modes. */
+static const int sama5_pm_modes[] = {
+ AT91_PM_STANDBY,
+ AT91_PM_ULP0,
+ AT91_PM_ULP0_FAST,
+};
static void __init sama5_dt_device_init(void)
{
of_platform_default_populate(NULL, NULL, NULL);
- sama5_pm_init();
+ sama5_pm_init(sama5_pm_modes, ARRAY_SIZE(sama5_pm_modes));
}
static const char *const sama5_dt_board_compat[] __initconst = {
@@ -44,10 +52,19 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
.l2c_aux_mask = ~0UL,
MACHINE_END
+/* sama5d2 PM modes. */
+static const int sama5d2_pm_modes[] = {
+ AT91_PM_STANDBY,
+ AT91_PM_ULP0,
+ AT91_PM_ULP0_FAST,
+ AT91_PM_ULP1,
+ AT91_PM_BACKUP,
+};
+
static void __init sama5d2_init(void)
{
of_platform_default_populate(NULL, NULL, NULL);
- sama5d2_pm_init();
+ sama5d2_pm_init(sama5d2_pm_modes, ARRAY_SIZE(sama5d2_pm_modes));
}
static const char *const sama5d2_compat[] __initconst = {
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: at91: pm: put node after its usage
2020-08-03 7:25 ` Claudiu Beznea
@ 2020-08-03 7:25 ` Claudiu Beznea
-1 siblings, 0 replies; 14+ messages in thread
From: Claudiu Beznea @ 2020-08-03 7:25 UTC (permalink / raw)
To: nicolas.ferre, alexandre.belloni, ludovic.desroches
Cc: linux-arm-kernel, linux-kernel, Claudiu Beznea
of_find_matching_node_and_match() returns a node pointer with refcount
incremented. Use of_node_put() after the node has been used.
Fixes: 13f16017d3e3f ("ARM: at91: pm: Tie the USB clock mask to the pmc")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
arch/arm/mach-at91/pm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index caf87efc7eeb..3d2ee8273b4c 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -849,6 +849,7 @@ static void __init at91_pm_init(void (*pm_idle)(void))
pmc_np = of_find_matching_node_and_match(NULL, atmel_pmc_ids, &of_id);
soc_pm.data.pmc = of_iomap(pmc_np, 0);
+ of_node_put(pmc_np);
if (!soc_pm.data.pmc) {
pr_err("AT91: PM not supported, PMC not found\n");
return;
--
2.7.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] ARM: at91: pm: put node after its usage
@ 2020-08-03 7:25 ` Claudiu Beznea
0 siblings, 0 replies; 14+ messages in thread
From: Claudiu Beznea @ 2020-08-03 7:25 UTC (permalink / raw)
To: nicolas.ferre, alexandre.belloni, ludovic.desroches
Cc: linux-kernel, linux-arm-kernel, Claudiu Beznea
of_find_matching_node_and_match() returns a node pointer with refcount
incremented. Use of_node_put() after the node has been used.
Fixes: 13f16017d3e3f ("ARM: at91: pm: Tie the USB clock mask to the pmc")
Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
arch/arm/mach-at91/pm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index caf87efc7eeb..3d2ee8273b4c 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -849,6 +849,7 @@ static void __init at91_pm_init(void (*pm_idle)(void))
pmc_np = of_find_matching_node_and_match(NULL, atmel_pmc_ids, &of_id);
soc_pm.data.pmc = of_iomap(pmc_np, 0);
+ of_node_put(pmc_np);
if (!soc_pm.data.pmc) {
pr_err("AT91: PM not supported, PMC not found\n");
return;
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] ARM: at91: pm: add per soc validation of pm modes
2020-08-03 7:25 ` Claudiu Beznea
@ 2020-08-03 8:34 ` Alexandre Belloni
-1 siblings, 0 replies; 14+ messages in thread
From: Alexandre Belloni @ 2020-08-03 8:34 UTC (permalink / raw)
To: Claudiu Beznea
Cc: nicolas.ferre, ludovic.desroches, linux-arm-kernel, linux-kernel
On 03/08/2020 10:25:16+0300, Claudiu Beznea wrote:
> Not all SoCs supports all the PM mode. User may end up settings,
> e.g. backup mode, on a non SAMA5D2 device, but the mode to not be valid.
> If backup mode is used on a devices not supporting it there will be no
> way of resuming other than rebooting.
I don't think this actually happens as if backup mode is requested on a
non sama5d2 device, of_find_compatible_node(NULL, NULL,
"atmel,sama5d2-sfrbu") will fail and the PM will default to
AT91_PM_ULP0.
ULP1 will also default to ULP0 if the shdwc is not from sama5d2 or
sam9x60.
I'm guessing the actual issue is introduced with ulp0 fast.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
> arch/arm/mach-at91/at91rm9200.c | 10 +++++-
> arch/arm/mach-at91/at91sam9.c | 9 +++++-
> arch/arm/mach-at91/generic.h | 20 ++++++------
> arch/arm/mach-at91/pm.c | 69 +++++++++++++++++++++++++++++++++++++----
> arch/arm/mach-at91/sam9x60.c | 11 ++++++-
> arch/arm/mach-at91/sama5.c | 21 +++++++++++--
> 6 files changed, 119 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
> index 4f8186211619..7318d8e16797 100644
> --- a/arch/arm/mach-at91/at91rm9200.c
> +++ b/arch/arm/mach-at91/at91rm9200.c
> @@ -13,12 +13,20 @@
> #include <asm/mach/arch.h>
>
> #include "generic.h"
> +#include "pm.h"
> +
> +/* AT91RM9200 valid PM modes. */
> +static const int at91rm9200_pm_modes[] = {
> + AT91_PM_STANDBY,
> + AT91_PM_ULP0,
> +};
>
> static void __init at91rm9200_dt_device_init(void)
> {
> of_platform_default_populate(NULL, NULL, NULL);
>
> - at91rm9200_pm_init();
> + at91rm9200_pm_init(at91rm9200_pm_modes,
> + ARRAY_SIZE(at91rm9200_pm_modes));
> }
>
You can avoid the changes in the soc files and leave everything
contained in pm.c as each <soc>_pm_init has a known list of mode that
will very likely never change. Bonus points if you make the arrays
__initdata.
> static const char *const at91rm9200_dt_board_compat[] __initconst = {
> diff --git a/arch/arm/mach-at91/at91sam9.c b/arch/arm/mach-at91/at91sam9.c
> index 7e572189a5eb..683f2c35a116 100644
> --- a/arch/arm/mach-at91/at91sam9.c
> +++ b/arch/arm/mach-at91/at91sam9.c
> @@ -13,12 +13,19 @@
> #include <asm/system_misc.h>
>
> #include "generic.h"
> +#include "pm.h"
> +
> +/* AT91SAM9 valid PM modes. */
> +static const int at91sam9_pm_modes[] = {
> + AT91_PM_STANDBY,
> + AT91_PM_ULP0,
> +};
>
> static void __init at91sam9_init(void)
> {
> of_platform_default_populate(NULL, NULL, NULL);
>
> - at91sam9_pm_init();
> + at91sam9_pm_init(at91sam9_pm_modes, ARRAY_SIZE(at91sam9_pm_modes));
> }
>
> static const char *const at91_dt_board_compat[] __initconst = {
> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
> index 0a4cdcb4985b..610a12e5a71c 100644
> --- a/arch/arm/mach-at91/generic.h
> +++ b/arch/arm/mach-at91/generic.h
> @@ -9,17 +9,17 @@
> #define _AT91_GENERIC_H
>
> #ifdef CONFIG_PM
> -extern void __init at91rm9200_pm_init(void);
> -extern void __init at91sam9_pm_init(void);
> -extern void __init sam9x60_pm_init(void);
> -extern void __init sama5_pm_init(void);
> -extern void __init sama5d2_pm_init(void);
> +extern void __init at91rm9200_pm_init(const int *modes, int len);
> +extern void __init at91sam9_pm_init(const int *modes, int len);
> +extern void __init sam9x60_pm_init(const int *modes, int len);
> +extern void __init sama5_pm_init(const int *modes, int len);
> +extern void __init sama5d2_pm_init(const int *modes, int len);
> #else
> -static inline void __init at91rm9200_pm_init(void) { }
> -static inline void __init at91sam9_pm_init(void) { }
> -static inline void __init sam9x60_pm_init(void) { }
> -static inline void __init sama5_pm_init(void) { }
> -static inline void __init sama5d2_pm_init(void) { }
> +static inline void __init at91rm9200_pm_init(const int *modes, int len) { }
> +static inline void __init at91sam9_pm_init(const int *modes, int len) { }
> +static inline void __init sam9x60_pm_init(const int *modes, int len) { }
> +static inline void __init sama5_pm_init(const int *modes, int len) { }
> +static inline void __init sama5d2_pm_init(const int *modes, int len) { }
> #endif
>
Then this change is not necessary.
> #endif /* _AT91_GENERIC_H */
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 04fdcbd57100..caf87efc7eeb 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -785,6 +785,59 @@ static const struct of_device_id atmel_pmc_ids[] __initconst = {
> { /* sentinel */ },
> };
>
> +static void __init at91_pm_modes_validate(const int *modes, int len)
> +{
> + u8 i, located = 0;
> + int mode;
> +
> +#define STANDBY BIT(0)
> +#define SUSPEND BIT(1)
> +
> + for (i = 0; i < len; i++) {
> + if ((located & STANDBY) && (located & SUSPEND))
> + break;
> +
> + if (modes[i] == soc_pm.data.standby_mode &&
> + !(located & STANDBY)) {
> + located |= STANDBY;
> + continue;
> + }
> +
> + if (modes[i] == soc_pm.data.suspend_mode &&
> + !(located & SUSPEND)) {
> + located |= SUSPEND;
> + continue;
> + }
> + }
> +
> + if (!(located & STANDBY)) {
> + if (soc_pm.data.suspend_mode == AT91_PM_STANDBY)
> + mode = AT91_PM_ULP0;
> + else
> + mode = AT91_PM_STANDBY;
> +
> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
> + pm_modes[soc_pm.data.standby_mode].pattern,
> + pm_modes[mode].pattern);
> + soc_pm.data.standby_mode = mode;
> + }
> +
> + if (!(located & SUSPEND)) {
> + if (soc_pm.data.standby_mode == AT91_PM_ULP0)
> + mode = AT91_PM_STANDBY;
> + else
> + mode = AT91_PM_ULP0;
> +
> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
> + pm_modes[soc_pm.data.suspend_mode].pattern,
> + pm_modes[mode].pattern);
> + soc_pm.data.suspend_mode = mode;
> + }
> +
> +#undef STANDBY
> +#undef SUSPEND
You should use two booleans instead of a bit array, this would not make
the code longer and will avoid this #def/#undef thing.
> +}
> +
> static void __init at91_pm_init(void (*pm_idle)(void))
> {
> struct device_node *pmc_np;
> @@ -821,11 +874,12 @@ static void __init at91_pm_init(void (*pm_idle)(void))
> }
> }
>
> -void __init at91rm9200_pm_init(void)
> +void __init at91rm9200_pm_init(const int *modes, int len)
> {
> if (!IS_ENABLED(CONFIG_SOC_AT91RM9200))
> return;
>
> + at91_pm_modes_validate(modes, len);
> at91_dt_ramc();
>
> /*
> @@ -836,11 +890,12 @@ void __init at91rm9200_pm_init(void)
> at91_pm_init(at91rm9200_idle);
> }
>
> -void __init sam9x60_pm_init(void)
> +void __init sam9x60_pm_init(const int *modes, int len)
> {
> if (!IS_ENABLED(CONFIG_SOC_SAM9X60))
> return;
>
> + at91_pm_modes_validate(modes, len);
> at91_pm_modes_init();
> at91_dt_ramc();
> at91_pm_init(at91sam9x60_idle);
> @@ -849,31 +904,33 @@ void __init sam9x60_pm_init(void)
> soc_pm.config_pmc_ws = at91_sam9x60_config_pmc_ws;
> }
>
> -void __init at91sam9_pm_init(void)
> +void __init at91sam9_pm_init(const int *modes, int len)
> {
> if (!IS_ENABLED(CONFIG_SOC_AT91SAM9))
> return;
>
> + at91_pm_modes_validate(modes, len);
> at91_dt_ramc();
> at91_pm_init(at91sam9_idle);
> }
>
> -void __init sama5_pm_init(void)
> +void __init sama5_pm_init(const int *modes, int len)
> {
> if (!IS_ENABLED(CONFIG_SOC_SAMA5))
> return;
>
> + at91_pm_modes_validate(modes, len);
> at91_dt_ramc();
> at91_pm_init(NULL);
> }
>
> -void __init sama5d2_pm_init(void)
> +void __init sama5d2_pm_init(const int *modes, int len)
> {
> if (!IS_ENABLED(CONFIG_SOC_SAMA5D2))
> return;
>
> + sama5_pm_init(modes, len);
> at91_pm_modes_init();
> - sama5_pm_init();
>
> soc_pm.ws_ids = sama5d2_ws_ids;
> soc_pm.config_shdwc_ws = at91_sama5d2_config_shdwc_ws;
> diff --git a/arch/arm/mach-at91/sam9x60.c b/arch/arm/mach-at91/sam9x60.c
> index d8c739d25458..d7c869c7b542 100644
> --- a/arch/arm/mach-at91/sam9x60.c
> +++ b/arch/arm/mach-at91/sam9x60.c
> @@ -14,12 +14,21 @@
> #include <asm/system_misc.h>
>
> #include "generic.h"
> +#include "pm.h"
> +
> +/* SAM9X60 valid PM modes. */
> +static const int sam9x60_pm_modes[] = {
> + AT91_PM_STANDBY,
> + AT91_PM_ULP0,
> + AT91_PM_ULP0_FAST,
> + AT91_PM_ULP1,
> +};
>
> static void __init sam9x60_init(void)
> {
> of_platform_default_populate(NULL, NULL, NULL);
>
> - sam9x60_pm_init();
> + sam9x60_pm_init(sam9x60_pm_modes, ARRAY_SIZE(sam9x60_pm_modes));
> }
>
> static const char *const sam9x60_dt_board_compat[] __initconst = {
> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
> index 89dab7cf01e8..7eb124612a10 100644
> --- a/arch/arm/mach-at91/sama5.c
> +++ b/arch/arm/mach-at91/sama5.c
> @@ -14,11 +14,19 @@
> #include <asm/system_misc.h>
>
> #include "generic.h"
> +#include "pm.h"
> +
> +/* SAMA5 valid PM modes. */
> +static const int sama5_pm_modes[] = {
> + AT91_PM_STANDBY,
> + AT91_PM_ULP0,
> + AT91_PM_ULP0_FAST,
> +};
>
> static void __init sama5_dt_device_init(void)
> {
> of_platform_default_populate(NULL, NULL, NULL);
> - sama5_pm_init();
> + sama5_pm_init(sama5_pm_modes, ARRAY_SIZE(sama5_pm_modes));
> }
>
> static const char *const sama5_dt_board_compat[] __initconst = {
> @@ -44,10 +52,19 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
> .l2c_aux_mask = ~0UL,
> MACHINE_END
>
> +/* sama5d2 PM modes. */
> +static const int sama5d2_pm_modes[] = {
> + AT91_PM_STANDBY,
> + AT91_PM_ULP0,
> + AT91_PM_ULP0_FAST,
> + AT91_PM_ULP1,
> + AT91_PM_BACKUP,
> +};
> +
> static void __init sama5d2_init(void)
> {
> of_platform_default_populate(NULL, NULL, NULL);
> - sama5d2_pm_init();
> + sama5d2_pm_init(sama5d2_pm_modes, ARRAY_SIZE(sama5d2_pm_modes));
> }
>
> static const char *const sama5d2_compat[] __initconst = {
> --
> 2.7.4
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] ARM: at91: pm: add per soc validation of pm modes
@ 2020-08-03 8:34 ` Alexandre Belloni
0 siblings, 0 replies; 14+ messages in thread
From: Alexandre Belloni @ 2020-08-03 8:34 UTC (permalink / raw)
To: Claudiu Beznea; +Cc: ludovic.desroches, linux-arm-kernel, linux-kernel
On 03/08/2020 10:25:16+0300, Claudiu Beznea wrote:
> Not all SoCs supports all the PM mode. User may end up settings,
> e.g. backup mode, on a non SAMA5D2 device, but the mode to not be valid.
> If backup mode is used on a devices not supporting it there will be no
> way of resuming other than rebooting.
I don't think this actually happens as if backup mode is requested on a
non sama5d2 device, of_find_compatible_node(NULL, NULL,
"atmel,sama5d2-sfrbu") will fail and the PM will default to
AT91_PM_ULP0.
ULP1 will also default to ULP0 if the shdwc is not from sama5d2 or
sam9x60.
I'm guessing the actual issue is introduced with ulp0 fast.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
> arch/arm/mach-at91/at91rm9200.c | 10 +++++-
> arch/arm/mach-at91/at91sam9.c | 9 +++++-
> arch/arm/mach-at91/generic.h | 20 ++++++------
> arch/arm/mach-at91/pm.c | 69 +++++++++++++++++++++++++++++++++++++----
> arch/arm/mach-at91/sam9x60.c | 11 ++++++-
> arch/arm/mach-at91/sama5.c | 21 +++++++++++--
> 6 files changed, 119 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
> index 4f8186211619..7318d8e16797 100644
> --- a/arch/arm/mach-at91/at91rm9200.c
> +++ b/arch/arm/mach-at91/at91rm9200.c
> @@ -13,12 +13,20 @@
> #include <asm/mach/arch.h>
>
> #include "generic.h"
> +#include "pm.h"
> +
> +/* AT91RM9200 valid PM modes. */
> +static const int at91rm9200_pm_modes[] = {
> + AT91_PM_STANDBY,
> + AT91_PM_ULP0,
> +};
>
> static void __init at91rm9200_dt_device_init(void)
> {
> of_platform_default_populate(NULL, NULL, NULL);
>
> - at91rm9200_pm_init();
> + at91rm9200_pm_init(at91rm9200_pm_modes,
> + ARRAY_SIZE(at91rm9200_pm_modes));
> }
>
You can avoid the changes in the soc files and leave everything
contained in pm.c as each <soc>_pm_init has a known list of mode that
will very likely never change. Bonus points if you make the arrays
__initdata.
> static const char *const at91rm9200_dt_board_compat[] __initconst = {
> diff --git a/arch/arm/mach-at91/at91sam9.c b/arch/arm/mach-at91/at91sam9.c
> index 7e572189a5eb..683f2c35a116 100644
> --- a/arch/arm/mach-at91/at91sam9.c
> +++ b/arch/arm/mach-at91/at91sam9.c
> @@ -13,12 +13,19 @@
> #include <asm/system_misc.h>
>
> #include "generic.h"
> +#include "pm.h"
> +
> +/* AT91SAM9 valid PM modes. */
> +static const int at91sam9_pm_modes[] = {
> + AT91_PM_STANDBY,
> + AT91_PM_ULP0,
> +};
>
> static void __init at91sam9_init(void)
> {
> of_platform_default_populate(NULL, NULL, NULL);
>
> - at91sam9_pm_init();
> + at91sam9_pm_init(at91sam9_pm_modes, ARRAY_SIZE(at91sam9_pm_modes));
> }
>
> static const char *const at91_dt_board_compat[] __initconst = {
> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
> index 0a4cdcb4985b..610a12e5a71c 100644
> --- a/arch/arm/mach-at91/generic.h
> +++ b/arch/arm/mach-at91/generic.h
> @@ -9,17 +9,17 @@
> #define _AT91_GENERIC_H
>
> #ifdef CONFIG_PM
> -extern void __init at91rm9200_pm_init(void);
> -extern void __init at91sam9_pm_init(void);
> -extern void __init sam9x60_pm_init(void);
> -extern void __init sama5_pm_init(void);
> -extern void __init sama5d2_pm_init(void);
> +extern void __init at91rm9200_pm_init(const int *modes, int len);
> +extern void __init at91sam9_pm_init(const int *modes, int len);
> +extern void __init sam9x60_pm_init(const int *modes, int len);
> +extern void __init sama5_pm_init(const int *modes, int len);
> +extern void __init sama5d2_pm_init(const int *modes, int len);
> #else
> -static inline void __init at91rm9200_pm_init(void) { }
> -static inline void __init at91sam9_pm_init(void) { }
> -static inline void __init sam9x60_pm_init(void) { }
> -static inline void __init sama5_pm_init(void) { }
> -static inline void __init sama5d2_pm_init(void) { }
> +static inline void __init at91rm9200_pm_init(const int *modes, int len) { }
> +static inline void __init at91sam9_pm_init(const int *modes, int len) { }
> +static inline void __init sam9x60_pm_init(const int *modes, int len) { }
> +static inline void __init sama5_pm_init(const int *modes, int len) { }
> +static inline void __init sama5d2_pm_init(const int *modes, int len) { }
> #endif
>
Then this change is not necessary.
> #endif /* _AT91_GENERIC_H */
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 04fdcbd57100..caf87efc7eeb 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -785,6 +785,59 @@ static const struct of_device_id atmel_pmc_ids[] __initconst = {
> { /* sentinel */ },
> };
>
> +static void __init at91_pm_modes_validate(const int *modes, int len)
> +{
> + u8 i, located = 0;
> + int mode;
> +
> +#define STANDBY BIT(0)
> +#define SUSPEND BIT(1)
> +
> + for (i = 0; i < len; i++) {
> + if ((located & STANDBY) && (located & SUSPEND))
> + break;
> +
> + if (modes[i] == soc_pm.data.standby_mode &&
> + !(located & STANDBY)) {
> + located |= STANDBY;
> + continue;
> + }
> +
> + if (modes[i] == soc_pm.data.suspend_mode &&
> + !(located & SUSPEND)) {
> + located |= SUSPEND;
> + continue;
> + }
> + }
> +
> + if (!(located & STANDBY)) {
> + if (soc_pm.data.suspend_mode == AT91_PM_STANDBY)
> + mode = AT91_PM_ULP0;
> + else
> + mode = AT91_PM_STANDBY;
> +
> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
> + pm_modes[soc_pm.data.standby_mode].pattern,
> + pm_modes[mode].pattern);
> + soc_pm.data.standby_mode = mode;
> + }
> +
> + if (!(located & SUSPEND)) {
> + if (soc_pm.data.standby_mode == AT91_PM_ULP0)
> + mode = AT91_PM_STANDBY;
> + else
> + mode = AT91_PM_ULP0;
> +
> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
> + pm_modes[soc_pm.data.suspend_mode].pattern,
> + pm_modes[mode].pattern);
> + soc_pm.data.suspend_mode = mode;
> + }
> +
> +#undef STANDBY
> +#undef SUSPEND
You should use two booleans instead of a bit array, this would not make
the code longer and will avoid this #def/#undef thing.
> +}
> +
> static void __init at91_pm_init(void (*pm_idle)(void))
> {
> struct device_node *pmc_np;
> @@ -821,11 +874,12 @@ static void __init at91_pm_init(void (*pm_idle)(void))
> }
> }
>
> -void __init at91rm9200_pm_init(void)
> +void __init at91rm9200_pm_init(const int *modes, int len)
> {
> if (!IS_ENABLED(CONFIG_SOC_AT91RM9200))
> return;
>
> + at91_pm_modes_validate(modes, len);
> at91_dt_ramc();
>
> /*
> @@ -836,11 +890,12 @@ void __init at91rm9200_pm_init(void)
> at91_pm_init(at91rm9200_idle);
> }
>
> -void __init sam9x60_pm_init(void)
> +void __init sam9x60_pm_init(const int *modes, int len)
> {
> if (!IS_ENABLED(CONFIG_SOC_SAM9X60))
> return;
>
> + at91_pm_modes_validate(modes, len);
> at91_pm_modes_init();
> at91_dt_ramc();
> at91_pm_init(at91sam9x60_idle);
> @@ -849,31 +904,33 @@ void __init sam9x60_pm_init(void)
> soc_pm.config_pmc_ws = at91_sam9x60_config_pmc_ws;
> }
>
> -void __init at91sam9_pm_init(void)
> +void __init at91sam9_pm_init(const int *modes, int len)
> {
> if (!IS_ENABLED(CONFIG_SOC_AT91SAM9))
> return;
>
> + at91_pm_modes_validate(modes, len);
> at91_dt_ramc();
> at91_pm_init(at91sam9_idle);
> }
>
> -void __init sama5_pm_init(void)
> +void __init sama5_pm_init(const int *modes, int len)
> {
> if (!IS_ENABLED(CONFIG_SOC_SAMA5))
> return;
>
> + at91_pm_modes_validate(modes, len);
> at91_dt_ramc();
> at91_pm_init(NULL);
> }
>
> -void __init sama5d2_pm_init(void)
> +void __init sama5d2_pm_init(const int *modes, int len)
> {
> if (!IS_ENABLED(CONFIG_SOC_SAMA5D2))
> return;
>
> + sama5_pm_init(modes, len);
> at91_pm_modes_init();
> - sama5_pm_init();
>
> soc_pm.ws_ids = sama5d2_ws_ids;
> soc_pm.config_shdwc_ws = at91_sama5d2_config_shdwc_ws;
> diff --git a/arch/arm/mach-at91/sam9x60.c b/arch/arm/mach-at91/sam9x60.c
> index d8c739d25458..d7c869c7b542 100644
> --- a/arch/arm/mach-at91/sam9x60.c
> +++ b/arch/arm/mach-at91/sam9x60.c
> @@ -14,12 +14,21 @@
> #include <asm/system_misc.h>
>
> #include "generic.h"
> +#include "pm.h"
> +
> +/* SAM9X60 valid PM modes. */
> +static const int sam9x60_pm_modes[] = {
> + AT91_PM_STANDBY,
> + AT91_PM_ULP0,
> + AT91_PM_ULP0_FAST,
> + AT91_PM_ULP1,
> +};
>
> static void __init sam9x60_init(void)
> {
> of_platform_default_populate(NULL, NULL, NULL);
>
> - sam9x60_pm_init();
> + sam9x60_pm_init(sam9x60_pm_modes, ARRAY_SIZE(sam9x60_pm_modes));
> }
>
> static const char *const sam9x60_dt_board_compat[] __initconst = {
> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
> index 89dab7cf01e8..7eb124612a10 100644
> --- a/arch/arm/mach-at91/sama5.c
> +++ b/arch/arm/mach-at91/sama5.c
> @@ -14,11 +14,19 @@
> #include <asm/system_misc.h>
>
> #include "generic.h"
> +#include "pm.h"
> +
> +/* SAMA5 valid PM modes. */
> +static const int sama5_pm_modes[] = {
> + AT91_PM_STANDBY,
> + AT91_PM_ULP0,
> + AT91_PM_ULP0_FAST,
> +};
>
> static void __init sama5_dt_device_init(void)
> {
> of_platform_default_populate(NULL, NULL, NULL);
> - sama5_pm_init();
> + sama5_pm_init(sama5_pm_modes, ARRAY_SIZE(sama5_pm_modes));
> }
>
> static const char *const sama5_dt_board_compat[] __initconst = {
> @@ -44,10 +52,19 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
> .l2c_aux_mask = ~0UL,
> MACHINE_END
>
> +/* sama5d2 PM modes. */
> +static const int sama5d2_pm_modes[] = {
> + AT91_PM_STANDBY,
> + AT91_PM_ULP0,
> + AT91_PM_ULP0_FAST,
> + AT91_PM_ULP1,
> + AT91_PM_BACKUP,
> +};
> +
> static void __init sama5d2_init(void)
> {
> of_platform_default_populate(NULL, NULL, NULL);
> - sama5d2_pm_init();
> + sama5d2_pm_init(sama5d2_pm_modes, ARRAY_SIZE(sama5d2_pm_modes));
> }
>
> static const char *const sama5d2_compat[] __initconst = {
> --
> 2.7.4
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] ARM: at91: pm: add per soc validation of pm modes
2020-08-03 8:34 ` Alexandre Belloni
@ 2020-08-03 10:54 ` Claudiu.Beznea
-1 siblings, 0 replies; 14+ messages in thread
From: Claudiu.Beznea @ 2020-08-03 10:54 UTC (permalink / raw)
To: alexandre.belloni
Cc: Nicolas.Ferre, Ludovic.Desroches, linux-arm-kernel, linux-kernel
On 03.08.2020 11:34, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 03/08/2020 10:25:16+0300, Claudiu Beznea wrote:
>> Not all SoCs supports all the PM mode. User may end up settings,
>> e.g. backup mode, on a non SAMA5D2 device, but the mode to not be valid.
>> If backup mode is used on a devices not supporting it there will be no
>> way of resuming other than rebooting.
>
> I don't think this actually happens as if backup mode is requested on a
> non sama5d2 device, of_find_compatible_node(NULL, NULL,
> "atmel,sama5d2-sfrbu") will fail and the PM will default to
> AT91_PM_ULP0.
of_find_compatible_node(NULL, NULL, "atmel,sama5d2-sfrbu") is called by
at91_pm_backup_init() which is called by at91_pm_modes_init() which is
called only by sam9x60_pm_init() and sama5d2_pm_init(). Same thing with
shutdown controller compatibles. Concluding, backup could be requested for
SoCs calling:
- at91rm9200_pm_init()
- at91sam9_pm_init()
- sama5_pm_init()
The other solution for this would be to call at91_pm_modes_init() also in
the functions above. I choose the approach in this patch to avoid tracking
the dependency b/w compatibles and supported modes.
Let me know what would you prefer as a solution.
>
> ULP1 will also default to ULP0 if the shdwc is not from sama5d2 or
> sam9x60.>
> I'm guessing the actual issue is introduced with ulp0 fast.
>
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>> arch/arm/mach-at91/at91rm9200.c | 10 +++++-
>> arch/arm/mach-at91/at91sam9.c | 9 +++++-
>> arch/arm/mach-at91/generic.h | 20 ++++++------
>> arch/arm/mach-at91/pm.c | 69 +++++++++++++++++++++++++++++++++++++----
>> arch/arm/mach-at91/sam9x60.c | 11 ++++++-
>> arch/arm/mach-at91/sama5.c | 21 +++++++++++--
>> 6 files changed, 119 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
>> index 4f8186211619..7318d8e16797 100644
>> --- a/arch/arm/mach-at91/at91rm9200.c
>> +++ b/arch/arm/mach-at91/at91rm9200.c
>> @@ -13,12 +13,20 @@
>> #include <asm/mach/arch.h>
>>
>> #include "generic.h"
>> +#include "pm.h"
>> +
>> +/* AT91RM9200 valid PM modes. */
>> +static const int at91rm9200_pm_modes[] = {
>> + AT91_PM_STANDBY,
>> + AT91_PM_ULP0,
>> +};
>>
>> static void __init at91rm9200_dt_device_init(void)
>> {
>> of_platform_default_populate(NULL, NULL, NULL);
>>
>> - at91rm9200_pm_init();
>> + at91rm9200_pm_init(at91rm9200_pm_modes,
>> + ARRAY_SIZE(at91rm9200_pm_modes));
>> }
>>
>
> You can avoid the changes in the soc files and leave everything
> contained in pm.c as each <soc>_pm_init has a known list of mode that
> will very likely never change. Bonus points if you make the arrays
> __initdata.
That was my initial approach. I went with the one in this patch to avoid
adding yet another struct of_device_id array in pm.c and keep code in pm.c
somehow platform independent. If you feel is better your way, then OK, I
have no problems with moving it in pm.c.
>
>> static const char *const at91rm9200_dt_board_compat[] __initconst = {
>> diff --git a/arch/arm/mach-at91/at91sam9.c b/arch/arm/mach-at91/at91sam9.c
>> index 7e572189a5eb..683f2c35a116 100644
>> --- a/arch/arm/mach-at91/at91sam9.c
>> +++ b/arch/arm/mach-at91/at91sam9.c
>> @@ -13,12 +13,19 @@
>> #include <asm/system_misc.h>
>>
>> #include "generic.h"
>> +#include "pm.h"
>> +
>> +/* AT91SAM9 valid PM modes. */
>> +static const int at91sam9_pm_modes[] = {
>> + AT91_PM_STANDBY,
>> + AT91_PM_ULP0,
>> +};
>>
>> static void __init at91sam9_init(void)
>> {
>> of_platform_default_populate(NULL, NULL, NULL);
>>
>> - at91sam9_pm_init();
>> + at91sam9_pm_init(at91sam9_pm_modes, ARRAY_SIZE(at91sam9_pm_modes));
>> }
>>
>> static const char *const at91_dt_board_compat[] __initconst = {
>> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
>> index 0a4cdcb4985b..610a12e5a71c 100644
>> --- a/arch/arm/mach-at91/generic.h
>> +++ b/arch/arm/mach-at91/generic.h
>> @@ -9,17 +9,17 @@
>> #define _AT91_GENERIC_H
>>
>> #ifdef CONFIG_PM
>> -extern void __init at91rm9200_pm_init(void);
>> -extern void __init at91sam9_pm_init(void);
>> -extern void __init sam9x60_pm_init(void);
>> -extern void __init sama5_pm_init(void);
>> -extern void __init sama5d2_pm_init(void);
>> +extern void __init at91rm9200_pm_init(const int *modes, int len);
>> +extern void __init at91sam9_pm_init(const int *modes, int len);
>> +extern void __init sam9x60_pm_init(const int *modes, int len);
>> +extern void __init sama5_pm_init(const int *modes, int len);
>> +extern void __init sama5d2_pm_init(const int *modes, int len);
>> #else
>> -static inline void __init at91rm9200_pm_init(void) { }
>> -static inline void __init at91sam9_pm_init(void) { }
>> -static inline void __init sam9x60_pm_init(void) { }
>> -static inline void __init sama5_pm_init(void) { }
>> -static inline void __init sama5d2_pm_init(void) { }
>> +static inline void __init at91rm9200_pm_init(const int *modes, int len) { }
>> +static inline void __init at91sam9_pm_init(const int *modes, int len) { }
>> +static inline void __init sam9x60_pm_init(const int *modes, int len) { }
>> +static inline void __init sama5_pm_init(const int *modes, int len) { }
>> +static inline void __init sama5d2_pm_init(const int *modes, int len) { }
>> #endif
>>
>
> Then this change is not necessary.
>
>> #endif /* _AT91_GENERIC_H */
>> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
>> index 04fdcbd57100..caf87efc7eeb 100644
>> --- a/arch/arm/mach-at91/pm.c
>> +++ b/arch/arm/mach-at91/pm.c
>> @@ -785,6 +785,59 @@ static const struct of_device_id atmel_pmc_ids[] __initconst = {
>> { /* sentinel */ },
>> };
>>
>> +static void __init at91_pm_modes_validate(const int *modes, int len)
>> +{
>> + u8 i, located = 0;
>> + int mode;
>> +
>> +#define STANDBY BIT(0)
>> +#define SUSPEND BIT(1)
>> +
>> + for (i = 0; i < len; i++) {
>> + if ((located & STANDBY) && (located & SUSPEND))
>> + break;
>> +
>> + if (modes[i] == soc_pm.data.standby_mode &&
>> + !(located & STANDBY)) {
>> + located |= STANDBY;
>> + continue;
>> + }
>> +
>> + if (modes[i] == soc_pm.data.suspend_mode &&
>> + !(located & SUSPEND)) {
>> + located |= SUSPEND;
>> + continue;
>> + }
>> + }
>> +
>> + if (!(located & STANDBY)) {
>> + if (soc_pm.data.suspend_mode == AT91_PM_STANDBY)
>> + mode = AT91_PM_ULP0;
>> + else
>> + mode = AT91_PM_STANDBY;
>> +
>> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
>> + pm_modes[soc_pm.data.standby_mode].pattern,
>> + pm_modes[mode].pattern);
>> + soc_pm.data.standby_mode = mode;
>> + }
>> +
>> + if (!(located & SUSPEND)) {
>> + if (soc_pm.data.standby_mode == AT91_PM_ULP0)
>> + mode = AT91_PM_STANDBY;
>> + else
>> + mode = AT91_PM_ULP0;
>> +
>> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
>> + pm_modes[soc_pm.data.suspend_mode].pattern,
>> + pm_modes[mode].pattern);
>> + soc_pm.data.suspend_mode = mode;
>> + }
>> +
>> +#undef STANDBY
>> +#undef SUSPEND
>
> You should use two booleans instead of a bit array, this would not make
> the code longer and will avoid this #def/#undef thing.
OK, I have no strong opinion on this. I'll switch to booleans.
>
>> +}
>> +
>> static void __init at91_pm_init(void (*pm_idle)(void))
>> {
>> struct device_node *pmc_np;
>> @@ -821,11 +874,12 @@ static void __init at91_pm_init(void (*pm_idle)(void))
>> }
>> }
>>
>> -void __init at91rm9200_pm_init(void)
>> +void __init at91rm9200_pm_init(const int *modes, int len)
>> {
>> if (!IS_ENABLED(CONFIG_SOC_AT91RM9200))
>> return;
>>
>> + at91_pm_modes_validate(modes, len);
>> at91_dt_ramc();
>>
>> /*
>> @@ -836,11 +890,12 @@ void __init at91rm9200_pm_init(void)
>> at91_pm_init(at91rm9200_idle);
>> }
>>
>> -void __init sam9x60_pm_init(void)
>> +void __init sam9x60_pm_init(const int *modes, int len)
>> {
>> if (!IS_ENABLED(CONFIG_SOC_SAM9X60))
>> return;
>>
>> + at91_pm_modes_validate(modes, len);
>> at91_pm_modes_init();
>> at91_dt_ramc();
>> at91_pm_init(at91sam9x60_idle);
>> @@ -849,31 +904,33 @@ void __init sam9x60_pm_init(void)
>> soc_pm.config_pmc_ws = at91_sam9x60_config_pmc_ws;
>> }
>>
>> -void __init at91sam9_pm_init(void)
>> +void __init at91sam9_pm_init(const int *modes, int len)
>> {
>> if (!IS_ENABLED(CONFIG_SOC_AT91SAM9))
>> return;
>>
>> + at91_pm_modes_validate(modes, len);
>> at91_dt_ramc();
>> at91_pm_init(at91sam9_idle);
>> }
>>
>> -void __init sama5_pm_init(void)
>> +void __init sama5_pm_init(const int *modes, int len)
>> {
>> if (!IS_ENABLED(CONFIG_SOC_SAMA5))
>> return;
>>
>> + at91_pm_modes_validate(modes, len);
>> at91_dt_ramc();
>> at91_pm_init(NULL);
>> }
>>
>> -void __init sama5d2_pm_init(void)
>> +void __init sama5d2_pm_init(const int *modes, int len)
>> {
>> if (!IS_ENABLED(CONFIG_SOC_SAMA5D2))
>> return;
>>
>> + sama5_pm_init(modes, len);
>> at91_pm_modes_init();
>> - sama5_pm_init();
>>
>> soc_pm.ws_ids = sama5d2_ws_ids;
>> soc_pm.config_shdwc_ws = at91_sama5d2_config_shdwc_ws;
>> diff --git a/arch/arm/mach-at91/sam9x60.c b/arch/arm/mach-at91/sam9x60.c
>> index d8c739d25458..d7c869c7b542 100644
>> --- a/arch/arm/mach-at91/sam9x60.c
>> +++ b/arch/arm/mach-at91/sam9x60.c
>> @@ -14,12 +14,21 @@
>> #include <asm/system_misc.h>
>>
>> #include "generic.h"
>> +#include "pm.h"
>> +
>> +/* SAM9X60 valid PM modes. */
>> +static const int sam9x60_pm_modes[] = {
>> + AT91_PM_STANDBY,
>> + AT91_PM_ULP0,
>> + AT91_PM_ULP0_FAST,
>> + AT91_PM_ULP1,
>> +};
>>
>> static void __init sam9x60_init(void)
>> {
>> of_platform_default_populate(NULL, NULL, NULL);
>>
>> - sam9x60_pm_init();
>> + sam9x60_pm_init(sam9x60_pm_modes, ARRAY_SIZE(sam9x60_pm_modes));
>> }
>>
>> static const char *const sam9x60_dt_board_compat[] __initconst = {
>> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
>> index 89dab7cf01e8..7eb124612a10 100644
>> --- a/arch/arm/mach-at91/sama5.c
>> +++ b/arch/arm/mach-at91/sama5.c
>> @@ -14,11 +14,19 @@
>> #include <asm/system_misc.h>
>>
>> #include "generic.h"
>> +#include "pm.h"
>> +
>> +/* SAMA5 valid PM modes. */
>> +static const int sama5_pm_modes[] = {
>> + AT91_PM_STANDBY,
>> + AT91_PM_ULP0,
>> + AT91_PM_ULP0_FAST,
>> +};
>>
>> static void __init sama5_dt_device_init(void)
>> {
>> of_platform_default_populate(NULL, NULL, NULL);
>> - sama5_pm_init();
>> + sama5_pm_init(sama5_pm_modes, ARRAY_SIZE(sama5_pm_modes));
>> }
>>
>> static const char *const sama5_dt_board_compat[] __initconst = {
>> @@ -44,10 +52,19 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
>> .l2c_aux_mask = ~0UL,
>> MACHINE_END
>>
>> +/* sama5d2 PM modes. */
>> +static const int sama5d2_pm_modes[] = {
>> + AT91_PM_STANDBY,
>> + AT91_PM_ULP0,
>> + AT91_PM_ULP0_FAST,
>> + AT91_PM_ULP1,
>> + AT91_PM_BACKUP,
>> +};
>> +
>> static void __init sama5d2_init(void)
>> {
>> of_platform_default_populate(NULL, NULL, NULL);
>> - sama5d2_pm_init();
>> + sama5d2_pm_init(sama5d2_pm_modes, ARRAY_SIZE(sama5d2_pm_modes));
>> }
>>
>> static const char *const sama5d2_compat[] __initconst = {
>> --
>> 2.7.4
>>
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] ARM: at91: pm: add per soc validation of pm modes
@ 2020-08-03 10:54 ` Claudiu.Beznea
0 siblings, 0 replies; 14+ messages in thread
From: Claudiu.Beznea @ 2020-08-03 10:54 UTC (permalink / raw)
To: alexandre.belloni; +Cc: Ludovic.Desroches, linux-arm-kernel, linux-kernel
On 03.08.2020 11:34, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 03/08/2020 10:25:16+0300, Claudiu Beznea wrote:
>> Not all SoCs supports all the PM mode. User may end up settings,
>> e.g. backup mode, on a non SAMA5D2 device, but the mode to not be valid.
>> If backup mode is used on a devices not supporting it there will be no
>> way of resuming other than rebooting.
>
> I don't think this actually happens as if backup mode is requested on a
> non sama5d2 device, of_find_compatible_node(NULL, NULL,
> "atmel,sama5d2-sfrbu") will fail and the PM will default to
> AT91_PM_ULP0.
of_find_compatible_node(NULL, NULL, "atmel,sama5d2-sfrbu") is called by
at91_pm_backup_init() which is called by at91_pm_modes_init() which is
called only by sam9x60_pm_init() and sama5d2_pm_init(). Same thing with
shutdown controller compatibles. Concluding, backup could be requested for
SoCs calling:
- at91rm9200_pm_init()
- at91sam9_pm_init()
- sama5_pm_init()
The other solution for this would be to call at91_pm_modes_init() also in
the functions above. I choose the approach in this patch to avoid tracking
the dependency b/w compatibles and supported modes.
Let me know what would you prefer as a solution.
>
> ULP1 will also default to ULP0 if the shdwc is not from sama5d2 or
> sam9x60.>
> I'm guessing the actual issue is introduced with ulp0 fast.
>
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>> arch/arm/mach-at91/at91rm9200.c | 10 +++++-
>> arch/arm/mach-at91/at91sam9.c | 9 +++++-
>> arch/arm/mach-at91/generic.h | 20 ++++++------
>> arch/arm/mach-at91/pm.c | 69 +++++++++++++++++++++++++++++++++++++----
>> arch/arm/mach-at91/sam9x60.c | 11 ++++++-
>> arch/arm/mach-at91/sama5.c | 21 +++++++++++--
>> 6 files changed, 119 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
>> index 4f8186211619..7318d8e16797 100644
>> --- a/arch/arm/mach-at91/at91rm9200.c
>> +++ b/arch/arm/mach-at91/at91rm9200.c
>> @@ -13,12 +13,20 @@
>> #include <asm/mach/arch.h>
>>
>> #include "generic.h"
>> +#include "pm.h"
>> +
>> +/* AT91RM9200 valid PM modes. */
>> +static const int at91rm9200_pm_modes[] = {
>> + AT91_PM_STANDBY,
>> + AT91_PM_ULP0,
>> +};
>>
>> static void __init at91rm9200_dt_device_init(void)
>> {
>> of_platform_default_populate(NULL, NULL, NULL);
>>
>> - at91rm9200_pm_init();
>> + at91rm9200_pm_init(at91rm9200_pm_modes,
>> + ARRAY_SIZE(at91rm9200_pm_modes));
>> }
>>
>
> You can avoid the changes in the soc files and leave everything
> contained in pm.c as each <soc>_pm_init has a known list of mode that
> will very likely never change. Bonus points if you make the arrays
> __initdata.
That was my initial approach. I went with the one in this patch to avoid
adding yet another struct of_device_id array in pm.c and keep code in pm.c
somehow platform independent. If you feel is better your way, then OK, I
have no problems with moving it in pm.c.
>
>> static const char *const at91rm9200_dt_board_compat[] __initconst = {
>> diff --git a/arch/arm/mach-at91/at91sam9.c b/arch/arm/mach-at91/at91sam9.c
>> index 7e572189a5eb..683f2c35a116 100644
>> --- a/arch/arm/mach-at91/at91sam9.c
>> +++ b/arch/arm/mach-at91/at91sam9.c
>> @@ -13,12 +13,19 @@
>> #include <asm/system_misc.h>
>>
>> #include "generic.h"
>> +#include "pm.h"
>> +
>> +/* AT91SAM9 valid PM modes. */
>> +static const int at91sam9_pm_modes[] = {
>> + AT91_PM_STANDBY,
>> + AT91_PM_ULP0,
>> +};
>>
>> static void __init at91sam9_init(void)
>> {
>> of_platform_default_populate(NULL, NULL, NULL);
>>
>> - at91sam9_pm_init();
>> + at91sam9_pm_init(at91sam9_pm_modes, ARRAY_SIZE(at91sam9_pm_modes));
>> }
>>
>> static const char *const at91_dt_board_compat[] __initconst = {
>> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
>> index 0a4cdcb4985b..610a12e5a71c 100644
>> --- a/arch/arm/mach-at91/generic.h
>> +++ b/arch/arm/mach-at91/generic.h
>> @@ -9,17 +9,17 @@
>> #define _AT91_GENERIC_H
>>
>> #ifdef CONFIG_PM
>> -extern void __init at91rm9200_pm_init(void);
>> -extern void __init at91sam9_pm_init(void);
>> -extern void __init sam9x60_pm_init(void);
>> -extern void __init sama5_pm_init(void);
>> -extern void __init sama5d2_pm_init(void);
>> +extern void __init at91rm9200_pm_init(const int *modes, int len);
>> +extern void __init at91sam9_pm_init(const int *modes, int len);
>> +extern void __init sam9x60_pm_init(const int *modes, int len);
>> +extern void __init sama5_pm_init(const int *modes, int len);
>> +extern void __init sama5d2_pm_init(const int *modes, int len);
>> #else
>> -static inline void __init at91rm9200_pm_init(void) { }
>> -static inline void __init at91sam9_pm_init(void) { }
>> -static inline void __init sam9x60_pm_init(void) { }
>> -static inline void __init sama5_pm_init(void) { }
>> -static inline void __init sama5d2_pm_init(void) { }
>> +static inline void __init at91rm9200_pm_init(const int *modes, int len) { }
>> +static inline void __init at91sam9_pm_init(const int *modes, int len) { }
>> +static inline void __init sam9x60_pm_init(const int *modes, int len) { }
>> +static inline void __init sama5_pm_init(const int *modes, int len) { }
>> +static inline void __init sama5d2_pm_init(const int *modes, int len) { }
>> #endif
>>
>
> Then this change is not necessary.
>
>> #endif /* _AT91_GENERIC_H */
>> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
>> index 04fdcbd57100..caf87efc7eeb 100644
>> --- a/arch/arm/mach-at91/pm.c
>> +++ b/arch/arm/mach-at91/pm.c
>> @@ -785,6 +785,59 @@ static const struct of_device_id atmel_pmc_ids[] __initconst = {
>> { /* sentinel */ },
>> };
>>
>> +static void __init at91_pm_modes_validate(const int *modes, int len)
>> +{
>> + u8 i, located = 0;
>> + int mode;
>> +
>> +#define STANDBY BIT(0)
>> +#define SUSPEND BIT(1)
>> +
>> + for (i = 0; i < len; i++) {
>> + if ((located & STANDBY) && (located & SUSPEND))
>> + break;
>> +
>> + if (modes[i] == soc_pm.data.standby_mode &&
>> + !(located & STANDBY)) {
>> + located |= STANDBY;
>> + continue;
>> + }
>> +
>> + if (modes[i] == soc_pm.data.suspend_mode &&
>> + !(located & SUSPEND)) {
>> + located |= SUSPEND;
>> + continue;
>> + }
>> + }
>> +
>> + if (!(located & STANDBY)) {
>> + if (soc_pm.data.suspend_mode == AT91_PM_STANDBY)
>> + mode = AT91_PM_ULP0;
>> + else
>> + mode = AT91_PM_STANDBY;
>> +
>> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
>> + pm_modes[soc_pm.data.standby_mode].pattern,
>> + pm_modes[mode].pattern);
>> + soc_pm.data.standby_mode = mode;
>> + }
>> +
>> + if (!(located & SUSPEND)) {
>> + if (soc_pm.data.standby_mode == AT91_PM_ULP0)
>> + mode = AT91_PM_STANDBY;
>> + else
>> + mode = AT91_PM_ULP0;
>> +
>> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
>> + pm_modes[soc_pm.data.suspend_mode].pattern,
>> + pm_modes[mode].pattern);
>> + soc_pm.data.suspend_mode = mode;
>> + }
>> +
>> +#undef STANDBY
>> +#undef SUSPEND
>
> You should use two booleans instead of a bit array, this would not make
> the code longer and will avoid this #def/#undef thing.
OK, I have no strong opinion on this. I'll switch to booleans.
>
>> +}
>> +
>> static void __init at91_pm_init(void (*pm_idle)(void))
>> {
>> struct device_node *pmc_np;
>> @@ -821,11 +874,12 @@ static void __init at91_pm_init(void (*pm_idle)(void))
>> }
>> }
>>
>> -void __init at91rm9200_pm_init(void)
>> +void __init at91rm9200_pm_init(const int *modes, int len)
>> {
>> if (!IS_ENABLED(CONFIG_SOC_AT91RM9200))
>> return;
>>
>> + at91_pm_modes_validate(modes, len);
>> at91_dt_ramc();
>>
>> /*
>> @@ -836,11 +890,12 @@ void __init at91rm9200_pm_init(void)
>> at91_pm_init(at91rm9200_idle);
>> }
>>
>> -void __init sam9x60_pm_init(void)
>> +void __init sam9x60_pm_init(const int *modes, int len)
>> {
>> if (!IS_ENABLED(CONFIG_SOC_SAM9X60))
>> return;
>>
>> + at91_pm_modes_validate(modes, len);
>> at91_pm_modes_init();
>> at91_dt_ramc();
>> at91_pm_init(at91sam9x60_idle);
>> @@ -849,31 +904,33 @@ void __init sam9x60_pm_init(void)
>> soc_pm.config_pmc_ws = at91_sam9x60_config_pmc_ws;
>> }
>>
>> -void __init at91sam9_pm_init(void)
>> +void __init at91sam9_pm_init(const int *modes, int len)
>> {
>> if (!IS_ENABLED(CONFIG_SOC_AT91SAM9))
>> return;
>>
>> + at91_pm_modes_validate(modes, len);
>> at91_dt_ramc();
>> at91_pm_init(at91sam9_idle);
>> }
>>
>> -void __init sama5_pm_init(void)
>> +void __init sama5_pm_init(const int *modes, int len)
>> {
>> if (!IS_ENABLED(CONFIG_SOC_SAMA5))
>> return;
>>
>> + at91_pm_modes_validate(modes, len);
>> at91_dt_ramc();
>> at91_pm_init(NULL);
>> }
>>
>> -void __init sama5d2_pm_init(void)
>> +void __init sama5d2_pm_init(const int *modes, int len)
>> {
>> if (!IS_ENABLED(CONFIG_SOC_SAMA5D2))
>> return;
>>
>> + sama5_pm_init(modes, len);
>> at91_pm_modes_init();
>> - sama5_pm_init();
>>
>> soc_pm.ws_ids = sama5d2_ws_ids;
>> soc_pm.config_shdwc_ws = at91_sama5d2_config_shdwc_ws;
>> diff --git a/arch/arm/mach-at91/sam9x60.c b/arch/arm/mach-at91/sam9x60.c
>> index d8c739d25458..d7c869c7b542 100644
>> --- a/arch/arm/mach-at91/sam9x60.c
>> +++ b/arch/arm/mach-at91/sam9x60.c
>> @@ -14,12 +14,21 @@
>> #include <asm/system_misc.h>
>>
>> #include "generic.h"
>> +#include "pm.h"
>> +
>> +/* SAM9X60 valid PM modes. */
>> +static const int sam9x60_pm_modes[] = {
>> + AT91_PM_STANDBY,
>> + AT91_PM_ULP0,
>> + AT91_PM_ULP0_FAST,
>> + AT91_PM_ULP1,
>> +};
>>
>> static void __init sam9x60_init(void)
>> {
>> of_platform_default_populate(NULL, NULL, NULL);
>>
>> - sam9x60_pm_init();
>> + sam9x60_pm_init(sam9x60_pm_modes, ARRAY_SIZE(sam9x60_pm_modes));
>> }
>>
>> static const char *const sam9x60_dt_board_compat[] __initconst = {
>> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
>> index 89dab7cf01e8..7eb124612a10 100644
>> --- a/arch/arm/mach-at91/sama5.c
>> +++ b/arch/arm/mach-at91/sama5.c
>> @@ -14,11 +14,19 @@
>> #include <asm/system_misc.h>
>>
>> #include "generic.h"
>> +#include "pm.h"
>> +
>> +/* SAMA5 valid PM modes. */
>> +static const int sama5_pm_modes[] = {
>> + AT91_PM_STANDBY,
>> + AT91_PM_ULP0,
>> + AT91_PM_ULP0_FAST,
>> +};
>>
>> static void __init sama5_dt_device_init(void)
>> {
>> of_platform_default_populate(NULL, NULL, NULL);
>> - sama5_pm_init();
>> + sama5_pm_init(sama5_pm_modes, ARRAY_SIZE(sama5_pm_modes));
>> }
>>
>> static const char *const sama5_dt_board_compat[] __initconst = {
>> @@ -44,10 +52,19 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
>> .l2c_aux_mask = ~0UL,
>> MACHINE_END
>>
>> +/* sama5d2 PM modes. */
>> +static const int sama5d2_pm_modes[] = {
>> + AT91_PM_STANDBY,
>> + AT91_PM_ULP0,
>> + AT91_PM_ULP0_FAST,
>> + AT91_PM_ULP1,
>> + AT91_PM_BACKUP,
>> +};
>> +
>> static void __init sama5d2_init(void)
>> {
>> of_platform_default_populate(NULL, NULL, NULL);
>> - sama5d2_pm_init();
>> + sama5d2_pm_init(sama5d2_pm_modes, ARRAY_SIZE(sama5d2_pm_modes));
>> }
>>
>> static const char *const sama5d2_compat[] __initconst = {
>> --
>> 2.7.4
>>
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] ARM: at91: pm: add per soc validation of pm modes
2020-08-03 10:54 ` Claudiu.Beznea
@ 2020-08-03 12:51 ` Alexandre Belloni
-1 siblings, 0 replies; 14+ messages in thread
From: Alexandre Belloni @ 2020-08-03 12:51 UTC (permalink / raw)
To: Claudiu.Beznea
Cc: Nicolas.Ferre, Ludovic.Desroches, linux-arm-kernel, linux-kernel
On 03/08/2020 10:54:51+0000, Claudiu.Beznea@microchip.com wrote:
> >> diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
> >> index 4f8186211619..7318d8e16797 100644
> >> --- a/arch/arm/mach-at91/at91rm9200.c
> >> +++ b/arch/arm/mach-at91/at91rm9200.c
> >> @@ -13,12 +13,20 @@
> >> #include <asm/mach/arch.h>
> >>
> >> #include "generic.h"
> >> +#include "pm.h"
> >> +
> >> +/* AT91RM9200 valid PM modes. */
> >> +static const int at91rm9200_pm_modes[] = {
> >> + AT91_PM_STANDBY,
> >> + AT91_PM_ULP0,
> >> +};
> >>
> >> static void __init at91rm9200_dt_device_init(void)
> >> {
> >> of_platform_default_populate(NULL, NULL, NULL);
> >>
> >> - at91rm9200_pm_init();
> >> + at91rm9200_pm_init(at91rm9200_pm_modes,
> >> + ARRAY_SIZE(at91rm9200_pm_modes));
> >> }
> >>
> >
> > You can avoid the changes in the soc files and leave everything
> > contained in pm.c as each <soc>_pm_init has a known list of mode that
> > will very likely never change. Bonus points if you make the arrays
> > __initdata.
>
> That was my initial approach. I went with the one in this patch to avoid
> adding yet another struct of_device_id array in pm.c and keep code in pm.c
> somehow platform independent. If you feel is better your way, then OK, I
> have no problems with moving it in pm.c.
>
You don't need an other struct of_device_id array as you already have
all the _pm_init functions broken out by soc family.
Going further, for at91rm9200_pm_init and at91sam9_pm_init, maybe you
can simply enforce soc_pm.data.standby_mode = AT91_PM_STANDBY and
soc_pm.data.suspend_mode = AT91_PM_ULP0 instead of checking the modes.
> >
> >> static const char *const at91rm9200_dt_board_compat[] __initconst = {
> >> diff --git a/arch/arm/mach-at91/at91sam9.c b/arch/arm/mach-at91/at91sam9.c
> >> index 7e572189a5eb..683f2c35a116 100644
> >> --- a/arch/arm/mach-at91/at91sam9.c
> >> +++ b/arch/arm/mach-at91/at91sam9.c
> >> @@ -13,12 +13,19 @@
> >> #include <asm/system_misc.h>
> >>
> >> #include "generic.h"
> >> +#include "pm.h"
> >> +
> >> +/* AT91SAM9 valid PM modes. */
> >> +static const int at91sam9_pm_modes[] = {
> >> + AT91_PM_STANDBY,
> >> + AT91_PM_ULP0,
> >> +};
> >>
> >> static void __init at91sam9_init(void)
> >> {
> >> of_platform_default_populate(NULL, NULL, NULL);
> >>
> >> - at91sam9_pm_init();
> >> + at91sam9_pm_init(at91sam9_pm_modes, ARRAY_SIZE(at91sam9_pm_modes));
> >> }
> >>
> >> static const char *const at91_dt_board_compat[] __initconst = {
> >> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
> >> index 0a4cdcb4985b..610a12e5a71c 100644
> >> --- a/arch/arm/mach-at91/generic.h
> >> +++ b/arch/arm/mach-at91/generic.h
> >> @@ -9,17 +9,17 @@
> >> #define _AT91_GENERIC_H
> >>
> >> #ifdef CONFIG_PM
> >> -extern void __init at91rm9200_pm_init(void);
> >> -extern void __init at91sam9_pm_init(void);
> >> -extern void __init sam9x60_pm_init(void);
> >> -extern void __init sama5_pm_init(void);
> >> -extern void __init sama5d2_pm_init(void);
> >> +extern void __init at91rm9200_pm_init(const int *modes, int len);
> >> +extern void __init at91sam9_pm_init(const int *modes, int len);
> >> +extern void __init sam9x60_pm_init(const int *modes, int len);
> >> +extern void __init sama5_pm_init(const int *modes, int len);
> >> +extern void __init sama5d2_pm_init(const int *modes, int len);
> >> #else
> >> -static inline void __init at91rm9200_pm_init(void) { }
> >> -static inline void __init at91sam9_pm_init(void) { }
> >> -static inline void __init sam9x60_pm_init(void) { }
> >> -static inline void __init sama5_pm_init(void) { }
> >> -static inline void __init sama5d2_pm_init(void) { }
> >> +static inline void __init at91rm9200_pm_init(const int *modes, int len) { }
> >> +static inline void __init at91sam9_pm_init(const int *modes, int len) { }
> >> +static inline void __init sam9x60_pm_init(const int *modes, int len) { }
> >> +static inline void __init sama5_pm_init(const int *modes, int len) { }
> >> +static inline void __init sama5d2_pm_init(const int *modes, int len) { }
> >> #endif
> >>
> >
> > Then this change is not necessary.
> >
> >> #endif /* _AT91_GENERIC_H */
> >> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> >> index 04fdcbd57100..caf87efc7eeb 100644
> >> --- a/arch/arm/mach-at91/pm.c
> >> +++ b/arch/arm/mach-at91/pm.c
> >> @@ -785,6 +785,59 @@ static const struct of_device_id atmel_pmc_ids[] __initconst = {
> >> { /* sentinel */ },
> >> };
> >>
> >> +static void __init at91_pm_modes_validate(const int *modes, int len)
> >> +{
> >> + u8 i, located = 0;
> >> + int mode;
> >> +
> >> +#define STANDBY BIT(0)
> >> +#define SUSPEND BIT(1)
> >> +
> >> + for (i = 0; i < len; i++) {
> >> + if ((located & STANDBY) && (located & SUSPEND))
> >> + break;
> >> +
> >> + if (modes[i] == soc_pm.data.standby_mode &&
> >> + !(located & STANDBY)) {
> >> + located |= STANDBY;
> >> + continue;
> >> + }
> >> +
> >> + if (modes[i] == soc_pm.data.suspend_mode &&
> >> + !(located & SUSPEND)) {
> >> + located |= SUSPEND;
> >> + continue;
> >> + }
> >> + }
> >> +
> >> + if (!(located & STANDBY)) {
> >> + if (soc_pm.data.suspend_mode == AT91_PM_STANDBY)
> >> + mode = AT91_PM_ULP0;
> >> + else
> >> + mode = AT91_PM_STANDBY;
> >> +
> >> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
> >> + pm_modes[soc_pm.data.standby_mode].pattern,
> >> + pm_modes[mode].pattern);
> >> + soc_pm.data.standby_mode = mode;
> >> + }
> >> +
> >> + if (!(located & SUSPEND)) {
> >> + if (soc_pm.data.standby_mode == AT91_PM_ULP0)
> >> + mode = AT91_PM_STANDBY;
> >> + else
> >> + mode = AT91_PM_ULP0;
> >> +
> >> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
> >> + pm_modes[soc_pm.data.suspend_mode].pattern,
> >> + pm_modes[mode].pattern);
> >> + soc_pm.data.suspend_mode = mode;
> >> + }
> >> +
> >> +#undef STANDBY
> >> +#undef SUSPEND
> >
> > You should use two booleans instead of a bit array, this would not make
> > the code longer and will avoid this #def/#undef thing.
>
> OK, I have no strong opinion on this. I'll switch to booleans.
>
> >
> >> +}
> >> +
> >> static void __init at91_pm_init(void (*pm_idle)(void))
> >> {
> >> struct device_node *pmc_np;
> >> @@ -821,11 +874,12 @@ static void __init at91_pm_init(void (*pm_idle)(void))
> >> }
> >> }
> >>
> >> -void __init at91rm9200_pm_init(void)
> >> +void __init at91rm9200_pm_init(const int *modes, int len)
> >> {
> >> if (!IS_ENABLED(CONFIG_SOC_AT91RM9200))
> >> return;
> >>
> >> + at91_pm_modes_validate(modes, len);
> >> at91_dt_ramc();
> >>
> >> /*
> >> @@ -836,11 +890,12 @@ void __init at91rm9200_pm_init(void)
> >> at91_pm_init(at91rm9200_idle);
> >> }
> >>
> >> -void __init sam9x60_pm_init(void)
> >> +void __init sam9x60_pm_init(const int *modes, int len)
> >> {
> >> if (!IS_ENABLED(CONFIG_SOC_SAM9X60))
> >> return;
> >>
> >> + at91_pm_modes_validate(modes, len);
> >> at91_pm_modes_init();
> >> at91_dt_ramc();
> >> at91_pm_init(at91sam9x60_idle);
> >> @@ -849,31 +904,33 @@ void __init sam9x60_pm_init(void)
> >> soc_pm.config_pmc_ws = at91_sam9x60_config_pmc_ws;
> >> }
> >>
> >> -void __init at91sam9_pm_init(void)
> >> +void __init at91sam9_pm_init(const int *modes, int len)
> >> {
> >> if (!IS_ENABLED(CONFIG_SOC_AT91SAM9))
> >> return;
> >>
> >> + at91_pm_modes_validate(modes, len);
> >> at91_dt_ramc();
> >> at91_pm_init(at91sam9_idle);
> >> }
> >>
> >> -void __init sama5_pm_init(void)
> >> +void __init sama5_pm_init(const int *modes, int len)
> >> {
> >> if (!IS_ENABLED(CONFIG_SOC_SAMA5))
> >> return;
> >>
> >> + at91_pm_modes_validate(modes, len);
> >> at91_dt_ramc();
> >> at91_pm_init(NULL);
> >> }
> >>
> >> -void __init sama5d2_pm_init(void)
> >> +void __init sama5d2_pm_init(const int *modes, int len)
> >> {
> >> if (!IS_ENABLED(CONFIG_SOC_SAMA5D2))
> >> return;
> >>
> >> + sama5_pm_init(modes, len);
> >> at91_pm_modes_init();
> >> - sama5_pm_init();
> >>
> >> soc_pm.ws_ids = sama5d2_ws_ids;
> >> soc_pm.config_shdwc_ws = at91_sama5d2_config_shdwc_ws;
> >> diff --git a/arch/arm/mach-at91/sam9x60.c b/arch/arm/mach-at91/sam9x60.c
> >> index d8c739d25458..d7c869c7b542 100644
> >> --- a/arch/arm/mach-at91/sam9x60.c
> >> +++ b/arch/arm/mach-at91/sam9x60.c
> >> @@ -14,12 +14,21 @@
> >> #include <asm/system_misc.h>
> >>
> >> #include "generic.h"
> >> +#include "pm.h"
> >> +
> >> +/* SAM9X60 valid PM modes. */
> >> +static const int sam9x60_pm_modes[] = {
> >> + AT91_PM_STANDBY,
> >> + AT91_PM_ULP0,
> >> + AT91_PM_ULP0_FAST,
> >> + AT91_PM_ULP1,
> >> +};
> >>
> >> static void __init sam9x60_init(void)
> >> {
> >> of_platform_default_populate(NULL, NULL, NULL);
> >>
> >> - sam9x60_pm_init();
> >> + sam9x60_pm_init(sam9x60_pm_modes, ARRAY_SIZE(sam9x60_pm_modes));
> >> }
> >>
> >> static const char *const sam9x60_dt_board_compat[] __initconst = {
> >> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
> >> index 89dab7cf01e8..7eb124612a10 100644
> >> --- a/arch/arm/mach-at91/sama5.c
> >> +++ b/arch/arm/mach-at91/sama5.c
> >> @@ -14,11 +14,19 @@
> >> #include <asm/system_misc.h>
> >>
> >> #include "generic.h"
> >> +#include "pm.h"
> >> +
> >> +/* SAMA5 valid PM modes. */
> >> +static const int sama5_pm_modes[] = {
> >> + AT91_PM_STANDBY,
> >> + AT91_PM_ULP0,
> >> + AT91_PM_ULP0_FAST,
> >> +};
> >>
> >> static void __init sama5_dt_device_init(void)
> >> {
> >> of_platform_default_populate(NULL, NULL, NULL);
> >> - sama5_pm_init();
> >> + sama5_pm_init(sama5_pm_modes, ARRAY_SIZE(sama5_pm_modes));
> >> }
> >>
> >> static const char *const sama5_dt_board_compat[] __initconst = {
> >> @@ -44,10 +52,19 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
> >> .l2c_aux_mask = ~0UL,
> >> MACHINE_END
> >>
> >> +/* sama5d2 PM modes. */
> >> +static const int sama5d2_pm_modes[] = {
> >> + AT91_PM_STANDBY,
> >> + AT91_PM_ULP0,
> >> + AT91_PM_ULP0_FAST,
> >> + AT91_PM_ULP1,
> >> + AT91_PM_BACKUP,
> >> +};
> >> +
> >> static void __init sama5d2_init(void)
> >> {
> >> of_platform_default_populate(NULL, NULL, NULL);
> >> - sama5d2_pm_init();
> >> + sama5d2_pm_init(sama5d2_pm_modes, ARRAY_SIZE(sama5d2_pm_modes));
> >> }
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] ARM: at91: pm: add per soc validation of pm modes
@ 2020-08-03 12:51 ` Alexandre Belloni
0 siblings, 0 replies; 14+ messages in thread
From: Alexandre Belloni @ 2020-08-03 12:51 UTC (permalink / raw)
To: Claudiu.Beznea; +Cc: Ludovic.Desroches, linux-arm-kernel, linux-kernel
On 03/08/2020 10:54:51+0000, Claudiu.Beznea@microchip.com wrote:
> >> diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c
> >> index 4f8186211619..7318d8e16797 100644
> >> --- a/arch/arm/mach-at91/at91rm9200.c
> >> +++ b/arch/arm/mach-at91/at91rm9200.c
> >> @@ -13,12 +13,20 @@
> >> #include <asm/mach/arch.h>
> >>
> >> #include "generic.h"
> >> +#include "pm.h"
> >> +
> >> +/* AT91RM9200 valid PM modes. */
> >> +static const int at91rm9200_pm_modes[] = {
> >> + AT91_PM_STANDBY,
> >> + AT91_PM_ULP0,
> >> +};
> >>
> >> static void __init at91rm9200_dt_device_init(void)
> >> {
> >> of_platform_default_populate(NULL, NULL, NULL);
> >>
> >> - at91rm9200_pm_init();
> >> + at91rm9200_pm_init(at91rm9200_pm_modes,
> >> + ARRAY_SIZE(at91rm9200_pm_modes));
> >> }
> >>
> >
> > You can avoid the changes in the soc files and leave everything
> > contained in pm.c as each <soc>_pm_init has a known list of mode that
> > will very likely never change. Bonus points if you make the arrays
> > __initdata.
>
> That was my initial approach. I went with the one in this patch to avoid
> adding yet another struct of_device_id array in pm.c and keep code in pm.c
> somehow platform independent. If you feel is better your way, then OK, I
> have no problems with moving it in pm.c.
>
You don't need an other struct of_device_id array as you already have
all the _pm_init functions broken out by soc family.
Going further, for at91rm9200_pm_init and at91sam9_pm_init, maybe you
can simply enforce soc_pm.data.standby_mode = AT91_PM_STANDBY and
soc_pm.data.suspend_mode = AT91_PM_ULP0 instead of checking the modes.
> >
> >> static const char *const at91rm9200_dt_board_compat[] __initconst = {
> >> diff --git a/arch/arm/mach-at91/at91sam9.c b/arch/arm/mach-at91/at91sam9.c
> >> index 7e572189a5eb..683f2c35a116 100644
> >> --- a/arch/arm/mach-at91/at91sam9.c
> >> +++ b/arch/arm/mach-at91/at91sam9.c
> >> @@ -13,12 +13,19 @@
> >> #include <asm/system_misc.h>
> >>
> >> #include "generic.h"
> >> +#include "pm.h"
> >> +
> >> +/* AT91SAM9 valid PM modes. */
> >> +static const int at91sam9_pm_modes[] = {
> >> + AT91_PM_STANDBY,
> >> + AT91_PM_ULP0,
> >> +};
> >>
> >> static void __init at91sam9_init(void)
> >> {
> >> of_platform_default_populate(NULL, NULL, NULL);
> >>
> >> - at91sam9_pm_init();
> >> + at91sam9_pm_init(at91sam9_pm_modes, ARRAY_SIZE(at91sam9_pm_modes));
> >> }
> >>
> >> static const char *const at91_dt_board_compat[] __initconst = {
> >> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
> >> index 0a4cdcb4985b..610a12e5a71c 100644
> >> --- a/arch/arm/mach-at91/generic.h
> >> +++ b/arch/arm/mach-at91/generic.h
> >> @@ -9,17 +9,17 @@
> >> #define _AT91_GENERIC_H
> >>
> >> #ifdef CONFIG_PM
> >> -extern void __init at91rm9200_pm_init(void);
> >> -extern void __init at91sam9_pm_init(void);
> >> -extern void __init sam9x60_pm_init(void);
> >> -extern void __init sama5_pm_init(void);
> >> -extern void __init sama5d2_pm_init(void);
> >> +extern void __init at91rm9200_pm_init(const int *modes, int len);
> >> +extern void __init at91sam9_pm_init(const int *modes, int len);
> >> +extern void __init sam9x60_pm_init(const int *modes, int len);
> >> +extern void __init sama5_pm_init(const int *modes, int len);
> >> +extern void __init sama5d2_pm_init(const int *modes, int len);
> >> #else
> >> -static inline void __init at91rm9200_pm_init(void) { }
> >> -static inline void __init at91sam9_pm_init(void) { }
> >> -static inline void __init sam9x60_pm_init(void) { }
> >> -static inline void __init sama5_pm_init(void) { }
> >> -static inline void __init sama5d2_pm_init(void) { }
> >> +static inline void __init at91rm9200_pm_init(const int *modes, int len) { }
> >> +static inline void __init at91sam9_pm_init(const int *modes, int len) { }
> >> +static inline void __init sam9x60_pm_init(const int *modes, int len) { }
> >> +static inline void __init sama5_pm_init(const int *modes, int len) { }
> >> +static inline void __init sama5d2_pm_init(const int *modes, int len) { }
> >> #endif
> >>
> >
> > Then this change is not necessary.
> >
> >> #endif /* _AT91_GENERIC_H */
> >> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> >> index 04fdcbd57100..caf87efc7eeb 100644
> >> --- a/arch/arm/mach-at91/pm.c
> >> +++ b/arch/arm/mach-at91/pm.c
> >> @@ -785,6 +785,59 @@ static const struct of_device_id atmel_pmc_ids[] __initconst = {
> >> { /* sentinel */ },
> >> };
> >>
> >> +static void __init at91_pm_modes_validate(const int *modes, int len)
> >> +{
> >> + u8 i, located = 0;
> >> + int mode;
> >> +
> >> +#define STANDBY BIT(0)
> >> +#define SUSPEND BIT(1)
> >> +
> >> + for (i = 0; i < len; i++) {
> >> + if ((located & STANDBY) && (located & SUSPEND))
> >> + break;
> >> +
> >> + if (modes[i] == soc_pm.data.standby_mode &&
> >> + !(located & STANDBY)) {
> >> + located |= STANDBY;
> >> + continue;
> >> + }
> >> +
> >> + if (modes[i] == soc_pm.data.suspend_mode &&
> >> + !(located & SUSPEND)) {
> >> + located |= SUSPEND;
> >> + continue;
> >> + }
> >> + }
> >> +
> >> + if (!(located & STANDBY)) {
> >> + if (soc_pm.data.suspend_mode == AT91_PM_STANDBY)
> >> + mode = AT91_PM_ULP0;
> >> + else
> >> + mode = AT91_PM_STANDBY;
> >> +
> >> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
> >> + pm_modes[soc_pm.data.standby_mode].pattern,
> >> + pm_modes[mode].pattern);
> >> + soc_pm.data.standby_mode = mode;
> >> + }
> >> +
> >> + if (!(located & SUSPEND)) {
> >> + if (soc_pm.data.standby_mode == AT91_PM_ULP0)
> >> + mode = AT91_PM_STANDBY;
> >> + else
> >> + mode = AT91_PM_ULP0;
> >> +
> >> + pr_warn("AT91: PM: %s mode not supported! Using %s.\n",
> >> + pm_modes[soc_pm.data.suspend_mode].pattern,
> >> + pm_modes[mode].pattern);
> >> + soc_pm.data.suspend_mode = mode;
> >> + }
> >> +
> >> +#undef STANDBY
> >> +#undef SUSPEND
> >
> > You should use two booleans instead of a bit array, this would not make
> > the code longer and will avoid this #def/#undef thing.
>
> OK, I have no strong opinion on this. I'll switch to booleans.
>
> >
> >> +}
> >> +
> >> static void __init at91_pm_init(void (*pm_idle)(void))
> >> {
> >> struct device_node *pmc_np;
> >> @@ -821,11 +874,12 @@ static void __init at91_pm_init(void (*pm_idle)(void))
> >> }
> >> }
> >>
> >> -void __init at91rm9200_pm_init(void)
> >> +void __init at91rm9200_pm_init(const int *modes, int len)
> >> {
> >> if (!IS_ENABLED(CONFIG_SOC_AT91RM9200))
> >> return;
> >>
> >> + at91_pm_modes_validate(modes, len);
> >> at91_dt_ramc();
> >>
> >> /*
> >> @@ -836,11 +890,12 @@ void __init at91rm9200_pm_init(void)
> >> at91_pm_init(at91rm9200_idle);
> >> }
> >>
> >> -void __init sam9x60_pm_init(void)
> >> +void __init sam9x60_pm_init(const int *modes, int len)
> >> {
> >> if (!IS_ENABLED(CONFIG_SOC_SAM9X60))
> >> return;
> >>
> >> + at91_pm_modes_validate(modes, len);
> >> at91_pm_modes_init();
> >> at91_dt_ramc();
> >> at91_pm_init(at91sam9x60_idle);
> >> @@ -849,31 +904,33 @@ void __init sam9x60_pm_init(void)
> >> soc_pm.config_pmc_ws = at91_sam9x60_config_pmc_ws;
> >> }
> >>
> >> -void __init at91sam9_pm_init(void)
> >> +void __init at91sam9_pm_init(const int *modes, int len)
> >> {
> >> if (!IS_ENABLED(CONFIG_SOC_AT91SAM9))
> >> return;
> >>
> >> + at91_pm_modes_validate(modes, len);
> >> at91_dt_ramc();
> >> at91_pm_init(at91sam9_idle);
> >> }
> >>
> >> -void __init sama5_pm_init(void)
> >> +void __init sama5_pm_init(const int *modes, int len)
> >> {
> >> if (!IS_ENABLED(CONFIG_SOC_SAMA5))
> >> return;
> >>
> >> + at91_pm_modes_validate(modes, len);
> >> at91_dt_ramc();
> >> at91_pm_init(NULL);
> >> }
> >>
> >> -void __init sama5d2_pm_init(void)
> >> +void __init sama5d2_pm_init(const int *modes, int len)
> >> {
> >> if (!IS_ENABLED(CONFIG_SOC_SAMA5D2))
> >> return;
> >>
> >> + sama5_pm_init(modes, len);
> >> at91_pm_modes_init();
> >> - sama5_pm_init();
> >>
> >> soc_pm.ws_ids = sama5d2_ws_ids;
> >> soc_pm.config_shdwc_ws = at91_sama5d2_config_shdwc_ws;
> >> diff --git a/arch/arm/mach-at91/sam9x60.c b/arch/arm/mach-at91/sam9x60.c
> >> index d8c739d25458..d7c869c7b542 100644
> >> --- a/arch/arm/mach-at91/sam9x60.c
> >> +++ b/arch/arm/mach-at91/sam9x60.c
> >> @@ -14,12 +14,21 @@
> >> #include <asm/system_misc.h>
> >>
> >> #include "generic.h"
> >> +#include "pm.h"
> >> +
> >> +/* SAM9X60 valid PM modes. */
> >> +static const int sam9x60_pm_modes[] = {
> >> + AT91_PM_STANDBY,
> >> + AT91_PM_ULP0,
> >> + AT91_PM_ULP0_FAST,
> >> + AT91_PM_ULP1,
> >> +};
> >>
> >> static void __init sam9x60_init(void)
> >> {
> >> of_platform_default_populate(NULL, NULL, NULL);
> >>
> >> - sam9x60_pm_init();
> >> + sam9x60_pm_init(sam9x60_pm_modes, ARRAY_SIZE(sam9x60_pm_modes));
> >> }
> >>
> >> static const char *const sam9x60_dt_board_compat[] __initconst = {
> >> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
> >> index 89dab7cf01e8..7eb124612a10 100644
> >> --- a/arch/arm/mach-at91/sama5.c
> >> +++ b/arch/arm/mach-at91/sama5.c
> >> @@ -14,11 +14,19 @@
> >> #include <asm/system_misc.h>
> >>
> >> #include "generic.h"
> >> +#include "pm.h"
> >> +
> >> +/* SAMA5 valid PM modes. */
> >> +static const int sama5_pm_modes[] = {
> >> + AT91_PM_STANDBY,
> >> + AT91_PM_ULP0,
> >> + AT91_PM_ULP0_FAST,
> >> +};
> >>
> >> static void __init sama5_dt_device_init(void)
> >> {
> >> of_platform_default_populate(NULL, NULL, NULL);
> >> - sama5_pm_init();
> >> + sama5_pm_init(sama5_pm_modes, ARRAY_SIZE(sama5_pm_modes));
> >> }
> >>
> >> static const char *const sama5_dt_board_compat[] __initconst = {
> >> @@ -44,10 +52,19 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
> >> .l2c_aux_mask = ~0UL,
> >> MACHINE_END
> >>
> >> +/* sama5d2 PM modes. */
> >> +static const int sama5d2_pm_modes[] = {
> >> + AT91_PM_STANDBY,
> >> + AT91_PM_ULP0,
> >> + AT91_PM_ULP0_FAST,
> >> + AT91_PM_ULP1,
> >> + AT91_PM_BACKUP,
> >> +};
> >> +
> >> static void __init sama5d2_init(void)
> >> {
> >> of_platform_default_populate(NULL, NULL, NULL);
> >> - sama5d2_pm_init();
> >> + sama5d2_pm_init(sama5d2_pm_modes, ARRAY_SIZE(sama5d2_pm_modes));
> >> }
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-08-03 12:53 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-03 7:25 [PATCH 0/3] AT91 PM improvements Claudiu Beznea
2020-08-03 7:25 ` Claudiu Beznea
2020-08-03 7:25 ` [PATCH 1/3] ARM: at91: pm: add support for ulp0 fast mode Claudiu Beznea
2020-08-03 7:25 ` Claudiu Beznea
2020-08-03 7:25 ` [PATCH 2/3] ARM: at91: pm: add per soc validation of pm modes Claudiu Beznea
2020-08-03 7:25 ` Claudiu Beznea
2020-08-03 8:34 ` Alexandre Belloni
2020-08-03 8:34 ` Alexandre Belloni
2020-08-03 10:54 ` Claudiu.Beznea
2020-08-03 10:54 ` Claudiu.Beznea
2020-08-03 12:51 ` Alexandre Belloni
2020-08-03 12:51 ` Alexandre Belloni
2020-08-03 7:25 ` [PATCH 3/3] ARM: at91: pm: put node after its usage Claudiu Beznea
2020-08-03 7:25 ` Claudiu Beznea
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.