From patchwork Fri Aug 12 09:56:04 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kegl Rohit X-Patchwork-Id: 597263 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 48EE8C25B0F for ; Fri, 12 Aug 2022 09:56:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236378AbiHLJ40 (ORCPT ); Fri, 12 Aug 2022 05:56:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45678 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237736AbiHLJ4S (ORCPT ); Fri, 12 Aug 2022 05:56:18 -0400 Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 26BA95FAE8 for ; Fri, 12 Aug 2022 02:56:17 -0700 (PDT) Received: by mail-wr1-x434.google.com with SMTP id j1so648202wrw.1 for ; Fri, 12 Aug 2022 02:56:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:subject:message-id:date:from:mime-version:from:to:cc; bh=hrJUt/lixuN/k6zIilPZKZz5TtcpiyzojSORFs5bM4Y=; b=mF9fvL2lgIF67cRhb2D0TLxM9I7hJDlOq9IUmgPMvERj22qVGlDgoQofcVnDbsEZoD 16f33bSENWhzqhMNe5W0NPsYhXGfAXX61PfKp/LpGj7XVqJLhsuX9GSEOoTbTBb6YBJ5 fTnJ9YjrDENwqJheHypj0BcjIZ/uYAyuz1mIUa+QTb4koaZlFXECbGpY7Dv7Crv5TjCF /1Suuf1BrvhAui3iADczKzQYUDlJsiB+pIJE1jCYRzfX1pzQQJ66howPn1SZz5wjZypU R/mPcoiABGrXDp7nQVaxuYslwLPgzRTTXWZI6fYCMQo5DZvs/N+AhgYWxl7Pq1TRLn9p 7jIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc; bh=hrJUt/lixuN/k6zIilPZKZz5TtcpiyzojSORFs5bM4Y=; b=7c69gbbAnKZ7mTj4YbguP668qjUzFPh8UNbPduDNnFglRI7wbGnrpuFvQy9+ebUq1W CNJewyapNboRTqIlWgbUk7WV8FTK+vIhlH7oZFXNvee9xtqpAAj9Bq4tXjj7X8uNLqGF /KOFZyXCOkF/pMkorWE13zff0aMq/oLBKAQ4O0gHwHhIDDXGHSQ4FUpJm1CvL8RWr/4v f6MmUkogslXFagOWPkN3lpFe3VvyzR2o+NiLrl1prkwHlzf6xmeWP+t/TsWOSse8UKLV 22V/8gjtfKzG5XTqr1cKquacynNMq+TTASHHkDmsppnn5afuMPEbLj3NbdsI7puhjuFn wk5g== X-Gm-Message-State: ACgBeo1MbuToIBwb454wkej3yr9FKYkzxxQKUlmndwtsYcnay36TYavZ ULn6BkmJxzNpYHKerw5gzZmJcZNjcsYQZQLt/AXbcUN1rtY= X-Google-Smtp-Source: AA6agR4yMEB9AC8N5e9QE/qE/rQzoFeZp4azaV+61BeTxb2xDEsRMjQZSilkjJWUs+EsLFusn9veYVmekN2+ThMoinE= X-Received: by 2002:a05:6000:3c6:b0:220:5efd:423c with SMTP id b6-20020a05600003c600b002205efd423cmr1695399wrg.214.1660298175651; Fri, 12 Aug 2022 02:56:15 -0700 (PDT) MIME-Version: 1.0 From: Kegl Rohit Date: Fri, 12 Aug 2022 11:56:04 +0200 Message-ID: Subject: rn5t618_wdt: WATCHDOG_NOWAYOUT not working To: linux-watchdog@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-watchdog@vger.kernel.org Hello! Our board uses the RN5T567 as /dev/watchdog source via i2c communication. RN5T567 is using the rn5t618_wdt.c driver Our kernel has CONFIG_WATCHDOG_NOWAYOUT=y enabled # Starting the wdt works as expected echo -n '1' > /dev/watchdog # Stopping the wdt works as expected no reboot will be issued echo -n 'V' > /dev/watchdog # Starting the wdt again will enable the wdt again # BUT while the wdt is triggered every second the system reboots while true; do echo -n '1' > /dev/watchdog; sleep 1; done Digging deeper into the issue I could find out that the remap is initialized to cache the register accesses RN5T618_WATCHDOG (0x0b) and RN5T618_PWRIRQ (0x13). So I expect that because of this caching the IRQ status bit was never reset. The status register must not be cached because its set by the RN5T567. Also it is not ideal to cache the access to the watchdog register which resets the counter via read write cycle. debugfs shows the regmap setting for these registers: [root@imx7d /sys/kernel/debug]# cat regmap/0-0033/access // third column means volatile yes or no … 0b: y y n n … 13: y y n n After marking these registers volatile, stopping the wdt and starting again seems to work. Furthermore it is not necessary to do a RN5T618_WATCHDOG read AND write cycle to reset the wdt counter. The source code states: /* The counter is restarted after a R/W access to watchdog register */ The RN5T567 datasheet states: “The count value of watchdog timer is cleared by accessing (R/W) to this register.” Tests showed that a single read is enough. I did not check other chip variants which use the same driver. In my opinion a write cycle is even dangerous if there is some strange situation and the write cycle disables the wdt or changes the wdt settings stored in this register. I still don't know why the irq status bit is cleared on every ping() but I kept it there. Attached is my patch tested with RN5T567. Is the rn5t618_wdt.c driver maintained? Strange that such issue was never noticed. diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c index 652a5e60067f..b414abcdfecb 100644 --- a/drivers/mfd/rn5t618.c +++ b/drivers/mfd/rn5t618.c @@ -47,8 +47,10 @@ static bool rn5t618_volatile_reg(struct device *dev, unsigned int reg) case RN5T618_RTC_SECONDS ... RN5T618_RTC_YEAR: case RN5T618_CHGSTATE: case RN5T618_CHGCTRL_IRR ... RN5T618_CHGERR_MONI: case RN5T618_CONTROL ... RN5T618_CC_AVEREG0: + case RN5T618_WATCHDOG: // should not be cached because of r/w to reset counter + case RN5T618_PWRIRQ: // should not be cached because its set by hardware return true; default: return false; } diff --git a/drivers/watchdog/rn5t618_wdt.c b/drivers/watchdog/rn5t618_wdt.c index 6e524c8e26a8..83d3f006f812 100644 --- a/drivers/watchdog/rn5t618_wdt.c +++ b/drivers/watchdog/rn5t618_wdt.c @@ -75,8 +75,13 @@ static int rn5t618_wdt_start(struct watchdog_device *wdt_dev) ret = rn5t618_wdt_set_timeout(wdt_dev, wdt_dev->timeout); if (ret) return ret; + /* Clear pending watchdog interrupt */ + ret = regmap_write(wdt->rn5t618->regmap, RN5T618_PWRIRQ, ~RN5T618_PWRIRQ_IR_WDOG); + if (ret) + return ret; + /* enable repower-on */ ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_REPCNT, RN5T618_REPCNT_REPWRON, RN5T618_REPCNT_REPWRON); @@ -114,15 +119,10 @@ static int rn5t618_wdt_ping(struct watchdog_device *wdt_dev) ret = regmap_read(wdt->rn5t618->regmap, RN5T618_WATCHDOG, &val); if (ret) return ret; - ret = regmap_write(wdt->rn5t618->regmap, RN5T618_WATCHDOG, val); - if (ret) - return ret; - /* Clear pending watchdog interrupt */ - return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIRQ, - RN5T618_PWRIRQ_IR_WDOG, 0); + return regmap_write(wdt->rn5t618->regmap, RN5T618_PWRIRQ, ~RN5T618_PWRIRQ_IR_WDOG); } static const struct watchdog_info rn5t618_wdt_info = { .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | -- 2.30.2