[PATCH] powerpc/44x: mask and shift to zero bug

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE3286d

6 messages, 4 authors, 2017-08-31 · open the first message on its own page

[PATCH] powerpc/44x: mask and shift to zero bug

From: Dan Carpenter <hidden>
Date: 2017-08-25 10:34:16

My static checker complains that 0x00001800 >> 13 is zero.  Looking at
the context, it seems like a copy and paste bug from the line below and
probably 0x3 << 13 or 0x00006000 was intended.

Fixes: 2af59f7d5c3e ("[POWERPC] 4xx: Add 405GPr and 405EP support in boot wrapper")
Signed-off-by: Dan Carpenter <redacted>
---
Not tested!
diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c
index 9d3bd4c45a24..f7da65169124 100644
--- a/arch/powerpc/boot/4xx.c
+++ b/arch/powerpc/boot/4xx.c
@@ -564,7 +564,7 @@ void ibm405gp_fixup_clocks(unsigned int sys_clk, unsigned int ser_clk)
 		fbdv = 16;
 	cbdv = ((pllmr & 0x00060000) >> 17) + 1; /* CPU:PLB */
 	opdv = ((pllmr & 0x00018000) >> 15) + 1; /* PLB:OPB */
-	ppdv = ((pllmr & 0x00001800) >> 13) + 1; /* PLB:PCI */
+	ppdv = ((pllmr & 0x00006000) >> 13) + 1; /* PLB:PCI */
 	epdv = ((pllmr & 0x00001800) >> 11) + 2; /* PLB:EBC */
 	udiv = ((cpc0_cr0 & 0x3e) >> 1) + 1;
 

Re: [PATCH] powerpc/44x: mask and shift to zero bug

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2017-08-27 04:58:58

On Fri, 2017-08-25 at 13:33 +0300, Dan Carpenter wrote:
quoted hunk
My static checker complains that 0x00001800 >> 13 is zero.  Looking at
the context, it seems like a copy and paste bug from the line below and
probably 0x3 << 13 or 0x00006000 was intended.

Fixes: 2af59f7d5c3e ("[POWERPC] 4xx: Add 405GPr and 405EP support in boot wrapper")
Signed-off-by: Dan Carpenter <redacted>
---
Not tested!
diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c
index 9d3bd4c45a24..f7da65169124 100644
--- a/arch/powerpc/boot/4xx.c
+++ b/arch/powerpc/boot/4xx.c
@@ -564,7 +564,7 @@ void ibm405gp_fixup_clocks(unsigned int sys_clk, unsigned int ser_clk)
 		fbdv = 16;
 	cbdv = ((pllmr & 0x00060000) >> 17) + 1; /* CPU:PLB */
 	opdv = ((pllmr & 0x00018000) >> 15) + 1; /* PLB:OPB */
-	ppdv = ((pllmr & 0x00001800) >> 13) + 1; /* PLB:PCI */
+	ppdv = ((pllmr & 0x00006000) >> 13) + 1; /* PLB:PCI */
 	epdv = ((pllmr & 0x00001800) >> 11) + 2; /* PLB:EBC */
 	udiv = ((cpc0_cr0 & 0x3e) >> 1) + 1; 
That rings a bell... Is this something we tried to fix before and had
problems ? The thing is when I opened the 405GP and EP manual PDF,
evince had memorized that this register was the last page I looked at
:-) And I don't remember how many years ago that is.

According to the 405gp spec ppdv is IBM bits 17,18 so your patch is
correct.

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Re: [PATCH] powerpc/44x: mask and shift to zero bug

From: Dan Carpenter <hidden>
Date: 2017-08-28 07:57:47

On Sun, Aug 27, 2017 at 02:56:31PM +1000, Benjamin Herrenschmidt wrote:
On Fri, 2017-08-25 at 13:33 +0300, Dan Carpenter wrote:
quoted
My static checker complains that 0x00001800 >> 13 is zero.  Looking at
the context, it seems like a copy and paste bug from the line below and
probably 0x3 << 13 or 0x00006000 was intended.

Fixes: 2af59f7d5c3e ("[POWERPC] 4xx: Add 405GPr and 405EP support in boot wrapper")
Signed-off-by: Dan Carpenter <redacted>
---
Not tested!
diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c
index 9d3bd4c45a24..f7da65169124 100644
--- a/arch/powerpc/boot/4xx.c
+++ b/arch/powerpc/boot/4xx.c
@@ -564,7 +564,7 @@ void ibm405gp_fixup_clocks(unsigned int sys_clk, unsigned int ser_clk)
 		fbdv = 16;
 	cbdv = ((pllmr & 0x00060000) >> 17) + 1; /* CPU:PLB */
 	opdv = ((pllmr & 0x00018000) >> 15) + 1; /* PLB:OPB */
-	ppdv = ((pllmr & 0x00001800) >> 13) + 1; /* PLB:PCI */
+	ppdv = ((pllmr & 0x00006000) >> 13) + 1; /* PLB:PCI */
 	epdv = ((pllmr & 0x00001800) >> 11) + 2; /* PLB:EBC */
 	udiv = ((cpc0_cr0 & 0x3e) >> 1) + 1; 
That rings a bell... Is this something we tried to fix before and had
problems ? The thing is when I opened the 405GP and EP manual PDF,
evince had memorized that this register was the last page I looked at
:-) And I don't remember how many years ago that is.

According to the 405gp spec ppdv is IBM bits 17,18 so your patch is
correct.

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Hm...  I did a search through my mailbox and you're right we discussed
this before.

[bug report] [POWERPC] 4xx: Add 405GPr and 405EP support in boot wrapper

I sent this email during kernel summit and neither of us could send a
patch at the time and we both problem forgot.  I definitely forgot.

regards,
dan carpenter

Re: [PATCH] powerpc/44x: mask and shift to zero bug

From: Dan Carpenter <hidden>
Date: 2017-08-28 08:36:52

On Mon, Aug 28, 2017 at 10:57:05AM +0300, Dan Carpenter wrote:
I sent this email during kernel summit and neither of us could send a
patch at the time and we both problem forgot.  I definitely forgot.
s/problem/probably/... I suck at email.  :(

regards,
dan carpenter

Re: [PATCH] powerpc/44x: mask and shift to zero bug

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2017-08-28 11:19:24

Dan Carpenter [off-list ref] writes:
On Sun, Aug 27, 2017 at 02:56:31PM +1000, Benjamin Herrenschmidt wrote:
quoted
On Fri, 2017-08-25 at 13:33 +0300, Dan Carpenter wrote:
quoted
My static checker complains that 0x00001800 >> 13 is zero.  Looking at
the context, it seems like a copy and paste bug from the line below and
probably 0x3 << 13 or 0x00006000 was intended.

Fixes: 2af59f7d5c3e ("[POWERPC] 4xx: Add 405GPr and 405EP support in boot wrapper")
Signed-off-by: Dan Carpenter <redacted>
---
Not tested!
diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c
index 9d3bd4c45a24..f7da65169124 100644
--- a/arch/powerpc/boot/4xx.c
+++ b/arch/powerpc/boot/4xx.c
@@ -564,7 +564,7 @@ void ibm405gp_fixup_clocks(unsigned int sys_clk, unsigned int ser_clk)
 		fbdv = 16;
 	cbdv = ((pllmr & 0x00060000) >> 17) + 1; /* CPU:PLB */
 	opdv = ((pllmr & 0x00018000) >> 15) + 1; /* PLB:OPB */
-	ppdv = ((pllmr & 0x00001800) >> 13) + 1; /* PLB:PCI */
+	ppdv = ((pllmr & 0x00006000) >> 13) + 1; /* PLB:PCI */
 	epdv = ((pllmr & 0x00001800) >> 11) + 2; /* PLB:EBC */
 	udiv = ((cpc0_cr0 & 0x3e) >> 1) + 1; 
That rings a bell... Is this something we tried to fix before and had
problems ? The thing is when I opened the 405GP and EP manual PDF,
evince had memorized that this register was the last page I looked at
:-) And I don't remember how many years ago that is.

According to the 405gp spec ppdv is IBM bits 17,18 so your patch is
correct.

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Hm...  I did a search through my mailbox and you're right we discussed
this before.

[bug report] [POWERPC] 4xx: Add 405GPr and 405EP support in boot wrapper

I sent this email during kernel summit and neither of us could send a
patch at the time and we both problem forgot.  I definitely forgot.
If you send a patch with the report, like you did this time, then it
will be remembered in patchwork. That doesn't mean we won't forget it,
but it makes is *less likely* we'll forget it :)

http://patchwork.ozlabs.org/project/linuxppc-dev/list/?submitter=9327&state=%2A&archive=both

cheers

Re: powerpc/44x: mask and shift to zero bug

From: Michael Ellerman <hidden>
Date: 2017-08-31 11:36:21

On Fri, 2017-08-25 at 10:33:40 UTC, Dan Carpenter wrote:
My static checker complains that 0x00001800 >> 13 is zero.  Looking at
the context, it seems like a copy and paste bug from the line below and
probably 0x3 << 13 or 0x00006000 was intended.

Fixes: 2af59f7d5c3e ("[POWERPC] 4xx: Add 405GPr and 405EP support in boot wrapper")
Signed-off-by: Dan Carpenter <redacted>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/8d046759f6ad75824fdf7b9c9a3da0

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help