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] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZKbDztmNp-KMXTDu@hovoldconsulting.com>
Date:   Thu, 6 Jul 2023 15:38:22 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc:     andersson@...nel.org, mturquette@...libre.com, sboyd@...nel.org,
        konrad.dybcio@...aro.org, johan+linaro@...nel.org,
        linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Revert "clk: gcc-sc8280xp: keep PCIe power-domains
 always-on"

On Thu, Jul 06, 2023 at 06:17:00PM +0530, Manivannan Sadhasivam wrote:
> This reverts commit 12d2a4769380f0dc9ba6f827839869db2b81ef00.

Please update the commit summary and remove or rephrase the above as
direct reverts are typically used for patches that were broken or causes
trouble (e.g rephrase as "allow pcie gdsdc to be disabled" or similar).

The patch in question was correct at the time even if it may no longer
be required, but see below.

> With the minimal system suspend support in place for the PCIe driver that
> keeps the interconnect path active, the ALWAYS_ON flags can now be dropped.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> ---
>  drivers/clk/qcom/gcc-sc8280xp.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gcc-sc8280xp.c b/drivers/clk/qcom/gcc-sc8280xp.c
> index 04a99dbaa57e..07eb6110442c 100644
> --- a/drivers/clk/qcom/gcc-sc8280xp.c
> +++ b/drivers/clk/qcom/gcc-sc8280xp.c
> @@ -6774,10 +6774,6 @@ static struct gdsc pcie_1_tunnel_gdsc = {
>  	.flags = VOTABLE,
>  };
>  
> -/*
> - * The Qualcomm PCIe driver does not yet implement suspend so to keep the
> - * PCIe power domains always-on for now.
> - */
>  static struct gdsc pcie_2a_gdsc = {
>  	.gdscr = 0x9d004,
>  	.collapse_ctrl = 0x52128,
> @@ -6786,7 +6782,7 @@ static struct gdsc pcie_2a_gdsc = {
>  		.name = "pcie_2a_gdsc",
>  	},
>  	.pwrsts = PWRSTS_OFF_ON,
> -	.flags = VOTABLE | ALWAYS_ON,
> +	.flags = VOTABLE,
>  };

Are you sure this is correct? Won't the controller state be lost if the
GDSC is powered off during suspend? Shouldn't this be PWRSTS_RET_ON at
least?

Johan

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ