[PATCH 1/2] arm: dts: zynq: Move crystal freq. to board level

[PATCH 1/2] arm: dts: zynq: Move crystal freq. to board level

[Peter Crosthwaite]

The fact that all supported boards use the same 33MHz crystal is a
co-incidence. The Zynq PS support a range of crystal freqs so the
hardcoded setting should be removed from the dtsi. Re-implement it
on the board level.

This prepares support for Zynq boards with different crystal
frequencies (e.g. the Digilent ZYBO).

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

---
Im guessing long term this should be converted to a fixed clock. But
I think this at least steps in that direction.
---
 arch/arm/boot/dts/zynq-7000.dtsi      | 1 -
 arch/arm/boot/dts/zynq-parallella.dts | 4 ++++
 arch/arm/boot/dts/zynq-zc702.dts      | 4 ++++
 arch/arm/boot/dts/zynq-zc706.dts      | 4 ++++
 arch/arm/boot/dts/zynq-zed.dts        | 4 ++++
 5 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index ce2ef5b..ee3e5d6 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -243,7 +243,6 @@
 			clkc: clkc@100 {
 				#clock-cells = <1>;
 				compatible = "xlnx,ps7-clkc";
-				ps-clk-frequency = <33333333>;
 				fclk-enable = <0>;
 				clock-output-names = "armpll", "ddrpll", "iopll", "cpu_6or4x",
 						"cpu_3or2x", "cpu_2x", "cpu_1x", "ddr2x", "ddr3x",
diff --git a/arch/arm/boot/dts/zynq-parallella.dts b/arch/arm/boot/dts/zynq-parallella.dts
index e1f51ca..cd84d45 100644
--- a/arch/arm/boot/dts/zynq-parallella.dts
+++ b/arch/arm/boot/dts/zynq-parallella.dts
@@ -83,3 +83,7 @@
 &uart1 {
 	status = "okay";
 };
+
+&clkc {
+	ps-clk-frequency = <33333333>;
+};
diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
index 94e2cda..24dcece 100644
--- a/arch/arm/boot/dts/zynq-zc702.dts
+++ b/arch/arm/boot/dts/zynq-zc702.dts
@@ -135,3 +135,7 @@
 &uart1 {
 	status = "okay";
 };
+
+&clkc {
+	ps-clk-frequency = <33333333>;
+};
diff --git a/arch/arm/boot/dts/zynq-zc706.dts b/arch/arm/boot/dts/zynq-zc706.dts
index a8bbdfb..7b88399 100644
--- a/arch/arm/boot/dts/zynq-zc706.dts
+++ b/arch/arm/boot/dts/zynq-zc706.dts
@@ -114,3 +114,7 @@
 &uart1 {
 	status = "okay";
 };
+
+&clkc {
+	ps-clk-frequency = <33333333>;
+};
diff --git a/arch/arm/boot/dts/zynq-zed.dts b/arch/arm/boot/dts/zynq-zed.dts
index 697779a..4662da2 100644
--- a/arch/arm/boot/dts/zynq-zed.dts
+++ b/arch/arm/boot/dts/zynq-zed.dts
@@ -46,3 +46,7 @@
 &uart1 {
 	status = "okay";
 };
+
+&clkc {
+	ps-clk-frequency = <33333333>;
+};
-- 
1.9.1

[PATCH 2/2] arm: dts: zynq: Add Digilent ZYBO board

[Peter Crosthwaite]

Add a DTS describing the Digilent ZYBO board. Similar to ZED but with
a 50MHz crystal instead of 33MHz.

Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 arch/arm/boot/dts/Makefile      |  3 ++-
 arch/arm/boot/dts/zynq-zybo.dts | 52 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/zynq-zybo.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index d872062..e5e77f3 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -500,7 +500,8 @@ dtb-$(CONFIG_ARCH_ZYNQ) += \
 	zynq-parallella.dtb \
 	zynq-zc702.dtb \
 	zynq-zc706.dtb \
-	zynq-zed.dtb
+	zynq-zed.dtb \
+	zynq-zybo.dtb
 dtb-$(CONFIG_MACH_ARMADA_370) += \
 	armada-370-db.dtb \
 	armada-370-mirabox.dtb \
diff --git a/arch/arm/boot/dts/zynq-zybo.dts b/arch/arm/boot/dts/zynq-zybo.dts
new file mode 100644
index 0000000..b02d9d3
--- /dev/null
+++ b/arch/arm/boot/dts/zynq-zybo.dts
@@ -0,0 +1,52 @@
+/*
+ *  Copyright (C) 2011 - 2014 Xilinx
+ *  Copyright (C) 2012 National Instruments Corp.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+/dts-v1/;
+/include/ "zynq-7000.dtsi"
+
+/ {
+	model = "Zynq ZYBO Development Board";
+	compatible = "xlnx,zynq-zybo", "xlnx,zynq-7000";
+
+	memory {
+		device_type = "memory";
+		reg = <0x0 0x20000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttyPS0,115200 earlyprintk";
+	};
+
+};
+
+&gem0 {
+	status = "okay";
+	phy-mode = "rgmii-id";
+	phy-handle = <&ethernet_phy>;
+
+	ethernet_phy: ethernet-phy@0 {
+		reg = <0>;
+	};
+};
+
+&sdhci0 {
+	status = "okay";
+};
+
+&uart1 {
+	status = "okay";
+};
+
+&clkc {
+	ps-clk-frequency = <50000000>;
+};
-- 
1.9.1

Re: [PATCH 2/2] arm: dts: zynq: Add Digilent ZYBO board

[Sören Brinkmann]

Hi Peter,

On Sun, 2014-11-09 at 01:38PM +1000, Peter Crosthwaite wrote:
> Add a DTS describing the Digilent ZYBO board. Similar to ZED but with

"Digilent ZYBO" here...

[...]
> +/ {
> +	model = "Zynq ZYBO Development Board";
> +	compatible = "xlnx,zynq-zybo", "xlnx,zynq-7000";

... "xlnx,zynq-zybo" here. Seems inconsistent. IMHO, there should rather be
a digilent vendor prefix.

Everything else looks pretty straight forward and good.

	Soren

Re: [PATCH 2/2] arm: dts: zynq: Add Digilent ZYBO board

[Peter Crosthwaite]

On Mon, Nov 10, 2014 at 7:47 AM, Sören Brinkmann
<soren.brinkmann@xilinx.com> wrote:
> Hi Peter,
>
> On Sun, 2014-11-09 at 01:38PM +1000, Peter Crosthwaite wrote:
>> Add a DTS describing the Digilent ZYBO board. Similar to ZED but with
>
> "Digilent ZYBO" here...
>
> [...]
>> +/ {
>> +     model = "Zynq ZYBO Development Board";
>> +     compatible = "xlnx,zynq-zybo", "xlnx,zynq-7000";
>
> ... "xlnx,zynq-zybo" here. Seems inconsistent. IMHO, there should rather be
> a digilent vendor prefix.
>

Was going for consistency with ZED which also makes this mistake:

    model = "Zynq Zed Development Board";
    compatible = "xlnx,zynq-zed", "xlnx,zynq-7000";

We have to choose between consistency and correctness. If we do fix it
though, what is Diglent's four-letter vendor prefix?

Regards,
Peter

> Everything else looks pretty straight forward and good.
>
>         Soren

Re: [PATCH 2/2] arm: dts: zynq: Add Digilent ZYBO board

[Sören Brinkmann]

On Tue, 2014-11-11 at 08:39AM +1000, Peter Crosthwaite wrote:
> On Mon, Nov 10, 2014 at 7:47 AM, Sören Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
> > Hi Peter,
> >
> > On Sun, 2014-11-09 at 01:38PM +1000, Peter Crosthwaite wrote:
> >> Add a DTS describing the Digilent ZYBO board. Similar to ZED but with
> >
> > "Digilent ZYBO" here...
> >
> > [...]
> >> +/ {
> >> +     model = "Zynq ZYBO Development Board";
> >> +     compatible = "xlnx,zynq-zybo", "xlnx,zynq-7000";
> >
> > ... "xlnx,zynq-zybo" here. Seems inconsistent. IMHO, there should rather be
> > a digilent vendor prefix.
> >
> 
> Was going for consistency with ZED which also makes this mistake:
> 
>     model = "Zynq Zed Development Board";
>     compatible = "xlnx,zynq-zed", "xlnx,zynq-7000";
> 
> We have to choose between consistency and correctness. If we do fix it
> though, what is Diglent's four-letter vendor prefix?

Since Digilent is the board vendor, I'm for using the appropriate
prefix. Vendor prefixes are found in
Documentation/devicetree/bindings/vendor-prefixes.txt.

	Soren

Re: [PATCH 2/2] arm: dts: zynq: Add Digilent ZYBO board

[Peter Crosthwaite]

On Tue, Nov 11, 2014 at 8:42 AM, Sören Brinkmann
<soren.brinkmann@xilinx.com> wrote:
> On Tue, 2014-11-11 at 08:39AM +1000, Peter Crosthwaite wrote:
>> On Mon, Nov 10, 2014 at 7:47 AM, Sören Brinkmann
>> <soren.brinkmann@xilinx.com> wrote:
>> > Hi Peter,
>> >
>> > On Sun, 2014-11-09 at 01:38PM +1000, Peter Crosthwaite wrote:
>> >> Add a DTS describing the Digilent ZYBO board. Similar to ZED but with
>> >
>> > "Digilent ZYBO" here...
>> >
>> > [...]
>> >> +/ {
>> >> +     model = "Zynq ZYBO Development Board";
>> >> +     compatible = "xlnx,zynq-zybo", "xlnx,zynq-7000";
>> >
>> > ... "xlnx,zynq-zybo" here. Seems inconsistent. IMHO, there should rather be
>> > a digilent vendor prefix.
>> >
>>
>> Was going for consistency with ZED which also makes this mistake:
>>
>>     model = "Zynq Zed Development Board";
>>     compatible = "xlnx,zynq-zed", "xlnx,zynq-7000";
>>
>> We have to choose between consistency and correctness. If we do fix it
>> though, what is Diglent's four-letter vendor prefix?
>
> Since Digilent is the board vendor, I'm for using the appropriate
> prefix. Vendor prefixes are found in
> Documentation/devicetree/bindings/vendor-prefixes.txt.
>

It's not there,

Steven, does Digilent have a preferred dts vendor prefix?

Regards,
Peter

>         Soren

Re: [PATCH 2/2] arm: dts: zynq: Add Digilent ZYBO board

[Michal Simek]

Hi,

I think you can simple use "digilent".
The first step is to send the patch with add digilent to
Documentation/devicetree/bindings/vendor-prefixes.txt

and then feel free to send patch for fixing digilent,zed
+ zybo dts.

Thanks,
Michal


On 11/11/2014 03:10 AM, Steve Wang wrote:
> Hi, Peter,
> 
> According to naming convention, I think "dglnt" is good enough ^.^
> 
> By the way, I think there is a need to address another issue that needs to be 
> addressed for ZYBO as well (maybe in a separate thread):
> MAC address pre-load from on-board EEPROM.
> I think ZYBO might still be the only Zynq-based board that has an on-board 
> EEPROM for MAC address as well.
> 
> Regards,
> Steve
> 
> -----Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote: -----
> To: Sören Brinkmann <soren.brinkmann@xilinx.com>, Steve Wang 
> <steven.wang@digilentinc.com>
> From: Peter Crosthwaite <crosthwaitepeter@gmail.com>
> Date: 11/10/2014 02:56PM
> Cc: linux-kernel@vger.kernel.org, michals@xilinx.com, sorenb@xilinx.com
> Subject: Re: [PATCH 2/2] arm: dts: zynq: Add Digilent ZYBO board
> 
> On Tue, Nov 11, 2014 at 8:42 AM, Sören Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
>  > On Tue, 2014-11-11 at 08:39AM +1000, Peter Crosthwaite wrote:
>  >> On Mon, Nov 10, 2014 at 7:47 AM, Sören Brinkmann
>  >> <soren.brinkmann@xilinx.com> wrote:
>  >> > Hi Peter,
>  >> >
>  >> > On Sun, 2014-11-09 at 01:38PM +1000, Peter Crosthwaite wrote:
>  >> >> Add a DTS describing the Digilent ZYBO board. Similar to ZED but with
>  >> >
>  >> > "Digilent ZYBO" here...
>  >> >
>  >> > [...]
>  >> >> +/ {
>  >> >> +     model = "Zynq ZYBO Development Board";
>  >> >> +     compatible = "xlnx,zynq-zybo", "xlnx,zynq-7000";
>  >> >
>  >> > ... "xlnx,zynq-zybo" here. Seems inconsistent. IMHO, there should rather be
>  >> > a digilent vendor prefix.
>  >> >
>  >>
>  >> Was going for consistency with ZED which also makes this mistake:
>  >>
>  >>     model = "Zynq Zed Development Board";
>  >>     compatible = "xlnx,zynq-zed", "xlnx,zynq-7000";
>  >>
>  >> We have to choose between consistency and correctness. If we do fix it
>  >> though, what is Diglent's four-letter vendor prefix?
>  >
>  > Since Digilent is the board vendor, I'm for using the appropriate
>  > prefix. Vendor prefixes are found in
>  > Documentation/devicetree/bindings/vendor-prefixes.txt.
>  >
> 
> It's not there,
> 
> Steven, does Digilent have a preferred dts vendor prefix?
> 
> Regards,
> Peter
> 
>  >         Soren
> 

Re: [PATCH 2/2] arm: dts: zynq: Add Digilent ZYBO board

[Peter Crosthwaite]

Hi Steve,

On Tue, Nov 11, 2014 at 12:10 PM, Steve Wang
<steven.wang@digilentinc.com> wrote:
> Hi, Peter,
>
> According to naming convention, I think "dglnt" is good enough ^.^
>
> By the way, I think there is a need to address another issue that needs to
> be addressed for ZYBO as well (maybe in a separate thread):
> MAC address pre-load from on-board EEPROM.

So i'm not doing any networking work so I don't have visibility of
this feature. My understanding though is its an extra feature and
default networking behaviour provided by the SoC should still be
functional making this follow up work?

The alternative would be to disable gem in the dts on first commit.

> I think ZYBO might still be the only Zynq-based board that has an on-board
> EEPROM for MAC address as well.
>

Is that just software policy though? Should there be DTS level
awareness of MAC address via EEPROM contents or should the dts just
describe the EEPROM?

Regards,
Peter

> Regards,
> Steve
>
> -----Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote: -----
> To: Sören Brinkmann <soren.brinkmann@xilinx.com>, Steve Wang
> <steven.wang@digilentinc.com>
> From: Peter Crosthwaite <crosthwaitepeter@gmail.com>
> Date: 11/10/2014 02:56PM
> Cc: linux-kernel@vger.kernel.org, michals@xilinx.com, sorenb@xilinx.com
> Subject: Re: [PATCH 2/2] arm: dts: zynq: Add Digilent ZYBO board
>
>
> On Tue, Nov 11, 2014 at 8:42 AM, Sören Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
>> On Tue, 2014-11-11 at 08:39AM +1000, Peter Crosthwaite wrote:
>>> On Mon, Nov 10, 2014 at 7:47 AM, Sören Brinkmann
>>> <soren.brinkmann@xilinx.com> wrote:
>>> > Hi Peter,
>>> >
>>> > On Sun, 2014-11-09 at 01:38PM +1000, Peter Crosthwaite wrote:
>>> >> Add a DTS describing the Digilent ZYBO board. Similar to ZED but with
>>> >
>>> > "Digilent ZYBO" here...
>>> >
>>> > [...]
>>> >> +/ {
>>> >> +     model = "Zynq ZYBO Development Board";
>>> >> +     compatible = "xlnx,zynq-zybo", "xlnx,zynq-7000";
>>> >
>>> > ... "xlnx,zynq-zybo" here. Seems inconsistent. IMHO, there should
>>> > rather be
>>> > a digilent vendor prefix.
>>> >
>>>
>>> Was going for consistency with ZED which also makes this mistake:
>>>
>>>     model = "Zynq Zed Development Board";
>>>     compatible = "xlnx,zynq-zed", "xlnx,zynq-7000";
>>>
>>> We have to choose between consistency and correctness. If we do fix it
>>> though, what is Diglent's four-letter vendor prefix?
>>
>> Since Digilent is the board vendor, I'm for using the appropriate
>> prefix. Vendor prefixes are found in
>> Documentation/devicetree/bindings/vendor-prefixes.txt.
>>
>
> It's not there,
>
> Steven, does Digilent have a preferred dts vendor prefix?
>
> Regards,
> Peter
>
>>         Soren

Re: [PATCH 1/2] arm: dts: zynq: Move crystal freq. to board level

[Sören Brinkmann]

On Sun, 2014-11-09 at 01:38PM +1000, Peter Crosthwaite wrote:
> The fact that all supported boards use the same 33MHz crystal is a
> co-incidence. The Zynq PS support a range of crystal freqs so the
> hardcoded setting should be removed from the dtsi. Re-implement it
> on the board level.
> 
> This prepares support for Zynq boards with different crystal
> frequencies (e.g. the Digilent ZYBO).

Even with the 33MHz in the dtsi you can override it on the board-level.
Just like the 'status' property is overriden in board dts files.

> 
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> 
> ---
> Im guessing long term this should be converted to a fixed clock. But
> I think this at least steps in that direction.

I was against that since it makes juggling with clock names more
difficult. The problem is that the CCF uses a global name space of clock
names. Not having the oscillator as fixed-clock in DT removed that
additional complexity. But it would be possible, I guess.

> ---
>  arch/arm/boot/dts/zynq-7000.dtsi      | 1 -
>  arch/arm/boot/dts/zynq-parallella.dts | 4 ++++
>  arch/arm/boot/dts/zynq-zc702.dts      | 4 ++++
>  arch/arm/boot/dts/zynq-zc706.dts      | 4 ++++
>  arch/arm/boot/dts/zynq-zed.dts        | 4 ++++
>  5 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index ce2ef5b..ee3e5d6 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -243,7 +243,6 @@
>  			clkc: clkc@100 {
>  				#clock-cells = <1>;
>  				compatible = "xlnx,ps7-clkc";
> -				ps-clk-frequency = <33333333>;
>  				fclk-enable = <0>;
>  				clock-output-names = "armpll", "ddrpll", "iopll", "cpu_6or4x",
>  						"cpu_3or2x", "cpu_2x", "cpu_1x", "ddr2x", "ddr3x",
> diff --git a/arch/arm/boot/dts/zynq-parallella.dts b/arch/arm/boot/dts/zynq-parallella.dts
> index e1f51ca..cd84d45 100644
> --- a/arch/arm/boot/dts/zynq-parallella.dts
> +++ b/arch/arm/boot/dts/zynq-parallella.dts
> @@ -83,3 +83,7 @@
>  &uart1 {
>  	status = "okay";
>  };
> +
> +&clkc {
> +	ps-clk-frequency = <33333333>;
> +};
> diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
> index 94e2cda..24dcece 100644
> --- a/arch/arm/boot/dts/zynq-zc702.dts
> +++ b/arch/arm/boot/dts/zynq-zc702.dts
> @@ -135,3 +135,7 @@
>  &uart1 {
>  	status = "okay";
>  };
> +
> +&clkc {
> +	ps-clk-frequency = <33333333>;
> +};
> diff --git a/arch/arm/boot/dts/zynq-zc706.dts b/arch/arm/boot/dts/zynq-zc706.dts
> index a8bbdfb..7b88399 100644
> --- a/arch/arm/boot/dts/zynq-zc706.dts
> +++ b/arch/arm/boot/dts/zynq-zc706.dts
> @@ -114,3 +114,7 @@
>  &uart1 {
>  	status = "okay";
>  };
> +
> +&clkc {
> +	ps-clk-frequency = <33333333>;
> +};
> diff --git a/arch/arm/boot/dts/zynq-zed.dts b/arch/arm/boot/dts/zynq-zed.dts
> index 697779a..4662da2 100644
> --- a/arch/arm/boot/dts/zynq-zed.dts
> +++ b/arch/arm/boot/dts/zynq-zed.dts
> @@ -46,3 +46,7 @@
>  &uart1 {
>  	status = "okay";
>  };
> +
> +&clkc {
> +	ps-clk-frequency = <33333333>;
> +};

I think we currently have the nodes more or less in alphabetical order.
I'd prefer if we maintained that.

Other than that:
Acked-by: Soren Brinkmann <soren.brinkmann@xilinx.com>

	Soren

Re: [PATCH 1/2] arm: dts: zynq: Move crystal freq. to board level

[Peter Crosthwaite]

On Mon, Nov 10, 2014 at 7:34 AM, Sören Brinkmann
<soren.brinkmann@xilinx.com> wrote:
> On Sun, 2014-11-09 at 01:38PM +1000, Peter Crosthwaite wrote:
>> The fact that all supported boards use the same 33MHz crystal is a
>> co-incidence. The Zynq PS support a range of crystal freqs so the
>> hardcoded setting should be removed from the dtsi. Re-implement it
>> on the board level.
>>
>> This prepares support for Zynq boards with different crystal
>> frequencies (e.g. the Digilent ZYBO).
>
> Even with the 33MHz in the dtsi you can override it on the board-level.
> Just like the 'status' property is overriden in board dts files.
>

Do you want the deletion undone? Even with override capability I think
it should be removed as the number is board level specific and the
dtsi should be limited to SoC level information.

My understanding is the driver will default to 33MHz as well as a
catch-all, so anyone rolling their own top level OOT DTS is going to
be ok too.

>>
>> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>>
>> ---
>> Im guessing long term this should be converted to a fixed clock. But
>> I think this at least steps in that direction.
>
> I was against that since it makes juggling with clock names more
> difficult. The problem is that the CCF uses a global name space of clock
> names.

I thought it was just a

clocks = < &phandle >

Where's the namespacing issue?

Btw I think the clocks=phandle would be populated the the dts as well.
So the DTSI would have no clocks = node, and the dts must populate it.
This allows support for an on-board off-soc clock controller
controlling the PS clock (which is in theory supported by the SoC).

 Not having the oscillator as fixed-clock in DT removed that
> additional complexity. But it would be possible, I guess.
>
>> ---
>>  arch/arm/boot/dts/zynq-7000.dtsi      | 1 -
>>  arch/arm/boot/dts/zynq-parallella.dts | 4 ++++
>>  arch/arm/boot/dts/zynq-zc702.dts      | 4 ++++
>>  arch/arm/boot/dts/zynq-zc706.dts      | 4 ++++
>>  arch/arm/boot/dts/zynq-zed.dts        | 4 ++++
>>  5 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
>> index ce2ef5b..ee3e5d6 100644
>> --- a/arch/arm/boot/dts/zynq-7000.dtsi
>> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
>> @@ -243,7 +243,6 @@
>>                       clkc: clkc@100 {
>>                               #clock-cells = <1>;
>>                               compatible = "xlnx,ps7-clkc";
>> -                             ps-clk-frequency = <33333333>;
>>                               fclk-enable = <0>;
>>                               clock-output-names = "armpll", "ddrpll", "iopll", "cpu_6or4x",
>>                                               "cpu_3or2x", "cpu_2x", "cpu_1x", "ddr2x", "ddr3x",
>> diff --git a/arch/arm/boot/dts/zynq-parallella.dts b/arch/arm/boot/dts/zynq-parallella.dts
>> index e1f51ca..cd84d45 100644
>> --- a/arch/arm/boot/dts/zynq-parallella.dts
>> +++ b/arch/arm/boot/dts/zynq-parallella.dts
>> @@ -83,3 +83,7 @@
>>  &uart1 {
>>       status = "okay";
>>  };
>> +
>> +&clkc {
>> +     ps-clk-frequency = <33333333>;
>> +};
>> diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
>> index 94e2cda..24dcece 100644
>> --- a/arch/arm/boot/dts/zynq-zc702.dts
>> +++ b/arch/arm/boot/dts/zynq-zc702.dts
>> @@ -135,3 +135,7 @@
>>  &uart1 {
>>       status = "okay";
>>  };
>> +
>> +&clkc {
>> +     ps-clk-frequency = <33333333>;
>> +};
>> diff --git a/arch/arm/boot/dts/zynq-zc706.dts b/arch/arm/boot/dts/zynq-zc706.dts
>> index a8bbdfb..7b88399 100644
>> --- a/arch/arm/boot/dts/zynq-zc706.dts
>> +++ b/arch/arm/boot/dts/zynq-zc706.dts
>> @@ -114,3 +114,7 @@
>>  &uart1 {
>>       status = "okay";
>>  };
>> +
>> +&clkc {
>> +     ps-clk-frequency = <33333333>;
>> +};
>> diff --git a/arch/arm/boot/dts/zynq-zed.dts b/arch/arm/boot/dts/zynq-zed.dts
>> index 697779a..4662da2 100644
>> --- a/arch/arm/boot/dts/zynq-zed.dts
>> +++ b/arch/arm/boot/dts/zynq-zed.dts
>> @@ -46,3 +46,7 @@
>>  &uart1 {
>>       status = "okay";
>>  };
>> +
>> +&clkc {
>> +     ps-clk-frequency = <33333333>;
>> +};
>
> I think we currently have the nodes more or less in alphabetical order.
> I'd prefer if we maintained that.
>

Will fix.

Regards,
Peter

> Other than that:
> Acked-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
>
>         Soren

Re: [PATCH 1/2] arm: dts: zynq: Move crystal freq. to board level

[Sören Brinkmann]

On Tue, 2014-11-11 at 08:35AM +1000, Peter Crosthwaite wrote:
> On Mon, Nov 10, 2014 at 7:34 AM, Sören Brinkmann
> <soren.brinkmann@xilinx.com> wrote:
> > On Sun, 2014-11-09 at 01:38PM +1000, Peter Crosthwaite wrote:
> >> The fact that all supported boards use the same 33MHz crystal is a
> >> co-incidence. The Zynq PS support a range of crystal freqs so the
> >> hardcoded setting should be removed from the dtsi. Re-implement it
> >> on the board level.
> >>
> >> This prepares support for Zynq boards with different crystal
> >> frequencies (e.g. the Digilent ZYBO).
> >
> > Even with the 33MHz in the dtsi you can override it on the board-level.
> > Just like the 'status' property is overriden in board dts files.
> >
> 
> Do you want the deletion undone? Even with override capability I think
> it should be removed as the number is board level specific and the
> dtsi should be limited to SoC level information.

I'm fine with it. Just wanted to point out that patch 2 does not
strictly require this change and can stand on its own.

[...]
> >> Im guessing long term this should be converted to a fixed clock. But
> >> I think this at least steps in that direction.
> >
> > I was against that since it makes juggling with clock names more
> > difficult. The problem is that the CCF uses a global name space of clock
> > names.
> 
> I thought it was just a
> 
> clocks = < &phandle >
> 
> Where's the namespacing issue?
> 
> Btw I think the clocks=phandle would be populated the the dts as well.
> So the DTSI would have no clocks = node, and the dts must populate it.
> This allows support for an on-board off-soc clock controller
> controlling the PS clock (which is in theory supported by the SoC).

Every call to clk_register needs to be passed in the clock's parent
(unless it's a root clock). That parent is specified by its name. You
won't see it as user/consumer, but when implementing a clock-provider.
I think there is nothing preventing such a change, but it would make
things more complicated for no good reason. Even if you have an off-chip
clock controller, if that one doesn't provide a fixed clock input to
Zynq, things are likely to break.

	Sören