Re: [PATCH 05/15] cxl/acpi: Add probe function to detect restricted CXL hosts in RCD mode

From: Robert Richter <rrichter@amd.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Alison Schofield <alison.schofield@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Ira Weiny <ira.weiny@intel.com>,
	Ben Widawsky <bwidawsk@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>,
	<linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Len Brown <lenb@kernel.org>
Subject: Re: [PATCH 05/15] cxl/acpi: Add probe function to detect restricted CXL hosts in RCD mode
Date: Thu, 1 Sep 2022 08:01:05 +0200	[thread overview]
Message-ID: <YxBKoT2zlUVgXIry@rric.localdomain> (raw)
In-Reply-To: <20220831110804.00003812@huawei.com>

On 31.08.22 11:08:04, Jonathan Cameron wrote:
> On Wed, 31 Aug 2022 10:15:53 +0200

> Robert Richter <rrichter@amd.com> wrote:
> 
> > Restricted CXL device (RCD) mode (formerly CXL 1.1) uses a different
> > enumeration scheme other than CXL VH (formerly CXL 2.0). In RCD mode a
> > host/device (RCH-RCD) pair shows up as a legal PCIe hierarchy with an
> > ACPI host bridge ("PNP0A08" or "ACPI0016" HID) and RCiEP connected to
> > it with a description of the CXL device.
> > 
> > Add function cxl_restricted_host_probe() to probe RCD enumerated
> > devices. The function implements a loop that detects all CXL capable
> > ACPI PCI root bridges in the system (RCD mode only). The iterator
> > function cxl_find_next_rch() is introduced to walk through all of the
> > CXL hosts. The loop will then enable all CXL devices connected to the
> > host. For now, only implement an empty loop with an iterator that
> > returns all pci host bridges in the system.
> > 
> > The probe function is triggered by adding an own root device for RCHs.
> > This is different to CXL VH where an ACPI "ACPI0017" root device
> > exists. Its detection starts the CXL host detection. In RCD mode such
> > a device does not necessarily exists, so solve this by creating a
> > plain platform device that is not an ACPI device and is root only for
> > RCHs.
> 
> If I read this correctly that platform device is created whether or not
> there are any cxl devices in the system?
> 
> Can we create it only if we find some devices that will be placed
> under it later?

This would move the host detection from probe to init which I wanted
to avoid to better control driver init order dependencies.

I could add a put_device() at the end of a probe so that it will be
released in case no other references use it. This implies the refcount
is maintained for parent devices. Or this needs to be added to. So if
there are no children (hosts) attached to the root device after probe,
it will disappear.

> > @@ -531,7 +566,41 @@ static struct platform_driver cxl_acpi_driver = {
> >  	.id_table = cxl_test_ids,
> >  };
> >  
> > -module_platform_driver(cxl_acpi_driver);
> > +static void cxl_acpi_device_release(struct device *dev) { }
> 
> Why the empty release?  Perhaps introduce this only when it
> does something.

The core device driver requires this in device_release() to be setup.

There is nothing to do as the device is kept in a static struct.
That's why it's empty.

-Robert