scsi: ufs: ufs-mediatek: Change dbg select by check hw version
Subsystems (2)
PATCH v3 1/1 scsi: ufs: ufs-mediatek: Change dbg select by check hw version
From: Peter Wang <[email protected]>
Mediatek UFS dbg select setting is changed in new HW version. This patch check the HW version before set dbg select.
--- drivers/scsi/ufs/ufs-mediatek.c | 23 +++++++++++++++++++++-- drivers/scsi/ufs/ufs-mediatek.h | 5 +++++ 2 files changed, 26 insertions(+), 2 deletions(-)
drivers/scsi/ufs/ufs-mediatek.c +21 -2
@@ -296,6 +296,25 @@ static void ufs_mtk_setup_ref_clk_wait_us(struct ufs_hba *hba, host->ref_clk_ungating_wait_us = ungating_us; } +__no_kcsan +static void ufs_mtk_dbg_sel(struct ufs_hba *hba) +{ +static u32 hw_ver; + +if (!hw_ver) + hw_ver =ufshcd_readl (hba,REG_UFS_MTK_HW_VER ); + +if (((hw_ver >>16 ) &0xFF ) >=0x36 ) { +ufshcd_writel (hba,0x820820 ,REG_UFS_DEBUG_SEL ); +ufshcd_writel (hba,0x0 ,REG_UFS_DEBUG_SEL_B0 ); +ufshcd_writel (hba,0x55555555 ,REG_UFS_DEBUG_SEL_B1 ); +ufshcd_writel (hba,0xaaaaaaaa ,REG_UFS_DEBUG_SEL_B2 ); +ufshcd_writel (hba,0xffffffff ,REG_UFS_DEBUG_SEL_B3 ); + }else { +ufshcd_writel (hba,0x20 ,REG_UFS_DEBUG_SEL ); + } +} +static int ufs_mtk_wait_link_state (struct ufs_hba *hba,u32 state,unsigned long max_wait_ms) {
@@ -305,7 +324,7 @@ static int ufs_mtk_wait_link_state(struct ufs_hba *hba, u32 state, timeout =ktime_add_ms (ktime_get (), max_wait_ms); do { time_checked = ktime_get(); - ufshcd_writel(hba, 0x20, REG_UFS_DEBUG_SEL); + ufs_mtk_dbg_sel(hba); val = ufshcd_readl(hba, REG_UFS_PROBE); val = val >> 28;
@@ -1001,7 +1020,7 @@ static void ufs_mtk_dbg_register_dump(struct ufs_hba *hba) "MPHY Ctrl ");/* Direct debugging information to REG_MTK_PROBE */ -ufshcd_writel (hba,0x20 ,REG_UFS_DEBUG_SEL ); +ufs_mtk_dbg_sel (hba);ufshcd_dump_regs (hba,REG_UFS_PROBE ,0x4 ,"Debug Probe " ); }
drivers/scsi/ufs/ufs-mediatek.h +5 -0
@@ -15,9 +15,14 @@#define REG_UFS_REFCLK_CTRL 0x144#define REG_UFS_EXTREG 0x2100#define REG_UFS_MPHYCTRL 0x2200 +#define REG_UFS_MTK_HW_VER 0x2240#define REG_UFS_REJECT_MON 0x22AC#define REG_UFS_DEBUG_SEL 0x22C0#define REG_UFS_PROBE 0x22C8 +#define REG_UFS_DEBUG_SEL_B0 0x22D0 +#define REG_UFS_DEBUG_SEL_B1 0x22D4 +#define REG_UFS_DEBUG_SEL_B2 0x22D8 +#define REG_UFS_DEBUG_SEL_B3 0x22DC/* * Ref-clk control
Signature · 1 line
1.7.9.5
Hi Peter,
Quoted 4 lines · On Wed, 2021-09-01 at 14:04 +0800, [email protected] wrote:
From: Peter Wang <[email protected]>
Mediatek UFS dbg select setting is changed in new HW version. This patch check the HW version before set dbg select.
Nits: This patch checks the HW version before setting dbg select.
Quoted 13 lines
Signed-off-by Peter Wang <[email protected]>--- drivers/scsi/ufs/ufs-mediatek.c | 23 +++++++++++++++++++++-- drivers/scsi/ufs/ufs-mediatek.h | 5 +++++ 2 files changed, 26 insertions(+), 2 deletions(-)drivers/scsi/ufs/ufs- renamed from drivers/scsi/ufs/ufs-mediatek.c +0
-0
mediatek.c
index d2c2516..0050e01 100644
drivers/scsi/ufs/ufs-mediatek.c static void ufs_mtk_setup_ref_clk_wait_us(struct · line 300
Quoted 5 lines
drivers/scsi/ufs/ufs-mediatek.c +1
-0@@ -296,6 +296,25 @@ static void ufs_mtk_setup_ref_clk_wait_us(struct ufs_hba *hba, host->ref_clk_ungating_wait_us = ungating_us; } +__no_kcsan
This is rarely used in mainstream kernel. According to my grep results, __no_kcsan is only used by kcsan-test itself.
Besides, dbg select configuration may not be necessary if the mode is already configured before? I just wonder that can we avoid setting these registers every query?
drivers/scsi/ufs/ufs-mediatek.c static void ufs_mtk_setup_ref_clk_wait_us(struct ufs_hba *hba, · line 305
Quoted 6 lines
drivers/scsi/ufs/ufs-mediatek.c +6
-0+static void ufs_mtk_dbg_sel (struct ufs_hba *hba) +{ +static u32 hw_ver; + +if (!hw_ver) + hw_ver =ufshcd_readl (hba,REG_UFS_MTK_HW_VER );
Perhaps you can keep this version in struct host->hw_ver? Maybe you need to add a new member in that struct, for example, ip_ver.
Quoted 41 lines
drivers/scsi/ufs/ufs-mediatek.c +12
-0+ +if (((hw_ver >>16 ) &0xFF ) >=0x36 ) { +ufshcd_writel (hba,0x820820 ,REG_UFS_DEBUG_SEL ); +ufshcd_writel (hba,0x0 ,REG_UFS_DEBUG_SEL_B0 ); +ufshcd_writel (hba,0x55555555 ,REG_UFS_DEBUG_SEL_B1 ); +ufshcd_writel (hba,0xaaaaaaaa ,REG_UFS_DEBUG_SEL_B2 ); +ufshcd_writel (hba,0xffffffff ,REG_UFS_DEBUG_SEL_B3 ); + }else { +ufshcd_writel (hba,0x20 ,REG_UFS_DEBUG_SEL ); + } +} +static int ufs_mtk_wait_link_state (struct ufs_hba *hba,u32 state,unsigned long max_wait_ms){
drivers/scsi/ufs/ufs-mediatek.c +2
-2@@ -305,7 +324,7 @@ static int ufs_mtk_wait_link_state(struct ufs_hba *hba, u32 state, timeout =ktime_add_ms (ktime_get (), max_wait_ms); do { time_checked = ktime_get(); - ufshcd_writel(hba, 0x20, REG_UFS_DEBUG_SEL); + ufs_mtk_dbg_sel(hba); val = ufshcd_readl(hba, REG_UFS_PROBE); val = val >> 28;@@ -1001,7 +1020,7 @@ static void ufs_mtk_dbg_register_dump(struct ufs_hba *hba) "MPHY Ctrl ");/* Direct debugging information to REG_MTK_PROBE */ -ufshcd_writel (hba,0x20 ,REG_UFS_DEBUG_SEL ); +ufs_mtk_dbg_sel (hba);ufshcd_dump_regs (hba,REG_UFS_PROBE ,0x4 ,"Debug Probe " ); }drivers/scsi/ufs/ufs- renamed from drivers/scsi/ufs/ufs-mediatek.h +0
-0
mediatek.h
index 3f0d3bb..fc40c05 100644
drivers/scsi/ufs/ufs-mediatek.h line 18
Quoted 4 lines
drivers/scsi/ufs/ufs-mediatek.h +1
-0@@ -15,9 +15,14 @@#define REG_UFS_REFCLK_CTRL 0x144#define REG_UFS_EXTREG 0x2100#define REG_UFS_MPHYCTRL 0x2200 +#define REG_UFS_MTK_HW_VER 0x2240
HW_VER is somehow ambiguous, for example, how about REG_UFS_MTK_IP_VER?
drivers/scsi/ufs/ufs-mediatek.h line 25
Quoted 7 lines
drivers/scsi/ufs/ufs-mediatek.h +4
-0#define REG_UFS_REJECT_MON 0x22AC#define REG_UFS_DEBUG_SEL 0x22C0#define REG_UFS_PROBE 0x22C8 +#define REG_UFS_DEBUG_SEL_B0 0x22D0 +#define REG_UFS_DEBUG_SEL_B1 0x22D4 +#define REG_UFS_DEBUG_SEL_B2 0x22D8 +#define REG_UFS_DEBUG_SEL_B3 0x22DC
Perhaps the debug select design could be simplified in the future, for example, driver can query what it wants by reading only one register without configuring anything first? Although this is beyond the scope of this patch.
Thanks,
Stanley Chu
Quoted 3 lines
drivers/scsi/ufs/ufs-mediatek.h +0
-0/* * Ref-clk control
Quoted 34 lines · Stanley Chu
Hi Peter,
Quoted 4 lines · On Wed, 2021-09-01 at 14:04 +0800, [email protected] wrote:
From: Peter Wang <[email protected]>
Mediatek UFS dbg select setting is changed in new HW version. This patch check the HW version before set dbg select.
Nits: This patch checks the HW version before setting dbg select.
Quoted 16 lines
Signed-off-by Peter Wang <[email protected]>--- drivers/scsi/ufs/ufs-mediatek.c | 23 +++++++++++++++++++++-- drivers/scsi/ufs/ufs-mediatek.h | 5 +++++ 2 files changed, 26 insertions(+), 2 deletions(-)drivers/scsi/ufs/ufs- renamed from drivers/scsi/ufs/ufs-mediatek.c +0
-0
mediatek.c
index d2c2516..0050e01 100644drivers/scsi/ufs/ufs-mediatek.c +0
-0@@ -296,6 +296,25 @@ static voidufs_mtk_setup_ref_clk_wait_us(struct
ufs_hba*hba,drivers/scsi/ufs/ufs-mediatek.c static void ufs_mtk_setup_ref_clk_wait_us(struct ufs_hba *hba, · line 299
Quoted 4 lines
drivers/scsi/ufs/ufs-mediatek.c +1
-0host->ref_clk_ungating_wait_us = ungating_us; } +__no_kcsanThis is rarely used in mainstream kernel. According to my grep results,
__no_kcsanis only used by kcsan-test itself.
Quoted 4 lines
Besides, dbg select configuration may not be necessary if the mode is already configured before? I just wonder that can we avoid setting these registers every query?
Yes, will remove __no_kcsan and add a ip_ver in mediatek host struct to avoid KCSAN warning.
Dbg select value will clear by hci reset. Maybe we cand add a variable in mediatek host sruct to record if need set dbg select.
Quoted 46 lines
drivers/scsi/ufs/ufs-mediatek.c static void ufs_mtk_setup_ref_clk_wait_us(struct ufs_hba *hba, · line 305
Quoted 6 lines
drivers/scsi/ufs/ufs-mediatek.c +6
-0+static void ufs_mtk_dbg_sel (struct ufs_hba *hba) +{ +static u32 hw_ver; + +if (!hw_ver) + hw_ver =ufshcd_readl (hba,REG_UFS_MTK_HW_VER );Perhaps you can keep this version in struct
host->hw_ver? Maybe you need to add a new member in that struct, for example,ip_ver.Quoted 36 lines
drivers/scsi/ufs/ufs-mediatek.c +12
-0+ +if (((hw_ver >>16 ) &0xFF ) >=0x36 ) { +ufshcd_writel (hba,0x820820 ,REG_UFS_DEBUG_SEL ); +ufshcd_writel (hba,0x0 ,REG_UFS_DEBUG_SEL_B0 ); +ufshcd_writel (hba,0x55555555 ,REG_UFS_DEBUG_SEL_B1 ); +ufshcd_writel (hba,0xaaaaaaaa ,REG_UFS_DEBUG_SEL_B2 ); +ufshcd_writel (hba,0xffffffff ,REG_UFS_DEBUG_SEL_B3 ); + }else { +ufshcd_writel (hba,0x20 ,REG_UFS_DEBUG_SEL ); + } +} +static int ufs_mtk_wait_link_state (struct ufs_hba *hba,u32 state,unsigned long max_wait_ms) {@@ -305,7 +324,7 @@ static int ufs_mtk_wait_link_state(struct
ufs_hba
*hba, u32 state,drivers/scsi/ufs/ufs-mediatek.c +2
-2timeout =ktime_add_ms (ktime_get (), max_wait_ms); do { time_checked = ktime_get(); - ufshcd_writel(hba, 0x20, REG_UFS_DEBUG_SEL); + ufs_mtk_dbg_sel(hba); val = ufshcd_readl(hba, REG_UFS_PROBE); val = val >> 28;@@ -1001,7 +1020,7 @@ static void ufs_mtk_dbg_register_dump(struct ufs_hba *hba) "MPHY Ctrl ");/* Direct debugging information to REG_MTK_PROBE */ -ufshcd_writel (hba,0x20 ,REG_UFS_DEBUG_SEL ); +ufs_mtk_dbg_sel (hba);ufshcd_dump_regs (hba,REG_UFS_PROBE ,0x4 ,"Debug Probe " ); }
Quoted 15 lines
drivers/scsi/ufs/ufs-mediatek.h line 18
Quoted 11 lines
drivers/scsi/ufs/ufs- renamed from drivers/scsi/ufs/ufs-mediatek.h +0
-0
mediatek.h
index 3f0d3bb..fc40c05 100644drivers/scsi/ufs/ufs-mediatek.h +1
-0@@ -15,9 +15,14 @@#define REG_UFS_REFCLK_CTRL 0x144#define REG_UFS_EXTREG 0x2100#define REG_UFS_MPHYCTRL 0x2200 +#define REG_UFS_MTK_HW_VER 0x2240
HW_VERis somehow ambiguous, for example, how aboutREG_UFS_MTK_IP_VER?
Sure, could change naming for easy understand.
Quoted 22 lines
drivers/scsi/ufs/ufs-mediatek.h line 25
Quoted 7 lines
drivers/scsi/ufs/ufs-mediatek.h +4
-0#define REG_UFS_REJECT_MON 0x22AC#define REG_UFS_DEBUG_SEL 0x22C0#define REG_UFS_PROBE 0x22C8 +#define REG_UFS_DEBUG_SEL_B0 0x22D0 +#define REG_UFS_DEBUG_SEL_B1 0x22D4 +#define REG_UFS_DEBUG_SEL_B2 0x22D8 +#define REG_UFS_DEBUG_SEL_B3 0x22DCPerhaps the debug select design could be simplified in the future, for
example, driver can query what it wants by reading only one register without configuring anything first? Although this is beyond the scope of this patch.Thanks,
Stanley ChuQuoted 3 lines
drivers/scsi/ufs/ufs-mediatek.h +0
-0/* * Ref-clk control