* [PATCH v2 1/4] ASoC: sof: Use pure devres PCI
2025-04-23 8:28 [PATCH v2 0/4] AsoC: Phase out hybrid PCI devres Philipp Stanner
@ 2025-04-23 8:28 ` Philipp Stanner
2025-04-23 8:28 ` [PATCH v2 2/4] ASoC: intel/avs: " Philipp Stanner
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Philipp Stanner @ 2025-04-23 8:28 UTC (permalink / raw)
To: Cezary Rojewski, Liam Girdwood, Peter Ujfalusi, Bard Liao,
Ranjani Sridharan, Kai Vehmanen, Pierre-Louis Bossart, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Daniel Baluta, Philipp Stanner,
Amadeusz Sławiński, Charles Keepax, Damien Le Moal
Cc: linux-sound, linux-kernel, sound-open-firmware
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 surplus calls to PCI release functions, since pcim_ functions do
cleanup automatically.
Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
sound/soc/sof/sof-pci-dev.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c
index 2fc14b9a33d4..c50249aadea9 100644
--- a/sound/soc/sof/sof-pci-dev.c
+++ b/sound/soc/sof/sof-pci-dev.c
@@ -216,7 +216,7 @@ int sof_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id)
if (ret < 0)
return ret;
- ret = pci_request_regions(pci, "Audio DSP");
+ ret = pcim_request_all_regions(pci, "Audio DSP");
if (ret < 0)
return ret;
@@ -240,8 +240,7 @@ int sof_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id)
path_override->ipc_type = sof_pci_ipc_type;
} else {
dev_err(dev, "Invalid IPC type requested: %d\n", sof_pci_ipc_type);
- ret = -EINVAL;
- goto out;
+ return -EINVAL;
}
path_override->fw_path = fw_path;
@@ -271,13 +270,7 @@ int sof_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id)
sof_pdata->sof_probe_complete = sof_pci_probe_complete;
/* call sof helper for DSP hardware probe */
- ret = snd_sof_device_probe(dev, sof_pdata);
-
-out:
- if (ret)
- pci_release_regions(pci);
-
- return ret;
+ return snd_sof_device_probe(dev, sof_pdata);
}
EXPORT_SYMBOL_NS(sof_pci_probe, "SND_SOC_SOF_PCI_DEV");
@@ -290,9 +283,6 @@ void sof_pci_remove(struct pci_dev *pci)
if (snd_sof_device_probe_completed(&pci->dev) &&
!(sof_pci_debug & SOF_PCI_DISABLE_PM_RUNTIME))
pm_runtime_get_noresume(&pci->dev);
-
- /* release pci regions and disable device */
- pci_release_regions(pci);
}
EXPORT_SYMBOL_NS(sof_pci_remove, "SND_SOC_SOF_PCI_DEV");
--
2.48.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/4] ASoC: intel/avs: Use pure devres PCI
2025-04-23 8:28 [PATCH v2 0/4] AsoC: Phase out hybrid PCI devres Philipp Stanner
2025-04-23 8:28 ` [PATCH v2 1/4] ASoC: sof: Use pure devres PCI Philipp Stanner
@ 2025-04-23 8:28 ` Philipp Stanner
2025-04-24 10:21 ` Amadeusz Sławiński
2025-04-23 8:28 ` [PATCH v2 3/4] AsoC: intel/atom: " Philipp Stanner
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Philipp Stanner @ 2025-04-23 8:28 UTC (permalink / raw)
To: Cezary Rojewski, Liam Girdwood, Peter Ujfalusi, Bard Liao,
Ranjani Sridharan, Kai Vehmanen, Pierre-Louis Bossart, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Daniel Baluta, Philipp Stanner,
Amadeusz Sławiński, Charles Keepax, Damien Le Moal
Cc: linux-sound, linux-kernel, sound-open-firmware
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/4] ASoC: intel/avs: Use pure devres PCI
2025-04-23 8:28 ` [PATCH v2 2/4] ASoC: intel/avs: " Philipp Stanner
@ 2025-04-24 10:21 ` Amadeusz Sławiński
2025-04-24 11:33 ` Philipp Stanner
0 siblings, 1 reply; 9+ messages in thread
From: Amadeusz Sławiński @ 2025-04-24 10:21 UTC (permalink / raw)
To: Philipp Stanner, Cezary Rojewski, Liam Girdwood, Peter Ujfalusi,
Bard Liao, Ranjani Sridharan, Kai Vehmanen, Pierre-Louis Bossart,
Mark Brown, Jaroslav Kysela, Takashi Iwai, Daniel Baluta,
Charles Keepax, Damien Le Moal
Cc: linux-sound, linux-kernel, sound-open-firmware
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/4] ASoC: intel/avs: Use pure devres PCI
2025-04-24 10:21 ` Amadeusz Sławiński
@ 2025-04-24 11:33 ` Philipp Stanner
0 siblings, 0 replies; 9+ messages in thread
From: Philipp Stanner @ 2025-04-24 11:33 UTC (permalink / raw)
To: Amadeusz Sławiński, Philipp Stanner, Cezary Rojewski,
Liam Girdwood, Peter Ujfalusi, Bard Liao, Ranjani Sridharan,
Kai Vehmanen, Pierre-Louis Bossart, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Daniel Baluta, Charles Keepax, Damien Le Moal
Cc: linux-sound, linux-kernel, sound-open-firmware
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/4] AsoC: intel/atom: Use pure devres PCI
2025-04-23 8:28 [PATCH v2 0/4] AsoC: Phase out hybrid PCI devres Philipp Stanner
2025-04-23 8:28 ` [PATCH v2 1/4] ASoC: sof: Use pure devres PCI Philipp Stanner
2025-04-23 8:28 ` [PATCH v2 2/4] ASoC: intel/avs: " Philipp Stanner
@ 2025-04-23 8:28 ` Philipp Stanner
2025-04-23 8:28 ` [PATCH v2 4/4] AsoC: intel/atom: Return -ENOMEM if pcim_iomap() fails Philipp Stanner
2025-04-23 12:33 ` [PATCH v2 0/4] AsoC: Phase out hybrid PCI devres Cezary Rojewski
4 siblings, 0 replies; 9+ messages in thread
From: Philipp Stanner @ 2025-04-23 8:28 UTC (permalink / raw)
To: Cezary Rojewski, Liam Girdwood, Peter Ujfalusi, Bard Liao,
Ranjani Sridharan, Kai Vehmanen, Pierre-Louis Bossart, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Daniel Baluta, Philipp Stanner,
Amadeusz Sławiński, Charles Keepax, Damien Le Moal
Cc: linux-sound, linux-kernel, sound-open-firmware
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 call to pci_release_regions(), since pcim_ functions do
cleanup automatically.
Pass 0 as length parameter to pcim_iomap(), which is the standard way
for ioremapping an entire BAR.
Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
sound/soc/intel/atom/sst/sst_pci.c | 58 +++++++++++++-----------------
1 file changed, 24 insertions(+), 34 deletions(-)
diff --git a/sound/soc/intel/atom/sst/sst_pci.c b/sound/soc/intel/atom/sst/sst_pci.c
index d1e64c3500be..eadcf24cbdc3 100644
--- a/sound/soc/intel/atom/sst/sst_pci.c
+++ b/sound/soc/intel/atom/sst/sst_pci.c
@@ -26,7 +26,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
int ddr_base, ret = 0;
struct pci_dev *pci = ctx->pci;
- ret = pci_request_regions(pci, SST_DRV_NAME);
+ ret = pcim_request_all_regions(pci, SST_DRV_NAME);
if (ret)
return ret;
@@ -38,67 +38,57 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
ddr_base = relocate_imr_addr_mrfld(ctx->ddr_base);
if (!ctx->pdata->lib_info) {
dev_err(ctx->dev, "lib_info pointer NULL\n");
- ret = -EINVAL;
- goto do_release_regions;
+ return -EINVAL;
}
if (ddr_base != ctx->pdata->lib_info->mod_base) {
dev_err(ctx->dev,
"FW LSP DDR BASE does not match with IFWI\n");
- ret = -EINVAL;
- goto do_release_regions;
+ return -EINVAL;
}
ctx->ddr_end = pci_resource_end(pci, 0);
- ctx->ddr = pcim_iomap(pci, 0,
- pci_resource_len(pci, 0));
- if (!ctx->ddr) {
- ret = -EINVAL;
- goto do_release_regions;
- }
+ ctx->ddr = pcim_iomap(pci, 0, 0);
+ if (!ctx->ddr)
+ return -EINVAL;
+
dev_dbg(ctx->dev, "sst: DDR Ptr %p\n", ctx->ddr);
} else {
ctx->ddr = NULL;
}
/* SHIM */
ctx->shim_phy_add = pci_resource_start(pci, 1);
- ctx->shim = pcim_iomap(pci, 1, pci_resource_len(pci, 1));
- if (!ctx->shim) {
- ret = -EINVAL;
- goto do_release_regions;
- }
+ ctx->shim = pcim_iomap(pci, 1, 0);
+ if (!ctx->shim)
+ return -EINVAL;
+
dev_dbg(ctx->dev, "SST Shim Ptr %p\n", ctx->shim);
/* Shared SRAM */
ctx->mailbox_add = pci_resource_start(pci, 2);
- ctx->mailbox = pcim_iomap(pci, 2, pci_resource_len(pci, 2));
- if (!ctx->mailbox) {
- ret = -EINVAL;
- goto do_release_regions;
- }
+ ctx->mailbox = pcim_iomap(pci, 2, 0);
+ if (!ctx->mailbox)
+ return -EINVAL;
+
dev_dbg(ctx->dev, "SRAM Ptr %p\n", ctx->mailbox);
/* IRAM */
ctx->iram_end = pci_resource_end(pci, 3);
ctx->iram_base = pci_resource_start(pci, 3);
- ctx->iram = pcim_iomap(pci, 3, pci_resource_len(pci, 3));
- if (!ctx->iram) {
- ret = -EINVAL;
- goto do_release_regions;
- }
+ ctx->iram = pcim_iomap(pci, 3, 0);
+ if (!ctx->iram)
+ return -EINVAL;
+
dev_dbg(ctx->dev, "IRAM Ptr %p\n", ctx->iram);
/* DRAM */
ctx->dram_end = pci_resource_end(pci, 4);
ctx->dram_base = pci_resource_start(pci, 4);
- ctx->dram = pcim_iomap(pci, 4, pci_resource_len(pci, 4));
- if (!ctx->dram) {
- ret = -EINVAL;
- goto do_release_regions;
- }
+ ctx->dram = pcim_iomap(pci, 4, 0);
+ if (!ctx->dram)
+ return -EINVAL;
+
dev_dbg(ctx->dev, "DRAM Ptr %p\n", ctx->dram);
-do_release_regions:
- pci_release_regions(pci);
- return ret;
+ return 0;
}
/*
--
2.48.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 4/4] AsoC: intel/atom: Return -ENOMEM if pcim_iomap() fails
2025-04-23 8:28 [PATCH v2 0/4] AsoC: Phase out hybrid PCI devres Philipp Stanner
` (2 preceding siblings ...)
2025-04-23 8:28 ` [PATCH v2 3/4] AsoC: intel/atom: " Philipp Stanner
@ 2025-04-23 8:28 ` Philipp Stanner
2025-04-23 12:28 ` Cezary Rojewski
2025-04-23 12:33 ` [PATCH v2 0/4] AsoC: Phase out hybrid PCI devres Cezary Rojewski
4 siblings, 1 reply; 9+ messages in thread
From: Philipp Stanner @ 2025-04-23 8:28 UTC (permalink / raw)
To: Cezary Rojewski, Liam Girdwood, Peter Ujfalusi, Bard Liao,
Ranjani Sridharan, Kai Vehmanen, Pierre-Louis Bossart, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Daniel Baluta, Philipp Stanner,
Amadeusz Sławiński, Charles Keepax, Damien Le Moal
Cc: linux-sound, linux-kernel, sound-open-firmware
The error checks for pcim_iomap() have the function return -EINVAL.
-ENOMEM is a more appropriate error code.
Replace -EINVAL with -ENOMEM.
Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
sound/soc/intel/atom/sst/sst_pci.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/atom/sst/sst_pci.c b/sound/soc/intel/atom/sst/sst_pci.c
index eadcf24cbdc3..edc86519816d 100644
--- a/sound/soc/intel/atom/sst/sst_pci.c
+++ b/sound/soc/intel/atom/sst/sst_pci.c
@@ -49,7 +49,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
ctx->ddr = pcim_iomap(pci, 0, 0);
if (!ctx->ddr)
- return -EINVAL;
+ return -ENOMEM;
dev_dbg(ctx->dev, "sst: DDR Ptr %p\n", ctx->ddr);
} else {
@@ -59,7 +59,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
ctx->shim_phy_add = pci_resource_start(pci, 1);
ctx->shim = pcim_iomap(pci, 1, 0);
if (!ctx->shim)
- return -EINVAL;
+ return -ENOMEM;
dev_dbg(ctx->dev, "SST Shim Ptr %p\n", ctx->shim);
@@ -67,7 +67,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
ctx->mailbox_add = pci_resource_start(pci, 2);
ctx->mailbox = pcim_iomap(pci, 2, 0);
if (!ctx->mailbox)
- return -EINVAL;
+ return -ENOMEM;
dev_dbg(ctx->dev, "SRAM Ptr %p\n", ctx->mailbox);
@@ -76,7 +76,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
ctx->iram_base = pci_resource_start(pci, 3);
ctx->iram = pcim_iomap(pci, 3, 0);
if (!ctx->iram)
- return -EINVAL;
+ return -ENOMEM;
dev_dbg(ctx->dev, "IRAM Ptr %p\n", ctx->iram);
@@ -85,7 +85,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
ctx->dram_base = pci_resource_start(pci, 4);
ctx->dram = pcim_iomap(pci, 4, 0);
if (!ctx->dram)
- return -EINVAL;
+ return -ENOMEM;
dev_dbg(ctx->dev, "DRAM Ptr %p\n", ctx->dram);
return 0;
--
2.48.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 4/4] AsoC: intel/atom: Return -ENOMEM if pcim_iomap() fails
2025-04-23 8:28 ` [PATCH v2 4/4] AsoC: intel/atom: Return -ENOMEM if pcim_iomap() fails Philipp Stanner
@ 2025-04-23 12:28 ` Cezary Rojewski
0 siblings, 0 replies; 9+ messages in thread
From: Cezary Rojewski @ 2025-04-23 12:28 UTC (permalink / raw)
To: Philipp Stanner
Cc: linux-sound, linux-kernel, sound-open-firmware, Liam Girdwood,
Peter Ujfalusi, Bard Liao, Ranjani Sridharan, Kai Vehmanen,
Pierre-Louis Bossart, Mark Brown, Jaroslav Kysela, Takashi Iwai,
Daniel Baluta, Amadeusz Sławiński, Charles Keepax,
Damien Le Moal
On 2025-04-23 10:28 AM, Philipp Stanner wrote:
> The error checks for pcim_iomap() have the function return -EINVAL.
> -ENOMEM is a more appropriate error code.
>
> Replace -EINVAL with -ENOMEM.
Nitpicks:
I believe the last sentence is redundant, the title and the message say
it all.
Next, the suggest scope for the atom-driver is: 'ASoC: Intel: atom:'.
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
> sound/soc/intel/atom/sst/sst_pci.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/sound/soc/intel/atom/sst/sst_pci.c b/sound/soc/intel/atom/sst/sst_pci.c
> index eadcf24cbdc3..edc86519816d 100644
> --- a/sound/soc/intel/atom/sst/sst_pci.c
> +++ b/sound/soc/intel/atom/sst/sst_pci.c
> @@ -49,7 +49,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
>
> ctx->ddr = pcim_iomap(pci, 0, 0);
> if (!ctx->ddr)
> - return -EINVAL;
> + return -ENOMEM;
>
> dev_dbg(ctx->dev, "sst: DDR Ptr %p\n", ctx->ddr);
> } else {
> @@ -59,7 +59,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
> ctx->shim_phy_add = pci_resource_start(pci, 1);
> ctx->shim = pcim_iomap(pci, 1, 0);
> if (!ctx->shim)
> - return -EINVAL;
> + return -ENOMEM;
>
> dev_dbg(ctx->dev, "SST Shim Ptr %p\n", ctx->shim);
>
> @@ -67,7 +67,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
> ctx->mailbox_add = pci_resource_start(pci, 2);
> ctx->mailbox = pcim_iomap(pci, 2, 0);
> if (!ctx->mailbox)
> - return -EINVAL;
> + return -ENOMEM;
>
> dev_dbg(ctx->dev, "SRAM Ptr %p\n", ctx->mailbox);
>
> @@ -76,7 +76,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
> ctx->iram_base = pci_resource_start(pci, 3);
> ctx->iram = pcim_iomap(pci, 3, 0);
> if (!ctx->iram)
> - return -EINVAL;
> + return -ENOMEM;
>
> dev_dbg(ctx->dev, "IRAM Ptr %p\n", ctx->iram);
>
> @@ -85,7 +85,7 @@ static int sst_platform_get_resources(struct intel_sst_drv *ctx)
> ctx->dram_base = pci_resource_start(pci, 4);
> ctx->dram = pcim_iomap(pci, 4, 0);
> if (!ctx->dram)
> - return -EINVAL;
> + return -ENOMEM;
>
> dev_dbg(ctx->dev, "DRAM Ptr %p\n", ctx->dram);
> return 0;
Hi Philipp,
Thanks for the patch, this is certainly an additional effort, on top of
the pcim_xxx one. Couple of nitpicks above but nothing major. Regardless
if you decide to address them or not, feel free to add:
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/4] AsoC: Phase out hybrid PCI devres
2025-04-23 8:28 [PATCH v2 0/4] AsoC: Phase out hybrid PCI devres Philipp Stanner
` (3 preceding siblings ...)
2025-04-23 8:28 ` [PATCH v2 4/4] AsoC: intel/atom: Return -ENOMEM if pcim_iomap() fails Philipp Stanner
@ 2025-04-23 12:33 ` Cezary Rojewski
4 siblings, 0 replies; 9+ messages in thread
From: Cezary Rojewski @ 2025-04-23 12:33 UTC (permalink / raw)
To: Philipp Stanner
Cc: linux-sound, linux-kernel, sound-open-firmware, Liam Girdwood,
Peter Ujfalusi, Bard Liao, Ranjani Sridharan, Kai Vehmanen,
Pierre-Louis Bossart, Mark Brown, Jaroslav Kysela, Takashi Iwai,
Daniel Baluta, Amadeusz Sławiński, Charles Keepax,
Damien Le Moal
On 2025-04-23 10:28 AM, Philipp Stanner wrote:
> Changes in v2:
> - sof: simplify return. (Andy)
> - intel/atom: simplify return. (Andy)
> - Send a separate series for AsoC. (Andy)
> - intel/atom: Add another patch that switches EINVAL to ENOMEM. (Andy)
>
> Hi,
>
> a year ago we spent quite some work trying to get PCI into better shape.
> Some pci_ functions can be sometimes managed with devres, which is
> obviously bad. We want to provide an obvious API, where pci_ functions
> are never, and pcim_ functions are always managed.
>
> Thus, everyone enabling his device with pcim_enable_device() must be
> ported to pcim_ functions. Porting all users will later enable us to
> significantly simplify parts of the PCI subsystem. See here [1] for
> details.
>
> This patch series does that for sound.
>
> Feel free to squash the commits as you see fit.
>
> P.
>
> [1] https://elixir.bootlin.com/linux/v6.14-rc4/source/drivers/pci/devres.c#L18
>
> Philipp Stanner (4):
> ASoC: sof: Use pure devres PCI
> ASoC: intel/avs: Use pure devres PCI
> AsoC: intel/atom: Use pure devres PCI
> AsoC: intel/atom: Return -ENOMEM if pcim_iomap() fails
Nitpick: the scopes used in commit titles do not match recommendations.
Below the suggested ones:
'ASoC: Intel: atom:' for the atom-driver
'ASoC: Intel: avs:' for the avs-driver
> sound/soc/intel/atom/sst/sst_pci.c | 58 +++++++++++++-----------------
> sound/soc/intel/avs/core.c | 7 ++--
> sound/soc/sof/sof-pci-dev.c | 16 ++-------
> 3 files changed, 29 insertions(+), 52 deletions(-)
>
Hi Philipp,
Thank you for the contribution. I do not see any major issues so feel
free to add:
Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
Kind Regards,
Czarek
^ permalink raw reply [flat|nested] 9+ messages in thread