On Wed, 2008-12-10 at 10:44 +0200, Högander Jouni wrote:
> 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.
But the ST_DSS_IDLE doesn't help here. The kernel crashes after enabling
dss_ick, before your code can even enable dss_fck. The same happens if
you do it other way around, first dss_fck and then dss_ick.
> > 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.
Doesn't omap2_clk_wait_ready alread check this?
And again, the problem is not accessing DSS registers. The problem is
that clk_enable(dss_ick) crashes. so the following code crashes also:
static int __init test_init(void)
{
struct clk *c1 = clk_get(NULL, "dss_ick");
while(1) {
clk_enable(c1);
clk_disable(c1);
}
return 0;
}
>
> +
> + 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
> >
> >
>
--
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
