2016-11-07 13:09:41

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 0/6] Add STM32F4 missing clocks

From: Gabriel Fernandez <[email protected]>

This patch-set adds:
- I2S & SAI PLLs
- SDIO & 48 Mhz clocks
- LCD-TFT clock
- I2S & SAI clocks


Gabriel Fernandez (6):
clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards
clk: stm32f4: SDIO & 48Mhz clock management for STM32F469 board
clk: stm32f4: Add post divisor for I2S & SAI PLLs and Add lcd-tft
clock
clk: stm32f4: Add I2S clock
clk: stm32f4: Add SAI clocks
arm: dts: stm32f4: Add external I2S clock

arch/arm/boot/dts/stm32f429.dtsi | 8 +-
drivers/clk/clk-stm32f4.c | 441 +++++++++++++++++++++++++++++++++++++--
2 files changed, 434 insertions(+), 15 deletions(-)

--
1.9.1


2016-11-07 13:08:58

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 2/6] clk: stm32f4: SDIO & 48Mhz clock management for STM32F469 board

From: Gabriel Fernandez <[email protected]>

In the stm32f469 soc, the 48Mhz clock could be derived from pll-q or
from pll-sai-p.

The SDIO clock could be also derived from 48Mhz or from sys clock.

Signed-off-by: Gabriel Fernandez <[email protected]>
---
drivers/clk/clk-stm32f4.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index 7641acd..dda15bc 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -199,7 +199,7 @@ struct stm32f4_gate_data {
{ STM32F4_RCC_APB2ENR, 8, "adc1", "apb2_div" },
{ STM32F4_RCC_APB2ENR, 9, "adc2", "apb2_div" },
{ STM32F4_RCC_APB2ENR, 10, "adc3", "apb2_div" },
- { STM32F4_RCC_APB2ENR, 11, "sdio", "pll48" },
+ { STM32F4_RCC_APB2ENR, 11, "sdio", "sdmux" },
{ STM32F4_RCC_APB2ENR, 12, "spi1", "apb2_div" },
{ STM32F4_RCC_APB2ENR, 13, "spi4", "apb2_div" },
{ STM32F4_RCC_APB2ENR, 14, "syscfg", "apb2_div" },
@@ -940,6 +940,10 @@ static struct clk_hw *stm32_register_cclk(struct device *dev, const char *name,
"no-clock", "lse", "lsi", "hse-rtc"
};

+static const char *pll48_parents[2] = { "pll-q", "pllsai-p" };
+
+static const char *sdmux_parents[2] = { "pll48", "sys" };
+
struct stm32f4_clk_data {
const struct stm32f4_gate_data *gates_data;
const u64 *gates_map;
@@ -1109,6 +1113,18 @@ static void __init stm32f4_rcc_init(struct device_node *np)
goto fail;
}

+ if (of_device_is_compatible(np, "st,stm32f469-rcc")) {
+ clk_hw_register_mux_table(NULL, "pll48",
+ pll48_parents, ARRAY_SIZE(pll48_parents), 0,
+ base + STM32F4_RCC_DCKCFGR, 27, 1, 0, NULL,
+ &stm32f4_clk_lock);
+
+ clk_hw_register_mux_table(NULL, "sdmux",
+ sdmux_parents, ARRAY_SIZE(sdmux_parents), 0,
+ base + STM32F4_RCC_DCKCFGR, 28, 1, 0, NULL,
+ &stm32f4_clk_lock);
+ }
+
of_clk_add_hw_provider(np, stm32f4_rcc_lookup_clk, NULL);
return;
fail:
--
1.9.1

2016-11-07 13:09:19

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 4/6] clk: stm32f4: Add I2S clock

From: Gabriel Fernandez <[email protected]>

This patch introduces I2S clock for stm32f4 soc.
The I2S clock could be derived from an external clock or from pll-i2s

Signed-off-by: Gabriel Fernandez <[email protected]>
---
drivers/clk/clk-stm32f4.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index 5fa5d51..b7cb359 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -216,6 +216,7 @@ enum {
SYSTICK, FCLK, CLK_LSI, CLK_LSE, CLK_HSE_RTC, CLK_RTC,
PLL_VCO_I2S, PLL_VCO_SAI,
CLK_LCD,
+ CLK_I2S,
END_PRIMARY_CLK
};

@@ -967,6 +968,8 @@ static struct clk_hw *stm32_register_cclk(struct device *dev, const char *name,

static const char *sdmux_parents[2] = { "pll48", "sys" };

+static const char *i2s_parents[2] = { "plli2s-r", NULL };
+
struct stm32f4_clk_data {
const struct stm32f4_gate_data *gates_data;
const u64 *gates_map;
@@ -1005,7 +1008,7 @@ struct stm32f4_clk_data {

static void __init stm32f4_rcc_init(struct device_node *np)
{
- const char *hse_clk;
+ const char *hse_clk, *i2s_in_clk;
int n;
const struct of_device_id *match;
const struct stm32f4_clk_data *data;
@@ -1038,6 +1041,7 @@ static void __init stm32f4_rcc_init(struct device_node *np)
stm32f4_gate_map = data->gates_map;

hse_clk = of_clk_get_parent_name(np, 0);
+ i2s_in_clk = of_clk_get_parent_name(np, 1);

clk_register_fixed_rate_with_accuracy(NULL, "hsi", NULL, 0,
16000000, 160000);
@@ -1053,6 +1057,12 @@ static void __init stm32f4_rcc_init(struct device_node *np)
clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll(pllsrc,
&data->pll_data[2], &stm32f4_clk_lock);

+ i2s_parents[1] = i2s_in_clk;
+
+ clks[CLK_I2S] = clk_hw_register_mux_table(NULL, "i2s",
+ i2s_parents, ARRAY_SIZE(i2s_parents), 0,
+ base + STM32F4_RCC_CFGR, 23, 1, 0, NULL,
+ &stm32f4_clk_lock);
sys_parents[1] = hse_clk;
clk_register_mux_table(
NULL, "sys", sys_parents, ARRAY_SIZE(sys_parents), 0,
--
1.9.1

2016-11-07 13:09:39

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 3/6] clk: stm32f4: Add post divisor for I2S & SAI PLLs and Add lcd-tft clock

From: Gabriel Fernandez <[email protected]>

This patch adds post dividers of I2S & SAI PLLs.
These dividers are managed by a dedicated register (RCC_DCKCFGR).
The PLL should be off before a set rate.
This patch also introduces the lcd-tft clock.

Signed-off-by: Gabriel Fernandez <[email protected]>
---
drivers/clk/clk-stm32f4.c | 27 +++++++++++++++++++++++++--
1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index dda15bc..5fa5d51 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -215,6 +215,7 @@ struct stm32f4_gate_data {
enum {
SYSTICK, FCLK, CLK_LSI, CLK_LSE, CLK_HSE_RTC, CLK_RTC,
PLL_VCO_I2S, PLL_VCO_SAI,
+ CLK_LCD,
END_PRIMARY_CLK
};

@@ -599,6 +600,9 @@ static struct clk_hw *clk_register_pll_div(const char *name,
static const struct clk_div_table pll_divp_table[] = {
{ 0, 2 }, { 1, 4 }, { 2, 6 }, { 3, 8 },
};
+static const struct clk_div_table pll_lcd_div_table[] = {
+ { 0, 2 }, { 1, 4 }, { 2, 8 }, { 3, 16 },
+};

/*
* Decode current PLL state and (statically) model the state we inherit from
@@ -659,16 +663,35 @@ static struct clk_hw *stm32f4_rcc_register_pll(const char *pllsrc,
clk_register_pll_div(data->p_name, data->vco_name, 0, reg,
16, 2, 0, pll_divp_table, pll_hw, lock);

- if (data->q_name)
+ if (data->q_name) {
clk_register_pll_div(data->q_name, data->vco_name, 0, reg,
24, 4, CLK_DIVIDER_ONE_BASED, NULL,
pll_hw, lock);

- if (data->r_name)
+ if (data->pll_num == PLL_I2S)
+ clk_register_pll_div("plli2s-q-div", data->q_name,
+ 0, base + STM32F4_RCC_DCKCFGR,
+ 0, 5, 0, NULL, pll_hw, &stm32f4_clk_lock);
+
+ if (data->pll_num == PLL_SAI)
+ clk_register_pll_div("pllsai-q-div", data->q_name,
+ 0, base + STM32F4_RCC_DCKCFGR,
+ 8, 5, 0, NULL, pll_hw, &stm32f4_clk_lock);
+ }
+
+ if (data->r_name) {
clk_register_pll_div(data->r_name, data->vco_name, 0, reg,
28, 3, CLK_DIVIDER_ONE_BASED, NULL, pll_hw,
lock);

+ if (data->pll_num == PLL_SAI)
+ clks[CLK_LCD] = clk_register_pll_div("lcd-tft",
+ data->r_name, CLK_SET_RATE_PARENT,
+ base + STM32F4_RCC_DCKCFGR, 16, 2, 0,
+ pll_lcd_div_table, pll_hw,
+ &stm32f4_clk_lock);
+ }
+
return pll_hw;
}

--
1.9.1

2016-11-07 13:09:36

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards

From: Gabriel Fernandez <[email protected]>

This patch introduces PLL_I2S and PLL_SAI.
Vco clock of these PLLs can be modify by DT (only n multiplicator,
m divider is still fixed by the boot-loader).
Each PLL has 3 dividers. PLL should be off when we modify the rate.

Signed-off-by: Gabriel Fernandez <[email protected]>
---
drivers/clk/clk-stm32f4.c | 371 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 359 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index c2661e2..7641acd 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -28,6 +28,7 @@
#include <linux/regmap.h>
#include <linux/mfd/syscon.h>

+#define STM32F4_RCC_CR 0x00
#define STM32F4_RCC_PLLCFGR 0x04
#define STM32F4_RCC_CFGR 0x08
#define STM32F4_RCC_AHB1ENR 0x30
@@ -37,6 +38,9 @@
#define STM32F4_RCC_APB2ENR 0x44
#define STM32F4_RCC_BDCR 0x70
#define STM32F4_RCC_CSR 0x74
+#define STM32F4_RCC_PLLI2SCFGR 0x84
+#define STM32F4_RCC_PLLSAICFGR 0x88
+#define STM32F4_RCC_DCKCFGR 0x8c

struct stm32f4_gate_data {
u8 offset;
@@ -208,7 +212,11 @@ struct stm32f4_gate_data {
{ STM32F4_RCC_APB2ENR, 26, "ltdc", "apb2_div" },
};

-enum { SYSTICK, FCLK, CLK_LSI, CLK_LSE, CLK_HSE_RTC, CLK_RTC, END_PRIMARY_CLK };
+enum {
+ SYSTICK, FCLK, CLK_LSI, CLK_LSE, CLK_HSE_RTC, CLK_RTC,
+ PLL_VCO_I2S, PLL_VCO_SAI,
+ END_PRIMARY_CLK
+};

/*
* This bitmask tells us which bit offsets (0..192) on STM32F4[23]xxx
@@ -324,23 +332,344 @@ static struct clk *clk_register_apb_mul(struct device *dev, const char *name,
return clk;
}

+enum {
+ PLL,
+ PLL_I2S,
+ PLL_SAI,
+};
+
+struct stm32f4_pll {
+ spinlock_t *lock;
+ struct clk_hw hw;
+ u8 offset;
+ u8 bit_idx;
+ u8 bit_rdy_idx;
+ u8 status;
+ u8 n_start;
+};
+
+struct stm32f4_pll_data {
+ u8 pll_num;
+ u8 n_start;
+ const char *vco_name;
+ const char *p_name;
+ const char *q_name;
+ const char *r_name;
+};
+
+static const struct stm32f4_pll_data stm32f429_pll[] = {
+ { PLL, 192, "vco", "pll", "pll48", NULL, },
+ { PLL_I2S, 192, "vco-i2s", NULL, "plli2s-q", "plli2s-r", },
+ { PLL_SAI, 49, "vco-sai", NULL, "pllsai-q", "pllsai-r", },
+};
+
+static const struct stm32f4_pll_data stm32f469_pll[] = {
+ { PLL, 50, "vco", "pll", "pll-q", NULL, },
+ { PLL_I2S, 50, "vco-i2s", NULL, "plli2s-q", "plli2s-r", },
+ { PLL_SAI, 50, "vco-sai", "pllsai-p", "pllsai-q", "pllsai-r", },
+};
+
+#define to_stm32f4_pll(_hw) container_of(_hw, struct stm32f4_pll, hw)
+
+static int stm32f4_pll_is_enabled(struct clk_hw *hw)
+{
+ struct stm32f4_pll *pll = to_stm32f4_pll(hw);
+
+ return pll->status;
+}
+
+static int __stm32f4_pll_enable(struct clk_hw *hw)
+{
+ struct stm32f4_pll *pll = to_stm32f4_pll(hw);
+ unsigned long reg;
+ int ret = 0;
+
+ if (stm32f4_pll_is_enabled(hw))
+ return 0;
+
+ reg = readl(base + STM32F4_RCC_CR) | (1 << pll->bit_idx);
+ writel(reg, base + STM32F4_RCC_CR);
+
+ ret = readl_relaxed_poll_timeout_atomic(base + STM32F4_RCC_CR, reg,
+ reg & (1 << pll->bit_rdy_idx), 0, 10000);
+
+ pll->status = 1;
+
+ return ret;
+}
+
+static int stm32f4_pll_enable(struct clk_hw *hw)
+{
+ struct stm32f4_pll *pll = to_stm32f4_pll(hw);
+ int ret = 0;
+ unsigned long flags = 0;
+
+ if (pll->lock)
+ spin_lock_irqsave(pll->lock, flags);
+
+ ret = __stm32f4_pll_enable(hw);
+
+ if (pll->lock)
+ spin_unlock_irqrestore(pll->lock, flags);
+
+ return ret;
+}
+
+static void __stm32f4_pll_disable(struct clk_hw *hw)
+{
+ struct stm32f4_pll *pll = to_stm32f4_pll(hw);
+ unsigned long reg;
+
+ reg = readl(base + STM32F4_RCC_CR) & ~(1 << pll->bit_idx);
+
+ writel(reg, base + STM32F4_RCC_CR);
+
+ pll->status = 0;
+}
+
+static void stm32f4_pll_disable(struct clk_hw *hw)
+{
+ struct stm32f4_pll *pll = to_stm32f4_pll(hw);
+ unsigned long flags = 0;
+
+ if (pll->lock)
+ spin_lock_irqsave(pll->lock, flags);
+
+ __stm32f4_pll_disable(hw);
+
+ if (pll->lock)
+ spin_unlock_irqrestore(pll->lock, flags);
+}
+
+static unsigned long stm32f4_pll_recalc(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct stm32f4_pll *pll = to_stm32f4_pll(hw);
+ unsigned long pllcfgr = readl(base + STM32F4_RCC_PLLCFGR);
+ unsigned long pllm = pllcfgr & 0x3f;
+ unsigned long plln = (readl(base + pll->offset) >> 6) & 0x1ff;
+
+ return (parent_rate / pllm) * plln;
+}
+
+static long stm32f4_pll_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ struct stm32f4_pll *pll = to_stm32f4_pll(hw);
+ unsigned long pllcfgr = readl(base + STM32F4_RCC_PLLCFGR);
+ unsigned long m = pllcfgr & 0x3f;
+ unsigned long n;
+
+ n = (rate * m) / *prate;
+
+ if (n < pll->n_start)
+ n = pll->n_start;
+ else if (n > 432)
+ n = 432;
+
+ return (*prate / m) * n;
+}
+
+static int stm32f4_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct stm32f4_pll *pll = to_stm32f4_pll(hw);
+ unsigned long pllcfgr = readl(base + STM32F4_RCC_PLLCFGR);
+ unsigned long m = pllcfgr & 0x3f;
+ unsigned long n;
+ unsigned long val;
+ int pll_state;
+
+ pll_state = stm32f4_pll_is_enabled(hw);
+
+ if (pll_state)
+ stm32f4_pll_disable(hw);
+
+ n = (rate * m) / parent_rate;
+
+ val = readl(base + pll->offset) & ~(0x1ff << 6);
+
+ writel(val | ((n & 0x1ff) << 6), base + pll->offset);
+
+ if (pll_state)
+ stm32f4_pll_enable(hw);
+
+ return 0;
+}
+
+static const struct clk_ops stm32f4_pll_gate_ops = {
+ .enable = stm32f4_pll_enable,
+ .disable = stm32f4_pll_disable,
+ .is_enabled = stm32f4_pll_is_enabled,
+ .recalc_rate = stm32f4_pll_recalc,
+ .round_rate = stm32f4_pll_round_rate,
+ .set_rate = stm32f4_pll_set_rate,
+};
+
+struct stm32f4_pll_div {
+ struct clk_divider div;
+ struct clk_hw *hw_pll;
+};
+
+#define to_pll_div_clk(_div) container_of(_div, struct stm32f4_pll_div, div)
+
+static unsigned long stm32f4_pll_div_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ return clk_divider_ops.recalc_rate(hw, parent_rate);
+}
+
+static long stm32f4_pll_div_round_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long *prate)
+{
+ return clk_divider_ops.round_rate(hw, rate, prate);
+}
+
+static int stm32f4_pll_div_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ int pll_state, ret;
+
+ struct clk_divider *div = to_clk_divider(hw);
+ struct stm32f4_pll_div *pll_div = to_pll_div_clk(div);
+
+ pll_state = stm32f4_pll_is_enabled(pll_div->hw_pll);
+
+ if (pll_state)
+ stm32f4_pll_disable(pll_div->hw_pll);
+
+ ret = clk_divider_ops.set_rate(hw, rate, parent_rate);
+
+ if (pll_state)
+ stm32f4_pll_enable(pll_div->hw_pll);
+
+ return ret;
+}
+
+const struct clk_ops stm32f4_pll_div_ops = {
+ .recalc_rate = stm32f4_pll_div_recalc_rate,
+ .round_rate = stm32f4_pll_div_round_rate,
+ .set_rate = stm32f4_pll_div_set_rate,
+};
+
+static struct clk_hw *clk_register_pll_div(const char *name,
+ const char *parent_name, unsigned long flags,
+ void __iomem *reg, u8 shift, u8 width,
+ u8 clk_divider_flags, const struct clk_div_table *table,
+ struct clk_hw *pll_hw, spinlock_t *lock)
+{
+ struct stm32f4_pll_div *pll_div;
+ struct clk_hw *hw;
+ struct clk_init_data init;
+ int ret;
+
+ /* allocate the divider */
+ pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
+ if (!pll_div)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = name;
+ init.ops = &stm32f4_pll_div_ops;
+ init.flags = flags;
+ init.parent_names = (parent_name ? &parent_name : NULL);
+ init.num_parents = (parent_name ? 1 : 0);
+
+ /* struct clk_divider assignments */
+ pll_div->div.reg = reg;
+ pll_div->div.shift = shift;
+ pll_div->div.width = width;
+ pll_div->div.flags = clk_divider_flags;
+ pll_div->div.lock = lock;
+ pll_div->div.table = table;
+ pll_div->div.hw.init = &init;
+
+ pll_div->hw_pll = pll_hw;
+
+ /* register the clock */
+ hw = &pll_div->div.hw;
+ ret = clk_hw_register(NULL, hw);
+ if (ret) {
+ kfree(pll_div);
+ hw = ERR_PTR(ret);
+ }
+
+ return hw;
+}
+
+static const struct clk_div_table pll_divp_table[] = {
+ { 0, 2 }, { 1, 4 }, { 2, 6 }, { 3, 8 },
+};
+
/*
* Decode current PLL state and (statically) model the state we inherit from
* the bootloader.
*/
-static void stm32f4_rcc_register_pll(const char *hse_clk, const char *hsi_clk)
+
+static struct clk_hw *stm32f4_rcc_register_pll(const char *pllsrc,
+ const struct stm32f4_pll_data *data, spinlock_t *lock)
{
- unsigned long pllcfgr = readl(base + STM32F4_RCC_PLLCFGR);
+ struct stm32f4_pll *pll;
+ struct clk_init_data init = { NULL };
+ void __iomem *reg;
+ struct clk_hw *pll_hw;
+ int ret;
+
+ pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+ if (!pll)
+ return ERR_PTR(-ENOMEM);
+
+ init.name = data->vco_name;
+ init.ops = &stm32f4_pll_gate_ops;
+ init.flags = CLK_IGNORE_UNUSED;
+ init.parent_names = &pllsrc;
+ init.num_parents = 1;
+
+ pll->hw.init = &init;
+
+ switch (data->pll_num) {
+ case PLL:
+ pll->offset = STM32F4_RCC_PLLCFGR;
+ pll->bit_idx = 24;
+ pll->bit_rdy_idx = 25;
+ break;
+ case PLL_I2S:
+ pll->offset = STM32F4_RCC_PLLI2SCFGR;
+ pll->bit_idx = 26;
+ pll->bit_rdy_idx = 27;
+ break;
+ case PLL_SAI:
+ pll->offset = STM32F4_RCC_PLLSAICFGR;
+ pll->bit_idx = 28;
+ pll->bit_rdy_idx = 29;
+ break;
+ };
+
+ pll->n_start = data->n_start;
+ pll->status = (readl(base + STM32F4_RCC_CR) >> pll->bit_idx) & 0x1;
+ reg = base + pll->offset;
+
+ pll_hw = &pll->hw;
+ ret = clk_hw_register(NULL, pll_hw);
+ if (ret) {
+ kfree(pll);
+ return ERR_PTR(ret);
+ }
+
+ if (data->p_name)
+ clk_register_pll_div(data->p_name, data->vco_name, 0, reg,
+ 16, 2, 0, pll_divp_table, pll_hw, lock);
+
+ if (data->q_name)
+ clk_register_pll_div(data->q_name, data->vco_name, 0, reg,
+ 24, 4, CLK_DIVIDER_ONE_BASED, NULL,
+ pll_hw, lock);

- unsigned long pllm = pllcfgr & 0x3f;
- unsigned long plln = (pllcfgr >> 6) & 0x1ff;
- unsigned long pllp = BIT(((pllcfgr >> 16) & 3) + 1);
- const char *pllsrc = pllcfgr & BIT(22) ? hse_clk : hsi_clk;
- unsigned long pllq = (pllcfgr >> 24) & 0xf;
+ if (data->r_name)
+ clk_register_pll_div(data->r_name, data->vco_name, 0, reg,
+ 28, 3, CLK_DIVIDER_ONE_BASED, NULL, pll_hw,
+ lock);

- clk_register_fixed_factor(NULL, "vco", pllsrc, 0, plln, pllm);
- clk_register_fixed_factor(NULL, "pll", "vco", 0, 1, pllp);
- clk_register_fixed_factor(NULL, "pll48", "vco", 0, 1, pllq);
+ return pll_hw;
}

/*
@@ -615,18 +944,24 @@ struct stm32f4_clk_data {
const struct stm32f4_gate_data *gates_data;
const u64 *gates_map;
int gates_num;
+ const struct stm32f4_pll_data *pll_data;
+ int pll_num;
};

static const struct stm32f4_clk_data stm32f429_clk_data = {
.gates_data = stm32f429_gates,
.gates_map = stm32f42xx_gate_map,
.gates_num = ARRAY_SIZE(stm32f429_gates),
+ .pll_data = stm32f429_pll,
+ .pll_num = ARRAY_SIZE(stm32f429_pll),
};

static const struct stm32f4_clk_data stm32f469_clk_data = {
.gates_data = stm32f469_gates,
.gates_map = stm32f46xx_gate_map,
.gates_num = ARRAY_SIZE(stm32f469_gates),
+ .pll_data = stm32f469_pll,
+ .pll_num = ARRAY_SIZE(stm32f469_pll),
};

static const struct of_device_id stm32f4_of_match[] = {
@@ -647,6 +982,8 @@ static void __init stm32f4_rcc_init(struct device_node *np)
int n;
const struct of_device_id *match;
const struct stm32f4_clk_data *data;
+ unsigned long pllcfgr;
+ const char *pllsrc;

base = of_iomap(np, 0);
if (!base) {
@@ -677,7 +1014,17 @@ static void __init stm32f4_rcc_init(struct device_node *np)

clk_register_fixed_rate_with_accuracy(NULL, "hsi", NULL, 0,
16000000, 160000);
- stm32f4_rcc_register_pll(hse_clk, "hsi");
+ pllcfgr = readl(base + STM32F4_RCC_PLLCFGR);
+ pllsrc = pllcfgr & BIT(22) ? hse_clk : "hsi";
+
+ stm32f4_rcc_register_pll(pllsrc, &data->pll_data[0],
+ &stm32f4_clk_lock);
+
+ clks[PLL_VCO_I2S] = stm32f4_rcc_register_pll(pllsrc,
+ &data->pll_data[1], &stm32f4_clk_lock);
+
+ clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll(pllsrc,
+ &data->pll_data[2], &stm32f4_clk_lock);

sys_parents[1] = hse_clk;
clk_register_mux_table(
--
1.9.1

2016-11-07 13:10:22

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 5/6] clk: stm32f4: Add SAI clocks

From: Gabriel Fernandez <[email protected]>

This patch introduces SAI clocks for stm32f4 socs.

Signed-off-by: Gabriel Fernandez <[email protected]>
---
drivers/clk/clk-stm32f4.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index b7cb359..c305659 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -217,6 +217,7 @@ enum {
PLL_VCO_I2S, PLL_VCO_SAI,
CLK_LCD,
CLK_I2S,
+ CLK_SAI1, CLK_SAI2,
END_PRIMARY_CLK
};

@@ -970,6 +971,9 @@ static struct clk_hw *stm32_register_cclk(struct device *dev, const char *name,

static const char *i2s_parents[2] = { "plli2s-r", NULL };

+static const char *sai_parents[4] = { "pllsai-q-div", "plli2s-q-div", NULL,
+ "no-clock" };
+
struct stm32f4_clk_data {
const struct stm32f4_gate_data *gates_data;
const u64 *gates_map;
@@ -1063,6 +1067,19 @@ static void __init stm32f4_rcc_init(struct device_node *np)
i2s_parents, ARRAY_SIZE(i2s_parents), 0,
base + STM32F4_RCC_CFGR, 23, 1, 0, NULL,
&stm32f4_clk_lock);
+
+ sai_parents[2] = i2s_in_clk;
+
+ clks[CLK_SAI1] = clk_hw_register_mux_table(NULL, "sai1-clk",
+ sai_parents, ARRAY_SIZE(sai_parents), 0,
+ base + STM32F4_RCC_DCKCFGR, 20, 1, 0, NULL,
+ &stm32f4_clk_lock);
+
+ clks[CLK_SAI2] = clk_hw_register_mux_table(NULL, "sai2-clk",
+ sai_parents, ARRAY_SIZE(sai_parents), 0,
+ base + STM32F4_RCC_DCKCFGR, 22, 1, 0, NULL,
+ &stm32f4_clk_lock);
+
sys_parents[1] = hse_clk;
clk_register_mux_table(
NULL, "sys", sys_parents, ARRAY_SIZE(sys_parents), 0,
--
1.9.1

2016-11-07 13:10:19

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH 6/6] arm: dts: stm32f4: Add external I2S clock

From: Gabriel Fernandez <[email protected]>

This patch adds an external I2S clock in the DT.
The I2S clock could be derived from an external I2S clock or by I2S pll.

Signed-off-by: Gabriel Fernandez <[email protected]>
---
arch/arm/boot/dts/stm32f429.dtsi | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 2700449..14da6ce 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -68,6 +68,12 @@
compatible = "fixed-clock";
clock-frequency = <32000>;
};
+
+ clk_i2s_ckin: i2s-ckin {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <0>;
+ };
};

soc {
@@ -356,7 +362,7 @@
#clock-cells = <2>;
compatible = "st,stm32f42xx-rcc", "st,stm32-rcc";
reg = <0x40023800 0x400>;
- clocks = <&clk_hse>;
+ clocks = <&clk_hse>, <&clk_i2s_ckin>;
st,syscfg = <&pwrcfg>;
};

--
1.9.1

2016-11-07 13:55:36

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH 2/6] clk: stm32f4: SDIO & 48Mhz clock management for STM32F469 board

On 07/11/16 13:05, [email protected] wrote:
> From: Gabriel Fernandez <[email protected]>
>
> In the stm32f469 soc, the 48Mhz clock could be derived from pll-q or
> from pll-sai-p.
>
> The SDIO clock could be also derived from 48Mhz or from sys clock.
>
> Signed-off-by: Gabriel Fernandez <[email protected]>
> ---
> drivers/clk/clk-stm32f4.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
> index 7641acd..dda15bc 100644
> --- a/drivers/clk/clk-stm32f4.c
> +++ b/drivers/clk/clk-stm32f4.c
> @@ -199,7 +199,7 @@ struct stm32f4_gate_data {
> { STM32F4_RCC_APB2ENR, 8, "adc1", "apb2_div" },
> { STM32F4_RCC_APB2ENR, 9, "adc2", "apb2_div" },
> { STM32F4_RCC_APB2ENR, 10, "adc3", "apb2_div" },
> - { STM32F4_RCC_APB2ENR, 11, "sdio", "pll48" },
> + { STM32F4_RCC_APB2ENR, 11, "sdio", "sdmux" },

I'm confused. How do the "sdmux" clock come to exist on STM32F429?


> { STM32F4_RCC_APB2ENR, 12, "spi1", "apb2_div" },
> { STM32F4_RCC_APB2ENR, 13, "spi4", "apb2_div" },
> { STM32F4_RCC_APB2ENR, 14, "syscfg", "apb2_div" },
> @@ -940,6 +940,10 @@ static struct clk_hw *stm32_register_cclk(struct device *dev, const char *name,
> "no-clock", "lse", "lsi", "hse-rtc"
> };
>
> +static const char *pll48_parents[2] = { "pll-q", "pllsai-p" };
> +
> +static const char *sdmux_parents[2] = { "pll48", "sys" };
> +
> struct stm32f4_clk_data {
> const struct stm32f4_gate_data *gates_data;
> const u64 *gates_map;
> @@ -1109,6 +1113,18 @@ static void __init stm32f4_rcc_init(struct device_node *np)
> goto fail;
> }
>
> + if (of_device_is_compatible(np, "st,stm32f469-rcc")) {
> + clk_hw_register_mux_table(NULL, "pll48",
> + pll48_parents, ARRAY_SIZE(pll48_parents), 0,
> + base + STM32F4_RCC_DCKCFGR, 27, 1, 0, NULL,
> + &stm32f4_clk_lock);
> +
> + clk_hw_register_mux_table(NULL, "sdmux",
> + sdmux_parents, ARRAY_SIZE(sdmux_parents), 0,
> + base + STM32F4_RCC_DCKCFGR, 28, 1, 0, NULL,
> + &stm32f4_clk_lock);
> + }
> +
> of_clk_add_hw_provider(np, stm32f4_rcc_lookup_clk, NULL);
> return;
> fail:
>

2016-11-07 13:58:47

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH 3/6] clk: stm32f4: Add post divisor for I2S & SAI PLLs and Add lcd-tft clock

On 07/11/16 13:05, [email protected] wrote:
> From: Gabriel Fernandez <[email protected]>
>
> This patch adds post dividers of I2S & SAI PLLs.
> These dividers are managed by a dedicated register (RCC_DCKCFGR).
> The PLL should be off before a set rate.
> This patch also introduces the lcd-tft clock.
>
> Signed-off-by: Gabriel Fernandez <[email protected]>
> ---
> drivers/clk/clk-stm32f4.c | 27 +++++++++++++++++++++++++--
> 1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
> index dda15bc..5fa5d51 100644
> --- a/drivers/clk/clk-stm32f4.c
> +++ b/drivers/clk/clk-stm32f4.c
> @@ -215,6 +215,7 @@ struct stm32f4_gate_data {
> enum {
> SYSTICK, FCLK, CLK_LSI, CLK_LSE, CLK_HSE_RTC, CLK_RTC,
> PLL_VCO_I2S, PLL_VCO_SAI,
> + CLK_LCD,
> END_PRIMARY_CLK
> };
>
> @@ -599,6 +600,9 @@ static struct clk_hw *clk_register_pll_div(const char *name,
> static const struct clk_div_table pll_divp_table[] = {
> { 0, 2 }, { 1, 4 }, { 2, 6 }, { 3, 8 },
> };
> +static const struct clk_div_table pll_lcd_div_table[] = {
> + { 0, 2 }, { 1, 4 }, { 2, 8 }, { 3, 16 },
> +};
>
> /*
> * Decode current PLL state and (statically) model the state we inherit from
> @@ -659,16 +663,35 @@ static struct clk_hw *stm32f4_rcc_register_pll(const char *pllsrc,
> clk_register_pll_div(data->p_name, data->vco_name, 0, reg,
> 16, 2, 0, pll_divp_table, pll_hw, lock);
>
> - if (data->q_name)
> + if (data->q_name) {
> clk_register_pll_div(data->q_name, data->vco_name, 0, reg,
> 24, 4, CLK_DIVIDER_ONE_BASED, NULL,
> pll_hw, lock);
>
> - if (data->r_name)
> + if (data->pll_num == PLL_I2S)
> + clk_register_pll_div("plli2s-q-div", data->q_name,
> + 0, base + STM32F4_RCC_DCKCFGR,
> + 0, 5, 0, NULL, pll_hw, &stm32f4_clk_lock);
> +
> + if (data->pll_num == PLL_SAI)
> + clk_register_pll_div("pllsai-q-div", data->q_name,
> + 0, base + STM32F4_RCC_DCKCFGR,
> + 8, 5, 0, NULL, pll_hw, &stm32f4_clk_lock);
> + }

Shouldn't this be in the config structures?

It seems very odd to me to allow the config structures to control
whether we take the branch or not and then add these hard coded hacks.


Daniel.

2016-11-07 14:06:44

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards

Hi Daniel,

Thanks for reviewing.


On 11/07/2016 02:53 PM, Daniel Thompson wrote:
> On 07/11/16 13:05, [email protected] wrote:
>> From: Gabriel Fernandez <[email protected]>
>>
>> This patch introduces PLL_I2S and PLL_SAI.
>> Vco clock of these PLLs can be modify by DT (only n multiplicator,
>> m divider is still fixed by the boot-loader).
>> Each PLL has 3 dividers. PLL should be off when we modify the rate.
>>
>> Signed-off-by: Gabriel Fernandez <[email protected]>
>> ---
>> drivers/clk/clk-stm32f4.c | 371
>> ++++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 359 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
>> index c2661e2..7641acd 100644
>> --- a/drivers/clk/clk-stm32f4.c
>> +++ b/drivers/clk/clk-stm32f4.c
>> @@ -28,6 +28,7 @@
> > ...
>> +static const struct clk_div_table pll_divp_table[] = {
>> + { 0, 2 }, { 1, 4 }, { 2, 6 }, { 3, 8 },
>> +};
>> +
>> /*
>> * Decode current PLL state and (statically) model the state we
>> inherit from
>> * the bootloader.
>> */
>
> This comment isn't right. For a start the model is no longer static.
>
you're right, i will suppress it.

>
>> @@ -615,18 +944,24 @@ struct stm32f4_clk_data {
>> const struct stm32f4_gate_data *gates_data;
>> const u64 *gates_map;
>> int gates_num;
>> + const struct stm32f4_pll_data *pll_data;
>> + int pll_num;
>
> pll_num is unused.
>
ok

BR

Gabriel

>
> Daniel.

2016-11-07 14:07:02

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: Re: [PATCH 2/6] clk: stm32f4: SDIO & 48Mhz clock management for STM32F469 board

Hi Daniel,


On 11/07/2016 02:55 PM, Daniel Thompson wrote:
> On 07/11/16 13:05, [email protected] wrote:
>> From: Gabriel Fernandez <[email protected]>
>>
>> In the stm32f469 soc, the 48Mhz clock could be derived from pll-q or
>> from pll-sai-p.
>>
>> The SDIO clock could be also derived from 48Mhz or from sys clock.
>>
>> Signed-off-by: Gabriel Fernandez <[email protected]>
>> ---
>> drivers/clk/clk-stm32f4.c | 18 +++++++++++++++++-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
>> index 7641acd..dda15bc 100644
>> --- a/drivers/clk/clk-stm32f4.c
>> +++ b/drivers/clk/clk-stm32f4.c
>> @@ -199,7 +199,7 @@ struct stm32f4_gate_data {
>> { STM32F4_RCC_APB2ENR, 8, "adc1", "apb2_div" },
>> { STM32F4_RCC_APB2ENR, 9, "adc2", "apb2_div" },
>> { STM32F4_RCC_APB2ENR, 10, "adc3", "apb2_div" },
>> - { STM32F4_RCC_APB2ENR, 11, "sdio", "pll48" },
>> + { STM32F4_RCC_APB2ENR, 11, "sdio", "sdmux" },
>
> I'm confused. How do the "sdmux" clock come to exist on STM32F429?
>
"sdmux" only exist on STM32F469 (struct stm32f4_gate_data stm32f469_gates[])

BR

Gabriel
>
>> { STM32F4_RCC_APB2ENR, 12, "spi1", "apb2_div" },
>> { STM32F4_RCC_APB2ENR, 13, "spi4", "apb2_div" },
>> { STM32F4_RCC_APB2ENR, 14, "syscfg", "apb2_div" },
>> @@ -940,6 +940,10 @@ static struct clk_hw *stm32_register_cclk(struct
>> device *dev, const char *name,
>> "no-clock", "lse", "lsi", "hse-rtc"
>> };
>>
>> +static const char *pll48_parents[2] = { "pll-q", "pllsai-p" };
>> +
>> +static const char *sdmux_parents[2] = { "pll48", "sys" };
>> +
>> struct stm32f4_clk_data {
>> const struct stm32f4_gate_data *gates_data;
>> const u64 *gates_map;
>> @@ -1109,6 +1113,18 @@ static void __init stm32f4_rcc_init(struct
>> device_node *np)
>> goto fail;
>> }
>>
>> + if (of_device_is_compatible(np, "st,stm32f469-rcc")) {
>> + clk_hw_register_mux_table(NULL, "pll48",
>> + pll48_parents, ARRAY_SIZE(pll48_parents), 0,
>> + base + STM32F4_RCC_DCKCFGR, 27, 1, 0, NULL,
>> + &stm32f4_clk_lock);
>> +
>> + clk_hw_register_mux_table(NULL, "sdmux",
>> + sdmux_parents, ARRAY_SIZE(sdmux_parents), 0,
>> + base + STM32F4_RCC_DCKCFGR, 28, 1, 0, NULL,
>> + &stm32f4_clk_lock);
>> + }
>> +
>> of_clk_add_hw_provider(np, stm32f4_rcc_lookup_clk, NULL);
>> return;
>> fail:
>>
>

2016-11-07 14:15:06

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: Re: [PATCH 3/6] clk: stm32f4: Add post divisor for I2S & SAI PLLs and Add lcd-tft clock

Hi Daniel,


On 11/07/2016 02:58 PM, Daniel Thompson wrote:
> On 07/11/16 13:05, [email protected] wrote:
>> From: Gabriel Fernandez <[email protected]>
>>
>> This patch adds post dividers of I2S & SAI PLLs.
>> These dividers are managed by a dedicated register (RCC_DCKCFGR).
>> The PLL should be off before a set rate.
>> This patch also introduces the lcd-tft clock.
>>
>> Signed-off-by: Gabriel Fernandez <[email protected]>
>> ---
>> drivers/clk/clk-stm32f4.c | 27 +++++++++++++++++++++++++--
>> 1 file changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
>> index dda15bc..5fa5d51 100644
>> --- a/drivers/clk/clk-stm32f4.c
>> +++ b/drivers/clk/clk-stm32f4.c
>> @@ -215,6 +215,7 @@ struct stm32f4_gate_data {
>> enum {
>> SYSTICK, FCLK, CLK_LSI, CLK_LSE, CLK_HSE_RTC, CLK_RTC,
>> PLL_VCO_I2S, PLL_VCO_SAI,
>> + CLK_LCD,
>> END_PRIMARY_CLK
>> };
>>
>> @@ -599,6 +600,9 @@ static struct clk_hw *clk_register_pll_div(const
>> char *name,
>> static const struct clk_div_table pll_divp_table[] = {
>> { 0, 2 }, { 1, 4 }, { 2, 6 }, { 3, 8 },
>> };
>> +static const struct clk_div_table pll_lcd_div_table[] = {
>> + { 0, 2 }, { 1, 4 }, { 2, 8 }, { 3, 16 },
>> +};
>>
>> /*
>> * Decode current PLL state and (statically) model the state we
>> inherit from
>> @@ -659,16 +663,35 @@ static struct clk_hw
>> *stm32f4_rcc_register_pll(const char *pllsrc,
>> clk_register_pll_div(data->p_name, data->vco_name, 0, reg,
>> 16, 2, 0, pll_divp_table, pll_hw, lock);
>>
>> - if (data->q_name)
>> + if (data->q_name) {
>> clk_register_pll_div(data->q_name, data->vco_name, 0, reg,
>> 24, 4, CLK_DIVIDER_ONE_BASED, NULL,
>> pll_hw, lock);
>>
>> - if (data->r_name)
>> + if (data->pll_num == PLL_I2S)
>> + clk_register_pll_div("plli2s-q-div", data->q_name,
>> + 0, base + STM32F4_RCC_DCKCFGR,
>> + 0, 5, 0, NULL, pll_hw, &stm32f4_clk_lock);
>> +
>> + if (data->pll_num == PLL_SAI)
>> + clk_register_pll_div("pllsai-q-div", data->q_name,
>> + 0, base + STM32F4_RCC_DCKCFGR,
>> + 8, 5, 0, NULL, pll_hw, &stm32f4_clk_lock);
>> + }
>
> Shouldn't this be in the config structures?
>
> It seems very odd to me to allow the config structures to control
> whether we take the branch or not and then add these hard coded hacks.
>
ok i will put it in the config structure.

BR
Gabriel.

>
> Daniel.

2016-11-07 14:15:02

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH 4/6] clk: stm32f4: Add I2S clock

On 07/11/16 13:05, [email protected] wrote:
> From: Gabriel Fernandez <[email protected]>
>
> This patch introduces I2S clock for stm32f4 soc.
> The I2S clock could be derived from an external clock or from pll-i2s
>
> Signed-off-by: Gabriel Fernandez <[email protected]>
> ---
> drivers/clk/clk-stm32f4.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
> index 5fa5d51..b7cb359 100644
> --- a/drivers/clk/clk-stm32f4.c
> +++ b/drivers/clk/clk-stm32f4.c
> @@ -216,6 +216,7 @@ enum {
> SYSTICK, FCLK, CLK_LSI, CLK_LSE, CLK_HSE_RTC, CLK_RTC,
> PLL_VCO_I2S, PLL_VCO_SAI,
> CLK_LCD,
> + CLK_I2S,

Sorry, this has just clicked and it applies to most of the other
patches, but adding things to this list effectively extends the clock
bindings (i.e. the list of valid "other" clocks access with a primary
index of 1).

This list if a list of "arbitrary" constants by which DT periphericals
can be linked to specific clocks.

So...

1) If a clock is introduced here we should update the clock binding
documentations.

2) If no peripheral can connect to the clock (because it is internal
to the clock gen logic and peripherals must connect to the gated
version) it should not be included in this enum.

3) I failed to mention this when the four undocumented clocks
(LSI, LSE, HSE_RTC and RTC) were added.

4) I *should* have added a comment explaining the above to the code.


> END_PRIMARY_CLK
> };
>
> @@ -967,6 +968,8 @@ static struct clk_hw *stm32_register_cclk(struct device *dev, const char *name,
>
> static const char *sdmux_parents[2] = { "pll48", "sys" };
>
> +static const char *i2s_parents[2] = { "plli2s-r", NULL };
> +
> struct stm32f4_clk_data {
> const struct stm32f4_gate_data *gates_data;
> const u64 *gates_map;
> @@ -1005,7 +1008,7 @@ struct stm32f4_clk_data {
>
> static void __init stm32f4_rcc_init(struct device_node *np)
> {
> - const char *hse_clk;
> + const char *hse_clk, *i2s_in_clk;
> int n;
> const struct of_device_id *match;
> const struct stm32f4_clk_data *data;
> @@ -1038,6 +1041,7 @@ static void __init stm32f4_rcc_init(struct device_node *np)
> stm32f4_gate_map = data->gates_map;
>
> hse_clk = of_clk_get_parent_name(np, 0);
> + i2s_in_clk = of_clk_get_parent_name(np, 1);

Again this looks like a change to the DT bindings.

Also does the code work if i2s_in_clk is NULL or as you hoping to get
away with a not-backwards compatible change?


Daniel.


>
> clk_register_fixed_rate_with_accuracy(NULL, "hsi", NULL, 0,
> 16000000, 160000);
> @@ -1053,6 +1057,12 @@ static void __init stm32f4_rcc_init(struct device_node *np)
> clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll(pllsrc,
> &data->pll_data[2], &stm32f4_clk_lock);
>
> + i2s_parents[1] = i2s_in_clk;
> +
> + clks[CLK_I2S] = clk_hw_register_mux_table(NULL, "i2s",
> + i2s_parents, ARRAY_SIZE(i2s_parents), 0,
> + base + STM32F4_RCC_CFGR, 23, 1, 0, NULL,
> + &stm32f4_clk_lock);
> sys_parents[1] = hse_clk;
> clk_register_mux_table(
> NULL, "sys", sys_parents, ARRAY_SIZE(sys_parents), 0,
>

2016-11-07 14:57:12

by Radoslaw Pietrzyk

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards

> +static struct clk_hw *clk_register_pll_div(const char *name,
> + const char *parent_name, unsigned long flags,
> + void __iomem *reg, u8 shift, u8 width,
> + u8 clk_divider_flags, const struct clk_div_table *table,
> + struct clk_hw *pll_hw, spinlock_t *lock)
> +{
> + struct stm32f4_pll_div *pll_div;
> + struct clk_hw *hw;
> + struct clk_init_data init;
> + int ret;
> +
> + /* allocate the divider */
> + pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
> + if (!pll_div)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &stm32f4_pll_div_ops;
> + init.flags = flags;
Maybe it's worth to have CLK_SET_RATE_PARENT here and the VCO clock
should have CLK_SET_RATE_GATE flag and we can get rid of custom
divider ops.


> -static void stm32f4_rcc_register_pll(const char *hse_clk, const char *hsi_clk)
> +
> +static struct clk_hw *stm32f4_rcc_register_pll(const char *pllsrc,
> + const struct stm32f4_pll_data *data, spinlock_t *lock)
> {
> - unsigned long pllcfgr = readl(base + STM32F4_RCC_PLLCFGR);
> + struct stm32f4_pll *pll;
> + struct clk_init_data init = { NULL };
> + void __iomem *reg;
> + struct clk_hw *pll_hw;
> + int ret;
> +
> + pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> + if (!pll)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = data->vco_name;
> + init.ops = &stm32f4_pll_gate_ops;
> + init.flags = CLK_IGNORE_UNUSED;
CLK_SET_RATE_GATE here

Moreover why not having VCO as a composite clock from gate and mult ?

According to docs SAI VCO (don't know about I2S ) must be within
certain range so clk_set_rate_range should be somewhere.

2016-11-07 17:49:23

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards

On 07/11/16 13:05, [email protected] wrote:
> From: Gabriel Fernandez <[email protected]>
>
> This patch introduces PLL_I2S and PLL_SAI.
> Vco clock of these PLLs can be modify by DT (only n multiplicator,
> m divider is still fixed by the boot-loader).
> Each PLL has 3 dividers. PLL should be off when we modify the rate.
>
> Signed-off-by: Gabriel Fernandez <[email protected]>
> ---
> drivers/clk/clk-stm32f4.c | 371 ++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 359 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
> index c2661e2..7641acd 100644
> --- a/drivers/clk/clk-stm32f4.c
> +++ b/drivers/clk/clk-stm32f4.c
> @@ -28,6 +28,7 @@
> ...
> +static const struct clk_div_table pll_divp_table[] = {
> + { 0, 2 }, { 1, 4 }, { 2, 6 }, { 3, 8 },
> +};
> +
> /*
> * Decode current PLL state and (statically) model the state we inherit from
> * the bootloader.
> */

This comment isn't right. For a start the model is no longer static.


> @@ -615,18 +944,24 @@ struct stm32f4_clk_data {
> const struct stm32f4_gate_data *gates_data;
> const u64 *gates_map;
> int gates_num;
> + const struct stm32f4_pll_data *pll_data;
> + int pll_num;

pll_num is unused.


Daniel.

2016-11-08 08:36:48

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards

Hi Radosław

Many thanks for reviewing.

On 11/07/2016 03:57 PM, Radosław Pietrzyk wrote:
>> +static struct clk_hw *clk_register_pll_div(const char *name,
>> + const char *parent_name, unsigned long flags,
>> + void __iomem *reg, u8 shift, u8 width,
>> + u8 clk_divider_flags, const struct clk_div_table *table,
>> + struct clk_hw *pll_hw, spinlock_t *lock)
>> +{
>> + struct stm32f4_pll_div *pll_div;
>> + struct clk_hw *hw;
>> + struct clk_init_data init;
>> + int ret;
>> +
>> + /* allocate the divider */
>> + pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
>> + if (!pll_div)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + init.name = name;
>> + init.ops = &stm32f4_pll_div_ops;
>> + init.flags = flags;
> Maybe it's worth to have CLK_SET_RATE_PARENT here and the VCO clock
> should have CLK_SET_RATE_GATE flag and we can get rid of custom
> divider ops.
I don't want to offer the possibility to change the vco clock through
the divisor of the pll (only by a boot-loader or by DT).

e.g. if i make a set rate on lcd-tft clock, i don't want to change the
SAI frequencies.

I used same structure for internal divisors of the pll (p, q, r) and for
post divisors (plli2s-q-div, pllsai-q-div & pllsai-r-div).
That why the CLK_SET_RATE_PARENT flag is transmit by parameter.

These divisors are similar because we have to switch off the pll before
changing the rate.

>
>
>> -static void stm32f4_rcc_register_pll(const char *hse_clk, const char *hsi_clk)
>> +
>> +static struct clk_hw *stm32f4_rcc_register_pll(const char *pllsrc,
>> + const struct stm32f4_pll_data *data, spinlock_t *lock)
>> {
>> - unsigned long pllcfgr = readl(base + STM32F4_RCC_PLLCFGR);
>> + struct stm32f4_pll *pll;
>> + struct clk_init_data init = { NULL };
>> + void __iomem *reg;
>> + struct clk_hw *pll_hw;
>> + int ret;
>> +
>> + pll = kzalloc(sizeof(*pll), GFP_KERNEL);
>> + if (!pll)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + init.name = data->vco_name;
>> + init.ops = &stm32f4_pll_gate_ops;
>> + init.flags = CLK_IGNORE_UNUSED;
> CLK_SET_RATE_GATE here
>
> Moreover why not having VCO as a composite clock from gate and mult ?
Yes, that sounds a good idea.

> According to docs SAI VCO (don't know about I2S ) must be within
> certain range so clk_set_rate_range should be somewhere.

2016-11-08 08:52:32

by Radoslaw Pietrzyk

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards

2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <[email protected]>:
> Hi Radosław
>
> Many thanks for reviewing.
>
> On 11/07/2016 03:57 PM, Radosław Pietrzyk wrote:
>>>
>>> +static struct clk_hw *clk_register_pll_div(const char *name,
>>> + const char *parent_name, unsigned long flags,
>>> + void __iomem *reg, u8 shift, u8 width,
>>> + u8 clk_divider_flags, const struct clk_div_table *table,
>>> + struct clk_hw *pll_hw, spinlock_t *lock)
>>> +{
>>> + struct stm32f4_pll_div *pll_div;
>>> + struct clk_hw *hw;
>>> + struct clk_init_data init;
>>> + int ret;
>>> +
>>> + /* allocate the divider */
>>> + pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
>>> + if (!pll_div)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + init.name = name;
>>> + init.ops = &stm32f4_pll_div_ops;
>>> + init.flags = flags;
>>
>> Maybe it's worth to have CLK_SET_RATE_PARENT here and the VCO clock
>> should have CLK_SET_RATE_GATE flag and we can get rid of custom
>> divider ops.
>
> I don't want to offer the possibility to change the vco clock through the
> divisor of the pll (only by a boot-loader or by DT).
>
> e.g. if i make a set rate on lcd-tft clock, i don't want to change the SAI
> frequencies.
>
> I used same structure for internal divisors of the pll (p, q, r) and for
> post divisors (plli2s-q-div, pllsai-q-div & pllsai-r-div).
> That why the CLK_SET_RATE_PARENT flag is transmit by parameter.
>
> These divisors are similar because we have to switch off the pll before
> changing the rate.
>
But changing pll and lcd dividers only may not be enough for getting
very specific pixelclocks and that might require changing the VCO
frequency itself. The rest of the SAI tree should be recalculated
then.

2016-11-08 16:20:06

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards

On 11/08/2016 09:52 AM, Radosław Pietrzyk wrote:
> 2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <[email protected]>:
>> Hi Radosław
>>
>> Many thanks for reviewing.
>>
>> On 11/07/2016 03:57 PM, Radosław Pietrzyk wrote:
>>>> +static struct clk_hw *clk_register_pll_div(const char *name,
>>>> + const char *parent_name, unsigned long flags,
>>>> + void __iomem *reg, u8 shift, u8 width,
>>>> + u8 clk_divider_flags, const struct clk_div_table *table,
>>>> + struct clk_hw *pll_hw, spinlock_t *lock)
>>>> +{
>>>> + struct stm32f4_pll_div *pll_div;
>>>> + struct clk_hw *hw;
>>>> + struct clk_init_data init;
>>>> + int ret;
>>>> +
>>>> + /* allocate the divider */
>>>> + pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
>>>> + if (!pll_div)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + init.name = name;
>>>> + init.ops = &stm32f4_pll_div_ops;
>>>> + init.flags = flags;
>>> Maybe it's worth to have CLK_SET_RATE_PARENT here and the VCO clock
>>> should have CLK_SET_RATE_GATE flag and we can get rid of custom
>>> divider ops.
>> I don't want to offer the possibility to change the vco clock through the
>> divisor of the pll (only by a boot-loader or by DT).
>>
>> e.g. if i make a set rate on lcd-tft clock, i don't want to change the SAI
>> frequencies.
>>
>> I used same structure for internal divisors of the pll (p, q, r) and for
>> post divisors (plli2s-q-div, pllsai-q-div & pllsai-r-div).
>> That why the CLK_SET_RATE_PARENT flag is transmit by parameter.
>>
>> These divisors are similar because we have to switch off the pll before
>> changing the rate.
>>
> But changing pll and lcd dividers only may not be enough for getting
> very specific pixelclocks and that might require changing the VCO
> frequency itself. The rest of the SAI tree should be recalculated
> then.
I agree but it seems to be too much complicated to recalculate all PLL
divisors if we change the vco clock.
You mean to use Clock notifier callback ?

2016-11-08 16:27:46

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: Re: [PATCH 4/6] clk: stm32f4: Add I2S clock



On 11/07/2016 03:14 PM, Daniel Thompson wrote:
> On 07/11/16 13:05, [email protected] wrote:
>> From: Gabriel Fernandez <[email protected]>
>>
>> This patch introduces I2S clock for stm32f4 soc.
>> The I2S clock could be derived from an external clock or from pll-i2s
>>
>> Signed-off-by: Gabriel Fernandez <[email protected]>
>> ---
>> drivers/clk/clk-stm32f4.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
>> index 5fa5d51..b7cb359 100644
>> --- a/drivers/clk/clk-stm32f4.c
>> +++ b/drivers/clk/clk-stm32f4.c
>> @@ -216,6 +216,7 @@ enum {
>> SYSTICK, FCLK, CLK_LSI, CLK_LSE, CLK_HSE_RTC, CLK_RTC,
>> PLL_VCO_I2S, PLL_VCO_SAI,
>> CLK_LCD,
>> + CLK_I2S,
>
> Sorry, this has just clicked and it applies to most of the other
> patches, but adding things to this list effectively extends the clock
> bindings (i.e. the list of valid "other" clocks access with a primary
> index of 1).
>
> This list if a list of "arbitrary" constants by which DT periphericals
> can be linked to specific clocks.
>
> So...
>
> 1) If a clock is introduced here we should update the clock binding
> documentations.
>
> 2) If no peripheral can connect to the clock (because it is internal
> to the clock gen logic and peripherals must connect to the gated
> version) it should not be included in this enum.
>
> 3) I failed to mention this when the four undocumented clocks
> (LSI, LSE, HSE_RTC and RTC) were added.
>
> 4) I *should* have added a comment explaining the above to the code.
>
>
ok i agree

>> END_PRIMARY_CLK
>> };
>>
>> @@ -967,6 +968,8 @@ static struct clk_hw *stm32_register_cclk(struct
>> device *dev, const char *name,
>>
>> static const char *sdmux_parents[2] = { "pll48", "sys" };
>>
>> +static const char *i2s_parents[2] = { "plli2s-r", NULL };
>> +
>> struct stm32f4_clk_data {
>> const struct stm32f4_gate_data *gates_data;
>> const u64 *gates_map;
>> @@ -1005,7 +1008,7 @@ struct stm32f4_clk_data {
>>
>> static void __init stm32f4_rcc_init(struct device_node *np)
>> {
>> - const char *hse_clk;
>> + const char *hse_clk, *i2s_in_clk;
>> int n;
>> const struct of_device_id *match;
>> const struct stm32f4_clk_data *data;
>> @@ -1038,6 +1041,7 @@ static void __init stm32f4_rcc_init(struct
>> device_node *np)
>> stm32f4_gate_map = data->gates_map;
>>
>> hse_clk = of_clk_get_parent_name(np, 0);
>> + i2s_in_clk = of_clk_get_parent_name(np, 1);
>
> Again this looks like a change to the DT bindings.
>
ok

> Also does the code work if i2s_in_clk is NULL or as you hoping to get
> away with a not-backwards compatible change?
>
>
yes it works if i2s_in_clk is NULL.

BR
Gabriel

> Daniel.
>
>
>>
>> clk_register_fixed_rate_with_accuracy(NULL, "hsi", NULL, 0,
>> 16000000, 160000);
>> @@ -1053,6 +1057,12 @@ static void __init stm32f4_rcc_init(struct
>> device_node *np)
>> clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll(pllsrc,
>> &data->pll_data[2], &stm32f4_clk_lock);
>>
>> + i2s_parents[1] = i2s_in_clk;
>> +
>> + clks[CLK_I2S] = clk_hw_register_mux_table(NULL, "i2s",
>> + i2s_parents, ARRAY_SIZE(i2s_parents), 0,
>> + base + STM32F4_RCC_CFGR, 23, 1, 0, NULL,
>> + &stm32f4_clk_lock);
>> sys_parents[1] = hse_clk;
>> clk_register_mux_table(
>> NULL, "sys", sys_parents, ARRAY_SIZE(sys_parents), 0,
>>
>

2016-11-09 08:10:33

by Radoslaw Pietrzyk

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards

I would expect that VCO clock will force recalculation for all its
children if I am not mistaken.

2016-11-08 17:19 GMT+01:00 Gabriel Fernandez <[email protected]>:
> On 11/08/2016 09:52 AM, Radosław Pietrzyk wrote:
>>
>> 2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <[email protected]>:
>>>
>>> Hi Radosław
>>>
>>> Many thanks for reviewing.
>>>
>>> On 11/07/2016 03:57 PM, Radosław Pietrzyk wrote:
>>>>>
>>>>> +static struct clk_hw *clk_register_pll_div(const char *name,
>>>>> + const char *parent_name, unsigned long flags,
>>>>> + void __iomem *reg, u8 shift, u8 width,
>>>>> + u8 clk_divider_flags, const struct clk_div_table
>>>>> *table,
>>>>> + struct clk_hw *pll_hw, spinlock_t *lock)
>>>>> +{
>>>>> + struct stm32f4_pll_div *pll_div;
>>>>> + struct clk_hw *hw;
>>>>> + struct clk_init_data init;
>>>>> + int ret;
>>>>> +
>>>>> + /* allocate the divider */
>>>>> + pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
>>>>> + if (!pll_div)
>>>>> + return ERR_PTR(-ENOMEM);
>>>>> +
>>>>> + init.name = name;
>>>>> + init.ops = &stm32f4_pll_div_ops;
>>>>> + init.flags = flags;
>>>>
>>>> Maybe it's worth to have CLK_SET_RATE_PARENT here and the VCO clock
>>>> should have CLK_SET_RATE_GATE flag and we can get rid of custom
>>>> divider ops.
>>>
>>> I don't want to offer the possibility to change the vco clock through the
>>> divisor of the pll (only by a boot-loader or by DT).
>>>
>>> e.g. if i make a set rate on lcd-tft clock, i don't want to change the
>>> SAI
>>> frequencies.
>>>
>>> I used same structure for internal divisors of the pll (p, q, r) and for
>>> post divisors (plli2s-q-div, pllsai-q-div & pllsai-r-div).
>>> That why the CLK_SET_RATE_PARENT flag is transmit by parameter.
>>>
>>> These divisors are similar because we have to switch off the pll before
>>> changing the rate.
>>>
>> But changing pll and lcd dividers only may not be enough for getting
>> very specific pixelclocks and that might require changing the VCO
>> frequency itself. The rest of the SAI tree should be recalculated
>> then.
>
> I agree but it seems to be too much complicated to recalculate all PLL
> divisors if we change the vco clock.
> You mean to use Clock notifier callback ?

2016-11-09 09:52:04

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: Re: [PATCH 1/6] clk: stm32f4: Add PLL_I2S & PLL_SAI for STM32F429/469 boards



On 11/09/2016 09:10 AM, Radosław Pietrzyk wrote:
> I would expect that VCO clock will force recalculation for all its
> children if I am not mistaken.
Sure

BR
Gabriel.
>
> 2016-11-08 17:19 GMT+01:00 Gabriel Fernandez <[email protected]>:
>> On 11/08/2016 09:52 AM, Radosław Pietrzyk wrote:
>>> 2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <[email protected]>:
>>>> Hi Radosław
>>>>
>>>> Many thanks for reviewing.
>>>>
>>>> On 11/07/2016 03:57 PM, Radosław Pietrzyk wrote:
>>>>>> +static struct clk_hw *clk_register_pll_div(const char *name,
>>>>>> + const char *parent_name, unsigned long flags,
>>>>>> + void __iomem *reg, u8 shift, u8 width,
>>>>>> + u8 clk_divider_flags, const struct clk_div_table
>>>>>> *table,
>>>>>> + struct clk_hw *pll_hw, spinlock_t *lock)
>>>>>> +{
>>>>>> + struct stm32f4_pll_div *pll_div;
>>>>>> + struct clk_hw *hw;
>>>>>> + struct clk_init_data init;
>>>>>> + int ret;
>>>>>> +
>>>>>> + /* allocate the divider */
>>>>>> + pll_div = kzalloc(sizeof(*pll_div), GFP_KERNEL);
>>>>>> + if (!pll_div)
>>>>>> + return ERR_PTR(-ENOMEM);
>>>>>> +
>>>>>> + init.name = name;
>>>>>> + init.ops = &stm32f4_pll_div_ops;
>>>>>> + init.flags = flags;
>>>>> Maybe it's worth to have CLK_SET_RATE_PARENT here and the VCO clock
>>>>> should have CLK_SET_RATE_GATE flag and we can get rid of custom
>>>>> divider ops.
>>>> I don't want to offer the possibility to change the vco clock through the
>>>> divisor of the pll (only by a boot-loader or by DT).
>>>>
>>>> e.g. if i make a set rate on lcd-tft clock, i don't want to change the
>>>> SAI
>>>> frequencies.
>>>>
>>>> I used same structure for internal divisors of the pll (p, q, r) and for
>>>> post divisors (plli2s-q-div, pllsai-q-div & pllsai-r-div).
>>>> That why the CLK_SET_RATE_PARENT flag is transmit by parameter.
>>>>
>>>> These divisors are similar because we have to switch off the pll before
>>>> changing the rate.
>>>>
>>> But changing pll and lcd dividers only may not be enough for getting
>>> very specific pixelclocks and that might require changing the VCO
>>> frequency itself. The rest of the SAI tree should be recalculated
>>> then.
>> I agree but it seems to be too much complicated to recalculate all PLL
>> divisors if we change the vco clock.
>> You mean to use Clock notifier callback ?