Thread (6 messages) 6 messages, 4 authors, 2026-02-10

Re: [PATCH] arm64: dts: mediatek: mt8189: Add pinmux macro header file

From: Chen-Yu Tsai <wens@kernel.org>
Date: 2026-02-10 16:21:55
Also in: linux-devicetree, linux-mediatek, lkml

On Tue, Feb 10, 2026 at 10:26 PM AngeloGioacchino Del Regno
[off-list ref] wrote:
Il 10/02/26 13:48, Chen-Yu Tsai ha scritto:
quoted
On Tue, Feb 10, 2026 at 7:03 PM AngeloGioacchino Del Regno
[off-list ref] wrote:
quoted
Il 09/02/26 22:48, David Lechner ha scritto:
quoted
On 9/18/25 9:03 PM, Cathy Xu wrote:
quoted
Add the pinctrl header file on MediaTek mt8189.

Signed-off-by: Cathy Xu <redacted>
---
This patch is base on the patch series:
https://patchwork.kernel.org/project/linux-mediatek/list/?series=981475
[1] dt-bindings: pinctrl: mediatek: Add support for mt8189
[2] arm64: dts: mediatek: mt8189: Add pinmux macro header file
[3] pinctrl: mediatek: Add pinctrl driver on mt8189
Since patch [1] and [3] of the series have already been merged, this
patch(patch [2]) is being resent individually after modifications.
---
   arch/arm64/boot/dts/mediatek/mt8189-pinfunc.h | 1125 +++++++++++++++++
   1 file changed, 1125 insertions(+)
   create mode 100644 arch/arm64/boot/dts/mediatek/mt8189-pinfunc.h
diff --git a/arch/arm64/boot/dts/mediatek/mt8189-pinfunc.h b/arch/arm64/boot/dts/mediatek/mt8189-pinfunc.h
new file mode 100644
index 000000000000..df69f50c267a
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt8189-pinfunc.h
General question:

Why do we have similar files in two different places different places?

$ ls arch/arm64/boot/dts/mediatek/*-pin*
arch/arm64/boot/dts/mediatek/mt2712-pinfunc.h
arch/arm64/boot/dts/mediatek/mt6878-pinfunc.h
arch/arm64/boot/dts/mediatek/mt6893-pinfunc.h
arch/arm64/boot/dts/mediatek/mt8167-pinfunc.h
arch/arm64/boot/dts/mediatek/mt8173-pinfunc.h
arch/arm64/boot/dts/mediatek/mt8196-pinfunc.h
arch/arm64/boot/dts/mediatek/mt8516-pinfunc.h

$ ls include/dt-bindings/pinctrl/mt*
include/dt-bindings/pinctrl/mt65xx.h
include/dt-bindings/pinctrl/mt6779-pinfunc.h
include/dt-bindings/pinctrl/mt6795-pinfunc.h
include/dt-bindings/pinctrl/mt6797-pinfunc.h
include/dt-bindings/pinctrl/mt7623-pinfunc.h
include/dt-bindings/pinctrl/mt8135-pinfunc.h
include/dt-bindings/pinctrl/mt8183-pinfunc.h
include/dt-bindings/pinctrl/mt8186-pinfunc.h
include/dt-bindings/pinctrl/mt8192-pinfunc.h
include/dt-bindings/pinctrl/mt8195-pinfunc.h
include/dt-bindings/pinctrl/mt8365-pinfunc.h


Plus one different naming pattern.

$ ls include/dt-bindings/pinctrl/mediatek,*
include/dt-bindings/pinctrl/mediatek,mt8188-pinfunc.h



Which one is preferred?
The MediaTek pinctrl must gain compatibility with standard pinctrl bindings. Until
then, bindings maintainers decided that these headers must go to the dts/mediatek
folder.

It is my desire to (but lack of time on my side hits hard) do the right thing and
make the MediaTek pinctrl drivers to actually "understand" standard bindings.
The headers encode the pin numbers and mux values in a way that the
"pinmux" property requires, all the while giving them meaningful names.

I suppose you could consider them part of the binding, as the pin controller
binding assembles all the individual PIO blocks in the SoC to produce one
unified view of all the pins. How they are ordered is important.

Plus the datasheets are horrible to read, as the pins aren't always numbered,
but are referred to using symbolic names like I2S2_MCLK.
quoted
I'd be - of course - happy if anyone else beats me on time (which wouldn't be hard
really) and pushes a series to fix this situation.

Just to be clear - right now, the MTK pinctrl DT looks like:

         panel_default_pins: panel-default-pins {
                 pins-rst {
                         pinmux = <PINMUX_GPIO108__FUNC_GPIO108>;
                         output-high;
                 };

                 pins-en {
                         pinmux = <PINMUX_GPIO48__FUNC_GPIO48>;
                         output-low;
                 };
         };

         spi1_pins: spi1-pins {
                 pins {
                         pinmux = <PINMUX_GPIO136__FUNC_SPIM1_CSB>,
                                  <PINMUX_GPIO137__FUNC_SPIM1_CLK>,
                                  <PINMUX_GPIO138__FUNC_SPIM1_MO>,
                                  <PINMUX_GPIO139__FUNC_SPIM1_MI>;
                         bias-disable;
                 };
         };
To be fair, the above is one valid kind of generic pinmux description.

 From Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml :

While not required to be used, there are 3 generic forms of pin muxing nodes
which pin controller devices can use.

For hardware where pin multiplexing configurations have to be specified for
each single pin the number of required sub-nodes containing "pin" and
"function" properties can quickly escalate and become hard to write and
maintain.

For cases like this, the pin controller driver may use the pinmux helper
property, where the pin identifier is provided with mux configuration settings
in a pinmux group. A pinmux group consists of the pin identifier and mux
settings represented as a single integer or an array of integers.

The pinmux property accepts an array of pinmux groups, each of them describing
a single pin multiplexing configuration.

  - end quote -

So Mediatek is following one of the generic pinmux bindings. It's not the
only one using this scheme either. STM32 and some of the Renesas platforms
also follow it.
Not saying that MediaTek is the only one that uses such bindings style, at all.

I admit I was too tough about that, but as of the current state, the *binding*
is not generic, and it's strictly tied to the GPIO Controller IP version of one
specific SoC.

While this style is generic, the actual pinmux *definitions* in the header are
not generic - that's what I wanted to say, and I admit I went a bit too vague
with words that are easy to misunderstand.
quoted
quoted
....but the driver should gain compatibility with nodes which would look like:

         panel_default_pins: panel-default-pins {
                 pins-rst {
                         pins = "gpio108";
                         function = "gpio";
                         output-high;
                 };

                 pins-en {
                         pins = "gpio48";
                         function = "gpio";
                         output-low;
                 };
         };

         spi1_pins: spi1-pins {
                 pins-bus {
                         pins = "gpio136", "gpio137", "gpio138", "gpio139",
                         function = "spi_m1";
Why is it "spi_m1", not "spi1"?
PINMUX_GPIO138__FUNC_SPIM1_MO -> s/PINMUX_GPIO138__FUNC_//g/ and s/_MO//g

M1 stands for "Master 1" - that's because technically there could be a different
pinfunc for SPI "Slave 1" function.

That's SoC-specific anyway, not all of them have SPIS1, not all of them need
a different function, and... you get the point, I'm sure :-)
quoted
Honestly you likely don't want this, or rather you don't want a huge table
of pins and pinmux values and strings in the kernel. It takes a lot of time
to write, even more time to review, and takes up a lot of space for each
pinctrl driver. And those are generally built-in.

The Allwinner platform has gone in the reverse direction: instead of having
a huge table, we put the mux value in the DT using a custom property.
See the following for discussions:

https://patchwork.ozlabs.org/project/linux-gpio/cover/20171113012523.2328-1-andre.przywara@arm.com/
https://patchwork.ozlabs.org/project/linux-gpio/patch/20171113012523.2328-2-andre.przywara@arm.com/

And this is what finally landed:

https://lore.kernel.org/linux-gpio/20250306235827.4895-7-andre.przywara@arm.com/ (local)

Has it caused a bit of trouble? Perhaps. I was working on various peripherals
on a new board and put in the wrong mux value and didn't notice for a couple
days.
Then we must find a way to decouple hardware-specific information from the actual
header I think?
By "hardware-specific information" I assume you mean how the different I/O
blocks are arranged and ordered to form a contiguous pin range?

We could take a look at the Rockchip design: IIRC it has a number of GPIO
controllers which handle the pin specific configs, but the pinmuxing is
done from a separate GRF (general register field) region. The GPIO
controllers' registers have the same layout, and the binding and driver
assume the same number of pins per block. Actual missing pins are just
skipped over.

I don't remember off the top of my head how the MediaTek hardware cuts
its set of controls across the hardware, but it might be similar. But
as I mentioned earlier, MediaTek doesn't number pins, and it certainly
doesn't split them into banks.

Hope that gives you some ideas to work with.
Alternatively - that's what I have understood - and if I've understood that wrong,
this needs clarification from the bindings maintainers, and why they wanted the
MediaTek pinctrl bindings to get moved to arch/arm64/boot/dts/mediatek/ instead of
include/dt-bindings/pinctrl/

Bindings maintainers, any word on this?

Did I misunderstand anything in past reviews ... from krzk if I remember correctly?
I think the reason was that they looked like macros for every pin/function
combination, and nothing more.

For any other SoC that had a more *rigid* PIO block where the numbering
is predictable, then yes, it would seem like helpful macros to make the
raw numbers more human readable.


ChenYu
Cheers,
Angelo
quoted

ChenYu
quoted
                         bias-disable;
                 };
         };

.... or

         spi1_pins: spi1-pins {
                 pins-bus {
                         function = "spi_m1";
                         groups = "spi_m1_pins";
                         bias-disable;
                 };
         };

That's the entire situation.

Cheers,
Angelo
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help