Add devicetree support for max6639

max6639_platform_data is not used by any in-kernel driver and does not address the MAX6639 fans separately.
Move to device tree configuration with explicit properties to configure each fan.

Non-DT platform can still use this module with its default configuration.

Signed-off-by Marcello Sylvester Bauer <[email protected]>

Signed-off-by Naresh Solanki <[email protected]>

---
 drivers/hwmon/Kconfig   |   1 +
 drivers/hwmon/max6639.c | 465 +++++++++++++++++++++++++++++++++-------
 2 files changed, 392 insertions(+), 74 deletions(-)

drivers/hwmon/Kconfig +1 -0

index 5695b266abcf..ad1f6742ca50 100644
@@ -1106,6 +1106,7 @@ config SENSORS_MAX6621
 config SENSORS_MAX6639
 	tristate "Maxim MAX6639 sensor chip"
 	depends on I2C
+	depends on PWM
 	help
 	  If you say yes here you get support for the MAX6639
 	  sensor chips.

drivers/hwmon/max6639.c +0 -0

index 9b895402c80d..d1ae77e8f72e 100644

drivers/hwmon/max6639.c +2 -1

+#include <linux/pwm.h>
 
 /* Addresses to scan */
 static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END };

@@ -54,11 +54,20 @@ static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END };
 #define MAX6639_GCONFIG_PWM_FREQ_HI		0x08
 
 #define MAX6639_FAN_CONFIG1_PWM			0x80
-
+#define MAX6639_REG_FAN_CONFIG2a_PWM_POL	0x02

drivers/hwmon/max6639.c +126 -67

 #define FAN_FROM_REG(val, rpm_range)	((val) == 0 || (val) == 255 ? \
 				0 : (rpm_ranges[rpm_range] * 30) / (val))
 #define TEMP_LIMIT_TO_REG(val)	clamp_val((val) / 1000, 0, 255)

@@ -76,20 +85,24 @@ struct max6639_data {
 	u16 temp[2];		/* Temperature, in 1/8 C, 0..255 C */
 	bool temp_fault[2];	/* Detected temperature diode failure */
 	u8 fan[2];		/* Register value: TACH count for fans >=30 */
+	struct pwm_device	*pwmd[2]; /* max6639 has two pwm device */
+	u32 target_rpm[2];
+	u32 max_rpm[2];
+
 	u8 status;		/* Detected channel alarms and fan failures */
 
 	/* Register values only written to */
-	u8 pwm[2];		/* Register value: Duty cycle 0..120 */
 	u8 temp_therm[2];	/* THERM Temperature, 0..255 C (->_max) */
 	u8 temp_alert[2];	/* ALERT Temperature, 0..255 C (->_crit) */
 	u8 temp_ot[2];		/* OT Temperature, 0..255 C (->_emergency) */
 
 	/* Register values initialized only once */
-	u8 ppr;			/* Pulses per rotation 0..3 for 1..4 ppr */
-	u8 rpm_range;		/* Index in above rpm_ranges table */
-
+	u8 ppr[2];		/* Pulses per rotation 0..3 for 1..4 ppr */
+	u8 rpm_range[2];	/* Index in above rpm_ranges table */
 	/* Optional regulator for FAN supply */
 	struct regulator *reg;
+	/* max6639 pwm chip */
+	struct pwm_chip chip;
 };
 
 static struct max6639_data *max6639_update_device(struct device *dev)

@@ -280,8 +293,12 @@ static ssize_t pwm_show(struct device *dev, struct device_attribute *dev_attr,
 {
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
 	struct max6639_data *data = dev_get_drvdata(dev);
+	struct pwm_state state;
+
+	pwm_get_state(data->pwmd[attr->index], &state);
+
+	return sprintf(buf, "%d\n", pwm_get_relative_duty_cycle(&state, 255));
 
-	return sprintf(buf, "%d\n", data->pwm[attr->index] * 255 / 120);
 }
 
 static ssize_t pwm_store(struct device *dev,

@@ -290,9 +307,9 @@ static ssize_t pwm_store(struct device *dev,
 {
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
 	struct max6639_data *data = dev_get_drvdata(dev);
-	struct i2c_client *client = data->client;
 	unsigned long val;
 	int res;
+	struct pwm_state state;
 
 	res = kstrtoul(buf, 10, &val);
 	if (res)

@@ -300,12 +317,12 @@ static ssize_t pwm_store(struct device *dev,
 
 	val = clamp_val(val, 0, 255);
 
-	mutex_lock(&data->update_lock);
-	data->pwm[attr->index] = (u8)(val * 120 / 255);
-	i2c_smbus_write_byte_data(client,
-				  MAX6639_REG_TARGTDUTY(attr->index),
-				  data->pwm[attr->index]);
-	mutex_unlock(&data->update_lock);
+	pwm_get_state(data->pwmd[attr->index], &state);
+	pwm_set_relative_duty_cycle(&state, val, 255);
+	res = pwm_apply_state(data->pwmd[attr->index], &state);
+	if (res)
+		return res;
+
 	return count;
 }
 

@@ -319,7 +336,7 @@ static ssize_t fan_input_show(struct device *dev,
 		return PTR_ERR(data);
 
 	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[attr->index],
-		       data->rpm_range));
+		       data->rpm_range[attr->index]));
 }
 
 static ssize_t alarm_show(struct device *dev,

@@ -386,29 +403,41 @@ static struct attribute *max6639_attrs[] = {
 ATTRIBUTE_GROUPS(max6639);
 
 /*
- *  returns respective index in rpm_ranges table
- *  1 by default on invalid range
+ *  Get respective index in rpm_ranges table
  */
-static int rpm_range_to_reg(int range)
+static int rpm_range_to_index(struct device *dev, u8 *index, int rpm)
 {
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(rpm_ranges); i++) {
-		if (rpm_ranges[i] == range)
-			return i;
+	if (rpm < 0)
+		return -EINVAL;
+
+	/* Set index based on chip support */
+	switch (rpm) {
+	case 0 ... 2000:
+		*index = 0;
+		break;
+	case 2001 ... 4000:
+		*index = 1;
+		break;
+	case 4001 ... 8000:
+		*index = 2;
+		break;
+	case 8001 ... 16000:
+		*index = 3;
+		break;
+	default:
+		/* Use max range for higher RPM */
+		dev_warn(dev,
+		    "RPM higher than supported range. Default to 16000 RPM");
+		*index = 3;
 	}
-
-	return 1; /* default: 4000 RPM */
+	return 0;
 }
 
 static int max6639_init_client(struct i2c_client *client,
 			       struct max6639_data *data)
 {
-	struct max6639_platform_data *max6639_info =
-		dev_get_platdata(&client->dev);
-	int i;
-	int rpm_range = 1; /* default: 4000 RPM */
-	int err;
+	int i, err;
+	struct pwm_state state;
 
 	/* Reset chip to default values, see below for GCONFIG setup */
 	err = i2c_smbus_write_byte_data(client, MAX6639_REG_GCONFIG,

@@ -416,51 +445,29 @@ static int max6639_init_client(struct i2c_client *client,
 	if (err)
 		goto exit;
 
-	/* Fans pulse per revolution is 2 by default */
-	if (max6639_info && max6639_info->ppr > 0 &&
-			max6639_info->ppr < 5)
-		data->ppr = max6639_info->ppr;
-	else
-		data->ppr = 2;
-	data->ppr -= 1;
-
-	if (max6639_info)
-		rpm_range = rpm_range_to_reg(max6639_info->rpm_range);
-	data->rpm_range = rpm_range;
-
 	for (i = 0; i < 2; i++) {
 
 		/* Set Fan pulse per revolution */
-		err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_FAN_PPR(i),
-				data->ppr << 6);
+		err = i2c_smbus_write_byte_data(client,	MAX6639_REG_FAN_PPR(i),
+						data->ppr[i] << 6);
 		if (err)
 			goto exit;
 
 		/* Fans config PWM, RPM */
 		err = i2c_smbus_write_byte_data(client,
-			MAX6639_REG_FAN_CONFIG1(i),
-			MAX6639_FAN_CONFIG1_PWM | rpm_range);
-		if (err)
-			goto exit;
-
-		/* Fans PWM polarity high by default */
-		if (max6639_info && max6639_info->pwm_polarity == 0)
-			err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_FAN_CONFIG2a(i), 0x00);
-		else
-			err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_FAN_CONFIG2a(i), 0x02);
+						MAX6639_REG_FAN_CONFIG1(i),
+						MAX6639_FAN_CONFIG1_PWM |
+						data->rpm_range[i]);
 		if (err)
 			goto exit;
 
 		/*
-		 * /THERM full speed enable,
+		 * /THERM full speed disable,
 		 * PWM frequency 25kHz, see also GCONFIG below
 		 */
 		err = i2c_smbus_write_byte_data(client,
-			MAX6639_REG_FAN_CONFIG3(i),
-			MAX6639_FAN_CONFIG3_THERM_FULL_SPEED | 0x03);
+						MAX6639_REG_FAN_CONFIG3(i),
+						0x03);
 		if (err)
 			goto exit;
 

@@ -469,31 +476,35 @@ static int max6639_init_client(struct i2c_client *client,
 		data->temp_alert[i] = 90;
 		data->temp_ot[i] = 100;
 		err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_THERM_LIMIT(i),
-				data->temp_therm[i]);
+						MAX6639_REG_THERM_LIMIT(i),
+						data->temp_therm[i]);
 		if (err)
 			goto exit;
 		err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_ALERT_LIMIT(i),
-				data->temp_alert[i]);
+						MAX6639_REG_ALERT_LIMIT(i),
+						data->temp_alert[i]);
 		if (err)
 			goto exit;
 		err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_OT_LIMIT(i), data->temp_ot[i]);
+						MAX6639_REG_OT_LIMIT(i),
+						data->temp_ot[i]);
 		if (err)
 			goto exit;
 
-		/* PWM 120/120 (i.e. 100%) */
-		data->pwm[i] = 120;
-		err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_TARGTDUTY(i), data->pwm[i]);
+		/* Configure PWM controller */
+		pwm_get_state(data->pwmd[i], &state);
+		pwm_set_relative_duty_cycle(&state, data->target_rpm[i],
+					    data->max_rpm[i]);
+		err = pwm_apply_state(data->pwmd[i], &state);
 		if (err)
 			goto exit;
+
 	}
 	/* Start monitoring */
 	err = i2c_smbus_write_byte_data(client, MAX6639_REG_GCONFIG,
 		MAX6639_GCONFIG_DISABLE_TIMEOUT | MAX6639_GCONFIG_CH2_LOCAL |
 		MAX6639_GCONFIG_PWM_FREQ_HI);
+
 exit:
 	return err;
 }

@@ -524,12 +535,276 @@ static void max6639_regulator_disable(void *data)
 	regulator_disable(data);
 }
 
+static int max6639_probe_child_from_dt(struct i2c_client *client,
+				      struct device_node *child,
+				      struct max6639_data *data)
+
+{
+	struct device *dev = &client->dev;
+	u32 i, maxrpm;
+	int val, err;
+
+	err = of_property_read_u32(child, "reg", &i);
+	if (err) {
+		dev_err(dev, "missing reg property of %pOFn\n", child);
+		return err;
+	}
+
+	if (i >= 2) {
+		dev_err(dev, "invalid reg %d of %pOFn\n", i, child);
+		return -EINVAL;
+	}
+
+	err = of_property_read_u32(child, "pulses-per-revolution", &val);
+	if (err) {
+		dev_err(dev, "missing pulses-per-revolution property of %pOFn",
+			child);
+		return err;
+	}
+
+	if (val < 0 || val > 5) {
+		dev_err(dev, "invalid pulses-per-revolution %d of %pOFn\n", val,
+			child);
+		return -EINVAL;
+	}
+	data->ppr[i] = val;
+
+	err = of_property_read_u32(child, "max-rpm", &maxrpm);
+	if (err) {
+		dev_err(dev, "missing max-rpm property of %pOFn\n", child);
+		return err;
+	}
+
+	err = rpm_range_to_index(dev, &data->rpm_range[i], maxrpm);
+	if (err) {
+		dev_err(dev, "invalid max-rpm %d of %pOFn\n", maxrpm, child);
+		return err;
+	}
+	data->max_rpm[i] = maxrpm;
+
+	err = of_property_read_u32(child, "target-rpm", &val);
+	/* Use provided target RPM else default to maxrpm */
+	if (!err)
+		data->target_rpm[i] = val;
+	else
+		data->target_rpm[i] = maxrpm;
+
+	/* Get pwms property for PWM control */
+	data->pwmd[i] = devm_fwnode_pwm_get(dev, &child->fwnode, NULL);
+

drivers/hwmon/max6639.c +33 -0

+	dev_dbg(dev, "Failed to configure pwm for fan %d", i);
+	return PTR_ERR_OR_ZERO(data->pwmd[i]);
+}
+
+static int max6639_probe_from_dt(struct i2c_client *client,
+				 struct max6639_data *data)
+{
+	struct device *dev = &client->dev;
+	const struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int err;
+
+	/* Compatible with non-DT platforms */
+	if (!np)
+		return 0;
+
+	for_each_child_of_node(np, child) {
+		if (strcmp(child->name, "fan"))
+			continue;
+
+		err = max6639_probe_child_from_dt(client, child, data);
+		if (err) {
+			of_node_put(child);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static struct max6639_data *to_max6639_pwm(struct pwm_chip *chip)
+{
+	return container_of(chip, struct max6639_data, chip);

drivers/hwmon/max6639.c +17 -0

+	unsigned int freq;
+
+	mutex_lock(&data->update_lock);
+
+	value = i2c_smbus_read_byte_data(client, MAX6639_REG_FAN_CONFIG1(i));
+	if (value < 0)
+		goto abort;
+
+	if (value & MAX6639_FAN_CONFIG1_PWM) {
+		state->enabled = true;
+
+		/* Determine frequency from respective registers */
+		value = i2c_smbus_read_byte_data(client,
+						 MAX6639_REG_FAN_CONFIG3(i));
+		if (value < 0)
+			goto abort;
+		x = value & MAX6639_FAN_CONFIG3_FREQ_MASK;

drivers/hwmon/max6639.c +10 -0

+		value = i2c_smbus_read_byte_data(client,
+						 MAX6639_REG_FAN_CONFIG2a(i));
+		if (value < 0)
+			goto abort;
+		value &= MAX6639_REG_FAN_CONFIG2a_PWM_POL;
+		state->polarity = (value != 0);
+	} else
+		state->enabled = false;
+
+abort:

drivers/hwmon/max6639.c +10 -0

+		value = i2c_smbus_read_byte_data(client,
+						 MAX6639_REG_FAN_CONFIG3(i));
+		if (value < 0)
+			goto abort;
+		value &= ~MAX6639_FAN_CONFIG3_FREQ_MASK;
+		value |= (x & MAX6639_FAN_CONFIG3_FREQ_MASK);
+		value = i2c_smbus_write_byte_data(client,
+						  MAX6639_REG_FAN_CONFIG3(i),
+						  value);
+

drivers/hwmon/max6639.c +16 -0

+		value = i2c_smbus_write_byte_data(client,
+						  MAX6639_REG_TARGTDUTY(i),
+						  value);
+		if (value < 0)
+			goto abort;
+	}
+
+	/* Configure polarity */
+	if (state->polarity != cstate.polarity) {
+		value = i2c_smbus_read_byte_data(client,
+						 MAX6639_REG_FAN_CONFIG2a(i));
+		if (value < 0)
+			goto abort;
+		if (state->polarity == PWM_POLARITY_NORMAL)
+			value |= MAX6639_REG_FAN_CONFIG2a_PWM_POL;
+		else

drivers/hwmon/max6639.c +28 -1

+
+	value = i2c_smbus_read_byte_data(client, MAX6639_REG_FAN_CONFIG1(i));
+	if (value < 0)
+		goto abort;
+	if (state->enabled)
+		value |= MAX6639_FAN_CONFIG1_PWM;
+	else
+		value &= ~MAX6639_FAN_CONFIG1_PWM;
+
+	value = i2c_smbus_write_byte_data(client, MAX6639_REG_FAN_CONFIG1(i),
+					  value);
+	if (value < 0)
+		goto abort;
+	value = 0;
+
+abort:
+	mutex_unlock(&data->update_lock);
+
+	return value;
+}
+
+static const struct pwm_ops max6639_pwm_ops = {
+	.apply = max6639_pwm_apply,
+	.get_state = max6639_pwm_get_state,
+	.owner = THIS_MODULE,
+};
+
 static int max6639_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	struct max6639_data *data;
 	struct device *hwmon_dev;
-	int err;
+	int err, i;
 
 	data = devm_kzalloc(dev, sizeof(struct max6639_data), GFP_KERNEL);
 	if (!data)

drivers/hwmon/max6639.c +18 -0

+			return err;
+		}
 		data->reg = NULL;
 	} else {
 		/* Spin up fans */

@@ -560,6 +850,22 @@ static int max6639_probe(struct i2c_client *client)
 
 	mutex_init(&data->update_lock);
 
+	/* Below are defaults leter overridden by DT properties */
+	for (i = 0; i < 2; i++) {
+		/* 4000 RPM */
+		data->rpm_range[i] = 1;
+		data->ppr[i] = 2;
+		/* Max. temp. 80C/90C/100C */
+		data->temp_therm[i] = 80;
+		data->temp_alert[i] = 90;
+		data->temp_ot[i] = 100;
+	}
+
+	/* Probe from DT to get configuration */
+	err = max6639_probe_from_dt(client, data);
+	if (err)
+		return err;
+
 	/* Initialize the max6639 chip */
 	err = max6639_init_client(client, data);
 	if (err < 0)

drivers/hwmon/max6639.c +1 -0

 static int max6639_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);

@@ -608,6 +915,7 @@ static int max6639_resume(struct device *dev)
 	return i2c_smbus_write_byte_data(client,
 			MAX6639_REG_GCONFIG, ret & ~MAX6639_GCONFIG_STANDBY);
 }
+#endif
 
 static const struct i2c_device_id max6639_id[] = {
 	{"max6639", 0},

drivers/hwmon/max6639.c +8 -0

+static const struct of_device_id maxim_of_platform_match[] = {
+	{.compatible = "maxim,max6639"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, maxim_of_platform_match);
+#endif
+
+static SIMPLE_DEV_PM_OPS(max6639_pm_ops, max6639_suspend, max6639_resume);

Hello,

Quoted 4 lines · Naresh Solanki

max6639_platform_data is not used by any in-kernel driver and does not address the MAX6639 fans separately.
Move to device tree configuration with explicit properties to configure each fan.

My overall impression is that this patch mixes too much things. IMHO it should be split in (at least)

- Add dt support
- Drop platform support
- Add PWM provider support
- Make use of the PWM API

maybe also add the 2nd PWM in a separate step.

Some more comments inline.

Quoted 25 lines

Non-DT platform can still use this module with its default configuration.

Signed-off-by Marcello Sylvester Bauer <[email protected]>

Signed-off-by Naresh Solanki <[email protected]>

---
  drivers/hwmon/Kconfig   |   1 +
  drivers/hwmon/max6639.c | 465 +++++++++++++++++++++++++++++++++-------
  2 files changed, 392 insertions(+), 74 deletions(-)

drivers/hwmon/Kconfig +1 -0

index 5695b266abcf..ad1f6742ca50 100644
@@ -1106,6 +1106,7 @@ config SENSORS_MAX6621
  config SENSORS_MAX6639
  	tristate "Maxim MAX6639 sensor chip"
  	depends on I2C
+	depends on PWM
  	help
  	  If you say yes here you get support for the MAX6639
  	  sensor chips.

drivers/hwmon/max6639.c +0 -1

index 9b895402c80d..d1ae77e8f72e 100644
--- a/drivers/hwmon/max6639.c

drivers/hwmon/max6639.c line 22

Quoted 6 lines

+++ b/drivers/hwmon/max6639.c

drivers/hwmon/max6639.c +0 -1

@@ -19,7 +19,7 @@
  #include <linux/hwmon-sysfs.h>
  #include <linux/err.h>
  #include <linux/mutex.h>
-#include <linux/platform_data/max6639.h>

This file is now unused and can be dropped.

Quoted 18 lines

drivers/hwmon/max6639.c +5 -1

+#include <linux/pwm.h>
 
  /* Addresses to scan */
  static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END };

@@ -54,11 +54,20 @@ static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END };
  #define MAX6639_GCONFIG_PWM_FREQ_HI		0x08
 
  #define MAX6639_FAN_CONFIG1_PWM			0x80
-
+#define MAX6639_REG_FAN_CONFIG2a_PWM_POL	0x02
  #define MAX6639_FAN_CONFIG3_THERM_FULL_SPEED	0x40
+#define MAX6639_FAN_CONFIG3_FREQ_MASK		0x03
+#define MAX6639_REG_TARGTDUTY_SLOT		120
 
+/* Tach supported range. This internally controls tach frequency */
  static const int rpm_ranges[] = { 2000, 4000, 8000, 16000 };
 

drivers/hwmon/max6639.c static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END }; · line 67

Quoted 3 lines

drivers/hwmon/max6639.c +3 -0

+/* Supported PWM frequency */
+static const unsigned int freq_table[] = { 20, 33, 50, 100, 5000, 8333, 12500,
+					25000 };

I would have put these in a single line, or one line per freq, or at least aligned the last value such that it is at a higher column than the opening {.

drivers/hwmon/max6639.c static const unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END }; · line 70

Quoted 4 lines

drivers/hwmon/max6639.c +3 -0

+
+
+

Three empty lines? One or two only should do it.

Quoted 314 lines

drivers/hwmon/max6639.c +131 -67

  #define FAN_FROM_REG(val, rpm_range)	((val) == 0 || (val) == 255 ? \
  				0 : (rpm_ranges[rpm_range] * 30) / (val))
  #define TEMP_LIMIT_TO_REG(val)	clamp_val((val) / 1000, 0, 255)

@@ -76,20 +85,24 @@ struct max6639_data {
  	u16 temp[2];		/* Temperature, in 1/8 C, 0..255 C */
  	bool temp_fault[2];	/* Detected temperature diode failure */
  	u8 fan[2];		/* Register value: TACH count for fans >=30 */
+	struct pwm_device	*pwmd[2]; /* max6639 has two pwm device */
+	u32 target_rpm[2];
+	u32 max_rpm[2];
+
  	u8 status;		/* Detected channel alarms and fan failures */
 
  	/* Register values only written to */
-	u8 pwm[2];		/* Register value: Duty cycle 0..120 */
  	u8 temp_therm[2];	/* THERM Temperature, 0..255 C (->_max) */
  	u8 temp_alert[2];	/* ALERT Temperature, 0..255 C (->_crit) */
  	u8 temp_ot[2];		/* OT Temperature, 0..255 C (->_emergency) */
 
  	/* Register values initialized only once */
-	u8 ppr;			/* Pulses per rotation 0..3 for 1..4 ppr */
-	u8 rpm_range;		/* Index in above rpm_ranges table */
-
+	u8 ppr[2];		/* Pulses per rotation 0..3 for 1..4 ppr */
+	u8 rpm_range[2];	/* Index in above rpm_ranges table */
  	/* Optional regulator for FAN supply */
  	struct regulator *reg;
+	/* max6639 pwm chip */
+	struct pwm_chip chip;
  };
 
  static struct max6639_data *max6639_update_device(struct device *dev)

@@ -280,8 +293,12 @@ static ssize_t pwm_show(struct device *dev, struct device_attribute *dev_attr,
  {
  	struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
  	struct max6639_data *data = dev_get_drvdata(dev);
+	struct pwm_state state;
+
+	pwm_get_state(data->pwmd[attr->index], &state);
+
+	return sprintf(buf, "%d\n", pwm_get_relative_duty_cycle(&state, 255));
 
-	return sprintf(buf, "%d\n", data->pwm[attr->index] * 255 / 120);
  }
 
  static ssize_t pwm_store(struct device *dev,

@@ -290,9 +307,9 @@ static ssize_t pwm_store(struct device *dev,
  {
  	struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr);
  	struct max6639_data *data = dev_get_drvdata(dev);
-	struct i2c_client *client = data->client;
  	unsigned long val;
  	int res;
+	struct pwm_state state;
 
  	res = kstrtoul(buf, 10, &val);
  	if (res)

@@ -300,12 +317,12 @@ static ssize_t pwm_store(struct device *dev,
 
  	val = clamp_val(val, 0, 255);
 
-	mutex_lock(&data->update_lock);
-	data->pwm[attr->index] = (u8)(val * 120 / 255);
-	i2c_smbus_write_byte_data(client,
-				  MAX6639_REG_TARGTDUTY(attr->index),
-				  data->pwm[attr->index]);
-	mutex_unlock(&data->update_lock);
+	pwm_get_state(data->pwmd[attr->index], &state);
+	pwm_set_relative_duty_cycle(&state, val, 255);
+	res = pwm_apply_state(data->pwmd[attr->index], &state);
+	if (res)
+		return res;
+
  	return count;
  }
 

@@ -319,7 +336,7 @@ static ssize_t fan_input_show(struct device *dev,
  		return PTR_ERR(data);
 
  	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[attr->index],
-		       data->rpm_range));
+		       data->rpm_range[attr->index]));
  }
 
  static ssize_t alarm_show(struct device *dev,

@@ -386,29 +403,41 @@ static struct attribute *max6639_attrs[] = {
  ATTRIBUTE_GROUPS(max6639);
 
  /*
- *  returns respective index in rpm_ranges table
- *  1 by default on invalid range
+ *  Get respective index in rpm_ranges table
   */
-static int rpm_range_to_reg(int range)
+static int rpm_range_to_index(struct device *dev, u8 *index, int rpm)
  {
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(rpm_ranges); i++) {
-		if (rpm_ranges[i] == range)
-			return i;
+	if (rpm < 0)
+		return -EINVAL;
+
+	/* Set index based on chip support */
+	switch (rpm) {
+	case 0 ... 2000:
+		*index = 0;
+		break;
+	case 2001 ... 4000:
+		*index = 1;
+		break;
+	case 4001 ... 8000:
+		*index = 2;
+		break;
+	case 8001 ... 16000:
+		*index = 3;
+		break;
+	default:
+		/* Use max range for higher RPM */
+		dev_warn(dev,
+		    "RPM higher than supported range. Default to 16000 RPM");
+		*index = 3;
  	}
-
-	return 1; /* default: 4000 RPM */
+	return 0;
  }
 
  static int max6639_init_client(struct i2c_client *client,
  			       struct max6639_data *data)
  {
-	struct max6639_platform_data *max6639_info =
-		dev_get_platdata(&client->dev);
-	int i;
-	int rpm_range = 1; /* default: 4000 RPM */
-	int err;
+	int i, err;
+	struct pwm_state state;
 
  	/* Reset chip to default values, see below for GCONFIG setup */
  	err = i2c_smbus_write_byte_data(client, MAX6639_REG_GCONFIG,

@@ -416,51 +445,29 @@ static int max6639_init_client(struct i2c_client *client,
  	if (err)
  		goto exit;
 
-	/* Fans pulse per revolution is 2 by default */
-	if (max6639_info && max6639_info->ppr > 0 &&
-			max6639_info->ppr < 5)
-		data->ppr = max6639_info->ppr;
-	else
-		data->ppr = 2;
-	data->ppr -= 1;
-
-	if (max6639_info)
-		rpm_range = rpm_range_to_reg(max6639_info->rpm_range);
-	data->rpm_range = rpm_range;
-
  	for (i = 0; i < 2; i++) {
 
  		/* Set Fan pulse per revolution */
-		err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_FAN_PPR(i),
-				data->ppr << 6);
+		err = i2c_smbus_write_byte_data(client,	MAX6639_REG_FAN_PPR(i),
+						data->ppr[i] << 6);
  		if (err)
  			goto exit;
 
  		/* Fans config PWM, RPM */
  		err = i2c_smbus_write_byte_data(client,
-			MAX6639_REG_FAN_CONFIG1(i),
-			MAX6639_FAN_CONFIG1_PWM | rpm_range);
-		if (err)
-			goto exit;
-
-		/* Fans PWM polarity high by default */
-		if (max6639_info && max6639_info->pwm_polarity == 0)
-			err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_FAN_CONFIG2a(i), 0x00);
-		else
-			err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_FAN_CONFIG2a(i), 0x02);
+						MAX6639_REG_FAN_CONFIG1(i),
+						MAX6639_FAN_CONFIG1_PWM |
+						data->rpm_range[i]);
  		if (err)
  			goto exit;
 
  		/*
-		 * /THERM full speed enable,
+		 * /THERM full speed disable,
  		 * PWM frequency 25kHz, see also GCONFIG below
  		 */
  		err = i2c_smbus_write_byte_data(client,
-			MAX6639_REG_FAN_CONFIG3(i),
-			MAX6639_FAN_CONFIG3_THERM_FULL_SPEED | 0x03);
+						MAX6639_REG_FAN_CONFIG3(i),
+						0x03);
  		if (err)
  			goto exit;
 

@@ -469,31 +476,35 @@ static int max6639_init_client(struct i2c_client *client,
  		data->temp_alert[i] = 90;
  		data->temp_ot[i] = 100;
  		err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_THERM_LIMIT(i),
-				data->temp_therm[i]);
+						MAX6639_REG_THERM_LIMIT(i),
+						data->temp_therm[i]);
  		if (err)
  			goto exit;
  		err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_ALERT_LIMIT(i),
-				data->temp_alert[i]);
+						MAX6639_REG_ALERT_LIMIT(i),
+						data->temp_alert[i]);
  		if (err)
  			goto exit;
  		err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_OT_LIMIT(i), data->temp_ot[i]);
+						MAX6639_REG_OT_LIMIT(i),
+						data->temp_ot[i]);
  		if (err)
  			goto exit;
 
-		/* PWM 120/120 (i.e. 100%) */
-		data->pwm[i] = 120;
-		err = i2c_smbus_write_byte_data(client,
-				MAX6639_REG_TARGTDUTY(i), data->pwm[i]);
+		/* Configure PWM controller */
+		pwm_get_state(data->pwmd[i], &state);
+		pwm_set_relative_duty_cycle(&state, data->target_rpm[i],
+					    data->max_rpm[i]);
+		err = pwm_apply_state(data->pwmd[i], &state);
  		if (err)
  			goto exit;
+
  	}
  	/* Start monitoring */
  	err = i2c_smbus_write_byte_data(client, MAX6639_REG_GCONFIG,
  		MAX6639_GCONFIG_DISABLE_TIMEOUT | MAX6639_GCONFIG_CH2_LOCAL |
  		MAX6639_GCONFIG_PWM_FREQ_HI);
+
  exit:
  	return err;
  }

@@ -524,12 +535,276 @@ static void max6639_regulator_disable(void *data)
  	regulator_disable(data);
  }
 
+static int max6639_probe_child_from_dt(struct i2c_client *client,
+				      struct device_node *child,
+				      struct max6639_data *data)
+
+{
+	struct device *dev = &client->dev;
+	u32 i, maxrpm;
+	int val, err;
+
+	err = of_property_read_u32(child, "reg", &i);
+	if (err) {
+		dev_err(dev, "missing reg property of %pOFn\n", child);
+		return err;
+	}
+
+	if (i >= 2) {
+		dev_err(dev, "invalid reg %d of %pOFn\n", i, child);
+		return -EINVAL;
+	}
+
+	err = of_property_read_u32(child, "pulses-per-revolution", &val);
+	if (err) {
+		dev_err(dev, "missing pulses-per-revolution property of %pOFn",
+			child);
+		return err;
+	}
+
+	if (val < 0 || val > 5) {
+		dev_err(dev, "invalid pulses-per-revolution %d of %pOFn\n", val,
+			child);
+		return -EINVAL;
+	}
+	data->ppr[i] = val;
+
+	err = of_property_read_u32(child, "max-rpm", &maxrpm);
+	if (err) {
+		dev_err(dev, "missing max-rpm property of %pOFn\n", child);
+		return err;
+	}
+
+	err = rpm_range_to_index(dev, &data->rpm_range[i], maxrpm);
+	if (err) {
+		dev_err(dev, "invalid max-rpm %d of %pOFn\n", maxrpm, child);
+		return err;
+	}
+	data->max_rpm[i] = maxrpm;
+
+	err = of_property_read_u32(child, "target-rpm", &val);
+	/* Use provided target RPM else default to maxrpm */
+	if (!err)
+		data->target_rpm[i] = val;
+	else
+		data->target_rpm[i] = maxrpm;
+
+	/* Get pwms property for PWM control */
+	data->pwmd[i] = devm_fwnode_pwm_get(dev, &child->fwnode, NULL);
+
+	if (!IS_ERR(data->pwmd[i]))
+		return 0;
+
+	if (PTR_ERR(data->pwmd[i]) == -EPROBE_DEFER)
+		return PTR_ERR(data->pwmd[i]);

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 604

Quoted 5 lines

drivers/hwmon/max6639.c +5 -0

+
+	dev_dbg(dev, "Using chip default PWM");
+	data->pwmd[i] = pwm_request_from_chip(&data->chip, i, NULL);
+	if (!IS_ERR(data->pwmd[i]))
+		return 0;

Are these PWMs usuable at all for a consumer other than the driver itself? If not I'd doubt the added value of this patch.

Quoted 39 lines

drivers/hwmon/max6639.c +38 -0

+	dev_dbg(dev, "Failed to configure pwm for fan %d", i);
+	return PTR_ERR_OR_ZERO(data->pwmd[i]);
+}
+
+static int max6639_probe_from_dt(struct i2c_client *client,
+				 struct max6639_data *data)
+{
+	struct device *dev = &client->dev;
+	const struct device_node *np = dev->of_node;
+	struct device_node *child;
+	int err;
+
+	/* Compatible with non-DT platforms */
+	if (!np)
+		return 0;
+
+	for_each_child_of_node(np, child) {
+		if (strcmp(child->name, "fan"))
+			continue;
+
+		err = max6639_probe_child_from_dt(client, child, data);
+		if (err) {
+			of_node_put(child);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static struct max6639_data *to_max6639_pwm(struct pwm_chip *chip)
+{
+	return container_of(chip, struct max6639_data, chip);
+}
+
+static void max6639_pwm_get_state(struct pwm_chip *chip,
+				  struct pwm_device *pwm,
+				  struct pwm_state *state)

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 648

Quoted 5 lines

drivers/hwmon/max6639.c +5 -0

+{
+
+	struct max6639_data *data = to_max6639_pwm(chip);
+	struct i2c_client *client = data->client;
+	int value, i = pwm->hwpwm, x;

Please use pwm->hwpwm directly instead of i, this makes the code IMHO a bit more readable.

Quoted 23 lines

drivers/hwmon/max6639.c +22 -0

+	unsigned int freq;
+
+	mutex_lock(&data->update_lock);
+
+	value = i2c_smbus_read_byte_data(client, MAX6639_REG_FAN_CONFIG1(i));
+	if (value < 0)
+		goto abort;
+
+	if (value & MAX6639_FAN_CONFIG1_PWM) {
+		state->enabled = true;
+
+		/* Determine frequency from respective registers */
+		value = i2c_smbus_read_byte_data(client,
+						 MAX6639_REG_FAN_CONFIG3(i));
+		if (value < 0)
+			goto abort;
+		x = value & MAX6639_FAN_CONFIG3_FREQ_MASK;
+
+		value = i2c_smbus_read_byte_data(client, MAX6639_REG_GCONFIG);
+		if (value < 0)
+			goto abort;
+		if (value & MAX6639_GCONFIG_PWM_FREQ_HI)

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 675

Quoted 5 lines

drivers/hwmon/max6639.c +5 -0

+			x |= 0x4;
+		x &= 0x7;
+		freq = freq_table[x];
+
+		state->period = DIV_ROUND_UP_ULL(NSEC_PER_SEC, freq);

both NSEC_PER_SEC and freq fit into an unsigned int, so you can use the (cheaper) DIV_ROUND_UP here.

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 683

Quoted 9 lines

drivers/hwmon/max6639.c +8 -0

+
+		value = i2c_smbus_read_byte_data(client,
+						 MAX6639_REG_TARGTDUTY(i));
+		if (value < 0)
+			goto abort;
+		/* max6639 supports 120 slots only */
+		state->duty_cycle = DIV_ROUND_UP_ULL(value * state->period,
+						     120);

s/120/MAX6639_REG_TARGTDUTY_SLOT/ ?

value * state->period might overflow. Use something like mul_u64_u32_div. (Hmm, there doesn't seem to be a variant for unsigned * unsigned / unsigned?)

You're loosing precision here. Consider freq = 8333 and TARGTDUTY = 97 then you have state->period = 120005 and calulate duty_cycle as:

	DIV_ROUND_UP_ULL(97 * 120005, 120) = 97005

. The exact value is (I guess)

	97 * NSEC_PER_SEC / (8333 * 120) = 97003.880

Quoted 15 lines

drivers/hwmon/max6639.c +15 -0

+		value = i2c_smbus_read_byte_data(client,
+						 MAX6639_REG_FAN_CONFIG2a(i));
+		if (value < 0)
+			goto abort;
+		value &= MAX6639_REG_FAN_CONFIG2a_PWM_POL;
+		state->polarity = (value != 0);
+	} else
+		state->enabled = false;
+
+abort:
+	mutex_unlock(&data->update_lock);
+
+}
+
+static int max6639_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 704

Quoted 5 lines

drivers/hwmon/max6639.c +5 -0

+			     const struct pwm_state *state)
+{
+	struct max6639_data *data = to_max6639_pwm(chip);
+	struct i2c_client *client = data->client;
+	int value = 0, i = pwm->hwpwm, x;

Please use pwm->hwpwm directly instead of i, this makes the code IMHO a bit more readable.

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 708

Quoted 5 lines

drivers/hwmon/max6639.c +4 -0

+	unsigned int freq;
+	struct pwm_state cstate;
+
+	pwm_get_state(pwm, &cstate);

Please don't use pwm API functions in PWM callbacks. Accessing pwm->state is fine here.

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 718

Quoted 10 lines

drivers/hwmon/max6639.c +9 -0

+	mutex_lock(&data->update_lock);
+
+	if (state->period != cstate.period) {
+		/* Configure frequency */
+		freq = DIV_ROUND_UP_ULL(NSEC_PER_SEC, state->period);
+		/* Chip supports limited number of frequency */
+		for (x = 0; x < sizeof(freq_table); x++)
+			if (freq < freq_table[x])
+				break;

For state->period = 80000 we get:

	freq = 12500

and then x = 7 is picked while x = 6 would be an exact match.

I think you just need

	if (freq <= freq_table[x])

Quoted 15 lines

drivers/hwmon/max6639.c +15 -0

+		value = i2c_smbus_read_byte_data(client,
+						 MAX6639_REG_FAN_CONFIG3(i));
+		if (value < 0)
+			goto abort;
+		value &= ~MAX6639_FAN_CONFIG3_FREQ_MASK;
+		value |= (x & MAX6639_FAN_CONFIG3_FREQ_MASK);
+		value = i2c_smbus_write_byte_data(client,
+						  MAX6639_REG_FAN_CONFIG3(i),
+						  value);
+
+		value = i2c_smbus_read_byte_data(client, MAX6639_REG_GCONFIG);
+		if (value < 0)
+			goto abort;
+
+		if ((value & MAX6639_GCONFIG_PWM_FREQ_HI) && (x >> 2))

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 739

Quoted 5 lines

drivers/hwmon/max6639.c +5 -0

+			value &= ~MAX6639_GCONFIG_PWM_FREQ_HI;
+		else
+			value |= MAX6639_GCONFIG_PWM_FREQ_HI;
+		value = i2c_smbus_write_byte_data(client, MAX6639_REG_GCONFIG,
+						  value);

Why does it depend on MAX6639_GCONFIG_PWM_FREQ_HI being already set if you need to set it?

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 745

Quoted 6 lines

drivers/hwmon/max6639.c +6 -0

+		if (value < 0)
+			goto abort;
+	}
+
+	/* Configure dutycycle */
+	if (state->duty_cycle != cstate.duty_cycle) {

That check is wrong if you go from

	.duty_cycle = 50000; .period = 200000

to

	.duty_cycle = 50000; .period = 100000

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 748

Quoted 3 lines

drivers/hwmon/max6639.c +3 -0

+		value = DIV_ROUND_UP_ULL(
+				state->duty_cycle * MAX6639_REG_TARGTDUTY_SLOT,
+				state->period);

Please round down here. (And test with PWM_DEBUG which might have told you given the right tests.) Also please use the real period to determine the register value. (Otherwise for a request with

	.duty_cycle = 40000; .period = 150000

where a real period of 120004.8 ns is picked you get MAX6639_REG_TARGTDUTY_SLOT = 32 which results in a duty cycle of 32001.28 ns, while with MAX6639_REG_TARGTDUTY_SLOT = 39 you'd get 39001.56 ns.

Quoted 16 lines

drivers/hwmon/max6639.c +16 -0

+		value = i2c_smbus_write_byte_data(client,
+						  MAX6639_REG_TARGTDUTY(i),
+						  value);
+		if (value < 0)
+			goto abort;
+	}
+
+	/* Configure polarity */
+	if (state->polarity != cstate.polarity) {
+		value = i2c_smbus_read_byte_data(client,
+						 MAX6639_REG_FAN_CONFIG2a(i));
+		if (value < 0)
+			goto abort;
+		if (state->polarity == PWM_POLARITY_NORMAL)
+			value |= MAX6639_REG_FAN_CONFIG2a_PWM_POL;
+		else

drivers/hwmon/max6639.c static void max6639_regulator_disable(void *data) · line 774

Quoted 10 lines

drivers/hwmon/max6639.c +10 -0

+			value &= ~MAX6639_REG_FAN_CONFIG2a_PWM_POL;
+		value = i2c_smbus_write_byte_data(client,
+						  MAX6639_REG_FAN_CONFIG2a(i),
+						  value);
+		if (value < 0)
+			goto abort;
+	}
+
+	if (state->enabled == cstate.enabled)
+		goto abort;

Please use goto only for error handling.

Quoted 41 lines

Quoted 1 line

+

drivers/hwmon/max6639.c +27 -1

+	value = i2c_smbus_read_byte_data(client, MAX6639_REG_FAN_CONFIG1(i));
+	if (value < 0)
+		goto abort;
+	if (state->enabled)
+		value |= MAX6639_FAN_CONFIG1_PWM;
+	else
+		value &= ~MAX6639_FAN_CONFIG1_PWM;
+
+	value = i2c_smbus_write_byte_data(client, MAX6639_REG_FAN_CONFIG1(i),
+					  value);
+	if (value < 0)
+		goto abort;
+	value = 0;
+
+abort:
+	mutex_unlock(&data->update_lock);
+
+	return value;
+}
+
+static const struct pwm_ops max6639_pwm_ops = {
+	.apply = max6639_pwm_apply,
+	.get_state = max6639_pwm_get_state,
+	.owner = THIS_MODULE,
+};
+
  static int max6639_probe(struct i2c_client *client)
  {
  	struct device *dev = &client->dev;
  	struct max6639_data *data;
  	struct device *hwmon_dev;
-	int err;
+	int err, i;
 
  	data = devm_kzalloc(dev, sizeof(struct max6639_data), GFP_KERNEL);
  	if (!data)

@@ -537,11 +812,26 @@ static int max6639_probe(struct i2c_client *client)
 
  	data->client = client;
 

drivers/hwmon/max6639.c static int max6639_probe(struct i2c_client *client) · line 819

Quoted 5 lines

drivers/hwmon/max6639.c +5 -0

+	/* Add PWM controller of max6639 */
+	data->chip.dev = dev;
+	data->chip.ops = &max6639_pwm_ops;
+	data->chip.npwm = 2;
+	data->chip.of_pwm_n_cells = 3;

Please drop this, of_pwmchip_add() overwrites it anyhow.

drivers/hwmon/max6639.c static int max6639_probe(struct i2c_client *client) · line 823

Quoted 4 lines

drivers/hwmon/max6639.c +4 -0

+
+	err = devm_pwmchip_add(dev, &data->chip);
+	if (err < 0) {
+		dev_err(dev, "failed to add PWM chip %d\n", err);

dev_err_probe please.

Quoted 6 lines

drivers/hwmon/max6639.c +3 -1

+		return err;
+	}
+
  	data->reg = devm_regulator_get_optional(dev, "fan");
  	if (IS_ERR(data->reg)) {
-		if (PTR_ERR(data->reg) != -ENODEV)

drivers/hwmon/max6639.c static int max6639_probe(struct i2c_client *client) · line 832

Quoted 6 lines

drivers/hwmon/max6639.c +4 -2

-			return PTR_ERR(data->reg);
-
+		if (PTR_ERR(data->reg) != -ENODEV) {
+			err = (int)PTR_ERR(data->reg);
+			dev_warn(dev, "Failed looking up fan supply: %d\n",
+				 err);

unrelated change. Also dev_probe_err is better suited here.

Quoted 32 lines

drivers/hwmon/max6639.c +18 -0

+			return err;
+		}
  		data->reg = NULL;
  	} else {
  		/* Spin up fans */

@@ -560,6 +850,22 @@ static int max6639_probe(struct i2c_client *client)
 
  	mutex_init(&data->update_lock);
 
+	/* Below are defaults leter overridden by DT properties */
+	for (i = 0; i < 2; i++) {
+		/* 4000 RPM */
+		data->rpm_range[i] = 1;
+		data->ppr[i] = 2;
+		/* Max. temp. 80C/90C/100C */
+		data->temp_therm[i] = 80;
+		data->temp_alert[i] = 90;
+		data->temp_ot[i] = 100;
+	}
+
+	/* Probe from DT to get configuration */
+	err = max6639_probe_from_dt(client, data);
+	if (err)
+		return err;
+
  	/* Initialize the max6639 chip */
  	err = max6639_init_client(client, data);
  	if (err < 0)

@@ -571,6 +877,7 @@ static int max6639_probe(struct i2c_client *client)
  	return PTR_ERR_OR_ZERO(hwmon_dev);
  }
 

drivers/hwmon/max6639.c static int max6639_probe(struct i2c_client *client) · line 880

Quoted 1 line

drivers/hwmon/max6639.c +1 -0

+#if IS_ENABLED(CONFIG_PM_SLEEP)

unrelated change

Quoted 12 lines

drivers/hwmon/max6639.c +1 -0

  static int max6639_suspend(struct device *dev)
  {
  	struct i2c_client *client = to_i2c_client(dev);

@@ -608,6 +915,7 @@ static int max6639_resume(struct device *dev)
  	return i2c_smbus_write_byte_data(client,
  			MAX6639_REG_GCONFIG, ret & ~MAX6639_GCONFIG_STANDBY);
  }
+#endif
 
  static const struct i2c_device_id max6639_id[] = {
  	{"max6639", 0},

drivers/hwmon/max6639.c static const struct i2c_device_id max6639_id[] = { · line 927

Quoted 5 lines

drivers/hwmon/max6639.c +1 -1

@@ -616,13 +924,22 @@ static const struct i2c_device_id max6639_id[] = {
 
  MODULE_DEVICE_TABLE(i2c, max6639_id);
 
-static DEFINE_SIMPLE_DEV_PM_OPS(max6639_pm_ops, max6639_suspend, max6639_resume);
+#ifdef CONFIG_OF

Note that ACPI also uses of_match_table.

Quoted 9 lines

drivers/hwmon/max6639.c +8 -0

+static const struct of_device_id maxim_of_platform_match[] = {
+	{.compatible = "maxim,max6639"},
+	{},
+};
+MODULE_DEVICE_TABLE(of, maxim_of_platform_match);
+#endif
+
+static SIMPLE_DEV_PM_OPS(max6639_pm_ops, max6639_suspend, max6639_resume);
 

drivers/hwmon/max6639.c static const struct i2c_device_id max6639_id[] = { · line 945

Quoted 9 lines

drivers/hwmon/max6639.c +1 -0

  static struct i2c_driver max6639_driver = {
  	.class = I2C_CLASS_HWMON,
  	.driver = {
  		   .name = "max6639",
  		   .pm = pm_sleep_ptr(&max6639_pm_ops),
+		   .of_match_table = of_match_ptr(maxim_of_platform_match),
  		   },
  	.probe_new = max6639_probe,
  	.id_table = max6639_id,

Best regards
Uwe