[PATCH 1/1] pinctrl: imx1: Allow parsing DT without function nodes

[PATCH 1/1] pinctrl: imx1: Allow parsing DT without function nodes

[Frank Li]

The old format to define pinctrl settings for imx in DT has two hierarchy
levels. The first level are function device nodes. The second level are
pingroups which contain a property fsl,pins. The original ntention was to
define all pin functions in a single dtsi file and just reference the
correct ones in the board files.

The commit ("5fcdf6a7ed95e pinctrl: imx: Allow parsing DT without function
nodes") already make moden i.MX chip support flatten layout.

Make legacy chipes (more than 15 years) support this flatten layout also.

Fixes: e948cbdc41d6f ("ARM: dts: imx: remove redundant intermediate node in pinmux hierarchy")
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
build test only

Sébastien Szymanski:
	Can you help test it? I am happy i.MX27 still alive.

Frank
---
 drivers/pinctrl/freescale/pinctrl-imx1-core.c | 48 ++++++++++++++++---
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx1-core.c b/drivers/pinctrl/freescale/pinctrl-imx1-core.c
index b36c8a1461b7c..bf07e0c64a098 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx1-core.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx1-core.c
@@ -540,10 +540,34 @@ static int imx1_pinctrl_parse_functions(struct device_node *np,
 	return 0;
 }
 
+/*
+ * Check if the DT contains pins in the direct child nodes. This indicates the
+ * newer DT format to store pins. This function returns true if the first found
+ * fsl,pins property is in a child of np. Otherwise false is returned.
+ */
+static bool imx1_pinctrl_dt_is_flat_functions(struct device_node *np)
+{
+	struct device_node *function_np;
+	struct device_node *pinctrl_np;
+
+	for_each_child_of_node(np, function_np) {
+		if (of_property_read_bool(function_np, "fsl,pins"))
+			return true;
+
+		for_each_child_of_node(function_np, pinctrl_np) {
+			if (of_property_read_bool(pinctrl_np, "fsl,pins"))
+				return false;
+		}
+	}
+
+	return true;
+}
+
 static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
 		struct imx1_pinctrl *pctl, struct imx1_pinctrl_soc_info *info)
 {
 	struct device_node *np = pdev->dev.of_node;
+	bool flat_funcs;
 	int ret;
 	u32 nfuncs = 0;
 	u32 ngroups = 0;
@@ -552,9 +576,15 @@ static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
 	if (!np)
 		return -ENODEV;
 
-	for_each_child_of_node_scoped(np, child) {
-		++nfuncs;
-		ngroups += of_get_child_count(child);
+	flat_funcs = imx1_pinctrl_dt_is_flat_functions(np);
+	if (flat_funcs) {
+		nfuncs = 1;
+		ngroups = of_get_child_count(np);
+	} else {
+		for_each_child_of_node_scoped(np, child) {
+			++nfuncs;
+			ngroups += of_get_child_count(child);
+		}
 	}
 
 	if (!nfuncs) {
@@ -574,10 +604,14 @@ static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
 	if (!info->functions || !info->groups)
 		return -ENOMEM;
 
-	for_each_child_of_node_scoped(np, child) {
-		ret = imx1_pinctrl_parse_functions(child, info, ifunc++);
-		if (ret == -ENOMEM)
-			return -ENOMEM;
+	if (flat_funcs) {
+		imx1_pinctrl_parse_functions(np, info, 0);
+	} else {
+		for_each_child_of_node_scoped(np, child) {
+			ret = imx1_pinctrl_parse_functions(child, info, ifunc++);
+			if (ret == -ENOMEM)
+				return -ENOMEM;
+		}
 	}
 
 	return 0;
-- 
2.43.0

Re: [PATCH 1/1] pinctrl: imx1: Allow parsing DT without function nodes

[Sébastien Szymanski]

Hello,

On 5/4/26 7:08 PM, Frank Li wrote:
> The old format to define pinctrl settings for imx in DT has two hierarchy
> levels. The first level are function device nodes. The second level are
> pingroups which contain a property fsl,pins. The original ntention was to
> define all pin functions in a single dtsi file and just reference the
> correct ones in the board files.
> 
> The commit ("5fcdf6a7ed95e pinctrl: imx: Allow parsing DT without function
> nodes") already make moden i.MX chip support flatten layout.
> 
> Make legacy chipes (more than 15 years) support this flatten layout also.
> 
> Fixes: e948cbdc41d6f ("ARM: dts: imx: remove redundant intermediate node in pinmux hierarchy")
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> build test only
> 
> Sébastien Szymanski:
> 	Can you help test it? I am happy i.MX27 still alive.

Sure! Thanks for the patch! With this patch applied on Linux 7.1-rc2 the 
board boots again. I see the following messages, though:

[    0.085139] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins 
property (gpio)
[    0.085226] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins 
property (gpio)
[    0.085281] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins 
property (gpio)
[    0.085327] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins 
property (gpio)
[    0.085372] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins 
property (gpio)
[    0.085416] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins 
property (gpio)
[    0.092693] imx27-pinctrl 10015000.pinmux: initialized IMX pinctrl driver

That's because there is no fsl,pins property in the 6 gpio subnodes.

Tested-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>

I one have comment bellow.

> 
> Frank
> ---
>   drivers/pinctrl/freescale/pinctrl-imx1-core.c | 48 ++++++++++++++++---
>   1 file changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx1-core.c b/drivers/pinctrl/freescale/pinctrl-imx1-core.c
> index b36c8a1461b7c..bf07e0c64a098 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx1-core.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx1-core.c
> @@ -540,10 +540,34 @@ static int imx1_pinctrl_parse_functions(struct device_node *np,
>   	return 0;
>   }
>   
> +/*
> + * Check if the DT contains pins in the direct child nodes. This indicates the
> + * newer DT format to store pins. This function returns true if the first found
> + * fsl,pins property is in a child of np. Otherwise false is returned.
> + */
> +static bool imx1_pinctrl_dt_is_flat_functions(struct device_node *np)
> +{
> +	struct device_node *function_np;
> +	struct device_node *pinctrl_np;
> +
> +	for_each_child_of_node(np, function_np) {
> +		if (of_property_read_bool(function_np, "fsl,pins"))

Isn't of_property_present better here...

> +			return true;
> +
> +		for_each_child_of_node(function_np, pinctrl_np) {
> +			if (of_property_read_bool(pinctrl_np, "fsl,pins"))

...and here ?

Regards,

> +				return false;
> +		}
> +	}
> +
> +	return true;
> +}
> +
>   static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
>   		struct imx1_pinctrl *pctl, struct imx1_pinctrl_soc_info *info)
>   {
>   	struct device_node *np = pdev->dev.of_node;
> +	bool flat_funcs;
>   	int ret;
>   	u32 nfuncs = 0;
>   	u32 ngroups = 0;
> @@ -552,9 +576,15 @@ static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
>   	if (!np)
>   		return -ENODEV;
>   
> -	for_each_child_of_node_scoped(np, child) {
> -		++nfuncs;
> -		ngroups += of_get_child_count(child);
> +	flat_funcs = imx1_pinctrl_dt_is_flat_functions(np);
> +	if (flat_funcs) {
> +		nfuncs = 1;
> +		ngroups = of_get_child_count(np);
> +	} else {
> +		for_each_child_of_node_scoped(np, child) {
> +			++nfuncs;
> +			ngroups += of_get_child_count(child);
> +		}
>   	}
>   
>   	if (!nfuncs) {
> @@ -574,10 +604,14 @@ static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
>   	if (!info->functions || !info->groups)
>   		return -ENOMEM;
>   
> -	for_each_child_of_node_scoped(np, child) {
> -		ret = imx1_pinctrl_parse_functions(child, info, ifunc++);
> -		if (ret == -ENOMEM)
> -			return -ENOMEM;
> +	if (flat_funcs) {
> +		imx1_pinctrl_parse_functions(np, info, 0);
> +	} else {
> +		for_each_child_of_node_scoped(np, child) {
> +			ret = imx1_pinctrl_parse_functions(child, info, ifunc++);
> +			if (ret == -ENOMEM)
> +				return -ENOMEM;
> +		}
>   	}
>   
>   	return 0;


-- 
Sébastien Szymanski, Armadeus Systems
Software engineer

Re: [PATCH 1/1] pinctrl: imx1: Allow parsing DT without function nodes

[Frank Li]

On Tue, May 05, 2026 at 10:32:48AM +0200, Sébastien Szymanski wrote:
> Hello,
>
> On 5/4/26 7:08 PM, Frank Li wrote:
> > The old format to define pinctrl settings for imx in DT has two hierarchy
> > levels. The first level are function device nodes. The second level are
> > pingroups which contain a property fsl,pins. The original ntention was to
> > define all pin functions in a single dtsi file and just reference the
> > correct ones in the board files.
> >
> > The commit ("5fcdf6a7ed95e pinctrl: imx: Allow parsing DT without function
> > nodes") already make moden i.MX chip support flatten layout.
> >
> > Make legacy chipes (more than 15 years) support this flatten layout also.
> >
> > Fixes: e948cbdc41d6f ("ARM: dts: imx: remove redundant intermediate node in pinmux hierarchy")
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > build test only
> >
> > Sébastien Szymanski:
> > 	Can you help test it? I am happy i.MX27 still alive.
>
> Sure! Thanks for the patch! With this patch applied on Linux 7.1-rc2 the
> board boots again. I see the following messages, though:
>
> [    0.085139] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins property
> (gpio)
> [    0.085226] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins property
> (gpio)
> [    0.085281] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins property
> (gpio)
> [    0.085327] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins property
> (gpio)
> [    0.085372] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins property
> (gpio)
> [    0.085416] imx27-pinctrl 10015000.pinmux: Not a valid fsl,pins property
> (gpio)
> [    0.092693] imx27-pinctrl 10015000.pinmux: initialized IMX pinctrl driver
>
> That's because there is no fsl,pins property in the 6 gpio subnodes.
>
> Tested-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
>
> I one have comment bellow.
>
> >
> > Frank
> > ---
> >   drivers/pinctrl/freescale/pinctrl-imx1-core.c | 48 ++++++++++++++++---
> >   1 file changed, 41 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/pinctrl/freescale/pinctrl-imx1-core.c b/drivers/pinctrl/freescale/pinctrl-imx1-core.c
> > index b36c8a1461b7c..bf07e0c64a098 100644
> > --- a/drivers/pinctrl/freescale/pinctrl-imx1-core.c
> > +++ b/drivers/pinctrl/freescale/pinctrl-imx1-core.c
> > @@ -540,10 +540,34 @@ static int imx1_pinctrl_parse_functions(struct device_node *np,
> >   	return 0;
> >   }
> > +/*
> > + * Check if the DT contains pins in the direct child nodes. This indicates the
> > + * newer DT format to store pins. This function returns true if the first found
> > + * fsl,pins property is in a child of np. Otherwise false is returned.
> > + */
> > +static bool imx1_pinctrl_dt_is_flat_functions(struct device_node *np)
> > +{
> > +	struct device_node *function_np;
> > +	struct device_node *pinctrl_np;
> > +
> > +	for_each_child_of_node(np, function_np) {
> > +		if (of_property_read_bool(function_np, "fsl,pins"))
>
> Isn't of_property_present better here...
>
> > +			return true;
> > +
> > +		for_each_child_of_node(function_np, pinctrl_np) {
> > +			if (of_property_read_bool(pinctrl_np, "fsl,pins"))
>
> ...and here ?

Yes, I will update it and send v2.

Frank
>
> Regards,
>
> > +				return false;
> > +		}
> > +	}
> > +
> > +	return true;
> > +}
> > +
> >   static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
> >   		struct imx1_pinctrl *pctl, struct imx1_pinctrl_soc_info *info)
> >   {
> >   	struct device_node *np = pdev->dev.of_node;
> > +	bool flat_funcs;
> >   	int ret;
> >   	u32 nfuncs = 0;
> >   	u32 ngroups = 0;
> > @@ -552,9 +576,15 @@ static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
> >   	if (!np)
> >   		return -ENODEV;
> > -	for_each_child_of_node_scoped(np, child) {
> > -		++nfuncs;
> > -		ngroups += of_get_child_count(child);
> > +	flat_funcs = imx1_pinctrl_dt_is_flat_functions(np);
> > +	if (flat_funcs) {
> > +		nfuncs = 1;
> > +		ngroups = of_get_child_count(np);
> > +	} else {
> > +		for_each_child_of_node_scoped(np, child) {
> > +			++nfuncs;
> > +			ngroups += of_get_child_count(child);
> > +		}
> >   	}
> >   	if (!nfuncs) {
> > @@ -574,10 +604,14 @@ static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
> >   	if (!info->functions || !info->groups)
> >   		return -ENOMEM;
> > -	for_each_child_of_node_scoped(np, child) {
> > -		ret = imx1_pinctrl_parse_functions(child, info, ifunc++);
> > -		if (ret == -ENOMEM)
> > -			return -ENOMEM;
> > +	if (flat_funcs) {
> > +		imx1_pinctrl_parse_functions(np, info, 0);
> > +	} else {
> > +		for_each_child_of_node_scoped(np, child) {
> > +			ret = imx1_pinctrl_parse_functions(child, info, ifunc++);
> > +			if (ret == -ENOMEM)
> > +				return -ENOMEM;
> > +		}
> >   	}
> >   	return 0;
>
>
> --
> Sébastien Szymanski, Armadeus Systems
> Software engineer