MHI MMIO endian issue

MHI MMIO endian issue

[Martin Fäcknitz]

Hello,

the function mhi_init_mmio configures MMIO registers using the reg_info
table:

  eg_info[] = {
    {
	  CCABAP_HIGHER,
      upper_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr),
	},
	...

  mhi_write_reg(mhi_cntrl, base, reg_info[i].offset,
    reg_info[i].val);

However, the values are in host byte orders, not in device byte order. I
have a PPC64 system which is big endian, but the device is little
endian. Without wrapping each value in eg_info using cpu_to_le32 the
device (sdx55) doesn't response:

  cpu_to_le32(upper_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr))

Hard to believe i am the first one which has this issue? Let me know if
i should make a patch.

-- 
Martin Fäcknitz

hotsplots GmbH
Rotherstr. 22
10245 Berlin
Fon: +49 (0)30 29 77 348-0
Fax: +49 (0)30 29 77 348-99
E-Mail: faecknitz@hotsplots.de

Amtsgericht Charlottenburg HRB 93460B
Geschäftsführung: Dipl. Ing. Sascha Schaub

Re: MHI MMIO endian issue

[Mayank Rana]

Hi Martin

On 4/30/2024 2:30 PM, Martin Fäcknitz wrote:
> Hello,
> 
> the function mhi_init_mmio configures MMIO registers using the reg_info
> table:
> 
>    eg_info[] = {
>      {
> 	  CCABAP_HIGHER,
>        upper_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr),
> 	},
> 	...
> 
>    mhi_write_reg(mhi_cntrl, base, reg_info[i].offset,
>      reg_info[i].val);
> 
> However, the values are in host byte orders, not in device byte order. I
> have a PPC64 system which is big endian, but the device is little
> endian. Without wrapping each value in eg_info using cpu_to_le32 the
> device (sdx55) doesn't response:
> 
>    cpu_to_le32(upper_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr))
> 
> Hard to believe i am the first one which has this issue? Let me know if
> i should make a patch.
Yes, Please. At least I am not aware of running Linux MHI host stack on 
big endian architecture.

Regards,
Mayank

Re: MHI MMIO endian issue

[Manivannan Sadhasivam]

+ Paul Davey

On Tue, Apr 30, 2024 at 11:30:57PM +0200, Martin Fäcknitz wrote:
> Hello,
> 
> the function mhi_init_mmio configures MMIO registers using the reg_info
> table:
> 
>   eg_info[] = {
>     {
> 	  CCABAP_HIGHER,
>       upper_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr),
> 	},
> 	...
> 
>   mhi_write_reg(mhi_cntrl, base, reg_info[i].offset,
>     reg_info[i].val);
> 
> However, the values are in host byte orders, not in device byte order. I
> have a PPC64 system which is big endian, but the device is little
> endian. Without wrapping each value in eg_info using cpu_to_le32 the
> device (sdx55) doesn't response:
> 
>   cpu_to_le32(upper_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr))
> 
> Hard to believe i am the first one which has this issue? Let me know if
> i should make a patch.
> 

You are not the first one to try MHI on a big endian machine. Paul did that a
while ago and fixed some issues w.r.t endianess [1]. But I'm wondering how Paul
didn't hit the issue that you are seeing.

Anyway, I think the fix is to modify mhi_{read/write}_reg to accept '__le32'
for the 'val' parameter and fix all the callers.

I can cook a patch if needed, but won't be able to do so for atleast 2-3 weeks.

- Mani

[1] https://lore.kernel.org/mhi/20220212182117.49438-3-manivannan.sadhasivam@linaro.org/

> -- 
> Martin Fäcknitz
> 
> hotsplots GmbH
> Rotherstr. 22
> 10245 Berlin
> Fon: +49 (0)30 29 77 348-0
> Fax: +49 (0)30 29 77 348-99
> E-Mail: faecknitz@hotsplots.de
> 
> Amtsgericht Charlottenburg HRB 93460B
> Geschäftsführung: Dipl. Ing. Sascha Schaub

-- 
மணிவண்ணன் சதாசிவம்

Re: MHI MMIO endian issue

[Paul Davey]

On Wed, 2024-05-01 at 12:25 +0530, Manivannan Sadhasivam wrote:
> 
> You are not the first one to try MHI on a big endian machine. Paul
> did that a
> while ago and fixed some issues w.r.t endianess [1]. But I'm
> wondering how Paul
> didn't hit the issue that you are seeing.
> 
I suspect the reason I did not see this is that the platform I was
using the MHI stack on has some support for hardware endianess
translation in PCIe BARs which was likely masking the difference for
the MMIO registers but did not do anything about the host RAM accesses
from the MHI modem.

I am not sure how prevalent that sort of feature is or how it is best
to deal with MMIO access if it may be enabled, does this come up with
any other PCIe devices?  Some of the atheros ethernet NIC drivers (at
least atl1e and atl1c) appear to translate dma buffer addresses and
lengths to little endian in descriptors but not other fields in those
descriptors and not any MMIO register accesses.
> 
> [1]
> https://lore.kernel.org/mhi/20220212182117.49438-3-manivannan.sadhasivam@linaro.org/
> 
> 

Thanks,
Paul