LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2 0/2] Fix UFS and devfreq interaction
@ 2018-05-04 22:44 Bjorn Andersson
  2018-05-04 22:44 ` [PATCH v2 1/2] scsi: ufs: Extract devfreq registration Bjorn Andersson
  2018-05-04 22:44 ` [PATCH v2 2/2] scsi: ufs: Use freq table with devfreq Bjorn Andersson
  0 siblings, 2 replies; 6+ messages in thread
From: Bjorn Andersson @ 2018-05-04 22:44 UTC (permalink / raw)
  To: Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen
  Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, linux-scsi, linux-pm,
	linux-kernel, linux-arm-msm, Vivek Gautam

With the introduction of f1d981eaecf8 ("PM / devfreq: Use the available min/max
frequency") the UFS host controller driver (UFSHCD) stopped probing for
platforms that supports frequency scaling, e.g. all modern Qualcomm platforms.

The cause of this was UFSHCD's reliance of not registering any frequencies and
then being called by devfreq to switch between the frequencies 0 and UINT_MAX.

The devfreq code implies that the client is able to pass the frequency table,
instead of relying on opp tables, but as concluded after v1 this is not
compliant with devfreq cooling, which will enable and disable opp entries in
order to limit the valid frequencies. So instead the UFSHCD driver is modified
to read the freq-table and register the first clock's two rates as the two
available opp levels.


This follows the first patch which facilitates the implementation of this in a
clean fashion, and removes the kernel panic which previously happened when
devfreq initialization failed.


With this UFS is once again functional on the db820c, and is needed to get UFS
working on SDM845 (both tested).

Bjorn Andersson (2):
  scsi: ufs: Extract devfreq registration
  scsi: ufs: Use freq table with devfreq

 drivers/scsi/ufs/ufshcd.c | 76 +++++++++++++++++++++++++++++++--------
 1 file changed, 61 insertions(+), 15 deletions(-)

-- 
2.17.0

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

* [PATCH v2 1/2] scsi: ufs: Extract devfreq registration
  2018-05-04 22:44 [PATCH v2 0/2] Fix UFS and devfreq interaction Bjorn Andersson
@ 2018-05-04 22:44 ` Bjorn Andersson
  2018-05-08  1:59   ` Chanwoo Choi
  2018-05-04 22:44 ` [PATCH v2 2/2] scsi: ufs: Use freq table with devfreq Bjorn Andersson
  1 sibling, 1 reply; 6+ messages in thread
From: Bjorn Andersson @ 2018-05-04 22:44 UTC (permalink / raw)
  To: Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen
  Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, linux-scsi, linux-pm,
	linux-kernel, linux-arm-msm, Vivek Gautam

Failing to register with devfreq leaves hba->devfreq assigned, which
causes the error path to dereference the ERR_PTR(). Rather than bolting
on more conditionals, move the call of devm_devfreq_add_device() into
it's own function and only update hba->devfreq once it's successfully
registered.

The subsequent patch builds upon this to make UFS actually work again,
as it's been broken since f1d981eaecf8 ("PM / devfreq: Use the available
min/max frequency")

Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- None

 drivers/scsi/ufs/ufshcd.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8f22a980b1a7..2253f24309ec 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1255,6 +1255,26 @@ static struct devfreq_dev_profile ufs_devfreq_profile = {
 	.get_dev_status	= ufshcd_devfreq_get_dev_status,
 };
 
+static int ufshcd_devfreq_init(struct ufs_hba *hba)
+{
+	struct devfreq *devfreq;
+	int ret;
+
+	devfreq = devm_devfreq_add_device(hba->dev,
+			&ufs_devfreq_profile,
+			"simple_ondemand",
+			NULL);
+	if (IS_ERR(devfreq)) {
+		ret = PTR_ERR(devfreq);
+		dev_err(hba->dev, "Unable to register with devfreq %d\n", ret);
+		return ret;
+	}
+
+	hba->devfreq = devfreq;
+
+	return 0;
+}
+
 static void __ufshcd_suspend_clkscaling(struct ufs_hba *hba)
 {
 	unsigned long flags;
@@ -6399,16 +6419,9 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
 				sizeof(struct ufs_pa_layer_attr));
 			hba->clk_scaling.saved_pwr_info.is_valid = true;
 			if (!hba->devfreq) {
-				hba->devfreq = devm_devfreq_add_device(hba->dev,
-							&ufs_devfreq_profile,
-							"simple_ondemand",
-							NULL);
-				if (IS_ERR(hba->devfreq)) {
-					ret = PTR_ERR(hba->devfreq);
-					dev_err(hba->dev, "Unable to register with devfreq %d\n",
-							ret);
+				ret = ufshcd_devfreq_init(hba);
+				if (ret)
 					goto out;
-				}
 			}
 			hba->clk_scaling.is_allowed = true;
 		}
-- 
2.17.0

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

* [PATCH v2 2/2] scsi: ufs: Use freq table with devfreq
  2018-05-04 22:44 [PATCH v2 0/2] Fix UFS and devfreq interaction Bjorn Andersson
  2018-05-04 22:44 ` [PATCH v2 1/2] scsi: ufs: Extract devfreq registration Bjorn Andersson
@ 2018-05-04 22:44 ` Bjorn Andersson
  2018-05-08  2:51   ` Chanwoo Choi
  2018-05-16 21:20   ` Subhash Jadavani
  1 sibling, 2 replies; 6+ messages in thread
From: Bjorn Andersson @ 2018-05-04 22:44 UTC (permalink / raw)
  To: Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen
  Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, linux-scsi, linux-pm,
	linux-kernel, linux-arm-msm, Vivek Gautam

devfreq requires that the client operates on actual frequencies, not
only 0 and UMAX_INT and as such UFS brok with the introduction of
f1d981eaecf8 ("PM / devfreq: Use the available min/max frequency").

This patch registers the frequencies of the first clock as opp levels
and use these to determine if we're trying to step up or down.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Chances since v1:
- Register min_freq and max_freq as opp levels.
- Unregister opp levels on removal, to make e.g. probe defer working

 drivers/scsi/ufs/ufshcd.c | 47 +++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 2253f24309ec..257614b889bd 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1168,16 +1168,13 @@ static int ufshcd_devfreq_target(struct device *dev,
 	struct ufs_hba *hba = dev_get_drvdata(dev);
 	ktime_t start;
 	bool scale_up, sched_clk_scaling_suspend_work = false;
+	struct list_head *clk_list = &hba->clk_list_head;
+	struct ufs_clk_info *clki;
 	unsigned long irq_flags;
 
 	if (!ufshcd_is_clkscaling_supported(hba))
 		return -EINVAL;
 
-	if ((*freq > 0) && (*freq < UINT_MAX)) {
-		dev_err(hba->dev, "%s: invalid freq = %lu\n", __func__, *freq);
-		return -EINVAL;
-	}
-
 	spin_lock_irqsave(hba->host->host_lock, irq_flags);
 	if (ufshcd_eh_in_progress(hba)) {
 		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
@@ -1187,7 +1184,13 @@ static int ufshcd_devfreq_target(struct device *dev,
 	if (!hba->clk_scaling.active_reqs)
 		sched_clk_scaling_suspend_work = true;
 
-	scale_up = (*freq == UINT_MAX) ? true : false;
+	if (list_empty(clk_list)) {
+		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
+		goto out;
+	}
+
+	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
+	scale_up = (*freq == clki->max_freq) ? true : false;
 	if (!ufshcd_is_devfreq_scaling_required(hba, scale_up)) {
 		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
 		ret = 0;
@@ -1257,16 +1260,29 @@ static struct devfreq_dev_profile ufs_devfreq_profile = {
 
 static int ufshcd_devfreq_init(struct ufs_hba *hba)
 {
+	struct list_head *clk_list = &hba->clk_list_head;
+	struct ufs_clk_info *clki;
 	struct devfreq *devfreq;
 	int ret;
 
-	devfreq = devm_devfreq_add_device(hba->dev,
+	/* Skip devfreq if we don't have any clocks in the list */
+	if (list_empty(clk_list))
+		return 0;
+
+	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
+	dev_pm_opp_add(hba->dev, clki->min_freq, 0);
+	dev_pm_opp_add(hba->dev, clki->max_freq, 0);
+
+	devfreq = devfreq_add_device(hba->dev,
 			&ufs_devfreq_profile,
 			"simple_ondemand",
 			NULL);
 	if (IS_ERR(devfreq)) {
 		ret = PTR_ERR(devfreq);
 		dev_err(hba->dev, "Unable to register with devfreq %d\n", ret);
+
+		dev_pm_opp_remove(hba->dev, clki->min_freq);
+		dev_pm_opp_remove(hba->dev, clki->max_freq);
 		return ret;
 	}
 
@@ -1275,6 +1291,22 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
 	return 0;
 }
 
+static void ufshcd_devfreq_remove(struct ufs_hba *hba)
+{
+	struct list_head *clk_list = &hba->clk_list_head;
+	struct ufs_clk_info *clki;
+
+	if (!hba->devfreq)
+		return;
+
+	devfreq_remove_device(hba->devfreq);
+	hba->devfreq = NULL;
+
+	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
+	dev_pm_opp_remove(hba->dev, clki->min_freq);
+	dev_pm_opp_remove(hba->dev, clki->max_freq);
+}
+
 static void __ufshcd_suspend_clkscaling(struct ufs_hba *hba)
 {
 	unsigned long flags;
@@ -6966,6 +6998,7 @@ static void ufshcd_hba_exit(struct ufs_hba *hba)
 			if (hba->devfreq)
 				ufshcd_suspend_clkscaling(hba);
 			destroy_workqueue(hba->clk_scaling.workq);
+			ufshcd_devfreq_remove(hba);
 		}
 		ufshcd_setup_clocks(hba, false);
 		ufshcd_setup_hba_vreg(hba, false);
-- 
2.17.0

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

* Re: [PATCH v2 1/2] scsi: ufs: Extract devfreq registration
  2018-05-04 22:44 ` [PATCH v2 1/2] scsi: ufs: Extract devfreq registration Bjorn Andersson
@ 2018-05-08  1:59   ` Chanwoo Choi
  0 siblings, 0 replies; 6+ messages in thread
From: Chanwoo Choi @ 2018-05-08  1:59 UTC (permalink / raw)
  To: Bjorn Andersson, Vinayak Holikatti, Martin K. Petersen
  Cc: MyungJoo Ham, Kyungmin Park, linux-scsi, linux-pm, linux-kernel,
	linux-arm-msm, Vivek Gautam

Hi,

On 2018년 05월 05일 07:44, Bjorn Andersson wrote:
> Failing to register with devfreq leaves hba->devfreq assigned, which
> causes the error path to dereference the ERR_PTR(). Rather than bolting
> on more conditionals, move the call of devm_devfreq_add_device() into
> it's own function and only update hba->devfreq once it's successfully
> registered.
> 
> The subsequent patch builds upon this to make UFS actually work again,
> as it's been broken since f1d981eaecf8 ("PM / devfreq: Use the available
> min/max frequency")
> 
> Reviewed-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v1:
> - None
> 
>  drivers/scsi/ufs/ufshcd.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 8f22a980b1a7..2253f24309ec 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1255,6 +1255,26 @@ static struct devfreq_dev_profile ufs_devfreq_profile = {
>  	.get_dev_status	= ufshcd_devfreq_get_dev_status,
>  };
>  
> +static int ufshcd_devfreq_init(struct ufs_hba *hba)
> +{
> +	struct devfreq *devfreq;
> +	int ret;
> +
> +	devfreq = devm_devfreq_add_device(hba->dev,
> +			&ufs_devfreq_profile,
> +			"simple_ondemand",

You better to use 'DEVFREQ_GOV_SIMPLE_ONDEMAND' definition
in include/linux/devfreq.h in order to prevent the some typo.
- aa7c352f9841 ("PM / devfreq: Define the constant governor name")


> +			NULL);
> +	if (IS_ERR(devfreq)) {
> +		ret = PTR_ERR(devfreq);
> +		dev_err(hba->dev, "Unable to register with devfreq %d\n", ret);
> +		return ret;
> +	}
> +
> +	hba->devfreq = devfreq;
> +
> +	return 0;
> +}
> +
>  static void __ufshcd_suspend_clkscaling(struct ufs_hba *hba)
>  {
>  	unsigned long flags;
> @@ -6399,16 +6419,9 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
>  				sizeof(struct ufs_pa_layer_attr));
>  			hba->clk_scaling.saved_pwr_info.is_valid = true;
>  			if (!hba->devfreq) {
> -				hba->devfreq = devm_devfreq_add_device(hba->dev,
> -							&ufs_devfreq_profile,
> -							"simple_ondemand",
> -							NULL);
> -				if (IS_ERR(hba->devfreq)) {
> -					ret = PTR_ERR(hba->devfreq);
> -					dev_err(hba->dev, "Unable to register with devfreq %d\n",
> -							ret);
> +				ret = ufshcd_devfreq_init(hba);
> +				if (ret)
>  					goto out;
> -				}
>  			}
>  			hba->clk_scaling.is_allowed = true;
>  		}
> 

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

* Re: [PATCH v2 2/2] scsi: ufs: Use freq table with devfreq
  2018-05-04 22:44 ` [PATCH v2 2/2] scsi: ufs: Use freq table with devfreq Bjorn Andersson
@ 2018-05-08  2:51   ` Chanwoo Choi
  2018-05-16 21:20   ` Subhash Jadavani
  1 sibling, 0 replies; 6+ messages in thread
From: Chanwoo Choi @ 2018-05-08  2:51 UTC (permalink / raw)
  To: Bjorn Andersson, Vinayak Holikatti, Martin K. Petersen
  Cc: MyungJoo Ham, Kyungmin Park, linux-scsi, linux-pm, linux-kernel,
	linux-arm-msm, Vivek Gautam

Hi,

On 2018년 05월 05일 07:44, Bjorn Andersson wrote:
> devfreq requires that the client operates on actual frequencies, not
> only 0 and UMAX_INT and as such UFS brok with the introduction of
> f1d981eaecf8 ("PM / devfreq: Use the available min/max frequency").
> 
> This patch registers the frequencies of the first clock as opp levels
> and use these to determine if we're trying to step up or down.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Chances since v1:
> - Register min_freq and max_freq as opp levels.
> - Unregister opp levels on removal, to make e.g. probe defer working
> 
>  drivers/scsi/ufs/ufshcd.c | 47 +++++++++++++++++++++++++++++++++------
>  1 file changed, 40 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 2253f24309ec..257614b889bd 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1168,16 +1168,13 @@ static int ufshcd_devfreq_target(struct device *dev,
>  	struct ufs_hba *hba = dev_get_drvdata(dev);
>  	ktime_t start;
>  	bool scale_up, sched_clk_scaling_suspend_work = false;
> +	struct list_head *clk_list = &hba->clk_list_head;
> +	struct ufs_clk_info *clki;
>  	unsigned long irq_flags;
>  
>  	if (!ufshcd_is_clkscaling_supported(hba))
>  		return -EINVAL;
>  
> -	if ((*freq > 0) && (*freq < UINT_MAX)) {
> -		dev_err(hba->dev, "%s: invalid freq = %lu\n", __func__, *freq);
> -		return -EINVAL;
> -	}
> -
>  	spin_lock_irqsave(hba->host->host_lock, irq_flags);
>  	if (ufshcd_eh_in_progress(hba)) {
>  		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
> @@ -1187,7 +1184,13 @@ static int ufshcd_devfreq_target(struct device *dev,
>  	if (!hba->clk_scaling.active_reqs)
>  		sched_clk_scaling_suspend_work = true;
>  
> -	scale_up = (*freq == UINT_MAX) ? true : false;
> +	if (list_empty(clk_list)) {
> +		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
> +		goto out;
> +	}
> +
> +	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
> +	scale_up = (*freq == clki->max_freq) ? true : false;
>  	if (!ufshcd_is_devfreq_scaling_required(hba, scale_up)) {
>  		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
>  		ret = 0;
> @@ -1257,16 +1260,29 @@ static struct devfreq_dev_profile ufs_devfreq_profile = {
>  
>  static int ufshcd_devfreq_init(struct ufs_hba *hba)
>  {
> +	struct list_head *clk_list = &hba->clk_list_head;
> +	struct ufs_clk_info *clki;
>  	struct devfreq *devfreq;
>  	int ret;
>  
> -	devfreq = devm_devfreq_add_device(hba->dev,
> +	/* Skip devfreq if we don't have any clocks in the list */
> +	if (list_empty(clk_list))
> +		return 0;
> +
> +	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
> +	dev_pm_opp_add(hba->dev, clki->min_freq, 0);
> +	dev_pm_opp_add(hba->dev, clki->max_freq, 0);
> +
> +	devfreq = devfreq_add_device(hba->dev,
>  			&ufs_devfreq_profile,
>  			"simple_ondemand",
>  			NULL);
>  	if (IS_ERR(devfreq)) {
>  		ret = PTR_ERR(devfreq);
>  		dev_err(hba->dev, "Unable to register with devfreq %d\n", ret);
> +
> +		dev_pm_opp_remove(hba->dev, clki->min_freq);
> +		dev_pm_opp_remove(hba->dev, clki->max_freq);
>  		return ret;
>  	}
>  
> @@ -1275,6 +1291,22 @@ static int ufshcd_devfreq_init(struct ufs_hba *hba)
>  	return 0;
>  }
>  
> +static void ufshcd_devfreq_remove(struct ufs_hba *hba)
> +{
> +	struct list_head *clk_list = &hba->clk_list_head;
> +	struct ufs_clk_info *clki;
> +
> +	if (!hba->devfreq)
> +		return;
> +
> +	devfreq_remove_device(hba->devfreq);
> +	hba->devfreq = NULL;
> +
> +	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
> +	dev_pm_opp_remove(hba->dev, clki->min_freq);
> +	dev_pm_opp_remove(hba->dev, clki->max_freq);
> +}
> +
>  static void __ufshcd_suspend_clkscaling(struct ufs_hba *hba)
>  {
>  	unsigned long flags;
> @@ -6966,6 +6998,7 @@ static void ufshcd_hba_exit(struct ufs_hba *hba)
>  			if (hba->devfreq)
>  				ufshcd_suspend_clkscaling(hba);
>  			destroy_workqueue(hba->clk_scaling.workq);
> +			ufshcd_devfreq_remove(hba);
>  		}
>  		ufshcd_setup_clocks(hba, false);
>  		ufshcd_setup_hba_vreg(hba, false);
> 

For using OPP entries for devfreq:
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

* Re: [PATCH v2 2/2] scsi: ufs: Use freq table with devfreq
  2018-05-04 22:44 ` [PATCH v2 2/2] scsi: ufs: Use freq table with devfreq Bjorn Andersson
  2018-05-08  2:51   ` Chanwoo Choi
@ 2018-05-16 21:20   ` Subhash Jadavani
  1 sibling, 0 replies; 6+ messages in thread
From: Subhash Jadavani @ 2018-05-16 21:20 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Vinayak Holikatti, James E.J. Bottomley, Martin K. Petersen,
	MyungJoo Ham, Kyungmin Park, Chanwoo Choi, linux-scsi, linux-pm,
	linux-kernel, linux-arm-msm, Vivek Gautam, linux-scsi-owner

On 2018-05-04 15:44, Bjorn Andersson wrote:
> devfreq requires that the client operates on actual frequencies, not
> only 0 and UMAX_INT and as such UFS brok with the introduction of
> f1d981eaecf8 ("PM / devfreq: Use the available min/max frequency").
> 
> This patch registers the frequencies of the first clock as opp levels
> and use these to determine if we're trying to step up or down.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Chances since v1:
> - Register min_freq and max_freq as opp levels.
> - Unregister opp levels on removal, to make e.g. probe defer working
> 
>  drivers/scsi/ufs/ufshcd.c | 47 +++++++++++++++++++++++++++++++++------
>  1 file changed, 40 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 2253f24309ec..257614b889bd 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -1168,16 +1168,13 @@ static int ufshcd_devfreq_target(struct device 
> *dev,
>  	struct ufs_hba *hba = dev_get_drvdata(dev);
>  	ktime_t start;
>  	bool scale_up, sched_clk_scaling_suspend_work = false;
> +	struct list_head *clk_list = &hba->clk_list_head;
> +	struct ufs_clk_info *clki;
>  	unsigned long irq_flags;
> 
>  	if (!ufshcd_is_clkscaling_supported(hba))
>  		return -EINVAL;
> 
> -	if ((*freq > 0) && (*freq < UINT_MAX)) {
> -		dev_err(hba->dev, "%s: invalid freq = %lu\n", __func__, *freq);
> -		return -EINVAL;
> -	}
> -
>  	spin_lock_irqsave(hba->host->host_lock, irq_flags);
>  	if (ufshcd_eh_in_progress(hba)) {
>  		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
> @@ -1187,7 +1184,13 @@ static int ufshcd_devfreq_target(struct device 
> *dev,
>  	if (!hba->clk_scaling.active_reqs)
>  		sched_clk_scaling_suspend_work = true;
> 
> -	scale_up = (*freq == UINT_MAX) ? true : false;
> +	if (list_empty(clk_list)) {
> +		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
> +		goto out;
> +	}
> +
> +	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
> +	scale_up = (*freq == clki->max_freq) ? true : false;
>  	if (!ufshcd_is_devfreq_scaling_required(hba, scale_up)) {
>  		spin_unlock_irqrestore(hba->host->host_lock, irq_flags);
>  		ret = 0;
> @@ -1257,16 +1260,29 @@ static struct devfreq_dev_profile
> ufs_devfreq_profile = {
> 
>  static int ufshcd_devfreq_init(struct ufs_hba *hba)
>  {
> +	struct list_head *clk_list = &hba->clk_list_head;
> +	struct ufs_clk_info *clki;
>  	struct devfreq *devfreq;
>  	int ret;
> 
> -	devfreq = devm_devfreq_add_device(hba->dev,
> +	/* Skip devfreq if we don't have any clocks in the list */
> +	if (list_empty(clk_list))
> +		return 0;
> +
> +	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
> +	dev_pm_opp_add(hba->dev, clki->min_freq, 0);
> +	dev_pm_opp_add(hba->dev, clki->max_freq, 0);
> +
> +	devfreq = devfreq_add_device(hba->dev,
>  			&ufs_devfreq_profile,
>  			"simple_ondemand",
>  			NULL);
>  	if (IS_ERR(devfreq)) {
>  		ret = PTR_ERR(devfreq);
>  		dev_err(hba->dev, "Unable to register with devfreq %d\n", ret);
> +
> +		dev_pm_opp_remove(hba->dev, clki->min_freq);
> +		dev_pm_opp_remove(hba->dev, clki->max_freq);
>  		return ret;
>  	}
> 
> @@ -1275,6 +1291,22 @@ static int ufshcd_devfreq_init(struct ufs_hba 
> *hba)
>  	return 0;
>  }
> 
> +static void ufshcd_devfreq_remove(struct ufs_hba *hba)
> +{
> +	struct list_head *clk_list = &hba->clk_list_head;
> +	struct ufs_clk_info *clki;
> +
> +	if (!hba->devfreq)
> +		return;
> +
> +	devfreq_remove_device(hba->devfreq);
> +	hba->devfreq = NULL;
> +
> +	clki = list_first_entry(clk_list, struct ufs_clk_info, list);
> +	dev_pm_opp_remove(hba->dev, clki->min_freq);
> +	dev_pm_opp_remove(hba->dev, clki->max_freq);
> +}
> +
>  static void __ufshcd_suspend_clkscaling(struct ufs_hba *hba)
>  {
>  	unsigned long flags;
> @@ -6966,6 +6998,7 @@ static void ufshcd_hba_exit(struct ufs_hba *hba)
>  			if (hba->devfreq)
>  				ufshcd_suspend_clkscaling(hba);
>  			destroy_workqueue(hba->clk_scaling.workq);
> +			ufshcd_devfreq_remove(hba);
>  		}
>  		ufshcd_setup_clocks(hba, false);
>  		ufshcd_setup_hba_vreg(hba, false);

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

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

end of thread, other threads:[~2018-05-16 21:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04 22:44 [PATCH v2 0/2] Fix UFS and devfreq interaction Bjorn Andersson
2018-05-04 22:44 ` [PATCH v2 1/2] scsi: ufs: Extract devfreq registration Bjorn Andersson
2018-05-08  1:59   ` Chanwoo Choi
2018-05-04 22:44 ` [PATCH v2 2/2] scsi: ufs: Use freq table with devfreq Bjorn Andersson
2018-05-08  2:51   ` Chanwoo Choi
2018-05-16 21:20   ` Subhash Jadavani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).