From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AD1FDCD13CF for ; Mon, 2 Sep 2024 20:29:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=/acNWIGC7J/l+lCj9xbwfTsap0VDe5s6cnY9+w/1Cgo=; b=mqRDa5O5imFq+9y3jBOEhZOoUv uV5rrvVHbIpQhbZcFmeASYyS6f8tTP+EA4iupgbfSjLoUyn0oakc8lbgymMUTG4b9be4OrXFDiiCU Mq5njQZHFmoKYDNWnf3rJmIYDBscogPOznPtVCOQqgQCjDdHTXoCFUea/2TQdKfcdpl3GqwEQbswX n9f3Jkbrn7YMMxojAVY6zcFo7a+MZxdrt9woGa0PiS63cuqmmoZ3+WCbqNgp1apCSk7uYwIWQuGva HH5SnGlWFhOLdd4V81GhmX9Q8TkGKO5EFka7ak3i7On0zQtUAobOK2HsAc2A9f8PoV+p+G3cJJMzT eS9Vys/Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1slDfx-0000000FVFn-05Ew; Mon, 02 Sep 2024 20:29:25 +0000 Received: from relay3-d.mail.gandi.net ([217.70.183.195]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1slDey-0000000FV7a-3869 for linux-arm-kernel@lists.infradead.org; Mon, 02 Sep 2024 20:28:26 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id 0B9FA60002; Mon, 2 Sep 2024 20:28:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1725308901; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=/acNWIGC7J/l+lCj9xbwfTsap0VDe5s6cnY9+w/1Cgo=; b=F3Qc+8AXy4XVfyxeE/R7AjyxDNhZStf4Ngzq3cE/0udfpTXLq5utZIhTRlno2r5HNxDqFW 7FYsHvNI2HirrQv1DaQjwcuBZmNSDzFcz9CxhEiebQYoCy+gfOJ1k0jarbgqFu22bbaOb6 tFlKj00ASPoxjX4Lc1JOzAnRHtjPVzDuHY42HxzOtCqemhBOtE7G7Y8LOm1dL2UYQyzxKP 0AQEna6l8JcAoX7cFb43qflsYcmmYoMZs5aTfU7KEFdxqlrNUEJ654L+/DOz7negI+iw0D ctLVRyaFGnT/XT/FPCu2zeftLhyP90QGqO6j8prD0p4RtcUoLB+7bMrWA6ja5w== Date: Mon, 2 Sep 2024 22:28:19 +0200 From: Alexandre Belloni To: claudiu beznea Cc: geert+renesas@glider.be, mturquette@baylibre.com, sboyd@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, magnus.damm@gmail.com, p.zabel@pengutronix.de, linux-renesas-soc@vger.kernel.org, linux-clk@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-rtc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Claudiu Beznea , kernel test robot Subject: Re: [PATCH v3 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC Message-ID: <20240902202819e2bf5630@mail.local> References: <20240830130218.3377060-1-claudiu.beznea.uj@bp.renesas.com> <20240830130218.3377060-7-claudiu.beznea.uj@bp.renesas.com> <202408302225417622f1e7@mail.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-GND-Sasl: alexandre.belloni@bootlin.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240902_132825_072599_9494404A X-CRM114-Status: GOOD ( 31.29 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 02/09/2024 17:49:14+0300, claudiu beznea wrote: > >> + /* Disable alarm, periodic interrupts. */ > >> + rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0); > > > > Why do you disable alarms on driver remove? I think you need to add a > > comment if this is because it can't system up, else this is a bad > > practice. > > The RTC cannot power on the system after a power off. It can't also resume > it from a deep sleep state (when only the SoC area where the RTC resides > remains power on (there is no way to signal from RTC to the power supply > chain that an alarm happened)). It can only wake it up from s2idle mode > where all SoC components remains powered on. > > Also, w/o this change the RTC remains blocked under the following scenarios > if the interrupts are not disabled in remove: > > 1/ Configure wake alarm and unbind the RTC driver with the following commands: > # echo +10 > /sys/class/rtc/rtc0/wakealarm > # echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > unbind > # sleep 12 > # echo /sys/bus/platform/drivers/rtc-rtca3/1004ec00.rtc > bind > > When rebinding the re-configuration of the RTC device times out: > [ 121.854190] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup > the RTC! > [ 121.861511] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed > with error -110 > -sh: echo: write error: Connection timed out > > 2/ Configure wake alarm, unbind the RTC driver and switch to s2idle with > the following commands: > # echo s2idle > /sys/power/mem_sleep > # echo +10 > /sys/class/rtc/rtc0/wakealarm > # echo /sys/bus/platform/drivers/rtc-rtca/31004ec00.rtc > unbind > # echo mem > /sys/power/state > # #system is resumed by non RTC wakeup source (as the RTC alarm is not > working anymore in this case) > # echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > bind > > The system is not waked up from RTC alarm (as expected) and the rebinding > fails again: > > [ 172.483688] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup > the RTC! > [ 172.491003] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed > with error -110 > -sh: echo: write error: Connection timed out > > 3/ configure the RTC alarm, unbind and power off (with the following commands): > # echo +60 > /sys/class/rtc/rtc0/wakealarm > # echo /sys/bus/platform/drivers/rtc-rtca/1004ec00.rtc > unbind > # poweroff > > The system is not started after 60 seconds and at the next reboot the RTC > configuration on probe is failing the same: > > [ 0.292068] rtc-rtca3 1004ec00.rtc: error -ETIMEDOUT: Failed to setup > the RTC! > [ 0.292182] rtc-rtca3 1004ec00.rtc: probe with driver rtc-rtca3 failed > with error -110 > > In all scenarios the RTC is recovered only if removing/re-applying the > power to the SoC area where it resides. > > These tests were done with the patches in this series and then I tried it > with the following diff on top of the patches in this series. The results > were the same: > > diff --git a/drivers/rtc/rtc-renesas-rtca3.c b/drivers/rtc/rtc-renesas-rtca3.c > index 822c055b6e4d..720fdac3adc6 100644 > --- a/drivers/rtc/rtc-renesas-rtca3.c > +++ b/drivers/rtc/rtc-renesas-rtca3.c > @@ -586,7 +586,7 @@ static int rtca3_initial_setup(struct clk *clk, struct > rtca3_priv *priv) > usleep_range(sleep_us, sleep_us + 10); > > /* Disable alarm and carry interrupts. */ > - mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE; > + mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE | RTCA3_RCR1_PIE; > ret = rtca3_alarm_irq_set_helper(priv, mask, 0); > if (ret) > return ret; > @@ -784,7 +784,7 @@ static void rtca3_remove(struct platform_device *pdev) > guard(spinlock_irqsave)(&priv->lock); > > /* Disable alarm, periodic interrupts. */ > - rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0); > + //rtca3_alarm_irq_set_helper(priv, RTCA3_RCR1_AIE | RTCA3_RCR1_PIE, 0); > } Thanks for the detailed explanation. Can you add a small comment, I really want t avoid people cargo-culting this behavior as this has already been the case. -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com