Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1431319 > unrolled thread

[PATCH 0/5] hwmon: New hwmon registration API

Started byGuenter Roeck <linux@roeck-us.net>
First post2016-06-26 05:30 +0200
Last post2016-07-08 15:50 +0200
Articles 6 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/5] hwmon: New hwmon registration API Guenter Roeck <linux@roeck-us.net> - 2016-06-26 05:30 +0200
    [PATCH 6/7] hwmon: (core) Add fan attribute support to new API Guenter Roeck <linux@roeck-us.net> - 2016-06-26 05:30 +0200
    [PATCH 7/7] hwmon: (core) Document new kernel API Guenter Roeck <linux@roeck-us.net> - 2016-06-26 05:30 +0200
    [PATCH 5/7] hwmon: (core) Add energy and humidity attribute support to new API Guenter Roeck <linux@roeck-us.net> - 2016-06-26 05:30 +0200
    Re: [PATCH 0/5] hwmon: New hwmon registration API Punit Agrawal <punit.agrawal@arm.com> - 2016-07-08 11:40 +0200
      Re: [PATCH 0/5] hwmon: New hwmon registration API Guenter Roeck <linux@roeck-us.net> - 2016-07-08 15:50 +0200

#1431319 — [PATCH 0/5] hwmon: New hwmon registration API

FromGuenter Roeck <linux@roeck-us.net>
Date2016-06-26 05:30 +0200
Subject[PATCH 0/5] hwmon: New hwmon registration API
Message-ID<rO8VH-5TX-3@gated-at.bofh.it>
Up to now, each hwmon driver has to implement its own sysfs attributes.
This requires a lot of template code, and distracts from the driver's
core function to read and write chip registers.

To be able to reduce driver complexity, move sensor attribute handling
and thermal zone registration into the hwmon core. By using the new API,
driver size is typically reduced by 20-50% depending on driver complexity
and the number of sysfs attributes supported.

The first patch of the series introduces the API as well as support
for temperature sensors. Subsequent patches introduce support for
voltage, current, power, energy, humidity, and fan speed sensors.

The series was tested by converting several drivers (lm75, lm90, tmp102,
tmp421, ltc4245) to the new API. Testing was done with with real chips
as well as with the hwmon driver module test code available at
https://github.com/groeck/module-tests.

[toc] | [next] | [standalone]


#1431320 — [PATCH 6/7] hwmon: (core) Add fan attribute support to new API

FromGuenter Roeck <linux@roeck-us.net>
Date2016-06-26 05:30 +0200
Subject[PATCH 6/7] hwmon: (core) Add fan attribute support to new API
Message-ID<rO8VI-5TX-15@gated-at.bofh.it>
In reply to#1431319
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/hwmon.c | 16 ++++++++++++++++
 include/linux/hwmon.h | 27 +++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 363ec5660d0c..ca18a6f4f9bc 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -363,6 +363,20 @@ static const char * const hwmon_humidity_attr_templates[] = {
 	[hwmon_humidity_fault] = "humidity%d_fault",
 };
 
+static const char * const hwmon_fan_attr_templates[] = {
+	[hwmon_fan_input] = "fan%d_input",
+	[hwmon_fan_label] = "fan%d_label",
+	[hwmon_fan_min] = "fan%d_min",
+	[hwmon_fan_max] = "fan%d_max",
+	[hwmon_fan_div] = "fan%d_div",
+	[hwmon_fan_pulses] = "fan%d_pulses",
+	[hwmon_fan_target] = "fan%d_target",
+	[hwmon_fan_alarm] = "fan%d_alarm",
+	[hwmon_fan_min_alarm] = "fan%d_min_alarm",
+	[hwmon_fan_max_alarm] = "fan%d_max_alarm",
+	[hwmon_fan_fault] = "fan%d_fault",
+};
+
 static const char * const *__templates[] = {
 	[hwmon_chip] = hwmon_chip_attr_templates,
 	[hwmon_temp] = hwmon_temp_attr_templates,
@@ -371,6 +385,7 @@ static const char * const *__templates[] = {
 	[hwmon_power] = hwmon_power_attr_templates,
 	[hwmon_energy] = hwmon_energy_attr_templates,
 	[hwmon_humidity] = hwmon_humidity_attr_templates,
+	[hwmon_fan] = hwmon_fan_attr_templates,
 };
 
 static const int __templates_size[] = {
@@ -381,6 +396,7 @@ static const int __templates_size[] = {
 	[hwmon_power] = ARRAY_SIZE(hwmon_power_attr_templates),
 	[hwmon_energy] = ARRAY_SIZE(hwmon_energy_attr_templates),
 	[hwmon_humidity] = ARRAY_SIZE(hwmon_humidity_attr_templates),
+	[hwmon_fan] = ARRAY_SIZE(hwmon_fan_attr_templates),
 };
 
 static int hwmon_num_channel_attrs(const struct hwmon_channel_info *info)
diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
index a0b82027ae07..b2fdaaa7b7ba 100644
--- a/include/linux/hwmon.h
+++ b/include/linux/hwmon.h
@@ -27,6 +27,7 @@ enum hwmon_sensor_types {
 	hwmon_power,
 	hwmon_energy,
 	hwmon_humidity,
+	hwmon_fan,
 };
 
 enum hwmon_chip_attributes {
@@ -245,6 +246,32 @@ enum hwmon_humidity_attributes {
 #define HWMON_H_ALARM			BIT(hwmon_humidity_alarm)
 #define HWMON_H_FAULT			BIT(hwmon_humidity_fault)
 
+enum hwmon_fan_attributes {
+	hwmon_fan_input,
+	hwmon_fan_label,
+	hwmon_fan_min,
+	hwmon_fan_max,
+	hwmon_fan_div,
+	hwmon_fan_pulses,
+	hwmon_fan_target,
+	hwmon_fan_alarm,
+	hwmon_fan_min_alarm,
+	hwmon_fan_max_alarm,
+	hwmon_fan_fault,
+};
+
+#define HWMON_F_INPUT			BIT(hwmon_fan_input)
+#define HWMON_F_LABEL			BIT(hwmon_fan_label)
+#define HWMON_F_MIN			BIT(hwmon_fan_min)
+#define HWMON_F_MAX			BIT(hwmon_fan_max)
+#define HWMON_F_DIV			BIT(hwmon_fan_div)
+#define HWMON_F_PULSES			BIT(hwmon_fan_pulses)
+#define HWMON_F_TARGET			BIT(hwmon_fan_target)
+#define HWMON_F_ALARM			BIT(hwmon_fan_alarm)
+#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)
+
 /**
  * struct hwmon_ops - hwmon device operations
  * @is_visible: Callback to return attribute visibility.
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1431321 — [PATCH 7/7] hwmon: (core) Document new kernel API

FromGuenter Roeck <linux@roeck-us.net>
Date2016-06-26 05:30 +0200
Subject[PATCH 7/7] hwmon: (core) Document new kernel API
Message-ID<rO8VI-5TX-11@gated-at.bofh.it>
In reply to#1431319
Describe the new registration API function as well as the data
structures it requires.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/hwmon/hwmon-kernel-api.txt | 229 ++++++++++++++++++++++++++++++-
 1 file changed, 227 insertions(+), 2 deletions(-)

diff --git a/Documentation/hwmon/hwmon-kernel-api.txt b/Documentation/hwmon/hwmon-kernel-api.txt
index 2ecdbfc85ecf..1a96c36532f2 100644
--- a/Documentation/hwmon/hwmon-kernel-api.txt
+++ b/Documentation/hwmon/hwmon-kernel-api.txt
@@ -34,6 +34,19 @@ devm_hwmon_device_register_with_groups(struct device *dev,
 				       const char *name, void *drvdata,
 				       const struct attribute_group **groups);
 
+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 **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 **groups);
+
 void hwmon_device_unregister(struct device *dev);
 void devm_hwmon_device_unregister(struct device *dev);
 
@@ -60,15 +73,227 @@ devm_hwmon_device_register_with_groups is similar to
 hwmon_device_register_with_groups. However, it is device managed, meaning the
 hwmon device does not have to be removed explicitly by the removal function.
 
+hwmon_device_register_with_info is the most comprehensive and preferred means
+to register 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.
+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.
+
 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 or with hwmon_device_register_with_groups.
+hwmon_device_register, hwmon_device_register_with_groups, or
+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 devm_hwmon_device_register_with_groups.
+the call to devm_hwmon_device_register_with_groups and if the automatic
+(device managed) removal would be too late.
+
+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 **groups
+			Null-terminated list of additional 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 **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);
+};
+
+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 priovided 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 apply to the entire chip.
+  * 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
+
+* 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 *lm75_info[] = {
+	&lm75_chip,
+	&lm75_temp,
+	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,
+};
+
+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.
+
+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), S_IRUGO, or 'S_IRUGO | S_IWUSR'.
+
+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
+--------------------------------
+
+If the hardware monitoring device is registered with
+hwmon_device_register_with_info or devm_hwmon_device_register_with_info,
+it is most likely not necessary to provide sysfs atributes. Only non-standard
+sysfs attributes need to be provided when one of those registration functions
+is used.
 
 The header file linux/hwmon-sysfs.h provides a number of useful macros to
 declare and use hardware monitoring sysfs attributes.
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1431322 — [PATCH 5/7] hwmon: (core) Add energy and humidity attribute support to new API

FromGuenter Roeck <linux@roeck-us.net>
Date2016-06-26 05:30 +0200
Subject[PATCH 5/7] hwmon: (core) Add energy and humidity attribute support to new API
Message-ID<rO8VI-5TX-13@gated-at.bofh.it>
In reply to#1431319
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/hwmon.c | 20 ++++++++++++++++++++
 include/linux/hwmon.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index dff07e8f9509..363ec5660d0c 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -347,12 +347,30 @@ static const char * const hwmon_power_attr_templates[] = {
 	[hwmon_power_crit_alarm] = "power%d_crit_alarm",
 };
 
+static const char * const hwmon_energy_attr_templates[] = {
+	[hwmon_energy_input] = "energy%d_input",
+	[hwmon_energy_label] = "energy%d_label",
+};
+
+static const char * const hwmon_humidity_attr_templates[] = {
+	[hwmon_humidity_input] = "humidity%d_input",
+	[hwmon_humidity_label] = "humidity%d_label",
+	[hwmon_humidity_min] = "humidity%d_min",
+	[hwmon_humidity_min_hyst] = "humidity%d_min_hyst",
+	[hwmon_humidity_max] = "humidity%d_max",
+	[hwmon_humidity_max_hyst] = "humidity%d_max_hyst",
+	[hwmon_humidity_alarm] = "humidity%d_alarm",
+	[hwmon_humidity_fault] = "humidity%d_fault",
+};
+
 static const char * const *__templates[] = {
 	[hwmon_chip] = hwmon_chip_attr_templates,
 	[hwmon_temp] = hwmon_temp_attr_templates,
 	[hwmon_in] = hwmon_in_attr_templates,
 	[hwmon_curr] = hwmon_curr_attr_templates,
 	[hwmon_power] = hwmon_power_attr_templates,
+	[hwmon_energy] = hwmon_energy_attr_templates,
+	[hwmon_humidity] = hwmon_humidity_attr_templates,
 };
 
 static const int __templates_size[] = {
@@ -361,6 +379,8 @@ static const int __templates_size[] = {
 	[hwmon_in] = ARRAY_SIZE(hwmon_in_attr_templates),
 	[hwmon_curr] = ARRAY_SIZE(hwmon_curr_attr_templates),
 	[hwmon_power] = ARRAY_SIZE(hwmon_power_attr_templates),
+	[hwmon_energy] = ARRAY_SIZE(hwmon_energy_attr_templates),
+	[hwmon_humidity] = ARRAY_SIZE(hwmon_humidity_attr_templates),
 };
 
 static int hwmon_num_channel_attrs(const struct hwmon_channel_info *info)
diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
index 4de6e2af29ea..a0b82027ae07 100644
--- a/include/linux/hwmon.h
+++ b/include/linux/hwmon.h
@@ -26,6 +26,7 @@ enum hwmon_sensor_types {
 	hwmon_curr,
 	hwmon_power,
 	hwmon_energy,
+	hwmon_humidity,
 };
 
 enum hwmon_chip_attributes {
@@ -216,6 +217,34 @@ enum hwmon_power_attributes {
 #define HWMON_P_MAX_ALARM		BIT(hwmon_power_max_alarm)
 #define HWMON_P_CRIT_ALARM		BIT(hwmon_power_crit_alarm)
 
+enum hwmon_energy_attributes {
+	hwmon_energy_input,
+	hwmon_energy_label,
+};
+
+#define HWMON_E_INPUT			BIT(hwmon_energy_input)
+#define HWMON_E_LABEL			BIT(hwmon_energy_label)
+
+enum hwmon_humidity_attributes {
+	hwmon_humidity_input,
+	hwmon_humidity_label,
+	hwmon_humidity_min,
+	hwmon_humidity_min_hyst,
+	hwmon_humidity_max,
+	hwmon_humidity_max_hyst,
+	hwmon_humidity_alarm,
+	hwmon_humidity_fault,
+};
+
+#define HWMON_H_INPUT			BIT(hwmon_humidity_input)
+#define HWMON_H_LABEL			BIT(hwmon_humidity_label)
+#define HWMON_H_MIN			BIT(hwmon_humidity_min)
+#define HWMON_H_MIN_HYST		BIT(hwmon_humidity_min_hyst)
+#define HWMON_H_MAX			BIT(hwmon_humidity_max)
+#define HWMON_H_MAX_HYST		BIT(hwmon_humidity_max_hyst)
+#define HWMON_H_ALARM			BIT(hwmon_humidity_alarm)
+#define HWMON_H_FAULT			BIT(hwmon_humidity_fault)
+
 /**
  * struct hwmon_ops - hwmon device operations
  * @is_visible: Callback to return attribute visibility.
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]


#1439231

FromPunit Agrawal <punit.agrawal@arm.com>
Date2016-07-08 11:40 +0200
Message-ID<rSAqn-2Tv-47@gated-at.bofh.it>
In reply to#1431319
Hi Guenter,

Guenter Roeck <linux@roeck-us.net> writes:

> Up to now, each hwmon driver has to implement its own sysfs attributes.
> This requires a lot of template code, and distracts from the driver's
> core function to read and write chip registers.
>
> To be able to reduce driver complexity, move sensor attribute handling
> and thermal zone registration into the hwmon core. By using the new API,
> driver size is typically reduced by 20-50% depending on driver complexity
> and the number of sysfs attributes supported.
>
> The first patch of the series introduces the API as well as support
> for temperature sensors. Subsequent patches introduce support for
> voltage, current, power, energy, humidity, and fan speed sensors.
>
> The series was tested by converting several drivers (lm75, lm90, tmp102,
> tmp421, ltc4245) to the new API. Testing was done with with real chips
> as well as with the hwmon driver module test code available at
> https://github.com/groeck/module-tests.

I like this series - it takes all of the attributes' handling out of the
individual driver code and moving it to hwmon core.

Having attempted a port of scpi-hwmon.c, I think that driver will not
gain a big savings in line count. Though it'll help separate access to
sensors from sysfs related code - which I think is worth the change.

FWIW,

        Acked-by: Punit Agrawal <punit.agrawal@arm.com>

Thanks,
Punit

> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]


#1439458

FromGuenter Roeck <linux@roeck-us.net>
Date2016-07-08 15:50 +0200
Message-ID<rSEkh-5sf-3@gated-at.bofh.it>
In reply to#1439231
Hi Punit,

On 07/08/2016 02:31 AM, Punit Agrawal wrote:
> Hi Guenter,
>
> Guenter Roeck <linux@roeck-us.net> writes:
>
>> Up to now, each hwmon driver has to implement its own sysfs attributes.
>> This requires a lot of template code, and distracts from the driver's
>> core function to read and write chip registers.
>>
>> To be able to reduce driver complexity, move sensor attribute handling
>> and thermal zone registration into the hwmon core. By using the new API,
>> driver size is typically reduced by 20-50% depending on driver complexity
>> and the number of sysfs attributes supported.
>>
>> The first patch of the series introduces the API as well as support
>> for temperature sensors. Subsequent patches introduce support for
>> voltage, current, power, energy, humidity, and fan speed sensors.
>>
>> The series was tested by converting several drivers (lm75, lm90, tmp102,
>> tmp421, ltc4245) to the new API. Testing was done with with real chips
>> as well as with the hwmon driver module test code available at
>> https://github.com/groeck/module-tests.
>
> I like this series - it takes all of the attributes' handling out of the
> individual driver code and moving it to hwmon core.
>
> Having attempted a port of scpi-hwmon.c, I think that driver will not
> gain a big savings in line count. Though it'll help separate access to
> sensors from sysfs related code - which I think is worth the change.
>

 From what I have seen, savings are for the most part on the binary image size.
I have not seen much if any savings in terms of LOC, though that may in part
be because of my coding style.

Here is the result from the conversions I have done so far.

Old:

    text	   data	    bss	    dec	    hex	filename
    5730	   4056	     64	   9850	   267a	drivers/hwmon/lm95245.o
    5941	   4240	     64	  10245	   2805	drivers/hwmon/lm95241.o
    5361	   3584	     64	   9009	   2331	drivers/hwmon/jc42.o
    8609	  10872	     64	  19545	   4c59	drivers/hwmon/max31790.o
    9080	  13232	     64	  22376	   5768	drivers/hwmon/nct7904.o
    6574	   8498	     64	  15136	   3b20	drivers/hwmon/ltc4245.o
    4516	   3464	     64	   8044	   1f6c	drivers/hwmon/tmp421.o
    4223	   2744	     64	   7031	   1b77	drivers/hwmon/tmp102.o
   21757	  13496	     64	  35317	   89f5	drivers/hwmon/lm90.o
    6804	   3240	     64	  10108	   277c	drivers/hwmon/lm75.o

New:

    text	   data	    bss	    dec	    hex	filename
    5166	   1568	    128	   6862	   1ace	drivers/hwmon/lm95245.o
    4334	   1664	     64	   6062	   17ae	drivers/hwmon/lm95241.o
    4579	   1456	     64	   6099	   17d3	drivers/hwmon/jc42.o
    3687	   1312	     64	   5063	   13c7	drivers/hwmon/max31790.o
    3905	   1720	     64	   5689	   1639	drivers/hwmon/nct7904.o
    3989	   1658	     64	   5711	   164f	drivers/hwmon/ltc4245.o
    3557	   1408	     64	   5029	   13a5	drivers/hwmon/tmp421.o
    4037	   2200	     64	   6301	   189d	drivers/hwmon/tmp102.o
   16485	   4288	     64	  20837	   5165	drivers/hwmon/lm90.o
    5762	   2128	     64	   7954	   1f12	drivers/hwmon/lm75.o

This is with x86_64.

The largest savings are in drivers with simple access methods and
a large number of attributes. The scpi driver is somewhat of an
exception since it creates its attribute data structures dynamically;
I would not expect to see much savings in that driver.

> FWIW,
>
>          Acked-by: Punit Agrawal <punit.agrawal@arm.com>
>

Thanks!

Guenter

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web