Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 2/2] PM / devfreq: Use exclusively PM QoS to determine
 frequency limits
To:     Matthias Kaehlcke <mka@chromium.org>,
        MyungJoo Ham <myungjoo.ham@samsung.com>,
        Kyungmin Park <kyungmin.park@samsung.com>,
        Zhang Rui <rui.zhang@intel.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Amit Kucheria <amit.kucheria@verdurent.com>
Cc:     Leonard Crestez <leonard.crestez@nxp.com>,
        linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org
From:   Chanwoo Choi <cw00.choi@samsung.com>
Organization: Samsung Electronics
Message-ID: <c793c2e5-dd0e-bff4-9769-26344afe914e@samsung.com>
Date:   Mon, 13 Jan 2020 16:31:26 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101
        Thunderbird/59.0
MIME-Version: 1.0
In-Reply-To: <20200110094913.2.Ie8eacf976ce7a13e421592f5c1ab8dbdc537da5c@changeid>
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 101P
DLP-Filter: Pass
References: <20200110094913.1.I146403d05b9ec82f48b807efd416a57f545b447a@changeid>
        <CGME20200110174932epcas1p345b0e750b48cc9e351dca14e0dd4de86@epcas1p3.samsung.com>
        <20200110094913.2.Ie8eacf976ce7a13e421592f5c1ab8dbdc537da5c@changeid>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,


Any device driver except for devfreq_cooling.c might
use dev_pm_opp_enable/disable interface. 
So, don't need to remove the devfreq->scaling_max_freq
and devfreq->scaling_min_freq for supporting OPP interface.

Regards,
Chanwoo Choi

On 1/11/20 2:49 AM, Matthias Kaehlcke wrote:
> Traditionally devfreq cooling devices dynamically disabled OPPs
> that shouldn't be used because of thermal pressure. Devfreq cooling
> devices now use PM QoS to set frequency limits, hence the devfreq
> code dealing that deals with disabled OPPs can be removed.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> 
>  drivers/devfreq/devfreq.c | 75 +++++----------------------------------
>  include/linux/devfreq.h   |  4 ---
>  2 files changed, 8 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 57f6944d65a6..ec66e2c27cc4 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -73,34 +73,6 @@ static struct devfreq *find_device_devfreq(struct device *dev)
>  	return ERR_PTR(-ENODEV);
>  }
>  
> -static unsigned long find_available_min_freq(struct devfreq *devfreq)
> -{
> -	struct dev_pm_opp *opp;
> -	unsigned long min_freq = 0;
> -
> -	opp = dev_pm_opp_find_freq_ceil(devfreq->dev.parent, &min_freq);
> -	if (IS_ERR(opp))
> -		min_freq = 0;
> -	else
> -		dev_pm_opp_put(opp);
> -
> -	return min_freq;
> -}
> -
> -static unsigned long find_available_max_freq(struct devfreq *devfreq)
> -{
> -	struct dev_pm_opp *opp;
> -	unsigned long max_freq = ULONG_MAX;
> -
> -	opp = dev_pm_opp_find_freq_floor(devfreq->dev.parent, &max_freq);
> -	if (IS_ERR(opp))
> -		max_freq = 0;
> -	else
> -		dev_pm_opp_put(opp);
> -
> -	return max_freq;
> -}
> -
>  /**
>   * get_freq_range() - Get the current freq range
>   * @devfreq:	the devfreq instance
> @@ -141,10 +113,6 @@ static void get_freq_range(struct devfreq *devfreq,
>  		*max_freq = min(*max_freq,
>  				(unsigned long)HZ_PER_KHZ * qos_max_freq);
>  
> -	/* Apply constraints from OPP interface */
> -	*min_freq = max(*min_freq, devfreq->scaling_min_freq);
> -	*max_freq = min(*max_freq, devfreq->scaling_max_freq);
> -
>  	if (*min_freq > *max_freq)
>  		*min_freq = *max_freq;
>  }
> @@ -610,23 +578,10 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
>  				 void *devp)
>  {
>  	struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
> -	int err = -EINVAL;
> +	int err;
>  
>  	mutex_lock(&devfreq->lock);
> -
> -	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
> -	if (!devfreq->scaling_min_freq)
> -		goto out;
> -
> -	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
> -	if (!devfreq->scaling_max_freq) {
> -		devfreq->scaling_max_freq = ULONG_MAX;
> -		goto out;
> -	}
> -
>  	err = update_devfreq(devfreq);
> -
> -out:
>  	mutex_unlock(&devfreq->lock);
>  	if (err)
>  		dev_err(devfreq->dev.parent,
> @@ -781,19 +736,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  		mutex_lock(&devfreq->lock);
>  	}
>  
> -	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
> -	if (!devfreq->scaling_min_freq) {
> -		mutex_unlock(&devfreq->lock);
> -		err = -EINVAL;
> +	err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req,
> +				     DEV_PM_QOS_MIN_FREQUENCY, 0);
> +	if (err < 0)
>  		goto err_dev;
> -	}
> -
> -	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
> -	if (!devfreq->scaling_max_freq) {
> -		mutex_unlock(&devfreq->lock);
> -		err = -EINVAL;
> +	err = dev_pm_qos_add_request(dev, &devfreq->user_max_freq_req,
> +				     DEV_PM_QOS_MAX_FREQUENCY,
> +				     PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
> +	if (err < 0)
>  		goto err_dev;
> -	}
>  
>  	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
>  	atomic_set(&devfreq->suspend_count, 0);
> @@ -834,16 +785,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  
>  	mutex_unlock(&devfreq->lock);
>  
> -	err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req,
> -				     DEV_PM_QOS_MIN_FREQUENCY, 0);
> -	if (err < 0)
> -		goto err_devfreq;
> -	err = dev_pm_qos_add_request(dev, &devfreq->user_max_freq_req,
> -				     DEV_PM_QOS_MAX_FREQUENCY,
> -				     PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
> -	if (err < 0)
> -		goto err_devfreq;
> -
>  	devfreq->nb_min.notifier_call = qos_min_notifier_call;
>  	err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_min,
>  				      DEV_PM_QOS_MIN_FREQUENCY);
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index fb376b5b7281..cb75f23ad2f4 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -126,8 +126,6 @@ struct devfreq_dev_profile {
>   *		touch this.
>   * @user_min_freq_req:	PM QoS minimum frequency request from user (via sysfs)
>   * @user_max_freq_req:	PM QoS maximum frequency request from user (via sysfs)
> - * @scaling_min_freq:	Limit minimum frequency requested by OPP interface
> - * @scaling_max_freq:	Limit maximum frequency requested by OPP interface
>   * @stop_polling:	 devfreq polling status of a device.
>   * @suspend_freq:	 frequency of a device set during suspend phase.
>   * @resume_freq:	 frequency of a device set in resume phase.
> @@ -166,8 +164,6 @@ struct devfreq {
>  
>  	struct dev_pm_qos_request user_min_freq_req;
>  	struct dev_pm_qos_request user_max_freq_req;
> -	unsigned long scaling_min_freq;
> -	unsigned long scaling_max_freq;
>  	bool stop_polling;
>  
>  	unsigned long suspend_freq;
>