| Message ID | 1630325486-11741-1-git-send-email-peter.wang@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] scsi: ufs: ufs-mediatek: Change dbg select by check hw version | expand |
Hi Bart, REG_UFS_MTK_HW_VER is a read only mediatek dedicated register. So, hw_ver will get a const value for mediatek to decide how to use debug select. It only need read once, no need multi-threads protected. Thanks Peter On Mon, 2021-08-30 at 19:47 -0700, Bart Van Assche wrote: > On 8/30/21 05:11, peter.wang@mediatek.com wrote: > > +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); > > + } > > +} > > Can ufs_mtk_dbg_sel() be called from multiple threads at the same > time? > Does the 'hw_ver' variable need to be protected against concurrent > writes? > > Thanks, > > Bart.
On 8/31/21 19:29, Peter Wang wrote: > REG_UFS_MTK_HW_VER is a read only mediatek dedicated register. > So, hw_ver will get a const value for mediatek to decide how to use > debug select. It only need read once, no need multi-threads protected. Hi Peter, The above can be concluded easily by a human but not by software. If this code is analyzed with KCSAN then KCSAN will probably complain. See also Documentation/dev-tools/kcsan.rst. Anyway, I'm fine with this patch. Thanks, Bart.
Hi Bart, Really appreciate your reminder. We will upload new patch to bypss KCSAN. Thank you very much. Peter On Tue, 2021-08-31 at 19:55 -0700, Bart Van Assche wrote: > On 8/31/21 19:29, Peter Wang wrote: > > REG_UFS_MTK_HW_VER is a read only mediatek dedicated register. > > So, hw_ver will get a const value for mediatek to decide how to use > > debug select. It only need read once, no need multi-threads > > protected. > > Hi Peter, > > The above can be concluded easily by a human but not by software. If > this code is analyzed with KCSAN then KCSAN will probably complain. > See > also Documentation/dev-tools/kcsan.rst. > > Anyway, I'm fine with this patch. > > Thanks, > > Bart.
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c index d2c2516..c7c18d8 100644 --- a/drivers/scsi/ufs/ufs-mediatek.c +++ b/drivers/scsi/ufs/ufs-mediatek.c @@ -296,6 +296,24 @@ static void ufs_mtk_setup_ref_clk_wait_us(struct ufs_hba *hba, host->ref_clk_ungating_wait_us = ungating_us; } +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 +323,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 +1019,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 "); } diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h index 3f0d3bb..10da6e3 100644 --- a/drivers/scsi/ufs/ufs-mediatek.h +++ b/drivers/scsi/ufs/ufs-mediatek.h @@ -18,6 +18,10 @@ #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