On Sun, Apr 14, 2019 at 07:37:35AM -0700, Guenter Roeck wrote:
>On 4/13/19 9:03 AM, Adamski, Krzysztof (Nokia - PL/Wroclaw) wrote:
>>Manufacturer specific SAMPLES_FOR_AVG register allows setting the number
>>of samples used in computing the average values (PMBUS_VIRT_READ_*_AVG).
>>The number we write is an exponent of base 2 of the number of samples so
>>for example writing 3 will result in 8 samples average.
>>
>>Signed-off-by: Krzysztof Adamski <krzysztof.adamski@xxxxxxxxx>
>>---
>> drivers/hwmon/pmbus/lm25066.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>>diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c
>>index 53db78753a0d..b560d33db459 100644
>>--- a/drivers/hwmon/pmbus/lm25066.c
>>+++ b/drivers/hwmon/pmbus/lm25066.c
>>@@ -26,6 +26,7 @@
>> #include <linux/err.h>
>> #include <linux/slab.h>
>> #include <linux/i2c.h>
>>+#include <linux/log2.h>
>> #include "pmbus.h"
>> enum chips { lm25056, lm25066, lm5064, lm5066, lm5066i };
>>@@ -39,12 +40,15 @@ enum chips { lm25056, lm25066, lm5064, lm5066, lm5066i };
>> #define LM25066_CLEAR_PIN_PEAK 0xd6
>> #define LM25066_DEVICE_SETUP 0xd9
>> #define LM25066_READ_AVG_VIN 0xdc
>>+#define LM25066_SAMPLES_FOR_AVG 0xdb
>> #define LM25066_READ_AVG_VOUT 0xdd
>> #define LM25066_READ_AVG_IIN 0xde
>> #define LM25066_READ_AVG_PIN 0xdf
>> #define LM25066_DEV_SETUP_CL BIT(4) /* Current limit */
>>+#define LM25066_SAMPLES_FOR_AVG_MAX 4096
>>+
>> /* LM25056 only */
>> #define LM25056_VAUX_OV_WARN_LIMIT 0xe3
>>@@ -284,6 +288,10 @@ static int lm25066_read_word_data(struct i2c_client *client, int page, int reg)
>> case PMBUS_VIRT_RESET_PIN_HISTORY:
>> ret = 0;
>> break;
>>+ case PMBUS_VIRT_SAMPLES:
>>+ ret = pmbus_read_byte_data(client, 0, LM25066_SAMPLES_FOR_AVG);
>
> if (ret < 0)
> return ret;
>
Good point. However the convention for this switch is to break instead
of returning early. I would stick with that if you don't mind.
>>+ ret = 1 << ret;
>>+ break;
>> default:
>> ret = -ENODATA;
>> break;
>>@@ -398,6 +406,11 @@ static int lm25066_write_word_data(struct i2c_client *client, int page, int reg,
>> case PMBUS_VIRT_RESET_PIN_HISTORY:
>> ret = pmbus_write_byte(client, 0, LM25066_CLEAR_PIN_PEAK);
>> break;
>>+ case PMBUS_VIRT_SAMPLES:
>>+ word = clamp_val(word, 0, LM25066_SAMPLES_FOR_AVG_MAX);
>
>0 is not valid, and ilog2(0) returns -1 (or is undefined).
True, I'll clamp with minimum 1 to fix that. I don't think it's worth
returning an error for 0. Do you?
>
>>+ ret = pmbus_write_byte_data(client, 0, LM25066_SAMPLES_FOR_AVG,
>>+ ilog2(word));
>>+ break;
>> default:
>> ret = -ENODATA;
>> break;
>>@@ -438,7 +451,7 @@ static int lm25066_probe(struct i2c_client *client,
>> info->func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VMON
>> | PMBUS_HAVE_PIN | PMBUS_HAVE_IIN | PMBUS_HAVE_STATUS_INPUT
>>- | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
>>+ | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_SAMPLES;
>> if (data->id == lm25056) {
>> info->func[0] |= PMBUS_HAVE_STATUS_VMON;
>>
>
Krzysztof
