All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v01 0/3] Power Capping Framework
@ 2013-08-02 18:08 Srinivas Pandruvada
  2013-08-02 18:08 ` [RFC v01 1/3] PowerCap: Documentation Srinivas Pandruvada
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Srinivas Pandruvada @ 2013-08-02 18:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Srinivas Pandruvada

Highlights
As suggested
- Didn't use any raw kobject calls, only using device_xx calls.
- No device_create_file/device_remove_file for attributes, so that
user space can see all attributes when notification is received for
device creation.

Overview
With the evolution of technologies, which enables power monitoring and limiting,
more and more devices are able to constrain their power consumption under certain
limits. There are several use cases for such technologies:
- Power monitoring: Each device can report its power consumption.
- Power Limiting: Setting power limits on the devices allows users to guard against
platform reaching max system power level.
- Maximize performance: While staying below a power limit, it allows devices to
automatically adjust performance to meet demands
- Dynamic control and re-budgeting: If each device can be constrained to some power,
extra power can redistributed to other devices, which needs additional performance.

One such example of technology is RAPL (Running Average Power Limit) mechanism
available in the latest Intel Processors. Intel is slowly adding many devices under
RAPL control. Also there are other technologies available, for power capping various
devices. Soon it is very likely that other vendors are also adding or considering
such implementation.

Power Capping framework is an effort to have a uniform interface available to Linux
drivers, which will enable
- A uniform sys-fs interface for all devices which can offer power capping
- A common API for drivers, which will avoid code duplication and easy
implementation of client drivers.

Once this framework is approved, we will submit a RAPL client driver using this
framework.

Revisions:

v01:
Use device model only to register zones and controllers.

v00:
Presented options


Srinivas Pandruvada (3):
  PowerCap: Documentation
  PowerCap: Add class driver
  PowerCap: Added to drivers build

 Documentation/powercap/PowerCappingFramework.txt | 677 ++++++++++++++++
 drivers/Kconfig                                  |   2 +
 drivers/Makefile                                 |   1 +
 drivers/powercap/Kconfig                         |  16 +
 drivers/powercap/Makefile                        |   5 +
 drivers/powercap/powercap_sys.c                  | 985 +++++++++++++++++++++++
 include/linux/powercap.h                         | 300 +++++++
 7 files changed, 1986 insertions(+)
 create mode 100644 Documentation/powercap/PowerCappingFramework.txt
 create mode 100644 drivers/powercap/Kconfig
 create mode 100644 drivers/powercap/Makefile
 create mode 100644 drivers/powercap/powercap_sys.c
 create mode 100644 include/linux/powercap.h

-- 
1.8.3.1


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

* [RFC v01 1/3] PowerCap: Documentation
  2013-08-02 18:08 [RFC v01 0/3] Power Capping Framework Srinivas Pandruvada
@ 2013-08-02 18:08 ` Srinivas Pandruvada
  2013-08-03  0:10   ` Joe Perches
  2013-08-05 19:09   ` Jonathan Corbet
  2013-08-02 18:08 ` [RFC v01 2/3] PowerCap: Add class driver Srinivas Pandruvada
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 20+ messages in thread
From: Srinivas Pandruvada @ 2013-08-02 18:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Srinivas Pandruvada

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=a, Size: 21965 bytes --]

Added power cap framework documentation. This explains the use of power capping framework,
sys-fs and programming interface.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 Documentation/powercap/PowerCappingFramework.txt | 677 +++++++++++++++++++++++
 1 file changed, 677 insertions(+)
 create mode 100644 Documentation/powercap/PowerCappingFramework.txt

diff --git a/Documentation/powercap/PowerCappingFramework.txt b/Documentation/powercap/PowerCappingFramework.txt
new file mode 100644
index 0000000..74a4563
--- /dev/null
+++ b/Documentation/powercap/PowerCappingFramework.txt
@@ -0,0 +1,677 @@
+Power Capping Framework
+==================================
+
+The Linux Power Capping Framework provides user-space with a common
+API to kernel-mode power-capping drivers.  At the same time,
+it provides the hardware-specific power-capping drivers with
+a uniform API to user-space.
+
+Terminology
+=========================
+The Power Capping framework organizes power capping devices under a tree structure.
+At the root level, each device is under some "controller", which is the enabler
+of technology. For example this can be "RAPL".
+Under each controllers, there are multiple power zones, which can be independently
+monitored and controlled.
+Each power zone can be organized as a tree with parent, children and siblings.
+Each power zone defines attributes to enable power monitoring and constraints.
+
+Example Sysfs interface tree:
+
+/sys/devices/virtual/power_cap
+└── intel-rapl
+    ├── intel-rapl:0
+    │   ├── constraint_0_name
+    │   ├── constraint_0_power_limit_uw
+    │   ├── constraint_0_time_window_us
+    │   ├── constraint_1_name
+    │   ├── constraint_1_power_limit_uw
+    │   ├── constraint_1_time_window_us
+    │   ├── device -> ../../intel-rapl
+    │   ├── energy_uj
+    │   ├── intel-rapl:0:0
+    │   │   ├── constraint_0_name
+    │   │   ├── constraint_0_power_limit_uw
+    │   │   ├── constraint_0_time_window_us
+    │   │   ├── constraint_1_name
+    │   │   ├── constraint_1_power_limit_uw
+    │   │   ├── constraint_1_time_window_us
+    │   │   ├── device -> ../../intel-rapl:0
+    │   │   ├── energy_uj
+    │   │   ├── max_energy_range_uj
+    │   │   ├── name
+    │   │   ├── power
+    │   │   │   ├── async
+    │   │   │   ├── autosuspend_delay_ms
+    │   │   │   ├── control
+    │   │   │   ├── runtime_active_kids
+    │   │   │   ├── runtime_active_time
+    │   │   │   ├── runtime_enabled
+    │   │   │   ├── runtime_status
+    │   │   │   ├── runtime_suspended_time
+    │   │   │   └── runtime_usage
+    │   │   ├── subsystem -> ../../../../../../class/power_cap
+    │   │   ├── type
+    │   │   └── uevent
+    │   ├── intel-rapl:0:1
+    │   │   ├── constraint_0_name
+    │   │   ├── constraint_0_power_limit_uw
+    │   │   ├── constraint_0_time_window_us
+    │   │   ├── constraint_1_name
+    │   │   ├── constraint_1_power_limit_uw
+    │   │   ├── constraint_1_time_window_us
+    │   │   ├── device -> ../../intel-rapl:0
+    │   │   ├── energy_uj
+    │   │   ├── max_energy_range_uj
+    │   │   ├── name
+    │   │   ├── power
+    │   │   │   ├── async
+    │   │   │   ├── autosuspend_delay_ms
+    │   │   │   ├── control
+    │   │   │   ├── runtime_active_kids
+    │   │   │   ├── runtime_active_time
+    │   │   │   ├── runtime_enabled
+    │   │   │   ├── runtime_status
+    │   │   │   ├── runtime_suspended_time
+    │   │   │   └── runtime_usage
+    │   │   ├── subsystem -> ../../../../../../class/power_cap
+    │   │   ├── type
+    │   │   └── uevent
+    │   ├── max_energy_range_uj
+    │   ├── max_power_range_uw
+    │   ├── name
+    │   ├── power
+    │   │   ├── async
+    │   │   ├── autosuspend_delay_ms
+    │   │   ├── control
+    │   │   ├── runtime_active_kids
+    │   │   ├── runtime_active_time
+    │   │   ├── runtime_enabled
+    │   │   ├── runtime_status
+    │   │   ├── runtime_suspended_time
+    │   │   └── runtime_usage
+    │   ├── subsystem -> ../../../../../class/power_cap
+    │   ├── type
+    │   └── uevent
+    ├── intel-rapl:1
+    │   ├── constraint_0_name
+    │   ├── constraint_0_power_limit_uw
+    │   ├── constraint_0_time_window_us
+    │   ├── constraint_1_name
+    │   ├── constraint_1_power_limit_uw
+    │   ├── constraint_1_time_window_us
+    │   ├── device -> ../../intel-rapl
+    │   ├── energy_uj
+    │   ├── intel-rapl:1:0
+    │   │   ├── constraint_0_name
+    │   │   ├── constraint_0_power_limit_uw
+    │   │   ├── constraint_0_time_window_us
+    │   │   ├── constraint_1_name
+    │   │   ├── constraint_1_power_limit_uw
+    │   │   ├── constraint_1_time_window_us
+    │   │   ├── device -> ../../intel-rapl:1
+    │   │   ├── energy_uj
+    │   │   ├── max_energy_range_uj
+    │   │   ├── name
+    │   │   ├── power
+    │   │   │   ├── async
+    │   │   │   ├── autosuspend_delay_ms
+    │   │   │   ├── control
+    │   │   │   ├── runtime_active_kids
+    │   │   │   ├── runtime_active_time
+    │   │   │   ├── runtime_enabled
+    │   │   │   ├── runtime_status
+    │   │   │   ├── runtime_suspended_time
+    │   │   │   └── runtime_usage
+    │   │   ├── subsystem -> ../../../../../../class/power_cap
+    │   │   ├── type
+    │   │   └── uevent
+    │   ├── intel-rapl:1:1
+    │   │   ├── constraint_0_name
+    │   │   ├── constraint_0_power_limit_uw
+    │   │   ├── constraint_0_time_window_us
+    │   │   ├── constraint_1_name
+    │   │   ├── constraint_1_power_limit_uw
+    │   │   ├── constraint_1_time_window_us
+    │   │   ├── device -> ../../intel-rapl:1
+    │   │   ├── energy_uj
+    │   │   ├── max_energy_range_uj
+    │   │   ├── name
+    │   │   ├── power
+    │   │   │   ├── async
+    │   │   │   ├── autosuspend_delay_ms
+    │   │   │   ├── control
+    │   │   │   ├── runtime_active_kids
+    │   │   │   ├── runtime_active_time
+    │   │   │   ├── runtime_enabled
+    │   │   │   ├── runtime_status
+    │   │   │   ├── runtime_suspended_time
+    │   │   │   └── runtime_usage
+    │   │   ├── subsystem -> ../../../../../../class/power_cap
+    │   │   ├── type
+    │   │   └── uevent
+    │   ├── max_energy_range_uj
+    │   ├── max_power_range_uw
+    │   ├── name
+    │   ├── powerintel-rapl:0
+    │   │   ├── async
+    │   │   ├── autosuspend_delay_ms
+    │   │   ├── control
+    │   │   ├── runtime_active_kids
+    │   │   ├── runtime_active_time
+    │   │   ├── runtime_enabled
+    │   │   ├── runtime_status
+    │   │   ├── runtime_suspended_time
+    │   │   └── runtime_usage
+    │   ├── subsystem -> ../../../../../class/power_cap
+    │   ├── type
+    │   └── uevent
+    ├── power
+    │   ├── async
+    │   ├── autosuspend_delay_ms
+    │   ├── control
+    │   ├── runtime_active_kids
+    │   ├── runtime_active_time
+    │   ├── runtime_enabled
+    │   ├── runtime_status
+    │   ├── runtime_suspended_time
+    │   └── runtime_usage
+    ├── subsystem -> ../../../../class/power_cap
+    ├── type
+    └── uevent
+
+
+For example, above powercap sysfs tree represents RAPL(Running Average Power Limit)
+type controls available in the Intel® 64 and IA-32 Processor Architectures. Here
+under controller "intel-rapl" there are two CPU packages (package-0/1), which can
+provide power monitoring and controls (intel-rapl:0 and intel-rapl:1). Each power
+zone has a name.
+For example:
+cat /sys/class/power_cap/intel-rapl/intel-rapl:0/name
+package-0
+ 
+In addition to providing monitoring and control at package level, each package
+is further divided into child power zones (called domains in the RAPL specifications).
+Here zones represent controls for core and dram  parts. These zones can be represented
+as children of package.
+For example:
+cat /sys/class/power_cap/intel-rapl/intel-rapl:0/intel-rapl:0:1/name
+dram
+
+Under RAPL framework there are two constraints, one for
+short term and one for long term, with two different time windows. These can be
+represented as two constraints, with different time windows, power limits and names.
+For example:
+	constraint_0_name
+	constraint_0_power_limit_uw
+	constraint_0_time_window_us
+	constraint_1_name
+	constraint_1_power_limit_uw
+	constraint_1_time_window_us
+
+Power Zone Attributes
+=================================
+Monitoring attributes
+----------------------
+	
+energy_uj (rw): Current energy counter in micro-joules. Write to energy counter
+resets the counter to zero. If the counter can not be reset, then this attribute
+is read-only.
+
+max_energy_range_uj (ro): Range of the above energy counter in micro-joules.
+
+power_uw (rw): Current power counter in micro-watts. Write to this counter
+resets the counter to zero. If the counter can not be reset, then this attribute
+is read-only.
+max_power_range_uw (ro): Range of the above energy counter in micro-watts.
+
+It is possible that some domains can have both power and energy counters and
+ranges, but at least one is mandatory.
+
+Constraints
+----------------
+power_limit_uw (rw): Power limit in micro-watts, which should be applicable for
+the time window specified by "time_window_us".
+time_window_us (rw): Time window in micro seconds.
+name (ro): An optional name of the constraint
+
+In addition each node has an attribute "type", which shows, whether is a controller
+or power zone.
+
+Power Cap Client Driver Interface
+==================================
+The API summary:
+
+Call powercap_allocate_controller to define a controller with a name.
+Call powercap_zone_register for each power zone for this controller.
+power zones can have other power zone as a parent or don't have a
+parent.
+During powercap_zone_register defines number of constraints and callbacks.
+
+To Free a power zone call powercap_zone_unregister.
+To free a controller call powercap_deallocate_controller.
+
+Rest of this document is generated by using kernel-doc on
+powercap.h
+
+struct powercap_zone_constraint_ops - Define constraint callbacks
+
+struct powercap_zone_constraint_ops {
+	int (* set_power_limit_uw) (struct powercap_zone_device *, int, u64);
+	int (* get_power_limit_uw) (struct powercap_zone_device *, int, u64 *);
+	int (* set_time_window_us) (struct powercap_zone_device *, int, u64);
+	int (* get_time_window_us) (struct powercap_zone_device *, int, u64 *);
+	int (* get_max_power_uw) (struct powercap_zone_device *, int, u64 *);
+	int (* get_min_power_uw) (struct powercap_zone_device *, int, u64 *);
+	int (* get_max_time_window_us) (struct powercap_zone_device *, int, u64 *);
+	int (* get_min_time_window_us) (struct powercap_zone_device *, int, u64 *);
+	const char *(* get_name) (struct powercap_zone_device *, int);
+	void (* cleanup) (struct powercap_zone_device *, int);
+};
+
+Members:
+
+set_power_limit_uw
+	Set power limit in micro-watts.
+
+get_power_limit_uw
+	Get power limit in micro-watts.
+
+set_time_window_us
+	Set time window in micro-seconds.
+
+get_time_window_us
+	Get time window in micro-seconds.
+
+get_max_power_uw
+	Get max power allowed in micro-watts.
+
+get_min_power_uw
+	Get min power allowed in micro-watts.
+
+get_max_time_window_us
+	Get max time window allowed in micro-seconds.
+
+get_min_time_window_us
+	Get min time window allowed in micro-seconds.
+
+get_name
+	Get the name of constraint
+
+cleanup
+	Do any clean up before the constraint is freed
+This structure is used to define the constraint callbacks for the client
+drivers. The following callbacks are mandatory and can't be NULL:
+ set_power_limit_uw
+ get_power_limit_uw
+ set_time_window_us
+ get_time_window_us
+ get_name
+
+
+
+
+
+struct powercap_controller - Defines a powercap controller
+
+struct powercap_controller {
+	char name[POWERCAP_CTRL_NAME_LENGTH + 1];
+	struct device device;
+	struct idr idr;
+	void * root_node;
+	struct mutex node_lock;
+	struct list_head ctrl_inst;
+};
+
+Members:
+
+name[POWERCAP_CTRL_NAME_LENGTH + 1]
+	name of controller
+
+device
+	device for this controller
+
+idr
+	idr to have unique id for its child
+
+root_node
+	Root holding power zones for this controller
+
+node_lock
+	mutex for node
+
+ctrl_inst
+	link to the controller list
+
+
+
+Description:
+
+Defines powercap controller instance
+
+
+struct powercap_zone_ops - Define power zone callbacks
+
+struct powercap_zone_ops {
+	int (* get_max_energy_range_uj) (struct powercap_zone_device *, u64 *);
+	int (* get_energy_uj) (struct powercap_zone_device *, u64 *);
+	int (* reset_energy_uj) (struct powercap_zone_device *);
+	int (* get_max_power_range_uw) (struct powercap_zone_device *, u64 *);
+	int (* get_power_uw) (struct powercap_zone_device *, u64 *);
+	int (* reset_power_uw) (struct powercap_zone_device *);
+	void (* cleanup) (struct powercap_zone_device *);
+};
+
+Members:
+
+get_max_energy_range_uj
+	Get maximum range of energy counter in
+				micro-joules.
+
+get_energy_uj
+	Get current energy counter in micro-joules.
+
+reset_energy_uj
+	Reset micro-joules energy counter.
+
+get_max_power_range_uw
+	Get maximum range of power counter in
+				micro-watts.
+
+get_power_uw
+	Get current power counter in micro-watts.
+
+reset_power_uw
+	Reset micro-watts power counter.
+
+cleanup
+	Do any clean up before the zone is freed
+
+
+
+Description:
+
+This structure defines zone callbacks to be implemented by client drivers.
+Client drives can define both energy and power related callbacks. But at
+the least one type (either power or energy) is mandatory.
+
+
+struct powercap_zone_attr - Defines a per zone attribute group
+
+struct powercap_zone_attr {
+	struct attribute * zone_dev_attrs[POWERCAP_ZONE_MAX_ATTRS];
+	int zone_attr_count;
+	struct attribute * const_dev_attrs[POWERCAP_CONSTRAINTS_MAX_ATTRS];
+	int const_attr_count;
+	struct attribute_group dev_zone_attr_group;
+	struct attribute_group dev_const_attr_group;
+	int attr_grp_cnt;
+	const struct attribute_group * dev_attr_groups[POWERCAP_MAX_ATTR_GROUPS + 1];
+};
+
+Members:
+
+zone_dev_attrs[POWERCAP_ZONE_MAX_ATTRS]
+	Device attribute list for power zone.
+
+zone_attr_count
+	Number of power zone attributes.
+
+const_dev_attrs[POWERCAP_CONSTRAINTS_MAX_ATTRS]
+	Constraint attributes.
+
+const_attr_count
+	Number of constraint related attributes
+
+dev_zone_attr_group
+	Attribute group for power zone attributes
+
+dev_const_attr_group
+	Attribute group for constraints
+
+attr_grp_cnt
+	Number of attribute groups
+
+dev_attr_groups[POWERCAP_MAX_ATTR_GROUPS + 1]
+	Used to assign to dev->group
+
+
+
+Description:
+
+Used to add an attribute group unique to a zone based on registry.
+
+
+struct powercap_zone_device - Defines instance of a power cap zone
+
+struct powercap_zone_device {
+	int id;
+	char * zone_dev_name;
+	char name[POWERCAP_ZONE_NAME_LENGTH + 1];
+	void * controller_inst;
+	const struct powercap_zone_ops * ops;
+	struct device device;
+	struct powercap_zone_attr attrs;
+	void * node;
+	int const_id_cnt;
+	struct mutex lock;
+	struct idr idr;
+	struct idr * par_idr;
+	void * drv_data;
+	struct list_head constraint_list;
+};
+
+Members:
+
+id
+	Unique id
+
+zone_dev_name
+	Zone device sysfs node name
+
+name[POWERCAP_ZONE_NAME_LENGTH + 1]
+	Power zone name.
+
+controller_inst
+	Controller instance for this zone
+
+ops
+	Pointer to the zone operation structure.
+
+device
+	Instance of a device.
+
+attrs
+	Attributes associated with this device
+
+node
+	Node pointer to insert to a tree data structure.
+
+const_id_cnt
+	Constraint id count
+
+lock
+	Mutex to protect zone related operations.
+
+idr
+	Instance to an idr entry for children zones.
+
+par_idr
+	To remove reference from the parent idr
+
+drv_data
+	Driver private data
+
+constraint_list
+	Link to the next power zone for this controller.
+
+
+
+Description:
+
+This defines a power zone instance. The fields of this structure are
+private, and should not be used by client drivers.
+
+
+Name:
+
+powercap_set_zone_data - Set private data for a zone
+
+Synopsis:
+
+void powercap_set_zone_data (struct powercap_zone_device * pcd_dev,
+                             void * pdata);
+
+Arguments:
+
+pcd_dev
+	A pointer to the valid zone instance.
+
+pdata
+	A pointer to the user private data.
+
+
+Description:
+
+Allows client drivers to associate some private data to zone instance.
+
+
+Name:
+
+powercap_get_zone_data - Get private data for a zone
+
+Synopsis:
+
+void * powercap_get_zone_data (struct powercap_zone_device * pcd_dev);
+
+Arguments:
+
+pcd_dev
+	A pointer to the valid zone instance.
+
+
+Description:
+
+Allows client drivers to get private data associate with a zone,
+using call to powercap_set_zone_data.
+
+
+Name:
+
+powercap_allocate_controller - Allocates a controller
+
+Synopsis:
+
+struct powercap_controller * powercap_allocate_controller (const char * controller_name);
+
+Arguments:
+
+controller_name
+	The Name of this controller, which will be shown
+			in the sysfs Interface.
+
+
+Description:
+
+Used to create a controller with the power capping class. Here controller
+can represent a type of technology, which can control a range of power zones.
+For example a controller can be RAPL (Running Average Power Limit)
+Intel® 64 and IA-32 Processor Architectures. The name can be any string
+which must be unique, otherwise this function returns NULL.
+On successful allocation, this API returns a pointer to the
+controller instance.
+
+
+Name:
+
+powercap_deallocate_controller - Deallocates a controller
+
+Synopsis:
+
+void powercap_deallocate_controller (struct powercap_controller * instance);
+
+Arguments:
+
+instance
+	A pointer to the valid controller instance.
+
+
+Description:
+
+Used to deallocate a controller with the power capping class. This
+takes only one argument, which is the pointer to the instance returned
+by a call to powercap_allocate_controller.
+When a controller is deallocated, all zones and associated constraints
+are freed.
+
+
+Name:
+
+powercap_zone_register - Register a power zone
+
+Synopsis:
+
+struct powercap_zone_device * powercap_zone_register (struct powercap_controller * controller,
+                                                      const char * name,
+                                                      struct powercap_zone_device * parent,
+                                                      const struct powercap_zone_ops * ops,
+                                                      int no_constraints,
+                                                      struct powercap_zone_constraint_ops * const_ops);
+
+Arguments:
+
+controller
+	A controller instance under which this zone operates.
+
+name
+	A name for this zone.
+
+parent
+	A pointer to the parent power zone instance if any or NULL
+
+ops
+	Pointer to zone operation callback structure.
+
+no_constraints
+	Number of constraints for this zone
+
+const_ops
+	Pointer to constraint callback structure
+
+
+Description:
+
+Used to register a power zone for a controller. Zones are organized in
+a tree like structure in sysfs under a controller.
+A power zone must a register a pointer to a structure representing zone
+callbacks.
+A power zone can have a some other power zone as a parent or it can be
+NULL to appear as a direct descendant of a controller.
+Each power zone can have number of constraints. Constraints appears
+under zones as attributes with unique id.
+
+
+Name:
+
+powercap_zone_unregister - Unregister a zone device
+
+Synopsis:
+
+int powercap_zone_unregister (struct powercap_controller * controller,
+                              struct powercap_zone_device * pcd_dev);
+
+Arguments:
+
+controller
+	A pointer to the valid instance of a controller.
+
+pcd_dev
+	A pointer to the valid zone instance for a controller
+
+
+Description:
+
+Used to unregister a zone device for a controller. Once a zone is
+unregistered then all its children and associated constraints are freed.
+
+
-- 
1.8.3.1


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

* [RFC v01 2/3] PowerCap: Add class driver
  2013-08-02 18:08 [RFC v01 0/3] Power Capping Framework Srinivas Pandruvada
  2013-08-02 18:08 ` [RFC v01 1/3] PowerCap: Documentation Srinivas Pandruvada
@ 2013-08-02 18:08 ` Srinivas Pandruvada
  2013-08-02 22:43   ` Joe Perches
  2013-08-02 18:08 ` [RFC v01 3/3] PowerCap: Added to drivers build Srinivas Pandruvada
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Srinivas Pandruvada @ 2013-08-02 18:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Srinivas Pandruvada

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=a, Size: 40747 bytes --]

Added power cap class driver, which provides an API for client drivers
to use and provide a consistant sys-fs interface to user mode.
For details on API refer to PowerCappingFramework.txt under
Documentation/powercap.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/powercap/Kconfig        |  16 +
 drivers/powercap/Makefile       |   5 +
 drivers/powercap/powercap_sys.c | 985 ++++++++++++++++++++++++++++++++++++++++
 include/linux/powercap.h        | 300 ++++++++++++
 4 files changed, 1306 insertions(+)
 create mode 100644 drivers/powercap/Kconfig
 create mode 100644 drivers/powercap/Makefile
 create mode 100644 drivers/powercap/powercap_sys.c
 create mode 100644 include/linux/powercap.h

diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig
new file mode 100644
index 0000000..f70b7b9
--- /dev/null
+++ b/drivers/powercap/Kconfig
@@ -0,0 +1,16 @@
+#
+# Generic powercap sysfs drivers configuration
+#
+
+menuconfig POWERCAP_SUPPORT
+	tristate "Generic powercap sysfs driver"
+	help
+	  A Power Capping Sysfs driver offers a generic mechanism for
+	  power capping. Usually it's made up of one or more controllers,
+	  power zones and constraints.
+	  If you want this support, you should say Y or M here.
+
+if POWERCAP_SUPPORT
+# Add client driver config here.
+
+endif
diff --git a/drivers/powercap/Makefile b/drivers/powercap/Makefile
new file mode 100644
index 0000000..f2acfed
--- /dev/null
+++ b/drivers/powercap/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for powercap drivers
+#
+
+obj-$(CONFIG_POWERCAP_SUPPORT)	+= powercap_sys.o
diff --git a/drivers/powercap/powercap_sys.c b/drivers/powercap/powercap_sys.c
new file mode 100644
index 0000000..5f038e4
--- /dev/null
+++ b/drivers/powercap/powercap_sys.c
@@ -0,0 +1,985 @@
+/*
+ * powercap sysfs class driver
+ * Copyright (c) 2013, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/powercap.h>
+
+#define POWERCAP_CONSTRAINT_NAME_LEN		20
+#define POWERCAP_ZONE_CONSTRAINT_DEV_NAME_LEN	30
+
+/**
+ * struct powercap_zone_node- Defines a node containing power zones
+ * @next:		Pointer to sibling
+ * @children_count:	Number of children for this node
+ * @child:		Pointer to first child
+ * @parent:		Pointer to the parent
+ * @pcd_dev:		pointer to power zone device in this node
+ *
+ * A power zone node to be part of a tree.
+ */
+struct powercap_zone_node {
+	struct powercap_zone_node *next;
+	int children_count;
+	struct powercap_zone_node *child;
+	struct powercap_zone_node *parent;
+	struct powercap_zone_device *pcd_dev;
+};
+
+/**
+ * struct powercap_zone_constraint_attrs - Define constraint attribute
+ * @name:		Constraint attribute name.
+ * @attr:		Device attribute
+ *
+ * Define each attribute, with a name, based on the constraint id.
+ */
+struct powercap_constraint_attr {
+	char name[POWERCAP_ZONE_CONSTRAINT_DEV_NAME_LEN + 1];
+	struct device_attribute attr;
+};
+
+/**
+ * struct powercap_zone_constraint- Defines instance of a constraint
+ * @id:			Instance Id of this constraint.
+ * @pcd_dev:		Pointer to the power zone for this constraint.
+ * @ops:		Pointer to the constraint callbacks.
+ * @priv_data:		Constaint private data
+ * @list:		Link to other constraints for this power zone.
+ *
+ * This defines a constraint instance.
+ */
+struct powercap_zone_constraint {
+	int id;
+	struct powercap_zone_device *pcd_dev;
+	struct powercap_zone_constraint_ops *ops;
+	struct powercap_constraint_attr power_limit_attr;
+	struct powercap_constraint_attr time_window_attr;
+	struct powercap_constraint_attr max_power_attr;
+	struct powercap_constraint_attr min_power_attr;
+	struct powercap_constraint_attr max_time_window_attr;
+	struct powercap_constraint_attr min_time_window_attr;
+	struct powercap_constraint_attr name_attr;
+	struct list_head list;
+};
+
+/* A list of powercap controllers */
+static LIST_HEAD(powercap_cntrl_list);
+/* Mutex to protect list of powercap controllers */
+static DEFINE_MUTEX(powercap_cntrl_list_lock);
+
+/*
+ * Power Zone attributes: Each power zone registered with this framework
+ * contains two types of attributes:
+ * One with fixed name: E.g. energy_uj
+ * One with variable name: E.g. constraint_0_power_limit_uw
+ * Using two attribute groups, which will be used during device_register.
+ * The fixed name attributes are using static DEVICE_ATTR.
+ * For variable name device_attribute fields are initialized using
+ * sysfs_attr_init and assigning a name (constraint_X_power_limit_uw, here
+ * X can be 0 to max integer)
+ */
+
+/* Power zone ro attributes define */
+#define powercap_attr_ro(_name)		\
+	static DEVICE_ATTR(_name, 0444, show_##_name, NULL)
+
+/* Power zone rw attributes define */
+#define powercap_attr_rw(_name)		\
+	static DEVICE_ATTR(_name, 0644, show_##_name, store_##_name)
+
+/* constraint attributes define rw */
+#define powercap_const_attr_rw(_name)		\
+	static DEVICE_ATTR(_name, 0644, show_constraint_##_name, \
+				store_constraint_##_name)
+/* constraint attributes define ro */
+#define powercap_const_attr_ro(_name)		\
+	static DEVICE_ATTR(_name, 0644, show_constraint_##_name, NULL)
+
+/* Power zone show function */
+#define define_device_show(_attr)		\
+static ssize_t show_##_attr(struct device *dev, struct device_attribute *attr,\
+			char *buf) \
+{ \
+	u64 value; \
+	ssize_t len = -EINVAL; \
+	struct powercap_zone_device *pcd_dev = dev_get_drvdata(dev); \
+	\
+	if (pcd_dev && pcd_dev->ops && pcd_dev->ops->get_##_attr) { \
+		mutex_lock(&pcd_dev->lock); \
+		if (!pcd_dev->ops->get_##_attr(pcd_dev, &value)) \
+			len = sprintf(buf, "%lld\n", value); \
+		mutex_unlock(&pcd_dev->lock); \
+	} \
+	\
+	return len; \
+}
+
+/* Power zone store function; only reset is possible */
+#define define_device_store(_attr)		\
+static ssize_t store_##_attr(struct device *dev,\
+				struct device_attribute *attr, \
+				const char *buf, size_t count) \
+{ \
+	int err; \
+	struct powercap_zone_device *pcd_dev = dev_get_drvdata(dev); \
+	u64 value; \
+	\
+	err = kstrtoull(buf, 10, &value); \
+	if (err) \
+		return -EINVAL; \
+	if (value) \
+		return -EINVAL; \
+	if (pcd_dev && pcd_dev->ops && pcd_dev->ops->reset_##_attr) { \
+		mutex_lock(&pcd_dev->lock); \
+		if (!pcd_dev->ops->reset_##_attr(pcd_dev)) { \
+			mutex_unlock(&pcd_dev->lock); \
+			return count; \
+		} \
+		mutex_unlock(&pcd_dev->lock); \
+	} \
+	\
+	return -EINVAL; \
+}
+
+/* Find constraint pointer from an ID */
+static struct powercap_zone_constraint *find_constraint(
+			struct powercap_zone_device *pcd_dev, int id)
+{
+	struct powercap_zone_constraint *pconst = NULL;
+
+	list_for_each_entry(pconst, &pcd_dev->constraint_list, list) {
+		if (pconst->id == id) {
+			return pconst;
+			break;
+		}
+	}
+
+	return pconst;
+}
+
+/* Power zone constraint show function */
+#define define_device_constraint_show(_attr) \
+static ssize_t show_constraint_##_attr(struct device *dev, \
+				struct device_attribute *attr,\
+				char *buf) \
+{ \
+	u64 value; \
+	ssize_t len = -ENODATA; \
+	struct powercap_zone_device *pcd_dev = dev_get_drvdata(dev); \
+	int id; \
+	struct powercap_zone_constraint *pconst;\
+	\
+	if (!pcd_dev) \
+		return -EINVAL; \
+	if (!sscanf(attr->attr.name, "constraint_%d_", &id)) \
+		return -EINVAL; \
+	mutex_lock(&pcd_dev->lock); \
+	pconst = find_constraint(pcd_dev, id); \
+	if (pconst && pconst->ops && pconst->ops->get_##_attr) { \
+		if (!pconst->ops->get_##_attr(pcd_dev, id, &value)) \
+			len = sprintf(buf, "%lld\n", value); \
+	} \
+	mutex_unlock(&pcd_dev->lock); \
+	\
+	return len; \
+}
+
+/* Power zone constraint store function */
+#define define_device_constraint_store(_attr) \
+static ssize_t store_constraint_##_attr(struct device *dev,\
+				struct device_attribute *attr, \
+				const char *buf, size_t count) \
+{ \
+	int err; \
+	u64 value; \
+	struct powercap_zone_device *pcd_dev = dev_get_drvdata(dev); \
+	int id; \
+	struct powercap_zone_constraint *pconst;\
+	\
+	if (!pcd_dev) \
+		return -EINVAL; \
+	if (!sscanf(attr->attr.name, "constraint_%d_", &id)) \
+		return -EINVAL; \
+	err = kstrtoull(buf, 10, &value); \
+	if (err) \
+		return -EINVAL; \
+	mutex_lock(&pcd_dev->lock); \
+	pconst = find_constraint(pcd_dev, id); \
+	if (pconst && pconst->ops && pconst->ops->set_##_attr) { \
+		if (!pconst->ops->set_##_attr(pcd_dev, id, value)) { \
+			mutex_unlock(&pcd_dev->lock); \
+			return count; \
+		} \
+	} \
+	mutex_unlock(&pcd_dev->lock); \
+	\
+	return -ENODATA; \
+}
+
+/* Power zone information callbacks */
+define_device_show(power_uw);
+define_device_store(power_uw);
+define_device_show(max_power_range_uw);
+define_device_show(energy_uj);
+define_device_store(energy_uj);
+define_device_show(max_energy_range_uj);
+
+/* Power zone attributes */
+powercap_attr_ro(max_power_range_uw);
+powercap_attr_rw(power_uw);
+powercap_attr_ro(max_energy_range_uj);
+powercap_attr_rw(energy_uj);
+
+/* Power zone constraint attributes callbacks */
+define_device_constraint_show(power_limit_uw);
+define_device_constraint_store(power_limit_uw);
+define_device_constraint_show(time_window_us);
+define_device_constraint_store(time_window_us);
+define_device_constraint_show(max_power_uw);
+define_device_constraint_show(min_power_uw);
+define_device_constraint_show(max_time_window_us);
+define_device_constraint_show(min_time_window_us);
+
+static ssize_t show_constraint_name(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	const char *name;
+	struct powercap_zone_device *pcd_dev = dev_get_drvdata(dev);
+	int id;
+	ssize_t len = -ENODATA;
+	struct powercap_zone_constraint *pconst;
+
+	if (!pcd_dev)
+		return -EINVAL;
+	if (!sscanf(attr->attr.name, "constraint_%d_", &id))
+		return -EINVAL;
+	mutex_lock(&pcd_dev->lock);
+	pconst = find_constraint(pcd_dev, id);
+	if (pconst && pconst->ops && pconst->ops->get_name) {
+		name = pconst->ops->get_name(pcd_dev, id);
+		if (name) {
+			snprintf(buf, POWERCAP_CONSTRAINT_NAME_LEN,
+								"%s\n", name);
+			buf[POWERCAP_CONSTRAINT_NAME_LEN] = '\0';
+			len = strlen(buf);
+		}
+	}
+	mutex_unlock(&pcd_dev->lock);
+
+	return len;
+}
+
+static void create_constraint_attribute(struct powercap_zone_constraint *pconst,
+				const char *name,
+				int mode,
+				struct powercap_constraint_attr *attr,
+				ssize_t (*show)(struct device *,
+					struct device_attribute *, char *),
+				ssize_t (*store)(struct device *,
+					struct device_attribute *,
+				const char *, size_t)
+				)
+{
+	snprintf(attr->name, POWERCAP_ZONE_CONSTRAINT_DEV_NAME_LEN,
+					"constraint_%d_%s", pconst->id, name);
+	attr->name[POWERCAP_ZONE_CONSTRAINT_DEV_NAME_LEN] = '\0';
+	sysfs_attr_init(&attr->attr);
+	attr->attr.attr.name = attr->name;
+	attr->attr.attr.mode = mode;
+	attr->attr.show = show;
+	attr->attr.store = store;
+}
+
+/* Create a constraint attribute, if it has required call backs */
+static int create_constraints(struct powercap_zone_constraint *pconst)
+{
+	int count;
+	struct powercap_zone_device *pcd_dev;
+
+	if (!pconst->ops)
+		return -EINVAL;
+	if (!pconst->ops->get_name ||
+			!pconst->ops->set_power_limit_uw ||
+			!pconst->ops->set_power_limit_uw ||
+			!pconst->ops->get_time_window_us ||
+			!pconst->ops->set_time_window_us) {
+		return -EINVAL;
+	}
+	pcd_dev = pconst->pcd_dev;
+	count = pcd_dev->attrs.const_attr_count;
+	if (count >=
+		(POWERCAP_CONSTRAINTS_MAX_ATTRS - POWERCAP_CONSTRAINTS_ATTRS))
+		return -EINVAL;
+	create_constraint_attribute(pconst, "name", S_IRUGO,
+				&pconst->name_attr,
+				show_constraint_name,
+				NULL);
+	pcd_dev->attrs.const_dev_attrs[count++] =
+				&pconst->name_attr.attr.attr;
+
+	create_constraint_attribute(pconst, "power_limit_uw",
+					S_IWUSR | S_IRUGO,
+					&pconst->power_limit_attr,
+					show_constraint_power_limit_uw,
+					store_constraint_power_limit_uw);
+	pcd_dev->attrs.const_dev_attrs[count++] =
+				&pconst->power_limit_attr.attr.attr;
+
+	create_constraint_attribute(pconst, "time_window_us",
+					S_IWUSR | S_IRUGO,
+					&pconst->time_window_attr,
+					show_constraint_time_window_us,
+					store_constraint_time_window_us);
+	pcd_dev->attrs.const_dev_attrs[count++] =
+				&pconst->time_window_attr.attr.attr;
+
+	if (pconst->ops->get_max_power_uw) {
+		create_constraint_attribute(pconst, "max_power_uw", S_IRUGO,
+					&pconst->max_power_attr,
+					show_constraint_max_power_uw,
+					NULL);
+		pcd_dev->attrs.const_dev_attrs[count++] =
+					&pconst->max_power_attr.attr.attr;
+	}
+
+	if (pconst->ops->get_min_power_uw) {
+		create_constraint_attribute(pconst, "min_power_uw", S_IRUGO,
+					&pconst->min_power_attr,
+					show_constraint_min_power_uw,
+					NULL);
+		pcd_dev->attrs.const_dev_attrs[count++] =
+					&pconst->min_power_attr.attr.attr;
+	}
+	if (pconst->ops->get_max_time_window_us) {
+		create_constraint_attribute(pconst, "max_time_window_us",
+					S_IRUGO,
+					&pconst->max_time_window_attr,
+					show_constraint_max_time_window_us,
+					NULL);
+		pcd_dev->attrs.const_dev_attrs[count++] =
+				&pconst->max_time_window_attr.attr.attr;
+	}
+	if (pconst->ops->get_max_time_window_us) {
+		create_constraint_attribute(pconst, "min_time_window_us",
+					S_IRUGO,
+					&pconst->min_time_window_attr,
+					show_constraint_min_time_window_us,
+					NULL);
+		pcd_dev->attrs.const_dev_attrs[count++] =
+				&pconst->min_time_window_attr.attr.attr;
+	}
+
+	pcd_dev->attrs.const_attr_count = count;
+
+	return 0;
+}
+
+struct powercap_zone_constraint *powercap_zone_add_constraint(
+				struct powercap_zone_device *pcd_dev,
+				struct powercap_zone_constraint_ops *ops)
+{
+	struct powercap_zone_constraint *pconst;
+	int result;
+
+	if (!pcd_dev)
+		return ERR_PTR(-EINVAL);
+
+	dev_dbg(&pcd_dev->device, "powercap_zone_add_constraint\n");
+	pconst = kzalloc(sizeof(*pconst), GFP_KERNEL);
+	if (!pconst)
+		return ERR_PTR(-ENOMEM);
+	pconst->pcd_dev = pcd_dev;
+	pconst->ops = ops;
+	mutex_lock(&pcd_dev->lock);
+	pconst->id = pcd_dev->const_id_cnt;
+	result = create_constraints(pconst);
+	if (result) {
+		kfree(pconst);
+		mutex_unlock(&pcd_dev->lock);
+		return ERR_PTR(result);
+	}
+	pcd_dev->const_id_cnt++;
+	list_add_tail(&pconst->list, &pcd_dev->constraint_list);
+	mutex_unlock(&pcd_dev->lock);
+
+	return pconst;
+}
+
+static void delete_constraints(struct powercap_zone_device *pcd_dev)
+{
+	struct powercap_zone_constraint *p, *n;
+
+	/*
+	 * No need to hold locks as this is called either when device
+	 * is not created or from device_release callback
+	 */
+	/* Delete all constraints */
+	list_for_each_entry_safe(p, n, &pcd_dev->constraint_list, list) {
+		list_del(&p->list);
+		kfree(p);
+	}
+}
+
+static int create_constraint_attributes(struct powercap_zone_device *pcd_dev,
+				int nr_constraints,
+				struct powercap_zone_constraint_ops *const_ops)
+{
+	int i;
+	int ret = 0;
+
+	if (pcd_dev->attrs.attr_grp_cnt >= POWERCAP_MAX_ATTR_GROUPS)
+		return -ENOMEM;
+
+	for (i = 0; i < nr_constraints; ++i) {
+		if (!powercap_zone_add_constraint(pcd_dev, const_ops)) {
+			ret = -ENOMEM;
+			break;
+		}
+	}
+	if (ret) {
+		delete_constraints(pcd_dev);
+		return ret;
+	}
+	pcd_dev->attrs.const_dev_attrs[pcd_dev->attrs.const_attr_count] = NULL;
+	pcd_dev->attrs.dev_const_attr_group.attrs =
+						pcd_dev->attrs.const_dev_attrs;
+	pcd_dev->attrs.dev_attr_groups[pcd_dev->attrs.attr_grp_cnt] =
+					&pcd_dev->attrs.dev_const_attr_group;
+
+	pcd_dev->attrs.attr_grp_cnt++;
+
+	return ret;
+}
+
+/* Allocate a node of a tree */
+static struct powercap_zone_node *create_node(
+					struct powercap_zone_device *pcd_dev)
+{
+	struct powercap_zone_node *node;
+
+	node = kzalloc(sizeof(*node), GFP_KERNEL);
+	if (!node)
+		return NULL;
+
+	node->pcd_dev = pcd_dev;
+
+	return node;
+}
+
+/* Insert a node into a tree */
+static void insert_node(struct powercap_controller *ctrl,
+				struct powercap_zone_node *elem,
+				struct powercap_zone_node *parent)
+{
+	struct powercap_zone_node *node;
+
+	mutex_lock(&ctrl->node_lock);
+	if (!ctrl->root_node) {
+		ctrl->root_node = elem;
+		mutex_unlock(&ctrl->node_lock);
+		return;
+	}
+	if (!parent)
+		parent = ctrl->root_node;
+	elem->parent = parent;
+	if (!parent->child)
+		parent->child = elem;
+	else {
+		/* Not a first child */
+		node = parent->child;
+		while (node->next)
+			node = node->next;
+		node->next = elem;
+	}
+	parent->children_count++;
+	mutex_unlock(&ctrl->node_lock);
+}
+
+/* Delete a node, once node is deleted, its children will be deleted */
+static void delete_node(struct powercap_controller *ctrl,
+			struct powercap_zone_node *node,
+			void (*del_callback)(struct powercap_zone_device *))
+{
+	struct powercap_zone_node *node_store;
+	struct powercap_zone_node *node_limit = node;
+	bool root_node_delete = false;
+
+
+	mutex_lock(&ctrl->node_lock);
+
+	if (node == ctrl->root_node)
+		root_node_delete = true;
+
+	while (node) {
+		node_store = node;
+		if (node->child) {
+			node = node->child;
+		} else {
+			/* reached leaf node */
+			struct powercap_zone_node *_tnode;
+			if (node_store->pcd_dev) {
+				dev_dbg(&node_store->pcd_dev->device,
+				"Delete child %s of parent %s\n",
+				node_store->pcd_dev->name,
+				node_store->parent ?
+				node_store->parent->pcd_dev->name : "ROOT");
+			}
+			/* Point node to next sibling */
+			node = node_store->next;
+			if (!node)
+				node = node_store->parent; /* back to root */
+			/*
+			 *Before the leaf is deleted, remove references from
+			 *parent and siblings
+			 */
+			_tnode = node_store->parent;
+			if (_tnode) {
+				_tnode->children_count--;
+				if (_tnode->child == node_store) {
+					/*very first child*/
+					_tnode->child = node_store->next;
+				} else {
+					/*Not a first child*/
+					struct powercap_zone_node *_node =
+								_tnode->child;
+					struct powercap_zone_node *_pnode =
+								_node;
+
+					while (_node != node_store) {
+						_pnode = _node;
+						_node = _node->next;
+					}
+					_pnode->next = node_store->next;
+				}
+			}
+			if (node_store->pcd_dev) {
+				/* Ready to delete */
+				(*del_callback)(node_store->pcd_dev);
+			}
+			if (node_store == node_limit) {
+				kfree(node_store);
+				break;
+			}
+			kfree(node_store); /* Leaf node is freed */
+			/* zone memory is freed in the device_release */
+		}
+	}
+	/*
+	 * If the request was for root node,
+	 * then whole tree is deleted
+	 */
+	if (root_node_delete)
+		ctrl->root_node = NULL;
+
+	mutex_unlock(&ctrl->node_lock);
+}
+
+/* Search a tree for a controller for a zone instance */
+static bool search_node(struct powercap_controller *ctrl,
+			struct powercap_zone_device *pcd_dev)
+{
+	bool valid = true;
+	bool found = false;
+	struct powercap_zone_node *node;
+
+	mutex_lock(&ctrl->node_lock);
+
+	node = ctrl->root_node;
+
+	while (node) {
+		if (valid) {
+			if (node->pcd_dev == pcd_dev) {
+				found = true;
+				break;
+			}
+		}
+		/* First check if is node has children, then siblings */
+		if (node->child && valid) {
+			node = node->child;
+			valid = true;
+		} else if (node->next) {
+			node = node->next;
+			valid = true;
+		} else {
+			/* Reached leaf, go back to parent and traverse */
+			node = node->parent;
+			valid = false;
+		}
+	}
+	mutex_unlock(&ctrl->node_lock);
+
+	return found;
+}
+
+/* Check the presence of a controller in the controller list */
+static bool check_controller_validity(void *controller)
+{
+	struct powercap_controller *pos = NULL;
+	bool found = false;
+
+	mutex_lock(&powercap_cntrl_list_lock);
+
+	list_for_each_entry(pos, &powercap_cntrl_list, ctrl_inst) {
+		if (pos == controller) {
+			found = true;
+			break;
+		}
+	}
+	mutex_unlock(&powercap_cntrl_list_lock);
+
+	return found;
+}
+
+static ssize_t show_name(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct powercap_zone_device *pcd_dev = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", pcd_dev->name);
+}
+
+static DEVICE_ATTR(name, 0444, show_name, NULL);
+
+/* Create zone and attributes in sys-fs */
+static int create_power_zone_attributes(struct powercap_zone_device *pcd_dev)
+{
+
+	int count = 0;
+
+	pcd_dev->attrs.attr_grp_cnt = 0;
+	/**
+	* Limit is POWERCAP_ZONE_MAX_ATTRS = 10, Only adding 5 attrs.
+	* So not checking range after each addition
+	*/
+	pcd_dev->attrs.zone_dev_attrs[count++] = &dev_attr_name.attr;
+	if (pcd_dev->ops->get_max_energy_range_uj)
+		pcd_dev->attrs.zone_dev_attrs[count++] =
+					&dev_attr_max_energy_range_uj.attr;
+	if (pcd_dev->ops->get_energy_uj)
+		pcd_dev->attrs.zone_dev_attrs[count++] =
+					&dev_attr_energy_uj.attr;
+	if (pcd_dev->ops->get_power_uw)
+		pcd_dev->attrs.zone_dev_attrs[count++] =
+					&dev_attr_power_uw.attr;
+	if (pcd_dev->ops->get_max_power_range_uw)
+		pcd_dev->attrs.zone_dev_attrs[count++] =
+					&dev_attr_max_power_range_uw.attr;
+	pcd_dev->attrs.zone_dev_attrs[count] = NULL;
+	pcd_dev->attrs.zone_attr_count = count;
+	pcd_dev->attrs.dev_zone_attr_group.attrs =
+					pcd_dev->attrs.zone_dev_attrs;
+	pcd_dev->attrs.dev_attr_groups[0] =
+					&pcd_dev->attrs.dev_zone_attr_group;
+	pcd_dev->attrs.attr_grp_cnt++;
+
+	return 0;
+}
+
+static void delete_zone(struct powercap_zone_device *pcd_dev)
+{
+	struct powercap_zone_constraint *p;
+
+	dev_dbg(&pcd_dev->device, "deleting %s\n", pcd_dev->name);
+	mutex_lock(&pcd_dev->lock);
+
+	list_for_each_entry(p, &pcd_dev->constraint_list, list) {
+		if (p->ops->cleanup)
+			p->ops->cleanup(pcd_dev, p->id);
+	}
+	if (pcd_dev->ops->cleanup)
+		pcd_dev->ops->cleanup(pcd_dev);
+
+	mutex_unlock(&pcd_dev->lock);
+
+	device_unregister(&pcd_dev->device);
+}
+
+static void powercap_release(struct device *dev)
+{
+	if (dev->parent) {
+		struct powercap_zone_device *pcd_dev = dev_get_drvdata(dev);
+
+		dev_dbg(dev, "powercap_release zone %s\n", pcd_dev->name);
+
+		delete_constraints(pcd_dev);
+		/* Remove id from parent idr struct */
+		idr_remove(pcd_dev->par_idr, pcd_dev->id);
+		/* Destroy idrs allocated for this zone */
+		idr_destroy(&pcd_dev->idr);
+		kfree(pcd_dev->zone_dev_name);
+		mutex_destroy(&pcd_dev->lock);
+		kfree(pcd_dev);
+	} else {
+		struct powercap_controller *instance = dev_get_drvdata(dev);
+
+		dev_dbg(dev, "powercap_release controller %s\n",
+							instance->name);
+		idr_destroy(&instance->idr);
+		mutex_destroy(&instance->node_lock);
+		kfree(instance);
+	}
+}
+
+static ssize_t type_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	if (dev->parent)
+		strcpy(buf, "power-zone\n");
+	else
+		strcpy(buf, "controller\n");
+
+	return strlen(buf);
+}
+
+static struct device_attribute powercap_def_attrs[] = {
+		__ATTR_RO(type),
+		__ATTR_NULL
+};
+
+static struct class powercap_class = {
+	.name = "power_cap",
+	.dev_release = powercap_release,
+	.dev_attrs = powercap_def_attrs,
+};
+
+struct powercap_zone_device *powercap_zone_register(
+				struct powercap_controller *controller,
+				const char *name,
+				struct powercap_zone_device *parent,
+				const struct powercap_zone_ops *ops,
+				int nr_constraints,
+				struct powercap_zone_constraint_ops *const_ops)
+{
+	int result;
+	struct powercap_zone_device *pcd_dev;
+	struct device *dev_ptr;
+	struct idr *idr_ptr;
+	char *parent_name;
+	int name_sz;
+
+	if (!name || !controller)
+		return ERR_PTR(-EINVAL);
+	if (strlen(name) > POWERCAP_ZONE_NAME_LENGTH)
+		return ERR_PTR(-EINVAL);
+	if (!ops)
+		return ERR_PTR(-EINVAL);
+	if (!ops->get_energy_uj && !ops->get_power_uw)
+		return ERR_PTR(-EINVAL);
+	if (!check_controller_validity(controller))
+		return ERR_PTR(-EINVAL);
+	if (parent && !search_node(controller, parent))
+		return ERR_PTR(-EINVAL);
+	pcd_dev = kzalloc(sizeof(*pcd_dev), GFP_KERNEL);
+	if (!pcd_dev)
+		return ERR_PTR(-ENOMEM);
+
+	INIT_LIST_HEAD(&pcd_dev->constraint_list);
+	mutex_init(&pcd_dev->lock);
+	pcd_dev->ops = ops;
+	pcd_dev->controller_inst = controller;
+	strncpy(pcd_dev->name, name, POWERCAP_ZONE_NAME_LENGTH);
+	pcd_dev->name[POWERCAP_ZONE_NAME_LENGTH] = '\0';
+	if (!parent) {
+		dev_ptr = &controller->device;
+		idr_ptr = &controller->idr;
+		parent_name = controller->name;
+	} else {
+		dev_ptr = &parent->device;
+		idr_ptr = &parent->idr;
+		parent_name = parent->zone_dev_name;
+	}
+	pcd_dev->par_idr = idr_ptr;
+	pcd_dev->device.class = &powercap_class;
+
+	/* allocate enough which can accommodate parent + ":" + int value */
+	name_sz = strlen(parent_name) +	sizeof(int)*2 + 2;
+	pcd_dev->zone_dev_name =  kmalloc(name_sz, GFP_KERNEL);
+	if (!pcd_dev->zone_dev_name) {
+		result = -ENOMEM;
+		goto err_name_alloc;
+	}
+	mutex_lock(&controller->node_lock);
+	/* Using idr to get the unique id */
+	result = idr_alloc(pcd_dev->par_idr, NULL, 0, 0, GFP_KERNEL);
+	if (result < 0) {
+		mutex_unlock(&controller->node_lock);
+		goto err_idr_alloc;
+	}
+	pcd_dev->id = result;
+	idr_init(&pcd_dev->idr);
+
+	snprintf(pcd_dev->zone_dev_name, name_sz, "%s:%x",
+						parent_name, pcd_dev->id);
+	dev_set_name(&pcd_dev->device, pcd_dev->zone_dev_name);
+	pcd_dev->device.parent = dev_ptr;
+
+	create_power_zone_attributes(pcd_dev);
+	create_constraint_attributes(pcd_dev, nr_constraints, const_ops);
+	pcd_dev->attrs.dev_attr_groups[pcd_dev->attrs.attr_grp_cnt] = NULL;
+	pcd_dev->device.groups = pcd_dev->attrs.dev_attr_groups;
+
+	result = device_register(&pcd_dev->device);
+	if (result) {
+		delete_constraints(pcd_dev);
+		idr_remove(pcd_dev->par_idr, pcd_dev->id);
+		mutex_unlock(&controller->node_lock);
+		goto err_dev_reg;
+	}
+	mutex_unlock(&controller->node_lock);
+
+	dev_set_drvdata(&pcd_dev->device, pcd_dev);
+	pcd_dev->node = create_node(pcd_dev);
+	if (!pcd_dev->node) {
+		result = -ENOMEM;
+		goto err_dev_reg_done;
+	}
+	if (parent)
+		insert_node(controller, pcd_dev->node, parent->node);
+	else
+		insert_node(controller, pcd_dev->node, NULL);
+
+	return pcd_dev;
+
+err_dev_reg:
+	idr_destroy(&pcd_dev->idr);
+err_idr_alloc:
+	kfree(pcd_dev->zone_dev_name);
+err_name_alloc:
+	mutex_destroy(&pcd_dev->lock);
+	kfree(pcd_dev);
+	return ERR_PTR(result);
+
+err_dev_reg_done:
+	device_unregister(&pcd_dev->device);
+	return ERR_PTR(result);
+}
+EXPORT_SYMBOL_GPL(powercap_zone_register);
+
+int powercap_zone_unregister(struct powercap_controller *controller,
+				struct powercap_zone_device *pcd_dev)
+{
+	if (!pcd_dev || !controller)
+		return -EINVAL;
+
+	if (!search_node(controller, pcd_dev))
+		return -EINVAL;
+
+	delete_node(controller, pcd_dev->node, delete_zone);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(powercap_zone_unregister);
+
+struct powercap_controller *powercap_allocate_controller(
+					const char *controller_name)
+{
+	struct powercap_controller *cntrl;
+	int result;
+
+	if (!controller_name)
+		return ERR_PTR(-EINVAL);
+
+	if (strlen(controller_name) > POWERCAP_CTRL_NAME_LENGTH)
+		return ERR_PTR(-EINVAL);
+
+	cntrl = kzalloc(sizeof(struct powercap_controller), GFP_KERNEL);
+	if (!cntrl)
+		return ERR_PTR(-ENOMEM);
+
+	mutex_init(&cntrl->node_lock);
+	INIT_LIST_HEAD(&cntrl->ctrl_inst);
+	strncpy(cntrl->name, controller_name, POWERCAP_CTRL_NAME_LENGTH);
+	cntrl->name[POWERCAP_CTRL_NAME_LENGTH] = '\0';
+	cntrl->device.class = &powercap_class;
+	dev_set_name(&cntrl->device, cntrl->name);
+	result = device_register(&cntrl->device);
+	if (result) {
+		kfree(cntrl);
+		return ERR_PTR(result);
+	}
+	dev_set_drvdata(&cntrl->device, cntrl);
+
+	cntrl->root_node = create_node(NULL);
+	if (!cntrl->root_node) {
+		result = -ENOMEM;
+		goto unregister;
+	}
+	idr_init(&cntrl->idr);
+	mutex_lock(&powercap_cntrl_list_lock);
+	list_add_tail(&cntrl->ctrl_inst, &powercap_cntrl_list);
+	mutex_unlock(&powercap_cntrl_list_lock);
+
+	return cntrl;
+
+unregister:
+	device_unregister(&cntrl->device);
+	return ERR_PTR(result);
+}
+EXPORT_SYMBOL_GPL(powercap_allocate_controller);
+
+void powercap_deallocate_controller(struct powercap_controller *instance)
+{
+	struct powercap_controller *pos = NULL;
+
+	mutex_lock(&powercap_cntrl_list_lock);
+
+	list_for_each_entry(pos, &powercap_cntrl_list, ctrl_inst) {
+		if (pos == instance)
+			break;
+	}
+	if (pos != instance) {
+		/* instance not found */
+		mutex_unlock(&powercap_cntrl_list_lock);
+		return;
+	}
+	list_del(&instance->ctrl_inst);
+	delete_node(instance, instance->root_node, delete_zone);
+
+	mutex_unlock(&powercap_cntrl_list_lock);
+
+	device_unregister(&instance->device);
+}
+EXPORT_SYMBOL_GPL(powercap_deallocate_controller);
+
+static int __init powercap_init(void)
+{
+	int result = 0;
+
+	result = class_register(&powercap_class);
+	if (result)
+		return result;
+
+	return result;
+}
+
+static void __exit powercap_exit(void)
+{
+	class_unregister(&powercap_class);
+}
+
+fs_initcall(powercap_init);
+module_exit(powercap_exit);
+
+MODULE_DESCRIPTION("PowerCap SysFs Driver");
+MODULE_AUTHOR("Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/powercap.h b/include/linux/powercap.h
new file mode 100644
index 0000000..9967bd0
--- /dev/null
+++ b/include/linux/powercap.h
@@ -0,0 +1,300 @@
+/*
+ * powercap.h : Exports all power class sysfs interface
+ * Copyright (c) 2013, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.
+ *
+ */
+
+#ifndef __POWERCAP_H__
+#define __POWERCAP_H__
+
+#include <linux/device.h>
+#include <linux/idr.h>
+
+/*
+ * A power cap class device can contain multiple powercap controllers.
+ * Each controller can have multiple power zones, which can be independently
+ * controlled. Each power zone can have one or more constraints.
+ */
+
+#define POWERCAP_CTRL_NAME_LENGTH			30
+#define POWERCAP_ZONE_NAME_LENGTH			30
+
+struct powercap_zone_device;
+struct powercap_zone_constraint;
+
+/**
+ * struct powercap_zone_constraint_ops - Define constraint callbacks
+ * @set_power_limit_uw:		Set power limit in micro-watts.
+ * @get_power_limit_uw:		Get power limit in micro-watts.
+ * @set_time_window_us:		Set time window in micro-seconds.
+ * @get_time_window_us:		Get time window in micro-seconds.
+ * @get_max_power_uw:		Get max power allowed in micro-watts.
+ * @get_min_power_uw:		Get min power allowed in micro-watts.
+ * @get_max_time_window_us:	Get max time window allowed in micro-seconds.
+ * @get_min_time_window_us:	Get min time window allowed in micro-seconds.
+ * @get_name:			Get the name of constraint
+ * @cleanup:			Do any clean up before the constraint is freed
+ * This structure is used to define the constraint callbacks for the client
+ * drivers. The following callbacks are mandatory and can't be NULL:
+ *  set_power_limit_uw
+ *  get_power_limit_uw
+ *  set_time_window_us
+ *  get_time_window_us
+ *  get_name
+ */
+struct powercap_zone_constraint_ops {
+	int (*set_power_limit_uw)
+			(struct powercap_zone_device *, int, u64);
+	int (*get_power_limit_uw)
+			(struct powercap_zone_device *, int, u64 *);
+
+	int (*set_time_window_us)
+			(struct powercap_zone_device *, int, u64);
+	int (*get_time_window_us)
+			(struct powercap_zone_device *, int, u64 *);
+
+	int (*get_max_power_uw)
+			(struct powercap_zone_device *, int, u64 *);
+	int (*get_min_power_uw)
+			(struct powercap_zone_device *, int, u64 *);
+
+	int (*get_max_time_window_us)
+			(struct powercap_zone_device *, int, u64 *);
+	int (*get_min_time_window_us)
+			(struct powercap_zone_device *, int, u64 *);
+	const char *(*get_name) (struct powercap_zone_device *, int);
+	void (*cleanup) (struct powercap_zone_device *, int);
+};
+
+/**
+ * struct powercap_controller- Defines a powercap controller
+ * @name:		name of controller
+ * @device:		device for this controller
+ * @idr	:		idr to have unique id for its child
+ * @root_node:		Root holding power zones for this controller
+ * @node_lock:		mutex for node
+ * @ctrl_inst:		link to the controller list
+ *
+ * Defines powercap controller instance
+ */
+struct powercap_controller {
+	char name[POWERCAP_CTRL_NAME_LENGTH + 1];
+	struct device device;
+	struct idr idr;
+	void *root_node;
+	struct mutex node_lock;
+	struct list_head ctrl_inst;
+};
+
+/**
+ * struct powercap_zone_ops - Define power zone callbacks
+ * @get_max_energy_range_uj:	Get maximum range of energy counter in
+ *				micro-joules.
+ * @get_energy_uj:		Get current energy counter in micro-joules.
+ * @reset_energy_uj:		Reset micro-joules energy counter.
+ * @get_max_power_range_uw:	Get maximum range of power counter in
+ *				micro-watts.
+ * @get_power_uw:		Get current power counter in micro-watts.
+ * @reset_power_uw:		Reset micro-watts power counter.
+ * @cleanup:			Do any clean up before the zone is freed
+ *
+ * This structure defines zone callbacks to be implemented by client drivers.
+ * Client drives can define both energy and power related callbacks. But at
+ * the least one type (either power or energy) is mandatory.
+ */
+struct powercap_zone_ops {
+	int (*get_max_energy_range_uj)
+			(struct powercap_zone_device *, u64 *);
+	int (*get_energy_uj)
+			(struct powercap_zone_device *, u64 *);
+	int (*reset_energy_uj)
+			(struct powercap_zone_device *);
+
+	int (*get_max_power_range_uw)
+			(struct powercap_zone_device *, u64 *);
+	int (*get_power_uw)
+			(struct powercap_zone_device *, u64 *);
+	int (*reset_power_uw) (struct powercap_zone_device *);
+
+	void (*cleanup) (struct powercap_zone_device *);
+};
+
+#define	POWERCAP_ZONE_MAX_ATTRS		10 /* Currently only max 5 */
+#define	POWERCAP_CONSTRAINTS_ATTRS	8  /*  5 attrs/constraints */
+#define	POWERCAP_CONSTRAINTS_MAX_ATTRS	10 * POWERCAP_CONSTRAINTS_ATTRS
+					/* For 10 constraints per zone */
+#define POWERCAP_MAX_ATTR_GROUPS	2 /* One for zone and constraint */
+/**
+ * struct powercap_zone_attr- Defines a per zone attribute group
+ * @zone_dev_attrs:	Device attribute list for power zone.
+ * @zone_attr_count:	Number of power zone attributes.
+ * @const_dev_attrs:	Constraint attributes.
+ * @const_attr_count:	Number of constraint related attributes
+ * @dev_zone_attr_group: Attribute group for power zone attributes
+ * @dev_const_attr_group: Attribute group for constraints
+ * @attr_grp_cnt:	Number of attribute groups
+ * @dev_attr_groups:	Used to assign to dev->group
+ *
+ * Used to add an attribute group unique to a zone based on registry.
+ */
+struct powercap_zone_attr {
+	struct attribute *zone_dev_attrs[POWERCAP_ZONE_MAX_ATTRS];
+	int zone_attr_count;
+	struct attribute *const_dev_attrs[POWERCAP_CONSTRAINTS_MAX_ATTRS];
+	int const_attr_count;
+	struct attribute_group dev_zone_attr_group;
+	struct attribute_group dev_const_attr_group;
+	int attr_grp_cnt;
+	const struct attribute_group
+			*dev_attr_groups[POWERCAP_MAX_ATTR_GROUPS + 1];
+};
+
+/**
+ * struct powercap_zone_device- Defines instance of a power cap zone
+ * @id:			Unique id
+ * @zone_dev_name:	Zone device sysfs node name
+ * @name:		Power zone name.
+ * @controller_inst:	Controller instance for this zone
+ * @ops:		Pointer to the zone operation structure.
+ * @device:		Instance of a device.
+ * @attrs:		Attributes associated with this device
+ * @node:		Node pointer to insert to a tree data structure.
+ * @const_id_cnt:	Constraint id count
+ * @lock:		Mutex to protect zone related operations.
+ * @idr:		Instance to an idr entry for children zones.
+ * @par_idr:		To remove reference from the parent idr
+ * @drv_data:		Driver private data
+ * @constraint_list:	Link to the next power zone for this controller.
+ *
+ * This defines a power zone instance. The fields of this structure are
+ * private, and should not be used by client drivers.
+ */
+struct powercap_zone_device {
+	int id;
+	char *zone_dev_name;
+	char name[POWERCAP_ZONE_NAME_LENGTH + 1];
+	void *controller_inst;
+	const struct powercap_zone_ops *ops;
+	struct device device;
+	struct powercap_zone_attr attrs;
+	void *node;
+	int const_id_cnt;
+	struct mutex lock;
+	struct idr idr;
+	struct idr *par_idr;
+	void *drv_data;
+	struct list_head constraint_list;
+};
+
+/* For clients to get their device pointer, may be used for dev_dbgs */
+#define POWERCAP_GET_DEV(p_zone)	(&pzone->device)
+
+/**
+* powercap_set_zone_data() - Set private data for a zone
+* @pcd_dev:	A pointer to the valid zone instance.
+* @pdata:	A pointer to the user private data.
+*
+* Allows client drivers to associate some private data to zone instance.
+*/
+static inline void powercap_set_zone_data(struct powercap_zone_device *pcd_dev,
+						void *pdata)
+{
+	if (pcd_dev)
+		pcd_dev->drv_data = pdata;
+}
+
+/**
+* powercap_get_zone_data() - Get private data for a zone
+* @pcd_dev:	A pointer to the valid zone instance.
+*
+* Allows client drivers to get private data associate with a zone,
+* using call to powercap_set_zone_data.
+*/
+static inline void *powercap_get_zone_data(struct powercap_zone_device *pcd_dev)
+{
+	if (pcd_dev)
+		return pcd_dev->drv_data;
+	return NULL;
+}
+
+/* Controller allocate/deallocate API */
+
+/**
+* powercap_allocate_controller() - Allocates a controller
+* @controller_name:	The Name of this controller, which will be shown
+*			in the sysfs Interface.
+*
+* Used to create a controller with the power capping class. Here controller
+* can represent a type of technology, which can control a range of power zones.
+* For example a controller can be RAPL (Running Average Power Limit)
+* Intel® 64 and IA-32 Processor Architectures. The name can be any string
+* which must be unique, otherwise this function returns NULL.
+* On successful allocation, this API returns a pointer to the
+* controller instance.
+*/
+struct powercap_controller *powercap_allocate_controller(
+						const char *controller_name);
+
+/**
+* powercap_deallocate_controller() - Deallocates a controller
+* @instance:	A pointer to the valid controller instance.
+*
+* Used to deallocate a controller with the power capping class. This
+* takes only one argument, which is the pointer to the instance returned
+* by a call to powercap_allocate_controller.
+* When a controller is deallocated, all zones and associated constraints
+* are freed.
+*/
+void powercap_deallocate_controller(struct powercap_controller *instance);
+
+/* Zone register/unregister API */
+
+/**
+* powercap_zone_register() - Register a power zone
+* @controller:	A controller instance under which this zone operates.
+* @name:	A name for this zone.
+* @parent:	A pointer to the parent power zone instance if any or NULL
+* @ops:		Pointer to zone operation callback structure.
+* @no_constraints: Number of constraints for this zone
+* @const_ops:	Pointer to constraint callback structure
+*
+* Used to register a power zone for a controller. Zones are organized in
+* a tree like structure in sysfs under a controller.
+* A power zone must a register a pointer to a structure representing zone
+* callbacks.
+* A power zone can have a some other power zone as a parent or it can be
+* NULL to appear as a direct descendant of a controller.
+* Each power zone can have number of constraints. Constraints appears
+* under zones as attributes with unique id.
+*/
+struct powercap_zone_device *powercap_zone_register(
+			struct powercap_controller *controller,
+			const char *name,
+			struct powercap_zone_device *parent,
+			const struct powercap_zone_ops *ops,
+			int no_constraints,
+			struct powercap_zone_constraint_ops *const_ops);
+/**
+* powercap_zone_unregister() - Unregister a zone device
+* @controller:	A pointer to the valid instance of a controller.
+* @pcd_dev:	A pointer to the valid zone instance for a controller
+*
+* Used to unregister a zone device for a controller. Once a zone is
+* unregistered then all its children and associated constraints are freed.
+*/
+int powercap_zone_unregister(struct powercap_controller *controller,
+				struct powercap_zone_device *pcd_dev);
+
+#endif
-- 
1.8.3.1


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

* [RFC v01 3/3] PowerCap: Added to drivers build
  2013-08-02 18:08 [RFC v01 0/3] Power Capping Framework Srinivas Pandruvada
  2013-08-02 18:08 ` [RFC v01 1/3] PowerCap: Documentation Srinivas Pandruvada
  2013-08-02 18:08 ` [RFC v01 2/3] PowerCap: Add class driver Srinivas Pandruvada
@ 2013-08-02 18:08 ` Srinivas Pandruvada
  2013-08-02 22:29 ` [RFC v01 0/3] Power Capping Framework Greg KH
  2013-08-02 22:30 ` Greg KH
  4 siblings, 0 replies; 20+ messages in thread
From: Srinivas Pandruvada @ 2013-08-02 18:08 UTC (permalink / raw)
  To: linux-kernel; +Cc: gregkh, Srinivas Pandruvada

Added changes to Makefile and Kconfig to include in driver build.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/Kconfig  | 2 ++
 drivers/Makefile | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 9953a42..89ff2f8 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -166,4 +166,6 @@ source "drivers/ipack/Kconfig"
 
 source "drivers/reset/Kconfig"
 
+source "drivers/powercap/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 130abc1..e207b2d 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -152,3 +152,4 @@ obj-$(CONFIG_IIO)		+= iio/
 obj-$(CONFIG_VME_BUS)		+= vme/
 obj-$(CONFIG_IPACK_BUS)		+= ipack/
 obj-$(CONFIG_NTB)		+= ntb/
+obj-$(CONFIG_POWERCAP_SUPPORT)	+= powercap/
-- 
1.8.3.1


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

* Re: [RFC v01 0/3] Power Capping Framework
  2013-08-02 18:08 [RFC v01 0/3] Power Capping Framework Srinivas Pandruvada
                   ` (2 preceding siblings ...)
  2013-08-02 18:08 ` [RFC v01 3/3] PowerCap: Added to drivers build Srinivas Pandruvada
@ 2013-08-02 22:29 ` Greg KH
  2013-08-02 23:52   ` Srinivas Pandruvada
  2013-08-02 22:30 ` Greg KH
  4 siblings, 1 reply; 20+ messages in thread
From: Greg KH @ 2013-08-02 22:29 UTC (permalink / raw)
  To: Srinivas Pandruvada; +Cc: linux-kernel

On Fri, Aug 02, 2013 at 11:08:49AM -0700, Srinivas Pandruvada wrote:
> - A uniform sys-fs interface for all devices which can offer power capping

There is no "-" in sysfs please.

> - A common API for drivers, which will avoid code duplication and easy
> implementation of client drivers.
> 
> Once this framework is approved, we will submit a RAPL client driver using this
> framework.

No, you need users of a framework in order for it to be approved, we
don't add infrastructure without users.

Especially as what usually happens is, when you add actual users, the
framework changes to fix the bugs found in it :)

Ideally you will have more than one client driver submitted, as a
"framework" for just one driver seems a bit odd, don't you think?

Also, you add lots of new sysfs files, those need to be documented in
Documentation/ABI/ with this series.

thanks,

greg k-h

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

* Re: [RFC v01 0/3] Power Capping Framework
  2013-08-02 18:08 [RFC v01 0/3] Power Capping Framework Srinivas Pandruvada
                   ` (3 preceding siblings ...)
  2013-08-02 22:29 ` [RFC v01 0/3] Power Capping Framework Greg KH
@ 2013-08-02 22:30 ` Greg KH
  2013-08-02 22:33   ` Joe Perches
  2013-08-03  0:03   ` Srinivas Pandruvada
  4 siblings, 2 replies; 20+ messages in thread
From: Greg KH @ 2013-08-02 22:30 UTC (permalink / raw)
  To: Srinivas Pandruvada; +Cc: linux-kernel

On Fri, Aug 02, 2013 at 11:08:49AM -0700, Srinivas Pandruvada wrote:
> Once this framework is approved, we will submit a RAPL client driver using this
> framework.

Also, given that you work with a lot of kernel developers, I would like
to see their review and signed-off-by: on these patches before you
resend them, please.  That way I know that they agree with this
framework as a way for their future changes in this area.

thanks,

greg k-h

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

* Re: [RFC v01 0/3] Power Capping Framework
  2013-08-02 22:30 ` Greg KH
@ 2013-08-02 22:33   ` Joe Perches
  2013-08-02 22:50     ` Greg KH
  2013-08-03  0:03   ` Srinivas Pandruvada
  1 sibling, 1 reply; 20+ messages in thread
From: Joe Perches @ 2013-08-02 22:33 UTC (permalink / raw)
  To: Greg KH; +Cc: Srinivas Pandruvada, linux-kernel

On Sat, 2013-08-03 at 06:30 +0800, Greg KH wrote:
> On Fri, Aug 02, 2013 at 11:08:49AM -0700, Srinivas Pandruvada wrote:
> > Once this framework is approved, we will submit a RAPL client driver using this
> > framework.
> 
> Also, given that you work with a lot of kernel developers, I would like
> to see their review and signed-off-by: on these patches before you
> resend them, please.  That way I know that they agree with this
> framework as a way for their future changes in this area.

Before is a bit hard on public mailing lists isn't it?

Shouldn't the reviewed-by/acked-by/signed-off-by's be
done after a real patch is submitted?

This is just an RFC patchset.



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

* Re: [RFC v01 2/3] PowerCap: Add class driver
  2013-08-02 18:08 ` [RFC v01 2/3] PowerCap: Add class driver Srinivas Pandruvada
@ 2013-08-02 22:43   ` Joe Perches
  2013-08-03  0:06     ` Srinivas Pandruvada
  0 siblings, 1 reply; 20+ messages in thread
From: Joe Perches @ 2013-08-02 22:43 UTC (permalink / raw)
  To: Srinivas Pandruvada; +Cc: linux-kernel, gregkh

On Fri, 2013-08-02 at 11:08 -0700, Srinivas Pandruvada wrote:
> Added power cap class driver, which provides an API for client drivers
> to use and provide a consistant sys-fs interface to user mode.

Just some stylistic notes.

> diff --git a/drivers/powercap/powercap_sys.c b/drivers/powercap/powercap_sys.c
[]
> +/* Power zone show function */
> +#define define_device_show(_attr)		\
> +static ssize_t show_##_attr(struct device *dev, struct device_attribute *attr,\
> +			char *buf) \

Using _attr and another variable named attr is at least
visually confusing.

Maybe use name or type instead of _attr?

> +{ \
> +	u64 value; \
> +	ssize_t len = -EINVAL; \
> +	struct powercap_zone_device *pcd_dev = dev_get_drvdata(dev); \
> +	\
> +	if (pcd_dev && pcd_dev->ops && pcd_dev->ops->get_##_attr) { \
> +		mutex_lock(&pcd_dev->lock); \
> +		if (!pcd_dev->ops->get_##_attr(pcd_dev, &value)) \
> +			len = sprintf(buf, "%lld\n", value); \
> +		mutex_unlock(&pcd_dev->lock); \
> +	} \
> +	\
> +	return len; \
> +}
> +
> +/* Power zone store function; only reset is possible */
> +#define define_device_store(_attr)		\
> +static ssize_t store_##_attr(struct device *dev,\
> +				struct device_attribute *attr, \
> +				const char *buf, size_t count) \

here too

[]

> +/* Power zone constraint show function */
> +#define define_device_constraint_show(_attr) \
> +static ssize_t show_constraint_##_attr(struct device *dev, \
> +				struct device_attribute *attr,\
> +				char *buf) \

and here

[]

> +/* Power zone constraint store function */
> +#define define_device_constraint_store(_attr) \
> +static ssize_t store_constraint_##_attr(struct device *dev,\
> +				struct device_attribute *attr, \
> +				const char *buf, size_t count) \

etc...

> +struct powercap_zone_constraint *powercap_zone_add_constraint(
> +				struct powercap_zone_device *pcd_dev,
> +				struct powercap_zone_constraint_ops *ops)
> +{
[]
> +	dev_dbg(&pcd_dev->device, "powercap_zone_add_constraint\n");

These logging messages aren't useful.
function tracing works very well.



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

* Re: [RFC v01 0/3] Power Capping Framework
  2013-08-02 22:33   ` Joe Perches
@ 2013-08-02 22:50     ` Greg KH
  0 siblings, 0 replies; 20+ messages in thread
From: Greg KH @ 2013-08-02 22:50 UTC (permalink / raw)
  To: Joe Perches; +Cc: Srinivas Pandruvada, linux-kernel

On Fri, Aug 02, 2013 at 03:33:26PM -0700, Joe Perches wrote:
> On Sat, 2013-08-03 at 06:30 +0800, Greg KH wrote:
> > On Fri, Aug 02, 2013 at 11:08:49AM -0700, Srinivas Pandruvada wrote:
> > > Once this framework is approved, we will submit a RAPL client driver using this
> > > framework.
> > 
> > Also, given that you work with a lot of kernel developers, I would like
> > to see their review and signed-off-by: on these patches before you
> > resend them, please.  That way I know that they agree with this
> > framework as a way for their future changes in this area.
> 
> Before is a bit hard on public mailing lists isn't it?

That's implying that Intel doesn't have internal mailing lists where
kernel patches are reviewed and proposed before being made public...

> Shouldn't the reviewed-by/acked-by/signed-off-by's be
> done after a real patch is submitted?

That's fine too, but then the needed people need to be cc:ed.

thanks,

greg k-h

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

* Re: [RFC v01 0/3] Power Capping Framework
  2013-08-02 22:29 ` [RFC v01 0/3] Power Capping Framework Greg KH
@ 2013-08-02 23:52   ` Srinivas Pandruvada
  2013-08-03  0:53     ` Greg KH
  0 siblings, 1 reply; 20+ messages in thread
From: Srinivas Pandruvada @ 2013-08-02 23:52 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, Brown, Len, Rafael J. Wysocki, Arjan van de Ven

On 08/02/2013 03:29 PM, Greg KH wrote:
> On Fri, Aug 02, 2013 at 11:08:49AM -0700, Srinivas Pandruvada wrote:
>> - A uniform sys-fs interface for all devices which can offer power capping
> There is no "-" in sysfs please.
OK.
>> - A common API for drivers, which will avoid code duplication and easy
>> implementation of client drivers.
>>
>> Once this framework is approved, we will submit a RAPL client driver using this
>> framework.
> No, you need users of a framework in order for it to be approved, we
> don't add infrastructure without users.
>
> Especially as what usually happens is, when you add actual users, the
> framework changes to fix the bugs found in it :)
I will post the one client driver, which is already using this framework 
as this series.
> Ideally you will have more than one client driver submitted, as a
> "framework" for just one driver seems a bit odd, don't you think?
There are other groups and vendors interested in using this framework. 
But they want to make sure that this
framework can go to upstream Linux. We will provide one client using 
this framework at this time.
Do you think this is a problem?
> Also, you add lots of new sysfs files, those need to be documented in
> Documentation/ABI/ with this series.
I have a Documentation patch, which describes ABI and framework. It is 
under Documentation/powercap.
Do I need to move this to Documentation/ABI?
> thanks,
>
> greg k-h
Thanks,
Srinivas


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

* Re: [RFC v01 0/3] Power Capping Framework
  2013-08-02 22:30 ` Greg KH
  2013-08-02 22:33   ` Joe Perches
@ 2013-08-03  0:03   ` Srinivas Pandruvada
  2013-08-03  0:54     ` Greg KH
  1 sibling, 1 reply; 20+ messages in thread
From: Srinivas Pandruvada @ 2013-08-03  0:03 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, Rafael J. Wysocki, Brown, Len, Arjan van de Ven

On 08/02/2013 03:30 PM, Greg KH wrote:
> On Fri, Aug 02, 2013 at 11:08:49AM -0700, Srinivas Pandruvada wrote:
>> Once this framework is approved, we will submit a RAPL client driver using this
>> framework.
> Also, given that you work with a lot of kernel developers, I would like
> to see their review and signed-off-by: on these patches before you
> resend them, please.  That way I know that they agree with this
> framework as a way for their future changes in this area.
The framework was presented internally number of times to Intel Linux 
Architects (including Rafael, Len Brown  and Arjan (copied in this email)).
Actual implementation, was reviewed by few developers. But I am 
expecting lots of positive/negative feedback on implementation
from community.
> thanks,
>
> greg k-h
>
Thanks,
Srinivas

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

* Re: [RFC v01 2/3] PowerCap: Add class driver
  2013-08-02 22:43   ` Joe Perches
@ 2013-08-03  0:06     ` Srinivas Pandruvada
  0 siblings, 0 replies; 20+ messages in thread
From: Srinivas Pandruvada @ 2013-08-03  0:06 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, gregkh

On 08/02/2013 03:43 PM, Joe Perches wrote:
> On Fri, 2013-08-02 at 11:08 -0700, Srinivas Pandruvada wrote:
>> Added power cap class driver, which provides an API for client drivers
>> to use and provide a consistant sys-fs interface to user mode.
> Just some stylistic notes.
>
>> diff --git a/drivers/powercap/powercap_sys.c b/drivers/powercap/powercap_sys.c
> []
>> +/* Power zone show function */
>> +#define define_device_show(_attr)		\
>> +static ssize_t show_##_attr(struct device *dev, struct device_attribute *attr,\
>> +			char *buf) \
> Using _attr and another variable named attr is at least
> visually confusing.
>
> Maybe use name or type instead of _attr?
>
>> +{ \
>> +	u64 value; \
>> +	ssize_t len = -EINVAL; \
>> +	struct powercap_zone_device *pcd_dev = dev_get_drvdata(dev); \
>> +	\
>> +	if (pcd_dev && pcd_dev->ops && pcd_dev->ops->get_##_attr) { \
>> +		mutex_lock(&pcd_dev->lock); \
>> +		if (!pcd_dev->ops->get_##_attr(pcd_dev, &value)) \
>> +			len = sprintf(buf, "%lld\n", value); \
>> +		mutex_unlock(&pcd_dev->lock); \
>> +	} \
>> +	\
>> +	return len; \
>> +}
>> +
>> +/* Power zone store function; only reset is possible */
>> +#define define_device_store(_attr)		\
>> +static ssize_t store_##_attr(struct device *dev,\
>> +				struct device_attribute *attr, \
>> +				const char *buf, size_t count) \
> here too
>
> []
>
>> +/* Power zone constraint show function */
>> +#define define_device_constraint_show(_attr) \
>> +static ssize_t show_constraint_##_attr(struct device *dev, \
>> +				struct device_attribute *attr,\
>> +				char *buf) \
> and here
>
> []
>
>> +/* Power zone constraint store function */
>> +#define define_device_constraint_store(_attr) \
>> +static ssize_t store_constraint_##_attr(struct device *dev,\
>> +				struct device_attribute *attr, \
>> +				const char *buf, size_t count) \
> etc...
>
>> +struct powercap_zone_constraint *powercap_zone_add_constraint(
>> +				struct powercap_zone_device *pcd_dev,
>> +				struct powercap_zone_constraint_ops *ops)
>> +{
> []
>> +	dev_dbg(&pcd_dev->device, "powercap_zone_add_constraint\n");
> These logging messages aren't useful.
> function tracing works very well.
>
>
>
Thanks. Will change in the next patchset.

Thanks,
Srinivas

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

* Re: [RFC v01 1/3] PowerCap: Documentation
  2013-08-02 18:08 ` [RFC v01 1/3] PowerCap: Documentation Srinivas Pandruvada
@ 2013-08-03  0:10   ` Joe Perches
  2013-08-03  0:23     ` Srinivas Pandruvada
  2013-08-05 19:09   ` Jonathan Corbet
  1 sibling, 1 reply; 20+ messages in thread
From: Joe Perches @ 2013-08-03  0:10 UTC (permalink / raw)
  To: Srinivas Pandruvada; +Cc: linux-kernel, gregkh

On Fri, 2013-08-02 at 11:08 -0700, Srinivas Pandruvada wrote:
> Added power cap framework documentation. This explains the use of power capping framework,
> sys-fs and programming interface.

[]

> +Power Zone Attributes
> +=================================
> +Monitoring attributes
> +----------------------
> +	
> +energy_uj (rw): Current energy counter in micro-joules. Write to energy counter
> +resets the counter to zero. If the counter can not be reset, then this attribute
> +is read-only.
> +
> +max_energy_range_uj (ro): Range of the above energy counter in micro-joules.
> +
> +power_uw (rw): Current power counter in micro-watts. Write to this counter
> +resets the counter to zero. If the counter can not be reset, then this attribute
> +is read-only.
> +max_power_range_uw (ro): Range of the above energy counter in micro-watts.
> +
> +It is possible that some domains can have both power and energy counters and
> +ranges, but at least one is mandatory.

Given that the ranges seem to be u64s, perhaps the
lower bounds are too high.  Why not nano/pico/fempto
watts/joules/seconds?



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

* Re: [RFC v01 1/3] PowerCap: Documentation
  2013-08-03  0:10   ` Joe Perches
@ 2013-08-03  0:23     ` Srinivas Pandruvada
  2013-08-03  0:25       ` Joe Perches
  0 siblings, 1 reply; 20+ messages in thread
From: Srinivas Pandruvada @ 2013-08-03  0:23 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, gregkh

On 08/02/2013 05:10 PM, Joe Perches wrote:
> On Fri, 2013-08-02 at 11:08 -0700, Srinivas Pandruvada wrote:
>> Added power cap framework documentation. This explains the use of power capping framework,
>> sys-fs and programming interface.
> []
Didn't understand this comment.
>> +Power Zone Attributes
>> +=================================
>> +Monitoring attributes
>> +----------------------
>> +	
>> +energy_uj (rw): Current energy counter in micro-joules. Write to energy counter
>> +resets the counter to zero. If the counter can not be reset, then this attribute
>> +is read-only.
>> +
>> +max_energy_range_uj (ro): Range of the above energy counter in micro-joules.
>> +
>> +power_uw (rw): Current power counter in micro-watts. Write to this counter
>> +resets the counter to zero. If the counter can not be reset, then this attribute
>> +is read-only.
>> +max_power_range_uw (ro): Range of the above energy counter in micro-watts.
>> +
>> +It is possible that some domains can have both power and energy counters and
>> +ranges, but at least one is mandatory.
> Given that the ranges seem to be u64s, perhaps the
> lower bounds are too high.  Why not nano/pico/fempto
> watts/joules/seconds?
I think they are too small to realistically set a limits on power or 
measure. Let's see what others think.
>
>
Thanks,
Srinivas

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

* Re: [RFC v01 1/3] PowerCap: Documentation
  2013-08-03  0:23     ` Srinivas Pandruvada
@ 2013-08-03  0:25       ` Joe Perches
  0 siblings, 0 replies; 20+ messages in thread
From: Joe Perches @ 2013-08-03  0:25 UTC (permalink / raw)
  To: Srinivas Pandruvada; +Cc: linux-kernel, gregkh

On Fri, 2013-08-02 at 17:23 -0700, Srinivas Pandruvada wrote:
> On 08/02/2013 05:10 PM, Joe Perches wrote:
> > On Fri, 2013-08-02 at 11:08 -0700, Srinivas Pandruvada wrote:
> >> Added power cap framework documentation. This explains the use of power capping framework,
> >> sys-fs and programming interface.
> > []
> Didn't understand this comment.

Not a comment, just notation showing skipping a bunch
context.

> > Given that the ranges seem to be u64s, perhaps the
> > lower bounds are too high.  Why not nano/pico/fempto
> > watts/joules/seconds?
> I think they are too small to realistically set a limits on power or 
> measure. Let's see what others think.

No rush.


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

* Re: [RFC v01 0/3] Power Capping Framework
  2013-08-02 23:52   ` Srinivas Pandruvada
@ 2013-08-03  0:53     ` Greg KH
  2013-08-04 19:36       ` Arjan van de Ven
  0 siblings, 1 reply; 20+ messages in thread
From: Greg KH @ 2013-08-03  0:53 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: linux-kernel, Brown, Len, Rafael J. Wysocki, Arjan van de Ven

On Fri, Aug 02, 2013 at 04:52:21PM -0700, Srinivas Pandruvada wrote:
> On 08/02/2013 03:29 PM, Greg KH wrote:
> >On Fri, Aug 02, 2013 at 11:08:49AM -0700, Srinivas Pandruvada wrote:
> >>- A uniform sys-fs interface for all devices which can offer power capping
> >There is no "-" in sysfs please.
> OK.
> >>- A common API for drivers, which will avoid code duplication and easy
> >>implementation of client drivers.
> >>
> >>Once this framework is approved, we will submit a RAPL client driver using this
> >>framework.
> >No, you need users of a framework in order for it to be approved, we
> >don't add infrastructure without users.
> >
> >Especially as what usually happens is, when you add actual users, the
> >framework changes to fix the bugs found in it :)
> I will post the one client driver, which is already using this
> framework as this series.
> >Ideally you will have more than one client driver submitted, as a
> >"framework" for just one driver seems a bit odd, don't you think?
> There are other groups and vendors interested in using this
> framework. But they want to make sure that this
> framework can go to upstream Linux. We will provide one client using
> this framework at this time.
> Do you think this is a problem?

I would love to see the feedback from those groups and vendors as well,
to ensure that they agree with this.  Ideally, they would sign off on
the patches, and provide clients that work with the framework at the
same time.  Otherwise, I just have to take your word for it :)


> >Also, you add lots of new sysfs files, those need to be documented in
> >Documentation/ABI/ with this series.
> I have a Documentation patch, which describes ABI and framework. It
> is under Documentation/powercap.
> Do I need to move this to Documentation/ABI?

The files in that directory need to follow the format that is described
in Documentation/ABI/README.  I suggest keeping the file you currently
have, and also adding entries into the ABI/ subdirectory as well.

thanks,

greg k-h

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

* Re: [RFC v01 0/3] Power Capping Framework
  2013-08-03  0:03   ` Srinivas Pandruvada
@ 2013-08-03  0:54     ` Greg KH
  0 siblings, 0 replies; 20+ messages in thread
From: Greg KH @ 2013-08-03  0:54 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: linux-kernel, Rafael J. Wysocki, Brown, Len, Arjan van de Ven

On Fri, Aug 02, 2013 at 05:03:42PM -0700, Srinivas Pandruvada wrote:
> On 08/02/2013 03:30 PM, Greg KH wrote:
> >On Fri, Aug 02, 2013 at 11:08:49AM -0700, Srinivas Pandruvada wrote:
> >>Once this framework is approved, we will submit a RAPL client driver using this
> >>framework.
> >Also, given that you work with a lot of kernel developers, I would like
> >to see their review and signed-off-by: on these patches before you
> >resend them, please.  That way I know that they agree with this
> >framework as a way for their future changes in this area.
> The framework was presented internally number of times to Intel
> Linux Architects (including Rafael, Len Brown  and Arjan (copied in
> this email)).

That's good to hear, ideally they would sign off on this as well to show
people like me that they agree with this, otherwise I have no idea that
they have even looked at it.

thanks,

greg k-h

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

* Re: [RFC v01 0/3] Power Capping Framework
  2013-08-03  0:53     ` Greg KH
@ 2013-08-04 19:36       ` Arjan van de Ven
  0 siblings, 0 replies; 20+ messages in thread
From: Arjan van de Ven @ 2013-08-04 19:36 UTC (permalink / raw)
  To: Greg KH; +Cc: Srinivas Pandruvada, linux-kernel, Brown, Len, Rafael J. Wysocki

On 8/2/2013 5:53 PM, Greg KH wrote:
>> I will post the one client driver, which is already using this
>> framework as this series.
>>> Ideally you will have more than one client driver submitted, as a
>>> "framework" for just one driver seems a bit odd, don't you think?
>> There are other groups and vendors interested in using this
>> framework. But they want to make sure that this
>> framework can go to upstream Linux. We will provide one client using
>> this framework at this time.
>> Do you think this is a problem?
>
> I would love to see the feedback from those groups and vendors as well,
> to ensure that they agree with this.  Ideally, they would sign off on
> the patches, and provide clients that work with the framework at the
> same time.  Otherwise, I just have to take your word for it :)

I suspect that in some ways Intel is a year or two ahead in this regard,
we'll see if others are willing to speak up yet ;-)



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

* Re: [RFC v01 1/3] PowerCap: Documentation
  2013-08-02 18:08 ` [RFC v01 1/3] PowerCap: Documentation Srinivas Pandruvada
  2013-08-03  0:10   ` Joe Perches
@ 2013-08-05 19:09   ` Jonathan Corbet
  2013-08-05 19:52     ` Srinivas Pandruvada
  1 sibling, 1 reply; 20+ messages in thread
From: Jonathan Corbet @ 2013-08-05 19:09 UTC (permalink / raw)
  To: Srinivas Pandruvada; +Cc: linux-kernel, gregkh

On Fri,  2 Aug 2013 11:08:50 -0700
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> +power_uw (rw): Current power counter in micro-watts. Write to this counter
> +resets the counter to zero. If the counter can not be reset, then this attribute
> +is read-only.

Sorry if I'm slow, but... power is an instantaneous quantity, so why would
you use a counter for it?  And why would you want to reset it to zero?

Thanks,

jon

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

* Re: [RFC v01 1/3] PowerCap: Documentation
  2013-08-05 19:09   ` Jonathan Corbet
@ 2013-08-05 19:52     ` Srinivas Pandruvada
  0 siblings, 0 replies; 20+ messages in thread
From: Srinivas Pandruvada @ 2013-08-05 19:52 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: linux-kernel, gregkh

On 08/05/2013 12:09 PM, Jonathan Corbet wrote:
> On Fri,  2 Aug 2013 11:08:50 -0700
> Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
>
>> +power_uw (rw): Current power counter in micro-watts. Write to this counter
>> +resets the counter to zero. If the counter can not be reset, then this attribute
>> +is read-only.
> Sorry if I'm slow, but... power is an instantaneous quantity, so why would
> you use a counter for it?  And why would you want to reset it to zero?
It is an instantaneous value. When I say counter, either in hardware or 
in driver somewhere,
it is reading energy and using a time delta and continuously updating 
this value.
Reset to zero will allow user space to reset its internal energy counter 
used to calculate power.
Our hardware at this time can only provide energy_uj at this time, added 
power_uw for some future devices.
This was suggested in internal reviews. But based on feedback, we can 
drop this attribute totally,
till we have a device needing this.
> Thanks,
>
> jon
>


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

end of thread, other threads:[~2013-08-05 19:44 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-02 18:08 [RFC v01 0/3] Power Capping Framework Srinivas Pandruvada
2013-08-02 18:08 ` [RFC v01 1/3] PowerCap: Documentation Srinivas Pandruvada
2013-08-03  0:10   ` Joe Perches
2013-08-03  0:23     ` Srinivas Pandruvada
2013-08-03  0:25       ` Joe Perches
2013-08-05 19:09   ` Jonathan Corbet
2013-08-05 19:52     ` Srinivas Pandruvada
2013-08-02 18:08 ` [RFC v01 2/3] PowerCap: Add class driver Srinivas Pandruvada
2013-08-02 22:43   ` Joe Perches
2013-08-03  0:06     ` Srinivas Pandruvada
2013-08-02 18:08 ` [RFC v01 3/3] PowerCap: Added to drivers build Srinivas Pandruvada
2013-08-02 22:29 ` [RFC v01 0/3] Power Capping Framework Greg KH
2013-08-02 23:52   ` Srinivas Pandruvada
2013-08-03  0:53     ` Greg KH
2013-08-04 19:36       ` Arjan van de Ven
2013-08-02 22:30 ` Greg KH
2013-08-02 22:33   ` Joe Perches
2013-08-02 22:50     ` Greg KH
2013-08-03  0:03   ` Srinivas Pandruvada
2013-08-03  0:54     ` Greg KH

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