On Mon, Aug 08, 2005 at 07:51:57PM -0700, Todd Poynor wrote: > +static void powerop_kobj_release(struct kobject *kobj) > +{ > + return; > +} Hint, if your release function is just a noop like this, your code is wrong. The kernel requires you to have a release function for a reason. Please fix it. > +struct powerop_param_attribute { > + int index; > + struct attribute attr; > +}; space vs. tab issue. > +static ssize_t > +powerop_param_attr_show(struct kobject * kobj, struct attribute * attr, > + char * buf) > +{ > + struct powerop_param_attribute * param_attr = to_param_attr(attr); > + struct powerop_point point; > + ssize_t ret = 0; > + > + if ((ret = powerop_get_point(&point)) == 0) > + ret = sprintf(buf, "%d\n", point.param[param_attr->index]); Please break this up into 3 lines instead of 2 to make it easier to read and maintain over time. You do this in other places too, please fix them too. thanks, greg k-h