Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v10 2/3] interconnect: qcom: Add MSM8916 interconnect
 provider driver
To:     Dmitry Osipenko <digetx@gmail.com>, robh+dt@kernel.org,
        linux-pm@vger.kernel.org
Cc:     agross@kernel.org, bjorn.andersson@linaro.org,
        evgreen@chromium.org, daidavid1@codeaurora.org,
        vincent.guittot@linaro.org, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20191030153904.8715-1-georgi.djakov@linaro.org>
 <20191030153904.8715-3-georgi.djakov@linaro.org>
 <88315b5a-1354-acf9-d57d-b301fb78cfa4@gmail.com>
From:   Georgi Djakov <georgi.djakov@linaro.org>
Openpgp: preference=signencrypt
Message-ID: <95eb0a55-d36b-b66e-ae64-18aa5baafff7@linaro.org>
Date:   Wed, 13 Nov 2019 15:24:19 +0200
MIME-Version: 1.0
In-Reply-To: <88315b5a-1354-acf9-d57d-b301fb78cfa4@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Dmitry,

On 13.11.19 г. 15:03 ч., Dmitry Osipenko wrote:
> 30.10.2019 18:39, Georgi Djakov пишет:
>> Add driver for the Qualcomm interconnect buses found in MSM8916 based
>> platforms. The topology consists of three NoCs that are controlled by
>> a remote processor that collects the aggregated bandwidth for each
>> master-slave pairs.
>>
[..]
>> +static int qnoc_remove(struct platform_device *pdev)
>> +{
>> +	struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
>> +	struct icc_provider *provider = &qp->provider;
>> +	struct icc_node *n;
>> +
>> +	list_for_each_entry(n, &provider->nodes, node_list) {
>> +		icc_node_del(n);
>> +		icc_node_destroy(n->id);
>> +	}
> 
> Hello Georgi,
> 
> While examining the interconnect API and current drivers' code, I
> noticed that everybody are copying this chunk of code which should crash
> kernel because removing node from a list during the traverse is allowed
> only when list_for_each_entry_safe() is used.

Nice catch. Thank you!

> Seems the IMX driver (which is under review now on the ML) is the only
> driver that does the removing procedure correctly.
> 
> Maybe it won't hurt to factor out the removal of provider's nodes into a
> common helper.
> 

Yes, this is a very good idea. Will do it.

Thanks,
Georgi