Re: [PATCH v6 02/12] PCI/ACPI: Add PERST# Assertion Delay _DSM method

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Jan 13, 2026 at 10:12:03PM +0530, Badal Nilawar wrote:
> From: Anshuman Gupta <anshuman.gupta@intel.com>
> 
> Implement _DSM Method 0Bh as per PCI Firmware r3.3, sec 4.6.10, to request
> fixed delay in timing between the time the PME_TO_Ack message is received
> at the PCI Express Downstream Port that originated the PME_Turn_Off
> message, and the time the platform asserts PERST# to the slot during the
> corresponding Endpoint’s or PCI Express Upstream Port’s transition to
> D3cold while the system is in an ACPI operational state.
> Host platform supporting this feature ensures that device is observing
> this delay in every applicable D3Cold transition.

Thanks for this work!

Add blank lines between paragraphs.

s/D3Cold/D3cold/ to match other uses in drivers/pci/

> Co-developed-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> For VRSR feature with PERST# Assertion delay device will get enough time
> to transition to auxiliary power before main power removal.
> ---
>  drivers/pci/pci-acpi.c   | 60 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci-acpi.h |  9 +++++-
>  2 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index 645d3005ba50..73eaee20a270 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -1554,6 +1554,66 @@ int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_mw,
>  }
>  EXPORT_SYMBOL_GPL(pci_acpi_request_d3cold_aux_power);
>  
> +/**
> + * pci_acpi_add_perst_assertion_delay - Request PERST# Delay via ACPI DSM
> + * @dev: PCI device instance
> + * @delay_us: Requested delay_us
> + *
> + * Request PERST# Assertion Delay to platform firmware, via Root Port/
> + * Switch Downstream Port ACPI _DSM Function 0Bh, for the specified
> + * PCI device.
> + * Evaluate the _DSM and handle the response accordingly.

Add blank line between paragraphs.  Actually, you can just omit the
last sentence because it doesn't tell us anything useful.

> + * Return: returns 0 on success and errno on failure.

s/Return: returns 0/Return: 0/

> + */
> +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,
> +	};
> +

Spurious blank line.

> +	union acpi_object *out_obj;
> +	int result, ret = -EINVAL;

"ret" is unnecessary; see below.

> +	struct pci_dev *bdev;
> +	acpi_handle handle;
> +
> +	if (!dev)
> +		return -EINVAL;
> +
> +	for (bdev = dev; bdev; bdev = pci_upstream_bridge(bdev)) {

I think bdev should start with pci_upstream_bridge(dev).

IIUC you intend that "dev" is the Endpoint or Upstream Port for which
a driver wants PERST# assertion to be delayed.  Per sec 4.6.11, the
_DSM must be in a Downstream Port, not the device itself.

Sec 4.6.11 also says we should track this per Downstream Port and
request the maximum of delays requested by any child.  So I think we
need to:

  - add a perst_delay in struct pci_dev

  - when we find this _DSM, set
    bdev.perst_delay = max(bdev.perst_delay, delay_us)

  - pass bdev.perst_delay to the _DSM instead of delay_us

> +		handle = ACPI_HANDLE(&bdev->dev);
> +		if (handle &&
> +		    acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4,
> +				   1 << DSM_PCI_PERST_ASSERTION_DELAY))
> +			break;
> +	}
> +
> +	if (!bdev)
> +		return -ENODEV;
> +
> +	out_obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&bdev->dev),
> +					  &pci_acpi_dsm_guid, 4,
> +					  DSM_PCI_PERST_ASSERTION_DELAY,
> +					  &in_obj, ACPI_TYPE_INTEGER);
> +	if (!out_obj)
> +		return -EINVAL;
> +
> +	result = out_obj->integer.value;
> +	ACPI_FREE(out_obj);
> +
> +	if (result == delay_us) {
> +		pci_info(dev, "PERST# Assertion Delay set to %u microseconds\n", delay_us);

I think this message should use "bdev" instead of "dev".  I know the
request is for "dev", but the delay connected to bdev and applies to
all functions below the Downstream Port.

Wrap such that the string itself isn't broken across lines, but the
"delay_us" is one the next line (as in the "failed" message below).

> +		ret = 0;
> +	} else {
> +		pci_info(dev, "PERST# Assertion Delay request failed, using %u microseconds\n",
> +			 result);
> +	}
> +
> +	return ret;

No need for the "else"; we can just return error early and unindent
the normal path:

  if (result != delay_us) {
    pci_info(bdev, "... failed");
    return -EINVAL;
  }

  pci_info(bdev, "... set");
  return 0;

> +}