diff mbox series

[v1,3/4] bus: mhi: core: Add support to pause or resume channel data transfers

Message ID 1604961850-27671-4-git-send-email-bbhatt@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series Updates to MHI channel handling | expand

Commit Message

Bhaumik Bhatt Nov. 9, 2020, 10:44 p.m. UTC
Some MHI clients may want to request for pausing or resuming of the
data transfers for their channels. Enable them to do so using the new
APIs provided for the same.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
 drivers/bus/mhi/core/main.c | 41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mhi.h         | 16 ++++++++++++++++
 2 files changed, 57 insertions(+)

Comments

Loic Poulain Nov. 10, 2020, 11:14 a.m. UTC | #1
Hi Bhaumik,

On Mon, 9 Nov 2020 at 23:44, Bhaumik Bhatt <bbhatt@codeaurora.org> wrote:
>
> Some MHI clients may want to request for pausing or resuming of the
> data transfers for their channels. Enable them to do so using the new
> APIs provided for the same.
>
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> ---
>  drivers/bus/mhi/core/main.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/mhi.h         | 16 ++++++++++++++++
>  2 files changed, 57 insertions(+)
>
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 1226933..01845c6 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -1560,6 +1560,47 @@ void mhi_unprepare_from_transfer(struct mhi_device *mhi_dev)
>  }
>  EXPORT_SYMBOL_GPL(mhi_unprepare_from_transfer);
>
> +static int mhi_update_transfer_state(struct mhi_device *mhi_dev,
> +                                    enum mhi_ch_state_type to_state)
> +{
> +       struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> +       struct mhi_chan *mhi_chan;
> +       int dir, ret;
> +
> +       for (dir = 0; dir < 2; dir++) {
> +               mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
> +
> +               if (!mhi_chan)
> +                       continue;
> +
> +               /*
> +                * Bail out if one of the channels fail as client will reset
> +                * both upon failure
> +                */
> +               mutex_lock(&mhi_chan->mutex);
> +               ret = mhi_update_channel_state(mhi_cntrl, mhi_chan, to_state);
> +               if (ret) {
> +                       mutex_unlock(&mhi_chan->mutex);
> +                       return ret;
> +               }
> +               mutex_unlock(&mhi_chan->mutex);
> +       }
> +
> +       return 0;
> +}
> +
> +int mhi_pause_transfer(struct mhi_device *mhi_dev)
> +{
> +       return mhi_update_transfer_state(mhi_dev, MHI_CH_STATE_TYPE_STOP);
> +}
> +EXPORT_SYMBOL_GPL(mhi_pause_transfer);
> +
> +int mhi_resume_transfer(struct mhi_device *mhi_dev)
> +{
> +       return mhi_update_transfer_state(mhi_dev, MHI_CH_STATE_TYPE_START);
> +}
> +EXPORT_SYMBOL_GPL(mhi_resume_transfer);

Look like it is stop and start, not pause and resume?

TBH maybe we should rework/clarify MHI core and having well-defined
states, maybe something like that:

1. When MHI core detects device for a driver, MHI core resets and
initializes the channel(s), then call client driver probe function
    => channel UNKNOWN->DISABLED state
    => channel DISABLED->ENABLED state
2. When driver is ready for sending data, drivers calls mhi_start_transfer
    => Channel is ENABLED->RUNNING state
3. Driver performs normal data transfers
4. The driver can suspend/resume transfer, it stops (suspend) the channel, can
    => Channel is RUNNING->STOP
    => Channel is STOP->RUNNING
   ...
5. When device is removed, MHI core reset the channel
    => channel is (RUNNING|STOP) -> DISABLED

Today mhi_prepare_for_transfer performs both ENABLE and RUNNING
transition, the idea would be to keep channel enabling/disabling in
the MHI core (before/after driver probe/remove) and channel start/stop
managed by the client driver.

Regards,
Loic
Bhaumik Bhatt Nov. 11, 2020, 12:40 a.m. UTC | #2
Hi Loic,

On 2020-11-10 03:14, Loic Poulain wrote:
> Hi Bhaumik,
> 
> On Mon, 9 Nov 2020 at 23:44, Bhaumik Bhatt <bbhatt@codeaurora.org> 
> wrote:
>> 
>> Some MHI clients may want to request for pausing or resuming of the
>> data transfers for their channels. Enable them to do so using the new
>> APIs provided for the same.
>> 
>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> ---
>>  drivers/bus/mhi/core/main.c | 41 
>> +++++++++++++++++++++++++++++++++++++++++
>>  include/linux/mhi.h         | 16 ++++++++++++++++
>>  2 files changed, 57 insertions(+)
>> 
>> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
>> index 1226933..01845c6 100644
>> --- a/drivers/bus/mhi/core/main.c
>> +++ b/drivers/bus/mhi/core/main.c
>> @@ -1560,6 +1560,47 @@ void mhi_unprepare_from_transfer(struct 
>> mhi_device *mhi_dev)
>>  }
>>  EXPORT_SYMBOL_GPL(mhi_unprepare_from_transfer);
>> 
>> +static int mhi_update_transfer_state(struct mhi_device *mhi_dev,
>> +                                    enum mhi_ch_state_type to_state)
>> +{
>> +       struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
>> +       struct mhi_chan *mhi_chan;
>> +       int dir, ret;
>> +
>> +       for (dir = 0; dir < 2; dir++) {
>> +               mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
>> +
>> +               if (!mhi_chan)
>> +                       continue;
>> +
>> +               /*
>> +                * Bail out if one of the channels fail as client will 
>> reset
>> +                * both upon failure
>> +                */
>> +               mutex_lock(&mhi_chan->mutex);
>> +               ret = mhi_update_channel_state(mhi_cntrl, mhi_chan, 
>> to_state);
>> +               if (ret) {
>> +                       mutex_unlock(&mhi_chan->mutex);
>> +                       return ret;
>> +               }
>> +               mutex_unlock(&mhi_chan->mutex);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +int mhi_pause_transfer(struct mhi_device *mhi_dev)
>> +{
>> +       return mhi_update_transfer_state(mhi_dev, 
>> MHI_CH_STATE_TYPE_STOP);
>> +}
>> +EXPORT_SYMBOL_GPL(mhi_pause_transfer);
>> +
>> +int mhi_resume_transfer(struct mhi_device *mhi_dev)
>> +{
>> +       return mhi_update_transfer_state(mhi_dev, 
>> MHI_CH_STATE_TYPE_START);
>> +}
>> +EXPORT_SYMBOL_GPL(mhi_resume_transfer);
> 
> Look like it is stop and start, not pause and resume?
I wanted to keep it pause and resume because it could get confusing for 
someone
looking at this pair of APIs, that a client driver would also need to 
"start"
channels after "preparing" them. Since that is not that case, and the
mhi_prepare_for_transfer() API itself is supposed to also start the 
channels, it
would be better to keep these as "pause" and "resume" instead IMO.

Any comments in favor or "stop" and "start"?
> 
> TBH maybe we should rework/clarify MHI core and having well-defined
> states, maybe something like that:
> 
> 1. When MHI core detects device for a driver, MHI core resets and
> initializes the channel(s), then call client driver probe function
>     => channel UNKNOWN->DISABLED state
>     => channel DISABLED->ENABLED state
> 2. When driver is ready for sending data, drivers calls 
> mhi_start_transfer
>     => Channel is ENABLED->RUNNING state
> 3. Driver performs normal data transfers
> 4. The driver can suspend/resume transfer, it stops (suspend) the 
> channel, can
>     => Channel is RUNNING->STOP
>     => Channel is STOP->RUNNING
>    ...
> 5. When device is removed, MHI core reset the channel
>     => channel is (RUNNING|STOP) -> DISABLED
> 
> Today mhi_prepare_for_transfer performs both ENABLE and RUNNING
> transition, the idea would be to keep channel enabling/disabling in
> the MHI core (before/after driver probe/remove) and channel start/stop
> managed by the client driver.
> 
> Regards,
> Loic

Your idea is good but it would not have much additional benefits and 
would
involve MHI core "enabling" channels and allocating memory for each 
channel
context when they are only declared as supported by the controller but 
are not
actually being put to use.

mhi_prepare_for_transfer() does both channel context initialization and 
starts
the channels, which is good because it allocates memory when needed. So, 
this
benefits system memory if a controller with support for many channels 
exists but
only a few channels are used.

Regarding the states to track from host:
-> DISABLED (We know channels are not active: in reset state or not 
probed yet)
-> ENABLED (Active and running when needed for data transfers)
-> STOP (Paused: leaves the channel context as is since channels are not 
reset)
-> SUSPENDED (Unload in progress: Entered before resetting 
channels/remove())

BTW, we have the debugfs entry for "channels" that dumps the context to 
show
exactly what the channel states are from device perspective. We can rely 
on it
if needed.

If there are some comments I can add to make things clear, please let me 
know.

Thanks,
Bhaumik
Loic Poulain Nov. 11, 2020, 9:33 a.m. UTC | #3
Hi Bhaumik,

On Wed, 11 Nov 2020 at 01:40, Bhaumik Bhatt <bbhatt@codeaurora.org> wrote:
>
> Hi Loic,
>
> On 2020-11-10 03:14, Loic Poulain wrote:
> > Hi Bhaumik,
> >
> > On Mon, 9 Nov 2020 at 23:44, Bhaumik Bhatt <bbhatt@codeaurora.org>
> > wrote:
> >>
> >> Some MHI clients may want to request for pausing or resuming of the
> >> data transfers for their channels. Enable them to do so using the new
> >> APIs provided for the same.
> >>
> >> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> >> ---
> >>  drivers/bus/mhi/core/main.c | 41
> >> +++++++++++++++++++++++++++++++++++++++++
> >>  include/linux/mhi.h         | 16 ++++++++++++++++
> >>  2 files changed, 57 insertions(+)
> >>
> >> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> >> index 1226933..01845c6 100644
> >> --- a/drivers/bus/mhi/core/main.c
> >> +++ b/drivers/bus/mhi/core/main.c
> >> @@ -1560,6 +1560,47 @@ void mhi_unprepare_from_transfer(struct
> >> mhi_device *mhi_dev)
> >>  }
> >>  EXPORT_SYMBOL_GPL(mhi_unprepare_from_transfer);
> >>
> >> +static int mhi_update_transfer_state(struct mhi_device *mhi_dev,
> >> +                                    enum mhi_ch_state_type to_state)
> >> +{
> >> +       struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> >> +       struct mhi_chan *mhi_chan;
> >> +       int dir, ret;
> >> +
> >> +       for (dir = 0; dir < 2; dir++) {
> >> +               mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
> >> +
> >> +               if (!mhi_chan)
> >> +                       continue;
> >> +
> >> +               /*
> >> +                * Bail out if one of the channels fail as client will
> >> reset
> >> +                * both upon failure
> >> +                */
> >> +               mutex_lock(&mhi_chan->mutex);
> >> +               ret = mhi_update_channel_state(mhi_cntrl, mhi_chan,
> >> to_state);
> >> +               if (ret) {
> >> +                       mutex_unlock(&mhi_chan->mutex);
> >> +                       return ret;
> >> +               }
> >> +               mutex_unlock(&mhi_chan->mutex);
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +int mhi_pause_transfer(struct mhi_device *mhi_dev)
> >> +{
> >> +       return mhi_update_transfer_state(mhi_dev,
> >> MHI_CH_STATE_TYPE_STOP);
> >> +}
> >> +EXPORT_SYMBOL_GPL(mhi_pause_transfer);
> >> +
> >> +int mhi_resume_transfer(struct mhi_device *mhi_dev)
> >> +{
> >> +       return mhi_update_transfer_state(mhi_dev,
> >> MHI_CH_STATE_TYPE_START);
> >> +}
> >> +EXPORT_SYMBOL_GPL(mhi_resume_transfer);
> >
> > Look like it is stop and start, not pause and resume?
> I wanted to keep it pause and resume because it could get confusing for
> someone
> looking at this pair of APIs, that a client driver would also need to
> "start"
> channels after "preparing" them. Since that is not that case, and the
> mhi_prepare_for_transfer() API itself is supposed to also start the
> channels, it

Yes, because prepare_for_transfer is actually init_and_start. I'm not
in favor of hiding what is really done at mhi_core level, start is
start and stop is stop, if it's correctly documented that should not
be confusing. just saying (stop moves channels in stop state, start in
enabled state), but other opinions are welcome.

> would be better to keep these as "pause" and "resume" instead IMO.
>
> Any comments in favor or "stop" and "start"?
> >
> > TBH maybe we should rework/clarify MHI core and having well-defined
> > states, maybe something like that:
> >
> > 1. When MHI core detects device for a driver, MHI core resets and
> > initializes the channel(s), then call client driver probe function
> >     => channel UNKNOWN->DISABLED state
> >     => channel DISABLED->ENABLED state
> > 2. When driver is ready for sending data, drivers calls
> > mhi_start_transfer
> >     => Channel is ENABLED->RUNNING state
> > 3. Driver performs normal data transfers
> > 4. The driver can suspend/resume transfer, it stops (suspend) the
> > channel, can
> >     => Channel is RUNNING->STOP
> >     => Channel is STOP->RUNNING
> >    ...
> > 5. When device is removed, MHI core reset the channel
> >     => channel is (RUNNING|STOP) -> DISABLED
> >
> > Today mhi_prepare_for_transfer performs both ENABLE and RUNNING
> > transition, the idea would be to keep channel enabling/disabling in
> > the MHI core (before/after driver probe/remove) and channel start/stop
> > managed by the client driver.
> >
> > Regards,
> > Loic
>
> Your idea is good but it would not have much additional benefits and
> would
> involve MHI core "enabling" channels and allocating memory for each
> channel
> context when they are only declared as supported by the controller but
> are not
> actually being put to use.

Ok, your point is valid.

>
> mhi_prepare_for_transfer() does both channel context initialization and
> starts
> the channels, which is good because it allocates memory when needed. So,
> this
> benefits system memory if a controller with support for many channels
> exists but
> only a few channels are used.
>
> Regarding the states to track from host:
> -> DISABLED (We know channels are not active: in reset state or not
> probed yet)
> -> ENABLED (Active and running when needed for data transfers)
> -> STOP (Paused: leaves the channel context as is since channels are not
> reset)
> -> SUSPENDED (Unload in progress: Entered before resetting
> channels/remove())
>
> BTW, we have the debugfs entry for "channels" that dumps the context to
> show
> exactly what the channel states are from device perspective. We can rely
> on it
> if needed.
>
> If there are some comments I can add to make things clear, please let me
> know.
>
> Thanks,
> Bhaumik
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
> a Linux Foundation Collaborative Project
Bhaumik Bhatt Nov. 11, 2020, 6:11 p.m. UTC | #4
Hi Loic,

On 2020-11-11 01:33, Loic Poulain wrote:
> Hi Bhaumik,
> 
> On Wed, 11 Nov 2020 at 01:40, Bhaumik Bhatt <bbhatt@codeaurora.org> 
> wrote:
>> 
>> Hi Loic,
>> 
>> On 2020-11-10 03:14, Loic Poulain wrote:
>> > Hi Bhaumik,
>> >
>> > On Mon, 9 Nov 2020 at 23:44, Bhaumik Bhatt <bbhatt@codeaurora.org>
>> > wrote:
>> >>
>> >> Some MHI clients may want to request for pausing or resuming of the
>> >> data transfers for their channels. Enable them to do so using the new
>> >> APIs provided for the same.
>> >>
>> >> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> >> ---
>> >>  drivers/bus/mhi/core/main.c | 41
>> >> +++++++++++++++++++++++++++++++++++++++++
>> >>  include/linux/mhi.h         | 16 ++++++++++++++++
>> >>  2 files changed, 57 insertions(+)
>> >>
>> >> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
>> >> index 1226933..01845c6 100644
>> >> --- a/drivers/bus/mhi/core/main.c
>> >> +++ b/drivers/bus/mhi/core/main.c
>> >> @@ -1560,6 +1560,47 @@ void mhi_unprepare_from_transfer(struct
>> >> mhi_device *mhi_dev)
>> >>  }
>> >>  EXPORT_SYMBOL_GPL(mhi_unprepare_from_transfer);
>> >>
>> >> +static int mhi_update_transfer_state(struct mhi_device *mhi_dev,
>> >> +                                    enum mhi_ch_state_type to_state)
>> >> +{
>> >> +       struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
>> >> +       struct mhi_chan *mhi_chan;
>> >> +       int dir, ret;
>> >> +
>> >> +       for (dir = 0; dir < 2; dir++) {
>> >> +               mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
>> >> +
>> >> +               if (!mhi_chan)
>> >> +                       continue;
>> >> +
>> >> +               /*
>> >> +                * Bail out if one of the channels fail as client will
>> >> reset
>> >> +                * both upon failure
>> >> +                */
>> >> +               mutex_lock(&mhi_chan->mutex);
>> >> +               ret = mhi_update_channel_state(mhi_cntrl, mhi_chan,
>> >> to_state);
>> >> +               if (ret) {
>> >> +                       mutex_unlock(&mhi_chan->mutex);
>> >> +                       return ret;
>> >> +               }
>> >> +               mutex_unlock(&mhi_chan->mutex);
>> >> +       }
>> >> +
>> >> +       return 0;
>> >> +}
>> >> +
>> >> +int mhi_pause_transfer(struct mhi_device *mhi_dev)
>> >> +{
>> >> +       return mhi_update_transfer_state(mhi_dev,
>> >> MHI_CH_STATE_TYPE_STOP);
>> >> +}
>> >> +EXPORT_SYMBOL_GPL(mhi_pause_transfer);
>> >> +
>> >> +int mhi_resume_transfer(struct mhi_device *mhi_dev)
>> >> +{
>> >> +       return mhi_update_transfer_state(mhi_dev,
>> >> MHI_CH_STATE_TYPE_START);
>> >> +}
>> >> +EXPORT_SYMBOL_GPL(mhi_resume_transfer);
>> >
>> > Look like it is stop and start, not pause and resume?
>> I wanted to keep it pause and resume because it could get confusing 
>> for
>> someone
>> looking at this pair of APIs, that a client driver would also need to
>> "start"
>> channels after "preparing" them. Since that is not that case, and the
>> mhi_prepare_for_transfer() API itself is supposed to also start the
>> channels, it
> 
> Yes, because prepare_for_transfer is actually init_and_start. I'm not
> in favor of hiding what is really done at mhi_core level, start is
> start and stop is stop, if it's correctly documented that should not
> be confusing. just saying (stop moves channels in stop state, start in
> enabled state), but other opinions are welcome.
> 
I can rename it and have it documented in the mhi_prepare_for_transfer() 
API
that we actually already start the channel, so it is not required to be 
used
at first. I can improve this documentation in mhi.h as a separate patch.

Later, if a client driver wants to issue stop and start commands, it can 
do so.
I'm not too picky with the name. Maybe Mani or someone else may have 
more
comments.

Thanks for looking in to this.
>> would be better to keep these as "pause" and "resume" instead IMO.
>> 
>> Any comments in favor or "stop" and "start"?
>> >
>> > TBH maybe we should rework/clarify MHI core and having well-defined
>> > states, maybe something like that:
>> >
>> > 1. When MHI core detects device for a driver, MHI core resets and
>> > initializes the channel(s), then call client driver probe function
>> >     => channel UNKNOWN->DISABLED state
>> >     => channel DISABLED->ENABLED state
>> > 2. When driver is ready for sending data, drivers calls
>> > mhi_start_transfer
>> >     => Channel is ENABLED->RUNNING state
>> > 3. Driver performs normal data transfers
>> > 4. The driver can suspend/resume transfer, it stops (suspend) the
>> > channel, can
>> >     => Channel is RUNNING->STOP
>> >     => Channel is STOP->RUNNING
>> >    ...
>> > 5. When device is removed, MHI core reset the channel
>> >     => channel is (RUNNING|STOP) -> DISABLED
>> >
>> > Today mhi_prepare_for_transfer performs both ENABLE and RUNNING
>> > transition, the idea would be to keep channel enabling/disabling in
>> > the MHI core (before/after driver probe/remove) and channel start/stop
>> > managed by the client driver.
>> >
>> > Regards,
>> > Loic
>> 
>> Your idea is good but it would not have much additional benefits and
>> would
>> involve MHI core "enabling" channels and allocating memory for each
>> channel
>> context when they are only declared as supported by the controller but
>> are not
>> actually being put to use.
> 
> Ok, your point is valid.
> 
>> 
>> mhi_prepare_for_transfer() does both channel context initialization 
>> and
>> starts
>> the channels, which is good because it allocates memory when needed. 
>> So,
>> this
>> benefits system memory if a controller with support for many channels
>> exists but
>> only a few channels are used.
>> 
>> Regarding the states to track from host:
>> -> DISABLED (We know channels are not active: in reset state or not
>> probed yet)
>> -> ENABLED (Active and running when needed for data transfers)
>> -> STOP (Paused: leaves the channel context as is since channels are 
>> not
>> reset)
>> -> SUSPENDED (Unload in progress: Entered before resetting
>> channels/remove())
>> 
>> BTW, we have the debugfs entry for "channels" that dumps the context 
>> to
>> show
>> exactly what the channel states are from device perspective. We can 
>> rely
>> on it
>> if needed.
>> 
>> If there are some comments I can add to make things clear, please let 
>> me
>> know.
>> 
>> Thanks,
>> Bhaumik
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project

Thanks,
Bhaumik
Manivannan Sadhasivam Nov. 12, 2020, 3:24 a.m. UTC | #5
On Wed, Nov 11, 2020 at 10:11:37AM -0800, Bhaumik Bhatt wrote:
> Hi Loic,
> 

[...]

> > > > Look like it is stop and start, not pause and resume?
> > > I wanted to keep it pause and resume because it could get confusing
> > > for
> > > someone
> > > looking at this pair of APIs, that a client driver would also need to
> > > "start"
> > > channels after "preparing" them. Since that is not that case, and the
> > > mhi_prepare_for_transfer() API itself is supposed to also start the
> > > channels, it
> > 
> > Yes, because prepare_for_transfer is actually init_and_start. I'm not
> > in favor of hiding what is really done at mhi_core level, start is
> > start and stop is stop, if it's correctly documented that should not
> > be confusing. just saying (stop moves channels in stop state, start in
> > enabled state), but other opinions are welcome.
> > 
> I can rename it and have it documented in the mhi_prepare_for_transfer() API
> that we actually already start the channel, so it is not required to be used
> at first. I can improve this documentation in mhi.h as a separate patch.
> 
> Later, if a client driver wants to issue stop and start commands, it can do
> so.
> I'm not too picky with the name. Maybe Mani or someone else may have more
> comments.
> 

Please use start and stop to match what the function is doing. We should always
name the APIs with respect to their function.

Thanks,
Mani
diff mbox series

Patch

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 1226933..01845c6 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -1560,6 +1560,47 @@  void mhi_unprepare_from_transfer(struct mhi_device *mhi_dev)
 }
 EXPORT_SYMBOL_GPL(mhi_unprepare_from_transfer);
 
+static int mhi_update_transfer_state(struct mhi_device *mhi_dev,
+				     enum mhi_ch_state_type to_state)
+{
+	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+	struct mhi_chan *mhi_chan;
+	int dir, ret;
+
+	for (dir = 0; dir < 2; dir++) {
+		mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
+
+		if (!mhi_chan)
+			continue;
+
+		/*
+		 * Bail out if one of the channels fail as client will reset
+		 * both upon failure
+		 */
+		mutex_lock(&mhi_chan->mutex);
+		ret = mhi_update_channel_state(mhi_cntrl, mhi_chan, to_state);
+		if (ret) {
+			mutex_unlock(&mhi_chan->mutex);
+			return ret;
+		}
+		mutex_unlock(&mhi_chan->mutex);
+	}
+
+	return 0;
+}
+
+int mhi_pause_transfer(struct mhi_device *mhi_dev)
+{
+	return mhi_update_transfer_state(mhi_dev, MHI_CH_STATE_TYPE_STOP);
+}
+EXPORT_SYMBOL_GPL(mhi_pause_transfer);
+
+int mhi_resume_transfer(struct mhi_device *mhi_dev)
+{
+	return mhi_update_transfer_state(mhi_dev, MHI_CH_STATE_TYPE_START);
+}
+EXPORT_SYMBOL_GPL(mhi_resume_transfer);
+
 int mhi_poll(struct mhi_device *mhi_dev, u32 budget)
 {
 	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 52b3c60..4d48d49 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -702,6 +702,22 @@  int mhi_prepare_for_transfer(struct mhi_device *mhi_dev);
 void mhi_unprepare_from_transfer(struct mhi_device *mhi_dev);
 
 /**
+ * mhi_pause_transfer - Pause ongoing channel activity
+ * Moves both UL and DL channels to STOP state to halt
+ * pending transfers.
+ * @mhi_dev: Device associated with the channels
+ */
+int mhi_pause_transfer(struct mhi_device *mhi_dev);
+
+/**
+ * mhi_resume_transfer - Resume channel activity
+ * Moves both UL and DL channels to START state to
+ * resume transfers.
+ * @mhi_dev: Device associated with the channels
+ */
+int mhi_resume_transfer(struct mhi_device *mhi_dev);
+
+/**
  * mhi_poll - Poll for any available data in DL direction
  * @mhi_dev: Device associated with the channels
  * @budget: # of events to process