[PATCH 0/6] Add STM32F4 missing clocks

[PATCH 0/6] Add STM32F4 missing clocks

[gabriel.fernandez]

From: Gabriel Fernandez <gabriel.fernandez@st.com>

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

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

[gabriel.fernandez]

From: Gabriel Fernandez <gabriel.fernandez@st.com>

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 <gabriel.fernandez@st.com>
---
 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

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

[Daniel Thompson]

On 07/11/16 13:05, gabriel.fernandez@st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>
> 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 <gabriel.fernandez@st.com>
> ---
>  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.

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

[Gabriel Fernandez]

Hi Daniel,

Thanks for reviewing.


On 11/07/2016 02:53 PM, Daniel Thompson wrote:
> On 07/11/16 13:05, gabriel.fernandez@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> 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 <gabriel.fernandez@st.com>
>> ---
>>  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.

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

[Radosław Pietrzyk]

> +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.

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

[Gabriel Fernandez]

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.

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

[Radosław Pietrzyk]

2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <gabriel.fernandez@st.com>:
> 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.

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

[Gabriel Fernandez]

On 11/08/2016 09:52 AM, Radosław Pietrzyk wrote:
> 2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <gabriel.fernandez@st.com>:
>> 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 ?

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

[Radosław Pietrzyk]

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 <gabriel.fernandez@st.com>:
> On 11/08/2016 09:52 AM, Radosław Pietrzyk wrote:
>>
>> 2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <gabriel.fernandez@st.com>:
>>>
>>> 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 ?

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

[Gabriel Fernandez]

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 <gabriel.fernandez@st.com>:
>> On 11/08/2016 09:52 AM, Radosław Pietrzyk wrote:
>>> 2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <gabriel.fernandez@st.com>:
>>>> 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 ?

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

[gabriel.fernandez]

From: Gabriel Fernandez <gabriel.fernandez@st.com>

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 <gabriel.fernandez@st.com>
---
 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

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

[Daniel Thompson]

On 07/11/16 13:05, gabriel.fernandez@st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>
> 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 <gabriel.fernandez@st.com>
> ---
>  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:
>

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

[Gabriel Fernandez]

Hi Daniel,


On 11/07/2016 02:55 PM, Daniel Thompson wrote:
> On 07/11/16 13:05, gabriel.fernandez@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> 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 <gabriel.fernandez@st.com>
>> ---
>>  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:
>>
>

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

[gabriel.fernandez]

From: Gabriel Fernandez <gabriel.fernandez@st.com>

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 <gabriel.fernandez@st.com>
---
 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

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

[Daniel Thompson]

On 07/11/16 13:05, gabriel.fernandez@st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>
> 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 <gabriel.fernandez@st.com>
> ---
>  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.

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

[Gabriel Fernandez]

Hi Daniel,


On 11/07/2016 02:58 PM, Daniel Thompson wrote:
> On 07/11/16 13:05, gabriel.fernandez@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> 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 <gabriel.fernandez@st.com>
>> ---
>>  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.

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

[gabriel.fernandez]

From: Gabriel Fernandez <gabriel.fernandez@st.com>

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 <gabriel.fernandez@st.com>
---
 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

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

[Daniel Thompson]

On 07/11/16 13:05, gabriel.fernandez@st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>
> 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 <gabriel.fernandez@st.com>
> ---
>  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,
>

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

[Gabriel Fernandez]

On 11/07/2016 03:14 PM, Daniel Thompson wrote:
> On 07/11/16 13:05, gabriel.fernandez@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> 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 <gabriel.fernandez@st.com>
>> ---
>>  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,
>>
>

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

[gabriel.fernandez]

From: Gabriel Fernandez <gabriel.fernandez@st.com>

This patch introduces SAI clocks for stm32f4 socs.

Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
 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

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

[gabriel.fernandez]

From: Gabriel Fernandez <gabriel.fernandez@st.com>

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 <gabriel.fernandez@st.com>
---
 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