Re: [PATCH] cxl/acpi: Fix CXL_ACPI and CXL_PMEM Kconfig tristate mismatch

[<dan.j.williams@intel.com>]

Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> Commit e7e222ad73d9 moves devm_cxl_add_nvdimm_bridge() into the cxl_pmem
> file, which has independent config compile options for built-in or
> module. The call from cxl_acpi_probe() is guarded by
> IS_ENABLED(CONFIG_CXL_PMEM), which evaluates to true for both =y and =m.
> 
> When CONFIG_CXL_PMEM=m, a built-in cxl_acpi attempts to reference a
> symbol exported by a module, which fails to link. CXL_PMEM cannot simply
> be promoted to =y in this configuration because it depends on LIBNVDIMM,
> which may itself be =m.
> 
> Add a Kconfig dependency to prevent CXL_ACPI from being built-in when
> CXL_PMEM is a module. This contrains CXL_ACPI to =m when CXL_PMEM=m,
> while still allowing CXL_ACPI to be freely configured when CXL_PMEM is
> either built-in or disabled.
> 
> Fixes: e7e222ad73d9 ("cxl: Move devm_cxl_add_nvdimm_bridge() to cxl_pmem.ko")
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  drivers/cxl/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 4589bf11d3fe0..80aeb0d556bd7 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -59,6 +59,7 @@ config CXL_ACPI
>  	tristate "CXL ACPI: Platform Support"
>  	depends on ACPI
>  	depends on ACPI_NUMA
> +	depends on CXL_PMEM || !CXL_PMEM

So I do not think this is sufficient because there is no
devm_cxl_add_nvdimm_bridge() stub in the CONFIG_CXL_PMEM=n case, right?

I came up with this. I do not like that it puts ifdefs in the code, but
then again I do not think add_root_nvdimm_bridge() is big enough to
deserve its own C file.

-- 8< --
diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index 4589bf11d3fe..277877da8263 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -75,6 +75,11 @@ config CXL_ACPI
 
 	  If unsure say 'm'.
 
+config CXL_ACPI_PMEM
+	def_bool y
+	depends on CXL_ACPI
+	depends on CXL_PMEM >= CXL_ACPI
+
 config CXL_PMEM
 	tristate "CXL PMEM: Persistent Memory Support"
 	depends on LIBNVDIMM
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 127537628817..a42ad5035958 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -741,6 +741,7 @@ static int add_host_bridge_uport(struct device *match, void *arg)
 	return 0;
 }
 
+#ifdef CONFIG_CXL_ACPI_PMEM
 static int add_root_nvdimm_bridge(struct device *match, void *data)
 {
 	struct cxl_decoder *cxld;
@@ -765,6 +766,18 @@ static int add_root_nvdimm_bridge(struct device *match, void *data)
 	return 1;
 }
 
+static int setup_pmem(struct cxl_port *root_port)
+{
+	return device_for_each_child(&root_port->dev, root_port,
+				     add_root_nvdimm_bridge);
+}
+#else
+static int setup_pmem(struct cxl_port *root_port)
+{
+	return 0;
+}
+#endif
+
 static struct lock_class_key cxl_root_key;
 
 static void cxl_acpi_lock_reset_class(void *dev)
@@ -952,9 +965,7 @@ static int cxl_acpi_probe(struct platform_device *pdev)
 	if (rc < 0)
 		return rc;
 
-	if (IS_ENABLED(CONFIG_CXL_PMEM))
-		rc = device_for_each_child(&root_port->dev, root_port,
-					   add_root_nvdimm_bridge);
+	rc = setup_pmem(root_port);
 	if (rc < 0)
 		return rc;