CINXE.COM
LKML: Ciprian Marian Costea: Re: [PATCH v6 2/4] rtc: s32g: add NXP S32G2/S32G3 SoC support
<?xml version="1.0" encoding="UTF-8" standalone="yes"?> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"><html xmlns="http://www.w3.org/1999/xhtml"><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /><title>LKML: Ciprian Marian Costea: Re: [PATCH v6 2/4] rtc: s32g: add NXP S32G2/S32G3 SoC support</title><link href="/css/message.css" rel="stylesheet" type="text/css" /><link href="/css/wrap.css" rel="alternate stylesheet" type="text/css" title="wrap" /><link href="/css/nowrap.css" rel="stylesheet" type="text/css" title="nowrap" /><link href="/favicon.ico" rel="shortcut icon" /><script src="/js/simple-calendar.js" type="text/javascript"></script><script src="/js/styleswitcher.js" type="text/javascript"></script><link rel="alternate" type="application/rss+xml" title="lkml.org : last 100 messages" href="/rss.php" /><link rel="alternate" type="application/rss+xml" title="lkml.org : last messages by Ciprian Marian Costea" href="/groupie.php?aid=" /><!--Matomo--><script> var _paq = window._paq = window._paq || []; /* tracker methods like "setCustomDimension" should be called before "trackPageView" */ _paq.push(["setDoNotTrack", true]); _paq.push(["disableCookies"]); _paq.push(['trackPageView']); _paq.push(['enableLinkTracking']); (function() { var u="//m.lkml.org/"; _paq.push(['setTrackerUrl', u+'matomo.php']); _paq.push(['setSiteId', '1']); var d=document, g=d.createElement('script'), s=d.getElementsByTagName('script')[0]; g.async=true; g.src=u+'matomo.js'; s.parentNode.insertBefore(g,s); })(); </script><!--End Matomo Code--></head><body onload="es.jasper.simpleCalendar.init();" itemscope="itemscope" itemtype="http://schema.org/BlogPosting"><table border="0" cellpadding="0" cellspacing="0"><tr><td width="180" align="center"><a href="/"><img style="border:0;width:135px;height:32px" src="/images/toprowlk.gif" alt="lkml.org" /></a></td><td width="32">聽</td><td class="nb"><div><a class="nb" href="/lkml"> [lkml]</a> 聽 <a class="nb" href="/lkml/2025"> [2025]</a> 聽 <a class="nb" href="/lkml/2025/2"> [Feb]</a> 聽 <a class="nb" href="/lkml/2025/2/6"> [6]</a> 聽 <a class="nb" href="/lkml/last100"> [last100]</a> 聽 <a href="/rss.php"><img src="/images/rss-or.gif" border="0" alt="RSS Feed" /></a></div><div>Views: <a href="#" class="nowrap" onclick="setActiveStyleSheet('wrap');return false;">[wrap]</a><a href="#" class="wrap" onclick="setActiveStyleSheet('nowrap');return false;">[no wrap]</a> 聽 <a class="nb" href="/lkml/mheaders/2025/2/6/528" onclick="this.href='/lkml/headers'+'/2025/2/6/528';">[headers]</a>聽 <a href="/lkml/bounce/2025/2/6/528">[forward]</a>聽 </div></td><td width="32">聽</td></tr><tr><td valign="top"><div class="es-jasper-simpleCalendar" baseurl="/lkml/"></div><div class="threadlist">Messages in this thread</div><ul class="threadlist"><li class="root"><a href="/lkml/2024/12/6/275">First message in thread</a></li><li><a href="/lkml/2024/12/6/277">Ciprian Costea</a><ul><li><a href="/lkml/2024/12/6/330">"Arnd Bergmann"</a><ul><li><a href="/lkml/2024/12/6/734">Ciprian Marian Costea</a><ul><li><a href="/lkml/2024/12/6/777">"Arnd Bergmann"</a><ul><li><a href="/lkml/2024/12/9/1479">Ciprian Marian Costea</a></li></ul></li></ul></li></ul></li><li><a href="/lkml/2024/12/10/239">kernel test robot</a></li><li><a href="/lkml/2024/12/11/59">Alexandre Belloni</a><ul><li class="origin"><a href="">Ciprian Marian Costea</a></li></ul></li></ul></li></ul></td><td width="32" rowspan="2" class="c" valign="top"><img src="/images/icornerl.gif" width="32" height="32" alt="/" /></td><td class="c" rowspan="2" valign="top" style="padding-top: 1em"><table><tr><td><table><tr><td class="lp">Date</td><td class="rp" itemprop="datePublished">Thu, 6 Feb 2025 12:36:02 +0200</td></tr><tr><td class="lp">Subject</td><td class="rp" itemprop="name">Re: [PATCH v6 2/4] rtc: s32g: add NXP S32G2/S32G3 SoC support</td></tr><tr><td class="lp">From</td><td class="rp" itemprop="author">Ciprian Marian Costea <></td></tr></table></td><td></td></tr></table><pre itemprop="articleBody">On 12/11/2024 1:25 AM, Alexandre Belloni wrote:<br /><br />Hello Alexandre,<br /><br />Thank you for your detailed review on this driver.<br />Sorry for responding to this review so late, I've just now found some <br />time to continue work on this RTC driver.<br /><br />> On 06/12/2024 09:09:53+0200, Ciprian Costea wrote:<br />>> +/*<br />>> + * S32G2 and S32G3 SoCs have RTC clock source1 reserved and<br />>> + * should not be used.<br />>> + */<br />>> +#define RTC_CLK_SRC1_RESERVED BIT(1)<br />>> +<br />>> +enum {<br />>> + DIV1 = 1,<br />>> + DIV32 = 32,<br />>> + DIV512 = 512,<br />>> + DIV512_32 = 16384<br />>> +};<br />>> +<br />>> +static const char *rtc_clk_src[RTC_CLK_MUX_SIZE] = {<br />>> + "source0",<br />>> + "source1",<br />>> + "source2",<br />>> + "source3"<br />>> +};<br />>> +<br />>> +struct rtc_time_base {<br />>> + s64 sec;<br />>> + u64 cycles;<br />>> + struct rtc_time tm;<br />> > I don't think storing an rtc_time is necessary. I don't even think<br />> cycles is necessary.<br />> <br /><br />Indeed, switching to usage of just APIVAL (as you've suggested) instead <br />of relying also on RTCVAL would make 'rtc_time' redundant. Will remove.<br /><br />>> +};<br />>> +<br />>> +struct rtc_priv {<br />>> + struct rtc_device *rdev;<br />>> + void __iomem *rtc_base;<br />>> + struct clk *ipg;<br />>> + struct clk *clk_src;<br />>> + const struct rtc_soc_data *rtc_data;<br />>> + struct rtc_time_base base;<br />>> + u64 rtc_hz;<br />>> + int irq;<br />>> + int clk_src_idx;<br />>> +};<br />>> +<br />>> +struct rtc_soc_data {<br />>> + u32 clk_div;<br />>> + u32 reserved_clk_mask;<br />>> +};<br />>> +<br />>> +static const struct rtc_soc_data rtc_s32g2_data = {<br />>> + .clk_div = DIV512,<br />> <br />> If you input clock rate is higher that 16kHz, why don't you divide by<br />> 16384?<br />> <br /><br />Yes, the default input clock rate is ~32KHz. I will enable both divisors <br />to increase the RTC counter resolution.<br /><br />>> + .reserved_clk_mask = RTC_CLK_SRC1_RESERVED,<br />>> +};<br />>> +<br />>> +static u64 cycles_to_sec(u64 hz, u64 cycles)<br />>> +{<br />>> + return div_u64(cycles, hz);<br />>> +}<br />>> +<br />>> +/**<br />>> + * sec_to_rtcval - Convert a number of seconds to a value suitable for<br />>> + * RTCVAL in our clock's<br />>> + * current configuration.<br />>> + * @priv: Pointer to the 'rtc_priv' structure<br />>> + * @seconds: Number of seconds to convert<br />>> + * @rtcval: The value to go into RTCVAL[RTCVAL]<br />>> + *<br />>> + * Return: 0 for success, -EINVAL if @seconds push the counter past the<br />>> + * 32bit register range<br />>> + */<br />>> +static int sec_to_rtcval(const struct rtc_priv *priv,<br />>> + unsigned long seconds, u32 *rtcval)<br />>> +{<br />>> + u32 delta_cnt;<br />>> +<br />>> + if (!seconds || seconds > cycles_to_sec(priv->rtc_hz, RTCCNT_MAX_VAL))<br />>> + return -EINVAL;<br />>> +<br />>> + /*<br />>> + * RTCCNT is read-only; we must return a value relative to the<br />>> + * current value of the counter (and hope we don't linger around<br />>> + * too much before we get to enable the interrupt)<br />>> + */<br />>> + delta_cnt = seconds * priv->rtc_hz;<br />>> + *rtcval = delta_cnt + ioread32(priv->rtc_base + RTCCNT_OFFSET);<br />>> +<br />>> + return 0;<br />>> +}<br />>> +<br />>> +static irqreturn_t s32g_rtc_handler(int irq, void *dev)<br />>> +{<br />>> + struct rtc_priv *priv = platform_get_drvdata(dev);<br />>> + u32 status;<br />>> +<br />>> + status = ioread32(priv->rtc_base + RTCS_OFFSET);<br />>> +<br />>> + if (status & RTCS_RTCF) {<br />>> + iowrite32(0x0, priv->rtc_base + RTCVAL_OFFSET);<br />>> + iowrite32(status | RTCS_RTCF, priv->rtc_base + RTCS_OFFSET);<br />>> + rtc_update_irq(priv->rdev, 1, RTC_AF);<br />>> + }<br />>> +<br />>> + if (status & RTCS_APIF) {<br />>> + iowrite32(status | RTCS_APIF, priv->rtc_base + RTCS_OFFSET);<br />>> + rtc_update_irq(priv->rdev, 1, RTC_PF);<br />> <br />> I don't think you use APIF as a periodic interrupt so it doesn't really<br />> make sense to use RTC_PF instead of RTC_AF.<br />> <br /><br />Correct. I will change to `rtc_update_irq(priv->rdev, 1, RTC_IRQF | <br />RTC_AF);`<br /><br />>> + }<br />>> +<br />>> + return IRQ_HANDLED;<br />>> +}<br />>> +<br />>> +static s64 s32g_rtc_get_time_or_alrm(struct rtc_priv *priv,<br />>> + u32 offset)<br />>> +{<br />>> + u32 counter;<br />>> +<br />>> + counter = ioread32(priv->rtc_base + offset);<br />>> +<br />>> + if (counter < priv->base.cycles)<br />>> + return -EINVAL;<br />>> +<br />>> + counter -= priv->base.cycles;<br />>> +<br />>> + return priv->base.sec + cycles_to_sec(priv->rtc_hz, counter);<br />>> +}<br />>> +<br />>> +static int s32g_rtc_read_time(struct device *dev,<br />>> + struct rtc_time *tm)<br />>> +{<br />>> + struct rtc_priv *priv = dev_get_drvdata(dev);<br />>> + s64 sec;<br />>> +<br />>> + sec = s32g_rtc_get_time_or_alrm(priv, RTCCNT_OFFSET);<br />>> + if (sec < 0)<br />>> + return -EINVAL;<br />>> +<br />>> + rtc_time64_to_tm(sec, tm);<br />>> +<br />>> + return 0;<br />>> +}<br />>> +<br />>> +static int s32g_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)<br />>> +{<br />>> + struct rtc_priv *priv = dev_get_drvdata(dev);<br />>> + u32 rtcc, rtccnt, rtcval;<br />>> + s64 sec;<br />>> +<br />>> + sec = s32g_rtc_get_time_or_alrm(priv, RTCVAL_OFFSET);<br />>> + if (sec < 0)<br />>> + return -EINVAL;<br />>> +<br />>> + rtc_time64_to_tm(sec, &alrm->time);<br />>> +<br />>> + rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);<br />>> + alrm->enabled = sec && (rtcc & RTCC_RTCIE);<br />>> +<br />>> + alrm->pending = 0;<br />>> + if (alrm->enabled) {<br />>> + rtccnt = ioread32(priv->rtc_base + RTCCNT_OFFSET);<br />>> + rtcval = ioread32(priv->rtc_base + RTCVAL_OFFSET);<br />>> +<br />>> + if (rtccnt < rtcval)<br />>> + alrm->pending = 1;<br />> <br />> This limits the range of your alarm, why don't you simply check whether<br />> RTCF is set?<br />> <br /><br />Nice idea. I will refactor in V7.<br /><br />>> + }<br />>> +<br />>> + return 0;<br />>> +}<br />>> +<br />>> +static int s32g_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)<br />>> +{<br />>> + struct rtc_priv *priv = dev_get_drvdata(dev);<br />>> + u32 rtcc;<br />>> +<br />>> + if (!priv->irq)<br />>> + return -EIO;<br />> <br />> <br />> This will never happen as you are not letting probe finish when you<br />> can't request the irq.<br />> ><br /><br />Correct. I will remove this redundant check.<br /><br />>> +<br />>> + rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);<br />>> + if (enabled)<br />>> + rtcc |= RTCC_RTCIE;<br />>> +<br />>> + iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);<br />>> +<br />>> + return 0;<br />>> +}<br />>> +<br />>> +static int s32g_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)<br />>> +{<br />>> + struct rtc_priv *priv = dev_get_drvdata(dev);<br />>> + struct rtc_time time_crt;<br />>> + long long t_crt, t_alrm;<br />>> + u32 rtcval, rtcs;<br />>> + int ret = 0;<br />>> +<br />>> + iowrite32(0x0, priv->rtc_base + RTCVAL_OFFSET);<br />>> +<br />>> + t_alrm = rtc_tm_to_time64(&alrm->time);<br />>> +<br />>> + /*<br />>> + * Assuming the alarm is being set relative to the same time<br />>> + * returned by our s32g_rtc_read_time callback<br />>> + */<br />>> + ret = s32g_rtc_read_time(dev, &time_crt);<br />>> + if (ret)<br />>> + return ret;<br />>> +<br />>> + t_crt = rtc_tm_to_time64(&time_crt);<br />>> + ret = sec_to_rtcval(priv, t_alrm - t_crt, &rtcval);<br />>> + if (ret) {<br />>> + dev_warn(dev, "Alarm is set too far in the future\n");<br />>> + return -ERANGE;<br />>> + }<br />>> +<br />>> + ret = read_poll_timeout(ioread32, rtcs, !(rtcs & RTCS_INV_RTC),<br />>> + 0, RTC_SYNCH_TIMEOUT, false, priv->rtc_base + RTCS_OFFSET);<br />>> + if (ret)<br />>> + return ret;<br />>> +<br />>> + iowrite32(rtcval, priv->rtc_base + RTCVAL_OFFSET);<br />>> +<br />>> + return 0;<br />>> +}<br />>> +<br />>> +static int s32g_rtc_set_time(struct device *dev,<br />>> + struct rtc_time *time)<br />>> +{<br />>> + struct rtc_priv *priv = dev_get_drvdata(dev);<br />>> +<br />>> + priv->base.cycles = ioread32(priv->rtc_base + RTCCNT_OFFSET);<br />>> + priv->base.sec = rtc_tm_to_time64(time);<br />>> +<br />> <br />> To simplify all the calculations you are doing, I suggest you reset<br />> RTCCNT here and store the epoch of the rtc as a number of seconds.<br />> <br />> This wll allow you to avoid having to read the counter in set_alarm<br />> also, you then get a direct conversion for RTCVAL as this will simply be<br />> rtc_tm_to_time64(&alrm->time) - epoch that you have to convert in cycles<br />> <br />> You will also then know right away whether this is too large to fit in a<br />> 32bit register.<br />> <br />> <br /><br />Unfortunatelly, the RTCCNT register is not writable. Hence it cannot be <br />reset. The only way to reset it would be to disable & enable the RTC <br />module via 'RTCC_CNTEN' which would not be acceptable in this callback <br />and in the end it is something to be avoided.<br /><br />Nevertheless, I believe by using just APIVAL (as you've suggested) <br />instead of relying also on RTCVAL would greatly reduce the complexity of <br />this driver. I will refactor in V7.<br /><br />>> + return 0;<br />>> +}<br />>> +<br />>> +/*<br />>> + * Disable the 32-bit free running counter.<br />>> + * This allows Clock Source and Divisors selection<br />>> + * to be performed without causing synchronization issues.<br />>> + */<br />>> +static void s32g_rtc_disable(struct rtc_priv *priv)<br />>> +{<br />>> + u32 rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);<br />>> +<br />>> + rtcc &= ~RTCC_CNTEN;<br />>> + iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);<br />>> +}<br />>> +<br />>> +static void s32g_rtc_enable(struct rtc_priv *priv)<br />>> +{<br />>> + u32 rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);<br />>> +<br />>> + rtcc |= RTCC_CNTEN;<br />>> + iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);<br />>> +}<br />>> +<br />>> +static int rtc_clk_src_setup(struct rtc_priv *priv)<br />>> +{<br />>> + u32 rtcc = 0;<br />>> +<br />>> + if (priv->rtc_data->reserved_clk_mask & (1 << priv->clk_src_idx))<br />>> + return -EOPNOTSUPP;<br />>> +<br />>> + rtcc = FIELD_PREP(RTCC_CLKSEL_MASK, priv->clk_src_idx);<br />>> +<br />>> + switch (priv->rtc_data->clk_div) {<br />>> + case DIV512_32:<br />>> + rtcc |= RTCC_DIV512EN;<br />>> + rtcc |= RTCC_DIV32EN;<br />>> + break;<br />>> + case DIV512:<br />>> + rtcc |= RTCC_DIV512EN;<br />>> + break;<br />>> + case DIV32:<br />>> + rtcc |= RTCC_DIV32EN;<br />>> + break;<br />>> + case DIV1:<br />>> + break;<br />>> + default:<br />>> + return -EINVAL;<br />>> + }<br />>> +<br />>> + rtcc |= RTCC_RTCIE;<br />>> + /*<br />>> + * Make sure the CNTEN is 0 before we configure<br />>> + * the clock source and dividers.<br />>> + */<br />>> + s32g_rtc_disable(priv);<br />>> + iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);<br />>> + s32g_rtc_enable(priv);<br />>> +<br />>> + return 0;<br />>> +}<br />>> +<br />>> +static const struct rtc_class_ops rtc_ops = {<br />>> + .read_time = s32g_rtc_read_time,<br />>> + .set_time = s32g_rtc_set_time,<br />>> + .read_alarm = s32g_rtc_read_alarm,<br />>> + .set_alarm = s32g_rtc_set_alarm,<br />>> + .alarm_irq_enable = s32g_rtc_alarm_irq_enable,<br />>> +};<br />>> +<br />>> +static int rtc_clk_dts_setup(struct rtc_priv *priv,<br />>> + struct device *dev)<br />>> +{<br />>> + int i;<br />>> +<br />>> + priv->ipg = devm_clk_get_enabled(dev, "ipg");<br />>> + if (IS_ERR(priv->ipg))<br />>> + return dev_err_probe(dev, PTR_ERR(priv->ipg),<br />>> + "Failed to get 'ipg' clock\n");<br />>> +<br />>> + for (i = 0; i < RTC_CLK_MUX_SIZE; i++) {<br />>> + priv->clk_src = devm_clk_get_enabled(dev, rtc_clk_src[i]);<br />>> + if (!IS_ERR(priv->clk_src)) {<br />>> + priv->clk_src_idx = i;<br />>> + break;<br />>> + }<br />>> + }<br />>> +<br />>> + if (IS_ERR(priv->clk_src))<br />>> + return dev_err_probe(dev, PTR_ERR(priv->clk_src),<br />>> + "Failed to get rtc module clock source\n");<br />>> +<br />>> + return 0;<br />>> +}<br />>> +<br />>> +static int s32g_rtc_probe(struct platform_device *pdev)<br />>> +{<br />>> + struct device *dev = &pdev->dev;<br />>> + struct rtc_priv *priv;<br />>> + int ret = 0;<br />>> +<br />>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);<br />>> + if (!priv)<br />>> + return -ENOMEM;<br />>> +<br />>> + priv->rtc_data = of_device_get_match_data(dev);<br />>> + if (!priv->rtc_data)<br />>> + return -ENODEV;<br />>> +<br />>> + priv->rtc_base = devm_platform_ioremap_resource(pdev, 0);<br />>> + if (IS_ERR(priv->rtc_base))<br />>> + return PTR_ERR(priv->rtc_base);<br />>> +<br />>> + device_init_wakeup(dev, true);<br />>> +<br />>> + ret = rtc_clk_dts_setup(priv, dev);<br />>> + if (ret)<br />>> + return ret;<br />>> +<br />>> + priv->rdev = devm_rtc_allocate_device(dev);<br />>> + if (IS_ERR(priv->rdev))<br />>> + return PTR_ERR(priv->rdev);<br />>> +<br />>> + ret = rtc_clk_src_setup(priv);<br />>> + if (ret)<br />>> + return ret;<br />>> +<br />>> + priv->rtc_hz = clk_get_rate(priv->clk_src);<br />>> + if (!priv->rtc_hz) {<br />>> + dev_err(dev, "Failed to get RTC frequency\n");<br />>> + ret = -EINVAL;<br />>> + goto disable_rtc;<br />>> + }<br />>> +<br />>> + priv->rtc_hz /= priv->rtc_data->clk_div;<br />>> +<br />>> + platform_set_drvdata(pdev, priv);<br />>> + priv->rdev->ops = &rtc_ops;<br />>> +<br />>> + priv->irq = platform_get_irq(pdev, 0);<br />>> + if (priv->irq < 0) {<br />>> + ret = priv->irq;<br />>> + goto disable_rtc;<br />>> + }<br />>> +<br />>> + ret = devm_request_irq(dev, priv->irq,<br />>> + s32g_rtc_handler, 0, dev_name(dev), pdev);<br />>> + if (ret) {<br />>> + dev_err(dev, "Request interrupt %d failed, error: %d\n",<br />>> + priv->irq, ret);<br />>> + goto disable_rtc;<br />>> + }<br />>> +<br />>> + ret = devm_rtc_register_device(priv->rdev);<br />>> + if (ret)<br />>> + goto disable_rtc;<br />>> +<br />>> + return 0;<br />>> +<br />>> +disable_rtc:<br />>> + s32g_rtc_disable(priv);<br />>> + return ret;<br />>> +}<br />>> +<br />>> +static void s32g_enable_api_irq(struct device *dev, unsigned int enabled)<br />>> +{<br />>> + struct rtc_priv *priv = dev_get_drvdata(dev);<br />>> + u32 api_irq = RTCC_APIEN | RTCC_APIIE;<br />>> + u32 rtcc;<br />>> +<br />>> + rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);<br />>> + if (enabled)<br />>> + rtcc |= api_irq;<br />>> + else<br />>> + rtcc &= ~api_irq;<br />>> + iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);<br />>> +}<br />>> +<br />>> +static int s32g_rtc_suspend(struct device *dev)<br />>> +{<br />>> + struct rtc_priv *init_priv = dev_get_drvdata(dev);<br />>> + struct rtc_priv priv;<br />>> + long long base_sec;<br />>> + u32 rtcval, rtccnt, offset;<br />>> + int ret = 0;<br />>> + u32 sec;<br />>> +<br />>> + if (!device_may_wakeup(dev))<br />>> + return 0;<br />>> +<br />>> + /* Save last known timestamp */<br />>> + ret = s32g_rtc_read_time(dev, &init_priv->base.tm);<br />>> + if (ret)<br />>> + return ret;<br />> <br />> I don't think that whole calculation is necessary as you are never<br />> actually resetting RTCCNT in suspend<br />> <br /><br />The RTC module needs to be reinitialized during resume, because the RTC <br />registers are being reset during Standby/Suspend to RAM operations.<br /><br />Nevertheless, I will greatly reduce the complexity of these calculations <br />in V7.<br /><br />>> +<br />>> + /*<br />>> + * Use a local copy of the RTC control block to<br />>> + * avoid restoring it on resume path.<br />>> + */<br />>> + memcpy(&priv, init_priv, sizeof(priv));<br />>> +<br />>> + rtccnt = ioread32(init_priv->rtc_base + RTCCNT_OFFSET);<br />>> + rtcval = ioread32(init_priv->rtc_base + RTCVAL_OFFSET);<br />>> + offset = rtcval - rtccnt;<br />>> + sec = cycles_to_sec(init_priv->rtc_hz, offset);<br />>> +<br />>> + /* Adjust for the number of seconds we'll be asleep */<br />>> + base_sec = rtc_tm_to_time64(&init_priv->base.tm);<br />>> + base_sec += sec;<br />>> + rtc_time64_to_tm(base_sec, &init_priv->base.tm);<br />>> +<br />>> + ret = sec_to_rtcval(&priv, sec, &rtcval);<br />>> + if (ret) {<br />>> + dev_warn(dev, "Alarm is too far in the future\n");<br />>> + return -ERANGE;<br />>> + }<br />>> +<br />>> + s32g_enable_api_irq(dev, 1);<br />>> + iowrite32(offset, priv.rtc_base + APIVAL_OFFSET);<br />> <br />> What about always using APIVAL instead of RTCVAL so you don't have<br />> anything to do in s32g_rtc_suspend.<br />> <br /><br />This is a great idea. I will update in V7.<br /><br />> <br />>> +<br />>> + return ret;<br />>> +}<br />>> +<br />>> +static int s32g_rtc_resume(struct device *dev)<br />>> +{<br />>> + struct rtc_priv *priv = dev_get_drvdata(dev);<br />>> + int ret;<br />>> +<br />>> + if (!device_may_wakeup(dev))<br />>> + return 0;<br />>> +<br />>> + /* Disable wake-up interrupts */<br />>> + s32g_enable_api_irq(dev, 0);<br />>> +<br />>> + ret = rtc_clk_src_setup(priv);<br />>> + if (ret)<br />>> + return ret;<br />> <br />> I don't think this is necessary.<br /><br />It is, because RTC registers are being reset during Standby/Suspend to <br />RAM operations.<br /><br />>> +<br />>> + /*<br />>> + * Now RTCCNT has just been reset, and is out of sync with priv->base;<br />>> + * reapply the saved time settings.<br />>> + */<br />>> + return s32g_rtc_set_time(dev, &priv->base.tm);<br />> <br />> And so this is useless too so yo udon't actually have anything to do in<br />> s32g_rtc_resume.<br />> <br /><br />I will refactor (simplify) the entire store of time logic from suspend & <br />resume in V7.<br /><br />Regards,<br />Ciprian<br /><br />>> +}<br />>> +<br />>> +static const struct of_device_id rtc_dt_ids[] = {<br />>> + { .compatible = "nxp,s32g2-rtc", .data = &rtc_s32g2_data},<br />>> + { /* sentinel */ },<br />>> +};<br />>> +<br />>> +static DEFINE_SIMPLE_DEV_PM_OPS(s32g_rtc_pm_ops,<br />>> + s32g_rtc_suspend, s32g_rtc_resume);<br />>> +<br />>> +static struct platform_driver s32g_rtc_driver = {<br />>> + .driver = {<br />>> + .name = "s32g-rtc",<br />>> + .pm = pm_sleep_ptr(&s32g_rtc_pm_ops),<br />>> + .of_match_table = rtc_dt_ids,<br />>> + },<br />>> + .probe = s32g_rtc_probe,<br />>> +};<br />>> +module_platform_driver(s32g_rtc_driver);<br />>> +<br />>> +MODULE_AUTHOR("NXP");<br />>> +MODULE_DESCRIPTION("NXP RTC driver for S32G2/S32G3");<br />>> +MODULE_LICENSE("GPL");<br />>> -- <br />>> 2.45.2<br />>><br />> <br /><br /><br /></pre></td><td width="32" rowspan="2" class="c" valign="top"><img src="/images/icornerr.gif" width="32" height="32" alt="\" /></td></tr><tr><td align="right" valign="bottom"> 聽 </td></tr><tr><td align="right" valign="bottom">聽</td><td class="c" valign="bottom" style="padding-bottom: 0px"><img src="/images/bcornerl.gif" width="32" height="32" alt="\" /></td><td class="c">聽</td><td class="c" valign="bottom" style="padding-bottom: 0px"><img src="/images/bcornerr.gif" width="32" height="32" alt="/" /></td></tr><tr><td align="right" valign="top" colspan="2"> 聽 </td><td class="lm">Last update: 2025-02-06 11:36 聽聽 [W:0.616 / U:0.227 seconds]<br />漏2003-2020 <a href="http://blog.jasper.es/"><span itemprop="editor">Jasper Spaans</span></a>|hosted at <a href="https://www.digitalocean.com/?refcode=9a8e99d24cf9">Digital Ocean</a> and my Meterkast|<a href="http://blog.jasper.es/categories.html#lkml-ref">Read the blog</a></td><td>聽</td></tr></table><script language="javascript" src="/js/styleswitcher.js" type="text/javascript"></script></body></html>