On 12/3/2025 3:54 pm, Andy Shevchenko wrote:
On Wed, Mar 12, 2025 at 4:30 AM Choong Yong Liang
<yong.liang.choong@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx/#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]
Hi Andy,
Thank you for your detailed feedback and suggestions. I'll make the
necessary adjustments to the patch based on your comments above.
Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
Signed-off-by: Choong Yong Liang <yong.liang.choong@xxxxxxxxxxxxxxx>
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).
I believe the sequence is still correct, as the solution was provided by
David, and he should be the main author. I'm just the submitter, so my
sign-off should be placed last.
...
+#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.
Sure, let's wait for more feedback.
