Messages in this thread | | | Subject | Re: [RFC v01 2/3] PowerCap: Add class driver | From | Joe Perches <> | Date | Fri, 02 Aug 2013 15:43:27 -0700 |
| |
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.
| |