[PATCH v3 0/2] hwmon: (adm1275) Add sample averaging binding support

[PATCH v3 0/2] hwmon: (adm1275) Add sample averaging binding support

[Potin Lai]

This patch series allow user config PWR_AVG and VI_AVG in PMON_CONF
register by adding properties in device tree.

Example:
	adm1278@11 {
		compatible = "adi,adm1278";
		......
		adi,volt-curr-sample-average = <128>;
		adi,power-sample-average = <128>;
	};

LINK: [v1] https://lore.kernel.org/all/20220223163817.30583-1-potin.lai@quantatw.com/
LINK: [v2] https://lore.kernel.org/all/20220224154329.9755-1-potin.lai@quantatw.com/

Changes v2 --> v3:
- change property type back to u32, use logical value instead of register
  value
- fix typo in properties description
- add if-block to descript "adi,power-sample-average" not alloed if
  compatible not in the enum list

Changes v1 --> v2:
- use more descriptive property name
- change property type from u32 to u8 
- add property value check, valid range between 1 and 7

Potin Lai (2):
  hwmon: (adm1275) Allow setting sample averaging
  dt-bindings: hwmon: Add sample averaging properties for ADM1275

 .../bindings/hwmon/adi,adm1275.yaml           | 39 +++++++++++++++++++
 drivers/hwmon/pmbus/adm1275.c                 | 36 +++++++++++++++++
 2 files changed, 75 insertions(+)

-- 
2.17.1

[PATCH v3 1/2] hwmon: (adm1275) Allow setting sample averaging

[Potin Lai]

Current driver assume PWR_AVG and VI_AVG as 1 by default, and user needs
to set sample averaging via sysfs manually.

This patch parses the properties below from device tree, and setting
sample averaging during probe. Allowed input value from 1 to 128. If the
inputed value is not power of 2, the sample averaging number will be
configured with the smaller and cloest power of 2.

- adi,power-sample-average
- adi,volt-curr-sample-average

Signed-off-by: Potin Lai <potin.lai@quantatw.com>
---
 drivers/hwmon/pmbus/adm1275.c | 36 +++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
index d311e0557401..212c7f3c59b0 100644
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -475,6 +475,7 @@ static int adm1275_probe(struct i2c_client *client)
 	int vindex = -1, voindex = -1, cindex = -1, pindex = -1;
 	int tindex = -1;
 	u32 shunt;
+	u32 avg;
 
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_READ_BYTE_DATA
@@ -756,6 +757,41 @@ static int adm1275_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
+	if (data->have_power_sampling &&
+	    of_property_read_u32(client->dev.of_node,
+				"adi,power-sample-average", &avg) == 0) {
+		if (avg > ADM1275_SAMPLES_AVG_MAX || avg < 1)
+			return -EINVAL;
+		dev_info(&client->dev,
+			"Setting power sample averaging number to %u",
+			BIT(ilog2(avg)));
+		ret = adm1275_write_pmon_config(data, client, true,
+						ilog2(avg));
+		if (ret < 0) {
+			dev_err(&client->dev,
+				"Setting power sample averaging failed with error %d",
+				ret);
+			return ret;
+		}
+	}
+
+	if (of_property_read_u32(client->dev.of_node,
+				"adi,volt-curr-sample-average", &avg) == 0) {
+		if (avg > ADM1275_SAMPLES_AVG_MAX || avg < 1)
+			return -EINVAL;
+		dev_info(&client->dev,
+			"Setting voltage and current sample averaging number to %u",
+			BIT(ilog2(avg)));
+		ret = adm1275_write_pmon_config(data, client, false,
+						ilog2(avg));
+		if (ret < 0) {
+			dev_err(&client->dev,
+				"Setting voltage and current sample averaging failed with error %d",
+				ret);
+			return ret;
+		}
+	}
+
 	if (voindex < 0)
 		voindex = vindex;
 	if (vindex >= 0) {
-- 
2.17.1

Re: [PATCH v3 1/2] hwmon: (adm1275) Allow setting sample averaging

[kernel test robot]

Hi Potin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on robh/for-next v5.17-rc6 next-20220225]
[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/Potin-Lai/hwmon-adm1275-Add-sample-averaging-binding-support/20220228-183914
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220228/202202282046.WwmsqJeJ-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/e64fc74c4ca9f31e72265039c6bce3497169c8a2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Potin-Lai/hwmon-adm1275-Add-sample-averaging-binding-support/20220228-183914
        git checkout e64fc74c4ca9f31e72265039c6bce3497169c8a2
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/hwmon/pmbus/

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

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/acpi.h:15,
                    from include/linux/i2c.h:13,
                    from drivers/hwmon/pmbus/adm1275.c:15:
   drivers/hwmon/pmbus/adm1275.c: In function 'adm1275_probe':
>> drivers/hwmon/pmbus/adm1275.c:766:25: warning: format '%u' expects argument of type 'unsigned int', but argument 3 has type 'long unsigned int' [-Wformat=]
     766 |                         "Setting power sample averaging number to %u",
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:150:58: note: in expansion of macro 'dev_fmt'
     150 |         dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                          ^~~~~~~
   drivers/hwmon/pmbus/adm1275.c:765:17: note: in expansion of macro 'dev_info'
     765 |                 dev_info(&client->dev,
         |                 ^~~~~~~~
   drivers/hwmon/pmbus/adm1275.c:766:68: note: format string is defined here
     766 |                         "Setting power sample averaging number to %u",
         |                                                                   ~^
         |                                                                    |
         |                                                                    unsigned int
         |                                                                   %lu
   In file included from include/linux/device.h:15,
                    from include/linux/acpi.h:15,
                    from include/linux/i2c.h:13,
                    from drivers/hwmon/pmbus/adm1275.c:15:
   drivers/hwmon/pmbus/adm1275.c:783:25: warning: format '%u' expects argument of type 'unsigned int', but argument 3 has type 'long unsigned int' [-Wformat=]
     783 |                         "Setting voltage and current sample averaging number to %u",
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
   include/linux/dev_printk.h:150:58: note: in expansion of macro 'dev_fmt'
     150 |         dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                          ^~~~~~~
   drivers/hwmon/pmbus/adm1275.c:782:17: note: in expansion of macro 'dev_info'
     782 |                 dev_info(&client->dev,
         |                 ^~~~~~~~
   drivers/hwmon/pmbus/adm1275.c:783:82: note: format string is defined here
     783 |                         "Setting voltage and current sample averaging number to %u",
         |                                                                                 ~^
         |                                                                                  |
         |                                                                                  unsigned int
         |                                                                                 %lu


vim +766 drivers/hwmon/pmbus/adm1275.c

   464	
   465	static int adm1275_probe(struct i2c_client *client)
   466	{
   467		s32 (*config_read_fn)(const struct i2c_client *client, u8 reg);
   468		u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1];
   469		int config, device_config;
   470		int ret;
   471		struct pmbus_driver_info *info;
   472		struct adm1275_data *data;
   473		const struct i2c_device_id *mid;
   474		const struct coefficients *coefficients;
   475		int vindex = -1, voindex = -1, cindex = -1, pindex = -1;
   476		int tindex = -1;
   477		u32 shunt;
   478		u32 avg;
   479	
   480		if (!i2c_check_functionality(client->adapter,
   481					     I2C_FUNC_SMBUS_READ_BYTE_DATA
   482					     | I2C_FUNC_SMBUS_BLOCK_DATA))
   483			return -ENODEV;
   484	
   485		ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, block_buffer);
   486		if (ret < 0) {
   487			dev_err(&client->dev, "Failed to read Manufacturer ID\n");
   488			return ret;
   489		}
   490		if (ret != 3 || strncmp(block_buffer, "ADI", 3)) {
   491			dev_err(&client->dev, "Unsupported Manufacturer ID\n");
   492			return -ENODEV;
   493		}
   494	
   495		ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, block_buffer);
   496		if (ret < 0) {
   497			dev_err(&client->dev, "Failed to read Manufacturer Model\n");
   498			return ret;
   499		}
   500		for (mid = adm1275_id; mid->name[0]; mid++) {
   501			if (!strncasecmp(mid->name, block_buffer, strlen(mid->name)))
   502				break;
   503		}
   504		if (!mid->name[0]) {
   505			dev_err(&client->dev, "Unsupported device\n");
   506			return -ENODEV;
   507		}
   508	
   509		if (strcmp(client->name, mid->name) != 0)
   510			dev_notice(&client->dev,
   511				   "Device mismatch: Configured %s, detected %s\n",
   512				   client->name, mid->name);
   513	
   514		if (mid->driver_data == adm1272 || mid->driver_data == adm1278 ||
   515		    mid->driver_data == adm1293 || mid->driver_data == adm1294)
   516			config_read_fn = i2c_smbus_read_word_data;
   517		else
   518			config_read_fn = i2c_smbus_read_byte_data;
   519		config = config_read_fn(client, ADM1275_PMON_CONFIG);
   520		if (config < 0)
   521			return config;
   522	
   523		device_config = config_read_fn(client, ADM1275_DEVICE_CONFIG);
   524		if (device_config < 0)
   525			return device_config;
   526	
   527		data = devm_kzalloc(&client->dev, sizeof(struct adm1275_data),
   528				    GFP_KERNEL);
   529		if (!data)
   530			return -ENOMEM;
   531	
   532		if (of_property_read_u32(client->dev.of_node,
   533					 "shunt-resistor-micro-ohms", &shunt))
   534			shunt = 1000; /* 1 mOhm if not set via DT */
   535	
   536		if (shunt == 0)
   537			return -EINVAL;
   538	
   539		data->id = mid->driver_data;
   540	
   541		info = &data->info;
   542	
   543		info->pages = 1;
   544		info->format[PSC_VOLTAGE_IN] = direct;
   545		info->format[PSC_VOLTAGE_OUT] = direct;
   546		info->format[PSC_CURRENT_OUT] = direct;
   547		info->format[PSC_POWER] = direct;
   548		info->format[PSC_TEMPERATURE] = direct;
   549		info->func[0] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
   550				PMBUS_HAVE_SAMPLES;
   551	
   552		info->read_word_data = adm1275_read_word_data;
   553		info->read_byte_data = adm1275_read_byte_data;
   554		info->write_word_data = adm1275_write_word_data;
   555	
   556		switch (data->id) {
   557		case adm1075:
   558			if (device_config & ADM1275_IOUT_WARN2_SELECT)
   559				data->have_oc_fault = true;
   560			else
   561				data->have_uc_fault = true;
   562			data->have_pin_max = true;
   563			data->have_vaux_status = true;
   564	
   565			coefficients = adm1075_coefficients;
   566			vindex = 0;
   567			switch (config & ADM1075_IRANGE_MASK) {
   568			case ADM1075_IRANGE_25:
   569				cindex = 1;
   570				pindex = 3;
   571				break;
   572			case ADM1075_IRANGE_50:
   573				cindex = 2;
   574				pindex = 4;
   575				break;
   576			default:
   577				dev_err(&client->dev, "Invalid input current range");
   578				break;
   579			}
   580	
   581			info->func[0] |= PMBUS_HAVE_VIN | PMBUS_HAVE_PIN
   582			  | PMBUS_HAVE_STATUS_INPUT;
   583			if (config & ADM1275_VIN_VOUT_SELECT)
   584				info->func[0] |=
   585				  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
   586			break;
   587		case adm1272:
   588			data->have_vout = true;
   589			data->have_pin_max = true;
   590			data->have_temp_max = true;
   591			data->have_power_sampling = true;
   592	
   593			coefficients = adm1272_coefficients;
   594			vindex = (config & ADM1275_VRANGE) ? 1 : 0;
   595			cindex = (config & ADM1272_IRANGE) ? 3 : 2;
   596			/* pindex depends on the combination of the above */
   597			switch (config & (ADM1275_VRANGE | ADM1272_IRANGE)) {
   598			case 0:
   599			default:
   600				pindex = 4;
   601				break;
   602			case ADM1275_VRANGE:
   603				pindex = 5;
   604				break;
   605			case ADM1272_IRANGE:
   606				pindex = 6;
   607				break;
   608			case ADM1275_VRANGE | ADM1272_IRANGE:
   609				pindex = 7;
   610				break;
   611			}
   612			tindex = 8;
   613	
   614			info->func[0] |= PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT |
   615				PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
   616				PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
   617	
   618			/* Enable VOUT & TEMP1 if not enabled (disabled by default) */
   619			if ((config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) !=
   620			    (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) {
   621				config |= ADM1278_VOUT_EN | ADM1278_TEMP1_EN;
   622				ret = i2c_smbus_write_byte_data(client,
   623								ADM1275_PMON_CONFIG,
   624								config);
   625				if (ret < 0) {
   626					dev_err(&client->dev,
   627						"Failed to enable VOUT monitoring\n");
   628					return -ENODEV;
   629				}
   630			}
   631			if (config & ADM1278_VIN_EN)
   632				info->func[0] |= PMBUS_HAVE_VIN;
   633			break;
   634		case adm1275:
   635			if (device_config & ADM1275_IOUT_WARN2_SELECT)
   636				data->have_oc_fault = true;
   637			else
   638				data->have_uc_fault = true;
   639			data->have_vout = true;
   640	
   641			coefficients = adm1275_coefficients;
   642			vindex = (config & ADM1275_VRANGE) ? 0 : 1;
   643			cindex = 2;
   644	
   645			if (config & ADM1275_VIN_VOUT_SELECT)
   646				info->func[0] |=
   647				  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
   648			else
   649				info->func[0] |=
   650				  PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT;
   651			break;
   652		case adm1276:
   653			if (device_config & ADM1275_IOUT_WARN2_SELECT)
   654				data->have_oc_fault = true;
   655			else
   656				data->have_uc_fault = true;
   657			data->have_vout = true;
   658			data->have_pin_max = true;
   659	
   660			coefficients = adm1276_coefficients;
   661			vindex = (config & ADM1275_VRANGE) ? 0 : 1;
   662			cindex = 2;
   663			pindex = (config & ADM1275_VRANGE) ? 3 : 4;
   664	
   665			info->func[0] |= PMBUS_HAVE_VIN | PMBUS_HAVE_PIN
   666			  | PMBUS_HAVE_STATUS_INPUT;
   667			if (config & ADM1275_VIN_VOUT_SELECT)
   668				info->func[0] |=
   669				  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
   670			break;
   671		case adm1278:
   672			data->have_vout = true;
   673			data->have_pin_max = true;
   674			data->have_temp_max = true;
   675			data->have_power_sampling = true;
   676	
   677			coefficients = adm1278_coefficients;
   678			vindex = 0;
   679			cindex = 1;
   680			pindex = 2;
   681			tindex = 3;
   682	
   683			info->func[0] |= PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT |
   684				PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
   685				PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
   686	
   687			/* Enable VOUT & TEMP1 if not enabled (disabled by default) */
   688			if ((config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) !=
   689			    (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) {
   690				config |= ADM1278_VOUT_EN | ADM1278_TEMP1_EN;
   691				ret = i2c_smbus_write_byte_data(client,
   692								ADM1275_PMON_CONFIG,
   693								config);
   694				if (ret < 0) {
   695					dev_err(&client->dev,
   696						"Failed to enable VOUT monitoring\n");
   697					return -ENODEV;
   698				}
   699			}
   700	
   701			if (config & ADM1278_VIN_EN)
   702				info->func[0] |= PMBUS_HAVE_VIN;
   703			break;
   704		case adm1293:
   705		case adm1294:
   706			data->have_iout_min = true;
   707			data->have_pin_min = true;
   708			data->have_pin_max = true;
   709			data->have_mfr_vaux_status = true;
   710			data->have_power_sampling = true;
   711	
   712			coefficients = adm1293_coefficients;
   713	
   714			voindex = 0;
   715			switch (config & ADM1293_VIN_SEL_MASK) {
   716			case ADM1293_VIN_SEL_012:	/* 1.2V */
   717				vindex = 0;
   718				break;
   719			case ADM1293_VIN_SEL_074:	/* 7.4V */
   720				vindex = 1;
   721				break;
   722			case ADM1293_VIN_SEL_210:	/* 21V */
   723				vindex = 2;
   724				break;
   725			default:			/* disabled */
   726				break;
   727			}
   728	
   729			switch (config & ADM1293_IRANGE_MASK) {
   730			case ADM1293_IRANGE_25:
   731				cindex = 3;
   732				break;
   733			case ADM1293_IRANGE_50:
   734				cindex = 4;
   735				break;
   736			case ADM1293_IRANGE_100:
   737				cindex = 5;
   738				break;
   739			case ADM1293_IRANGE_200:
   740				cindex = 6;
   741				break;
   742			}
   743	
   744			if (vindex >= 0)
   745				pindex = 7 + vindex * 4 + (cindex - 3);
   746	
   747			if (config & ADM1293_VAUX_EN)
   748				info->func[0] |=
   749					PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
   750	
   751			info->func[0] |= PMBUS_HAVE_PIN |
   752				PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT;
   753	
   754			break;
   755		default:
   756			dev_err(&client->dev, "Unsupported device\n");
   757			return -ENODEV;
   758		}
   759	
   760		if (data->have_power_sampling &&
   761		    of_property_read_u32(client->dev.of_node,
   762					"adi,power-sample-average", &avg) == 0) {
   763			if (avg > ADM1275_SAMPLES_AVG_MAX || avg < 1)
   764				return -EINVAL;
   765			dev_info(&client->dev,
 > 766				"Setting power sample averaging number to %u",
   767				BIT(ilog2(avg)));
   768			ret = adm1275_write_pmon_config(data, client, true,
   769							ilog2(avg));
   770			if (ret < 0) {
   771				dev_err(&client->dev,
   772					"Setting power sample averaging failed with error %d",
   773					ret);
   774				return ret;
   775			}
   776		}
   777	
   778		if (of_property_read_u32(client->dev.of_node,
   779					"adi,volt-curr-sample-average", &avg) == 0) {
   780			if (avg > ADM1275_SAMPLES_AVG_MAX || avg < 1)
   781				return -EINVAL;
   782			dev_info(&client->dev,
   783				"Setting voltage and current sample averaging number to %u",
   784				BIT(ilog2(avg)));
   785			ret = adm1275_write_pmon_config(data, client, false,
   786							ilog2(avg));
   787			if (ret < 0) {
   788				dev_err(&client->dev,
   789					"Setting voltage and current sample averaging failed with error %d",
   790					ret);
   791				return ret;
   792			}
   793		}
   794	
   795		if (voindex < 0)
   796			voindex = vindex;
   797		if (vindex >= 0) {
   798			info->m[PSC_VOLTAGE_IN] = coefficients[vindex].m;
   799			info->b[PSC_VOLTAGE_IN] = coefficients[vindex].b;
   800			info->R[PSC_VOLTAGE_IN] = coefficients[vindex].R;
   801		}
   802		if (voindex >= 0) {
   803			info->m[PSC_VOLTAGE_OUT] = coefficients[voindex].m;
   804			info->b[PSC_VOLTAGE_OUT] = coefficients[voindex].b;
   805			info->R[PSC_VOLTAGE_OUT] = coefficients[voindex].R;
   806		}
   807		if (cindex >= 0) {
   808			/* Scale current with sense resistor value */
   809			info->m[PSC_CURRENT_OUT] =
   810				coefficients[cindex].m * shunt / 1000;
   811			info->b[PSC_CURRENT_OUT] = coefficients[cindex].b;
   812			info->R[PSC_CURRENT_OUT] = coefficients[cindex].R;
   813		}
   814		if (pindex >= 0) {
   815			info->m[PSC_POWER] =
   816				coefficients[pindex].m * shunt / 1000;
   817			info->b[PSC_POWER] = coefficients[pindex].b;
   818			info->R[PSC_POWER] = coefficients[pindex].R;
   819		}
   820		if (tindex >= 0) {
   821			info->m[PSC_TEMPERATURE] = coefficients[tindex].m;
   822			info->b[PSC_TEMPERATURE] = coefficients[tindex].b;
   823			info->R[PSC_TEMPERATURE] = coefficients[tindex].R;
   824		}
   825	
   826		return pmbus_do_probe(client, info);
   827	}
   828	

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

Re: [PATCH v3 1/2] hwmon: (adm1275) Allow setting sample averaging

[kernel test robot]

Hi Potin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on robh/for-next v5.17-rc6 next-20220225]
[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/Potin-Lai/hwmon-adm1275-Add-sample-averaging-binding-support/20220228-183914
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-randconfig-a003-20220228 (https://download.01.org/0day-ci/archive/20220228/202202282133.tDaIrm2N-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/e64fc74c4ca9f31e72265039c6bce3497169c8a2
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Potin-Lai/hwmon-adm1275-Add-sample-averaging-binding-support/20220228-183914
        git checkout e64fc74c4ca9f31e72265039c6bce3497169c8a2
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/hwmon/pmbus/

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

All warnings (new ones prefixed by >>):

>> drivers/hwmon/pmbus/adm1275.c:767:4: warning: format specifies type 'unsigned int' but the argument has type 'unsigned long' [-Wformat]
                           BIT(ilog2(avg)));
                           ^~~~~~~~~~~~~~~
   include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
           dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
                                                                    ~~~     ^~~~~~~~~~~
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
                   _p_func(dev, fmt, ##__VA_ARGS__);                       \
                                ~~~    ^~~~~~~~~~~
   include/vdso/bits.h:7:19: note: expanded from macro 'BIT'
   #define BIT(nr)                 (UL(1) << (nr))
                                   ^~~~~~~~~~~~~~~
   drivers/hwmon/pmbus/adm1275.c:784:4: warning: format specifies type 'unsigned int' but the argument has type 'unsigned long' [-Wformat]
                           BIT(ilog2(avg)));
                           ^~~~~~~~~~~~~~~
   include/linux/dev_printk.h:150:67: note: expanded from macro 'dev_info'
           dev_printk_index_wrap(_dev_info, KERN_INFO, dev, dev_fmt(fmt), ##__VA_ARGS__)
                                                                    ~~~     ^~~~~~~~~~~
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
                   _p_func(dev, fmt, ##__VA_ARGS__);                       \
                                ~~~    ^~~~~~~~~~~
   include/vdso/bits.h:7:19: note: expanded from macro 'BIT'
   #define BIT(nr)                 (UL(1) << (nr))
                                   ^~~~~~~~~~~~~~~
   2 warnings generated.


vim +767 drivers/hwmon/pmbus/adm1275.c

   464	
   465	static int adm1275_probe(struct i2c_client *client)
   466	{
   467		s32 (*config_read_fn)(const struct i2c_client *client, u8 reg);
   468		u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1];
   469		int config, device_config;
   470		int ret;
   471		struct pmbus_driver_info *info;
   472		struct adm1275_data *data;
   473		const struct i2c_device_id *mid;
   474		const struct coefficients *coefficients;
   475		int vindex = -1, voindex = -1, cindex = -1, pindex = -1;
   476		int tindex = -1;
   477		u32 shunt;
   478		u32 avg;
   479	
   480		if (!i2c_check_functionality(client->adapter,
   481					     I2C_FUNC_SMBUS_READ_BYTE_DATA
   482					     | I2C_FUNC_SMBUS_BLOCK_DATA))
   483			return -ENODEV;
   484	
   485		ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, block_buffer);
   486		if (ret < 0) {
   487			dev_err(&client->dev, "Failed to read Manufacturer ID\n");
   488			return ret;
   489		}
   490		if (ret != 3 || strncmp(block_buffer, "ADI", 3)) {
   491			dev_err(&client->dev, "Unsupported Manufacturer ID\n");
   492			return -ENODEV;
   493		}
   494	
   495		ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, block_buffer);
   496		if (ret < 0) {
   497			dev_err(&client->dev, "Failed to read Manufacturer Model\n");
   498			return ret;
   499		}
   500		for (mid = adm1275_id; mid->name[0]; mid++) {
   501			if (!strncasecmp(mid->name, block_buffer, strlen(mid->name)))
   502				break;
   503		}
   504		if (!mid->name[0]) {
   505			dev_err(&client->dev, "Unsupported device\n");
   506			return -ENODEV;
   507		}
   508	
   509		if (strcmp(client->name, mid->name) != 0)
   510			dev_notice(&client->dev,
   511				   "Device mismatch: Configured %s, detected %s\n",
   512				   client->name, mid->name);
   513	
   514		if (mid->driver_data == adm1272 || mid->driver_data == adm1278 ||
   515		    mid->driver_data == adm1293 || mid->driver_data == adm1294)
   516			config_read_fn = i2c_smbus_read_word_data;
   517		else
   518			config_read_fn = i2c_smbus_read_byte_data;
   519		config = config_read_fn(client, ADM1275_PMON_CONFIG);
   520		if (config < 0)
   521			return config;
   522	
   523		device_config = config_read_fn(client, ADM1275_DEVICE_CONFIG);
   524		if (device_config < 0)
   525			return device_config;
   526	
   527		data = devm_kzalloc(&client->dev, sizeof(struct adm1275_data),
   528				    GFP_KERNEL);
   529		if (!data)
   530			return -ENOMEM;
   531	
   532		if (of_property_read_u32(client->dev.of_node,
   533					 "shunt-resistor-micro-ohms", &shunt))
   534			shunt = 1000; /* 1 mOhm if not set via DT */
   535	
   536		if (shunt == 0)
   537			return -EINVAL;
   538	
   539		data->id = mid->driver_data;
   540	
   541		info = &data->info;
   542	
   543		info->pages = 1;
   544		info->format[PSC_VOLTAGE_IN] = direct;
   545		info->format[PSC_VOLTAGE_OUT] = direct;
   546		info->format[PSC_CURRENT_OUT] = direct;
   547		info->format[PSC_POWER] = direct;
   548		info->format[PSC_TEMPERATURE] = direct;
   549		info->func[0] = PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT |
   550				PMBUS_HAVE_SAMPLES;
   551	
   552		info->read_word_data = adm1275_read_word_data;
   553		info->read_byte_data = adm1275_read_byte_data;
   554		info->write_word_data = adm1275_write_word_data;
   555	
   556		switch (data->id) {
   557		case adm1075:
   558			if (device_config & ADM1275_IOUT_WARN2_SELECT)
   559				data->have_oc_fault = true;
   560			else
   561				data->have_uc_fault = true;
   562			data->have_pin_max = true;
   563			data->have_vaux_status = true;
   564	
   565			coefficients = adm1075_coefficients;
   566			vindex = 0;
   567			switch (config & ADM1075_IRANGE_MASK) {
   568			case ADM1075_IRANGE_25:
   569				cindex = 1;
   570				pindex = 3;
   571				break;
   572			case ADM1075_IRANGE_50:
   573				cindex = 2;
   574				pindex = 4;
   575				break;
   576			default:
   577				dev_err(&client->dev, "Invalid input current range");
   578				break;
   579			}
   580	
   581			info->func[0] |= PMBUS_HAVE_VIN | PMBUS_HAVE_PIN
   582			  | PMBUS_HAVE_STATUS_INPUT;
   583			if (config & ADM1275_VIN_VOUT_SELECT)
   584				info->func[0] |=
   585				  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
   586			break;
   587		case adm1272:
   588			data->have_vout = true;
   589			data->have_pin_max = true;
   590			data->have_temp_max = true;
   591			data->have_power_sampling = true;
   592	
   593			coefficients = adm1272_coefficients;
   594			vindex = (config & ADM1275_VRANGE) ? 1 : 0;
   595			cindex = (config & ADM1272_IRANGE) ? 3 : 2;
   596			/* pindex depends on the combination of the above */
   597			switch (config & (ADM1275_VRANGE | ADM1272_IRANGE)) {
   598			case 0:
   599			default:
   600				pindex = 4;
   601				break;
   602			case ADM1275_VRANGE:
   603				pindex = 5;
   604				break;
   605			case ADM1272_IRANGE:
   606				pindex = 6;
   607				break;
   608			case ADM1275_VRANGE | ADM1272_IRANGE:
   609				pindex = 7;
   610				break;
   611			}
   612			tindex = 8;
   613	
   614			info->func[0] |= PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT |
   615				PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
   616				PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
   617	
   618			/* Enable VOUT & TEMP1 if not enabled (disabled by default) */
   619			if ((config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) !=
   620			    (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) {
   621				config |= ADM1278_VOUT_EN | ADM1278_TEMP1_EN;
   622				ret = i2c_smbus_write_byte_data(client,
   623								ADM1275_PMON_CONFIG,
   624								config);
   625				if (ret < 0) {
   626					dev_err(&client->dev,
   627						"Failed to enable VOUT monitoring\n");
   628					return -ENODEV;
   629				}
   630			}
   631			if (config & ADM1278_VIN_EN)
   632				info->func[0] |= PMBUS_HAVE_VIN;
   633			break;
   634		case adm1275:
   635			if (device_config & ADM1275_IOUT_WARN2_SELECT)
   636				data->have_oc_fault = true;
   637			else
   638				data->have_uc_fault = true;
   639			data->have_vout = true;
   640	
   641			coefficients = adm1275_coefficients;
   642			vindex = (config & ADM1275_VRANGE) ? 0 : 1;
   643			cindex = 2;
   644	
   645			if (config & ADM1275_VIN_VOUT_SELECT)
   646				info->func[0] |=
   647				  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
   648			else
   649				info->func[0] |=
   650				  PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT;
   651			break;
   652		case adm1276:
   653			if (device_config & ADM1275_IOUT_WARN2_SELECT)
   654				data->have_oc_fault = true;
   655			else
   656				data->have_uc_fault = true;
   657			data->have_vout = true;
   658			data->have_pin_max = true;
   659	
   660			coefficients = adm1276_coefficients;
   661			vindex = (config & ADM1275_VRANGE) ? 0 : 1;
   662			cindex = 2;
   663			pindex = (config & ADM1275_VRANGE) ? 3 : 4;
   664	
   665			info->func[0] |= PMBUS_HAVE_VIN | PMBUS_HAVE_PIN
   666			  | PMBUS_HAVE_STATUS_INPUT;
   667			if (config & ADM1275_VIN_VOUT_SELECT)
   668				info->func[0] |=
   669				  PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
   670			break;
   671		case adm1278:
   672			data->have_vout = true;
   673			data->have_pin_max = true;
   674			data->have_temp_max = true;
   675			data->have_power_sampling = true;
   676	
   677			coefficients = adm1278_coefficients;
   678			vindex = 0;
   679			cindex = 1;
   680			pindex = 2;
   681			tindex = 3;
   682	
   683			info->func[0] |= PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT |
   684				PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
   685				PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
   686	
   687			/* Enable VOUT & TEMP1 if not enabled (disabled by default) */
   688			if ((config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) !=
   689			    (ADM1278_VOUT_EN | ADM1278_TEMP1_EN)) {
   690				config |= ADM1278_VOUT_EN | ADM1278_TEMP1_EN;
   691				ret = i2c_smbus_write_byte_data(client,
   692								ADM1275_PMON_CONFIG,
   693								config);
   694				if (ret < 0) {
   695					dev_err(&client->dev,
   696						"Failed to enable VOUT monitoring\n");
   697					return -ENODEV;
   698				}
   699			}
   700	
   701			if (config & ADM1278_VIN_EN)
   702				info->func[0] |= PMBUS_HAVE_VIN;
   703			break;
   704		case adm1293:
   705		case adm1294:
   706			data->have_iout_min = true;
   707			data->have_pin_min = true;
   708			data->have_pin_max = true;
   709			data->have_mfr_vaux_status = true;
   710			data->have_power_sampling = true;
   711	
   712			coefficients = adm1293_coefficients;
   713	
   714			voindex = 0;
   715			switch (config & ADM1293_VIN_SEL_MASK) {
   716			case ADM1293_VIN_SEL_012:	/* 1.2V */
   717				vindex = 0;
   718				break;
   719			case ADM1293_VIN_SEL_074:	/* 7.4V */
   720				vindex = 1;
   721				break;
   722			case ADM1293_VIN_SEL_210:	/* 21V */
   723				vindex = 2;
   724				break;
   725			default:			/* disabled */
   726				break;
   727			}
   728	
   729			switch (config & ADM1293_IRANGE_MASK) {
   730			case ADM1293_IRANGE_25:
   731				cindex = 3;
   732				break;
   733			case ADM1293_IRANGE_50:
   734				cindex = 4;
   735				break;
   736			case ADM1293_IRANGE_100:
   737				cindex = 5;
   738				break;
   739			case ADM1293_IRANGE_200:
   740				cindex = 6;
   741				break;
   742			}
   743	
   744			if (vindex >= 0)
   745				pindex = 7 + vindex * 4 + (cindex - 3);
   746	
   747			if (config & ADM1293_VAUX_EN)
   748				info->func[0] |=
   749					PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
   750	
   751			info->func[0] |= PMBUS_HAVE_PIN |
   752				PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT;
   753	
   754			break;
   755		default:
   756			dev_err(&client->dev, "Unsupported device\n");
   757			return -ENODEV;
   758		}
   759	
   760		if (data->have_power_sampling &&
   761		    of_property_read_u32(client->dev.of_node,
   762					"adi,power-sample-average", &avg) == 0) {
   763			if (avg > ADM1275_SAMPLES_AVG_MAX || avg < 1)
   764				return -EINVAL;
   765			dev_info(&client->dev,
   766				"Setting power sample averaging number to %u",
 > 767				BIT(ilog2(avg)));
   768			ret = adm1275_write_pmon_config(data, client, true,
   769							ilog2(avg));
   770			if (ret < 0) {
   771				dev_err(&client->dev,
   772					"Setting power sample averaging failed with error %d",
   773					ret);
   774				return ret;
   775			}
   776		}
   777	
   778		if (of_property_read_u32(client->dev.of_node,
   779					"adi,volt-curr-sample-average", &avg) == 0) {
   780			if (avg > ADM1275_SAMPLES_AVG_MAX || avg < 1)
   781				return -EINVAL;
   782			dev_info(&client->dev,
   783				"Setting voltage and current sample averaging number to %u",
   784				BIT(ilog2(avg)));
   785			ret = adm1275_write_pmon_config(data, client, false,
   786							ilog2(avg));
   787			if (ret < 0) {
   788				dev_err(&client->dev,
   789					"Setting voltage and current sample averaging failed with error %d",
   790					ret);
   791				return ret;
   792			}
   793		}
   794	
   795		if (voindex < 0)
   796			voindex = vindex;
   797		if (vindex >= 0) {
   798			info->m[PSC_VOLTAGE_IN] = coefficients[vindex].m;
   799			info->b[PSC_VOLTAGE_IN] = coefficients[vindex].b;
   800			info->R[PSC_VOLTAGE_IN] = coefficients[vindex].R;
   801		}
   802		if (voindex >= 0) {
   803			info->m[PSC_VOLTAGE_OUT] = coefficients[voindex].m;
   804			info->b[PSC_VOLTAGE_OUT] = coefficients[voindex].b;
   805			info->R[PSC_VOLTAGE_OUT] = coefficients[voindex].R;
   806		}
   807		if (cindex >= 0) {
   808			/* Scale current with sense resistor value */
   809			info->m[PSC_CURRENT_OUT] =
   810				coefficients[cindex].m * shunt / 1000;
   811			info->b[PSC_CURRENT_OUT] = coefficients[cindex].b;
   812			info->R[PSC_CURRENT_OUT] = coefficients[cindex].R;
   813		}
   814		if (pindex >= 0) {
   815			info->m[PSC_POWER] =
   816				coefficients[pindex].m * shunt / 1000;
   817			info->b[PSC_POWER] = coefficients[pindex].b;
   818			info->R[PSC_POWER] = coefficients[pindex].R;
   819		}
   820		if (tindex >= 0) {
   821			info->m[PSC_TEMPERATURE] = coefficients[tindex].m;
   822			info->b[PSC_TEMPERATURE] = coefficients[tindex].b;
   823			info->R[PSC_TEMPERATURE] = coefficients[tindex].R;
   824		}
   825	
   826		return pmbus_do_probe(client, info);
   827	}
   828	

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

Re: [PATCH v3 1/2] hwmon: (adm1275) Allow setting sample averaging

[Guenter Roeck]

On 2/28/22 02:37, Potin Lai wrote:
> Current driver assume PWR_AVG and VI_AVG as 1 by default, and user needs
> to set sample averaging via sysfs manually.
> 
> This patch parses the properties below from device tree, and setting
> sample averaging during probe. Allowed input value from 1 to 128. If the
> inputed value is not power of 2, the sample averaging number will be
> configured with the smaller and cloest power of 2.
> 
> - adi,power-sample-average
> - adi,volt-curr-sample-average
> 
> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
> ---
>   drivers/hwmon/pmbus/adm1275.c | 36 +++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
> index d311e0557401..212c7f3c59b0 100644
> --- a/drivers/hwmon/pmbus/adm1275.c
> +++ b/drivers/hwmon/pmbus/adm1275.c
> @@ -475,6 +475,7 @@ static int adm1275_probe(struct i2c_client *client)
>   	int vindex = -1, voindex = -1, cindex = -1, pindex = -1;
>   	int tindex = -1;
>   	u32 shunt;
> +	u32 avg;
>   
>   	if (!i2c_check_functionality(client->adapter,
>   				     I2C_FUNC_SMBUS_READ_BYTE_DATA
> @@ -756,6 +757,41 @@ static int adm1275_probe(struct i2c_client *client)
>   		return -ENODEV;
>   	}
>   
> +	if (data->have_power_sampling &&
> +	    of_property_read_u32(client->dev.of_node,
> +				"adi,power-sample-average", &avg) == 0) {
> +		if (avg > ADM1275_SAMPLES_AVG_MAX || avg < 1)
> +			return -EINVAL;
> +		dev_info(&client->dev,
> +			"Setting power sample averaging number to %u",
> +			BIT(ilog2(avg)));
> +		ret = adm1275_write_pmon_config(data, client, true,
> +						ilog2(avg));
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"Setting power sample averaging failed with error %d",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
> +	if (of_property_read_u32(client->dev.of_node,
> +				"adi,volt-curr-sample-average", &avg) == 0) {
> +		if (avg > ADM1275_SAMPLES_AVG_MAX || avg < 1)
> +			return -EINVAL;
> +		dev_info(&client->dev,
> +			"Setting voltage and current sample averaging number to %u",
> +			BIT(ilog2(avg)));

Please no such logging noise. Imagine if everyone would do that -
the log would be full with similar messages.

Thanks,
Guenter

> +		ret = adm1275_write_pmon_config(data, client, false,
> +						ilog2(avg));
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"Setting voltage and current sample averaging failed with error %d",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
>   	if (voindex < 0)
>   		voindex = vindex;
>   	if (vindex >= 0) {

[PATCH v3 2/2] dt-bindings: hwmon: Add sample averaging properties for ADM1275

[Potin Lai]

Add documentation of new properties for sample averaging in PMON_CONFIG
register.

New properties:
- adi,volt-curr-sample-average
- adi,power-sample-average

Signed-off-by: Potin Lai <potin.lai@quantatw.com>

doc
---
 .../bindings/hwmon/adi,adm1275.yaml           | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
index 223393d7cafd..bc4206b257a8 100644
--- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
+++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
@@ -37,6 +37,43 @@ properties:
     description:
       Shunt resistor value in micro-Ohm.
 
+  adi,volt-curr-sample-average:
+    description: |
+      Number of samples to be used to report voltage and current values.
+      If the configured value is not a power of 2, sample averaging number
+      will be configured with smaller and closest power of 2.
+
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 128
+    default: 1
+
+  adi,power-sample-average:
+    description: |
+      Number of samples to be used to report power values.
+      If the configured value is not a power of 2, sample averaging number
+      will be configured with smaller and closest power of 2.
+
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 128
+    default: 1
+
+if:
+  not:
+    properties:
+      compatible:
+        contains:
+          enum:
+          - adi,adm1272
+          - adi,adm1278
+          - adi,adm1293
+          - adi,adm1294
+then:
+  properties:
+    adi,power-sample-average:
+      description: This property is not allowed.
+
 required:
   - compatible
   - reg
@@ -53,5 +90,7 @@ examples:
             compatible = "adi,adm1272";
             reg = <0x10>;
             shunt-resistor-micro-ohms = <500>;
+            adi,volt-curr-sample-average = <128>;
+            adi,power-sample-average = <128>;
         };
     };
-- 
2.17.1

Re: [PATCH v3 2/2] dt-bindings: hwmon: Add sample averaging properties for ADM1275

[Krzysztof Kozlowski]

On 28/02/2022 11:37, Potin Lai wrote:
> Add documentation of new properties for sample averaging in PMON_CONFIG
> register.
> 
> New properties:
> - adi,volt-curr-sample-average
> - adi,power-sample-average
> 
> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
> 
> doc

You have weirdly formatted commit msg.

> ---
>  .../bindings/hwmon/adi,adm1275.yaml           | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> index 223393d7cafd..bc4206b257a8 100644
> --- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> @@ -37,6 +37,43 @@ properties:
>      description:
>        Shunt resistor value in micro-Ohm.
>  
> +  adi,volt-curr-sample-average:
> +    description: |
> +      Number of samples to be used to report voltage and current values.
> +      If the configured value is not a power of 2, sample averaging number
> +      will be configured with smaller and closest power of 2.
> +
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 128
> +    default: 1
> +
> +  adi,power-sample-average:
> +    description: |
> +      Number of samples to be used to report power values.
> +      If the configured value is not a power of 2, sample averaging number
> +      will be configured with smaller and closest power of 2.
> +
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 128
> +    default: 1
> +
> +if:

This should be in allOf.

> +  not:

Remove negation and list devices where it is not allowed.

> +    properties:
> +      compatible:
> +        contains:
> +          enum:
> +          - adi,adm1272
> +          - adi,adm1278
> +          - adi,adm1293
> +          - adi,adm1294
> +then:
> +  properties:
> +    adi,power-sample-average:
> +      description: This property is not allowed.

This does not work. Please test it - add not allowed property to such
devices and look for error. I gave you the example how it should be
done. Why doing it in a different way which does not work?



> +
>  required:
>    - compatible
>    - reg
> @@ -53,5 +90,7 @@ examples:
>              compatible = "adi,adm1272";
>              reg = <0x10>;
>              shunt-resistor-micro-ohms = <500>;
> +            adi,volt-curr-sample-average = <128>;
> +            adi,power-sample-average = <128>;
>          };
>      };


Best regards,
Krzysztof

Re: [PATCH v3 2/2] dt-bindings: hwmon: Add sample averaging properties for ADM1275

[POTIN LAI]

Krzysztof Kozlowski 於 28/02/2022 10:25 pm 寫道:
> On 28/02/2022 11:37, Potin Lai wrote:
>> Add documentation of new properties for sample averaging in PMON_CONFIG
>> register.
>>
>> New properties:
>> - adi,volt-curr-sample-average
>> - adi,power-sample-average
>>
>> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
>>
>> doc
> You have weirdly formatted commit msg.
It must be pasted from somewhere accidentally, sorry.
>> ---
>>  .../bindings/hwmon/adi,adm1275.yaml           | 39 +++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>> index 223393d7cafd..bc4206b257a8 100644
>> --- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>> @@ -37,6 +37,43 @@ properties:
>>      description:
>>        Shunt resistor value in micro-Ohm.
>>  
>> +  adi,volt-curr-sample-average:
>> +    description: |
>> +      Number of samples to be used to report voltage and current values.
>> +      If the configured value is not a power of 2, sample averaging number
>> +      will be configured with smaller and closest power of 2.
>> +
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 1
>> +    maximum: 128
>> +    default: 1
>> +
>> +  adi,power-sample-average:
>> +    description: |
>> +      Number of samples to be used to report power values.
>> +      If the configured value is not a power of 2, sample averaging number
>> +      will be configured with smaller and closest power of 2.
>> +
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 1
>> +    maximum: 128
>> +    default: 1
>> +
>> +if:
> This should be in allOf.
will add it.
>
>> +  not:
> Remove negation and list devices where it is not allowed.
will remove it.
>
>> +    properties:
>> +      compatible:
>> +        contains:
>> +          enum:
>> +          - adi,adm1272
>> +          - adi,adm1278
>> +          - adi,adm1293
>> +          - adi,adm1294
>> +then:
>> +  properties:
>> +    adi,power-sample-average:
>> +      description: This property is not allowed.
> This does not work. Please test it - add not allowed property to such
> devices and look for error. I gave you the example how it should be
> done. Why doing it in a different way which does not work?
>
Sorry for misunderstanding from original example. I rechecked the example and made a modification as below, before sending out new patch, would you mind help me review it and let me know if anything improper? Thank you.


dependencies:
  adi,enable-power-sample-average: [ 'adi,power-sample-average' ]
  adi,power-sample-average: [ 'adi,enable-power-sample-average' ]

allOf:
  - if:
      properties:
        compatible:
          contains:
            enum:
              - adi,adm1272
              - adi,adm1278
              - adi,adm1293
              - adi,adm1294
    then:
      required:
        - adi,enable-power-sample-average
    else:
      properties:
        adi,enable-power-sample-average: false


Thanks,
Potin
>
>> +
>>  required:
>>    - compatible
>>    - reg
>> @@ -53,5 +90,7 @@ examples:
>>              compatible = "adi,adm1272";
>>              reg = <0x10>;
>>              shunt-resistor-micro-ohms = <500>;
>> +            adi,volt-curr-sample-average = <128>;
>> +            adi,power-sample-average = <128>;
>>          };
>>      };
>
> Best regards,
> Krzysztof

Re: [PATCH v3 2/2] dt-bindings: hwmon: Add sample averaging properties for ADM1275

[Krzysztof Kozlowski]

On 28/02/2022 17:13, POTIN LAI wrote:
> 
> Krzysztof Kozlowski 於 28/02/2022 10:25 pm 寫道:
>> On 28/02/2022 11:37, Potin Lai wrote:
>>> Add documentation of new properties for sample averaging in PMON_CONFIG
>>> register.
>>>
>>> New properties:
>>> - adi,volt-curr-sample-average
>>> - adi,power-sample-average
>>>
>>> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
>>>
>>> doc
>> You have weirdly formatted commit msg.
> It must be pasted from somewhere accidentally, sorry.
>>> ---
>>>  .../bindings/hwmon/adi,adm1275.yaml           | 39 +++++++++++++++++++
>>>  1 file changed, 39 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>>> index 223393d7cafd..bc4206b257a8 100644
>>> --- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>>> +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>>> @@ -37,6 +37,43 @@ properties:
>>>      description:
>>>        Shunt resistor value in micro-Ohm.
>>>  
>>> +  adi,volt-curr-sample-average:
>>> +    description: |
>>> +      Number of samples to be used to report voltage and current values.
>>> +      If the configured value is not a power of 2, sample averaging number
>>> +      will be configured with smaller and closest power of 2.
>>> +
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    minimum: 1
>>> +    maximum: 128
>>> +    default: 1
>>> +
>>> +  adi,power-sample-average:
>>> +    description: |
>>> +      Number of samples to be used to report power values.
>>> +      If the configured value is not a power of 2, sample averaging number
>>> +      will be configured with smaller and closest power of 2.
>>> +
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    minimum: 1
>>> +    maximum: 128
>>> +    default: 1
>>> +
>>> +if:
>> This should be in allOf.
> will add it.
>>
>>> +  not:
>> Remove negation and list devices where it is not allowed.
> will remove it.
>>
>>> +    properties:
>>> +      compatible:
>>> +        contains:
>>> +          enum:
>>> +          - adi,adm1272
>>> +          - adi,adm1278
>>> +          - adi,adm1293
>>> +          - adi,adm1294
>>> +then:
>>> +  properties:
>>> +    adi,power-sample-average:
>>> +      description: This property is not allowed.
>> This does not work. Please test it - add not allowed property to such
>> devices and look for error. I gave you the example how it should be
>> done. Why doing it in a different way which does not work?
>>
> Sorry for misunderstanding from original example. I rechecked the example and made a modification as below, before sending out new patch, would you mind help me review it and let me know if anything improper? Thank you.

Don't take the example by copying and pasting. It has to be adjusted, I
just explained there in example how to disallow a property.

> 
> 
> dependencies:
>   adi,enable-power-sample-average: [ 'adi,power-sample-average' ]
>   adi,power-sample-average: [ 'adi,enable-power-sample-average' ]
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - adi,adm1272
>               - adi,adm1278
>               - adi,adm1293
>               - adi,adm1294
>     then:
>       required:
>         - adi,enable-power-sample-average

This does not look correct, because it is not a required property.

You should have "if:then: adi,enable-power-sample-average: false" etc"

>     else:
>       properties:
>         adi,enable-power-sample-average: false
> 
> 


Best regards,
Krzysztof

Re: [PATCH v3 2/2] dt-bindings: hwmon: Add sample averaging properties for ADM1275

[Guenter Roeck]

On 2/28/22 02:37, Potin Lai wrote:
> Add documentation of new properties for sample averaging in PMON_CONFIG
> register.
> 
> New properties:
> - adi,volt-curr-sample-average
> - adi,power-sample-average
> 
> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
> 
> doc
> ---
>   .../bindings/hwmon/adi,adm1275.yaml           | 39 +++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> index 223393d7cafd..bc4206b257a8 100644
> --- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> @@ -37,6 +37,43 @@ properties:
>       description:
>         Shunt resistor value in micro-Ohm.
>   
> +  adi,volt-curr-sample-average:
> +    description: |
> +      Number of samples to be used to report voltage and current values.
> +      If the configured value is not a power of 2, sample averaging number
> +      will be configured with smaller and closest power of 2.
> +
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 128
> +    default: 1
> +
> +  adi,power-sample-average:
> +    description: |
> +      Number of samples to be used to report power values.
> +      If the configured value is not a power of 2, sample averaging number
> +      will be configured with smaller and closest power of 2.
> +

Both properties should only accept 1, 2, 4, 8, 16, 32, 64, and 128.

We accept non-exact numbers in sysfs attributes because we can not
expect the end users to know permitted values, but devicetree authors
should know what is acceptable. Valid values can be expressed here
easily with something like
	enum: [1, 2, 4, 8, 16, 32, 64, 128]

This can also be easily tested in the code by ensuring that
the devicetree property is in the range of 1..128 and has exactly
one bit set, such as
	if (!val || val > 128 || hweight32(val) != 1)
or with something like
	if (!val || val > 128 || BIT(__fls(val)) != val)

Guenter

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 128
> +    default: 1
> +
> +if:
> +  not:
> +    properties:
> +      compatible:
> +        contains:
> +          enum:
> +          - adi,adm1272
> +          - adi,adm1278
> +          - adi,adm1293
> +          - adi,adm1294
> +then:
> +  properties:
> +    adi,power-sample-average:
> +      description: This property is not allowed.
> +
>   required:
>     - compatible
>     - reg
> @@ -53,5 +90,7 @@ examples:
>               compatible = "adi,adm1272";
>               reg = <0x10>;
>               shunt-resistor-micro-ohms = <500>;
> +            adi,volt-curr-sample-average = <128>;
> +            adi,power-sample-average = <128>;
>           };
>       };

Re: [PATCH v3 2/2] dt-bindings: hwmon: Add sample averaging properties for ADM1275

[POTIN LAI]

Guenter Roeck 於 28/02/2022 10:54 pm 寫道:
> On 2/28/22 02:37, Potin Lai wrote:
>> Add documentation of new properties for sample averaging in PMON_CONFIG
>> register.
>>
>> New properties:
>> - adi,volt-curr-sample-average
>> - adi,power-sample-average
>>
>> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
>>
>> doc
>> ---
>>   .../bindings/hwmon/adi,adm1275.yaml           | 39 +++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>> index 223393d7cafd..bc4206b257a8 100644
>> --- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>> @@ -37,6 +37,43 @@ properties:
>>       description:
>>         Shunt resistor value in micro-Ohm.
>>   +  adi,volt-curr-sample-average:
>> +    description: |
>> +      Number of samples to be used to report voltage and current values.
>> +      If the configured value is not a power of 2, sample averaging number
>> +      will be configured with smaller and closest power of 2.
>> +
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 1
>> +    maximum: 128
>> +    default: 1
>> +
>> +  adi,power-sample-average:
>> +    description: |
>> +      Number of samples to be used to report power values.
>> +      If the configured value is not a power of 2, sample averaging number
>> +      will be configured with smaller and closest power of 2.
>> +
>
> Both properties should only accept 1, 2, 4, 8, 16, 32, 64, and 128.
>
> We accept non-exact numbers in sysfs attributes because we can not
> expect the end users to know permitted values, but devicetree authors
> should know what is acceptable. Valid values can be expressed here
> easily with something like
>     enum: [1, 2, 4, 8, 16, 32, 64, 128]
>
> This can also be easily tested in the code by ensuring that
> the devicetree property is in the range of 1..128 and has exactly
> one bit set, such as
>     if (!val || val > 128 || hweight32(val) != 1)
> or with something like
>     if (!val || val > 128 || BIT(__fls(val)) != val)
>
> Guenter
>
Got it, will make the property value only allowed with listed values, and add check in drive code.


Thanks,

Potin

>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 1
>> +    maximum: 128
>> +    default: 1
>> +
>> +if:
>> +  not:
>> +    properties:
>> +      compatible:
>> +        contains:
>> +          enum:
>> +          - adi,adm1272
>> +          - adi,adm1278
>> +          - adi,adm1293
>> +          - adi,adm1294
>> +then:
>> +  properties:
>> +    adi,power-sample-average:
>> +      description: This property is not allowed.
>> +
>>   required:
>>     - compatible
>>     - reg
>> @@ -53,5 +90,7 @@ examples:
>>               compatible = "adi,adm1272";
>>               reg = <0x10>;
>>               shunt-resistor-micro-ohms = <500>;
>> +            adi,volt-curr-sample-average = <128>;
>> +            adi,power-sample-average = <128>;
>>           };
>>       };
>

Re: [PATCH v3 2/2] dt-bindings: hwmon: Add sample averaging properties for ADM1275

[Rob Herring]

On Mon, 28 Feb 2022 18:37:16 +0800, Potin Lai wrote:
> Add documentation of new properties for sample averaging in PMON_CONFIG
> register.
> 
> New properties:
> - adi,volt-curr-sample-average
> - adi,power-sample-average
> 
> Signed-off-by: Potin Lai <potin.lai@quantatw.com>
> 
> doc
> ---
>  .../bindings/hwmon/adi,adm1275.yaml           | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml:68:11: [warning] wrong indentation: expected 12 but found 10 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1598637

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.