| Message ID | 1314899316-2708-1-git-send-email-m-karicheri2@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Hello. On 09/01/2011 09:48 PM, Murali Karicheri wrote: > There are 5 LSB bits defined in PDSTAT and the code currently uses > a mask of 1 bit to check the status. Also there is PDSTAT and PDCTL > registers defined for ARM domain and DSP domain where as the code > always read the ARM PDSTAT register and DSP PDCTL register. This patch > fixes these issues. > Reviewed-by: Sergei Shtylyov<sshtylyov@mvista.com> I haven't reviewed this patch yet. Maybe you mean "Suggested-by:"? > Signed-off-by: Murali Karicheri<m-karicheri2@ti.com> > --- > comments against previous version of the patch addressed:- Hm, I seem to have missed the previous version then... > - Moved the bug fix to a separate patch > arch/arm/mach-davinci/include/mach/psc.h | 2 +- > arch/arm/mach-davinci/psc.c | 19 ++++++++++--------- > 2 files changed, 11 insertions(+), 10 deletions(-) > > diff --git a/arch/arm/mach-davinci/include/mach/psc.h b/arch/arm/mach-davinci/include/mach/psc.h > index 47fd0bc..63c4366 100644 > --- a/arch/arm/mach-davinci/include/mach/psc.h > +++ b/arch/arm/mach-davinci/include/mach/psc.h > @@ -233,7 +233,7 @@ > #define PTCMD 0x120 > #define PTSTAT 0x128 > #define PDSTAT 0x200 > -#define PDCTL1 0x304 > +#define PDCTL 0x300 > #define MDSTAT 0x800 > #define MDCTL 0xA00 > > diff --git a/arch/arm/mach-davinci/psc.c b/arch/arm/mach-davinci/psc.c > index 1fb6bdf..f157d9c 100644 > --- a/arch/arm/mach-davinci/psc.c > +++ b/arch/arm/mach-davinci/psc.c > @@ -52,7 +52,7 @@ int __init davinci_psc_is_clk_active(unsigned int ctlr, unsigned int id) > void davinci_psc_config(unsigned int domain, unsigned int ctlr, > unsigned int id, bool enable, u32 flags) > { > - u32 epcpr, ptcmd, ptstat, pdstat, pdctl1, mdstat, mdctl; > + u32 epcpr, ptcmd, ptstat, pdstat, pdctl, mdstat, mdctl; > void __iomem *psc_base; > struct davinci_soc_info *soc_info =&davinci_soc_info; > u32 next_state = PSC_STATE_ENABLE; > @@ -79,11 +79,11 @@ void davinci_psc_config(unsigned int domain, unsigned int ctlr, > mdctl |= MDCTL_FORCE; > __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); > + pdstat = __raw_readl(psc_base + PDSTAT + 4 * domain); > + if ((pdstat& 0x1F) == 0) { I suggest to make this mask a #define in <mach/psc.h>, like we have MDSTAT_STATE_MASK there. > + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); > + pdctl |= 0x1; > + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); > > ptcmd = 1<< domain; > __raw_writel(ptcmd, psc_base + PTCMD); > @@ -92,9 +92,10 @@ void davinci_psc_config(unsigned int domain, unsigned int ctlr, > epcpr = __raw_readl(psc_base + EPCPR); > } while ((((epcpr>> domain)& 1) == 0)); Does this work for ARM domain, i.e. does it require an external power supply too? > - pdctl1 = __raw_readl(psc_base + PDCTL1); > - pdctl1 |= 0x100; > - __raw_writel(pdctl1, psc_base + PDCTL1); > + 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); WBR, Sergei
>> >> > Reviewed-by: Sergei Shtylyov<sshtylyov@mvista.com> >> >> I haven't reviewed this patch yet. Maybe you mean "Suggested-by:"? You have reviewed v1 of the patch that has the fix and enhancement as a single patch. You had suggested To move the fix to a separate patch which I had accepted. Did you suggest without reviewing the code? :) Can I add you as "Reviewed-by" in the next version of the patch? >> >> > Signed-off-by: Murali Karicheri<m-karicheri2@ti.com> >> > --- >> > comments against previous version of the patch addressed:- >> >> Hm, I seem to have missed the previous version then... >> >> > - Moved the bug fix to a separate patch >> >> > arch/arm/mach-davinci/include/mach/psc.h | 2 +- >> > arch/arm/mach-davinci/psc.c | 19 ++++++++++--------- >> > 2 files changed, 11 insertions(+), 10 deletions(-) >> > >> > diff --git a/arch/arm/mach-davinci/include/mach/psc.h b/arch/arm/mach- >> davinci/include/mach/psc.h >> > index 47fd0bc..63c4366 100644 >> > --- a/arch/arm/mach-davinci/include/mach/psc.h >> > +++ b/arch/arm/mach-davinci/include/mach/psc.h >> > @@ -233,7 +233,7 @@ >> > #define PTCMD 0x120 >> > #define PTSTAT 0x128 >> > #define PDSTAT 0x200 >> > -#define PDCTL1 0x304 >> > +#define PDCTL 0x300 >> > #define MDSTAT 0x800 >> > #define MDCTL 0xA00 >> > >> > diff --git a/arch/arm/mach-davinci/psc.c b/arch/arm/mach-davinci/psc.c >> > index 1fb6bdf..f157d9c 100644 >> > --- a/arch/arm/mach-davinci/psc.c >> > +++ b/arch/arm/mach-davinci/psc.c >> > @@ -52,7 +52,7 @@ int __init davinci_psc_is_clk_active(unsigned int >> ctlr, unsigned int id) >> > void davinci_psc_config(unsigned int domain, unsigned int ctlr, >> > unsigned int id, bool enable, u32 flags) >> > { >> > - u32 epcpr, ptcmd, ptstat, pdstat, pdctl1, mdstat, mdctl; >> > + u32 epcpr, ptcmd, ptstat, pdstat, pdctl, mdstat, mdctl; >> > void __iomem *psc_base; >> > struct davinci_soc_info *soc_info =&davinci_soc_info; >> > u32 next_state = PSC_STATE_ENABLE; >> > @@ -79,11 +79,11 @@ void davinci_psc_config(unsigned int domain, >> unsigned int ctlr, >> > mdctl |= MDCTL_FORCE; >> > __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); >> > + pdstat = __raw_readl(psc_base + PDSTAT + 4 * domain); >> > + if ((pdstat& 0x1F) == 0) { >> >> I suggest to make this mask a #define in <mach/psc.h>, like we have >> MDSTAT_STATE_MASK there. Ok. I will do. >> >> > + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); >> > + pdctl |= 0x1; >> > + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); >> > >> > ptcmd = 1<< domain; >> > __raw_writel(ptcmd, psc_base + PTCMD); >> > @@ -92,9 +92,10 @@ void davinci_psc_config(unsigned int domain, unsigned >> int ctlr, >> > epcpr = __raw_readl(psc_base + EPCPR); >> > } while ((((epcpr>> domain)& 1) == 0)); >> >> Does this work for ARM domain, i.e. does it require an external power >> supply >> too? Not sure. Will someone actively using the DaVinci platforms answer this question? >> >> > - pdctl1 = __raw_readl(psc_base + PDCTL1); >> > - pdctl1 |= 0x100; >> > - __raw_writel(pdctl1, psc_base + PDCTL1); >> > + 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); >> >> WBR, Sergei
Hello. On 03-09-2011 0:02, Karicheri, Muralidharan wrote: >>>> Reviewed-by: Sergei Shtylyov<sshtylyov@mvista.com> >>> I haven't reviewed this patch yet. Maybe you mean "Suggested-by:"? > You have reviewed v1 of the patch that has the fix and enhancement as a single patch. You had suggested > To move the fix IIRC, I was only talking about the mask fix. But this patch seems OK. > to a separate patch which I had accepted. Did you suggest without reviewing the code? :) > Can I add you as "Reviewed-by" in the next version of the patch? Yes. >>>> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com> [...] >>>> + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); >>>> + pdctl |= 0x1; >>>> + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); >>>> >>>> ptcmd = 1<< domain; >>>> __raw_writel(ptcmd, psc_base + PTCMD); >>>> @@ -92,9 +92,10 @@ void davinci_psc_config(unsigned int domain, unsigned >>> int ctlr, >>>> epcpr = __raw_readl(psc_base + EPCPR); >>>> } while ((((epcpr>> domain)& 1) == 0)); >>> Does this work for ARM domain, i.e. does it require an external power >>> supply too? > Not sure. Will someone actively using the DaVinci platforms answer this question? I may try to verify on my DM6446 if I'll find the time... >>> >>>> - pdctl1 = __raw_readl(psc_base + PDCTL1); >>>> - pdctl1 |= 0x100; >>>> - __raw_writel(pdctl1, psc_base + PDCTL1); >>>> + 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); WBR, Sergei
Hello. On 09/03/2011 05:06 AM, Sergei Shtylyov wrote: >>>>> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com> > [...] >>>>> + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); >>>>> + pdctl |= 0x1; >>>>> + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); >>>>> >>>>> ptcmd = 1<< domain; >>>>> __raw_writel(ptcmd, psc_base + PTCMD); >>>>> @@ -92,9 +92,10 @@ void davinci_psc_config(unsigned int domain, unsigned >>>> int ctlr, >>>>> epcpr = __raw_readl(psc_base + EPCPR); >>>>> } while ((((epcpr>> domain)& 1) == 0)); >>>> Does this work for ARM domain, i.e. does it require an external power >>>> supply too? >> Not sure. Will someone actively using the DaVinci platforms answer this question? > I may try to verify on my DM6446 if I'll find the time... Hmm, DA8x0 documentation doesn't even mention the EPCPR register, so I'm not sure how this loop is supposed to work on those chips... unless this path is never actually executed -- which I'm trying to check now... WBR, Sergei
Hello. On 09/06/2011 06:17 PM, Sergei Shtylyov wrote: >>>>>> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com> >> [...] >>>>>> + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); >>>>>> + pdctl |= 0x1; >>>>>> + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); >>>>>> >>>>>> ptcmd = 1<< domain; >>>>>> __raw_writel(ptcmd, psc_base + PTCMD); >>>>>> @@ -92,9 +92,10 @@ void davinci_psc_config(unsigned int domain, unsigned >>>>> int ctlr, >>>>>> epcpr = __raw_readl(psc_base + EPCPR); >>>>>> } while ((((epcpr>> domain)& 1) == 0)); >>>>> Does this work for ARM domain, i.e. does it require an external power >>>>> supply too? >>> Not sure. Will someone actively using the DaVinci platforms answer this >>> question? >> I may try to verify on my DM6446 if I'll find the time... > Hmm, DA8x0 documentation doesn't even mention the EPCPR register, so I'm not > sure how this loop is supposed to work on those chips... unless this path is > never actually executed -- which I'm trying to check now... Have verified that the patch was never being executed indeed. If I make it executed, the board happily locks up. So looks like this code has even more issues... WBR, Sergei
Hello. On 09-06-2011 06:36 PM, Sergei Shtylyov wrote: >>>>>>> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com> >>> [...] >>>>>>> + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); >>>>>>> + pdctl |= 0x1; >>>>>>> + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); >>>>>>> >>>>>>> ptcmd = 1<< domain; >>>>>>> __raw_writel(ptcmd, psc_base + PTCMD); >>>>>>> @@ -92,9 +92,10 @@ void davinci_psc_config(unsigned int domain, unsigned >>>>>> int ctlr, >>>>>>> epcpr = __raw_readl(psc_base + EPCPR); >>>>>>> } while ((((epcpr>> domain)& 1) == 0)); >>>>>> Does this work for ARM domain, i.e. does it require an external power >>>>>> supply too? >>>> Not sure. Will someone actively using the DaVinci platforms answer this >>>> question? >>> I may try to verify on my DM6446 if I'll find the time... >> Hmm, DA8x0 documentation doesn't even mention the EPCPR register, so I'm not >> sure how this loop is supposed to work on those chips... unless this path is >> never actually executed -- which I'm trying to check now... > Have verified that the patch was never being executed indeed. If I make it > executed, the board happily locks up. So looks like this code has even more > issues... Seeing the same on DM6446 BUT the test is wrong -- of course, I just can't power down domain 0 and then power it up to see if the domain 0's bit gets set in the EPCPR register. :-) Well, given that domain 0 is always enabled, my above question doesn't make much sense anyway, so your patch is fine with me. :-) WBR, Sergei
Hello. On 09/01/2011 09:48 PM, Murali Karicheri wrote: > There are 5 LSB bits defined in PDSTAT and the code currently uses > a mask of 1 bit to check the status. Also there is PDSTAT and PDCTL > registers defined for ARM domain and DSP domain where as the code > always read the ARM PDSTAT register and DSP PDCTL register. This patch Domain 0 PDSTAT and domain 1 PDCTL, to be precise. At least on DA8xx, these domains are called differently than on real DaVincis. > fixes these issues. The real issue seems that domain 1 was never properly powered up (unless domain 1 had been also powered up by the bootloader), because the PDSTAT0 check always indicated that the domain 0 had been already powered up, so we were falling thru to the 'else' alternative. > Reviewed-by: Sergei Shtylyov <sshtylyov@mvista.com> > Signed-off-by: Murali Karicheri<m-karicheri2@ti.com> WBR, Sergei
Hello. On 09/01/2011 10:48 PM, Sergei Shtylyov wrote: >> There are 5 LSB bits defined in PDSTAT and the code currently uses >> a mask of 1 bit to check the status. Also there is PDSTAT and PDCTL >> registers defined for ARM domain and DSP domain where as the code >> always read the ARM PDSTAT register and DSP PDCTL register. This patch >> fixes these issues. >> Reviewed-by: Sergei Shtylyov<sshtylyov@mvista.com> >> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com> >> arch/arm/mach-davinci/include/mach/psc.h | 2 +- >> arch/arm/mach-davinci/psc.c | 19 ++++++++++--------- >> 2 files changed, 11 insertions(+), 10 deletions(-) >> >> diff --git a/arch/arm/mach-davinci/psc.c b/arch/arm/mach-davinci/psc.c >> index 1fb6bdf..f157d9c 100644 >> --- a/arch/arm/mach-davinci/psc.c >> +++ b/arch/arm/mach-davinci/psc.c [...] >> @@ -92,9 +92,10 @@ void davinci_psc_config(unsigned int domain, unsigned int >> ctlr, >> epcpr = __raw_readl(psc_base + EPCPR); >> } while ((((epcpr>> domain)& 1) == 0)); > Does this work for ARM domain, i.e. does it require an external power supply too? >> - pdctl1 = __raw_readl(psc_base + PDCTL1); >> - pdctl1 |= 0x100; >> - __raw_writel(pdctl1, psc_base + PDCTL1); >> + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); >> + pdctl |= 0x100; >> + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); >> + Didn't notice before: this extra empty line is not needed here. WBR, Sergei
Sergei, Thanks for all your investigation work and comments. I will take care of your recent comments against v2 in v3. Looks like the mask is being corrected by a different Patch (ARM: davinci: correct MDSTAT_STATE_MASK) and I can remove the same from my patch. Sekhar, Do you have a branch I can use to create my patch as it is dependent on "ARM: davinci: correct MDSTAT_STATE_MASK" ? Murali Karicheri Software Design Engineer email: m-karicheri2@ti.com >> -----Original Message----- >> From: Sergei Shtylyov [mailto:sshtylyov@mvista.com] >> Sent: Wednesday, September 07, 2011 10:55 AM >> To: Karicheri, Muralidharan >> Cc: davinci-linux-open-source@linux.davincidsp.com; Nori, Sekhar; Hilman, >> Kevin >> Subject: Re: [PATCH v2 1/2] ARM: davinci - fix incorrect offsets and mask >> usage in psc code >> >> Hello. >> >> On 09/01/2011 10:48 PM, Sergei Shtylyov wrote: >> >> >> There are 5 LSB bits defined in PDSTAT and the code currently uses >> >> a mask of 1 bit to check the status. Also there is PDSTAT and PDCTL >> >> registers defined for ARM domain and DSP domain where as the code >> >> always read the ARM PDSTAT register and DSP PDCTL register. This patch >> >> fixes these issues. >> >> >> Reviewed-by: Sergei Shtylyov<sshtylyov@mvista.com> >> >> >> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com> >> >> >> arch/arm/mach-davinci/include/mach/psc.h | 2 +- >> >> arch/arm/mach-davinci/psc.c | 19 ++++++++++--------- >> >> 2 files changed, 11 insertions(+), 10 deletions(-) >> >> >> >> diff --git a/arch/arm/mach-davinci/psc.c b/arch/arm/mach-davinci/psc.c >> >> index 1fb6bdf..f157d9c 100644 >> >> --- a/arch/arm/mach-davinci/psc.c >> >> +++ b/arch/arm/mach-davinci/psc.c >> [...] >> >> @@ -92,9 +92,10 @@ void davinci_psc_config(unsigned int domain, >> unsigned int >> >> ctlr, >> >> epcpr = __raw_readl(psc_base + EPCPR); >> >> } while ((((epcpr>> domain)& 1) == 0)); >> >> > Does this work for ARM domain, i.e. does it require an external power >> supply too? >> >> >> - pdctl1 = __raw_readl(psc_base + PDCTL1); >> >> - pdctl1 |= 0x100; >> >> - __raw_writel(pdctl1, psc_base + PDCTL1); >> >> + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); >> >> + pdctl |= 0x100; >> >> + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); >> >> + >> >> Didn't notice before: this extra empty line is not needed here. >> >> WBR, Sergei
Sekhar, Ignore my email, I can use linux-davinci tree to get the patch that I mentioned. Murali Karicheri Software Design Engineer email: m-karicheri2@ti.com Phone: (301) 407 9583 >> -----Original Message----- >> From: davinci-linux-open-source-bounces@linux.davincidsp.com >> [mailto:davinci-linux-open-source-bounces@linux.davincidsp.com] On Behalf >> Of Karicheri, Muralidharan >> Sent: Wednesday, September 07, 2011 11:33 AM >> To: Sergei Shtylyov; Nori, Sekhar >> Cc: davinci-linux-open-source@linux.davincidsp.com >> Subject: RE: [PATCH v2 1/2] ARM: davinci - fix incorrect offsets and mask >> usage in psc code >> >> Sergei, >> >> Thanks for all your investigation work and comments. I will take care of >> your recent comments against v2 in v3. Looks like the mask is being >> corrected by a different Patch (ARM: davinci: correct MDSTAT_STATE_MASK) >> and I can remove the same from my patch. >> >> >> Sekhar, >> >> Do you have a branch I can use to create my patch as it is dependent on >> "ARM: davinci: correct MDSTAT_STATE_MASK" ? >> >> Murali Karicheri >> Software Design Engineer >> email: m-karicheri2@ti.com >> >> >> >> -----Original Message----- >> >> From: Sergei Shtylyov [mailto:sshtylyov@mvista.com] >> >> Sent: Wednesday, September 07, 2011 10:55 AM >> >> To: Karicheri, Muralidharan >> >> Cc: davinci-linux-open-source@linux.davincidsp.com; Nori, Sekhar; >> Hilman, >> >> Kevin >> >> Subject: Re: [PATCH v2 1/2] ARM: davinci - fix incorrect offsets and >> mask >> >> usage in psc code >> >> >> >> Hello. >> >> >> >> On 09/01/2011 10:48 PM, Sergei Shtylyov wrote: >> >> >> >> >> There are 5 LSB bits defined in PDSTAT and the code currently uses >> >> >> a mask of 1 bit to check the status. Also there is PDSTAT and PDCTL >> >> >> registers defined for ARM domain and DSP domain where as the code >> >> >> always read the ARM PDSTAT register and DSP PDCTL register. This >> patch >> >> >> fixes these issues. >> >> >> >> >> Reviewed-by: Sergei Shtylyov<sshtylyov@mvista.com> >> >> >> >> >> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com> >> >> >> >> >> arch/arm/mach-davinci/include/mach/psc.h | 2 +- >> >> >> arch/arm/mach-davinci/psc.c | 19 ++++++++++--------- >> >> >> 2 files changed, 11 insertions(+), 10 deletions(-) >> >> >> >> >> >> diff --git a/arch/arm/mach-davinci/psc.c b/arch/arm/mach- >> davinci/psc.c >> >> >> index 1fb6bdf..f157d9c 100644 >> >> >> --- a/arch/arm/mach-davinci/psc.c >> >> >> +++ b/arch/arm/mach-davinci/psc.c >> >> [...] >> >> >> @@ -92,9 +92,10 @@ void davinci_psc_config(unsigned int domain, >> >> unsigned int >> >> >> ctlr, >> >> >> epcpr = __raw_readl(psc_base + EPCPR); >> >> >> } while ((((epcpr>> domain)& 1) == 0)); >> >> >> >> > Does this work for ARM domain, i.e. does it require an external power >> >> supply too? >> >> >> >> >> - pdctl1 = __raw_readl(psc_base + PDCTL1); >> >> >> - pdctl1 |= 0x100; >> >> >> - __raw_writel(pdctl1, psc_base + PDCTL1); >> >> >> + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); >> >> >> + pdctl |= 0x100; >> >> >> + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); >> >> >> + >> >> >> >> Didn't notice before: this extra empty line is not needed here. >> >> >> >> WBR, Sergei >> _______________________________________________ >> Davinci-linux-open-source mailing list >> Davinci-linux-open-source@linux.davincidsp.com >> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
On 09/07/2011 07:32 PM, Karicheri, Muralidharan wrote: > Sergei, > Thanks for all your investigation work and comments. I will take care of your recent comments against v2 in v3. Looks like the mask is being corrected by a different Patch (ARM: davinci: correct MDSTAT_STATE_MASK) and I can remove the same from my patch. This is a different register, MDSTAT vs PDSTAT in our case... WBR, Sergei
Ok. Murali Karicheri Software Design Engineer email: m-karicheri2@ti.com Phone: (301) 407 9583 >> -----Original Message----- >> From: Sergei Shtylyov [mailto:sshtylyov@mvista.com] >> Sent: Wednesday, September 07, 2011 2:18 PM >> To: Karicheri, Muralidharan >> Cc: Nori, Sekhar; davinci-linux-open-source@linux.davincidsp.com; Hilman, >> Kevin >> Subject: Re: [PATCH v2 1/2] ARM: davinci - fix incorrect offsets and mask >> usage in psc code >> >> On 09/07/2011 07:32 PM, Karicheri, Muralidharan wrote: >> > Sergei, >> >> > Thanks for all your investigation work and comments. I will take care of >> your recent comments against v2 in v3. Looks like the mask is being >> corrected by a different Patch (ARM: davinci: correct MDSTAT_STATE_MASK) >> and I can remove the same from my patch. >> >> This is a different register, MDSTAT vs PDSTAT in our case... >> >> WBR, Sergei
Hi Murali, On Wed, Sep 07, 2011 at 21:02:45, Karicheri, Muralidharan wrote: > Sergei, > > Thanks for all your investigation work and comments. I will take care of > your recent comments against v2 in v3. Also, can you please ensure linux-arm-kernel@lists.infradead.org is CCed on future submissions. LAK is a subscribers only list, so if you are not already subscribed, please subscribe before posting. Thanks, Sekhar
diff --git a/arch/arm/mach-davinci/include/mach/psc.h b/arch/arm/mach-davinci/include/mach/psc.h index 47fd0bc..63c4366 100644 --- a/arch/arm/mach-davinci/include/mach/psc.h +++ b/arch/arm/mach-davinci/include/mach/psc.h @@ -233,7 +233,7 @@ #define PTCMD 0x120 #define PTSTAT 0x128 #define PDSTAT 0x200 -#define PDCTL1 0x304 +#define PDCTL 0x300 #define MDSTAT 0x800 #define MDCTL 0xA00 diff --git a/arch/arm/mach-davinci/psc.c b/arch/arm/mach-davinci/psc.c index 1fb6bdf..f157d9c 100644 --- a/arch/arm/mach-davinci/psc.c +++ b/arch/arm/mach-davinci/psc.c @@ -52,7 +52,7 @@ int __init davinci_psc_is_clk_active(unsigned int ctlr, unsigned int id) void davinci_psc_config(unsigned int domain, unsigned int ctlr, unsigned int id, bool enable, u32 flags) { - u32 epcpr, ptcmd, ptstat, pdstat, pdctl1, mdstat, mdctl; + u32 epcpr, ptcmd, ptstat, pdstat, pdctl, mdstat, mdctl; void __iomem *psc_base; struct davinci_soc_info *soc_info = &davinci_soc_info; u32 next_state = PSC_STATE_ENABLE; @@ -79,11 +79,11 @@ void davinci_psc_config(unsigned int domain, unsigned int ctlr, mdctl |= MDCTL_FORCE; __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); + pdstat = __raw_readl(psc_base + PDSTAT + 4 * domain); + if ((pdstat & 0x1F) == 0) { + pdctl = __raw_readl(psc_base + PDCTL + 4 * domain); + pdctl |= 0x1; + __raw_writel(pdctl, psc_base + PDCTL + 4 * domain); ptcmd = 1 << domain; __raw_writel(ptcmd, psc_base + PTCMD); @@ -92,9 +92,10 @@ void davinci_psc_config(unsigned int domain, unsigned int ctlr, epcpr = __raw_readl(psc_base + EPCPR); } while ((((epcpr >> domain) & 1) == 0)); - pdctl1 = __raw_readl(psc_base + PDCTL1); - pdctl1 |= 0x100; - __raw_writel(pdctl1, psc_base + PDCTL1); + 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);