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

[Guenter Roeck <linux@roeck-us.net>]

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