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.hdiff --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.hGeneral 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, Angeloquoted
ChenYuquoted
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