From 4c6a4da84431415b1f451e2715a17487f9b3474e Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 19 Aug 2021 17:19:23 -0500 Subject: [PATCH 1/5] net: ipa: don't use ipa_clock_get() in "ipa_main.c" We need the hardware to be powered starting at the config stage of initialization when the IPA driver probes. And we need it powered when the driver is removed, at least until the deconfig stage has completed. Replace callers of ipa_clock_get() in ipa_probe() and ipa_exit(), calling pm_runtime_get_sync() instead. Replace the corresponding callers of ipa_clock_put(), calling pm_runtime_put() instead. The only error we expect when getting power would occur when the system is suspended. The ->probe and ->remove driver callbacks won't be called when suspended, so issue a WARN() call if an error is seen getting power. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_main.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index 69fa4b3120fd..3969aef6c437 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -737,13 +738,13 @@ static int ipa_probe(struct platform_device *pdev) goto err_table_exit; /* The clock needs to be active for config and setup */ - ret = ipa_clock_get(ipa); + ret = pm_runtime_get_sync(dev); if (WARN_ON(ret < 0)) - goto err_clock_put; + goto err_power_put; ret = ipa_config(ipa, data); if (ret) - goto err_clock_put; + goto err_power_put; dev_info(dev, "IPA driver initialized"); @@ -765,14 +766,14 @@ static int ipa_probe(struct platform_device *pdev) if (ret) goto err_deconfig; done: - (void)ipa_clock_put(ipa); + (void)pm_runtime_put(dev); return 0; err_deconfig: ipa_deconfig(ipa); -err_clock_put: - (void)ipa_clock_put(ipa); +err_power_put: + (void)pm_runtime_put(dev); ipa_modem_exit(ipa); err_table_exit: ipa_table_exit(ipa); @@ -798,9 +799,9 @@ static int ipa_remove(struct platform_device *pdev) struct ipa_clock *clock = ipa->clock; int ret; - ret = ipa_clock_get(ipa); + ret = pm_runtime_get_sync(&pdev->dev); if (WARN_ON(ret < 0)) - goto out_clock_put; + goto out_power_put; if (ipa->setup_complete) { ret = ipa_modem_stop(ipa); @@ -816,8 +817,8 @@ static int ipa_remove(struct platform_device *pdev) } ipa_deconfig(ipa); -out_clock_put: - (void)ipa_clock_put(ipa); +out_power_put: + (void)pm_runtime_put(&pdev->dev); ipa_modem_exit(ipa); ipa_table_exit(ipa); From c43adc75dc2dee8cc5a29a722c1c1d5a00b434c3 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 19 Aug 2021 17:19:24 -0500 Subject: [PATCH 2/5] net: ipa: don't use ipa_clock_get() in "ipa_smp2p.c" If the "modem-init" Device Tree property is present for a platform, the modem performs early IPA hardware initialization, and signals this is complete with an "ipa-setup-ready" SMP2P interrupt. This triggers a call to ipa_setup(), which requires the hardware to be powered. Replace the call to ipa_clock_get() in this case with a call to pm_runtime_get_sync(). And replace the corresponding calls to ipa_clock_put() with calls to pm_runtime_put() instead. There is a chance we get an error when taking this power reference. This is an unlikely scenario, where system suspend is initiated just before the modem signals it has finished initializing the IPA hardware. For now we'll just accept that this could occur, and report it if it does. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_smp2p.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/net/ipa/ipa_smp2p.c b/drivers/net/ipa/ipa_smp2p.c index 04b977cf9159..f6e2061cd391 100644 --- a/drivers/net/ipa/ipa_smp2p.c +++ b/drivers/net/ipa/ipa_smp2p.c @@ -16,7 +16,6 @@ #include "ipa_smp2p.h" #include "ipa.h" #include "ipa_uc.h" -#include "ipa_clock.h" /** * DOC: IPA SMP2P communication with the modem @@ -153,6 +152,7 @@ static void ipa_smp2p_panic_notifier_unregister(struct ipa_smp2p *smp2p) static irqreturn_t ipa_smp2p_modem_setup_ready_isr(int irq, void *dev_id) { struct ipa_smp2p *smp2p = dev_id; + struct device *dev; int ret; mutex_lock(&smp2p->mutex); @@ -161,17 +161,20 @@ static irqreturn_t ipa_smp2p_modem_setup_ready_isr(int irq, void *dev_id) goto out_mutex_unlock; smp2p->disabled = true; /* If any others arrive, ignore them */ - /* The clock needs to be active for setup */ - ret = ipa_clock_get(smp2p->ipa); - if (WARN_ON(ret < 0)) - goto out_clock_put; + /* Power needs to be active for setup */ + dev = &smp2p->ipa->pdev->dev; + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + dev_err(dev, "error %d getting power for setup\n", ret); + goto out_power_put; + } /* An error here won't cause driver shutdown, so warn if one occurs */ ret = ipa_setup(smp2p->ipa); WARN(ret != 0, "error %d from ipa_setup()\n", ret); -out_clock_put: - (void)ipa_clock_put(smp2p->ipa); +out_power_put: + (void)pm_runtime_put(dev); out_mutex_unlock: mutex_unlock(&smp2p->mutex); @@ -211,7 +214,7 @@ static void ipa_smp2p_clock_release(struct ipa *ipa) if (!ipa->smp2p->clock_on) return; - (void)ipa_clock_put(ipa); + (void)pm_runtime_put(&ipa->pdev->dev); ipa->smp2p->clock_on = false; } From 799c5c24b7acc8af0086f1cbff5be3af7f63f6f1 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 19 Aug 2021 17:19:25 -0500 Subject: [PATCH 3/5] net: ipa: don't use ipa_clock_get() in "ipa_uc.c" Replace the ipa_clock_get() call in ipa_uc_clock() when taking the "proxy" clock reference for the microcontroller with a call to pm_runtime_get_sync(). Replace calls of ipa_clock_put() for the microcontroller with pm_runtime_put() calls instead. There is a chance we get an error when taking the microcontroller power reference. This is an unlikely scenario, where system suspend is initiated just before we learn the modem is booting. For now we'll just accept that this could occur, and report it if it does. Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_uc.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/net/ipa/ipa_uc.c b/drivers/net/ipa/ipa_uc.c index 9c8818c39073..a0bdd25b65b4 100644 --- a/drivers/net/ipa/ipa_uc.c +++ b/drivers/net/ipa/ipa_uc.c @@ -7,9 +7,9 @@ #include #include #include +#include #include "ipa.h" -#include "ipa_clock.h" #include "ipa_uc.h" /** @@ -154,7 +154,7 @@ static void ipa_uc_response_hdlr(struct ipa *ipa, enum ipa_irq_id irq_id) case IPA_UC_RESPONSE_INIT_COMPLETED: if (ipa->uc_clocked) { ipa->uc_loaded = true; - (void)ipa_clock_put(ipa); + (void)pm_runtime_put(dev); ipa->uc_clocked = false; } else { dev_warn(dev, "unexpected init_completed response\n"); @@ -182,25 +182,29 @@ void ipa_uc_deconfig(struct ipa *ipa) ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_UC_1); ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_UC_0); if (ipa->uc_clocked) - (void)ipa_clock_put(ipa); + (void)pm_runtime_put(&ipa->pdev->dev); } /* Take a proxy clock reference for the microcontroller */ void ipa_uc_clock(struct ipa *ipa) { static bool already; + struct device *dev; int ret; if (already) return; already = true; /* Only do this on first boot */ - /* This clock reference dropped in ipa_uc_response_hdlr() above */ - ret = ipa_clock_get(ipa); - if (WARN(ret < 0, "error %d getting proxy clock\n", ret)) - (void)ipa_clock_put(ipa); - - ipa->uc_clocked = ret >= 0; + /* This power reference dropped in ipa_uc_response_hdlr() above */ + dev = &ipa->pdev->dev; + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + pm_runtime_put_noidle(dev); + dev_err(dev, "error %d getting proxy power\n", ret); + } else { + ipa->uc_clocked = true; + } } /* Send a command to the microcontroller */ From 724c2d743688f296263df0223c3d95987dfc427b Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 19 Aug 2021 17:19:26 -0500 Subject: [PATCH 4/5] net: ipa: don't use ipa_clock_get() in "ipa_modem.c" When we open or close the modem network device we need to ensure the hardware is powered. Replace the callers of ipa_clock_get() found in ipa_open() and ipa_stop() with calls to pm_runtime_get_sync(). If an error is returned, simply return that error to the caller (without any error or warning message). This could conceivably occur if the function was called while the system was suspended, but that really shouldn't happen. Replace corresponding calls to ipa_clock_put() with pm_runtime_put() also. If the modem crashes we also need to ensure the hardware is powered to recover. If getting power returns an error there's not much we can do, but at least report the error. (Ideally the remoteproc SSR code would ensure the AP was not suspended when it sends the notification, but that is not (yet) the case.) Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_modem.c | 40 +++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/drivers/net/ipa/ipa_modem.c b/drivers/net/ipa/ipa_modem.c index 16d87910305e..11f0204a9695 100644 --- a/drivers/net/ipa/ipa_modem.c +++ b/drivers/net/ipa/ipa_modem.c @@ -49,15 +49,17 @@ static int ipa_open(struct net_device *netdev) { struct ipa_priv *priv = netdev_priv(netdev); struct ipa *ipa = priv->ipa; + struct device *dev; int ret; - ret = ipa_clock_get(ipa); - if (WARN_ON(ret < 0)) - goto err_clock_put; + dev = &ipa->pdev->dev; + ret = pm_runtime_get_sync(dev); + if (ret < 0) + goto err_power_put; ret = ipa_endpoint_enable_one(ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]); if (ret) - goto err_clock_put; + goto err_power_put; ret = ipa_endpoint_enable_one(ipa->name_map[IPA_ENDPOINT_AP_MODEM_RX]); if (ret) @@ -65,14 +67,14 @@ static int ipa_open(struct net_device *netdev) netif_start_queue(netdev); - (void)ipa_clock_put(ipa); + (void)pm_runtime_put(dev); return 0; err_disable_tx: ipa_endpoint_disable_one(ipa->name_map[IPA_ENDPOINT_AP_MODEM_TX]); -err_clock_put: - (void)ipa_clock_put(ipa); +err_power_put: + (void)pm_runtime_put(dev); return ret; } @@ -82,18 +84,20 @@ static int ipa_stop(struct net_device *netdev) { struct ipa_priv *priv = netdev_priv(netdev); struct ipa *ipa = priv->ipa; + struct device *dev; int ret; - ret = ipa_clock_get(ipa); - if (WARN_ON(ret < 0)) - goto out_clock_put; + dev = &ipa->pdev->dev; + ret = pm_runtime_get_sync(dev); + if (ret < 0) + goto out_power_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); +out_power_put: + (void)pm_runtime_put(dev); return 0; } @@ -362,9 +366,11 @@ static void ipa_modem_crashed(struct ipa *ipa) struct device *dev = &ipa->pdev->dev; int ret; - ret = ipa_clock_get(ipa); - if (WARN_ON(ret < 0)) - goto out_clock_put; + ret = pm_runtime_get_sync(dev); + if (ret < 0) { + dev_err(dev, "error %d getting power to handle crash\n", ret); + goto out_power_put; + } ipa_endpoint_modem_pause_all(ipa, true); @@ -391,8 +397,8 @@ static void ipa_modem_crashed(struct ipa *ipa) if (ret) dev_err(dev, "error %d zeroing modem memory regions\n", ret); -out_clock_put: - (void)ipa_clock_put(ipa); +out_power_put: + (void)pm_runtime_put(dev); } static int ipa_modem_notify(struct notifier_block *nb, unsigned long action, From c3f115aa5e1b6459e2ccd711277435397dd7c6e9 Mon Sep 17 00:00:00 2001 From: Alex Elder Date: Thu, 19 Aug 2021 17:19:27 -0500 Subject: [PATCH 5/5] net: ipa: kill ipa_clock_get() The only remaining user of the ipa_clock_{get,put}() interface is ipa_isr_thread(). Replace calls to ipa_clock_get() there calling pm_runtime_get_sync() instead. And call pm_runtime_put() there rather than ipa_clock_put(). Warn if we ever get an error. With that, we can get rid of ipa_clock_get() and ipa_clock_put(). Signed-off-by: Alex Elder Signed-off-by: David S. Miller --- drivers/net/ipa/ipa_clock.c | 17 ----------------- drivers/net/ipa/ipa_clock.h | 24 ------------------------ drivers/net/ipa/ipa_interrupt.c | 14 +++++++------- 3 files changed, 7 insertions(+), 48 deletions(-) diff --git a/drivers/net/ipa/ipa_clock.c b/drivers/net/ipa/ipa_clock.c index 74eb9ecdd19b..149b24da0bcc 100644 --- a/drivers/net/ipa/ipa_clock.c +++ b/drivers/net/ipa/ipa_clock.c @@ -272,23 +272,6 @@ static int ipa_runtime_idle(struct device *dev) return -EAGAIN; } -/* Get an IPA clock reference. If the reference count is non-zero, it is - * incremented and return is immediate. Otherwise the IPA clock is - * enabled. - */ -int ipa_clock_get(struct ipa *ipa) -{ - return pm_runtime_get_sync(&ipa->pdev->dev); -} - -/* Attempt to remove an IPA clock reference. If this represents the - * last reference, disable the IPA clock. - */ -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); diff --git a/drivers/net/ipa/ipa_clock.h b/drivers/net/ipa/ipa_clock.h index 64cd15981b1d..7b7864f3029b 100644 --- a/drivers/net/ipa/ipa_clock.h +++ b/drivers/net/ipa/ipa_clock.h @@ -70,28 +70,4 @@ struct ipa_clock *ipa_clock_init(struct device *dev, */ void ipa_clock_exit(struct ipa_clock *clock); -/** - * ipa_clock_get() - Get an IPA clock reference - * @ipa: IPA pointer - * - * Return: 0 if clock started, 1 if clock already running, or a negative - * error code - * - * This call blocks if this is the first reference. A reference is - * taken even if an error occurs starting the IPA clock. - */ -int ipa_clock_get(struct ipa *ipa); - -/** - * ipa_clock_put() - Drop an IPA clock reference - * @ipa: IPA pointer - * - * Return: 0 if successful, or a negative error code - * - * This drops a clock reference. If the last reference is being dropped, - * the clock is stopped and RX endpoints are suspended. This call will - * not block unless the last reference is dropped. - */ -int ipa_clock_put(struct ipa *ipa); - #endif /* _IPA_CLOCK_H_ */ diff --git a/drivers/net/ipa/ipa_interrupt.c b/drivers/net/ipa/ipa_interrupt.c index 934c14e066a0..3fecaadb4a37 100644 --- a/drivers/net/ipa/ipa_interrupt.c +++ b/drivers/net/ipa/ipa_interrupt.c @@ -21,9 +21,9 @@ #include #include +#include #include "ipa.h" -#include "ipa_clock.h" #include "ipa_reg.h" #include "ipa_endpoint.h" #include "ipa_interrupt.h" @@ -80,14 +80,16 @@ static irqreturn_t ipa_isr_thread(int irq, void *dev_id) struct ipa_interrupt *interrupt = dev_id; struct ipa *ipa = interrupt->ipa; u32 enabled = interrupt->enabled; + struct device *dev; u32 pending; u32 offset; u32 mask; int ret; - ret = ipa_clock_get(ipa); + dev = &ipa->pdev->dev; + ret = pm_runtime_get_sync(dev); if (WARN_ON(ret < 0)) - goto out_clock_put; + goto out_power_put; /* The status register indicates which conditions are present, * including conditions whose interrupt is not enabled. Handle @@ -108,15 +110,13 @@ static irqreturn_t ipa_isr_thread(int irq, void *dev_id) /* If any disabled interrupts are pending, clear them */ if (pending) { - struct device *dev = &ipa->pdev->dev; - dev_dbg(dev, "clearing disabled IPA interrupts 0x%08x\n", pending); offset = ipa_reg_irq_clr_offset(ipa->version); iowrite32(pending, ipa->reg_virt + offset); } -out_clock_put: - (void)ipa_clock_put(ipa); +out_power_put: + (void)pm_runtime_put(dev); return IRQ_HANDLED; }