| Message ID | 20200912004532.1386-5-elder@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net: ipa: wake up system on RX available | expand |
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.andersson@linaro.org> Regards, Bjorn > Signed-off-by: Alex Elder <elder@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 >
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; }
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. Signed-off-by: Alex Elder <elder@linaro.org> --- drivers/net/ipa/ipa_clock.c | 14 ++++---------- drivers/net/ipa/ipa_main.c | 8 ++++++++ 2 files changed, 12 insertions(+), 10 deletions(-) -- 2.20.1