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