Messages in this thread | | | Date | Mon, 14 Nov 2005 23:05:36 +0100 | From | Pavel Machek <> | Subject | Re: More cleanups for sharpsl_pm.c |
| |
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |