[PATCH 1/1] ipr: add writeq definition if needed

[PATCH 1/1] ipr: add writeq definition if needed

[Wayne Boyer]

Compiling the driver will fail on 32 bit powerpc and other
architectures where writeq is not defined.  This patch adds a
definition for writeq.

Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
---

 drivers/scsi/ipr.h |    8 ++++++++
 1 file changed, 8 insertions(+)

Index: b/drivers/scsi/ipr.h
===================================================================
--- a/drivers/scsi/ipr.h	2010-06-08 10:06:48.000000000 -0700
+++ b/drivers/scsi/ipr.h	2010-06-08 15:14:42.000000000 -0700
@@ -1860,4 +1860,12 @@ static inline int ipr_sdt_is_fmt2(u32 sd
 	return 0;
 }

+#ifndef writeq
+static inline void writeq(u64 val, void __iomem *addr)
+{
+        writel(((u32) (val >> 32)), addr);
+        writel(((u32) (val)), (addr + 4));
+}
 #endif
+
+#endif /* _IPR_H */

Re: [PATCH 1/1] ipr: add writeq definition if needed

[Brian King]

Acked-by: Brian King <brking@linux.vnet.ibm.com>

On 06/09/2010 10:24 AM, Wayne Boyer wrote:
> Compiling the driver will fail on 32 bit powerpc and other
> architectures where writeq is not defined.  This patch adds a
> definition for writeq.
> 
> Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
> ---
> 
>  drivers/scsi/ipr.h |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> Index: b/drivers/scsi/ipr.h
> ===================================================================
> --- a/drivers/scsi/ipr.h	2010-06-08 10:06:48.000000000 -0700
> +++ b/drivers/scsi/ipr.h	2010-06-08 15:14:42.000000000 -0700
> @@ -1860,4 +1860,12 @@ static inline int ipr_sdt_is_fmt2(u32 sd
>  	return 0;
>  }
> 
> +#ifndef writeq
> +static inline void writeq(u64 val, void __iomem *addr)
> +{
> +        writel(((u32) (val >> 32)), addr);
> +        writel(((u32) (val)), (addr + 4));
> +}
>  #endif
> +
> +#endif /* _IPR_H */
> 


-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center

Re: [PATCH 1/1] ipr: add writeq definition if needed

[James Bottomley]

On Wed, 2010-06-09 at 08:24 -0700, Wayne Boyer wrote:
> Compiling the driver will fail on 32 bit powerpc and other
> architectures where writeq is not defined.  This patch adds a
> definition for writeq.
> 
> Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
> ---
> 
>  drivers/scsi/ipr.h |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> Index: b/drivers/scsi/ipr.h
> ===================================================================
> --- a/drivers/scsi/ipr.h	2010-06-08 10:06:48.000000000 -0700
> +++ b/drivers/scsi/ipr.h	2010-06-08 15:14:42.000000000 -0700
> @@ -1860,4 +1860,12 @@ static inline int ipr_sdt_is_fmt2(u32 sd
>  	return 0;
>  }
> 
> +#ifndef writeq
> +static inline void writeq(u64 val, void __iomem *addr)
> +{
> +        writel(((u32) (val >> 32)), addr);
> +        writel(((u32) (val)), (addr + 4));

Are you sure this is what you want?  The point about writeq is to do an
atomic write of a 64 bit quantity.  Doing two writel's loses the
atomicity (and can confuse some hardware if not done in the right
order).  If this code above works correctly, why bother with the writeq?

James

Re: [PATCH 1/1] ipr: add writeq definition if needed

[Wayne Boyer]

On 06/09/2010 09:20 AM, James Bottomley wrote:
> On Wed, 2010-06-09 at 08:24 -0700, Wayne Boyer wrote:
>> Compiling the driver will fail on 32 bit powerpc and other
>> architectures where writeq is not defined.  This patch adds a
>> definition for writeq.
>>
>> Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
>> ---
>>
>>  drivers/scsi/ipr.h |    8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> Index: b/drivers/scsi/ipr.h
>> ===================================================================
>> --- a/drivers/scsi/ipr.h	2010-06-08 10:06:48.000000000 -0700
>> +++ b/drivers/scsi/ipr.h	2010-06-08 15:14:42.000000000 -0700
>> @@ -1860,4 +1860,12 @@ static inline int ipr_sdt_is_fmt2(u32 sd
>>  	return 0;
>>  }
>>
>> +#ifndef writeq
>> +static inline void writeq(u64 val, void __iomem *addr)
>> +{
>> +        writel(((u32) (val >> 32)), addr);
>> +        writel(((u32) (val)), (addr + 4));
> 
> Are you sure this is what you want?  The point about writeq is to do an
> atomic write of a 64 bit quantity.  Doing two writel's loses the
> atomicity (and can confuse some hardware if not done in the right
> order).  If this code above works correctly, why bother with the writeq?
> 

We do want to issue 8 byte MMIO writes where we can for the new adapters
for better performance.  We want to use writeq on platforms that support
it.  I did test the driver using this define with the new hardware and it
does work as expected.

-- 
Wayne Boyer
IBM - Beaverton, Oregon
LTC S/W Development - eServerIO
(503) 578-5236, T/L 775-5236

Re: [PATCH 1/1] ipr: add writeq definition if needed

[James Bottomley]

On Wed, 2010-06-09 at 10:45 -0700, Wayne Boyer wrote:
> On 06/09/2010 09:20 AM, James Bottomley wrote:
> > On Wed, 2010-06-09 at 08:24 -0700, Wayne Boyer wrote:
> >> Compiling the driver will fail on 32 bit powerpc and other
> >> architectures where writeq is not defined.  This patch adds a
> >> definition for writeq.
> >>
> >> Signed-off-by: Wayne Boyer <wayneb@linux.vnet.ibm.com>
> >> ---
> >>
> >>  drivers/scsi/ipr.h |    8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> Index: b/drivers/scsi/ipr.h
> >> ===================================================================
> >> --- a/drivers/scsi/ipr.h	2010-06-08 10:06:48.000000000 -0700
> >> +++ b/drivers/scsi/ipr.h	2010-06-08 15:14:42.000000000 -0700
> >> @@ -1860,4 +1860,12 @@ static inline int ipr_sdt_is_fmt2(u32 sd
> >>  	return 0;
> >>  }
> >>
> >> +#ifndef writeq
> >> +static inline void writeq(u64 val, void __iomem *addr)
> >> +{
> >> +        writel(((u32) (val >> 32)), addr);
> >> +        writel(((u32) (val)), (addr + 4));
> > 
> > Are you sure this is what you want?  The point about writeq is to do an
> > atomic write of a 64 bit quantity.  Doing two writel's loses the
> > atomicity (and can confuse some hardware if not done in the right
> > order).  If this code above works correctly, why bother with the writeq?
> > 
> 
> We do want to issue 8 byte MMIO writes where we can for the new adapters
> for better performance.  We want to use writeq on platforms that support
> it.  I did test the driver using this define with the new hardware and it
> does work as expected.

OK, that's fine, just checking.

I suppose we should have some type of writeq_nonatomic() so I don't have
to check, but this is the first I've seen of these, so it's likely not
very common.

James