[PATCH 1/6] scsi: ufs: make ufshcd_is_{device_present,hba_active}() return bool

[PATCH 1/6] scsi: ufs: make ufshcd_is_{device_present,hba_active}() return bool

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

ufshcd driver generally uses bool for is_xxx type things instead of int,
so conform to its style.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 drivers/scsi/ufs/ufshcd.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b7e5128..b006f1e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -585,12 +585,12 @@ static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba)
  *			      the host controller
  * @hba: pointer to adapter instance
  *
- * Returns 1 if device present, 0 if no device detected
+ * Returns true if device present, false if no device detected
  */
-static inline int ufshcd_is_device_present(struct ufs_hba *hba)
+static inline bool ufshcd_is_device_present(struct ufs_hba *hba)
 {
 	return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) &
-						DEVICE_PRESENT) ? 1 : 0;
+						DEVICE_PRESENT) ? true : false;
 }
 
 /**
@@ -832,11 +832,11 @@ static inline void ufshcd_hba_start(struct ufs_hba *hba)
  * ufshcd_is_hba_active - Get controller state
  * @hba: per adapter instance
  *
- * Returns zero if controller is active, 1 otherwise
+ * Returns false if controller is active, true otherwise
  */
-static inline int ufshcd_is_hba_active(struct ufs_hba *hba)
+static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
 {
-	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
+	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? false : true;
 }
 
 static const char *ufschd_uic_link_state_to_string(
-- 
2.9.3

[PATCH 2/6] scsi: ufs: use existing macro CONTROLLER_ENABLE to test register bit

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

(Note this commit directly goes on top of the previous one)

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 drivers/scsi/ufs/ufshcd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b006f1e..dd46259 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -836,7 +836,8 @@ static inline void ufshcd_hba_start(struct ufs_hba *hba)
  */
 static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
 {
-	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? false : true;
+	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & CONTROLLER_ENABLE)
+		? false : true;
 }
 
 static const char *ufschd_uic_link_state_to_string(
-- 
2.9.3

Re: [PATCH 2/6] scsi: ufs: use existing macro CONTROLLER_ENABLE to test register bit

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> (Note this commit directly goes on top of the previous one)
> 
> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b006f1e..dd46259 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -836,7 +836,8 @@ static inline void ufshcd_hba_start(struct ufs_hba 
> *hba)
>   */
>  static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
>  {
> -	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? false : 
> true;
> +	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & CONTROLLER_ENABLE)
> +		? false : true;
>  }
> 
>  static const char *ufschd_uic_link_state_to_string(

Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 3/6] scsi: ufs: non functional macro fix

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

Not having () isn't likely to do any harm in this case, but all the
other macros below do have it. Also add "are" in a comment.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 drivers/scsi/ufs/ufshcd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index dd46259..089b76f 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -151,11 +151,11 @@ enum {
 };
 
 #define ufshcd_set_eh_in_progress(h) \
-	(h->eh_flags |= UFSHCD_EH_IN_PROGRESS)
+	((h)->eh_flags |= UFSHCD_EH_IN_PROGRESS)
 #define ufshcd_eh_in_progress(h) \
-	(h->eh_flags & UFSHCD_EH_IN_PROGRESS)
+	((h)->eh_flags & UFSHCD_EH_IN_PROGRESS)
 #define ufshcd_clear_eh_in_progress(h) \
-	(h->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)
+	((h)->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)
 
 #define ufshcd_set_ufs_dev_active(h) \
 	((h)->curr_dev_pwr_mode = UFS_ACTIVE_PWR_MODE)
@@ -1491,7 +1491,7 @@ int ufshcd_hold(struct ufs_hba *hba, bool async)
 			break;
 		}
 		/*
-		 * If we here, it means gating work is either done or
+		 * If we are here, it means gating work is either done or
 		 * currently running. Hence, fall through to cancel gating
 		 * work and to enable clocks.
 		 */
-- 
2.9.3

Re: [PATCH 3/6] scsi: ufs: non functional macro fix

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> Not having () isn't likely to do any harm in this case, but all the
> other macros below do have it. Also add "are" in a comment.
> 
> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index dd46259..089b76f 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -151,11 +151,11 @@ enum {
>  };
> 
>  #define ufshcd_set_eh_in_progress(h) \
> -	(h->eh_flags |= UFSHCD_EH_IN_PROGRESS)
> +	((h)->eh_flags |= UFSHCD_EH_IN_PROGRESS)
>  #define ufshcd_eh_in_progress(h) \
> -	(h->eh_flags & UFSHCD_EH_IN_PROGRESS)
> +	((h)->eh_flags & UFSHCD_EH_IN_PROGRESS)
>  #define ufshcd_clear_eh_in_progress(h) \
> -	(h->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)
> +	((h)->eh_flags &= ~UFSHCD_EH_IN_PROGRESS)
> 
>  #define ufshcd_set_ufs_dev_active(h) \
>  	((h)->curr_dev_pwr_mode = UFS_ACTIVE_PWR_MODE)
> @@ -1491,7 +1491,7 @@ int ufshcd_hold(struct ufs_hba *hba, bool async)
>  			break;
>  		}
>  		/*
> -		 * If we here, it means gating work is either done or
> +		 * If we are here, it means gating work is either done or
>  		 * currently running. Hence, fall through to cancel gating
>  		 * work and to enable clocks.
>  		 */

Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 4/6] scsi: ufs: add missing macros for register bits from UFSHCI spec

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

Add macros for register bits that can be found in JESD223C (v2.1).

Not all registers are defined in ufshci.h (i.e. some are unused
whether macros are defined or undefined), but all the bits for
those registers that are already defined should appear here.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 drivers/scsi/ufs/ufshci.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index d14e9b9..88acfd3 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -48,6 +48,7 @@ enum {
 	REG_UFS_VERSION				= 0x08,
 	REG_CONTROLLER_DEV_ID			= 0x10,
 	REG_CONTROLLER_PROD_ID			= 0x14,
+	REG_AUTO_HIBERNATE_IDLE_TIMER		= 0x18,
 	REG_INTERRUPT_STATUS			= 0x20,
 	REG_INTERRUPT_ENABLE			= 0x24,
 	REG_CONTROLLER_STATUS			= 0x30,
@@ -171,6 +172,7 @@ enum {
 /* HCE - Host Controller Enable 34h */
 #define CONTROLLER_ENABLE	UFS_BIT(0)
 #define CONTROLLER_DISABLE	0x0
+#define CRYPTO_GENERAL_ENABLE	UFS_BIT(1)
 
 /* UECPA - Host UIC Error Code PHY Adapter Layer 38h */
 #define UIC_PHY_ADAPTER_LAYER_ERROR			UFS_BIT(31)
-- 
2.9.3

Re: [PATCH 4/6] scsi: ufs: add missing macros for register bits from UFSHCI spec

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> Add macros for register bits that can be found in JESD223C (v2.1).
> 
> Not all registers are defined in ufshci.h (i.e. some are unused
> whether macros are defined or undefined), but all the bits for
> those registers that are already defined should appear here.
> 
> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
> ---
>  drivers/scsi/ufs/ufshci.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
> index d14e9b9..88acfd3 100644
> --- a/drivers/scsi/ufs/ufshci.h
> +++ b/drivers/scsi/ufs/ufshci.h
> @@ -48,6 +48,7 @@ enum {
>  	REG_UFS_VERSION				= 0x08,
>  	REG_CONTROLLER_DEV_ID			= 0x10,
>  	REG_CONTROLLER_PROD_ID			= 0x14,
> +	REG_AUTO_HIBERNATE_IDLE_TIMER		= 0x18,
>  	REG_INTERRUPT_STATUS			= 0x20,
>  	REG_INTERRUPT_ENABLE			= 0x24,
>  	REG_CONTROLLER_STATUS			= 0x30,
> @@ -171,6 +172,7 @@ enum {
>  /* HCE - Host Controller Enable 34h */
>  #define CONTROLLER_ENABLE	UFS_BIT(0)
>  #define CONTROLLER_DISABLE	0x0
> +#define CRYPTO_GENERAL_ENABLE	UFS_BIT(1)
> 
>  /* UECPA - Host UIC Error Code PHY Adapter Layer 38h */
>  #define UIC_PHY_ADAPTER_LAYER_ERROR			UFS_BIT(31)

Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 5/6] scsi: ufs: remove deprecated enum for hw interrupt

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

These flags are no longer needed after 2fbd009b in 2013.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 drivers/scsi/ufs/ufshcd.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 089b76f..109a762 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -143,13 +143,6 @@ enum {
 	UFSHCD_UIC_DME_ERROR = (1 << 5), /* DME error */
 };
 
-/* Interrupt configuration options */
-enum {
-	UFSHCD_INT_DISABLE,
-	UFSHCD_INT_ENABLE,
-	UFSHCD_INT_CLEAR,
-};
-
 #define ufshcd_set_eh_in_progress(h) \
 	((h)->eh_flags |= UFSHCD_EH_IN_PROGRESS)
 #define ufshcd_eh_in_progress(h) \
-- 
2.9.3

Re: [PATCH 5/6] scsi: ufs: remove deprecated enum for hw interrupt

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> These flags are no longer needed after 2fbd009b in 2013.
> 
> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 089b76f..109a762 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -143,13 +143,6 @@ enum {
>  	UFSHCD_UIC_DME_ERROR = (1 << 5), /* DME error */
>  };
> 
> -/* Interrupt configuration options */
> -enum {
> -	UFSHCD_INT_DISABLE,
> -	UFSHCD_INT_ENABLE,
> -	UFSHCD_INT_CLEAR,
> -};
> -
>  #define ufshcd_set_eh_in_progress(h) \
>  	((h)->eh_flags |= UFSHCD_EH_IN_PROGRESS)
>  #define ufshcd_eh_in_progress(h) \


Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 6/6] scsi: ufs: just use sizeof() for snprintf()

[kusumi.tomohiro]

From: Tomohiro Kusumi <tkusumi@tuxera.com>

Not much reason to use ARRAY_SIZE() when we know it's for a C string.

Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
---
 drivers/scsi/ufs/ufshcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 109a762..790c19c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7880,7 +7880,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 		INIT_WORK(&hba->clk_scaling.resume_work,
 			  ufshcd_clk_scaling_resume_work);
 
-		snprintf(wq_name, ARRAY_SIZE(wq_name), "ufs_clkscaling_%d",
+		snprintf(wq_name, sizeof(wq_name), "ufs_clkscaling_%d",
 			 host->host_no);
 		hba->clk_scaling.workq = create_singlethread_workqueue(wq_name);
 
-- 
2.9.3

Re: [PATCH 6/6] scsi: ufs: just use sizeof() for snprintf()

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> Not much reason to use ARRAY_SIZE() when we know it's for a C string.
> 
> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 109a762..790c19c 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7880,7 +7880,7 @@ int ufshcd_init(struct ufs_hba *hba, void
> __iomem *mmio_base, unsigned int irq)
>  		INIT_WORK(&hba->clk_scaling.resume_work,
>  			  ufshcd_clk_scaling_resume_work);
> 
> -		snprintf(wq_name, ARRAY_SIZE(wq_name), "ufs_clkscaling_%d",
> +		snprintf(wq_name, sizeof(wq_name), "ufs_clkscaling_%d",
>  			 host->host_no);
>  		hba->clk_scaling.workq = create_singlethread_workqueue(wq_name);


Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/6] scsi: ufs: make ufshcd_is_{device_present,hba_active}() return bool

[Subhash Jadavani]

On 2017-03-28 06:49, kusumi.tomohiro@gmail.com wrote:
> From: Tomohiro Kusumi <tkusumi@tuxera.com>
> 
> ufshcd driver generally uses bool for is_xxx type things instead of 
> int,
> so conform to its style.
> 
> Signed-off-by: Tomohiro Kusumi <tkusumi@tuxera.com>
> ---
>  drivers/scsi/ufs/ufshcd.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b7e5128..b006f1e 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -585,12 +585,12 @@ static inline u32 ufshcd_get_ufs_version(struct
> ufs_hba *hba)
>   *			      the host controller
>   * @hba: pointer to adapter instance
>   *
> - * Returns 1 if device present, 0 if no device detected
> + * Returns true if device present, false if no device detected
>   */
> -static inline int ufshcd_is_device_present(struct ufs_hba *hba)
> +static inline bool ufshcd_is_device_present(struct ufs_hba *hba)
>  {
>  	return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) &
> -						DEVICE_PRESENT) ? 1 : 0;
> +						DEVICE_PRESENT) ? true : false;
>  }
> 
>  /**
> @@ -832,11 +832,11 @@ static inline void ufshcd_hba_start(struct 
> ufs_hba *hba)
>   * ufshcd_is_hba_active - Get controller state
>   * @hba: per adapter instance
>   *
> - * Returns zero if controller is active, 1 otherwise
> + * Returns false if controller is active, true otherwise
>   */
> -static inline int ufshcd_is_hba_active(struct ufs_hba *hba)
> +static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
>  {
> -	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? 0 : 1;
> +	return (ufshcd_readl(hba, REG_CONTROLLER_ENABLE) & 0x1) ? false : 
> true;
>  }
> 
>  static const char *ufschd_uic_link_state_to_string(

Looks good to me.
Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/6] scsi: ufs: make ufshcd_is_{device_present,hba_active}() return bool

[Martin K. Petersen]

kusumi.tomohiro@gmail.com writes:

> ufshcd driver generally uses bool for is_xxx type things instead of int,
> so conform to its style.

Applied patches 1-6 to 4.12/scsi-queue.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering