Re: More cleanups for sharpsl_pm.c

[Pavel Machek]

Hi!

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

I'm quite convinced it is easier to read...

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

...but if this code still changes rapidly, it can probably wait.

> > +/* 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.

Well, apm status is not quite critical, right? And if 5% is remaining,
it *is* low battery; if that gets more precise over time, that is
okay.
								Pavel
-- 
Thanks, Sharp!
-
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/