linux-kernel - Re: [PATCH v1] platform/x86: intel_pmc_ipc: add option to build without ACPI

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: <CAHp75VekTbp++4yw4yDhtB96K+C0w1uHiVih5x-jO+TWRLiPmw@mail.gmail.com>
Date: Wed, 12 Mar 2025 09:54:52 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Choong Yong Liang <yong.liang.choong@...ux.intel.com>
Cc: Hans de Goede <hdegoede@...hat.com>, Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, 
	David E Box <david.e.box@...ux.intel.com>, platform-driver-x86@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] platform/x86: intel_pmc_ipc: add option to build
 without ACPI

On Wed, Mar 12, 2025 at 4:30 AM Choong Yong Liang
<yong.liang.choong@...ux.intel.com> wrote:

Thank you, my comments below.

> This patch introduces a configuration option that allows users to

s/This patch introduces/Introduce/

> build the intel_pmc_ipc driver without ACPI support. This is useful
> for systems where ACPI is not available or desired.
>
> Based on the discussion from the patch: https://patchwork.kernel.org/
> project/netdevbpf/patch/20250227121522.1802832-6-
> yong.liang.choong@...ux.intel.com/#26280764, it was necessary to
> provide this option to accommodate specific use cases.

Make it a Link tag, like

"...from the patch [1], it was..."


Link: https://.... [1]


> Signed-off-by: David E. Box <david.e.box@...ux.intel.com>
> Signed-off-by: Choong Yong Liang <yong.liang.choong@...ux.intel.com>

This is wrong as either it's a wrong tag (SoB --> Suggested-by?), or
missing Co-developed-by, or wrong order (but in that case David should
have sent the patch).

...

> +#if CONFIG_ACPI

Better to have #ifdef, but see below

>  static inline int intel_pmc_ipc(struct pmc_ipc_cmd *ipc_cmd, struct pmc_ipc_rbuf *rbuf)
>  {

>  }
> +#else
> +static inline int intel_pmc_ipc(struct pmc_ipc_cmd *ipc_cmd, struct pmc_ipc_rbuf *rbuf)
> +{ return -ENODEV; }
> +#endif /* CONFIG_ACPI */

Since it's already static inline, it might be more natural to have
this inside the function. The current is usually used for the C impl.
+ static inline stub, like

#ifdef FOO
int foo(...);
#else
static inline int foo(...) { return ... }
#endif

But I'm not insisting, it's up to the PDx86 maintainers.

-- 
With Best Regards,
Andy Shevchenko