Re: [PATCH v5 2/6] w1: ds2438: fixed if brackets coding style issue

[Luiz Sampaio]

On Fri, Apr 09, 2021 at 07:40:57AM -0700, Joe Perches wrote:
> On Fri, 2021-04-09 at 00:09 -0300, Luiz Sampaio wrote:
> > Since there is only one statement inside the if clause, no brackets are
> > required.
> []
> > diff --git 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, ) == 0) {
> > +   if (w1_ds2438_get_current(sl, ) == 0)
> >     ret = snprintf(buf, count, "%i\n", voltage);
> > -   } else
> > +   else
> >     ret = -EIO;
> >  
> > 
> >     return ret;
> 
> to me this would look better using a style like the below:
> (and it might be better using sysfs_emit and not snprintf too)
> 
> ---
>  drivers/w1/slaves/w1_ds2438.c | 36 
>  1 file changed, 12 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
> index 5cfb0ae23e91..9115c5a9bc4f 100644
> --- a/drivers/w1/slaves/w1_ds2438.c
> +++ b/drivers/w1/slaves/w1_ds2438.c
> @@ -279,7 +279,6 @@ static ssize_t iad_read(struct file *filp, struct kobject 
> *kobj,
>   loff_t off, size_t count)
>  {
>   struct w1_slave *sl = kobj_to_w1_slave(kobj);
> - int ret;
>   int16_t voltage;
>  
>   if (off != 0)
> @@ -287,12 +286,10 @@ static ssize_t iad_read(struct file *filp, struct 
> kobject *kobj,
>   if (!buf)
>   return -EINVAL;
>  
> - if (w1_ds2438_get_current(sl, ) == 0) {
> - ret = snprintf(buf, count, "%i\n", voltage);
> - } else
> - ret = -EIO;
> + if (w1_ds2438_get_current(sl, ))
> + return -EIO;
>  
> - return ret;
> + return snprintf(buf, count, "%i\n", voltage);
>  }
>  
>  static ssize_t page0_read(struct file *filp, struct kobject *kobj,
> @@ -330,7 +327,6 @@ static ssize_t temperature_read(struct file *filp, struct 
> kobject *kobj,
>   loff_t off, size_t count)
>  {
>   struct w1_slave *sl = kobj_to_w1_slave(kobj);
> - int ret;
>   int16_t temp;
>  
>   if (off != 0)
> @@ -338,12 +334,10 @@ static ssize_t temperature_read(struct file *filp, 
> struct kobject *kobj,
>   if (!buf)
>   return -EINVAL;
>  
> - if (w1_ds2438_get_temperature(sl, ) == 0) {
> - ret = snprintf(buf, count, "%i\n", temp);
> - } else
> - ret = -EIO;
> + if (w1_ds2438_get_temperature(sl, ))
> + return -EIO;
>  
> - return ret;
> + return snprintf(buf, count, "%i\n", temp);
>  }
>  
>  static ssize_t vad_read(struct file *filp, struct kobject *kobj,
> @@ -351,7 +345,6 @@ static ssize_t vad_read(struct file *filp, struct kobject 
> *kobj,
>   loff_t off, size_t count)
>  {
>   struct w1_slave *sl = kobj_to_w1_slave(kobj);
> - int ret;
>   uint16_t voltage;
>  
>   if (off != 0)
> @@ -359,12 +352,10 @@ 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, ) == 0) {
> - ret = snprintf(buf, count, "%u\n", voltage);
> - } else
> - ret = -EIO;
> + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ))
> + return -EIO;
>  
> - return ret;
> + return snprintf(buf, count, "%u\n", voltage);
>  }
>  
>  static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
> @@ -372,7 +363,6 @@ static ssize_t vdd_read(struct file *filp, struct kobject 
> *kobj,
>   loff_t off, size_t count)
>  {
>   struct w1_slave *sl = kobj_to_w1_slave(kobj);
> - int ret;
>   uint16_t voltage;
>  
>   if (off != 0)
> @@ -380,12 +370,10 @@ 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, ) == 0) {
> - ret = snprintf(buf, count, "%u\n", voltage);
> - } else
> - ret = -EIO;
> + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ))
> + return -EIO;
>  
> - return ret;
> + return snprintf(buf, count, "%u\n", voltage);
>  }
>  
>  static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0);
>

Sorry for the late reply! I agree, this would look nicer. I will wait for
the current revision and change this for the next one. 

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

[Luiz Sampaio]

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 
---
 .../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 
 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 
+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(_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_sla

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

[Luiz Sampaio]

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 
---
 .../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 ..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 
+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(>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, _buf, count);
+   ret = count;
+   } else
+   ret = -EIO;
+
+   mutex_unlock(>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[] = {
_attr_iad,
_attr_page0,
+   _attr_page1,
_attr_temperature,
_attr_vad,
_attr_vdd,
-- 
2.30.1

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_current(sl, ) == 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, ) == 0) {
+   if (w1_ds2438_get_temperature(sl, ) == 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

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

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

[Luiz Sampaio]

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 
---
 .../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 
 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 
+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(_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_sla

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

[Luiz Sampaio]

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 
---
 .../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 ..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 
+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(>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, _buf, count);
+   ret = count;
+   } else
+   ret = -EIO;
+
+   mutex_unlock(>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[] = {
_attr_iad,
_attr_page0,
+   _attr_page1,
_attr_temperature,
_attr_vad,
_attr_vdd,
-- 
2.30.1

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_current(sl, ) == 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, ) == 0) {
+   if (w1_ds2438_get_temperature(sl, ) == 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

Changed the permissions to preferred octal style.

Signed-off-by: Luiz Sampaio 
---
 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

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

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

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

[Luiz Sampaio]

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 
---
 .../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 
 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 
+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(_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_sla

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

[Luiz Sampaio]

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 
---
 .../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 ..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 
+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(>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, _buf, count);
+   ret = count;
+   } else
+   ret = -EIO;
+
+   mutex_unlock(>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[] = {
_attr_iad,
_attr_page0,
+   _attr_page1,
_attr_temperature,
_attr_vad,
_attr_vdd,
-- 
2.30.1

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

Changed the permissions to preferred octal style.

Signed-off-by: Luiz Sampaio 
---
 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

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_current(sl, ) == 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, ) == 0) {
+   if (w1_ds2438_get_temperature(sl, ) == 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

Re: [PATCH v4 6/9] w1: ds2438: fixed a coding style issue to preferred octal style

[Luiz Sampaio]

On Mon, Apr 05, 2021 at 12:53:22PM +0200, Greg KH wrote:
> On Mon, Apr 05, 2021 at 07:50:06AM -0300, Luiz Sampaio wrote:
> > Changed the permissions to preferred octal style.
> > 
> > Signed-off-by: Luiz Sampaio 
> > ---
> >  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);
> 
> Why not BIN_ATTR_RW() instead?
> 
> thanks,
> 
> greg k-h

I agree! Thanks for the review. I will change for the next version.

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

[Luiz Sampaio]

On Mon, Apr 05, 2021 at 12:53:38PM +0200, Greg KH wrote:
> On Mon, Apr 05, 2021 at 07:50:00AM -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.
> 
> Please linewrap your text here :(
>

Hello! Thanks for the review! I'm sorry about this again, I did it for my 
patches but forgot the cover letter. Won't repeat! 

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

[Luiz Sampaio]

On Mon, Apr 05, 2021 at 01:04:59PM +0200, Greg KH wrote:
> 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 
> > ---
> >  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

Hello! I'm sorry about some errors, this is my first patch and I'm not sure 
about some things in the documentation. I really appreciate the responses and 
guidance.
The file I need to add to Documentation/ABI/ is going to be in testing or 
stable? And the file I need to create can be called, for instance, 
sysfs-driver-w1_ds2438?

Thanks!

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

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 
---
 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(_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(>master->bus_mutex);
+
+   if (w1_ds2438_change_offset_register(sl, buf) == 0)
+   ret = count;
+   else
+   ret = -EIO;
+
+   mutex_unlock(>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_ds24

[PATCH v4 6/9] w1: ds2438: fixed a coding style issue to preferred octal style

[Luiz Sampaio]

Changed the permissions to preferred octal style.

Signed-off-by: Luiz Sampaio 
---
 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

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

[Luiz Sampaio]

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 
---
 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(>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, _buf, count);
+   ret = count;
+   } else
+   ret = -EIO;
+
+   mutex_unlock(>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[] = {
_attr_iad,
_attr_page0,
+   _attr_page1,
_attr_temperature,
_attr_vad,
_attr_vdd,
-- 
2.30.1

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_temperature(sl, ) == 0)
ret = snprintf(buf, count, "%i\n", temp);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_current(sl, ) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

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 
---
 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(_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(>master->bus_mutex);
+
+   if (w1_ds2438_change_offset_register(sl, buf) == 0)
+   ret = count;
+   else
+   ret = -EIO;
+
+   mutex_unlock(>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_ds24

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

[Luiz Sampaio]

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 
---
 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(>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, _buf, count);
+   ret = count;
+   } else
+   ret = -EIO;
+
+   mutex_unlock(>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[] = {
_attr_iad,
_attr_page0,
+   _attr_page1,
_attr_temperature,
_attr_vad,
_attr_vdd,
-- 
2.30.1

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

Changed the permissions to preferred octal style.

Signed-off-by: Luiz Sampaio 
---
 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

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_temperature(sl, ) == 0)
ret = snprintf(buf, count, "%i\n", temp);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_current(sl, ) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

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

[Luiz Sampaio]

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 
---
 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(_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(>master->bus_mutex);
+
+   if (w1_ds2438_change_offset_register(sl, buf) == 0)
+   ret = count;
+   else
+   ret = -EIO;
+
+   mutex_unlock(>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[] = {
   

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

[Luiz Sampaio]

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 
---
 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(>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, _buf, count);
+   ret = count;
+   } else
+   ret = -EIO;
+
+   mutex_unlock(>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[] = {
_attr_iad,
_attr_page0,
+   _attr_page1,
_attr_temperature,
_attr_vad,
_attr_vdd,
-- 
2.30.1

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

Changed the permissions to preferred octal style.

Signed-off-by: Luiz Sampaio 
---
 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

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_current(sl, ) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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

Signed-off-by: Luiz Sampaio 
---
 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, ) == 0) {
+   if (w1_ds2438_get_temperature(sl, ) == 0)
ret = snprintf(buf, count, "%i\n", temp);
-   } else
+   else
ret = -EIO;
 
return ret;
-- 
2.30.1

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

[Luiz Sampaio]

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 
---
 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

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

[Luiz Sampaio]

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

[PATCH 3/3] w1: ds2438: adding support for accessing page1 registers

[Luiz Sampaio]

The ds2438 has a register in page1 called offset, which dictates
how the current measurement is done. If the offset register is not
calibrated well, pulling zero current from a battery shows a non-zero
current measurement.
Gives write access to the offset register. This way, a user following
the calibration steps explained in the datasheet can use this driver
to fully calibrate de current measurements.

Signed-off-by: Luiz Sampaio 
---
 drivers/w1/slaves/w1_ds2438.c | 94 ++-
 1 file changed, 92 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 08f4b451c349..5d58986bb1a9 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;
@@ -62,13 +71,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] = pageno;
+   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] = pageno;
+   w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);
 
count = w1_read_block(sl->master, buf, DS2438_PAGE_SIZE + 1);
@@ -182,6 +191,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(_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last 
register 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)
 {
@@ -323,6 +360,55 @@ 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(>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, _buf, count);
+   ret = count;
+   } else
+   ret = -EIO;
+
+   mutex_unlock(>master->bus_mutex);
+
+   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(>master->bus_mutex);
+
+   if (w1_ds2438_change_offset_register(sl, buf) == 0)
+   ret = count;
+   else
+   ret = -EIO;
+
+   mutex_unlock(>master->bus_mutex);
+
+   return ret;
+}
+
 static ssize_t temperature_re

[PATCH 2/3] w1: ds2438: fixed bug in get_page function

[Luiz Sampaio]

The get_page function always returned register values from page 0.
Fixes by writing the actual page number to the bus.

Signed-off-by: Luiz Sampaio 
---
 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 a9884fc8c726..08f4b451c349 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] = 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] = pageno;
w1_write_block(sl->master, w1_buf, 2);
 
count = w1_read_block(sl->master, buf, DS2438_PAGE_SIZE + 1);
-- 
2.30.1

[PATCH 1/3] w1: ds2438: fixed a coding style issues

[Luiz Sampaio]

Fixed coding style issues.

Signed-off-by: Luiz Sampaio 
---
 drivers/w1/slaves/w1_ds2438.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 5cfb0ae23e91..a9884fc8c726 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -154,11 +154,9 @@ 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;
}
 
@@ -287,9 +285,9 @@ static ssize_t iad_read(struct file *filp, struct kobject 
*kobj,
if (!buf)
return -EINVAL;
 
-   if (w1_ds2438_get_current(sl, ) == 0) {
+   if (w1_ds2438_get_current(sl, ) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
@@ -338,9 +336,9 @@ static ssize_t temperature_read(struct file *filp, struct 
kobject *kobj,
if (!buf)
return -EINVAL;
 
-   if (w1_ds2438_get_temperature(sl, ) == 0) {
+   if (w1_ds2438_get_temperature(sl, ) == 0)
ret = snprintf(buf, count, "%i\n", temp);
-   } else
+   else
ret = -EIO;
 
return ret;
@@ -359,9 +357,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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
return ret;
@@ -380,15 +378,15 @@ 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, ) == 0) {
+   if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
-   } else
+   else
ret = -EIO;
 
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

[PATCH 0/3] w1: ds2438: adding support for calibration of current measurements

[Luiz Sampaio]

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 to read the page1 registers 
(including the offset register) and to write to the offset register. The ds2438 
datasheet shows a calibration routine, and with this patch, the user can to 
this quickly by write 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

Luiz Sampaio (3):
  w1: ds2438: fixed a coding style issues
  w1: ds2438: fixed bug in get_page function
  w1: ds2438: adding support for accessing page1 registers

 drivers/w1/slaves/w1_ds2438.c | 120 +-
 1 file changed, 104 insertions(+), 16 deletions(-)

-- 
2.30.1

[PATCH] w1: ds2438: fixing bug in get_page function

[Luiz Sampaio]

In the w1_ds2438_get_page function, there is an argument to change the page
number you want to read from the chip. But this was always getting the page
0, not the pageno page. Fixed it.

Also fixed coding style issue.

Signed-off-by: Luiz Sampaio 
---
 drivers/w1/slaves/w1_ds2438.c | 31 +++
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 5cfb0ae23e91..0eb667f0cf6e 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] = 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] = pageno;
     w1_write_block(sl->master, w1_buf, 2);
 
     count = w1_read_block(sl->master, buf, DS2438_PAGE_SIZE + 1);
@@ -154,11 +154,10 @@ 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;
 }
 
@@ -287,9 +286,9 @@ static ssize_t iad_read(struct file *filp, struct kobject 
*kobj,
 if (!buf)
     return -EINVAL;
 
-    if (w1_ds2438_get_current(sl, ) == 0) {
+    if (w1_ds2438_get_current(sl, ) == 0)
     ret = snprintf(buf, count, "%i\n", voltage);
-    } else
+    else
     ret = -EIO;
 
 return ret;
@@ -338,9 +337,9 @@ static ssize_t temperature_read(struct file *filp, struct 
kobject *kobj,
 if (!buf)
     return -EINVAL;
 
-    if (w1_ds2438_get_temperature(sl, ) == 0) {
+    if (w1_ds2438_get_temperature(sl, ) == 0)
     ret = snprintf(buf, count, "%i\n", temp);
-    } else
+    else
     ret = -EIO;
 
 return ret;
@@ -359,9 +358,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, ) == 0) {
+    if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, ) == 0)
     ret = snprintf(buf, count, "%u\n", voltage);
-    } else
+    else
     ret = -EIO;
 
 return ret;
@@ -380,15 +379,15 @@ 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, ) == 0) {
+    if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, ) == 0)
     ret = snprintf(buf, count, "%u\n", voltage);
-    } else
+    else
     ret = -EIO;
 
 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