[PATCH v2 0/4] Add support for variable sample time in lm75 driver

[PATCH v2 0/4] Add support for variable sample time in lm75 driver

[Iker Perez]

From: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>

Hello,

The objective of following patch series is to add support to lm75 driver
to be able to configure the sample time of it's supported devices,
particularly the tmp75b.

The applied changes involve:
	* Replace the current switch-case method for configuration
	  parameters selection to a structure storing them. This method
	  allows easier management of the parameters.
	* Split the writing of configuration registers into a separate
	  function. This method saves code in later patches.
	* Include new fields in lm75_params to add support for multiple
	  sample times.
	* Split the lm75_write functionality into separate, simpler,
	  functions.
	* Add support for configuring the devices via their sysfs nodes.

The patch series was based on linux-next's master branch.

Thank you Guenter Roeck, Michael Drake, Thomas Preston and Tom Eccles for
your time, help and feedback.

Regards,

Iker Perez del Palomar Sustatxa

Iker Perez del Palomar Sustatxa (4):
  hwmon: (lm75) Create structure to save all the configuration
    parameters.
  hwmon: (lm75) Create function from code to write into registers
  hwmon: (lm75) Add new fields into lm75_params_
  hwmon: (lm75) Modularize lm75_write and make hwmon_chip writable

 drivers/hwmon/lm75.c | 390 ++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 274 insertions(+), 116 deletions(-)

-- 
2.11.0

[PATCH v2 1/4] hwmon: (lm75) Create structure to save all the configuration parameters.

[Iker Perez]

From: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>

* Add to lm75_data kind field to store the kind of device the driver is
  working with.
* Add an structure to store the configuration parameters of all the
  supported devices.
* Delete resolution_limits from lm75_data and include them in the structure
  described above.
* Add a pointer to the configuration parameters structure to be used as a
  reference to obtain the parameters.
* Delete switch-case approach to get the device configuration parameters.
* The structure is cleaner and easier to maintain.

Signed-off-by: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>
---

Changes since v1:
- In the lm75_params structure documentation there have been the next changes:
        - @resolution has been corrected to @default_resolution.
        - @resolution_limits description has been corrected.
- pct2075 device's parameters has been included in device_params structure.

 drivers/hwmon/lm75.c | 282 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 176 insertions(+), 106 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index a2d3f2ce3e1d..65a1131bfa7e 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -18,7 +18,6 @@
 #include <linux/regmap.h>
 #include "lm75.h"
 
-
 /*
  * This driver handles the LM75 and compatible digital temperature sensors.
  */
@@ -51,6 +50,28 @@ enum lm75_type {		/* keep sorted in alphabetical order */
 	tmp75c,
 };
 
+/**
+ * struct lm75_params - lm75 configuration parameters.
+ * @set_mask:		Bits to set in configuration register when configuring
+ *			the chip.
+ * @clr_mask:		Bits to clear in configuration register when configuring
+ *			the chip.
+ * @default_resolution:	Default number of bits to represent the temperature
+ *			value.
+ * @resolution_limits:	Limit register resolution. Optional. Should be set if
+ *			the resolution of limit registers does not match the
+ *			resolution of the temperature register.
+ * default_sample_time:	Sample time to be set by default.
+ */
+
+struct lm75_params {
+	u8		set_mask;
+	u8		clr_mask;
+	u8		default_resolution;
+	u8		resolution_limits;
+	unsigned int	default_sample_time;
+};
+
 /* Addresses scanned */
 static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c,
 					0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
@@ -63,15 +84,151 @@ static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c,
 
 /* Each client has this additional data */
 struct lm75_data {
-	struct i2c_client	*client;
-	struct regmap		*regmap;
-	u8			orig_conf;
-	u8			resolution;	/* In bits, between 9 and 16 */
-	u8			resolution_limits;
-	unsigned int		sample_time;	/* In ms */
+	struct i2c_client		*client;
+	struct regmap			*regmap;
+	u8				orig_conf;
+	u8				current_conf;
+	u8				resolution;	/* In bits, 9 to 16 */
+	unsigned int			sample_time;	/* In ms */
+	enum lm75_type			kind;
+	const struct lm75_params	*params;
 };
 
 /*-----------------------------------------------------------------------*/
+/* The structure below stores the configuration values of the supported devices.
+ * In case of being supported multiple configurations, the default one must
+ * always be the first element of the array
+ */
+static const struct lm75_params device_params[] = {
+	[adt75] = {
+		.clr_mask = 1 << 5,	/* not one-shot mode */
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC / 8,
+	},
+	[ds1775] = {
+		.clr_mask = 3 << 5,
+		.set_mask = 2 << 5,	/* 11-bit mode */
+		.default_resolution = 11,
+		.default_sample_time = MSEC_PER_SEC,
+	},
+	[ds75] = {
+		.clr_mask = 3 << 5,
+		.set_mask = 2 << 5,	/* 11-bit mode */
+		.default_resolution = 11,
+		.default_sample_time = MSEC_PER_SEC,
+	},
+	[stds75] = {
+		.clr_mask = 3 << 5,
+		.set_mask = 2 << 5,	/* 11-bit mode */
+		.default_resolution = 11,
+		.default_sample_time = MSEC_PER_SEC,
+	},
+	[stlm75] = {
+		.default_resolution = 9,
+		.default_sample_time = MSEC_PER_SEC / 5,
+	},
+	[ds7505] = {
+		.set_mask = 3 << 5,	/* 12-bit mode*/
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC / 4,
+	},
+	[g751] = {
+		.default_resolution = 9,
+		.default_sample_time = MSEC_PER_SEC / 2,
+	},
+	[lm75] = {
+		.default_resolution = 9,
+		.default_sample_time = MSEC_PER_SEC / 2,
+	},
+	[lm75a] = {
+		.default_resolution = 9,
+		.default_sample_time = MSEC_PER_SEC / 2,
+	},
+	[lm75b] = {
+		.default_resolution = 11,
+		.default_sample_time = MSEC_PER_SEC / 4,
+	},
+	[max6625] = {
+		.default_resolution = 9,
+		.default_sample_time = MSEC_PER_SEC / 4,
+	},
+	[max6626] = {
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC / 4,
+		.resolution_limits = 9,
+	},
+	[max31725] = {
+		.default_resolution = 16,
+		.default_sample_time = MSEC_PER_SEC / 8,
+	},
+	[tcn75] = {
+		.default_resolution = 9,
+		.default_sample_time = MSEC_PER_SEC / 8,
+	},
+	[pct2075] = {
+		.default_resolution = 11,
+		.default_sample_time = MSEC_PER_SEC / 10,
+	},
+	[mcp980x] = {
+		.set_mask = 3 << 5,	/* 12-bit mode */
+		.clr_mask = 1 << 7,	/* not one-shot mode */
+		.default_resolution = 12,
+		.resolution_limits = 9,
+		.default_sample_time = MSEC_PER_SEC,
+	},
+	[tmp100] = {
+		.set_mask = 3 << 5,	/* 12-bit mode */
+		.clr_mask = 1 << 7,	/* not one-shot mode */
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC,
+	},
+	[tmp101] = {
+		.set_mask = 3 << 5,	/* 12-bit mode */
+		.clr_mask = 1 << 7,	/* not one-shot mode */
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC,
+	},
+	[tmp112] = {
+		.set_mask = 3 << 5,	/* 12-bit mode */
+		.clr_mask = 1 << 7,	/* no one-shot mode*/
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC / 4,
+	},
+	[tmp105] = {
+		.set_mask = 3 << 5,	/* 12-bit mode */
+		.clr_mask = 1 << 7,	/* not one-shot mode*/
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC / 2,
+	},
+	[tmp175] = {
+		.set_mask = 3 << 5,	/* 12-bit mode */
+		.clr_mask = 1 << 7,	/* not one-shot mode*/
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC / 2,
+	},
+	[tmp275] = {
+		.set_mask = 3 << 5,	/* 12-bit mode */
+		.clr_mask = 1 << 7,	/* not one-shot mode*/
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC / 2,
+	},
+	[tmp75] = {
+		.set_mask = 3 << 5,	/* 12-bit mode */
+		.clr_mask = 1 << 7,	/* not one-shot mode*/
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC / 2,
+	},
+	[tmp75b] = { /* not one-shot mode, Conversion rate 37Hz */
+		.clr_mask = 1 << 7 | 3 << 5,
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC / 37,
+	},
+	[tmp75c] = {
+		.clr_mask = 1 << 5,	/*not one-shot mode*/
+		.default_resolution = 12,
+		.default_sample_time = MSEC_PER_SEC / 4,
+	}
+};
 
 static inline long lm75_reg_to_mc(s16 temp, u8 resolution)
 {
@@ -146,8 +303,8 @@ static int lm75_write(struct device *dev, enum hwmon_sensor_types type,
 	 * Resolution of limit registers is assumed to be the same as the
 	 * temperature input register resolution unless given explicitly.
 	 */
-	if (data->resolution_limits)
-		resolution = data->resolution_limits;
+	if (data->params->resolution_limits)
+		resolution = data->params->resolution_limits;
 	else
 		resolution = data->resolution;
 
@@ -239,7 +396,6 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct device *hwmon_dev;
 	struct lm75_data *data;
 	int status, err;
-	u8 set_mask, clr_mask;
 	int new;
 	enum lm75_type kind;
 
@@ -257,6 +413,7 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return -ENOMEM;
 
 	data->client = client;
+	data->kind = kind;
 
 	data->regmap = devm_regmap_init_i2c(client, &lm75_regmap_config);
 	if (IS_ERR(data->regmap))
@@ -265,109 +422,22 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	/* Set to LM75 resolution (9 bits, 1/2 degree C) and range.
 	 * Then tweak to be more precise when appropriate.
 	 */
-	set_mask = 0;
-	clr_mask = LM75_SHUTDOWN;		/* continuous conversions */
-
-	switch (kind) {
-	case adt75:
-		clr_mask |= 1 << 5;		/* not one-shot mode */
-		data->resolution = 12;
-		data->sample_time = MSEC_PER_SEC / 8;
-		break;
-	case ds1775:
-	case ds75:
-	case stds75:
-		clr_mask |= 3 << 5;
-		set_mask |= 2 << 5;		/* 11-bit mode */
-		data->resolution = 11;
-		data->sample_time = MSEC_PER_SEC;
-		break;
-	case stlm75:
-		data->resolution = 9;
-		data->sample_time = MSEC_PER_SEC / 5;
-		break;
-	case ds7505:
-		set_mask |= 3 << 5;		/* 12-bit mode */
-		data->resolution = 12;
-		data->sample_time = MSEC_PER_SEC / 4;
-		break;
-	case g751:
-	case lm75:
-	case lm75a:
-		data->resolution = 9;
-		data->sample_time = MSEC_PER_SEC / 2;
-		break;
-	case lm75b:
-		data->resolution = 11;
-		data->sample_time = MSEC_PER_SEC / 4;
-		break;
-	case max6625:
-		data->resolution = 9;
-		data->sample_time = MSEC_PER_SEC / 4;
-		break;
-	case max6626:
-		data->resolution = 12;
-		data->resolution_limits = 9;
-		data->sample_time = MSEC_PER_SEC / 4;
-		break;
-	case max31725:
-		data->resolution = 16;
-		data->sample_time = MSEC_PER_SEC / 8;
-		break;
-	case tcn75:
-		data->resolution = 9;
-		data->sample_time = MSEC_PER_SEC / 8;
-		break;
-	case pct2075:
-		data->resolution = 11;
-		data->sample_time = MSEC_PER_SEC / 10;
-		break;
-	case mcp980x:
-		data->resolution_limits = 9;
-		/* fall through */
-	case tmp100:
-	case tmp101:
-		set_mask |= 3 << 5;		/* 12-bit mode */
-		data->resolution = 12;
-		data->sample_time = MSEC_PER_SEC;
-		clr_mask |= 1 << 7;		/* not one-shot mode */
-		break;
-	case tmp112:
-		set_mask |= 3 << 5;		/* 12-bit mode */
-		clr_mask |= 1 << 7;		/* not one-shot mode */
-		data->resolution = 12;
-		data->sample_time = MSEC_PER_SEC / 4;
-		break;
-	case tmp105:
-	case tmp175:
-	case tmp275:
-	case tmp75:
-		set_mask |= 3 << 5;		/* 12-bit mode */
-		clr_mask |= 1 << 7;		/* not one-shot mode */
-		data->resolution = 12;
-		data->sample_time = MSEC_PER_SEC / 2;
-		break;
-	case tmp75b:  /* not one-shot mode, Conversion rate 37Hz */
-		clr_mask |= 1 << 7 | 0x3 << 5;
-		data->resolution = 12;
-		data->sample_time = MSEC_PER_SEC / 37;
-		break;
-	case tmp75c:
-		clr_mask |= 1 << 5;		/* not one-shot mode */
-		data->resolution = 12;
-		data->sample_time = MSEC_PER_SEC / 4;
-		break;
-	}
 
-	/* configure as specified */
+	data->params = &device_params[data->kind];
+
+	/* Save default sample time and resolution*/
+	data->sample_time = data->params->default_sample_time;
+	data->resolution = data->params->default_resolution;
+
+	/* Cache original configuration */
 	status = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
 	if (status < 0) {
 		dev_dbg(dev, "Can't read config? %d\n", status);
 		return status;
 	}
 	data->orig_conf = status;
-	new = status & ~clr_mask;
-	new |= set_mask;
+	new = status & ~(data->params->clr_mask | LM75_SHUTDOWN);
+	new |= data->params->set_mask;
 	if (status != new)
 		i2c_smbus_write_byte_data(client, LM75_REG_CONF, new);
 
-- 
2.11.0

[PATCH v2 2/4] hwmon: (lm75) Create function from code to write into registers

[Iker Perez]

From: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>

Wrap the existing code to write configurations into registers in
a function.

Added error handling to the function.

Signed-off-by: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>
---
 drivers/hwmon/lm75.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 65a1131bfa7e..a32d7952d799 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -235,6 +235,27 @@ static inline long lm75_reg_to_mc(s16 temp, u8 resolution)
 	return ((temp >> (16 - resolution)) * 1000) >> (resolution - 8);
 }
 
+static int lm75_write_config(struct lm75_data *data, u8 set_mask,
+			     u8 clr_mask)
+{
+	u8 value;
+
+	clr_mask |= LM75_SHUTDOWN;
+	value = data->current_conf & ~clr_mask;
+	value |= set_mask;
+
+	if (data->current_conf != value) {
+		s32 err;
+
+		err = i2c_smbus_write_byte_data(data->client, LM75_REG_CONF,
+						value);
+		if (err)
+			return err;
+		data->current_conf = value;
+	}
+	return 0;
+}
+
 static int lm75_read(struct device *dev, enum hwmon_sensor_types type,
 		     u32 attr, int channel, long *val)
 {
@@ -396,7 +417,6 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	struct device *hwmon_dev;
 	struct lm75_data *data;
 	int status, err;
-	int new;
 	enum lm75_type kind;
 
 	if (client->dev.of_node)
@@ -436,16 +456,16 @@ lm75_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		return status;
 	}
 	data->orig_conf = status;
-	new = status & ~(data->params->clr_mask | LM75_SHUTDOWN);
-	new |= data->params->set_mask;
-	if (status != new)
-		i2c_smbus_write_byte_data(client, LM75_REG_CONF, new);
+	data->current_conf = status;
 
-	err = devm_add_action_or_reset(dev, lm75_remove, data);
+	err = lm75_write_config(data, data->params->set_mask,
+				data->params->clr_mask);
 	if (err)
 		return err;
 
-	dev_dbg(dev, "Config %02x\n", new);
+	err = devm_add_action_or_reset(dev, lm75_remove, data);
+	if (err)
+		return err;
 
 	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
 							 data, &lm75_chip_info,
-- 
2.11.0

[PATCH v2 3/4] hwmon: (lm75) Add new fields into lm75_params_

[Iker Perez]

From: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>

The new fields are included to prepare the driver for next patch. The
fields are:

* *resolutions: Stores all the supported resolutions by the device.
* num_sample_times: Stores the number of possible sample times.
* *sample_times: Stores all the possible sample times to be set.
* sample_set_masks: The set_masks for the possible sample times
* sample_clr_mask: Clear mask to set the default sample time.

Signed-off-by: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>
---

Changes since v1:
- In the lm75_params structure documentation there have been the next changes:
        - @num_sample_times description has been extended.
        - @sample_times description has been extended.
        - @sample_set_masks description has been extended.
        - @resolutions description has been included.

 drivers/hwmon/lm75.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index a32d7952d799..ed72455bcfa3 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -61,15 +61,34 @@ enum lm75_type {		/* keep sorted in alphabetical order */
  * @resolution_limits:	Limit register resolution. Optional. Should be set if
  *			the resolution of limit registers does not match the
  *			resolution of the temperature register.
+ * @resolutions		List of resolutions associated with sample times.
+ *			Optional. Should be set if num_sample_times is larger
+ *			than 1, and if the resolution changes with sample times.
+ *			If set, number of entries must match num_sample_times.
  * default_sample_time:	Sample time to be set by default.
+ * @num_sample_times:	Number of possible sample times to be set. Optional.
+ *			Should be set if the number of sample times is larger
+ *			than one.
+ * @sample_times:	All the possible sample times to be set. Mandatory if
+ *			num_sample_times is larger than 1. If set, number of
+ *			entries must match num_sample_times.
+ * @sample_set_masks:	All the set_masks for the possible sample times.
+ *			Mandatory if num_sample_times is larger than 1.
+ *			If set, number of entries must match num_sample_times.
+ * @sample_clr_mask:	Clear mask to set the default sample time.
  */
 
 struct lm75_params {
-	u8		set_mask;
-	u8		clr_mask;
-	u8		default_resolution;
-	u8		resolution_limits;
-	unsigned int	default_sample_time;
+	u8			set_mask;
+	u8			clr_mask;
+	u8			default_resolution;
+	u8			resolution_limits;
+	const u8		*resolutions;
+	unsigned int		default_sample_time;
+	u8			num_sample_times;
+	const unsigned int	*sample_times;
+	const u8		*sample_set_masks;
+	u8			sample_clr_mask;
 };
 
 /* Addresses scanned */
@@ -221,7 +240,14 @@ static const struct lm75_params device_params[] = {
 	[tmp75b] = { /* not one-shot mode, Conversion rate 37Hz */
 		.clr_mask = 1 << 7 | 3 << 5,
 		.default_resolution = 12,
+		.sample_set_masks = (u8 []){ 0 << 5, 1 << 5, 2 << 5,
+			3 << 5 },
+		.sample_clr_mask = 3 << 5,
 		.default_sample_time = MSEC_PER_SEC / 37,
+		.sample_times = (unsigned int []){ MSEC_PER_SEC / 37,
+			MSEC_PER_SEC / 18,
+			MSEC_PER_SEC / 9, MSEC_PER_SEC / 4 },
+		.num_sample_times = 4,
 	},
 	[tmp75c] = {
 		.clr_mask = 1 << 5,	/*not one-shot mode*/
-- 
2.11.0

[PATCH v2 4/4] hwmon: (lm75) Modularize lm75_write and make hwmon_chip writable

[Iker Perez]

From: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>

* Create two separate functions to write into hwmon_temp and hwmon_chip.
* Call the functions from lm75_write.
* Make hwm_chip writable if the chip supports more than one sample time.

Signed-off-by: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>
---
 drivers/hwmon/lm75.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 47 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index ed72455bcfa3..f83bfd7ceef7 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -16,6 +16,7 @@
 #include <linux/of_device.h>
 #include <linux/of.h>
 #include <linux/regmap.h>
+#include <linux/util_macros.h>
 #include "lm75.h"
 
 /*
@@ -325,16 +326,12 @@ static int lm75_read(struct device *dev, enum hwmon_sensor_types type,
 	return 0;
 }
 
-static int lm75_write(struct device *dev, enum hwmon_sensor_types type,
-		      u32 attr, int channel, long temp)
+static int lm75_write_temp(struct device *dev, u32 attr, long temp)
 {
 	struct lm75_data *data = dev_get_drvdata(dev);
 	u8 resolution;
 	int reg;
 
-	if (type != hwmon_temp)
-		return -EINVAL;
-
 	switch (attr) {
 	case hwmon_temp_max:
 		reg = LM75_REG_MAX;
@@ -362,13 +359,58 @@ static int lm75_write(struct device *dev, enum hwmon_sensor_types type,
 	return regmap_write(data->regmap, reg, temp);
 }
 
+static int lm75_write_chip(struct device *dev, u32 attr, long val)
+{
+	struct lm75_data *data = dev_get_drvdata(dev);
+	u8 index;
+	s32 err;
+
+	switch (attr) {
+	case hwmon_chip_update_interval:
+		index = find_closest(val, data->params->sample_times,
+				     (int)data->params->num_sample_times);
+
+		err = lm75_write_config(data,
+					data->params->sample_set_masks[index],
+					data->params->sample_clr_mask);
+		if (err)
+			return err;
+		data->sample_time = data->params->sample_times[index];
+
+		if (data->params->resolutions)
+			data->resolution = data->params->resolutions[index];
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int lm75_write(struct device *dev, enum hwmon_sensor_types type,
+		      u32 attr, int channel, long val)
+{
+	switch (type) {
+	case hwmon_chip:
+		return lm75_write_chip(dev, attr, val);
+	case hwmon_temp:
+		return lm75_write_temp(dev, attr, val);
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static umode_t lm75_is_visible(const void *data, enum hwmon_sensor_types type,
 			       u32 attr, int channel)
 {
+	const struct lm75_data *config_data = data;
+
 	switch (type) {
 	case hwmon_chip:
 		switch (attr) {
 		case hwmon_chip_update_interval:
+			if (config_data->params->num_sample_times > 1)
+				return 0644;
 			return 0444;
 		}
 		break;
-- 
2.11.0

Re: [PATCH v2 3/4] hwmon: (lm75) Add new fields into lm75_params_

[Guenter Roeck]

On Thu, Aug 08, 2019 at 09:02:45AM +0100, Iker Perez wrote:
> From: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>
> 
> The new fields are included to prepare the driver for next patch. The
> fields are:
> 
> * *resolutions: Stores all the supported resolutions by the device.
> * num_sample_times: Stores the number of possible sample times.
> * *sample_times: Stores all the possible sample times to be set.
> * sample_set_masks: The set_masks for the possible sample times
> * sample_clr_mask: Clear mask to set the default sample time.
> 
> Signed-off-by: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>
> ---
> 
> Changes since v1:
> - In the lm75_params structure documentation there have been the next changes:
>         - @num_sample_times description has been extended.
>         - @sample_times description has been extended.
>         - @sample_set_masks description has been extended.
>         - @resolutions description has been included.
> 
>  drivers/hwmon/lm75.c | 36 +++++++++++++++++++++++++++++++-----
>  1 file changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index a32d7952d799..ed72455bcfa3 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -61,15 +61,34 @@ enum lm75_type {		/* keep sorted in alphabetical order */
>   * @resolution_limits:	Limit register resolution. Optional. Should be set if
>   *			the resolution of limit registers does not match the
>   *			resolution of the temperature register.
> + * @resolutions		List of resolutions associated with sample times.

@resolutions:

> + *			Optional. Should be set if num_sample_times is larger
> + *			than 1, and if the resolution changes with sample times.
> + *			If set, number of entries must match num_sample_times.
>   * default_sample_time:	Sample time to be set by default.

@default_sample_time:

No need to resend; I'll fix that up when applying.

> + * @num_sample_times:	Number of possible sample times to be set. Optional.
> + *			Should be set if the number of sample times is larger
> + *			than one.
> + * @sample_times:	All the possible sample times to be set. Mandatory if
> + *			num_sample_times is larger than 1. If set, number of
> + *			entries must match num_sample_times.
> + * @sample_set_masks:	All the set_masks for the possible sample times.
> + *			Mandatory if num_sample_times is larger than 1.
> + *			If set, number of entries must match num_sample_times.
> + * @sample_clr_mask:	Clear mask to set the default sample time.
>   */
>  
>  struct lm75_params {
> -	u8		set_mask;
> -	u8		clr_mask;
> -	u8		default_resolution;
> -	u8		resolution_limits;
> -	unsigned int	default_sample_time;
> +	u8			set_mask;
> +	u8			clr_mask;
> +	u8			default_resolution;
> +	u8			resolution_limits;
> +	const u8		*resolutions;
> +	unsigned int		default_sample_time;
> +	u8			num_sample_times;
> +	const unsigned int	*sample_times;
> +	const u8		*sample_set_masks;
> +	u8			sample_clr_mask;
>  };
>  
>  /* Addresses scanned */
> @@ -221,7 +240,14 @@ static const struct lm75_params device_params[] = {
>  	[tmp75b] = { /* not one-shot mode, Conversion rate 37Hz */
>  		.clr_mask = 1 << 7 | 3 << 5,
>  		.default_resolution = 12,
> +		.sample_set_masks = (u8 []){ 0 << 5, 1 << 5, 2 << 5,
> +			3 << 5 },
> +		.sample_clr_mask = 3 << 5,
>  		.default_sample_time = MSEC_PER_SEC / 37,
> +		.sample_times = (unsigned int []){ MSEC_PER_SEC / 37,
> +			MSEC_PER_SEC / 18,
> +			MSEC_PER_SEC / 9, MSEC_PER_SEC / 4 },
> +		.num_sample_times = 4,
>  	},
>  	[tmp75c] = {
>  		.clr_mask = 1 << 5,	/*not one-shot mode*/

Re: [PATCH v2 0/4] Add support for variable sample time in lm75 driver

[Guenter Roeck]

On Thu, Aug 08, 2019 at 09:02:42AM +0100, Iker Perez wrote:
> From: Iker Perez del Palomar Sustatxa <iker.perez@codethink.co.uk>
> 
> Hello,
> 
> The objective of following patch series is to add support to lm75 driver
> to be able to configure the sample time of it's supported devices,
> particularly the tmp75b.
> 
> The applied changes involve:
> 	* Replace the current switch-case method for configuration
> 	  parameters selection to a structure storing them. This method
> 	  allows easier management of the parameters.
> 	* Split the writing of configuration registers into a separate
> 	  function. This method saves code in later patches.
> 	* Include new fields in lm75_params to add support for multiple
> 	  sample times.
> 	* Split the lm75_write functionality into separate, simpler,
> 	  functions.
> 	* Add support for configuring the devices via their sysfs nodes.
> 
> The patch series was based on linux-next's master branch.
> 
> Thank you Guenter Roeck, Michael Drake, Thomas Preston and Tom Eccles for
> your time, help and feedback.
> 

Series applied to hwmon-next.

Thanks,
Guenter