Re: [PATCH v2 2/4] litex: add common LiteX header

[Greg Kroah-Hartman]

On Wed, Oct 23, 2019 at 11:47:04AM +0200, Mateusz Holenko wrote:
> +#ifdef __LITTLE_ENDIAN
> +# define LITEX_READ_REG(addr)                  ioread32(addr)
> +# define LITEX_READ_REG_OFF(addr, off)         ioread32(addr + off)
> +# define LITEX_WRITE_REG(val, addr)            iowrite32(val, addr)
> +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32(val, addr + off)
> +#else
> +# define LITEX_READ_REG(addr)                  ioread32be(addr)
> +# define LITEX_READ_REG_OFF(addr, off)         ioread32be(addr + off)
> +# define LITEX_WRITE_REG(val, addr)            iowrite32be(val, addr)
> +# define LITEX_WRITE_REG_OFF(val, addr, off)   iowrite32be(val, addr + off)
> +#endif

I just noticed this.

Ick, this is not good.  You will run into problems in the future with
this, I can guarantee it.  What about systems where the CPU is one
endian and the hardware in the other?  It will happen trust us.

Make these real functions (inline is nice) and pass in the pointer to
the device so you can test for it and call the correct function based on
the cpu/hardware type.

And what about bitfields?  What endian are they for your
system/hardware?

Almost no kernel code should EVER be testing __LITTLE_ENDIAN, don't add
to it as it is not a good idea.

thanks,

greg k-h