Hi Loic,
On 2/26/21 2:53 AM, Loic Poulain wrote:
mhi_queue returns an error when the doorbell is not accessible in
the current state. This can happen when the device is in non M0
state, like M3, and needs to be waken-up prior ringing the DB. This
case is managed earlier by triggering an asynchronous M3 exit via
controller resume/suspend callbacks, that in turn will cause M0
transition and DB update.
So, since it's not an error but just delaying of doorbell update, there
is no reason to return an error.
This also fixes a use after free error for skb case, indeed a caller
queuing skb will try to free the skb if the queueing fails, but in
that case queueing has been done.
Fixes: a8f75cb348fd ("mhi: core: Factorize mhi queuing")
Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx>
Reviewed-by: Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx>
Reviewed-by: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>
---
v2: - Fix/reword commit message
- Add Fixes tag
drivers/bus/mhi/core/main.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 7fc2482..c780234 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -1031,12 +1031,8 @@ static int mhi_queue(struct mhi_device *mhi_dev, struct mhi_buf_info *buf_info,
if (mhi_chan->dir == DMA_TO_DEVICE)
atomic_inc(&mhi_cntrl->pending_pkts);
- if (unlikely(!MHI_DB_ACCESS_VALID(mhi_cntrl))) {
- ret = -EIO;
- goto exit_unlock;
- }
-
- mhi_ring_chan_db(mhi_cntrl, mhi_chan);
+ if (likely(MHI_DB_ACCESS_VALID(mhi_cntrl)))
+ mhi_ring_chan_db(mhi_cntrl, mhi_chan);
Caller of all of the ring db APIs mhi_ring_chan/er/cmd_db are checking
this if condition "if (likely(MHI_DB_ACCESS_VALID(mhi_cntrl)))" every
where in the code. Does it make sense to move this check inside the APIs
itself, as a clean up.
exit_unlock:
read_unlock_irqrestore(&mhi_cntrl->pm_lock, flags);
Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
