On Fri 11 Sep 19:45 CDT 2020, Alex Elder wrote:
> Currently, when (before) the last IPA clock reference is dropped,
> all endpoints are suspended. And whenever the first IPA clock
> reference is taken, all endpoints are resumed (or started).
>
> In most cases there's no need to start endpoints when the clock
> starts. So move the calls to ipa_endpoint_suspend() and
> ipa_endpoint_resume() out of ipa_clock_put() and ipa_clock_get(),
> respectiely. Instead, only suspend endpoints when handling a system
> suspend, and only resume endpoints when handling a system resume.
>
Reviewed-by: Bjorn Andersson <bjorn.anders...@linaro.org>
Regards,
Bjorn
> Signed-off-by: Alex Elder <el...@linaro.org>
> ---
> drivers/net/ipa/ipa_clock.c | 14 ++++----------
> drivers/net/ipa/ipa_main.c | 8 ++++++++
> 2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c
> index b703866f2e20b..a2c0fde058199 100644
> --- a/drivers/net/ipa/ipa_clock.c
> +++ b/drivers/net/ipa/ipa_clock.c
> @@ -200,9 +200,8 @@ bool ipa_clock_get_additional(struct ipa *ipa)
>
> /* Get an IPA clock reference. If the reference count is non-zero, it is
> * incremented and return is immediate. Otherwise it is checked again
> - * under protection of the mutex, and if appropriate the clock (and
> - * interconnects) are enabled suspended endpoints (if any) are resumed
> - * before returning.
> + * under protection of the mutex, and if appropriate the IPA clock
> + * is enabled.
> *
> * Incrementing the reference count is intentionally deferred until
> * after the clock is running and endpoints are resumed.
> @@ -229,17 +228,14 @@ void ipa_clock_get(struct ipa *ipa)
> goto out_mutex_unlock;
> }
>
> - ipa_endpoint_resume(ipa);
> -
> refcount_set(&clock->count, 1);
>
> out_mutex_unlock:
> mutex_unlock(&clock->mutex);
> }
>
> -/* Attempt to remove an IPA clock reference. If this represents the last
> - * reference, suspend endpoints and disable the clock (and interconnects)
> - * under protection of a mutex.
> +/* Attempt to remove an IPA clock reference. If this represents the
> + * last reference, disable the IPA clock under protection of the mutex.
> */
> void ipa_clock_put(struct ipa *ipa)
> {
> @@ -249,8 +245,6 @@ void ipa_clock_put(struct ipa *ipa)
> if (!refcount_dec_and_mutex_lock(&clock->count, &clock->mutex))
> return;
>
> - ipa_endpoint_suspend(ipa);
> -
> ipa_clock_disable(ipa);
>
> mutex_unlock(&clock->mutex);
> diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c
> index cfdf60ded86ca..3b68b53c99015 100644
> --- a/drivers/net/ipa/ipa_main.c
> +++ b/drivers/net/ipa/ipa_main.c
> @@ -913,11 +913,15 @@ static int ipa_remove(struct platform_device *pdev)
> * Return: Always returns zero
> *
> * Called by the PM framework when a system suspend operation is invoked.
> + * Suspends endpoints and releases the clock reference held to keep
> + * the IPA clock running until this point.
> */
> static int ipa_suspend(struct device *dev)
> {
> struct ipa *ipa = dev_get_drvdata(dev);
>
> + ipa_endpoint_suspend(ipa);
> +
> ipa_clock_put(ipa);
> if (!test_and_clear_bit(IPA_FLAG_CLOCK_HELD, ipa->flags))
> dev_err(dev, "suspend: missing suspend clock reference\n");
> @@ -932,6 +936,8 @@ static int ipa_suspend(struct device *dev)
> * Return: Always returns 0
> *
> * Called by the PM framework when a system resume operation is invoked.
> + * Takes an IPA clock reference to keep the clock running until suspend,
> + * and resumes endpoints.
> */
> static int ipa_resume(struct device *dev)
> {
> @@ -945,6 +951,8 @@ static int ipa_resume(struct device *dev)
> else
> dev_err(dev, "resume: duplicate suspend clock reference\n");
>
> + ipa_endpoint_resume(ipa);
> +
> return 0;
> }
>
> --
> 2.20.1
>
