Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752913Ab3CZRMx (ORCPT ); Tue, 26 Mar 2013 13:12:53 -0400 Received: from mga01.intel.com ([192.55.52.88]:25577 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751712Ab3CZRMv (ORCPT ); Tue, 26 Mar 2013 13:12:51 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.84,913,1355126400"; d="scan'208";a="308709759" Message-ID: <1364317940.13292.4.camel@rzhang1-mobl4> Subject: RE: [RFC PATCH 3/5] Thermal: build thermal governors into thermal_sys module From: Zhang Rui To: "R, Durgadoss" Cc: "linux-pm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "amit.daniel@samsung.com" , "andi@lisas.de" Date: Tue, 26 Mar 2013 11:12:20 -0600 In-Reply-To: <4D68720C2E767A4AA6A8796D42C8EB59C59803@BGSMSX101.gar.corp.intel.com> References: <1364315169-15427-1-git-send-email-rui.zhang@intel.com> <1364315169-15427-4-git-send-email-rui.zhang@intel.com> <4D68720C2E767A4AA6A8796D42C8EB59C59803@BGSMSX101.gar.corp.intel.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10173 Lines: 319 On Tue, 2013-03-26 at 10:53 -0600, R, Durgadoss wrote: > Hi Rui, > > > -----Original Message----- > > From: Zhang, Rui > > Sent: Tuesday, March 26, 2013 9:56 PM > > To: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org > > Cc: amit.daniel@samsung.com; R, Durgadoss; andi@lisas.de; Zhang, Rui > > Subject: [RFC PATCH 3/5] Thermal: build thermal governors into thermal_sys > > module > > > > Signed-off-by: Zhang Rui > > --- > > drivers/thermal/Makefile | 6 +++--- > > drivers/thermal/fair_share.c | 15 ++------------- > > drivers/thermal/step_wise.c | 16 ++-------------- > > drivers/thermal/thermal_core.c | 36 > > ++++++++++++++++++++++++++++++++++-- > > drivers/thermal/thermal_core.h | 25 +++++++++++++++++++++++++ > > drivers/thermal/user_space.c | 15 ++------------- > > include/linux/thermal.h | 1 - > > 7 files changed, 68 insertions(+), 46 deletions(-) > > > > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile > > index b2009bd..b7fffc7 100644 > > --- a/drivers/thermal/Makefile > > +++ b/drivers/thermal/Makefile > > @@ -6,9 +6,9 @@ obj-$(CONFIG_THERMAL) += thermal_sys.o > > thermal_sys-y += thermal_core.o > > > > # governors > > -obj-$(CONFIG_THERMAL_GOV_FAIR_SHARE) += fair_share.o > > -obj-$(CONFIG_THERMAL_GOV_STEP_WISE) += step_wise.o > > -obj-$(CONFIG_THERMAL_GOV_USER_SPACE) += user_space.o > > +thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE) += > > fair_share.o > > +thermal_sys-$(CONFIG_THERMAL_GOV_STEP_WISE) += step_wise.o > > +thermal_sys-$(CONFIG_THERMAL_GOV_USER_SPACE) += > > user_space.o > > > > # cpufreq cooling > > obj-$(CONFIG_CPU_THERMAL) += cpu_cooling.o > > diff --git a/drivers/thermal/fair_share.c b/drivers/thermal/fair_share.c > > index 792479f..944ba2f 100644 > > --- a/drivers/thermal/fair_share.c > > +++ b/drivers/thermal/fair_share.c > > @@ -22,9 +22,6 @@ > > * > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > ~~~~~~~~~~~~~~~~ > > */ > > > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > - > > -#include > > #include > > > > #include "thermal_core.h" > > @@ -111,23 +108,15 @@ static int fair_share_throttle(struct > > thermal_zone_device *tz, int trip) > > static struct thermal_governor thermal_gov_fair_share = { > > .name = "fair_share", > > .throttle = fair_share_throttle, > > - .owner = THIS_MODULE, > > }; > > > > -static int __init thermal_gov_fair_share_init(void) > > +int thermal_gov_fair_share_register(void) > > { > > return thermal_register_governor(&thermal_gov_fair_share); > > } > > > > -static void __exit thermal_gov_fair_share_exit(void) > > +void thermal_gov_fair_share_unregister(void) > > { > > thermal_unregister_governor(&thermal_gov_fair_share); > > } > > > > -/* This should load after thermal framework */ > > -fs_initcall(thermal_gov_fair_share_init); > > -module_exit(thermal_gov_fair_share_exit); > > - > > -MODULE_AUTHOR("Durgadoss R"); > > -MODULE_DESCRIPTION("A simple weight based thermal throttling > > governor"); > > -MODULE_LICENSE("GPL"); > > diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c > > index 407cde3..a6c9666 100644 > > --- a/drivers/thermal/step_wise.c > > +++ b/drivers/thermal/step_wise.c > > @@ -22,9 +22,6 @@ > > * > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > ~~~~~~~~~~~~~~~~ > > */ > > > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > - > > -#include > > #include > > > > #include "thermal_core.h" > > @@ -180,23 +177,14 @@ static int step_wise_throttle(struct > > thermal_zone_device *tz, int trip) > > static struct thermal_governor thermal_gov_step_wise = { > > .name = "step_wise", > > .throttle = step_wise_throttle, > > - .owner = THIS_MODULE, > > }; > > > > -static int __init thermal_gov_step_wise_init(void) > > +int thermal_gov_step_wise_register(void) > > { > > return thermal_register_governor(&thermal_gov_step_wise); > > } > > > > -static void __exit thermal_gov_step_wise_exit(void) > > +void thermal_gov_step_wise_unregister(void) > > { > > thermal_unregister_governor(&thermal_gov_step_wise); > > } > > - > > -/* This should load after thermal framework */ > > -fs_initcall(thermal_gov_step_wise_init); > > -module_exit(thermal_gov_step_wise_exit); > > - > > -MODULE_AUTHOR("Durgadoss R"); > > -MODULE_DESCRIPTION("A step-by-step thermal throttling governor"); > > -MODULE_LICENSE("GPL"); > > diff --git a/drivers/thermal/thermal_core.c > > b/drivers/thermal/thermal_core.c > > index 845ed6e..eac9745 100644 > > --- a/drivers/thermal/thermal_core.c > > +++ b/drivers/thermal/thermal_core.c > > @@ -1858,22 +1858,54 @@ static inline int genetlink_init(void) { return 0; } > > static inline void genetlink_exit(void) {} > > #endif /* !CONFIG_NET */ > > > > +static int __init thermal_register_governors(void) > > +{ > > + int result; > > + > > + result = thermal_gov_step_wise_register(); > > + if (result) > > [Patches 1,2 are fine with me]. > thanks. > A pr_err statement here, saying the registration failed, > would help us a lot. > yes. sounds reasonable to me. Will add it in V2. > > + return result; > > + > > + result = thermal_gov_fair_share_register(); > > + if (result) > > + return result; > > + > > + result = thermal_gov_user_space_register(); > > Same check + print statement for the other two as well. > yep. > > + > > + return result; > > +} > > + > > +static void __exit thermal_unregister_governors(void) > > +{ > > + thermal_gov_step_wise_unregister(); > > + thermal_gov_fair_share_unregister(); > > + thermal_gov_user_space_unregister(); > > +} > > + > > static int __init thermal_init(void) > > { > > - int result = 0; > > + int result; > > + > > + result = thermal_register_governors(); > > + if (result) > > + return result; > > > > result = class_register(&thermal_class); > > if (result) > > Shouldn't we do thermal_unregister_governors here ? > > > return result; > > > > result = genetlink_init(); > > + if (result) > > + class_unregister(&thermal_class); > > Shouldn't we do thermal_unregister_governors here ? > Probably a goto would make things simpler.. > IMO, these are not worth cleaning up because the driver will be unloaded soon. thanks, rui > > + > > return result; > > } > > > > static void __exit thermal_exit(void) > > { > > - class_unregister(&thermal_class); > > genetlink_exit(); > > + class_unregister(&thermal_class); > > + thermal_unregister_governors(); > > } > > > > fs_initcall(thermal_init); > > diff --git a/drivers/thermal/thermal_core.h > > b/drivers/thermal/thermal_core.h > > index 0d3205a..f84ea0f 100644 > > --- a/drivers/thermal/thermal_core.h > > +++ b/drivers/thermal/thermal_core.h > > @@ -50,4 +50,29 @@ struct thermal_instance { > > struct list_head cdev_node; /* node in cdev->thermal_instances */ > > }; > > > > + > > +#ifdef CONFIG_THERMAL_GOV_STEP_WISE > > +extern int thermal_gov_step_wise_register(void); > > +extern void thermal_gov_step_wise_unregister(void); > > +#else > > +static inline int thermal_gov_step_wise_register(void) { return 0; } > > +static inline void thermal_gov_step_wise_unregister(void) {} > > +#endif /* CONFIG_THERMAL_GOV_STEP_WISE */ > > + > > +#ifdef CONFIG_THERMAL_GOV_FAIR_SHARE > > +extern int thermal_gov_fair_share_register(void); > > +extern void thermal_gov_fair_share_unregister(void); > > +#else > > +static inline int thermal_gov_fair_share_register(void) { return 0; } > > +static inline void thermal_gov_fair_share_unregister(void) {} > > +#endif /* CONFIG_THERMAL_GOV_FAIR_SHARE */ > > + > > +#ifdef CONFIG_THERMAL_GOV_USER_SPACE > > +extern int thermal_gov_user_space_register(void); > > +extern void thermal_gov_user_space_unregister(void); > > +#else > > +static inline int thermal_gov_user_space_register(void) { return 0; } > > +static inline void thermal_gov_user_space_unregister(void) {} > > +#endif /* CONFIG_THERMAL_GOV_USER_SPACE */ > > + > > #endif /* __THERMAL_CORE_H__ */ > > diff --git a/drivers/thermal/user_space.c b/drivers/thermal/user_space.c > > index 6bbb380..10adcdd 100644 > > --- a/drivers/thermal/user_space.c > > +++ b/drivers/thermal/user_space.c > > @@ -22,9 +22,6 @@ > > * > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > ~~~~~~~~~~~~~~~~ > > */ > > > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > - > > -#include > > #include > > > > #include "thermal_core.h" > > @@ -46,23 +43,15 @@ static int notify_user_space(struct > > thermal_zone_device *tz, int trip) > > static struct thermal_governor thermal_gov_user_space = { > > .name = "user_space", > > .throttle = notify_user_space, > > - .owner = THIS_MODULE, > > }; > > > > -static int __init thermal_gov_user_space_init(void) > > +int thermal_gov_user_space_register(void) > > { > > return thermal_register_governor(&thermal_gov_user_space); > > } > > > > -static void __exit thermal_gov_user_space_exit(void) > > +void thermal_gov_user_space_unregister(void) > > { > > thermal_unregister_governor(&thermal_gov_user_space); > > } > > > > -/* This should load after thermal framework */ > > -fs_initcall(thermal_gov_user_space_init); > > -module_exit(thermal_gov_user_space_exit); > > - > > -MODULE_AUTHOR("Durgadoss R"); > > -MODULE_DESCRIPTION("A user space Thermal notifier"); > > -MODULE_LICENSE("GPL"); > > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > > index f0bd7f9..2eeec01 100644 > > --- a/include/linux/thermal.h > > +++ b/include/linux/thermal.h > > @@ -184,7 +184,6 @@ struct thermal_governor { > > char name[THERMAL_NAME_LENGTH]; > > int (*throttle)(struct thermal_zone_device *tz, int trip); > > struct list_head governor_list; > > - struct module *owner; > > }; > > > > /* Structure that holds binding parameters for a zone */ > > -- > > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/