Messages in this thread | | | Date | Fri, 02 Aug 2013 17:06:58 -0700 | From | Srinivas Pandruvada <> | Subject | Re: [RFC v01 2/3] PowerCap: Add class driver |
| |
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
| |