Re: [PATCH net-next v9 5/6] net: stmmac: configure SerDes according to the interface mode

On Fri, Mar 7, 2025 at 7:28 AM Choong Yong Liang
[off-list ref] wrote:

On 7/3/2025 4:52 am, David E. Box wrote:

quoted

On Thu, 2025-03-06 at 20:56 +0800, Choong Yong Liang wrote:

quoted

On 6/3/2025 5:05 pm, Andy Shevchenko wrote:

quoted

On Thu, Mar 6, 2025 at 10:39 AM Choong Yong Liang
[off-list ref] wrote:

quoted

On 6/3/2025 3:15 pm, Andy Shevchenko wrote:

quoted

Thu, Feb 27, 2025 at 08:15:21PM +0800, Choong Yong Liang kirjoitti:

...

quoted

quoted

quoted

quoted

quoted

quoted

config DWMAC_INTEL
       default X86
       depends on X86 && STMMAC_ETH && PCI
       depends on COMMON_CLK
+    depends on ACPI

Stray and unexplained change. Please, fix it. We don't need the
dependencies
which are not realised in the compile time.

The dependency on ACPI is necessary because the intel_pmc_ipc.h header
relies on ACPI functionality to interact with the Intel PMC.

So, that header has to be fixed as ACPI here is really unneeded
dependency for the cases when somebody (for whatever reasons) want to
build a kernel without ACPI support but with the driver enabled for
let's say PCI device.

quoted

quoted

Thank you for your feedback, Andy.
I appreciate your insights regarding the ACPI dependency.
The intel_pmc_ipc.h header is under the ownership of David E Box, who
focuses on the platform code, while my focus is on the netdev.

if you could kindly look into making the ACPI dependency optional in the
intel_pmc_ipc.h header, it would be greatly appreciated.
I am more than willing to provide any support necessary to ensure a smooth
resolution.

Choong you only need put the function under a #if CONFIG_ACPI block and provide
an alternative that returns an error when the code is not build. Like this,

#if CONFIG_ACPI
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

quoted

This patch series has already been accepted, but we recognize the
importance of addressing this issue in the next patch series for upstream.
Our goal is to ensure that the driver can be compiled and function
correctly in both ACPI and non-ACPI environments.

Thank you both for your understanding and collaboration.

The current ACPI dependency for the config DWMAC_INTEL is necessary,

I can argue on this. The driver worked without problems on the cases I
explained, so the dependency introduced very recently and only for a
subset of the cases. What you probably wanted to say is that "the
dependency is needed to avoid compilation errors in CONFIG_ACPI=n
cases since the used API doesn't (yet) provide the necessary stubs".
With that being assumed I agree.

but I
agree on making it optional.

Implementing the suggestion from David using the "#if CONFIG_ACPI" approach
would address your concern about users who need to build a kernel without
ACPI support.

If you are okay with this approach, then I will submit the solution for
upstream.

Yes, please do it as the ACPI dependency brings a few hundreds of
kilobytes into the kernel with a lot of possible unneeded stuff.

-- 
With Best Regards,
Andy Shevchenko