From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>

Add driver to support the SMC-based watchdog timer provided by the
Gunyah Hypervisor.

On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
through MMIO is not available. Depending on the hypervisor
configuration, the watchdog is either fully emulated or exposed via
ARM's SMC Calling Conventions (SMCCC) through the Vendor Specific
Hypervisor Service Calls space.

When the SMC-based interface is enabled, a device tree overlay is used
to provide the pretimeout interrupt configuration.

Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
---
 MAINTAINERS                   |   2 +
 drivers/watchdog/Kconfig      |  13 ++
 drivers/watchdog/Makefile     |   1 +
 drivers/watchdog/gunyah_wdt.c | 268 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/gunyah_errno.h  |  77 ++++++++++++
 5 files changed, 361 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 03b74513e4ac..5e491211d96c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3084,10 +3084,12 @@ F:	arch/arm64/boot/dts/qcom/
 F:	drivers/bus/qcom*
 F:	drivers/firmware/qcom/
 F:	drivers/soc/qcom/
+F:	drivers/watchdog/gunyah_wdt.c
 F:	include/dt-bindings/arm/qcom,ids.h
 F:	include/dt-bindings/firmware/qcom,scm.h
 F:	include/dt-bindings/soc/qcom*
 F:	include/linux/firmware/qcom
+F:	include/linux/gunyah_errno.h
 F:	include/linux/soc/qcom/
 F:	include/soc/qcom/
 
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0c25b2ed44eb..2fed83e06990 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -2343,4 +2343,17 @@ config KEEMBAY_WATCHDOG
 	  To compile this driver as a module, choose M here: the
 	  module will be called keembay_wdt.
 
+config GUNYAH_WATCHDOG
+	tristate "Qualcomm Gunyah Watchdog"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on HAVE_ARM_SMCCC
+	select WATCHDOG_CORE
+	help
+	  Say Y here to include support for watchdog timer provided by the
+	  Gunyah hypervisor. The driver uses ARM SMC Calling Convention (SMCCC)
+	  to interact with Gunyah Watchdog.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called gh_wdt.
+
 endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index bbd4d62d2cc3..308379782bc3 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -102,6 +102,7 @@ obj-$(CONFIG_MSC313E_WATCHDOG) += msc313e_wdt.o
 obj-$(CONFIG_APPLE_WATCHDOG) += apple_wdt.o
 obj-$(CONFIG_SUNPLUS_WATCHDOG) += sunplus_wdt.o
 obj-$(CONFIG_MARVELL_GTI_WDT) += marvell_gti_wdt.o
+obj-$(CONFIG_GUNYAH_WATCHDOG) += gunyah_wdt.o
 
 # X86 (i386 + ia64 + x86_64) Architecture
 obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
diff --git a/drivers/watchdog/gunyah_wdt.c b/drivers/watchdog/gunyah_wdt.c
new file mode 100644
index 000000000000..56e8e21510d3
--- /dev/null
+++ b/drivers/watchdog/gunyah_wdt.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/delay.h>
+#include <linux/gunyah_errno.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/watchdog.h>
+
+#define GUNYAH_WDT_SMCCC_CALL_VAL(func_id) \
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_32,\
+			   ARM_SMCCC_OWNER_VENDOR_HYP, func_id)
+
+/* SMCCC function IDs for watchdog operations */
+#define GUNYAH_WDT_CONTROL   GUNYAH_WDT_SMCCC_CALL_VAL(0x0005)
+#define GUNYAH_WDT_STATUS    GUNYAH_WDT_SMCCC_CALL_VAL(0x0006)
+#define GUNYAH_WDT_PING       GUNYAH_WDT_SMCCC_CALL_VAL(0x0007)
+#define GUNYAH_WDT_SET_TIME  GUNYAH_WDT_SMCCC_CALL_VAL(0x0008)
+
+/*
+ * Control values for GUNYAH_WDT_CONTROL.
+ * Bit 0 is used to enable or disable the watchdog. If this bit is set,
+ * then the watchdog is enabled and vice versa.
+ * Bit 1 should always be set to 1 as this bit is reserved in Gunyah and
+ * it's expected to be 1.
+ */
+#define WDT_CTRL_ENABLE  (BIT(1) | BIT(0))
+#define WDT_CTRL_DISABLE BIT(1)
+
+struct gunyah_wdt {
+	unsigned int pretimeout_irq;
+	struct watchdog_device wdd;
+};
+
+static int gunyah_wdt_call(unsigned long func_id, unsigned long arg1,
+			   unsigned long arg2, struct arm_smccc_res *res)
+{
+	arm_smccc_1_1_smc(func_id, arg1, arg2, res);
+	return gunyah_error_remap(res->a0);
+}
+
+static int gunyah_wdt_start(struct watchdog_device *wdd)
+{
+	struct arm_smccc_res res;
+	unsigned int timeout_ms;
+	unsigned int pretimeout_ms;
+	int ret;
+
+	ret = gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_DISABLE, 0, &res);
+	if (ret)
+		return ret;
+
+	timeout_ms = wdd->timeout * 1000;
+	pretimeout_ms = wdd->pretimeout * 1000;
+	ret = gunyah_wdt_call(GUNYAH_WDT_SET_TIME,
+			      pretimeout_ms, timeout_ms, &res);
+	if (ret)
+		return ret;
+
+	return gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_ENABLE, 0, &res);
+}
+
+static int gunyah_wdt_stop(struct watchdog_device *wdd)
+{
+	struct arm_smccc_res res;
+
+	return gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_DISABLE, 0, &res);
+}
+
+static int gunyah_wdt_ping(struct watchdog_device *wdd)
+{
+	struct arm_smccc_res res;
+
+	return gunyah_wdt_call(GUNYAH_WDT_PING, 0, 0, &res);
+}
+
+static int gunyah_wdt_set_timeout(struct watchdog_device *wdd,
+		     unsigned int timeout_sec)
+{
+	wdd->timeout = timeout_sec;
+
+	if (watchdog_active(wdd))
+		return gunyah_wdt_start(wdd);
+
+	return 0;
+}
+
+static int gunyah_wdt_set_pretimeout(struct watchdog_device *wdd,
+				     unsigned int pretimeout_sec)
+{
+	wdd->pretimeout = pretimeout_sec;
+
+	if (watchdog_active(wdd))
+		return gunyah_wdt_start(wdd);
+
+	return 0;
+}
+
+static unsigned int gunyah_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+	struct arm_smccc_res res;
+	unsigned int seconds_since_last_ping;
+	int ret;
+
+	ret = gunyah_wdt_call(GUNYAH_WDT_STATUS, 0, 0, &res);
+	if (ret)
+		return 0;
+
+	seconds_since_last_ping = res.a2 / 1000;
+	if (seconds_since_last_ping > wdd->timeout)
+		return 0;
+
+	return wdd->timeout - seconds_since_last_ping;
+}
+
+static int gunyah_wdt_restart(struct watchdog_device *wdd,
+			      unsigned long action, void *data)
+{
+	struct arm_smccc_res res;
+
+	/* Set timeout and pretimeout to 1ms and send a ping */
+	gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_ENABLE, 0, &res);
+	gunyah_wdt_call(GUNYAH_WDT_SET_TIME, 1, 1, &res);
+	gunyah_wdt_call(GUNYAH_WDT_PING, 0, 0, &res);
+
+	/* Wait to make sure reset occurs */
+	mdelay(100);
+
+	return 0;
+}
+
+static const struct watchdog_info gunyah_wdt_info = {
+	.identity = "Gunyah Watchdog",
+	.firmware_version = 0,
+	.options = WDIOF_SETTIMEOUT
+		 | WDIOF_PRETIMEOUT
+		 | WDIOF_KEEPALIVEPING
+		 | WDIOF_MAGICCLOSE,
+};
+
+static const struct watchdog_ops gunyah_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = gunyah_wdt_start,
+	.stop = gunyah_wdt_stop,
+	.ping = gunyah_wdt_ping,
+	.set_timeout = gunyah_wdt_set_timeout,
+	.set_pretimeout = gunyah_wdt_set_pretimeout,
+	.get_timeleft = gunyah_wdt_get_timeleft,
+	.restart = gunyah_wdt_restart
+};
+
+static irqreturn_t gunyah_wdt_pretimeout_handler(int irq, void *arg)
+{
+	struct watchdog_device *wdd = arg;
+
+	watchdog_notify_pretimeout(wdd);
+
+	return IRQ_HANDLED;
+}
+
+static int gunyah_wdt_probe(struct platform_device *pdev)
+{
+	struct gunyah_wdt *wdt;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
+	if (!wdt)
+		return -ENOMEM;
+
+	wdt->wdd.info = &gunyah_wdt_info;
+	wdt->wdd.ops = &gunyah_wdt_ops;
+	wdt->wdd.parent = dev;
+
+	/*
+	 * Although Gunyah expects 16-bit unsigned int values as timeout values
+	 * in milliseconds, values above 0x8000 are reserved. This limits the
+	 * max timeout value to 32 seconds.
+	 */
+	wdt->wdd.max_timeout = 32; /* seconds */
+	wdt->wdd.min_timeout = 1; /* seconds */
+	wdt->wdd.timeout = wdt->wdd.max_timeout;
+	wdt->wdd.pretimeout = wdt->wdd.timeout - 2;
+
+	gunyah_wdt_stop(&wdt->wdd);
+	watchdog_init_timeout(&wdt->wdd, 0, dev);
+
+	platform_set_drvdata(pdev, wdt);
+
+	watchdog_set_restart_priority(&wdt->wdd, 0);
+	ret = devm_watchdog_register_device(dev, &wdt->wdd);
+	if (ret) {
+		dev_err(dev, "Failed to register watchdog device: %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Register the pretimeout irq as rising edge triggered irrespective of
+	 * the irqflags passed by Gunyah to make the driver compatible with
+	 * pretimeout governors like noop.
+	 */
+	wdt->pretimeout_irq = platform_get_irq(pdev, 0);
+	ret = devm_request_irq(dev, wdt->pretimeout_irq,
+			       gunyah_wdt_pretimeout_handler,
+			       IRQF_TRIGGER_RISING,
+			       "wdt_pretimeout", &wdt->wdd);
+	if (ret) {
+		dev_err(dev, "Failed to register pretimeout irq: %d\n", ret);
+		return ret;
+	}
+
+	dev_dbg(dev, "Gunyah watchdog registered\n");
+	return 0;
+}
+
+static int __maybe_unused gunyah_wdt_suspend(struct device *dev)
+{
+	struct gunyah_wdt *wdt = dev_get_drvdata(dev);
+
+	if (watchdog_active(&wdt->wdd))
+		gunyah_wdt_stop(&wdt->wdd);
+
+	return 0;
+}
+
+static int __maybe_unused gunyah_wdt_resume(struct device *dev)
+{
+	struct gunyah_wdt *wdt = dev_get_drvdata(dev);
+
+	if (watchdog_active(&wdt->wdd))
+		gunyah_wdt_start(&wdt->wdd);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(gunyah_wdt_pm_ops, gunyah_wdt_suspend, gunyah_wdt_resume);
+
+static const struct of_device_id gunyah_wdt_of_match[] = {
+	{ .compatible = "qcom,gh-watchdog" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, gunyah_wdt_of_match);
+
+static struct platform_driver gunyah_wdt_driver = {
+	.probe = gunyah_wdt_probe,
+	.driver = {
+		.name = "gunyah-wdt",
+		.of_match_table = gunyah_wdt_of_match,
+		.pm = pm_sleep_ptr(&gunyah_wdt_pm_ops),
+	},
+};
+
+static int __init gunyah_wdt_init(void)
+{
+	return platform_driver_register(&gunyah_wdt_driver);
+}
+
+module_init(gunyah_wdt_init);
+
+MODULE_DESCRIPTION("Gunyah Watchdog Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/gunyah_errno.h b/include/linux/gunyah_errno.h
new file mode 100644
index 000000000000..518228e333bd
--- /dev/null
+++ b/include/linux/gunyah_errno.h
@@ -0,0 +1,77 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#ifndef _LINUX_GUNYAH_ERRNO_H
+#define _LINUX_GUNYAH_ERRNO_H
+
+#include <linux/errno.h>
+
+enum gunyah_error {
+	GUNYAH_ERROR_OK				= 0,
+	GUNYAH_ERROR_UNIMPLEMENTED		= -1,
+	GUNYAH_ERROR_RETRY			= -2,
+
+	GUNYAH_ERROR_ARG_INVAL			= 1,
+	GUNYAH_ERROR_ARG_SIZE			= 2,
+	GUNYAH_ERROR_ARG_ALIGN			= 3,
+
+	GUNYAH_ERROR_NOMEM			= 10,
+
+	GUNYAH_ERROR_ADDR_OVFL			= 20,
+	GUNYAH_ERROR_ADDR_UNFL			= 21,
+	GUNYAH_ERROR_ADDR_INVAL			= 22,
+
+	GUNYAH_ERROR_DENIED			= 30,
+	GUNYAH_ERROR_BUSY			= 31,
+	GUNYAH_ERROR_IDLE			= 32,
+
+	GUNYAH_ERROR_IRQ_BOUND			= 40,
+	GUNYAH_ERROR_IRQ_UNBOUND		= 41,
+
+	GUNYAH_ERROR_CSPACE_CAP_NULL		= 50,
+	GUNYAH_ERROR_CSPACE_CAP_REVOKED		= 51,
+	GUNYAH_ERROR_CSPACE_WRONG_OBJ_TYPE	= 52,
+	GUNYAH_ERROR_CSPACE_INSUF_RIGHTS	= 53,
+	GUNYAH_ERROR_CSPACE_FULL		= 54,
+
+	GUNYAH_ERROR_MSGQUEUE_EMPTY		= 60,
+	GUNYAH_ERROR_MSGQUEUE_FULL		= 61,
+};
+
+/**
+ * gunyah_error_remap() - Remap Gunyah hypervisor errors into a Linux error code
+ * @gunyah_error: Gunyah hypercall return value
+ */
+static inline int gunyah_error_remap(enum gunyah_error gunyah_error)
+{
+	switch (gunyah_error) {
+	case GUNYAH_ERROR_OK:
+		return 0;
+	case GUNYAH_ERROR_NOMEM:
+		return -ENOMEM;
+	case GUNYAH_ERROR_DENIED:
+	case GUNYAH_ERROR_CSPACE_CAP_NULL:
+	case GUNYAH_ERROR_CSPACE_CAP_REVOKED:
+	case GUNYAH_ERROR_CSPACE_WRONG_OBJ_TYPE:
+	case GUNYAH_ERROR_CSPACE_INSUF_RIGHTS:
+	case GUNYAH_ERROR_CSPACE_FULL:
+		return -EACCES;
+	case GUNYAH_ERROR_BUSY:
+	case GUNYAH_ERROR_IDLE:
+		return -EBUSY;
+	case GUNYAH_ERROR_IRQ_BOUND:
+	case GUNYAH_ERROR_IRQ_UNBOUND:
+	case GUNYAH_ERROR_MSGQUEUE_FULL:
+	case GUNYAH_ERROR_MSGQUEUE_EMPTY:
+		return -EIO;
+	case GUNYAH_ERROR_UNIMPLEMENTED:
+	case GUNYAH_ERROR_RETRY:
+		return -EOPNOTSUPP;
+	default:
+		return -EINVAL;
+	}
+}
+
+#endif

-- 
2.43.0

On Wed, Sep 03, 2025 at 07:34:00PM +0000, Hrishabh Rajput via B4 Relay wrote:
> +static int gunyah_wdt_call(unsigned long func_id, unsigned long arg1,
> +			   unsigned long arg2, struct arm_smccc_res *res)
> +{
> +	arm_smccc_1_1_smc(func_id, arg1, arg2, res);
> +	return gunyah_error_remap(res->a0);
> +}
> +
> +static int gunyah_wdt_start(struct watchdog_device *wdd)
> +{
> +	struct arm_smccc_res res;
> +	unsigned int timeout_ms;
> +	unsigned int pretimeout_ms;
> +	int ret;
> +
> +	ret = gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_DISABLE, 0, &res);
> +	if (ret)
> +		return ret;

When I ran a simple echo test, it failed here on SM8650 with -EINVAL. May be Gunyah
does not allow disabling watchdog when it is not enabled in the first
place. May be something you can check if this is a difference between
8750 vs 8650.

It also points out that your patch needs some prints upon error. Pls
check and update the patch accordingly.

> +
> +	timeout_ms = wdd->timeout * 1000;
> +	pretimeout_ms = wdd->pretimeout * 1000;
> +	ret = gunyah_wdt_call(GUNYAH_WDT_SET_TIME,
> +			      pretimeout_ms, timeout_ms, &res);
> +	if (ret)
> +		return ret;
> +
> +	return gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_ENABLE, 0, &res);
> +}

On 9/4/25 7:11 PM, Pavan Kondeti wrote:
> On Wed, Sep 03, 2025 at 07:34:00PM +0000, Hrishabh Rajput via B4 Relay wrote:
>> +static int gunyah_wdt_call(unsigned long func_id, unsigned long arg1,
>> +			   unsigned long arg2, struct arm_smccc_res *res)
>> +{
>> +	arm_smccc_1_1_smc(func_id, arg1, arg2, res);
>> +	return gunyah_error_remap(res->a0);
>> +}
>> +
>> +static int gunyah_wdt_start(struct watchdog_device *wdd)
>> +{
>> +	struct arm_smccc_res res;
>> +	unsigned int timeout_ms;
>> +	unsigned int pretimeout_ms;
>> +	int ret;
>> +
>> +	ret = gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_DISABLE, 0, &res);
>> +	if (ret)
>> +		return ret;
> 
> When I ran a simple echo test, it failed here on SM8650 with -EINVAL. May be Gunyah
> does not allow disabling watchdog when it is not enabled in the first
> place. May be something you can check if this is a difference between
> 8750 vs 8650.

Hm, makes one wonder if the opposite (won't enable when already enabled)
is true..

Konrad

On Wed, Sep 03, 2025 at 07:34:00PM +0000, Hrishabh Rajput via B4 Relay wrote:
> From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
> 
> Add driver to support the SMC-based watchdog timer provided by the
> Gunyah Hypervisor.
> 

Start the commit message with a problem description, end with a
technical description of the solution. I.e. move this paragraph down.

> On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
> through MMIO is not available. Depending on the hypervisor
> configuration, the watchdog is either fully emulated or exposed via
> ARM's SMC Calling Conventions (SMCCC) through the Vendor Specific
> Hypervisor Service Calls space.
> 
> When the SMC-based interface is enabled, a device tree overlay is used
> to provide the pretimeout interrupt configuration.
> 
> Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
[..]
> diff --git a/drivers/watchdog/gunyah_wdt.c b/drivers/watchdog/gunyah_wdt.c
[..]
> +#define GUNYAH_WDT_SMCCC_CALL_VAL(func_id) \
> +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_32,\
> +			   ARM_SMCCC_OWNER_VENDOR_HYP, func_id)
> +
> +/* SMCCC function IDs for watchdog operations */
> +#define GUNYAH_WDT_CONTROL   GUNYAH_WDT_SMCCC_CALL_VAL(0x0005)
> +#define GUNYAH_WDT_STATUS    GUNYAH_WDT_SMCCC_CALL_VAL(0x0006)
> +#define GUNYAH_WDT_PING       GUNYAH_WDT_SMCCC_CALL_VAL(0x0007)

Uneven indentation.

> +#define GUNYAH_WDT_SET_TIME  GUNYAH_WDT_SMCCC_CALL_VAL(0x0008)
> +
> +/*
> + * Control values for GUNYAH_WDT_CONTROL.
> + * Bit 0 is used to enable or disable the watchdog. If this bit is set,
> + * then the watchdog is enabled and vice versa.
> + * Bit 1 should always be set to 1 as this bit is reserved in Gunyah and
> + * it's expected to be 1.
> + */
> +#define WDT_CTRL_ENABLE  (BIT(1) | BIT(0))
> +#define WDT_CTRL_DISABLE BIT(1)
> +
> +struct gunyah_wdt {
> +	unsigned int pretimeout_irq;

This is only used momentarily in gunyah_wdt_probe(), make it a local
variable.

> +	struct watchdog_device wdd;

Which means that gunyah_wdt is just watchdog_device, so you can drop
gunyah_wdt completely, and put wdd directly in drvdata.

> +};
> +
[..]
> +static int __init gunyah_wdt_init(void)
> +{
> +	return platform_driver_register(&gunyah_wdt_driver);
> +}
> +
> +module_init(gunyah_wdt_init);

module_platform_driver(gunyah_wdt_driver);

> +
> +MODULE_DESCRIPTION("Gunyah Watchdog Driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/gunyah_errno.h b/include/linux/gunyah_errno.h
> new file mode 100644
> index 000000000000..518228e333bd
> --- /dev/null
> +++ b/include/linux/gunyah_errno.h
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */

Isn't this content solely used from within gunyah_wdt.c? Why is it a
separate header file? Just move it into the c-file.

Regards,
Bjorn

> +
> +#ifndef _LINUX_GUNYAH_ERRNO_H
> +#define _LINUX_GUNYAH_ERRNO_H
> +
> +#include <linux/errno.h>
> +
> +enum gunyah_error {
> +	GUNYAH_ERROR_OK				= 0,
> +	GUNYAH_ERROR_UNIMPLEMENTED		= -1,
> +	GUNYAH_ERROR_RETRY			= -2,
> +
> +	GUNYAH_ERROR_ARG_INVAL			= 1,
> +	GUNYAH_ERROR_ARG_SIZE			= 2,
> +	GUNYAH_ERROR_ARG_ALIGN			= 3,
> +
> +	GUNYAH_ERROR_NOMEM			= 10,
> +
> +	GUNYAH_ERROR_ADDR_OVFL			= 20,
> +	GUNYAH_ERROR_ADDR_UNFL			= 21,
> +	GUNYAH_ERROR_ADDR_INVAL			= 22,
> +
> +	GUNYAH_ERROR_DENIED			= 30,
> +	GUNYAH_ERROR_BUSY			= 31,
> +	GUNYAH_ERROR_IDLE			= 32,
> +
> +	GUNYAH_ERROR_IRQ_BOUND			= 40,
> +	GUNYAH_ERROR_IRQ_UNBOUND		= 41,
> +
> +	GUNYAH_ERROR_CSPACE_CAP_NULL		= 50,
> +	GUNYAH_ERROR_CSPACE_CAP_REVOKED		= 51,
> +	GUNYAH_ERROR_CSPACE_WRONG_OBJ_TYPE	= 52,
> +	GUNYAH_ERROR_CSPACE_INSUF_RIGHTS	= 53,
> +	GUNYAH_ERROR_CSPACE_FULL		= 54,
> +
> +	GUNYAH_ERROR_MSGQUEUE_EMPTY		= 60,
> +	GUNYAH_ERROR_MSGQUEUE_FULL		= 61,
> +};
> +
> +/**
> + * gunyah_error_remap() - Remap Gunyah hypervisor errors into a Linux error code
> + * @gunyah_error: Gunyah hypercall return value
> + */
> +static inline int gunyah_error_remap(enum gunyah_error gunyah_error)
> +{
> +	switch (gunyah_error) {
> +	case GUNYAH_ERROR_OK:
> +		return 0;
> +	case GUNYAH_ERROR_NOMEM:
> +		return -ENOMEM;
> +	case GUNYAH_ERROR_DENIED:
> +	case GUNYAH_ERROR_CSPACE_CAP_NULL:
> +	case GUNYAH_ERROR_CSPACE_CAP_REVOKED:
> +	case GUNYAH_ERROR_CSPACE_WRONG_OBJ_TYPE:
> +	case GUNYAH_ERROR_CSPACE_INSUF_RIGHTS:
> +	case GUNYAH_ERROR_CSPACE_FULL:
> +		return -EACCES;
> +	case GUNYAH_ERROR_BUSY:
> +	case GUNYAH_ERROR_IDLE:
> +		return -EBUSY;
> +	case GUNYAH_ERROR_IRQ_BOUND:
> +	case GUNYAH_ERROR_IRQ_UNBOUND:
> +	case GUNYAH_ERROR_MSGQUEUE_FULL:
> +	case GUNYAH_ERROR_MSGQUEUE_EMPTY:
> +		return -EIO;
> +	case GUNYAH_ERROR_UNIMPLEMENTED:
> +	case GUNYAH_ERROR_RETRY:
> +		return -EOPNOTSUPP;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +#endif
> 
> -- 
> 2.43.0
> 
>

On 9/4/2025 1:43 AM, Bjorn Andersson wrote:
> On Wed, Sep 03, 2025 at 07:34:00PM +0000, Hrishabh Rajput via B4 Relay wrote:
>> From: Hrishabh Rajput<hrishabh.rajput@oss.qualcomm.com>
>>
>> Add driver to support the SMC-based watchdog timer provided by the
>> Gunyah Hypervisor.
>>
> Start the commit message with a problem description, end with a
> technical description of the solution. I.e. move this paragraph down.


Thanks, that would make more sense. Will rearrange this.

>> On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
>> through MMIO is not available. Depending on the hypervisor
>> configuration, the watchdog is either fully emulated or exposed via
>> ARM's SMC Calling Conventions (SMCCC) through the Vendor Specific
>> Hypervisor Service Calls space.
>>
>> When the SMC-based interface is enabled, a device tree overlay is used
>> to provide the pretimeout interrupt configuration.
>>
>> Signed-off-by: Hrishabh Rajput<hrishabh.rajput@oss.qualcomm.com>
> [..]
>> diff --git a/drivers/watchdog/gunyah_wdt.c b/drivers/watchdog/gunyah_wdt.c
> [..]
>> +#define GUNYAH_WDT_SMCCC_CALL_VAL(func_id) \
>> +	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, ARM_SMCCC_SMC_32,\
>> +			   ARM_SMCCC_OWNER_VENDOR_HYP, func_id)
>> +
>> +/* SMCCC function IDs for watchdog operations */
>> +#define GUNYAH_WDT_CONTROL   GUNYAH_WDT_SMCCC_CALL_VAL(0x0005)
>> +#define GUNYAH_WDT_STATUS    GUNYAH_WDT_SMCCC_CALL_VAL(0x0006)
>> +#define GUNYAH_WDT_PING       GUNYAH_WDT_SMCCC_CALL_VAL(0x0007)
> Uneven indentation.


This crept in somehow. Will fix it. Thanks.

>> +#define GUNYAH_WDT_SET_TIME  GUNYAH_WDT_SMCCC_CALL_VAL(0x0008)
>> +
>> +/*
>> + * Control values for GUNYAH_WDT_CONTROL.
>> + * Bit 0 is used to enable or disable the watchdog. If this bit is set,
>> + * then the watchdog is enabled and vice versa.
>> + * Bit 1 should always be set to 1 as this bit is reserved in Gunyah and
>> + * it's expected to be 1.
>> + */
>> +#define WDT_CTRL_ENABLE  (BIT(1) | BIT(0))
>> +#define WDT_CTRL_DISABLE BIT(1)
>> +
>> +struct gunyah_wdt {
>> +	unsigned int pretimeout_irq;
> This is only used momentarily in gunyah_wdt_probe(), make it a local
> variable.
>> +	struct watchdog_device wdd;
> Which means that gunyah_wdt is just watchdog_device, so you can drop
> gunyah_wdt completely, and put wdd directly in drvdata.


That would definitely be a better way to do it. Thanks.

>> +};
>> +
> [..]
>> +static int __init gunyah_wdt_init(void)
>> +{
>> +	return platform_driver_register(&gunyah_wdt_driver);
>> +}
>> +
>> +module_init(gunyah_wdt_init);
> module_platform_driver(gunyah_wdt_driver);


This is intentional. I intend to keep this module persistent. No 
module_exit(gunyah_wdt_exit).

>> +
>> +MODULE_DESCRIPTION("Gunyah Watchdog Driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/linux/gunyah_errno.h b/include/linux/gunyah_errno.h
>> new file mode 100644
>> index 000000000000..518228e333bd
>> --- /dev/null
>> +++ b/include/linux/gunyah_errno.h
>> @@ -0,0 +1,77 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
> Isn't this content solely used from within gunyah_wdt.c? Why is it a
> separate header file? Just move it into the c-file.
> Regards,
> Bjorn


This header file is partially taken from [1] and I have only renamed it 
to gh_errno.h.

The error codes are not specific to watchdog and we have other drivers 
in the patch series [2] (which [1] is a part of) that would be using this.

[1] 
https://lore.kernel.org/all/20240222-gunyah-v17-3-1e9da6763d38@quicinc.com/

[2] 
https://lore.kernel.org/all/20240222-gunyah-v17-0-1e9da6763d38@quicinc.com/

Thanks,

Hrishabh

>> +
>> +#ifndef _LINUX_GUNYAH_ERRNO_H
>> +#define _LINUX_GUNYAH_ERRNO_H
>> +
>> +#include <linux/errno.h>
>> +
>> +enum gunyah_error {
>> +	GUNYAH_ERROR_OK				= 0,
>> +	GUNYAH_ERROR_UNIMPLEMENTED		= -1,
>> +	GUNYAH_ERROR_RETRY			= -2,
>> +
>> +	GUNYAH_ERROR_ARG_INVAL			= 1,
>> +	GUNYAH_ERROR_ARG_SIZE			= 2,
>> +	GUNYAH_ERROR_ARG_ALIGN			= 3,
>> +
>> +	GUNYAH_ERROR_NOMEM			= 10,
>> +
>> +	GUNYAH_ERROR_ADDR_OVFL			= 20,
>> +	GUNYAH_ERROR_ADDR_UNFL			= 21,
>> +	GUNYAH_ERROR_ADDR_INVAL			= 22,
>> +
>> +	GUNYAH_ERROR_DENIED			= 30,
>> +	GUNYAH_ERROR_BUSY			= 31,
>> +	GUNYAH_ERROR_IDLE			= 32,
>> +
>> +	GUNYAH_ERROR_IRQ_BOUND			= 40,
>> +	GUNYAH_ERROR_IRQ_UNBOUND		= 41,
>> +
>> +	GUNYAH_ERROR_CSPACE_CAP_NULL		= 50,
>> +	GUNYAH_ERROR_CSPACE_CAP_REVOKED		= 51,
>> +	GUNYAH_ERROR_CSPACE_WRONG_OBJ_TYPE	= 52,
>> +	GUNYAH_ERROR_CSPACE_INSUF_RIGHTS	= 53,
>> +	GUNYAH_ERROR_CSPACE_FULL		= 54,
>> +
>> +	GUNYAH_ERROR_MSGQUEUE_EMPTY		= 60,
>> +	GUNYAH_ERROR_MSGQUEUE_FULL		= 61,
>> +};
>> +
>> +/**
>> + * gunyah_error_remap() - Remap Gunyah hypervisor errors into a Linux error code
>> + * @gunyah_error: Gunyah hypercall return value
>> + */
>> +static inline int gunyah_error_remap(enum gunyah_error gunyah_error)
>> +{
>> +	switch (gunyah_error) {
>> +	case GUNYAH_ERROR_OK:
>> +		return 0;
>> +	case GUNYAH_ERROR_NOMEM:
>> +		return -ENOMEM;
>> +	case GUNYAH_ERROR_DENIED:
>> +	case GUNYAH_ERROR_CSPACE_CAP_NULL:
>> +	case GUNYAH_ERROR_CSPACE_CAP_REVOKED:
>> +	case GUNYAH_ERROR_CSPACE_WRONG_OBJ_TYPE:
>> +	case GUNYAH_ERROR_CSPACE_INSUF_RIGHTS:
>> +	case GUNYAH_ERROR_CSPACE_FULL:
>> +		return -EACCES;
>> +	case GUNYAH_ERROR_BUSY:
>> +	case GUNYAH_ERROR_IDLE:
>> +		return -EBUSY;
>> +	case GUNYAH_ERROR_IRQ_BOUND:
>> +	case GUNYAH_ERROR_IRQ_UNBOUND:
>> +	case GUNYAH_ERROR_MSGQUEUE_FULL:
>> +	case GUNYAH_ERROR_MSGQUEUE_EMPTY:
>> +		return -EIO;
>> +	case GUNYAH_ERROR_UNIMPLEMENTED:
>> +	case GUNYAH_ERROR_RETRY:
>> +		return -EOPNOTSUPP;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>> +#endif
>>
>> -- 
>> 2.43.0
>>
>>

On Thu, Sep 04, 2025 at 05:10:33PM +0530, Hrishabh Rajput wrote:
> On 9/4/2025 1:43 AM, Bjorn Andersson wrote:
> > On Wed, Sep 03, 2025 at 07:34:00PM +0000, Hrishabh Rajput via B4 Relay wrote:
> > > diff --git a/drivers/watchdog/gunyah_wdt.c b/drivers/watchdog/gunyah_wdt.c
[..]
> > > +static int __init gunyah_wdt_init(void)
> > > +{
> > > +	return platform_driver_register(&gunyah_wdt_driver);
> > > +}
> > > +
> > > +module_init(gunyah_wdt_init);
> > module_platform_driver(gunyah_wdt_driver);
> 
> 
> This is intentional. I intend to keep this module persistent. No
> module_exit(gunyah_wdt_exit).
> 

I'm not sure I see the reason for doing so, but if that really is what
you meant, you should say so in the commit message or a comment -
otherwise someone will send a patch "fixing" it first thing tomorrow.

> > > +
> > > +MODULE_DESCRIPTION("Gunyah Watchdog Driver");
> > > +MODULE_LICENSE("GPL");
> > > diff --git a/include/linux/gunyah_errno.h b/include/linux/gunyah_errno.h
> > > new file mode 100644
> > > index 000000000000..518228e333bd
> > > --- /dev/null
> > > +++ b/include/linux/gunyah_errno.h
> > > @@ -0,0 +1,77 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > > + */
> > Isn't this content solely used from within gunyah_wdt.c? Why is it a
> > separate header file? Just move it into the c-file.
> > Regards,
> > Bjorn
> 
> 
> This header file is partially taken from [1] and I have only renamed it to
> gh_errno.h.

Let's not sprinkle include/linux/ with include files which might be
useful some day in the future. Put the information in the c-file and
when there is a second user (e.g. [1] is resubmitted) we can migrate it
to a include file at that time.

Thanks,
Bjorn

> 
> The error codes are not specific to watchdog and we have other drivers in
> the patch series [2] (which [1] is a part of) that would be using this.
> 
> [1]
> https://lore.kernel.org/all/20240222-gunyah-v17-3-1e9da6763d38@quicinc.com/
> 
> [2]
> https://lore.kernel.org/all/20240222-gunyah-v17-0-1e9da6763d38@quicinc.com/