Re: Bug in linux omap clock framework?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tomi Valkeinen <tomi.valkeinen@xxxxxxxxx> writes:

> On Wed, 2008-12-10 at 09:37 +0200, Högander Jouni wrote:
>> "ext Tomi Valkeinen" <tomi.valkeinen@xxxxxxxxx> writes:
>> 
>> > Hi,
>> >
>> > On Mon, 2008-12-08 at 02:24 -0700, ext Paul Walmsley wrote:
>> >> Hi Tomi,
>> >> 
>> >> On Mon, 8 Dec 2008, Tomi Valkeinen wrote:
>> >> 
>> >> > On Sat, 2008-12-06 at 16:51 -0700, ext Paul Walmsley wrote:
>> >> > > Hi Tomi,
>> >> > > 
>> >> > > nice test case.
>> >> > > 
>> >> > > On Fri, 5 Dec 2008, Tomi.Valkeinen@xxxxxxxxx wrote:
>> >> > > 
>> >> > > > I have had strange clk_enable() crashes with DSS2, and now I managed to
>> >> > > > isolate it. With the included patch, on OMAP3 SDP board, with default
>> >> > > > kernel config, I always get the crash below. In this example case it
>> >> > > > happens at specific time on boot, but with DSS2 it happens randomly at
>> >> > > > runtime, when I enable the DSS clocks.
>> >> > > 
>> >> 
>> >> At this point my guess would be that it is a DSS driver problem, but I 
>> >> don't think it is clear yet.
>> >> 
>> >
>> > I don't know much about power and clock domains, but I believe I found
>> > the reason and fix for this. At least this should point to the right
>> > direction.
>> >
>> > It looks to me that when I do clk_enable() to a clock which is inside a
>> > turned off powerdomain, clk_enable() function will return before the
>> > powerdomain is fully turned on. Reading PM_PWSTST_DSS shows that the
>> > powerdomain is still in transition state.
>> >
>> > The following change waits until the powerdomain has finished the
>> > transition. At least it fixed the problem for me, and other things seem
>> > to be still working =).
>> >
>> >
>> > diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
>> > index fa62f14..f713d0b 100644
>> > --- a/arch/arm/mach-omap2/clockdomain.c
>> > +++ b/arch/arm/mach-omap2/clockdomain.c
>> > @@ -567,6 +567,8 @@ int omap2_clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
>> >         else
>> >                 omap2_clkdm_wakeup(clkdm);
>> >  
>> > +       pwrdm_wait_transition(clkdm->pwrdm.ptr);
>> > +
>> 
>> Altough this patch is fixing the problem, I'll guess it's because of
>> delay it causes rather than waiting for
>> PM_PWSTST_DSS. omap2_clkdm_clk_enable alone doesn't switch pwrdm to
>> on. This is because clockdomain/powerdomains are controlled by HW (hw
>> supervised).
>> 
>> I think the right answer is to use ST_*_IDLE & ST_*_STDBY bits in
>> omap2_clk_wait_ready.
>
> But ST_DSS_IDLE says active only after both interface and functional
> clock are on, so it doesn't help here.

Yes and in ST_DSS_IDLE description it is saying that if it's "0x1
Display sub-system is in idle mode and cannot be accessed". So it only
way to get it out of idle is to enable both clocks it needs to be done.

If you look at arch/arm/mach-omap2/clock.c:omap2_clk_wait_ready it is
actually checking that both clocks are enabled.

>
> Why is it wrong to wait for PM_PWSTST_DSS? Do you mean that this crash
> is not caused by the DSS being not powered on, but by something else,
> and so the small delay just accidentally fixes the problem?

Something like that.

If you look at your code:

+static int __init omapfb_init(void)
+{
+	struct clk *c1, *c2, *c3;
+
+	c1 = clk_get(NULL, "dss_ick");
+	c2 = clk_get(NULL, "dss1_alwon_fck");
+	c3 = clk_get(NULL, "dss_tv_fck");
+
+	base = ioremap(DISPC_BASE, SZ_1K);
+	printk("mapped to %p\n", base);
+
+	clk_enable(c1);

At this point ST_DSS_IDLE should not be yet checked (because fck is
not enabled yet.

+	clk_enable(c2);

But at this point (actually inside clk_enable and
omap2_clk_wait_ready) ST_DSS_IDLE should be checked before those
register accesses in next two lines. You can verify my comment by
adding "while(ST_DSS_IDLE);" here.

+
+	dispc_write_reg(DISPC_IRQENABLE, 0);
+	dispc_write_reg(DISPC_IRQSTATUS, 0);
+
+	clk_disable(c1);
+	clk_disable(c2);
+
+	printk("usecounts %d, %d, %d\n", clk_get_usecount(c1),
+			clk_get_usecount(c2),
+			clk_get_usecount(c3));
+
+	/*clk_enable(c2);*/
+	/*clk_enable(c3);*/
+
+	while(1) {
+		clk_enable(c1);
+		clk_disable(c1);
+	}
+
+	return 0;
+}
+
+module_init(omapfb_init);
+MODULE_LICENSE("GPL");
>  Tomi
>
>

-- 
Jouni Högander

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

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux