[U-Boot] I/O accessors on SuperH and endianness

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

Hello,

Has anyone any comments/suggestions on the below questions? How is this
problem solved on other architectures?

Thanks,

Thomas

On Mon, 28 Aug 2017 14:32:49 +0200, Thomas Petazzoni wrote:
> Hello,
> 
> As you've noticed, I'm porting U-Boot to a SH4 board running
> big-endian. The big-endian choice cannot be changed, because it's
> selected by the HW design: moving to little endian would require a
> modification of the board.
> 
> The serial_sh driver was working fine in big endian, with no change.
> However, the sh_eth driver was not working in big endian mode. After
> investigation, I realized that:
> 
>  - sh_serial is using the read/write (readb, writeb, readw, writew,
>    etc.) macros to access I/O registers
> 
>  - sh_eth is using the in/out macros to access I/O registers
> 
> The in/out macros assume the device registers are little endian, so
> when the CPU is running big endian, they do an endianness conversion.
> However, on SuperH, when the CPU runs big endian, the device registers
> are also big endian, so there should be no endianness conversion.
> 
> On the other hand, read/write, when __mem_pci is not defined, do not do
> any endianness conversion. And this is why sh_serial was working out of
> the box. Changing sh_eth to use read/write instead of in/out also made
> it work in big endian mode.
> 
> However, if for some reason I enable PCI on this platform, __mem_pci
> will be defined, and read/write will perform endianness conversion,
> breaking support for the platform.
> 
> So what is the appropriate solution here? Use read/write like sh_serial
> is doing today, and ignore the potential problem? Use __raw_*()
> variants everywhere? What if a driver is shared with another
> platform/architecture where the devices remain little endian even if
> the CPU is running big endian?
> 
> Best regards,
> 
> Thomas
> 
> For the record, here is the current patch I have on sh_eth (a few other
> changes are needed, but not directly related) :
> 
> diff --git a/drivers/net/sh_eth.h b/drivers/net/sh_eth.h
> index a09a6d7..0e65f97 100644
> --- a/drivers/net/sh_eth.h
> +++ b/drivers/net/sh_eth.h
> @@ -675,11 +675,11 @@ static inline unsigned long sh_eth_reg_addr(struct sh_eth_dev *eth,
>  static inline void sh_eth_write(struct sh_eth_dev *eth, unsigned long data,
>                                 int enum_index)
>  {
> -       outl(data, sh_eth_reg_addr(eth, enum_index));
> +       writel(data, sh_eth_reg_addr(eth, enum_index));
>  }
>  
>  static inline unsigned long sh_eth_read(struct sh_eth_dev *eth,
>                                         int enum_index)
>  {
> -       return inl(sh_eth_reg_addr(eth, enum_index));
> +       return readl(sh_eth_reg_addr(eth, enum_index));
>  }
> 
> 
> 



-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com