linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clk: ux500: Initialize DSI clocks before registering
@ 2019-12-22 20:29 Linus Walleij
  2019-12-22 21:46 ` Linus Walleij
  0 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2019-12-22 20:29 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, Linus Walleij, Ulf Hansson, Stephan Gerhold

The DSI clocks need to be initialized properly and cannot
rely on boot defaults, this is especially important for the
U8420 SYSCLK variant of the PRCMU firmware which requires
that you poke these registers right before trying to use
the HS DSI clock. Some vital setup such as the PRCM_TVCLK_MGT
register will not happen unless we call this making some
displays fail to work on the U8420.

Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/clk/ux500/u8500_of_clk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/ux500/u8500_of_clk.c b/drivers/clk/ux500/u8500_of_clk.c
index 0aedd42fad52..c1043a6b383f 100644
--- a/drivers/clk/ux500/u8500_of_clk.c
+++ b/drivers/clk/ux500/u8500_of_clk.c
@@ -65,6 +65,9 @@ static void u8500_clk_init(struct device_node *np)
 	u32 bases[CLKRST_MAX];
 	int i;
 
+	/* Initialize display clocks */
+	prcmu_set_display_clocks();
+
 	for (i = 0; i < ARRAY_SIZE(bases); i++) {
 		struct resource r;
 
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: ux500: Initialize DSI clocks before registering
  2019-12-22 20:29 [PATCH] clk: ux500: Initialize DSI clocks before registering Linus Walleij
@ 2019-12-22 21:46 ` Linus Walleij
  2019-12-23  9:16   ` Ulf Hansson
  0 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2019-12-22 21:46 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd; +Cc: linux-clk, Ulf Hansson, Stephan Gerhold

On Sun, Dec 22, 2019 at 9:29 PM Linus Walleij <linus.walleij@linaro.org> wrote:

> The DSI clocks need to be initialized properly and cannot
> rely on boot defaults, this is especially important for the
> U8420 SYSCLK variant of the PRCMU firmware which requires
> that you poke these registers right before trying to use
> the HS DSI clock. Some vital setup such as the PRCM_TVCLK_MGT
> register will not happen unless we call this making some
> displays fail to work on the U8420.
>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Sorry I got lost in the macros for CLK_MGT_ENTRY
that does poke these registers including PRCM_TVCLK_MGT.

I need to figure out what is actually going wrong with the
DSI clocks here.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: ux500: Initialize DSI clocks before registering
  2019-12-22 21:46 ` Linus Walleij
@ 2019-12-23  9:16   ` Ulf Hansson
  2019-12-23  9:28     ` Viresh Kumar
  2019-12-23 16:25     ` Linus Walleij
  0 siblings, 2 replies; 6+ messages in thread
From: Ulf Hansson @ 2019-12-23  9:16 UTC (permalink / raw)
  To: Linus Walleij, Viresh Kumar
  Cc: Michael Turquette, Stephen Boyd, linux-clk, Stephan Gerhold

+ Viresh

On Sun, 22 Dec 2019 at 22:46, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Sun, Dec 22, 2019 at 9:29 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> > The DSI clocks need to be initialized properly and cannot
> > rely on boot defaults, this is especially important for the
> > U8420 SYSCLK variant of the PRCMU firmware which requires
> > that you poke these registers right before trying to use
> > the HS DSI clock. Some vital setup such as the PRCM_TVCLK_MGT
> > register will not happen unless we call this making some
> > displays fail to work on the U8420.
> >
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: Stephan Gerhold <stephan@gerhold.net>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> Sorry I got lost in the macros for CLK_MGT_ENTRY
> that does poke these registers including PRCM_TVCLK_MGT.
>
> I need to figure out what is actually going wrong with the
> DSI clocks here.

In the downstream kernel, I authored a patch that added a new per
clock config flag "CLK_BOOT_NOGATE", which was set for DSI clocks and
its friends. If this flag was set, the clock was prevented from being
unprepared/disabled by the common clock core, even if unused.

This was only needed during the boot process, when the bootloaders
have turned on the clock, HW wise. As then those needed to stay on,
else things would break, in one way or the other.

I looked at using CLK_IGNORE_UNUSED, but it doesn't help, when there
is another leaf clock using the same parent as the DSI clock (or
something along those lines). In principle, all "unused" clocks can
become disabled/unprepared, way before the late_initcall
clk_disable_unused() get called - if there is a leaf clock being
disabled.

Finally, to allow clocks that had the new CLK_BOOT_NOGATE flag set, to
be unprepared/disabled after boot, the flag was cleared for all clocks
in the path of the late_initcall path clk_disable_unused().

I can quickly re-base and submit these patches, if you like, but I am
guessing the clk maintainers may object to it, but I don't know.

That said, maybe the suggested "Boot constraint framework" [1], that
has been worked on by Viresh, is the proper solution for this.

Hope this helps!

Kind regards
Uffe

[1]
https://lore.kernel.org/lkml/cover.1519380923.git.viresh.kumar@linaro.org/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: ux500: Initialize DSI clocks before registering
  2019-12-23  9:16   ` Ulf Hansson
@ 2019-12-23  9:28     ` Viresh Kumar
  2019-12-23  9:33       ` Ulf Hansson
  2019-12-23 16:25     ` Linus Walleij
  1 sibling, 1 reply; 6+ messages in thread
From: Viresh Kumar @ 2019-12-23  9:28 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Linus Walleij, Viresh Kumar, Michael Turquette, Stephen Boyd,
	linux-clk, Stephan Gerhold

On 23-12-19, 10:16, Ulf Hansson wrote:
> That said, maybe the suggested "Boot constraint framework" [1], that
> has been worked on by Viresh, is the proper solution for this.

The Boot constraint stuff may get replaced by this series from
Saravana, which is already queued by Greg now.. It should fix this
issue I believe.

https://lore.kernel.org/lkml/20190904211126.47518-1-saravanak@google.com/

-- 
viresh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: ux500: Initialize DSI clocks before registering
  2019-12-23  9:28     ` Viresh Kumar
@ 2019-12-23  9:33       ` Ulf Hansson
  0 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2019-12-23  9:33 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Linus Walleij, Viresh Kumar, Michael Turquette, Stephen Boyd,
	linux-clk, Stephan Gerhold

On Mon, 23 Dec 2019 at 10:28, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 23-12-19, 10:16, Ulf Hansson wrote:
> > That said, maybe the suggested "Boot constraint framework" [1], that
> > has been worked on by Viresh, is the proper solution for this.
>
> The Boot constraint stuff may get replaced by this series from
> Saravana, which is already queued by Greg now.. It should fix this
> issue I believe.
>
> https://lore.kernel.org/lkml/20190904211126.47518-1-saravanak@google.com/

Thanks, I will have a closer look then.

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: ux500: Initialize DSI clocks before registering
  2019-12-23  9:16   ` Ulf Hansson
  2019-12-23  9:28     ` Viresh Kumar
@ 2019-12-23 16:25     ` Linus Walleij
  1 sibling, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-12-23 16:25 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Viresh Kumar, Michael Turquette, Stephen Boyd, linux-clk,
	Stephan Gerhold

On Mon, Dec 23, 2019 at 10:16 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On Sun, 22 Dec 2019 at 22:46, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Sun, Dec 22, 2019 at 9:29 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > > The DSI clocks need to be initialized properly and cannot
> > > rely on boot defaults, this is especially important for the
> > > U8420 SYSCLK variant of the PRCMU firmware which requires
> > > that you poke these registers right before trying to use
> > > the HS DSI clock. Some vital setup such as the PRCM_TVCLK_MGT
> > > register will not happen unless we call this making some
> > > displays fail to work on the U8420.
> > >
> > > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > > Cc: Stephan Gerhold <stephan@gerhold.net>
> > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >
> > Sorry I got lost in the macros for CLK_MGT_ENTRY
> > that does poke these registers including PRCM_TVCLK_MGT.
> >
> > I need to figure out what is actually going wrong with the
> > DSI clocks here.
>
> In the downstream kernel, I authored a patch that added a new per
> clock config flag "CLK_BOOT_NOGATE", which was set for DSI clocks and
> its friends. If this flag was set, the clock was prevented from being
> unprepared/disabled by the common clock core, even if unused.
>
> This was only needed during the boot process, when the bootloaders
> have turned on the clock, HW wise. As then those needed to stay on,
> else things would break, in one way or the other.

What I'm seeing after some tests is that the boot loader
(splash screen) sets PRCMU_DSI0ESCCLK to 9600000
(9.6MHz) and if I try to switch that to 19.2 MHz which the
component should be fine with (and a vendor kernel
is using it) things bug up.

A workaround that works is to set the clock to 9.6MHz
so it matches what the boot loader splash set it to,
the display works fine if I do this.

From what I can see the problem is maybe that the clock
framework is trying to re-initialize a PRCMU clock that was
already turned on, but I haven't looked closer.

Do you think that it could possibly be that the problem isn't
really that of disabling the clock, but actually that the code
re-enables a clock that is already enabled?

I.e. to switch the frequency of this clock we need to go
through a disable/set_rate/enable sequence, but as it is
now, we will issue set_rate() on a clock that is already
enabled, just that the framework isn't aware that is is
already enabled?

I.e. we would need something like regulator-boot-on
but for clocks, clock-boot-on;?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-12-23 16:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-22 20:29 [PATCH] clk: ux500: Initialize DSI clocks before registering Linus Walleij
2019-12-22 21:46 ` Linus Walleij
2019-12-23  9:16   ` Ulf Hansson
2019-12-23  9:28     ` Viresh Kumar
2019-12-23  9:33       ` Ulf Hansson
2019-12-23 16:25     ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).