Re: [PATCH RFC v3 0/2] Modern pinctrl for Exynos5250 devices

Hi Lukas,

On 8/6/26 9:26 PM, Lukas Timmermann wrote:
> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Thu, Jul 02, 2026 at 07:27:01PM +0200, Quentin Schulz wrote:
>> Hi Lukas,
>>
>> Can you highlight how they are incompatible? I think the pinctrl part is
>> identical?
>>
> After checking the DTs, I came to the same conclusion. I was wrong.
> Sorry.
>>
>> 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)?
>>
> As already stated, I saw a large difference a long time ago and assumed
> it has to be incompatible. I'm sorry for the confusion. This patch is

I'm glad we came to the same conclusion, no worries :)

> nearly a year old at this point.
> 
>> 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
> 
> Thanks for the suggestion. I didn't think of that.
>>
>> 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 :)
> 
> That is the main issue now. I don't have all of these devices. I just
> got the exynos5250-manta which is relatively new in the linux kernel.
> 

I was more thinking of running a diff between U-Boot DTS and Linux 
kernel DTS (the one we have in dts/upstream) and check the differences. 
Based on the differences, check the drivers in U-Boot can handle the 
Linux kernel DTS just fine. If some changes are required, a mix between 
adding Device Tree properties for U-Boot to a U-Boot-specific 
-u-boot.dtsi file in arch/arm/dts/ and/or adapting the drivers to 
support the Linux kernel DTS and its properties (and one can take 
inspiration from the Linux kernel driver for that), would be required.

If done carefully, this doesn't necessarily require testing on real 
devices as (hopefully!) the Linux kernel Device Tree has been tested on 
real devices and we have a reference implementation for drivers in the 
Linux kernel.

> I don't think I could test this patch with other devices than my own any
> time soon and as pinctrl is such a core functionality, this could go wrong.
> 
> Also I noticed, that the older s5p_gpio driver does everything my port
> needs right now. So this doesn't block my upcoming patch.
> 

OK. Only add stuff once you actually need it, we aren't really 
interested in merging unused (and possibly untested) things.

> What do you think? Should we proceed with this? It could break stuff.
> 

If you're not planning on using it right now ("the older s5p_gpio driver 
does everything my port needs right now"), then no.

If you implement it the way I suggested, one would need to enable the 
PINCTRL_EXYNOS5250 (which is disabled by default) for behavior to change 
so it shouldn't break stuff simply by merging the patch.

> Sorry for not responding in time. I'm quite busy at the moment.
> 

I have no expectation when it comes to time to answer, it's mail, it's 
asynchronous by design :) No time pressure on my side, especially when 
it's not about fixing a really bad issue we introduced since last release.

Cheers,
Quentin