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

On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
through MMIO is not available on all platforms. 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 Gunyah is not present or Gunyah emulates MMIO-based watchdog, we
expect MMIO watchdog device to be present in the devicetree. If we
detect this device node, we don't proceed ahead. Otherwise, we go ahead
and invoke GUNYAH_WDT_STATUS SMC to initiate the discovery of the
SMC-based watchdog.

Add driver to support the SMC-based watchdog provided by the Gunyah
Hypervisor. module_exit() is intentionally not implemented as this
driver is intended to be a persistent module.

Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
---
Gunyah is a Type-I hypervisor which was introduced in the patch series
[1]. It is an open source hypervisor. The source repo is available at
[2].

The Gunyah Hypervisor doesn't allow its Virtual Machines to directly
access the MMIO watchdog. It either provides the fully emulated MMIO
based watchdog interface or the SMC-based watchdog interface depending
on the hypervisor configuration.
The SMC-based watchdog follows ARM's SMC Calling Convention (SMCCC)
version 1.1 and uses Vendor Specific Hypervisor Service Calls space.

This patch series adds support for the SMC-based watchdog interface
provided by the Gunyah Hypervisor.

This series is tested on SM8750 platform.

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

[2]
https://github.com/quic/gunyah-hypervisor
---
Changes in v2:
- Move away from platform driver model since the devicetree overlay does
  not happen by default.
  See https://lore.kernel.org/all/91002189-9d9e-48a2-8424-c42705fed3f8@quicinc.com/
- Only when MMIO-based watchdog device is absent in the devicetree,
  proceed to detect SMC-based watchdog using GUNYAH_WDT_STATUS SMC and
  initialize if SMC returns success.
- Implement pm notifiers as gunyah_wdt is no longer a platform driver so
  dev_pm_ops cannot be used.
- Pretimeout IRQ is no longer supported.
- Remove struct gunyah_wdt since it is not required.
- Move the contents of gunyah_errno.h to gunyah_wdt.c.
- Link to v1: https://lore.kernel.org/r/20250903-gunyah_watchdog-v1-0-3ae690530e4b@oss.qualcomm.com
---
 MAINTAINERS                   |   1 +
 drivers/watchdog/Kconfig      |  14 ++
 drivers/watchdog/Makefile     |   1 +
 drivers/watchdog/gunyah_wdt.c | 310 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 326 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c0b444e5fd5a..56dbd0d3e31b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3083,6 +3083,7 @@ 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*
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0c25b2ed44eb..c6572d707ee9 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -2343,4 +2343,18 @@ 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
+	depends on CONFIG_OF
+	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 gunyah_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..0e4b8196709e
--- /dev/null
+++ b/drivers/watchdog/gunyah_wdt.c
@@ -0,0 +1,310 @@
+// 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/errno.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/suspend.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)
+
+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;
+	}
+}
+
+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;
+	int ret;
+
+	ret = gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_DISABLE, 0, &res);
+	if (ret && watchdog_active(wdd)) {
+		pr_err("%s: Failed to stop gunyah wdt %d\n", __func__, ret);
+		return ret;
+	}
+
+	timeout_ms = wdd->timeout * 1000;
+	ret = gunyah_wdt_call(GUNYAH_WDT_SET_TIME,
+			      timeout_ms, timeout_ms, &res);
+	if (ret) {
+		pr_err("%s: Failed to set timeout for gunyah wdt %d\n",
+		       __func__,  ret);
+		return ret;
+	}
+
+	ret = gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_ENABLE, 0, &res);
+	if (ret)
+		pr_err("%s: Failed to start gunyah wdt %d\n", __func__, ret);
+
+	return ret;
+}
+
+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 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 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_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,
+	.get_timeleft = gunyah_wdt_get_timeleft,
+	.restart = gunyah_wdt_restart
+};
+
+static int gunyah_wdt_pm_notifier(struct notifier_block *nb, unsigned long mode,
+				  void *data)
+{
+	struct watchdog_device *wdd;
+	int ret = 0;
+
+	wdd = container_of(nb, struct watchdog_device, pm_nb);
+
+	if (!watchdog_active(wdd))
+		return NOTIFY_DONE;
+
+	switch (mode) {
+	case PM_HIBERNATION_PREPARE:
+	case PM_RESTORE_PREPARE:
+	case PM_SUSPEND_PREPARE:
+		gunyah_wdt_ping(wdd);
+		ret = gunyah_wdt_stop(wdd);
+		if (ret)
+			return NOTIFY_BAD;
+		break;
+	case PM_POST_HIBERNATION:
+	case PM_POST_RESTORE:
+	case PM_POST_SUSPEND:
+		ret = gunyah_wdt_start(wdd);
+		if (ret)
+			return NOTIFY_BAD;
+		gunyah_wdt_ping(wdd);
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static int __init gunyah_wdt_init(void)
+{
+	struct arm_smccc_res res;
+	struct watchdog_device *wdd;
+	struct device_node *np;
+	int ret;
+
+	np = of_find_compatible_node(NULL, NULL, "qcom,kpss-wdt");
+	if (np) {
+		of_node_put(np);
+		return -ENODEV;
+	}
+
+	np = of_find_compatible_node(NULL, NULL, "arm,sbsa-gwdt");
+	if (np) {
+		of_node_put(np);
+		return -ENODEV;
+	}
+
+	ret = gunyah_wdt_call(GUNYAH_WDT_STATUS, 0, 0, &res);
+	if (ret)
+		return -ENODEV;
+
+	wdd = kzalloc(sizeof(*wdd), GFP_KERNEL);
+	if (!wdd)
+		return -ENOMEM;
+
+	wdd->info = &gunyah_wdt_info;
+	wdd->ops = &gunyah_wdt_ops;
+
+	/*
+	 * 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.
+	 */
+	wdd->max_timeout = 32; /* seconds */
+	wdd->min_timeout = 1; /* seconds */
+	wdd->timeout = wdd->max_timeout;
+
+	gunyah_wdt_stop(wdd);
+
+	watchdog_set_restart_priority(wdd, 0);
+
+	wdd->pm_nb.notifier_call = gunyah_wdt_pm_notifier;
+	ret = register_pm_notifier(&wdd->pm_nb);
+	if (ret)
+		pr_warn("Failed to register pm handler: %d\n", ret);
+
+	ret = watchdog_register_device(wdd);
+	if (ret) {
+		pr_err("Failed to register watchdog device: %d\n", ret);
+		unregister_pm_notifier(&wdd->pm_nb);
+		kfree(wdd);
+		return ret;
+	}
+
+	pr_debug("Gunyah watchdog registered\n");
+	return 0;
+}
+
+module_init(gunyah_wdt_init);
+
+MODULE_DESCRIPTION("Gunyah Watchdog Driver");
+MODULE_LICENSE("GPL");

---
base-commit: 038d61fd642278bab63ee8ef722c50d10ab01e8f
change-id: 20250903-gunyah_watchdog-2d2649438e29

Best regards,
-- 
Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>

On 10/6/25 00:37, Hrishabh Rajput via B4 Relay wrote:
> From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
> 
> On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
> through MMIO is not available on all platforms. 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 Gunyah is not present or Gunyah emulates MMIO-based watchdog, we
> expect MMIO watchdog device to be present in the devicetree. If we
> detect this device node, we don't proceed ahead. Otherwise, we go ahead
> and invoke GUNYAH_WDT_STATUS SMC to initiate the discovery of the
> SMC-based watchdog.
> 
> Add driver to support the SMC-based watchdog provided by the Gunyah
> Hypervisor. module_exit() is intentionally not implemented as this
> driver is intended to be a persistent module.
> 
> Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
> ---
> Gunyah is a Type-I hypervisor which was introduced in the patch series
> [1]. It is an open source hypervisor. The source repo is available at
> [2].
> 
> The Gunyah Hypervisor doesn't allow its Virtual Machines to directly
> access the MMIO watchdog. It either provides the fully emulated MMIO
> based watchdog interface or the SMC-based watchdog interface depending
> on the hypervisor configuration.
> The SMC-based watchdog follows ARM's SMC Calling Convention (SMCCC)
> version 1.1 and uses Vendor Specific Hypervisor Service Calls space.
> 
> This patch series adds support for the SMC-based watchdog interface
> provided by the Gunyah Hypervisor.
> 
> This series is tested on SM8750 platform.
> 
> [1]
> https://lore.kernel.org/all/20240222-gunyah-v17-0-1e9da6763d38@quicinc.com/
> 
> [2]
> https://github.com/quic/gunyah-hypervisor
> ---
> Changes in v2:
> - Move away from platform driver model since the devicetree overlay does
>    not happen by default.

This is just wrong. Platform drivers do not depend on devicetree. I am not even
going to review the rest of the driver.

Guenter

On 10/6/2025 7:48 PM, Guenter Roeck wrote:
> On 10/6/25 00:37, Hrishabh Rajput via B4 Relay wrote:
>> From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
>>
>> On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
>> through MMIO is not available on all platforms. 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 Gunyah is not present or Gunyah emulates MMIO-based watchdog, we
>> expect MMIO watchdog device to be present in the devicetree. If we
>> detect this device node, we don't proceed ahead. Otherwise, we go ahead
>> and invoke GUNYAH_WDT_STATUS SMC to initiate the discovery of the
>> SMC-based watchdog.
>>
>> Add driver to support the SMC-based watchdog provided by the Gunyah
>> Hypervisor. module_exit() is intentionally not implemented as this
>> driver is intended to be a persistent module.
>>
>> Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
>> ---
>> Gunyah is a Type-I hypervisor which was introduced in the patch series
>> [1]. It is an open source hypervisor. The source repo is available at
>> [2].
>>
>> The Gunyah Hypervisor doesn't allow its Virtual Machines to directly
>> access the MMIO watchdog. It either provides the fully emulated MMIO
>> based watchdog interface or the SMC-based watchdog interface depending
>> on the hypervisor configuration.
>> The SMC-based watchdog follows ARM's SMC Calling Convention (SMCCC)
>> version 1.1 and uses Vendor Specific Hypervisor Service Calls space.
>>
>> This patch series adds support for the SMC-based watchdog interface
>> provided by the Gunyah Hypervisor.
>>
>> This series is tested on SM8750 platform.
>>
>> [1]
>> https://lore.kernel.org/all/20240222-gunyah-v17-0-1e9da6763d38@quicinc.com/ 
>>
>>
>> [2]
>> https://github.com/quic/gunyah-hypervisor
>> ---
>> Changes in v2:
>> - Move away from platform driver model since the devicetree overlay does
>>    not happen by default.
>
> This is just wrong. Platform drivers do not depend on devicetree. I am 
> not even
> going to review the rest of the driver. 

Thanks for pointing out the mistake here. Platform drivers are 
independent of devicetree. Therefore the line you've pointed to is wrong 
as it erroneously portrays that the platform drivers are dependent on 
devicetrees. It is a mistake and I would rephrase it to following to 
make the intent clearer:

"Do not depend on devicetree to discover (and probe) watchdog as 
devicetree overlay does not happen by default. Instead invoke 
GUNYAH_WDT_STATUS SMC Call to discover (and initialize) the watchdog."


Thanks,
Hrishabh

On 10/6/25 23:52, Hrishabh Rajput wrote:
> 
> On 10/6/2025 7:48 PM, Guenter Roeck wrote:
>> On 10/6/25 00:37, Hrishabh Rajput via B4 Relay wrote:
>>> From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
>>>
>>> On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
>>> through MMIO is not available on all platforms. 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 Gunyah is not present or Gunyah emulates MMIO-based watchdog, we
>>> expect MMIO watchdog device to be present in the devicetree. If we
>>> detect this device node, we don't proceed ahead. Otherwise, we go ahead
>>> and invoke GUNYAH_WDT_STATUS SMC to initiate the discovery of the
>>> SMC-based watchdog.
>>>
>>> Add driver to support the SMC-based watchdog provided by the Gunyah
>>> Hypervisor. module_exit() is intentionally not implemented as this
>>> driver is intended to be a persistent module.
>>>
>>> Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
>>> ---
>>> Gunyah is a Type-I hypervisor which was introduced in the patch series
>>> [1]. It is an open source hypervisor. The source repo is available at
>>> [2].
>>>
>>> The Gunyah Hypervisor doesn't allow its Virtual Machines to directly
>>> access the MMIO watchdog. It either provides the fully emulated MMIO
>>> based watchdog interface or the SMC-based watchdog interface depending
>>> on the hypervisor configuration.
>>> The SMC-based watchdog follows ARM's SMC Calling Convention (SMCCC)
>>> version 1.1 and uses Vendor Specific Hypervisor Service Calls space.
>>>
>>> This patch series adds support for the SMC-based watchdog interface
>>> provided by the Gunyah Hypervisor.
>>>
>>> This series is tested on SM8750 platform.
>>>
>>> [1]
>>> https://lore.kernel.org/all/20240222-gunyah-v17-0-1e9da6763d38@quicinc.com/
>>>
>>> [2]
>>> https://github.com/quic/gunyah-hypervisor
>>> ---
>>> Changes in v2:
>>> - Move away from platform driver model since the devicetree overlay does
>>>    not happen by default.
>>
>> This is just wrong. Platform drivers do not depend on devicetree. I am not even
>> going to review the rest of the driver. 
> 
> Thanks for pointing out the mistake here. Platform drivers are independent of devicetree. Therefore the line you've pointed to is wrong as it erroneously portrays that the platform drivers are dependent on devicetrees. It is a mistake and I would rephrase it to following to make the intent clearer:
> 
> "Do not depend on devicetree to discover (and probe) watchdog as devicetree overlay does not happen by default. Instead invoke GUNYAH_WDT_STATUS SMC Call to discover (and initialize) the watchdog."
> 

Let _me_ rephrase: A platform driver does not depend on devicetree.
This can and should be a platform driver.

Guenter

On 10/6/25 9:37 AM, Hrishabh Rajput via B4 Relay wrote:
> From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
> 
> On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
> through MMIO is not available on all platforms. 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 Gunyah is not present or Gunyah emulates MMIO-based watchdog, we
> expect MMIO watchdog device to be present in the devicetree. If we
> detect this device node, we don't proceed ahead. Otherwise, we go ahead
> and invoke GUNYAH_WDT_STATUS SMC to initiate the discovery of the
> SMC-based watchdog.
> 
> Add driver to support the SMC-based watchdog provided by the Gunyah
> Hypervisor. module_exit() is intentionally not implemented as this
> driver is intended to be a persistent module.
> 
> Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
> ---

[...]

> +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,
> +};

Can the calls we make in this driver produce all of these errors?

I'm asking only because we want to minimize the footprint

[...]

> +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;

> +static int __init gunyah_wdt_init(void)
> +{
> +	struct arm_smccc_res res;
> +	struct watchdog_device *wdd;
> +	struct device_node *np;
> +	int ret;
> +
> +	np = of_find_compatible_node(NULL, NULL, "qcom,kpss-wdt");
> +	if (np) {
> +		of_node_put(np);
> +		return -ENODEV;
> +	}
> +
> +	np = of_find_compatible_node(NULL, NULL, "arm,sbsa-gwdt");
> +	if (np) {
> +		of_node_put(np);
> +		return -ENODEV;
> +	}

Please add a comment about how the above two compatibles tie into our logic
(e.g. "Some builds of Gunyah expose a memory-mapped legacy-Qualcomm or Arm
SBSA watchdog instance instead")

Konrad

On 10/6/2025 7:17 PM, Konrad Dybcio wrote:
> On 10/6/25 9:37 AM, Hrishabh Rajput via B4 Relay wrote:
>> From: Hrishabh Rajput<hrishabh.rajput@oss.qualcomm.com>
>>
>> On Qualcomm SoCs running under the Gunyah hypervisor, access to watchdog
>> through MMIO is not available on all platforms. 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 Gunyah is not present or Gunyah emulates MMIO-based watchdog, we
>> expect MMIO watchdog device to be present in the devicetree. If we
>> detect this device node, we don't proceed ahead. Otherwise, we go ahead
>> and invoke GUNYAH_WDT_STATUS SMC to initiate the discovery of the
>> SMC-based watchdog.
>>
>> Add driver to support the SMC-based watchdog provided by the Gunyah
>> Hypervisor. module_exit() is intentionally not implemented as this
>> driver is intended to be a persistent module.
>>
>> Signed-off-by: Hrishabh Rajput<hrishabh.rajput@oss.qualcomm.com>
>> ---
> [...]
>
>> +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,
>> +};
> Can the calls we make in this driver produce all of these errors?
>
> I'm asking only because we want to minimize the footprint

Agreed. Thanks for pointing this out. It is better to not include the 
errors which aren't produced here in this driver. The excluded errors 
can be added later and the whole thing can be moved to gunyah_errno.h 
when other drivers are introduced.

I just checked, in this driver only the following errors can be 
produced: GUNYAH_ERROR_OK, GUNYAH_ERROR_ARG_INVAL and 
GUNYAH_ERROR_UNIMPLEMENTED. I will remove the rest.

Thanks,
Hrishabh

On 06/10/2025 16:37, Hrishabh Rajput via B4 Relay wrote:
> +
> +static int __init gunyah_wdt_init(void)
> +{
> +	struct arm_smccc_res res;
> +	struct watchdog_device *wdd;
> +	struct device_node *np;
> +	int ret;
> +
> +	np = of_find_compatible_node(NULL, NULL, "qcom,kpss-wdt");
> +	if (np) {
> +		of_node_put(np);
> +		return -ENODEV;
> +	}
> +
> +	np = of_find_compatible_node(NULL, NULL, "arm,sbsa-gwdt");
> +	if (np) {
> +		of_node_put(np);
> +		return -ENODEV;
> +	}
> +
> +	ret = gunyah_wdt_call(GUNYAH_WDT_STATUS, 0, 0, &res);
> +	if (ret)
> +		return -ENODEV;

No, your hypervisor driver (which you have) should start the module via
adding platform/aux/something devices. Now you are running this on every
machine, which is clearly wrong...

Best regards,
Krzysztof

On Mon, Oct 06, 2025 at 05:56:42PM +0900, Krzysztof Kozlowski wrote:
> On 06/10/2025 16:37, Hrishabh Rajput via B4 Relay wrote:
> > +
> > +static int __init gunyah_wdt_init(void)
> > +{
> > +	struct arm_smccc_res res;
> > +	struct watchdog_device *wdd;
> > +	struct device_node *np;
> > +	int ret;
> > +
> > +	np = of_find_compatible_node(NULL, NULL, "qcom,kpss-wdt");
> > +	if (np) {
> > +		of_node_put(np);
> > +		return -ENODEV;
> > +	}
> > +
> > +	np = of_find_compatible_node(NULL, NULL, "arm,sbsa-gwdt");
> > +	if (np) {
> > +		of_node_put(np);
> > +		return -ENODEV;
> > +	}
> > +
> > +	ret = gunyah_wdt_call(GUNYAH_WDT_STATUS, 0, 0, &res);
> > +	if (ret)
> > +		return -ENODEV;
> 
> No, your hypervisor driver (which you have) should start the module via
> adding platform/aux/something devices. Now you are running this on every
> machine, which is clearly wrong...
> 

This is a good point. Thanks for bringing it up. We don't have a
hypervisor glue driver (yet!) that can add an aux device. Based on v1
feedback, we would like to be a standalone module that can self discover
gunyah hypercall interface.

Currently this driver depends on ARCH_QCOM || COMPILE_TEST. So,
technically this can be built and loaded on all non-Qualcomm machines.

We can make the STATUS SMCC before looking for the other watchdog
devices and fail early.

Our Gunyah glue driver [1] do make SMCC call to establish that we
are actually a guest under Gunyah. Since our intention here is to
support watchdog on as many as platform as possible, it is better not to
tie this with glue driver and make it a stand alone and self discovery
module.

If this is not an acceptable solution (Please let us know), we can find other 
ways to limit it to only work on Qualcomm machines. For ex: Socinfo
platform device is added from SMEM driver which make it only probed on 
Qualcomm machines. We can look into this. 

Thanks,
Pavan


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

On 06/10/2025 19:03, Pavan Kondeti wrote:
> On Mon, Oct 06, 2025 at 05:56:42PM +0900, Krzysztof Kozlowski wrote:
>> On 06/10/2025 16:37, Hrishabh Rajput via B4 Relay wrote:
>>> +
>>> +static int __init gunyah_wdt_init(void)
>>> +{
>>> +	struct arm_smccc_res res;
>>> +	struct watchdog_device *wdd;
>>> +	struct device_node *np;
>>> +	int ret;
>>> +
>>> +	np = of_find_compatible_node(NULL, NULL, "qcom,kpss-wdt");
>>> +	if (np) {
>>> +		of_node_put(np);
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	np = of_find_compatible_node(NULL, NULL, "arm,sbsa-gwdt");
>>> +	if (np) {
>>> +		of_node_put(np);
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	ret = gunyah_wdt_call(GUNYAH_WDT_STATUS, 0, 0, &res);
>>> +	if (ret)
>>> +		return -ENODEV;
>>
>> No, your hypervisor driver (which you have) should start the module via
>> adding platform/aux/something devices. Now you are running this on every
>> machine, which is clearly wrong...
>>
> 
> This is a good point. Thanks for bringing it up. We don't have a
> hypervisor glue driver (yet!) that can add an aux device. Based on v1
> feedback, we would like to be a standalone module that can self discover
> gunyah hypercall interface.
> 
> Currently this driver depends on ARCH_QCOM || COMPILE_TEST. So,
> technically this can be built and loaded on all non-Qualcomm machines.


Not technically, but practically. We do not make single-platform kernels
anymore, it's not 2010. Entire arm64 is multiarch.

> 
> We can make the STATUS SMCC before looking for the other watchdog
> devices and fail early.
> 
> Our Gunyah glue driver [1] do make SMCC call to establish that we
> are actually a guest under Gunyah. Since our intention here is to
> support watchdog on as many as platform as possible, it is better not to
> tie this with glue driver and make it a stand alone and self discovery
> module.


I think you should have only one driver pinging for Gunyah, so glue
driver or this. Not both. If you add such SMC here, then how do you
determine the platform in the glue driver? Via DT? Then DT supersedes this.

> 
> If this is not an acceptable solution (Please let us know), we can find other 
> ways to limit it to only work on Qualcomm machines. For ex: Socinfo
> platform device is added from SMEM driver which make it only probed on 
> Qualcomm machines. We can look into this. 


To me socinfo feels even better. That way only, really only qcom devices
will execute this SMC.

Best regards,
Krzysztof

On Mon, Oct 06, 2025 at 10:03:59PM +0900, Krzysztof Kozlowski wrote:
> On 06/10/2025 19:03, Pavan Kondeti wrote:
> > On Mon, Oct 06, 2025 at 05:56:42PM +0900, Krzysztof Kozlowski wrote:
> >> On 06/10/2025 16:37, Hrishabh Rajput via B4 Relay wrote:
> >>> +
> >>> +static int __init gunyah_wdt_init(void)
> >>> +{
> >>> +	struct arm_smccc_res res;
> >>> +	struct watchdog_device *wdd;
> >>> +	struct device_node *np;
> >>> +	int ret;
> >>> +
> >>> +	np = of_find_compatible_node(NULL, NULL, "qcom,kpss-wdt");
> >>> +	if (np) {
> >>> +		of_node_put(np);
> >>> +		return -ENODEV;
> >>> +	}
> >>> +
> >>> +	np = of_find_compatible_node(NULL, NULL, "arm,sbsa-gwdt");
> >>> +	if (np) {
> >>> +		of_node_put(np);
> >>> +		return -ENODEV;
> >>> +	}
> >>> +
> >>> +	ret = gunyah_wdt_call(GUNYAH_WDT_STATUS, 0, 0, &res);
> >>> +	if (ret)
> >>> +		return -ENODEV;
> >>
> >> No, your hypervisor driver (which you have) should start the module via
> >> adding platform/aux/something devices. Now you are running this on every
> >> machine, which is clearly wrong...
> >>
> > 
> > This is a good point. Thanks for bringing it up. We don't have a
> > hypervisor glue driver (yet!) that can add an aux device. Based on v1
> > feedback, we would like to be a standalone module that can self discover
> > gunyah hypercall interface.
> > 
> > Currently this driver depends on ARCH_QCOM || COMPILE_TEST. So,
> > technically this can be built and loaded on all non-Qualcomm machines.
> 
> 
> Not technically, but practically. We do not make single-platform kernels
> anymore, it's not 2010. Entire arm64 is multiarch.

Thanks, I understand that we build single kernel image that works across
machines. However, I wonder do all modules built say from
arch/arm64/configs/defconfig gets loaded? Usually, the modules
corresponding to drivers for which devices are registered (modalias
based) gets loaded, correct? In this case, we don't have a device, so
there may be an explicit rule to load this module. I totally get your
point on why it would be preferred to make this module active only on
QCOM platform.

> 
> > 
> > We can make the STATUS SMCC before looking for the other watchdog
> > devices and fail early.
> > 
> > Our Gunyah glue driver [1] do make SMCC call to establish that we
> > are actually a guest under Gunyah. Since our intention here is to
> > support watchdog on as many as platform as possible, it is better not to
> > tie this with glue driver and make it a stand alone and self discovery
> > module.
> 
> 
> I think you should have only one driver pinging for Gunyah, so glue
> driver or this. Not both. If you add such SMC here, then how do you
> determine the platform in the glue driver? Via DT? Then DT supersedes this.

The SMCC that this module would be using is specific to Gunyah watchdog
interface. So there is no real dependency w/ Gunyah glue driver. I
understand your point that there should not be two drivers probing the
watchdog.

> 
> > 
> > If this is not an acceptable solution (Please let us know), we can find other 
> > ways to limit it to only work on Qualcomm machines. For ex: Socinfo
> > platform device is added from SMEM driver which make it only probed on 
> > Qualcomm machines. We can look into this. 
> 
> 
> To me socinfo feels even better. That way only, really only qcom devices
> will execute this SMC.
> 

Ok, we will look into this.

Thanks,
Pavan