Thread (39 messages) 39 messages, 5 authors, 2021-03-31

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

From: Andy Shevchenko <hidden>
Date: 2021-03-19 21:25:45
Also in: linux-gpio, lkml

On Fri, Mar 19, 2021 at 11:20 PM Sander Vanheule [off-list ref] wrote:
On Fri, 2021-03-19 at 19:57 +0200, Andy Shevchenko wrote:
quoted
On Fri, Mar 19, 2021 at 5:51 PM Sander Vanheule
[off-list ref] wrote:
quoted
On Wed, 2021-03-17 at 15:08 +0200, Andy Shevchenko wrote:
quoted
On Mon, Mar 15, 2021 at 11:11 PM Sander Vanheule <
sander@svanheule.net> wrote:
...
quoted
quoted
quoted
quoted
+       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.
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)?

-- 
With Best Regards,
Andy Shevchenko
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help