Re: [PATCH v7 01/10] mfd: nct6694: Move module type macros to shared header
| Newsgroups | org.kernel.vger.linux-watchdog,dev.linux.lists.mfd,org.kernel.vger.linux-can,org.kernel.vger.linux-hwmon,org.kernel.vger.linux-rtc |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]> 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/[email protected]?part=1