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 528B0C27C75 for ; Fri, 14 Jun 2024 09:22:21 +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=Ez2HfY1x87bBiR5tU1ndmRD1HXIhQKtusmjdRjvINyc=; b=X96C5DiD8O43JN88XaV1buQa/G BTBOF0N5qQBO+CX0QUFrXbrWe6Y7W5K2mqX0xiXnhl82qBrNZSxSSM572A7hpyLco183UHsXiyDgo tWnrQNnoXSFd3px13iuxGX9rUH4ZmNC78tZxQgzXoM4ONMTxfTiQKCXO222HE/XVLESugucXn61cL D2pEq9bQRX+ndfB7Qaq2T4ct2bIqJFDqlvnX5pvlfV3riC1v0jcCYql33nE8cWjVnDuuWUTDebQf8 g4fVXBvV5Or0JDFrxGoHXLOpcf12ifjlc3xlVrN1AkAZfR6sfpqCzZo5wCJ/zFIq99xQrJdH1yDHF 6fIGUw3w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sI38J-00000002AAw-3nvw; Fri, 14 Jun 2024 09:22:07 +0000 Received: from relay7-d.mail.gandi.net ([2001:4b98:dc4:8::227]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sI38G-00000002A78-2Oau for linux-arm-kernel@lists.infradead.org; Fri, 14 Jun 2024 09:22:06 +0000 Received: by mail.gandi.net (Postfix) with ESMTPSA id 18A8020007; Fri, 14 Jun 2024 09:21:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1718356919; 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=Ez2HfY1x87bBiR5tU1ndmRD1HXIhQKtusmjdRjvINyc=; b=eddZwAv/wquEluA9pJKGZdXbksACURRWevQ1N6byNcOheTyrgeIOEKGM632e9au9gW12gQ d2IxX2j69f6CaKGOi7zMsIE/c8VglP6S/U+jZH8xfuuktleI2iYtT0rLGs1Ag4CfImo49O ykvT85mdUKaa7LHIXeElrU+UREGO8qXAi4fttIkO8dfY1mKB8PsN+mFWw9f4Hb90xtlkNw 2bd9AEFqSEIcYj7GmfcyU6uGyb7q17zQARUK2sl6jjvRKAhWKoscCXefYxXCXRGhu13Pss JH6CDhtFF+dymhpdZuKYBfpN1i80pAuQF/47Qf0w2yxgJxqB1JqrcZpL6TwwUw== Date: Fri, 14 Jun 2024 11:21:57 +0200 From: Alexandre Belloni To: Claudiu Cc: geert+renesas@glider.be, mturquette@baylibre.com, sboyd@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, lee@kernel.org, magnus.damm@gmail.com, 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 Subject: Re: [PATCH 06/12] rtc: renesas-rtca3: Add driver for RTCA-3 available on Renesas RZ/G3S SoC Message-ID: <2024061409215756e6a10c@mail.local> References: <20240614071932.1014067-1-claudiu.beznea.uj@bp.renesas.com> <20240614071932.1014067-7-claudiu.beznea.uj@bp.renesas.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240614071932.1014067-7-claudiu.beznea.uj@bp.renesas.com> X-GND-Sasl: alexandre.belloni@bootlin.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240614_022204_939479_383971E2 X-CRM114-Status: GOOD ( 23.58 ) 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 Hello Claudiu, On 14/06/2024 10:19:26+0300, Claudiu wrote: > +static int rtca3_initial_setup(struct rtca3_priv *priv) > +{ > + unsigned long osc32k_rate; > + u8 pes, tmp, mask; > + u32 sleep_us; > + int ret; > + > + osc32k_rate = clk_get_rate(priv->clk); > + if (!osc32k_rate) > + return -EINVAL; > + > + sleep_us = DIV_ROUND_UP_ULL(1000000ULL, osc32k_rate) * 6; > + > + priv->ppb.ten_sec = DIV_ROUND_CLOSEST_ULL(1000000000ULL, (osc32k_rate * 10)); > + priv->ppb.sixty_sec = DIV_ROUND_CLOSEST_ULL(1000000000ULL, (osc32k_rate * 60)); > + > + /* > + * According to HW manual (section 22.4.2. Clock and count mode setting procedure) > + * we need to wait at least 6 cycles of the 32KHz clock after clock was enabled. > + */ > + usleep_range(sleep_us, sleep_us + 10); > + > + /* Disable alarm and carry interrupts. */ > + mask = RTCA3_RCR1_AIE | RTCA3_RCR1_CIE; > + rtca3_byte_update_bits(priv, RTCA3_RCR1, mask, 0); > + ret = readb_poll_timeout(priv->base + RTCA3_RCR1, tmp, !(tmp & mask), > + 10, RTCA3_DEFAULT_TIMEOUT_US); > + if (ret) > + return ret; > + > + /* > + * Stop the RTC and set to 12 hours mode and calendar count mode. > + * RCR2.START initial value is undefined so we need to stop here > + * all the time. > + */ Certainly not, if you stop the RTC on probe, you lose the time information, this must only be done when the RTC has never been initialised. The whole goal of the RTC is the keep time across reboots, its lifecycle is longer than the system. Also, why do you insist on 12H-mode? The proper thing to do is to support 12H-mode on read but always use 24H-mode when setting the time. > + mask = RTCA3_RCR2_START | RTCA3_RCR2_HR24 | RTCA3_RCR2_CNTMD; > + writeb(0, priv->base + RTCA3_RCR2); > + ret = readb_poll_timeout(priv->base + RTCA3_RCR2, tmp, !(tmp & mask), > + 10, RTCA3_DEFAULT_TIMEOUT_US); > + if (ret) > + return ret; > + > + /* Execute reset and wait for reset and calendar count mode to be applied. */ > + mask = RTCA3_RCR2_RESET | RTCA3_RCR2_CNTMD; > + writeb(RTCA3_RCR2_RESET, priv->base + RTCA3_RCR2); > + ret = readb_poll_timeout(priv->base + RTCA3_RCR2, tmp, !(tmp & mask), > + 10, RTCA3_RESET_TIMEOUT_US); > + if (ret) > + return ret; > + > + /* > + * According to HW manual (section 22.6.3. Notes on writing to and reading > + * from registers) after reset we need to wait 6 clock cycles before > + * writing to RTC registers. > + */ > + usleep_range(sleep_us, sleep_us + 10); > + > + /* Set no adjustment. */ > + writeb(0, priv->base + RTCA3_RADJ); > + ret = readb_poll_timeout(priv->base + RTCA3_RADJ, tmp, !tmp, 10, > + RTCA3_DEFAULT_TIMEOUT_US); > + > + /* Start the RTC and enable automatic time error adjustment. */ > + mask = RTCA3_RCR2_START | RTCA3_RCR2_AADJE; > + writeb(RTCA3_RCR2_START | RTCA3_RCR2_AADJE, priv->base + RTCA3_RCR2); > + ret = readb_poll_timeout(priv->base + RTCA3_RCR2, tmp, ((tmp & mask) == mask), > + 10, RTCA3_START_TIMEOUT_US); > + if (ret) > + return ret; > + > + /* > + * According to HW manual (section 22.6.4. Notes on writing to and reading > + * from registers) we need to wait 1/128 seconds while the clock is operating > + * (RCR2.START bit = 1) to be able to read the counters after a return from > + * reset. > + */ > + usleep_range(8000, 9000); > + > + /* Set period interrupt to 1/64 seconds. It is necessary for alarm setup. */ > + pes = FIELD_PREP(RTCA3_RCR1_PES, RTCA3_RCR1_PES_1_64_SEC); > + rtca3_byte_update_bits(priv, RTCA3_RCR1, RTCA3_RCR1_PES, pes); > + return readb_poll_timeout(priv->base + RTCA3_RCR1, tmp, ((tmp & RTCA3_RCR1_PES) == pes), > + 10, RTCA3_DEFAULT_TIMEOUT_US); > +} > + > +static int rtca3_request_irqs(struct platform_device *pdev, struct rtca3_priv *priv) > +{ > + struct device *dev = &pdev->dev; > + int ret, irq; > + > + irq = platform_get_irq_byname(pdev, "alarm"); > + if (irq < 0) > + return dev_err_probe(dev, irq, "Failed to get alarm IRQ!\n"); > + > + ret = devm_request_irq(dev, irq, rtca3_alarm_handler, 0, "rtca3-alarm", priv); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to request alarm IRQ!\n"); > + priv->wakeup_irq = irq; > + > + irq = platform_get_irq_byname(pdev, "period"); > + if (irq < 0) > + return dev_err_probe(dev, irq, "Failed to get period IRQ!\n"); > + > + ret = devm_request_irq(dev, irq, rtca3_periodic_handler, 0, "rtca3-period", priv); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to request period IRQ!\n"); > + > + /* > + * Driver doesn't implement carry handler. Just get the IRQ here > + * for backward compatibility, in case carry support will be added later. > + */ > + irq = platform_get_irq_byname(pdev, "carry"); > + if (irq < 0) > + return dev_err_probe(dev, irq, "Failed to get carry IRQ!\n"); > + > + return 0; > +} > + > +static int rtca3_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct rtca3_priv *priv; > + int ret; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + priv->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(priv->base)) > + return PTR_ERR(priv->base); > + > + priv->clk = devm_clk_get_enabled(dev, "counter"); > + if (IS_ERR(priv->clk)) > + return PTR_ERR(priv->clk); > + > + platform_set_drvdata(pdev, priv); > + > + spin_lock_init(&priv->lock); > + atomic_set(&priv->alrm_sstep, RTCA3_ALRM_SSTEP_DONE); > + init_completion(&priv->set_alarm_completion); > + > + ret = rtca3_initial_setup(priv); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to setup the RTC!\n"); > + > + ret = rtca3_request_irqs(pdev, priv); > + if (ret) > + return ret; > + > + device_init_wakeup(&pdev->dev, 1); > + > + priv->rtc_dev = devm_rtc_allocate_device(&pdev->dev); > + if (IS_ERR(priv->rtc_dev)) > + return PTR_ERR(priv->rtc_dev); > + > + priv->rtc_dev->ops = &rtca3_ops; > + priv->rtc_dev->max_user_freq = 256; > + priv->rtc_dev->range_min = mktime64(1999, 1, 1, 0, 0, 0); > + priv->rtc_dev->range_max = mktime64(2098, 12, 31, 23, 59, 59); This very much looks like the range should be 2000 to 2099, why do you want to shift it? -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com