Re: [PATCH] bus: mhi: Fix channel close issue on driver remove

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020-10-09 02:07, Loic Poulain wrote:
Some MHI device drivers need to stop the channels in their driver
remove callback (e.g. module unloading), but the unprepare function
is aborted because MHI core moved the channels to suspended state
prior calling driver remove callback. This prevents the driver to
send a proper MHI RESET CHAN command to the device. Device is then
unaware of the stopped state of these channels.

This causes issue when driver tries to start the channels again (e.g.
module is reloaded), since device considers channels as already
started (inconsistent state).

Fix this by allowing channel reset when channel is suspended.

Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx>
---
 drivers/bus/mhi/core/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index d20967a..a588eac 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -1232,7 +1232,8 @@ static void __mhi_unprepare_channel(struct
mhi_controller *mhi_cntrl,
 	/* no more processing events for this channel */
 	mutex_lock(&mhi_chan->mutex);
 	write_lock_irq(&mhi_chan->lock);
-	if (mhi_chan->ch_state != MHI_CH_STATE_ENABLED) {
+	if (mhi_chan->ch_state != MHI_CH_STATE_ENABLED &&
+	    mhi_chan->ch_state != MHI_CH_STATE_SUSPENDED) {
 		write_unlock_irq(&mhi_chan->lock);
 		mutex_unlock(&mhi_chan->mutex);
 		return;
Hi Loic,

There should not be any reason for drivers to do an "unprepare" and send a reset channel command during remove, as the channel context gets cleaned up after the remove callback
returns.

We do not want to allow moving from MHI_CH_STATE_SUSPENDED to MHI_CH_STATE_DISABLED state because if a remove is called, channel context being cleaned up implies a reset.

Also, I have a bunch of channel state machine related patches coming up soon which solve this issue and more. We are also introducing some missing features with that.

It would be nice if you can review/comment on those as it overhauls the state machine.

Let me know what you think.

Thanks,
Bhaumik
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux