[PATCH] pinctrl: nuvoton: Fix boot on ma35dx platforms

[PATCH] pinctrl: nuvoton: Fix boot on ma35dx platforms

[Miquel Raynal]

As part of a wider cleanup trying to get rid of OF specific APIs, an
incorrect (and partially unrelated) cleanup was introduced.

The goal was to replace a device_for_each_chil_node() loop including an
additional condition inside by a macro doing both the loop and the
check on a single line.

The snippet:

	device_for_each_child_node(dev, child)
		if (fwnode_property_present(child, "gpio-controller"))
			continue;

was replaced by:

	for_each_gpiochip_node(dev, child)

which expands into:

	device_for_each_child_node(dev, child)
		for_each_if(fwnode_property_present(child, "gpio-controller"))

This change is actually doing the opposite of what was initially
expected, breaking the probe of this driver, breaking at the same time
the whole boot of Nuvoton platforms (no more console, the kernel WARN()).

Revert these two changes to roll back to the correct behavior.

Fixes: 693c9ecd8326 ("pinctrl: nuvoton: Reduce use of OF-specific APIs")
Cc: stable@vger.kernel.org
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/pinctrl/nuvoton/pinctrl-ma35.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/nuvoton/pinctrl-ma35.c b/drivers/pinctrl/nuvoton/pinctrl-ma35.c
index 06ae1fe8b8c5..b51704bafd81 100644
--- a/drivers/pinctrl/nuvoton/pinctrl-ma35.c
+++ b/drivers/pinctrl/nuvoton/pinctrl-ma35.c
@@ -1074,7 +1074,10 @@ static int ma35_pinctrl_probe_dt(struct platform_device *pdev, struct ma35_pinct
 	u32 idx = 0;
 	int ret;
 
-	for_each_gpiochip_node(dev, child) {
+	device_for_each_child_node(dev, child) {
+		if (fwnode_property_present(child, "gpio-controller"))
+			continue;
+
 		npctl->nfunctions++;
 		npctl->ngroups += of_get_child_count(to_of_node(child));
 	}
@@ -1092,7 +1095,10 @@ static int ma35_pinctrl_probe_dt(struct platform_device *pdev, struct ma35_pinct
 	if (!npctl->groups)
 		return -ENOMEM;
 
-	for_each_gpiochip_node(dev, child) {
+	device_for_each_child_node(dev, child) {
+		if (fwnode_property_present(child, "gpio-controller"))
+			continue;
+
 		ret = ma35_pinctrl_parse_functions(child, npctl, idx++);
 		if (ret) {
 			fwnode_handle_put(child);
-- 
2.48.1

Re: [PATCH] pinctrl: nuvoton: Fix boot on ma35dx platforms

[Andy Shevchenko]

On Fri, Jun 13, 2025 at 9:13 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> As part of a wider cleanup trying to get rid of OF specific APIs, an
> incorrect (and partially unrelated) cleanup was introduced.
>
> The goal was to replace a device_for_each_chil_node() loop including an
> additional condition inside by a macro doing both the loop and the
> check on a single line.
>
> The snippet:
>
>         device_for_each_child_node(dev, child)
>                 if (fwnode_property_present(child, "gpio-controller"))
>                         continue;
>
> was replaced by:
>
>         for_each_gpiochip_node(dev, child)
>
> which expands into:
>
>         device_for_each_child_node(dev, child)
>                 for_each_if(fwnode_property_present(child, "gpio-controller"))
>
> This change is actually doing the opposite of what was initially
> expected, breaking the probe of this driver, breaking at the same time
> the whole boot of Nuvoton platforms (no more console, the kernel WARN()).
>
> Revert these two changes to roll back to the correct behavior.

Thank you for the report and the fix.
Can we also add a comment on top of each of these if:s to prevent from
blind change in the future?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] pinctrl: nuvoton: Fix boot on ma35dx platforms

[Miquel Raynal]

Hi Andy,

On 14/06/2025 at 00:07:51 +03, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Fri, Jun 13, 2025 at 9:13 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>>
>> As part of a wider cleanup trying to get rid of OF specific APIs, an
>> incorrect (and partially unrelated) cleanup was introduced.
>>
>> The goal was to replace a device_for_each_chil_node() loop including an
>> additional condition inside by a macro doing both the loop and the
>> check on a single line.
>>
>> The snippet:
>>
>>         device_for_each_child_node(dev, child)
>>                 if (fwnode_property_present(child, "gpio-controller"))
>>                         continue;
>>
>> was replaced by:
>>
>>         for_each_gpiochip_node(dev, child)
>>
>> which expands into:
>>
>>         device_for_each_child_node(dev, child)
>>                 for_each_if(fwnode_property_present(child, "gpio-controller"))
>>
>> This change is actually doing the opposite of what was initially
>> expected, breaking the probe of this driver, breaking at the same time
>> the whole boot of Nuvoton platforms (no more console, the kernel WARN()).
>>
>> Revert these two changes to roll back to the correct behavior.
>
> Thank you for the report and the fix.
> Can we also add a comment on top of each of these if:s to prevent from
> blind change in the future?

I believe it's fine like that, The difference is subtle, it is okay and
unavoidable to make this kind of small mistake sometimes.

Cheers,
Miquèl

Re: [PATCH] pinctrl: nuvoton: Fix boot on ma35dx platforms

[Linus Walleij]

On Fri, Jun 13, 2025 at 8:13 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> As part of a wider cleanup trying to get rid of OF specific APIs, an
> incorrect (and partially unrelated) cleanup was introduced.
>
> The goal was to replace a device_for_each_chil_node() loop including an
> additional condition inside by a macro doing both the loop and the
> check on a single line.
>
> The snippet:
>
>         device_for_each_child_node(dev, child)
>                 if (fwnode_property_present(child, "gpio-controller"))
>                         continue;
>
> was replaced by:
>
>         for_each_gpiochip_node(dev, child)
>
> which expands into:
>
>         device_for_each_child_node(dev, child)
>                 for_each_if(fwnode_property_present(child, "gpio-controller"))
>
> This change is actually doing the opposite of what was initially
> expected, breaking the probe of this driver, breaking at the same time
> the whole boot of Nuvoton platforms (no more console, the kernel WARN()).
>
> Revert these two changes to roll back to the correct behavior.
>
> Fixes: 693c9ecd8326 ("pinctrl: nuvoton: Reduce use of OF-specific APIs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Patch applied for fixes.

Yours,
Linus Walleij