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

2021-03-23 Thread Greg KH
On Mon, Mar 22, 2021 at 11:44:10PM -0300, Luiz Sampaio wrote:
> @@ -395,6 +483,8 @@ static BIN_ATTR_RO(vdd, 0/* real length varies */);
>  static struct bin_attribute *w1_ds2438_bin_attrs[] = {
>   _attr_iad,
>   _attr_page0,
> + _attr_page1,
> + _attr_offset,

You are adding new sysfs files, but no new Documentation/ABI entries :(

please fix that up for your next submission.

thanks,

greg k-h


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

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

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

diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index 08f4b451c349..5d58986bb1a9 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -49,6 +49,15 @@
 #define DS2438_CURRENT_MSB 0x06
 #define DS2438_THRESHOLD   0x07
 
+/* Page #1 definitions */
+#define DS2438_ETM_0   0x00
+#define DS2438_ETM_1   0x01
+#define DS2438_ETM_2   0x02
+#define DS2438_ETM_3   0x03
+#define DS2438_ICA 0x04
+#define DS2438_OFFSET_LSB  0x05
+#define DS2438_OFFSET_MSB  0x06
+
 static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf)
 {
unsigned int retries = W1_DS2438_RETRIES;
@@ -62,13 +71,13 @@ static int w1_ds2438_get_page(struct w1_slave *sl, int 
pageno, u8 *buf)
if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_RECALL_MEMORY;
-   w1_buf[1] = pageno;
+   w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);
 
if (w1_reset_select_slave(sl))
continue;
w1_buf[0] = W1_DS2438_READ_SCRATCH;
-   w1_buf[1] = pageno;
+   w1_buf[1] = (u8)pageno;
w1_write_block(sl->master, w1_buf, 2);
 
count = w1_read_block(sl->master, buf, DS2438_PAGE_SIZE + 1);
@@ -182,6 +191,34 @@ static int w1_ds2438_change_config_bit(struct w1_slave 
*sl, u8 mask, u8 value)
return -1;
 }
 
+static int w1_ds2438_change_offset_register(struct w1_slave *sl, u8 *value)
+{
+   unsigned int retries = W1_DS2438_RETRIES;
+   u8 w1_buf[9];
+   u8 w1_page1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+   if (w1_ds2438_get_page(sl, 1, w1_page1_buf) == 0) {
+   memcpy(_buf[2], w1_page1_buf, DS2438_PAGE_SIZE - 1); /* last 
register is reserved */
+   w1_buf[7] = value[0]; /* change only offset register */
+   w1_buf[8] = value[1];
+   while (retries--) {
+   if (w1_reset_select_slave(sl))
+   continue;
+   w1_buf[0] = W1_DS2438_WRITE_SCRATCH;
+   w1_buf[1] = 0x01; /* write to page 1 */
+   w1_write_block(sl->master, w1_buf, 9);
+
+   if (w1_reset_select_slave(sl))
+   continue;
+   w1_buf[0] = W1_DS2438_COPY_SCRATCH;
+   w1_buf[1] = 0x01;
+   w1_write_block(sl->master, w1_buf, 2);
+   return 0;
+   }
+   }
+   return -1;
+}
+
 static int w1_ds2438_get_voltage(struct w1_slave *sl,
 int adc_input, uint16_t *voltage)
 {
@@ -323,6 +360,55 @@ static ssize_t page0_read(struct file *filp, struct 
kobject *kobj,
return ret;
 }
 
+static ssize_t page1_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr, char *buf,
+ loff_t off, size_t count)
+{
+   struct w1_slave *sl = kobj_to_w1_slave(kobj);
+   int ret;
+   u8 w1_buf[DS2438_PAGE_SIZE + 1 /*for CRC*/];
+
+   if (off != 0)
+   return 0;
+   if (!buf)
+   return -EINVAL;
+
+   mutex_lock(>master->bus_mutex);
+
+   /* Read no more than page1 size */
+   if (count > DS2438_PAGE_SIZE)
+   count = DS2438_PAGE_SIZE;
+
+   if (w1_ds2438_get_page(sl, 1, w1_buf) == 0) {
+   memcpy(buf, _buf, count);
+   ret = count;
+   } else
+   ret = -EIO;
+
+   mutex_unlock(>master->bus_mutex);
+
+   return ret;
+}
+
+static ssize_t offset_write(struct file *filp, struct kobject *kobj,
+   struct bin_attribute *bin_attr, char *buf,
+   loff_t off, size_t count)
+{
+   struct w1_slave *sl = kobj_to_w1_slave(kobj);
+   int ret;
+
+   mutex_lock(>master->bus_mutex);
+
+   if (w1_ds2438_change_offset_register(sl, buf) == 0)
+   ret = count;
+   else
+   ret = -EIO;
+
+   mutex_unlock(>master->bus_mutex);
+
+   return ret;
+}
+
 static ssize_t temperature_read(struct file *filp, struct kobject *kobj,