Re: [PATCH v2 2/2] gpio: Add Realtek Otto GPIO support

[Andy Shevchenko]

On Fri, Mar 19, 2021 at 11:48 PM Sander Vanheule <sander@xxxxxxxxxxxxx> wrote:
> On Fri, 2021-03-19 at 23:24 +0200, Andy Shevchenko wrote:
> > On Fri, Mar 19, 2021 at 11:20 PM Sander Vanheule <
> > sander@xxxxxxxxxxxxx> wrote:
> > > On Fri, 2021-03-19 at 19:57 +0200, Andy Shevchenko wrote:

...

> > > 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.
> >
> > How many registers (per GPIO / port) do you have?
> > Can you list them and show endianess of the data for each of them and
> > for old and new hardware (something like a 3 column table)?
>
> Each GPIO bank, with 32 GPIO lines, consists of four 8-line ports.
> There are seven registers per port, but only five are used:
>
>        |        | Data    | RTL8380    | RTL9300
> Reg    | Offset | type    | byte order | byte order
> -------+--------+---------+------------+-----------
> DIR    | 0x08   | 4 * u8  | A-B-C-D    | D-C-B-A
> DATA   | 0x0C   | 4 * u8  | A-B-C-D    | D-C-B-A
> ISR    | 0x10   | 4 * u8  | A-B-C-D    | D-C-B-A
> IMR_AB | 0x14   | 2 * u16 | A-A-B-B    | B-B-A-A
> IMR_CD | 0x18   | 2 * u16 | C-C-D-D    | D-D-C-C
>
> The unused other registers are all 4*u8.

You mean that they are following the same rules as DIR/DATA/ISR. right?

> A-B-C-D means:  (A << 24) | (B << 16) | (C << 8) | D
> A-A-B-B means:  (A << 16) | B

If the above is true for unused registers, it's clearly hardware endianness.

You need special treatment for IMR, but in general it follows the
logic behind the others.

So, you need some kind of I/O accessors like
 read_u8_reg()
 write_u8_reg()
 read_u16_reg()
 write_u16_reg()

And depending on endianess of hardware to call proper set of them.

-- 
With Best Regards,
Andy Shevchenko