[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_JsqJ6ND_CdW5ktSf_mVxaVyye=ZRVbGiJk6E5-ExvoBN9Hg@mail.gmail.com>
Date: Tue, 28 Mar 2017 10:19:10 -0500
From: Rob Herring <robh@...nel.org>
To: Gabriel Fernandez <gabriel.fernandez@...com>
Cc: Mark Rutland <mark.rutland@....com>,
Russell King <linux@...linux.org.uk>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...com>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...eaurora.org>,
Nicolas Pitre <nico@...aro.org>, Arnd Bergmann <arnd@...db.de>,
Daniel Thompson <daniel.thompson@...aro.org>,
Andrea Merello <andrea.merello@...il.com>,
Radosław Pietrzyk <radoslaw.pietrzyk@...il.com>,
Lee Jones <lee.jones@...aro.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linux-clk <linux-clk@...r.kernel.org>,
Ludovic Barre <ludovic.barre@...com>,
Olivier Bideau <olivier.bideau@...com>,
Amelie Delaunay <amelie.delaunay@...com>
Subject: Re: [PATCH] clk: stm32h7: Add stm32h743 clock driver
On Tue, Mar 28, 2017 at 1:20 AM, Gabriel Fernandez
<gabriel.fernandez@...com> wrote:
>
>
> On 03/27/2017 09:04 PM, Rob Herring wrote:
>>
>> On Fri, Mar 24, 2017 at 4:41 AM, Gabriel Fernandez
>> <gabriel.fernandez@...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.fernandez@...com
>>>> wrote:
>>>>>
>>>>> From: Gabriel Fernandez <gabriel.fernandez@...com>
>>>>>
>>>>> This patch enables clocks for STM32H743 boards.
>>>>>
>>>>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@...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 0000000..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@...24400 {
>>>>> + #reset-cells = <1>;
>>>>> + #clock-cells = <2>
>>>>> + compatible = "st,stm32h743-rcc", "st,stm32-rcc";
>>>>> + reg = <0x58024400 0x400>;
>>>>> + clocks = <&clk_hse>, <&clk_lse>, <&clk_i2s_ckin>;
>>>>> +
>>>>> + st,syscfg = <&pwrcfg>;
>>>>> +
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> +
>>>>> + vco1@...24430 {
>>>>> + #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@...24438 {
>>>>> + #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)
Okay, then 0 for integer mode is fine. Also state what the default means.
Rob
Powered by blists - more mailing lists