Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 12 May 2020 14:47:53 -0700
From:   Matthias Kaehlcke <mka@chromium.org>
To:     Georgi Djakov <georgi.djakov@linaro.org>
Cc:     vireshk@kernel.org, nm@ti.com, sboyd@kernel.org, rjw@rjwysocki.net,
        saravanak@google.com, sibis@codeaurora.org, robh+dt@kernel.org,
        rnayak@codeaurora.org, bjorn.andersson@linaro.org,
        vincent.guittot@linaro.org, jcrouse@codeaurora.org,
        evgreen@chromium.org, linux-pm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 08/10] cpufreq: dt: Validate all interconnect paths
Message-ID: <20200512214753.GJ4525@google.com>
References: <20200512125327.1868-1-georgi.djakov@linaro.org>
 <20200512125327.1868-9-georgi.djakov@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20200512125327.1868-9-georgi.djakov@linaro.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Georgi,

On Tue, May 12, 2020 at 03:53:25PM +0300, Georgi Djakov wrote:
> Currently when we check for the available resources, we assume that there
> is only one interconnect path, but in fact it could be more than one. Do
> some validation to determine the number of paths and verify if each one
> of them is available.
> 
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
> v8:
> * New patch.
> 
>  drivers/cpufreq/cpufreq-dt.c | 49 ++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> index 4ecef3257532..3dd28c2c1633 100644
> --- a/drivers/cpufreq/cpufreq-dt.c
> +++ b/drivers/cpufreq/cpufreq-dt.c
> @@ -91,12 +91,54 @@ static const char *find_supply_name(struct device *dev)
>  	return name;
>  }
>  
> +static int find_icc_paths(struct device *dev)
> +{
> +	struct device_node *np;
> +	struct icc_path **paths;
> +	int i, count, num_paths;
> +	int ret = 0;
> +
> +	np = of_node_get(dev->of_node);
> +	if (!np)
> +		return 0;
> +
> +	count = of_count_phandle_with_args(np, "interconnects",
> +					   "#interconnect-cells");
> +	of_node_put(np);
> +	if (count < 0)
> +		return 0;
> +
> +	/* two phandles when #interconnect-cells = <1> */
> +	if (count % 2) {
> +		dev_err(dev, "%s: Invalid interconnects values\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	num_paths = count / 2;
> +	paths = kcalloc(num_paths, sizeof(*paths), GFP_KERNEL);
> +	if (!paths)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < num_paths; i++) {
> +		paths[i] = of_icc_get_by_index(dev, i);
> +		ret = PTR_ERR_OR_ZERO(paths[i]);
> +		if (ret)
> +			break;
> +	}
> +
> +	while (i--)
> +		icc_put(paths[i]);

Since the function only does a validation and throws the paths away
afterwards you don't really need the dynamic allocation and 'icc_put'
loop. Just have a single 'struct icc_path' pointer and call icc_put()
inside the for loop.