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

2021-04-05 Thread Greg KH
On Mon, Apr 05, 2021 at 09:44:01AM -0300, Luiz Sampaio wrote:
> 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.

No problem, again you should try wrapping your email lines :)

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

stable I guess as you know this is what you want to do.

And the file name seems right, thanks.

greg k-h


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

2021-04-05 Thread 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!


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

2021-04-05 Thread Greg KH
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


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

2021-04-05 Thread 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_ds2438_bin_attrs[] = {
_attr_iad,
_attr_page0,
_attr_page1,
+