[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220510030504.GA1577548@roeck-us.net>
Date: Mon, 9 May 2022 20:05:04 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Corentin Labbe <clabbe@...libre.com>
Cc: jdelvare@...e.com, linux-hwmon@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] hwmon: acpi_power_meter: convert to
hwmon_device_register_with_info
On Mon, May 09, 2022 at 06:30:10AM +0000, Corentin Labbe wrote:
> Booting lead to a hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info().
> So let's convert the driver to use hwmon_device_register_with_info().
>
This is hardly readable, and results in a checkpatch warning.
Please rephrase.
> Signed-off-by: Corentin Labbe <clabbe@...libre.com>
> ---
> drivers/hwmon/acpi_power_meter.c | 509 +++++++++++++------------------
> 1 file changed, 219 insertions(+), 290 deletions(-)
>
> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
> index d2545a1be9fc..03a144c884aa 100644
> --- a/drivers/hwmon/acpi_power_meter.c
> +++ b/drivers/hwmon/acpi_power_meter.c
The following include files seem unnecessary.
linux/hwmon-sysfs.h
linux/kdev_t.h
linux/sched.h
On the other side,
linux/kobject.h
should be included.
> @@ -23,7 +23,8 @@
> #define ACPI_POWER_METER_DEVICE_NAME "Power Meter"
> #define ACPI_POWER_METER_CLASS "pwr_meter_resource"
>
> -#define NUM_SENSORS 17
> +#define TRIP_MIN 0
> +#define TRIP_MAX 1
Please use
#define<space>DEFINE<tab>value
>
> #define POWER_METER_CAN_MEASURE (1 << 0)
> #define POWER_METER_CAN_TRIP (1 << 1)
> @@ -38,11 +39,6 @@
> #define METER_NOTIFY_CAPPING 0x83
> #define METER_NOTIFY_INTERVAL 0x84
>
> -#define POWER_AVERAGE_NAME "power1_average"
> -#define POWER_CAP_NAME "power1_cap"
> -#define POWER_AVG_INTERVAL_NAME "power1_average_interval"
> -#define POWER_ALARM_NAME "power1_alarm"
> -
> static int cap_in_hardware;
> static bool force_cap_on;
>
> @@ -85,8 +81,6 @@ struct acpi_power_meter_resource {
> u64 avg_interval;
> int sensors_valid;
> unsigned long sensors_last_updated;
> - struct sensor_device_attribute sensors[NUM_SENSORS];
> - int num_sensors;
> s64 trip[2];
> int num_domain_devices;
> struct acpi_device **domain_devices;
Around here is struct sensor_template which is now unused.
Please remove.
> @@ -122,47 +116,32 @@ static int update_avg_interval(struct acpi_power_meter_resource *resource)
> return 0;
> }
>
> -static ssize_t show_avg_interval(struct device *dev,
> - struct device_attribute *devattr,
> - char *buf)
> +static int acpi_power_average_interval_read(struct acpi_power_meter_resource *resource)
> {
> - struct acpi_device *acpi_dev = to_acpi_device(dev);
> - struct acpi_power_meter_resource *resource = acpi_dev->driver_data;
> -
> mutex_lock(&resource->lock);
> update_avg_interval(resource);
> mutex_unlock(&resource->lock);
>
> - return sprintf(buf, "%llu\n", resource->avg_interval);
> + return resource->avg_interval;
> }
>
> -static ssize_t set_avg_interval(struct device *dev,
> - struct device_attribute *devattr,
> - const char *buf, size_t count)
> +static int set_average_interval(struct acpi_power_meter_resource *resource, long val)
> {
> - struct acpi_device *acpi_dev = to_acpi_device(dev);
> - struct acpi_power_meter_resource *resource = acpi_dev->driver_data;
> union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> struct acpi_object_list args = { 1, &arg0 };
> - int res;
> - unsigned long temp;
> unsigned long long data;
> acpi_status status;
>
> - res = kstrtoul(buf, 10, &temp);
> - if (res)
> - return res;
> -
> - if (temp > resource->caps.max_avg_interval ||
> - temp < resource->caps.min_avg_interval)
> + if (val > resource->caps.max_avg_interval ||
> + val < resource->caps.min_avg_interval)
> return -EINVAL;
> - arg0.integer.value = temp;
> + arg0.integer.value = val;
>
> mutex_lock(&resource->lock);
> status = acpi_evaluate_integer(resource->acpi_dev->handle, "_PAI",
> &args, &data);
> if (ACPI_SUCCESS(status))
> - resource->avg_interval = temp;
> + resource->avg_interval = val;
> mutex_unlock(&resource->lock);
>
> if (ACPI_FAILURE(status)) {
> @@ -175,7 +154,7 @@ static ssize_t set_avg_interval(struct device *dev,
> if (data)
> return -EINVAL;
>
> - return count;
> + return 0;
> }
>
> /* Cap functions */
> @@ -196,46 +175,33 @@ static int update_cap(struct acpi_power_meter_resource *resource)
> return 0;
> }
>
> -static ssize_t show_cap(struct device *dev,
> - struct device_attribute *devattr,
> - char *buf)
> +static int acpi_power_cap_read(struct acpi_power_meter_resource *resource,
> + long *val)
> {
> - struct acpi_device *acpi_dev = to_acpi_device(dev);
> - struct acpi_power_meter_resource *resource = acpi_dev->driver_data;
> -
> mutex_lock(&resource->lock);
> update_cap(resource);
> mutex_unlock(&resource->lock);
>
> - return sprintf(buf, "%llu\n", resource->cap * 1000);
> + return resource->cap * 1000;
> }
>
> -static ssize_t set_cap(struct device *dev, struct device_attribute *devattr,
> - const char *buf, size_t count)
> +static int set_cap(struct acpi_power_meter_resource *resource, long val)
> {
> - struct acpi_device *acpi_dev = to_acpi_device(dev);
> - struct acpi_power_meter_resource *resource = acpi_dev->driver_data;
> union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> struct acpi_object_list args = { 1, &arg0 };
> - int res;
> - unsigned long temp;
> unsigned long long data;
> acpi_status status;
>
> - res = kstrtoul(buf, 10, &temp);
> - if (res)
> - return res;
> -
> - temp = DIV_ROUND_CLOSEST(temp, 1000);
> - if (temp > resource->caps.max_cap || temp < resource->caps.min_cap)
> + val = DIV_ROUND_CLOSEST(val, 1000);
> + if (val > resource->caps.max_cap || val < resource->caps.min_cap)
> return -EINVAL;
> - arg0.integer.value = temp;
> + arg0.integer.value = val;
>
> mutex_lock(&resource->lock);
> status = acpi_evaluate_integer(resource->acpi_dev->handle, "_SHL",
> &args, &data);
> if (ACPI_SUCCESS(status))
> - resource->cap = temp;
> + resource->cap = val;
> mutex_unlock(&resource->lock);
>
> if (ACPI_FAILURE(status)) {
> @@ -248,7 +214,7 @@ static ssize_t set_cap(struct device *dev, struct device_attribute *devattr,
> if (data)
> return -EINVAL;
>
> - return count;
> + return 0;
> }
>
> /* Power meter trip points */
> @@ -263,12 +229,12 @@ static int set_acpi_trip(struct acpi_power_meter_resource *resource)
> acpi_status status;
>
> /* Both trip levels must be set */
> - if (resource->trip[0] < 0 || resource->trip[1] < 0)
> + if (resource->trip[TRIP_MIN] < 0 || resource->trip[TRIP_MAX] < 0)
> return 0;
>
> /* This driver stores min, max; ACPI wants max, min. */
> - arg_objs[0].integer.value = resource->trip[1];
> - arg_objs[1].integer.value = resource->trip[0];
> + arg_objs[0].integer.value = resource->trip[TRIP_MAX];
> + arg_objs[1].integer.value = resource->trip[TRIP_MIN];
>
> status = acpi_evaluate_integer(resource->acpi_dev->handle, "_PTP",
> &args, &data);
> @@ -285,30 +251,18 @@ static int set_acpi_trip(struct acpi_power_meter_resource *resource)
> return 0;
> }
>
> -static ssize_t set_trip(struct device *dev, struct device_attribute *devattr,
> - const char *buf, size_t count)
> +static int set_trip(struct acpi_power_meter_resource *resource, long val, int triptype)
> {
> - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> - struct acpi_device *acpi_dev = to_acpi_device(dev);
> - struct acpi_power_meter_resource *resource = acpi_dev->driver_data;
> int res;
> - unsigned long temp;
>
> - res = kstrtoul(buf, 10, &temp);
> - if (res)
> - return res;
> -
> - temp = DIV_ROUND_CLOSEST(temp, 1000);
> + val = DIV_ROUND_CLOSEST(val, 1000);
>
> mutex_lock(&resource->lock);
> - resource->trip[attr->index - 7] = temp;
> + resource->trip[triptype] = val;
> res = set_acpi_trip(resource);
> mutex_unlock(&resource->lock);
>
> - if (res)
> - return res;
> -
> - return count;
> + return res;
> }
>
> /* Power meter */
> @@ -337,33 +291,26 @@ static int update_meter(struct acpi_power_meter_resource *resource)
> return 0;
> }
>
> -static ssize_t show_power(struct device *dev,
> - struct device_attribute *devattr,
> - char *buf)
> +static int acpi_power_power_read(struct acpi_power_meter_resource *resource,
> + long *val)
> {
> - struct acpi_device *acpi_dev = to_acpi_device(dev);
> - struct acpi_power_meter_resource *resource = acpi_dev->driver_data;
> -
> mutex_lock(&resource->lock);
> update_meter(resource);
> mutex_unlock(&resource->lock);
>
> - return sprintf(buf, "%llu\n", resource->power * 1000);
> + *val = resource->power * 1000;
> + return 0;
> }
>
> /* Miscellaneous */
> -static ssize_t show_str(struct device *dev,
> - struct device_attribute *devattr,
> - char *buf)
> +static ssize_t show_str(struct device *dev, int index, char *buf)
> {
> - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> - struct acpi_device *acpi_dev = to_acpi_device(dev);
> - struct acpi_power_meter_resource *resource = acpi_dev->driver_data;
> + struct acpi_power_meter_resource *resource = dev_get_drvdata(dev);
> acpi_string val;
> int ret;
>
> mutex_lock(&resource->lock);
> - switch (attr->index) {
> + switch (index) {
> case 0:
> val = resource->model_number;
> break;
> @@ -375,7 +322,7 @@ static ssize_t show_str(struct device *dev,
> break;
> default:
> WARN(1, "Implementation error: unexpected attribute index %d\n",
> - attr->index);
> + index);
> val = "";
> break;
> }
> @@ -384,141 +331,138 @@ static ssize_t show_str(struct device *dev,
> return ret;
> }
>
> -static ssize_t show_val(struct device *dev,
> - struct device_attribute *devattr,
> - char *buf)
> +static ssize_t power1_is_battery_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> {
> - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> struct acpi_device *acpi_dev = to_acpi_device(dev);
> struct acpi_power_meter_resource *resource = acpi_dev->driver_data;
> - u64 val = 0;
> + int val;
>
> - switch (attr->index) {
> - case 0:
> - val = resource->caps.min_avg_interval;
> + if (resource->caps.flags & POWER_METER_IS_BATTERY)
> + val = 1;
> + else
> + val = 0;
val = !!(resource->caps.flags & POWER_METER_IS_BATTERY);
would avoid the if()
> + return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t power1_model_number_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return show_str(dev, 0, buf);
> +}
> +
> +static ssize_t power1_serial_number_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return show_str(dev, 1, buf);
> +}
> +
> +static ssize_t power1_oem_info_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return show_str(dev, 2, buf);
> +}
> +
> +static int acpi_power_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct acpi_power_meter_resource *resource = dev_get_drvdata(dev);
> +
> + switch (attr) {
> + case hwmon_power_average:
> + return acpi_power_power_read(resource, val);
> + case hwmon_power_average_interval_min:
> + *val = resource->caps.min_avg_interval;
> break;
> - case 1:
> - val = resource->caps.max_avg_interval;
> + case hwmon_power_average_interval_max:
> + *val = resource->caps.max_avg_interval;
> break;
> - case 2:
> - val = resource->caps.min_cap * 1000;
> + case hwmon_power_cap_min:
> + *val = resource->caps.min_cap * 1000;
> break;
> - case 3:
> - val = resource->caps.max_cap * 1000;
> + case hwmon_power_cap_max:
> + *val = resource->caps.max_cap * 1000;
> break;
> - case 4:
> + case hwmon_power_cap:
> + *val = acpi_power_cap_read(resource, val);
> + break;
> + case hwmon_power_cap_hyst:
> if (resource->caps.hysteresis == UNKNOWN_HYSTERESIS)
> - return sprintf(buf, "unknown\n");
> + return -EINVAL;
return -ENODATA;
>
> - val = resource->caps.hysteresis * 1000;
> + *val = resource->caps.hysteresis * 1000;
> break;
> - case 5:
> - if (resource->caps.flags & POWER_METER_IS_BATTERY)
> - val = 1;
> - else
> - val = 0;
> - break;
> - case 6:
> + case hwmon_power_alarm:
> if (resource->power > resource->cap)
> - val = 1;
> + *val = 1;
> else
> - val = 0;
> + *val = 0;
*val = !!(resource->power > resource->cap);
> break;
> - case 7:
> - case 8:
> - if (resource->trip[attr->index - 7] < 0)
> - return sprintf(buf, "unknown\n");
> -
> - val = resource->trip[attr->index - 7] * 1000;
> + case hwmon_power_average_min:
> + if (resource->trip[TRIP_MIN] < 0)
> + return -EINVAL;
return -ENODATA;
> + *val = resource->trip[TRIP_MIN] * 1000;
> + break;
> + case hwmon_power_average_max:
> + if (resource->trip[TRIP_MAX] < 0)
> + return -EINVAL;
return -ENODATA;
> + *val = resource->trip[TRIP_MAX] * 1000;
> + break;
> + case hwmon_power_average_interval:
> + *val = acpi_power_average_interval_read(resource);
> + break;
> + case hwmon_power_accuracy:
> + *val = div_u64(resource->caps.accuracy, 1000);
> break;
> default:
> WARN(1, "Implementation error: unexpected attribute index %d\n",
> - attr->index);
> - break;
> + attr);
> + return -EOPNOTSUPP;
> }
>
> - return sprintf(buf, "%llu\n", val);
> -}
> -
> -static ssize_t show_accuracy(struct device *dev,
> - struct device_attribute *devattr,
> - char *buf)
> -{
> - struct acpi_device *acpi_dev = to_acpi_device(dev);
> - struct acpi_power_meter_resource *resource = acpi_dev->driver_data;
> - unsigned int acc = resource->caps.accuracy;
> -
> - return sprintf(buf, "%u.%u%%\n", acc / 1000, acc % 1000);
> + return 0;
> }
>
> -static ssize_t show_name(struct device *dev,
> - struct device_attribute *devattr,
> - char *buf)
> +static int acpi_power_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> {
> - return sprintf(buf, "%s\n", ACPI_POWER_METER_NAME);
> -}
> -
> -#define RO_SENSOR_TEMPLATE(_label, _show, _index) \
> - { \
> - .label = _label, \
> - .show = _show, \
> - .index = _index, \
> - }
> -
> -#define RW_SENSOR_TEMPLATE(_label, _show, _set, _index) \
> - { \
> - .label = _label, \
> - .show = _show, \
> - .set = _set, \
> - .index = _index, \
> + struct acpi_power_meter_resource *resource = dev_get_drvdata(dev);
> +
> + switch (attr) {
> + case hwmon_power_average_min:
> + return set_trip(resource, TRIP_MIN, val);
> + case hwmon_power_average_max:
> + return set_trip(resource, TRIP_MAX, val);
> + case hwmon_power_cap:
> + if (resource->caps.configurable_cap)
> + return set_cap(resource, val);
> + else
else after return is unnecessary.
> + return -EINVAL;
> + case hwmon_power_average_interval:
> + return set_average_interval(resource, val);
> + default:
> + return -EOPNOTSUPP;
> }
> +}
>
> -/* Sensor descriptions. If you add a sensor, update NUM_SENSORS above! */
> -static struct sensor_template meter_attrs[] = {
> - RO_SENSOR_TEMPLATE(POWER_AVERAGE_NAME, show_power, 0),
> - RO_SENSOR_TEMPLATE("power1_accuracy", show_accuracy, 0),
> - RO_SENSOR_TEMPLATE("power1_average_interval_min", show_val, 0),
> - RO_SENSOR_TEMPLATE("power1_average_interval_max", show_val, 1),
> - RO_SENSOR_TEMPLATE("power1_is_battery", show_val, 5),
> - RW_SENSOR_TEMPLATE(POWER_AVG_INTERVAL_NAME, show_avg_interval,
> - set_avg_interval, 0),
> - {},
> -};
> -
> -static struct sensor_template misc_cap_attrs[] = {
> - RO_SENSOR_TEMPLATE("power1_cap_min", show_val, 2),
> - RO_SENSOR_TEMPLATE("power1_cap_max", show_val, 3),
> - RO_SENSOR_TEMPLATE("power1_cap_hyst", show_val, 4),
> - RO_SENSOR_TEMPLATE(POWER_ALARM_NAME, show_val, 6),
> - {},
> -};
> -
> -static struct sensor_template ro_cap_attrs[] = {
> - RO_SENSOR_TEMPLATE(POWER_CAP_NAME, show_cap, 0),
> - {},
> -};
> -
> -static struct sensor_template rw_cap_attrs[] = {
> - RW_SENSOR_TEMPLATE(POWER_CAP_NAME, show_cap, set_cap, 0),
> - {},
> -};
> -
> -static struct sensor_template trip_attrs[] = {
> - RW_SENSOR_TEMPLATE("power1_average_min", show_val, set_trip, 7),
> - RW_SENSOR_TEMPLATE("power1_average_max", show_val, set_trip, 8),
> - {},
> -};
> -
> -static struct sensor_template misc_attrs[] = {
> - RO_SENSOR_TEMPLATE("name", show_name, 0),
> - RO_SENSOR_TEMPLATE("power1_model_number", show_str, 0),
> - RO_SENSOR_TEMPLATE("power1_oem_info", show_str, 2),
> - RO_SENSOR_TEMPLATE("power1_serial_number", show_str, 1),
> - {},
> +static DEVICE_ATTR_RO(power1_is_battery);
> +static DEVICE_ATTR_RO(power1_model_number);
> +static DEVICE_ATTR_RO(power1_oem_info);
> +static DEVICE_ATTR_RO(power1_serial_number);
> +
> +static struct attribute *acpi_power_attrs[] = {
> + &dev_attr_power1_is_battery.attr,
> + &dev_attr_power1_model_number.attr,
> + &dev_attr_power1_oem_info.attr,
> + &dev_attr_power1_serial_number.attr,
> + NULL
> };
>
> -#undef RO_SENSOR_TEMPLATE
> -#undef RW_SENSOR_TEMPLATE
> +ATTRIBUTE_GROUPS(acpi_power);
>
> /* Read power domain data */
> static void remove_domain_devices(struct acpi_power_meter_resource *resource)
> @@ -621,55 +565,52 @@ static int read_domain_devices(struct acpi_power_meter_resource *resource)
> return res;
> }
>
> -/* Registration and deregistration */
> -static int register_attrs(struct acpi_power_meter_resource *resource,
> - struct sensor_template *attrs)
> -{
> - struct device *dev = &resource->acpi_dev->dev;
> - struct sensor_device_attribute *sensors =
> - &resource->sensors[resource->num_sensors];
> - int res = 0;
> -
> - while (attrs->label) {
> - sensors->dev_attr.attr.name = attrs->label;
> - sensors->dev_attr.attr.mode = 0444;
> - sensors->dev_attr.show = attrs->show;
> - sensors->index = attrs->index;
> -
> - if (attrs->set) {
> - sensors->dev_attr.attr.mode |= 0200;
> - sensors->dev_attr.store = attrs->set;
> - }
> -
> - sysfs_attr_init(&sensors->dev_attr.attr);
> - res = device_create_file(dev, &sensors->dev_attr);
> - if (res) {
> - sensors->dev_attr.attr.name = NULL;
> - goto error;
> - }
> - sensors++;
> - resource->num_sensors++;
> - attrs++;
> - }
> -
> -error:
> - return res;
> -}
> -
> -static void remove_attrs(struct acpi_power_meter_resource *resource)
> +static umode_t acpi_power_is_visible(const void *data,
> + enum hwmon_sensor_types type,
> + u32 attr, int channel)
> {
> - int i;
> + const struct acpi_power_meter_resource *resource = data;
>
> - for (i = 0; i < resource->num_sensors; i++) {
> - if (!resource->sensors[i].dev_attr.attr.name)
> - continue;
> - device_remove_file(&resource->acpi_dev->dev,
> - &resource->sensors[i].dev_attr);
> + switch (attr) {
> + case hwmon_power_average_min:
> + case hwmon_power_average_max:
> + if (resource->caps.flags & POWER_METER_CAN_TRIP)
> + return 0644;
> + break;
> + case hwmon_power_average:
> + case hwmon_power_accuracy:
> + case hwmon_power_average_interval_min:
> + case hwmon_power_average_interval_max:
> + if (resource->caps.flags & POWER_METER_CAN_MEASURE)
> + return 0444;
> + break;
> + case hwmon_power_average_interval:
> + if (resource->caps.flags & POWER_METER_CAN_MEASURE)
> + return 0644;
> + break;
> + case hwmon_power_cap:
> + if (!can_cap_in_hardware())
> + return 0;
> + if (!(resource->caps.flags & POWER_METER_CAN_CAP))
> + return 0;
> + if (resource->caps.configurable_cap)
> + return 0644;
> + return 0444;
> + break;
Drop "break;" after return.
> + case hwmon_power_cap_min:
> + case hwmon_power_cap_max:
> + case hwmon_power_cap_hyst:
> + case hwmon_power_cap_alarm:
> + if (!can_cap_in_hardware())
> + return 0;
> + if (resource->caps.flags & POWER_METER_CAN_CAP)
> + return 0444;
Consistency would be nice: In the above code the condition is negated.
> + break;
> + default:
> + break;
> }
>
> - remove_domain_devices(resource);
> -
> - resource->num_sensors = 0;
> + return 0;
> }
>
> static int setup_attrs(struct acpi_power_meter_resource *resource)
> @@ -680,47 +621,11 @@ static int setup_attrs(struct acpi_power_meter_resource *resource)
> if (res)
> return res;
>
> - if (resource->caps.flags & POWER_METER_CAN_MEASURE) {
> - res = register_attrs(resource, meter_attrs);
> - if (res)
> - goto error;
> + if (resource->caps.flags & POWER_METER_CAN_CAP && !can_cap_in_hardware()) {
> + dev_warn(&resource->acpi_dev->dev,
> + "Ignoring unsafe software power cap!\n");
> }
> -
> - if (resource->caps.flags & POWER_METER_CAN_CAP) {
> - if (!can_cap_in_hardware()) {
> - dev_warn(&resource->acpi_dev->dev,
> - "Ignoring unsafe software power cap!\n");
> - goto skip_unsafe_cap;
> - }
> -
> - if (resource->caps.configurable_cap)
> - res = register_attrs(resource, rw_cap_attrs);
> - else
> - res = register_attrs(resource, ro_cap_attrs);
> -
> - if (res)
> - goto error;
> -
> - res = register_attrs(resource, misc_cap_attrs);
> - if (res)
> - goto error;
> - }
> -
> -skip_unsafe_cap:
> - if (resource->caps.flags & POWER_METER_CAN_TRIP) {
> - res = register_attrs(resource, trip_attrs);
> - if (res)
> - goto error;
> - }
> -
> - res = register_attrs(resource, misc_attrs);
> - if (res)
> - goto error;
> -
> - return res;
> -error:
> - remove_attrs(resource);
> - return res;
> + return 0;
> }
>
> static void free_capabilities(struct acpi_power_meter_resource *resource)
> @@ -795,7 +700,6 @@ static int read_capabilities(struct acpi_power_meter_resource *resource)
> res = -EINVAL;
> goto error;
> }
> -
> *str = kcalloc(element->string.length + 1, sizeof(u8),
> GFP_KERNEL);
> if (!*str) {
> @@ -836,20 +740,20 @@ static void acpi_power_meter_notify(struct acpi_device *device, u32 event)
> if (res)
> break;
>
> - remove_attrs(resource);
> + remove_domain_devices(resource);
> setup_attrs(resource);
> break;
> case METER_NOTIFY_TRIP:
> - sysfs_notify(&device->dev.kobj, NULL, POWER_AVERAGE_NAME);
> + hwmon_notify_event(&device->dev, hwmon_power, hwmon_power_average, 0);
> break;
> case METER_NOTIFY_CAP:
> - sysfs_notify(&device->dev.kobj, NULL, POWER_CAP_NAME);
> + hwmon_notify_event(&device->dev, hwmon_power, hwmon_power_cap, 0);
> break;
> case METER_NOTIFY_INTERVAL:
> - sysfs_notify(&device->dev.kobj, NULL, POWER_AVG_INTERVAL_NAME);
> + hwmon_notify_event(&device->dev, hwmon_power, hwmon_power_average_interval, 0);
> break;
> case METER_NOTIFY_CAPPING:
> - sysfs_notify(&device->dev.kobj, NULL, POWER_ALARM_NAME);
> + hwmon_notify_event(&device->dev, hwmon_power, hwmon_power_alarm, 0);
> dev_info(&device->dev, "Capping in progress.\n");
> break;
> default:
> @@ -861,6 +765,28 @@ static void acpi_power_meter_notify(struct acpi_device *device, u32 event)
> dev_name(&device->dev), event, 0);
> }
>
> +static const struct hwmon_channel_info *acpi_power_info[] = {
> + HWMON_CHANNEL_INFO(power,
> + HWMON_P_AVERAGE | HWMON_P_AVERAGE_INTERVAL |
> + HWMON_P_AVERAGE_MIN | HWMON_P_AVERAGE_MAX |
> + HWMON_P_CAP | HWMON_P_CAP_HYST |
> + HWMON_P_CAP_MIN | HWMON_P_CAP_MAX |
> + HWMON_P_ACCURACY
> + ),
> + NULL,
> +};
> +
> +static const struct hwmon_ops acpi_power_hwmon_ops = {
> + .is_visible = acpi_power_is_visible,
> + .read = acpi_power_read,
> + .write = acpi_power_write,
> +};
> +
> +static const struct hwmon_chip_info acpi_power_chip_info = {
> + .ops = &acpi_power_hwmon_ops,
> + .info = acpi_power_info,
> +};
> +
> static int acpi_power_meter_add(struct acpi_device *device)
> {
> int res;
> @@ -891,7 +817,10 @@ static int acpi_power_meter_add(struct acpi_device *device)
> if (res)
> goto exit_free_capability;
>
> - resource->hwmon_dev = hwmon_device_register(&device->dev);
> + resource->hwmon_dev = hwmon_device_register_with_info(&device->dev,
> + ACPI_POWER_METER_NAME,
> + resource, &acpi_power_chip_info,
> + acpi_power_groups);
> if (IS_ERR(resource->hwmon_dev)) {
> res = PTR_ERR(resource->hwmon_dev);
> goto exit_remove;
> @@ -901,7 +830,7 @@ static int acpi_power_meter_add(struct acpi_device *device)
> goto exit;
>
> exit_remove:
> - remove_attrs(resource);
> + remove_domain_devices(resource);
> exit_free_capability:
> free_capabilities(resource);
> exit_free:
> @@ -920,7 +849,7 @@ static int acpi_power_meter_remove(struct acpi_device *device)
> resource = acpi_driver_data(device);
> hwmon_device_unregister(resource->hwmon_dev);
>
> - remove_attrs(resource);
> + remove_domain_devices(resource);
> free_capabilities(resource);
>
> kfree(resource);
Powered by blists - more mailing lists