On Wed, Apr 2, 2025 at 4:21 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>
> On Wed, Apr 02, 2025 at 01:06:42PM +0200, Rafael J. Wysocki wrote:
> > On Tue, Apr 1, 2025 at 5:36 PM Anshuman Gupta <anshuman.gupta@xxxxxxxxx> wrote:
> > >
> > > Implement _DSM Method 11 as per PCI firmware specs
> > > section 4.6.11 Rev 3.3.
>
> > > +int pci_acpi_add_perst_assertion_delay(struct pci_dev *dev, u32 delay_us)
> > > +{
> > > + union acpi_object in_obj = {
> > > + .integer.type = ACPI_TYPE_INTEGER,
> > > + .integer.value = delay_us,
> > > + };
> > > +
> > > + union acpi_object *out_obj;
> > > + acpi_handle handle;
> > > + int result, ret = -EINVAL;
> > > +
> > > + if (!dev || !ACPI_HANDLE(&dev->dev))
> > > + return -EINVAL;
> > > +
> > > + handle = ACPI_HANDLE(&dev->dev);
> > > +
> > > + out_obj = acpi_evaluate_dsm_typed(handle, &pci_acpi_dsm_guid, 4,
> >
> > This is something I haven't noticed in the previous patch, but also
> > applies to it.
> >
> > Why is rev 4 of the interface hard-coded here?
>
> Thanks for asking this because it's related to the whole _DSM revision
> question that I don't understand.
>
> If we didn't use rev 4 here, what should we use? The PCI Firmware
> spec, r3.3, sec 4.6.11, documents this interface and says "lowest
> valid Revision ID value is 4", so that's the source of the 4.
Well, the "lowest vaild Revision ID" does not generally mean the "only
valid Revision ID".
> My argument is that the spec documents rev 4, the kernel code was
> tested with rev 4, so what would be the benefit of using a different
> revision here?
I'm talking about using a symbol to represent the number 4, not about
possibly using a different number, along the lines of using, say,
ACPI_FADT_LOW_POWER_S0 instead of putting BIT(21) directly into the
code.
The value is not likely to change, but using a symbol for representing
it has merit (it can be meaningfully used in searches, it can be
documented etc.).
Now, I'm not sure how likely it is for the PCI Firmware spec to bump
up the revision of this interface (I suppose that it will do so if a
new function is defined), but even if it does so, the kernel will have
to check both the new revision and rev 4 anyway, in case the firmware
doesn't know about the new revision.
