* [RFC,PATCHv0 0/3] Add support for GMT G762/G763 PWM fan controller
@ 2013-04-18 22:27 Arnaud Ebalard
2013-04-18 22:28 ` [PATCH 1/3] Add support for GMT G72/G763 " Arnaud Ebalard
` (2 more replies)
0 siblings, 3 replies; 34+ messages in thread
From: Arnaud Ebalard @ 2013-04-18 22:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
This series adds support for GMT G762/G763. This work is based on a
basic version for 2.6.31 kernel developed Olivier Mouchet (kept as
author for this reason in g762.c) for LaCie NAS. Updates have been
performed to run on recent kernels. Supported has been completed and
additional features added: ability to configure various characteristics
from .dts file, better initialization, alarms and error reporting
support, gear mode, polarity, fan pulse per revolution, fan startup
voltage control. The following detailed datasheet has been used as a
basis for this work:
http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf
The purpose for this v0 is to get some feedback on the mistakes I may
have made or missing things to get the patch accepted.
The patch was developed for and tested against the GMT G762 fan
controller used in a Netgear ReadyNAS Duo v2 (kirkwood 88F6282-based
NAS). This is the main reason for the device tree bindings provided in
first patch. The patches are against current ARM tree; tell me if you
need me to rebase it against something else.
Patch 2/3 provides documentation for DT bindings. Documentation for the
sysfs interface provided by the driver is available in patch 3/3.
As a side note, I found it difficult to extend g760a driver to add g762
features; hence the new g762.c file.
Comments welcome,
Cheers,
a+
Arnaud Ebalard (3):
Add support for GMT G72/G763 PWM fan controller
Add DT documentation for G762 PWM fan controller
Add documentation for g762 driver
Documentation/devicetree/bindings/hwmon/g762.txt | 37 +
Documentation/hwmon/g762 | 89 ++
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/g762.c | 1159 ++++++++++++++++++++++
5 files changed, 1296 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/g762.txt
create mode 100644 Documentation/hwmon/g762
create mode 100644 drivers/hwmon/g762.c
--
1.7.10.4
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller 2013-04-18 22:27 [RFC,PATCHv0 0/3] Add support for GMT G762/G763 PWM fan controller Arnaud Ebalard @ 2013-04-18 22:28 ` Arnaud Ebalard 2013-04-19 4:35 ` Guenter Roeck ` (2 more replies) 2013-04-18 22:28 ` [RFC,PATCHv0 2/3] Add DT documentation for G762 " Arnaud Ebalard 2013-04-18 22:28 ` [PATCH 3/3] Add documentation for g762 driver Arnaud Ebalard 2 siblings, 3 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-18 22:28 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Arnaud Ebalard <arno@natisbad.org> Tested-by: Arnaud Ebalard <arno@natisbad.org> --- drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/g762.c | 1159 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1170 insertions(+) create mode 100644 drivers/hwmon/g762.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 89ac1cb..0c6ddee 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" + depends on I2C + help + If you say yes here you get support for Global Mixed-mode + Technology Inc G762 fan speed PWM controller chips. + + This driver can also be built as a module. If so, the module + will be called g762a. + 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..8c4cb39 --- /dev/null +++ b/drivers/hwmon/g762.c @@ -0,0 +1,1159 @@ +/* + * 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@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. Supported has been completed and additional + * features added: ability to configure various characteristics from + * .dts file, better initialization, alarms and error reporting support, + * gear mode, polarity, fan pulse per revolution, fan startup voltage + * control. The following detailed datasheet has been used as a basis + * for this work: + * + * 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@gmail.com> + * + * based on g760a code written by Herbert Valerio Riedel <hvr@gnu.org> + * Copyright (C) 2007 Herbert Valerio Riedel <hvr@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 + */ + +#include <linux/device.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/slab.h> +#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[] = { + { DRVNAME, 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 1 +#define FAN_MODE_OPEN_LOOP 0 + +/* 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 also supports PWM mode) */ +#define G762_DEFAULT_CLK 32768 +#define G762_DEFAULT_FAN_CLK_DIV 1 +#define G762_DEFAULT_PULSE_PER_REV 2 +#define G762_DEFAULT_OUT_MODE 0 +#define G762_DEFAULT_FAN_MODE 1 + +/* register data is read (and cached) at most once per second */ +#define G762_UPDATE_INTERVAL (HZ) + +/* 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 */ + unsigned int valid:1; + 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)) + +/* 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); +} + +#ifdef CONFIG_OF +static struct of_device_id g762_dt_match[] = { + { .compatible = "gmt,g762" }, + { .compatible = "gmt,g763" }, + { }, +}; +#endif + +static int g762_probe(struct i2c_client *client, + const struct i2c_device_id *id); +static int g762_remove(struct i2c_client *client); + +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, +}; + +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, u16 value) +{ + return i2c_smbus_write_byte_data(client, reg, value); +} + +/* + * sysfs attributes + */ + +/* 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; + } + mutex_unlock(&data->update_lock); + + return data; +} + +/* 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 function for pwm1_mode sysfs file. Get fan speed control + * mode i.e. pwm (1) or linear (0) */ +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); +} + +static ssize_t do_set_pwm_mode(struct device *dev, u8 val) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = g762_update_client(dev); + + 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 */ + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_OUT_MODE; + break; + default: + mutex_unlock(&data->update_lock); + return -EINVAL; + } + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1); + mutex_unlock(&data->update_lock); + + return val; +} + +/* Write function for pwm1_mode sysfs file. Set fan speed control + * mode as pwm (1) or linear (0) */ +static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *da, + const char *buf, size_t count) +{ + unsigned long val; + ssize_t ret; + + if (kstrtoul(buf, 10, &val)) + return -EINVAL; + + ret = do_set_pwm_mode(dev, (u8)val); + if (ret < 0) + return -EINVAL; + + return count; +} + +/* Read function for pwm1_freq sysfs file. */ +static ssize_t get_pwm_freq(struct device *dev, + struct device_attribute *da, char *buf) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = i2c_get_clientdata(client); + + return sprintf(buf, "%d\n", data->clk); +} + +/* Write function for pwm1_freq sysfs file. */ +static ssize_t set_pwm_freq(struct device *dev, + struct device_attribute *da, + const char *buf, size_t count) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = i2c_get_clientdata(client); + unsigned long val; + + if (kstrtoul(buf, 10, &val) || !val) + return -EINVAL; + + mutex_lock(&data->update_lock); + data->clk = val; + mutex_unlock(&data->update_lock); + + return count; +} + +/* Read function for fan1_div sysfs file. Get fan controller prescaler */ +static ssize_t get_fan_clk_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 do_set_fan_clk_div(struct device *dev, u8 val) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = g762_update_client(dev); + + mutex_lock(&data->update_lock); + switch (val) { + case 1: + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_CLK_DIV_ID0; + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_CLK_DIV_ID1; + break; + case 2: + data->fan_cmd1 |= G762_REG_FAN_CMD1_CLK_DIV_ID0; + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_CLK_DIV_ID1; + break; + case 4: + data->fan_cmd1 &= (u8) ~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: + mutex_unlock(&data->update_lock); + return -EINVAL; + } + g762_write_value(client, G762_REG_FAN_CMD1, (u16)data->fan_cmd1); + mutex_unlock(&data->update_lock); + + return val; +} + +/* Write function for fan1_div file. Set fan controller prescaler */ +static ssize_t set_fan_clk_div(struct device *dev, + struct device_attribute *da, + const char *buf, size_t count) +{ + unsigned long val; + ssize_t ret; + + if (kstrtoul(buf, 10, &val)) + return -EINVAL; + + ret = do_set_fan_clk_div(dev, (u8) val); + if (ret < 0) + return -EINVAL; + + return count; +} + +/* Read function for fan1_gear_mode sysfs file. Get fan gear mode */ +static ssize_t get_fan_gear_mode(struct device *dev, + struct device_attribute *da, char *buf) +{ + struct g762_data *data = g762_update_client(dev); + u8 fan_gear_mode; + + fan_gear_mode = data->fan_cmd2 & (G762_REG_FAN_CMD2_GEAR_MODE_0 | + G762_REG_FAN_CMD2_GEAR_MODE_1); + fan_gear_mode = fan_gear_mode >> 2; + + return sprintf(buf, "%d\n", fan_gear_mode); +} + +static ssize_t do_set_fan_gear_mode(struct device *dev, u8 val) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = g762_update_client(dev); + + mutex_lock(&data->update_lock); + switch (val) { + case 0: + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_GEAR_MODE_0; + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_GEAR_MODE_1; + break; + case 1: + data->fan_cmd2 |= G762_REG_FAN_CMD2_GEAR_MODE_0; + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_GEAR_MODE_1; + break; + case 2: + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_GEAR_MODE_0; + data->fan_cmd2 |= G762_REG_FAN_CMD2_GEAR_MODE_1; + break; + default: + mutex_unlock(&data->update_lock); + return -EINVAL; + } + g762_write_value(client, G762_REG_FAN_CMD2, (u16)data->fan_cmd2); + mutex_unlock(&data->update_lock); + + return val; +} + +/* Write function for fan1_gear_mode sysfs file. Write fan gear mode */ +static ssize_t set_fan_gear_mode(struct device *dev, + struct device_attribute *da, + const char *buf, size_t count) +{ + unsigned long val; + ssize_t ret; + + if (kstrtoul(buf, 10, &val)) + return -EINVAL; + + ret = do_set_fan_gear_mode(dev, (u8) val); + if (ret < 0) + return -EINVAL; + + return count; +} + +/* Read function for fan1_pulses. Returns either 2 or 4 */ +static ssize_t get_fan_pulses(struct device *dev, struct device_attribute *da, + char *buf) +{ + struct g762_data *data = g762_update_client(dev); + + return sprintf(buf, "%d\n", PULSE_FROM_REG(data->fan_cmd1)); +} + +/* Set pulse per revolution value. Accepts either 2 or 4. */ +static ssize_t do_set_fan_pulses(struct device *dev, u8 val) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = g762_update_client(dev); + + mutex_lock(&data->update_lock); + switch (val) { + case 2: + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_PULSE_PER_REV; + break; + case 4: + data->fan_cmd1 |= G762_REG_FAN_CMD1_PULSE_PER_REV; + break; + default: + mutex_unlock(&data->update_lock); + return -EINVAL; + } + g762_write_value(client, G762_REG_FAN_CMD1, (u16)data->fan_cmd1); + mutex_unlock(&data->update_lock); + + return val; +} + +/* Write function for fan1_pulses. Accepts either 2 or 4 */ +static ssize_t set_fan_pulses(struct device *dev, struct device_attribute *da, + const char *buf, size_t count) +{ + unsigned long val; + ssize_t ret; + + if (kstrtoul(buf, 10, &val) || (val != 2 && val != 4)) + return -EINVAL; + + ret = do_set_fan_pulses(dev, (u8) val); + if (ret < 0) + 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@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. */ +static ssize_t get_speed_control_mode(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 do_set_speed_control_mode(struct device *dev, u8 val) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = g762_update_client(dev); + + if (!val) + return -EINVAL; + val -= 1; + + mutex_lock(&data->update_lock); + switch (val) { + case FAN_MODE_CLOSED_LOOP: + data->fan_cmd1 |= G762_REG_FAN_CMD1_FAN_MODE; + break; + case FAN_MODE_OPEN_LOOP: + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_FAN_MODE; + break; + default: + mutex_unlock(&data->update_lock); + return -EINVAL; + } + + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1); + mutex_unlock(&data->update_lock); + + return val; +} + +static ssize_t set_speed_control_mode(struct device *dev, + struct device_attribute *da, + const char *buf, size_t count) +{ + unsigned long val; + ssize_t ret; + + if (kstrtoul(buf, 10, &val)) + return -EINVAL; + + ret = do_set_speed_control_mode(dev, val); + if (ret < 0) + return -EINVAL; + + return count; +} + +/* Read function for pwm1_polarity (0: negative duty, 1: positive duty) */ +static ssize_t get_pwm_polarity(struct device *dev, struct device_attribute *da, + char *buf) +{ + struct g762_data *data = g762_update_client(dev); + u8 pwm_polarity; + + pwm_polarity = (data->fan_cmd1 & G762_REG_FAN_CMD1_PWM_POLARITY) >> 1; + + return sprintf(buf, "%d\n", pwm_polarity); +} + +/* Write function for pwm1_polarity (0: negative duty, 1: positive duty) */ +static ssize_t set_pwm_polarity(struct device *dev, struct device_attribute *da, + const char *buf, size_t count) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = g762_update_client(dev); + unsigned long val; + + if (kstrtoul(buf, 10, &val)) + return -EINVAL; + + mutex_lock(&data->update_lock); + switch (val) { + case 0: /* i.e. negative duty */ + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_PWM_POLARITY; + break; + case 1: /* i.e. positive duty */ + data->fan_cmd1 |= G762_REG_FAN_CMD1_PWM_POLARITY; + break; + default: + mutex_unlock(&data->update_lock); + return -EINVAL; + } + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1); + mutex_unlock(&data->update_lock); + + 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; + } + mutex_unlock(&data->update_lock); + + return sprintf(buf, "%d\n", val); +} + +static ssize_t do_set_pwm(struct device *dev, u8 val) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = g762_update_client(dev); + + mutex_lock(&data->update_lock); + if (data->fan_cmd1 & G762_REG_FAN_CMD1_FAN_MODE) { /* closed-loop */ + val = PWM_TO_CNT(clamp_val(val, 0, 255)); + data->set_cnt = val; + g762_write_value(client, G762_REG_SET_CNT, val); + } else { /* open-loop */ + data->set_out = val; + g762_write_value(client, G762_REG_SET_OUT, val); + } + mutex_unlock(&data->update_lock); + + return val; +} + +static ssize_t set_pwm(struct device *dev, struct device_attribute *da, + const char *buf, size_t count) +{ + unsigned long val; + ssize_t ret; + + if (kstrtoul(buf, 10, &val)) + return -EINVAL; + + ret = do_set_pwm(dev, (u8)val); + if (ret < 0) + 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; + } + mutex_unlock(&data->update_lock); + + return err; +} + +static ssize_t do_set_fan_target(struct device *dev, unsigned int val) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = g762_update_client(dev); + ssize_t err; + + 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); + err = val; + } else { + err = -EINVAL; + } + 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 int val; + ssize_t ret; + + if (sscanf(buf, "%u", &val) != 1) + return -EINVAL; + + ret = do_set_fan_target(dev, val); + if (ret < 0) + return -EINVAL; + + return count; +} + +/* read function for 'fan1_fault_detection' sysfs file */ +static ssize_t get_fan_failure_detection(struct device *dev, + struct device_attribute *da, + char *buf) +{ + struct g762_data *data = g762_update_client(dev); + unsigned int enabled; + + enabled = (data->fan_cmd1 & G762_REG_FAN_CMD1_DET_FAN_FAIL) ? 1 : 0; + + return sprintf(buf, "%u\n", enabled); +} + +/* enable (or disable) fan failure detection */ +static ssize_t do_fan_failure_detection_toggle(struct device *dev, + unsigned int enable) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = g762_update_client(dev); + + mutex_lock(&data->update_lock); + switch (enable) { + case 0: + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_DET_FAN_FAIL; + break; + case 1: + data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_FAIL; + break; + default: + mutex_unlock(&data->update_lock); + return -EINVAL; + } + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1); + mutex_unlock(&data->update_lock); + + return enable; +} + +/* write function for 'fan1_fault_detection' sysfs file */ +static ssize_t set_fan_failure_detection(struct device *dev, + struct device_attribute *da, + const char *buf, size_t count) +{ + unsigned int val; + ssize_t ret; + + if (sscanf(buf, "%u", &val) != 1) + return -EINVAL; + + ret = do_fan_failure_detection_toggle(dev, val); + if (ret < 0) + 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; + + return sprintf(buf, "%u\n", val); +} + +/* read function for 'fan1_alarm_detection' sysfs file */ +static ssize_t get_fan_ooc_detection(struct device *dev, + struct device_attribute *da, + char *buf) +{ + struct g762_data *data = g762_update_client(dev); + unsigned int enabled; + + enabled = (data->fan_cmd1 & G762_REG_FAN_CMD1_DET_FAN_FAIL) ? 1 : 0; + + return sprintf(buf, "%u\n", enabled); +} + +/* enable (or disable) fan out of control detection */ +static ssize_t 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); + + mutex_lock(&data->update_lock); + switch (enable) { + case 0: + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_DET_FAN_OOC; + break; + case 1: + data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_OOC; + break; + default: + mutex_unlock(&data->update_lock); + return -EINVAL; + } + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1); + mutex_unlock(&data->update_lock); + + return enable; +} + +/* write function for 'fan1_alarm_detection' sysfs file */ +static ssize_t set_fan_ooc_detection(struct device *dev, + struct device_attribute *da, + const char *buf, size_t count) +{ + unsigned int val; + ssize_t ret; + + if (sscanf(buf, "%u", &val) != 1) + return -EINVAL; + + ret = do_fan_ooc_detection_toggle(dev, val); + if (ret < 0) + return -EINVAL; + + return count; +} + +/* 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; + + return sprintf(buf, "%u\n", val); +} + +/* Read function for fan1_startup_voltage */ +static ssize_t get_fan_startup_voltage(struct device *dev, + struct device_attribute *da, + char *buf) +{ + struct g762_data *data = g762_update_client(dev); + u8 val; + + val = data->fan_cmd2 & (G762_REG_FAN_CMD2_FAN_STARTV_1 | + G762_REG_FAN_CMD2_FAN_STARTV_0); + + return sprintf(buf, "%d\n", val); +} + +/* Write function for fan1_startup_voltage */ +static ssize_t set_fan_startup_voltage(struct device *dev, + struct device_attribute *da, + const char *buf, size_t count) +{ + struct i2c_client *client = to_i2c_client(dev); + struct g762_data *data = g762_update_client(dev); + unsigned long val; + + if (kstrtoul(buf, 10, &val)) + return -EINVAL; + + mutex_lock(&data->update_lock); + switch (val) { + case 0: + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_FAN_STARTV_0; + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_FAN_STARTV_1; + break; + case 1: + data->fan_cmd2 |= G762_REG_FAN_CMD2_FAN_STARTV_0; + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_FAN_STARTV_1; + break; + case 2: + data->fan_cmd2 &= (u8) ~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: + mutex_unlock(&data->update_lock); + return -EINVAL; + } + g762_write_value(client, G762_REG_FAN_CMD2, (u16)data->fan_cmd2); + mutex_unlock(&data->update_lock); + + return count; +} + +static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, get_pwm, set_pwm); +static DEVICE_ATTR(pwm1_polarity, S_IWUSR | S_IRUGO, + get_pwm_polarity, set_pwm_polarity); +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_speed_control_mode, set_speed_control_mode); +static DEVICE_ATTR(pwm1_freq, S_IWUSR | S_IRUGO, + get_pwm_freq, set_pwm_freq); + +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_alarm_detection, S_IWUSR | S_IRUGO, + get_fan_ooc_detection, set_fan_ooc_detection); +static DEVICE_ATTR(fan1_fault, S_IRUGO, get_fan_failure, NULL); +static DEVICE_ATTR(fan1_fault_detection, S_IWUSR | S_IRUGO, + get_fan_failure_detection, set_fan_failure_detection); +static DEVICE_ATTR(fan1_target, S_IWUSR | S_IRUGO, + get_fan_target, set_fan_target); +static DEVICE_ATTR(fan1_pulses, S_IWUSR | S_IRUGO, + get_fan_pulses, set_fan_pulses); +static DEVICE_ATTR(fan1_div, S_IWUSR | S_IRUGO, + get_fan_clk_div, set_fan_clk_div); +static DEVICE_ATTR(fan1_gear_mode, S_IWUSR | S_IRUGO, + get_fan_gear_mode, set_fan_gear_mode); +static DEVICE_ATTR(fan1_startup_voltage, S_IWUSR | S_IRUGO, + get_fan_startup_voltage, set_fan_startup_voltage); + +/* Driver data */ +static struct attribute *g762_attributes[] = { + &dev_attr_fan1_input.attr, + &dev_attr_fan1_alarm.attr, + &dev_attr_fan1_alarm_detection.attr, + &dev_attr_fan1_fault.attr, + &dev_attr_fan1_fault_detection.attr, + &dev_attr_fan1_target.attr, + &dev_attr_fan1_pulses.attr, + &dev_attr_fan1_div.attr, + &dev_attr_fan1_gear_mode.attr, + &dev_attr_fan1_startup_voltage.attr, + &dev_attr_pwm1.attr, + &dev_attr_pwm1_polarity.attr, + &dev_attr_pwm1_mode.attr, + &dev_attr_pwm1_enable.attr, + &dev_attr_pwm1_freq.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) +{ + u32 pwm_freq, fan_clk_div, fan_pulses, pwm_mode, fan_target, pwm_enable; + struct g762_data *data; + int err; + + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA)) + return -ENODEV; + + data = devm_kzalloc(&client->dev, sizeof(struct g762_data), GFP_KERNEL); + if (!data) + return -ENOMEM; + i2c_set_clientdata(client, data); + + data->client = client; + mutex_init(&data->update_lock); + + /* Setup default configuration for now. Those may get modified below + * in CONFIG_OF-protected block (via .dts file). Final values will + * then be pushed to the device */ + pwm_freq = G762_DEFAULT_CLK; + fan_clk_div = G762_DEFAULT_FAN_CLK_DIV; + fan_pulses = G762_DEFAULT_PULSE_PER_REV; + pwm_mode = G762_DEFAULT_OUT_MODE; + pwm_enable = G762_DEFAULT_FAN_MODE + 1; /* shift w/ sysfs interface */ + fan_target = 0xffffffff; /* dummy value */ + + /* 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); + +#ifdef CONFIG_OF + if (client->dev.of_node) { + const __be32 *prop; + int len; + + prop = of_get_property(client->dev.of_node, "pwm_freq", &len); + if (prop && len == sizeof(u32)) { + pwm_freq = be32_to_cpu(prop[0]); + dev_info(&client->dev, "found pwm_freq (%d)\n", pwm_freq); + } + + prop = of_get_property(client->dev.of_node, "fan_div", &len); + if (prop && len == sizeof(u32)) { + fan_clk_div = be32_to_cpu(prop[0]); + dev_info(&client->dev, "found fan_div (%d)\n", + fan_clk_div); + } + + prop = of_get_property(client->dev.of_node, "fan_pulses", &len); + if (prop && len == sizeof(u32)) { + fan_pulses = be32_to_cpu(prop[0]); + dev_info(&client->dev, "found fan_pulses (%d)\n", + fan_pulses); + } + + prop = of_get_property(client->dev.of_node, "pwm_mode", &len); + if (prop && len == sizeof(u32)) { + pwm_mode = be32_to_cpu(prop[0]); + dev_info(&client->dev, "found pwm_mode (%d)\n", + pwm_mode); + } + + prop = of_get_property(client->dev.of_node, "fan_target", &len); + if (prop && len == sizeof(u32)) { + fan_target = be32_to_cpu(prop[0]); + dev_info(&client->dev, "found fan_target (%d)\n", + fan_target); + } + + prop = of_get_property(client->dev.of_node, "pwm_enable", &len); + if (prop && len == sizeof(u32)) { + pwm_enable = be32_to_cpu(prop[0]); + dev_info(&client->dev, "found pwm_enable (%d)\n", + pwm_enable); + } + } +#endif + + /* Now, let's set final fan out mode (pwm or linear), loop mode (closed + * or open loop), clock freq, pulse per rev, fan clock div, fan target + * value, and speed control method (closed or open loop) */ + err = do_set_pwm_mode(&client->dev, pwm_mode); + if (err < 0) { + dev_info(&client->dev, "unable to set pwm_mode (%d)\n", + pwm_mode); + } + + err = do_set_speed_control_mode(&client->dev, pwm_enable); + if (err < 0) { + dev_info(&client->dev, "unable to set pwm_enable (%d)\n", + pwm_enable); + } + + err = do_set_fan_clk_div(&client->dev, fan_clk_div); + if (err < 0) { + dev_info(&client->dev, "unable to set fan_clk_div (%d)\n", + fan_clk_div); + } + + err = do_set_fan_pulses(&client->dev, fan_pulses); + if (err < 0) { + dev_info(&client->dev, "unable to set fan_pulses (%d)\n", + fan_pulses); + } + + data->clk = pwm_freq; + + if (fan_target != 0xffffffff) { + err = do_set_fan_target(&client->dev, fan_target); + if (err < 0) { + dev_info(&client->dev, "unable to set fan_target (%d)\n", + fan_target); + } + } + + /* Register sysfs hooks */ + err = sysfs_create_group(&client->dev.kobj, &g762_group); + if (err) + 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); + return err; + } + + dev_info(&data->client->dev, "device successfully initialized\n"); + + return 0; +} + +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"); + return 0; +} + +module_i2c_driver(g762_driver); + +MODULE_AUTHOR("Olivier Mouchet <olivier.mouchet@gmail.com>"); +MODULE_DESCRIPTION("GMT G762 driver"); +MODULE_LICENSE("GPL"); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller 2013-04-18 22:28 ` [PATCH 1/3] Add support for GMT G72/G763 " Arnaud Ebalard @ 2013-04-19 4:35 ` Guenter Roeck 2013-04-19 5:34 ` Arnaud Ebalard 2013-04-23 22:05 ` [PATCHv1 0/3] hwmon: " Arnaud Ebalard 2013-04-19 5:50 ` [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller Andrew Lunn 2013-04-19 6:05 ` Jean Delvare 2 siblings, 2 replies; 34+ messages in thread From: Guenter Roeck @ 2013-04-19 4:35 UTC (permalink / raw) To: linux-arm-kernel On Fri, Apr 19, 2013 at 12:28:21AM +0200, Arnaud Ebalard wrote: > > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > Tested-by: Arnaud Ebalard <arno@natisbad.org> > --- > drivers/hwmon/Kconfig | 10 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/g762.c | 1159 ++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 1170 insertions(+) > create mode 100644 drivers/hwmon/g762.c > checkpatch says: total: 1 errors, 15 warnings, 0 checks, 1182 lines checked Please fix those. Also, please make sure there are spaces around operators. Additional comments inline. This is not a complete review; I gave up after a while. Please fix the problems and resubmit. > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 89ac1cb..0c6ddee 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" > + depends on I2C > + help > + If you say yes here you get support for Global Mixed-mode > + Technology Inc G762 fan speed PWM controller chips. > + > + This driver can also be built as a module. If so, the module > + will be called g762a. > + > 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..8c4cb39 > --- /dev/null > +++ b/drivers/hwmon/g762.c > @@ -0,0 +1,1159 @@ > +/* > + * 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@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. Supported has been completed and additional > + * features added: ability to configure various characteristics from > + * .dts file, better initialization, alarms and error reporting support, > + * gear mode, polarity, fan pulse per revolution, fan startup voltage > + * control. The following detailed datasheet has been used as a basis > + * for this work: > + * > + * 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@gmail.com> > + * > + * based on g760a code written by Herbert Valerio Riedel <hvr@gnu.org> > + * Copyright (C) 2007 Herbert Valerio Riedel <hvr@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 > + */ > + > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/slab.h> > +#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[] = { > + { DRVNAME, 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 1 > +#define FAN_MODE_OPEN_LOOP 0 > + > +/* 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 also supports PWM mode) */ > +#define G762_DEFAULT_CLK 32768 > +#define G762_DEFAULT_FAN_CLK_DIV 1 > +#define G762_DEFAULT_PULSE_PER_REV 2 > +#define G762_DEFAULT_OUT_MODE 0 > +#define G762_DEFAULT_FAN_MODE 1 > + > +/* register data is read (and cached) at most once per second */ > +#define G762_UPDATE_INTERVAL (HZ) > + > +/* 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) Always put ( ) around macro arguments. > + > +/* 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 */ > + unsigned int valid:1; Please use bool. > + 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 */ Extra line for */, please > + 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)) > + > +/* 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. */ /* * Please follow CodingStyle for multi-line comments */ > +static inline unsigned int rpm_from_cnt(u8 cnt, u32 clk, u16 p, tab after cnt ? > + 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); > +} > + > +#ifdef CONFIG_OF > +static struct of_device_id g762_dt_match[] = { > + { .compatible = "gmt,g762" }, > + { .compatible = "gmt,g763" }, > + { }, > +}; > +#endif > + > +static int g762_probe(struct i2c_client *client, > + const struct i2c_device_id *id); > +static int g762_remove(struct i2c_client *client); > + Please rearrange the code to not require forward declarations. > +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, > +}; > + > +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, u16 value) > +{ > + return i2c_smbus_write_byte_data(client, reg, value); > +} > + > +/* > + * sysfs attributes > + */ > + > +/* 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; > + } > + mutex_unlock(&data->update_lock); > + > + return data; > +} > + > +/* 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 function for pwm1_mode sysfs file. Get fan speed control > + * mode i.e. pwm (1) or linear (0) */ > +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); > +} > + > +static ssize_t do_set_pwm_mode(struct device *dev, u8 val) Why ssize_t ? > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = g762_update_client(dev); > + > + 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 */ > + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_OUT_MODE; Unnecessary typecast. Several times. > + break; > + default: > + mutex_unlock(&data->update_lock); > + return -EINVAL; > + } > + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1); > + mutex_unlock(&data->update_lock); > + > + return val; > +} > + > +/* Write function for pwm1_mode sysfs file. Set fan speed control > + * mode as pwm (1) or linear (0) */ > +static ssize_t set_pwm_mode(struct device *dev, struct device_attribute *da, > + const char *buf, size_t count) > +{ > + unsigned long val; > + ssize_t ret; Why ssize_t ? > + > + if (kstrtoul(buf, 10, &val)) > + return -EINVAL; > + > + ret = do_set_pwm_mode(dev, (u8)val); No overflow protection ? User writes 256, gets 0 ? > + if (ret < 0) > + return -EINVAL; > + > + return count; > +} > + > +/* Read function for pwm1_freq sysfs file. */ > +static ssize_t get_pwm_freq(struct device *dev, > + struct device_attribute *da, char *buf) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = i2c_get_clientdata(client); > + > + return sprintf(buf, "%d\n", data->clk); > +} > + > +/* Write function for pwm1_freq sysfs file. */ > +static ssize_t set_pwm_freq(struct device *dev, > + struct device_attribute *da, > + const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = i2c_get_clientdata(client); > + unsigned long val; > + > + if (kstrtoul(buf, 10, &val) || !val) > + return -EINVAL; > + No limits, no overflow protection ? > + mutex_lock(&data->update_lock); > + data->clk = val; > + mutex_unlock(&data->update_lock); Now that lock is really unnecessary. What is clk for, anyway ? Doesn't it have to be written into the chip ? If it is a board specific constant, shouldn't it be provided with platform data or device tree data instead ? What is the purpose of being able to write it at runtime ? > + > + return count; > +} > + > +/* Read function for fan1_div sysfs file. Get fan controller prescaler */ > +static ssize_t get_fan_clk_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 do_set_fan_clk_div(struct device *dev, u8 val) Why ssize_t ? > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = g762_update_client(dev); > + > + mutex_lock(&data->update_lock); > + switch (val) { > + case 1: > + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_CLK_DIV_ID0; > + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_CLK_DIV_ID1; Unnecessary typecasts. > + break; > + case 2: > + data->fan_cmd1 |= G762_REG_FAN_CMD1_CLK_DIV_ID0; > + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_CLK_DIV_ID1; > + break; > + case 4: > + data->fan_cmd1 &= (u8) ~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: > + mutex_unlock(&data->update_lock); > + return -EINVAL; > + } > + g762_write_value(client, G762_REG_FAN_CMD1, (u16)data->fan_cmd1); > + mutex_unlock(&data->update_lock); > + > + return val; What is the purpose of returning val here (instead of returning 0 for no error) ? > +} > + > +/* Write function for fan1_div file. Set fan controller prescaler */ > +static ssize_t set_fan_clk_div(struct device *dev, > + struct device_attribute *da, > + const char *buf, size_t count) > +{ > + unsigned long val; > + ssize_t ret; Why ssize_t ? > + > + if (kstrtoul(buf, 10, &val)) > + return -EINVAL; > + No overflow protection ? > + ret = do_set_fan_clk_div(dev, (u8) val); > + if (ret < 0) > + return -EINVAL; > + > + return count; > +} > + > +/* Read function for fan1_gear_mode sysfs file. Get fan gear mode */ > +static ssize_t get_fan_gear_mode(struct device *dev, > + struct device_attribute *da, char *buf) > +{ > + struct g762_data *data = g762_update_client(dev); > + u8 fan_gear_mode; > + > + fan_gear_mode = data->fan_cmd2 & (G762_REG_FAN_CMD2_GEAR_MODE_0 | > + G762_REG_FAN_CMD2_GEAR_MODE_1); > + fan_gear_mode = fan_gear_mode >> 2; > + > + return sprintf(buf, "%d\n", fan_gear_mode); > +} > + > +static ssize_t do_set_fan_gear_mode(struct device *dev, u8 val) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = g762_update_client(dev); > + > + mutex_lock(&data->update_lock); > + switch (val) { > + case 0: > + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_GEAR_MODE_0; > + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_GEAR_MODE_1; > + break; > + case 1: > + data->fan_cmd2 |= G762_REG_FAN_CMD2_GEAR_MODE_0; > + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_GEAR_MODE_1; > + break; > + case 2: > + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_GEAR_MODE_0; > + data->fan_cmd2 |= G762_REG_FAN_CMD2_GEAR_MODE_1; > + break; > + default: > + mutex_unlock(&data->update_lock); > + return -EINVAL; > + } > + g762_write_value(client, G762_REG_FAN_CMD2, (u16)data->fan_cmd2); > + mutex_unlock(&data->update_lock); > + > + return val; > +} > + > +/* Write function for fan1_gear_mode sysfs file. Write fan gear mode */ > +static ssize_t set_fan_gear_mode(struct device *dev, > + struct device_attribute *da, > + const char *buf, size_t count) > +{ > + unsigned long val; > + ssize_t ret; Why ssize_t ? > + > + if (kstrtoul(buf, 10, &val)) > + return -EINVAL; > + > + ret = do_set_fan_gear_mode(dev, (u8) val); > + if (ret < 0) > + return -EINVAL; > + > + return count; > +} > + > +/* Read function for fan1_pulses. Returns either 2 or 4 */ > +static ssize_t get_fan_pulses(struct device *dev, struct device_attribute *da, > + char *buf) > +{ > + struct g762_data *data = g762_update_client(dev); > + > + return sprintf(buf, "%d\n", PULSE_FROM_REG(data->fan_cmd1)); > +} > + > +/* Set pulse per revolution value. Accepts either 2 or 4. */ > +static ssize_t do_set_fan_pulses(struct device *dev, u8 val) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = g762_update_client(dev); > + > + mutex_lock(&data->update_lock); > + switch (val) { > + case 2: > + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_PULSE_PER_REV; > + break; > + case 4: > + data->fan_cmd1 |= G762_REG_FAN_CMD1_PULSE_PER_REV; > + break; > + default: > + mutex_unlock(&data->update_lock); > + return -EINVAL; > + } > + g762_write_value(client, G762_REG_FAN_CMD1, (u16)data->fan_cmd1); > + mutex_unlock(&data->update_lock); > + > + return val; > +} > + > +/* Write function for fan1_pulses. Accepts either 2 or 4 */ > +static ssize_t set_fan_pulses(struct device *dev, struct device_attribute *da, > + const char *buf, size_t count) > +{ > + unsigned long val; > + ssize_t ret; Why ssize_t ? > + > + if (kstrtoul(buf, 10, &val) || (val != 2 && val != 4)) > + return -EINVAL; > + > + ret = do_set_fan_pulses(dev, (u8) val); > + if (ret < 0) > + 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. */ > +static ssize_t get_speed_control_mode(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 do_set_speed_control_mode(struct device *dev, u8 val) Why ssize_t ? > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = g762_update_client(dev); > + > + if (!val) > + return -EINVAL; > + val -= 1; > + > + mutex_lock(&data->update_lock); > + switch (val) { > + case FAN_MODE_CLOSED_LOOP: > + data->fan_cmd1 |= G762_REG_FAN_CMD1_FAN_MODE; > + break; > + case FAN_MODE_OPEN_LOOP: > + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_FAN_MODE; > + break; > + default: > + mutex_unlock(&data->update_lock); > + return -EINVAL; > + } > + > + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1); > + mutex_unlock(&data->update_lock); > + > + return val; > +} > + > +static ssize_t set_speed_control_mode(struct device *dev, > + struct device_attribute *da, > + const char *buf, size_t count) > +{ > + unsigned long val; > + ssize_t ret; And again ... > + > + if (kstrtoul(buf, 10, &val)) > + return -EINVAL; > + > + ret = do_set_speed_control_mode(dev, val); > + if (ret < 0) > + return -EINVAL; > + > + return count; > +} > + > +/* Read function for pwm1_polarity (0: negative duty, 1: positive duty) */ > +static ssize_t get_pwm_polarity(struct device *dev, struct device_attribute *da, > + char *buf) > +{ > + struct g762_data *data = g762_update_client(dev); > + u8 pwm_polarity; > + > + pwm_polarity = (data->fan_cmd1 & G762_REG_FAN_CMD1_PWM_POLARITY) >> 1; > + > + return sprintf(buf, "%d\n", pwm_polarity); > +} > + > +/* Write function for pwm1_polarity (0: negative duty, 1: positive duty) */ > +static ssize_t set_pwm_polarity(struct device *dev, struct device_attribute *da, > + const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = g762_update_client(dev); > + unsigned long val; > + > + if (kstrtoul(buf, 10, &val)) > + return -EINVAL; > + > + mutex_lock(&data->update_lock); > + switch (val) { > + case 0: /* i.e. negative duty */ > + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_PWM_POLARITY; > + break; > + case 1: /* i.e. positive duty */ > + data->fan_cmd1 |= G762_REG_FAN_CMD1_PWM_POLARITY; > + break; > + default: > + mutex_unlock(&data->update_lock); > + return -EINVAL; > + } > + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1); > + mutex_unlock(&data->update_lock); > + > + 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; > + } > + mutex_unlock(&data->update_lock); > + > + return sprintf(buf, "%d\n", val); > +} > + > +static ssize_t do_set_pwm(struct device *dev, u8 val) and again. > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = g762_update_client(dev); > + > + mutex_lock(&data->update_lock); > + if (data->fan_cmd1 & G762_REG_FAN_CMD1_FAN_MODE) { /* closed-loop */ > + val = PWM_TO_CNT(clamp_val(val, 0, 255)); > + data->set_cnt = val; > + g762_write_value(client, G762_REG_SET_CNT, val); > + } else { /* open-loop */ > + data->set_out = val; > + g762_write_value(client, G762_REG_SET_OUT, val); > + } > + mutex_unlock(&data->update_lock); > + > + return val; Why return val ? Why return anything in the first place ? > +} > + > +static ssize_t set_pwm(struct device *dev, struct device_attribute *da, > + const char *buf, size_t count) > +{ > + unsigned long val; > + ssize_t ret; > + > + if (kstrtoul(buf, 10, &val)) > + return -EINVAL; > + > + ret = do_set_pwm(dev, (u8)val); > + if (ret < 0) > + 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; > + } > + mutex_unlock(&data->update_lock); > + > + return err; > +} > + > +static ssize_t do_set_fan_target(struct device *dev, unsigned int val) No longer commenting on ssize_t. > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = g762_update_client(dev); > + ssize_t err; > + > + 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); > + err = val; > + } else { > + err = -EINVAL; > + } > + 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 int val; > + ssize_t ret; > + > + if (sscanf(buf, "%u", &val) != 1) > + return -EINVAL; > + > + ret = do_set_fan_target(dev, val); > + if (ret < 0) > + return -EINVAL; > + > + return count; > +} > + > +/* read function for 'fan1_fault_detection' sysfs file */ > +static ssize_t get_fan_failure_detection(struct device *dev, > + struct device_attribute *da, > + char *buf) > +{ > + struct g762_data *data = g762_update_client(dev); > + unsigned int enabled; > + > + enabled = (data->fan_cmd1 & G762_REG_FAN_CMD1_DET_FAN_FAIL) ? 1 : 0; > + > + return sprintf(buf, "%u\n", enabled); > +} > + > +/* enable (or disable) fan failure detection */ > +static ssize_t do_fan_failure_detection_toggle(struct device *dev, > + unsigned int enable) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = g762_update_client(dev); > + > + mutex_lock(&data->update_lock); > + switch (enable) { > + case 0: > + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_DET_FAN_FAIL; > + break; > + case 1: > + data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_FAIL; > + break; > + default: > + mutex_unlock(&data->update_lock); > + return -EINVAL; CodingStyle: Single return point. > + } > + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1); > + mutex_unlock(&data->update_lock); > + > + return enable; > +} > + > +/* write function for 'fan1_fault_detection' sysfs file */ > +static ssize_t set_fan_failure_detection(struct device *dev, > + struct device_attribute *da, > + const char *buf, size_t count) > +{ > + unsigned int val; > + ssize_t ret; > + > + if (sscanf(buf, "%u", &val) != 1) > + return -EINVAL; > + > + ret = do_fan_failure_detection_toggle(dev, val); > + if (ret < 0) > + 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; > + > + return sprintf(buf, "%u\n", val); > +} > + > +/* read function for 'fan1_alarm_detection' sysfs file */ > +static ssize_t get_fan_ooc_detection(struct device *dev, > + struct device_attribute *da, > + char *buf) > +{ > + struct g762_data *data = g762_update_client(dev); > + unsigned int enabled; > + > + enabled = (data->fan_cmd1 & G762_REG_FAN_CMD1_DET_FAN_FAIL) ? 1 : 0; > + > + return sprintf(buf, "%u\n", enabled); > +} > + > +/* enable (or disable) fan out of control detection */ > +static ssize_t 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); > + > + mutex_lock(&data->update_lock); > + switch (enable) { > + case 0: > + data->fan_cmd1 &= (u8) ~G762_REG_FAN_CMD1_DET_FAN_OOC; > + break; > + case 1: > + data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_OOC; > + break; > + default: > + mutex_unlock(&data->update_lock); > + return -EINVAL; CodingStyle > + } > + g762_write_value(client, G762_REG_FAN_CMD1, data->fan_cmd1); > + mutex_unlock(&data->update_lock); > + > + return enable; > +} > + > +/* write function for 'fan1_alarm_detection' sysfs file */ > +static ssize_t set_fan_ooc_detection(struct device *dev, > + struct device_attribute *da, > + const char *buf, size_t count) > +{ > + unsigned int val; > + ssize_t ret; > + > + if (sscanf(buf, "%u", &val) != 1) > + return -EINVAL; > + > + ret = do_fan_ooc_detection_toggle(dev, val); > + if (ret < 0) > + return -EINVAL; > + > + return count; > +} > + > +/* 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; > + > + return sprintf(buf, "%u\n", val); > +} > + > +/* Read function for fan1_startup_voltage */ > +static ssize_t get_fan_startup_voltage(struct device *dev, > + struct device_attribute *da, > + char *buf) > +{ > + struct g762_data *data = g762_update_client(dev); > + u8 val; > + > + val = data->fan_cmd2 & (G762_REG_FAN_CMD2_FAN_STARTV_1 | > + G762_REG_FAN_CMD2_FAN_STARTV_0); > + > + return sprintf(buf, "%d\n", val); > +} > + > +/* Write function for fan1_startup_voltage */ > +static ssize_t set_fan_startup_voltage(struct device *dev, > + struct device_attribute *da, > + const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct g762_data *data = g762_update_client(dev); > + unsigned long val; > + > + if (kstrtoul(buf, 10, &val)) > + return -EINVAL; > + > + mutex_lock(&data->update_lock); > + switch (val) { > + case 0: > + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_FAN_STARTV_0; > + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_FAN_STARTV_1; > + break; > + case 1: > + data->fan_cmd2 |= G762_REG_FAN_CMD2_FAN_STARTV_0; > + data->fan_cmd2 &= (u8) ~G762_REG_FAN_CMD2_FAN_STARTV_1; > + break; > + case 2: > + data->fan_cmd2 &= (u8) ~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: > + mutex_unlock(&data->update_lock); > + return -EINVAL; > + } > + g762_write_value(client, G762_REG_FAN_CMD2, (u16)data->fan_cmd2); > + mutex_unlock(&data->update_lock); > + > + return count; > +} > + > +static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, get_pwm, set_pwm); > +static DEVICE_ATTR(pwm1_polarity, S_IWUSR | S_IRUGO, > + get_pwm_polarity, set_pwm_polarity); > +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_speed_control_mode, set_speed_control_mode); > +static DEVICE_ATTR(pwm1_freq, S_IWUSR | S_IRUGO, > + get_pwm_freq, set_pwm_freq); > + > +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_alarm_detection, S_IWUSR | S_IRUGO, > + get_fan_ooc_detection, set_fan_ooc_detection); I don't know ... why not just enable it ? I would suggest to look through the non-standard attributes to see which ones are really needed at runtime. Seems to me they can (and should) all be configuration values, set either through devicetree or platform data. > +static DEVICE_ATTR(fan1_fault, S_IRUGO, get_fan_failure, NULL); > +static DEVICE_ATTR(fan1_fault_detection, S_IWUSR | S_IRUGO, > + get_fan_failure_detection, set_fan_failure_detection); > +static DEVICE_ATTR(fan1_target, S_IWUSR | S_IRUGO, > + get_fan_target, set_fan_target); > +static DEVICE_ATTR(fan1_pulses, S_IWUSR | S_IRUGO, > + get_fan_pulses, set_fan_pulses); > +static DEVICE_ATTR(fan1_div, S_IWUSR | S_IRUGO, > + get_fan_clk_div, set_fan_clk_div); > +static DEVICE_ATTR(fan1_gear_mode, S_IWUSR | S_IRUGO, > + get_fan_gear_mode, set_fan_gear_mode); > +static DEVICE_ATTR(fan1_startup_voltage, S_IWUSR | S_IRUGO, > + get_fan_startup_voltage, set_fan_startup_voltage); > + > +/* Driver data */ > +static struct attribute *g762_attributes[] = { > + &dev_attr_fan1_input.attr, > + &dev_attr_fan1_alarm.attr, > + &dev_attr_fan1_alarm_detection.attr, > + &dev_attr_fan1_fault.attr, > + &dev_attr_fan1_fault_detection.attr, > + &dev_attr_fan1_target.attr, > + &dev_attr_fan1_pulses.attr, > + &dev_attr_fan1_div.attr, > + &dev_attr_fan1_gear_mode.attr, > + &dev_attr_fan1_startup_voltage.attr, > + &dev_attr_pwm1.attr, > + &dev_attr_pwm1_polarity.attr, > + &dev_attr_pwm1_mode.attr, > + &dev_attr_pwm1_enable.attr, > + &dev_attr_pwm1_freq.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) > +{ > + u32 pwm_freq, fan_clk_div, fan_pulses, pwm_mode, fan_target, pwm_enable; > + struct g762_data *data; > + int err; > + > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA)) > + return -ENODEV; > + > + data = devm_kzalloc(&client->dev, sizeof(struct g762_data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + i2c_set_clientdata(client, data); > + > + data->client = client; > + mutex_init(&data->update_lock); > + > + /* Setup default configuration for now. Those may get modified below > + * in CONFIG_OF-protected block (via .dts file). Final values will > + * then be pushed to the device */ > + pwm_freq = G762_DEFAULT_CLK; > + fan_clk_div = G762_DEFAULT_FAN_CLK_DIV; > + fan_pulses = G762_DEFAULT_PULSE_PER_REV; > + pwm_mode = G762_DEFAULT_OUT_MODE; > + pwm_enable = G762_DEFAULT_FAN_MODE + 1; /* shift w/ sysfs interface */ > + fan_target = 0xffffffff; /* dummy value */ > + > + /* 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); > + > +#ifdef CONFIG_OF It would be better to have a separate function for this. > + if (client->dev.of_node) { > + const __be32 *prop; > + int len; > + > + prop = of_get_property(client->dev.of_node, "pwm_freq", &len); > + if (prop && len == sizeof(u32)) { > + pwm_freq = be32_to_cpu(prop[0]); > + dev_info(&client->dev, "found pwm_freq (%d)\n", pwm_freq); > + } > + > + prop = of_get_property(client->dev.of_node, "fan_div", &len); > + if (prop && len == sizeof(u32)) { > + fan_clk_div = be32_to_cpu(prop[0]); > + dev_info(&client->dev, "found fan_div (%d)\n", > + fan_clk_div); > + } > + > + prop = of_get_property(client->dev.of_node, "fan_pulses", &len); > + if (prop && len == sizeof(u32)) { > + fan_pulses = be32_to_cpu(prop[0]); > + dev_info(&client->dev, "found fan_pulses (%d)\n", > + fan_pulses); > + } > + > + prop = of_get_property(client->dev.of_node, "pwm_mode", &len); > + if (prop && len == sizeof(u32)) { > + pwm_mode = be32_to_cpu(prop[0]); > + dev_info(&client->dev, "found pwm_mode (%d)\n", > + pwm_mode); > + } > + > + prop = of_get_property(client->dev.of_node, "fan_target", &len); > + if (prop && len == sizeof(u32)) { > + fan_target = be32_to_cpu(prop[0]); > + dev_info(&client->dev, "found fan_target (%d)\n", > + fan_target); > + } > + > + prop = of_get_property(client->dev.of_node, "pwm_enable", &len); > + if (prop && len == sizeof(u32)) { > + pwm_enable = be32_to_cpu(prop[0]); > + dev_info(&client->dev, "found pwm_enable (%d)\n", > + pwm_enable); > + } > + } > +#endif > + > + /* Now, let's set final fan out mode (pwm or linear), loop mode (closed > + * or open loop), clock freq, pulse per rev, fan clock div, fan target > + * value, and speed control method (closed or open loop) */ > + err = do_set_pwm_mode(&client->dev, pwm_mode); > + if (err < 0) { > + dev_info(&client->dev, "unable to set pwm_mode (%d)\n", > + pwm_mode); > + } > + > + err = do_set_speed_control_mode(&client->dev, pwm_enable); > + if (err < 0) { > + dev_info(&client->dev, "unable to set pwm_enable (%d)\n", > + pwm_enable); None of those are errors ? > + } > + > + err = do_set_fan_clk_div(&client->dev, fan_clk_div); > + if (err < 0) { > + dev_info(&client->dev, "unable to set fan_clk_div (%d)\n", > + fan_clk_div); > + } > + > + err = do_set_fan_pulses(&client->dev, fan_pulses); > + if (err < 0) { > + dev_info(&client->dev, "unable to set fan_pulses (%d)\n", > + fan_pulses); > + } > + > + data->clk = pwm_freq; > + > + if (fan_target != 0xffffffff) { > + err = do_set_fan_target(&client->dev, fan_target); > + if (err < 0) { > + dev_info(&client->dev, "unable to set fan_target (%d)\n", > + fan_target); > + } > + } > + > + /* Register sysfs hooks */ > + err = sysfs_create_group(&client->dev.kobj, &g762_group); > + if (err) > + return err; > + data->hwmon_dev = (struct device *)hwmon_device_register(&client->dev); What is this typecast about ? > + if (IS_ERR(data->hwmon_dev)) { > + err = PTR_ERR(data->hwmon_dev); > + sysfs_remove_group(&client->dev.kobj, &g762_group); > + return err; > + } > + > + dev_info(&data->client->dev, "device successfully initialized\n"); > + > + return 0; > +} > + > +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"); > + return 0; > +} > + > +module_i2c_driver(g762_driver); > + > +MODULE_AUTHOR("Olivier Mouchet <olivier.mouchet@gmail.com>"); > +MODULE_DESCRIPTION("GMT G762 driver"); > +MODULE_LICENSE("GPL"); > -- > 1.7.10.4 > > ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller 2013-04-19 4:35 ` Guenter Roeck @ 2013-04-19 5:34 ` Arnaud Ebalard 2013-04-23 22:05 ` [PATCHv1 0/3] hwmon: " Arnaud Ebalard 1 sibling, 0 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-19 5:34 UTC (permalink / raw) To: linux-arm-kernel Hi, Guenter Roeck <linux@roeck-us.net> writes: > On Fri, Apr 19, 2013 at 12:28:21AM +0200, Arnaud Ebalard wrote: >> >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org> >> Tested-by: Arnaud Ebalard <arno@natisbad.org> >> --- >> drivers/hwmon/Kconfig | 10 + >> drivers/hwmon/Makefile | 1 + >> drivers/hwmon/g762.c | 1159 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 1170 insertions(+) >> create mode 100644 drivers/hwmon/g762.c >> > checkpatch says: > > total: 1 errors, 15 warnings, 0 checks, 1182 lines checked > > Please fix those. Also, please make sure there are spaces around operators. > > Additional comments inline. This is not a complete review; I gave up after > a while. Please fix the problems and resubmit. Thanks for taking the time and sorry for some lame errors (missing spaces around operators, spaces issues, unchecked overflow). Regarding the ssize_t, I will replace it by an int where appropriate; basically all the functions not returning a size value or an error (i.e. those not using sprintf). Will try and resubmit a v1 with all thoses fixed and additional review done after the week end Cheers, a+ ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 0/3] hwmon: GMT G72/G763 PWM fan controller 2013-04-19 4:35 ` Guenter Roeck 2013-04-19 5:34 ` Arnaud Ebalard @ 2013-04-23 22:05 ` Arnaud Ebalard 2013-04-23 22:05 ` [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 " Arnaud Ebalard ` (2 more replies) 1 sibling, 3 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-23 22:05 UTC (permalink / raw) To: linux-arm-kernel Hi, This series adds support for GMT G762/G763. This work is based on a basic version for 2.6.31 kernel developed Olivier Mouchet (kept as author for this reason in g762.c) for LaCie NAS. Updates have been performed to run on recent kernels. Supported has been completed and additional features added: ability to configure various characteristics from .dts file, better initialization, alarms and error reporting support, gear mode, polarity, fan pulse per revolution, fan startup voltage control. The following detailed datasheet has been used as a basis for this work: http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf The patch was developed for and tested against the GMT G762 fan controller used in a Netgear ReadyNAS Duo v2 (kirkwood 88F6282-based NAS). This is the main reason for the device tree bindings provided in first patch. The patches are against current ARM tree; tell me if you need me to rebase it against something else. Patch 2 and 3 provides documentation for the driver and DT bindings, respectively. I hope the comments provided on v0 have all been correctly taken into account. A list of changes is provided below. Comments welcome, Cheers, a+ Changes since v0: Removed forward declaration Used bool for 'valid' field instead of bit field. Protected macro args Fixed typo in subject line Added mention for G763 support in Kconfig Fixed typo in driver name in Kconfig Do not use DRVNAME in i2c_device_id g762_id[] Following discussions, kept DEVICE_ATTR (i.e. no switch to SENSOR_DEVICE_ATTR) Removed useless casts when flipping bit values Sanity check user input value (e.g. to prevent 256 to silenty become 0) Added extra lines for multiline comments when needed Removed various testing knobs Make removed knobs available via DT Passed checkpatch script on the patch Removed useless lock protection againt clk setting Moved all setter at the beginning of the file Removed bad (u16) casts in g762_write_value() calls Added config structure and helpers Provide specific helper to overload config from dts Arnaud Ebalard (3): Add support for GMT G762/G763 PWM fan controller Add documentation for g762 driver Add DT documentation for g762 driver Documentation/devicetree/bindings/hwmon/g762.txt | 57 ++ Documentation/hwmon/g762 | 67 ++ drivers/hwmon/Kconfig | 10 + drivers/hwmon/Makefile | 1 + drivers/hwmon/g762.c | 1058 ++++++++++++++++++++++ 5 files changed, 1193 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/g762.txt create mode 100644 Documentation/hwmon/g762 create mode 100644 drivers/hwmon/g762.c -- 1.7.10.4 ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-23 22:05 ` [PATCHv1 0/3] hwmon: " Arnaud Ebalard @ 2013-04-23 22:05 ` Arnaud Ebalard 2013-04-24 5:37 ` Andrew Lunn ` (4 more replies) 2013-04-23 22:06 ` [PATCHv1 2/3] hwmon: Add documentation for g762 driver Arnaud Ebalard 2013-04-23 22:06 ` [PATCHv1 3/3] hwmon: Add DT " Arnaud Ebalard 2 siblings, 5 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-23 22:05 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Arnaud Ebalard <arno@natisbad.org> Tested-by: Arnaud Ebalard <arno@natisbad.org> --- 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@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@gmail.com> + * + * based on g760a code written by Herbert Valerio Riedel <hvr@gnu.org> + * Copyright (C) 2007 Herbert Valerio Riedel <hvr@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 + */ + +#include <linux/device.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/slab.h> +#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) + +/* + * 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)) + +/* + * 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); +} + +/* 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; + } + mutex_unlock(&data->update_lock); + + return data; +} + + + +/* + * helpers for passing hardware characteristics via DT. Some of those + * are also used by sysfs handlers (write function) later in the file. + */ + + +/* 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; + + 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; + } + 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); + 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)) { + 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); + } +} + +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) { }; +#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). + */ +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); +} + +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. + */ +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; + } + 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; + } + 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; + + 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; + + 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); + +/* 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; + + if (!i2c_check_functionality(client->adapter, + I2C_FUNC_SMBUS_BYTE_DATA)) { + err = -ENODEV; + goto out; + } + + data = devm_kzalloc(&client->dev, sizeof(struct g762_data), GFP_KERNEL); + if (!data) { + err = -ENOMEM; + goto out; + } + 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; + + /* Register sysfs hooks */ + err = sysfs_create_group(&client->dev.kobj, &g762_group); + if (err) + goto out; + + 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); + goto out; + } + + dev_info(&data->client->dev, "device successfully initialized\n"); + + 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"); + 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@gmail.com>"); +MODULE_DESCRIPTION("GMT G762 driver"); +MODULE_LICENSE("GPL"); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-23 22:05 ` [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 " Arnaud Ebalard @ 2013-04-24 5:37 ` Andrew Lunn 2013-04-24 9:06 ` Arnaud Ebalard 2013-04-24 17:06 ` Simon Guinot ` (3 subsequent siblings) 4 siblings, 1 reply; 34+ messages in thread From: Andrew Lunn @ 2013-04-24 5:37 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote: > > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > Tested-by: Arnaud Ebalard <arno@natisbad.org> > --- > 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@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@gmail.com> > + * > + * based on g760a code written by Herbert Valerio Riedel <hvr@gnu.org> > + * Copyright (C) 2007 Herbert Valerio Riedel <hvr@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 > + */ > + > +#include <linux/device.h> > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/slab.h> > +#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) > + > +/* > + * 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) > + */ You could consider using regmap for holding this cache. http://elceurope2012.sched.org/event/100619b669ce5767341624253aa03659?iframe=no&w=900&sidebar=yes&bg=no#.UXdspHLQ5jM http://elinux.org/ELCE_Europe_2012_Presentations Nice documentation by the way. > +/* > + * Helpers to import hardware characteristics from .dts file and overload > + * default config values. > + */ > + > +#ifdef CONFIG_OF Can the driver be used without device tree? Would it be simpler to just add depends OF in the Kconfig entry? Andrew ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-24 5:37 ` Andrew Lunn @ 2013-04-24 9:06 ` Arnaud Ebalard 2013-04-24 10:04 ` Simon Guinot 2013-04-24 13:38 ` Guenter Roeck 0 siblings, 2 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-24 9:06 UTC (permalink / raw) To: linux-arm-kernel Hi Andrew, Andrew Lunn <andrew@lunn.ch> writes: >> +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) >> + */ > > You could consider using regmap for holding this cache. > > http://elceurope2012.sched.org/event/100619b669ce5767341624253aa03659?iframe=no&w=900&sidebar=yes&bg=no#.UXdspHLQ5jM > > http://elinux.org/ELCE_Europe_2012_Presentations Interesting. As I am not yet familiar w/ regmap I would prefer having the driver accepted during merge window with current data structure and then convert it to regmap. But I will take a look (e.g. will study fca1dd03 for instance). Thanks for the pointers. >> +/* >> + * Helpers to import hardware characteristics from .dts file and overload >> + * default config values. >> + */ >> + >> +#ifdef CONFIG_OF > > Can the driver be used without device tree? Would it be simpler to > just add depends OF in the Kconfig entry? It can be used if the default params (or those configured by u-boot I guess) fit your needs. I think it would be fairly easy to extend the driver later to expose g762_config struct to allow parameters to be set w/o using OF. If someone wants to do that, I think it is better to not depend on OF in Kconfig at the moment but I have not strong argument other that that one. I'll let you decide. Cheers, a+ ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-24 9:06 ` Arnaud Ebalard @ 2013-04-24 10:04 ` Simon Guinot 2013-04-24 10:50 ` Arnaud Ebalard 2013-04-24 13:38 ` Guenter Roeck 1 sibling, 1 reply; 34+ messages in thread From: Simon Guinot @ 2013-04-24 10:04 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 24, 2013 at 11:06:57AM +0200, Arnaud Ebalard wrote: > Hi Andrew, > > Andrew Lunn <andrew@lunn.ch> writes: > > >> +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) > >> + */ > > > > You could consider using regmap for holding this cache. > > > > http://elceurope2012.sched.org/event/100619b669ce5767341624253aa03659?iframe=no&w=900&sidebar=yes&bg=no#.UXdspHLQ5jM > > > > http://elinux.org/ELCE_Europe_2012_Presentations > > Interesting. As I am not yet familiar w/ regmap I would prefer having > the driver accepted during merge window with current data structure and > then convert it to regmap. But I will take a look (e.g. will study > fca1dd03 for instance). Thanks for the pointers. > > >> +/* > >> + * Helpers to import hardware characteristics from .dts file and overload > >> + * default config values. > >> + */ > >> + > >> +#ifdef CONFIG_OF > > > > Can the driver be used without device tree? Would it be simpler to > > just add depends OF in the Kconfig entry? > > It can be used if the default params (or those configured by u-boot I > guess) fit your needs. I think it would be fairly easy to extend the > driver later to expose g762_config struct to allow parameters to be set > w/o using OF. If someone wants to do that, I think it is better to not > depend on OF in Kconfig at the moment but I have not strong argument > other that that one. I'll let you decide. A g762 device is embedded on the 2Big Network v2 board (net2big_v2), which is not DT compliant. Then, I think it could be nice to allow device registration from board setup files. Regards, Simon -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130424/4eb49d91/attachment.sig> ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-24 10:04 ` Simon Guinot @ 2013-04-24 10:50 ` Arnaud Ebalard 0 siblings, 0 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-24 10:50 UTC (permalink / raw) To: linux-arm-kernel Hi Simon, Simon Guinot <simon.guinot@sequanux.org> writes: >> It can be used if the default params (or those configured by u-boot I >> guess) fit your needs. I think it would be fairly easy to extend the >> driver later to expose g762_config struct to allow parameters to be set >> w/o using OF. If someone wants to do that, I think it is better to not >> depend on OF in Kconfig at the moment but I have not strong argument >> other that that one. I'll let you decide. > > A g762 device is embedded on the 2Big Network v2 board (net2big_v2), > which is not DT compliant. Then, I think it could be nice to allow > device registration from board setup files. ok, I will try and provide that for v2 then. :-) Cheers, a+ ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-24 9:06 ` Arnaud Ebalard 2013-04-24 10:04 ` Simon Guinot @ 2013-04-24 13:38 ` Guenter Roeck 2013-04-24 20:28 ` Arnaud Ebalard 1 sibling, 1 reply; 34+ messages in thread From: Guenter Roeck @ 2013-04-24 13:38 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 24, 2013 at 11:06:57AM +0200, Arnaud Ebalard wrote: > Hi Andrew, > > Andrew Lunn <andrew@lunn.ch> writes: > > >> +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) > >> + */ > > > > You could consider using regmap for holding this cache. > > > > http://elceurope2012.sched.org/event/100619b669ce5767341624253aa03659?iframe=no&w=900&sidebar=yes&bg=no#.UXdspHLQ5jM > > > > http://elinux.org/ELCE_Europe_2012_Presentations > > Interesting. As I am not yet familiar w/ regmap I would prefer having > the driver accepted during merge window with current data structure and > then convert it to regmap. But I will take a look (e.g. will study > fca1dd03 for instance). Thanks for the pointers. > I have not had time for a detailed review, but adding regmap support will not be a requirement. Note that it is too late for 3.10, so the driver will have to wait for 3.11. > >> +/* > >> + * Helpers to import hardware characteristics from .dts file and overload > >> + * default config values. > >> + */ > >> + > >> +#ifdef CONFIG_OF > > > > Can the driver be used without device tree? Would it be simpler to > > just add depends OF in the Kconfig entry? > > It can be used if the default params (or those configured by u-boot I > guess) fit your needs. I think it would be fairly easy to extend the > driver later to expose g762_config struct to allow parameters to be set > w/o using OF. If someone wants to do that, I think it is better to not > depend on OF in Kconfig at the moment but I have not strong argument > other that that one. I'll let you decide. > Agreed. As long as there are major platforms not supporting device tree, and as long as device tree overlays are not supported, it must not be made mandatory. Especially for I2C and SPI devices I reserve the right to be able test the hardware on an X86 system and not require a reboot to do so. Guenter ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-24 13:38 ` Guenter Roeck @ 2013-04-24 20:28 ` Arnaud Ebalard 2013-04-24 22:47 ` Guenter Roeck 0 siblings, 1 reply; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-24 20:28 UTC (permalink / raw) To: linux-arm-kernel Hi, Guenter Roeck <linux@roeck-us.net> writes: >> > You could consider using regmap for holding this cache. >> > >> > http://elceurope2012.sched.org/event/100619b669ce5767341624253aa03659?iframe=no&w=900&sidebar=yes&bg=no#.UXdspHLQ5jM >> > >> > http://elinux.org/ELCE_Europe_2012_Presentations >> >> Interesting. As I am not yet familiar w/ regmap I would prefer having >> the driver accepted during merge window with current data structure and >> then convert it to regmap. But I will take a look (e.g. will study >> fca1dd03 for instance). Thanks for the pointers. >> > I have not had time for a detailed review, but adding regmap support > will not be a requirement. > > Note that it is too late for 3.10, so the driver will have to wait for > 3.11. Well, I think I can live with it; The only bad thing is that it kills my excuses not to spend time on regmap ;-) >> >> +/* >> >> + * Helpers to import hardware characteristics from .dts file and overload >> >> + * default config values. >> >> + */ >> >> + >> >> +#ifdef CONFIG_OF >> > >> > Can the driver be used without device tree? Would it be simpler to >> > just add depends OF in the Kconfig entry? >> >> It can be used if the default params (or those configured by u-boot I >> guess) fit your needs. I think it would be fairly easy to extend the >> driver later to expose g762_config struct to allow parameters to be set >> w/o using OF. If someone wants to do that, I think it is better to not >> depend on OF in Kconfig at the moment but I have not strong argument >> other that that one. I'll let you decide. >> > Agreed. As long as there are major platforms not supporting device tree, > and as long as device tree overlays are not supported, it must not be made > mandatory. Especially for I2C and SPI devices I reserve the right to be able > test the hardware on an X86 system and not require a reboot to do so. Understood. Following Simon's post, I think it's useful to implement some init function for non DT-enabled platforms. a+ ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-24 20:28 ` Arnaud Ebalard @ 2013-04-24 22:47 ` Guenter Roeck 2013-04-25 10:14 ` Simon Guinot 0 siblings, 1 reply; 34+ messages in thread From: Guenter Roeck @ 2013-04-24 22:47 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 24, 2013 at 10:28:47PM +0200, Arnaud Ebalard wrote: > Hi, > > Guenter Roeck <linux@roeck-us.net> writes: > > >> > You could consider using regmap for holding this cache. > >> > > >> > http://elceurope2012.sched.org/event/100619b669ce5767341624253aa03659?iframe=no&w=900&sidebar=yes&bg=no#.UXdspHLQ5jM > >> > > >> > http://elinux.org/ELCE_Europe_2012_Presentations > >> > >> Interesting. As I am not yet familiar w/ regmap I would prefer having > >> the driver accepted during merge window with current data structure and > >> then convert it to regmap. But I will take a look (e.g. will study > >> fca1dd03 for instance). Thanks for the pointers. > >> > > I have not had time for a detailed review, but adding regmap support > > will not be a requirement. > > > > Note that it is too late for 3.10, so the driver will have to wait for > > 3.11. > > Well, I think I can live with it; The only bad thing is that it kills my > excuses not to spend time on regmap ;-) > Your call. > > >> >> +/* > >> >> + * Helpers to import hardware characteristics from .dts file and overload > >> >> + * default config values. > >> >> + */ > >> >> + > >> >> +#ifdef CONFIG_OF > >> > > >> > Can the driver be used without device tree? Would it be simpler to > >> > just add depends OF in the Kconfig entry? > >> > >> It can be used if the default params (or those configured by u-boot I > >> guess) fit your needs. I think it would be fairly easy to extend the > >> driver later to expose g762_config struct to allow parameters to be set > >> w/o using OF. If someone wants to do that, I think it is better to not > >> depend on OF in Kconfig at the moment but I have not strong argument > >> other that that one. I'll let you decide. > >> > > Agreed. As long as there are major platforms not supporting device tree, > > and as long as device tree overlays are not supported, it must not be made > > mandatory. Especially for I2C and SPI devices I reserve the right to be able > > test the hardware on an X86 system and not require a reboot to do so. > > Understood. Following Simon's post, I think it's useful to implement > some init function for non DT-enabled platforms. > Sure, as long as Simon commits to test it and - if possible - to provide the necessary platform code. Guenter ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-24 22:47 ` Guenter Roeck @ 2013-04-25 10:14 ` Simon Guinot 0 siblings, 0 replies; 34+ messages in thread From: Simon Guinot @ 2013-04-25 10:14 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 24, 2013 at 03:47:31PM -0700, Guenter Roeck wrote: > On Wed, Apr 24, 2013 at 10:28:47PM +0200, Arnaud Ebalard wrote: > > Hi, > > > > Guenter Roeck <linux@roeck-us.net> writes: > > > > >> > You could consider using regmap for holding this cache. > > >> > > > >> > http://elceurope2012.sched.org/event/100619b669ce5767341624253aa03659?iframe=no&w=900&sidebar=yes&bg=no#.UXdspHLQ5jM > > >> > > > >> > http://elinux.org/ELCE_Europe_2012_Presentations > > >> > > >> Interesting. As I am not yet familiar w/ regmap I would prefer having > > >> the driver accepted during merge window with current data structure and > > >> then convert it to regmap. But I will take a look (e.g. will study > > >> fca1dd03 for instance). Thanks for the pointers. > > >> > > > I have not had time for a detailed review, but adding regmap support > > > will not be a requirement. > > > > > > Note that it is too late for 3.10, so the driver will have to wait for > > > 3.11. > > > > Well, I think I can live with it; The only bad thing is that it kills my > > excuses not to spend time on regmap ;-) > > > Your call. > > > > > >> >> +/* > > >> >> + * Helpers to import hardware characteristics from .dts file and overload > > >> >> + * default config values. > > >> >> + */ > > >> >> + > > >> >> +#ifdef CONFIG_OF > > >> > > > >> > Can the driver be used without device tree? Would it be simpler to > > >> > just add depends OF in the Kconfig entry? > > >> > > >> It can be used if the default params (or those configured by u-boot I > > >> guess) fit your needs. I think it would be fairly easy to extend the > > >> driver later to expose g762_config struct to allow parameters to be set > > >> w/o using OF. If someone wants to do that, I think it is better to not > > >> depend on OF in Kconfig at the moment but I have not strong argument > > >> other that that one. I'll let you decide. > > >> > > > Agreed. As long as there are major platforms not supporting device tree, > > > and as long as device tree overlays are not supported, it must not be made > > > mandatory. Especially for I2C and SPI devices I reserve the right to be able > > > test the hardware on an X86 system and not require a reboot to do so. > > > > Understood. Following Simon's post, I think it's useful to implement > > some init function for non DT-enabled platforms. > > > Sure, as long as Simon commits to test it and - if possible - to provide > the necessary platform code. Hi Guenter, I commit to do whatever needed to have g762 support on the 2Big Network v2 and 2Big NAS boards. But note that I am can't test the options "pwm_enable" and "fan_target". As we are using a two wire fan on this boards, this options are not relevant. Moreover except for the out_mode/pwm_mode setting, I am rather happy with the default configuration. Simon -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130425/fdf1f927/attachment-0001.sig> ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-23 22:05 ` [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 " Arnaud Ebalard 2013-04-24 5:37 ` Andrew Lunn @ 2013-04-24 17:06 ` Simon Guinot 2013-04-24 23:37 ` Guenter Roeck ` (2 subsequent siblings) 4 siblings, 0 replies; 34+ messages in thread From: Simon Guinot @ 2013-04-24 17:06 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote: > > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > Tested-by: Arnaud Ebalard <arno@natisbad.org> Hi Arnaud, Thanks for this patch. I have partially tested this driver (with the default configuration) on a net2big_v2 board. I say "partially" because on this board, a two wire fan is connected to the g762. This means that the measure speed and alarm features as well as the closed-loop mode are not available. But all what is left seems to work as expected. See below for a few remarks and questions about the driver code. > --- > 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 Snip > +/* 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); I noticed you never check the value returned by the functions g762_{read,write}_value(). Is that because the functions i2c_smbus_{read,write}_byte_data() are not likely to fail ? > + 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; > + > + 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; > + } > + 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; > +} It seems to me that all the functions above are more or less using the same code. For example do_set_fan_pulses, do_set_pwm_enable and do_set_pwm_polarity are very similar. Then, there is probably room for some code factorisation ? For example, you could use some functions to set or clear a mask against a given register. Or something else... > + > + > + > +/* > + * 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); > + 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)) { Is that correct ? Here, g762_update_client() is called several times in a short time interval. This means that the registers are not re-read after each write operation (only after the first). If the chip updates internally some registers after a write, then a mismatch between struct g762_data and the real registers values could happen. > + ret = -EINVAL; > + } > + > + return ret; > +} Snip > +/* > + * 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; > + } I don't think you need to use braces for the above statement. Regards, Simon -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130424/9838afdc/attachment-0001.sig> ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-23 22:05 ` [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 " Arnaud Ebalard 2013-04-24 5:37 ` Andrew Lunn 2013-04-24 17:06 ` Simon Guinot @ 2013-04-24 23:37 ` Guenter Roeck 2013-04-25 9:58 ` Simon Guinot 2013-04-27 14:03 ` Simon Guinot 4 siblings, 0 replies; 34+ messages in thread From: Guenter Roeck @ 2013-04-24 23:37 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote: > > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > Tested-by: Arnaud Ebalard <arno@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@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@gmail.com> > + * > + * based on g760a code written by Herbert Valerio Riedel <hvr@gnu.org> > + * Copyright (C) 2007 Herbert Valerio Riedel <hvr@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@gmail.com>"); > +MODULE_DESCRIPTION("GMT G762 driver"); > +MODULE_LICENSE("GPL"); > -- > 1.7.10.4 > > ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-23 22:05 ` [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 " Arnaud Ebalard ` (2 preceding siblings ...) 2013-04-24 23:37 ` Guenter Roeck @ 2013-04-25 9:58 ` Simon Guinot 2013-04-27 14:03 ` Simon Guinot 4 siblings, 0 replies; 34+ messages in thread From: Simon Guinot @ 2013-04-25 9:58 UTC (permalink / raw) To: linux-arm-kernel Hi Arnaud, On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote: > 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 Snip > + > +/* > + * 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 Maybe you could use here the same macros as the ones used in the do_set_* functions: #define G762_DEFAULT_OUT_MODE OUT_MODE_DAC #define G762_DEFAULT_FAN_MODE FAN_MODE_CLOSED_LOOP Simon -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130425/3ed71656/attachment.sig> ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-23 22:05 ` [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 " Arnaud Ebalard ` (3 preceding siblings ...) 2013-04-25 9:58 ` Simon Guinot @ 2013-04-27 14:03 ` Simon Guinot 2013-04-27 14:12 ` Jean Delvare 2013-04-27 16:56 ` Guenter Roeck 4 siblings, 2 replies; 34+ messages in thread From: Simon Guinot @ 2013-04-27 14:03 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote: > + /* > + * 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; One more comment... Sorry for the multiple replies :) I am not sure that applying a configuration anyway is a good idea. I think it could be best to stick with the bootloader configuration if no platform data nor device tree data are given. Simon -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130427/d3d421c7/attachment.sig> ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-27 14:03 ` Simon Guinot @ 2013-04-27 14:12 ` Jean Delvare 2013-04-27 16:56 ` Guenter Roeck 1 sibling, 0 replies; 34+ messages in thread From: Jean Delvare @ 2013-04-27 14:12 UTC (permalink / raw) To: linux-arm-kernel On Sat, 27 Apr 2013 16:03:31 +0200, Simon Guinot wrote: > On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote: > > + /* > > + * 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; > > One more comment... Sorry for the multiple replies :) > > I am not sure that applying a configuration anyway is a good idea. > I think it could be best to stick with the bootloader configuration if > no platform data nor device tree data are given. FWIW, the common policy for hwmon drivers is actually to not change the device configuration unless they really have to. -- Jean Delvare ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-27 14:03 ` Simon Guinot 2013-04-27 14:12 ` Jean Delvare @ 2013-04-27 16:56 ` Guenter Roeck 2013-04-27 18:55 ` Arnaud Ebalard 1 sibling, 1 reply; 34+ messages in thread From: Guenter Roeck @ 2013-04-27 16:56 UTC (permalink / raw) To: linux-arm-kernel On Sat, Apr 27, 2013 at 04:03:31PM +0200, Simon Guinot wrote: > On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote: > > + /* > > + * 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; > > One more comment... Sorry for the multiple replies :) > > I am not sure that applying a configuration anyway is a good idea. > I think it could be best to stick with the bootloader configuration if > no platform data nor device tree data are given. > Yes, good point. You are absolutely right. Guenter ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 PWM fan controller 2013-04-27 16:56 ` Guenter Roeck @ 2013-04-27 18:55 ` Arnaud Ebalard 0 siblings, 0 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-27 18:55 UTC (permalink / raw) To: linux-arm-kernel Hi Guenter, Jean, Simon, Guenter Roeck <linux@roeck-us.net> writes: > On Sat, Apr 27, 2013 at 04:03:31PM +0200, Simon Guinot wrote: >> On Wed, Apr 24, 2013 at 12:05:56AM +0200, Arnaud Ebalard wrote: >> > + /* >> > + * 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; >> >> One more comment... Sorry for the multiple replies :) >> >> I am not sure that applying a configuration anyway is a good idea. >> I think it could be best to stick with the bootloader configuration if >> no platform data nor device tree data are given. >> > Yes, good point. You are absolutely right. Thanks you all for your feedback. I will take all your comments into account and work on a new version. I am somewhat unavailable next week so may not be able to push something before next week end. Cheers, a+ ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 2/3] hwmon: Add documentation for g762 driver 2013-04-23 22:05 ` [PATCHv1 0/3] hwmon: " Arnaud Ebalard 2013-04-23 22:05 ` [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 " Arnaud Ebalard @ 2013-04-23 22:06 ` Arnaud Ebalard 2013-04-24 17:32 ` Guenter Roeck 2013-04-23 22:06 ` [PATCHv1 3/3] hwmon: Add DT " Arnaud Ebalard 2 siblings, 1 reply; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-23 22:06 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Arnaud Ebalard <arno@natisbad.org> --- Documentation/hwmon/g762 | 67 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 Documentation/hwmon/g762 diff --git a/Documentation/hwmon/g762 b/Documentation/hwmon/g762 new file mode 100644 index 0000000..5015829 --- /dev/null +++ b/Documentation/hwmon/g762 @@ -0,0 +1,67 @@ +Kernel driver g762 +================== + +This documentation is based on G760a one, GMT G762 datasheet available +at http://natisbad.org/NAS/ref/GMT_EDS-762_763-080710-0.2.pdf (used +for the development of the driver) and sysfs bindings described in +Documentation/hwmon/sysfs-interface. + +The GMT G762 Fan Speed PWM Controller is connected directly to a fan +and performs closed-loop or open-loop control of the fan speed. Two +modes - PWM or linear - are supported by the device. + +The following entries are available to the user in a subdirectory of +/sys/bus/i2c/drivers/g762/ to control the operation of the device. +This can be done manually using the following entries but is usually +done via a userland daemon like fancontrol. + +Note that those entries do not provide ways to setup the specific +hardware characteristics of the system (reference clock, pulses per +fan revolution, ...); Those can be modified via devicetree bindings +documented in Documentation/devicetree/bindings/hwmon/g762.txt. + + + fan1_target: set desired fan speed. This only makes sense in closed-loop + fan speed control (i.e. when pwm1_enable is set to 2). + + fan1_input: provide current fan rotation value in RPM as reported by + the fan to the device. + + fan1_div: fan clock divisor. Supported value are 1, 2, 4 and 8. Default + value is 1. + + fan1_gear_mode: fan gear mode. Supported values are 0, 1 and 2. Default + value is 0. The value is a characteristic of the system + and a higher value can be set for more precisely control + fans with a high rotation speed. Note that this affects the + measurable speed range, not the read value. + + fan1_fault: reports fan failure, i.e. no transition on fan gear pin for + about 0.7s (if the fan is not voluntarily set off). + + fan1_alarm: in closed-loop control mode, if fan RPM value is 25% out + of the programmed value for over 6 seconds 'fan1_alarm' is + set to 1. + + pwm1: get or set PWM fan control value. This is an integer value + between 0 and 255. 0 stops the fan, 255 makes it run at + full speed. + + pwm1_enable: set current fan speed control mode i.e. 1 for manual fan + speed control (open-loop), 2 for automatic fan speed control + (closed-loop). + + pwm1_mode: set or get fan driving mode: 1 for PWM mode, 0 for linear + mode. Default is linear mode. + + +In PWM mode ('pwm1_mode' set to 1), the fan speed is programmed either by +setting a value between 0 and 255 via 'pwm1' entry (0 stops the fan, 255 +makes it run at full speed). This can also be done by passing the +expected RPM value via 'fan1_target'. Current fan speed value can be +retrieved via 'fan1_input'. This fan speed value is computed based on +the parameters associated with the physical characteristics of the +system: a reference clock source frequency, a number of pulses per fan +revolution, etc. + +Note that the driver will update its values at most once per second. -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv1 2/3] hwmon: Add documentation for g762 driver 2013-04-23 22:06 ` [PATCHv1 2/3] hwmon: Add documentation for g762 driver Arnaud Ebalard @ 2013-04-24 17:32 ` Guenter Roeck 2013-04-24 20:33 ` Arnaud Ebalard 0 siblings, 1 reply; 34+ messages in thread From: Guenter Roeck @ 2013-04-24 17:32 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 24, 2013 at 12:06:08AM +0200, Arnaud Ebalard wrote: > > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > --- > Documentation/hwmon/g762 | 67 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 67 insertions(+) > create mode 100644 Documentation/hwmon/g762 > > diff --git a/Documentation/hwmon/g762 b/Documentation/hwmon/g762 > new file mode 100644 > index 0000000..5015829 > --- /dev/null > +++ b/Documentation/hwmon/g762 > @@ -0,0 +1,67 @@ > +Kernel driver g762 > +================== > + > +This documentation is based on G760a one, GMT G762 datasheet available G760a ? If this refers to Documentation/hwmon/g760a, I think it is just confusing. > +at http://natisbad.org/NAS/ref/GMT_EDS-762_763-080710-0.2.pdf (used > +for the development of the driver) and sysfs bindings described in I would hope that you used the data sheet for the driver ;), but that information does not add much if any value here. I would suggest to drop it. Just point to the data sheet. > +Documentation/hwmon/sysfs-interface. > + > +The GMT G762 Fan Speed PWM Controller is connected directly to a fan > +and performs closed-loop or open-loop control of the fan speed. Two > +modes - PWM or linear - are supported by the device. > + > +The following entries are available to the user in a subdirectory of > +/sys/bus/i2c/drivers/g762/ to control the operation of the device. > +This can be done manually using the following entries but is usually > +done via a userland daemon like fancontrol. > + > +Note that those entries do not provide ways to setup the specific > +hardware characteristics of the system (reference clock, pulses per > +fan revolution, ...); Those can be modified via devicetree bindings > +documented in Documentation/devicetree/bindings/hwmon/g762.txt. > + > + > + fan1_target: set desired fan speed. This only makes sense in closed-loop > + fan speed control (i.e. when pwm1_enable is set to 2). > + > + fan1_input: provide current fan rotation value in RPM as reported by > + the fan to the device. > + > + fan1_div: fan clock divisor. Supported value are 1, 2, 4 and 8. Default > + value is 1. > + > + fan1_gear_mode: fan gear mode. Supported values are 0, 1 and 2. Default > + value is 0. The value is a characteristic of the system > + and a higher value can be set for more precisely control > + fans with a high rotation speed. Note that this affects the > + measurable speed range, not the read value. > + This attribute no longer exists. > + fan1_fault: reports fan failure, i.e. no transition on fan gear pin for > + about 0.7s (if the fan is not voluntarily set off). > + > + fan1_alarm: in closed-loop control mode, if fan RPM value is 25% out > + of the programmed value for over 6 seconds 'fan1_alarm' is > + set to 1. > + > + pwm1: get or set PWM fan control value. This is an integer value > + between 0 and 255. 0 stops the fan, 255 makes it run at > + full speed. > + > + pwm1_enable: set current fan speed control mode i.e. 1 for manual fan > + speed control (open-loop), 2 for automatic fan speed control > + (closed-loop). > + > + pwm1_mode: set or get fan driving mode: 1 for PWM mode, 0 for linear > + mode. Default is linear mode. > + > + > +In PWM mode ('pwm1_mode' set to 1), the fan speed is programmed either by > +setting a value between 0 and 255 via 'pwm1' entry (0 stops the fan, 255 > +makes it run at full speed). This can also be done by passing the > +expected RPM value via 'fan1_target'. Current fan speed value can be > +retrieved via 'fan1_input'. This fan speed value is computed based on > +the parameters associated with the physical characteristics of the > +system: a reference clock source frequency, a number of pulses per fan > +revolution, etc. > + > +Note that the driver will update its values at most once per second. > -- > 1.7.10.4 > > ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 2/3] hwmon: Add documentation for g762 driver 2013-04-24 17:32 ` Guenter Roeck @ 2013-04-24 20:33 ` Arnaud Ebalard 0 siblings, 0 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-24 20:33 UTC (permalink / raw) To: linux-arm-kernel Hi, Guenter Roeck <linux@roeck-us.net> writes: > On Wed, Apr 24, 2013 at 12:06:08AM +0200, Arnaud Ebalard wrote: >> >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org> >> --- >> Documentation/hwmon/g762 | 67 ++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 67 insertions(+) >> create mode 100644 Documentation/hwmon/g762 >> >> diff --git a/Documentation/hwmon/g762 b/Documentation/hwmon/g762 >> new file mode 100644 >> index 0000000..5015829 >> --- /dev/null >> +++ b/Documentation/hwmon/g762 >> @@ -0,0 +1,67 @@ >> +Kernel driver g762 >> +================== >> + >> +This documentation is based on G760a one, GMT G762 datasheet available > > G760a ? If this refers to Documentation/hwmon/g760a, I think it is just > confusing. > >> +at http://natisbad.org/NAS/ref/GMT_EDS-762_763-080710-0.2.pdf (used >> +for the development of the driver) and sysfs bindings described in > > I would hope that you used the data sheet for the driver ;), but that > information does not add much if any value here. I would suggest > to drop it. Just point to the data sheet. > >> + fan1_gear_mode: fan gear mode. Supported values are 0, 1 and 2. Default >> + value is 0. The value is a characteristic of the system >> + and a higher value can be set for more precisely control >> + fans with a high rotation speed. Note that this affects the >> + measurable speed range, not the read value. >> + > This attribute no longer exists. All thoses will be fixed in v2. a+ ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 3/3] hwmon: Add DT documentation for g762 driver 2013-04-23 22:05 ` [PATCHv1 0/3] hwmon: " Arnaud Ebalard 2013-04-23 22:05 ` [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 " Arnaud Ebalard 2013-04-23 22:06 ` [PATCHv1 2/3] hwmon: Add documentation for g762 driver Arnaud Ebalard @ 2013-04-23 22:06 ` Arnaud Ebalard 2013-04-23 22:23 ` Jason Cooper 2 siblings, 1 reply; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-23 22:06 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Arnaud Ebalard <arno@natisbad.org> --- Documentation/devicetree/bindings/hwmon/g762.txt | 57 ++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/g762.txt diff --git a/Documentation/devicetree/bindings/hwmon/g762.txt b/Documentation/devicetree/bindings/hwmon/g762.txt new file mode 100644 index 0000000..87d7cf5 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/g762.txt @@ -0,0 +1,57 @@ +GMT G762/G763 PWM Fan controller + +Required node properties: + + - "compatible": must be either "gmt,g762" or "gmt,g763" + - "reg": I2C bus address of the device + +Optional properties: + + - "pwm_mode": fan driving mode. 1 for PWM mode, 0 for linear. Default + value is 0, i.e. linear. + + - "pwm_enable": fan speed control. 1 for open-loop, 2 for closed-loop. + Default value is 2, i.e. closed-loop. + + - "pwm_freq": reference clock frequency for PWM mode in Hz. Default is + 32768. + + - "fan_pulses": number of pulses per fan revolution. Supported values + are 2 and 4. Default is 2. + + - "fan_div": fan clock frequency divisor value. Supported values are 1, + 2, 4 and 8. Default is 1. + + - "fan_target": initial target fan speed in RPM. By default, current + value is not modified. Only works in closed-loop fan speed + control, i.e. when pwm_enable has already been set to 2. + + - "fan_startv": fan startup voltage. Accepted values are 0, 1, 2 and 3. + Default value is 1. The higher the more. + + - "pwm_polarity": pwm polarity. Accepted values are 0 (positive duty) + and 1 (negative duty). Default is 0 i.e. positive duty. + + - "fan_gear_mode": fan gear mode. Supported values are 0, 1 and 2. + Default is 0. + + +Additional information on operational parameters for the device is available +in Documentation/hwmon/g762. A detailed datasheet for the device is available +at http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf. + +Example g762 node: + + g762: g762 at 3e { + compatible = "gmt,g762"; + reg = <0x3e>; + pwm_mode = <1>; /* closed-loop control */ + pwm_enable = <2>; /* PWM mode */ + pwm_freq = <8192>; /* PWM reference clock freq */ + fan_pulses = <2>; /* 2 pulses per rev */ + fan_div = <2>; /* fan clock divisor */ + fan_target = <2000>; /* target fan speed at 2000 RPM */ + fan_gear_mode = <0>; /* default */ + fan_startv = <1>; /* default */ + pwm_polarity = <0>; /* default */ + }; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCHv1 3/3] hwmon: Add DT documentation for g762 driver 2013-04-23 22:06 ` [PATCHv1 3/3] hwmon: Add DT " Arnaud Ebalard @ 2013-04-23 22:23 ` Jason Cooper 2013-04-24 5:43 ` Arnaud Ebalard 0 siblings, 1 reply; 34+ messages in thread From: Jason Cooper @ 2013-04-23 22:23 UTC (permalink / raw) To: linux-arm-kernel On Wed, Apr 24, 2013 at 12:06:20AM +0200, Arnaud Ebalard wrote: > > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > --- > Documentation/devicetree/bindings/hwmon/g762.txt | 57 ++++++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/g762.txt > > diff --git a/Documentation/devicetree/bindings/hwmon/g762.txt b/Documentation/devicetree/bindings/hwmon/g762.txt > new file mode 100644 > index 0000000..87d7cf5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/g762.txt > @@ -0,0 +1,57 @@ > +GMT G762/G763 PWM Fan controller > + > +Required node properties: > + > + - "compatible": must be either "gmt,g762" or "gmt,g763" > + - "reg": I2C bus address of the device > + > +Optional properties: > + > + - "pwm_mode": fan driving mode. 1 for PWM mode, 0 for linear. Default > + value is 0, i.e. linear. > + > + - "pwm_enable": fan speed control. 1 for open-loop, 2 for closed-loop. > + Default value is 2, i.e. closed-loop. > + > + - "pwm_freq": reference clock frequency for PWM mode in Hz. Default is > + 32768. > + > + - "fan_pulses": number of pulses per fan revolution. Supported values > + are 2 and 4. Default is 2. > + > + - "fan_div": fan clock frequency divisor value. Supported values are 1, > + 2, 4 and 8. Default is 1. > + > + - "fan_target": initial target fan speed in RPM. By default, current > + value is not modified. Only works in closed-loop fan speed > + control, i.e. when pwm_enable has already been set to 2. > + > + - "fan_startv": fan startup voltage. Accepted values are 0, 1, 2 and 3. > + Default value is 1. The higher the more. > + > + - "pwm_polarity": pwm polarity. Accepted values are 0 (positive duty) > + and 1 (negative duty). Default is 0 i.e. positive duty. > + > + - "fan_gear_mode": fan gear mode. Supported values are 0, 1 and 2. > + Default is 0. > + > + > +Additional information on operational parameters for the device is available > +in Documentation/hwmon/g762. A detailed datasheet for the device is available > +at http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf. > + > +Example g762 node: > + > + g762: g762 at 3e { > + compatible = "gmt,g762"; > + reg = <0x3e>; > + pwm_mode = <1>; /* closed-loop control */ > + pwm_enable = <2>; /* PWM mode */ > + pwm_freq = <8192>; /* PWM reference clock freq */ > + fan_pulses = <2>; /* 2 pulses per rev */ > + fan_div = <2>; /* fan clock divisor */ > + fan_target = <2000>; /* target fan speed at 2000 RPM */ > + fan_gear_mode = <0>; /* default */ > + fan_startv = <1>; /* default */ > + pwm_polarity = <0>; /* default */ > + }; I was just giving this quick overview and noticed that you have leading whitespace issues in the above block... thx, Jason, ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCHv1 3/3] hwmon: Add DT documentation for g762 driver 2013-04-23 22:23 ` Jason Cooper @ 2013-04-24 5:43 ` Arnaud Ebalard 0 siblings, 0 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-24 5:43 UTC (permalink / raw) To: linux-arm-kernel Hi Jason, Jason Cooper <jason@lakedaemon.net> writes: >> +Example g762 node: >> + >> + g762: g762 at 3e { >> + compatible = "gmt,g762"; >> + reg = <0x3e>; >> + pwm_mode = <1>; /* closed-loop control */ >> + pwm_enable = <2>; /* PWM mode */ >> + pwm_freq = <8192>; /* PWM reference clock freq */ >> + fan_pulses = <2>; /* 2 pulses per rev */ >> + fan_div = <2>; /* fan clock divisor */ >> + fan_target = <2000>; /* target fan speed at 2000 RPM */ >> + fan_gear_mode = <0>; /* default */ >> + fan_startv = <1>; /* default */ >> + pwm_polarity = <0>; /* default */ >> + }; > > I was just giving this quick overview and noticed that you have > leading whitespace issues in the above block... Will fix that in next round. Sorry for that and thanks for your time. Cheers, a+ ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller 2013-04-18 22:28 ` [PATCH 1/3] Add support for GMT G72/G763 " Arnaud Ebalard 2013-04-19 4:35 ` Guenter Roeck @ 2013-04-19 5:50 ` Andrew Lunn 2013-04-19 11:30 ` Arnaud Ebalard 2013-04-19 6:05 ` Jean Delvare 2 siblings, 1 reply; 34+ messages in thread From: Andrew Lunn @ 2013-04-19 5:50 UTC (permalink / raw) To: linux-arm-kernel Hi Arnaud > +static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, get_pwm, set_pwm); > +static DEVICE_ATTR(pwm1_polarity, S_IWUSR | S_IRUGO, > + get_pwm_polarity, set_pwm_polarity); > +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_speed_control_mode, set_speed_control_mode); > +static DEVICE_ATTR(pwm1_freq, S_IWUSR | S_IRUGO, > + get_pwm_freq, set_pwm_freq); > + > +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_alarm_detection, S_IWUSR | S_IRUGO, > + get_fan_ooc_detection, set_fan_ooc_detection); > +static DEVICE_ATTR(fan1_fault, S_IRUGO, get_fan_failure, NULL); > +static DEVICE_ATTR(fan1_fault_detection, S_IWUSR | S_IRUGO, > + get_fan_failure_detection, set_fan_failure_detection); > +static DEVICE_ATTR(fan1_target, S_IWUSR | S_IRUGO, > + get_fan_target, set_fan_target); > +static DEVICE_ATTR(fan1_pulses, S_IWUSR | S_IRUGO, > + get_fan_pulses, set_fan_pulses); > +static DEVICE_ATTR(fan1_div, S_IWUSR | S_IRUGO, > + get_fan_clk_div, set_fan_clk_div); > +static DEVICE_ATTR(fan1_gear_mode, S_IWUSR | S_IRUGO, > + get_fan_gear_mode, set_fan_gear_mode); > +static DEVICE_ATTR(fan1_startup_voltage, S_IWUSR | S_IRUGO, > + get_fan_startup_voltage, set_fan_startup_voltage); It is normal to use SENSOR_ATTR, not DEVICE_ATTR. Take a look at the existing fan drivers. I also think a lot of these are not needed. They are fixed properties of the board and cannot change dynamically. They are set once using DT and user space does not need to care. Andrew ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller 2013-04-19 5:50 ` [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller Andrew Lunn @ 2013-04-19 11:30 ` Arnaud Ebalard 2013-04-19 13:37 ` Guenter Roeck 0 siblings, 1 reply; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-19 11:30 UTC (permalink / raw) To: linux-arm-kernel Hi, Andrew Lunn <andrew@lunn.ch> writes: > Hi Arnaud > >> +static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, get_pwm, set_pwm); >> +static DEVICE_ATTR(pwm1_polarity, S_IWUSR | S_IRUGO, >> + get_pwm_polarity, set_pwm_polarity); >> +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_speed_control_mode, set_speed_control_mode); >> +static DEVICE_ATTR(pwm1_freq, S_IWUSR | S_IRUGO, >> + get_pwm_freq, set_pwm_freq); >> + >> +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_alarm_detection, S_IWUSR | S_IRUGO, >> + get_fan_ooc_detection, set_fan_ooc_detection); >> +static DEVICE_ATTR(fan1_fault, S_IRUGO, get_fan_failure, NULL); >> +static DEVICE_ATTR(fan1_fault_detection, S_IWUSR | S_IRUGO, >> + get_fan_failure_detection, set_fan_failure_detection); >> +static DEVICE_ATTR(fan1_target, S_IWUSR | S_IRUGO, >> + get_fan_target, set_fan_target); >> +static DEVICE_ATTR(fan1_pulses, S_IWUSR | S_IRUGO, >> + get_fan_pulses, set_fan_pulses); >> +static DEVICE_ATTR(fan1_div, S_IWUSR | S_IRUGO, >> + get_fan_clk_div, set_fan_clk_div); >> +static DEVICE_ATTR(fan1_gear_mode, S_IWUSR | S_IRUGO, >> + get_fan_gear_mode, set_fan_gear_mode); >> +static DEVICE_ATTR(fan1_startup_voltage, S_IWUSR | S_IRUGO, >> + get_fan_startup_voltage, set_fan_startup_voltage); > > It is normal to use SENSOR_ATTR, not DEVICE_ATTR. Take a look at the > existing fan drivers. I will take a look and use SENSOR_ATTR if it is the expected way. For the records, the g760a.c which I used as basis uses DEVICE_ATTR ;-) > I also think a lot of these are not needed. They are fixed properties > of the board and cannot change dynamically. They are set once using DT > and user space does not need to care. I added those knobs for a simple reason: I don't have all the various characteristics of the target platform I am working on (Netgear ReadyNAS Duo v2) and needed to be able to test various values w/o rebooting to get those rights. I thought this knobs would be useful for people with the same issue (for instance, the new ReadyNAS 102 also has a G762) and would not hurt. Anyway, I will split the patch in two parts and keep the useless knobs on my side. Thanks for your feedback, Cheers, a+ ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller 2013-04-19 11:30 ` Arnaud Ebalard @ 2013-04-19 13:37 ` Guenter Roeck 0 siblings, 0 replies; 34+ messages in thread From: Guenter Roeck @ 2013-04-19 13:37 UTC (permalink / raw) To: linux-arm-kernel On Fri, Apr 19, 2013 at 01:30:51PM +0200, Arnaud Ebalard wrote: > Hi, > > Andrew Lunn <andrew@lunn.ch> writes: > > > Hi Arnaud > > > >> +static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, get_pwm, set_pwm); > >> +static DEVICE_ATTR(pwm1_polarity, S_IWUSR | S_IRUGO, > >> + get_pwm_polarity, set_pwm_polarity); > >> +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_speed_control_mode, set_speed_control_mode); > >> +static DEVICE_ATTR(pwm1_freq, S_IWUSR | S_IRUGO, > >> + get_pwm_freq, set_pwm_freq); > >> + > >> +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_alarm_detection, S_IWUSR | S_IRUGO, > >> + get_fan_ooc_detection, set_fan_ooc_detection); > >> +static DEVICE_ATTR(fan1_fault, S_IRUGO, get_fan_failure, NULL); > >> +static DEVICE_ATTR(fan1_fault_detection, S_IWUSR | S_IRUGO, > >> + get_fan_failure_detection, set_fan_failure_detection); > >> +static DEVICE_ATTR(fan1_target, S_IWUSR | S_IRUGO, > >> + get_fan_target, set_fan_target); > >> +static DEVICE_ATTR(fan1_pulses, S_IWUSR | S_IRUGO, > >> + get_fan_pulses, set_fan_pulses); > >> +static DEVICE_ATTR(fan1_div, S_IWUSR | S_IRUGO, > >> + get_fan_clk_div, set_fan_clk_div); > >> +static DEVICE_ATTR(fan1_gear_mode, S_IWUSR | S_IRUGO, > >> + get_fan_gear_mode, set_fan_gear_mode); > >> +static DEVICE_ATTR(fan1_startup_voltage, S_IWUSR | S_IRUGO, > >> + get_fan_startup_voltage, set_fan_startup_voltage); > > > > It is normal to use SENSOR_ATTR, not DEVICE_ATTR. Take a look at the > > existing fan drivers. > > I will take a look and use SENSOR_ATTR if it is the expected way. For > the records, the g760a.c which I used as basis uses DEVICE_ATTR ;-) > You only need SENSOR_ATTR if you have an index parameter. Otherwise DEVICE_ATTR is just fine. I would actually reject the patch if I notice that it uses SENSOR_ATTR without need for a parameter. Besides, in the context used, it would probably be SENSOR_DEVICE_ATTR. > > I also think a lot of these are not needed. They are fixed properties > > of the board and cannot change dynamically. They are set once using DT > > and user space does not need to care. > > I added those knobs for a simple reason: I don't have all the various > characteristics of the target platform I am working on (Netgear > ReadyNAS Duo v2) and needed to be able to test various values w/o > rebooting to get those rights. I thought this knobs would be useful for > people with the same issue (for instance, the new ReadyNAS 102 also has > a G762) and would not hurt. > Testing is not a reason to add non-standard attributes. Thanks, Guenter > Anyway, I will split the patch in two parts and keep the useless knobs > on my side. > > Thanks for your feedback, > > Cheers, > > a+ > ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller 2013-04-18 22:28 ` [PATCH 1/3] Add support for GMT G72/G763 " Arnaud Ebalard 2013-04-19 4:35 ` Guenter Roeck 2013-04-19 5:50 ` [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller Andrew Lunn @ 2013-04-19 6:05 ` Jean Delvare 2013-04-19 11:31 ` Arnaud Ebalard 2 siblings, 1 reply; 34+ messages in thread From: Jean Delvare @ 2013-04-19 6:05 UTC (permalink / raw) To: linux-arm-kernel Hi Arnaud, Just a few things I noticed... Typo in subject line: G762, not G72. On Fri, 19 Apr 2013 00:28:21 +0200, Arnaud Ebalard wrote: > > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > Tested-by: Arnaud Ebalard <arno@natisbad.org> > --- > drivers/hwmon/Kconfig | 10 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/g762.c | 1159 ++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 1170 insertions(+) > create mode 100644 drivers/hwmon/g762.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 89ac1cb..0c6ddee 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" > + depends on I2C > + help > + If you say yes here you get support for Global Mixed-mode > + Technology Inc G762 fan speed PWM controller chips. If your driver also supports the G763 it should be mentioned here. > + > + This driver can also be built as a module. If so, the module > + will be called g762a. This is not how your driver is actually named. > + > 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..8c4cb39 > --- /dev/null > +++ b/drivers/hwmon/g762.c > (...) > +#define DRVNAME "g762" > + > +static const struct i2c_device_id g762_id[] = { > + { DRVNAME, 0 }, No, this is a list of device names, nor driver names, so the use of DRVNAME is inappropriate. Also, again, if your driver supports the g763 it should be listed here too. > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, g762_id); -- Jean Delvare ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller 2013-04-19 6:05 ` Jean Delvare @ 2013-04-19 11:31 ` Arnaud Ebalard 0 siblings, 0 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-19 11:31 UTC (permalink / raw) To: linux-arm-kernel Hi, Jean Delvare <khali@linux-fr.org> writes: > Just a few things I noticed... Ack. Will be fixed in next round. Thanks Cheers, a+ ^ permalink raw reply [flat|nested] 34+ messages in thread
* [RFC,PATCHv0 2/3] Add DT documentation for G762 PWM fan controller 2013-04-18 22:27 [RFC,PATCHv0 0/3] Add support for GMT G762/G763 PWM fan controller Arnaud Ebalard 2013-04-18 22:28 ` [PATCH 1/3] Add support for GMT G72/G763 " Arnaud Ebalard @ 2013-04-18 22:28 ` Arnaud Ebalard 2013-04-18 22:28 ` [PATCH 3/3] Add documentation for g762 driver Arnaud Ebalard 2 siblings, 0 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-18 22:28 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Arnaud Ebalard <arno@natisbad.org> --- Documentation/devicetree/bindings/hwmon/g762.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/g762.txt diff --git a/Documentation/devicetree/bindings/hwmon/g762.txt b/Documentation/devicetree/bindings/hwmon/g762.txt new file mode 100644 index 0000000..9eee838 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/g762.txt @@ -0,0 +1,37 @@ +GMT G762/G763 PWM Fan controller + +Required node properties: + + - "compatible": must be either "gmt,g762" or "gmt,g763" + - "reg": I2C bus address of the device + +Optional properties: + + - "pwm_mode": fan driving mode. 1 for PWM mode, 0 for linear. Default + value is 0, i.e. linear. + - "pwm_enable": fan speed control. 1 for open-loop, 2 for closed-loop. + Default value is 2, i.e. closed-loop. + - "pwm_freq": reference clock frequency for PWM mode in Hz. Default is + 32768. + - "fan_pulses": number of pulses per fan revolution. Supported values + are 2 and 4. Default is 2. + - "fan_div": fan clock frequency divisor value. Supported values are + 1, 2, 4 and 8. Default is 1. + - "fan_target": initial target fan speed in RPM. By default, current + value is not modified. + +Previous optional properties provides devicetree access to the entries +described in Documentation/hwmon/g762 + +Example g762 node: + + g762: g762 at 3e { + compatible = "gmt,g762"; + reg = <0x3e>; + pwm_mode = <1>; /* closed-loop control */ + pwm_enable = <2>; /* PWM mode */ + pwm_freq = <8192>; /* PWM reference clock freq */ + fan_pulses = <2>; /* 2 pulses per rev */ + fan_div = <2>; /* fan clock divisor */ + fan_target = <2000>; /* target fan speed at 2000 RPM */ + }; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH 3/3] Add documentation for g762 driver 2013-04-18 22:27 [RFC,PATCHv0 0/3] Add support for GMT G762/G763 PWM fan controller Arnaud Ebalard 2013-04-18 22:28 ` [PATCH 1/3] Add support for GMT G72/G763 " Arnaud Ebalard 2013-04-18 22:28 ` [RFC,PATCHv0 2/3] Add DT documentation for G762 " Arnaud Ebalard @ 2013-04-18 22:28 ` Arnaud Ebalard 2 siblings, 0 replies; 34+ messages in thread From: Arnaud Ebalard @ 2013-04-18 22:28 UTC (permalink / raw) To: linux-arm-kernel Signed-off-by: Arnaud Ebalard <arno@natisbad.org> --- Documentation/hwmon/g762 | 89 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 Documentation/hwmon/g762 diff --git a/Documentation/hwmon/g762 b/Documentation/hwmon/g762 new file mode 100644 index 0000000..f281b3d --- /dev/null +++ b/Documentation/hwmon/g762 @@ -0,0 +1,89 @@ +Kernel driver g762 +================== + +This documentation is based on G760a one, GMT G762 datasheet available +at http://natisbad.org/NAS/ref/GMT_EDS-762_763-080710-0.2.pdf (used +for the development of the driver) and sysfs bindings described in +Documentation/hwmon/sysfs-interface. + +The GMT G762 Fan Speed PWM Controller is connected directly to a fan +and performs closed-loop or open-loop control of the fan speed. Two +modes - PWM or linear - are supported by the device. + +The following entries are available to the user in a subdirectory of +/sys/bus/i2c/drivers/g762/ to control the operation of the device. +Some of those can be used to fit operations of the controller to the +characteristics of the system (reference clock value, fan gear mode, +number of pulses per fan revolution, ...). Other more common +parameters can be used to interact with the fan and control its speed +(set a rotation speed and get current speed for instance). This can be +done manually using the following entries but is usually done via a +userland daemon like fancontrol. + + + fan1_target: set desired fan speed. This only makes sense in closed-loop + fan speed control (i.e. when pwm1_enable is set to 2). + + fan1_input: provide current fan rotation value in RPM as reported by + the fan to the device. + + fan1_pulses: number of tachometer pulses per fan revolution. Supported + values are 2 and 4. Default value is 2. + + fan1_div: fan clock divisor. Supported value are 1, 2, 4 and 8. Default + value is 1. + + fan1_gear_mode: fan gear mode. Supported values are 0, 1 and 2. Default + value is 0. The value is a characteristic of the system + and a higher value can be set for more precisely control + fans with a high rotation speed. Note that this affects the + measurable speed range, not the read value. + + fan1_fault: reports fan failure, i.e. no transition on fan gear pin for + about 0.7s (if the fan is not voluntarily set off). + + fan1_fault_detection: enable fan failure detection and reporting as + described above. + + fan1_alarm: in closed-loop control mode, if fan RPM value is 25% out + of the programmed value for over 6 seconds 'fan1_alarm' is + set to 1. + + fan1_alarm_detection: enable fan out of control detection and reporting + as described above. + + fan1_startup_voltage: get or set fan startup voltage. Supported values + are 0, 1, 2, 3 (respectively for 1.0V, 1.5V, 2.0V and 2.5V). + + + + pwm1: get or set PWM fan control value. This is an integer value + between 0 and 255. 0 stops the fan, 255 makes it run at + full speed. + + pwm1_polarity: set or get the PWM polarity, 0 for positive duty, 1 for + negative duty + + pwm1_enable: set current fan speed control mode i.e. 1 for manual fan + speed control (open-loop), 2 for automatic fan speed control + (closed-loop). + + pwm1_mode: set or get fan driving mode: 1 for PWM mode, 0 for linear + mode. Default is linear mode. + + pwm1_freq: set or get value of PWM reference clock frequency. Default + value is 32768 (Hz). This parameter only makes sense when + the device is set to operate in PWM mode (i.e. pwm1_mode + is 1). + + +In PWM mode ('pwm1_mode' set to 1), the fan speed is programmed either by +setting a value between 0 and 255 via 'pwm1' entry (0 stops the fan, 255 +makes it run at full speed). This can also be done by passing the +expected RPM value via 'fan1_target'. Current fan speed value can be +retrieved via 'fan1_input'. This fan speed value is computed based on +the parameters associated with the physical characteristics of the +system: a reference clock source frequency, a number of pulses per fan +revolution, etc. + +Note that the driver will update its values at most once per second. -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
end of thread, other threads:[~2013-04-27 18:55 UTC | newest] Thread overview: 34+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-18 22:27 [RFC,PATCHv0 0/3] Add support for GMT G762/G763 PWM fan controller Arnaud Ebalard 2013-04-18 22:28 ` [PATCH 1/3] Add support for GMT G72/G763 " Arnaud Ebalard 2013-04-19 4:35 ` Guenter Roeck 2013-04-19 5:34 ` Arnaud Ebalard 2013-04-23 22:05 ` [PATCHv1 0/3] hwmon: " Arnaud Ebalard 2013-04-23 22:05 ` [PATCHv1 1/3] hwmon: Add support for GMT G762/G763 " Arnaud Ebalard 2013-04-24 5:37 ` Andrew Lunn 2013-04-24 9:06 ` Arnaud Ebalard 2013-04-24 10:04 ` Simon Guinot 2013-04-24 10:50 ` Arnaud Ebalard 2013-04-24 13:38 ` Guenter Roeck 2013-04-24 20:28 ` Arnaud Ebalard 2013-04-24 22:47 ` Guenter Roeck 2013-04-25 10:14 ` Simon Guinot 2013-04-24 17:06 ` Simon Guinot 2013-04-24 23:37 ` Guenter Roeck 2013-04-25 9:58 ` Simon Guinot 2013-04-27 14:03 ` Simon Guinot 2013-04-27 14:12 ` Jean Delvare 2013-04-27 16:56 ` Guenter Roeck 2013-04-27 18:55 ` Arnaud Ebalard 2013-04-23 22:06 ` [PATCHv1 2/3] hwmon: Add documentation for g762 driver Arnaud Ebalard 2013-04-24 17:32 ` Guenter Roeck 2013-04-24 20:33 ` Arnaud Ebalard 2013-04-23 22:06 ` [PATCHv1 3/3] hwmon: Add DT " Arnaud Ebalard 2013-04-23 22:23 ` Jason Cooper 2013-04-24 5:43 ` Arnaud Ebalard 2013-04-19 5:50 ` [PATCH 1/3] Add support for GMT G72/G763 PWM fan controller Andrew Lunn 2013-04-19 11:30 ` Arnaud Ebalard 2013-04-19 13:37 ` Guenter Roeck 2013-04-19 6:05 ` Jean Delvare 2013-04-19 11:31 ` Arnaud Ebalard 2013-04-18 22:28 ` [RFC,PATCHv0 2/3] Add DT documentation for G762 " Arnaud Ebalard 2013-04-18 22:28 ` [PATCH 3/3] Add documentation for g762 driver Arnaud Ebalard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).