On Thu, May 29, 2025 at 1:14 PM Badal Nilawar <badal.nilawar@xxxxxxxxx> wrote:
>
> From: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
>
> Implement _DSM method 0Ah according to PCI firmware specifications,
> section 4.6.10 Rev 3.3., to request auxilary power needed for the
> device when in D3Cold.
>
> Note that this implementation assumes only a single device below the
> Downstream Port will request for Aux Power Limit under a given
> Root Port because it does not track and aggregate requests
> from all child devices below the Downstream Port as required
> by Section 4.6.10 Rev 3.3.
>
> One possible mitigation would be only allowing only first PCIe
> Non-Bridge Endpoint Function 0 driver to call_DSM method 0Ah.
>
> Signed-off-by: Varun Gupta <varun.gupta@xxxxxxxxx>
What's this S-o-b for?
> Signed-off-by: Badal Nilawar <badal.nilawar@xxxxxxxxx>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
> ---
> V2(Bjorn/Rafael):
> - Call acpi_dsm_check() to find method 0Ah supported
> - Return retry interval to caller
> V3(Kuppuswamy)
> - Add NULL check for retry interval
> ---
> drivers/pci/pci-acpi.c | 87 ++++++++++++++++++++++++++++++++++++++++
> include/linux/pci-acpi.h | 8 ++++
> 2 files changed, 95 insertions(+)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index af370628e583..87f30910a5f1 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -1421,6 +1421,93 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev,
> ACPI_FREE(obj);
> }
>
> +/**
> + * pci_acpi_request_d3cold_aux_power - Request aux power while device is in D3Cold
> + * @dev: PCI device instance
> + * @requested_power: Requested auxiliary power in milliwatts
> + * @retry_interval: Retry interval returned by platform to retry auxiliary
> + * power request
> + *
> + * This function sends a request to the host BIOS via root port ACPI _DSM Function 0Ah
> + * for the auxiliary power needed by the PCI device when it is in D3Cold.
> + * It checks and evaluates the _DSM (Device Specific Method) to request the auxiliary
> + * power and handles the response accordingly.
> + *
> + * This function shall be only called by 1st non-bridge Endpoint driver
> + * on Function 0. For a Multi-Function Device, the driver for Function 0 is
> + * required to report an aggregate power requirement covering all
> + * functions contained within the device.
> + *
> + * Return: Returns 0 on success and errno on failure.
> + */
> +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power,
> + u32 *retry_interval)
> +{
> + union acpi_object in_obj = {
> + .integer.type = ACPI_TYPE_INTEGER,
> + .integer.value = requested_power,
> + };
> +
> + union acpi_object *out_obj;
> + acpi_handle handle;
> + int result, ret = -EINVAL;
> +
> + if (!dev || !retry_interval)
> + return -EINVAL;
> +
> + handle = ACPI_HANDLE(&dev->dev);
> + if (!handle)
> + return -EINVAL;
> +
> + if (!acpi_check_dsm(handle, &pci_acpi_dsm_guid, 4, 1 << DSM_PCI_D3COLD_AUX_POWER_LIMIT)) {
> + pci_dbg(dev, "ACPI _DSM 0%Xh not supported\n", DSM_PCI_D3COLD_AUX_POWER_LIMIT);
> + return -ENODEV;
> + }
> +
> + out_obj = acpi_evaluate_dsm_typed(handle, &pci_acpi_dsm_guid, 4,
> + DSM_PCI_D3COLD_AUX_POWER_LIMIT,
> + &in_obj, ACPI_TYPE_INTEGER);
> + if (!out_obj)
> + return -EINVAL;
> +
> + result = out_obj->integer.value;
> + if (retry_interval)
> + *retry_interval = 0;
> +
> + switch (result) {
> + case 0x0:
It would be better to use an enum for the possible return values.
> + pci_dbg(dev, "D3cold Aux Power %u mW request denied\n",
> + requested_power);
> + break;
> + case 0x1:
> + pci_info(dev, "D3cold Aux Power request granted: %u mW\n",
> + requested_power);
> + ret = 0;
> + break;
> + case 0x2:
> + pci_info(dev, "D3cold Aux Power: Main power won't be removed\n");
> + ret = -EBUSY;
> + break;
> + default:
> + if (result >= 0x11 && result <= 0x1F) {
if (!(result & ~0x1F))
I think, and it would be better to use a symbol for this mask (and below too).
> + if (retry_interval) {
This has been checked already and is guaranteed to be nonzero at this point.
> + *retry_interval = result & 0xF;
> + pci_warn(dev, "D3cold Aux Power request needs retry interval: %u seconds\n",
> + *retry_interval);
> + ret = -EAGAIN;
> + }
> + } else {
> + pci_err(dev, "D3cold Aux Power: Reserved or unsupported response: 0x%x\n",
> + result);
> + }
> + break;
> + }
> +
> + ACPI_FREE(out_obj);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_acpi_request_d3cold_aux_power);
> +
> static void pci_acpi_set_external_facing(struct pci_dev *dev)
> {
> u8 val;
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 078225b514d4..6079306ad754 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -121,6 +121,7 @@ extern const guid_t pci_acpi_dsm_guid;
> #define DSM_PCI_DEVICE_NAME 0x07
> #define DSM_PCI_POWER_ON_RESET_DELAY 0x08
> #define DSM_PCI_DEVICE_READINESS_DURATIONS 0x09
> +#define DSM_PCI_D3COLD_AUX_POWER_LIMIT 0x0A
>
> #ifdef CONFIG_PCIE_EDR
> void pci_acpi_add_edr_notifier(struct pci_dev *pdev);
> @@ -132,10 +133,17 @@ static inline void pci_acpi_remove_edr_notifier(struct pci_dev *pdev) { }
>
> int pci_acpi_set_companion_lookup_hook(struct acpi_device *(*func)(struct pci_dev *));
> void pci_acpi_clear_companion_lookup_hook(void);
> +int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power,
> + u32 *retry_interval);
>
> #else /* CONFIG_ACPI */
> static inline void acpi_pci_add_bus(struct pci_bus *bus) { }
> static inline void acpi_pci_remove_bus(struct pci_bus *bus) { }
> +static inline int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power,
> + u32 *retry_interval)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif /* CONFIG_ACPI */
>
> #endif /* _PCI_ACPI_H_ */
> --
