On Mon, 2014-01-13 at 19:17 +0800, Wei Ni wrote: > In the of-thermal driver, it register a thermal zone device > without governor, since the governers are not static, we > should be able to update to a new one in run-time, so I add > a new fucntion thermal_update_governor() to do it. > > In the future, the governor will be more complex, it may want > to tune governor-specific parameters/configuration for > different thermal zone at different run-time, so I add > start/stop for the governor and add governor_data as governor's > parameters, so that the thermal platform driver can change > the governor with start/stop, and tune the governor parameters > in run-time.
are you going to introduce a new governor with .start/.stop implemented or are you going to introduce .start/.stop callbacks for the current governors? I'd like to see if there is any real request for this. > > Signed-off-by: Wei Ni <[email protected]> > --- > Documentation/thermal/sysfs-api.txt | 5 +++ > drivers/thermal/thermal_core.c | 80 > +++++++++++++++++++++++++++++++---- > include/linux/thermal.h | 12 ++++++ > 3 files changed, 88 insertions(+), 9 deletions(-) > > diff --git a/Documentation/thermal/sysfs-api.txt > b/Documentation/thermal/sysfs-api.txt > index 6a70b55c..7efd8e6 100644 > --- a/Documentation/thermal/sysfs-api.txt > +++ b/Documentation/thermal/sysfs-api.txt > @@ -405,3 +405,8 @@ platform data is provided, this uses the step_wise > throttling policy. > This function serves as an arbitrator to set the state of a cooling > device. It sets the cooling device to the deepest cooling state if > possible. > + > +5.5:thermal_update_governor: > +This function update the thermal zone's governor to a new one and update > +the governor data of the new governor for that thermal zone. Returns 0 if > +success, an erro code otherwise. > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c > index aab1df8..d922baf 100644 > --- a/drivers/thermal/thermal_core.c > +++ b/drivers/thermal/thermal_core.c > @@ -93,10 +93,17 @@ int thermal_register_governor(struct thermal_governor > *governor) > name = pos->tzp->governor_name; > else > name = DEFAULT_THERMAL_GOVERNOR; > - if (!strnicmp(name, governor->name, THERMAL_NAME_LENGTH)) > + if (!strnicmp(name, governor->name, THERMAL_NAME_LENGTH)) { > + if (governor->start) { > + err = governor->start(pos); > + if (err < 0) > + goto exit; > + } > pos->governor = governor; > + } > } > > +exit: > mutex_unlock(&thermal_list_lock); > mutex_unlock(&thermal_governor_lock); > > @@ -119,8 +126,11 @@ void thermal_unregister_governor(struct thermal_governor > *governor) > > list_for_each_entry(pos, &thermal_tz_list, node) { > if (!strnicmp(pos->governor->name, governor->name, > - THERMAL_NAME_LENGTH)) > + THERMAL_NAME_LENGTH)) { > + if (pos->governor->stop) > + pos->governor->stop(pos); > pos->governor = NULL; > + } > } > > mutex_unlock(&thermal_list_lock); > @@ -130,6 +140,51 @@ exit: > return; > } > > +/** > + * thermal_update_governor() - update the thermal zone device's governor > + * to a new one. > + * @tzd: pointer of thermal zone device, which need to update governor. > + * @name: new governor name. > + * @gov_data: governor data for the new governor if needed. > + * > + * Return: On success returns 0, an error code otherwise > + */ > +int thermal_update_governor(struct thermal_zone_device *tzd, > + const char *name, void *gov_data) > +{ > + struct thermal_governor *old_gov, *new_gov; > + int ret = 0; > + > + mutex_lock(&thermal_governor_lock); > + > + new_gov = __find_governor(name); > + if (!new_gov) { > + ret = -EINVAL; > + goto exit; > + } > + > + old_gov = tzd->governor; > + > + if (old_gov && old_gov->stop) > + old_gov->stop(tzd); > + > + if (new_gov->start) { > + ret = new_gov->start(tzd); > + if (ret < 0) { > + if (old_gov && old_gov->start) > + old_gov->start(tzd); > + goto exit; > + } > + } > + > + tzd->governor = new_gov; > + tzd->governor_data = gov_data; > + > +exit: > + mutex_unlock(&thermal_governor_lock); > + return ret; > +} > + > static int get_idr(struct idr *idr, struct mutex *lock, int *id) > { > int ret; > @@ -734,22 +789,17 @@ policy_store(struct device *dev, struct > device_attribute *attr, > { > int ret = -EINVAL; > struct thermal_zone_device *tz = to_thermal_zone(dev); > - struct thermal_governor *gov; > char name[THERMAL_NAME_LENGTH]; > > snprintf(name, sizeof(name), "%s", buf); > > - mutex_lock(&thermal_governor_lock); > - > - gov = __find_governor(strim(name)); > - if (!gov) > + ret = thermal_update_governor(tz, strim(name), NULL); > + if (ret < 0) > goto exit; > > - tz->governor = gov; > ret = count; > > exit: > - mutex_unlock(&thermal_governor_lock); > return ret; > } > > @@ -1559,6 +1609,12 @@ struct thermal_zone_device > *thermal_zone_device_register(const char *type, > else > tz->governor = __find_governor(DEFAULT_THERMAL_GOVERNOR); > > + if (tz->governor->start) { Note that tz->governor may be NULL at this time. although this seems like a bug to me. > + result = tz->governor->start(tz); > + if (result < 0) > + tz->governor = NULL; > + } > + why not invoke thermal_update_governor() directly here? thanks, rui > mutex_unlock(&thermal_governor_lock); > > if (!tz->tzp || !tz->tzp->no_hwmon) { > @@ -1585,6 +1641,9 @@ struct thermal_zone_device > *thermal_zone_device_register(const char *type, > return tz; > > unregister: > + if (tz->governor && tz->governor->stop) > + tz->governor->stop(tz); > + tz->governor = NULL; > release_idr(&thermal_tz_idr, &thermal_idr_lock, tz->id); > device_unregister(&tz->device); > return ERR_PTR(result); > @@ -1648,6 +1707,9 @@ void thermal_zone_device_unregister(struct > thermal_zone_device *tz) > device_remove_file(&tz->device, &dev_attr_policy); > device_remove_file(&tz->device, &dev_attr_available_policies); > remove_trip_attrs(tz); > + > + if (tz->governor && tz->governor->stop) > + tz->governor->stop(tz); > tz->governor = NULL; > > thermal_remove_hwmon_sysfs(tz); > diff --git a/include/linux/thermal.h b/include/linux/thermal.h > index f7e11c7..a473736 100644 > --- a/include/linux/thermal.h > +++ b/include/linux/thermal.h > @@ -177,6 +177,7 @@ struct thermal_zone_device { > struct thermal_zone_device_ops *ops; > const struct thermal_zone_params *tzp; > struct thermal_governor *governor; > + void *governor_data; > struct list_head thermal_instances; > struct idr idr; > struct mutex lock; /* protect thermal_instances list */ > @@ -187,6 +188,15 @@ struct thermal_zone_device { > /* Structure that holds thermal governor information */ > struct thermal_governor { > char name[THERMAL_NAME_LENGTH]; > + > + /* > + * The start and stop operations will be called when thermal zone is > + * registered and when change governor via sysfs or driver in running > + * time. > + */ > + int (*start)(struct thermal_zone_device *tz); > + void (*stop)(struct thermal_zone_device *tz); > + > int (*throttle)(struct thermal_zone_device *tz, int trip); > struct list_head governor_list; > }; > @@ -293,6 +303,8 @@ struct thermal_instance *get_thermal_instance(struct > thermal_zone_device *, > struct thermal_cooling_device *, int); > void thermal_cdev_update(struct thermal_cooling_device *); > void thermal_notify_framework(struct thermal_zone_device *, int); > +int thermal_update_governor(struct thermal_zone_device *, const char *, > + void *); > > #ifdef CONFIG_NET > extern int thermal_generate_netlink_event(struct thermal_zone_device *tz, -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
