LKML Archive on
help / color / mirror / Atom feed
From: Marcel Ziswiler <>
To: "" <>,
	"" <>
Cc: "" <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>,
	"" <>
Subject: Re: [PATCH] ARM: tegra: fix ulpi regression on tegra20
Date: Mon, 23 Apr 2018 22:05:44 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hi Dmitry

On Fri, 2018-04-20 at 13:50 +0300, Dmitry Osipenko wrote:
> ...
> I managed to find CDEV clocks in TRM this time.

And where exactly in which TRM? In all my attempts at finding anything
CDEV2 is just a pad group which includes the DAP_MCLK2 pad in question.

> Seems assigning CDEV2 clock to
> "ulpi-link" was correct

Sorry, but I do really not see how you can get to any such conclusion.

Whatever that CDEV2 clock exactly is. Its offset 93 points towards the
at bit position 29 reading "Enable clock to DEV2 pad". While the TRM
misses further documenting what exactly that DEV2 pad should be I guess
it may point towards our suspected DAP_MCLK2. So for some reason
besides PLL_P_OUT4 which is what that pad is actually muxed to also
some additional DEV2 pad clock needs enabling. In addition there would
be a CLK_RST_CONTROLLER_MISC_CLK_ENB_0 register where one could also
specify a DEV2_OSC_DIV_SEL but I would assume that one only applies if
the pad in question being muxed to OSC which is not the case at least
in all device trees I have looked at.

> and both CDEV2 and PLL_P_OUT4 should be enabled,

Agreed, basically the DAP_MCLK2 pad seems gated by an additional enable
called CLK_ENB_DEV2_OUT.

> should gate the PLL_P_OUT4 that feeds USB HW and PLL_P_OUT4 should be
> always-enabled because it is enabled by init_table, but apparently it
> is getting
> disabled erroneously.

At least that was the case back when I had trouble getting ULPI to work
on T20. Strangely latest -next right now does no longer seem to suffer
from that same issue even if my patch is reverted but as mentioned
before nobody stops the required PLL_P_OUT4 which is what is actually
driving DAP_MCLK2 to not be changed behind the scenes breaking the
whole thing again.

> Marcel, could you please revert your patch, add
> "trace_event=clk_enable,clk_disable,clk_set_parent tp_printk" to
> kernels cmdline
> and post the log?

Yes, the only difference in those traces is whether or not that non-
existent CDEV2 clock is enabled or not:

[    1.897521] clk_enable: cdev2_fixed
[    1.901008] clk_enable: cdev2

I also do have an explanation why it kept working in my case. Probably
the boot ROM or U-Boot is already setting that bit.

> It looks like there is some clk framework bug,

My conclusion is that there should be a DAP_MCLK2 clock which is gated
by that CLK_ENB_DEV2_OUT but really outputs a T20 internal clock
according to its pin muxing which if set to OSC may be further divided
down by DEV2_OSC_DIV_SEL.

> but just in case please also try
> to apply this patch:
> diff --git a/drivers/clk/tegra/clk-tegra-periph.c
> b/drivers/clk/tegra/clk-tegra-periph.c
> index 2acba2986bc6..407bd0c0ac2f 100644
> --- a/drivers/clk/tegra/clk-tegra-periph.c
> +++ b/drivers/clk/tegra/clk-tegra-periph.c
> @@ -1024,7 +1024,7 @@ static void __init init_pllp(void __iomem
> *clk_base, void
> __iomem *pmc_base,
>                 if (dt_clk) {
>                         clk =
> tegra_clk_register_pll_out("pll_p_out4",
>                                         "pll_p_out4_div", clk_base +
> -                                       17, 16, CLK_IGNORE_UNUSED |
> +                                       17, 16, CLK_IS_CRITICAL |
>                                         CLK_SET_RATE_PARENT, 0,
>                                         &PLLP_OUTB_lock);
>                         *dt_clk = clk;

I did not have to do any such but maybe that would at least prevent any
further issues on PLL_P_OUT4. However I still believe this is kind of
wrong as PLL_P_OUT4 is what drives DAP_MCLK2 in its current muxing
which is connected to the ULPI transceivers REFCLK pin.

BTW: That pin is usually to be driven at 24 MHz and not 26 MHz which
CDEV2 claims to be at.

What do you think?



  reply	other threads:[~2018-04-23 22:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-19 15:12 Marcel Ziswiler
2018-04-20  8:52 ` Marc Dietrich
2018-04-20 10:50   ` Dmitry Osipenko
2018-04-23 22:05     ` Marcel Ziswiler [this message]
2018-04-24 14:38       ` Dmitry Osipenko
2018-04-26 11:39         ` Peter De Schrijver
2018-04-23 15:42   ` Marcel Ziswiler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH] ARM: tegra: fix ulpi regression on tegra20' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).