On Wed, Mar 23, 2022 at 12:24 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>
> On Fri, Mar 18, 2022 at 10:30 PM Vishal Verma <vishal.l.verma@xxxxxxxxx> wrote:
> >
> > From: Dan Williams <dan.j.williams@xxxxxxxxx>
> >
> > 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/acpi/acpi_bus.h | 1 +
> > drivers/acpi/pci_root.c | 62 +++++++++++++++++++++++++++++++----------
> > 2 files changed, 48 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index ca88c4706f2b..768ef1584055 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -585,6 +585,7 @@ struct acpi_pci_root {
> > struct acpi_device * device;
> > struct pci_bus *bus;
> > u16 segment;
> > + bool cxl_osc_disable;
> > struct resource secondary; /* downstream bus range */
> >
> > u32 osc_support_set; /* _OSC state of support bits */
> > diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> > index b76db99cced3..2d834504096b 100644
> > --- a/drivers/acpi/pci_root.c
> > +++ b/drivers/acpi/pci_root.c
> > @@ -170,20 +170,47 @@ static void decode_osc_control(struct acpi_pci_root *root, char *msg, u32 word)
> > ARRAY_SIZE(pci_osc_control_bit));
> > }
> >
> > -static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
> > +static bool is_pcie(struct acpi_pci_root *root)
> > +{
> > + return strcmp(acpi_device_hid(root->device), "PNP0A08") == 0;
> > +}
> >
> > -static acpi_status acpi_pci_run_osc(acpi_handle handle,
> > +static bool is_cxl(struct acpi_pci_root *root)
> > +{
> > + if (root->cxl_osc_disable)
> > + return false;
> > + return strcmp(acpi_device_hid(root->device), "ACPI0016") == 0;
>
> One more thing.
>
> This will cause the device ID matching to take place every time
> is_cxl() is called which seems a bit excessive to me (and the same
> applies to the PCIe counterpart).
>
> IMV it would make sense to have a "bridge_type" field instead of
> cxl_osc_disable in acpi_pci_root and use that for all of the checks.
>
> Moreover, IIUC every CXL bridge is also a PCIe one, so the
> "bridge_type" could be say 1 for PCIe and 2 for CXL and checks like
> "bridge_type >= PCIe" would cover both these types then.
Sounds good, I like it.
