Re: [PATCH] w1_therm: Free the correct variable

2020-05-20 Thread Akira shimahara
Hi,

Le mercredi 20 mai 2020 à 15:00 +0300, Dan Carpenter a écrit :
> The problem is that we change "p_args" to point to the middle of the
> string so when we free it at the end of the function it's not freeing
> the same pointer that we originally allocated.
> 
> Fixes: e2c94d6f5720 ("w1_therm: adding alarm sysfs entry")
> Signed-off-by: Dan Carpenter 
> ---
> From static analysis.  I guess it must not cause too much of a problem
> at run time?
> 
>  drivers/w1/slaves/w1_therm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
> index cc4b88056b33..a6c85e486671 100644
> --- a/drivers/w1/slaves/w1_therm.c
> +++ b/drivers/w1/slaves/w1_therm.c
> @@ -1526,8 +1526,9 @@ static ssize_t alarms_store(struct device *device,
>   int temp, ret = -EINVAL;
>   char *token = NULL;
>   s8 tl, th, tt;  /* 1 byte per value + temp ring order */
> - char *p_args = kmalloc(size, GFP_KERNEL);
> + char *p_args, *orig;
>  
> + p_args = orig = kmalloc(size, GFP_KERNEL);
>   /* Safe string copys as buf is const */
>   if (!p_args) {
>   dev_warn(device,
> @@ -1611,7 +1612,7 @@ static ssize_t alarms_store(struct device *device,
>  
>  free_m:
>   /* free allocated memory */
> - kfree(p_args);
> + kfree(orig);
>  
>   return size;
>  }

I tested it on several devices to be safe, no issue at runtime.
Sorry for the mistake and thanks for the fix.

Akira Shimahara



Re: [PATCH v7 1/9] w1_therm: adding code comments and code reordering

2020-05-15 Thread Akira shimahara
Le vendredi 15 mai 2020 à 16:29 +0200, Greg KH a écrit :
> On Mon, May 11, 2020 at 10:35:35PM +0200, Akira Shimahara wrote:
> > Adding code comments to split code in dedicated parts. After the global
> > declarations (defines, macros and function declarations), code is organized
> > as follow :
> >  - Device and family dependent structures and functions
> >  - Interfaces functions
> >  - Helpers functions
> >  - Hardware functions
> >  - Sysfs interface functions
> > 
> > Signed-off-by: Akira Shimahara 
> > ---
> > Main motivation on the first patch of this serie is to clean up the code,
> > document it and reorder it to prepare the next patches, which are clearer
> > after this.
> > 
> > One main point is to keep all device/family dependent code gather at the
> > beginning to ease adding new devices if required.
> 
> Thanks for sticking with this.  Nice work, all now queued up.
> 
> greg k-h

Hi, 

I also would like to thanks you all for your time and your involvment to help
these patch to be pushed up.

Akira Shimahara



[PATCH v7 9/9] w1_therm: adding bulk read support to trigger multiple conversion on bus

2020-05-11 Thread Akira Shimahara
Adding bulk read support:
Sending a 'trigger' command in the dedicated sysfs entry of bus master
device send a conversion command for all the slaves on the bus. The sysfs
entry is added as soon as at least one device supporting this feature
is detected on the bus.

The behavior of the sysfs reading temperature on the device is as follow:
 * If no bulk read pending, trigger a conversion on the device, wait for
 the conversion to be done, read the temperature in device RAM
 * If a bulk read has been trigger, access directly the device RAM
This behavior is the same on the 2 sysfs entries ('temperature' and
'w1_slave').

Reading the therm_bulk_read sysfs give the status of bulk operations:
 * '-1': conversion in progress on at least 1 sensor
 * '1': conversion complete but at least one sensor has not been read yet
 * '0': no bulk operation. Reading temperature on ecah device will trigger
a conversion

As not all devices support bulk read feature, it has been added in device
family structure.

The attribute is set at master level as soon as a supporting device is
discover. It is removed when the last supported device leave the bus.
The count of supported device is kept with the static counter
bulk_read_device_counter.

A strong pull up is apply on the line if at least one device required it.
The duration of the pull up is the max time required by a device on the
line, which depends on the resolution settings of each device. The strong
pull up could be adjust with the a module parameter.

Updating documentation in Documentation/ABI/testing/sysfs-driver-w1_therm
and Documentation/w1/slaves/w1_therm.rst accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements
Changes in v7:
- Formatting code comments and correcting comments mistakes

 .../ABI/testing/sysfs-driver-w1_therm |  36 ++-
 Documentation/w1/slaves/w1_therm.rst  |  50 +++-
 drivers/w1/slaves/w1_therm.c  | 251 +-
 3 files changed, 322 insertions(+), 15 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index f289520..076659d 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -62,9 +62,16 @@ Date:May 2020
 Contact:   Akira Shimahara 
 Description:
(RO) return the temperature in 1/1000 degC.
-   Note that the conversion duration depend on the resolution (if
-   device support this feature). It takes 94ms in 9bits
-   resolution, 750ms for 12bits.
+   * If a bulk read has been triggered, it will directly
+   return the temperature computed when the bulk read
+   occurred, if available. If not yet available, nothing
+   is returned (a debug kernel message is sent), you
+   should retry later on.
+   * If no bulk read has been triggered, it will trigger
+   a conversion and send the result. Note that the
+   conversion duration depend on the resolution (if
+   device support this feature). It takes 94ms in 9bits
+   resolution, 750ms for 12bits.
 Users: any user space application which wants to communicate with
w1_term device
 
@@ -85,4 +92,25 @@ Description:
refer to Documentation/w1/slaves/w1_therm.rst for detailed
information.
 Users: any user space application which wants to communicate with
-   w1_term device
\ No newline at end of file
+   w1_term device
+
+
+What:  /sys/bus/w1/devices/w1_bus_masterXX/therm_bulk_read
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) trigger a bulk read conversion. read the status
+   *read*:
+   * '-1': conversion in progress on at least 1 sensor
+   * '1' : conversion complete but at least one sensor
+   value has not been read yet
+   * '0' : no bulk operation. Reading temperature will
+   trigger a conversion on each device
+   *write*: 'trigger': trigger a bulk read on all supporting
+   devices on the bus
+   Note that if a bulk read is sent but one sensor is not read
+   immediately, the next access to temperature on this device
+   will return the temperature measured at the time of issue
+   of the bulk read command (not the current temperature).
+Users: any user space application which wants to communicate with
+   w1_term device
diff

[PATCH v7 7/9] w1_therm: optimizing temperature read timings

2020-05-11 Thread Akira Shimahara
Optimizing temperature reading by reducing waiting conversion time
according to device resolution settings, as per device specification.
This is device dependent as not all the devices supports resolution
setting, so it has been added in device family structures.

The process to read the temperature on the device has been adapted in a
new function 'convert_t()', which replace the former 'read_therm()', is
introduce to deal with this timing. Strong pull up is also applied during
the required time, according to device power status needs and
'strong_pullup' module parameter.

'temperature_from_RAM()' function is introduced to get the correct
temperature computation (device dependent) from device RAM data.

An new sysfs entry has been added to ouptut only temperature. The old
entry w1_slave has been kept for compatibility, without changing its
output format.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements
Changes in v7:
- Formatting code comments and correcting comments mistakes

 .../ABI/testing/sysfs-driver-w1_therm |  12 +
 drivers/w1/slaves/w1_therm.c  | 286 +++---
 2 files changed, 197 insertions(+), 101 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 8b7ee89..6ffd3e3 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -41,6 +41,18 @@ Users:   any user space application which wants 
to communicate with
w1_term device
 
 
+What:  /sys/bus/w1/devices/.../temperature
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RO) return the temperature in 1/1000 degC.
+   Note that the conversion duration depend on the resolution (if
+   device support this feature). It takes 94ms in 9bits
+   resolution, 750ms for 12bits.
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 957b503..46756ea 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -93,6 +93,7 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * @reserved: not used here
  * @f: pointer to the device binding structure
  * @convert: pointer to the device conversion function
+ * @get_conversion_time: pointer to the device conversion time function
  * @set_resolution: pointer to the device set_resolution function
  * @get_resolution: pointer to the device get_resolution function
  */
@@ -101,6 +102,7 @@ struct w1_therm_family_converter {
u16 reserved;
struct w1_family*f;
int (*convert)(u8 rom[9]);
+   int (*get_conversion_time)(struct w1_slave *sl);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
 };
@@ -153,6 +155,15 @@ struct therm_info {
  */
 static int reset_select_slave(struct w1_slave *sl);
 
+/**
+ * convert_t() - Query the device for temperature conversion and read
+ * @sl: pointer to the slave to read
+ * @info: pointer to a structure to store the read results
+ *
+ * Return: 0 if success, -kernel error code otherwise
+ */
+static int convert_t(struct w1_slave *sl, struct therm_info *info);
+
 /**
  * read_scratchpad() - read the data in device RAM
  * @sl: pointer to the slave to read
@@ -213,6 +224,9 @@ static ssize_t w1_slave_store(struct device *device,
 static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
 
+static ssize_t temperature_show(struct device *device,
+   struct device_attribute *attr, char *buf);
+
 static ssize_t ext_power_show(struct device *device,
struct device_attribute *attr, char *buf);
 
@@ -229,6 +243,7 @@ static ssize_t eeprom_store(struct device *device,
 
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
+static DEVICE_ATTR_RO(temperature);
 static DEVICE_ATTR_RO(ext_power);
 static DEVICE_ATTR_RW(resolution);
 static DEVICE_ATTR_WO(eeprom);
@@ -259,6 +274,7 @@ static void w1_therm_remove_slave(struct w1_slave *sl);
 
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+   _attr_temperature.attr,
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
@@ -267,6 +283,7 @@ static struct attribute *w1_therm_attrs[] = {
 
 static struct attribute *w1_ds18s20_attrs[] = {
_attr_w1_slave.attr,
+   _attr_temperature.attr

[PATCH v7 8/9] w1_therm: adding alarm sysfs entry

2020-05-11 Thread Akira Shimahara
Adding device alarms settings by a dedicated sysfs entry alarms (RW):
read or write TH and TL in the device RAM. Checking devices in alarm
state could be performed using the master search command.

As alarms temperature level are store in a 8 bit register on the device
and are signed values, a safe cast shall be performed using the min and
max temperature that device are able to measure. This is done by 
int_to_short inline function.

A 'write_data' field is added in the device structure, to bind the
correct writing function, as some devices may have 2 or 3 bytes RAM.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements
Changes in v7:
- Formatting code comments and correcting comments mistakes

 .../ABI/testing/sysfs-driver-w1_therm |  16 ++
 drivers/w1/slaves/w1_therm.c  | 161 ++
 2 files changed, 177 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 6ffd3e3..f289520 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,19 @@
+What:  /sys/bus/w1/devices/.../alarms
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) read or write TH and TL (Temperature High an Low) alarms.
+   Values shall be space separated and in the device range
+   (typical -55 degC to 125 degC), if not values will be trimmed
+   to device min/max capabilities. Values are integer as they are
+   stored in a 8bit register in the device. Lowest value is
+   automatically put to TL. Once set, alarms could be search at
+   master level, refer to Documentation/w1/w1_generic.rst for
+   detailed information
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../eeprom
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 46756ea..fac9908 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -58,6 +58,9 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 #define EEPROM_CMD_WRITE"save" /* cmd for write eeprom sysfs */
 #define EEPROM_CMD_READ "restore"  /* cmd for read eeprom sysfs */
 
+#define MIN_TEMP   -55 /* min temperature that can be mesured */
+#define MAX_TEMP   125 /* max temperature that can be mesured */
+
 /* Helpers Macros */
 
 /*
@@ -96,6 +99,7 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * @get_conversion_time: pointer to the device conversion time function
  * @set_resolution: pointer to the device set_resolution function
  * @get_resolution: pointer to the device get_resolution function
+ * @write_data: pointer to the device writing function (2 or 3 bytes)
  */
 struct w1_therm_family_converter {
u8  broken;
@@ -105,6 +109,7 @@ struct w1_therm_family_converter {
int (*get_conversion_time)(struct w1_slave *sl);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
+   int (*write_data)(struct w1_slave *sl, const u8 *data);
 };
 
 /**
@@ -239,6 +244,12 @@ static ssize_t resolution_store(struct device *device,
 static ssize_t eeprom_store(struct device *device,
struct device_attribute *attr, const char *buf, size_t size);
 
+static ssize_t alarms_store(struct device *device,
+   struct device_attribute *attr, const char *buf, size_t size);
+
+static ssize_t alarms_show(struct device *device,
+   struct device_attribute *attr, char *buf);
+
 /* Attributes declarations */
 
 static DEVICE_ATTR_RW(w1_slave);
@@ -247,6 +258,7 @@ static DEVICE_ATTR_RO(temperature);
 static DEVICE_ATTR_RO(ext_power);
 static DEVICE_ATTR_RW(resolution);
 static DEVICE_ATTR_WO(eeprom);
+static DEVICE_ATTR_RW(alarms);
 
 /* Interface Functions declaration */
 
@@ -278,6 +290,7 @@ static struct attribute *w1_therm_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -286,6 +299,7 @@ static struct attribute *w1_ds18s20_attrs[] = {
_attr_temperature.attr,
_attr_ext_power.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -296,6 +310,7 @@ static struct attribute *w1_ds28ea00_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -556,6 +57

[PATCH v7 5/9] w1_therm: adding resolution sysfs entry

2020-05-11 Thread Akira Shimahara
Adding resolution sysfs entry (RW) to get or set the device resolution
Write values are managed as follow:
* '9..12': resolution to set in bit
* Anything else: do nothing
Read values are :
* '9..12': device resolution in bit
* '-xx': xx is kernel error when reading the resolution

Only supported devices will show the sysfs entry. A new family has been
created for DS18S20 devices as they do not implement resolution feature.

The resolution of each device is check when the device is
discover by the bus master, in 'w1_therm_add_slave(struct w1_slave *)'.
The status is stored in the device structure w1_therm_family_data so
that the driver always knows the resolution of each device, which could
be used later to determine the required conversion duration (resolution
dependent).

The resolution is re evaluate each time a user read or write the sysfs
entry.

To avoid looping through the w1_therm_families at run time, the pointer
'specific_functions' is set up to the correct 'w1_therm_family_converter'
when the slave is added (which mean when it is discovered by the master).
This initialization is done by a helper function 
'device_family(struct w1_slave *sl)', and a dedicated macro 
'SLAVE_SPECIFIC_FUNC(sl)' allow the access to the specific function of the
slave device.

'read_scratchpad' and 'write_scratchpad' are the hardware functions to
access the device RAM, as per protocol specification.

It cancel the former 'precision' functions, which was only set and never
read (so not stored in the device struct).

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements
Changes in v7:
- Formatting code comments and correcting comments mistakes

 .../ABI/testing/sysfs-driver-w1_therm |  17 +
 drivers/w1/slaves/w1_therm.c  | 442 ++
 2 files changed, 361 insertions(+), 98 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 99d73ee..7ed95e9 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -10,6 +10,23 @@ Users:   any user space application which wants 
to communicate with
w1_term device
 
 
+What:  /sys/bus/w1/devices/.../resolution
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) get or set the device resolution (on supported devices,
+   if not, this entry is not present). Note that the resolution
+   will be changed only in device RAM, so it will be cleared when
+   power is lost. Trigger a 'save' to EEPROM command to keep
+   values after power-on. Read or write are :
+   * '9..12': device resolution in bit
+   or resolution to set in bit
+   * '-xx': xx is kernel error when reading the resolution
+   * Anything else: do nothing
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index a148f2d..e30dab8 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -51,6 +51,13 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
 /* Helpers Macros */
 
+/*
+ * return a pointer on the slave w1_therm_family_converter struct:
+ * always test family data existence before using this macro
+ */
+#define SLAVE_SPECIFIC_FUNC(sl) \
+   (((struct w1_therm_family_data *)(sl->family_data))->specific_functions)
+
 /*
  * return the power mode of the sl slave : 1-ext, 0-parasite, <0 unknown
  * always test family data existence before using this macro
@@ -58,6 +65,13 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 #define SLAVE_POWERMODE(sl) \
(((struct w1_therm_family_data *)(sl->family_data))->external_powered)
 
+/*
+ * return the resolution in bit of the sl slave : <0 unknown
+ * always test family data existence before using this macro
+ */
+#define SLAVE_RESOLUTION(sl) \
+   (((struct w1_therm_family_data *)(sl->family_data))->resolution)
+
 /* return the address of the refcnt in the family data */
 #define THERM_REFCNT(family_data) \
(&((struct w1_therm_family_data *)family_data)->refcnt)
@@ -70,7 +84,8 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * @reserved: not used here
  * @f: pointer to the device binding structure
  * @convert: pointer to the device conversion function
- * @precision: pointer to the device precision functi

[PATCH v7 6/9] w1_therm: adding eeprom sysfs entry

2020-05-11 Thread Akira Shimahara
The driver implement 2 hardware functions to access device RAM:
 * copy_scratchpad
 * recall_scratchpad
They act according to device specifications.

As EEPROM operations are not device dependent (all w1_therm can perform
EEPROM read/write operation following the same protocol), it is removed
from device families structures.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements
Changes in v7:
- Formatting code comments and correcting comments mistakes

 .../ABI/testing/sysfs-driver-w1_therm |  14 ++
 drivers/w1/slaves/w1_therm.c  | 175 --
 2 files changed, 132 insertions(+), 57 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 7ed95e9..8b7ee89 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,17 @@
+What:  /sys/bus/w1/devices/.../eeprom
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (WO) writing that file will either trigger a save of the
+   device data to its embedded EEPROM, either restore data
+   embedded in device EEPROM. Be aware that devices support
+   limited EEPROM writing cycles (typical 50k)
+   * 'save': save device RAM to EEPROM
+   * 'restore': restore EEPROM data in device RAM
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../ext_power
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index e30dab8..957b503 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -43,12 +43,21 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/* This command should be in public header w1.h but is not */
+#define W1_RECALL_EEPROM   0xB8
+
 /* Nb of try for an operation */
 #define W1_THERM_MAX_TRY   5
 
 /* ms delay to retry bus mutex */
 #define W1_THERM_RETRY_DELAY   20
 
+/* delay in ms to write in EEPROM */
+#define W1_THERM_EEPROM_WRITE_DELAY10
+
+#define EEPROM_CMD_WRITE"save" /* cmd for write eeprom sysfs */
+#define EEPROM_CMD_READ "restore"  /* cmd for read eeprom sysfs */
+
 /* Helpers Macros */
 
 /*
@@ -86,7 +95,6 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * @convert: pointer to the device conversion function
  * @set_resolution: pointer to the device set_resolution function
  * @get_resolution: pointer to the device get_resolution function
- * @eeprom: pointer to eeprom function
  */
 struct w1_therm_family_converter {
u8  broken;
@@ -95,7 +103,6 @@ struct w1_therm_family_converter {
int (*convert)(u8 rom[9]);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
-   int (*eeprom)(struct device *device);
 };
 
 /**
@@ -165,6 +172,22 @@ static int read_scratchpad(struct w1_slave *sl, struct 
therm_info *info);
  */
 static int write_scratchpad(struct w1_slave *sl, const u8 *data, u8 nb_bytes);
 
+/**
+ * copy_scratchpad() - Copy the content of scratchpad in device EEPROM
+ * @sl: slave involved
+ *
+ * Return: 0 if success, -kernel error code otherwise
+ */
+static int copy_scratchpad(struct w1_slave *sl);
+
+/**
+ * recall_eeprom() - Restore EEPROM data to device RAM
+ * @sl: slave involved
+ *
+ * Return: 0 if success, -kernel error code otherwise
+ */
+static int recall_eeprom(struct w1_slave *sl);
+
 /**
  * read_powermode() - Query the power mode of the slave
  * @sl: slave to retrieve the power mode
@@ -199,12 +222,16 @@ static ssize_t resolution_show(struct device *device,
 static ssize_t resolution_store(struct device *device,
struct device_attribute *attr, const char *buf, size_t size);
 
+static ssize_t eeprom_store(struct device *device,
+   struct device_attribute *attr, const char *buf, size_t size);
+
 /* Attributes declarations */
 
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
 static DEVICE_ATTR_RO(ext_power);
 static DEVICE_ATTR_RW(resolution);
+static DEVICE_ATTR_WO(eeprom);
 
 /* Interface Functions declaration */
 
@@ -234,12 +261,14 @@ static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
_attr_ext_power.attr,
_attr_resolution.attr,
+   _attr_eeprom.attr,
NULL,
 };
 
 static struct attribute *w1_ds18s20_attrs[] = {
_attr_w1_slave.attr,
_attr_ext_power.attr,
+

[PATCH v7 4/9] w1_therm: adding ext_power sysfs entry

2020-05-11 Thread Akira Shimahara
Adding ext_power sysfs entry (RO). Return the power status of the device:
 - 0: device parasite powered
 - 1: device externally powered
 - xx: xx is kernel error

The power status of each device is check when the device is
discover by the bus master, in 'w1_therm_add_slave(struct w1_slave *)'.
The status is stored in the device structure w1_therm_family_data so
that the driver always knows the power state of each device, which could
be used later to determine the required strong pull up to apply on the
line.

The power status is re evaluate each time the sysfs ext_power read by
a user.

The hardware function 'read_powermode(struct w1_slave *sl)' act just as
per device specifications, sending W1_READ_PSUPPLY command on the bus,
and issue a read time slot, reading only one bit.

A helper function 'bool bus_mutex_lock(struct mutex *lock)' is introduced.
It try to aquire the bus mutex several times (W1_THERM_MAX_TRY), waiting
W1_THERM_RETRY_DELAY between two attempt.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements
Changes in v7:
- Formatting code comments and correcting comments mistakes

 .../ABI/testing/sysfs-driver-w1_therm |  12 ++
 drivers/w1/slaves/w1_therm.c  | 137 ++
 2 files changed, 149 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 4a85663..99d73ee 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,15 @@
+What:  /sys/bus/w1/devices/.../ext_power
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RO) return the power status by asking the device
+   * '0': device parasite powered
+   * '1': device externally powered
+   * '-xx': xx is kernel error when reading power status
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 6a54fc4..a148f2d 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -43,8 +43,21 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/* Nb of try for an operation */
+#define W1_THERM_MAX_TRY   5
+
+/* ms delay to retry bus mutex */
+#define W1_THERM_RETRY_DELAY   20
+
 /* Helpers Macros */
 
+/*
+ * return the power mode of the sl slave : 1-ext, 0-parasite, <0 unknown
+ * always test family data existence before using this macro
+ */
+#define SLAVE_POWERMODE(sl) \
+   (((struct w1_therm_family_data *)(sl->family_data))->external_powered)
+
 /* return the address of the refcnt in the family data */
 #define THERM_REFCNT(family_data) \
(&((struct w1_therm_family_data *)family_data)->refcnt)
@@ -73,10 +86,14 @@ struct w1_therm_family_converter {
  * struct w1_therm_family_data - device data
  * @rom: ROM device id (64bit Lasered ROM code + 1 CRC byte)
  * @refcnt: ref count
+ * @external_powered:  1 device powered externally,
+ * 0 device parasite powered,
+ * -x error or undefined
  */
 struct w1_therm_family_data {
uint8_t rom[9];
atomic_t refcnt;
+   int external_powered;
 };
 
 /**
@@ -109,6 +126,20 @@ struct therm_info {
  */
 static int reset_select_slave(struct w1_slave *sl);
 
+/**
+ * read_powermode() - Query the power mode of the slave
+ * @sl: slave to retrieve the power mode
+ *
+ * Ask the device to get its power mode (external or parasite)
+ * and store the power status in the  w1_therm_family_data.
+ *
+ * Return:
+ * * 0 parasite powered device
+ * * 1 externally powered device
+ * * <0 kernel error code
+ */
+static int read_powermode(struct w1_slave *sl);
+
 /* Sysfs interface declaration */
 
 static ssize_t w1_slave_show(struct device *device,
@@ -120,10 +151,14 @@ static ssize_t w1_slave_store(struct device *device,
 static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
 
+static ssize_t ext_power_show(struct device *device,
+   struct device_attribute *attr, char *buf);
+
 /* Attributes declarations */
 
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
+static DEVICE_ATTR_RO(ext_power);
 
 /* Interface Functions declaration */
 
@@ -151,12 +186,14 @@ static void w1_therm_remove_slave(struct w1_slave *sl);
 
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+   _attr_ext_power.attr,

[PATCH v7 3/9] w1_therm: adding sysfs-driver-w1_therm doc

2020-05-11 Thread Akira Shimahara
Adding a sysfs-driver-w1_therm documentation file in
Documentation/ABI/testing. It describe the onlys sysfs entry of w1_therm
module, based on Documentation/w1/slaves/w1_therm.rst

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements
Changes in v7:
- Formatting code comments and correcting comments mistakes

 Documentation/ABI/testing/sysfs-driver-w1_therm | 17 +
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_therm

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
new file mode 100644
index 000..4a85663
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -0,0 +1,17 @@
+What:  /sys/bus/w1/devices/.../w1_slave
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) return the temperature in 1/1000 degC.
+   *read*: return 2 lines with the hexa output data sent on the
+   bus, return the CRC check and temperature in 1/1000 degC
+   *write* :
+   * '0' : save the 2 or 3 bytes to the device EEPROM
+   (i.e. TH, TL and config register)
+   * '9..12' : set the device resolution in RAM
+   (if supported)
+   * Anything else: do nothing
+   refer to Documentation/w1/slaves/w1_therm.rst for detailed
+   information.
+Users: any user space application which wants to communicate with
+   w1_term device
\ No newline at end of file
-- 
2.26.2



[PATCH v7 2/9] w1_therm: fix reset_select_slave during discovery

2020-05-11 Thread Akira Shimahara
Fix reset_select_slave issue during devices discovery by the master on
bus. The w1_reset_select_slave() from w1_io.c, which was previously used,
assume that if the slave count is 1 there is only one slave attached on
the bus. This is not always true. For example when discovering devices,
when the first device is discover by the bus master, its slave count is
1, but some other slaves may be on the bus.

In that case instead of adressing command to the attached slave the 
master throw a SKIP ROM command so that all slaves attached on the bus
will answer simultenaously causing data collision.

A dedicated reset_select_slave() function is implemented here,
it always perform an adressing to each slave using the MATCH ROM
command.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements
Changes in v7:
- Formatting code comments and correcting comments mistakes

 drivers/w1/slaves/w1_therm.c | 48 ++--
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 67204b3..6a54fc4 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -90,6 +91,24 @@ struct therm_info {
u8 verdict;
 };
 
+/* Hardware Functions declaration */
+
+/**
+ * reset_select_slave() - reset and select a slave
+ * @sl: the slave to select
+ *
+ * Resets the bus and select the slave by sending a ROM MATCH cmd
+ * w1_reset_select_slave() from w1_io.c could not be used here because
+ * it sent a SKIP ROM command if only one device is on the line.
+ * At the beginning of the such process, sl->master->slave_count is 1 even if
+ * more devices are on the line, causing collision on the line.
+ *
+ * Context: The w1 master lock must be held.
+ *
+ * Return: 0 if success, negative kernel error code otherwise.
+ */
+static int reset_select_slave(struct w1_slave *sl);
+
 /* Sysfs interface declaration */
 
 static ssize_t w1_slave_show(struct device *device,
@@ -301,7 +320,7 @@ static inline int w1_DS18B20_precision(struct device 
*device, int val)
while (max_trying--) {
crc = 0;
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
int count = 0;
 
/* read values to only alter precision bits */
@@ -314,7 +333,7 @@ static inline int w1_DS18B20_precision(struct device 
*device, int val)
if (rom[8] == crc) {
rom[4] = (rom[4] & ~mask) | (precision_bits & 
mask);
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
w1_write_8(dev, W1_WRITE_SCRATCHPAD);
w1_write_8(dev, rom[2]);
w1_write_8(dev, rom[3]);
@@ -460,6 +479,21 @@ static void w1_therm_remove_slave(struct w1_slave *sl)
 
 /* Hardware Functions */
 
+/* Safe version of reset_select_slave - avoid using the one in w_io.c */
+static int reset_select_slave(struct w1_slave *sl)
+{
+   u8 match[9] = { W1_MATCH_ROM, };
+   u64 rn = le64_to_cpu(*((u64 *)>reg_num));
+
+   if (w1_reset_bus(sl->master))
+   return -ENODEV;
+
+   memcpy([1], , 8);
+   w1_write_block(sl->master, match, 9);
+
+   return 0;
+}
+
 static ssize_t read_therm(struct device *device,
  struct w1_slave *sl, struct therm_info *info)
 {
@@ -487,7 +521,7 @@ static ssize_t read_therm(struct device *device,
info->verdict = 0;
info->crc = 0;
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
int count = 0;
unsigned int tm = 750;
unsigned long sleep_rem;
@@ -495,7 +529,7 @@ static ssize_t read_therm(struct device *device,
w1_write_8(dev, W1_READ_PSUPPLY);
external_power = w1_read_8(dev);
 
-   if (w1_reset_select_slave(sl))
+   if (reset_select_slave(sl))
continue;
 
/* 750ms strong pullup (or delay) after the convert */
@@ -525,7 +559,7 @@ static ssize_t read_therm(struct device *device,
}
}
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
 
w1_write_8(dev, W1_READ_SCRATCHPAD);
count = w1_read_block(dev, info->rom, 9);
@@ -577,7 +611,7 @@ static inline int w1_th

[PATCH v7 1/9] w1_therm: adding code comments and code reordering

2020-05-11 Thread Akira Shimahara
Adding code comments to split code in dedicated parts. After the global
declarations (defines, macros and function declarations), code is organized
as follow :
 - Device and family dependent structures and functions
 - Interfaces functions
 - Helpers functions
 - Hardware functions
 - Sysfs interface functions

Signed-off-by: Akira Shimahara 
---
Main motivation on the first patch of this serie is to clean up the code,
document it and reorder it to prepare the next patches, which are clearer
after this.

One main point is to keep all device/family dependent code gather at the
beginning to ease adding new devices if required.

Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements
Changes in v7:
- Formatting code comments and correcting comments mistakes

 drivers/w1/slaves/w1_therm.c | 427 +--
 1 file changed, 259 insertions(+), 168 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 18f08d7..67204b3 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -25,7 +25,8 @@
 #define W1_THERM_DS18250x3B
 #define W1_THERM_DS28EA00  0x42
 
-/* Allow the strong pullup to be disabled, but default to enabled.
+/*
+ * Allow the strong pullup to be disabled, but default to enabled.
  * If it was disabled a parasite powered device might not get the require
  * current to do a temperature conversion.  If it is enabled parasite powered
  * devices have a better chance of getting the current required.
@@ -41,42 +42,55 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/* Helpers Macros */
+
+/* return the address of the refcnt in the family data */
+#define THERM_REFCNT(family_data) \
+   (&((struct w1_therm_family_data *)family_data)->refcnt)
+
+/* Structs definition */
+
+/**
+ * struct w1_therm_family_converter - bind device specific functions
+ * @broken: flag for non-registred families
+ * @reserved: not used here
+ * @f: pointer to the device binding structure
+ * @convert: pointer to the device conversion function
+ * @precision: pointer to the device precision function
+ * @eeprom: pointer to eeprom function
+ */
+struct w1_therm_family_converter {
+   u8  broken;
+   u16 reserved;
+   struct w1_family*f;
+   int (*convert)(u8 rom[9]);
+   int (*precision)(struct device *device, int val);
+   int (*eeprom)(struct device *device);
+};
+
+/**
+ * struct w1_therm_family_data - device data
+ * @rom: ROM device id (64bit Lasered ROM code + 1 CRC byte)
+ * @refcnt: ref count
+ */
 struct w1_therm_family_data {
uint8_t rom[9];
atomic_t refcnt;
 };
 
+/**
+ * struct therm_info - store temperature reading
+ * @rom: read device data (8 data bytes + 1 CRC byte)
+ * @crc: computed crc from rom
+ * @verdict: 1 crc checked, 0 crc not matching
+ */
 struct therm_info {
u8 rom[9];
u8 crc;
u8 verdict;
 };
 
-/* return the address of the refcnt in the family data */
-#define THERM_REFCNT(family_data) \
-   (&((struct w1_therm_family_data *)family_data)->refcnt)
-
-static int w1_therm_add_slave(struct w1_slave *sl)
-{
-   sl->family_data = kzalloc(sizeof(struct w1_therm_family_data),
-   GFP_KERNEL);
-   if (!sl->family_data)
-   return -ENOMEM;
-   atomic_set(THERM_REFCNT(sl->family_data), 1);
-   return 0;
-}
-
-static void w1_therm_remove_slave(struct w1_slave *sl)
-{
-   int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data));
-
-   while (refcnt) {
-   msleep(1000);
-   refcnt = atomic_read(THERM_REFCNT(sl->family_data));
-   }
-   kfree(sl->family_data);
-   sl->family_data = NULL;
-}
+/* Sysfs interface declaration */
 
 static ssize_t w1_slave_show(struct device *device,
struct device_attribute *attr, char *buf);
@@ -87,9 +101,35 @@ static ssize_t w1_slave_store(struct device *device,
 static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
 
+/* Attributes declarations */
+
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
 
+/* Interface Functions declaration */
+
+/**
+ * w1_therm_add_slave() - Called when a new slave is discovered
+ * @sl: slave just discovered by the master.
+ *
+ * Called by the master when the slave is discovered on the bus. Used to
+ * initialize slave state before the beginning of any communication.
+ *
+ * Return: 0 - If success, negative kernel code otherwise
+ */
+static int w1_therm_add_slave(struct w1_slave *sl);
+
+/**
+ * w1_therm_remove_slave() - Called when a slave is removed
+ * @sl: slave to be removed.
+ *
+ * Called by the master when the slave is considered not to be o

Re: [PATCH v6 1/9] w1_therm: adding code comments and code reordering

2020-05-11 Thread Akira shimahara
Hi,

Le dimanche 10 mai 2020 à 20:33 -0700, Randy Dunlap a écrit :
> Hi,
> 
> A few more comments here (inline):
> 
> On 5/10/20 7:15 AM, Akira Shimahara wrote:
> 
> >  drivers/w1/slaves/w1_therm.c | 398 ---
> >  1 file changed, 232 insertions(+), 166 deletions(-)
> > 
> > diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
> > index 18f08d7..890cf09 100644
> > --- a/drivers/w1/slaves/w1_therm.c
> > +++ b/drivers/w1/slaves/w1_therm.c
> > @@ -41,42 +41,55 @@
> >  static int w1_strong_pullup = 1;
> >  module_param_named(strong_pullup, w1_strong_pullup, int, 0);
> >  
> > +/* Helpers Macros */
> > +
> > +/* return the address of the refcnt in the family data */
> > +#define THERM_REFCNT(family_data) \
> > +   (&((struct w1_therm_family_data *)family_data)->refcnt)
> > +
> > +/* Structs definition */
> > +
> > +/**
> > + * struct w1_therm_family_converter - bind device specific functions
> > + * @broken: flag for non registred families
> 
> non-registered

Noted, will be corrected

> 
> > + * @reserved: not used here
> > + * @f: pointer to the device binding structure
> > + * @convert: pointer to the device conversion function
> > + * @precision: pointer to the device precicion function
> 
> precision

Noted, will be corrected

> 
> > + * @eeprom: pointer to eeprom function
> > + */
> > +struct w1_therm_family_converter {
> > +   u8  broken;
> > +   u16 reserved;
> > +   struct w1_family*f;
> > +   int (*convert)(u8 rom[9]);
> > +   int (*precision)(struct device *device, int val);
> > +   int (*eeprom)(struct device *device);
> > +};
> > +
> > +/**
> > + * struct w1_therm_family_data - device data
> > + * @rom: ROM id of the device
> > + * @refcnt: ref count
> > + */
> >  struct w1_therm_family_data {
> > uint8_t rom[9];
> 
> Why is "rom" 9 bytes in length?  Does it come from some
> spec or standard?  Can it be a macro instead of an arbitrary
> magic number?

9 bytes is required per device communication protocol (8 data bytes + 1 CRC). I
will detailled that in comments but I don't think that macro is required.

> 
> > atomic_t refcnt;
> >  };
> >  
> > +/**
> > + * struct therm_info - store temperature reading
> > + * @rom: readen device data
> 
> read

Noted, will be corrected

> 
> > + * @crc: computed crc from rom
> > + * @verdict: 1 crc checked, 0 crc not matching
> > + */
> >  struct therm_info {
> > u8 rom[9];
> > u8 crc;
> > u8 verdict;
> >  };
> >  
> ...
> 

Same as above

> >  
> > +/* Interface Functions declaration */
> > +
> > +/**
> > + * w1_therm_add_slave() - Called when a new slave is discovered
> > + * @sl: slave just discovered by the master.
> > + *
> > + * Called by the master when the slave is discovered on the bus.Used to
> 
>bus. Used to
> 
> > + * initialized slave state before the beginning of any communication.
> 
>   initialize

Noted, will be corrected

> 
> > + *
> > + * Return: 0 - If success, negative kernel code otherwise
> > + */
> > +static int w1_therm_add_slave(struct w1_slave *sl);
> > +
> > +/**
> > + * w1_therm_remove_slave() - Called when a slave is removed
> > + * @sl: slave to be removed.
> > + *
> > + * Called by the master when the slave is considered not to be on the bus
> > + * anymore. Used to free memory.
> > + */
> > +static void w1_therm_remove_slave(struct w1_slave *sl);
> > +
> > +/* Family attributes */
> > +
> >  static struct attribute *w1_therm_attrs[] = {
> > _attr_w1_slave.attr,
> > NULL,
> > @@ -101,6 +140,8 @@ static struct attribute *w1_ds28ea00_attrs[] = {
> > NULL,
> >  };
> >  
> > +/* Attribute groups */
> > +
> >  ATTRIBUTE_GROUPS(w1_therm);
> >  ATTRIBUTE_GROUPS(w1_ds28ea00);
> >  
> > @@ -154,6 +195,8 @@ static const struct hwmon_chip_info w1_chip_info = {
> >  #define W1_CHIPINFONULL
> >  #endif
> >  
> > +/* Family operations */
> > +
> >  static struct w1_family_ops w1_therm_fops = {
> > .add_slave  = w1_therm_add_slave,
> > .remove_slave   = w1_therm_remove_slave,
> > @@ -168,6 +211,8 @@ static struct w

Re: [PATCH v6 5/9] w1_therm: adding resolution sysfs entry

2020-05-11 Thread Akira shimahara
Hi,

Le dimanche 10 mai 2020 à 20:25 -0700, Randy Dunlap a écrit :
> Hi,
> 
> 
> 
> The kernel-doc comment changes look good.  Thanks for doing that.
> 
> 
> 
> 
> 
> On 5/10/20 7:17 AM, Akira Shimahara wrote:
> 
> 
> 
> > diff --git a/drivers/w1/slaves/w1_therm.c
> > b/drivers/w1/slaves/w1_therm.c
> > index 08579dc..b1734ae 100644
> > --- a/drivers/w1/slaves/w1_therm.c
> > +++ b/drivers/w1/slaves/w1_therm.c
> > @@ -50,12 +50,24 @@ module_param_named(strong_pullup,
> > w1_strong_pullup, int, 0);
> >   
> >   /* Helpers Macros */
> >   
> > +/* return a pointer on the slave w1_therm_family_converter struct:
> > + * always test family data existence before
> > + */
> 
> 
> /*
> 
>  * Kernel multi-line comment coding style (except for networking
> source code)
> 
>  * is like this.
> 
>  */
> 
> 
> 
> (in multiple places)
> 
> 
> 
> > +/**
> > + * write_scratchpad() - write nb_bytes in the device RAM
> > + * @sl: pointer to the slave to write in
> > + * @data: pointer to an array of 3 bytes, as 3 bytes MUST be
> > written
> > + * @nb_bytes: Nb bytes to be written (2 for DS18S20, 3 for other
> > devices)
> 
> 
> If Nb means Number, please spell it out.
> 
> 
> 
> > + *
> > + * Return: 0 if success, -kernel error code otherwise
> > + */
> > +static int write_scratchpad(struct w1_slave *sl, const u8 *data,
> > u8 nb_bytes);
> > +
> >   /**
> >* read_powermode() - Query the power mode of the slave
> >* @sl: slave to retrieve the power mode
> 
> 
> 
> 
> thanks.
> 
> --
> 
> ~Randy

Thanks for your comments and your time,
well noted, I will do
accordingly.

Akira Shimahara



[PATCH v6 8/9] w1_therm: adding alarm sysfs entry

2020-05-10 Thread Akira Shimahara
Adding device alarms settings by a dedicated sysfs entry alarms (RW):
read or write TH and TL in the device RAM. Checking devices in alarm
state could be performed using the master search command.

As alarms temperature level are store in a 8 bit register on the device
and are signed values, a safe cast shall be performed using the min and
max temperature that device are able to measure. This is done by 
int_to_short inline function.

A 'write_data' field is added in the device structure, to bind the
correct writing function, as some devices may have 2 or 3 bytes RAM.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements

 .../ABI/testing/sysfs-driver-w1_therm |  16 ++
 drivers/w1/slaves/w1_therm.c  | 160 ++
 2 files changed, 176 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 6ffd3e3..f289520 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,19 @@
+What:  /sys/bus/w1/devices/.../alarms
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) read or write TH and TL (Temperature High an Low) alarms.
+   Values shall be space separated and in the device range
+   (typical -55 degC to 125 degC), if not values will be trimmed
+   to device min/max capabilities. Values are integer as they are
+   stored in a 8bit register in the device. Lowest value is
+   automatically put to TL. Once set, alarms could be search at
+   master level, refer to Documentation/w1/w1_generic.rst for
+   detailed information
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../eeprom
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 93855ca..1301b16 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -57,6 +57,9 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 #define EEPROM_CMD_WRITE"save" /* cmd for write eeprom sysfs */
 #define EEPROM_CMD_READ "restore"  /* cmd for read eeprom sysfs */
 
+#define MIN_TEMP   -55 /* min temperature that can be mesured */
+#define MAX_TEMP   125 /* max temperature that can be mesured */
+
 /* Helpers Macros */
 
 /* return a pointer on the slave w1_therm_family_converter struct:
@@ -92,6 +95,7 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * @get_conversion_time: pointer to the device conversion time function
  * @set_resolution: pointer to the device set_resolution function
  * @get_resolution: pointer to the device get_resolution function
+ * @write_data: pointer to the device writing function (2 or 3 bytes)
  */
 struct w1_therm_family_converter {
u8  broken;
@@ -101,6 +105,7 @@ struct w1_therm_family_converter {
int (*get_conversion_time)(struct w1_slave *sl);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
+   int (*write_data)(struct w1_slave *sl, const u8 *data);
 };
 
 /**
@@ -235,6 +240,12 @@ static ssize_t resolution_store(struct device *device,
 static ssize_t eeprom_store(struct device *device,
struct device_attribute *attr, const char *buf, size_t size);
 
+static ssize_t alarms_store(struct device *device,
+   struct device_attribute *attr, const char *buf, size_t size);
+
+static ssize_t alarms_show(struct device *device,
+   struct device_attribute *attr, char *buf);
+
 /* Attributes declarations */
 
 static DEVICE_ATTR_RW(w1_slave);
@@ -243,6 +254,7 @@ static DEVICE_ATTR_RO(temperature);
 static DEVICE_ATTR_RO(ext_power);
 static DEVICE_ATTR_RW(resolution);
 static DEVICE_ATTR_WO(eeprom);
+static DEVICE_ATTR_RW(alarms);
 
 /* Interface Functions declaration */
 
@@ -274,6 +286,7 @@ static struct attribute *w1_therm_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -282,6 +295,7 @@ static struct attribute *w1_ds18s20_attrs[] = {
_attr_temperature.attr,
_attr_ext_power.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -292,6 +306,7 @@ static struct attribute *w1_ds28ea00_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -537,6 +55

[PATCH v6 9/9] w1_therm: adding bulk read support to trigger multiple conversion on bus

2020-05-10 Thread Akira Shimahara
Adding bulk read support:
Sending a 'trigger' command in the dedicated sysfs entry of bus master
device send a conversion command for all the slaves on the bus. The sysfs
entry is added as soon as at least one device supporting this feature
is detected on the bus.

The behavior of the sysfs reading temperature on the device is as follow:
 * If no bulk read pending, trigger a conversion on the device, wait for
 the conversion to be done, read the temperature in device RAM
 * If a bulk read has been trigger, access directly the device RAM
This behavior is the same on the 2 sysfs entries ('temperature' and
'w1_slave').

Reading the therm_bulk_read sysfs give the status of bulk operations:
 * '-1': conversion in progress on at least 1 sensor
 * '1': conversion complete but at least one sensor has not been read yet
 * '0': no bulk operation. Reading temperature on ecah device will trigger
a conversion

As not all devices support bulk read feature, it has been added in device
family structure.

The attribute is set at master level as soon as a supporting device is
discover. It is removed when the last supported device leave the bus.
The count of supported device is kept with the static counter
bulk_read_device_counter.

A strong pull up is apply on the line if at least one device required it.
The duration of the pull up is the max time required by a device on the
line, which depends on the resolution settings of each device. The strong
pull up could be adjust with the a module parameter.

Updating documentation in Documentation/ABI/testing/sysfs-driver-w1_therm
and Documentation/w1/slaves/w1_therm.rst accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements

 .../ABI/testing/sysfs-driver-w1_therm |  36 ++-
 Documentation/w1/slaves/w1_therm.rst  |  50 +++-
 drivers/w1/slaves/w1_therm.c  | 247 +-
 3 files changed, 318 insertions(+), 15 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index f289520..076659d 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -62,9 +62,16 @@ Date:May 2020
 Contact:   Akira Shimahara 
 Description:
(RO) return the temperature in 1/1000 degC.
-   Note that the conversion duration depend on the resolution (if
-   device support this feature). It takes 94ms in 9bits
-   resolution, 750ms for 12bits.
+   * If a bulk read has been triggered, it will directly
+   return the temperature computed when the bulk read
+   occurred, if available. If not yet available, nothing
+   is returned (a debug kernel message is sent), you
+   should retry later on.
+   * If no bulk read has been triggered, it will trigger
+   a conversion and send the result. Note that the
+   conversion duration depend on the resolution (if
+   device support this feature). It takes 94ms in 9bits
+   resolution, 750ms for 12bits.
 Users: any user space application which wants to communicate with
w1_term device
 
@@ -85,4 +92,25 @@ Description:
refer to Documentation/w1/slaves/w1_therm.rst for detailed
information.
 Users: any user space application which wants to communicate with
-   w1_term device
\ No newline at end of file
+   w1_term device
+
+
+What:  /sys/bus/w1/devices/w1_bus_masterXX/therm_bulk_read
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) trigger a bulk read conversion. read the status
+   *read*:
+   * '-1': conversion in progress on at least 1 sensor
+   * '1' : conversion complete but at least one sensor
+   value has not been read yet
+   * '0' : no bulk operation. Reading temperature will
+   trigger a conversion on each device
+   *write*: 'trigger': trigger a bulk read on all supporting
+   devices on the bus
+   Note that if a bulk read is sent but one sensor is not read
+   immediately, the next access to temperature on this device
+   will return the temperature measured at the time of issue
+   of the bulk read command (not the current temperature).
+Users: any user space application which wants to communicate with
+   w1_term device
diff --git a/Documentation/w1/slaves/w1_therm.rst 
b/Documentation/w1/slaves

[PATCH v6 5/9] w1_therm: adding resolution sysfs entry

2020-05-10 Thread Akira Shimahara
Adding resolution sysfs entry (RW) to get or set the device resolution
Write values are managed as follow:
* '9..12': resolution to set in bit
* Anything else: do nothing
Read values are :
* '9..12': device resolution in bit
* '-xx': xx is kernel error when reading the resolution

Only supported devices will show the sysfs entry. A new family has been
created for DS18S20 devices as they do not implement resolution feature.

The resolution of each device is check when the device is
discover by the bus master, in 'w1_therm_add_slave(struct w1_slave *)'.
The status is stored in the device structure w1_therm_family_data so
that the driver always knows the resolution of each device, which could
be used later to determine the required conversion duration (resolution
dependent).

The resolution is re evaluate each time a user read or write the sysfs
entry.

To avoid looping through the w1_therm_families at run time, the pointer
'specific_functions' is set up to the correct 'w1_therm_family_converter'
when the slave is added (which mean when it is discovered by the master).
This initialization is done by a helper function 
'device_family(struct w1_slave *sl)', and a dedicated macro 
'SLAVE_SPECIFIC_FUNC(sl)' allow the access to the specific function of the
slave device.

'read_scratchpad' and 'write_scratchpad' are the hardware functions to
access the device RAM, as per protocol specification.

It cancel the former 'precision' functions, which was only set and never
read (so not stored in the device struct).

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements

 .../ABI/testing/sysfs-driver-w1_therm |  17 +
 drivers/w1/slaves/w1_therm.c  | 438 ++
 2 files changed, 357 insertions(+), 98 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 99d73ee..7ed95e9 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -10,6 +10,23 @@ Users:   any user space application which wants 
to communicate with
w1_term device
 
 
+What:  /sys/bus/w1/devices/.../resolution
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) get or set the device resolution (on supported devices,
+   if not, this entry is not present). Note that the resolution
+   will be changed only in device RAM, so it will be cleared when
+   power is lost. Trigger a 'save' to EEPROM command to keep
+   values after power-on. Read or write are :
+   * '9..12': device resolution in bit
+   or resolution to set in bit
+   * '-xx': xx is kernel error when reading the resolution
+   * Anything else: do nothing
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 08579dc..b1734ae 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -50,12 +50,24 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
 /* Helpers Macros */
 
+/* return a pointer on the slave w1_therm_family_converter struct:
+ * always test family data existence before
+ */
+#define SLAVE_SPECIFIC_FUNC(sl) \
+   (((struct w1_therm_family_data *)(sl->family_data))->specific_functions)
+
 /* return the power mode of the sl slave : 1-ext, 0-parasite, <0 unknown
  * always test family data existence before
  */
 #define SLAVE_POWERMODE(sl) \
(((struct w1_therm_family_data *)(sl->family_data))->external_powered)
 
+/* return the resolution in bit of the sl slave : <0 unknown
+ * always test family data existence before
+ */
+#define SLAVE_RESOLUTION(sl) \
+   (((struct w1_therm_family_data *)(sl->family_data))->resolution)
+
 /* return the address of the refcnt in the family data */
 #define THERM_REFCNT(family_data) \
(&((struct w1_therm_family_data *)family_data)->refcnt)
@@ -68,7 +80,8 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * @reserved: not used here
  * @f: pointer to the device binding structure
  * @convert: pointer to the device conversion function
- * @precision: pointer to the device precicion function
+ * @set_resolution: pointer to the device set_resolution function
+ * @get_resolution: pointer to the device get_resolution function
  * @eeprom: pointer to eeprom function
  */
 struct w1_therm_family_conve

[PATCH v6 7/9] w1_therm: optimizing temperature read timings

2020-05-10 Thread Akira Shimahara
Optimizing temperature reading by reducing waiting conversion time
according to device resolution settings, as per device specification.
This is device dependent as not all the devices supports resolution
setting, so it has been added in device family structures.

The process to read the temperature on the device has been adapted in a
new function 'convert_t()', which replace the former 'read_therm()', is
introduce to deal with this timing. Strong pull up is also applied during
the required time, according to device power status needs and
'strong_pullup' module parameter.

'temperature_from_RAM()' function is introduced to get the correct
temperature computation (device dependent) from device RAM data.

An new sysfs entry has been added to ouptut only temperature. The old
entry w1_slave has been kept for compatibility, without changing its
output format.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements

 .../ABI/testing/sysfs-driver-w1_therm |  12 +
 drivers/w1/slaves/w1_therm.c  | 284 --
 2 files changed, 201 insertions(+), 95 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 8b7ee89..6ffd3e3 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -41,6 +41,18 @@ Users:   any user space application which wants 
to communicate with
w1_term device
 
 
+What:  /sys/bus/w1/devices/.../temperature
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RO) return the temperature in 1/1000 degC.
+   Note that the conversion duration depend on the resolution (if
+   device support this feature). It takes 94ms in 9bits
+   resolution, 750ms for 12bits.
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index c90a91f..93855ca 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -89,6 +89,7 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * @reserved: not used here
  * @f: pointer to the device binding structure
  * @convert: pointer to the device conversion function
+ * @get_conversion_time: pointer to the device conversion time function
  * @set_resolution: pointer to the device set_resolution function
  * @get_resolution: pointer to the device get_resolution function
  */
@@ -97,6 +98,7 @@ struct w1_therm_family_converter {
u16 reserved;
struct w1_family*f;
int (*convert)(u8 rom[9]);
+   int (*get_conversion_time)(struct w1_slave *sl);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
 };
@@ -149,6 +151,15 @@ struct therm_info {
  */
 static int reset_select_slave(struct w1_slave *sl);
 
+/**
+ * convert_t() - Query the device for temperature conversion and read
+ * @sl: pointer to the slave to read
+ * @info: pointer to a structure to store the read results
+ *
+ * Return: 0 if success, -kernel error code otherwise
+ */
+static int convert_t(struct w1_slave *sl, struct therm_info *info);
+
 /**
  * read_scratchpad() - read the data in device RAM
  * @sl: pointer to the slave to read
@@ -209,6 +220,9 @@ static ssize_t w1_slave_store(struct device *device,
 static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
 
+static ssize_t temperature_show(struct device *device,
+   struct device_attribute *attr, char *buf);
+
 static ssize_t ext_power_show(struct device *device,
struct device_attribute *attr, char *buf);
 
@@ -225,6 +239,7 @@ static ssize_t eeprom_store(struct device *device,
 
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
+static DEVICE_ATTR_RO(temperature);
 static DEVICE_ATTR_RO(ext_power);
 static DEVICE_ATTR_RW(resolution);
 static DEVICE_ATTR_WO(eeprom);
@@ -255,6 +270,7 @@ static void w1_therm_remove_slave(struct w1_slave *sl);
 
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+   _attr_temperature.attr,
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
@@ -263,6 +279,7 @@ static struct attribute *w1_therm_attrs[] = {
 
 static struct attribute *w1_ds18s20_attrs[] = {
_attr_w1_slave.attr,
+   _attr_temperature.attr,
_attr_ext_power.attr,
_attr_eeprom.attr,
NULL,
@@ -271,6

[PATCH v6 6/9] w1_therm: adding eeprom sysfs entry

2020-05-10 Thread Akira Shimahara
Adding eeprom sysfs entry (WO) to trigger either device EEPROM save
(by writing 'save' in the sysfs), either device EEPROM restore (by writing
'restore' in the sysfs). All the RAM of the device will be save/restore,
whatever its size (actually 2 or 3 bytes): resolution config register and
alarms level will be save/restore.

The driver implement 2 hardware functions to access device RAM:
 * copy_scratchpad
 * recall_scratchpad
They act according to device specifications.

As EEPROM operations are not device dependent (all w1_therm can perform
EEPROM read/write operation following the same protocol), it is removed
from device families structures.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements

 .../ABI/testing/sysfs-driver-w1_therm |  14 ++
 drivers/w1/slaves/w1_therm.c  | 177 --
 2 files changed, 133 insertions(+), 58 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 7ed95e9..8b7ee89 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,17 @@
+What:  /sys/bus/w1/devices/.../eeprom
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (WO) writing that file will either trigger a save of the
+   device data to its embedded EEPROM, either restore data
+   embedded in device EEPROM. Be aware that devices support
+   limited EEPROM writing cycles (typical 50k)
+   * 'save': save device RAM to EEPROM
+   * 'restore': restore EEPROM data in device RAM
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../ext_power
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index b1734ae..c90a91f 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -42,12 +42,21 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/* This command should be in public header w1.h but is not */
+#define W1_RECALL_EEPROM   0xB8
+
 /* Nb of try for an operation */
 #define W1_THERM_MAX_TRY   5
 
 /* ms delay to retry bus mutex */
 #define W1_THERM_RETRY_DELAY   20
 
+/* delay in ms to write in EEPROM */
+#define W1_THERM_EEPROM_WRITE_DELAY10
+
+#define EEPROM_CMD_WRITE"save" /* cmd for write eeprom sysfs */
+#define EEPROM_CMD_READ "restore"  /* cmd for read eeprom sysfs */
+
 /* Helpers Macros */
 
 /* return a pointer on the slave w1_therm_family_converter struct:
@@ -82,7 +91,6 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * @convert: pointer to the device conversion function
  * @set_resolution: pointer to the device set_resolution function
  * @get_resolution: pointer to the device get_resolution function
- * @eeprom: pointer to eeprom function
  */
 struct w1_therm_family_converter {
u8  broken;
@@ -91,7 +99,6 @@ struct w1_therm_family_converter {
int (*convert)(u8 rom[9]);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
-   int (*eeprom)(struct device *device);
 };
 
 /**
@@ -161,6 +168,22 @@ static int read_scratchpad(struct w1_slave *sl, struct 
therm_info *info);
  */
 static int write_scratchpad(struct w1_slave *sl, const u8 *data, u8 nb_bytes);
 
+/**
+ * copy_scratchpad() - Copy the content of scratchpad in device EEPROM
+ * @sl: slave involved
+ *
+ * Return: 0 if success, -kernel error code otherwise
+ */
+static int copy_scratchpad(struct w1_slave *sl);
+
+/**
+ * recall_eeprom() - Restore EEPROM data to device RAM
+ * @sl: slave involved
+ *
+ * Return: 0 if success, -kernel error code otherwise
+ */
+static int recall_eeprom(struct w1_slave *sl);
+
 /**
  * read_powermode() - Query the power mode of the slave
  * @sl: slave to retrieve the power mode
@@ -195,12 +218,16 @@ static ssize_t resolution_show(struct device *device,
 static ssize_t resolution_store(struct device *device,
struct device_attribute *attr, const char *buf, size_t size);
 
+static ssize_t eeprom_store(struct device *device,
+   struct device_attribute *attr, const char *buf, size_t size);
+
 /* Attributes declarations */
 
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
 static DEVICE_ATTR_RO(ext_power);
 static DEVICE_ATTR_RW(resolution);
+static DEVICE_ATTR_WO(eeprom);
 
 /* Interface Functions declaration */
 
@@ -230,

[PATCH v6 4/9] w1_therm: adding ext_power sysfs entry

2020-05-10 Thread Akira Shimahara
Adding ext_power sysfs entry (RO). Return the power status of the device:
 - 0: device parasite powered
 - 1: device externally powered
 - xx: xx is kernel error

The power status of each device is check when the device is
discover by the bus master, in 'w1_therm_add_slave(struct w1_slave *)'.
The status is stored in the device structure w1_therm_family_data so
that the driver always knows the power state of each device, which could
be used later to determine the required strong pull up to apply on the
line.

The power status is re evaluate each time the sysfs ext_power read by
a user.

The hardware function 'read_powermode(struct w1_slave *sl)' act just as
per device specifications, sending W1_READ_PSUPPLY command on the bus,
and issue a read time slot, reading only one bit.

A helper function 'bool bus_mutex_lock(struct mutex *lock)' is introduced.
It try to aquire the bus mutex several times (W1_THERM_MAX_TRY), waiting
W1_THERM_RETRY_DELAY between two attempt.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements

 .../ABI/testing/sysfs-driver-w1_therm |  12 ++
 drivers/w1/slaves/w1_therm.c  | 135 ++
 2 files changed, 147 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 4a85663..99d73ee 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,15 @@
+What:  /sys/bus/w1/devices/.../ext_power
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RO) return the power status by asking the device
+   * '0': device parasite powered
+   * '1': device externally powered
+   * '-xx': xx is kernel error when reading power status
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 5de1a67..08579dc 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -42,8 +42,20 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/* Nb of try for an operation */
+#define W1_THERM_MAX_TRY   5
+
+/* ms delay to retry bus mutex */
+#define W1_THERM_RETRY_DELAY   20
+
 /* Helpers Macros */
 
+/* return the power mode of the sl slave : 1-ext, 0-parasite, <0 unknown
+ * always test family data existence before
+ */
+#define SLAVE_POWERMODE(sl) \
+   (((struct w1_therm_family_data *)(sl->family_data))->external_powered)
+
 /* return the address of the refcnt in the family data */
 #define THERM_REFCNT(family_data) \
(&((struct w1_therm_family_data *)family_data)->refcnt)
@@ -72,10 +84,14 @@ struct w1_therm_family_converter {
  * struct w1_therm_family_data - device data
  * @rom: ROM id of the device
  * @refcnt: ref count
+ * @external_powered:  1 device powered externally,
+ * 0 device parasite powered,
+ * -x error or undefined
  */
 struct w1_therm_family_data {
uint8_t rom[9];
atomic_t refcnt;
+   int external_powered;
 };
 
 /**
@@ -108,6 +124,20 @@ struct therm_info {
  */
 static int reset_select_slave(struct w1_slave *sl);
 
+/**
+ * read_powermode() - Query the power mode of the slave
+ * @sl: slave to retrieve the power mode
+ *
+ * Ask the device to get its power mode (external or parasite)
+ * and store the power status in the  w1_therm_family_data.
+ *
+ * Return:
+ * * 0 parasite powered device
+ * * 1 externally powered device
+ * * <0 kernel error code
+ */
+static int read_powermode(struct w1_slave *sl);
+
 /* Sysfs interface declaration */
 
 static ssize_t w1_slave_show(struct device *device,
@@ -119,10 +149,14 @@ static ssize_t w1_slave_store(struct device *device,
 static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
 
+static ssize_t ext_power_show(struct device *device,
+   struct device_attribute *attr, char *buf);
+
 /* Attributes declarations */
 
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
+static DEVICE_ATTR_RO(ext_power);
 
 /* Interface Functions declaration */
 
@@ -150,12 +184,14 @@ static void w1_therm_remove_slave(struct w1_slave *sl);
 
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+   _attr_ext_power.attr,
NULL,
 };
 
 static struct attribute *w1_ds28ea00_attrs[] = {
_attr_w1_slave.attr,
_attr_w1_seq.attr,
+   _

[PATCH v6 3/9] w1_therm: adding sysfs-driver-w1_therm doc

2020-05-10 Thread Akira Shimahara
Adding a sysfs-driver-w1_therm documentation file in
Documentation/ABI/testing. It describe the onlys sysfs entry of w1_therm
module, based on Documentation/w1/slaves/w1_therm.rst

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements

 Documentation/ABI/testing/sysfs-driver-w1_therm | 17 +
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_therm

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
new file mode 100644
index 000..4a85663
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -0,0 +1,17 @@
+What:  /sys/bus/w1/devices/.../w1_slave
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) return the temperature in 1/1000 degC.
+   *read*: return 2 lines with the hexa output data sent on the
+   bus, return the CRC check and temperature in 1/1000 degC
+   *write* :
+   * '0' : save the 2 or 3 bytes to the device EEPROM
+   (i.e. TH, TL and config register)
+   * '9..12' : set the device resolution in RAM
+   (if supported)
+   * Anything else: do nothing
+   refer to Documentation/w1/slaves/w1_therm.rst for detailed
+   information.
+Users: any user space application which wants to communicate with
+   w1_term device
\ No newline at end of file
-- 
2.26.2



[PATCH v6 2/9] w1_therm: fix reset_select_slave during discovery

2020-05-10 Thread Akira Shimahara
Fix reset_select_slave issue during devices discovery by the master on
bus. The w1_reset_select_slave() from w1_io.c, which was previously used,
assume that if the slave count is 1 there is only one slave attached on
the bus. This is not always true. For example when discovering devices,
when the first device is discover by the bus master, its slave count is
1, but some other slaves may be on the bus.

In that case instead of adressing command to the attached slave the 
master throw a SKIP ROM command so that all slaves attached on the bus
will answer simultenaously causing data collision.

A dedicated reset_select_slave() function is implemented here,
it always perform an adressing to each slave using the MATCH ROM
command.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements

 drivers/w1/slaves/w1_therm.c | 48 ++--
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 890cf09..5de1a67 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -89,6 +90,24 @@ struct therm_info {
u8 verdict;
 };
 
+/* Hardware Functions declaration */
+
+/**
+ * reset_select_slave() - reset and select a slave
+ * @sl: the slave to select
+ *
+ * Resets the bus and select the slave by sending a ROM MATCH cmd
+ * w1_reset_select_slave() from w1_io.c could not be used here because
+ * it sent a SKIP ROM command if only one device is on the line.
+ * At the beginning of the such process, sl->master->slave_count is 1 even if
+ * more devices are on the line, causing collision on the line.
+ *
+ * Context: The w1 master lock must be held.
+ *
+ * Return: 0 if success, negative kernel error code otherwise.
+ */
+static int reset_select_slave(struct w1_slave *sl);
+
 /* Sysfs interface declaration */
 
 static ssize_t w1_slave_show(struct device *device,
@@ -300,7 +319,7 @@ static inline int w1_DS18B20_precision(struct device 
*device, int val)
while (max_trying--) {
crc = 0;
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
int count = 0;
 
/* read values to only alter precision bits */
@@ -313,7 +332,7 @@ static inline int w1_DS18B20_precision(struct device 
*device, int val)
if (rom[8] == crc) {
rom[4] = (rom[4] & ~mask) | (precision_bits & 
mask);
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
w1_write_8(dev, W1_WRITE_SCRATCHPAD);
w1_write_8(dev, rom[2]);
w1_write_8(dev, rom[3]);
@@ -435,6 +454,21 @@ static void w1_therm_remove_slave(struct w1_slave *sl)
 
 /* Hardware Functions */
 
+/* Safe version of reset_select_slave - avoid using the one in w_io.c */
+static int reset_select_slave(struct w1_slave *sl)
+{
+   u8 match[9] = { W1_MATCH_ROM, };
+   u64 rn = le64_to_cpu(*((u64 *)>reg_num));
+
+   if (w1_reset_bus(sl->master))
+   return -ENODEV;
+
+   memcpy([1], , 8);
+   w1_write_block(sl->master, match, 9);
+
+   return 0;
+}
+
 static ssize_t read_therm(struct device *device,
  struct w1_slave *sl, struct therm_info *info)
 {
@@ -462,7 +496,7 @@ static ssize_t read_therm(struct device *device,
info->verdict = 0;
info->crc = 0;
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
int count = 0;
unsigned int tm = 750;
unsigned long sleep_rem;
@@ -470,7 +504,7 @@ static ssize_t read_therm(struct device *device,
w1_write_8(dev, W1_READ_PSUPPLY);
external_power = w1_read_8(dev);
 
-   if (w1_reset_select_slave(sl))
+   if (reset_select_slave(sl))
continue;
 
/* 750ms strong pullup (or delay) after the convert */
@@ -500,7 +534,7 @@ static ssize_t read_therm(struct device *device,
}
}
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
 
w1_write_8(dev, W1_READ_SCRATCHPAD);
count = w1_read_block(dev, info->rom, 9);
@@ -552,7 +586,7 @@ static inline int w1_therm_eeprom(struct device *device)
memset(rom, 0, sizeof(ro

[PATCH v6 1/9] w1_therm: adding code comments and code reordering

2020-05-10 Thread Akira Shimahara
Adding code comments to split code in dedicated parts. After the global
declarations (defines, macros and function declarations), code is organized
as follow :
 - Device and family dependent structures and functions
 - Interfaces functions
 - Helpers functions
 - Hardware functions
 - Sysfs interface functions

Signed-off-by: Akira Shimahara 
---
Main motivation on the first patch of this serie is to clean up the code,
document it and reorder it to prepare the next patches, which are clearer
after this.

One main point is to keep all device/family dependent code gather at the
beginning to ease adding new devices if required.

Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include
Changes in v6:
- Formatting code comments according to kernel-doc requirements

 drivers/w1/slaves/w1_therm.c | 398 ---
 1 file changed, 232 insertions(+), 166 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 18f08d7..890cf09 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -41,42 +41,55 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/* Helpers Macros */
+
+/* return the address of the refcnt in the family data */
+#define THERM_REFCNT(family_data) \
+   (&((struct w1_therm_family_data *)family_data)->refcnt)
+
+/* Structs definition */
+
+/**
+ * struct w1_therm_family_converter - bind device specific functions
+ * @broken: flag for non registred families
+ * @reserved: not used here
+ * @f: pointer to the device binding structure
+ * @convert: pointer to the device conversion function
+ * @precision: pointer to the device precicion function
+ * @eeprom: pointer to eeprom function
+ */
+struct w1_therm_family_converter {
+   u8  broken;
+   u16 reserved;
+   struct w1_family*f;
+   int (*convert)(u8 rom[9]);
+   int (*precision)(struct device *device, int val);
+   int (*eeprom)(struct device *device);
+};
+
+/**
+ * struct w1_therm_family_data - device data
+ * @rom: ROM id of the device
+ * @refcnt: ref count
+ */
 struct w1_therm_family_data {
uint8_t rom[9];
atomic_t refcnt;
 };
 
+/**
+ * struct therm_info - store temperature reading
+ * @rom: readen device data
+ * @crc: computed crc from rom
+ * @verdict: 1 crc checked, 0 crc not matching
+ */
 struct therm_info {
u8 rom[9];
u8 crc;
u8 verdict;
 };
 
-/* return the address of the refcnt in the family data */
-#define THERM_REFCNT(family_data) \
-   (&((struct w1_therm_family_data *)family_data)->refcnt)
-
-static int w1_therm_add_slave(struct w1_slave *sl)
-{
-   sl->family_data = kzalloc(sizeof(struct w1_therm_family_data),
-   GFP_KERNEL);
-   if (!sl->family_data)
-   return -ENOMEM;
-   atomic_set(THERM_REFCNT(sl->family_data), 1);
-   return 0;
-}
-
-static void w1_therm_remove_slave(struct w1_slave *sl)
-{
-   int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data));
-
-   while (refcnt) {
-   msleep(1000);
-   refcnt = atomic_read(THERM_REFCNT(sl->family_data));
-   }
-   kfree(sl->family_data);
-   sl->family_data = NULL;
-}
+/* Sysfs interface declaration */
 
 static ssize_t w1_slave_show(struct device *device,
struct device_attribute *attr, char *buf);
@@ -87,9 +100,35 @@ static ssize_t w1_slave_store(struct device *device,
 static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
 
+/* Attributes declarations */
+
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
 
+/* Interface Functions declaration */
+
+/**
+ * w1_therm_add_slave() - Called when a new slave is discovered
+ * @sl: slave just discovered by the master.
+ *
+ * Called by the master when the slave is discovered on the bus.Used to
+ * initialized slave state before the beginning of any communication.
+ *
+ * Return: 0 - If success, negative kernel code otherwise
+ */
+static int w1_therm_add_slave(struct w1_slave *sl);
+
+/**
+ * w1_therm_remove_slave() - Called when a slave is removed
+ * @sl: slave to be removed.
+ *
+ * Called by the master when the slave is considered not to be on the bus
+ * anymore. Used to free memory.
+ */
+static void w1_therm_remove_slave(struct w1_slave *sl);
+
+/* Family attributes */
+
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
NULL,
@@ -101,6 +140,8 @@ static struct attribute *w1_ds28ea00_attrs[] = {
NULL,
 };
 
+/* Attribute groups */
+
 ATTRIBUTE_GROUPS(w1_therm);
 ATTRIBUTE_GROUPS(w1_ds28ea00);
 
@@ -154,6 +195,8 @@ static const struct hwmon_chip_info w1_chip_info = {
 #define W1_CHIPINFONULL
 #endif
 
+/* Family operations */
+
 static struct w1_family_ops w1_therm_fops = {
.

Re: [PATCH v5 1/9] w1_therm: adding code comments and code reordering

2020-05-10 Thread Akira shimahara
Le samedi 09 mai 2020 à 19:26 -0700, Randy Dunlap a écrit :
> > +
> > +/*--Structs---
> > --*/
> > +
> > +/**
> > + * struct w1_therm_family_converter
> > + * \brief Used to bind standard function call
> > + * to device specific function
> > + * it could be routed to NULL if device don't support feature
> > + */
> 
> 
> Hi,
> 
> 
> 
> All of the struct and function documentation comments in all patches
> in
> 
> this patch series should be using kernel-doc notation instead of the
> above
> 
> (whatever it is; I don't know what it is).
> 
> 
> 
> 
> 
> > +struct w1_therm_family_converter {
> > + u8  broken;
> > + u16 reserved;
> > + struct w1_family*f;
> > + int (*convert)(u8 rom[9]);
> > + int (*precision)(struct device *device, int val);
> > + int (*eeprom)(struct device *device);
> > +};
> > +
> 
> 
> thanks.
> 
> --
> 
> ~Randy

Hi,

Well noted, I will do it and re submit the serie. Thanks for your time.

Akira SHIMAHARA



[PATCH v5 9/9] w1_therm: adding bulk read support to trigger multiple conversion on bus

2020-05-09 Thread Akira Shimahara
Adding bulk read support:
Sending a 'trigger' command in the dedicated sysfs entry of bus master
device send a conversion command for all the slaves on the bus. The sysfs
entry is added as soon as at least one device supporting this feature
is detected on the bus.

The behavior of the sysfs reading temperature on the device is as follow:
 * If no bulk read pending, trigger a conversion on the device, wait for
 the conversion to be done, read the temperature in device RAM
 * If a bulk read has been trigger, access directly the device RAM
This behavior is the same on the 2 sysfs entries ('temperature' and
'w1_slave').

Reading the therm_bulk_read sysfs give the status of bulk operations:
 * '-1': conversion in progress on at least 1 sensor
 * '1': conversion complete but at least one sensor has not been read yet
 * '0': no bulk operation. Reading temperature on ecah device will trigger
a conversion

As not all devices support bulk read feature, it has been added in device
family structure.

The attribute is set at master level as soon as a supporting device is
discover. It is removed when the last supported device leave the bus.
The count of supported device is kept with the static counter
bulk_read_device_counter.

A strong pull up is apply on the line if at least one device required it.
The duration of the pull up is the max time required by a device on the
line, which depends on the resolution settings of each device. The strong
pull up could be adjust with the a module parameter.

Updating documentation in Documentation/ABI/testing/sysfs-driver-w1_therm
and Documentation/w1/slaves/w1_therm.rst accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include

 .../ABI/testing/sysfs-driver-w1_therm |  36 ++-
 Documentation/w1/slaves/w1_therm.rst  |  50 +++-
 drivers/w1/slaves/w1_therm.c  | 247 +-
 3 files changed, 318 insertions(+), 15 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index f289520..076659d 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -62,9 +62,16 @@ Date:May 2020
 Contact:   Akira Shimahara 
 Description:
(RO) return the temperature in 1/1000 degC.
-   Note that the conversion duration depend on the resolution (if
-   device support this feature). It takes 94ms in 9bits
-   resolution, 750ms for 12bits.
+   * If a bulk read has been triggered, it will directly
+   return the temperature computed when the bulk read
+   occurred, if available. If not yet available, nothing
+   is returned (a debug kernel message is sent), you
+   should retry later on.
+   * If no bulk read has been triggered, it will trigger
+   a conversion and send the result. Note that the
+   conversion duration depend on the resolution (if
+   device support this feature). It takes 94ms in 9bits
+   resolution, 750ms for 12bits.
 Users: any user space application which wants to communicate with
w1_term device
 
@@ -85,4 +92,25 @@ Description:
refer to Documentation/w1/slaves/w1_therm.rst for detailed
information.
 Users: any user space application which wants to communicate with
-   w1_term device
\ No newline at end of file
+   w1_term device
+
+
+What:  /sys/bus/w1/devices/w1_bus_masterXX/therm_bulk_read
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) trigger a bulk read conversion. read the status
+   *read*:
+   * '-1': conversion in progress on at least 1 sensor
+   * '1' : conversion complete but at least one sensor
+   value has not been read yet
+   * '0' : no bulk operation. Reading temperature will
+   trigger a conversion on each device
+   *write*: 'trigger': trigger a bulk read on all supporting
+   devices on the bus
+   Note that if a bulk read is sent but one sensor is not read
+   immediately, the next access to temperature on this device
+   will return the temperature measured at the time of issue
+   of the bulk read command (not the current temperature).
+Users: any user space application which wants to communicate with
+   w1_term device
diff --git a/Documentation/w1/slaves/w1_therm.rst 
b/Documentation/w1/slaves/w1_therm.rst
index 82e8ffe..7c42f00 100644
--- a/Documentation/w1/slaves/w1_therm.rst

[PATCH v5 7/9] w1_therm: optimizing temperature read timings

2020-05-09 Thread Akira Shimahara
Optimizing temperature reading by reducing waiting conversion time
according to device resolution settings, as per device specification.
This is device dependent as not all the devices supports resolution
setting, so it has been added in device family structures.

The process to read the temperature on the device has been adapted in a
new function 'convert_t()', which replace the former 'read_therm()', is
introduce to deal with this timing. Strong pull up is also applied during
the required time, according to device power status needs and
'strong_pullup' module parameter.

'temperature_from_RAM()' function is introduced to get the correct
temperature computation (device dependent) from device RAM data.

An new sysfs entry has been added to ouptut only temperature. The old
entry w1_slave has been kept for compatibility, without changing its
output format.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include

 .../ABI/testing/sysfs-driver-w1_therm |  12 +
 drivers/w1/slaves/w1_therm.c  | 278 --
 2 files changed, 195 insertions(+), 95 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 8b7ee89..6ffd3e3 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -41,6 +41,18 @@ Users:   any user space application which wants 
to communicate with
w1_term device
 
 
+What:  /sys/bus/w1/devices/.../temperature
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RO) return the temperature in 1/1000 degC.
+   Note that the conversion duration depend on the resolution (if
+   device support this feature). It takes 94ms in 9bits
+   resolution, 750ms for 12bits.
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index d3f83f8..2955cae 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -94,6 +94,7 @@ struct w1_therm_family_converter {
u16 reserved;
struct w1_family*f;
int (*convert)(u8 rom[9]);
+   int (*get_conversion_time)(struct w1_slave *sl);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
 };
@@ -144,6 +145,13 @@ struct therm_info {
  */
 static int reset_select_slave(struct w1_slave *sl);
 
+/** convert_t()
+ * \param sl pointer to the slave to read
+ * \param info pointer to a structure to store the read results
+ * \return 0 if success, -kernel error code otherwise
+ */
+static int convert_t(struct w1_slave *sl, struct therm_info *info);
+
 /** read_scratchpad()
  * \param sl pointer to the slave to read
  * \param info pointer to a structure to store the read results
@@ -201,6 +209,15 @@ static ssize_t w1_slave_store(struct device *device,
 static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
 
+/** \brief A callback function to output the temperature
+ *  Main differences with w1_slave :
+ * No hardware check (just read the stored device infos)
+ * No formatting
+ *  \return temperature (1/1000°)
+ */
+static ssize_t temperature_show(struct device *device,
+   struct device_attribute *attr, char *buf);
+
 /** \brief A callback function to output the power mode of the device
  * Once done, it is stored in the sl->family_data to avoid doing the test
  * during data read
@@ -235,6 +252,7 @@ static ssize_t eeprom_store(struct device *device,
 
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
+static DEVICE_ATTR_RO(temperature);
 static DEVICE_ATTR_RO(ext_power);
 static DEVICE_ATTR_RW(resolution);
 static DEVICE_ATTR_WO(eeprom);
@@ -258,6 +276,7 @@ static void w1_therm_remove_slave(struct w1_slave *sl);
 
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+   _attr_temperature.attr,
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
@@ -266,6 +285,7 @@ static struct attribute *w1_therm_attrs[] = {
 
 static struct attribute *w1_ds18s20_attrs[] = {
_attr_w1_slave.attr,
+   _attr_temperature.attr,
_attr_ext_power.attr,
_attr_eeprom.attr,
NULL,
@@ -274,6 +294,7 @@ static struct attribute *w1_ds18s20_attrs[] = {
 static struct attribute *w1_ds28ea00_attrs[] = {
_attr_w1_slave.attr,
_attr_w1_seq.attr,
+   _attr_temperature.attr,
_attr_ext_power.a

[PATCH v5 8/9] w1_therm: adding alarm sysfs entry

2020-05-09 Thread Akira Shimahara
Adding device alarms settings by a dedicated sysfs entry alarms (RW):
read or write TH and TL in the device RAM. Checking devices in alarm
state could be performed using the master search command.

As alarms temperature level are store in a 8 bit register on the device
and are signed values, a safe cast shall be performed using the min and
max temperature that device are able to measure. This is done by 
int_to_short inline function.

A 'write_data' field is added in the device structure, to bind the
correct writing function, as some devices may have 2 or 3 bytes RAM.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include

 .../ABI/testing/sysfs-driver-w1_therm |  16 ++
 drivers/w1/slaves/w1_therm.c  | 158 ++
 2 files changed, 174 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 6ffd3e3..f289520 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,19 @@
+What:  /sys/bus/w1/devices/.../alarms
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) read or write TH and TL (Temperature High an Low) alarms.
+   Values shall be space separated and in the device range
+   (typical -55 degC to 125 degC), if not values will be trimmed
+   to device min/max capabilities. Values are integer as they are
+   stored in a 8bit register in the device. Lowest value is
+   automatically put to TL. Once set, alarms could be search at
+   master level, refer to Documentation/w1/w1_generic.rst for
+   detailed information
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../eeprom
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 2955cae..cd65d0b 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -57,6 +57,9 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 #define EEPROM_CMD_WRITE"save" /* cmd for write eeprom sysfs */
 #define EEPROM_CMD_READ "restore"  /* cmd for read eeprom sysfs */
 
+#define MIN_TEMP   -55 /* min temperature that can be mesured */
+#define MAX_TEMP   125 /* max temperature that can be mesured */
+
 /*---Macros-*/
 
 /* return a pointer on the slave w1_therm_family_converter struct:
@@ -97,6 +100,7 @@ struct w1_therm_family_converter {
int (*get_conversion_time)(struct w1_slave *sl);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
+   int (*write_data)(struct w1_slave *sl, const u8 *data);
 };
 
 /**
@@ -248,6 +252,18 @@ static ssize_t resolution_store(struct device *device,
 static ssize_t eeprom_store(struct device *device,
struct device_attribute *attr, const char *buf, size_t size);
 
+/** \brief A callback function to set the alarms level
+ *  \param device represents the master device
+ */
+static ssize_t alarms_store(struct device *device,
+   struct device_attribute *attr, const char *buf, size_t size);
+
+/** \brief A callback function to get the alarms level
+ *  \return Low and High alarm, separate by one space
+ */
+static ssize_t alarms_show(struct device *device,
+   struct device_attribute *attr, char *buf);
+
 /*-Attributes declarations--*/
 
 static DEVICE_ATTR_RW(w1_slave);
@@ -256,6 +272,7 @@ static DEVICE_ATTR_RO(temperature);
 static DEVICE_ATTR_RO(ext_power);
 static DEVICE_ATTR_RW(resolution);
 static DEVICE_ATTR_WO(eeprom);
+static DEVICE_ATTR_RW(alarms);
 
 /*Interface Functions declaration---*/
 
@@ -280,6 +297,7 @@ static struct attribute *w1_therm_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -288,6 +306,7 @@ static struct attribute *w1_ds18s20_attrs[] = {
_attr_temperature.attr,
_attr_ext_power.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -298,6 +317,7 @@ static struct attribute *w1_ds28ea00_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -543,6 +563,7 @@ static struct w1_therm_family_converter w1_therm_families[] 
= {
.get_conversion_time= w1_DS18S20_convert_time,
   

[PATCH v5 6/9] w1_therm: adding eeprom sysfs entry

2020-05-09 Thread Akira Shimahara
Adding eeprom sysfs entry (WO) to trigger either device EEPROM save
(by writing 'save' in the sysfs), either device EEPROM restore (by writing
'restore' in the sysfs). All the RAM of the device will be save/restore,
whatever its size (actually 2 or 3 bytes): resolution config register and
alarms level will be save/restore.

The driver implement 2 hardware functions to access device RAM:
 * copy_scratchpad
 * recall_scratchpad
They act according to device specifications.

As EEPROM operations are not device dependent (all w1_therm can perform
EEPROM read/write operation following the same protocol), it is removed
from device families structures.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include

 .../ABI/testing/sysfs-driver-w1_therm |  14 ++
 drivers/w1/slaves/w1_therm.c  | 176 --
 2 files changed, 133 insertions(+), 57 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 7ed95e9..8b7ee89 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,17 @@
+What:  /sys/bus/w1/devices/.../eeprom
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (WO) writing that file will either trigger a save of the
+   device data to its embedded EEPROM, either restore data
+   embedded in device EEPROM. Be aware that devices support
+   limited EEPROM writing cycles (typical 50k)
+   * 'save': save device RAM to EEPROM
+   * 'restore': restore EEPROM data in device RAM
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../ext_power
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 159bfc1..d3f83f8 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -42,12 +42,21 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/* This command should be in public header w1.h but is not */
+#define W1_RECALL_EEPROM   0xB8
+
 /* Nb of try for an operation */
 #define W1_THERM_MAX_TRY   5
 
 /* ms delay to retry bus mutex */
 #define W1_THERM_RETRY_DELAY   20
 
+/* delay in ms to write in EEPROM */
+#define W1_THERM_EEPROM_WRITE_DELAY10
+
+#define EEPROM_CMD_WRITE"save" /* cmd for write eeprom sysfs */
+#define EEPROM_CMD_READ "restore"  /* cmd for read eeprom sysfs */
+
 /*---Macros-*/
 
 /* return a pointer on the slave w1_therm_family_converter struct:
@@ -87,7 +96,6 @@ struct w1_therm_family_converter {
int (*convert)(u8 rom[9]);
int (*set_resolution)(struct w1_slave *sl, int val);
int (*get_resolution)(struct w1_slave *sl);
-   int (*eeprom)(struct device *device);
 };
 
 /**
@@ -151,6 +159,19 @@ static int read_scratchpad(struct w1_slave *sl, struct 
therm_info *info);
  */
 static int write_scratchpad(struct w1_slave *sl, const u8 *data, u8 nb_bytes);
 
+/** copy_scratchpad() - Copy the content of scratchpad in device EEPROM
+ *  \param sl slave involved
+ *  \return 0 if success, -kernel error code otherwise
+ */
+static int copy_scratchpad(struct w1_slave *sl);
+
+/** recall_eeprom()
+ * \brief retrieve EEPROM data to device RAM
+ * \param sl slave involved
+ * \return 0 if success, -kernel error code otherwise
+ */
+static int recall_eeprom(struct w1_slave *sl);
+
 /** read_powermode()
  * \brief ask the device to get its power mode {external, parasite}
  * \param sl slave to be interrogated
@@ -204,12 +225,19 @@ static ssize_t resolution_show(struct device *device,
 static ssize_t resolution_store(struct device *device,
struct device_attribute *attr, const char *buf, size_t size);
 
+/** \brief A callback function to let the user read/write device EEPROM
+ *  \param check EEPROM_CMD_WRITE & EEPROM_CMD_READ macros
+ */
+static ssize_t eeprom_store(struct device *device,
+   struct device_attribute *attr, const char *buf, size_t size);
+
 /*-Attributes declarations--*/
 
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
 static DEVICE_ATTR_RO(ext_power);
 static DEVICE_ATTR_RW(resolution);
+static DEVICE_ATTR_WO(eeprom);
 
 /*Interface Functions declaration---*/
 
@@ -232,12 +260,14 @@ static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
_attr_ext_power.attr,
 

[PATCH v5 5/9] w1_therm: adding resolution sysfs entry

2020-05-09 Thread Akira Shimahara
Adding resolution sysfs entry (RW) to get or set the device resolution
Write values are managed as follow:
* '9..12': resolution to set in bit
* Anything else: do nothing
Read values are :
* '9..12': device resolution in bit
* '-xx': xx is kernel error when reading the resolution

Only supported devices will show the sysfs entry. A new family has been
created for DS18S20 devices as they do not implement resolution feature.

The resolution of each device is check when the device is
discover by the bus master, in 'w1_therm_add_slave(struct w1_slave *)'.
The status is stored in the device structure w1_therm_family_data so
that the driver always knows the resolution of each device, which could
be used later to determine the required conversion duration (resolution
dependent).

The resolution is re evaluate each time a user read or write the sysfs
entry.

To avoid looping through the w1_therm_families at run time, the pointer
'specific_functions' is set up to the correct 'w1_therm_family_converter'
when the slave is added (which mean when it is discovered by the master).
This initialization is done by a helper function 
'device_family(struct w1_slave *sl)', and a dedicated macro 
'SLAVE_SPECIFIC_FUNC(sl)' allow the access to the specific function of the
slave device.

'read_scratchpad' and 'write_scratchpad' are the hardware functions to
access the device RAM, as per protocol specification.

It cancel the former 'precision' functions, which was only set and never
read (so not stored in the device struct).

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include

 .../ABI/testing/sysfs-driver-w1_therm |  17 +
 drivers/w1/slaves/w1_therm.c  | 436 ++
 2 files changed, 356 insertions(+), 97 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 99d73ee..7ed95e9 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -10,6 +10,23 @@ Users:   any user space application which wants 
to communicate with
w1_term device
 
 
+What:  /sys/bus/w1/devices/.../resolution
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) get or set the device resolution (on supported devices,
+   if not, this entry is not present). Note that the resolution
+   will be changed only in device RAM, so it will be cleared when
+   power is lost. Trigger a 'save' to EEPROM command to keep
+   values after power-on. Read or write are :
+   * '9..12': device resolution in bit
+   or resolution to set in bit
+   * '-xx': xx is kernel error when reading the resolution
+   * Anything else: do nothing
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index ab15cca..159bfc1 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -50,12 +50,24 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
 /*---Macros-*/
 
+/* return a pointer on the slave w1_therm_family_converter struct:
+ * always test family data existence before
+ */
+#define SLAVE_SPECIFIC_FUNC(sl) \
+   (((struct w1_therm_family_data *)(sl->family_data))->specific_functions)
+
 /* return the power mode of the sl slave : 1-ext, 0-parasite, <0 unknown
  * always test family data existence before
  */
 #define SLAVE_POWERMODE(sl) \
(((struct w1_therm_family_data *)(sl->family_data))->external_powered)
 
+/* return the resolution in bit of the sl slave : <0 unknown
+ * always test family data existence before
+ */
+#define SLAVE_RESOLUTION(sl) \
+   (((struct w1_therm_family_data *)(sl->family_data))->resolution)
+
 /* return the address of the refcnt in the family data */
 #define THERM_REFCNT(family_data) \
(&((struct w1_therm_family_data *)family_data)->refcnt)
@@ -73,7 +85,8 @@ struct w1_therm_family_converter {
u16 reserved;
struct w1_family*f;
int (*convert)(u8 rom[9]);
-   int (*precision)(struct device *device, int val);
+   int (*set_resolution)(struct w1_slave *sl, int val);
+   int (*get_resolution)(struct w1_slave *sl);
int (*eeprom)(struct device *device);
 };
 
@@ -91,6 +104,8 @@ struct w1_therm_fami

[PATCH v5 4/9] w1_therm: adding ext_power sysfs entry

2020-05-09 Thread Akira Shimahara
Adding ext_power sysfs entry (RO). Return the power status of the device:
 - 0: device parasite powered
 - 1: device externally powered
 - xx: xx is kernel error

The power status of each device is check when the device is
discover by the bus master, in 'w1_therm_add_slave(struct w1_slave *)'.
The status is stored in the device structure w1_therm_family_data so
that the driver always knows the power state of each device, which could
be used later to determine the required strong pull up to apply on the
line.

The power status is re evaluate each time the sysfs ext_power read by
a user.

The hardware function 'read_powermode(struct w1_slave *sl)' act just as
per device specifications, sending W1_READ_PSUPPLY command on the bus,
and issue a read time slot, reading only one bit.

A helper function 'bool bus_mutex_lock(struct mutex *lock)' is introduced.
It try to aquire the bus mutex several times (W1_THERM_MAX_TRY), waiting
W1_THERM_RETRY_DELAY between two attempt.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include

 .../ABI/testing/sysfs-driver-w1_therm |  12 ++
 drivers/w1/slaves/w1_therm.c  | 129 ++
 2 files changed, 141 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 4a85663..99d73ee 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,15 @@
+What:  /sys/bus/w1/devices/.../ext_power
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RO) return the power status by asking the device
+   * '0': device parasite powered
+   * '1': device externally powered
+   * '-xx': xx is kernel error when reading power status
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  May 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 4dd139b..ab15cca 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -42,8 +42,20 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/* Nb of try for an operation */
+#define W1_THERM_MAX_TRY   5
+
+/* ms delay to retry bus mutex */
+#define W1_THERM_RETRY_DELAY   20
+
 /*---Macros-*/
 
+/* return the power mode of the sl slave : 1-ext, 0-parasite, <0 unknown
+ * always test family data existence before
+ */
+#define SLAVE_POWERMODE(sl) \
+   (((struct w1_therm_family_data *)(sl->family_data))->external_powered)
+
 /* return the address of the refcnt in the family data */
 #define THERM_REFCNT(family_data) \
(&((struct w1_therm_family_data *)family_data)->refcnt)
@@ -78,6 +90,7 @@ struct w1_therm_family_converter {
 struct w1_therm_family_data {
uint8_t rom[9];
atomic_t refcnt;
+   int external_powered;
 };
 
 /**
@@ -108,6 +121,15 @@ struct therm_info {
  */
 static int reset_select_slave(struct w1_slave *sl);
 
+/** read_powermode()
+ * \brief ask the device to get its power mode {external, parasite}
+ * \param sl slave to be interrogated
+ * \return 0 parasite powered device
+ * 1 externally powered device
+ * <0 kernel error code
+ */
+static int read_powermode(struct w1_slave *sl);
+
 /*---Interface sysfs declaration*/
 
 /** \brief A callback function to output the temperature Old way
@@ -128,10 +150,21 @@ static ssize_t w1_slave_store(struct device *device,
 static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
 
+/** \brief A callback function to output the power mode of the device
+ * Once done, it is stored in the sl->family_data to avoid doing the test
+ * during data read
+ *  \return0 : device parasite powered
+ * 1 : device externally powered
+ * -xx : xx is kernel error code
+ */
+static ssize_t ext_power_show(struct device *device,
+   struct device_attribute *attr, char *buf);
+
 /*-Attributes declarations--*/
 
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
+static DEVICE_ATTR_RO(ext_power);
 
 /*Interface Functions declaration---*/
 
@@ -152,12 +185,14 @@ static void w1_therm_remove_slave(struct w1_slave *sl);
 
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+   _attr_ext_power.attr,
NULL,
 };
 
 s

[PATCH v5 3/9] w1_therm: adding sysfs-driver-w1_therm doc

2020-05-09 Thread Akira Shimahara
Adding a sysfs-driver-w1_therm documentation file in
Documentation/ABI/testing. It describe the onlys sysfs entry of w1_therm
module, based on Documentation/w1/slaves/w1_therm.rst

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include

 Documentation/ABI/testing/sysfs-driver-w1_therm | 17 +
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_therm

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
new file mode 100644
index 000..4a85663
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -0,0 +1,17 @@
+What:  /sys/bus/w1/devices/.../w1_slave
+Date:  May 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) return the temperature in 1/1000 degC.
+   *read*: return 2 lines with the hexa output data sent on the
+   bus, return the CRC check and temperature in 1/1000 degC
+   *write* :
+   * '0' : save the 2 or 3 bytes to the device EEPROM
+   (i.e. TH, TL and config register)
+   * '9..12' : set the device resolution in RAM
+   (if supported)
+   * Anything else: do nothing
+   refer to Documentation/w1/slaves/w1_therm.rst for detailed
+   information.
+Users: any user space application which wants to communicate with
+   w1_term device
\ No newline at end of file
-- 
2.26.2



[PATCH v5 2/9] w1_therm: fix reset_select_slave during discovery

2020-05-09 Thread Akira Shimahara
Fix reset_select_slave issue during devices discovery by the master on
bus. The w1_reset_select_slave() from w1_io.c, which was previously used,
assume that if the slave count is 1 there is only one slave attached on
the bus. This is not always true. For example when discovering devices,
when the first device is discover by the bus master, its slave count is
1, but some other slaves may be on the bus.

In that case instead of adressing command to the attached slave the 
master throw a SKIP ROM command so that all slaves attached on the bus
will answer simultenaously causing data collision.

A dedicated reset_select_slave() function is implemented here,
it always perform an adressing to each slave using the MATCH ROM
command.

Signed-off-by: Akira Shimahara 
---
Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include

 drivers/w1/slaves/w1_therm.c | 45 ++--
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index f7147b2..4dd139b 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -92,6 +93,21 @@ struct therm_info {
u8 verdict;
 };
 
+/*-Hardware Functions declaration---*/
+
+/**
+ * reset_select_slave() - reset and select a slave
+ * \brief Resets the bus and select the slave by sending a ROM MATCH cmd
+ * w1_reset_select_slave() from w1_io.c could not be used here because
+ * it sent a SKIP ROM command if only one device is on the line.
+ * At the beginning of the such process, sl->master->slave_count is 1 even if
+ * more devices are on the line, causing collision on the line.
+ * The w1 master lock must be held.
+ * \param sl the slave to select
+ * \return 0 if success, negative kernel error code otherwise
+ */
+static int reset_select_slave(struct w1_slave *sl);
+
 /*---Interface sysfs declaration*/
 
 /** \brief A callback function to output the temperature Old way
@@ -305,7 +321,7 @@ static inline int w1_DS18B20_precision(struct device 
*device, int val)
while (max_trying--) {
crc = 0;
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
int count = 0;
 
/* read values to only alter precision bits */
@@ -318,7 +334,7 @@ static inline int w1_DS18B20_precision(struct device 
*device, int val)
if (rom[8] == crc) {
rom[4] = (rom[4] & ~mask) | (precision_bits & 
mask);
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
w1_write_8(dev, W1_WRITE_SCRATCHPAD);
w1_write_8(dev, rom[2]);
w1_write_8(dev, rom[3]);
@@ -440,6 +456,21 @@ static void w1_therm_remove_slave(struct w1_slave *sl)
 
 /*---Hardware Functions-*/
 
+/* Safe version of reser_select_slave - avoid using the one in w_io.c */
+static int reset_select_slave(struct w1_slave *sl)
+{
+   u8 match[9] = { W1_MATCH_ROM, };
+   u64 rn = le64_to_cpu(*((u64 *)>reg_num));
+
+   if (w1_reset_bus(sl->master))
+   return -ENODEV;
+
+   memcpy([1], , 8);
+   w1_write_block(sl->master, match, 9);
+
+   return 0;
+}
+
 static ssize_t read_therm(struct device *device,
  struct w1_slave *sl, struct therm_info *info)
 {
@@ -467,7 +498,7 @@ static ssize_t read_therm(struct device *device,
info->verdict = 0;
info->crc = 0;
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
int count = 0;
unsigned int tm = 750;
unsigned long sleep_rem;
@@ -475,7 +506,7 @@ static ssize_t read_therm(struct device *device,
w1_write_8(dev, W1_READ_PSUPPLY);
external_power = w1_read_8(dev);
 
-   if (w1_reset_select_slave(sl))
+   if (reset_select_slave(sl))
continue;
 
/* 750ms strong pullup (or delay) after the convert */
@@ -505,7 +536,7 @@ static ssize_t read_therm(struct device *device,
}
}
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
 
w1_write_8(dev, W1_READ_SCRATCHPAD);
count = w1_read_block(dev, info->rom, 9);
@@ -557,7 +588,7 @@ static inline int w1_th

[PATCH v5 1/9] w1_therm: adding code comments and code reordering

2020-05-09 Thread Akira Shimahara
Adding code comments to split code in dedicated parts. After the global
declarations (defines, macros and function declarations), code is organized
as follow :
 - Device and family dependent structures and functions
 - Interfaces functions
 - Helpers functions
 - Hardware functions
 - Sysfs interface functions

Signed-off-by: Akira Shimahara 
---
Main motivation on the first patch of this serie is to clean up the code,
document it and reorder it to prepare the next patches, which are clearer
after this.

One main point is to keep all device/family dependent code gather at the
beginning to ease adding new devices if required.

Changes in v5:
- All patch serie in one .c file
- Correcting some comments
- adding  include

 drivers/w1/slaves/w1_therm.c | 403 ---
 1 file changed, 237 insertions(+), 166 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 18f08d7..f7147b2 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -41,55 +41,99 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/*---Macros-*/
+
+/* return the address of the refcnt in the family data */
+#define THERM_REFCNT(family_data) \
+   (&((struct w1_therm_family_data *)family_data)->refcnt)
+
+/*--Structs-*/
+
+/**
+ * struct w1_therm_family_converter
+ * \brief Used to bind standard function call
+ * to device specific function
+ * it could be routed to NULL if device don't support feature
+ */
+struct w1_therm_family_converter {
+   u8  broken;
+   u16 reserved;
+   struct w1_family*f;
+   int (*convert)(u8 rom[9]);
+   int (*precision)(struct device *device, int val);
+   int (*eeprom)(struct device *device);
+};
+
+/**
+ * struct w1_therm_family_data
+ * \param rom data
+ * \param refcnt ref count
+ * \param external_powered
+ * 1 device powered externally,
+ * 0 device parasite powered,
+ * -x error or undefined
+ * \param resolution resolution in bit of the device, refcnt)
-
-static int w1_therm_add_slave(struct w1_slave *sl)
-{
-   sl->family_data = kzalloc(sizeof(struct w1_therm_family_data),
-   GFP_KERNEL);
-   if (!sl->family_data)
-   return -ENOMEM;
-   atomic_set(THERM_REFCNT(sl->family_data), 1);
-   return 0;
-}
-
-static void w1_therm_remove_slave(struct w1_slave *sl)
-{
-   int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data));
-
-   while (refcnt) {
-   msleep(1000);
-   refcnt = atomic_read(THERM_REFCNT(sl->family_data));
-   }
-   kfree(sl->family_data);
-   sl->family_data = NULL;
-}
+/*---Interface sysfs declaration*/
 
+/** \brief A callback function to output the temperature Old way
+ *  read temperature and return the result in the sys file
+ *  This has been kept for compatibility
+ */
 static ssize_t w1_slave_show(struct device *device,
struct device_attribute *attr, char *buf);
 
+/** \brief A callback function to set the resolution Old way
+ *  This has been kept for compatibility
+ *  \param 0, it write config in the EEPROM
+ *  \param 9..12, it set the resolution in the RAM
+ */
 static ssize_t w1_slave_store(struct device *device,
struct device_attribute *attr, const char *buf, size_t size);
 
 static ssize_t w1_seq_show(struct device *device,
struct device_attribute *attr, char *buf);
 
+/*-Attributes declarations--*/
+
 static DEVICE_ATTR_RW(w1_slave);
 static DEVICE_ATTR_RO(w1_seq);
 
+/*Interface Functions declaration---*/
+
+/** w1_therm_add_slave() - Called each time a search discover a new device
+ * \brief used to initialized slave (family datas)
+ * \param sl slave just discovered
+ * \return 0 - If success, negative kernel code otherwise
+ */
+static int w1_therm_add_slave(struct w1_slave *sl);
+
+/** w1_therm_remove_slave() - Called each time a slave is removed
+ * \brief used to free memory
+ * \param sl slave to be removed
+ */
+static void w1_therm_remove_slave(struct w1_slave *sl);
+
+/*--Family attributes---*/
+
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
NULL,
@@ -101,6 +145,8 @@ static struct attribute *w1_ds28ea00_attrs[] = {
NULL,
 };
 
+/*--Attribute groups*/
+
 ATTRIBUTE_GROUPS(w1_therm);
 ATTRIBUTE_GROUPS(w1_ds28ea00);
 
@@ -154,6 +200,8 @@ static const struct hwmon_chip_info w1_chip_info = {
 #define W1_CHIPINFONULL
 #endif
 
+/*---

Re: [PATCH v4 1/9] w1_therm: creating w1_therm.h

2020-05-06 Thread Akira shimahara
Le mercredi 06 mai 2020 à 08:51 +0200, Greg KH a écrit :
> > > Why is this function needed to be declared in this .h file?
> > > Why is any of this needed?  For some reason I thought you needed
> > > a .h
> > > file to make things simpler for other .c files, but if all of
> > > this is
> > > static, it's not needed at all, right?
> > > thanks,
> > > greg k-h
> > Hello,
> > Yes, you are right, header file could be avoided. But we separate
> > it
> > from .c for clarity purpose, and to ease future developpment (for
> > example adding support of new devices).
> > If you absolutely want to put everything in the .c file, I can do
> > it,
> > let me know.
> 
> 
> Keep it all in a .c file, only use .h files for when you need to
> share
> 
> it across multiple .c files, otherwise it's not needed.
> 
> 
> 
> thanks,
> 
> 
> 
> greg k-h

Hi, 

Ok well noted, I will do it tomorrow, and keep it as 8 patches.

Thanks for your time

Akira Shimahara



Re: [PATCH v4 2/9] w1_therm: fix reset_select_slave at beginning of search process

2020-05-05 Thread Akira shimahara
Le mardi 05 mai 2020 à 16:49 +0200, Greg KH a écrit :
> >   /*Hardware Functions-
> > -*/
> >   
> > +/* Safe version of reser_select_slave - avoid using the one in
> > w_io.c */
> > +static int reset_select_slave(struct w1_slave *sl)
> > +{
> > + u8 match[9] = { W1_MATCH_ROM, };
> > + u64 rn = le64_to_cpu(*((u64 *)>reg_num));
> > +
> > + if (w1_reset_bus(sl->master))
> > + return -ENODEV;
> > +
> > + memcpy([1], , 8);
> > + w1_write_block(sl->master, match, 9);
> > +
> > + return 0;
> > +}
> 
> 
> If you put this higher up in the .c file, no function definition is
> 
> needed in the .h file at all, right?
> 
> 
> 
> thanks,
> 
> 
> 
> greg k-h

Yes, everything could be put in the .c file, but I think we will loose
clarity.
Please, let me know what you prefer.

Thanks

Akira Shimahara



Re: [PATCH v4 1/9] w1_therm: creating w1_therm.h

2020-05-05 Thread Akira shimahara
Le mardi 05 mai 2020 à 16:48 +0200, Greg KH a écrit :
> > Creating w1_therm.h header to organize code. Organize the
> > w1_therm.c file
> > to gather hardware functions, device specific functions, interface
> > functions and sysfs functions.
> > Signed-off-by: Akira Shimahara 
> > ---
> >   drivers/w1/slaves/w1_therm.c | 302 +++---
> > -
> >   drivers/w1/slaves/w1_therm.h | 138 
> >   2 files changed, 269 insertions(+), 171 deletions(-)
> >   create mode 100644 drivers/w1/slaves/w1_therm.h
> 
> 
> Wait, why is a .h file needed for just a single .c file?
> 
> 
> 
> 
> 
> 
> 
> 
> 
> >   static ssize_t read_therm(struct device *device,
> 
> 
> 
> 
> > +/** read_therm()
> > + * @param sl pointer to the slave to read
> > + * @param info pointer to a structure to store the read results
> > + * @return 0 if success, -kernel error code otherwise
> > + */
> > +static ssize_t read_therm(struct device *device,
> > + struct w1_slave *sl, struct therm_info
> > *info);
> > +
> 
> 
> Why is this function needed to be declared in this .h file?
> 
> 
> 
> Why is any of this needed?  For some reason I thought you needed a .h
> 
> file to make things simpler for other .c files, but if all of this is
> 
> static, it's not needed at all, right?
> 
> 
> 
> thanks,
> 
> 
> 
> greg k-h

Hello,

Yes, you are right, header file could be avoided. But we separate it
from .c for clarity purpose, and to ease future developpment (for
example adding support of new devices).

If you absolutely want to put everything in the .c file, I can do it,
let me know.

Thanks ahead,

Akira Shimahara



Re: [PATCH v3 2/5] w1_therm: adding sysfs entry to check device power

2020-04-30 Thread Akira shimahara
Le jeudi 30 avril 2020 à 13:21 +0200, Greg KH a écrit :
> On Thu, Apr 30, 2020 at 12:34:03PM +0200, Akira shimahara wrote:
> > Hello,
> > 
> > Le mercredi 29 avril 2020 à 18:18 +0300, Evgeniy Polyakov a écrit :
> > > Hi
> > > 
> > > 
> > > 
> > > 29.04.2020, 16:47, "Greg KH" :
> > > 
> > > 
> > > 
> > > > >  +What: /sys/bus/w1/devices/.../w1_slave
> > > > >  +Date: Apr 2020
> > > > >  +Contact: Akira Shimahara 
> > > > >  +Description:
> > > > >  + (RW) return the temperature in 1/1000 degC.
> > > > >  + *read*: return 2 lines with the hexa output data sent on
> > > > > the
> > > > >  + bus, return the CRC check and temperature in 1/1000 degC
> > > > the w1_slave file returns a temperature???
> > > > And sysfs is 1 value-per-file, not multiple lines.
> > > 
> > > It was 'content crc' previously, and probably a good idea would
> > > be to
> > > add just one file with 'content'
> >  
> > That's the purpose of the new sysfs entry 'temperature'. It only
> > content temperature. As already mentionned we have to keep the
> > w1_slave
> > entry for compatibility purpose, all existing user application
> > parse
> > this file.
> 
> That's fine, but the document you wrote here says the file is called
> "w1_slave", not "temperature" :)
> 

Yes because it is the patch 2/5. At this stage, I just create the
sysfs-driver-w1_therm doc for the old w1_slave sysfs entry. The
temperature  sysfs entry is introduce in v3 patch 3/5 (and v4 patch
7/9)and the sysfs-driver-w1_therm is updated accordingly.

Akira Shimahara



Re: [PATCH v3 2/5] w1_therm: adding sysfs entry to check device power

2020-04-30 Thread Akira shimahara
Hello,

Le mercredi 29 avril 2020 à 18:18 +0300, Evgeniy Polyakov a écrit :
> Hi
> 
> 
> 
> 29.04.2020, 16:47, "Greg KH" :
> 
> 
> 
> > >  +What: /sys/bus/w1/devices/.../w1_slave
> > >  +Date: Apr 2020
> > >  +Contact: Akira Shimahara 
> > >  +Description:
> > >  + (RW) return the temperature in 1/1000 degC.
> > >  + *read*: return 2 lines with the hexa output data sent on the
> > >  + bus, return the CRC check and temperature in 1/1000 degC
> > the w1_slave file returns a temperature???
> > And sysfs is 1 value-per-file, not multiple lines.
> 
> 
> It was 'content crc' previously, and probably a good idea would be to
> add just one file with 'content'
 
That's the purpose of the new sysfs entry 'temperature'. It only
content temperature. As already mentionned we have to keep the w1_slave
entry for compatibility purpose, all existing user application parse
this file.

I submitted the patch series yesterday night, splitting it in 9
patches.

Regards,

Akira Shimahara



[PATCH v4 9/9] w1_therm: adding bulk read support to trigger multiple conversion on the bus

2020-04-29 Thread Akira Shimahara
Patch for enhacement of w1_therm module. Adding bulk read support.
Sending a 'trigger' command in the dedicated sysfs entry of bus master
device send a conversion command for all the slaves on the bus. The sysfs
entry is added as soon as at least one device supporting this feature
is detected on the bus.

The behavior of the sysfs reading temperature on the device is as follow:
 * If no bulk read pending, trigger a conversion on the device, wait for
 the conversion to be done, read the temperature in device RAM
* If a bulk read has been trigger, access directly the device RAM

Reading the therm_bulk_read sysfs give the status of bulk operations:
 * `-1`: conversion in progress on at least 1 sensor
 * `1`: conversion complete but at least one sensor has not been read yet
 * `0`: no bulk operation. Reading temperature on ecah device will trigger
a conversion

As not all devices support bulk read feature, it has been added in device
family structure.

The attribute is set at master level as soon as a supporting device is
discover. It is removed when the last supported device leave the bus.
The count of supported device is kept with the static counter
bulk_read_device_counter.

A strong pull up is apply on the line if at least one device required it.
The duration of the pull up is the max time required by a device on the
line, which depends on the resolution settings of each device. The strong
pull up could be adjust with the a module parameter.

Updating documentation in Documentation/ABI/testing/sysfs-driver-w1_therm
and Documentation/w1/slaves/w1_therm.rst accordingly.

Signed-off-by: Akira Shimahara 
---
 .../ABI/testing/sysfs-driver-w1_therm |  36 ++-
 Documentation/w1/slaves/w1_therm.rst  |  50 +++-
 drivers/w1/slaves/w1_therm.c  | 214 +-
 drivers/w1/slaves/w1_therm.h  |  46 
 4 files changed, 331 insertions(+), 15 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 39488a4..1f911ed 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -61,9 +61,16 @@ Date:Apr 2020
 Contact:   Akira Shimahara 
 Description:
(RO) return the temperature in 1/1000 degC.
-   Note that the conversion duration depend on the resolution (if
-   device support this feature). It takes 94ms in 9bits
-   resolution, 750ms for 12bits.
+   * If a bulk read has been triggered, it will directly
+   return the temperature computed when the bulk read
+   occurred, if available. If not yet available, nothing
+   is returned (a debug kernel message is sent), you
+   should retry later on.
+   * If no bulk read has been triggered, it will trigger
+   a conversion and send the result. Note that the
+   conversion duration depend on the resolution (if
+   device support this feature). It takes 94ms in 9bits
+   resolution, 750ms for 12bits.
 Users: any user space application which wants to communicate with
w1_term device
 
@@ -84,4 +91,25 @@ Description:
refer to Documentation/w1/slaves/w1_therm.rst for detailed
information.
 Users: any user space application which wants to communicate with
-   w1_term device
\ No newline at end of file
+   w1_term device
+
+
+What:  /sys/bus/w1/devices/w1_bus_masterXX/therm_bulk_read
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) trigger a bulk read conversion. read the status
+   *read*:
+   * `-1`: conversion in progress on at least 1 sensor
+   * `1` : conversion complete but at least one sensor
+   value has not been read yet
+   * `0` : no bulk operation. Reading temperature will
+   trigger a conversion on each device
+   *write*: `trigger`: trigger a bulk read on all supporting
+   devices on the bus
+   Note that if a bulk read is sent but one sensor is not read
+   immediately, the next access to temperature on this device
+   will return the temperature measured at the time of issue
+   of the bulk read command (not the current temperature).
+Users: any user space application which wants to communicate with
+   w1_term device
diff --git a/Documentation/w1/slaves/w1_therm.rst 
b/Documentation/w1/slaves/w1_therm.rst
index 82e8ffe..06eaff1 100644
--- a/Documentation/w1/slaves/w1_therm.rst
+++ b/Documentation/w1/slaves/w1_therm.rst
@@ -26,20 +26,31

[PATCH v4 8/9] w1_therm: adding alarm sysfs entry to set device internal alarms values

2020-04-29 Thread Akira Shimahara
Adding device alarms settings by a dedicated sysfs entry alarms (RW):
read or write TH and TL in the device RAM. Checking devices in alarm
state could be performed using the master search command.

As alarms temperature level are store in a 8 bit register on the device
and are signed values, a safe cast shall be performed using the min and
max temperature that device are able to measure. This is done by 
int_to_short inline function.

Updating doc in Documentation/ABI/testing/sysfs-driver-w1_therm
accordingly.

Signed-off-by: Akira Shimahara 
---
 .../ABI/testing/sysfs-driver-w1_therm |  15 ++
 drivers/w1/slaves/w1_therm.c  | 138 ++
 drivers/w1/slaves/w1_therm.h  |  22 +++
 3 files changed, 175 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 279e13d..39488a4 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,18 @@
+What:  /sys/bus/w1/devices/.../alarms
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) read or write TH and TL (Temperature High an Low) alarms.
+   Values shall be space separated and in the device range
+   (typical -55 degC to 125 degC). Values are integer as they
+   are store in a 8bit register in the device.
+   Lowest value is automatically put to TL.
+   Once set, alarms could be search at master level, refer to
+   Documentation/w1/w1_generic.rst for detailed information
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../eeprom
 Date:  Apr 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 2c89d9f..90217b1 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -68,6 +68,12 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * (device do that automatically on power-up)
  *
  *
+ * alarms (RW) : read TH and TL (Temperature High an Low) alarms
+ * Values shall be space separated and in the device range
+ * (typically -55° to 125°)
+ * Values are integer are they are store in a 8bit field in the device
+ * Lowest value is automatically put to TL
+ *
  */
 
 /*
@@ -81,6 +87,7 @@ static struct attribute *w1_therm_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -89,6 +96,7 @@ static struct attribute *w1_ds18s20_attrs[] = {
_attr_temperature.attr,
_attr_ext_power.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -99,6 +107,7 @@ static struct attribute *w1_ds28ea00_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -206,6 +215,7 @@ static struct w1_family w1_therm_family_DS1825 = {
 };
 
 /*--Device capability description---*/
+
 static struct w1_therm_family_converter w1_therm_families[] = {
{
.f  = _therm_family_DS18S20,
@@ -431,6 +441,14 @@ static inline int temperature_from_RAM(struct w1_slave 
*sl, u8 rom[9])
return 0;  /* No device family */
 }
 
+static inline s8 int_to_short(int i)
+{
+   /* Prepare to cast to short by eliminating out of range values */
+   i = i > MAX_TEMP ? MAX_TEMP : i;
+   i = i < MIN_TEMP ? MIN_TEMP : i;
+   return (s8) i;
+}
+
 /*-Interface Functions--*/
 
 static int w1_therm_add_slave(struct w1_slave *sl)
@@ -1000,6 +1018,126 @@ static ssize_t eeprom_store(struct device *device,
return size;
 }
 
+static ssize_t alarms_show(struct device *device,
+   struct device_attribute *attr, char *buf)
+{
+   struct w1_slave *sl = dev_to_w1_slave(device);
+   int ret = -ENODEV;
+   s8 th = 0, tl = 0;
+   struct therm_info scratchpad;
+
+   ret = read_scratchpad(sl, );
+
+   if (!ret)   {
+   th = scratchpad.rom[2]; // TH is byte 2
+   tl = scratchpad.rom[3]; // TL is byte 3
+   } else {
+   dev_info(device,
+   "%s: error reading alarms register %d\n",
+   __func__, ret);
+   }
+
+   return sprintf(buf, "%hd %hd\n", tl, th);
+}
+
+static ssize_t alarms_store(struct device *device,
+   struct device_attribute *attr, const char *buf, size_t size)
+{
+   struct w1_slave *sl = dev_to_w1_slave(device);
+   struct therm_info info;
+   u8 new_config_register[3];  /* array of data to be written */
+   int temp, ret = -E

[PATCH v4 6/9] w1_therm: adding eeprom sysfs entry

2020-04-29 Thread Akira Shimahara
Adding eeprom sysfs entry (WO) to trigger either device EEPROM save
(by writing 'save' in the sysfs) either device EEPROM restore (by writing
'restore' in the sysfs). All the RAM of the device will be save/restore,
whatever its size : resolution config register and alarms level will be
save/restore.

The driver implement 2 basics functions to access device RAM:
 * copy_scratchpad
 * recall_scratchpad
They act according to device specifications.

As EEPROM operations are not device dependent (all w1_therm can perform
EEPROM read/write operation following the same protocol), it is removed
from device families structures.

Signed-off-by: Akira Shimahara 
---
 .../ABI/testing/sysfs-driver-w1_therm |  14 ++
 drivers/w1/slaves/w1_therm.c  | 151 +++---
 drivers/w1/slaves/w1_therm.h  |  36 -
 3 files changed, 142 insertions(+), 59 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index e52dd6a..9dcbbed 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,17 @@
+What:  /sys/bus/w1/devices/.../eeprom
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (WO) writing that file will either trigger a save of the
+   device data to its embedded EEPROM, either restore data
+   embedded in device EEPROM. Be aware that devices support
+   limited EEPROM writing cycles (typical 50k)
+   * `save`: save device RAM to EEPROM
+   * `restore`: restore EEPROM data in device RAM
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../ext_power
 Date:  Apr 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 382d679..caa4615 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -57,6 +57,13 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * resolution (RW):
  * . -xx   : xx is kernel error
  * . 9..12 : resolution set in bit (or resolution to set in bit)
+ *
+ * eeprom (WO): be aware that eeprom writing cycles count is limited
+ * . 'save':   save device RAM to EEPROM
+ * . 'restore' :   restore EEPROM data in device RAM
+ * (device do that automatically on power-up)
+ *
+ *
  */
 
 /*
@@ -68,12 +75,14 @@ static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
_attr_ext_power.attr,
_attr_resolution.attr,
+   _attr_eeprom.attr,
NULL,
 };
 
 static struct attribute *w1_ds18s20_attrs[] = {
_attr_w1_slave.attr,
_attr_ext_power.attr,
+   _attr_eeprom.attr,
NULL,
 };
 static struct attribute *w1_ds28ea00_attrs[] = {
@@ -81,6 +90,7 @@ static struct attribute *w1_ds28ea00_attrs[] = {
_attr_w1_seq.attr,
_attr_ext_power.attr,
_attr_resolution.attr,
+   _attr_eeprom.attr,
NULL,
 };
 
@@ -89,7 +99,6 @@ ATTRIBUTE_GROUPS(w1_therm);
 ATTRIBUTE_GROUPS(w1_ds18s20);
 ATTRIBUTE_GROUPS(w1_ds28ea00);
 
-
 #if IS_REACHABLE(CONFIG_HWMON)
 static int w1_read_temp(struct device *dev, u32 attr, int channel,
long *val);
@@ -196,7 +205,6 @@ static struct w1_therm_family_converter w1_therm_families[] 
= {
.set_resolution = NULL, // no config register
.get_resolution = NULL, // no config register
.write_data = w1_DS18S20_write_data,
-   .eeprom = w1_therm_eeprom
},
{
.f  = _therm_family_DS1822,
@@ -204,7 +212,6 @@ static struct w1_therm_family_converter w1_therm_families[] 
= {
.set_resolution = w1_DS18B20_set_resolution,
.get_resolution = w1_DS18B20_get_resolution,
.write_data = w1_DS18B20_write_data,
-   .eeprom = w1_therm_eeprom
},
{
.f  = _therm_family_DS18B20,
@@ -212,7 +219,6 @@ static struct w1_therm_family_converter w1_therm_families[] 
= {
.set_resolution = w1_DS18B20_set_resolution,
.get_resolution = w1_DS18B20_get_resolution,
.write_data = w1_DS18B20_write_data,
-   .eeprom = w1_therm_eeprom
},
{
.f  = _therm_family_DS28EA00,
@@ -220,7 +226,6 @@ static struct w1_therm_family_converter w1_therm_families[] 
= {
.set_resolution = w1_DS18B20_set_resolution,
.get_resolution = w1_DS18B20_get_resolution,
.write_data

[PATCH v4 7/9] w1_therm: optimizing temperature reading timings

2020-04-29 Thread Akira Shimahara
Optimizing temperature reading by reducing waiting conversion time
according to device resolution settings, as per device specification.
This is device dependent as not all the devices supports resolution
setting, so it has been added in device family structures.

The process to read the temperature on the device has been adapted in
a new function convert_t() to deal with this timing. Strong pull up is
also applied during the required time, according to device power status
needs and strong_pullup module parameter.

An new sysfs entry has been added to ouptut only temperature. The old
entry w1_slave has been kept for compatibility, without changing its
output format.

Updating doc in Documentation/ABI/testing/sysfs-driver-w1_therm
accordingly.

Signed-off-by: Akira Shimahara 
---
 .../ABI/testing/sysfs-driver-w1_therm |  12 +
 drivers/w1/slaves/w1_therm.c  | 266 +++---
 drivers/w1/slaves/w1_therm.h  |  34 ++-
 3 files changed, 215 insertions(+), 97 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 9dcbbed..279e13d 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -41,6 +41,18 @@ Users:   any user space application which wants 
to communicate with
w1_term device
 
 
+What:  /sys/bus/w1/devices/.../temperature
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RO) return the temperature in 1/1000 degC.
+   Note that the conversion duration depend on the resolution (if
+   device support this feature). It takes 94ms in 9bits
+   resolution, 750ms for 12bits.
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  Apr 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index caa4615..2c89d9f 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -49,6 +49,10 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * (i.e. TH, TL and config register)
  * .9..12: set the device resolution in RAM (if supported)
  * .Else : do nothing
+ *
+ * temperature (RO):
+ * . temperature in 1/1000°
+ *
  * ext_power (RO):
  * . -xx : xx is kernel error
  * . 0 : device parasite powered
@@ -73,6 +77,7 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+   _attr_temperature.attr,
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
@@ -81,13 +86,16 @@ static struct attribute *w1_therm_attrs[] = {
 
 static struct attribute *w1_ds18s20_attrs[] = {
_attr_w1_slave.attr,
+   _attr_temperature.attr,
_attr_ext_power.attr,
_attr_eeprom.attr,
NULL,
 };
+
 static struct attribute *w1_ds28ea00_attrs[] = {
_attr_w1_slave.attr,
_attr_w1_seq.attr,
+   _attr_temperature.attr,
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
@@ -202,6 +210,7 @@ static struct w1_therm_family_converter w1_therm_families[] 
= {
{
.f  = _therm_family_DS18S20,
.convert= w1_DS18S20_convert_temp,
+   .get_conversion_time= w1_DS18S20_convert_time,
.set_resolution = NULL, // no config register
.get_resolution = NULL, // no config register
.write_data = w1_DS18S20_write_data,
@@ -209,6 +218,7 @@ static struct w1_therm_family_converter w1_therm_families[] 
= {
{
.f  = _therm_family_DS1822,
.convert= w1_DS18B20_convert_temp,
+   .get_conversion_time= w1_DS18B20_convert_time,
.set_resolution = w1_DS18B20_set_resolution,
.get_resolution = w1_DS18B20_get_resolution,
.write_data = w1_DS18B20_write_data,
@@ -216,6 +226,7 @@ static struct w1_therm_family_converter w1_therm_families[] 
= {
{
.f  = _therm_family_DS18B20,
.convert= w1_DS18B20_convert_temp,
+   .get_conversion_time= w1_DS18B20_convert_time,
.set_resolution = w1_DS18B20_set_resolution,
.get_resolution = w1_DS18B20_get_resolution,
.write_data = w1_DS18B20_write_data,
@@ -223,6 +234,7 @@ static struct w1_therm_family_converter w1_therm_families[] 
= {
{
.f  = _therm_family_DS28EA00

[PATCH v4 5/9] w1_therm: adding resolution sysfs entry

2020-04-29 Thread Akira Shimahara
Adding resolution sysfs entry (RW) to get or set the device resolution
Write values are managed as follow:
* `9..12`: resolution to set in bit
* Anything else: do nothing
Read values are :
* `9..12`: device resolution in bit
* `-xx`: xx is kernel error when reading the resolution

Only supported devices will show the sysfs entry. A new family has been
created for DS18S20 devices as they do not implement resolution feature.

The resolution of each device is check when the device is
discover by the bus master, in w1_therm_add_slave(struct w1_slave *).
The status is stored in the device structure w1_therm_family_data so
that the driver always knows the resolution of each device, which could
be used later to determine the required conversion duration (resolution
dependent).

The resolution is re evaluate each time a user read or write the sysfs
entry.

Signed-off-by: Akira Shimahara 
---
 .../ABI/testing/sysfs-driver-w1_therm |  17 +
 drivers/w1/slaves/w1_therm.c  | 404 ++
 drivers/w1/slaves/w1_therm.h  |  91 +++-
 3 files changed, 408 insertions(+), 104 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 9aaf625..e52dd6a 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -10,6 +10,23 @@ Users:   any user space application which wants 
to communicate with
w1_term device
 
 
+What:  /sys/bus/w1/devices/.../resolution
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) get or set the device resolution (on supported devices,
+   if not, this entry is not present). Note that the resolution
+   will be changed only in device RAM, so it will be cleared when
+   power is lost. Trigger a `save` to EEPROM command to keep
+   values after power-on. Read or write are :
+   * `9..12`: device resolution in bit
+   or resolution to set in bit
+   * `-xx`: xx is kernel error when reading the resolution
+   * Anything else: do nothing
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  Apr 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index f10a210..382d679 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -37,23 +37,59 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/*
+ * sysfile interface:
+ * w1_slave (RW) : Old driver way, kept for compatibility
+ * read :
+ * return 2 lines with the hexa output of the device
+ * return the CRC check
+ * return temperature in 1/1000°
+ * write :
+ * .0 :save the 2 or 3 bytes to the device EEPROM
+ * (i.e. TH, TL and config register)
+ * .9..12: set the device resolution in RAM (if supported)
+ * .Else : do nothing
+ * ext_power (RO):
+ * . -xx : xx is kernel error
+ * . 0 : device parasite powered
+ * . 1 : device externally powered
+ *
+ * resolution (RW):
+ * . -xx   : xx is kernel error
+ * . 9..12 : resolution set in bit (or resolution to set in bit)
+ */
+
+/*
+ * struct attribute for each device type
+ * This will enable entry in sysfs, it should match device capability
+ */
+
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
_attr_ext_power.attr,
+   _attr_resolution.attr,
NULL,
 };
 
+static struct attribute *w1_ds18s20_attrs[] = {
+   _attr_w1_slave.attr,
+   _attr_ext_power.attr,
+   NULL,
+};
 static struct attribute *w1_ds28ea00_attrs[] = {
_attr_w1_slave.attr,
_attr_w1_seq.attr,
_attr_ext_power.attr,
+   _attr_resolution.attr,
NULL,
 };
 
 /*--attribute groups*/
 ATTRIBUTE_GROUPS(w1_therm);
+ATTRIBUTE_GROUPS(w1_ds18s20);
 ATTRIBUTE_GROUPS(w1_ds28ea00);
 
+
 #if IS_REACHABLE(CONFIG_HWMON)
 static int w1_read_temp(struct device *dev, u32 attr, int channel,
long *val);
@@ -112,6 +148,13 @@ static struct w1_family_ops w1_therm_fops = {
.chip_info  = W1_CHIPINFO,
 };
 
+static struct w1_family_ops w1_ds18s20_fops = {
+   .add_slave  = w1_therm_add_slave,
+   .remove_slave   = w1_therm_remove_slave,
+   .groups = w1_ds18s20_groups,
+   .chip_info  = W1_CHIPINFO,
+};
+
 static struct w1_family_ops w1_ds28ea00_fops = {
.add_slave  = w1_therm_add_slave,
.remove_slave   = w1_therm_remove_slave,
@@ -122,7 +165,7 @@ static struct

[PATCH v4 4/9] w1_therm: adding ext_power sysfs entry

2020-04-29 Thread Akira Shimahara
Adding ext_power sysfs entry (RO). Return the power status of the device:
 - 0: device parasite powered
 - 1: device externally powered
 - xx: xx is kernel error

The power status of each device is check when the device is
discover by the bus master, in w1_therm_add_slave(struct w1_slave *).
The status is stored in the device structure w1_therm_family_data so
that the driver always knows the power state of each device, which could
be used later to determine the required strong pull up to apply on the
line.

The power status is re evaluate each time the sysfs ext_power read by
a user.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly.

Signed-off-by: Akira Shimahara 
---
 .../ABI/testing/sysfs-driver-w1_therm | 12 +++
 drivers/w1/slaves/w1_therm.c  | 94 ++-
 drivers/w1/slaves/w1_therm.h  | 43 -
 3 files changed, 146 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 4108f4c..9aaf625 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,15 @@
+What:  /sys/bus/w1/devices/.../ext_power
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RO) return the power status by asking the device
+   * `0`: device parasite powered
+   * `1`: device externally powered
+   * `-xx`: xx is kernel error when reading power status
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  Apr 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 6245950..f10a210 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -39,12 +39,14 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+   _attr_ext_power.attr,
NULL,
 };
 
 static struct attribute *w1_ds28ea00_attrs[] = {
_attr_w1_slave.attr,
_attr_w1_seq.attr,
+   _attr_ext_power.attr,
NULL,
 };
 
@@ -294,6 +296,27 @@ static inline int w1_DS18S20_convert_temp(u8 rom[9])
return t;
 }
 
+/* Helpers Functions*/
+
+static inline bool bus_mutex_lock(struct mutex *lock)
+{
+   int max_trying = W1_THERM_MAX_TRY;
+
+   /* try to acquire the mutex, if not, sleep retry_delay before retry) */
+   while (mutex_lock_interruptible(lock) != 0 && max_trying > 0) {
+   unsigned long sleep_rem;
+
+   sleep_rem = msleep_interruptible(W1_THERM_RETRY_DELAY);
+   if (!sleep_rem)
+   max_trying--;
+   }
+
+   if (!max_trying)
+   return false;   /* Didn't acquire the bus mutex */
+
+   return true;
+}
+
 /*-Interface Functions--*/
 static int w1_therm_add_slave(struct w1_slave *sl)
 {
@@ -302,6 +325,16 @@ static int w1_therm_add_slave(struct w1_slave *sl)
if (!sl->family_data)
return -ENOMEM;
atomic_set(THERM_REFCNT(sl->family_data), 1);
+
+   /* Getting the power mode of the device {external, parasite}*/
+   SLAVE_POWERMODE(sl) = read_powermode(sl);
+
+   if (SLAVE_POWERMODE(sl) < 0) {
+   /* no error returned as device has been added */
+   dev_warn(>dev,
+   "%s: Device has been added, but power_mode may be 
corrupted. err=%d\n",
+__func__, SLAVE_POWERMODE(sl));
+   }
return 0;
 }
 
@@ -512,6 +545,43 @@ error:
return ret;
 }
 
+static int read_powermode(struct w1_slave *sl)
+{
+   struct w1_master *dev_master = sl->master;
+   int max_trying = W1_THERM_MAX_TRY;
+   int  ret = -ENODEV;
+
+   if (!sl->family_data)
+   goto error;
+
+   /* prevent the slave from going away in sleep */
+   atomic_inc(THERM_REFCNT(sl->family_data));
+
+   if (!bus_mutex_lock(_master->bus_mutex)) {
+   ret = -EAGAIN;  // Didn't acquire the mutex
+   goto dec_refcnt;
+   }
+
+   while ((max_trying--) && (ret < 0)) {
+   /* safe version to select slave */
+   if (!reset_select_slave(sl)) {
+   w1_write_8(dev_master, W1_READ_PSUPPLY);
+   /* Read only one bit,
+* 1 is externally powered,
+* 0 is parasite powered
+*/
+   ret = w1_touch_bit(dev_master, 1);
+   /* ret should be either 1 either 0 */
+  

[PATCH v4 3/9] w1_therm: adding sysfs-driver-w1_therm documentation

2020-04-29 Thread Akira Shimahara
Adding a sysfs-driver-w1_therm documentation file in
Documentation/ABI/testing. It describe the onlys sysfs entry of w1_therm
module, based on Documentation/w1/slaves/w1_therm.rst

Signed-off-by: Akira Shimahara 
---
 Documentation/ABI/testing/sysfs-driver-w1_therm | 17 +
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_therm

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
new file mode 100644
index 000..4108f4c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -0,0 +1,17 @@
+What:  /sys/bus/w1/devices/.../w1_slave
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) return the temperature in 1/1000 degC.
+   *read*: return 2 lines with the hexa output data sent on the
+   bus, return the CRC check and temperature in 1/1000 degC
+   *write* :
+   * `0` : save the 2 or 3 bytes to the device EEPROM
+   (i.e. TH, TL and config register)
+   * `9..12` : set the device resolution in RAM
+   (if supported)
+   * Anything else: do nothing
+   refer to Documentation/w1/slaves/w1_therm.rst for detailed
+   information.
+Users: any user space application which wants to communicate with
+   w1_term device
\ No newline at end of file
-- 
2.26.2



[PATCH v4 2/9] w1_therm: fix reset_select_slave at beginning of search process

2020-04-29 Thread Akira Shimahara
Fix reset_select_slave issue during devices discovery by the master on
bus. The w1_reset_select_slave() from w1_io.c, which was previously used,
assume that if the slave count is 1 there is only one slave attached on
the bus. This is not always true. For example when discovering devices,
when the first device is discover by the bus master, its slave count is
1, but some other slaves may be on the bus.

In that case instead of adressing command to the attached slave the 
master throw a SKIP ROM command so that all slaves attached on the bus
will answer simultenaously causing data collision.

A dedicated reset_select_slave() function is implemented here,
it always perform an adressing to each slave using the MATCH ROM
command.

Signed-off-by: Akira Shimahara 
---
 drivers/w1/slaves/w1_therm.c | 29 ++---
 drivers/w1/slaves/w1_therm.h | 13 +
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index f027360..6245950 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -235,7 +235,7 @@ static inline int w1_DS18B20_precision(struct device 
*device, int val)
while (max_trying--) {
crc = 0;
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
int count = 0;
 
/* read values to only alter precision bits */
@@ -248,7 +248,7 @@ static inline int w1_DS18B20_precision(struct device 
*device, int val)
if (rom[8] == crc) {
rom[4] = (rom[4] & ~mask) | (precision_bits & 
mask);
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
w1_write_8(dev, W1_WRITE_SCRATCHPAD);
w1_write_8(dev, rom[2]);
w1_write_8(dev, rom[3]);
@@ -319,6 +319,21 @@ static void w1_therm_remove_slave(struct w1_slave *sl)
 
 /*Hardware Functions--*/
 
+/* Safe version of reser_select_slave - avoid using the one in w_io.c */
+static int reset_select_slave(struct w1_slave *sl)
+{
+   u8 match[9] = { W1_MATCH_ROM, };
+   u64 rn = le64_to_cpu(*((u64 *)>reg_num));
+
+   if (w1_reset_bus(sl->master))
+   return -ENODEV;
+
+   memcpy([1], , 8);
+   w1_write_block(sl->master, match, 9);
+
+   return 0;
+}
+
 static inline int w1_convert_temp(u8 rom[9], u8 fid)
 {
int i;
@@ -357,7 +372,7 @@ static ssize_t read_therm(struct device *device,
info->verdict = 0;
info->crc = 0;
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
int count = 0;
unsigned int tm = 750;
unsigned long sleep_rem;
@@ -365,7 +380,7 @@ static ssize_t read_therm(struct device *device,
w1_write_8(dev, W1_READ_PSUPPLY);
external_power = w1_read_8(dev);
 
-   if (w1_reset_select_slave(sl))
+   if (reset_select_slave(sl))
continue;
 
/* 750ms strong pullup (or delay) after the convert */
@@ -395,7 +410,7 @@ static ssize_t read_therm(struct device *device,
}
}
 
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
 
w1_write_8(dev, W1_READ_SCRATCHPAD);
count = w1_read_block(dev, info->rom, 9);
@@ -447,7 +462,7 @@ static inline int w1_therm_eeprom(struct device *device)
memset(rom, 0, sizeof(rom));
 
while (max_trying--) {
-   if (!w1_reset_select_slave(sl)) {
+   if (!reset_select_slave(sl)) {
unsigned int tm = 10;
unsigned long sleep_rem;
 
@@ -455,7 +470,7 @@ static inline int w1_therm_eeprom(struct device *device)
w1_write_8(dev, W1_READ_PSUPPLY);
external_power = w1_read_8(dev);
 
-   if (w1_reset_select_slave(sl))
+   if (reset_select_slave(sl))
continue;
 
/* 10ms strong pullup/delay after the copy command */
diff --git a/drivers/w1/slaves/w1_therm.h b/drivers/w1/slaves/w1_therm.h
index 8aa69cc..ba11c96 100644
--- a/drivers/w1/slaves/w1_therm.h
+++ b/drivers/w1/slaves/w1_therm.h
@@ -87,6 +87,19 @@ static inline int w1_DS18S20_convert_temp(u8 rom[9]);
 
 /*---Hardware Functions-*/
 
+/**
+ * reset_select_slave() - reset and select

[PATCH v4 1/9] w1_therm: creating w1_therm.h

2020-04-29 Thread Akira Shimahara
Creating w1_therm.h header to organize code. Organize the w1_therm.c file
to gather hardware functions, device specific functions, interface
functions and sysfs functions.

Signed-off-by: Akira Shimahara 
---
 drivers/w1/slaves/w1_therm.c | 302 +++
 drivers/w1/slaves/w1_therm.h | 138 
 2 files changed, 269 insertions(+), 171 deletions(-)
 create mode 100644 drivers/w1/slaves/w1_therm.h

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 18f08d7..f027360 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -5,19 +5,15 @@
  * Copyright (c) 2004 Evgeniy Polyakov 
  */
 
-#include 
-
 #include 
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
 
-#include 
+#include "w1_therm.h"
 
 #define W1_THERM_DS18S20   0x10
 #define W1_THERM_DS18220x22
@@ -41,55 +37,6 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
-struct w1_therm_family_data {
-   uint8_t rom[9];
-   atomic_t refcnt;
-};
-
-struct therm_info {
-   u8 rom[9];
-   u8 crc;
-   u8 verdict;
-};
-
-/* return the address of the refcnt in the family data */
-#define THERM_REFCNT(family_data) \
-   (&((struct w1_therm_family_data *)family_data)->refcnt)
-
-static int w1_therm_add_slave(struct w1_slave *sl)
-{
-   sl->family_data = kzalloc(sizeof(struct w1_therm_family_data),
-   GFP_KERNEL);
-   if (!sl->family_data)
-   return -ENOMEM;
-   atomic_set(THERM_REFCNT(sl->family_data), 1);
-   return 0;
-}
-
-static void w1_therm_remove_slave(struct w1_slave *sl)
-{
-   int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data));
-
-   while (refcnt) {
-   msleep(1000);
-   refcnt = atomic_read(THERM_REFCNT(sl->family_data));
-   }
-   kfree(sl->family_data);
-   sl->family_data = NULL;
-}
-
-static ssize_t w1_slave_show(struct device *device,
-   struct device_attribute *attr, char *buf);
-
-static ssize_t w1_slave_store(struct device *device,
-   struct device_attribute *attr, const char *buf, size_t size);
-
-static ssize_t w1_seq_show(struct device *device,
-   struct device_attribute *attr, char *buf);
-
-static DEVICE_ATTR_RW(w1_slave);
-static DEVICE_ATTR_RO(w1_seq);
-
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
NULL,
@@ -101,6 +48,7 @@ static struct attribute *w1_ds28ea00_attrs[] = {
NULL,
 };
 
+/*--attribute groups*/
 ATTRIBUTE_GROUPS(w1_therm);
 ATTRIBUTE_GROUPS(w1_ds28ea00);
 
@@ -154,6 +102,7 @@ static const struct hwmon_chip_info w1_chip_info = {
 #define W1_CHIPINFONULL
 #endif
 
+/*--family operations---*/
 static struct w1_family_ops w1_therm_fops = {
.add_slave  = w1_therm_add_slave,
.remove_slave   = w1_therm_remove_slave,
@@ -168,6 +117,7 @@ static struct w1_family_ops w1_ds28ea00_fops = {
.chip_info  = W1_CHIPINFO,
 };
 
+/*family binding on operations struct---*/
 static struct w1_family w1_therm_family_DS18S20 = {
.fid = W1_THERM_DS18S20,
.fops = _therm_fops,
@@ -193,26 +143,7 @@ static struct w1_family w1_therm_family_DS1825 = {
.fops = _therm_fops,
 };
 
-struct w1_therm_family_converter {
-   u8  broken;
-   u16 reserved;
-   struct w1_family*f;
-   int (*convert)(u8 rom[9]);
-   int (*precision)(struct device *device, int val);
-   int (*eeprom)(struct device *device);
-};
-
-/* write configuration to eeprom */
-static inline int w1_therm_eeprom(struct device *device);
-
-/* Set precision for conversion */
-static inline int w1_DS18B20_precision(struct device *device, int val);
-static inline int w1_DS18S20_precision(struct device *device, int val);
-
-/* The return value is millidegrees Centigrade. */
-static inline int w1_DS18B20_convert_temp(u8 rom[9]);
-static inline int w1_DS18S20_convert_temp(u8 rom[9]);
-
+/*--Device capability description---*/
 static struct w1_therm_family_converter w1_therm_families[] = {
{
.f  = _therm_family_DS18S20,
@@ -246,78 +177,7 @@ static struct w1_therm_family_converter 
w1_therm_families[] = {
}
 };
 
-static inline int w1_therm_eeprom(struct device *device)
-{
-   struct w1_slave *sl = dev_to_w1_slave(device);
-   struct w1_master *dev = sl->master;
-   u8 rom[9], external_power;
-   int ret, max_trying = 10;
-   u8 *family_data = sl->family_data;
-
-   if (!sl->family_data) {
-   ret = -ENODEV;
-   goto error;
-   

Re: [PATCH v3 3/5] w1_therm: Optimizing read timing by checking device resolution. Updating documentation

2020-04-29 Thread Akira shimahara
Le mercredi 29 avril 2020 à 15:47 +0200, Greg KH a écrit :
> On Wed, Apr 29, 2020 at 03:33:20PM +0200, Akira Shimahara wrote:
> > Patch for enhacement of w1_therm module. Added features :
> >  - Optimized conversion time regarding to device resolution
> >  - Dedicated sysfs entry for, resolution set/get, eeprom
> > save/restore,
> > and additionnal temperature entry to ease user data collect.
> >  - Code optimization to mitigate bus traffic
> 
> When you have to list different things you did in a single patch,
> that's
> a huge sign it needs to be multiple patches.  Please break this up
> into
> one-logical-change-per-patch.
> 

I will try but this block is hard to split as everything is connected.
I will separate it, but some some patches will be useless if applied
without the next ones.

> thanks,
> 
> greg k-h



_

2020-04-29 Thread Akira shimahara
Le mercredi 29 avril 2020 à 15:46 +0200, Greg KH a écrit :
> On Wed, Apr 29, 2020 at 03:32:04PM +0200, Akira Shimahara wrote:
> > Patch for enhacement of w1_therm module.
> > Adding ext_power sysfs entry (RO). Return the power status of the
> > device:
> >  - 0: device parasite powered
> >  - 1: device externally powered
> >  - xx: xx is kernel error
> > 
> > Creating Documentation/ABI/testing/sysfs-driver-w1_therm for the
> > old 
> > driver sysfs and this new entry.
> > 
> > Signed-off-by: Akira Shimahara 
> > ---
> >  .../ABI/testing/sysfs-driver-w1_therm | 29 ++
> >  drivers/w1/slaves/w1_therm.c  | 93
> > ++-
> >  drivers/w1/slaves/w1_therm.h  | 44 -
> >  3 files changed, 163 insertions(+), 3 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_therm
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm
> > b/Documentation/ABI/testing/sysfs-driver-w1_therm
> > new file mode 100644
> > index 000..9aaf625
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
> > @@ -0,0 +1,29 @@
> > +What:  /sys/bus/w1/devices/.../ext_power
> > +Date:  Apr 2020
> > +Contact:   Akira Shimahara 
> > +Description:
> > +   (RO) return the power status by asking the device
> > +   * `0`: device parasite powered
> > +   * `1`: device externally powered
> > +   * `-xx`: xx is kernel error when reading power
> > status
> > +Users:     any user space application which wants to
> > communicate with
> > +   w1_term device
> > +
> > +
> > +What:  /sys/bus/w1/devices/.../w1_slave
> > +Date:  Apr 2020
> > +Contact:   Akira Shimahara 
> > +Description:
> > +   (RW) return the temperature in 1/1000 degC.
> > +   *read*: return 2 lines with the hexa output data sent
> > on the
> > +   bus, return the CRC check and temperature in 1/1000
> > degC
> 
> the w1_slave file returns a temperature???
> 
> And sysfs is 1 value-per-file, not multiple lines.
> 
> And as this is a temperature, what's wrong with the iio interface
> that
> handles temperature already?  Don't go making up new userspace apis
> when
> we already have good ones today :)

Yes the existing syfs w1_slave return 2 lines, user application 
catch normally only the temperature. We keep it as many userspace
application are already based on this output, but to ease user
to catch the only purpose of these devices (temperature sensors),
we added on entry which return only the temperature (avoiding
string parsing ,... on both side i.e. driver and user app).

> 
> > +   *write* :
> > +   * `0` : save the 2 or 3 bytes to the device
> > EEPROM
> > +   (i.e. TH, TL and config register)
> > +   * `9..12` : set the device resolution in RAM
> > +   (if supported)
> 
> I don't understand these write values, how do they match up to a
> temperature readin?

This is the previous implementation, and yes, it was not very clear.
These value are not connected to temperature reading. The sysfs 
entry was used to enter user value to device RAM, in 2 registers:
if 0 it trigger a save to the device EEPROM, if the value is between
9 and 12, it stores in the resolution register of the device.
In the next patches, we add more sysfs entries to separate things.

> 
> > +   * Anything else: do nothing
> > +   refer to Documentation/w1/slaves/w1_therm.rst for
> > detailed
> > +   information.
> > +Users: any user space application which wants to
> > communicate with
> > +   w1_term device
> > \ No newline at end of file
> > diff --git a/drivers/w1/slaves/w1_therm.c
> > b/drivers/w1/slaves/w1_therm.c
> > index 6245950..a530853 100644
> > --- a/drivers/w1/slaves/w1_therm.c
> > +++ b/drivers/w1/slaves/w1_therm.c
> > @@ -39,12 +39,14 @@ module_param_named(strong_pullup,
> > w1_strong_pullup, int, 0);
> >  
> >  static struct attribute *w1_therm_attrs[] = {
> > _attr_w1_slave.attr,
> > +   _attr_ext_power.attr,
> > NULL,
> >  };
> >  
> >  static struct attribute *w1_ds28ea00_attrs[] = {
> > _attr_w1_slave.attr,
> > _attr_w1_seq.attr,
> > +   _attr_ext_power.attr,
> > NULL,
> >  };
> >  
> > @@ -294,6 +296,26 @@ static inli

Re: [PATCH v3 1/5] w1_therm: fix reset_select_slave. Creating w1_therm.h

2020-04-29 Thread Akira shimahara
Le mercredi 29 avril 2020 à 15:43 +0200, Greg KH a écrit :
> On Wed, Apr 29, 2020 at 03:30:48PM +0200, Akira Shimahara wrote:
> > Patch for enhancement of w1_therm module.
> >  - Creating a w1_therm.h file to clean up the code and documenting
> > it.
> >  - fix reset_select_slave function: w1_reset_select_slave() from
> > w1_io.c
> > could not be used here because a SKIP ROM command is sent if
> > only
> > one device is on the line. At the beginning of the search
> > process,
> > sl->master->slave_count is 1 even if more devices are on the
> > line,
> > causing data collision in the bus.
> 
> This should be three different patches, one to create the file by
> moving
> the code, one to document it, and the last to do the fix.
> 
> thanks,
> 
> greg k-h

Ok, I will do

Akira Shimahara



[PATCH v3 5/5] w1_therm: adding bulk read support. Updating docs in w1_therm.rst and sysfs-driver-w1_therm

2020-04-29 Thread Akira Shimahara
Patch for enhacement of w1_therm module. Adding bulk read support.
Sending a 'trigger' command in the dedicated sysfs entry of the bus master
device send a conversion command for all the slaves on the bus. The sysfs
entry is added as soon as at least one device supporting this feature
is detected on the bus.

A strong pull up is apply on the line if at least one device required it.
The duration of the pull up is the max time required by a device on the
line, which depends on the resolution settings of each device. The strong
pull up could be adjust with the a module parameter.

Updating documentation in Documentation/ABI/testing/sysfs-driver-w1_therm
and Documentation/w1/slaves/w1_therm.rst

Signed-off-by: Akira Shimahara 
---
 .../ABI/testing/sysfs-driver-w1_therm |  36 ++-
 Documentation/w1/slaves/w1_therm.rst  |  50 +++-
 drivers/w1/slaves/w1_therm.c  | 217 +-
 drivers/w1/slaves/w1_therm.h  |  47 
 4 files changed, 335 insertions(+), 15 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 39488a4..1f911ed 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -61,9 +61,16 @@ Date:Apr 2020
 Contact:   Akira Shimahara 
 Description:
(RO) return the temperature in 1/1000 degC.
-   Note that the conversion duration depend on the resolution (if
-   device support this feature). It takes 94ms in 9bits
-   resolution, 750ms for 12bits.
+   * If a bulk read has been triggered, it will directly
+   return the temperature computed when the bulk read
+   occurred, if available. If not yet available, nothing
+   is returned (a debug kernel message is sent), you
+   should retry later on.
+   * If no bulk read has been triggered, it will trigger
+   a conversion and send the result. Note that the
+   conversion duration depend on the resolution (if
+   device support this feature). It takes 94ms in 9bits
+   resolution, 750ms for 12bits.
 Users: any user space application which wants to communicate with
w1_term device
 
@@ -84,4 +91,25 @@ Description:
refer to Documentation/w1/slaves/w1_therm.rst for detailed
information.
 Users: any user space application which wants to communicate with
-   w1_term device
\ No newline at end of file
+   w1_term device
+
+
+What:  /sys/bus/w1/devices/w1_bus_masterXX/therm_bulk_read
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) trigger a bulk read conversion. read the status
+   *read*:
+   * `-1`: conversion in progress on at least 1 sensor
+   * `1` : conversion complete but at least one sensor
+   value has not been read yet
+   * `0` : no bulk operation. Reading temperature will
+   trigger a conversion on each device
+   *write*: `trigger`: trigger a bulk read on all supporting
+   devices on the bus
+   Note that if a bulk read is sent but one sensor is not read
+   immediately, the next access to temperature on this device
+   will return the temperature measured at the time of issue
+   of the bulk read command (not the current temperature).
+Users: any user space application which wants to communicate with
+   w1_term device
diff --git a/Documentation/w1/slaves/w1_therm.rst 
b/Documentation/w1/slaves/w1_therm.rst
index 82e8ffe..06eaff1 100644
--- a/Documentation/w1/slaves/w1_therm.rst
+++ b/Documentation/w1/slaves/w1_therm.rst
@@ -26,20 +26,31 @@ W1_THERM_DS1825 0x3B
 W1_THERM_DS28EA00  0x42
    
 
-Support is provided through the sysfs w1_slave file.  Each open and
+Support is provided through the sysfs w1_slave file. Each open and
 read sequence will initiate a temperature conversion then provide two
-lines of ASCII output.  The first line contains the nine hex bytes
+lines of ASCII output. The first line contains the nine hex bytes
 read along with a calculated crc value and YES or NO if it matched.
-If the crc matched the returned values are retained.  The second line
+If the crc matched the returned values are retained. The second line
 displays the retained values along with a temperature in millidegrees
 Centigrade after t=.
 
-Parasite powered devices are limited to one slave performing a
-temperature conversion at a time.  If none of the devices are parasite
-powered it would be possible

[PATCH v3 4/5] w1_therm: alarms support by adding sysfs entry. Updating doc

2020-04-29 Thread Akira Shimahara
Patch for enhacement of w1_therm module. Adding device alarms settings
by a dedicated sysfs entry alarms (RW) : read or write TH and TL in the
device RAM. Checking devices in alarm state could be performed using
the master search command.

Updating doc in Documentation/ABI/testing/sysfs-driver-w1_therm
accordingly.

Signed-off-by: Akira Shimahara 
---
 .../ABI/testing/sysfs-driver-w1_therm |  15 ++
 drivers/w1/slaves/w1_therm.c  | 129 ++
 drivers/w1/slaves/w1_therm.h  |  12 ++
 3 files changed, 156 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 279e13d..39488a4 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,18 @@
+What:  /sys/bus/w1/devices/.../alarms
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) read or write TH and TL (Temperature High an Low) alarms.
+   Values shall be space separated and in the device range
+   (typical -55 degC to 125 degC). Values are integer as they
+   are store in a 8bit register in the device.
+   Lowest value is automatically put to TL.
+   Once set, alarms could be search at master level, refer to
+   Documentation/w1/w1_generic.rst for detailed information
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../eeprom
 Date:  Apr 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 3ddf2b1..db75048 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -68,6 +68,12 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
  * (device do that automatically on power-up)
  *
  *
+ * alarms (RW) : read TH and TL (Temperature High an Low) alarms
+ * Values shall be space separated and in the device range
+ * (typically -55° to 125°)
+ * Values are integer are they are store in a 8bit field in the device
+ * Lowest value is automatically put to TL
+ *
  */
 
 /*
@@ -81,6 +87,7 @@ static struct attribute *w1_therm_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -89,6 +96,7 @@ static struct attribute *w1_ds18s20_attrs[] = {
_attr_temperature.attr,
_attr_ext_power.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -99,6 +107,7 @@ static struct attribute *w1_ds28ea00_attrs[] = {
_attr_ext_power.attr,
_attr_resolution.attr,
_attr_eeprom.attr,
+   _attr_alarms.attr,
NULL,
 };
 
@@ -207,6 +216,7 @@ static struct w1_family w1_therm_family_DS1825 = {
 };
 
 /*--Device capability description---*/
+
 static struct w1_therm_family_converter w1_therm_families[] = {
{
.f  = _therm_family_DS18S20,
@@ -1009,6 +1019,125 @@ static ssize_t eeprom_store(struct device *device,
return size;
 }
 
+static ssize_t alarms_show(struct device *device,
+   struct device_attribute *attr, char *buf)
+{
+   struct w1_slave *sl = dev_to_w1_slave(device);
+   int ret = -ENODEV;
+   s8 th = 0, tl = 0;
+   struct therm_info scratchpad;
+
+   ret = read_scratchpad(sl, );
+
+   if (!ret)   {
+   th = scratchpad.rom[2]; // TH is byte 2
+   tl = scratchpad.rom[3]; // TL is byte 3
+   } else {
+   dev_info(device,
+   "%s: error reading alarms register %d\n",
+   __func__, ret);
+   }
+
+   return sprintf(buf, "%hd %hd\n", tl, th);
+}
+
+static ssize_t alarms_store(struct device *device,
+   struct device_attribute *attr, const char *buf, size_t size)
+{
+   struct w1_slave *sl = dev_to_w1_slave(device);
+   struct therm_info info;
+   u8 new_config_register[3];  /* array of data to be written */
+   int temp, ret = -EINVAL;
+   char *token = NULL;
+   s8 tl, th, tt;  /* 1 byte per value + temp ring order */
+   char *p_args = kmalloc(size, GFP_KERNEL);
+
+   /* Safe string copys as buf is const */
+   if (!p_args) {
+   dev_warn(device,
+   "%s: error unable to allocate memory %d\n",
+   __func__, -ENOMEM);
+   return size;
+   }
+   strcpy(p_args, buf);
+
+   /* Split string using space char */
+   token = strsep(_args, " ");
+
+   if (!token) {
+   dev_info(device,
+   "%s: error parsing args %d\n", __func__, -EINVAL);
+   got

[PATCH v3 3/5] w1_therm: Optimizing read timing by checking device resolution. Updating documentation

2020-04-29 Thread Akira Shimahara
Patch for enhacement of w1_therm module. Added features :
 - Optimized conversion time regarding to device resolution
 - Dedicated sysfs entry for, resolution set/get, eeprom save/restore,
and additionnal temperature entry to ease user data collect.
 - Code optimization to mitigate bus traffic

The status of each device (power status and resolution if supported)
is stored in a dedicated structure in the driver, so that device are not
interrogated during temperature conversion process. The status is
evaluated as soon as a device is connected on the bus, and re evaluated
when user either set values either by get them, using the corresponding
sysfs entry.

A dedicated EEPROM sysfs entry allow the user to save or restore device
registers from/to device EEPROM.

Updating Documentation/ABI/testing/sysfs-driver-w1_therm accordingly
with w1_therm_add_slavenew sysfs entries

Signed-off-by: Akira Shimahara 
---
 .../ABI/testing/sysfs-driver-w1_therm |  43 +
 drivers/w1/slaves/w1_therm.c  | 785 +-
 drivers/w1/slaves/w1_therm.h  | 168 +++-
 3 files changed, 760 insertions(+), 236 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 9aaf625..279e13d 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -1,3 +1,17 @@
+What:  /sys/bus/w1/devices/.../eeprom
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (WO) writing that file will either trigger a save of the
+   device data to its embedded EEPROM, either restore data
+   embedded in device EEPROM. Be aware that devices support
+   limited EEPROM writing cycles (typical 50k)
+   * `save`: save device RAM to EEPROM
+   * `restore`: restore EEPROM data in device RAM
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../ext_power
 Date:  Apr 2020
 Contact:   Akira Shimahara 
@@ -10,6 +24,35 @@ Users:   any user space application which wants 
to communicate with
w1_term device
 
 
+What:  /sys/bus/w1/devices/.../resolution
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) get or set the device resolution (on supported devices,
+   if not, this entry is not present). Note that the resolution
+   will be changed only in device RAM, so it will be cleared when
+   power is lost. Trigger a `save` to EEPROM command to keep
+   values after power-on. Read or write are :
+   * `9..12`: device resolution in bit
+   or resolution to set in bit
+   * `-xx`: xx is kernel error when reading the resolution
+   * Anything else: do nothing
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
+What:  /sys/bus/w1/devices/.../temperature
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RO) return the temperature in 1/1000 degC.
+   Note that the conversion duration depend on the resolution (if
+   device support this feature). It takes 94ms in 9bits
+   resolution, 750ms for 12bits.
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
 What:  /sys/bus/w1/devices/.../w1_slave
 Date:  Apr 2020
 Contact:   Akira Shimahara 
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index a530853..3ddf2b1 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -37,23 +37,77 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+/*
+ * sysfile interface:
+ * w1_slave (RW) : Old driver way, kept for compatibility
+ * read :
+ * return 2 lines with the hexa output of the device
+ * return the CRC check
+ * return temperature in 1/1000°
+ * write :
+ * .0 :save the 2 or 3 bytes to the device EEPROM
+ * (i.e. TH, TL and config register)
+ * .9..12: set the device resolution in RAM (if supported)
+ * .Else : do nothing
+ *
+ * temperature (RO):
+ * . temperature in 1/1000°
+ *
+ * ext_power (RO):
+ * . -xx : xx is kernel error refer to /usr/include/asm/errno.h
+ * . 0 : device parasite powered
+ * . 1 : device externally powered
+ *
+ * resolution (RW):
+ * . -xx   : xx is kernel error refer to /usr/include/asm/errno.h
+ * . 9..12 : resolution set in bit (or resolution to set in bit)
+ *
+ * eeprom (WO): be aware that eeprom writing cycles count

[PATCH v3 2/5] w1_therm: adding sysfs entry to check device power

2020-04-29 Thread Akira Shimahara
Patch for enhacement of w1_therm module.
Adding ext_power sysfs entry (RO). Return the power status of the device:
 - 0: device parasite powered
 - 1: device externally powered
 - xx: xx is kernel error

Creating Documentation/ABI/testing/sysfs-driver-w1_therm for the old 
driver sysfs and this new entry.

Signed-off-by: Akira Shimahara 
---
 .../ABI/testing/sysfs-driver-w1_therm | 29 ++
 drivers/w1/slaves/w1_therm.c  | 93 ++-
 drivers/w1/slaves/w1_therm.h  | 44 -
 3 files changed, 163 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-w1_therm

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm 
b/Documentation/ABI/testing/sysfs-driver-w1_therm
new file mode 100644
index 000..9aaf625
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -0,0 +1,29 @@
+What:  /sys/bus/w1/devices/.../ext_power
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RO) return the power status by asking the device
+   * `0`: device parasite powered
+   * `1`: device externally powered
+   * `-xx`: xx is kernel error when reading power status
+Users: any user space application which wants to communicate with
+   w1_term device
+
+
+What:  /sys/bus/w1/devices/.../w1_slave
+Date:  Apr 2020
+Contact:   Akira Shimahara 
+Description:
+   (RW) return the temperature in 1/1000 degC.
+   *read*: return 2 lines with the hexa output data sent on the
+   bus, return the CRC check and temperature in 1/1000 degC
+   *write* :
+   * `0` : save the 2 or 3 bytes to the device EEPROM
+   (i.e. TH, TL and config register)
+   * `9..12` : set the device resolution in RAM
+   (if supported)
+   * Anything else: do nothing
+   refer to Documentation/w1/slaves/w1_therm.rst for detailed
+   information.
+Users: any user space application which wants to communicate with
+   w1_term device
\ No newline at end of file
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 6245950..a530853 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -39,12 +39,14 @@ module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
+   _attr_ext_power.attr,
NULL,
 };
 
 static struct attribute *w1_ds28ea00_attrs[] = {
_attr_w1_slave.attr,
_attr_w1_seq.attr,
+   _attr_ext_power.attr,
NULL,
 };
 
@@ -294,6 +296,26 @@ static inline int w1_DS18S20_convert_temp(u8 rom[9])
return t;
 }
 
+/* Helpers Functions*/
+
+static inline bool bus_mutex_lock(struct mutex *lock)
+{
+   int max_trying = W1_THERM_MAX_TRY;
+   /* try to acquire the mutex, if not, sleep retry_delay before retry) */
+   while (mutex_lock_interruptible(lock) != 0 && max_trying > 0) {
+   unsigned long sleep_rem;
+
+   sleep_rem = msleep_interruptible(W1_THERM_RETRY_DELAY);
+   if (!sleep_rem)
+   max_trying--;
+   }
+
+   if (!max_trying)
+   return false;   /* Didn't acquire the bus mutex */
+
+   return true;
+}
+
 /*-Interface Functions--*/
 static int w1_therm_add_slave(struct w1_slave *sl)
 {
@@ -302,6 +324,16 @@ static int w1_therm_add_slave(struct w1_slave *sl)
if (!sl->family_data)
return -ENOMEM;
atomic_set(THERM_REFCNT(sl->family_data), 1);
+
+   /* Getting the power mode of the device {external, parasite}*/
+   SLAVE_POWERMODE(sl) = read_powermode(sl);
+
+   if (SLAVE_POWERMODE(sl) < 0) {
+   /* no error returned as device has been added */
+   dev_warn(>dev,
+   "%s: Device has been added, but power_mode may be 
corrupted. err=%d\n",
+__func__, SLAVE_POWERMODE(sl));
+   }
return 0;
 }
 
@@ -512,6 +544,43 @@ error:
return ret;
 }
 
+static int read_powermode(struct w1_slave *sl)
+{
+   struct w1_master *dev_master = sl->master;
+   int max_trying = W1_THERM_MAX_TRY;
+   int  ret = -ENODEV;
+
+   if (!sl->family_data)
+   goto error;
+
+   /* prevent the slave from going away in sleep */
+   atomic_inc(THERM_REFCNT(sl->family_data));
+
+   if (!bus_mutex_lock(_master->bus_mutex)) {
+   ret = -EAGAIN;  // Didn't acquire the mutex
+   goto dec_refcnt;
+   }
+
+   while ((max_trying--) && (ret &l

[PATCH v3 1/5] w1_therm: fix reset_select_slave. Creating w1_therm.h

2020-04-29 Thread Akira Shimahara
Patch for enhancement of w1_therm module.
 - Creating a w1_therm.h file to clean up the code and documenting it.
 - fix reset_select_slave function: w1_reset_select_slave() from w1_io.c
could not be used here because a SKIP ROM command is sent if only
one device is on the line. At the beginning of the search process,
sl->master->slave_count is 1 even if more devices are on the line,
causing data collision in the bus.

Signed-off-by: Akira Shimahara 
---
 drivers/w1/slaves/w1_therm.c | 325 ---
 drivers/w1/slaves/w1_therm.h | 151 
 2 files changed, 301 insertions(+), 175 deletions(-)
 create mode 100644 drivers/w1/slaves/w1_therm.h

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 18f08d7..6245950 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -5,19 +5,15 @@
  * Copyright (c) 2004 Evgeniy Polyakov 
  */
 
-#include 
-
 #include 
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
 
-#include 
+#include "w1_therm.h"
 
 #define W1_THERM_DS18S20   0x10
 #define W1_THERM_DS18220x22
@@ -41,55 +37,6 @@
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
-struct w1_therm_family_data {
-   uint8_t rom[9];
-   atomic_t refcnt;
-};
-
-struct therm_info {
-   u8 rom[9];
-   u8 crc;
-   u8 verdict;
-};
-
-/* return the address of the refcnt in the family data */
-#define THERM_REFCNT(family_data) \
-   (&((struct w1_therm_family_data *)family_data)->refcnt)
-
-static int w1_therm_add_slave(struct w1_slave *sl)
-{
-   sl->family_data = kzalloc(sizeof(struct w1_therm_family_data),
-   GFP_KERNEL);
-   if (!sl->family_data)
-   return -ENOMEM;
-   atomic_set(THERM_REFCNT(sl->family_data), 1);
-   return 0;
-}
-
-static void w1_therm_remove_slave(struct w1_slave *sl)
-{
-   int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data));
-
-   while (refcnt) {
-   msleep(1000);
-   refcnt = atomic_read(THERM_REFCNT(sl->family_data));
-   }
-   kfree(sl->family_data);
-   sl->family_data = NULL;
-}
-
-static ssize_t w1_slave_show(struct device *device,
-   struct device_attribute *attr, char *buf);
-
-static ssize_t w1_slave_store(struct device *device,
-   struct device_attribute *attr, const char *buf, size_t size);
-
-static ssize_t w1_seq_show(struct device *device,
-   struct device_attribute *attr, char *buf);
-
-static DEVICE_ATTR_RW(w1_slave);
-static DEVICE_ATTR_RO(w1_seq);
-
 static struct attribute *w1_therm_attrs[] = {
_attr_w1_slave.attr,
NULL,
@@ -101,6 +48,7 @@ static struct attribute *w1_ds28ea00_attrs[] = {
NULL,
 };
 
+/*--attribute groups*/
 ATTRIBUTE_GROUPS(w1_therm);
 ATTRIBUTE_GROUPS(w1_ds28ea00);
 
@@ -154,6 +102,7 @@ static const struct hwmon_chip_info w1_chip_info = {
 #define W1_CHIPINFONULL
 #endif
 
+/*--family operations---*/
 static struct w1_family_ops w1_therm_fops = {
.add_slave  = w1_therm_add_slave,
.remove_slave   = w1_therm_remove_slave,
@@ -168,6 +117,7 @@ static struct w1_family_ops w1_ds28ea00_fops = {
.chip_info  = W1_CHIPINFO,
 };
 
+/*family binding on operations struct---*/
 static struct w1_family w1_therm_family_DS18S20 = {
.fid = W1_THERM_DS18S20,
.fops = _therm_fops,
@@ -193,26 +143,7 @@ static struct w1_family w1_therm_family_DS1825 = {
.fops = _therm_fops,
 };
 
-struct w1_therm_family_converter {
-   u8  broken;
-   u16 reserved;
-   struct w1_family*f;
-   int (*convert)(u8 rom[9]);
-   int (*precision)(struct device *device, int val);
-   int (*eeprom)(struct device *device);
-};
-
-/* write configuration to eeprom */
-static inline int w1_therm_eeprom(struct device *device);
-
-/* Set precision for conversion */
-static inline int w1_DS18B20_precision(struct device *device, int val);
-static inline int w1_DS18S20_precision(struct device *device, int val);
-
-/* The return value is millidegrees Centigrade. */
-static inline int w1_DS18B20_convert_temp(u8 rom[9]);
-static inline int w1_DS18S20_convert_temp(u8 rom[9]);
-
+/*--Device capability description---*/
 static struct w1_therm_family_converter w1_therm_families[] = {
{
.f  = _therm_family_DS18S20,
@@ -246,78 +177,7 @@ static struct w1_therm_family_converter 
w1_therm_families[] = {
}
 };
 
-static inline int w1_therm_eeprom(struct device *device)
-{
-   struct w1_slave *

Re: [PATCH v2 1/2] Changes in w1_therm.c and adding w1_therm.h

2020-04-28 Thread Akira shimahara
Le mardi 28 avril 2020 à 14:44 +0200, Greg KH a écrit :
> On Sun, Apr 26, 2020 at 07:20:30PM +0200, Akira shimahara wrote:
> > Le dimanche 26 avril 2020 à 19:09 +0200, Greg KH a écrit :
> > > On Sun, Apr 26, 2020 at 02:36:52PM +0200, Akira shimahara wrote:
> > > > Le dimanche 26 avril 2020 à 09:51 +0200, Greg KH a écrit :
> > > > > On Sat, Apr 25, 2020 at 05:31:41PM +0200, Akira Shimahara
> > > > > wrote:
> > > > > > From: Akira SHIMAHARA 
> > > > > > 
> > > > > > Patch for enhacement of w1_therm module. Added features :
> > > > > >  - Bulk read : send one command for all the slaves
> > > > > > on the bus to trigger temperature conversion
> > > > > >  - Optimized conversion time regarding to device resolution
> > > > > >  - Dedicated sysfs entry for powering read,
> > > > > > resolution set/get, eeprom save/restore
> > > > > >  - Alarms settings and reading
> > > > > >  - Code optimization to mitigate bus traffic
> > > > > > (devices information are stored to avoid
> > > > > > interrogating each device every-time)
> > > > > > 
> > > > > > Following sysfs entry are added :
> > > > > >  - temperature (RO) : return the temperature in 1/1000°
> > > > > >  - ext_power (RO) : return the power status of the device
> > > > > >  - resolution (RW) : get or set the device resolution
> > > > > > (supported
> > > > > > devices)
> > > > > >  - eeprom (WO) :trigger a save or restore to/from device
> > > > > > EEPROM
> > > > > >  - alarms (RW) : read or write TH and TL in the device RAM
> > > > > >  - therm_bulk_read (RW) : Attribute at master level to
> > > > > > trigger
> > > > > > bulk read and to survey the progress of devices
> > > > > > conversions
> > > > > >  - w1_slave has been kept for compatibility
> > > > > > 
> > > > > > Main motivation was to improve temperature reading speed,
> > > > > > which
> > > > > > depend
> > > > > > on resolution settings of devices. The module store the
> > > > > > powwer
> > > > > > status and
> > > > > > the resolution of each device so that during reading
> > > > > > operation,
> > > > > > no
> > > > > > transaction is required on the bus, which improve speed.
> > > > > > The hardware status is checked as soon as a new device is
> > > > > > detected, 
> > > > > > when a user change occurred, or when the corresponding sys
> > > > > > file
> > > > > > is 
> > > > > > accessed by user.
> > > > > > 
> > > > > > The bulk read allow to trigger convserion of all devices on
> > > > > > the
> > > > > > bus
> > > > > > at
> > > > > > the same time. It will apply a strong pull up on the line
> > > > > > if at
> > > > > > least
> > > > > > one device required it. The duration of the pull up is the
> > > > > > max
> > > > > > time
> > > > > > required by a device on the line.
> > > > > > 
> > > > > > Please let me know any feedback you have on this patch.
> > > > > > 
> > > > > > Thanks ahead,
> > > > > > 
> > > > > > Signed-off-by: Akira Shimahara 
> > > > > > ---
> > > > > > Changes in v2:
> > > > > >  - Adding documentation in Documentatin/ABI/testing/sysfs-
> > > > > > driver-
> > > > > > w1_therm
> > > > > >  - Updating existing documentation in
> > > > > > Documentation/w1/slaves/w1_therm.rst
> > > > > > 
> > > > > >  drivers/w1/slaves/w1_therm.c | 1406
> > > > > > ++--
> > > > > > --
> > > > > >  drivers/w1/slaves/w1_therm.h |  386 ++
> > > > > >  2 files changed, 1470 insertions(+), 322 deletions(-)
> > > > > >  create mode 100644 drivers/w1/slaves/w1_therm.h
> > > > > 
> > > > > No documentation files are added here :(
> > > > > 
> > > > 
> > > > It's in the PATCH 2/2 included in the previous mail. You want
> > > > me to
> > > > merge in one commit ?. I thought it was easier for you to keep
> > > > a
> > > > track
> > > > of the v1.
> > > 
> > > What previous mail?  I don't see a patch 2/2 here, did you not
> > > cc:
> > > me?
> > > 
> > > thanks,
> > > 
> > > greg kh
> > 
> > I'm so sorry Greg, I made a mistake. Please find hereby the patch
> > 2/2.
> > Let me know if you want me to merge into 1 commit.
> 
> No, multiple patches are good.
> 
> And the first patch really should be split up into smaller pieces
> too.
> Each patch should only do 1 thing, not lots of things all at once.  
> 
> Can you do that and send a patch series please?
> 
> thanks,
> 
> greg k-h

Well noted, I will do it tomorrow

Regards

Akira Shimahara