Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2993039AbbEEOID (ORCPT ); Tue, 5 May 2015 10:08:03 -0400 Received: from mail-wi0-f180.google.com ([209.85.212.180]:38413 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753776AbbEEOH6 (ORCPT ); Tue, 5 May 2015 10:07:58 -0400 Message-ID: <5548CEA2.8020807@linaro.org> Date: Tue, 05 May 2015 15:07:30 +0100 From: Daniel Thompson User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Maxime Coquelin , Philipp Zabel CC: =?UTF-8?B?VXdlIEtsZWluZS1Lw7ZuaWc=?= , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , Geert Uytterhoeven , Rob Herring , Linus Walleij , Arnd Bergmann , Stefan Agner , Peter Meerwald , Paul Bolle , Peter Hurley , Andy Shevchenko , Chanwoo Choi , Russell King , Daniel Lezcano , Joe Perches , Vladimir Zapolskiy , Jonathan Corbet , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Thomas Gleixner , Greg Kroah-Hartman , Jiri Slaby , Andrew Morton , "David S. Miller" , Mauro Carvalho Chehab , Antti Palosaari , Tejun Heo , Will Deacon , Nikolay Borisov , Rusty Russell , Kees Cook , Michal Marek , "linux-doc@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-gpio@vger.kernel.org" , "linux-serial@vger.kernel.org" , Linux-Arch , "linux-api@vger.kernel.org" , Nicolae Rosia , Kamil Lulko , Lee Jones Subject: Re: [PATCH v7 05/15] dt-bindings: Document the STM32 reset bindings References: <1430410844-16062-1-git-send-email-mcoquelin.stm32@gmail.com> <1430410844-16062-6-git-send-email-mcoquelin.stm32@gmail.com> <55433467.2010603@linaro.org> <5544A069.5000808@linaro.org> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6567 Lines: 205 On 04/05/15 12:25, Maxime Coquelin wrote: > 2015-05-02 12:01 GMT+02:00 Daniel Thompson : >> On 02/05/15 08:55, Maxime Coquelin wrote: >>> >>> 2015-05-01 10:08 GMT+02:00 Daniel Thompson : >>>> >>>> On 30/04/15 17:20, Maxime Coquelin wrote: >>>>> >>>>> >>>>> This adds documentation of device tree bindings for the >>>>> STM32 reset controller. >>>>> >>>>> Tested-by: Chanwoo Choi >>>>> Acked-by: Philipp Zabel >>>>> Acked-by: Rob Herring >>>>> Signed-off-by: Maxime Coquelin >>>>> --- >>>>> .../devicetree/bindings/reset/st,stm32-rcc.txt | 107 >>>>> +++++++++++++++++++++ >>>>> 1 file changed, 107 insertions(+) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/reset/st,stm32-rcc.txt >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt >>>>> b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt >>>>> new file mode 100644 >>>>> index 0000000..c1b0f8d >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt >>>>> @@ -0,0 +1,107 @@ >>>>> +STMicroelectronics STM32 Peripheral Reset Controller >>>>> +==================================================== >>>>> + >>>>> +The RCC IP is both a reset and a clock controller. This documentation >>>>> only >>>>> +documents the reset part. >>>>> + >>>>> +Please also refer to reset.txt in this directory for common reset >>>>> +controller binding usage. >>>>> + >>>>> +Required properties: >>>>> +- compatible: Should be "st,stm32-rcc" >>>>> +- reg: should be register base and length as documented in the >>>>> + datasheet >>>>> +- #reset-cells: 1, see below >>>>> + >>>>> +example: >>>>> + >>>>> +rcc: reset@40023800 { >>>>> + #reset-cells = <1>; >>>>> + compatible = "st,stm32-rcc"; >>>> >>>> >>>> >>>> Do you intend the clock driver to use the same compatible string (given >>>> it >>>> is the same bit of hardware). >>>> >>>> If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me >>>> that >>>> the register layout of the PLLs and dividers can be the same on the f7 >>>> parts >>>> (and later). >>> >>> >>> I agree we need a compatible dedicate to f4 series for clocks, and >>> maybe even one for f429 (to be checked). >>> For the reset part, we don't have this need. >>> >>> So either we use only "st,stm32f4" as you suggest, or we can have both >>> in device tree: >>> >>> rcc: reset@40023800 { >>> #reset-cells = <1>; >>> compatible = "st,stm32f4-rcc", "st,stm32-rcc"; >>> reg = <0x40023800 0x400>; >>> }; >>> >>> What do you think? >> >> >> Having both makes sense. The reset driver probably doesn't care about >> differences between F4 and F7 (I know very little about F7 but I can't think >> of any obvious h/ware evolution that would confuse the current reset >> driver). >> >> >>>>> + reg = <0x40023800 0x400>; >>>>> +}; >>>>> + >>>>> +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 = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 + >>>>> 12 >>>>> = 140 >>>>> + >>>>> +example: >>>>> + >>>>> + timer2 { >>>>> + resets = <&rcc 256>; >>>>> + }; >>>>> + >>>>> +List of valid indices for STM32F429: >>>>> + - gpioa: 128 >>>>> + - gpiob: 129 >>>>> ... >>>>> >>>>> ... >>>>> + - sai1: 310 >>>>> + - ltdc: 314 >>>> >>>> >>>> >>>> These numbers are stable for all STM32F4 family parts. Should this table >>>> go >>>> into a dt-bindings header file? >>>> >>> >>> This has already been discussed with Philipp and Arnd in earlier >>> versions of this series [0]. >>> I initially created a header file, and we decided going this way finally. >> >> >> Thanks for the link. I had overlooked that (I only really started paying >> attention at v5; I should probably have looked further back before >> commenting). >> >> However... >> >> Arnd's concerns about mergability of headers can also be met by using h/ware >> values in the header file can't there. To be honest my comment was pretty >> heavily influenced after having read a recent patch from Rob Herring ( >> https://lkml.org/lkml/2015/5/1/14 ) which does exactly this. >> >> The main reason I got interested in having a header is that the reset bits >> and the clock gate bits are encoded using the same bit patterns so I >> wondering it we could express that only once. > > Ok, I understand your need, and it makes sense. > The problem is that the values defined today cannot be re-used > directly for clocks. > Since the calculation is starting from rcc base, there is a 128 offset. > > To re-use the same values, maybe we should create a mfd dt-binding header file. > It would contain the bits definition starting from 0, and define > macros for both reset and clocks to add the offsets. > > For example, includes/dt-bindings/mfd/stm32f4-rcc.h would look like: > > #define GPIOA 0 > #define GPIOB 1 > ... > #define LTDC 186 > > #define STM32F4_RESET(x) (x + 128) > #define STM32F4_CLOCK(x) (x + 384) > > Then, in DT, a reset would be described like this: > > timer2 { > resets = <&rcc STM32F4_RESET(TIM2)>; > }; > > Phillip, Daniel, does that look acceptable to you? Doesn't look unreasonable. I am a little uneasy simply because there are very few similar header files in that directory but I haven't thought of a better idea. Daniel. > > Kind regards, > Maxime >> >> I guess it doesn't matter that much, especially given there is only one >> .dtsi file, and we can add a header later and remain binary compatible. >> However if the same number set does end up repeated in different .dtsi files >> I think that would motivate adding a header for F4 family. >> >> >> Daniel. >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/