Re: [PATCH v2 05/10] ASoC: amd: acp: add pm ops support for acp pci driver

[<Claudiu.Beznea@microchip.com>]

On 26.06.2023 16:55, Syed Saba Kareem wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Add pm ops support for common acp pci driver.
> 
> Signed-off-by: Syed Saba Kareem <Syed.SabaKareem@amd.com>
> ---
>  sound/soc/amd/acp/acp-pci.c | 46 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/sound/soc/amd/acp/acp-pci.c b/sound/soc/amd/acp/acp-pci.c
> index 4fedad1b740e..a32c14a109b7 100644
> --- a/sound/soc/amd/acp/acp-pci.c
> +++ b/sound/soc/amd/acp/acp-pci.c
> @@ -16,6 +16,7 @@
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
>  #include <linux/module.h>
> +#include <linux/pm_runtime.h>
> 
>  #include "amd.h"
>  #include "../mach-config.h"
> @@ -141,6 +142,11 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id
>                 goto unregister_dmic_dev;
>         }
>         chip->chip_pdev = pdev;
> +       dev_set_drvdata(&pci->dev, chip);
> +       pm_runtime_set_autosuspend_delay(&pci->dev, 2000);
> +       pm_runtime_use_autosuspend(&pci->dev);
> +       pm_runtime_put_noidle(&pci->dev);
> +       pm_runtime_allow(&pci->dev);
>         return ret;
> 
>  unregister_dmic_dev:
> @@ -153,12 +159,49 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id
>         return ret;
>  };
> 
> +static int __maybe_unused snd_acp_suspend(struct device *dev)

You can get rid of __maybe_unused here if (see below)

> +{
> +       struct acp_chip_info *chip;
> +       int ret;
> +
> +       chip = dev_get_drvdata(dev);
> +       ret = acp_deinit(chip->base);
> +       if (ret)
> +               dev_err(dev, "ACP de-init failed\n");
> +       return ret;
> +}
> +
> +static int __maybe_unused snd_acp_resume(struct device *dev)

(same here)

> +{
> +       struct acp_chip_info *chip;
> +       struct acp_dev_data *adata;
> +       struct device child;
> +       int ret;
> +
> +       chip = dev_get_drvdata(dev);
> +       ret = acp_init(chip);
> +       if (ret)
> +               dev_err(dev, "ACP init failed\n");
> +       child = chip->chip_pdev->dev;
> +       adata = dev_get_drvdata(&child);
> +       if (adata)
> +               acp_enable_interrupts(adata);
> +       return ret;
> +}
> +
> +static const struct dev_pm_ops acp_pm_ops = {
> +       SET_RUNTIME_PM_OPS(snd_acp_suspend, snd_acp_resume, NULL)

you use RUNTIME_PM_OPS()

> +       SET_SYSTEM_SLEEP_PM_OPS(snd_acp_suspend, snd_acp_resume)

and SYSTEM_SLEEP_PM_OPS()

> +};
> +
>  static void acp_pci_remove(struct pci_dev *pci)
>  {
>         struct acp_chip_info *chip;
>         int ret;
> 
>         chip = pci_get_drvdata(pci);
> +       pm_runtime_forbid(&pci->dev);
> +       pm_runtime_get_noresume(&pci->dev);
>         if (dmic_dev)
>                 platform_device_unregister(dmic_dev);
>         if (pdev)
> @@ -181,6 +224,9 @@ static struct pci_driver snd_amd_acp_pci_driver = {
>         .id_table = acp_pci_ids,
>         .probe = acp_pci_probe,
>         .remove = acp_pci_remove,
> +       .driver = {
> +               .pm = &acp_pm_ops,

You can use pm_ptr()/pm_sleep_ptr()

> +       },
>  };
>  module_pci_driver(snd_amd_acp_pci_driver);
> 
> --
> 2.25.1
>