On Wed, 2021-01-13 at 13:40 +0100, Rafael J. Wysocki wrote: > On Tue, Jan 12, 2021 at 1:29 AM Ben Widawsky <ben.widaw...@intel.com> wrote: > > > > From: Vishal Verma <vishal.l.ve...@intel.com> > > > > Add an acpi_cxl module to coordinate the ACPI portions of the CXL > > (Compute eXpress Link) interconnect. This driver binds to ACPI0017 > > objects in the ACPI tree, and coordinates access to the resources > > provided by the ACPI CEDT (CXL Early Discovery Table). > > > > It also coordinates operations of the root port _OSC object to notify > > platform firmware that the OS has native support for the CXL > > capabilities of endpoints. > > This doesn't happen here, but in the next patch. > > > Note: the actbl1.h changes are speculative. The expectation is that they > > will arrive through the ACPICA tree in due time. > > So why don't you put them into a separate patch and drop it from the > series when not necessary any more?
[snip] > > +/* > > + * If/when CXL support is defined by other platform firmware the kernel > > + * will need a mechanism to select between the platform specific version > > + * of this routine, until then, hard-code ACPI assumptions > > + */ > > +int cxl_bus_acquire(struct pci_dev *pdev) > > +{ > > + struct acpi_device *adev; > > + struct pci_dev *root_port; > > + struct device *root; > > + > > + root_port = pcie_find_root_port(pdev); > > + if (!root_port) > > + return -ENXIO; > > + > > + root = root_port->dev.parent; > > + if (!root) > > + return -ENXIO; > > + > > + adev = ACPI_COMPANION(root); > > + if (!adev) > > + return -ENXIO; > > + > > + /* TODO: OSC enabling */ > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(cxl_bus_acquire); > > I would move the addition of cxl_bus_acquire() entirely to the next > patch, it looks quite confusing to me as is. Makes sense - and also agreed with all of your other comments. I've cleaned this up for the next revision. Thanks Rafael! >