2022-02-28 10:48:04

by Potin Lai

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

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 <[email protected]>
---
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


2022-02-28 18:43:25

by kernel test robot

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

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/[email protected]/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 <[email protected]>

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/[email protected]

2022-02-28 19:17:18

by kernel test robot

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

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/[email protected]/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 <[email protected]>

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/[email protected]

2022-02-28 20:01:08

by Guenter Roeck

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

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 <[email protected]>
> ---
> 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) {