[PATCH v2 0/5] ARM: zynq: DT update

[PATCH v2 0/5] ARM: zynq: DT update

[Soren Brinkmann]

Hi,

here's the complete series now. One fix for cpufreq from our vendor tree
and migration to pre-processor syntax.

	Thanks,
	Sören

Changes in v2:
 - Send full series (added patches 1, 4, 5)

Soren Brinkmann (5):
  ARM: zynq: DT: Add 'clock-latency' property
  ARM: zynq: dt: Convert to preprocessor includes
  ARM: zynq: dt: Use #defines for interrupt specifiers
  dt-bindings: clock: Add Zynq-7000 header
  ARM: zynq: dt: Use #defines for clock specifiers

 arch/arm/boot/dts/zynq-7000.dtsi      | 64 ++++++++++++++++++++++-------------
 arch/arm/boot/dts/zynq-zc702.dts      |  2 +-
 arch/arm/boot/dts/zynq-zc706.dts      |  2 +-
 arch/arm/boot/dts/zynq-zed.dts        |  2 +-
 include/dt-bindings/clock/zynq-7000.h | 64 +++++++++++++++++++++++++++++++++++
 5 files changed, 107 insertions(+), 27 deletions(-)
 create mode 100644 include/dt-bindings/clock/zynq-7000.h

-- 
1.9.1.1.gbb9f595

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/5] ARM: zynq: DT: Add 'clock-latency' property

[Soren Brinkmann]

Specify the 'clock-latency' property to avoid certain cpufreq governors
from refusing to work with the following error:
  ondemand governor failed, too long transition latency of HW, fallback to performance governor

Reported-by: Mike Looijmans <mike.looijmans@topic.nl>
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
Tested-by: Mike Looijmans <mike.looijmans@topic.nl>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
This is a fix from our vendor tree

Changes in v2:
 - This patch has been added

---
 arch/arm/boot/dts/zynq-7000.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 789d0bacc110..20a13cba65a3 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -24,6 +24,7 @@
 			device_type = "cpu";
 			reg = <0>;
 			clocks = <&clkc 3>;
+			clock-latency = <1000>;
 			operating-points = <
 				/* kHz    uV */
 				666667  1000000
-- 
1.9.1.1.gbb9f595

Re: [PATCH v2 1/5] ARM: zynq: DT: Add 'clock-latency' property

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]

On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
> Specify the 'clock-latency' property to avoid certain cpufreq governors
> from refusing to work with the following error:
>   ondemand governor failed, too long transition latency of HW, fallback to performance governor
> 
> Reported-by: Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org>
> Signed-off-by: Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> Tested-by: Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org>
> Signed-off-by: Michal Simek <michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
> This is a fix from our vendor tree
> 
> Changes in v2:
>  - This patch has been added
> 
> ---
>  arch/arm/boot/dts/zynq-7000.dtsi | 1 +
>  1 file changed, 1 insertion(+)

Applied to zynq-dt.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH v2 1/5] ARM: zynq: DT: Add 'clock-latency' property

[Sören Brinkmann]

On Mon, 2014-04-07 at 07:46AM +0200, Michal Simek wrote:
> On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
> > Specify the 'clock-latency' property to avoid certain cpufreq governors
> > from refusing to work with the following error:
> >   ondemand governor failed, too long transition latency of HW, fallback to performance governor
> > 
> > Reported-by: Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org>
> > Signed-off-by: Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> > Tested-by: Mike Looijmans <mike.looijmans-Oq418RWZeHk@public.gmane.org>
> > Signed-off-by: Michal Simek <michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> > ---
> > This is a fix from our vendor tree
> > 
> > Changes in v2:
> >  - This patch has been added
> > 
> > ---
> >  arch/arm/boot/dts/zynq-7000.dtsi | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Applied to zynq-dt.

Actually, this should probably go in as fix into the current (3.15)
release cycle.

	Thanks,
	Sören


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/5] ARM: zynq: dt: Convert to preprocessor includes

[Soren Brinkmann]

Convert all Zynq DT files to the dtc preprocessor include syntax.
This allows to include header files in the devicetrees like other
SoC-types already do.

Inspired-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
(http://www.spinics.net/lists/arm-kernel/msg319832.html)

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---

Changes in v2: None

 arch/arm/boot/dts/zynq-7000.dtsi | 3 ++-
 arch/arm/boot/dts/zynq-zc702.dts | 2 +-
 arch/arm/boot/dts/zynq-zc706.dts | 2 +-
 arch/arm/boot/dts/zynq-zed.dts   | 2 +-
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 20a13cba65a3..66613d04de5d 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -10,7 +10,8 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  */
-/include/ "skeleton.dtsi"
+
+#include "skeleton.dtsi"
 
 / {
 	compatible = "xlnx,zynq-7000";
diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
index c913f77a21eb..07713d3c716a 100644
--- a/arch/arm/boot/dts/zynq-zc702.dts
+++ b/arch/arm/boot/dts/zynq-zc702.dts
@@ -12,7 +12,7 @@
  * GNU General Public License for more details.
  */
 /dts-v1/;
-/include/ "zynq-7000.dtsi"
+#include "zynq-7000.dtsi"
 
 / {
 	model = "Zynq ZC702 Development Board";
diff --git a/arch/arm/boot/dts/zynq-zc706.dts b/arch/arm/boot/dts/zynq-zc706.dts
index 88f62c50382e..cf6566cdb02a 100644
--- a/arch/arm/boot/dts/zynq-zc706.dts
+++ b/arch/arm/boot/dts/zynq-zc706.dts
@@ -13,7 +13,7 @@
  * GNU General Public License for more details.
  */
 /dts-v1/;
-/include/ "zynq-7000.dtsi"
+#include "zynq-7000.dtsi"
 
 / {
 	model = "Zynq ZC706 Development Board";
diff --git a/arch/arm/boot/dts/zynq-zed.dts b/arch/arm/boot/dts/zynq-zed.dts
index 82d7ef1a9a9c..1541716e2cfb 100644
--- a/arch/arm/boot/dts/zynq-zed.dts
+++ b/arch/arm/boot/dts/zynq-zed.dts
@@ -13,7 +13,7 @@
  * GNU General Public License for more details.
  */
 /dts-v1/;
-/include/ "zynq-7000.dtsi"
+#include "zynq-7000.dtsi"
 
 / {
 	model = "Zynq Zed Development Board";
-- 
1.9.1.1.gbb9f595

Re: [PATCH v2 2/5] ARM: zynq: dt: Convert to preprocessor includes

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]

Hi Soren,

On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
> Convert all Zynq DT files to the dtc preprocessor include syntax.
> This allows to include header files in the devicetrees like other
> SoC-types already do.
> 
> Inspired-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> (http://www.spinics.net/lists/arm-kernel/msg319832.html)
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>

These 4 patches needs more wider discussion if this is helpful or
not. Currently I can't see any value in it because everything
is just generated and fixed. I think I had the same discussion
with Laurent some weeks ago regarding this.
IRC the origin idea to use this was especially for people who
writing these DTS by hand which is not our case - at least
for majority of our customers.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH v2 2/5] ARM: zynq: dt: Convert to preprocessor includes

[Mike Looijmans]

On 04/07/2014 07:58 AM, Michal Simek wrote:
> Hi Soren,
>
> On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
>> Convert all Zynq DT files to the dtc preprocessor include syntax.
>> This allows to include header files in the devicetrees like other
>> SoC-types already do.
>>
>> Inspired-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> (http://www.spinics.net/lists/arm-kernel/msg319832.html)
>>
>> Signed-off-by: Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
>
> These 4 patches needs more wider discussion if this is helpful or
> not. Currently I can't see any value in it because everything
> is just generated and fixed. I think I had the same discussion
> with Laurent some weeks ago regarding this.

I would be kinda neutral here. I'd consider it helpful, it improves 
readability (regardless of whether they are generated or hand crafted). That's 
convenient for things like interrupt sensitivity, I can't remember whether 4 
is level or edge type. On the other hand, the clock indices are just as much 
magic numbers as the memory addresses. If I suspect an error in that area, I'd 
start by lokking in /sys/kernel/debug/clk but wouldn't start in the DT.

> IRC the origin idea to use this was especially for people who
> writing these DTS by hand which is not our case - at least
> for majority of our customers.

I write them by hand. Is there any other way?

Mike.


Met vriendelijke groet / kind regards,

Mike Looijmans

TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) (0) 499 33 69 79
Telefax:  (+31) (0) 499 33 69 70
E-mail: mike.looijmans-Oq418RWZeHk@public.gmane.org
Website: www.topic.nl

Please consider the environment before printing this e-mail

Visit us at the Hannover Messe 7 - 11 April 2014 - Hall 002/D10 (Dutch Pavillion)
http://www.hannovermesse.de/exhibitor/topic-embedded-products/V229623

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/5] ARM: zynq: dt: Convert to preprocessor includes

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2211 bytes --]

Hi Mike,

On 04/07/2014 02:17 PM, Mike Looijmans wrote:
> On 04/07/2014 07:58 AM, Michal Simek wrote:
>> Hi Soren,
>>
>> On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
>>> Convert all Zynq DT files to the dtc preprocessor include syntax.
>>> This allows to include header files in the devicetrees like other
>>> SoC-types already do.
>>>
>>> Inspired-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> (http://www.spinics.net/lists/arm-kernel/msg319832.html)
>>>
>>> Signed-off-by: Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
>>
>> These 4 patches needs more wider discussion if this is helpful or
>> not. Currently I can't see any value in it because everything
>> is just generated and fixed. I think I had the same discussion
>> with Laurent some weeks ago regarding this.
> 
> I would be kinda neutral here. I'd consider it helpful, it improves readability (regardless of whether they 
are generated or hand crafted). That's convenient for things like interrupt sensitivity,
I can't remember whether 4 is level or edge type. On the other hand, the clock indices are just
 as much magic numbers as the memory addresses. If I suspect an error in that area, I'd start by lokking
in /sys/kernel/debug/clk but wouldn't start in the DT.
> 
>> IRC the origin idea to use this was especially for people who
>> writing these DTS by hand which is not our case - at least
>> for majority of our customers.
> 
> I write them by hand. Is there any other way?

Device-tree BSP and in 2014.01 there will be new BSP which just generate
them directly from the Vivado tools which just target your reference design.
You can connect your custom IP (or Xilinx or 3rd party) directly to the GIC
which using different IRQ sensitivity with whatever register addresses
and make no sense to write it by hand.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH v2 2/5] ARM: zynq: dt: Convert to preprocessor includes

[Jason Gunthorpe]

On Mon, Apr 07, 2014 at 02:24:07PM +0200, Michal Simek wrote:

> Device-tree BSP and in 2014.01 there will be new BSP which just
> generate them directly from the Vivado tools which just target your
> reference design.  You can connect your custom IP (or Xilinx or 3rd
> party) directly to the GIC which using different IRQ sensitivity
> with whatever register addresses and make no sense to write it by
> hand.

On our Zynq design here we ended up being unwilling to use platform
generation from Vivado. Basically all our IP was custom, so there was
no win at all to invoking the complexity of the automatic tools.

Thus we write the DT by hand, and our DT is complex, integrating
peripherals that span two FPGAs.

I think the in-kernel DT should use the kernel conventions, which
means using #include and the binding constants over magic values.

Jason
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/5] ARM: zynq: dt: Convert to preprocessor includes

[Steffen Trumtrar]

Hi!

On Mon, Apr 07, 2014 at 11:10:12AM -0600, Jason Gunthorpe wrote:
> On Mon, Apr 07, 2014 at 02:24:07PM +0200, Michal Simek wrote:
> 
> > Device-tree BSP and in 2014.01 there will be new BSP which just
> > generate them directly from the Vivado tools which just target your
> > reference design.  You can connect your custom IP (or Xilinx or 3rd
> > party) directly to the GIC which using different IRQ sensitivity
> > with whatever register addresses and make no sense to write it by
> > hand.
> 
> On our Zynq design here we ended up being unwilling to use platform
> generation from Vivado. Basically all our IP was custom, so there was
> no win at all to invoking the complexity of the automatic tools.
> 
> Thus we write the DT by hand, and our DT is complex, integrating
> peripherals that span two FPGAs.
> 
> I think the in-kernel DT should use the kernel conventions, which
> means using #include and the binding constants over magic values.
> 

ACK.

If in doubt follow common mainline practice. Although using includes
for DT is not necessarily common practice, readability of DTs is
really important IMHO.

Regards,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/5] ARM: zynq: dt: Convert to preprocessor includes

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]

On 04/07/2014 08:02 PM, Steffen Trumtrar wrote:
> Hi!
> 
> On Mon, Apr 07, 2014 at 11:10:12AM -0600, Jason Gunthorpe wrote:
>> On Mon, Apr 07, 2014 at 02:24:07PM +0200, Michal Simek wrote:
>>
>>> Device-tree BSP and in 2014.01 there will be new BSP which just
>>> generate them directly from the Vivado tools which just target your
>>> reference design.  You can connect your custom IP (or Xilinx or 3rd
>>> party) directly to the GIC which using different IRQ sensitivity
>>> with whatever register addresses and make no sense to write it by
>>> hand.
>>
>> On our Zynq design here we ended up being unwilling to use platform
>> generation from Vivado. Basically all our IP was custom, so there was
>> no win at all to invoking the complexity of the automatic tools.
>>
>> Thus we write the DT by hand, and our DT is complex, integrating
>> peripherals that span two FPGAs.
>>
>> I think the in-kernel DT should use the kernel conventions, which
>> means using #include and the binding constants over magic values.
>>
> 
> ACK.
> 
> If in doubt follow common mainline practice. Although using includes
> for DT is not necessarily common practice, readability of DTs is
> really important IMHO.

Let me give you one example. When you add xilinx intc controller
to zynq HW design which uses gic with headers you are using
then you will find out that sensitivity for both controllers
are just different.
This is just one case I am aware of. I expect there will be one more.

Also dtses from kernel are copied to other project because separation
from kernel hasn't be done but there is any plan regarding this.

Using dtc preprocessor and macros improve DTS readability but at the same
time force other users to copy all necessary files from the kernel
to that projects which is just hassle.
With example above there are also cases where using macros is just wrong
that's why I don't want to start to use it.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH v2 2/5] ARM: zynq: dt: Convert to preprocessor includes

[Jason Gunthorpe]

On Tue, Apr 08, 2014 at 09:03:27AM +0200, Michal Simek wrote:

> > If in doubt follow common mainline practice. Although using includes
> > for DT is not necessarily common practice, readability of DTs is
> > really important IMHO.
> 
> Let me give you one example. When you add xilinx intc controller
> to zynq HW design which uses gic with headers you are using
> then you will find out that sensitivity for both controllers
> are just different.
> This is just one case I am aware of. I expect there will be one more.

I'm not sure I see the problem here, just because some bindings can't
use the standard shared constants doesn't mean the GIC bindings and
users should avoid them. The binding documentation is supposed to make
it clear what is correct.

It is just as easy to get confused with numbers, does 4 mean
XILINX_INTC_IRQ_RISING or IRQ_TYPE_LEVEL_HIGH ?

> Using dtc preprocessor and macros improve DTS readability but at the same
> time force other users to copy all necessary files from the kernel
> to that projects which is just hassle.

You can run the DTS through cpp before you export it out of the kernel
environment, then you get a flat file with no includes.

The shared kernel conventions are more important than constraints from
outside projects.

Jason
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/5] ARM: zynq: dt: Convert to preprocessor includes

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2230 bytes --]

On 04/08/2014 07:27 PM, Jason Gunthorpe wrote:
> On Tue, Apr 08, 2014 at 09:03:27AM +0200, Michal Simek wrote:
> 
>>> If in doubt follow common mainline practice. Although using includes
>>> for DT is not necessarily common practice, readability of DTs is
>>> really important IMHO.
>>
>> Let me give you one example. When you add xilinx intc controller
>> to zynq HW design which uses gic with headers you are using
>> then you will find out that sensitivity for both controllers
>> are just different.
>> This is just one case I am aware of. I expect there will be one more.
> 
> I'm not sure I see the problem here, just because some bindings can't
> use the standard shared constants doesn't mean the GIC bindings and
> users should avoid them. The binding documentation is supposed to make
> it clear what is correct.
> 
> It is just as easy to get confused with numbers, does 4 mean
> XILINX_INTC_IRQ_RISING or IRQ_TYPE_LEVEL_HIGH ?

That's why you have there biding documentation to exactly know what it is.

>> Using dtc preprocessor and macros improve DTS readability but at the same
>> time force other users to copy all necessary files from the kernel
>> to that projects which is just hassle.
> 
> You can run the DTS through cpp before you export it out of the kernel
> environment, then you get a flat file with no includes.

What's the result?
1. DTSI and DTS together which completely break hierarchy
2. DTS without comments

It means, yes, you get a file when you go through cpp but different
then you have now.

> The shared kernel conventions are more important than constraints from
> outside projects.

zynq-7000.dtsi is fixed and you can't just change it based on your project.
For things which are in your board file like zynq-zc702 then you can use
whatever you like.

Maybe I just need some time to get used to it but currently...

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH v2 2/5] ARM: zynq: dt: Convert to preprocessor includes

[Sören Brinkmann]

On Mon, 2014-04-07 at 02:17PM +0200, Mike Looijmans wrote:
> On 04/07/2014 07:58 AM, Michal Simek wrote:
> >Hi Soren,
> >
> >On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
> >>Convert all Zynq DT files to the dtc preprocessor include syntax.
> >>This allows to include header files in the devicetrees like other
> >>SoC-types already do.
> >>
> >>Inspired-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> >>(http://www.spinics.net/lists/arm-kernel/msg319832.html)
> >>
> >>Signed-off-by: Soren Brinkmann <soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> >
> >These 4 patches needs more wider discussion if this is helpful or
> >not. Currently I can't see any value in it because everything
> >is just generated and fixed. I think I had the same discussion
> >with Laurent some weeks ago regarding this.
> 
> I would be kinda neutral here. I'd consider it helpful, it improves
> readability (regardless of whether they are generated or hand
> crafted). That's convenient for things like interrupt sensitivity, I
> can't remember whether 4 is level or edge type. On the other hand,
> the clock indices are just as much magic numbers as the memory
> addresses. If I suspect an error in that area, I'd start by lokking
> in /sys/kernel/debug/clk but wouldn't start in the DT.

I agree. Readability is better this way. And whether our generator spits
out magic numbers of these symbolic names should not be a big
difference, is it?

	Sören


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/5] ARM: zynq: dt: Use #defines for interrupt specifiers

[Soren Brinkmann]

Use #defines from the common include file to describe
interrupt specifiers.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---

Changes in v2: None

 arch/arm/boot/dts/zynq-7000.dtsi | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 66613d04de5d..36a34ffa30af 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -12,6 +12,7 @@
  */
 
 #include "skeleton.dtsi"
+#include <dt-bindings/interrupt-controller/arm-gic.h>
 
 / {
 	compatible = "xlnx,zynq-7000";
@@ -44,7 +45,7 @@
 
 	pmu {
 		compatible = "arm,cortex-a9-pmu";
-		interrupts = <0 5 4>, <0 6 4>;
+		interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
 		interrupt-parent = <&intc>;
 		reg = < 0xf8891000 0x1000 0xf8893000 0x1000 >;
 	};
@@ -80,7 +81,7 @@
 			clocks = <&clkc 23>, <&clkc 40>;
 			clock-names = "ref_clk", "aper_clk";
 			reg = <0xE0000000 0x1000>;
-			interrupts = <0 27 4>;
+			interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
 		uart1: uart@e0001000 {
@@ -89,14 +90,14 @@
 			clocks = <&clkc 24>, <&clkc 41>;
 			clock-names = "ref_clk", "aper_clk";
 			reg = <0xE0001000 0x1000>;
-			interrupts = <0 50 4>;
+			interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
 		gem0: ethernet@e000b000 {
 			compatible = "cdns,gem";
 			reg = <0xe000b000 0x4000>;
 			status = "disabled";
-			interrupts = <0 22 4>;
+			interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clkc 30>, <&clkc 30>, <&clkc 13>;
 			clock-names = "pclk", "hclk", "tx_clk";
 		};
@@ -105,7 +106,7 @@
 			compatible = "cdns,gem";
 			reg = <0xe000c000 0x4000>;
 			status = "disabled";
-			interrupts = <0 45 4>;
+			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clkc 31>, <&clkc 31>, <&clkc 14>;
 			clock-names = "pclk", "hclk", "tx_clk";
 		};
@@ -116,7 +117,7 @@
 			clock-names = "clk_xin", "clk_ahb";
 			clocks = <&clkc 21>, <&clkc 32>;
 			interrupt-parent = <&intc>;
-			interrupts = <0 24 4>;
+			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xe0100000 0x1000>;
 		} ;
 
@@ -126,7 +127,7 @@
 			clock-names = "clk_xin", "clk_ahb";
 			clocks = <&clkc 22>, <&clkc 33>;
 			interrupt-parent = <&intc>;
-			interrupts = <0 47 4>;
+			interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xe0101000 0x1000>;
 		} ;
 
@@ -160,14 +161,16 @@
 		global_timer: timer@f8f00200 {
 			compatible = "arm,cortex-a9-global-timer";
 			reg = <0xf8f00200 0x20>;
-			interrupts = <1 11 0x301>;
+			interrupts = <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_EDGE_RISING)>;
 			interrupt-parent = <&intc>;
 			clocks = <&clkc 4>;
 		};
 
 		ttc0: ttc0@f8001000 {
 			interrupt-parent = <&intc>;
-			interrupts = < 0 10 4 0 11 4 0 12 4 >;
+			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
 			compatible = "cdns,ttc";
 			clocks = <&clkc 6>;
 			reg = <0xF8001000 0x1000>;
@@ -175,14 +178,16 @@
 
 		ttc1: ttc1@f8002000 {
 			interrupt-parent = <&intc>;
-			interrupts = < 0 37 4 0 38 4 0 39 4 >;
+			interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
 			compatible = "cdns,ttc";
 			clocks = <&clkc 6>;
 			reg = <0xF8002000 0x1000>;
 		};
 		scutimer: scutimer@f8f00600 {
 			interrupt-parent = <&intc>;
-			interrupts = < 1 13 0x301 >;
+			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_EDGE_RISING)>;
 			compatible = "arm,cortex-a9-twd-timer";
 			reg = < 0xf8f00600 0x20 >;
 			clocks = <&clkc 4>;
-- 
1.9.1.1.gbb9f595

[PATCH v2 4/5] dt-bindings: clock: Add Zynq-7000 header

[Soren Brinkmann]

Add header file with symbolic names for Zynq's clocks.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---

Changes in v2:
 - this patch has been added

---
 include/dt-bindings/clock/zynq-7000.h | 64 +++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 include/dt-bindings/clock/zynq-7000.h

diff --git a/include/dt-bindings/clock/zynq-7000.h b/include/dt-bindings/clock/zynq-7000.h
new file mode 100644
index 000000000000..851f5cffe481
--- /dev/null
+++ b/include/dt-bindings/clock/zynq-7000.h
@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2014 Xilinx Inc.
+ * Author: Sören Brinkmann <soren.brinkmann@xilinx.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Device Tree binding constants for Zynq 7000 clock controller.
+*/
+
+#ifndef _DT_BINDINGS_CLOCK_ZYNQ_7000_H
+#define _DT_BINDINGS_CLOCK_ZYNQ_7000_H
+
+#define ZYNQ_CLK_ARMPLL		0
+#define ZYNQ_CLK_DDRPLL		1
+#define ZYNQ_CLK_IOPLL		2
+#define ZYNQ_CLK_CPU_6OR4X	3
+#define ZYNQ_CLK_CPU_3OR2X	4
+#define ZYNQ_CLK_CPU_2X		5
+#define ZYNQ_CLK_CPU_1X		6
+#define ZYNQ_CLK_DDR2X		7
+#define ZYNQ_CLK_DDR3X		8
+#define ZYNQ_CLK_DCI		9
+#define ZYNQ_CLK_LQSPI		10
+#define ZYNQ_CLK_SMC		11
+#define ZYNQ_CLK_PCAP		12
+#define ZYNQ_CLK_GEM0		13
+#define ZYNQ_CLK_GEM1		14
+#define ZYNQ_CLK_FCLK0		15
+#define ZYNQ_CLK_FCLK1		16
+#define ZYNQ_CLK_FCLK2		17
+#define ZYNQ_CLK_FCLK3		18
+#define ZYNQ_CLK_CAN0		19
+#define ZYNQ_CLK_CAN1		20
+#define ZYNQ_CLK_SDIO0		21
+#define ZYNQ_CLK_SDIO1		22
+#define ZYNQ_CLK_UART0		23
+#define ZYNQ_CLK_UART1		24
+#define ZYNQ_CLK_SPI0		25
+#define ZYNQ_CLK_SPI1		26
+#define ZYNQ_CLK_DMA		27
+#define ZYNQ_CLK_USB0_APER	28
+#define ZYNQ_CLK_USB1_APER	29
+#define ZYNQ_CLK_GEM0_APER	30
+#define ZYNQ_CLK_GEM1_APER	31
+#define ZYNQ_CLK_SDIO0_APER	32
+#define ZYNQ_CLK_SDIO1_APER	33
+#define ZYNQ_CLK_SPI0_APER	34
+#define ZYNQ_CLK_SPI1_APER	35
+#define ZYNQ_CLK_CAN0_APER	36
+#define ZYNQ_CLK_CAN1_APER	37
+#define ZYNQ_CLK_I2C0_APER	38
+#define ZYNQ_CLK_I2C1_APER	39
+#define ZYNQ_CLK_UART0_APER	40
+#define ZYNQ_CLK_UART1_APER	41
+#define ZYNQ_CLK_GPIO_APER	42
+#define ZYNQ_CLK_LQSPI_APER	43
+#define ZYNQ_CLK_SMC_APER	44
+#define ZYNQ_CLK_SWDT		45
+#define ZYNQ_CLK_DBG_TRC	46
+#define ZYNQ_CLK_DBG_APB	47
+
+#endif /* _DT_BINDINGS_CLOCK_ZYNQ_7000_H */
-- 
1.9.1.1.gbb9f595

[PATCH v2 5/5] ARM: zynq: dt: Use #defines for clock specifiers

[Soren Brinkmann]

Use symbolic names instead of bare numbers to specify clocks.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---

Changes in v2:
 - this patch has been added

---
 arch/arm/boot/dts/zynq-7000.dtsi | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 36a34ffa30af..9a54b49d0fd2 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -13,6 +13,7 @@
 
 #include "skeleton.dtsi"
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/zynq-7000.h>
 
 / {
 	compatible = "xlnx,zynq-7000";
@@ -25,8 +26,8 @@
 			compatible = "arm,cortex-a9";
 			device_type = "cpu";
 			reg = <0>;
-			clocks = <&clkc 3>;
 			clock-latency = <1000>;
+			clocks = <&clkc ZYNQ_CLK_CPU_6OR4X>;
 			operating-points = <
 				/* kHz    uV */
 				666667  1000000
@@ -39,7 +40,7 @@
 			compatible = "arm,cortex-a9";
 			device_type = "cpu";
 			reg = <1>;
-			clocks = <&clkc 3>;
+			clocks = <&clkc ZYNQ_CLK_CPU_6OR4X>;
 		};
 	};
 
@@ -78,7 +79,8 @@
 		uart0: uart@e0000000 {
 			compatible = "xlnx,xuartps";
 			status = "disabled";
-			clocks = <&clkc 23>, <&clkc 40>;
+			clocks = <&clkc ZYNQ_CLK_UART0>,
+				 <&clkc ZYNQ_CLK_UART0_APER>;
 			clock-names = "ref_clk", "aper_clk";
 			reg = <0xE0000000 0x1000>;
 			interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
@@ -87,7 +89,8 @@
 		uart1: uart@e0001000 {
 			compatible = "xlnx,xuartps";
 			status = "disabled";
-			clocks = <&clkc 24>, <&clkc 41>;
+			clocks = <&clkc ZYNQ_CLK_UART1>,
+				 <&clkc ZYNQ_CLK_UART1_APER>;
 			clock-names = "ref_clk", "aper_clk";
 			reg = <0xE0001000 0x1000>;
 			interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
@@ -98,7 +101,9 @@
 			reg = <0xe000b000 0x4000>;
 			status = "disabled";
 			interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clkc 30>, <&clkc 30>, <&clkc 13>;
+			clocks = <&clkc ZYNQ_CLK_GEM0_APER>,
+				 <&clkc ZYNQ_CLK_GEM0_APER>,
+				 <&clkc ZYNQ_CLK_GEM0>;
 			clock-names = "pclk", "hclk", "tx_clk";
 		};
 
@@ -107,7 +112,9 @@
 			reg = <0xe000c000 0x4000>;
 			status = "disabled";
 			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clkc 31>, <&clkc 31>, <&clkc 14>;
+			clocks = <&clkc ZYNQ_CLK_GEM1_APER>,
+				 <&clkc ZYNQ_CLK_GEM1_APER>,
+				 <&clkc ZYNQ_CLK_GEM1>;
 			clock-names = "pclk", "hclk", "tx_clk";
 		};
 
@@ -115,7 +122,8 @@
 			compatible = "arasan,sdhci-8.9a";
 			status = "disabled";
 			clock-names = "clk_xin", "clk_ahb";
-			clocks = <&clkc 21>, <&clkc 32>;
+			clocks = <&clkc ZYNQ_CLK_SDIO0>,
+				 <&clkc ZYNQ_CLK_SDIO0_APER>;
 			interrupt-parent = <&intc>;
 			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xe0100000 0x1000>;
@@ -125,7 +133,8 @@
 			compatible = "arasan,sdhci-8.9a";
 			status = "disabled";
 			clock-names = "clk_xin", "clk_ahb";
-			clocks = <&clkc 22>, <&clkc 33>;
+			clocks = <&clkc ZYNQ_CLK_SDIO1>,
+				 <&clkc ZYNQ_CLK_SDIO1_APER>;
 			interrupt-parent = <&intc>;
 			interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xe0101000 0x1000>;
@@ -163,7 +172,7 @@
 			reg = <0xf8f00200 0x20>;
 			interrupts = <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_EDGE_RISING)>;
 			interrupt-parent = <&intc>;
-			clocks = <&clkc 4>;
+			clocks = <&clkc ZYNQ_CLK_CPU_3OR2X>;
 		};
 
 		ttc0: ttc0@f8001000 {
@@ -172,7 +181,7 @@
 				     <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
 			compatible = "cdns,ttc";
-			clocks = <&clkc 6>;
+			clocks = <&clkc ZYNQ_CLK_CPU_1X>;
 			reg = <0xF8001000 0x1000>;
 		};
 
@@ -182,7 +191,7 @@
 				     <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
 			compatible = "cdns,ttc";
-			clocks = <&clkc 6>;
+			clocks = <&clkc ZYNQ_CLK_CPU_1X>;
 			reg = <0xF8002000 0x1000>;
 		};
 		scutimer: scutimer@f8f00600 {
@@ -190,7 +199,7 @@
 			interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_EDGE_RISING)>;
 			compatible = "arm,cortex-a9-twd-timer";
 			reg = < 0xf8f00600 0x20 >;
-			clocks = <&clkc 4>;
+			clocks = <&clkc ZYNQ_CLK_CPU_3OR2X>;
 		} ;
 	};
 };
-- 
1.9.1.1.gbb9f595