Linux Watchdog driver development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: a0282524688@gmail.com
Cc: linux-watchdog@vger.kernel.org, lee@kernel.org,
	linux-hwmon@vger.kernel.org, linux-can@vger.kernel.org,
	linux-rtc@vger.kernel.org, "Vincent Mailhol" <mailhol@kernel.org>,
	"Marc Kleine-Budde" <mkl@pengutronix.de>,
	mfd@lists.linux.dev, "Oleksij Rempel" <o.rempel@pengutronix.de>,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Oliver Hartkopp" <socketcan@hartkopp.net>
Subject: Re: [PATCH v7 01/10] mfd: nct6694: Move module type macros to shared header
Date: Fri, 21 Aug 2026 03:45:50 +0000	[thread overview]
Message-ID: <20260821034550.D9FDD1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260821033505.4017901-2-a0282524688@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Moving driver-specific macros into a global header that facilitates bespoke parent-child accessors.
--

--- Patch [1]: [PATCH v7 01/10] mfd: nct6694: Move module type macros to shared header ---
commit ddd165427cd793b4dafe2d040a37ea0bf7d0d5fc
Author: Ming Yu <a0282524688@gmail.com>

mfd: nct6694: Move module type macros to shared header

This commit moves module type macros from individual sub-device drivers into the
global MFD header include/linux/mfd/nct6694.h. The commit message states this
is a prerequisite for supporting multiple transport interfaces without
duplicating these definitions.

> This is a prerequisite for supporting multiple transport interfaces
> (USB, HIF) without duplicating these definitions.

[Severity: Medium]
Is it accurate that multiple transport interfaces use these definitions?

Since these module ID macros appear to be used exclusively by their respective
child drivers, shouldn't they remain local to those drivers rather than being
centralized to prevent duplicated definitions?

[ ... ]

> diff --git a/include/linux/mfd/nct6694.h b/include/linux/mfd/nct6694.h
> index 6eb9be2cd4a01..3c683e317aa32 100644
> --- a/include/linux/mfd/nct6694.h
> +++ b/include/linux/mfd/nct6694.h
> @@ -8,6 +8,15 @@
>  #ifndef __MFD_NCT6694_H
>  #define __MFD_NCT6694_H
>  
> +#define NCT6694_HWMON_MOD	0x00
> +#define NCT6694_PWM_MOD		0x01
> +#define NCT6694_I2C_MOD		0x03
> +#define NCT6694_CANFD_MOD	0x05
> +#define NCT6694_WDT_MOD		0x07
> +#define NCT6694_RTC_MOD		0x08
> +#define NCT6694_RPT_MOD		0xFF
> +#define NCT6694_GPIO_MOD	NCT6694_RPT_MOD

[Severity: Medium]
Does centralizing these macros here violate the MFD subsystem guidelines?

The guidelines state that driver-specific definitions should not be placed in
the global include/linux/mfd/ directory if they are only used by the parent
and its immediate children. Keeping them local to drivers/mfd/ or the
individual sub-devices is preferred.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821033505.4017901-1-a0282524688@gmail.com?part=1

      reply	other threads:[~2026-08-21  3:45 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260821033505.4017901-1-a0282524688@gmail.com>
2026-08-21  3:34 ` [PATCH v7 01/10] mfd: nct6694: Move module type macros to shared header a0282524688
2026-08-21  3:45   ` sashiko-bot [this message]

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=20260821034550.D9FDD1F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=a0282524688@gmail.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=lee@kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=mailhol@kernel.org \
    --cc=mfd@lists.linux.dev \
    --cc=mkl@pengutronix.de \
    --cc=o.rempel@pengutronix.de \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=socketcan@hartkopp.net \
    /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