Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933440AbbENQe0 (ORCPT ); Thu, 14 May 2015 12:34:26 -0400 Received: from mail-wi0-f169.google.com ([209.85.212.169]:35832 "EHLO mail-wi0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964886AbbENQeS (ORCPT ); Thu, 14 May 2015 12:34:18 -0400 MIME-Version: 1.0 In-Reply-To: <2672178.pv4FEQMD9D@wuerfel> References: <1431158038-3813-1-git-send-email-mcoquelin.stm32@gmail.com> <5553A608.9080402@linaro.org> <2672178.pv4FEQMD9D@wuerfel> Date: Thu, 14 May 2015 18:34:15 +0200 Message-ID: Subject: Re: [PATCH v8 14/16] ARM: dts: Introduce STM32F429 MCU From: Maxime Coquelin To: Arnd Bergmann Cc: "linux-arm-kernel@lists.infradead.org" , Daniel Thompson , Mark Rutland , "linux-doc@vger.kernel.org" , Linus Walleij , Will Deacon , Stefan Agner , Nikolay Borisov , Peter Meerwald , "linux-api@vger.kernel.org" , Lee Jones , Mauro Carvalho Chehab , Linux-Arch , Russell King , Jonathan Corbet , Jiri Slaby , Daniel Lezcano , Chanwoo Choi , Andy Shevchenko , Antti Palosaari , Geert Uytterhoeven , "linux-serial@vger.kernel.org" , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , "devicetree@vger.kernel.org" , Kees Cook , Pawel Moll , Ian Campbell , Kamil Lulko , Rusty Russell , Tejun Heo , Rob Herring , Thomas Gleixner , Nicolae Rosia , Michal Marek , Paul Bolle , Peter Hurley , "linux-gpio@vger.kernel.org" , Greg Kroah-Hartman , "linux-kernel@vger.kernel.org" , "David S. Miller" , Philipp Zabel , Kumar Gala , Joe Perches , Andrew Morton , =?UTF-8?Q?Andreas_F=C3=A4rber?= , Vladimir Zapolskiy 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: 3489 Lines: 88 2015-05-13 21:37 GMT+02:00 Arnd Bergmann : > On Wednesday 13 May 2015 20:29:12 Daniel Thompson wrote: >> On 13/05/15 17:54, Maxime Coquelin wrote: >> > 2015-05-13 18:37 GMT+02:00 Arnd Bergmann : >> >> >> >> We should definitely try to use the same compatible string for all of >> >> them, and make a binding that is easy to use. >> >> >> >> I haven't fully understood the requirements for the various parts that >> >> are involved here. My understanding so far was that the driver could >> >> use the index from the first cell and compute >> >> >> >> void __iomem *reset_reg = rcc_base + 0x10 + 4 * index; >> >> void __iomem *clock_reg = rcc_base + 0x30 + 4 * index; >> > >> > This calculation is true, but we have to take into account there is a >> > hole in the middle, between AHB3, and APB1 register: >> >> ... and equally importantly, only allows us to use hardware mappings for >> the gated clocks. >> >> > AHB1RSTR : offset = 0x10, index = 0 >> > AHB2RSTR : offset = 0x14, index = 1 >> > AHB3RSTR : offset = 0x18, index = 2 >> > : offset = 0x1c, index = 3 >> > APB1RSTR : offset = 0x20, index = 4 >> > APB2RSTR : offset = 0x24, index = 5 >> > >> > So we have to carefully document this hole in the bindings, maybe by >> > listing indexes in the documentation? >> >> The register set has PLL, mux and dividers in the registers at 0x00, >> 0x04 and 0x08. >> >> Many of these clocks can be kept out of DT entirely because they are >> only there to feed other parts of the clock tree. However some of the >> dividers flow directly into cells that appear in device tree (such as >> the systick) and so we need to be able to reference them. >> >> In other words the proposed mapping cannot allow us to express the >> dividers properly (because the index would have to be negative): >> void __iomem *clock_reg = rcc_base + 0x30 + 4 * index; >> >> Thus I'd favour using different indexes for reset and clock bindings, >> both using the naive mapping function: >> void __iomem *reg = rcc_base + 4 * index >> >> I think that its so much easier to check against the datasheet like >> that. Admittedly is we follow the block-of-4-bytes idiom we have to >> divide a hex number by four but thats not so hard and we end up with: >> >> resets = <&rcc 8 0>; >> clocks = <&rcc 16 0>; >> >> At the end of the day if we say we want to follow the datasheet, lets be >> do it in the most direct way properly. Daniel, I'm fine with your proposal. Doing that, we can have a single compatible string for stm32 family, even if the reset start offset change between two chips. >> >> >> PS >> I've written a custom lookup function to to get from the DT index to an >> offset into the struct clk *array I'm using. That means I don't care >> much about any big holes in the register space. > > How about using the first cell to indicate the type (pll, mux, div, gate) > and the second cell for the number (between 0 and 256)? That way, the > gates numbers would match the reset numbers, and your internal mapping > function would look a bit nicer. That's another option. In this case, for reset, we will only need one cell, right? Regards, Maxime > > Arnd -- 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/