[5/5] clk: tegra: add tegra20 automotive init

From: Thomas Preston <thomas.preston@codethink.co.uk>

Add intialisation for the tegra20 automotive parts
which have different initialisation requirements than
the tegra20. These have been copied from the 2.6 BSP
supplied by nvidia for these parts.

Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
Signed-off-by: Kejia Hu <kejia.hu@codethink.co.uk>
---
 drivers/clk/tegra/clk-tegra20.c | 78 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 2 deletions(-)

07.02.2019 15:54, Kejia Hu пишет:
> From: Thomas Preston <thomas.preston@codethink.co.uk>
> 
> Add intialisation for the tegra20 automotive parts
> which have different initialisation requirements than
> the tegra20. These have been copied from the 2.6 BSP
> supplied by nvidia for these parts.
> 
> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
> Signed-off-by: Kejia Hu <kejia.hu@codethink.co.uk>
> ---
>  drivers/clk/tegra/clk-tegra20.c | 78 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
> index c71b61162a32..1b8c7d40fd55 100644
> --- a/drivers/clk/tegra/clk-tegra20.c
> +++ b/drivers/clk/tegra/clk-tegra20.c
> @@ -22,6 +22,7 @@
>  #include <linux/clk/tegra.h>
>  #include <linux/delay.h>
>  #include <dt-bindings/clock/tegra20-car.h>
> +#include <soc/tegra/common.h>
>  
>  #include "clk.h"
>  #include "clk-id.h"
> @@ -534,7 +535,7 @@ static struct tegra_clk tegra20_clks[tegra_clk_max] __initdata = {
>  	[tegra_clk_sdmmc4] = { .dt_id = TEGRA20_CLK_SDMMC4, .present = true },
>  	[tegra_clk_la] = { .dt_id = TEGRA20_CLK_LA, .present = true },
>  	[tegra_clk_csite] = { .dt_id = TEGRA20_CLK_CSITE, .present = true },
> -	[tegra_clk_vfir] = { .dt_id = TEGRA20_CLK_VFIR, .present = true },
> +	[tegra_clk_vfir] = { .dt_id = TEGRA20_CLK_VFIR, .present = false },
>  	[tegra_clk_mipi] = { .dt_id = TEGRA20_CLK_MIPI, .present = true },
>  	[tegra_clk_nor] = { .dt_id = TEGRA20_CLK_NOR, .present = true },
>  	[tegra_clk_rtc] = { .dt_id = TEGRA20_CLK_RTC, .present = true },
> @@ -1091,9 +1092,82 @@ static struct tegra_clk_init_table init_table[] __initdata = {
>  	{ TEGRA20_CLK_CLK_MAX, TEGRA20_CLK_CLK_MAX, 0, 0 },
>  };
>  
> +/* Tegra20 automotive part initialisation table.
> + *
> + * These values have been derived from the Nvidia supplied BSP for their
> + * automotive parts.
> + */
> +
> +static struct tegra_clk_init_table init_table_t20a[] __initdata = {
> +	{TEGRA20_CLK_PLL_P, TEGRA20_CLK_CLK_MAX, 216000000, 1},
> +	{TEGRA20_CLK_PLL_P_OUT1, TEGRA20_CLK_CLK_MAX, 28800000, 1},
> +	{TEGRA20_CLK_PLL_P_OUT2, TEGRA20_CLK_CLK_MAX, 48000000, 1},
> +	{TEGRA20_CLK_PLL_P_OUT3, TEGRA20_CLK_CLK_MAX, 72000000, 1},
> +	{TEGRA20_CLK_PLL_P_OUT4, TEGRA20_CLK_CLK_MAX, 240000000, 1},
> +	{TEGRA20_CLK_PLL_M, TEGRA20_CLK_CLK_MAX, 666000000, 1 },
> +	{TEGRA20_CLK_PLL_M_OUT1, TEGRA20_CLK_CLK_MAX, 266400000, 1 },
> +	{TEGRA20_CLK_PLL_C, TEGRA20_CLK_CLK_MAX, 600000000, 1},
> +	{TEGRA20_CLK_PLL_C_OUT1, TEGRA20_CLK_CLK_MAX, 240000000, 1},
> +	{TEGRA20_CLK_SCLK, TEGRA20_CLK_PLL_C_OUT1, 0, 1},
> +	{TEGRA20_CLK_HCLK, TEGRA20_CLK_CLK_MAX, 0, 1},
> +	{TEGRA20_CLK_PCLK, TEGRA20_CLK_CLK_MAX, 120000000, 1},
> +	{TEGRA20_CLK_CSITE, TEGRA20_CLK_CLK_MAX, 0, 1},
> +	{TEGRA20_CLK_EMC, TEGRA20_CLK_CLK_MAX, 0, 1},
> +	{TEGRA20_CLK_PLL_D, TEGRA20_CLK_CLK_MAX, 594000000, 1},
> +	{TEGRA20_CLK_PLL_D_OUT0, TEGRA20_CLK_CLK_MAX, 594000000/2, 1},
> +	{TEGRA20_CLK_CCLK, TEGRA20_CLK_CLK_MAX, 0, 1},
> +	{TEGRA20_CLK_UARTA, TEGRA20_CLK_PLL_P, 0, 0},
> +	{TEGRA20_CLK_UARTB, TEGRA20_CLK_PLL_P, 216000000, 1},
> +	{TEGRA20_CLK_UARTC, TEGRA20_CLK_PLL_P, 0, 0},
> +	{TEGRA20_CLK_UARTD, TEGRA20_CLK_PLL_P, 0, 0},
> +	{TEGRA20_CLK_UARTE, TEGRA20_CLK_PLL_P, 0, 0},
> +	{TEGRA20_CLK_PLL_A, TEGRA20_CLK_CLK_MAX, 56448000, 1},
> +	{TEGRA20_CLK_PLL_A_OUT0, TEGRA20_CLK_CLK_MAX, 11289600, 1},
> +	{TEGRA20_CLK_BLINK, TEGRA20_CLK_CLK_MAX, 32768, 0},
> +	{TEGRA20_CLK_I2S1, TEGRA20_CLK_PLL_A_OUT0, 11289600, 0},
> +	{TEGRA20_CLK_I2S2, TEGRA20_CLK_PLL_A_OUT0, 11289600, 0},
> +	{TEGRA20_CLK_SDMMC1, TEGRA20_CLK_PLL_P, 48000000, 0},
> +	{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},
> +	{TEGRA20_CLK_SBC1, TEGRA20_CLK_PLL_C, 12000000, 0},
> +	{TEGRA20_CLK_SBC2, TEGRA20_CLK_PLL_C, 12000000, 0},
> +	{TEGRA20_CLK_SBC3, TEGRA20_CLK_PLL_C, 12000000, 0},
> +	{TEGRA20_CLK_SBC4, TEGRA20_CLK_PLL_C, 12000000, 0},
> +	{TEGRA20_CLK_HOST1X, TEGRA20_CLK_PLL_M, 266400000, 1},
> +	{TEGRA20_CLK_CSUS, TEGRA20_CLK_CLK_MAX, 12000000, 1},
> +	{TEGRA20_CLK_DISP1, TEGRA20_CLK_PLL_D_OUT0, 297000000, 1, },    /* disp1 from pll_d (no div) */
> +	{TEGRA20_CLK_GR2D, TEGRA20_CLK_CLK_MAX, 300000000, 1},
> +	{TEGRA20_CLK_GR3D, TEGRA20_CLK_CLK_MAX, 300000000, 1},
> +	{TEGRA20_CLK_VI, TEGRA20_CLK_PLL_M, 111000000, 1 },
> +	{TEGRA20_CLK_VI_SENSOR, TEGRA20_CLK_PLL_M, 111000000, 1 },
> +	{TEGRA20_CLK_I2C1, TEGRA20_CLK_PLL_P, 3000000, 0 },
> +	{TEGRA20_CLK_I2C2, TEGRA20_CLK_PLL_P, 3000000, 0 },
> +	{TEGRA20_CLK_I2C3, TEGRA20_CLK_PLL_P, 3000000, 0 },
> +	{TEGRA20_CLK_DVC, TEGRA20_CLK_PLL_P, 3000000, 0 },
> +	{TEGRA20_CLK_PWM, TEGRA20_CLK_CLK_32K, 32768, 0 },
> +	{TEGRA20_CLK_EPP, TEGRA20_CLK_PLL_M, 266400000, 1 },
> +	{TEGRA20_CLK_MPE, TEGRA20_CLK_PLL_C, 300000000, 1},
> +	{TEGRA20_CLK_NDFLASH, TEGRA20_CLK_PLL_P, 86500000, 1 },
> +	{TEGRA20_CLK_VDE, TEGRA20_CLK_PLL_C, 300000000, 0 },
> +	{TEGRA20_CLK_SPDIF_IN, TEGRA20_CLK_PLL_M, 22200000, 0 },
> +	{TEGRA20_CLK_SPDIF_OUT, TEGRA20_CLK_PLL_A_OUT0, 5644800, 0 },
> +	{TEGRA20_CLK_PLL_E, TEGRA20_CLK_CLK_MAX, 1200000000, 0 },
> +	{TEGRA20_CLK_CLK_MAX, TEGRA20_CLK_CLK_MAX, 0, 0}, /* This MUST be the last entry */
> +};
> +
> +
>  static void __init tegra20_clock_apply_init_table(void)
>  {
> -	tegra_init_from_table(init_table, clks, TEGRA20_CLK_CLK_MAX);
> +	struct tegra_clk_init_table *table = init_table;
> +
> +	if (soc_is_tegra_auto()) {
> +		pr_info("Initialise Tegra Automotive clocks\n");
> +		table = init_table_t20a;
> +	}
> +
> +	tegra_init_from_table(table, clks, TEGRA20_CLK_CLK_MAX);
>  }
>  
>  /*
> 

Hi, 

Quick comment.. this all looks very generic and much more board-specific than automotive. Nowadays device-tree has support for specifying clock parents and rates per board DT, see "Assigned clock parents and rates" of [0]. Hence it looks like there is no real need to hardcode those values in the driver. Please take a look at the assigned-clocks.

[0] https://www.kernel.org/doc/Documentation/devicetree/bindings/clock/clock-bindings.txt

On Thu, 2019-02-07 at 16:23 +0300, Dmitry Osipenko wrote:
> this all looks very generic and much more board-specific than automotive

Hi,

Thanks for your reply.

We got the clock data from nvidia pdk for the automotive which didn't
set the clocks up in the same way as commercial parts, and we were told
that there are certain clock restrictions for the automotive parts that
the commercial ones do not have.

Applying different clock configs for each nodes doesn't seems to be a
good way, as they will be too scattered in the device tree. We proposed
to add a new root node for the clock originally[1], and further
discussion suggested us to distinguish the variation by reading the SKU
info, thus came this patch. We can discuss further though.

Regards

[1] https://www.spinics.net/lists/arm-kernel/msg665344.html

08.02.2019 13:10, Kejia Hu пишет:
> On Thu, 2019-02-07 at 16:23 +0300, Dmitry Osipenko wrote:
>> this all looks very generic and much more board-specific than automotive
> 
> Hi,
> 
> Thanks for your reply.
> 
> We got the clock data from nvidia pdk for the automotive which didn't
> set the clocks up in the same way as commercial parts, and we were told
> that there are certain clock restrictions for the automotive parts that
> the commercial ones do not have.
> 
> Applying different clock configs for each nodes doesn't seems to be a
> good way, as they will be too scattered in the device tree. We proposed
> to add a new root node for the clock originally[1], and further
> discussion suggested us to distinguish the variation by reading the SKU
> info, thus came this patch. We can discuss further though.
> 
> Regards
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg665344.html
> 

I don't see any problems with specifying clock configs for each node in the device tree. It also looks to me that the clock configs you're suggesting in this series are only relevant for a one specific board (or maybe family). You could move out the assigned-clock configs into a separate DTSI file that will be shared by all of "automotive" boards.

Maybe it will make sense to have a custom init-table to avoid some of the fatal clock configurations during of early boot of the kernel. But maybe in this case we could just limit those vital clocks to a saner-common value?