Messages in this thread | | | Subject | Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms | From | Gabriel Fernandez <> | Date | Mon, 22 Aug 2016 18:06:20 +0200 |
| |
Hi Mike,
you forgot me ?
Best Regards
Gabriel
On 07/11/2016 08:58 AM, Gabriel Fernandez wrote: > > > On 07/08/2016 06:08 PM, Michael Turquette wrote: >> Quoting Gabriel Fernandez (2016-07-08 02:12:35) >>> Hi Mike, >>> >>> On 07/08/2016 03:43 AM, Michael Turquette wrote: >>>> 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. >>> The goal of my patchset was to be aligned with DRM / KMS development >>> and >>> to offer >>> the possibility to make a correct video playback on STiH407/STiH410 >>> platform. >>> Our milestone is the 4.8 for that. >>> >>> Currently people need these patches to work. >>> I'm not sure it's a good time to re-evaluate the whole thing. >>> >>> Is it possible to re-evaluate later ? >> Are you OK to break ABI later? Or at a minimum, deprecate the current >> binding (maintain it forever for legacy platforms) and create a new >> clock controller binding description that supersedes the legacy binding >> for all new platforms? >> >> If the answer to either question is "yes", then I'm OK to put it aside >> for now. But if the answer to both is "no", and this patch series is >> breaking ABI, then we really should fix it now. > > Hi Mike, > i m ok to break ABI later. > > Many Thanks ! > > Best Regards > > Gabriel. > >> Regards, >> Mike >> >>> Best regards, >>> Gabriel >>> >>>> Regards, >>>> Mike >>>> >>>>>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org> >>>>>> --- >>>>>> .../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" >
| |