diff mbox series

[v1] platform/x86: intel_pmc_ipc: add option to build without ACPI

Message ID 20250312022955.1418234-1-yong.liang.choong@linux.intel.com (mailing list archive)
State Changes Requested, archived
Headers show
Series [v1] platform/x86: intel_pmc_ipc: add option to build without ACPI | expand

Commit Message

Choong Yong Liang March 12, 2025, 2:29 a.m. UTC
This patch introduces a configuration option that allows users to
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@linux.intel.com/#26280764, it was necessary to
provide this option to accommodate specific use cases.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
---
 include/linux/platform_data/x86/intel_pmc_ipc.h | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Andy Shevchenko March 12, 2025, 7:54 a.m. UTC | #1
On Wed, Mar 12, 2025 at 4:30 AM Choong Yong Liang
<yong.liang.choong@linux.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@linux.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@linux.intel.com>
> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.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.
Choong Yong Liang March 12, 2025, 11:23 a.m. UTC | #2
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@linux.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@linux.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]
> 
> 
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@linux.intel.com>
>> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.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).
> 
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.
Andy Shevchenko March 12, 2025, 2 p.m. UTC | #3
On Wed, Mar 12, 2025 at 1:23 PM Choong Yong Liang
<yong.liang.choong@linux.intel.com> wrote:
> 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@linux.intel.com> wrote:

...

> 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@linux.intel.com>
> >> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.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).
> >
> 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.

You are the submitter *and* the author in this case. As I said, that
SoB chain is wrong in its current form.
Choong Yong Liang March 13, 2025, 8:59 a.m. UTC | #4
On 12/3/2025 10:00 pm, Andy Shevchenko wrote:
> On Wed, Mar 12, 2025 at 1:23 PM Choong Yong Liang
> <yong.liang.choong@linux.intel.com> wrote:
>> 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@linux.intel.com> wrote:
> 
> ...
> 
>> 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@linux.intel.com>
>>>> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.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).
>>>
>> 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.
> 
> You are the submitter *and* the author in this case. As I said, that
> SoB chain is wrong in its current form.
> 
Hi Andy,

I believe this time I have correctly attributed the authorship by following 
the guide from:
https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
---
From: David E. Box <david.e.box@linux.intel.com>

<changelog>

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Co-developed-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
---
diff mbox series

Patch

diff --git a/include/linux/platform_data/x86/intel_pmc_ipc.h b/include/linux/platform_data/x86/intel_pmc_ipc.h
index 6e603a8c075f..a852f1a02532 100644
--- a/include/linux/platform_data/x86/intel_pmc_ipc.h
+++ b/include/linux/platform_data/x86/intel_pmc_ipc.h
@@ -34,6 +34,7 @@  struct pmc_ipc_rbuf {
  *
  * Return: 0 on success. Non-zero on mailbox error
  */
+#if CONFIG_ACPI
 static inline int intel_pmc_ipc(struct pmc_ipc_cmd *ipc_cmd, struct pmc_ipc_rbuf *rbuf)
 {
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
@@ -90,5 +91,9 @@  static inline int intel_pmc_ipc(struct pmc_ipc_cmd *ipc_cmd, struct pmc_ipc_rbuf
 
 	return 0;
 }
+#else
+static inline int intel_pmc_ipc(struct pmc_ipc_cmd *ipc_cmd, struct pmc_ipc_rbuf *rbuf)
+{ return -ENODEV; }
+#endif /* CONFIG_ACPI */
 
 #endif /* INTEL_PMC_IPC_H */