On Wed, Jul 25, 2018 at 8:32 PM, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Wed, Jul 25, 2018 at 12:42 PM, Sreekanth Reddy
> <sreekanth.reddy@xxxxxxxxxxxx> wrote:
>> 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.
>
>> - __u64 data_out = b;
>> + __u64 data_out = cpu_to_le64(b);
>>
>> spin_lock_irqsave(writeq_lock, flags);
>> writel((u32)(data_out), addr);
>
> Wouldn't be the same as
>
> __raw_writel(data_out >> 0, addr);
> __raw_writel(data_out >> 32, addr + 4);
> mmiowb();
>
> ?
Yes, I can replace the writel() APIs with __raw_writel() with mmiowb()
memory barrier. Hoping this doesn't create any other side effects.
I will post new patch with this change tomorrow after doing some
testing in this area.
>
>> _base_writeq(__u64 b, volatile void __iomem *addr, spinlock_t *writeq_lock)
>> {
>> - writeq(b, addr);
>> + writeq(cpu_to_le64(b), addr);
>
> Similar here
> __raw_writeq(b, addr);
> mmiowb();
>
>
>> - writel((u32)(request[i]), &ioc->chip->Doorbell);
>> + writel(cpu_to_le32(request[i]), &ioc->chip->Doorbell);
>
> This kind of endianess play (including above) should make sparse unhappy.
>
> Did you run it with
>
> C=1 CF=-D__CHECK_ENDIAN__
>
> ?
Yes I run it on 4.18 kernel and I don't see any error or warning for
these lines.
Thanks,
Sreekanth
>
> --
> With Best Regards,
> Andy Shevchenko
