[PATCH v4 02/11] PCI/ACPI: Per root port allow one Aux power limit request

[Rafael J. Wysocki]

On Thu, May 29, 2025 at 11:41 PM Sathyanarayanan Kuppuswamy
<sathyanarayanan.kuppuswamy at linux.intel.com> wrote:
>
>
> On 5/29/25 4:16 AM, Badal Nilawar wrote:
> > For given root port allow one Aux power limit request.
> >
> > Cc: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
> > Cc: Anshuman Gupta <anshuman.gupta at intel.com>
> > Signed-off-by: Badal Nilawar <badal.nilawar at intel.com>
> > ---
> >   drivers/acpi/scan.c     |  1 +
> >   drivers/pci/pci-acpi.c  | 25 ++++++++++++++++++++++++-
> >   include/acpi/acpi_bus.h |  2 ++
> >   3 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index fb1fe9f3b1a3..9ae7be9db01a 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -745,6 +745,7 @@ int acpi_device_add(struct acpi_device *device)
> >       INIT_LIST_HEAD(&device->physical_node_list);
> >       INIT_LIST_HEAD(&device->del_list);
> >       mutex_init(&device->physical_node_lock);
> > +     mutex_init(&device->power.aux_pwr_lock);
> >
> >       mutex_lock(&acpi_device_lock);
> >
> > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> > index 87f30910a5f1..d33efba4ca94 100644
> > --- a/drivers/pci/pci-acpi.c
> > +++ b/drivers/pci/pci-acpi.c
> > @@ -1451,6 +1451,7 @@ int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power,
> >       union acpi_object *out_obj;
> >       acpi_handle handle;
> >       int result, ret = -EINVAL;
> > +     struct acpi_device *adev;
> >
> >       if (!dev || !retry_interval)
> >               return -EINVAL;
> > @@ -1464,11 +1465,27 @@ int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power,
> >               return -ENODEV;
> >       }
> >
> > +     adev = ACPI_COMPANION(&dev->dev);
> > +     if (!adev)
> > +             return -EINVAL;
> > +
> > +     mutex_lock(&adev->power.aux_pwr_lock);
> > +
> > +     /* Check if aux power already granted */
> > +     if (adev->power.aux_power_limit) {
> > +             pci_info(dev, "D3cold Aux Power request already granted: %u mW\n",
> > +                      adev->power.aux_power_limit);
> > +             mutex_unlock(&adev->power.aux_pwr_lock);
> > +             return -EPERM;
> > +     }
> > +
> >       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)
> > +     if (!out_obj) {
> > +             mutex_unlock(&adev->power.aux_pwr_lock);
> >               return -EINVAL;
> > +     }
> >
> >       result = out_obj->integer.value;
> >       if (retry_interval)
> > @@ -1478,14 +1495,17 @@ int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power,
> >       case 0x0:
> >               pci_dbg(dev, "D3cold Aux Power %u mW request denied\n",
> >                       requested_power);
> > +             adev->power.aux_power_limit = 0;
> >               break;
> >       case 0x1:
> >               pci_info(dev, "D3cold Aux Power request granted: %u mW\n",
> >                        requested_power);
> > +             adev->power.aux_power_limit = requested_power;
> >               ret = 0;
> >               break;
> >       case 0x2:
> >               pci_info(dev, "D3cold Aux Power: Main power won't be removed\n");
> > +             adev->power.aux_power_limit = 0;
> >               ret = -EBUSY;
> >               break;
> >       default:
> > @@ -1500,9 +1520,12 @@ int pci_acpi_request_d3cold_aux_power(struct pci_dev *dev, u32 requested_power,
> >                       pci_err(dev, "D3cold Aux Power: Reserved or unsupported response: 0x%x\n",
> >                               result);
> >               }
> > +             adev->power.aux_power_limit = 0;
> >               break;
> >       }
> >
> > +     mutex_unlock(&adev->power.aux_pwr_lock);
> > +
> >       ACPI_FREE(out_obj);
> >       return ret;
> >   }
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index aad1a95e6863..c4ce3d84be00 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -294,6 +294,8 @@ struct acpi_device_power {
> >       struct acpi_device_power_flags flags;
> >       struct acpi_device_power_state states[ACPI_D_STATE_COUNT];      /* Power states (D0-D3Cold) */
> >       u8 state_for_enumeration; /* Deepest power state for enumeration */
> > +     u32 aux_power_limit;            /* aux power limit granted by bios */
> > +     struct mutex aux_pwr_lock;      /* prevent concurrent aux power limit requests */
>
>
> Do you need a new lock ?

Yes.

> Is it possible to reuse existing mutex like device_lock()?

No.

Doing such things results in code where nobody knows what the given
lock scope is.