Received-SPF: pass (google.com: domain of linux-kernel+bounces-128106-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 2 Apr 2024 17:09:17 +0300
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Peng Fan <peng.fan@nxp.com>
Cc: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Cristian Marussi <cristian.marussi@arm.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Dan Carpenter <dan.carpenter@linaro.org>,
	"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>,
	Oleksii Moisieiev <oleksii_moisieiev@epam.com>
Subject: Re: [PATCH v7 4/4] pinctrl: Implementation of the generic
 scmi-pinctrl driver
Message-ID: <ZgwRjXbf_EV23S-P@smile.fi.intel.com>
References: <20240402-pinctrl-scmi-v7-0-3ea519d12cf7@nxp.com>
 <20240402-pinctrl-scmi-v7-4-3ea519d12cf7@nxp.com>
 <ZgwGpZ6S13vjk8jh@smile.fi.intel.com>
 <DU0PR04MB9417D0D33573E99D440D7BD7883E2@DU0PR04MB9417.eurprd04.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DU0PR04MB9417D0D33573E99D440D7BD7883E2@DU0PR04MB9417.eurprd04.prod.outlook.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

On Tue, Apr 02, 2024 at 01:59:19PM +0000, Peng Fan wrote:
> > On Tue, Apr 02, 2024 at 10:22:24AM +0800, Peng Fan (OSS) wrote:

..

> > > +#include <linux/device.h>
> > > +#include <linux/err.h>
> > > +#include <linux/module.h>
> > > +#include <linux/seq_file.h>
> > > +#include <linux/scmi_protocol.h>
> > > +#include <linux/slab.h>
> > 
> > Missing headers.
> 
> Not sure there is an easy way to filter out what is missed.

And?..

You are the author, not me. You know your code much better and
it will be quite easy to perform. I may miss things, but reading
briefly the 1000 lines and get what headers are required takes
no more than half an hour.

(Tools that help me, in case I don't remember by heart, are
 `cscope` and `git grep ...`.)

..

> > > +		ret = pinctrl_ops->name_get(pmx->ph, i, PIN_TYPE,
> > &pins[i].name);
> > > +		if (ret)
> > 
> > How does the cleanup work for the previously assigned pin names? Is it
> > needed?
> 
> No need. The "name" memory region is allocated in firmware pinctrl
> Protocol init phase.
> 
> > Maybe a comment?
> 
> ok.  As below.
> /*
>  * The region for name is handled by the scmi firmware driver, 
>  * no need free here
> */

LGTM.

> > > +			return dev_err_probe(pmx->dev, ret,
> > > +					     "Can't get name for pin %d", i);

-- 
With Best Regards,
Andy Shevchenko