[PATCH v1 1/1] dmaengine: acpi: Free resource list at appropriate time

[PATCH v1 1/1] dmaengine: acpi: Free resource list at appropriate time

[Andy Shevchenko]

In one case we don't free resources when formally should, and
in the other we do unneeded "double free" (emptying an empty
list).

Both are not critical issues at all, they just make code robust
against any possible future changes in the flow.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/acpi-dma.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/acpi-dma.c b/drivers/dma/acpi-dma.c
index be73021ecbd6..b053a7f96f85 100644
--- a/drivers/dma/acpi-dma.c
+++ b/drivers/dma/acpi-dma.c
@@ -55,7 +55,7 @@ static int acpi_dma_parse_resource_group(const struct acpi_csrt_group *grp,
 
 	INIT_LIST_HEAD(&resource_list);
 	ret = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
-	if (ret <= 0)
+	if (ret < 0)
 		return 0;
 
 	list_for_each_entry(rentry, &resource_list, node) {
@@ -370,10 +370,11 @@ struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
 	INIT_LIST_HEAD(&resource_list);
 	ret = acpi_dev_get_resources(adev, &resource_list,
 				     acpi_dma_parse_fixed_dma, &pdata);
-	acpi_dev_free_resource_list(&resource_list);
 	if (ret < 0)
 		return ERR_PTR(ret);
 
+	acpi_dev_free_resource_list(&resource_list);
+
 	if (dma_spec->slave_id < 0 || dma_spec->chan_id < 0)
 		return ERR_PTR(-ENODEV);
 
-- 
2.50.1

Re: [PATCH v1 1/1] dmaengine: acpi: Free resource list at appropriate time

[Frank Li]

On Wed, Jun 17, 2026 at 11:14:21AM +0200, Andy Shevchenko wrote:
> In one case we don't free resources when formally should, and
> in the other we do unneeded "double free" (emptying an empty
> list).
>
> Both are not critical issues at all, they just make code robust
> against any possible future changes in the flow.

what are you talking about, can use straight forward words.

You just move acpi_dev_free_resource_list() after check return value
of acpi_dev_get_resources().

>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/dma/acpi-dma.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/acpi-dma.c b/drivers/dma/acpi-dma.c
> index be73021ecbd6..b053a7f96f85 100644
> --- a/drivers/dma/acpi-dma.c
> +++ b/drivers/dma/acpi-dma.c
> @@ -55,7 +55,7 @@ static int acpi_dma_parse_resource_group(const struct acpi_csrt_group *grp,
>
>  	INIT_LIST_HEAD(&resource_list);
>  	ret = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
> -	if (ret <= 0)
> +	if (ret < 0)
>  		return 0;

Dose this change related with this patch?

Frank
>
>  	list_for_each_entry(rentry, &resource_list, node) {
> @@ -370,10 +370,11 @@ struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
>  	INIT_LIST_HEAD(&resource_list);
>  	ret = acpi_dev_get_resources(adev, &resource_list,
>  				     acpi_dma_parse_fixed_dma, &pdata);
> -	acpi_dev_free_resource_list(&resource_list);
>  	if (ret < 0)
>  		return ERR_PTR(ret);
>
> +	acpi_dev_free_resource_list(&resource_list);
> +
>  	if (dma_spec->slave_id < 0 || dma_spec->chan_id < 0)
>  		return ERR_PTR(-ENODEV);
>
> --
> 2.50.1
>

Re: [PATCH v1 1/1] dmaengine: acpi: Free resource list at appropriate time

[Andy Shevchenko]

On Wed, Jun 17, 2026 at 04:52:09PM -0400, Frank Li wrote:
> On Wed, Jun 17, 2026 at 11:14:21AM +0200, Andy Shevchenko wrote:
> > In one case we don't free resources when formally should, and
> > in the other we do unneeded "double free" (emptying an empty
> > list).
> >
> > Both are not critical issues at all, they just make code robust
> > against any possible future changes in the flow.
> 
> what are you talking about, can use straight forward words.
> 
> You just move acpi_dev_free_resource_list() after check return value
> of acpi_dev_get_resources().

Not really. There are _two_ cases. And while they are slightly different,
the solution is to make them behave in the same ways.

...

> >  	INIT_LIST_HEAD(&resource_list);
> >  	ret = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
> > -	if (ret <= 0)
> > +	if (ret < 0)
> >  		return 0;
> 
> Dose this change related with this patch?

Yes. It's mentioned in the very first part of the first sentence in the commit
message.

> >
> >  	list_for_each_entry(rentry, &resource_list, node) {
> > @@ -370,10 +370,11 @@ struct dma_chan *acpi_dma_request_slave_chan_by_index(struct device *dev,
> >  	INIT_LIST_HEAD(&resource_list);
> >  	ret = acpi_dev_get_resources(adev, &resource_list,
> >  				     acpi_dma_parse_fixed_dma, &pdata);
> > -	acpi_dev_free_resource_list(&resource_list);
> >  	if (ret < 0)
> >  		return ERR_PTR(ret);
> >
> > +	acpi_dev_free_resource_list(&resource_list);
> > +
> >  	if (dma_spec->slave_id < 0 || dma_spec->chan_id < 0)
> >  		return ERR_PTR(-ENODEV);
> >
> > --
> > 2.50.1
> >

-- 
With Best Regards,
Andy Shevchenko