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

[PATCH 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 (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 | 373 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 257 insertions(+), 116 deletions(-)

-- 
2.11.0

[PATCH 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>
---
 drivers/hwmon/lm75.c | 275 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 169 insertions(+), 106 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index a2d3f2ce3e1d..1c012301b6ca 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,25 @@ enum lm75_type {		/* keep sorted in alphabetical order */
 	tmp75c,
 };
 
+/**
+ * struct lm75_params - lm75 configuration parameters.
+ * @set_mask:		Bits to set in cofiguration register when configuring
+ *			the chip.
+ * @clr_mask:		Bits to clear in configuration register when configuring
+ *			the chip.
+ * @resolution:		Number of bits to represent the temperatue value.
+ * @resolution_limits:	Resolution range.
+ * 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 +81,147 @@ 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,
+	},
+	[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 +296,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 +389,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 +406,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 +415,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

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

[Guenter Roeck]

On Tue, Aug 06, 2019 at 10:11:04AM +0100, Iker Perez wrote:
> 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>
> ---
>  drivers/hwmon/lm75.c | 275 +++++++++++++++++++++++++++++++--------------------
>  1 file changed, 169 insertions(+), 106 deletions(-)
> 
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index a2d3f2ce3e1d..1c012301b6ca 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,25 @@ enum lm75_type {		/* keep sorted in alphabetical order */
>  	tmp75c,
>  };
>  
> +/**
> + * struct lm75_params - lm75 configuration parameters.
> + * @set_mask:		Bits to set in cofiguration register when configuring
> + *			the chip.
> + * @clr_mask:		Bits to clear in configuration register when configuring
> + *			the chip.
> + * @resolution:		Number of bits to represent the temperatue value.
> + * @resolution_limits:	Resolution range.
> + * 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 +81,147 @@ 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,
> +	},
> +	[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 +296,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 +389,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 +406,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 +415,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;

This entry got lost in the rebase to hwmon-next.

Guenter

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

[Guenter Roeck]

On Tue, Aug 06, 2019 at 10:11:04AM +0100, Iker Perez wrote:
> 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>
> ---
>  drivers/hwmon/lm75.c | 275 +++++++++++++++++++++++++++++++--------------------
>  1 file changed, 169 insertions(+), 106 deletions(-)
> 
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index a2d3f2ce3e1d..1c012301b6ca 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,25 @@ enum lm75_type {		/* keep sorted in alphabetical order */
>  	tmp75c,
>  };
>  
> +/**
> + * struct lm75_params - lm75 configuration parameters.
> + * @set_mask:		Bits to set in cofiguration register when configuring
> + *			the chip.
> + * @clr_mask:		Bits to clear in configuration register when configuring
> + *			the chip.
> + * @resolution:		Number of bits to represent the temperatue value.
> + * @resolution_limits:	Resolution range.

I had to look this up to remember. Turns out this isn't the resolution range.
Please replace the description with something like:

				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 +81,147 @@ 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,
> +	},
> +	[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 +296,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 +389,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 +406,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 +415,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 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 1c012301b6ca..477ac0732ddf 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -228,6 +228,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)
 {
@@ -389,7 +410,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)
@@ -429,16 +449,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 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>
---
 drivers/hwmon/lm75.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 477ac0732ddf..a8d0a6fb9762 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -58,15 +58,24 @@ enum lm75_type {		/* keep sorted in alphabetical order */
  *			the chip.
  * @resolution:		Number of bits to represent the temperatue value.
  * @resolution_limits:	Resolution range.
+ * @num_sample_times:	Number of possible sample times to be set.
  * default_sample_time:	Sample time to be set by default.
+ * @sample_times:	All the possible sample times to be set.
+ * @sample_set_masks:	All the set_masks for the possible 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 */
@@ -214,7 +223,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

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

[Guenter Roeck]

On Tue, Aug 06, 2019 at 10:11:06AM +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>
> ---
>  drivers/hwmon/lm75.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
> index 477ac0732ddf..a8d0a6fb9762 100644
> --- a/drivers/hwmon/lm75.c
> +++ b/drivers/hwmon/lm75.c
> @@ -58,15 +58,24 @@ enum lm75_type {		/* keep sorted in alphabetical order */
>   *			the chip.
>   * @resolution:		Number of bits to represent the temperatue value.
>   * @resolution_limits:	Resolution range.
> + * @num_sample_times:	Number of possible sample times to be set.

Please add something like:
				Optional. Should be set if the number
				of sample times is larger than one.

>   * default_sample_time:	Sample time to be set by default.
> + * @sample_times:	All the possible sample times to be set.

Please add something like:
				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.

Please add something like:
				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.

Please add something like:

 * @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.

>   */
>  
>  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 */
> @@ -214,7 +223,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 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 a8d0a6fb9762..7b757ec146e9 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"
 
 /*
@@ -308,16 +309,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;
@@ -345,13 +342,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