Hi,
Any suggestion/update over my previous mail. I am using 4.13 kernel.
Thanks,
Sreekanth
On Sat, Jun 30, 2018 at 12:34 AM, Sreekanth Reddy
<sreekanth.reddy@xxxxxxxxxxxx> wrote:
> 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
