2021-04-03 01:25:06

by Luiz Sampaio

Subject: [PATCH v2 0/9] w1: ds2438: adding support for calibration of current measurements

The following patches aim to make a user able to calibrate the current measurement of the DS2438. This chip uses a offset register in page1, which is added to the current register to give the user the current measurement. If this value is wrong, the user will get an offset current value, even if the current is zero, for instance. This patch gives support for reading the page1 registers (including the offset register) and for writing to the offset register. The DS2438 datasheet shows a calibration routine, and with this patch, the user can do this quickly by writing the correct value to the offset register. This patch was tested on real hardware using a power supply and an electronic load.
Please help to review this series of patches.

Best regards!
Sampaio
---
Changes in v2:
- Using git send-email to send the patches
- Adding documentation as requested
- Separating the coding style changes in different patches as requested

Luiz Sampaio (9):
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixing bug that would always get page0
w1: ds2438: adding support for reading page1
w1: ds2438: support for writing to offset register

Documentation/w1/slaves/w1_ds2438.rst | 19 +++-
drivers/w1/slaves/w1_ds2438.c | 122 ++++++++++++++++++++++----
2 files changed, 124 insertions(+), 17 deletions(-)

--
2.30.1

2021-04-03 01:26:33

by Luiz Sampaio

Subject: [PATCH v2 8/9] w1: ds2438: adding support for reading page1

Added a sysfs entry to support reading the page1 registers. This registers
contain Elapsed Time Meter (ETM) data, which shows for how long the chip is
on, as well as an Offset Register data, which can be used to calibrate the
current measurement of the chip.

Signed-off-by: Luiz Sampaio <[email protected]>
---
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++++++++++
2 files changed, 49 insertions(+)

diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ef6217ecb1cb..2cfdfedb584f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07

+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,

static BIN_ATTR(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-03 01:26:33

by Luiz Sampaio

Subject: [PATCH v2 9/9] w1: ds2438: support for writing to offset register

Added a sysfs entry to support writing to the offset register on page1.
This register is used to calibrate the chip canceling offset errors in the
current ADC. This means that, over time, reading the IAD register will not
return the correct current measurement, it will have an offset. Writing to
the offset register if the two's complement of the current register while
passing zero current to the load will calibrate the measurements. This
change was tested on real hardware and it was able to calibrate the chip
correctly.

Signed-off-by: Luiz Sampaio <[email protected]>
---
Documentation/w1/slaves/w1_ds2438.rst | 11 +++++-
drivers/w1/slaves/w1_ds2438.c | 49 +++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index ac8d0d4b0d0e..5c5573991351 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -22,7 +22,7 @@ is also often used in weather stations and applications such as: rain gauge,
wind speed/direction measuring, humidity sensing, etc.

Current support is provided through the following sysfs files (all files
-except "iad" are readonly):
+except "iad" and "offset" are readonly):

"iad"
-----
@@ -52,6 +52,15 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"offset"
+-------
+This file controls the 2-byte Offset Register of the chip.
+Writing a 2-byte value will change the Offset Register, which changes the
+current measurement done by the chip. Changing this register to the two's complement
+of the current register while forcing zero current through the load will calibrate
+the chip, canceling offset errors in the current ADC.
+
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 2cfdfedb584f..223a9aae6e2d 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}

+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* register 7 is reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-03 01:26:57

by Luiz Sampaio

Subject: [PATCH v2 7/9] w1: ds2438: fixing bug that would always get page0

The purpose of the w1_ds2438_get_page function is to get the register
values at the page passed as the pageno parameter. However, the page0 was
hardcoded, such that the function always returned the page0 contents. Fixed
so that the function can retrieve any page.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ccb06b8c2d78..ef6217ecb1cb 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -62,13 +62,13 @@ static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_RECALL_MEMORY;
- w1_buf[1] = 0x00;
+ w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);

if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_READ_SCRATCH;
- w1_buf[1] = 0x00;
+ w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);

count = w1_read_block(sl->master, buf, DS2438_PAGE_SIZE + 1);
--
2.30.1

2021-04-03 01:27:19

by Luiz Sampaio

Subject: [PATCH v2 6/9] w1: ds2438: fixed a coding style issue

Changed the permissions to preferred octal style.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 56e53a748059..ccb06b8c2d78 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -388,7 +388,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
return ret;
}

-static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0);
+static BIN_ATTR(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
--
2.30.1

2021-04-03 03:17:50

by kernel test robot

Subject: Re: [PATCH v2 6/9] w1: ds2438: fixed a coding style issue

Hi Luiz,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc5 next-20210401]
[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/Luiz-Sampaio/w1-ds2438-adding-support-for-calibration-of-current-measurements/20210403-092618
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git d93a0d43e3d0ba9e19387be4dae4a8d5b175a8d7
config: alpha-randconfig-r023-20210402 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.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/6282c64dc6cf3656cc3a9034b04f22d2a655ad39
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Sampaio/w1-ds2438-adding-support-for-calibration-of-current-measurements/20210403-092618
git checkout 6282c64dc6cf3656cc3a9034b04f22d2a655ad39
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/w1/slaves/w1_ds2438.c:391:40: error: macro "BIN_ATTR" requires 5 arguments, but only 4 given
391 | static BIN_ATTR(iad, 0664, iad_write, 0);
| ^
In file included from include/linux/kobject.h:20,
from include/linux/module.h:20,
from drivers/w1/slaves/w1_ds2438.c:9:
include/linux/sysfs.h:219: note: macro "BIN_ATTR" defined here
219 | #define BIN_ATTR(_name, _mode, _read, _write, _size) \
|
>> drivers/w1/slaves/w1_ds2438.c:391:8: error: type defaults to 'int' in declaration of 'BIN_ATTR' [-Werror=implicit-int]
391 | static BIN_ATTR(iad, 0664, iad_write, 0);
| ^~~~~~~~
>> drivers/w1/slaves/w1_ds2438.c:398:3: error: 'bin_attr_iad' undeclared here (not in a function); did you mean 'bin_attr_vad'?
398 | &bin_attr_iad,
| ^~~~~~~~~~~~
| bin_attr_vad
drivers/w1/slaves/w1_ds2438.c:391:8: warning: 'BIN_ATTR' defined but not used [-Wunused-variable]
391 | static BIN_ATTR(iad, 0664, iad_write, 0);
| ^~~~~~~~
drivers/w1/slaves/w1_ds2438.c:277:16: warning: 'iad_read' defined but not used [-Wunused-function]
277 | static ssize_t iad_read(struct file *filp, struct kobject *kobj,
| ^~~~~~~~
drivers/w1/slaves/w1_ds2438.c:255:16: warning: 'iad_write' defined but not used [-Wunused-function]
255 | static ssize_t iad_write(struct file *filp, struct kobject *kobj,
| ^~~~~~~~~
cc1: some warnings being treated as errors


vim +/BIN_ATTR +391 drivers/w1/slaves/w1_ds2438.c

390
> 391 static BIN_ATTR(iad, 0664, iad_write, 0);
392 static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
393 static BIN_ATTR_RO(temperature, 0/* real length varies */);
394 static BIN_ATTR_RO(vad, 0/* real length varies */);
395 static BIN_ATTR_RO(vdd, 0/* real length varies */);
396
397 static struct bin_attribute *w1_ds2438_bin_attrs[] = {
> 398 &bin_attr_iad,
399 &bin_attr_page0,
400 &bin_attr_temperature,
401 &bin_attr_vad,
402 &bin_attr_vdd,
403 NULL,
404 };
405

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2021-04-03 03:30:56

by kernel test robot

Subject: Re: [PATCH v2 6/9] w1: ds2438: fixed a coding style issue

Hi Luiz,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc5 next-20210401]
[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/Luiz-Sampaio/w1-ds2438-adding-support-for-calibration-of-current-measurements/20210403-092618
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git d93a0d43e3d0ba9e19387be4dae4a8d5b175a8d7
config: powerpc64-randconfig-r026-20210402 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 0fe8af94688aa03c01913c2001d6a1a911f42ce6)
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
# install powerpc64 cross compiling tool for clang build
# apt-get install binutils-powerpc64-linux-gnu
# https://github.com/0day-ci/linux/commit/6282c64dc6cf3656cc3a9034b04f22d2a655ad39
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Sampaio/w1-ds2438-adding-support-for-calibration-of-current-measurements/20210403-092618
git checkout 6282c64dc6cf3656cc3a9034b04f22d2a655ad39
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/w1/slaves/w1_ds2438.c:391:40: error: too few arguments provided to function-like macro invocation
static BIN_ATTR(iad, 0664, iad_write, 0);
^
include/linux/sysfs.h:219:9: note: macro 'BIN_ATTR' defined here
#define BIN_ATTR(_name, _mode, _read, _write, _size) \
^
>> drivers/w1/slaves/w1_ds2438.c:398:3: error: use of undeclared identifier 'bin_attr_iad'; did you mean 'bin_attr_vad'?
&bin_attr_iad,
^~~~~~~~~~~~
bin_attr_vad
drivers/w1/slaves/w1_ds2438.c:394:8: note: 'bin_attr_vad' declared here
static BIN_ATTR_RO(vad, 0/* real length varies */);
^
include/linux/sysfs.h:224:22: note: expanded from macro 'BIN_ATTR_RO'
struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
^
<scratch space>:49:1: note: expanded from here
bin_attr_vad
^
2 errors generated.


vim +391 drivers/w1/slaves/w1_ds2438.c

390
> 391 static BIN_ATTR(iad, 0664, iad_write, 0);
392 static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
393 static BIN_ATTR_RO(temperature, 0/* real length varies */);
394 static BIN_ATTR_RO(vad, 0/* real length varies */);
395 static BIN_ATTR_RO(vdd, 0/* real length varies */);
396
397 static struct bin_attribute *w1_ds2438_bin_attrs[] = {
> 398 &bin_attr_iad,
399 &bin_attr_page0,
400 &bin_attr_temperature,
401 &bin_attr_vad,
402 &bin_attr_vdd,
403 NULL,
404 };
405

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2021-04-03 04:48:15

by Luiz Sampaio

Subject: [PATCH v3 0/9] w1: ds2438: adding support for calibration of current measurements

The following patches aim to make a user able to calibrate the current measurement of the DS2438. This chip uses a offset register in page1, which is added to the current register to give the user the current measurement. If this value is wrong, the user will get an offset current value, even if the current is zero, for instance. This patch gives support for reading the page1 registers (including the offset register) and for writing to the offset register. The DS2438 datasheet shows a calibration routine, and with this patch, the user can do this quickly by writing the correct value to the offset register. This patch was tested on real hardware using a power supply and an electronic load.
Please help to review this series of patches.

Best regards!
Sampaio
---
Changes in v3:
- I accidentally added a wrong line that would not compile. I'm sorry. Fixed it.

Changes in v2:
- Using git send-email to send the patches
- Adding documentation as requested
- Separating the coding style changes in different patches as requested

Luiz Sampaio (9):
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixing bug that would always get page0
w1: ds2438: adding support for reading page1
w1: ds2438: support for writing to offset register

Documentation/w1/slaves/w1_ds2438.rst | 19 +++-
drivers/w1/slaves/w1_ds2438.c | 122 ++++++++++++++++++++++----
2 files changed, 124 insertions(+), 17 deletions(-)

--
2.30.1

2021-04-03 04:48:33

by Luiz Sampaio

Subject: [PATCH v3 1/9] w1: ds2438: fixed a coding style issue

There is an if statement and, if the function goes into it, it returns.
So, the next else is not required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 5cfb0ae23e91..148921fb9702 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -154,11 +154,11 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)

if ((status & mask) == value)
return 0; /* already set as requested */
- else {
- /* changing bit */
- status ^= mask;
- perform_write = 1;
- }
+
+ /* changing bit */
+ status ^= mask;
+ perform_write = 1;
+
break;
}

--
2.30.1

2021-04-03 04:48:40

by Luiz Sampaio

Subject: [PATCH v3 3/9] w1: ds2438: fixed a coding style issue

Since there is only one statement inside the if clause, no brackets
are required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index a5bb53042c93..eca50cec304f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_temperature(sl, &temp) == 0) {
+ if (w1_ds2438_get_temperature(sl, &temp) == 0)
ret = snprintf(buf, count, "%i\n", temp);
- } else
+ else
ret = -EIO;

return ret;
--
2.30.1

2021-04-03 04:48:41

by Luiz Sampaio

Subject: [PATCH v3 2/9] w1: ds2438: fixed a coding style issue

Since there is only one statement inside the if clause, no brackets
are required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 148921fb9702..a5bb53042c93 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_current(sl, &voltage) == 0) {
+ if (w1_ds2438_get_current(sl, &voltage) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
--
2.30.1

2021-04-03 04:48:45

by Luiz Sampaio

Subject: [PATCH v3 4/9] w1: ds2438: fixed a coding style issue

Since there is only one statement inside the if clause, no brackets
are required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index eca50cec304f..eeb593294fbd 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
--
2.30.1

2021-04-03 04:48:51

by Luiz Sampaio

Subject: [PATCH v3 5/9] w1: ds2438: fixed a coding style issue

Since there is only one statement inside the if clause, no brackets
are required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index eeb593294fbd..56e53a748059 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
--
2.30.1

2021-04-03 04:48:56

by Luiz Sampaio

Subject: [PATCH v3 6/9] w1: ds2438: fixed a coding style issue

Changed the permissions to preferred octal style.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 56e53a748059..ccb06b8c2d78 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -388,7 +388,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
return ret;
}

-static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0);
+static BIN_ATTR(iad, 0664, iad_read, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
--
2.30.1

2021-04-03 04:49:53

by Luiz Sampaio

Subject: [PATCH v3 7/9] w1: ds2438: fixing bug that would always get page0

The purpose of the w1_ds2438_get_page function is to get the register
values at the page passed as the pageno parameter. However, the page0 was
hardcoded, such that the function always returned the page0 contents. Fixed
so that the function can retrieve any page.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ccb06b8c2d78..ef6217ecb1cb 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -62,13 +62,13 @@ static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_RECALL_MEMORY;
- w1_buf[1] = 0x00;
+ w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);

if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_READ_SCRATCH;
- w1_buf[1] = 0x00;
+ w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);

count = w1_read_block(sl->master, buf, DS2438_PAGE_SIZE + 1);
--
2.30.1

2021-04-03 04:50:26

by Luiz Sampaio

Subject: [PATCH v3 8/9] w1: ds2438: adding support for reading page1

Added a sysfs entry to support reading the page1 registers. This registers
contain Elapsed Time Meter (ETM) data, which shows for how long the chip is
on, as well as an Offset Register data, which can be used to calibrate the
current measurement of the chip.

Signed-off-by: Luiz Sampaio <[email protected]>
---
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++++++++++
2 files changed, 49 insertions(+)

diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ef6217ecb1cb..2cfdfedb584f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07

+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,

static BIN_ATTR(iad, 0664, iad_read, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-03 04:50:33

by Luiz Sampaio

Subject: [PATCH v3 9/9] w1: ds2438: support for writing to offset register

Added a sysfs entry to support writing to the offset register on page1.
This register is used to calibrate the chip canceling offset errors in the
current ADC. This means that, over time, reading the IAD register will not
return the correct current measurement, it will have an offset. Writing to
the offset register if the two's complement of the current register while
passing zero current to the load will calibrate the measurements. This
change was tested on real hardware and it was able to calibrate the chip
correctly.

Signed-off-by: Luiz Sampaio <[email protected]>
---
Documentation/w1/slaves/w1_ds2438.rst | 11 +++++-
drivers/w1/slaves/w1_ds2438.c | 49 +++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index ac8d0d4b0d0e..5c5573991351 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -22,7 +22,7 @@ is also often used in weather stations and applications such as: rain gauge,
wind speed/direction measuring, humidity sensing, etc.

Current support is provided through the following sysfs files (all files
-except "iad" are readonly):
+except "iad" and "offset" are readonly):

"iad"
-----
@@ -52,6 +52,15 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"offset"
+-------
+This file controls the 2-byte Offset Register of the chip.
+Writing a 2-byte value will change the Offset Register, which changes the
+current measurement done by the chip. Changing this register to the two's complement
+of the current register while forcing zero current through the load will calibrate
+the chip, canceling offset errors in the current ADC.
+
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 2cfdfedb584f..223a9aae6e2d 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}

+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* register 7 is reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR(iad, 0664, iad_read, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-05 18:02:00

by Greg Kroah-Hartman

Subject: Re: [PATCH v3 0/9] w1: ds2438: adding support for calibration of current measurements

On Sat, Apr 03, 2021 at 01:45:47AM -0300, Luiz Sampaio wrote:
> The following patches aim to make a user able to calibrate the current measurement of the DS2438. This chip uses a offset register in page1, which is added to the current register to give the user the current measurement. If this value is wrong, the user will get an offset current value, even if the current is zero, for instance. This patch gives support for reading the page1 registers (including the offset register) and for writing to the offset register. The DS2438 datasheet shows a calibration routine, and with this patch, the user can do this quickly by writing the correct value to the offset register. This patch was tested on real hardware using a power supply and an electronic load.
> Please help to review this series of patches.
>
> Best regards!
> Sampaio
> ---
> Changes in v3:
> - I accidentally added a wrong line that would not compile. I'm sorry. Fixed it.
>
> Changes in v2:
> - Using git send-email to send the patches
> - Adding documentation as requested
> - Separating the coding style changes in different patches as requested
>
> Luiz Sampaio (9):
> w1: ds2438: fixed a coding style issue
> w1: ds2438: fixed a coding style issue
> w1: ds2438: fixed a coding style issue
> w1: ds2438: fixed a coding style issue
> w1: ds2438: fixed a coding style issue
> w1: ds2438: fixed a coding style issue

You can not have different patches that do different things, yet have
identical subject lines.

Please make them unique.

thanks,

greg k-h

2021-04-05 19:04:54

by Luiz Sampaio

Subject: [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements

The following patches aim to make a user able to calibrate the current measurement of the DS2438. This chip uses a offset register in page1, which is added to the current register to give the user the current measurement. If this value is wrong, the user will get an offset current value, even if the current is zero, for instance. This patch gives support for reading the page1 registers (including the offset register) and for writing to the offset register. The DS2438 datasheet shows a calibration routine, and with this patch, the user can do this quickly by writing the correct value to the offset register. This patch was tested on real hardware using a power supply and an electronic load.
Please help to review this series of patches.

Best regards!
Sampaio
---
Changes in v4:
- Fixing different patches with identical subject lines as requested

Changes in v3:
- I accidentally added a wrong line that would not compile. I'm sorry. Fixed it.

Changes in v2:
- Using git send-email to send the patches
- Adding documentation as requested
- Separating the coding style changes in different patches as requested

Luiz Sampaio (9):
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixing bug that would always get page0
w1: ds2438: adding support for reading page1
w1: ds2438: support for writing to offset register

Documentation/w1/slaves/w1_ds2438.rst | 19 +++-
drivers/w1/slaves/w1_ds2438.c | 122 ++++++++++++++++++++++----
2 files changed, 124 insertions(+), 17 deletions(-)

--
2.30.1

2021-04-05 19:08:41

by Luiz Sampaio

Subject: [PATCH v4 3/9] w1: ds2438: fixed a coding style issue in temperature_read

Since there is only one statement inside the if clause, no brackets
are required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index a5bb53042c93..eca50cec304f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_temperature(sl, &temp) == 0) {
+ if (w1_ds2438_get_temperature(sl, &temp) == 0)
ret = snprintf(buf, count, "%i\n", temp);
- } else
+ else
ret = -EIO;

return ret;
--
2.30.1

2021-04-05 19:26:33

by Luiz Sampaio

Subject: [PATCH v4 1/9] w1: ds2438: fixed a coding style issue after return

There is an if statement and, if the function goes into it, it returns.
So, the next else is not required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 5cfb0ae23e91..148921fb9702 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -154,11 +154,11 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)

if ((status & mask) == value)
return 0; /* already set as requested */
- else {
- /* changing bit */
- status ^= mask;
- perform_write = 1;
- }
+
+ /* changing bit */
+ status ^= mask;
+ perform_write = 1;
+
break;
}

--
2.30.1

2021-04-05 19:27:29

by Luiz Sampaio

Subject: [PATCH v4 2/9] w1: ds2438: fixed a coding style issue in iad_read

Since there is only one statement inside the if clause, no brackets
are required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 148921fb9702..a5bb53042c93 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_current(sl, &voltage) == 0) {
+ if (w1_ds2438_get_current(sl, &voltage) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
--
2.30.1

2021-04-05 19:29:45

by Luiz Sampaio

Subject: [PATCH v4 4/9] w1: ds2438: fixed a coding style issue in vad_read

Since there is only one statement inside the if clause, no brackets
are required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index eca50cec304f..eeb593294fbd 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
--
2.30.1

2021-04-05 19:30:30

by Luiz Sampaio

Subject: [PATCH v4 5/9] w1: ds2438: fixed a coding style issue in vdd_read

Since there is only one statement inside the if clause, no brackets
are required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index eeb593294fbd..56e53a748059 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
--
2.30.1

2021-04-05 19:33:25

by Luiz Sampaio

Subject: [PATCH v4 8/9] w1: ds2438: adding support for reading page1

Added a sysfs entry to support reading the page1 registers. This registers
contain Elapsed Time Meter (ETM) data, which shows for how long the chip is
on, as well as an Offset Register data, which can be used to calibrate the
current measurement of the chip.

Signed-off-by: Luiz Sampaio <[email protected]>
---
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++++++++++
2 files changed, 49 insertions(+)

diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ef6217ecb1cb..2cfdfedb584f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07

+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,

static BIN_ATTR(iad, 0664, iad_read, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-05 19:33:27

by Greg Kroah-Hartman

Subject: Re: [PATCH v4 2/9] w1: ds2438: fixed a coding style issue in iad_read

On Mon, Apr 05, 2021 at 07:50:02AM -0300, Luiz Sampaio wrote:
> Since there is only one statement inside the if clause, no brackets
> are required.
>
> Signed-off-by: Luiz Sampaio <[email protected]>
> ---
> drivers/w1/slaves/w1_ds2438.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

As this does the same type of fix as the next 3 patches, to the same
file, they should be all merged together into 1 patch.

thanks,

greg k-h

2021-04-05 19:33:34

by Luiz Sampaio

Subject: [PATCH v4 9/9] w1: ds2438: support for writing to offset register

Added a sysfs entry to support writing to the offset register on page1.
This register is used to calibrate the chip canceling offset errors in the
current ADC. This means that, over time, reading the IAD register will not
return the correct current measurement, it will have an offset. Writing to
the offset register if the two's complement of the current register while
passing zero current to the load will calibrate the measurements. This
change was tested on real hardware and it was able to calibrate the chip
correctly.

Signed-off-by: Luiz Sampaio <[email protected]>
---
Documentation/w1/slaves/w1_ds2438.rst | 11 +++++-
drivers/w1/slaves/w1_ds2438.c | 49 +++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index ac8d0d4b0d0e..5c5573991351 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -22,7 +22,7 @@ is also often used in weather stations and applications such as: rain gauge,
wind speed/direction measuring, humidity sensing, etc.

Current support is provided through the following sysfs files (all files
-except "iad" are readonly):
+except "iad" and "offset" are readonly):

"iad"
-----
@@ -52,6 +52,15 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"offset"
+-------
+This file controls the 2-byte Offset Register of the chip.
+Writing a 2-byte value will change the Offset Register, which changes the
+current measurement done by the chip. Changing this register to the two's complement
+of the current register while forcing zero current through the load will calibrate
+the chip, canceling offset errors in the current ADC.
+
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 2cfdfedb584f..223a9aae6e2d 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}

+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* register 7 is reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR(iad, 0664, iad_read, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-05 19:35:21

by Greg Kroah-Hartman

Subject: Re: [PATCH v4 9/9] w1: ds2438: support for writing to offset register

On Mon, Apr 05, 2021 at 07:50:09AM -0300, Luiz Sampaio wrote:
> Added a sysfs entry to support writing to the offset register on page1.
> This register is used to calibrate the chip canceling offset errors in the
> current ADC. This means that, over time, reading the IAD register will not
> return the correct current measurement, it will have an offset. Writing to
> the offset register if the two's complement of the current register while
> passing zero current to the load will calibrate the measurements. This
> change was tested on real hardware and it was able to calibrate the chip
> correctly.
>
> Signed-off-by: Luiz Sampaio <[email protected]>
> ---
> Documentation/w1/slaves/w1_ds2438.rst | 11 +++++-
> drivers/w1/slaves/w1_ds2438.c | 49 +++++++++++++++++++++++++++
> 2 files changed, 59 insertions(+), 1 deletion(-)

In this, and the previous patch, you added new sysfs files, but no
update to Documentation/ABI/ for them. Please fix that up.

thanks,

greg k-h

2021-04-09 03:10:30

by Luiz Sampaio

Subject: [PATCH v5 0/9] w1: ds2438: adding support for calibration of current measurements

The following patches aim to make a user able to calibrate the current
measurement of the DS2438. This chip uses a offset register in page1, which
is added to the current register to give the user the current measurement.
If this value is wrong, the user will get an offset current value, even if
the current is zero, for instance. This patch gives support for reading the
page1 registers (including the offset register) and for writing to the
offset register. The DS2438 datasheet shows a calibration routine, and with
this patch, the user can do this quickly by writing the correct value to
the offset register. This patch was tested on real hardware using a power
supply and an electronic load.
Please help to review this series of patches.

Best regards!
Sampaio
---
Changes in v5:
- Merged all brackets coding style issue in one patch
- Changing from BIN_ATTR to BIN_ATTR_RW in w1_ds2438.c
- Wrapping email lines
- Addind Documentation/ABI/

Changes in v4:
- Fixing different patches with identical subject lines as requested

Changes in v3:
- I accidentally added a wrong line that would not compile. I'm sorry. Fixed it.

Changes in v2:
- Using git send-email to send the patches
- Adding documentation as requested
- Separating the coding style changes in different patches as requested

Luiz Sampaio (9):
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixing bug that would always get page0
w1: ds2438: adding support for reading page1
w1: ds2438: support for writing to offset register

Documentation/w1/slaves/w1_ds2438.rst | 19 +++-
drivers/w1/slaves/w1_ds2438.c | 122 ++++++++++++++++++++++----
2 files changed, 124 insertions(+), 17 deletions(-)

--
2.30.1

2021-04-09 03:10:36

by Luiz Sampaio

Subject: [PATCH v5 1/6] w1: ds2438: fixed a coding style issue

There is an if statement and, if the function goes into it, it returns. So,
the next else is not required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 5cfb0ae23e91..148921fb9702 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -154,11 +154,11 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)

if ((status & mask) == value)
return 0; /* already set as requested */
- else {
- /* changing bit */
- status ^= mask;
- perform_write = 1;
- }
+
+ /* changing bit */
+ status ^= mask;
+ perform_write = 1;
+
break;
}

--
2.30.1

2021-04-09 03:10:50

by Luiz Sampaio

Subject: [PATCH v5 3/6] w1: ds2438: fixed a coding style issue

Changed the permissions to preferred octal style.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 56e53a748059..ccb06b8c2d78 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -388,7 +388,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
return ret;
}

-static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0);
+static BIN_ATTR(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
--
2.30.1

2021-04-09 03:10:57

by Luiz Sampaio

Subject: [PATCH v5 4/6] w1: ds2438: fixing bug that would always get page0

The purpose of the w1_ds2438_get_page function is to get the register
values at the page passed as the pageno parameter. However, the page0 was
hardcoded, such that the function always returned the page0 contents. Fixed
so that the function can retrieve any page.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ccb06b8c2d78..ef6217ecb1cb 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -62,13 +62,13 @@ static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_RECALL_MEMORY;
- w1_buf[1] = 0x00;
+ w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);

if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_READ_SCRATCH;
- w1_buf[1] = 0x00;
+ w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);

count = w1_read_block(sl->master, buf, DS2438_PAGE_SIZE + 1);
--
2.30.1

2021-04-09 03:11:02

by Luiz Sampaio

Subject: [PATCH v5 5/6] w1: ds2438: adding support for reading page1

Added a sysfs entry to support reading the page1 registers. This registers
contain Elapsed Time Meter (ETM) data, which shows for how long the chip is
on, as well as an Offset Register data, which can be used to calibrate the
current measurement of the chip.

Signed-off-by: Luiz Sampaio <[email protected]>
---
.../ABI/stable/sysfs-driver-w1_ds2438 | 6 +++
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 Documentation/ABI/stable/sysfs-driver-w1_ds2438

diff --git a/Documentation/ABI/stable/sysfs-driver-w1_ds2438 b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
new file mode 100644
index 000000000000..fa47437c11d9
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
@@ -0,0 +1,6 @@
+What: /sys/bus/w1/devices/.../page1
+Date: April 2021
+Contact: Luiz Sampaio <[email protected]>
+Description: read the contents of the page1 of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ef6217ecb1cb..2cfdfedb584f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07

+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,

static BIN_ATTR(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-09 03:11:37

by Luiz Sampaio

Subject: [PATCH v5 6/6] w1: ds2438: support for writing to offset register

Added a sysfs entry to support writing to the offset register on page1.
This register is used to calibrate the chip canceling offset errors in the
current ADC. This means that, over time, reading the IAD register will not
return the correct current measurement, it will have an offset. Writing to
the offset register if the two's complement of the current register while
passing zero current to the load will calibrate the measurements. This
change was tested on real hardware and it was able to calibrate the chip
correctly.

Signed-off-by: Luiz Sampaio <[email protected]>
---
.../ABI/stable/sysfs-driver-w1_ds2438 | 7 +++
Documentation/w1/slaves/w1_ds2438.rst | 11 ++++-
drivers/w1/slaves/w1_ds2438.c | 49 +++++++++++++++++++
3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/stable/sysfs-driver-w1_ds2438 b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
index fa47437c11d9..d2e7681cc287 100644
--- a/Documentation/ABI/stable/sysfs-driver-w1_ds2438
+++ b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
@@ -4,3 +4,10 @@ Contact: Luiz Sampaio <[email protected]>
Description: read the contents of the page1 of the DS2438
see Documentation/w1/slaves/w1_ds2438.rst for detailed information
Users: any user space application which wants to communicate with DS2438
+
+What: /sys/bus/w1/devices/.../offset
+Date: April 2021
+Contact: Luiz Sampaio <[email protected]>
+Description: write the contents to the offset register of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index ac8d0d4b0d0e..5c5573991351 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -22,7 +22,7 @@ is also often used in weather stations and applications such as: rain gauge,
wind speed/direction measuring, humidity sensing, etc.

Current support is provided through the following sysfs files (all files
-except "iad" are readonly):
+except "iad" and "offset" are readonly):

"iad"
-----
@@ -52,6 +52,15 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"offset"
+-------
+This file controls the 2-byte Offset Register of the chip.
+Writing a 2-byte value will change the Offset Register, which changes the
+current measurement done by the chip. Changing this register to the two's complement
+of the current register while forcing zero current through the load will calibrate
+the chip, canceling offset errors in the current ADC.
+
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 2cfdfedb584f..223a9aae6e2d 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}

+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last register reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-09 03:15:44

by Luiz Sampaio

Subject: [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements

The following patches aim to make a user able to calibrate the current
measurement of the DS2438. This chip uses a offset register in page1, which
is added to the current register to give the user the current measurement.
If this value is wrong, the user will get an offset current value, even if
the current is zero, for instance. This patch gives support for reading the
page1 registers (including the offset register) and for writing to the
offset register. The DS2438 datasheet shows a calibration routine, and with
this patch, the user can do this quickly by writing the correct value to
the offset register. This patch was tested on real hardware using a power
supply and an electronic load.
Please help to review this series of patches.

Best regards!
Sampaio
---
Changes in v6:
- Actually changing from BIN_ATTR to BIN_ATTR_RW

Changes in v5:
- Merged all brackets coding style issue in one patch
- Changing from BIN_ATTR to BIN_ATTR_RW in w1_ds2438.c
- Wrapping email lines
- Addind Documentation/ABI/

Changes in v4:
- Fixing different patches with identical subject lines as requested

Changes in v3:
- I accidentally added a wrong line that would not compile. I'm sorry. Fixed it.

Changes in v2:
- Using git send-email to send the patches
- Adding documentation as requested
- Separating the coding style changes in different patches as requested

Luiz Sampaio (9):
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed a coding style issue
w1: ds2438: fixing bug that would always get page0
w1: ds2438: adding support for reading page1
w1: ds2438: support for writing to offset register

Documentation/w1/slaves/w1_ds2438.rst | 19 +++-
drivers/w1/slaves/w1_ds2438.c | 122 ++++++++++++++++++++++----
2 files changed, 124 insertions(+), 17 deletions(-)

--
2.30.1

2021-04-09 03:16:23

by Luiz Sampaio

Subject: [PATCH v6 2/6] w1: ds2438: fixed if brackets coding style issue

Since there is only one statement inside the if clause, no brackets are
required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 148921fb9702..56e53a748059 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_current(sl, &voltage) == 0) {
+ if (w1_ds2438_get_current(sl, &voltage) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
@@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_temperature(sl, &temp) == 0) {
+ if (w1_ds2438_get_temperature(sl, &temp) == 0)
ret = snprintf(buf, count, "%i\n", temp);
- } else
+ else
ret = -EIO;

return ret;
@@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
@@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
--
2.30.1

2021-04-09 03:16:42

by Luiz Sampaio

Subject: [PATCH v6 3/6] w1: ds2438: fixed a coding style issue

Changed the permissions to preferred octal style.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 56e53a748059..ccb06b8c2d78 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -388,7 +388,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
return ret;
}

-static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0);
+static BIN_ATTR_RW(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
--
2.30.1

2021-04-09 03:16:49

by Luiz Sampaio

Subject: [PATCH v6 4/6] w1: ds2438: fixing bug that would always get page0

The purpose of the w1_ds2438_get_page function is to get the register
values at the page passed as the pageno parameter. However, the page0 was
hardcoded, such that the function always returned the page0 contents. Fixed
so that the function can retrieve any page.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ccb06b8c2d78..ef6217ecb1cb 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -62,13 +62,13 @@ static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_RECALL_MEMORY;
- w1_buf[1] = 0x00;
+ w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);

if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_READ_SCRATCH;
- w1_buf[1] = 0x00;
+ w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);

count = w1_read_block(sl->master, buf, DS2438_PAGE_SIZE + 1);
--
2.30.1

2021-04-09 03:16:49

by Luiz Sampaio

Subject: [PATCH v6 6/6] w1: ds2438: support for writing to offset register

Added a sysfs entry to support writing to the offset register on page1.
This register is used to calibrate the chip canceling offset errors in the
current ADC. This means that, over time, reading the IAD register will not
return the correct current measurement, it will have an offset. Writing to
the offset register if the two's complement of the current register while
passing zero current to the load will calibrate the measurements. This
change was tested on real hardware and it was able to calibrate the chip
correctly.

Signed-off-by: Luiz Sampaio <[email protected]>
---
.../ABI/stable/sysfs-driver-w1_ds2438 | 7 +++
Documentation/w1/slaves/w1_ds2438.rst | 11 ++++-
drivers/w1/slaves/w1_ds2438.c | 49 +++++++++++++++++++
3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/stable/sysfs-driver-w1_ds2438 b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
index fa47437c11d9..d2e7681cc287 100644
--- a/Documentation/ABI/stable/sysfs-driver-w1_ds2438
+++ b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
@@ -4,3 +4,10 @@ Contact: Luiz Sampaio <[email protected]>
Description: read the contents of the page1 of the DS2438
see Documentation/w1/slaves/w1_ds2438.rst for detailed information
Users: any user space application which wants to communicate with DS2438
+
+What: /sys/bus/w1/devices/.../offset
+Date: April 2021
+Contact: Luiz Sampaio <[email protected]>
+Description: write the contents to the offset register of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index ac8d0d4b0d0e..5c5573991351 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -22,7 +22,7 @@ is also often used in weather stations and applications such as: rain gauge,
wind speed/direction measuring, humidity sensing, etc.

Current support is provided through the following sysfs files (all files
-except "iad" are readonly):
+except "iad" and "offset" are readonly):

"iad"
-----
@@ -52,6 +52,15 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"offset"
+-------
+This file controls the 2-byte Offset Register of the chip.
+Writing a 2-byte value will change the Offset Register, which changes the
+current measurement done by the chip. Changing this register to the two's complement
+of the current register while forcing zero current through the load will calibrate
+the chip, canceling offset errors in the current ADC.
+
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 2cfdfedb584f..223a9aae6e2d 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}

+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last register reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR_RW(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-09 03:17:34

by Luiz Sampaio

Subject: [PATCH v6 5/6] w1: ds2438: adding support for reading page1

Added a sysfs entry to support reading the page1 registers. This registers
contain Elapsed Time Meter (ETM) data, which shows for how long the chip is
on, as well as an Offset Register data, which can be used to calibrate the
current measurement of the chip.

Signed-off-by: Luiz Sampaio <[email protected]>
---
.../ABI/stable/sysfs-driver-w1_ds2438 | 6 +++
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 Documentation/ABI/stable/sysfs-driver-w1_ds2438

diff --git a/Documentation/ABI/stable/sysfs-driver-w1_ds2438 b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
new file mode 100644
index 000000000000..fa47437c11d9
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
@@ -0,0 +1,6 @@
+What: /sys/bus/w1/devices/.../page1
+Date: April 2021
+Contact: Luiz Sampaio <[email protected]>
+Description: read the contents of the page1 of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ef6217ecb1cb..2cfdfedb584f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07

+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,

static BIN_ATTR_RW(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-09 03:18:13

by Luiz Sampaio

Subject: [PATCH v6 1/6] w1: ds2438: fixed a coding style issue

There is an if statement and, if the function goes into it, it returns. So,
the next else is not required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 5cfb0ae23e91..148921fb9702 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -154,11 +154,11 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)

if ((status & mask) == value)
return 0; /* already set as requested */
- else {
- /* changing bit */
- status ^= mask;
- perform_write = 1;
- }
+
+ /* changing bit */
+ status ^= mask;
+ perform_write = 1;
+
break;
}

--
2.30.1

2021-04-09 09:46:01

by kernel test robot

Subject: Re: [PATCH v6 3/6] w1: ds2438: fixed a coding style issue

Hi Luiz,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc6 next-20210408]
[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/Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 17e7124aad766b3f158943acb51467f86220afe9
config: x86_64-randconfig-a004-20210409 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project dd453a1389b6a7e6d9214b449d3c54981b1a89b6)
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
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
git checkout 3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/w1/slaves/w1_ds2438.c:391:31: error: too many arguments provided to function-like macro invocation
static BIN_ATTR_RW(iad, 0664, iad_write, 0);
^
include/linux/sysfs.h:229:9: note: macro 'BIN_ATTR_RW' defined here
#define BIN_ATTR_RW(_name, _size) \
^
drivers/w1/slaves/w1_ds2438.c:398:3: error: use of undeclared identifier 'bin_attr_iad'; did you mean 'bin_attr_vad'?
&bin_attr_iad,
^~~~~~~~~~~~
bin_attr_vad
drivers/w1/slaves/w1_ds2438.c:394:8: note: 'bin_attr_vad' declared here
static BIN_ATTR_RO(vad, 0/* real length varies */);
^
include/linux/sysfs.h:224:22: note: expanded from macro 'BIN_ATTR_RO'
struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
^
<scratch space>:113:1: note: expanded from here
bin_attr_vad
^
2 errors generated.


vim +391 drivers/w1/slaves/w1_ds2438.c

390
> 391 static BIN_ATTR_RW(iad, 0664, iad_write, 0);
392 static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
393 static BIN_ATTR_RO(temperature, 0/* real length varies */);
394 static BIN_ATTR_RO(vad, 0/* real length varies */);
395 static BIN_ATTR_RO(vdd, 0/* real length varies */);
396

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2021-04-09 10:46:03

by kernel test robot

Subject: Re: [PATCH v6 3/6] w1: ds2438: fixed a coding style issue

Hi Luiz,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc6 next-20210408]
[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/Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 17e7124aad766b3f158943acb51467f86220afe9
config: arm-randconfig-r032-20210409 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.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/3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
git checkout 3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/w1/slaves/w1_ds2438.c:391:43: error: macro "BIN_ATTR_RW" passed 4 arguments, but takes just 2
391 | static BIN_ATTR_RW(iad, 0664, iad_write, 0);
| ^
In file included from include/linux/kobject.h:20,
from include/linux/module.h:20,
from drivers/w1/slaves/w1_ds2438.c:9:
include/linux/sysfs.h:229: note: macro "BIN_ATTR_RW" defined here
229 | #define BIN_ATTR_RW(_name, _size) \
|
>> drivers/w1/slaves/w1_ds2438.c:391:8: error: type defaults to 'int' in declaration of 'BIN_ATTR_RW' [-Werror=implicit-int]
391 | static BIN_ATTR_RW(iad, 0664, iad_write, 0);
| ^~~~~~~~~~~
drivers/w1/slaves/w1_ds2438.c:398:3: error: 'bin_attr_iad' undeclared here (not in a function); did you mean 'bin_attr_vad'?
398 | &bin_attr_iad,
| ^~~~~~~~~~~~
| bin_attr_vad
drivers/w1/slaves/w1_ds2438.c:391:8: warning: 'BIN_ATTR_RW' defined but not used [-Wunused-variable]
391 | static BIN_ATTR_RW(iad, 0664, iad_write, 0);
| ^~~~~~~~~~~
drivers/w1/slaves/w1_ds2438.c:277:16: warning: 'iad_read' defined but not used [-Wunused-function]
277 | static ssize_t iad_read(struct file *filp, struct kobject *kobj,
| ^~~~~~~~
drivers/w1/slaves/w1_ds2438.c:255:16: warning: 'iad_write' defined but not used [-Wunused-function]
255 | static ssize_t iad_write(struct file *filp, struct kobject *kobj,
| ^~~~~~~~~
cc1: some warnings being treated as errors

Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for ADI_AXI_ADC
Depends on IIO && HAS_IOMEM && OF
Selected by
- AD9467 && IIO && SPI


vim +/BIN_ATTR_RW +391 drivers/w1/slaves/w1_ds2438.c

390
> 391 static BIN_ATTR_RW(iad, 0664, iad_write, 0);
392 static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
393 static BIN_ATTR_RO(temperature, 0/* real length varies */);
394 static BIN_ATTR_RO(vad, 0/* real length varies */);
395 static BIN_ATTR_RO(vdd, 0/* real length varies */);
396

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2021-04-10 08:39:47

by Greg Kroah-Hartman

Subject: Re: [PATCH v6 3/6] w1: ds2438: fixed a coding style issue

On Fri, Apr 09, 2021 at 12:15:30AM -0300, Luiz Sampaio wrote:
> Changed the permissions to preferred octal style.
>
> Signed-off-by: Luiz Sampaio <[email protected]>
> ---
> drivers/w1/slaves/w1_ds2438.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
> index 56e53a748059..ccb06b8c2d78 100644
> --- a/drivers/w1/slaves/w1_ds2438.c
> +++ b/drivers/w1/slaves/w1_ds2438.c
> @@ -388,7 +388,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
> return ret;
> }
>
> -static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0);
> +static BIN_ATTR_RW(iad, 0664, iad_write, 0);

You obviously did not build this commit :(

And you did more than just fix a "coding style issue".

thanks,

greg k-h

2021-04-16 22:18:14

by Luiz Sampaio

Subject: [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements

The following patches aim to make a user able to calibrate the current
measurement of the DS2438. This chip uses a offset register in page1, which
is added to the current register to give the user the current measurement.
If this value is wrong, the user will get an offset current value, even if
the current is zero, for instance. This patch gives support for reading the
page1 registers (including the offset register) and for writing to the
offset register. The DS2438 datasheet shows a calibration routine, and with
this patch, the user can do this quickly by writing the correct value to
the offset register. This patch was tested on real hardware using a power
supply and an electronic load.
Please help to review this series of patches.

Best regards!
Sampaio
---
Changes in v7:
- Build test again

Changes in v6:
- Actually changing from BIN_ATTR to BIN_ATTR_RW

---
Changes in v5:
- Merged all brackets coding style issue in one patch
- Changing from BIN_ATTR to BIN_ATTR_RW in w1_ds2438.c
- Wrapping email lines
- Addind Documentation/ABI/

Changes in v4:
- Fixing different patches with identical subject lines as requested

Changes in v3:
- I accidentally added a wrong line that would not compile. I'm sorry. Fixed it.

Changes in v2:
- Using git send-email to send the patches
- Adding documentation as requested
- Separating the coding style changes in different patches as requested

Luiz Sampaio (6):
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed if brackets coding style issue
w1: ds2438: changed sysfs macro for rw file
w1: ds2438: fixing bug that would always get page0
w1: ds2438: adding support for reading page1
w1: ds2438: support for writing to offset register

.../ABI/stable/sysfs-driver-w1_ds2438 | 13 ++
Documentation/w1/slaves/w1_ds2438.rst | 19 ++-
drivers/w1/slaves/w1_ds2438.c | 122 +++++++++++++++---
3 files changed, 137 insertions(+), 17 deletions(-)
create mode 100644 Documentation/ABI/stable/sysfs-driver-w1_ds2438

--
2.30.1

2021-04-16 22:18:24

by Luiz Sampaio

Subject: [PATCH v7 1/6] w1: ds2438: fixed a coding style issue

There is an if statement and, if the function goes into it, it returns. So,
the next else is not required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 5cfb0ae23e91..148921fb9702 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -154,11 +154,11 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)

if ((status & mask) == value)
return 0; /* already set as requested */
- else {
- /* changing bit */
- status ^= mask;
- perform_write = 1;
- }
+
+ /* changing bit */
+ status ^= mask;
+ perform_write = 1;
+
break;
}

--
2.30.1

2021-04-16 22:18:30

by Luiz Sampaio

Subject: [PATCH v7 2/6] w1: ds2438: fixed if brackets coding style issue

Since there is only one statement inside the if clause, no brackets are
required.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 148921fb9702..56e53a748059 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_current(sl, &voltage) == 0) {
+ if (w1_ds2438_get_current(sl, &voltage) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
@@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_temperature(sl, &temp) == 0) {
+ if (w1_ds2438_get_temperature(sl, &temp) == 0)
ret = snprintf(buf, count, "%i\n", temp);
- } else
+ else
ret = -EIO;

return ret;
@@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
@@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;

- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;

return ret;
--
2.30.1

2021-04-16 22:18:39

by Luiz Sampaio

Subject: [PATCH v7 3/6] w1: ds2438: changed sysfs macro for rw file

The iad sysfs file has permissions for read and write. Changed to the
recommended macro BIN_ATTR_RW.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 56e53a748059..910e25163898 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -388,7 +388,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
return ret;
}

-static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0);
+static BIN_ATTR_RW(iad, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
--
2.30.1

2021-04-16 22:19:09

by Luiz Sampaio

Subject: [PATCH v7 5/6] w1: ds2438: adding support for reading page1

Added a sysfs entry to support reading the page1 registers. This registers
contain Elapsed Time Meter (ETM) data, which shows for how long the chip is
on, as well as an Offset Register data, which can be used to calibrate the
current measurement of the chip.

Signed-off-by: Luiz Sampaio <[email protected]>
---
.../ABI/stable/sysfs-driver-w1_ds2438 | 6 +++
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 Documentation/ABI/stable/sysfs-driver-w1_ds2438

diff --git a/Documentation/ABI/stable/sysfs-driver-w1_ds2438 b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
new file mode 100644
index 000000000000..fa47437c11d9
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
@@ -0,0 +1,6 @@
+What: /sys/bus/w1/devices/.../page1
+Date: April 2021
+Contact: Luiz Sampaio <[email protected]>
+Description: read the contents of the page1 of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 1e95f3a256c7..42080ae779f0 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07

+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,

static BIN_ATTR_RW(iad, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-16 22:19:51

by Luiz Sampaio

Subject: [PATCH v7 6/6] w1: ds2438: support for writing to offset register

Added a sysfs entry to support writing to the offset register on page1.
This register is used to calibrate the chip canceling offset errors in the
current ADC. This means that, over time, reading the IAD register will not
return the correct current measurement, it will have an offset. Writing to
the offset register if the two's complement of the current register while
passing zero current to the load will calibrate the measurements. This
change was tested on real hardware and it was able to calibrate the chip
correctly.

Signed-off-by: Luiz Sampaio <[email protected]>
---
.../ABI/stable/sysfs-driver-w1_ds2438 | 7 +++
Documentation/w1/slaves/w1_ds2438.rst | 11 ++++-
drivers/w1/slaves/w1_ds2438.c | 49 +++++++++++++++++++
3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/stable/sysfs-driver-w1_ds2438 b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
index fa47437c11d9..d2e7681cc287 100644
--- a/Documentation/ABI/stable/sysfs-driver-w1_ds2438
+++ b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
@@ -4,3 +4,10 @@ Contact: Luiz Sampaio <[email protected]>
Description: read the contents of the page1 of the DS2438
see Documentation/w1/slaves/w1_ds2438.rst for detailed information
Users: any user space application which wants to communicate with DS2438
+
+What: /sys/bus/w1/devices/.../offset
+Date: April 2021
+Contact: Luiz Sampaio <[email protected]>
+Description: write the contents to the offset register of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index ac8d0d4b0d0e..5c5573991351 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -22,7 +22,7 @@ is also often used in weather stations and applications such as: rain gauge,
wind speed/direction measuring, humidity sensing, etc.

Current support is provided through the following sysfs files (all files
-except "iad" are readonly):
+except "iad" and "offset" are readonly):

"iad"
-----
@@ -52,6 +52,15 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.

+"offset"
+-------
+This file controls the 2-byte Offset Register of the chip.
+Writing a 2-byte value will change the Offset Register, which changes the
+current measurement done by the chip. Changing this register to the two's complement
+of the current register while forcing zero current through the load will calibrate
+the chip, canceling offset errors in the current ADC.
+
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 42080ae779f0..9c39bd6f5fcc 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}

+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last register reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}

+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR_RW(iad, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1

2021-04-16 22:20:46

by Luiz Sampaio

Subject: [PATCH v7 4/6] w1: ds2438: fixing bug that would always get page0

The purpose of the w1_ds2438_get_page function is to get the register
values at the page passed as the pageno parameter. However, the page0 was
hardcoded, such that the function always returned the page0 contents. Fixed
so that the function can retrieve any page.

Signed-off-by: Luiz Sampaio <[email protected]>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 910e25163898..1e95f3a256c7 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -62,13 +62,13 @@ static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_RECALL_MEMORY;
- w1_buf[1] = 0x00;
+ w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);

if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_READ_SCRATCH;
- w1_buf[1] = 0x00;
+ w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);

count = w1_read_block(sl->master, buf, DS2438_PAGE_SIZE + 1);
--
2.30.1

2021-04-17 05:30:05

by kernel test robot

Subject: Re: [PATCH v7 6/6] w1: ds2438: support for writing to offset register

Hi Luiz,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.12-rc7 next-20210416]
[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/Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210417-071754
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 151501160401e2dc669ea7dac2c599b53f220c33
config: csky-randconfig-m031-20210416 (attached as .config)
compiler: csky-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

smatch warnings:
drivers/w1/slaves/w1_ds2438.c:218 w1_ds2438_change_offset_register() warn: inconsistent indenting

vim +218 drivers/w1/slaves/w1_ds2438.c

195
196 static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
197 {
198 unsigned int retries = W1_DS2438_RETRIES;
199 u8 w1_buf[9];
200 u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
201
202 if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
203 memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last register reserved */
204 w1_buf[7] = value[0]; /* change only offset register */
205 w1_buf[8] = value[1];
206 while (retries--) {
207 if (w1_reset_select_slave(sl))
208 continue;
209 w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
210 w1_buf[1] = 0x01; /* write to page 1 */
211 w1_write_block(sl->master, w1_buf, 9);
212
213 if (w1_reset_select_slave(sl))
214 continue;
215 w1_buf[0] = W1_DS2438_COPY_SCRATCH;
216 w1_buf[1] = 0x01;
217 w1_write_block(sl->master, w1_buf, 2);
> 218 return 0;
219 }
220 }
221 return -1;
222 }
223

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2021-05-14 17:42:47

by Greg Kroah-Hartman

Subject: Re: [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements

On Fri, Apr 16, 2021 at 07:17:33PM -0300, Luiz Sampaio wrote:
> The following patches aim to make a user able to calibrate the current
> measurement of the DS2438. This chip uses a offset register in page1, which
> is added to the current register to give the user the current measurement.
> If this value is wrong, the user will get an offset current value, even if
> the current is zero, for instance. This patch gives support for reading the
> page1 registers (including the offset register) and for writing to the
> offset register. The DS2438 datasheet shows a calibration routine, and with
> this patch, the user can do this quickly by writing the correct value to
> the offset register. This patch was tested on real hardware using a power
> supply and an electronic load.
> Please help to review this series of patches.
>
> Best regards!
> Sampaio
> ---
> Changes in v7:
> - Build test again

Can you fix the warning in patch 6/6 that the kernel test robot found,
and resend?

thanks,

greg k-h

2021-05-19 22:32:04

by Luiz Sampaio

Subject: [PATCH v8 0/6] w1: ds2438: adding support for calibration of current measurements

The following patches aim to make a user able to calibrate the current
measurement of the DS2438. This chip uses a offset register in page1, which
is added to the current register to give the user the current measurement.
If this value is wrong, the user will get an offset current value, even if
the current is zero, for instance. This patch gives support for reading the
page1 registers (including the offset register) and for writing to the
offset register. The DS2438 datasheet shows a calibration routine, and with
this patch, the user can do this quickly by writing the correct value to
the offset register. This patch was tested on real hardware using a power
supply and an electronic load.
Please help to review this series of patches.

Best regards!
Sampaio
---
Changes in v8:
- Fixing bot identation warning

Changes in v7:
- Build test again

Changes in v6:
- Actually changing from BIN_ATTR to BIN_ATTR_RW

---
Changes in v5:
- Merged all brackets coding style issue in one patch
- Changing from BIN_ATTR to BIN_ATTR_RW in w1_ds2438.c
- Wrapping email lines
- Addind Documentation/ABI/

Changes in v4:
- Fixing different patches with identical subject lines as requested

Changes in v3:
- I accidentally added a wrong line that would not compile. I'm sorry. Fixed it.

Changes in v2:
- Using git send-email to send the patches
- Adding documentation as requested
- Separating the coding style changes in different patches as requested

Luiz Sampaio (6):
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed if brackets coding style issue
w1: ds2438: changed sysfs macro for rw file
w1: ds2438: fixing bug that would always get page0
w1: ds2438: adding support for reading page1
w1: ds2438: support for writing to offset register

.../ABI/stable/sysfs-driver-w1_ds2438 | 13 ++
Documentation/w1/slaves/w1_ds2438.rst | 19 ++-
drivers/w1/slaves/w1_ds2438.c | 122 +++++++++++++++---
3 files changed, 137 insertions(+), 17 deletions(-)
create mode 100644 Documentation/ABI/stable/sysfs-driver-w1_ds2438

--
2.30.1