[v4,7/7] arm64: dts: mt8173: Add subsystem clock controller device nodes
diff mbox

Message ID 1437706925-3222-8-git-send-email-jamesjj.liao@mediatek.com
State New
Headers show

Commit Message

James Liao July 24, 2015, 3:02 a.m. UTC
This patch adds device nodes providing subsystem clocks on MT8173,
includes mmsys, imgsys, vdecsys, vencsys and vencltsys.

Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Comments

Daniel Kurtz July 24, 2015, 11:32 a.m. UTC | #1
On Fri, Jul 24, 2015 at 11:02 AM, James Liao <jamesjj.liao@mediatek.com> wrote:
> This patch adds device nodes providing subsystem clocks on MT8173,
> includes mmsys, imgsys, vdecsys, vencsys and vencltsys.
>
> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi | 37 ++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index a2f63e4..8731d24 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -88,6 +88,13 @@
>                 #clock-cells = <0>;
>         };
>
> +       cpum_ck: dummy_clk {

I'm not a big fan of this "dummy_clk".
The 'name' part of the devicetree node is supposed to be generic.
So, perhaps just oscillator@2, and move it down below clk32k: oscillator@1.
Otherwise:

Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>


-Dan


> +               compatible = "fixed-clock";
> +               #clock-cells = <0>;
> +               clock-frequency = <0>;
> +               clock-output-names = "cpum_ck";
> +       };
> +
>         clk26m: oscillator@0 {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
> @@ -227,6 +234,36 @@
>                         clocks = <&uart_clk>;
>                         status = "disabled";
>                 };
> +
> +               mmsys: clock-controller@14000000 {
> +                       compatible = "mediatek,mt8173-mmsys", "syscon";
> +                       reg = <0 0x14000000 0 0x1000>;
> +                       #clock-cells = <1>;
> +               };
> +
> +               imgsys: clock-controller@15000000 {
> +                       compatible = "mediatek,mt8173-imgsys", "syscon";
> +                       reg = <0 0x15000000 0 0x1000>;
> +                       #clock-cells = <1>;
> +               };
> +
> +               vdecsys: clock-controller@16000000 {
> +                       compatible = "mediatek,mt8173-vdecsys", "syscon";
> +                       reg = <0 0x16000000 0 0x1000>;
> +                       #clock-cells = <1>;
> +               };
> +
> +               vencsys: clock-controller@18000000 {
> +                       compatible = "mediatek,mt8173-vencsys", "syscon";
> +                       reg = <0 0x18000000 0 0x1000>;
> +                       #clock-cells = <1>;
> +               };
> +
> +               vencltsys: clock-controller@19000000 {
> +                       compatible = "mediatek,mt8173-vencltsys", "syscon";
> +                       reg = <0 0x19000000 0 0x1000>;
> +                       #clock-cells = <1>;
> +               };
>         };
>  };
>
> --
> 1.8.1.1.dirty
>
James Liao July 27, 2015, 2:56 a.m. UTC | #2
Hi Daniel,

On Fri, 2015-07-24 at 19:32 +0800, Daniel Kurtz wrote:
> > @@ -88,6 +88,13 @@
> >                 #clock-cells = <0>;
> >         };
> >
> > +       cpum_ck: dummy_clk {
> 
> I'm not a big fan of this "dummy_clk".
> The 'name' part of the devicetree node is supposed to be generic.
> So, perhaps just oscillator@2, and move it down below clk32k: oscillator@1.
> Otherwise:

cpum_ck is a test clock which only available in IC test. It's empty on
MT8173 evaluation or production boards. Should we name this kind of
empty clock as an oscillator? Or is there a better name for it?


Best regards,

James
Matthias Brugger July 27, 2015, 10:21 a.m. UTC | #3
On Monday, July 27, 2015 10:56:22 AM James Liao wrote:
> Hi Daniel,
> 
> On Fri, 2015-07-24 at 19:32 +0800, Daniel Kurtz wrote:
> > > @@ -88,6 +88,13 @@
> > > 
> > >                 #clock-cells = <0>;
> > >         
> > >         };
> > > 
> > > +       cpum_ck: dummy_clk {
> > 
> > I'm not a big fan of this "dummy_clk".
> > The 'name' part of the devicetree node is supposed to be generic.
> > So, perhaps just oscillator@2, and move it down below clk32k:
> > oscillator@1.
> 
> > Otherwise:
> cpum_ck is a test clock which only available in IC test. It's empty on
> MT8173 evaluation or production boards. Should we name this kind of
> empty clock as an oscillator? Or is there a better name for it?
> 

So if it will never be part of any available boards, why do you want to add 
it?

Regards,
Matthias
James Liao July 28, 2015, 5:29 a.m. UTC | #4
On Mon, 2015-07-27 at 03:21 -0700, Matthias Brugger wrote:
> On Monday, July 27, 2015 10:56:22 AM James Liao wrote:
> > Hi Daniel,
> > 
> > On Fri, 2015-07-24 at 19:32 +0800, Daniel Kurtz wrote:
> > > > @@ -88,6 +88,13 @@
> > > > 
> > > >                 #clock-cells = <0>;
> > > >         
> > > >         };
> > > > 
> > > > +       cpum_ck: dummy_clk {
> > > 
> > > I'm not a big fan of this "dummy_clk".
> > > The 'name' part of the devicetree node is supposed to be generic.
> > > So, perhaps just oscillator@2, and move it down below clk32k:
> > > oscillator@1.
> > 
> > > Otherwise:
> > cpum_ck is a test clock which only available in IC test. It's empty on
> > MT8173 evaluation or production boards. Should we name this kind of
> > empty clock as an oscillator? Or is there a better name for it?
> > 
> 
> So if it will never be part of any available boards, why do you want to add 
> it?

infra_cpum is a clock gate, and its clock comes from an external clock.
In previous versions we named the external clock as "clk_null", but it's
not accepted. A specified name is preferred even it's not available on
some boards. So I named it as cpum_ck in this patch.


Best regards,

James

Patch
diff mbox

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index a2f63e4..8731d24 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -88,6 +88,13 @@ 
 		#clock-cells = <0>;
 	};
 
+	cpum_ck: dummy_clk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+		clock-output-names = "cpum_ck";
+	};
+
 	clk26m: oscillator@0 {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -227,6 +234,36 @@ 
 			clocks = <&uart_clk>;
 			status = "disabled";
 		};
+
+		mmsys: clock-controller@14000000 {
+			compatible = "mediatek,mt8173-mmsys", "syscon";
+			reg = <0 0x14000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		imgsys: clock-controller@15000000 {
+			compatible = "mediatek,mt8173-imgsys", "syscon";
+			reg = <0 0x15000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		vdecsys: clock-controller@16000000 {
+			compatible = "mediatek,mt8173-vdecsys", "syscon";
+			reg = <0 0x16000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		vencsys: clock-controller@18000000 {
+			compatible = "mediatek,mt8173-vencsys", "syscon";
+			reg = <0 0x18000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		vencltsys: clock-controller@19000000 {
+			compatible = "mediatek,mt8173-vencltsys", "syscon";
+			reg = <0 0x19000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
 	};
 };