Re: [REGRESSION] rtc/interface.c: kills suspend-to-ram

[John Stultz]

On 04/16/2012 07:30 PM, Mark Lord wrote:
> Thanks for looking into it, John.
>
> I also spent many more hours digging away at it here today,
> and I now understand (mostly) what is happening and why.
>
> The code above introduces a new access to the RTC that never existed before.
> For the case where the Alarm has never been enabled by software,
> I believe the code above will still try to "disable" it.
> That's the new behaviour we didn't have prior to this patch.
>
> And.. on some of the systems I'm testing on, the BIOS setup has
> the RTC Alarm "enabled", which means "under BIOS control",
> as opposed to "disabled" which means "under software control".
>
> It's the "under BIOS control" systems that the above patch breaks.
>
> So I think the code may just need to be slightly more clever,
> and not disable an Alarm that was never enabled by software in the first place.

Thanks for the extra info. Although I'm still a little perplexed why 
that's causing trouble.
When "under BIOS control" is the RTC unusable by the kernel? Will any 
access cause problems? Or just the extra disable path?

On a hunch, I wonder if your tripping over the alarmtimer initialization 
issue that was recently fixed.
Have you also seen this issue w/ 3.4-rc2+ ?

I still can't trigger anything similar playing with the BIOS options for 
my system. If its not too much trouble, could you try the following two 
changes?

thanks
-john

I guess I'm curious why you're hitting the rtc_alarm_disable if you're 
not using the alarm. If you use the following diff, can you provide the 
resulting stack traces?

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index eb415bd..4c98ee5 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -786,7 +786,8 @@ static void rtc_alarm_disable(struct rtc_device *rtc)
 	if (!rtc->ops || !rtc->ops->alarm_irq_enable)
 		return;

-	rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
+	//rtc->ops->alarm_irq_enable(rtc->dev.parent, false);
+	dump_stack();
 }

 /**




Then un-comment/re-add the alarm_irq_enable() call above, and try the 
following, to see if the behavior changes? Then re-add each line one by 
one to see if you can isolate where things go wrong in the cmos code?

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 7d5f56e..c500bce 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -318,9 +318,9 @@ static void cmos_irq_disable(struct cmos_rtc *cmos, unsigned char mask)
 	rtc_control = CMOS_READ(RTC_CONTROL);
 	rtc_control&= ~mask;
 	CMOS_WRITE(rtc_control, RTC_CONTROL);
-	hpet_mask_rtc_irq_bit(mask);
+	//hpet_mask_rtc_irq_bit(mask);

-	cmos_checkintr(cmos, rtc_control);
+	//cmos_checkintr(cmos, rtc_control);
 }

 static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)

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