Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms
On 08/25/2016 02:11 AM, Michael Turquette wrote: Quoting Gabriel Fernandez (2016-08-22 09:06:20) 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. Hi Gabriel, This change never received any other reviews, and no pings before v4.7 was released. Sorry that it fell through the cracks, but always feel free to re-ping leading up to the merge window. Can you rebase this against -rc1? Also, do you have a plan to rework the binding to move away from the one-node-per-clock style? Regards, Mike Hi Mike, Ok i will send a v3 rebased on a rc1. Concerning the binding rework, i plan to start work at the end of October. Thanks Gabriel Many Thanks ! Best Regards Gabriel. Regards, Mike Best regards, Gabriel 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"
Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms
Quoting Gabriel Fernandez (2016-08-22 09:06:20) > 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. Hi Gabriel, This change never received any other reviews, and no pings before v4.7 was released. Sorry that it fell through the cracks, but always feel free to re-ping leading up to the merge window. Can you rebase this against -rc1? Also, do you have a plan to rework the binding to move away from the one-node-per-clock style? Regards, Mike > > > > Many Thanks ! > > > > Best Regards > > > > Gabriel. > > > >> Regards, > >> Mike > >> > >>> Best regards, > >>> Gabriel > >>> > 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
Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms
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 --- .../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"
Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms
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 --- .../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"
Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms
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. Regards, Mike > > Best regards, > Gabriel > > > 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" >
Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms
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 ? Best regards, Gabriel 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"
Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms
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"
Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms
Hi Rob, On 19 June 2016 at 17:04, Rob Herring wrote: > 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... > >> 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. Yes you are right, it's 0 and 1. I wait remarks from Mike or Stephen before send a V3. Thanks Rob BR Gabriel > >> + "st,stih407-clkgen-plla9" >> + "st,stih418-clkgen-plla9"
Re: [RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms
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... > 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"
[RESEND PATCH v2 02/13] drivers: clk: st: Simplify clock binding of STiH4xx platforms
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)
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"
+ "st,stih407-clkgen-plla9"
+ "st,stih418-clkgen-plla9"
- #clock-cells : From common clock binding; shall be set to 1.
@@ -29,7 +28,7 @@ Example:
clockgen_a9_pll: clockgen-a9-pll {
#clock-cells = <1>;
- compatible = "st,stih407-plls-c32-a9",
"st,clkgen-plls-c32";
+ compatible = "st,stih407-clkgen-plla9";
clocks = <&clk_sysin>;
diff --git a/Documentation/devicetree/bindings/clock/st/st,clkgen.txt
b/Documentation/devicetree/bindings/clock/st/st,clkgen.txt
index c764d6b..c35390f 100644
--- a/Documentation/devicetree/bindings/clock/st/st,clkgen.txt
+++ b/Documentation/devicetree/bindings/clock/st/st,clkgen.txt
@@ -48,7 +48,7 @@ Example:
clk_s_a0_pll: clk-s-a0-pll {
#clock-cells = <1>;
- compatible = "st,stih407-plls-c32-a0",
"st,clkgen-plls-c32";
+ compatible = "st,clkgen-pll0";
clocks = <&clk_sysin>;
diff --git a/Documentation/devicetree/bindings/clock/st/st,quadfs.txt
b/Documentation/devicetree/bindings/clock/st/st,quadfs.txt
index 6d81b11..d93d493 100644
--- a/Documentation/devicetree/bindings/clock/st/st,quadfs.txt
+++ b/Documentation/devicetree/bindings/clock/st/st,quadfs.txt
@@ -11,8 +11,8 @@ This binding uses the common clock binding[1].
Required properties:
- compatible : shall be:
- "st,stih407-quadfs660-C","st,quadfs"
- "st,stih407-quadfs660-D","st,quadfs"
+ "st,quadfs"
+ "st,quadfs-pll"
- #clock-cells : from common clock binding; shall be set to 1.
@@ -33,7 +33,7 @@ Example:
clk_s_c0_quadfs: clk-s-c0-quadfs@9103000 {
#clock-cells = <1>;
- compatible = "st,stih407-quadfs660-C", "st,quadfs";
+ compatible = "st,quadfs-pll";
reg = <0x9103000 0x1000>;
clocks = <&clk_sysin>;
diff --git a/drivers/clk/st/clkgen-fsyn.c b/drivers/clk/st/clkgen-fsyn.c
index 4f43b2e..53fd047 100644
--- a/drivers/clk/st/clkgen-fsyn.c
+++ b/drivers/clk/st/clkgen-fsyn.c
@@ -819,18 +819,6 @@ static struct clk * __init st_clk_register_quadfs_fsynth(
return clk;
}
-static const struct of_device_id quadfs_of_match[] = {
- {
- .compatible = "st,stih407-quadfs660-C",
- .data = &st_fs660c32_C
- },
- {
- .compatible = "st,stih407-quadfs660-D",
- .data = &st_fs660c32_D
- },
- {}
-};
-
static void __init st_of_create_quadfs_fsynths(
struct device_node *np, const char *pll_name,
struct clkgen_quadfs_data *quadfs, void __iomem *reg,
@@ -886,18 +874,14 @@ static void __init st_of_create_quadfs_fsynths(
of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
}
-static void __init st_of_quadfs_
