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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ndwwddd7vzjpgvzg55whdno4ondfxvyg25p2jbdsvy4lmzsfyy@jnn3wywc7xtp>
Date: Thu, 4 Sep 2025 08:47:18 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Hrishabh Rajput <hrishabh.rajput@....qualcomm.com>
Cc: Konrad Dybcio <konradybcio@...nel.org>, 
	Wim Van Sebroeck <wim@...ux-watchdog.org>, Guenter Roeck <linux@...ck-us.net>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, linux-arm-msm@...r.kernel.org, linux-watchdog@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: 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:
> > 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/