[PATCH 1/3] clk: tegra: Do not warn unnecessarily

[PATCH 1/3] clk: tegra: Do not warn unnecessarily

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

There is no need to warn if the reference PLL is enabled with the
correct defaults. Only warn if the boot values don't match the defaults.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/clk/tegra/clk-tegra210.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index e1ba62d2b1a0..4904ac4a75db 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -984,8 +984,6 @@ static void tegra210_pllre_set_defaults(struct tegra_clk_pll *pllre)
 	pllre->params->defaults_set = true;
 
 	if (val & PLL_ENABLE) {
-		pr_warn("PLL_RE already enabled. Postponing set full defaults\n");
-
 		/*
 		 * PLL is ON: check if defaults already set, then set those
 		 * that can be updated in flight.
@@ -1012,6 +1010,9 @@ static void tegra210_pllre_set_defaults(struct tegra_clk_pll *pllre)
 		writel_relaxed(val, clk_base + pllre->params->ext_misc_reg[0]);
 		udelay(1);
 
+		if (!pllre->params->defaults_set)
+			pr_warn("PLL_RE already enabled. Postponing set full defaults\n");
+
 		return;
 	}
 
-- 
2.21.0

[PATCH 2/3] clk: tegra: Warn if an enabled PLL is in IDDQ

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

A PLL in IDDQ doesn't work, whether it's enabled or not. This is not a
configuration that makes sense, so warn about it.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/clk/tegra/clk-tegra210.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index 4904ac4a75db..793c7acaf4e2 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -1003,8 +1003,12 @@ static void tegra210_pllre_set_defaults(struct tegra_clk_pll *pllre)
 		_pll_misc_chk_default(clk_base, pllre->params, 0, val,
 				~mask & PLLRE_MISC0_WRITE_MASK);
 
-		/* Enable lock detect */
+		/* The PLL doesn't work if it's in IDDQ. */
 		val = readl_relaxed(clk_base + pllre->params->ext_misc_reg[0]);
+		if (val & PLLRE_MISC0_IDDQ)
+			pr_warn("unexpected IDDQ bit set for enabled clock\n");
+
+		/* Enable lock detect */
 		val &= ~mask;
 		val |= PLLRE_MISC0_DEFAULT_VALUE & mask;
 		writel_relaxed(val, clk_base + pllre->params->ext_misc_reg[0]);
-- 
2.21.0

Re: [PATCH 2/3] clk: tegra: Warn if an enabled PLL is in IDDQ

[Jon Hunter]

On 13/06/2019 17:12, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> A PLL in IDDQ doesn't work, whether it's enabled or not. This is not a
> configuration that makes sense, so warn about it.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/clk/tegra/clk-tegra210.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index 4904ac4a75db..793c7acaf4e2 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -1003,8 +1003,12 @@ static void tegra210_pllre_set_defaults(struct tegra_clk_pll *pllre)
>  		_pll_misc_chk_default(clk_base, pllre->params, 0, val,
>  				~mask & PLLRE_MISC0_WRITE_MASK);
>  
> -		/* Enable lock detect */
> +		/* The PLL doesn't work if it's in IDDQ. */
>  		val = readl_relaxed(clk_base + pllre->params->ext_misc_reg[0]);
> +		if (val & PLLRE_MISC0_IDDQ)
> +			pr_warn("unexpected IDDQ bit set for enabled clock\n");
> +
> +		/* Enable lock detect */
>  		val &= ~mask;
>  		val |= PLLRE_MISC0_DEFAULT_VALUE & mask;
>  		writel_relaxed(val, clk_base + pllre->params->ext_misc_reg[0]);

Looks like there are a few other instances of this that could be fixed
as well.

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH 2/3] clk: tegra: Warn if an enabled PLL is in IDDQ

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]

On Mon, Jun 17, 2019 at 11:39:48AM +0100, Jon Hunter wrote:
> 
> On 13/06/2019 17:12, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > A PLL in IDDQ doesn't work, whether it's enabled or not. This is not a
> > configuration that makes sense, so warn about it.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/clk/tegra/clk-tegra210.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> > index 4904ac4a75db..793c7acaf4e2 100644
> > --- a/drivers/clk/tegra/clk-tegra210.c
> > +++ b/drivers/clk/tegra/clk-tegra210.c
> > @@ -1003,8 +1003,12 @@ static void tegra210_pllre_set_defaults(struct tegra_clk_pll *pllre)
> >  		_pll_misc_chk_default(clk_base, pllre->params, 0, val,
> >  				~mask & PLLRE_MISC0_WRITE_MASK);
> >  
> > -		/* Enable lock detect */
> > +		/* The PLL doesn't work if it's in IDDQ. */
> >  		val = readl_relaxed(clk_base + pllre->params->ext_misc_reg[0]);
> > +		if (val & PLLRE_MISC0_IDDQ)
> > +			pr_warn("unexpected IDDQ bit set for enabled clock\n");
> > +
> > +		/* Enable lock detect */
> >  		val &= ~mask;
> >  		val |= PLLRE_MISC0_DEFAULT_VALUE & mask;
> >  		writel_relaxed(val, clk_base + pllre->params->ext_misc_reg[0]);
> 
> Looks like there are a few other instances of this that could be fixed
> as well.

Yeah. This is the only one that's showing up in logs, so I was
concentrating on this one. I'll make another pass over the driver and
clean up the rest of them in a separate patch.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3] clk: tegra: Warn if an enabled PLL is in IDDQ

[Stephen Boyd]

Quoting Thierry Reding (2019-06-13 09:12:24)
> From: Thierry Reding <treding@nvidia.com>
> 
> A PLL in IDDQ doesn't work, whether it's enabled or not. This is not a
> configuration that makes sense, so warn about it.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---

Applied to clk-next

[PATCH 3/3] clk: tegra: Do not enable PLL_RE_VCO on Tegra210

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

It turns out that this PLL is not used on Tegra210, so there's no need
to enable it via the init table. Remove the init table entry for this
PLL to avoid it getting enabled at boot time. If the bootloader enabled
it and forgot to turn it off, the common clock framework will now know
to disable it because it is unused.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/clk/tegra/clk-tegra210.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index 793c7acaf4e2..13537e4458b5 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -3337,7 +3337,6 @@ static struct tegra_clk_init_table init_table[] __initdata = {
 	{ TEGRA210_CLK_DFLL_SOC, TEGRA210_CLK_PLL_P, 51000000, 1 },
 	{ TEGRA210_CLK_DFLL_REF, TEGRA210_CLK_PLL_P, 51000000, 1 },
 	{ TEGRA210_CLK_SBC4, TEGRA210_CLK_PLL_P, 12000000, 1 },
-	{ TEGRA210_CLK_PLL_RE_VCO, TEGRA210_CLK_CLK_MAX, 672000000, 1 },
 	{ TEGRA210_CLK_XUSB_GATE, TEGRA210_CLK_CLK_MAX, 0, 1 },
 	{ TEGRA210_CLK_XUSB_SS_SRC, TEGRA210_CLK_PLL_U_480M, 120000000, 0 },
 	{ TEGRA210_CLK_XUSB_FS_SRC, TEGRA210_CLK_PLL_U_48M, 48000000, 0 },
-- 
2.21.0

Re: [PATCH 3/3] clk: tegra: Do not enable PLL_RE_VCO on Tegra210

[Jon Hunter]

On 13/06/2019 17:12, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> It turns out that this PLL is not used on Tegra210, so there's no need
> to enable it via the init table. Remove the init table entry for this
> PLL to avoid it getting enabled at boot time. If the bootloader enabled
> it and forgot to turn it off, the common clock framework will now know
> to disable it because it is unused.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/clk/tegra/clk-tegra210.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index 793c7acaf4e2..13537e4458b5 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -3337,7 +3337,6 @@ static struct tegra_clk_init_table init_table[] __initdata = {
>  	{ TEGRA210_CLK_DFLL_SOC, TEGRA210_CLK_PLL_P, 51000000, 1 },
>  	{ TEGRA210_CLK_DFLL_REF, TEGRA210_CLK_PLL_P, 51000000, 1 },
>  	{ TEGRA210_CLK_SBC4, TEGRA210_CLK_PLL_P, 12000000, 1 },
> -	{ TEGRA210_CLK_PLL_RE_VCO, TEGRA210_CLK_CLK_MAX, 672000000, 1 },
>  	{ TEGRA210_CLK_XUSB_GATE, TEGRA210_CLK_CLK_MAX, 0, 1 },
>  	{ TEGRA210_CLK_XUSB_SS_SRC, TEGRA210_CLK_PLL_U_480M, 120000000, 0 },
>  	{ TEGRA210_CLK_XUSB_FS_SRC, TEGRA210_CLK_PLL_U_48M, 48000000, 0 },

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH 3/3] clk: tegra: Do not enable PLL_RE_VCO on Tegra210

[Stephen Boyd]

Quoting Thierry Reding (2019-06-13 09:12:25)
> From: Thierry Reding <treding@nvidia.com>
> 
> It turns out that this PLL is not used on Tegra210, so there's no need
> to enable it via the init table. Remove the init table entry for this
> PLL to avoid it getting enabled at boot time. If the bootloader enabled
> it and forgot to turn it off, the common clock framework will now know
> to disable it because it is unused.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---

Applied to clk-next

Re: [PATCH 1/3] clk: tegra: Do not warn unnecessarily

[Jon Hunter]

On 13/06/2019 17:12, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> There is no need to warn if the reference PLL is enabled with the
> correct defaults. Only warn if the boot values don't match the defaults.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/clk/tegra/clk-tegra210.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index e1ba62d2b1a0..4904ac4a75db 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -984,8 +984,6 @@ static void tegra210_pllre_set_defaults(struct tegra_clk_pll *pllre)
>  	pllre->params->defaults_set = true;
>  
>  	if (val & PLL_ENABLE) {
> -		pr_warn("PLL_RE already enabled. Postponing set full defaults\n");
> -
>  		/*
>  		 * PLL is ON: check if defaults already set, then set those
>  		 * that can be updated in flight.
> @@ -1012,6 +1010,9 @@ static void tegra210_pllre_set_defaults(struct tegra_clk_pll *pllre)
>  		writel_relaxed(val, clk_base + pllre->params->ext_misc_reg[0]);
>  		udelay(1);
>  
> +		if (!pllre->params->defaults_set)
> +			pr_warn("PLL_RE already enabled. Postponing set full defaults\n");
> +
>  		return;
>  	}

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic

Re: [PATCH 1/3] clk: tegra: Do not warn unnecessarily

[Stephen Boyd]

Quoting Thierry Reding (2019-06-13 09:12:23)
> From: Thierry Reding <treding@nvidia.com>
> 
> There is no need to warn if the reference PLL is enabled with the
> correct defaults. Only warn if the boot values don't match the defaults.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---

Applied to clk-next