linux-kernel - Re: [PATCH v8 14/16] ARM: dts: Introduce STM32F429 MCU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2672178.pv4FEQMD9D@wuerfel>
Date:	Wed, 13 May 2015 21:37:25 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	linux-arm-kernel@...ts.infradead.org
Cc:	Daniel Thompson <daniel.thompson@...aro.org>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Mark Rutland <mark.rutland@....com>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Will Deacon <will.deacon@....com>,
	Stefan Agner <stefan@...er.ch>,
	Nikolay Borisov <Nikolay.Borisov@....com>,
	Peter Meerwald <pmeerw@...erw.net>,
	"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
	Lee Jones <lee.jones@...aro.org>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Linux-Arch <linux-arch@...r.kernel.org>,
	Russell King <linux@....linux.org.uk>,
	Jonathan Corbet <corbet@....net>, Jiri Slaby <jslaby@...e.cz>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Chanwoo Choi <cw00.choi@...sung.com>,
	Andy Shevchenko <andy.shevchenko@...il.com>,
	Antti Palosaari <crope@....fi>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Kees Cook <keescook@...omium.org>,
	Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kamil Lulko <rev13@...pl>,
	Rusty Russell <rusty@...tcorp.com.au>,
	Tejun Heo <tj@...nel.org>, Rob Herring <robh+dt@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Nicolae Rosia <nicolae.rosia@...il.com>,
	Michal Marek <mmarek@...e.cz>, Paul Bolle <pebolle@...cali.nl>,
	Peter Hurley <peter@...leysoftware.com>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Kumar Gala <galak@...eaurora.org>,
	Joe Perches <joe@...ches.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andreas Färber <afaerber@...e.de>,
	Vladimir Zapolskiy <vladimir_zapolskiy@...tor.com>
Subject: Re: [PATCH v8 14/16] ARM: dts: Introduce STM32F429 MCU

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 <arnd@...db.de>:
> >>
> >> 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
> > <HOLE >     : 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.
> 
> 
> 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.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/