From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Turquette Subject: Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms Date: Thu, 07 Jul 2016 18:43:33 -0700 Message-ID: <146794221378.73491.2803164814485320331@resonance> References: <1466068833-5055-1-git-send-email-gabriel.fernandez@linaro.org> <1466068833-5055-3-git-send-email-gabriel.fernandez@linaro.org> <20160619150458.GA26824@rob-hp-laptop> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <20160619150458.GA26824@rob-hp-laptop> Sender: linux-kernel-owner@vger.kernel.org To: Rob Herring , Gabriel Fernandez Cc: Mark Rutland , Ian Campbell , Kumar Gala , Srinivas Kandagatla , Maxime Coquelin , Patrice Chotard , Russell King , Stephen Boyd , Olivier Bideau , Geert Uytterhoeven , Sebastian Hesselbarth , Andrzej Hajda , Pankaj Dev , Dinh Nguyen , Arnd Bergmann , Thierry Reding , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kernel@stlinux.com, linux-clk@vger.kernel.org, Lee Jones , Peter Gri List-Id: devicetree@vger.kernel.org Quoting Rob Herring (2016-06-19 08:04:58) > On Thu, Jun 16, 2016 at 11:20:22AM +0200, Gabriel Fernandez wrote: > > This patch reworks the clock binding to avoid too much detail in DT. > > Now we have only compatible string per type of clock > > (remark from Rob https://lkml.org/lkml/2016/5/25/492) > > > > I have no idea what the clock trees and clock controller in these chips > look like, so it's hard to say if the changes here are good. It still > looks like things are somewhat fine grained clocks in DT. I'll leave > it up to the platform maintainers to decide... Is this series breaking ABI? If yes, why not do what Maxime did for the Allwinner/sunxi clocks and just fully convert over to a one-node-per-clock-controller binding? This one-node-per-clock stuff is pretty unfortunate, and if we're deprecating platforms (patch #1) then now might be a good time to re-evaluate the whole thing. Regards, Mike > > > Signed-off-by: Gabriel Fernandez > > --- > > .../devicetree/bindings/clock/st/st,clkgen-mux.txt | 2 +- > > .../devicetree/bindings/clock/st/st,clkgen-pll.txt | 11 ++-- > > .../devicetree/bindings/clock/st/st,clkgen.txt | 2 +- > > .../devicetree/bindings/clock/st/st,quadfs.txt | 6 +-- > > drivers/clk/st/clkgen-fsyn.c | 41 ++++++-------- > > drivers/clk/st/clkgen-mux.c | 28 ++++------ > > drivers/clk/st/clkgen-pll.c | 62 ++++++++++------------ > > 7 files changed, 65 insertions(+), 87 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/clock/st/st,clkgen-mux.txt b/Documentation/devicetree/bindings/clock/st/st,clkgen-mux.txt > > index 4d277d6..9a46cb1d7 100644 > > --- a/Documentation/devicetree/bindings/clock/st/st,clkgen-mux.txt > > +++ b/Documentation/devicetree/bindings/clock/st/st,clkgen-mux.txt > > @@ -10,7 +10,7 @@ This binding uses the common clock binding[1]. > > Required properties: > > > > - compatible : shall be: > > - "st,stih407-clkgen-a9-mux", "st,clkgen-mux" > > + "st,stih407-clkgen-a9-mux" > > > > - #clock-cells : from common clock binding; shall be set to 0. > > > > diff --git a/Documentation/devicetree/bindings/clock/st/st,clkgen-pll.txt b/Documentation/devicetree/bindings/clock/st/st,clkgen-pll.txt > > index c9fd674..be0b043 100644 > > --- a/Documentation/devicetree/bindings/clock/st/st,clkgen-pll.txt > > +++ b/Documentation/devicetree/bindings/clock/st/st,clkgen-pll.txt > > @@ -9,11 +9,10 @@ Base address is located to the parent node. See clock binding[2] > > Required properties: > > > > - compatible : shall be: > > - "st,stih407-plls-c32-a0", "st,clkgen-plls-c32" > > - "st,stih407-plls-c32-a9", "st,clkgen-plls-c32" > > - "sst,plls-c32-cx_0", "st,clkgen-plls-c32" > > - "sst,plls-c32-cx_1", "st,clkgen-plls-c32" > > - "st,stih418-plls-c28-a9", "st,clkgen-plls-c32" > > > + "st,clkgen-pll0" > > + "st,clkgen-pll0" > > Repeated. Supposed to be 0 and 1? This seems a bit generic, too. > > > + "st,stih407-clkgen-plla9" > > + "st,stih418-clkgen-plla9"