linux-kernel - Re: [PATCH] drm/msm/dsi: dsi_phy_14nm: Take ready-bit into account in poll_for

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]

Message-ID: <92d901c6-88e1-778d-dcef-39e460693237@linaro.org>
Date:   Fri, 1 Oct 2021 20:01:13 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Marijn Suijten <marijn.suijten@...ainline.org>,
        phone-devel@...r.kernel.org
Cc:     ~postmarketos/upstreaming@...ts.sr.ht,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Martin Botka <martin.botka@...ainline.org>,
        Jami Kettunen <jami.kettunen@...ainline.org>,
        Pavel Dubrova <pashadubrova@...il.com>,
        Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Abhinav Kumar <abhinavk@...eaurora.org>,
        Archit Taneja <architt@...eaurora.org>,
        linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/msm/dsi: dsi_phy_14nm: Take ready-bit into account in
 poll_for_ready

On 06/09/2021 23:25, Marijn Suijten wrote:
> The downstream driver models this PLL lock check as an if-elseif-else.
> The only way to reach the else case where pll_locked=true [1] is by
> succeeding both readl_poll_timeout_atomic calls (which return zero on
> success) in the if _and_ elseif condition.  Hence both the "lock" and
> "ready" bit need to be tested in the SM_READY_STATUS register before
> considering the PLL locked and ready to go.
> 
> Tested on the Sony Xperia XA2 Ultra (nile-discovery, sdm630).
> 
> [1]: https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/drivers/clk/qcom/mdss/mdss-dsi-pll-14nm-util.c?h=LA.UM.9.2.1.r1-08000-sdm660.0#n302
> 
> Fixes: f079f6d999cb ("drm/msm/dsi: Add PHY/PLL for 8x96")
> Signed-off-by: Marijn Suijten <marijn.suijten@...ainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>

> ---
>   drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 30 +++++++++++-----------
>   1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> index 8905f365c932..789b08c24d25 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
> @@ -110,14 +110,13 @@ static struct dsi_pll_14nm *pll_14nm_list[DSI_MAX];
>   static bool pll_14nm_poll_for_ready(struct dsi_pll_14nm *pll_14nm,
>   				    u32 nb_tries, u32 timeout_us)
>   {
> -	bool pll_locked = false;
> +	bool pll_locked = false, pll_ready = false;
>   	void __iomem *base = pll_14nm->phy->pll_base;
>   	u32 tries, val;
>   
>   	tries = nb_tries;
>   	while (tries--) {
> -		val = dsi_phy_read(base +
> -			       REG_DSI_14nm_PHY_PLL_RESET_SM_READY_STATUS);
> +		val = dsi_phy_read(base + REG_DSI_14nm_PHY_PLL_RESET_SM_READY_STATUS);
>   		pll_locked = !!(val & BIT(5));
>   
>   		if (pll_locked)
> @@ -126,23 +125,24 @@ static bool pll_14nm_poll_for_ready(struct dsi_pll_14nm *pll_14nm,
>   		udelay(timeout_us);
>   	}
>   
> -	if (!pll_locked) {
> -		tries = nb_tries;
> -		while (tries--) {
> -			val = dsi_phy_read(base +
> -				REG_DSI_14nm_PHY_PLL_RESET_SM_READY_STATUS);
> -			pll_locked = !!(val & BIT(0));
> +	if (!pll_locked)
> +		goto out;
>   
> -			if (pll_locked)
> -				break;
> +	tries = nb_tries;
> +	while (tries--) {
> +		val = dsi_phy_read(base + REG_DSI_14nm_PHY_PLL_RESET_SM_READY_STATUS);
> +		pll_ready = !!(val & BIT(0));
>   
> -			udelay(timeout_us);
> -		}
> +		if (pll_ready)
> +			break;
> +
> +		udelay(timeout_us);
>   	}
>   
> -	DBG("DSI PLL is %slocked", pll_locked ? "" : "*not* ");
> +out:
> +	DBG("DSI PLL is %slocked, %sready", pll_locked ? "" : "*not* ", pll_ready ? "" : "*not* ");
>   
> -	return pll_locked;
> +	return pll_locked && pll_ready;
>   }
>   
>   static void dsi_pll_14nm_config_init(struct dsi_pll_config *pconf)
> 


-- 
With best wishes
Dmitry