Re: [PATCH -next] clk: st: clkgen-pll: remove unused variable 'st_pll3200c32_407_a0'
Acked-by: Gabriel Fernandez
From: Stephen Boyd
Sent: Friday, August 16, 2019 7:36 PM
To: YueHaibing; alli...@lohutok.net; gre...@linuxfoundation.org;
mturque...@baylibre.com; Gabriel FERNANDEZ
Cc: linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; YueHaibing
Subject: Re: [PATCH -next] clk: st: clkgen-pll: remove unused variable
'st_pll3200c32_407_a0'
Quoting YueHaibing (2019-08-16 06:55:23)
> drivers/clk/st/clkgen-pll.c:64:37: warning:
> st_pll3200c32_407_a0 defined but not used [-Wunused-const-variable=]
>
> It is never used, so can be removed.
>
> Reported-by: Hulk Robot
> Signed-off-by: YueHaibing
> ---
Adding Gabriel, please ack/review.
> drivers/clk/st/clkgen-pll.c | 13 -
> 1 file changed, 13 deletions(-)
>
> diff --git a/drivers/clk/st/clkgen-pll.c b/drivers/clk/st/clkgen-pll.c
> index d8a688b..c3952f2 100644
> --- a/drivers/clk/st/clkgen-pll.c
> +++ b/drivers/clk/st/clkgen-pll.c
> @@ -61,19 +61,6 @@ static const struct clk_ops stm_pll3200c32_ops;
> static const struct clk_ops stm_pll3200c32_a9_ops;
> static const struct clk_ops stm_pll4600c28_ops;
>
> -static const struct clkgen_pll_data st_pll3200c32_407_a0 = {
> - /* 407 A0 */
> - .pdn_status = CLKGEN_FIELD(0x2a0, 0x1,8),
> - .pdn_ctrl = CLKGEN_FIELD(0x2a0, 0x1,8),
> - .locked_status = CLKGEN_FIELD(0x2a0, 0x1,24),
> - .ndiv = CLKGEN_FIELD(0x2a4, C32_NDIV_MASK, 16),
> - .idf= CLKGEN_FIELD(0x2a4, C32_IDF_MASK, 0x0),
> - .num_odfs = 1,
> - .odf= { CLKGEN_FIELD(0x2b4, C32_ODF_MASK, 0) },
> - .odf_gate = { CLKGEN_FIELD(0x2b4, 0x1,6) },
> - .ops= _pll3200c32_ops,
> -};
> -
> static const struct clkgen_pll_data st_pll3200c32_cx_0 = {
> /* 407 C0 PLL0 */
> .pdn_status = CLKGEN_FIELD(0x2a0, 0x1,8),
Re: [PATCH -next] clk: st: clkgen-fsyn: remove unused variable 'st_quadfs_fs660c32_ops'
Acked-by: Gabriel Fernandez
From: Stephen Boyd
Sent: Friday, August 16, 2019 7:36 PM
To: YueHaibing; i...@metux.net; mturque...@baylibre.com; r...@kernel.org;
Gabriel FERNANDEZ
Cc: linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; YueHaibing
Subject: Re: [PATCH -next] clk: st: clkgen-fsyn: remove unused variable
'st_quadfs_fs660c32_ops'
Quoting YueHaibing (2019-08-16 06:53:41)
> drivers/clk/st/clkgen-fsyn.c:70:29: warning:
> st_quadfs_fs660c32_ops defined but not used [-Wunused-const-variable=]
>
> It is never used, so can be removed.
>
> Reported-by: Hulk Robot
> Signed-off-by: YueHaibing
> ---
Adding Gabriel, please ack/review.
> drivers/clk/st/clkgen-fsyn.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/clk/st/clkgen-fsyn.c b/drivers/clk/st/clkgen-fsyn.c
> index ca1ccdb..a156bd0 100644
> --- a/drivers/clk/st/clkgen-fsyn.c
> +++ b/drivers/clk/st/clkgen-fsyn.c
> @@ -67,7 +67,6 @@ struct clkgen_quadfs_data {
> };
>
> static const struct clk_ops st_quadfs_pll_c32_ops;
> -static const struct clk_ops st_quadfs_fs660c32_ops;
>
> static int clk_fs660c32_dig_get_params(unsigned long input,
> unsigned long output, struct stm_fs *fs);
[PATCH 1/2] clk: stm32: Introduce clocks of STM32F769 board
STM32F769 clocks are derived from STM32746 clocks.
main differences are:
- new source clock for SAI1 and SAI2 (HSI or HSE)
- Add DFSDM & DSI clocks
Signed-off-by: Gabriel Fernandez
---
.../bindings/clock/st,stm32-rcc.txt | 6 +
drivers/clk/clk-stm32f4.c | 307 +-
include/dt-bindings/clock/stm32fx-clock.h | 7 +-
3 files changed, 310 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
b/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
index b240121d2ac9..cfa04b614d8a 100644
--- a/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
+++ b/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
@@ -11,6 +11,8 @@ Required properties:
"st,stm32f42xx-rcc"
"st,stm32f469-rcc"
"st,stm32f746-rcc"
+ "st,stm32f769-rcc"
+
- reg: should be register base and length as documented in the
datasheet
- #reset-cells: 1, see below
@@ -102,6 +104,10 @@ The secondary index is bound with the following magic
numbers:
28 CLK_I2C3
29 CLK_I2C4
30 CLK_LPTIMER (LPTimer1 clock)
+ 31 CLK_PLL_SRC
+ 32 CLK_DFSDM1
+ 33 CLK_ADFSDM1
+ 34 CLK_F769_DSI
)
Example:
diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index cdaa567c8042..fdac33a9be2f 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -300,6 +300,85 @@ static const struct stm32f4_gate_data stm32f746_gates[]
__initconst = {
{ STM32F4_RCC_APB2ENR, 26, "ltdc", "apb2_div" },
};
+static const struct stm32f4_gate_data stm32f769_gates[] __initconst = {
+ { STM32F4_RCC_AHB1ENR, 0, "gpioa","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 1, "gpiob","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 2, "gpioc","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 3, "gpiod","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 4, "gpioe","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 5, "gpiof","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 6, "gpiog","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 7, "gpioh","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 8, "gpioi","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 9, "gpioj","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 10, "gpiok","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 12, "crc", "ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 18, "bkpsra", "ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 20, "dtcmram", "ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 21, "dma1", "ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 22, "dma2", "ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 23, "dma2d","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 25, "ethmac", "ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 26, "ethmactx", "ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 27, "ethmacrx", "ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 28, "ethmacptp","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 29, "otghs","ahb_div" },
+ { STM32F4_RCC_AHB1ENR, 30, "otghsulpi","ahb_div" },
+
+ { STM32F4_RCC_AHB2ENR, 0, "dcmi", "ahb_div" },
+ { STM32F4_RCC_AHB2ENR, 1, "jpeg", "ahb_div" },
+ { STM32F4_RCC_AHB2ENR, 4, "cryp", "ahb_div" },
+ { STM32F4_RCC_AHB2ENR, 5, "hash", "ahb_div" },
+ { STM32F4_RCC_AHB2ENR, 6, "rng", "pll48" },
+ { STM32F4_RCC_AHB2ENR, 7, "otgfs","pll48" },
+
+ { STM32F4_RCC_AHB3ENR, 0, "fmc", "ahb_div",
+ CLK_IGNORE_UNUSED },
+ { STM32F4_RCC_AHB3ENR, 1, "qspi", "ahb_div",
+ CLK_IGNORE_UNUSED },
+
+ { STM32F4_RCC_APB1ENR, 0, "tim2", "apb1_mul" },
+ { STM32F4_RCC_APB1ENR, 1, "tim3", "apb1_mul" },
+ { STM32F4_RCC_APB1ENR, 2, "tim4", "apb1_mul" },
+ { STM32F4_RCC_APB1ENR, 3, "tim5", "apb1_mul" },
+ { STM32F4_RCC_APB1ENR, 4, "tim6&
[PATCH 0/2] clk: stm32: STM32F769 clocks
STM32F769 board is a derived of STM32F746 board. Concerning clocks, main differences are: - new source clock for SAI1 and SAI2 (HSI or HSE) - Add DFSDM & DSI clock Gabriel Fernandez (2): clk: stm32: Introduce clocks of STM32F769 board ARM: dts: stm32: Enable STM32F769 clock driver .../bindings/clock/st,stm32-rcc.txt | 6 + arch/arm/boot/dts/stm32f769-disco.dts | 4 + drivers/clk/clk-stm32f4.c | 307 +- include/dt-bindings/clock/stm32fx-clock.h | 7 +- 4 files changed, 314 insertions(+), 10 deletions(-) -- 2.17.1
[PATCH 2/2] ARM: dts: stm32: Enable STM32F769 clock driver
This patch enables clocks for STM32F769 boards.
Signed-off-by: Gabriel Fernandez
---
arch/arm/boot/dts/stm32f769-disco.dts | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f769-disco.dts
b/arch/arm/boot/dts/stm32f769-disco.dts
index 3c7216844a9b..6f1d0ac8c31c 100644
--- a/arch/arm/boot/dts/stm32f769-disco.dts
+++ b/arch/arm/boot/dts/stm32f769-disco.dts
@@ -102,6 +102,10 @@
};
};
+ {
+ compatible = "st,stm32f769-rcc", "st,stm32f746-rcc", "st,stm32-rcc";
+};
+
{
pinctrl-0 = <_pins_a>;
pinctrl-names = "default";
--
2.17.1
Re: [PATCH] Input: st-keyscan - fix potential zalloc NULL dereference
Sorry ignore this patch (bad mailing list) Best Regard Gabriel On 2/12/19 4:24 PM, gabriel.fernan...@st.com wrote: > From: Gabriel Fernandez > > This patch fixes the following static checker warning: > > drivers/input/keyboard/st-keyscan.c:156 keyscan_probe() > error: potential zalloc NULL dereference: 'keypad_data->input_dev' > > Reported-by: Dan Carpenter > Signed-off-by: Gabriel Fernandez > --- > drivers/input/keyboard/st-keyscan.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/keyboard/st-keyscan.c > b/drivers/input/keyboard/st-keyscan.c > index babcfb165e4f..3b85631fde91 100644 > --- a/drivers/input/keyboard/st-keyscan.c > +++ b/drivers/input/keyboard/st-keyscan.c > @@ -153,6 +153,8 @@ static int keyscan_probe(struct platform_device *pdev) > > input_dev->id.bustype = BUS_HOST; > > + keypad_data->input_dev = input_dev; > + > error = keypad_matrix_key_parse_dt(keypad_data); > if (error) > return error; > @@ -168,8 +170,6 @@ static int keyscan_probe(struct platform_device *pdev) > > input_set_drvdata(input_dev, keypad_data); > > - keypad_data->input_dev = input_dev; > - > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > keypad_data->base = devm_ioremap_resource(>dev, res); > if (IS_ERR(keypad_data->base))
Re: [bug report] Input: add st-keyscan driver
Hi all,
I prefer to fix it. I guess people used their own correction.
I will send you a fix asap.
Best Regards
Gabriel
On 1/27/19 2:20 AM, Ken Sloat wrote:
> On Sat, Jan 26, 2019 at 5:15 PM Dmitry Torokhov
> wrote:
>> On Sat, Jan 26, 2019 at 1:25 PM Ken Sloat
>> wrote:
>>> On Tue, Jan 22, 2019 at 1:53 PM Dan Carpenter
>>> wrote:
>>>> Hello Gabriel FERNANDEZ,
>>> Hello Dan,
>>>
>>> I have added CCs for the maintainers as well as Gabriel Fernandez as
>>> currently you just have the linux-input mailing list
>>>
>>>> The patch 062589b13991: "Input: add st-keyscan driver" from Apr 12,
>>>> 2014, leads to the following static checker warning:
>>>>
>>>> drivers/input/keyboard/st-keyscan.c:156 keyscan_probe()
>>>> error: potential zalloc NULL dereference: 'keypad_data->input_dev'
>>>>
>>>> drivers/input/keyboard/st-keyscan.c
>>>> 125 static int keyscan_probe(struct platform_device *pdev)
>>>> 126 {
>>>> 127 struct st_keyscan *keypad_data;
>>>> 128 struct input_dev *input_dev;
>>>> 129 struct resource *res;
>>>> 130 int error;
>>>> 131
>>>> 132 if (!pdev->dev.of_node) {
>>>> 133 dev_err(>dev, "no DT data present\n");
>>>> 134 return -EINVAL;
>>>> 135 }
>>>> 136
>>>> 137 keypad_data = devm_kzalloc(>dev,
>>>> sizeof(*keypad_data),
>>>> 138GFP_KERNEL);
>>>> 139 if (!keypad_data)
>>>> 140 return -ENOMEM;
>>>> 141
>>>> 142 input_dev = devm_input_allocate_device(>dev);
>>>> 143 if (!input_dev) {
>>>> 144 dev_err(>dev, "failed to allocate the input
>>>> device\n");
>>>> 145 return -ENOMEM;
>>>> 146 }
>>>> 147
>>>> 148 input_dev->name = pdev->name;
>>>> 149 input_dev->phys = "keyscan-keys/input0";
>>>> 150 input_dev->dev.parent = >dev;
>>>> 151 input_dev->open = keyscan_open;
>>>> 152 input_dev->close = keyscan_close;
>>>> 153
>>>> 154 input_dev->id.bustype = BUS_HOST;
>>>> 155
>>>> --> 156 error = keypad_matrix_key_parse_dt(keypad_data);
>>>> ^^^
>>> I agree with you this would be a problem
>>> to clarify the NULL derefence would occur here within
>>> keypad_matrix_key_parse_dt
>>>
>>> struct device *dev = keypad_data->input_dev->dev.parent;
>>>
>>>> This assumes we have set "keypad_data->input_dev = input_dev;" but we
>>>> don't do that until...
>>>>
>>>> 157 if (error)
>>>> 158 return error;
>>>> 159
>>>> 160 error = matrix_keypad_build_keymap(NULL, NULL,
>>>> 161keypad_data->n_rows,
>>>> 162keypad_data->n_cols,
>>>> 163NULL, input_dev);
>>>> 164 if (error) {
>>>> 165 dev_err(>dev, "failed to build keymap\n");
>>>> 166 return error;
>>>> 167 }
>>>> 168
>>>> 169 input_set_drvdata(input_dev, keypad_data);
>>>> 170
>>>> 171 keypad_data->input_dev = input_dev;
>>>> ^^
>>>>
>>>> this line here. This driver has never worked and it was included almost
>>>> five years ago. Is it worth fixing?
>>>>
>>>> 172
>>>> 173 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> 174 keypad_data->base = devm_ioremap_resource(>dev,
>>>> res);
>>>> 175 if (IS_ERR(keypad_data->base))
>>>> 176 return PTR_ERR(
Re: [PATCH] clk: stm32mp1: Add CLK_IGNORE_UNUSED to ck_sys_dbg clock
Thanks Stephen On 05/15/2018 08:23 PM, Stephen Boyd wrote: > Quoting gabriel.fernan...@st.com (2018-04-24 00:58:43) >> From: Gabriel Fernandez <gabriel.fernan...@st.com> >> >> Don't disable the dbg clock if was set by bootloader. >> >> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com> >> --- > Applied to clk-next >
Re: [PATCH] clk: stm32mp1: Add CLK_IGNORE_UNUSED to ck_sys_dbg clock
Thanks Stephen On 05/15/2018 08:23 PM, Stephen Boyd wrote: > Quoting gabriel.fernan...@st.com (2018-04-24 00:58:43) >> From: Gabriel Fernandez >> >> Don't disable the dbg clock if was set by bootloader. >> >> Signed-off-by: Gabriel Fernandez >> --- > Applied to clk-next >
Re: clk: stm32mp1: Fix a memory leak in 'clk_stm32_register_gate_ops()'
Hi Christophe,
Many thanks !
Acked-by: Gabriel Fernandez <gabriel.fernan...@st.com>
On 05/13/2018 01:17 PM, Christophe Jaillet wrote:
> We allocate some memory which is neither used, nor referenced by anything.
> So axe it.
>
> Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
> ---
> This patch as not been compile-tested, I don't have the corresponding arch
> and have not taken time to cross-compile it.
> ---
> drivers/clk/clk-stm32mp1.c | 9 +
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/clk/clk-stm32mp1.c b/drivers/clk/clk-stm32mp1.c
> index edd3cf451401..dfb9cb5bd0c4 100644
> --- a/drivers/clk/clk-stm32mp1.c
> +++ b/drivers/clk/clk-stm32mp1.c
> @@ -579,14 +579,9 @@ clk_stm32_register_gate_ops(struct device *dev,
> spinlock_t *lock)
> {
> struct clk_init_data init = { NULL };
> - struct clk_gate *gate;
> struct clk_hw *hw;
> int ret;
>
> - gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> - if (!gate)
> - return ERR_PTR(-ENOMEM);
> -
> init.name = name;
> init.parent_names = _name;
> init.num_parents = 1;
> @@ -604,10 +599,8 @@ clk_stm32_register_gate_ops(struct device *dev,
> hw->init =
>
> ret = clk_hw_register(dev, hw);
> - if (ret) {
> - kfree(gate);
> + if (ret)
> hw = ERR_PTR(ret);
> - }
>
> return hw;
> }
Re: clk: stm32mp1: Fix a memory leak in 'clk_stm32_register_gate_ops()'
Hi Christophe,
Many thanks !
Acked-by: Gabriel Fernandez
On 05/13/2018 01:17 PM, Christophe Jaillet wrote:
> We allocate some memory which is neither used, nor referenced by anything.
> So axe it.
>
> Signed-off-by: Christophe JAILLET
> ---
> This patch as not been compile-tested, I don't have the corresponding arch
> and have not taken time to cross-compile it.
> ---
> drivers/clk/clk-stm32mp1.c | 9 +
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/clk/clk-stm32mp1.c b/drivers/clk/clk-stm32mp1.c
> index edd3cf451401..dfb9cb5bd0c4 100644
> --- a/drivers/clk/clk-stm32mp1.c
> +++ b/drivers/clk/clk-stm32mp1.c
> @@ -579,14 +579,9 @@ clk_stm32_register_gate_ops(struct device *dev,
> spinlock_t *lock)
> {
> struct clk_init_data init = { NULL };
> - struct clk_gate *gate;
> struct clk_hw *hw;
> int ret;
>
> - gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> - if (!gate)
> - return ERR_PTR(-ENOMEM);
> -
> init.name = name;
> init.parent_names = _name;
> init.num_parents = 1;
> @@ -604,10 +599,8 @@ clk_stm32_register_gate_ops(struct device *dev,
> hw->init =
>
> ret = clk_hw_register(dev, hw);
> - if (ret) {
> - kfree(gate);
> + if (ret)
> hw = ERR_PTR(ret);
> - }
>
> return hw;
> }
Re: [PATCH] clk: stm32mp1: Add CLK_IGNORE_UNUSED to ck_sys_dbg clock
Hi Stephen, This patch is not a critical fix for this merge window. Thanks Gabriel On 05/02/2018 12:10 AM, Stephen Boyd wrote: > Quoting gabriel.fernan...@st.com (2018-04-24 00:58:43) >> From: Gabriel Fernandez <gabriel.fernan...@st.com> >> >> Don't disable the dbg clock if was set by bootloader. >> >> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com> >> --- > Is this a critical fix for this merge window? Please add "Fixes:" tag. >
Re: [PATCH] clk: stm32mp1: Add CLK_IGNORE_UNUSED to ck_sys_dbg clock
Hi Stephen, This patch is not a critical fix for this merge window. Thanks Gabriel On 05/02/2018 12:10 AM, Stephen Boyd wrote: > Quoting gabriel.fernan...@st.com (2018-04-24 00:58:43) >> From: Gabriel Fernandez >> >> Don't disable the dbg clock if was set by bootloader. >> >> Signed-off-by: Gabriel Fernandez >> --- > Is this a critical fix for this merge window? Please add "Fixes:" tag. >
Re: [PATCH RESEND 2/2] clk: stm32: Add DSI clock for STM32F469 Board
Thanks Stephen ! Best Regards Gabriel On 03/19/2018 09:45 PM, Stephen Boyd wrote: > Quoting gabriel.fernan...@st.com (2018-03-08 22:57:31) >> From: Gabriel Fernandez <gabriel.fernan...@st.com> >> >> This patch adds DSI clock for STM32F469 board >> >> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com> >> --- > Applied to clk-next >
Re: [PATCH RESEND 2/2] clk: stm32: Add DSI clock for STM32F469 Board
Thanks Stephen ! Best Regards Gabriel On 03/19/2018 09:45 PM, Stephen Boyd wrote: > Quoting gabriel.fernan...@st.com (2018-03-08 22:57:31) >> From: Gabriel Fernandez >> >> This patch adds DSI clock for STM32F469 board >> >> Signed-off-by: Gabriel Fernandez >> --- > Applied to clk-next >
Re: [PATCH v3 1/2] dt-bindings: reset: add STM32MP1 resets
Many Thanks Philipp, Best Regards Gabriel On 03/19/2018 10:02 AM, Philipp Zabel wrote: > Hi Gabriel, > > On Mon, 2018-03-19 at 08:25 +0100, gabriel.fernan...@st.com wrote: >> From: Gabriel Fernandez <gabriel.fernan...@st.com> >> >> This patch adds the reset binding entry for STM32MP1 >> >> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com> >> Reviewed-by: Rob Herring <r...@kernel.org> > thank you, both applied to reset/next. > > regards > Philipp
Re: [PATCH v3 1/2] dt-bindings: reset: add STM32MP1 resets
Many Thanks Philipp, Best Regards Gabriel On 03/19/2018 10:02 AM, Philipp Zabel wrote: > Hi Gabriel, > > On Mon, 2018-03-19 at 08:25 +0100, gabriel.fernan...@st.com wrote: >> From: Gabriel Fernandez >> >> This patch adds the reset binding entry for STM32MP1 >> >> Signed-off-by: Gabriel Fernandez >> Reviewed-by: Rob Herring > thank you, both applied to reset/next. > > regards > Philipp
Re: [PATCH v2 2/2] reset: stm32mp1: Enable stm32mp1 reset driver
Hi Philipp,
Thanks for reviewing.
On 03/16/2018 02:29 PM, Philipp Zabel wrote:
> Hi Gabriel,
>
> this looks mostly good to me, a few questions and comments below:
>
> On Wed, 2018-03-14 at 17:30 +0100, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> stm32mp1 RCC IP 1 has a reset SET register and a reset CLEAR register.
>>
>> Writing '0' on reset SET register has no effect
>> Writing '1' on reset SET register
>> activates the reset of the corresponding peripheral
>>
>> Writing '0' on reset CLEAR register has no effect
>> Writing '1' on reset CLEAR register
>> releases the reset of the corresponding peripheral
>>
>> See Documentation/devicetree/bindings/clock/st,stm32mp1-rcc.txt
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
>> ---
>> drivers/reset/Kconfig | 6 ++
>> drivers/reset/Makefile | 1 +
>> drivers/reset/reset-stm32mp1.c | 122
>> +
>> 3 files changed, 129 insertions(+)
>> create mode 100644 drivers/reset/reset-stm32mp1.c
>>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 1efbc6c..c0b292b 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -97,6 +97,12 @@ config RESET_SIMPLE
>> - Allwinner SoCs
>> - ZTE's zx2967 family
>>
>> +config RESET_STM32MP157
>> +bool "STM32MP157 Reset Driver" if COMPILE_TEST
>> +default MACH_STM32MP157
>> +help
>> + This enables the RCC reset controller driver for STM32 MPUs.
>> +
>> config RESET_SUNXI
>> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
>> default ARCH_SUNXI
>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>> index 132c24f..c1261dc 100644
>> --- a/drivers/reset/Makefile
>> +++ b/drivers/reset/Makefile
>> @@ -15,6 +15,7 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
>> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
>> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
>> obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
>> +obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
>> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>> obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
>> obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
>> diff --git a/drivers/reset/reset-stm32mp1.c b/drivers/reset/reset-stm32mp1.c
>> new file mode 100644
>> index 000..5e25388
>> --- /dev/null
>> +++ b/drivers/reset/reset-stm32mp1.c
>> @@ -0,0 +1,122 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
>> + * Author: Gabriel Fernandez <gabriel.fernan...@st.com> for
>> STMicroelectronics.
>> + */
>> +
>> +#include
> This does not seem to be necessary.
right
>> +#include
>> +#include
>> +#include
>> +#include
> This does not seem to be necessary either.
ok
>> +#include
>> +#include
>> +
>> +#define CLR_OFFSET 0x4
>> +
>> +struct stm32_reset_data {
>> +struct reset_controller_dev rcdev;
>> +void __iomem*membase;
>> +};
>> +
>> +static inline struct stm32_reset_data *
>> +to_stm32_reset_data(struct reset_controller_dev *rcdev)
>> +{
>> +return container_of(rcdev, struct stm32_reset_data, rcdev);
>> +}
>> +
>> +static int stm32_reset_update(struct reset_controller_dev *rcdev,
>> + unsigned long id, bool assert)
>> +{
>> +struct stm32_reset_data *data = to_stm32_reset_data(rcdev);
>> +int reg_width = sizeof(u32);
>> +int bank = id / (reg_width * BITS_PER_BYTE);
>> +int offset = id % (reg_width * BITS_PER_BYTE);
>> +void __iomem *addr;
>> +
>> +addr = data->membase + (bank * reg_width);
>> +if (!assert)
>> +addr += CLR_OFFSET;
>> +
>> +writel(BIT(offset), addr);
>> +
>> +return 0;
>> +}
>> +
>> +static int stm32_reset_assert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> +return stm32_reset_update(rcdev, id, true);
>> +}
>> +
>> +static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
>> +unsigned long id)
>> +{
>> +return stm32_reset_update(rcdev, id, false);
>> +}
>> +
>> +stati
Re: [PATCH v2 2/2] reset: stm32mp1: Enable stm32mp1 reset driver
Hi Philipp,
Thanks for reviewing.
On 03/16/2018 02:29 PM, Philipp Zabel wrote:
> Hi Gabriel,
>
> this looks mostly good to me, a few questions and comments below:
>
> On Wed, 2018-03-14 at 17:30 +0100, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez
>>
>> stm32mp1 RCC IP 1 has a reset SET register and a reset CLEAR register.
>>
>> Writing '0' on reset SET register has no effect
>> Writing '1' on reset SET register
>> activates the reset of the corresponding peripheral
>>
>> Writing '0' on reset CLEAR register has no effect
>> Writing '1' on reset CLEAR register
>> releases the reset of the corresponding peripheral
>>
>> See Documentation/devicetree/bindings/clock/st,stm32mp1-rcc.txt
>>
>> Signed-off-by: Gabriel Fernandez
>> ---
>> drivers/reset/Kconfig | 6 ++
>> drivers/reset/Makefile | 1 +
>> drivers/reset/reset-stm32mp1.c | 122
>> +
>> 3 files changed, 129 insertions(+)
>> create mode 100644 drivers/reset/reset-stm32mp1.c
>>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 1efbc6c..c0b292b 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -97,6 +97,12 @@ config RESET_SIMPLE
>> - Allwinner SoCs
>> - ZTE's zx2967 family
>>
>> +config RESET_STM32MP157
>> +bool "STM32MP157 Reset Driver" if COMPILE_TEST
>> +default MACH_STM32MP157
>> +help
>> + This enables the RCC reset controller driver for STM32 MPUs.
>> +
>> config RESET_SUNXI
>> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
>> default ARCH_SUNXI
>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>> index 132c24f..c1261dc 100644
>> --- a/drivers/reset/Makefile
>> +++ b/drivers/reset/Makefile
>> @@ -15,6 +15,7 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
>> obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
>> obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
>> obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
>> +obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
>> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>> obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
>> obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
>> diff --git a/drivers/reset/reset-stm32mp1.c b/drivers/reset/reset-stm32mp1.c
>> new file mode 100644
>> index 000..5e25388
>> --- /dev/null
>> +++ b/drivers/reset/reset-stm32mp1.c
>> @@ -0,0 +1,122 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
>> + * Author: Gabriel Fernandez for
>> STMicroelectronics.
>> + */
>> +
>> +#include
> This does not seem to be necessary.
right
>> +#include
>> +#include
>> +#include
>> +#include
> This does not seem to be necessary either.
ok
>> +#include
>> +#include
>> +
>> +#define CLR_OFFSET 0x4
>> +
>> +struct stm32_reset_data {
>> +struct reset_controller_dev rcdev;
>> +void __iomem*membase;
>> +};
>> +
>> +static inline struct stm32_reset_data *
>> +to_stm32_reset_data(struct reset_controller_dev *rcdev)
>> +{
>> +return container_of(rcdev, struct stm32_reset_data, rcdev);
>> +}
>> +
>> +static int stm32_reset_update(struct reset_controller_dev *rcdev,
>> + unsigned long id, bool assert)
>> +{
>> +struct stm32_reset_data *data = to_stm32_reset_data(rcdev);
>> +int reg_width = sizeof(u32);
>> +int bank = id / (reg_width * BITS_PER_BYTE);
>> +int offset = id % (reg_width * BITS_PER_BYTE);
>> +void __iomem *addr;
>> +
>> +addr = data->membase + (bank * reg_width);
>> +if (!assert)
>> +addr += CLR_OFFSET;
>> +
>> +writel(BIT(offset), addr);
>> +
>> +return 0;
>> +}
>> +
>> +static int stm32_reset_assert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> +return stm32_reset_update(rcdev, id, true);
>> +}
>> +
>> +static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
>> +unsigned long id)
>> +{
>> +return stm32_reset_update(rcdev, id, false);
>> +}
>> +
>> +static int stm32_reset_status(struct reset_controller_dev *rcdev,
>> + unsigned long i
Re: [PATCH 0/2] Introduce STM32MP1 Reset driver
Hi Philipp, Okay, i too support the idea to add custom reset driver. Many Thanks Philipp. Best regards Gabriel On 03/14/2018 10:12 AM, Philipp Zabel wrote: > Hi Gabriel, > > On Tue, 2018-03-13 at 17:34 +0100, gabriel.fernan...@st.com wrote: >> From: Gabriel Fernandez <gabriel.fernan...@st.com> >> >> This patch-set enables the reset of STM32MP1. >> It uses the reset simple driver by introducing the clear register offset >> parameter. >> STM32MP1 reset IP has a register to assert by writing '1' and another >> register to de-assert by writing '1'. >> The offset between this two registers is '0x4'. > I worry a bit about feature creep in the simple-reset driver. > Your patch on its own is simple enough, and I'm not opposed to add a > SET/CLR feature on principle, but there are a few issues: > > The RESET_SIMPLE Kconfig description currently says: > "This enables a simple reset controller driver for reset lines that > that can be asserted and deasserted by toggling bits in a contiguous, > exclusive register space." > That would have to be extended to mention SET/CLR register pairs as an > alternative. > > What about status (reset_simple_status)? Can current reset line status > be read back from the SET register, as is currently tried? If not, is > there a way to read current reset line status back at all? > > The data->lock spinlock is only needed to protect the read-modify-write > cycle on a toggle register, for separate SET/CLR register access the > locking is not necessary. > > At this point, it may or may not be easier to add a custom reset driver. > Either way you go, this is missing binding documentation for the > st,stm32mp1-rcc compatible in Documentation/devicetree/bindings/reset. > >> The patch 'dt-bindings: reset: add STM32MP1 resets' could be squashed >> with the patch: >> 'dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings' >> commit 3830681d354f >> >> Gabriel Fernandez (2): >>dt-bindings: reset: add STM32MP1 resets >>reset: simple: Enable stm32mp1 reset driver >> >> drivers/reset/reset-simple.c| 27 +-- >> drivers/reset/reset-simple.h| 1 + >> include/dt-bindings/reset/stm32mp1-resets.h | 108 >> >> 3 files changed, 130 insertions(+), 6 deletions(-) >> create mode 100644 include/dt-bindings/reset/stm32mp1-resets.h > regards > Philipp
Re: [PATCH 0/2] Introduce STM32MP1 Reset driver
Hi Philipp, Okay, i too support the idea to add custom reset driver. Many Thanks Philipp. Best regards Gabriel On 03/14/2018 10:12 AM, Philipp Zabel wrote: > Hi Gabriel, > > On Tue, 2018-03-13 at 17:34 +0100, gabriel.fernan...@st.com wrote: >> From: Gabriel Fernandez >> >> This patch-set enables the reset of STM32MP1. >> It uses the reset simple driver by introducing the clear register offset >> parameter. >> STM32MP1 reset IP has a register to assert by writing '1' and another >> register to de-assert by writing '1'. >> The offset between this two registers is '0x4'. > I worry a bit about feature creep in the simple-reset driver. > Your patch on its own is simple enough, and I'm not opposed to add a > SET/CLR feature on principle, but there are a few issues: > > The RESET_SIMPLE Kconfig description currently says: > "This enables a simple reset controller driver for reset lines that > that can be asserted and deasserted by toggling bits in a contiguous, > exclusive register space." > That would have to be extended to mention SET/CLR register pairs as an > alternative. > > What about status (reset_simple_status)? Can current reset line status > be read back from the SET register, as is currently tried? If not, is > there a way to read current reset line status back at all? > > The data->lock spinlock is only needed to protect the read-modify-write > cycle on a toggle register, for separate SET/CLR register access the > locking is not necessary. > > At this point, it may or may not be easier to add a custom reset driver. > Either way you go, this is missing binding documentation for the > st,stm32mp1-rcc compatible in Documentation/devicetree/bindings/reset. > >> The patch 'dt-bindings: reset: add STM32MP1 resets' could be squashed >> with the patch: >> 'dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings' >> commit 3830681d354f >> >> Gabriel Fernandez (2): >>dt-bindings: reset: add STM32MP1 resets >>reset: simple: Enable stm32mp1 reset driver >> >> drivers/reset/reset-simple.c| 27 +-- >> drivers/reset/reset-simple.h| 1 + >> include/dt-bindings/reset/stm32mp1-resets.h | 108 >> >> 3 files changed, 130 insertions(+), 6 deletions(-) >> create mode 100644 include/dt-bindings/reset/stm32mp1-resets.h > regards > Philipp
Re: [PATCH v2 01/12] dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings
Thanks Rob ! Best Regards Gabriel On 03/10/2018 12:53 AM, Rob Herring wrote: > On Thu, Mar 08, 2018 at 05:53:54PM +0100, gabriel.fernan...@st.com wrote: >> From: Gabriel Fernandez <gabriel.fernan...@st.com> >> >> The RCC block is responsible of the management of the clock and reset >> generation for the complete circuit. >> >> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com> >> --- >> .../devicetree/bindings/clock/st,stm32mp1-rcc.txt | 60 >> ++ >> 1 file changed, 60 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/clock/st,stm32mp1-rcc.txt > Reviewed-by: Rob Herring <r...@kernel.org>
Re: [PATCH v2 01/12] dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings
Thanks Rob ! Best Regards Gabriel On 03/10/2018 12:53 AM, Rob Herring wrote: > On Thu, Mar 08, 2018 at 05:53:54PM +0100, gabriel.fernan...@st.com wrote: >> From: Gabriel Fernandez >> >> The RCC block is responsible of the management of the clock and reset >> generation for the complete circuit. >> >> Signed-off-by: Gabriel Fernandez >> --- >> .../devicetree/bindings/clock/st,stm32mp1-rcc.txt | 60 >> ++ >> 1 file changed, 60 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/clock/st,stm32mp1-rcc.txt > Reviewed-by: Rob Herring
Re: [PATCH v2 00/12] Introduce STM32MP1 clock driver
Many Thanks Mike ! Best Regards Gabriel. On 03/11/2018 11:42 PM, Michael Turquette wrote: > Excerpts from gabriel.fernan...@st.com's message of March 8, 2018 8:53 > am: >> From: Gabriel Fernandez <gabriel.fernan...@st.com> >> >> v2: >> - Don't use MFD, use existing binding of STM32 RCC. >> - Rework Peripheral and Kernel clocks >> - cosmetic changes >> >> This patch-set introduces clock driver for STM32MP157 based on Arm >> Cortex-A7. >> The driver patch is split in several patches (by kind of clock) to >> facilitate >> code reviewing. > > Applied to clk-stm32mp1 towards v4.17. > > Regards, > Mike > >> >> Gabriel Fernandez (12): >> dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings >> clk: stm32mp1: Introduce STM32MP1 clock driver >> clk: stm32mp1: add MP1 gate for hse/hsi/csi oscillators >> clk: stm32mp1: add Source Clocks for PLLs >> clk: stm32mp1: add PLL clocks >> clk: stm32mp1: add Post-dividers for PLL >> clk: stm32mp1: add Sub System clocks >> clk: stm32mp1: add Kernel timers >> clk: stm32mp1: add Peripheral & Kernel Clocks >> clk: stm32mp1: add RTC clock >> clk: stm32mp1: add MCO clocks >> clk: stm32mp1: add Debug clocks >> >> .../devicetree/bindings/clock/st,stm32mp1-rcc.txt | 60 + >> drivers/clk/Kconfig | 6 + >> drivers/clk/Makefile | 1 + >> drivers/clk/clk-stm32mp1.c | 2117 >> >> include/dt-bindings/clock/stm32mp1-clks.h | 254 +++ >> 5 files changed, 2438 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/clock/st,stm32mp1-rcc.txt >> create mode 100644 drivers/clk/clk-stm32mp1.c >> create mode 100644 include/dt-bindings/clock/stm32mp1-clks.h >> >> -- >> 1.9.1 >> >>
Re: [PATCH v2 00/12] Introduce STM32MP1 clock driver
Many Thanks Mike ! Best Regards Gabriel. On 03/11/2018 11:42 PM, Michael Turquette wrote: > Excerpts from gabriel.fernan...@st.com's message of March 8, 2018 8:53 > am: >> From: Gabriel Fernandez >> >> v2: >> - Don't use MFD, use existing binding of STM32 RCC. >> - Rework Peripheral and Kernel clocks >> - cosmetic changes >> >> This patch-set introduces clock driver for STM32MP157 based on Arm >> Cortex-A7. >> The driver patch is split in several patches (by kind of clock) to >> facilitate >> code reviewing. > > Applied to clk-stm32mp1 towards v4.17. > > Regards, > Mike > >> >> Gabriel Fernandez (12): >> dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings >> clk: stm32mp1: Introduce STM32MP1 clock driver >> clk: stm32mp1: add MP1 gate for hse/hsi/csi oscillators >> clk: stm32mp1: add Source Clocks for PLLs >> clk: stm32mp1: add PLL clocks >> clk: stm32mp1: add Post-dividers for PLL >> clk: stm32mp1: add Sub System clocks >> clk: stm32mp1: add Kernel timers >> clk: stm32mp1: add Peripheral & Kernel Clocks >> clk: stm32mp1: add RTC clock >> clk: stm32mp1: add MCO clocks >> clk: stm32mp1: add Debug clocks >> >> .../devicetree/bindings/clock/st,stm32mp1-rcc.txt | 60 + >> drivers/clk/Kconfig | 6 + >> drivers/clk/Makefile | 1 + >> drivers/clk/clk-stm32mp1.c | 2117 >> >> include/dt-bindings/clock/stm32mp1-clks.h | 254 +++ >> 5 files changed, 2438 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/clock/st,stm32mp1-rcc.txt >> create mode 100644 drivers/clk/clk-stm32mp1.c >> create mode 100644 include/dt-bindings/clock/stm32mp1-clks.h >> >> -- >> 1.9.1 >> >>
Re: [PATCH 01/14] dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings
On 02/07/2018 07:03 PM, Rob Herring wrote:
> On Mon, Feb 5, 2018 at 1:01 AM, Gabriel FERNANDEZ
> <gabriel.fernan...@st.com> wrote:
>> Hi Rob,
>>
>> Thanks for reviewing.
>>
>>
>> On 02/05/2018 07:09 AM, Rob Herring wrote:
>>> On Fri, Feb 02, 2018 at 03:03:29PM +0100,gabriel.fernan...@st.com wrote:
>>>> From: Gabriel Fernandez<gabriel.fernan...@st.com>
>>>>
>>>> The RCC block is responsible of the management of the clock and reset
>>>> generation for the complete circuit.
>>>>
>>>> Signed-off-by: Gabriel Fernandez<gabriel.fernan...@st.com>
>>>> ---
>>>>.../devicetree/bindings/mfd/st,stm32-rcc.txt | 85
>>>> ++
>>>>1 file changed, 85 insertions(+)
>>>>create mode 100644
>>>> Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>>> b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>>> new file mode 100644
>>>> index 000..28017a1
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>>> @@ -0,0 +1,85 @@
>>>> +STMicroelectronics STM32 Peripheral Reset Clock Controller
>>>> +==
>>>> +
>>>> +The RCC IP is both a reset and a clock controller.
>>>> +
>>>> +Please also refer to reset.txt for common reset controller binding usage.
>>>> +
>>>> +Please also refer to clock-bindings.txt for common clock controller
>>>> +binding usage.
>>>> +
>>>> +
>>>> +Required properties:
>>>> +- compatible: "simple-mfd", "syscon"
>>>> +- reg: should be register base and length as documented in the datasheet
>>>> +
>>>> +- Sub-nodes:
>>>> + - compatible: "st,stm32mp1-rcc-clk"
>>>> +- #clock-cells: 1, device nodes should specify the clock in their
>>>> + "clocks" property, containing a phandle to the clock device node,
>>>> + an index specifying the clock to use.
>>>> +
>>>> + - compatible: "st,stm32mp1-rcc-rst"
>>>> +- #reset-cells: Shall be 1
>>>> +
>>>> +Example:
>>>> +rcc: rcc@5000 {
>>>> +compatible = "syscon", "simple-mfd";
>>>> +reg = <0x5000 0x1000>;
>>>> +
>>>> +rcc_clk: rcc-clk@5000 {
>>>> +#clock-cells = <1>;
>>>> +compatible = "st,stm32mp1-rcc-clk";
>>>> +};
>>>> +
>>>> +rcc_rst: rcc-reset@5000 {
>>> You should not have the same unit-address twice.
>>>
>>> IMO, this should just be:
>>>
>>>rcc: rcc@5000 {
>>>compatible = "st-stm32mp1-rcc";
>>>reg = <0x5000 0x1000>;
>>>#clock-cells = <1>;
>>>#reset-cells = <1>;
>>>};
>>>
>>> There's no reason a node can't provide more than 1 function.
>> RCC is an dedicated IP for clocks and resets, but also for power
>> management (patches will be sent later)
> If there's additional functions, they should be part of the binding
> now, not later. bindings should not unnecessarily evolve.
Yes you're right i will do it.
>> Then i need to probe 3 drivers with same IP.
> Drivers and DT nodes don't have to be 1-1. A parent driver can create
> additional child devices.
>
> Also, looking at your existing bindings for RCC IP, it is done as I suggested.
>
>> It's also a way to avoid use of 'CLK_OF_DECLARE_DRIVER' and i need it to
>> probe the 3th driver.
> Sounds like a Linux problem, not a DT issue.
>
> Rob
RCC is an IP block wich exposed multiple functionalities (clock, reset,
power management),
mfd should be the best solution to populate each functionality in
natural way,
and to access to registers ?
Gabriel
Re: [PATCH 01/14] dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings
On 02/07/2018 07:03 PM, Rob Herring wrote:
> On Mon, Feb 5, 2018 at 1:01 AM, Gabriel FERNANDEZ
> wrote:
>> Hi Rob,
>>
>> Thanks for reviewing.
>>
>>
>> On 02/05/2018 07:09 AM, Rob Herring wrote:
>>> On Fri, Feb 02, 2018 at 03:03:29PM +0100,gabriel.fernan...@st.com wrote:
>>>> From: Gabriel Fernandez
>>>>
>>>> The RCC block is responsible of the management of the clock and reset
>>>> generation for the complete circuit.
>>>>
>>>> Signed-off-by: Gabriel Fernandez
>>>> ---
>>>>.../devicetree/bindings/mfd/st,stm32-rcc.txt | 85
>>>> ++
>>>>1 file changed, 85 insertions(+)
>>>>create mode 100644
>>>> Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>>> b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>>> new file mode 100644
>>>> index 000..28017a1
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>>> @@ -0,0 +1,85 @@
>>>> +STMicroelectronics STM32 Peripheral Reset Clock Controller
>>>> +==
>>>> +
>>>> +The RCC IP is both a reset and a clock controller.
>>>> +
>>>> +Please also refer to reset.txt for common reset controller binding usage.
>>>> +
>>>> +Please also refer to clock-bindings.txt for common clock controller
>>>> +binding usage.
>>>> +
>>>> +
>>>> +Required properties:
>>>> +- compatible: "simple-mfd", "syscon"
>>>> +- reg: should be register base and length as documented in the datasheet
>>>> +
>>>> +- Sub-nodes:
>>>> + - compatible: "st,stm32mp1-rcc-clk"
>>>> +- #clock-cells: 1, device nodes should specify the clock in their
>>>> + "clocks" property, containing a phandle to the clock device node,
>>>> + an index specifying the clock to use.
>>>> +
>>>> + - compatible: "st,stm32mp1-rcc-rst"
>>>> +- #reset-cells: Shall be 1
>>>> +
>>>> +Example:
>>>> +rcc: rcc@5000 {
>>>> +compatible = "syscon", "simple-mfd";
>>>> +reg = <0x5000 0x1000>;
>>>> +
>>>> +rcc_clk: rcc-clk@5000 {
>>>> +#clock-cells = <1>;
>>>> +compatible = "st,stm32mp1-rcc-clk";
>>>> +};
>>>> +
>>>> +rcc_rst: rcc-reset@5000 {
>>> You should not have the same unit-address twice.
>>>
>>> IMO, this should just be:
>>>
>>>rcc: rcc@5000 {
>>>compatible = "st-stm32mp1-rcc";
>>>reg = <0x5000 0x1000>;
>>>#clock-cells = <1>;
>>>#reset-cells = <1>;
>>>};
>>>
>>> There's no reason a node can't provide more than 1 function.
>> RCC is an dedicated IP for clocks and resets, but also for power
>> management (patches will be sent later)
> If there's additional functions, they should be part of the binding
> now, not later. bindings should not unnecessarily evolve.
Yes you're right i will do it.
>> Then i need to probe 3 drivers with same IP.
> Drivers and DT nodes don't have to be 1-1. A parent driver can create
> additional child devices.
>
> Also, looking at your existing bindings for RCC IP, it is done as I suggested.
>
>> It's also a way to avoid use of 'CLK_OF_DECLARE_DRIVER' and i need it to
>> probe the 3th driver.
> Sounds like a Linux problem, not a DT issue.
>
> Rob
RCC is an IP block wich exposed multiple functionalities (clock, reset,
power management),
mfd should be the best solution to populate each functionality in
natural way,
and to access to registers ?
Gabriel
Re: [PATCH 01/14] dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings
On 02/05/2018 07:09 AM, Rob Herring wrote:
> On Fri, Feb 02, 2018 at 03:03:29PM +0100, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> The RCC block is responsible of the management of the clock and reset
>> generation for the complete circuit.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
>> ---
>> .../devicetree/bindings/mfd/st,stm32-rcc.txt | 85
>> ++
>> 1 file changed, 85 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> new file mode 100644
>> index 000..28017a1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> @@ -0,0 +1,85 @@
>> +STMicroelectronics STM32 Peripheral Reset Clock Controller
>> +==
>> +
>> +The RCC IP is both a reset and a clock controller.
>> +
>> +Please also refer to reset.txt for common reset controller binding usage.
>> +
>> +Please also refer to clock-bindings.txt for common clock controller
>> +binding usage.
>> +
>> +
>> +Required properties:
>> +- compatible: "simple-mfd", "syscon"
>
>> +- reg: should be register base and length as documented in the datasheet
>> +
>> +- Sub-nodes:
>> + - compatible: "st,stm32mp1-rcc-clk"
>> +- #clock-cells: 1, device nodes should specify the clock in their
>> + "clocks" property, containing a phandle to the clock device node,
>> + an index specifying the clock to use.
>> +
>> + - compatible: "st,stm32mp1-rcc-rst"
>> +- #reset-cells: Shall be 1
>> +
>> +Example:
>> +rcc: rcc@5000 {
>> +compatible = "syscon", "simple-mfd";
>> +reg = <0x5000 0x1000>;
>> +
>> +rcc_clk: rcc-clk@5000 {
>> +#clock-cells = <1>;
>> +compatible = "st,stm32mp1-rcc-clk";
>> +};
>> +
>> +rcc_rst: rcc-reset@5000 {
> You should not have the same unit-address twice.
i can change if you want into:
rcc: rcc@5000 {
compatible = "syscon", "simple-mfd";
reg = <0x5000 0x1000>;
rcc_clk: rcc-clk {
#clock-cells = <1>;
compatible = "st,stm32mp1-rcc-clk";
};
rcc_rst: rcc-reset {
#reset-cells = <1>;
compatible = "st,stm32mp1-rcc-rst";
};
BR
Gabriel
> IMO, this should just be:
>
> rcc: rcc@5000 {
> compatible = "st-stm32mp1-rcc";
> reg = <0x5000 0x1000>;
> #clock-cells = <1>;
> #reset-cells = <1>;
> };
>
> There's no reason a node can't provide more than 1 function.
>
>
>> +#reset-cells = <1>;
>> +compatible = "st,stm32mp1-rcc-rst";
>> +};
>> +};
>> +
>> +Specifying clocks
>> +=
>> +
>> +All available clocks are defined as preprocessor macros in
>> +dt-bindings/clock/stm32mp1-clks.h header and can be used in device
>> +tree sources.
>> +
>> +Example:
>> +
>> +/* Accessing DMA1 clock */
>> +... {
>> +clocks = <_clk DMA1>
>> +};
>> +
>> +/* Accessing SPI6 kernel clock */
>> +... {
>> +clocks = <_clk SPI6_K>
>> +};
> Other than the path to header, the clock binding explains all this. No
> need to duplicate here.
>
>> +
>> +Specifying softreset control of devices
>> +===
>> +
>> +Device nodes should specify the reset channel required in their "resets"
>> +property, containing a phandle to the reset device node and an index
>> specifying
>> +which channel to use.
>> +The index is the bit number within the RCC registers bank, starting from RCC
>> +base address.
>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>> +Where bit_offset is the bit offset within the register.
>> +
>> +For example on STM32MP1, for LTDC reset:
>> + ltdc = APB4_RSTSETR_offset / 4 * 32 + LTDC_bit_offset
>> + = 0x180 / 4 * 32 + 0 = 3072
>> +
>> +The list of valid indices for STM32MP1 is available in:
>> +include/dt-bindings/reset-controller/stm32mp1-resets.h
>> +
>> +This file implements defines like:
>> +#define LTDC_R 3072
>> +
>> +example:
>> +
>> +ltdc {
>> +resets = <_rst LTDC_R>;
>> +};
>> --
>> 1.9.1
>>
Re: [PATCH 01/14] dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings
On 02/05/2018 07:09 AM, Rob Herring wrote:
> On Fri, Feb 02, 2018 at 03:03:29PM +0100, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez
>>
>> The RCC block is responsible of the management of the clock and reset
>> generation for the complete circuit.
>>
>> Signed-off-by: Gabriel Fernandez
>> ---
>> .../devicetree/bindings/mfd/st,stm32-rcc.txt | 85
>> ++
>> 1 file changed, 85 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> new file mode 100644
>> index 000..28017a1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> @@ -0,0 +1,85 @@
>> +STMicroelectronics STM32 Peripheral Reset Clock Controller
>> +==
>> +
>> +The RCC IP is both a reset and a clock controller.
>> +
>> +Please also refer to reset.txt for common reset controller binding usage.
>> +
>> +Please also refer to clock-bindings.txt for common clock controller
>> +binding usage.
>> +
>> +
>> +Required properties:
>> +- compatible: "simple-mfd", "syscon"
>
>> +- reg: should be register base and length as documented in the datasheet
>> +
>> +- Sub-nodes:
>> + - compatible: "st,stm32mp1-rcc-clk"
>> +- #clock-cells: 1, device nodes should specify the clock in their
>> + "clocks" property, containing a phandle to the clock device node,
>> + an index specifying the clock to use.
>> +
>> + - compatible: "st,stm32mp1-rcc-rst"
>> +- #reset-cells: Shall be 1
>> +
>> +Example:
>> +rcc: rcc@5000 {
>> +compatible = "syscon", "simple-mfd";
>> +reg = <0x5000 0x1000>;
>> +
>> +rcc_clk: rcc-clk@5000 {
>> +#clock-cells = <1>;
>> +compatible = "st,stm32mp1-rcc-clk";
>> +};
>> +
>> +rcc_rst: rcc-reset@5000 {
> You should not have the same unit-address twice.
i can change if you want into:
rcc: rcc@5000 {
compatible = "syscon", "simple-mfd";
reg = <0x5000 0x1000>;
rcc_clk: rcc-clk {
#clock-cells = <1>;
compatible = "st,stm32mp1-rcc-clk";
};
rcc_rst: rcc-reset {
#reset-cells = <1>;
compatible = "st,stm32mp1-rcc-rst";
};
BR
Gabriel
> IMO, this should just be:
>
> rcc: rcc@5000 {
> compatible = "st-stm32mp1-rcc";
> reg = <0x5000 0x1000>;
> #clock-cells = <1>;
> #reset-cells = <1>;
> };
>
> There's no reason a node can't provide more than 1 function.
>
>
>> +#reset-cells = <1>;
>> +compatible = "st,stm32mp1-rcc-rst";
>> +};
>> +};
>> +
>> +Specifying clocks
>> +=
>> +
>> +All available clocks are defined as preprocessor macros in
>> +dt-bindings/clock/stm32mp1-clks.h header and can be used in device
>> +tree sources.
>> +
>> +Example:
>> +
>> +/* Accessing DMA1 clock */
>> +... {
>> +clocks = <_clk DMA1>
>> +};
>> +
>> +/* Accessing SPI6 kernel clock */
>> +... {
>> +clocks = <_clk SPI6_K>
>> +};
> Other than the path to header, the clock binding explains all this. No
> need to duplicate here.
>
>> +
>> +Specifying softreset control of devices
>> +===
>> +
>> +Device nodes should specify the reset channel required in their "resets"
>> +property, containing a phandle to the reset device node and an index
>> specifying
>> +which channel to use.
>> +The index is the bit number within the RCC registers bank, starting from RCC
>> +base address.
>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>> +Where bit_offset is the bit offset within the register.
>> +
>> +For example on STM32MP1, for LTDC reset:
>> + ltdc = APB4_RSTSETR_offset / 4 * 32 + LTDC_bit_offset
>> + = 0x180 / 4 * 32 + 0 = 3072
>> +
>> +The list of valid indices for STM32MP1 is available in:
>> +include/dt-bindings/reset-controller/stm32mp1-resets.h
>> +
>> +This file implements defines like:
>> +#define LTDC_R 3072
>> +
>> +example:
>> +
>> +ltdc {
>> +resets = <_rst LTDC_R>;
>> +};
>> --
>> 1.9.1
>>
Re: [PATCH 02/14] dt-bindings: clock: add STM32MP1 clocks
On 02/05/2018 07:09 AM, Rob Herring wrote: > On Fri, Feb 02, 2018 at 03:03:30PM +0100, gabriel.fernan...@st.com wrote: >> From: Gabriel Fernandez <gabriel.fernan...@st.com> >> >> This patch adds the clock binding entry for STM32MP1 >> >> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com> >> --- >> include/dt-bindings/clock/stm32mp1-clks.h | 248 >> ++ >> 1 file changed, 248 insertions(+) >> create mode 100644 include/dt-bindings/clock/stm32mp1-clks.h > You can squash this into the previous patch. Ok Thanks! BR Gabriel
Re: [PATCH 02/14] dt-bindings: clock: add STM32MP1 clocks
On 02/05/2018 07:09 AM, Rob Herring wrote: > On Fri, Feb 02, 2018 at 03:03:30PM +0100, gabriel.fernan...@st.com wrote: >> From: Gabriel Fernandez >> >> This patch adds the clock binding entry for STM32MP1 >> >> Signed-off-by: Gabriel Fernandez >> --- >> include/dt-bindings/clock/stm32mp1-clks.h | 248 >> ++ >> 1 file changed, 248 insertions(+) >> create mode 100644 include/dt-bindings/clock/stm32mp1-clks.h > You can squash this into the previous patch. Ok Thanks! BR Gabriel
Re: [PATCH 01/14] dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings
Hi Rob,
Thanks for reviewing.
On 02/05/2018 07:09 AM, Rob Herring wrote:
> On Fri, Feb 02, 2018 at 03:03:29PM +0100, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> The RCC block is responsible of the management of the clock and reset
>> generation for the complete circuit.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
>> ---
>> .../devicetree/bindings/mfd/st,stm32-rcc.txt | 85
>> ++
>> 1 file changed, 85 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> new file mode 100644
>> index 000..28017a1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> @@ -0,0 +1,85 @@
>> +STMicroelectronics STM32 Peripheral Reset Clock Controller
>> +==
>> +
>> +The RCC IP is both a reset and a clock controller.
>> +
>> +Please also refer to reset.txt for common reset controller binding usage.
>> +
>> +Please also refer to clock-bindings.txt for common clock controller
>> +binding usage.
>> +
>> +
>> +Required properties:
>> +- compatible: "simple-mfd", "syscon"
>
>> +- reg: should be register base and length as documented in the datasheet
>> +
>> +- Sub-nodes:
>> + - compatible: "st,stm32mp1-rcc-clk"
>> +- #clock-cells: 1, device nodes should specify the clock in their
>> + "clocks" property, containing a phandle to the clock device node,
>> + an index specifying the clock to use.
>> +
>> + - compatible: "st,stm32mp1-rcc-rst"
>> +- #reset-cells: Shall be 1
>> +
>> +Example:
>> +rcc: rcc@5000 {
>> +compatible = "syscon", "simple-mfd";
>> +reg = <0x5000 0x1000>;
>> +
>> +rcc_clk: rcc-clk@5000 {
>> +#clock-cells = <1>;
>> +compatible = "st,stm32mp1-rcc-clk";
>> +};
>> +
>> +rcc_rst: rcc-reset@5000 {
> You should not have the same unit-address twice.
>
> IMO, this should just be:
>
> rcc: rcc@5000 {
> compatible = "st-stm32mp1-rcc";
> reg = <0x5000 0x1000>;
> #clock-cells = <1>;
> #reset-cells = <1>;
> };
>
> There's no reason a node can't provide more than 1 function.
RCC is an dedicated IP for clocks and resets, but also for power
management (patches will be sent later)
Then i need to probe 3 drivers with same IP.
It's also a way to avoid use of 'CLK_OF_DECLARE_DRIVER' and i need it to
probe the 3th driver.
BR
Gabriel
>
>
>> +#reset-cells = <1>;
>> +compatible = "st,stm32mp1-rcc-rst";
>> +};
>> +};
>> +
>> +Specifying clocks
>> +=
>> +
>> +All available clocks are defined as preprocessor macros in
>> +dt-bindings/clock/stm32mp1-clks.h header and can be used in device
>> +tree sources.
>> +
>> +Example:
>> +
>> +/* Accessing DMA1 clock */
>> +... {
>> +clocks = <_clk DMA1>
>> +};
>> +
>> +/* Accessing SPI6 kernel clock */
>> +... {
>> +clocks = <_clk SPI6_K>
>> +};
> Other than the path to header, the clock binding explains all this. No
> need to duplicate here.
ok
>> +
>> +Specifying softreset control of devices
>> +===
>> +
>> +Device nodes should specify the reset channel required in their "resets"
>> +property, containing a phandle to the reset device node and an index
>> specifying
>> +which channel to use.
>> +The index is the bit number within the RCC registers bank, starting from RCC
>> +base address.
>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>> +Where bit_offset is the bit offset within the register.
>> +
>> +For example on STM32MP1, for LTDC reset:
>> + ltdc = APB4_RSTSETR_offset / 4 * 32 + LTDC_bit_offset
>> + = 0x180 / 4 * 32 + 0 = 3072
>> +
>> +The list of valid indices for STM32MP1 is available in:
>> +include/dt-bindings/reset-controller/stm32mp1-resets.h
>> +
>> +This file implements defines like:
>> +#define LTDC_R 3072
>> +
>> +example:
>> +
>> +ltdc {
>> +resets = <_rst LTDC_R>;
>> +};
>> --
>> 1.9.1
>>
Re: [PATCH 01/14] dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings
Hi Rob,
Thanks for reviewing.
On 02/05/2018 07:09 AM, Rob Herring wrote:
> On Fri, Feb 02, 2018 at 03:03:29PM +0100, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez
>>
>> The RCC block is responsible of the management of the clock and reset
>> generation for the complete circuit.
>>
>> Signed-off-by: Gabriel Fernandez
>> ---
>> .../devicetree/bindings/mfd/st,stm32-rcc.txt | 85
>> ++
>> 1 file changed, 85 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> new file mode 100644
>> index 000..28017a1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/st,stm32-rcc.txt
>> @@ -0,0 +1,85 @@
>> +STMicroelectronics STM32 Peripheral Reset Clock Controller
>> +==
>> +
>> +The RCC IP is both a reset and a clock controller.
>> +
>> +Please also refer to reset.txt for common reset controller binding usage.
>> +
>> +Please also refer to clock-bindings.txt for common clock controller
>> +binding usage.
>> +
>> +
>> +Required properties:
>> +- compatible: "simple-mfd", "syscon"
>
>> +- reg: should be register base and length as documented in the datasheet
>> +
>> +- Sub-nodes:
>> + - compatible: "st,stm32mp1-rcc-clk"
>> +- #clock-cells: 1, device nodes should specify the clock in their
>> + "clocks" property, containing a phandle to the clock device node,
>> + an index specifying the clock to use.
>> +
>> + - compatible: "st,stm32mp1-rcc-rst"
>> +- #reset-cells: Shall be 1
>> +
>> +Example:
>> +rcc: rcc@5000 {
>> +compatible = "syscon", "simple-mfd";
>> +reg = <0x5000 0x1000>;
>> +
>> +rcc_clk: rcc-clk@5000 {
>> +#clock-cells = <1>;
>> +compatible = "st,stm32mp1-rcc-clk";
>> +};
>> +
>> +rcc_rst: rcc-reset@5000 {
> You should not have the same unit-address twice.
>
> IMO, this should just be:
>
> rcc: rcc@5000 {
> compatible = "st-stm32mp1-rcc";
> reg = <0x5000 0x1000>;
> #clock-cells = <1>;
> #reset-cells = <1>;
> };
>
> There's no reason a node can't provide more than 1 function.
RCC is an dedicated IP for clocks and resets, but also for power
management (patches will be sent later)
Then i need to probe 3 drivers with same IP.
It's also a way to avoid use of 'CLK_OF_DECLARE_DRIVER' and i need it to
probe the 3th driver.
BR
Gabriel
>
>
>> +#reset-cells = <1>;
>> +compatible = "st,stm32mp1-rcc-rst";
>> +};
>> +};
>> +
>> +Specifying clocks
>> +=
>> +
>> +All available clocks are defined as preprocessor macros in
>> +dt-bindings/clock/stm32mp1-clks.h header and can be used in device
>> +tree sources.
>> +
>> +Example:
>> +
>> +/* Accessing DMA1 clock */
>> +... {
>> +clocks = <_clk DMA1>
>> +};
>> +
>> +/* Accessing SPI6 kernel clock */
>> +... {
>> +clocks = <_clk SPI6_K>
>> +};
> Other than the path to header, the clock binding explains all this. No
> need to duplicate here.
ok
>> +
>> +Specifying softreset control of devices
>> +===
>> +
>> +Device nodes should specify the reset channel required in their "resets"
>> +property, containing a phandle to the reset device node and an index
>> specifying
>> +which channel to use.
>> +The index is the bit number within the RCC registers bank, starting from RCC
>> +base address.
>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>> +Where bit_offset is the bit offset within the register.
>> +
>> +For example on STM32MP1, for LTDC reset:
>> + ltdc = APB4_RSTSETR_offset / 4 * 32 + LTDC_bit_offset
>> + = 0x180 / 4 * 32 + 0 = 3072
>> +
>> +The list of valid indices for STM32MP1 is available in:
>> +include/dt-bindings/reset-controller/stm32mp1-resets.h
>> +
>> +This file implements defines like:
>> +#define LTDC_R 3072
>> +
>> +example:
>> +
>> +ltdc {
>> +resets = <_rst LTDC_R>;
>> +};
>> --
>> 1.9.1
>>
Re: [PATCH 2/2] clk: stm32: Add DSI clock for STM32F469 Board
Hi Rob,
Thanks for reviewing.
On 01/29/2018 07:56 PM, Rob Herring wrote:
> On Thu, Jan 18, 2018 at 03:49:40PM +0100, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> This patch adds DSI clock for STM32F469 board
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
>> ---
>> drivers/clk/clk-stm32f4.c | 11 ++-
>> include/dt-bindings/clock/stm32fx-clock.h | 3 ++-
>> 2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
>> index da44f8d..3c28798 100644
>> --- a/drivers/clk/clk-stm32f4.c
>> +++ b/drivers/clk/clk-stm32f4.c
>> @@ -521,7 +521,7 @@ struct stm32f4_pll_data {
>> };
>>
>> static const struct stm32f4_pll_data stm32f469_pll[MAX_PLL_DIV] = {
>> -{ PLL, 50, { "pll", "pll-q",NULL } },
>> +{ PLL, 50, { "pll", "pll-q","pll-r"} },
>> { PLL_I2S, 50, { "plli2s-p", "plli2s-q", "plli2s-r" } },
>> { PLL_SAI, 50, { "pllsai-p", "pllsai-q", "pllsai-r" } },
>> };
>> @@ -1047,6 +1047,8 @@ static struct clk_hw *stm32_register_cclk(struct
>> device *dev, const char *name,
>> "no-clock", "lse", "lsi", "hse-rtc"
>> };
>>
>> +static const char *dsi_parent[2] = { NULL, "pll-r" };
>> +
>> static const char *lcd_parent[1] = { "pllsai-r-div" };
>>
>> static const char *i2s_parents[2] = { "plli2s-r", NULL };
>> @@ -1156,6 +1158,12 @@ struct stm32f4_clk_data {
>> NO_GATE, 0,
>> 0
>> },
>> +{
>> +CLK_F469_DSI, "dsi", dsi_parent, ARRAY_SIZE(dsi_parent),
>> +STM32F4_RCC_DCKCFGR, 29, 1,
>> +STM32F4_RCC_APB2ENR, 27,
>> +CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT
>> +},
>> };
>>
>> static const struct stm32_aux_clk stm32f746_aux_clk[] = {
>> @@ -1450,6 +1458,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);
>> +dsi_parent[0] = hse_clk;
>>
>> i2s_in_clk = of_clk_get_parent_name(np, 1);
>>
>> diff --git a/include/dt-bindings/clock/stm32fx-clock.h
>> b/include/dt-bindings/clock/stm32fx-clock.h
>> index 4d523b0..58d8b51 100644
>> --- a/include/dt-bindings/clock/stm32fx-clock.h
>> +++ b/include/dt-bindings/clock/stm32fx-clock.h
>> @@ -35,8 +35,9 @@
>> #define CLK_SAIQ_PDIV 13
>> #define CLK_HSI14
>> #define CLK_SYSCLK 15
>> +#define CLK_F469_DSI16
>>
>> -#define END_PRIMARY_CLK 16
>> +#define END_PRIMARY_CLK 17
>>
>> #define CLK_HDMI_CEC 16
>> #define CLK_SPDIF 17
> This looks suspicious. What's the relationship of these clocks?
I have just added CLK_F469_DSI in the binding, and shifted the end of
primary clock for F4 clocks.
'CLK_F469_DSI' binding is only used for STM32F469 and not for STM32F746
(that why CLK_HDMI_CEC can use the index 16)
BR
Gabriel.
>
> Rob
Re: [PATCH 2/2] clk: stm32: Add DSI clock for STM32F469 Board
Hi Rob,
Thanks for reviewing.
On 01/29/2018 07:56 PM, Rob Herring wrote:
> On Thu, Jan 18, 2018 at 03:49:40PM +0100, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez
>>
>> This patch adds DSI clock for STM32F469 board
>>
>> Signed-off-by: Gabriel Fernandez
>> ---
>> drivers/clk/clk-stm32f4.c | 11 ++-
>> include/dt-bindings/clock/stm32fx-clock.h | 3 ++-
>> 2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
>> index da44f8d..3c28798 100644
>> --- a/drivers/clk/clk-stm32f4.c
>> +++ b/drivers/clk/clk-stm32f4.c
>> @@ -521,7 +521,7 @@ struct stm32f4_pll_data {
>> };
>>
>> static const struct stm32f4_pll_data stm32f469_pll[MAX_PLL_DIV] = {
>> -{ PLL, 50, { "pll", "pll-q",NULL } },
>> +{ PLL, 50, { "pll", "pll-q","pll-r"} },
>> { PLL_I2S, 50, { "plli2s-p", "plli2s-q", "plli2s-r" } },
>> { PLL_SAI, 50, { "pllsai-p", "pllsai-q", "pllsai-r" } },
>> };
>> @@ -1047,6 +1047,8 @@ static struct clk_hw *stm32_register_cclk(struct
>> device *dev, const char *name,
>> "no-clock", "lse", "lsi", "hse-rtc"
>> };
>>
>> +static const char *dsi_parent[2] = { NULL, "pll-r" };
>> +
>> static const char *lcd_parent[1] = { "pllsai-r-div" };
>>
>> static const char *i2s_parents[2] = { "plli2s-r", NULL };
>> @@ -1156,6 +1158,12 @@ struct stm32f4_clk_data {
>> NO_GATE, 0,
>> 0
>> },
>> +{
>> +CLK_F469_DSI, "dsi", dsi_parent, ARRAY_SIZE(dsi_parent),
>> +STM32F4_RCC_DCKCFGR, 29, 1,
>> +STM32F4_RCC_APB2ENR, 27,
>> +CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT
>> +},
>> };
>>
>> static const struct stm32_aux_clk stm32f746_aux_clk[] = {
>> @@ -1450,6 +1458,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);
>> +dsi_parent[0] = hse_clk;
>>
>> i2s_in_clk = of_clk_get_parent_name(np, 1);
>>
>> diff --git a/include/dt-bindings/clock/stm32fx-clock.h
>> b/include/dt-bindings/clock/stm32fx-clock.h
>> index 4d523b0..58d8b51 100644
>> --- a/include/dt-bindings/clock/stm32fx-clock.h
>> +++ b/include/dt-bindings/clock/stm32fx-clock.h
>> @@ -35,8 +35,9 @@
>> #define CLK_SAIQ_PDIV 13
>> #define CLK_HSI14
>> #define CLK_SYSCLK 15
>> +#define CLK_F469_DSI16
>>
>> -#define END_PRIMARY_CLK 16
>> +#define END_PRIMARY_CLK 17
>>
>> #define CLK_HDMI_CEC 16
>> #define CLK_SPDIF 17
> This looks suspicious. What's the relationship of these clocks?
I have just added CLK_F469_DSI in the binding, and shifted the end of
primary clock for F4 clocks.
'CLK_F469_DSI' binding is only used for STM32F469 and not for STM32F746
(that why CLK_HDMI_CEC can use the index 16)
BR
Gabriel.
>
> Rob
Re: [PATCH] clk: stm32: add configuration flags for each of the stm32 drivers
Hi Benjamin, Just remove the extra blanck line. Otherwise you can addmy Acked-by: Gabriel Fernandez <gabriel.fernan...@st.com> Best regards Gabriel On 01/15/2018 03:21 PM, Benjamin Gaignard wrote: > Add two configuration flags to be able to not compile all the time > stm32f and stm32h7 drivers when ARCH_STM32 is set. > That help to save some space on those small platforms. > > Signed-off-by: Benjamin Gaignard <benjamin.gaign...@st.com> > --- > drivers/clk/Kconfig | 15 +++ > drivers/clk/Makefile | 4 ++-- > 2 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 1c4e1aa6767e..c876eb828fc4 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -226,6 +226,21 @@ config COMMON_CLK_VC5 > This driver supports the IDT VersaClock 5 and VersaClock 6 > programmable clock generators. > > +config COMMON_CLK_STM32F > + bool "Clock driver for stm32f4 and stm32f7 SoC families" > + depends on MACH_STM32F429 || MACH_STM32F469 || MACH_STM32F746 > + help > + ---help--- > + Support for stm32f4 and stm32f7 SoC families clocks > + > +config COMMON_CLK_STM32H7 > + bool "Clock driver for stm32h7 SoC family" > + depends on MACH_STM32H743 > + help > + ---help--- > + Support for stm32h7 SoC family clocks > + > + Removeextrablankline > source "drivers/clk/bcm/Kconfig" > source "drivers/clk/hisilicon/Kconfig" > source "drivers/clk/imgtec/Kconfig" > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index f7f761b02bed..4a8e063a7159 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -44,8 +44,8 @@ obj-$(CONFIG_COMMON_CLK_SCPI) += clk-scpi.o > obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o > obj-$(CONFIG_COMMON_CLK_SI514) += clk-si514.o > obj-$(CONFIG_COMMON_CLK_SI570) += clk-si570.o > -obj-$(CONFIG_ARCH_STM32) += clk-stm32f4.o > -obj-$(CONFIG_ARCH_STM32) += clk-stm32h7.o > +obj-$(CONFIG_COMMON_CLK_STM32F) += clk-stm32f4.o > +obj-$(CONFIG_COMMON_CLK_STM32H7) += clk-stm32h7.o > obj-$(CONFIG_ARCH_TANGO)+= clk-tango4.o > obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o > obj-$(CONFIG_ARCH_U300) += clk-u300.o
Re: [PATCH] clk: stm32: add configuration flags for each of the stm32 drivers
Hi Benjamin, Just remove the extra blanck line. Otherwise you can addmy Acked-by: Gabriel Fernandez Best regards Gabriel On 01/15/2018 03:21 PM, Benjamin Gaignard wrote: > Add two configuration flags to be able to not compile all the time > stm32f and stm32h7 drivers when ARCH_STM32 is set. > That help to save some space on those small platforms. > > Signed-off-by: Benjamin Gaignard > --- > drivers/clk/Kconfig | 15 +++ > drivers/clk/Makefile | 4 ++-- > 2 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 1c4e1aa6767e..c876eb828fc4 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -226,6 +226,21 @@ config COMMON_CLK_VC5 > This driver supports the IDT VersaClock 5 and VersaClock 6 > programmable clock generators. > > +config COMMON_CLK_STM32F > + bool "Clock driver for stm32f4 and stm32f7 SoC families" > + depends on MACH_STM32F429 || MACH_STM32F469 || MACH_STM32F746 > + help > + ---help--- > + Support for stm32f4 and stm32f7 SoC families clocks > + > +config COMMON_CLK_STM32H7 > + bool "Clock driver for stm32h7 SoC family" > + depends on MACH_STM32H743 > + help > + ---help--- > + Support for stm32h7 SoC family clocks > + > + Removeextrablankline > source "drivers/clk/bcm/Kconfig" > source "drivers/clk/hisilicon/Kconfig" > source "drivers/clk/imgtec/Kconfig" > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index f7f761b02bed..4a8e063a7159 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -44,8 +44,8 @@ obj-$(CONFIG_COMMON_CLK_SCPI) += clk-scpi.o > obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o > obj-$(CONFIG_COMMON_CLK_SI514) += clk-si514.o > obj-$(CONFIG_COMMON_CLK_SI570) += clk-si570.o > -obj-$(CONFIG_ARCH_STM32) += clk-stm32f4.o > -obj-$(CONFIG_ARCH_STM32) += clk-stm32h7.o > +obj-$(CONFIG_COMMON_CLK_STM32F) += clk-stm32f4.o > +obj-$(CONFIG_COMMON_CLK_STM32H7) += clk-stm32h7.o > obj-$(CONFIG_ARCH_TANGO)+= clk-tango4.o > obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o > obj-$(CONFIG_ARCH_U300) += clk-u300.o
Re: [PATCH v2] clk: stm32-h7: fix copyright
Acked-by: Gabriel Fernandez <gabriel.fernan...@st.com> On 11/30/2017 09:41 AM, Benjamin Gaignard wrote: > Uniformize STMicroelectronics copyrights header > Add SPDX identifier > > Signed-off-by: Benjamin Gaignard <benjamin.gaign...@st.com> > Acked-by: Alexandre TORGUE <alexandre.tor...@st.com> > CC: Gabriel Fernandez <gabriel.fernan...@st.com> > --- > drivers/clk/clk-stm32h7.c | 19 +++ > 1 file changed, 3 insertions(+), 16 deletions(-) > > diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c > index 61c3e40507d3..db2b162c0d4c 100644 > --- a/drivers/clk/clk-stm32h7.c > +++ b/drivers/clk/clk-stm32h7.c > @@ -1,20 +1,7 @@ > +// SPDX-License-Identifier: GPL-2.0 > /* > - * Copyright (C) Gabriel Fernandez 2017 > - * Author: Gabriel Fernandez <gabriel.fernan...@st.com> > - * > - * License terms: GPL V2.0. > - * > - * This program is free software; you can redistribute it and/or modify it > - * under the terms and conditions of the GNU General Public License, > - * version 2, as published by the Free Software Foundation. > - * > - * This program is distributed in the hope it will be useful, but WITHOUT > - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > - * more details. > - * > - * You should have received a copy of the GNU General Public License along > with > - * this program. If not, see <http://www.gnu.org/licenses/>. > + * Copyright (C) STMicroelectronics 2017 > + * Author: Gabriel Fernandez <gabriel.fernan...@st.com> for > STMicroelectronics. >*/ > > #include
Re: [PATCH v2] clk: stm32-h7: fix copyright
Acked-by: Gabriel Fernandez On 11/30/2017 09:41 AM, Benjamin Gaignard wrote: > Uniformize STMicroelectronics copyrights header > Add SPDX identifier > > Signed-off-by: Benjamin Gaignard > Acked-by: Alexandre TORGUE > CC: Gabriel Fernandez > --- > drivers/clk/clk-stm32h7.c | 19 +++ > 1 file changed, 3 insertions(+), 16 deletions(-) > > diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c > index 61c3e40507d3..db2b162c0d4c 100644 > --- a/drivers/clk/clk-stm32h7.c > +++ b/drivers/clk/clk-stm32h7.c > @@ -1,20 +1,7 @@ > +// SPDX-License-Identifier: GPL-2.0 > /* > - * Copyright (C) Gabriel Fernandez 2017 > - * Author: Gabriel Fernandez > - * > - * License terms: GPL V2.0. > - * > - * This program is free software; you can redistribute it and/or modify it > - * under the terms and conditions of the GNU General Public License, > - * version 2, as published by the Free Software Foundation. > - * > - * This program is distributed in the hope it will be useful, but WITHOUT > - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > - * more details. > - * > - * You should have received a copy of the GNU General Public License along > with > - * this program. If not, see <http://www.gnu.org/licenses/>. > + * Copyright (C) STMicroelectronics 2017 > + * Author: Gabriel Fernandez for > STMicroelectronics. >*/ > > #include
Re: linux-4.14-rc1/drivers/clk/clk-stm32h7.c: 2 * possible typo ?
I missed David ...
On 09/19/2017 11:22 AM, Gabriel FERNANDEZ wrote:
> Hi David,
>
> I confirm it's typo issue. I 'm ready to send a fix for that.
>
> Many Thank's
>
> Gabriel
>
>> Hello there,
>>
>> 1.
>>
>> linux-4.14-rc1/drivers/clk/clk-stm32h7.c:387]: (style) Same expression
>> on both sides of '&&'.
>>
>> Source code is
>>
>> if (gcfg->mux && gcfg->mux) {
>>
>> 2.
>>
>> [linux-4.14-rc1/drivers/clk/clk-stm32h7.c:413]: (style) Same expression
>> on both sides of '&&'.
>>
>> Duplicate.
>>
>> Regards
>>
>> David Binderman
Re: linux-4.14-rc1/drivers/clk/clk-stm32h7.c: 2 * possible typo ?
I missed David ...
On 09/19/2017 11:22 AM, Gabriel FERNANDEZ wrote:
> Hi David,
>
> I confirm it's typo issue. I 'm ready to send a fix for that.
>
> Many Thank's
>
> Gabriel
>
>> Hello there,
>>
>> 1.
>>
>> linux-4.14-rc1/drivers/clk/clk-stm32h7.c:387]: (style) Same expression
>> on both sides of '&&'.
>>
>> Source code is
>>
>> if (gcfg->mux && gcfg->mux) {
>>
>> 2.
>>
>> [linux-4.14-rc1/drivers/clk/clk-stm32h7.c:413]: (style) Same expression
>> on both sides of '&&'.
>>
>> Duplicate.
>>
>> Regards
>>
>> David Binderman
Re: linux-4.14-rc1/drivers/clk/clk-stm32h7.c: 2 * possible typo ?
Hi David,
I confirm it's typo issue. I 'm ready to send a fix for that.
Many Thank's
Gabriel
> Hello there,
>
> 1.
>
> linux-4.14-rc1/drivers/clk/clk-stm32h7.c:387]: (style) Same expression
> on both sides of '&&'.
>
> Source code is
>
> if (gcfg->mux && gcfg->mux) {
>
> 2.
>
> [linux-4.14-rc1/drivers/clk/clk-stm32h7.c:413]: (style) Same expression
> on both sides of '&&'.
>
> Duplicate.
>
> Regards
>
> David Binderman
Re: linux-4.14-rc1/drivers/clk/clk-stm32h7.c: 2 * possible typo ?
Hi David,
I confirm it's typo issue. I 'm ready to send a fix for that.
Many Thank's
Gabriel
> Hello there,
>
> 1.
>
> linux-4.14-rc1/drivers/clk/clk-stm32h7.c:387]: (style) Same expression
> on both sides of '&&'.
>
> Source code is
>
> if (gcfg->mux && gcfg->mux) {
>
> 2.
>
> [linux-4.14-rc1/drivers/clk/clk-stm32h7.c:413]: (style) Same expression
> on both sides of '&&'.
>
> Duplicate.
>
> Regards
>
> David Binderman
Re: [v3,3/5] reset: stm32: use the reset-simple driver
et_data,
> - rcdev);
> - int bank = id / BITS_PER_LONG;
> - int offset = id % BITS_PER_LONG;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(>lock, flags);
> -
> - reg = readl(data->membase + (bank * 4));
> - writel(reg & ~BIT(offset), data->membase + (bank * 4));
> -
> - spin_unlock_irqrestore(>lock, flags);
> -
> - return 0;
> -}
> -
> -static const struct reset_control_ops stm32_reset_ops = {
> - .assert = stm32_reset_assert,
> - .deassert = stm32_reset_deassert,
> -};
> -
> -static const struct of_device_id stm32_reset_dt_ids[] = {
> - { .compatible = "st,stm32-rcc", },
> - { /* sentinel */ },
> -};
> -
> -static int stm32_reset_probe(struct platform_device *pdev)
> -{
> - struct stm32_reset_data *data;
> - struct resource *res;
> -
> - data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->membase = devm_ioremap_resource(>dev, res);
> - if (IS_ERR(data->membase))
> - return PTR_ERR(data->membase);
> -
> - spin_lock_init(>lock);
> -
> - data->rcdev.owner = THIS_MODULE;
> - data->rcdev.nr_resets = resource_size(res) * 8;
> - data->rcdev.ops = _reset_ops;
> - data->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(>dev, >rcdev);
> -}
> -
> -static struct platform_driver stm32_reset_driver = {
> - .probe = stm32_reset_probe,
> - .driver = {
> - .name = "stm32-rcc-reset",
> - .of_match_table = stm32_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(stm32_reset_driver);
Acked-by: Gabriel Fernandez <gabriel.fernan...@st.com>
Re: [v3,3/5] reset: stm32: use the reset-simple driver
ITS_PER_LONG;
> - int offset = id % BITS_PER_LONG;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(>lock, flags);
> -
> - reg = readl(data->membase + (bank * 4));
> - writel(reg & ~BIT(offset), data->membase + (bank * 4));
> -
> - spin_unlock_irqrestore(>lock, flags);
> -
> - return 0;
> -}
> -
> -static const struct reset_control_ops stm32_reset_ops = {
> - .assert = stm32_reset_assert,
> - .deassert = stm32_reset_deassert,
> -};
> -
> -static const struct of_device_id stm32_reset_dt_ids[] = {
> - { .compatible = "st,stm32-rcc", },
> - { /* sentinel */ },
> -};
> -
> -static int stm32_reset_probe(struct platform_device *pdev)
> -{
> - struct stm32_reset_data *data;
> - struct resource *res;
> -
> - data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> -
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - data->membase = devm_ioremap_resource(>dev, res);
> - if (IS_ERR(data->membase))
> - return PTR_ERR(data->membase);
> -
> - spin_lock_init(>lock);
> -
> - data->rcdev.owner = THIS_MODULE;
> - data->rcdev.nr_resets = resource_size(res) * 8;
> - data->rcdev.ops = _reset_ops;
> - data->rcdev.of_node = pdev->dev.of_node;
> -
> - return devm_reset_controller_register(>dev, >rcdev);
> -}
> -
> -static struct platform_driver stm32_reset_driver = {
> - .probe = stm32_reset_probe,
> - .driver = {
> - .name = "stm32-rcc-reset",
> - .of_match_table = stm32_reset_dt_ids,
> - },
> -};
> -builtin_platform_driver(stm32_reset_driver);
Acked-by: Gabriel Fernandez
Re: [PATCH v7 3/3] clk: stm32h7: Add stm32h743 clock driver
On 07/19/2017 11:20 PM, Vladimir Zapolskiy wrote:
> Hello Gabriel,
>
> On 07/19/2017 05:25 PM, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> for MFD changes:
>> Acked-by: Lee Jones <lee.jo...@linaro.org>
>>
>> for DT-Bindings
>> Acked-by: Rob Herring <r...@kernel.org>
>> ---
>> .../devicetree/bindings/clock/st,stm32h7-rcc.txt | 82 ++
> I'll provide some review comments about device tree bindings only.
>
>> Hi drivers/clk/Makefile |1 +
>> drivers/clk/clk-stm32h7.c | 1409
>>
>> include/dt-bindings/clock/stm32h7-clks.h | 165 +++
>> include/dt-bindings/mfd/stm32h7-rcc.h | 136 ++
>> 5 files changed, 1793 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> create mode 100644 drivers/clk/clk-stm32h7.c
>> create mode 100644 include/dt-bindings/clock/stm32h7-clks.h
>> create mode 100644 include/dt-bindings/mfd/stm32h7-rcc.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> new file mode 100644
>> index 000..442c50c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> @@ -0,0 +1,82 @@
>> +STMicroelectronics STM32H7 Reset and Clock Controller
>> +=
>> +
>> +The RCC IP is both a reset and a clock controller.
>> +
>> +Please refer to clock-bindings.txt for common clock controller binding
>> usage.
>> +Please also refer to reset.txt for common reset controller binding usage.
>> +
>> +Required properties:
>> +- compatible: Should be:
>> + "st,stm32h743-rcc"
>> +
>> +- reg: should be register base and length as documented in the
>> + datasheet
>> +
>> +- #reset-cells: 1, see below
>> +
>> +- #clock-cells : from common clock binding; shall be set to 1
>> +
>> +- clocks: External oscillator clock phandle
>> + - high speed external clock signal (HSE)
>> + - low speed external clock signal (LSE)
>> + - external I2S clock (I2S_CKIN)
>> +
>> +Optional properties:
>> +- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
>> + write protection (RTC clock).
>> +
>> +Example:
>> +
>> +rcc: rcc@58024400 {
> 'rcc' as a generic device node name is awkward.
>
> I believe the main function of the device is clock controller (unlikely
> a generic reset controller can be converted into a clock controller),
> the locations of the document and device driver also indicate that
> primarily it is a clock controller, so I suggest to replace device node
> name with 'clock-controller' like below:
>
> rcc: clock-controller@58024400 {
>
>> +#reset-cells = <1>;
>> +#clock-cells = <2>
> Missing trailing semicolon ^^^
>
> My recommendation is to move #reset-cells and #clock-cells properties
> down after 'reg' or 'clocks' property in the list.
>
>> +compatible = "st,stm32h743-rcc", "st,stm32-rcc";
>> +reg = <0x58024400 0x400>;
>> +clocks = <_hse>, <_lse>, <_i2s_ckin>;
>> +
>> +st,syscfg = <>;
>> +
>> +#address-cells = <1>;
>> +#size-cells = <0>;
> Please drop #address-cells and #size-cells properties completely, from
> the document the device node does not define any children subnodes.
>
>> +};
>> +
>> +The peripheral clock consumer should specify the desired clock by
>> +having the clock ID in its "clocks" phandle cell.
>> +
>> +All available clocks are defined as preprocessor macros in
>> +dt-bindings/clock/stm32h7-clks.h header and can be used in device
>> +tree sources.
>> +
>> +Example:
>> +
>> +timer5: timer@4c00 {
>> +compatible = "st,stm32-timer";
>> +reg = <0x4c00 0x400>;
>> +interrupts = <50>;
>> +clocks = < TIM5_CK>;
>> +
> Please remote the empty line above.
>
>> +
Re: [PATCH v7 3/3] clk: stm32h7: Add stm32h743 clock driver
On 07/19/2017 11:20 PM, Vladimir Zapolskiy wrote:
> Hello Gabriel,
>
> On 07/19/2017 05:25 PM, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez
>>
>> for MFD changes:
>> Acked-by: Lee Jones
>>
>> for DT-Bindings
>> Acked-by: Rob Herring
>> ---
>> .../devicetree/bindings/clock/st,stm32h7-rcc.txt | 82 ++
> I'll provide some review comments about device tree bindings only.
>
>> Hi drivers/clk/Makefile |1 +
>> drivers/clk/clk-stm32h7.c | 1409
>>
>> include/dt-bindings/clock/stm32h7-clks.h | 165 +++
>> include/dt-bindings/mfd/stm32h7-rcc.h | 136 ++
>> 5 files changed, 1793 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> create mode 100644 drivers/clk/clk-stm32h7.c
>> create mode 100644 include/dt-bindings/clock/stm32h7-clks.h
>> create mode 100644 include/dt-bindings/mfd/stm32h7-rcc.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> new file mode 100644
>> index 000..442c50c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> @@ -0,0 +1,82 @@
>> +STMicroelectronics STM32H7 Reset and Clock Controller
>> +=
>> +
>> +The RCC IP is both a reset and a clock controller.
>> +
>> +Please refer to clock-bindings.txt for common clock controller binding
>> usage.
>> +Please also refer to reset.txt for common reset controller binding usage.
>> +
>> +Required properties:
>> +- compatible: Should be:
>> + "st,stm32h743-rcc"
>> +
>> +- reg: should be register base and length as documented in the
>> + datasheet
>> +
>> +- #reset-cells: 1, see below
>> +
>> +- #clock-cells : from common clock binding; shall be set to 1
>> +
>> +- clocks: External oscillator clock phandle
>> + - high speed external clock signal (HSE)
>> + - low speed external clock signal (LSE)
>> + - external I2S clock (I2S_CKIN)
>> +
>> +Optional properties:
>> +- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
>> + write protection (RTC clock).
>> +
>> +Example:
>> +
>> +rcc: rcc@58024400 {
> 'rcc' as a generic device node name is awkward.
>
> I believe the main function of the device is clock controller (unlikely
> a generic reset controller can be converted into a clock controller),
> the locations of the document and device driver also indicate that
> primarily it is a clock controller, so I suggest to replace device node
> name with 'clock-controller' like below:
>
> rcc: clock-controller@58024400 {
>
>> +#reset-cells = <1>;
>> +#clock-cells = <2>
> Missing trailing semicolon ^^^
>
> My recommendation is to move #reset-cells and #clock-cells properties
> down after 'reg' or 'clocks' property in the list.
>
>> +compatible = "st,stm32h743-rcc", "st,stm32-rcc";
>> +reg = <0x58024400 0x400>;
>> +clocks = <_hse>, <_lse>, <_i2s_ckin>;
>> +
>> +st,syscfg = <>;
>> +
>> +#address-cells = <1>;
>> +#size-cells = <0>;
> Please drop #address-cells and #size-cells properties completely, from
> the document the device node does not define any children subnodes.
>
>> +};
>> +
>> +The peripheral clock consumer should specify the desired clock by
>> +having the clock ID in its "clocks" phandle cell.
>> +
>> +All available clocks are defined as preprocessor macros in
>> +dt-bindings/clock/stm32h7-clks.h header and can be used in device
>> +tree sources.
>> +
>> +Example:
>> +
>> +timer5: timer@4c00 {
>> +compatible = "st,stm32-timer";
>> +reg = <0x4c00 0x400>;
>> +interrupts = <50>;
>> +clocks = < TIM5_CK>;
>> +
> Please remote the empty line above.
>
>> +};
>> +
>> +Specifying softreset control of devices
>> +===
>> +
>> +Device nodes should
Re: [PATCH v7 3/3] clk: stm32h7: Add stm32h743 clock driver
On 07/21/2017 10:37 PM, Stephen Boyd wrote:
> On 07/20, Vladimir Zapolskiy wrote:
>> Hi Gabriel,
>>
>> On 07/20/2017 11:31 AM, Gabriel FERNANDEZ wrote:
>>> Hi Vladimir,
>>>
>>>
>>> On 07/19/2017 11:20 PM, Vladimir Zapolskiy wrote:
>>>> Hello Gabriel,
>>>>
>>>> On 07/19/2017 05:25 PM, gabriel.fernan...@st.com wrote:
>>>>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>>>> +
>>>>> + rcc: rcc@58024400 {
>>>> 'rcc' as a generic device node name is awkward.
>>>>
>>>> I believe the main function of the device is clock controller (unlikely
>>>> a generic reset controller can be converted into a clock controller),
>>>> the locations of the document and device driver also indicate that
>>>> primarily it is a clock controller, so I suggest to replace device node
>>>> name with 'clock-controller' like below:
>>> I prefer to keep rcc node name, to be coherent with the other ST
>>> platforms (STM32F4/F7)
>> the thing is, a device node name is expected to comply with ePAPR or
>> the devicetree specification, which says
>>
>> The name of a node should be somewhat generic, reflecting
>> the function of the device and not its precise programming model.
>>
>> If devicetree and CCF maintainers are fine with 'rcc', I won't object,
>> my role is just to emphasize the found issue and recommend to use another
>> and more common name 'clock-controller', it is a simple and fortunately
>> backward compatible change to other ST platforms as well.
> Yes. It should be generic so clock-controller or
> clock-reset-controller is appropriate here.
>
ok i will change order... reset-clock-controller name to match with rcc.
Best Regards
Gabriel
Re: [PATCH v7 3/3] clk: stm32h7: Add stm32h743 clock driver
On 07/21/2017 10:37 PM, Stephen Boyd wrote:
> On 07/20, Vladimir Zapolskiy wrote:
>> Hi Gabriel,
>>
>> On 07/20/2017 11:31 AM, Gabriel FERNANDEZ wrote:
>>> Hi Vladimir,
>>>
>>>
>>> On 07/19/2017 11:20 PM, Vladimir Zapolskiy wrote:
>>>> Hello Gabriel,
>>>>
>>>> On 07/19/2017 05:25 PM, gabriel.fernan...@st.com wrote:
>>>>> From: Gabriel Fernandez
>>>>> +
>>>>> + rcc: rcc@58024400 {
>>>> 'rcc' as a generic device node name is awkward.
>>>>
>>>> I believe the main function of the device is clock controller (unlikely
>>>> a generic reset controller can be converted into a clock controller),
>>>> the locations of the document and device driver also indicate that
>>>> primarily it is a clock controller, so I suggest to replace device node
>>>> name with 'clock-controller' like below:
>>> I prefer to keep rcc node name, to be coherent with the other ST
>>> platforms (STM32F4/F7)
>> the thing is, a device node name is expected to comply with ePAPR or
>> the devicetree specification, which says
>>
>> The name of a node should be somewhat generic, reflecting
>> the function of the device and not its precise programming model.
>>
>> If devicetree and CCF maintainers are fine with 'rcc', I won't object,
>> my role is just to emphasize the found issue and recommend to use another
>> and more common name 'clock-controller', it is a simple and fortunately
>> backward compatible change to other ST platforms as well.
> Yes. It should be generic so clock-controller or
> clock-reset-controller is appropriate here.
>
ok i will change order... reset-clock-controller name to match with rcc.
Best Regards
Gabriel
Re: [PATCH v7 3/3] clk: stm32h7: Add stm32h743 clock driver
Hi Vladimir,
On 07/19/2017 11:20 PM, Vladimir Zapolskiy wrote:
> Hello Gabriel,
>
> On 07/19/2017 05:25 PM, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> for MFD changes:
>> Acked-by: Lee Jones <lee.jo...@linaro.org>
>>
>> for DT-Bindings
>> Acked-by: Rob Herring <r...@kernel.org>
>> ---
>> .../devicetree/bindings/clock/st,stm32h7-rcc.txt | 82 ++
> I'll provide some review comments about device tree bindings only.
>
>> drivers/clk/Makefile |1 +
>> drivers/clk/clk-stm32h7.c | 1409
>>
>> include/dt-bindings/clock/stm32h7-clks.h | 165 +++
>> include/dt-bindings/mfd/stm32h7-rcc.h | 136 ++
>> 5 files changed, 1793 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> create mode 100644 drivers/clk/clk-stm32h7.c
>> create mode 100644 include/dt-bindings/clock/stm32h7-clks.h
>> create mode 100644 include/dt-bindings/mfd/stm32h7-rcc.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> new file mode 100644
>> index 000..442c50c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> @@ -0,0 +1,82 @@
>> +STMicroelectronics STM32H7 Reset and Clock Controller
>> +=
>> +
>> +The RCC IP is both a reset and a clock controller.
>> +
>> +Please refer to clock-bindings.txt for common clock controller binding
>> usage.
>> +Please also refer to reset.txt for common reset controller binding usage.
>> +
>> +Required properties:
>> +- compatible: Should be:
>> + "st,stm32h743-rcc"
>> +
>> +- reg: should be register base and length as documented in the
>> + datasheet
>> +
>> +- #reset-cells: 1, see below
>> +
>> +- #clock-cells : from common clock binding; shall be set to 1
>> +
>> +- clocks: External oscillator clock phandle
>> + - high speed external clock signal (HSE)
>> + - low speed external clock signal (LSE)
>> + - external I2S clock (I2S_CKIN)
>> +
>> +Optional properties:
>> +- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
>> + write protection (RTC clock).
>> +
>> +Example:
>> +
>> +rcc: rcc@58024400 {
> 'rcc' as a generic device node name is awkward.
>
> I believe the main function of the device is clock controller (unlikely
> a generic reset controller can be converted into a clock controller),
> the locations of the document and device driver also indicate that
> primarily it is a clock controller, so I suggest to replace device node
> name with 'clock-controller' like below:
I prefer to keep rcc node name, to be coherent with the other ST
platforms (STM32F4/F7)
> rcc: clock-controller@58024400 {
>
>> +#reset-cells = <1>;
>> +#clock-cells = <2>
> Missing trailing semicolon ^^^
ok
> My recommendation is to move #reset-cells and #clock-cells properties
> down after 'reg' or 'clocks' property in the list.
ok
>
>> +compatible = "st,stm32h743-rcc", "st,stm32-rcc";
>> +reg = <0x58024400 0x400>;
>> +clocks = <_hse>, <_lse>, <_i2s_ckin>;
>> +
>> +st,syscfg = <>;
>> +
>> +#address-cells = <1>;
>> +#size-cells = <0>;
> Please drop #address-cells and #size-cells properties completely, from
> the document the device node does not define any children subnodes.
ok
>> +};
>> +
>> +The peripheral clock consumer should specify the desired clock by
>> +having the clock ID in its "clocks" phandle cell.
>> +
>> +All available clocks are defined as preprocessor macros in
>> +dt-bindings/clock/stm32h7-clks.h header and can be used in device
>> +tree sources.
>> +
>> +Example:
>> +
>> +timer5: timer@4c00 {
>> +compatible = "st,stm32-timer";
>> +reg = <0x4c00 0x400>;
>> +interrupts = <50>;
>> +clocks = < TIM5_CK>;
>> +
> Please remote
Re: [PATCH v7 3/3] clk: stm32h7: Add stm32h743 clock driver
Hi Vladimir,
On 07/19/2017 11:20 PM, Vladimir Zapolskiy wrote:
> Hello Gabriel,
>
> On 07/19/2017 05:25 PM, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez
>>
>> for MFD changes:
>> Acked-by: Lee Jones
>>
>> for DT-Bindings
>> Acked-by: Rob Herring
>> ---
>> .../devicetree/bindings/clock/st,stm32h7-rcc.txt | 82 ++
> I'll provide some review comments about device tree bindings only.
>
>> drivers/clk/Makefile |1 +
>> drivers/clk/clk-stm32h7.c | 1409
>>
>> include/dt-bindings/clock/stm32h7-clks.h | 165 +++
>> include/dt-bindings/mfd/stm32h7-rcc.h | 136 ++
>> 5 files changed, 1793 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> create mode 100644 drivers/clk/clk-stm32h7.c
>> create mode 100644 include/dt-bindings/clock/stm32h7-clks.h
>> create mode 100644 include/dt-bindings/mfd/stm32h7-rcc.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> new file mode 100644
>> index 000..442c50c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> @@ -0,0 +1,82 @@
>> +STMicroelectronics STM32H7 Reset and Clock Controller
>> +=
>> +
>> +The RCC IP is both a reset and a clock controller.
>> +
>> +Please refer to clock-bindings.txt for common clock controller binding
>> usage.
>> +Please also refer to reset.txt for common reset controller binding usage.
>> +
>> +Required properties:
>> +- compatible: Should be:
>> + "st,stm32h743-rcc"
>> +
>> +- reg: should be register base and length as documented in the
>> + datasheet
>> +
>> +- #reset-cells: 1, see below
>> +
>> +- #clock-cells : from common clock binding; shall be set to 1
>> +
>> +- clocks: External oscillator clock phandle
>> + - high speed external clock signal (HSE)
>> + - low speed external clock signal (LSE)
>> + - external I2S clock (I2S_CKIN)
>> +
>> +Optional properties:
>> +- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
>> + write protection (RTC clock).
>> +
>> +Example:
>> +
>> +rcc: rcc@58024400 {
> 'rcc' as a generic device node name is awkward.
>
> I believe the main function of the device is clock controller (unlikely
> a generic reset controller can be converted into a clock controller),
> the locations of the document and device driver also indicate that
> primarily it is a clock controller, so I suggest to replace device node
> name with 'clock-controller' like below:
I prefer to keep rcc node name, to be coherent with the other ST
platforms (STM32F4/F7)
> rcc: clock-controller@58024400 {
>
>> +#reset-cells = <1>;
>> +#clock-cells = <2>
> Missing trailing semicolon ^^^
ok
> My recommendation is to move #reset-cells and #clock-cells properties
> down after 'reg' or 'clocks' property in the list.
ok
>
>> +compatible = "st,stm32h743-rcc", "st,stm32-rcc";
>> +reg = <0x58024400 0x400>;
>> +clocks = <_hse>, <_lse>, <_i2s_ckin>;
>> +
>> +st,syscfg = <>;
>> +
>> +#address-cells = <1>;
>> +#size-cells = <0>;
> Please drop #address-cells and #size-cells properties completely, from
> the document the device node does not define any children subnodes.
ok
>> +};
>> +
>> +The peripheral clock consumer should specify the desired clock by
>> +having the clock ID in its "clocks" phandle cell.
>> +
>> +All available clocks are defined as preprocessor macros in
>> +dt-bindings/clock/stm32h7-clks.h header and can be used in device
>> +tree sources.
>> +
>> +Example:
>> +
>> +timer5: timer@4c00 {
>> +compatible = "st,stm32-timer";
>> +reg = <0x4c00 0x400>;
>> +interrupts = <50>;
>> +clocks = < TIM5_CK>;
>> +
> Please remote the empty line above.
ok
>> +};
>> +
>> +Spec
Re: [PATCH v6 3/3] clk: stm32h7: Add stm32h743 clock driver
Hi Vladimi,
Many thanks for the code review
On 07/18/2017 10:19 PM, Vladimir Zapolskiy wrote:
> Hello Gabriel,
>
> On 07/18/2017 10:53 AM, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> for MFD changes:
>> Acked-by: Lee Jones <lee.jo...@linaro.org>
>>
>> for DT-Bindings
>> Acked-by: Rob Herring <r...@kernel.org>
>> ---
>> .../devicetree/bindings/clock/st,stm32h7-rcc.txt | 81 ++
>> drivers/clk/Makefile |1 +
>> drivers/clk/clk-stm32h7.c | 1522
>>
>> include/dt-bindings/clock/stm32h7-clks.h | 165 +++
>> include/dt-bindings/mfd/stm32h7-rcc.h | 136 ++
>> 5 files changed, 1905 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> create mode 100644 drivers/clk/clk-stm32h7.c
>> create mode 100644 include/dt-bindings/clock/stm32h7-clks.h
>> create mode 100644 include/dt-bindings/mfd/stm32h7-rcc.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> new file mode 100644
>> index 000..e41e4ac
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> @@ -0,0 +1,81 @@
>> +STMicroelectronics STM32H7 Reset and Clock Controller
>> +=
>> +
>> +The RCC IP is both a reset and a clock controller.
>> +
>> +Please refer to clock-bindings.txt for common clock controller binding
>> usage.
>> +Please also refer to reset.txt for common reset controller binding usage.
>> +
>> +Required properties:
>> +- compatible: Should be:
>> + "st,stm32h743-rcc"
>> +
>> +- reg: should be register base and length as documented in the
>> + datasheet
>> +
>> +- #reset-cells: 1, see below
>> +
>> +- #clock-cells : from common clock binding; shall be set to 1
>> +
>> +- clocks: External oscillator clock phandle
>> + - high speed external clock signal (HSE)
>> + - low speed external clock signal (LSE)
>> + - external I2S clock (I2S_CKIN)
>> +
>> +- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
>> + write protection (RTC clock).
>> +
> please make a clear decision if "st,syscfg" property is mandatory or not.
> From the driver code this property is optional, and the clock driver
> is expected to work properly, if the property is omitted. Do I miss
> anything?
You right, in the driver code it's optional.
I will change it in dt binding documentation.
>
>> diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
>> new file mode 100644
>> index 000..2608c40
>> --- /dev/null
>> +++ b/drivers/clk/clk-stm32h7.c
> [snip]
>
>> +static const char * const ltdc_src[] = {"pll3_r"};
>> +
>> +/* Power domain helper */
>> +static inline void disable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (1 << 8));
>> +}
>> +
>> +static inline void enable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (0 << 8));
> (0 << 8) is zero.
ok
>
>> +}
> IMHO a version below is better:
>
> static inline void enable_power_domain_write_protection(bool enable)
> {
> if (pdrm)
> regmap_update_bits(pdrm, 0x00, BIT(8), enable ? 0x0: BIT(8));
> }
>
>> +
>> +static inline bool is_enable_power_domain_write_protection(void)
> is_enabled_...
ok
>> +{
>> +if (pdrm) {
>> +u32 val;
>> +
>> +regmap_read(pdrm, 0x00, );
>> +
>> +return !(val & 0x100);
>> +}
>> +return 0;
>> +}
> Please replace (1 << 8) and 0x100 all above with a macro or at least
> BIT(8).
ok
>> +
>> +/* Gate clock with ready bit and backup domain management */
>> +struct stm32_ready_gate {
>> +struct clk_gate gate;
>> +u8 bit_rdy;
>> +u8 backup_domain;
>> +};
>> +
>> +#define to_ready_gate_clk(_rgate) container_of(_rgate, struct
>> stm32_ready_gate,\
>> +gate)
Re: [PATCH v6 3/3] clk: stm32h7: Add stm32h743 clock driver
Hi Vladimi,
Many thanks for the code review
On 07/18/2017 10:19 PM, Vladimir Zapolskiy wrote:
> Hello Gabriel,
>
> On 07/18/2017 10:53 AM, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez
>>
>> for MFD changes:
>> Acked-by: Lee Jones
>>
>> for DT-Bindings
>> Acked-by: Rob Herring
>> ---
>> .../devicetree/bindings/clock/st,stm32h7-rcc.txt | 81 ++
>> drivers/clk/Makefile |1 +
>> drivers/clk/clk-stm32h7.c | 1522
>>
>> include/dt-bindings/clock/stm32h7-clks.h | 165 +++
>> include/dt-bindings/mfd/stm32h7-rcc.h | 136 ++
>> 5 files changed, 1905 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> create mode 100644 drivers/clk/clk-stm32h7.c
>> create mode 100644 include/dt-bindings/clock/stm32h7-clks.h
>> create mode 100644 include/dt-bindings/mfd/stm32h7-rcc.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> new file mode 100644
>> index 000..e41e4ac
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
>> @@ -0,0 +1,81 @@
>> +STMicroelectronics STM32H7 Reset and Clock Controller
>> +=
>> +
>> +The RCC IP is both a reset and a clock controller.
>> +
>> +Please refer to clock-bindings.txt for common clock controller binding
>> usage.
>> +Please also refer to reset.txt for common reset controller binding usage.
>> +
>> +Required properties:
>> +- compatible: Should be:
>> + "st,stm32h743-rcc"
>> +
>> +- reg: should be register base and length as documented in the
>> + datasheet
>> +
>> +- #reset-cells: 1, see below
>> +
>> +- #clock-cells : from common clock binding; shall be set to 1
>> +
>> +- clocks: External oscillator clock phandle
>> + - high speed external clock signal (HSE)
>> + - low speed external clock signal (LSE)
>> + - external I2S clock (I2S_CKIN)
>> +
>> +- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
>> + write protection (RTC clock).
>> +
> please make a clear decision if "st,syscfg" property is mandatory or not.
> From the driver code this property is optional, and the clock driver
> is expected to work properly, if the property is omitted. Do I miss
> anything?
You right, in the driver code it's optional.
I will change it in dt binding documentation.
>
>> diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
>> new file mode 100644
>> index 000..2608c40
>> --- /dev/null
>> +++ b/drivers/clk/clk-stm32h7.c
> [snip]
>
>> +static const char * const ltdc_src[] = {"pll3_r"};
>> +
>> +/* Power domain helper */
>> +static inline void disable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (1 << 8));
>> +}
>> +
>> +static inline void enable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (0 << 8));
> (0 << 8) is zero.
ok
>
>> +}
> IMHO a version below is better:
>
> static inline void enable_power_domain_write_protection(bool enable)
> {
> if (pdrm)
> regmap_update_bits(pdrm, 0x00, BIT(8), enable ? 0x0: BIT(8));
> }
>
>> +
>> +static inline bool is_enable_power_domain_write_protection(void)
> is_enabled_...
ok
>> +{
>> +if (pdrm) {
>> +u32 val;
>> +
>> +regmap_read(pdrm, 0x00, );
>> +
>> +return !(val & 0x100);
>> +}
>> +return 0;
>> +}
> Please replace (1 << 8) and 0x100 all above with a macro or at least
> BIT(8).
ok
>> +
>> +/* Gate clock with ready bit and backup domain management */
>> +struct stm32_ready_gate {
>> +struct clk_gate gate;
>> +u8 bit_rdy;
>> +u8 backup_domain;
>> +};
>> +
>> +#define to_ready_gate_clk(_rgate) container_of(_rgate, struct
>> stm32_ready_gate,\
>> +gate)
>> +
>> +#define RGATE_TIMEOUT 1
>> +
>> +static int ready_gate_clk_enable(struct clk_hw *h
Re: [PATCH v6 1/3] clk: nxp: clk-lpc32xx: rename clk_gate_is_enabled()
Hi Vladimir,
Many thanks for the code review.
On 07/18/2017 09:48 PM, Vladimir Zapolskiy wrote:
> Hello Gabriel,
>
> On 07/18/2017 10:53 AM, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> We need to export clk_gate_is_enabled() from clk framework, then
> first of all let's clarify if you really need to export clk_gate_is_enabled()
> from the CCF.
Yes i really need to export clk_gate_is_enabled()
>> to avoid compilation issue we have to rename clk_gate_is_enabled()
>> in NXP LPC32xx clock driver.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
>> ---
>> drivers/clk/nxp/clk-lpc32xx.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
>> index 5b98ff9..1cc71ad 100644
>> --- a/drivers/clk/nxp/clk-lpc32xx.c
>> +++ b/drivers/clk/nxp/clk-lpc32xx.c
>> @@ -903,7 +903,7 @@ static void clk_gate_disable(struct clk_hw *hw)
>> regmap_update_bits(clk_regmap, clk->reg, mask, val);
>> }
>>
>> -static int clk_gate_is_enabled(struct clk_hw *hw)
>> +static int __clk_gate_is_enabled(struct clk_hw *hw)
>> {
>> struct lpc32xx_clk_gate *clk = to_lpc32xx_gate(hw);
>> u32 val;
>> @@ -918,7 +918,7 @@ static int clk_gate_is_enabled(struct clk_hw *hw)
>> static const struct clk_ops lpc32xx_clk_gate_ops = {
>> .enable = clk_gate_enable,
>> .disable = clk_gate_disable,
>> -.is_enabled = clk_gate_is_enabled,
>> +.is_enabled = __clk_gate_is_enabled,
> In case if this change gets continuation, here I want to see the same
> prefixes for all functions and no underscores, namely it shall be
> * lpc32xx_clk_gate_enable(),
> * lpc32xx_clk_gate_disable(),
> * lpc32xx_clk_gate_is_enabled().
ok il will use same prefixes for all functions
Best regards
Gabriel.
>> };
>>
>> #define div_mask(width)((1 << (width)) - 1)
>>
> --
> With best wishes,
> Vladimir
Re: [PATCH v6 1/3] clk: nxp: clk-lpc32xx: rename clk_gate_is_enabled()
Hi Vladimir,
Many thanks for the code review.
On 07/18/2017 09:48 PM, Vladimir Zapolskiy wrote:
> Hello Gabriel,
>
> On 07/18/2017 10:53 AM, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez
>>
>> We need to export clk_gate_is_enabled() from clk framework, then
> first of all let's clarify if you really need to export clk_gate_is_enabled()
> from the CCF.
Yes i really need to export clk_gate_is_enabled()
>> to avoid compilation issue we have to rename clk_gate_is_enabled()
>> in NXP LPC32xx clock driver.
>>
>> Signed-off-by: Gabriel Fernandez
>> ---
>> drivers/clk/nxp/clk-lpc32xx.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
>> index 5b98ff9..1cc71ad 100644
>> --- a/drivers/clk/nxp/clk-lpc32xx.c
>> +++ b/drivers/clk/nxp/clk-lpc32xx.c
>> @@ -903,7 +903,7 @@ static void clk_gate_disable(struct clk_hw *hw)
>> regmap_update_bits(clk_regmap, clk->reg, mask, val);
>> }
>>
>> -static int clk_gate_is_enabled(struct clk_hw *hw)
>> +static int __clk_gate_is_enabled(struct clk_hw *hw)
>> {
>> struct lpc32xx_clk_gate *clk = to_lpc32xx_gate(hw);
>> u32 val;
>> @@ -918,7 +918,7 @@ static int clk_gate_is_enabled(struct clk_hw *hw)
>> static const struct clk_ops lpc32xx_clk_gate_ops = {
>> .enable = clk_gate_enable,
>> .disable = clk_gate_disable,
>> -.is_enabled = clk_gate_is_enabled,
>> +.is_enabled = __clk_gate_is_enabled,
> In case if this change gets continuation, here I want to see the same
> prefixes for all functions and no underscores, namely it shall be
> * lpc32xx_clk_gate_enable(),
> * lpc32xx_clk_gate_disable(),
> * lpc32xx_clk_gate_is_enabled().
ok il will use same prefixes for all functions
Best regards
Gabriel.
>> };
>>
>> #define div_mask(width)((1 << (width)) - 1)
>>
> --
> With best wishes,
> Vladimir
Re: [PATCH v5 1/2] clk: gate: expose clk_gate_ops::is_enabled
Hi Stephen,
On 07/14/2017 08:52 PM, kbuild test robot wrote:
> Hi Gabriel,
>
> [auto build test ERROR on clk/clk-next]
> [also build test ERROR on v4.12 next-20170714]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/gabriel-fernandez-st-com/clk-stm32h7-Add-stm32h743-clock-driver/20170714-170518
> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> config: arm-lpc32xx_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget
> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm
>
> All errors (new ones prefixed by >>):
>
>>> drivers/clk/nxp/clk-lpc32xx.c:906:12: error: static declaration of
>>> 'clk_gate_is_enabled' follows non-static declaration
> static int clk_gate_is_enabled(struct clk_hw *hw)
> ^~~
> In file included from drivers/clk/nxp/clk-lpc32xx.c:13:0:
> include/linux/clk-provider.h:346:5: note: previous declaration of
> 'clk_gate_is_enabled' was here
> int clk_gate_is_enabled(struct clk_hw *hw);
> ^~~
>
> vim +/clk_gate_is_enabled +906 drivers/clk/nxp/clk-lpc32xx.c
>
> f7c82a60 Vladimir Zapolskiy 2015-12-06 905
> f7c82a60 Vladimir Zapolskiy 2015-12-06 @906 static int
> clk_gate_is_enabled(struct clk_hw *hw)
> f7c82a60 Vladimir Zapolskiy 2015-12-06 907 {
> f7c82a60 Vladimir Zapolskiy 2015-12-06 908 struct lpc32xx_clk_gate *clk =
> to_lpc32xx_gate(hw);
> f7c82a60 Vladimir Zapolskiy 2015-12-06 909 u32 val;
> f7c82a60 Vladimir Zapolskiy 2015-12-06 910 bool is_set;
> f7c82a60 Vladimir Zapolskiy 2015-12-06 911
> f7c82a60 Vladimir Zapolskiy 2015-12-06 912 regmap_read(clk_regmap,
> clk->reg, );
> f7c82a60 Vladimir Zapolskiy 2015-12-06 913 is_set = val &
> BIT(clk->bit_idx);
> f7c82a60 Vladimir Zapolskiy 2015-12-06 914
> f7c82a60 Vladimir Zapolskiy 2015-12-06 915 return (clk->flags &
> CLK_GATE_SET_TO_DISABLE ? !is_set : is_set);
> f7c82a60 Vladimir Zapolskiy 2015-12-06 916 }
> f7c82a60 Vladimir Zapolskiy 2015-12-06 917
> EXPORT_SYMBOL_GPL(__clk_gate_is_enabled);
>
>
> :: The code at line 906 was first introduced by commit
> :: f7c82a60ba26c2f003662bcb2cff131021c1e828 clk: lpc32xx: add common
> clock framework driver
>
> :: TO: Vladimir Zapolskiy <v...@mleia.com>
> :: CC: Michael Turquette <mturque...@baylibre.com>
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
Rename 'clk_gate_is_enabled' into'__clk_gate_is_enabled' from clk-gate.c
file, is it a good solution for you ?
i could add also EXPORT_SYMBOL_GPL(__clk_gate_is_enabled) if you are ok.
Best Regards
Gabriel
Re: [PATCH v5 1/2] clk: gate: expose clk_gate_ops::is_enabled
Hi Stephen,
On 07/14/2017 08:52 PM, kbuild test robot wrote:
> Hi Gabriel,
>
> [auto build test ERROR on clk/clk-next]
> [also build test ERROR on v4.12 next-20170714]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/gabriel-fernandez-st-com/clk-stm32h7-Add-stm32h743-clock-driver/20170714-170518
> base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> config: arm-lpc32xx_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget
> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm
>
> All errors (new ones prefixed by >>):
>
>>> drivers/clk/nxp/clk-lpc32xx.c:906:12: error: static declaration of
>>> 'clk_gate_is_enabled' follows non-static declaration
> static int clk_gate_is_enabled(struct clk_hw *hw)
> ^~~
> In file included from drivers/clk/nxp/clk-lpc32xx.c:13:0:
> include/linux/clk-provider.h:346:5: note: previous declaration of
> 'clk_gate_is_enabled' was here
> int clk_gate_is_enabled(struct clk_hw *hw);
> ^~~
>
> vim +/clk_gate_is_enabled +906 drivers/clk/nxp/clk-lpc32xx.c
>
> f7c82a60 Vladimir Zapolskiy 2015-12-06 905
> f7c82a60 Vladimir Zapolskiy 2015-12-06 @906 static int
> clk_gate_is_enabled(struct clk_hw *hw)
> f7c82a60 Vladimir Zapolskiy 2015-12-06 907 {
> f7c82a60 Vladimir Zapolskiy 2015-12-06 908 struct lpc32xx_clk_gate *clk =
> to_lpc32xx_gate(hw);
> f7c82a60 Vladimir Zapolskiy 2015-12-06 909 u32 val;
> f7c82a60 Vladimir Zapolskiy 2015-12-06 910 bool is_set;
> f7c82a60 Vladimir Zapolskiy 2015-12-06 911
> f7c82a60 Vladimir Zapolskiy 2015-12-06 912 regmap_read(clk_regmap,
> clk->reg, );
> f7c82a60 Vladimir Zapolskiy 2015-12-06 913 is_set = val &
> BIT(clk->bit_idx);
> f7c82a60 Vladimir Zapolskiy 2015-12-06 914
> f7c82a60 Vladimir Zapolskiy 2015-12-06 915 return (clk->flags &
> CLK_GATE_SET_TO_DISABLE ? !is_set : is_set);
> f7c82a60 Vladimir Zapolskiy 2015-12-06 916 }
> f7c82a60 Vladimir Zapolskiy 2015-12-06 917
> EXPORT_SYMBOL_GPL(__clk_gate_is_enabled);
>
>
> :: The code at line 906 was first introduced by commit
> :: f7c82a60ba26c2f003662bcb2cff131021c1e828 clk: lpc32xx: add common
> clock framework driver
>
> :: TO: Vladimir Zapolskiy
> :: CC: Michael Turquette
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
Rename 'clk_gate_is_enabled' into'__clk_gate_is_enabled' from clk-gate.c
file, is it a good solution for you ?
i could add also EXPORT_SYMBOL_GPL(__clk_gate_is_enabled) if you are ok.
Best Regards
Gabriel
Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver
On 06/30/2017 08:54 PM, Stephen Boyd wrote: > On 06/30, Gabriel FERNANDEZ wrote: >> >> On 06/30/2017 02:20 AM, Stephen Boyd wrote: >>> On 06/29, Gabriel FERNANDEZ wrote: >>>> On 06/28/2017 05:59 PM, Stephen Boyd wrote: >>>>> On 06/27, Gabriel FERNANDEZ wrote: >>>>>> On 06/22/2017 12:07 AM, Stephen Boyd wrote: >>>>>>> readl_poll_timeout? >>>>>>> >>>>>> if i use readl_poll_timeout (wich use 'ktime_get()') it can be >>>>>> operational only after the selection of clocksource ? (device_initcall). >>>>>> And then if a driver turn on a clock before, it could blocked the linux >>>>>> console ? >>>>>> >>>>> Ok. I wonder if we could add some sort of starting check to >>>>> readl_poll_timeout() that tests system_state for booting vs. >>>>> scheduling? That should be sufficient to handle this case? >>>>> >>>> Oops i think i understood my problem... >>>> i used readl_poll_timeout in atomic context. >>>> I have to move my code in the .prepare ops. >>>> >>>> If you are ok with that i will send a v5 >>>> >>> There's readl_poll_timeout_atomic() for those modes. >>> >> yes it's exactly the test i made (use 'readl_poll_timeout()_atomic' in >> .enable ops) but i'm blocked. >> >> if i do the same in .prepare ops with 'readl_poll_timeout()' it's ok. > I'm still confused. readl_poll_timeout_atomic() uses ktime_get(), > and so does readl_poll_timeout(), so how does moving to the > prepare op fix the problem? What's the actual problem? > Yes both use ktime_get(). The issue concerns internal/external oscillator clocks (time stabilization could be long) and if a driver wants to enable one these clocks before device_initcall(). By default the clocksource is the jiffies until the end of the boot (fs_initcall) Then after, the best clocksource is selected (arm_system_timer or stm32 timer, etc...) There is no problem after because the counter is a hardware register. But the jiffies counter is incremented by interruption and enable op does not allow to be interrupted. (we can with prepare op).
Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver
On 06/30/2017 08:54 PM, Stephen Boyd wrote: > On 06/30, Gabriel FERNANDEZ wrote: >> >> On 06/30/2017 02:20 AM, Stephen Boyd wrote: >>> On 06/29, Gabriel FERNANDEZ wrote: >>>> On 06/28/2017 05:59 PM, Stephen Boyd wrote: >>>>> On 06/27, Gabriel FERNANDEZ wrote: >>>>>> On 06/22/2017 12:07 AM, Stephen Boyd wrote: >>>>>>> readl_poll_timeout? >>>>>>> >>>>>> if i use readl_poll_timeout (wich use 'ktime_get()') it can be >>>>>> operational only after the selection of clocksource ? (device_initcall). >>>>>> And then if a driver turn on a clock before, it could blocked the linux >>>>>> console ? >>>>>> >>>>> Ok. I wonder if we could add some sort of starting check to >>>>> readl_poll_timeout() that tests system_state for booting vs. >>>>> scheduling? That should be sufficient to handle this case? >>>>> >>>> Oops i think i understood my problem... >>>> i used readl_poll_timeout in atomic context. >>>> I have to move my code in the .prepare ops. >>>> >>>> If you are ok with that i will send a v5 >>>> >>> There's readl_poll_timeout_atomic() for those modes. >>> >> yes it's exactly the test i made (use 'readl_poll_timeout()_atomic' in >> .enable ops) but i'm blocked. >> >> if i do the same in .prepare ops with 'readl_poll_timeout()' it's ok. > I'm still confused. readl_poll_timeout_atomic() uses ktime_get(), > and so does readl_poll_timeout(), so how does moving to the > prepare op fix the problem? What's the actual problem? > Yes both use ktime_get(). The issue concerns internal/external oscillator clocks (time stabilization could be long) and if a driver wants to enable one these clocks before device_initcall(). By default the clocksource is the jiffies until the end of the boot (fs_initcall) Then after, the best clocksource is selected (arm_system_timer or stm32 timer, etc...) There is no problem after because the counter is a hardware register. But the jiffies counter is incremented by interruption and enable op does not allow to be interrupted. (we can with prepare op).
Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver
On 06/30/2017 02:20 AM, Stephen Boyd wrote: > On 06/29, Gabriel FERNANDEZ wrote: >> >> On 06/28/2017 05:59 PM, Stephen Boyd wrote: >>> On 06/27, Gabriel FERNANDEZ wrote: >>>> On 06/22/2017 12:07 AM, Stephen Boyd wrote: >>>>> readl_poll_timeout? >>>>> >>>> if i use readl_poll_timeout (wich use 'ktime_get()') it can be >>>> operational only after the selection of clocksource ? (device_initcall). >>>> And then if a driver turn on a clock before, it could blocked the linux >>>> console ? >>>> >>> Ok. I wonder if we could add some sort of starting check to >>> readl_poll_timeout() that tests system_state for booting vs. >>> scheduling? That should be sufficient to handle this case? >>> >> Oops i think i understood my problem... >> i used readl_poll_timeout in atomic context. >> I have to move my code in the .prepare ops. >> >> If you are ok with that i will send a v5 >> > There's readl_poll_timeout_atomic() for those modes. > yes it's exactly the test i made (use 'readl_poll_timeout()_atomic' in .enable ops) but i'm blocked. if i do the same in .prepare ops with 'readl_poll_timeout()' it's ok.
Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver
On 06/30/2017 02:20 AM, Stephen Boyd wrote: > On 06/29, Gabriel FERNANDEZ wrote: >> >> On 06/28/2017 05:59 PM, Stephen Boyd wrote: >>> On 06/27, Gabriel FERNANDEZ wrote: >>>> On 06/22/2017 12:07 AM, Stephen Boyd wrote: >>>>> readl_poll_timeout? >>>>> >>>> if i use readl_poll_timeout (wich use 'ktime_get()') it can be >>>> operational only after the selection of clocksource ? (device_initcall). >>>> And then if a driver turn on a clock before, it could blocked the linux >>>> console ? >>>> >>> Ok. I wonder if we could add some sort of starting check to >>> readl_poll_timeout() that tests system_state for booting vs. >>> scheduling? That should be sufficient to handle this case? >>> >> Oops i think i understood my problem... >> i used readl_poll_timeout in atomic context. >> I have to move my code in the .prepare ops. >> >> If you are ok with that i will send a v5 >> > There's readl_poll_timeout_atomic() for those modes. > yes it's exactly the test i made (use 'readl_poll_timeout()_atomic' in .enable ops) but i'm blocked. if i do the same in .prepare ops with 'readl_poll_timeout()' it's ok.
Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver
On 06/28/2017 05:59 PM, Stephen Boyd wrote: > On 06/27, Gabriel FERNANDEZ wrote: >> >> On 06/22/2017 12:07 AM, Stephen Boyd wrote: >>> readl_poll_timeout? >>> >> if i use readl_poll_timeout (wich use 'ktime_get()') it can be >> operational only after the selection of clocksource ? (device_initcall). >> And then if a driver turn on a clock before, it could blocked the linux >> console ? >> > Ok. I wonder if we could add some sort of starting check to > readl_poll_timeout() that tests system_state for booting vs. > scheduling? That should be sufficient to handle this case? > Oops i think i understood my problem... i used readl_poll_timeout in atomic context. I have to move my code in the .prepare ops. If you are ok with that i will send a v5 Thanks Gabriel
Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver
On 06/28/2017 05:59 PM, Stephen Boyd wrote: > On 06/27, Gabriel FERNANDEZ wrote: >> >> On 06/22/2017 12:07 AM, Stephen Boyd wrote: >>> readl_poll_timeout? >>> >> if i use readl_poll_timeout (wich use 'ktime_get()') it can be >> operational only after the selection of clocksource ? (device_initcall). >> And then if a driver turn on a clock before, it could blocked the linux >> console ? >> > Ok. I wonder if we could add some sort of starting check to > readl_poll_timeout() that tests system_state for booting vs. > scheduling? That should be sufficient to handle this case? > Oops i think i understood my problem... i used readl_poll_timeout in atomic context. I have to move my code in the .prepare ops. If you are ok with that i will send a v5 Thanks Gabriel
Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver
On 06/22/2017 12:07 AM, Stephen Boyd wrote:
> On 06/07, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> for MFD changes:
>> Acked-by: Lee Jones <lee.jo...@linaro.org>
>>
>> for DT-Bindings
>> Acked-by: Rob Herring <r...@kernel.org>
>> v4:
>>- rename lock into stm32rcc_lock
>>- don't use clk_readl()
>>- remove useless parentheses with GENMASK
>>- fix parents of timer_x clocks
>>- suppress pll configuration from DT
>>- fix kbuild warning
>>
>> v3:
>>- fix compatible string "stm32h7-pll" into "st,stm32h7-pll"
>>- fix bad parent name for mco2 clock
>>- set CLK_SET_RATE_PARENT for ltdc clock
>>- set CLK_IGNORE_UNUSED for pll1
>>- disable power domain write protection on disable ops if needed
>>
>>
>> v2:
>>- rename compatible string "stm32,pll" into "stm32h7-pll"
>>- suppress "st,pllrge" property
>>- suppress "st, frac-status" property
>>- change management of "st,frac" property
>> 0 : enable 0 pll integer mode
>> other values : enable pll in fractional mode (value is
>> the fractional factor)
> Please drop the changelog from commit text.
>
>> diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
>> new file mode 100644
>> index 000..2907c1f
>> --- /dev/null
>> +++ b/drivers/clk/clk-stm32h7.c
>> @@ -0,0 +1,1532 @@
>> +/* Power domain helper */
>> +static inline void disable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (1 << 8));
>> +}
>> +
>> +static inline void enable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (0 << 8));
>> +}
>> +
>> +static inline int is_enable_power_domain_write_protection(void)
> Return bool, not int?
>
>> +{
>> +if (pdrm) {
>> +u32 val;
>> +
>> +regmap_read(pdrm, 0x00, );
>> +
>> +return !(val & 0x100);
>> +}
>> +return -1;
> Returning -1 looks odd.
>
>> +}
>> +
>> +/* Gate clock with ready bit and backup domain management */
>> +struct stm32_ready_gate {
>> +struct clk_gate gate;
>> +u8 bit_rdy;
>> +u8 backup_domain;
>> +};
>> +
>> +#define to_ready_gate_clk(_rgate) container_of(_rgate, struct
>> stm32_ready_gate,\
>> +gate)
>> +
>> +#define RGATE_TIMEOUT 60
>> +
>> +static int ready_gate_clk_is_enabled(struct clk_hw *hw)
>> +{
>> +return clk_gate_ops.is_enabled(hw);
>> +}
> Perhaps we should expose clk_gate_ops::is_enabled as functions
> that can be directly called and assigned in places like this so
> we don't need wrapper functions that do nothing besides forward
> the call.
>
>> +
>> +static int ready_gate_clk_enable(struct clk_hw *hw)
>> +{
>> +struct clk_gate *gate = to_clk_gate(hw);
>> +struct stm32_ready_gate *rgate = to_ready_gate_clk(gate);
>> +int dbp_status;
>> +int bit_status;
>> +unsigned int timeout = RGATE_TIMEOUT;
>> +
>> +if (clk_gate_ops.is_enabled(hw))
>> +return 0;
>> +
>> +dbp_status = is_enable_power_domain_write_protection();
>> +
>> +if (rgate->backup_domain && dbp_status)
>> +disable_power_domain_write_protection();
>> +
>> +clk_gate_ops.enable(hw);
>> +
>> +do {
>> +bit_status = !(readl(gate->reg) & BIT(rgate->bit_rdy));
>> +
>> +if (bit_status)
>> +udelay(1000);
>> +
>> +} while (bit_status && --timeout);
> readl_poll_timeout?
if i use readl_poll_timeout (wich use 'ktime_get()') it can be
operational only after the selection of clocksource ? (device_initcall).
And then if a driver turn on a clock before, it could blocked the linux
console ?
>
>> +
>> +/* RTC clock */
>> +static u8 rtc_mux_get_parent(struct clk_hw *hw)
>> +{
>> +return clk_mux_ops.get_parent(hw);
>> +}
>> +
>> +static int rtc_mux_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> +int dbp_status
Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver
On 06/22/2017 12:07 AM, Stephen Boyd wrote:
> On 06/07, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez
>>
>> for MFD changes:
>> Acked-by: Lee Jones
>>
>> for DT-Bindings
>> Acked-by: Rob Herring
>> v4:
>>- rename lock into stm32rcc_lock
>>- don't use clk_readl()
>>- remove useless parentheses with GENMASK
>>- fix parents of timer_x clocks
>>- suppress pll configuration from DT
>>- fix kbuild warning
>>
>> v3:
>>- fix compatible string "stm32h7-pll" into "st,stm32h7-pll"
>>- fix bad parent name for mco2 clock
>>- set CLK_SET_RATE_PARENT for ltdc clock
>>- set CLK_IGNORE_UNUSED for pll1
>>- disable power domain write protection on disable ops if needed
>>
>>
>> v2:
>>- rename compatible string "stm32,pll" into "stm32h7-pll"
>>- suppress "st,pllrge" property
>>- suppress "st, frac-status" property
>>- change management of "st,frac" property
>> 0 : enable 0 pll integer mode
>> other values : enable pll in fractional mode (value is
>> the fractional factor)
> Please drop the changelog from commit text.
>
>> diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
>> new file mode 100644
>> index 000..2907c1f
>> --- /dev/null
>> +++ b/drivers/clk/clk-stm32h7.c
>> @@ -0,0 +1,1532 @@
>> +/* Power domain helper */
>> +static inline void disable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (1 << 8));
>> +}
>> +
>> +static inline void enable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (0 << 8));
>> +}
>> +
>> +static inline int is_enable_power_domain_write_protection(void)
> Return bool, not int?
>
>> +{
>> +if (pdrm) {
>> +u32 val;
>> +
>> +regmap_read(pdrm, 0x00, );
>> +
>> +return !(val & 0x100);
>> +}
>> +return -1;
> Returning -1 looks odd.
>
>> +}
>> +
>> +/* Gate clock with ready bit and backup domain management */
>> +struct stm32_ready_gate {
>> +struct clk_gate gate;
>> +u8 bit_rdy;
>> +u8 backup_domain;
>> +};
>> +
>> +#define to_ready_gate_clk(_rgate) container_of(_rgate, struct
>> stm32_ready_gate,\
>> +gate)
>> +
>> +#define RGATE_TIMEOUT 60
>> +
>> +static int ready_gate_clk_is_enabled(struct clk_hw *hw)
>> +{
>> +return clk_gate_ops.is_enabled(hw);
>> +}
> Perhaps we should expose clk_gate_ops::is_enabled as functions
> that can be directly called and assigned in places like this so
> we don't need wrapper functions that do nothing besides forward
> the call.
>
>> +
>> +static int ready_gate_clk_enable(struct clk_hw *hw)
>> +{
>> +struct clk_gate *gate = to_clk_gate(hw);
>> +struct stm32_ready_gate *rgate = to_ready_gate_clk(gate);
>> +int dbp_status;
>> +int bit_status;
>> +unsigned int timeout = RGATE_TIMEOUT;
>> +
>> +if (clk_gate_ops.is_enabled(hw))
>> +return 0;
>> +
>> +dbp_status = is_enable_power_domain_write_protection();
>> +
>> +if (rgate->backup_domain && dbp_status)
>> +disable_power_domain_write_protection();
>> +
>> +clk_gate_ops.enable(hw);
>> +
>> +do {
>> +bit_status = !(readl(gate->reg) & BIT(rgate->bit_rdy));
>> +
>> +if (bit_status)
>> +udelay(1000);
>> +
>> +} while (bit_status && --timeout);
> readl_poll_timeout?
if i use readl_poll_timeout (wich use 'ktime_get()') it can be
operational only after the selection of clocksource ? (device_initcall).
And then if a driver turn on a clock before, it could blocked the linux
console ?
>
>> +
>> +/* RTC clock */
>> +static u8 rtc_mux_get_parent(struct clk_hw *hw)
>> +{
>> +return clk_mux_ops.get_parent(hw);
>> +}
>> +
>> +static int rtc_mux_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> +int dbp_status;
>> +int err;
>> +
>> +dbp_status = is_enable_power_domain_write_protection();
>&g
Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver
Hi Stephen,
Thanks for reviewing.
On 06/22/2017 12:07 AM, Stephen Boyd wrote:
> On 06/07, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
>>
>> for MFD changes:
>> Acked-by: Lee Jones <lee.jo...@linaro.org>
>>
>> for DT-Bindings
>> Acked-by: Rob Herring <r...@kernel.org>
>> v4:
>>- rename lock into stm32rcc_lock
>>- don't use clk_readl()
>>- remove useless parentheses with GENMASK
>>- fix parents of timer_x clocks
>>- suppress pll configuration from DT
>>- fix kbuild warning
>>
>> v3:
>>- fix compatible string "stm32h7-pll" into "st,stm32h7-pll"
>>- fix bad parent name for mco2 clock
>>- set CLK_SET_RATE_PARENT for ltdc clock
>>- set CLK_IGNORE_UNUSED for pll1
>>- disable power domain write protection on disable ops if needed
>>
>>
>> v2:
>>- rename compatible string "stm32,pll" into "stm32h7-pll"
>>- suppress "st,pllrge" property
>>- suppress "st, frac-status" property
>>- change management of "st,frac" property
>> 0 : enable 0 pll integer mode
>> other values : enable pll in fractional mode (value is
>> the fractional factor)
> Please drop the changelog from commit text.
strange, i added the changelog after 'git format-patch'
>
>> diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
>> new file mode 100644
>> index 000..2907c1f
>> --- /dev/null
>> +++ b/drivers/clk/clk-stm32h7.c
>> @@ -0,0 +1,1532 @@
>> +/* Power domain helper */
>> +static inline void disable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (1 << 8));
>> +}
>> +
>> +static inline void enable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (0 << 8));
>> +}
>> +
>> +static inline int is_enable_power_domain_write_protection(void)
> Return bool, not int?
ok
>
>> +{
>> +if (pdrm) {
>> +u32 val;
>> +
>> +regmap_read(pdrm, 0x00, );
>> +
>> +return !(val & 0x100);
>> +}
>> +return -1;
> Returning -1 looks odd.
ok i will change it
>
>> +}
>> +
>> +/* Gate clock with ready bit and backup domain management */
>> +struct stm32_ready_gate {
>> +struct clk_gate gate;
>> +u8 bit_rdy;
>> +u8 backup_domain;
>> +};
>> +
>> +#define to_ready_gate_clk(_rgate) container_of(_rgate, struct
>> stm32_ready_gate,\
>> +gate)
>> +
>> +#define RGATE_TIMEOUT 60
>> +
>> +static int ready_gate_clk_is_enabled(struct clk_hw *hw)
>> +{
>> +return clk_gate_ops.is_enabled(hw);
>> +}
> Perhaps we should expose clk_gate_ops::is_enabled as functions
> that can be directly called and assigned in places like this so
> we don't need wrapper functions that do nothing besides forward
> the call.
ok i will add a patch in clk.c and clk-provider.h to export
'clk_gate_is_enabled'
>
>> +
>> +static int ready_gate_clk_enable(struct clk_hw *hw)
>> +{
>> +struct clk_gate *gate = to_clk_gate(hw);
>> +struct stm32_ready_gate *rgate = to_ready_gate_clk(gate);
>> +int dbp_status;
>> +int bit_status;
>> +unsigned int timeout = RGATE_TIMEOUT;
>> +
>> +if (clk_gate_ops.is_enabled(hw))
>> +return 0;
>> +
>> +dbp_status = is_enable_power_domain_write_protection();
>> +
>> +if (rgate->backup_domain && dbp_status)
>> +disable_power_domain_write_protection();
>> +
>> +clk_gate_ops.enable(hw);
>> +
>> +do {
>> +bit_status = !(readl(gate->reg) & BIT(rgate->bit_rdy));
>> +
>> +if (bit_status)
>> +udelay(1000);
>> +
>> +} while (bit_status && --timeout);
> readl_poll_timeout?
last time it didn't work, i will investigate again
>> +
>> +/* RTC clock */
>> +static u8 rtc_mux_get_parent(struct clk_hw *hw)
>> +{
>> +return clk_mux_ops.get_parent(hw);
>> +}
>> +
>> +static int rtc_mux_set_parent(struct clk_hw *hw, u8 index)
>>
Re: [RESEND PATCH v4] clk: stm32h7: Add stm32h743 clock driver
Hi Stephen,
Thanks for reviewing.
On 06/22/2017 12:07 AM, Stephen Boyd wrote:
> On 06/07, gabriel.fernan...@st.com wrote:
>> From: Gabriel Fernandez
>>
>> This patch enables clocks for STM32H743 boards.
>>
>> Signed-off-by: Gabriel Fernandez
>>
>> for MFD changes:
>> Acked-by: Lee Jones
>>
>> for DT-Bindings
>> Acked-by: Rob Herring
>> v4:
>>- rename lock into stm32rcc_lock
>>- don't use clk_readl()
>>- remove useless parentheses with GENMASK
>>- fix parents of timer_x clocks
>>- suppress pll configuration from DT
>>- fix kbuild warning
>>
>> v3:
>>- fix compatible string "stm32h7-pll" into "st,stm32h7-pll"
>>- fix bad parent name for mco2 clock
>>- set CLK_SET_RATE_PARENT for ltdc clock
>>- set CLK_IGNORE_UNUSED for pll1
>>- disable power domain write protection on disable ops if needed
>>
>>
>> v2:
>>- rename compatible string "stm32,pll" into "stm32h7-pll"
>>- suppress "st,pllrge" property
>>- suppress "st, frac-status" property
>>- change management of "st,frac" property
>> 0 : enable 0 pll integer mode
>> other values : enable pll in fractional mode (value is
>> the fractional factor)
> Please drop the changelog from commit text.
strange, i added the changelog after 'git format-patch'
>
>> diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
>> new file mode 100644
>> index 000..2907c1f
>> --- /dev/null
>> +++ b/drivers/clk/clk-stm32h7.c
>> @@ -0,0 +1,1532 @@
>> +/* Power domain helper */
>> +static inline void disable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (1 << 8));
>> +}
>> +
>> +static inline void enable_power_domain_write_protection(void)
>> +{
>> +if (pdrm)
>> +regmap_update_bits(pdrm, 0x00, (1 << 8), (0 << 8));
>> +}
>> +
>> +static inline int is_enable_power_domain_write_protection(void)
> Return bool, not int?
ok
>
>> +{
>> +if (pdrm) {
>> +u32 val;
>> +
>> +regmap_read(pdrm, 0x00, );
>> +
>> +return !(val & 0x100);
>> +}
>> +return -1;
> Returning -1 looks odd.
ok i will change it
>
>> +}
>> +
>> +/* Gate clock with ready bit and backup domain management */
>> +struct stm32_ready_gate {
>> +struct clk_gate gate;
>> +u8 bit_rdy;
>> +u8 backup_domain;
>> +};
>> +
>> +#define to_ready_gate_clk(_rgate) container_of(_rgate, struct
>> stm32_ready_gate,\
>> +gate)
>> +
>> +#define RGATE_TIMEOUT 60
>> +
>> +static int ready_gate_clk_is_enabled(struct clk_hw *hw)
>> +{
>> +return clk_gate_ops.is_enabled(hw);
>> +}
> Perhaps we should expose clk_gate_ops::is_enabled as functions
> that can be directly called and assigned in places like this so
> we don't need wrapper functions that do nothing besides forward
> the call.
ok i will add a patch in clk.c and clk-provider.h to export
'clk_gate_is_enabled'
>
>> +
>> +static int ready_gate_clk_enable(struct clk_hw *hw)
>> +{
>> +struct clk_gate *gate = to_clk_gate(hw);
>> +struct stm32_ready_gate *rgate = to_ready_gate_clk(gate);
>> +int dbp_status;
>> +int bit_status;
>> +unsigned int timeout = RGATE_TIMEOUT;
>> +
>> +if (clk_gate_ops.is_enabled(hw))
>> +return 0;
>> +
>> +dbp_status = is_enable_power_domain_write_protection();
>> +
>> +if (rgate->backup_domain && dbp_status)
>> +disable_power_domain_write_protection();
>> +
>> +clk_gate_ops.enable(hw);
>> +
>> +do {
>> +bit_status = !(readl(gate->reg) & BIT(rgate->bit_rdy));
>> +
>> +if (bit_status)
>> +udelay(1000);
>> +
>> +} while (bit_status && --timeout);
> readl_poll_timeout?
last time it didn't work, i will investigate again
>> +
>> +/* RTC clock */
>> +static u8 rtc_mux_get_parent(struct clk_hw *hw)
>> +{
>> +return clk_mux_ops.get_parent(hw);
>> +}
>> +
>> +static int rtc_mux_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> +int dbp_status;
>> +int err;
>> +
>> +dbp_status = is_enable_powe
Re: [PATCH] clk: stm32h7: Add stm32h743 clock driver
Hi Stephen
Sorry for delay i was on sick live
On 04/07/2017 09:51 PM, Stephen Boyd wrote:
On 04/06, Gabriel Fernandez wrote:
On 04/06/2017 12:32 AM, Stephen Boyd wrote:
On 03/15, gabriel.fernan...@st.com wrote:
diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
new file mode 100644
index 000..9d4b587
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
@@ -0,0 +1,152 @@
+
+ rcc: rcc@58024400 {
+ #reset-cells = <1>;
+ #clock-cells = <2>
+ compatible = "st,stm32h743-rcc", "st,stm32-rcc";
+ reg = <0x58024400 0x400>;
+ clocks = <_hse>, <_lse>, <_i2s_ckin>;
+
+ st,syscfg = <>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vco1@58024430 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <0>;
reg is super confusing and doesn't match unit address.
ok i fixed it in the v2
+ };
Why? Shouldn't we know this from the compatible string how many
PLLs there are and where they're located? Export the PLLs through
rcc node's clock-cells?
Because i need to offer the possibility to change the PLL VCO
frequencies at the start-up of this driver clock.
The VCO algorithm needs a division factor, a multiplication factor
and a fractional factor.
Lot's of solution are possible for one frequency and it's nightmare
to satisfy the 3 output dividers of the PLL.
Sure, but do we need to configure that on a per-board basis or a
per-SoC basis? If it's just some configuration, I wonder why we
don't put that into the driver and base it off some compatible
string that includes the SoC the device is for.
I prefer to let in first, the responsibility of the boot loader to
change VCO parameters.
Then i propose a new version without DT configuration of PLL's
are you ok for that ?
best regards
Gabriel
To simply
Re: [PATCH] clk: stm32h7: Add stm32h743 clock driver
Hi Stephen
Sorry for delay i was on sick live
On 04/07/2017 09:51 PM, Stephen Boyd wrote:
On 04/06, Gabriel Fernandez wrote:
On 04/06/2017 12:32 AM, Stephen Boyd wrote:
On 03/15, gabriel.fernan...@st.com wrote:
diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
new file mode 100644
index 000..9d4b587
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
@@ -0,0 +1,152 @@
+
+ rcc: rcc@58024400 {
+ #reset-cells = <1>;
+ #clock-cells = <2>
+ compatible = "st,stm32h743-rcc", "st,stm32-rcc";
+ reg = <0x58024400 0x400>;
+ clocks = <_hse>, <_lse>, <_i2s_ckin>;
+
+ st,syscfg = <>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vco1@58024430 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <0>;
reg is super confusing and doesn't match unit address.
ok i fixed it in the v2
+ };
Why? Shouldn't we know this from the compatible string how many
PLLs there are and where they're located? Export the PLLs through
rcc node's clock-cells?
Because i need to offer the possibility to change the PLL VCO
frequencies at the start-up of this driver clock.
The VCO algorithm needs a division factor, a multiplication factor
and a fractional factor.
Lot's of solution are possible for one frequency and it's nightmare
to satisfy the 3 output dividers of the PLL.
Sure, but do we need to configure that on a per-board basis or a
per-SoC basis? If it's just some configuration, I wonder why we
don't put that into the driver and base it off some compatible
string that includes the SoC the device is for.
I prefer to let in first, the responsibility of the boot loader to
change VCO parameters.
Then i propose a new version without DT configuration of PLL's
are you ok for that ?
best regards
Gabriel
To simply
Re: [PATCH] clk: stm32h7: Add stm32h743 clock driver
Hi Stephen,
On 04/06/2017 12:32 AM, Stephen Boyd wrote:
On 03/15, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez <gabriel.fernan...@st.com>
This patch enables clocks for STM32H743 boards.
Like what clocks exactly? All of them?
Yes all of them, it's new IP
diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
new file mode 100644
index 000..9d4b587
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
@@ -0,0 +1,152 @@
+STMicroelectronics STM32H7 Reset and Clock Controller
+=
+
+The RCC IP is both a reset and a clock controller.
+
+Please refer to clock-bindings.txt for common clock controller binding usage.
+Please also refer to reset.txt for common reset controller binding usage.
+
+Required properties:
+- compatible: Should be:
+ "st,stm32h743-rcc"
+
+- reg: should be register base and length as documented in the
+ datasheet
+
+- #reset-cells: 1, see below
+
+- #clock-cells : from common clock binding; shall be set to 1
+
+- clocks: External oscillator clock phandle
+ - high speed external clock signal (HSE)
+ - low speed external clock signal (LSE)
+ - external I2S clock (I2S_CKIN)
+
+- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
+ write protection (RTC clock).
+
+- pll x node: Allow to register a pll with specific parameters.
+ Please see PLL section below.
+
+Example:
+
+ rcc: rcc@58024400 {
+ #reset-cells = <1>;
+ #clock-cells = <2>
+ compatible = "st,stm32h743-rcc", "st,stm32-rcc";
+ reg = <0x58024400 0x400>;
+ clocks = <_hse>, <_lse>, <_i2s_ckin>;
+
+ st,syscfg = <>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vco1@58024430 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <0>;
reg is super confusing and doesn't match unit address.
ok i fixed it in the v2
+ };
Why? Shouldn't we know this from the compatible string how many
PLLs there are and where they're located? Export the PLLs through
rcc node's clock-cells?
Because i need to offer the possibility to change the PLL VCO
frequencies at the start-up of this driver clock.
The VCO algorithm needs a division factor, a multiplication factor and a
fractional factor.
Lot's of solution are possible for one frequency and it's nightmare to
satisfy the 3 output dividers of the PLL.
+
+ vco2@58024438 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <1>;
reg is super confusing and doesn't match unit address.
+ st,clock-div = <2>;
+ st,clock-mult = <40>;
+ st,frac-status = <0>;
+ st,frac = <0>;
+ st,vcosel = <1>;
+ st,pllrge = <2>;
Does this stuff change on a per-board basis? I hope none of these
properties need to be in DT.
These properties are optionals.
I absolute need it to custumize VCO frequencies of a pll without the
boot loader..
i suppressed "st,frac-status" and "st,pllrge" in the v2
+ };
+ };
+
+
+STM32H7 PLL
+---
+
[...]
+
+Specifying softreset control of devices
+===
+
+Device nodes should specify the reset channel required in their "resets"
+property, containing a phandle to the reset device node and an index specifying
+which channel to use.
+The index is the bit number within the RCC registers bank, starting from RCC
+base address.
+It is calculated as: index = register_offset / 4 * 32 + bit_offset.
+Where bit_offset is the bit offset within the register.
+
+For example, for CRC reset:
+ crc = AHB4RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x88 / 4 * 32 + 19 =
1107
+
+All available preprocessor macros for reset are defined
dt-bindings//mfd/stm32h7-rcc.h
One too many slashes?
ok i will fix it
+header and can be used in device tree sources.
+
+example:
+
+ timer2 {
+ resets = < STM32H7_APB1L_RESET(TIM2)>;
+ };
diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
new file mode 100644
index 000..c8eb729
--- /dev/null
+++ b/drivers/clk/clk-stm32h7.c
@@ -0,0 +1,1586 @@
+/*
+ * Copyright (C) Gabriel Fernandez 2017
+ * Author: Gabriel Fernandez <gabriel.fernan...@st.com>
+ *
+ * License terms: GPL V2.0.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU Gen
Re: [PATCH] clk: stm32h7: Add stm32h743 clock driver
Hi Stephen,
On 04/06/2017 12:32 AM, Stephen Boyd wrote:
On 03/15, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez
This patch enables clocks for STM32H743 boards.
Like what clocks exactly? All of them?
Yes all of them, it's new IP
diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
new file mode 100644
index 000..9d4b587
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
@@ -0,0 +1,152 @@
+STMicroelectronics STM32H7 Reset and Clock Controller
+=
+
+The RCC IP is both a reset and a clock controller.
+
+Please refer to clock-bindings.txt for common clock controller binding usage.
+Please also refer to reset.txt for common reset controller binding usage.
+
+Required properties:
+- compatible: Should be:
+ "st,stm32h743-rcc"
+
+- reg: should be register base and length as documented in the
+ datasheet
+
+- #reset-cells: 1, see below
+
+- #clock-cells : from common clock binding; shall be set to 1
+
+- clocks: External oscillator clock phandle
+ - high speed external clock signal (HSE)
+ - low speed external clock signal (LSE)
+ - external I2S clock (I2S_CKIN)
+
+- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
+ write protection (RTC clock).
+
+- pll x node: Allow to register a pll with specific parameters.
+ Please see PLL section below.
+
+Example:
+
+ rcc: rcc@58024400 {
+ #reset-cells = <1>;
+ #clock-cells = <2>
+ compatible = "st,stm32h743-rcc", "st,stm32-rcc";
+ reg = <0x58024400 0x400>;
+ clocks = <_hse>, <_lse>, <_i2s_ckin>;
+
+ st,syscfg = <>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vco1@58024430 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <0>;
reg is super confusing and doesn't match unit address.
ok i fixed it in the v2
+ };
Why? Shouldn't we know this from the compatible string how many
PLLs there are and where they're located? Export the PLLs through
rcc node's clock-cells?
Because i need to offer the possibility to change the PLL VCO
frequencies at the start-up of this driver clock.
The VCO algorithm needs a division factor, a multiplication factor and a
fractional factor.
Lot's of solution are possible for one frequency and it's nightmare to
satisfy the 3 output dividers of the PLL.
+
+ vco2@58024438 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <1>;
reg is super confusing and doesn't match unit address.
+ st,clock-div = <2>;
+ st,clock-mult = <40>;
+ st,frac-status = <0>;
+ st,frac = <0>;
+ st,vcosel = <1>;
+ st,pllrge = <2>;
Does this stuff change on a per-board basis? I hope none of these
properties need to be in DT.
These properties are optionals.
I absolute need it to custumize VCO frequencies of a pll without the
boot loader..
i suppressed "st,frac-status" and "st,pllrge" in the v2
+ };
+ };
+
+
+STM32H7 PLL
+---
+
[...]
+
+Specifying softreset control of devices
+===
+
+Device nodes should specify the reset channel required in their "resets"
+property, containing a phandle to the reset device node and an index specifying
+which channel to use.
+The index is the bit number within the RCC registers bank, starting from RCC
+base address.
+It is calculated as: index = register_offset / 4 * 32 + bit_offset.
+Where bit_offset is the bit offset within the register.
+
+For example, for CRC reset:
+ crc = AHB4RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x88 / 4 * 32 + 19 =
1107
+
+All available preprocessor macros for reset are defined
dt-bindings//mfd/stm32h7-rcc.h
One too many slashes?
ok i will fix it
+header and can be used in device tree sources.
+
+example:
+
+ timer2 {
+ resets = < STM32H7_APB1L_RESET(TIM2)>;
+ };
diff --git a/drivers/clk/clk-stm32h7.c b/drivers/clk/clk-stm32h7.c
new file mode 100644
index 000..c8eb729
--- /dev/null
+++ b/drivers/clk/clk-stm32h7.c
@@ -0,0 +1,1586 @@
+/*
+ * Copyright (C) Gabriel Fernandez 2017
+ * Author: Gabriel Fernandez
+ *
+ * License terms: GPL V2.0.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software F
Re: [PATCH v2] clk: stm32h7: Add stm32h743 clock driver
On 04/03/2017 06:04 PM, Rob Herring wrote: On Mon, Apr 3, 2017 at 9:39 AM, Rob Herring <r...@kernel.org> wrote: On Wed, Mar 29, 2017 at 11:08:22AM +0200, gabriel.fernan...@st.com wrote: From: Gabriel Fernandez <gabriel.fernan...@st.com> This patch enables clocks for STM32H743 boards. Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com> Just for the MFD changes: Acked-by: Lee Jones <lee.jo...@linaro.org> +Required properties for pll node: +- compatible: Should be: + "stm32h7-pll" stm,stm32h7-pll Err, I meant st,stm32h7-pll. Oops, sorry i will fix it. Thank's Gabriel With that, Acked-by: Rob Herring <r...@kernel.org>
Re: [PATCH v2] clk: stm32h7: Add stm32h743 clock driver
On 04/03/2017 06:04 PM, Rob Herring wrote: On Mon, Apr 3, 2017 at 9:39 AM, Rob Herring wrote: On Wed, Mar 29, 2017 at 11:08:22AM +0200, gabriel.fernan...@st.com wrote: From: Gabriel Fernandez This patch enables clocks for STM32H743 boards. Signed-off-by: Gabriel Fernandez Just for the MFD changes: Acked-by: Lee Jones +Required properties for pll node: +- compatible: Should be: + "stm32h7-pll" stm,stm32h7-pll Err, I meant st,stm32h7-pll. Oops, sorry i will fix it. Thank's Gabriel With that, Acked-by: Rob Herring
Re: [PATCH] clk: stm32h7: Add stm32h743 clock driver
On 03/27/2017 09:04 PM, Rob Herring wrote:
On Fri, Mar 24, 2017 at 4:41 AM, Gabriel Fernandez
<gabriel.fernan...@st.com> wrote:
Hi Rob,
Thanks for reviewing
On 03/24/2017 03:06 AM, Rob Herring wrote:
On Wed, Mar 15, 2017 at 10:23:30AM +0100, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez <gabriel.fernan...@st.com>
This patch enables clocks for STM32H743 boards.
Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
---
.../devicetree/bindings/clock/st,stm32h7-rcc.txt | 152 ++
drivers/clk/Makefile |1 +
drivers/clk/clk-stm32h7.c | 1586
include/dt-bindings/clock/stm32h7-clks.h | 165 ++
include/dt-bindings/mfd/stm32h7-rcc.h | 138 ++
5 files changed, 2042 insertions(+)
create mode 100644
Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
create mode 100644 drivers/clk/clk-stm32h7.c
create mode 100644 include/dt-bindings/clock/stm32h7-clks.h
create mode 100644 include/dt-bindings/mfd/stm32h7-rcc.h
diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
new file mode 100644
index 000..9d4b587
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
@@ -0,0 +1,152 @@
+STMicroelectronics STM32H7 Reset and Clock Controller
+=
+
+The RCC IP is both a reset and a clock controller.
+
+Please refer to clock-bindings.txt for common clock controller binding
usage.
+Please also refer to reset.txt for common reset controller binding
usage.
+
+Required properties:
+- compatible: Should be:
+ "st,stm32h743-rcc"
+
+- reg: should be register base and length as documented in the
+ datasheet
+
+- #reset-cells: 1, see below
+
+- #clock-cells : from common clock binding; shall be set to 1
+
+- clocks: External oscillator clock phandle
+ - high speed external clock signal (HSE)
+ - low speed external clock signal (LSE)
+ - external I2S clock (I2S_CKIN)
+
+- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup
domain
+ write protection (RTC clock).
+
+- pll x node: Allow to register a pll with specific parameters.
+ Please see PLL section below.
+
+Example:
+
+ rcc: rcc@58024400 {
+ #reset-cells = <1>;
+ #clock-cells = <2>
+ compatible = "st,stm32h743-rcc", "st,stm32-rcc";
+ reg = <0x58024400 0x400>;
+ clocks = <_hse>, <_lse>, <_i2s_ckin>;
+
+ st,syscfg = <>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vco1@58024430 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <0>;
The reg addr value and unit address don't match.
ok il will change into
vco1@0 {
reg = <0>;
+ };
+
+ vco2@58024438 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <1>;
+ st,clock-div = <2>;
+ st,clock-mult = <40>;
+ st,frac-status = <0>;
+ st,frac = <0>;
+ st,vcosel = <1>;
+ st,pllrge = <2>;
+ };
+ };
+
+
+STM32H7 PLL
+---
+
+The VCO of STM32 PLL could be reprensented like this:
+
+ Vref-
+>| / DIVM |>| x DIVN | --> VCO
+ -
+^
+|
+ ---
+| FRACN |
+ ---
+
+When the PLL is configured in integer mode:
+- VCO = ( Vref / DIVM ) * DIVN
+
+When the PLL is configured in fractional mode:
+- VCO = ( Vref / DIVM ) * ( DIVN + FRACN / 2^13)
+
+
+Required properties for pll node:
+- compatible: Should be:
+ "stm32,pll"
Only 1 single PLL design for all STM32 chips ever?
no, i can change into "stm32h7,pll"
Actually, should be "st,stm32h7-pll".
ok
+
+- #clock-cells: from common clock binding; shall be set to 0
+- reg: Should be the pll number.
+
+Optional properties:
+- st,clock-div: DIVM division factor : <1..63>
+- st,clock-mult: DIVN multiplication factor : <4..512>
+
+- st,frac-status:
+ - 0 Pll is configured in integer mode
+ - 1 Pll is configure in fractional mode
Isn't this implied by the presence of the next property?
do you prefer this ?
- st,frac :
0 : pll is configured in integer mode
1..8191 : Fractional part of the multiplication factor and pll is configured
in fractional mode
Why not no st,frac property means integer mode?
No
no st,frac property means no change (keep default values from bootloader)
Best Regards
Gabriel
Rob
Re: [PATCH] clk: stm32h7: Add stm32h743 clock driver
On 03/27/2017 09:04 PM, Rob Herring wrote:
On Fri, Mar 24, 2017 at 4:41 AM, Gabriel Fernandez
wrote:
Hi Rob,
Thanks for reviewing
On 03/24/2017 03:06 AM, Rob Herring wrote:
On Wed, Mar 15, 2017 at 10:23:30AM +0100, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez
This patch enables clocks for STM32H743 boards.
Signed-off-by: Gabriel Fernandez
---
.../devicetree/bindings/clock/st,stm32h7-rcc.txt | 152 ++
drivers/clk/Makefile |1 +
drivers/clk/clk-stm32h7.c | 1586
include/dt-bindings/clock/stm32h7-clks.h | 165 ++
include/dt-bindings/mfd/stm32h7-rcc.h | 138 ++
5 files changed, 2042 insertions(+)
create mode 100644
Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
create mode 100644 drivers/clk/clk-stm32h7.c
create mode 100644 include/dt-bindings/clock/stm32h7-clks.h
create mode 100644 include/dt-bindings/mfd/stm32h7-rcc.h
diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
new file mode 100644
index 000..9d4b587
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
@@ -0,0 +1,152 @@
+STMicroelectronics STM32H7 Reset and Clock Controller
+=
+
+The RCC IP is both a reset and a clock controller.
+
+Please refer to clock-bindings.txt for common clock controller binding
usage.
+Please also refer to reset.txt for common reset controller binding
usage.
+
+Required properties:
+- compatible: Should be:
+ "st,stm32h743-rcc"
+
+- reg: should be register base and length as documented in the
+ datasheet
+
+- #reset-cells: 1, see below
+
+- #clock-cells : from common clock binding; shall be set to 1
+
+- clocks: External oscillator clock phandle
+ - high speed external clock signal (HSE)
+ - low speed external clock signal (LSE)
+ - external I2S clock (I2S_CKIN)
+
+- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup
domain
+ write protection (RTC clock).
+
+- pll x node: Allow to register a pll with specific parameters.
+ Please see PLL section below.
+
+Example:
+
+ rcc: rcc@58024400 {
+ #reset-cells = <1>;
+ #clock-cells = <2>
+ compatible = "st,stm32h743-rcc", "st,stm32-rcc";
+ reg = <0x58024400 0x400>;
+ clocks = <_hse>, <_lse>, <_i2s_ckin>;
+
+ st,syscfg = <>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vco1@58024430 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <0>;
The reg addr value and unit address don't match.
ok il will change into
vco1@0 {
reg = <0>;
+ };
+
+ vco2@58024438 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <1>;
+ st,clock-div = <2>;
+ st,clock-mult = <40>;
+ st,frac-status = <0>;
+ st,frac = <0>;
+ st,vcosel = <1>;
+ st,pllrge = <2>;
+ };
+ };
+
+
+STM32H7 PLL
+---
+
+The VCO of STM32 PLL could be reprensented like this:
+
+ Vref-
+>| / DIVM |>| x DIVN | --> VCO
+ -
+^
+|
+ ---
+| FRACN |
+ ---
+
+When the PLL is configured in integer mode:
+- VCO = ( Vref / DIVM ) * DIVN
+
+When the PLL is configured in fractional mode:
+- VCO = ( Vref / DIVM ) * ( DIVN + FRACN / 2^13)
+
+
+Required properties for pll node:
+- compatible: Should be:
+ "stm32,pll"
Only 1 single PLL design for all STM32 chips ever?
no, i can change into "stm32h7,pll"
Actually, should be "st,stm32h7-pll".
ok
+
+- #clock-cells: from common clock binding; shall be set to 0
+- reg: Should be the pll number.
+
+Optional properties:
+- st,clock-div: DIVM division factor : <1..63>
+- st,clock-mult: DIVN multiplication factor : <4..512>
+
+- st,frac-status:
+ - 0 Pll is configured in integer mode
+ - 1 Pll is configure in fractional mode
Isn't this implied by the presence of the next property?
do you prefer this ?
- st,frac :
0 : pll is configured in integer mode
1..8191 : Fractional part of the multiplication factor and pll is configured
in fractional mode
Why not no st,frac property means integer mode?
No
no st,frac property means no change (keep default values from bootloader)
Best Regards
Gabriel
Rob
Re: [PATCH] clk: stm32h7: Add stm32h743 clock driver
Hi Rob,
Thanks for reviewing
On 03/24/2017 03:06 AM, Rob Herring wrote:
On Wed, Mar 15, 2017 at 10:23:30AM +0100, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez <gabriel.fernan...@st.com>
This patch enables clocks for STM32H743 boards.
Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
---
.../devicetree/bindings/clock/st,stm32h7-rcc.txt | 152 ++
drivers/clk/Makefile |1 +
drivers/clk/clk-stm32h7.c | 1586
include/dt-bindings/clock/stm32h7-clks.h | 165 ++
include/dt-bindings/mfd/stm32h7-rcc.h | 138 ++
5 files changed, 2042 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
create mode 100644 drivers/clk/clk-stm32h7.c
create mode 100644 include/dt-bindings/clock/stm32h7-clks.h
create mode 100644 include/dt-bindings/mfd/stm32h7-rcc.h
diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
new file mode 100644
index 000..9d4b587
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
@@ -0,0 +1,152 @@
+STMicroelectronics STM32H7 Reset and Clock Controller
+=
+
+The RCC IP is both a reset and a clock controller.
+
+Please refer to clock-bindings.txt for common clock controller binding usage.
+Please also refer to reset.txt for common reset controller binding usage.
+
+Required properties:
+- compatible: Should be:
+ "st,stm32h743-rcc"
+
+- reg: should be register base and length as documented in the
+ datasheet
+
+- #reset-cells: 1, see below
+
+- #clock-cells : from common clock binding; shall be set to 1
+
+- clocks: External oscillator clock phandle
+ - high speed external clock signal (HSE)
+ - low speed external clock signal (LSE)
+ - external I2S clock (I2S_CKIN)
+
+- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
+ write protection (RTC clock).
+
+- pll x node: Allow to register a pll with specific parameters.
+ Please see PLL section below.
+
+Example:
+
+ rcc: rcc@58024400 {
+ #reset-cells = <1>;
+ #clock-cells = <2>
+ compatible = "st,stm32h743-rcc", "st,stm32-rcc";
+ reg = <0x58024400 0x400>;
+ clocks = <_hse>, <_lse>, <_i2s_ckin>;
+
+ st,syscfg = <>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vco1@58024430 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <0>;
The reg addr value and unit address don't match.
ok il will change into
vco1@0 {
reg = <0>;
+ };
+
+ vco2@58024438 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <1>;
+ st,clock-div = <2>;
+ st,clock-mult = <40>;
+ st,frac-status = <0>;
+ st,frac = <0>;
+ st,vcosel = <1>;
+ st,pllrge = <2>;
+ };
+ };
+
+
+STM32H7 PLL
+---
+
+The VCO of STM32 PLL could be reprensented like this:
+
+ Vref-
+>| / DIVM |>| x DIVN | --> VCO
+ -
+^
+|
+ ---
+| FRACN |
+ ---
+
+When the PLL is configured in integer mode:
+- VCO = ( Vref / DIVM ) * DIVN
+
+When the PLL is configured in fractional mode:
+- VCO = ( Vref / DIVM ) * ( DIVN + FRACN / 2^13)
+
+
+Required properties for pll node:
+- compatible: Should be:
+ "stm32,pll"
Only 1 single PLL design for all STM32 chips ever?
no, i can change into "stm32h7,pll"
+
+- #clock-cells: from common clock binding; shall be set to 0
+- reg: Should be the pll number.
+
+Optional properties:
+- st,clock-div: DIVM division factor : <1..63>
+- st,clock-mult: DIVN multiplication factor : <4..512>
+
+- st,frac-status:
+ - 0 Pll is configured in integer mode
+ - 1 Pll is configure in fractional mode
Isn't this implied by the presence of the next property?
do you prefer this ?
- st,frac :
0 : pll is configured in integer mode
1..8191 : Fractional part of the multiplication factor and pll is
configured in fractional mode
+
+- st,frac: Fractional part of the multiplication factor : <0..8191>
+
+- st,vcosel: VCO selection
+ - 0: Wide VCO range:192 to 836 MHz
+ - 1: Medium VCO range:150 to 420
Re: [PATCH] clk: stm32h7: Add stm32h743 clock driver
Hi Rob,
Thanks for reviewing
On 03/24/2017 03:06 AM, Rob Herring wrote:
On Wed, Mar 15, 2017 at 10:23:30AM +0100, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez
This patch enables clocks for STM32H743 boards.
Signed-off-by: Gabriel Fernandez
---
.../devicetree/bindings/clock/st,stm32h7-rcc.txt | 152 ++
drivers/clk/Makefile |1 +
drivers/clk/clk-stm32h7.c | 1586
include/dt-bindings/clock/stm32h7-clks.h | 165 ++
include/dt-bindings/mfd/stm32h7-rcc.h | 138 ++
5 files changed, 2042 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
create mode 100644 drivers/clk/clk-stm32h7.c
create mode 100644 include/dt-bindings/clock/stm32h7-clks.h
create mode 100644 include/dt-bindings/mfd/stm32h7-rcc.h
diff --git a/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
new file mode 100644
index 000..9d4b587
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/st,stm32h7-rcc.txt
@@ -0,0 +1,152 @@
+STMicroelectronics STM32H7 Reset and Clock Controller
+=
+
+The RCC IP is both a reset and a clock controller.
+
+Please refer to clock-bindings.txt for common clock controller binding usage.
+Please also refer to reset.txt for common reset controller binding usage.
+
+Required properties:
+- compatible: Should be:
+ "st,stm32h743-rcc"
+
+- reg: should be register base and length as documented in the
+ datasheet
+
+- #reset-cells: 1, see below
+
+- #clock-cells : from common clock binding; shall be set to 1
+
+- clocks: External oscillator clock phandle
+ - high speed external clock signal (HSE)
+ - low speed external clock signal (LSE)
+ - external I2S clock (I2S_CKIN)
+
+- st,syscfg: phandle for pwrcfg, mandatory to disable/enable backup domain
+ write protection (RTC clock).
+
+- pll x node: Allow to register a pll with specific parameters.
+ Please see PLL section below.
+
+Example:
+
+ rcc: rcc@58024400 {
+ #reset-cells = <1>;
+ #clock-cells = <2>
+ compatible = "st,stm32h743-rcc", "st,stm32-rcc";
+ reg = <0x58024400 0x400>;
+ clocks = <_hse>, <_lse>, <_i2s_ckin>;
+
+ st,syscfg = <>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vco1@58024430 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <0>;
The reg addr value and unit address don't match.
ok il will change into
vco1@0 {
reg = <0>;
+ };
+
+ vco2@58024438 {
+ #clock-cells = <0>;
+ compatible = "stm32,pll";
+ reg = <1>;
+ st,clock-div = <2>;
+ st,clock-mult = <40>;
+ st,frac-status = <0>;
+ st,frac = <0>;
+ st,vcosel = <1>;
+ st,pllrge = <2>;
+ };
+ };
+
+
+STM32H7 PLL
+---
+
+The VCO of STM32 PLL could be reprensented like this:
+
+ Vref-
+>| / DIVM |>| x DIVN | --> VCO
+ -
+^
+|
+ ---
+| FRACN |
+ ---
+
+When the PLL is configured in integer mode:
+- VCO = ( Vref / DIVM ) * DIVN
+
+When the PLL is configured in fractional mode:
+- VCO = ( Vref / DIVM ) * ( DIVN + FRACN / 2^13)
+
+
+Required properties for pll node:
+- compatible: Should be:
+ "stm32,pll"
Only 1 single PLL design for all STM32 chips ever?
no, i can change into "stm32h7,pll"
+
+- #clock-cells: from common clock binding; shall be set to 0
+- reg: Should be the pll number.
+
+Optional properties:
+- st,clock-div: DIVM division factor : <1..63>
+- st,clock-mult: DIVN multiplication factor : <4..512>
+
+- st,frac-status:
+ - 0 Pll is configured in integer mode
+ - 1 Pll is configure in fractional mode
Isn't this implied by the presence of the next property?
do you prefer this ?
- st,frac :
0 : pll is configured in integer mode
1..8191 : Fractional part of the multiplication factor and pll is
configured in fractional mode
+
+- st,frac: Fractional part of the multiplication factor : <0..8191>
+
+- st,vcosel: VCO selection
+ - 0: Wide VCO range:192 to 836 MHz
+ - 1: Medium VCO range:150 to 420 MHz
+
+- st,pllrge: PLL input frequency range
+ - 0: The PLL
Re: [PATCH 0/2] STM32F4 clock fixes
Hi Stephen, On 03/15/2017 09:43 PM, Stephen Boyd wrote: On 03/15, gabriel.fernan...@st.com wrote: From: Gabriel Fernandez <gabriel.fernan...@st.com> This patch-set contains 2 fixes. One concerning exclusion of wrong values for PLLQ (0 & 1) And the second is a fix about timeout management of PLL and LSE/LSI clocks. But neither of the patches have a "Fixes" tag. Can you please indicate what commits they're fixing? Okay i will send a v2 with "Fixes" tags BR Gabriel
Re: [PATCH 0/2] STM32F4 clock fixes
Hi Stephen, On 03/15/2017 09:43 PM, Stephen Boyd wrote: On 03/15, gabriel.fernan...@st.com wrote: From: Gabriel Fernandez This patch-set contains 2 fixes. One concerning exclusion of wrong values for PLLQ (0 & 1) And the second is a fix about timeout management of PLL and LSE/LSI clocks. But neither of the patches have a "Fixes" tag. Can you please indicate what commits they're fixing? Okay i will send a v2 with "Fixes" tags BR Gabriel
Re: [Resend PATCH v2 2/3] dt-bindings: mfd: stm32f4: Add missing binding definition
Hi Lee, On 02/01/2017 02:31 PM, Lee Jones wrote: On Wed, 01 Feb 2017, gabriel.fernan...@st.com wrote: From: Gabriel Fernandez <gabriel.fernan...@st.com> This patch adds missing binding definition (backupram, ethernet, otg, qspi, adc & dsi) What is RCC? Reset & Clock Control Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com> --- include/dt-bindings/mfd/stm32f4-rcc.h | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/include/dt-bindings/mfd/stm32f4-rcc.h b/include/dt-bindings/mfd/stm32f4-rcc.h index f662b19..082a81c 100644 --- a/include/dt-bindings/mfd/stm32f4-rcc.h +++ b/include/dt-bindings/mfd/stm32f4-rcc.h @@ -18,11 +18,17 @@ #define STM32F4_RCC_AHB1_GPIOJ9 #define STM32F4_RCC_AHB1_GPIOK10 #define STM32F4_RCC_AHB1_CRC 12 +#define STM32F4_RCC_AHB1_BKPSRAM 18 +#define STM32F4_RCC_AHB1_CCMDATARAM20 #define STM32F4_RCC_AHB1_DMA1 21 #define STM32F4_RCC_AHB1_DMA2 22 #define STM32F4_RCC_AHB1_DMA2D23 #define STM32F4_RCC_AHB1_ETHMAC 25 -#define STM32F4_RCC_AHB1_OTGHS 29 +#define STM32F4_RCC_AHB1_ETHMACTX 26 +#define STM32F4_RCC_AHB1_ETHMACRX 27 +#define STM32F4_RCC_AHB1_ETHMACPTP 28 +#define STM32F4_RCC_AHB1_OTGHS 29 +#define STM32F4_RCC_AHB1_OTGHSULPI 30 #define STM32F4_AHB1_RESET(bit) (STM32F4_RCC_AHB1_##bit + (0x10 * 8)) #define STM32F4_AHB1_CLOCK(bit) (STM32F4_RCC_AHB1_##bit) @@ -40,6 +46,7 @@ /* AHB3 */ #define STM32F4_RCC_AHB3_FMC 0 +#define STM32F4_RCC_AHB3_QSPI 1 #define STM32F4_AHB3_RESET(bit) (STM32F4_RCC_AHB3_##bit + (0x18 * 8)) #define STM32F4_AHB3_CLOCK(bit) (STM32F4_RCC_AHB3_##bit + 0x40) @@ -79,7 +86,9 @@ #define STM32F4_RCC_APB2_TIM8 1 #define STM32F4_RCC_APB2_USART1 4 #define STM32F4_RCC_APB2_USART6 5 -#define STM32F4_RCC_APB2_ADC 8 +#define STM32F4_RCC_APB2_ADC1 8 +#define STM32F4_RCC_APB2_ADC2 9 +#define STM32F4_RCC_APB2_ADC3 10 #define STM32F4_RCC_APB2_SDIO 11 #define STM32F4_RCC_APB2_SPI1 12 #define STM32F4_RCC_APB2_SPI4 13 @@ -91,6 +100,7 @@ #define STM32F4_RCC_APB2_SPI6 21 #define STM32F4_RCC_APB2_SAI1 22 #define STM32F4_RCC_APB2_LTDC 26 +#define STM32F4_RCC_APB2_DSI 27 #define STM32F4_APB2_RESET(bit) (STM32F4_RCC_APB2_##bit + (0x24 * 8)) #define STM32F4_APB2_CLOCK(bit) (STM32F4_RCC_APB2_##bit + 0xA0)
Re: [Resend PATCH v2 2/3] dt-bindings: mfd: stm32f4: Add missing binding definition
Hi Lee, On 02/01/2017 02:31 PM, Lee Jones wrote: On Wed, 01 Feb 2017, gabriel.fernan...@st.com wrote: From: Gabriel Fernandez This patch adds missing binding definition (backupram, ethernet, otg, qspi, adc & dsi) What is RCC? Reset & Clock Control Signed-off-by: Gabriel Fernandez --- include/dt-bindings/mfd/stm32f4-rcc.h | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/include/dt-bindings/mfd/stm32f4-rcc.h b/include/dt-bindings/mfd/stm32f4-rcc.h index f662b19..082a81c 100644 --- a/include/dt-bindings/mfd/stm32f4-rcc.h +++ b/include/dt-bindings/mfd/stm32f4-rcc.h @@ -18,11 +18,17 @@ #define STM32F4_RCC_AHB1_GPIOJ9 #define STM32F4_RCC_AHB1_GPIOK10 #define STM32F4_RCC_AHB1_CRC 12 +#define STM32F4_RCC_AHB1_BKPSRAM 18 +#define STM32F4_RCC_AHB1_CCMDATARAM20 #define STM32F4_RCC_AHB1_DMA1 21 #define STM32F4_RCC_AHB1_DMA2 22 #define STM32F4_RCC_AHB1_DMA2D23 #define STM32F4_RCC_AHB1_ETHMAC 25 -#define STM32F4_RCC_AHB1_OTGHS 29 +#define STM32F4_RCC_AHB1_ETHMACTX 26 +#define STM32F4_RCC_AHB1_ETHMACRX 27 +#define STM32F4_RCC_AHB1_ETHMACPTP 28 +#define STM32F4_RCC_AHB1_OTGHS 29 +#define STM32F4_RCC_AHB1_OTGHSULPI 30 #define STM32F4_AHB1_RESET(bit) (STM32F4_RCC_AHB1_##bit + (0x10 * 8)) #define STM32F4_AHB1_CLOCK(bit) (STM32F4_RCC_AHB1_##bit) @@ -40,6 +46,7 @@ /* AHB3 */ #define STM32F4_RCC_AHB3_FMC 0 +#define STM32F4_RCC_AHB3_QSPI 1 #define STM32F4_AHB3_RESET(bit) (STM32F4_RCC_AHB3_##bit + (0x18 * 8)) #define STM32F4_AHB3_CLOCK(bit) (STM32F4_RCC_AHB3_##bit + 0x40) @@ -79,7 +86,9 @@ #define STM32F4_RCC_APB2_TIM8 1 #define STM32F4_RCC_APB2_USART1 4 #define STM32F4_RCC_APB2_USART6 5 -#define STM32F4_RCC_APB2_ADC 8 +#define STM32F4_RCC_APB2_ADC1 8 +#define STM32F4_RCC_APB2_ADC2 9 +#define STM32F4_RCC_APB2_ADC3 10 #define STM32F4_RCC_APB2_SDIO 11 #define STM32F4_RCC_APB2_SPI1 12 #define STM32F4_RCC_APB2_SPI4 13 @@ -91,6 +100,7 @@ #define STM32F4_RCC_APB2_SPI6 21 #define STM32F4_RCC_APB2_SAI1 22 #define STM32F4_RCC_APB2_LTDC 26 +#define STM32F4_RCC_APB2_DSI 27 #define STM32F4_APB2_RESET(bit) (STM32F4_RCC_APB2_##bit + (0x24 * 8)) #define STM32F4_APB2_CLOCK(bit) (STM32F4_RCC_APB2_##bit + 0xA0)
Re: [PATCH] clk: stm32f4: avoid uninitialized variable access
On 01/11/2017 02:40 PM, Arnd Bergmann wrote:
The failure path in the newly added function tries to free an
uninitialized pointer:
drivers/clk/clk-stm32f4.c: In function 'stm32f4_rcc_init':
drivers/clk/clk-stm32f4.c:1106:4: error: 'gate' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
I'm adding an initialization to NULL here to make the kfree()
succeed, and I'm also rearranging the cleanup so that the
same kfree() is used for any error path, making the function
slightly more robust against newly introduced bugs in the
error handling.
Fixes: daf2d117cbca ("clk: stm32f4: Add lcd-tft clock")
Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
drivers/clk/clk-stm32f4.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
Acked-by: Gabriel Fernandez <gabriel.fernan...@st.com>
Re: [PATCH] clk: stm32f4: avoid uninitialized variable access
On 01/11/2017 02:40 PM, Arnd Bergmann wrote:
The failure path in the newly added function tries to free an
uninitialized pointer:
drivers/clk/clk-stm32f4.c: In function 'stm32f4_rcc_init':
drivers/clk/clk-stm32f4.c:1106:4: error: 'gate' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
I'm adding an initialization to NULL here to make the kfree()
succeed, and I'm also rearranging the cleanup so that the
same kfree() is used for any error path, making the function
slightly more robust against newly introduced bugs in the
error handling.
Fixes: daf2d117cbca ("clk: stm32f4: Add lcd-tft clock")
Signed-off-by: Arnd Bergmann
---
drivers/clk/clk-stm32f4.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
Acked-by: Gabriel Fernandez
Re: [PATCH v2 4/9] clk: stm32f4: Add lcd-tft clock
Hi Rob, Thanks for reviewing On 11/30/2016 09:53 PM, Rob Herring wrote: On Thu, Nov 24, 2016 at 03:45:44PM +0100, gabriel.fernan...@st.com wrote: From: Gabriel Fernandez <gabriel.fernan...@st.com> This patch introduces lcd-tft clock for stm32f4 soc. Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com> --- .../devicetree/bindings/clock/st,stm32-rcc.txt | 1 + drivers/clk/clk-stm32f4.c | 118 + include/dt-bindings/clock/stm32f4-clock.h | 3 +- 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/include/dt-bindings/clock/stm32f4-clock.h b/include/dt-bindings/clock/stm32f4-clock.h index 56b8e10..1be4a3a 100644 --- a/include/dt-bindings/clock/stm32f4-clock.h +++ b/include/dt-bindings/clock/stm32f4-clock.h @@ -27,7 +27,8 @@ #define CLK_RTC 5 #define PLL_VCO_I2S 6 #define PLL_VCO_SAI 7 +#define CLK_LCD8 -#define END_PRIMARY_CLK 8 +#define END_PRIMARY_CLK9 Do you really need this? Having this change could cause compatibility problems between dtb and kernel versions. Please restructure the patch series and put all of the binding changes including this header into a single patch. Incrementally add s/w features, not h/w. Rob Okay Best Regards Gabriel
Re: [PATCH v2 4/9] clk: stm32f4: Add lcd-tft clock
Hi Rob, Thanks for reviewing On 11/30/2016 09:53 PM, Rob Herring wrote: On Thu, Nov 24, 2016 at 03:45:44PM +0100, gabriel.fernan...@st.com wrote: From: Gabriel Fernandez This patch introduces lcd-tft clock for stm32f4 soc. Signed-off-by: Gabriel Fernandez --- .../devicetree/bindings/clock/st,stm32-rcc.txt | 1 + drivers/clk/clk-stm32f4.c | 118 + include/dt-bindings/clock/stm32f4-clock.h | 3 +- 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/include/dt-bindings/clock/stm32f4-clock.h b/include/dt-bindings/clock/stm32f4-clock.h index 56b8e10..1be4a3a 100644 --- a/include/dt-bindings/clock/stm32f4-clock.h +++ b/include/dt-bindings/clock/stm32f4-clock.h @@ -27,7 +27,8 @@ #define CLK_RTC 5 #define PLL_VCO_I2S 6 #define PLL_VCO_SAI 7 +#define CLK_LCD8 -#define END_PRIMARY_CLK 8 +#define END_PRIMARY_CLK9 Do you really need this? Having this change could cause compatibility problems between dtb and kernel versions. Please restructure the patch series and put all of the binding changes including this header into a single patch. Incrementally add s/w features, not h/w. Rob Okay Best Regards Gabriel
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 <gabriel.fernan...@st.com>:
On 11/08/2016 09:52 AM, Radosław Pietrzyk wrote:
2016-11-08 9:35 GMT+01:00 Gabriel Fernandez <gabriel.fernan...@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 = _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
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 :
On 11/08/2016 09:52 AM, Radosław Pietrzyk wrote:
2016-11-08 9:35 GMT+01:00 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 = _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 4/6] clk: stm32f4: Add I2S clock
On 11/07/2016 03:14 PM, Daniel Thompson wrote:
On 07/11/16 13:05, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez <gabriel.fernan...@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.fernan...@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,
1600, 16);
@@ -1053,6 +1057,12 @@ static void __init stm32f4_rcc_init(struct
device_node *np)
clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll(pllsrc,
>pll_data[2], _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,
+_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
On 11/07/2016 03:14 PM, Daniel Thompson wrote:
On 07/11/16 13:05, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez
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
---
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,
1600, 16);
@@ -1053,6 +1057,12 @@ static void __init stm32f4_rcc_init(struct
device_node *np)
clks[PLL_VCO_SAI] = stm32f4_rcc_register_pll(pllsrc,
>pll_data[2], _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,
+_clk_lock);
sys_parents[1] = hse_clk;
clk_register_mux_table(
NULL, "sys", sys_parents, ARRAY_SIZE(sys_parents), 0,
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 <gabriel.fernan...@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 = _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
On 11/08/2016 09:52 AM, Radosław Pietrzyk wrote:
2016-11-08 9:35 GMT+01:00 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 = _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
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 = _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 = _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
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 = _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 = _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 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, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez <gabriel.fernan...@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.fernan...@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, _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, _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.
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, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez
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
---
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, _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, _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.
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, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez <gabriel.fernan...@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.fernan...@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,
+_clk_lock);
+
+clk_hw_register_mux_table(NULL, "sdmux",
+sdmux_parents, ARRAY_SIZE(sdmux_parents), 0,
+base + STM32F4_RCC_DCKCFGR, 28, 1, 0, NULL,
+_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
Hi Daniel,
On 11/07/2016 02:55 PM, Daniel Thompson wrote:
On 07/11/16 13:05, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez
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
---
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,
+_clk_lock);
+
+clk_hw_register_mux_table(NULL, "sdmux",
+sdmux_parents, ARRAY_SIZE(sdmux_parents), 0,
+base + STM32F4_RCC_DCKCFGR, 28, 1, 0, NULL,
+_clk_lock);
+}
+
of_clk_add_hw_provider(np, stm32f4_rcc_lookup_clk, NULL);
return;
fail:
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, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez <gabriel.fernan...@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.fernan...@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
Hi Daniel,
Thanks for reviewing.
On 11/07/2016 02:53 PM, Daniel Thompson wrote:
On 07/11/16 13:05, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez
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
---
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/2] ARM: dts: stm32f429: add LSI and LSE clocks
Hi Alexandre,
On 11/04/2016 11:15 AM, Alexandre Torgue wrote:
Gabriel,
On 11/04/2016 09:52 AM, gabriel.fernan...@st.com wrote:
From: Gabriel Fernandez <gabriel.fernan...@st.com>
This patch adds lsi / lse oscillators. These clocks can be use by
RTC clocks.
The clock drivers needs to disable the power domain write protection
using
syscon / regmap to enable these clocks.
Is it the same than you sent in last series ? If yes I will take it
directly as review has already been done.
Yes
BR
Gabriel.
regards
Alex
Signed-off-by: Gabriel Fernandez <gabriel.fernan...@st.com>
---
arch/arm/boot/dts/stm32f429.dtsi | 18 ++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f429.dtsi
b/arch/arm/boot/dts/stm32f429.dtsi
index 336ee4f..2700449 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -56,6 +56,18 @@
compatible = "fixed-clock";
clock-frequency = <0>;
};
+
+clk-lse {
+#clock-cells = <0>;
+compatible = "fixed-clock";
+clock-frequency = <32768>;
+};
+
+clk-lsi {
+#clock-cells = <0>;
+compatible = "fixed-clock";
+clock-frequency = <32000>;
+};
};
soc {
@@ -185,6 +197,11 @@
interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>,
<23>, <40>, <41>, <42>, <62>, <76>;
};
+pwrcfg: power-config@40007000 {
+compatible = "syscon";
+reg = <0x40007000 0x400>;
+};
+
pin-controller {
#address-cells = <1>;
#size-cells = <1>;
@@ -340,6 +357,7 @@
compatible = "st,stm32f42xx-rcc", "st,stm32-rcc";
reg = <0x40023800 0x400>;
clocks = <_hse>;
+st,syscfg = <>;
};
dma1: dma-controller@40026000 {
