From: Guenter Roeck <linux@roeck-us.net>
To: "Marek Behún" <kabel@kernel.org>,
"Gregory CLEMENT" <gregory.clement@bootlin.com>,
"Arnd Bergmann" <arnd@arndb.de>,
soc@kernel.org, arm@kernel.org,
"Andy Shevchenko" <andy@kernel.org>,
"Linus Walleij" <linus.walleij@linaro.org>,
"Bartosz Golaszewski" <brgl@bgdev.pl>,
linux-watchdog@vger.kernel.org,
"Wim Van Sebroeck" <wim@linux-watchdog.org>
Subject: Re: [PATCH v3 5/7] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog
Date: Mon, 23 Oct 2023 07:47:44 -0700 [thread overview]
Message-ID: <2439af40-0c78-6b41-1746-c71b67af217a@roeck-us.net> (raw)
In-Reply-To: <20231023143130.11602-6-kabel@kernel.org>
On 10/23/23 07:31, Marek Behún wrote:
> Add support for the watchdog mechanism provided by the MCU.
>
> Signed-off-by: Marek Behún <kabel@kernel.org>
Any reason for keeping this out of drivers/watchdog ?
> ---
> drivers/platform/cznic/Kconfig | 2 +
> drivers/platform/cznic/Makefile | 1 +
> .../platform/cznic/turris-omnia-mcu-base.c | 4 +
> .../cznic/turris-omnia-mcu-watchdog.c | 122 ++++++++++++++++++
> drivers/platform/cznic/turris-omnia-mcu.h | 24 ++++
> 5 files changed, 153 insertions(+)
> create mode 100644 drivers/platform/cznic/turris-omnia-mcu-watchdog.c
>
> diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
> index 0a752aa654fa..e2649cdecc38 100644
> --- a/drivers/platform/cznic/Kconfig
> +++ b/drivers/platform/cznic/Kconfig
> @@ -20,6 +20,7 @@ config TURRIS_OMNIA_MCU
> select GPIOLIB
> select GPIOLIB_IRQCHIP
> select RTC_CLASS
> + select WATCHDOG_CORE
> help
> Say Y here to add support for the features implemented by the
> microcontroller on the CZ.NIC's Turris Omnia SOHO router.
> @@ -27,6 +28,7 @@ config TURRIS_OMNIA_MCU
> - board poweroff into true low power mode (with voltage regulators
> disabled) and the ability to configure wake up from this mode (via
> rtcwake)
> + - MCU watchdog
> - GPIO pins
> - to get front button press events (the front button can be
> configured either to generate press events to the CPU or to change
> diff --git a/drivers/platform/cznic/Makefile b/drivers/platform/cznic/Makefile
> index 6f1470d1f673..a43997a12d74 100644
> --- a/drivers/platform/cznic/Makefile
> +++ b/drivers/platform/cznic/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_TURRIS_OMNIA_MCU) += turris-omnia-mcu.o
> turris-omnia-mcu-objs := turris-omnia-mcu-base.o
> turris-omnia-mcu-objs += turris-omnia-mcu-gpio.o
> turris-omnia-mcu-objs += turris-omnia-mcu-sys-off-wakeup.o
> +turris-omnia-mcu-objs += turris-omnia-mcu-watchdog.o
> diff --git a/drivers/platform/cznic/turris-omnia-mcu-base.c b/drivers/platform/cznic/turris-omnia-mcu-base.c
> index 942061a0ee66..521397bfc022 100644
> --- a/drivers/platform/cznic/turris-omnia-mcu-base.c
> +++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
> @@ -243,6 +243,10 @@ static int omnia_mcu_probe(struct i2c_client *client)
> if (err)
> return err;
>
> + err = omnia_mcu_register_watchdog(mcu);
> + if (err)
> + return err;
> +
> return omnia_mcu_register_gpiochip(mcu);
> }
>
> diff --git a/drivers/platform/cznic/turris-omnia-mcu-watchdog.c b/drivers/platform/cznic/turris-omnia-mcu-watchdog.c
> new file mode 100644
> index 000000000000..6685166bc4c7
> --- /dev/null
> +++ b/drivers/platform/cznic/turris-omnia-mcu-watchdog.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CZ.NIC's Turris Omnia MCU watchdog driver
> + *
> + * 2023 by Marek Behún <kabel@kernel.org>
> + */
> +
> +#include <linux/moduleparam.h>
> +#include <linux/turris-omnia-mcu-interface.h>
> +#include <linux/types.h>
> +#include <linux/watchdog.h>
> +
> +#include "turris-omnia-mcu.h"
> +
> +#define WATCHDOG_TIMEOUT 120
> +
> +static unsigned int timeout;
> +module_param(timeout, int, 0);
> +MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds");
> +
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +static int omnia_wdt_start(struct watchdog_device *wdt)
> +{
> + struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
> +
> + return omnia_cmd_write_u8(mcu->client, CMD_SET_WATCHDOG_STATE, 1);
> +}
> +
> +static int omnia_wdt_stop(struct watchdog_device *wdt)
> +{
> + struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
> +
> + return omnia_cmd_write_u8(mcu->client, CMD_SET_WATCHDOG_STATE, 0);
> +}
> +
> +static int omnia_wdt_ping(struct watchdog_device *wdt)
> +{
> + struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
> +
> + return omnia_cmd_write_u8(mcu->client, CMD_SET_WATCHDOG_STATE, 1);
> +}
> +
> +static int omnia_wdt_set_timeout(struct watchdog_device *wdt,
> + unsigned int timeout)
> +{
> + struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
> +
> + return omnia_cmd_write_u16(mcu->client, CMD_SET_WDT_TIMEOUT,
> + timeout * 10);
> +}
> +
> +static unsigned int omnia_wdt_get_timeleft(struct watchdog_device *wdt)
> +{
> + struct omnia_mcu *mcu = watchdog_get_drvdata(wdt);
> + int ret;
> +
> + ret = omnia_cmd_read_u16(mcu->client, CMD_GET_WDT_TIMELEFT);
> + if (ret < 0) {
> + dev_err(&mcu->client->dev, "Cannot get watchdog timeleft: %d\n",
> + ret);
> + return 0;
> + }
> +
> + return ret / 10;
> +}
> +
> +static const struct watchdog_info omnia_wdt_info = {
> + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> + .identity = "Turris Omnia MCU Watchdog",
> +};
> +
> +static const struct watchdog_ops omnia_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = omnia_wdt_start,
> + .stop = omnia_wdt_stop,
> + .ping = omnia_wdt_ping,
> + .set_timeout = omnia_wdt_set_timeout,
> + .get_timeleft = omnia_wdt_get_timeleft,
> +};
> +
> +int omnia_mcu_register_watchdog(struct omnia_mcu *mcu)
> +{
> + struct device *dev = &mcu->client->dev;
> + int ret;
> +
> + if (!(mcu->features & FEAT_WDT_PING))
> + return 0;
> +
Why check this here and not in the calling code ?
> + mcu->wdt.info = &omnia_wdt_info;
> + mcu->wdt.ops = &omnia_wdt_ops;
> + mcu->wdt.parent = dev;
> + mcu->wdt.min_timeout = 1;
> + mcu->wdt.max_timeout = 6553; /* 65535 deciseconds */
> +
> + mcu->wdt.timeout = WATCHDOG_TIMEOUT;
> + watchdog_init_timeout(&mcu->wdt, timeout, dev);
> +
> + watchdog_set_drvdata(&mcu->wdt, mcu);
> +
> + omnia_wdt_set_timeout(&mcu->wdt, mcu->wdt.timeout);
> +
> + ret = omnia_cmd_read_u8(mcu->client, CMD_GET_WATCHDOG_STATE);
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "Cannot get MCU watchdog state\n");
> +
> + if (ret)
> + set_bit(WDOG_HW_RUNNING, &mcu->wdt.status);
> +
> + watchdog_set_nowayout(&mcu->wdt, nowayout);
> + watchdog_stop_on_reboot(&mcu->wdt);
> + ret = devm_watchdog_register_device(dev, &mcu->wdt);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Cannot register MCU watchdog\n");
> +
> + return 0;
> +}
> diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
> index c6a8036e0534..db2d18d6eea8 100644
> --- a/drivers/platform/cznic/turris-omnia-mcu.h
> +++ b/drivers/platform/cznic/turris-omnia-mcu.h
> @@ -14,6 +14,7 @@
> #include <linux/mutex.h>
> #include <linux/rtc.h>
> #include <linux/types.h>
> +#include <linux/watchdog.h>
> #include <linux/workqueue.h>
> #include <asm/byteorder.h>
> #include <asm/unaligned.h>
> @@ -36,6 +37,9 @@ struct omnia_mcu {
> struct rtc_device *rtcdev;
> u32 rtc_alarm;
> bool front_button_poweron;
> +
> + /* MCU watchdog */
> + struct watchdog_device wdt;
This should be internal to the watchdog driver.
> };
>
> static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
> @@ -48,6 +52,25 @@ static inline int omnia_cmd_write(const struct i2c_client *client, void *cmd,
> return ret < 0 ? ret : 0;
> }
>
> +static inline int omnia_cmd_write_u8(const struct i2c_client *client, u8 cmd,
> + u8 val)
> +{
> + u8 buf[2] = { cmd, val };
> +
> + return omnia_cmd_write(client, buf, sizeof(buf));
> +}
> +
> +static inline int omnia_cmd_write_u16(const struct i2c_client *client, u8 cmd,
> + u16 val)
> +{
> + u8 buf[3];
> +
> + buf[0] = cmd;
> + put_unaligned_le16(val, &buf[1]);
> +
> + return omnia_cmd_write(client, buf, sizeof(buf));
> +}
> +
> static inline int omnia_cmd_write_u32(const struct i2c_client *client, u8 cmd,
> u32 val)
> {
> @@ -140,5 +163,6 @@ extern const struct attribute_group omnia_mcu_poweroff_group;
>
> int omnia_mcu_register_gpiochip(struct omnia_mcu *mcu);
> int omnia_mcu_register_sys_off_and_wakeup(struct omnia_mcu *mcu);
> +int omnia_mcu_register_watchdog(struct omnia_mcu *mcu);
>
> #endif /* __TURRIS_OMNIA_MCU_H */
parent reply other threads:[~2023-10-23 14:47 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <20231023143130.11602-6-kabel@kernel.org>]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2439af40-0c78-6b41-1746-c71b67af217a@roeck-us.net \
--to=linux@roeck-us.net \
--cc=andy@kernel.org \
--cc=arm@kernel.org \
--cc=arnd@arndb.de \
--cc=brgl@bgdev.pl \
--cc=gregory.clement@bootlin.com \
--cc=kabel@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=soc@kernel.org \
--cc=wim@linux-watchdog.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).