Re: [PATCH V8 2/2] firmware: qcom: scm: Add wait-queue handling logic

[Sibi Sankar]

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);
> > +