On Mon, Jul 25, 2016 at 02:28:48PM +0200, Mirza Krak wrote:

2016-07-25 13:17 GMT+02:00 Thierry Reding [off-list ref]:

quoted

On Tue, Jul 19, 2016 at 03:36:32PM +0200, Mirza Krak wrote:

quoted

From: Mirza Krak <redacted>

Add TEGRA20_CLK_NOR to init tabel and set a "sane" default rate.

Signed-off-by: Mirza Krak <redacted>
---
 drivers/clk/tegra/clk-tegra20.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 837e5cb..aefc044 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c

@@ -1047,6 +1047,7 @@ static struct tegra_clk_init_table init_table[] __initdata = {
      { TEGRA20_CLK_SDMMC3, TEGRA20_CLK_PLL_P, 48000000, 0 },
      { TEGRA20_CLK_SDMMC4, TEGRA20_CLK_PLL_P, 48000000, 0 },
      { TEGRA20_CLK_SPI, TEGRA20_CLK_PLL_P, 20000000, 0 },
+     { TEGRA20_CLK_NOR, TEGRA20_CLK_PLL_P, 86500000, 0 },

Yay for inconsistent naming in the hardware. It would've been nice if
this clock was called GMI. Oh well...

I am allowed to change clk name? Can do that when I re-spin this series.

No, leave it as-is. The clock is referred to as NOR in the clock and
reset controller. I was just saying that it would've been nice if it had
been named GMI consistently.

quoted

Could you perhaps explain in the commit message why 86.5 MHz is a sane
default? I'm totally unfamiliar with this controller, so maybe it's
self-explanatory, but it seems a rather odd value for a clock frequency.

I used a value that I have in a downstream kernel based on the L4T.
This frequency is well tested and has worked for me, and wanted to
avoid setting it to maximum rate. My guess is that they used 86.5 MHz
because 92 MHz is max rate on Tegra2 and they put it slightly below
that.

What is otherwise recommended when initializing clocks? The rate could
depend on the chip that is attached but otherwise I would like to set
it close to max (or max) rate for performance.

I think we usually set the default rate to the maximum and let drivers
clock them down as they see fit. This is so that things run fast by
default and we often don't have much in the way of power management, at
least not when drivers are first merged. Maximum clock rate ensure that
we get good performance by default, rather than having to rely on
drivers to kick it up a notch.

Thierry