* [PATCH v2] ufs: mcq: Adding a function for MCQ enable
@ 2023-12-21 6:56 ` Chanwoo Lee
0 siblings, 0 replies; 6+ messages in thread
From: Chanwoo Lee @ 2023-12-21 6:56 UTC (permalink / raw)
To: alim.akhtar, avri.altman, bvanassche, jejb, martin.petersen,
peter.wang, chu.stanley, matthias.bgg, angelogioacchino.delregno,
stanley.chu, quic_cang, mani, quic_asutoshd, powen.kao,
quic_nguyenb, yang.lee, beanhuo, Arthur.Simchaev, ebiggers,
linux-scsi, linux-kernel, linux-mediatek, linux-arm-kernel
Cc: grant.jung, jt77.jang, dh0421.hwang, sh043.lee, ChanWoo Lee
From: ChanWoo Lee <cw9316.lee@samsung.com>
The REG_UFS_MEM_CFG register is too general(broad)
and it is difficult to know the meaning only with a value of 0x1.
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@samsung.com>
* v1->v2:
1) Excluding ESI_ENABLE
2) Replace with ufshcd_rmwl, BIT()
3) Separating hba->mcq_enabled
---
drivers/ufs/core/ufs-mcq.c | 6 ++++++
drivers/ufs/core/ufshcd.c | 4 +---
drivers/ufs/host/ufs-mediatek.c | 4 +---
include/ufs/ufshcd.h | 1 +
include/ufs/ufshci.h | 3 +++
5 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
index 0787456c2b89..edc752e55878 100644
--- a/drivers/ufs/core/ufs-mcq.c
+++ b/drivers/ufs/core/ufs-mcq.c
@@ -399,6 +399,12 @@ void ufshcd_mcq_enable_esi(struct ufs_hba *hba)
}
EXPORT_SYMBOL_GPL(ufshcd_mcq_enable_esi);
+void ufshcd_mcq_enable(struct ufs_hba *hba)
+{
+ ufshcd_rmwl(hba, MCQ_MODE_SELECT, MCQ_MODE_SELECT, REG_UFS_MEM_CFG);
+}
+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..30df6f6a72c6 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -8723,9 +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);
+ ufshcd_mcq_enable(hba);
hba->mcq_enabled = true;
dev_info(hba->dev, "MCQ configured, nr_queues=%d, io_queues=%d, read_queue=%d, poll_queues=%d, queue_depth=%d\n",
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);
}
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..2a6989a70671 100644
--- a/include/ufs/ufshci.h
+++ b/include/ufs/ufshci.h
@@ -282,6 +282,9 @@ enum {
/* UTMRLRSR - UTP Task Management Request Run-Stop Register 80h */
#define UTP_TASK_REQ_LIST_RUN_STOP_BIT 0x1
+/* REG_UFS_MEM_CFG - Global Config Registers 300h */
+#define MCQ_MODE_SELECT BIT(0)
+
/* CQISy - CQ y Interrupt Status Register */
#define UFSHCD_MCQ_CQIS_TAIL_ENT_PUSH_STS 0x1
--
2.29.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] ufs: mcq: Adding a function for MCQ enable
2023-12-21 6:56 ` Chanwoo Lee
@ 2023-12-22 6:31 ` Peter Wang (王信友)
-1 siblings, 0 replies; 6+ messages in thread
From: Peter Wang (王信友) @ 2023-12-22 6:31 UTC (permalink / raw)
To: ebiggers@google.com, linux-kernel@vger.kernel.org,
linux-mediatek@lists.infradead.org, quic_nguyenb@quicinc.com,
jejb@linux.ibm.com, beanhuo@micron.com, avri.altman@wdc.com,
bvanassche@acm.org, cw9316.lee@samsung.com,
martin.petersen@oracle.com, quic_asutoshd@quicinc.com,
alim.akhtar@samsung.com, chu.stanley@gmail.com,
linux-scsi@vger.kernel.org, Powen Kao (高伯文),
Arthur.Simchaev@wdc.com, mani@kernel.org,
Stanley Chu (朱原陞), matthias.bgg@gmail.com,
linux-arm-kernel@lists.infradead.org, quic_cang@quicinc.com,
angelogioacchino.delregno@collabora.com,
yang.lee@linux.alibaba.com
Cc: jt77.jang@samsung.com, sh043.lee@samsung.com,
grant.jung@samsung.com, dh0421.hwang@samsung.com
On Thu, 2023-12-21 at 15:56 +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@samsung.com>
>
> The REG_UFS_MEM_CFG register is too general(broad)
> and it is difficult to know the meaning only with a value of 0x1.
> 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@samsung.com>
>
> * v1->v2:
> 1) Excluding ESI_ENABLE
> 2) Replace with ufshcd_rmwl, BIT()
> 3) Separating hba->mcq_enabled
> ---
>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ufs: mcq: Adding a function for MCQ enable
@ 2023-12-22 6:31 ` Peter Wang (王信友)
0 siblings, 0 replies; 6+ messages in thread
From: Peter Wang (王信友) @ 2023-12-22 6:31 UTC (permalink / raw)
To: ebiggers@google.com, linux-kernel@vger.kernel.org,
linux-mediatek@lists.infradead.org, quic_nguyenb@quicinc.com,
jejb@linux.ibm.com, beanhuo@micron.com, avri.altman@wdc.com,
bvanassche@acm.org, cw9316.lee@samsung.com,
martin.petersen@oracle.com, quic_asutoshd@quicinc.com,
alim.akhtar@samsung.com, chu.stanley@gmail.com,
linux-scsi@vger.kernel.org, Powen Kao (高伯文),
Arthur.Simchaev@wdc.com, mani@kernel.org,
Stanley Chu (朱原陞), matthias.bgg@gmail.com,
linux-arm-kernel@lists.infradead.org, quic_cang@quicinc.com,
angelogioacchino.delregno@collabora.com,
yang.lee@linux.alibaba.com
Cc: jt77.jang@samsung.com, sh043.lee@samsung.com,
grant.jung@samsung.com, dh0421.hwang@samsung.com
On Thu, 2023-12-21 at 15:56 +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@samsung.com>
>
> The REG_UFS_MEM_CFG register is too general(broad)
> and it is difficult to know the meaning only with a value of 0x1.
> 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@samsung.com>
>
> * v1->v2:
> 1) Excluding ESI_ENABLE
> 2) Replace with ufshcd_rmwl, BIT()
> 3) Separating hba->mcq_enabled
> ---
>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ufs: mcq: Adding a function for MCQ enable
2023-12-21 6:56 ` Chanwoo Lee
@ 2023-12-27 6:30 ` Manivannan Sadhasivam
-1 siblings, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2023-12-27 6:30 UTC (permalink / raw)
To: Chanwoo Lee
Cc: alim.akhtar, avri.altman, bvanassche, jejb, martin.petersen,
peter.wang, chu.stanley, matthias.bgg, angelogioacchino.delregno,
stanley.chu, quic_cang, quic_asutoshd, powen.kao, quic_nguyenb,
yang.lee, beanhuo, Arthur.Simchaev, ebiggers, linux-scsi,
linux-kernel, linux-mediatek, linux-arm-kernel, grant.jung,
jt77.jang, dh0421.hwang, sh043.lee
On Thu, Dec 21, 2023 at 03:56:08PM +0900, Chanwoo Lee wrote:
> From: ChanWoo Lee <cw9316.lee@samsung.com>
>
> The REG_UFS_MEM_CFG register is too general(broad)
> and it is difficult to know the meaning only with a value of 0x1.
> 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@samsung.com>
I would reword the subject and description as below:
```
ufs: mcq: Add definition for REG_UFS_MEM_CFG register
Instead of hardcoding the register field, add the proper definition. While
at it, let's also use ufshcd_rmwl() to simplify updating this register.
```
- Mani
>
> * v1->v2:
> 1) Excluding ESI_ENABLE
> 2) Replace with ufshcd_rmwl, BIT()
> 3) Separating hba->mcq_enabled
> ---
> drivers/ufs/core/ufs-mcq.c | 6 ++++++
> drivers/ufs/core/ufshcd.c | 4 +---
> drivers/ufs/host/ufs-mediatek.c | 4 +---
> include/ufs/ufshcd.h | 1 +
> include/ufs/ufshci.h | 3 +++
> 5 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 0787456c2b89..edc752e55878 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -399,6 +399,12 @@ void ufshcd_mcq_enable_esi(struct ufs_hba *hba)
> }
> EXPORT_SYMBOL_GPL(ufshcd_mcq_enable_esi);
>
> +void ufshcd_mcq_enable(struct ufs_hba *hba)
> +{
> + ufshcd_rmwl(hba, MCQ_MODE_SELECT, MCQ_MODE_SELECT, REG_UFS_MEM_CFG);
> +}
> +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..30df6f6a72c6 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8723,9 +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);
> + ufshcd_mcq_enable(hba);
> hba->mcq_enabled = true;
>
> dev_info(hba->dev, "MCQ configured, nr_queues=%d, io_queues=%d, read_queue=%d, poll_queues=%d, queue_depth=%d\n",
> 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);
> }
>
> 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..2a6989a70671 100644
> --- a/include/ufs/ufshci.h
> +++ b/include/ufs/ufshci.h
> @@ -282,6 +282,9 @@ enum {
> /* UTMRLRSR - UTP Task Management Request Run-Stop Register 80h */
> #define UTP_TASK_REQ_LIST_RUN_STOP_BIT 0x1
>
> +/* REG_UFS_MEM_CFG - Global Config Registers 300h */
> +#define MCQ_MODE_SELECT BIT(0)
> +
> /* CQISy - CQ y Interrupt Status Register */
> #define UFSHCD_MCQ_CQIS_TAIL_ENT_PUSH_STS 0x1
>
> --
> 2.29.0
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] ufs: mcq: Adding a function for MCQ enable
@ 2023-12-27 6:30 ` Manivannan Sadhasivam
0 siblings, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2023-12-27 6:30 UTC (permalink / raw)
To: Chanwoo Lee
Cc: alim.akhtar, avri.altman, bvanassche, jejb, martin.petersen,
peter.wang, chu.stanley, matthias.bgg, angelogioacchino.delregno,
stanley.chu, quic_cang, quic_asutoshd, powen.kao, quic_nguyenb,
yang.lee, beanhuo, Arthur.Simchaev, ebiggers, linux-scsi,
linux-kernel, linux-mediatek, linux-arm-kernel, grant.jung,
jt77.jang, dh0421.hwang, sh043.lee
On Thu, Dec 21, 2023 at 03:56:08PM +0900, Chanwoo Lee wrote:
> From: ChanWoo Lee <cw9316.lee@samsung.com>
>
> The REG_UFS_MEM_CFG register is too general(broad)
> and it is difficult to know the meaning only with a value of 0x1.
> 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@samsung.com>
I would reword the subject and description as below:
```
ufs: mcq: Add definition for REG_UFS_MEM_CFG register
Instead of hardcoding the register field, add the proper definition. While
at it, let's also use ufshcd_rmwl() to simplify updating this register.
```
- Mani
>
> * v1->v2:
> 1) Excluding ESI_ENABLE
> 2) Replace with ufshcd_rmwl, BIT()
> 3) Separating hba->mcq_enabled
> ---
> drivers/ufs/core/ufs-mcq.c | 6 ++++++
> drivers/ufs/core/ufshcd.c | 4 +---
> drivers/ufs/host/ufs-mediatek.c | 4 +---
> include/ufs/ufshcd.h | 1 +
> include/ufs/ufshci.h | 3 +++
> 5 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ufs/core/ufs-mcq.c b/drivers/ufs/core/ufs-mcq.c
> index 0787456c2b89..edc752e55878 100644
> --- a/drivers/ufs/core/ufs-mcq.c
> +++ b/drivers/ufs/core/ufs-mcq.c
> @@ -399,6 +399,12 @@ void ufshcd_mcq_enable_esi(struct ufs_hba *hba)
> }
> EXPORT_SYMBOL_GPL(ufshcd_mcq_enable_esi);
>
> +void ufshcd_mcq_enable(struct ufs_hba *hba)
> +{
> + ufshcd_rmwl(hba, MCQ_MODE_SELECT, MCQ_MODE_SELECT, REG_UFS_MEM_CFG);
> +}
> +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..30df6f6a72c6 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -8723,9 +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);
> + ufshcd_mcq_enable(hba);
> hba->mcq_enabled = true;
>
> dev_info(hba->dev, "MCQ configured, nr_queues=%d, io_queues=%d, read_queue=%d, poll_queues=%d, queue_depth=%d\n",
> 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);
> }
>
> 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..2a6989a70671 100644
> --- a/include/ufs/ufshci.h
> +++ b/include/ufs/ufshci.h
> @@ -282,6 +282,9 @@ enum {
> /* UTMRLRSR - UTP Task Management Request Run-Stop Register 80h */
> #define UTP_TASK_REQ_LIST_RUN_STOP_BIT 0x1
>
> +/* REG_UFS_MEM_CFG - Global Config Registers 300h */
> +#define MCQ_MODE_SELECT BIT(0)
> +
> /* CQISy - CQ y Interrupt Status Register */
> #define UFSHCD_MCQ_CQIS_TAIL_ENT_PUSH_STS 0x1
>
> --
> 2.29.0
>
--
மணிவண்ணன் சதாசிவம்
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread