[PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Omar Ramirez Luna]

Given that dm timer framework doesn't support request of clocks
by soft | hard irqs because some recent changes, tidspbridge needs
to request its clocks on init and enable/disable them on demand.

This was first seen on 3.2-rc1.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/staging/tidspbridge/core/dsp-clock.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/dsp-clock.c b/drivers/staging/tidspbridge/core/dsp-clock.c
index 3d1279c..7eb5617 100644
--- a/drivers/staging/tidspbridge/core/dsp-clock.c
+++ b/drivers/staging/tidspbridge/core/dsp-clock.c
@@ -54,6 +54,7 @@
 
 /* Bridge GPT id (1 - 4), DM Timer id (5 - 8) */
 #define DMT_ID(id) ((id) + 4)
+#define DM_TIMER_CLOCKS		4
 
 /* Bridge MCBSP id (6 - 10), OMAP Mcbsp id (0 - 4) */
 #define MCBSP_ID(id) ((id) - 6)
@@ -114,8 +115,13 @@ static s8 get_clk_type(u8 id)
  */
 void dsp_clk_exit(void)
 {
+	int i;
+
 	dsp_clock_disable_all(dsp_clocks);
 
+	for (i = 0; i < DM_TIMER_CLOCKS; i++)
+		omap_dm_timer_free(timer[i]);
+
 	clk_put(iva2_clk);
 	clk_put(ssi.sst_fck);
 	clk_put(ssi.ssr_fck);
@@ -130,9 +136,13 @@ void dsp_clk_exit(void)
 void dsp_clk_init(void)
 {
 	static struct platform_device dspbridge_device;
+	int i, id;
 
 	dspbridge_device.dev.bus = &platform_bus_type;
 
+	for (i = 0, id = 5; i < DM_TIMER_CLOCKS; i++, id++)
+		timer[i] = omap_dm_timer_request_specific(id);
+
 	iva2_clk = clk_get(&dspbridge_device.dev, "iva2_ck");
 	if (IS_ERR(iva2_clk))
 		dev_err(bridge, "failed to get iva2 clock %p\n", iva2_clk);
@@ -204,8 +214,7 @@ int dsp_clk_enable(enum dsp_clk_id clk_id)
 		clk_enable(iva2_clk);
 		break;
 	case GPT_CLK:
-		timer[clk_id - 1] =
-				omap_dm_timer_request_specific(DMT_ID(clk_id));
+		status = omap_dm_timer_start(timer[clk_id - 1]);
 		break;
 #ifdef CONFIG_OMAP_MCBSP
 	case MCBSP_CLK:
@@ -281,7 +290,7 @@ int dsp_clk_disable(enum dsp_clk_id clk_id)
 		clk_disable(iva2_clk);
 		break;
 	case GPT_CLK:
-		omap_dm_timer_free(timer[clk_id - 1]);
+		status = omap_dm_timer_stop(timer[clk_id - 1]);
 		break;
 #ifdef CONFIG_OMAP_MCBSP
 	case MCBSP_CLK:
-- 
1.7.0.4

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Felipe Contreras]

On Sat, Nov 19, 2011 at 12:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> Given that dm timer framework doesn't support request of clocks
> by soft | hard irqs because some recent changes, tidspbridge needs
> to request its clocks on init and enable/disable them on demand.
>
> This was first seen on 3.2-rc1.
>
> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>

This looks similar similar to this one:
http://article.gmane.org/gmane.linux.ports.arm.omap/61816

Are you sure this is what we want instead of that one?

Cheers.

-- 
Felipe Contreras

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Ramirez Luna, Omar]

On Wed, Dec 7, 2011 at 4:11 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sat, Nov 19, 2011 at 12:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>> Given that dm timer framework doesn't support request of clocks
>> by soft | hard irqs because some recent changes, tidspbridge needs
>> to request its clocks on init and enable/disable them on demand.
>>
>> This was first seen on 3.2-rc1.
>>
>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>
> This looks similar similar to this one:
> http://article.gmane.org/gmane.linux.ports.arm.omap/61816
>
> Are you sure this is what we want instead of that one?

I believe your patch only takes care of gpt5 and gpt6, but not of the
other 2 that could be requested by the dsp, let's say gpt8 for avsync.

As for this patch, dm timer framework should support request on soft
or hard irq[1], but it can't because the underlying functions to use
clk_get->clk_get_sys which holds a mutex.

I believe your error was seen because of patches not in mainline, but
the concept between the patches is similar indeed, however mine:

- Handles the other gpt that can be requested by the dsp.
- Uses start/stop according to future plans of making enable/disable static.
- Might be temporal if *dm_timer_request* functions are fixed and
there is an overall feeling that we can revert to use
request+start/stop+free on request.

Regards,
Omar,
---
[1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg58665.html

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Felipe Contreras]

On Thu, Dec 8, 2011 at 9:10 PM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
> On Wed, Dec 7, 2011 at 4:11 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Sat, Nov 19, 2011 at 12:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>>> Given that dm timer framework doesn't support request of clocks
>>> by soft | hard irqs because some recent changes, tidspbridge needs
>>> to request its clocks on init and enable/disable them on demand.
>>>
>>> This was first seen on 3.2-rc1.
>>>
>>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>>
>> This looks similar similar to this one:
>> http://article.gmane.org/gmane.linux.ports.arm.omap/61816
>>
>> Are you sure this is what we want instead of that one?
>
> I believe your patch only takes care of gpt5 and gpt6, but not of the
> other 2 that could be requested by the dsp, let's say gpt8 for avsync.

I'm not really sure about that... Judging from the code, the only
timers needed used to be initialized on bridge_brd_start based on the
symbols of the baseimage. If you say that's not enough, then I guess
that's fine (not really surprising that the old code missed that).

> As for this patch, dm timer framework should support request on soft
> or hard irq[1], but it can't because the underlying functions to use
> clk_get->clk_get_sys which holds a mutex.
>
> I believe your error was seen because of patches not in mainline, but
> the concept between the patches is similar indeed, however mine:

Yes, it's basically the same, the only difference is that I'm
initializing only the clocks that are supposed to be needed by the
baseimage.

> - Handles the other gpt that can be requested by the dsp.

And I guess which clocks are going to be used is not discoverable somehow...

> - Uses start/stop according to future plans of making enable/disable static.

Yeah, but that's easy to change s/enable/start/ s/disable/stop/

> - Might be temporal if *dm_timer_request* functions are fixed and
> there is an overall feeling that we can revert to use
> request+start/stop+free on request.

At which point we would end up with something similar to my patch :)

Anyway, I guess there's not much problem in requesting extra clocks
that we would not use.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Felipe Contreras]

On Sat, Nov 19, 2011 at 12:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> Given that dm timer framework doesn't support request of clocks
> by soft | hard irqs because some recent changes, tidspbridge needs
> to request its clocks on init and enable/disable them on demand.
>
> This was first seen on 3.2-rc1.
>
> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>

Tested-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>

Something the commit message didn't mention is that this fixes a nasty
continuous loop that happens as soon as you load the module.
Definitely 3.2-fix material IMO :)

Cheers.

-- 
Felipe Contreras

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Ramirez Luna, Omar]

On Thu, Dec 8, 2011 at 1:50 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, Dec 8, 2011 at 9:10 PM, Ramirez Luna, Omar <omar.ramirez@ti.com> wrote:
>> On Wed, Dec 7, 2011 at 4:11 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> On Sat, Nov 19, 2011 at 12:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>>>> Given that dm timer framework doesn't support request of clocks
>>>> by soft | hard irqs because some recent changes, tidspbridge needs
>>>> to request its clocks on init and enable/disable them on demand.
>>>>
>>>> This was first seen on 3.2-rc1.
>>>>
>>>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>>>
>>> This looks similar similar to this one:
>>> http://article.gmane.org/gmane.linux.ports.arm.omap/61816
>>>
>>> Are you sure this is what we want instead of that one?
>>
>> I believe your patch only takes care of gpt5 and gpt6, but not of the
>> other 2 that could be requested by the dsp, let's say gpt8 for avsync.
>
> I'm not really sure about that... Judging from the code, the only
> timers needed used to be initialized on bridge_brd_start based on the
> symbols of the baseimage. If you say that's not enough, then I guess
> that's fine (not really surprising that the old code missed that).

I meant the dsp by itself can generate that request. That is why it is
not explicitly seen in the code.

>> - Handles the other gpt that can be requested by the dsp.
>
> And I guess which clocks are going to be used is not discoverable somehow...

It is basically the pool of clocks of the dsp is formed by mcbsp, gpt,
wdt, ssi. The dsp can request any gpt from 5 to 8 according to the
binary you are running on the dsp side. That's the reason it is
somewhat hidden what clock the dsp needs.

>> - Uses start/stop according to future plans of making enable/disable static.
>
> Yeah, but that's easy to change s/enable/start/ s/disable/stop/

Agree

>> - Might be temporal if *dm_timer_request* functions are fixed and
>> there is an overall feeling that we can revert to use
>> request+start/stop+free on request.
>
> At which point we would end up with something similar to my patch :)

Yes, plus handling the other clocks that can be available to the dsp.

> Anyway, I guess there's not much problem in requesting extra clocks
> that we would not use.

AV playback does use gpt8, but for systems that doesn't use gpt8 it is
best to use the old approach of just request/free prior to dm timer
changes.

Regards,

Omar

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Ramirez Luna, Omar]

On Thu, Dec 8, 2011 at 2:11 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sat, Nov 19, 2011 at 12:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
>> Given that dm timer framework doesn't support request of clocks
>> by soft | hard irqs because some recent changes, tidspbridge needs
>> to request its clocks on init and enable/disable them on demand.
>>
>> This was first seen on 3.2-rc1.
>>
>> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>
> Tested-by: Felipe Contreras <felipe.contreras@gmail.com>
> Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>

Thanks!

> Something the commit message didn't mention is that this fixes a nasty
> continuous loop that happens as soon as you load the module.

Indeed.

> Definitely 3.2-fix material IMO :)

I believe it was pushed to staging-next... I'm rooting so it can be
pushed to staging-linus :)

Regards,

Omar

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Greg KH]

On Thu, Dec 08, 2011 at 02:16:10PM -0600, Ramirez Luna, Omar wrote:
> On Thu, Dec 8, 2011 at 2:11 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> > On Sat, Nov 19, 2011 at 12:18 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> >> Given that dm timer framework doesn't support request of clocks
> >> by soft | hard irqs because some recent changes, tidspbridge needs
> >> to request its clocks on init and enable/disable them on demand.
> >>
> >> This was first seen on 3.2-rc1.
> >>
> >> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
> >
> > Tested-by: Felipe Contreras <felipe.contreras@gmail.com>
> > Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>
> 
> Thanks!
> 
> > Something the commit message didn't mention is that this fixes a nasty
> > continuous loop that happens as soon as you load the module.
> 
> Indeed.
> 
> > Definitely 3.2-fix material IMO :)
> 
> I believe it was pushed to staging-next... I'm rooting so it can be
> pushed to staging-linus :)

As it wasn't originally stated that this was a problem that needed to be
fixed for 3.2, can it wait for 3.3-rc1?  Especially given that there is
still discussion about this?

thanks,

greg k-h

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Felipe Contreras]

On Fri, Dec 9, 2011 at 2:07 AM, Greg KH <greg@kroah.com> wrote:
> On Thu, Dec 08, 2011 at 02:16:10PM -0600, Ramirez Luna, Omar wrote:
>> On Thu, Dec 8, 2011 at 2:11 PM, Felipe Contreras
>> > Definitely 3.2-fix material IMO :)
>>
>> I believe it was pushed to staging-next... I'm rooting so it can be
>> pushed to staging-linus :)
>
> As it wasn't originally stated that this was a problem that needed to be
> fixed for 3.2, can it wait for 3.3-rc1?  Especially given that there is
> still discussion about this?

Well, if you want the bridgedriver to hang the whole system in 3.2,
which I don't think is the case. There's no more discussion, at least
not from my side, and I don't think anybody has raised any more
issues. There's really no reason not to merge this patch, as it really
cannot get worst than the current situation, unless the patch makes
the device explode, or something like that, which is not the case :)

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Ramirez Luna, Omar]

On Thu, Dec 8, 2011 at 6:07 PM, Greg KH <greg@kroah.com> wrote:
>> > Definitely 3.2-fix material IMO :)
>>
>> I believe it was pushed to staging-next... I'm rooting so it can be
>> pushed to staging-linus :)
>
> As it wasn't originally stated that this was a problem that needed to be
> fixed for 3.2, can it wait for 3.3-rc1?  Especially given that there is
> still discussion about this?

No discussion from me either.

The original intention was to include this patch for 3.2 because of
the recent changes in dm timer framework, I apologize if "This was
first seen on 3.2-rc1." is too ambiguous. I hope this can make it for
3.2 though.

Thanks,

Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Greg KH]

On Thu, Dec 08, 2011 at 07:44:28PM -0600, Ramirez Luna, Omar wrote:
> On Thu, Dec 8, 2011 at 6:07 PM, Greg KH <greg@kroah.com> wrote:
> >> > Definitely 3.2-fix material IMO :)
> >>
> >> I believe it was pushed to staging-next... I'm rooting so it can be
> >> pushed to staging-linus :)
> >
> > As it wasn't originally stated that this was a problem that needed to be
> > fixed for 3.2, can it wait for 3.3-rc1?  Especially given that there is
> > still discussion about this?
> 
> No discussion from me either.
> 
> The original intention was to include this patch for 3.2 because of
> the recent changes in dm timer framework, I apologize if "This was
> first seen on 3.2-rc1." is too ambiguous. I hope this can make it for
> 3.2 though.

How about for 3.2.1?  I'll try to get this into 3.2, but I can't
guarantee anything so late in the release cycle, sorry.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] staging: tidspbridge: request dmtimer clocks on init

[Ramirez Luna, Omar]

On Fri, Dec 9, 2011 at 4:38 PM, Greg KH <greg@kroah.com> wrote:
> How about for 3.2.1?  I'll try to get this into 3.2, but I can't
> guarantee anything so late in the release cycle, sorry.

Sounds fair, fingers crossed for 3.2 ;)

Thanks,

Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html