From mboxrd@z Thu Jan 1 00:00:00 1970 From: Philipp Zabel Subject: Re: [PATCH v2 07/18] drivers: reset: Add STM32 reset driver Date: Wed, 11 Mar 2015 14:08:28 +0100 Message-ID: <1426079308.3101.57.camel@pengutronix.de> References: <1424455277-29983-1-git-send-email-mcoquelin.stm32@gmail.com> <2383270.cAJPOxrkvo@wuerfel> <2455342.zT71E5iyaL@wuerfel> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: Sender: linux-doc-owner@vger.kernel.org To: Maxime Coquelin Cc: Arnd Bergmann , Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= , Andreas =?ISO-8859-1?Q?F=E4rber?= , Geert Uytterhoeven , Rob Herring , Jonathan Corbet , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Russell King , Daniel Lezcano , Thomas Gleixner , Linus Walleij , Greg Kroah-Hartman , Jiri Slaby , Andrew Morton , "David S. Miller" , Mauro Carvalho Chehab , Joe Perches , Antti Palosaari , Tejun Heo List-Id: linux-arch.vger.kernel.org Am Dienstag, den 10.03.2015, 22:20 +0100 schrieb Maxime Coquelin: > 2015-03-10 21:21 GMT+01:00 Arnd Bergmann : > > On Tuesday 10 March 2015 16:44:24 Maxime Coquelin wrote: > >> 2015-03-10 16:02 GMT+01:00 Arnd Bergmann : > >> > On Friday 20 February 2015 19:01:06 Maxime Coquelin wrote: > >> >> +/* AHB1 */ > >> >> +#define GPIOA_RESET 0 > >> >> +#define GPIOB_RESET 1 > >> >> +#define GPIOC_RESET 2 > >> >> +#define GPIOD_RESET 3 > >> >> +#define GPIOE_RESET 4 > >> >> +#define GPIOF_RESET 5 > >> >> +#define GPIOG_RESET 6 > >> >> +#define GPIOH_RESET 7 > >> >> +#define GPIOI_RESET 8 > >> >> +#define GPIOJ_RESET 9 > >> >> +#define GPIOK_RESET 10 > >> >> > >> > > >> > As these are just the hardware numbers, it's better to not make = them > >> > part of the binding at all. Instead, just document in the bindin= g that > >> > one is supposed to pass the hardware number as the argument. > >> > >> The reset controller is part of the RCC (Reset & Clock Controller)= IP. > >> In this version, I only provided the reset registers to the reset > >> controller driver, but as per Andreas F=C3=A4rber remark, I should= avec a > >> single DT node for both the resets and clocks. > >> > >> In the next version I am preparing, the defines doesn't look as > >> trivial as in this version, GPIOA_RESET being 128 for instance. > >> > >> Is it fine for you if I keep the defines part of the binding? > >> > >> > > > > It's always better to avoid these files entirely, as they are > > a frequent source of merge dependencies, and they make it less > > obvious what's going on than having binary values in the dtb > > that make sense. >=20 > I agree it is always painful to have to have to manage these merge de= pendencies. > What I will do, if Philipp agrees, is to list all the values in the > binding documentation. >=20 > Doing that, the user of a reset won't have to do the calculation, and > no more merge dependencies. I'd prefer to have #defines for the reset bits if they are named in the documentation and use the names in the dts. But if you want to referenc= e reset bits by number in the device tree instead, I won't insist. Consider using two cells in the phandle for register and bit offset instead of a single number that arbitrarily starts at 128. regards Philipp From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from metis.ext.pengutronix.de ([92.198.50.35]:35018 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752738AbbCKNJc (ORCPT ); Wed, 11 Mar 2015 09:09:32 -0400 Message-ID: <1426079308.3101.57.camel@pengutronix.de> Subject: Re: [PATCH v2 07/18] drivers: reset: Add STM32 reset driver From: Philipp Zabel Date: Wed, 11 Mar 2015 14:08:28 +0100 In-Reply-To: References: <1424455277-29983-1-git-send-email-mcoquelin.stm32@gmail.com> <2383270.cAJPOxrkvo@wuerfel> <2455342.zT71E5iyaL@wuerfel> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: Maxime Coquelin Cc: Arnd Bergmann , Uwe =?ISO-8859-1?Q?Kleine-K=F6nig?= , Andreas =?ISO-8859-1?Q?F=E4rber?= , Geert Uytterhoeven , Rob Herring , Jonathan Corbet , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Russell King , Daniel Lezcano , Thomas Gleixner , Linus Walleij , Greg Kroah-Hartman , Jiri Slaby , 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" Message-ID: <20150311130828.pW1brDk7stZ3ySSJL32h2weMsaw5ZJ1DwpZhAm4N4Co@z> Am Dienstag, den 10.03.2015, 22:20 +0100 schrieb Maxime Coquelin: > 2015-03-10 21:21 GMT+01:00 Arnd Bergmann : > > On Tuesday 10 March 2015 16:44:24 Maxime Coquelin wrote: > >> 2015-03-10 16:02 GMT+01:00 Arnd Bergmann : > >> > On Friday 20 February 2015 19:01:06 Maxime Coquelin wrote: > >> >> +/* AHB1 */ > >> >> +#define GPIOA_RESET 0 > >> >> +#define GPIOB_RESET 1 > >> >> +#define GPIOC_RESET 2 > >> >> +#define GPIOD_RESET 3 > >> >> +#define GPIOE_RESET 4 > >> >> +#define GPIOF_RESET 5 > >> >> +#define GPIOG_RESET 6 > >> >> +#define GPIOH_RESET 7 > >> >> +#define GPIOI_RESET 8 > >> >> +#define GPIOJ_RESET 9 > >> >> +#define GPIOK_RESET 10 > >> >> > >> > > >> > As these are just the hardware numbers, it's better to not make them > >> > part of the binding at all. Instead, just document in the binding that > >> > one is supposed to pass the hardware number as the argument. > >> > >> The reset controller is part of the RCC (Reset & Clock Controller) IP. > >> In this version, I only provided the reset registers to the reset > >> controller driver, but as per Andreas Färber remark, I should avec a > >> single DT node for both the resets and clocks. > >> > >> In the next version I am preparing, the defines doesn't look as > >> trivial as in this version, GPIOA_RESET being 128 for instance. > >> > >> Is it fine for you if I keep the defines part of the binding? > >> > >> > > > > It's always better to avoid these files entirely, as they are > > a frequent source of merge dependencies, and they make it less > > obvious what's going on than having binary values in the dtb > > that make sense. > > I agree it is always painful to have to have to manage these merge dependencies. > What I will do, if Philipp agrees, is to list all the values in the > binding documentation. > > Doing that, the user of a reset won't have to do the calculation, and > no more merge dependencies. I'd prefer to have #defines for the reset bits if they are named in the documentation and use the names in the dts. But if you want to reference reset bits by number in the device tree instead, I won't insist. Consider using two cells in the phandle for register and bit offset instead of a single number that arbitrarily starts at 128. regards Philipp