Patchwork powerpc/wdrtas: Update wdrtas_get_interval to use rtas_data_buf

login
register
mail settings
Submitter Mark Nelson
Date March 24, 2009, 6:30 a.m.
Message ID <200903241730.41711.markn@au1.ibm.com>
Download mbox | patch
Permalink /patch/24941/
State Accepted
Commit b6966b1be568e83a48585369731f1123a6f85157
Delegated to: Paul Mackerras
Headers show

Comments

Mark Nelson - March 24, 2009, 6:30 a.m.
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(-)
Adrian Reber - March 24, 2009, 12:31 p.m.
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
Utz Bacher - March 24, 2009, 2:31 p.m.
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
Mark Nelson - March 24, 2009, 10:51 p.m.
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
Mark Nelson - March 24, 2009, 10:53 p.m.
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

Patch

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);