linux-kernel - Re: [PATCH 2/2] acpi_power_meter: hwmon driver for ACPI 4.0 power meters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 27 Jul 2009 14:45:55 +0800
From:	Zhang Rui <rui.zhang@...el.com>
To:	"Darrick J. Wong" <djwong@...ibm.com>
Cc:	Len Brown <lenb@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	lm-sensors <lm-sensors@...sensors.org>,
	linux-acpi <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH 2/2] acpi_power_meter: hwmon driver for ACPI 4.0 power
	meters

Hi, Darrick,

great job. :)
comments in-line.

On Sat, 2009-07-25 at 08:43 +0800, Darrick J. Wong wrote:
> +/* Handle ACPI event notifications */
> +static void acpi_power_meter_notify(struct acpi_device *device, u32 event)
> +{
> +       struct acpi_power_meter_resource *resource;
> +       int res;
> +
> +       if (!device || !acpi_driver_data(device))
> +               return;
> +
> +       resource = acpi_driver_data(device);
> +
> +       mutex_lock(&resource->lock);
> +       switch (event) {
> +       case METER_NOTIFY_CONFIG:
> +               res = read_capabilities(resource);
> +               if (res)
> +                       break;
> +
> +               remove_attrs(resource);
> +               setup_attrs(resource);
> +               break;
> +       case METER_NOTIFY_TRIP:
> +               update_meter(resource);
> +               break;
> +       case METER_NOTIFY_CAP:
> +               update_cap(resource);
> +               break;
> +       case METER_NOTIFY_INTERVAL:
> +               update_avg_interval(resource);
> +               break;
> +       case METER_NOTIFY_CAPPING:
> +               dev_info(&device->dev, "Capping in progress.\n");
> +               break;
> +       default:
> +               BUG();
> +       }
> +       mutex_unlock(&resource->lock);
> +}

we should at least send a netlink event for an ACPI power meter
notification, shouldn't we?

> +
> +static int acpi_power_meter_add(struct acpi_device *device)
> +{
> +       int res;
> +       struct acpi_power_meter_resource *resource;
> +
> +       if (!device)
> +               return -EINVAL;
> +
> +       resource = kzalloc(sizeof(struct acpi_power_meter_resource),
> +                          GFP_KERNEL);
> +       if (!resource)
> +               return -ENOMEM;
> +
> +       resource->sensors_valid = 0;
> +       resource->acpi_dev = device;
> +       mutex_init(&resource->lock);
> +       strcpy(acpi_device_name(device), ACPI_POWER_METER_DEVICE_NAME);
> +       strcpy(acpi_device_class(device), ACPI_POWER_METER_CLASS);
> +       device->driver_data = resource;
> +
> +       res = read_capabilities(resource);
> +       if (res)
> +               goto exit_free;
> +
> +       resource->trip[0] = resource->trip[1] = -1;
> +
> +       res = setup_attrs(resource);
> +       if (res)
> +               goto exit_free;
> +
> +       resource->hwmon_dev = hwmon_device_register(&device->dev);
> +       if (IS_ERR(resource->hwmon_dev)) {
> +               res = PTR_ERR(resource->hwmon_dev);
> +               goto exit_remove;
> +       }
> +

we should create the hwmon sysfs I/F for the hwmon device,
i.e. add the hwmon attributes
under /sys/devices/LINUXSYS:00/.../ACPI000D:00/hwmon0/
rather than /sys/devices/LINUXSYS:00/.../ACPI000D:00/

> +       res = 0;
> +       goto exit;
> +
> +exit_remove:
> +       remove_attrs(resource);
> +exit_free:
> +       kfree(resource);
> +exit:
> +       return res;
> +}
> +
> +
> +static int __init acpi_power_meter_init(void)
> +{
> +       int result;
> +
	  if (acpi_disable)
		return -ENODEV;

> +       result = acpi_bus_register_driver(&acpi_power_meter_driver);
> +       if (result < 0)
> +               return -ENODEV;
> +
> +       return 0;
> +}
> +
> +static void __exit acpi_power_meter_exit(void)
> +{
> +       acpi_bus_unregister_driver(&acpi_power_meter_driver);
> +}
> +
> +MODULE_AUTHOR("Darrick J. Wong <djwong@...ibm.com>");
> +MODULE_DESCRIPTION("ACPI 4.0 power meter driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(acpi_power_meter_init);
> +module_exit(acpi_power_meter_exit);
> 
plus, _PMD is not supported in this driver, right?
I agree with Yakui that we can create some ACPI device sysfs attributes
besides the hwmon ones. e.g. exporting devices measured by the current
ACPI power meter device to user space.

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/