Fix RTAS watchdog driver temperature read functionality

PATCH Superseded Adrian Reber 4 participants 9 messages Mar 16, 2009 – Jun 25, 2009

Subsystems (2)

PowerPC Watchdog device drivers

lore

Share & tools

lore

Adrian Reber

author

2009-03-16 12:44 UTC #0 lore raw

PATCH 1/1 Fix RTAS watchdog driver temperature read functionality

Using the RTAS watchdog driver to read out the temperature crashes on a PXCAB:

Unable to handle kernel paging request for data at address 0xfe347b50
Faulting instruction address: 0xc00000000001af64
Oops: Kernel access of bad area, sig: 11 [#1]

This (removing "(void *)__pa") fixes it.

Signed-off-by Adrian Reber <[email protected]>

---
 drivers/watchdog/wdrtas.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

Fix for:

Unable to handle kernel paging request for data at address 0xfe347b50
Faulting instruction address: 0xc00000000001af64
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=4 NUMA Cell
Modules linked in:
NIP: c00000000001af64 LR: c00000000001af28 CTR: c000000000281888
REGS: c0000000fe3477a0 TRAP: 0300   Not tainted  (2.6.29-rc6-11955-g2450cf5)
MSR: 9000000000001032 <ME,IR,DR>  CR: 22000422  XER: 00000000
DAR: 00000000fe347b50, DSISR: 0000000042000000

TASK = c0000000fe4886f0[1184] 'a.out' THREAD: c0000000fe344000 CPU: 1
GPR00: 000000000000002a c0000000fe347a20 c000000000ab2160 c000000000007bc0
GPR04: 0000000022000422 c000000000281888 0000000000000000 00000400001e6138
GPR08: 0000000000000008 c000000000b23ce4 0000000000000008 00000000fe347b54
GPR12: 0000000082000424 c000000000b23680 c0000000003b37b0 c000000000441620
GPR16: c0000000004412e0 c000000000441130 000000000e154ec8 c0000000003b4580
GPR20: 0000000000000000 00000000100a0370 00000000100a0390 c000000000b23cc8
GPR24: 0000000000000001 0000000000000002 00000000fe347b50 0000000000000002
GPR28: 0000000000000010 c000000000b23cb0 c000000000a59588 0000000000000002 NIP [c00000000001af64] .rtas_call+0x1ac/0x1f8
LR [c00000000001af28] .rtas_call+0x170/0x1f8

Call Trace:
[c0000000fe347a20] [c00000000001af28] .rtas_call+0x170/0x1f8 (unreliable)          
[c0000000fe347ae0] [c0000000002812e8] .wdrtas_get_temperature+0x48/0xa8  
[c0000000fe347b70] [c000000000281948] .wdrtas_ioctl+0xc0/0x300
[c0000000fe347c00] [c0000000000f58a4] .vfs_ioctl+0x54/0x138
[c0000000fe347c90] [c0000000000f5a20] .do_vfs_ioctl+0x98/0x738
[c0000000fe347d80] [c0000000000f6114] .SyS_ioctl+0x54/0xc8
[c0000000fe347e30] [c0000000000074b4] syscall_exit+0x0/0x40
Instruction dump:
2fba0000 419e0034 3939fffe 39400004 79290020 39290002 79281764 e93d0070 
7d7a5214 7c09502e 394a0004 7faa4000 <900bfffc> 409effe8 e93d0070 ebe90002 
---[ end trace 084fb7ab3feda750 ]---

drivers/watchdog/wdrtas.c +1 -2

index 5d3b1a8..30ee1f2 100644
@@ -218,8 +218,7 @@ static int wdrtas_get_temperature(void)
 	int temperature = 0;
 
 	result = rtas_call(wdrtas_token_get_sensor_state, 2, 2,
-			   (void *)__pa(&temperature),
-			   WDRTAS_THERMAL_SENSOR, 0);
+			   &temperature, WDRTAS_THERMAL_SENSOR, 0);
 
 	if (result < 0)
 		printk(KERN_WARNING "wdrtas: reading the thermal sensor "

Signature · 1 line

1.5.6.6

Benjamin Herrenschmidt

2009-03-24 04:27 UTC #1 lore raw

Quoted 8 lines · Adrian Reber

Using the RTAS watchdog driver to read out the temperature crashes on a PXCAB:
Unable to handle kernel paging request for data at address 0xfe347b50
Faulting instruction address: 0xc00000000001af64
Oops: Kernel access of bad area, sig: 11 [#1]
This (removing "(void *)__pa") fixes it.

Looks correct, but why not use rtas_get_sensor() instead ? (Exported by arch/powerpc/kernel/rtas.c

Cheers,
Ben.

Quoted 54 lines

Signed-off-by Adrian Reber <[email protected]>
---
 drivers/watchdog/wdrtas.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
Fix for:
Unable to handle kernel paging request for data at address 0xfe347b50
Faulting instruction address: 0xc00000000001af64
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=4 NUMA Cell
Modules linked in:
NIP: c00000000001af64 LR: c00000000001af28 CTR: c000000000281888
REGS: c0000000fe3477a0 TRAP: 0300   Not tainted  (2.6.29-rc6-11955-g2450cf5)
MSR: 9000000000001032 <ME,IR,DR>  CR: 22000422  XER: 00000000
DAR: 00000000fe347b50, DSISR: 0000000042000000
TASK = c0000000fe4886f0[1184] 'a.out' THREAD: c0000000fe344000 CPU: 1
GPR00: 000000000000002a c0000000fe347a20 c000000000ab2160 c000000000007bc0
GPR04: 0000000022000422 c000000000281888 0000000000000000 00000400001e6138
GPR08: 0000000000000008 c000000000b23ce4 0000000000000008 00000000fe347b54
GPR12: 0000000082000424 c000000000b23680 c0000000003b37b0 c000000000441620
GPR16: c0000000004412e0 c000000000441130 000000000e154ec8 c0000000003b4580
GPR20: 0000000000000000 00000000100a0370 00000000100a0390 c000000000b23cc8
GPR24: 0000000000000001 0000000000000002 00000000fe347b50 0000000000000002
GPR28: 0000000000000010 c000000000b23cb0 c000000000a59588 0000000000000002 NIP [c00000000001af64] .rtas_call+0x1ac/0x1f8
LR [c00000000001af28] .rtas_call+0x170/0x1f8
Call Trace:
[c0000000fe347a20] [c00000000001af28] .rtas_call+0x170/0x1f8 (unreliable)          
[c0000000fe347ae0] [c0000000002812e8] .wdrtas_get_temperature+0x48/0xa8  
[c0000000fe347b70] [c000000000281948] .wdrtas_ioctl+0xc0/0x300
[c0000000fe347c00] [c0000000000f58a4] .vfs_ioctl+0x54/0x138
[c0000000fe347c90] [c0000000000f5a20] .do_vfs_ioctl+0x98/0x738
[c0000000fe347d80] [c0000000000f6114] .SyS_ioctl+0x54/0xc8
[c0000000fe347e30] [c0000000000074b4] syscall_exit+0x0/0x40
Instruction dump:
2fba0000 419e0034 3939fffe 39400004 79290020 39290002 79281764 e93d0070 
7d7a5214 7c09502e 394a0004 7faa4000 <900bfffc> 409effe8 e93d0070 ebe90002 
---[ end trace 084fb7ab3feda750 ]---
drivers/watchdog/wdrtas.c +1 -2
index 5d3b1a8..30ee1f2 100644
@@ -218,8 +218,7 @@ static int wdrtas_get_temperature(void)
 	int temperature = 0;
 
 	result = rtas_call(wdrtas_token_get_sensor_state, 2, 2,
-			   (void *)__pa(&temperature),
-			   WDRTAS_THERMAL_SENSOR, 0);
+			   &temperature, WDRTAS_THERMAL_SENSOR, 0);
 
 	if (result < 0)
 		printk(KERN_WARNING "wdrtas: reading the thermal sensor "

Adrian Reber

author

2009-03-24 11:45 UTC #2 lore raw

Quoted 4 lines · Benjamin Herrenschmidt

Quoted 3 lines · Adrian Reber
Using the RTAS watchdog driver to read out the temperature crashes on a PXCAB:

Quoted 8 lines

Quoted 5 lines
Unable to handle kernel paging request for data at address 0xfe347b50
Faulting instruction address: 0xc00000000001af64
Oops: Kernel access of bad area, sig: 11 [#1]
This (removing "(void *)__pa") fixes it.
Looks correct, but why not use rtas_get_sensor() instead ? (Exported by arch/powerpc/kernel/rtas.c

Ah, you mean something like?

 drivers/watchdog/wdrtas.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

drivers/watchdog/wdrtas.c +3 -5

index 5d3b1a8..af2bbfa 100644
@@ -214,16 +214,14 @@ static void wdrtas_timer_keepalive(void)
  */
 static int wdrtas_get_temperature(void)
 {
-	long result;
+	int result;
 	int temperature = 0;
 
-	result = rtas_call(wdrtas_token_get_sensor_state, 2, 2,
-			   (void *)__pa(&temperature),
-			   WDRTAS_THERMAL_SENSOR, 0);
+	result = rtas_get_sensor(WDRTAS_THERMAL_SENSOR, 0, &temperature);
 
 	if (result < 0)
 		printk(KERN_WARNING "wdrtas: reading the thermal sensor "
-		       "faild: %li\n", result);
+		       "faild: %i\n", result);
 	else
 		temperature = ((temperature * 9) / 5) + 32; /* fahrenheit */

Adrian

Benjamin Herrenschmidt

2009-03-24 21:00 UTC #3 lore raw

Quoted 9 lines · Adrian Reber

Quoted 8 lines · Benjamin Herrenschmidt
Quoted 7 lines · Adrian Reber
Using the RTAS watchdog driver to read out the temperature crashes on a PXCAB:
Unable to handle kernel paging request for data at address 0xfe347b50
Faulting instruction address: 0xc00000000001af64
Oops: Kernel access of bad area, sig: 11 [#1]

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
This (removing "(void *)__pa") fixes it.
Looks correct, but why not use rtas_get_sensor() instead ? (Exported by arch/powerpc/kernel/rtas.c
Ah, you mean something like?

Right, now turn it into a proper patch submission :-)

Cheers,
Ben.

Quoted 28 lines

 drivers/watchdog/wdrtas.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

drivers/watchdog/wdrtas.c +3 -5

index 5d3b1a8..af2bbfa 100644
@@ -214,16 +214,14 @@ static void wdrtas_timer_keepalive(void)
  */
 static int wdrtas_get_temperature(void)
 {
-	long result;
+	int result;
 	int temperature = 0;
 
-	result = rtas_call(wdrtas_token_get_sensor_state, 2, 2,
-			   (void *)__pa(&temperature),
-			   WDRTAS_THERMAL_SENSOR, 0);
+	result = rtas_get_sensor(WDRTAS_THERMAL_SENSOR, 0, &temperature);
 
 	if (result < 0)
 		printk(KERN_WARNING "wdrtas: reading the thermal sensor "
-		       "faild: %li\n", result);
+		       "faild: %i\n", result);
 	else
 		temperature = ((temperature * 9) / 5) + 32; /* fahrenheit */

Adrian

Segher Boessenkool

2009-03-25 03:47 UTC #4 lore raw

Quoted 8 lines

Right, now turn it into a proper patch submission :-)
drivers/watchdog/wdrtas.c static void wdrtas_timer_keepalive(void) · line 226
Quoted 6 lines
drivers/watchdog/wdrtas.c +1 -1
 	if (result < 0)
 		printk(KERN_WARNING "wdrtas: reading the thermal sensor "
-		       "faild: %li\n", result);
+		       "faild: %i\n", result);
 	else
 		temperature = ((temperature * 9) / 5) + 32; /* fahrenheit */

Fix the spelling of "failed" while you're at it? (And Fahrenheit maybe).

Segher

Adrian Reber

author

2009-03-26 12:05 UTC #5 lore raw

PATCH v2 1/1 Fix RTAS watchdog driver temperature read functionality

In reply to Benjamin Herrenschmidt

Using the RTAS watchdog driver to read out the temperature crashes on a PXCAB:

Unable to handle kernel paging request for data at address 0xfe347b50
Faulting instruction address: 0xc00000000001af64
Oops: Kernel access of bad area, sig: 11 [#1]

The wrong usage of "(void *)__pa(&temperature)" in rtas_call() is removed by using the function rtas_get_sensor() which does the right thing.

Signed-off-by Adrian Reber <[email protected]>

---
v2: use rtas_get_sensor(); typo fixed

---
 drivers/watchdog/wdrtas.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

drivers/watchdog/wdrtas.c +3 -5

index 5d3b1a8..a9f00dc 100644
@@ -214,16 +214,14 @@ static void wdrtas_timer_keepalive(void)
  */
 static int wdrtas_get_temperature(void)
 {
-	long result;
+	int result;
 	int temperature = 0;
 
-	result = rtas_call(wdrtas_token_get_sensor_state, 2, 2,
-			   (void *)__pa(&temperature),
-			   WDRTAS_THERMAL_SENSOR, 0);
+	result = rtas_get_sensor(WDRTAS_THERMAL_SENSOR, 0, &temperature);
 
 	if (result < 0)
 		printk(KERN_WARNING "wdrtas: reading the thermal sensor "
-		       "faild: %li\n", result);
+		       "failed: %i\n", result);
 	else
 		temperature = ((temperature * 9) / 5) + 32; /* fahrenheit */

Signature · 1 line

1.5.6.6

Adrian Reber

author

2009-06-11 10:58 UTC #6 lore raw

I forgot about this patch. Can this still be merged?

Quoted 44 lines · Adrian Reber

Using the RTAS watchdog driver to read out the temperature crashes on a PXCAB:

Unable to handle kernel paging request for data at address 0xfe347b50
Faulting instruction address: 0xc00000000001af64
Oops: Kernel access of bad area, sig: 11 [#1]

The wrong usage of "(void *)__pa(&temperature)" in rtas_call() is removed by using the function rtas_get_sensor() which does the right thing.

Signed-off-by Adrian Reber <[email protected]>

---
v2: use rtas_get_sensor(); typo fixed

---
 drivers/watchdog/wdrtas.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

drivers/watchdog/wdrtas.c +3 -5

index 5d3b1a8..a9f00dc 100644
@@ -214,16 +214,14 @@ static void wdrtas_timer_keepalive(void)
  */
 static int wdrtas_get_temperature(void)
 {
-	long result;
+	int result;
 	int temperature = 0;
 
-	result = rtas_call(wdrtas_token_get_sensor_state, 2, 2,
-			   (void *)__pa(&temperature),
-			   WDRTAS_THERMAL_SENSOR, 0);
+	result = rtas_get_sensor(WDRTAS_THERMAL_SENSOR, 0, &temperature);
 
 	if (result < 0)
 		printk(KERN_WARNING "wdrtas: reading the thermal sensor "
-		       "faild: %li\n", result);
+		       "failed: %i\n", result);
 	else
 		temperature = ((temperature * 9) / 5) + 32; /* fahrenheit */

Signature · 1 line

1.5.6.6

Utz Bacher

Acked-by

2009-06-25 11:22 UTC #7 lore raw

Adrian Reber wrote on 11.06.2009 12:52:17:

Quoted 47 lines

I forgot about this patch. Can this still be merged?

Quoted 42 lines · Adrian Reber

Using the RTAS watchdog driver to read out the temperature crashes on a PXCAB:

Unable to handle kernel paging request for data at address 0xfe347b50
Faulting instruction address: 0xc00000000001af64
Oops: Kernel access of bad area, sig: 11 [#1]

The wrong usage of "(void *)__pa(&temperature)" in rtas_call() is removed by using the function rtas_get_sensor() which does the right thing.

Signed-off-by Adrian Reber <[email protected]>

---
v2: use rtas_get_sensor(); typo fixed

---
 drivers/watchdog/wdrtas.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

drivers/watchdog/wdrtas.c +3 -5

index 5d3b1a8..a9f00dc 100644
@@ -214,16 +214,14 @@ static void wdrtas_timer_keepalive(void)
  */
 static int wdrtas_get_temperature(void)
 {
-   long result;
+   int result;
    int temperature = 0;
 
-   result = rtas_call(wdrtas_token_get_sensor_state, 2, 2,
-            (void *)__pa(&temperature),
-            WDRTAS_THERMAL_SENSOR, 0);
+   result = rtas_get_sensor(WDRTAS_THERMAL_SENSOR, 0, &temperature);
 
    if (result < 0)
       printk(KERN_WARNING "wdrtas: reading the thermal sensor "
-             "faild: %li\n", result);
+             "failed: %i\n", result);
    else
       temperature = ((temperature * 9) / 5) + 32; /* fahrenheit */

Quoted 2 lines

Quoted 2 lines
Signature · 1 line
1.5.6.6

makes sense. Second version also looks good to me -- thanks!

Acked-by Utz Bacher <[email protected]>

Utz

:wq

Benjamin Herrenschmidt

2009-06-25 11:28 UTC #8 lore raw

Quoted 4 lines · Utz Bacher

Adrian Reber wrote on 11.06.2009 12:52:17:
Quoted 2 lines
I forgot about this patch. Can this still be merged?

Probably, can you re-send it in proper format to the list so it gets into patchwork (unless it's already there in which case I'll pick it up anyway) ?

Cheers,
Ben.

Quoted 57 lines

Quoted 6 lines · Adrian Reber
Quoted 4 lines
Using the RTAS watchdog driver to read out the temperature crashes on a PXCAB:
Unable to handle kernel paging request for data at address
0xfe347b50
Quoted 26 lines
Quoted 26 lines
Faulting instruction address: 0xc00000000001af64
Oops: Kernel access of bad area, sig: 11 [#1]
The wrong usage of "(void *)__pa(&temperature)" in rtas_call() is removed by using the function rtas_get_sensor() which does the right thing.
Signed-off-by Adrian Reber <[email protected]>
---
v2: use rtas_get_sensor(); typo fixed
---
 drivers/watchdog/wdrtas.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
drivers/watchdog/wdrtas.c +1 -1
index 5d3b1a8..a9f00dc 100644
@@ -214,16 +214,14 @@ static void wdrtas_timer_keepalive(void)
  */
 static int wdrtas_get_temperature(void)
 {
-   long result;
+   int result;
    int temperature = 0;
 
Quoted 4 lines
drivers/watchdog/wdrtas.c static void wdrtas_timer_keepalive(void)
Quoted 4 lines
drivers/watchdog/wdrtas.c +1 -3
-   result = rtas_call(wdrtas_token_get_sensor_state, 2, 2,
-            (void *)__pa(&temperature),
-            WDRTAS_THERMAL_SENSOR, 0);
+   result = rtas_get_sensor(WDRTAS_THERMAL_SENSOR, 0,
&temperature);
Quoted 7 lines
drivers/watchdog/wdrtas.c static void wdrtas_timer_keepalive(void)
Quoted 7 lines
drivers/watchdog/wdrtas.c +1 -1
    if (result < 0)
       printk(KERN_WARNING "wdrtas: reading the thermal sensor "
-             "faild: %li\n", result);
+             "failed: %i\n", result);
    else
       temperature = ((temperature * 9) / 5) + 32; /* fahrenheit
*/
Quoted 3 lines
Quoted 3 lines
Signature · 2 lines
-- 
1.5.6.6
makes sense. Second version also looks good to me -- thanks!
Acked-by Utz Bacher <[email protected]>
Utz
:wq