[PATCH] DaVinci: only poll EPCPR on DM644x and DM355

[PATCH] DaVinci: only poll EPCPR on DM644x and DM355

[Sergei Shtylyov]

EPCPR register and PDCTL.EPCGOOD bit exist only on DaVinci DM644x and DM35x,
so do not try to poll EPCPR and set PDCTL.EPCGOOD on the other SoCs -- it would
lead to lock up if some power domain hasn't been powered up by this time (which
hasn't happened yet on any board, it seems).

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

---
The patch is against the recent DaVinci tree plus this patch:

http://linux.davincidsp.com/pipermail/davinci-linux-open-source/2011-September/023308.html

Index: linux-davinci/arch/arm/mach-davinci/psc.c
===================================================================
--- linux-davinci.orig/arch/arm/mach-davinci/psc.c
+++ linux-davinci/arch/arm/mach-davinci/psc.c
@@ -88,14 +88,19 @@ void davinci_psc_config(unsigned int dom
 		ptcmd = 1 << domain;
 		__raw_writel(ptcmd, psc_base + PTCMD);
 
-		do {
-			epcpr = __raw_readl(psc_base + EPCPR);
-		} while ((((epcpr >> domain) & 1) == 0));
-
-		pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
-		pdctl |= 0x100;
-		__raw_writel(pdctl, psc_base + PDCTL + 4 * domain);
-
+		/*
+		 * EPCPR register and PDCTL.EPCGOOD bit exist only on DaVinci
+		 * DM644x and DM35x...
+		 */
+		if (cpu_is_davinci_dm644x() || cpu_is_davinci_dm355()) {
+			do {
+				epcpr = __raw_readl(psc_base + EPCPR);
+			} while (((epcpr >> domain) & 1) == 0);
+
+			pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
+			pdctl |= 0x100;
+			__raw_writel(pdctl, psc_base + PDCTL + 4 * domain);
+		}
 	} else {
 		ptcmd = 1 << domain;
 		__raw_writel(ptcmd, psc_base + PTCMD);

[PATCH] DaVinci: only poll EPCPR on DM644x and DM355

[Karicheri, Muralidharan]

Sergei,

Thanks for the patch. Looks good to me.

Murali Karicheri
Software Design Engineer
email: m-karicheri2 at ti.com


>> -----Original Message-----
>> From: davinci-linux-open-source-bounces at linux.davincidsp.com
>> [mailto:davinci-linux-open-source-bounces at linux.davincidsp.com] On Behalf
>> Of Sergei Shtylyov
>> Sent: Thursday, September 15, 2011 10:30 AM
>> To: Hilman, Kevin; davinci-linux-open-source at linux.davincidsp.com; Nori,
>> Sekhar
>> Cc: linux-arm-kernel at lists.infradead.org
>> Subject: [PATCH] DaVinci: only poll EPCPR on DM644x and DM355
>> 
>> EPCPR register and PDCTL.EPCGOOD bit exist only on DaVinci DM644x and
>> DM35x,
>> so do not try to poll EPCPR and set PDCTL.EPCGOOD on the other SoCs -- it
>> would
>> lead to lock up if some power domain hasn't been powered up by this time
>> (which
>> hasn't happened yet on any board, it seems).
>> 
>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>> 
>> ---
>> The patch is against the recent DaVinci tree plus this patch:
>> 
>> http://linux.davincidsp.com/pipermail/davinci-linux-open-source/2011-
>> September/023308.html
>> 
>> Index: linux-davinci/arch/arm/mach-davinci/psc.c
>> ===================================================================
>> --- linux-davinci.orig/arch/arm/mach-davinci/psc.c
>> +++ linux-davinci/arch/arm/mach-davinci/psc.c
>> @@ -88,14 +88,19 @@ void davinci_psc_config(unsigned int dom
>>  		ptcmd = 1 << domain;
>>  		__raw_writel(ptcmd, psc_base + PTCMD);
>> 
>> -		do {
>> -			epcpr = __raw_readl(psc_base + EPCPR);
>> -		} while ((((epcpr >> domain) & 1) == 0));
>> -
>> -		pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
>> -		pdctl |= 0x100;
>> -		__raw_writel(pdctl, psc_base + PDCTL + 4 * domain);
>> -
>> +		/*
>> +		 * EPCPR register and PDCTL.EPCGOOD bit exist only on DaVinci
>> +		 * DM644x and DM35x...
>> +		 */
>> +		if (cpu_is_davinci_dm644x() || cpu_is_davinci_dm355()) {
>> +			do {
>> +				epcpr = __raw_readl(psc_base + EPCPR);
>> +			} while (((epcpr >> domain) & 1) == 0);
>> +
>> +			pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
>> +			pdctl |= 0x100;
>> +			__raw_writel(pdctl, psc_base + PDCTL + 4 * domain);
>> +		}
>>  	} else {
>>  		ptcmd = 1 << domain;
>>  		__raw_writel(ptcmd, psc_base + PTCMD);
>> _______________________________________________
>> Davinci-linux-open-source mailing list
>> Davinci-linux-open-source at linux.davincidsp.com
>> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

[PATCH] DaVinci: only poll EPCPR on DM644x and DM355

[Nori, Sekhar]

Hi Sergei,

On Thu, Sep 15, 2011 at 19:59:48, Sergei Shtylyov wrote:
> EPCPR register and PDCTL.EPCGOOD bit exist only on DaVinci DM644x and DM35x,
> so do not try to poll EPCPR and set PDCTL.EPCGOOD on the other SoCs -- it would
> lead to lock up if some power domain hasn't been powered up by this time (which
> hasn't happened yet on any board, it seems).
> 
> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

Firstly, sorry about feedback this late. I was involved in
the bring-up of a new TI SoC which took much more of my
time than I anticipated.

So, I looked at power domain support for each of the 6
DaVinci SoCs we support (don't have the specifications
for tnetv107x; and code does not have evidence of a separate
DSP power domain).

It looks like none of the SoCs except DM6446 actually support
powering down the DSP power domain.

DM6467, DM355, DM365 all have a single "Always ON" power
domain. DM355 specification actually talks about EPCPR
and EPCGOOD but that's probably due to copy paste from
DM644x specification than anything else.

OMAP-L137 and OMAP-L138 have additional power domains for DSP
and Shared RAM, but do not support powering them down.

So, looks like the only SoC where PDSTAT might indicate a powered
down domain is DM644x and existing code to looks alright for
that SoC.

At this time, it would be better to leave the code as-is and
revisit it if/when a new SoC with programmable power domain
support comes along.

Thanks,
Sekhar

[PATCH] DaVinci: only poll EPCPR on DM644x and DM355

[Sergei Shtylyov]

Hello.

On 23-10-2011 15:10, Nori, Sekhar wrote:

>> EPCPR register and PDCTL.EPCGOOD bit exist only on DaVinci DM644x and DM35x,
>> so do not try to poll EPCPR and set PDCTL.EPCGOOD on the other SoCs -- it would
>> lead to lock up if some power domain hasn't been powered up by this time (which
>> hasn't happened yet on any board, it seems).

>> Signed-off-by: Sergei Shtylyov<sshtylyov@ru.mvista.com>

> Firstly, sorry about feedback this late. I was involved in
> the bring-up of a new TI SoC which took much more of my
> time than I anticipated.

> So, I looked at power domain support for each of the 6
> DaVinci SoCs we support (don't have the specifications

    But we support more than 6 SoCs... :-)

> for tnetv107x; and code does not have evidence of a separate
> DSP power domain).

> It looks like none of the SoCs except DM6446 actually support
> powering down the DSP power domain.

> DM6467, DM355, DM365 all have a single "Always ON" power
> domain. DM355 specification actually talks about EPCPR
> and EPCGOOD but that's probably due to copy paste from
> DM644x specification than anything else.

> OMAP-L137 and OMAP-L138 have additional power domains for DSP
> and Shared RAM, but do not support powering them down.

    I haven't found such words in either OMAP-L137 or OMAP-L138 datasheets.
What they say is:

<<
- On PSC0 PD1/PD_DSP Domain: Controls the sleep state for DSP L1 and L2 Memories
- On PSC1 PD1/PD_SHRAM Domain: Controls the sleep state for the 128K Shared RAM
 >>

    Although "OMAP-L137 Application Processor System Reference Guide" indeed 
said that powering off domain 1 is not supported.
    Actually, I was able to power down DSP/shared RAM domains on DA830 (at 
least the state transition completed); although the domains were on, at least 
after U-Boot. That's how I checked that the code powering up these domains 
actually locks up on this SoC.

> So, looks like the only SoC where PDSTAT might indicate a powered
> down domain is DM644x and existing code to looks alright for
> that SoC.

> At this time, it would be better to leave the code as-is and
> revisit it if/when a new SoC with programmable power domain
> support comes along.

    At least on DA830 power domains appear to be programmable. So I'd still 
like the patch to be applied (I could drop DM355 check though).

> Thanks,
> Sekhar

WBR, Sergei

[PATCH] DaVinci: only poll EPCPR on DM644x and DM355

[Nori, Sekhar]

On Sun, Oct 23, 2011 at 18:13:10, Sergei Shtylyov wrote:
> On 23-10-2011 15:10, Nori, Sekhar wrote:

[...]

> > OMAP-L137 and OMAP-L138 have additional power domains for DSP
> > and Shared RAM, but do not support powering them down.
> 
>     I haven't found such words in either OMAP-L137 or OMAP-L138 datasheets.
> What they say is:
> 
> <<
> - On PSC0 PD1/PD_DSP Domain: Controls the sleep state for DSP L1 and L2 Memories
> - On PSC1 PD1/PD_SHRAM Domain: Controls the sleep state for the 128K Shared RAM
>  >>
> 
>     Although "OMAP-L137 Application Processor System Reference Guide" indeed 
> said that powering off domain 1 is not supported.

Right. There is a statement put in for shared ram as well.

"
Currently powering down the RAMs via the pseudo/RAM power domain is not 
supported; therefore, these domains and the RAM should be left in their 
default power on state.
"

BTW, it looks like a separate "System Reference Guide" doesn't exist
anymore and all the OMAP-L1x user guides have been consolidated to a
single "Technical Reference Manual".

>     Actually, I was able to power down DSP/shared RAM domains on DA830 (at 
> least the state transition completed); although the domains were on, at least 
> after U-Boot. That's how I checked that the code powering up these domains 
> actually locks up on this SoC.

Okay. Surely there must have been some corner case issues found
which probably led the chip design team to make this feature
unsupported.

> > So, looks like the only SoC where PDSTAT might indicate a powered
> > down domain is DM644x and existing code to looks alright for
> > that SoC.
> 
> > At this time, it would be better to leave the code as-is and
> > revisit it if/when a new SoC with programmable power domain
> > support comes along.
> 
>     At least on DA830 power domains appear to be programmable. So I'd still 
> like the patch to be applied (I could drop DM355 check though).

The problem would be that power domain state transition procedure isn't
documented for any SoC other than DM644x. So, we can never be sure
that just avoiding EPCR poll is sufficient for DA830. Software attempting
this would be operating out of spec.

So, how about allowing a power domain transition for DM644x and bailing
out with a warning if a power domain on any other SoC is found to be off?

This way users attempting this will be warned and fix their boot loader.

Thanks,
Sekhar

[PATCH] DaVinci: only poll EPCPR on DM644x and DM355

[Sergei Shtylyov]

Hello.

On 23.10.2011 18:18, Nori, Sekhar wrote:

>>> OMAP-L137 and OMAP-L138 have additional power domains for DSP
>>> and Shared RAM, but do not support powering them down.
>>      I haven't found such words in either OMAP-L137 or OMAP-L138 datasheets.
>> What they say is:
>>
>> <<
>> - On PSC0 PD1/PD_DSP Domain: Controls the sleep state for DSP L1 and L2 Memories
>> - On PSC1 PD1/PD_SHRAM Domain: Controls the sleep state for the 128K Shared RAM
>>   >>
>>
>>      Although "OMAP-L137 Application Processor System Reference Guide" indeed
>> said that powering off domain 1 is not supported.
> Right. There is a statement put in for shared ram as well.

    That's domain 1 too, just on PSC1. :-)

> "
> Currently powering down the RAMs via the pseudo/RAM power domain is not
> supported; therefore, these domains and the RAM should be left in their
> default power on state.
> "
>
> BTW, it looks like a separate "System Reference Guide" doesn't exist
> anymore and all the OMAP-L1x user guides have been consolidated to a
> single "Technical Reference Manual".

     Thanks, didn't know it...

>>      Actually, I was able to power down DSP/shared RAM domains on DA830 (at
>> least the state transition completed); although the domains were on, at least
>> after U-Boot. That's how I checked that the code powering up these domains
>> actually locks up on this SoC.
> Okay. Surely there must have been some corner case issues found
> which probably led the chip design team to make this feature
> unsupported.
>
>>> So, looks like the only SoC where PDSTAT might indicate a powered
>>> down domain is DM644x and existing code to looks alright for
>>> that SoC.
>>> At this time, it would be better to leave the code as-is and
>>> revisit it if/when a new SoC with programmable power domain
>>> support comes along.
>>      At least on DA830 power domains appear to be programmable. So I'd still
>> like the patch to be applied (I could drop DM355 check though).
> The problem would be that power domain state transition procedure isn't
> documented for any SoC other than DM644x. So, we can never be sure
> that just avoiding EPCR poll is sufficient for DA830. Software attempting
> this would be operating out of spec.
>
> So, how about allowing a power domain transition for DM644x and bailing
> out with a warning if a power domain on any other SoC is found to be off?
>
> This way users attempting this will be warned and fix their boot loader.

     Good idea. I probably won't be able to recast the patch soon as I'm 
on vacations and don't have Linux on my laptop (yet).

> Thanks,
> Sekhar

WBR, Sergei

[PATCH] DaVinci: can only power up domains on DM644x

[Sergei Shtylyov]

Only DM644x has the domain power up procedure documented; for all other SoCs a
bootloader probably should have powered the domain up. Print error message and
give up if it hasn't done so.

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

---
Sekhar, I haven't forgotten about that patch we discussed, just lacked time...

 arch/arm/mach-davinci/psc.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Index: linux-davinci/arch/arm/mach-davinci/psc.c
===================================================================
--- linux-davinci.orig/arch/arm/mach-davinci/psc.c
+++ linux-davinci/arch/arm/mach-davinci/psc.c
@@ -81,6 +81,14 @@ void davinci_psc_config(unsigned int dom
 
 	pdstat = __raw_readl(psc_base + PDSTAT + 4 * domain);
 	if ((pdstat & PDSTAT_STATE_MASK) == 0) {
+		/* Only DM644x has the domain power up procedure documented. */
+		if (!cpu_is_davinci_dm644x()) {
+			pr_err("Don't know how to power on domain %u of PSC%u, "
+			       "probably bootloader should have done this!",
+			       domain, ctlr);
+			BUG();
+		}
+
 		pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
 		pdctl |= PDCTL_NEXT;
 		__raw_writel(pdctl, psc_base + PDCTL + 4 * domain);

[PATCH v2] DaVinci: can only power up domains on DM644x

[Sergei Shtylyov]

Only DM644x has the domain power up procedure documented; for all other SoCs a
bootloader probably should have powered the domain up. Print error message and
give up if it hasn't done so.

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

---
Duh, forgot a newline in the message!

 arch/arm/mach-davinci/psc.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Index: linux-davinci/arch/arm/mach-davinci/psc.c
===================================================================
--- linux-davinci.orig/arch/arm/mach-davinci/psc.c
+++ linux-davinci/arch/arm/mach-davinci/psc.c
@@ -81,6 +81,14 @@ void davinci_psc_config(unsigned int dom
 
 	pdstat = __raw_readl(psc_base + PDSTAT + 4 * domain);
 	if ((pdstat & PDSTAT_STATE_MASK) == 0) {
+		/* Only DM644x has the domain power up procedure documented. */
+		if (!cpu_is_davinci_dm644x()) {
+			pr_err("Don't know how to power on domain %u of PSC%u, "
+			       "probably bootloader should have done this!\n",
+			       domain, ctlr);
+			BUG();
+		}
+
 		pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
 		pdctl |= PDCTL_NEXT;
 		__raw_writel(pdctl, psc_base + PDCTL + 4 * domain);

[PATCH v2] DaVinci: can only power up domains on DM644x

[Karicheri, Muralidharan]

Sekhar & Sergei,

This will affect the new SoCs that have capabilities to power up different domains. Why do we need this enforcement? We are working on a DaVinci variant SoC that can do power up multiple domains (That is why I had a patch earlier to support
Multiple PD which is already part of the tree). So I don't want this patch to go through unless I see a reason for pushing this.

Murali Karicheri
Software Design Engineer
email: m-karicheri2 at ti.com

>> -----Original Message-----
>> From: davinci-linux-open-source-bounces at linux.davincidsp.com
>> [mailto:davinci-linux-open-source-bounces at linux.davincidsp.com] On Behalf
>> Of Sergei Shtylyov
>> Sent: Friday, January 06, 2012 1:49 PM
>> To: davinci-linux-open-source at linux.davincidsp.com; Nori, Sekhar; linux-
>> arm-kernel at lists.infradead.org
>> Subject: [PATCH v2] DaVinci: can only power up domains on DM644x
>> 
>> Only DM644x has the domain power up procedure documented; for all other
>> SoCs a
>> bootloader probably should have powered the domain up. Print error message
>> and
>> give up if it hasn't done so.
>> 
>> Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>> 
>> ---
>> Duh, forgot a newline in the message!
>> 
>>  arch/arm/mach-davinci/psc.c |    8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> Index: linux-davinci/arch/arm/mach-davinci/psc.c
>> ===================================================================
>> --- linux-davinci.orig/arch/arm/mach-davinci/psc.c
>> +++ linux-davinci/arch/arm/mach-davinci/psc.c
>> @@ -81,6 +81,14 @@ void davinci_psc_config(unsigned int dom
>> 
>>  	pdstat = __raw_readl(psc_base + PDSTAT + 4 * domain);
>>  	if ((pdstat & PDSTAT_STATE_MASK) == 0) {
>> +		/* Only DM644x has the domain power up procedure documented.
>> */
>> +		if (!cpu_is_davinci_dm644x()) {
>> +			pr_err("Don't know how to power on domain %u of PSC%u, "
>> +			       "probably bootloader should have done this!\n",
>> +			       domain, ctlr);
>> +			BUG();
>> +		}
>> +
>>  		pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
>>  		pdctl |= PDCTL_NEXT;
>>  		__raw_writel(pdctl, psc_base + PDCTL + 4 * domain);
>> _______________________________________________
>> Davinci-linux-open-source mailing list
>> Davinci-linux-open-source at linux.davincidsp.com
>> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

[PATCH v2] DaVinci: can only power up domains on DM644x

[Nori, Sekhar]

Hi Sergei,

On Thu, Jan 12, 2012 at 17:30:15, Karicheri, Muralidharan wrote:
> Sekhar & Sergei,
> 
> This will affect the new SoCs that have capabilities to power up different domains. Why do we need this enforcement? We are working on a DaVinci variant SoC that can do power up multiple domains (That is why I had a patch earlier to support
> Multiple PD which is already part of the tree). So I don't want this patch to go through unless I see a reason for pushing this.

I agree with Murali here. The cpu_is_ checking in the patch
is going to quickly turn ugly as new SoCs with power domain
control are added.

On SoCs where domain power up procedure is not documented,
even bootloader cannot be responsible for powering up the
domain. The domain will be powered up in hardware itself;
so the power domain status cannot show the domain as powered
off.

A better change in this area would be to add a timeout to the
infinite loop reading epcpr. This will enable proper error
reporting.

Thanks,
Sekhar

[PATCH v2] DaVinci: can only power up domains on DM644x

[Sergei Shtylyov]

Hello.

On 01/12/2012 03:00 PM, Karicheri, Muralidharan wrote:

> Sekhar & Sergei,

> This will affect the new SoCs that have capabilities to power up different domains. Why do we need this enforcement? We are working on a DaVinci variant SoC that can do power up multiple domains

    Will their power-up procedure be the same as for DM644x?

> (That is why I had a patch earlier to support
> Multiple PD which is already part of the tree). So I don't want this patch to go through unless I see a reason for pushing this.

    Why you didn't voice your concern when Sekhar has suggested this approach?

> Murali Karicheri
> Software Design Engineer
> email: m-karicheri2 at ti.com

WBR, Sergei