Received-SPF: pass (google.com: domain of linux-kernel+bounces-165873-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Peng Fan <peng.fan@nxp.com>
To: Dan Carpenter <dan.carpenter@linaro.org>, "Peng Fan (OSS)"
	<peng.fan@oss.nxp.com>
CC: Linus Walleij <linus.walleij@linaro.org>, Thierry Reding
	<thierry.reding@gmail.com>, Jonathan Hunter <jonathanh@nvidia.com>, Dvorkin
 Dmitry <dvorkin@tibbo.com>, Wells Lu <wellslutw@gmail.com>, Maxime Coquelin
	<mcoquelin.stm32@gmail.com>, Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Emil Renner Berthing <kernel@esmil.dk>, Jianlong Huang
	<jianlong.huang@starfivetech.com>, Hal Feng <hal.feng@starfivetech.com>,
	Orson Zhai <orsonzhai@gmail.com>, Baolin Wang
	<baolin.wang@linux.alibaba.com>, Chunyan Zhang <zhang.lyra@gmail.com>, Viresh
 Kumar <vireshk@kernel.org>, Shiraz Hashim <shiraz.linux.kernel@gmail.com>,
	"soc@kernel.org" <soc@kernel.org>, Krzysztof Kozlowski <krzk@kernel.org>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>, Alim Akhtar
	<alim.akhtar@samsung.com>, Geert Uytterhoeven <geert+renesas@glider.be>,
	Patrice Chotard <patrice.chotard@foss.st.com>, Heiko Stuebner
	<heiko@sntech.de>, Damien Le Moal <dlemoal@kernel.org>, Ludovic Desroches
	<ludovic.desroches@microchip.com>, Nicolas Ferre
	<nicolas.ferre@microchip.com>, Alexandre Belloni
	<alexandre.belloni@bootlin.com>, Claudiu Beznea <claudiu.beznea@tuxon.dev>,
	Aisheng Dong <aisheng.dong@nxp.com>, Fabio Estevam <festevam@gmail.com>,
	Shawn Guo <shawnguo@kernel.org>, Jacky Bai <ping.bai@nxp.com>, Pengutronix
 Kernel Team <kernel@pengutronix.de>, Chester Lin <chester62515@gmail.com>,
	Matthias Brugger <mbrugger@suse.com>, "Ghennadi Procopciuc (OSS)"
	<ghennadi.procopciuc@oss.nxp.com>, Sean Wang <sean.wang@kernel.org>, Matthias
 Brugger <matthias.bgg@gmail.com>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>, Sascha Hauer
	<s.hauer@pengutronix.de>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel
 Stanley <joel@jms.id.au>, "linux-gpio@vger.kernel.org"
	<linux-gpio@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "linux-tegra@vger.kernel.org"
	<linux-tegra@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-stm32@st-md-mailman.stormreply.com"
	<linux-stm32@st-md-mailman.stormreply.com>,
	"linux-samsung-soc@vger.kernel.org" <linux-samsung-soc@vger.kernel.org>,
	"linux-renesas-soc@vger.kernel.org" <linux-renesas-soc@vger.kernel.org>,
	"linux-rockchip@lists.infradead.org" <linux-rockchip@lists.infradead.org>,
	"linux-riscv@lists.infradead.org" <linux-riscv@lists.infradead.org>,
	"linux-mediatek@lists.infradead.org" <linux-mediatek@lists.infradead.org>,
	"imx@lists.linux.dev" <imx@lists.linux.dev>, "linux-aspeed@lists.ozlabs.org"
	<linux-aspeed@lists.ozlabs.org>, "openbmc@lists.ozlabs.org"
	<openbmc@lists.ozlabs.org>
Subject: RE: [PATCH 01/21] pinctrl: ti: iodelay: Use scope based of_node_put()
 cleanups
Thread-Topic: [PATCH 01/21] pinctrl: ti: iodelay: Use scope based
 of_node_put() cleanups
Thread-Index: AQHam8Xj7phXRd8k+Eq6Rs1zefiOObGCYIcAgAC3AKA=
Date: Thu, 2 May 2024 00:28:42 +0000
Message-ID:
 <DU0PR04MB9417AD5892A1A45E6AE18D8688182@DU0PR04MB9417.eurprd04.prod.outlook.com>
References: <20240501-pinctrl-cleanup-v1-0-797ceca46e5c@nxp.com>
 <20240501-pinctrl-cleanup-v1-1-797ceca46e5c@nxp.com>
 <ee5c8637-b8b2-491b-b011-e399942691dc@moroto.mountain>
In-Reply-To: <ee5c8637-b8b2-491b-b011-e399942691dc@moroto.mountain>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?CpVog6jI5OKurZJAU0XhgI1TVxEpz+zzjIglGeKnV+kYFgVQpUJpj7oZv3l/?=
 =?us-ascii?Q?9ctRCgWOY6x1qJhee0xaWwqwctu7Wuvns8lNyx7oviqqyHUUvy6SPcTuJ6B+?=
 =?us-ascii?Q?6iyauZENPOgyHvMId0Cv49R6ET82zHf+4DE64UqiVvZmcnNPnu7rxPopKYSW?=
 =?us-ascii?Q?yU58e61oGCEMcZmfVX8yWz2kcPMoy3O8ZXFah3zc2w7hsJ78ESjRrwDaS7Vr?=
 =?us-ascii?Q?z1LTyZ9qzsUHjoLcXrcisqMH3ALAHEs9qCK0Ru2L92WLy7gLpFah7fUXORUf?=
 =?us-ascii?Q?c7M0HFoT6Imcjwqf9XmhkjV6i6gC/U99qWbo5hN7L8viTGos5vs+RxDN6pND?=
 =?us-ascii?Q?lTgF2lVblVe1+XEO1eyOzYwP7txpzHSbh7eetLm+BBvNEwO0WP0PgjmnTq3L?=
 =?us-ascii?Q?4yZNVNmMivRqrgTBU51WmTiAgdoPespEhCI7nee804oqvY7U9H40ATRtH5H2?=
 =?us-ascii?Q?djif00m+tGZc6hETjDC6W/M090h1cl3JSTgZS9NTrkbPRZpvFDbqYhDPyecg?=
 =?us-ascii?Q?JAm4GK+zZIJkM+F90n6OCf9tNTzUbsYaJ7Sl6+/5j/TQS2LmlTEhVnt+Iwfj?=
 =?us-ascii?Q?mdhe5BZ6GsUsFvZ6zpVBhkSpqoNyoHWn/asmsQ7onmc9YLQTiU0DAMvAds9G?=
 =?us-ascii?Q?lEgP3xit9oKfgf+vVENqo+vjSHsvlWfagbvqtMoFJqH7WvrFMIr5o7YNpRGL?=
 =?us-ascii?Q?DJlY/ctBGpD5E2qoBx7MQuXxkw1ynJ+HYUf0duzLqSoIA8FbvyTaIE8AQzdu?=
 =?us-ascii?Q?qEYwgem8x/BYwRA68JB/mq03MtqofZU3rVUNjp/lLQdm+e7vib88gY3P3Aoq?=
 =?us-ascii?Q?TAvPAa25ygj7ALXkOmtJqs6ODgTB05UTyi7vc09ZloVFM1oakNTjUTNmyIO3?=
 =?us-ascii?Q?poINkGPCg59gJRY7tNWEFWVADaWAdhM/4ZUejxDLWDfXD6JauuWwSjKtB4ZY?=
 =?us-ascii?Q?2OfmKInCQYJXC9SpROmHIGsxdKYWj6Ak+ReFhZcA1045HRoQLN4dMIolMP1J?=
 =?us-ascii?Q?2XZooo3r513l1OjIFtMxbHbGUTyDLTSL9CeEFtNuH9lYY1neQVDE5TLLAFHK?=
 =?us-ascii?Q?T6F6rPT3ym4RCTi1nMyEyINBn5stuTTBM1vNIGX1BkitfCLoeb7h0C/BXQx5?=
 =?us-ascii?Q?cWApztl5EWXgXUUepSshbgc0y432VcIqkx2Zi24uRRXMjJ2JFV7Ji9T+tlBn?=
 =?us-ascii?Q?7+R8rOng04OEm0IVdodMODxDorPGeDoieMZDAM4f+N6TEt6B5tPkFMn+8Asz?=
 =?us-ascii?Q?Y4YhwUbmB9/tiCYMpzRNlNjnQ+HvTshWEeza0kd1+uREqyzE659qp9lYq4D8?=
 =?us-ascii?Q?lcjn9+5rFgiyBbWMFzW+zVOidSuWBcRqmNavWcGyUMPDZkfC3m4fvsikRqq5?=
 =?us-ascii?Q?5INb2gRbOW0Ikx4UP01fOyzh9mJ5iYwNAtHentOnZEmV/p7U43dAnPGYaB6H?=
 =?us-ascii?Q?KOEdPq5sPTa6MCiycNaRAEsE1h0kbeqUsFAqxSl8hRqOoSQ2eS5nn9tnmeH2?=
 =?us-ascii?Q?F5+MhyIQUO1O+jfaeawPnqU8JLJH2aNxcG5M+rAvb8kpbRWTFO5KpYL1PScE?=
 =?us-ascii?Q?dZHQSv7K2xuIaqpD2Zw=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DU0PR04MB9417.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 65567ff6-a480-4765-c129-08dc6a3ed231
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 May 2024 00:28:42.6706
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: aI8YFo/Skwu6zxqn8HobTDaVWaQ3DE1QfrDFSC77Z1GbtbvHAb/vXlE5OMCOq7z2b7VvcAGEVu75OyLmNIggOw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR04MB8092

> Subject: Re: [PATCH 01/21] pinctrl: ti: iodelay: Use scope based of_node_=
put()
> cleanups
>=20
> On Wed, May 01, 2024 at 08:55:59PM +0800, Peng Fan (OSS) wrote:
> > @@ -879,16 +874,12 @@ static int ti_iodelay_probe(struct
> platform_device *pdev)
> >  	ret =3D pinctrl_register_and_init(&iod->desc, dev, iod, &iod->pctl);
> >  	if (ret) {
> >  		dev_err(dev, "Failed to register pinctrl\n");
> > -		goto exit_out;
> > +		return ret;
> >  	}
> >
> >  	platform_set_drvdata(pdev, iod);
> >
> >  	return pinctrl_enable(iod->pctl);
> > -
> > -exit_out:
> > -	of_node_put(np);
> > -	return ret;
> >  }
>=20
> This will call of_node_put() on the success path so it's a behavior chang=
e.  The
> original code is buggy, it's supposed to call of_node_put() on the succes=
s path
> here or in ti_iodelay_remove().
>=20
> If it's supposed to call of_node_put() here, then fine, this is bugfix bu=
t if it's
> supposed to call it in ti_iodelay_remove() then we need to save the point=
er
> somewhere using no_free_ptr().  Probably saving ->np is the safest choice=
?
>=20
> The original code is already a little bit buggy because it doesn't check =
for
> pinctrl_enable() errors and cleanup.

It was introduced by=20
commit 6118714275f0a313ecc296a87ed1af32d9691bed (tag: pinctrl-v4.11-4)
Author: Tony Lindgren <tony@atomide.com>
Date:   Thu Mar 30 09:16:39 2017 -0700

    pinctrl: core: Fix pinctrl_register_and_init() with pinctrl_enable()

of_node_put is expected in probe, not in remove.

Thanks,
Peng.

>=20
>=20
> diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti=
/pinctrl-ti-
> iodelay.c
> index 040f2c46a868..f40a1476e4ff 100644
> --- a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> +++ b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> @@ -156,6 +156,7 @@ struct ti_iodelay_device {
>=20
>  	const struct ti_iodelay_reg_data *reg_data;
>  	struct ti_iodelay_reg_values reg_init_conf_values;
> +	struct device_node *np;
>  };
>=20
>  /**
> @@ -884,7 +885,12 @@ static int ti_iodelay_probe(struct platform_device
> *pdev)
>=20
>  	platform_set_drvdata(pdev, iod);
>=20
> -	return pinctrl_enable(iod->pctl);
> +	ret =3D pinctrl_enable(iod->pctl);
> +	if (ret)
> +		goto exit_out;
> +
> +	iod->np =3D no_free_ptr(np);
> +	return 0;
>=20
>  exit_out:
>  	of_node_put(np);
> @@ -903,6 +909,7 @@ static void ti_iodelay_remove(struct platform_device
> *pdev)
>  		pinctrl_unregister(iod->pctl);
>=20
>  	ti_iodelay_pinconf_deinit_dev(iod);
> +	of_node_put(iod->np);
>=20
>  	/* Expect other allocations to be freed by devm */  }
>=20
>=20
>=20