1. Patchew
  2. linux
  3. [PATCH 0/3] Add max31875 support
  4. View patch

[PATCH 1/3] hwmon: (max31827) refactor enum chips to chip info

John Erasmus Mari Geronimo posted 3 patches 11 months, 2 weeks ago
[PATCH 1/3] hwmon: (max31827) refactor enum chips to chip info
Posted by John Erasmus Mari Geronimo 11 months, 2 weeks ago 
Utilized chip info to replace enum chips to cater similar devices with
different configurations.

Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
---
 drivers/hwmon/max31827.c | 86 ++++++++++++++++++++--------------------
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/drivers/hwmon/max31827.c b/drivers/hwmon/max31827.c
index 48e8f8ba4..5d319d401 100644
--- a/drivers/hwmon/max31827.c
+++ b/drivers/hwmon/max31827.c
@@ -47,12 +47,6 @@
 #define MAX31827_M_DGR_TO_16_BIT(x)	(((x) << 4) / 1000)
 #define MAX31827_DEVICE_ENABLE(x)	((x) ? 0xA : 0x0)
 
-/*
- * The enum passed in the .data pointer of struct of_device_id must
- * start with a value != 0 since that is a requirement for using
- * device_get_match_data().
- */
-enum chips { max31827 = 1, max31828, max31829 };
 
 enum max31827_cnv {
 	MAX31827_CNV_1_DIV_64_HZ = 1,
@@ -95,6 +89,17 @@ static const u16 max31827_conv_times[] = {
 	[MAX31827_RES_12_BIT] = MAX31827_12_BIT_CNV_TIME,
 };
 
+struct max31827_state;
+static const struct max31827_chip_info max31827;
+static const struct max31827_chip_info max31828;
+static const struct max31827_chip_info max31829;
+static const struct max31827_chip_info max31875;
+
+struct max31827_chip_info {
+	u8 alarm_pol_default;
+	u32 fault_q_default;
+};
+
 struct max31827_state {
 	/*
 	 * Prevent simultaneous access to the i2c client.
@@ -104,6 +109,7 @@ struct max31827_state {
 	bool enable;
 	unsigned int resolution;
 	unsigned int update_interval;
+	const struct max31827_chip_info *chip_info;
 };
 
 static const struct regmap_config max31827_regmap = {
@@ -493,9 +499,9 @@ static struct attribute *max31827_attrs[] = {
 ATTRIBUTE_GROUPS(max31827);
 
 static const struct i2c_device_id max31827_i2c_ids[] = {
-	{ "max31827", max31827 },
-	{ "max31828", max31828 },
-	{ "max31829", max31829 },
+	{ "max31827", (kernel_ulong_t)&max31827 },
+	{ "max31828", (kernel_ulong_t)&max31828 },
+	{ "max31829", (kernel_ulong_t)&max31829 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, max31827_i2c_ids);
@@ -506,7 +512,6 @@ static int max31827_init_client(struct max31827_state *st,
 	struct fwnode_handle *fwnode;
 	unsigned int res = 0;
 	u32 data, lsb_idx;
-	enum chips type;
 	bool prop;
 	int ret;
 
@@ -523,8 +528,6 @@ static int max31827_init_client(struct max31827_state *st,
 	prop = fwnode_property_read_bool(fwnode, "adi,timeout-enable");
 	res |= FIELD_PREP(MAX31827_CONFIGURATION_TIMEOUT_MASK, !prop);
 
-	type = (enum chips)(uintptr_t)device_get_match_data(dev);
-
 	if (fwnode_property_present(fwnode, "adi,alarm-pol")) {
 		ret = fwnode_property_read_u32(fwnode, "adi,alarm-pol", &data);
 		if (ret)
@@ -535,19 +538,8 @@ static int max31827_init_client(struct max31827_state *st,
 		/*
 		 * Set default value.
 		 */
-		switch (type) {
-		case max31827:
-		case max31828:
-			res |= FIELD_PREP(MAX31827_CONFIGURATION_ALRM_POL_MASK,
-					  MAX31827_ALRM_POL_LOW);
-			break;
-		case max31829:
-			res |= FIELD_PREP(MAX31827_CONFIGURATION_ALRM_POL_MASK,
-					  MAX31827_ALRM_POL_HIGH);
-			break;
-		default:
-			return -EOPNOTSUPP;
-		}
+		res |= FIELD_PREP(MAX31827_CONFIGURATION_ALRM_POL_MASK,
+				  st->chip_info->alarm_pol_default);
 	}
 
 	if (fwnode_property_present(fwnode, "adi,fault-q")) {
@@ -571,19 +563,8 @@ static int max31827_init_client(struct max31827_state *st,
 		/*
 		 * Set default value.
 		 */
-		switch (type) {
-		case max31827:
-			res |= FIELD_PREP(MAX31827_CONFIGURATION_FLT_Q_MASK,
-					  MAX31827_FLT_Q_1);
-			break;
-		case max31828:
-		case max31829:
-			res |= FIELD_PREP(MAX31827_CONFIGURATION_FLT_Q_MASK,
-					  MAX31827_FLT_Q_4);
-			break;
-		default:
-			return -EOPNOTSUPP;
-		}
+		res |= FIELD_PREP(MAX31827_CONFIGURATION_FLT_Q_MASK,
+				  st->chip_info->fault_q_default);
 	}
 
 	return regmap_write(st->regmap, MAX31827_CONFIGURATION_REG, res);
@@ -604,11 +585,25 @@ static const struct hwmon_ops max31827_hwmon_ops = {
 	.write = max31827_write,
 };
 
-static const struct hwmon_chip_info max31827_chip_info = {
+static const struct hwmon_chip_info max31827_hwmon_chip_info = {
 	.ops = &max31827_hwmon_ops,
 	.info = max31827_info,
 };
 
+static const struct max31827_chip_info max31827 = {
+	.alarm_pol_default = MAX31827_ALRM_POL_LOW,
+	.fault_q_default = MAX31827_FLT_Q_1,
+};
+
+static const struct max31827_chip_info max31828 = {
+	.alarm_pol_default = MAX31827_ALRM_POL_LOW,
+	.fault_q_default = MAX31827_FLT_Q_4,
+
+static const struct max31827_chip_info max31829 = {
+	.alarm_pol_default = MAX31827_ALRM_POL_HIGH,
+	.fault_q_default = MAX31827_FLT_Q_4,
+};
+
 static int max31827_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -623,6 +618,10 @@ static int max31827_probe(struct i2c_client *client)
 	if (!st)
 		return -ENOMEM;
 
+	st->chip_info = device_get_match_data(dev);
+	if (!st->chip_info)
+		return -ENODEV;
+
 	mutex_init(&st->lock);
 
 	st->regmap = devm_regmap_init_i2c(client, &max31827_regmap);
@@ -639,7 +638,7 @@ static int max31827_probe(struct i2c_client *client)
 		return err;
 
 	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, st,
-							 &max31827_chip_info,
+							 &max31827_hwmon_chip_info,
 							 max31827_groups);
 
 	return PTR_ERR_OR_ZERO(hwmon_dev);
@@ -648,15 +647,15 @@ static int max31827_probe(struct i2c_client *client)
 static const struct of_device_id max31827_of_match[] = {
 	{
 		.compatible = "adi,max31827",
-		.data = (void *)max31827
+		.data = &max31827
 	},
 	{
 		.compatible = "adi,max31828",
-		.data = (void *)max31828
+		.data = &max31828
 	},
 	{
 		.compatible = "adi,max31829",
-		.data = (void *)max31829
+		.data = &max31829
 	},
 	{ }
 };
@@ -672,6 +671,7 @@ static struct i2c_driver max31827_driver = {
 };
 module_i2c_driver(max31827_driver);
 
+MODULE_AUTHOR("John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>");
 MODULE_AUTHOR("Daniel Matyas <daniel.matyas@analog.com>");
 MODULE_DESCRIPTION("Maxim MAX31827 low-power temperature switch driver");
 MODULE_LICENSE("GPL");
-- 
2.34.1
Re: [PATCH 1/3] hwmon: (max31827) refactor enum chips to chip info
Posted by kernel test robot 11 months, 2 weeks ago 
Hi John,

kernel test robot noticed the following build warnings:

[auto build test WARNING on adc52dd4078067fabf1431036ba180eafd8a7eee]

url:    https://github.com/intel-lab-lkp/linux/commits/John-Erasmus-Mari-Geronimo/hwmon-max31827-refactor-enum-chips-to-chip-info/20250108-162739
base:   adc52dd4078067fabf1431036ba180eafd8a7eee
patch link:    https://lore.kernel.org/r/20250108082531.15467-2-johnerasmusmari.geronimo%40analog.com
patch subject: [PATCH 1/3] hwmon: (max31827) refactor enum chips to chip info
config: hexagon-randconfig-001-20250109 (https://download.01.org/0day-ci/archive/20250109/202501091228.9APcsnSs-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250109/202501091228.9APcsnSs-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501091228.9APcsnSs-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/hwmon/max31827.c:93:40: warning: tentative definition of variable with internal linkage has incomplete non-array type 'const struct max31827_chip_info' [-Wtentative-definition-incomplete-type]
   static const struct max31827_chip_info max31827;
                                          ^
   drivers/hwmon/max31827.c:93:21: note: forward declaration of 'struct max31827_chip_info'
   static const struct max31827_chip_info max31827;
                       ^
   drivers/hwmon/max31827.c:94:40: warning: tentative definition of variable with internal linkage has incomplete non-array type 'const struct max31827_chip_info' [-Wtentative-definition-incomplete-type]
   static const struct max31827_chip_info max31828;
                                          ^
   drivers/hwmon/max31827.c:93:21: note: forward declaration of 'struct max31827_chip_info'
   static const struct max31827_chip_info max31827;
                       ^
   drivers/hwmon/max31827.c:95:40: warning: tentative definition of variable with internal linkage has incomplete non-array type 'const struct max31827_chip_info' [-Wtentative-definition-incomplete-type]
   static const struct max31827_chip_info max31829;
                                          ^
   drivers/hwmon/max31827.c:93:21: note: forward declaration of 'struct max31827_chip_info'
   static const struct max31827_chip_info max31827;
                       ^
   drivers/hwmon/max31827.c:96:40: warning: tentative definition of variable with internal linkage has incomplete non-array type 'const struct max31827_chip_info' [-Wtentative-definition-incomplete-type]
   static const struct max31827_chip_info max31875;
                                          ^
   drivers/hwmon/max31827.c:93:21: note: forward declaration of 'struct max31827_chip_info'
   static const struct max31827_chip_info max31827;
                       ^
   drivers/hwmon/max31827.c:602:1: error: expected expression
   static const struct max31827_chip_info max31829 = {
   ^
   drivers/hwmon/max31827.c:677:23: error: expected '}'
   MODULE_LICENSE("GPL");
                         ^
   drivers/hwmon/max31827.c:598:51: note: to match this '{'
   static const struct max31827_chip_info max31828 = {
                                                     ^
   drivers/hwmon/max31827.c:677:23: error: expected ';' after top level declarator
   MODULE_LICENSE("GPL");
                         ^
                         ;
   4 warnings and 3 errors generated.


vim +93 drivers/hwmon/max31827.c

    91	
    92	struct max31827_state;
  > 93	static const struct max31827_chip_info max31827;
    94	static const struct max31827_chip_info max31828;
    95	static const struct max31827_chip_info max31829;
    96	static const struct max31827_chip_info max31875;
    97	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 1/3] hwmon: (max31827) refactor enum chips to chip info
Posted by Guenter Roeck 11 months, 2 weeks ago 
On 1/8/25 00:25, John Erasmus Mari Geronimo wrote:
> Utilized chip info to replace enum chips to cater similar devices with
> different configurations.
> 
> Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
> ---
>   drivers/hwmon/max31827.c | 86 ++++++++++++++++++++--------------------
>   1 file changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/hwmon/max31827.c b/drivers/hwmon/max31827.c
> index 48e8f8ba4..5d319d401 100644
> --- a/drivers/hwmon/max31827.c
> +++ b/drivers/hwmon/max31827.c
> @@ -47,12 +47,6 @@
>   #define MAX31827_M_DGR_TO_16_BIT(x)	(((x) << 4) / 1000)
>   #define MAX31827_DEVICE_ENABLE(x)	((x) ? 0xA : 0x0)
>   
> -/*
> - * The enum passed in the .data pointer of struct of_device_id must
> - * start with a value != 0 since that is a requirement for using
> - * device_get_match_data().
> - */
> -enum chips { max31827 = 1, max31828, max31829 };
>   
>   enum max31827_cnv {
>   	MAX31827_CNV_1_DIV_64_HZ = 1,
> @@ -95,6 +89,17 @@ static const u16 max31827_conv_times[] = {
>   	[MAX31827_RES_12_BIT] = MAX31827_12_BIT_CNV_TIME,
>   };
>   
> +struct max31827_state;
> +static const struct max31827_chip_info max31827;
> +static const struct max31827_chip_info max31828;
> +static const struct max31827_chip_info max31829;
> +static const struct max31827_chip_info max31875;
> +

 From Documentation/hwmon/submitting-patches.rst:

* Avoid forward declarations if you can. Rearrange the code if necessary.

Please follow that guidance.

Thanks,
Guenter

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.