Re: More cleanups for sharpsl_pm.c

From: Richard Purdie
Date: Sat Nov 12 2005 - 18:26:52 EST


On Fri, 2005-11-11 at 00:56 +0100, Pavel Machek wrote:
> sharpsl.c uses macros to hide method calls, in quite a confusing
> way. This just inlines the macros, so it is easy to see what is going
> on.

I'm not totally convinced this makes it easier to read. To me,
CHARGE_ON(); is more readable than sharpsl_pm.machinfo->charge(1);. Yes,
you need to look up what the macro does but the names give a fairly good
idea.

ALso, keeping the macros means when I implement the LED trigger for
charging, I don't have to edit every function in sharpsl_pm but can just
tweak the header and add an extra level of LED functions. Given that,
I'd prefer to leave these as they are for now.

> +/* FIXME:
> + why not simply get_percentage, and base it off that?
> +*/
> if (sharpsl_pm.charge_mode == CHRG_ON) {
> high_thresh = sharpsl_pm.machinfo->status_high_acin;
> low_thresh = sharpsl_pm.machinfo->status_low_acin;

The percentage curves is likely to change in the future and I doubt
anyone would remember to update these values. I'd therefore prefer for
them to be independent of the lookup table.

(The table will change once I get more discharge profiles from users and
can work out a more accurate discharge curve).


Richard

-
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/