[PATCH RFC v3 0/2] Modern pinctrl for Exynos5250 devices
Quentin Schulz
quentin.schulz at cherry.de
Thu Jul 2 19:27:01 CEST 2026
Hi Lukas,
On 5/19/26 6:32 PM, Lukas Timmermann wrote:
> [You don't often get email from linux at timmermann.space. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Wed, May 06, 2026 at 03:14:11PM +0200, Quentin Schulz wrote:
>> Hi Lukas,
>>
>> On 5/6/26 1:13 AM, Lukas Timmermann wrote:
>>> This patch series adds the necessary files to use a modern pinctrl driver
>>> with an exynos5250 SoC. The changes are well tested and are working.
>>>
>>> In contrast to v1, this reenables GPIO functionality. But as I'm still
>>> lacking experience, I'm not quite sure on how to prevent s5p_gpio to
>>> load when using upstream DTS in a clean way. Because of that, this
>>> patch series is marked as an RFC.
>>> checkpatch complains about using #ifdef.
>>>
>>
>> We already have an issue with samsung,exynos78x0-gpio so we'd need a similar
>> work-around for that one.
>>
>> An easy way to sort this out would be to know if we know of devices which
>> are still using the old binding? Qualcomm is rather special, is Samsung
>> Exynos also special with regard to how it handles Device Trees? Do we get it
>> from an earlier boot stage or is everything coming from U-Boot itself? If
>> the latter, we can always migrate the existing devices to the new binding
>> and remove support for the old binding and with it the same compatible in
>> two drivers. Another question would be, does it make sense for both drivers
>> to be compiled? If not (e.g. it's only pinctrl IPs inside an SoC and there's
>> only one pinctrl device per SoC, so if your device supports the new binding,
>> you only need the new driver). Then we can add a
>> depends on !PINCTRL_EXYNOS5250
>> for CONFIG_S5P (we probably should have a different symbol for the GPIO
>> driver though, maybe CONFIG_GPIO_S5P?).
>>
>> Cheers,
>> Quentin
>
> I'm sorry. Some of these Infos should've been in my original message:
>
> This driver replaces the pinctrl functionality in the old s5p_gpio
I don't see how s5p_gpio is currently doing pinctrl. Do you rather mean
that the s5p_gpio driver is matching the pinctrl compatible and thus we
cannot have two drivers match the same compatible?
> driver. It doesn't replace GPIO functionality itself. So both drivers
> have to be compiled in order to work with modern upstream DTs. This
> driver binds the older s5p_gpio driver during its own binding.
>
> The pinctrl dt-bindings are incompatible to each other. This driver fixes
Can you highlight how they are incompatible? I think the pinctrl part is
identical?
arch/arm/dts/exynos5250.dtsi
/ {
[...]
pinctrl_0: pinctrl at 11400000 {
compatible = "samsung,exynos5250-pinctrl";
reg = <0x11400000 0x1000>;
interrupts = <0 46 0>;
wakup_eint: wakeup-interrupt-controller {
compatible = "samsung,exynos4210-wakeup-eint";
interrupt-parent = <&gic>;
interrupts = <0 32 0>;
};
};
pinctrl_1: pinctrl at 13400000 {
compatible = "samsung,exynos5250-pinctrl";
reg = <0x13400000 0x1000>;
interrupts = <0 45 0>;
};
pinctrl_2: pinctrl at 10d10000 {
compatible = "samsung,exynos5250-pinctrl";
reg = <0x10d10000 0x1000>;
interrupts = <0 50 0>;
};
pinctrl_3: pinctrl at 03860000 {
compatible = "samsung,exynos5250-pinctrl";
reg = <0x03860000 0x1000>;
interrupts = <0 47 0>;
};
[...]
};
+ arch/arm/dts/exynos5250-pinctrl.dtsi
/ {
pinctrl at 11400000 {
gpa0: gpa0 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpa1: gpa1 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpa2: gpa2 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpb0: gpb0 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpb1: gpb1 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpb2: gpb2 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpb3: gpb3 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpc0: gpc0 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpc1: gpc1 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpc2: gpc2 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpc3: gpc3 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpd0: gpd0 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpd1: gpd1 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpy0: gpy0 {
gpio-controller;
#gpio-cells = <2>;
};
gpy1: gpy1 {
gpio-controller;
#gpio-cells = <2>;
};
gpy2: gpy2 {
gpio-controller;
#gpio-cells = <2>;
};
gpy3: gpy3 {
gpio-controller;
#gpio-cells = <2>;
};
gpy4: gpy4 {
gpio-controller;
#gpio-cells = <2>;
};
gpy5: gpy5 {
gpio-controller;
#gpio-cells = <2>;
};
gpy6: gpy6 {
gpio-controller;
#gpio-cells = <2>;
};
gpc4: gpc4 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpx0: gpx0 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
interrupt-parent = <&combiner>;
#interrupt-cells = <2>;
interrupts = <23 0>, <24 0>, <25 0>, <25 1>,
<26 0>, <26 1>, <27 0>, <27 1>;
};
gpx1: gpx1 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
interrupt-parent = <&combiner>;
#interrupt-cells = <2>;
interrupts = <28 0>, <28 1>, <29 0>, <29 1>,
<30 0>, <30 1>, <31 0>, <31 1>;
};
gpx2: gpx2 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpx3: gpx3 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
};
pinctrl at 13400000 {
gpe0: gpe0 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpe1: gpe1 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpf0: gpf0 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpf1: gpf1 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpg0: gpg0 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpg1: gpg1 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpg2: gpg2 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gph0: gph0 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gph1: gph1 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
};
pinctrl at 10d10000 {
gpv0: gpv0 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpv1: gpv1 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpv2: gpv2 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpv3: gpv3 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpv4: gpv4 {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
};
pinctrl at 03860000 {
gpz: gpz {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
};
};
vs dts/upstream/src/arm/samsung/exynos5250.dtsi
/{
[...]
pinctrl_0: pinctrl at 11400000 {
compatible = "samsung,exynos5250-pinctrl";
reg = <0x11400000 0x1000>;
interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
wakup_eint: wakeup-interrupt-controller {
compatible =
"samsung,exynos4210-wakeup-eint";
interrupt-parent = <&gic>;
interrupts = <GIC_SPI 32
IRQ_TYPE_LEVEL_HIGH>;
};
};
pinctrl_1: pinctrl at 13400000 {
compatible = "samsung,exynos5250-pinctrl";
reg = <0x13400000 0x1000>;
interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
};
pinctrl_2: pinctrl at 10d10000 {
compatible = "samsung,exynos5250-pinctrl";
reg = <0x10d10000 0x1000>;
interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
};
pinctrl_3: pinctrl at 3860000 {
compatible = "samsung,exynos5250-pinctrl";
reg = <0x03860000 0x1000>;
interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
power-domains = <&pd_mau>;
};
+ dts/upstream/src/arm/samsung/exynos5250-pinctrl.dtsi
&pinctrl_0 {
gpa0: gpa0-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpa1: gpa1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpa2: gpa2-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpb0: gpb0-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpb1: gpb1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpb2: gpb2-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpb3: gpb3-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpc0: gpc0-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpc1: gpc1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpc2: gpc2-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpc3: gpc3-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpd0: gpd0-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpd1: gpd1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpy0: gpy0-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
};
gpy1: gpy1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
};
gpy2: gpy2-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
};
gpy3: gpy3-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
};
gpy4: gpy4-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
};
gpy5: gpy5-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
};
gpy6: gpy6-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
};
gpc4: gpc4-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpx0: gpx0-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
interrupt-parent = <&combiner>;
#interrupt-cells = <2>;
interrupts = <23 0>, <24 0>, <25 0>, <25 1>,
<26 0>, <26 1>, <27 0>, <27 1>;
};
gpx1: gpx1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
interrupt-parent = <&combiner>;
#interrupt-cells = <2>;
interrupts = <28 0>, <28 1>, <29 0>, <29 1>,
<30 0>, <30 1>, <31 0>, <31 1>;
};
gpx2: gpx2-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpx3: gpx3-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
[...]
};
&pinctrl_1 {
gpe0: gpe0-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpe1: gpe1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpf0: gpf0-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpf1: gpf1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpg0: gpg0-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpg1: gpg1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpg2: gpg2-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gph0: gph0-gpio-bank {
gpio-controller;
#gpio-cells = <2>
interrupt-controller;
#interrupt-cells = <2>;
};
gph1: gph1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
[...]
};
&pinctrl_2 {
gpv0: gpv0-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpv1: gpv1-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpv2: gpv2-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpv3: gpv3-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
gpv4: gpv4-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
[...]
};
&pinctrl_3 {
gpz: gpz-gpio-bank {
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
};
[...]
};
I can see the interrupts property in the pinctrl@ node is 0 vs 4
(IRQ_TYPE_LEVEL_HIGH) and the node name is slightly different (I don't
think it matters).
The pinctrl nodes are also in the root (/) node in U-Boot, while in
upstream DTS it's under /soc (don't think it matters provided something
takes care of probing /soc and its children?).
The gpio nodes have an additional -gpio-bank suffix in upstream DTS,
don't think it matters either.
There are additional pinconf/pinmux/pingroups (I believe?) nodes but
those don't have a gpio-controller property so they shouldn't matter?
I must be missing something, can you tell me what?
> pinctrl with upstream DTs and the older one must be used on it's own
> when relying on U-Boots deprecated DTs.
>
> DTs are provided by U-Boot itself, at least that's the case on my device
> (exynos5250-manta).
>
> If I understood you correctly, we could modify the deprecated exynos5250.dtsi
> to fix this situation. But I don't know about possible side effects
> and I'm not that experienced in kernel/bootloader development. Is this an option?
>
> I had split the gpio symbol into it's own thing before and could add that
> back into v4 but it's functionally the same. (Both drivers needed for
> upstream DT)
>
No, I think it's fine to have pinctrl and GPIO split like you suggested
here.
Can you explain what makes your pinctrl driver require the upstream
Device Tree (the one in dts/) and cannot work with the U-Boot device
tree (the one in arch/arm/dts)?
We have a problem if it's requiring an upstream Device Tree because your
new driver doesn't enforce it (via a depends on OF_UPSTREAM in Kconfig),
and we have one if it doesn't, because the ifdefery in s5p_gpio is then
incorrect as you can compile with CONFIG_S5P AND
CONFIG_PINCTRL_EXYNOS5250 with both trying to bind against the same
compatible.
I think what you want in the second patch (provided I'm right the
pinctrl driver can work with the U-Boot DTS) is instead:
#if CONFIG_IS_ENABLED(PINCTRL) && IS_ENABLED(CONFIG_PINCTRL_EXYNOS5250)
{ .compatible = "samsung,exynos5250-pinctrl" },
#endif
and add a nice comment around those lines to say that we do this because
drivers/pinctrl/exynos/pinctrl-exynos5250.c binds against the same
compatible and will also bind s5p_gpio against the same node.
Otherwise, my suggestion was to maybe remove the compatible entirely
from s5p_gpio. For that, you'd need to make sure the new driver is
compatible with the device tree they are using (I'm assuming U-Boot
device tree and not upstream) and you'd need to enable CONFIG_PINCTRL
(and possibly CONFIG_SPL/TPL_PINCTRL) + CONFIG_EXYNOS5250 for defconfigs
where CONFIG_S5P is enabled (and in all xPL/proper phases, depending on
which CONFIG_GPIO and CONFIG_SPL/TPL_GPIO is enabled).
Maybe there's also a path to migrate the four Exynos5250 defconfigs to
use upstream Device Tree but one would need to compare the current
U-Boot device tree we have and the upstream one (in dts/) and check that
all drivers are compatible with the upstream device tree before
switching them. Not a requirement for your series, but it'd be nice as a
follow if this is something I can convince you into having a look :)
Cheers,
Quentin
More information about the U-Boot
mailing list