From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762646AbdJRCMq (ORCPT ); Tue, 17 Oct 2017 22:12:46 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:53682 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759195AbdJRCMn (ORCPT ); Tue, 17 Oct 2017 22:12:43 -0400 X-AuditID: b6c32a46-acbff70000001153-2d-59e6b898c70e MIME-version: 1.0 Content-transfer-encoding: 8BIT Content-type: text/plain; charset="utf-8" Message-id: <59E6B89A.4060901@samsung.com> Date: Wed, 18 Oct 2017 11:12:42 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: myungjoo.ham@gmail.com Cc: Kyungmin Park , "Rafael J . Wysocki" , chanwoo@kernel.org, =?UTF-8?B?64yA7J246riw?= , LKML , Linux PM list Subject: Re: [PATCH v4 3/8] PM / devfreq: Use the available min/max frequency In-reply-to: <59E6AED6.70703@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrDKsWRmVeSWpSXmKPExsWy7bCmme7MHc8iDSa/5raYeOMKi8Wk+xNY LM42vWG3uLxrDpvF594jjBbPN3WyWjxe8Zbdgd1j56y77B6L97xk8ti0qpPNo2/LKkaPz5vk AlijUm0yUhNTUosUUvOS81My89JtlbyD453jTc0MDHUNLS3MlRTyEnNTbZVcfAJ03TJzgM5Q UihLzCkFCgUkFhcr6dvZFOWXlqQqZOQXl9gqRRsaGukZGpjrGRkZ6ZkYx1oZmQKVJKRmrPoj XvBBuWL6io3MDYyLZboYOTkkBEwkpi08y9bFyMUhJLCDUWL9tf8sEM53RomjHXtZYKr6Xk2F qtrNKHHh+np2kASvgKDEj8n3gIo4OJgF5CWOXMoGCTMLaEq8+DIJatA9RomDq1qZIOq1JK7N 284GYrMIqEqcufKHGcRmA4rvf3EDLM4voChx9cdjRhBbVCBCYuf8b2C7RASkJF5+PMUEMpRZ 4BejxOPf98ASwgL+Eguf32AFsTmBNn9o+ccOUiQhcIJN4vuR91AvuEg8OHWVDcIWlnh1fAs7 hC0t8WzVRkaIhnZGic1z7rFAOB2MEvdXNrJCVBlLnOpqZIJ4jk+i4/BfdpCfJQR4JTrahCBM D4npZ7wgqh0l9s87wAoNLiaJ80c3Mk1glJuFFGKzECE2CynEFjAyr2IUSy0ozk1PLTYqMNIr TswtLs1L10vOz93ECE55Wm47GJec8znEKMDBqMTD+0PxWaQQa2JZcWXuIUYJDmYlEd7ja4FC vCmJlVWpRfnxRaU5qcWHGE2BAT6RWUo0OR+YjvNK4g1NLA1MzMyMzM0sgClMnLdu27UIIYH0 xJLU7NTUgtQimD4mDk6pBsaFSrO1U7e51kS891f/fP7u10PCGvIqAceKw9k3TTq4rPTIwSdW VyI6OvVKb0o/lNwht0668G9Jdu5KjXdqkot+isnsfrvlwLK+E44s8Tc0rdTXnnPZtdH6LVsf 54Kv9XoR579l/mi0OvS5QWqasJ1wxR/BwBsPMg11+CfcsnzwLXL54kS7IlslluKMREMt5qLi RABqZhgvjwMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrFLMWRmVeSWpSXmKPExsVy+t9jAd0ZO55FGixs0bOYeOMKi8Wk+xNY LM42vWG3uLxrDpvF594jjBbPN3WyWjxe8Zbdgd1j56y77B6L97xk8ti0qpPNo2/LKkaPz5vk AlijuGxSUnMyy1KL9O0SuDJW/REv+KBcMX3FRuYGxsUyXYycHBICJhJ9r6aygdhCAjsZJW4d zAexeQUEJX5MvsfSxcjBwSwgL3HkUjaEqS4xZUpuFyMXUPUDRon9a1cxQZRrSVybtx1sDIuA qsSZK3+YQWw2oPj+FzfA4vwCihJXfzxmBJkjKhAh0X2iEiQsIiAl8fLjKSaQmcwCvxglHv+Z A7ZWWMBX4va6AIhde5kk2jZ/AtvFKaAp8aHlH/sERoFZSC6dhXDpLIRLFzAyr2KUTC0ozk3P LTYqMMpLLdcrTswtLs1L10vOz93ECAzxbYe1+ncwPl4Sf4hRgINRiYf3h+KzSCHWxLLiytxD jBIczEoivMfXAoV4UxIrq1KL8uOLSnNSiw8xSnOwKInz8ucfixQSSE8sSc1OTS1ILYLJMnFw SjUwLtUP5qv0582cbLD1/2Vjs918M/mmdy8/xrMix2Bjx/ubi787CkncWP+2+GrzNZ2vG7fv DPtQz26otE5vJ0+7w5bZQup+sk65n1bOXRjGonx/q+AhM+Oz9k4sL0ofbFO6/vaj5m67C8+S HP1z7zJkXDW9WNevMaUsynujnGIqm5dY5dSkjiW5SizFGYmGWsxFxYkA4jVytW0CAAA= X-CMS-MailID: 20171018021240epcas2p48f19b2b08068864879a8aae01d3590d7 X-Msg-Generator: CA X-Sender-IP: 182.195.42.143 X-Local-Sender: =?UTF-8?B?7LWc7LCs7JqwG1RpemVuIFBsYXRmb3JtIExhYihTL1fshLw=?= =?UTF-8?B?7YSwKRvsgrzshLHsoITsnpAbU2VuaW9yIEVuZ2luZWVy?= X-Global-Sender: =?UTF-8?B?Q2hhbndvbyBDaG9pG1RpemVuIFBsYXRmb3JtIExhYi4bU2Ft?= =?UTF-8?B?c3VuZyBFbGVjdHJvbmljcxtTZW5pb3IgRW5naW5lZXI=?= X-Sender-Code: =?UTF-8?B?QzEwG1RFTEUbQzEwVjgxMTE=?= CMS-TYPE: 102P DLP-Filter: Pass X-CFilter-Loop: Reflected X-CMS-RootMailID: 20171013074833epcas1p433f03287f42a5b6dc16e796efa2a3bf3 X-RootMTR: 20171013074833epcas1p433f03287f42a5b6dc16e796efa2a3bf3 References: <1507880904-31956-1-git-send-email-cw00.choi@samsung.com> <1507880904-31956-4-git-send-email-cw00.choi@samsung.com> <59E6AED6.70703@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017년 10월 18일 10:31, Chanwoo Choi wrote: > Hi, > > On 2017년 10월 17일 23:43, MyungJoo Ham wrote: >> On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi wrote: >>> The commit a76caf55e5b35 ("thermal: Add devfreq cooling") is able >>> to disable OPP as a cooling device. In result, both update_devfreq() >>> and {min|max}_freq_show() have to consider the 'opp->available' >>> status of each OPP. >>> >>> So, this patch adds the 'scaling_{min|max}_freq' to struct devfreq >>> in order to indicate the available mininum and maximum frequency >>> by adjusting OPP interface such as dev_pm_opp_{disable|enable}(). >>> The 'scaling_{min|max}_freq' are used for on both update_devfreq() >>> and {min|max}_freq_show(). >>> >>> Signed-off-by: Chanwoo Choi >>> --- >>> drivers/devfreq/devfreq.c | 36 ++++++++++++++++++++++++++++-------- >>> include/linux/devfreq.h | 4 ++++ >>> 2 files changed, 32 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>> index b6ba24e5db0d..9de013ffeb67 100644 >>> --- a/drivers/devfreq/devfreq.c >>> +++ b/drivers/devfreq/devfreq.c >> [] >>> @@ -494,6 +499,19 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type, >>> int ret; >>> >>> mutex_lock(&devfreq->lock); >>> + >>> + devfreq->scaling_min_freq = find_available_min_freq(devfreq); >>> + if (!devfreq->scaling_min_freq) { >>> + mutex_unlock(&devfreq->lock); >>> + return -EINVAL; >>> + } >>> + >>> + devfreq->scaling_max_freq = find_available_max_freq(devfreq); >>> + if (!devfreq->max_freq) { >> >> 1. s/max_freq/scaling/max_freq/ ?? > > My mistake. The scaling_max_freq is right. I'll fix it. > >> >> 2. what if thermal is not active or has never triggered any event and >> the user has never stated max/min? (making scaling_*_freq zero) > > > The devfreq-cooling.c of tmu uses the OPP interface > and then OPP interface affect the scaling_min/max_freq of devfreq > through dev_pm_opp_disable/enable(). So, even if 'thermal is not active > or has never triggered any event', devfreq will use the OPP interface > as a mandatory. > > In result, I think that devfreq should maintain the correct frequency > of scaling_min/max_freq indicating the 'limit minimum/maximum frequency > requested by OPP interface' instead of zero. > > So, I'll change the description of scaling_min/max_freq as following: > (by devfreq-cooling -> by OPP interface) > On v4: > + * @scaling_min_freq: Limit minimum frequency requested by devfreq-cooling > + * @scaling_max_freq: Limit maximum frequency requested by devfreq-cooling > > On v5: > + * @scaling_min_freq: Limit minimum frequency requested by OPP interface > + * @scaling_max_freq: Limit maximum frequency requested by OPP interface > > > And, this patch showed the wrong value of min/max_freq_show() by my mistake. > I showed the 'min/max_freq' directly through min/max_freq_show() > without comparing with scaling_min/max_freq. So, I'll fix this issue as following: > --------------- > On v5: > static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr, > char *buf) > { > - return sprintf(buf, "%lu\n", to_devfreq(dev)->min_freq); > + struct devfreq *df = to_devfreq(dev); > + > + return sprintf(buf, "%lu\n", MAX(df->scaling_min_freq, df->min_freq)); > } > > static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr, > @@ -1161,7 +1183,9 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr, > static ssize_t max_freq_show(struct device *dev, struct device_attribute *attr, > char *buf) > { > - return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq); > + struct devfreq *df = to_devfreq(dev); > + > + return sprintf(buf, "%lu\n", MIN(df->scaling_max_freq, df->max_freq)); > --------------- If you agree my opinion, I'll send v5 patchset right now because if patch3 gets the review, everything is done without patch8. As I replied, I'll drop the patch8 from this patchset. > > >> >>> + mutex_unlock(&devfreq->lock); >>> + return -EINVAL; >>> + } >>> + >>> ret = update_devfreq(devfreq); >>> mutex_unlock(&devfreq->lock); >>> >> >> >> > > -- Best Regards, Chanwoo Choi Samsung Electronics