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: <1b462f1a-327b-4a8f-8de7-909dbad0ddf8@quicinc.com>
Date:   Wed, 16 Nov 2022 12:11:22 +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 V4 2/2] firmware: qcom: scm: Add wait-queue handling logic
To:     Guru Das Srinagesh <quic_gurus@quicinc.com>
CC:     <andersson@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
        <robh+dt@kernel.org>, <agross@kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <konrad.dybcio@somainline.org>,
        <robimarko@gmail.com>, <quic_rjendra@quicinc.com>
References: <20221114082644.28739-1-quic_sibis@quicinc.com>
 <20221114082644.28739-3-quic_sibis@quicinc.com>
 <20221115053336.GA29781@quicinc.com>
Content-Language: en-US
From:   Sibi Sankar <quic_sibis@quicinc.com>
In-Reply-To: <20221115053336.GA29781@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hey Guru,

Thanks for taking time to review the series.

On 11/15/22 11:03, Guru Das Srinagesh wrote:
> On Nov 14 2022 13:56, Sibi Sankar wrote:
> 
> (snip)
> 
>> +static irqreturn_t qcom_scm_irq_handler(int irq, void *data)
>> +{
>> +	int ret;
>> +	struct qcom_scm *scm = data;
>> +	struct completion *wq_to_wake;
>> +	u32 wq_ctx, flags, more_pending = 0;
>> +
>> +	do {
>> +		ret = scm_get_wq_ctx(&wq_ctx, &flags, &more_pending);
>> +		if (ret) {
>> +			dev_err(scm->dev, "GET_WQ_CTX SMC call failed: %d\n", ret);
>> +			goto out;
>> +		}
>> +
>> +		wq_to_wake = qcom_scm_lookup_wq(scm, wq_ctx);
>> +		if (IS_ERR_OR_NULL(wq_to_wake)) {
>> +			dev_err(scm->dev, "No waitqueue found for wq_ctx %d: %ld\n",
>> +				wq_ctx, PTR_ERR(wq_to_wake));
>> +			goto out;
>> +		}
>> +
>> +		if (flags != QCOM_SMC_WAITQ_FLAG_WAKE_ONE &&
>> +		    flags != QCOM_SMC_WAITQ_FLAG_WAKE_ALL) {
>> +			dev_err(scm->dev, "Invalid Flags found for wq_ctx: %u\n", flags);
>> +			goto out;
>> +		}
>> +
>> +		complete(wq_to_wake);
> 
> Need to call complete() or complete_all() based on the flags.

with the current implementation we should get away with
just complete for now but I can add them back in the way
Bjorn wanted i.e. with the bool wake_all in the next
re-spin.

- Sibi

> 
>> +	} while (more_pending);
>> +
>> +out:
>> +	return IRQ_HANDLED;
>> +}