Skip to contentSkip to conversation

DaVinci: correct PDSTAT.STATE mask

PATCH Superseded Sergei Shtylyov 4 participants 6 messages Sep 16, 2011 – Oct 13, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 DaVinci: correct PDSTAT.STATE mask

MDSTAT.STATE occupies bits 0..5 according to all available documentation, so fix the masks which previously was leaving out the itermediate state indicator bit.

---
Analogous Linux patch has been queued in the linux-davinci tree:

http://linux.davincidsp.com/pipermail/davinci-linux-open-source/2011-July/023075.html

 arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S |    6 +++---
 arch/arm/cpu/arm926ejs/davinci/psc.c           |    4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)
arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S +3 -3
Index: u-boot/arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S
===================================================================
@@ -268,7 +268,7 @@ checkStatClkStop:
 checkDDRStatClkStop:
 	ldr	r6, MDSTAT_DDR2
 	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $0x3f
 	cmp	r7, $0x03
 	bne	checkDDRStatClkStop
 
@@ -343,7 +343,7 @@ checkStatClkStop2:
 checkDDRStatClkStop2:
 	ldr	r6, MDSTAT_DDR2
 	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $0x3f
 	cmp	r7, $0x01
 	bne	checkDDRStatClkStop2
 
@@ -374,7 +374,7 @@ checkStatClkEn2:
 checkDDRStatClkEn2:
 	ldr	r6, MDSTAT_DDR2
 	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $0x3f
 	cmp	r7, $0x03
 	bne	checkDDRStatClkEn2
 
arch/arm/cpu/arm926ejs/davinci/psc.c +2 -2
Index: u-boot/arch/arm/cpu/arm926ejs/davinci/psc.c
===================================================================
@@ -83,7 +83,7 @@ void lpsc_on(unsigned int id)
 	while (readl(ptstat) & 0x01)
 		continue;
 
-	if ((readl(mdstat) & 0x1f) == 0x03)
+	if ((readl(mdstat) & 0x3f) == 0x03)
 		return; /* Already on and enabled */
 
 	writel(readl(mdctl) | 0x03, mdctl);
@@ -114,7 +114,7 @@ void lpsc_on(unsigned int id)
 
 	while (readl(ptstat) & 0x01)
 		continue;
-	while ((readl(mdstat) & 0x1f) != 0x03)
+	while ((readl(mdstat) & 0x3f) != 0x03)
 		continue;
 }
 
#1 lore raw

PATCH v2 1/1 DaVinci: correct MDSTAT.STATE mask

MDSTAT.STATE occupies bits 0..5 according to all available documentation, so fix the masks which previously was leaving out the intermediate state indicator bit.

---
Resending with the corrected subject/description... Analogous Linux patch has been queued in the linux-davinci tree:

http://linux.davincidsp.com/pipermail/davinci-linux-open-source/2011-July/023075.html

 arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S |    6 +++---
 arch/arm/cpu/arm926ejs/davinci/psc.c           |    4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)
arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S +3 -3
Index: u-boot/arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S
===================================================================
@@ -268,7 +268,7 @@ checkStatClkStop:
 checkDDRStatClkStop:
 	ldr	r6, MDSTAT_DDR2
 	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $0x3f
 	cmp	r7, $0x03
 	bne	checkDDRStatClkStop
 
@@ -343,7 +343,7 @@ checkStatClkStop2:
 checkDDRStatClkStop2:
 	ldr	r6, MDSTAT_DDR2
 	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $0x3f
 	cmp	r7, $0x01
 	bne	checkDDRStatClkStop2
 
@@ -374,7 +374,7 @@ checkStatClkEn2:
 checkDDRStatClkEn2:
 	ldr	r6, MDSTAT_DDR2
 	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $0x3f
 	cmp	r7, $0x03
 	bne	checkDDRStatClkEn2
 
arch/arm/cpu/arm926ejs/davinci/psc.c +2 -2
Index: u-boot/arch/arm/cpu/arm926ejs/davinci/psc.c
===================================================================
@@ -83,7 +83,7 @@ void lpsc_on(unsigned int id)
 	while (readl(ptstat) & 0x01)
 		continue;
 
-	if ((readl(mdstat) & 0x1f) == 0x03)
+	if ((readl(mdstat) & 0x3f) == 0x03)
 		return; /* Already on and enabled */
 
 	writel(readl(mdctl) | 0x03, mdctl);
@@ -114,7 +114,7 @@ void lpsc_on(unsigned int id)
 
 	while (readl(ptstat) & 0x01)
 		continue;
-	while ((readl(mdstat) & 0x1f) != 0x03)
+	while ((readl(mdstat) & 0x3f) != 0x03)
 		continue;
 }
 
#2 lore raw

PATCH v2 1/1 DaVinci: correct MDSTAT.STATE mask

Dear Sergei Shtylyov,

Quoted 20 lines · In message <[email protected]> you wrote:

MDSTAT.STATE occupies bits 0..5 according to all available documentation, so fix the masks which previously was leaving out the intermediate state indicator bit.

---
Resending with the corrected subject/description... Analogous Linux patch has been queued in the linux-davinci tree:

http://linux.davincidsp.com/pipermail/davinci-linux-open-source/2011-July/023075.html

 arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S |    6 +++---
 arch/arm/cpu/arm926ejs/davinci/psc.c           |    4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

Index: u-boot/arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S
===================================================================

arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S checkStatClkStop: · line 271

Quoted 5 lines
arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S +1 -1
@@ -268,7 +268,7 @@ checkStatClkStop:
 checkDDRStatClkStop:
 	ldr	r6, MDSTAT_DDR2
 	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $0x3f

Don't you think it's high time to replace these magic constants with at least somewhat meaningful symbolic names (so it would be sufficient to fix this in a single place) ?

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Let's say the docs present a simplified view of reality...    :-)
                      - Larry Wall in  <[email protected]>
#3 lore raw

PATCH v2 1/1 DaVinci: correct MDSTAT.STATE mask

Hello.

Quoted 1 line · Wolfgang Denk

Dear Sergei Shtylyov,

Thank you for the really quick response. :-)

Quoted 15 lines
Quoted 15 lines

MDSTAT.STATE occupies bits 0..5 according to all available documentation, so fix the masks which previously was leaving out the intermediate state indicator bit.

---
Resending with the corrected subject/description... Analogous Linux patch has been queued in the linux-davinci tree:

http://linux.davincidsp.com/pipermail/davinci-linux-open-source/2011-July/023075.html

  arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S |    6 +++---
  arch/arm/cpu/arm926ejs/davinci/psc.c           |    4 ++--
  2 files changed, 5 insertions(+), 5 deletions(-)
Quoted 14 lines

arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S checkStatClkStop: · line 271

Quoted 10 lines

Index: u-boot/arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S
===================================================================

arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S +1 -1
@@ -268,7 +268,7 @@ checkStatClkStop:
  checkDDRStatClkStop:
  	ldr	r6, MDSTAT_DDR2
  	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $0x3f

Don't you think it's high time to replace these magic constants with at least somewhat meaningful symbolic names (so it would be sufficient to fix this in a single place) ?

I agree. Do you mean I should recast this patch, or do two patches
instead: one adding the symbolic name and another correcting its value?

Quoted 3 lines

Best regards,

Wolfgang Denk

WBR, Sergei

#4 lore raw

PATCH v3 1/1 DaVinci: correct MDSTAT.STATE mask

In reply to Sergei Shtylyov

MDSTAT.STATE occupies bits 0..5 according to all available documentation, so fix the mask which previously was leaving out the intermediate state indicator bit.

While at it, introduce two #define's for that mask -- unfortunately, we can't use a single #define as the assembly code can't include <asm/arch/hardware.h> due to C-specfic constructs in it.

---
 arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S |    8 +++++---
 arch/arm/cpu/arm926ejs/davinci/psc.c           |    4 ++--
 arch/arm/include/asm/arch-davinci/hardware.h   |    2 ++
 3 files changed, 9 insertions(+), 5 deletions(-)
arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S +5 -3
Index: u-boot/arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S
===================================================================
@@ -45,6 +45,8 @@
 
 #include <config.h>
 
+#define MDSTAT_STATE	0x3f
+
 .globl	lowlevel_init
 lowlevel_init:
 
@@ -268,7 +270,7 @@ checkStatClkStop:
 checkDDRStatClkStop:
 	ldr	r6, MDSTAT_DDR2
 	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $MDSTAT_STATE
 	cmp	r7, $0x03
 	bne	checkDDRStatClkStop
 
@@ -343,7 +345,7 @@ checkStatClkStop2:
 checkDDRStatClkStop2:
 	ldr	r6, MDSTAT_DDR2
 	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $MDSTAT_STATE
 	cmp	r7, $0x01
 	bne	checkDDRStatClkStop2
 
@@ -374,7 +376,7 @@ checkStatClkEn2:
 checkDDRStatClkEn2:
 	ldr	r6, MDSTAT_DDR2
 	ldr	r7, [r6]
-	and	r7, r7, $0x1f
+	and	r7, r7, $MDSTAT_STATE
 	cmp	r7, $0x03
 	bne	checkDDRStatClkEn2
 
arch/arm/cpu/arm926ejs/davinci/psc.c +2 -2
Index: u-boot/arch/arm/cpu/arm926ejs/davinci/psc.c
===================================================================
@@ -83,7 +83,7 @@ void lpsc_on(unsigned int id)
 	while (readl(ptstat) & 0x01)
 		continue;
 
-	if ((readl(mdstat) & 0x1f) == 0x03)
+	if ((readl(mdstat) & PSC_MDSTAT_STATE) == 0x03)
 		return; /* Already on and enabled */
 
 	writel(readl(mdctl) | 0x03, mdctl);
@@ -114,7 +114,7 @@ void lpsc_on(unsigned int id)
 
 	while (readl(ptstat) & 0x01)
 		continue;
-	while ((readl(mdstat) & 0x1f) != 0x03)
+	while ((readl(mdstat) & PSC_MDSTAT_STATE) != 0x03)
 		continue;
 }
 
arch/arm/include/asm/arch-davinci/hardware.h +2 -0
Index: u-boot/arch/arm/include/asm/arch-davinci/hardware.h
===================================================================
@@ -347,6 +347,8 @@ struct davinci_psc_regs {
 
 #endif /* CONFIG_SOC_DA8XX */
 
+#define PSC_MDSTAT_STATE		0x3f
+
 #ifndef CONFIG_SOC_DA8XX
 
 /* Miscellania... */
#5 lore raw

PATCH v3 1/1 DaVinci: correct MDSTAT.STATE mask

Hi,

Quoted 7 lines · Sergei Shtylyov

MDSTAT.STATE occupies bits 0..5 according to all available documentation, so fix the mask which previously was leaving out the intermediate state indicator bit.

While at it, introduce two #define's for that mask -- unfortunately, we can't use a single #define as the assembly code can't include <asm/arch/hardware.h> due to C-specfic constructs in it.

Quoted 7 lines

---

?arch/arm/cpu/arm926ejs/davinci/lowlevel_init.S | ? ?8 +++++---
?arch/arm/cpu/arm926ejs/davinci/psc.c ? ? ? ? ? | ? ?4 ++--
?arch/arm/include/asm/arch-davinci/hardware.h ? | ? ?2 ++

?3 files changed, 9 insertions(+), 5 deletions(-)

What's the state of this patch? Could this be merged please? Thank you! Regards, Christian