On Fri, 30 May 2025 11:02:27 +0100
Jonathan Cameron <jonathan.came...@huawei.com> wrote:
> On Tue, 27 May 2025 09:40:07 +0200
> Eric Auger <eric.au...@redhat.com> wrote:
>
> > acpi_dsdt_add_pci_osc() name is confusing as it gives the impression
> > it appends the _OSC method but in fact it also appends the _DSM method
> > for the host bridge. Let's split the function into two separate ones
> > and let them return the method Aml pointer instead. This matches the
> > way it is done on x86 (build_q35_osc_method). In a subsequent patch
> > we will replace the gpex method by the q35 implementation that will
> > become shared between ARM and x86.
> >
> > acpi_dsdt_add_host_bridge_methods is a new top helper that generates
> > both the _OSC and _DSM methods.
> >
> > Signed-off-by: Eric Auger <eric.au...@redhat.com>
> > Reviewed-by: Gustavo Romero <gustavo.rom...@linaro.org>
>
> Makes complete sense. I've had local equivalent of this on the CXL
> tree for a while as without it we don't register the _DSM for the
> CXL path (and we should). However, can you modify it a little to
> make that easier for me? Basically make sure the _DSM is registered
> for the CXL path as well.
>
[...]
unless CXL is root host bridge, current _DSM shouldn't be added to it.
read on comment below.
> > @@ -124,7 +125,17 @@ static void acpi_dsdt_add_pci_osc(Aml *dev, bool
> > enable_native_pcie_hotplug)
> > byte_list[0] = 0;
> > buf = aml_buffer(1, byte_list);
> > aml_append(method, aml_return(buf));
> > - aml_append(dev, method);
> > + return method;
> > +}
> > +
> > +static void acpi_dsdt_add_host_bridge_methods(Aml *dev,
> > + bool
> > enable_native_pcie_hotplug)
> > +{
> > + aml_append(dev, aml_name_decl("SUPP", aml_int(0)));
> > + aml_append(dev, aml_name_decl("CTRL", aml_int(0)));
>
> These two declarations seem to be very much part of the _OSC build though not
> within the the method. I 'think' you get left with them later with no users.
> So move them into the osc build here and they will naturally go away when
> you move to the generic code.
>
> They end up unused in the DSDT at the end of the series.
>
> I ran a quick GPEX + pxb-pcie test and we do get the odd mix that the OSC for
> the GPEX say no native hotplug but the OSC for the PXB allows it.
It's fine for each PXB to have it's own _OSC.
Also current incarnation of ACPI pcihp doesn't support PXBs at all,
it would be wrong to enable the on PXBs.
Thus I'd avoid touching CXL related code paths from this series.
I'm working on extending ACPI pcihp to PXBs
(for the same reason as Eric does for arm/virt, i.e. enable acpi-index support
there).
I can add CXL bits then if there is a need/demand for that in CXL land.
[...]
