From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754092AbcC1DML (ORCPT ); Sun, 27 Mar 2016 23:12:11 -0400 Received: from mailout4.samsung.com ([203.254.224.34]:42122 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753511AbcC1DMJ (ORCPT ); Sun, 27 Mar 2016 23:12:09 -0400 X-AuditID: cbfee68f-f79c86d0000012ad-cd-56f8a0f5d5c3 Date: Mon, 28 Mar 2016 03:11:49 +0000 (GMT) From: MyungJoo Ham Subject: Re: [PATCH v6 06/21] PM / devfreq: Add new passive governor To: =?utf-8?Q?=EC=B5=9C=EC=B0=AC=EC=9A=B0?= , =?utf-8?Q?=EB=B0=95=EA=B2=BD=EB=AF=BC?= , =?utf-8?Q?=ED=81=AC=EC=89=AC=EC=8B=9C=ED=86=A0=ED=94=84?= , "kgene@kernel.org" Cc: "rjw@rjwysocki.net" , "robh+dt@kernel.org" , "pawel.moll@arm.com" , "mark.rutland@arm.com" , "ijc+devicetree@hellion.org.uk" , "galak@codeaurora.org" , "linux@arm.linux.org.uk" , "linux.amoon@gmail.com" , "m.reichl@fivetechno.de" , "tjakobi@math.uni-bielefeld.de" , =?utf-8?Q?=EB=8C=80=EC=9D=B8=EA=B8=B0?= , "linux-kernel@vger.kernel.org" , "linux-pm@vger.kernel.org" , "linux-samsung-soc@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" Reply-to: myungjoo.ham@samsung.com MIME-version: 1.0 X-MTR: 20160328031121517@myungjoo.ham Msgkey: 20160328031121517@myungjoo.ham X-EPLocale: ko_KR.utf-8 X-Priority: 3 X-EPWebmail-Msg-Type: personal X-EPWebmail-Reply-Demand: 0 X-EPApproval-Locale: X-EPHeader: ML X-MLAttribute: X-RootMTR: 20160328031121517@myungjoo.ham X-ParentMTR: X-ArchiveUser: X-CPGSPASS: N X-ConfirmMail: N,general Content-type: text/plain; charset=utf-8 MIME-version: 1.0 Message-id: <1314094711.28841459134703593.JavaMail.weblogic@epmlwas02a> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrEIsWRmVeSWpSXmKPExsWyRsSkWPfrgh9hBmdmilpc3jWHzYHR4/Mm uQDGKC6blNSczLLUIn27BK6Ms20fmQqOmVR0HNrL1MA4xbiLkZNDSEBdYtGSk2wgtoSAicSM B2+YIWwxiQv31gPFuYBqljJKnF86kxGm6PjPdkaIxBxGiXl9rUBVHBwsAqoSP89ygZhsAnoS Mz8ng5QLC7hI3Ou5yg5iiwhMYJJY/K8ApJVZYCG7xJMfl5ghjlCSWLPvFQuIzSsgKHFy5hMW iF2qEk8Pv2CGiKtJNE64ygoRF5e4MPcSO4TNKzGj/SlUvZzEtK9roB6Qljg/awMjzDOLvz+G ivNLHLu9gwnCFpCYeuYgVI2WxOkjL6HifBJrFr5lganfdWo5M8yu+1vmQtVISGxteQJ2D7OA osSU7ofsIL8zC2hKrN+lj+oVDiDbXeL0c2uQ1yUEpnJITPhwjH0Co9IsJGWzkEyahTAJWckC RpZVjKKpBckFxUnpRcZ6xYm5xaV56XrJ+bmbGIFJ4fS/Z/07GO8esD7EKMDBqMTDm2H5I0yI NbGsuDL3EKMpMI4mMkuJJucDU09eSbyhsZmRhamJqbGRuaWZkjjvQqmfwUIC6YklqdmpqQWp RfFFpTmpxYcYmTg4pRoYBVn2zzvq+JwlxCEu6m5tYNqipP3b83ScTmW9fmGjqxHieazmeOiF v5x73po/97PYGfzE7v0+6XlbA47rFMX7ebE42r+belc7nLcitnTiXY+Fq791Oy0ozWb4a1vT wuicm3X5f7/pnZn/2W8UmGjnHxbwzD0vKct9YH3TWYXQuHLduqvbJJcrsRRnJBpqMRcVJwIA hSKn4AUDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrMKsWRmVeSWpSXmKPExsVy+t/tft2vC36EGTzeIGBxedccNgdGj8+b 5AIYo9JsMlITU1KLFFLzkvNTMvPSbZW8g+Od403NDAx1DS0tzJUU8hJzU22VXHwCdN0yc4CG KimUJeaUAoUCEouLlfTtbIryS0tSFTLyi0tslaINzY30jAz0TI30DI1jrQwNDIxMgWoS0jLO tn1kKjhmUtFxaC9TA+MU4y5GTg4hAXWJRUtOsoHYEgImEsd/tjNC2GISF+6tB4pzAdXMYZSY 19cK5HBwsAioSvw8ywVisgnoScz8nAxSLizgInGv5yo7iC0iMIFJYvG/ApBWZoGF7BJPflxi htilJLFm3ysWEJtXQFDi5MwnLBC7VCWeHn7BDBFXk2iccJUVIi4ucWHuJXYIm1diRvtTqHo5 iWlf1zBD2NIS52dtgLt58ffHUHF+iWO3dzBB2AISU88chKrRkjh95CVUnE9izcK3LDD1u04t Z4bZdX/LXKgaCYmtLU/A7mEWUJSY0v2QHeR3ZgFNifW79FG9wgFku0ucfm49gVF2FpLMLCTN sxCakZUsYGRZxSiaWpBcUJyUXmGsV5yYW1yal66XnJ+7iRGcgJ4t3sH4/7z1IUYBDkYlHt4M yx9hQqyJZcWVuYcYJTiYlUR4t84FCvGmJFZWpRblxxeV5qQWH2I0BcbYRGYp0eR8YHLMK4k3 NDY2MTMxtTSxMDA1VxLnDfi7LkxIID2xJDU7NbUgtQimj4mDU6qB0anyt4R+3aM9tY9/a4fc n3T8+PbZNQLFh/yKZr0T/mu9VmrVnIs7biWZBcRNPF+x4uVUpetf3/RnrNWf350x12KH9Vuu VTyXjntbsJdGK57jq44sO9Gce8N35Q/F9BMdOxYG3W1hvzRJ7m/DpzW8D+5pXL5TZC86OU09 8cMb18KJfw2bziUlbFNiKc5INNRiLipOBACsIIAAVgMAAA== DLP-Filter: Pass X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u2S3CLmU007295 [] > Suggested-by: Myungjoo Ham > Signed-off-by: Chanwoo Choi > [tjakobi: Reported RCU locking issue and cw00.choi fix it.] > Reported-by: Tobias Jakobi > [m.reichl and linux.amoon: Tested it on exynos4412-odroidu3 board] > Tested-by: Markus Reichl > Tested-by: Anand Moon > --- > drivers/devfreq/Kconfig | 7 ++ > drivers/devfreq/Makefile | 1 + > drivers/devfreq/devfreq.c | 17 ++++ > drivers/devfreq/governor.h | 15 +++ > drivers/devfreq/governor_passive.c | 192 +++++++++++++++++++++++++++++++++++++ > include/linux/devfreq.h | 3 + > 6 files changed, 235 insertions(+) > create mode 100644 drivers/devfreq/governor_passive.c > > diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig [] > diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile > index 8af8aaf922a8..2633087d5c63 100644 > --- a/drivers/devfreq/Makefile > +++ b/drivers/devfreq/Makefile > @@ -4,6 +4,7 @@ obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND) += governor_simpleondemand.o > obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE) += governor_performance.o > obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE) += governor_powersave.o > obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o > +obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o > > # DEVFREQ Drivers > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 1d6c803804d5..9f84bbc2994c 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -478,7 +478,13 @@ static void _remove_devfreq(struct devfreq *devfreq) > dev_warn(&devfreq->dev, "releasing devfreq which doesn't exist\n"); > return; > } > + > + if (devfreq->governor->type == DEVFREQ_GOV_PASSIVE) > + devfreq_passive_unregister_notifier(devfreq); > + > list_del(&devfreq->node); > + list_del_init(&devfreq->passive_node); > + Let's do this inside governor_passive.c, you already have the DEVFREQ_GOV_STOP signal. This may be called more frequently because the START-STOP pair has wider usage than add-remove pair. However, still, it's ok to be detached during the "STOP"ed but not removed state. > mutex_unlock(&devfreq_list_lock); > > if (devfreq->governor) > @@ -598,6 +604,17 @@ struct devfreq *devfreq_add_device(struct device *dev, > goto err_init; > } > > + if (devfreq->governor->type == DEVFREQ_GOV_PASSIVE) { > + struct devfreq *parent_devfreq = (struct devfreq *)data; > + list_add(&devfreq->passive_node, &parent_devfreq->passive_node); > + > + err = devfreq_passive_register_notifier(devfreq); > + if (err <0) > + goto err_init; > + } else { > + INIT_LIST_HEAD(&devfreq->passive_node); > + } > + Same as above. We can reuse DEVFREQ_GOV_START for this. With DEVFREQ_GOV_START/STOP, we can entirely remove any modifications in the devfreq.c, governor.h, devfreq.h. Besides, such an approach removed the need for the patch 05/21. We no longer (or yet) need "governor type". > return devfreq; > > err_init: > diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h > index cf19b923c362..64d1dffcdb43 100644 > --- a/drivers/devfreq/governor.h > +++ b/drivers/devfreq/governor.h as mentioned above, we don't need to modify this file. > diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c > new file mode 100644 > index 000000000000..521e93b68c11 > --- /dev/null > +++ b/drivers/devfreq/governor_passive.c > @@ -0,0 +1,192 @@ > +/* > + * linux/drivers/devfreq/governor_passive.c > + * > + * Copyright (C) 2016 Samsung Electronics > + * Author: Chanwoo Choi > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include > +#include > +#include > +#include Doubly included > +#include "governor.h" > + [] > +static int devfreq_passive_event_handler(struct devfreq *devfreq, > + unsigned int event, void *data) > +{ > + return 0; Let's handle DEVFREQ_GOV_START/STOP event here. > +} > + > +static struct devfreq_governor devfreq_passive = { > + .name = "passive", > + .type = DEVFREQ_GOV_PASSIVE, Let's not use .type enum, yet. We don't this this. At least for now. > + .get_target_freq = devfreq_passive_get_target_freq, > + .event_handler = devfreq_passive_event_handler, > +}; > + [] > diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h As mentioned above, we don't need to modify this file.