Re: [PATCH net-next 6/9] net: phy: add backplane kr driver support

[Florinel Iordache <florinel.iordache@xxxxxxx>]

> On 3/26/2020 6:07 PM, Andrew Lunn wrote:
> >> +static u32 le_ioread32(void __iomem *reg) {
> >> +    return ioread32(reg);
> >> +}
> >> +
> >> +static void le_iowrite32(u32 value, void __iomem *reg) {
> >> +    iowrite32(value, reg);
> >> +}
> >> +
> >> +static u32 be_ioread32(void __iomem *reg) {
> >> +    return ioread32be(reg);
> >> +}
> >> +
> >> +static void be_iowrite32(u32 value, void __iomem *reg) {
> >> +    iowrite32be(value, reg);
> >> +}
> >
> > This is very surprising to me. I've not got my head around the
> > structure of this code yet, but i'm surprised to see memory mapped
> > access functions in generic code.
> 
> This abstraction makes no sense whatsoever, you already have
> io{read,write}32{be,} to deal with the correct endian, and you can use the
> standard Device Tree properties 'big-endian', 'little-endian', 'native-endian' to
> decide which of those of to use. If you need to introduce a wrapper or indirect
> function calls to select the correct I/O accessor, that is fine of course.
> --
> Florian

Hi Florian,
I need these wrappers in generic code in order to automatically assign the proper
I/O accessor in the following structure according to endianness specified in DT.

/* Endianness specific memory I/O */
struct mem_io {
	u32 (*read32)(void __iomem *addr);
	void (*write32)(u32 value, void __iomem *addr);
};

And then the usage is straightforward in device specific code:
io.read32(&reg_base->tcsr3) ...
io.write32((io.read32(&reg_base->tcsr1) ... 

without the need to check endianness at each call and select which read/write function to use.
This is done in order to reduce the overall number of LOC (lines of code).

Florin.