[PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller
Guenter Roeck
linux at roeck-us.net
Thu Apr 25 09:37:39 EST 2013
On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote:
>
> Signed-off-by: Arnaud Ebalard <arno at natisbad.org>
> Tested-by: Arnaud Ebalard <arno at natisbad.org>
Tested-by is not needed here; I sure hope you tested your own code.
It is only used if someone else tested it.
> ---
> drivers/hwmon/Kconfig | 10 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/g762.c | 1058 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1069 insertions(+)
> create mode 100644 drivers/hwmon/g762.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 89ac1cb..cb4879c 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -423,6 +423,16 @@ config SENSORS_G760A
> This driver can also be built as a module. If so, the module
> will be called g760a.
>
> +config SENSORS_G762
> + tristate "GMT G762 and G763"
> + depends on I2C
> + help
> + If you say yes here you get support for Global Mixed-mode
> + Technology Inc G762 and G763 fan speed PWM controller chips.
> +
> + This driver can also be built as a module. If so, the module
> + will be called g762.
> +
> config SENSORS_GL518SM
> tristate "Genesys Logic GL518SM"
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 8d6d97e..4b49504 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_SENSORS_F75375S) += f75375s.o
> obj-$(CONFIG_SENSORS_FAM15H_POWER) += fam15h_power.o
> obj-$(CONFIG_SENSORS_FSCHMD) += fschmd.o
> obj-$(CONFIG_SENSORS_G760A) += g760a.o
> +obj-$(CONFIG_SENSORS_G762) += g762.o
> obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
> obj-$(CONFIG_SENSORS_GL520SM) += gl520sm.o
> obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
> diff --git a/drivers/hwmon/g762.c b/drivers/hwmon/g762.c
> new file mode 100644
> index 0000000..810b019
> --- /dev/null
> +++ b/drivers/hwmon/g762.c
> @@ -0,0 +1,1058 @@
> +/*
> + * g762 - Driver for the Global Mixed-mode Technology Inc. fan speed PWM
> + * controller chips from G762 family, i.e. G762 and G763
> + *
> + * Copyright (C) 2013, Arnaud EBALARD <arno at natisbad.org>
> + *
> + * This work is based on a basic version for 2.6.31 kernel developed
> + * by Olivier Mouchet for LaCie NAS. Updates have been performed to run
> + * on recent kernels. Additional features have been added. Ability to
> + * configure various characteristics via .dts file has been added.
> + * Detailed datasheet on which this development is based is available
> + * here:
> + *
> + * http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf
> + *
> + * Headers from previous developments have been kept below:
> + *
> + * Copyright (c) 2009 LaCie
> + *
> + * Author: Olivier Mouchet <olivier.mouchet at gmail.com>
> + *
> + * based on g760a code written by Herbert Valerio Riedel <hvr at gnu.org>
> + * Copyright (C) 2007 Herbert Valerio Riedel <hvr at gnu.org>
> + *
> + * g762: minimal datasheet available at:
> + * http://www.gmt.com.tw/product/datasheet/EDS-762_3.pdf
> + *
> + * 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
Please drop the address.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
Is this include needed ?
> +#include <linux/jiffies.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +
> +#define DRVNAME "g762"
> +
> +static const struct i2c_device_id g762_id[] = {
> + { "g762", 0 },
> + { "g763", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, g762_id);
> +
> +enum g762_regs {
> + G762_REG_SET_CNT = 0x00,
> + G762_REG_ACT_CNT = 0x01,
> + G762_REG_FAN_STA = 0x02,
> + G762_REG_SET_OUT = 0x03,
> + G762_REG_FAN_CMD1 = 0x04,
> + G762_REG_FAN_CMD2 = 0x05,
> +};
> +
> +/* Config register bits */
> +#define G762_REG_FAN_CMD1_DET_FAN_FAIL 0x80 /* enable fan_fail signal */
> +#define G762_REG_FAN_CMD1_DET_FAN_OOC 0x40 /* enable fan_out_of_control */
> +#define G762_REG_FAN_CMD1_OUT_MODE 0x20 /* out mode, pwm or dac */
> +#define G762_REG_FAN_CMD1_FAN_MODE 0x10 /* fan mode: close or open loop */
> +#define G762_REG_FAN_CMD1_CLK_DIV_ID1 0x08 /* clock divisor value */
> +#define G762_REG_FAN_CMD1_CLK_DIV_ID0 0x04
> +#define G762_REG_FAN_CMD1_PWM_POLARITY 0x02 /* pwm polarity */
> +#define G762_REG_FAN_CMD1_PULSE_PER_REV 0x01 /* pulse per fan revolution */
> +
> +#define G762_REG_FAN_CMD2_GEAR_MODE_1 0x08 /* fan gear mode */
> +#define G762_REG_FAN_CMD2_GEAR_MODE_0 0x04
> +#define G762_REG_FAN_CMD2_FAN_STARTV_1 0x02 /* fan startup voltage */
> +#define G762_REG_FAN_CMD2_FAN_STARTV_0 0x01
> +
> +#define G762_REG_FAN_STA_FAIL 0x02 /* fan fail */
> +#define G762_REG_FAN_STA_OOC 0x01 /* fan out of control */
> +
> +/* config register values */
> +#define OUT_MODE_PWM 1
> +#define OUT_MODE_DAC 0
> +
> +#define FAN_MODE_CLOSED_LOOP 2
> +#define FAN_MODE_OPEN_LOOP 1
> +
> +/* register data is read (and cached) at most once per second */
> +#define G762_UPDATE_INTERVAL (HZ)
( ) are not needed here.
> +
> +/*
> + * extract pulse count per fan revolution value (2 or 4) from given
> + * FAN_CMD1 register value
> + */
> +#define PULSE_FROM_REG(reg) \
> + ((((reg) & G762_REG_FAN_CMD1_PULSE_PER_REV)+1) << 1)
> +
> +/*
> + * extract fan clock divisor (1, 2, 4 or 8) from given FAN_CMD1
> + * register value
> + */
> +#define CLKDIV_FROM_REG(reg) \
> + (1 << (((reg) & (G762_REG_FAN_CMD1_CLK_DIV_ID0 | \
> + G762_REG_FAN_CMD1_CLK_DIV_ID1)) >> 2))
> +
> +/*
> + * extract fan gear mode value (0, 1 or 2) from given FAN_CMD2
> + * register value
> + */
> +#define GEARMODE_FROM_REG(reg) \
> + (1 << (((reg) & (G762_REG_FAN_CMD2_GEAR_MODE_0 | \
> + G762_REG_FAN_CMD2_GEAR_MODE_1)) >> 2))
> +
> +struct g762_data {
> + struct i2c_client *client;
> + struct device *hwmon_dev;
> +
> + /* update mutex */
> + struct mutex update_lock;
> +
> + /* board specific parameters. */
> + u32 clk; /* default 32kHz */
> +
> + /* g762 register cache */
> + bool valid;
> + unsigned long last_updated; /* in jiffies */
> +
> + u8 set_cnt; /* RPM cmd in close loop control */
> + u8 act_cnt; /* formula: cnt = (CLK * 30)/(rpm * P) */
> + u8 fan_sta; /* bit 0: set when actual fan speed is more than
> + * 25% outside requested fan speed
> + * bit 1: set when no transition occurs on fan
> + * pin for 0.7s
> + */
> + u8 set_out; /* output voltage/PWM duty in open loop control */
> + u8 fan_cmd1; /* 0: FG_PLS_ID0 FG pulses count per revolution
> + * 0: 2 counts per revolution
> + * 1: 4 counts per revolution
> + * 1: PWM_POLARITY 1: negative_duty
> + * 0: positive_duty
> + * 2,3: [FG_CLOCK_ID0, FG_CLK_ID1]
> + * 00: Divide fan clock by 1
> + * 01: Divide fan clock by 2
> + * 10: Divide fan clock by 4
> + * 11: Divide fan clock by 8
> + * 4: FAN_MODE 1:close-loop, 0:open-loop
> + * 5: OUT_MODE 1:PWM, 0:DAC
> + * 6: DET_FAN_OOC enable "fan ooc" status
> + * 7: DET_FAN_FAIL enable "fan fail" status
> + */
> + u8 fan_cmd2; /* 0,1: FAN_STARTV 0,1,2,3 -> 0,32,64,96 dac_code
> + * 2,3: FG_GEAR_MODE
> + * 00: div = 1
> + * 01: div = 2
> + * 10: div = 4
> + * 4: Mask ALERT# (g763 only)
> + */
> +};
> +
> +/*
> + * sysfs PWM interface uses value from 0 to 255 when g762 FAN_SET_CNT register
> + * uses values from 255 (off) to 0 (full speed).
> + */
> +#define PWM_FROM_CNT(cnt) (0xff-(cnt))
> +#define PWM_TO_CNT(pwm) (0xff-(pwm))
Please use standard coding style spacing rules for spaces around bianry
operators (ie space before and behind).
> +
> +/*
> + * Convert count value from fan controller register into fan speed RPM value.
> + * Note that the datasheet documents a basic formula. Influence of additional
> + * parameters have been infered from examples in the datasheet and tests:
> + * fan clock divisor, fan gear mode.
> + */
> +static inline unsigned int rpm_from_cnt(u8 cnt, u32 clk, u16 p,
> + u8 clk_div, u8 gear_mode)
> +{
> + if (cnt == 0 || cnt == 0xff)
> + return 0;
> +
> + return (clk*30*(gear_mode+1))/(cnt*p*clk_div);
> +}
> +
> +/* Convert fan RPM value from sysfs into count value */
> +static inline unsigned char cnt_from_rpm(u32 rpm, u32 clk, u16 p,
> + u8 clk_div, u8 gear_mode)
> +{
> + return (rpm == 0) ? 0xff : (clk*30*(gear_mode+1))/(rpm*p*clk_div);
> +}
> +
> +static inline int g762_read_value(struct i2c_client *client,
> + enum g762_regs reg)
> +{
> + return i2c_smbus_read_byte_data(client, reg);
> +}
> +
> +static inline int g762_write_value(struct i2c_client *client,
> + enum g762_regs reg, u8 value)
> +{
> + return i2c_smbus_write_byte_data(client, reg, value);
> +}
AFAICS the retrun value from g762_write_value is never checked.
Either check it, or make it a void.
[ I would prefer if you would drop the above two functions entirely;
I don' see the value in having them ]
> +
> +/* helper to grab and cache data, at most one time per second */
> +static struct g762_data *g762_update_client(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = i2c_get_clientdata(client);
> +
> + mutex_lock(&data->update_lock);
> + if (time_after(jiffies, data->last_updated + G762_UPDATE_INTERVAL) ||
> + unlikely(!data->valid)) {
> + data->set_cnt = g762_read_value(client, G762_REG_SET_CNT);
> + data->act_cnt = g762_read_value(client, G762_REG_ACT_CNT);
> + data->fan_sta = g762_read_value(client, G762_REG_FAN_STA);
> + data->set_out = g762_read_value(client, G762_REG_SET_OUT);
> + data->fan_cmd1 = g762_read_value(client, G762_REG_FAN_CMD1);
> + data->fan_cmd2 = g762_read_value(client, G762_REG_FAN_CMD2);
> +
> + data->last_updated = jiffies;
> + data->valid = 1;
= true;
Sure you don't want to check for errors ?
> + }
> + mutex_unlock(&data->update_lock);
> +
> + return data;
> +}
> +
> +
> +
One empty line is sufficient.
> +/*
> + * helpers for passing hardware characteristics via DT. Some of those
> + * are also used by sysfs handlers (write function) later in the file.
> + */
> +
> +
Same here and elsewhere.
> +/* Set pwm mode. Accepts either 0 (pwm mode) or 1 (linear mode) */
> +static int do_set_pwm_mode(struct device *dev, unsigned long val)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = 0;
> +
> + mutex_lock(&data->update_lock);
> + switch (val) {
> + case OUT_MODE_PWM:
> + data->fan_cmd1 |= G762_REG_FAN_CMD1_OUT_MODE;
> + break;
> + case OUT_MODE_DAC: /* i.e. linear mode */
> + data->fan_cmd1 &= ~G762_REG_FAN_CMD1_OUT_MODE;
> + break;
> + default:
> + ret = -EINVAL;
> + goto out;
> + }
> + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1);
> + out:
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +/*
> + * Set reference clock. Accepted values are between 0 and 0xffffff.
> + * Note that this is an internal parameter, i.e. value is not passed
> + * to the device.
> + */
> +static int do_set_pwm_freq(struct device *dev, unsigned long val)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = i2c_get_clientdata(client);
> + int ret = -EINVAL;
> +
Seems to me that
if (val > 0xffffff)
return -EINVAL;
data->clk = val;
return 0;
would be a bit simpler.
> + if (val <= 0xffffff) {
> + data->clk = val;
> + ret = 0;
> + }
> +
> + return ret;
> +}
> +
> +/* Set fan clock divisor. Accepted values are 1, 2, 4 and 8. */
> +static int do_set_fan_div(struct device *dev, unsigned long val)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = 0;
> +
> + mutex_lock(&data->update_lock);
> + switch (val) {
> + case 1:
> + data->fan_cmd1 &= ~G762_REG_FAN_CMD1_CLK_DIV_ID0;
> + data->fan_cmd1 &= ~G762_REG_FAN_CMD1_CLK_DIV_ID1;
> + break;
> + case 2:
> + data->fan_cmd1 |= G762_REG_FAN_CMD1_CLK_DIV_ID0;
> + data->fan_cmd1 &= ~G762_REG_FAN_CMD1_CLK_DIV_ID1;
> + break;
> + case 4:
> + data->fan_cmd1 &= ~G762_REG_FAN_CMD1_CLK_DIV_ID0;
> + data->fan_cmd1 |= G762_REG_FAN_CMD1_CLK_DIV_ID1;
> + break;
> + case 8:
> + data->fan_cmd1 |= G762_REG_FAN_CMD1_CLK_DIV_ID0;
> + data->fan_cmd1 |= G762_REG_FAN_CMD1_CLK_DIV_ID1;
> + break;
> + default:
> + ret = -EINVAL;
> + goto out;
> + }
> + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1);
> + out:
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +/* Set fan gear mode. Accepted values are either 0, 1 or 2. */
> +static int do_set_fan_gear_mode(struct device *dev, unsigned long val)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = 0;
> +
> + mutex_lock(&data->update_lock);
> + switch (val) {
> + case 0:
> + data->fan_cmd2 &= ~G762_REG_FAN_CMD2_GEAR_MODE_0;
> + data->fan_cmd2 &= ~G762_REG_FAN_CMD2_GEAR_MODE_1;
> + break;
> + case 1:
> + data->fan_cmd2 |= G762_REG_FAN_CMD2_GEAR_MODE_0;
> + data->fan_cmd2 &= ~G762_REG_FAN_CMD2_GEAR_MODE_1;
> + break;
> + case 2:
> + data->fan_cmd2 &= ~G762_REG_FAN_CMD2_GEAR_MODE_0;
> + data->fan_cmd2 |= G762_REG_FAN_CMD2_GEAR_MODE_1;
> + break;
> + default:
> + ret = -EINVAL;
> + goto out;
> + }
> + g762_write_value(client, G762_REG_FAN_CMD2, data->fan_cmd2);
> + out:
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +/* Set pulse per revolution value. Accepts either 2 or 4. */
> +static int do_set_fan_pulses(struct device *dev, unsigned long val)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = 0;
> +
> + mutex_lock(&data->update_lock);
> + switch (val) {
> + case 2:
> + data->fan_cmd1 &= ~G762_REG_FAN_CMD1_PULSE_PER_REV;
> + break;
> + case 4:
> + data->fan_cmd1 |= G762_REG_FAN_CMD1_PULSE_PER_REV;
> + break;
> + default:
> + ret = -EINVAL;
> + goto out;
> + }
> + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1);
> + out:
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +/* Set fan mode. Accepts either 1 (open loop) or 2 (closed loop). */
> +static int do_set_pwm_enable(struct device *dev, unsigned long val)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = 0;
> +
> + mutex_lock(&data->update_lock);
> + switch (val) {
> + case FAN_MODE_OPEN_LOOP:
> + data->fan_cmd1 &= ~G762_REG_FAN_CMD1_FAN_MODE;
> + break;
> + case FAN_MODE_CLOSED_LOOP:
> + data->fan_cmd1 |= G762_REG_FAN_CMD1_FAN_MODE;
> + break;
> + default:
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1);
> + out:
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +/* Set pwm polarity (0 for negative duty, 1 for positive duty) */
> +static int do_set_pwm_polarity(struct device *dev, unsigned long val)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = 0;
> +
> + mutex_lock(&data->update_lock);
> + switch (val) {
> + case 0: /* i.e. negative duty */
> + data->fan_cmd1 &= ~G762_REG_FAN_CMD1_PWM_POLARITY;
> + break;
> + case 1: /* i.e. positive duty */
> + data->fan_cmd1 |= G762_REG_FAN_CMD1_PWM_POLARITY;
> + break;
> + default:
> + ret = -EINVAL;
> + goto out;
> + }
> + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1);
> + out:
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +/*
> + * Set pwm value. Accepted values are between 0 and 255. Note that the
> + * internal register used for setting the value depends ont the fan
> + * mode of the device.
> + */
> +static int do_set_pwm(struct device *dev, unsigned long val)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = -EINVAL;
> +
> + mutex_lock(&data->update_lock);
> + if (val > 255)
> + goto out;
> +
> + if (data->fan_cmd1 & G762_REG_FAN_CMD1_FAN_MODE) { /* closed-loop */
> + data->set_cnt = PWM_TO_CNT(val);
> + g762_write_value(client, G762_REG_SET_CNT, (u8)val);
> + } else { /* open-loop */
> + data->set_out = val;
> + g762_write_value(client, G762_REG_SET_OUT, (u8)val);
> + }
> +
> + ret = 0;
> + out:
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +/* Set fan RPM value. This only makes sense in closed-loop mode. */
> +static int do_set_fan_target(struct device *dev, unsigned long val)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = -EINVAL;
> +
> + mutex_lock(&data->update_lock);
> + if (data->fan_cmd1 & G762_REG_FAN_CMD1_FAN_MODE) { /* closed-loop */
> + data->set_cnt = cnt_from_rpm(val, data->clk,
> + PULSE_FROM_REG(data->fan_cmd1),
> + CLKDIV_FROM_REG(data->fan_cmd1),
> + GEARMODE_FROM_REG(data->fan_cmd2));
> + g762_write_value(client, G762_REG_SET_CNT, data->set_cnt);
> + ret = 0;
> + }
That implies that you have to set the mode first, which is undesirable context.
One may want to set the target speed first, then set the mode.
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +/* Enable/disable fan failure detection. Accepted values are 1 and 0. */
> +static int do_fan_failure_detection_toggle(struct device *dev,
> + unsigned long enable)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = 0;
> +
> + mutex_lock(&data->update_lock);
> + switch (enable) {
> + case 0:
> + data->fan_cmd1 &= ~G762_REG_FAN_CMD1_DET_FAN_FAIL;
> + break;
> + case 1:
> + data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_FAIL;
> + break;
> + default:
> + ret = -EINVAL;
> + goto out;
> + }
> + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1);
> + out:
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +/* Enable/disable fan out of control detection. Accepted values are 1 and 0 */
> +static int do_fan_ooc_detection_toggle(struct device *dev, unsigned int enable)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = 0;
> +
> + mutex_lock(&data->update_lock);
> + switch (enable) {
> + case 0:
> + data->fan_cmd1 &= ~G762_REG_FAN_CMD1_DET_FAN_OOC;
> + break;
> + case 1:
> + data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_OOC;
> + break;
> + default:
> + ret = -EINVAL;
> + goto out;
> + }
> + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1);
> + out:
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +/* Set startup voltage. Accepted values are either 0, 1, 2 or 3. */
> +static int do_set_fan_startv(struct device *dev, unsigned long val)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct g762_data *data = g762_update_client(dev);
> + int ret = 0;
> +
> + mutex_lock(&data->update_lock);
> + switch (val) {
> + case 0:
> + data->fan_cmd2 &= ~G762_REG_FAN_CMD2_FAN_STARTV_0;
> + data->fan_cmd2 &= ~G762_REG_FAN_CMD2_FAN_STARTV_1;
> + break;
> + case 1:
> + data->fan_cmd2 |= G762_REG_FAN_CMD2_FAN_STARTV_0;
> + data->fan_cmd2 &= ~G762_REG_FAN_CMD2_FAN_STARTV_1;
> + break;
> + case 2:
> + data->fan_cmd2 &= ~G762_REG_FAN_CMD2_FAN_STARTV_0;
> + data->fan_cmd2 |= G762_REG_FAN_CMD2_FAN_STARTV_1;
> + break;
> + case 3:
> + data->fan_cmd2 |= G762_REG_FAN_CMD2_FAN_STARTV_0;
> + data->fan_cmd2 |= G762_REG_FAN_CMD2_FAN_STARTV_1;
> + break;
> + default:
> + ret = -EINVAL;
> + goto out;
> + }
> + g762_write_value(client, G762_REG_FAN_CMD2, data->fan_cmd2);
> + out:
> + mutex_unlock(&data->update_lock);
> +
> + return ret;
> +}
> +
> +
> +
> +/*
> + * Configuration-related definitions
> + */
> +
> +struct g762_config {
> + u32 fan_startv;
> + u32 fan_gear_mode;
> + u32 fan_div;
> + u32 fan_pulses;
> + u32 fan_target;
> + u32 pwm_polarity;
> + u32 pwm_enable;
> + u32 pwm_freq;
> + u32 pwm_mode;
> +};
> +
> +/*
> + * g762 default values: it is assumed that the fan is set for a 32KHz clock
> + * source, a fan clock divisor of 1 and two pulses per revolution. Default
> + * fan driving mode is linear mode (g762/g763 also support PWM mode). Note
> + * the specific init value for properties which may be left unmodified.
> + */
> +#define G762_DEFAULT_CLK 32768
> +#define G762_DEFAULT_FAN_DIV 1
> +#define G762_DEFAULT_FAN_PULSES 2
> +#define G762_DEFAULT_OUT_MODE 0
> +#define G762_DEFAULT_FAN_MODE 2
> +#define G762_DEFAULT_FAN_STARTV 1
> +#define G762_DEFAULT_FAN_GEAR_MODE 0
> +#define G762_DEFAULT_FAN_POLARITY 0
> +#define G762_UNTOUCHED_VAL 0xffffffff
> +
> +static void g762_config_init(struct g762_config *conf)
> +{
> + conf->pwm_enable = G762_DEFAULT_FAN_MODE;
> + conf->pwm_mode = G762_DEFAULT_OUT_MODE;
> + conf->pwm_freq = G762_DEFAULT_CLK;
> + conf->pwm_polarity = G762_DEFAULT_FAN_POLARITY;
> + conf->fan_pulses = G762_DEFAULT_FAN_PULSES;
> + conf->fan_div = G762_DEFAULT_FAN_DIV;
> + conf->fan_startv = G762_DEFAULT_FAN_STARTV;
> + conf->fan_gear_mode = G762_DEFAULT_FAN_GEAR_MODE;
> + conf->fan_target = G762_UNTOUCHED_VAL;
> +}
> +
> +static inline int g762_one_prop_commit(struct i2c_client *client,
> + u32 pval, const char *pname,
> + int (*psetter)(struct device *dev,
> + unsigned long val))
> +{
> + int ret = 0;
> +
> + if ((pval != G762_UNTOUCHED_VAL) && (*psetter)(&client->dev, pval)) {
> + dev_info(&client->dev, "unable to set %s (%d)\n", pname, pval);
info for an error ?
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static int g762_config_commit(struct i2c_client *client,
> + struct g762_config *conf)
> +{
> + int ret = 0;
> +
> + if (g762_one_prop_commit(client, conf->pwm_mode,
> + "pwm_mode", &do_set_pwm_mode) ||
> + g762_one_prop_commit(client, conf->pwm_enable,
> + "pwm_enable", &do_set_pwm_enable) ||
> + g762_one_prop_commit(client, conf->fan_div,
> + "fan_div", &do_set_fan_div) ||
> + g762_one_prop_commit(client, conf->fan_pulses,
> + "fan_pulses", &do_set_fan_pulses) ||
> + g762_one_prop_commit(client, conf->pwm_freq,
> + "pwm_freq", &do_set_pwm_freq) ||
> + g762_one_prop_commit(client, conf->fan_gear_mode,
> + "fan_gear_mode", &do_set_fan_gear_mode) ||
> + g762_one_prop_commit(client, conf->pwm_polarity,
> + "pwm_polarity", &do_set_pwm_polarity) ||
> + g762_one_prop_commit(client, conf->fan_startv,
> + "fan_startv", &do_set_fan_startv) ||
> + g762_one_prop_commit(client, conf->fan_target,
> + "fan_target", &do_set_fan_target)) {
& in front of function pointers is not needed.
You'll need to set ->valid to false if you call g762_update_client(),
as the changed configuration will affect readings.
> + ret = -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +
> +/*
> + * Helpers to import hardware characteristics from .dts file and overload
> + * default config values.
> + */
> +
> +#ifdef CONFIG_OF
> +static struct of_device_id g762_dt_match[] = {
> + { .compatible = "gmt,g762" },
> + { .compatible = "gmt,g763" },
> + { },
> +};
> +
> +static inline void g762_of_import_one_prop(struct i2c_client *client,
> + u32 *dest, const char *pname)
> +{
> + const __be32 *prop;
> + int len;
> +
> + prop = of_get_property(client->dev.of_node, pname, &len);
> + if (prop && len == sizeof(u32)) {
> + *dest = be32_to_cpu(prop[0]);
> + dev_info(&client->dev, "found %s (%d)\n", pname, *dest);
Please, no. We don't want to clog the log that much. Make it dev_dbg if you
think you really need it, or drop the message entirely.
> + }
> +}
> +
> +static void g762_config_of_overload(struct i2c_client *client,
> + struct g762_config *conf)
> +{
> + if (!client->dev.of_node)
> + return;
> +
> + g762_of_import_one_prop(client, &conf->fan_gear_mode, "fan_gear_mode");
> + g762_of_import_one_prop(client, &conf->pwm_polarity, "pwm_polarity");
> + g762_of_import_one_prop(client, &conf->fan_startv, "fan_startv");
> + g762_of_import_one_prop(client, &conf->pwm_freq, "pwm_freq");
> + g762_of_import_one_prop(client, &conf->fan_div, "fan_div");
> + g762_of_import_one_prop(client, &conf->fan_pulses, "fan_pulses");
> + g762_of_import_one_prop(client, &conf->pwm_mode, "pwm_mode");
> + g762_of_import_one_prop(client, &conf->fan_target, "fan_target");
> + g762_of_import_one_prop(client, &conf->pwm_enable, "pwm_enable");
> +}
> +#else
> +static void g762_config_of_overload(struct i2c_client *client,
> + struct g762_config *conf) { };
; after } not needed
> +#endif
> +
> +
> +
> +/*
> + * sysfs attributes
> + */
> +
> +
> +/*
> + * Read function for fan1_input sysfs file. Return current fan RPM value, or
> + * 0 if fan is out of control
> + */
> +static ssize_t get_fan_rpm(struct device *dev, struct device_attribute *da,
> + char *buf)
> +{
> + struct g762_data *data = g762_update_client(dev);
> + unsigned int rpm = 0;
> +
> + mutex_lock(&data->update_lock);
> + /* reverse logic: fan out of control reporting is enabled low */
> + if (data->fan_sta & G762_REG_FAN_STA_OOC) {
> + rpm = rpm_from_cnt(data->act_cnt, data->clk,
> + PULSE_FROM_REG(data->fan_cmd1),
> + CLKDIV_FROM_REG(data->fan_cmd1),
> + GEARMODE_FROM_REG(data->fan_cmd2));
> + }
> + mutex_unlock(&data->update_lock);
> +
> + return sprintf(buf, "%u\n", rpm);
> +}
> +
> +
> +/*
> + * Read and write functions for pwm1_mode sysfs file. Get and set fan speed
> + * control mode i.e. pwm (1) or linear (0).
> + */
I assume you mean "DC mode" ?
> +static ssize_t get_pwm_mode(struct device *dev, struct device_attribute *da,
> + char *buf)
> +{
> + struct g762_data *data = g762_update_client(dev);
> + int pwm_mode;
> +
> + pwm_mode = (data->fan_cmd1 & G762_REG_FAN_CMD1_OUT_MODE) ? 1 : 0;
> +
> + return sprintf(buf, "%d\n", pwm_mode);
return sprintf(buf, "%d\n",
!!(data->fan_cmd1 & G762_REG_FAN_CMD1_OUT_MODE));
> +}
> +
> +static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + unsigned long val;
> +
> + if (kstrtoul(buf, 10, &val) || do_set_pwm_mode(dev, val))
> + return -EINVAL;
> +
> + return count;
> +}
> +
> +
> +/*
> + * Read and write functions for fan1_div sysfs file. Get and set fan
> + * controller prescaler value
> + */
> +static ssize_t get_fan_div(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct g762_data *data = g762_update_client(dev);
> +
> + return sprintf(buf, "%d\n", CLKDIV_FROM_REG(data->fan_cmd1));
> +}
> +
> +static ssize_t set_fan_div(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + unsigned long val;
> +
> + if (kstrtoul(buf, 10, &val) || do_set_fan_div(dev, val))
> + return -EINVAL;
> +
> + return count;
> +}
> +
> +
> +/*
> + * Following documentation about hwmon's sysfs interface, a pwm1_enable node
> + * should accept followings:
> + *
> + * 0 : no fan speed control (i.e. fan at full speed)
> + * 1 : manual fan speed control enabled (use pwm[1-*]) (open-loop)
> + * 2+: automatic fan speed control enabled (use fan[1-*]_enable)(close-loop)
> + *
> + * but we do not accept 0 as "no-control" mode is not supported by g762,
> + * -EINVAL is returned in this case.
Sure is - you can set the fan to full speed and ignore subsequent
requests to change the speed.
> + */
> +static ssize_t get_pwm_enable(struct device *dev,
> + struct device_attribute *da, char *buf)
> +{
> + struct g762_data *data = g762_update_client(dev);
> + int fan_mode;
> +
> + fan_mode = (data->fan_cmd1 & G762_REG_FAN_CMD1_FAN_MODE) ? 2 : 1;
> +
> + return sprintf(buf, "%d\n", fan_mode);
> +}
> +
> +static ssize_t set_pwm_enable(struct device *dev,
> + struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + unsigned long val;
> +
> + if (kstrtoul(buf, 10, &val) || do_set_pwm_enable(dev, val))
> + return -EINVAL;
> +
> + return count;
> +}
> +
> +
> +/*
> + * Get/set the fan speed in open-loop mode using pwm1 sysfs file. Speed is
> + * given as a relative value from 0 to 255, where 255 is maximum speed. Note
> + * that this is done by writing directly to the chip's DAC, it won't change
> + * the closed loop speed set by fan1_target. Also note that due to rounding
> + * errors it is possible that you don't read back exactly the value you have
> + * set.
> + */
> +static ssize_t get_pwm(struct device *dev, struct device_attribute *da,
> + char *buf)
> +{
> + struct g762_data *data = g762_update_client(dev);
> + int val;
> +
> + mutex_lock(&data->update_lock);
> + if (data->fan_cmd1 & G762_REG_FAN_CMD1_FAN_MODE) { /* closed-loop */
> + val = PWM_FROM_CNT(data->set_cnt);
> + } else { /* open-loop */
> + val = data->set_out;
> + }
Wonder why checkpatch doesn't complain about the { }.
> + mutex_unlock(&data->update_lock);
> +
> + return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t set_pwm(struct device *dev, struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + unsigned long val;
> +
> + if (kstrtoul(buf, 10, &val) || do_set_pwm(dev, val))
> + return -EINVAL;
> +
> + return count;
> +}
> +
> +
> +/*
> + * Get/set the fan speed in closed-loop mode using fan1_target sysfs file. Speed
> + * is given as a rpm value, then G762 will automatically regulate the fan speed
> + * using pulses from fan tachometer.
> + *
> + * Refer to rpm_from_cnt implementation to get info about count number
> + * calculation.
> + *
> + * Also note that due to rounding errors it is possible that you don't read
> + * back exactly the value you have set.
> + */
> +static ssize_t get_fan_target(struct device *dev, struct device_attribute *da,
> + char *buf)
> +{
> + struct g762_data *data = g762_update_client(dev);
> + unsigned int rpm;
> + ssize_t err;
> +
> + mutex_lock(&data->update_lock);
> + if (data->fan_cmd1 & G762_REG_FAN_CMD1_FAN_MODE) { /* closed-loop */
> + rpm = rpm_from_cnt(data->set_cnt, data->clk,
> + PULSE_FROM_REG(data->fan_cmd1),
> + CLKDIV_FROM_REG(data->fan_cmd1),
> + GEARMODE_FROM_REG(data->fan_cmd2));
> + err = sprintf(buf, "%u\n", rpm);
> + } else { /* open-loop */
> + err = -EINVAL;
Can't you just return the current value, even if not used ?
If the returned value can differ, you might want to cleat ->valid after setting
it.
> + }
> + mutex_unlock(&data->update_lock);
> +
> + return err;
> +}
> +
> +static ssize_t set_fan_target(struct device *dev, struct device_attribute *da,
> + const char *buf, size_t count)
> +{
> + unsigned long val;
> +
> + if (kstrtoul(buf, 10, &val) || do_set_fan_target(dev, val))
> + return -EINVAL;
> +
> + return count;
> +}
> +
> +
> +/* read function for fan1_fault sysfs file. */
> +static ssize_t get_fan_failure(struct device *dev, struct device_attribute *da,
> + char *buf)
> +{
> + struct g762_data *data = g762_update_client(dev);
> + unsigned int val;
> +
> + val = (data->fan_sta & G762_REG_FAN_STA_FAIL) ? 1 : 0;
> +
You can use the !! trick here again.
> + return sprintf(buf, "%u\n", val);
> +}
> +
> +
> +/* read function for fan1_alarm sysfs file. */
> +static ssize_t get_fan_ooc(struct device *dev, struct device_attribute *da,
> + char *buf)
> +{
> + struct g762_data *data = g762_update_client(dev);
> + unsigned int val;
> +
> + /* ooc condition is enabled low */
> + val = (data->fan_sta & G762_REG_FAN_STA_OOC) ? 0 : 1;
> +
Same here.
> + return sprintf(buf, "%u\n", val);
> +}
> +
> +
> +static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO,
> + get_pwm, set_pwm);
> +static DEVICE_ATTR(pwm1_mode, S_IWUSR | S_IRUGO,
> + get_pwm_mode, set_pwm_mode);
> +static DEVICE_ATTR(pwm1_enable, S_IWUSR | S_IRUGO,
> + get_pwm_enable, set_pwm_enable);
> +
> +static DEVICE_ATTR(fan1_input, S_IRUGO,
> + get_fan_rpm, NULL);
> +static DEVICE_ATTR(fan1_alarm, S_IRUGO,
> + get_fan_ooc, NULL);
> +static DEVICE_ATTR(fan1_fault, S_IRUGO,
> + get_fan_failure, NULL);
> +static DEVICE_ATTR(fan1_target, S_IWUSR | S_IRUGO,
> + get_fan_target, set_fan_target);
> +static DEVICE_ATTR(fan1_div, S_IWUSR | S_IRUGO,
> + get_fan_div, set_fan_div);
> +
Most if not all of the above don't need two lines.
> +/* Driver data */
> +static struct attribute *g762_attributes[] = {
> + &dev_attr_fan1_input.attr,
> + &dev_attr_fan1_alarm.attr,
> + &dev_attr_fan1_fault.attr,
> + &dev_attr_fan1_target.attr,
> + &dev_attr_fan1_div.attr,
> + &dev_attr_pwm1.attr,
> + &dev_attr_pwm1_mode.attr,
> + &dev_attr_pwm1_enable.attr,
> + NULL
> +};
> +
> +static const struct attribute_group g762_group = {
> + .attrs = g762_attributes,
> +};
> +
> +static int g762_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> + struct g762_data *data;
> + struct g762_config conf;
> + int err = 0;
Unnecessary initialization.
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_BYTE_DATA)) {
> + err = -ENODEV;
Just return -ENODEV.
> + goto out;
> + }
> +
> + data = devm_kzalloc(&client->dev, sizeof(struct g762_data), GFP_KERNEL);
> + if (!data) {
> + err = -ENOMEM;
> + goto out;
Just return -ENOMEM.
> + }
> + i2c_set_clientdata(client, data);
> +
> + data->client = client;
> + mutex_init(&data->update_lock);
> +
> + /* Enable fan protection and fan fail detection by default */
> + do_fan_ooc_detection_toggle(&client->dev, 1);
> + do_fan_failure_detection_toggle(&client->dev, 1);
> +
> + /*
> + * Set default configuration values before passing the structure
> + * to OF helpers to overload them using those provided by .dts
> + * file (if any). Final config is then commited.
> + */
> + g762_config_init(&conf);
> + g762_config_of_overload(client, &conf);
> + err = g762_config_commit(client, &conf);
> + if (err)
> + goto out;
Just return err.
> +
> + /* Register sysfs hooks */
> + err = sysfs_create_group(&client->dev.kobj, &g762_group);
> + if (err)
> + goto out;
just return err.
> +
> + data->hwmon_dev = (struct device *)hwmon_device_register(&client->dev);
> + if (IS_ERR(data->hwmon_dev)) {
> + err = PTR_ERR(data->hwmon_dev);
> + sysfs_remove_group(&client->dev.kobj, &g762_group);
sysfs_remove_group() should be in the error exit path, not here.
> + goto out;
> + }
> +
> + dev_info(&data->client->dev, "device successfully initialized\n");
> +
Is that useful ?
> + out:
> + return err;
> +}
> +
> +static int g762_remove(struct i2c_client *client)
> +{
> + struct g762_data *data = i2c_get_clientdata(client);
> +
> + hwmon_device_unregister(data->hwmon_dev);
> + sysfs_remove_group(&client->dev.kobj, &g762_group);
> +
> + dev_info(&data->client->dev, "device successfully removed\n");
Is that useful ?
> + return 0;
> +}
> +
> +static struct i2c_driver g762_driver = {
> + .driver = {
> + .name = DRVNAME,
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(g762_dt_match),
> + },
> + .probe = g762_probe,
> + .remove = g762_remove,
> + .id_table = g762_id,
> +};
> +
> +module_i2c_driver(g762_driver);
> +
> +MODULE_AUTHOR("Olivier Mouchet <olivier.mouchet at gmail.com>");
> +MODULE_DESCRIPTION("GMT G762 driver");
> +MODULE_LICENSE("GPL");
> --
> 1.7.10.4
>
>
More information about the devicetree-discuss
mailing list