All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] scsi: ufs: ufs-mediatek: Change dbg select by check hw version
@ 2021-09-01  6:04 ` peter.wang
  0 siblings, 0 replies; 6+ messages in thread
From: peter.wang @ 2021-09-01  6:04 UTC (permalink / raw)
  To: stanley.chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
  Cc: wsd_upstream, linux-mediatek, peter.wang, chun-hung.wu,
	alice.chao, cc.chou, chaotian.jing, jiajie.hao, powen.kao,
	jonathan.hsu, qilin.tan, lin.gui

From: Peter Wang <peter.wang@mediatek.com>

Mediatek UFS dbg select setting is changed in new HW version.
This patch check the HW version before set dbg select.

Signed-off-by: Peter Wang <peter.wang@mediatek.com>
---
 drivers/scsi/ufs/ufs-mediatek.c |   23 +++++++++++++++++++++--
 drivers/scsi/ufs/ufs-mediatek.h |    5 +++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index d2c2516..0050e01 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -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 ");
 }
 
diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h
index 3f0d3bb..fc40c05 100644
--- a/drivers/scsi/ufs/ufs-mediatek.h
+++ b/drivers/scsi/ufs/ufs-mediatek.h
@@ -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
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3] scsi: ufs: ufs-mediatek: Change dbg select by check hw version
@ 2021-09-01  6:04 ` peter.wang
  0 siblings, 0 replies; 6+ messages in thread
From: peter.wang @ 2021-09-01  6:04 UTC (permalink / raw)
  To: stanley.chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
  Cc: wsd_upstream, linux-mediatek, peter.wang, chun-hung.wu,
	alice.chao, cc.chou, chaotian.jing, jiajie.hao, powen.kao,
	jonathan.hsu, qilin.tan, lin.gui

From: Peter Wang <peter.wang@mediatek.com>

Mediatek UFS dbg select setting is changed in new HW version.
This patch check the HW version before set dbg select.

Signed-off-by: Peter Wang <peter.wang@mediatek.com>
---
 drivers/scsi/ufs/ufs-mediatek.c |   23 +++++++++++++++++++++--
 drivers/scsi/ufs/ufs-mediatek.h |    5 +++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index d2c2516..0050e01 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -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 ");
 }
 
diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h
index 3f0d3bb..fc40c05 100644
--- a/drivers/scsi/ufs/ufs-mediatek.h
+++ b/drivers/scsi/ufs/ufs-mediatek.h
@@ -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
-- 
1.7.9.5
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] scsi: ufs: ufs-mediatek: Change dbg select by check hw version
  2021-09-01  6:04 ` peter.wang
@ 2021-09-01  7:35   ` Stanley Chu
  -1 siblings, 0 replies; 6+ messages in thread
From: Stanley Chu @ 2021-09-01  7:35 UTC (permalink / raw)
  To: peter.wang, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
  Cc: wsd_upstream, linux-mediatek, chun-hung.wu, alice.chao, cc.chou,
	chaotian.jing, jiajie.hao, powen.kao, jonathan.hsu, qilin.tan,
	lin.gui

Hi Peter,

On Wed, 2021-09-01 at 14:04 +0800, peter.wang@mediatek.com wrote:
> From: Peter Wang <peter.wang@mediatek.com>
> 
> 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.
> 
> Signed-off-by: Peter Wang <peter.wang@mediatek.com>
> ---
>  drivers/scsi/ufs/ufs-mediatek.c |   23 +++++++++++++++++++++--
>  drivers/scsi/ufs/ufs-mediatek.h |    5 +++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-
> mediatek.c
> index d2c2516..0050e01 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.c
> +++ b/drivers/scsi/ufs/ufs-mediatek.c
> @@ -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?

> +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.

> +
> +	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 ");
>  }
>  
> diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-
> mediatek.h
> index 3f0d3bb..fc40c05 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.h
> +++ b/drivers/scsi/ufs/ufs-mediatek.h
> @@ -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?

>  #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

>  
>  /*
>   * Ref-clk control

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] scsi: ufs: ufs-mediatek: Change dbg select by check hw version
@ 2021-09-01  7:35   ` Stanley Chu
  0 siblings, 0 replies; 6+ messages in thread
From: Stanley Chu @ 2021-09-01  7:35 UTC (permalink / raw)
  To: peter.wang, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
  Cc: wsd_upstream, linux-mediatek, chun-hung.wu, alice.chao, cc.chou,
	chaotian.jing, jiajie.hao, powen.kao, jonathan.hsu, qilin.tan,
	lin.gui

Hi Peter,

On Wed, 2021-09-01 at 14:04 +0800, peter.wang@mediatek.com wrote:
> From: Peter Wang <peter.wang@mediatek.com>
> 
> 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.
> 
> Signed-off-by: Peter Wang <peter.wang@mediatek.com>
> ---
>  drivers/scsi/ufs/ufs-mediatek.c |   23 +++++++++++++++++++++--
>  drivers/scsi/ufs/ufs-mediatek.h |    5 +++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-
> mediatek.c
> index d2c2516..0050e01 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.c
> +++ b/drivers/scsi/ufs/ufs-mediatek.c
> @@ -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?

> +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.

> +
> +	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 ");
>  }
>  
> diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-
> mediatek.h
> index 3f0d3bb..fc40c05 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.h
> +++ b/drivers/scsi/ufs/ufs-mediatek.h
> @@ -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?

>  #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

>  
>  /*
>   * Ref-clk control
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] scsi: ufs: ufs-mediatek: Change dbg select by check hw version
  2021-09-01  7:35   ` Stanley Chu
@ 2021-09-02  2:12     ` Peter Wang
  -1 siblings, 0 replies; 6+ messages in thread
From: Peter Wang @ 2021-09-02  2:12 UTC (permalink / raw)
  To: Stanley Chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
  Cc: wsd_upstream, linux-mediatek, chun-hung.wu, alice.chao, cc.chou,
	chaotian.jing, jiajie.hao, powen.kao, jonathan.hsu, qilin.tan,
	lin.gui

On Wed, 2021-09-01 at 15:35 +0800, Stanley Chu wrote:
> Hi Peter,
> 
> On Wed, 2021-09-01 at 14:04 +0800, peter.wang@mediatek.com wrote:
> > From: Peter Wang <peter.wang@mediatek.com>
> > 
> > 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.
> > 
> > Signed-off-by: Peter Wang <peter.wang@mediatek.com>
> > ---
> >  drivers/scsi/ufs/ufs-mediatek.c |   23 +++++++++++++++++++++--
> >  drivers/scsi/ufs/ufs-mediatek.h |    5 +++++
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/scsi/ufs/ufs-mediatek.c
> > b/drivers/scsi/ufs/ufs-
> > mediatek.c
> > index d2c2516..0050e01 100644
> > --- a/drivers/scsi/ufs/ufs-mediatek.c
> > +++ b/drivers/scsi/ufs/ufs-mediatek.c
> > @@ -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?
> 

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.

> > +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.
> 
> > +
> > +	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 ");
> >  }
> >  
> > diff --git a/drivers/scsi/ufs/ufs-mediatek.h
> > b/drivers/scsi/ufs/ufs-
> > mediatek.h
> > index 3f0d3bb..fc40c05 100644
> > --- a/drivers/scsi/ufs/ufs-mediatek.h
> > +++ b/drivers/scsi/ufs/ufs-mediatek.h
> > @@ -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?
> 

Sure, could change naming for easy understand.

> >  #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
> 
> >  
> >  /*
> >   * Ref-clk control
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] scsi: ufs: ufs-mediatek: Change dbg select by check hw version
@ 2021-09-02  2:12     ` Peter Wang
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Wang @ 2021-09-02  2:12 UTC (permalink / raw)
  To: Stanley Chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
  Cc: wsd_upstream, linux-mediatek, chun-hung.wu, alice.chao, cc.chou,
	chaotian.jing, jiajie.hao, powen.kao, jonathan.hsu, qilin.tan,
	lin.gui

On Wed, 2021-09-01 at 15:35 +0800, Stanley Chu wrote:
> Hi Peter,
> 
> On Wed, 2021-09-01 at 14:04 +0800, peter.wang@mediatek.com wrote:
> > From: Peter Wang <peter.wang@mediatek.com>
> > 
> > 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.
> > 
> > Signed-off-by: Peter Wang <peter.wang@mediatek.com>
> > ---
> >  drivers/scsi/ufs/ufs-mediatek.c |   23 +++++++++++++++++++++--
> >  drivers/scsi/ufs/ufs-mediatek.h |    5 +++++
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/scsi/ufs/ufs-mediatek.c
> > b/drivers/scsi/ufs/ufs-
> > mediatek.c
> > index d2c2516..0050e01 100644
> > --- a/drivers/scsi/ufs/ufs-mediatek.c
> > +++ b/drivers/scsi/ufs/ufs-mediatek.c
> > @@ -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?
> 

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.

> > +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.
> 
> > +
> > +	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 ");
> >  }
> >  
> > diff --git a/drivers/scsi/ufs/ufs-mediatek.h
> > b/drivers/scsi/ufs/ufs-
> > mediatek.h
> > index 3f0d3bb..fc40c05 100644
> > --- a/drivers/scsi/ufs/ufs-mediatek.h
> > +++ b/drivers/scsi/ufs/ufs-mediatek.h
> > @@ -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?
> 

Sure, could change naming for easy understand.

> >  #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
> 
> >  
> >  /*
> >   * Ref-clk control
> 
> 
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-02  2:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-01  6:04 [PATCH v3] scsi: ufs: ufs-mediatek: Change dbg select by check hw version peter.wang
2021-09-01  6:04 ` peter.wang
2021-09-01  7:35 ` Stanley Chu
2021-09-01  7:35   ` Stanley Chu
2021-09-02  2:12   ` Peter Wang
2021-09-02  2:12     ` Peter Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.