On Fri, 2021-03-19 at 19:57 +0200, Andy Shevchenko wrote: > On Fri, Mar 19, 2021 at 5:51 PM Sander Vanheule > <san...@svanheule.net> wrote: > > On Wed, 2021-03-17 at 15:08 +0200, Andy Shevchenko wrote: > > > On Mon, Mar 15, 2021 at 11:11 PM Sander Vanheule < > > > san...@svanheule.net> wrote: > > ... > > > > > +#include <linux/swab.h> > > > > > > Not sure why you need this? See below. > > > > > + return swab32(readl(ctrl->base + > > > > REALTEK_GPIO_REG_ISR)); > > > > > > Why swab?! How is this supposed to work on BE CPUs? > > > Ditto for all swabXX() usage. > > > > My use of swab32/swahw32 has little to do with the CPU being BE or > > LE, > > but more with the register packing in the GPIO peripheral. > > > > The supported SoCs have port layout A-B-C-D in the registers, where > > firmware built with Realtek's SDK always denotes A0 as the first > > GPIO > > line. So bit 24 in a register has the value for A0 (with the > > exception > > of the IMR register). > > > > I wrote these wrapper functions to be able to use the BIT() macro > > with > > the GPIO line number, similar to how gpio-mmio uses ioread32be() > > when > > the BGPIOF_BIG_ENDIAN_BYTE_ORDER flag is used. > > > > For the IMR register, port A again comes first, but is now 16 bits > > wide > > instead of 8, with A0 at bits 16:17. That's why swahw32 is used for > > this register. > > > > On the currently unsupported RTL9300-series, the port layout is > > reversed: D-C-B-A. GPIO line A0 is then at bit 0, so the swapping > > functions won't be required. When support for this alternate port > > layout is added, some code will need to be added to differentiate > > between the two cases. > > Yes, you have different endianess on the hardware level, why not to > use the proper accessors (with or without utilization of the above > mentioned BGPIOF_BIG_ENDIAN_BYTE_ORDER)?
The point I was trying to make, is that it isn't an endianess issue. I shouldn't have used a register with single byte values to try to illustrate that. Consider instead the interrupt masking registers. To write the IMR bits for port A (GPIO 0-7), a 16-bit value must be written. This value (e.g. u16 port_a_imr) is always BE, independent of the packing order of the ports in the registers: // On RTL8380: port A is in the upper word writew(port_a_imr, base + OFFSET_IMR_AB); // On RTL9300: port A is in the lower word writew(port_a_imr, base + OFFSET_IMR_AB + 2); I want the low GPIO lines to be in the lower half-word, so I can manipulate GPIO lines 0-15 with simple mask and shift operations. It just so happens, that all registers needed by bgpio_init contain single-byte values. With BGPIO_BIG_ENDIAN_BYTE_ORDER the port order is reversed as required, but it's a bit of a misnomer here. Best, Sander
