dmaengine Archive on lore.kernel.org
 help / color / Atom feed
From: Dmitry Osipenko <digetx@gmail.com>
To: "Jon Hunter" <jonathanh@nvidia.com>,
	"Laxman Dewangan" <ldewangan@nvidia.com>,
	"Vinod Koul" <vkoul@kernel.org>,
	"Dan Williams" <dan.j.williams@intel.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Michał Mirosław" <mirq-linux@rere.qmqm.pl>
Cc: dmaengine@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 09/14] dmaengine: tegra-apb: Clean up runtime PM teardown
Date: Thu, 16 Jan 2020 20:18:11 +0300
Message-ID: <11cdf32b-23d5-8a4e-0832-3c75e90b8abe@gmail.com> (raw)
In-Reply-To: <9a5c4f82-5653-8d81-e304-76675aff5d8f@nvidia.com>

15.01.2020 12:57, Jon Hunter пишет:
> 
> On 12/01/2020 17:30, Dmitry Osipenko wrote:
>> It's cleaner to teardown RPM by revering the enable sequence, which makes
>> code much easier to follow.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/dma/tegra20-apb-dma.c | 22 +++++++++++++---------
>>  1 file changed, 13 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
>> index 7158bd3145c4..cc4a9ca20780 100644
>> --- a/drivers/dma/tegra20-apb-dma.c
>> +++ b/drivers/dma/tegra20-apb-dma.c
>> @@ -1429,13 +1429,15 @@ static int tegra_dma_probe(struct platform_device *pdev)
>>  	spin_lock_init(&tdma->global_lock);
>>  
>>  	pm_runtime_enable(&pdev->dev);
>> -	if (!pm_runtime_enabled(&pdev->dev))
>> +	if (!pm_runtime_enabled(&pdev->dev)) {
>>  		ret = tegra_dma_runtime_resume(&pdev->dev);
>> -	else
>> +		if (ret)
>> +			return ret;
>> +	} else {
>>  		ret = pm_runtime_get_sync(&pdev->dev);
>> -
>> -	if (ret < 0)
>> -		goto err_pm_disable;
>> +		if (ret < 0)
>> +			goto err_pm_disable;
>> +	}
>>  
>>  	/* Reset DMA controller */
>>  	reset_control_assert(tdma->rst);
>> @@ -1545,9 +1547,10 @@ static int tegra_dma_probe(struct platform_device *pdev)
>>  	dma_async_device_unregister(&tdma->dma_dev);
>>  
>>  err_pm_disable:
>> -	pm_runtime_disable(&pdev->dev);
>> -	if (!pm_runtime_status_suspended(&pdev->dev))
>> +	if (!pm_runtime_enabled(&pdev->dev))
>>  		tegra_dma_runtime_suspend(&pdev->dev);
>> +	else
>> +		pm_runtime_disable(&pdev->dev);
>>  
>>  	return ret;
>>  }
>> @@ -1558,9 +1561,10 @@ static int tegra_dma_remove(struct platform_device *pdev)
>>  
>>  	dma_async_device_unregister(&tdma->dma_dev);
>>  
>> -	pm_runtime_disable(&pdev->dev);
>> -	if (!pm_runtime_status_suspended(&pdev->dev))
>> +	if (!pm_runtime_enabled(&pdev->dev))
>>  		tegra_dma_runtime_suspend(&pdev->dev);
>> +	else
>> +		pm_runtime_disable(&pdev->dev);
> 
> Looks like dma_async_device_unregister() will warn if a client still has
> a channel requested but does not prevent the unregister from completing.
> So it could be possible that we could be leaving the controller active now.

It's a drivers dependency bug if DMA driver's module isn't properly
refcounted and thus could be removed while it has active users. Nothing
we can do about it here, the actual source of the bug needs to be fixed.

Perhaps Tegra DMA driver could inc/dec module's refcounf on channel's
request/free, but I think it should be responsibility of the DMA core to
care about the refcounting (if it doesn't do it already).

  reply index

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-12 17:29 [PATCH v4 00/14] NVIDIA Tegra APB DMA driver fixes and improvements Dmitry Osipenko
2020-01-12 17:29 ` [PATCH v4 01/14] dmaengine: tegra-apb: Fix use-after-free Dmitry Osipenko
2020-01-14 15:09   ` Jon Hunter
2020-01-14 20:33     ` Dmitry Osipenko
2020-01-15  9:00       ` Jon Hunter
2020-01-16 20:10         ` Dmitry Osipenko
2020-01-12 17:29 ` [PATCH v4 02/14] dmaengine: tegra-apb: Implement synchronization callback Dmitry Osipenko
2020-01-14 15:15   ` Jon Hunter
2020-01-14 21:02     ` Dmitry Osipenko
2020-01-15  9:18       ` Jon Hunter
2020-01-15 10:25         ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 03/14] dmaengine: tegra-apb: Prevent race conditions on channel's freeing Dmitry Osipenko
2020-01-14 15:16   ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 04/14] dmaengine: tegra-apb: Clean up tasklet releasing Dmitry Osipenko
2020-01-14 15:36   ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 05/14] dmaengine: tegra-apb: Prevent race conditions of tasklet vs free list Dmitry Osipenko
2020-01-14 15:43   ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 06/14] dmaengine: tegra-apb: Use devm_platform_ioremap_resource Dmitry Osipenko
2020-01-14 15:44   ` Jon Hunter
2020-01-12 17:29 ` [PATCH v4 07/14] dmaengine: tegra-apb: Use devm_request_irq Dmitry Osipenko
2020-01-14 15:44   ` Jon Hunter
2020-01-12 17:30 ` [PATCH v4 08/14] dmaengine: tegra-apb: Fix coding style problems Dmitry Osipenko
2020-01-15  9:49   ` Jon Hunter
2020-01-16 17:37     ` Dmitry Osipenko
2020-01-12 17:30 ` [PATCH v4 09/14] dmaengine: tegra-apb: Clean up runtime PM teardown Dmitry Osipenko
2020-01-15  9:57   ` Jon Hunter
2020-01-16 17:18     ` Dmitry Osipenko [this message]
2020-01-12 17:30 ` [PATCH v4 10/14] dmaengine: tegra-apb: Keep clock enabled only during of DMA transfer Dmitry Osipenko
2020-01-15 10:08   ` Jon Hunter
2020-01-16 17:01     ` Dmitry Osipenko
2020-01-12 17:30 ` [PATCH v4 11/14] dmaengine: tegra-apb: Clean up suspend-resume Dmitry Osipenko
2020-01-12 17:30 ` [PATCH v4 12/14] dmaengine: tegra-apb: Add missing of_dma_controller_free Dmitry Osipenko
2020-01-15 10:10   ` Jon Hunter
2020-01-12 17:30 ` [PATCH v4 13/14] dmaengine: tegra-apb: Allow to compile as a loadable kernel module Dmitry Osipenko
2020-01-15 10:10   ` Jon Hunter
2020-01-12 17:30 ` [PATCH v4 14/14] dmaengine: tegra-apb: Remove MODULE_ALIAS Dmitry Osipenko
2020-01-15 10:11   ` Jon Hunter

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=11cdf32b-23d5-8a4e-0832-3c75e90b8abe@gmail.com \
    --to=digetx@gmail.com \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=jonathanh@nvidia.com \
    --cc=ldewangan@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=mirq-linux@rere.qmqm.pl \
    --cc=thierry.reding@gmail.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org
	public-inbox-index dmaengine

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git