Sergei, Thanks for your comments. See my response inline with a [MK] prefix.
-----Original Message----- From: Sergei Shtylyov [mailto:[email protected]] Sent: Tuesday, August 23, 2011 6:48 AM To: Karicheri, Muralidharan Cc: [email protected]; Nori, Sekhar Subject: Re: [PATCH v1] ARM: davinci: enhancement to support multiple power domains Hello. On 22-08-2011 18:47, [email protected] wrote: > From: Murali Karicheri<[email protected]> > Compared to initial version here are the changes:- > - Rebased to linux-davinci v3.0-rc7 master branch > - Added missing changes to psc files. This passage should follow the --- tear line. [MK] Will do in the next revision. > 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) { > - 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. 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?). [MK] Good catch! The existing code uses bitmask 1 though 5 bits are allocated. For DM644x, value Can be 0 or 1 and hence will be fine. But I think we need to change the mask to 0x1F. Also the PDSTAT is available per domain, and not bit per domain (again you are correct). So should be changed to pdstat = __raw_readl(psc_base + PDSTAT + 4 * domain); > + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); > + pdctl |= 0x1; > + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); This seems correct... WBR, Sergei _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
