[PATCH] dmaengine: pl330: set subsys_initcall level

[PATCH] dmaengine: pl330: set subsys_initcall level

[Bumyong Lee]

module_amba_driver is macro for module_init/exit
module_init is device_initcall level when it configured
with built-in driver.

pl330 is dmaengine driver. because slave drivers depend on
dmaengine drivers, dmaengine drivers is more appropriate
subsys_initcall.

Signed-off-by: Bumyong Lee <bumyong.lee@samsung.com>
---
 drivers/dma/pl330.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 3cf0b38387ae..4970830d7ab0 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -3270,7 +3270,17 @@ static struct amba_driver pl330_driver = {
 	.remove = pl330_remove,
 };
 
-module_amba_driver(pl330_driver);
+static int __init pl330_init(void)
+{
+	return amba_driver_register(&pl330_driver);
+}
+subsys_initcall(pl330_init);
+
+static void __exit pl330_exit(void)
+{
+	amba_driver_unregister(&pl330_driver);
+}
+module_exit(pl330_exit);
 
 MODULE_AUTHOR("Jaswinder Singh <jassisinghbrar@gmail.com>");
 MODULE_DESCRIPTION("API Driver for PL330 DMAC");
-- 
2.42.0

Re: [PATCH] dmaengine: pl330: set subsys_initcall level

[Krzysztof Kozlowski]

On 31/10/2023 04:48, Bumyong Lee wrote:
> module_amba_driver is macro for module_init/exit
> module_init is device_initcall level when it configured
> with built-in driver.
> 
> pl330 is dmaengine driver. because slave drivers depend on
> dmaengine drivers, dmaengine drivers is more appropriate
> subsys_initcall.

The same is true for all resource providers and we do not manually order
them via initcalls. Sorry, this was fine as is. Implement defer for your
drivers, not hack initcalls. If you upstreamed them, then it could even
work out of the box for you .

Best regards,
Krzysztof

RE: [PATCH] dmaengine: pl330: set subsys_initcall level

[bumyong.lee]

> On 31/10/2023 04:48, Bumyong Lee wrote:
> > module_amba_driver is macro for module_init/exit module_init is
> > device_initcall level when it configured with built-in driver.
> >
> > pl330 is dmaengine driver. because slave drivers depend on dmaengine
> > drivers, dmaengine drivers is more appropriate subsys_initcall.
> 
> The same is true for all resource providers and we do not manually order
> them via initcalls. Sorry, this was fine as is. Implement defer for your
> drivers, not hack initcalls. If you upstreamed them, then it could even
> work out of the box for you .
> 
> Best regards,
> Krzysztof

I agree with your opinion that the drivers using dma-engine should implement to defer probe when dma is not initialized yet in their probe function execution.
But if dma-engine driver and slave driver is the same initcall level, then a lot of slave drivers should defer probe every time of boot.

I think it's better to use subsys_initcall for pl330 like other dmaengine drivers regardless of slave driver's implementation in order to reduce defer operations.

Best regards
Bumyong Lee

Re: [PATCH] dmaengine: pl330: set subsys_initcall level

[Krzysztof Kozlowski]

On 09/11/2023 05:34, bumyong.lee wrote:
>> On 31/10/2023 04:48, Bumyong Lee wrote:
>>> module_amba_driver is macro for module_init/exit module_init is
>>> device_initcall level when it configured with built-in driver.
>>>
>>> pl330 is dmaengine driver. because slave drivers depend on dmaengine
>>> drivers, dmaengine drivers is more appropriate subsys_initcall.
>>
>> The same is true for all resource providers and we do not manually order
>> them via initcalls. Sorry, this was fine as is. Implement defer for your
>> drivers, not hack initcalls. If you upstreamed them, then it could even
>> work out of the box for you .
>>
>> Best regards,
>> Krzysztof
> 
> I agree with your opinion that the drivers using dma-engine should implement to defer probe when dma is not initialized yet in their probe function execution.

Please wrap your emails to match mailing list discussion style.

> But if dma-engine driver and slave driver is the same initcall level, then a lot of slave drivers should defer probe every time of boot.

Which and how many drivers have this problem?

> 
> I think it's better to use subsys_initcall for pl330 like other dmaengine drivers regardless of slave driver's implementation in order to reduce defer operations.

pl330 requires other resources, so manual ordering via initcalls leads
to other problems. You solve one, create others. And no analysis of
practical impact was provided in commit msg.

Best regards,
Krzysztof