linux-kernel - Re: [PATCH] ufs: mcq: Adding a function for MCQ enable

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]
Date: Wed, 20 Dec 2023 07:03:10 +0000
From: Peter Wang (王信友) <peter.wang@...iatek.com>
To: "ebiggers@...gle.com" <ebiggers@...gle.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"quic_nguyenb@...cinc.com" <quic_nguyenb@...cinc.com>, "jejb@...ux.ibm.com"
	<jejb@...ux.ibm.com>, "beanhuo@...ron.com" <beanhuo@...ron.com>,
	"avri.altman@....com" <avri.altman@....com>, "bvanassche@....org"
	<bvanassche@....org>, "cw9316.lee@...sung.com" <cw9316.lee@...sung.com>,
	"martin.petersen@...cle.com" <martin.petersen@...cle.com>,
	"quic_asutoshd@...cinc.com" <quic_asutoshd@...cinc.com>,
	"alim.akhtar@...sung.com" <alim.akhtar@...sung.com>, "chu.stanley@...il.com"
	<chu.stanley@...il.com>, "linux-scsi@...r.kernel.org"
	<linux-scsi@...r.kernel.org>, Powen Kao (高伯文)
	<Powen.Kao@...iatek.com>, "Arthur.Simchaev@....com"
	<Arthur.Simchaev@....com>, "mani@...nel.org" <mani@...nel.org>,
	Stanley Chu (朱原陞) <stanley.chu@...iatek.com>,
	"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "quic_cang@...cinc.com"
	<quic_cang@...cinc.com>, "angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>, "yang.lee@...ux.alibaba.com"
	<yang.lee@...ux.alibaba.com>
CC: "jt77.jang@...sung.com" <jt77.jang@...sung.com>, "sh043.lee@...sung.com"
	<sh043.lee@...sung.com>, "grant.jung@...sung.com" <grant.jung@...sung.com>,
	"dh0421.hwang@...sung.com" <dh0421.hwang@...sung.com>
Subject: Re: [PATCH] ufs: mcq: Adding a function for MCQ enable

On Wed, 2023-12-20 at 14:27 +0900, Chanwoo Lee wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  From: ChanWoo Lee <cw9316.lee@...sung.com>
> 
> The REG_UFS_MEM_CFG register is too general(broad)
> and it is difficult to know the meaning of only values of 0x1 and
> 0x2.
> So far, comments were required.
> 
> Therefore, I have added new functions and defines
> to improve code readability/reusability.
> 
> Signed-off-by: ChanWoo Lee <cw9316.lee@...sung.com>
> ---
>  drivers/ufs/core/ufs-mcq.c      | 10 +++++++++-
>  drivers/ufs/core/ufshcd.c       |  5 +----
>  drivers/ufs/host/ufs-mediatek.c |  4 +---
>  include/ufs/ufshcd.h            |  1 +
>  include/ufs/ufshci.h            |  4 ++++
>  5 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 0787456c2b89..a34ef3aac540 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -394,11 +394,19 @@
> EXPORT_SYMBOL_GPL(ufshcd_mcq_make_queues_operational);
>  
>  void ufshcd_mcq_enable_esi(struct ufs_hba *hba)
>  {
> -ufshcd_writel(hba, ufshcd_readl(hba, REG_UFS_MEM_CFG) | 0x2,
> +ufshcd_writel(hba, ufshcd_readl(hba, REG_UFS_MEM_CFG) | ESI_ENABLE,
>        REG_UFS_MEM_CFG);
>  }
>  EXPORT_SYMBOL_GPL(ufshcd_mcq_enable_esi);
>  
> +void ufshcd_mcq_enable(struct ufs_hba *hba)
> +{
> +ufshcd_writel(hba, ufshcd_readl(hba, REG_UFS_MEM_CFG) |
> MCQ_MODE_SELECT,
> +      REG_UFS_MEM_CFG);
> +hba->mcq_enabled = true;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_mcq_enable);
> +
>  void ufshcd_mcq_config_esi(struct ufs_hba *hba, struct msi_msg *msg)
>  {
>  ufshcd_writel(hba, msg->address_lo, REG_UFS_ESILBA);
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index ae9936fc6ffb..8195e01e7a3f 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8723,10 +8723,7 @@ static void ufshcd_config_mcq(struct ufs_hba
> *hba)
>  hba->host->can_queue = hba->nutrs - UFSHCD_NUM_RESERVED;
>  hba->reserved_slot = hba->nutrs - UFSHCD_NUM_RESERVED;
>  
> -/* Select MCQ mode */
> -ufshcd_writel(hba, ufshcd_readl(hba, REG_UFS_MEM_CFG) | 0x1,
> -      REG_UFS_MEM_CFG);
> -hba->mcq_enabled = true;
> +ufshcd_mcq_enable(hba);
>  
>  dev_info(hba->dev, "MCQ configured, nr_queues=%d, io_queues=%d,
> read_queue=%d, poll_queues=%d, queue_depth=%d\n",
>   hba->nr_hw_queues, hba->nr_queues[HCTX_TYPE_DEFAULT],
> diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-
> mediatek.c
> index fc61790d289b..1048add66419 100644
> --- a/drivers/ufs/host/ufs-mediatek.c
> +++ b/drivers/ufs/host/ufs-mediatek.c
> @@ -1219,9 +1219,7 @@ static int ufs_mtk_link_set_hpm(struct ufs_hba
> *hba)
>  ufs_mtk_config_mcq(hba, false);
>  ufshcd_mcq_make_queues_operational(hba);
>  ufshcd_mcq_config_mac(hba, hba->nutrs);
> -/* Enable MCQ mode */
> -ufshcd_writel(hba, ufshcd_readl(hba, REG_UFS_MEM_CFG) | 0x1,
> -      REG_UFS_MEM_CFG);
> +ufshcd_mcq_enable(hba);

Hi Chanwoo,

It is better change 0x1 to MCQ_MODE_SELECT only.
And do not change hba->mcq_enabled value because we enable mcq 
by check hba->mcq_enabled is true.

Thanks.
Peter



>  }
>  
>  if (err)
> diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
> index d862c8ddce03..a96c45fa4b4b 100644
> --- a/include/ufs/ufshcd.h
> +++ b/include/ufs/ufshcd.h
> @@ -1257,6 +1257,7 @@ unsigned long ufshcd_mcq_poll_cqe_lock(struct
> ufs_hba *hba,
>   struct ufs_hw_queue *hwq);
>  void ufshcd_mcq_make_queues_operational(struct ufs_hba *hba);
>  void ufshcd_mcq_enable_esi(struct ufs_hba *hba);
> +void ufshcd_mcq_enable(struct ufs_hba *hba);
>  void ufshcd_mcq_config_esi(struct ufs_hba *hba, struct msi_msg
> *msg);
>  
>  int ufshcd_opp_config_clks(struct device *dev, struct opp_table
> *opp_table,
> diff --git a/include/ufs/ufshci.h b/include/ufs/ufshci.h
> index d5accacae6bc..e669fad11fd4 100644
> --- a/include/ufs/ufshci.h
> +++ b/include/ufs/ufshci.h
> @@ -282,6 +282,10 @@ enum {
>  /* UTMRLRSR - UTP Task Management Request Run-Stop Register 80h */
>  #define UTP_TASK_REQ_LIST_RUN_STOP_BIT0x1
>  
> +/* REG_UFS_MEM_CFG - Global Config Registers 300h */
> +#define MCQ_MODE_SELECT 0x1
> +#define ESI_ENABLE0x2
> +
>  /* CQISy - CQ y Interrupt Status Register  */
>  #define UFSHCD_MCQ_CQIS_TAIL_ENT_PUSH_STS0x1
>  
> -- 
> 2.29.0
>