Messages in this thread |  | | Date | Sun, 1 Jan 2023 12:32:28 -0600 | Subject | Re: [PATCH net-next 3/6] net: ipa: enable IPA interrupt handlers separate from registration | From | Alex Elder <> |
| |
On 12/31/22 11:56 AM, Caleb Connolly wrote: > > > On 30/12/2022 23:22, Alex Elder wrote: >> Expose ipa_interrupt_enable() and have functions that register >> IPA interrupt handlers enable them directly, rather than having the >> registration process do that. Do the same for disabling IPA >> interrupt handlers. > > Hi, >> >> Signed-off-by: Alex Elder <elder@linaro.org> >> --- >> drivers/net/ipa/ipa_interrupt.c | 8 ++------ >> drivers/net/ipa/ipa_interrupt.h | 14 ++++++++++++++ >> drivers/net/ipa/ipa_power.c | 6 +++++- >> drivers/net/ipa/ipa_uc.c | 4 ++++ >> 4 files changed, 25 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/net/ipa/ipa_interrupt.c >> b/drivers/net/ipa/ipa_interrupt.c >> index 7b7388c14806f..87f4b94d02a3f 100644 >> --- a/drivers/net/ipa/ipa_interrupt.c >> +++ b/drivers/net/ipa/ipa_interrupt.c >> @@ -135,7 +135,7 @@ static void ipa_interrupt_enabled_update(struct >> ipa *ipa) >> } >> /* Enable an IPA interrupt type */ >> -static void ipa_interrupt_enable(struct ipa *ipa, enum ipa_irq_id >> ipa_irq) >> +void ipa_interrupt_enable(struct ipa *ipa, enum ipa_irq_id ipa_irq) >> { >> /* Update the IPA interrupt mask to enable it */ >> ipa->interrupt->enabled |= BIT(ipa_irq); >> @@ -143,7 +143,7 @@ static void ipa_interrupt_enable(struct ipa *ipa, >> enum ipa_irq_id ipa_irq) >> } >> /* Disable an IPA interrupt type */ >> -static void ipa_interrupt_disable(struct ipa *ipa, enum ipa_irq_id >> ipa_irq) >> +void ipa_interrupt_disable(struct ipa *ipa, enum ipa_irq_id ipa_irq) >> { >> /* Update the IPA interrupt mask to disable it */ >> ipa->interrupt->enabled &= ~BIT(ipa_irq); >> @@ -232,8 +232,6 @@ void ipa_interrupt_add(struct ipa_interrupt >> *interrupt, >> return; >> interrupt->handler[ipa_irq] = handler; >> - >> - ipa_interrupt_enable(interrupt->ipa, ipa_irq); >> } >> /* Remove the handler for an IPA interrupt type */ >> @@ -243,8 +241,6 @@ ipa_interrupt_remove(struct ipa_interrupt >> *interrupt, enum ipa_irq_id ipa_irq) >> if (WARN_ON(ipa_irq >= IPA_IRQ_COUNT)) >> return; >> - ipa_interrupt_disable(interrupt->ipa, ipa_irq); >> - >> interrupt->handler[ipa_irq] = NULL; >> } >> diff --git a/drivers/net/ipa/ipa_interrupt.h >> b/drivers/net/ipa/ipa_interrupt.h >> index f31fd9965fdc6..5f7d2e90ea337 100644 >> --- a/drivers/net/ipa/ipa_interrupt.h >> +++ b/drivers/net/ipa/ipa_interrupt.h >> @@ -85,6 +85,20 @@ void ipa_interrupt_suspend_clear_all(struct >> ipa_interrupt *interrupt); >> */ >> void ipa_interrupt_simulate_suspend(struct ipa_interrupt *interrupt); >> +/** >> + * ipa_interrupt_enable() - Enable an IPA interrupt type >> + * @ipa: IPA pointer >> + * @ipa_irq: IPA interrupt ID >> + */ >> +void ipa_interrupt_enable(struct ipa *ipa, enum ipa_irq_id ipa_irq); > > I think you forgot a forward declaration for enum ipa_irq_id
Thanks, I'll verify this and will send v2 with a fix once net-next is open for business again.
-Alex
> > Kind Regards, > Caleb >> + >> +/** >> + * ipa_interrupt_disable() - Disable an IPA interrupt type >> + * @ipa: IPA pointer >> + * @ipa_irq: IPA interrupt ID >> + */ >> +void ipa_interrupt_disable(struct ipa *ipa, enum ipa_irq_id ipa_irq); >> + >> /** >> * ipa_interrupt_config() - Configure the IPA interrupt framework >> * @ipa: IPA pointer >> diff --git a/drivers/net/ipa/ipa_power.c b/drivers/net/ipa/ipa_power.c >> index 8420f93128a26..9148d606d5fc2 100644 >> --- a/drivers/net/ipa/ipa_power.c >> +++ b/drivers/net/ipa/ipa_power.c >> @@ -337,10 +337,13 @@ int ipa_power_setup(struct ipa *ipa) >> ipa_interrupt_add(ipa->interrupt, IPA_IRQ_TX_SUSPEND, >> ipa_suspend_handler); >> + ipa_interrupt_enable(ipa, IPA_IRQ_TX_SUSPEND); >> ret = device_init_wakeup(&ipa->pdev->dev, true); >> - if (ret) >> + if (ret) { >> + ipa_interrupt_disable(ipa, IPA_IRQ_TX_SUSPEND); >> ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_TX_SUSPEND); >> + } >> return ret; >> } >> @@ -348,6 +351,7 @@ int ipa_power_setup(struct ipa *ipa) >> void ipa_power_teardown(struct ipa *ipa) >> { >> (void)device_init_wakeup(&ipa->pdev->dev, false); >> + ipa_interrupt_disable(ipa, IPA_IRQ_TX_SUSPEND); >> ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_TX_SUSPEND); >> } >> diff --git a/drivers/net/ipa/ipa_uc.c b/drivers/net/ipa/ipa_uc.c >> index 0a890b44c09e1..af541758d047f 100644 >> --- a/drivers/net/ipa/ipa_uc.c >> +++ b/drivers/net/ipa/ipa_uc.c >> @@ -187,7 +187,9 @@ void ipa_uc_config(struct ipa *ipa) >> ipa->uc_powered = false; >> ipa->uc_loaded = false; >> ipa_interrupt_add(interrupt, IPA_IRQ_UC_0, >> ipa_uc_interrupt_handler); >> + ipa_interrupt_enable(ipa, IPA_IRQ_UC_0); >> ipa_interrupt_add(interrupt, IPA_IRQ_UC_1, >> ipa_uc_interrupt_handler); >> + ipa_interrupt_enable(ipa, IPA_IRQ_UC_1); >> } >> /* Inverse of ipa_uc_config() */ >> @@ -195,7 +197,9 @@ void ipa_uc_deconfig(struct ipa *ipa) >> { >> struct device *dev = &ipa->pdev->dev; >> + ipa_interrupt_disable(ipa, IPA_IRQ_UC_1); >> ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_UC_1); >> + ipa_interrupt_disable(ipa, IPA_IRQ_UC_0); >> ipa_interrupt_remove(ipa->interrupt, IPA_IRQ_UC_0); >> if (ipa->uc_loaded) >> ipa_power_retention(ipa, false);
|  |