Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <4f5a4175371ac7973061cd4f9d19674ac308672c.1582048155.git.amit.kucheria@linaro.org>
References: <cover.1582048155.git.amit.kucheria@linaro.org> <4f5a4175371ac7973061cd4f9d19674ac308672c.1582048155.git.amit.kucheria@linaro.org>
Subject: Re: [PATCH v5 5/8] drivers: thermal: tsens: Add critical interrupt support
From:   Stephen Boyd <swboyd@chromium.org>
Cc:     Amit Kucheria <amit.kucheria@verdurent.com>,
        linux-pm@vger.kernel.org
To:     Amit Kucheria <amit.kucheria@linaro.org>,
        Andy Gross <agross@kernel.org>, bjorn.andersson@linaro.org,
        daniel.lezcano@linaro.org, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org, sivaa@codeaurora.org
Date:   Wed, 19 Feb 2020 16:05:36 -0800
Message-ID: <158215713699.184098.4863049384855658604@swboyd.mtv.corp.google.com>
User-Agent: alot/0.9
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Quoting Amit Kucheria (2020-02-18 10:12:09)
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 0e7cf5236932..5b003d598234 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -125,6 +125,28 @@ static int tsens_register(struct tsens_priv *priv)
>                 goto err_put_device;
>         }
> =20
> +       if (priv->feat->crit_int) {
> +               irq_crit =3D platform_get_irq_byname(pdev, "critical");
> +               if (irq_crit < 0) {
> +                       ret =3D irq_crit;
> +                       /* For old DTs with no IRQ defined */
> +                       if (irq_crit =3D=3D -ENXIO)
> +                               ret =3D 0;
> +                       goto err_crit_int;
> +               }
> +               ret =3D devm_request_threaded_irq(&pdev->dev, irq_crit,
> +                                               NULL, tsens_critical_irq_=
thread,
> +                                               IRQF_ONESHOT,
> +                                               dev_name(&pdev->dev), pri=
v);
> +               if (ret) {
> +                       dev_err(&pdev->dev, "%s: failed to get critical i=
rq\n", __func__);
> +                       goto err_crit_int;
> +               }
> +
> +               enable_irq_wake(irq_crit);
> +       }
> +
> +err_crit_int:

Why use a goto? Can't this be done with if-else statements?

       if (priv->feat->crit_int) {
               irq_crit =3D platform_get_irq_byname(pdev, "critical");
               if (irq_crit < 0) {
                       ...
               } else {
                       ret =3D devm_request_threaded_irq(&pdev->dev, irq_cr=
it,
                                                       NULL, tsens_critical=
_irq_thread,
                                                       IRQF_ONESHOT,
                                                       dev_name(&pdev->dev)=
, priv);
                       if (ret)
                               dev_err(&pdev->dev, "%s: failed to get criti=
cal irq\n", __func__);
                       else
                               enable_irq_wake(irq_crit);
               }
       }

Or if the nesting is so deep that we need goto labels then perhaps it
needs to be another function.

>         enable_irq_wake(irq);
> =20
>  err_put_device: