[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <959c67c9-fb19-4601-84b7-87503779789d@quicinc.com>
Date: Mon, 6 Jan 2025 17:02:01 +0530
From: Ram Kumar Dwivedi <quic_rdwivedi@...cinc.com>
To: <manivannan.sadhasivam@...aro.org>,
<James.Bottomley@...senPartnership.com>, <martin.petersen@...cle.com>,
<andersson@...nel.org>, <bvanassche@....org>, <ebiggers@...nel.org>
CC: <linux-arm-msm@...r.kernel.org>, <linux-scsi@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
Naveen Kumar Goud Arepalli
<quic_narepall@...cinc.com>,
Nitin Rawat <quic_nitirawa@...cinc.com>
Subject: Re: [PATCH V8] scsi: ufs: qcom: Enable UFS Shared ICE Feature
A Gentle reminder, it’s been almost two weeks!!.
On 24-Dec-24 9:17 PM, Ram Kumar Dwivedi wrote:
> By default, the UFS controller allocates a fixed number of RX
> and TX engines statically. Consequently, when UFS reads are in
> progress, the TX ICE engines remain idle, and vice versa.
> This leads to inefficient utilization of RX and TX engines.
>
> To address this limitation, enable the UFS shared ICE feature for
> Qualcomm UFS V5.0 and above. This feature utilizes a pool of crypto
> cores for both TX streams (UFS Write – Encryption) and RX streams
> (UFS Read – Decryption). With this approach, crypto cores are
> dynamically allocated to either the RX or TX stream as needed.
>
> Co-developed-by: Naveen Kumar Goud Arepalli <quic_narepall@...cinc.com>
> Signed-off-by: Naveen Kumar Goud Arepalli <quic_narepall@...cinc.com>
> Co-developed-by: Nitin Rawat <quic_nitirawa@...cinc.com>
> Signed-off-by: Nitin Rawat <quic_nitirawa@...cinc.com>
> Signed-off-by: Ram Kumar Dwivedi <quic_rdwivedi@...cinc.com>
> ---
> Changes from v7:
> 1. Addressed Eric's comment to perform ice configuration only if
> UFSHCD_CAP_CRYPTO is enabled.
>
> Changes from v6:
> 1. Addressed Eric's comment to replace is_ice_config_supported() helper
> function with a conditional check for UFS_QCOM_CAP_ICE_CONFIG.
>
> Changes from v5:
> 1. Addressed Bart's comment to declare the "val" variable with
> the "static" keyword.
>
> Changes from v4:
> 1. Addressed Bart's comment to use get_unaligned_le32() instead of
> bit shifting and to declare val with the const keyword.
>
> Changes from v3:
> 1. Addressed Bart's comment to change the data type of "config" to u32
> and "val" to uint8_t.
>
> Changes from v2:
> 1. Refactored the code to have a single algorithm in the code and
> enabled by default.
> 2. Revised the commit message to incorporate the refactored change.
> 3. Qcom host capabilities are now enabled in a separate function.
>
> Changes from v1:
> 1. Addressed Rob's and Krzysztof's comment to fix dt binding compilation
> issue.
> 2. Addressed Rob's comment to enable the nodes in example.
> 3. Addressed Eric's comment to rephrase patch commit description.
> Used terminology as ICE allocator instead of ICE algorithm.
> 4. Addressed Christophe's comment to align the comment as per kernel doc.
> ---
> drivers/ufs/host/ufs-qcom.c | 38 ++++++++++++++++++++++++++++++++++
> drivers/ufs/host/ufs-qcom.h | 41 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 78 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> index 68040b2ab5f8..ffc67b5d5c3e 100644
> --- a/drivers/ufs/host/ufs-qcom.c
> +++ b/drivers/ufs/host/ufs-qcom.c
> @@ -15,6 +15,7 @@
> #include <linux/platform_device.h>
> #include <linux/reset-controller.h>
> #include <linux/time.h>
> +#include <linux/unaligned.h>
>
> #include <soc/qcom/ice.h>
>
> @@ -105,6 +106,26 @@ static struct ufs_qcom_host *rcdev_to_ufs_host(struct reset_controller_dev *rcd)
> }
>
> #ifdef CONFIG_SCSI_UFS_CRYPTO
> +/**
> + * ufs_qcom_config_ice_allocator() - ICE core allocator configuration
> + *
> + * @host: pointer to qcom specific variant structure.
> + */
> +static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
> +{
> + struct ufs_hba *hba = host->hba;
> + static const uint8_t val[4] = { NUM_RX_R1W0, NUM_TX_R0W1, NUM_RX_R1W1, NUM_TX_R1W1 };
> + u32 config;
> +
> + if (!(host->caps & UFS_QCOM_CAP_ICE_CONFIG) ||
> + !(host->hba->caps & UFSHCD_CAP_CRYPTO))
> + return;
> +
> + config = get_unaligned_le32(val);
> +
> + ufshcd_writel(hba, ICE_ALLOCATOR_TYPE, REG_UFS_MEM_ICE_CONFIG);
> + ufshcd_writel(hba, config, REG_UFS_MEM_ICE_NUM_CORE);
> +}
>
> static inline void ufs_qcom_ice_enable(struct ufs_qcom_host *host)
> {
> @@ -196,6 +217,11 @@ static inline int ufs_qcom_ice_suspend(struct ufs_qcom_host *host)
> {
> return 0;
> }
> +
> +static void ufs_qcom_config_ice_allocator(struct ufs_qcom_host *host)
> +{
> +}
> +
> #endif
>
> static void ufs_qcom_disable_lane_clks(struct ufs_qcom_host *host)
> @@ -435,6 +461,8 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba,
> err = ufs_qcom_enable_lane_clks(host);
> break;
> case POST_CHANGE:
> + ufs_qcom_config_ice_allocator(host);
> +
> /* check if UFS PHY moved from DISABLED to HIBERN8 */
> err = ufs_qcom_check_hibern8(hba);
> ufs_qcom_enable_hw_clk_gating(hba);
> @@ -932,6 +960,14 @@ static void ufs_qcom_set_host_params(struct ufs_hba *hba)
> host_params->hs_tx_gear = host_params->hs_rx_gear = ufs_qcom_get_hs_gear(hba);
> }
>
> +static void ufs_qcom_set_host_caps(struct ufs_hba *hba)
> +{
> + struct ufs_qcom_host *host = ufshcd_get_variant(hba);
> +
> + if (host->hw_ver.major >= 0x5)
> + host->caps |= UFS_QCOM_CAP_ICE_CONFIG;
> +}
> +
> static void ufs_qcom_set_caps(struct ufs_hba *hba)
> {
> hba->caps |= UFSHCD_CAP_CLK_GATING | UFSHCD_CAP_HIBERN8_WITH_CLK_GATING;
> @@ -940,6 +976,8 @@ static void ufs_qcom_set_caps(struct ufs_hba *hba)
> hba->caps |= UFSHCD_CAP_WB_EN;
> hba->caps |= UFSHCD_CAP_AGGR_POWER_COLLAPSE;
> hba->caps |= UFSHCD_CAP_RPM_AUTOSUSPEND;
> +
> + ufs_qcom_set_host_caps(hba);
> }
>
> /**
> diff --git a/drivers/ufs/host/ufs-qcom.h b/drivers/ufs/host/ufs-qcom.h
> index b9de170983c9..92e2278b6a54 100644
> --- a/drivers/ufs/host/ufs-qcom.h
> +++ b/drivers/ufs/host/ufs-qcom.h
> @@ -196,7 +196,8 @@ struct ufs_qcom_host {
> #ifdef CONFIG_SCSI_UFS_CRYPTO
> struct qcom_ice *ice;
> #endif
> -
> + #define UFS_QCOM_CAP_ICE_CONFIG BIT(0)
> + u32 caps;
> void __iomem *dev_ref_clk_ctrl_mmio;
> bool is_dev_ref_clk_enabled;
> struct ufs_hw_version hw_ver;
> @@ -226,6 +227,44 @@ ufs_qcom_get_debug_reg_offset(struct ufs_qcom_host *host, u32 reg)
> return UFS_CNTLR_3_x_x_VEN_REGS_OFFSET(reg);
> };
>
> +#ifdef CONFIG_SCSI_UFS_CRYPTO
> +
> +/* ICE configuration to share AES engines among TX stream and RX stream */
> +#define ICE_ALLOCATOR_TYPE 2
> +#define REG_UFS_MEM_ICE_CONFIG 0x260C
> +#define REG_UFS_MEM_ICE_NUM_CORE 0x2664
> +
> +
> +/*
> + * Number of cores allocated for RX stream when Read data block received and
> + * Write data block is not in progress
> + */
> +#define NUM_RX_R1W0 28
> +
> +/*
> + * Number of cores allocated for TX stream when Device asked to send write
> + * data block and Read data block is not in progress
> + */
> +#define NUM_TX_R0W1 28
> +
> +/*
> + * Number of cores allocated for RX stream when Read data block received and
> + * Write data block is in progress
> + * OR
> + * Device asked to send write data block and Read data block is in progress
> + */
> +#define NUM_RX_R1W1 15
> +
> +/*
> + * Number of cores allocated for TX stream (UFS write) when Read data block
> + * received and Write data block is in progress
> + * OR
> + * Device asked to send write data block and Read data block is in progress
> + */
> +#define NUM_TX_R1W1 13
> +
> +#endif /* UFS_CRYPTO */
> +
> #define ufs_qcom_is_link_off(hba) ufshcd_is_link_off(hba)
> #define ufs_qcom_is_link_active(hba) ufshcd_is_link_active(hba)
> #define ufs_qcom_is_link_hibern8(hba) ufshcd_is_link_hibern8(hba)
Powered by blists - more mailing lists