| Message ID | 1533015396-10401-1-git-send-email-sreekanth.reddy@broadcom.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v1,RESEND] mpt3sas: Swap I/O memory read value back to cpu endianness | expand |
Andy, Please review the changes Sreekanth made to address your feedback. > Swap the I/O memory read value back to cpu endianness before storing it > in a data structures which are defined in the MPI headers where > u8 components are not defined in the endianness order. > > In this area from day one mpt3sas driver is using le32_to_cpu() & > cpu_to_le32() APIs. But in the patch cf6bf9710c > (mpt3sas: Bug fix for big endian systems) we have removed these APIs > before reading I/O memory which we should haven't done it. So > in this patch I am correcting it by adding these APIs back > before accessing I/O memory. > > v1: Changelog: > Replaced writeq API with __raw_writeq() & mmiowb() APIs.
On Thu, 2018-08-02 at 16:16 -0400, Martin K. Petersen wrote: > Andy, > > Please review the changes Sreekanth made to address your feedback. From my point of view they look good. I assume Sreekanth tested them on real hardware and as I remember davem@ actually the one who possesses sparc machine with this hardware. > > > Swap the I/O memory read value back to cpu endianness before storing > > it > > in a data structures which are defined in the MPI headers where > > u8 components are not defined in the endianness order. > > > > In this area from day one mpt3sas driver is using le32_to_cpu() & > > cpu_to_le32() APIs. But in the patch cf6bf9710c > > (mpt3sas: Bug fix for big endian systems) we have removed these APIs > > before reading I/O memory which we should haven't done it. So > > in this patch I am correcting it by adding these APIs back > > before accessing I/O memory. > > > > v1: Changelog: > > Replaced writeq API with __raw_writeq() & mmiowb() APIs. > >
On Fri, Aug 3, 2018 at 3:07 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Thu, 2018-08-02 at 16:16 -0400, Martin K. Petersen wrote: >> Andy, >> >> Please review the changes Sreekanth made to address your feedback. > > From my point of view they look good. > > I assume Sreekanth tested them on real hardware and as I remember davem@ > actually the one who possesses sparc machine with this hardware. Yes I have tested this patch changes on SPARC64 system and it is working fine. Thanks, Sreekanth > >> >> > Swap the I/O memory read value back to cpu endianness before storing >> > it >> > in a data structures which are defined in the MPI headers where >> > u8 components are not defined in the endianness order. >> > >> > In this area from day one mpt3sas driver is using le32_to_cpu() & >> > cpu_to_le32() APIs. But in the patch cf6bf9710c >> > (mpt3sas: Bug fix for big endian systems) we have removed these APIs >> > before reading I/O memory which we should haven't done it. So >> > in this patch I am correcting it by adding these APIs back >> > before accessing I/O memory. >> > >> > v1: Changelog: >> > Replaced writeq API with __raw_writeq() & mmiowb() APIs. >> >> > > -- > Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Intel Finland Oy
On Fri, 2018-08-03 at 16:04 +0530, Sreekanth Reddy wrote: > On Fri, Aug 3, 2018 at 3:07 PM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, 2018-08-02 at 16:16 -0400, Martin K. Petersen wrote: > > > Andy, > > > > > > Please review the changes Sreekanth made to address your feedback. > > > > From my point of view they look good. > > > > I assume Sreekanth tested them on real hardware and as I remember > > davem@ > > actually the one who possesses sparc machine with this hardware. > > Yes I have tested this patch changes on SPARC64 system and it is > working fine. > Thanks for confirming. From code prospective Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Thanks, > Sreekanth > > > > > > > > > > Swap the I/O memory read value back to cpu endianness before > > > > storing > > > > it > > > > in a data structures which are defined in the MPI headers where > > > > u8 components are not defined in the endianness order. > > > > > > > > In this area from day one mpt3sas driver is using le32_to_cpu() > > > > & > > > > cpu_to_le32() APIs. But in the patch cf6bf9710c > > > > (mpt3sas: Bug fix for big endian systems) we have removed these > > > > APIs > > > > before reading I/O memory which we should haven't done it. So > > > > in this patch I am correcting it by adding these APIs back > > > > before accessing I/O memory. > > > > > > > > v1: Changelog: > > > > Replaced writeq API with __raw_writeq() & mmiowb() APIs. > > > > > > > > > > -- > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Intel Finland Oy
From: Sreekanth Reddy <sreekanth.reddy@broadcom.com> Date: Fri, 3 Aug 2018 16:04:15 +0530 > On Fri, Aug 3, 2018 at 3:07 PM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: >> On Thu, 2018-08-02 at 16:16 -0400, Martin K. Petersen wrote: >>> Andy, >>> >>> Please review the changes Sreekanth made to address your feedback. >> >> From my point of view they look good. >> >> I assume Sreekanth tested them on real hardware and as I remember davem@ >> actually the one who possesses sparc machine with this hardware. > > Yes I have tested this patch changes on SPARC64 system and it is working fine. I don't have access to my sparc64 hardware at the moment, so please don't wait for my testing in order to merge this fix. Thanks!
Sreekanth, > Swap the I/O memory read value back to cpu endianness before storing > it in a data structures which are defined in the MPI headers where u8 > components are not defined in the endianness order. Applied to 4.18/scsi-fixes, thanks!
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index 94359d8..c75e88a 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -3350,11 +3350,10 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc, spinlock_t *writeq_lock) { unsigned long flags; - __u64 data_out = b; spin_lock_irqsave(writeq_lock, flags); - writel((u32)(data_out), addr); - writel((u32)(data_out >> 32), (addr + 4)); + __raw_writel((u32)(b), addr); + __raw_writel((u32)(b >> 32), (addr + 4)); mmiowb(); spin_unlock_irqrestore(writeq_lock, flags); } @@ -3374,7 +3373,8 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc, static inline void _base_writeq(__u64 b, volatile void __iomem *addr, spinlock_t *writeq_lock) { - writeq(b, addr); + __raw_writeq(b, addr); + mmiowb(); } #else static inline void @@ -5275,7 +5275,7 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc, /* send message 32-bits at a time */ for (i = 0, failed = 0; i < request_bytes/4 && !failed; i++) { - writel((u32)(request[i]), &ioc->chip->Doorbell); + writel(cpu_to_le32(request[i]), &ioc->chip->Doorbell); if ((_base_wait_for_doorbell_ack(ioc, 5))) failed = 1; } @@ -5296,7 +5296,7 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc, } /* read the first two 16-bits, it gives the total length of the reply */ - reply[0] = (u16)(readl(&ioc->chip->Doorbell) + reply[0] = le16_to_cpu(readl(&ioc->chip->Doorbell) & MPI2_DOORBELL_DATA_MASK); writel(0, &ioc->chip->HostInterruptStatus); if ((_base_wait_for_doorbell_int(ioc, 5))) { @@ -5305,7 +5305,7 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc, ioc->name, __LINE__); return -EFAULT; } - reply[1] = (u16)(readl(&ioc->chip->Doorbell) + reply[1] = le16_to_cpu(readl(&ioc->chip->Doorbell) & MPI2_DOORBELL_DATA_MASK); writel(0, &ioc->chip->HostInterruptStatus); @@ -5319,7 +5319,7 @@ void mpt3sas_base_clear_st(struct MPT3SAS_ADAPTER *ioc, if (i >= reply_bytes/2) /* overflow case */ readl(&ioc->chip->Doorbell); else - reply[i] = (u16)(readl(&ioc->chip->Doorbell) + reply[i] = le16_to_cpu(readl(&ioc->chip->Doorbell) & MPI2_DOORBELL_DATA_MASK); writel(0, &ioc->chip->HostInterruptStatus); }
Swap the I/O memory read value back to cpu endianness before storing it in a data structures which are defined in the MPI headers where u8 components are not defined in the endianness order. In this area from day one mpt3sas driver is using le32_to_cpu() & cpu_to_le32() APIs. But in the patch cf6bf9710c (mpt3sas: Bug fix for big endian systems) we have removed these APIs before reading I/O memory which we should haven't done it. So in this patch I am correcting it by adding these APIs back before accessing I/O memory. v1: Changelog: Replaced writeq API with __raw_writeq() & mmiowb() APIs. Signed-off-by: Sreekanth Reddy <sreekanth.reddy@broadcom.com> --- drivers/scsi/mpt3sas/mpt3sas_base.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)