Messages in this thread | | | Date | Tue, 19 Jul 2022 16:03:20 +0530 | Subject | Re: [PATCH 5/5] firmware: qcom: scm: Add wait-queue handling logic | From | Rajendra Nayak <> |
| |
On 7/14/2022 6:27 AM, Guru Das Srinagesh wrote: > On Jul 01 2022 16:51, Rajendra Nayak wrote: >> >> >> On 7/1/2022 4:32 PM, Rajendra Nayak wrote: >>> >>> >>> On 6/28/2022 1:14 AM, Guru Das Srinagesh wrote: >>>> Add logic to handle QCOM_SCM_WAITQ_SLEEP or QCOM_SCM_WAITQ_WAKE return >>>> codes. >>>> >>>> Scenario 1: Requests made by 2 different VMs: >>>> >>>> VM_1 VM_2 Firmware >>>> │ │ │ >>>> │ │ │ >>>> │ │ │ >>>> │ │ │ >>>> │ REQUEST_1 │ │ >>>> ├────────────────────────┼─────────────────────────────────┤ >>>> │ │ │ >>>> │ │ ┌──┼──┐ >>>> │ │ │ │ │ >>>> │ │ REQUEST_2 │ │ │ >>>> │ ├──────────────────────────────┼──┤ │ >>>> │ │ │ │ │Resource >>>> │ │ │ │ │is busy >>>> │ │ {WQ_SLEEP} │ │ │ >>>> │ │◄─────────────────────────────┼──┤ │ >>>> │ │ wq_ctx, smc_call_ctx │ │ │ >>>> │ │ └──┼──┘ >>>> │ REQUEST_1 COMPLETE │ │ >>>> │◄───────────────────────┼─────────────────────────────────┤ >>>> │ │ │ >>>> │ │ IRQ │ >>>> │ │◄─-------------------------------│ >>>> │ │ │ >>>> │ │ get_wq_ctx() │ >>>> │ ├────────────────────────────────►│ >>>> │ │ │ >>>> │ │ │ >>>> │ │◄────────────────────────────────┤ >>>> │ │ wq_ctx, flags, and │ >>>> │ │ more_pending │ >>>> │ │ │ >>>> │ │ │ >>>> │ │ wq_resume(smc_call_ctx) │ >>>> │ ├────────────────────────────────►│ >>>> │ │ │ >>>> │ │ │ >>>> │ │ REQUEST_2 COMPLETE │ >>>> │ │◄────────────────────────────────┤ >>>> │ │ │ >>>> │ │ │ >>>> >>>> Scenario 2: Two Requests coming in from same VM: >>>> >>>> VM_1 Firmware >>>> │ │ >>>> │ │ >>>> │ │ >>>> │ │ >>>> │ REQUEST_1 │ >>>> ├──────────────────────────────────────────────────────────┤ >>>> │ │ >>>> │ ┌────┼───┐ >>>> │ │ │ │ >>>> │ │ │ │ >>>> │ │ │ │ >>>> │ REQUEST_2 │ │ │ >>>> ├─────────────────────────────────────────────────────┼───►│ │ >>>> │ │ │ │Resource >>>> │ │ │ │is busy >>>> │ {WQ_SLEEP} │ │ │ >>>> │◄────────────────────────────────────────────────────┼────┤ │ >>>> │ wq_ctx, req2_smc_call_ctx │ │ │ >>>> │ │ │ │ >>>> │ └────┼───┘ >>>> │ │ >>>> │ {WQ_WAKE} │ >>>> │◄─────────────────────────────────────────────────────────┤ >>>> │ wq_ctx, req1_smc_call_ctx, flags │ >>> >>> >>> This is perhaps the same thing I asked on the previous patch, >>> I am guessing {WQ_WAKE} is returned in respone to REQUEST_1? >>> How do you know in this case if REQUEST_1 was a success or failure? >>> >> >> Ok looking at this some more, I think what we are saying is that the FW returns >> {WQ_WAKE} to REQUEST_1, we then call wq_wake_ack and the return of >> *that* will tell if REQUEST_1 was success or failure? >> Did I get it right? > > Yes, that is correct. I should have added an explanatory note in the commit > message to this effect: > > > │ {WQ_WAKE} <-- Return value │ > │◄─────────────────────────────────────────────────────────┤ > │ wq_ctx, req1_smc_call_ctx, flags <-- Its payload │ > > What this means is that the WQ_WAKE is sent by the FW to VM1 (direction of > arrow is from right to left) and that the additional data packed as payload > indicate that it is meant for REQUEST_1 (`req1_smc_call_ctx`). > > Hopefully this will help understand the diagram better.
Ok thanks for the explanation, I actually had a few more comments down in that patch which you did not answer, can you clarify them too?
>> + } else if ((long)res->a0 < 0) { >> + /* Error, simply return to caller */ >> + break;
if my understanding above is correct, shouldn't we do a >> + if (wq) >> + scm_waitq_flag_handler(wq, flags); in the error case also?
Also why no just scm_waitq_flag_handler(wq, flags); before fill_wq_wake_ack_args(smc, smc_call_ctx);?
> > Thank you. > > Guru Das.
| |