diff mbox series

[V3,2/4] watchdog: imx_sc: Add i.MX system controller watchdog support

Message ID 1551060850-22553-3-git-send-email-Anson.Huang@nxp.com (mailing list archive)
State New, archived
Headers show
Series Add i.MX8QXP system controller watchdog support | expand

Commit Message

Anson Huang Feb. 25, 2019, 2:19 a.m. UTC
i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
inside, the system controller is in charge of controlling power,
clock and watchdog etc..

This patch adds i.MX system controller watchdog driver support,
watchdog operation needs to be done in secure EL3 mode via
ARM-Trusted-Firmware, using SMC call, CPU will trap into
ARM-Trusted-Firmware and then it will request system controller
to do watchdog operation via IPC.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V2:
	- improve watchdog_init_timeout() operation and error check, setting it
	  via module parameter "timeout", if it is invalid, roll back to use defaul
	  timeout value DEFAULT_TIMEOUT;
	- change compatible string to "fsl,imx-sc-wdt" to make driver more generic
	  for all i.MX platforms with system controller watchdog inside.
---
 drivers/watchdog/Kconfig      |  13 +++
 drivers/watchdog/Makefile     |   1 +
 drivers/watchdog/imx_sc_wdt.c | 183 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 197 insertions(+)
 create mode 100644 drivers/watchdog/imx_sc_wdt.c

Comments

Guenter Roeck Feb. 26, 2019, 10:38 p.m. UTC | #1
On Mon, Feb 25, 2019 at 02:19:10AM +0000, Anson Huang wrote:
> i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
> inside, the system controller is in charge of controlling power,
> clock and watchdog etc..
> 
> This patch adds i.MX system controller watchdog driver support,
> watchdog operation needs to be done in secure EL3 mode via
> ARM-Trusted-Firmware, using SMC call, CPU will trap into
> ARM-Trusted-Firmware and then it will request system controller
> to do watchdog operation via IPC.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V2:
> 	- improve watchdog_init_timeout() operation and error check, setting it
> 	  via module parameter "timeout", if it is invalid, roll back to use defaul
> 	  timeout value DEFAULT_TIMEOUT;
> 	- change compatible string to "fsl,imx-sc-wdt" to make driver more generic
> 	  for all i.MX platforms with system controller watchdog inside.
> ---
>  drivers/watchdog/Kconfig      |  13 +++
>  drivers/watchdog/Makefile     |   1 +
>  drivers/watchdog/imx_sc_wdt.c | 183 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 197 insertions(+)
>  create mode 100644 drivers/watchdog/imx_sc_wdt.c
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 65c3c42..5c5b8ba 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -625,6 +625,19 @@ config IMX2_WDT
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called imx2_wdt.
>  
> +config IMX_SC_WDT
> +	tristate "IMX SC Watchdog"
> +	depends on ARCH_MXC || COMPILE_TEST

Wondering: Did you test on other architectures ? The code calls
arm_smccc_smc(), which is unlikely to be available on non-ARM
platforms, and I don't see a dummy declaration for those.

> +	select WATCHDOG_CORE
> +	help
> +	  This is the driver for the system controller watchdog
> +	  on the NXP i.MX SoCs with system controller inside.
> +	  If you have one of these processors and wish to have
> +	  watchdog support enabled, say Y, otherwise say N.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called imx_sc_wdt.
> +
>  config UX500_WATCHDOG
>  	tristate "ST-Ericsson Ux500 watchdog"
>  	depends on MFD_DB8500_PRCMU
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 4e78a8c..0c9da63 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -68,6 +68,7 @@ obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o
>  obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o
>  obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
>  obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
> +obj-$(CONFIG_IMX_SC_WDT) += imx_sc_wdt.o
>  obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
>  obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
>  obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> diff --git a/drivers/watchdog/imx_sc_wdt.c b/drivers/watchdog/imx_sc_wdt.c
> new file mode 100644
> index 0000000..7b22575
> --- /dev/null
> +++ b/drivers/watchdog/imx_sc_wdt.c
> @@ -0,0 +1,183 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018-2019 NXP.
> + */
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/io.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include <linux/watchdog.h>
> +
> +#define DEFAULT_TIMEOUT 60
> +/*
> + * Software timer tick implemented in scfw side, support 10ms to 0xffffffff ms
> + * in theory, but for normal case, 1s~128s is enough, you can change this max
> + * value in case it's not enough.
> + */
> +#define MAX_TIMEOUT 128
> +
> +#define IMX_SIP_TIMER			0xC2000002
> +#define IMX_SIP_TIMER_START_WDOG		0x01
> +#define IMX_SIP_TIMER_STOP_WDOG		0x02
> +#define IMX_SIP_TIMER_SET_WDOG_ACT	0x03
> +#define IMX_SIP_TIMER_PING_WDOG		0x04
> +#define IMX_SIP_TIMER_SET_TIMEOUT_WDOG	0x05
> +#define IMX_SIP_TIMER_GET_WDOG_STAT	0x06
> +#define IMX_SIP_TIMER_SET_PRETIME_WDOG	0x07
> +
> +#define SC_TIMER_WDOG_ACTION_PARTITION	0
> +
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, bool, 0000);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +static unsigned int timeout = DEFAULT_TIMEOUT;

I assume this means that you specifically do _not_ want to support
timeout configuration via devicetree (unless the user explicitly sets
the timeout to 0 here).

Not that it really matters, since it looks like Rob won't accept a
watchdog node anyway.

> +module_param(timeout, uint, 0000);
> +MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds (default="
> +		 __MODULE_STRING(DEFAULT_TIMEOUT) ")");
> +
> +static int imx_sc_wdt_ping(struct watchdog_device *wdog)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_PING_WDOG,
> +		      0, 0, 0, 0, 0, 0, &res);
> +
> +	return res.a0;

I didn't comment on this before, but I am wondering: Does the 
system controller code return Linux error codes ? Is this
documented somewhere ?

> +}
> +
> +static int imx_sc_wdt_start(struct watchdog_device *wdog)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_START_WDOG,
> +		      0, 0, 0, 0, 0, 0, &res);
> +	if (res.a0)
> +		return res.a0;
> +
> +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_SET_WDOG_ACT,
> +		      SC_TIMER_WDOG_ACTION_PARTITION,
> +		      0, 0, 0, 0, 0, &res);
> +	return res.a0;
> +}
> +
> +static int imx_sc_wdt_stop(struct watchdog_device *wdog)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_STOP_WDOG,
> +		      0, 0, 0, 0, 0, 0, &res);
> +
> +	return res.a0;
> +}
> +
> +static int imx_sc_wdt_set_timeout(struct watchdog_device *wdog,
> +				unsigned int timeout)
> +{
> +	struct arm_smccc_res res;
> +
> +	wdog->timeout = timeout;
> +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_SET_TIMEOUT_WDOG,
> +		      timeout * 1000, 0, 0, 0, 0, 0, &res);
> +
> +	return res.a0;
> +}
> +
> +static const struct watchdog_ops imx_sc_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = imx_sc_wdt_start,
> +	.stop  = imx_sc_wdt_stop,
> +	.ping  = imx_sc_wdt_ping,
> +	.set_timeout = imx_sc_wdt_set_timeout,
> +};
> +
> +static const struct watchdog_info imx_sc_wdt_info = {
> +	.identity	= "i.MX SC watchdog timer",
> +	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> +			  WDIOF_MAGICCLOSE | WDIOF_PRETIMEOUT,
> +};
> +
> +static int imx_sc_wdt_probe(struct platform_device *pdev)
> +{
> +	struct watchdog_device *imx_sc_wdd;
> +	int ret;
> +
> +	imx_sc_wdd = devm_kzalloc(&pdev->dev, sizeof(*imx_sc_wdd), GFP_KERNEL);
> +	if (!imx_sc_wdd)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, imx_sc_wdd);
> +
> +	imx_sc_wdd->info = &imx_sc_wdt_info;
> +	imx_sc_wdd->ops = &imx_sc_wdt_ops;
> +	imx_sc_wdd->min_timeout = 1;
> +	imx_sc_wdd->max_timeout = MAX_TIMEOUT;
> +	imx_sc_wdd->parent = &pdev->dev;
> +	imx_sc_wdd->timeout = DEFAULT_TIMEOUT;
> +
> +	ret = watchdog_init_timeout(imx_sc_wdd, timeout, &pdev->dev);
> +	if (ret)
> +		dev_warn(&pdev->dev, "Failed to set timeout value, using default\n");
> +
> +	watchdog_stop_on_reboot(imx_sc_wdd);
> +	watchdog_stop_on_unregister(imx_sc_wdd);
> +
> +	ret = devm_watchdog_register_device(&pdev->dev, imx_sc_wdd);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to register watchdog device\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id imx_sc_wdt_dt_ids[] = {
> +	{ .compatible = "fsl,imx-sc-wdt", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx_sc_wdt_dt_ids);

I assume this will be dropped and replaced with platform code,
per feedback from Rob.

> +
> +static int __maybe_unused imx_sc_wdt_suspend(struct device *dev)
> +{
> +	struct watchdog_device *imx_sc_wdd = dev_get_drvdata(dev);
> +
> +	if (watchdog_active(imx_sc_wdd))
> +		imx_sc_wdt_stop(imx_sc_wdd);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused imx_sc_wdt_resume(struct device *dev)
> +{
> +	struct watchdog_device *imx_sc_wdd = dev_get_drvdata(dev);
> +
> +	if (watchdog_active(imx_sc_wdd))
> +		imx_sc_wdt_start(imx_sc_wdd);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(imx_sc_wdt_pm_ops,
> +			 imx_sc_wdt_suspend, imx_sc_wdt_resume);
> +
> +static struct platform_driver imx_sc_wdt_driver = {
> +	.probe		= imx_sc_wdt_probe,
> +	.driver		= {
> +		.name	= "imx-sc-wdt",
> +		.of_match_table = imx_sc_wdt_dt_ids,
> +		.pm	= &imx_sc_wdt_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(imx_sc_wdt_driver);
> +
> +MODULE_AUTHOR("Robin Gong <yibin.gong@nxp.com>");
> +MODULE_DESCRIPTION("NXP i.MX system controller watchdog driver");
> +MODULE_LICENSE("GPL v2");

The module license conflicts with the SPDX license which says 2.0+.
Anson Huang Feb. 28, 2019, 6:15 a.m. UTC | #2
Hi, Guenter

Best Regards!
Anson Huang

> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter
> Roeck
> Sent: 2019年2月27日 6:39
> To: Anson Huang <anson.huang@nxp.com>
> Cc: robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> catalin.marinas@arm.com; will.deacon@arm.com; wim@linux-watchdog.org;
> Aisheng Dong <aisheng.dong@nxp.com>; ulf.hansson@linaro.org; Daniel
> Baluta <daniel.baluta@nxp.com>; Andy Gross <andy.gross@linaro.org>;
> horms+renesas@verge.net.au; heiko@sntech.de; arnd@arndb.de;
> olof@lixom.net; bjorn.andersson@linaro.org; jagan@amarulasolutions.com;
> enric.balletbo@collabora.com; marc.w.gonzalez@free.fr;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-watchdog@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>
> Subject: Re: [PATCH V3 2/4] watchdog: imx_sc: Add i.MX system controller
> watchdog support
> 
> On Mon, Feb 25, 2019 at 02:19:10AM +0000, Anson Huang wrote:
> > i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
> > inside, the system controller is in charge of controlling power, clock
> > and watchdog etc..
> >
> > This patch adds i.MX system controller watchdog driver support,
> > watchdog operation needs to be done in secure EL3 mode via
> > ARM-Trusted-Firmware, using SMC call, CPU will trap into
> > ARM-Trusted-Firmware and then it will request system controller to do
> > watchdog operation via IPC.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V2:
> > 	- improve watchdog_init_timeout() operation and error check,
> setting it
> > 	  via module parameter "timeout", if it is invalid, roll back to use
> defaul
> > 	  timeout value DEFAULT_TIMEOUT;
> > 	- change compatible string to "fsl,imx-sc-wdt" to make driver more
> generic
> > 	  for all i.MX platforms with system controller watchdog inside.
> > ---
> >  drivers/watchdog/Kconfig      |  13 +++
> >  drivers/watchdog/Makefile     |   1 +
> >  drivers/watchdog/imx_sc_wdt.c | 183
> > ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 197 insertions(+)
> >  create mode 100644 drivers/watchdog/imx_sc_wdt.c
> >
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index
> > 65c3c42..5c5b8ba 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -625,6 +625,19 @@ config IMX2_WDT
> >  	  To compile this driver as a module, choose M here: the
> >  	  module will be called imx2_wdt.
> >
> > +config IMX_SC_WDT
> > +	tristate "IMX SC Watchdog"
> > +	depends on ARCH_MXC || COMPILE_TEST
> 
> Wondering: Did you test on other architectures ? The code calls
> arm_smccc_smc(), which is unlikely to be available on non-ARM platforms,
> and I don't see a dummy declaration for those.

Although the ARCH_MXC means for Freescale i.MX SoCs which are all with
ARM platforms, but since this driver is targeted for i.MX SoC ARMv8 with system
controller inside, so I will add ARM64 dependency here.

+       depends on (ARCH_MXC && ARM64) || COMPILE_TEST

> 
> > +	select WATCHDOG_CORE
> > +	help
> > +	  This is the driver for the system controller watchdog
> > +	  on the NXP i.MX SoCs with system controller inside.
> > +	  If you have one of these processors and wish to have
> > +	  watchdog support enabled, say Y, otherwise say N.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called imx_sc_wdt.
> > +
> >  config UX500_WATCHDOG
> >  	tristate "ST-Ericsson Ux500 watchdog"
> >  	depends on MFD_DB8500_PRCMU
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index 4e78a8c..0c9da63 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -68,6 +68,7 @@ obj-$(CONFIG_NUC900_WATCHDOG) +=
> nuc900_wdt.o
> >  obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o
> >  obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
> >  obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
> > +obj-$(CONFIG_IMX_SC_WDT) += imx_sc_wdt.o
> >  obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
> >  obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
> >  obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o diff --git
> > a/drivers/watchdog/imx_sc_wdt.c b/drivers/watchdog/imx_sc_wdt.c new
> > file mode 100644 index 0000000..7b22575
> > --- /dev/null
> > +++ b/drivers/watchdog/imx_sc_wdt.c
> > @@ -0,0 +1,183 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2018-2019 NXP.
> > + */
> > +
> > +#include <linux/arm-smccc.h>
> > +#include <linux/io.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/moduleparam.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/reboot.h>
> > +#include <linux/watchdog.h>
> > +
> > +#define DEFAULT_TIMEOUT 60
> > +/*
> > + * Software timer tick implemented in scfw side, support 10ms to
> > +0xffffffff ms
> > + * in theory, but for normal case, 1s~128s is enough, you can change
> > +this max
> > + * value in case it's not enough.
> > + */
> > +#define MAX_TIMEOUT 128
> > +
> > +#define IMX_SIP_TIMER			0xC2000002
> > +#define IMX_SIP_TIMER_START_WDOG		0x01
> > +#define IMX_SIP_TIMER_STOP_WDOG		0x02
> > +#define IMX_SIP_TIMER_SET_WDOG_ACT	0x03
> > +#define IMX_SIP_TIMER_PING_WDOG		0x04
> > +#define IMX_SIP_TIMER_SET_TIMEOUT_WDOG	0x05
> > +#define IMX_SIP_TIMER_GET_WDOG_STAT	0x06
> > +#define IMX_SIP_TIMER_SET_PRETIME_WDOG	0x07
> > +
> > +#define SC_TIMER_WDOG_ACTION_PARTITION	0
> > +
> > +static bool nowayout = WATCHDOG_NOWAYOUT;
> module_param(nowayout,
> > +bool, 0000); MODULE_PARM_DESC(nowayout, "Watchdog cannot be
> stopped
> > +once started (default="
> > +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> > +
> > +static unsigned int timeout = DEFAULT_TIMEOUT;
> 
> I assume this means that you specifically do _not_ want to support timeout
> configuration via devicetree (unless the user explicitly sets the timeout to 0
> here).
> 
> Not that it really matters, since it looks like Rob won't accept a watchdog
> node anyway.

Yes, currently we will NOT get it from devicetree, although user can still pass
it via insmod parameter, but if the parameter is NOT specified or is invalid,
default value will be used.

> 
> > +module_param(timeout, uint, 0000);
> > +MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds (default="
> > +		 __MODULE_STRING(DEFAULT_TIMEOUT) ")");
> > +
> > +static int imx_sc_wdt_ping(struct watchdog_device *wdog) {
> > +	struct arm_smccc_res res;
> > +
> > +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_PING_WDOG,
> > +		      0, 0, 0, 0, 0, 0, &res);
> > +
> > +	return res.a0;
> 
> I didn't comment on this before, but I am wondering: Does the system
> controller code return Linux error codes ? Is this documented somewhere ?

I checked our system controller firmware, the ONLY possible error code system controller
returns is SC_ERR_NOACCESS, so I will make it return -EACCES if error happen, for the
ping operation, system controller always returns 0, so I will make it return 0.

> 
> > +}
> > +
> > +static int imx_sc_wdt_start(struct watchdog_device *wdog) {
> > +	struct arm_smccc_res res;
> > +
> > +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_START_WDOG,
> > +		      0, 0, 0, 0, 0, 0, &res);
> > +	if (res.a0)
> > +		return res.a0;
> > +
> > +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_SET_WDOG_ACT,
> > +		      SC_TIMER_WDOG_ACTION_PARTITION,
> > +		      0, 0, 0, 0, 0, &res);
> > +	return res.a0;
> > +}
> > +
> > +static int imx_sc_wdt_stop(struct watchdog_device *wdog) {
> > +	struct arm_smccc_res res;
> > +
> > +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_STOP_WDOG,
> > +		      0, 0, 0, 0, 0, 0, &res);
> > +
> > +	return res.a0;
> > +}
> > +
> > +static int imx_sc_wdt_set_timeout(struct watchdog_device *wdog,
> > +				unsigned int timeout)
> > +{
> > +	struct arm_smccc_res res;
> > +
> > +	wdog->timeout = timeout;
> > +	arm_smccc_smc(IMX_SIP_TIMER,
> IMX_SIP_TIMER_SET_TIMEOUT_WDOG,
> > +		      timeout * 1000, 0, 0, 0, 0, 0, &res);
> > +
> > +	return res.a0;
> > +}
> > +
> > +static const struct watchdog_ops imx_sc_wdt_ops = {
> > +	.owner = THIS_MODULE,
> > +	.start = imx_sc_wdt_start,
> > +	.stop  = imx_sc_wdt_stop,
> > +	.ping  = imx_sc_wdt_ping,
> > +	.set_timeout = imx_sc_wdt_set_timeout, };
> > +
> > +static const struct watchdog_info imx_sc_wdt_info = {
> > +	.identity	= "i.MX SC watchdog timer",
> > +	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> > +			  WDIOF_MAGICCLOSE | WDIOF_PRETIMEOUT, };
> > +
> > +static int imx_sc_wdt_probe(struct platform_device *pdev) {
> > +	struct watchdog_device *imx_sc_wdd;
> > +	int ret;
> > +
> > +	imx_sc_wdd = devm_kzalloc(&pdev->dev, sizeof(*imx_sc_wdd),
> GFP_KERNEL);
> > +	if (!imx_sc_wdd)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, imx_sc_wdd);
> > +
> > +	imx_sc_wdd->info = &imx_sc_wdt_info;
> > +	imx_sc_wdd->ops = &imx_sc_wdt_ops;
> > +	imx_sc_wdd->min_timeout = 1;
> > +	imx_sc_wdd->max_timeout = MAX_TIMEOUT;
> > +	imx_sc_wdd->parent = &pdev->dev;
> > +	imx_sc_wdd->timeout = DEFAULT_TIMEOUT;
> > +
> > +	ret = watchdog_init_timeout(imx_sc_wdd, timeout, &pdev->dev);
> > +	if (ret)
> > +		dev_warn(&pdev->dev, "Failed to set timeout value, using
> > +default\n");
> > +
> > +	watchdog_stop_on_reboot(imx_sc_wdd);
> > +	watchdog_stop_on_unregister(imx_sc_wdd);
> > +
> > +	ret = devm_watchdog_register_device(&pdev->dev, imx_sc_wdd);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to register watchdog device\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id imx_sc_wdt_dt_ids[] = {
> > +	{ .compatible = "fsl,imx-sc-wdt", },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx_sc_wdt_dt_ids);
> 
> I assume this will be dropped and replaced with platform code, per feedback
> from Rob.

Yes, I will remove it in V4, just register platform device in driver directly.

> 
> > +
> > +static int __maybe_unused imx_sc_wdt_suspend(struct device *dev) {
> > +	struct watchdog_device *imx_sc_wdd = dev_get_drvdata(dev);
> > +
> > +	if (watchdog_active(imx_sc_wdd))
> > +		imx_sc_wdt_stop(imx_sc_wdd);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused imx_sc_wdt_resume(struct device *dev) {
> > +	struct watchdog_device *imx_sc_wdd = dev_get_drvdata(dev);
> > +
> > +	if (watchdog_active(imx_sc_wdd))
> > +		imx_sc_wdt_start(imx_sc_wdd);
> > +
> > +	return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(imx_sc_wdt_pm_ops,
> > +			 imx_sc_wdt_suspend, imx_sc_wdt_resume);
> > +
> > +static struct platform_driver imx_sc_wdt_driver = {
> > +	.probe		= imx_sc_wdt_probe,
> > +	.driver		= {
> > +		.name	= "imx-sc-wdt",
> > +		.of_match_table = imx_sc_wdt_dt_ids,
> > +		.pm	= &imx_sc_wdt_pm_ops,
> > +	},
> > +};
> > +
> > +module_platform_driver(imx_sc_wdt_driver);
> > +
> > +MODULE_AUTHOR("Robin Gong <yibin.gong@nxp.com>");
> > +MODULE_DESCRIPTION("NXP i.MX system controller watchdog driver");
> > +MODULE_LICENSE("GPL v2");
> 
> The module license conflicts with the SPDX license which says 2.0+.

OK, I will change the SPDX license to 2.0.

Please help review V4 patch series, the dt-binding and dts patch will be removed.

Thanks,
Anson.
Guenter Roeck Feb. 28, 2019, 2:25 p.m. UTC | #3
On 2/27/19 10:15 PM, Anson Huang wrote:
> Hi, Guenter
> 
> Best Regards!
> Anson Huang
> 
>> -----Original Message-----
>> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter
>> Roeck
>> Sent: 2019年2月27日 6:39
>> To: Anson Huang <anson.huang@nxp.com>
>> Cc: robh+dt@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org;
>> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
>> catalin.marinas@arm.com; will.deacon@arm.com; wim@linux-watchdog.org;
>> Aisheng Dong <aisheng.dong@nxp.com>; ulf.hansson@linaro.org; Daniel
>> Baluta <daniel.baluta@nxp.com>; Andy Gross <andy.gross@linaro.org>;
>> horms+renesas@verge.net.au; heiko@sntech.de; arnd@arndb.de;
>> olof@lixom.net; bjorn.andersson@linaro.org; jagan@amarulasolutions.com;
>> enric.balletbo@collabora.com; marc.w.gonzalez@free.fr;
>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; linux-watchdog@vger.kernel.org; dl-linux-imx
>> <linux-imx@nxp.com>
>> Subject: Re: [PATCH V3 2/4] watchdog: imx_sc: Add i.MX system controller
>> watchdog support
>>
>> On Mon, Feb 25, 2019 at 02:19:10AM +0000, Anson Huang wrote:
>>> i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
>>> inside, the system controller is in charge of controlling power, clock
>>> and watchdog etc..
>>>
>>> This patch adds i.MX system controller watchdog driver support,
>>> watchdog operation needs to be done in secure EL3 mode via
>>> ARM-Trusted-Firmware, using SMC call, CPU will trap into
>>> ARM-Trusted-Firmware and then it will request system controller to do
>>> watchdog operation via IPC.
>>>
>>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>>> ---
>>> Changes since V2:
>>> 	- improve watchdog_init_timeout() operation and error check,
>> setting it
>>> 	  via module parameter "timeout", if it is invalid, roll back to use
>> defaul
>>> 	  timeout value DEFAULT_TIMEOUT;
>>> 	- change compatible string to "fsl,imx-sc-wdt" to make driver more
>> generic
>>> 	  for all i.MX platforms with system controller watchdog inside.
>>> ---
>>>   drivers/watchdog/Kconfig      |  13 +++
>>>   drivers/watchdog/Makefile     |   1 +
>>>   drivers/watchdog/imx_sc_wdt.c | 183
>>> ++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 197 insertions(+)
>>>   create mode 100644 drivers/watchdog/imx_sc_wdt.c
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index
>>> 65c3c42..5c5b8ba 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -625,6 +625,19 @@ config IMX2_WDT
>>>   	  To compile this driver as a module, choose M here: the
>>>   	  module will be called imx2_wdt.
>>>
>>> +config IMX_SC_WDT
>>> +	tristate "IMX SC Watchdog"
>>> +	depends on ARCH_MXC || COMPILE_TEST
>>
>> Wondering: Did you test on other architectures ? The code calls
>> arm_smccc_smc(), which is unlikely to be available on non-ARM platforms,
>> and I don't see a dummy declaration for those.
> 
> Although the ARCH_MXC means for Freescale i.MX SoCs which are all with
> ARM platforms, but since this driver is targeted for i.MX SoC ARMv8 with system
> controller inside, so I will add ARM64 dependency here.
> 
> +       depends on (ARCH_MXC && ARM64) || COMPILE_TEST
> 

You are missing my point. It will still try to compile with, say,
alpha:allmodconfig (or x86_64:allmodconfig). Did you try that ?

Guenter

>>
>>> +	select WATCHDOG_CORE
>>> +	help
>>> +	  This is the driver for the system controller watchdog
>>> +	  on the NXP i.MX SoCs with system controller inside.
>>> +	  If you have one of these processors and wish to have
>>> +	  watchdog support enabled, say Y, otherwise say N.
>>> +
>>> +	  To compile this driver as a module, choose M here: the
>>> +	  module will be called imx_sc_wdt.
>>> +
>>>   config UX500_WATCHDOG
>>>   	tristate "ST-Ericsson Ux500 watchdog"
>>>   	depends on MFD_DB8500_PRCMU
>>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>>> index 4e78a8c..0c9da63 100644
>>> --- a/drivers/watchdog/Makefile
>>> +++ b/drivers/watchdog/Makefile
>>> @@ -68,6 +68,7 @@ obj-$(CONFIG_NUC900_WATCHDOG) +=
>> nuc900_wdt.o
>>>   obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o
>>>   obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
>>>   obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
>>> +obj-$(CONFIG_IMX_SC_WDT) += imx_sc_wdt.o
>>>   obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
>>>   obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
>>>   obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o diff --git
>>> a/drivers/watchdog/imx_sc_wdt.c b/drivers/watchdog/imx_sc_wdt.c new
>>> file mode 100644 index 0000000..7b22575
>>> --- /dev/null
>>> +++ b/drivers/watchdog/imx_sc_wdt.c
>>> @@ -0,0 +1,183 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Copyright 2018-2019 NXP.
>>> + */
>>> +
>>> +#include <linux/arm-smccc.h>
>>> +#include <linux/io.h>
>>> +#include <linux/init.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/moduleparam.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/reboot.h>
>>> +#include <linux/watchdog.h>
>>> +
>>> +#define DEFAULT_TIMEOUT 60
>>> +/*
>>> + * Software timer tick implemented in scfw side, support 10ms to
>>> +0xffffffff ms
>>> + * in theory, but for normal case, 1s~128s is enough, you can change
>>> +this max
>>> + * value in case it's not enough.
>>> + */
>>> +#define MAX_TIMEOUT 128
>>> +
>>> +#define IMX_SIP_TIMER			0xC2000002
>>> +#define IMX_SIP_TIMER_START_WDOG		0x01
>>> +#define IMX_SIP_TIMER_STOP_WDOG		0x02
>>> +#define IMX_SIP_TIMER_SET_WDOG_ACT	0x03
>>> +#define IMX_SIP_TIMER_PING_WDOG		0x04
>>> +#define IMX_SIP_TIMER_SET_TIMEOUT_WDOG	0x05
>>> +#define IMX_SIP_TIMER_GET_WDOG_STAT	0x06
>>> +#define IMX_SIP_TIMER_SET_PRETIME_WDOG	0x07
>>> +
>>> +#define SC_TIMER_WDOG_ACTION_PARTITION	0
>>> +
>>> +static bool nowayout = WATCHDOG_NOWAYOUT;
>> module_param(nowayout,
>>> +bool, 0000); MODULE_PARM_DESC(nowayout, "Watchdog cannot be
>> stopped
>>> +once started (default="
>>> +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>>> +
>>> +static unsigned int timeout = DEFAULT_TIMEOUT;
>>
>> I assume this means that you specifically do _not_ want to support timeout
>> configuration via devicetree (unless the user explicitly sets the timeout to 0
>> here).
>>
>> Not that it really matters, since it looks like Rob won't accept a watchdog
>> node anyway.
> 
> Yes, currently we will NOT get it from devicetree, although user can still pass
> it via insmod parameter, but if the parameter is NOT specified or is invalid,
> default value will be used.
> 
>>
>>> +module_param(timeout, uint, 0000);
>>> +MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds (default="
>>> +		 __MODULE_STRING(DEFAULT_TIMEOUT) ")");
>>> +
>>> +static int imx_sc_wdt_ping(struct watchdog_device *wdog) {
>>> +	struct arm_smccc_res res;
>>> +
>>> +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_PING_WDOG,
>>> +		      0, 0, 0, 0, 0, 0, &res);
>>> +
>>> +	return res.a0;
>>
>> I didn't comment on this before, but I am wondering: Does the system
>> controller code return Linux error codes ? Is this documented somewhere ?
> 
> I checked our system controller firmware, the ONLY possible error code system controller
> returns is SC_ERR_NOACCESS, so I will make it return -EACCES if error happen, for the
> ping operation, system controller always returns 0, so I will make it return 0.
> 
>>
>>> +}
>>> +
>>> +static int imx_sc_wdt_start(struct watchdog_device *wdog) {
>>> +	struct arm_smccc_res res;
>>> +
>>> +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_START_WDOG,
>>> +		      0, 0, 0, 0, 0, 0, &res);
>>> +	if (res.a0)
>>> +		return res.a0;
>>> +
>>> +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_SET_WDOG_ACT,
>>> +		      SC_TIMER_WDOG_ACTION_PARTITION,
>>> +		      0, 0, 0, 0, 0, &res);
>>> +	return res.a0;
>>> +}
>>> +
>>> +static int imx_sc_wdt_stop(struct watchdog_device *wdog) {
>>> +	struct arm_smccc_res res;
>>> +
>>> +	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_STOP_WDOG,
>>> +		      0, 0, 0, 0, 0, 0, &res);
>>> +
>>> +	return res.a0;
>>> +}
>>> +
>>> +static int imx_sc_wdt_set_timeout(struct watchdog_device *wdog,
>>> +				unsigned int timeout)
>>> +{
>>> +	struct arm_smccc_res res;
>>> +
>>> +	wdog->timeout = timeout;
>>> +	arm_smccc_smc(IMX_SIP_TIMER,
>> IMX_SIP_TIMER_SET_TIMEOUT_WDOG,
>>> +		      timeout * 1000, 0, 0, 0, 0, 0, &res);
>>> +
>>> +	return res.a0;
>>> +}
>>> +
>>> +static const struct watchdog_ops imx_sc_wdt_ops = {
>>> +	.owner = THIS_MODULE,
>>> +	.start = imx_sc_wdt_start,
>>> +	.stop  = imx_sc_wdt_stop,
>>> +	.ping  = imx_sc_wdt_ping,
>>> +	.set_timeout = imx_sc_wdt_set_timeout, };
>>> +
>>> +static const struct watchdog_info imx_sc_wdt_info = {
>>> +	.identity	= "i.MX SC watchdog timer",
>>> +	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
>>> +			  WDIOF_MAGICCLOSE | WDIOF_PRETIMEOUT, };
>>> +
>>> +static int imx_sc_wdt_probe(struct platform_device *pdev) {
>>> +	struct watchdog_device *imx_sc_wdd;
>>> +	int ret;
>>> +
>>> +	imx_sc_wdd = devm_kzalloc(&pdev->dev, sizeof(*imx_sc_wdd),
>> GFP_KERNEL);
>>> +	if (!imx_sc_wdd)
>>> +		return -ENOMEM;
>>> +
>>> +	platform_set_drvdata(pdev, imx_sc_wdd);
>>> +
>>> +	imx_sc_wdd->info = &imx_sc_wdt_info;
>>> +	imx_sc_wdd->ops = &imx_sc_wdt_ops;
>>> +	imx_sc_wdd->min_timeout = 1;
>>> +	imx_sc_wdd->max_timeout = MAX_TIMEOUT;
>>> +	imx_sc_wdd->parent = &pdev->dev;
>>> +	imx_sc_wdd->timeout = DEFAULT_TIMEOUT;
>>> +
>>> +	ret = watchdog_init_timeout(imx_sc_wdd, timeout, &pdev->dev);
>>> +	if (ret)
>>> +		dev_warn(&pdev->dev, "Failed to set timeout value, using
>>> +default\n");
>>> +
>>> +	watchdog_stop_on_reboot(imx_sc_wdd);
>>> +	watchdog_stop_on_unregister(imx_sc_wdd);
>>> +
>>> +	ret = devm_watchdog_register_device(&pdev->dev, imx_sc_wdd);
>>> +	if (ret) {
>>> +		dev_err(&pdev->dev, "Failed to register watchdog device\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct of_device_id imx_sc_wdt_dt_ids[] = {
>>> +	{ .compatible = "fsl,imx-sc-wdt", },
>>> +	{ /* sentinel */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, imx_sc_wdt_dt_ids);
>>
>> I assume this will be dropped and replaced with platform code, per feedback
>> from Rob.
> 
> Yes, I will remove it in V4, just register platform device in driver directly.
> 
>>
>>> +
>>> +static int __maybe_unused imx_sc_wdt_suspend(struct device *dev) {
>>> +	struct watchdog_device *imx_sc_wdd = dev_get_drvdata(dev);
>>> +
>>> +	if (watchdog_active(imx_sc_wdd))
>>> +		imx_sc_wdt_stop(imx_sc_wdd);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int __maybe_unused imx_sc_wdt_resume(struct device *dev) {
>>> +	struct watchdog_device *imx_sc_wdd = dev_get_drvdata(dev);
>>> +
>>> +	if (watchdog_active(imx_sc_wdd))
>>> +		imx_sc_wdt_start(imx_sc_wdd);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(imx_sc_wdt_pm_ops,
>>> +			 imx_sc_wdt_suspend, imx_sc_wdt_resume);
>>> +
>>> +static struct platform_driver imx_sc_wdt_driver = {
>>> +	.probe		= imx_sc_wdt_probe,
>>> +	.driver		= {
>>> +		.name	= "imx-sc-wdt",
>>> +		.of_match_table = imx_sc_wdt_dt_ids,
>>> +		.pm	= &imx_sc_wdt_pm_ops,
>>> +	},
>>> +};
>>> +
>>> +module_platform_driver(imx_sc_wdt_driver);
>>> +
>>> +MODULE_AUTHOR("Robin Gong <yibin.gong@nxp.com>");
>>> +MODULE_DESCRIPTION("NXP i.MX system controller watchdog driver");
>>> +MODULE_LICENSE("GPL v2");
>>
>> The module license conflicts with the SPDX license which says 2.0+.
> 
> OK, I will change the SPDX license to 2.0.
> 
> Please help review V4 patch series, the dt-binding and dts patch will be removed.
> 
> Thanks,
> Anson.
>
diff mbox series

Patch

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 65c3c42..5c5b8ba 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -625,6 +625,19 @@  config IMX2_WDT
 	  To compile this driver as a module, choose M here: the
 	  module will be called imx2_wdt.
 
+config IMX_SC_WDT
+	tristate "IMX SC Watchdog"
+	depends on ARCH_MXC || COMPILE_TEST
+	select WATCHDOG_CORE
+	help
+	  This is the driver for the system controller watchdog
+	  on the NXP i.MX SoCs with system controller inside.
+	  If you have one of these processors and wish to have
+	  watchdog support enabled, say Y, otherwise say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called imx_sc_wdt.
+
 config UX500_WATCHDOG
 	tristate "ST-Ericsson Ux500 watchdog"
 	depends on MFD_DB8500_PRCMU
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 4e78a8c..0c9da63 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -68,6 +68,7 @@  obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o
 obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o
 obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
 obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
+obj-$(CONFIG_IMX_SC_WDT) += imx_sc_wdt.o
 obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
 obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
 obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
diff --git a/drivers/watchdog/imx_sc_wdt.c b/drivers/watchdog/imx_sc_wdt.c
new file mode 100644
index 0000000..7b22575
--- /dev/null
+++ b/drivers/watchdog/imx_sc_wdt.c
@@ -0,0 +1,183 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2018-2019 NXP.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reboot.h>
+#include <linux/watchdog.h>
+
+#define DEFAULT_TIMEOUT 60
+/*
+ * Software timer tick implemented in scfw side, support 10ms to 0xffffffff ms
+ * in theory, but for normal case, 1s~128s is enough, you can change this max
+ * value in case it's not enough.
+ */
+#define MAX_TIMEOUT 128
+
+#define IMX_SIP_TIMER			0xC2000002
+#define IMX_SIP_TIMER_START_WDOG		0x01
+#define IMX_SIP_TIMER_STOP_WDOG		0x02
+#define IMX_SIP_TIMER_SET_WDOG_ACT	0x03
+#define IMX_SIP_TIMER_PING_WDOG		0x04
+#define IMX_SIP_TIMER_SET_TIMEOUT_WDOG	0x05
+#define IMX_SIP_TIMER_GET_WDOG_STAT	0x06
+#define IMX_SIP_TIMER_SET_PRETIME_WDOG	0x07
+
+#define SC_TIMER_WDOG_ACTION_PARTITION	0
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0000);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+static unsigned int timeout = DEFAULT_TIMEOUT;
+module_param(timeout, uint, 0000);
+MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds (default="
+		 __MODULE_STRING(DEFAULT_TIMEOUT) ")");
+
+static int imx_sc_wdt_ping(struct watchdog_device *wdog)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_PING_WDOG,
+		      0, 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
+static int imx_sc_wdt_start(struct watchdog_device *wdog)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_START_WDOG,
+		      0, 0, 0, 0, 0, 0, &res);
+	if (res.a0)
+		return res.a0;
+
+	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_SET_WDOG_ACT,
+		      SC_TIMER_WDOG_ACTION_PARTITION,
+		      0, 0, 0, 0, 0, &res);
+	return res.a0;
+}
+
+static int imx_sc_wdt_stop(struct watchdog_device *wdog)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_STOP_WDOG,
+		      0, 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
+static int imx_sc_wdt_set_timeout(struct watchdog_device *wdog,
+				unsigned int timeout)
+{
+	struct arm_smccc_res res;
+
+	wdog->timeout = timeout;
+	arm_smccc_smc(IMX_SIP_TIMER, IMX_SIP_TIMER_SET_TIMEOUT_WDOG,
+		      timeout * 1000, 0, 0, 0, 0, 0, &res);
+
+	return res.a0;
+}
+
+static const struct watchdog_ops imx_sc_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = imx_sc_wdt_start,
+	.stop  = imx_sc_wdt_stop,
+	.ping  = imx_sc_wdt_ping,
+	.set_timeout = imx_sc_wdt_set_timeout,
+};
+
+static const struct watchdog_info imx_sc_wdt_info = {
+	.identity	= "i.MX SC watchdog timer",
+	.options	= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE | WDIOF_PRETIMEOUT,
+};
+
+static int imx_sc_wdt_probe(struct platform_device *pdev)
+{
+	struct watchdog_device *imx_sc_wdd;
+	int ret;
+
+	imx_sc_wdd = devm_kzalloc(&pdev->dev, sizeof(*imx_sc_wdd), GFP_KERNEL);
+	if (!imx_sc_wdd)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, imx_sc_wdd);
+
+	imx_sc_wdd->info = &imx_sc_wdt_info;
+	imx_sc_wdd->ops = &imx_sc_wdt_ops;
+	imx_sc_wdd->min_timeout = 1;
+	imx_sc_wdd->max_timeout = MAX_TIMEOUT;
+	imx_sc_wdd->parent = &pdev->dev;
+	imx_sc_wdd->timeout = DEFAULT_TIMEOUT;
+
+	ret = watchdog_init_timeout(imx_sc_wdd, timeout, &pdev->dev);
+	if (ret)
+		dev_warn(&pdev->dev, "Failed to set timeout value, using default\n");
+
+	watchdog_stop_on_reboot(imx_sc_wdd);
+	watchdog_stop_on_unregister(imx_sc_wdd);
+
+	ret = devm_watchdog_register_device(&pdev->dev, imx_sc_wdd);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register watchdog device\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id imx_sc_wdt_dt_ids[] = {
+	{ .compatible = "fsl,imx-sc-wdt", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx_sc_wdt_dt_ids);
+
+static int __maybe_unused imx_sc_wdt_suspend(struct device *dev)
+{
+	struct watchdog_device *imx_sc_wdd = dev_get_drvdata(dev);
+
+	if (watchdog_active(imx_sc_wdd))
+		imx_sc_wdt_stop(imx_sc_wdd);
+
+	return 0;
+}
+
+static int __maybe_unused imx_sc_wdt_resume(struct device *dev)
+{
+	struct watchdog_device *imx_sc_wdd = dev_get_drvdata(dev);
+
+	if (watchdog_active(imx_sc_wdd))
+		imx_sc_wdt_start(imx_sc_wdd);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(imx_sc_wdt_pm_ops,
+			 imx_sc_wdt_suspend, imx_sc_wdt_resume);
+
+static struct platform_driver imx_sc_wdt_driver = {
+	.probe		= imx_sc_wdt_probe,
+	.driver		= {
+		.name	= "imx-sc-wdt",
+		.of_match_table = imx_sc_wdt_dt_ids,
+		.pm	= &imx_sc_wdt_pm_ops,
+	},
+};
+
+module_platform_driver(imx_sc_wdt_driver);
+
+MODULE_AUTHOR("Robin Gong <yibin.gong@nxp.com>");
+MODULE_DESCRIPTION("NXP i.MX system controller watchdog driver");
+MODULE_LICENSE("GPL v2");