All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH resend] scsi: ufs: ufshcd_dump_regs to use memcpy_fromio
@ 2018-06-14  8:14 Tomas Winkler
  2018-06-14 14:48 ` Avri Altman
  2018-06-19  2:20 ` Martin K. Petersen
  0 siblings, 2 replies; 3+ messages in thread
From: Tomas Winkler @ 2018-06-14  8:14 UTC (permalink / raw)
  To: James E . J . Bottomley, Martin K . Petersen, Vinayak Holikatti,
	Adrian Hunter
  Cc: Christoph Hellwig, Vivek Gautam, Subhash Jadavani, Alex Lemberg,
	Avri Altman, linux-scsi, linux-kernel, Tomas Winkler

ufshcd_dump_regs should use memcpy_fromio to read host registers
instead of directly accessing using memcpy.
The same function is utilized in ufs-qcom.

Elminite compilation warning
drivers/scsi/ufs/ufshcd.c:356:9: warning: incorrect type in argument 6 (different address spaces)
drivers/scsi/ufs/ufshcd.c:356:9:    expected void const *buf
drivers/scsi/ufs/ufshcd.c:356:9:    got void [noderef] <asn:2>*mmio_base

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 drivers/scsi/ufs/ufs-qcom.c | 21 ++++++---------------
 drivers/scsi/ufs/ufshcd.c   | 35 ++++++++++++++++++++++++-----------
 drivers/scsi/ufs/ufshcd.h   |  3 +++
 3 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 2b38db2eeafa..77ac98ea80d4 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -50,19 +50,10 @@ static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host);
 static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
 						       u32 clk_cycles);
 
-static void ufs_qcom_dump_regs(struct ufs_hba *hba, int offset, int len,
-		char *prefix)
-{
-	print_hex_dump(KERN_ERR, prefix,
-			len > 4 ? DUMP_PREFIX_OFFSET : DUMP_PREFIX_NONE,
-			16, 4, (void __force *)hba->mmio_base + offset,
-			len * 4, false);
-}
-
 static void ufs_qcom_dump_regs_wrapper(struct ufs_hba *hba, int offset, int len,
-		char *prefix, void *priv)
+				       const char *prefix, void *priv)
 {
-	ufs_qcom_dump_regs(hba, offset, len, prefix);
+	ufshcd_dump_regs(hba, offset, len * 4, prefix);
 }
 
 static int ufs_qcom_get_connected_tx_lanes(struct ufs_hba *hba, u32 *tx_lanes)
@@ -1431,7 +1422,7 @@ static int ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
 
 static void ufs_qcom_print_hw_debug_reg_all(struct ufs_hba *hba,
 		void *priv, void (*print_fn)(struct ufs_hba *hba,
-		int offset, int num_regs, char *str, void *priv))
+		int offset, int num_regs, const char *str, void *priv))
 {
 	u32 reg;
 	struct ufs_qcom_host *host;
@@ -1613,7 +1604,7 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host)
 
 static void ufs_qcom_testbus_read(struct ufs_hba *hba)
 {
-	ufs_qcom_dump_regs(hba, UFS_TEST_BUS, 1, "UFS_TEST_BUS ");
+	ufshcd_dump_regs(hba, UFS_TEST_BUS, 4, "UFS_TEST_BUS ");
 }
 
 static void ufs_qcom_print_unipro_testbus(struct ufs_hba *hba)
@@ -1639,8 +1630,8 @@ static void ufs_qcom_print_unipro_testbus(struct ufs_hba *hba)
 
 static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)
 {
-	ufs_qcom_dump_regs(hba, REG_UFS_SYS1CLK_1US, 16,
-			"HCI Vendor Specific Registers ");
+	ufshcd_dump_regs(hba, REG_UFS_SYS1CLK_1US, 16 * 4,
+			 "HCI Vendor Specific Registers ");
 
 	/* sleep a bit intermittently as we are dumping too much data */
 	ufs_qcom_print_hw_debug_reg_all(hba, NULL, ufs_qcom_dump_regs_wrapper);
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index d0a1674915a1..972fa85e2b24 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -99,8 +99,29 @@
 		_ret;                                                   \
 	})
 
-#define ufshcd_hex_dump(prefix_str, buf, len) \
-print_hex_dump(KERN_ERR, prefix_str, DUMP_PREFIX_OFFSET, 16, 4, buf, len, false)
+#define ufshcd_hex_dump(prefix_str, buf, len) do {                       \
+	size_t __len = (len);                                            \
+	print_hex_dump(KERN_ERR, prefix_str,                             \
+		       __len > 4 ? DUMP_PREFIX_OFFSET : DUMP_PREFIX_NONE,\
+		       16, 4, buf, __len, false);                        \
+} while (0)
+
+int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
+		     const char *prefix)
+{
+	u8 *regs;
+
+	regs = kzalloc(len, GFP_KERNEL);
+	if (!regs)
+		return -ENOMEM;
+
+	memcpy_fromio(regs, hba->mmio_base + offset, len);
+	ufshcd_hex_dump(prefix, regs, len);
+	kfree(regs);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ufshcd_dump_regs);
 
 enum {
 	UFSHCD_MAX_CHANNEL	= 0,
@@ -376,15 +397,7 @@ static void ufshcd_print_uic_err_hist(struct ufs_hba *hba,
 
 static void ufshcd_print_host_regs(struct ufs_hba *hba)
 {
-	/*
-	 * hex_dump reads its data without the readl macro. This might
-	 * cause inconsistency issues on some platform, as the printed
-	 * values may be from cache and not the most recent value.
-	 * To know whether you are looking at an un-cached version verify
-	 * that IORESOURCE_MEM flag is on when xxx_get_resource() is invoked
-	 * during platform/pci probe function.
-	 */
-	ufshcd_hex_dump("host regs: ", hba->mmio_base, UFSHCI_REG_SPACE_SIZE);
+	ufshcd_dump_regs(hba, 0, UFSHCI_REG_SPACE_SIZE, "host_regs: ");
 	dev_err(hba->dev, "hba->ufs_version = 0x%x, hba->capabilities = 0x%x\n",
 		hba->ufs_version, hba->capabilities);
 	dev_err(hba->dev,
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 8110dcd04d22..aad959ba3d9b 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -1022,4 +1022,7 @@ static inline u8 ufshcd_scsi_to_upiu_lun(unsigned int scsi_lun)
 		return scsi_lun & UFS_UPIU_MAX_UNIT_NUM_ID;
 }
 
+int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
+		     const char *prefix);
+
 #endif /* End of Header */
-- 
2.14.4


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

* RE: [PATCH resend] scsi: ufs: ufshcd_dump_regs to use memcpy_fromio
  2018-06-14  8:14 [PATCH resend] scsi: ufs: ufshcd_dump_regs to use memcpy_fromio Tomas Winkler
@ 2018-06-14 14:48 ` Avri Altman
  2018-06-19  2:20 ` Martin K. Petersen
  1 sibling, 0 replies; 3+ messages in thread
From: Avri Altman @ 2018-06-14 14:48 UTC (permalink / raw)
  To: Tomas Winkler, James E . J . Bottomley, Martin K . Petersen,
	Vinayak Holikatti, Adrian Hunter
  Cc: Christoph Hellwig, Vivek Gautam, Subhash Jadavani, Alex Lemberg,
	linux-scsi, linux-kernel



> -----Original Message-----
> From: Tomas Winkler <tomas.winkler@intel.com>
> Sent: Thursday, June 14, 2018 11:14 AM
> To: James E . J . Bottomley <jejb@linux.vnet.ibm.com>; Martin K . Petersen
> <martin.petersen@oracle.com>; Vinayak Holikatti
> <vinholikatti@gmail.com>; Adrian Hunter <adrian.hunter@intel.com>
> Cc: Christoph Hellwig <hch@lst.de>; Vivek Gautam
> <vivek.gautam@codeaurora.org>; Subhash Jadavani
> <subhashj@codeaurora.org>; Alex Lemberg <Alex.Lemberg@wdc.com>; Avri
> Altman <Avri.Altman@wdc.com>; linux-scsi@vger.kernel.org; linux-
> kernel@vger.kernel.org; Tomas Winkler <tomas.winkler@intel.com>
> Subject: [PATCH resend] scsi: ufs: ufshcd_dump_regs to use memcpy_fromio
> 
> ufshcd_dump_regs should use memcpy_fromio to read host registers instead
> of directly accessing using memcpy.
> The same function is utilized in ufs-qcom.
> 
> Elminite compilation warning
> drivers/scsi/ufs/ufshcd.c:356:9: warning: incorrect type in argument 6
> (different address spaces)
> drivers/scsi/ufs/ufshcd.c:356:9:    expected void const *buf
> drivers/scsi/ufs/ufshcd.c:356:9:    got void [noderef] <asn:2>*mmio_base
> 
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

> ---
>  drivers/scsi/ufs/ufs-qcom.c | 21 ++++++---------------
>  drivers/scsi/ufs/ufshcd.c   | 35 ++++++++++++++++++++++++-----------
>  drivers/scsi/ufs/ufshcd.h   |  3 +++
>  3 files changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index
> 2b38db2eeafa..77ac98ea80d4 100644
> --- a/drivers/scsi/ufs/ufs-qcom.c
> +++ b/drivers/scsi/ufs/ufs-qcom.c
> @@ -50,19 +50,10 @@ static void ufs_qcom_get_default_testbus_cfg(struct
> ufs_qcom_host *host);  static int
> ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
>  						       u32 clk_cycles);
> 
> -static void ufs_qcom_dump_regs(struct ufs_hba *hba, int offset, int len,
> -		char *prefix)
> -{
> -	print_hex_dump(KERN_ERR, prefix,
> -			len > 4 ? DUMP_PREFIX_OFFSET :
> DUMP_PREFIX_NONE,
> -			16, 4, (void __force *)hba->mmio_base + offset,
> -			len * 4, false);
> -}
> -
>  static void ufs_qcom_dump_regs_wrapper(struct ufs_hba *hba, int offset,
> int len,
> -		char *prefix, void *priv)
> +				       const char *prefix, void *priv)
>  {
> -	ufs_qcom_dump_regs(hba, offset, len, prefix);
> +	ufshcd_dump_regs(hba, offset, len * 4, prefix);
>  }
> 
>  static int ufs_qcom_get_connected_tx_lanes(struct ufs_hba *hba, u32
> *tx_lanes) @@ -1431,7 +1422,7 @@ static int
> ufs_qcom_clk_scale_notify(struct ufs_hba *hba,
> 
>  static void ufs_qcom_print_hw_debug_reg_all(struct ufs_hba *hba,
>  		void *priv, void (*print_fn)(struct ufs_hba *hba,
> -		int offset, int num_regs, char *str, void *priv))
> +		int offset, int num_regs, const char *str, void *priv))
>  {
>  	u32 reg;
>  	struct ufs_qcom_host *host;
> @@ -1613,7 +1604,7 @@ int ufs_qcom_testbus_config(struct
> ufs_qcom_host *host)
> 
>  static void ufs_qcom_testbus_read(struct ufs_hba *hba)  {
> -	ufs_qcom_dump_regs(hba, UFS_TEST_BUS, 1, "UFS_TEST_BUS ");
> +	ufshcd_dump_regs(hba, UFS_TEST_BUS, 4, "UFS_TEST_BUS ");
>  }
> 
>  static void ufs_qcom_print_unipro_testbus(struct ufs_hba *hba) @@ -
> 1639,8 +1630,8 @@ static void ufs_qcom_print_unipro_testbus(struct
> ufs_hba *hba)
> 
>  static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)  {
> -	ufs_qcom_dump_regs(hba, REG_UFS_SYS1CLK_1US, 16,
> -			"HCI Vendor Specific Registers ");
> +	ufshcd_dump_regs(hba, REG_UFS_SYS1CLK_1US, 16 * 4,
> +			 "HCI Vendor Specific Registers ");
> 
>  	/* sleep a bit intermittently as we are dumping too much data */
>  	ufs_qcom_print_hw_debug_reg_all(hba, NULL,
> ufs_qcom_dump_regs_wrapper); diff --git a/drivers/scsi/ufs/ufshcd.c
> b/drivers/scsi/ufs/ufshcd.c index d0a1674915a1..972fa85e2b24 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -99,8 +99,29 @@
>  		_ret;                                                   \
>  	})
> 
> -#define ufshcd_hex_dump(prefix_str, buf, len) \ -
> print_hex_dump(KERN_ERR, prefix_str, DUMP_PREFIX_OFFSET, 16, 4, buf,
> len, false)
> +#define ufshcd_hex_dump(prefix_str, buf, len) do {                       \
> +	size_t __len = (len);                                            \
> +	print_hex_dump(KERN_ERR, prefix_str,                             \
> +		       __len > 4 ? DUMP_PREFIX_OFFSET :
> DUMP_PREFIX_NONE,\
> +		       16, 4, buf, __len, false);                        \
> +} while (0)
> +
> +int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
> +		     const char *prefix)
> +{
> +	u8 *regs;
> +
> +	regs = kzalloc(len, GFP_KERNEL);
> +	if (!regs)
> +		return -ENOMEM;
> +
> +	memcpy_fromio(regs, hba->mmio_base + offset, len);
> +	ufshcd_hex_dump(prefix, regs, len);
> +	kfree(regs);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ufshcd_dump_regs);
> 
>  enum {
>  	UFSHCD_MAX_CHANNEL	= 0,
> @@ -376,15 +397,7 @@ static void ufshcd_print_uic_err_hist(struct ufs_hba
> *hba,
> 
>  static void ufshcd_print_host_regs(struct ufs_hba *hba)  {
> -	/*
> -	 * hex_dump reads its data without the readl macro. This might
> -	 * cause inconsistency issues on some platform, as the printed
> -	 * values may be from cache and not the most recent value.
> -	 * To know whether you are looking at an un-cached version verify
> -	 * that IORESOURCE_MEM flag is on when xxx_get_resource() is
> invoked
> -	 * during platform/pci probe function.
> -	 */
> -	ufshcd_hex_dump("host regs: ", hba->mmio_base,
> UFSHCI_REG_SPACE_SIZE);
> +	ufshcd_dump_regs(hba, 0, UFSHCI_REG_SPACE_SIZE, "host_regs: ");
>  	dev_err(hba->dev, "hba->ufs_version = 0x%x, hba->capabilities =
> 0x%x\n",
>  		hba->ufs_version, hba->capabilities);
>  	dev_err(hba->dev,
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index
> 8110dcd04d22..aad959ba3d9b 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -1022,4 +1022,7 @@ static inline u8 ufshcd_scsi_to_upiu_lun(unsigned
> int scsi_lun)
>  		return scsi_lun & UFS_UPIU_MAX_UNIT_NUM_ID;  }
> 
> +int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
> +		     const char *prefix);
> +
>  #endif /* End of Header */
> --
> 2.14.4


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

* Re: [PATCH resend] scsi: ufs: ufshcd_dump_regs to use memcpy_fromio
  2018-06-14  8:14 [PATCH resend] scsi: ufs: ufshcd_dump_regs to use memcpy_fromio Tomas Winkler
  2018-06-14 14:48 ` Avri Altman
@ 2018-06-19  2:20 ` Martin K. Petersen
  1 sibling, 0 replies; 3+ messages in thread
From: Martin K. Petersen @ 2018-06-19  2:20 UTC (permalink / raw)
  To: Tomas Winkler
  Cc: James E . J . Bottomley, Martin K . Petersen, Vinayak Holikatti,
	Adrian Hunter, Christoph Hellwig, Vivek Gautam, Subhash Jadavani,
	Alex Lemberg, Avri Altman, linux-scsi, linux-kernel


Tomas,

> ufshcd_dump_regs should use memcpy_fromio to read host registers
> instead of directly accessing using memcpy.  The same function is
> utilized in ufs-qcom.

Applied to 4.19/scsi-queue. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

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

end of thread, other threads:[~2018-06-19  2:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-14  8:14 [PATCH resend] scsi: ufs: ufshcd_dump_regs to use memcpy_fromio Tomas Winkler
2018-06-14 14:48 ` Avri Altman
2018-06-19  2:20 ` Martin K. Petersen

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.