On Tue, 2022-04-05 at 15:47 +0200, Rafael J. Wysocki wrote:
> First off, I would change the subject to something like "Prefer CXL
> _OSC to PCI _OSC for CXL host bridges". As is, it kind of suggests
> that the CXL _OSC is preferred in all cases.
>
> On Thu, Mar 31, 2022 at 10:20 PM Vishal Verma <vishal.l.verma@xxxxxxxxx> wrote:
> >
> > From: Dan Williams <dan.j.williams@xxxxxxxxx>
> >
> > OB In preparation for negotiating OS control of CXL _OSC features, do the
> > minimal enabling to use CXL _OSC to handle the base PCIe feature
> > negotiation. Recall that CXL _OSC is a super-set of PCIe _OSC and the
> > CXL 2.0 specification mandates: "If a CXL Host Bridge device exposes CXL
> > _OSC, CXL aware OSPM shall evaluate CXL _OSC and not evaluate PCIe
> > _OSC."
> >
> > Rather than pass a boolean flag alongside @root to all the helper
> > functions that need to consider PCIe specifics, add is_pcie() and
> > is_cxl() helper functions to check the flavor of @root. This also
> > allows for dynamic fallback to PCIe _OSC in cases where an attempt to
> > use CXL _OXC fails. This can happen on CXL 1.1 platforms that publish
> > ACPI0016 devices to indicate CXL host bridges, but do not publish the
> > optional CXL _OSC method. CXL _OSC is mandatory for CXL 2.0 hosts.
> >
> > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> > Cc: Robert Moore <robert.moore@xxxxxxxxx>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> > Signed-off-by: Vishal Verma <vishal.l.verma@xxxxxxxxx>
> > ---
> > include/linux/acpi.h | 4 +++
> > include/acpi/acpi_bus.h | 6 ++++
> > drivers/acpi/pci_root.c | 70 ++++++++++++++++++++++++++++++++---------
> > 3 files changed, 65 insertions(+), 15 deletions(-)
> >
[..]
> >
> > @@ -587,8 +619,16 @@ static int acpi_pci_root_add(struct acpi_device *device,
> >
> > root->mcfg_addr = acpi_pci_root_get_mcfg_addr(handle);
> >
> > - is_pcie = strcmp(acpi_device_hid(device), "PNP0A08") == 0;
> > - negotiate_os_control(root, &no_aspm, is_pcie);
> > + acpi_hid = acpi_device_hid(root->device);
> > + if (strcmp(acpi_hid, "PNP0A08") == 0)
> > + root->bridge_type = ACPI_BRIDGE_TYPE_PCIE;
> > + else if (strcmp(acpi_hid, "ACPI0016") == 0)
> > + root->bridge_type = ACPI_BRIDGE_TYPE_CXL;
> > + else
> > + dev_warn(&device->dev, "unknown bridge type with hid: %s\n",
> > + acpi_hid);
>
> Second, because "device" is an ACPI device object and it has a _HID,
> its name should include the ID in this case, so including it into the
> message once more is redundant.
>
> Also, if this is neither PCIe nor CXL, it should be a "legacy" PCI
> host bridge and that is not an error, so maybe use dev_dbg("Assuming
> non-PCIe host bridge\n")?
Agreed with both points, I'll make the changes for v5.
