[PATCH v2] pinctrl: single: add big endian support

[Linus Walleij]

On Sat, Oct 10, 2026 at 7:57 PM Rustam Adilov <adilov at disroot.org> wrote:

> Add support for big endian pin controllers by taking into account
> a big-endian device tree property.
>
> This is needed on big endian MIPS platforms like RTL930X, RTL931X
> and RTL9607C who use pinctrl-single, because when SWAP_IO_SPACE is
> enabled, the mmio accessors in pinctrl-single perform a byte swap.
>
> Signed-off-by: Rustam Adilov <adilov at disroot.org>

I'm not so sure about expanding the usage of pinctrl-single, we have
said that this is used in the cases when you want to stash all the
hardware data into the device tree (such as which groups and functions
that exist) and it seems mostly done to save memory for the kernel
so you don't need to compile in that data for all systems.

The argument that this is "simple" if you just have some hardware
data in a file is moot these days IMO, because you can now just
give that data to an LLM and it will spit out any group and function
definitions you want.

So I'd like to hear some of the reasoning about why you want to
do this thing, we have other drivers doing this but without using
pinctrl-single.

One of the driver maintainers have to ACK it too.

> +static unsigned int pcs_readw_be(void __iomem *reg)
> +{
> +       return ioread16be(reg);
> +}
> +
> +static unsigned int pcs_readl_be(void __iomem *reg)
> +{
> +       return ioread32be(reg);
> +}
> +
> +static void pcs_writew_be(unsigned int val, void __iomem *reg)
> +{
> +       iowrite16be(val, reg);
> +}
> +
> +static void pcs_writel_be(unsigned int val, void __iomem *reg)
> +{
> +       iowrite32be(val, reg);
> +}

Why this indirection? Can't you just assign the standard
accessors directly?

>         case 16:
> -               pcs->read = pcs_readw;
> -               pcs->write = pcs_writew;
> +               if (byte_be) {
> +                       pcs->read = pcs_readw_be;
> +                       pcs->write = pcs_writew_be;
> +               } else {
> +                       pcs->read = pcs_readw;
> +                       pcs->write = pcs_writew;

Maybe they should all do that actually, can't we rewrite
this so we just:

if (byte_be) {
  pcs->read = ioread16be;
  pcs->write = iowrite16be;
} else {
  pcs->read = ioread16;
  pcs->write = iowrite16;

I don't see how the wrappers help and I hope the compiler
just optimize them away anyway.

Yours,
Linus Walleij