Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <20ad397b7975775d69d6c0ea902ca362fa3cf395.camel@intel.com>
Subject: Re: [PATCH 7/7] rtc: cmos: Add suspend/resume endurance testing hook
From:   Zhang Rui <rui.zhang@intel.com>
To:     "Rafael J. Wysocki" <rafael@kernel.org>
Cc:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Kalle Valo <kvalo@kernel.org>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Linux PM <linux-pm@vger.kernel.org>,
        ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
        linux-rtc@vger.kernel.org,
        "open list:NETWORKING DRIVERS (WIRELESS)" 
        <linux-wireless@vger.kernel.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        merez@codeaurora.org, mat.jonczyk@o2.pl,
        Sumeet Pawnikar <sumeet.r.pawnikar@intel.com>,
        Len Brown <len.brown@intel.com>
Date:   Thu, 19 May 2022 00:07:12 +0800
In-Reply-To: <CAJZ5v0h=pYZkbhN2EiYzUGn36Q4-2tMyzfUP0uyFO=Sybse4DA@mail.gmail.com>
References: <20220505015814.3727692-1-rui.zhang@intel.com>
         <20220505015814.3727692-8-rui.zhang@intel.com>
         <CAJZ5v0jt1OND_d08mC0TC1LZ-JGANDY5fiDmH5RUfdtRk1vZFw@mail.gmail.com>
         <2dc4aa933d07add206a2aeefa15a4837aca6ff62.camel@intel.com>
         <CAJZ5v0h=pYZkbhN2EiYzUGn36Q4-2tMyzfUP0uyFO=Sybse4DA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2022-05-18 at 17:02 +0200, Rafael J. Wysocki wrote:
> On Wed, May 18, 2022 at 4:45 PM Zhang Rui <rui.zhang@intel.com>
> wrote:
> > 
> > On Tue, 2022-05-17 at 17:14 +0200, Rafael J. Wysocki wrote:
> > > On Thu, May 5, 2022 at 3:58 AM Zhang Rui <rui.zhang@intel.com>
> > > wrote:
> > > > 
> > > > Automated suspend/resume testing uses the RTC for wakeup.
> > > > A short rtcwake period is desirable, so that more
> > > > suspend/resume
> > > > cycles can be completed, while the machine is available for
> > > > testing.
> > > > 
> > > > But if too short a wake interval is specified, the event can
> > > > occur,
> > > > while still suspending, and then no event wakes the suspended
> > > > system
> > > > until the user notices that testing has stalled, and manually
> > > > intervenes.
> > > 
> > > If the wakeup event occurs while still suspending, it should
> > > abort
> > > the
> > > suspend in progress, shouldn't it?  But the above implies that it
> > > doesn't do that.
> > > 
> > > If this is fixed, wouldn't it address the issue at hand?
> > 
> > I think the rootcause of the original problem is that
> > 1. on some systems, the ACPI RTC Fixed event is used during suspend
> > only, and the ACPI Fixed event is enabled in the rtc-cmos driver
> > .suspend() callback
> > and
> > 2. if the RTC Alarm already expires before .suspend() invoked, we
> > will
> > lose the ACPI RTC Fixed Event as well as the wakeup event, say 20
> > seconds delay in freeze processes.
> 
> Well, the RTC Fixed event can be armed in a PM/HIBERNATE notifier and
> if it fires before .suspend() runs, system wakeup can be triggered
> from there.

Agreed.

Len,
Do you recall any other case that we may miss the RTC wakeup event?

> 
> > But, even if that problem is fixed, the suspend aborts and "fails"
> > as
> > expected, this is still a problem for the suspend-automation
> > scenario,
> > because the system actually can suspend successfully if we don't
> > set
> > the RTC alarm too aggressively. And in PCH overheating case, surely
> > we
> > will get false alarms, because we will never use a 60s+ rtc alarm
> > for
> > suspend-automation.
> 
> I'm not sure why this is a problem.
> 
> It only means that occasionally the system will not reach the final
> "suspended" state, but that can happen regardless.

It is not a kernel problem.
It is a problem for suspend-automation. Because suspend-automation is
chasing for kernel suspend problems, and IMO, cases like suspend aborts
because of long suspend delay from PCH thermal driver are not kernel
problems.

It would be nice to leverage a kernel I/F to get rid of such issues, 
But if the patch is rejected, I agree we can live without it.

thanks,
rui