All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] w1: w1_therm: Add sysfs entries to control conversion time and driver features
@ 2020-09-04 16:00 Ivan Zaentsev
  2020-09-04 16:00 ` [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device Ivan Zaentsev
  0 siblings, 1 reply; 14+ messages in thread
From: Ivan Zaentsev @ 2020-09-04 16:00 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: Jonathan Corbet, Akira Shimahara, Greg Kroah-Hartman,
	Mauro Carvalho Chehab, Dan Carpenter, Colin Ian King,
	linux-kernel, linux-doc, Evgeny Boger, Ivan Zaentsev

The conversion time of common DS18B20 clones deviates from
datasheet specs. Allow adjustment and automatic measure of the
conversion time.

Add 'conv_time' sysfs attribute:
  *read*: Current conversion time in milliseconds.
  *write*:
     '0': Set default conversion time.
     '1': Measure and set the conversion time. Make a
          single temperature conversion, poll and measure
          an actual value. Measured value is increased
          by 20% for temperature drift. A new conversion
          time is returned by reading the same attribute.
     other positive value:
        Set the conversion time in milliseconds.

The setting is active until a resolution change. Then it is reset to
default conversion time for a new resolution.

Add 'features' sysfs attribute to control optional driver settings
per device. Bit masks to read/write (logical OR):
  1: Enable check for conversion success. If byte 6 of
     scratchpad memory is 0xC after conversion, and
     temperature reads 85.00 (powerup value) or 127.94
     (insufficient power) - return a conversion error.

  2: Enable poll for conversion completion. Generate read cycles
     after the conversion start and wait for 1's. In parasite
     power mode this feature is not available.

There are some clones of DS18B20 with fixed 12 bit resolution. Make the
driver verify the resolution by reading back the device after resolution
change.

Signed-off-by: Ivan Zaentsev <ivan.zaentsev@wirenboard.ru>
---
 .../ABI/testing/sysfs-driver-w1_therm         |  47 +++
 Documentation/w1/slaves/w1_therm.rst          |  37 +-
 drivers/w1/slaves/w1_therm.c                  | 357 ++++++++++++++++--
 3 files changed, 410 insertions(+), 31 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm b/Documentation/ABI/testing/sysfs-driver-w1_therm
index 9b488c0afdfa..b44b51a88c5e 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -53,6 +53,9 @@ Description:
 			or resolution to set in bit
 			* '-xx': xx is kernel error when reading the resolution
 			* Anything else: do nothing
+		Some DS18B20 clones are fixed in 12-bit resolution, so the
+		actual resolution is read back from the chip and verified. Error
+		is reported if the results differ.
 Users:		any user space application which wants to communicate with
 		w1_term device
 
@@ -114,3 +117,47 @@ Description:
 		of the bulk read command (not the current temperature).
 Users:		any user space application which wants to communicate with
 		w1_term device
+
+
+What:		/sys/bus/w1/devices/.../conv_time
+Date:		July 2020
+Contact:	Ivan Zaentsev <ivan.zaentsev@wirenboard.ru>
+Description:
+		(RW) Get, set, or measure a temperature conversion time. The
+		setting remains active until a resolution change. Then it is
+		reset to default (datasheet) conversion time for a new
+		resolution.
+
+		*read*: Actual conversion time in milliseconds. *write*:
+			'0': Set the default conversion time from the datasheet.
+			'1': Measure and set the conversion time. Make a single
+			     temperature conversion, measure an actual value.
+			     Increase it by 20% for temperature range. A new
+			     conversion time can be obtained by reading this
+			     same attribute.
+			other positive value:
+			     Set the conversion time in milliseconds.
+
+Users:		An application using the w1_term device
+
+
+What:		/sys/bus/w1/devices/.../features
+Date:		July 2020
+Contact:	Ivan Zaentsev <ivan.zaentsev@wirenboard.ru>
+Description:
+		(RW) Control optional driver settings.
+		Bit masks to read/write (logical OR):
+
+                1: Enable check for conversion success. If byte 6 of
+                   scratchpad memory is 0xC after conversion, and
+                   temperature reads 85.00 (powerup value) or 127.94
+                   (insufficient power) - return a conversion error.
+
+                2: Enable poll for conversion completion. Generate read cycles
+                   after the conversion start and wait for 1's. In parasite
+                   power mode this feature is not available.
+
+		*read*:  Currently selected features, bitwise OR.
+		*write*: Select features, bitwise OR.
+
+Users:		An application using the w1_term device
diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
index cc4edae17751..f1148181f53e 100644
--- a/Documentation/w1/slaves/w1_therm.rst
+++ b/Documentation/w1/slaves/w1_therm.rst
@@ -52,6 +52,19 @@ 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).
 
+A strong pullup will be applied during the conversion if required.
+
+``conv_time`` sysfs entry is used to get current conversion time (read), and
+adjust it (write). A temperature conversion time depends on the device type and
+it's current resolution. Default conversion time is set by the driver according
+to the device datasheet. A conversion time for many original device clones
+deviate from datasheet specs. There are three options: 1) manually set the
+correct conversion time by writing a value in milliseconds to ``conv_time``; 2)
+auto measure and set a conversion time by writing ``1`` to
+``conv_time``; 3) use ``features`` entry to enable poll for conversion
+completion. Options 2, 3 can't be used in parasite power mode. To get back to
+the default conversion time write ``0`` to ``conv_time``.
+
 Writing a value between 9 and 12 to the sysfs w1_slave file will change the
 precision of the sensor for the next readings. This value is in (volatile)
 SRAM, so it is reset when the sensor gets power-cycled.
@@ -61,11 +74,14 @@ has to be written to the sysfs w1_slave file. Since the EEPROM has a limited
 amount of writes (>50k), this command should be used wisely.
 
 Alternatively, resolution can be set or read (value from 9 to 12) using the
-dedicated resolution sysfs entry on each device. This sysfs entry is not
-present for devices not supporting this feature. Driver will adjust the
-correct conversion time for each device regarding to its resolution setting.
-In particular, strong pullup will be applied if required during the conversion
-duration.
+dedicated resolution sysfs entry on each device. This sysfs entry is not present
+for devices not supporting this feature.
+
+Some non-genuine DS18B20 chips are
+fixed in 12-bit mode only, so the actual resolution is read back from the chip
+and verified by the driver.
+
+Note: Changing the resolution reverts the conversion time to default.
 
 The write-only sysfs entry eeprom is an alternative for EEPROM operations:
   * 'save': will save device RAM to EEPROM
@@ -104,3 +120,14 @@ location of the chip in the 1-wire bus without needing pre-existing
 knowledge of the bus ordering.  Support is provided through the sysfs
 w1_seq file.  The file will contain a single line with an integer value
 representing the device index in the bus starting at 0.
+
+``features`` sysfs entry controls optional driver settings per device.
+Insufficient power in parasite mode, line noise and insufficient conversion time
+may lead to conversion failure. Original DS18B20 and some clones allow for
+detection of invalid conversion. Write bit mask ``1`` to ``features`` to enable
+checking the conversion success. If byte 6 of scratchpad memory is 0xC after
+conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
+power), the driver returns a conversion error. Bit mask ``2`` enables poll for
+conversion completion (normal power only) by generating read cycles on the bus
+after conversion starts. In parasite power mode this feature is not available.
+Feature bit masks may be combined (OR).
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index c1b4eda16719..9b2d96335a70 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -17,6 +17,7 @@
 #include <linux/delay.h>
 #include <linux/hwmon.h>
 #include <linux/string.h>
+#include <linux/jiffies.h>
 
 #include <linux/w1.h>
 
@@ -65,6 +66,20 @@ static u16 bulk_read_device_counter; /* =0 as per C standard */
 #define MIN_TEMP	-55	/* min temperature that can be mesured */
 #define MAX_TEMP	125	/* max temperature that can be mesured */
 
+/* Allowed values for sysfs conv_time attribute */
+#define CONV_TIME_DEFAULT 0
+#define CONV_TIME_MEASURE 1
+
+/* Bits in sysfs "features" value */
+#define W1_THERM_CHECK_RESULT 1	/* Enable conversion success check */
+#define W1_THERM_POLL_COMPLETION 2	/* Poll for conversion completion */
+#define W1_THERM_FEATURES_MASK 3		/* All values mask */
+
+/* Poll period in milliseconds. Should be less then a shortest operation on the device */
+#define W1_POLL_PERIOD 32
+#define W1_POLL_CONVERT_TEMP 2000	/* Timeout for W1_CONVERT_TEMP, ms */
+#define W1_POLL_RECALL_EEPROM 500	/* Timeout for W1_RECALL_EEPROM, ms*/
+
 /* Helpers Macros */
 
 /*
@@ -88,6 +103,20 @@ static u16 bulk_read_device_counter; /* =0 as per C standard */
 #define SLAVE_RESOLUTION(sl) \
 	(((struct w1_therm_family_data *)(sl->family_data))->resolution)
 
+/*
+ * return the conv_time_override of the sl slave
+ * always test family data existence before using this macro
+ */
+ #define SLAVE_CONV_TIME_OVERRIDE(sl) \
+	(((struct w1_therm_family_data *)(sl->family_data))->conv_time_override)
+
+/*
+ * return the features of the sl slave
+ * always test family data existence before using this macro
+ */
+ #define SLAVE_FEATURES(sl) \
+	(((struct w1_therm_family_data *)(sl->family_data))->features)
+
 /*
  * return whether or not a converT command has been issued to the slave
  * * 0: no bulk read is pending
@@ -136,6 +165,8 @@ struct w1_therm_family_converter {
  *				-x error or undefined
  * @resolution: current device resolution
  * @convert_triggered: conversion state of the device
+ * @conv_time_override: user selected conversion time or CONV_TIME_DEFAULT
+ * @features: bit mask - enable temperature validity check, poll for completion
  * @specific_functions: pointer to struct of device specific function
  */
 struct w1_therm_family_data {
@@ -144,6 +175,8 @@ struct w1_therm_family_data {
 	int external_powered;
 	int resolution;
 	int convert_triggered;
+	int conv_time_override;
+	unsigned int features;
 	struct w1_therm_family_converter *specific_functions;
 };
 
@@ -285,6 +318,19 @@ static ssize_t therm_bulk_read_store(struct device *device,
 static ssize_t therm_bulk_read_show(struct device *device,
 	struct device_attribute *attr, char *buf);
 
+static ssize_t conv_time_show(struct device *device,
+			      struct device_attribute *attr, char *buf);
+
+static ssize_t conv_time_store(struct device *device,
+			       struct device_attribute *attr, const char *buf,
+			       size_t size);
+
+static ssize_t features_show(struct device *device,
+			      struct device_attribute *attr, char *buf);
+
+static ssize_t features_store(struct device *device,
+			       struct device_attribute *attr, const char *buf,
+			       size_t size);
 /* Attributes declarations */
 
 static DEVICE_ATTR_RW(w1_slave);
@@ -294,6 +340,8 @@ static DEVICE_ATTR_RO(ext_power);
 static DEVICE_ATTR_RW(resolution);
 static DEVICE_ATTR_WO(eeprom);
 static DEVICE_ATTR_RW(alarms);
+static DEVICE_ATTR_RW(conv_time);
+static DEVICE_ATTR_RW(features);
 
 static DEVICE_ATTR_RW(therm_bulk_read); /* attribut at master level */
 
@@ -328,6 +376,8 @@ static struct attribute *w1_therm_attrs[] = {
 	&dev_attr_resolution.attr,
 	&dev_attr_eeprom.attr,
 	&dev_attr_alarms.attr,
+	&dev_attr_conv_time.attr,
+	&dev_attr_features.attr,
 	NULL,
 };
 
@@ -337,6 +387,8 @@ static struct attribute *w1_ds18s20_attrs[] = {
 	&dev_attr_ext_power.attr,
 	&dev_attr_eeprom.attr,
 	&dev_attr_alarms.attr,
+	&dev_attr_conv_time.attr,
+	&dev_attr_features.attr,
 	NULL,
 };
 
@@ -348,6 +400,8 @@ static struct attribute *w1_ds28ea00_attrs[] = {
 	&dev_attr_resolution.attr,
 	&dev_attr_eeprom.attr,
 	&dev_attr_alarms.attr,
+	&dev_attr_conv_time.attr,
+	&dev_attr_features.attr,
 	NULL,
 };
 
@@ -466,7 +520,10 @@ static inline int w1_DS18B20_convert_time(struct w1_slave *sl)
 	if (!sl->family_data)
 		return -ENODEV;	/* device unknown */
 
-	/* return time in ms for conversion operation */
+	if (SLAVE_CONV_TIME_OVERRIDE(sl) != CONV_TIME_DEFAULT)
+		return SLAVE_CONV_TIME_OVERRIDE(sl);
+
+	/* Return the conversion time from datasheet, depending on resolution */
 	switch (SLAVE_RESOLUTION(sl)) {
 	case 9:
 		ret = 95;
@@ -486,8 +543,13 @@ static inline int w1_DS18B20_convert_time(struct w1_slave *sl)
 
 static inline int w1_DS18S20_convert_time(struct w1_slave *sl)
 {
-	(void)(sl);
-	return 750; /* always 750ms for DS18S20 */
+	if (!sl->family_data)
+		return -ENODEV;	/* device unknown */
+
+	if (SLAVE_CONV_TIME_OVERRIDE(sl) == CONV_TIME_DEFAULT)
+		return 750; /* default for DS18S20 */
+	else
+		return SLAVE_CONV_TIME_OVERRIDE(sl);
 }
 
 static inline int w1_DS18B20_write_data(struct w1_slave *sl,
@@ -507,7 +569,7 @@ static inline int w1_DS18B20_set_resolution(struct w1_slave *sl, int val)
 {
 	int ret;
 	u8 new_config_register[3];	/* array of data to be written */
-	struct therm_info info;
+	struct therm_info info, info2;
 
 	/* resolution of DS18B20 is in the range [9..12] bits */
 	if (val < 9 || val > 12)
@@ -532,8 +594,18 @@ static inline int w1_DS18B20_set_resolution(struct w1_slave *sl, int val)
 
 	/* Write data in the device RAM */
 	ret = w1_DS18B20_write_data(sl, new_config_register);
+	if (ret)
+		return ret;
 
-	return ret;
+	/* Some DS18B20 clones don't support resolution change, read back to verify */
+	ret = read_scratchpad(sl, &info2);
+	if (ret)
+		return ret;
+
+	if ((info2.rom[4] & 0x9F) == (info.rom[4] & 0x9F))
+		return 0;
+	else
+		return -EIO;
 }
 
 static inline int w1_DS18B20_get_resolution(struct w1_slave *sl)
@@ -699,6 +771,22 @@ static inline bool bus_mutex_lock(struct mutex *lock)
 	return true;
 }
 
+/**
+ * check_family_data() - Check if family data and specific functions are present
+ * @sl: W1 device data
+ *
+ * Return: 0 - OK, negative value - error
+ */
+static int check_family_data(struct w1_slave *sl)
+{
+	if ((!sl->family_data) || (!SLAVE_SPECIFIC_FUNC(sl))) {
+		dev_info(&sl->dev,
+			 "%s: Device is not supported by the driver\n", __func__);
+		return -EINVAL;  /* No device family */
+	}
+	return 0;
+}
+
 /**
  * support_bulk_read() - check if slave support bulk read
  * @sl: device to check the ability
@@ -883,6 +971,34 @@ static int reset_select_slave(struct w1_slave *sl)
 	return 0;
 }
 
+/**
+ * w1_poll_completion - Poll for operation completion, with timeout
+ * @dev_master: the device master of the bus
+ * @tout_ms: timeout in milliseconds
+ *
+ * The device is answering 0's while an operation is in progress and 1's after it completes
+ * Timeout may happen if the previous command was not recognised due to a line noise
+ *
+ * Return: 0 - OK, negative error - timeout
+ */
+int w1_poll_completion(struct w1_master *dev_master, int tout_ms)
+{
+	int i;
+
+	for (i = 0; i < tout_ms/W1_POLL_PERIOD; i++) {
+		/* Delay is before poll, for device to recognize a command */
+		msleep(W1_POLL_PERIOD);
+
+		/* Compare all 8 bits to mitigate a noise on the bus */
+		if (w1_read_8(dev_master) == 0xFF)
+			break;
+	}
+	if (i == tout_ms/W1_POLL_PERIOD)
+		return -EIO;
+
+	return 0;
+}
+
 static int convert_t(struct w1_slave *sl, struct therm_info *info)
 {
 	struct w1_master *dev_master = sl->master;
@@ -898,6 +1014,13 @@ static int convert_t(struct w1_slave *sl, struct therm_info *info)
 					(!SLAVE_POWERMODE(sl) &&
 					w1_strong_pullup));
 
+	if (strong_pullup && SLAVE_FEATURES(sl) & W1_THERM_POLL_COMPLETION) {
+		dev_warn(&sl->dev,
+			"%s: Disabling W1_THERM_POLL_COMPLETION in parasite power mode.\n",
+			__func__);
+		SLAVE_FEATURES(sl) &= ~W1_THERM_POLL_COMPLETION;
+	}
+
 	/* get conversion duration device and id dependent */
 	t_conv = conversion_time(sl);
 
@@ -933,15 +1056,38 @@ static int convert_t(struct w1_slave *sl, struct therm_info *info)
 				}
 				mutex_unlock(&dev_master->bus_mutex);
 			} else { /*no device need pullup */
-				mutex_unlock(&dev_master->bus_mutex);
-
-				sleep_rem = msleep_interruptible(t_conv);
-				if (sleep_rem != 0) {
-					ret = -EINTR;
-					goto dec_refcnt;
+				if (SLAVE_FEATURES(sl) & W1_THERM_POLL_COMPLETION) {
+					ret = w1_poll_completion(dev_master, W1_POLL_CONVERT_TEMP);
+					if (ret) {
+						dev_dbg(&sl->dev, "%s: Timeout\n", __func__);
+						goto mt_unlock;
+					}
+					mutex_unlock(&dev_master->bus_mutex);
+				} else {
+					/* Fixed delay */
+					mutex_unlock(&dev_master->bus_mutex);
+					sleep_rem = msleep_interruptible(t_conv);
+					if (sleep_rem != 0) {
+						ret = -EINTR;
+						goto dec_refcnt;
+					}
 				}
 			}
 			ret = read_scratchpad(sl, info);
+
+			/* If enabled, check for conversion success */
+			if ((SLAVE_FEATURES(sl) & W1_THERM_CHECK_RESULT) &&
+				(info->rom[6] == 0xC) &&
+				((info->rom[1] == 0x5 && info->rom[0] == 0x50) ||
+				(info->rom[1] == 0x7 && info->rom[0] == 0xFF))
+			) {
+				/* Invalid reading (scratchpad byte 6 = 0xC)
+				 * due to insufficient conversion time
+				 * or power failure.
+				 */
+				ret = -EIO;
+			}
+
 			goto dec_refcnt;
 		}
 
@@ -955,6 +1101,76 @@ static int convert_t(struct w1_slave *sl, struct therm_info *info)
 	return ret;
 }
 
+static int conv_time_measure(struct w1_slave *sl, int *conv_time)
+{
+	struct therm_info inf,
+		*info = &inf;
+	struct w1_master *dev_master = sl->master;
+	int max_trying = W1_THERM_MAX_TRY;
+	int ret = -ENODEV;
+	bool strong_pullup;
+
+	if (!sl->family_data)
+		goto error;
+
+	strong_pullup = (w1_strong_pullup == 2 ||
+		(!SLAVE_POWERMODE(sl) &&
+		w1_strong_pullup));
+
+	if (strong_pullup) {
+		pr_info("%s: Measure with strong_pullup is not supported.\n", __func__);
+		return -EINVAL;
+	}
+
+	memset(info->rom, 0, sizeof(info->rom));
+
+	/* prevent the slave from going away in sleep */
+	atomic_inc(THERM_REFCNT(sl->family_data));
+
+	if (!bus_mutex_lock(&dev_master->bus_mutex)) {
+		ret = -EAGAIN;	/* Didn't acquire the mutex */
+		goto dec_refcnt;
+	}
+
+	while (max_trying-- && ret) { /* ret should be 0 */
+		info->verdict = 0;
+		info->crc = 0;
+		/* safe version to select slave */
+		if (!reset_select_slave(sl)) {
+			int j_start, j_end;
+
+			/*no device need pullup */
+			w1_write_8(dev_master, W1_CONVERT_TEMP);
+
+			j_start = jiffies;
+			ret = w1_poll_completion(dev_master, W1_POLL_CONVERT_TEMP);
+			if (ret) {
+				dev_dbg(&sl->dev, "%s: Timeout\n", __func__);
+				goto mt_unlock;
+			}
+			j_end = jiffies;
+			/* 1.2x increase for variation and changes over temperature range */
+			*conv_time = jiffies_to_msecs(j_end-j_start)*12/10;
+			pr_debug("W1 Measure complete, conv_time = %d, HZ=%d.\n",
+				*conv_time, HZ);
+			if (*conv_time <= CONV_TIME_MEASURE) {
+				ret = -EIO;
+				goto mt_unlock;
+			}
+			mutex_unlock(&dev_master->bus_mutex);
+			ret = read_scratchpad(sl, info);
+			goto dec_refcnt;
+		}
+
+	}
+mt_unlock:
+	mutex_unlock(&dev_master->bus_mutex);
+dec_refcnt:
+	atomic_dec(THERM_REFCNT(sl->family_data));
+error:
+	return ret;
+}
+
 static int read_scratchpad(struct w1_slave *sl, struct therm_info *info)
 {
 	struct w1_master *dev_master = sl->master;
@@ -1118,10 +1334,7 @@ static int recall_eeprom(struct w1_slave *sl)
 		if (!reset_select_slave(sl)) {
 
 			w1_write_8(dev_master, W1_RECALL_EEPROM);
-
-			ret = 1; /* Slave will pull line to 0 */
-			while (ret)
-				ret = 1 - w1_touch_bit(dev_master, 1);
+			ret = w1_poll_completion(dev_master, W1_POLL_RECALL_EEPROM);
 		}
 
 	}
@@ -1345,11 +1558,13 @@ static ssize_t w1_slave_store(struct device *device,
 	}
 
 	if (ret) {
-		dev_info(device,
-			"%s: writing error %d\n", __func__, ret);
-		/* return size to avoid call back again */
-	} else
-		SLAVE_RESOLUTION(sl) = val;
+		dev_warn(device, "%s: Set resolution - error %d\n", __func__, ret);
+		/* Propagate error to userspace */
+		return ret;
+	}
+	SLAVE_RESOLUTION(sl) = val;
+	/* Reset the conversion time to default - it depends on resolution */
+	SLAVE_CONV_TIME_OVERRIDE(sl) = CONV_TIME_DEFAULT;
 
 	return size; /* always return size to avoid infinite calling */
 }
@@ -1465,12 +1680,12 @@ static ssize_t resolution_store(struct device *device,
 	/* get the correct function depending on the device */
 	ret = SLAVE_SPECIFIC_FUNC(sl)->set_resolution(sl, val);
 
-	if (ret) {
-		dev_info(device,
-			"%s: writing error %d\n", __func__, ret);
-		/* return size to avoid call back again */
-	} else
-		SLAVE_RESOLUTION(sl) = val;
+	if (ret)
+		return ret;
+
+	SLAVE_RESOLUTION(sl) = val;
+	/* Reset the conversion time to default because it depends on resolution */
+	SLAVE_CONV_TIME_OVERRIDE(sl) = CONV_TIME_DEFAULT;
 
 	return size;
 }
@@ -1660,6 +1875,96 @@ static ssize_t therm_bulk_read_show(struct device *device,
 	return sprintf(buf, "%d\n", ret);
 }
 
+static ssize_t conv_time_show(struct device *device,
+	struct device_attribute *attr, char *buf)
+{
+	struct w1_slave *sl = dev_to_w1_slave(device);
+
+	if ((!sl->family_data) || (!SLAVE_SPECIFIC_FUNC(sl))) {
+		dev_info(device,
+			"%s: Device is not supported by the driver\n", __func__);
+		return 0;  /* No device family */
+	}
+	return sprintf(buf, "%d\n", conversion_time(sl));
+}
+
+static ssize_t conv_time_store(struct device *device,
+	struct device_attribute *attr, const char *buf, size_t size)
+{
+	int val, ret = 0;
+	struct w1_slave *sl = dev_to_w1_slave(device);
+
+	if (kstrtoint(buf, 10, &val)) /* converting user entry to int */
+		return -EINVAL;
+
+	if (check_family_data(sl))
+		return -ENODEV;
+
+	if (val != CONV_TIME_MEASURE) {
+		if (val >= CONV_TIME_DEFAULT)
+			SLAVE_CONV_TIME_OVERRIDE(sl) = val;
+		else
+			return -EINVAL;
+
+	} else {
+		int conv_time;
+
+		ret = conv_time_measure(sl, &conv_time);
+		if (ret)
+			return -EIO;
+		SLAVE_CONV_TIME_OVERRIDE(sl) = conv_time;
+	}
+	return size;
+}
+
+static ssize_t features_show(struct device *device,
+			     struct device_attribute *attr, char *buf)
+{
+	struct w1_slave *sl = dev_to_w1_slave(device);
+
+	if ((!sl->family_data) || (!SLAVE_SPECIFIC_FUNC(sl))) {
+		dev_info(device,
+			 "%s: Device not supported by the driver\n", __func__);
+		return 0;  /* No device family */
+	}
+	return sprintf(buf, "%u\n", SLAVE_FEATURES(sl));
+}
+
+static ssize_t features_store(struct device *device,
+			      struct device_attribute *attr, const char *buf, size_t size)
+{
+	int val, ret = 0;
+	bool strong_pullup;
+	struct w1_slave *sl = dev_to_w1_slave(device);
+
+	ret = kstrtouint(buf, 10, &val); /* converting user entry to int */
+	if (ret)
+		return -EINVAL;  /* invalid number */
+
+	if ((!sl->family_data) || (!SLAVE_SPECIFIC_FUNC(sl))) {
+		dev_info(device, "%s: Device not supported by the driver\n", __func__);
+		return -ENODEV;
+	}
+
+	if ((val & W1_THERM_FEATURES_MASK) != val)
+		return -EINVAL;
+
+	SLAVE_FEATURES(sl) = val;
+
+	strong_pullup = (w1_strong_pullup == 2 ||
+			 (!SLAVE_POWERMODE(sl) &&
+			  w1_strong_pullup));
+
+	if (strong_pullup && SLAVE_FEATURES(sl) & W1_THERM_POLL_COMPLETION) {
+		dev_warn(&sl->dev,
+			 "%s: W1_THERM_POLL_COMPLETION disabled in parasite power mode.\n",
+			 __func__);
+		SLAVE_FEATURES(sl) &= ~W1_THERM_POLL_COMPLETION;
+	}
+
+	return size;
+}
+
 #if IS_REACHABLE(CONFIG_HWMON)
 static int w1_read_temp(struct device *device, u32 attr, int channel,
 			long *val)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-09-04 16:00 [PATCH 1/2] w1: w1_therm: Add sysfs entries to control conversion time and driver features Ivan Zaentsev
@ 2020-09-04 16:00 ` Ivan Zaentsev
  2020-10-06 13:19   ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 14+ messages in thread
From: Ivan Zaentsev @ 2020-09-04 16:00 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: Jonathan Corbet, Akira Shimahara, Greg Kroah-Hartman,
	Mauro Carvalho Chehab, Dan Carpenter, Colin Ian King,
	linux-kernel, linux-doc, Evgeny Boger, Ivan Zaentsev

GX20MH01 device shares family number 0x28 with DS18B20. The device
is generally compatible with DS18B20. Added are the lowest 2^-5, 2^-6
temperature bits in Config register; R2 bit in Config register
enabling 13 and 14 bit resolutions. It is powered up in 14 bit mode.

Signed-off-by: Ivan Zaentsev <ivan.zaentsev@wirenboard.ru>
---
 .../ABI/testing/sysfs-driver-w1_therm         |   4 +-
 Documentation/w1/slaves/w1_therm.rst          |  15 ++-
 drivers/w1/slaves/w1_therm.c                  | 106 +++++++++++++-----
 3 files changed, 92 insertions(+), 33 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-w1_therm b/Documentation/ABI/testing/sysfs-driver-w1_therm
index b44b51a88c5e..9f05bcdcd762 100644
--- a/Documentation/ABI/testing/sysfs-driver-w1_therm
+++ b/Documentation/ABI/testing/sysfs-driver-w1_therm
@@ -49,7 +49,7 @@ Description:
 		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
+			* '9..14': device resolution in bit
 			or resolution to set in bit
 			* '-xx': xx is kernel error when reading the resolution
 			* Anything else: do nothing
@@ -89,7 +89,7 @@ Description:
 		*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
+			* '9..14' : set the device resolution in RAM
 			(if supported)
 			* Anything else: do nothing
 		refer to Documentation/w1/slaves/w1_therm.rst for detailed
diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
index f1148181f53e..00376501a5ef 100644
--- a/Documentation/w1/slaves/w1_therm.rst
+++ b/Documentation/w1/slaves/w1_therm.rst
@@ -6,6 +6,7 @@ Supported chips:
 
   * Maxim ds18*20 based temperature sensors.
   * Maxim ds1825 based temperature sensors.
+  * GXCAS GC20MH01 temperature sensor.
 
 Author: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
 
@@ -13,8 +14,8 @@ Author: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
 Description
 -----------
 
-w1_therm provides basic temperature conversion for ds18*20 devices, and the
-ds28ea00 device.
+w1_therm provides basic temperature conversion for ds18*20, ds28ea00, GX20MH01
+devices.
 
 Supported family codes:
 
@@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
 power), the driver returns a conversion error. Bit mask ``2`` enables poll for
 conversion completion (normal power only) by generating read cycles on the bus
 after conversion starts. In parasite power mode this feature is not available.
-Feature bit masks may be combined (OR).
+Feature bit masks may be combined (OR). See accompanying sysfs documentation:
+:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
+
+GX20MH01 device shares family number 0x28 with DS18*20. The device is generally
+compatible with DS18B20. Added are lowest 2^-5, 2^-6 temperature bits in Config
+register; R2 bit in Config register enabling 13 and 14 bit resolutions. The
+device is powered up in 14-bit resolution mode. The conversion times specified
+in the datasheet are too low and have to be increased. The device supports
+driver features ``1`` and ``2``.
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 9b2d96335a70..f6b0e0320ffc 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -80,6 +80,18 @@ static u16 bulk_read_device_counter; /* =0 as per C standard */
 #define W1_POLL_CONVERT_TEMP 2000	/* Timeout for W1_CONVERT_TEMP, ms */
 #define W1_POLL_RECALL_EEPROM 500	/* Timeout for W1_RECALL_EEPROM, ms*/
 
+/* Masks for resolution functions, work with all devices */
+/* Bit mask for config register for all devices, bits 7,6,5 */
+#define W1_THERM_RESOLUTION_MASK 0xE0
+/* Bit offset of resolution in config register for all devices */
+#define W1_THERM_RESOLUTION_SHIFT 5
+/* Bit offset of resolution in config register for all devices */
+#define W1_THERM_RESOLUTION_SHIFT 5
+/* Add this to bit value to get resolution */
+#define W1_THERM_RESOLUTION_MIN 9
+/* Maximum allowed value */
+#define W1_THERM_RESOLUTION_MAX 14
+
 /* Helpers Macros */
 
 /*
@@ -523,7 +535,9 @@ static inline int w1_DS18B20_convert_time(struct w1_slave *sl)
 	if (SLAVE_CONV_TIME_OVERRIDE(sl) != CONV_TIME_DEFAULT)
 		return SLAVE_CONV_TIME_OVERRIDE(sl);
 
-	/* Return the conversion time from datasheet, depending on resolution */
+	/* Return the conversion time, depending on resolution,
+	 * select maximum conversion time among all compatible devices
+	 */
 	switch (SLAVE_RESOLUTION(sl)) {
 	case 9:
 		ret = 95;
@@ -535,6 +549,14 @@ static inline int w1_DS18B20_convert_time(struct w1_slave *sl)
 		ret = 375;
 		break;
 	case 12:
+		ret = 750;
+		break;
+	case 13:
+		ret = 850;  /* GX20MH01 only. Datasheet says 500ms, but that's not enough. */
+		break;
+	case 14:
+		ret = 1600; /* GX20MH01 only. Datasheet says 1000ms - not enough */
+		break;
 	default:
 		ret = 750;
 	}
@@ -568,62 +590,71 @@ static inline int w1_DS18S20_write_data(struct w1_slave *sl,
 static inline int w1_DS18B20_set_resolution(struct w1_slave *sl, int val)
 {
 	int ret;
-	u8 new_config_register[3];	/* array of data to be written */
 	struct therm_info info, info2;
 
-	/* resolution of DS18B20 is in the range [9..12] bits */
-	if (val < 9 || val > 12)
+	/* DS18B20 resolution is 9 to 12 bits */
+	/* GX20MH01 resolution is 9 to 14 bits */
+	if (val < W1_THERM_RESOLUTION_MIN || val > W1_THERM_RESOLUTION_MAX)
 		return -EINVAL;
 
-	val -= 9; /* soustract 9 the lowest resolution in bit */
-	val = (val << 5); /* shift to position bit 5 & bit 6 */
+	/* Calc bit value from resolution */
+	val = (val - W1_THERM_RESOLUTION_MIN) << W1_THERM_RESOLUTION_SHIFT;
 
 	/*
 	 * Read the scratchpad to change only the required bits
 	 * (bit5 & bit 6 from byte 4)
 	 */
 	ret = read_scratchpad(sl, &info);
-	if (!ret) {
-		new_config_register[0] = info.rom[2];
-		new_config_register[1] = info.rom[3];
-		/* config register is byte 4 & mask 0b10011111*/
-		new_config_register[2] = (info.rom[4] & 0x9F) |
-					(u8) val;
-	} else
+
+	if (ret)
 		return ret;
 
+
+	info.rom[4] &= ~W1_THERM_RESOLUTION_MASK;
+	info.rom[4] |= val;
+
 	/* Write data in the device RAM */
-	ret = w1_DS18B20_write_data(sl, new_config_register);
+	ret = w1_DS18B20_write_data(sl, info.rom + 2);
 	if (ret)
 		return ret;
 
-	/* Some DS18B20 clones don't support resolution change, read back to verify */
+	/* Have to read back the resolution to verify an actual value
+	 * GX20MH01 and DS18B20 are indistinguishable by family number, but resolutions differ
+	 * Some DS18B20 clones don't support resolution change
+	 */
 	ret = read_scratchpad(sl, &info2);
 	if (ret)
+		/* Scratchpad read fail */
 		return ret;
 
-	if ((info2.rom[4] & 0x9F) == (info.rom[4] & 0x9F))
+	if ((info2.rom[4] & W1_THERM_RESOLUTION_MASK) == (info.rom[4] & W1_THERM_RESOLUTION_MASK))
 		return 0;
-	else
-		return -EIO;
+
+	/* Resolution verify error */
+	return -EIO;
 }
 
 static inline int w1_DS18B20_get_resolution(struct w1_slave *sl)
 {
 	int ret;
-	u8 config_register;
+	int resolution;
 	struct therm_info info;
 
 	ret = read_scratchpad(sl, &info);
 
-	if (!ret)	{
-		config_register = info.rom[4]; /* config register is byte 4 */
-		config_register &= 0x60; /* 0b01100000 keep only bit 5 & 6 */
-		config_register = (config_register >> 5);	/* shift */
-		config_register += 9; /* add 9 the lowest resolution in bit */
-		ret = (int) config_register;
-	}
-	return ret;
+	if (ret)
+		return ret;
+
+	resolution = ((info.rom[4] & W1_THERM_RESOLUTION_MASK) >> W1_THERM_RESOLUTION_SHIFT)
+		+ W1_THERM_RESOLUTION_MIN;
+	/* GX20MH01 has one special case:
+	 *   >=14 means 14 bits when getting resolution from bit value.
+	 * Other devices have no more then 12 bits.
+	 */
+	if (resolution > W1_THERM_RESOLUTION_MAX)
+		resolution = W1_THERM_RESOLUTION_MAX;
+
+	return resolution;
 }
 
 /**
@@ -636,11 +667,28 @@ static inline int w1_DS18B20_get_resolution(struct w1_slave *sl)
  */
 static inline int w1_DS18B20_convert_temp(u8 rom[9])
 {
-	s16 t = le16_to_cpup((__le16 *)rom);
+	int t;
+	u32 bv;
+
+	/* Config register bit R2 = 1 - GX20MH01 in 13 or 14 bit resolution mode */
+	if (rom[4] & 0x80) {
+		/* Signed 16-bit value to unsigned, cpu order */
+		bv = le16_to_cpup((__le16 *)rom);
+
+		/* Insert two temperature bits from config register */
+		/* Avoid arithmetic shift of signed value */
+		bv = (bv << 2) | (rom[4] & 3);
 
+		t = (int) sign_extend32(bv, 17); /* Degrees, lowest bit is 2^-6 */
+		return (t*1000)/64;  /* Millidegrees */
+	}
+
+	t = (int)le16_to_cpup((__le16 *)rom);
 	return t*1000/16;
 }
 
+
+
 /**
  * w1_DS18S20_convert_temp() - temperature computation for DS18S20
  * @rom: data read from device RAM (8 data bytes + 1 CRC byte)
@@ -672,6 +720,7 @@ static inline int w1_DS18S20_convert_temp(u8 rom[9])
 }
 
 /* Device capability description */
+/* GX20MH01 device shares family number and structure with DS18B20 */
 
 static struct w1_therm_family_converter w1_therm_families[] = {
 	{
@@ -693,6 +742,7 @@ static struct w1_therm_family_converter w1_therm_families[] = {
 		.bulk_read			= true
 	},
 	{
+		/* Also used for GX20MH01 */
 		.f				= &w1_therm_family_DS18B20,
 		.convert			= w1_DS18B20_convert_temp,
 		.get_conversion_time	= w1_DS18B20_convert_time,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-09-04 16:00 ` [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device Ivan Zaentsev
@ 2020-10-06 13:19   ` Mauro Carvalho Chehab
  2020-10-07  7:32     ` Ivan Zaentsev
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2020-10-06 13:19 UTC (permalink / raw)
  To: Ivan Zaentsev
  Cc: Evgeniy Polyakov, Jonathan Corbet, Akira Shimahara,
	Greg Kroah-Hartman, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

Em Fri,  4 Sep 2020 19:00:04 +0300
Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:

> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> index f1148181f53e..00376501a5ef 100644
> --- a/Documentation/w1/slaves/w1_therm.rst
> +++ b/Documentation/w1/slaves/w1_therm.rst

>  
> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
>  conversion completion (normal power only) by generating read cycles on the bus
>  after conversion starts. In parasite power mode this feature is not available.
> -Feature bit masks may be combined (OR).
> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> +

As warned by Sphinx, this cross-reference is broken:

	.../Documentation/w1/slaves/w1_therm.rst:125: WARNING: undefined label: w1_therm (if the link has no caption the label must precede a section header)

Not sure what you wanted to point here.


Thanks,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-06 13:19   ` Mauro Carvalho Chehab
@ 2020-10-07  7:32     ` Ivan Zaentsev
  2020-10-07  8:57       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 14+ messages in thread
From: Ivan Zaentsev @ 2020-10-07  7:32 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Evgeniy Polyakov, Jonathan Corbet, Akira Shimahara,
	Greg Kroah-Hartman, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:

>> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
>> index f1148181f53e..00376501a5ef 100644
>> --- a/Documentation/w1/slaves/w1_therm.rst
>> +++ b/Documentation/w1/slaves/w1_therm.rst

>>  
>> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
>>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
>>  conversion completion (normal power only) by generating read cycles on the bus
>>  after conversion starts. In parasite power mode this feature is not available.
>> -Feature bit masks may be combined (OR).
>> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
>> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
>> +

> As warned by Sphinx, this cross-reference is broken:

>         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> undefined label: w1_therm (if the link has no caption the label must precede a section header)

Would this be ok?

"More details in Documentation/ABI/testing/sysfs-driver-w1_therm"

> Not sure what you wanted to point here.

A link to a driver's sysfs interface, but sysfs docs are text
files and seem to not be included in Sphynx Docs.

-- 
Best regards,
 Ivan                            mailto:ivan.zaentsev@wirenboard.ru


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-07  7:32     ` Ivan Zaentsev
@ 2020-10-07  8:57       ` Mauro Carvalho Chehab
  2020-10-07  9:06         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2020-10-07  8:57 UTC (permalink / raw)
  To: Ivan Zaentsev
  Cc: Evgeniy Polyakov, Jonathan Corbet, Akira Shimahara,
	Greg Kroah-Hartman, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

Em Wed, 7 Oct 2020 10:32:27 +0300
Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:

> Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:
> 
> >> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> >> index f1148181f53e..00376501a5ef 100644
> >> --- a/Documentation/w1/slaves/w1_therm.rst
> >> +++ b/Documentation/w1/slaves/w1_therm.rst  
> 
> >>  
> >> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
> >>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
> >>  conversion completion (normal power only) by generating read cycles on the bus
> >>  after conversion starts. In parasite power mode this feature is not available.
> >> -Feature bit masks may be combined (OR).
> >> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> >> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> >> +  
> 
> > As warned by Sphinx, this cross-reference is broken:  
> 
> >         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> > undefined label: w1_therm (if the link has no caption the label must precede a section header)  
> 
> Would this be ok?

Yeah, sure!

> 
> "More details in Documentation/ABI/testing/sysfs-driver-w1_therm"
> 
> > Not sure what you wanted to point here.  
> 
> A link to a driver's sysfs interface, but sysfs docs are text
> files and seem to not be included in Sphynx Docs.

I sent upstream sometime ago a patch series adding ABI to Sphinx, but I 
was not merged, not sure why:

	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v5.6

Yet, the approach such series took were not to convert ABI files
to ReST, but, instead, to use a script that would do that.

The rationale were to avoid needing to touch all ABI files at
the same time[1]. 

In any case, with such approach a cross-reference won't work.

So, I guess that the next best thing to do is to just mention
the file like what you suggested.

[1] It would be easy to run the script I wrote to convert the
files to ReST directly.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-07  8:57       ` Mauro Carvalho Chehab
@ 2020-10-07  9:06         ` Greg Kroah-Hartman
  2020-10-07 11:05           ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-07  9:06 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Ivan Zaentsev, Evgeniy Polyakov, Jonathan Corbet,
	Akira Shimahara, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

On Wed, Oct 07, 2020 at 10:57:02AM +0200, Mauro Carvalho Chehab wrote:
> Em Wed, 7 Oct 2020 10:32:27 +0300
> Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:
> 
> > Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:
> > 
> > >> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> > >> index f1148181f53e..00376501a5ef 100644
> > >> --- a/Documentation/w1/slaves/w1_therm.rst
> > >> +++ b/Documentation/w1/slaves/w1_therm.rst  
> > 
> > >>  
> > >> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
> > >>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
> > >>  conversion completion (normal power only) by generating read cycles on the bus
> > >>  after conversion starts. In parasite power mode this feature is not available.
> > >> -Feature bit masks may be combined (OR).
> > >> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> > >> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> > >> +  
> > 
> > > As warned by Sphinx, this cross-reference is broken:  
> > 
> > >         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> > > undefined label: w1_therm (if the link has no caption the label must precede a section header)  
> > 
> > Would this be ok?
> 
> Yeah, sure!
> 
> > 
> > "More details in Documentation/ABI/testing/sysfs-driver-w1_therm"
> > 
> > > Not sure what you wanted to point here.  
> > 
> > A link to a driver's sysfs interface, but sysfs docs are text
> > files and seem to not be included in Sphynx Docs.
> 
> I sent upstream sometime ago a patch series adding ABI to Sphinx, but I 
> was not merged, not sure why:
> 
> 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v5.6

I think the raft of different patches floating around at the time made
me totally confused as to what was, and was not, the latest versions.

I'll be glad to look at them again, if you want to rebase after 5.10-rc1
is out and resend them, as I think this should be showing up in the
documentation.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-07  9:06         ` Greg Kroah-Hartman
@ 2020-10-07 11:05           ` Mauro Carvalho Chehab
  2020-10-07 11:43             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2020-10-07 11:05 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Ivan Zaentsev, Evgeniy Polyakov, Jonathan Corbet,
	Akira Shimahara, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

Em Wed, 7 Oct 2020 11:06:19 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:

> On Wed, Oct 07, 2020 at 10:57:02AM +0200, Mauro Carvalho Chehab wrote:
> > Em Wed, 7 Oct 2020 10:32:27 +0300
> > Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:
> >   
> > > Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:
> > >   
> > > >> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> > > >> index f1148181f53e..00376501a5ef 100644
> > > >> --- a/Documentation/w1/slaves/w1_therm.rst
> > > >> +++ b/Documentation/w1/slaves/w1_therm.rst    
> > >   
> > > >>  
> > > >> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
> > > >>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
> > > >>  conversion completion (normal power only) by generating read cycles on the bus
> > > >>  after conversion starts. In parasite power mode this feature is not available.
> > > >> -Feature bit masks may be combined (OR).
> > > >> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> > > >> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> > > >> +    
> > >   
> > > > As warned by Sphinx, this cross-reference is broken:    
> > >   
> > > >         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> > > > undefined label: w1_therm (if the link has no caption the label must precede a section header)    
> > > 
> > > Would this be ok?  
> > 
> > Yeah, sure!
> >   
> > > 
> > > "More details in Documentation/ABI/testing/sysfs-driver-w1_therm"
> > >   
> > > > Not sure what you wanted to point here.    
> > > 
> > > A link to a driver's sysfs interface, but sysfs docs are text
> > > files and seem to not be included in Sphynx Docs.  
> > 
> > I sent upstream sometime ago a patch series adding ABI to Sphinx, but I 
> > was not merged, not sure why:
> > 
> > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v5.6  
> 
> I think the raft of different patches floating around at the time made
> me totally confused as to what was, and was not, the latest versions.

Yeah, there were lots of patches floating around that time.

I also recall that someone (Jeni?) asked if the best wouldn't be to
just convert the ABI files to ReST directly.

> I'll be glad to look at them again, if you want to rebase after 5.10-rc1
> is out and resend them, as I think this should be showing up in the
> documentation.

Surely. I'll rebase them after 5.10-rc1 and re-submit. 

What strategy do you prefer? Keep the files with the same format as
today (allowing them to optionally have ReST markups) or to convert
them to .rst directly?

In the latter case, the best would be to apply it as early as possible
after 5.10-rc1, as it may cause conflicts with other patches being
submitted for 5.11.

In the former case, as there are a few other places like w1_therm that want
cross-references to ABI, I'll try to automate a way to generate identifiers
for processed ABI files.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-07 11:05           ` Mauro Carvalho Chehab
@ 2020-10-07 11:43             ` Greg Kroah-Hartman
  2020-10-07 11:59               ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-07 11:43 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Ivan Zaentsev, Evgeniy Polyakov, Jonathan Corbet,
	Akira Shimahara, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

On Wed, Oct 07, 2020 at 01:05:49PM +0200, Mauro Carvalho Chehab wrote:
> Em Wed, 7 Oct 2020 11:06:19 +0200
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> 
> > On Wed, Oct 07, 2020 at 10:57:02AM +0200, Mauro Carvalho Chehab wrote:
> > > Em Wed, 7 Oct 2020 10:32:27 +0300
> > > Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:
> > >   
> > > > Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:
> > > >   
> > > > >> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> > > > >> index f1148181f53e..00376501a5ef 100644
> > > > >> --- a/Documentation/w1/slaves/w1_therm.rst
> > > > >> +++ b/Documentation/w1/slaves/w1_therm.rst    
> > > >   
> > > > >>  
> > > > >> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
> > > > >>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
> > > > >>  conversion completion (normal power only) by generating read cycles on the bus
> > > > >>  after conversion starts. In parasite power mode this feature is not available.
> > > > >> -Feature bit masks may be combined (OR).
> > > > >> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> > > > >> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> > > > >> +    
> > > >   
> > > > > As warned by Sphinx, this cross-reference is broken:    
> > > >   
> > > > >         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> > > > > undefined label: w1_therm (if the link has no caption the label must precede a section header)    
> > > > 
> > > > Would this be ok?  
> > > 
> > > Yeah, sure!
> > >   
> > > > 
> > > > "More details in Documentation/ABI/testing/sysfs-driver-w1_therm"
> > > >   
> > > > > Not sure what you wanted to point here.    
> > > > 
> > > > A link to a driver's sysfs interface, but sysfs docs are text
> > > > files and seem to not be included in Sphynx Docs.  
> > > 
> > > I sent upstream sometime ago a patch series adding ABI to Sphinx, but I 
> > > was not merged, not sure why:
> > > 
> > > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v5.6  
> > 
> > I think the raft of different patches floating around at the time made
> > me totally confused as to what was, and was not, the latest versions.
> 
> Yeah, there were lots of patches floating around that time.
> 
> I also recall that someone (Jeni?) asked if the best wouldn't be to
> just convert the ABI files to ReST directly.
> 
> > I'll be glad to look at them again, if you want to rebase after 5.10-rc1
> > is out and resend them, as I think this should be showing up in the
> > documentation.
> 
> Surely. I'll rebase them after 5.10-rc1 and re-submit. 
> 
> What strategy do you prefer? Keep the files with the same format as
> today (allowing them to optionally have ReST markups) or to convert
> them to .rst directly?
> 
> In the latter case, the best would be to apply it as early as possible
> after 5.10-rc1, as it may cause conflicts with other patches being
> submitted for 5.11.

The existing format if at all possible, doing wholesale changes is a
mess and wouldn't be recommended.

I think you already fixed up the entries that had problems being parsed
in the past, if not, we can resolve those as well.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-07 11:43             ` Greg Kroah-Hartman
@ 2020-10-07 11:59               ` Mauro Carvalho Chehab
  2020-10-21 16:28                 ` Adding ABI to htmldocs - Was: " Mauro Carvalho Chehab
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2020-10-07 11:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Ivan Zaentsev, Evgeniy Polyakov, Jonathan Corbet,
	Akira Shimahara, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

Em Wed, 7 Oct 2020 13:43:59 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:

> On Wed, Oct 07, 2020 at 01:05:49PM +0200, Mauro Carvalho Chehab wrote:
> > Em Wed, 7 Oct 2020 11:06:19 +0200
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> >   
> > > On Wed, Oct 07, 2020 at 10:57:02AM +0200, Mauro Carvalho Chehab wrote:  
> > > > Em Wed, 7 Oct 2020 10:32:27 +0300
> > > > Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:
> > > >     
> > > > > Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:
> > > > >     
> > > > > >> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> > > > > >> index f1148181f53e..00376501a5ef 100644
> > > > > >> --- a/Documentation/w1/slaves/w1_therm.rst
> > > > > >> +++ b/Documentation/w1/slaves/w1_therm.rst      
> > > > >     
> > > > > >>  
> > > > > >> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
> > > > > >>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
> > > > > >>  conversion completion (normal power only) by generating read cycles on the bus
> > > > > >>  after conversion starts. In parasite power mode this feature is not available.
> > > > > >> -Feature bit masks may be combined (OR).
> > > > > >> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> > > > > >> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> > > > > >> +      
> > > > >     
> > > > > > As warned by Sphinx, this cross-reference is broken:      
> > > > >     
> > > > > >         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> > > > > > undefined label: w1_therm (if the link has no caption the label must precede a section header)      
> > > > > 
> > > > > Would this be ok?    
> > > > 
> > > > Yeah, sure!
> > > >     
> > > > > 
> > > > > "More details in Documentation/ABI/testing/sysfs-driver-w1_therm"
> > > > >     
> > > > > > Not sure what you wanted to point here.      
> > > > > 
> > > > > A link to a driver's sysfs interface, but sysfs docs are text
> > > > > files and seem to not be included in Sphynx Docs.    
> > > > 
> > > > I sent upstream sometime ago a patch series adding ABI to Sphinx, but I 
> > > > was not merged, not sure why:
> > > > 
> > > > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v5.6    
> > > 
> > > I think the raft of different patches floating around at the time made
> > > me totally confused as to what was, and was not, the latest versions.  
> > 
> > Yeah, there were lots of patches floating around that time.
> > 
> > I also recall that someone (Jeni?) asked if the best wouldn't be to
> > just convert the ABI files to ReST directly.
> >   
> > > I'll be glad to look at them again, if you want to rebase after 5.10-rc1
> > > is out and resend them, as I think this should be showing up in the
> > > documentation.  
> > 
> > Surely. I'll rebase them after 5.10-rc1 and re-submit. 
> > 
> > What strategy do you prefer? Keep the files with the same format as
> > today (allowing them to optionally have ReST markups) or to convert
> > them to .rst directly?
> > 
> > In the latter case, the best would be to apply it as early as possible
> > after 5.10-rc1, as it may cause conflicts with other patches being
> > submitted for 5.11.  
> 
> The existing format if at all possible, doing wholesale changes is a
> mess and wouldn't be recommended.

Yeah, merging it would indeed be a mess. At long term, though, it could 
be easier to maintain.

> I think you already fixed up the entries that had problems being parsed
> in the past, if not, we can resolve those as well.

Yes. The series start with fixes. I suspect several of them
(if not all) were already merged, but if anything is missing, I can fix 
at the upcoming rebased series.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Adding ABI to htmldocs - Was: Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-07 11:59               ` Mauro Carvalho Chehab
@ 2020-10-21 16:28                 ` Mauro Carvalho Chehab
  2020-10-21 16:58                   ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2020-10-21 16:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Ivan Zaentsev, Evgeniy Polyakov, Jonathan Corbet,
	Akira Shimahara, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

Hi greg,

Em Wed, 7 Oct 2020 13:59:34 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

> Em Wed, 7 Oct 2020 13:43:59 +0200
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> 
> > On Wed, Oct 07, 2020 at 01:05:49PM +0200, Mauro Carvalho Chehab wrote:  
> > > Em Wed, 7 Oct 2020 11:06:19 +0200
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> > >     
> > > > On Wed, Oct 07, 2020 at 10:57:02AM +0200, Mauro Carvalho Chehab wrote:    
> > > > > Em Wed, 7 Oct 2020 10:32:27 +0300
> > > > > Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:
> > > > >       
> > > > > > Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:
> > > > > >       
> > > > > > >> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> > > > > > >> index f1148181f53e..00376501a5ef 100644
> > > > > > >> --- a/Documentation/w1/slaves/w1_therm.rst
> > > > > > >> +++ b/Documentation/w1/slaves/w1_therm.rst        
> > > > > >       
> > > > > > >>  
> > > > > > >> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
> > > > > > >>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
> > > > > > >>  conversion completion (normal power only) by generating read cycles on the bus
> > > > > > >>  after conversion starts. In parasite power mode this feature is not available.
> > > > > > >> -Feature bit masks may be combined (OR).
> > > > > > >> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> > > > > > >> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> > > > > > >> +        
> > > > > >       
> > > > > > > As warned by Sphinx, this cross-reference is broken:        
> > > > > >       
> > > > > > >         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> > > > > > > undefined label: w1_therm (if the link has no caption the label must precede a section header)        
> > > > > > 
> > > > > > Would this be ok?      
> > > > > 
> > > > > Yeah, sure!
> > > > >       
> > > > > > 
> > > > > > "More details in Documentation/ABI/testing/sysfs-driver-w1_therm"
> > > > > >       
> > > > > > > Not sure what you wanted to point here.        
> > > > > > 
> > > > > > A link to a driver's sysfs interface, but sysfs docs are text
> > > > > > files and seem to not be included in Sphynx Docs.      
> > > > > 
> > > > > I sent upstream sometime ago a patch series adding ABI to Sphinx, but I 
> > > > > was not merged, not sure why:
> > > > > 
> > > > > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v5.6      
> > > > 
> > > > I think the raft of different patches floating around at the time made
> > > > me totally confused as to what was, and was not, the latest versions.    
> > > 
> > > Yeah, there were lots of patches floating around that time.
> > > 
> > > I also recall that someone (Jeni?) asked if the best wouldn't be to
> > > just convert the ABI files to ReST directly.
> > >     
> > > > I'll be glad to look at them again, if you want to rebase after 5.10-rc1
> > > > is out and resend them, as I think this should be showing up in the
> > > > documentation.    
> > > 
> > > Surely. I'll rebase them after 5.10-rc1 and re-submit. 
> > > 
> > > What strategy do you prefer? Keep the files with the same format as
> > > today (allowing them to optionally have ReST markups) or to convert
> > > them to .rst directly?
> > > 
> > > In the latter case, the best would be to apply it as early as possible
> > > after 5.10-rc1, as it may cause conflicts with other patches being
> > > submitted for 5.11.    
> > 
> > The existing format if at all possible, doing wholesale changes is a
> > mess and wouldn't be recommended.  
> 
> Yeah, merging it would indeed be a mess. At long term, though, it could 
> be easier to maintain.
> 
> > I think you already fixed up the entries that had problems being parsed
> > in the past, if not, we can resolve those as well.  
> 
> Yes. The series start with fixes. I suspect several of them
> (if not all) were already merged, but if anything is missing, I can fix 
> at the upcoming rebased series.

Rebasing the patch series was easier than what I expected:

	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v6

Yet, while fixing one build issue, I noticed that there are multiple
files defining the same ABI, with different contents.

Right now, scripts/get_abi.pl assumes that "what" is unique. Well, sorts
of. When it finds a duplicated entry, it merges the description, 
preserving the fields from the last parsed entry.

I ended adding a patch to detect those ABI duplication:

	https://git.linuxtv.org/mchehab/experimental.git/commit/?h=abi_patches_v6&id=6868914605cb0ebffe3fd07d344c246e1e4cd94e

I'm enclosing the results.

One such example is this one:

	3 duplicated entries for /sys/class/leds/<led>/hw_pattern: on file(s) sysfs-class-led-trigger-pattern sysfs-class-led-driver-sc27xx sysfs-class-led-driver-el15203000

It sounds that different drivers define and use this ABI, but
each one with different meanings. 

There are even some cases where the same file define the same ABI twice:

	2 duplicated entries for /sys/class/power_supply/<supply_name>/temp_alert_min: on file(s) sysfs-class-power

Not sure what's the best way to document things like that, or if
the fix would be to drop/merge those.

Any ideas?

Thanks,
Mauro

$ ./scripts/get_abi.pl validate
2 duplicated entries for /sys/class/infiniband/<device>/ports/<port-num>/counters/symbol_error, /sys/class/infiniband/<device>/ports/<port-num>/counters/port_rcv_errors, /sys/class/infiniband/<device>/ports/<port-num>/counters/port_rcv_remote_physical_errors, /sys/class/infiniband/<device>/ports/<port-num>/counters/port_rcv_switch_relay_errors, /sys/class/infiniband/<device>/ports/<port-num>/counters/link_error_recovery, /sys/class/infiniband/<device>/ports/<port-num>/counters/port_xmit_constraint_errors, /sys/class/infiniband/<device>/ports/<port-num>/counters/port_rcv_contraint_errors, /sys/class/infiniband/<device>/ports/<port-num>/counters/local_link_integrity_errors, /sys/class/infiniband/<device>/ports/<port-num>/counters/excessive_buffer_overrun_errors, /sys/class/infiniband/<device>/ports/<port-num>/counters/port_xmit_data, /sys/class/infiniband/<device>/ports/<port-num>/counters/port_rcv_data, /sys/class/infiniband/<device>/ports/<port-num>/counters/port_xmit_packets, /sys/c
 lass/infiniband/<device>/ports/<port-num>/counters/port_rcv_packets, /sys/class/infiniband/<device>/ports/<port-num>/counters/unicast_rcv_packets, /sys/class/infiniband/<device>/ports/<port-num>/counters/unicast_xmit_packets, /sys/class/infiniband/<device>/ports/<port-num>/counters/multicast_rcv_packets, /sys/class/infiniband/<device>/ports/<port-num>/counters/multicast_xmit_packets, /sys/class/infiniband/<device>/ports/<port-num>/counters/link_downed, /sys/class/infiniband/<device>/ports/<port-num>/counters/port_xmit_discards, /sys/class/infiniband/<device>/ports/<port-num>/counters/VL15_dropped, /sys/class/infiniband/<device>/ports/<port-num>/counters/port_xmit_wait: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/qibX/ports/N/sl2vl/[0-15]: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband_verbs/abi_version: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/backlight/<backlight>/ambient_light_zone: on file(s) sysfs-class-backlight-adp8860 sysfs-class-backlight-driver-adp8870
3 duplicated entries for /sys/class/leds/<led>/hw_pattern: on file(s) sysfs-class-led-trigger-pattern sysfs-class-led-driver-sc27xx sysfs-class-led-driver-el15203000
2 duplicated entries for /sys/bus/vmbus/devices/<UUID>/channels/<N>/cpu: on file(s) sysfs-bus-vmbus
2 duplicated entries for /sys/class/infiniband/qibX/ports/N/CCMgtA/cc_settings_bin, /sys/class/infiniband/qibX/ports/N/CCMgtA/cc_table_bin: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/hfi1_X/hw_rev, /sys/class/infiniband/hfi1_X/board_id, /sys/class/infiniband/hfi1_X/nctxts, /sys/class/infiniband/hfi1_X/serial, /sys/class/infiniband/hfi1_X/chip_reset, /sys/class/infiniband/hfi1_X/boardversion, /sys/class/infiniband/hfi1_X/nfreectxts, /sys/class/infiniband/hfi1_X/tempsense: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/<hca>/ports/<port-number>/gid_attrs/ndevs/<gid-index>: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/hfi1_X/sdma_N/cpu_list, /sys/class/infiniband/hfi1_X/sdma_N/vl: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/bus/iio/devices/iio:deviceX/out_currentY_raw: on file(s) sysfs-bus-iio-light-lm3533-als sysfs-bus-iio-health-afe440x
2 duplicated entries for /sys/class/infiniband/vmw_pvrdmaX/hw_rev, /sys/class/infiniband/vmw_pvrdmaX/hca_type, /sys/class/infiniband/vmw_pvrdmaX/board_id: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/ocrdmaX/hw_rev: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/mlx4_X/iov/<pci-slot-num>/ports/<m>/smi_enabled, /sys/class/infiniband/mlx4_X/iov/<pci-slot-num>/ports/<m>/enable_smi_admin: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/mlx4_X/iov/ports/<port-num>/gids/<n>, /sys/class/infiniband/mlx4_X/iov/ports/<port-num>/admin_guids/<n>, /sys/class/infiniband/mlx4_X/iov/ports/<port-num>/pkeys/<n>, /sys/class/infiniband/mlx4_X/iov/ports/<port-num>/mcgs/, /sys/class/infiniband/mlx4_X/iov/ports/<pci-slot-num>/ports/<m>/gid_idx/0, /sys/class/infiniband/mlx4_X/iov/ports/<pci-slot-num>/ports/<m>/pkey_idx/<n>: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/qibX/ports/N/linkstate/loopback, /sys/class/infiniband/qibX/ports/N/linkstate/led_override, /sys/class/infiniband/qibX/ports/N/linkstate/hrtbt_enable, /sys/class/infiniband/qibX/ports/N/linkstate/status, /sys/class/infiniband/qibX/ports/N/linkstate/status_str: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband_mad/umadN/ibdev, /sys/class/infiniband_mad/umadN/port, /sys/class/infiniband_mad/issmN/ibdev, /sys/class/infiniband_mad/issmN/port: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/power_supply/<supply_name>/temp_alert_max: on file(s) sysfs-class-power
2 duplicated entries for /sys/class/infiniband/mlx4_X/hw_rev, /sys/class/infiniband/mlx4_X/hca_type, /sys/class/infiniband/mlx4_X/board_id: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/<hca>/ports/<port-number>/gid_attrs/types/<gid-index>: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/power_supply/<supply_name>/current_max: on file(s) sysfs-class-power
2 duplicated entries for /sys/class/infiniband/<device>/ports/<port-num>/link_layer: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/qedrX/hw_rev, /sys/class/infiniband/qedrX/hca_type: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/power_supply/<supply_name>/current_avg: on file(s) sysfs-class-power
2 duplicated entries for /sys/class/infiniband/qibX/version, /sys/class/infiniband/qibX/hw_rev, /sys/class/infiniband/qibX/hca_type, /sys/class/infiniband/qibX/board_id, /sys/class/infiniband/qibX/boardversion, /sys/class/infiniband/qibX/nctxts, /sys/class/infiniband/qibX/localbus_info, /sys/class/infiniband/qibX/tempsense, /sys/class/infiniband/qibX/serial, /sys/class/infiniband/qibX/nfreectxts, /sys/class/infiniband/qibX/chip_reset: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/bus/iio/devices/iio:deviceX/sensor_sensitivity: on file(s) sysfs-bus-iio-distance-srf08 sysfs-bus-iio-proximity-as3935
2 duplicated entries for /sys/class/infiniband/usnic_X/qpn/summary, /sys/class/infiniband/usnic_X/qpn/context: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/kernel/iommu_groups/reserved_regions: on file(s) sysfs-kernel-iommu_groups
2 duplicated entries for /sys/class/power_supply/<supply_name>/temp_max: on file(s) sysfs-class-power
2 duplicated entries for /sys/class/infiniband_verbs/uverbsN/ibdev, /sys/class/infiniband_verbs/uverbsN/abi_version: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/bnxt_reX/hw_rev, /sys/class/infiniband/bnxt_reX/hca_type: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/mlx5_X/hw_rev, /sys/class/infiniband/mlx5_X/hca_type, /sys/class/infiniband/mlx5_X/reg_pages, /sys/class/infiniband/mlx5_X/fw_pages: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/<device-name>/hw_counters/lifespan, /sys/class/infiniband/<device-name>/ports/<port-num>/hw_counters/lifespan: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/cxgb4_X/hw_rev, /sys/class/infiniband/cxgb4_X/hca_type, /sys/class/infiniband/cxgb4_X/board_id: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/usnic_X/board_id, /sys/class/infiniband/usnic_X/config, /sys/class/infiniband/usnic_X/qp_per_vf, /sys/class/infiniband/usnic_X/max_vf, /sys/class/infiniband/usnic_X/cq_per_vf, /sys/class/infiniband/usnic_X/iface: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/leds/<led>/brightness: on file(s) sysfs-class-led sysfs-class-led-multicolor
2 duplicated entries for /sys/class/infiniband/qibX/ports/N/diag_counters/rc_resends, /sys/class/infiniband/qibX/ports/N/diag_counters/seq_naks, /sys/class/infiniband/qibX/ports/N/diag_counters/rdma_seq, /sys/class/infiniband/qibX/ports/N/diag_counters/rnr_naks, /sys/class/infiniband/qibX/ports/N/diag_counters/other_naks, /sys/class/infiniband/qibX/ports/N/diag_counters/rc_timeouts, /sys/class/infiniband/qibX/ports/N/diag_counters/look_pkts, /sys/class/infiniband/qibX/ports/N/diag_counters/pkt_drops, /sys/class/infiniband/qibX/ports/N/diag_counters/dma_wait, /sys/class/infiniband/qibX/ports/N/diag_counters/unaligned: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode: on file(s) sysfs-bus-iio-timer-stm32 sysfs-bus-iio-lptimer-stm32
2 duplicated entries for /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency: on file(s) sysfs-bus-iio-frequency-adf4371 sysfs-bus-iio
2 duplicated entries for /sys/class/power_supply/<supply_name>/current_now: on file(s) sysfs-class-power
2 duplicated entries for /sys/class/infiniband/ocrdmaX/hca_type: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/hfi1_X/ports/N/sc2vl/[0-31], /sys/class/infiniband/hfi1_X/ports/N/sl2sc/[0-31], /sys/class/infiniband/hfi1_X/ports/N/vl2mtu/[0-15]: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/c2port/c2portX/flash_erase: on file(s) sysfs-c2port
2 duplicated entries for /sys/class/infiniband/i40iwX/hw_rev, /sys/class/infiniband/i40iwX/hca_type, /sys/class/infiniband/i40iwX/board_id: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/power_supply/<supply_name>/temp_min: on file(s) sysfs-class-power
2 duplicated entries for /sys/bus/w1/devices/.../eeprom: on file(s) sysfs-driver-w1_ds28e04 sysfs-driver-w1_therm
2 duplicated entries for /sys/bus/iio/devices/iio:deviceX/in_count0_preset: on file(s) sysfs-bus-iio-timer-stm32 sysfs-bus-iio-lptimer-stm32
2 duplicated entries for /sys/class/infiniband/<device>/ports/<port-num>/lid, /sys/class/infiniband/<device>/ports/<port-num>/rate, /sys/class/infiniband/<device>/ports/<port-num>/lid_mask_count, /sys/class/infiniband/<device>/ports/<port-num>/sm_sl, /sys/class/infiniband/<device>/ports/<port-num>/sm_lid, /sys/class/infiniband/<device>/ports/<port-num>/state, /sys/class/infiniband/<device>/ports/<port-num>/phys_state, /sys/class/infiniband/<device>/ports/<port-num>/cap_mask: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/<device>/node_desc: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/mthcaX/hw_rev, /sys/class/infiniband/mthcaX/hca_type, /sys/class/infiniband/mthcaX/board_id: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/leds/<led>/repeat: on file(s) sysfs-class-led-trigger-pattern sysfs-class-led-driver-el15203000
2 duplicated entries for /sys/class/power_supply/<supply_name>/temp_alert_min: on file(s) sysfs-class-power
2 duplicated entries for /sys/class/infiniband/hfi1_X/ports/N/CCMgtA/cc_settings_bin, /sys/class/infiniband/hfi1_X/ports/N/CCMgtA/cc_table_bin, /sys/class/infiniband/hfi1_X/ports/N/CCMgtA/cc_prescan: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/power_supply/<supply_name>/temp: on file(s) sysfs-class-power
2 duplicated entries for /sys/bus/iio/devices/iio:deviceX/out_current_heater_raw, /sys/bus/iio/devices/iio:deviceX/out_current_heater_raw_available: on file(s) sysfs-bus-iio-humidity-hdc2010 sysfs-bus-iio-humidity-hdc100x
2 duplicated entries for /sys/class/infiniband_mad/abi_version: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/<device>/fw_ver: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/infiniband/<device>/node_type, /sys/class/infiniband/<device>/node_guid, /sys/class/infiniband/<device>/sys_image_guid: on file(s) sysfs-class-infiniband sysfs-class-infiniband.org
2 duplicated entries for /sys/class/backlight/<backlight>/ambient_light_level: on file(s) sysfs-class-backlight-adp8860 sysfs-class-backlight-driver-adp8870
2 duplicated entries for /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available: on file(s) sysfs-bus-iio-timer-stm32 sysfs-bus-iio-lptimer-stm32



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Adding ABI to htmldocs - Was: Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-21 16:28                 ` Adding ABI to htmldocs - Was: " Mauro Carvalho Chehab
@ 2020-10-21 16:58                   ` Greg Kroah-Hartman
  2020-10-22  8:19                     ` Mauro Carvalho Chehab
  2020-10-29 14:28                     ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2020-10-21 16:58 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Ivan Zaentsev, Evgeniy Polyakov, Jonathan Corbet,
	Akira Shimahara, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

On Wed, Oct 21, 2020 at 06:28:43PM +0200, Mauro Carvalho Chehab wrote:
> Hi greg,
> 
> Em Wed, 7 Oct 2020 13:59:34 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> 
> > Em Wed, 7 Oct 2020 13:43:59 +0200
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> > 
> > > On Wed, Oct 07, 2020 at 01:05:49PM +0200, Mauro Carvalho Chehab wrote:  
> > > > Em Wed, 7 Oct 2020 11:06:19 +0200
> > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> > > >     
> > > > > On Wed, Oct 07, 2020 at 10:57:02AM +0200, Mauro Carvalho Chehab wrote:    
> > > > > > Em Wed, 7 Oct 2020 10:32:27 +0300
> > > > > > Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:
> > > > > >       
> > > > > > > Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:
> > > > > > >       
> > > > > > > >> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> > > > > > > >> index f1148181f53e..00376501a5ef 100644
> > > > > > > >> --- a/Documentation/w1/slaves/w1_therm.rst
> > > > > > > >> +++ b/Documentation/w1/slaves/w1_therm.rst        
> > > > > > >       
> > > > > > > >>  
> > > > > > > >> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
> > > > > > > >>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
> > > > > > > >>  conversion completion (normal power only) by generating read cycles on the bus
> > > > > > > >>  after conversion starts. In parasite power mode this feature is not available.
> > > > > > > >> -Feature bit masks may be combined (OR).
> > > > > > > >> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> > > > > > > >> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> > > > > > > >> +        
> > > > > > >       
> > > > > > > > As warned by Sphinx, this cross-reference is broken:        
> > > > > > >       
> > > > > > > >         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> > > > > > > > undefined label: w1_therm (if the link has no caption the label must precede a section header)        
> > > > > > > 
> > > > > > > Would this be ok?      
> > > > > > 
> > > > > > Yeah, sure!
> > > > > >       
> > > > > > > 
> > > > > > > "More details in Documentation/ABI/testing/sysfs-driver-w1_therm"
> > > > > > >       
> > > > > > > > Not sure what you wanted to point here.        
> > > > > > > 
> > > > > > > A link to a driver's sysfs interface, but sysfs docs are text
> > > > > > > files and seem to not be included in Sphynx Docs.      
> > > > > > 
> > > > > > I sent upstream sometime ago a patch series adding ABI to Sphinx, but I 
> > > > > > was not merged, not sure why:
> > > > > > 
> > > > > > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v5.6      
> > > > > 
> > > > > I think the raft of different patches floating around at the time made
> > > > > me totally confused as to what was, and was not, the latest versions.    
> > > > 
> > > > Yeah, there were lots of patches floating around that time.
> > > > 
> > > > I also recall that someone (Jeni?) asked if the best wouldn't be to
> > > > just convert the ABI files to ReST directly.
> > > >     
> > > > > I'll be glad to look at them again, if you want to rebase after 5.10-rc1
> > > > > is out and resend them, as I think this should be showing up in the
> > > > > documentation.    
> > > > 
> > > > Surely. I'll rebase them after 5.10-rc1 and re-submit. 
> > > > 
> > > > What strategy do you prefer? Keep the files with the same format as
> > > > today (allowing them to optionally have ReST markups) or to convert
> > > > them to .rst directly?
> > > > 
> > > > In the latter case, the best would be to apply it as early as possible
> > > > after 5.10-rc1, as it may cause conflicts with other patches being
> > > > submitted for 5.11.    
> > > 
> > > The existing format if at all possible, doing wholesale changes is a
> > > mess and wouldn't be recommended.  
> > 
> > Yeah, merging it would indeed be a mess. At long term, though, it could 
> > be easier to maintain.
> > 
> > > I think you already fixed up the entries that had problems being parsed
> > > in the past, if not, we can resolve those as well.  
> > 
> > Yes. The series start with fixes. I suspect several of them
> > (if not all) were already merged, but if anything is missing, I can fix 
> > at the upcoming rebased series.
> 
> Rebasing the patch series was easier than what I expected:
> 
> 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v6
> 
> Yet, while fixing one build issue, I noticed that there are multiple
> files defining the same ABI, with different contents.
> 
> Right now, scripts/get_abi.pl assumes that "what" is unique. Well, sorts
> of. When it finds a duplicated entry, it merges the description, 
> preserving the fields from the last parsed entry.
> 
> I ended adding a patch to detect those ABI duplication:
> 
> 	https://git.linuxtv.org/mchehab/experimental.git/commit/?h=abi_patches_v6&id=6868914605cb0ebffe3fd07d344c246e1e4cd94e
> 
> I'm enclosing the results.
> 
> One such example is this one:
> 
> 	3 duplicated entries for /sys/class/leds/<led>/hw_pattern: on file(s) sysfs-class-led-trigger-pattern sysfs-class-led-driver-sc27xx sysfs-class-led-driver-el15203000
> 
> It sounds that different drivers define and use this ABI, but
> each one with different meanings. 
> 
> There are even some cases where the same file define the same ABI twice:
> 
> 	2 duplicated entries for /sys/class/power_supply/<supply_name>/temp_alert_min: on file(s) sysfs-class-power
> 
> Not sure what's the best way to document things like that, or if
> the fix would be to drop/merge those.
> 
> Any ideas?

We should merge them to be the correct representation.  The
driver-specific ones for LED should just be dropped to use the
class-generic one.

I guess just take them one at a time :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Adding ABI to htmldocs - Was: Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-21 16:58                   ` Greg Kroah-Hartman
@ 2020-10-22  8:19                     ` Mauro Carvalho Chehab
  2020-10-29 14:28                     ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2020-10-22  8:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Ivan Zaentsev, Evgeniy Polyakov, Jonathan Corbet,
	Akira Shimahara, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

Em Wed, 21 Oct 2020 18:58:19 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:

> On Wed, Oct 21, 2020 at 06:28:43PM +0200, Mauro Carvalho Chehab wrote:
> > Hi greg,
> > 
> > Em Wed, 7 Oct 2020 13:59:34 +0200
> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> > 
> > > Em Wed, 7 Oct 2020 13:43:59 +0200
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> > > 
> > > > On Wed, Oct 07, 2020 at 01:05:49PM +0200, Mauro Carvalho Chehab wrote:  
> > > > > Em Wed, 7 Oct 2020 11:06:19 +0200
> > > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> > > > >     
> > > > > > On Wed, Oct 07, 2020 at 10:57:02AM +0200, Mauro Carvalho Chehab wrote:    
> > > > > > > Em Wed, 7 Oct 2020 10:32:27 +0300
> > > > > > > Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:
> > > > > > >       
> > > > > > > > Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:
> > > > > > > >       
> > > > > > > > >> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> > > > > > > > >> index f1148181f53e..00376501a5ef 100644
> > > > > > > > >> --- a/Documentation/w1/slaves/w1_therm.rst
> > > > > > > > >> +++ b/Documentation/w1/slaves/w1_therm.rst        
> > > > > > > >       
> > > > > > > > >>  
> > > > > > > > >> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
> > > > > > > > >>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
> > > > > > > > >>  conversion completion (normal power only) by generating read cycles on the bus
> > > > > > > > >>  after conversion starts. In parasite power mode this feature is not available.
> > > > > > > > >> -Feature bit masks may be combined (OR).
> > > > > > > > >> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> > > > > > > > >> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> > > > > > > > >> +        
> > > > > > > >       
> > > > > > > > > As warned by Sphinx, this cross-reference is broken:        
> > > > > > > >       
> > > > > > > > >         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> > > > > > > > > undefined label: w1_therm (if the link has no caption the label must precede a section header)        
> > > > > > > > 
> > > > > > > > Would this be ok?      
> > > > > > > 
> > > > > > > Yeah, sure!
> > > > > > >       
> > > > > > > > 
> > > > > > > > "More details in Documentation/ABI/testing/sysfs-driver-w1_therm"
> > > > > > > >       
> > > > > > > > > Not sure what you wanted to point here.        
> > > > > > > > 
> > > > > > > > A link to a driver's sysfs interface, but sysfs docs are text
> > > > > > > > files and seem to not be included in Sphynx Docs.      
> > > > > > > 
> > > > > > > I sent upstream sometime ago a patch series adding ABI to Sphinx, but I 
> > > > > > > was not merged, not sure why:
> > > > > > > 
> > > > > > > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v5.6      
> > > > > > 
> > > > > > I think the raft of different patches floating around at the time made
> > > > > > me totally confused as to what was, and was not, the latest versions.    
> > > > > 
> > > > > Yeah, there were lots of patches floating around that time.
> > > > > 
> > > > > I also recall that someone (Jeni?) asked if the best wouldn't be to
> > > > > just convert the ABI files to ReST directly.
> > > > >     
> > > > > > I'll be glad to look at them again, if you want to rebase after 5.10-rc1
> > > > > > is out and resend them, as I think this should be showing up in the
> > > > > > documentation.    
> > > > > 
> > > > > Surely. I'll rebase them after 5.10-rc1 and re-submit. 
> > > > > 
> > > > > What strategy do you prefer? Keep the files with the same format as
> > > > > today (allowing them to optionally have ReST markups) or to convert
> > > > > them to .rst directly?
> > > > > 
> > > > > In the latter case, the best would be to apply it as early as possible
> > > > > after 5.10-rc1, as it may cause conflicts with other patches being
> > > > > submitted for 5.11.    
> > > > 
> > > > The existing format if at all possible, doing wholesale changes is a
> > > > mess and wouldn't be recommended.  
> > > 
> > > Yeah, merging it would indeed be a mess. At long term, though, it could 
> > > be easier to maintain.
> > > 
> > > > I think you already fixed up the entries that had problems being parsed
> > > > in the past, if not, we can resolve those as well.  
> > > 
> > > Yes. The series start with fixes. I suspect several of them
> > > (if not all) were already merged, but if anything is missing, I can fix 
> > > at the upcoming rebased series.
> > 
> > Rebasing the patch series was easier than what I expected:
> > 
> > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v6
> > 
> > Yet, while fixing one build issue, I noticed that there are multiple
> > files defining the same ABI, with different contents.
> > 
> > Right now, scripts/get_abi.pl assumes that "what" is unique. Well, sorts
> > of. When it finds a duplicated entry, it merges the description, 
> > preserving the fields from the last parsed entry.
> > 
> > I ended adding a patch to detect those ABI duplication:
> > 
> > 	https://git.linuxtv.org/mchehab/experimental.git/commit/?h=abi_patches_v6&id=6868914605cb0ebffe3fd07d344c246e1e4cd94e
> > 
> > I'm enclosing the results.
> > 
> > One such example is this one:
> > 
> > 	3 duplicated entries for /sys/class/leds/<led>/hw_pattern: on file(s) sysfs-class-led-trigger-pattern sysfs-class-led-driver-sc27xx sysfs-class-led-driver-el15203000
> > 
> > It sounds that different drivers define and use this ABI, but
> > each one with different meanings. 
> > 
> > There are even some cases where the same file define the same ABI twice:
> > 
> > 	2 duplicated entries for /sys/class/power_supply/<supply_name>/temp_alert_min: on file(s) sysfs-class-power
> > 
> > Not sure what's the best way to document things like that, or if
> > the fix would be to drop/merge those.
> > 
> > Any ideas?
> 
> We should merge them to be the correct representation. 

Ok, makes sense to me.

I improved the script to better report such issues:

	$ ./scripts/get_abi.pl --dir Documentation/ABI/testing validate
	Warning: /sys/bus/iio/devices/iio:deviceX/in_count0_preset is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:101 Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:1
	Warning: /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:118 Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:15
	Warning: /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:112 Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:9
	Warning: /sys/bus/iio/devices/iio:deviceX/out_altvoltageY_frequency is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-frequency-adf4371:1 Documentation/ABI/testing/sysfs-bus-iio:599
	Warning: /sys/bus/iio/devices/iio:deviceX/out_currentY_raw is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-light-lm3533-als:44 Documentation/ABI/testing/sysfs-bus-iio-health-afe440x:35
	Warning: /sys/bus/iio/devices/iio:deviceX/out_current_heater_raw, /sys/bus/iio/devices/iio:deviceX/out_current_heater_raw_available is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010:2 Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc100x:2
	Warning: /sys/bus/iio/devices/iio:deviceX/sensor_sensitivity is defined 2 times: Documentation/ABI/testing/sysfs-bus-iio-distance-srf08:1 Documentation/ABI/testing/sysfs-bus-iio-proximity-as3935:9
	Warning: /sys/class/backlight/<backlight>/ambient_light_level is defined 2 times: Documentation/ABI/testing/sysfs-class-backlight-adp8860:9 Documentation/ABI/testing/sysfs-class-backlight-driver-adp8870:31
	Warning: /sys/class/backlight/<backlight>/ambient_light_zone is defined 2 times: Documentation/ABI/testing/sysfs-class-backlight-adp8860:19 Documentation/ABI/testing/sysfs-class-backlight-driver-adp8870:41
	Warning: /sys/class/c2port/c2portX/flash_erase is defined 2 times: Documentation/ABI/testing/sysfs-c2port:61 Documentation/ABI/testing/sysfs-c2port:69
	Warning: /sys/class/leds/<led>/brightness is defined 2 times: Documentation/ABI/testing/sysfs-class-led:1 Documentation/ABI/testing/sysfs-class-led-multicolor:1
	Warning: /sys/class/leds/<led>/hw_pattern is defined 3 times: Documentation/ABI/testing/sysfs-class-led-trigger-pattern:15 Documentation/ABI/testing/sysfs-class-led-driver-sc27xx:1 Documentation/ABI/testing/sysfs-class-led-driver-el15203000:1
	Warning: /sys/class/leds/<led>/repeat is defined 2 times: Documentation/ABI/testing/sysfs-class-led-trigger-pattern:29 Documentation/ABI/testing/sysfs-class-led-driver-el15203000:136
	Warning: /sys/class/power_supply/<supply_name>/current_avg is defined 2 times: Documentation/ABI/testing/sysfs-class-power:105 Documentation/ABI/testing/sysfs-class-power:386
	Warning: /sys/class/power_supply/<supply_name>/current_max is defined 2 times: Documentation/ABI/testing/sysfs-class-power:117 Documentation/ABI/testing/sysfs-class-power:399
	Warning: /sys/class/power_supply/<supply_name>/current_now is defined 2 times: Documentation/ABI/testing/sysfs-class-power:126 Documentation/ABI/testing/sysfs-class-power:409
	Warning: /sys/class/power_supply/<supply_name>/temp is defined 2 times: Documentation/ABI/testing/sysfs-class-power:276 Documentation/ABI/testing/sysfs-class-power:486
	Warning: /sys/class/power_supply/<supply_name>/temp_alert_max is defined 2 times: Documentation/ABI/testing/sysfs-class-power:286 Documentation/ABI/testing/sysfs-class-power:498
	Warning: /sys/class/power_supply/<supply_name>/temp_alert_min is defined 2 times: Documentation/ABI/testing/sysfs-class-power:301 Documentation/ABI/testing/sysfs-class-power:514
	Warning: /sys/class/power_supply/<supply_name>/temp_max is defined 2 times: Documentation/ABI/testing/sysfs-class-power:317 Documentation/ABI/testing/sysfs-class-power:530
	Warning: /sys/class/power_supply/<supply_name>/temp_min is defined 2 times: Documentation/ABI/testing/sysfs-class-power:328 Documentation/ABI/testing/sysfs-class-power:540
	Warning: /sys/kernel/iommu_groups/reserved_regions is defined 2 times: Documentation/ABI/testing/sysfs-kernel-iommu_groups:16 Documentation/ABI/testing/sysfs-kernel-iommu_groups:28

With the line numbers where each occurrence happens, it should be
easier to solve such warnings.

> The driver-specific ones for LED should just be dropped to use the
> class-generic one.

I suspect that, instead of just dropping them, it could make sense to
move driver-specific ones to the ReST file for that driver,
like the ones inside Documentation/leds/.

> I guess just take them one at a time :)

Makes sense to me. Btw, right now there are 60 warnings like that.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Adding ABI to htmldocs - Was: Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-21 16:58                   ` Greg Kroah-Hartman
  2020-10-22  8:19                     ` Mauro Carvalho Chehab
@ 2020-10-29 14:28                     ` Mauro Carvalho Chehab
  2020-11-09 17:32                       ` Greg Kroah-Hartman
  1 sibling, 1 reply; 14+ messages in thread
From: Mauro Carvalho Chehab @ 2020-10-29 14:28 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Ivan Zaentsev, Evgeniy Polyakov, Jonathan Corbet,
	Akira Shimahara, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

Em Wed, 21 Oct 2020 18:58:19 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:

> On Wed, Oct 21, 2020 at 06:28:43PM +0200, Mauro Carvalho Chehab wrote:
> > Hi greg,
> > 
> > Em Wed, 7 Oct 2020 13:59:34 +0200
> > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> >   
> > > Em Wed, 7 Oct 2020 13:43:59 +0200
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> > >   
> > > > On Wed, Oct 07, 2020 at 01:05:49PM +0200, Mauro Carvalho Chehab wrote:    
> > > > > Em Wed, 7 Oct 2020 11:06:19 +0200
> > > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> > > > >       
> > > > > > On Wed, Oct 07, 2020 at 10:57:02AM +0200, Mauro Carvalho Chehab wrote:      
> > > > > > > Em Wed, 7 Oct 2020 10:32:27 +0300
> > > > > > > Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:
> > > > > > >         
> > > > > > > > Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:
> > > > > > > >         
> > > > > > > > >> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> > > > > > > > >> index f1148181f53e..00376501a5ef 100644
> > > > > > > > >> --- a/Documentation/w1/slaves/w1_therm.rst
> > > > > > > > >> +++ b/Documentation/w1/slaves/w1_therm.rst          
> > > > > > > >         
> > > > > > > > >>  
> > > > > > > > >> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
> > > > > > > > >>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
> > > > > > > > >>  conversion completion (normal power only) by generating read cycles on the bus
> > > > > > > > >>  after conversion starts. In parasite power mode this feature is not available.
> > > > > > > > >> -Feature bit masks may be combined (OR).
> > > > > > > > >> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> > > > > > > > >> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> > > > > > > > >> +          
> > > > > > > >         
> > > > > > > > > As warned by Sphinx, this cross-reference is broken:          
> > > > > > > >         
> > > > > > > > >         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> > > > > > > > > undefined label: w1_therm (if the link has no caption the label must precede a section header)          
> > > > > > > > 
> > > > > > > > Would this be ok?        
> > > > > > > 
> > > > > > > Yeah, sure!
> > > > > > >         
> > > > > > > > 
> > > > > > > > "More details in Documentation/ABI/testing/sysfs-driver-w1_therm"
> > > > > > > >         
> > > > > > > > > Not sure what you wanted to point here.          
> > > > > > > > 
> > > > > > > > A link to a driver's sysfs interface, but sysfs docs are text
> > > > > > > > files and seem to not be included in Sphynx Docs.        
> > > > > > > 
> > > > > > > I sent upstream sometime ago a patch series adding ABI to Sphinx, but I 
> > > > > > > was not merged, not sure why:
> > > > > > > 
> > > > > > > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v5.6        
> > > > > > 
> > > > > > I think the raft of different patches floating around at the time made
> > > > > > me totally confused as to what was, and was not, the latest versions.      
> > > > > 
> > > > > Yeah, there were lots of patches floating around that time.
> > > > > 
> > > > > I also recall that someone (Jeni?) asked if the best wouldn't be to
> > > > > just convert the ABI files to ReST directly.
> > > > >       
> > > > > > I'll be glad to look at them again, if you want to rebase after 5.10-rc1
> > > > > > is out and resend them, as I think this should be showing up in the
> > > > > > documentation.      
> > > > > 
> > > > > Surely. I'll rebase them after 5.10-rc1 and re-submit. 
> > > > > 
> > > > > What strategy do you prefer? Keep the files with the same format as
> > > > > today (allowing them to optionally have ReST markups) or to convert
> > > > > them to .rst directly?
> > > > > 
> > > > > In the latter case, the best would be to apply it as early as possible
> > > > > after 5.10-rc1, as it may cause conflicts with other patches being
> > > > > submitted for 5.11.      
> > > > 
> > > > The existing format if at all possible, doing wholesale changes is a
> > > > mess and wouldn't be recommended.    
> > > 
> > > Yeah, merging it would indeed be a mess. At long term, though, it could 
> > > be easier to maintain.
> > >   
> > > > I think you already fixed up the entries that had problems being parsed
> > > > in the past, if not, we can resolve those as well.    
> > > 
> > > Yes. The series start with fixes. I suspect several of them
> > > (if not all) were already merged, but if anything is missing, I can fix 
> > > at the upcoming rebased series.  
> > 
> > Rebasing the patch series was easier than what I expected:
> > 
> > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v6
> > 
> > Yet, while fixing one build issue, I noticed that there are multiple
> > files defining the same ABI, with different contents.
> > 
> > Right now, scripts/get_abi.pl assumes that "what" is unique. Well, sorts
> > of. When it finds a duplicated entry, it merges the description, 
> > preserving the fields from the last parsed entry.
> > 
> > I ended adding a patch to detect those ABI duplication:
> > 
> > 	https://git.linuxtv.org/mchehab/experimental.git/commit/?h=abi_patches_v6&id=6868914605cb0ebffe3fd07d344c246e1e4cd94e
> > 
> > I'm enclosing the results.
> > 
> > One such example is this one:
> > 
> > 	3 duplicated entries for /sys/class/leds/<led>/hw_pattern: on file(s) sysfs-class-led-trigger-pattern sysfs-class-led-driver-sc27xx sysfs-class-led-driver-el15203000
> > 
> > It sounds that different drivers define and use this ABI, but
> > each one with different meanings. 
> > 
> > There are even some cases where the same file define the same ABI twice:
> > 
> > 	2 duplicated entries for /sys/class/power_supply/<supply_name>/temp_alert_min: on file(s) sysfs-class-power
> > 
> > Not sure what's the best way to document things like that, or if
> > the fix would be to drop/merge those.
> > 
> > Any ideas?  
> 
> We should merge them to be the correct representation.  The
> driver-specific ones for LED should just be dropped to use the
> class-generic one.
> 
> I guess just take them one at a time :)

I'm trying to address each one of the duplicated ones...
I'm now stuck with this one:

At Documentation/ABI/testing/sysfs-driver-w1_therm, it has:

	What:		/sys/bus/w1/devices/.../eeprom
	Date:		May 2020
	Contact:	Akira Shimahara <akira215corp@gmail.com>
	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

Which defines the same ABI as this one:

	What:		/sys/bus/w1/devices/.../eeprom
	Date:		May 2012
	Contact:	Markus Franke <franm@hrz.tu-chemnitz.de>
	Description:	read/write the contents of the EEPROM memory of the DS28E04-100
			see Documentation/w1/slaves/w1_ds28e04.rst for detailed information
	Users:		any user space application which wants to communicate with DS28E04-100

Which is further described at Documentation/w1/slaves/w1_ds28e04.rst:

   Memory Access

	A read operation on the "eeprom" file reads the given amount of bytes
	from the EEPROM of the DS28E04.

	A write operation on the "eeprom" file writes the given byte sequence
	to the EEPROM of the DS28E04. If CRC checking mode is enabled only
	fully aligned blocks of 32 bytes with valid CRC16 values (in bytes 30
	and 31) are allowed to be written.

-

This specific duplication seems very evil, as if someone does:

	echo restore > /sys/bus/w1/devices/.../eeprom

and the device is a DS28E04-100, its eeprom will be erased instead
of being restored!

Not sure how this could be solved without causing regressions.

As the new ABI is from May 2020, added on this commit:

  commit 45d457a4cf24455eefd076a01a3d86414fc2ff1e
  Author: Akira Shimahara <akira215corp@gmail.com>
  Date:   Mon May 11 22:37:25 2020 +0200

    w1_therm: adding eeprom sysfs entry
    
    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 <akira215corp@gmail.com>
    Link: https://lore.kernel.org/r/20200511203725.410844-1-akira215corp@gmail.com
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

(probably reached Kernel 5.8):

	$ git describe 45d457a4cf244
	v5.7-rc5-92-g45d457a4cf24

I guess the solution would be to rename the new one.

Comments?


Thanks,
Mauro

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Adding ABI to htmldocs - Was: Re: [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device.
  2020-10-29 14:28                     ` Mauro Carvalho Chehab
@ 2020-11-09 17:32                       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2020-11-09 17:32 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Ivan Zaentsev, Evgeniy Polyakov, Jonathan Corbet,
	Akira Shimahara, Dan Carpenter, Colin Ian King, linux-kernel,
	linux-doc, Evgeny Boger

On Thu, Oct 29, 2020 at 03:28:45PM +0100, Mauro Carvalho Chehab wrote:
> Em Wed, 21 Oct 2020 18:58:19 +0200
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> 
> > On Wed, Oct 21, 2020 at 06:28:43PM +0200, Mauro Carvalho Chehab wrote:
> > > Hi greg,
> > > 
> > > Em Wed, 7 Oct 2020 13:59:34 +0200
> > > Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> > >   
> > > > Em Wed, 7 Oct 2020 13:43:59 +0200
> > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> > > >   
> > > > > On Wed, Oct 07, 2020 at 01:05:49PM +0200, Mauro Carvalho Chehab wrote:    
> > > > > > Em Wed, 7 Oct 2020 11:06:19 +0200
> > > > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> > > > > >       
> > > > > > > On Wed, Oct 07, 2020 at 10:57:02AM +0200, Mauro Carvalho Chehab wrote:      
> > > > > > > > Em Wed, 7 Oct 2020 10:32:27 +0300
> > > > > > > > Ivan Zaentsev <ivan.zaentsev@wirenboard.ru> escreveu:
> > > > > > > >         
> > > > > > > > > Tuesday, October 6, 2020, 4:19:15 PM, Mauro Carvalho Chehab wrote:
> > > > > > > > >         
> > > > > > > > > >> diff --git a/Documentation/w1/slaves/w1_therm.rst b/Documentation/w1/slaves/w1_therm.rst
> > > > > > > > > >> index f1148181f53e..00376501a5ef 100644
> > > > > > > > > >> --- a/Documentation/w1/slaves/w1_therm.rst
> > > > > > > > > >> +++ b/Documentation/w1/slaves/w1_therm.rst          
> > > > > > > > >         
> > > > > > > > > >>  
> > > > > > > > > >> @@ -130,4 +131,12 @@ conversion and temperature reads 85.00 (powerup value) or 127.94 (insufficient
> > > > > > > > > >>  power), the driver returns a conversion error. Bit mask ``2`` enables poll for
> > > > > > > > > >>  conversion completion (normal power only) by generating read cycles on the bus
> > > > > > > > > >>  after conversion starts. In parasite power mode this feature is not available.
> > > > > > > > > >> -Feature bit masks may be combined (OR).
> > > > > > > > > >> +Feature bit masks may be combined (OR). See accompanying sysfs documentation:
> > > > > > > > > >> +:ref:`Documentation/w1/slaves/w1_therm.rst <w1_therm>`
> > > > > > > > > >> +          
> > > > > > > > >         
> > > > > > > > > > As warned by Sphinx, this cross-reference is broken:          
> > > > > > > > >         
> > > > > > > > > >         .../Documentation/w1/slaves/w1_therm.rst:125: WARNING:
> > > > > > > > > > undefined label: w1_therm (if the link has no caption the label must precede a section header)          
> > > > > > > > > 
> > > > > > > > > Would this be ok?        
> > > > > > > > 
> > > > > > > > Yeah, sure!
> > > > > > > >         
> > > > > > > > > 
> > > > > > > > > "More details in Documentation/ABI/testing/sysfs-driver-w1_therm"
> > > > > > > > >         
> > > > > > > > > > Not sure what you wanted to point here.          
> > > > > > > > > 
> > > > > > > > > A link to a driver's sysfs interface, but sysfs docs are text
> > > > > > > > > files and seem to not be included in Sphynx Docs.        
> > > > > > > > 
> > > > > > > > I sent upstream sometime ago a patch series adding ABI to Sphinx, but I 
> > > > > > > > was not merged, not sure why:
> > > > > > > > 
> > > > > > > > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v5.6        
> > > > > > > 
> > > > > > > I think the raft of different patches floating around at the time made
> > > > > > > me totally confused as to what was, and was not, the latest versions.      
> > > > > > 
> > > > > > Yeah, there were lots of patches floating around that time.
> > > > > > 
> > > > > > I also recall that someone (Jeni?) asked if the best wouldn't be to
> > > > > > just convert the ABI files to ReST directly.
> > > > > >       
> > > > > > > I'll be glad to look at them again, if you want to rebase after 5.10-rc1
> > > > > > > is out and resend them, as I think this should be showing up in the
> > > > > > > documentation.      
> > > > > > 
> > > > > > Surely. I'll rebase them after 5.10-rc1 and re-submit. 
> > > > > > 
> > > > > > What strategy do you prefer? Keep the files with the same format as
> > > > > > today (allowing them to optionally have ReST markups) or to convert
> > > > > > them to .rst directly?
> > > > > > 
> > > > > > In the latter case, the best would be to apply it as early as possible
> > > > > > after 5.10-rc1, as it may cause conflicts with other patches being
> > > > > > submitted for 5.11.      
> > > > > 
> > > > > The existing format if at all possible, doing wholesale changes is a
> > > > > mess and wouldn't be recommended.    
> > > > 
> > > > Yeah, merging it would indeed be a mess. At long term, though, it could 
> > > > be easier to maintain.
> > > >   
> > > > > I think you already fixed up the entries that had problems being parsed
> > > > > in the past, if not, we can resolve those as well.    
> > > > 
> > > > Yes. The series start with fixes. I suspect several of them
> > > > (if not all) were already merged, but if anything is missing, I can fix 
> > > > at the upcoming rebased series.  
> > > 
> > > Rebasing the patch series was easier than what I expected:
> > > 
> > > 	https://git.linuxtv.org/mchehab/experimental.git/log/?h=abi_patches_v6
> > > 
> > > Yet, while fixing one build issue, I noticed that there are multiple
> > > files defining the same ABI, with different contents.
> > > 
> > > Right now, scripts/get_abi.pl assumes that "what" is unique. Well, sorts
> > > of. When it finds a duplicated entry, it merges the description, 
> > > preserving the fields from the last parsed entry.
> > > 
> > > I ended adding a patch to detect those ABI duplication:
> > > 
> > > 	https://git.linuxtv.org/mchehab/experimental.git/commit/?h=abi_patches_v6&id=6868914605cb0ebffe3fd07d344c246e1e4cd94e
> > > 
> > > I'm enclosing the results.
> > > 
> > > One such example is this one:
> > > 
> > > 	3 duplicated entries for /sys/class/leds/<led>/hw_pattern: on file(s) sysfs-class-led-trigger-pattern sysfs-class-led-driver-sc27xx sysfs-class-led-driver-el15203000
> > > 
> > > It sounds that different drivers define and use this ABI, but
> > > each one with different meanings. 
> > > 
> > > There are even some cases where the same file define the same ABI twice:
> > > 
> > > 	2 duplicated entries for /sys/class/power_supply/<supply_name>/temp_alert_min: on file(s) sysfs-class-power
> > > 
> > > Not sure what's the best way to document things like that, or if
> > > the fix would be to drop/merge those.
> > > 
> > > Any ideas?  
> > 
> > We should merge them to be the correct representation.  The
> > driver-specific ones for LED should just be dropped to use the
> > class-generic one.
> > 
> > I guess just take them one at a time :)
> 
> I'm trying to address each one of the duplicated ones...
> I'm now stuck with this one:
> 
> At Documentation/ABI/testing/sysfs-driver-w1_therm, it has:
> 
> 	What:		/sys/bus/w1/devices/.../eeprom
> 	Date:		May 2020
> 	Contact:	Akira Shimahara <akira215corp@gmail.com>
> 	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
> 
> Which defines the same ABI as this one:
> 
> 	What:		/sys/bus/w1/devices/.../eeprom
> 	Date:		May 2012
> 	Contact:	Markus Franke <franm@hrz.tu-chemnitz.de>
> 	Description:	read/write the contents of the EEPROM memory of the DS28E04-100
> 			see Documentation/w1/slaves/w1_ds28e04.rst for detailed information
> 	Users:		any user space application which wants to communicate with DS28E04-100
> 
> Which is further described at Documentation/w1/slaves/w1_ds28e04.rst:
> 
>    Memory Access
> 
> 	A read operation on the "eeprom" file reads the given amount of bytes
> 	from the EEPROM of the DS28E04.
> 
> 	A write operation on the "eeprom" file writes the given byte sequence
> 	to the EEPROM of the DS28E04. If CRC checking mode is enabled only
> 	fully aligned blocks of 32 bytes with valid CRC16 values (in bytes 30
> 	and 31) are allowed to be written.
> 
> -
> 
> This specific duplication seems very evil, as if someone does:
> 
> 	echo restore > /sys/bus/w1/devices/.../eeprom
> 
> and the device is a DS28E04-100, its eeprom will be erased instead
> of being restored!
> 
> Not sure how this could be solved without causing regressions.
> 
> As the new ABI is from May 2020, added on this commit:
> 
>   commit 45d457a4cf24455eefd076a01a3d86414fc2ff1e
>   Author: Akira Shimahara <akira215corp@gmail.com>
>   Date:   Mon May 11 22:37:25 2020 +0200
> 
>     w1_therm: adding eeprom sysfs entry
>     
>     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 <akira215corp@gmail.com>
>     Link: https://lore.kernel.org/r/20200511203725.410844-1-akira215corp@gmail.com
>     Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> (probably reached Kernel 5.8):
> 
> 	$ git describe 45d457a4cf244
> 	v5.7-rc5-92-g45d457a4cf24
> 
> I guess the solution would be to rename the new one.

We should rename the new one before anyone actually writes any new
userspace code for it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-11-09 17:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04 16:00 [PATCH 1/2] w1: w1_therm: Add sysfs entries to control conversion time and driver features Ivan Zaentsev
2020-09-04 16:00 ` [PATCH 2/2] w1: w1_therm: Add support for GXCAS GX20MH01 device Ivan Zaentsev
2020-10-06 13:19   ` Mauro Carvalho Chehab
2020-10-07  7:32     ` Ivan Zaentsev
2020-10-07  8:57       ` Mauro Carvalho Chehab
2020-10-07  9:06         ` Greg Kroah-Hartman
2020-10-07 11:05           ` Mauro Carvalho Chehab
2020-10-07 11:43             ` Greg Kroah-Hartman
2020-10-07 11:59               ` Mauro Carvalho Chehab
2020-10-21 16:28                 ` Adding ABI to htmldocs - Was: " Mauro Carvalho Chehab
2020-10-21 16:58                   ` Greg Kroah-Hartman
2020-10-22  8:19                     ` Mauro Carvalho Chehab
2020-10-29 14:28                     ` Mauro Carvalho Chehab
2020-11-09 17:32                       ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.