Re: [PATCH 2/4] pinctrl: renesas: rzn1: free maps on DT map failure

[Thierry Reding <thierry.reding@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2158 bytes --]

On Tue, Sep 29, 2026 at 04:07:05PM +0800, jiale yao wrote:
> Hi Geert,
> 
> At 2026-09-28 21:19:48, "Geert Uytterhoeven" <geert@linux-m68k.org> wrote:
> >Hi Jiale,
> >
> >On Thu, 24 Sept 2026 at 13:19, Jiale Yao <yaojiale02@163.com> wrote:
> >> rzn1_dt_node_to_map_one() appends mappings for each DT node.  If a
> >> node fails after map storage has been reserved, the top-level callback
> >> returns without releasing mappings built for the current and previous
> >> nodes.
> >>
> >> Free the accumulated map and clear the output parameters on every error
> >> path.
> >>
> >> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> >
> >Thanks for your patch!
> >
> >> --- a/drivers/pinctrl/renesas/pinctrl-rzn1.c
> >> +++ b/drivers/pinctrl/renesas/pinctrl-rzn1.c
> >> @@ -411,15 +411,21 @@ static int rzn1_dt_node_to_map(struct pinctrl_dev *pctldev,
> >>
> >>         ret = rzn1_dt_node_to_map_one(pctldev, np, map, num_maps);
> >>         if (ret < 0)
> >> -               return ret;
> >> +               goto err_free_map;
> >>
> >>         for_each_child_of_node_scoped(np, child) {
> >>                 ret = rzn1_dt_node_to_map_one(pctldev, child, map, num_maps);
> >>                 if (ret < 0)
> >> -                       return ret;
> >> +                       goto err_free_map;
> >>         }
> >>
> >>         return 0;
> >> +
> >> +err_free_map:
> >> +       pinctrl_utils_free_map(pctldev, *map, *num_maps);
> >
> >Nice catch!
> >
> >> +       *map = NULL;
> >> +       *num_maps = 0;
> >
> >Is the resetting actually needed?
> >Only pinctrl-generic.c, pinctrl-sprd.c, and pinctrl-tegra-xusb.c seem
> >to do that
> 
> No, the resetting is not needed, as the caller does not inspect the
> output parameters when dt_node_to_map() returns an error.
> 
> I'll drop both assignments and update the commit message in v2.

It's not clear to me if you meant drop both assignments for all patches,
but that's what you should do. The reasoning about the caller not doing
anything with the output parameters on error holds for all
implementations, so resetting in any driver is useless.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]