On 08/23/2011 02:47 PM, Sergei Shtylyov wrote:

- Rebased to linux-davinci v3.0-rc7 master branch
- Added missing changes to psc files.

This passage should follow the --- tear line.

In one of the new SoC that we are working on, there are multiple power
domains similar to that in C6670. Currently clock module assumes
that there are only two power domains (ARM and DSP). This patch is
added to allow porting of Linux on to the above SoC.

Boot tested this change on DM365 and OMAP L137

Signed-off-by: Murali Karicheri<[email protected]>
[...]

diff --git a/arch/arm/mach-davinci/psc.c b/arch/arm/mach-davinci/psc.c
index 1fb6bdf..2445050 100644
--- a/arch/arm/mach-davinci/psc.c
+++ b/arch/arm/mach-davinci/psc.c
[...]
@@ -80,10 +80,10 @@ void davinci_psc_config(unsigned int domain, unsigned int
ctlr,
__raw_writel(mdctl, psc_base + MDCTL + 4 * id);

pdstat = __raw_readl(psc_base + PDSTAT);
- if ((pdstat & 0x00000001) == 0) {

The current code doesn't make much sense to me -- Kevin, as the original author, could you explain why this cross-dependency between domains 0 and 1? The code below should never be executed in my opinion, since domain 0 is always-on, so 'pdstat & 1' should never be 0 -- although my DA830 manual says it's 0 by default even though PDSTAT0.NEXT is initially 1...
   I could understand if we had:

        if (domain) {

instead if this check...
And the bit mask certainly isn't right as it excludes the "intermediate state" bit 4.

- pdctl1 = __raw_readl(psc_base + PDCTL1);
- pdctl1 |= 0x1;
- __raw_writel(pdctl1, psc_base + PDCTL1);
+ if ((pdstat& (1<< domain)) == 0) {

Hm, are you sure? I'm looking into the "TMS320DM644x DMSoC ARM Subsystem
Reference Guide" and "OMAP-L137 Applications Processor System Reference Guide"
and the PDSTAT.STATE field is in bits 0-4 there, and it's not laid out as bit
per domain (PTCMD is laid out as bit per domain.

   So, this part certainly isn't right.

Shouldn't we read PDSTAT0 for
the ARM domain and PDSTAT1 for the DSP domain instead? I.e. the current code
does not seem correct (why we always change PDCTL1 and PDCTL0 for ARM domain
instead?).

   It seems I missed 'not' before PDCTL0...
Well, actually there's no need to touch PDCTL0 as domain 0 seems to always be always-on one.

+ pdctl = __raw_readl(psc_base + PDCTL + 4 * domain);
+ pdctl |= 0x1;
+ __raw_writel(pdctl, psc_base + PDCTL + 4 * domain);

This seems correct...

... and should switch on non always-on domains. Although I'm not sure about the following bit twiddling concerning the external power source...

WBR, Sergei
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to