Re: [RFC PATCH 1/9] cxl/acpi: Add an acpi_cxl module for the CXL interconnect

From: Randy Dunlap <rdunlap@infradead.org>
To: Ben Widawsky <ben.widawsky@intel.com>, linux-cxl@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-acpi@vger.kernel.org,
	Dan Williams <dan.j.williams@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	"Kelley, Sean V" <sean.v.kelley@intel.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>
Subject: Re: [RFC PATCH 1/9] cxl/acpi: Add an acpi_cxl module for the CXL interconnect
Date: Tue, 10 Nov 2020 22:17:23 -0800	[thread overview]
Message-ID: <57238bdc-0dc0-f6df-19b7-cb5e5bc3fc28@infradead.org> (raw)
In-Reply-To: <20201111054356.793390-2-ben.widawsky@intel.com>

Hi,

On 11/10/20 9:43 PM, Ben Widawsky wrote:
> ---
>  drivers/Kconfig       |   1 +
>  drivers/Makefile      |   1 +
>  drivers/cxl/Kconfig   |  30 +++++++++++
>  drivers/cxl/Makefile  |   5 ++
>  drivers/cxl/acpi.c    | 119 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/acpi.h    |  15 ++++++
>  include/acpi/actbl1.h |  52 ++++++++++++++++++
>  7 files changed, 223 insertions(+)

> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> new file mode 100644
> index 000000000000..dd724bd364df
> --- /dev/null
> +++ b/drivers/cxl/Kconfig
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +menuconfig CXL_BUS
> +	tristate "CXL (Compute Express Link) Devices Support"
> +	help
> +	  CXL is a bus that is electrically compatible with PCI-E, but layers
> +	  three protocols on that signalling (CXL.io, CXL.cache, and CXL.mem). The
> +	  CXL.cache protocol allows devices to hold cachelines locally, the
> +	  CXL.mem protocol allows devices to be fully coherent memory targets, the
> +	  CXL.io protocol is equivalent to PCI-E. Say 'y' to enable support for
> +	  the configuration and management of devices supporting these protocols.
> +
> +if CXL_BUS
> +
> +config CXL_BUS_PROVIDER
> +	tristate
> +
> +config CXL_ACPI
> +	tristate "CXL Platform Support"
> +	depends on ACPI
> +	default CXL_BUS

Please provide some justification for something other than the
default default of 'n'. We try hard not to add drivers/modules that
are not required for bootup.

> +	select CXL_BUS_PROVIDER
> +	help
> +	  CXL Platform Support is a prerequisite for any CXL device driver that
> +	  wants to claim ownership of the component register space. By default
> +	  platform firmware assumes Linux is unaware of CXL capabilities and
> +	  requires explicit opt-in. This platform component also mediates
> +	  resources described by the CEDT (CXL Early Discovery Table)

end sentence with '.'

> +
> +	  Say 'y' to enable CXL (Compute Express Link) drivers.

	  or 'm'

> +endif

-- 
~Randy