Hi All,
Here is the issue which we are observing when driver don't use
le16_to_cpu() in below code snippet on Sparc64 machine when driver is
reading 2 bytes of data which is posted by HBA firmware,
u32 reply1;
reply1 = readl(&ioc->chip->Doorbell);
reply[1] = (reply1 & MPI2_DOORBELL_DATA_MASK);
printk("LSI debug.. 0x%x, 0x%x, 0x%x \n", reply1, reply[1]);
writel(0, &ioc->chip->HostInterruptStatus);
printk("LSI MsgLength :%d\n", default_reply->MsgLength);
When I execute above code I got below output on Sparc64 machine,
LSI debug.. 0x1c000311, 0x311
LSI MsgLength :3
When I execute same code in x86 machine then I got below output,
LSI debug.. 0x1c000311, 0x311
LSI MsgLength :17
Correct message (Here I am referring IOCFacts message) Length is 17
words. But on Sparc64 machine we got message length as 3 words which
is wrong.
Here is data structure of default reply message,
typedef struct _MPI2_DEFAULT_REPLY {
U16 FunctionDependent1; /*0x00 */
U8 MsgLength; /*0x02 */
U8 Function; /*0x03 */
U16 FunctionDependent2; /*0x04 */
U8 FunctionDependent3; /*0x06 */
U8 MsgFlags; /*0x07 */
U8 VP_ID; /*0x08 */
U8 VF_ID; /*0x09 */
U16 Reserved1; /*0x0A */
U16 FunctionDependent5; /*0x0C */
U16 IOCStatus; /*0x0E */
U32 IOCLogInfo; /*0x10 */
} MPI2_DEFAULT_REPLY, *PTR_MPI2_DEFAULT_REPLY,
MPI2DefaultReply_t, *pMPI2DefaultReply_t;
Until host reads correct number of reply words, IOC won't clear
Doorbel Used bit and hence we see below error message while loading
the driver and IOC initialization fails.
Jun 28 02:21:57 localhost kernel: mpt4sas_cm0: _base_get_ioc_facts
Jun 28 02:21:57 localhost kernel: mpt4sas_cm0: _base_wait_for_iocstate
Jun 28 02:21:57 localhost kernel: mpt4sas_cm0: doorbell is in use (line=5241)
Jun 28 02:21:57 localhost kernel: mpt4sas_cm0: _base_get_ioc_facts:
handshake failed (r=-14)
Jun 28 02:21:57 localhost kernel: mpt4sas_cm0: mpt3sas_base_free_resources
Jun 28 02:21:57 localhost kernel: mpt4sas_cm0: _base_make_ioc_ready
Jun 28 02:21:57 localhost kernel: mpt4sas_cm0: mpt3sas_base_unmap_resources
Jun 28 02:21:57 localhost kernel: mpt4sas_cm0: _base_release_memory_pools
Jun 28 02:21:57 localhost kernel: mpt4sas_cm0: failure at
/home/chaitra/mpt3sas_with_sparse_patch/mpt3sas_scsih.c:10776/_scsih_probe()
Thanks,
Sreekanth
On Sat, Jun 30, 2018 at 12:06 AM, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Fri, Jun 29, 2018 at 7:06 PM, James Bottomley
> <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
>> On Fri, 2018-06-29 at 10:58 -0400, Chaitra P B wrote:
>>> "scsi: mpt3sas: Bug fix for big endian systems"
>>>
>>> Above patch with commit id "cf6bf9710cabba1fe94a4349f4eb8db623c77ebc"
>>> was posted to fix sparse warnings. While posting this patch it was
>>> assumed that readl() & writel() APIs internally calls le32_to_cpu() &
>>> cpu_to_le32() APIs respectively. Looks like it is not true for all
>>> architecture
>>
>> Just a minute, it damn well should be. The definition of readl/writel
>> is barriers and little endian (you can see this in asm-generic/io.h).
>>
>> Which architecture is getting this wrong? Because it sounds like
>> that's what we need to fix rather than doing something like this in all
>> drivers.
>>
>> Sparc (and parisc) definitely do the little endian thing, so if this
>> code is what it takes to get them working again, it looks like you're
>> double swapping somewhere. I really think cf6bf9710c needs to be
>> reverted and you should look again at your sparse warnings. Since the
>> driver is using the non-raw readX/writeX it should be cpu endian
>> internally which cf6bf9710c upsets.
>
> And we definitely won't see the constructions like
> writeq(cpu_to_le64()) in the code, because it's weird.
> If I get it correctly it's equivalent to __raw_writeq().
>
> --
> With Best Regards,
> Andy Shevchenko
