Re: [PATCH] ASoC: stm32: sai: add missing put_device()

Re: [PATCH] ASoC: stm32: sai: add missing put_device()

[Olivier MOYSAN]

Hi Wen,

On 2/9/19 11:41 AM, Wen Yang wrote:
> The of_find_device_by_node() takes a reference to the underlying device
> structure, we should release that reference.
> 
> Fixes: 7dd0d835582f ("ASoC: stm32: sai: simplify sync modes management")
> Signed-off-by: Wen Yang <yellowriver2010@hotmail.com>
> ---
>   sound/soc/stm/stm32_sai.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/stm/stm32_sai.c b/sound/soc/stm/stm32_sai.c
> index bcb35ca..14c9591 100644
> --- a/sound/soc/stm/stm32_sai.c
> +++ b/sound/soc/stm/stm32_sai.c
> @@ -112,16 +112,21 @@ static int stm32_sai_set_sync(struct stm32_sai_data *sai_client,

goto error also in previous test
	if (!pdev) {
		...
		ret = -ENODEV;
		goto error;
	}

>   	if (!sai_provider) {
>   		dev_err(&sai_client->pdev->dev,
>   			"SAI sync provider data not found\n");
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out_put_dev;
>   	}
>   
>   	/* Configure sync client */
>   	ret = stm32_sai_sync_conf_client(sai_client, synci);
>   	if (ret < 0)
> -		return ret;
> +		goto out_put_dev;
>   
>   	/* Configure sync provider */
> -	return stm32_sai_sync_conf_provider(sai_provider, synco);
> +	ret = stm32_sai_sync_conf_provider(sai_provider, synco);
> +
> +out_put_dev:
> +	put_device(&pdev->dev);
> +	return ret;

Here I propose:
error:
	of_node_put(np_provider);
	return ret;

>   }
>   
>   static int stm32_sai_probe(struct platform_device *pdev)
> 

Thanks for your patch. Please, see my comments above.

Regards
Olivier

Re: [PATCH] ASoC: stm32: sai: add missing put_device()

[Olivier MOYSAN]

Acked-by: Olivier Moysan <olivier.moysan@st.com>

On 2/13/19 3:41 PM, Wen Yang wrote:
> From: Olivier MOYSAN <olivier.moysan@st.com>
> Sent: 11 February 2019 15:09
> To: Wen Yang; Arnaud POULIQUEN; Liam Girdwood; Mark Brown; Jaroslav Kysela; Takashi Iwai; Maxime Coquelin; Alexandre TORGUE
> Cc: alsa-devel@alsa-project.org; linux-stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] ASoC: stm32: sai: add missing put_device()
> 
>> On 2/9/19 11:41 AM, Wen Yang wrote:
>>> The of_find_device_by_node() takes a reference to the underlying device
>>> structure, we should release that reference.
>>>
>>> Fixes: 7dd0d835582f ("ASoC: stm32: sai: simplify sync modes management")
>>> Signed-off-by: Wen Yang <yellowriver2010@hotmail.com>
>>> ---
>>>    sound/soc/stm/stm32_sai.c | 11 ++++++++---
>>>    1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/sound/soc/stm/stm32_sai.c b/sound/soc/stm/stm32_sai.c
>>> index bcb35ca..14c9591 100644
>>> --- a/sound/soc/stm/stm32_sai.c
>>> +++ b/sound/soc/stm/stm32_sai.c
>>> @@ -112,16 +112,21 @@ static int stm32_sai_set_sync(struct stm32_sai_data *sai_client,
>>
>> goto error also in previous test
>>        if (!pdev) {
>>                ...
>>                ret = -ENODEV;
>>                 goto error;
>>         }
>>
>>>        if (!sai_provider) {
>>>                dev_err(&sai_client->pdev->dev,
>>>                        "SAI sync provider data not found\n");
>>> -             return -EINVAL;
>>> +             ret = -EINVAL;
>>> +             goto out_put_dev;
>>>        }
>>>
>>>        /* Configure sync client */
>>>        ret = stm32_sai_sync_conf_client(sai_client, synci);
>>>        if (ret < 0)
>>> -             return ret;
>>> +             goto out_put_dev;
>>>
>>>        /* Configure sync provider */
>>> -     return stm32_sai_sync_conf_provider(sai_provider, synco);
>>> +     ret = stm32_sai_sync_conf_provider(sai_provider, synco);
>>> +
>>> +out_put_dev:
>>> +     put_device(&pdev->dev);
>>> +     return ret;
>>
>> Here I propose:
>> error:
>>         of_node_put(np_provider);
>>         return ret;
>>
>>>    }
>>>
>>>    static int stm32_sai_probe(struct platform_device *pdev)
>>>
> 
>> Thanks for your patch. Please, see my comments above.
> 
> Thanks for your comments, in this patch we only fix the problem of missing put_device().
> The problem of missing of_node_put() is a bit more complicated:
> For the variable np_provider(np_sync_provider):
> 1, it is obtained by of_get_parent(), but it is not released;
> 2, error code not obtained when calling sai->pdata->set_sync()
> 

I agree, release of np_sync_provider node is missing, as well as
returned error on set_sync.
So yes, this requires a dedicated patch.
Thanks

Regards
Olivier

> We will submit another patch to fix the failure of np_sync_provider
> Thank you.
> > Regards
> Wen
> 
> 

Applied "ASoC: stm32: sai: add missing put_device()" to the asoc tree

[Mark Brown]

The patch

   ASoC: stm32: sai: add missing put_device()

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 1c3816a194870e7a6622345dab7fb56c7d708613 Mon Sep 17 00:00:00 2001
From: Wen Yang <yellowriver2010@hotmail.com>
Date: Sat, 9 Feb 2019 10:41:09 +0000
Subject: [PATCH] ASoC: stm32: sai: add missing put_device()

The of_find_device_by_node() takes a reference to the underlying device
structure, we should release that reference.

Fixes: 7dd0d835582f ("ASoC: stm32: sai: simplify sync modes management")
Signed-off-by: Wen Yang <yellowriver2010@hotmail.com>
Acked-by: Olivier Moysan <olivier.moysan@st.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/stm/stm32_sai.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/sound/soc/stm/stm32_sai.c b/sound/soc/stm/stm32_sai.c
index bcb35cae2a2c..14c9591aae42 100644
--- a/sound/soc/stm/stm32_sai.c
+++ b/sound/soc/stm/stm32_sai.c
@@ -112,16 +112,21 @@ static int stm32_sai_set_sync(struct stm32_sai_data *sai_client,
 	if (!sai_provider) {
 		dev_err(&sai_client->pdev->dev,
 			"SAI sync provider data not found\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out_put_dev;
 	}
 
 	/* Configure sync client */
 	ret = stm32_sai_sync_conf_client(sai_client, synci);
 	if (ret < 0)
-		return ret;
+		goto out_put_dev;
 
 	/* Configure sync provider */
-	return stm32_sai_sync_conf_provider(sai_provider, synco);
+	ret = stm32_sai_sync_conf_provider(sai_provider, synco);
+
+out_put_dev:
+	put_device(&pdev->dev);
+	return ret;
 }
 
 static int stm32_sai_probe(struct platform_device *pdev)
-- 
2.20.1