linux-kernel - Re: [PATCH v2 2/4] dmaengine: Add STM32 DMA driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOAejn1XqvZDmCgGr5rmZoz2EzFUyxtmpSm0_atLr81rSJrEcw@mail.gmail.com>
Date:	Wed, 14 Oct 2015 15:41:58 +0200
From:	"M'boumba Cedric Madianga" <cedric.madianga@...il.com>
To:	Daniel Thompson <daniel.thompson@...aro.org>
Cc:	Maxime Coquelin <mcoquelin.stm32@...il.com>, robh+dt@...nel.org,
	pawel.moll@....com, Mark Rutland <mark.rutland@....com>,
	ijc+devicetree@...lion.org.uk, Kumar Gala <galak@...eaurora.org>,
	linux@....linux.org.uk, Vinod Koul <vinod.koul@...el.com>,
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, dmaengine@...r.kernel.org
Subject: Re: [PATCH v2 2/4] dmaengine: Add STM32 DMA driver

2015-10-14 15:29 GMT+02:00 Daniel Thompson <daniel.thompson@...aro.org>:
> On 14/10/15 14:17, M'boumba Cedric Madianga wrote:
>>
>> Hi Daniel,
>>
>>>>> +
>>>>> +static int stm32_dma_remove(struct platform_device *pdev)
>>>>> +{
>>>>> +       struct stm32_dma_device *dmadev = platform_get_drvdata(pdev);
>>>>> +
>>>>> +       of_dma_controller_free(pdev->dev.of_node);
>>>>> +
>>>>> +       dma_async_device_unregister(&dmadev->ddev);
>>>>> +
>>>>> +       clk_disable_unprepare(dmadev->clk);
>>>>
>>>>
>>>>
>>>> What is the purpose of disabling/unpreparing the clock here?
>>>> stm32_dma_alloc_chan_resources() and stm32_dma_free_chan_resources()
>>>> should
>>>> pair up and the clock should already be stopped.
>>
>>
>> stm32_dma_remove() could be called during an on-going transfer during
>> module unload.
>> So in that case, it seems that disabling/unpreparing the clock is needed.
>
>
> Really?
>
> I think we need to be sure any on-going transfers are stopped.
>
> There are multiple reasons for this, not least the risk of executing a
> callback that has been freed, but the one related to my point is that a
> single clk_disable_unprepare() will remain broken because if you don't know
> that the transfers have stopped then you don't know how many on-going
> transfers there are.

In the next version, I am going to stop the DMA, free all interrupts
and kill tasklet in stm32_dma_remove.
In that way, all on-going transfers will be lost as we don't have to
wait the end of remaining transfer in order to execute this function
as quickly as possible.
But even with this improvement, I think I have to disable the clock here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/