From 4b2274d3811a25831068025ce20bf2adbd48c101 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Fri, 23 Feb 2024 07:39:25 -0600 Subject: net: ipa: don't bother aborting system resume The IPA interrupt can fire if there is data to be delivered to a GSI channel that is suspended. This condition occurs in three scenarios. First, runtime power management automatically suspends the IPA hardware after half a second of inactivity. This has nothing to do with system suspend, so a SYSTEM IPA power flag is used to avoid calling pm_wakeup_dev_event() when runtime suspended. Second, if the system is suspended, the receipt of an IPA interrupt should trigger a system resume. Configuring the IPA interrupt for wakeup accomplishes this. Finally, if system suspend is underway and the IPA interrupt fires, we currently call pm_wakeup_dev_event() to abort the system suspend. The IPA driver correctly handles quiescing the hardware before suspending it, so there's really no need to abort a suspend in progress in the third case. We can simply quiesce and suspend things, and be done. Incoming data can still wake the system after it's suspended. The IPA interrupt has wakeup mode enabled, so if it fires *after* we've suspended, it will trigger a wakeup (if not disabled via sysfs). Stop calling pm_wakeup_dev_event() to abort a system suspend in progress in ipa_power_suspend_handler(). Signed-off-by: Alex Elder Signed-off-by: Paolo Abeni --- drivers/net/ipa/ipa_power.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/ipa_power.c b/drivers/net/ipa/ipa_power.c index 128a816f6523..694bc71e0a17 100644 --- a/drivers/net/ipa/ipa_power.c +++ b/drivers/net/ipa/ipa_power.c @@ -220,8 +220,9 @@ void ipa_power_suspend_handler(struct ipa *ipa, enum ipa_irq_id irq_id) * system suspend, trigger a system resume. */ if (!__test_and_set_bit(IPA_POWER_FLAG_RESUMED, ipa->power->flags)) - if (test_bit(IPA_POWER_FLAG_SYSTEM, ipa->power->flags)) - pm_wakeup_dev_event(&ipa->pdev->dev, 0, true); + if (test_bit(IPA_POWER_FLAG_SYSTEM, ipa->power->flags)) { + ; + } /* Acknowledge/clear the suspend interrupt on all endpoints */ ipa_interrupt_suspend_clear_all(ipa->interrupt); -- cgit v1.2.3-59-g8ed1b From 54f19ff7676fc60572f1e4725908094032523a95 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Fri, 23 Feb 2024 07:39:26 -0600 Subject: net: ipa: kill IPA_POWER_FLAG_SYSTEM The SYSTEM IPA power flag is set, cleared, and tested. But nothing happens based on its value when tested, so it serves no purpose. Get rid of this flag. Signed-off-by: Alex Elder Signed-off-by: Paolo Abeni --- drivers/net/ipa/ipa_power.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/net/ipa/ipa_power.c b/drivers/net/ipa/ipa_power.c index 694bc71e0a17..be9e859e853f 100644 --- a/drivers/net/ipa/ipa_power.c +++ b/drivers/net/ipa/ipa_power.c @@ -37,12 +37,10 @@ /** * enum ipa_power_flag - IPA power flags * @IPA_POWER_FLAG_RESUMED: Whether resume from suspend has been signaled - * @IPA_POWER_FLAG_SYSTEM: Hardware is system (not runtime) suspended * @IPA_POWER_FLAG_COUNT: Number of defined power flags */ enum ipa_power_flag { IPA_POWER_FLAG_RESUMED, - IPA_POWER_FLAG_SYSTEM, IPA_POWER_FLAG_COUNT, /* Last; not a flag */ }; @@ -173,8 +171,6 @@ static int ipa_suspend(struct device *dev) { struct ipa *ipa = dev_get_drvdata(dev); - __set_bit(IPA_POWER_FLAG_SYSTEM, ipa->power->flags); - /* Increment the disable depth to ensure that the IRQ won't * be re-enabled until the matching _enable call in * ipa_resume(). We do this to ensure that the interrupt @@ -196,8 +192,6 @@ static int ipa_resume(struct device *dev) ret = pm_runtime_force_resume(dev); - __clear_bit(IPA_POWER_FLAG_SYSTEM, ipa->power->flags); - /* Now that PM runtime is enabled again it's safe * to turn the IRQ back on and process any data * that was received during suspend. @@ -219,10 +213,9 @@ void ipa_power_suspend_handler(struct ipa *ipa, enum ipa_irq_id irq_id) * just to handle the interrupt, so we're done. If we are in a * system suspend, trigger a system resume. */ - if (!__test_and_set_bit(IPA_POWER_FLAG_RESUMED, ipa->power->flags)) - if (test_bit(IPA_POWER_FLAG_SYSTEM, ipa->power->flags)) { - ; - } + if (!__test_and_set_bit(IPA_POWER_FLAG_RESUMED, ipa->power->flags)) { + ; + } /* Acknowledge/clear the suspend interrupt on all endpoints */ ipa_interrupt_suspend_clear_all(ipa->interrupt); -- cgit v1.2.3-59-g8ed1b From dae5d6e8f0ecbed0481a1a1cc11399f65b0a9b8a Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Fri, 23 Feb 2024 07:39:27 -0600 Subject: net: ipa: kill the IPA_POWER_FLAG_RESUMED flag The IPA_POWER_FLAG_RESUMED was originally used to avoid calling pm_wakeup_dev_event() more than once when handling a SUSPEND interrupt. This call is no longer made, so there' no need for the flag, so get rid of it. That leaves no more IPA power flags usefully defined, so just get rid of the bitmap in the IPA power structure and the definition of the ipa_power_flag enumerated type. Signed-off-by: Alex Elder Signed-off-by: Paolo Abeni --- drivers/net/ipa/ipa_power.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/drivers/net/ipa/ipa_power.c b/drivers/net/ipa/ipa_power.c index be9e859e853f..eee251d67f81 100644 --- a/drivers/net/ipa/ipa_power.c +++ b/drivers/net/ipa/ipa_power.c @@ -34,22 +34,11 @@ #define IPA_AUTOSUSPEND_DELAY 500 /* milliseconds */ -/** - * enum ipa_power_flag - IPA power flags - * @IPA_POWER_FLAG_RESUMED: Whether resume from suspend has been signaled - * @IPA_POWER_FLAG_COUNT: Number of defined power flags - */ -enum ipa_power_flag { - IPA_POWER_FLAG_RESUMED, - IPA_POWER_FLAG_COUNT, /* Last; not a flag */ -}; - /** * struct ipa_power - IPA power management information * @dev: IPA device pointer * @core: IPA core clock * @qmp: QMP handle for AOSS communication - * @flags: Boolean state flags * @interconnect_count: Number of elements in interconnect[] * @interconnect: Interconnect array */ @@ -57,7 +46,6 @@ struct ipa_power { struct device *dev; struct clk *core; struct qmp *qmp; - DECLARE_BITMAP(flags, IPA_POWER_FLAG_COUNT); u32 interconnect_count; struct icc_bulk_data interconnect[] __counted_by(interconnect_count); }; @@ -139,7 +127,6 @@ static int ipa_runtime_suspend(struct device *dev) /* Endpoints aren't usable until setup is complete */ if (ipa->setup_complete) { - __clear_bit(IPA_POWER_FLAG_RESUMED, ipa->power->flags); ipa_endpoint_suspend(ipa); gsi_suspend(&ipa->gsi); } @@ -209,14 +196,6 @@ u32 ipa_core_clock_rate(struct ipa *ipa) void ipa_power_suspend_handler(struct ipa *ipa, enum ipa_irq_id irq_id) { - /* To handle an IPA interrupt we will have resumed the hardware - * just to handle the interrupt, so we're done. If we are in a - * system suspend, trigger a system resume. - */ - if (!__test_and_set_bit(IPA_POWER_FLAG_RESUMED, ipa->power->flags)) { - ; - } - /* Acknowledge/clear the suspend interrupt on all endpoints */ ipa_interrupt_suspend_clear_all(ipa->interrupt); } -- cgit v1.2.3-59-g8ed1b From ef63ca78da2e5cfbf7b795345cb9649902b77022 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Fri, 23 Feb 2024 07:39:28 -0600 Subject: net: ipa: move ipa_interrupt_suspend_clear_all() up The next patch makes ipa_interrupt_suspend_clear_all() static, calling it only within "ipa_interrupt.c". Move its definition higher in the file so no declaration is needed. Signed-off-by: Alex Elder Signed-off-by: Paolo Abeni --- drivers/net/ipa/ipa_interrupt.c | 48 ++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/drivers/net/ipa/ipa_interrupt.c b/drivers/net/ipa/ipa_interrupt.c index a78c692f2d3c..e5e01655e8c2 100644 --- a/drivers/net/ipa/ipa_interrupt.c +++ b/drivers/net/ipa/ipa_interrupt.c @@ -43,6 +43,30 @@ struct ipa_interrupt { u32 enabled; }; +/* Clear the suspend interrupt for all endpoints that signaled it */ +void ipa_interrupt_suspend_clear_all(struct ipa_interrupt *interrupt) +{ + struct ipa *ipa = interrupt->ipa; + u32 unit_count; + u32 unit; + + unit_count = DIV_ROUND_UP(ipa->endpoint_count, 32); + for (unit = 0; unit < unit_count; unit++) { + const struct reg *reg; + u32 val; + + reg = ipa_reg(ipa, IRQ_SUSPEND_INFO); + val = ioread32(ipa->reg_virt + reg_n_offset(reg, unit)); + + /* SUSPEND interrupt status isn't cleared on IPA version 3.0 */ + if (ipa->version == IPA_VERSION_3_0) + continue; + + reg = ipa_reg(ipa, IRQ_SUSPEND_CLR); + iowrite32(val, ipa->reg_virt + reg_n_offset(reg, unit)); + } +} + /* Process a particular interrupt type that has been received */ static void ipa_interrupt_process(struct ipa_interrupt *interrupt, u32 irq_id) { @@ -205,30 +229,6 @@ ipa_interrupt_suspend_disable(struct ipa_interrupt *interrupt, u32 endpoint_id) ipa_interrupt_suspend_control(interrupt, endpoint_id, false); } -/* Clear the suspend interrupt for all endpoints that signaled it */ -void ipa_interrupt_suspend_clear_all(struct ipa_interrupt *interrupt) -{ - struct ipa *ipa = interrupt->ipa; - u32 unit_count; - u32 unit; - - unit_count = DIV_ROUND_UP(ipa->endpoint_count, 32); - for (unit = 0; unit < unit_count; unit++) { - const struct reg *reg; - u32 val; - - reg = ipa_reg(ipa, IRQ_SUSPEND_INFO); - val = ioread32(ipa->reg_virt + reg_n_offset(reg, unit)); - - /* SUSPEND interrupt status isn't cleared on IPA version 3.0 */ - if (ipa->version == IPA_VERSION_3_0) - continue; - - reg = ipa_reg(ipa, IRQ_SUSPEND_CLR); - iowrite32(val, ipa->reg_virt + reg_n_offset(reg, unit)); - } -} - /* Simulate arrival of an IPA TX_SUSPEND interrupt */ void ipa_interrupt_simulate_suspend(struct ipa_interrupt *interrupt) { -- cgit v1.2.3-59-g8ed1b From 423df2e09d3b893ce83bc35ca81657829604968e Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Fri, 23 Feb 2024 07:39:29 -0600 Subject: net: ipa: kill ipa_power_suspend_handler() Now that ipa_power_suspend_handler() is a trivial wrapper around ipa_interrupt_suspend_clear_all(), we can open-code it in the one place it's used, and get rid of the function. Signed-off-by: Alex Elder Signed-off-by: Paolo Abeni --- drivers/net/ipa/ipa_interrupt.c | 4 ++-- drivers/net/ipa/ipa_interrupt.h | 8 -------- drivers/net/ipa/ipa_power.c | 6 ------ drivers/net/ipa/ipa_power.h | 11 ----------- 4 files changed, 2 insertions(+), 27 deletions(-) diff --git a/drivers/net/ipa/ipa_interrupt.c b/drivers/net/ipa/ipa_interrupt.c index e5e01655e8c2..501962cc4e90 100644 --- a/drivers/net/ipa/ipa_interrupt.c +++ b/drivers/net/ipa/ipa_interrupt.c @@ -44,7 +44,7 @@ struct ipa_interrupt { }; /* Clear the suspend interrupt for all endpoints that signaled it */ -void ipa_interrupt_suspend_clear_all(struct ipa_interrupt *interrupt) +static void ipa_interrupt_suspend_clear_all(struct ipa_interrupt *interrupt) { struct ipa *ipa = interrupt->ipa; u32 unit_count; @@ -94,7 +94,7 @@ static void ipa_interrupt_process(struct ipa_interrupt *interrupt, u32 irq_id) * caused the interrupt, so defer clearing until after * the handler has been called. */ - ipa_power_suspend_handler(ipa, irq_id); + ipa_interrupt_suspend_clear_all(interrupt); fallthrough; default: /* Silently ignore (and clear) any other condition */ diff --git a/drivers/net/ipa/ipa_interrupt.h b/drivers/net/ipa/ipa_interrupt.h index 12e3e798ccb3..53e1b71685c7 100644 --- a/drivers/net/ipa/ipa_interrupt.h +++ b/drivers/net/ipa/ipa_interrupt.h @@ -34,14 +34,6 @@ void ipa_interrupt_suspend_enable(struct ipa_interrupt *interrupt, void ipa_interrupt_suspend_disable(struct ipa_interrupt *interrupt, u32 endpoint_id); -/** - * ipa_interrupt_suspend_clear_all - clear all suspend interrupts - * @interrupt: IPA interrupt structure - * - * Clear the TX_SUSPEND interrupt for all endpoints that signaled it. - */ -void ipa_interrupt_suspend_clear_all(struct ipa_interrupt *interrupt); - /** * ipa_interrupt_simulate_suspend() - Simulate TX_SUSPEND IPA interrupt * @interrupt: IPA interrupt structure diff --git a/drivers/net/ipa/ipa_power.c b/drivers/net/ipa/ipa_power.c index eee251d67f81..0f635b8356bf 100644 --- a/drivers/net/ipa/ipa_power.c +++ b/drivers/net/ipa/ipa_power.c @@ -194,12 +194,6 @@ u32 ipa_core_clock_rate(struct ipa *ipa) return ipa->power ? (u32)clk_get_rate(ipa->power->core) : 0; } -void ipa_power_suspend_handler(struct ipa *ipa, enum ipa_irq_id irq_id) -{ - /* Acknowledge/clear the suspend interrupt on all endpoints */ - ipa_interrupt_suspend_clear_all(ipa->interrupt); -} - static int ipa_power_retention_init(struct ipa_power *power) { struct qmp *qmp = qmp_get(power->dev); diff --git a/drivers/net/ipa/ipa_power.h b/drivers/net/ipa/ipa_power.h index 718aacf5e2b2..227cc04bea80 100644 --- a/drivers/net/ipa/ipa_power.h +++ b/drivers/net/ipa/ipa_power.h @@ -30,17 +30,6 @@ u32 ipa_core_clock_rate(struct ipa *ipa); */ void ipa_power_retention(struct ipa *ipa, bool enable); -/** - * ipa_power_suspend_handler() - Handler for SUSPEND IPA interrupts - * @ipa: IPA pointer - * @irq_id: IPA interrupt ID (unused) - * - * If an RX endpoint is suspended, and the IPA has a packet destined for - * that endpoint, the IPA generates a SUSPEND interrupt to inform the AP - * that it should resume the endpoint. - */ -void ipa_power_suspend_handler(struct ipa *ipa, enum ipa_irq_id irq_id); - /** * ipa_power_setup() - Set up IPA power management * @ipa: IPA pointer -- cgit v1.2.3-59-g8ed1b From f9345952e74a77ba905b5a23252bffde48162ef3 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Fri, 23 Feb 2024 07:39:30 -0600 Subject: net: ipa: don't bother zeroing an already zero register In ipa_interrupt_suspend_clear_all(), if the SUSPEND_INFO register read contains no set bits, there's no interrupt condition to clear. Skip the write to the clear register in that case. Signed-off-by: Alex Elder Signed-off-by: Paolo Abeni --- drivers/net/ipa/ipa_interrupt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ipa/ipa_interrupt.c b/drivers/net/ipa/ipa_interrupt.c index 501962cc4e90..4d80bf77a532 100644 --- a/drivers/net/ipa/ipa_interrupt.c +++ b/drivers/net/ipa/ipa_interrupt.c @@ -59,7 +59,7 @@ static void ipa_interrupt_suspend_clear_all(struct ipa_interrupt *interrupt) val = ioread32(ipa->reg_virt + reg_n_offset(reg, unit)); /* SUSPEND interrupt status isn't cleared on IPA version 3.0 */ - if (ipa->version == IPA_VERSION_3_0) + if (!val || ipa->version == IPA_VERSION_3_0) continue; reg = ipa_reg(ipa, IRQ_SUSPEND_CLR); -- cgit v1.2.3-59-g8ed1b