[patch] sharpsl_pm refactor

[patch] sharpsl_pm refactor

[Pavel Machek]

This prepares sharpsl_pm.c for collie. Without nested if()s, #ifdefs
can be added. Also warn users about charging in unsuitable
temperature.

Signed-off-by: Pavel Machek <pavel@suse.cz>

PATCH FOLLOWS
KernelVersion: 2.6.18-rc1-git

diff --git a/arch/arm/common/sharpsl_pm.c b/arch/arm/common/sharpsl_pm.c
index 045e37e..12beac3 100644
--- a/arch/arm/common/sharpsl_pm.c
+++ b/arch/arm/common/sharpsl_pm.c
@@ -276,13 +284,19 @@ static void sharpsl_chrg_full_timer(unsi
 		dev_dbg(sharpsl_pm.dev, "Charge Full: AC removed - stop charging!\n");
 		if (sharpsl_pm.charge_mode == CHRG_ON)
 			sharpsl_charge_off();
-	} else if (sharpsl_pm.full_count < 2) {
+		return;
+	} 
+	if (sharpsl_pm.full_count < 2) {
 		dev_dbg(sharpsl_pm.dev, "Charge Full: Count too low\n");
 		schedule_work(&toggle_charger);
-	} else if (time_after(jiffies, sharpsl_pm.charge_start_time + SHARPSL_CHARGE_FINISH_TIME)) {
+		return;
+	} 
+	if (time_after(jiffies, sharpsl_pm.charge_start_time + SHARPSL_CHARGE_FINISH_TIME)) {
 		dev_dbg(sharpsl_pm.dev, "Charge Full: Interrupt generated too slowly - retry.\n");
 		schedule_work(&toggle_charger);
-	} else {
+		return;
+	}
+	{
 		sharpsl_charge_off();
 		sharpsl_pm.charge_mode = CHRG_DONE;
 		dev_dbg(sharpsl_pm.dev, "Charge Full: Charging Finished\n");
@@ -412,8 +429,10 @@ static int sharpsl_check_battery_temp(vo
 	val = get_select_val(buff);
 
 	dev_dbg(sharpsl_pm.dev, "Temperature: %d\n", val);
-	if (val > sharpsl_pm.machinfo->charge_on_temp)
+	if (val > sharpsl_pm.machinfo->charge_on_temp) {
+		printk(KERN_WARNING "Not charging: temperature out of limits.\n"); 
 		return -1;
+	}
 
 	return 0;
 }

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [patch] sharpsl_pm refactor

[Richard Purdie]

On Fri, 2006-07-07 at 13:48 +0200, Pavel Machek wrote:
> This prepares sharpsl_pm.c for collie. Without nested if()s, #ifdefs
> can be added. 

I'm unconvinced as to why collie needs an ifdef in there and looking at
what I think you're leading to, its ugly. Perhaps you could change the 2
to a variable set by the machine instead or something, depending upon
your intention.

Rather than post these patches straight to the patch system, perhaps you
could also post them for discussion first as discussing them once
they're submitted seems wrong (and we have to remember to remove the
patch system from the cc).

> Also warn users about charging in unsuitable
> temperature.

I'm ok with that bit.

Richard

Re: [patch] sharpsl_pm refactor

[Pavel Machek]

Hi!

> > This prepares sharpsl_pm.c for collie. Without nested if()s, #ifdefs
> > can be added. 
> 
> I'm unconvinced as to why collie needs an ifdef in there and looking at
> what I think you're leading to, its ugly. Perhaps you could change the 2
> to a variable set by the machine instead or something, depending upon
> your intention.

Well, I hate the if/else maze -- IMO returns are more readable. Anyway
collie needs both count and time checks disabled, AFAICT.

> Rather than post these patches straight to the patch system, perhaps you
> could also post them for discussion first as discussing them once
> they're submitted seems wrong (and we have to remember to remove the
> patch system from the cc).

Sorry about that. Yep, I'm used to work with akpm...
							Pavel
-- 
Thanks for all the (sleeping) penguins.

Re: [patch] sharpsl_pm refactor

[Richard Purdie]

On Fri, 2006-07-07 at 14:01 +0000, Pavel Machek wrote:
> > I'm unconvinced as to why collie needs an ifdef in there and looking at
> > what I think you're leading to, its ugly. Perhaps you could change the 2
> > to a variable set by the machine instead or something, depending upon
> > your intention.
> 
> Well, I hate the if/else maze -- IMO returns are more readable. Anyway
> collie needs both count and time checks disabled, AFAICT.

To me it looks much worse after you changed it as I can understand it at
the moment and afterwards with the ifdefs in, I can't.

Ignoring that issue, why does collie need them disabled? Do they break
collie somehow or is this just because the sharp driver didn't do it?

I'd prefer to keep the charging techniques the same across as many of
the devices as we can and I can't see how this technique causes a
problem. The charging hardware and the battery is very similar across
the models (although you wouldn't believe it looking at the charging
driver).

Richard

Re: [patch] sharpsl_pm refactor

[Pavel Machek]

Hi!

> > > I'm unconvinced as to why collie needs an ifdef in there and looking at
> > > what I think you're leading to, its ugly. Perhaps you could change the 2
> > > to a variable set by the machine instead or something, depending upon
> > > your intention.
> > 
> > Well, I hate the if/else maze -- IMO returns are more readable. Anyway
> > collie needs both count and time checks disabled, AFAICT.
> 
> To me it looks much worse after you changed it as I can understand it at
> the moment and afterwards with the ifdefs in, I can't.

Well, maybe you'll not get the ifdefs after all... They were just
handy in my tree and code with returns (vs. code with if/else maze)
looked better to me.

> Ignoring that issue, why does collie need them disabled? Do they break
> collie somehow or is this just because the sharp driver didn't do
> it?

Sharp driver did not do it, and forcing charge 3 times when charger
tells me that it is done seems a bit cruel. What is worse, if I get
charge-too-fast timeout too long, it will keep charging battery
over-and-over-and-over-and-over. 

> I'd prefer to keep the charging techniques the same across as many of
> the devices as we can and I can't see how this technique causes a
> problem. The charging hardware and the battery is very similar across
> the models (although you wouldn't believe it looking at the charging
> driver).

Okay, you may be right here. I am just trying to be careful -- hot
lithium scares me a bit ;-).
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html