[PATCH v1 1/9] scsi: ufs: add support to allow non standard behaviours (quirks)

[PATCH v1 1/9] scsi: ufs: add support to allow non standard behaviours (quirks)

[Asutosh Das]

From: Subhash Jadavani <subhashj@codeaurora.org>

Some implementation of UFS host controller HW might have some
non-standard behaviours (quirks) when compared to behaviour
specified by UFSHCI specification. This patch adds a quirk
for broken hibernate support.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index f51758f..e996a08 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -613,6 +613,9 @@ struct ufs_hba {
 	 * enabled via HCE register.
 	 */
 	#define UFSHCI_QUIRK_BROKEN_HCE				0x400
+
+	/* HIBERN8 support is broken */
+	#define UFSHCD_QUIRK_BROKEN_HIBERN8			0x800
 	unsigned int quirks;	/* Deviations from standard UFSHCI spec. */
 
 	/* Device deviations from standard UFS device spec. */
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v1 2/9] scsi: Allow auto suspend override by low-level driver

[Asutosh Das]

From: Sujit Reddy Thumma <sthumma@codeaurora.org>

Until now the scsi mid-layer forbids runtime suspend till userspace
enables it. This is mainly to quarantine some disks with broken
runtime power management or have high latencies executing suspend
resume callbacks. If the userspace doesn't enable the runtime suspend
the underlying hardware will be always on even when it is not doing
any useful work and thus wasting power.

Some low-level drivers for the controllers can efficiently use runtime
power management to reduce power consumption and improve battery life.
Allow runtime suspend parameters override within the LLD itself
instead of waiting for userspace to control the power management.

Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/scsi_scan.c   | 4 ++++
 drivers/scsi/scsi_sysfs.c  | 3 ++-
 drivers/scsi/sd.c          | 2 ++
 include/scsi/scsi_device.h | 3 +++
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 0880d97..5b7232a 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -969,6 +969,10 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 
 	transport_configure_device(&sdev->sdev_gendev);
 
+	/* The LLD can override auto suspend tunables in ->slave_configure() */
+	sdev->use_rpm_auto = 0;
+	sdev->autosuspend_delay = SCSI_DEFAULT_AUTOSUSPEND_DELAY;
+
 	if (sdev->host->hostt->slave_configure) {
 		ret = sdev->host->hostt->slave_configure(sdev);
 		if (ret) {
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 7943b76..706e778 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1266,7 +1266,8 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 	device_enable_async_suspend(&sdev->sdev_gendev);
 	scsi_autopm_get_target(starget);
 	pm_runtime_set_active(&sdev->sdev_gendev);
-	pm_runtime_forbid(&sdev->sdev_gendev);
+	if (!sdev->use_rpm_auto)
+		pm_runtime_forbid(&sdev->sdev_gendev);
 	pm_runtime_enable(&sdev->sdev_gendev);
 	scsi_autopm_put_target(starget);
 
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a6201e6..205624f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3269,6 +3269,8 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
 	}
 
 	blk_pm_runtime_init(sdp->request_queue, dev);
+	if (sdp->autosuspend_delay >= 0)
+		pm_runtime_set_autosuspend_delay(dev, sdp->autosuspend_delay);
 	device_add_disk(dev, gd);
 	if (sdkp->capacity)
 		sd_dif_config_host(sdkp);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 4c36af6..1d5ae90 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -197,7 +197,10 @@ struct scsi_device {
 	unsigned broken_fua:1;		/* Don't set FUA bit */
 	unsigned lun_in_cdb:1;		/* Store LUN bits in CDB[1] */
 	unsigned unmap_limit_for_ws:1;	/* Use the UNMAP limit for WRITE SAME */
+	unsigned use_rpm_auto:1; /* Enable runtime PM auto suspend */
 
+#define SCSI_DEFAULT_AUTOSUSPEND_DELAY  -1
+	int autosuspend_delay;
 	atomic_t disk_events_disable_depth; /* disable depth for disk events */
 
 	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v1 2/9] scsi: Allow auto suspend override by low-level driver

[Adrian Hunter]

On 06/07/18 15:30, Asutosh Das wrote:
> From: Sujit Reddy Thumma <sthumma@codeaurora.org>
> 
> Until now the scsi mid-layer forbids runtime suspend till userspace
> enables it. This is mainly to quarantine some disks with broken
> runtime power management or have high latencies executing suspend
> resume callbacks. If the userspace doesn't enable the runtime suspend
> the underlying hardware will be always on even when it is not doing
> any useful work and thus wasting power.
> 
> Some low-level drivers for the controllers can efficiently use runtime
> power management to reduce power consumption and improve battery life.
> Allow runtime suspend parameters override within the LLD itself
> instead of waiting for userspace to control the power management.
> 
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>

Notwithstanding one minor comment below:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/scsi/scsi_scan.c   | 4 ++++
>  drivers/scsi/scsi_sysfs.c  | 3 ++-
>  drivers/scsi/sd.c          | 2 ++
>  include/scsi/scsi_device.h | 3 +++
>  4 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 0880d97..5b7232a 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -969,6 +969,10 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
>  
>  	transport_configure_device(&sdev->sdev_gendev);
>  
> +	/* The LLD can override auto suspend tunables in ->slave_configure() */
> +	sdev->use_rpm_auto = 0;
> +	sdev->autosuspend_delay = SCSI_DEFAULT_AUTOSUSPEND_DELAY;
> +
>  	if (sdev->host->hostt->slave_configure) {
>  		ret = sdev->host->hostt->slave_configure(sdev);
>  		if (ret) {
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 7943b76..706e778 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1266,7 +1266,8 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
>  	device_enable_async_suspend(&sdev->sdev_gendev);
>  	scsi_autopm_get_target(starget);
>  	pm_runtime_set_active(&sdev->sdev_gendev);
> -	pm_runtime_forbid(&sdev->sdev_gendev);
> +	if (!sdev->use_rpm_auto)
> +		pm_runtime_forbid(&sdev->sdev_gendev);
>  	pm_runtime_enable(&sdev->sdev_gendev);
>  	scsi_autopm_put_target(starget);
>  
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index a6201e6..205624f 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3269,6 +3269,8 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
>  	}
>  
>  	blk_pm_runtime_init(sdp->request_queue, dev);
> +	if (sdp->autosuspend_delay >= 0)
> +		pm_runtime_set_autosuspend_delay(dev, sdp->autosuspend_delay);

Wouldn't it be nicer to pass the autosuspend_delay into blk_pm_runtime_init()?

>  	device_add_disk(dev, gd);
>  	if (sdkp->capacity)
>  		sd_dif_config_host(sdkp);
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 4c36af6..1d5ae90 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -197,7 +197,10 @@ struct scsi_device {
>  	unsigned broken_fua:1;		/* Don't set FUA bit */
>  	unsigned lun_in_cdb:1;		/* Store LUN bits in CDB[1] */
>  	unsigned unmap_limit_for_ws:1;	/* Use the UNMAP limit for WRITE SAME */
> +	unsigned use_rpm_auto:1; /* Enable runtime PM auto suspend */
>  
> +#define SCSI_DEFAULT_AUTOSUSPEND_DELAY  -1
> +	int autosuspend_delay;
>  	atomic_t disk_events_disable_depth; /* disable depth for disk events */
>  
>  	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
> 

Re: [PATCH v1 2/9] scsi: Allow auto suspend override by low-level driver

[Subhash Jadavani]

On 2018-07-06 05:30, Asutosh Das wrote:
> From: Sujit Reddy Thumma <sthumma@codeaurora.org>
> 
> Until now the scsi mid-layer forbids runtime suspend till userspace
> enables it. This is mainly to quarantine some disks with broken
> runtime power management or have high latencies executing suspend
> resume callbacks. If the userspace doesn't enable the runtime suspend
> the underlying hardware will be always on even when it is not doing
> any useful work and thus wasting power.
> 
> Some low-level drivers for the controllers can efficiently use runtime
> power management to reduce power consumption and improve battery life.
> Allow runtime suspend parameters override within the LLD itself
> instead of waiting for userspace to control the power management.
> 
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
>  drivers/scsi/scsi_scan.c   | 4 ++++
>  drivers/scsi/scsi_sysfs.c  | 3 ++-
>  drivers/scsi/sd.c          | 2 ++
>  include/scsi/scsi_device.h | 3 +++
>  4 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 0880d97..5b7232a 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -969,6 +969,10 @@ static int scsi_add_lun(struct scsi_device *sdev,
> unsigned char *inq_result,
> 
>  	transport_configure_device(&sdev->sdev_gendev);
> 
> +	/* The LLD can override auto suspend tunables in ->slave_configure() 
> */
> +	sdev->use_rpm_auto = 0;
> +	sdev->autosuspend_delay = SCSI_DEFAULT_AUTOSUSPEND_DELAY;
> +
>  	if (sdev->host->hostt->slave_configure) {
>  		ret = sdev->host->hostt->slave_configure(sdev);
>  		if (ret) {
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 7943b76..706e778 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1266,7 +1266,8 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
>  	device_enable_async_suspend(&sdev->sdev_gendev);
>  	scsi_autopm_get_target(starget);
>  	pm_runtime_set_active(&sdev->sdev_gendev);
> -	pm_runtime_forbid(&sdev->sdev_gendev);
> +	if (!sdev->use_rpm_auto)
> +		pm_runtime_forbid(&sdev->sdev_gendev);
>  	pm_runtime_enable(&sdev->sdev_gendev);
>  	scsi_autopm_put_target(starget);
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index a6201e6..205624f 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3269,6 +3269,8 @@ static void sd_probe_async(void *data,
> async_cookie_t cookie)
>  	}
> 
>  	blk_pm_runtime_init(sdp->request_queue, dev);
> +	if (sdp->autosuspend_delay >= 0)
> +		pm_runtime_set_autosuspend_delay(dev, sdp->autosuspend_delay);
>  	device_add_disk(dev, gd);
>  	if (sdkp->capacity)
>  		sd_dif_config_host(sdkp);
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 4c36af6..1d5ae90 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -197,7 +197,10 @@ struct scsi_device {
>  	unsigned broken_fua:1;		/* Don't set FUA bit */
>  	unsigned lun_in_cdb:1;		/* Store LUN bits in CDB[1] */
>  	unsigned unmap_limit_for_ws:1;	/* Use the UNMAP limit for WRITE SAME 
> */
> +	unsigned use_rpm_auto:1; /* Enable runtime PM auto suspend */
> 
> +#define SCSI_DEFAULT_AUTOSUSPEND_DELAY  -1
> +	int autosuspend_delay;
>  	atomic_t disk_events_disable_depth; /* disable depth for disk events 
> */
> 
>  	DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported 
> events */

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 v1 3/9] scsi: ufs: Override auto suspend tunables for ufs

[Asutosh Das]

From: Sujit Reddy Thumma <sthumma@codeaurora.org>

Override auto suspend tunables for UFS device LUNs during
initialization so as to efficiently manage background operations
and the power consumption.

Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
Signed-off-by: Gilad Broner <gbroner@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 77e2b3e..b03f3ea 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -89,6 +89,9 @@
 /* Interrupt aggregation default timeout, unit: 40us */
 #define INT_AGGR_DEF_TO	0x02
 
+/* default value of auto suspend is 3 seconds */
+#define UFSHCD_AUTO_SUSPEND_DELAY_MS 3000 /* millisecs */
+
 #define ufshcd_toggle_vreg(_dev, _vreg, _on)				\
 	({                                                              \
 		int _ret;                                               \
@@ -4528,6 +4531,9 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
 	blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
 	blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
 
+	sdev->autosuspend_delay = UFSHCD_AUTO_SUSPEND_DELAY_MS;
+	sdev->use_rpm_auto = 1;
+
 	return 0;
 }
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v1 3/9] scsi: ufs: Override auto suspend tunables for ufs

[Adrian Hunter]

On 06/07/18 15:30, Asutosh Das wrote:
> From: Sujit Reddy Thumma <sthumma@codeaurora.org>
> 
> Override auto suspend tunables for UFS device LUNs during
> initialization so as to efficiently manage background operations
> and the power consumption.
> 
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> Signed-off-by: Gilad Broner <gbroner@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/scsi/ufs/ufshcd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 77e2b3e..b03f3ea 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -89,6 +89,9 @@
>  /* Interrupt aggregation default timeout, unit: 40us */
>  #define INT_AGGR_DEF_TO	0x02
>  
> +/* default value of auto suspend is 3 seconds */
> +#define UFSHCD_AUTO_SUSPEND_DELAY_MS 3000 /* millisecs */
> +
>  #define ufshcd_toggle_vreg(_dev, _vreg, _on)				\
>  	({                                                              \
>  		int _ret;                                               \
> @@ -4528,6 +4531,9 @@ static int ufshcd_slave_configure(struct scsi_device *sdev)
>  	blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
>  	blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
>  
> +	sdev->autosuspend_delay = UFSHCD_AUTO_SUSPEND_DELAY_MS;
> +	sdev->use_rpm_auto = 1;
> +
>  	return 0;
>  }
>  
> 

Re: [PATCH v1 3/9] scsi: ufs: Override auto suspend tunables for ufs

[Subhash Jadavani]

On 2018-07-06 05:30, Asutosh Das wrote:
> From: Sujit Reddy Thumma <sthumma@codeaurora.org>
> 
> Override auto suspend tunables for UFS device LUNs during
> initialization so as to efficiently manage background operations
> and the power consumption.
> 
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> Signed-off-by: Gilad Broner <gbroner@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufshcd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 77e2b3e..b03f3ea 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -89,6 +89,9 @@
>  /* Interrupt aggregation default timeout, unit: 40us */
>  #define INT_AGGR_DEF_TO	0x02
> 
> +/* default value of auto suspend is 3 seconds */
> +#define UFSHCD_AUTO_SUSPEND_DELAY_MS 3000 /* millisecs */
> +
>  #define ufshcd_toggle_vreg(_dev, _vreg, _on)				\
>  	({                                                              \
>  		int _ret;                                               \
> @@ -4528,6 +4531,9 @@ static int ufshcd_slave_configure(struct
> scsi_device *sdev)
>  	blk_queue_update_dma_pad(q, PRDT_DATA_BYTE_COUNT_PAD - 1);
>  	blk_queue_max_segment_size(q, PRDT_DATA_BYTE_COUNT_MAX);
> 
> +	sdev->autosuspend_delay = UFSHCD_AUTO_SUSPEND_DELAY_MS;
> +	sdev->use_rpm_auto = 1;
> +
>  	return 0;
>  }

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 v1 4/9] scsi: ufs: add option to change default UFS power management level

[Asutosh Das]

From: Subhash Jadavani <subhashj@codeaurora.org>

UFS device and link can be put in multiple different low power modes hence
UFS driver supports multiple different low power modes. By default UFS
driver selects the default (optimal) low power mode (which gives moderate
power savings and have relatively less enter and exit latencies) but
we might have to tune this default power mode for different chipset
platforms to meet the low power requirements/goals. Hence this patch
adds option to change default UFS low power mode (level).

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      | 11 ++++++++
 drivers/scsi/ufs/ufshcd-pltfrm.c                   | 14 +++++++++++
 drivers/scsi/ufs/ufshcd.c                          | 29 +++++++++++++++-------
 drivers/scsi/ufs/ufshcd.h                          |  4 +--
 4 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
index c39dfef..f564d9a 100644
--- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
+++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
@@ -38,6 +38,15 @@ Optional properties:
 			  defined or a value in the array is "0" then it is assumed
 			  that the frequency is set by the parent clock or a
 			  fixed rate clock source.
+- rpm-level		: UFS Runtime power management level. Following PM levels are supported:
+			  0 - Both UFS device and Link in active state (Highest power consumption)
+			  1 - UFS device in active state but Link in Hibern8 state
+			  2 - UFS device in Sleep state but Link in active state
+			  3 - UFS device in Sleep state and Link in hibern8 state (default PM level)
+			  4 - UFS device in Power-down state and Link in Hibern8 state
+			  5 - UFS device in Power-down state and Link in OFF state (Lowest power consumption)
+- spm-level		: UFS System power management level. Allowed PM levels are same as rpm-level.
+
 -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
 			  Note that it is assume same number of lanes is used both
 			  directions at once. If not specified, default is 2 lanes per direction.
@@ -66,4 +75,6 @@ Example:
 		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
 		phys = <&ufsphy1>;
 		phy-names = "ufsphy";
+		rpm-level = <3>;
+		spm-level = <5>;
 	};
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index e82bde0..7dba799 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -221,6 +221,19 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
 	return err;
 }
 
+static void ufshcd_parse_pm_levels(struct ufs_hba *hba)
+{
+	struct device *dev = hba->dev;
+	struct device_node *np = dev->of_node;
+
+	if (np) {
+		if (of_property_read_u32(np, "rpm-level", &hba->rpm_lvl))
+			hba->rpm_lvl = -1;
+		if (of_property_read_u32(np, "spm-level", &hba->spm_lvl))
+			hba->spm_lvl = -1;
+	}
+}
+
 #ifdef CONFIG_PM
 /**
  * ufshcd_pltfrm_suspend - suspend power management function
@@ -340,6 +353,7 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 		goto dealloc_host;
 	}
 
+	ufshcd_parse_pm_levels(hba);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b03f3ea..e950204 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -192,6 +192,14 @@ struct ufs_pm_lvl_states ufs_pm_lvl_states[] = {
 	return UFS_PM_LVL_0;
 }
 
+static inline bool ufshcd_is_valid_pm_lvl(int lvl)
+{
+	if (lvl >= 0 && lvl < ARRAY_SIZE(ufs_pm_lvl_states))
+		return true;
+	else
+		return false;
+}
+
 static struct ufs_dev_fix ufs_fixups[] = {
 	/* UFS cards deviations table */
 	UFS_FIX(UFS_VENDOR_SAMSUNG, UFS_ANY_MODEL,
@@ -8051,16 +8059,19 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	}
 
 	/*
-	 * Set the default power management level for runtime and system PM.
-	 * Default power saving mode is to keep UFS link in Hibern8 state
-	 * and UFS device in sleep state.
+	 * If rpm_lvl and and spm_lvl are not already set to valid levels,
+	 * set the default power management level for UFS runtime and system
+	 * suspend. Default power saving mode selected is keeping UFS link in
+	 * Hibern8 state and UFS device in sleep state.
 	 */
-	hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
-						UFS_SLEEP_PWR_MODE,
-						UIC_LINK_HIBERN8_STATE);
-	hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
-						UFS_SLEEP_PWR_MODE,
-						UIC_LINK_HIBERN8_STATE);
+	if (!ufshcd_is_valid_pm_lvl(hba->rpm_lvl))
+		hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
+							UFS_SLEEP_PWR_MODE,
+							UIC_LINK_HIBERN8_STATE);
+	if (!ufshcd_is_valid_pm_lvl(hba->spm_lvl))
+		hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
+							UFS_SLEEP_PWR_MODE,
+							UIC_LINK_HIBERN8_STATE);
 
 	/* Set the default auto-hiberate idle timer value to 150 ms */
 	if (hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT) {
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index e996a08..a2e1d5c 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -526,9 +526,9 @@ struct ufs_hba {
 	enum ufs_dev_pwr_mode curr_dev_pwr_mode;
 	enum uic_link_state uic_link_state;
 	/* Desired UFS power management level during runtime PM */
-	enum ufs_pm_level rpm_lvl;
+	int rpm_lvl;
 	/* Desired UFS power management level during system PM */
-	enum ufs_pm_level spm_lvl;
+	int spm_lvl;
 	struct device_attribute rpm_lvl_attr;
 	struct device_attribute spm_lvl_attr;
 	int pm_op_in_progress;
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v1 4/9] scsi: ufs: add option to change default UFS power management level

[Adrian Hunter]

On 06/07/18 15:30, Asutosh Das wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
> 
> UFS device and link can be put in multiple different low power modes hence
> UFS driver supports multiple different low power modes. By default UFS
> driver selects the default (optimal) low power mode (which gives moderate
> power savings and have relatively less enter and exit latencies) but
> we might have to tune this default power mode for different chipset
> platforms to meet the low power requirements/goals. Hence this patch
> adds option to change default UFS low power mode (level).
> 
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      | 11 ++++++++
>  drivers/scsi/ufs/ufshcd-pltfrm.c                   | 14 +++++++++++
>  drivers/scsi/ufs/ufshcd.c                          | 29 +++++++++++++++-------
>  drivers/scsi/ufs/ufshcd.h                          |  4 +--
>  4 files changed, 47 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef..f564d9a 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -38,6 +38,15 @@ Optional properties:
>  			  defined or a value in the array is "0" then it is assumed
>  			  that the frequency is set by the parent clock or a
>  			  fixed rate clock source.
> +- rpm-level		: UFS Runtime power management level. Following PM levels are supported:
> +			  0 - Both UFS device and Link in active state (Highest power consumption)
> +			  1 - UFS device in active state but Link in Hibern8 state
> +			  2 - UFS device in Sleep state but Link in active state
> +			  3 - UFS device in Sleep state and Link in hibern8 state (default PM level)
> +			  4 - UFS device in Power-down state and Link in Hibern8 state
> +			  5 - UFS device in Power-down state and Link in OFF state (Lowest power consumption)
> +- spm-level		: UFS System power management level. Allowed PM levels are same as rpm-level.
> +
>  -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
>  			  Note that it is assume same number of lanes is used both
>  			  directions at once. If not specified, default is 2 lanes per direction.
> @@ -66,4 +75,6 @@ Example:
>  		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
>  		phys = <&ufsphy1>;
>  		phy-names = "ufsphy";
> +		rpm-level = <3>;
> +		spm-level = <5>;
>  	};
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index e82bde0..7dba799 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -221,6 +221,19 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
>  	return err;
>  }
>  
> +static void ufshcd_parse_pm_levels(struct ufs_hba *hba)
> +{
> +	struct device *dev = hba->dev;
> +	struct device_node *np = dev->of_node;
> +
> +	if (np) {
> +		if (of_property_read_u32(np, "rpm-level", &hba->rpm_lvl))
> +			hba->rpm_lvl = -1;
> +		if (of_property_read_u32(np, "spm-level", &hba->spm_lvl))
> +			hba->spm_lvl = -1;

These are generically useful to all UFSHC drivers, so they should be
device_property_read_u32() and they should be read for all drivers not just
pltfrm i.e. move this code into ufshcd.c

> +	}
> +}
> +
>  #ifdef CONFIG_PM
>  /**
>   * ufshcd_pltfrm_suspend - suspend power management function
> @@ -340,6 +353,7 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>  		goto dealloc_host;
>  	}
>  
> +	ufshcd_parse_pm_levels(hba);
>  	pm_runtime_set_active(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);
>  
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index b03f3ea..e950204 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -192,6 +192,14 @@ struct ufs_pm_lvl_states ufs_pm_lvl_states[] = {
>  	return UFS_PM_LVL_0;
>  }
>  
> +static inline bool ufshcd_is_valid_pm_lvl(int lvl)
> +{
> +	if (lvl >= 0 && lvl < ARRAY_SIZE(ufs_pm_lvl_states))
> +		return true;
> +	else
> +		return false;
> +}
> +
>  static struct ufs_dev_fix ufs_fixups[] = {
>  	/* UFS cards deviations table */
>  	UFS_FIX(UFS_VENDOR_SAMSUNG, UFS_ANY_MODEL,
> @@ -8051,16 +8059,19 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>  	}
>  
>  	/*
> -	 * Set the default power management level for runtime and system PM.
> -	 * Default power saving mode is to keep UFS link in Hibern8 state
> -	 * and UFS device in sleep state.
> +	 * If rpm_lvl and and spm_lvl are not already set to valid levels,
> +	 * set the default power management level for UFS runtime and system
> +	 * suspend. Default power saving mode selected is keeping UFS link in
> +	 * Hibern8 state and UFS device in sleep state.
>  	 */
> -	hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> -						UFS_SLEEP_PWR_MODE,
> -						UIC_LINK_HIBERN8_STATE);
> -	hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> -						UFS_SLEEP_PWR_MODE,
> -						UIC_LINK_HIBERN8_STATE);
> +	if (!ufshcd_is_valid_pm_lvl(hba->rpm_lvl))
> +		hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> +							UFS_SLEEP_PWR_MODE,
> +							UIC_LINK_HIBERN8_STATE);
> +	if (!ufshcd_is_valid_pm_lvl(hba->spm_lvl))
> +		hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
> +							UFS_SLEEP_PWR_MODE,
> +							UIC_LINK_HIBERN8_STATE);
>  
>  	/* Set the default auto-hiberate idle timer value to 150 ms */
>  	if (hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT) {
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index e996a08..a2e1d5c 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -526,9 +526,9 @@ struct ufs_hba {
>  	enum ufs_dev_pwr_mode curr_dev_pwr_mode;
>  	enum uic_link_state uic_link_state;
>  	/* Desired UFS power management level during runtime PM */
> -	enum ufs_pm_level rpm_lvl;
> +	int rpm_lvl;
>  	/* Desired UFS power management level during system PM */
> -	enum ufs_pm_level spm_lvl;
> +	int spm_lvl;
>  	struct device_attribute rpm_lvl_attr;
>  	struct device_attribute spm_lvl_attr;
>  	int pm_op_in_progress;
> 

Re: [PATCH v1 4/9] scsi: ufs: add option to change default UFS power management level

[Asutosh Das (asd)]

On 7/11/2018 4:20 PM, Adrian Hunter wrote:
> On 06/07/18 15:30, Asutosh Das wrote:
>> From: Subhash Jadavani <subhashj@codeaurora.org>
>>
>> UFS device and link can be put in multiple different low power modes hence
>> UFS driver supports multiple different low power modes. By default UFS
>> driver selects the default (optimal) low power mode (which gives moderate
>> power savings and have relatively less enter and exit latencies) but
>> we might have to tune this default power mode for different chipset
>> platforms to meet the low power requirements/goals. Hence this patch
>> adds option to change default UFS low power mode (level).
>>
>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> ---
>>   .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      | 11 ++++++++
>>   drivers/scsi/ufs/ufshcd-pltfrm.c                   | 14 +++++++++++
>>   drivers/scsi/ufs/ufshcd.c                          | 29 +++++++++++++++-------
>>   drivers/scsi/ufs/ufshcd.h                          |  4 +--
>>   4 files changed, 47 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> index c39dfef..f564d9a 100644
>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> @@ -38,6 +38,15 @@ Optional properties:
>>   			  defined or a value in the array is "0" then it is assumed
>>   			  that the frequency is set by the parent clock or a
>>   			  fixed rate clock source.
>> +- rpm-level		: UFS Runtime power management level. Following PM levels are supported:
>> +			  0 - Both UFS device and Link in active state (Highest power consumption)
>> +			  1 - UFS device in active state but Link in Hibern8 state
>> +			  2 - UFS device in Sleep state but Link in active state
>> +			  3 - UFS device in Sleep state and Link in hibern8 state (default PM level)
>> +			  4 - UFS device in Power-down state and Link in Hibern8 state
>> +			  5 - UFS device in Power-down state and Link in OFF state (Lowest power consumption)
>> +- spm-level		: UFS System power management level. Allowed PM levels are same as rpm-level.
>> +
>>   -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
>>   			  Note that it is assume same number of lanes is used both
>>   			  directions at once. If not specified, default is 2 lanes per direction.
>> @@ -66,4 +75,6 @@ Example:
>>   		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
>>   		phys = <&ufsphy1>;
>>   		phy-names = "ufsphy";
>> +		rpm-level = <3>;
>> +		spm-level = <5>;
>>   	};
>> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
>> index e82bde0..7dba799 100644
>> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
>> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
>> @@ -221,6 +221,19 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
>>   	return err;
>>   }
>>   
>> +static void ufshcd_parse_pm_levels(struct ufs_hba *hba)
>> +{
>> +	struct device *dev = hba->dev;
>> +	struct device_node *np = dev->of_node;
>> +
>> +	if (np) {
>> +		if (of_property_read_u32(np, "rpm-level", &hba->rpm_lvl))
>> +			hba->rpm_lvl = -1;
>> +		if (of_property_read_u32(np, "spm-level", &hba->spm_lvl))
>> +			hba->spm_lvl = -1;
> 
> These are generically useful to all UFSHC drivers, so they should be
> device_property_read_u32() and they should be read for all drivers not just
> pltfrm i.e. move this code into ufshcd.c
> 
>> +	}
>> +}
>> +
>>   #ifdef CONFIG_PM
>>   /**
>>    * ufshcd_pltfrm_suspend - suspend power management function
>> @@ -340,6 +353,7 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>>   		goto dealloc_host;
>>   	}
>>   
>> +	ufshcd_parse_pm_levels(hba);
>>   	pm_runtime_set_active(&pdev->dev);
>>   	pm_runtime_enable(&pdev->dev);
>>   
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index b03f3ea..e950204 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -192,6 +192,14 @@ struct ufs_pm_lvl_states ufs_pm_lvl_states[] = {
>>   	return UFS_PM_LVL_0;
>>   }
>>   
>> +static inline bool ufshcd_is_valid_pm_lvl(int lvl)
>> +{
>> +	if (lvl >= 0 && lvl < ARRAY_SIZE(ufs_pm_lvl_states))
>> +		return true;
>> +	else
>> +		return false;
>> +}
>> +
>>   static struct ufs_dev_fix ufs_fixups[] = {
>>   	/* UFS cards deviations table */
>>   	UFS_FIX(UFS_VENDOR_SAMSUNG, UFS_ANY_MODEL,
>> @@ -8051,16 +8059,19 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>>   	}
>>   
>>   	/*
>> -	 * Set the default power management level for runtime and system PM.
>> -	 * Default power saving mode is to keep UFS link in Hibern8 state
>> -	 * and UFS device in sleep state.
>> +	 * If rpm_lvl and and spm_lvl are not already set to valid levels,
>> +	 * set the default power management level for UFS runtime and system
>> +	 * suspend. Default power saving mode selected is keeping UFS link in
>> +	 * Hibern8 state and UFS device in sleep state.
>>   	 */
>> -	hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
>> -						UFS_SLEEP_PWR_MODE,
>> -						UIC_LINK_HIBERN8_STATE);
>> -	hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
>> -						UFS_SLEEP_PWR_MODE,
>> -						UIC_LINK_HIBERN8_STATE);
>> +	if (!ufshcd_is_valid_pm_lvl(hba->rpm_lvl))
>> +		hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
>> +							UFS_SLEEP_PWR_MODE,
>> +							UIC_LINK_HIBERN8_STATE);
>> +	if (!ufshcd_is_valid_pm_lvl(hba->spm_lvl))
>> +		hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
>> +							UFS_SLEEP_PWR_MODE,
>> +							UIC_LINK_HIBERN8_STATE);
>>   
>>   	/* Set the default auto-hiberate idle timer value to 150 ms */
>>   	if (hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT) {
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index e996a08..a2e1d5c 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -526,9 +526,9 @@ struct ufs_hba {
>>   	enum ufs_dev_pwr_mode curr_dev_pwr_mode;
>>   	enum uic_link_state uic_link_state;
>>   	/* Desired UFS power management level during runtime PM */
>> -	enum ufs_pm_level rpm_lvl;
>> +	int rpm_lvl;
>>   	/* Desired UFS power management level during system PM */
>> -	enum ufs_pm_level spm_lvl;
>> +	int spm_lvl;
>>   	struct device_attribute rpm_lvl_attr;
>>   	struct device_attribute spm_lvl_attr;
>>   	int pm_op_in_progress;
>>
> 

Thanks. I'll make the changes and push it in v2.

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

Re: [PATCH v1 4/9] scsi: ufs: add option to change default UFS power management level

[Rob Herring]

On Fri, Jul 06, 2018 at 06:00:31PM +0530, Asutosh Das wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
> 
> UFS device and link can be put in multiple different low power modes hence
> UFS driver supports multiple different low power modes. By default UFS
> driver selects the default (optimal) low power mode (which gives moderate
> power savings and have relatively less enter and exit latencies) but
> we might have to tune this default power mode for different chipset
> platforms to meet the low power requirements/goals. Hence this patch
> adds option to change default UFS low power mode (level).
> 
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      | 11 ++++++++
>  drivers/scsi/ufs/ufshcd-pltfrm.c                   | 14 +++++++++++
>  drivers/scsi/ufs/ufshcd.c                          | 29 +++++++++++++++-------
>  drivers/scsi/ufs/ufshcd.h                          |  4 +--
>  4 files changed, 47 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef..f564d9a 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -38,6 +38,15 @@ Optional properties:
>  			  defined or a value in the array is "0" then it is assumed
>  			  that the frequency is set by the parent clock or a
>  			  fixed rate clock source.
> +- rpm-level		: UFS Runtime power management level. Following PM levels are supported:
> +			  0 - Both UFS device and Link in active state (Highest power consumption)
> +			  1 - UFS device in active state but Link in Hibern8 state
> +			  2 - UFS device in Sleep state but Link in active state
> +			  3 - UFS device in Sleep state and Link in hibern8 state (default PM level)
> +			  4 - UFS device in Power-down state and Link in Hibern8 state
> +			  5 - UFS device in Power-down state and Link in OFF state (Lowest power consumption)
> +- spm-level		: UFS System power management level. Allowed PM levels are same as rpm-level.

What's the default?

I assume these are minimums? The OS can pick higher power states. This 
seems to be a bit Linux specific (as 'runtime PM' could be considered 
Linux specific). For every other device, we don't put this type of 
information in DT, but is user controlled. So really, wouldn't 1 
property be sufficient for cases where a mode doesn't work due to 
some h/w limitation. Otherwise, it is an OS or user decision.

> +
>  -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
>  			  Note that it is assume same number of lanes is used both
>  			  directions at once. If not specified, default is 2 lanes per direction.
> @@ -66,4 +75,6 @@ Example:
>  		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
>  		phys = <&ufsphy1>;
>  		phy-names = "ufsphy";
> +		rpm-level = <3>;

Why specified if 3 is the default?

> +		spm-level = <5>;

These seem like sane defaults. When and why would you use some 
different?

Rob

Re: [PATCH v1 4/9] scsi: ufs: add option to change default UFS power management level

[Asutosh Das (asd)]

On 7/12/2018 2:03 AM, Rob Herring wrote:
> On Fri, Jul 06, 2018 at 06:00:31PM +0530, Asutosh Das wrote:
>> From: Subhash Jadavani <subhashj@codeaurora.org>
>>
>> UFS device and link can be put in multiple different low power modes hence
>> UFS driver supports multiple different low power modes. By default UFS
>> driver selects the default (optimal) low power mode (which gives moderate
>> power savings and have relatively less enter and exit latencies) but
>> we might have to tune this default power mode for different chipset
>> platforms to meet the low power requirements/goals. Hence this patch
>> adds option to change default UFS low power mode (level).
>>
>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> ---
>>   .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      | 11 ++++++++
>>   drivers/scsi/ufs/ufshcd-pltfrm.c                   | 14 +++++++++++
>>   drivers/scsi/ufs/ufshcd.c                          | 29 +++++++++++++++-------
>>   drivers/scsi/ufs/ufshcd.h                          |  4 +--
>>   4 files changed, 47 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> index c39dfef..f564d9a 100644
>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> @@ -38,6 +38,15 @@ Optional properties:
>>   			  defined or a value in the array is "0" then it is assumed
>>   			  that the frequency is set by the parent clock or a
>>   			  fixed rate clock source.
>> +- rpm-level		: UFS Runtime power management level. Following PM levels are supported:
>> +			  0 - Both UFS device and Link in active state (Highest power consumption)
>> +			  1 - UFS device in active state but Link in Hibern8 state
>> +			  2 - UFS device in Sleep state but Link in active state
>> +			  3 - UFS device in Sleep state and Link in hibern8 state (default PM level)
>> +			  4 - UFS device in Power-down state and Link in Hibern8 state
>> +			  5 - UFS device in Power-down state and Link in OFF state (Lowest power consumption)
>> +- spm-level		: UFS System power management level. Allowed PM levels are same as rpm-level.
> 
> What's the default?
> 
> I assume these are minimums? The OS can pick higher power states. This
> seems to be a bit Linux specific (as 'runtime PM' could be considered
> Linux specific). For every other device, we don't put this type of
> information in DT, but is user controlled.
I didn't completely understand your comment.
Do you not want these properties to be in DT file?
When you say user-controlled, do you mean control it through sysfs entries?

> So really, wouldn't 1
> property be sufficient for cases where a mode doesn't work due to
> some h/w limitation. Otherwise, it is an OS or user decision.
I didn't completely understand this. Could you please elaborate on your 
intent here?

> 
>> +
>>   -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
>>   			  Note that it is assume same number of lanes is used both
>>   			  directions at once. If not specified, default is 2 lanes per direction.
>> @@ -66,4 +75,6 @@ Example:
>>   		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
>>   		phys = <&ufsphy1>;
>>   		phy-names = "ufsphy";
>> +		rpm-level = <3>;
> 
> Why specified if 3 is the default?
Ah yes - that should be removed.
I'll remove it in v2.

> 
>> +		spm-level = <5>;
> 
> These seem like sane defaults. When and why would you use some
> different?
I think each of the deeper sleep modes are associated with an increasing 
wakeup latency. For e.g. '0' would have the highest power-consumption 
and no resume latency at all as compared to '5'.
So depending on use-cases other modes may be chosen.

> 
> Rob
> 

-asd

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

Re: [PATCH v1 4/9] scsi: ufs: add option to change default UFS power management level

[Rob Herring]

On Sun, Jul 22, 2018 at 9:20 PM Asutosh Das (asd)
<asutoshd@codeaurora.org> wrote:
>
> On 7/12/2018 2:03 AM, Rob Herring wrote:
> > On Fri, Jul 06, 2018 at 06:00:31PM +0530, Asutosh Das wrote:
> >> From: Subhash Jadavani <subhashj@codeaurora.org>
> >>
> >> UFS device and link can be put in multiple different low power modes hence
> >> UFS driver supports multiple different low power modes. By default UFS
> >> driver selects the default (optimal) low power mode (which gives moderate
> >> power savings and have relatively less enter and exit latencies) but
> >> we might have to tune this default power mode for different chipset
> >> platforms to meet the low power requirements/goals. Hence this patch
> >> adds option to change default UFS low power mode (level).
> >>
> >> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> >> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> >> Signed-off-by: Can Guo <cang@codeaurora.org>
> >> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> >> ---
> >>   .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      | 11 ++++++++
> >>   drivers/scsi/ufs/ufshcd-pltfrm.c                   | 14 +++++++++++
> >>   drivers/scsi/ufs/ufshcd.c                          | 29 +++++++++++++++-------
> >>   drivers/scsi/ufs/ufshcd.h                          |  4 +--
> >>   4 files changed, 47 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> >> index c39dfef..f564d9a 100644
> >> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> >> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> >> @@ -38,6 +38,15 @@ Optional properties:
> >>                        defined or a value in the array is "0" then it is assumed
> >>                        that the frequency is set by the parent clock or a
> >>                        fixed rate clock source.
> >> +- rpm-level         : UFS Runtime power management level. Following PM levels are supported:
> >> +                      0 - Both UFS device and Link in active state (Highest power consumption)
> >> +                      1 - UFS device in active state but Link in Hibern8 state
> >> +                      2 - UFS device in Sleep state but Link in active state
> >> +                      3 - UFS device in Sleep state and Link in hibern8 state (default PM level)
> >> +                      4 - UFS device in Power-down state and Link in Hibern8 state
> >> +                      5 - UFS device in Power-down state and Link in OFF state (Lowest power consumption)
> >> +- spm-level         : UFS System power management level. Allowed PM levels are same as rpm-level.
> >
> > What's the default?
> >
> > I assume these are minimums? The OS can pick higher power states. This
> > seems to be a bit Linux specific (as 'runtime PM' could be considered
> > Linux specific). For every other device, we don't put this type of
> > information in DT, but is user controlled.
> I didn't completely understand your comment.
> Do you not want these properties to be in DT file?

Right, not if it is a user decision.

> When you say user-controlled, do you mean control it through sysfs entries?

Yes.

> > So really, wouldn't 1
> > property be sufficient for cases where a mode doesn't work due to
> > some h/w limitation. Otherwise, it is an OS or user decision.
> I didn't completely understand this. Could you please elaborate on your
> intent here?

The case that makes sense for this to be in DT is if there are h/w
limitations that prevent some low power modes. In such a case, that
limit is not likely specific to runtime PM or system suspend.

> >>   -lanes-per-direction       : number of lanes available per direction - either 1 or 2.
> >>                        Note that it is assume same number of lanes is used both
> >>                        directions at once. If not specified, default is 2 lanes per direction.
> >> @@ -66,4 +75,6 @@ Example:
> >>              freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
> >>              phys = <&ufsphy1>;
> >>              phy-names = "ufsphy";
> >> +            rpm-level = <3>;
> >
> > Why specified if 3 is the default?
> Ah yes - that should be removed.
> I'll remove it in v2.
>
> >
> >> +            spm-level = <5>;
> >
> > These seem like sane defaults. When and why would you use some
> > different?
> I think each of the deeper sleep modes are associated with an increasing
> wakeup latency. For e.g. '0' would have the highest power-consumption
> and no resume latency at all as compared to '5'.
> So depending on use-cases other modes may be chosen.

The use-case can change in a running system. For example if you are
plugged in, then you probably don't want to enter a lower power mode.

Rob

Re: [PATCH v1 4/9] scsi: ufs: add option to change default UFS power management level

[Asutosh Das (asd)]

On 7/23/2018 8:06 PM, Rob Herring wrote:
> On Sun, Jul 22, 2018 at 9:20 PM Asutosh Das (asd)
> <asutoshd@codeaurora.org> wrote:
>>
>> On 7/12/2018 2:03 AM, Rob Herring wrote:
>>> On Fri, Jul 06, 2018 at 06:00:31PM +0530, Asutosh Das wrote:
>>>> From: Subhash Jadavani <subhashj@codeaurora.org>
>>>>
>>>> UFS device and link can be put in multiple different low power modes hence
>>>> UFS driver supports multiple different low power modes. By default UFS
>>>> driver selects the default (optimal) low power mode (which gives moderate
>>>> power savings and have relatively less enter and exit latencies) but
>>>> we might have to tune this default power mode for different chipset
>>>> platforms to meet the low power requirements/goals. Hence this patch
>>>> adds option to change default UFS low power mode (level).
>>>>
>>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>>>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>>> Signed-off-by: Can Guo <cang@codeaurora.org>
>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>> ---
>>>>    .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      | 11 ++++++++
>>>>    drivers/scsi/ufs/ufshcd-pltfrm.c                   | 14 +++++++++++
>>>>    drivers/scsi/ufs/ufshcd.c                          | 29 +++++++++++++++-------
>>>>    drivers/scsi/ufs/ufshcd.h                          |  4 +--
>>>>    4 files changed, 47 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> index c39dfef..f564d9a 100644
>>>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> @@ -38,6 +38,15 @@ Optional properties:
>>>>                         defined or a value in the array is "0" then it is assumed
>>>>                         that the frequency is set by the parent clock or a
>>>>                         fixed rate clock source.
>>>> +- rpm-level         : UFS Runtime power management level. Following PM levels are supported:
>>>> +                      0 - Both UFS device and Link in active state (Highest power consumption)
>>>> +                      1 - UFS device in active state but Link in Hibern8 state
>>>> +                      2 - UFS device in Sleep state but Link in active state
>>>> +                      3 - UFS device in Sleep state and Link in hibern8 state (default PM level)
>>>> +                      4 - UFS device in Power-down state and Link in Hibern8 state
>>>> +                      5 - UFS device in Power-down state and Link in OFF state (Lowest power consumption)
>>>> +- spm-level         : UFS System power management level. Allowed PM levels are same as rpm-level.
>>>
>>> What's the default?
>>>
>>> I assume these are minimums? The OS can pick higher power states. This
>>> seems to be a bit Linux specific (as 'runtime PM' could be considered
>>> Linux specific). For every other device, we don't put this type of
>>> information in DT, but is user controlled.
>> I didn't completely understand your comment.
>> Do you not want these properties to be in DT file?
> 
> Right, not if it is a user decision.
> 
>> When you say user-controlled, do you mean control it through sysfs entries?
> 
> Yes.
> 
>>> So really, wouldn't 1
>>> property be sufficient for cases where a mode doesn't work due to
>>> some h/w limitation. Otherwise, it is an OS or user decision.
>> I didn't completely understand this. Could you please elaborate on your
>> intent here?
> 
> The case that makes sense for this to be in DT is if there are h/w
> limitations that prevent some low power modes. In such a case, that
> limit is not likely specific to runtime PM or system suspend.
> 
>>>>    -lanes-per-direction       : number of lanes available per direction - either 1 or 2.
>>>>                         Note that it is assume same number of lanes is used both
>>>>                         directions at once. If not specified, default is 2 lanes per direction.
>>>> @@ -66,4 +75,6 @@ Example:
>>>>               freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
>>>>               phys = <&ufsphy1>;
>>>>               phy-names = "ufsphy";
>>>> +            rpm-level = <3>;
>>>
>>> Why specified if 3 is the default?
>> Ah yes - that should be removed.
>> I'll remove it in v2.
>>
>>>
>>>> +            spm-level = <5>;
>>>
>>> These seem like sane defaults. When and why would you use some
>>> different?
>> I think each of the deeper sleep modes are associated with an increasing
>> wakeup latency. For e.g. '0' would have the highest power-consumption
>> and no resume latency at all as compared to '5'.
>> So depending on use-cases other modes may be chosen.
> 
> The use-case can change in a running system. For example if you are
> plugged in, then you probably don't want to enter a lower power mode.
> 
> Rob
> 

Ok - Thanks.
I'm gonna make the following changes in v2:
1. Remove these entries from DT
2. Add support for these entries in sysfs.
3. Keep 3 as the default value for [rpm/spm]-levels

-asd

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

[PATCH v1 5/9] scsi: ufs: add support for hibern8 on idle

[Asutosh Das]

From: Subhash Jadavani <subhashj@codeaurora.org>

In order to save power we should put the UFS link into hibern8 as soon as
UFS link is idle and power measurement of active usecases (like audio/video
playback/recording) show that putting UFS link in hibern8 @ 10ms of idle
(if not earlier) would save significant power.

Our current available solution is to do hibern8 with clock gating @idle
timeout of 150ms. As clock gating has huge latencies (7ms each in enter and
exit), we cannot bring down the idle timeout to <=10ms without degrading
UFS throughput. Hence this change has added support to enter into hibern8
with another idle timer.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c  | 372 ++++++++++++++++++++++++++++++++++++++++-----
 drivers/scsi/ufs/ufshcd.h  |  39 +++++
 include/trace/events/ufs.h |  20 +++
 3 files changed, 397 insertions(+), 34 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index e950204..8a56ef6 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -246,6 +246,8 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
 static irqreturn_t ufshcd_intr(int irq, void *__hba);
 static int ufshcd_change_power_mode(struct ufs_hba *hba,
 			     struct ufs_pa_layer_attr *pwr_mode);
+static void ufshcd_hold_all(struct ufs_hba *hba);
+static void ufshcd_release_all(struct ufs_hba *hba);
 static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag)
 {
 	return tag >= 0 && tag < hba->nutrs;
@@ -853,6 +855,18 @@ static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
 		? false : true;
 }
 
+static const char *ufshcd_hibern8_on_idle_state_to_string(
+			enum ufshcd_hibern8_on_idle_state state)
+{
+	switch (state) {
+	case HIBERN8_ENTERED:		return "HIBERN8_ENTERED";
+	case HIBERN8_EXITED:		return "HIBERN8_EXITED";
+	case REQ_HIBERN8_ENTER:		return "REQ_HIBERN8_ENTER";
+	case REQ_HIBERN8_EXIT:		return "REQ_HIBERN8_EXIT";
+	default:			return "UNKNOWN_STATE";
+	}
+}
+
 u32 ufshcd_get_local_unipro_ver(struct ufs_hba *hba)
 {
 	/* HCI version 1.0 and 1.1 supports UniPro 1.41 */
@@ -993,7 +1007,7 @@ static int ufshcd_wait_for_doorbell_clr(struct ufs_hba *hba,
 	bool timeout = false, do_last_check = false;
 	ktime_t start;
 
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 	spin_lock_irqsave(hba->host->host_lock, flags);
 	/*
 	 * Wait for all the outstanding tasks/transfer requests.
@@ -1038,7 +1052,7 @@ static int ufshcd_wait_for_doorbell_clr(struct ufs_hba *hba,
 	}
 out:
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 	return ret;
 }
 
@@ -1601,8 +1615,16 @@ static void ufshcd_gate_work(struct work_struct *work)
 
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 
+	if (ufshcd_is_hibern8_on_idle_allowed(hba))
+		/*
+		 * Hibern8 enter work (on Idle) needs clocks to be ON hence
+		 * make sure that it is flushed before turning off the clocks.
+		 */
+		flush_delayed_work(&hba->hibern8_on_idle.enter_work);
+
 	/* put the link into hibern8 mode before turning off clocks */
-	if (ufshcd_can_hibern8_during_gating(hba)) {
+	if (ufshcd_can_hibern8_during_gating(hba) &&
+	    ufshcd_is_link_active(hba)) {
 		if (ufshcd_uic_hibern8_enter(hba)) {
 			hba->clk_gating.state = CLKS_ON;
 			trace_ufshcd_clk_gating(dev_name(hba->dev),
@@ -1732,6 +1754,8 @@ static void ufshcd_init_clk_gating(struct ufs_hba *hba)
 {
 	char wq_name[sizeof("ufs_clk_gating_00")];
 
+	hba->clk_gating.state = CLKS_ON;
+
 	if (!ufshcd_is_clkgating_allowed(hba))
 		return;
 
@@ -1774,6 +1798,246 @@ static void ufshcd_exit_clk_gating(struct ufs_hba *hba)
 	destroy_workqueue(hba->clk_gating.clk_gating_workq);
 }
 
+/**
+ * ufshcd_hibern8_hold - Make sure that link is not in hibern8.
+ *
+ * @hba: per adapter instance
+ * @async: This indicates whether caller wants to exit hibern8 asynchronously.
+ *
+ * Exit from hibern8 mode and set the link as active.
+ *
+ * Return 0 on success, non-zero on failure.
+ */
+int ufshcd_hibern8_hold(struct ufs_hba *hba, bool async)
+{
+	int rc = 0;
+	unsigned long flags;
+
+	if (!ufshcd_is_hibern8_on_idle_allowed(hba))
+		goto out;
+
+	spin_lock_irqsave(hba->host->host_lock, flags);
+	hba->hibern8_on_idle.active_reqs++;
+
+start:
+	switch (hba->hibern8_on_idle.state) {
+	case HIBERN8_EXITED:
+		break;
+	case REQ_HIBERN8_ENTER:
+		if (cancel_delayed_work(&hba->hibern8_on_idle.enter_work)) {
+			hba->hibern8_on_idle.state = HIBERN8_EXITED;
+			trace_ufshcd_hibern8_on_idle(dev_name(hba->dev),
+				ufshcd_hibern8_on_idle_state_to_string(
+					hba->hibern8_on_idle.state));
+			break;
+		}
+		/*
+		 * If we here, it means Hibern8 enter work is either done or
+		 * currently running. Hence, fall through to cancel hibern8
+		 * work and exit hibern8.
+		 */
+	case HIBERN8_ENTERED:
+		scsi_block_requests(hba->host);
+		hba->hibern8_on_idle.state = REQ_HIBERN8_EXIT;
+		trace_ufshcd_hibern8_on_idle(dev_name(hba->dev),
+			ufshcd_hibern8_on_idle_state_to_string(
+				hba->hibern8_on_idle.state));
+		schedule_work(&hba->hibern8_on_idle.exit_work);
+		/*
+		 * fall through to check if we should wait for this
+		 * work to be done or not.
+		 */
+	case REQ_HIBERN8_EXIT:
+		if (async) {
+			rc = -EAGAIN;
+			hba->hibern8_on_idle.active_reqs--;
+			break;
+		}
+		spin_unlock_irqrestore(hba->host->host_lock, flags);
+		flush_work(&hba->hibern8_on_idle.exit_work);
+		/* Make sure state is HIBERN8_EXITED before returning */
+		spin_lock_irqsave(hba->host->host_lock, flags);
+		goto start;
+
+	default:
+		dev_err(hba->dev, "%s: H8 is in invalid state %d\n",
+				__func__, hba->hibern8_on_idle.state);
+		break;
+	}
+	spin_unlock_irqrestore(hba->host->host_lock, flags);
+out:
+	return rc;
+}
+
+/* host lock must be held before calling this variant */
+static void __ufshcd_hibern8_release(struct ufs_hba *hba)
+{
+	unsigned long delay_in_jiffies;
+
+	if (!ufshcd_is_hibern8_on_idle_allowed(hba))
+		return;
+
+	hba->hibern8_on_idle.active_reqs--;
+	WARN_ON(hba->hibern8_on_idle.active_reqs < 0);
+
+	if (hba->hibern8_on_idle.active_reqs
+		|| hba->hibern8_on_idle.is_suspended
+		|| hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL
+		|| hba->lrb_in_use || hba->outstanding_tasks
+		|| hba->active_uic_cmd || hba->uic_async_done)
+		return;
+
+	hba->hibern8_on_idle.state = REQ_HIBERN8_ENTER;
+	trace_ufshcd_hibern8_on_idle(dev_name(hba->dev),
+			ufshcd_hibern8_on_idle_state_to_string(
+				hba->hibern8_on_idle.state));
+	/*
+	 * Scheduling the delayed work after 1 jiffies will make the work to
+	 * get schedule any time from 0ms to 1000/HZ ms which is not desirable
+	 * for hibern8 enter work as it may impact the performance if it gets
+	 * scheduled almost immediately. Hence make sure that hibern8 enter
+	 * work gets scheduled atleast after 2 jiffies (any time between
+	 * 1000/HZ ms to 2000/HZ ms).
+	 */
+	delay_in_jiffies = msecs_to_jiffies(hba->hibern8_on_idle.delay_ms);
+	if (delay_in_jiffies == 1)
+		delay_in_jiffies++;
+
+	schedule_delayed_work(&hba->hibern8_on_idle.enter_work,
+			      delay_in_jiffies);
+}
+
+void ufshcd_hibern8_release(struct ufs_hba *hba)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(hba->host->host_lock, flags);
+	__ufshcd_hibern8_release(hba);
+	spin_unlock_irqrestore(hba->host->host_lock, flags);
+}
+
+static void ufshcd_hibern8_enter_work(struct work_struct *work)
+{
+	struct ufs_hba *hba = container_of(work, struct ufs_hba,
+					   hibern8_on_idle.enter_work.work);
+	unsigned long flags;
+
+	spin_lock_irqsave(hba->host->host_lock, flags);
+	if (hba->hibern8_on_idle.is_suspended) {
+		hba->hibern8_on_idle.state = HIBERN8_EXITED;
+		trace_ufshcd_hibern8_on_idle(dev_name(hba->dev),
+				ufshcd_hibern8_on_idle_state_to_string(
+					hba->hibern8_on_idle.state));
+		goto rel_lock;
+	}
+
+	if (hba->hibern8_on_idle.active_reqs
+		|| hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL
+		|| hba->lrb_in_use || hba->outstanding_tasks
+		|| hba->active_uic_cmd || hba->uic_async_done)
+		goto rel_lock;
+
+	spin_unlock_irqrestore(hba->host->host_lock, flags);
+
+	if (ufshcd_is_link_active(hba) && ufshcd_uic_hibern8_enter(hba)) {
+		/* Enter failed */
+		hba->hibern8_on_idle.state = HIBERN8_EXITED;
+		trace_ufshcd_hibern8_on_idle(dev_name(hba->dev),
+				ufshcd_hibern8_on_idle_state_to_string(
+					hba->hibern8_on_idle.state));
+		goto out;
+	}
+	ufshcd_set_link_hibern8(hba);
+
+	/*
+	 * In case you are here to cancel this work the hibern8_on_idle.state
+	 * would be marked as REQ_HIBERN8_EXIT. In this case keep the state
+	 * as REQ_HIBERN8_EXIT which would anyway imply that we are in hibern8
+	 * and a request to exit from it is pending. By doing this way,
+	 * we keep the state machine in tact and this would ultimately
+	 * prevent from doing cancel work multiple times when there are
+	 * new requests arriving before the current cancel work is done.
+	 */
+	spin_lock_irqsave(hba->host->host_lock, flags);
+	if (hba->hibern8_on_idle.state == REQ_HIBERN8_ENTER) {
+		hba->hibern8_on_idle.state = HIBERN8_ENTERED;
+		trace_ufshcd_hibern8_on_idle(dev_name(hba->dev),
+				ufshcd_hibern8_on_idle_state_to_string(
+					hba->hibern8_on_idle.state));
+	}
+rel_lock:
+	spin_unlock_irqrestore(hba->host->host_lock, flags);
+out:
+	return;
+}
+
+static void ufshcd_hibern8_exit_work(struct work_struct *work)
+{
+	int ret;
+	unsigned long flags;
+	struct ufs_hba *hba = container_of(work, struct ufs_hba,
+					   hibern8_on_idle.exit_work);
+
+	cancel_delayed_work_sync(&hba->hibern8_on_idle.enter_work);
+
+	spin_lock_irqsave(hba->host->host_lock, flags);
+	if ((hba->hibern8_on_idle.state == HIBERN8_EXITED)
+	     || ufshcd_is_link_active(hba)) {
+		hba->hibern8_on_idle.state = HIBERN8_EXITED;
+		spin_unlock_irqrestore(hba->host->host_lock, flags);
+		goto unblock_reqs;
+	}
+	spin_unlock_irqrestore(hba->host->host_lock, flags);
+
+	/* Exit from hibern8 */
+	if (ufshcd_is_link_hibern8(hba)) {
+		ret = ufshcd_uic_hibern8_exit(hba);
+		if (!ret) {
+			spin_lock_irqsave(hba->host->host_lock, flags);
+			ufshcd_set_link_active(hba);
+			hba->hibern8_on_idle.state = HIBERN8_EXITED;
+			trace_ufshcd_hibern8_on_idle(dev_name(hba->dev),
+					ufshcd_hibern8_on_idle_state_to_string(
+						hba->hibern8_on_idle.state));
+			spin_unlock_irqrestore(hba->host->host_lock, flags);
+		}
+	}
+unblock_reqs:
+	scsi_unblock_requests(hba->host);
+}
+
+static void ufshcd_init_hibern8_on_idle(struct ufs_hba *hba)
+{
+	if (!ufshcd_is_hibern8_on_idle_allowed(hba))
+		return;
+
+	INIT_DELAYED_WORK(&hba->hibern8_on_idle.enter_work,
+			  ufshcd_hibern8_enter_work);
+	INIT_WORK(&hba->hibern8_on_idle.exit_work, ufshcd_hibern8_exit_work);
+
+	hba->hibern8_on_idle.delay_ms = 10;
+	hba->hibern8_on_idle.state = HIBERN8_EXITED;
+}
+
+static void ufshcd_exit_hibern8_on_idle(struct ufs_hba *hba)
+{
+	if (!ufshcd_is_hibern8_on_idle_allowed(hba))
+		return;
+	/* Don't have anything to do for now */
+}
+
+static void ufshcd_hold_all(struct ufs_hba *hba)
+{
+	ufshcd_hold(hba, false);
+	ufshcd_hibern8_hold(hba, false);
+}
+
+static void ufshcd_release_all(struct ufs_hba *hba)
+{
+	ufshcd_hibern8_release(hba);
+	ufshcd_release(hba);
+}
+
 /* Must be called with host lock acquired */
 static void ufshcd_clk_scaling_start_busy(struct ufs_hba *hba)
 {
@@ -2026,7 +2290,7 @@ static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
 	int ret;
 	unsigned long flags;
 
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 	mutex_lock(&hba->uic_cmd_mutex);
 	ufshcd_add_delay_before_dme_cmd(hba);
 
@@ -2038,7 +2302,7 @@ static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
 
 	mutex_unlock(&hba->uic_cmd_mutex);
 
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 	return ret;
 }
 
@@ -2410,7 +2674,18 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 		clear_bit_unlock(tag, &hba->lrb_in_use);
 		goto out;
 	}
-	WARN_ON(hba->clk_gating.state != CLKS_ON);
+	if (ufshcd_is_clkgating_allowed(hba))
+		WARN_ON(hba->clk_gating.state != CLKS_ON);
+
+	err = ufshcd_hibern8_hold(hba, true);
+	if (err) {
+		clear_bit_unlock(tag, &hba->lrb_in_use);
+		err = SCSI_MLQUEUE_HOST_BUSY;
+		ufshcd_release(hba);
+		goto out;
+	}
+	if (ufshcd_is_hibern8_on_idle_allowed(hba))
+		WARN_ON(hba->hibern8_on_idle.state != HIBERN8_EXITED);
 
 	lrbp = &hba->lrb[tag];
 
@@ -2731,7 +3006,7 @@ int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
 
 	BUG_ON(!hba);
 
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 	mutex_lock(&hba->dev_cmd.lock);
 	ufshcd_init_query(hba, &request, &response, opcode, idn, index,
 			selector);
@@ -2775,7 +3050,7 @@ int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode,
 
 out_unlock:
 	mutex_unlock(&hba->dev_cmd.lock);
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 	return err;
 }
 
@@ -2799,7 +3074,7 @@ int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
 
 	BUG_ON(!hba);
 
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 	if (!attr_val) {
 		dev_err(hba->dev, "%s: attribute value required for opcode 0x%x\n",
 				__func__, opcode);
@@ -2839,7 +3114,7 @@ int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode,
 out_unlock:
 	mutex_unlock(&hba->dev_cmd.lock);
 out:
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 	return err;
 }
 
@@ -2890,7 +3165,7 @@ static int __ufshcd_query_descriptor(struct ufs_hba *hba,
 
 	BUG_ON(!hba);
 
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 	if (!desc_buf) {
 		dev_err(hba->dev, "%s: descriptor buffer required for opcode 0x%x\n",
 				__func__, opcode);
@@ -2940,7 +3215,7 @@ static int __ufshcd_query_descriptor(struct ufs_hba *hba,
 out_unlock:
 	mutex_unlock(&hba->dev_cmd.lock);
 out:
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 	return err;
 }
 
@@ -3751,10 +4026,9 @@ static int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
 	uic_cmd.command = UIC_CMD_DME_SET;
 	uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
 	uic_cmd.argument3 = mode;
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 	ret = ufshcd_uic_pwr_ctrl(hba, &uic_cmd);
-	ufshcd_release(hba);
-
+	ufshcd_release_all(hba);
 out:
 	return ret;
 }
@@ -4368,7 +4642,7 @@ static int ufshcd_verify_dev_init(struct ufs_hba *hba)
 	int err = 0;
 	int retries;
 
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 	mutex_lock(&hba->dev_cmd.lock);
 	for (retries = NOP_OUT_RETRIES; retries > 0; retries--) {
 		err = ufshcd_exec_dev_cmd(hba, DEV_CMD_TYPE_NOP,
@@ -4380,7 +4654,7 @@ static int ufshcd_verify_dev_init(struct ufs_hba *hba)
 		dev_dbg(hba->dev, "%s: error %d retrying\n", __func__, err);
 	}
 	mutex_unlock(&hba->dev_cmd.lock);
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 
 	if (err)
 		dev_err(hba->dev, "%s: NOP OUT failed %d\n", __func__, err);
@@ -4777,6 +5051,7 @@ static void __ufshcd_transfer_req_compl(struct ufs_hba *hba,
 			/* Do not touch lrbp after scsi done */
 			cmd->scsi_done(cmd);
 			__ufshcd_release(hba);
+			__ufshcd_hibern8_release(hba);
 		} else if (lrbp->command_type == UTP_CMD_TYPE_DEV_MANAGE ||
 			lrbp->command_type == UTP_CMD_TYPE_UFS_STORAGE) {
 			if (hba->dev_cmd.complete) {
@@ -5224,7 +5499,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 	hba = container_of(work, struct ufs_hba, eh_work);
 
 	pm_runtime_get_sync(hba->dev);
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 
 	spin_lock_irqsave(hba->host->host_lock, flags);
 	if (hba->ufshcd_state == UFSHCD_STATE_RESET)
@@ -5334,7 +5609,7 @@ static void ufshcd_err_handler(struct work_struct *work)
 out:
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 	ufshcd_scsi_unblock_requests(hba);
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 	pm_runtime_put_sync(hba->dev);
 }
 
@@ -5590,7 +5865,7 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int lun_id, int task_id,
 	 * the maximum wait time is bounded by %TM_CMD_TIMEOUT.
 	 */
 	wait_event(hba->tm_tag_wq, ufshcd_get_tm_free_slot(hba, &free_slot));
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 
 	spin_lock_irqsave(host->host_lock, flags);
 	task_req_descp = hba->utmrdl_base_addr;
@@ -5650,7 +5925,7 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int lun_id, int task_id,
 	ufshcd_put_tm_slot(hba, free_slot);
 	wake_up(&hba->tm_tag_wq);
 
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 	return err;
 }
 
@@ -5763,7 +6038,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN)
 		return ufshcd_eh_host_reset_handler(cmd);
 
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 	reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 	/* If command is already aborted/completed, return SUCCESS */
 	if (!(test_bit(tag, &hba->outstanding_reqs))) {
@@ -5884,10 +6159,10 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
 	}
 
 	/*
-	 * This ufshcd_release() corresponds to the original scsi cmd that got
-	 * aborted here (as we won't get any IRQ for it).
+	 * This ufshcd_release_all() corresponds to the original scsi cmd that
+	 * got aborted here (as we won't get any IRQ for it).
 	 */
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 	return err;
 }
 
@@ -5975,7 +6250,7 @@ static int ufshcd_eh_host_reset_handler(struct scsi_cmnd *cmd)
 
 	hba = shost_priv(cmd->device->host);
 
-	ufshcd_hold(hba, false);
+	ufshcd_hold_all(hba);
 	/*
 	 * Check if there is any race with fatal error handling.
 	 * If so, wait for it to complete. Even though fatal error
@@ -6010,7 +6285,7 @@ static int ufshcd_eh_host_reset_handler(struct scsi_cmnd *cmd)
 	ufshcd_clear_eh_in_progress(hba);
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 	return err;
 }
 
@@ -6644,7 +6919,13 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
 {
 	struct ufs_hba *hba = (struct ufs_hba *)data;
 
+	/*
+	 * Don't allow clock gating and hibern8 enter for faster device
+	 * detection.
+	 */
+	ufshcd_hold_all(hba);
 	ufshcd_probe_hba(hba);
+	ufshcd_release_all(hba);
 }
 
 static enum blk_eh_timer_return ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
@@ -7439,8 +7720,10 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	 * If we can't transition into any of the low power modes
 	 * just gate the clocks.
 	 */
-	ufshcd_hold(hba, false);
+	WARN_ON(hba->hibern8_on_idle.active_reqs);
+	ufshcd_hold_all(hba);
 	hba->clk_gating.is_suspended = true;
+	hba->hibern8_on_idle.is_suspended = true;
 
 	if (hba->clk_scaling.is_allowed) {
 		cancel_work_sync(&hba->clk_scaling.suspend_work);
@@ -7493,6 +7776,10 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	if (ret)
 		goto set_dev_active;
 
+	if (ufshcd_is_link_hibern8(hba) &&
+	    ufshcd_is_hibern8_on_idle_allowed(hba))
+		hba->hibern8_on_idle.state = HIBERN8_ENTERED;
+
 	ufshcd_vreg_set_lpm(hba);
 
 disable_clks:
@@ -7511,8 +7798,11 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 		/* If link is active, device ref_clk can't be switched off */
 		__ufshcd_setup_clocks(hba, false, true);
 
-	hba->clk_gating.state = CLKS_OFF;
-	trace_ufshcd_clk_gating(dev_name(hba->dev), hba->clk_gating.state);
+	if (ufshcd_is_clkgating_allowed(hba)) {
+		hba->clk_gating.state = CLKS_OFF;
+		trace_ufshcd_clk_gating(dev_name(hba->dev),
+					hba->clk_gating.state);
+	}
 	/*
 	 * Disable the host irq as host controller as there won't be any
 	 * host controller transaction expected till resume.
@@ -7534,10 +7824,11 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	if (!ufshcd_set_dev_pwr_mode(hba, UFS_ACTIVE_PWR_MODE))
 		ufshcd_disable_auto_bkops(hba);
 enable_gating:
+	hba->hibern8_on_idle.is_suspended = false;
 	if (hba->clk_scaling.is_allowed)
 		ufshcd_resume_clkscaling(hba);
 	hba->clk_gating.is_suspended = false;
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 out:
 	hba->pm_op_in_progress = 0;
 	return ret;
@@ -7587,10 +7878,13 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 
 	if (ufshcd_is_link_hibern8(hba)) {
 		ret = ufshcd_uic_hibern8_exit(hba);
-		if (!ret)
+		if (!ret) {
 			ufshcd_set_link_active(hba);
-		else
+			if (ufshcd_is_hibern8_on_idle_allowed(hba))
+				hba->hibern8_on_idle.state = HIBERN8_EXITED;
+		} else {
 			goto vendor_suspend;
+		}
 	} else if (ufshcd_is_link_off(hba)) {
 		ret = ufshcd_host_reset_and_restore(hba);
 		/*
@@ -7599,6 +7893,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 		 */
 		if (ret || !ufshcd_is_link_active(hba))
 			goto vendor_suspend;
+		if (ufshcd_is_hibern8_on_idle_allowed(hba))
+			hba->hibern8_on_idle.state = HIBERN8_EXITED;
 	}
 
 	if (!ufshcd_is_ufs_dev_active(hba)) {
@@ -7617,12 +7913,13 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 		ufshcd_urgent_bkops(hba);
 
 	hba->clk_gating.is_suspended = false;
+	hba->hibern8_on_idle.is_suspended = false;
 
 	if (hba->clk_scaling.is_allowed)
 		ufshcd_resume_clkscaling(hba);
 
 	/* Schedule clock gating in case of no access to UFS device yet */
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 
 	/* Enable Auto-Hibernate if configured */
 	ufshcd_auto_hibern8_enable(hba);
@@ -7631,6 +7928,9 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 
 set_old_link_state:
 	ufshcd_link_state_transition(hba, old_link_state, 0);
+	if (ufshcd_is_link_hibern8(hba) &&
+	    ufshcd_is_hibern8_on_idle_allowed(hba))
+		hba->hibern8_on_idle.state = HIBERN8_ENTERED;
 vendor_suspend:
 	ufshcd_vops_suspend(hba, pm_op);
 disable_vreg:
@@ -7640,6 +7940,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	if (hba->clk_scaling.is_allowed)
 		ufshcd_suspend_clkscaling(hba);
 	ufshcd_setup_clocks(hba, false);
+	if (ufshcd_is_clkgating_allowed(hba))
+		hba->clk_gating.state = CLKS_OFF;
 out:
 	hba->pm_op_in_progress = 0;
 	return ret;
@@ -7842,6 +8144,7 @@ void ufshcd_remove(struct ufs_hba *hba)
 	ufshcd_hba_stop(hba, true);
 
 	ufshcd_exit_clk_gating(hba);
+	ufshcd_exit_hibern8_on_idle(hba);
 	if (ufshcd_is_clkscaling_supported(hba))
 		device_remove_file(hba->dev, &hba->clk_scaling.enable_attr);
 	ufshcd_hba_exit(hba);
@@ -8004,6 +8307,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	init_waitqueue_head(&hba->dev_cmd.tag_wq);
 
 	ufshcd_init_clk_gating(hba);
+	ufshcd_init_hibern8_on_idle(hba);
 
 	/*
 	 * In order to avoid any spurious interrupt immediately after
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index a2e1d5c..eaccc76 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -370,6 +370,35 @@ struct ufs_saved_pwr_info {
 	bool is_valid;
 };
 
+/* Hibern8 state  */
+enum ufshcd_hibern8_on_idle_state {
+	HIBERN8_ENTERED,
+	HIBERN8_EXITED,
+	REQ_HIBERN8_ENTER,
+	REQ_HIBERN8_EXIT,
+};
+
+/**
+ * struct ufs_hibern8_on_idle - UFS Hibern8 on idle related data
+ * @enter_work: worker to put UFS link in hibern8 after some delay as
+ * specified in delay_ms
+ * @exit_work: worker to bring UFS link out of hibern8
+ * @state: the current hibern8 state
+ * @delay_ms: hibern8 enter delay in ms
+ * @is_suspended: hibern8 enter is suspended when set to 1 which can be used
+ * during suspend/resume
+ * @active_reqs: number of requests that are pending and should be waited for
+ * completion before scheduling delayed "enter_work".
+ */
+struct ufs_hibern8_on_idle {
+	struct delayed_work enter_work;
+	struct work_struct exit_work;
+	enum ufshcd_hibern8_on_idle_state state;
+	unsigned long delay_ms;
+	bool is_suspended;
+	int active_reqs;
+};
+
 /**
  * struct ufs_clk_scaling - UFS clock scaling related data
  * @active_reqs: number of requests that are pending. If this is zero when
@@ -496,6 +525,7 @@ struct ufs_stats {
  * @clk_list_head: UFS host controller clocks list node head
  * @pwr_info: holds current power mode
  * @max_pwr_info: keeps the device max valid pwm
+ * @hibern8_on_idle: UFS Hibern8 on idle related data
  * @desc_size: descriptor sizes reported by device
  * @urgent_bkops_lvl: keeps track of urgent bkops level for device
  * @is_urgent_bkops_lvl_checked: keeps track if the urgent bkops level for
@@ -670,6 +700,8 @@ struct ufs_hba {
 	struct ufs_pwr_mode_info max_pwr_info;
 
 	struct ufs_clk_gating clk_gating;
+	struct ufs_hibern8_on_idle hibern8_on_idle;
+
 	/* Control to enable/disable host capabilities */
 	u32 caps;
 	/* Allow dynamic clk gating */
@@ -686,6 +718,9 @@ struct ufs_hba {
 	 * CAUTION: Enabling this might reduce overall UFS throughput.
 	 */
 #define UFSHCD_CAP_INTR_AGGR (1 << 4)
+	/* Allow standalone Hibern8 enter on idle */
+#define UFSHCD_CAP_HIBERN8_ENTER_ON_IDLE (1 << 5)
+
 	/*
 	 * This capability allows the device auto-bkops to be always enabled
 	 * except during suspend (both runtime and suspend).
@@ -724,6 +759,10 @@ static inline bool ufshcd_can_autobkops_during_suspend(struct ufs_hba *hba)
 {
 	return hba->caps & UFSHCD_CAP_AUTO_BKOPS_SUSPEND;
 }
+static inline bool ufshcd_is_hibern8_on_idle_allowed(struct ufs_hba *hba)
+{
+	return hba->caps & UFSHCD_CAP_HIBERN8_ENTER_ON_IDLE;
+}
 
 static inline bool ufshcd_is_intr_aggr_allowed(struct ufs_hba *hba)
 {
diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
index bf6f826..66ec728 100644
--- a/include/trace/events/ufs.h
+++ b/include/trace/events/ufs.h
@@ -75,6 +75,26 @@
 		__print_symbolic(__entry->state, UFSCHD_CLK_GATING_STATES))
 );
 
+TRACE_EVENT(ufshcd_hibern8_on_idle,
+
+	TP_PROTO(const char *dev_name, const char *state),
+
+	TP_ARGS(dev_name, state),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name)
+		__string(state, state)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name, dev_name);
+		__assign_str(state, state);
+	),
+
+	TP_printk("%s: state changed to %s",
+		__get_str(dev_name), __get_str(state))
+);
+
 TRACE_EVENT(ufshcd_clk_scaling,
 
 	TP_PROTO(const char *dev_name, const char *state, const char *clk,
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v1 6/9] scsi: ufs: optimize clock, pm_qos, hibern8 handling in queuecommand

[Asutosh Das]

From: Subhash Jadavani <subhashj@codeaurora.org>

ufshcd_queuecommand() vote for the resources in this order: clocks,
pm_qos latency, hibern8 exit. If any of these votes are not already
applied, each one has to be applied asynchronously and in that case we are
releasing all the previously applied resource votes (for example, if
hibern8 exit has to be completed asynchronously, we release the votes for
pm_qos and clocks as well). This is not a optimal solution instead we
should skip scheduling the unvoting work for already voted resources.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/ufs/ufs-qcom.c |  2 +-
 drivers/scsi/ufs/ufshcd.c   | 33 +++++++++++++++++----------------
 drivers/scsi/ufs/ufshcd.h   |  2 +-
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index 221820a..fa01924 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -1605,7 +1605,7 @@ int ufs_qcom_testbus_config(struct ufs_qcom_host *host)
 	 * committed before returning.
 	 */
 	mb();
-	ufshcd_release(host->hba);
+	ufshcd_release(host->hba, false);
 	pm_runtime_put_sync(host->hba->dev);
 
 	return 0;
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8a56ef6..40d9c35 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1175,7 +1175,7 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, bool scale_up)
 
 out:
 	ufshcd_clock_scaling_unprepare(hba);
-	ufshcd_release(hba);
+	ufshcd_release_all(hba);
 	return ret;
 }
 
@@ -1447,7 +1447,7 @@ static ssize_t ufshcd_clkscale_enable_store(struct device *dev,
 					__func__, err);
 	}
 
-	ufshcd_release(hba);
+	ufshcd_release(hba, false);
 	pm_runtime_put_sync(hba->dev);
 out:
 	return count;
@@ -1662,7 +1662,7 @@ static void ufshcd_gate_work(struct work_struct *work)
 }
 
 /* host lock must be held before calling this variant */
-static void __ufshcd_release(struct ufs_hba *hba)
+static void __ufshcd_release(struct ufs_hba *hba, bool no_sched)
 {
 	if (!ufshcd_is_clkgating_allowed(hba))
 		return;
@@ -1673,7 +1673,7 @@ static void __ufshcd_release(struct ufs_hba *hba)
 		|| hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL
 		|| hba->lrb_in_use || hba->outstanding_tasks
 		|| hba->active_uic_cmd || hba->uic_async_done
-		|| ufshcd_eh_in_progress(hba))
+		|| ufshcd_eh_in_progress(hba) || no_sched)
 		return;
 
 	hba->clk_gating.state = REQ_CLKS_OFF;
@@ -1682,12 +1682,12 @@ static void __ufshcd_release(struct ufs_hba *hba)
 			msecs_to_jiffies(hba->clk_gating.delay_ms));
 }
 
-void ufshcd_release(struct ufs_hba *hba)
+void ufshcd_release(struct ufs_hba *hba, bool no_sched)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(hba->host->host_lock, flags);
-	__ufshcd_release(hba);
+	__ufshcd_release(hba, no_sched);
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 }
 EXPORT_SYMBOL_GPL(ufshcd_release);
@@ -1738,7 +1738,7 @@ static ssize_t ufshcd_clkgate_enable_store(struct device *dev,
 		goto out;
 
 	if (value) {
-		ufshcd_release(hba);
+		ufshcd_release(hba, false);
 	} else {
 		spin_lock_irqsave(hba->host->host_lock, flags);
 		hba->clk_gating.active_reqs++;
@@ -1870,7 +1870,7 @@ int ufshcd_hibern8_hold(struct ufs_hba *hba, bool async)
 }
 
 /* host lock must be held before calling this variant */
-static void __ufshcd_hibern8_release(struct ufs_hba *hba)
+static void __ufshcd_hibern8_release(struct ufs_hba *hba, bool no_sched)
 {
 	unsigned long delay_in_jiffies;
 
@@ -1884,7 +1884,8 @@ static void __ufshcd_hibern8_release(struct ufs_hba *hba)
 		|| hba->hibern8_on_idle.is_suspended
 		|| hba->ufshcd_state != UFSHCD_STATE_OPERATIONAL
 		|| hba->lrb_in_use || hba->outstanding_tasks
-		|| hba->active_uic_cmd || hba->uic_async_done)
+		|| hba->active_uic_cmd || hba->uic_async_done
+		|| ufshcd_eh_in_progress(hba) || no_sched)
 		return;
 
 	hba->hibern8_on_idle.state = REQ_HIBERN8_ENTER;
@@ -1907,12 +1908,12 @@ static void __ufshcd_hibern8_release(struct ufs_hba *hba)
 			      delay_in_jiffies);
 }
 
-void ufshcd_hibern8_release(struct ufs_hba *hba)
+void ufshcd_hibern8_release(struct ufs_hba *hba, bool no_sched)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(hba->host->host_lock, flags);
-	__ufshcd_hibern8_release(hba);
+	__ufshcd_hibern8_release(hba, no_sched);
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 }
 
@@ -2034,8 +2035,8 @@ static void ufshcd_hold_all(struct ufs_hba *hba)
 
 static void ufshcd_release_all(struct ufs_hba *hba)
 {
-	ufshcd_hibern8_release(hba);
-	ufshcd_release(hba);
+	ufshcd_hibern8_release(hba, false);
+	ufshcd_release(hba, false);
 }
 
 /* Must be called with host lock acquired */
@@ -2681,7 +2682,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd)
 	if (err) {
 		clear_bit_unlock(tag, &hba->lrb_in_use);
 		err = SCSI_MLQUEUE_HOST_BUSY;
-		ufshcd_release(hba);
+		ufshcd_release(hba, true);
 		goto out;
 	}
 	if (ufshcd_is_hibern8_on_idle_allowed(hba))
@@ -5050,8 +5051,8 @@ static void __ufshcd_transfer_req_compl(struct ufs_hba *hba,
 			clear_bit_unlock(index, &hba->lrb_in_use);
 			/* Do not touch lrbp after scsi done */
 			cmd->scsi_done(cmd);
-			__ufshcd_release(hba);
-			__ufshcd_hibern8_release(hba);
+			__ufshcd_release(hba, false);
+			__ufshcd_hibern8_release(hba, false);
 		} else if (lrbp->command_type == UTP_CMD_TYPE_DEV_MANAGE ||
 			lrbp->command_type == UTP_CMD_TYPE_UFS_STORAGE) {
 			if (hba->dev_cmd.complete) {
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index eaccc76..f79a639 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -927,7 +927,7 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index,
 			    u8 *buf, u32 size, bool ascii);
 
 int ufshcd_hold(struct ufs_hba *hba, bool async);
-void ufshcd_release(struct ufs_hba *hba);
+void ufshcd_release(struct ufs_hba *hba, bool no_sched);
 
 int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id,
 	int *desc_length);
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v1 7/9] scsi: ufs: add UFS power collapse support during hibern8

[Asutosh Das]

From: Subhash Jadavani <subhashj@codeaurora.org>

UFS host controller hardware may allow the host controller
to be power collapsed when UFS link is hibern8 state, this
change allows the UFS host controller to be power collapsed
during hibern8.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c |  8 ++++++--
 drivers/scsi/ufs/ufshcd.h | 13 ++++++++++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 40d9c35..50588cf 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7673,13 +7673,17 @@ static int ufshcd_vreg_set_hpm(struct ufs_hba *hba)
 
 static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba)
 {
-	if (ufshcd_is_link_off(hba))
+	if (ufshcd_is_link_off(hba) ||
+	    (ufshcd_is_link_hibern8(hba)
+	     && ufshcd_is_power_collapse_during_hibern8_allowed(hba)))
 		ufshcd_setup_hba_vreg(hba, false);
 }
 
 static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba)
 {
-	if (ufshcd_is_link_off(hba))
+	if (ufshcd_is_link_off(hba) ||
+	    (ufshcd_is_link_hibern8(hba)
+	     && ufshcd_is_power_collapse_during_hibern8_allowed(hba)))
 		ufshcd_setup_hba_vreg(hba, true);
 }
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index f79a639..8c5f987 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -728,7 +728,12 @@ struct ufs_hba {
 	 * to do background operation when it's active but it might degrade
 	 * the performance of ongoing read/write operations.
 	 */
-#define UFSHCD_CAP_KEEP_AUTO_BKOPS_ENABLED_EXCEPT_SUSPEND (1 << 5)
+#define UFSHCD_CAP_KEEP_AUTO_BKOPS_ENABLED_EXCEPT_SUSPEND (1 << 6)
+	/*
+	 * If host controller hardware can be power collapsed when UFS link is
+	 * in hibern8 then enable this cap.
+	 */
+#define UFSHCD_CAP_POWER_COLLAPSE_DURING_HIBERN8 (1 << 7)
 
 	struct devfreq *devfreq;
 	struct ufs_clk_scaling clk_scaling;
@@ -764,6 +769,12 @@ static inline bool ufshcd_is_hibern8_on_idle_allowed(struct ufs_hba *hba)
 	return hba->caps & UFSHCD_CAP_HIBERN8_ENTER_ON_IDLE;
 }
 
+static inline bool ufshcd_is_power_collapse_during_hibern8_allowed(
+						struct ufs_hba *hba)
+{
+	return !!(hba->caps & UFSHCD_CAP_POWER_COLLAPSE_DURING_HIBERN8);
+}
+
 static inline bool ufshcd_is_intr_aggr_allowed(struct ufs_hba *hba)
 {
 /* DWC UFS Core has the Interrupt aggregation feature but is not detectable*/
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v1 7/9] scsi: ufs: add UFS power collapse support during hibern8

[Subhash Jadavani]

On 2018-07-06 05:30, Asutosh Das wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
> 
> UFS host controller hardware may allow the host controller
> to be power collapsed when UFS link is hibern8 state, this
> change allows the UFS host controller to be power collapsed
> during hibern8.
> 
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufshcd.c |  8 ++++++--
>  drivers/scsi/ufs/ufshcd.h | 13 ++++++++++++-
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 40d9c35..50588cf 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7673,13 +7673,17 @@ static int ufshcd_vreg_set_hpm(struct ufs_hba 
> *hba)
> 
>  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba)
>  {
> -	if (ufshcd_is_link_off(hba))
> +	if (ufshcd_is_link_off(hba) ||
> +	    (ufshcd_is_link_hibern8(hba)
> +	     && ufshcd_is_power_collapse_during_hibern8_allowed(hba)))
>  		ufshcd_setup_hba_vreg(hba, false);

I guess we have to handle the UFS host controller power collapse via 
"power-domains" which would requires us to specify the "power-domains" 
attribute under UFS controller's DT node and then linux core power 
framework should automatically collapse the UFS controller post runtime 
suspend. This also means we can't power collapse UFS controller 
aggressively (as part of clock gating) but it should still be fine from 
power point of view.

>  }
> 
>  static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba)
>  {
> -	if (ufshcd_is_link_off(hba))
> +	if (ufshcd_is_link_off(hba) ||
> +	    (ufshcd_is_link_hibern8(hba)
> +	     && ufshcd_is_power_collapse_during_hibern8_allowed(hba)))
>  		ufshcd_setup_hba_vreg(hba, true);
>  }
> 
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index f79a639..8c5f987 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -728,7 +728,12 @@ struct ufs_hba {
>  	 * to do background operation when it's active but it might degrade
>  	 * the performance of ongoing read/write operations.
>  	 */
> -#define UFSHCD_CAP_KEEP_AUTO_BKOPS_ENABLED_EXCEPT_SUSPEND (1 << 5)
> +#define UFSHCD_CAP_KEEP_AUTO_BKOPS_ENABLED_EXCEPT_SUSPEND (1 << 6)
> +	/*
> +	 * If host controller hardware can be power collapsed when UFS link 
> is
> +	 * in hibern8 then enable this cap.
> +	 */
> +#define UFSHCD_CAP_POWER_COLLAPSE_DURING_HIBERN8 (1 << 7)
> 
>  	struct devfreq *devfreq;
>  	struct ufs_clk_scaling clk_scaling;
> @@ -764,6 +769,12 @@ static inline bool
> ufshcd_is_hibern8_on_idle_allowed(struct ufs_hba *hba)
>  	return hba->caps & UFSHCD_CAP_HIBERN8_ENTER_ON_IDLE;
>  }
> 
> +static inline bool ufshcd_is_power_collapse_during_hibern8_allowed(
> +						struct ufs_hba *hba)
> +{
> +	return !!(hba->caps & UFSHCD_CAP_POWER_COLLAPSE_DURING_HIBERN8);
> +}
> +
>  static inline bool ufshcd_is_intr_aggr_allowed(struct ufs_hba *hba)
>  {
>  /* DWC UFS Core has the Interrupt aggregation feature but is not 
> detectable*/

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

Re: [PATCH v1 7/9] scsi: ufs: add UFS power collapse support during hibern8

[Asutosh Das (asd)]

On 7/21/2018 5:28 AM, Subhash Jadavani wrote:
> On 2018-07-06 05:30, Asutosh Das wrote:
>> From: Subhash Jadavani <subhashj@codeaurora.org>
>>
>> UFS host controller hardware may allow the host controller
>> to be power collapsed when UFS link is hibern8 state, this
>> change allows the UFS host controller to be power collapsed
>> during hibern8.
>>
>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> ---
>>  drivers/scsi/ufs/ufshcd.c |  8 ++++++--
>>  drivers/scsi/ufs/ufshcd.h | 13 ++++++++++++-
>>  2 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 40d9c35..50588cf 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -7673,13 +7673,17 @@ static int ufshcd_vreg_set_hpm(struct ufs_hba 
>> *hba)
>>
>>  static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba)
>>  {
>> -    if (ufshcd_is_link_off(hba))
>> +    if (ufshcd_is_link_off(hba) ||
>> +        (ufshcd_is_link_hibern8(hba)
>> +         && ufshcd_is_power_collapse_during_hibern8_allowed(hba)))
>>          ufshcd_setup_hba_vreg(hba, false);
> 
> I guess we have to handle the UFS host controller power collapse via 
> "power-domains" which would requires us to specify the "power-domains" 
> attribute under UFS controller's DT node and then linux core power 
> framework should automatically collapse the UFS controller post runtime 
> suspend. This also means we can't power collapse UFS controller 
> aggressively (as part of clock gating) but it should still be fine from 
> power point of view.
> 
>>  }
>>
>>  static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba)
>>  {
>> -    if (ufshcd_is_link_off(hba))
>> +    if (ufshcd_is_link_off(hba) ||
>> +        (ufshcd_is_link_hibern8(hba)
>> +         && ufshcd_is_power_collapse_during_hibern8_allowed(hba)))
>>          ufshcd_setup_hba_vreg(hba, true);
>>  }
>>
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index f79a639..8c5f987 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -728,7 +728,12 @@ struct ufs_hba {
>>       * to do background operation when it's active but it might degrade
>>       * the performance of ongoing read/write operations.
>>       */
>> -#define UFSHCD_CAP_KEEP_AUTO_BKOPS_ENABLED_EXCEPT_SUSPEND (1 << 5)
>> +#define UFSHCD_CAP_KEEP_AUTO_BKOPS_ENABLED_EXCEPT_SUSPEND (1 << 6)
>> +    /*
>> +     * If host controller hardware can be power collapsed when UFS 
>> link is
>> +     * in hibern8 then enable this cap.
>> +     */
>> +#define UFSHCD_CAP_POWER_COLLAPSE_DURING_HIBERN8 (1 << 7)
>>
>>      struct devfreq *devfreq;
>>      struct ufs_clk_scaling clk_scaling;
>> @@ -764,6 +769,12 @@ static inline bool
>> ufshcd_is_hibern8_on_idle_allowed(struct ufs_hba *hba)
>>      return hba->caps & UFSHCD_CAP_HIBERN8_ENTER_ON_IDLE;
>>  }
>>
>> +static inline bool ufshcd_is_power_collapse_during_hibern8_allowed(
>> +                        struct ufs_hba *hba)
>> +{
>> +    return !!(hba->caps & UFSHCD_CAP_POWER_COLLAPSE_DURING_HIBERN8);
>> +}
>> +
>>  static inline bool ufshcd_is_intr_aggr_allowed(struct ufs_hba *hba)
>>  {
>>  /* DWC UFS Core has the Interrupt aggregation feature but is not 
>> detectable*/
> 

Thanks. I'll check the 'power-domains' part and get back with the changes.


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

[PATCH v1 8/9] scsi: ufs: enable runtime pm only after ufshcd init

[Asutosh Das]

From: Gilad Broner <gbroner@codeaurora.org>

Previous code enables runtime pm before ufshcd_init() is completed,
and before the hba struct is stored in the platform device private
data. This means that pm runtime calls will have null hba pointer
as well as partially initialized driver.
Instead, enable pm runtime only after ufshcd_init() is done and
after hba struct is stored in the platform device private data.

Signed-off-by: Gilad Broner <gbroner@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd-pltfrm.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 7dba799..8332d99 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -354,24 +354,21 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 	}
 
 	ufshcd_parse_pm_levels(hba);
-	pm_runtime_set_active(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
 
 	ufshcd_init_lanes_per_dir(hba);
 
 	err = ufshcd_init(hba, mmio_base, irq);
 	if (err) {
 		dev_err(dev, "Initialization failed\n");
-		goto out_disable_rpm;
+		goto dealloc_host;
 	}
 
 	platform_set_drvdata(pdev, hba);
 
-	return 0;
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
 
-out_disable_rpm:
-	pm_runtime_disable(&pdev->dev);
-	pm_runtime_set_suspended(&pdev->dev);
+	return 0;
 dealloc_host:
 	ufshcd_dealloc_host(hba);
 out:
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v1 8/9] scsi: ufs: enable runtime pm only after ufshcd init

[Subhash Jadavani]

On 2018-07-06 05:30, Asutosh Das wrote:
> From: Gilad Broner <gbroner@codeaurora.org>
> 
> Previous code enables runtime pm before ufshcd_init() is completed,
> and before the hba struct is stored in the platform device private
> data. This means that pm runtime calls will have null hba pointer
> as well as partially initialized driver.
> Instead, enable pm runtime only after ufshcd_init() is done and
> after hba struct is stored in the platform device private data.
> 
> Signed-off-by: Gilad Broner <gbroner@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufshcd-pltfrm.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c 
> b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index 7dba799..8332d99 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -354,24 +354,21 @@ int ufshcd_pltfrm_init(struct platform_device 
> *pdev,
>  	}
> 
>  	ufshcd_parse_pm_levels(hba);
> -	pm_runtime_set_active(&pdev->dev);
> -	pm_runtime_enable(&pdev->dev);
> 
>  	ufshcd_init_lanes_per_dir(hba);
> 
>  	err = ufshcd_init(hba, mmio_base, irq);
>  	if (err) {
>  		dev_err(dev, "Initialization failed\n");
> -		goto out_disable_rpm;
> +		goto dealloc_host;
>  	}
> 
>  	platform_set_drvdata(pdev, hba);
> 
> -	return 0;
> +	pm_runtime_set_active(&pdev->dev);
> +	pm_runtime_enable(&pdev->dev);
> 
> -out_disable_rpm:
> -	pm_runtime_disable(&pdev->dev);
> -	pm_runtime_set_suspended(&pdev->dev);
> +	return 0;
>  dealloc_host:
>  	ufshcd_dealloc_host(hba);
>  out:

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 v1 9/9] scsi: ufs: enable FASTAUTO mode during low load condition

[Asutosh Das]

From: Subhash Jadavani <subhashj@codeaurora.org>

We are currently running UFS link in HS-G3 FAST mode during high load
condition for best possible performance and in HS-G2 FAST mode during
low load condition to save power. As we are anyway scaling down from
HS-G3 to HS-G2, we can also change the mode from FAST to FASTAUTO.
So we looked at the performance numbers with HS-G2 FASTAUTO mode and
they are good enough for most of the low bandwidth usecases. But Samsung
UFS memory devices are exception which has really low sequential read
throughput in FAST AUTO mode hence we will only be enabling FAST AUTO mode
for other UFS device vendors.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 50588cf..a6e43f9 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1088,6 +1088,10 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, bool scale_up)
 			/* scale down gear */
 			new_pwr_info.gear_tx = UFS_MIN_GEAR_TO_SCALE_DOWN;
 			new_pwr_info.gear_rx = UFS_MIN_GEAR_TO_SCALE_DOWN;
+			if (!(hba->dev_quirks & UFS_DEVICE_NO_FASTAUTO)) {
+				new_pwr_info.pwr_tx = FASTAUTO_MODE;
+				new_pwr_info.pwr_rx = FASTAUTO_MODE;
+			}
 		}
 	}
 
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.