From d430fe4bac024812f50b8a2ad7a3639128c9db06 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 12 Aug 2021 14:50:30 -0500 Subject: [PATCH 1/6] net: ipa: enable wakeup in ipa_power_setup() Move the call to enable the IPA interrupt as a wakeup interrupt into ipa_power_setup(), disable it in ipa_power_teardown(). Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_clock.c | 11 ++++++++++- drivers/net/ipa/ipa_clock.h | 4 +++- drivers/net/ipa/ipa_main.c | 6 +----- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c index 6df66c574d5942..cdbaba6618e9e2 100644 --- a/drivers/net/ipa/ipa_clock.c +++ b/drivers/net/ipa/ipa_clock.c @@ -310,14 +310,23 @@ static void ipa_suspend_handler(struct ipa *ipa, enum ipa_irq_id irq_id) ipa_interrupt_suspend_clear_all(ipa->interrupt); } -void ipa_power_setup(struct ipa *ipa) +int ipa_power_setup(struct ipa *ipa) { + int ret; + ipa_interrupt_add(ipa->interrupt, IPA_IRQ_TX_SUSPEND, ipa_suspend_handler); + + ret = device_init_wakeup(&ipa->pdev->dev, true); + if (ret) + ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_TX_SUSPEND); + + return ret; } void ipa_power_teardown(struct ipa *ipa) { + (void)device_init_wakeup(&ipa->pdev->dev, false); ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_TX_SUSPEND); } diff --git a/drivers/net/ipa/ipa_clock.h b/drivers/net/ipa/ipa_clock.h index 5c118f2c42e7a7..5c53241336a1a0 100644 --- a/drivers/net/ipa/ipa_clock.h +++ b/drivers/net/ipa/ipa_clock.h @@ -25,8 +25,10 @@ u32 ipa_clock_rate(struct ipa *ipa); /** * ipa_power_setup() - Set up IPA power management * @ipa: IPA pointer + * + * Return: 0 if successful, or a negative error code */ -void ipa_power_setup(struct ipa *ipa); +int ipa_power_setup(struct ipa *ipa); /** * ipa_power_teardown() - Inverse of ipa_power_setup() diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index 581b75488c6fea..69fa4b3120fd3f 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -101,9 +101,7 @@ int ipa_setup(struct ipa *ipa) if (ret) return ret; - ipa_power_setup(ipa); - - ret = device_init_wakeup(dev, true); + ret = ipa_power_setup(ipa); if (ret) goto err_gsi_teardown; @@ -154,7 +152,6 @@ int ipa_setup(struct ipa *ipa) err_endpoint_teardown: ipa_endpoint_teardown(ipa); ipa_power_teardown(ipa); - (void)device_init_wakeup(dev, false); err_gsi_teardown: gsi_teardown(&ipa->gsi); @@ -181,7 +178,6 @@ static void ipa_teardown(struct ipa *ipa) ipa_endpoint_disable_one(command_endpoint); ipa_endpoint_teardown(ipa); ipa_power_teardown(ipa); - (void)device_init_wakeup(&ipa->pdev->dev, false); gsi_teardown(&ipa->gsi); } From b9c532c11cab21d23a67c2d80a02a444c9e07ac6 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 12 Aug 2021 14:50:31 -0500 Subject: [PATCH 2/6] net: ipa: distinguish system from runtime suspend Add a new flag that is set when the hardware is suspended due to a system suspend operation, distingishing it from runtime suspend. Use it in the SUSPEND IPA interrupt handler to determine whether to trigger a system resume because of the event. Define new suspend and resume power management callback functions to set and clear the new flag, respectively. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_clock.c | 38 ++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c index cdbaba6618e9e2..8f25107c1f1e7b 100644 --- a/drivers/net/ipa/ipa_clock.c +++ b/drivers/net/ipa/ipa_clock.c @@ -47,10 +47,12 @@ struct ipa_interconnect { /** * 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 */ }; @@ -281,6 +283,27 @@ int ipa_clock_put(struct ipa *ipa) return pm_runtime_put(&ipa->pdev->dev); } +static int ipa_suspend(struct device *dev) +{ + struct ipa *ipa = dev_get_drvdata(dev); + + __set_bit(IPA_POWER_FLAG_SYSTEM, ipa->clock->flags); + + return pm_runtime_force_suspend(dev); +} + +static int ipa_resume(struct device *dev) +{ + struct ipa *ipa = dev_get_drvdata(dev); + int ret; + + ret = pm_runtime_force_resume(dev); + + __clear_bit(IPA_POWER_FLAG_SYSTEM, ipa->clock->flags); + + return ret; +} + /* Return the current IPA core clock rate */ u32 ipa_clock_rate(struct ipa *ipa) { @@ -299,12 +322,13 @@ u32 ipa_clock_rate(struct ipa *ipa) */ static void ipa_suspend_handler(struct ipa *ipa, enum ipa_irq_id irq_id) { - /* Just report the event, and let system resume handle the rest. - * More than one endpoint could signal this; if so, ignore - * all but the first. + /* 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->clock->flags)) - pm_wakeup_dev_event(&ipa->pdev->dev, 0, true); + if (!__test_and_set_bit(IPA_POWER_FLAG_RESUMED, ipa->clock->flags)) + if (test_bit(IPA_POWER_FLAG_SYSTEM, ipa->clock->flags)) + pm_wakeup_dev_event(&ipa->pdev->dev, 0, true); /* Acknowledge/clear the suspend interrupt on all endpoints */ ipa_interrupt_suspend_clear_all(ipa->interrupt); @@ -390,8 +414,8 @@ void ipa_clock_exit(struct ipa_clock *clock) } const struct dev_pm_ops ipa_pm_ops = { - .suspend = pm_runtime_force_suspend, - .resume = pm_runtime_force_resume, + .suspend = ipa_suspend, + .resume = ipa_resume, .runtime_suspend = ipa_runtime_suspend, .runtime_resume = ipa_runtime_resume, .runtime_idle = ipa_runtime_idle, From a96e73fa1269a1d1b932f465ed0a803d4c153258 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 12 Aug 2021 14:50:32 -0500 Subject: [PATCH 3/6] net: ipa: re-enable transmit in PM WQ context Create a new work structure in the modem private data, and use it to re-enable the modem network device transmit queue when resuming. This is needed by the next patch, which stops the TX queue if IPA power isn't active when a transmit request arrives. Packets will start arriving the instant the TX queue is enabled, but resuming isn't complete until ipa_modem_resume() returns. This way we're sure to be resumed before transmits are allowed again. Cancel it before calling ipa_stop() in ipa_modem_stop() to ensure the transmit queue restart completes before it gets stopped there. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_modem.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/ipa_modem.c b/drivers/net/ipa/ipa_modem.c index 06e44afd2cf66a..0a3b034614b613 100644 --- a/drivers/net/ipa/ipa_modem.c +++ b/drivers/net/ipa/ipa_modem.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include "ipa.h" @@ -33,9 +34,14 @@ enum ipa_modem_state { IPA_MODEM_STATE_STOPPING, }; -/** struct ipa_priv - IPA network device private data */ +/** + * struct ipa_priv - IPA network device private data + * @ipa: IPA pointer + * @work: Work structure used to wake the modem netdev TX queue + */ struct ipa_priv { struct ipa *ipa; + struct work_struct work; }; /** ipa_open() - Opens the modem network interface */ @@ -189,6 +195,21 @@ void ipa_modem_suspend(struct net_device *netdev) ipa_endpoint_suspend_one(ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]); } +/** + * ipa_modem_wake_queue_work() - enable modem netdev queue + * @work: Work structure + * + * Re-enable transmit on the modem network device. This is called + * in (power management) work queue context, scheduled when resuming + * the modem. + */ +static void ipa_modem_wake_queue_work(struct work_struct *work) +{ + struct ipa_priv *priv = container_of(work, struct ipa_priv, work); + + netif_wake_queue(priv->ipa->modem_netdev); +} + /** ipa_modem_resume() - resume callback for runtime_pm * @dev: pointer to device * @@ -205,7 +226,8 @@ void ipa_modem_resume(struct net_device *netdev) ipa_endpoint_resume_one(ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]); ipa_endpoint_resume_one(ipa->name_map[IPA_ENDPOINT_AP_MODEM_RX]); - netif_wake_queue(netdev); + /* Arrange for the TX queue to be restarted */ + (void)queue_pm_work(&priv->work); } int ipa_modem_start(struct ipa *ipa) @@ -233,6 +255,7 @@ int ipa_modem_start(struct ipa *ipa) SET_NETDEV_DEV(netdev, &ipa->pdev->dev); priv = netdev_priv(netdev); priv->ipa = ipa; + INIT_WORK(&priv->work, ipa_modem_wake_queue_work); ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]->netdev = netdev; ipa->name_map[IPA_ENDPOINT_AP_MODEM_RX]->netdev = netdev; ipa->modem_netdev = netdev; @@ -277,6 +300,9 @@ int ipa_modem_stop(struct ipa *ipa) /* Clean up the netdev and endpoints if it was started */ if (netdev) { + struct ipa_priv *priv = netdev_priv(netdev); + + cancel_work_sync(&priv->work); /* If it was opened, stop it first */ if (netdev->flags & IFF_UP) (void)ipa_stop(netdev); From 6b51f802d652b9f053ef5103dc33b7a55c67860c Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 12 Aug 2021 14:50:33 -0500 Subject: [PATCH 4/6] net: ipa: ensure hardware has power in ipa_start_xmit() We need to ensure the hardware is powered when we transmit a packet. But if it's not, we can't block to wait for it. So asynchronously request power in ipa_start_xmit(), and only proceed if the return value indicates the power state is active. If the hardware is not active, a runtime resume request will have been initiated. In that case, stop the network stack from further transmit attempts until the resume completes. Return NETDEV_TX_BUSY, to retry sending the packet once the queue is restarted. If the power request returns an error (other than -EINPROGRESS, which just means a resume requested elsewhere isn't complete), just drop the packet. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_modem.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/drivers/net/ipa/ipa_modem.c b/drivers/net/ipa/ipa_modem.c index 0a3b034614b613..aa1b483d9f7dbf 100644 --- a/drivers/net/ipa/ipa_modem.c +++ b/drivers/net/ipa/ipa_modem.c @@ -106,6 +106,7 @@ static int ipa_start_xmit(struct sk_buff *skb, struct net_device *netdev) struct ipa_endpoint *endpoint; struct ipa *ipa = priv->ipa; u32 skb_len = skb->len; + struct device *dev; int ret; if (!skb_len) @@ -115,7 +116,31 @@ static int ipa_start_xmit(struct sk_buff *skb, struct net_device *netdev) if (endpoint->data->qmap && skb->protocol != htons(ETH_P_MAP)) goto err_drop_skb; + /* The hardware must be powered for us to transmit */ + dev = &ipa->pdev->dev; + ret = pm_runtime_get(dev); + if (ret < 1) { + /* If a resume won't happen, just drop the packet */ + if (ret < 0 && ret != -EINPROGRESS) { + pm_runtime_put_noidle(dev); + goto err_drop_skb; + } + + /* No power (yet). Stop the network stack from transmitting + * until we're resumed; ipa_modem_resume() arranges for the + * TX queue to be started again. + */ + netif_stop_queue(netdev); + + (void)pm_runtime_put(dev); + + return NETDEV_TX_BUSY; + } + ret = ipa_endpoint_skb_tx(endpoint, skb); + + (void)pm_runtime_put(dev); + if (ret) { if (ret != -E2BIG) return NETDEV_TX_BUSY; @@ -201,7 +226,10 @@ void ipa_modem_suspend(struct net_device *netdev) * * Re-enable transmit on the modem network device. This is called * in (power management) work queue context, scheduled when resuming - * the modem. + * the modem. We can't enable the queue directly in ipa_modem_resume() + * because transmits restart the instant the queue is awakened; but the + * device power state won't be ACTIVE until *after* ipa_modem_resume() + * returns. */ static void ipa_modem_wake_queue_work(struct work_struct *work) { From 8dcf8bb30f17d4ac1233be877c1650c5a1b34ca8 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 12 Aug 2021 14:50:34 -0500 Subject: [PATCH 5/6] net: ipa: don't stop TX on suspend Currently we stop the modem netdev transmit queue when suspending the hardware. For system suspend this ensured we'd never attempt to transmit while attempting to suspend the modem endpoints. For runtime suspend, the IPA hardware might get suspended while the system is operating. In that case we want an attempt to transmit a packet to cause the hardware to resume if necessary. But if we disable the queue this cannot happen. So stop disabling the queue on suspend. In case we end up disabling it in ipa_start_xmit() (see the previous commit), we still arrange to start the TX queue on resume. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_modem.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ipa/ipa_modem.c b/drivers/net/ipa/ipa_modem.c index aa1b483d9f7dbf..b176910d728689 100644 --- a/drivers/net/ipa/ipa_modem.c +++ b/drivers/net/ipa/ipa_modem.c @@ -214,8 +214,6 @@ void ipa_modem_suspend(struct net_device *netdev) if (!(netdev->flags & IFF_UP)) return; - netif_stop_queue(netdev); - ipa_endpoint_suspend_one(ipa->name_map[IPA_ENDPOINT_AP_MODEM_RX]); ipa_endpoint_suspend_one(ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]); } From 8dc181f2cd621e8eaa3d9d432e5ebf0175244c4a Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 12 Aug 2021 14:50:35 -0500 Subject: [PATCH 6/6] net: ipa: don't hold clock reference while netdev open Currently a clock reference is taken whenever the ->ndo_open callback for the modem netdev is called. That reference is dropped when the device is closed, in ipa_stop(). We no longer need this, because ipa_start_xmit() now handles the situation where the hardware power state is not active. Drop the clock reference in ipa_open() when we're done, and take a new reference in ipa_stop() before we begin closing the interface. Finally (and unrelated, but trivial), change the return type of ipa_start_xmit() to be netdev_tx_t instead of int. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_modem.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/net/ipa/ipa_modem.c b/drivers/net/ipa/ipa_modem.c index b176910d728689..c8724af935b858 100644 --- a/drivers/net/ipa/ipa_modem.c +++ b/drivers/net/ipa/ipa_modem.c @@ -65,6 +65,8 @@ static int ipa_open(struct net_device *netdev) netif_start_queue(netdev); + (void)ipa_clock_put(ipa); + return 0; err_disable_tx: @@ -80,12 +82,17 @@ static int ipa_stop(struct net_device *netdev) { struct ipa_priv *priv = netdev_priv(netdev); struct ipa *ipa = priv->ipa; + int ret; + + ret = ipa_clock_get(ipa); + if (WARN_ON(ret < 0)) + goto out_clock_put; netif_stop_queue(netdev); ipa_endpoint_disable_one(ipa->name_map[IPA_ENDPOINT_AP_MODEM_RX]); ipa_endpoint_disable_one(ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]); - +out_clock_put: (void)ipa_clock_put(ipa); return 0; @@ -99,7 +106,8 @@ static int ipa_stop(struct net_device *netdev) * NETDEV_TX_OK: Success * NETDEV_TX_BUSY: Error while transmitting the skb. Try again later */ -static int ipa_start_xmit(struct sk_buff *skb, struct net_device *netdev) +static netdev_tx_t +ipa_start_xmit(struct sk_buff *skb, struct net_device *netdev) { struct net_device_stats *stats = &netdev->stats; struct ipa_priv *priv = netdev_priv(netdev);