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>
>>>
>>> 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 0000000..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)
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.
>> rcc: clock-controller@58024400 {
>>
>>> + #reset-cells = <1>;
>>> + #clock-cells = <2>
>> Missing trailing semicolon ^^^
> ok
--
With best wishes,
Vladimir
