[PATCH/RFC] via-pmu: remove mdelays from suspend/resume code

[PATCH/RFC] via-pmu: remove mdelays from suspend/resume code

[Johannes Berg]

I don't see a reason for either of those mdelay()s, is there any? Works
fine for me without them...

johannes
---
 drivers/macintosh/via-pmu.c |   20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

--- everything.orig/drivers/macintosh/via-pmu.c	2008-04-03 17:27:29.000000000 +0200
+++ everything/drivers/macintosh/via-pmu.c	2008-04-03 17:28:23.000000000 +0200
@@ -2165,8 +2165,6 @@ static void pmac_suspend_disable_irqs(vo
 
 static int powerbook_sleep(suspend_state_t state)
 {
-	int error = 0;
-
 	/* Wait for completion of async requests */
 	while (!batt_req.complete)
 		pmu_poll();
@@ -2183,25 +2181,15 @@ static int powerbook_sleep(suspend_state
 
 	switch (pmu_kind) {
 	case PMU_OHARE_BASED:
-		error = powerbook_sleep_3400();
-		break;
+		return powerbook_sleep_3400();
 	case PMU_HEATHROW_BASED:
 	case PMU_PADDINGTON_BASED:
-		error = powerbook_sleep_grackle();
-		break;
+		return powerbook_sleep_grackle();
 	case PMU_KEYLARGO_BASED:
-		error = powerbook_sleep_Core99();
-		break;
+		return powerbook_sleep_Core99();
 	default:
 		return -ENOSYS;
 	}
-
-	if (error)
-		return error;
-
-	mdelay(100);
-
-	return 0;
 }
 
 static void pmac_suspend_enable_irqs(void)
@@ -2210,8 +2198,6 @@ static void pmac_suspend_enable_irqs(voi
 	adb_int_pending = 1;
 	via_pmu_interrupt(0, NULL);
 
-	mdelay(10);
-
 	/* Call platform functions marked "on wake" */
 	pmac_pfunc_base_resume();
 	pmac_pfunc_i2c_resume();

Re: [PATCH/RFC] via-pmu: remove mdelays from suspend/resume code

[Brad Boyer]

On Thu, Apr 03, 2008 at 05:44:44PM +0200, Johannes Berg wrote:
> I don't see a reason for either of those mdelay()s, is there any? Works
> fine for me without them...

Which hardware revisions did you test? I suspect the mdelay calls were
added to work around timing issues in one of the older PMU chips. Some
of them are very timing sensitive, and sleep in particular is basically
a pile of steps that got hacked until they worked on older systems.

	Brad Boyer
	flar@allandria.com

Re: [PATCH/RFC] via-pmu: remove mdelays from suspend/resume code

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 824 bytes --]


On Fri, 2008-04-04 at 10:20 -0700, Brad Boyer wrote:
> On Thu, Apr 03, 2008 at 05:44:44PM +0200, Johannes Berg wrote:
> > I don't see a reason for either of those mdelay()s, is there any? Works
> > fine for me without them...
> 
> Which hardware revisions did you test? I suspect the mdelay calls were
> added to work around timing issues in one of the older PMU chips. Some
> of them are very timing sensitive, and sleep in particular is basically
> a pile of steps that got hacked until they worked on older systems.

Mine's a 5,6 powerbook with PMU firmware 0x0c. The mdelay(100) seems
fairly large though for a point where we're out of pmu code already, and
the other one doesn't really seem too pmu related anyway. In any case,
just wanted to float that, don't see a particular need for it.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH/RFC] via-pmu: remove mdelays from suspend/resume code

[Benjamin Herrenschmidt]

On Fri, 2008-04-04 at 21:21 +0200, Johannes Berg wrote:
> On Fri, 2008-04-04 at 10:20 -0700, Brad Boyer wrote:
> > On Thu, Apr 03, 2008 at 05:44:44PM +0200, Johannes Berg wrote:
> > > I don't see a reason for either of those mdelay()s, is there any? Works
> > > fine for me without them...
> > 
> > Which hardware revisions did you test? I suspect the mdelay calls were
> > added to work around timing issues in one of the older PMU chips. Some
> > of them are very timing sensitive, and sleep in particular is basically
> > a pile of steps that got hacked until they worked on older systems.
> 
> Mine's a 5,6 powerbook with PMU firmware 0x0c. The mdelay(100) seems
> fairly large though for a point where we're out of pmu code already, and
> the other one doesn't really seem too pmu related anyway. In any case,
> just wanted to float that, don't see a particular need for it.

One of those was meant to let devices settle. The reason is that when
waking from sleep, a lot of HW on the mobo is actually powered up (it
was down during sleep). This delay comes from half paranoia / half
experience, and is meant to ensure all those chips got a chance to
settle and complete their power-on reset sequence before we start
banging all over the place.

Cheers,
Ben.