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

From: Guru Das Srinagesh
Date: Wed Nov 16 2022 - 17:55:45 EST


On Nov 16 2022 12:11, Sibi Sankar wrote:
> 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.

Sounds good, let's consume the flags according to their intended purpose.

Thank you.

Guru Das.