[PATCH] dw_dmac: Replace subsys_init() with arch_initcall()

[PATCH] dw_dmac: Replace subsys_init() with arch_initcall()

[Viresh Kumar]

In some cases users of dw_dmac, amba-pl022, are initialized before dw_dmac, and
if they try to use dw_dmac, they simply fail. So its better we register init()
routine of driver using arch_initcall() instead of subsys_init(), so that dma
driver is available at the earliest possible.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/dma/dw_dmac.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 54d72a8..4b580e7 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1566,7 +1566,7 @@ static int __init dw_init(void)
 {
 	return platform_driver_probe(&dw_driver, dw_probe);
 }
-subsys_initcall(dw_init);
+arch_initcall(dw_init);
 
 static void __exit dw_exit(void)
 {
-- 
1.7.2.2

Re: [PATCH] dw_dmac: Replace subsys_init() with arch_initcall()

[Koul, Vinod]

On Mon, 2011-05-16 at 10:20 +0530, Viresh Kumar wrote:
> In some cases users of dw_dmac, amba-pl022, are initialized before dw_dmac, and
> if they try to use dw_dmac, they simply fail. So its better we register init()
> routine of driver using arch_initcall() instead of subsys_init(), so that dma
> driver is available at the earliest possible.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  drivers/dma/dw_dmac.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 54d72a8..4b580e7 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -1566,7 +1566,7 @@ static int __init dw_init(void)
>  {
>  	return platform_driver_probe(&dw_driver, dw_probe);
>  }
> -subsys_initcall(dw_init);
> +arch_initcall(dw_init);
arch_init is usually kept for subystem initialization.
Why cant you move the amba-pl022 init to module_init, that should take
care of this.

-- 
~Vinod

Re: [PATCH] dw_dmac: Replace subsys_init() with arch_initcall()

[viresh kumar]

On 05/16/2011 09:36 PM, Koul, Vinod wrote:
> On Mon, 2011-05-16 at 10:20 +0530, Viresh Kumar wrote:
>> In some cases users of dw_dmac, amba-pl022, are initialized before dw_dmac, and
>> if they try to use dw_dmac, they simply fail. So its better we register init()
>> routine of driver using arch_initcall() instead of subsys_init(), so that dma
>> driver is available at the earliest possible.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>> ---
>>  drivers/dma/dw_dmac.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
>> index 54d72a8..4b580e7 100644
>> --- a/drivers/dma/dw_dmac.c
>> +++ b/drivers/dma/dw_dmac.c
>> @@ -1566,7 +1566,7 @@ static int __init dw_init(void)
>>  {
>>  	return platform_driver_probe(&dw_driver, dw_probe);
>>  }
>> -subsys_initcall(dw_init);
>> +arch_initcall(dw_init);
> arch_init is usually kept for subystem initialization.
> Why cant you move the amba-pl022 init to module_init, that should take
> care of this.
> 

Actually, amba-pl022 init was moved to subsys_initcall earlier with
following patch:

commit 25c8e03bdb769dfe2381f8b7942f05b0eb4bdf31
Author: Linus Walleij <linus.walleij@stericsson.com>
Date:   Mon Sep 6 11:02:12 2010 +0200

    spi/pl022: move probe call to subsys_initcall()
    
    The PL022 SPI bus is sometimes used for early stuff like
    regulators that need to be present at module_init() time, so
    we move this to a subsys_initcall().
    
    Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
    Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/spi/amba-pl022.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

And so i moved dma's init to arch_initcall. Also the similar approach is
taken by drivers/dma/ste_dma40.c

Also, maybe it makes more sense for dma to be up at the earliest.
Isn't it?

-- 
viresh

Re: [PATCH] dw_dmac: Replace subsys_init() with arch_initcall()

[Koul, Vinod]

On Tue, 2011-05-17 at 09:36 +0530, viresh kumar wrote:
> On 05/16/2011 09:36 PM, Koul, Vinod wrote:
> > On Mon, 2011-05-16 at 10:20 +0530, Viresh Kumar wrote:
> >> In some cases users of dw_dmac, amba-pl022, are initialized before dw_dmac, and
> >> if they try to use dw_dmac, they simply fail. So its better we register init()
> >> routine of driver using arch_initcall() instead of subsys_init(), so that dma
> >> driver is available at the earliest possible.
> >>
> >> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> >> ---
> >>  drivers/dma/dw_dmac.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> >> index 54d72a8..4b580e7 100644
> >> --- a/drivers/dma/dw_dmac.c
> >> +++ b/drivers/dma/dw_dmac.c
> >> @@ -1566,7 +1566,7 @@ static int __init dw_init(void)
> >>  {
> >>  	return platform_driver_probe(&dw_driver, dw_probe);
> >>  }
> >> -subsys_initcall(dw_init);
> >> +arch_initcall(dw_init);
> > arch_init is usually kept for subystem initialization.
> > Why cant you move the amba-pl022 init to module_init, that should take
> > care of this.
> > 
> 
> Actually, amba-pl022 init was moved to subsys_initcall earlier with
> following patch:
> 
> And so i moved dma's init to arch_initcall. Also the similar approach is
> taken by drivers/dma/ste_dma40.c
> 
> Also, maybe it makes more sense for dma to be up at the earliest.
> Isn't it?

As I said before arch init is supposed to used for initialization of the
subsystems not for a driver init.
You need to ensure that with given init levels and makefile order your
dependencies are properly resolved, rather than this approach.

Like move whatever dependent on amba to late_init and amba to module
init, that would be advisable approach rather than above.


HTH
-- 
~Vinod

Re: [PATCH] dw_dmac: Replace subsys_init() with arch_initcall()

[viresh kumar]

On 05/17/2011 09:13 AM, Koul, Vinod wrote:
> On Tue, 2011-05-17 at 09:36 +0530, viresh kumar wrote:
>> On 05/16/2011 09:36 PM, Koul, Vinod wrote:
>>> On Mon, 2011-05-16 at 10:20 +0530, Viresh Kumar wrote:
>>>> In some cases users of dw_dmac, amba-pl022, are initialized before dw_dmac, and
>>>> if they try to use dw_dmac, they simply fail. So its better we register init()
>>>> routine of driver using arch_initcall() instead of subsys_init(), so that dma
>>>> driver is available at the earliest possible.
>>>>
>>>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>>>> ---
>>>>  drivers/dma/dw_dmac.c |    2 +-
>>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
>>>> index 54d72a8..4b580e7 100644
>>>> --- a/drivers/dma/dw_dmac.c
>>>> +++ b/drivers/dma/dw_dmac.c
>>>> @@ -1566,7 +1566,7 @@ static int __init dw_init(void)
>>>>  {
>>>>  	return platform_driver_probe(&dw_driver, dw_probe);
>>>>  }
>>>> -subsys_initcall(dw_init);
>>>> +arch_initcall(dw_init);
>>> arch_init is usually kept for subystem initialization.
>>> Why cant you move the amba-pl022 init to module_init, that should take
>>> care of this.
>>>
>>
>> Actually, amba-pl022 init was moved to subsys_initcall earlier with
>> following patch:
>>
>> And so i moved dma's init to arch_initcall. Also the similar approach is
>> taken by drivers/dma/ste_dma40.c
>>
>> Also, maybe it makes more sense for dma to be up at the earliest.
>> Isn't it?
> 
> As I said before arch init is supposed to used for initialization of the
> subsystems not for a driver init.
> You need to ensure that with given init levels and makefile order your
> dependencies are properly resolved, rather than this approach.
> 
> Like move whatever dependent on amba to late_init and amba to module
> init, that would be advisable approach rather than above.
> 

As i said earlier, with following patch from Linus:

commit 25c8e03bdb769dfe2381f8b7942f05b0eb4bdf31
Author: Linus Walleij <linus.walleij@stericsson.com>
Date:   Mon Sep 6 11:02:12 2010 +0200

    spi/pl022: move probe call to subsys_initcall()
    
    The PL022 SPI bus is sometimes used for early stuff like
    regulators that need to be present at module_init() time, so
    we move this to a subsys_initcall().
    
    Signed-off-by: Linus Walleij <linus.walleij@stericsson.com>
    Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 drivers/spi/amba-pl022.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

SPI bus is required for regulators and so probably he can't have
module_init() for amba-pl022. And so, DMA has nothing more than
arch_initcall().

@Linus: What do you say? Can we get back to module_init() for amba-pl022?
And subsys_initcall() for DMA.

-- 
viresh

Re: [PATCH] dw_dmac: Replace subsys_init() with arch_initcall()

[Linus Walleij]

2011/5/17 viresh kumar <viresh.kumar@st.com>:

> SPI bus is required for regulators and so probably he can't have
> module_init() for amba-pl022.

Nope. Regulators have to be at subsystem_init() because lots
of  drivers called in module_init() == driver_init() will need
to enable them to even be able to talk to the hardware they
are regulating.

e.g. an externally powered I2C device. If you cannot put power
to it you cannot probe it.

Then you end up in a dependency: we need to have SPI
to enble the regulators - the SPI device containing the
regulators is a device that is self-powered.

So then of course the SPI bus controller has to be
available, so it has to be subsys_initcall() as well.
Luckily as it happens it is earlier in the linkfile than
regulators, so using the same initlevel as regulators
will work (else we would be in hell).

But since the PL022 uses DMA, we have to have DMA
before SPI, and it is currently linked *after* SPI in the
Makefile.

> @Linus: What do you say? Can we get back to module_init() for amba-pl022?
> And subsys_initcall() for DMA.

No, but I sent a patch that will make it possible to have both
as subsys_init(), check it out, it seems to work for me.

Can you test it? (Sorry forgot to add you on CC, title is
"[PATCH] dmaengine: move link order".

Yours,
Linus Walleij

Re: [PATCH] dw_dmac: Replace subsys_init() with arch_initcall()

[viresh kumar]

On 05/18/2011 05:51 PM, Linus Walleij wrote:
>> > @Linus: What do you say? Can we get back to module_init() for amba-pl022?
>> > And subsys_initcall() for DMA.
> No, but I sent a patch that will make it possible to have both
> as subsys_init(), check it out, it seems to work for me.
> 
> Can you test it? (Sorry forgot to add you on CC, title is
> "[PATCH] dmaengine: move link order".

That patch looks good to me. So with that maybe this patch will no
longer be required.

-- 
viresh

Re: [PATCH] dw_dmac: Replace subsys_init() with arch_initcall()

[Koul, Vinod]

On Thu, 2011-05-19 at 10:51 +0530, viresh kumar wrote:
> On 05/18/2011 05:51 PM, Linus Walleij wrote:
> >> > @Linus: What do you say? Can we get back to module_init() for amba-pl022?
> >> > And subsys_initcall() for DMA.
> > No, but I sent a patch that will make it possible to have both
> > as subsys_init(), check it out, it seems to work for me.
I am okay with this approach...

Linus, though I am still not convinced why st-dma would need to be
arch_init?

> > 
> > Can you test it? (Sorry forgot to add you on CC, title is
> > "[PATCH] dmaengine: move link order".
> 
> That patch looks good to me. So with that maybe this patch will no
> longer be required.
> 

-- 
~Vinod

Re: [PATCH] dw_dmac: Replace subsys_init() with arch_initcall()

[viresh kumar]

On 05/19/2011 04:53 PM, Koul, Vinod wrote:
> On Thu, 2011-05-19 at 10:51 +0530, viresh kumar wrote:
>> On 05/18/2011 05:51 PM, Linus Walleij wrote:
>>>>> @Linus: What do you say? Can we get back to module_init() for amba-pl022?
>>>>> And subsys_initcall() for DMA.
>>> No, but I sent a patch that will make it possible to have both
>>> as subsys_init(), check it out, it seems to work for me.
> I am okay with this approach...
> 
> Linus, though I am still not convinced why st-dma would need to be
> arch_init?
> 

I think you missed this. In Linus's patch following is also done.

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index fab68a5..7d3e93d 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2951,4 +2951,4 @@ int __init stedma40_init(void)
 {
        return platform_driver_probe(&d40_driver, d40_probe);
 }
-arch_initcall(stedma40_init);
+subsys_initcall(stedma40_init);

-- 
viresh