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

On Thu, Sep 04, 2025 at 05:10:33PM +0530, Hrishabh Rajput wrote:

On 9/4/2025 1:43 AM, Bjorn Andersson wrote:

quoted

On Wed, Sep 03, 2025 at 07:34:00PM +0000, Hrishabh Rajput via B4 Relay wrote:

quoted

diff --git a/drivers/watchdog/gunyah_wdt.c b/drivers/watchdog/gunyah_wdt.c

[..]

quoted

quoted

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

quoted

quoted

+
+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/ (local)

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