* Re: [PATCH 5/5] clk: tegra: add tegra20 automotive init
[not found] ` <20190207125408.8776-6-kejia.hu@codethink.co.uk>
@ 2019-02-07 13:23 ` Dmitry Osipenko
2019-02-08 10:10 ` Kejia Hu
0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Osipenko @ 2019-02-07 13:23 UTC (permalink / raw)
To: Kejia Hu, linux-tegra, linux-arm-kernel; +Cc: Thomas Preston
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread