ARM: OMAP4: core retention support

From: Rajendra Nayak <[email protected]>

On OMAP4 most modules/hwmods support module level context status. On OMAP3 and earlier, we relyed on the power domain level context status. Identify all such modules using a 'HWMOD_CONTEXT_REG' flag, all such hwmods already have a valid 'context_offs' populated in .prcm structure.

Hi Tero,

Quoted 2 lines · Tero Kristo

From: Rajendra Nayak <[email protected]>

Quoted 4 lines

On OMAP4 most modules/hwmods support module level context status. On OMAP3 and earlier, we relyed on the power domain level context status. Identify all such modules using a 'HWMOD_CONTEXT_REG' flag, all such hwmods already have a valid 'context_offs' populated in .prcm structure.

Is it necessary to add another flag? Can't we just check if context_offs is non-zero? Would save adding a lot more lines to an already large file :-)

Quoted 10 lines · Jon Hunter

Hi Tero,

Quoted 6 lines · Tero Kristo

From: Rajendra Nayak<[email protected]>

On OMAP4 most modules/hwmods support module level context status. On OMAP3 and earlier, we relyed on the power domain level context status. Identify all such modules using a 'HWMOD_CONTEXT_REG' flag, all such hwmods already have a valid 'context_offs' populated in .prcm structure.

Quoted 3 lines

Is it necessary to add another flag? Can't we just check if context_offs is non-zero? Would save adding a lot more lines to an already large file :-)

Actually one of the older versions of this patch was just checking against a non-zero value, but it was decided to be changed as potentially the context_offs can be zero even if it is a valid offset.

Quoted 11 lines · Tero Kristo

Quoted 10 lines · Jon Hunter

Hi Tero,

Quoted 6 lines · Tero Kristo

From: Rajendra Nayak<[email protected]>

On OMAP4 most modules/hwmods support module level context status. On OMAP3 and earlier, we relyed on the power domain level context status. Identify all such modules using a 'HWMOD_CONTEXT_REG' flag, all such hwmods already have a valid 'context_offs' populated in .prcm structure.

Quoted 7 lines

Quoted 3 lines

Is it necessary to add another flag? Can't we just check if context_offs is non-zero? Would save adding a lot more lines to an already large file :-)

Actually one of the older versions of this patch was just checking against a non-zero value, but it was decided to be changed as potentially the context_offs can be zero even if it is a valid offset.

Yeah, but still, every OMAP4 IPs are supporting that except two of them I guess, so it is a pity to add that to every IPs.

We'd better add a HWMOD_NO_CONTEXT_REG to the few IPs that are not supporting that. Since OMAP 2 & 3 does not have this feature at all, we can check on the cpu revision.

I think the issue raised by Rajendra was about AM35xx that looks like an OMAP3 variant but does have these registers like an OMAP4 variant:-(

Quoted 16 lines · Benoit

Quoted 15 lines · Tero Kristo

Quoted 13 lines · Jon Hunter

Hi Tero,

Quoted 6 lines · Tero Kristo

From: Rajendra Nayak<[email protected]>

On OMAP4 most modules/hwmods support module level context status. On OMAP3 and earlier, we relyed on the power domain level context status. Identify all such modules using a 'HWMOD_CONTEXT_REG' flag, all such hwmods already have a valid 'context_offs' populated in .prcm structure.

Is it necessary to add another flag? Can't we just check if context_offs is non-zero? Would save adding a lot more lines to an already large file :-)

Quoted 3 lines

Quoted 3 lines

Actually one of the older versions of this patch was just checking against a non-zero value, but it was decided to be changed as potentially the context_offs can be zero even if it is a valid offset.

Potentially? Is that the case on OMAP4/5 today? I don't see any for OMAP4 in mainline.

If zero really is a valid offset somewhere (where?), then we could use -1 (or USHRT_MAX in this case.)

Quoted 11 lines

Yeah, but still, every OMAP4 IPs are supporting that except two of them I guess, so it is a pity to add that to every IPs.

We'd better add a HWMOD_NO_CONTEXT_REG to the few IPs that are not supporting that. Since OMAP 2& 3 does not have this feature at all, we can check on the cpu revision.

I think the issue raised by Rajendra was about AM35xx that looks like an OMAP3 variant but does have these registers like an OMAP4 variant:-(

If AM335x is missing it for *all* IPs, that's easy enough to solve without bloating the data file: just set .context_offs field (or flag) to the magic value for all IPs during hwmod registration.

Paul/Benoit should make the call whether to use a special value in the .context_offs field (0 or -1) or add a new flag. If a flag is chosen, I agree with Benoit that it should indicate the *lack* of the feature, since having the feature is the norm.

Kevin

Quoted 22 lines · Kevin Hilman

Quoted 20 lines · Benoit

Quoted 19 lines · Tero Kristo

Quoted 17 lines · Jon Hunter

Hi Tero,

Quoted 8 lines · Tero Kristo

From: Rajendra Nayak<[email protected]>

On OMAP4 most modules/hwmods support module level context status. On OMAP3 and earlier, we relyed on the power domain level context status.
Identify all such modules using a 'HWMOD_CONTEXT_REG' flag, all such hwmods already have a valid 'context_offs' populated in .prcm structure.

Is it necessary to add another flag? Can't we just check if context_offs
is non-zero? Would save adding a lot more lines to an already large file
:-)

Quoted 6 lines

Quoted 3 lines

Quoted 3 lines

Actually one of the older versions of this patch was just checking against a non-zero value, but it was decided to be changed as potentially the context_offs can be zero even if it is a valid offset.

Potentially? Is that the case on OMAP4/5 today? I don't see any for OMAP4 in mainline.

No, we don;t have any such cases today in either OMAP4 or OMAP5.

Quoted 3 lines

If zero really is a valid offset somewhere (where?), then we could use -1 (or USHRT_MAX in this case.)

This makes sense for OMAP4 and beyond (and same with having a flag to indicate the *lack* of having the feature) as it will mean just adding a few entries in hwmod data files to indicate IP blocks (very few) which do not support this feature.

However since none of OMAP2/3 varients (except I guess the AMxxxx family) support this, it would also mean we mark
*most* blocks in OMAP2/3 to indicate they *lack* this feature, which would mean bloating the OMAP2/3 data files, but your comment below about doing it for all IPs during hwmod registration makes sense at least for OMAP2 since *all* blocks can be marked at registration. OMAP3 would probably need more data files to be updated to indicate which ones support and which ones don't.

Having said that I also see 'context_reg' being defined inside omap_hwmod_omap4_prcm would need to be fixed if we have to support this for SoCs which fall as OMAP3 varients.

Quoted 24 lines

Quoted 5 lines

Yeah, but still, every OMAP4 IPs are supporting that except two of them I guess, so it is a pity to add that to every IPs.

Quoted 7 lines

We'd better add a HWMOD_NO_CONTEXT_REG to the few IPs that are not supporting that. Since OMAP 2& 3 does not have this feature at all, we can check on the cpu revision.

I think the issue raised by Rajendra was about AM35xx that looks like an OMAP3 variant but does have these registers like an OMAP4 variant:-(

If AM335x is missing it for *all* IPs, that's easy enough to solve without bloating the data file: just set .context_offs field (or flag) to the magic value for all IPs during hwmod registration.

Paul/Benoit should make the call whether to use a special value in the .context_offs field (0 or -1) or add a new flag. If a flag is chosen, I agree with Benoit that it should indicate the *lack* of the feature, since having the feature is the norm.

Kevin

Quoted 50 lines · Rajendra Nayak

Quoted 22 lines · Kevin Hilman

Quoted 20 lines · Benoit

Quoted 19 lines · Tero Kristo

Quoted 17 lines · Jon Hunter

Hi Tero,

Quoted 8 lines · Tero Kristo

From: Rajendra Nayak<[email protected]>

On OMAP4 most modules/hwmods support module level context status. On OMAP3 and earlier, we relyed on the power domain level context status.
Identify all such modules using a 'HWMOD_CONTEXT_REG' flag, all such hwmods already have a valid 'context_offs' populated in .prcm structure.

Is it necessary to add another flag? Can't we just check if context_offs
is non-zero? Would save adding a lot more lines to an already large file
:-)

Quoted 6 lines

Quoted 3 lines

Quoted 3 lines

Actually one of the older versions of this patch was just checking against a non-zero value, but it was decided to be changed as potentially the context_offs can be zero even if it is a valid offset.

Potentially? Is that the case on OMAP4/5 today? I don't see any for OMAP4 in mainline.

No, we don;t have any such cases today in either OMAP4 or OMAP5.

Quoted 3 lines

If zero really is a valid offset somewhere (where?), then we could use -1 (or USHRT_MAX in this case.)

This makes sense for OMAP4 and beyond (and same with having a flag to indicate the *lack* of having the feature) as it will mean just adding a few entries in hwmod data files to indicate IP blocks (very few) which do not support this feature.

However since none of OMAP2/3 varients (except I guess the AMxxxx family) support this, it would also mean we mark
*most* blocks in OMAP2/3 to indicate they *lack* this feature, which would mean bloating the OMAP2/3 data files, but your comment below about doing it for all IPs during hwmod registration makes sense at least for OMAP2 since *all* blocks can be marked at registration. OMAP3 would probably need more data files to be updated to indicate which ones support and which ones don't.

Quoted 3 lines

Having said that I also see 'context_reg' being defined inside omap_hwmod_omap4_prcm would need to be fixed if we have to support this for SoCs which fall as OMAP3 varients.

I just went back and looked at Vaibhavs patch which adds am33xx hwmod data and I think none of what I said above is a problem. I think we can safely mark the few blocks on OMAP4 which do not have a valid context_reg with -1 or USHRT_MAX as you suggested and mark all OMAP2/3 blocks with this at registration.

Benoit/Paul, does that sound good?

Quoted 26 lines

Quoted 25 lines

Quoted 6 lines

Yeah, but still, every OMAP4 IPs are supporting that except two of them I guess, so it is a pity to add that to every IPs.

Quoted 7 lines

We'd better add a HWMOD_NO_CONTEXT_REG to the few IPs that are not supporting that. Since OMAP 2& 3 does not have this feature at all, we can check on the cpu revision.

I think the issue raised by Rajendra was about AM35xx that looks like an OMAP3 variant but does have these registers like an OMAP4 variant:-(

If AM335x is missing it for *all* IPs, that's easy enough to solve without bloating the data file: just set .context_offs field (or flag) to the magic value for all IPs during hwmod registration.

Paul/Benoit should make the call whether to use a special value in the .context_offs field (0 or -1) or add a new flag. If a flag is chosen, I agree with Benoit that it should indicate the *lack* of the feature, since having the feature is the norm.

Kevin

Quoted 61 lines · Rajendra Nayak

Quoted 50 lines · Rajendra Nayak

Quoted 22 lines · Kevin Hilman

Quoted 20 lines · Benoit

Quoted 19 lines · Tero Kristo

Quoted 17 lines · Jon Hunter

Hi Tero,

Quoted 8 lines · Tero Kristo

From: Rajendra Nayak<[email protected]>

On OMAP4 most modules/hwmods support module level context status. On OMAP3 and earlier, we relyed on the power domain level context status.
Identify all such modules using a 'HWMOD_CONTEXT_REG' flag, all such hwmods already have a valid 'context_offs' populated in .prcm structure.

Is it necessary to add another flag? Can't we just check if context_offs
is non-zero? Would save adding a lot more lines to an already large file
:-)

Quoted 6 lines

Quoted 3 lines

Quoted 3 lines

Actually one of the older versions of this patch was just checking against a non-zero value, but it was decided to be changed as potentially the context_offs can be zero even if it is a valid offset.

Potentially? Is that the case on OMAP4/5 today? I don't see any for OMAP4 in mainline.

No, we don;t have any such cases today in either OMAP4 or OMAP5.

Quoted 3 lines

If zero really is a valid offset somewhere (where?), then we could use -1 (or USHRT_MAX in this case.)

This makes sense for OMAP4 and beyond (and same with having a flag to indicate the *lack* of having the feature) as it will mean just adding a few entries in hwmod data files to indicate IP blocks (very few) which do not support this feature.

However since none of OMAP2/3 varients (except I guess the AMxxxx family) support this, it would also mean we mark
*most* blocks in OMAP2/3 to indicate they *lack* this feature, which would mean bloating the OMAP2/3 data files, but your comment below about doing it for all IPs during hwmod registration makes sense at least for OMAP2 since *all* blocks can be marked at registration. OMAP3 would probably need more data files to be updated to indicate which ones support and which ones don't.

Quoted 3 lines

Having said that I also see 'context_reg' being defined inside omap_hwmod_omap4_prcm would need to be fixed if we have to support this for SoCs which fall as OMAP3 varients.

I just went back and looked at Vaibhavs patch which adds am33xx hwmod data and I think none of what I said above is a problem. I think we can safely mark the few blocks on OMAP4 which do not have a valid context_reg with -1 or USHRT_MAX as you suggested and mark all OMAP2/3 blocks with this at registration.

Quoted 1 line

Benoit/Paul, does that sound good?

Any comments to this? This is blocking v6 for this set.

Also, who is going to generate the hwmod data?

From: Axel Haslam <[email protected]>

On OMAP4, there is no support to read previous logic state or previous memory state achieved when a power domain transitions to RET. Instead there are module level context registers.

In order to support the powerdomain level logic/mem_off_counters on OMAP4, instead use the previous power state achieved (RET) and the *programmed* logic/mem RET state to derive if a powerdomain lost logic or did not.

If the powerdomain is programmed to enter RET state and lose logic in RET state, knowing that the powerdomain entered RET is good enough to derive that the logic was lost as well, in such cases.

Hi Tero,

Quoted 6 lines · Tero

From: Axel Haslam <[email protected]>

On OMAP4, there is no support to read previous logic state or previous memory state achieved when a power domain transitions to RET. Instead there are module level context registers.

Quoted 9 lines

In order to support the powerdomain level logic/mem_off_counters on OMAP4, instead use the previous power state achieved (RET) and the *programmed* logic/mem RET state to derive if a powerdomain lost logic or did not.

If the powerdomain is programmed to enter RET state and lose logic in RET state, knowing that the powerdomain entered RET is good enough to derive that the logic was lost as well, in such cases.

Unfortunately this won't scale for AM33xx devices :(
It neither has module level context registers nor previous logic/memory state registers in PRCM.

At a top level, there's a Cortex-M3 (CM3) to assist the low power state transitions and the indication of successful transition to a low power states is handled as part of the IPC mechanism between the MPU (A8) and CM3 which is s/w defined.

Since the various APIs like omap_hwmod_get_context_loss_context() and friends are necessary to have correct context saves and restores in drivers we'll need to add another API for AM33xx which basically builds on the IPC mechanism and updates the counters.

So, instead of the fallback mechanism that is currently in place, can the implementation for updating the logic/mem counters be converted to make use of function pointers. When AM33xx PM support comes in, we can just define a custom function and not pollute the code with cpu_is_* checks.

Quoted 32 lines · Vaibhav

Hi Tero,

Quoted 6 lines · Tero

From: Axel Haslam <[email protected]>

On OMAP4, there is no support to read previous logic state or previous memory state achieved when a power domain transitions to RET. Instead there are module level context registers.

Quoted 9 lines

In order to support the powerdomain level logic/mem_off_counters on OMAP4, instead use the previous power state achieved (RET) and the *programmed* logic/mem RET state to derive if a powerdomain lost logic or did not.

If the powerdomain is programmed to enter RET state and lose logic in RET state, knowing that the powerdomain entered RET is good enough to derive that the logic was lost as well, in such cases.

Unfortunately this won't scale for AM33xx devices :(
It neither has module level context registers nor previous logic/memory state registers in PRCM.

At a top level, there's a Cortex-M3 (CM3) to assist the low power state transitions and the indication of successful transition to a low power states is handled as part of the IPC mechanism between the MPU (A8) and CM3 which is s/w defined.

Since the various APIs like omap_hwmod_get_context_loss_context() and friends are necessary to have correct context saves and restores in drivers we'll need to add another API for AM33xx which basically builds on the IPC mechanism and updates the counters.

Quoted 4 lines

So, instead of the fallback mechanism that is currently in place, can the implementation for updating the logic/mem counters be converted to make use of function pointers. When AM33xx PM support comes in, we can just define a custom function and not pollute the code with cpu_is_* checks.

I think that should be fine. We can add arch_hwmod struct or such (similar to arch_clkdm / arch_pwrdm) if need be. However, I think we should just expand the support towards that way once we can implement the am33xx support properly with a separate patch. I guess this doesn't break anything on am33xx currently, right?

From: Axel Haslam <[email protected]>

On OMAP4, there is no support to read previous logic state or previous memory state achieved when a power domain transitions to RET. Instead there are module level context registers.

In order to support the powerdomain level logic/mem_off_counters on OMAP4, instead use the previous power state achieved (RET) and the *programmed* logic/mem RET state to derive if a powerdomain lost logic or did not.

If the powerdomain is programmed to enter RET state and lose logic in RET state, knowing that the powerdomain entered RET is good enough to derive that the logic was lost as well, in such cases.

Signed-off-by Axel Haslam <[email protected]>

[[email protected]: Updated changelog, updated patch to support mem counters]

Signed-off-by Rajendra Nayak <[email protected]>

Signed-off-by Tero Kristo <[email protected]>

---
 arch/arm/mach-omap2/powerdomain.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

arch/arm/mach-omap2/powerdomain.c +10 -6

index 96ad3dbe..068f06b 100644
@@ -124,18 +124,22 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
 static void _update_logic_membank_counters(struct powerdomain *pwrdm)
 {
 	int i;
-	u8 prev_logic_pwrst, prev_mem_pwrst;
+	int logic_pwrst, mem_pwrst;
+
+	logic_pwrst = pwrdm_read_prev_logic_pwrst(pwrdm);
+	if (logic_pwrst == -EINVAL)
+		logic_pwrst = pwrdm_read_logic_retst(pwrdm);
 
-	prev_logic_pwrst = pwrdm_read_prev_logic_pwrst(pwrdm);
 	if ((pwrdm->pwrsts_logic_ret == PWRSTS_OFF_RET) &&
-	    (prev_logic_pwrst == PWRDM_POWER_OFF))
+	    (logic_pwrst == PWRDM_POWER_OFF))
 		pwrdm->ret_logic_off_counter++;
 
 	for (i = 0; i < pwrdm->banks; i++) {
-		prev_mem_pwrst = pwrdm_read_prev_mem_pwrst(pwrdm, i);
-
+		mem_pwrst = pwrdm_read_prev_mem_pwrst(pwrdm, i);
+		if (mem_pwrst == -EINVAL)
+			mem_pwrst = pwrdm_read_mem_retst(pwrdm, i);
 		if ((pwrdm->pwrsts_mem_ret[i] == PWRSTS_OFF_RET) &&
-		    (prev_mem_pwrst == PWRDM_POWER_OFF))
+		    (mem_pwrst == PWRDM_POWER_OFF))
 			pwrdm->ret_mem_off_counter[i]++;
 	}
 }

Quoted 10 lines · Tero Kristo

From: Axel Haslam <[email protected]>

On OMAP4, there is no support to read previous logic state or previous memory state achieved when a power domain transitions to RET. Instead there are module level context registers.

In order to support the powerdomain level logic/mem_off_counters on OMAP4, instead use the previous power state achieved (RET) and the *programmed* logic/mem RET state to derive if a powerdomain lost logic or did not.

OK, but this also changes the behavior for OMAP3 as well, right? I don't see in the changelog how this affects OMAP3 and whether it is still expected to work on OMAP3. When changing common code like this, the changelog should describe the impacts on to all affected SoCs.

As suggested by Vaibhav Bedia, now might be the right time to add this function to the SoC specific function pointers (struct pwrdm_ops.)

Doing that, the existing function could be used for OMAP3 (and OMAP4 if the changelog describes that it can/should be used for both.)

Then, when AM33xx support is added, it will be obvious where they will need to plugin support for that SoC.

Kevin

Quoted 45 lines

If the powerdomain is programmed to enter RET state and lose logic in RET state, knowing that the powerdomain entered RET is good enough to derive that the logic was lost as well, in such cases.

Signed-off-by Axel Haslam <[email protected]>

[rnayak at ti.com: Updated changelog, updated patch to support mem counters]

Signed-off-by Rajendra Nayak <[email protected]>

Signed-off-by Tero Kristo <[email protected]>

---
 arch/arm/mach-omap2/powerdomain.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

arch/arm/mach-omap2/powerdomain.c +10 -6

index 96ad3dbe..068f06b 100644
@@ -124,18 +124,22 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
 static void _update_logic_membank_counters(struct powerdomain *pwrdm)
 {
 	int i;
-	u8 prev_logic_pwrst, prev_mem_pwrst;
+	int logic_pwrst, mem_pwrst;
+
+	logic_pwrst = pwrdm_read_prev_logic_pwrst(pwrdm);
+	if (logic_pwrst == -EINVAL)
+		logic_pwrst = pwrdm_read_logic_retst(pwrdm);
 
-	prev_logic_pwrst = pwrdm_read_prev_logic_pwrst(pwrdm);
 	if ((pwrdm->pwrsts_logic_ret == PWRSTS_OFF_RET) &&
-	    (prev_logic_pwrst == PWRDM_POWER_OFF))
+	    (logic_pwrst == PWRDM_POWER_OFF))
 		pwrdm->ret_logic_off_counter++;
 
 	for (i = 0; i < pwrdm->banks; i++) {
-		prev_mem_pwrst = pwrdm_read_prev_mem_pwrst(pwrdm, i);
-
+		mem_pwrst = pwrdm_read_prev_mem_pwrst(pwrdm, i);
+		if (mem_pwrst == -EINVAL)
+			mem_pwrst = pwrdm_read_mem_retst(pwrdm, i);
 		if ((pwrdm->pwrsts_mem_ret[i] == PWRSTS_OFF_RET) &&
-		    (prev_mem_pwrst == PWRDM_POWER_OFF))
+		    (mem_pwrst == PWRDM_POWER_OFF))
 			pwrdm->ret_mem_off_counter[i]++;
 	}
 }

Quoted 10 lines · Tero Kristo

From: Axel Haslam <[email protected]>

On OMAP4, there is no support to read previous logic state or previous memory state achieved when a power domain transitions to RET. Instead there are module level context registers.

In order to support the powerdomain level logic/mem_off_counters on OMAP4, instead use the previous power state achieved (RET) and the *programmed* logic/mem RET state to derive if a powerdomain lost logic or did not.

OK, but this also changes the behavior for OMAP3 as well, right? I don't see in the changelog how this affects OMAP3 and whether it is still expected to work on OMAP3. When changing common code like this, the changelog should describe the impacts on to all affected SoCs.

As suggested by Vaibhav Bedia, now might be the right time to add this function to the SoC specific function pointers (struct pwrdm_ops.)

Doing that, the existing function could be used for OMAP3 (and OMAP4 if the changelog describes that it can/should be used for both.)

Then, when AM33xx support is added, it will be obvious where they will need to plugin support for that SoC.

Quoted 18 lines · Kevin Hilman

Quoted 10 lines · Tero Kristo

From: Axel Haslam<[email protected]>

On OMAP4, there is no support to read previous logic state or previous memory state achieved when a power domain transitions to RET. Instead there are module level context registers.

In order to support the powerdomain level logic/mem_off_counters on OMAP4, instead use the previous power state achieved (RET) and the *programmed* logic/mem RET state to derive if a powerdomain lost logic or did not.

OK, but this also changes the behavior for OMAP3 as well, right? I don't see in the changelog how this affects OMAP3 and whether it is still expected to work on OMAP3. When changing common code like this, the changelog should describe the impacts on to all affected SoCs.

Quoted 8 lines

As suggested by Vaibhav Bedia, now might be the right time to add this function to the SoC specific function pointers (struct pwrdm_ops.)

Doing that, the existing function could be used for OMAP3 (and OMAP4 if the changelog describes that it can/should be used for both.)

Then, when AM33xx support is added, it will be obvious where they will need to plugin support for that SoC.

How about the following patch? It will add a couple of redundant read_prev_pwrst calls, but works in the same way as the original patch, without touching the generic code. Also, as there have been talks about adding caching for some of the pwrdm registers (especially the prev_pwrst), this might not be that big of an issue.

If this looks good to you, I'll re-post the set with this patch.

(quoted diff) +0 -1

-Tero

arch/arm/mach-omap2/powerdomain44xx.c +0 -0

index 9bfb8a0..3d5e8d4 100644

arch/arm/mach-omap2/powerdomain44xx.c +2 -0

+}
+
  static int omap4_pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8
 bank)

  {
  	u32 m, v;

arch/arm/mach-omap2/powerdomain44xx.c +8 -0

@@ -183,6 +191,14 @@ static int omap4_pwrdm_read_mem_retst(struct
 powerdomain *pwrdm, u8 bank)
  	return v;
  }
 
+static int omap4_pwrdm_read_prev_mem_pwrst(struct powerdomain *pwrdm,
 u8 bank)
+{
+	if (omap4_pwrdm_read_prev_pwrst(pwrdm) != PWRDM_POWER_RET)
+		return PWRDM_POWER_ON;
+
+	return omap4_pwrdm_read_mem_retst(pwrdm, bank);
+}
+
  static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm)

  {
  	u32 c = 0;

arch/arm/mach-omap2/powerdomain44xx.c +2 -0

@@ -256,9 +272,11 @@ struct pwrdm_ops omap4_pwrdm_operations = {
  	.pwrdm_clear_all_prev_pwrst	= omap4_pwrdm_clear_all_prev_pwrst,
  	.pwrdm_set_logic_retst	= omap4_pwrdm_set_logic_retst,
  	.pwrdm_read_logic_pwrst	= omap4_pwrdm_read_logic_pwrst,
+	.pwrdm_read_prev_logic_pwrst	= omap4_pwrdm_read_prev_logic_pwrst,
  	.pwrdm_read_logic_retst	= omap4_pwrdm_read_logic_retst,
  	.pwrdm_read_mem_pwrst	= omap4_pwrdm_read_mem_pwrst,
  	.pwrdm_read_mem_retst	= omap4_pwrdm_read_mem_retst,
+	.pwrdm_read_prev_mem_pwrst	= omap4_pwrdm_read_prev_mem_pwrst,
  	.pwrdm_set_mem_onst	= omap4_pwrdm_set_mem_onst,
  	.pwrdm_set_mem_retst	= omap4_pwrdm_set_mem_retst,
  	.pwrdm_wait_transition	= omap4_pwrdm_wait_transition,

Quoted 49 lines · Tero Kristo

Quoted 18 lines · Kevin Hilman

Quoted 10 lines · Tero Kristo

From: Axel Haslam<[email protected]>

On OMAP4, there is no support to read previous logic state or previous memory state achieved when a power domain transitions to RET. Instead there are module level context registers.

In order to support the powerdomain level logic/mem_off_counters on OMAP4, instead use the previous power state achieved (RET) and the *programmed* logic/mem RET state to derive if a powerdomain lost logic or did not.

OK, but this also changes the behavior for OMAP3 as well, right? I don't see in the changelog how this affects OMAP3 and whether it is still expected to work on OMAP3. When changing common code like this, the changelog should describe the impacts on to all affected SoCs.

Quoted 8 lines

As suggested by Vaibhav Bedia, now might be the right time to add this function to the SoC specific function pointers (struct pwrdm_ops.)

Doing that, the existing function could be used for OMAP3 (and OMAP4 if the changelog describes that it can/should be used for both.)

Then, when AM33xx support is added, it will be obvious where they will need to plugin support for that SoC.

How about the following patch? It will add a couple of redundant read_prev_pwrst calls, but works in the same way as the original patch, without touching the generic code. Also, as there have been talks about adding caching for some of the pwrdm registers (especially the prev_pwrst), this might not be that big of an issue.

If this looks good to you, I'll re-post the set with this patch.

(quoted diff) +0 -1

-Tero

arch/arm/mach-omap2/powerdomain44xx.c +0 -0

index 9bfb8a0..3d5e8d4 100644
@@ -155,6 +155,14 @@ static int omap4_pwrdm_read_logic_retst(struct
 powerdomain *pwrdm)
  	return v;
  }
 

arch/arm/mach-omap2/powerdomain44xx.c static int omap4_pwrdm_read_logic_retst(struct · line 164

Quoted 6 lines

arch/arm/mach-omap2/powerdomain44xx.c +6 -0

+static int omap4_pwrdm_read_prev_logic_pwrst(struct powerdomain *pwrdm)
+{
+	if (omap4_pwrdm_read_prev_pwrst(pwrdm) != PWRDM_POWER_RET)
+		return PWRDM_POWER_ON;
+
+	return omap4_pwrdm_read_logic_retst(pwrdm);

Looks good to me. Do these ever get called with target state programmed to OFF?

regards,
Rajendra

Quoted 37 lines

arch/arm/mach-omap2/powerdomain44xx.c +2 -0

+}
+
  static int omap4_pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8
 bank)

  {
  	u32 m, v;

arch/arm/mach-omap2/powerdomain44xx.c +8 -0

@@ -183,6 +191,14 @@ static int omap4_pwrdm_read_mem_retst(struct
 powerdomain *pwrdm, u8 bank)
  	return v;
  }
 
+static int omap4_pwrdm_read_prev_mem_pwrst(struct powerdomain *pwrdm,
 u8 bank)
+{
+	if (omap4_pwrdm_read_prev_pwrst(pwrdm) != PWRDM_POWER_RET)
+		return PWRDM_POWER_ON;
+
+	return omap4_pwrdm_read_mem_retst(pwrdm, bank);
+}
+
  static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm)

  {
  	u32 c = 0;

arch/arm/mach-omap2/powerdomain44xx.c +2 -0

@@ -256,9 +272,11 @@ struct pwrdm_ops omap4_pwrdm_operations = {
  	.pwrdm_clear_all_prev_pwrst	= omap4_pwrdm_clear_all_prev_pwrst,
  	.pwrdm_set_logic_retst	= omap4_pwrdm_set_logic_retst,
  	.pwrdm_read_logic_pwrst	= omap4_pwrdm_read_logic_pwrst,
+	.pwrdm_read_prev_logic_pwrst	= omap4_pwrdm_read_prev_logic_pwrst,
  	.pwrdm_read_logic_retst	= omap4_pwrdm_read_logic_retst,
  	.pwrdm_read_mem_pwrst	= omap4_pwrdm_read_mem_pwrst,
  	.pwrdm_read_mem_retst	= omap4_pwrdm_read_mem_retst,
+	.pwrdm_read_prev_mem_pwrst	= omap4_pwrdm_read_prev_mem_pwrst,
  	.pwrdm_set_mem_onst	= omap4_pwrdm_set_mem_onst,
  	.pwrdm_set_mem_retst	= omap4_pwrdm_set_mem_retst,
  	.pwrdm_wait_transition	= omap4_pwrdm_wait_transition,

Quoted 50 lines · Rajendra Nayak

Quoted 49 lines · Tero Kristo

Quoted 18 lines · Kevin Hilman

Quoted 10 lines · Tero Kristo

From: Axel Haslam<[email protected]>

On OMAP4, there is no support to read previous logic state or previous memory state achieved when a power domain transitions to RET. Instead there are module level context registers.

In order to support the powerdomain level logic/mem_off_counters on OMAP4, instead use the previous power state achieved (RET) and the *programmed* logic/mem RET state to derive if a powerdomain lost logic or did not.

OK, but this also changes the behavior for OMAP3 as well, right? I don't see in the changelog how this affects OMAP3 and whether it is still expected to work on OMAP3. When changing common code like this, the changelog should describe the impacts on to all affected SoCs.

Quoted 8 lines

As suggested by Vaibhav Bedia, now might be the right time to add this function to the SoC specific function pointers (struct pwrdm_ops.)

Doing that, the existing function could be used for OMAP3 (and OMAP4 if the changelog describes that it can/should be used for both.)

Then, when AM33xx support is added, it will be obvious where they will need to plugin support for that SoC.

How about the following patch? It will add a couple of redundant read_prev_pwrst calls, but works in the same way as the original patch, without touching the generic code. Also, as there have been talks about adding caching for some of the pwrdm registers (especially the prev_pwrst), this might not be that big of an issue.

If this looks good to you, I'll re-post the set with this patch.

(quoted diff) +0 -1

-Tero

arch/arm/mach-omap2/powerdomain44xx.c +0 -0

index 9bfb8a0..3d5e8d4 100644
@@ -155,6 +155,14 @@ static int omap4_pwrdm_read_logic_retst(struct
 powerdomain *pwrdm)
   	return v;
   }
 

Quoted 9 lines

arch/arm/mach-omap2/powerdomain44xx.c static int omap4_pwrdm_read_logic_retst(struct · line 164

Quoted 6 lines

arch/arm/mach-omap2/powerdomain44xx.c +6 -0

+static int omap4_pwrdm_read_prev_logic_pwrst(struct powerdomain *pwrdm)
+{
+	if (omap4_pwrdm_read_prev_pwrst(pwrdm) != PWRDM_POWER_RET)
+		return PWRDM_POWER_ON;
+
+	return omap4_pwrdm_read_logic_retst(pwrdm);

Looks good to me. Do these ever get called with target state programmed to OFF?

At least with current kernel code, no. But you are right, it might be better to change these to check against> PWRDM_POWER_RET.

-Tero

Quoted 42 lines

regards,
Rajendra

Quoted 37 lines

arch/arm/mach-omap2/powerdomain44xx.c +2 -0

+}
+
   static int omap4_pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8
 bank)

   {
   	u32 m, v;

arch/arm/mach-omap2/powerdomain44xx.c +8 -0

@@ -183,6 +191,14 @@ static int omap4_pwrdm_read_mem_retst(struct
 powerdomain *pwrdm, u8 bank)
   	return v;
   }
 
+static int omap4_pwrdm_read_prev_mem_pwrst(struct powerdomain *pwrdm,
 u8 bank)
+{
+	if (omap4_pwrdm_read_prev_pwrst(pwrdm) != PWRDM_POWER_RET)
+		return PWRDM_POWER_ON;
+
+	return omap4_pwrdm_read_mem_retst(pwrdm, bank);
+}
+
   static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm)

   {
   	u32 c = 0;

arch/arm/mach-omap2/powerdomain44xx.c +2 -0

@@ -256,9 +272,11 @@ struct pwrdm_ops omap4_pwrdm_operations = {
   	.pwrdm_clear_all_prev_pwrst	= omap4_pwrdm_clear_all_prev_pwrst,
   	.pwrdm_set_logic_retst	= omap4_pwrdm_set_logic_retst,
   	.pwrdm_read_logic_pwrst	= omap4_pwrdm_read_logic_pwrst,
+	.pwrdm_read_prev_logic_pwrst	= omap4_pwrdm_read_prev_logic_pwrst,
   	.pwrdm_read_logic_retst	= omap4_pwrdm_read_logic_retst,
   	.pwrdm_read_mem_pwrst	= omap4_pwrdm_read_mem_pwrst,
   	.pwrdm_read_mem_retst	= omap4_pwrdm_read_mem_retst,
+	.pwrdm_read_prev_mem_pwrst	= omap4_pwrdm_read_prev_mem_pwrst,
   	.pwrdm_set_mem_onst	= omap4_pwrdm_set_mem_onst,
   	.pwrdm_set_mem_retst	= omap4_pwrdm_set_mem_retst,
   	.pwrdm_wait_transition	= omap4_pwrdm_wait_transition,

Quoted 61 lines · Tero Kristo

Quoted 59 lines · Rajendra Nayak

Quoted 45 lines · Tero Kristo

Quoted 18 lines · Kevin Hilman

Quoted 10 lines · Tero Kristo

From: Axel Haslam<[email protected]>

On OMAP4, there is no support to read previous logic state or previous memory state achieved when a power domain transitions to RET. Instead there are module level context registers.

In order to support the powerdomain level logic/mem_off_counters on OMAP4, instead use the previous power state achieved (RET) and the *programmed* logic/mem RET state to derive if a powerdomain lost logic or did not.

OK, but this also changes the behavior for OMAP3 as well, right? I don't see in the changelog how this affects OMAP3 and whether it is still expected to work on OMAP3. When changing common code like this, the changelog should describe the impacts on to all affected SoCs.

Quoted 8 lines

As suggested by Vaibhav Bedia, now might be the right time to add this function to the SoC specific function pointers (struct pwrdm_ops.)

Doing that, the existing function could be used for OMAP3 (and OMAP4 if the changelog describes that it can/should be used for both.)

Then, when AM33xx support is added, it will be obvious where they will need to plugin support for that SoC.

How about the following patch? It will add a couple of redundant read_prev_pwrst calls, but works in the same way as the original patch, without touching the generic code. Also, as there have been talks about adding caching for some of the pwrdm registers (especially the prev_pwrst), this might not be that big of an issue.

If this looks good to you, I'll re-post the set with this patch.

(quoted diff) +0 -1

-Tero

arch/arm/mach-omap2/powerdomain44xx.c +0 -0

index 9bfb8a0..3d5e8d4 100644

arch/arm/mach-omap2/powerdomain44xx.c static int omap4_pwrdm_read_logic_retst(struct · line 164

Quoted 10 lines

arch/arm/mach-omap2/powerdomain44xx.c +6 -0

@@ -155,6 +155,14 @@ static int omap4_pwrdm_read_logic_retst(struct
 powerdomain *pwrdm)
   	return v;
   }
 
+static int omap4_pwrdm_read_prev_logic_pwrst(struct powerdomain *pwrdm)
+{
+	if (omap4_pwrdm_read_prev_pwrst(pwrdm) != PWRDM_POWER_RET)
+		return PWRDM_POWER_ON;
+
+	return omap4_pwrdm_read_logic_retst(pwrdm);

Looks good to me. Do these ever get called with target state programmed to OFF?

Quoted 2 lines

At least with current kernel code, no. But you are right, it might be better to change these to check against> PWRDM_POWER_RET.

But in case of target power state being programmed to OFF, the logic retst might not be programmed at all, so in case of target state being OFF, returning the logic retst programmed might as well return PWRDM_POWER_ON itself, right?

Quoted 48 lines

-Tero

Quoted 42 lines

regards,
Rajendra

Quoted 37 lines

arch/arm/mach-omap2/powerdomain44xx.c +2 -0

+}
+
   static int omap4_pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8
 bank)

   {
   	u32 m, v;

arch/arm/mach-omap2/powerdomain44xx.c +8 -0

@@ -183,6 +191,14 @@ static int omap4_pwrdm_read_mem_retst(struct
 powerdomain *pwrdm, u8 bank)
   	return v;
   }
 
+static int omap4_pwrdm_read_prev_mem_pwrst(struct powerdomain *pwrdm,
 u8 bank)
+{
+	if (omap4_pwrdm_read_prev_pwrst(pwrdm) != PWRDM_POWER_RET)
+		return PWRDM_POWER_ON;
+
+	return omap4_pwrdm_read_mem_retst(pwrdm, bank);
+}
+
   static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm)

   {
   	u32 c = 0;

arch/arm/mach-omap2/powerdomain44xx.c +2 -0

@@ -256,9 +272,11 @@ struct pwrdm_ops omap4_pwrdm_operations = {
   	.pwrdm_clear_all_prev_pwrst	= omap4_pwrdm_clear_all_prev_pwrst,
   	.pwrdm_set_logic_retst	= omap4_pwrdm_set_logic_retst,
   	.pwrdm_read_logic_pwrst	= omap4_pwrdm_read_logic_pwrst,
+	.pwrdm_read_prev_logic_pwrst	= omap4_pwrdm_read_prev_logic_pwrst,
   	.pwrdm_read_logic_retst	= omap4_pwrdm_read_logic_retst,
   	.pwrdm_read_mem_pwrst	= omap4_pwrdm_read_mem_pwrst,
   	.pwrdm_read_mem_retst	= omap4_pwrdm_read_mem_retst,
+	.pwrdm_read_prev_mem_pwrst	= omap4_pwrdm_read_prev_mem_pwrst,
   	.pwrdm_set_mem_onst	= omap4_pwrdm_set_mem_onst,
   	.pwrdm_set_mem_retst	= omap4_pwrdm_set_mem_retst,
   	.pwrdm_wait_transition	= omap4_pwrdm_wait_transition,

PM debug now contains a file that can be used to control OSWR support enable / disable on OMAP4. Also removed the off_mode_enable file for the same platform as it is unsupported.

Signed-off-by Tero Kristo <[email protected]>

---

?arch/arm/mach-omap2/pm-debug.c | ? 20 ++++++++++++++++----
?arch/arm/mach-omap2/pm.h ? ? ? | ? ?1 +
?arch/arm/mach-omap2/pm44xx.c ? | ? 16 ++++++++++++++++

?3 files changed, 33 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/pm-debug.c +0 -0

index 814bcd9..d9a8e42 100644
@@ -39,6 +39,7 @@

?#include "pm.h"

?u32 enable_off_mode;
+static u32 enable_oswr_mode;

?#ifdef CONFIG_DEBUG_FS
?#include <linux/debugfs.h>

arch/arm/mach-omap2/pm-debug.c +0 -0

@@ -247,10 +248,13 @@ static int option_set(void *data, u64 val)
 ? ? ? ? ? ? ? ? ? ? ? ?omap_pm_enable_off_mode();

? ? ? ? ? ? ? ?else
? ? ? ? ? ? ? ? ? ? ? ?omap_pm_disable_off_mode();
- ? ? ? ? ? ? ? if (cpu_is_omap34xx())
- ? ? ? ? ? ? ? ? ? ? ? omap3_pm_off_mode_enable(val); +
+ ? ? ? ? ? ? ? omap3_pm_off_mode_enable(val);

? ? ? ?}

+ ? ? ? if (option == &enable_oswr_mode)
+ ? ? ? ? ? ? ? omap4_pm_oswr_mode_enable(val);
+
? ? ? ?return 0;
?}

arch/arm/mach-omap2/pm-debug.c static int __init pm_dbg_init(void) · line 284

Quoted 10 lines

arch/arm/mach-omap2/pm-debug.c +4 -2

@@ -274,8 +278,16 @@ static int __init pm_dbg_init(void)
 
       pwrdm_for_each(pwrdms_setup, (void *)d);
 
-       (void) debugfs_create_file("enable_off_mode", S_IRUGO | S_IWUSR, d,
-                                  &enable_off_mode, &pm_dbg_option_fops);
+       if (cpu_is_omap34xx())
+               (void) debugfs_create_file("enable_off_mode",
+                       S_IRUGO | S_IWUSR, d, &enable_off_mode,
+                       &pm_dbg_option_fops);

Is the enable_off_mode entry needed on other OMAP platorms (OMAP<3)?