[PATCH] powerpc/wdrtas: Update wdrtas_get_interval to use rtas_data_buf

[PATCH] powerpc/wdrtas: Update wdrtas_get_interval to use rtas_data_buf

[Mark Nelson]

The buffer passed to the ibm,get-system-parameter RTAS call must be
in the RMA. To ensure we pass an address in the RMA use rtas_data_buf
for the actual RTAS call and then copy the result to value. We can't
just make it static because this can be compiled in as a module.

Also add the WDRTAS_SP_SPI_LEN so we don't litter '4' throughout the
function.

Signed-off-by: Mark Nelson <markn@au1.ibm.com>
---

Adrian, does this patch cause any problems for your pxcabs?

Thanks!
Mark

 drivers/watchdog/wdrtas.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Index: upstream/drivers/watchdog/wdrtas.c
===================================================================
--- upstream.orig/drivers/watchdog/wdrtas.c
+++ upstream/drivers/watchdog/wdrtas.c
@@ -106,6 +106,8 @@ static int wdrtas_set_interval(int inter
 	return result;
 }
 
+#define WDRTAS_SP_SPI_LEN 4
+
 /**
  * wdrtas_get_interval - returns the current watchdog interval
  * @fallback_value: value (in seconds) to use, if the RTAS call fails
@@ -119,10 +121,17 @@ static int wdrtas_set_interval(int inter
 static int wdrtas_get_interval(int fallback_value)
 {
 	long result;
-	char value[4];
+	char value[WDRTAS_SP_SPI_LEN];
 
+	spin_lock(&rtas_data_buf_lock);
+	memset(rtas_data_buf, 0, WDRTAS_SP_SPI_LEN);
 	result = rtas_call(wdrtas_token_get_sp, 3, 1, NULL,
-			   WDRTAS_SP_SPI, (void *)__pa(&value), 4);
+			   WDRTAS_SP_SPI, __pa(rtas_data_buf),
+			   WDRTAS_SP_SPI_LEN);
+
+	memcpy(value, rtas_data_buf, WDRTAS_SP_SPI_LEN);
+	spin_unlock(&rtas_data_buf_lock);
+
 	if (value[0] != 0 || value[1] != 2 || value[3] != 0 || result < 0) {
 		printk(KERN_WARNING "wdrtas: could not get sp_spi watchdog "
 		       "timeout (%li). Continuing\n", result);

Re: [PATCH] powerpc/wdrtas: Update wdrtas_get_interval to use rtas_data_buf

[Adrian Reber]

On Tue, Mar 24, 2009 at 05:30:41PM +1100, Mark Nelson wrote:
> The buffer passed to the ibm,get-system-parameter RTAS call must be
> in the RMA. To ensure we pass an address in the RMA use rtas_data_buf
> for the actual RTAS call and then copy the result to value. We can't
> just make it static because this can be compiled in as a module.
> 
> Also add the WDRTAS_SP_SPI_LEN so we don't litter '4' throughout the
> function.
> 
> Signed-off-by: Mark Nelson <markn@au1.ibm.com>
> ---
> 
> Adrian, does this patch cause any problems for your pxcabs?

No, it even helps. I have no tried the watchdog until now, but without the
patch I get: 

   wdrtas: could not get sp_spi watchdog timeout (0). Continuing

and with the patch it reads the correct value. So only with your patch
it works like it is supposed to. Thanks!

Tested-by: Adrian Reber <adrian@lisas.de>

> Thanks!
> Mark
> 
>  drivers/watchdog/wdrtas.c |   13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> Index: upstream/drivers/watchdog/wdrtas.c
> ===================================================================
> --- upstream.orig/drivers/watchdog/wdrtas.c
> +++ upstream/drivers/watchdog/wdrtas.c
> @@ -106,6 +106,8 @@ static int wdrtas_set_interval(int inter
>  	return result;
>  }
>  
> +#define WDRTAS_SP_SPI_LEN 4
> +
>  /**
>   * wdrtas_get_interval - returns the current watchdog interval
>   * @fallback_value: value (in seconds) to use, if the RTAS call fails
> @@ -119,10 +121,17 @@ static int wdrtas_set_interval(int inter
>  static int wdrtas_get_interval(int fallback_value)
>  {
>  	long result;
> -	char value[4];
> +	char value[WDRTAS_SP_SPI_LEN];
>  
> +	spin_lock(&rtas_data_buf_lock);
> +	memset(rtas_data_buf, 0, WDRTAS_SP_SPI_LEN);
>  	result = rtas_call(wdrtas_token_get_sp, 3, 1, NULL,
> -			   WDRTAS_SP_SPI, (void *)__pa(&value), 4);
> +			   WDRTAS_SP_SPI, __pa(rtas_data_buf),
> +			   WDRTAS_SP_SPI_LEN);
> +
> +	memcpy(value, rtas_data_buf, WDRTAS_SP_SPI_LEN);
> +	spin_unlock(&rtas_data_buf_lock);
> +
>  	if (value[0] != 0 || value[1] != 2 || value[3] != 0 || result < 0) {
>  		printk(KERN_WARNING "wdrtas: could not get sp_spi watchdog "
>  		       "timeout (%li). Continuing\n", result);

		Adrian

-- 
Adrian Reber <adrian@lisas.de>            http://lisas.de/~adrian/
There's coffee in that nebula!
		-- Capt. Kathryn Janeway, Star Trek: Voyager, "The Cloud"

Re: [PATCH] powerpc/wdrtas: Update wdrtas_get_interval to use rtas_data_buf

[Utz Bacher]

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]

Adrian Reber <adrian@lisas.de> wrote on 24.03.2009 13:31:31:
> On Tue, Mar 24, 2009 at 05:30:41PM +1100, Mark Nelson wrote:
> > The buffer passed to the ibm,get-system-parameter RTAS call must be
> > in the RMA. To ensure we pass an address in the RMA use rtas_data_buf
> > for the actual RTAS call and then copy the result to value. We can't
> > just make it static because this can be compiled in as a module.
> > 
> > Also add the WDRTAS_SP_SPI_LEN so we don't litter '4' throughout the
> > function.
> > 
> > Signed-off-by: Mark Nelson <markn@au1.ibm.com>
> > ---
> > 
> > Adrian, does this patch cause any problems for your pxcabs?
> 
> No, it even helps. I have no tried the watchdog until now, but without 
the
> patch I get: 
> 
>    wdrtas: could not get sp_spi watchdog timeout (0). Continuing
> 
> and with the patch it reads the correct value. So only with your patch
> it works like it is supposed to. Thanks!
> 
> Tested-by: Adrian Reber <adrian@lisas.de>

looks good to me.

Acked-by: Utz Bacher <utz.bacher@de.ibm.com>

Utz

:wq

[-- Attachment #2: Type: text/html, Size: 1456 bytes --]

Re: [PATCH] powerpc/wdrtas: Update wdrtas_get_interval to use rtas_data_buf

[Mark Nelson]

On Tue, 24 Mar 2009 11:31:31 pm Adrian Reber wrote:
> On Tue, Mar 24, 2009 at 05:30:41PM +1100, Mark Nelson wrote:
> > The buffer passed to the ibm,get-system-parameter RTAS call must be
> > in the RMA. To ensure we pass an address in the RMA use rtas_data_buf
> > for the actual RTAS call and then copy the result to value. We can't
> > just make it static because this can be compiled in as a module.
> > 
> > Also add the WDRTAS_SP_SPI_LEN so we don't litter '4' throughout the
> > function.
> > 
> > Signed-off-by: Mark Nelson <markn@au1.ibm.com>
> > ---
> > 
> > Adrian, does this patch cause any problems for your pxcabs?
> 
> No, it even helps. I have no tried the watchdog until now, but without the
> patch I get: 
> 
>    wdrtas: could not get sp_spi watchdog timeout (0). Continuing
> 
> and with the patch it reads the correct value. So only with your patch
> it works like it is supposed to. Thanks!
> 
> Tested-by: Adrian Reber <adrian@lisas.de>

Good to hear

Thanks for testing!
Mark

Re: [PATCH] powerpc/wdrtas: Update wdrtas_get_interval to use rtas_data_buf

[Mark Nelson]

On Wed, 25 Mar 2009 01:31:32 am Utz Bacher wrote:
> Adrian Reber <adrian@lisas.de> wrote on 24.03.2009 13:31:31:
> > On Tue, Mar 24, 2009 at 05:30:41PM +1100, Mark Nelson wrote:
> > > The buffer passed to the ibm,get-system-parameter RTAS call must be
> > > in the RMA. To ensure we pass an address in the RMA use rtas_data_buf
> > > for the actual RTAS call and then copy the result to value. We can't
> > > just make it static because this can be compiled in as a module.
> > > 
> > > Also add the WDRTAS_SP_SPI_LEN so we don't litter '4' throughout the
> > > function.
> > > 
> > > Signed-off-by: Mark Nelson <markn@au1.ibm.com>
> > > ---
> > > 
> > > Adrian, does this patch cause any problems for your pxcabs?
> > 
> > No, it even helps. I have no tried the watchdog until now, but without 
> the
> > patch I get: 
> > 
> >    wdrtas: could not get sp_spi watchdog timeout (0). Continuing
> > 
> > and with the patch it reads the correct value. So only with your patch
> > it works like it is supposed to. Thanks!
> > 
> > Tested-by: Adrian Reber <adrian@lisas.de>
> 
> looks good to me.
> 
> Acked-by: Utz Bacher <utz.bacher@de.ibm.com>

Thanks for looking over it!
Mark