Received-SPF: pass (google.com: domain of linux-kernel+bounces-103940-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Peng Fan <peng.fan@nxp.com>
To: Dan Carpenter <dan.carpenter@linaro.org>, "Peng Fan (OSS)"
	<peng.fan@oss.nxp.com>
CC: Sudeep Holla <sudeep.holla@arm.com>, Cristian Marussi
	<cristian.marussi@arm.com>, Rob Herring <robh+dt@kernel.org>, Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley
	<conor+dt@kernel.org>, Oleksii Moisieiev <oleksii_moisieiev@epam.com>, Linus
 Walleij <linus.walleij@linaro.org>, Shawn Guo <shawnguo@kernel.org>, Sascha
 Hauer <s.hauer@pengutronix.de>, Pengutronix Kernel Team
	<kernel@pengutronix.de>, Fabio Estevam <festevam@gmail.com>, dl-linux-imx
	<linux-imx@nxp.com>, "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "devicetree@vger.kernel.org"
	<devicetree@vger.kernel.org>, "linux-gpio@vger.kernel.org"
	<linux-gpio@vger.kernel.org>
Subject: RE: [PATCH v5 4/4] pinctrl: Implementation of the generic
 scmi-pinctrl driver
Thread-Topic: [PATCH v5 4/4] pinctrl: Implementation of the generic
 scmi-pinctrl driver
Thread-Index: AQHadhNdlT3LNwW+z0GotvRK+LURY7E3XT6AgACaO0A=
Date: Fri, 15 Mar 2024 00:44:34 +0000
Message-ID:
 <DU0PR04MB9417F0C53A0B112E7A8A334288282@DU0PR04MB9417.eurprd04.prod.outlook.com>
References: <20240314-pinctrl-scmi-v5-0-b19576e557f2@nxp.com>
 <20240314-pinctrl-scmi-v5-4-b19576e557f2@nxp.com>
 <55ec2392-c196-4669-a339-12ef336707fa@moroto.mountain>
In-Reply-To: <55ec2392-c196-4669-a339-12ef336707fa@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?esvZqgotgQXHrYdT3GhKqFkxVpHOeYbEqAmJL+Ztuux/6TIGsYxlPNxG/6Gu?=
 =?us-ascii?Q?6sAGUnrMM5zQd3wVWr3Nhld87ycNR2Ch2A1MqPlMKe8p4MQWcFXANPpaDLZI?=
 =?us-ascii?Q?+Ab78hOd3yM2nl6Lnh+VumMjkV00FGvUslHQuoWKhphm9PBqucutaM939e4w?=
 =?us-ascii?Q?kUdlwUmBUO6OyBD3XoWNZd14tSej7h0ArBKg1StrTpMPwItmZ2yTiOlX28zc?=
 =?us-ascii?Q?T6yxtiKvc+OnmQuWqKRPZoADFNnn+Hb9+I7T0j5bG4kqTLKghIAZbJ3JaZ5b?=
 =?us-ascii?Q?bDsxAa7oK17AgAgCW5LQHyN//YE7X6Gnhb0rYK+MVElNTe5CIwyIoSvkDY4X?=
 =?us-ascii?Q?T2Lr+rn6kXQAd6yaS3qGBG2P+oeXXgY039rWkdmMWCZ0G7fjefOEZ04BbojA?=
 =?us-ascii?Q?p6WogpblYR+gYV6+e5GNSG1BY/UDVHmDoFE2YynwLNbnmIpLQ/moWKD8Y/eN?=
 =?us-ascii?Q?fvY+te0Dh2SBsHbWqu1XLFa2d4QwL0AauGlDtmdrg9YeW7OjGpoHcPj/Cw96?=
 =?us-ascii?Q?lGFW3QYqB0IJtiouy/rkIuKbZAKmWSBMThtCzfMa2dkpCuHzqvl5h6ZW0jzL?=
 =?us-ascii?Q?sbWCp26pEWRat+p9tKzpTSpZsb10rVMsaccXW3jZ56nc5/LeJL3ArbWASRSE?=
 =?us-ascii?Q?FzLHPNyFqqrATomur6P9uhMQjYKnAcfMqrVB5Odxz7LJV/sgKFoZ8pwIKRf2?=
 =?us-ascii?Q?Y5JSUUzwlon9B1oGe9mR2quOZcRNTsqSmCmws+DHiYefdoIvE4TYbxqj7qzp?=
 =?us-ascii?Q?sezPLFXpT44mkDydChvKx02EgHJrZYUlqNe1ZQP/7p2yf/Gykq+aF1YVqHIT?=
 =?us-ascii?Q?DFJHdcpHcXh1/WcQ3kdKzzR5G5Y+r9QxBTEymjhxC6tRJm/BXwBreAq31EqY?=
 =?us-ascii?Q?n3gmFWkT4XSnaCaqpB7LBFPDBRCqBLPPgyY2sGrQKmF396VzQTBIy7gQ25sS?=
 =?us-ascii?Q?TKSca67Nw8m8Al5W3o/lwVdnGTkhuEZK4T3EXPzv93ByuBkbnWc6nQu58RHf?=
 =?us-ascii?Q?DVNEkAtUbd0a0DVx7BJboIFuTSaU9ATdT5YGFBBCb84YAFhALGX+g99j9OA3?=
 =?us-ascii?Q?/u8u84Y1O2b1hQhFp8cDZFtmxvCxJmE2MddBwdkMsV+L6lWNqafd++ofNVsQ?=
 =?us-ascii?Q?nDpJ+O35CdWmbf+roFu4wn8YMon5hFYPNVYFbx/B/0O3gL34LJosiZUB17j8?=
 =?us-ascii?Q?ceSkfZfQr0eadF1j+T+TdFteMh/YVQy2d3xzRFTQzYJqk2MzyAAIWyJH9QkW?=
 =?us-ascii?Q?X8zLo51sTTjj/M7NjxfixAQb6ECnMogUDlLCkiIAbjo/q1nDQUHWAJjU3pgw?=
 =?us-ascii?Q?W4hBr2TQrYB20APgpoA+U/Xo/v9T829mdlLd4aFoIe4M191LnUxQMuvnKh7b?=
 =?us-ascii?Q?WTm0CKTY26zh4487eARew6cd6UFIuhloxPDjhWNc8P9jinOv+/K6YSX9ZOJx?=
 =?us-ascii?Q?gm5tR8p3ez8nWINYGC24JIqgitppcYdDwF/Eedv4L95gI12ra/DhT7Zz5XUD?=
 =?us-ascii?Q?xhM/1PsSXrGhWw42EZJU6iJWmtAlfaWG7Ge2P/kqzl42rgoN9BTcv+vh1tA8?=
 =?us-ascii?Q?Y/DFwczJo9AFfc8zBRM=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: 2e7b1b11-fdbd-4ca4-8b0b-08dc44891585
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Mar 2024 00:44:34.2035
 (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: nGpiqq2VK3AGXLjmcY/p6c6PCL0tM46t+U046vmPIbYuyHjuKhY+US5xNZdnwgvVHgEYlr1GthYzOy/PKx0tFg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR04MB10065

> Subject: Re: [PATCH v5 4/4] pinctrl: Implementation of the generic scmi-
> pinctrl driver
>=20
> On Thu, Mar 14, 2024 at 09:35:21PM +0800, Peng Fan (OSS) wrote:
> > +static int pinctrl_scmi_get_function_groups(struct pinctrl_dev *pctlde=
v,
> > +					    unsigned int selector,
> > +					    const char * const **groups,
> > +					    unsigned int * const num_groups)
> {
> > +	const unsigned int *group_ids;
> > +	int ret, i;
> > +	struct scmi_pinctrl *pmx =3D pinctrl_dev_get_drvdata(pctldev);
> > +
> > +	if (!groups || !num_groups)
> > +		return -EINVAL;
> > +
> > +	if (selector < pmx->nr_functions &&
> > +	    pmx->functions[selector].num_groups) {
>=20
> If pmx->functions[selector].num_groups is set then we assume that
> functions[selector].groups has been allocated.
>=20
> > +		*groups =3D (const char * const *)pmx-
> >functions[selector].groups;
> > +		*num_groups =3D pmx->functions[selector].num_groups;
> > +		return 0;
> > +	}
> > +
> > +	ret =3D pinctrl_ops->function_groups_get(pmx->ph, selector,
> > +					       &pmx-
> >functions[selector].num_groups,
> > +					       &group_ids);
>=20
> However, pmx->functions[selector].num_groups is set here and not cleared
> on the error paths.  Or instead of clearing the .num_groups it would be n=
ice
> to pass a local variable and only do the
> pmx->functions[selector].num_groups =3D local assignment right before the
> success return.

So you concern is I should clear the pmx->functions[selector].num_groups in
err path, right?

Thanks,
Peng.

>=20
> regards,
> dan carpenter
>=20
> > +	if (ret) {
> > +		dev_err(pmx->dev, "Unable to get function groups, err %d",
> ret);
> > +		return ret;
> > +	}
> > +
> > +	*num_groups =3D pmx->functions[selector].num_groups;
> > +	if (!*num_groups)
> > +		return -EINVAL;
> > +
> > +	pmx->functions[selector].groups =3D
> > +		devm_kcalloc(pmx->dev, *num_groups,
> > +			     sizeof(*pmx->functions[selector].groups),
> > +			     GFP_KERNEL);
> > +	if (!pmx->functions[selector].groups)
> > +		return -ENOMEM;
> > +
> > +	for (i =3D 0; i < *num_groups; i++) {
> > +		pmx->functions[selector].groups[i] =3D
> > +			pinctrl_scmi_get_group_name(pmx->pctldev,
> > +						    group_ids[i]);
> > +		if (!pmx->functions[selector].groups[i]) {
> > +			ret =3D -ENOMEM;
> > +			goto err_free;
> > +		}
> > +	}
> > +
> > +	*groups =3D (const char * const *)pmx->functions[selector].groups;
> > +
> > +	return 0;
> > +
> > +err_free:
> > +	devm_kfree(pmx->dev, pmx->functions[selector].groups);
> > +
> > +	return ret;
> > +}