linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hwmon: (nct7802) Make RTD modes configurable.
@ 2021-09-10 13:07 Oskar Senft
  2021-09-10 16:21 ` Guenter Roeck
  2021-09-10 21:42 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Oskar Senft @ 2021-09-10 13:07 UTC (permalink / raw)
  To: linux-hwmon, linux-kernel; +Cc: Jean Delvare, Guenter Roeck, Oskar Senft

This change allows the RTD modes to be configurable via device tree
bindings. When the setting is not present via device tree, the driver
still defaults to the previous behavior where the RTD modes are left
alone.

Signed-off-by: Oskar Senft <osk@google.com>
---
 drivers/hwmon/nct7802.c | 94 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
index 604af2f6103a..6a6ab529bdd3 100644
--- a/drivers/hwmon/nct7802.c
+++ b/drivers/hwmon/nct7802.c
@@ -51,6 +51,24 @@ static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = {
 #define REG_CHIP_ID		0xfe
 #define REG_VERSION_ID		0xff
 
+/*
+ * Sensor modes according to 7.2.32 Mode Selection Register
+ */
+#define RTD_MODE_CLOSED		0x0
+#define RTD_MODE_CURRENT	0x1
+#define RTD_MODE_THERMISTOR	0x2
+#define RTD_MODE_VOLTAGE	0x3
+#define RTD_MODE_UNDEFINED	0xf
+
+#define MODE_BITS_MASK		0x3
+
+/*
+ * Bit offsets for sensors modes in REG_MODE
+ */
+#define MODE_OFFSET_RTD1	0
+#define MODE_OFFSET_RTD2	2
+#define MODE_OFFSET_RTD3	4
+
 /*
  * Data structures and manipulation thereof
  */
@@ -1038,7 +1056,9 @@ static const struct regmap_config nct7802_regmap_config = {
 	.volatile_reg = nct7802_regmap_is_volatile,
 };
 
-static int nct7802_init_chip(struct nct7802_data *data)
+static int nct7802_init_chip(struct nct7802_data *data,
+	unsigned char rtd1_mode, unsigned char rtd2_mode,
+	unsigned char rtd3_mode)
 {
 	int err;
 
@@ -1052,15 +1072,57 @@ static int nct7802_init_chip(struct nct7802_data *data)
 	if (err)
 		return err;
 
+	/* Configure sensor modes */
+	if ((rtd1_mode & MODE_BITS_MASK) == rtd1_mode) {
+		err = regmap_update_bits(data->regmap, REG_MODE,
+			MODE_BITS_MASK << MODE_OFFSET_RTD1,
+			rtd1_mode << MODE_OFFSET_RTD1);
+		if (err)
+			return err;
+	}
+	if ((rtd2_mode & MODE_BITS_MASK) == rtd2_mode) {
+		err = regmap_update_bits(data->regmap, REG_MODE,
+			MODE_BITS_MASK << MODE_OFFSET_RTD2,
+			rtd2_mode << MODE_OFFSET_RTD2);
+		if (err)
+			return err;
+	}
+	if ((rtd3_mode & MODE_BITS_MASK) == rtd3_mode) {
+		err = regmap_update_bits(data->regmap, REG_MODE,
+			MODE_BITS_MASK << MODE_OFFSET_RTD3,
+			rtd3_mode << MODE_OFFSET_RTD3);
+		if (err)
+			return err;
+	}
+
 	/* Enable Vcore and VCC voltage monitoring */
 	return regmap_update_bits(data->regmap, REG_VMON_ENABLE, 0x03, 0x03);
 }
 
+static unsigned char rtd_mode_from_string(const char *value)
+{
+	if (!strcmp(value, "closed"))
+		return RTD_MODE_CLOSED;
+	if (!strcmp(value, "current"))
+		return RTD_MODE_CURRENT;
+	if (!strcmp(value, "thermistor"))
+		return RTD_MODE_THERMISTOR;
+	if (!strcmp(value, "voltage"))
+		return RTD_MODE_VOLTAGE;
+
+	return RTD_MODE_UNDEFINED;
+}
+
 static int nct7802_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
 	struct nct7802_data *data;
 	struct device *hwmon_dev;
+	int rtd_mode_count;
+	unsigned char rtd1_mode = RTD_MODE_UNDEFINED;
+	unsigned char rtd2_mode = RTD_MODE_UNDEFINED;
+	unsigned char rtd3_mode = RTD_MODE_UNDEFINED;
+	const char *prop_value;
 	int ret;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
@@ -1074,7 +1136,25 @@ static int nct7802_probe(struct i2c_client *client)
 	mutex_init(&data->access_lock);
 	mutex_init(&data->in_alarm_lock);
 
-	ret = nct7802_init_chip(data);
+	if (dev->of_node) {
+		rtd_mode_count = of_property_count_strings(dev->of_node,
+			"nuvoton,rtd-modes");
+
+		if (rtd_mode_count > 0)
+			if (!of_property_read_string_index(dev->of_node,
+				"nuvoton,rtd-modes", 0, &prop_value))
+				rtd1_mode = rtd_mode_from_string(prop_value);
+		if (rtd_mode_count > 1)
+			if (!of_property_read_string_index(dev->of_node,
+				"nuvoton,rtd-modes", 1, &prop_value))
+				rtd2_mode = rtd_mode_from_string(prop_value);
+		if (rtd_mode_count > 2)
+			if (!of_property_read_string_index(dev->of_node,
+				"nuvoton,rtd-modes", 2, &prop_value))
+				rtd3_mode = rtd_mode_from_string(prop_value);
+	}
+
+	ret = nct7802_init_chip(data, rtd1_mode, rtd2_mode, rtd3_mode);
 	if (ret < 0)
 		return ret;
 
@@ -1094,10 +1174,20 @@ static const struct i2c_device_id nct7802_idtable[] = {
 };
 MODULE_DEVICE_TABLE(i2c, nct7802_idtable);
 
+static const struct of_device_id __maybe_unused nct7802_of_match[] = {
+	{
+		.compatible = "nuvoton,nct7802",
+		.data = 0
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, nct7802_of_match);
+
 static struct i2c_driver nct7802_driver = {
 	.class = I2C_CLASS_HWMON,
 	.driver = {
 		.name = DRVNAME,
+		.of_match_table = of_match_ptr(nct7802_of_match),
 	},
 	.detect = nct7802_detect,
 	.probe_new = nct7802_probe,
-- 
2.33.0.309.g3052b89438-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] hwmon: (nct7802) Make RTD modes configurable.
  2021-09-10 13:07 [PATCH] hwmon: (nct7802) Make RTD modes configurable Oskar Senft
@ 2021-09-10 16:21 ` Guenter Roeck
  2021-09-10 21:42 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2021-09-10 16:21 UTC (permalink / raw)
  To: Oskar Senft, linux-hwmon, linux-kernel; +Cc: Jean Delvare

On 9/10/21 6:07 AM, Oskar Senft wrote:
> This change allows the RTD modes to be configurable via device tree
> bindings. When the setting is not present via device tree, the driver
> still defaults to the previous behavior where the RTD modes are left
> alone.
> 
> Signed-off-by: Oskar Senft <osk@google.com>
> ---
>   drivers/hwmon/nct7802.c | 94 ++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 92 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
> index 604af2f6103a..6a6ab529bdd3 100644
> --- a/drivers/hwmon/nct7802.c
> +++ b/drivers/hwmon/nct7802.c
> @@ -51,6 +51,24 @@ static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = {
>   #define REG_CHIP_ID		0xfe
>   #define REG_VERSION_ID		0xff
>   
> +/*
> + * Sensor modes according to 7.2.32 Mode Selection Register
> + */
> +#define RTD_MODE_CLOSED		0x0
> +#define RTD_MODE_CURRENT	0x1
> +#define RTD_MODE_THERMISTOR	0x2
> +#define RTD_MODE_VOLTAGE	0x3
> +#define RTD_MODE_UNDEFINED	0xf
> +
> +#define MODE_BITS_MASK		0x3
> +
> +/*
> + * Bit offsets for sensors modes in REG_MODE
> + */
> +#define MODE_OFFSET_RTD1	0
> +#define MODE_OFFSET_RTD2	2
> +#define MODE_OFFSET_RTD3	4
> +

I think the access can be defined in a macro, with the index as parameter.
to be able to use it in the temp_type_{read,store} functions.


>   /*
>    * Data structures and manipulation thereof
>    */
> @@ -1038,7 +1056,9 @@ static const struct regmap_config nct7802_regmap_config = {
>   	.volatile_reg = nct7802_regmap_is_volatile,
>   };
>   
> -static int nct7802_init_chip(struct nct7802_data *data)
> +static int nct7802_init_chip(struct nct7802_data *data,
> +	unsigned char rtd1_mode, unsigned char rtd2_mode,
> +	unsigned char rtd3_mode)
>   {
>   	int err;
>   
> @@ -1052,15 +1072,57 @@ static int nct7802_init_chip(struct nct7802_data *data)
>   	if (err)
>   		return err;
>   
> +	/* Configure sensor modes */
> +	if ((rtd1_mode & MODE_BITS_MASK) == rtd1_mode) {

This is an odd way of checking if the mode is set. Why not just
"if (rtd1_mode != RTD_MODE_UNDEFINED)" ?

> +		err = regmap_update_bits(data->regmap, REG_MODE,
> +			MODE_BITS_MASK << MODE_OFFSET_RTD1,
> +			rtd1_mode << MODE_OFFSET_RTD1);
> +		if (err)
> +			return err;
> +	}
> +	if ((rtd2_mode & MODE_BITS_MASK) == rtd2_mode) {
> +		err = regmap_update_bits(data->regmap, REG_MODE,
> +			MODE_BITS_MASK << MODE_OFFSET_RTD2,
> +			rtd2_mode << MODE_OFFSET_RTD2);
> +		if (err)
> +			return err;
> +	}
> +	if ((rtd3_mode & MODE_BITS_MASK) == rtd3_mode) {
> +		err = regmap_update_bits(data->regmap, REG_MODE,
> +			MODE_BITS_MASK << MODE_OFFSET_RTD3,
> +			rtd3_mode << MODE_OFFSET_RTD3);
> +		if (err)
> +			return err;
> +	}

This should all be handled in a single register update. Read the mode register,
do all the bit updates, then write it back as complete register if it changed.

> +
>   	/* Enable Vcore and VCC voltage monitoring */
>   	return regmap_update_bits(data->regmap, REG_VMON_ENABLE, 0x03, 0x03);
>   }
>   
> +static unsigned char rtd_mode_from_string(const char *value)
> +{
> +	if (!strcmp(value, "closed"))
> +		return RTD_MODE_CLOSED;
> +	if (!strcmp(value, "current"))
> +		return RTD_MODE_CURRENT;
> +	if (!strcmp(value, "thermistor"))
> +		return RTD_MODE_THERMISTOR;
> +	if (!strcmp(value, "voltage"))
> +		return RTD_MODE_VOLTAGE;
> +
> +	return RTD_MODE_UNDEFINED;
> +}
> +
>   static int nct7802_probe(struct i2c_client *client)
>   {
>   	struct device *dev = &client->dev;
>   	struct nct7802_data *data;
>   	struct device *hwmon_dev;
> +	int rtd_mode_count;
> +	unsigned char rtd1_mode = RTD_MODE_UNDEFINED;
> +	unsigned char rtd2_mode = RTD_MODE_UNDEFINED;
> +	unsigned char rtd3_mode = RTD_MODE_UNDEFINED;
> +	const char *prop_value;
>   	int ret;
>   
>   	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> @@ -1074,7 +1136,25 @@ static int nct7802_probe(struct i2c_client *client)
>   	mutex_init(&data->access_lock);
>   	mutex_init(&data->in_alarm_lock);
>   
> -	ret = nct7802_init_chip(data);
> +	if (dev->of_node) {
> +		rtd_mode_count = of_property_count_strings(dev->of_node,
> +			"nuvoton,rtd-modes");
> +
> +		if (rtd_mode_count > 0)
> +			if (!of_property_read_string_index(dev->of_node,
> +				"nuvoton,rtd-modes", 0, &prop_value))
> +				rtd1_mode = rtd_mode_from_string(prop_value);
> +		if (rtd_mode_count > 1)
> +			if (!of_property_read_string_index(dev->of_node,
> +				"nuvoton,rtd-modes", 1, &prop_value))
> +				rtd2_mode = rtd_mode_from_string(prop_value);
> +		if (rtd_mode_count > 2)
> +			if (!of_property_read_string_index(dev->of_node,
> +				"nuvoton,rtd-modes", 2, &prop_value))
> +				rtd3_mode = rtd_mode_from_string(prop_value);
> +	}
> +

Better do this in nct7802_init_chip(), and pass dev as parameter.

> +	ret = nct7802_init_chip(data, rtd1_mode, rtd2_mode, rtd3_mode); >   	if (ret < 0)
>   		return ret;
>   
> @@ -1094,10 +1174,20 @@ static const struct i2c_device_id nct7802_idtable[] = {
>   };
>   MODULE_DEVICE_TABLE(i2c, nct7802_idtable);
>   
> +static const struct of_device_id __maybe_unused nct7802_of_match[] = {
> +	{
> +		.compatible = "nuvoton,nct7802",
> +		.data = 0

Unnecessary.

> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, nct7802_of_match);
> +
>   static struct i2c_driver nct7802_driver = {
>   	.class = I2C_CLASS_HWMON,
>   	.driver = {
>   		.name = DRVNAME,
> +		.of_match_table = of_match_ptr(nct7802_of_match),
>   	},
>   	.detect = nct7802_detect,
>   	.probe_new = nct7802_probe,
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] hwmon: (nct7802) Make RTD modes configurable.
  2021-09-10 13:07 [PATCH] hwmon: (nct7802) Make RTD modes configurable Oskar Senft
  2021-09-10 16:21 ` Guenter Roeck
@ 2021-09-10 21:42 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2021-09-10 21:42 UTC (permalink / raw)
  To: Oskar Senft, linux-hwmon, linux-kernel
  Cc: kbuild-all, Jean Delvare, Guenter Roeck, Oskar Senft

[-- Attachment #1: Type: text/plain, Size: 1918 bytes --]

Hi Oskar,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on linus/master linux/master v5.14 next-20210910]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Oskar-Senft/hwmon-nct7802-Make-RTD-modes-configurable/20210910-210934
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-randconfig-s002-20210910 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/6e3bd2e138e62f277fbfce192023e44f2ddda2c3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oskar-Senft/hwmon-nct7802-Make-RTD-modes-configurable/20210910-210934
        git checkout 6e3bd2e138e62f277fbfce192023e44f2ddda2c3
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/hwmon/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/hwmon/nct7802.c:1180:25: sparse: sparse: Using plain integer as NULL pointer

vim +1180 drivers/hwmon/nct7802.c

  1176	
  1177	static const struct of_device_id __maybe_unused nct7802_of_match[] = {
  1178		{
  1179			.compatible = "nuvoton,nct7802",
> 1180			.data = 0
  1181		},
  1182		{ },
  1183	};
  1184	MODULE_DEVICE_TABLE(of, nct7802_of_match);
  1185	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32212 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-10 21:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-10 13:07 [PATCH] hwmon: (nct7802) Make RTD modes configurable Oskar Senft
2021-09-10 16:21 ` Guenter Roeck
2021-09-10 21:42 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).