[RFC 00/11] hwmon: Modernize documentation

[RFC 00/11] hwmon: Modernize documentation

[James Seo]

To help summon the beautifully documented kernel of the future
promised by the move to ReST, this series revises the hwmon subsystem
documentation to make it more comprehensive and internally consistent.
The latter goal also requires a few API and ABI additions that should
have been in tree but weren't.

James Seo (11):
  Documentation/hwmon: Move misplaced entry in hwmon docs index
  hwmon: (core) Rename last parameter of devm_hwmon_register_with_info()
  hwmon: (core) Revise kerneldoc comments
  Documentation/hwmon: Revise hwmon kernel API reference
  Documentation/hwmon: Revise PMBus core documentation
  Documentation/hwmon: Revise patch submission checklist
  Documentation/hwmon: Revise sysfs interface specification
  Documentation/hwmon: Revise userspace tools documentation
  ABI: sysfs-class-hwmon: Revise hwmon ABI documentation
  hwmon: (core) Add missing beep-related standard attributes
  ABI: sysfs-class-hwmon: Add missing hwmon standard attributes

 Documentation/ABI/testing/sysfs-class-hwmon | 901 +++++++++++++-----
 Documentation/hwmon/hwmon-kernel-api.rst    | 740 ++++++++-------
 Documentation/hwmon/index.rst               |   2 +-
 Documentation/hwmon/pmbus-core.rst          | 348 ++++---
 Documentation/hwmon/submitting-patches.rst  | 110 +--
 Documentation/hwmon/sysfs-interface.rst     | 984 +++++++++-----------
 Documentation/hwmon/userspace-tools.rst     | 129 ++-
 drivers/hwmon/hwmon.c                       | 169 ++--
 include/linux/hwmon-sysfs.h                 |  19 +-
 include/linux/hwmon.h                       |  90 +-
 10 files changed, 2037 insertions(+), 1455 deletions(-)


base-commit: 1a5304fecee523060f26e2778d9d8e33c0562df3
-- 
2.34.1

[RFC 01/11] Documentation/hwmon: Move misplaced entry in hwmon docs index

[James Seo]

Move the entry for the inspur-ipsps1 driver so that it no
longer appears in the hwmon docs TOC as a document relating
to the hwmon subsystem itself.

Signed-off-by: James Seo <james@equiv.tech>
---
 Documentation/hwmon/index.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index fa1208c62855..03b30a94a9e6 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -9,7 +9,6 @@ Hardware Monitoring
 
    hwmon-kernel-api
    pmbus-core
-   inspur-ipsps1
    submitting-patches
    sysfs-interface
    userspace-tools
@@ -85,6 +84,7 @@ Hardware Monitoring Kernel Drivers
    ina2xx
    ina238
    ina3221
+   inspur-ipsps1
    intel-m10-bmc-hwmon
    ir35221
    ir38064
-- 
2.34.1

[RFC 02/11] hwmon: (core) Rename last parameter of devm_hwmon_register_with_info()

[James Seo]

Change the name of the groups parameter of
devm_hwmon_device_register_with_info() to extra_groups to
match the name given by the hwmon API reference and in
hwmon_device_register_with_info().

Signed-off-by: James Seo <james@equiv.tech>
---
 drivers/hwmon/hwmon.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 573b83b6c08c..5f205686065e 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -1029,7 +1029,7 @@ EXPORT_SYMBOL_GPL(devm_hwmon_device_register_with_groups);
  * @name:	hwmon name attribute
  * @drvdata:	driver data to attach to created device
  * @chip:	pointer to hwmon chip information
- * @groups:	pointer to list of driver specific attribute groups
+ * @extra_groups: pointer to list of driver specific attribute groups
  *
  * Returns the pointer to the new device. The new device is automatically
  * unregistered with the parent device.
@@ -1038,7 +1038,7 @@ struct device *
 devm_hwmon_device_register_with_info(struct device *dev, const char *name,
 				     void *drvdata,
 				     const struct hwmon_chip_info *chip,
-				     const struct attribute_group **groups)
+				     const struct attribute_group **extra_groups)
 {
 	struct device **ptr, *hwdev;
 
@@ -1050,7 +1050,7 @@ devm_hwmon_device_register_with_info(struct device *dev, const char *name,
 		return ERR_PTR(-ENOMEM);
 
 	hwdev = hwmon_device_register_with_info(dev, name, drvdata, chip,
-						groups);
+						extra_groups);
 	if (IS_ERR(hwdev))
 		goto error;
 
-- 
2.34.1

[RFC 03/11] hwmon: (core) Revise kerneldoc comments

[James Seo]

Move function and data structure documentation from hwmon
kernel API reference into kerneldocs. Fix minor issues
(spelling, grammar, etc.) and rewrite content as necessary so
that the resulting kerneldocs are less terse and suitable for
inclusion into docs.

Note that the following terms were introduced alongside
expanded explanations of their importance and usage:
* chip descriptor: struct hwmon_chip_info
* callback descriptor: struct hwmon_ops
* channel descriptor: struct hwmon_channel_info

Signed-off-by: James Seo <james@equiv.tech>
---
 drivers/hwmon/hwmon.c       | 160 ++++++++++++++++++++++--------------
 include/linux/hwmon-sysfs.h |  19 ++++-
 include/linux/hwmon.h       |  80 +++++++++++++-----
 3 files changed, 179 insertions(+), 80 deletions(-)

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 5f205686065e..cc140cf99290 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -864,16 +864,19 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
 }
 
 /**
- * hwmon_device_register_with_groups - register w/ hwmon
- * @dev: the parent device
- * @name: hwmon name attribute
- * @drvdata: driver data to attach to created device
- * @groups: List of attribute groups to create
+ * hwmon_device_register_with_groups - Register a hardware monitoring device
+ * (deprecated).
+ * @dev: A pointer to the parent device.
+ * @name: The ``hwmon`` device name (mandatory).
+ * @drvdata: A pointer to the private driver data structure.
+ * @groups: A pointer to a ``NULL``-terminated list of attribute groups.
  *
- * hwmon_device_unregister() must be called when the device is no
- * longer needed.
+ * The result is a newly allocated ``hwmon`` device with ``sysfs`` attributes
+ * as specified by @name and @groups.
  *
- * Returns the pointer to the new device.
+ * Return: On success: A pointer to the new ``hwmon`` device.
+ * hwmon_device_unregister() must be called on it when it is no longer needed.
+ * On failure, a &PTR_ERR.
  */
 struct device *
 hwmon_device_register_with_groups(struct device *dev, const char *name,
@@ -888,18 +891,25 @@ hwmon_device_register_with_groups(struct device *dev, const char *name,
 EXPORT_SYMBOL_GPL(hwmon_device_register_with_groups);
 
 /**
- * hwmon_device_register_with_info - register w/ hwmon
- * @dev: the parent device (mandatory)
- * @name: hwmon name attribute (mandatory)
- * @drvdata: driver data to attach to created device (optional)
- * @chip: pointer to hwmon chip information (mandatory)
- * @extra_groups: pointer to list of additional non-standard attribute groups
- *	(optional)
+ * hwmon_device_register_with_info - Register a hardware monitoring device.
+ * @dev: A pointer to the parent device (mandatory).
+ * @name: The ``hwmon`` device name (mandatory).
+ * @drvdata: A pointer to the private driver data structure.
+ * @chip: A pointer to the chip descriptor (mandatory).
+ * @extra_groups: A pointer to a ``NULL``-terminated list of additional
+ *	non-standard ``sysfs`` attribute groups.
  *
- * hwmon_device_unregister() must be called when the device is no
- * longer needed.
+ * Creates the standard ``sysfs`` attributes in the hardware monitoring core,
+ * letting the driver focus on reading from and writing to the chip instead of
+ * having to bother with ``sysfs`` attributes.
  *
- * Returns the pointer to the new device.
+ * The result is a newly allocated ``hwmon`` device with standard ``sysfs``
+ * attributes as specified by @name and @groups and additional attributes as
+ * specified by @extra_groups.
+ *
+ * Return: On success, a pointer to the new ``hwmon`` device.
+ * hwmon_device_unregister() must be called on it when it is no longer needed.
+ * On failure, a &PTR_ERR.
  */
 struct device *
 hwmon_device_register_with_info(struct device *dev, const char *name,
@@ -918,18 +928,18 @@ hwmon_device_register_with_info(struct device *dev, const char *name,
 EXPORT_SYMBOL_GPL(hwmon_device_register_with_info);
 
 /**
- * hwmon_device_register_for_thermal - register hwmon device for thermal subsystem
- * @dev: the parent device
- * @name: hwmon name attribute
- * @drvdata: driver data to attach to created device
+ * hwmon_device_register_for_thermal - Register a hardware monitoring device
+ * for the thermal subsystem (restricted).
+ * @dev: A pointer to the parent device.
+ * @name: The ``hwmon`` device name (mandatory).
+ * @drvdata: A pointer to the private driver data structure.
  *
  * The use of this function is restricted. It is provided for legacy reasons
  * and must only be called from the thermal subsystem.
  *
- * hwmon_device_unregister() must be called when the device is no
- * longer needed.
- *
- * Returns the pointer to the new device.
+ * Return: On success, a pointer to the new ``hwmon`` device.
+ * hwmon_device_unregister() must be called on it when it is no longer needed.
+ * On failure, a &PTR_ERR.
  */
 struct device *
 hwmon_device_register_for_thermal(struct device *dev, const char *name,
@@ -943,13 +953,12 @@ hwmon_device_register_for_thermal(struct device *dev, const char *name,
 EXPORT_SYMBOL_NS_GPL(hwmon_device_register_for_thermal, HWMON_THERMAL);
 
 /**
- * hwmon_device_register - register w/ hwmon
- * @dev: the device to register
- *
- * hwmon_device_unregister() must be called when the device is no
- * longer needed.
+ * hwmon_device_register - Register a hardware monitoring device (deprecated).
+ * @dev: A pointer to the device.
  *
- * Returns the pointer to the new device.
+ * Return: On success, a pointer to the new ``hwmon`` device.
+ * hwmon_device_unregister() must be called on it when it is no longer needed.
+ * On failure, a &PTR_ERR.
  */
 struct device *hwmon_device_register(struct device *dev)
 {
@@ -961,9 +970,12 @@ struct device *hwmon_device_register(struct device *dev)
 EXPORT_SYMBOL_GPL(hwmon_device_register);
 
 /**
- * hwmon_device_unregister - removes the previously registered class device
+ * hwmon_device_unregister - Unregister a previously registered hardware
+ * monitoring device.
+ * @dev: A pointer to the registered ``hwmon`` device (mandatory).
  *
- * @dev: the class device to destroy
+ * Must be called from the driver remove function if the hardware monitoring
+ * device was not registered with a device-managed registration function.
  */
 void hwmon_device_unregister(struct device *dev)
 {
@@ -986,14 +998,20 @@ static void devm_hwmon_release(struct device *dev, void *res)
 }
 
 /**
- * devm_hwmon_device_register_with_groups - register w/ hwmon
- * @dev: the parent device
- * @name: hwmon name attribute
- * @drvdata: driver data to attach to created device
- * @groups: List of attribute groups to create
+ * devm_hwmon_device_register_with_groups - Register a hardware monitoring
+ * device (deprecated).
+ * @dev: A pointer to the parent device (mandatory).
+ * @name: The ``hwmon`` device name (mandatory).
+ * @drvdata: A pointer to the private driver data structure.
+ * @groups: A pointer to a ``NULL``-terminated list of attribute groups.
  *
- * Returns the pointer to the new device. The new device is automatically
- * unregistered with the parent device.
+ * Similar to hwmon_device_register_with_groups(), but the ``hwmon`` device it
+ * creates is device-managed, meaning it does not have to be unregistered
+ * explicitly by hwmon_device_unregister().
+ *
+ * Return: On success, a pointer to the new ``hwmon`` device.
+ * The new device is automatically unregistered with the parent device.
+ * On failure, a &PTR_ERR.
  */
 struct device *
 devm_hwmon_device_register_with_groups(struct device *dev, const char *name,
@@ -1024,15 +1042,21 @@ devm_hwmon_device_register_with_groups(struct device *dev, const char *name,
 EXPORT_SYMBOL_GPL(devm_hwmon_device_register_with_groups);
 
 /**
- * devm_hwmon_device_register_with_info - register w/ hwmon
- * @dev:	the parent device
- * @name:	hwmon name attribute
- * @drvdata:	driver data to attach to created device
- * @chip:	pointer to hwmon chip information
- * @extra_groups: pointer to list of driver specific attribute groups
+ * devm_hwmon_device_register_with_info - Register a hardware monitoring device.
+ * @dev: A pointer to the parent device (mandatory).
+ * @name: The ``hwmon`` device name (mandatory).
+ * @drvdata: A pointer to the private driver data structure.
+ * @chip: A pointer to the chip descriptor (mandatory).
+ * @extra_groups: A pointer to a ``NULL``-terminated list of additional
+ *	non-standard ``sysfs`` attribute groups.
+ *
+ * Similar to hwmon_device_register_with_info(), but the ``hwmon`` device it
+ * creates is device-managed, meaning it does not have to be unregistered
+ * explicitly by hwmon_device_unregister().
  *
- * Returns the pointer to the new device. The new device is automatically
- * unregistered with the parent device.
+ * Return: On success, a pointer to the new ``hwmon`` device.
+ * The new device is automatically unregistered with the parent device.
+ * On failure, a &PTR_ERR.
  */
 struct device *
 devm_hwmon_device_register_with_info(struct device *dev, const char *name,
@@ -1073,9 +1097,18 @@ static int devm_hwmon_match(struct device *dev, void *res, void *data)
 }
 
 /**
- * devm_hwmon_device_unregister - removes a previously registered hwmon device
+ * devm_hwmon_device_unregister - Unregister a previously registered hardware
+ * monitoring device.
  *
- * @dev: the parent device of the device to unregister
+ * @dev: A pointer to the parent device of the device to unregister.
+ *
+ * Does not normally have to be called. Only needed for error handling, and
+ * then only if both of the following apply:
+ *
+ *   - the driver probe fails after the call to
+ *     devm_hwmon_device_register_with_info() (or to the deprecated
+ *     :c:func:`devm_hwmon_device_register_with_groups()`, if applicable), and
+ *   - the automatic (device-managed) removal would be too late.
  */
 void devm_hwmon_device_unregister(struct device *dev)
 {
@@ -1102,14 +1135,20 @@ static char *__hwmon_sanitize_name(struct device *dev, const char *old_name)
 }
 
 /**
- * hwmon_sanitize_name - Replaces invalid characters in a hwmon name
- * @name: NUL-terminated name
+ * hwmon_sanitize_name - Replaces invalid characters in a ``hwmon`` name.
+ * @name: The ``hwmon`` device name to be sanitized (mandatory).
+ *
+ * If the driver doesn't use a static device name (for example, it uses
+ * :c:func:`dev_name()`), and therefore cannot make sure the name only
+ * contains valid characters, this convenience function can be used.
  *
  * Allocates a new string where any invalid characters will be replaced
  * by an underscore. It is the responsibility of the caller to release
  * the memory.
  *
- * Returns newly allocated name, or ERR_PTR on error.
+ * Return: On success, a sanitized duplicate string.
+ * It must be freed using kfree() when it is no longer needed.
+ * On failure, a &PTR_ERR.
  */
 char *hwmon_sanitize_name(const char *name)
 {
@@ -1118,14 +1157,15 @@ char *hwmon_sanitize_name(const char *name)
 EXPORT_SYMBOL_GPL(hwmon_sanitize_name);
 
 /**
- * devm_hwmon_sanitize_name - resource managed hwmon_sanitize_name()
- * @dev: device to allocate memory for
- * @name: NUL-terminated name
+ * devm_hwmon_sanitize_name - Replaces invalid characters in a ``hwmon`` name.
+ * @dev: A pointer to the device to allocate memory for.
+ * @name: The ``hwmon`` device name to be sanitized (mandatory).
  *
- * Allocates a new string where any invalid characters will be replaced
- * by an underscore.
+ * Similar to hwmon_sanitize_name(), but device-managed, meaning its result
+ * does not have to be explicitly freed by kfree().
  *
- * Returns newly allocated name, or ERR_PTR on error.
+ * Return: On success, a sanitized duplicate string.
+ * On failure, a &PTR_ERR.
  */
 char *devm_hwmon_sanitize_name(struct device *dev, const char *name)
 {
diff --git a/include/linux/hwmon-sysfs.h b/include/linux/hwmon-sysfs.h
index d896713359cd..e49963dea342 100644
--- a/include/linux/hwmon-sysfs.h
+++ b/include/linux/hwmon-sysfs.h
@@ -10,7 +10,15 @@
 #include <linux/device.h>
 #include <linux/kstrtox.h>
 
-struct sensor_device_attribute{
+/**
+ * struct sensor_device_attribute - A convenience structure for defining
+ * ``sysfs`` attributes.
+ * @dev_attr: An exported device attribute.
+ * @index: Additional context.
+ *
+ * May be useful if manually defining attributes. Not needed otherwise.
+ */
+struct sensor_device_attribute {
 	struct device_attribute dev_attr;
 	int index;
 };
@@ -43,6 +51,15 @@ struct sensor_device_attribute sensor_dev_attr_##_name		\
 #define SENSOR_DEVICE_ATTR_WO(_name, _func, _index)		\
 	SENSOR_DEVICE_ATTR(_name, 0200, NULL, _func##_store, _index)
 
+/**
+ * struct sensor_device_attribute_2 - A convenience structure for defining
+ * ``sysfs`` attributes.
+ * @dev_attr: An exported device attribute.
+ * @index: Additional context.
+ * @nr: More additional context.
+ *
+ * Similar to struct sensor_device_attribute, but with even more context.
+ */
 struct sensor_device_attribute_2 {
 	struct device_attribute dev_attr;
 	u8 index;
diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
index 492dd27a5dd8..fe80e8e24b5a 100644
--- a/include/linux/hwmon.h
+++ b/include/linux/hwmon.h
@@ -17,6 +17,22 @@
 struct device;
 struct attribute_group;
 
+/**
+ * enum hwmon_sensor_types - Supported hardware monitoring sensor types.
+ * @hwmon_chip: Virtual sensor type used to describe attributes that are not
+ *	bound to a specific input or output.
+ * @hwmon_temp: Temperature sensor.
+ * @hwmon_in: Voltage sensor.
+ * @hwmon_curr: Current sensor.
+ * @hwmon_power: Power sensor.
+ * @hwmon_energy: Energy sensor.
+ * @hwmon_humidity: Humidity sensor.
+ * @hwmon_fan: Fan speed sensor.
+ * @hwmon_pwm: PWM control.
+ * @hwmon_intrusion: Chassis intrusion sensor.
+ * @hwmon_max: Not a sensor type. Code may refer to this value to determine
+ *	the number of currently supported sensor types.
+ */
 enum hwmon_sensor_types {
 	hwmon_chip,
 	hwmon_temp,
@@ -349,10 +365,9 @@ enum hwmon_intrusion_attributes {
 #define HWMON_INTRUSION_BEEP		BIT(hwmon_intrusion_beep)
 
 /**
- * struct hwmon_ops - hwmon device operations
+ * struct hwmon_ops - A ``hwmon`` callback descriptor.
  * @is_visible: Callback to return attribute visibility. Mandatory.
- *		Parameters are:
- *		@const void *drvdata:
+ *		@drvdata:
  *			Pointer to driver-private data structure passed
  *			as argument to hwmon_device_register_with_info().
  *		@type:	Sensor type
@@ -363,7 +378,6 @@ enum hwmon_intrusion_attributes {
  *		If the return value is 0, no attribute will be created.
  * @read:	Read callback for data attributes. Mandatory if readable
  *		data attributes are present.
- *		Parameters are:
  *		@dev:	Pointer to hardware monitoring device
  *		@type:	Sensor type
  *		@attr:	Sensor attribute
@@ -374,7 +388,6 @@ enum hwmon_intrusion_attributes {
  * @read_string:
  *		Read callback for string attributes. Mandatory if string
  *		attributes are present.
- *		Parameters are:
  *		@dev:	Pointer to hardware monitoring device
  *		@type:	Sensor type
  *		@attr:	Sensor attribute
@@ -384,7 +397,6 @@ enum hwmon_intrusion_attributes {
  *		The function returns 0 on success or a negative error number.
  * @write:	Write callback for data attributes. Mandatory if writeable
  *		data attributes are present.
- *		Parameters are:
  *		@dev:	Pointer to hardware monitoring device
  *		@type:	Sensor type
  *		@attr:	Sensor attribute
@@ -405,10 +417,39 @@ struct hwmon_ops {
 };
 
 /**
- * struct hwmon_channel_info - Channel information
- * @type:	Channel type.
- * @config:	Pointer to NULL-terminated list of channel parameters.
- *		Use for per-channel attributes.
+ * struct hwmon_channel_info - A ``hwmon`` channel descriptor.
+ * @type: The ``hwmon`` sensor type of the channels described.
+ * @config: A pointer to a ``0``-terminated list of channel configurations.
+ *	Use for per-channel attributes.
+ *
+ * Enumerates and specifies the standard ``sysfs`` attributes that should be
+ * created for all sensors of the same type that a chip provides.
+ *
+ * A *channel* refers to a single sensor, and a *channel configuration* is a
+ * :c:type:`u32` bitfield containing a combination of bit values describing
+ * that sensor's capabilities. A chip descriptor-aware hardware monitoring
+ * device registration function creates one or more standard ``sysfs``
+ * attributes for that sensor based on the capability bits set in its channel
+ * configuration.
+ *
+ * The complete list of bit values indicating individual attribute support is
+ * defined in ``<linux/hwmon.h>``. Definition prefixes are as follows:
+ *
+ * ============================ =======================================================
+ * Prefix			Description
+ * ============================ =======================================================
+ * ``HWMON_C_xxxx``		Chip attributes, for use with ``hwmon_chip``.
+ * ``HWMON_T_xxxx``		Temperature attributes, for use with ``hwmon_temp``.
+ * ``HWMON_I_xxxx``		Voltage attributes, for use with ``hwmon_in``.
+ * ``HWMON_C_xxxx``		Current attributes, for use with ``hwmon_curr``.
+ *				Notice the prefix overlap with chip attributes.
+ * ``HWMON_P_xxxx``		Power attributes, for use with ``hwmon_power``.
+ * ``HWMON_E_xxxx``		Energy attributes, for use with ``hwmon_energy``.
+ * ``HWMON_H_xxxx``		Humidity attributes, for use with ``hwmon_humidity``.
+ * ``HWMON_F_xxxx``		Fan speed attributes, for use with ``hwmon_fan``.
+ * ``HWMON_PWM_xxxx``		PWM control attributes, for use with ``hwmon_pwm``.
+ * ``HWMON_INTRUSION_xxxx``	Intrusion attributes, for use with ``hwmon_intrusion``.
+ * ============================ =======================================================
  */
 struct hwmon_channel_info {
 	enum hwmon_sensor_types type;
@@ -424,9 +465,13 @@ struct hwmon_channel_info {
 	})
 
 /**
- * struct hwmon_chip_info - Chip configuration
- * @ops:	Pointer to hwmon operations.
- * @info:	Null-terminated list of channel information.
+ * struct hwmon_chip_info - A ``hwmon`` chip descriptor.
+ * @ops: A pointer to the chip's callback descriptor.
+ * @info: A ``NULL``-terminated list of the chip's channel descriptors.
+ *
+ * Describes the capabilities of a chip (whether real or virtual), the types
+ * of its sensors, the number of sensors of each type, and the standard
+ * ``sysfs`` attributes that should be created for each sensor.
  */
 struct hwmon_chip_info {
 	const struct hwmon_ops *ops;
@@ -472,13 +517,10 @@ char *hwmon_sanitize_name(const char *name);
 char *devm_hwmon_sanitize_name(struct device *dev, const char *name);
 
 /**
- * hwmon_is_bad_char - Is the char invalid in a hwmon name
- * @ch: the char to be considered
- *
- * hwmon_is_bad_char() can be used to determine if the given character
- * may not be used in a hwmon name.
+ * hwmon_is_bad_char - Validate a char that might be used in a ``hwmon`` name.
+ * @ch: The char to be considered.
  *
- * Returns true if the char is invalid, false otherwise.
+ * Return: If the char would be invalid, ``true``. Otherwise, ``false``.
  */
 static inline bool hwmon_is_bad_char(const char ch)
 {
-- 
2.34.1

[RFC 04/11] Documentation/hwmon: Revise hwmon kernel API reference

[James Seo]

Reorganize content into coherent sections.
Use kerneldocs to document functions and data structures when
possible and add more details on using various API facilities.
Fix minor issues (typos, grammar, etc.) and add markup.

Signed-off-by: James Seo <james@equiv.tech>
---
 Documentation/hwmon/hwmon-kernel-api.rst | 740 ++++++++++++-----------
 1 file changed, 384 insertions(+), 356 deletions(-)

diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
index c2d1e0299d8d..cf084e040522 100644
--- a/Documentation/hwmon/hwmon-kernel-api.rst
+++ b/Documentation/hwmon/hwmon-kernel-api.rst
@@ -1,374 +1,402 @@
+========================================
 The Linux Hardware Monitoring kernel API
 ========================================
 
 Guenter Roeck
 
+.. contents::
+
 Introduction
-------------
+============
 
 This document describes the API that can be used by hardware monitoring
 drivers that want to use the hardware monitoring framework.
 
-This document does not describe what a hardware monitoring (hwmon) Driver or
-Device is. It also does not describe the API which can be used by user space
-to communicate with a hardware monitoring device. If you want to know this
-then please read the following file: Documentation/hwmon/sysfs-interface.rst.
+This document does not describe what a hardware monitoring (``hwmon``) driver
+or device is, nor does it describe the API for communicating with a hardware
+monitoring device from userspace. For more information on these topics,
+please read Documentation/hwmon/sysfs-interface.rst.
 
-For additional guidelines on how to write and improve hwmon drivers, please
-also read Documentation/hwmon/submitting-patches.rst.
+For additional guidelines on how to write and improve ``hwmon`` drivers,
+please also read Documentation/hwmon/submitting-patches.rst.
 
 The API
--------
-Each hardware monitoring driver must #include <linux/hwmon.h> and, in some
-cases, <linux/hwmon-sysfs.h>. linux/hwmon.h declares the following
-register/unregister functions::
-
-  struct device *
-  hwmon_device_register_with_info(struct device *dev,
-				  const char *name, void *drvdata,
-				  const struct hwmon_chip_info *info,
-				  const struct attribute_group **extra_groups);
-
-  struct device *
-  devm_hwmon_device_register_with_info(struct device *dev,
-				       const char *name,
-				       void *drvdata,
-				       const struct hwmon_chip_info *info,
-				       const struct attribute_group **extra_groups);
-
-  void hwmon_device_unregister(struct device *dev);
-
-  void devm_hwmon_device_unregister(struct device *dev);
-
-  char *hwmon_sanitize_name(const char *name);
-
-  char *devm_hwmon_sanitize_name(struct device *dev, const char *name);
-
-hwmon_device_register_with_info registers a hardware monitoring device.
-It creates the standard sysfs attributes in the hardware monitoring core,
-letting the driver focus on reading from and writing to the chip instead
-of having to bother with sysfs attributes. The parent device parameter
-as well as the chip parameter must not be NULL. Its parameters are described
-in more detail below.
-
-devm_hwmon_device_register_with_info is similar to
-hwmon_device_register_with_info. However, it is device managed, meaning the
-hwmon device does not have to be removed explicitly by the removal function.
-
-All other hardware monitoring device registration functions are deprecated
-and must not be used in new drivers.
-
-hwmon_device_unregister deregisters a registered hardware monitoring device.
-The parameter of this function is the pointer to the registered hardware
-monitoring device structure. This function must be called from the driver
-remove function if the hardware monitoring device was registered with
-hwmon_device_register_with_info.
-
-devm_hwmon_device_unregister does not normally have to be called. It is only
-needed for error handling, and only needed if the driver probe fails after
-the call to hwmon_device_register_with_info and if the automatic (device
-managed) removal would be too late.
-
-All supported hwmon device registration functions only accept valid device
-names. Device names including invalid characters (whitespace, '*', or '-')
-will be rejected. The 'name' parameter is mandatory.
-
-If the driver doesn't use a static device name (for example it uses
-dev_name()), and therefore cannot make sure the name only contains valid
-characters, hwmon_sanitize_name can be used. This convenience function
-will duplicate the string and replace any invalid characters with an
-underscore. It will allocate memory for the new string and it is the
-responsibility of the caller to release the memory when the device is
-removed.
-
-devm_hwmon_sanitize_name is the resource managed version of
-hwmon_sanitize_name; the memory will be freed automatically on device
-removal.
-
-Using devm_hwmon_device_register_with_info()
---------------------------------------------
-
-hwmon_device_register_with_info() registers a hardware monitoring device.
-The parameters to this function are
-
-=============================================== ===============================================
-`struct device *dev`				Pointer to parent device
-`const char *name`				Device name
-`void *drvdata`					Driver private data
-`const struct hwmon_chip_info *info`		Pointer to chip description.
-`const struct attribute_group **extra_groups` 	Null-terminated list of additional non-standard
-						sysfs attribute groups.
-=============================================== ===============================================
-
-This function returns a pointer to the created hardware monitoring device
-on success and a negative error code for failure.
-
-The hwmon_chip_info structure looks as follows::
-
-	struct hwmon_chip_info {
-		const struct hwmon_ops *ops;
-		const struct hwmon_channel_info * const *info;
-	};
-
-It contains the following fields:
-
-* ops:
-	Pointer to device operations.
-* info:
-	NULL-terminated list of device channel descriptors.
-
-The list of hwmon operations is defined as::
-
-  struct hwmon_ops {
-	umode_t (*is_visible)(const void *, enum hwmon_sensor_types type,
-			      u32 attr, int);
-	int (*read)(struct device *, enum hwmon_sensor_types type,
-		    u32 attr, int, long *);
-	int (*write)(struct device *, enum hwmon_sensor_types type,
-		     u32 attr, int, long);
-  };
+=======
+
+Each hardware monitoring driver must ``#include <linux/hwmon.h>`` and, in some
+cases, ``<linux/hwmon-sysfs.h>``.
+
+Functions
+---------
+
+``<linux/hwmon.h>`` declares several functions related to hardware monitoring
+device registration and deregistration.
+
+.. attention::
+  All functions not listed here are deprecated and **must not** be used in new
+  drivers.
+
+All supported ``hwmon`` device registration functions have a mandatory **name**
+parameter that must contain a valid device name. Device names including invalid
+characters (whitespace, '``*``', or '``-``') will be rejected. This becomes the
+device's ``name`` standard attribute in ``sysfs``.
+
+Drivers may later retrieve the pointer passed as the **drvdata** parameter to a
+registration function by using :c:func:`dev_get_drvdata()` on the pointer to the
+``hwmon`` device.
+
+These functions signal failure by returning "a :c:type:`PTR_ERR`", a pointer
+value containing an error code that can be obtained using :c:func:`PTR_ERR()`.
+
+``hwmon_device_register_with_info()``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. _hwmon_device_register_with_info:
+.. kernel-doc:: drivers/hwmon/hwmon.c
+  :identifiers: hwmon_device_register_with_info
+
+``devm_hwmon_device_register_with_info()``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. _devm_hwmon_device_register_with_info:
+.. kernel-doc:: drivers/hwmon/hwmon.c
+  :identifiers: devm_hwmon_device_register_with_info
+
+``hwmon_device_unregister()``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: drivers/hwmon/hwmon.c
+  :identifiers: hwmon_device_unregister
+
+``devm_hwmon_device_unregister()``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: drivers/hwmon/hwmon.c
+  :identifiers: devm_hwmon_device_unregister
+
+``hwmon_sanitize_name()``
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: drivers/hwmon/hwmon.c
+  :identifiers: hwmon_sanitize_name
+
+``devm_hwmon_sanitize_name()``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: drivers/hwmon/hwmon.c
+  :identifiers: devm_hwmon_sanitize_name
+
+Data structures
+---------------
+
+The following data structures are declared in ``<linux/hwmon.h>``,
+except for struct sensor_device_attribute and struct sensor_device_attribute,
+which are declared in ``<linux/hwmon_sysfs.h>``.
+
+``enum hwmon_sensor_types``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/linux/hwmon.h
+  :identifiers: hwmon_sensor_types
+
+``struct hwmon_chip_info``
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/linux/hwmon.h
+  :identifiers: hwmon_chip_info
+
+``struct hwmon_ops``
+~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/linux/hwmon.h
+  :identifiers: hwmon_ops
+
+**Description**
 
-It defines the following operations.
-
-* is_visible:
-    Pointer to a function to return the file mode for each supported
-    attribute. This function is mandatory.
-
-* read:
-    Pointer to a function for reading a value from the chip. This function
-    is optional, but must be provided if any readable attributes exist.
-
-* write:
-    Pointer to a function for writing a value to the chip. This function is
-    optional, but must be provided if any writeable attributes exist.
-
-Each sensor channel is described with struct hwmon_channel_info, which is
-defined as follows::
-
-	struct hwmon_channel_info {
-		enum hwmon_sensor_types type;
-		u32 *config;
-	};
-
-It contains following fields:
-
-* type:
-    The hardware monitoring sensor type.
-
-    Supported sensor types are
-
-     ================== ==================================================
-     hwmon_chip		A virtual sensor type, used to describe attributes
-			which are not bound to a specific input or output
-     hwmon_temp		Temperature sensor
-     hwmon_in		Voltage sensor
-     hwmon_curr		Current sensor
-     hwmon_power		Power sensor
-     hwmon_energy	Energy sensor
-     hwmon_humidity	Humidity sensor
-     hwmon_fan		Fan speed sensor
-     hwmon_pwm		PWM control
-     ================== ==================================================
-
-* config:
-    Pointer to a 0-terminated list of configuration values for each
-    sensor of the given type. Each value is a combination of bit values
-    describing the attributes supposed by a single sensor.
-
-As an example, here is the complete description file for a LM75 compatible
-sensor chip. The chip has a single temperature sensor. The driver wants to
-register with the thermal subsystem (HWMON_C_REGISTER_TZ), and it supports
-the update_interval attribute (HWMON_C_UPDATE_INTERVAL). The chip supports
-reading the temperature (HWMON_T_INPUT), it has a maximum temperature
-register (HWMON_T_MAX) as well as a maximum temperature hysteresis register
-(HWMON_T_MAX_HYST)::
-
-	static const u32 lm75_chip_config[] = {
-		HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL,
-		0
-	};
-
-	static const struct hwmon_channel_info lm75_chip = {
-		.type = hwmon_chip,
-		.config = lm75_chip_config,
-	};
-
-	static const u32 lm75_temp_config[] = {
-		HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST,
-		0
-	};
-
-	static const struct hwmon_channel_info lm75_temp = {
-		.type = hwmon_temp,
-		.config = lm75_temp_config,
-	};
-
-	static const struct hwmon_channel_info * const lm75_info[] = {
-		&lm75_chip,
-		&lm75_temp,
-		NULL
-	};
-
-	The HWMON_CHANNEL_INFO() macro can and should be used when possible.
-	With this macro, the above example can be simplified to
-
-	static const struct hwmon_channel_info * const lm75_info[] = {
-		HWMON_CHANNEL_INFO(chip,
-				HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL),
-		HWMON_CHANNEL_INFO(temp,
-				HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST),
-		NULL
-	};
-
-	The remaining declarations are as follows.
-
-	static const struct hwmon_ops lm75_hwmon_ops = {
-		.is_visible = lm75_is_visible,
-		.read = lm75_read,
-		.write = lm75_write,
-	};
-
-	static const struct hwmon_chip_info lm75_chip_info = {
-		.ops = &lm75_hwmon_ops,
-		.info = lm75_info,
-	};
-
-A complete list of bit values indicating individual attribute support
-is defined in include/linux/hwmon.h. Definition prefixes are as follows.
-
-=============== =================================================
-HWMON_C_xxxx	Chip attributes, for use with hwmon_chip.
-HWMON_T_xxxx	Temperature attributes, for use with hwmon_temp.
-HWMON_I_xxxx	Voltage attributes, for use with hwmon_in.
-HWMON_C_xxxx	Current attributes, for use with hwmon_curr.
-		Notice the prefix overlap with chip attributes.
-HWMON_P_xxxx	Power attributes, for use with hwmon_power.
-HWMON_E_xxxx	Energy attributes, for use with hwmon_energy.
-HWMON_H_xxxx	Humidity attributes, for use with hwmon_humidity.
-HWMON_F_xxxx	Fan speed attributes, for use with hwmon_fan.
-HWMON_PWM_xxxx	PWM control attributes, for use with hwmon_pwm.
-=============== =================================================
+Please see `Driver callback functions`_ for more information.
+
+``struct hwmon_channel_info``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/linux/hwmon.h
+  :identifiers: hwmon_channel_info
+
+``struct sensor_device_attribute``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/linux/hwmon-sysfs.h
+  :identifiers: sensor_device_attribute
+
+Please see `Driver-provided sysfs attributes`_ for usage scenarios.
+
+``struct sensor_device_attribute_2``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/linux/hwmon-sysfs.h
+  :identifiers: sensor_device_attribute_2
 
 Driver callback functions
 -------------------------
 
-Each driver provides is_visible, read, and write functions. Parameters
-and return values for those functions are as follows::
-
-  umode_t is_visible_func(const void *data, enum hwmon_sensor_types type,
-			  u32 attr, int channel)
-
-Parameters:
-	data:
-		Pointer to device private data structure.
-	type:
-		The sensor type.
-	attr:
-		Attribute identifier associated with a specific attribute.
-		For example, the attribute value for HWMON_T_INPUT would be
-		hwmon_temp_input. For complete mappings of bit fields to
-		attribute values please see include/linux/hwmon.h.
-	channel:
-		The sensor channel number.
-
-Return value:
-	The file mode for this attribute. Typically, this will be 0 (the
-	attribute will not be created), 0444, or 0644.
-
-::
-
-	int read_func(struct device *dev, enum hwmon_sensor_types type,
-		      u32 attr, int channel, long *val)
-
-Parameters:
-	dev:
-		Pointer to the hardware monitoring device.
-	type:
-		The sensor type.
-	attr:
-		Attribute identifier associated with a specific attribute.
-		For example, the attribute value for HWMON_T_INPUT would be
-		hwmon_temp_input. For complete mappings please see
-		include/linux/hwmon.h.
-	channel:
-		The sensor channel number.
-	val:
-		Pointer to attribute value.
-
-Return value:
-	0 on success, a negative error number otherwise.
-
-::
-
-	int write_func(struct device *dev, enum hwmon_sensor_types type,
-		       u32 attr, int channel, long val)
-
-Parameters:
-	dev:
-		Pointer to the hardware monitoring device.
-	type:
-		The sensor type.
-	attr:
-		Attribute identifier associated with a specific attribute.
-		For example, the attribute value for HWMON_T_INPUT would be
-		hwmon_temp_input. For complete mappings please see
-		include/linux/hwmon.h.
-	channel:
-		The sensor channel number.
-	val:
-		The value to write to the chip.
-
-Return value:
-	0 on success, a negative error number otherwise.
-
-
-Driver-provided sysfs attributes
---------------------------------
-
-In most situations it should not be necessary for a driver to provide sysfs
-attributes since the hardware monitoring core creates those internally.
-Only additional non-standard sysfs attributes need to be provided.
-
-The header file linux/hwmon-sysfs.h provides a number of useful macros to
-declare and use hardware monitoring sysfs attributes.
-
-In many cases, you can use the existing define DEVICE_ATTR or its variants
-DEVICE_ATTR_{RW,RO,WO} to declare such attributes. This is feasible if an
-attribute has no additional context. However, in many cases there will be
-additional information such as a sensor index which will need to be passed
-to the sysfs attribute handling function.
-
-SENSOR_DEVICE_ATTR and SENSOR_DEVICE_ATTR_2 can be used to define attributes
-which need such additional context information. SENSOR_DEVICE_ATTR requires
-one additional argument, SENSOR_DEVICE_ATTR_2 requires two.
-
-Simplified variants of SENSOR_DEVICE_ATTR and SENSOR_DEVICE_ATTR_2 are available
-and should be used if standard attribute permissions and function names are
-feasible. Standard permissions are 0644 for SENSOR_DEVICE_ATTR[_2]_RW,
-0444 for SENSOR_DEVICE_ATTR[_2]_RO, and 0200 for SENSOR_DEVICE_ATTR[_2]_WO.
-Standard functions, similar to DEVICE_ATTR_{RW,RO,WO}, have _show and _store
-appended to the provided function name.
-
-SENSOR_DEVICE_ATTR and its variants define a struct sensor_device_attribute
-variable. This structure has the following fields::
-
-	struct sensor_device_attribute {
-		struct device_attribute dev_attr;
-		int index;
-	};
-
-You can use to_sensor_dev_attr to get the pointer to this structure from the
-attribute read or write function. Its parameter is the device to which the
-attribute is attached.
-
-SENSOR_DEVICE_ATTR_2 and its variants define a struct sensor_device_attribute_2
-variable, which is defined as follows::
-
-	struct sensor_device_attribute_2 {
-		struct device_attribute dev_attr;
-		u8 index;
-		u8 nr;
-	};
-
-Use to_sensor_dev_attr_2 to get the pointer to this structure. Its parameter
-is the device to which the attribute is attached.
+.. kernel-docs do not render correctly for function pointer struct fields.
+
+If the driver is using one of the recommended
+`chip descriptor <#struct-hwmon-chip-info>`_-aware device registration functions
+(devm_hwmon_device_register_with_info() or hwmon_device_register_with_info()),
+it must implement callback functions with the function signatures below and fill
+in its chip descriptor's **ops** `callback descriptor <#struct-hwmon-ops>`_
+field as needed.
+
+If this is not the case, or if the driver provides additional non-standard
+attributes, please see `Driver-provided sysfs attributes`_.
+
+For complete mappings of **attr** bit fields to attribute values, please see
+``<linux/hwmon.h>``.
+
+.. note::
+  The callback function names below are for illustration only.
+
+``is_visible_func()``
+~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
+
+  umode_t is_visible_func(const void *drvdata, enum hwmon_sensor_types type,
+                          u32 attr, int channel)
+
+..
+
+  **Parameters**
+
+  ``const void *drvdata``
+    A pointer to the device private data structure.
+    This is the **drvdata** pointer provided during registration.
+
+  ``enum hwmon_sensor_types type``
+    The ``hwmon`` sensor type.
+
+  ``u32 attr``
+    The attribute identifier associated with a specific attribute.
+
+    For example, the attribute value for ``HWMON_T_INPUT`` would be
+    ``hwmon_temp_input``.
+
+  ``int channel``
+    The sensor channel number.
+
+  **Description**
+
+  Implements the ``hwmon`` callback :c:func:`is_visible()`. Mandatory.
+
+  Drivers request automatic creation of standard ``sysfs`` attributes by
+  specifying them in a `channel descriptor <#struct-hwmon-channel-info>`_
+  list. This function is called once per attribute during device registration
+  to determine the file mode that the attribute will have.
+
+  **Return**
+
+  The file mode for the specified attribute. Typically, this will be
+  ``0`` (the attribute will not be created after all), ``0444``, or ``0644``.
+
+``read_func()``
+~~~~~~~~~~~~~~~
+
+.. code-block:: c
+
+  int read_func(struct device *dev, enum hwmon_sensor_types type,
+                u32 attr, int channel, long *val)
+
+..
+
+  **Parameters**
+
+  ``struct device *dev``
+    A pointer to the hardware monitoring device.
+
+  ``enum hwmon_sensor_types type``
+    The ``hwmon`` sensor type.
+
+  ``u32 attr``
+    The attribute identifier associated with a specific attribute.
+
+    For example, the attribute value for ``HWMON_T_INPUT`` would be
+    ``hwmon_temp_input``.
+
+  ``int channel``
+    The sensor channel number.
+
+  ``long *val``
+    An out pointer to the attribute value.
+
+    A string representation of the value will be exported to
+    userspace as the result of reading from the attribute.
+
+  **Description**
+
+  Implements the ``hwmon`` callback :c:func:`read()`.
+  Mandatory if any readable attributes exist.
+
+  Called when the user reads from a readable ``sysfs`` attribute.
+
+  **Return**
+
+  On success, ``0``. On failure, a negative error code.
+
+``write_func()``
+~~~~~~~~~~~~~~~~
+
+.. code-block:: c
+
+  int write_func(struct device *dev, enum hwmon_sensor_types type,
+                 u32 attr, int channel, long val)
+
+..
+
+  **Parameters**
+
+  ``struct device *dev``
+    A pointer to the hardware monitoring device.
+
+  ``enum hwmon_sensor_types type``
+    The ``hwmon`` sensor type.
+
+  ``u32 attr``
+    The attribute identifier associated with a specific attribute.
+
+    For example, the attribute value for ``HWMON_T_INPUT`` would be
+    ``hwmon_temp_input``.
+
+  ``int channel``
+    The sensor channel number.
+
+  ``long val``
+    The value to write to the chip.
+
+    This is the string the user wrote to the ``sysfs`` attribute,
+    parsed to a ``long`` value.
+
+  **Description**
+  Implements the ``hwmon`` callback :c:func:`read()`. Mandatory if any
+  readable attributes exist.
+
+  Called when the user reads from a readable ``sysfs`` attribute.
+
+  **Return**
+
+  On success, ``0``. On failure, a negative error code.
+
+Driver-provided ``sysfs`` attributes
+------------------------------------
+
+.. note::
+  In most situations it should not be necessary for a driver to provide its own
+  ``sysfs`` attributes, since the hardware monitoring core creates those
+  internally.
+
+  Unless using a deprecated device registration function, only additional
+  non-standard ``sysfs`` attributes need to be provided, and non-standard
+  attributes are generally discouraged unless really needed.
+
+The header file ``<linux/hwmon-sysfs.h>`` provides a number of useful macros to
+declare and use hardware monitoring ``sysfs`` attributes.
+
+In many cases, you can use the existing ``DEVICE_ATTR`` macro from
+``<linux/device.h>`` or its variants ``SENSOR_ATTR_{RW,RO,WO}`` to declare such
+attributes. This is feasible if an attribute has no additional context. However,
+in many cases there will be additional information, such as a sensor index,
+which will need to be passed to the ``sysfs`` attribute handling function.
+
+The ``SENSOR_DEVICE_ATTR`` and ``SENSOR_DEVICE_ATTR_2`` macros can be used to
+define attributes which need such additional context information.
+``SENSOR_DEVICE_ATTR`` requires one additional argument.
+``SENSOR_DEVICE_ATTR_2`` requires two.
+
+Simplified variants of ``SENSOR_DEVICE_ATTR`` and ``SENSOR_DEVICE_ATTR_2`` are
+available, and should be used if standard attribute permissions and function
+names are feasible. Standard permissions are ``0644`` for
+``SENSOR_DEVICE_ATTR[_2]_RW``, ``0444`` for ``SENSOR_DEVICE_ATTR[_2]_RO``, and
+``0200`` for ``SENSOR_DEVICE_ATTR[_2]_WO``. Standard functions, similar to
+``DEVICE_ATTR_{RW,RO,WO}``, have ``_show`` and ``_store`` appended to the
+provided function name.
+
+``SENSOR_DEVICE_ATTR`` and its variants expand to a declaration of a struct
+sensor_device_attribute variable. You can use the :c:func:`to_sensor_dev_attr()`
+function-like macro to get the pointer to this structure from the attribute read
+or write function. Its parameter is the device to which the attribute is
+attached.
+
+``SENSOR_DEVICE_ATTR_2`` and its variants expand to a declaration of a struct
+sensor_device_attribute_2 variable. Use the :c:func:`to_sensor_dev_attr_2()`
+function-like macro to get the pointer to this structure. Its parameter is the
+device to which the attribute is attached.
+
+Example: An LM75-compatible sensor chip
+=======================================
+
+As an example, here is the `chip descriptor <#struct-hwmon-chip-info>`_ for an
+LM75-compatible sensor chip, comprising a
+`callback descriptor <#struct-hwmon-ops>`_  and a list of
+`channel descriptors <#struct-hwmon-channel-info>`_.
+
+First, consider the chip's channel descriptors. Notice the following:
+
+* The chip has a single temperature sensor.
+* The driver wants to register with the thermal subsystem
+  (``HWMON_C_REGISTER_TZ``), and it supports the ``update_interval`` attribute
+  (``HWMON_C_UPDATE_INTERVAL``), requiring a virtual channel to be defined.
+* The chip supports reading the temperature (``HWMON_T_INPUT``), and it has a
+  maximum temperature register (``HWMON_T_MAX``) as well as a maximum
+  temperature hysteresis register (``HWMON_T_MAX_HYST``).
+
+.. code-block:: c
+
+  static const u32 lm75_chip_config[] = {
+          HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL,
+          0
+  };
+
+  static const struct hwmon_channel_info lm75_chip = {
+          .type = hwmon_chip,
+          .config = lm75_chip_config,
+  };
+
+  static const u32 lm75_temp_config[] = {
+          HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST,
+          0
+  };
+
+  static const struct hwmon_channel_info lm75_temp = {
+          .type = hwmon_temp,
+          .config = lm75_temp_config,
+  };
+
+  static const struct hwmon_channel_info * const lm75_info[] = {
+          &lm75_chip,
+          &lm75_temp,
+          NULL
+  };
+
+There is also a ``HWMON_CHANNEL_INFO()`` macro that can and should be used when
+possible in order to simplify the declaration of a channel descriptor list.
+With this macro, the complete chip descriptor definition becomes:
+
+.. code-block:: c
+
+  static const struct hwmon_channel_info * const lm75_info[] = {
+          HWMON_CHANNEL_INFO(chip,
+                             HWMON_C_REGISTER_TZ | HWMON_C_UPDATE_INTERVAL),
+          HWMON_CHANNEL_INFO(temp,
+                             HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST),
+          NULL
+  };
+
+  static const struct hwmon_ops lm75_hwmon_ops = {
+          .is_visible = lm75_is_visible,
+          .read = lm75_read,
+          .write = lm75_write,
+  };
+
+  static const struct hwmon_chip_info lm75_chip_info = {
+          .ops = &lm75_hwmon_ops,
+          .info = lm75_info,
+  };
-- 
2.34.1

[RFC 05/11] Documentation/hwmon: Revise PMBus core documentation

[James Seo]

Organize content into sections.
Fix minor issues (typos, grammar, etc.) and add markup.

Signed-off-by: James Seo <james@equiv.tech>
---
 Documentation/hwmon/pmbus-core.rst | 348 +++++++++++++++++------------
 1 file changed, 199 insertions(+), 149 deletions(-)

diff --git a/Documentation/hwmon/pmbus-core.rst b/Documentation/hwmon/pmbus-core.rst
index cff93adf6e42..2f88a03446d2 100644
--- a/Documentation/hwmon/pmbus-core.rst
+++ b/Documentation/hwmon/pmbus-core.rst
@@ -2,75 +2,82 @@
 PMBus core driver and internal API
 ==================================
 
+.. contents::
+
 Introduction
 ============
 
-[from pmbus.org] The Power Management Bus (PMBus) is an open standard
-power-management protocol with a fully defined command language that facilitates
-communication with power converters and other devices in a power system. The
-protocol is implemented over the industry-standard SMBus serial interface and
-enables programming, control, and real-time monitoring of compliant power
-conversion products. This flexible and highly versatile standard allows for
-communication between devices based on both analog and digital technologies, and
-provides true interoperability which will reduce design complexity and shorten
-time to market for power system designers. Pioneered by leading power supply and
-semiconductor companies, this open power system standard is maintained and
-promoted by the PMBus Implementers Forum (PMBus-IF), comprising 30+ adopters
-with the objective to provide support to, and facilitate adoption among, users.
+  The Power Management Bus (PMBus) is an open standard power-management protocol
+  with a fully defined command language that facilitates communication with
+  power converters and other devices in a power system. The protocol is
+  implemented over the industry-standard SMBus serial interface and enables
+  programming, control, and real-time monitoring of compliant power conversion
+  products. This flexible and highly versatile standard allows for communication
+  between devices based on both analog and digital technologies, and provides
+  true interoperability which will reduce design complexity and shorten time to
+  market for power system designers. Pioneered by leading power supply and
+  semiconductor companies, this open power system standard is maintained and
+  promoted by the PMBus Implementers Forum (PMBus-IF), comprising 30+ adopters
+  with the objective to provide support to, and facilitate adoption among,
+  users.
+
+    (from pmbus.org)
 
 Unfortunately, while PMBus commands are standardized, there are no mandatory
 commands, and manufacturers can add as many non-standard commands as they like.
-Also, different PMBUs devices act differently if non-supported commands are
-executed. Some devices return an error, some devices return 0xff or 0xffff and
-set a status error flag, and some devices may simply hang up.
+Also, different PMBus devices act differently if non-supported commands are
+executed. Some devices return an error, some devices return ``0xff`` or
+``0xffff`` and set a status error flag, and some devices may simply hang up.
 
 Despite all those difficulties, a generic PMBus device driver is still useful
-and supported since kernel version 2.6.39. However, it was necessary to support
-device specific extensions in addition to the core PMBus driver, since it is
-simply unknown what new device specific functionality PMBus device developers
-come up with next.
+and has been supported since kernel version 2.6.39. However, it was necessary to
+support device-specific extensions in addition to the core PMBus driver, since
+it is simply unknown what new device-specific functionality PMBus device
+developers will come up with next.
 
 To make device specific extensions as scalable as possible, and to avoid having
 to modify the core PMBus driver repeatedly for new devices, the PMBus driver was
-split into core, generic, and device specific code. The core code (in
-pmbus_core.c) provides generic functionality. The generic code (in pmbus.c)
-provides support for generic PMBus devices. Device specific code is responsible
-for device specific initialization and, if needed, maps device specific
-functionality into generic functionality. This is to some degree comparable
-to PCI code, where generic code is augmented as needed with quirks for all kinds
-of devices.
+split into core, generic, and device-specific code.
+The core code (in ``pmbus_core.c``) provides generic functionality.
+The generic code (in ``pmbus.c``) provides support for generic PMBus devices.
+Device-specific code is responsible for device-specific initialization and, if
+needed, maps device-specific functionality into generic functionality.
+This is to some degree comparable to PCI code, where generic code is augmented
+as needed with quirks for all kinds of devices.
 
 PMBus device capabilities auto-detection
 ========================================
 
-For generic PMBus devices, code in pmbus.c attempts to auto-detect all supported
-PMBus commands. Auto-detection is somewhat limited, since there are simply too
-many variables to consider. For example, it is almost impossible to autodetect
-which PMBus commands are paged and which commands are replicated across all
-pages (see the PMBus specification for details on multi-page PMBus devices).
+For generic PMBus devices, code in ``pmbus.c`` attempts to auto-detect all
+supported PMBus commands. Auto-detection is somewhat limited, since there are
+simply too many variables to consider. For example, it is almost impossible to
+autodetect which PMBus commands are paged and which commands are replicated
+across all pages (see the PMBus specification for details on multi-page PMBus
+devices).
 
-For this reason, it often makes sense to provide a device specific driver if not
+For this reason, it often makes sense to provide a device-specific driver if not
 all commands can be auto-detected. The data structures in this driver can be
 used to inform the core driver about functionality supported by individual
 chips.
 
 Some commands are always auto-detected. This applies to all limit commands
-(lcrit, min, max, and crit attributes) as well as associated alarm attributes.
-Limits and alarm attributes are auto-detected because there are simply too many
-possible combinations to provide a manual configuration interface.
+(``lcrit``, ``min``, ``max``, and ``crit`` attributes) as well as associated
+``alarm`` attributes. Limits and alarm attributes are auto-detected because
+there are simply too many possible combinations to provide a manual
+configuration interface.
 
 PMBus internal API
 ==================
 
 The API between core and device specific PMBus code is defined in
-drivers/hwmon/pmbus/pmbus.h. In addition to the internal API, pmbus.h defines
-standard PMBus commands and virtual PMBus commands.
+``drivers/hwmon/pmbus/pmbus.h``. In addition to the internal API, ``pmbus.h``
+defines standard PMBus commands and virtual PMBus commands.
 
 Standard PMBus commands
 -----------------------
 
-Standard PMBus commands (commands values 0x00 to 0xff) are defined in the PMBUs
-specification.
+Standard PMBus commands (commands values ``0x00`` to ``0xff``) are defined in
+the PMBus specification.
 
 Virtual PMBus commands
 ----------------------
@@ -79,26 +86,26 @@ Virtual PMBus commands are provided to enable support for non-standard
 functionality which has been implemented by several chip vendors and is thus
 desirable to support.
 
-Virtual PMBus commands start with command value 0x100 and can thus easily be
-distinguished from standard PMBus commands (which can not have values larger
-than 0xff). Support for virtual PMBus commands is device specific and thus has
-to be implemented in device specific code.
+Virtual PMBus commands start with command value ``0x100`` and can thus easily be
+distinguished from standard PMBus commands (which cannot have values larger than
+``0xff``). Support for virtual PMBus commands is device-specific and thus has
+to be implemented in device-specific code.
 
-Virtual commands are named PMBUS_VIRT_xxx and start with PMBUS_VIRT_BASE. All
-virtual commands are word sized.
+Virtual commands are named ``PMBUS_VIRT_xxx`` and start with
+``PMBUS_VIRT_BASE``. All virtual commands are word-sized.
 
 There are currently two types of virtual commands.
 
-- READ commands are read-only; writes are either ignored or return an error.
-- RESET commands are read/write. Reading reset registers returns zero
-  (used for detection), writing any value causes the associated history to be
+- ``READ`` commands are read-only; writes are either ignored or return an error.
+- ``RESET`` commands are read/write. Reading reset registers returns zero
+  (used for detection). Writing any value causes the associated history to be
   reset.
 
-Virtual commands have to be handled in device specific driver code. Chip driver
+Virtual commands have to be handled in device-specific driver code. Chip driver
 code returns non-negative values if a virtual command is supported, or a
-negative error code if not. The chip driver may return -ENODATA or any other
-Linux error code in this case, though an error code other than -ENODATA is
-handled more efficiently and thus preferred. Either case, the calling PMBus
+negative error code if not. The chip driver may return ``-ENODATA`` or any other
+Linux error code in this case, though an error code other than ``-ENODATA`` is
+handled more efficiently and thus preferred. In either case, the calling PMBus
 core code will abort if the chip driver returns an error code when reading
 or writing virtual registers (in other words, the PMBus core code will never
 send a virtual command to a chip).
@@ -106,8 +113,8 @@ send a virtual command to a chip).
 PMBus driver information
 ------------------------
 
-PMBus driver information, defined in struct pmbus_driver_info, is the main means
-for device specific drivers to pass information to the core PMBus driver.
+PMBus driver information, defined in ``struct pmbus_driver_info``, is the main
+means for device specific drivers to pass information to the core PMBus driver.
 Specifically, it provides the following information.
 
 - For devices supporting its data in Direct Data Format, it provides coefficients
@@ -119,14 +126,14 @@ Specifically, it provides the following information.
 - Several function entry points are provided to support overriding and/or
   augmenting generic command execution. This functionality can be used to map
   non-standard PMBus commands to standard commands, or to augment standard
-  command return values with device specific information.
+  command return values with device-specific information.
 
 PEC Support
 ===========
 
 Many PMBus devices support SMBus PEC (Packet Error Checking). If supported
 by both the I2C adapter and by the PMBus chip, it is by default enabled.
-If PEC is supported, the PMBus core driver adds an attribute named 'pec' to
+If PEC is supported, the PMBus core driver adds an attribute named ``pec`` to
 the I2C device. This attribute can be used to control PEC support in the
 communication with the PMBus chip.
 
@@ -137,69 +144,83 @@ Functions provided by chip driver
 ---------------------------------
 
 All functions return the command return value (read) or zero (write) if
-successful. A return value of -ENODATA indicates that there is no manufacturer
-specific command, but that a standard PMBus command may exist. Any other
-negative return value indicates that the commands does not exist for this
+successful. A return value of ``-ENODATA`` indicates that there is no
+manufacturer-specific command, but that a standard PMBus command may exist. Any
+other negative return value indicates that the command does not exist for this
 chip, and that no attempt should be made to read or write the standard
 command.
 
 As mentioned above, an exception to this rule applies to virtual commands,
-which *must* be handled in driver specific code. See "Virtual PMBus Commands"
+which **must** be handled in driver specific code. See "Virtual PMBus Commands"
 above for more details.
 
-Command execution in the core PMBus driver code is as follows::
+Command execution in the core PMBus driver code is as follows:
+
+.. code-block:: c
 
-	if (chip_access_function) {
-		status = chip_access_function();
-		if (status != -ENODATA)
-			return status;
-	}
-	if (command >= PMBUS_VIRT_BASE)	/* For word commands/registers only */
-		return -EINVAL;
-	return generic_access();
+  if (chip_access_function) {
+          status = chip_access_function();
+          if (status != -ENODATA)
+                  return status;
+  }
+  if (command >= PMBUS_VIRT_BASE) /* For word commands/registers only */
+          return -EINVAL;
+  return generic_access();
 
-Chip drivers may provide pointers to the following functions in struct
-pmbus_driver_info. All functions are optional.
+Chip drivers may provide pointers to the following functions in
+``struct pmbus_driver_info``. All functions are optional.
 
-::
+``read_byte_data()``
+~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
 
   int (*read_byte_data)(struct i2c_client *client, int page, int reg);
 
-Read byte from page <page>, register <reg>.
-<page> may be -1, which means "current page".
+Read byte from page **page**, register **reg**.
+**page** may be ``-1``, which means "current page".
+
+``read_word_data()``
+~~~~~~~~~~~~~~~~~~~~
 
+.. code-block:: c
 
-::
+  int (*read_word_data)(struct i2c_client *client, int page, int phase, int reg);
 
-  int (*read_word_data)(struct i2c_client *client, int page, int phase,
-                        int reg);
+Read word from page **page**, phase **phase**, register **reg**. If the chip
+does not support multiple phases, the **phase** parameter can be ignored. If the
+chip supports multiple phases, a phase value of ``0xff`` indicates all phases.
 
-Read word from page <page>, phase <phase>, register <reg>. If the chip does not
-support multiple phases, the phase parameter can be ignored. If the chip
-supports multiple phases, a phase value of 0xff indicates all phases.
+``write_word_data()``
+~~~~~~~~~~~~~~~~~~~~~
 
-::
+.. code-block:: c
 
-  int (*write_word_data)(struct i2c_client *client, int page, int reg,
-			 u16 word);
+  int (*write_word_data)(struct i2c_client *client, int page, int reg, u16 word);
 
-Write word to page <page>, register <reg>.
+Write word to page **page**, register **reg**.
 
-::
+``write_byte()``
+~~~~~~~~~~~~~~~~
+
+.. code-block:: c
 
   int (*write_byte)(struct i2c_client *client, int page, u8 value);
 
-Write byte to page <page>, register <reg>.
-<page> may be -1, which means "current page".
+Write byte to page **page**, register **reg**.
+**page** may be ``-1``, which means "current page".
+
+``identify()``
+~~~~~~~~~~~~~~
 
-::
+.. code-block:: c
 
   int (*identify)(struct i2c_client *client, struct pmbus_driver_info *info);
 
 Determine supported PMBus functionality. This function is only necessary
 if a chip driver supports multiple chips, and the chip functionality is not
 pre-determined. It is currently only used by the generic pmbus driver
-(pmbus.c).
+(``pmbus.c``).
 
 Functions exported by core driver
 ---------------------------------
@@ -208,119 +229,148 @@ Chip drivers are expected to use the following functions to read or write
 PMBus registers. Chip drivers may also use direct I2C commands. If direct I2C
 commands are used, the chip driver code must not directly modify the current
 page, since the selected page is cached in the core driver and the core driver
-will assume that it is selected. Using pmbus_set_page() to select a new page
-is mandatory.
+will assume that it is selected. Using :c:func:`pmbus_set_page()` to select a
+new page is mandatory.
 
-::
+``pmbus_set_page()``
+~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
 
   int pmbus_set_page(struct i2c_client *client, u8 page, u8 phase);
 
-Set PMBus page register to <page> and <phase> for subsequent commands.
-If the chip does not support multiple phases, the phase parameter is
-ignored. Otherwise, a phase value of 0xff selects all phases.
+Set PMBus page register to **page** and **phase** for subsequent commands.
+If the chip does not support multiple phases, the **phase** parameter is
+ignored. Otherwise, a phase value of ``0xff`` selects all phases.
+
+``pmbus_read_word_data()``
+~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-::
+.. code-block:: c
 
-  int pmbus_read_word_data(struct i2c_client *client, u8 page, u8 phase,
-                           u8 reg);
+  int pmbus_read_word_data(struct i2c_client *client, u8 page, u8 phase, u8 reg);
 
-Read word data from <page>, <phase>, <reg>. Similar to
+Read word data from **page**, **phase**, **reg**. Similar to
 i2c_smbus_read_word_data(), but selects page and phase first. If the chip does
 not support multiple phases, the phase parameter is ignored. Otherwise, a phase
-value of 0xff selects all phases.
+value of ``0xff`` selects all phases.
 
-::
+``pmbus_write_word_data()``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-  int pmbus_write_word_data(struct i2c_client *client, u8 page, u8 reg,
-			    u16 word);
+.. code-block:: c
 
-Write word data to <page>, <reg>. Similar to i2c_smbus_write_word_data(), but
-selects page first.
+  int pmbus_write_word_data(struct i2c_client *client, u8 page, u8 reg, u16 word);
 
-::
+Write word data to **page**, **reg**. Similar to i2c_smbus_write_word_data(),
+but selects page first.
+
+``pmbus_read_byte_data()``
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
 
   int pmbus_read_byte_data(struct i2c_client *client, int page, u8 reg);
 
-Read byte data from <page>, <reg>. Similar to i2c_smbus_read_byte_data(), but
-selects page first. <page> may be -1, which means "current page".
+Read byte data from **page**, **reg**. Similar to i2c_smbus_read_byte_data(),
+but selects page first. **page** may be ``-1``, which means "current page".
 
-::
+``pmbus_write_byte()``
+~~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
 
   int pmbus_write_byte(struct i2c_client *client, int page, u8 value);
 
-Write byte data to <page>, <reg>. Similar to i2c_smbus_write_byte(), but
-selects page first. <page> may be -1, which means "current page".
+Write byte data to **page**, **reg**. Similar to i2c_smbus_write_byte(), but
+selects page first. **page** may be ``-1``, which means "current page".
+
+``pmbus_clear_faults()``
+~~~~~~~~~~~~~~~~~~~~~~~~
 
-::
+.. code-block:: c
 
   void pmbus_clear_faults(struct i2c_client *client);
 
 Execute PMBus "Clear Fault" command on all chip pages.
 This function calls the device specific write_byte function if defined.
-Therefore, it must _not_ be called from that function.
+Therefore, it **must not** be called from that function.
 
-::
+``pmbus_check_byte_register()``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
 
   bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
 
-Check if byte register exists. Return true if the register exists, false
+Check if byte register exists. Return ``true`` if the register exists, ``false``
 otherwise.
 This function calls the device specific write_byte function if defined to
-obtain the chip status. Therefore, it must _not_ be called from that function.
+obtain the chip status. Therefore, it **must not** be called from that function.
+
+``pmbus_check_word_register()``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-::
+.. code-block:: c
 
   bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);
 
-Check if word register exists. Return true if the register exists, false
+Check if word register exists. Return ``true`` if the register exists, ``false``
 otherwise.
 This function calls the device specific write_byte function if defined to
-obtain the chip status. Therefore, it must _not_ be called from that function.
+obtain the chip status. Therefore, it **must not** be called from that function.
 
-::
+``pmbus_do_probe()``
+~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
 
   int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info *info);
 
 Execute probe function. Similar to standard probe function for other drivers,
-with the pointer to struct pmbus_driver_info as additional argument. Calls
+with the pointer to ``struct pmbus_driver_info`` as additional argument. Calls
 identify function if supported. Must only be called from device probe
 function.
 
-::
+``pmbus_driver_info()``
+~~~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: c
 
-  const struct pmbus_driver_info
-	*pmbus_get_driver_info(struct i2c_client *client);
+  const struct pmbus_driver_info *pmbus_get_driver_info(struct i2c_client *client);
 
-Return pointer to struct pmbus_driver_info as passed to pmbus_do_probe().
+Return pointer to ``struct pmbus_driver_info`` as passed to
+:c:func:`pmbus_do_probe()`.
 
 
 PMBus driver platform data
 ==========================
 
-PMBus platform data is defined in include/linux/pmbus.h. Platform data
-currently provides a flags field with four bits used::
+PMBus platform data is defined in ``<linux/pmbus.h>``. Platform data
+currently provides a flags field with four bits used:
 
-	#define PMBUS_SKIP_STATUS_CHECK			BIT(0)
+.. code-block:: c
 
-	#define PMBUS_WRITE_PROTECTED			BIT(1)
+  #define PMBUS_SKIP_STATUS_CHECK                 BIT(0)
 
-	#define PMBUS_NO_CAPABILITY			BIT(2)
+  #define PMBUS_WRITE_PROTECTED                   BIT(1)
 
-	#define PMBUS_READ_STATUS_AFTER_FAILED_CHECK	BIT(3)
+  #define PMBUS_NO_CAPABILITY                     BIT(2)
 
-	struct pmbus_platform_data {
-		u32 flags;              /* Device specific flags */
+  #define PMBUS_READ_STATUS_AFTER_FAILED_CHECK    BIT(3)
 
-		/* regulator support */
-		int num_regulators;
-		struct regulator_init_data *reg_init_data;
-	};
+  struct pmbus_platform_data {
+          u32 flags;              /* Device specific flags */
 
+          /* regulator support */
+          int num_regulators;
+          struct regulator_init_data *reg_init_data;
+  };
 
 Flags
 -----
 
-PMBUS_SKIP_STATUS_CHECK
+``PMBUS_SKIP_STATUS_CHECK``
 
 During register detection, skip checking the status register for
 communication or command errors.
@@ -328,33 +378,33 @@ communication or command errors.
 Some PMBus chips respond with valid data when trying to read an unsupported
 register. For such chips, checking the status register is mandatory when
 trying to determine if a chip register exists or not.
-Other PMBus chips don't support the STATUS_CML register, or report
+Other PMBus chips don't support the ``STATUS_CML`` register, or report
 communication errors for no explicable reason. For such chips, checking the
 status register must be disabled.
 
 Some i2c controllers do not support single-byte commands (write commands with
 no data, i2c_smbus_write_byte()). With such controllers, clearing the status
-register is impossible, and the PMBUS_SKIP_STATUS_CHECK flag must be set.
+register is impossible, and the ``PMBUS_SKIP_STATUS_CHECK`` flag must be set.
 
-PMBUS_WRITE_PROTECTED
+``PMBUS_WRITE_PROTECTED``
 
 Set if the chip is write protected and write protection is not determined
-by the standard WRITE_PROTECT command.
+by the standard ``WRITE_PROTECT`` command.
 
-PMBUS_NO_CAPABILITY
+``PMBUS_NO_CAPABILITY``
 
-Some PMBus chips don't respond with valid data when reading the CAPABILITY
+Some PMBus chips don't respond with valid data when reading the ``CAPABILITY``
 register. For such chips, this flag should be set so that the PMBus core
-driver doesn't use CAPABILITY to determine it's behavior.
+driver doesn't use ``CAPABILITY`` to determine it's behavior.
 
-PMBUS_READ_STATUS_AFTER_FAILED_CHECK
+``PMBUS_READ_STATUS_AFTER_FAILED_CHECK``
 
-Read the STATUS register after each failed register check.
+Read the ``STATUS`` register after each failed register check.
 
 Some PMBus chips end up in an undefined state when trying to read an
 unsupported register. For such chips, it is necessary to reset the
-chip pmbus controller to a known state after a failed register check.
+chip PMBus controller to a known state after a failed register check.
 This can be done by reading a known register. By setting this flag the
-driver will try to read the STATUS register after each failed
+driver will try to read the ``STATUS`` register after each failed
 register check. This read may fail, but it will put the chip into a
 known state.
-- 
2.34.1

[RFC 06/11] Documentation/hwmon: Revise patch submission checklist

[James Seo]

Drop capitals from document title for consistency.
Fix minor issues (typos, grammar, etc.) and add markup.

Signed-off-by: James Seo <james@equiv.tech>
---
 Documentation/hwmon/submitting-patches.rst | 110 +++++++++++----------
 1 file changed, 58 insertions(+), 52 deletions(-)

diff --git a/Documentation/hwmon/submitting-patches.rst b/Documentation/hwmon/submitting-patches.rst
index 6482c4f137dc..3ef4b20223ea 100644
--- a/Documentation/hwmon/submitting-patches.rst
+++ b/Documentation/hwmon/submitting-patches.rst
@@ -1,13 +1,15 @@
-How to Get Your Patch Accepted Into the Hwmon Subsystem
 =======================================================
+How to get your patch accepted into the hwmon subsystem
+=======================================================
+
+.. contents::
 
 This text is a collection of suggestions for people writing patches or
-drivers for the hwmon subsystem. Following these suggestions will greatly
+drivers for the ``hwmon`` subsystem. Following these suggestions will greatly
 increase the chances of your change being accepted.
 
-
-1. General
-----------
+General
+=======
 
 * It should be unnecessary to mention, but please read and follow:
 
@@ -15,69 +17,69 @@ increase the chances of your change being accepted.
     - Documentation/process/submitting-patches.rst
     - Documentation/process/coding-style.rst
 
-* Please run your patch through 'checkpatch --strict'. There should be no
-  errors, no warnings, and few if any check messages. If there are any
-  messages, please be prepared to explain.
+* Please run your patch through ``scripts/checkpatch.pl --strict``.
+  There should be no errors, no warnings, and few if any check messages.
+  If there are any messages, please be prepared to explain.
 
 * Please use the standard multi-line comment style. Do not mix C and C++
   style comments in a single driver (with the exception of the SPDX license
   identifier).
 
-* If your patch generates checkpatch errors, warnings, or check messages,
+* If your patch generates ``checkpatch`` errors, warnings, or check messages,
   please refrain from explanations such as "I prefer that coding style".
-  Keep in mind that each unnecessary message helps hiding a real problem,
+  Keep in mind that each unnecessary message helps to hide a real problem,
   and a consistent coding style makes it easier for others to understand
   and review the code.
 
 * Please test your patch thoroughly. We are not your test group.
-  Sometimes a patch can not or not completely be tested because of missing
+  Sometimes a patch cannot or cannot completely be tested because of missing
   hardware. In such cases, you should test-build the code on at least one
-  architecture. If run-time testing was not achieved, it should be written
+  architecture. If runtime testing was not achieved, this should be written
   explicitly below the patch header.
 
 * If your patch (or the driver) is affected by configuration options such as
-  CONFIG_SMP, make sure it compiles for all configuration variants.
+  ``CONFIG_SMP``, make sure it compiles for all configuration variants.
 
 
-2. Adding functionality to existing drivers
--------------------------------------------
+Adding functionality to existing drivers
+========================================
 
-* Make sure the documentation in Documentation/hwmon/<driver_name>.rst is up to
-  date.
+* Make sure the documentation in ``Documentation/hwmon/<driver_name>.rst`` is
+  up-to-date.
 
-* Make sure the information in Kconfig is up to date.
+* Make sure the information in ``Kconfig`` is up-to-date.
 
 * If the added functionality requires some cleanup or structural changes, split
   your patch into a cleanup part and the actual addition. This makes it easier
-  to review your changes, and to bisect any resulting problems.
+  to review your changes and to bisect any resulting problems.
 
 * Never mix bug fixes, cleanup, and functional enhancements in a single patch.
 
 
-3. New drivers
---------------
+New drivers
+===========
 
-* Running your patch or driver file(s) through checkpatch does not mean its
+* Running your patch or driver file(s) through ``checkpatch`` does not mean its
   formatting is clean. If unsure about formatting in your new driver, run it
-  through Lindent. Lindent is not perfect, and you may have to do some minor
-  cleanup, but it is a good start.
+  through ``scripts/Lindent``. ``Lindent`` is not perfect, and you may have to
+  do some minor cleanup, but it is a good start.
 
-* Consider adding yourself to MAINTAINERS.
+* Consider adding yourself to ``MAINTAINERS``.
 
-* Document the driver in Documentation/hwmon/<driver_name>.rst.
+* Document the driver in ``Documentation/hwmon/<driver_name>.rst``.
 
-* Add the driver to Kconfig and Makefile in alphabetical order.
+* Add the driver to ``Kconfig`` and ``Makefile`` in alphabetical order.
 
-* Make sure that all dependencies are listed in Kconfig.
+* Make sure that all dependencies are listed in ``Kconfig``.
 
 * Please list include files in alphabetic order.
 
-* Please align continuation lines with '(' on the previous line.
+* Please align continuation lines with '``(``' on the previous line.
 
 * Avoid forward declarations if you can. Rearrange the code if necessary.
 
 * Avoid macros to generate groups of sensor attributes. It not only confuses
-  checkpatch, but also makes it more difficult to review the code.
+  ``checkpatch``, but also makes it more difficult to review the code.
 
 * Avoid calculations in macros and macro-generated functions. While such macros
   may save a line or so in the source, it obfuscates the code and makes code
@@ -88,16 +90,17 @@ increase the chances of your change being accepted.
   generate an error message just because a runtime operation failed. Report
   errors to user space instead, using an appropriate error code. Keep in mind
   that kernel error log messages not only fill up the kernel log, but also are
-  printed synchronously, most likely with interrupt disabled, often to a serial
-  console. Excessive logging can seriously affect system performance.
+  printed synchronously, most likely with interrupts disabled, and often to a
+  serial console. Excessive logging can seriously affect system performance.
 
-* Use devres functions whenever possible to allocate resources. For rationale
-  and supported functions, please see Documentation/driver-api/driver-model/devres.rst.
-  If a function is not supported by devres, consider using devm_add_action().
+* Use ``devres`` functions whenever possible to allocate resources. For
+  rationale and supported functions, please see
+  Documentation/driver-api/driver-model/devres.rst. If a function is not
+  supported by ``devres``, consider using devm_add_action().
 
 * If the driver has a detect function, make sure it is silent. Debug messages
   and messages printed after a successful detection are acceptable, but it
-  must not print messages such as "Chip XXX not found/supported".
+  must not print messages such as ``Chip XXX not found/supported``.
 
   Keep in mind that the detect function will run for all drivers supporting an
   address if a chip is detected on that address. Unnecessary messages will just
@@ -105,14 +108,14 @@ increase the chances of your change being accepted.
 
 * Provide a detect function if and only if a chip can be detected reliably.
 
-* Only the following I2C addresses shall be probed: 0x18-0x1f, 0x28-0x2f,
-  0x48-0x4f, 0x58, 0x5c, 0x73 and 0x77. Probing other addresses is strongly
-  discouraged as it is known to cause trouble with other (non-hwmon) I2C
-  chips. If your chip lives at an address which can't be probed then the
-  device will have to be instantiated explicitly (which is always better
-  anyway.)
+* Only the following I2C addresses shall be probed: ``0x18``-``0x1f``,
+  ``0x28``-``0x2f``, ``0x48``-``0x4f``, ``0x58``, ``0x5c``, ``0x73``,
+  and ``0x77``. Probing other addresses is strongly discouraged, as it is
+  known to cause trouble with other (non-``hwmon``) I2C chips. If your chip
+  lives at an address which can't be probed, then the device will have to be
+  instantiated explicitly (which is always better anyway).
 
-* Avoid writing to chip registers in the detect function. If you have to write,
+* Avoid writing to chip registers in the detect function. If you must,
   only do it after you have already gathered enough data to be certain that the
   detection is going to be successful.
 
@@ -121,25 +124,28 @@ increase the chances of your change being accepted.
 
 * Make sure there are no race conditions in the probe function. Specifically,
   completely initialize your chip and your driver first, then register with
-  the hwmon subsystem.
+  the ``hwmon`` subsystem.
 
-* Use devm_hwmon_device_register_with_info() or, if your driver needs a remove
-  function, hwmon_device_register_with_info() to register your driver with the
-  hwmon subsystem. Try using devm_add_action() instead of a remove function if
-  possible. Do not use any of the deprecated registration functions.
+* Use
+  :ref:`devm_hwmon_device_register_with_info() <devm_hwmon_device_register_with_info>`
+  or, if your driver needs a remove function,
+  :ref:`hwmon_device_register_with_info() <hwmon_device_register_with_info>` to
+  register your driver with the ``hwmon`` subsystem. Try using devm_add_action()
+  instead of a remove function if possible. Do not use any of the deprecated
+  registration functions.
 
-* Your driver should be buildable as module. If not, please be prepared to
+* Your driver should be buildable as a module. If not, please be prepared to
   explain why it has to be built into the kernel.
 
-* Do not provide support for deprecated sysfs attributes.
+* Do not provide support for deprecated ``sysfs`` attributes.
 
 * Do not create non-standard attributes unless really needed. If you have to use
   non-standard attributes, or you believe you do, discuss it on the mailing list
-  first. Either case, provide a detailed explanation why you need the
+  first. In either case, provide a detailed explanation why you need the
   non-standard attribute(s).
   Standard attributes are specified in Documentation/hwmon/sysfs-interface.rst.
 
-* When deciding which sysfs attributes to support, look at the chip's
+* When deciding which ``sysfs`` attributes to support, look at the chip's
   capabilities. While we do not expect your driver to support everything the
   chip may offer, it should at least support all limits and alarms.
 
-- 
2.34.1

[RFC 07/11] Documentation/hwmon: Revise sysfs interface specification

[James Seo]

Reorganize content into coherent sections.
Shorten long attribute descriptions (save them for the ABI).
Mark the section on interpreting attribute writes as outdated.
Fix minor issues (typos, grammar, etc.) and add markup.

Signed-off-by: James Seo <james@equiv.tech>
---
 Documentation/hwmon/sysfs-interface.rst | 984 +++++++++++-------------
 1 file changed, 440 insertions(+), 544 deletions(-)

diff --git a/Documentation/hwmon/sysfs-interface.rst b/Documentation/hwmon/sysfs-interface.rst
index f76e9f8cc1ad..ab93554578ac 100644
--- a/Documentation/hwmon/sysfs-interface.rst
+++ b/Documentation/hwmon/sysfs-interface.rst
@@ -1,15 +1,21 @@
+================================================
 Naming and data format standards for sysfs files
 ================================================
 
-The libsensors library offers an interface to the raw sensors data
-through the sysfs interface. Since lm-sensors 3.0.0, libsensors is
-completely chip-independent. It assumes that all the kernel drivers
-implement the standard sysfs interface described in this document.
+.. contents::
+
+Introduction
+============
+
+The ``libsensors`` library offers an interface to the raw sensors data
+through the ``sysfs`` interface. Since ``lm-sensors`` 3.0.0, ``libsensors``
+is completely chip-independent. It assumes that all the kernel drivers
+implement the standard ``sysfs`` interface described in this document.
 This makes adding or updating support for any given chip very easy, as
-libsensors, and applications using it, do not need to be modified.
-This is a major improvement compared to lm-sensors 2.
+``libsensors``, and applications using it, do not need to be modified.
+This is a major improvement compared to ``lm-sensors`` 2.
 
-Note that motherboards vary widely in the connections to sensor chips.
+Note that motherboards vary widely in their connections to sensor chips.
 There is no standard that ensures, for example, that the second
 temperature sensor is connected to the CPU, or that the second fan is on
 the CPU. Also, some values reported by the chips need some computation
@@ -19,638 +25,528 @@ range using external resistors. Since the values of these resistors
 can change from motherboard to motherboard, the conversions cannot be
 hard coded into the driver and have to be done in user space.
 
-For this reason, even if we aim at a chip-independent libsensors, it will
-still require a configuration file (e.g. /etc/sensors.conf) for proper
+For this reason, even if we aim at a chip-independent ``libsensors``, it will
+still require a configuration file (e.g.``/etc/sensors3.conf``) for proper
 values conversion, labeling of inputs and hiding of unused inputs.
 
-An alternative method that some programs use is to access the sysfs
+An alternative method that some programs use is to access the ``sysfs``
 files directly. This document briefly describes the standards that the
 drivers follow, so that an application program can scan for entries and
 access this data in a simple and consistent way. That said, such programs
 will have to implement conversion, labeling and hiding of inputs. For
 this reason, it is still not recommended to bypass the library.
 
-Each chip gets its own directory in the sysfs /sys/devices tree.  To
+Each chip gets its own directory in the ``sysfs`` ``/sys/devices`` tree.  To
 find all sensor chips, it is easier to follow the device symlinks from
-`/sys/class/hwmon/hwmon*`.
-
-Up to lm-sensors 3.0.0, libsensors looks for hardware monitoring attributes
-in the "physical" device directory. Since lm-sensors 3.0.1, attributes found
-in the hwmon "class" device directory are also supported. Complex drivers
-(e.g. drivers for multifunction chips) may want to use this possibility to
-avoid namespace pollution. The only drawback will be that older versions of
-libsensors won't support the driver in question.
-
-All sysfs values are fixed point numbers.
-
-There is only one value per file, unlike the older /proc specification.
-The common scheme for files naming is: <type><number>_<item>. Usual
-types for sensor chips are "in" (voltage), "temp" (temperature) and
-"fan" (fan). Usual items are "input" (measured value), "max" (high
-threshold, "min" (low threshold). Numbering usually starts from 1,
-except for voltages which start from 0 (because most data sheets use
-this). A number is always used for elements that can be present more
-than once, even if there is a single element of the given type on the
-specific chip. Other files do not refer to a specific element, so
-they have a simple name, and no number.
-
-Alarms are direct indications read from the chips. The drivers do NOT
-make comparisons of readings to thresholds. This allows violations
-between readings to be caught and alarmed. The exact definition of an
-alarm (for example, whether a threshold must be met or must be exceeded
-to cause an alarm) is chip-dependent.
+``/sys/class/hwmon/hwmon*``.
 
-When setting values of hwmon sysfs attributes, the string representation of
-the desired value must be written, note that strings which are not a number
-are interpreted as 0! For more on how written strings are interpreted see the
-"sysfs attribute writes interpretation" section at the end of this file.
+Up to ``lm-sensors`` 3.0.0, ``libsensors`` looks for hardware monitoring
+attributes in the ``physical`` device directory. Since ``lm-sensors`` 3.0.1,
+attributes found in the ``hwmon`` ``class`` device directory are also
+supported. Complex drivers (e.g. drivers for multifunction chips) may want to
+use this possibility to avoid namespace pollution. The only drawback will be
+that older versions of ``libsensors`` won't support the driver in question.
 
-Attribute access
-----------------
+Interface specification
+=======================
 
-Hardware monitoring sysfs attributes are displayed by unrestricted userspace
-applications. For this reason, all standard ABI attributes shall be world
-readable. Writeable standard ABI attributes shall be writeable only for
-privileged users.
+All ``sysfs`` values are fixed point numbers. There is only one value per
+file, unlike the older ``/proc`` specification.
 
--------------------------------------------------------------------------
+Names
+-----
 
-======= ===========================================
-`[0-*]`	denotes any positive number starting from 0
-`[1-*]`	denotes any positive number starting from 1
-RO	read only value
-WO	write only value
-RW	read/write value
-======= ===========================================
+The common scheme for file naming is: ``<type><number>_<item>``.
 
-Read/write values may be read-only for some chips, depending on the
-hardware implementation.
-
-All entries (except name) are optional, and should only be created in a
-given driver if the chip has the feature.
+Usual types for sensor chips are ``in`` (voltage), ``temp`` (temperature), and
+``fan`` (fan).
 
-See Documentation/ABI/testing/sysfs-class-hwmon for a complete description
-of the attributes.
+Usual items are ``input`` (measured value), ``max`` (high threshold),  and
+``min`` (low threshold).
 
-*****************
-Global attributes
-*****************
+Numbering usually starts from ``1``, except for voltages, which start from
+``0`` (because most data sheets use this). A number is always used for
+elements that can be present more than once, even if there is a single
+element of the given type on the specific chip. Other files do not refer
+to a specific element, so they have a simple name, and no number.
 
-`name`
-		The chip name.
+Access modes
+------------
 
-`label`
-		A descriptive label that allows to uniquely identify a device
-		within the system.
-
-`update_interval`
-		The interval at which the chip will update readings.
+Hardware monitoring ``sysfs`` attributes are displayed by unrestricted
+userspace applications. For this reason, all standard ABI attributes shall be
+world readable. Writeable standard ABI attributes shall be writeable only for
+privileged users.
 
+Setting attributes
+------------------
 
-********
-Voltages
-********
+When setting values of ``hwmon`` ``sysfs`` attributes, the string
+representation of the desired value must be written.
 
-`in[0-*]_min`
-		Voltage min value.
+Interpreting writes to attributes
+---------------------------------
 
-`in[0-*]_lcrit`
-		Voltage critical min value.
+.. warning ::
+  This section is outdated and in need of attention.  Please use
+  this information with caution, and please consider sending patches
+  to update it.
 
-`in[0-*]_max`
-		Voltage max value.
+Note that strings which are not a number are interpreted as ``0``!
 
-`in[0-*]_crit`
-		Voltage critical max value.
+``hwmon`` ``sysfs`` attributes always contain numbers, so the first thing to
+do is to convert the input to a number. There are 2 ways to do this depending
+on whether the number can be negative or not:
 
-`in[0-*]_input`
-		Voltage input value.
+.. code-block:: c
 
-`in[0-*]_average`
-		Average voltage
+  unsigned long u = simple_strtoul(buf, NULL, 10);
+  long s = simple_strtol(buf, NULL, 10);
 
-`in[0-*]_lowest`
-		Historical minimum voltage
+with **buf** being the buffer with the user input being passed by the kernel.
+Notice that we do not use the second argument of strto[u]l, and thus cannot
+tell when ``0`` is returned, if this was really ``0`` or is caused by invalid
+input. This is done deliberately as checking this everywhere would add a lot
+of code to the kernel.
 
-`in[0-*]_highest`
-		Historical maximum voltage
+Notice that it is important to always store the converted value in an
+unsigned long or long, so that no wrap around can happen before any further
+checking.
 
-`in[0-*]_reset_history`
-		Reset inX_lowest and inX_highest
+After the input string is converted to an (unsigned) long, the value should be
+checked if its acceptable. Be careful with further conversions on the value
+before checking it for validity, as these conversions could still cause a wrap
+around before the check. For example do not multiply the result, and only
+add/subtract if it has been divided before the add/subtract.
 
-`in_reset_history`
-		Reset inX_lowest and inX_highest for all sensors
+What to do if a value is found to be invalid depends on the type of the
+sysfs attribute that is being set. If it is a continuous setting like a
+``tempX_max`` or ``inX_max`` attribute, then the value should be clamped to
+its limits using clamp_val(value, min_limit, max_limit). If it is not
+continuous, like for example a ``tempX_type``, then when an invalid value is
+written, ``-EINVAL`` should be returned.
 
-`in[0-*]_label`
-		Suggested voltage channel label.
+Example1, ``temp1_max``, register is a signed 8-bit value (-128 - 127 degrees):
 
-`in[0-*]_enable`
-		Enable or disable the sensors.
+.. code-block:: c
 
-`cpu[0-*]_vid`
-		CPU core reference voltage.
+  long v = simple_strtol(buf, NULL, 10) / 1000;
+  v = clamp_val(v, -128, 127);
+  /* write v to register */
 
-`vrm`
-		Voltage Regulator Module version number.
+Example2, fan divider setting, valid values ``2``, ``4`` and ``8``:
 
-`in[0-*]_rated_min`
-		Minimum rated voltage.
+.. code-block:: c
 
-`in[0-*]_rated_max`
-		Maximum rated voltage.
+  unsigned long v = simple_strtoul(buf, NULL, 10);
 
-Also see the Alarms section for status flags associated with voltages.
+  switch (v) {
+  case 2: v = 1; break;
+  case 4: v = 2; break;
+  case 8: v = 3; break;
+  default:
+          return -EINVAL;
+  }
+  /* write v to register */
 
 
-****
-Fans
-****
+Standard attributes
+===================
 
-`fan[1-*]_min`
-		Fan minimum value
+See ``Documentation/ABI/testing/sysfs-class-hwmon`` for a complete description
+of standard attributes, including units, allowed values, error codes, and
+required behaviors.
 
-`fan[1-*]_max`
-		Fan maximum value
+========= ================================================
+Text      Meaning
+========= ================================================
+``[0-*]`` Denotes any positive number starting from ``0``.
+``[1-*]`` Denotes any positive number starting from ``1``.
+**RO**    Read-only value.
+**WO**    Write-only value.
+**RW**    Read/write value.
 
-`fan[1-*]_input`
-		Fan input value.
+          May be read-only for some chips, depending on
+	  the hardware implementation.
+========= ================================================
 
-`fan[1-*]_div`
-		Fan divisor.
+All entries (except ``name``) are optional, and should only be created in a
+given driver if the chip has the feature.
 
-`fan[1-*]_pulses`
-		Number of tachometer pulses per fan revolution.
+Attributes by type
+------------------
 
-`fan[1-*]_target`
-		Desired fan speed
+Global
+~~~~~~
 
-`fan[1-*]_label`
-		Suggested fan channel label.
+=================== ====== =============================
+Name                Perms  Description
+=================== ====== =============================
+``name``            **RO** Chip name.
+``label``           **RO** Chip label.
+``update_interval`` **RW** Chip reading update interval.
+=================== ====== =============================
 
-`fan[1-*]_enable`
-		Enable or disable the sensors.
+Also see `Alarms`_ and `Averages`_ for additional attributes.
 
-Also see the Alarms section for status flags associated with fans.
+Voltages
+~~~~~~~~
+
+========================= ====== =========================================
+Name                      Perms  Description
+========================= ====== =========================================
+``in[0-*]_min``           **RW** Voltage min value.
+``in[0-*]_lcrit``         **RW** Voltage critical min value.
+``in[0-*]_max``           **RW** Voltage max value.
+``in[0-*]_crit``          **RW** Voltage critical max value.
+``in[0-*]_input``         **RO** Voltage input value.
+``in[0-*]_average``       **RO** Average voltage.
+``in[0-*]_lowest``        **RO** Historical minimum voltage.
+``in[0-*]_highest``       **RO** Historical maximum voltage.
+``in[0-*]_reset_history`` **WO** Reset ``in[0-*]_lowest`` and
+                                 ``in[0-*]_highest``.
+``in_reset_history``      **WO** Reset ``lowest`` and ``highest``
+                                 for all ``in[0-*]`` sensors.
+``in[0-*]_label``         **RO** Suggested voltage channel label.
+``in[0-*]_enable``        **RW** Enable or disable the sensor.
+``cpu[0-*]_vid``          **RO** CPU core reference voltage.
+``vrm``                   **RW** Voltage Regulator Module version number.
+``in[0-*]_rated_min``     **RO** Minimum rated voltage.
+``in[0-*]_rated_max``     **RO** Maximum rated voltage.
+========================= ====== =========================================
+
+Also see `Alarms`_ and `Averages`_ for status flags and additional attributes
+associated with voltages.
 
+Fans
+~~~~
+
+=================== ====== ===============================================
+Name                Perms  Description
+=================== ====== ===============================================
+``fan[1-*]_min``    **RW** Fan minimum value.
+``fan[1-*]_max``    **RW** Fan maximum value.
+``fan[1-*]_input``  **RO** Fan input value.
+``fan[1-*]_div``    **RW** Fan divisor.
+``fan[1-*]_pulses`` **RW** Number of tachometer pulses per fan revolution.
+``fan[1-*]_target`` **RW** Desired fan speed.
+``fan[1-*]_label``  **RO** Suggested fan channel label.
+``fan[1-*]_enable`` **RW** Enable or disable the sensor.
+=================== ====== ===============================================
+
+Also see `Alarms and faults`_ for status flags and additional attributes
+associated with fans.
 
-***
 PWM
-***
-
-`pwm[1-*]`
-		Pulse width modulation fan control.
-
-`pwm[1-*]_enable`
-		Fan speed control method.
-
-`pwm[1-*]_mode`
-		direct current or pulse-width modulation.
-
-`pwm[1-*]_freq`
-		Base PWM frequency in Hz.
-
-`pwm[1-*]_auto_channels_temp`
-		Select which temperature channels affect this PWM output in
-		auto mode.
-
-`pwm[1-*]_auto_point[1-*]_pwm` / `pwm[1-*]_auto_point[1-*]_temp` / `pwm[1-*]_auto_point[1-*]_temp_hyst`
-		Define the PWM vs temperature curve.
-
-`temp[1-*]_auto_point[1-*]_pwm` / `temp[1-*]_auto_point[1-*]_temp` / `temp[1-*]_auto_point[1-*]_temp_hyst`
-		Define the PWM vs temperature curve.
-
-There is a third case where trip points are associated to both PWM output
-channels and temperature channels: the PWM values are associated to PWM
-output channels while the temperature values are associated to temperature
+~~~
+
++-------------------------------------------+--------+-------------------------------------+
+| Name                                      | Perms  | Description                         |
++===========================================+========+=====================================+
+| ``pwm[1-*]``                              | **RW** | Pulse-width modulation fan control. |
++-------------------------------------------+--------+-------------------------------------+
+| ``pwm[1-*]_enable``                       | **RW** | Fan speed control method.           |
++-------------------------------------------+--------+-------------------------------------+
+| ``pwm[1-*]_mode``                         | **RW** | Fan speed control mode.             |
++-------------------------------------------+--------+-------------------------------------+
+| ``pwm[1-*]_freq``                         | **RW** | Base PWM frequency in Hz.           |
++-------------------------------------------+--------+-------------------------------------+
+| ``pwm[1-*]_auto_channels_temp``           | **RW** | Auto mode channel mask.             |
++-------------------------------------------+--------+-------------------------------------+
+| | ``pwm[1-*]_auto_point[1-*]_pwm``        | **RW** | PWM temperature curve definition    |
+| | ``pwm[1-*]_auto_point[1-*]_temp``       |        | for chips that associate trip       |
+| | ``pwm[1-*]_auto_point[1-*]_temp_hyst``  |        | points with PWM output channels.    |
++-------------------------------------------+--------+-------------------------------------+
+| | ``temp[1-*]_auto_point[1-*]_pwm``       | **RW** | PWM temperature curve definition    |
+| | ``temp[1-*]_auto_point[1-*]_temp``      |        | for chips that associate trip       |
+| | ``temp[1-*]_auto_point[1-*]_temp_hyst`` |        | points with temperature channels.   |
++-------------------------------------------+--------+-------------------------------------+
+
+Please see ``Documentation/ABI/testing/sysfs-class-hwmon`` for more
+information on PWM vs. temperature curves.
+
+There is a third case where trip points are associated with both PWM output
+channels and temperature channels: the PWM values are associated with PWM
+output channels while the temperature values are associated with temperature
 channels. In that case, the result is determined by the mapping between
 temperature inputs and PWM outputs. When several temperature inputs are
 mapped to a given PWM output, this leads to several candidate PWM values.
 The actual result is up to the chip, but in general the highest candidate
 value (fastest fan speed) wins.
 
-
-************
 Temperatures
-************
-
-`temp[1-*]_type`
-		Sensor type selection.
-
-`temp[1-*]_max`
-		Temperature max value.
-
-`temp[1-*]_min`
-		Temperature min value.
-
-`temp[1-*]_max_hyst`
-		Temperature hysteresis value for max limit.
-
-`temp[1-*]_min_hyst`
-		Temperature hysteresis value for min limit.
-
-`temp[1-*]_input`
-		Temperature input value.
-
-`temp[1-*]_crit`
-		Temperature critical max value, typically greater than
-		corresponding temp_max values.
-
-`temp[1-*]_crit_hyst`
-		Temperature hysteresis value for critical limit.
-
-`temp[1-*]_emergency`
-		Temperature emergency max value, for chips supporting more than
-		two upper temperature limits.
-
-`temp[1-*]_emergency_hyst`
-		Temperature hysteresis value for emergency limit.
-
-`temp[1-*]_lcrit`
-		Temperature critical min value, typically lower than
-		corresponding temp_min values.
-
-`temp[1-*]_lcrit_hyst`
-		Temperature hysteresis value for critical min limit.
-
-`temp[1-*]_offset`
-		Temperature offset which is added to the temperature reading
-		by the chip.
-
-`temp[1-*]_label`
-		Suggested temperature channel label.
-
-`temp[1-*]_lowest`
-		Historical minimum temperature
-
-`temp[1-*]_highest`
-		Historical maximum temperature
-
-`temp[1-*]_reset_history`
-		Reset temp_lowest and temp_highest
-
-`temp_reset_history`
-		Reset temp_lowest and temp_highest for all sensors
-
-`temp[1-*]_enable`
-		Enable or disable the sensors.
-
-`temp[1-*]_rated_min`
-		Minimum rated temperature.
-
-`temp[1-*]_rated_max`
-		Maximum rated temperature.
+~~~~~~~~~~~~
+
+============================ ====== ===========================================
+Name                         Perms  Description
+============================ ====== ===========================================
+``temp[1-*]_type``           **RW** Sensor type selection.
+``temp[1-*]_max``            **RW** Temperature max value.
+``temp[1-*]_min``            **RW** Temperature min value.
+``temp[1-*]_max_hyst``       **RW** Temperature hysteresis value for max limit.
+``temp[1-*]_min_hyst``       **RW** Temperature hysteresis value for min limit.
+``temp[1-*]_input``          **RO** Temperature input value.
+``temp[1-*]_crit``           **RW** Temperature critical max value.
+``temp[1-*]_crit_hyst``      **RW** Temperature hysteresis value
+                                    for critical limit.
+``temp[1-*]_emergency``      **RW** Temperature emergency max value.
+``temp[1-*]_emergency_hyst`` **RW** Temperature hysteresis value
+                                    for emergency limit.
+``temp[1-*]_lcrit``          **RW** Temperature critical min value.
+``temp[1-*]_lcrit_hyst``     **RW** Temperature hysteresis value
+                                    for critical min limit.
+``temp[1-*]_offset``         **RW** Temperature offset.
+``temp[1-*]_label``          **RO** Suggested temperature channel label.
+``temp[1-*]_lowest``         **RO** Historical minimum temperature.
+``temp[1-*]_highest``        **RO** Historical maximum temperature.
+``temp[1-*]_reset_history``  **WO** Reset ``temp[1-*]_lowest`` and
+                                    ``temp[1-*]_highest``.
+``temp_reset_history``       **WO** Reset ``lowest`` and ``highest``
+                                    for all ``temp[1-*]`` sensors.
+``temp[1-*]_enable``         **RW** Enable or disable the sensor.
+``temp[1-*]_rated_min``      **RO** Minimum rated temperature.
+``temp[1-*]_rated_max``      **RO** Maximum rated temperature.
+============================ ====== ===========================================
 
 Some chips measure temperature using external thermistors and an ADC, and
 report the temperature measurement as a voltage. Converting this voltage
 back to a temperature (or the other way around for limits) requires
 mathematical functions not available in the kernel, so the conversion
-must occur in user space. For these chips, all temp* files described
+must occur in user space. For these chips, all ``temp*`` files described
 above should contain values expressed in millivolt instead of millidegree
 Celsius. In other words, such temperature channels are handled as voltage
 channels by the driver.
 
-Also see the Alarms section for status flags associated with temperatures.
+Also see `Alarms and faults`_ and `Averages`_ for status flags and additional
+attributes associated with temperatures.
 
-
-********
 Currents
-********
-
-`curr[1-*]_max`
-		Current max value.
-
-`curr[1-*]_min`
-		Current min value.
-
-`curr[1-*]_lcrit`
-		Current critical low value
-
-`curr[1-*]_crit`
-		Current critical high value.
-
-`curr[1-*]_input`
-		Current input value.
-
-`curr[1-*]_average`
-		Average current use.
-
-`curr[1-*]_lowest`
-		Historical minimum current.
-
-`curr[1-*]_highest`
-		Historical maximum current.
+~~~~~~~~
+
+=========================== ====== ================================
+Name                        Perms  Description
+=========================== ====== ================================
+``curr[1-*]_max``           **RW** Current max value.
+``curr[1-*]_min``           **RW** Current min value.
+``curr[1-*]_lcrit``         **RW** Current critical min value.
+``curr[1-*]_crit``          **RW** Current critical max value.
+``curr[1-*]_input``         **RO** Current input value.
+``curr[1-*]_average``       **RO** Average current use.
+``curr[1-*]_lowest``        **RO** Historical minimum current.
+``curr[1-*]_highest``       **RO** Historical maximum current.
+``curr[1-*]_reset_history`` **WO** Reset ``curr[1-*]_lowest`` and
+                                   ``curr[1-*]_highest``.
+``curr_reset_history``      **WO** Reset ``lowest`` and ``highest``
+                                   for all ``curr[1-*]`` sensors.
+``curr[1-*]_enable``        **RW** Enable or disable the sensor.
+``curr[1-*]_rated_min``     **RO** Minimum rated current.
+``curr[1-*]_rated_max``     **RO** Maximum rated current.
+=========================== ====== ================================
+
+Also see `Alarms`_ and `Averages`_ for status flags and additional attributes
+associated with currents.
 
-`curr[1-*]_reset_history`
-		Reset currX_lowest and currX_highest
-
-		WO
-
-`curr_reset_history`
-		Reset currX_lowest and currX_highest for all sensors.
-
-`curr[1-*]_enable`
-		Enable or disable the sensors.
-
-`curr[1-*]_rated_min`
-		Minimum rated current.
-
-`curr[1-*]_rated_max`
-		Maximum rated current.
-
-Also see the Alarms section for status flags associated with currents.
-
-*****
 Power
-*****
-
-`power[1-*]_average`
-		Average power use.
-
-`power[1-*]_average_interval`
-		Power use averaging interval.
+~~~~~
+
++-------------------------------------+--------+---------------------------------------+
+| Name                                | Perms  | Description                           |
++=====================================+========+=======================================+
+| ``power[1-*]_average``              | **RO** | Average power use.                    |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_average_interval``     | **RW** | Power use averaging interval.         |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_average_interval_max`` | **RO** | Maximum power use averaging interval. |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_average_interval_min`` | **RO** | Minimum power use averaging interval. |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_average_highest``      | **RO** | Historical average maximum power use. |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_average_lowest``       | **RO** | Historical average minimum power use. |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_average_max``          | **RW** | Maximum average power                 |
+|                                     |        | notification threshold.               |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_average_min``          | **RW** | Minimum average power                 |
+|                                     |        | notification threshold.               |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_input``                | **RO** | Instantaneous power use.              |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_input_highest``        | **RO** | Historical maximum power use.         |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_input_lowest``         | **RO** | Historical minimum power use.         |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_reset_history``        | **WO** | | Reset ``power[1-*]_input_highest``, |
+|                                     |        | | ``power[1-*]_input_lowest``,        |
+|                                     |        | | ``power[1-*]_average_highest``, and |
+|                                     |        | | ``power[1-*]_average_lowest``.      |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_accuracy``             | **RO** | Accuracy of the power meter.          |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_cap``                  | **RW** | Power reduction threshold.            |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_cap_hyst``             | **RW** | Threshold hysteresis margin.          |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_cap_max``              | **RO** | Maximum cap that can be set.          |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_cap_min``              | **RO** | Minimum cap that can be set.          |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_max``                  | **RW** | Maximum power.                        |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_crit``                 | **RW** | Critical power reduction threshold.   |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_enable``               | **RW** | Enable or disable the sensor.         |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_rated_min``            | **RO** | Minimum rated power.                  |
++-------------------------------------+--------+---------------------------------------+
+| ``power[1-*]_rated_max``            | **RO** | Maximum rated power.                  |
++-------------------------------------+--------+---------------------------------------+
+
+Please see ``Documentation/ABI/testing/sysfs-class-hwmon`` for more
+information on thresholds and notifications/actions.
+
+Also see `Alarms`_ and `Averages`_ for status flags and additional attributes
+associated with power readings.
 
-`power[1-*]_average_interval_max`
-		Maximum power use averaging interval.
-
-`power[1-*]_average_interval_min`
-		Minimum power use averaging interval.
-
-`power[1-*]_average_highest`
-		Historical average maximum power use
-
-`power[1-*]_average_lowest`
-		Historical average minimum power use
-
-`power[1-*]_average_max`
-		A poll notification is sent to `power[1-*]_average` when
-		power use rises above this value.
-
-`power[1-*]_average_min`
-		A poll notification is sent to `power[1-*]_average` when
-		power use sinks below this value.
-
-`power[1-*]_input`
-		Instantaneous power use.
-
-`power[1-*]_input_highest`
-		Historical maximum power use
-
-`power[1-*]_input_lowest`
-		Historical minimum power use.
-
-`power[1-*]_reset_history`
-		Reset input_highest, input_lowest, average_highest and
-		average_lowest.
-
-`power[1-*]_accuracy`
-		Accuracy of the power meter.
-
-`power[1-*]_cap`
-		If power use rises above this limit, the
-		system should take action to reduce power use.
-
-`power[1-*]_cap_hyst`
-		Margin of hysteresis built around capping and notification.
-
-`power[1-*]_cap_max`
-		Maximum cap that can be set.
-
-`power[1-*]_cap_min`
-		Minimum cap that can be set.
-
-`power[1-*]_max`
-		Maximum power.
-
-`power[1-*]_crit`
-				Critical maximum power.
-
-				If power rises to or above this limit, the
-				system is expected take drastic action to reduce
-				power consumption, such as a system shutdown or
-				a forced powerdown of some devices.
-
-				Unit: microWatt
-
-				RW
-
-`power[1-*]_enable`
-				Enable or disable the sensors.
-
-				When disabled the sensor read will return
-				-ENODATA.
-
-				- 1: Enable
-				- 0: Disable
-
-				RW
-
-`power[1-*]_rated_min`
-				Minimum rated power.
-
-				Unit: microWatt
-
-				RO
-
-`power[1-*]_rated_max`
-				Maximum rated power.
-
-				Unit: microWatt
-
-				RO
-
-Also see the Alarms section for status flags associated with power readings.
-
-******
 Energy
-******
-
-`energy[1-*]_input`
-				Cumulative energy use
-
-				Unit: microJoule
-
-				RO
-
-`energy[1-*]_enable`
-				Enable or disable the sensors.
-
-				When disabled the sensor read will return
-				-ENODATA.
-
-				- 1: Enable
-				- 0: Disable
+~~~~~~
 
-				RW
+====================== ====== =============================
+Name                   Perms  Description
+====================== ====== =============================
+``energy[1-*]_input``  **RO** Cumulative energy use.
+``energy[1-*]_enable`` **RW** Enable or disable the sensor.
+====================== ====== =============================
 
-********
 Humidity
-********
+~~~~~~~~
 
-`humidity[1-*]_input`
-		Humidity.
+=========================== ====== =============================
+Name                        Perms  Description
+=========================== ====== =============================
+``humidity[1-*]_input``     **RO** Humidity.
+``humidity[1-*]_enable``    **RW** Enable or disable the sensor.
+``humidity[1-*]_rated_min`` **RO** Minimum rated humidity.
+``humidity[1-*]_rated_max`` **RW** Maximum rated humidity.
+=========================== ====== =============================
 
-`humidity[1-*]_enable`
-		Enable or disable the sensors.
+Intrusion
+~~~~~~~~~
 
-`humidity[1-*]_rated_min`
-		Minimum rated humidity.
+See `Alarms`_ for more information.
 
-`humidity[1-*]_rated_max`
-		Maximum rated humidity.
+Alarms and faults
+-----------------
 
-******
 Alarms
-******
+~~~~~~
 
-Each channel or limit may have an associated alarm file, containing a
-boolean value. 1 means than an alarm condition exists, 0 means no alarm.
+Each channel or limit may have an associated ``alarm`` attribute, containing
+a boolean value. ``1`` means that an alarm condition exists, ``0`` means no
+alarm.
 
-Usually a given chip will either use channel-related alarms, or
-limit-related alarms, not both. The driver should just reflect the hardware
-implementation.
-
-+-------------------------------+-----------------------+
-| **`in[0-*]_alarm`,		| Channel alarm		|
-| `curr[1-*]_alarm`,		|			|
-| `power[1-*]_alarm`,		|   - 0: no alarm	|
-| `fan[1-*]_alarm`,		|   - 1: alarm		|
-| `temp[1-*]_alarm`**		|			|
-|				|   RO			|
-+-------------------------------+-----------------------+
-
-**OR**
-
-+-------------------------------+-----------------------+
-| **`in[0-*]_min_alarm`,	| Limit alarm		|
-| `in[0-*]_max_alarm`,		|			|
-| `in[0-*]_lcrit_alarm`,	|   - 0: no alarm	|
-| `in[0-*]_crit_alarm`,		|   - 1: alarm		|
-| `curr[1-*]_min_alarm`,	|			|
-| `curr[1-*]_max_alarm`,	| RO			|
-| `curr[1-*]_lcrit_alarm`,	|			|
-| `curr[1-*]_crit_alarm`,	|			|
-| `power[1-*]_cap_alarm`,	|			|
-| `power[1-*]_max_alarm`,	|			|
-| `power[1-*]_crit_alarm`,	|			|
-| `fan[1-*]_min_alarm`,		|			|
-| `fan[1-*]_max_alarm`,		|			|
-| `temp[1-*]_min_alarm`,	|			|
-| `temp[1-*]_max_alarm`,	|			|
-| `temp[1-*]_lcrit_alarm`,	|			|
-| `temp[1-*]_crit_alarm`,	|			|
-| `temp[1-*]_emergency_alarm`**	|			|
-+-------------------------------+-----------------------+
-
-Each input channel may have an associated fault file. This can be used
-to notify open diodes, unconnected fans etc. where the hardware
-supports it. When this boolean has value 1, the measurement for that
-channel should not be trusted.
+Alarms are direct indications read from the chips. The drivers **do not**
+make comparisons of readings to thresholds. This allows violations
+between readings to be caught and alarmed. The exact definition of an
+alarm (for example, whether a threshold must be met or must be exceeded
+to cause an alarm) is chip-dependent.
 
-`fan[1-*]_fault` / `temp[1-*]_fault`
-		Input fault condition.
++---------------------------------+--------+------------------------------+
+| Name                            | Perms  | Description                  |
++=================================+========+==============================+
+| | ``in[0-*]_alarm``             | **RO** | Channel alarm indicator.     |
+| | ``curr[1-*]_alarm``           |        |                              |
+| | ``power[1-*]_alarm``          |        |                              |
+| | ``fan[1-*]_alarm``            |        |                              |
+| | ``temp[1-*]_alarm``           |        |                              |
++---------------------------------+        +------------------------------+
+| | ``in[0-*]_min_alarm``         |        | Limit alarm indicator.       |
+| | ``in[0-*]_max_alarm``         |        |                              |
+| | ``in[0-*]_lcrit_alarm``       |        |                              |
+| | ``in[0-*]_crit_alarm``        |        |                              |
+| | ``curr[1-*]_min_alarm``       |        |                              |
+| | ``curr[1-*]_max_alarm``       |        |                              |
+| | ``curr[1-*]_lcrit_alarm``     |        |                              |
+| | ``curr[1-*]_crit_alarm``      |        |                              |
+| | ``power[1-*]_cap_alarm``      |        |                              |
+| | ``power[1-*]_max_alarm``      |        |                              |
+| | ``power[1-*]_crit_alarm``     |        |                              |
+| | ``fan[1-*]_min_alarm``        |        |                              |
+| | ``fan[1-*]_max_alarm``        |        |                              |
+| | ``temp[1-*]_min_alarm``       |        |                              |
+| | ``temp[1-*]_max_alarm``       |        |                              |
+| | ``temp[1-*]_lcrit_alarm``     |        |                              |
+| | ``temp[1-*]_emergency_alarm`` |        |                              |
++---------------------------------+--------+                              |
+| ``temp[1-*]_crit_alarm``        | **RW** |                              |
++---------------------------------+        +------------------------------+
+| ``intrusion[1-*]_alarm``        |        | Chassis intrusion indicator. |
++---------------------------------+--------+------------------------------+
+
+Usually a given chip will either use channel-related alarms or limit-related
+alarms, but not both. The driver should just reflect the hardware
+implementation.
 
 Some chips also offer the possibility to get beeped when an alarm occurs:
 
-`beep_enable`
-		Master beep enable.
-
-`in[0-*]_beep`, `curr[1-*]_beep`, `fan[1-*]_beep`, `temp[1-*]_beep`,
-		Channel beep.
++-------------------------+--------+--------------------------+
+| Name                    | Perms  | Description              |
++=========================+========+==========================+
+| ``beep_enable``         | **RW** | Enable or disable beeps. |
++-------------------------+        +--------------------------+
+| | ``in[0-*]_beep``      |        | Channel beep.            |
+| | ``curr[1-*]_beep``    |        |                          |
+| | ``fan[1-*]_beep``     |        |                          |
+| | ``temp[1-*]_beep``    |        |                          |
++-------------------------+        +--------------------------+
+| ``intrusion[1-*]_beep`` |        | Chassis intrusion beep.  |
++-------------------------+--------+--------------------------+
 
 In theory, a chip could provide per-limit beep masking, but no such chip
-was seen so far.
+has been seen so far.
 
 Old drivers provided a different, non-standard interface to alarms and
-beeps. These interface files are deprecated, but will be kept around
+beeps. These interface attributes are deprecated, but will be kept around
 for compatibility reasons:
 
-`alarms`
-		Alarm bitmask.
-
-`beep_mask`
-		Bitmask for beep.
-
-
-*******************
-Intrusion detection
-*******************
-
-`intrusion[0-*]_alarm`
-		Chassis intrusion detection.
-
-`intrusion[0-*]_beep`
-		Chassis intrusion beep.
-
-****************************
-Average sample configuration
-****************************
-
-Devices allowing for reading {in,power,curr,temp}_average values may export
-attributes for controlling number of samples used to compute average.
-
-+--------------+---------------------------------------------------------------+
-| samples      | Sets number of average samples for all types of measurements. |
-|	       |							       |
-|	       | RW							       |
-+--------------+---------------------------------------------------------------+
-| in_samples   | Sets number of average samples for specific type of	       |
-| power_samples| measurements.						       |
-| curr_samples |							       |
-| temp_samples | Note that on some devices it won't be possible to set all of  |
-|	       | them to different values so changing one might also change    |
-|	       | some others.						       |
-|	       |							       |
-|	       | RW							       |
-+--------------+---------------------------------------------------------------+
-
-sysfs attribute writes interpretation
--------------------------------------
-
-hwmon sysfs attributes always contain numbers, so the first thing to do is to
-convert the input to a number, there are 2 ways todo this depending whether
-the number can be negative or not::
+============= ====== =================
+Name          Perms  Description
+============= ====== =================
+``alarms``    **RW** Alarm bitmask.
+``beep_mask`` **RW** Bitmask for beep.
+============= ====== =================
 
-	unsigned long u = simple_strtoul(buf, NULL, 10);
-	long s = simple_strtol(buf, NULL, 10);
+Faults
+~~~~~~
 
-With buf being the buffer with the user input being passed by the kernel.
-Notice that we do not use the second argument of strto[u]l, and thus cannot
-tell when 0 is returned, if this was really 0 or is caused by invalid input.
-This is done deliberately as checking this everywhere would add a lot of
-code to the kernel.
-
-Notice that it is important to always store the converted value in an
-unsigned long or long, so that no wrap around can happen before any further
-checking.
-
-After the input string is converted to an (unsigned) long, the value should be
-checked if its acceptable. Be careful with further conversions on the value
-before checking it for validity, as these conversions could still cause a wrap
-around before the check. For example do not multiply the result, and only
-add/subtract if it has been divided before the add/subtract.
-
-What to do if a value is found to be invalid, depends on the type of the
-sysfs attribute that is being set. If it is a continuous setting like a
-tempX_max or inX_max attribute, then the value should be clamped to its
-limits using clamp_val(value, min_limit, max_limit). If it is not continuous
-like for example a tempX_type, then when an invalid value is written,
--EINVAL should be returned.
-
-Example1, temp1_max, register is a signed 8 bit value (-128 - 127 degrees)::
-
-	long v = simple_strtol(buf, NULL, 10) / 1000;
-	v = clamp_val(v, -128, 127);
-	/* write v to register */
-
-Example2, fan divider setting, valid values 2, 4 and 8::
-
-	unsigned long v = simple_strtoul(buf, NULL, 10);
+Each input channel may have an associated ``fault`` attribute. This can be
+used to notify open diodes, unconnected fans etc. where the hardware
+supports it. When this boolean has value ``1``, the measurement for that
+channel should not be trusted.
 
-	switch (v) {
-	case 2: v = 1; break;
-	case 4: v = 2; break;
-	case 8: v = 3; break;
-	default:
-		return -EINVAL;
-	}
-	/* write v to register */
++-----------------------+--------+--------------------------+
+| Name                  | Perms  | Description              |
++=======================+========+==========================+
+| | ``fan[1-*]_fault``  | **RO** | Channel fault indicator. |
+| | ``temp[1-*]_fault`` |        |                          |
++-----------------------+--------+--------------------------+
+
+Averages
+--------
+
+Devices allowing for reading ``average`` values may export attributes for
+controlling the number of samples used to compute the average.
+
++---------------------+--------+-------------------------------------------+
+| Name                | Perms  | Description                               |
++=====================+========+===========================================+
+| ``samples``         | **RW** | Samples in calculated average.            |
++---------------------+        +-------------------------------------------+
+| | ``in_samples``    |        | Samples in calculated average             |
+| | ``power_samples`` |        | for a specific sensor type.               |
+| | ``curr_samples``  |        |                                           |
+| | ``temp_samples``  |        | Note that on some devices, it won't be    |
+|                     |        | possible to set all of these to different |
+|                     |        | values, as changing one might also change |
+|                     |        | others.                                   |
++---------------------+--------+-------------------------------------------+
-- 
2.34.1

[RFC 08/11] Documentation/hwmon: Revise userspace tools documentation

[James Seo]

Rewrite lm-sensors installation and usage guidelines
and mark the list of third-party utilities as outdated.
Fix minor issues (typos, grammar, etc.) and add markup.

Signed-off-by: James Seo <james@equiv.tech>
---
 Documentation/hwmon/userspace-tools.rst | 129 +++++++++++++++++++-----
 1 file changed, 103 insertions(+), 26 deletions(-)

diff --git a/Documentation/hwmon/userspace-tools.rst b/Documentation/hwmon/userspace-tools.rst
index bf3797c8e734..79c932954e4a 100644
--- a/Documentation/hwmon/userspace-tools.rst
+++ b/Documentation/hwmon/userspace-tools.rst
@@ -1,43 +1,120 @@
+===============
 Userspace tools
 ===============
 
+.. contents::
+
 Introduction
+============
+
+Hardware monitoring information is pertinent to almost every Linux system.
+For instance, most computer mainboards manufactured since the late 1990s have
+reported system health data such as temperatures, voltages, and fan speeds.
+All modern CPUs, GPUs, and SoCs have built-in internal temperature sensors and
+often have other types of sensors as well. In addition, an increasing number
+of components, such as power supplies and liquid CPU coolers, are now embedded
+systems ("smart devices") capable of measuring and providing relevant sensor
+data themselves.
+
+The Linux Hardware Monitoring subsystem (``hwmon``) supports most common types
+of hardware monitoring sensors, regardless of how they are connected to the
+system. Sensor measurements are made available in the ``/sys`` virtual
+filesystem at ``/sys/class/hwmon``. Userspace tools are then used to display
+the measured values or configure the sensors and data sources in a more
+friendly manner.
+
+``lm-sensors``
+==============
+
+``lm-sensors`` is a collection of command-line utilities that allows you to
+view hardware monitoring information and set monitoring limits. On some
+systems, it is used to process raw sensor data into a usable form. It may also
+be used to probe the system at a low level to help detect available sensors.
+
+While active development has largely ceased as of 2023, its status is closer
+to "finished" than "abandoned". It has strong historical ties to ``hwmon``,
+and a properly configured installation of ``lm-sensors`` (or its
+``libsensors`` library component) is often required by other hardware
+monitoring utilities.
+
+Installation
 ------------
 
-Most mainboards have sensor chips to monitor system health (like temperatures,
-voltages, fans speed). They are often connected through an I2C bus, but some
-are also connected directly through the ISA bus.
+Most Linux distributions provide the ``lm-sensors`` suite as a package.
+It is recommended that you use this package for ease of installation.
+Please consult your Linux distribution's documentation for more information.
+
+If needed, sources may be found at https://hwmon.wiki.kernel.org/lm_sensors.
+Basic compilation, installation, and uninstallation may be accomplished with
+``make all``, ``make install``, and ``make uninstall``, respectively.
+
+Usage
+-----
+
+General hints to get things working after installation:
 
-The kernel drivers make the data from the sensor chips available in the /sys
-virtual filesystem. Userspace tools are then used to display the measured
-values or configure the chips in a more friendly manner.
+* Run the ``sensors`` command. Note the available sensors on your system.
 
-Lm-sensors
-----------
+* If you don't see the sensors you expect, here are a few possibilities:
 
-Core set of utilities that will allow you to obtain health information,
-setup monitoring limits etc. You can get them on their homepage
-http://www.lm-sensors.org/ or as a package from your Linux distribution.
+  * You must run the ``sensors-detect`` command to detect available sensors.
+    It will try to find the correct kernel modules (i.e. ``hwmon`` drivers)
+    for your hardware and make the necessary changes to load them
+    automatically. You may have to make these changes yourself in some cases.
 
-If from website:
-Get lm-sensors from project web site. Please note, you need only userspace
-part, so compile with "make user" and install with "make user_install".
+    .. attention::
+      Though unlikely, ``sensors-detect`` may damage your system while
+      conducting certain low-level checks. You will have the chance to refuse.
 
-General hints to get things working:
+  * You are missing the kernel module(s) you need, but it is just a matter of
+    installing it.
 
-0) get lm-sensors userspace utils
-1) compile all drivers in I2C and Hardware Monitoring sections as modules
-   in your kernel
-2) run sensors-detect script, it will tell you what modules you need to load.
-3) load them and run "sensors" command, you should see some results.
-4) fix sensors.conf, labels, limits, fan divisors
-5) if any more problems consult FAQ, or documentation
+    Some Linux distributions already include all in-kernel ``hwmon`` drivers
+    as part of a base installation, but others may only provide them in a
+    "kernel modules" or "extra kernel modules" package. Please consult your
+    Linux distribution's documentation for more information.
+
+    If you are using a self-compiled kernel, ensure that the necessary driver
+    is set to be compiled as modules in the kernel config, then recompile and
+    reinstall.
+
+    In some cases, a hardware vendor or an open-source developer provides an
+    unofficial third-party driver for your hardware. Please refer to the
+    documentation for that driver for more information.
+
+  * The module that you need exists and has been updated to work with your
+    hardware, but only for a later kernel version, or it has not been updated
+    yet, or it does not exist.
+
+    Unfortunately, you will need to upgrade your kernel or Linux distribution,
+    or wait and do so later, or implement support for your hardware yourself.
+
+* Once ``sensors`` is working, you may still need to edit its configuration
+  file (most likely ``/etc/sensors3.conf``) before its output becomes usable.
+
+  For example, this is necessary if several mainboards use the same sensor
+  chip for hardware monitoring duties, but due to design differences between
+  boards, the same measured value means different things on each.
+
+  You may also want to edit the config file to change sensor names, change how
+  displayed values are calculated, and/or hide sensors from view.
+
+  For more information, please refer to the ``man`` pages for ``sensors`` and
+  ``sensors.conf``.
+
+* If you run into more problems, consult FAQs and documentation as needed, or
+  try searching for your particular mainboard or computer system to see what
+  has worked for others.
 
 Other utilities
----------------
+===============
+
+.. warning ::
+  This section is outdated and in need of attention.  Please use this
+  information with caution, and please consider sending patches to update it.
 
 If you want some graphical indicators of system health look for applications
-like: gkrellm, ksensors, xsensors, wmtemp, wmsensors, wmgtemp, ksysguardd,
-hardware-monitor
+like: ``gkrellm``, ``ksensors``, ``xsensors``, ``wmtemp``, ``wmsensors``,
+``wmgtemp``, ``ksysguardd``, ``hardware-monitor``
 
-If you are server administrator you can try snmpd or mrtgutils.
+If you are server administrator you can try ``snmpd`` or ``mrtgutils``.
-- 
2.34.1

[RFC 09/11] ABI: sysfs-class-hwmon: Revise hwmon ABI documentation

[James Seo]

Reformat attribute descriptions to impose a consistent style
and improve rendering in the generated docs. For example,
some multi-line descriptions were being rendered as a single
line/paragraph due to missing blank line separators.

Harmonize short descriptions with those in the sysfs
interface spec and move in the longer explanations that were
shortened there.

Add cross-references (just file paths) in descriptions of
temperature attributes back to the sysfs interface spec
whence they were moved to explain why some sensor chips
report temperature as a voltage. Previously there was only a
note in the existing description of tempY_max directing the
reader to "see below" as though the move had never occurred.

Fix minor issues (typos, grammar, etc.) and add just a touch
of markup (inline literals only).

Link: https://lore.kernel.org/all/5f47619ed882b0b8d1c84b56f7ea17bac0854b77.1632994837.git.mchehab+huawei@kernel.org/
Signed-off-by: James Seo <james@equiv.tech>
---
 Documentation/ABI/testing/sysfs-class-hwmon | 563 +++++++++++---------
 1 file changed, 322 insertions(+), 241 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-hwmon b/Documentation/ABI/testing/sysfs-class-hwmon
index 638f4c6d4ec7..7fc914bc70e2 100644
--- a/Documentation/ABI/testing/sysfs-class-hwmon
+++ b/Documentation/ABI/testing/sysfs-class-hwmon
@@ -1,32 +1,38 @@
 What:		/sys/class/hwmon/hwmonX/name
 Description:
-		The chip name.
+		Chip name.
+
+		Represents the chip name. It is the only mandatory attribute.
+
 		This should be a short, lowercase string, not containing
 		whitespace, dashes, or the wildcard character '*'.
-		This attribute represents the chip name. It is the only
-		mandatory attribute.
+
 		I2C devices get this attribute created automatically.
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/label
 Description:
-		A descriptive label that allows to uniquely identify a
+		Chip label.
+
+		A descriptive freeform label that uniquely identifies a
 		device within the system.
-		The contents of the label are free-form.
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/update_interval
 Description:
-		The interval at which the chip will update readings.
-		Unit: millisecond
+		Chip reading update interval.
 
-		RW
+		The interval at which the chip will update readings.
 
 		Some devices have a variable update rate or interval.
 		This attribute can be used to change it to the desired value.
 
+		Unit: millisecond
+
+		RW
+
 What:		/sys/class/hwmon/hwmonX/inY_min
 Description:
 		Voltage min value.
@@ -39,14 +45,14 @@ What:		/sys/class/hwmon/hwmonX/inY_lcrit
 Description:
 		Voltage critical min value.
 
+		If voltage drops to or below this limit, the system may
+		take drastic action such as powering down or restarting.
+		At the very least, it should report a fault.
+
 		Unit: millivolt
 
 		RW
 
-		If voltage drops to or below this limit, the system may
-		take drastic action such as power down or reset. At the very
-		least, it should report a fault.
-
 What:		/sys/class/hwmon/hwmonX/inY_max
 Description:
 		Voltage max value.
@@ -59,39 +65,40 @@ What:		/sys/class/hwmon/hwmonX/inY_crit
 Description:
 		Voltage critical max value.
 
+		If voltage reaches or exceeds this limit, the system may
+		take drastic action such as powering down or restarting.
+		At the very least, it should report a fault.
+
 		Unit: millivolt
 
 		RW
 
-		If voltage reaches or exceeds this limit, the system may
-		take drastic action such as power down or reset. At the very
-		least, it should report a fault.
-
 What:		/sys/class/hwmon/hwmonX/inY_input
 Description:
 		Voltage input value.
 
-		Unit: millivolt
-
-		RO
+		The voltage measured on the chip pin. The actual voltage
+		depends on the scaling resistors on the motherboard, as
+		recommended in the chip datasheet.
 
-		Voltage measured on the chip pin.
+		This varies by chip and by motherboard. Because of this
+		variation, values are generally NOT scaled by the chip driver,
+		and scaling must be done by the application.
 
-		Actual voltage depends on the scaling resistors on the
-		motherboard, as recommended in the chip datasheet.
+		However, some drivers (notably ``lm87`` and ``via686a``) do
+		scale, because of internal resistors built into the chip.
+		These drivers will output the actual voltage.
 
-		This varies by chip and by motherboard.
-		Because of this variation, values are generally NOT scaled
-		by the chip driver, and must be done by the application.
-		However, some drivers (notably lm87 and via686a)
-		do scale, because of internal resistors built into a chip.
-		These drivers will output the actual voltage. Rule of
-		thumb: drivers should report the voltage values at the
+		Rule of	thumb: Drivers should report the voltage values at the
 		"pins" of the chip.
 
+		Unit: millivolt
+
+		RO
+
 What:		/sys/class/hwmon/hwmonX/inY_average
 Description:
-		Average voltage
+		Average voltage.
 
 		Unit: millivolt
 
@@ -99,7 +106,7 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/inY_lowest
 Description:
-		Historical minimum voltage
+		Historical minimum voltage.
 
 		Unit: millivolt
 
@@ -107,7 +114,7 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/inY_highest
 Description:
-		Historical maximum voltage
+		Historical maximum voltage.
 
 		Unit: millivolt
 
@@ -115,13 +122,13 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/inY_reset_history
 Description:
-		Reset inX_lowest and inX_highest
+		Reset ``inY_lowest`` and ``inY_highest``.
 
 		WO
 
 What:		/sys/class/hwmon/hwmonX/in_reset_history
 Description:
-		Reset inX_lowest and inX_highest for all sensors
+		Reset ``inY_lowest`` and ``inY_highest`` for all sensors.
 
 		WO
 
@@ -129,23 +136,22 @@ What:		/sys/class/hwmon/hwmonX/inY_label
 Description:
 		Suggested voltage channel label.
 
-		Text string
+		A text string.
 
-		Should only be created if the driver has hints about what
-		this voltage channel is being used for, and user-space
-		doesn't. In all other cases, the label is provided by
-		user-space.
+		Should only be created if the driver has hints about what this
+		voltage channel is being used for, and userspace doesn't.
+		In all other cases, the label is provided by userspace.
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/inY_enable
 Description:
-		Enable or disable the sensors.
+		Enable or disable the sensor.
 
-		When disabled the sensor read will return -ENODATA.
+		When disabled, reading the sensor returns ``-ENODATA``.
 
-		- 1: Enable
 		- 0: Disable
+		- 1: Enable
 
 		RW
 
@@ -153,18 +159,16 @@ What:		/sys/class/hwmon/hwmonX/cpuY_vid
 Description:
 		CPU core reference voltage.
 
+		Not always correct.
+
 		Unit: millivolt
 
 		RO
 
-		Not always correct.
-
 What:		/sys/class/hwmon/hwmonX/vrm
 Description:
 		Voltage Regulator Module version number.
 
-		RW (but changing it should no more be necessary)
-
 		Originally the VRM standard version multiplied by 10, but now
 		an arbitrary number, as not all standards have a version
 		number.
@@ -172,6 +176,8 @@ Description:
 		Affects the way the driver calculates the CPU core reference
 		voltage from the vid pins.
 
+		RW (but changing it should not be necessary)
+
 What:		/sys/class/hwmon/hwmonX/inY_rated_min
 Description:
 		Minimum rated voltage.
@@ -190,7 +196,7 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/fanY_min
 Description:
-		Fan minimum value
+		Fan minimum value.
 
 		Unit: revolution/min (RPM)
 
@@ -198,11 +204,12 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/fanY_max
 Description:
-		Fan maximum value
+		Fan maximum value.
+
+		Only rarely supported by the hardware.
 
 		Unit: revolution/min (RPM)
 
-		Only rarely supported by the hardware.
 		RW
 
 What:		/sys/class/hwmon/hwmonX/fanY_input
@@ -217,21 +224,19 @@ What:		/sys/class/hwmon/hwmonX/fanY_div
 Description:
 		Fan divisor.
 
-		Integer value in powers of two (1, 2, 4, 8, 16, 32, 64, 128).
-
-		RW
-
+		An integer power of two (1, 2, 4, 8, 16, 32, 64, 128).
 		Some chips only support values 1, 2, 4 and 8.
+
 		Note that this is actually an internal clock divisor, which
 		affects the measurable speed range, not the read value.
 
+		RW
+
 What:		/sys/class/hwmon/hwmonX/fanY_pulses
 Description:
 		Number of tachometer pulses per fan revolution.
 
-		Integer value, typically between 1 and 4.
-
-		RW
+		An integer value, typically between 1 and 4.
 
 		This value is a characteristic of the fan connected to the
 		device's input, so it has to be set in accordance with the fan
@@ -242,74 +247,80 @@ Description:
 		thus attribute) the value assumed by all devices is 2 pulses
 		per fan revolution.
 
+		RW
+
 What:		/sys/class/hwmon/hwmonX/fanY_target
 Description:
-		Desired fan speed
+		Desired fan speed.
+
+		Only makes sense if the chip supports closed-loop fan speed
+		control based on the measured fan speed.
 
 		Unit: revolution/min (RPM)
 
 		RW
 
-		Only makes sense if the chip supports closed-loop fan speed
-		control based on the measured fan speed.
-
 What:		/sys/class/hwmon/hwmonX/fanY_label
 Description:
 		Suggested fan channel label.
 
-		Text string
+		A text string.
 
-		Should only be created if the driver has hints about what
-		this fan channel is being used for, and user-space doesn't.
-		In all other cases, the label is provided by user-space.
+		Should only be created if the driver has hints about what this
+		fan channel is being used for, and userspace doesn't.
+		In all other cases, the label is provided by userspace.
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/fanY_enable
 Description:
-		Enable or disable the sensors.
+		Enable or disable the sensor.
 
-		When disabled the sensor read will return -ENODATA.
+		When disabled, reading the sensor returns ``-ENODATA``.
 
-		- 1: Enable
 		- 0: Disable
+		- 1: Enable
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/fanY_fault
 Description:
-		Reports if a fan has reported failure.
+		Fan channel fault indicator.
+
+		Indicates whether a fan has reported failure.
 
+		- 0: OK
 		- 1: Failed
-		- 0: Ok
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/pwmY
 Description:
-		Pulse width modulation fan control.
+		Pulse-width modulation fan control.
 
-		Integer value in the range 0 to 255
+		An integer value in the range 0 to 255.
 
-		RW
+		255 represents 100%.
 
-		255 is max or 100%.
+		RW
 
 What:		/sys/class/hwmon/hwmonX/pwmY_enable
 Description:
-		Fan speed control method:
-
-		- 0: no fan speed control (i.e. fan at full speed)
-		- 1: manual fan speed control enabled (using `pwmY`)
-		- 2+: automatic fan speed control enabled
+		Fan speed control method.
 
 		Check individual chip documentation files for automatic mode
 		details.
 
+		- 0: No fan speed control (i.e. fan at full speed)
+		- 1: Manual fan speed control enabled (using ``pwmY``)
+		- 2+: Automatic fan speed control enabled
+
 		RW
 
 What:		/sys/class/hwmon/hwmonX/pwmY_mode
 Description:
+		Fan speed control mode.
+
 		- 0: DC mode (direct current)
 		- 1: PWM mode (pulse-width modulation)
 
@@ -319,18 +330,24 @@ What:		/sys/class/hwmon/hwmonX/pwmY_freq
 Description:
 		Base PWM frequency in Hz.
 
-		Only possibly available when pwmN_mode is PWM, but not always
-		present even then.
+		May only be available when ``pwmY_mode`` is PWM,
+		but not always present even then.
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/pwmY_auto_channels_temp
 Description:
-		Select which temperature channels affect this PWM output in
-		auto mode.
+		Auto mode channel mask.
+
+		Select which temperature channels affect this PWM output
+		in auto mode.
 
-		Bitfield, 1 is temp1, 2 is temp2, 4 is temp3 etc...
-		Which values are possible depend on the chip used.
+		A bitfield. Possible values are chip-dependent. For example:
+
+		- 1: ``temp1``
+		- 2: ``temp2``
+		- 4: ``temp3``
+		- 5: ``temp1`` and ``temp3``
 
 		RW
 
@@ -338,10 +355,13 @@ What:		/sys/class/hwmon/hwmonX/pwmY_auto_pointZ_pwm
 What:		/sys/class/hwmon/hwmonX/pwmY_auto_pointZ_temp
 What:		/sys/class/hwmon/hwmonX/pwmY_auto_pointZ_temp_hyst
 Description:
-		Define the PWM vs temperature curve.
+		PWM temperature curve definition.
+
+		Define the PWM vs. temperature curve.
+		The number of trip points is chip-dependent.
 
-		Number of trip points is chip-dependent. Use this for chips
-		which associate trip points to PWM output channels.
+		Use this for chips that associate trip points
+		with PWM output channels.
 
 		RW
 
@@ -349,10 +369,13 @@ What:		/sys/class/hwmon/hwmonX/tempY_auto_pointZ_pwm
 What:		/sys/class/hwmon/hwmonX/tempY_auto_pointZ_temp
 What:		/sys/class/hwmon/hwmonX/tempY_auto_pointZ_temp_hyst
 Description:
-		Define the PWM vs temperature curve.
+		PWM temperature curve definition.
 
-		Number of trip points is chip-dependent. Use this for chips
-		which associate trip points to temperature channels.
+		Define the PWM vs. temperature curve.
+		The number of trip points is chip-dependent.
+
+		Use this for chips that associate trip points
+		with temperature channels.
 
 		RW
 
@@ -360,24 +383,25 @@ What:		/sys/class/hwmon/hwmonX/tempY_type
 Description:
 		Sensor type selection.
 
-		Integers 1 to 6
-
-		RW
+		Not all types are supported by all chips.
 
 		- 1: CPU embedded diode
 		- 2: 3904 transistor
-		- 3: thermal diode
-		- 4: thermistor
+		- 3: Thermal diode
+		- 4: Thermistor
 		- 5: AMD AMDSI
 		- 6: Intel PECI
 
-		Not all types are supported by all chips
+		RW
 
 What:		/sys/class/hwmon/hwmonX/tempY_max
 Description:
 		Temperature max value.
 
-		Unit: millidegree Celsius (or millivolt, see below)
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
 
 		RW
 
@@ -385,7 +409,10 @@ What:		/sys/class/hwmon/hwmonX/tempY_min
 Description:
 		Temperature min value.
 
-		Unit: millidegree Celsius
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
 
 		RW
 
@@ -393,20 +420,27 @@ What:		/sys/class/hwmon/hwmonX/tempY_max_hyst
 Description:
 		Temperature hysteresis value for max limit.
 
-		Unit: millidegree Celsius
+		Must be reported as an absolute temperature,
+		NOT as a delta from the max value.
 
-		Must be reported as an absolute temperature, NOT a delta
-		from the max value.
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/tempY_min_hyst
 Description:
 		Temperature hysteresis value for min limit.
-		Unit: millidegree Celsius
 
-		Must be reported as an absolute temperature, NOT a delta
-		from the min value.
+		Must be reported as an absolute temperature,
+		NOT as a delta from the min value.
+
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
 
 		RW
 
@@ -414,51 +448,67 @@ What:		/sys/class/hwmon/hwmonX/tempY_input
 Description:
 		Temperature input value.
 
-		Unit: millidegree Celsius
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/tempY_crit
 Description:
-		Temperature critical max value, typically greater than
-		corresponding temp_max values.
+		Temperature critical max value.
+
+		Typically greater than corresponding ``tempY_max`` values.
+
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
 
-		Unit: millidegree Celsius
+		Unit: millidegree Celsius (or millivolt)
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/tempY_crit_alarm
 Description:
-		Critical high temperature alarm flag.
-
-		- 0: OK
-		- 1: temperature has reached tempY_crit
-
-		RW
+		Temperature critical max alarm indicator.
 
 		Contrary to regular alarm flags which clear themselves
 		automatically when read, this one sticks until cleared by
 		the user. This is done by writing 0 to the file. Writing
 		other values is unsupported.
 
+		- 0: No alarm
+		- 1: Alarm
+
+		RW
+
 What:		/sys/class/hwmon/hwmonX/tempY_crit_hyst
 Description:
 		Temperature hysteresis value for critical limit.
 
-		Unit: millidegree Celsius
+		Must be reported as an absolute temperature,
+		NOT as a delta from the critical value.
+
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
 
-		Must be reported as an absolute temperature, NOT a delta
-		from the critical value.
+		Unit: millidegree Celsius (or millivolt)
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/tempY_emergency
 Description:
-		Temperature emergency max value, for chips supporting more than
-		two upper temperature limits. Must be equal or greater than
-		corresponding temp_crit values.
+		Temperature emergency max value.
 
-		Unit: millidegree Celsius
+		For chips supporting more than two upper temperature limits.
+
+		Must be equal to or greater than corresponding ``tempY_crit``
+		values.
+
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
 
 		RW
 
@@ -466,19 +516,26 @@ What:		/sys/class/hwmon/hwmonX/tempY_emergency_hyst
 Description:
 		Temperature hysteresis value for emergency limit.
 
-		Unit: millidegree Celsius
+		Must be reported as an absolute temperature,
+		NOT as a delta from the emergency value.
+
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
 
-		Must be reported as an absolute temperature, NOT a delta
-		from the emergency value.
+		Unit: millidegree Celsius (or millivolt)
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/tempY_lcrit
 Description:
-		Temperature critical min value, typically lower than
-		corresponding temp_min values.
+		Temperature critical min value.
 
-		Unit: millidegree Celsius
+		Typically lower than corresponding ``tempY_min`` values.
+
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
 
 		RW
 
@@ -486,68 +543,80 @@ What:		/sys/class/hwmon/hwmonX/tempY_lcrit_hyst
 Description:
 		Temperature hysteresis value for critical min limit.
 
-		Unit: millidegree Celsius
+		Must be reported as an absolute temperature,
+		NOT as a delta from the critical min value.
+
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
 
-		Must be reported as an absolute temperature, NOT a delta
-		from the critical min value.
+		Unit: millidegree Celsius (or millivolt)
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/tempY_offset
 Description:
-		Temperature offset which is added to the temperature reading
-		by the chip.
+		Temperature offset.
 
-		Unit: millidegree Celsius
+		An offset added to the temperature reading by the chip.
 
-		Read/Write value.
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
+
+		RW
 
 What:		/sys/class/hwmon/hwmonX/tempY_label
 Description:
 		Suggested temperature channel label.
 
-		Text string
+		A text string.
 
-		Should only be created if the driver has hints about what
-		this temperature channel is being used for, and user-space
-		doesn't. In all other cases, the label is provided by
-		user-space.
+		Should only be created if the driver has hints about what this
+		temperature channel is being used for, and userspace doesn't.
+		In all other cases, the label is provided by userspace.
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/tempY_lowest
 Description:
-		Historical minimum temperature
+		Historical minimum temperature.
+
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
 
-		Unit: millidegree Celsius
+		Unit: millidegree Celsius (or millivolt)
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/tempY_highest
 Description:
-		Historical maximum temperature
+		Historical maximum temperature.
 
-		Unit: millidegree Celsius
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/tempY_reset_history
 Description:
-		Reset temp_lowest and temp_highest
+		Reset ``tempY_lowest`` and ``tempY_highest``.
 
 		WO
 
 What:		/sys/class/hwmon/hwmonX/temp_reset_history
 Description:
-		Reset temp_lowest and temp_highest for all sensors
+		Reset ``tempY_lowest`` and ``tempY_highest`` for all sensors.
 
 		WO
 
 What:		/sys/class/hwmon/hwmonX/tempY_enable
 Description:
-		Enable or disable the sensors.
+		Enable or disable the sensor.
 
-		When disabled the sensor read will return -ENODATA.
+		When disabled, reading the sensor returns ``-ENODATA``.
 
 		- 1: Enable
 		- 0: Disable
@@ -558,7 +627,10 @@ What:		/sys/class/hwmon/hwmonX/tempY_rated_min
 Description:
 		Minimum rated temperature.
 
-		Unit: millidegree Celsius
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
 
 		RO
 
@@ -566,13 +638,16 @@ What:		/sys/class/hwmon/hwmonX/tempY_rated_max
 Description:
 		Maximum rated temperature.
 
-		Unit: millidegree Celsius
+		In some cases, this is a voltage. Please see
+		Documentation/hwmon/sysfs-interface.rst for more details.
+
+		Unit: millidegree Celsius (or millivolt)
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/currY_max
 Description:
-		Current max value
+		Current max value.
 
 		Unit: milliampere
 
@@ -588,7 +663,7 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/currY_lcrit
 Description:
-		Current critical low value
+		Current critical min value.
 
 		Unit: milliampere
 
@@ -596,7 +671,7 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/currY_crit
 Description:
-		Current critical high value.
+		Current critical max value.
 
 		Unit: milliampere
 
@@ -604,7 +679,7 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/currY_input
 Description:
-		Current input value
+		Current input value.
 
 		Unit: milliampere
 
@@ -612,7 +687,7 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/currY_average
 Description:
-		Average current use
+		Average current use.
 
 		Unit: milliampere
 
@@ -620,7 +695,7 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/currY_lowest
 Description:
-		Historical minimum current
+		Historical minimum current.
 
 		Unit: milliampere
 
@@ -628,27 +703,29 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/currY_highest
 Description:
-		Historical maximum current
+		Historical maximum current.
+
 		Unit: milliampere
+
 		RO
 
 What:		/sys/class/hwmon/hwmonX/currY_reset_history
 Description:
-		Reset currX_lowest and currX_highest
+		Reset ``currY_lowest`` and ``currY_highest``.
 
 		WO
 
 What:		/sys/class/hwmon/hwmonX/curr_reset_history
 Description:
-		Reset currX_lowest and currX_highest for all sensors
+		Reset ``currY_lowest`` and ``currY_highest`` for all sensors.
 
 		WO
 
 What:		/sys/class/hwmon/hwmonX/currY_enable
 Description:
-		Enable or disable the sensors.
+		Enable or disable the sensor.
 
-		When disabled the sensor read will return -ENODATA.
+		When disabled, reading the sensor returns ``-ENODATA``.
 
 		- 1: Enable
 		- 0: Disable
@@ -673,102 +750,105 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/powerY_average
 Description:
-		Average power use
+		Average power use.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/powerY_average_interval
 Description:
-		Power use averaging interval.  A poll
-		notification is sent to this file if the
-		hardware changes the averaging interval.
+		Power use averaging interval.
+
+		A poll notification is sent to this file if
+		the hardware changes the averaging interval.
 
-		Unit: milliseconds
+		Unit: millisecond
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/powerY_average_interval_max
 Description:
-		Maximum power use averaging interval
+		Maximum power use averaging interval.
 
-		Unit: milliseconds
+		Unit: millisecond
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/powerY_average_interval_min
 Description:
-		Minimum power use averaging interval
+		Minimum power use averaging interval.
 
-		Unit: milliseconds
+		Unit: millisecond
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/powerY_average_highest
 Description:
-		Historical average maximum power use
+		Historical average maximum power use.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/powerY_average_lowest
 Description:
-		Historical average minimum power use
+		Historical average minimum power use.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/powerY_average_max
 Description:
-		A poll notification is sent to
-		`powerY_average` when power use
-		rises above this value.
+		Maximum average power notification threshold.
+
+		A poll notification is sent to `powerY_average`
+		when power use rises above this value.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/powerY_average_min
 Description:
-		A poll notification is sent to
-		`powerY_average` when power use
-		sinks below this value.
+		Minimum average power notification threshold.
 
-		Unit: microWatt
+		A poll notification is sent to `powerY_average`
+		when power use sinks below this value.
+
+		Unit: microwatt
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/powerY_input
 Description:
-		Instantaneous power use
+		Instantaneous power use.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/powerY_input_highest
 Description:
-		Historical maximum power use
+		Historical maximum power use.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/powerY_input_lowest
 Description:
-		Historical minimum power use
+		Historical minimum power use.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/powerY_reset_history
 Description:
-		Reset input_highest, input_lowest,
-		average_highest and average_lowest.
+		Reset ``powerY_input_highest``, ``powerY_input_lowest``,
+		``powerY_average_highest``, and ``powerY_average_lowest``.
 
 		WO
 
@@ -776,29 +856,34 @@ What:		/sys/class/hwmon/hwmonX/powerY_accuracy
 Description:
 		Accuracy of the power meter.
 
-		Unit: Percent
+		Unit: percent
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/powerY_cap
 Description:
-		If power use rises above this limit, the
-		system should take action to reduce power use.
-		A poll notification is sent to this file if the
-		cap is changed by the hardware.  The `*_cap`
-		files only appear if the cap is known to be
-		enforced by hardware.
+		Power reduction threshold.
+
+		If power use rises above this limit, the system should
+		take action to reduce power use.
 
-		Unit: microWatt
+		A poll notification is sent to this file if the cap is
+		changed by the hardware.
+
+		The `powerY_cap` files only appear if the cap is known
+		to be enforced by hardware.
+
+		Unit: microwatt
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/powerY_cap_hyst
 Description:
-		Margin of hysteresis built around capping and
-		notification.
+		Threshold hysteresis margin.
 
-		Unit: microWatt
+		Margin of hysteresis built around capping and notification.
+
+		Unit: microwatt
 
 		RW
 
@@ -806,7 +891,7 @@ What:		/sys/class/hwmon/hwmonX/powerY_cap_max
 Description:
 		Maximum cap that can be set.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RO
 
@@ -814,7 +899,7 @@ What:		/sys/class/hwmon/hwmonX/powerY_cap_min
 Description:
 		Minimum cap that can be set.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RO
 
@@ -822,29 +907,27 @@ What:		/sys/class/hwmon/hwmonX/powerY_max
 Description:
 		Maximum power.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/powerY_crit
 Description:
-		Critical maximum power.
+		Critical power reduction threshold.
 
-		If power rises to or above this limit, the
-		system is expected take drastic action to reduce
-		power consumption, such as a system shutdown or
-		a forced powerdown of some devices.
+		If power rises to or above this limit, the system is expected
+		to take drastic action to reduce power consumption, such as a
+		system shutdown or a forced powerdown of some devices.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/powerY_enable
 Description:
-		Enable or disable the sensors.
+		Enable or disable the sensor.
 
-		When disabled the sensor read will return
-		-ENODATA.
+		When disabled, reading the sensor returns ``-ENODATA``.
 
 		- 1: Enable
 		- 0: Disable
@@ -855,7 +938,7 @@ What:		/sys/class/hwmon/hwmonX/powerY_rated_min
 Description:
 		Minimum rated power.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RO
 
@@ -863,24 +946,23 @@ What:		/sys/class/hwmon/hwmonX/powerY_rated_max
 Description:
 		Maximum rated power.
 
-		Unit: microWatt
+		Unit: microwatt
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/energyY_input
 Description:
-		Cumulative energy use
+		Cumulative energy use.
 
-		Unit: microJoule
+		Unit: microjoule
 
 		RO
 
 What:		/sys/class/hwmon/hwmonX/energyY_enable
 Description:
-		Enable or disable the sensors.
+		Enable or disable the sensor.
 
-		When disabled the sensor read will return
-		-ENODATA.
+		When disabled, reading the sensor returns ``-ENODATA``.
 
 		- 1: Enable
 		- 0: Disable
@@ -889,19 +971,17 @@ Description:
 
 What:		/sys/class/hwmon/hwmonX/humidityY_input
 Description:
-		Humidity
+		Humidity.
 
-		Unit: milli-percent (per cent mille, pcm)
+		Unit: millipercent (per cent mille, pcm)
 
 		RO
 
-
 What:		/sys/class/hwmon/hwmonX/humidityY_enable
 Description:
-		Enable or disable the sensors
+		Enable or disable the sensor.
 
-		When disabled the sensor read will return
-		-ENODATA.
+		When disabled, reading the sensor returns ``-ENODATA``.
 
 		- 1: Enable
 		- 0: Disable
@@ -912,7 +992,7 @@ What:		/sys/class/hwmon/hwmonX/humidityY_rated_min
 Description:
 		Minimum rated humidity.
 
-		Unit: milli-percent (per cent mille, pcm)
+		Unit: millipercent (per cent mille, pcm)
 
 		RO
 
@@ -920,39 +1000,40 @@ What:		/sys/class/hwmon/hwmonX/humidityY_rated_max
 Description:
 		Maximum rated humidity.
 
-		Unit: milli-percent (per cent mille, pcm)
+		Unit: millipercent (per cent mille, pcm)
 
 		RO
 
-
 What:		/sys/class/hwmon/hwmonX/intrusionY_alarm
 Description:
-		Chassis intrusion detection
-
-		- 0: OK
-		- 1: intrusion detected
-
-		RW
+		Chassis intrusion indicator.
 
 		Contrary to regular alarm flags which clear themselves
 		automatically when read, this one sticks until cleared by
 		the user. This is done by writing 0 to the file. Writing
 		other values is unsupported.
 
+		- 0: OK
+		- 1: Intrusion detected
+
+		RW
+
 What:		/sys/class/hwmon/hwmonX/intrusionY_beep
 Description:
-		Chassis intrusion beep
+		Chassis intrusion beep.
 
-		- 0: disable
-		- 1: enable
+		- 0: Disable
+		- 1: Enable
 
 		RW
 
 What:		/sys/class/hwmon/hwmonX/device/pec
 Description:
-		PEC support on I2C devices
+		PEC support on I2C devices.
 
-		- 0, off, n: disable
-		- 1, on, y: enable
+		- 0: Off
+		- 1: On
+		- n: Disable
+		- y: Enable
 
 		RW
-- 
2.34.1

[RFC 10/11] hwmon: (core) Add missing beep-related standard attributes

[James Seo]

beep_enable, inY_beep, currY_beep, fanY_beep, and tempY_beep
are standard attributes mentioned in the sysfs interface
specification but not implemented in the hwmon core. Since
these are not deprecated, implement them.

Adding beep_mask is not necessary, as it is deprecated and
the drivers already using it are manually defining it.

Signed-off-by: James Seo <james@equiv.tech>
---
 drivers/hwmon/hwmon.c |  5 +++++
 include/linux/hwmon.h | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index cc140cf99290..7b611a073be4 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -456,6 +456,7 @@ static const char * const hwmon_chip_attrs[] = {
 	[hwmon_chip_in_samples] = "in_samples",
 	[hwmon_chip_power_samples] = "power_samples",
 	[hwmon_chip_temp_samples] = "temp_samples",
+	[hwmon_chip_beep_enable] = "beep_enable",
 };
 
 static const char * const hwmon_temp_attr_templates[] = {
@@ -486,6 +487,7 @@ static const char * const hwmon_temp_attr_templates[] = {
 	[hwmon_temp_reset_history] = "temp%d_reset_history",
 	[hwmon_temp_rated_min] = "temp%d_rated_min",
 	[hwmon_temp_rated_max] = "temp%d_rated_max",
+	[hwmon_temp_beep] = "temp%d_beep",
 };
 
 static const char * const hwmon_in_attr_templates[] = {
@@ -507,6 +509,7 @@ static const char * const hwmon_in_attr_templates[] = {
 	[hwmon_in_crit_alarm] = "in%d_crit_alarm",
 	[hwmon_in_rated_min] = "in%d_rated_min",
 	[hwmon_in_rated_max] = "in%d_rated_max",
+	[hwmon_in_beep] = "in%d_beep",
 };
 
 static const char * const hwmon_curr_attr_templates[] = {
@@ -528,6 +531,7 @@ static const char * const hwmon_curr_attr_templates[] = {
 	[hwmon_curr_crit_alarm] = "curr%d_crit_alarm",
 	[hwmon_curr_rated_min] = "curr%d_rated_min",
 	[hwmon_curr_rated_max] = "curr%d_rated_max",
+	[hwmon_curr_beep] = "curr%d_beep",
 };
 
 static const char * const hwmon_power_attr_templates[] = {
@@ -562,6 +566,7 @@ static const char * const hwmon_power_attr_templates[] = {
 	[hwmon_power_crit_alarm] = "power%d_crit_alarm",
 	[hwmon_power_rated_min] = "power%d_rated_min",
 	[hwmon_power_rated_max] = "power%d_rated_max",
+	[hwmon_curr_beep] = "power%d_beep",
 };
 
 static const char * const hwmon_energy_attr_templates[] = {
diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
index fe80e8e24b5a..496448a5f331 100644
--- a/include/linux/hwmon.h
+++ b/include/linux/hwmon.h
@@ -60,6 +60,7 @@ enum hwmon_chip_attributes {
 	hwmon_chip_in_samples,
 	hwmon_chip_power_samples,
 	hwmon_chip_temp_samples,
+	hwmon_chip_beep_enable,
 };
 
 #define HWMON_C_TEMP_RESET_HISTORY	BIT(hwmon_chip_temp_reset_history)
@@ -74,6 +75,7 @@ enum hwmon_chip_attributes {
 #define HWMON_C_IN_SAMPLES		BIT(hwmon_chip_in_samples)
 #define HWMON_C_POWER_SAMPLES		BIT(hwmon_chip_power_samples)
 #define HWMON_C_TEMP_SAMPLES		BIT(hwmon_chip_temp_samples)
+#define HWMON_C_BEEP_ENABLE		BIT(hwmon_chip_beep_enable)
 
 enum hwmon_temp_attributes {
 	hwmon_temp_enable,
@@ -103,6 +105,7 @@ enum hwmon_temp_attributes {
 	hwmon_temp_reset_history,
 	hwmon_temp_rated_min,
 	hwmon_temp_rated_max,
+	hwmon_temp_beep,
 };
 
 #define HWMON_T_ENABLE		BIT(hwmon_temp_enable)
@@ -132,6 +135,7 @@ enum hwmon_temp_attributes {
 #define HWMON_T_RESET_HISTORY	BIT(hwmon_temp_reset_history)
 #define HWMON_T_RATED_MIN	BIT(hwmon_temp_rated_min)
 #define HWMON_T_RATED_MAX	BIT(hwmon_temp_rated_max)
+#define HWMON_T_BEEP		BIT(hwmon_temp_beep)
 
 enum hwmon_in_attributes {
 	hwmon_in_enable,
@@ -152,6 +156,7 @@ enum hwmon_in_attributes {
 	hwmon_in_crit_alarm,
 	hwmon_in_rated_min,
 	hwmon_in_rated_max,
+	hwmon_in_beep,
 };
 
 #define HWMON_I_ENABLE		BIT(hwmon_in_enable)
@@ -172,6 +177,7 @@ enum hwmon_in_attributes {
 #define HWMON_I_CRIT_ALARM	BIT(hwmon_in_crit_alarm)
 #define HWMON_I_RATED_MIN	BIT(hwmon_in_rated_min)
 #define HWMON_I_RATED_MAX	BIT(hwmon_in_rated_max)
+#define HWMON_I_BEEP		BIT(hwmon_in_beep)
 
 enum hwmon_curr_attributes {
 	hwmon_curr_enable,
@@ -192,6 +198,7 @@ enum hwmon_curr_attributes {
 	hwmon_curr_crit_alarm,
 	hwmon_curr_rated_min,
 	hwmon_curr_rated_max,
+	hwmon_curr_beep,
 };
 
 #define HWMON_C_ENABLE		BIT(hwmon_curr_enable)
@@ -212,6 +219,7 @@ enum hwmon_curr_attributes {
 #define HWMON_C_CRIT_ALARM	BIT(hwmon_curr_crit_alarm)
 #define HWMON_C_RATED_MIN	BIT(hwmon_curr_rated_min)
 #define HWMON_C_RATED_MAX	BIT(hwmon_curr_rated_max)
+#define HWMON_C_BEEP		BIT(hwmon_curr_beep)
 
 enum hwmon_power_attributes {
 	hwmon_power_enable,
@@ -328,6 +336,7 @@ enum hwmon_fan_attributes {
 	hwmon_fan_min_alarm,
 	hwmon_fan_max_alarm,
 	hwmon_fan_fault,
+	hwmon_fan_beep,
 };
 
 #define HWMON_F_ENABLE			BIT(hwmon_fan_enable)
@@ -342,6 +351,7 @@ enum hwmon_fan_attributes {
 #define HWMON_F_MIN_ALARM		BIT(hwmon_fan_min_alarm)
 #define HWMON_F_MAX_ALARM		BIT(hwmon_fan_max_alarm)
 #define HWMON_F_FAULT			BIT(hwmon_fan_fault)
+#define HWMON_F_BEEP			BIT(hwmon_fan_beep)
 
 enum hwmon_pwm_attributes {
 	hwmon_pwm_input,
-- 
2.34.1

[RFC 11/11] ABI: sysfs-class-hwmon: Add missing hwmon standard attributes

[James Seo]

Add definitions for the following non-deprecated standard
attributes implemented in the API and mentioned in the sysfs
interface spec but not found in the ABI documentation.

Alarm attributes:
* inY_alarm, inY_min_alarm, inY_max_alarm,
  inY_lcrit_alarm, inY_crit_alarm
* currY_alarm, currY_min_alarm, currY_max_alarm,
  currY_lcrit_alarm, currY_crit_alarm
* powerY_alarm, powerY_cap_alarm,
  powerY_max_alarm, powerY_crit_alarm
* fanY_alarm, fanY_min_alarm, fanY_max_alarm
* tempY_alarm, tempY_min_alarm, tempY_max_alarm,
  tempY_lcrit_alarm, tempY_emergency_alarm
  (tempY_crit_alarm already existed)

Beep attributes:
beep_enable, inY_beep, currY_beep, fanY_beep, tempY_beep

Sample average attributes:
samples, in_samples, power_samples, temp_samples, curr_samples

Fault attributes:
tempY_fault (fanY_fault already existed)

Signed-off-by: James Seo <james@equiv.tech>
---
 Documentation/ABI/testing/sysfs-class-hwmon | 358 +++++++++++++++++++-
 1 file changed, 344 insertions(+), 14 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-hwmon b/Documentation/ABI/testing/sysfs-class-hwmon
index 7fc914bc70e2..2f6884874812 100644
--- a/Documentation/ABI/testing/sysfs-class-hwmon
+++ b/Documentation/ABI/testing/sysfs-class-hwmon
@@ -33,6 +33,23 @@ Description:
 
 		RW
 
+What:		/sys/class/hwmon/hwmonX/beep_enable
+Description:
+		Enable or disable beeps.
+
+		- 0: No beeps
+		- 1: Beeps
+
+		RW
+
+What:		/sys/class/hwmon/hwmonX/samples
+Description:
+		Samples in calculated average.
+
+		Applies to all types of channels.
+
+		RW
+
 What:		/sys/class/hwmon/hwmonX/inY_min
 Description:
 		Voltage min value.
@@ -194,6 +211,76 @@ Description:
 
 		RO
 
+What:		/sys/class/hwmon/hwmonX/inY_alarm
+Description:
+		Voltage channel alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/inY_min_alarm
+Description:
+		Voltage min alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/inY_max_alarm
+Description:
+		Voltage max alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/inY_lcrit_alarm
+Description:
+		Voltage critical min alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/inY_crit_alarm
+Description:
+		Voltage critical max alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/inY_beep
+Description:
+		Voltage channel beep.
+
+		- 0: Disable
+		- 1: Enable
+
+		RW
+
+What:		/sys/class/hwmon/hwmonX/in_samples
+Description:
+		Voltage samples in calculated average.
+
+		RW
+
 What:		/sys/class/hwmon/hwmonX/fanY_min
 Description:
 		Fan minimum value.
@@ -283,6 +370,48 @@ Description:
 
 		RW
 
+What:		/sys/class/hwmon/hwmonX/fanY_alarm
+Description:
+		Fan channel alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/fanY_min_alarm
+Description:
+		Fan min alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/fanY_max_alarm
+Description:
+		Fan max alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/fanY_beep
+Description:
+		Fan channel beep.
+
+		- 0: Disable
+		- 1: Enable
+
+		RW
+
 What:		/sys/class/hwmon/hwmonX/fanY_fault
 Description:
 		Fan channel fault indicator.
@@ -468,20 +597,6 @@ Description:
 
 		RW
 
-What:		/sys/class/hwmon/hwmonX/tempY_crit_alarm
-Description:
-		Temperature critical max alarm indicator.
-
-		Contrary to regular alarm flags which clear themselves
-		automatically when read, this one sticks until cleared by
-		the user. This is done by writing 0 to the file. Writing
-		other values is unsupported.
-
-		- 0: No alarm
-		- 1: Alarm
-
-		RW
-
 What:		/sys/class/hwmon/hwmonX/tempY_crit_hyst
 Description:
 		Temperature hysteresis value for critical limit.
@@ -645,6 +760,101 @@ Description:
 
 		RO
 
+What:		/sys/class/hwmon/hwmonX/tempY_alarm
+Description:
+		Temperature channel alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/tempY_min_alarm
+Description:
+		Temperature min alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/tempY_max_alarm
+Description:
+		Temperature max alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/tempY_lcrit_alarm
+Description:
+		Temperature critical min alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/tempY_crit_alarm
+Description:
+		Temperature critical max alarm indicator.
+
+		Contrary to regular alarm flags which clear themselves
+		automatically when read, this one sticks until cleared by
+		the user. This is done by writing 0 to the file. Writing
+		other values is unsupported.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RW
+
+What:		/sys/class/hwmon/hwmonX/tempY_emergency_alarm
+Description:
+		Temperature emergency max alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/tempY_beep
+Description:
+		Temperature channel beep.
+
+		- 0: Disable
+		- 1: Enable
+
+		RW
+
+What:		/sys/class/hwmon/hwmonX/tempY_fault
+Description:
+		Temperature channel fault indicator.
+
+		Indicates whether a temperature sensor has reported failure.
+
+		- 0: OK
+		- 1: Failed
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/temp_samples
+Description:
+		Temperature samples in calculated average.
+
+		RW
+
 What:		/sys/class/hwmon/hwmonX/currY_max
 Description:
 		Current max value.
@@ -748,6 +958,76 @@ Description:
 
 		RO
 
+What:		/sys/class/hwmon/hwmonX/currY_alarm
+Description:
+		Current channel alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/currY_min_alarm
+Description:
+		Current min alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/currY_max_alarm
+Description:
+		Current max alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/currY_lcrit_alarm
+Description:
+		Current critical min alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/currY_crit_alarm
+Description:
+		Current critical max alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/currY_beep
+Description:
+		Current channel beep.
+
+		- 0: Disable
+		- 1: Enable
+
+		RW
+
+What:		/sys/class/hwmon/hwmonX/curr_samples
+Description:
+		Current samples in calculated average.
+
+		RW
+
 What:		/sys/class/hwmon/hwmonX/powerY_average
 Description:
 		Average power use.
@@ -950,6 +1230,56 @@ Description:
 
 		RO
 
+What:		/sys/class/hwmon/hwmonX/powerY_alarm
+Description:
+		Power channel alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/powerY_cap_alarm
+Description:
+		Power reduction alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/powerY_max_alarm
+Description:
+		Power max alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/powerY_crit_alarm
+Description:
+		Power critical reduction alarm indicator.
+
+		Clears itself when read.
+
+		- 0: No alarm
+		- 1: Alarm
+
+		RO
+
+What:		/sys/class/hwmon/hwmonX/power_samples
+Description:
+		Power samples in calculated average.
+
+		RW
+
 What:		/sys/class/hwmon/hwmonX/energyY_input
 Description:
 		Cumulative energy use.
-- 
2.34.1

Re: [RFC 02/11] hwmon: (core) Rename last parameter of devm_hwmon_register_with_info()

[Guenter Roeck]

On 5/4/23 00:57, James Seo wrote:
> Change the name of the groups parameter of
> devm_hwmon_device_register_with_info() to extra_groups to
> match the name given by the hwmon API reference and in
> hwmon_device_register_with_info().
> 
> Signed-off-by: James Seo <james@equiv.tech>
> ---
>   drivers/hwmon/hwmon.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index 573b83b6c08c..5f205686065e 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -1029,7 +1029,7 @@ EXPORT_SYMBOL_GPL(devm_hwmon_device_register_with_groups);
>    * @name:	hwmon name attribute
>    * @drvdata:	driver data to attach to created device
>    * @chip:	pointer to hwmon chip information
> - * @groups:	pointer to list of driver specific attribute groups
> + * @extra_groups: pointer to list of driver specific attribute groups
>    *
>    * Returns the pointer to the new device. The new device is automatically
>    * unregistered with the parent device.
> @@ -1038,7 +1038,7 @@ struct device *
>   devm_hwmon_device_register_with_info(struct device *dev, const char *name,
>   				     void *drvdata,
>   				     const struct hwmon_chip_info *chip,
> -				     const struct attribute_group **groups)
> +				     const struct attribute_group **extra_groups)

Please please please no such changes. I don't want to have to deal with
patch wars just because people believe variables should have other names.

Such changes add zero value unless one counts wasted review time as a "value".

Guenter

>   {
>   	struct device **ptr, *hwdev;
>   
> @@ -1050,7 +1050,7 @@ devm_hwmon_device_register_with_info(struct device *dev, const char *name,
>   		return ERR_PTR(-ENOMEM);
>   
>   	hwdev = hwmon_device_register_with_info(dev, name, drvdata, chip,
> -						groups);
> +						extra_groups);
>   	if (IS_ERR(hwdev))
>   		goto error;
>   

Re: [RFC 04/11] Documentation/hwmon: Revise hwmon kernel API reference

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

On Thu, May 04, 2023 at 12:57:45AM -0700, James Seo wrote:
> Reorganize content into coherent sections.
> Use kerneldocs to document functions and data structures when
> possible and add more details on using various API facilities.
> Fix minor issues (typos, grammar, etc.) and add markup.

LGTM, thanks!

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC 04/11] Documentation/hwmon: Revise hwmon kernel API reference

[Bagas Sanjaya]

On Thu, May 04, 2023 at 12:57:45AM -0700, James Seo wrote:
> +========================================
>  The Linux Hardware Monitoring kernel API
>  ========================================
>  
>  Guenter Roeck

I forget to mention this.

Elsewhere in the doc tree (Documentation/), metadata is usually marked
up like:

---- >8 ----
diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
index cf084e04052228..541835f7826352 100644
--- a/Documentation/hwmon/hwmon-kernel-api.rst
+++ b/Documentation/hwmon/hwmon-kernel-api.rst
@@ -2,7 +2,7 @@
 The Linux Hardware Monitoring kernel API
 ========================================
 
-Guenter Roeck
+:Author: Guenter Roeck
 
 .. contents::
 
Thanks.

-- 
An old man doll... just what I always wanted! - Clara

Re: [RFC 05/11] Documentation/hwmon: Revise PMBus core documentation

[Bagas Sanjaya]

On Thu, May 04, 2023 at 12:57:46AM -0700, James Seo wrote:
> -[from pmbus.org] The Power Management Bus (PMBus) is an open standard
> -power-management protocol with a fully defined command language that facilitates
> -communication with power converters and other devices in a power system. The
> -protocol is implemented over the industry-standard SMBus serial interface and
> -enables programming, control, and real-time monitoring of compliant power
> -conversion products. This flexible and highly versatile standard allows for
> -communication between devices based on both analog and digital technologies, and
> -provides true interoperability which will reduce design complexity and shorten
> -time to market for power system designers. Pioneered by leading power supply and
> -semiconductor companies, this open power system standard is maintained and
> -promoted by the PMBus Implementers Forum (PMBus-IF), comprising 30+ adopters
> -with the objective to provide support to, and facilitate adoption among, users.
> +  The Power Management Bus (PMBus) is an open standard power-management protocol
> +  with a fully defined command language that facilitates communication with
> +  power converters and other devices in a power system. The protocol is
> +  implemented over the industry-standard SMBus serial interface and enables
> +  programming, control, and real-time monitoring of compliant power conversion
> +  products. This flexible and highly versatile standard allows for communication
> +  between devices based on both analog and digital technologies, and provides
> +  true interoperability which will reduce design complexity and shorten time to
> +  market for power system designers. Pioneered by leading power supply and
> +  semiconductor companies, this open power system standard is maintained and
> +  promoted by the PMBus Implementers Forum (PMBus-IF), comprising 30+ adopters
> +  with the objective to provide support to, and facilitate adoption among,
> +  users.
> +
> +    (from pmbus.org)

Seems like a candidate for attribution:

---- >8 ----
diff --git a/Documentation/hwmon/pmbus-core.rst b/Documentation/hwmon/pmbus-core.rst
index 2f88a03446d2be..75404f088fd777 100644
--- a/Documentation/hwmon/pmbus-core.rst
+++ b/Documentation/hwmon/pmbus-core.rst
@@ -21,7 +21,7 @@ Introduction
   with the objective to provide support to, and facilitate adoption among,
   users.
 
-    (from pmbus.org)
+  -- (from pmbus.org)
 
 Unfortunately, while PMBus commands are standardized, there are no mandatory
 commands, and manufacturers can add as many non-standard commands as they like.

Otherwise the formatting LGTM, thanks!

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

Re: [RFC 05/11] Documentation/hwmon: Revise PMBus core documentation

[Guenter Roeck]

On 5/4/23 21:27, Bagas Sanjaya wrote:
> On Thu, May 04, 2023 at 12:57:46AM -0700, James Seo wrote:
>> -[from pmbus.org] The Power Management Bus (PMBus) is an open standard
>> -power-management protocol with a fully defined command language that facilitates
>> -communication with power converters and other devices in a power system. The
>> -protocol is implemented over the industry-standard SMBus serial interface and
>> -enables programming, control, and real-time monitoring of compliant power
>> -conversion products. This flexible and highly versatile standard allows for
>> -communication between devices based on both analog and digital technologies, and
>> -provides true interoperability which will reduce design complexity and shorten
>> -time to market for power system designers. Pioneered by leading power supply and
>> -semiconductor companies, this open power system standard is maintained and
>> -promoted by the PMBus Implementers Forum (PMBus-IF), comprising 30+ adopters
>> -with the objective to provide support to, and facilitate adoption among, users.
>> +  The Power Management Bus (PMBus) is an open standard power-management protocol
>> +  with a fully defined command language that facilitates communication with
>> +  power converters and other devices in a power system. The protocol is
>> +  implemented over the industry-standard SMBus serial interface and enables
>> +  programming, control, and real-time monitoring of compliant power conversion
>> +  products. This flexible and highly versatile standard allows for communication
>> +  between devices based on both analog and digital technologies, and provides
>> +  true interoperability which will reduce design complexity and shorten time to
>> +  market for power system designers. Pioneered by leading power supply and
>> +  semiconductor companies, this open power system standard is maintained and
>> +  promoted by the PMBus Implementers Forum (PMBus-IF), comprising 30+ adopters
>> +  with the objective to provide support to, and facilitate adoption among,
>> +  users.
>> +
>> +    (from pmbus.org)
> 
> Seems like a candidate for attribution:
> 

Yes, and I do not feel comfortable removing it.

Guenter

> ---- >8 ----
> diff --git a/Documentation/hwmon/pmbus-core.rst b/Documentation/hwmon/pmbus-core.rst
> index 2f88a03446d2be..75404f088fd777 100644
> --- a/Documentation/hwmon/pmbus-core.rst
> +++ b/Documentation/hwmon/pmbus-core.rst
> @@ -21,7 +21,7 @@ Introduction
>     with the objective to provide support to, and facilitate adoption among,
>     users.
>   
> -    (from pmbus.org)
> +  -- (from pmbus.org)
>   
>   Unfortunately, while PMBus commands are standardized, there are no mandatory
>   commands, and manufacturers can add as many non-standard commands as they like.
> 
> Otherwise the formatting LGTM, thanks!
> 
> Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> 

Re: [RFC 06/11] Documentation/hwmon: Revise patch submission checklist

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 2293 bytes --]

On Thu, May 04, 2023 at 12:57:47AM -0700, James Seo wrote:
> -* Only the following I2C addresses shall be probed: 0x18-0x1f, 0x28-0x2f,
> -  0x48-0x4f, 0x58, 0x5c, 0x73 and 0x77. Probing other addresses is strongly
> -  discouraged as it is known to cause trouble with other (non-hwmon) I2C
> -  chips. If your chip lives at an address which can't be probed then the
> -  device will have to be instantiated explicitly (which is always better
> -  anyway.)
> +* Only the following I2C addresses shall be probed: ``0x18``-``0x1f``,
> +  ``0x28``-``0x2f``, ``0x48``-``0x4f``, ``0x58``, ``0x5c``, ``0x73``,
> +  and ``0x77``. Probing other addresses is strongly discouraged, as it is
> +  known to cause trouble with other (non-``hwmon``) I2C chips. If your chip
> +  lives at an address which can't be probed, then the device will have to be
> +  instantiated explicitly (which is always better anyway).

IMO hwmon here refers to the subsystem (not code identifier name), so
inlining here isn't needed.

>  * Make sure there are no race conditions in the probe function. Specifically,
>    completely initialize your chip and your driver first, then register with
> -  the hwmon subsystem.
> +  the ``hwmon`` subsystem.

Also here.

>  
> -* Use devm_hwmon_device_register_with_info() or, if your driver needs a remove
> -  function, hwmon_device_register_with_info() to register your driver with the
> -  hwmon subsystem. Try using devm_add_action() instead of a remove function if
> -  possible. Do not use any of the deprecated registration functions.
> +* Use
> +  :ref:`devm_hwmon_device_register_with_info() <devm_hwmon_device_register_with_info>`
> +  or, if your driver needs a remove function,
> +  :ref:`hwmon_device_register_with_info() <hwmon_device_register_with_info>` to
> +  register your driver with the ``hwmon`` subsystem. Try using devm_add_action()
> +  instead of a remove function if possible. Do not use any of the deprecated
> +  registration functions.

And here.

> -* Do not provide support for deprecated sysfs attributes.
> +* Do not provide support for deprecated ``sysfs`` attributes.

Dunno if sysfs (as subsystem name) also needs to be inlined.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC 07/11] Documentation/hwmon: Revise sysfs interface specification

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

On Thu, May 04, 2023 at 12:57:48AM -0700, James Seo wrote:
> +**RO**    Read-only value.
> +**WO**    Write-only value.
> +**RW**    Read/write value.

Why are the permissions be emphasized?

> +| | ``pwm[1-*]_auto_point[1-*]_pwm``        | **RW** | PWM temperature curve definition    |
> +| | ``pwm[1-*]_auto_point[1-*]_temp``       |        | for chips that associate trip       |
> +| | ``pwm[1-*]_auto_point[1-*]_temp_hyst``  |        | points with PWM output channels.    |
> ++-------------------------------------------+--------+-------------------------------------+
> +| | ``temp[1-*]_auto_point[1-*]_pwm``       | **RW** | PWM temperature curve definition    |
> +| | ``temp[1-*]_auto_point[1-*]_temp``      |        | for chips that associate trip       |
> +| | ``temp[1-*]_auto_point[1-*]_temp_hyst`` |        | points with temperature channels.   |

Dunno if replacing line blocks with bullet lists should improve the semantic
of three sysfs files having the same description.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC 01/11] Documentation/hwmon: Move misplaced entry in hwmon docs index

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 350 bytes --]

On Thu, May 04, 2023 at 12:57:42AM -0700, James Seo wrote:
> Move the entry for the inspur-ipsps1 driver so that it no
> longer appears in the hwmon docs TOC as a document relating
> to the hwmon subsystem itself.

LGTM, thanks!

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC 08/11] Documentation/hwmon: Revise userspace tools documentation

[Bagas Sanjaya]

On Thu, May 04, 2023 at 12:57:49AM -0700, James Seo wrote:
> +If needed, sources may be found at https://hwmon.wiki.kernel.org/lm_sensors.
> +Basic compilation, installation, and uninstallation may be accomplished with
> +``make all``, ``make install``, and ``make uninstall``, respectively.
> +

The sources is on GitHub:

---- >8 ----
diff --git a/Documentation/hwmon/userspace-tools.rst b/Documentation/hwmon/userspace-tools.rst
index 79c932954e4af5..fd96ea92f72eea 100644
--- a/Documentation/hwmon/userspace-tools.rst
+++ b/Documentation/hwmon/userspace-tools.rst
@@ -44,9 +44,9 @@ Most Linux distributions provide the ``lm-sensors`` suite as a package.
 It is recommended that you use this package for ease of installation.
 Please consult your Linux distribution's documentation for more information.
 
-If needed, sources may be found at https://hwmon.wiki.kernel.org/lm_sensors.
-Basic compilation, installation, and uninstallation may be accomplished with
-``make all``, ``make install``, and ``make uninstall``, respectively.
+If the distribution package isn't available, sources can be obtained at
+`GitHub <https://github.com/lm-sensors/lm-sensors>`_.
+See ``INSTALL`` in the sources for how to build the suite.
 
 Usage
 -----

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

Re: [RFC 02/11] hwmon: (core) Rename last parameter of devm_hwmon_register_with_info()

[James Seo]

On Thu, May 04, 2023 at 08:29:57AM -0700, Guenter Roeck wrote:
> Please please please no such changes. I don't want to have to deal with
> patch wars just because people believe variables should have other names.
> 
> Such changes add zero value unless one counts wasted review time as a "value".
> 
> Guenter
>

Hello,

Of course arbitrarily changing variable names is pointless. But this
patch fulfills the intent of 848ba0a2f20dc121a3ef5272a24641d2bd963d8b,
which makes this change for devm_hwmon_device_register_with_info() in
hwmon-kernel-api.txt and in hwmon.h - but not in hwmon.c. The same
commit makes the same change for hwmon_device_register_with_info() in
all three files, so it obviously should have been in tree already.

The other reason for this patch is that for the purpose of generating
function documentation from kerneldocs, it is not feasible to call
this parameter "extra_groups" in the kerneldoc and still call it
"groups" in the function itself. Doing so results in the lines
"const struct attribute_group **groups / undescribed" and no mention
of "extra_groups" in the generated document. Leaving things as is, so
that [devm_]hwmon_device_register_with_info() have different names
for this parameter, is potentially confusing and more noticeable to
to the eye than I would like once rendered.

Is this good enough to proceed? And as a subsystem maintainer, is
there anything else, specifically or in general, that you would like
to see addressed?

James

Re: [RFC 04/11] Documentation/hwmon: Revise hwmon kernel API reference

[James Seo]

On Fri, May 05, 2023 at 11:20:49AM +0700, Bagas Sanjaya wrote:
> 
> I forget to mention this.
> 
> Elsewhere in the doc tree (Documentation/), metadata is usually marked
> up like:
> 
> ---- >8 ----
> diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
> index cf084e04052228..541835f7826352 100644
> --- a/Documentation/hwmon/hwmon-kernel-api.rst
> +++ b/Documentation/hwmon/hwmon-kernel-api.rst
> @@ -2,7 +2,7 @@
>  The Linux Hardware Monitoring kernel API
>  ========================================
>  
> -Guenter Roeck
> +:Author: Guenter Roeck
>  
>  .. contents::
>  
> Thanks.
> 
> -- 
> An old man doll... just what I always wanted! - Clara
Hi,

Thanks for reviewing.

I will add the :Author: tag to this document.

James

Re: [RFC 02/11] hwmon: (core) Rename last parameter of devm_hwmon_register_with_info()

[Guenter Roeck]

On 5/5/23 06:15, James Seo wrote:
> On Thu, May 04, 2023 at 08:29:57AM -0700, Guenter Roeck wrote:
>> Please please please no such changes. I don't want to have to deal with
>> patch wars just because people believe variables should have other names.
>>
>> Such changes add zero value unless one counts wasted review time as a "value".
>>
>> Guenter
>>
> 
> Hello,
> 
> Of course arbitrarily changing variable names is pointless. But this
> patch fulfills the intent of 848ba0a2f20dc121a3ef5272a24641d2bd963d8b,
> which makes this change for devm_hwmon_device_register_with_info() in
> hwmon-kernel-api.txt and in hwmon.h - but not in hwmon.c. The same
> commit makes the same change for hwmon_device_register_with_info() in
> all three files, so it obviously should have been in tree already.
> 
> The other reason for this patch is that for the purpose of generating
> function documentation from kerneldocs, it is not feasible to call
> this parameter "extra_groups" in the kerneldoc and still call it
> "groups" in the function itself. Doing so results in the lines
> "const struct attribute_group **groups / undescribed" and no mention
> of "extra_groups" in the generated document. Leaving things as is, so
> that [devm_]hwmon_device_register_with_info() have different names
> for this parameter, is potentially confusing and more noticeable to
> to the eye than I would like once rendered.
> 
> Is this good enough to proceed? And as a subsystem maintainer, is
> there anything else, specifically or in general, that you would like

Marginally. That should have been explained in more detail in the
description.

> to see addressed?
> 

I don't know. There are changes which seem to be more based on POV
than real improvement (such as the removal of the credit from the
PMBus document). I'll have to verify each and every reference to determine
if the change is appropriate. Also, the changes are substantial.
It will take a lot of time for me to review, and right now I do not have
that time. I have a hard time keeping up with code reviews.

Guenter

Re: [RFC 05/11] Documentation/hwmon: Revise PMBus core documentation

[James Seo]

On Fri, May 05, 2023 at 11:27:57AM +0700, Bagas Sanjaya wrote:
> 
> Seems like a candidate for attribution:
> 
> ---- >8 ----
> diff --git a/Documentation/hwmon/pmbus-core.rst b/Documentation/hwmon/pmbus-core.rst
> index 2f88a03446d2be..75404f088fd777 100644
> --- a/Documentation/hwmon/pmbus-core.rst
> +++ b/Documentation/hwmon/pmbus-core.rst
> @@ -21,7 +21,7 @@ Introduction
>    with the objective to provide support to, and facilitate adoption among,
>    users.
>  
> -    (from pmbus.org)
> +  -- (from pmbus.org)

Will do.

>  
>  Unfortunately, while PMBus commands are standardized, there are no mandatory
>  commands, and manufacturers can add as many non-standard commands as they like.
> 
> Otherwise the formatting LGTM, thanks!
> 
> Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
> 
> -- 
> An old man doll... just what I always wanted! - Clara

Re: [RFC 02/11] hwmon: (core) Rename last parameter of devm_hwmon_register_with_info()

[James Seo]

On Fri, May 05, 2023 at 06:30:53AM -0700, Guenter Roeck wrote:
> On 5/5/23 06:15, James Seo wrote:
>> On Thu, May 04, 2023 at 08:29:57AM -0700, Guenter Roeck wrote:
>> 
>> Hello,
>> 
>> Of course arbitrarily changing variable names is pointless. But this
>> patch fulfills the intent of 848ba0a2f20dc121a3ef5272a24641d2bd963d8b,
>> which makes this change for devm_hwmon_device_register_with_info() in
>> hwmon-kernel-api.txt and in hwmon.h - but not in hwmon.c. The same
>> commit makes the same change for hwmon_device_register_with_info() in
>> all three files, so it obviously should have been in tree already.
>> 
>> The other reason for this patch is that for the purpose of generating
>> function documentation from kerneldocs, it is not feasible to call
>> this parameter "extra_groups" in the kerneldoc and still call it
>> "groups" in the function itself. Doing so results in the lines
>> "const struct attribute_group **groups / undescribed" and no mention
>> of "extra_groups" in the generated document. Leaving things as is, so
>> that [devm_]hwmon_device_register_with_info() have different names
>> for this parameter, is potentially confusing and more noticeable to
>> to the eye than I would like once rendered.
>> 
>> Is this good enough to proceed? And as a subsystem maintainer, is
>> there anything else, specifically or in general, that you would like
> 
> Marginally. That should have been explained in more detail in the
> description.

OK, I will add more detail.

> 
>> to see addressed?
>> 
> 
> I don't know. There are changes which seem to be more based on POV
> than real improvement (such as the removal of the credit from the
> PMBus document). I'll have to verify each and every reference to determine
> if the change is appropriate. Also, the changes are substantial.

Yes, sorry. At some point comparing a local make htmldocs build to
docs.kernel.org became much easier to follow than slogging through
diffs, and some of the markup only makes sense once rendered next to
the automatic cross-references that Sphinx adds.

> It will take a lot of time for me to review, and right now I do not have
> that time. I have a hard time keeping up with code reviews.
> 
> Guenter
> 

No rush. Your time is always appreciated. As I have gathered some
feedback from Bagas, I will submit the smaller changes as a PATCH
series in a day or two and an updated RFC series that you can
tackle at your leisure.

James

Re: [RFC 06/11] Documentation/hwmon: Revise patch submission checklist

[James Seo]

On Fri, May 05, 2023 at 11:34:51AM +0700, Bagas Sanjaya wrote:
> On Thu, May 04, 2023 at 12:57:47AM -0700, James Seo wrote:
>> -* Only the following I2C addresses shall be probed: 0x18-0x1f, 0x28-0x2f,
>> -  0x48-0x4f, 0x58, 0x5c, 0x73 and 0x77. Probing other addresses is strongly
>> -  discouraged as it is known to cause trouble with other (non-hwmon) I2C
>> -  chips. If your chip lives at an address which can't be probed then the
>> -  device will have to be instantiated explicitly (which is always better
>> -  anyway.)
>> +* Only the following I2C addresses shall be probed: ``0x18``-``0x1f``,
>> +  ``0x28``-``0x2f``, ``0x48``-``0x4f``, ``0x58``, ``0x5c``, ``0x73``,
>> +  and ``0x77``. Probing other addresses is strongly discouraged, as it is
>> +  known to cause trouble with other (non-``hwmon``) I2C chips. If your chip
>> +  lives at an address which can't be probed, then the device will have to be
>> +  instantiated explicitly (which is always better anyway).
> 
> IMO hwmon here refers to the subsystem (not code identifier name), so
> inlining here isn't needed.
> 
>>  * Make sure there are no race conditions in the probe function. Specifically,
>>    completely initialize your chip and your driver first, then register with
>> -  the hwmon subsystem.
>> +  the ``hwmon`` subsystem.
> 
> Also here.
> 
>>  
>> -* Use devm_hwmon_device_register_with_info() or, if your driver needs a remove
>> -  function, hwmon_device_register_with_info() to register your driver with the
>> -  hwmon subsystem. Try using devm_add_action() instead of a remove function if
>> -  possible. Do not use any of the deprecated registration functions.
>> +* Use
>> +  :ref:`devm_hwmon_device_register_with_info() <devm_hwmon_device_register_with_info>`
>> +  or, if your driver needs a remove function,
>> +  :ref:`hwmon_device_register_with_info() <hwmon_device_register_with_info>` to
>> +  register your driver with the ``hwmon`` subsystem. Try using devm_add_action()
>> +  instead of a remove function if possible. Do not use any of the deprecated
>> +  registration functions.
> 
> And here.
> 
>> -* Do not provide support for deprecated sysfs attributes.
>> +* Do not provide support for deprecated ``sysfs`` attributes.
> 
> Dunno if sysfs (as subsystem name) also needs to be inlined.
> 
> Thanks.
> 
> -- 
> An old man doll... just what I always wanted! - Clara

Good point. I will remove inline references to hwmon and sysfs.

Re: [RFC 07/11] Documentation/hwmon: Revise sysfs interface specification

[James Seo]

On Fri, May 05, 2023 at 11:51:03AM +0700, Bagas Sanjaya wrote:
> On Thu, May 04, 2023 at 12:57:48AM -0700, James Seo wrote:
>> +**RO**    Read-only value.
>> +**WO**    Write-only value.
>> +**RW**    Read/write value.
> 
> Why are the permissions be emphasized?

I don't remember, to be honest. I will deemphasize permissions.

> 
>> +| | ``pwm[1-*]_auto_point[1-*]_pwm``        | **RW** | PWM temperature curve definition    |
>> +| | ``pwm[1-*]_auto_point[1-*]_temp``       |        | for chips that associate trip       |
>> +| | ``pwm[1-*]_auto_point[1-*]_temp_hyst``  |        | points with PWM output channels.    |
>> ++-------------------------------------------+--------+-------------------------------------+
>> +| | ``temp[1-*]_auto_point[1-*]_pwm``       | **RW** | PWM temperature curve definition    |
>> +| | ``temp[1-*]_auto_point[1-*]_temp``      |        | for chips that associate trip       |
>> +| | ``temp[1-*]_auto_point[1-*]_temp_hyst`` |        | points with temperature channels.   |
> 
> Dunno if replacing line blocks with bullet lists should improve the semantic
> of three sysfs files having the same description.
> 
> Thanks.
> 
> -- 
> An old man doll... just what I always wanted! - Clara

These are line blocks, not bullet lists. They ensure that each
attribute name is on its own line, avoiding strange wrapping like:

  pwm[1-*]_auto_point[1-*]_pwm pwm[1-*]_auto
  _point[1-*]_temp pwm[1-*]_auto_point[1-*]_temp_hyst

Re: [RFC 07/11] Documentation/hwmon: Revise sysfs interface specification

[Jonathan Corbet]

James Seo <james@equiv.tech> writes:

> On Fri, May 05, 2023 at 11:51:03AM +0700, Bagas Sanjaya wrote:
>> On Thu, May 04, 2023 at 12:57:48AM -0700, James Seo wrote:
>>> +**RO**    Read-only value.
>>> +**WO**    Write-only value.
>>> +**RW**    Read/write value.
>> 
>> Why are the permissions be emphasized?
>
> I don't remember, to be honest. I will deemphasize permissions.

Just to say ... you are interacting with a personality known for driving
contributors into the ground with nit-picky comments.  We do lean toward
minimal markup, but this is not an example that really needed to be
called out.  Do whatever seems best to you here.

Thanks,

jon

Re: [RFC 08/11] Documentation/hwmon: Revise userspace tools documentation

[James Seo]

On Fri, May 05, 2023 at 12:03:29PM +0700, Bagas Sanjaya wrote:
> On Thu, May 04, 2023 at 12:57:49AM -0700, James Seo wrote:
> > +If needed, sources may be found at https://hwmon.wiki.kernel.org/lm_sensors.
> > +Basic compilation, installation, and uninstallation may be accomplished with
> > +``make all``, ``make install``, and ``make uninstall``, respectively.
> > +
> 
> The sources is on GitHub:
> 
> ---- >8 ----
> diff --git a/Documentation/hwmon/userspace-tools.rst b/Documentation/hwmon/userspace-tools.rst
> index 79c932954e4af5..fd96ea92f72eea 100644
> --- a/Documentation/hwmon/userspace-tools.rst
> +++ b/Documentation/hwmon/userspace-tools.rst
> @@ -44,9 +44,9 @@ Most Linux distributions provide the ``lm-sensors`` suite as a package.
>  It is recommended that you use this package for ease of installation.
>  Please consult your Linux distribution's documentation for more information.
>  
> -If needed, sources may be found at https://hwmon.wiki.kernel.org/lm_sensors.
> -Basic compilation, installation, and uninstallation may be accomplished with
> -``make all``, ``make install``, and ``make uninstall``, respectively.
> +If the distribution package isn't available, sources can be obtained at
> +`GitHub <https://github.com/lm-sensors/lm-sensors>`_.
> +See ``INSTALL`` in the sources for how to build the suite.
>  
>  Usage
>  -----
> 
> Thanks.
> 
> -- 
> An old man doll... just what I always wanted! - Clara

The document previously mentioned the old official lm-sensors site,
so I thought I should mention the current one, which has a GitHub link
anyway. But I will change this and add a link to the current site
separately.

Thanks.

James

Re: [RFC 08/11] Documentation/hwmon: Revise userspace tools documentation

[Guenter Roeck]

On 5/5/23 08:12, James Seo wrote:
> On Fri, May 05, 2023 at 12:03:29PM +0700, Bagas Sanjaya wrote:
>> On Thu, May 04, 2023 at 12:57:49AM -0700, James Seo wrote:
>>> +If needed, sources may be found at https://hwmon.wiki.kernel.org/lm_sensors.
>>> +Basic compilation, installation, and uninstallation may be accomplished with
>>> +``make all``, ``make install``, and ``make uninstall``, respectively.
>>> +
>>
>> The sources is on GitHub:
>>

Above page has a "Download" link which points to the github repository.
Personally I think it is better to have a reference to a more generic
location such as to kernel.org in the kenrel and point to the
actual repository (which is more likely to change, after all)
from there.

Guenter

>> ---- >8 ----
>> diff --git a/Documentation/hwmon/userspace-tools.rst b/Documentation/hwmon/userspace-tools.rst
>> index 79c932954e4af5..fd96ea92f72eea 100644
>> --- a/Documentation/hwmon/userspace-tools.rst
>> +++ b/Documentation/hwmon/userspace-tools.rst
>> @@ -44,9 +44,9 @@ Most Linux distributions provide the ``lm-sensors`` suite as a package.
>>   It is recommended that you use this package for ease of installation.
>>   Please consult your Linux distribution's documentation for more information.
>>   
>> -If needed, sources may be found at https://hwmon.wiki.kernel.org/lm_sensors.
>> -Basic compilation, installation, and uninstallation may be accomplished with
>> -``make all``, ``make install``, and ``make uninstall``, respectively.
>> +If the distribution package isn't available, sources can be obtained at
>> +`GitHub <https://github.com/lm-sensors/lm-sensors>`_.
>> +See ``INSTALL`` in the sources for how to build the suite.
>>   
>>   Usage
>>   -----
>>
>> Thanks.
>>
>> -- 
>> An old man doll... just what I always wanted! - Clara
> 
> The document previously mentioned the old official lm-sensors site,
> so I thought I should mention the current one, which has a GitHub link
> anyway. But I will change this and add a link to the current site
> separately.
> 
> Thanks.
> 
> James

Re: [RFC 01/11] Documentation/hwmon: Move misplaced entry in hwmon docs index

[Guenter Roeck]

On Thu, May 04, 2023 at 12:57:42AM -0700, James Seo wrote:
> Move the entry for the inspur-ipsps1 driver so that it no
> longer appears in the hwmon docs TOC as a document relating
> to the hwmon subsystem itself.
> 
> Signed-off-by: James Seo <james@equiv.tech>
> Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

Applied to hwmon-next.

Guenter

> ---
>  Documentation/hwmon/index.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index fa1208c62855..03b30a94a9e6 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -9,7 +9,6 @@ Hardware Monitoring
>  
>     hwmon-kernel-api
>     pmbus-core
> -   inspur-ipsps1
>     submitting-patches
>     sysfs-interface
>     userspace-tools
> @@ -85,6 +84,7 @@ Hardware Monitoring Kernel Drivers
>     ina2xx
>     ina238
>     ina3221
> +   inspur-ipsps1
>     intel-m10-bmc-hwmon
>     ir35221
>     ir38064

Re: [RFC 04/11] Documentation/hwmon: Revise hwmon kernel API reference

[Guenter Roeck]

On 5/4/23 00:57, James Seo wrote:
> Reorganize content into coherent sections.
> Use kerneldocs to document functions and data structures when
> possible and add more details on using various API facilities.
> Fix minor issues (typos, grammar, etc.) and add markup.
> 
> Signed-off-by: James Seo <james@equiv.tech>
> ---
>   Documentation/hwmon/hwmon-kernel-api.rst | 740 ++++++++++++-----------
>   1 file changed, 384 insertions(+), 356 deletions(-)
> 
> diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
> index c2d1e0299d8d..cf084e040522 100644
> --- a/Documentation/hwmon/hwmon-kernel-api.rst
> +++ b/Documentation/hwmon/hwmon-kernel-api.rst
> @@ -1,374 +1,402 @@
> +========================================
>   The Linux Hardware Monitoring kernel API
>   ========================================
>   
>   Guenter Roeck
>   
> +.. contents::
> +
>   Introduction
> -------------
> +============
>   
>   This document describes the API that can be used by hardware monitoring
>   drivers that want to use the hardware monitoring framework.
>   
> -This document does not describe what a hardware monitoring (hwmon) Driver or
> -Device is. It also does not describe the API which can be used by user space
> -to communicate with a hardware monitoring device. If you want to know this
> -then please read the following file: Documentation/hwmon/sysfs-interface.rst.
> +This document does not describe what a hardware monitoring (``hwmon``) driver
> +or device is, nor does it describe the API for communicating with a hardware
> +monitoring device from userspace. For more information on these topics,
> +please read Documentation/hwmon/sysfs-interface.rst.
>   

There is a lot of nnecessary churn in your patches, where you reformat entire
paragrahps just to add a few "``". This makes it almost impossible to identify
the actual changes. For me it means that the review burden is so much that I'll
probably never get to actually review those changes.

Please split the changes into subjects, following the "one logical change per
patch" rule, and please refrain from changing entire paragraphs when adding
markups. Also, a reference to the markup rules explaining if and when markups
are warranted would be nice. For example, I have no idea why "hwmon" would
warrant a markup.

Guenter

Re: [RFC 04/11] Documentation/hwmon: Revise hwmon kernel API reference

[James Seo]

On Sat, May 06, 2023 at 06:52:09AM -0700, Guenter Roeck wrote:
> There is a lot of nnecessary churn in your patches, where you reformat entire
> paragrahps just to add a few "``". This makes it almost impossible to identify
> the actual changes. For me it means that the review burden is so much that I'll
> probably never get to actually review those changes.
> 
> Please split the changes into subjects, following the "one logical change per
> patch" rule, and please refrain from changing entire paragraphs when adding
> markups. Also, a reference to the markup rules explaining if and when markups
> are warranted would be nice. For example, I have no idea why "hwmon" would
> warrant a markup.
> 
> Guenter
>

Ack. Thanks for this. It's the sort of feedback I was hoping to receive
by submitting as an RFC series.

James

Re: [RFC 03/11] hwmon: (core) Revise kerneldoc comments

[Russell Haley]

> @@ -888,18 +891,25 @@ hwmon_device_register_with_groups(struct device *dev, const char *name,
>  EXPORT_SYMBOL_GPL(hwmon_device_register_with_groups);
>  
>  /**
> - * hwmon_device_register_with_info - register w/ hwmon
> - * @dev: the parent device (mandatory)
> - * @name: hwmon name attribute (mandatory)
> - * @drvdata: driver data to attach to created device (optional)
> - * @chip: pointer to hwmon chip information (mandatory)
> - * @extra_groups: pointer to list of additional non-standard attribute groups
> - *	(optional)
> + * hwmon_device_register_with_info - Register a hardware monitoring device.
> + * @dev: A pointer to the parent device (mandatory).
> + * @name: The ``hwmon`` device name (mandatory).
> + * @drvdata: A pointer to the private driver data structure.
> + * @chip: A pointer to the chip descriptor (mandatory).
> + * @extra_groups: A pointer to a ``NULL``-terminated list of additional
> + *	non-standard ``sysfs`` attribute groups.

This function has no @groups parameter, only @extra_groups.

>   *
> - * hwmon_device_unregister() must be called when the device is no
> - * longer needed.
> + * Creates the standard ``sysfs`` attributes in the hardware monitoring core,
> + * letting the driver focus on reading from and writing to the chip instead of
> + * having to bother with ``sysfs`` attributes.
>   *
> - * Returns the pointer to the new device.
> + * The result is a newly allocated ``hwmon`` device with standard ``sysfs``
> + * attributes as specified by @name and @groups and additional attributes as

But here a @groups parameter is mentioned.

> + * specified by @extra_groups.
> + *
> + * Return: On success, a pointer to the new ``hwmon`` device.
> + * hwmon_device_unregister() must be called on it when it is no longer needed.
> + * On failure, a &PTR_ERR.
>   */
>  struct device *
>  hwmon_device_register_with_info(struct device *dev, const char *name,