[PATCH v2 2/4] ASoC: intel/avs: Use pure devres PCI

Philipp Stanner posted 4 patches 8 months ago
There is a newer version of this series
[PATCH v2 2/4] ASoC: intel/avs: Use pure devres PCI
Posted by Philipp Stanner 8 months ago
pci_request_regions() is a hybrid function which becomes managed if
pcim_enable_device() was called before. This hybrid nature is deprecated
and should not be used anymore.

Replace pci_request_regions() with the always-managed function
pcim_request_all_regions().

Remove the goto jump to pci_release_regions(), since pcim_ functions
clean up automatically.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 sound/soc/intel/avs/core.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index 8fbf33e30dfc..dafe46973146 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -445,7 +445,7 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
 		return ret;
 	}
 
-	ret = pci_request_regions(pci, "AVS HDAudio");
+	ret = pcim_request_all_regions(pci, "AVS HDAudio");
 	if (ret < 0)
 		return ret;
 
@@ -454,8 +454,7 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
 	bus->remap_addr = pci_ioremap_bar(pci, 0);
 	if (!bus->remap_addr) {
 		dev_err(bus->dev, "ioremap error\n");
-		ret = -ENXIO;
-		goto err_remap_bar0;
+		return -ENXIO;
 	}
 
 	adev->dsp_ba = pci_ioremap_bar(pci, 4);
@@ -512,8 +511,6 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
 	iounmap(adev->dsp_ba);
 err_remap_bar4:
 	iounmap(bus->remap_addr);
-err_remap_bar0:
-	pci_release_regions(pci);
 	return ret;
 }
 
-- 
2.48.1
Re: [PATCH v2 2/4] ASoC: intel/avs: Use pure devres PCI
Posted by Amadeusz Sławiński 7 months, 4 weeks ago

On 2025-04-23 10:28, Philipp Stanner wrote:
> pci_request_regions() is a hybrid function which becomes managed if
> pcim_enable_device() was called before. This hybrid nature is deprecated
> and should not be used anymore.
> 
> Replace pci_request_regions() with the always-managed function
> pcim_request_all_regions().
> 
> Remove the goto jump to pci_release_regions(), since pcim_ functions
> clean up automatically.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>   sound/soc/intel/avs/core.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
> index 8fbf33e30dfc..dafe46973146 100644
> --- a/sound/soc/intel/avs/core.c
> +++ b/sound/soc/intel/avs/core.c
> @@ -445,7 +445,7 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
>   		return ret;
>   	}
>   
> -	ret = pci_request_regions(pci, "AVS HDAudio");
> +	ret = pcim_request_all_regions(pci, "AVS HDAudio");
>   	if (ret < 0)
>   		return ret;
>   
> @@ -454,8 +454,7 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
>   	bus->remap_addr = pci_ioremap_bar(pci, 0);
>   	if (!bus->remap_addr) {
>   		dev_err(bus->dev, "ioremap error\n");
> -		ret = -ENXIO;
> -		goto err_remap_bar0;
> +		return -ENXIO;
>   	}
>   
>   	adev->dsp_ba = pci_ioremap_bar(pci, 4);
> @@ -512,8 +511,6 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
>   	iounmap(adev->dsp_ba);
>   err_remap_bar4:
>   	iounmap(bus->remap_addr);
> -err_remap_bar0:
> -	pci_release_regions(pci);

Hm... shouldn't we also drop call to pci_release_regions() in 
avs_pci_remove()?

>   	return ret;
>   }
>   

Nitpick: If there will be v2, can you also align title with how it 
usually is in this directory:
ASoC: Intel: avs: Use pure devres PCI
Re: [PATCH v2 2/4] ASoC: intel/avs: Use pure devres PCI
Posted by Philipp Stanner 7 months, 4 weeks ago
On Thu, 2025-04-24 at 12:21 +0200, Amadeusz Sławiński wrote:
> 
> 
> On 2025-04-23 10:28, Philipp Stanner wrote:
> > pci_request_regions() is a hybrid function which becomes managed if
> > pcim_enable_device() was called before. This hybrid nature is
> > deprecated
> > and should not be used anymore.
> > 
> > Replace pci_request_regions() with the always-managed function
> > pcim_request_all_regions().
> > 
> > Remove the goto jump to pci_release_regions(), since pcim_
> > functions
> > clean up automatically.
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > ---
> >   sound/soc/intel/avs/core.c | 7 ++-----
> >   1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/sound/soc/intel/avs/core.c
> > b/sound/soc/intel/avs/core.c
> > index 8fbf33e30dfc..dafe46973146 100644
> > --- a/sound/soc/intel/avs/core.c
> > +++ b/sound/soc/intel/avs/core.c
> > @@ -445,7 +445,7 @@ static int avs_pci_probe(struct pci_dev *pci,
> > const struct pci_device_id *id)
> >   		return ret;
> >   	}
> >   
> > -	ret = pci_request_regions(pci, "AVS HDAudio");
> > +	ret = pcim_request_all_regions(pci, "AVS HDAudio");
> >   	if (ret < 0)
> >   		return ret;
> >   
> > @@ -454,8 +454,7 @@ static int avs_pci_probe(struct pci_dev *pci,
> > const struct pci_device_id *id)
> >   	bus->remap_addr = pci_ioremap_bar(pci, 0);
> >   	if (!bus->remap_addr) {
> >   		dev_err(bus->dev, "ioremap error\n");
> > -		ret = -ENXIO;
> > -		goto err_remap_bar0;
> > +		return -ENXIO;
> >   	}
> >   
> >   	adev->dsp_ba = pci_ioremap_bar(pci, 4);
> > @@ -512,8 +511,6 @@ static int avs_pci_probe(struct pci_dev *pci,
> > const struct pci_device_id *id)
> >   	iounmap(adev->dsp_ba);
> >   err_remap_bar4:
> >   	iounmap(bus->remap_addr);
> > -err_remap_bar0:
> > -	pci_release_regions(pci);
> 
> Hm... shouldn't we also drop call to pci_release_regions() in 
> avs_pci_remove()?

Oh, yes, we should!

And in soc/sof/sof-pci-dev.c it slipped me too.

Will reiterate.

Thx
P.

> 
> >   	return ret;
> >   }
> >   
> 
> Nitpick: If there will be v2, can you also align title with how it 
> usually is in this directory:
> ASoC: Intel: avs: Use pure devres PCI
>