Hi Guenter,
Thanks for your comments.
On Wed, Oct 20, 2010 at 11:41:45PM -0700, Guenter Roeck wrote:
> Hi Simon,
>
> looking much better. Bunch of additional comments inline.
>
> On Wed, Oct 20, 2010 at 12:37:26PM -0400, Simon Guinot wrote:
> > This patch adds hwmon support for the GPIO connected fan.
> >
> > Platform specific informations as GPIO pinout and speed conversion array
> > (rpm from/to GPIO value) are passed to the driver via platform_data.
> >
> > Signed-off-by: Simon Guinot <sguinot@xxxxxxxxx>
> > ---
> > drivers/hwmon/Kconfig | 9 +
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/gpio-fan.c | 557 ++++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/gpio-fan.h | 36 +++
> > 4 files changed, 603 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/hwmon/gpio-fan.c
> > create mode 100644 include/linux/gpio-fan.h
> >
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 4d4d09b..af318a1 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -368,6 +368,15 @@ config SENSORS_FSCHMD
> > This driver can also be built as a module. If so, the module
> > will be called fschmd.
> >
> > +config SENSORS_GPIO_FAN
> > + tristate "GPIO fan"
> > + depends on GENERIC_GPIO
> > + help
> > + If you say yes here you get support for the GPIO connected fan.
> > +
>
> "for GPIO connected fans.". Can be more than one.
My bad English...
>
> > + This driver can also be built as a module. If so, the module
> > + will be called gpio-fan.
> > +
> > config SENSORS_G760A
> > tristate "GMT G760A"
> > depends on I2C
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index e3c2484..5df7e4a 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -48,6 +48,7 @@ obj-$(CONFIG_SENSORS_F71805F) += f71805f.o
> > obj-$(CONFIG_SENSORS_F71882FG) += f71882fg.o
> > obj-$(CONFIG_SENSORS_F75375S) += f75375s.o
> > obj-$(CONFIG_SENSORS_FSCHMD) += fschmd.o
> > +obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
>
> "gp" is alphabetically after "g7". Please move.
It was the original place...
Ok, I have been confused with the Kconfig "sequence" order: put
SENSORS_GPIO_FAN before SENSORS_G760A seems wrong to me.
Why we don't have to respect the alphabetical order for Kconfig ?
>
> > obj-$(CONFIG_SENSORS_G760A) += g760a.o
> > obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
> > obj-$(CONFIG_SENSORS_GL520SM) += gl520sm.o
> > diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
> > new file mode 100644
> > index 0000000..e3131a6
> > --- /dev/null
> > +++ b/drivers/hwmon/gpio-fan.c
> > @@ -0,0 +1,557 @@
> > +/*
> > + * gpio-fan.c - Hwmon driver for GPIO connected fan.
> > + *
> > + * Copyright (C) 2010 LaCie
> > + *
> > + * Author: Simon Guinot <sguinot@xxxxxxxxx>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/slab.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/irq.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/err.h>
> > +#include <linux/mutex.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/gpio.h>
> > +#include <linux/gpio-fan.h>
> > +
> > +struct gpio_fan_data {
> > + struct platform_device *pdev;
> > + struct device *hwmon_dev;
> > + struct mutex lock; /* lock GPIOs operations. */
> > + int num_ctrl;
> > + unsigned *ctrl;
> > + int num_speed;
> > + struct gpio_fan_speed *speed;
> > + int speed_index;
> > +#ifdef CONFIG_PM
> > + int resume_speed;
> > +#endif
> > + int pwm_enable;
>
> I would prefer a bool here. After all, that is what it is.
As automatic fan speed control is not supported, it make sense.
>
> > + struct gpio_fan_alarm *alarm;
> > + struct work_struct alarm_work;
> > +};
> > +
> > +/*
> > + * Alarm GPIO.
> > + */
> > +
> > +static void fan_alarm_notify(struct work_struct *ws)
> > +{
> > + struct gpio_fan_data *fan_data =
> > + container_of(ws, struct gpio_fan_data, alarm_work);
> > +
> > + sysfs_notify(&fan_data->pdev->dev.kobj, NULL, "fan1_alarm");
> > + kobject_uevent(&fan_data->pdev->dev.kobj, KOBJ_CHANGE);
> > +}
> > +
> > +static irqreturn_t fan_alarm_irq_handler(int irq, void *dev_id)
> > +{
> > + struct gpio_fan_data *fan_data = dev_id;
> > +
> > + schedule_work(&fan_data->alarm_work);
> > +
> > + return IRQ_NONE;
> > +}
> > +
> > +static ssize_t show_fan_alarm(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> > + struct gpio_fan_alarm *alarm = fan_data->alarm;
> > + int value = gpio_get_value(alarm->gpio);
> > +
> > + if (alarm->active_low)
> > + value = !value;
> > +
> > + return sprintf(buf, "%d\n", value);
> > +}
> > +
> > +static DEVICE_ATTR(fan1_alarm, S_IRUGO, show_fan_alarm, NULL);
> > +
> > +static int fan_alarm_init(struct gpio_fan_data *fan_data,
> > + struct gpio_fan_alarm *alarm)
> > +{
> > + int err;
> > + int alarm_irq;
> > + struct platform_device *pdev = fan_data->pdev;
> > +
> > + fan_data->alarm = alarm;
> > +
> > + err = gpio_request(alarm->gpio, "GPIO fan alarm");
> > + if (err)
> > + return err;
> > +
> > + err = gpio_direction_input(alarm->gpio);
> > + if (err)
> > + goto err_free_gpio;
> > +
> > + err = device_create_file(&pdev->dev, &dev_attr_fan1_alarm);
> > + if (err)
> > + goto err_free_gpio;
> > +
> > + alarm_irq = gpio_to_irq(alarm->gpio);
> > + if (alarm_irq < 0)
> > + return 0;
> > +
> This confused me for a bit, so it may confuse others.
> Please add a note indicating that the above is not an error condition.
Ok.
>
> > + INIT_WORK(&fan_data->alarm_work, fan_alarm_notify);
> > + set_irq_type(alarm_irq, IRQ_TYPE_EDGE_BOTH);
> > + err = request_irq(alarm_irq, fan_alarm_irq_handler, IRQF_SHARED,
> > + "GPIO fan alarm", fan_data);
> > + if (err)
> > + goto err_free_sysfs;
> > +
> > + return 0;
> > +
> > +err_free_sysfs:
> > + device_remove_file(&pdev->dev, &dev_attr_fan1_alarm);
> > +err_free_gpio:
> > + gpio_free(alarm->gpio);
> > +
> > + return err;
> > +}
> > +
> > +static void fan_alarm_free(struct gpio_fan_data *fan_data)
> > +{
> > + struct platform_device *pdev = fan_data->pdev;
> > + int alarm_irq = gpio_to_irq(fan_data->alarm->gpio);
> > +
> > + if (alarm_irq >= 0)
> > + free_irq(alarm_irq, fan_data);
> > + device_remove_file(&pdev->dev, &dev_attr_fan1_alarm);
> > + gpio_free(fan_data->alarm->gpio);
> > +}
> > +
> > +/*
> > + * Control GPIOs.
> > + */
> > +
> Please add a note indicating that the function must be called with the lock held
> except during initialization.
> Such as "Must be called with data->update_lock held, except during initialization".
Ok.
>
> > +static void __set_fan_ctrl(struct gpio_fan_data *fan_data, int ctrl_val)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < fan_data->num_ctrl; i++)
> > + gpio_set_value(fan_data->ctrl[i], (ctrl_val >> i) & 1);
> > +}
> > +
> > +static int __get_fan_ctrl(struct gpio_fan_data *fan_data)
> > +{
> > + int i;
> > + int ctrl_val = 0;
> > +
> > + for (i = 0; i < fan_data->num_ctrl; i++) {
> > + int value;
> > +
> > + value = gpio_get_value(fan_data->ctrl[i]);
> > + ctrl_val |= (value << i);
> > + }
> > + return ctrl_val;
> > +}
> > +
>
> "Must be called with data->update_lock held, except during initialization".
Ok.
>
> > +static void set_fan_speed(struct gpio_fan_data *fan_data, int speed_index)
> > +{
> > + if (fan_data->speed_index == speed_index)
> > + return;
> > +
> > + __set_fan_ctrl(fan_data, fan_data->speed[speed_index].ctrl_val);
> > + fan_data->speed_index = speed_index;
> > +}
> > +
> > +static int get_fan_speed(struct gpio_fan_data *fan_data)
>
> get_fan_speed_index() might be a better name. The function doesn't
> really return the speed, but an index to the speed table.
I am not sure how useful is the information provided by the '_index'
suffix. Add '_index' here force to add '_index' everywhere and make
long function names...
Anyway, I will change this.
>
> > +{
> > + int ctrl_val = __get_fan_ctrl(fan_data);
> > + int i;
> > +
> > + for (i = 0; i < fan_data->num_speed; i++)
> > + if (fan_data->speed[i].ctrl_val == ctrl_val)
> > + return i;
> > +
> > + dev_warn(&fan_data->pdev->dev,
> > + "missing speed array entry for GPIO value %d\n", ctrl_val);
> > +
> 0x%x might be better here.
Yes.
>
> > + return -EINVAL;
> > +}
> > +
> > +static int rpm_to_speed(struct gpio_fan_data *fan_data, int rpm)
>
> rpm_to_speed_index would avoid confusion.
>
> > +{
> > + struct gpio_fan_speed *speed = fan_data->speed;
> > + int speed_index = fan_data->num_speed - 1; /* Maximum speed */
>
> You don't really need speed_index here.
>
> > + int i;
> > +
> > + for (i = 0; i < fan_data->num_speed; i++)
> > + if (speed[i].rpm >= rpm) {
>
> Would it make more sense to return the closest rpm instead ?
>
> For example, if one sets a value of 1900, and the closest value
> in the table is 1800, it might make more sense to set the speed
> to 1800 and not to, say, 3000, if that is the next value.
On the other hand, you can set rpm to 500 and the closest value could be
0 and the fan stop (or don't start). That's a disturbing experience.
If you agree, I prefer to set the upper speed.
>
> No strong opinion, though. One might as well argue that it is safer
> to run at the higher speed.
>
Some fans have problems with the low speeds and needs a little boost to
start up.
> > + speed_index = i;
> > + break;
>
> return i;
>
> > + }
> > + return speed_index;
>
> return fan_data->num_speed - 1;
Yes.
>
> > +}
> > +
> > +static ssize_t show_pwm(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> > + u8 pwm = DIV_ROUND_CLOSEST(fan_data->speed_index * 255,
> > + fan_data->num_speed - 1);
> > +
> > + return sprintf(buf, "%d\n", pwm);
> > +}
> > +
> > +static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> > + unsigned long pwm;
> > + int speed_index;
> > + int ret = count;
> > +
> > + if (strict_strtoul(buf, 10, &pwm) || pwm > 255)
> > + return -EINVAL;
> > +
> > + mutex_lock(&fan_data->lock);
> > +
> > + if (fan_data->pwm_enable != 1) {
>
> if (!fan_data->pwm_enable) {
>
> > + ret = -EPERM;
> > + goto exit_unlock;
> > + }
> > +
> > + speed_index = DIV_ROUND_UP(pwm * (fan_data->num_speed - 1), 255);
>
> The combination of DIV_ROUND_UP() and DIV_ROUND_CLOSEST() causes inconsistency.
>
> Assume num_speed = 8, pwm is set to 128.
>
> set: 128 * (8 - 1) / 255 = 3.513 ==> 4
> get: 4 * 255 / (8 - 1) = 145.7 ==> 146
> set: 146 * (8 - 1) / 255 = 4.007 ==> 5
> get: 5 * 255 / (8 - 1) = 182.142 ==> 182
> set: 182 * (8 - 1) / 255 = 4.996 ==> 5
>
> Unless there is a really good reason to use DIV_ROUND_UP(), you might
> want to use DIV_ROUND_CLOSEST() instead.
This choice is coherent with the rpm interface one and the reason is the
same: start the fan even with a low value. In your example, 36 is first
speed threshold.
>
> > + set_fan_speed(fan_data, speed_index);
> > +
> > +exit_unlock:
> > + mutex_unlock(&fan_data->lock);
> > +
> > + return ret;
> > +}
> > +
> > +static ssize_t show_pwm_enable(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> > +
> > + return sprintf(buf, "%d\n", fan_data->pwm_enable);
> > +}
> > +
> > +static ssize_t set_pwm_enable(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> > + unsigned long val;
> > +
> > + if (strict_strtoul(buf, 10, &val) || val > 1)
> > + return -EINVAL;
> > +
> > + if (fan_data->pwm_enable == val)
> > + return count;
> > +
> > + mutex_lock(&fan_data->lock);
> > +
> > + fan_data->pwm_enable = val;
> > +
> > + /* Disable manual control mode: set fan at full speed. */
> > + if (val == 0)
> > + set_fan_speed(fan_data, fan_data->num_speed - 1);
> > +
> > + mutex_unlock(&fan_data->lock);
> > +
> > + return count;
> > +}
> > +
> > +static ssize_t show_pwm_mode(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + return sprintf(buf, "0\n");
> > +}
> > +
> > +static ssize_t show_rpm_min(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> > +
> > + return sprintf(buf, "%d\n", fan_data->speed[0].rpm);
> > +}
> > +
> > +static ssize_t show_rpm_max(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> > +
> > + return sprintf(buf, "%d\n",
> > + fan_data->speed[fan_data->num_speed - 1].rpm);
> > +}
> > +
> > +static ssize_t show_rpm(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> > +
> > + return sprintf(buf, "%d\n", fan_data->speed[fan_data->speed_index].rpm);
> > +}
> > +
> > +static ssize_t set_rpm(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct gpio_fan_data *fan_data = dev_get_drvdata(dev);
> > + unsigned long rpm;
> > + int ret = count;
> > +
> > + if (strict_strtoul(buf, 10, &rpm))
> > + return -EINVAL;
> > +
> > + mutex_lock(&fan_data->lock);
> > +
> > + if (fan_data->pwm_enable != 1) {
>
> if (!fan_data->pwm_enable) {
Ok.
>
> > + ret = -EPERM;
> > + goto exit_unlock;
> > + }
> > +
> > + set_fan_speed(fan_data, rpm_to_speed(fan_data, rpm));
> > +
> > +exit_unlock:
> > + mutex_unlock(&fan_data->lock);
> > +
> > + return ret;
> > +}
> > +
> > +static DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm, set_pwm);
> > +static DEVICE_ATTR(pwm1_enable, S_IRUGO | S_IWUSR,
> > + show_pwm_enable, set_pwm_enable);
> > +static DEVICE_ATTR(pwm1_mode, S_IRUGO, show_pwm_mode, NULL);
> > +static DEVICE_ATTR(fan1_min, S_IRUGO, show_rpm_min, NULL);
> > +static DEVICE_ATTR(fan1_max, S_IRUGO, show_rpm_max, NULL);
> > +static DEVICE_ATTR(fan1_input, S_IRUGO, show_rpm, NULL);
> > +static DEVICE_ATTR(fan1_target, S_IRUGO | S_IWUSR, show_rpm, set_rpm);
> > +
> > +static struct attribute *gpio_fan_ctrl_attributes[] = {
> > + &dev_attr_pwm1.attr,
> > + &dev_attr_pwm1_enable.attr,
> > + &dev_attr_pwm1_mode.attr,
> > + &dev_attr_fan1_input.attr,
> > + &dev_attr_fan1_target.attr,
> > + &dev_attr_fan1_min.attr,
> > + &dev_attr_fan1_max.attr,
> > + NULL
> > +};
> > +
> > +static const struct attribute_group gpio_fan_ctrl_group = {
> > + .attrs = gpio_fan_ctrl_attributes,
> > +};
> > +
> > +static int fan_ctrl_init(struct gpio_fan_data *fan_data,
> > + struct gpio_fan_platform_data *pdata)
> > +{
> > + struct platform_device *pdev = fan_data->pdev;
> > + int num_ctrl = pdata->num_ctrl;
> > + unsigned *ctrl = pdata->ctrl;
> > + int i, err;
> > +
> > + for (i = 0; i < num_ctrl; i++) {
> > + err = gpio_request(ctrl[i], "GPIO fan control");
> > + if (err)
> > + goto err_free_gpio;
> > +
> > + err = gpio_direction_output(ctrl[i], gpio_get_value(ctrl[i]));
> > + if (err) {
> > + gpio_free(ctrl[i]);
> > + goto err_free_gpio;
> > + }
> > + }
> > +
> > + err = sysfs_create_group(&pdev->dev.kobj, &gpio_fan_ctrl_group);
> > + if (err)
> > + goto err_free_gpio;
> > +
> > + fan_data->num_ctrl = num_ctrl;
> > + fan_data->ctrl = ctrl;
> > + fan_data->num_speed = pdata->num_speed;
> > + fan_data->speed = pdata->speed;
> > + fan_data->pwm_enable = 1; /* Enable manual fan speed control. */
>
> fan_data->pwm_enable = true;
Ok.
>
> > + fan_data->speed_index = get_fan_speed(fan_data);
> > + if (fan_data->speed_index < 0) {
> > + /* Set fan speed to a known value. */
> > + set_fan_speed(fan_data, fan_data->num_speed / 2);
> > + dev_info(&pdev->dev, "set fan speed to %d rpm\n",
> > + fan_data->speed[fan_data->speed_index].rpm);
>
> I think a safer approach would be to return -ENODEV in this case.
> Better not to try any fan control at all if the GPIO bits report
> an unsupported value. It indicates that something is wrong.
Ok.
>
> > + }
> > +
> > + return 0;
> > +
> > +err_free_gpio:
> > + for (i = i - 1; i >= 0; i--)
> > + gpio_free(ctrl[i]);
> > +
> > + return err;
> > +}
> > +
> > +static void fan_ctrl_free(struct gpio_fan_data *fan_data)
> > +{
> > + struct platform_device *pdev = fan_data->pdev;
> > + int i;
> > +
> > + sysfs_remove_group(&pdev->dev.kobj, &gpio_fan_ctrl_group);
> > + for (i = 0; i < fan_data->num_ctrl; i++)
> > + gpio_free(fan_data->ctrl[i]);
> > +}
> > +
> > +/*
> > + * Platform driver.
> > + */
> > +
> > +static ssize_t show_name(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > + return sprintf(buf, "gpio-fan\n");
> > +}
> > +
> > +static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
> > +
> > +static int __devinit gpio_fan_probe(struct platform_device *pdev)
> > +{
> > + int err = 0;
> > + struct gpio_fan_data *fan_data;
> > + struct gpio_fan_platform_data *pdata = pdev->dev.platform_data;
> > +
> > + if (!pdata)
> > + return -EINVAL;
> > +
> > + fan_data = kzalloc(sizeof(struct gpio_fan_data), GFP_KERNEL);
> > + if (!fan_data)
> > + return -ENOMEM;
> > +
> > + fan_data->pdev = pdev;
> > + platform_set_drvdata(pdev, fan_data);
> > + mutex_init(&fan_data->lock);
> > +
> > + /* Configure alarm GPIO if available. */
> > + if (pdata->alarm) {
> > + err = fan_alarm_init(fan_data, pdata->alarm);
> > + if (err)
> > + goto err_free_data;
> > + }
> > +
> > + /* Configure control GPIOs if available. */
> > + if (pdata->ctrl && pdata->num_ctrl) {
>
> You might want to check for pdata->num_ctrl > 0 here.
> It could be negative, which would be bad.
Yes.
>
> > + if (!pdata->num_speed || !pdata->speed) {
>
> num_speed must be > 1. Otherwise there is a risk of division by zero if it is == 1.
Yes.
>
> > + err = -EINVAL;
> > + goto err_free_alarm;
> > + }
> > + err = fan_ctrl_init(fan_data, pdata);
> > + if (err)
> > + goto err_free_alarm;
> > + }
> > +
> > + err = device_create_file(&pdev->dev, &dev_attr_name);
> > + if (err)
> > + goto err_free_ctrl;
> > +
> > + /* Make this driver part of hwmon class. */
> > + fan_data->hwmon_dev = hwmon_device_register(&pdev->dev);
> > + if (IS_ERR(fan_data->hwmon_dev)) {
> > + err = PTR_ERR(fan_data->hwmon_dev);
> > + goto err_remove_name;
> > + }
> > +
> > + dev_info(&pdev->dev, "GPIO fan initialized\n");
> > +
> > + return 0;
> > +
> > +err_remove_name:
> > + device_remove_file(&pdev->dev, &dev_attr_name);
> > +err_free_ctrl:
> > + if (fan_data->ctrl)
> > + fan_ctrl_free(fan_data);
> > +err_free_alarm:
> > + if (fan_data->alarm)
> > + fan_alarm_free(fan_data);
> > +err_free_data:
> > + platform_set_drvdata(pdev, NULL);
> > + kfree(fan_data);
> > +
> > + return err;
> > +}
> > +
> > +static int __devexit gpio_fan_remove(struct platform_device *pdev)
> > +{
> > + struct gpio_fan_data *fan_data = platform_get_drvdata(pdev);
> > +
> > + hwmon_device_unregister(fan_data->hwmon_dev);
> > + device_remove_file(&pdev->dev, &dev_attr_name);
> > + if (fan_data->alarm)
> > + fan_alarm_free(fan_data);
> > + if (fan_data->ctrl)
> > + fan_ctrl_free(fan_data);
> > + kfree(fan_data);
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_PM
> > +static int gpio_fan_suspend(struct platform_device *pdev, pm_message_t state)
> > +{
> > + struct gpio_fan_data *fan_data = platform_get_drvdata(pdev);
> > +
> > + if (fan_data->ctrl) {
> > + fan_data->resume_speed = fan_data->speed_index;
> > + set_fan_speed(fan_data, 0);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int gpio_fan_resume(struct platform_device *pdev)
> > +{
> > + struct gpio_fan_data *fan_data = platform_get_drvdata(pdev);
> > +
> > + if (fan_data->ctrl)
> > + set_fan_speed(fan_data, fan_data->resume_speed);
> > +
> > + return 0;
> > +}
> > +#else
> > +#define gpio_fan_suspend NULL
> > +#define gpio_fan_resume NULL
> > +#endif
> > +
> > +static struct platform_driver gpio_fan_driver = {
> > + .probe = gpio_fan_probe,
> > + .remove = __devexit_p(gpio_fan_remove),
> > + .suspend = gpio_fan_suspend,
> > + .resume = gpio_fan_resume,
> > + .driver = {
> > + .name = "gpio-fan",
> > + },
> > +};
> > +
> > +static int __init gpio_fan_init(void)
> > +{
> > + return platform_driver_register(&gpio_fan_driver);
> > +}
> > +
> > +static void __exit gpio_fan_exit(void)
> > +{
> > + platform_driver_unregister(&gpio_fan_driver);
> > +}
> > +
> > +module_init(gpio_fan_init);
> > +module_exit(gpio_fan_exit);
> > +
> > +MODULE_AUTHOR("Simon Guinot <sguinot@xxxxxxxxx>");
> > +MODULE_DESCRIPTION("GPIO FAN driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:gpio-fan");
> > diff --git a/include/linux/gpio-fan.h b/include/linux/gpio-fan.h
> > new file mode 100644
> > index 0000000..0966591
> > --- /dev/null
> > +++ b/include/linux/gpio-fan.h
> > @@ -0,0 +1,36 @@
> > +/*
> > + * include/linux/gpio-fan.h
> > + *
> > + * Platform data structure for GPIO fan driver
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#ifndef __LINUX_GPIO_FAN_H
> > +#define __LINUX_GPIO_FAN_H
> > +
> > +struct gpio_fan_alarm {
> > + unsigned gpio;
> > + unsigned active_low;
> > +};
> > +
> > +struct gpio_fan_speed {
> > + int rpm;
> > + int ctrl_val;
> > +};
> > +
> > +struct gpio_fan_platform_data {
> > + int num_ctrl;
> > + unsigned *ctrl; /* fan control GPIOs. */
> > + struct gpio_fan_alarm *alarm; /* fan alarm GPIO. */
> > + /*
> > + * Speed conversion array: rpm from/to GPIO bit field.
> > + * This array _must_ be sorted in ascending rpm order.
> > + */
> > + int num_speed;
> > + struct gpio_fan_speed *speed;
> > +};
> > +
> > +#endif /* __LINUX_GPIO_FAN_H */
> > --
> > 1.6.3.1
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Attachment:
signature.asc
Description: Digital signature
_______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
