* [PATCH v4 1/9] w1: ds2438: fixed a coding style issue after return
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
@ 2021-04-05 10:50 ` Luiz Sampaio
2021-04-05 10:50 ` [PATCH v4 2/9] w1: ds2438: fixed a coding style issue in iad_read Luiz Sampaio
` (9 subsequent siblings)
10 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 10:50 UTC (permalink / raw)
To: zbr; +Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v4 2/9] w1: ds2438: fixed a coding style issue in iad_read
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
2021-04-05 10:50 ` [PATCH v4 1/9] w1: ds2438: fixed a coding style issue after return Luiz Sampaio
@ 2021-04-05 10:50 ` Luiz Sampaio
2021-04-05 10:52 ` Greg KH
2021-04-05 10:50 ` [PATCH v4 3/9] w1: ds2438: fixed a coding style issue in temperature_read Luiz Sampaio
` (8 subsequent siblings)
10 siblings, 1 reply; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 10:50 UTC (permalink / raw)
To: zbr; +Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, Luiz Sampaio
Since there is only one statement inside the if clause, no brackets
are required.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 148921fb9702..a5bb53042c93 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_current(sl, &voltage) == 0) {
+ if (w1_ds2438_get_current(sl, &voltage) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* Re: [PATCH v4 2/9] w1: ds2438: fixed a coding style issue in iad_read
2021-04-05 10:50 ` [PATCH v4 2/9] w1: ds2438: fixed a coding style issue in iad_read Luiz Sampaio
@ 2021-04-05 10:52 ` Greg KH
0 siblings, 0 replies; 82+ messages in thread
From: Greg KH @ 2021-04-05 10:52 UTC (permalink / raw)
To: Luiz Sampaio; +Cc: zbr, corbet, rikard.falkeborn, linux-kernel
On Mon, Apr 05, 2021 at 07:50:02AM -0300, Luiz Sampaio wrote:
> Since there is only one statement inside the if clause, no brackets
> are required.
>
> Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
> ---
> drivers/w1/slaves/w1_ds2438.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
As this does the same type of fix as the next 3 patches, to the same
file, they should be all merged together into 1 patch.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 82+ messages in thread
* [PATCH v4 3/9] w1: ds2438: fixed a coding style issue in temperature_read
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
2021-04-05 10:50 ` [PATCH v4 1/9] w1: ds2438: fixed a coding style issue after return Luiz Sampaio
2021-04-05 10:50 ` [PATCH v4 2/9] w1: ds2438: fixed a coding style issue in iad_read Luiz Sampaio
@ 2021-04-05 10:50 ` Luiz Sampaio
2021-04-05 10:50 ` [PATCH v4 4/9] w1: ds2438: fixed a coding style issue in vad_read Luiz Sampaio
` (7 subsequent siblings)
10 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 10:50 UTC (permalink / raw)
To: zbr; +Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, Luiz Sampaio
Since there is only one statement inside the if clause, no brackets
are required.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index a5bb53042c93..eca50cec304f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_temperature(sl, &temp) == 0) {
+ if (w1_ds2438_get_temperature(sl, &temp) == 0)
ret = snprintf(buf, count, "%i\n", temp);
- } else
+ else
ret = -EIO;
return ret;
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v4 4/9] w1: ds2438: fixed a coding style issue in vad_read
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (2 preceding siblings ...)
2021-04-05 10:50 ` [PATCH v4 3/9] w1: ds2438: fixed a coding style issue in temperature_read Luiz Sampaio
@ 2021-04-05 10:50 ` Luiz Sampaio
2021-04-05 10:50 ` [PATCH v4 5/9] w1: ds2438: fixed a coding style issue in vdd_read Luiz Sampaio
` (6 subsequent siblings)
10 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 10:50 UTC (permalink / raw)
To: zbr; +Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, Luiz Sampaio
Since there is only one statement inside the if clause, no brackets
are required.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index eca50cec304f..eeb593294fbd 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v4 5/9] w1: ds2438: fixed a coding style issue in vdd_read
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (3 preceding siblings ...)
2021-04-05 10:50 ` [PATCH v4 4/9] w1: ds2438: fixed a coding style issue in vad_read Luiz Sampaio
@ 2021-04-05 10:50 ` Luiz Sampaio
2021-04-05 10:50 ` [PATCH v4 6/9] w1: ds2438: fixed a coding style issue to preferred octal style Luiz Sampaio
` (5 subsequent siblings)
10 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 10:50 UTC (permalink / raw)
To: zbr; +Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, Luiz Sampaio
Since there is only one statement inside the if clause, no brackets
are required.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
drivers/w1/slaves/w1_ds2438.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index eeb593294fbd..56e53a748059 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v4 6/9] w1: ds2438: fixed a coding style issue to preferred octal style
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (4 preceding siblings ...)
2021-04-05 10:50 ` [PATCH v4 5/9] w1: ds2438: fixed a coding style issue in vdd_read Luiz Sampaio
@ 2021-04-05 10:50 ` Luiz Sampaio
2021-04-05 10:53 ` Greg KH
2021-04-05 10:50 ` [PATCH v4 7/9] w1: ds2438: fixing bug that would always get page0 Luiz Sampaio
` (4 subsequent siblings)
10 siblings, 1 reply; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 10:50 UTC (permalink / raw)
To: zbr; +Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, Luiz Sampaio
Changed the permissions to preferred octal style.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* Re: [PATCH v4 6/9] w1: ds2438: fixed a coding style issue to preferred octal style
2021-04-05 10:50 ` [PATCH v4 6/9] w1: ds2438: fixed a coding style issue to preferred octal style Luiz Sampaio
@ 2021-04-05 10:53 ` Greg KH
2021-04-05 12:47 ` Luiz Sampaio
0 siblings, 1 reply; 82+ messages in thread
From: Greg KH @ 2021-04-05 10:53 UTC (permalink / raw)
To: Luiz Sampaio; +Cc: zbr, corbet, rikard.falkeborn, linux-kernel
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 <sampaio.ime@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [PATCH v4 6/9] w1: ds2438: fixed a coding style issue to preferred octal style
2021-04-05 10:53 ` Greg KH
@ 2021-04-05 12:47 ` Luiz Sampaio
0 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 12:47 UTC (permalink / raw)
To: Greg KH; +Cc: zbr, corbet, rikard.falkeborn, gregkh, linux-kernel
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 <sampaio.ime@gmail.com>
> > ---
> > 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.
^ permalink raw reply [flat|nested] 82+ messages in thread
* [PATCH v4 7/9] w1: ds2438: fixing bug that would always get page0
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (5 preceding siblings ...)
2021-04-05 10:50 ` [PATCH v4 6/9] w1: ds2438: fixed a coding style issue to preferred octal style Luiz Sampaio
@ 2021-04-05 10:50 ` Luiz Sampaio
2021-04-05 10:50 ` [PATCH v4 8/9] w1: ds2438: adding support for reading page1 Luiz Sampaio
` (3 subsequent siblings)
10 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 10:50 UTC (permalink / raw)
To: zbr; +Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v4 8/9] w1: ds2438: adding support for reading page1
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (6 preceding siblings ...)
2021-04-05 10:50 ` [PATCH v4 7/9] w1: ds2438: fixing bug that would always get page0 Luiz Sampaio
@ 2021-04-05 10:50 ` Luiz Sampaio
2021-04-05 10:50 ` [PATCH v4 9/9] w1: ds2438: support for writing to offset register Luiz Sampaio
` (2 subsequent siblings)
10 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 10:50 UTC (permalink / raw)
To: zbr; +Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, 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 <sampaio.ime@gmail.com>
---
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.
+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ef6217ecb1cb..2cfdfedb584f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07
+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}
+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR(iad, 0664, iad_read, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v4 9/9] w1: ds2438: support for writing to offset register
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (7 preceding siblings ...)
2021-04-05 10:50 ` [PATCH v4 8/9] w1: ds2438: adding support for reading page1 Luiz Sampaio
@ 2021-04-05 10:50 ` Luiz Sampaio
2021-04-05 11:04 ` Greg KH
2021-04-05 10:53 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Greg KH
2021-04-09 3:09 ` [PATCH v5 " Luiz Sampaio
10 siblings, 1 reply; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 10:50 UTC (permalink / raw)
To: zbr; +Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, 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 <sampaio.ime@gmail.com>
---
Documentation/w1/slaves/w1_ds2438.rst | 11 +++++-
drivers/w1/slaves/w1_ds2438.c | 49 +++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index ac8d0d4b0d0e..5c5573991351 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -22,7 +22,7 @@ is also often used in weather stations and applications such as: rain gauge,
wind speed/direction measuring, humidity sensing, etc.
Current support is provided through the following sysfs files (all files
-except "iad" are readonly):
+except "iad" and "offset" are readonly):
"iad"
-----
@@ -52,6 +52,15 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.
+"offset"
+-------
+This file controls the 2-byte Offset Register of the chip.
+Writing a 2-byte value will change the Offset Register, which changes the
+current measurement done by the chip. Changing this register to the two's complement
+of the current register while forcing zero current through the load will calibrate
+the chip, canceling offset errors in the current ADC.
+
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 2cfdfedb584f..223a9aae6e2d 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}
+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* register 7 is reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}
+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR(iad, 0664, iad_read, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* Re: [PATCH v4 9/9] w1: ds2438: support for writing to offset register
2021-04-05 10:50 ` [PATCH v4 9/9] w1: ds2438: support for writing to offset register Luiz Sampaio
@ 2021-04-05 11:04 ` Greg KH
2021-04-05 12:44 ` Luiz Sampaio
0 siblings, 1 reply; 82+ messages in thread
From: Greg KH @ 2021-04-05 11:04 UTC (permalink / raw)
To: Luiz Sampaio; +Cc: zbr, corbet, rikard.falkeborn, linux-kernel
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 <sampaio.ime@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [PATCH v4 9/9] w1: ds2438: support for writing to offset register
2021-04-05 11:04 ` Greg KH
@ 2021-04-05 12:44 ` Luiz Sampaio
2021-04-05 12:50 ` Greg KH
0 siblings, 1 reply; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 12:44 UTC (permalink / raw)
To: Greg KH; +Cc: zbr, rikard.falkeborn, gregkh, linux-kernel
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 <sampaio.ime@gmail.com>
> > ---
> > 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!
^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [PATCH v4 9/9] w1: ds2438: support for writing to offset register
2021-04-05 12:44 ` Luiz Sampaio
@ 2021-04-05 12:50 ` Greg KH
0 siblings, 0 replies; 82+ messages in thread
From: Greg KH @ 2021-04-05 12:50 UTC (permalink / raw)
To: Luiz Sampaio; +Cc: zbr, rikard.falkeborn, linux-kernel
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 <sampaio.ime@gmail.com>
> > > ---
> > > 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
^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (8 preceding siblings ...)
2021-04-05 10:50 ` [PATCH v4 9/9] w1: ds2438: support for writing to offset register Luiz Sampaio
@ 2021-04-05 10:53 ` Greg KH
2021-04-05 12:45 ` Luiz Sampaio
2021-04-09 3:09 ` [PATCH v5 " Luiz Sampaio
10 siblings, 1 reply; 82+ messages in thread
From: Greg KH @ 2021-04-05 10:53 UTC (permalink / raw)
To: Luiz Sampaio; +Cc: zbr, corbet, rikard.falkeborn, linux-kernel
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 :(
^ permalink raw reply [flat|nested] 82+ messages in thread* Re: [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements
2021-04-05 10:53 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Greg KH
@ 2021-04-05 12:45 ` Luiz Sampaio
0 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-05 12:45 UTC (permalink / raw)
To: Greg KH; +Cc: zbr, corbet, rikard.falkeborn, gregkh, linux-kernel
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!
^ permalink raw reply [flat|nested] 82+ messages in thread
* [PATCH v5 0/9] w1: ds2438: adding support for calibration of current measurements
2021-04-05 10:50 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (9 preceding siblings ...)
2021-04-05 10:53 ` [PATCH v4 0/9] w1: ds2438: adding support for calibration of current measurements Greg KH
@ 2021-04-09 3:09 ` Luiz Sampaio
2021-04-09 3:09 ` [PATCH v5 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
` (6 more replies)
10 siblings, 7 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:09 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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
^ permalink raw reply [flat|nested] 82+ messages in thread* [PATCH v5 1/6] w1: ds2438: fixed a coding style issue
2021-04-09 3:09 ` [PATCH v5 " Luiz Sampaio
@ 2021-04-09 3:09 ` Luiz Sampaio
2021-04-09 3:09 ` [PATCH v5 2/6] w1: ds2438: fixed if brackets " Luiz Sampaio
` (5 subsequent siblings)
6 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:09 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v5 2/6] w1: ds2438: fixed if brackets coding style issue
2021-04-09 3:09 ` [PATCH v5 " Luiz Sampaio
2021-04-09 3:09 ` [PATCH v5 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
@ 2021-04-09 3:09 ` Luiz Sampaio
2021-04-09 4:39 ` Joe Perches
2021-04-09 14:40 ` Joe Perches
2021-04-09 3:09 ` [PATCH v5 3/6] w1: ds2438: fixed a " Luiz Sampaio
` (4 subsequent siblings)
6 siblings, 2 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:09 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, Luiz Sampaio
Since there is only one statement inside the if clause, no brackets are
required.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
drivers/w1/slaves/w1_ds2438.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 148921fb9702..56e53a748059 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_current(sl, &voltage) == 0) {
+ if (w1_ds2438_get_current(sl, &voltage) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
@@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_temperature(sl, &temp) == 0) {
+ if (w1_ds2438_get_temperature(sl, &temp) == 0)
ret = snprintf(buf, count, "%i\n", temp);
- } else
+ else
ret = -EIO;
return ret;
@@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
@@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* Re: [PATCH v5 2/6] w1: ds2438: fixed if brackets coding style issue
2021-04-09 3:09 ` [PATCH v5 2/6] w1: ds2438: fixed if brackets " Luiz Sampaio
@ 2021-04-09 4:39 ` Joe Perches
2021-04-09 14:40 ` Joe Perches
1 sibling, 0 replies; 82+ messages in thread
From: Joe Perches @ 2021-04-09 4:39 UTC (permalink / raw)
To: Luiz Sampaio, zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc
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.
>
> Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
> ---
> drivers/w1/slaves/w1_ds2438.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
> index 148921fb9702..56e53a748059 100644
> --- a/drivers/w1/slaves/w1_ds2438.c
> +++ b/drivers/w1/slaves/w1_ds2438.c
> @@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj,
> if (!buf)
> return -EINVAL;
>
>
> - if (w1_ds2438_get_current(sl, &voltage) == 0) {
> + if (w1_ds2438_get_current(sl, &voltage) == 0)
> ret = snprintf(buf, count, "%i\n", voltage);
> - } else
> + else
> ret = -EIO;
>
>
> return ret;
> @@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
> if (!buf)
> return -EINVAL;
>
>
> - if (w1_ds2438_get_temperature(sl, &temp) == 0) {
> + if (w1_ds2438_get_temperature(sl, &temp) == 0)
> ret = snprintf(buf, count, "%i\n", temp);
> - } else
> + else
> ret = -EIO;
>
>
> return ret;
> @@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj,
> if (!buf)
> return -EINVAL;
>
>
> - if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0) {
> + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0)
> ret = snprintf(buf, count, "%u\n", voltage);
> - } else
> + else
> ret = -EIO;
>
>
> return ret;
> @@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
> if (!buf)
> return -EINVAL;
>
>
> - if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0) {
> + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0)
> ret = snprintf(buf, count, "%u\n", voltage);
> - } else
> + else
> ret = -EIO;
>
>
> return ret;
^ permalink raw reply [flat|nested] 82+ messages in thread* Re: [PATCH v5 2/6] w1: ds2438: fixed if brackets coding style issue
2021-04-09 3:09 ` [PATCH v5 2/6] w1: ds2438: fixed if brackets " Luiz Sampaio
2021-04-09 4:39 ` Joe Perches
@ 2021-04-09 14:40 ` Joe Perches
2021-04-16 22:26 ` Luiz Sampaio
1 sibling, 1 reply; 82+ messages in thread
From: Joe Perches @ 2021-04-09 14:40 UTC (permalink / raw)
To: Luiz Sampaio, zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc
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, &voltage) == 0) {
> + if (w1_ds2438_get_current(sl, &voltage) == 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, &voltage) == 0) {
- ret = snprintf(buf, count, "%i\n", voltage);
- } else
- ret = -EIO;
+ if (w1_ds2438_get_current(sl, &voltage))
+ 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, &temp) == 0) {
- ret = snprintf(buf, count, "%i\n", temp);
- } else
- ret = -EIO;
+ if (w1_ds2438_get_temperature(sl, &temp))
+ 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, &voltage) == 0) {
- ret = snprintf(buf, count, "%u\n", voltage);
- } else
- ret = -EIO;
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage))
+ 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, &voltage) == 0) {
- ret = snprintf(buf, count, "%u\n", voltage);
- } else
- ret = -EIO;
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage))
+ 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);
^ permalink raw reply related [flat|nested] 82+ messages in thread* Re: [PATCH v5 2/6] w1: ds2438: fixed if brackets coding style issue
2021-04-09 14:40 ` Joe Perches
@ 2021-04-16 22:26 ` Luiz Sampaio
0 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-16 22:26 UTC (permalink / raw)
To: Joe Perches; +Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc
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, &voltage) == 0) {
> > + if (w1_ds2438_get_current(sl, &voltage) == 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, &voltage) == 0) {
> - ret = snprintf(buf, count, "%i\n", voltage);
> - } else
> - ret = -EIO;
> + if (w1_ds2438_get_current(sl, &voltage))
> + 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, &temp) == 0) {
> - ret = snprintf(buf, count, "%i\n", temp);
> - } else
> - ret = -EIO;
> + if (w1_ds2438_get_temperature(sl, &temp))
> + 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, &voltage) == 0) {
> - ret = snprintf(buf, count, "%u\n", voltage);
> - } else
> - ret = -EIO;
> + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage))
> + 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, &voltage) == 0) {
> - ret = snprintf(buf, count, "%u\n", voltage);
> - } else
> - ret = -EIO;
> + if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage))
> + 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.
^ permalink raw reply [flat|nested] 82+ messages in thread
* [PATCH v5 3/6] w1: ds2438: fixed a coding style issue
2021-04-09 3:09 ` [PATCH v5 " Luiz Sampaio
2021-04-09 3:09 ` [PATCH v5 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
2021-04-09 3:09 ` [PATCH v5 2/6] w1: ds2438: fixed if brackets " Luiz Sampaio
@ 2021-04-09 3:09 ` Luiz Sampaio
2021-04-09 3:09 ` [PATCH v5 4/6] w1: ds2438: fixing bug that would always get page0 Luiz Sampaio
` (3 subsequent siblings)
6 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:09 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, Luiz Sampaio
Changed the permissions to preferred octal style.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v5 4/6] w1: ds2438: fixing bug that would always get page0
2021-04-09 3:09 ` [PATCH v5 " Luiz Sampaio
` (2 preceding siblings ...)
2021-04-09 3:09 ` [PATCH v5 3/6] w1: ds2438: fixed a " Luiz Sampaio
@ 2021-04-09 3:09 ` Luiz Sampaio
2021-04-09 3:09 ` [PATCH v5 5/6] w1: ds2438: adding support for reading page1 Luiz Sampaio
` (2 subsequent siblings)
6 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:09 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v5 5/6] w1: ds2438: adding support for reading page1
2021-04-09 3:09 ` [PATCH v5 " Luiz Sampaio
` (3 preceding siblings ...)
2021-04-09 3:09 ` [PATCH v5 4/6] w1: ds2438: fixing bug that would always get page0 Luiz Sampaio
@ 2021-04-09 3:09 ` Luiz Sampaio
2021-04-09 3:09 ` [PATCH v5 6/6] w1: ds2438: support for writing to offset register Luiz Sampaio
2021-04-09 3:15 ` [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
6 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:09 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
.../ABI/stable/sysfs-driver-w1_ds2438 | 6 +++
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 Documentation/ABI/stable/sysfs-driver-w1_ds2438
diff --git a/Documentation/ABI/stable/sysfs-driver-w1_ds2438 b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
new file mode 100644
index 000000000000..fa47437c11d9
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
@@ -0,0 +1,6 @@
+What: /sys/bus/w1/devices/.../page1
+Date: April 2021
+Contact: Luiz Sampaio <sampaio.ime@gmail.com>
+Description: read the contents of the page1 of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.
+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ef6217ecb1cb..2cfdfedb584f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07
+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}
+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v5 6/6] w1: ds2438: support for writing to offset register
2021-04-09 3:09 ` [PATCH v5 " Luiz Sampaio
` (4 preceding siblings ...)
2021-04-09 3:09 ` [PATCH v5 5/6] w1: ds2438: adding support for reading page1 Luiz Sampaio
@ 2021-04-09 3:09 ` Luiz Sampaio
2021-04-09 3:15 ` [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
6 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:09 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
.../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 <sampaio.ime@gmail.com>
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 <sampaio.ime@gmail.com>
+Description: write the contents to the offset register of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index ac8d0d4b0d0e..5c5573991351 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -22,7 +22,7 @@ is also often used in weather stations and applications such as: rain gauge,
wind speed/direction measuring, humidity sensing, etc.
Current support is provided through the following sysfs files (all files
-except "iad" are readonly):
+except "iad" and "offset" are readonly):
"iad"
-----
@@ -52,6 +52,15 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.
+"offset"
+-------
+This file controls the 2-byte Offset Register of the chip.
+Writing a 2-byte value will change the Offset Register, which changes the
+current measurement done by the chip. Changing this register to the two's complement
+of the current register while forcing zero current through the load will calibrate
+the chip, canceling offset errors in the current ADC.
+
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 2cfdfedb584f..223a9aae6e2d 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}
+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last register reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}
+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements
2021-04-09 3:09 ` [PATCH v5 " Luiz Sampaio
` (5 preceding siblings ...)
2021-04-09 3:09 ` [PATCH v5 6/6] w1: ds2438: support for writing to offset register Luiz Sampaio
@ 2021-04-09 3:15 ` Luiz Sampaio
2021-04-09 3:15 ` [PATCH v6 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
` (7 more replies)
6 siblings, 8 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:15 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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
^ permalink raw reply [flat|nested] 82+ messages in thread* [PATCH v6 1/6] w1: ds2438: fixed a coding style issue
2021-04-09 3:15 ` [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
@ 2021-04-09 3:15 ` Luiz Sampaio
2021-04-09 3:15 ` [PATCH v6 2/6] w1: ds2438: fixed if brackets " Luiz Sampaio
` (6 subsequent siblings)
7 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:15 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v6 2/6] w1: ds2438: fixed if brackets coding style issue
2021-04-09 3:15 ` [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
2021-04-09 3:15 ` [PATCH v6 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
@ 2021-04-09 3:15 ` Luiz Sampaio
2021-04-09 3:15 ` [PATCH v6 3/6] w1: ds2438: fixed a " Luiz Sampaio
` (5 subsequent siblings)
7 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:15 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, Luiz Sampaio
Since there is only one statement inside the if clause, no brackets are
required.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
drivers/w1/slaves/w1_ds2438.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 148921fb9702..56e53a748059 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_current(sl, &voltage) == 0) {
+ if (w1_ds2438_get_current(sl, &voltage) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
@@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_temperature(sl, &temp) == 0) {
+ if (w1_ds2438_get_temperature(sl, &temp) == 0)
ret = snprintf(buf, count, "%i\n", temp);
- } else
+ else
ret = -EIO;
return ret;
@@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
@@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v6 3/6] w1: ds2438: fixed a coding style issue
2021-04-09 3:15 ` [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
2021-04-09 3:15 ` [PATCH v6 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
2021-04-09 3:15 ` [PATCH v6 2/6] w1: ds2438: fixed if brackets " Luiz Sampaio
@ 2021-04-09 3:15 ` Luiz Sampaio
2021-04-09 9:44 ` kernel test robot
` (2 more replies)
2021-04-09 3:15 ` [PATCH v6 4/6] w1: ds2438: fixing bug that would always get page0 Luiz Sampaio
` (4 subsequent siblings)
7 siblings, 3 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:15 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, Luiz Sampaio
Changed the permissions to preferred octal style.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* Re: [PATCH v6 3/6] w1: ds2438: fixed a coding style issue
2021-04-09 3:15 ` [PATCH v6 3/6] w1: ds2438: fixed a " Luiz Sampaio
@ 2021-04-09 9:44 ` kernel test robot
2021-04-09 10:43 ` kernel test robot
2021-04-10 8:38 ` Greg KH
2 siblings, 0 replies; 82+ messages in thread
From: kernel test robot @ 2021-04-09 9:44 UTC (permalink / raw)
To: Luiz Sampaio, zbr
Cc: kbuild-all, clang-built-linux, corbet, rikard.falkeborn, gregkh,
linux-kernel, linux-doc, Luiz Sampaio
[-- Attachment #1: Type: text/plain, Size: 3065 bytes --]
Hi Luiz,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc6 next-20210408]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 17e7124aad766b3f158943acb51467f86220afe9
config: x86_64-randconfig-a004-20210409 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project dd453a1389b6a7e6d9214b449d3c54981b1a89b6)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
git checkout 3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/w1/slaves/w1_ds2438.c:391:31: error: too many arguments provided to function-like macro invocation
static BIN_ATTR_RW(iad, 0664, iad_write, 0);
^
include/linux/sysfs.h:229:9: note: macro 'BIN_ATTR_RW' defined here
#define BIN_ATTR_RW(_name, _size) \
^
drivers/w1/slaves/w1_ds2438.c:398:3: error: use of undeclared identifier 'bin_attr_iad'; did you mean 'bin_attr_vad'?
&bin_attr_iad,
^~~~~~~~~~~~
bin_attr_vad
drivers/w1/slaves/w1_ds2438.c:394:8: note: 'bin_attr_vad' declared here
static BIN_ATTR_RO(vad, 0/* real length varies */);
^
include/linux/sysfs.h:224:22: note: expanded from macro 'BIN_ATTR_RO'
struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
^
<scratch space>:113:1: note: expanded from here
bin_attr_vad
^
2 errors generated.
vim +391 drivers/w1/slaves/w1_ds2438.c
390
> 391 static BIN_ATTR_RW(iad, 0664, iad_write, 0);
392 static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
393 static BIN_ATTR_RO(temperature, 0/* real length varies */);
394 static BIN_ATTR_RO(vad, 0/* real length varies */);
395 static BIN_ATTR_RO(vdd, 0/* real length varies */);
396
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34309 bytes --]
^ permalink raw reply [flat|nested] 82+ messages in thread* Re: [PATCH v6 3/6] w1: ds2438: fixed a coding style issue
@ 2021-04-09 9:44 ` kernel test robot
0 siblings, 0 replies; 82+ messages in thread
From: kernel test robot @ 2021-04-09 9:44 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3133 bytes --]
Hi Luiz,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc6 next-20210408]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 17e7124aad766b3f158943acb51467f86220afe9
config: x86_64-randconfig-a004-20210409 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project dd453a1389b6a7e6d9214b449d3c54981b1a89b6)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
git checkout 3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/w1/slaves/w1_ds2438.c:391:31: error: too many arguments provided to function-like macro invocation
static BIN_ATTR_RW(iad, 0664, iad_write, 0);
^
include/linux/sysfs.h:229:9: note: macro 'BIN_ATTR_RW' defined here
#define BIN_ATTR_RW(_name, _size) \
^
drivers/w1/slaves/w1_ds2438.c:398:3: error: use of undeclared identifier 'bin_attr_iad'; did you mean 'bin_attr_vad'?
&bin_attr_iad,
^~~~~~~~~~~~
bin_attr_vad
drivers/w1/slaves/w1_ds2438.c:394:8: note: 'bin_attr_vad' declared here
static BIN_ATTR_RO(vad, 0/* real length varies */);
^
include/linux/sysfs.h:224:22: note: expanded from macro 'BIN_ATTR_RO'
struct bin_attribute bin_attr_##_name = __BIN_ATTR_RO(_name, _size)
^
<scratch space>:113:1: note: expanded from here
bin_attr_vad
^
2 errors generated.
vim +391 drivers/w1/slaves/w1_ds2438.c
390
> 391 static BIN_ATTR_RW(iad, 0664, iad_write, 0);
392 static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
393 static BIN_ATTR_RO(temperature, 0/* real length varies */);
394 static BIN_ATTR_RO(vad, 0/* real length varies */);
395 static BIN_ATTR_RO(vdd, 0/* real length varies */);
396
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 34309 bytes --]
^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [PATCH v6 3/6] w1: ds2438: fixed a coding style issue
2021-04-09 3:15 ` [PATCH v6 3/6] w1: ds2438: fixed a " Luiz Sampaio
@ 2021-04-09 10:43 ` kernel test robot
2021-04-09 10:43 ` kernel test robot
2021-04-10 8:38 ` Greg KH
2 siblings, 0 replies; 82+ messages in thread
From: kernel test robot @ 2021-04-09 10:43 UTC (permalink / raw)
To: Luiz Sampaio, zbr
Cc: kbuild-all, corbet, rikard.falkeborn, gregkh, linux-kernel,
linux-doc, Luiz Sampaio
[-- Attachment #1: Type: text/plain, Size: 3713 bytes --]
Hi Luiz,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc6 next-20210408]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 17e7124aad766b3f158943acb51467f86220afe9
config: arm-randconfig-r032-20210409 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
git checkout 3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/w1/slaves/w1_ds2438.c:391:43: error: macro "BIN_ATTR_RW" passed 4 arguments, but takes just 2
391 | static BIN_ATTR_RW(iad, 0664, iad_write, 0);
| ^
In file included from include/linux/kobject.h:20,
from include/linux/module.h:20,
from drivers/w1/slaves/w1_ds2438.c:9:
include/linux/sysfs.h:229: note: macro "BIN_ATTR_RW" defined here
229 | #define BIN_ATTR_RW(_name, _size) \
|
>> drivers/w1/slaves/w1_ds2438.c:391:8: error: type defaults to 'int' in declaration of 'BIN_ATTR_RW' [-Werror=implicit-int]
391 | static BIN_ATTR_RW(iad, 0664, iad_write, 0);
| ^~~~~~~~~~~
drivers/w1/slaves/w1_ds2438.c:398:3: error: 'bin_attr_iad' undeclared here (not in a function); did you mean 'bin_attr_vad'?
398 | &bin_attr_iad,
| ^~~~~~~~~~~~
| bin_attr_vad
drivers/w1/slaves/w1_ds2438.c:391:8: warning: 'BIN_ATTR_RW' defined but not used [-Wunused-variable]
391 | static BIN_ATTR_RW(iad, 0664, iad_write, 0);
| ^~~~~~~~~~~
drivers/w1/slaves/w1_ds2438.c:277:16: warning: 'iad_read' defined but not used [-Wunused-function]
277 | static ssize_t iad_read(struct file *filp, struct kobject *kobj,
| ^~~~~~~~
drivers/w1/slaves/w1_ds2438.c:255:16: warning: 'iad_write' defined but not used [-Wunused-function]
255 | static ssize_t iad_write(struct file *filp, struct kobject *kobj,
| ^~~~~~~~~
cc1: some warnings being treated as errors
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for ADI_AXI_ADC
Depends on IIO && HAS_IOMEM && OF
Selected by
- AD9467 && IIO && SPI
vim +/BIN_ATTR_RW +391 drivers/w1/slaves/w1_ds2438.c
390
> 391 static BIN_ATTR_RW(iad, 0664, iad_write, 0);
392 static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
393 static BIN_ATTR_RO(temperature, 0/* real length varies */);
394 static BIN_ATTR_RO(vad, 0/* real length varies */);
395 static BIN_ATTR_RO(vdd, 0/* real length varies */);
396
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24566 bytes --]
^ permalink raw reply [flat|nested] 82+ messages in thread* Re: [PATCH v6 3/6] w1: ds2438: fixed a coding style issue
@ 2021-04-09 10:43 ` kernel test robot
0 siblings, 0 replies; 82+ messages in thread
From: kernel test robot @ 2021-04-09 10:43 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3791 bytes --]
Hi Luiz,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc6 next-20210408]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 17e7124aad766b3f158943acb51467f86220afe9
config: arm-randconfig-r032-20210409 (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210409-121608
git checkout 3ca70e59a342a9c6fd7db0bc937bbd0c80da9711
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/w1/slaves/w1_ds2438.c:391:43: error: macro "BIN_ATTR_RW" passed 4 arguments, but takes just 2
391 | static BIN_ATTR_RW(iad, 0664, iad_write, 0);
| ^
In file included from include/linux/kobject.h:20,
from include/linux/module.h:20,
from drivers/w1/slaves/w1_ds2438.c:9:
include/linux/sysfs.h:229: note: macro "BIN_ATTR_RW" defined here
229 | #define BIN_ATTR_RW(_name, _size) \
|
>> drivers/w1/slaves/w1_ds2438.c:391:8: error: type defaults to 'int' in declaration of 'BIN_ATTR_RW' [-Werror=implicit-int]
391 | static BIN_ATTR_RW(iad, 0664, iad_write, 0);
| ^~~~~~~~~~~
drivers/w1/slaves/w1_ds2438.c:398:3: error: 'bin_attr_iad' undeclared here (not in a function); did you mean 'bin_attr_vad'?
398 | &bin_attr_iad,
| ^~~~~~~~~~~~
| bin_attr_vad
drivers/w1/slaves/w1_ds2438.c:391:8: warning: 'BIN_ATTR_RW' defined but not used [-Wunused-variable]
391 | static BIN_ATTR_RW(iad, 0664, iad_write, 0);
| ^~~~~~~~~~~
drivers/w1/slaves/w1_ds2438.c:277:16: warning: 'iad_read' defined but not used [-Wunused-function]
277 | static ssize_t iad_read(struct file *filp, struct kobject *kobj,
| ^~~~~~~~
drivers/w1/slaves/w1_ds2438.c:255:16: warning: 'iad_write' defined but not used [-Wunused-function]
255 | static ssize_t iad_write(struct file *filp, struct kobject *kobj,
| ^~~~~~~~~
cc1: some warnings being treated as errors
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for ADI_AXI_ADC
Depends on IIO && HAS_IOMEM && OF
Selected by
- AD9467 && IIO && SPI
vim +/BIN_ATTR_RW +391 drivers/w1/slaves/w1_ds2438.c
390
> 391 static BIN_ATTR_RW(iad, 0664, iad_write, 0);
392 static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
393 static BIN_ATTR_RO(temperature, 0/* real length varies */);
394 static BIN_ATTR_RO(vad, 0/* real length varies */);
395 static BIN_ATTR_RO(vdd, 0/* real length varies */);
396
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 24566 bytes --]
^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [PATCH v6 3/6] w1: ds2438: fixed a coding style issue
2021-04-09 3:15 ` [PATCH v6 3/6] w1: ds2438: fixed a " Luiz Sampaio
2021-04-09 9:44 ` kernel test robot
2021-04-09 10:43 ` kernel test robot
@ 2021-04-10 8:38 ` Greg KH
2 siblings, 0 replies; 82+ messages in thread
From: Greg KH @ 2021-04-10 8:38 UTC (permalink / raw)
To: Luiz Sampaio; +Cc: zbr, corbet, rikard.falkeborn, linux-kernel, linux-doc
On Fri, Apr 09, 2021 at 12:15:30AM -0300, Luiz Sampaio wrote:
> Changed the permissions to preferred octal style.
>
> Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
> ---
> drivers/w1/slaves/w1_ds2438.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
> index 56e53a748059..ccb06b8c2d78 100644
> --- a/drivers/w1/slaves/w1_ds2438.c
> +++ b/drivers/w1/slaves/w1_ds2438.c
> @@ -388,7 +388,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
> return ret;
> }
>
> -static BIN_ATTR(iad, S_IRUGO | S_IWUSR | S_IWGRP, iad_read, iad_write, 0);
> +static BIN_ATTR_RW(iad, 0664, iad_write, 0);
You obviously did not build this commit :(
And you did more than just fix a "coding style issue".
thanks,
greg k-h
^ permalink raw reply [flat|nested] 82+ messages in thread
* [PATCH v6 4/6] w1: ds2438: fixing bug that would always get page0
2021-04-09 3:15 ` [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (2 preceding siblings ...)
2021-04-09 3:15 ` [PATCH v6 3/6] w1: ds2438: fixed a " Luiz Sampaio
@ 2021-04-09 3:15 ` Luiz Sampaio
2021-04-09 3:15 ` [PATCH v6 5/6] w1: ds2438: adding support for reading page1 Luiz Sampaio
` (3 subsequent siblings)
7 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:15 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v6 5/6] w1: ds2438: adding support for reading page1
2021-04-09 3:15 ` [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (3 preceding siblings ...)
2021-04-09 3:15 ` [PATCH v6 4/6] w1: ds2438: fixing bug that would always get page0 Luiz Sampaio
@ 2021-04-09 3:15 ` Luiz Sampaio
2021-04-09 3:15 ` [PATCH v6 6/6] w1: ds2438: support for writing to offset register Luiz Sampaio
` (2 subsequent siblings)
7 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:15 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
.../ABI/stable/sysfs-driver-w1_ds2438 | 6 +++
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 Documentation/ABI/stable/sysfs-driver-w1_ds2438
diff --git a/Documentation/ABI/stable/sysfs-driver-w1_ds2438 b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
new file mode 100644
index 000000000000..fa47437c11d9
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
@@ -0,0 +1,6 @@
+What: /sys/bus/w1/devices/.../page1
+Date: April 2021
+Contact: Luiz Sampaio <sampaio.ime@gmail.com>
+Description: read the contents of the page1 of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.
+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index ef6217ecb1cb..2cfdfedb584f 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07
+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}
+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR_RW(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v6 6/6] w1: ds2438: support for writing to offset register
2021-04-09 3:15 ` [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (4 preceding siblings ...)
2021-04-09 3:15 ` [PATCH v6 5/6] w1: ds2438: adding support for reading page1 Luiz Sampaio
@ 2021-04-09 3:15 ` Luiz Sampaio
2021-04-16 22:17 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 " Luiz Sampaio
7 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-09 3:15 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
.../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 <sampaio.ime@gmail.com>
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 <sampaio.ime@gmail.com>
+Description: write the contents to the offset register of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index ac8d0d4b0d0e..5c5573991351 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -22,7 +22,7 @@ is also often used in weather stations and applications such as: rain gauge,
wind speed/direction measuring, humidity sensing, etc.
Current support is provided through the following sysfs files (all files
-except "iad" are readonly):
+except "iad" and "offset" are readonly):
"iad"
-----
@@ -52,6 +52,15 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.
+"offset"
+-------
+This file controls the 2-byte Offset Register of the chip.
+Writing a 2-byte value will change the Offset Register, which changes the
+current measurement done by the chip. Changing this register to the two's complement
+of the current register while forcing zero current through the load will calibrate
+the chip, canceling offset errors in the current ADC.
+
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 2cfdfedb584f..223a9aae6e2d 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}
+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last register reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}
+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR_RW(iad, 0664, iad_write, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements
2021-04-09 3:15 ` [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (5 preceding siblings ...)
2021-04-09 3:15 ` [PATCH v6 6/6] w1: ds2438: support for writing to offset register Luiz Sampaio
@ 2021-04-16 22:17 ` Luiz Sampaio
2021-04-16 22:17 ` [PATCH v7 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
` (6 more replies)
2021-05-19 22:30 ` [PATCH v8 " Luiz Sampaio
7 siblings, 7 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-16 22:17 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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
^ permalink raw reply [flat|nested] 82+ messages in thread* [PATCH v7 1/6] w1: ds2438: fixed a coding style issue
2021-04-16 22:17 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
@ 2021-04-16 22:17 ` Luiz Sampaio
2021-04-16 22:17 ` [PATCH v7 2/6] w1: ds2438: fixed if brackets " Luiz Sampaio
` (5 subsequent siblings)
6 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-16 22:17 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v7 2/6] w1: ds2438: fixed if brackets coding style issue
2021-04-16 22:17 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
2021-04-16 22:17 ` [PATCH v7 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
@ 2021-04-16 22:17 ` Luiz Sampaio
2021-04-16 22:17 ` [PATCH v7 3/6] w1: ds2438: changed sysfs macro for rw file Luiz Sampaio
` (4 subsequent siblings)
6 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-16 22:17 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, Luiz Sampaio
Since there is only one statement inside the if clause, no brackets are
required.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
drivers/w1/slaves/w1_ds2438.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 148921fb9702..56e53a748059 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_current(sl, &voltage) == 0) {
+ if (w1_ds2438_get_current(sl, &voltage) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
@@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_temperature(sl, &temp) == 0) {
+ if (w1_ds2438_get_temperature(sl, &temp) == 0)
ret = snprintf(buf, count, "%i\n", temp);
- } else
+ else
ret = -EIO;
return ret;
@@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
@@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v7 3/6] w1: ds2438: changed sysfs macro for rw file
2021-04-16 22:17 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
2021-04-16 22:17 ` [PATCH v7 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
2021-04-16 22:17 ` [PATCH v7 2/6] w1: ds2438: fixed if brackets " Luiz Sampaio
@ 2021-04-16 22:17 ` Luiz Sampaio
2021-04-16 22:17 ` [PATCH v7 4/6] w1: ds2438: fixing bug that would always get page0 Luiz Sampaio
` (3 subsequent siblings)
6 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-16 22:17 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v7 4/6] w1: ds2438: fixing bug that would always get page0
2021-04-16 22:17 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (2 preceding siblings ...)
2021-04-16 22:17 ` [PATCH v7 3/6] w1: ds2438: changed sysfs macro for rw file Luiz Sampaio
@ 2021-04-16 22:17 ` Luiz Sampaio
2021-04-16 22:17 ` [PATCH v7 5/6] w1: ds2438: adding support for reading page1 Luiz Sampaio
` (2 subsequent siblings)
6 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-16 22:17 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v7 5/6] w1: ds2438: adding support for reading page1
2021-04-16 22:17 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (3 preceding siblings ...)
2021-04-16 22:17 ` [PATCH v7 4/6] w1: ds2438: fixing bug that would always get page0 Luiz Sampaio
@ 2021-04-16 22:17 ` Luiz Sampaio
2021-04-16 22:17 ` [PATCH v7 6/6] w1: ds2438: support for writing to offset register Luiz Sampaio
2021-05-14 11:47 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Greg KH
6 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-16 22:17 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
.../ABI/stable/sysfs-driver-w1_ds2438 | 6 +++
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 Documentation/ABI/stable/sysfs-driver-w1_ds2438
diff --git a/Documentation/ABI/stable/sysfs-driver-w1_ds2438 b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
new file mode 100644
index 000000000000..fa47437c11d9
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
@@ -0,0 +1,6 @@
+What: /sys/bus/w1/devices/.../page1
+Date: April 2021
+Contact: Luiz Sampaio <sampaio.ime@gmail.com>
+Description: read the contents of the page1 of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.
+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 1e95f3a256c7..42080ae779f0 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07
+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}
+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR_RW(iad, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v7 6/6] w1: ds2438: support for writing to offset register
2021-04-16 22:17 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (4 preceding siblings ...)
2021-04-16 22:17 ` [PATCH v7 5/6] w1: ds2438: adding support for reading page1 Luiz Sampaio
@ 2021-04-16 22:17 ` Luiz Sampaio
2021-04-17 5:25 ` kernel test robot
2021-05-14 11:47 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Greg KH
6 siblings, 1 reply; 82+ messages in thread
From: Luiz Sampaio @ 2021-04-16 22:17 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
.../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 <sampaio.ime@gmail.com>
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 <sampaio.ime@gmail.com>
+Description: write the contents to the offset register of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index ac8d0d4b0d0e..5c5573991351 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -22,7 +22,7 @@ is also often used in weather stations and applications such as: rain gauge,
wind speed/direction measuring, humidity sensing, etc.
Current support is provided through the following sysfs files (all files
-except "iad" are readonly):
+except "iad" and "offset" are readonly):
"iad"
-----
@@ -52,6 +52,15 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.
+"offset"
+-------
+This file controls the 2-byte Offset Register of the chip.
+Writing a 2-byte value will change the Offset Register, which changes the
+current measurement done by the chip. Changing this register to the two's complement
+of the current register while forcing zero current through the load will calibrate
+the chip, canceling offset errors in the current ADC.
+
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 42080ae779f0..9c39bd6f5fcc 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}
+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last register reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}
+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR_RW(iad, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* Re: [PATCH v7 6/6] w1: ds2438: support for writing to offset register
2021-04-16 22:17 ` [PATCH v7 6/6] w1: ds2438: support for writing to offset register Luiz Sampaio
@ 2021-04-17 5:25 ` kernel test robot
0 siblings, 0 replies; 82+ messages in thread
From: kernel test robot @ 2021-04-17 5:25 UTC (permalink / raw)
To: Luiz Sampaio, zbr
Cc: kbuild-all, corbet, rikard.falkeborn, gregkh, linux-kernel,
linux-doc, Luiz Sampaio
[-- Attachment #1: Type: text/plain, Size: 2054 bytes --]
Hi Luiz,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.12-rc7 next-20210416]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210417-071754
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 151501160401e2dc669ea7dac2c599b53f220c33
config: csky-randconfig-m031-20210416 (attached as .config)
compiler: csky-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
smatch warnings:
drivers/w1/slaves/w1_ds2438.c:218 w1_ds2438_change_offset_register() warn: inconsistent indenting
vim +218 drivers/w1/slaves/w1_ds2438.c
195
196 static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
197 {
198 unsigned int retries = W1_DS2438_RETRIES;
199 u8 w1_buf[9];
200 u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
201
202 if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
203 memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last register reserved */
204 w1_buf[7] = value[0]; /* change only offset register */
205 w1_buf[8] = value[1];
206 while (retries--) {
207 if (w1_reset_select_slave(sl))
208 continue;
209 w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
210 w1_buf[1] = 0x01; /* write to page 1 */
211 w1_write_block(sl->master, w1_buf, 9);
212
213 if (w1_reset_select_slave(sl))
214 continue;
215 w1_buf[0] = W1_DS2438_COPY_SCRATCH;
216 w1_buf[1] = 0x01;
217 w1_write_block(sl->master, w1_buf, 2);
> 218 return 0;
219 }
220 }
221 return -1;
222 }
223
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24550 bytes --]
^ permalink raw reply [flat|nested] 82+ messages in thread* Re: [PATCH v7 6/6] w1: ds2438: support for writing to offset register
@ 2021-04-17 5:25 ` kernel test robot
0 siblings, 0 replies; 82+ messages in thread
From: kernel test robot @ 2021-04-17 5:25 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2112 bytes --]
Hi Luiz,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.12-rc7 next-20210416]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Luiz-Sampaio/w1-ds2438-fixed-a-coding-style-issue/20210417-071754
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 151501160401e2dc669ea7dac2c599b53f220c33
config: csky-randconfig-m031-20210416 (attached as .config)
compiler: csky-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
smatch warnings:
drivers/w1/slaves/w1_ds2438.c:218 w1_ds2438_change_offset_register() warn: inconsistent indenting
vim +218 drivers/w1/slaves/w1_ds2438.c
195
196 static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
197 {
198 unsigned int retries = W1_DS2438_RETRIES;
199 u8 w1_buf[9];
200 u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
201
202 if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
203 memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last register reserved */
204 w1_buf[7] = value[0]; /* change only offset register */
205 w1_buf[8] = value[1];
206 while (retries--) {
207 if (w1_reset_select_slave(sl))
208 continue;
209 w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
210 w1_buf[1] = 0x01; /* write to page 1 */
211 w1_write_block(sl->master, w1_buf, 9);
212
213 if (w1_reset_select_slave(sl))
214 continue;
215 w1_buf[0] = W1_DS2438_COPY_SCRATCH;
216 w1_buf[1] = 0x01;
217 w1_write_block(sl->master, w1_buf, 2);
> 218 return 0;
219 }
220 }
221 return -1;
222 }
223
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 24550 bytes --]
^ permalink raw reply [flat|nested] 82+ messages in thread
* Re: [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements
2021-04-16 22:17 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (5 preceding siblings ...)
2021-04-16 22:17 ` [PATCH v7 6/6] w1: ds2438: support for writing to offset register Luiz Sampaio
@ 2021-05-14 11:47 ` Greg KH
6 siblings, 0 replies; 82+ messages in thread
From: Greg KH @ 2021-05-14 11:47 UTC (permalink / raw)
To: Luiz Sampaio; +Cc: zbr, corbet, rikard.falkeborn, linux-kernel, linux-doc
On Fri, Apr 16, 2021 at 07:17:33PM -0300, Luiz Sampaio wrote:
> The following patches aim to make a user able to calibrate the current
> measurement of the DS2438. This chip uses a offset register in page1, which
> is added to the current register to give the user the current measurement.
> If this value is wrong, the user will get an offset current value, even if
> the current is zero, for instance. This patch gives support for reading the
> page1 registers (including the offset register) and for writing to the
> offset register. The DS2438 datasheet shows a calibration routine, and with
> this patch, the user can do this quickly by writing the correct value to
> the offset register. This patch was tested on real hardware using a power
> supply and an electronic load.
> Please help to review this series of patches.
>
> Best regards!
> Sampaio
> ---
> Changes in v7:
> - Build test again
Can you fix the warning in patch 6/6 that the kernel test robot found,
and resend?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 82+ messages in thread
* [PATCH v8 0/6] w1: ds2438: adding support for calibration of current measurements
2021-04-09 3:15 ` [PATCH v6 0/9] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
` (6 preceding siblings ...)
2021-04-16 22:17 ` [PATCH v7 0/6] w1: ds2438: adding support for calibration of current measurements Luiz Sampaio
@ 2021-05-19 22:30 ` Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
` (5 more replies)
7 siblings, 6 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-05-19 22:30 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 v8:
- Fixing bot identation warning
Changes in v7:
- Build test again
Changes in v6:
- Actually changing from BIN_ATTR to BIN_ATTR_RW
---
Changes in v5:
- Merged all brackets coding style issue in one patch
- Changing from BIN_ATTR to BIN_ATTR_RW in w1_ds2438.c
- Wrapping email lines
- Addind Documentation/ABI/
Changes in v4:
- Fixing different patches with identical subject lines as requested
Changes in v3:
- I accidentally added a wrong line that would not compile. I'm sorry. Fixed it.
Changes in v2:
- Using git send-email to send the patches
- Adding documentation as requested
- Separating the coding style changes in different patches as requested
Luiz Sampaio (6):
w1: ds2438: fixed a coding style issue
w1: ds2438: fixed if brackets coding style issue
w1: ds2438: changed sysfs macro for rw file
w1: ds2438: fixing bug that would always get page0
w1: ds2438: adding support for reading page1
w1: ds2438: support for writing to offset register
.../ABI/stable/sysfs-driver-w1_ds2438 | 13 ++
Documentation/w1/slaves/w1_ds2438.rst | 19 ++-
drivers/w1/slaves/w1_ds2438.c | 122 +++++++++++++++---
3 files changed, 137 insertions(+), 17 deletions(-)
create mode 100644 Documentation/ABI/stable/sysfs-driver-w1_ds2438
--
2.30.1
^ permalink raw reply [flat|nested] 82+ messages in thread* [PATCH v8 1/6] w1: ds2438: fixed a coding style issue
2021-05-19 22:30 ` [PATCH v8 " Luiz Sampaio
@ 2021-05-19 22:30 ` Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 2/6] w1: ds2438: fixed if brackets " Luiz Sampaio
` (4 subsequent siblings)
5 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-05-19 22:30 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v8 2/6] w1: ds2438: fixed if brackets coding style issue
2021-05-19 22:30 ` [PATCH v8 " Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
@ 2021-05-19 22:30 ` Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 3/6] w1: ds2438: changed sysfs macro for rw file Luiz Sampaio
` (3 subsequent siblings)
5 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-05-19 22:30 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, Luiz Sampaio
Since there is only one statement inside the if clause, no brackets are
required.
Signed-off-by: Luiz Sampaio <sampaio.ime@gmail.com>
---
drivers/w1/slaves/w1_ds2438.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 148921fb9702..56e53a748059 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -287,9 +287,9 @@ static ssize_t iad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_current(sl, &voltage) == 0) {
+ if (w1_ds2438_get_current(sl, &voltage) == 0)
ret = snprintf(buf, count, "%i\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
@@ -338,9 +338,9 @@ static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_temperature(sl, &temp) == 0) {
+ if (w1_ds2438_get_temperature(sl, &temp) == 0)
ret = snprintf(buf, count, "%i\n", temp);
- } else
+ else
ret = -EIO;
return ret;
@@ -359,9 +359,9 @@ static ssize_t vad_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VAD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
@@ -380,9 +380,9 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
if (!buf)
return -EINVAL;
- if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0) {
+ if (w1_ds2438_get_voltage(sl, DS2438_ADC_INPUT_VDD, &voltage) == 0)
ret = snprintf(buf, count, "%u\n", voltage);
- } else
+ else
ret = -EIO;
return ret;
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v8 3/6] w1: ds2438: changed sysfs macro for rw file
2021-05-19 22:30 ` [PATCH v8 " Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 1/6] w1: ds2438: fixed a coding style issue Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 2/6] w1: ds2438: fixed if brackets " Luiz Sampaio
@ 2021-05-19 22:30 ` Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 4/6] w1: ds2438: fixing bug that would always get page0 Luiz Sampaio
` (2 subsequent siblings)
5 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-05-19 22:30 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v8 4/6] w1: ds2438: fixing bug that would always get page0
2021-05-19 22:30 ` [PATCH v8 " Luiz Sampaio
` (2 preceding siblings ...)
2021-05-19 22:30 ` [PATCH v8 3/6] w1: ds2438: changed sysfs macro for rw file Luiz Sampaio
@ 2021-05-19 22:30 ` Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 5/6] w1: ds2438: adding support for reading page1 Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 6/6] w1: ds2438: support for writing to offset register Luiz Sampaio
5 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-05-19 22:30 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v8 5/6] w1: ds2438: adding support for reading page1
2021-05-19 22:30 ` [PATCH v8 " Luiz Sampaio
` (3 preceding siblings ...)
2021-05-19 22:30 ` [PATCH v8 4/6] w1: ds2438: fixing bug that would always get page0 Luiz Sampaio
@ 2021-05-19 22:30 ` Luiz Sampaio
2021-05-19 22:30 ` [PATCH v8 6/6] w1: ds2438: support for writing to offset register Luiz Sampaio
5 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-05-19 22:30 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
.../ABI/stable/sysfs-driver-w1_ds2438 | 6 +++
Documentation/w1/slaves/w1_ds2438.rst | 8 ++++
drivers/w1/slaves/w1_ds2438.c | 41 +++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 Documentation/ABI/stable/sysfs-driver-w1_ds2438
diff --git a/Documentation/ABI/stable/sysfs-driver-w1_ds2438 b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
new file mode 100644
index 000000000000..fa47437c11d9
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-w1_ds2438
@@ -0,0 +1,6 @@
+What: /sys/bus/w1/devices/.../page1
+Date: April 2021
+Contact: Luiz Sampaio <sampaio.ime@gmail.com>
+Description: read the contents of the page1 of the DS2438
+ see Documentation/w1/slaves/w1_ds2438.rst for detailed information
+Users: any user space application which wants to communicate with DS2438
diff --git a/Documentation/w1/slaves/w1_ds2438.rst b/Documentation/w1/slaves/w1_ds2438.rst
index a29309a3f8e5..ac8d0d4b0d0e 100644
--- a/Documentation/w1/slaves/w1_ds2438.rst
+++ b/Documentation/w1/slaves/w1_ds2438.rst
@@ -44,6 +44,14 @@ Internally when this file is read, the additional CRC byte is also obtained
from the slave device. If it is correct, the 8 bytes page data are passed
to userspace, otherwise an I/O error is returned.
+"page1"
+-------
+This file provides full 8 bytes of the chip Page 1 (01h).
+This page contains the ICA, elapsed time meter and current offset data of the DS2438.
+Internally when this file is read, the additional CRC byte is also obtained
+from the slave device. If it is correct, the 8 bytes page data are passed
+to userspace, otherwise an I/O error is returned.
+
"temperature"
-------------
Opening and reading this file initiates the CONVERT_T (temperature conversion)
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 1e95f3a256c7..42080ae779f0 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
#define DS2438_CURRENT_MSB 0x06
#define DS2438_THRESHOLD 0x07
+/* Page #1 definitions */
+#define DS2438_ETM_0 0x00
+#define DS2438_ETM_1 0x01
+#define DS2438_ETM_2 0x02
+#define DS2438_ETM_3 0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB 0x05
+#define DS2438_OFFSET_MSB 0x06
+
static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
{
unsigned int retries = W1_DS2438_RETRIES;
@@ -325,6 +334,36 @@ static ssize_t page0_read(struct file *filp, struct kobject *kobj,
return ret;
}
+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+ u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (off != 0)
+ return 0;
+ if (!buf)
+ return -EINVAL;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ /* Read no more than page1 size */
+ if (count > DS2438_PAGE_SIZE)
+ count = DS2438_PAGE_SIZE;
+
+ if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+ memcpy(buf, &w1_buf, count);
+ ret = count;
+ } else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -390,6 +429,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR_RW(iad, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
+static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -397,6 +437,7 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
+ &bin_attr_page1,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread* [PATCH v8 6/6] w1: ds2438: support for writing to offset register
2021-05-19 22:30 ` [PATCH v8 " Luiz Sampaio
` (4 preceding siblings ...)
2021-05-19 22:30 ` [PATCH v8 5/6] w1: ds2438: adding support for reading page1 Luiz Sampaio
@ 2021-05-19 22:30 ` Luiz Sampaio
5 siblings, 0 replies; 82+ messages in thread
From: Luiz Sampaio @ 2021-05-19 22:30 UTC (permalink / raw)
To: zbr
Cc: corbet, rikard.falkeborn, gregkh, linux-kernel, linux-doc, 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 <sampaio.ime@gmail.com>
---
.../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 <sampaio.ime@gmail.com>
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 <sampaio.ime@gmail.com>
+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..01eaa5a17fba 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -193,6 +193,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave *sl, u8 mask, u8 value)
return -1;
}
+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+ unsigned int retries = W1_DS2438_RETRIES;
+ u8 w1_buf[9];
+ u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+ if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+ memcpy(&w1_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last register reserved */
+ w1_buf[7] = value[0]; /* change only offset register */
+ w1_buf[8] = value[1];
+ while (retries--) {
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+ w1_buf[1] = 0x01; /* write to page 1 */
+ w1_write_block(sl->master, w1_buf, 9);
+
+ if (w1_reset_select_slave(sl))
+ continue;
+ w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+ w1_buf[1] = 0x01;
+ w1_write_block(sl->master, w1_buf, 2);
+ return 0;
+ }
+ }
+ return -1;
+}
+
static int w1_ds2438_get_voltage(struct w1_slave *sl,
int adc_input, uint16_t *voltage)
{
@@ -364,6 +392,25 @@ static ssize_t page1_read(struct file *filp, struct kobject *kobj,
return ret;
}
+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct w1_slave *sl = kobj_to_w1_slave(kobj);
+ int ret;
+
+ mutex_lock(&sl->master->bus_mutex);
+
+ if (w1_ds2438_change_offset_register(sl, buf) == 0)
+ ret = count;
+ else
+ ret = -EIO;
+
+ mutex_unlock(&sl->master->bus_mutex);
+
+ return ret;
+}
+
static ssize_t temperature_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *bin_attr, char *buf,
loff_t off, size_t count)
@@ -430,6 +477,7 @@ static ssize_t vdd_read(struct file *filp, struct kobject *kobj,
static BIN_ATTR_RW(iad, 0);
static BIN_ATTR_RO(page0, DS2438_PAGE_SIZE);
static BIN_ATTR_RO(page1, DS2438_PAGE_SIZE);
+static BIN_ATTR_WO(offset, 2);
static BIN_ATTR_RO(temperature, 0/* real length varies */);
static BIN_ATTR_RO(vad, 0/* real length varies */);
static BIN_ATTR_RO(vdd, 0/* real length varies */);
@@ -438,6 +486,7 @@ static struct bin_attribute *w1_ds2438_bin_attrs[] = {
&bin_attr_iad,
&bin_attr_page0,
&bin_attr_page1,
+ &bin_attr_offset,
&bin_attr_temperature,
&bin_attr_vad,
&bin_attr_vdd,
--
2.30.1
^ permalink raw reply related [flat|nested] 82+ messages in thread