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
| ||
|
Date: Mon, 6 Feb 2017 14:59:14 -0800 From: Stephen Boyd <sboyd@...eaurora.org> To: Florian Fainelli <f.fainelli@...il.com> Cc: Markus Mayer <markus.mayer@...adcom.com>, Markus Mayer <code@...yer.net>, Michael Turquette <mturquette@...libre.com>, Rob Herring <robh+dt@...nel.org>, Mark Rutland <mark.rutland@....com>, Viresh Kumar <viresh.kumar@...aro.org>, "Rafael J . Wysocki" <rjw@...ysocki.net>, Arnd Bergmann <arnd@...db.de>, Broadcom Kernel List <bcm-kernel-feedback-list@...adcom.com>, Linux Clock List <linux-clk@...r.kernel.org>, Power Management List <linux-pm@...r.kernel.org>, Device Tree List <devicetree@...r.kernel.org>, ARM Kernel List <linux-arm-kernel@...ts.infradead.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org> Subject: Re: [PATCH v5 1/2] dt-bindings: brcm: clocks: add binding for brcmstb-cpu-clk-div On 02/03, Florian Fainelli wrote: > On 02/03/2017 12:06 PM, Stephen Boyd wrote: > > On 02/01, Markus Mayer wrote: > >> On 20 January 2017 at 16:52, Stephen Boyd <sboyd@...eaurora.org> wrote: > >>> > >>> Are these properties used? Please don't put these types of > >>> details in DT. > >> > >> Yeah, unfortunately they are. Luckily, I think the issue can be > >> resolved quite easily, because the user of those properties isn't > >> involved in this series. > >> > >> They are currently being used by a clock driver > >> ("drivers/clk/clk-brcmstb.c") that hasn't been upstreamed yet. I > >> performed some code archeology. While I wasn't 100% successful in > >> tracking down the origins of this interface, it looks like it was > >> designed this way a while back (4+ years or so), probably before > >> device tree best practices were developed or, at least, before they > >> were widely known. > >> > >> So, what I can do is to remove the properties from the official > >> binding. (I'll send an update to that effect shortly.) Once the > >> binding is accepted upstream, we can work on fixing up the design of > >> clk-brcmstb.c, so it doesn't rely on these properties anymore (and > >> derives them from the compatible string instead), and then proceed to > >> upstream that, as well. > > > > Ok. Sounds like some cleanup needs to be done on the way > > upstream. > > > >>> This register really looks like some offset in something larger. > >>> Is there some clock controller? What's the hw block at > >>> 0xf03e2000? Maybe I already asked this. > >> > >> It looks this way, but in this case, looks are deceiving. The address > >> and the length are really correct the way they are. > >> > >> This memory area holds a range of only loosely related configuration > >> registers. It's called the Bus Interface Unit Register Set and deals > >> with configuring the CPU in general. At address 0xf03e257c, there > >> happens to be the clock divider register we need, and it's really just > >> one register, i.e. 4 bytes. > > > > We've seen this style of hardware design before. I'd prefer we > > make the "Bus Interface Unit Register Set" into one device node > > and have a driver probe for it that registers this clock. If > > other things need to be controlled in there then the driver will > > do more than just register one clock, possibly hooking into > > multiple frameworks. The compatible string can indicate which SoC > > it is if the divider register offset changes or if the register > > layout is a total free for all. > > We already have another piece of drive code that manipulates registers > in the Bus Interface Unit located in drivers/soc/bcm/brcmstb/biuctrl.c > and which has little to nothing to do with the CPU's clock ratio. And > actually another one being submitted that deals with the CPU's > read-ahead cache. I would very much prefer we keep all of them separate > and dealing with just the register offset they need to do, but that does > not mean the Device Tree binding has to look that way though. > > The binding for the BIUCTRL register made it here: > > Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt > > so we should re-use that, and have a small piece of clock provided that > just uses the relevant register range within that larger register space > and provide the CLOCK_RATIO. Does that work? > Ok. That's fine. The existing binding will be updated to include this new subnode then for the clock component? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Powered by blists - more mailing lists