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

[Frank Li <Frank.li@nxp.com>]

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