linux-kernel - Re: [PATCH 2/2] clk: qcom: rcg2, msm/dsi: Fix hangs caused by register writes while clocks are off

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: <6a547cb2-4014-4918-8e5d-fc7593ba4f2c@ixit.cz>
Date: Mon, 15 Dec 2025 15:35:53 +0100
From: David Heidelberg <david@...t.cz>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Rob Clark <robin.clark@....qualcomm.com>,
 Dmitry Baryshkov <lumag@...nel.org>, Abhinav Kumar
 <abhinav.kumar@...ux.dev>, Jessica Zhang <jesszhan0024@...il.com>,
 Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Bjorn Andersson <andersson@...nel.org>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, Petr Hodina <petr.hodina@...tonmail.com>,
 linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 linux-clk@...r.kernel.org, phone-devel@...r.kernel.org
Subject: Re: [PATCH 2/2] clk: qcom: rcg2, msm/dsi: Fix hangs caused by
 register writes while clocks are off

On 13/12/2025 05:01, Dmitry Baryshkov wrote:
> On Sat, Dec 13, 2025 at 12:08:17AM +0100, David Heidelberg via B4 Relay wrote:
>> From: Petr Hodina <petr.hodina@...tonmail.com>
>>
>> This patch fixes system hangs that occur when RCG2 and DSI code paths
>> perform register accesses while the associated clocks or power domains
>> are disabled.
> 
> In general this should not be happening. Do you have a description of
> the corresponding code path?
> 
>>
>> For the Qualcomm RCG2 clock driver, updating M/N/D registers while the
>> clock is gated can cause the hardware to lock up. Avoid toggling the
>> update bit when the clock is disabled and instead write the configuration
>> directly.
>>
>> Signed-off-by: Petr Hodina <petr.hodina@...tonmail.com>
>> Signed-off-by: David Heidelberg <david@...t.cz>
>> ---
>>   drivers/clk/qcom/clk-rcg2.c        | 18 ++++++++++++++++++
>>   drivers/gpu/drm/msm/dsi/dsi_host.c | 13 +++++++++++++
>>   2 files changed, 31 insertions(+)
> 
> This needs to be split into two patches.

Thank you for the feedback! Petr promised to look into it and sent v2 
until end of the week.

David

> 
>>
>> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
>> index e18cb8807d735..a18d2b9319670 100644
>> --- a/drivers/clk/qcom/clk-rcg2.c
>> +++ b/drivers/clk/qcom/clk-rcg2.c
>> @@ -1182,6 +1182,24 @@ static int clk_pixel_set_rate(struct clk_hw *hw, unsigned long rate,
>>   		f.m = frac->num;
>>   		f.n = frac->den;
>>   
>> +		/*
>> +		 * If clock is disabled, update the M, N and D registers and
>> +		 * don't hit the update bit.
>> +		 */
>> +		if (!clk_hw_is_enabled(hw)) {
>> +			int ret;
>> +
>> +			ret = regmap_read(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg), &cfg);
>> +			if (ret)
>> +				return ret;
>> +
>> +			ret = __clk_rcg2_configure(rcg, &f, &cfg);
>> +			if (ret)
>> +				return ret;
>> +
>> +			return regmap_write(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg), cfg);
>> +		}
>> +
>>   		return clk_rcg2_configure(rcg, &f);
>>   	}
>>   	return -EINVAL;
>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> index e0de545d40775..374ed966e960b 100644
>> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
>> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
>> @@ -762,6 +762,12 @@ dsi_get_cmd_fmt(const enum mipi_dsi_pixel_format mipi_fmt)
>>   
>>   static void dsi_ctrl_disable(struct msm_dsi_host *msm_host)
>>   {
>> +	/* Check if we're already powered off before writing registers */
>> +	if (!msm_host->power_on) {
>> +		pr_info("DSI CTRL: Skipping register write - host already powered off\n");
> 
> It definitely should be dev_something. Probably dev_warn().
> 
>> +		return;
>> +	}
>> +
>>   	dsi_write(msm_host, REG_DSI_CTRL, 0);
>>   }
>>   
>> @@ -2489,6 +2495,8 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host)
>>   {
>>   	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
>>   	const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
>> +	int ret;
>> +
> 
> Extra empty line
> 
>>   
>>   	mutex_lock(&msm_host->dev_mutex);
>>   	if (!msm_host->power_on) {
>> @@ -2496,6 +2504,11 @@ int msm_dsi_host_power_off(struct mipi_dsi_host *host)
>>   		goto unlock_ret;
>>   	}
>>   
>> +	/* Ensure clocks are enabled before register access */
> 
> And this looks like yet another fix, prompting for a separate commmit.
> 
>> +	ret = pm_runtime_get_sync(&msm_host->pdev->dev);
>> +	if (ret < 0)
>> +		pm_runtime_put_noidle(&msm_host->pdev->dev);
> 
> pm_runtime_resume_and_get()
> 
> Also, where is a corresponding put() ? We are leaking the runtime PM
> counter otherwise.
> 
>> +
>>   	dsi_ctrl_disable(msm_host);
>>   
>>   	pinctrl_pm_select_sleep_state(&msm_host->pdev->dev);
>>
>> -- 
>> 2.51.0
>>
>>
> 

-- 
David Heidelberg