* [PATCH v2 1/4] hwmon: (lm75) Create structure to save all the configuration parameters.
2019-08-08 8:02 [PATCH v2 0/4] Add support for variable sample time in lm75 driver Iker Perez
@ 2019-08-08 8:02 ` Iker Perez
2019-08-08 8:02 ` [PATCH v2 2/4] hwmon: (lm75) Create function from code to write into registers Iker Perez
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Iker Perez @ 2019-08-08 8:02 UTC (permalink / raw)
To: linux-hwmon, linux
Cc: jdelvare, linux-kernel, Iker Perez del Palomar Sustatxa
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/4] hwmon: (lm75) Create function from code to write into registers
2019-08-08 8:02 [PATCH v2 0/4] Add support for variable sample time in lm75 driver Iker Perez
2019-08-08 8:02 ` [PATCH v2 1/4] hwmon: (lm75) Create structure to save all the configuration parameters Iker Perez
@ 2019-08-08 8:02 ` Iker Perez
2019-08-08 8:02 ` [PATCH v2 3/4] hwmon: (lm75) Add new fields into lm75_params_ Iker Perez
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Iker Perez @ 2019-08-08 8:02 UTC (permalink / raw)
To: linux-hwmon, linux
Cc: jdelvare, linux-kernel, Iker Perez del Palomar Sustatxa
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 4/4] hwmon: (lm75) Modularize lm75_write and make hwmon_chip writable
2019-08-08 8:02 [PATCH v2 0/4] Add support for variable sample time in lm75 driver Iker Perez
` (2 preceding siblings ...)
2019-08-08 8:02 ` [PATCH v2 3/4] hwmon: (lm75) Add new fields into lm75_params_ Iker Perez
@ 2019-08-08 8:02 ` Iker Perez
2019-08-08 14:14 ` [PATCH v2 0/4] Add support for variable sample time in lm75 driver Guenter Roeck
4 siblings, 0 replies; 7+ messages in thread
From: Iker Perez @ 2019-08-08 8:02 UTC (permalink / raw)
To: linux-hwmon, linux
Cc: jdelvare, linux-kernel, Iker Perez del Palomar Sustatxa
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
^ permalink raw reply related [flat|nested] 7+ messages in thread