Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964901AbbENTiP (ORCPT ); Thu, 14 May 2015 15:38:15 -0400 Received: from mail-wi0-f176.google.com ([209.85.212.176]:34211 "EHLO mail-wi0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965026AbbENTiM (ORCPT ); Thu, 14 May 2015 15:38:12 -0400 Message-ID: <5554F9A0.1050105@linaro.org> Date: Thu, 14 May 2015 20:38:08 +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 , Arnd Bergmann CC: "linux-arm-kernel@lists.infradead.org" , 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?B?VXdlIEtsZWluZS1Lw7ZuaWc=?= , "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?B?QW5kcmVhcyBGw6RyYmVy?= , Vladimir Zapolskiy Subject: Re: [PATCH v8 14/16] ARM: dts: Introduce STM32F429 MCU References: <1431158038-3813-1-git-send-email-mcoquelin.stm32@gmail.com> <5553A608.9080402@linaro.org> <2672178.pv4FEQMD9D@wuerfel> 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: 3858 Lines: 89 On 14/05/15 17:34, Maxime Coquelin wrote: > 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? I think so. For the reset, this is essentially no change versus v7 except for applying an offset of 0x10 when calculating the base address. I won't get time to polish up the clk driver this weekend but I will try to write a documentation file proposing some clock bindings for you to look at. 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/