Skip to contentSkip to conversation

ux500 : decouple/recouple gic from the PRCMU

PATCH Superseded Daniel Lezcano 2 participants 5 messages Feb 9, 2012

Subsystems (2)
Multifunction devices (MFD) Arm/nomadik/Ux500 architectures

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Daniel Lezcano

author
#0 lore raw

PATCH v2 1/1 ux500 : decouple/recouple gic from the PRCMU

This patch allows to decouple and recouple the gic from the PRCMU. This is needed to put the A9 core in retention mode with the cpuidle driver.

Signed-off-by Daniel Lezcano <[email protected]>
---
 drivers/mfd/db8500-prcmu.c       |   29 +++++++++++++++++++++++++++++
 include/linux/mfd/db8500-prcmu.h |    2 ++
 include/linux/mfd/dbx500-prcmu.h |   16 ++++++++++++++++
 3 files changed, 47 insertions(+), 0 deletions(-)
drivers/mfd/db8500-prcmu.c +29 -0
index af8e0ef..70f39a1 100644
@@ -787,6 +787,35 @@ int db8500_prcmu_set_power_state(u8 state, bool keep_ulp_clk, bool keep_ap_pll)
 	return 0;
 }
 
+#define PRCMU_A9_MASK_REQ               0x00000328
+#define PRCMU_A9_MASK_REQ_MASK          0x00000001
+#define PRCMU_GIC_DELAY                 1
+
+/* This function decouple the gic from the prcmu */
+void db8500_prcmu_gic_decouple(void)
+{
+	u32 val = readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ);
+
+	/* Set bit 0 register value to 1 */
+	writel((val & ~PRCMU_A9_MASK_REQ_MASK) | PRCMU_A9_MASK_REQ_MASK,
+	       _PRCMU_BASE + PRCMU_A9_MASK_REQ);
+
+	/* Make sure the register is updated */
+	readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ);
+
+	/* Wait a few cycles for the gic mask completion */
+	udelay(PRCMU_GIC_DELAY);
+}
+
+/* This function recouple the gic with the prcmu */
+void db8500_prcmu_gic_recouple(void)
+{
+	u32 val = readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ);
+
+	/* Set bit 0 register value to 0 */
+	writel((val & ~PRCMU_A9_MASK_REQ_MASK), _PRCMU_BASE + PRCMU_A9_MASK_REQ);
+}
+
 /* This function should only be called while mb0_transfer.lock is held. */
 static void config_wakeups(void)
 {
include/linux/mfd/db8500-prcmu.h +2 -0
index 60d27f7..4a1032c 100644
@@ -536,6 +536,8 @@ int prcmu_load_a9wdog(u8 id, u32 val);
 
 void db8500_prcmu_system_reset(u16 reset_code);
 int db8500_prcmu_set_power_state(u8 state, bool keep_ulp_clk, bool keep_ap_pll);
+void db8500_prcmu_gic_disable(void);
+void db8500_prcmu_gic_enable(void);
 void db8500_prcmu_enable_wakeups(u32 wakeups);
 int db8500_prcmu_set_epod(u16 epod_id, u8 epod_state);
 int db8500_prcmu_request_clock(u8 clock, bool enable);
include/linux/mfd/dbx500-prcmu.h +16 -0
index bac942f..a5fee69 100644
@@ -237,6 +237,22 @@ static inline int prcmu_set_power_state(u8 state, bool keep_ulp_clk,
 			keep_ap_pll);
 }
 
+static inline void prcmu_gic_disable(void)
+{
+	if (machine_is_u5500())
+		return;
+	else
+		return db8500_prcmu_gic_disable();
+}
+
+static inline void prcmu_gic_enable(void)
+{
+	if (machine_is_u5500())
+		return;
+	else
+		return db8500_prcmu_gic_enable();
+}
+
 static inline int prcmu_set_epod(u16 epod_id, u8 epod_state)
 {
 	if (machine_is_u5500())
Signature · 1 line
1.7.5.4
#1 lore raw

PATCH v2 1/1 ux500 : decouple/recouple gic from the PRCMU

Hi Daniel!

Please see my embedded comments.

Also should we not keep the functions in pm.c file? I think Linus W advised us not to move stuff around at this stage.

BR
Rickard

Quoted 21 lines

This patch allows to decouple and recouple the gic from the PRCMU. This is needed to put the A9 core in retention mode with the cpuidle driver.

Signed-off-by Daniel Lezcano<[email protected]>
---
  drivers/mfd/db8500-prcmu.c       |   29 +++++++++++++++++++++++++++++
  include/linux/mfd/db8500-prcmu.h |    2 ++
  include/linux/mfd/dbx500-prcmu.h |   16 ++++++++++++++++
  3 files changed, 47 insertions(+), 0 deletions(-)
drivers/mfd/db8500-prcmu.c +2 -0
index af8e0ef..70f39a1 100644
@@ -787,6 +787,35 @@ int db8500_prcmu_set_power_state(u8 state, bool keep_ulp_clk, bool keep_ap_pll)
  	return 0;
  }
 
+#define PRCMU_A9_MASK_REQ               0x00000328
+#define PRCMU_A9_MASK_REQ_MASK          0x00000001

drivers/mfd/db8500-prcmu.c int db8500_prcmu_set_power_state(u8 state, bool keep_ulp_clk, bool keep_ap_pll) · line 801

Quoted 10 lines
drivers/mfd/db8500-prcmu.c +10 -0
+#define PRCMU_GIC_DELAY                 1
+
+/* This function decouple the gic from the prcmu */
+void db8500_prcmu_gic_decouple(void)
+{
+	u32 val = readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ);
+
+	/* Set bit 0 register value to 1 */
+	writel((val&  ~PRCMU_A9_MASK_REQ_MASK) | PRCMU_A9_MASK_REQ_MASK,
+	       _PRCMU_BASE + PRCMU_A9_MASK_REQ);

No need to AND with ~PRCMU_A9_MASK_REQ_MASK. This is enough:

writel((val | PRCMU_A9_MASK_REQ_MASK,

	_PRCMU_BASE + PRCMU_A9_MASK_REQ);
Quoted 38 lines
drivers/mfd/db8500-prcmu.c +17 -0
+
+	/* Make sure the register is updated */
+	readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ);
+
+	/* Wait a few cycles for the gic mask completion */
+	udelay(PRCMU_GIC_DELAY);
+}
+
+/* This function recouple the gic with the prcmu */
+void db8500_prcmu_gic_recouple(void)
+{
+	u32 val = readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ);
+
+	/* Set bit 0 register value to 0 */
+	writel((val&  ~PRCMU_A9_MASK_REQ_MASK), _PRCMU_BASE + PRCMU_A9_MASK_REQ);
+}
+
  /* This function should only be called while mb0_transfer.lock is held. */
  static void config_wakeups(void)
  {
include/linux/mfd/db8500-prcmu.h +2 -0
index 60d27f7..4a1032c 100644
@@ -536,6 +536,8 @@ int prcmu_load_a9wdog(u8 id, u32 val);
 
  void db8500_prcmu_system_reset(u16 reset_code);
  int db8500_prcmu_set_power_state(u8 state, bool keep_ulp_clk, bool keep_ap_pll);
+void db8500_prcmu_gic_disable(void);
+void db8500_prcmu_gic_enable(void);
  void db8500_prcmu_enable_wakeups(u32 wakeups);
  int db8500_prcmu_set_epod(u16 epod_id, u8 epod_state);
  int db8500_prcmu_request_clock(u8 clock, bool enable);
include/linux/mfd/dbx500-prcmu.h +0 -0
index bac942f..a5fee69 100644

include/linux/mfd/dbx500-prcmu.h static inline int prcmu_set_power_state(u8 state, bool keep_ulp_clk, · line 246

Quoted 10 lines
include/linux/mfd/dbx500-prcmu.h +7 -0
@@ -237,6 +237,22 @@ static inline int prcmu_set_power_state(u8 state, bool keep_ulp_clk,
  			keep_ap_pll);
  }
 
+static inline void prcmu_gic_disable(void)
+{
+	if (machine_is_u5500())
+		return;
+	else
+		return db8500_prcmu_gic_disable();
+}

We have change name of this function in the .c file

include/linux/mfd/dbx500-prcmu.h static inline int prcmu_set_power_state(u8 state, bool keep_ulp_clk, · line 254

Quoted 8 lines
include/linux/mfd/dbx500-prcmu.h +8 -0
+
+static inline void prcmu_gic_enable(void)
+{
+	if (machine_is_u5500())
+		return;
+	else
+		return db8500_prcmu_gic_enable();
+}

We have change name of this function in the .c file

Quoted 4 lines
include/linux/mfd/dbx500-prcmu.h +1 -0
+
  static inline int prcmu_set_epod(u16 epod_id, u8 epod_state)
  {
  	if (machine_is_u5500())
author
#2 lore raw

PATCH v2 1/1 ux500 : decouple/recouple gic from the PRCMU

Quoted 6 lines · Rickard Andersson

Hi Daniel!

Please see my embedded comments.

Also should we not keep the functions in pm.c file? I think Linus W advised us not to move stuff around at this stage.

The pm.c file does not exists for mach-ux500. May be I misunderstood Linus but I thought he was saying to that we should "not move stuff around" by letting this code in the db8500-prcmu.c file for the moment and let Mattias do its cleanups.

Quoted 36 lines

BR
Rickard

Quoted 22 lines

This patch allows to decouple and recouple the gic from the PRCMU. This is needed to put the A9 core in retention mode with the cpuidle driver.

Signed-off-by Daniel Lezcano<[email protected]>

---
drivers/mfd/db8500-prcmu.c | 29 +++++++++++++++++++++++++++++ include/linux/mfd/db8500-prcmu.h | 2 ++ include/linux/mfd/dbx500-prcmu.h | 16 ++++++++++++++++

3 files changed, 47 insertions(+), 0 deletions(-)
drivers/mfd/db8500-prcmu.c +2 -0
index af8e0ef..70f39a1 100644
@@ -787,6 +787,35 @@ int db8500_prcmu_set_power_state(u8 state, bool
 keep_ulp_clk, bool keep_ap_pll)
 return 0;
 }
 
+#define PRCMU_A9_MASK_REQ 0x00000328
+#define PRCMU_A9_MASK_REQ_MASK 0x00000001

drivers/mfd/db8500-prcmu.c int db8500_prcmu_set_power_state(u8 state, bool · line 802

Quoted 10 lines
drivers/mfd/db8500-prcmu.c +10 -0
+#define PRCMU_GIC_DELAY 1
+
+/* This function decouple the gic from the prcmu */
+void db8500_prcmu_gic_decouple(void)
+{
+ u32 val = readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ);
+
+ /* Set bit 0 register value to 1 */
+ writel((val& ~PRCMU_A9_MASK_REQ_MASK) | PRCMU_A9_MASK_REQ_MASK,
+ _PRCMU_BASE + PRCMU_A9_MASK_REQ);

No need to AND with ~PRCMU_A9_MASK_REQ_MASK. This is enough:

Quoted 2 lines

writel((val | PRCMU_A9_MASK_REQ_MASK,
_PRCMU_BASE + PRCMU_A9_MASK_REQ);

Yep, right.

[ cut ]

Quoted 8 lines

drivers/mfd/db8500-prcmu.c int db8500_prcmu_set_power_state(u8 state, bool · line 809

Quoted 7 lines
drivers/mfd/db8500-prcmu.c +7 -0
+static inline void prcmu_gic_disable(void)
+{
+ if (machine_is_u5500())
+ return;
+ else
+ return db8500_prcmu_gic_disable();
+}

We have change name of this function in the .c file

Quoted 9 lines

drivers/mfd/db8500-prcmu.c int db8500_prcmu_set_power_state(u8 state, bool · line 817

Quoted 8 lines
drivers/mfd/db8500-prcmu.c +8 -0
+
+static inline void prcmu_gic_enable(void)
+{
+ if (machine_is_u5500())
+ return;
+ else
+ return db8500_prcmu_gic_enable();
+}

We have change name of this function in the .c file

Do you want that in db8500-prcmu.c and declare the function in db8500-prcmu.h or dbx500-prcmu.h ?

Signature · 5 lines
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
#3 lore raw

PATCH v2 1/1 ux500 : decouple/recouple gic from the PRCMU

Quoted 11 lines · Daniel Lezcano
Quoted 6 lines · Rickard Andersson

Hi Daniel!

Please see my embedded comments.

Also should we not keep the functions in pm.c file? I think Linus W advised us not to move stuff around at this stage.

The pm.c file does not exists for mach-ux500. May be I misunderstood Linus but I thought he was saying to that we should "not move stuff around" by letting this code in the db8500-prcmu.c file for the moment and let Mattias do its cleanups.

On our internal tree we have mach-ux500/pm/pm.c and this file contains the decouple-gic functions as well as other power management related help functions. By moving the code to drivers/mfd/db8500-prcmu.c I think we are moving code around. Especially since it not clear how db8500-prcmu.c will be handled in the future, db8500-prcmu.c should most likely be split into several parts where one part only handles the communication with the PRCMU firmware.

Quoted 64 lines
Quoted 36 lines

BR
Rickard

Quoted 22 lines

This patch allows to decouple and recouple the gic from the PRCMU. This is needed to put the A9 core in retention mode with the cpuidle driver.

Signed-off-by Daniel Lezcano<[email protected]>

---
drivers/mfd/db8500-prcmu.c | 29 +++++++++++++++++++++++++++++ include/linux/mfd/db8500-prcmu.h | 2 ++ include/linux/mfd/dbx500-prcmu.h | 16 ++++++++++++++++

3 files changed, 47 insertions(+), 0 deletions(-)
drivers/mfd/db8500-prcmu.c +2 -0
index af8e0ef..70f39a1 100644
@@ -787,6 +787,35 @@ int db8500_prcmu_set_power_state(u8 state, bool
 keep_ulp_clk, bool keep_ap_pll)
 return 0;
 }
 
+#define PRCMU_A9_MASK_REQ 0x00000328
+#define PRCMU_A9_MASK_REQ_MASK 0x00000001

drivers/mfd/db8500-prcmu.c int db8500_prcmu_set_power_state(u8 state, bool · line 802

Quoted 10 lines
drivers/mfd/db8500-prcmu.c +10 -0
+#define PRCMU_GIC_DELAY 1
+
+/* This function decouple the gic from the prcmu */
+void db8500_prcmu_gic_decouple(void)
+{
+ u32 val = readl(_PRCMU_BASE + PRCMU_A9_MASK_REQ);
+
+ /* Set bit 0 register value to 1 */
+ writel((val&  ~PRCMU_A9_MASK_REQ_MASK) | PRCMU_A9_MASK_REQ_MASK,
+ _PRCMU_BASE + PRCMU_A9_MASK_REQ);

No need to AND with ~PRCMU_A9_MASK_REQ_MASK. This is enough:

Quoted 2 lines

writel((val | PRCMU_A9_MASK_REQ_MASK,
_PRCMU_BASE + PRCMU_A9_MASK_REQ);

Yep, right.

[ cut ]

Quoted 8 lines

drivers/mfd/db8500-prcmu.c int db8500_prcmu_set_power_state(u8 state, bool · line 809

Quoted 7 lines
drivers/mfd/db8500-prcmu.c +7 -0
+static inline void prcmu_gic_disable(void)
+{
+ if (machine_is_u5500())
+ return;
+ else
+ return db8500_prcmu_gic_disable();
+}

We have change name of this function in the .c file

Quoted 9 lines

drivers/mfd/db8500-prcmu.c int db8500_prcmu_set_power_state(u8 state, bool · line 817

Quoted 8 lines
drivers/mfd/db8500-prcmu.c +8 -0
+
+static inline void prcmu_gic_enable(void)
+{
+ if (machine_is_u5500())
+ return;
+ else
+ return db8500_prcmu_gic_enable();
+}

We have change name of this function in the .c file

Do you want that in db8500-prcmu.c and declare the function in db8500-prcmu.h or dbx500-prcmu.h ?

author
#4 lore raw

PATCH v2 1/1 ux500 : decouple/recouple gic from the PRCMU

Quoted 12 lines · Rickard Andersson
Quoted 11 lines · Daniel Lezcano
Quoted 6 lines · Rickard Andersson

Hi Daniel!

Please see my embedded comments.

Also should we not keep the functions in pm.c file? I think Linus W advised us not to move stuff around at this stage.

The pm.c file does not exists for mach-ux500. May be I misunderstood Linus but I thought he was saying to that we should "not move stuff around" by letting this code in the db8500-prcmu.c file for the moment and let Mattias do its cleanups.

Quoted 7 lines

On our internal tree we have mach-ux500/pm/pm.c and this file contains the decouple-gic functions as well as other power management related help functions. By moving the code to drivers/mfd/db8500-prcmu.c I think we are moving code around. Especially since it not clear how db8500-prcmu.c will be handled in the future, db8500-prcmu.c should most likely be split into several parts where one part only handles the communication with the PRCMU firmware.

Rickard,

the reference is the mainstream code. Today there is no pm.c file and everything is located in db8500-prcmu.c. Except if I misunderstood, if Linus, who is the maintainer of the ux500 SoC, prefers to have this portion of code in this file until Mattias does its cleanups, that does not really matter if it is in pm.c or db8500-prcmu.c for the moment.

That does not mean this code won't be moved to pm.c in the future.

Thanks
-- Daniel

Signature · 5 lines
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog