From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Thompson Subject: Re: [PATCH v7 05/15] dt-bindings: Document the STM32 reset bindings Date: Sat, 02 May 2015 11:01:13 +0100 Message-ID: <5544A069.5000808@linaro.org> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f180.google.com ([209.85.212.180]:38567 "EHLO mail-wi0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751476AbbEBKBR (ORCPT ); Sat, 2 May 2015 06:01:17 -0400 Received: by wiun10 with SMTP id n10so67616468wiu.1 for ; Sat, 02 May 2015 03:01:16 -0700 (PDT) In-Reply-To: Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Maxime Coquelin Cc: =?UTF-8?B?VXdlIEtsZWluZS1Lw7ZuaWc=?= , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , Geert Uytterhoeven , Rob Herring , Philipp Zabel , 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 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. 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751961AbbEBKB0 (ORCPT ); Sat, 2 May 2015 06:01:26 -0400 Received: from mail-wi0-f172.google.com ([209.85.212.172]:34130 "EHLO mail-wi0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751851AbbEBKBR (ORCPT ); Sat, 2 May 2015 06:01:17 -0400 Message-ID: <5544A069.5000808@linaro.org> Date: Sat, 02 May 2015 11:01:13 +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 CC: =?UTF-8?B?VXdlIEtsZWluZS1Lw7ZuaWc=?= , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , Geert Uytterhoeven , Rob Herring , Philipp Zabel , 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 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> 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 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. 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. From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel.thompson@linaro.org (Daniel Thompson) Date: Sat, 02 May 2015 11:01:13 +0100 Subject: [PATCH v7 05/15] dt-bindings: Document the STM32 reset bindings In-Reply-To: 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> Message-ID: <5544A069.5000808@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 at 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 at 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. 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.