Linux Watchdog driver development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Paul Louvel" <paul.louvel@bootlin.com>
Cc: linux-watchdog@vger.kernel.org
Subject: Re: [PATCH v3 4/6] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data
Date: Sun, 04 Oct 2026 12:21:27 +0000	[thread overview]
Message-ID: <20261004122128.492B61F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20261004-w83627hf_wdt-improvements-v3-4-8e27b518595e@bootlin.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] watchdog: w83627hf: macros implicitly depend on local variable base
--

commit 7755bf14a50c6dd55f8caec9bfbe7bfcc9d7de23
Author: Paul Louvel <paul.louvel@bootlin.com>

watchdog: w83627hf_wdt: Store Super I/O configurations in platform data

Instead of using global variables, this commit stores Super I/O related
configurations in platform data.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index 44ec2f2e23eab..1ef7b61d7e558 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
[ ... ]
> @@ -76,9 +72,8 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
>  #define SIO_REG_CONF_ADDR0	0x2E
>  #define SIO_REG_CONF_ADDR1	0x4E
>  
> -#define WDT_EFER (wdt_io+0)   /* Extended Function Enable Registers */
> -#define WDT_EFIR (wdt_io+0)   /* Extended Function Index Register
> -							(same as EFER) */
> +#define WDT_EFER (base+0)   /* Extended Function Enable Registers */
> +#define WDT_EFIR (base+0)   /* Extended Function Index Register (same as EFER) */
>  #define WDT_EFDR (WDT_EFIR+1) /* Extended Function Data Register */

[Severity: Low]
Does this code implicitly rely on the calling function having a local variable
named base?

This isn't a runtime bug, but the patch replaces a global variable reference
with an undeclared identifier. This creates macros that rely on a magic local
variable name base in the scope of the caller, which can cause build failures
for future developers if they invoke these macros in a function scope that
lacks this local variable. This pattern is noted as an anti-pattern in the
Linux kernel coding style under "Things to avoid when using macros".

Could base be passed explicitly as a parameter to the macro to avoid this
implicit dependency? For example, redefining them as WDT_EFER(base) and
WDT_EFIR(base).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004-w83627hf_wdt-improvements-v3-0-8e27b518595e@bootlin.com?part=4

  reply	other threads:[~2026-10-04 12:21 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-04 12:12 [PATCH v3 0/6] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
2026-10-04 12:12 ` [PATCH v3 1/6] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros Paul Louvel
2026-10-06 14:24   ` Tzung-Bi Shih
2026-10-04 12:12 ` [PATCH v3 2/6] watchdog: w83627hf_wdt: Convert to platform driver model Paul Louvel
2026-10-06 14:24   ` Tzung-Bi Shih
2026-10-07 17:05   ` kernel test robot
2026-10-04 12:12 ` [PATCH v3 3/6] watchdog: w83627hf_wdt: Move register offsets into driver data Paul Louvel
2026-10-06 14:25   ` Tzung-Bi Shih
2026-10-09 14:12     ` Paul Louvel
2026-10-04 12:12 ` [PATCH v3 4/6] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data Paul Louvel
2026-10-04 12:21   ` sashiko-bot [this message]
2026-10-06 14:25   ` Tzung-Bi Shih
2026-10-04 12:12 ` [PATCH v3 5/6] watchdog: w83627hf_wdt: Add minute mode counting Paul Louvel
2026-10-04 12:12 ` [PATCH v3 6/6] watchdog: w83627hf_wdt: Report all initialization failures in probe Paul Louvel
2026-10-06 14:25   ` Tzung-Bi Shih
2026-10-09 14:16     ` Paul Louvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261004122128.492B61F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=paul.louvel@bootlin.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox