On Wed 2008-10-01 19:41:11, Matthew Garrett wrote:
quoted text
> The thermal layer passes temperatures and trip point types around as
> strings. This is fine for sysfs, but makes it hard to use them for other
> purposes in-kernel. Move the string conversion to the sysfs-specific
> code.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>

Still acked. Can you mail it to Len Brown for inclusion, Cc: akpm in
case len is sleeping? It is quite obvious anyway -- passing strings is
bad.


quoted text
> ---
> 
> This version also moves the mode setting (user/kernel) away from using 
> strings, which gets them out of the majority of the API.
> 
> diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
> index 2655bc1..5487a98 100644
> --- a/drivers/acpi/fan.c
> +++ b/drivers/acpi/fan.c
> @@ -69,27 +69,30 @@ static struct acpi_driver acpi_fan_driver = {
>  };
>  
>  /* thermal cooling device callbacks */
> -static int fan_get_max_state(struct thermal_cooling_device *cdev, char *buf)
> +static int fan_get_max_state(struct thermal_cooling_device *cdev, unsigned int
> +			     *state)
>  {
>  	/* ACPI fan device only support two states: ON/OFF */
> -	return sprintf(buf, "1\n");
> +	*state = 1;
> +	return 0;
>  }
>  
> -static int fan_get_cur_state(struct thermal_cooling_device *cdev, char *buf)
> +static int fan_get_cur_state(struct thermal_cooling_device *cdev, unsigned int
> +			     *state)
>  {
>  	struct acpi_device *device = cdev->devdata;
> -	int state;
>  	int result;
>  
>  	if (!device)
>  		return -EINVAL;
>  
> -	result = acpi_bus_get_power(device->handle, &state);
> +	result = acpi_bus_get_power(device->handle, state);
>  	if (result)
>  		return result;
>  
> -	return sprintf(buf, "%s\n", state == ACPI_STATE_D3 ? "0" :
> -			 (state == ACPI_STATE_D0 ? "1" : "unknown"));
> +	*state = (*state == ACPI_STATE_D3 ? 0 :
> +		 (*state == ACPI_STATE_D0 ? 1 : -1));
> +	return 0;
>  }
>  
>  static int
> diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> index ef34b18..4bc094c 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -374,7 +374,8 @@ static int acpi_processor_max_state(struct acpi_processor *pr)
>  	return max_state;
>  }
>  static int
> -processor_get_max_state(struct thermal_cooling_device *cdev, char *buf)
> +processor_get_max_state(struct thermal_cooling_device *cdev, unsigned int
> +			*state)
>  {
>  	struct acpi_device *device = cdev->devdata;
>  	struct acpi_processor *pr = acpi_driver_data(device);
> @@ -382,24 +383,24 @@ processor_get_max_state(struct thermal_cooling_device *cdev, char *buf)
>  	if (!device || !pr)
>  		return -EINVAL;
>  
> -	return sprintf(buf, "%d\n", acpi_processor_max_state(pr));
> +	*state = acpi_processor_max_state(pr);
> +	return 0;
>  }
>  
>  static int
> -processor_get_cur_state(struct thermal_cooling_device *cdev, char *buf)
> +processor_get_cur_state(struct thermal_cooling_device *cdev, unsigned int
> +			*cur_state)
>  {
>  	struct acpi_device *device = cdev->devdata;
>  	struct acpi_processor *pr = acpi_driver_data(device);
> -	int cur_state;
>  
>  	if (!device || !pr)
>  		return -EINVAL;
>  
> -	cur_state = cpufreq_get_cur_state(pr->id);
> +	*cur_state = cpufreq_get_cur_state(pr->id);
>  	if (pr->flags.throttling)
> -		cur_state += pr->throttling.state;
> -
> -	return sprintf(buf, "%d\n", cur_state);
> +		*cur_state += pr->throttling.state;
> +	return 0;
>  }
>  
>  static int
> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
> index 9127036..2e4e2a1 100644
> --- a/drivers/acpi/thermal.c
> +++ b/drivers/acpi/thermal.c
> @@ -934,7 +934,8 @@ static void acpi_thermal_check(void *data)
>  /* sys I/F for generic thermal sysfs support */
>  #define KELVIN_TO_MILLICELSIUS(t) (t * 100 - 273200)
>  
> -static int thermal_get_temp(struct thermal_zone_device *thermal, char *buf)
> +static int thermal_get_temp(struct thermal_zone_device *thermal,
> +			    int *temp)
>  {
>  	struct acpi_thermal *tz = thermal->devdata;
>  	int result;
> @@ -946,25 +947,25 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, char *buf)
>  	if (result)
>  		return result;
>  
> -	return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(tz->temperature));
> +	*temp = KELVIN_TO_MILLICELSIUS(tz->temperature);
> +	return 0;
>  }
>  
> -static const char enabled[] = "kernel";
> -static const char disabled[] = "user";
>  static int thermal_get_mode(struct thermal_zone_device *thermal,
> -				char *buf)
> +				enum thermal_mode_t *thermal_mode)
>  {
>  	struct acpi_thermal *tz = thermal->devdata;
>  
>  	if (!tz)
>  		return -EINVAL;
>  
> -	return sprintf(buf, "%s\n", tz->tz_enabled ?
> -			enabled : disabled);
> +	*thermal_mode = tz->tz_enabled ? THERMAL_MODE_KERNEL
> +		: THERMAL_MODE_USER;
> +	return 0;
>  }
>  
>  static int thermal_set_mode(struct thermal_zone_device *thermal,
> -				const char *buf)
> +				enum thermal_mode_t thermal_mode)
>  {
>  	struct acpi_thermal *tz = thermal->devdata;
>  	int enable;
> @@ -975,12 +976,10 @@ static int thermal_set_mode(struct thermal_zone_device *thermal,
>  	/*
>  	 * enable/disable thermal management from ACPI thermal driver
>  	 */
> -	if (!strncmp(buf, enabled, sizeof enabled - 1))
> +	if (thermal_mode == THERMAL_MODE_KERNEL)
>  		enable = 1;
> -	else if (!strncmp(buf, disabled, sizeof disabled - 1))
> -		enable = 0;
>  	else
> -		return -EINVAL;
> +		enable = 0;
>  
>  	if (enable != tz->tz_enabled) {
>  		tz->tz_enabled = enable;
> @@ -993,7 +992,7 @@ static int thermal_set_mode(struct thermal_zone_device *thermal,
>  }
>  
>  static int thermal_get_trip_type(struct thermal_zone_device *thermal,
> -				 int trip, char *buf)
> +				 int trip, enum thermal_trip_t *type)
>  {
>  	struct acpi_thermal *tz = thermal->devdata;
>  	int i;
> @@ -1002,27 +1001,35 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
>  		return -EINVAL;
>  
>  	if (tz->trips.critical.flags.valid) {
> -		if (!trip)
> -			return sprintf(buf, "critical\n");
> +		if (!trip) {
> +			*type = THERMAL_TRIP_CRITICAL;
> +			return 0;
> +		}
>  		trip--;
>  	}
>  
>  	if (tz->trips.hot.flags.valid) {
> -		if (!trip)
> -			return sprintf(buf, "hot\n");
> +		if (!trip) {
> +			*type = THERMAL_TRIP_HOT;
> +			return 0;
> +		}
>  		trip--;
>  	}
>  
>  	if (tz->trips.passive.flags.valid) {
> -		if (!trip)
> -			return sprintf(buf, "passive\n");
> +		if (!trip) {
> +			*type = THERMAL_TRIP_PASSIVE;
> +			return 0;
> +		}
>  		trip--;
>  	}
>  
>  	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
>  		tz->trips.active[i].flags.valid; i++) {
> -		if (!trip)
> -			return sprintf(buf, "active%d\n", i);
> +		if (!trip) {
> +			*type = THERMAL_TRIP_ACTIVE;
> +			return 0;
> +		}
>  		trip--;
>  	}
>  
> @@ -1030,7 +1037,7 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
>  }
>  
>  static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
> -				 int trip, char *buf)
> +				 int trip, int *temp)
>  {
>  	struct acpi_thermal *tz = thermal->devdata;
>  	int i;
> @@ -1039,31 +1046,40 @@ static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
>  		return -EINVAL;
>  
>  	if (tz->trips.critical.flags.valid) {
> -		if (!trip)
> -			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
> -				tz->trips.critical.temperature));
> +		if (!trip) {
> +			*temp = KELVIN_TO_MILLICELSIUS(
> +				tz->trips.critical.temperature);
> +			return 0;
> +		}
> +
>  		trip--;
>  	}
>  
>  	if (tz->trips.hot.flags.valid) {
> -		if (!trip)
> -			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
> -					tz->trips.hot.temperature));
> +		if (!trip) {
> +			*temp = KELVIN_TO_MILLICELSIUS(
> +				tz->trips.hot.temperature);
> +			return 0;
> +		}
>  		trip--;
>  	}
>  
>  	if (tz->trips.passive.flags.valid) {
> -		if (!trip)
> -			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
> -					tz->trips.passive.temperature));
> +		if (!trip) {
> +			*temp = KELVIN_TO_MILLICELSIUS(
> +				tz->trips.passive.temperature);
> +			return 0;
> +		}
>  		trip--;
>  	}
>  
>  	for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
>  		tz->trips.active[i].flags.valid; i++) {
> -		if (!trip)
> -			return sprintf(buf, "%ld\n", KELVIN_TO_MILLICELSIUS(
> -					tz->trips.active[i].temperature));
> +		if (!trip) {
> +			*temp = KELVIN_TO_MILLICELSIUS(
> +				tz->trips.active[i].temperature);
> +			return 0;
> +		}
>  		trip--;
>  	}
>  
> @@ -1071,7 +1087,7 @@ static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
>  }
>  
>  static int thermal_get_crit_temp(struct thermal_zone_device *thermal,
> -				unsigned long *temperature) {
> +				int *temperature) {
>  	struct acpi_thermal *tz = thermal->devdata;
>  
>  	if (tz->trips.critical.flags.valid) {
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index e8a51a1..bac2901 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -358,26 +358,30 @@ static struct output_properties acpi_output_properties = {
>  
>  
>  /* thermal cooling device callbacks */
> -static int video_get_max_state(struct thermal_cooling_device *cdev, char *buf)
> +static int video_get_max_state(struct thermal_cooling_device *cdev, unsigned
> +			       int *state)
>  {
>  	struct acpi_device *device = cdev->devdata;
>  	struct acpi_video_device *video = acpi_driver_data(device);
>  
> -	return sprintf(buf, "%d\n", video->brightness->count - 3);
> +	*state = video->brightness->count - 3;
> +	return 0;
>  }
>  
> -static int video_get_cur_state(struct thermal_cooling_device *cdev, char *buf)
> +static int video_get_cur_state(struct thermal_cooling_device *cdev, unsigned
> +			       int *state)
>  {
>  	struct acpi_device *device = cdev->devdata;
>  	struct acpi_video_device *video = acpi_driver_data(device);
>  	unsigned long level;
> -	int state;
> +	int offset;
>  
>  	acpi_video_device_lcd_get_level_current(video, &level);
> -	for (state = 2; state < video->brightness->count; state++)
> -		if (level == video->brightness->levels[state])
> -			return sprintf(buf, "%d\n",
> -				       video->brightness->count - state - 1);
> +	for (offset = 2; offset < video->brightness->count; offset++)
> +		if (level == video->brightness->levels[offset]) {
> +			*state = video->brightness->count - offset - 1;
> +			return 0;
> +		}
>  
>  	return -EINVAL;
>  }
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index fe07462..c1f7e65 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -104,22 +104,41 @@ static ssize_t
>  temp_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> +	int temperature;
> +	int ret;
>  
>  	if (!tz->ops->get_temp)
>  		return -EPERM;
>  
> -	return tz->ops->get_temp(tz, buf);
> +	ret = tz->ops->get_temp(tz, &temperature);
> +
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", temperature);
>  }
>  
>  static ssize_t
>  mode_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> +	enum thermal_mode_t mode;
> +	int ret;
>  
>  	if (!tz->ops->get_mode)
>  		return -EPERM;
>  
> -	return tz->ops->get_mode(tz, buf);
> +	ret = tz->ops->get_mode(tz, &mode);
> +
> +	if (ret)
> +		return ret;
> +
> +	if (mode == THERMAL_MODE_KERNEL)
> +		return sprintf(buf, "%s\n", "kernel");
> +	else if (mode == THERMAL_MODE_USER)
> +		return sprintf(buf, "%s\n", "user");
> +	else
> +		return sprintf(buf, "%s\n", "unknown");
>  }
>  
>  static ssize_t
> @@ -132,7 +151,13 @@ mode_store(struct device *dev, struct device_attribute *attr,
>  	if (!tz->ops->set_mode)
>  		return -EPERM;
>  
> -	result = tz->ops->set_mode(tz, buf);
> +	if (!strncmp(buf, "kernel", count))
> +		result = tz->ops->set_mode(tz, THERMAL_MODE_KERNEL);
> +	else if (!strncmp(buf, "user", count))
> +		result = tz->ops->set_mode(tz, THERMAL_MODE_USER);
> +	else
> +		return -EINVAL;
> +
>  	if (result)
>  		return result;
>  
> @@ -145,6 +170,8 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr,
>  {
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
>  	int trip;
> +	enum thermal_trip_t trip_type;
> +	int ret;
>  
>  	if (!tz->ops->get_trip_type)
>  		return -EPERM;
> @@ -152,7 +179,28 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr,
>  	if (!sscanf(attr->attr.name, "trip_point_%d_type", &trip))
>  		return -EINVAL;
>  
> -	return tz->ops->get_trip_type(tz, trip, buf);
> +
> +	ret = tz->ops->get_trip_type(tz, trip, &trip_type);
> +	if (ret)
> +		return ret;
> +
> +	switch (trip_type) {
> +	case THERMAL_TRIP_CRITICAL:
> +		return sprintf(buf, "critical\n");
> +		break;
> +	case THERMAL_TRIP_HOT:
> +		return sprintf(buf, "hot\n");
> +		break;
> +	case THERMAL_TRIP_PASSIVE:
> +		return sprintf(buf, "passive\n");
> +		break;
> +	case THERMAL_TRIP_ACTIVE:
> +		return sprintf(buf, "active\n");
> +		break;
> +	default:
> +		return sprintf(buf, "unknown\n");
> +		break;
> +	}
>  }
>  
>  static ssize_t
> @@ -160,7 +208,8 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
>  		     char *buf)
>  {
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> -	int trip;
> +	int trip, ret;
> +	int temperature;
>  
>  	if (!tz->ops->get_trip_temp)
>  		return -EPERM;
> @@ -168,7 +217,12 @@ trip_point_temp_show(struct device *dev, struct device_attribute *attr,
>  	if (!sscanf(attr->attr.name, "trip_point_%d_temp", &trip))
>  		return -EINVAL;
>  
> -	return tz->ops->get_trip_temp(tz, trip, buf);
> +	ret = tz->ops->get_trip_temp(tz, trip, &temperature);
> +
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", temperature);
>  }
>  
>  static DEVICE_ATTR(type, 0444, type_show, NULL);
> @@ -236,8 +290,12 @@ thermal_cooling_device_max_state_show(struct device *dev,
>  				      struct device_attribute *attr, char *buf)
>  {
>  	struct thermal_cooling_device *cdev = to_cooling_device(dev);
> +	int state, ret;
>  
> -	return cdev->ops->get_max_state(cdev, buf);
> +	ret = cdev->ops->get_max_state(cdev, &state);
> +	if (ret)
> +		return ret;
> +	return sprintf(buf, "%d\n", state);
>  }
>  
>  static ssize_t
> @@ -245,8 +303,12 @@ thermal_cooling_device_cur_state_show(struct device *dev,
>  				      struct device_attribute *attr, char *buf)
>  {
>  	struct thermal_cooling_device *cdev = to_cooling_device(dev);
> +	int state, ret;
>  
> -	return cdev->ops->get_cur_state(cdev, buf);
> +	ret = cdev->ops->get_cur_state(cdev, &state);
> +	if (ret)
> +		return ret;
> +	return sprintf(buf, "%d\n", state);
>  }
>  
>  static ssize_t
> @@ -312,13 +374,20 @@ static DEVICE_ATTR(name, 0444, name_show, NULL);
>  static ssize_t
>  temp_input_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
> +	long temperature;
> +	int ret;
>  	struct thermal_hwmon_attr *hwmon_attr
>  			= container_of(attr, struct thermal_hwmon_attr, attr);
>  	struct thermal_zone_device *tz
>  			= container_of(hwmon_attr, struct thermal_zone_device,
>  				       temp_input);
>  
> -	return tz->ops->get_temp(tz, buf);
> +	ret = tz->ops->get_temp(tz, &temperature);
> +
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%ld\n", temperature);
>  }
>  
>  static ssize_t
> @@ -330,8 +399,14 @@ temp_crit_show(struct device *dev, struct device_attribute *attr,
>  	struct thermal_zone_device *tz
>  			= container_of(hwmon_attr, struct thermal_zone_device,
>  				       temp_crit);
> +	long temperature;
> +	int ret;
> +
> +	ret = tz->ops->get_trip_temp(tz, 0, &temperature);
> +	if (ret)
> +		return ret;
>  
> -	return tz->ops->get_trip_temp(tz, 0, buf);
> +	return sprintf(buf, "%ld\n", temperature);
>  }
>  
>  
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 917707e..05e56d8 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -28,6 +28,12 @@
>  #include <linux/idr.h>
>  #include <linux/device.h>
>  
> +enum thermal_trip_t { THERMAL_TRIP_CRITICAL, THERMAL_TRIP_HOT,
> +		      THERMAL_TRIP_PASSIVE, THERMAL_TRIP_ACTIVE
> +};
> +
> +enum thermal_mode_t { THERMAL_MODE_KERNEL, THERMAL_MODE_USER };
> +
>  struct thermal_zone_device;
>  struct thermal_cooling_device;
>  
> @@ -36,17 +42,19 @@ struct thermal_zone_device_ops {
>  		     struct thermal_cooling_device *);
>  	int (*unbind) (struct thermal_zone_device *,
>  		       struct thermal_cooling_device *);
> -	int (*get_temp) (struct thermal_zone_device *, char *);
> -	int (*get_mode) (struct thermal_zone_device *, char *);
> -	int (*set_mode) (struct thermal_zone_device *, const char *);
> -	int (*get_trip_type) (struct thermal_zone_device *, int, char *);
> -	int (*get_trip_temp) (struct thermal_zone_device *, int, char *);
> -	int (*get_crit_temp) (struct thermal_zone_device *, unsigned long *);
> +	int (*get_temp) (struct thermal_zone_device *, int *);
> +	int (*get_mode) (struct thermal_zone_device *, enum thermal_mode_t *);
> +	int (*set_mode) (struct thermal_zone_device *, enum thermal_mode_t);
> +	int (*get_trip_type) (struct thermal_zone_device *, int,
> +			      enum thermal_trip_t *);
> +	int (*get_trip_temp) (struct thermal_zone_device *, int,
> +			      int *);
> +	int (*get_crit_temp) (struct thermal_zone_device *, int *);
>  };
>  
>  struct thermal_cooling_device_ops {
> -	int (*get_max_state) (struct thermal_cooling_device *, char *);
> -	int (*get_cur_state) (struct thermal_cooling_device *, char *);
> +	int (*get_max_state) (struct thermal_cooling_device *, unsigned int *);
> +	int (*get_cur_state) (struct thermal_cooling_device *, unsigned int *);
>  	int (*set_cur_state) (struct thermal_cooling_device *, unsigned int);
>  };
>  

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
unsubscribe notice
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/