Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <991d6680-480c-b9be-e09b-3b129dbb40a6@quicinc.com>
Date:   Fri, 13 Jan 2023 00:12:13 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH V8 2/2] firmware: qcom: scm: Add wait-queue handling logic
Content-Language: en-US
To:     Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
        <andersson@kernel.org>
CC:     <agross@kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <robh+dt@kernel.org>,
        <konrad.dybcio@somainline.org>, <robimarko@gmail.com>,
        <quic_gurus@quicinc.com>
References: <20230111101718.9225-1-quic_sibis@quicinc.com>
 <20230111101718.9225-3-quic_sibis@quicinc.com>
 <00e70137-3cc3-877e-5435-ac15f750f466@linaro.org>
From:   Sibi Sankar <quic_sibis@quicinc.com>
In-Reply-To: <00e70137-3cc3-877e-5435-ac15f750f466@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hey Srini,
Thanks for taking time to review the series.

On 1/11/23 21:31, Srinivas Kandagatla wrote:
> 
> 
> On 11/01/2023 10:17, Sibi Sankar wrote:
>> +static int qcom_scm_waitq_wakeup(struct qcom_scm *scm, unsigned int 
>> wq_ctx, bool wake_all)
>> +{
>> +    int ret;
>> +
>> +    ret = qcom_scm_assert_valid_wq_ctx(wq_ctx);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (wake_all)
>> +        complete_all(&__scm->waitq_comp);
> 
> As you explained in v7 that there will be only one caller at any point 
> in time and that will be synchronous, so complete_all here is a dead 
> code, isn't it?
> 
> Adding complete_all here is missleading and will require reinit 
> completion in case you want to reuse the same completion.
> 
> AFAIU, you should remove support to wake_all in this patchset and add it 
> when we really can do multiple scm calls simultaneously.
> 

ACK, IIRC v4 of the series had it dropped and was added
later on for feature parity but like you pointed out it's
dead code and should be added back later when there is support
for it. I'll fix this in the next re-spin.


> --srini
> 
> --srini
>> +    else
>> +        complete(&__scm->waitq_comp);
>> +