On Tue, 8 May 2012 21:48:17 +0530 Amit Daniel Kachhap <amit.kachhap@xxxxxxxxxx> wrote: > This code added creates a link between temperature sensors, linux thermal > framework and cooling devices for samsung exynos platform. This layer > monitors the temperature from the sensor and informs the generic thermal > layer to take the necessary cooling action. > > > ... > > +static void exynos_report_trigger(void) > +{ > + unsigned int i; > + char data[2]; > + char *envp[] = { data, NULL }; > + > + if (!th_zone || !th_zone->therm_dev) > + return; > + > + thermal_zone_device_update(th_zone->therm_dev); > + > + mutex_lock(&th_zone->therm_dev->lock); > + /* Find the level for which trip happened */ > + for (i = 0; i < th_zone->sensor_conf->trip_data.trip_count; i++) { > + if (th_zone->therm_dev->last_temperature < > + th_zone->sensor_conf->trip_data.trip_val[i] * 1000) > + break; > + } > + > + if (th_zone->mode == THERMAL_DEVICE_ENABLED) { > + if (i > 0) > + th_zone->therm_dev->polling_delay = ACTIVE_INTERVAL; > + else > + th_zone->therm_dev->polling_delay = IDLE_INTERVAL; > + } > + > + sprintf(data, "%u", i); yikes, if `i' exceeds 9, we have a stack scribble. Please review this and at least use snprintf(... sizeof(data)) to prevent accidents. > + kobject_uevent_env(&th_zone->therm_dev->device.kobj, KOBJ_CHANGE, envp); > + mutex_unlock(&th_zone->therm_dev->lock); > +} > + > > ... > > +/* Get trip temperature callback functions for thermal zone */ > +static int exynos_get_trip_temp(struct thermal_zone_device *thermal, int trip, > + unsigned long *temp) > +{ > + if (trip < 0 || trip > 2) I don't know what `trip' does and I don't know the meaning of the values 0, 1 and 2. Documentation, please. > + return -EINVAL; > + > + *temp = th_zone->sensor_conf->trip_data.trip_val[trip]; > + /* convert the temperature into millicelsius */ > + *temp = *temp * 1000; > + > + return 0; > +} > + > > ... > > +/* Bind callback functions for thermal zone */ > +static int exynos_bind(struct thermal_zone_device *thermal, > + struct thermal_cooling_device *cdev) > +{ > + int ret = 0; > + > + /* if the cooling device is the one from exynos4 bind it */ > + if (cdev != th_zone->cool_dev[0]) > + return 0; > + > + if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) { > + pr_err("error binding cooling dev inst 0\n"); > + return -EINVAL; > + } > + if (thermal_zone_bind_cooling_device(thermal, 1, cdev)) { > + pr_err("error binding cooling dev inst 1\n"); > + ret = -EINVAL; > + goto error_bind1; > + } There can never be more than two instances? > + return ret; > +error_bind1: > + thermal_zone_unbind_cooling_device(thermal, 0, cdev); > + return ret; > +} > + > > ... > > +/* Get temperature callback functions for thermal zone */ > +static int exynos_get_temp(struct thermal_zone_device *thermal, > + unsigned long *temp) > +{ > + void *data; > + > + if (!th_zone->sensor_conf) { > + pr_info("Temperature sensor not initialised\n"); > + return -EINVAL; > + } > + data = th_zone->sensor_conf->private_data; > + *temp = th_zone->sensor_conf->read_temperature(data); > + /* convert the temperature into millicelsius */ > + *temp = *temp * 1000; > + return 0; > +} > + > +/* Operation callback functions for thermal zone */ > +static struct thermal_zone_device_ops exynos_dev_ops = { Can it be const? That sometimes saves space, as the table doesn't need to be moved into writeable storage at runtime. > + .bind = exynos_bind, > + .unbind = exynos_unbind, > + .get_temp = exynos_get_temp, > + .get_mode = exynos_get_mode, > + .set_mode = exynos_set_mode, > + .get_trip_type = exynos_get_trip_type, > + .get_trip_temp = exynos_get_trip_temp, > + .get_crit_temp = exynos_get_crit_temp, > +}; > + > +/* Register with the in-kernel thermal management */ > +static int exynos_register_thermal(struct thermal_sensor_conf *sensor_conf) > +{ > + int ret, count, tab_size; > + struct freq_clip_table *tab_ptr; > + > + if (!sensor_conf || !sensor_conf->read_temperature) { > + pr_err("Temperature sensor not initialised\n"); > + return -EINVAL; > + } > + > + th_zone = kzalloc(sizeof(struct exynos_thermal_zone), GFP_KERNEL); > + if (!th_zone) { > + ret = -ENOMEM; > + goto err_unregister; This seems wrong? If we need to call exynos_unregister_thermal() on this error path then we needed to call it on the predecing one? Perhaps? > + } > + > + th_zone->sensor_conf = sensor_conf; > + > + tab_ptr = (struct freq_clip_table *)sensor_conf->cooling_data.freq_data; > + tab_size = sensor_conf->cooling_data.freq_clip_count; > + > + /* Register the cpufreq cooling device */ > + th_zone->cool_dev_size = 1; > + count = 0; > + th_zone->cool_dev[count] = cpufreq_cooling_register( > + (struct freq_clip_table *)&(tab_ptr[count]), > + tab_size, cpumask_of(0)); > + > + if (IS_ERR(th_zone->cool_dev[count])) { > + pr_err("Failed to register cpufreq cooling device\n"); > + ret = -EINVAL; > + th_zone->cool_dev_size = 0; > + goto err_unregister; > + } > + > + th_zone->therm_dev = thermal_zone_device_register(sensor_conf->name, > + 3, NULL, &exynos_dev_ops, 0, 0, 0, IDLE_INTERVAL); > + > + if (IS_ERR(th_zone->therm_dev)) { > + pr_err("Failed to register thermal zone device\n"); > + ret = -EINVAL; > + goto err_unregister; > + } > + th_zone->mode = THERMAL_DEVICE_ENABLED; > + > + pr_info("Exynos: Kernel Thermal management registered\n"); > + > + return 0; > + > +err_unregister: > + exynos_unregister_thermal(); > + return ret; > +} > + > > ... > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html