[PATCH v5 0/2] Add support for Gunyah Watchdog

[PATCH v5 0/2] Add support for Gunyah Watchdog

[Hrishabh Rajput via B4 Relay]

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

Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
---
Changes in v5:
- Move the gunyah_wdt device registration from the SMEM driver to the
  SCM driver. Add additional logic to check if we're running under the
  Gunyah Hypervisor.
- Implement .remove() for gunyah_wdt driver to make it not persistent.
- Link to v4: https://lore.kernel.org/r/20251031-gunyah_watchdog-v4-0-7abb1ee11315@oss.qualcomm.com

Changes in v4:
- Move the contents of gunyah_wdt_init() to qcom_smem_probe() to make
  sure we're registering the watchdog only on the Qualcomm devices.
- Link to v3: https://lore.kernel.org/r/20251028-gunyah_watchdog-v3-1-e6d1ea438b1d@oss.qualcomm.com

Changes in v3:
- Move back to platform driver model. In module init, determine if we're
  running on a Qualcomm device and there is no supported memory-mapped
  watchdog present. Then proceed to register platform device and driver
  for SMC-based Gunyah watchdog.
- To determine if we're running on a Qualcomm device we're checking the
  presence of "qcom,smem" compatible devicetree node. As an alternative,
  we also tried using socinfo for the same purpose. When both
  gunyah_wdt and socinfo drivers were made built-in, it couldn't be
  ensured that the socinfo driver probed successfully before gunyah_wdt
  init was called. Hence, we resorted to the devicetree node approach.
- Limit the errors listed in gunyah_error to the ones that can be
  produced by the driver.
- Link to v2: https://lore.kernel.org/r/20251006-gunyah_watchdog-v2-1-b99d41d45450@oss.qualcomm.com

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

---
Hrishabh Rajput (2):
      firmware: qcom: scm: Register gunyah watchdog device
      watchdog: Add driver for Gunyah Watchdog

 MAINTAINERS                      |   1 +
 drivers/firmware/qcom/qcom_scm.c |  51 ++++++++
 drivers/watchdog/Kconfig         |  13 ++
 drivers/watchdog/Makefile        |   1 +
 drivers/watchdog/gunyah_wdt.c    | 260 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 326 insertions(+)
---
base-commit: 6146a0f1dfae5d37442a9ddcba012add260bceb0
change-id: 20250903-gunyah_watchdog-2d2649438e29

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

[PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Hrishabh Rajput via B4 Relay]

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

To restrict Gunyah watchdog initialization to Qualcomm platforms running
under the Gunyah Hypervisor, register the watchdog device in the QCOM
SCM driver.

When Gunyah is not present or Gunyah emulates MMIO-based watchdog, we
expect Qualcomm watchdog or ARM SBSA watchdog device to be present in
the devicetree. First, we make sure we're running under the Gunyah
Hypervisor. Then we move to check if any of the above mentioned
watchdog device nodes are present, if not then we proceed to register
the SMC-based Gunyah watchdog device.

Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 51 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index e777b7cb9b12..71b79c0229da 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -2182,6 +2182,54 @@ int qcom_scm_qtee_callback_response(phys_addr_t buf, size_t buf_size,
 }
 EXPORT_SYMBOL(qcom_scm_qtee_callback_response);
 
+static void qcom_scm_gunyah_wdt_free(void *data)
+{
+	struct platform_device *gunyah_wdt_dev = data;
+
+	platform_device_unregister(gunyah_wdt_dev);
+}
+
+static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
+{
+	struct platform_device *gunyah_wdt_dev;
+	struct device_node *np;
+	bool of_wdt_available;
+	int i;
+	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
+				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
+	static const char * const of_wdt_compatible[] = {
+		"qcom,kpss-wdt",
+		"arm,sbsa-gwdt",
+	};
+
+	/* Bail out if we are not running under Gunyah */
+	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
+		return;
+
+	/*
+	 * Gunyah emulates either of Qualcomm watchdog or ARM SBSA watchdog on
+	 * newer platforms. Bail out if we find them in the devicetree.
+	 */
+	for (i = 0; i < ARRAY_SIZE(of_wdt_compatible); i++) {
+		np = of_find_compatible_node(NULL, NULL, of_wdt_compatible[i]);
+		of_wdt_available = of_device_is_available(np);
+		of_node_put(np);
+		if (of_wdt_available)
+			return;
+	}
+
+	gunyah_wdt_dev = platform_device_register_simple("gunyah-wdt", -1,
+							 NULL, 0);
+	if (IS_ERR(gunyah_wdt_dev)) {
+		dev_err(scm->dev, "Failed to register Gunyah watchdog device: %ld\n",
+			PTR_ERR(gunyah_wdt_dev));
+		return;
+	}
+
+	devm_add_action_or_reset(scm->dev, qcom_scm_gunyah_wdt_free,
+				 gunyah_wdt_dev);
+}
+
 static void qcom_scm_qtee_free(void *data)
 {
 	struct platform_device *qtee_dev = data;
@@ -2448,6 +2496,9 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	/* Initialize the QTEE object interface. */
 	qcom_scm_qtee_init(scm);
 
+	/* Initialize the Gunyah watchdog platform device. */
+	qcom_scm_gunyah_wdt_init(scm);
+
 	return 0;
 }
 

-- 
2.43.0

[PATCH v5 2/2] watchdog: Add driver for Gunyah Watchdog

[Hrishabh Rajput via B4 Relay]

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.

Add driver to support the SMC-based watchdog provided by the Gunyah
Hypervisor. Device registration is done in the QCOM SCM driver after
checks to restrict the watchdog initialization to Qualcomm devices
running under Gunyah.

Gunyah watchdog is not a hardware but an SMC-based vendor-specific
hypervisor interface provided by the Gunyah hypervisor. The design
involving QCOM SCM driver for registering the platform device has been
devised to avoid adding non-hardware nodes to devicetree.

Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
---
 MAINTAINERS                   |   1 +
 drivers/watchdog/Kconfig      |  13 +++
 drivers/watchdog/Makefile     |   1 +
 drivers/watchdog/gunyah_wdt.c | 260 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 275 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 46bd8e033042..b93237883f18 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3215,6 +3215,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 05008d937e40..bc6db9a1c116 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -2354,4 +2354,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 gunyah_wdt.
+
 endif # WATCHDOG
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index b680e4d3c1bc..1215efb7816d 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..addfd1733ad1
--- /dev/null
+++ b/drivers/watchdog/gunyah_wdt.c
@@ -0,0 +1,260 @@
+// 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/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.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)
+
+enum gunyah_error {
+	GUNYAH_ERROR_OK				= 0,
+	GUNYAH_ERROR_UNIMPLEMENTED		= -1,
+	GUNYAH_ERROR_ARG_INVAL			= 1,
+};
+
+/**
+ * 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_UNIMPLEMENTED:
+		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)
+{
+	unsigned int timeout_ms;
+	struct device *dev = wdd->parent;
+	int ret;
+
+	ret = gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_DISABLE, 0);
+	if (ret && watchdog_active(wdd)) {
+		dev_err(dev, "%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);
+	if (ret) {
+		dev_err(dev, "%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);
+	if (ret)
+		dev_err(dev, "%s: Failed to start gunyah wdt %d\n", __func__, ret);
+
+	return ret;
+}
+
+static int gunyah_wdt_stop(struct watchdog_device *wdd)
+{
+	return gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_DISABLE, 0);
+}
+
+static int gunyah_wdt_ping(struct watchdog_device *wdd)
+{
+	return gunyah_wdt_call(GUNYAH_WDT_PING, 0, 0);
+}
+
+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_get_time_since_last_ping(void)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_1_1_smc(GUNYAH_WDT_STATUS, 0, 0, &res);
+	if (res.a0)
+		return gunyah_error_remap(res.a0);
+
+	return res.a2 / 1000;
+}
+
+static unsigned int gunyah_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+	int seconds_since_last_ping;
+
+	seconds_since_last_ping = gunyah_wdt_get_time_since_last_ping();
+	if (seconds_since_last_ping < 0 ||
+	    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)
+{
+	/* Set timeout to 1ms and send a ping */
+	gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_DISABLE, 0);
+	gunyah_wdt_call(GUNYAH_WDT_SET_TIME, 1, 1);
+	gunyah_wdt_call(GUNYAH_WDT_CONTROL, WDT_CTRL_ENABLE, 0);
+	gunyah_wdt_call(GUNYAH_WDT_PING, 0, 0);
+
+	/* Wait to make sure reset occurs */
+	mdelay(100);
+
+	return 0;
+}
+
+static const struct watchdog_info gunyah_wdt_info = {
+	.identity = "Gunyah Watchdog",
+	.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_probe(struct platform_device *pdev)
+{
+	struct watchdog_device *wdd;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = gunyah_wdt_call(GUNYAH_WDT_STATUS, 0, 0);
+	if (ret) {
+		dev_err_probe(dev, ret, "status check failed\n");
+		return ret;
+	}
+
+	wdd = devm_kzalloc(dev, sizeof(*wdd), GFP_KERNEL);
+	if (!wdd)
+		return -ENOMEM;
+
+	wdd->info = &gunyah_wdt_info;
+	wdd->ops = &gunyah_wdt_ops;
+	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.
+	 */
+	wdd->max_timeout = 32; /* seconds */
+	wdd->min_timeout = 1; /* seconds */
+	wdd->timeout = wdd->max_timeout;
+
+	gunyah_wdt_stop(wdd);
+	platform_set_drvdata(pdev, wdd);
+	watchdog_set_restart_priority(wdd, 0);
+
+	return devm_watchdog_register_device(dev, wdd);
+}
+
+static void gunyah_wdt_remove(struct platform_device *pdev)
+{
+	struct watchdog_device *wdd = platform_get_drvdata(pdev);
+
+	gunyah_wdt_stop(wdd);
+}
+
+static int gunyah_wdt_suspend(struct device *dev)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	if (watchdog_active(wdd))
+		gunyah_wdt_stop(wdd);
+
+	return 0;
+}
+
+static int gunyah_wdt_resume(struct device *dev)
+{
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
+
+	if (watchdog_active(wdd))
+		gunyah_wdt_start(wdd);
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(gunyah_wdt_pm_ops, gunyah_wdt_suspend, gunyah_wdt_resume);
+
+/*
+ * Gunyah watchdog is a vendor-specific hypervisor interface provided by the
+ * Gunyah hypervisor. Using QCOM SCM driver to detect Gunyah watchdog SMCCC
+ * hypervisor service and register platform device when the service is available
+ * allows this driver to operate independently of the devicetree and avoids
+ * adding the non-hardware nodes to the devicetree.
+ */
+static const struct platform_device_id gunyah_wdt_id[] = {
+	{ .name = "gunyah-wdt" },
+	{}
+};
+MODULE_DEVICE_TABLE(platform, gunyah_wdt_id);
+
+static struct platform_driver gunyah_wdt_driver = {
+	.driver = {
+		.name = "gunyah-wdt",
+		.pm = pm_sleep_ptr(&gunyah_wdt_pm_ops),
+	},
+	.id_table = gunyah_wdt_id,
+	.probe = gunyah_wdt_probe,
+	.remove = gunyah_wdt_remove,
+};
+
+module_platform_driver(gunyah_wdt_driver);
+
+MODULE_DESCRIPTION("Gunyah Watchdog Driver");
+MODULE_LICENSE("GPL");

-- 
2.43.0

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[kernel test robot]

Hi Hrishabh,

kernel test robot noticed the following build errors:

[auto build test ERROR on 6146a0f1dfae5d37442a9ddcba012add260bceb0]

url:    https://github.com/intel-lab-lkp/linux/commits/Hrishabh-Rajput-via-B4-Relay/firmware-qcom-scm-Register-gunyah-watchdog-device/20251108-015559
base:   6146a0f1dfae5d37442a9ddcba012add260bceb0
patch link:    https://lore.kernel.org/r/20251107-gunyah_watchdog-v5-1-4c6e3fb6eb17%40oss.qualcomm.com
patch subject: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device
config: powerpc-randconfig-001-20251108 (https://download.01.org/0day-ci/archive/20251108/202511081706.0sVDjTBC-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251108/202511081706.0sVDjTBC-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511081706.0sVDjTBC-lkp@intel.com/

All errors (new ones prefixed by >>):

   powerpc-linux-ld: powerpc-linux-ld: DWARF error: could not find abbrev number 44
   drivers/firmware/qcom/qcom_scm.o: in function `qcom_scm_probe':
>> qcom_scm.c:(.text+0x349c): undefined reference to `arm_smccc_hypervisor_has_uuid'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[kernel test robot]

Hi Hrishabh,

kernel test robot noticed the following build errors:

[auto build test ERROR on 6146a0f1dfae5d37442a9ddcba012add260bceb0]

url:    https://github.com/intel-lab-lkp/linux/commits/Hrishabh-Rajput-via-B4-Relay/firmware-qcom-scm-Register-gunyah-watchdog-device/20251108-015559
base:   6146a0f1dfae5d37442a9ddcba012add260bceb0
patch link:    https://lore.kernel.org/r/20251107-gunyah_watchdog-v5-1-4c6e3fb6eb17%40oss.qualcomm.com
patch subject: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device
config: riscv-randconfig-r063-20251108 (https://download.01.org/0day-ci/archive/20251108/202511082023.F71T0M1w-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251108/202511082023.F71T0M1w-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511082023.F71T0M1w-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "arm_smccc_hypervisor_has_uuid" [drivers/firmware/qcom/qcom-scm.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Dmitry Baryshkov]

On Fri, Nov 07, 2025 at 05:53:08PM +0000, Hrishabh Rajput via B4 Relay wrote:
> From: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
> 
> To restrict Gunyah watchdog initialization to Qualcomm platforms running
> under the Gunyah Hypervisor, register the watchdog device in the QCOM
> SCM driver.
> 
> When Gunyah is not present or Gunyah emulates MMIO-based watchdog, we
> expect Qualcomm watchdog or ARM SBSA watchdog device to be present in
> the devicetree. First, we make sure we're running under the Gunyah
> Hypervisor. Then we move to check if any of the above mentioned
> watchdog device nodes are present, if not then we proceed to register
> the SMC-based Gunyah watchdog device.
> 
> Signed-off-by: Hrishabh Rajput <hrishabh.rajput@oss.qualcomm.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c | 51 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index e777b7cb9b12..71b79c0229da 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -2182,6 +2182,54 @@ int qcom_scm_qtee_callback_response(phys_addr_t buf, size_t buf_size,
>  }
>  EXPORT_SYMBOL(qcom_scm_qtee_callback_response);
>  
> +static void qcom_scm_gunyah_wdt_free(void *data)
> +{
> +	struct platform_device *gunyah_wdt_dev = data;
> +
> +	platform_device_unregister(gunyah_wdt_dev);
> +}
> +
> +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> +{
> +	struct platform_device *gunyah_wdt_dev;
> +	struct device_node *np;
> +	bool of_wdt_available;
> +	int i;
> +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,

static const?

> +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
> +	static const char * const of_wdt_compatible[] = {
> +		"qcom,kpss-wdt",
> +		"arm,sbsa-gwdt",
> +	};
> +
> +	/* Bail out if we are not running under Gunyah */
> +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> +		return;

This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'

> +

-- 
With best wishes
Dmitry

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Pavan Kondeti]

On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
> > +static void qcom_scm_gunyah_wdt_free(void *data)
> > +{
> > +	struct platform_device *gunyah_wdt_dev = data;
> > +
> > +	platform_device_unregister(gunyah_wdt_dev);
> > +}
> > +
> > +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> > +{
> > +	struct platform_device *gunyah_wdt_dev;
> > +	struct device_node *np;
> > +	bool of_wdt_available;
> > +	int i;
> > +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
> 
> static const?
> 
> > +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
> > +	static const char * const of_wdt_compatible[] = {
> > +		"qcom,kpss-wdt",
> > +		"arm,sbsa-gwdt",
> > +	};
> > +
> > +	/* Bail out if we are not running under Gunyah */
> > +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> > +		return;
> 
> This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
> 

Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.

Thanks,
Pavan

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Pavan Kondeti]

On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
> On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
> > > +static void qcom_scm_gunyah_wdt_free(void *data)
> > > +{
> > > +	struct platform_device *gunyah_wdt_dev = data;
> > > +
> > > +	platform_device_unregister(gunyah_wdt_dev);
> > > +}
> > > +
> > > +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> > > +{
> > > +	struct platform_device *gunyah_wdt_dev;
> > > +	struct device_node *np;
> > > +	bool of_wdt_available;
> > > +	int i;
> > > +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
> > 
> > static const?
> > 
> > > +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
> > > +	static const char * const of_wdt_compatible[] = {
> > > +		"qcom,kpss-wdt",
> > > +		"arm,sbsa-gwdt",
> > > +	};
> > > +
> > > +	/* Bail out if we are not running under Gunyah */
> > > +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> > > +		return;
> > 
> > This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
> > 
> 
> Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
> 

Dmitry / Bjorn,

We are debating on this internally on how to resolve this dependency

- QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
  QCOM_SCM compilation than what it is today.

- Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver 

- Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
  for any of the functions defined in drivers/firmware/smccc/smccc.c

We are trending towards the first option above. Please let us know if
you think otherwise.

Thanks,
Pavan

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Dmitry Baryshkov]

On Tue, Nov 11, 2025 at 10:51:43AM +0530, Pavan Kondeti wrote:
> On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
> > On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
> > > > +static void qcom_scm_gunyah_wdt_free(void *data)
> > > > +{
> > > > +	struct platform_device *gunyah_wdt_dev = data;
> > > > +
> > > > +	platform_device_unregister(gunyah_wdt_dev);
> > > > +}
> > > > +
> > > > +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> > > > +{
> > > > +	struct platform_device *gunyah_wdt_dev;
> > > > +	struct device_node *np;
> > > > +	bool of_wdt_available;
> > > > +	int i;
> > > > +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
> > > 
> > > static const?
> > > 
> > > > +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
> > > > +	static const char * const of_wdt_compatible[] = {
> > > > +		"qcom,kpss-wdt",
> > > > +		"arm,sbsa-gwdt",
> > > > +	};
> > > > +
> > > > +	/* Bail out if we are not running under Gunyah */
> > > > +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> > > > +		return;
> > > 
> > > This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
> > > 
> > 
> > Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
> > 
> 
> Dmitry / Bjorn,
> 
> We are debating on this internally on how to resolve this dependency
> 
> - QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
>   QCOM_SCM compilation than what it is today.
> 
> - Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver 
> 
> - Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
>   for any of the functions defined in drivers/firmware/smccc/smccc.c
> 
> We are trending towards the first option above. Please let us know if
> you think otherwise.

The same as before: 'select HAVE_ARM_SMCCC_DISCOVERY'.

-- 
With best wishes
Dmitry

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Krzysztof Kozlowski]

On 11/11/2025 11:34, Dmitry Baryshkov wrote:
> On Tue, Nov 11, 2025 at 10:51:43AM +0530, Pavan Kondeti wrote:
>> On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
>>> On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
>>>>> +static void qcom_scm_gunyah_wdt_free(void *data)
>>>>> +{
>>>>> +	struct platform_device *gunyah_wdt_dev = data;
>>>>> +
>>>>> +	platform_device_unregister(gunyah_wdt_dev);
>>>>> +}
>>>>> +
>>>>> +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
>>>>> +{
>>>>> +	struct platform_device *gunyah_wdt_dev;
>>>>> +	struct device_node *np;
>>>>> +	bool of_wdt_available;
>>>>> +	int i;
>>>>> +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
>>>>
>>>> static const?
>>>>
>>>>> +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
>>>>> +	static const char * const of_wdt_compatible[] = {
>>>>> +		"qcom,kpss-wdt",
>>>>> +		"arm,sbsa-gwdt",
>>>>> +	};
>>>>> +
>>>>> +	/* Bail out if we are not running under Gunyah */
>>>>> +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
>>>>> +		return;
>>>>
>>>> This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
>>>>
>>>
>>> Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
>>>
>>
>> Dmitry / Bjorn,
>>
>> We are debating on this internally on how to resolve this dependency
>>
>> - QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
>>   QCOM_SCM compilation than what it is today.
>>
>> - Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver 
>>
>> - Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
>>   for any of the functions defined in drivers/firmware/smccc/smccc.c
>>
>> We are trending towards the first option above. Please let us know if
>> you think otherwise.
> 
> The same as before: 'select HAVE_ARM_SMCCC_DISCOVERY'.

HAVE_ARM_SMCCC_DISCOVERY has a dependency which is not always selected
(e.g. ARM32), thus selecting it might lead to warnings of unmet
dependencies. Whichever they choose here, they need to be sure to
actually compile test it, because existing patch lacks that and reports
are proving lack of building.

Best regards,
Krzysztof

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Dmitry Baryshkov]

On Tue, Nov 11, 2025 at 11:41:51AM +0100, Krzysztof Kozlowski wrote:
> On 11/11/2025 11:34, Dmitry Baryshkov wrote:
> > On Tue, Nov 11, 2025 at 10:51:43AM +0530, Pavan Kondeti wrote:
> >> On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
> >>> On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
> >>>>> +static void qcom_scm_gunyah_wdt_free(void *data)
> >>>>> +{
> >>>>> +	struct platform_device *gunyah_wdt_dev = data;
> >>>>> +
> >>>>> +	platform_device_unregister(gunyah_wdt_dev);
> >>>>> +}
> >>>>> +
> >>>>> +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> >>>>> +{
> >>>>> +	struct platform_device *gunyah_wdt_dev;
> >>>>> +	struct device_node *np;
> >>>>> +	bool of_wdt_available;
> >>>>> +	int i;
> >>>>> +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
> >>>>
> >>>> static const?
> >>>>
> >>>>> +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
> >>>>> +	static const char * const of_wdt_compatible[] = {
> >>>>> +		"qcom,kpss-wdt",
> >>>>> +		"arm,sbsa-gwdt",
> >>>>> +	};
> >>>>> +
> >>>>> +	/* Bail out if we are not running under Gunyah */
> >>>>> +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> >>>>> +		return;
> >>>>
> >>>> This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
> >>>>
> >>>
> >>> Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
> >>>
> >>
> >> Dmitry / Bjorn,
> >>
> >> We are debating on this internally on how to resolve this dependency
> >>
> >> - QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
> >>   QCOM_SCM compilation than what it is today.
> >>
> >> - Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver 
> >>
> >> - Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
> >>   for any of the functions defined in drivers/firmware/smccc/smccc.c
> >>
> >> We are trending towards the first option above. Please let us know if
> >> you think otherwise.
> > 
> > The same as before: 'select HAVE_ARM_SMCCC_DISCOVERY'.
> 
> HAVE_ARM_SMCCC_DISCOVERY has a dependency which is not always selected
> (e.g. ARM32), thus selecting it might lead to warnings of unmet
> dependencies.

Then `if (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY))` might be a good
option here (and depend on GICv3 selecting it).

> Whichever they choose here, they need to be sure to
> actually compile test it, because existing patch lacks that and reports
> are proving lack of building.
> 
> Best regards,
> Krzysztof

-- 
With best wishes
Dmitry

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Hrishabh Rajput]

On 11/11/2025 5:52 PM, Dmitry Baryshkov wrote:
> On Tue, Nov 11, 2025 at 11:41:51AM +0100, Krzysztof Kozlowski wrote:
>> On 11/11/2025 11:34, Dmitry Baryshkov wrote:
>>> On Tue, Nov 11, 2025 at 10:51:43AM +0530, Pavan Kondeti wrote:
>>>> On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
>>>>> On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
>>>>>>> +static void qcom_scm_gunyah_wdt_free(void *data)
>>>>>>> +{
>>>>>>> +	struct platform_device *gunyah_wdt_dev = data;
>>>>>>> +
>>>>>>> +	platform_device_unregister(gunyah_wdt_dev);
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
>>>>>>> +{
>>>>>>> +	struct platform_device *gunyah_wdt_dev;
>>>>>>> +	struct device_node *np;
>>>>>>> +	bool of_wdt_available;
>>>>>>> +	int i;
>>>>>>> +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
>>>>>> static const?
>>>>>>
>>>>>>> +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
>>>>>>> +	static const char * const of_wdt_compatible[] = {
>>>>>>> +		"qcom,kpss-wdt",
>>>>>>> +		"arm,sbsa-gwdt",
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	/* Bail out if we are not running under Gunyah */
>>>>>>> +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
>>>>>>> +		return;
>>>>>> This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
>>>>>>
>>>>> Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
>>>>>
>>>> Dmitry / Bjorn,
>>>>
>>>> We are debating on this internally on how to resolve this dependency
>>>>
>>>> - QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
>>>>    QCOM_SCM compilation than what it is today.
>>>>
>>>> - Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver
>>>>
>>>> - Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
>>>>    for any of the functions defined in drivers/firmware/smccc/smccc.c
>>>>
>>>> We are trending towards the first option above. Please let us know if
>>>> you think otherwise.
>>> The same as before: 'select HAVE_ARM_SMCCC_DISCOVERY'.
>> HAVE_ARM_SMCCC_DISCOVERY has a dependency which is not always selected
>> (e.g. ARM32), thus selecting it might lead to warnings of unmet
>> dependencies.
> Then `if (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY))` might be a good
> option here (and depend on GICv3 selecting it).


Thanks a lot Dmitry, wemade the change below and compile tested on 
various architectures (ARM64, ARM32, x86, PowerPC, RISC-V and MIPS) and 
it was success.

We will include it in our next patch version, if there are no further 
concerns.

}; /* Bail out if we are not running under Gunyah */ - if 
(!arm_smccc_hypervisor_has_uuid(&gunyah_uuid)) + if 
(!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) || + 
!arm_smccc_hypervisor_has_uuid(&gunyah_uuid)) return; /*

Thanks,

Hrishabh

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Dmitry Baryshkov]

On Tue, Nov 11, 2025 at 07:30:59PM +0530, Hrishabh Rajput wrote:
> 
> On 11/11/2025 5:52 PM, Dmitry Baryshkov wrote:
> > On Tue, Nov 11, 2025 at 11:41:51AM +0100, Krzysztof Kozlowski wrote:
> > > On 11/11/2025 11:34, Dmitry Baryshkov wrote:
> > > > On Tue, Nov 11, 2025 at 10:51:43AM +0530, Pavan Kondeti wrote:
> > > > > On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
> > > > > > On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
> > > > > > > > +static void qcom_scm_gunyah_wdt_free(void *data)
> > > > > > > > +{
> > > > > > > > +	struct platform_device *gunyah_wdt_dev = data;
> > > > > > > > +
> > > > > > > > +	platform_device_unregister(gunyah_wdt_dev);
> > > > > > > > +}
> > > > > > > > +
> > > > > > > > +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> > > > > > > > +{
> > > > > > > > +	struct platform_device *gunyah_wdt_dev;
> > > > > > > > +	struct device_node *np;
> > > > > > > > +	bool of_wdt_available;
> > > > > > > > +	int i;
> > > > > > > > +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
> > > > > > > static const?
> > > > > > > 
> > > > > > > > +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
> > > > > > > > +	static const char * const of_wdt_compatible[] = {
> > > > > > > > +		"qcom,kpss-wdt",
> > > > > > > > +		"arm,sbsa-gwdt",
> > > > > > > > +	};
> > > > > > > > +
> > > > > > > > +	/* Bail out if we are not running under Gunyah */
> > > > > > > > +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> > > > > > > > +		return;
> > > > > > > This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
> > > > > > > 
> > > > > > Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
> > > > > > 
> > > > > Dmitry / Bjorn,
> > > > > 
> > > > > We are debating on this internally on how to resolve this dependency
> > > > > 
> > > > > - QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
> > > > >    QCOM_SCM compilation than what it is today.
> > > > > 
> > > > > - Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver
> > > > > 
> > > > > - Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
> > > > >    for any of the functions defined in drivers/firmware/smccc/smccc.c
> > > > > 
> > > > > We are trending towards the first option above. Please let us know if
> > > > > you think otherwise.
> > > > The same as before: 'select HAVE_ARM_SMCCC_DISCOVERY'.
> > > HAVE_ARM_SMCCC_DISCOVERY has a dependency which is not always selected
> > > (e.g. ARM32), thus selecting it might lead to warnings of unmet
> > > dependencies.
> > Then `if (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY))` might be a good
> > option here (and depend on GICv3 selecting it).
> 
> 
> Thanks a lot Dmitry, wemade the change below and compile tested on various
> architectures (ARM64, ARM32, x86, PowerPC, RISC-V and MIPS) and it was
> success.
> 
> We will include it in our next patch version, if there are no further
> concerns.
> 
> }; /* Bail out if we are not running under Gunyah */ - if
> (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid)) + if
> (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) || +
> !arm_smccc_hypervisor_has_uuid(&gunyah_uuid)) return; /*

Unreadable. Don't you read what you are sending?

-- 
With best wishes
Dmitry

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Bjorn Andersson]

On Tue, Nov 11, 2025 at 10:51:43AM +0530, Pavan Kondeti wrote:
> On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
> > On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
> > > > +static void qcom_scm_gunyah_wdt_free(void *data)
> > > > +{
> > > > +	struct platform_device *gunyah_wdt_dev = data;
> > > > +
> > > > +	platform_device_unregister(gunyah_wdt_dev);
> > > > +}
> > > > +
> > > > +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> > > > +{
> > > > +	struct platform_device *gunyah_wdt_dev;
> > > > +	struct device_node *np;
> > > > +	bool of_wdt_available;
> > > > +	int i;
> > > > +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
> > > 
> > > static const?
> > > 
> > > > +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
> > > > +	static const char * const of_wdt_compatible[] = {
> > > > +		"qcom,kpss-wdt",
> > > > +		"arm,sbsa-gwdt",
> > > > +	};
> > > > +
> > > > +	/* Bail out if we are not running under Gunyah */
> > > > +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> > > > +		return;
> > > 
> > > This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
> > > 
> > 
> > Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
> > 
> 
> Dmitry / Bjorn,
> 
> We are debating on this internally on how to resolve this dependency
> 
> - QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
>   QCOM_SCM compilation than what it is today.

What does that imply? What is the actual impact? (Do I need to go read
the dependency tree myself?)

> 
> - Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver 
> 
> - Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
>   for any of the functions defined in drivers/firmware/smccc/smccc.c
> 
> We are trending towards the first option above. Please let us know if
> you think otherwise.

What is this trend driven by? Is it coin toss or is there a reason? My
gut feeling is trending towards one of the latter two options...

But you're effectively asking us to go research these three options,
determine the pros/cons and then tell you what we think, at which point I
presume you will tell us what you think about each option.

It would be better if you made a suggestion and told us why you think
this is the best choice - then we can either agree with your reasoning,
or choose to ask more questions or do some research.

Regards,
Bjorn

> Thanks,
> Pavan

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Pavan Kondeti]

On Tue, Nov 11, 2025 at 10:38:27AM -0600, Bjorn Andersson wrote:
> On Tue, Nov 11, 2025 at 10:51:43AM +0530, Pavan Kondeti wrote:
> > On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
> > > On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
> > > > > +static void qcom_scm_gunyah_wdt_free(void *data)
> > > > > +{
> > > > > +	struct platform_device *gunyah_wdt_dev = data;
> > > > > +
> > > > > +	platform_device_unregister(gunyah_wdt_dev);
> > > > > +}
> > > > > +
> > > > > +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> > > > > +{
> > > > > +	struct platform_device *gunyah_wdt_dev;
> > > > > +	struct device_node *np;
> > > > > +	bool of_wdt_available;
> > > > > +	int i;
> > > > > +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
> > > > 
> > > > static const?
> > > > 
> > > > > +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
> > > > > +	static const char * const of_wdt_compatible[] = {
> > > > > +		"qcom,kpss-wdt",
> > > > > +		"arm,sbsa-gwdt",
> > > > > +	};
> > > > > +
> > > > > +	/* Bail out if we are not running under Gunyah */
> > > > > +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> > > > > +		return;
> > > > 
> > > > This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
> > > > 
> > > 
> > > Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
> > > 
> > 
> > Dmitry / Bjorn,
> > 
> > We are debating on this internally on how to resolve this dependency
> > 
> > - QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
> >   QCOM_SCM compilation than what it is today.
> 
> What does that imply? What is the actual impact? (Do I need to go read
> the dependency tree myself?)

Actually, I misunderstood how QCOM_SCM driver is enabled. It is being
selected by other drivers which needs functionality provided by QCOM_SCM
driver. So adding HAVE_ARM_SMCCC_DISCOVERY dependency does not make much
sense. Sorry, I should have done my homework properly. I was carried
away with `select` vs `depends on` approach. 

> 
> > 
> > - Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver 
> > 
> > - Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
> >   for any of the functions defined in drivers/firmware/smccc/smccc.c
> > 
> > We are trending towards the first option above. Please let us know if
> > you think otherwise.
> 
> What is this trend driven by? Is it coin toss or is there a reason? My
> gut feeling is trending towards one of the latter two options...

Thanks, we are going with #ifdefry around the new code that is added by
this patch.

> 
> But you're effectively asking us to go research these three options,
> determine the pros/cons and then tell you what we think, at which point I
> presume you will tell us what you think about each option.
> 
> It would be better if you made a suggestion and told us why you think
> this is the best choice - then we can either agree with your reasoning,
> or choose to ask more questions or do some research.
> 

Understood. I will keep this in mind while presenting choices from now
onwards.

Thanks,
Pavan

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Dmitry Baryshkov]

On Tue, Nov 11, 2025 at 10:31:10PM +0530, Pavan Kondeti wrote:
> On Tue, Nov 11, 2025 at 10:38:27AM -0600, Bjorn Andersson wrote:
> > On Tue, Nov 11, 2025 at 10:51:43AM +0530, Pavan Kondeti wrote:
> > > On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
> > > > On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
> > > > > > +static void qcom_scm_gunyah_wdt_free(void *data)
> > > > > > +{
> > > > > > +	struct platform_device *gunyah_wdt_dev = data;
> > > > > > +
> > > > > > +	platform_device_unregister(gunyah_wdt_dev);
> > > > > > +}
> > > > > > +
> > > > > > +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> > > > > > +{
> > > > > > +	struct platform_device *gunyah_wdt_dev;
> > > > > > +	struct device_node *np;
> > > > > > +	bool of_wdt_available;
> > > > > > +	int i;
> > > > > > +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
> > > > > 
> > > > > static const?
> > > > > 
> > > > > > +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
> > > > > > +	static const char * const of_wdt_compatible[] = {
> > > > > > +		"qcom,kpss-wdt",
> > > > > > +		"arm,sbsa-gwdt",
> > > > > > +	};
> > > > > > +
> > > > > > +	/* Bail out if we are not running under Gunyah */
> > > > > > +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> > > > > > +		return;
> > > > > 
> > > > > This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
> > > > > 
> > > > 
> > > > Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
> > > > 
> > > 
> > > Dmitry / Bjorn,
> > > 
> > > We are debating on this internally on how to resolve this dependency
> > > 
> > > - QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
> > >   QCOM_SCM compilation than what it is today.
> > 
> > What does that imply? What is the actual impact? (Do I need to go read
> > the dependency tree myself?)
> 
> Actually, I misunderstood how QCOM_SCM driver is enabled. It is being
> selected by other drivers which needs functionality provided by QCOM_SCM
> driver. So adding HAVE_ARM_SMCCC_DISCOVERY dependency does not make much
> sense. Sorry, I should have done my homework properly. I was carried
> away with `select` vs `depends on` approach. 
> 
> > 
> > > 
> > > - Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver 
> > > 
> > > - Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
> > >   for any of the functions defined in drivers/firmware/smccc/smccc.c
> > > 
> > > We are trending towards the first option above. Please let us know if
> > > you think otherwise.
> > 
> > What is this trend driven by? Is it coin toss or is there a reason? My
> > gut feeling is trending towards one of the latter two options...
> 
> Thanks, we are going with #ifdefry around the new code that is added by
> this patch.

It's preferred to use if(IS_ENABLED()) rather than #ifdef.

> 
> > 
> > But you're effectively asking us to go research these three options,
> > determine the pros/cons and then tell you what we think, at which point I
> > presume you will tell us what you think about each option.
> > 
> > It would be better if you made a suggestion and told us why you think
> > this is the best choice - then we can either agree with your reasoning,
> > or choose to ask more questions or do some research.
> > 
> 
> Understood. I will keep this in mind while presenting choices from now
> onwards.

-- 
With best wishes
Dmitry

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Hrishabh Rajput]

On 11/11/2025 8:35 PM, Dmitry Baryshkov wrote:
> On Tue, Nov 11, 2025 at 07:30:59PM +0530, Hrishabh Rajput wrote:
>> On 11/11/2025 5:52 PM, Dmitry Baryshkov wrote:
>>> On Tue, Nov 11, 2025 at 11:41:51AM +0100, Krzysztof Kozlowski wrote:
>>>> On 11/11/2025 11:34, Dmitry Baryshkov wrote:
>>>>> On Tue, Nov 11, 2025 at 10:51:43AM +0530, Pavan Kondeti wrote:
>>>>>> On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
>>>>>>> On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
>>>>>>>>> +static void qcom_scm_gunyah_wdt_free(void *data)
>>>>>>>>> +{
>>>>>>>>> +	struct platform_device *gunyah_wdt_dev = data;
>>>>>>>>> +
>>>>>>>>> +	platform_device_unregister(gunyah_wdt_dev);
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
>>>>>>>>> +{
>>>>>>>>> +	struct platform_device *gunyah_wdt_dev;
>>>>>>>>> +	struct device_node *np;
>>>>>>>>> +	bool of_wdt_available;
>>>>>>>>> +	int i;
>>>>>>>>> +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
>>>>>>>> static const?
>>>>>>>>
>>>>>>>>> +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
>>>>>>>>> +	static const char * const of_wdt_compatible[] = {
>>>>>>>>> +		"qcom,kpss-wdt",
>>>>>>>>> +		"arm,sbsa-gwdt",
>>>>>>>>> +	};
>>>>>>>>> +
>>>>>>>>> +	/* Bail out if we are not running under Gunyah */
>>>>>>>>> +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
>>>>>>>>> +		return;
>>>>>>>> This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
>>>>>>>>
>>>>>>> Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
>>>>>>>
>>>>>> Dmitry / Bjorn,
>>>>>>
>>>>>> We are debating on this internally on how to resolve this dependency
>>>>>>
>>>>>> - QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
>>>>>>     QCOM_SCM compilation than what it is today.
>>>>>>
>>>>>> - Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver
>>>>>>
>>>>>> - Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
>>>>>>     for any of the functions defined in drivers/firmware/smccc/smccc.c
>>>>>>
>>>>>> We are trending towards the first option above. Please let us know if
>>>>>> you think otherwise.
>>>>> The same as before: 'select HAVE_ARM_SMCCC_DISCOVERY'.
>>>> HAVE_ARM_SMCCC_DISCOVERY has a dependency which is not always selected
>>>> (e.g. ARM32), thus selecting it might lead to warnings of unmet
>>>> dependencies.
>>> Then `if (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY))` might be a good
>>> option here (and depend on GICv3 selecting it).
>> Thanks a lot Dmitry, wemade the change below and compile tested on various
>> architectures (ARM64, ARM32, x86, PowerPC, RISC-V and MIPS) and it was
>> success.
>>
>> We will include it in our next patch version, if there are no further
>> concerns.
>>
>> }; /* Bail out if we are not running under Gunyah */ - if
>> (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid)) + if
>> (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) || +
>> !arm_smccc_hypervisor_has_uuid(&gunyah_uuid)) return; /*
> Unreadable. Don't you read what you are sending?

Sorry, my mail client messed up the formatting while sending. Here is 
the proper version:

/* Bail out if we are not running under Gunyah */
-	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
+	if (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) ||
+	    !arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
		return;

Thanks,
Hrishabh

Re: [PATCH v5 1/2] firmware: qcom: scm: Register gunyah watchdog device

[Bjorn Andersson]

On Wed, Nov 12, 2025 at 10:07:26AM +0530, Hrishabh Rajput wrote:
> 
> On 11/11/2025 8:35 PM, Dmitry Baryshkov wrote:
> > On Tue, Nov 11, 2025 at 07:30:59PM +0530, Hrishabh Rajput wrote:
> > > On 11/11/2025 5:52 PM, Dmitry Baryshkov wrote:
> > > > On Tue, Nov 11, 2025 at 11:41:51AM +0100, Krzysztof Kozlowski wrote:
> > > > > On 11/11/2025 11:34, Dmitry Baryshkov wrote:
> > > > > > On Tue, Nov 11, 2025 at 10:51:43AM +0530, Pavan Kondeti wrote:
> > > > > > > On Mon, Nov 10, 2025 at 09:43:53AM +0530, Pavan Kondeti wrote:
> > > > > > > > On Sat, Nov 08, 2025 at 07:26:46PM +0200, Dmitry Baryshkov wrote:
> > > > > > > > > > +static void qcom_scm_gunyah_wdt_free(void *data)
> > > > > > > > > > +{
> > > > > > > > > > +	struct platform_device *gunyah_wdt_dev = data;
> > > > > > > > > > +
> > > > > > > > > > +	platform_device_unregister(gunyah_wdt_dev);
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +static void qcom_scm_gunyah_wdt_init(struct qcom_scm *scm)
> > > > > > > > > > +{
> > > > > > > > > > +	struct platform_device *gunyah_wdt_dev;
> > > > > > > > > > +	struct device_node *np;
> > > > > > > > > > +	bool of_wdt_available;
> > > > > > > > > > +	int i;
> > > > > > > > > > +	uuid_t gunyah_uuid = UUID_INIT(0xc1d58fcd, 0xa453, 0x5fdb, 0x92, 0x65,
> > > > > > > > > static const?
> > > > > > > > > 
> > > > > > > > > > +				       0xce, 0x36, 0x67, 0x3d, 0x5f, 0x14);
> > > > > > > > > > +	static const char * const of_wdt_compatible[] = {
> > > > > > > > > > +		"qcom,kpss-wdt",
> > > > > > > > > > +		"arm,sbsa-gwdt",
> > > > > > > > > > +	};
> > > > > > > > > > +
> > > > > > > > > > +	/* Bail out if we are not running under Gunyah */
> > > > > > > > > > +	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> > > > > > > > > > +		return;
> > > > > > > > > This rquires 'select HAVE_ARM_SMCCC_DISCOVERY'
> > > > > > > > > 
> > > > > > > > Probably `depends on HAVE_ARM_SMCCC_DISCOVERY` is correct here.
> > > > > > > > 
> > > > > > > Dmitry / Bjorn,
> > > > > > > 
> > > > > > > We are debating on this internally on how to resolve this dependency
> > > > > > > 
> > > > > > > - QCOM_SCM depends on HAVE_ARM_SMCCC_DISCOVERY which means restricting
> > > > > > >     QCOM_SCM compilation than what it is today.
> > > > > > > 
> > > > > > > - Adding #ifdefry around arm_smccc_hypervisor_has_uuid usage in qcom scm driver
> > > > > > > 
> > > > > > > - Adding stub for `arm_smccc_hypervisor_has_uuid()` which is not done
> > > > > > >     for any of the functions defined in drivers/firmware/smccc/smccc.c
> > > > > > > 
> > > > > > > We are trending towards the first option above. Please let us know if
> > > > > > > you think otherwise.
> > > > > > The same as before: 'select HAVE_ARM_SMCCC_DISCOVERY'.
> > > > > HAVE_ARM_SMCCC_DISCOVERY has a dependency which is not always selected
> > > > > (e.g. ARM32), thus selecting it might lead to warnings of unmet
> > > > > dependencies.
> > > > Then `if (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY))` might be a good
> > > > option here (and depend on GICv3 selecting it).
> > > Thanks a lot Dmitry, wemade the change below and compile tested on various
> > > architectures (ARM64, ARM32, x86, PowerPC, RISC-V and MIPS) and it was
> > > success.
> > > 
> > > We will include it in our next patch version, if there are no further
> > > concerns.
> > > 
> > > }; /* Bail out if we are not running under Gunyah */ - if
> > > (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid)) + if
> > > (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) || +
> > > !arm_smccc_hypervisor_has_uuid(&gunyah_uuid)) return; /*
> > Unreadable. Don't you read what you are sending?
> 
> Sorry, my mail client messed up the formatting while sending. Here is the
> proper version:
> 
> /* Bail out if we are not running under Gunyah */
> -	if (!arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> +	if (!IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY) ||
> +	    !arm_smccc_hypervisor_has_uuid(&gunyah_uuid))
> 		return;

LGTM

> 
> Thanks,
> Hrishabh
>