On Wed, 2018-02-14 at 20:10 +0200, Andy Shevchenko wrote:
> Since we have a writeq() for 32-bit architectures as provided by IO
> non-atomic helpers, there is no need to open code it.
>
> Moreover sparse complains about this:
>
> drivers/scsi/mpt3sas/mpt3sas_base.c:2975:16: expected unsigned long
> long val
> drivers/scsi/mpt3sas/mpt3sas_base.c:2975:16: got restricted __le64
> <noident>
>
> Fixing this by replacing custom writeq() with one provided by
> io-64-nonatomic-lo-hi.h header.
>
> Reported-by: kbuild test robot <fengguang.wu@xxxxxxxxx>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> drivers/scsi/mpt3sas/mpt3sas_base.c | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c
> b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 59a87ca328d3..a92ab4a801d7 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
> @@ -56,6 +56,7 @@
> #include <linux/interrupt.h>
> #include <linux/dma-mapping.h>
> #include <linux/io.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> #include <linux/time.h>
> #include <linux/ktime.h>
> #include <linux/kthread.h>
> @@ -2968,16 +2969,9 @@ mpt3sas_base_free_smid(struct MPT3SAS_ADAPTER
> *ioc, u16 smid)
> * @writeq_lock: spin lock
> *
> * Glue for handling an atomic 64 bit word to MMIO. This special
> handling takes
> - * care of 32 bit environment where its not quarenteed to send the
> entire word
> + * care of 32 bit environment where its not guaranteed to send the
> entire word
> * in one transfer.
> */
> -#if defined(writeq) && defined(CONFIG_64BIT)
> -static inline void
> -_base_writeq(__u64 b, volatile void __iomem *addr, spinlock_t
> *writeq_lock)
> -{
> - writeq(cpu_to_le64(b), addr);
> -}
> -#else
> static inline void
> _base_writeq(__u64 b, volatile void __iomem *addr, spinlock_t
> *writeq_lock)
> {
> @@ -2985,11 +2979,9 @@ _base_writeq(__u64 b, volatile void __iomem
> *addr, spinlock_t *writeq_lock)
> __u64 data_out = cpu_to_le64(b);
>
> spin_lock_irqsave(writeq_lock, flags);
> - writel((u32)(data_out), addr);
> - writel((u32)(data_out >> 32), (addr + 4));
> + writeq(data_out, addr);
> spin_unlock_irqrestore(writeq_lock, flags);
> }
> -#endif
This would rather defeat the purpose of the original code, I think.
The coding seems to indicate that if the platform can do a writeq
atomically (i.e. it's 64 bit and has the primitive) then it should and
not take the hit of using a lock. Otherwise the 64 bit value is
updated by two 32 bit writes protected by a lock to ensure we don't get
interleaving of the write.
So I think you can replace the double writel with a lo_hi_writeq but
you have to lose the lock if the platform supports the atomic 64 bit
write for performance reasons.
James
