| Message ID | 20240229205554.86762-2-elder@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: ipa: simplify device pointer access | expand |
On Thu, Feb 29, 2024 at 02:55:48PM -0600, Alex Elder wrote: > Change the return type of ipa_interrupt_config() to be an error > code rather than an IPA interrupt structure pointer, and assign the > the pointer within that function. > > Change ipa_interrupt_deconfig() to take the IPA pointer as argument > and have it invalidate the ipa->interrupt pointer. > > Signed-off-by: Alex Elder <elder@linaro.org> > --- > drivers/net/ipa/ipa_interrupt.c | 15 ++++++++++----- > drivers/net/ipa/ipa_interrupt.h | 10 +++++----- > drivers/net/ipa/ipa_main.c | 13 ++++--------- > 3 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/drivers/net/ipa/ipa_interrupt.c b/drivers/net/ipa/ipa_interrupt.c > index 4d80bf77a5323..a298d922dd871 100644 > --- a/drivers/net/ipa/ipa_interrupt.c > +++ b/drivers/net/ipa/ipa_interrupt.c > @@ -236,7 +236,7 @@ void ipa_interrupt_simulate_suspend(struct ipa_interrupt *interrupt) > } > > /* Configure the IPA interrupt framework */ > -struct ipa_interrupt *ipa_interrupt_config(struct ipa *ipa) > +int ipa_interrupt_config(struct ipa *ipa) Hi Alex, There are two cases where this function still returns a pointer. Around line 250: ret = platform_get_irq_byname(ipa->pdev, "ipa"); if (ret <= 0) { dev_err(dev, "DT error %d getting \"ipa\" IRQ property\n", ret); return ERR_PTR(ret ? : -EINVAL); } And around line 280: return interrupt; This does seem to be resolved in patch 2/7. But as it is, this patch breaks bisection. ...
On 3/1/24 10:26 AM, Simon Horman wrote:
> There are two cases where this function still returns a pointer.
Yeah I saw that. I'm in the middle of testing v2 right now.
I'm very sorry about that. My process includes testing
every patch but somehow I missed these errors on patches 1
and 2 this time. (The netdev CI doesn't report all the
problems with patch 2; I'm not sure why.)
Anyway, a new version is coming. Thanks for the note.
-Alex
On Fri, Mar 01, 2024 at 10:37:13AM -0600, Alex Elder wrote: > On 3/1/24 10:26 AM, Simon Horman wrote: > > There are two cases where this function still returns a pointer. > > Yeah I saw that. I'm in the middle of testing v2 right now. > > I'm very sorry about that. My process includes testing > every patch but somehow I missed these errors on patches 1 > and 2 this time. (The netdev CI doesn't report all the > problems with patch 2; I'm not sure why.) > > Anyway, a new version is coming. Thanks for the note. No problem, stuff happens.
diff --git a/drivers/net/ipa/ipa_interrupt.c b/drivers/net/ipa/ipa_interrupt.c index 4d80bf77a5323..a298d922dd871 100644 --- a/drivers/net/ipa/ipa_interrupt.c +++ b/drivers/net/ipa/ipa_interrupt.c @@ -236,7 +236,7 @@ void ipa_interrupt_simulate_suspend(struct ipa_interrupt *interrupt) } /* Configure the IPA interrupt framework */ -struct ipa_interrupt *ipa_interrupt_config(struct ipa *ipa) +int ipa_interrupt_config(struct ipa *ipa) { struct device *dev = &ipa->pdev->dev; struct ipa_interrupt *interrupt; @@ -254,10 +254,12 @@ struct ipa_interrupt *ipa_interrupt_config(struct ipa *ipa) interrupt = kzalloc(sizeof(*interrupt), GFP_KERNEL); if (!interrupt) - return ERR_PTR(-ENOMEM); + return -ENOMEM; interrupt->ipa = ipa; interrupt->irq = irq; + ipa->interrupt = interrupt; + /* Start with all IPA interrupts disabled */ reg = ipa_reg(ipa, IPA_IRQ_EN); iowrite32(0, ipa->reg_virt + reg_offset(reg)); @@ -282,13 +284,16 @@ struct ipa_interrupt *ipa_interrupt_config(struct ipa *ipa) err_kfree: kfree(interrupt); - return ERR_PTR(ret); + return ret; } /* Inverse of ipa_interrupt_config() */ -void ipa_interrupt_deconfig(struct ipa_interrupt *interrupt) +void ipa_interrupt_deconfig(struct ipa *ipa) { - struct device *dev = &interrupt->ipa->pdev->dev; + struct ipa_interrupt *interrupt = ipa->interrupt; + struct device *dev = &ipa->pdev->dev; + + ipa->interrupt = NULL; dev_pm_clear_wake_irq(dev); free_irq(interrupt->irq, interrupt); diff --git a/drivers/net/ipa/ipa_interrupt.h b/drivers/net/ipa/ipa_interrupt.h index 53e1b71685c75..1947654e3e360 100644 --- a/drivers/net/ipa/ipa_interrupt.h +++ b/drivers/net/ipa/ipa_interrupt.h @@ -76,17 +76,17 @@ void ipa_interrupt_irq_enable(struct ipa *ipa); void ipa_interrupt_irq_disable(struct ipa *ipa); /** - * ipa_interrupt_config() - Configure the IPA interrupt framework + * ipa_interrupt_config() - Configure IPA interrupts * @ipa: IPA pointer * - * Return: Pointer to IPA SMP2P info, or a pointer-coded error + * Return: 0 if successful, or a negative error code */ -struct ipa_interrupt *ipa_interrupt_config(struct ipa *ipa); +int ipa_interrupt_config(struct ipa *ipa); /** * ipa_interrupt_deconfig() - Inverse of ipa_interrupt_config() - * @interrupt: IPA interrupt structure + * @ipa: IPA pointer */ -void ipa_interrupt_deconfig(struct ipa_interrupt *interrupt); +void ipa_interrupt_deconfig(struct ipa *ipa); #endif /* _IPA_INTERRUPT_H_ */ diff --git a/drivers/net/ipa/ipa_main.c b/drivers/net/ipa/ipa_main.c index 00475fd7a2054..0c6e6719d99e3 100644 --- a/drivers/net/ipa/ipa_main.c +++ b/drivers/net/ipa/ipa_main.c @@ -542,12 +542,9 @@ static int ipa_config(struct ipa *ipa, const struct ipa_data *data) if (ret) goto err_hardware_deconfig; - ipa->interrupt = ipa_interrupt_config(ipa); - if (IS_ERR(ipa->interrupt)) { - ret = PTR_ERR(ipa->interrupt); - ipa->interrupt = NULL; + ret = ipa_interrupt_config(ipa); + if (ret) goto err_mem_deconfig; - } ipa_uc_config(ipa); @@ -572,8 +569,7 @@ static int ipa_config(struct ipa *ipa, const struct ipa_data *data) ipa_endpoint_deconfig(ipa); err_uc_deconfig: ipa_uc_deconfig(ipa); - ipa_interrupt_deconfig(ipa->interrupt); - ipa->interrupt = NULL; + ipa_interrupt_deconfig(ipa); err_mem_deconfig: ipa_mem_deconfig(ipa); err_hardware_deconfig: @@ -591,8 +587,7 @@ static void ipa_deconfig(struct ipa *ipa) ipa_modem_deconfig(ipa); ipa_endpoint_deconfig(ipa); ipa_uc_deconfig(ipa); - ipa_interrupt_deconfig(ipa->interrupt); - ipa->interrupt = NULL; + ipa_interrupt_deconfig(ipa); ipa_mem_deconfig(ipa); ipa_hardware_deconfig(ipa); }
Change the return type of ipa_interrupt_config() to be an error code rather than an IPA interrupt structure pointer, and assign the the pointer within that function. Change ipa_interrupt_deconfig() to take the IPA pointer as argument and have it invalidate the ipa->interrupt pointer. Signed-off-by: Alex Elder <elder@linaro.org> --- drivers/net/ipa/ipa_interrupt.c | 15 ++++++++++----- drivers/net/ipa/ipa_interrupt.h | 10 +++++----- drivers/net/ipa/ipa_main.c | 13 ++++--------- 3 files changed, 19 insertions(+), 19 deletions(-)