linux-kernel - Re: [PATCH v5 1/2] drm/msm/dp: no need to disable irq after devm_request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 24 May 2023 00:02:23 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Kuogee Hsieh <quic_khsieh@...cinc.com>
Cc:     dri-devel@...ts.freedesktop.org, robdclark@...il.com,
        sean@...rly.run, swboyd@...omium.org, dianders@...omium.org,
        vkoul@...nel.org, daniel@...ll.ch, airlied@...il.com,
        agross@...nel.org, andersson@...nel.org, quic_abhinavk@...cinc.com,
        quic_jesszhan@...cinc.com, quic_sbillaka@...cinc.com,
        marijn.suijten@...ainline.org, leonard@...sen.nl,
        freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/2] drm/msm/dp: no need to disable irq after devm_request_irq()

On Tue, 23 May 2023 at 22:39, Kuogee Hsieh <quic_khsieh@...cinc.com> wrote:
>
> devm_request_irq() will allocate irq and return with irq enabled.
> At current implementation irq are specified disabled after return from
> devm_request_irq() and re enabled later. It is redundant.

This is not correct. The disable_irq / enable_irq pair is not
redundant. If I understand correctly, it was done so to prevent HPD
IRQs from being delivered before HPD worker is initialized (which was
probably correct at the time of the mentioned commit). So this patch
either can come after the next patch (but then it should not have a
Fixes tag and the commit message should be changed to mention why
disable_irq() call is redundant) or it can be squashed together with
the next patch.

>
> Fixes: 8ede2ecc3e5e ("drm/msm/dp: Add DP compliance tests on Snapdragon Chipsets")
> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 3e13acdf..1af49b6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1094,12 +1094,6 @@ static void dp_display_config_hpd(struct dp_display_private *dp)
>                                 DP_DP_HPD_PLUG_INT_MASK |
>                                 DP_DP_HPD_UNPLUG_INT_MASK,
>                                 true);
> -
> -       /* Enable interrupt first time
> -        * we are leaving dp clocks on during disconnect
> -        * and never disable interrupt
> -        */
> -       enable_irq(dp->irq);
>  }
>
>  void dp_display_set_psr(struct msm_dp *dp_display, bool enter)
> @@ -1282,7 +1276,6 @@ int dp_display_request_irq(struct msm_dp *dp_display)
>                                 dp->irq, rc);
>                 return rc;
>         }
> -       disable_irq(dp->irq);
>
>         return 0;
>  }
> @@ -1570,7 +1563,6 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>                 dp_display_host_init(dp_priv);
>                 dp_catalog_ctrl_hpd_config(dp_priv->catalog);
>                 dp_display_host_phy_init(dp_priv);
> -               enable_irq(dp_priv->irq);
>
>                 /*
>                  * The code below assumes that the panel will finish probing
> @@ -1612,7 +1604,6 @@ static int dp_display_get_next_bridge(struct msm_dp *dp)
>
>  error:
>         if (dp->is_edp) {
> -               disable_irq(dp_priv->irq);
>                 dp_display_host_phy_exit(dp_priv);
>                 dp_display_host_deinit(dp_priv);
>         }
> --
> 2.7.4
>


-- 
With best wishes
Dmitry