linux-kernel - Re: [PATCH v1] ufs: ufs-qcom: Align programming sequence of Shared ICE for UFS controller v5

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: <ucr4imzntw6ghcvpeioprmva7gxrqnkphjirjppnqgdpq5ghss@y5nwjzzpvluj>
Date: Wed, 6 Aug 2025 16:44:14 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Palash Kambar <quic_pkambar@...cinc.com>
Cc: James.Bottomley@...senpartnership.com, martin.petersen@...cle.com, 
	linux-arm-msm@...r.kernel.org, linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org, 
	quic_nitirawa@...cinc.com
Subject: Re: [PATCH v1] ufs: ufs-qcom: Align programming sequence of Shared
 ICE for  UFS controller v5

On Wed, Aug 06, 2025 at 12:04:09PM GMT, Palash Kambar wrote:
> Disable of AES core in Shared ICE is not supported during power
> collapse for UFS Host Controller V5.0.
> 
> Hence follow below steps to reset the ICE upon exiting power collapse
> and align with Hw programming guide.
> 
> a. Write 0x18 to UFS_MEM_ICE_CFG
> b. Write 0x0 to UFS_MEM_ICE_CFG
> 
> Signed-off-by: Palash Kambar <quic_pkambar@...cinc.com>
> ---
>  drivers/ufs/host/ufs-qcom.c | 24 ++++++++++++++++++++++++
>  drivers/ufs/host/ufs-qcom.h |  2 ++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 444a09265ded..2744614bbc32 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -744,6 +744,8 @@ static int ufs_qcom_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op,
>  	if (ufs_qcom_is_link_off(hba) && host->device_reset)
>  		ufs_qcom_device_reset_ctrl(hba, true);
>  
> +	host->vdd_hba_pc = true;

What does this variable correspond to?

> +
>  	return ufs_qcom_ice_suspend(host);
>  }
>  
> @@ -759,6 +761,27 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
>  	return ufs_qcom_ice_resume(host);
>  }
>  
> +static void ufs_qcom_hibern8_notify(struct ufs_hba *hba,
> +				    enum uic_cmd_dme uic_cmd,
> +				    enum ufs_notify_change_status status)
> +{
> +	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +
> +	/* Apply shared ICE WA */

Are you really sure it is *shared ICE*?

> +	if (uic_cmd == UIC_CMD_DME_HIBER_EXIT &&
> +	    status == POST_CHANGE &&
> +	    host->hw_ver.major == 0x5 &&
> +	    host->hw_ver.minor == 0x0 &&
> +	    host->hw_ver.step == 0x0 &&
> +	    host->vdd_hba_pc) {
> +		host->vdd_hba_pc = false;
> +		ufshcd_writel(hba, 0x18, UFS_MEM_ICE);

Define the actual bits instead of writing magic values.

> +		ufshcd_readl(hba, UFS_MEM_ICE);
> +		ufshcd_writel(hba, 0x0, UFS_MEM_ICE);
> +		ufshcd_readl(hba, UFS_MEM_ICE);

Why do you need readl()? Writes to device memory won't get reordered.

> +	}
> +}
> +
>  static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable)
>  {
>  	if (host->dev_ref_clk_ctrl_mmio &&
> @@ -2258,6 +2281,7 @@ static const struct ufs_hba_variant_ops ufs_hba_qcom_vops = {
>  	.hce_enable_notify      = ufs_qcom_hce_enable_notify,
>  	.link_startup_notify    = ufs_qcom_link_startup_notify,
>  	.pwr_change_notify	= ufs_qcom_pwr_change_notify,
> +	.hibern8_notify		= ufs_qcom_hibern8_notify,

This callback is not called anywhere. Regardeless of that, why can't you use
ufs_qcom_clk_scale_notify()?

- Mani

-- 
மணிவண்ணன் சதாசிவம்