Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755244AbbBOOgg (ORCPT ); Sun, 15 Feb 2015 09:36:36 -0500 Received: from mail-we0-f177.google.com ([74.125.82.177]:53598 "EHLO mail-we0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754945AbbBOOgc (ORCPT ); Sun, 15 Feb 2015 09:36:32 -0500 MIME-Version: 1.0 In-Reply-To: <1423855102.4182.63.camel@pengutronix.de> References: <1423763164-5606-1-git-send-email-mcoquelin.stm32@gmail.com> <1423763164-5606-13-git-send-email-mcoquelin.stm32@gmail.com> <1423828078.4182.17.camel@pengutronix.de> <1423844735.4182.52.camel@pengutronix.de> <1423855102.4182.63.camel@pengutronix.de> Date: Sun, 15 Feb 2015 15:36:29 +0100 Message-ID: Subject: Re: [PATCH 12/14] ARM: dts: Introduce STM32F429 MCU From: Maxime Coquelin To: Philipp Zabel Cc: Jonathan Corbet , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Russell King , Daniel Lezcano , Thomas Gleixner , Linus Walleij , Greg Kroah-Hartman , Jiri Slaby , Arnd Bergmann , Andrew Morton , "David S. Miller" , Mauro Carvalho Chehab , Joe Perches , 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" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4226 Lines: 106 Hi Philipp, 2015-02-13 20:18 GMT+01:00 Philipp Zabel : > Am Freitag, den 13.02.2015, 17:41 +0100 schrieb Maxime Coquelin: >> 2015-02-13 17:25 GMT+01:00 Philipp Zabel : >> > Hi Maxime, >> > >> > Am Freitag, den 13.02.2015, 16:59 +0100 schrieb Maxime Coquelin: >> >> Hi Philipp, >> >> >> >> 2015-02-13 12:47 GMT+01:00 Philipp Zabel : >> >> > Hi Maxime, >> >> > >> >> > Am Donnerstag, den 12.02.2015, 18:46 +0100 schrieb Maxime Coquelin: >> >> > [...] >> >> >> + soc { >> >> >> + reset_ahb1: reset@40023810 { >> >> >> + #reset-cells = <1>; >> >> >> + compatible = "st,stm32-reset"; >> >> >> + reg = <0x40023810 0x4>; >> >> >> + }; >> >> >> + >> >> >> + reset_ahb2: reset@40023814 { >> >> >> + #reset-cells = <1>; >> >> >> + compatible = "st,stm32-reset"; >> >> >> + reg = <0x40023814 0x4>; >> >> >> + }; >> >> >> + >> >> >> + reset_ahb3: reset@40023818 { >> >> >> + #reset-cells = <1>; >> >> >> + compatible = "st,stm32-reset"; >> >> >> + reg = <0x40023818 0x4>; >> >> >> + }; >> >> >> + >> >> >> + reset_apb1: reset@40023820 { >> >> >> + #reset-cells = <1>; >> >> >> + compatible = "st,stm32-reset"; >> >> >> + reg = <0x40023820 0x4>; >> >> >> + }; >> >> >> + >> >> >> + reset_apb2: reset@40023824 { >> >> >> + #reset-cells = <1>; >> >> >> + compatible = "st,stm32-reset"; >> >> >> + reg = <0x40023824 0x4>; >> >> >> + }; >> >> > >> >> > These are mostly consecutive, single registers. I wonder if these are >> >> > part of the same IP block and thus should be grouped together into the >> >> > same reset controller node? >> >> >> >> What I could to is to have two instances. One for AHB and one for APB domain. >> >> Doing this, I will have one instance per domain, and only consecutive registers. >> >> Is it fine for you? >> > >> > Looking at >> > http://www.st.com/web/en/resource/technical/document/reference_manual/DM00031020.pdf >> > Table 34 (RCC register map and reset values), I'd say there is a single >> > "Reset and Clock Control" device at 0x40023800 - 0x40023884: >> > >> > soc { >> > rcc: rcc@40023800 { >> > #clock-cells = <1>; >> > #reset-cells = <1>; >> > compatible = "st,stm32-rcc"; >> > reg = <0x40023800 0x84>; >> > }; >> > >> > ... >> > >> > If you really want to describe the reset controller parts (offsets +0x10 >> > to +0x24) in a separate node, I won't argue against it too long, >> > although this is a somewhat arbitrary decision. >> > >> > In any case, the whole register at offset +0x1c is reserved, so there is >> > no reason to split the reset controller. It is ok to have unused ranges >> > as is already the case with reserved bits inside the used registers. >> >> Ok. I understand your point. >> But it will be more difficult at usage, because the node referencing >> the fourth reset bit of apb2 register will have to pass 164 as >> parameter. >> It is error prone IMHO. >> >> Other solution would be to add some defines for each reset line in the >> DT-Bindings, as we do today for STi platform. >> But it is giving an unneeded constraint between DT and reset trees. > > That is a bit unfortunate, but providing the named constants in > include/dt-bindings/reset/ makes for a much better readable device tree, > so I'd prefer that solution, even if it means having to coordinate pull > requests. Ok, I will add constants in include/dt-bindings/reset/ Thanks, Maxime > > regards > Philipp > -- 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/