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(-)
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 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@samsung.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);
}
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_BIT 0x1
+/* REG_UFS_MEM_CFG - Global Config Registers 300h */
+#define MCQ_MODE_SELECT 0x1
+#define ESI_ENABLE 0x2
+
/* CQISy - CQ y Interrupt Status Register */
#define UFSHCD_MCQ_CQIS_TAIL_ENT_PUSH_STS 0x1
--
2.29.0
On Wed, Dec 20, 2023 at 02:27:37PM +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 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@samsung.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); This change should be a separate patch. > } > 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); Use ufshcd_rmwl(). > + 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); hba->mcq_enabled flag will be set now which is not done previously. > } > > 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_BIT 0x1 > > +/* REG_UFS_MEM_CFG - Global Config Registers 300h */ > +#define MCQ_MODE_SELECT 0x1 > +#define ESI_ENABLE 0x2 Use BIT() macros. - Mani > + > /* CQISy - CQ y Interrupt Status Register */ > #define UFSHCD_MCQ_CQIS_TAIL_ENT_PUSH_STS 0x1 > > -- > 2.29.0 > -- மணிவண்ணன் சதாசிவம்
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@samsung.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@samsung.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 >
© 2016 - 2024 Red Hat, Inc.