* [PATCH v2 0/4] pmbus: Expand fan support and add MAX31785 driver
@ 2017-08-02 7:15 Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation Andrew Jeffery
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Andrew Jeffery @ 2017-08-02 7:15 UTC (permalink / raw)
To: linux, linux-hwmon
Cc: Andrew Jeffery, robh+dt, mark.rutland, jdelvare, devicetree,
linux-kernel, joel, mspinler, msbarth, openbmc
Hello,
v1[1] spent some time in the OpenBMC kernel tree and it shook out a few issues:
1. The machines I was testing against had pre-prammed the installed-bit in
FAN_CONFIG_1_2
2. There appears to be a hardware issue with some back-to-back writes to the
MAX31785
Point 1. is a policy issue so we should have the ability to specify behaviour
in the devicetree. As such the bindings documentation has been updated to suit.
Point 2, well, it's a little ugly. We're in contact with Maxim to better
understand the nature of the issue. Patch 4 introduces a work-around. I've kept
this separate from the introduction of the driver so it's implementation and
the demonstration of new pmbus core features isn't obscured. The work-around
has proved reliable so-far, across a number of machines.
[1] https://lkml.org/lkml/2017/7/27/116
Changes from v1 to v2:
* Add the use-stored-presence devicetree property describing the hardware as
canonical in terms of fan presence.
* Small optimsation in pmbus_update_fans(): Don't write FAN_CONFIG_x_y unless
the new value is different to the current value
* Update the MAX31785 driver to consume use-stored-presence from the devicetree
* Add the back-to-back write work-around
Changes between RFC v2 to (non-RFC) v1
* Clean up issues identified in comments on the pmbus core changes
* Define and implement bindings for the MAX31785
* Clean up issues identified in comments on the max31785 driver
Andrew Jeffery (4):
dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation
hwmon: pmbus: Add fan control support
pmbus: Add driver for Maxim MAX31785 Intelligent Fan Controller
pmbus: max31785: Work around back-to-back writes with FAN_CONFIG_1_2
.../devicetree/bindings/hwmon/pmbus/max31785.txt | 126 ++++
drivers/hwmon/pmbus/Kconfig | 10 +
drivers/hwmon/pmbus/Makefile | 1 +
drivers/hwmon/pmbus/max31785.c | 762 +++++++++++++++++++++
drivers/hwmon/pmbus/pmbus.h | 29 +
drivers/hwmon/pmbus/pmbus_core.c | 224 +++++-
6 files changed, 1137 insertions(+), 15 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
create mode 100644 drivers/hwmon/pmbus/max31785.c
--
2.11.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation
2017-08-02 7:15 [PATCH v2 0/4] pmbus: Expand fan support and add MAX31785 driver Andrew Jeffery
@ 2017-08-02 7:15 ` Andrew Jeffery
2017-08-10 16:13 ` Rob Herring
2017-08-02 7:15 ` [PATCH v2 2/4] hwmon: pmbus: Add fan control support Andrew Jeffery
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Andrew Jeffery @ 2017-08-02 7:15 UTC (permalink / raw)
To: linux, linux-hwmon
Cc: Andrew Jeffery, robh+dt, mark.rutland, jdelvare, devicetree,
linux-kernel, joel, mspinler, msbarth, openbmc
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
.../devicetree/bindings/hwmon/pmbus/max31785.txt | 126 +++++++++++++++++++++
1 file changed, 126 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
new file mode 100644
index 000000000000..8ddc4ea1958d
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
@@ -0,0 +1,126 @@
+Bindings for the Maxim MAX31785 Intelligent Fan Controller
+==========================================================
+
+Reference:
+[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
+
+Required properties:
+- compatible : One of "maxim,max31785" or "maxim,max31785a"
+- reg : I2C address, one of 0x52, 0x53, 0x54, 0x55.
+- #address-cells : Must be 1
+- #size-cells : Must be 0
+
+Optional properties:
+- use-stored-presence : Do not treat the devicetree description as canon for
+ fan presence (the 'installed' bit of FAN_CONFIG_*).
+ Instead, rely on the on the default value store of
+ the device to populate it.
+
+Capabilities are configured through subnodes of the controller's node.
+
+Fans
+----
+
+Only fans with subnodes present will be considered as installed. If
+use-stored-presence is present in the parent node, then only fans that are both
+defined in the devicetree and have their installed bit set are considered
+installed.
+
+Required subnode properties:
+- compatible : Must be "pmbus-fan"
+- reg : The PMBus page the properties apply to.
+- maxim,fan-rotor-input : The type of rotor measurement provided to the
+ controller. Must be either "tach" for tachometer
+ pulses or "lock" for a locked-rotor signal.
+- maxim,fan-lock-polarity: Required iff maxim,fan-rotor-input is "lock". Valid
+ values are "low" for active low, "high" for active
+ high.
+
+Optional subnode properties:
+- fan-mode : "rpm" or "pwm". Default value is "pwm".
+- tach-pulses : Tachometer pulses per revolution. Valid values are
+ 1, 2, 3 or 4. The default is 1.
+- maxim,fan-no-fault-ramp: Do not ramp the fan to 100% PWM duty on detecting a
+ fan fault
+- maxim,fan-startup : The number of rotations required before taking
+ emergency action for an unresponsive fan and driving
+ it with 100% or 0% PWM duty, depending on the state
+ of maxim,fan-no-fault-ramp. Valid values are 0
+ (automatic spin-up disabled), 2, 4, or 8. Default
+ value is 0.
+- maxim,fan-health : Enable automated fan health check
+- maxim,fan-ramp : Configures how fast the device ramps the PWM duty
+ cycle from one value to another. Valid values are 0
+ to 7 inclusive, with values 0 - 2 configuring a
+ 1000ms update rate and 1 - 3% duty respective duty
+ increase, and 3 - 7 a 200ms update rate with a 1 -
+ 5% respective duty increase. Default value is 0.
+- maxim,fan-no-watchdog : Do not ramp fan to 100% PWM duty on failure to
+ update desired fan rate inside 10s. This implies
+ maxim,tmp-no-fault-ramp
+- maxim,tmp-no-fault-ramp: Do not ramp fan to 100% PWM duty on temperature
+ sensor fault detection. This implies
+ maxim,fan-no-watchdog
+- maxim,tmp-hysteresis : The temperature hysteresis used to determine
+ transitions to lower fan speed bands in the
+ temperature/fan rate lookup table. Valid values are
+ 2, 4, 6 or 8 (degrees celcius). Default value is 2.
+- maxim,fan-dual-tach : Enable dual tachometer functionality
+- maxim,fan-pwm-freq : The PWM frequency. Valid values are 30, 50, 100, 150
+ and 25000 (Hz). Default value is 30Hz.
+- maxim,fan-lookup-table : A 16-element cell array of alternating temperature
+ and rate values representing the look up table. The
+ rate units are set through the fan-mode property.
+- maxim,fan-fault-pin-mon: Ramp fans to 100% PWM duty when the FAULT pin is
+ asserted
+
+Temperature
+-----------
+
+Required subnode properties:
+- compatible : Must be "pmbus-temperature"
+- reg : The PMBus page the properties apply to.
+
+Optional subnode properties:
+- maxim,tmp-offset : Valid values are 0 - 30 (degrees celcius) inclusive.
+ Default value is 0.
+- maxim,tmp-fans : An array of fan indexes whose fans are controlled by
+ the current temperature sensor. Fans are indexed from
+ zero. The valid values are 0 - 5 inclusive.
+
+Example:
+ fan-max31785: max31785@52 {
+ reg = <0x52>;
+ compatible = "maxim,max31785";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ fan@0 {
+ compatible = "pmbus-fan";
+ reg = <0>;
+ mode = "rpm";
+ tach-pulses = <1>;
+ maxim,fan-rotor-input = "tach";
+ maxim,fan-dual-tach;
+ };
+
+ fan@1 {
+ compatible = "pmbus-fan";
+ reg = <1>;
+ mode = "rpm";
+ tach-pulses = <1>;
+ maxim,fan-rotor-input = "tach";
+ maxim,tmp-hysteresis = <2>;
+ maxim,fan-lookup-table = <
+ /* Temperature RPM */
+ 0 1000
+ 10 2000
+ 20 3000
+ 30 4000
+ 40 5000
+ 50 6000
+ 60 7000
+ 70 8000
+ >;
+ };
+ };
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/4] hwmon: pmbus: Add fan control support
2017-08-02 7:15 [PATCH v2 0/4] pmbus: Expand fan support and add MAX31785 driver Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation Andrew Jeffery
@ 2017-08-02 7:15 ` Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 3/4] pmbus: Add driver for Maxim MAX31785 Intelligent Fan Controller Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 4/4] pmbus: max31785: Work around back-to-back writes with FAN_CONFIG_1_2 Andrew Jeffery
3 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2017-08-02 7:15 UTC (permalink / raw)
To: linux, linux-hwmon
Cc: Andrew Jeffery, robh+dt, mark.rutland, jdelvare, devicetree,
linux-kernel, joel, mspinler, msbarth, openbmc
Expose fanX_target, pwmX and pwmX_enable hwmon sysfs attributes.
Fans in a PMBus device are driven by the configuration of two registers:
FAN_CONFIG_x_y and FAN_COMMAND_x: FAN_CONFIG_x_y dictates how the fan
and the tacho operate (if installed), while FAN_COMMAND_x sets the
desired fan rate. The unit of FAN_COMMAND_x is dependent on the
operational fan mode, RPM or PWM percent duty, as determined by the
corresponding FAN_CONFIG_x_y.
The mapping of fanX_target, pwmX and pwmX_enable onto FAN_CONFIG_x_y and
FAN_COMMAND_x is implemented with the addition of virtual registers and
generic implementations in the core:
1. PMBUS_VIRT_FAN_TARGET_x
2. PMBUS_VIRT_PWM_x
3. PMBUS_VIRT_PWM_ENABLE_x
The virtual registers facilitate the necessary side-effects of each
access. Examples of the read case, assuming m = 1, b = 0, R = 0:
Read | With || Gives
-------------------------------------------------------
Attribute | FAN_CONFIG_x_y | FAN_COMMAND_y || Value
----------------------------------------------++-------
fanX_target | ~PB_FAN_z_RPM | 0x0001 || 1
pwm1 | ~PB_FAN_z_RPM | 0x0064 || 255
pwmX_enable | ~PB_FAN_z_RPM | 0x0001 || 1
fanX_target | PB_FAN_z_RPM | 0x0001 || 1
pwm1 | PB_FAN_z_RPM | 0x0064 || 0
pwmX_enable | PB_FAN_z_RPM | 0x0001 || 1
And the write case:
Write | With || Sets
-------------+-------++----------------+---------------
Attribute | Value || FAN_CONFIG_x_y | FAN_COMMAND_x
-------------+-------++----------------+---------------
fanX_target | 1 || PB_FAN_z_RPM | 0x0001
pwmX | 255 || ~PB_FAN_z_RPM | 0x0064
pwmX_enable | 1 || ~PB_FAN_z_RPM | 0x0064
Also, the DIRECT mode scaling of some controllers is different between
RPM and PWM percent duty control modes, so PSC_PWM is introduced to
capture the necessary coefficients.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/pmbus/pmbus.h | 29 +++++
drivers/hwmon/pmbus/pmbus_core.c | 224 ++++++++++++++++++++++++++++++++++++---
2 files changed, 238 insertions(+), 15 deletions(-)
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index bfcb13bae34b..a863b8fed16f 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -190,6 +190,28 @@ enum pmbus_regs {
PMBUS_VIRT_VMON_UV_FAULT_LIMIT,
PMBUS_VIRT_VMON_OV_FAULT_LIMIT,
PMBUS_VIRT_STATUS_VMON,
+
+ /*
+ * RPM and PWM Fan control
+ *
+ * Drivers wanting to expose PWM control must define the behaviour of
+ * PMBUS_VIRT_PWM_ENABLE_[1-4] in the {read,write}_word_data callback.
+ *
+ * pmbus core provides default implementations for
+ * PMBUS_VIRT_FAN_TARGET_[1-4] and PMBUS_VIRT_PWM_[1-4].
+ */
+ PMBUS_VIRT_FAN_TARGET_1,
+ PMBUS_VIRT_FAN_TARGET_2,
+ PMBUS_VIRT_FAN_TARGET_3,
+ PMBUS_VIRT_FAN_TARGET_4,
+ PMBUS_VIRT_PWM_1,
+ PMBUS_VIRT_PWM_2,
+ PMBUS_VIRT_PWM_3,
+ PMBUS_VIRT_PWM_4,
+ PMBUS_VIRT_PWM_ENABLE_1,
+ PMBUS_VIRT_PWM_ENABLE_2,
+ PMBUS_VIRT_PWM_ENABLE_3,
+ PMBUS_VIRT_PWM_ENABLE_4,
};
/*
@@ -223,6 +245,8 @@ enum pmbus_regs {
#define PB_FAN_1_RPM BIT(6)
#define PB_FAN_1_INSTALLED BIT(7)
+enum pmbus_fan_mode { percent = 0, rpm };
+
/*
* STATUS_BYTE, STATUS_WORD (lower)
*/
@@ -313,6 +337,7 @@ enum pmbus_sensor_classes {
PSC_POWER,
PSC_TEMPERATURE,
PSC_FAN,
+ PSC_PWM,
PSC_NUM_CLASSES /* Number of power sensor classes */
};
@@ -339,6 +364,8 @@ enum pmbus_sensor_classes {
#define PMBUS_HAVE_STATUS_FAN34 BIT(17)
#define PMBUS_HAVE_VMON BIT(18)
#define PMBUS_HAVE_STATUS_VMON BIT(19)
+#define PMBUS_HAVE_PWM12 BIT(20)
+#define PMBUS_HAVE_PWM34 BIT(21)
enum pmbus_data_format { linear = 0, direct, vid };
enum vrm_version { vr11 = 0, vr12 };
@@ -413,6 +440,8 @@ int pmbus_write_byte_data(struct i2c_client *client, int page, u8 reg,
u8 value);
int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,
u8 mask, u8 value);
+int pmbus_update_fan(struct i2c_client *client, int page, int id,
+ u8 config, u8 mask, u16 command);
void pmbus_clear_faults(struct i2c_client *client);
bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index ba59eaef2e07..93fb58787b99 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -63,6 +63,7 @@ struct pmbus_sensor {
u16 reg; /* register */
enum pmbus_sensor_classes class; /* sensor class */
bool update; /* runtime sensor update needed */
+ bool convert; /* Whether or not to apply linear/vid/direct */
int data; /* Sensor data.
Negative if there was a read error */
};
@@ -118,6 +119,27 @@ struct pmbus_data {
u8 currpage;
};
+static const int pmbus_fan_rpm_mask[] = {
+ PB_FAN_1_RPM,
+ PB_FAN_2_RPM,
+ PB_FAN_1_RPM,
+ PB_FAN_2_RPM,
+};
+
+static const int pmbus_fan_config_registers[] = {
+ PMBUS_FAN_CONFIG_12,
+ PMBUS_FAN_CONFIG_12,
+ PMBUS_FAN_CONFIG_34,
+ PMBUS_FAN_CONFIG_34
+};
+
+static const int pmbus_fan_command_registers[] = {
+ PMBUS_FAN_COMMAND_1,
+ PMBUS_FAN_COMMAND_2,
+ PMBUS_FAN_COMMAND_3,
+ PMBUS_FAN_COMMAND_4,
+};
+
void pmbus_clear_cache(struct i2c_client *client)
{
struct pmbus_data *data = i2c_get_clientdata(client);
@@ -188,6 +210,31 @@ int pmbus_write_word_data(struct i2c_client *client, u8 page, u8 reg, u16 word)
}
EXPORT_SYMBOL_GPL(pmbus_write_word_data);
+int pmbus_update_fan(struct i2c_client *client, int page, int id,
+ u8 config, u8 mask, u16 command)
+{
+ int from, rv;
+ u8 to;
+
+ from = pmbus_read_byte_data(client, page,
+ pmbus_fan_config_registers[id]);
+ if (from < 0)
+ return from;
+
+ to = (from & ~mask) | (config & mask);
+
+ if (to != from) {
+ rv = pmbus_write_byte_data(client, page,
+ pmbus_fan_config_registers[id], to);
+ if (rv < 0)
+ return rv;
+ }
+
+ return pmbus_write_word_data(client, page,
+ pmbus_fan_command_registers[id], command);
+}
+EXPORT_SYMBOL_GPL(pmbus_update_fan);
+
/*
* _pmbus_write_word_data() is similar to pmbus_write_word_data(), but checks if
* a device specific mapping function exists and calls it if necessary.
@@ -204,8 +251,40 @@ static int _pmbus_write_word_data(struct i2c_client *client, int page, int reg,
if (status != -ENODATA)
return status;
}
- if (reg >= PMBUS_VIRT_BASE)
- return -ENXIO;
+ if (reg >= PMBUS_VIRT_BASE) {
+ int id, bit;
+
+ switch (reg) {
+ case PMBUS_VIRT_FAN_TARGET_1:
+ case PMBUS_VIRT_FAN_TARGET_2:
+ case PMBUS_VIRT_FAN_TARGET_3:
+ case PMBUS_VIRT_FAN_TARGET_4:
+ id = reg - PMBUS_VIRT_FAN_TARGET_1;
+ bit = pmbus_fan_rpm_mask[id];
+ status = pmbus_update_fan(client, page, id, bit, bit,
+ word);
+ break;
+ case PMBUS_VIRT_PWM_1:
+ case PMBUS_VIRT_PWM_2:
+ case PMBUS_VIRT_PWM_3:
+ case PMBUS_VIRT_PWM_4:
+ {
+ u32 command = word;
+
+ id = reg - PMBUS_VIRT_PWM_1;
+ bit = pmbus_fan_rpm_mask[id];
+ command *= 100;
+ command /= 255;
+ status = pmbus_update_fan(client, page, id, 0, bit,
+ command);
+ break;
+ }
+ default:
+ status = -ENXIO;
+ break;
+ }
+ return status;
+ }
return pmbus_write_word_data(client, page, reg, word);
}
@@ -221,6 +300,9 @@ int pmbus_read_word_data(struct i2c_client *client, u8 page, u8 reg)
}
EXPORT_SYMBOL_GPL(pmbus_read_word_data);
+static int pmbus_get_fan_command(struct i2c_client *client, int page, int id,
+ enum pmbus_fan_mode mode);
+
/*
* _pmbus_read_word_data() is similar to pmbus_read_word_data(), but checks if
* a device specific mapping function exists and calls it if necessary.
@@ -236,8 +318,42 @@ static int _pmbus_read_word_data(struct i2c_client *client, int page, int reg)
if (status != -ENODATA)
return status;
}
- if (reg >= PMBUS_VIRT_BASE)
- return -ENXIO;
+ if (reg >= PMBUS_VIRT_BASE) {
+ int id;
+
+ switch (reg) {
+ case PMBUS_VIRT_FAN_TARGET_1:
+ case PMBUS_VIRT_FAN_TARGET_2:
+ case PMBUS_VIRT_FAN_TARGET_3:
+ case PMBUS_VIRT_FAN_TARGET_4:
+ id = reg - PMBUS_VIRT_FAN_TARGET_1;
+ status = pmbus_get_fan_command(client, page, id, rpm);
+ break;
+ case PMBUS_VIRT_PWM_1:
+ case PMBUS_VIRT_PWM_2:
+ case PMBUS_VIRT_PWM_3:
+ case PMBUS_VIRT_PWM_4:
+ {
+ int rv;
+
+ id = reg - PMBUS_VIRT_PWM_1;
+ rv = pmbus_get_fan_command(client, page, id, percent);
+ if (rv < 0)
+ return rv;
+
+ rv *= 255;
+ rv /= 100;
+
+ status = rv;
+ break;
+ }
+ default:
+ status = -ENXIO;
+ break;
+ }
+
+ return status;
+ }
return pmbus_read_word_data(client, page, reg);
}
@@ -304,6 +420,28 @@ static int _pmbus_read_byte_data(struct i2c_client *client, int page, int reg)
return pmbus_read_byte_data(client, page, reg);
}
+static int pmbus_get_fan_command(struct i2c_client *client, int page, int id,
+ enum pmbus_fan_mode mode)
+{
+ int config;
+
+ config = _pmbus_read_byte_data(client, page,
+ pmbus_fan_config_registers[id]);
+ if (config < 0)
+ return config;
+
+ /*
+ * We can't meaningfully translate between PWM and RPM, so if the
+ * attribute mode (fan[1-*]_target is RPM, pwm[1-*] and pwm[1-*]_enable
+ * are PWM) doesn't match the hardware mode, then report 0 instead.
+ */
+ if ((mode == rpm) != (!!(config & pmbus_fan_rpm_mask[id])))
+ return 0;
+
+ return _pmbus_read_word_data(client, page,
+ pmbus_fan_command_registers[id]);
+}
+
static void pmbus_clear_fault_page(struct i2c_client *client, int page)
{
_pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
@@ -489,7 +627,7 @@ static long pmbus_reg2data_direct(struct pmbus_data *data,
/* X = 1/m * (Y * 10^-R - b) */
R = -R;
/* scale result to milli-units for everything but fans */
- if (sensor->class != PSC_FAN) {
+ if (!(sensor->class == PSC_FAN || sensor->class == PSC_PWM)) {
R += 3;
b *= 1000;
}
@@ -539,6 +677,9 @@ static long pmbus_reg2data(struct pmbus_data *data, struct pmbus_sensor *sensor)
{
long val;
+ if (!sensor->convert)
+ return sensor->data;
+
switch (data->info->format[sensor->class]) {
case direct:
val = pmbus_reg2data_direct(data, sensor);
@@ -642,7 +783,7 @@ static u16 pmbus_data2reg_direct(struct pmbus_data *data,
}
/* Calculate Y = (m * X + b) * 10^R */
- if (sensor->class != PSC_FAN) {
+ if (!(sensor->class == PSC_FAN || sensor->class == PSC_PWM)) {
R -= 3; /* Adjust R and b for data in milli-units */
b *= 1000;
}
@@ -673,6 +814,9 @@ static u16 pmbus_data2reg(struct pmbus_data *data,
{
u16 regval;
+ if (!sensor->convert)
+ return val;
+
switch (data->info->format[sensor->class]) {
case direct:
regval = pmbus_data2reg_direct(data, sensor, val);
@@ -895,12 +1039,18 @@ static struct pmbus_sensor *pmbus_add_sensor(struct pmbus_data *data,
return NULL;
a = &sensor->attribute;
- snprintf(sensor->name, sizeof(sensor->name), "%s%d_%s",
- name, seq, type);
+ if (type)
+ snprintf(sensor->name, sizeof(sensor->name), "%s%d_%s",
+ name, seq, type);
+ else
+ snprintf(sensor->name, sizeof(sensor->name), "%s%d",
+ name, seq);
+
sensor->page = page;
sensor->reg = reg;
sensor->class = class;
sensor->update = update;
+ sensor->convert = true;
pmbus_dev_attr_init(a, sensor->name,
readonly ? S_IRUGO : S_IRUGO | S_IWUSR,
pmbus_show_sensor, pmbus_set_sensor);
@@ -1558,13 +1708,6 @@ static const int pmbus_fan_registers[] = {
PMBUS_READ_FAN_SPEED_4
};
-static const int pmbus_fan_config_registers[] = {
- PMBUS_FAN_CONFIG_12,
- PMBUS_FAN_CONFIG_12,
- PMBUS_FAN_CONFIG_34,
- PMBUS_FAN_CONFIG_34
-};
-
static const int pmbus_fan_status_registers[] = {
PMBUS_STATUS_FAN_12,
PMBUS_STATUS_FAN_12,
@@ -1587,6 +1730,48 @@ static const u32 pmbus_fan_status_flags[] = {
};
/* Fans */
+static int pmbus_add_fan_ctrl(struct i2c_client *client,
+ struct pmbus_data *data, int index, int page, int id,
+ u8 config)
+{
+ struct pmbus_sensor *sensor;
+ int rv;
+
+ rv = _pmbus_read_word_data(client, page,
+ pmbus_fan_command_registers[id]);
+ if (rv < 0)
+ return rv;
+
+ sensor = pmbus_add_sensor(data, "fan", "target", index, page,
+ PMBUS_VIRT_FAN_TARGET_1 + id, PSC_FAN,
+ true, false);
+
+ if (!sensor)
+ return -ENOMEM;
+
+ if (!((data->info->func[page] & PMBUS_HAVE_PWM12) ||
+ (data->info->func[page] & PMBUS_HAVE_PWM34)))
+ return 0;
+
+ sensor = pmbus_add_sensor(data, "pwm", NULL, index, page,
+ PMBUS_VIRT_PWM_1 + id, PSC_PWM,
+ true, false);
+
+ if (!sensor)
+ return -ENOMEM;
+
+ sensor = pmbus_add_sensor(data, "pwm", "enable", index, page,
+ PMBUS_VIRT_PWM_ENABLE_1 + id, PSC_PWM,
+ true, false);
+
+ if (!sensor)
+ return -ENOMEM;
+
+ sensor->convert = false;
+
+ return 0;
+}
+
static int pmbus_add_fan_attributes(struct i2c_client *client,
struct pmbus_data *data)
{
@@ -1624,6 +1809,15 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,
PSC_FAN, true, true) == NULL)
return -ENOMEM;
+ /* Fan control */
+ if (pmbus_check_word_register(client, page,
+ pmbus_fan_command_registers[f])) {
+ ret = pmbus_add_fan_ctrl(client, data, index,
+ page, f, regval);
+ if (ret < 0)
+ return ret;
+ }
+
/*
* Each fan status register covers multiple fans,
* so we have to do some magic.
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/4] pmbus: Add driver for Maxim MAX31785 Intelligent Fan Controller
2017-08-02 7:15 [PATCH v2 0/4] pmbus: Expand fan support and add MAX31785 driver Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 2/4] hwmon: pmbus: Add fan control support Andrew Jeffery
@ 2017-08-02 7:15 ` Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 4/4] pmbus: max31785: Work around back-to-back writes with FAN_CONFIG_1_2 Andrew Jeffery
3 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2017-08-02 7:15 UTC (permalink / raw)
To: linux, linux-hwmon
Cc: Andrew Jeffery, robh+dt, mark.rutland, jdelvare, devicetree,
linux-kernel, joel, mspinler, msbarth, openbmc
The Maxim MAX31785 is a PMBus device providing closed-loop,
multi-channel fan management with temperature and remote voltage
sensing. Various fan control features are provided, including PWM
frequency control, temperature hysteresis, dual tachometer measurements,
and fan health monitoring.
The driver implementation makes use of the new fan control virtual
registers exposed by the pmbus core. It mixes use of the default
implementations with some overrides via the read/write handlers to
handle FAN_COMMAND_1 on the MAX31785, whose definition breaks the value
range into various control bands dependent on RPM or PWM mode.
The dual tachometer feature is implemented in hardware with a TACHSEL
input to indicate the rotor under measurement, and exposed on the bus by
extending the READ_FAN_SPEED_1 word with two extra bytes*.
The need to read the non-standard four-byte response leads to a cut-down
implementation of i2c_smbus_xfer_emulated() included in the driver.
Further, to expose the second rotor tachometer value to userspace,
virtual fans are implemented by re-routing the FAN_CONFIG_1_2 register
from the undefined (in hardware) pages 23-28 to the same register on
pages 0-5, and similarly re-routing READ_FAN_SPEED_1 requests on 23-28
to pages 0-5 but extracting the second word of the four-byte response.
* The documentation recommends the slower rotor be associated with
TACHSEL=0, which provides the first word of the response. The TACHSEL=0
measurement is used by the controller's closed-loop fan management.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/pmbus/Kconfig | 10 +
drivers/hwmon/pmbus/Makefile | 1 +
drivers/hwmon/pmbus/max31785.c | 673 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 684 insertions(+)
create mode 100644 drivers/hwmon/pmbus/max31785.c
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index cad1229b7e17..5f2f3c6c7499 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -95,6 +95,16 @@ config SENSORS_MAX20751
This driver can also be built as a module. If so, the module will
be called max20751.
+config SENSORS_MAX31785
+ tristate "Maxim MAX31785 and compatibles"
+ default n
+ help
+ If you say yes here you get hardware monitoring support for Maxim
+ MAX31785.
+
+ This driver can also be built as a module. If so, the module will
+ be called max31785.
+
config SENSORS_MAX34440
tristate "Maxim MAX34440 and compatibles"
default n
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index 562132054aaf..4ea548a8af46 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_SENSORS_LTC2978) += ltc2978.o
obj-$(CONFIG_SENSORS_LTC3815) += ltc3815.o
obj-$(CONFIG_SENSORS_MAX16064) += max16064.o
obj-$(CONFIG_SENSORS_MAX20751) += max20751.o
+obj-$(CONFIG_SENSORS_MAX31785) += max31785.o
obj-$(CONFIG_SENSORS_MAX34440) += max34440.o
obj-$(CONFIG_SENSORS_MAX8688) += max8688.o
obj-$(CONFIG_SENSORS_TPS40422) += tps40422.o
diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
new file mode 100644
index 000000000000..c2b693badcea
--- /dev/null
+++ b/drivers/hwmon/pmbus/max31785.c
@@ -0,0 +1,673 @@
+/*
+ * Copyright (C) 2017 IBM Corp.
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include "pmbus.h"
+
+enum max31785_regs {
+ MFR_REVISION = 0x9b,
+ MFR_FAULT_RESPONSE = 0xd9,
+ MFR_TEMP_SENSOR_CONFIG = 0xf0,
+ MFR_FAN_CONFIG = 0xf1,
+ MFR_READ_FAN_PWM = 0xf3,
+ MFR_FAN_FAULT_LIMIT = 0xf5,
+ MFR_FAN_WARN_LIMIT = 0xf6,
+ MFR_FAN_PWM_AVG = 0xf8,
+};
+
+#define MAX31785 0x3030
+#define MAX31785A 0x3040
+
+#define MFR_TEMP_SENSOR_CONFIG_ENABLE BIT(15)
+#define MFR_TEMP_SENSOR_CONFIG_OFFSET GENMASK(14, 10)
+
+#define MFR_FAN_CONFIG_FREQ GENMASK(15, 13)
+#define MFR_FAN_CONFIG_DUAL_TACH BIT(12)
+#define MFR_FAN_CONFIG_HYS GENMASK(11, 10)
+#define MFR_FAN_CONFIG_TSFO BIT(9)
+#define MFR_FAN_CONFIG_TACHO BIT(8)
+#define MFR_FAN_CONFIG_RAMP GENMASK(7, 5)
+#define MFR_FAN_CONFIG_HEALTH BIT(4)
+#define MFR_FAN_CONFIG_ROTOR_HI_LO BIT(3)
+#define MFR_FAN_CONFIG_ROTOR BIT(2)
+#define MFR_FAN_CONFIG_SPIN GENMASK(1, 0)
+
+#define MFR_FAULT_RESPONSE_MONITOR BIT(0)
+
+#define MAX31785_CAP_READ_DUAL_TACH BIT(0)
+
+#define MAX31785_NR_PAGES 23
+
+static int max31785_read_byte_data(struct i2c_client *client, int page,
+ int reg)
+{
+ switch (reg) {
+ case PMBUS_VOUT_MODE:
+ if (page < MAX31785_NR_PAGES)
+ return -ENODATA;
+
+ return -ENOTSUPP;
+ case PMBUS_FAN_CONFIG_12:
+ if (page < MAX31785_NR_PAGES)
+ return -ENODATA;
+
+ return pmbus_read_byte_data(client, page - MAX31785_NR_PAGES,
+ reg);
+ }
+
+ return -ENODATA;
+}
+
+static int max31785_write_byte(struct i2c_client *client, int page, u8 value)
+{
+ if (page < MAX31785_NR_PAGES)
+ return -ENODATA;
+
+ return -ENOTSUPP;
+}
+
+static int max31785_read_long_data(struct i2c_client *client, int page,
+ int reg, u32 *data)
+{
+ unsigned char cmdbuf[1];
+ unsigned char rspbuf[4];
+ int rc;
+
+ struct i2c_msg msg[2] = {
+ {
+ .addr = client->addr,
+ .flags = 0,
+ .len = sizeof(cmdbuf),
+ .buf = cmdbuf,
+ },
+ {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .len = sizeof(rspbuf),
+ .buf = rspbuf,
+ },
+ };
+
+ cmdbuf[0] = reg;
+
+ rc = pmbus_set_page(client, page);
+ if (rc < 0)
+ return rc;
+
+ rc = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
+ if (rc < 0)
+ return rc;
+
+ *data = (rspbuf[0] << (0 * 8)) | (rspbuf[1] << (1 * 8)) |
+ (rspbuf[2] << (2 * 8)) | (rspbuf[3] << (3 * 8));
+
+ return rc;
+}
+
+static int max31785_get_pwm(struct i2c_client *client, int page)
+{
+ int config;
+ int command;
+
+ config = pmbus_read_byte_data(client, page, PMBUS_FAN_CONFIG_12);
+ if (config < 0)
+ return config;
+
+ command = pmbus_read_word_data(client, page, PMBUS_FAN_COMMAND_1);
+ if (command < 0)
+ return command;
+
+ if (!(config & PB_FAN_1_RPM)) {
+ if (command >= 0x8000)
+ return 0;
+ else if (command >= 0x2711)
+ return 0x2710;
+
+ return command;
+ }
+
+ return 0;
+}
+
+static int max31785_get_pwm_mode(struct i2c_client *client, int page)
+{
+ int config;
+ int command;
+
+ config = pmbus_read_byte_data(client, page, PMBUS_FAN_CONFIG_12);
+ if (config < 0)
+ return config;
+
+ command = pmbus_read_word_data(client, page, PMBUS_FAN_COMMAND_1);
+ if (command < 0)
+ return command;
+
+ if (!(config & PB_FAN_1_RPM)) {
+ if (command >= 0x8000)
+ return 2;
+ else if (command >= 0x2711)
+ return 0;
+
+ return 1;
+ }
+
+ return (command >= 0x8000) ? 2 : 1;
+}
+
+static int max31785_read_word_data(struct i2c_client *client, int page,
+ int reg)
+{
+ int rv;
+
+ switch (reg) {
+ case PMBUS_READ_FAN_SPEED_1:
+ {
+ u32 val;
+
+ if (page < MAX31785_NR_PAGES)
+ return -ENODATA;
+
+ rv = max31785_read_long_data(client, page - MAX31785_NR_PAGES,
+ reg, &val);
+ if (rv < 0)
+ return rv;
+
+ rv = (val >> 16) & 0xffff;
+ break;
+ }
+ case PMBUS_VIRT_PWM_1:
+ if (page >= MAX31785_NR_PAGES)
+ return -ENOTSUPP;
+
+ rv = max31785_get_pwm(client, page);
+ if (rv < 0)
+ return rv;
+
+ rv *= 255;
+ rv /= 100;
+ break;
+ case PMBUS_VIRT_PWM_ENABLE_1:
+ if (page >= MAX31785_NR_PAGES)
+ return -ENOTSUPP;
+
+ rv = max31785_get_pwm_mode(client, page);
+ break;
+ default:
+ rv = (page >= MAX31785_NR_PAGES) ? -ENXIO : -ENODATA;
+ break;
+ }
+
+ return rv;
+}
+
+static const int max31785_pwm_modes[] = { 0x7fff, 0x2710, 0xffff };
+
+static int max31785_write_word_data(struct i2c_client *client, int page,
+ int reg, u16 word)
+{
+ if (page >= MAX31785_NR_PAGES)
+ return -ENXIO;
+
+ switch (reg) {
+ case PMBUS_VIRT_PWM_ENABLE_1:
+ if (word >= ARRAY_SIZE(max31785_pwm_modes))
+ return -EINVAL;
+
+ return pmbus_update_fan(client, page, 0, 0, PB_FAN_1_RPM,
+ max31785_pwm_modes[word]);
+ default:
+ break;
+ }
+
+ return -ENODATA;
+}
+
+/*
+ * Returns negative error codes if an unrecoverable problem is detected, 0 if a
+ * recoverable problem is detected, or a positive value on success.
+ */
+static int max31785_of_fan_config(struct i2c_client *client,
+ struct pmbus_driver_info *info,
+ u32 capabilities, struct device_node *child)
+{
+ int mfr_cfg = 0, mfr_fault_resp = 0, pb_cfg;
+ struct device *dev = &client->dev;
+ char *lock_polarity = NULL;
+ const char *sval;
+ u32 page;
+ u32 uval;
+ int ret;
+
+ if (!of_device_is_compatible(child, "pmbus-fan"))
+ return 0;
+
+ ret = of_property_read_u32(child, "reg", &page);
+ if (ret < 0) {
+ dev_err(&client->dev, "Missing valid reg property\n");
+ return ret;
+ }
+
+ if (!(info->func[page] & PMBUS_HAVE_FAN12)) {
+ dev_err(dev, "Page %d does not have fan capabilities\n", page);
+ return -ENXIO;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
+ if (ret < 0)
+ return ret;
+
+ pb_cfg = i2c_smbus_read_byte_data(client, PMBUS_FAN_CONFIG_12);
+ if (pb_cfg < 0)
+ return pb_cfg;
+
+ if (of_property_read_bool(child->parent, "use-stored-presence")) {
+ if (!(pb_cfg & PB_FAN_1_INSTALLED))
+ dev_info(dev, "Fan %d is configured but not installed\n",
+ page);
+ } else {
+ pb_cfg |= PB_FAN_1_INSTALLED;
+ }
+
+ ret = of_property_read_string(child, "maxim,fan-rotor-input", &sval);
+ if (ret < 0) {
+ dev_err(dev, "Missing valid maxim,fan-rotor-input property for fan %d\n",
+ page);
+ return ret;
+ }
+
+ if (strcmp("tach", sval) && strcmp("lock", sval)) {
+ dev_err(dev, "maxim,fan-rotor-input has invalid value for fan %d: %s\n",
+ page, sval);
+ return -EINVAL;
+ } else if (!strcmp("lock", sval)) {
+ mfr_cfg |= MFR_FAN_CONFIG_ROTOR;
+
+ ret = i2c_smbus_write_word_data(client, MFR_FAN_FAULT_LIMIT, 1);
+ if (ret < 0)
+ return ret;
+
+ ret = of_property_read_string(child, "maxim,fan-lock-polarity",
+ &sval);
+ if (ret < 0) {
+ dev_err(dev, "Missing valid maxim,fan-lock-polarity property for fan %d\n",
+ page);
+ return ret;
+ }
+
+ if (strcmp("low", sval) && strcmp("high", sval)) {
+ dev_err(dev, "maxim,fan-lock-polarity has invalid value for fan %d: %s\n",
+ page, lock_polarity);
+ return -EINVAL;
+ } else if (!strcmp("high", sval))
+ mfr_cfg |= MFR_FAN_CONFIG_ROTOR_HI_LO;
+ }
+
+ if (!of_property_read_string(child, "fan-mode", &sval)) {
+ if (!strcmp("rpm", sval))
+ pb_cfg |= PB_FAN_1_RPM;
+ else if (!strcmp("pwm", sval))
+ pb_cfg &= ~PB_FAN_1_RPM;
+ else {
+ dev_err(dev, "fan-mode has invalid value for fan %d: %s\n",
+ page, sval);
+ return -EINVAL;
+ }
+ }
+
+ ret = of_property_read_u32(child, "tach-pulses", &uval);
+ if (ret < 0) {
+ pb_cfg &= ~PB_FAN_1_PULSE_MASK;
+ } else if (uval && (uval - 1) < 4) {
+ pb_cfg = ((pb_cfg & ~PB_FAN_1_PULSE_MASK) | ((uval - 1) << 4));
+ } else {
+ dev_err(dev, "tach-pulses has invalid value for fan %d: %u\n",
+ page, uval);
+ return -EINVAL;
+ }
+
+ if (of_property_read_bool(child, "maxim,fan-health"))
+ mfr_cfg |= MFR_FAN_CONFIG_HEALTH;
+
+ if (of_property_read_bool(child, "maxim,fan-no-watchdog") ||
+ of_property_read_bool(child, "maxim,tmp-no-fault-ramp"))
+ mfr_cfg |= MFR_FAN_CONFIG_TSFO;
+
+ if (of_property_read_bool(child, "maxim,fan-dual-tach")) {
+ mfr_cfg |= MFR_FAN_CONFIG_DUAL_TACH;
+
+ if (capabilities & MAX31785_CAP_READ_DUAL_TACH) {
+ int virtual = MAX31785_NR_PAGES + page;
+
+ info->pages = max(info->pages, virtual + 1);
+ info->func[virtual] |= PMBUS_HAVE_FAN12;
+ }
+ }
+
+ if (of_property_read_bool(child, "maxim,fan-no-fault-ramp"))
+ mfr_cfg |= MFR_FAN_CONFIG_TACHO;
+
+ if (!of_property_read_u32(child, "maxim,fan-startup", &uval)) {
+ uval /= 2;
+ if (uval < 5) {
+ mfr_cfg |= uval;
+ } else {
+ dev_err(dev, "maxim,fan-startup has invalid value for fan %d: %u\n",
+ page, uval);
+ return -EINVAL;
+ }
+ }
+
+ if (!of_property_read_u32(child, "maxim,fan-ramp", &uval)) {
+ if (uval < 8) {
+ mfr_cfg |= uval << 5;
+ } else {
+ dev_err(dev, "maxim,fan-ramp has invalid value for fan %d: %u\n",
+ page, uval);
+ return -EINVAL;
+ }
+ }
+
+ if (!of_property_read_u32(child, "maxim,tmp-hysteresis", &uval)) {
+ uval /= 2;
+ uval -= 1;
+ if (uval < 4) {
+ mfr_cfg |= uval << 10;
+ } else {
+ dev_err(dev, "maxim,tmp-hysteresis has invalid value for fan %d, %u\n",
+ page, uval);
+ return -EINVAL;
+ }
+ }
+
+ if (!of_property_read_u32(child, "maxim,fan-pwm-freq", &uval)) {
+ u16 val;
+
+ if (uval == 30) {
+ val = 0;
+ } else if (uval == 50) {
+ val = 1;
+ } else if (uval == 100) {
+ val = 2;
+ } else if (uval == 150) {
+ val = 3;
+ } else if (uval == 25000) {
+ val = 7;
+ } else {
+ dev_err(dev, "maxim,fan-pwm-freq has invalid value for fan %d: %u\n",
+ page, uval);
+ return -EINVAL;
+ }
+
+ mfr_cfg |= val << 13;
+ }
+
+ if (of_property_read_bool(child, "maxim,fan-fault-pin-mon"))
+ mfr_fault_resp |= MFR_FAULT_RESPONSE_MONITOR;
+
+ ret = i2c_smbus_write_byte_data(client, PMBUS_FAN_CONFIG_12,
+ pb_cfg & ~PB_FAN_1_INSTALLED);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_write_word_data(client, MFR_FAN_CONFIG, mfr_cfg);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, MFR_FAULT_RESPONSE,
+ mfr_fault_resp);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, PMBUS_FAN_CONFIG_12, pb_cfg);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * Fans are on pages 0 - 5. If the page property of a fan node is
+ * greater than 5 we will have errored in checks above out above.
+ * Therefore we don't need to cope with values up to 31, and the int
+ * return type is enough.
+ *
+ * The bit mask return value is used to populate a bitfield of fans
+ * who are both configured in the devicetree _and_ reported as
+ * installed by the hardware. Any fans that are not configured in the
+ * devicetree but are reported as installed by the hardware will have
+ * their hardware configuration updated to unset the installed bit.
+ */
+ return BIT(page);
+}
+
+static int max31785_of_tmp_config(struct i2c_client *client,
+ struct pmbus_driver_info *info,
+ struct device_node *child)
+{
+ struct device *dev = &client->dev;
+ u16 mfr_tmp_cfg = 0;
+ int nr_fans;
+ u32 page;
+ u32 uval;
+ int ret;
+ int i;
+
+ if (!of_device_is_compatible(child, "pmbus-temperature"))
+ return 0;
+
+ ret = of_property_read_u32(child, "reg", &page);
+ if (ret < 0) {
+ dev_err(&client->dev, "Missing valid reg property\n");
+ return ret;
+ }
+
+ if (!(info->func[page] & PMBUS_HAVE_TEMP)) {
+ dev_err(dev, "Page %d does not have temp capabilities\n", page);
+ return -ENXIO;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
+ if (ret < 0)
+ return ret;
+
+ if (!of_property_read_u32(child, "maxim,tmp-offset", &uval)) {
+ if (uval < 32)
+ mfr_tmp_cfg |= uval << 10;
+ }
+
+ nr_fans = of_property_count_elems_of_size(child, "maxim,tmp-fans",
+ sizeof(u32));
+ if (nr_fans > 0) {
+ mfr_tmp_cfg |= MFR_TEMP_SENSOR_CONFIG_ENABLE;
+
+ for (i = 0; i < nr_fans; i++) {
+ if (of_property_read_u32_index(child, "maxim,tmp-fans",
+ i, &uval))
+ continue;
+
+ if (uval < 6)
+ mfr_tmp_cfg |= BIT(uval);
+ }
+ }
+
+ ret = i2c_smbus_write_word_data(client, MFR_TEMP_SENSOR_CONFIG,
+ mfr_tmp_cfg);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+#define MAX31785_FAN_FUNCS \
+ (PMBUS_HAVE_FAN12 | PMBUS_HAVE_STATUS_FAN12 | PMBUS_HAVE_PWM12)
+
+#define MAX31785_TEMP_FUNCS \
+ (PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP)
+
+#define MAX31785_VOUT_FUNCS \
+ (PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT)
+
+static const struct pmbus_driver_info max31785_info = {
+ .pages = MAX31785_NR_PAGES,
+
+ .write_word_data = max31785_write_word_data,
+ .read_byte_data = max31785_read_byte_data,
+ .read_word_data = max31785_read_word_data,
+ .write_byte = max31785_write_byte,
+
+ /* RPM */
+ .format[PSC_FAN] = direct,
+ .m[PSC_FAN] = 1,
+ .b[PSC_FAN] = 0,
+ .R[PSC_FAN] = 0,
+ /* PWM */
+ .format[PSC_PWM] = direct,
+ .m[PSC_PWM] = 1,
+ .b[PSC_PWM] = 0,
+ .R[PSC_PWM] = 2,
+ .func[0] = MAX31785_FAN_FUNCS,
+ .func[1] = MAX31785_FAN_FUNCS,
+ .func[2] = MAX31785_FAN_FUNCS,
+ .func[3] = MAX31785_FAN_FUNCS,
+ .func[4] = MAX31785_FAN_FUNCS,
+ .func[5] = MAX31785_FAN_FUNCS,
+
+ .format[PSC_TEMPERATURE] = direct,
+ .m[PSC_TEMPERATURE] = 1,
+ .b[PSC_TEMPERATURE] = 0,
+ .R[PSC_TEMPERATURE] = 2,
+ .func[6] = MAX31785_TEMP_FUNCS,
+ .func[7] = MAX31785_TEMP_FUNCS,
+ .func[8] = MAX31785_TEMP_FUNCS,
+ .func[9] = MAX31785_TEMP_FUNCS,
+ .func[10] = MAX31785_TEMP_FUNCS,
+ .func[11] = MAX31785_TEMP_FUNCS,
+ .func[12] = MAX31785_TEMP_FUNCS,
+ .func[13] = MAX31785_TEMP_FUNCS,
+ .func[14] = MAX31785_TEMP_FUNCS,
+ .func[15] = MAX31785_TEMP_FUNCS,
+ .func[16] = MAX31785_TEMP_FUNCS,
+
+ .format[PSC_VOLTAGE_OUT] = direct,
+ .m[PSC_VOLTAGE_OUT] = 1,
+ .b[PSC_VOLTAGE_OUT] = 0,
+ .R[PSC_VOLTAGE_OUT] = 0,
+ .func[17] = MAX31785_VOUT_FUNCS,
+ .func[18] = MAX31785_VOUT_FUNCS,
+ .func[19] = MAX31785_VOUT_FUNCS,
+ .func[20] = MAX31785_VOUT_FUNCS,
+ .func[21] = MAX31785_VOUT_FUNCS,
+ .func[22] = MAX31785_VOUT_FUNCS,
+};
+
+static int max31785_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct device_node *child;
+ struct pmbus_driver_info *info;
+ u32 fans;
+ u32 caps;
+ s64 ret;
+ int i;
+
+ info = devm_kzalloc(dev, sizeof(struct pmbus_driver_info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ *info = max31785_info;
+
+ ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 255);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_read_word_data(client, MFR_REVISION);
+ if (ret < 0)
+ return ret;
+
+ caps = 0;
+ if (!strcmp("max31785a", id->name)) {
+ if (ret == MAX31785A)
+ caps |= MAX31785_CAP_READ_DUAL_TACH;
+ else
+ dev_warn(dev, "Expected max3175a, found max31785: cannot provide secondary tachometer readings\n");
+ } else if (!strcmp("max31785", id->name)) {
+ if (ret == MAX31785A)
+ dev_info(dev, "Expected max31785, found max3175a: suppressing secondary tachometer attributes\n");
+ } else {
+ return -EINVAL;
+ }
+
+
+ fans = 0;
+ for_each_child_of_node(dev->of_node, child) {
+ ret = max31785_of_fan_config(client, info, caps, child);
+ if (ret < 0) {
+ of_node_put(child);
+ return ret;
+ }
+
+ if (ret)
+ fans |= ret;
+
+ ret = max31785_of_tmp_config(client, info, child);
+ if (ret < 0) {
+ of_node_put(child);
+ return ret;
+ }
+ }
+
+ for (i = 0; i < MAX31785_NR_PAGES; i++) {
+ bool have_fan = !!(info->func[i] & PMBUS_HAVE_FAN12);
+ bool fan_configured = !!(fans & BIT(i));
+
+ if (!have_fan || fan_configured)
+ continue;
+
+ ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_read_byte_data(client, PMBUS_FAN_CONFIG_12);
+ if (ret < 0)
+ return ret;
+
+ ret &= ~PB_FAN_1_INSTALLED;
+ ret = i2c_smbus_write_word_data(client, PMBUS_FAN_CONFIG_12,
+ ret);
+ if (ret < 0)
+ return ret;
+ }
+
+ return pmbus_do_probe(client, id, info);
+}
+
+static const struct i2c_device_id max31785_id[] = {
+ { "max31785", 0 },
+ { "max31785a", 0 },
+ { },
+};
+
+MODULE_DEVICE_TABLE(i2c, max31785_id);
+
+static struct i2c_driver max31785_driver = {
+ .driver = {
+ .name = "max31785",
+ },
+ .probe = max31785_probe,
+ .remove = pmbus_do_remove,
+ .id_table = max31785_id,
+};
+
+module_i2c_driver(max31785_driver);
+
+MODULE_AUTHOR("Andrew Jeffery <andrew@aj.id.au>");
+MODULE_DESCRIPTION("PMBus driver for the Maxim MAX31785");
+MODULE_LICENSE("GPL");
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 4/4] pmbus: max31785: Work around back-to-back writes with FAN_CONFIG_1_2
2017-08-02 7:15 [PATCH v2 0/4] pmbus: Expand fan support and add MAX31785 driver Andrew Jeffery
` (2 preceding siblings ...)
2017-08-02 7:15 ` [PATCH v2 3/4] pmbus: Add driver for Maxim MAX31785 Intelligent Fan Controller Andrew Jeffery
@ 2017-08-02 7:15 ` Andrew Jeffery
2017-08-29 1:08 ` Andrew Jeffery
3 siblings, 1 reply; 9+ messages in thread
From: Andrew Jeffery @ 2017-08-02 7:15 UTC (permalink / raw)
To: linux, linux-hwmon
Cc: Andrew Jeffery, robh+dt, mark.rutland, jdelvare, devicetree,
linux-kernel, joel, mspinler, msbarth, openbmc
Testing of the pmbus max31785 driver implementation revealed occasional
NACKs from the device. Attempting the same transaction immediately after
the failure appears to always succeed. The NACK has consistently been
observed to happen on the second write of back-to-back writes to the
device, where the first write is to FAN_CONFIG_1_2. The NACK occurs
against the first data byte transmitted by the master on the second
write. The behaviour has the hallmarks of a PMBus Device Busy response,
but the busy bit is not set in the status byte.
Work around this undocumented behaviour by retrying any back-to-back
writes that occur after first writing FAN_CONFIG_1_2.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
drivers/hwmon/pmbus/max31785.c | 105 +++++++++++++++++++++++++++++++++++++----
1 file changed, 97 insertions(+), 8 deletions(-)
diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
index c2b693badcea..509b1a5a49b9 100644
--- a/drivers/hwmon/pmbus/max31785.c
+++ b/drivers/hwmon/pmbus/max31785.c
@@ -48,6 +48,55 @@ enum max31785_regs {
#define MAX31785_NR_PAGES 23
+/*
+ * MAX31785 dragons ahead
+ *
+ * It seems there's an undocumented timing constraint when performing
+ * back-to-back writes where the first write is to FAN_CONFIG_1_2. The device
+ * provides no indication of this besides NACK'ing master Txs; no bits are set
+ * in STATUS_BYTE to suggest anything has gone wrong.
+ *
+ * Given that, do a one-shot retry of the write.
+ *
+ * The max31785_*_write_*_data() functions should be used at any point where
+ * the prior write is to FAN_CONFIG_1_2.
+ */
+static int max31785_i2c_smbus_write_byte_data(struct i2c_client *client,
+ int command, u16 data)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(client, command, data);
+ if (ret == -EIO)
+ ret = i2c_smbus_write_byte_data(client, command, data);
+
+ return ret;
+}
+
+static int max31785_i2c_smbus_write_word_data(struct i2c_client *client,
+ int command, u16 data)
+{
+ int ret;
+
+ ret = i2c_smbus_write_word_data(client, command, data);
+ if (ret == -EIO)
+ ret = i2c_smbus_write_word_data(client, command, data);
+
+ return ret;
+}
+
+static int max31785_pmbus_write_word_data(struct i2c_client *client, int page,
+ int command, u16 data)
+{
+ int ret;
+
+ ret = pmbus_write_word_data(client, page, command, data);
+ if (ret == -EIO)
+ ret = pmbus_write_word_data(client, page, command, data);
+
+ return ret;
+}
+
static int max31785_read_byte_data(struct i2c_client *client, int page,
int reg)
{
@@ -210,6 +259,31 @@ static int max31785_read_word_data(struct i2c_client *client, int page,
return rv;
}
+static int max31785_update_fan(struct i2c_client *client, int page,
+ u8 config, u8 mask, u16 command)
+{
+ int from, rv;
+ u8 to;
+
+ from = pmbus_read_byte_data(client, page, PMBUS_FAN_CONFIG_12);
+ if (from < 0)
+ return from;
+
+ to = (from & ~mask) | (config & mask);
+
+ if (to != from) {
+ rv = pmbus_write_byte_data(client, page, PMBUS_FAN_CONFIG_12,
+ to);
+ if (rv < 0)
+ return rv;
+ }
+
+ rv = max31785_pmbus_write_word_data(client, page, PMBUS_FAN_COMMAND_1,
+ command);
+
+ return rv;
+}
+
static const int max31785_pwm_modes[] = { 0x7fff, 0x2710, 0xffff };
static int max31785_write_word_data(struct i2c_client *client, int page,
@@ -219,12 +293,24 @@ static int max31785_write_word_data(struct i2c_client *client, int page,
return -ENXIO;
switch (reg) {
+ case PMBUS_VIRT_FAN_TARGET_1:
+ return max31785_update_fan(client, page, PB_FAN_1_RPM,
+ PB_FAN_1_RPM, word);
+ case PMBUS_VIRT_PWM_1:
+ {
+ u32 command = word;
+
+ command *= 100;
+ command /= 255;
+ return max31785_update_fan(client, page, 0, PB_FAN_1_RPM,
+ command);
+ }
case PMBUS_VIRT_PWM_ENABLE_1:
if (word >= ARRAY_SIZE(max31785_pwm_modes))
return -EINVAL;
- return pmbus_update_fan(client, page, 0, 0, PB_FAN_1_RPM,
- max31785_pwm_modes[word]);
+ return max31785_update_fan(client, page, 0, PB_FAN_1_RPM,
+ max31785_pwm_modes[word]);
default:
break;
}
@@ -262,7 +348,7 @@ static int max31785_of_fan_config(struct i2c_client *client,
return -ENXIO;
}
- ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
+ ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
if (ret < 0)
return ret;
@@ -419,7 +505,8 @@ static int max31785_of_fan_config(struct i2c_client *client,
if (ret < 0)
return ret;
- ret = i2c_smbus_write_word_data(client, MFR_FAN_CONFIG, mfr_cfg);
+ ret = max31785_i2c_smbus_write_word_data(client, MFR_FAN_CONFIG,
+ mfr_cfg);
if (ret < 0)
return ret;
@@ -473,7 +560,7 @@ static int max31785_of_tmp_config(struct i2c_client *client,
return -ENXIO;
}
- ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
+ ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
if (ret < 0)
return ret;
@@ -631,7 +718,8 @@ static int max31785_probe(struct i2c_client *client,
if (!have_fan || fan_configured)
continue;
- ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);
+ ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE,
+ i);
if (ret < 0)
return ret;
@@ -640,8 +728,9 @@ static int max31785_probe(struct i2c_client *client,
return ret;
ret &= ~PB_FAN_1_INSTALLED;
- ret = i2c_smbus_write_word_data(client, PMBUS_FAN_CONFIG_12,
- ret);
+ ret = max31785_i2c_smbus_write_word_data(client,
+ PMBUS_FAN_CONFIG_12,
+ ret);
if (ret < 0)
return ret;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation
2017-08-02 7:15 ` [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation Andrew Jeffery
@ 2017-08-10 16:13 ` Rob Herring
2017-08-14 1:55 ` Andrew Jeffery
0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2017-08-10 16:13 UTC (permalink / raw)
To: Andrew Jeffery
Cc: linux, linux-hwmon, mark.rutland, jdelvare, devicetree,
linux-kernel, joel, mspinler, msbarth, openbmc
On Wed, Aug 02, 2017 at 04:45:38PM +0930, Andrew Jeffery wrote:
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> .../devicetree/bindings/hwmon/pmbus/max31785.txt | 126 +++++++++++++++++++++
> 1 file changed, 126 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
>
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> new file mode 100644
> index 000000000000..8ddc4ea1958d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> @@ -0,0 +1,126 @@
> +Bindings for the Maxim MAX31785 Intelligent Fan Controller
> +==========================================================
This is the second fan controller binding I've seen recently. The other
being hwmon/aspeed-pwm-tacho.txt. I think some of this needs to be
common. There's only so many types of fans, right?
And we have the thermal binding which you don't appear to tie into.
> +
> +Reference:
> +[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
> +
> +Required properties:
> +- compatible : One of "maxim,max31785" or "maxim,max31785a"
> +- reg : I2C address, one of 0x52, 0x53, 0x54, 0x55.
> +- #address-cells : Must be 1
> +- #size-cells : Must be 0
> +
> +Optional properties:
> +- use-stored-presence : Do not treat the devicetree description as canon for
Is this maxim specific? If so, needs a vendor prefix.
> + fan presence (the 'installed' bit of FAN_CONFIG_*).
> + Instead, rely on the on the default value store of
> + the device to populate it.
> +
> +Capabilities are configured through subnodes of the controller's node.
> +
> +Fans
> +----
> +
> +Only fans with subnodes present will be considered as installed. If
> +use-stored-presence is present in the parent node, then only fans that are both
> +defined in the devicetree and have their installed bit set are considered
> +installed.
> +
> +Required subnode properties:
> +- compatible : Must be "pmbus-fan"
What defines a pmbus-fan? Sounds generic, but then you have all these
maxim specific properties.
> +- reg : The PMBus page the properties apply to.
> +- maxim,fan-rotor-input : The type of rotor measurement provided to the
> + controller. Must be either "tach" for tachometer
> + pulses or "lock" for a locked-rotor signal.
> +- maxim,fan-lock-polarity: Required iff maxim,fan-rotor-input is "lock". Valid
> + values are "low" for active low, "high" for active
> + high.
> +
> +Optional subnode properties:
> +- fan-mode : "rpm" or "pwm". Default value is "pwm".
> +- tach-pulses : Tachometer pulses per revolution. Valid values are
> + 1, 2, 3 or 4. The default is 1.
> +- maxim,fan-no-fault-ramp: Do not ramp the fan to 100% PWM duty on detecting a
> + fan fault
> +- maxim,fan-startup : The number of rotations required before taking
> + emergency action for an unresponsive fan and driving
> + it with 100% or 0% PWM duty, depending on the state
> + of maxim,fan-no-fault-ramp. Valid values are 0
> + (automatic spin-up disabled), 2, 4, or 8. Default
> + value is 0.
> +- maxim,fan-health : Enable automated fan health check
> +- maxim,fan-ramp : Configures how fast the device ramps the PWM duty
> + cycle from one value to another. Valid values are 0
> + to 7 inclusive, with values 0 - 2 configuring a
> + 1000ms update rate and 1 - 3% duty respective duty
> + increase, and 3 - 7 a 200ms update rate with a 1 -
> + 5% respective duty increase. Default value is 0.
> +- maxim,fan-no-watchdog : Do not ramp fan to 100% PWM duty on failure to
> + update desired fan rate inside 10s. This implies
> + maxim,tmp-no-fault-ramp
> +- maxim,tmp-no-fault-ramp: Do not ramp fan to 100% PWM duty on temperature
> + sensor fault detection. This implies
> + maxim,fan-no-watchdog
> +- maxim,tmp-hysteresis : The temperature hysteresis used to determine
> + transitions to lower fan speed bands in the
> + temperature/fan rate lookup table. Valid values are
> + 2, 4, 6 or 8 (degrees celcius). Default value is 2.
> +- maxim,fan-dual-tach : Enable dual tachometer functionality
> +- maxim,fan-pwm-freq : The PWM frequency. Valid values are 30, 50, 100, 150
> + and 25000 (Hz). Default value is 30Hz.
> +- maxim,fan-lookup-table : A 16-element cell array of alternating temperature
> + and rate values representing the look up table. The
> + rate units are set through the fan-mode property.
> +- maxim,fan-fault-pin-mon: Ramp fans to 100% PWM duty when the FAULT pin is
> + asserted
> +
> +Temperature
> +-----------
> +
> +Required subnode properties:
> +- compatible : Must be "pmbus-temperature"
> +- reg : The PMBus page the properties apply to.
> +
> +Optional subnode properties:
> +- maxim,tmp-offset : Valid values are 0 - 30 (degrees celcius) inclusive.
> + Default value is 0.
> +- maxim,tmp-fans : An array of fan indexes whose fans are controlled by
> + the current temperature sensor. Fans are indexed from
> + zero. The valid values are 0 - 5 inclusive.
Why not use a phandle here.
> +
> +Example:
> + fan-max31785: max31785@52 {
> + reg = <0x52>;
> + compatible = "maxim,max31785";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + fan@0 {
> + compatible = "pmbus-fan";
> + reg = <0>;
> + mode = "rpm";
> + tach-pulses = <1>;
> + maxim,fan-rotor-input = "tach";
> + maxim,fan-dual-tach;
> + };
> +
> + fan@1 {
> + compatible = "pmbus-fan";
> + reg = <1>;
> + mode = "rpm";
> + tach-pulses = <1>;
> + maxim,fan-rotor-input = "tach";
> + maxim,tmp-hysteresis = <2>;
> + maxim,fan-lookup-table = <
> + /* Temperature RPM */
> + 0 1000
> + 10 2000
> + 20 3000
> + 30 4000
> + 40 5000
> + 50 6000
> + 60 7000
> + 70 8000
> + >;
> + };
> + };
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation
2017-08-10 16:13 ` Rob Herring
@ 2017-08-14 1:55 ` Andrew Jeffery
2017-09-08 3:58 ` Andrew Jeffery
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jeffery @ 2017-08-14 1:55 UTC (permalink / raw)
To: Rob Herring
Cc: linux, linux-hwmon, mark.rutland, jdelvare, devicetree,
linux-kernel, joel, mspinler, msbarth, openbmc
[-- Attachment #1: Type: text/plain, Size: 11648 bytes --]
On Thu, 2017-08-10 at 11:13 -0500, Rob Herring wrote:
> On Wed, Aug 02, 2017 at 04:45:38PM +0930, Andrew Jeffery wrote:
> > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > .../devicetree/bindings/hwmon/pmbus/max31785.txt | 126 +++++++++++++++++++++
> > 1 file changed, 126 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> > new file mode 100644
> > index 000000000000..8ddc4ea1958d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> > @@ -0,0 +1,126 @@
> > +Bindings for the Maxim MAX31785 Intelligent Fan Controller
> > +==========================================================
>
> This is the second fan controller binding I've seen recently. The other
> being hwmon/aspeed-pwm-tacho.txt. I think some of this needs to be
> common. There's only so many types of fans, right?
Heh, you'd think so, I'll take a look. However much of this is driven by the
PMBus specification and the Aspeed PWM/Tacho isn't a PMBus-based device.
I was hesitant to start a generic PMBus bindings document without having more
PMBus devices with bindings, but maybe that's the way I should go? It would
make clear what's from the fan-control parts of the PMBus specification and
what's here for the purpose of supporting the MAX31785.
>
> And we have the thermal binding which you don't appear to tie into.
I'll look into that also.
>
> > +
> > +Reference:
> > +[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
> > +
> > +Required properties:
> > +- compatible : One of "maxim,max31785" or "maxim,max31785a"
> > +- reg : I2C address, one of 0x52, 0x53, 0x54, 0x55.
> > +- #address-cells : Must be 1
> > +- #size-cells : Must be 0
> > +
> > +Optional properties:
> > +- use-stored-presence : Do not treat the devicetree description as canon for
>
> Is this maxim specific? If so, needs a vendor prefix.
PMBus specifies two 8-bit registers of the same structure: FAN_CONFIG_1_2
and FAN_CONFIG_3_4. It's not intended to be manufacturer-specific.
A bit in each nibble of FAN_CONFIG_* is specified as marking whether or not the
fan is installed. The intent of this property is to define whether the consumer
should consider the devicetree as canonical, or the device. In the absence of
the property consumer of the node should mark fans described in the devicetree
as installed (set the bit, and clear the bit for any fan pages that are not
described in the devicetree). Alternatively, the device may be pre-programmed
with a default register value that is suitable for the system the controller
resides in, in which case the devicetree consumer should itself consume the
installed bit from the device rather than set/clear it.
The two remaining fields in FAN_CONFIG_*, fan mode (RPM/PWM) and
tach-pulses-per-revolution (1-4), are covered by optional properties below.
>
> > + fan presence (the 'installed' bit of FAN_CONFIG_*).
> > + Instead, rely on the on the default value store of
> > + the device to populate it.
> > +
> > +Capabilities are configured through subnodes of the controller's node.
> > +
> > +Fans
> > +----
> > +
> > +Only fans with subnodes present will be considered as installed. If
> > +use-stored-presence is present in the parent node, then only fans that are both
> > +defined in the devicetree and have their installed bit set are considered
> > +installed.
> > +
> > +Required subnode properties:
> > +- compatible : Must be "pmbus-fan"
>
> What defines a pmbus-fan? Sounds generic, but then you have all these
> maxim specific properties.
It's driven by the two optional properties, fan-mode and tach-pulses, which
make up the remaining fields of the FAN_CONFIG_* registers from the PMBus
specification. PMBus reserves command ranges for manufacturer-specific use, and
Maxim chose to use one of these reserved commands as MFR_FAN_CONFIG
(Manufacturer-specific Fan Configuration). The vendor-prefixed properties deal
with the properties described in the 16-bit MFR_FAN_CONFIG register.
>
> > +- reg : The PMBus page the properties apply to.
> > +- maxim,fan-rotor-input : The type of rotor measurement provided to the
> > + controller. Must be either "tach" for tachometer
> > + pulses or "lock" for a locked-rotor signal.
> > +- maxim,fan-lock-polarity: Required iff maxim,fan-rotor-input is "lock". Valid
> > + values are "low" for active low, "high" for active
> > + high.
> > +
> > +Optional subnode properties:
> > +- fan-mode : "rpm" or "pwm". Default value is "pwm".
> > +- tach-pulses : Tachometer pulses per revolution. Valid values are
> > + 1, 2, 3 or 4. The default is 1.
> > +- maxim,fan-no-fault-ramp: Do not ramp the fan to 100% PWM duty on detecting a
> > + fan fault
> > +- maxim,fan-startup : The number of rotations required before taking
> > + emergency action for an unresponsive fan and driving
> > + it with 100% or 0% PWM duty, depending on the state
> > + of maxim,fan-no-fault-ramp. Valid values are 0
> > + (automatic spin-up disabled), 2, 4, or 8. Default
> > + value is 0.
> > +- maxim,fan-health : Enable automated fan health check
> > +- maxim,fan-ramp : Configures how fast the device ramps the PWM duty
> > + cycle from one value to another. Valid values are 0
> > + to 7 inclusive, with values 0 - 2 configuring a
> > + 1000ms update rate and 1 - 3% duty respective duty
> > + increase, and 3 - 7 a 200ms update rate with a 1 -
> > + 5% respective duty increase. Default value is 0.
> > +- maxim,fan-no-watchdog : Do not ramp fan to 100% PWM duty on failure to
> > + update desired fan rate inside 10s. This implies
> > + maxim,tmp-no-fault-ramp
> > +- maxim,tmp-no-fault-ramp: Do not ramp fan to 100% PWM duty on temperature
> > + sensor fault detection. This implies
> > + maxim,fan-no-watchdog
> > +- maxim,tmp-hysteresis : The temperature hysteresis used to determine
> > + transitions to lower fan speed bands in the
> > + temperature/fan rate lookup table. Valid values are
> > + 2, 4, 6 or 8 (degrees celcius). Default value is 2.
> > +- maxim,fan-dual-tach : Enable dual tachometer functionality
> > +- maxim,fan-pwm-freq : The PWM frequency. Valid values are 30, 50, 100, 150
> > + and 25000 (Hz). Default value is 30Hz.
> > +- maxim,fan-lookup-table : A 16-element cell array of alternating temperature
> > + and rate values representing the look up table. The
> > + rate units are set through the fan-mode property.
> > +- maxim,fan-fault-pin-mon: Ramp fans to 100% PWM duty when the FAULT pin is
> > + asserted
> > +
> > +Temperature
> > +-----------
> > +
> > +Required subnode properties:
> > +- compatible : Must be "pmbus-temperature"
> > +- reg : The PMBus page the properties apply to.
> > +
> > +Optional subnode properties:
> > +- maxim,tmp-offset : Valid values are 0 - 30 (degrees celcius) inclusive.
> > + Default value is 0.
> > +- maxim,tmp-fans : An array of fan indexes whose fans are controlled by
> > + the current temperature sensor. Fans are indexed from
> > + zero. The valid values are 0 - 5 inclusive.
>
> Why not use a phandle here.
I think that's a better approach. I'll rework it.
Thanks for the feedback.
Andrew
>
> > +
> > +Example:
> > > > > > + fan-max31785: max31785@52 {
> > > > + reg = <0x52>;
> > > > + compatible = "maxim,max31785";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > > > + fan@0 {
> > + compatible = "pmbus-fan";
> > + reg = <0>;
> > + mode = "rpm";
> > + tach-pulses = <1>;
> > + maxim,fan-rotor-input = "tach";
> > + maxim,fan-dual-tach;
> > + };
> > +
> > > > + fan@1 {
> > + compatible = "pmbus-fan";
> > + reg = <1>;
> > + mode = "rpm";
> > + tach-pulses = <1>;
> > + maxim,fan-rotor-input = "tach";
> > + maxim,tmp-hysteresis = <2>;
> > + maxim,fan-lookup-table = <
> > + /* Temperature RPM */
> > + 0 1000
> > + 10 2000
> > + 20 3000
> > + 30 4000
> > + 40 5000
> > + 50 6000
> > + 60 7000
> > + 70 8000
> > + >;
> > + };
> > > > + };
> > --
> > 2.11.0
> >
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 4/4] pmbus: max31785: Work around back-to-back writes with FAN_CONFIG_1_2
2017-08-02 7:15 ` [PATCH v2 4/4] pmbus: max31785: Work around back-to-back writes with FAN_CONFIG_1_2 Andrew Jeffery
@ 2017-08-29 1:08 ` Andrew Jeffery
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2017-08-29 1:08 UTC (permalink / raw)
To: linux, linux-hwmon
Cc: robh+dt, mark.rutland, jdelvare, devicetree, linux-kernel, joel,
mspinler, msbarth, openbmc
[-- Attachment #1: Type: text/plain, Size: 6978 bytes --]
On Wed, 2017-08-02 at 16:45 +0930, Andrew Jeffery wrote:
> Testing of the pmbus max31785 driver implementation revealed occasional
> NACKs from the device. Attempting the same transaction immediately after
> the failure appears to always succeed. The NACK has consistently been
> observed to happen on the second write of back-to-back writes to the
> device, where the first write is to FAN_CONFIG_1_2. The NACK occurs
> against the first data byte transmitted by the master on the second
> write. The behaviour has the hallmarks of a PMBus Device Busy response,
> but the busy bit is not set in the status byte.
>
> Work around this undocumented behaviour by retrying any back-to-back
> writes that occur after first writing FAN_CONFIG_1_2.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
I expect I'll be dropping this patch. At this point it looks like we
have another chip on the bus interfering with transactions to the
MAX31785. Checking the behaviour with a scope showed that SCL was being
held down by something that wasn't the master, but according to Maxim
the SCL pin on the MAX31785 is input-only and therefore it cannot
interfere in the manner we observed. Further testing has narrowed down
the list of candidates for the interference, but our investigation is
ongoing.
Andrew
> ---
> drivers/hwmon/pmbus/max31785.c | 105 +++++++++++++++++++++++++++++++++++++----
> 1 file changed, 97 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/max31785.c b/drivers/hwmon/pmbus/max31785.c
> index c2b693badcea..509b1a5a49b9 100644
> --- a/drivers/hwmon/pmbus/max31785.c
> +++ b/drivers/hwmon/pmbus/max31785.c
> @@ -48,6 +48,55 @@ enum max31785_regs {
>
> > #define MAX31785_NR_PAGES 23
>
> +/*
> + * MAX31785 dragons ahead
> + *
> + * It seems there's an undocumented timing constraint when performing
> + * back-to-back writes where the first write is to FAN_CONFIG_1_2. The device
> + * provides no indication of this besides NACK'ing master Txs; no bits are set
> + * in STATUS_BYTE to suggest anything has gone wrong.
> + *
> + * Given that, do a one-shot retry of the write.
> + *
> + * The max31785_*_write_*_data() functions should be used at any point where
> + * the prior write is to FAN_CONFIG_1_2.
> + */
> +static int max31785_i2c_smbus_write_byte_data(struct i2c_client *client,
> > + int command, u16 data)
> +{
> > + int ret;
> +
> > + ret = i2c_smbus_write_byte_data(client, command, data);
> > + if (ret == -EIO)
> > + ret = i2c_smbus_write_byte_data(client, command, data);
> +
> > + return ret;
> +}
> +
> +static int max31785_i2c_smbus_write_word_data(struct i2c_client *client,
> > + int command, u16 data)
> +{
> > + int ret;
> +
> > + ret = i2c_smbus_write_word_data(client, command, data);
> > + if (ret == -EIO)
> > + ret = i2c_smbus_write_word_data(client, command, data);
> +
> > + return ret;
> +}
> +
> +static int max31785_pmbus_write_word_data(struct i2c_client *client, int page,
> > + int command, u16 data)
> +{
> > + int ret;
> +
> > + ret = pmbus_write_word_data(client, page, command, data);
> > + if (ret == -EIO)
> > + ret = pmbus_write_word_data(client, page, command, data);
> +
> > + return ret;
> +}
> +
> static int max31785_read_byte_data(struct i2c_client *client, int page,
> > int reg)
> {
> @@ -210,6 +259,31 @@ static int max31785_read_word_data(struct i2c_client *client, int page,
> > return rv;
> }
>
> +static int max31785_update_fan(struct i2c_client *client, int page,
> > + u8 config, u8 mask, u16 command)
> +{
> > + int from, rv;
> > + u8 to;
> +
> > + from = pmbus_read_byte_data(client, page, PMBUS_FAN_CONFIG_12);
> > + if (from < 0)
> > + return from;
> +
> > + to = (from & ~mask) | (config & mask);
> +
> > + if (to != from) {
> > + rv = pmbus_write_byte_data(client, page, PMBUS_FAN_CONFIG_12,
> > + to);
> > + if (rv < 0)
> > + return rv;
> > + }
> +
> > + rv = max31785_pmbus_write_word_data(client, page, PMBUS_FAN_COMMAND_1,
> > + command);
> +
> > + return rv;
> +}
> +
> static const int max31785_pwm_modes[] = { 0x7fff, 0x2710, 0xffff };
>
> static int max31785_write_word_data(struct i2c_client *client, int page,
> @@ -219,12 +293,24 @@ static int max31785_write_word_data(struct i2c_client *client, int page,
> > return -ENXIO;
>
> > switch (reg) {
> > + case PMBUS_VIRT_FAN_TARGET_1:
> > + return max31785_update_fan(client, page, PB_FAN_1_RPM,
> > + PB_FAN_1_RPM, word);
> > + case PMBUS_VIRT_PWM_1:
> > + {
> > + u32 command = word;
> +
> > + command *= 100;
> > + command /= 255;
> > + return max31785_update_fan(client, page, 0, PB_FAN_1_RPM,
> > + command);
> > + }
> > case PMBUS_VIRT_PWM_ENABLE_1:
> > if (word >= ARRAY_SIZE(max31785_pwm_modes))
> > return -EINVAL;
>
> > - return pmbus_update_fan(client, page, 0, 0, PB_FAN_1_RPM,
> > - max31785_pwm_modes[word]);
> > + return max31785_update_fan(client, page, 0, PB_FAN_1_RPM,
> > + max31785_pwm_modes[word]);
> > default:
> > break;
> > }
> @@ -262,7 +348,7 @@ static int max31785_of_fan_config(struct i2c_client *client,
> > return -ENXIO;
> > }
>
> > - ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> > + ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> > if (ret < 0)
> > return ret;
>
> @@ -419,7 +505,8 @@ static int max31785_of_fan_config(struct i2c_client *client,
> > if (ret < 0)
> > return ret;
>
> > - ret = i2c_smbus_write_word_data(client, MFR_FAN_CONFIG, mfr_cfg);
> > + ret = max31785_i2c_smbus_write_word_data(client, MFR_FAN_CONFIG,
> > + mfr_cfg);
> > if (ret < 0)
> > return ret;
>
> @@ -473,7 +560,7 @@ static int max31785_of_tmp_config(struct i2c_client *client,
> > return -ENXIO;
> > }
>
> > - ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> > + ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE, page);
> > if (ret < 0)
> > return ret;
>
> @@ -631,7 +718,8 @@ static int max31785_probe(struct i2c_client *client,
> > if (!have_fan || fan_configured)
> > continue;
>
> > - ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, i);
> > + ret = max31785_i2c_smbus_write_byte_data(client, PMBUS_PAGE,
> > + i);
> > if (ret < 0)
> > return ret;
>
> @@ -640,8 +728,9 @@ static int max31785_probe(struct i2c_client *client,
> > return ret;
>
> > ret &= ~PB_FAN_1_INSTALLED;
> > - ret = i2c_smbus_write_word_data(client, PMBUS_FAN_CONFIG_12,
> > - ret);
> > + ret = max31785_i2c_smbus_write_word_data(client,
> > + PMBUS_FAN_CONFIG_12,
> > + ret);
> > if (ret < 0)
> > return ret;
> > }
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation
2017-08-14 1:55 ` Andrew Jeffery
@ 2017-09-08 3:58 ` Andrew Jeffery
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Jeffery @ 2017-09-08 3:58 UTC (permalink / raw)
To: Rob Herring
Cc: linux-hwmon, mark.rutland, jdelvare, devicetree, openbmc,
linux-kernel, linux
[-- Attachment #1: Type: text/plain, Size: 11632 bytes --]
On Mon, 2017-08-14 at 11:25 +0930, Andrew Jeffery wrote:
> On Thu, 2017-08-10 at 11:13 -0500, Rob Herring wrote:
> > On Wed, Aug 02, 2017 at 04:45:38PM +0930, Andrew Jeffery wrote:
> > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > >
> > > ---
> > > .../devicetree/bindings/hwmon/pmbus/max31785.txt | 126 +++++++++++++++++++++
> > > 1 file changed, 126 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> > > new file mode 100644
> > > index 000000000000..8ddc4ea1958d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> > > @@ -0,0 +1,126 @@
> > > +Bindings for the Maxim MAX31785 Intelligent Fan Controller
> > > +==========================================================
> >
> >
> > This is the second fan controller binding I've seen recently. The other
> > being hwmon/aspeed-pwm-tacho.txt. I think some of this needs to be
> > common. There's only so many types of fans, right?
>
> Heh, you'd think so, I'll take a look. However much of this is driven by the
> PMBus specification and the Aspeed PWM/Tacho isn't a PMBus-based device.
Following up on this, there's not much that conflicts between the two fan
descriptions. But there's not much in common either, just because there's
really not that much to it all. In both cases the interesting bits are in the
manufacturer specific properties. reg is applicable in either case. I require a
compatible here to separate fan support from temperature sensing.
What would you be looking for in a common fan description?
However, as an aside, I think there's a fundamental problem with the Aspeed
PWM/Tacho bindings. For reference, a vendor submitted this devicetree patch to
OpenBMC:
http://patchwork.ozlabs.org/patch/804862/
Particularly, I'd draw your attention to this part of the linked patch:
+ fan@0 {
+ reg = <0x00>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x00>;
+ };
+
+ fan@1 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x01>;
+ };
+
+ fan@2 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x02>;
+ };
As outlined in my comments on the patch in the patchwork link above, the
bindings are backwards for what is being described: Eight fans driven by one
PWM, so there is a mismatch between the unit and reg. reg is being used to
index the one PWM, and tach channels are being associated to the PWM.
I rather think fans should be represented by their tach input (the tach ID
assigned to reg), and a PWM channel be associated with the node via e.g. an
aspeed,pwm-ch property. There is the issue of dual-tach fans, but in the case
of the Aspeed PWM/Tach block the dual-tach lines would need to be wired to
separate tach inputs, still leaving us free to select the appropriate tach
input to drive the fan loop.
There are other issues such as the incorrect suggestion for the value of
#size-cells in the Aspeed PWM/Tach bindings.
>
> I was hesitant to start a generic PMBus bindings document without having more
> PMBus devices with bindings, but maybe that's the way I should go? It would
> make clear what's from the fan-control parts of the PMBus specification and
> what's here for the purpose of supporting the MAX31785.
>
> >
> > And we have the thermal binding which you don't appear to tie into.
>
> I'll look into that also.
I've done what I think I need to in order to resolve this.
>
> >
> > > +
> > > +Reference:
> > > +[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
> > > +
> > > +Required properties:
> > > +- compatible : One of "maxim,max31785" or "maxim,max31785a"
> > > +- reg : I2C address, one of 0x52, 0x53, 0x54, 0x55.
> > > +- #address-cells : Must be 1
> > > +- #size-cells : Must be 0
> > > +
> > > +Optional properties:
> > > +- use-stored-presence : Do not treat the devicetree description as canon for
> >
> >
> > Is this maxim specific? If so, needs a vendor prefix.
>
> PMBus specifies two 8-bit registers of the same structure: FAN_CONFIG_1_2
> and FAN_CONFIG_3_4. It's not intended to be manufacturer-specific.
>
> A bit in each nibble of FAN_CONFIG_* is specified as marking whether or not the
> fan is installed. The intent of this property is to define whether the consumer
> should consider the devicetree as canonical, or the device. In the absence of
> the property consumer of the node should mark fans described in the devicetree
> as installed (set the bit, and clear the bit for any fan pages that are not
> described in the devicetree). Alternatively, the device may be pre-programmed
> with a default register value that is suitable for the system the controller
> resides in, in which case the devicetree consumer should itself consume the
> installed bit from the device rather than set/clear it.
>
> The two remaining fields in FAN_CONFIG_*, fan mode (RPM/PWM) and
> tach-pulses-per-revolution (1-4), are covered by optional properties below.
>
> >
> > > + fan presence (the 'installed' bit of FAN_CONFIG_*).
> > > + Instead, rely on the on the default value store of
> > > + the device to populate it.
> > > +
> > > +Capabilities are configured through subnodes of the controller's node.
> > > +
> > > +Fans
> > > +----
> > > +
> > > +Only fans with subnodes present will be considered as installed. If
> > > +use-stored-presence is present in the parent node, then only fans that are both
> > > +defined in the devicetree and have their installed bit set are considered
> > > +installed.
> > > +
> > > +Required subnode properties:
> > > +- compatible : Must be "pmbus-fan"
> >
> >
> > What defines a pmbus-fan? Sounds generic, but then you have all these
> > maxim specific properties.
>
> It's driven by the two optional properties, fan-mode and tach-pulses, which
> make up the remaining fields of the FAN_CONFIG_* registers from the PMBus
> specification. PMBus reserves command ranges for manufacturer-specific use, and
> Maxim chose to use one of these reserved commands as MFR_FAN_CONFIG
> (Manufacturer-specific Fan Configuration). The vendor-prefixed properties deal
> with the properties described in the 16-bit MFR_FAN_CONFIG register.
>
> >
> > > +- reg : The PMBus page the properties apply to.
> > > +- maxim,fan-rotor-input : The type of rotor measurement provided to the
> > > + controller. Must be either "tach" for tachometer
> > > + pulses or "lock" for a locked-rotor signal.
> > > +- maxim,fan-lock-polarity: Required iff maxim,fan-rotor-input is "lock". Valid
> > > + values are "low" for active low, "high" for active
> > > + high.
> > > +
> > > +Optional subnode properties:
> > > +- fan-mode : "rpm" or "pwm". Default value is "pwm".
> > > +- tach-pulses : Tachometer pulses per revolution. Valid values are
> > > + 1, 2, 3 or 4. The default is 1.
> > > +- maxim,fan-no-fault-ramp: Do not ramp the fan to 100% PWM duty on detecting a
> > > + fan fault
> > > +- maxim,fan-startup : The number of rotations required before taking
> > > + emergency action for an unresponsive fan and driving
> > > + it with 100% or 0% PWM duty, depending on the state
> > > + of maxim,fan-no-fault-ramp. Valid values are 0
> > > + (automatic spin-up disabled), 2, 4, or 8. Default
> > > + value is 0.
> > > +- maxim,fan-health : Enable automated fan health check
> > > +- maxim,fan-ramp : Configures how fast the device ramps the PWM duty
> > > + cycle from one value to another. Valid values are 0
> > > + to 7 inclusive, with values 0 - 2 configuring a
> > > + 1000ms update rate and 1 - 3% duty respective duty
> > > + increase, and 3 - 7 a 200ms update rate with a 1 -
> > > + 5% respective duty increase. Default value is 0.
> > > +- maxim,fan-no-watchdog : Do not ramp fan to 100% PWM duty on failure to
> > > + update desired fan rate inside 10s. This implies
> > > + maxim,tmp-no-fault-ramp
> > > +- maxim,tmp-no-fault-ramp: Do not ramp fan to 100% PWM duty on temperature
> > > + sensor fault detection. This implies
> > > + maxim,fan-no-watchdog
> > > +- maxim,tmp-hysteresis : The temperature hysteresis used to determine
> > > + transitions to lower fan speed bands in the
> > > + temperature/fan rate lookup table. Valid values are
> > > + 2, 4, 6 or 8 (degrees celcius). Default value is 2.
> > > +- maxim,fan-dual-tach : Enable dual tachometer functionality
> > > +- maxim,fan-pwm-freq : The PWM frequency. Valid values are 30, 50, 100, 150
> > > + and 25000 (Hz). Default value is 30Hz.
> > > +- maxim,fan-lookup-table : A 16-element cell array of alternating temperature
> > > + and rate values representing the look up table. The
> > > + rate units are set through the fan-mode property.
> > > +- maxim,fan-fault-pin-mon: Ramp fans to 100% PWM duty when the FAULT pin is
> > > + asserted
> > > +
> > > +Temperature
> > > +-----------
> > > +
> > > +Required subnode properties:
> > > +- compatible : Must be "pmbus-temperature"
> > > +- reg : The PMBus page the properties apply to.
> > > +
> > > +Optional subnode properties:
> > > +- maxim,tmp-offset : Valid values are 0 - 30 (degrees celcius) inclusive.
> > > + Default value is 0.
> > > +- maxim,tmp-fans : An array of fan indexes whose fans are controlled by
> > > + the current temperature sensor. Fans are indexed from
> > > + zero. The valid values are 0 - 5 inclusive.
> >
> >
> > Why not use a phandle here.
>
> I think that's a better approach. I'll rework it.
I've addressed this.
Cheers,
Andrew
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-09-08 3:58 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-02 7:15 [PATCH v2 0/4] pmbus: Expand fan support and add MAX31785 driver Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation Andrew Jeffery
2017-08-10 16:13 ` Rob Herring
2017-08-14 1:55 ` Andrew Jeffery
2017-09-08 3:58 ` Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 2/4] hwmon: pmbus: Add fan control support Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 3/4] pmbus: Add driver for Maxim MAX31785 Intelligent Fan Controller Andrew Jeffery
2017-08-02 7:15 ` [PATCH v2 4/4] pmbus: max31785: Work around back-to-back writes with FAN_CONFIG_1_2 Andrew Jeffery
2017-08-29 1:08 ` Andrew Jeffery
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).