Re: [PATCH 3/7] [RFC] Battery monitoring class

From: Anton Vorontsov
Date: Thu Apr 12 2007 - 12:55:41 EST


Hello Randy,

On Wed, Apr 11, 2007 at 07:53:59PM -0700, Randy Dunlap wrote:
> On Thu, 12 Apr 2007 03:25:03 +0400 Anton Vorontsov wrote:
> > +void battery_status_changed(struct battery *bat)
> > +{
> > + pr_debug("%s\n", __FUNCTION__);
> > + #ifdef CONFIG_LEDS_TRIGGERS
>
> Please don't indent preprocessor controls (ifdef/endif etc.).
...
> Place 'switch' and 'case' at the same indent level. This prevents
> the "double-indent" for the code statements.
...
> We usually try to place a blank line between local data and code.
...
> space after "if"
...

Much thanks, will fix.

> > + device_remove_file(bat->dev, &dev_attr_##name);
> > +
> > + goto success;
> > +
> > +capacity_failed: remove_bat_attr_conditional(current);
> > +current_failed: remove_bat_attr_conditional(voltage);
> > +voltage_failed: remove_bat_attr_conditional(temp);
> > +temp_failed: remove_bat_attr_conditional(max_capacity);
> > +max_capacity_failed: remove_bat_attr_conditional(max_current);
> > +max_current_failed: remove_bat_attr_conditional(max_voltage);
> > +max_voltage_failed: remove_bat_attr_conditional(min_capacity);
> > +min_capacity_failed: remove_bat_attr_conditional(min_current);
> > +min_current_failed: remove_bat_attr_conditional(min_voltage);
> > +min_voltage_failed: remove_bat_attr_conditional(status);
>
> I thought there was a class_remove() or something like that?
> but I'm not sure of it.

[class_]device_destroy()? Yeah, but it's exactly same thing as
[class_]device_unregister.

But this is really good question. Should I manually clean up attributes,
or sysfs will take care? I.e. whole battery directory removes, and
sysfs removes files in it, or they just leaks?

I've took worst scenario, and done removal manually (I've grep'ed in
drivers/, and almost every driver also doing so).

> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***

Thanks for comments!

--
Anton Vorontsov
email: cbou@xxxxxxx
backup email: ya-cbou@xxxxxxxxx
irc://irc.freenode.org/bd2
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/