Messages in this thread | | | Date | Tue, 7 May 2024 14:37:37 +0200 | Subject | Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support | From | Amelie Delaunay <> |
| |
Hi Christophe,
Thanks for the review.
On 5/4/24 16:27, Christophe JAILLET wrote: > Le 23/04/2024 à 14:32, Amelie Delaunay a écrit : >> STM32 DMA3 driver supports the 3 hardware configurations of the STM32 >> DMA3 >> controller: >> - LPDMA (Low Power): 4 channels, no FIFO >> - GPDMA (General Purpose): 16 channels, FIFO from 8 to 32 bytes >> - HPDMA (High Performance): 16 channels, FIFO from 8 to 256 bytes >> Hardware configuration of the channels is retrieved from the hardware >> configuration registers. >> The client can specify its channel requirements through device tree. >> STM32 DMA3 channels can be individually reserved either because they are >> secure, or dedicated to another CPU. >> Indeed, channels availability depends on Resource Isolation Framework >> (RIF) configuration. RIF grants access to buses with Compartiment ID >> (CIF) filtering, secure and privilege level. It also assigns DMA channels >> to one or several processors. >> DMA channels used by Linux should be CID-filtered and statically assigned >> to CID1 or shared with other CPUs but using semaphore. In case CID >> filtering is not configured, dma-channel-mask property can be used to >> specify available DMA channels to the kernel, otherwise such channels >> will be marked as reserved and can't be used by Linux. >> >> Signed-off-by: Amelie Delaunay >> <amelie.delaunay-rj0Iel/JR4NBDgjK7y7TUQ@public.gmane.org> >> --- > > ... > >> + pm_runtime_set_active(&pdev->dev); >> + pm_runtime_enable(&pdev->dev); >> + pm_runtime_get_noresume(&pdev->dev); >> + pm_runtime_put(&pdev->dev); >> + >> + dev_info(&pdev->dev, "STM32 DMA3 registered rev:%lu.%lu\n", >> + FIELD_GET(VERR_MAJREV, verr), FIELD_GET(VERR_MINREV, verr)); >> + >> + return 0; >> + >> +err_clk_disable: >> + clk_disable_unprepare(ddata->clk); >> + >> + return ret; >> +} >> + >> +static void stm32_dma3_remove(struct platform_device *pdev) >> +{ >> + pm_runtime_disable(&pdev->dev); > > Hi, > > missing clk_disable_unprepare(ddata->clk);? > > as in the error handling path on the probe just above? > > CJ >
Clock is entirely managed by pm_runtime, except in error path of probe since pm_runtime is enabled only at the very end. Clock is enabled with pm_runtime_resume_and_get() when a channel is requested or when an asynchronous register access occurs (filter_fn, debugfs, runtime_resume) and clock is disabled with pm_runtime_put_sync() when releasing a channel or at the end of asynchronous register access (filter_fn, debugfs, runtime_suspend). Adding clk_disable_unprepare(ddata->clk); here leads to clock already disabled/unprepared warnings in drivers/clk/clk.c clk_core_disable()/clk_core_unprepare().
>> +} > > ... >
Regards, Amelie
| |