Messages in this thread | | | Date | Tue, 17 May 2022 11:34:05 +0200 | Subject | Re: [PATCH v3 3/5] soc: mediatek: pwrap: Move and check return value of platform_get_irq() | From | AngeloGioacchino Del Regno <> |
| |
Il 17/05/22 11:18, Matthias Brugger ha scritto: > > > On 16/05/2022 14:46, AngeloGioacchino Del Regno wrote: >> Move the call to platform_get_irq() earlier in the probe function >> and check for its return value: if no interrupt is specified, it >> wouldn't make sense to try to call devm_request_irq() so, in that >> case, we can simply return early. >> >> Moving the platform_get_irq() call also makes it possible to use >> one less goto, as clocks aren't required at that stage. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> >> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com> >> --- >> drivers/soc/mediatek/mtk-pmic-wrap.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c >> b/drivers/soc/mediatek/mtk-pmic-wrap.c >> index 852514366f1f..332cbcabc299 100644 >> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c >> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c >> @@ -2204,6 +2204,10 @@ static int pwrap_probe(struct platform_device *pdev) >> if (!wrp) >> return -ENOMEM; >> + irq = platform_get_irq(pdev, 0); >> + if (irq < 0) >> + return irq; >> + >> platform_set_drvdata(pdev, wrp); >> wrp->master = of_device_get_match_data(&pdev->dev); >> @@ -2316,7 +2320,6 @@ static int pwrap_probe(struct platform_device *pdev) >> if (HAS_CAP(wrp->master->caps, PWRAP_CAP_INT1_EN)) >> pwrap_writel(wrp, wrp->master->int1_en_all, PWRAP_INT1_EN); >> - irq = platform_get_irq(pdev, 0); > > For better readability of the code I'd prefer to keep platform_get_irq next to > devm_request_irq. I understand that you did this change so that you don't have to code > if (irq < 0) { > ret = irq; > goto err_out2; > } > > Or do I miss something? >
That's for the sake of reducing gotos in the code... but there's a bigger picture that I haven't explained in this commit and that will come later because I currently don't have the necessary time to perform a "decent" testing.
As I was explaining - the bigger pictures implies adding a new function for clock teardown, that we will add as a devm action:
devm_add_action_or_reset(&pdev->dev, pwrap_clk_disable_unprepare, wrp)
...so that we will be able to remove *all* gotos from the probe function.
Sounds good?
Cheers, Angelo
| |