[PATCH v4 0/5] zynq subarch cleanups

[PATCH v4 0/5] zynq subarch cleanups

[Josh Cartwright]

Hey all-

I think everything is in a good state to merge now, from here its a
discussion of process.

Thanks to everyone involved.

---
Things have been relatively quiet on the Zynq front lately.  This patchset does
a bit of cleanup of the Zynq subarchitecture.  It was the necessary set of
things I had to do to get a zynq target booting with the upstream qemu model.

Patches 1 and 2 move zynq to use the GIC and pl310 L2 cache controller device
tree bindings respectively.

Patch 3 removes unused clock infrastructure.  The plan is to rework the
out-of-tree Xilinx generic clk support into something suitable for merging.
What's in tree now just isn't used at all, and can be removed.

Patch 4 and 5 move around the static peripheral mappings into the vmalloc area.

---
Changes since v3:
  - Patch 3 also removes the zynq "use" of versatile

Changes since v2:
  - Reordered patchset to prevent remapping peripherals that were subsequently
    removed from the static map
  - Use DT bindings for the L2 cache controller

Changes since v1:
  - Make sure arm@kernel.org was included
  - Rebased on arm-soc/for-next
  - Added a cover letter
  - Elaborated a bit on why I removed CLKDEV_LOOKUP

---
Josh Cartwright (5):
  zynq: use GIC device tree bindings
  zynq: use pl310 device tree bindings
  zynq: remove use of CLKDEV_LOOKUP
  ARM: annotate VMALLOC_END definition with _AC
  zynq: move static peripheral mappings

 arch/arm/Kconfig                           |  1 -
 arch/arm/Makefile                          |  1 -
 arch/arm/boot/dts/zynq-ep107.dts           | 17 +++++++++++++---
 arch/arm/include/asm/pgtable.h             |  2 +-
 arch/arm/mach-zynq/common.c                | 23 ++++++++++-----------
 arch/arm/mach-zynq/include/mach/clkdev.h   | 32 ------------------------------
 arch/arm/mach-zynq/include/mach/zynq_soc.h | 29 ++++++++++++---------------
 7 files changed, 38 insertions(+), 67 deletions(-)
 delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h

-- 
1.8.0

[PATCH v4 1/5] zynq: use GIC device tree bindings

[Josh Cartwright]

The Zynq uses the cortex-a9-gic.  This eliminates the need to hardcode
register addresses.

Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
Cc: John Linn <john.linn@xilinx.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/boot/dts/zynq-ep107.dts           | 8 +++++---
 arch/arm/mach-zynq/common.c                | 7 ++++++-
 arch/arm/mach-zynq/include/mach/zynq_soc.h | 2 --
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq-ep107.dts
index 37ca192..7bfff4a 100644
--- a/arch/arm/boot/dts/zynq-ep107.dts
+++ b/arch/arm/boot/dts/zynq-ep107.dts
@@ -36,10 +36,12 @@
 		ranges;
 
 		intc: interrupt-controller@f8f01000 {
+			compatible = "arm,cortex-a9-gic";
+			#interrupt-cells = <3>;
+			#address-cells = <1>;
 			interrupt-controller;
-			compatible = "arm,gic";
-			reg = <0xF8F01000 0x1000>;
-			#interrupt-cells = <2>;
+			reg = <0xF8F01000 0x1000>,
+			      <0xF8F00100 0x100>;
 		};
 
 		uart0: uart@e0000000 {
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index ab5cfdd..d73963b 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -55,12 +55,17 @@ static void __init xilinx_init_machine(void)
 	of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
 }
 
+static struct of_device_id irq_match[] __initdata = {
+	{ .compatible = "arm,cortex-a9-gic", .data = gic_of_init, },
+	{ }
+};
+
 /**
  * xilinx_irq_init() - Interrupt controller initialization for the GIC.
  */
 static void __init xilinx_irq_init(void)
 {
-	gic_init(0, 29, SCU_GIC_DIST_BASE, SCU_GIC_CPU_BASE);
+	of_irq_init(irq_match);
 }
 
 /* The minimum devices needed to be mapped before the VM system is up and
diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
index d0d3f8f..3d1c6a6 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -35,8 +35,6 @@
 
 #define TTC0_BASE			IOMEM(TTC0_VIRT)
 #define SCU_PERIPH_BASE			IOMEM(SCU_PERIPH_VIRT)
-#define SCU_GIC_CPU_BASE		(SCU_PERIPH_BASE + 0x100)
-#define SCU_GIC_DIST_BASE		(SCU_PERIPH_BASE + 0x1000)
 #define PL310_L2CC_BASE			IOMEM(PL310_L2CC_VIRT)
 
 /*
-- 
1.8.0

RE: [PATCH v4 1/5] zynq: use GIC device tree bindings

[Michal Simek]

Hi Josh,

> -----Original Message-----
> From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> Sent: Wednesday, October 24, 2012 10:03 PM
> To: arm@kernel.org; Arnd Bergmann
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; John
> Linn; Nick Bowler; Michal Simek
> Subject: [PATCH v4 1/5] zynq: use GIC device tree bindings
>
> The Zynq uses the cortex-a9-gic.  This eliminates the need to hardcode register
> addresses.
>
> Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
> Cc: John Linn <john.linn@xilinx.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/boot/dts/zynq-ep107.dts           | 8 +++++---
>  arch/arm/mach-zynq/common.c                | 7 ++++++-
>  arch/arm/mach-zynq/include/mach/zynq_soc.h | 2 --
>  3 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq-
> ep107.dts
> index 37ca192..7bfff4a 100644
> --- a/arch/arm/boot/dts/zynq-ep107.dts
> +++ b/arch/arm/boot/dts/zynq-ep107.dts
> @@ -36,10 +36,12 @@
>               ranges;
>
>               intc: interrupt-controller@f8f01000 {
> +                     compatible = "arm,cortex-a9-gic";
> +                     #interrupt-cells = <3>;

If you change git to 3 cells format you should also change it for uart below.

Also will be the best to remove this dts entirely and replace it by existing
Platform.

Thanks,
Michal


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: [PATCH v4 1/5] zynq: use GIC device tree bindings

[Josh Cartwright]

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

On Sat, Oct 27, 2012 at 01:39:00PM +0000, Michal Simek wrote:
> Hi Josh,
> 
> > -----Original Message-----
> > From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> > Sent: Wednesday, October 24, 2012 10:03 PM
> > To: arm@kernel.org; Arnd Bergmann
> > Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; John
> > Linn; Nick Bowler; Michal Simek
> > Subject: [PATCH v4 1/5] zynq: use GIC device tree bindings
> >
> > The Zynq uses the cortex-a9-gic.  This eliminates the need to hardcode register
> > addresses.
> >
> > Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
> > Cc: John Linn <john.linn@xilinx.com>
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  arch/arm/boot/dts/zynq-ep107.dts           | 8 +++++---
> >  arch/arm/mach-zynq/common.c                | 7 ++++++-
> >  arch/arm/mach-zynq/include/mach/zynq_soc.h | 2 --
> >  3 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq-
> > ep107.dts
> > index 37ca192..7bfff4a 100644
> > --- a/arch/arm/boot/dts/zynq-ep107.dts
> > +++ b/arch/arm/boot/dts/zynq-ep107.dts
> > @@ -36,10 +36,12 @@
> >               ranges;
> >
> >               intc: interrupt-controller@f8f01000 {
> > +                     compatible = "arm,cortex-a9-gic";
> > +                     #interrupt-cells = <3>;
> 
> If you change git to 3 cells format you should also change it for uart below.
>
> Also will be the best to remove this dts entirely and replace it by existing
> Platform.

Do you mean to say get rid of the ep107 entirely and replace it with,
for example, a zc702 dts?

I have a follow up patchset which splits off a zynq-7000.dtsi and a
zynq-zc702.dts, and which also fixes the 3 interrupt cell problem.
Would you like for me to pull this into v5, or spin up a separate patch
series?

Thanks,

  Josh

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH v4 1/5] zynq: use GIC device tree bindings

[Michal Simek]

Hi Josh

> -----Original Message-----
> From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> Sent: Saturday, October 27, 2012 4:01 PM
> To: Michal Simek
> Cc: arm@kernel.org; Arnd Bergmann; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; John Linn; Nick Bowler
> Subject: Re: [PATCH v4 1/5] zynq: use GIC device tree bindings
> 
> On Sat, Oct 27, 2012 at 01:39:00PM +0000, Michal Simek wrote:
> > Hi Josh,
> >
> > > -----Original Message-----
> > > From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> > > Sent: Wednesday, October 24, 2012 10:03 PM
> > > To: arm@kernel.org; Arnd Bergmann
> > > Cc: linux-kernel@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; John Linn; Nick Bowler; Michal
> > > Simek
> > > Subject: [PATCH v4 1/5] zynq: use GIC device tree bindings
> > >
> > > The Zynq uses the cortex-a9-gic.  This eliminates the need to
> > > hardcode register addresses.
> > >
> > > Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
> > > Cc: John Linn <john.linn@xilinx.com>
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > >  arch/arm/boot/dts/zynq-ep107.dts           | 8 +++++---
> > >  arch/arm/mach-zynq/common.c                | 7 ++++++-
> > >  arch/arm/mach-zynq/include/mach/zynq_soc.h | 2 --
> > >  3 files changed, 11 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/zynq-ep107.dts
> > > b/arch/arm/boot/dts/zynq- ep107.dts index 37ca192..7bfff4a 100644
> > > --- a/arch/arm/boot/dts/zynq-ep107.dts
> > > +++ b/arch/arm/boot/dts/zynq-ep107.dts
> > > @@ -36,10 +36,12 @@
> > >               ranges;
> > >
> > >               intc: interrupt-controller@f8f01000 {
> > > +                     compatible = "arm,cortex-a9-gic";
> > > +                     #interrupt-cells = <3>;
> >
> > If you change git to 3 cells format you should also change it for uart below.
> >
> > Also will be the best to remove this dts entirely and replace it by
> > existing Platform.
> 
> Do you mean to say get rid of the ep107 entirely and replace it with, for
> example, a zc702 dts?

Yes, Ep107 platform is nothing what you can buy, that's why it is not the platform
Which should be supported in mainline. Supporting zc702 make sense.

> 
> I have a follow up patchset which splits off a zynq-7000.dtsi and a zynq-
> zc702.dts, and which also fixes the 3 interrupt cell problem.

I am not big fan to use dtsi solution because dts can be simple generated directly
>From Xilinx design tool based on your hw design. That's why I can't see any benefit
To have dtsi file. 

> Would you like for me to pull this into v5, or spin up a separate patch series?

Definitely not. I have checked patches but haven't got it work on the zc702.
Not sure if you have run it on real hw or just on the qemu as you have mentioned
In 5/5.

Thanks,
Michal

Re: [PATCH v4 1/5] zynq: use GIC device tree bindings

[Josh Cartwright]

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

On Sat, Oct 27, 2012 at 02:06:45PM +0000, Michal Simek wrote:
[...]
> I am not big fan to use dtsi solution because dts can be simple generated directly
> From Xilinx design tool based on your hw design. That's why I can't see any benefit
> To have dtsi file.

Can I ask you to reconsider?  We, for example, don't make any use of the
Xilinx dev tools to generate our device trees.  Having a dtsi allows for
easy extension of the zynq-7000 platform for our boards, without having
to carry duplicate data.

Is it going to be expected that users building kernels for their zynq
targets have access to the Xilinx EDK?

> > Would you like for me to pull this into v5, or spin up a separate patch series?
>
> Definitely not. I have checked patches but haven't got it work on the zc702.
> Not sure if you have run it on real hw or just on the qemu as you have mentioned
> In 5/5.

You're likely running into the issue Nick has identified in the thread
for patch 5 where the chosen virtual address for the uart doesn't seem
to work: http://www.spinics.net/lists/arm-kernel/msg203141.html

We haven't yet identified the root cause; any insight you can provide
here would be beneficial.

Otherwise, I'm considering reworking patch 5 to move the uart mapping to
a known working location.

Thanks,

   Josh

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH v4 1/5] zynq: use GIC device tree bindings

[Michal Simek]

> -----Original Message-----
> From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> Sent: Saturday, October 27, 2012 4:43 PM
> To: Michal Simek
> Cc: arm@kernel.org; Arnd Bergmann; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; John Linn; Nick Bowler
> Subject: Re: [PATCH v4 1/5] zynq: use GIC device tree bindings
> 
> On Sat, Oct 27, 2012 at 02:06:45PM +0000, Michal Simek wrote:
> [...]
> > I am not big fan to use dtsi solution because dts can be simple
> > generated directly From Xilinx design tool based on your hw design.
> > That's why I can't see any benefit To have dtsi file.
> 
> Can I ask you to reconsider? 

I am open to all solution which will help others. I am not definitely saying NO   for this features
I just haven't found a reason to support it.  

> We, for example, don't make any use of the Xilinx
> dev tools to generate our device trees.

Ok. How does your working flow looks like?
I mean you don't get any information from hardware guys how does your hw design look like?

>  Having a dtsi allows for easy extension
> of the zynq-7000 platform for our boards, without having to carry duplicate data.

ok. I think that make sense if you send the next your series as RFC to see how exactly
you would like to use it. 

In my workflow we generate DTS directly from design tool which I expect your hw
guys use because it is probably needed to generate boot.bin/fsbl/etc. 
Then there is one more additional step to setup device-tree bsp to generate DTS
which directly fits to your HW design. 
If you have the same boards with different programmable logic I understand
that you would like to share that PS part and then just add it that IPs in PL.

 
> Is it going to be expected that users building kernels for their zynq targets have
> access to the Xilinx EDK?

Definitely not. You can do it just once and of course you can write it by hand
and then just reusing. 

>From my point of view. You have to use design tools at least once to get bitstream
and boot.bin with fsbl. Please correct me if I am wrong.
In this step you can use device-tree BSP to generate DTS ( I doesn't need to be perfect with 
all attached devices on i2c,spi, phys, etc but it reflects your hardware).
You will get it in some seconds and your dts has correct irq numbers/ip description, compatible strings,
addresses, position in the system - if you use bus bridges, etc) and you can just directly use it
and kernel will boot. If you need to do changes in dts by hand, you can of course do it. 

> > > Would you like for me to pull this into v5, or spin up a separate patch series?
> >
> > Definitely not. I have checked patches but haven't got it work on the zc702.
> > Not sure if you have run it on real hw or just on the qemu as you have
> > mentioned In 5/5.
> 
> You're likely running into the issue Nick has identified in the thread for patch 5
> where the chosen virtual address for the uart doesn't seem to work:
> http://www.spinics.net/lists/arm-kernel/msg203141.html
> 
> We haven't yet identified the root cause; any insight you can provide here
> would be beneficial.
> 
> Otherwise, I'm considering reworking patch 5 to move the uart mapping to a
> known working location.

I just need to get some time to catch you.

thanks,
Michal

Re: [PATCH v4 1/5] zynq: use GIC device tree bindings

[Josh Cartwright]

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

On Sat, Oct 27, 2012 at 03:20:59PM +0000, Michal Simek wrote:
> On Saturday, October 27, 2012 4:43 PM, Josh Cartwright wrote:
> > On Sat, Oct 27, 2012 at 02:06:45PM +0000, Michal Simek wrote:
> > [...]
> > > I am not big fan to use dtsi solution because dts can be simple
> > > generated directly From Xilinx design tool based on your hw design.
> > > That's why I can't see any benefit To have dtsi file.
> >
> > Can I ask you to reconsider?
>
> I am open to all solution which will help others. I am not definitely
> saying NO for this features I just haven't found a reason to support
> it.
>
> > We, for example, don't make any use of the Xilinx
> > dev tools to generate our device trees.
>
> Ok. How does your working flow looks like?  I mean you don't get any
> information from hardware guys how does your hw design look like?

Our usecase may admittedly be a bit weird, because what logic is in the
PL is ultimately determined (and even implemented) by the end user and
is loaded at runtime.  There is a lot of machinery to make that happen,
but the point is that I don't have sufficient knowledge upfront to
generate appropriate bindings for what's in the PL.

> > Having a dtsi allows for easy extension of the zynq-7000 platform
> > for our boards, without having to carry duplicate data.
>
> ok. I think that make sense if you send the next your series as RFC to
> see how exactly you would like to use it.

It seems like you caught a glimpse of this in my COMMON_CLK patchset. :)

> In my workflow we generate DTS directly from design tool which I
> expect your hw guys use because it is probably needed to generate
> boot.bin/fsbl/etc.  Then there is one more additional step to setup
> device-tree bsp to generate DTS which directly fits to your HW design.
> If you have the same boards with different programmable logic I
> understand that you would like to share that PS part and then just add
> it that IPs in PL.
[..]
> From my point of view. You have to use design tools at least once to
> get bitstream and boot.bin with fsbl. Please correct me if I am wrong.
> In this step you can use device-tree BSP to generate DTS ( I doesn't
> need to be perfect with all attached devices on i2c,spi, phys, etc but
> it reflects your hardware).  You will get it in some seconds and your
> dts has correct irq numbers/ip description, compatible strings,
> addresses, position in the system - if you use bus bridges, etc) and
> you can just directly use it and kernel will boot. If you need to do
> changes in dts by hand, you can of course do it.

I wouldn't be as opposed to device tree generation if the device tree
generator was in tree.  Device tree bindings change, how would/could an
out-of-tree generator possibly handle changes in bindings?  Would a user
target the generator at a specific version of the kernel?  (An in tree
generator would also seem to necessitate a more formal specification
language for DT bindings).

It is odd to me that the use of a generator would be required to create
what is completely static data.  What I'm referring to here is the
collection of peripherals on the zynq-7000 that are not in the PL.  For
me, this requirement adds an unnecessary dependency on the Xilinx EDK
that I would like to avoid.

Would it make sense to limit what the device tree generator produces to
only what is in the PL?  (Forgive my ignorance about this tool, I've
never used it.)

This could just be an extension of what I've started to do with the
COMMON_CLK patchset.  A zynq board would be described using several
device tree snippets, one for the baked-in zynq-7000 peripheral set, one
for a generated description of what is in the PL, and one describing any
board-specific details (phy, etc).  Something like below:

zynq-7000.dtsi  : description of static zynq-7000 peripherals

	/ {
		amba : amba@0 {
			slcr: slcr@f8000000 {
				clocks {
					ps_clk : ps_clk {
						compatible = "fixed-clock";
					};
					...
				}
			};
			i2c0 : i2c0@e0004000 {
				compatible = "xlnx,i2cps";
				...
			};
			eth0 : eth0@e000b000 {
				compatible = "xlnx,emacps";
				...
			};
		};
	};

zynq-zc702-pl.dtsi : generated description of what is in the PL

	&amba {
		/* PL IP generated descriptions here. */
	};

zynq-zc702.dts  : board-specific descriptions (osc freq, i2c, spi, phys, etc)

	/include/ "zynq-7000.dtsi"
	/include/ "zynq-zc702-pl.dtsi"
	&ps_clk {
		clock-frequency = <33333330>;
	};
	&i2c0 {
		pca9548@74 {
			...
			reg = <0x74>;
			...
		};
	};
	&eth0 {
		phy@7 {
			compatible = "marvell,888e1116r";
			...
			reg = <0x7>;
		};
	};

Thoughts?

Thanks,
  Josh

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v4 1/5] zynq: use GIC device tree bindings

[Michal Simek]

2012/11/5 Josh Cartwright <josh.cartwright@ni.com>:
> On Sat, Oct 27, 2012 at 03:20:59PM +0000, Michal Simek wrote:
>> On Saturday, October 27, 2012 4:43 PM, Josh Cartwright wrote:
>> > On Sat, Oct 27, 2012 at 02:06:45PM +0000, Michal Simek wrote:
>> > [...]
>> > > I am not big fan to use dtsi solution because dts can be simple
>> > > generated directly From Xilinx design tool based on your hw design.
>> > > That's why I can't see any benefit To have dtsi file.
>> >
>> > Can I ask you to reconsider?
>>
>> I am open to all solution which will help others. I am not definitely
>> saying NO for this features I just haven't found a reason to support
>> it.
>>
>> > We, for example, don't make any use of the Xilinx
>> > dev tools to generate our device trees.
>>
>> Ok. How does your working flow looks like?  I mean you don't get any
>> information from hardware guys how does your hw design look like?
>
> Our usecase may admittedly be a bit weird, because what logic is in the
> PL is ultimately determined (and even implemented) by the end user and
> is loaded at runtime.  There is a lot of machinery to make that happen,
> but the point is that I don't have sufficient knowledge upfront to
> generate appropriate bindings for what's in the PL.

ok. It means that you need to use just the part of DTS without PL logic at all.
Does it mean that PL will be connected with any DTS fragment?



>> > Having a dtsi allows for easy extension of the zynq-7000 platform
>> > for our boards, without having to carry duplicate data.
>>
>> ok. I think that make sense if you send the next your series as RFC to
>> see how exactly you would like to use it.
>
> It seems like you caught a glimpse of this in my COMMON_CLK patchset. :)

Yes. Just need to get some time to analyze it.


>> In my workflow we generate DTS directly from design tool which I
>> expect your hw guys use because it is probably needed to generate
>> boot.bin/fsbl/etc.  Then there is one more additional step to setup
>> device-tree bsp to generate DTS which directly fits to your HW design.
>> If you have the same boards with different programmable logic I
>> understand that you would like to share that PS part and then just add
>> it that IPs in PL.
> [..]
>> From my point of view. You have to use design tools at least once to
>> get bitstream and boot.bin with fsbl. Please correct me if I am wrong.
>> In this step you can use device-tree BSP to generate DTS ( I doesn't
>> need to be perfect with all attached devices on i2c,spi, phys, etc but
>> it reflects your hardware).  You will get it in some seconds and your
>> dts has correct irq numbers/ip description, compatible strings,
>> addresses, position in the system - if you use bus bridges, etc) and
>> you can just directly use it and kernel will boot. If you need to do
>> changes in dts by hand, you can of course do it.
>
> I wouldn't be as opposed to device tree generation if the device tree
> generator was in tree.

Which tree do you exactly mean? Linux kernel or just any git tree?

Let me give you more information about the generator. It uses TCL in SDK
where it provides all structure from the system. It means device-tree generator
will read all information from design tool and based on that will generate
DTS file. It also means if user will setup specific irq lines in design, special
paramters setting in registers then all these values will be added to DTS.
.

> Device tree bindings change, how would/could an
> out-of-tree generator possibly handle changes in bindings?

What do you mean by that? Any example?
Generator will generate DTS on the current configuration which you have setup
in EDK.
For your case when you generate PL part at runtime you probably will use
just the part of DTS (that hard IPs)


>  Would a user
> target the generator at a specific version of the kernel?  (An in tree
> generator would also seem to necessitate a more formal specification
> language for DT bindings).

Device tree description in the hardware description which is not changing
across kernel versions. Xilinx device tree generator generates DTS
which reflects all standards.
We can send the generated output to device-tree mainling list and ask
for review.

For clk common patches this features is not implemented yet but it is easy
to add it and all generated DTSes will contains it.
If you think to label generator with specific version then we can use
any tag in the device-tree BSP tree.


> It is odd to me that the use of a generator would be required to create
> what is completely static data.  What I'm referring to here is the
> collection of peripherals on the zynq-7000 that are not in the PL.  For
> me, this requirement adds an unnecessary dependency on the Xilinx EDK
> that I would like to avoid.

I am not saying that you need to use it. If you want to write your DTS
by hand, you still can
but I expect that the most of zynq users will use generator and
generate it because
it is just easier than to describe it by hand and they can be sure
that all parameters are
correctly generated.
If you are using any non-standard solution where you will load pl
logic at runtime
then you can use just generated DTS for hardblock or write it by hand.


> Would it make sense to limit what the device tree generator produces to
> only what is in the PL?  (Forgive my ignorance about this tool, I've
> never used it.)

It is tcl script everything is possible.
Are you interesting to use it just for PL?

> This could just be an extension of what I've started to do with the
> COMMON_CLK patchset.  A zynq board would be described using several
> device tree snippets, one for the baked-in zynq-7000 peripheral set, one
> for a generated description of what is in the PL, and one describing any
> board-specific details (phy, etc).  Something like below:
>
> zynq-7000.dtsi  : description of static zynq-7000 peripherals
>
>         / {
>                 amba : amba@0 {
>                         slcr: slcr@f8000000 {
>                                 clocks {
>                                         ps_clk : ps_clk {
>                                                 compatible = "fixed-clock";
>                                         };
>                                         ...
>                                 }
>                         };
>                         i2c0 : i2c0@e0004000 {
>                                 compatible = "xlnx,i2cps";
>                                 ...
>                         };
>                         eth0 : eth0@e000b000 {
>                                 compatible = "xlnx,emacps";
>                                 ...
>                         };
>                 };
>         };
>
> zynq-zc702-pl.dtsi : generated description of what is in the PL
>
>         &amba {
>                 /* PL IP generated descriptions here. */
>         };
>
> zynq-zc702.dts  : board-specific descriptions (osc freq, i2c, spi, phys, etc)
>
>         /include/ "zynq-7000.dtsi"
>         /include/ "zynq-zc702-pl.dtsi"
>         &ps_clk {
>                 clock-frequency = <33333330>;
>         };
>         &i2c0 {
>                 pca9548@74 {
>                         ...
>                         reg = <0x74>;
>                         ...
>                 };
>         };
>         &eth0 {
>                 phy@7 {
>                         compatible = "marvell,888e1116r";
>                         ...
>                         reg = <0x7>;
>                 };
>         };
>
> Thoughts?


The problem which I have with it is that if we accept this solution then
we will have to use 3(4-because of skeleton.dtsi) files where 2 files will fix
the origin model.
I can't see the reason why not to use just one file which will
describe it directly.

If you want to use solution with several dtsi files and compose it as
you describe
then it is completely fine but forcing others to use this structure
and write dts by hand will be big pain for a lot of users.

Also in design tools you can setup if you use qspi,nor,nand flash
memory interface.
memory interface, baudrates, dma, ports to PL logic, connections, etc.
and from my point of view is very complicated to describe it by

There are a lot of combination which you can have on one reference board.
You can't enable all hard IPs at one time and use all of them that's
why you shouldn't
list all of them in the kernel.

>From my point of view make sense to have one DTS file in the kernel
and one defconfig
for the most popular zynq board where will be exactly written that this DTS is
connected to this reference hw design. If you want to get more reference design
go to this page and download it.
Adding all DTSes for zynq boards to the kernel is overkill.
If you want to use your hw design you can use this generator and
generate it or write it by
hand.

Thanks,
Michal

Re: [PATCH v4 1/5] zynq: use GIC device tree bindings

[Josh Cartwright]

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

On Wed, Nov 07, 2012 at 01:05:57PM +0100, Michal Simek wrote:
> 2012/11/5 Josh Cartwright <josh.cartwright@ni.com>:
[..]
> > Our usecase may admittedly be a bit weird, because what logic is in the
> > PL is ultimately determined (and even implemented) by the end user and
> > is loaded at runtime.  There is a lot of machinery to make that happen,
> > but the point is that I don't have sufficient knowledge upfront to
> > generate appropriate bindings for what's in the PL.
>
> ok. It means that you need to use just the part of DTS without PL logic at all.
> Does it mean that PL will be connected with any DTS fragment?

Yes.  For the time being, this is true.  We have our own mechanisms for
enumerating IP at runtime.

> > > > Having a dtsi allows for easy extension of the zynq-7000
> > > > platform for our boards, without having to carry duplicate data.
> > >
> > > ok. I think that make sense if you send the next your series as
> > > RFC to see how exactly you would like to use it.
> >
> > It seems like you caught a glimpse of this in my COMMON_CLK
> > patchset. :)
>
> Yes. Just need to get some time to analyze it.
>
[..]
> > I wouldn't be as opposed to device tree generation if the device tree
> > generator was in tree.
>
> Which tree do you exactly mean? Linux kernel or just any git tree?

No, I mean in the upstream Linux kernel tree.  I don't think this is
likely to happen.  My point here is that the generator necessarily has a
dependency on how the bindings are written.  If those bindings change
(or new bindings are added), the generator must be updated to generate
device trees according to the new bindings.

I fail to see how these changes are handled with your generator.

> Let me give you more information about the generator. It uses TCL in SDK
> where it provides all structure from the system. It means device-tree generator
> will read all information from design tool and based on that will generate
> DTS file. It also means if user will setup specific irq lines in design, special
> paramters setting in registers then all these values will be added to DTS.
>
> > Device tree bindings change, how would/could an out-of-tree
> > generator possibly handle changes in bindings?
>
> What do you mean by that? Any example?

Yes, I have a real life example.  In 3.2 (?), GIC bindings were added to
the kernel.  It was necessary for us to update our board descriptions to
reflect the new #interrupt-cells = <3>; and figure out the appropriate
interrupt numbers (which differed from how they were specified before).

How would your generator have known whether or not I was targetting a
kernel with the GIC bindings, and appropriately generate the GIC node,
and generate interruptspecs for all children with #interrupt-cells = <3>?

Or, maybe another example: say clk bindings are added to the upstream
kernel, and I would like to use a kernel that contains them on my board.
Say this has all happened before Xilinx has even released a new version
of their SDK.  How could I use your dts generator to output proper clk
nodes in my dts?

It seems the only way that Xilinx can possibly handle this is to tightly
couple the version of the kernel and their generator.

With increasing support for Zynq in the mainline kernel tree, it may
become more palatable for some existing users to switch to using the
upstream kernel instead of the Xilinx tree for their boards, and
coupling between the generator and target kernel version will be broken.

[..]
> > It is odd to me that the use of a generator would be required to create
> > what is completely static data.  What I'm referring to here is the
> > collection of peripherals on the zynq-7000 that are not in the PL.  For
> > me, this requirement adds an unnecessary dependency on the Xilinx EDK
> > that I would like to avoid.
>
> I am not saying that you need to use it. If you want to write your DTS
> by hand, you still can but I expect that the most of zynq users will
> use generator and generate it because it is just easier than to
> describe it by hand and they can be sure that all parameters are
> correctly generated.

Again, you can only make this assurance _for a specific version of the
kernel_.  If a user is not using the version of the kernel that came
with the SDK (and, maybe instead using a vanilla upstream kernel), all
bets are off.

> If you are using any non-standard solution where you will load pl
> logic at runtime then you can use just generated DTS for hardblock or
> write it by hand.

I choose 'write it by hand'.  I want what I write by hand to also be
useful to others by including the zynq-7000.dtsi in the upstream kernel.

[..]
> If you want to use solution with several dtsi files and compose it as
> you describe then it is completely fine but forcing others to use this
> structure and write dts by hand will be big pain for a lot of users.

Using a composed model in the upstream kernel doesn't force anything
upon the existing users of your generator.  They can still use whatever
gets spit out of your generator (assuming it generates nodes with
appropriate bindings).  Unless I'm missing something here.

> Also in design tools you can setup if you use qspi,nor,nand flash
> memory interface.
> memory interface, baudrates, dma, ports to PL logic, connections, etc.
> and from my point of view is very complicated to describe it by
>
> There are a lot of combination which you can have on one reference
> board.  You can't enable all hard IPs at one time and use all of them
> that's why you shouldn't list all of them in the kernel.

I disagree with this.  In my opinion, all of the "hard IPs" should be
described in the zynq-7000.dtsi, and those nodes which aren't available
explicitly disabled in the board-specific file.

> From my point of view make sense to have one DTS file in the kernel
> and one defconfig for the most popular zynq board where will be
> exactly written that this DTS is connected to this reference hw
> design. If you want to get more reference design go to this page and
> download it.  Adding all DTSes for zynq boards to the kernel is
> overkill.  If you want to use your hw design you can use this
> generator and generate it or write it by hand.

All I'm asking for is for there to be a common zynq-7000.dtsi that
describes all of the static PS logic ("hard IPs") in the upstream kernel
source that I can include in my own (hand maintained) board
descriptions.  It would be nice if there was an example of its use, like
with a zc702 board file also upstream, but it is not really important to
me.

I do not want a dependency on the EDK.

My request does not sound unreasonable to me and is what other platforms
are doing.

  Josh

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH v4 1/5] zynq: use GIC device tree bindings

[John Linn]

> -----Original Message-----
> From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> Sent: Wednesday, November 07, 2012 6:18 AM
> To: Michal Simek
> Cc: arm@kernel.org; Arnd Bergmann; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> John Linn; Nick Bowler
> Subject: Re: [PATCH v4 1/5] zynq: use GIC device tree bindings
> 
> On Wed, Nov 07, 2012 at 01:05:57PM +0100, Michal Simek wrote:
> > 2012/11/5 Josh Cartwright <josh.cartwright@ni.com>:
> [..]
> > > Our usecase may admittedly be a bit weird, because what logic is in the
> > > PL is ultimately determined (and even implemented) by the end user and
> > > is loaded at runtime.  There is a lot of machinery to make that happen,
> > > but the point is that I don't have sufficient knowledge upfront to
> > > generate appropriate bindings for what's in the PL.
> >
> > ok. It means that you need to use just the part of DTS without PL logic at all.
> > Does it mean that PL will be connected with any DTS fragment?
> 
> Yes.  For the time being, this is true.  We have our own mechanisms for
> enumerating IP at runtime.
> 
> > > > > Having a dtsi allows for easy extension of the zynq-7000
> > > > > platform for our boards, without having to carry duplicate data.
> > > >
> > > > ok. I think that make sense if you send the next your series as
> > > > RFC to see how exactly you would like to use it.
> > >
> > > It seems like you caught a glimpse of this in my COMMON_CLK
> > > patchset. :)
> >
> > Yes. Just need to get some time to analyze it.
> >
> [..]
> > > I wouldn't be as opposed to device tree generation if the device tree
> > > generator was in tree.
> >
> > Which tree do you exactly mean? Linux kernel or just any git tree?
> 
> No, I mean in the upstream Linux kernel tree.  I don't think this is
> likely to happen.  My point here is that the generator necessarily has a
> dependency on how the bindings are written.  If those bindings change
> (or new bindings are added), the generator must be updated to generate
> device trees according to the new bindings.
> 
> I fail to see how these changes are handled with your generator.
> 
> > Let me give you more information about the generator. It uses TCL in SDK
> > where it provides all structure from the system. It means device-tree generator
> > will read all information from design tool and based on that will generate
> > DTS file. It also means if user will setup specific irq lines in design, special
> > paramters setting in registers then all these values will be added to DTS.
> >
> > > Device tree bindings change, how would/could an out-of-tree
> > > generator possibly handle changes in bindings?
> >
> > What do you mean by that? Any example?
> 
> Yes, I have a real life example.  In 3.2 (?), GIC bindings were added to
> the kernel.  It was necessary for us to update our board descriptions to
> reflect the new #interrupt-cells = <3>; and figure out the appropriate
> interrupt numbers (which differed from how they were specified before).
> 
> How would your generator have known whether or not I was targetting a
> kernel with the GIC bindings, and appropriately generate the GIC node,
> and generate interruptspecs for all children with #interrupt-cells = <3>?
> 

Hi Josh,

Yes we see this problem coming too.

> Or, maybe another example: say clk bindings are added to the upstream
> kernel, and I would like to use a kernel that contains them on my board.
> Say this has all happened before Xilinx has even released a new version
> of their SDK.  How could I use your dts generator to output proper clk
> nodes in my dts?
> 
> It seems the only way that Xilinx can possibly handle this is to tightly
> couple the version of the kernel and their generator.
> 
> With increasing support for Zynq in the mainline kernel tree, it may
> become more palatable for some existing users to switch to using the
> upstream kernel instead of the Xilinx tree for their boards, and
> coupling between the generator and target kernel version will be broken.
> 
> [..]
> > > It is odd to me that the use of a generator would be required to create
> > > what is completely static data.  What I'm referring to here is the
> > > collection of peripherals on the zynq-7000 that are not in the PL.  For
> > > me, this requirement adds an unnecessary dependency on the Xilinx EDK
> > > that I would like to avoid.
> >
> > I am not saying that you need to use it. If you want to write your DTS
> > by hand, you still can but I expect that the most of zynq users will
> > use generator and generate it because it is just easier than to
> > describe it by hand and they can be sure that all parameters are
> > correctly generated.
> 
> Again, you can only make this assurance _for a specific version of the
> kernel_.  If a user is not using the version of the kernel that came
> with the SDK (and, maybe instead using a vanilla upstream kernel), all
> bets are off.
> 
> > If you are using any non-standard solution where you will load pl
> > logic at runtime then you can use just generated DTS for hardblock or
> > write it by hand.
> 
> I choose 'write it by hand'.  I want what I write by hand to also be
> useful to others by including the zynq-7000.dtsi in the upstream kernel.

Yes agreed.

> 
> [..]
> > If you want to use solution with several dtsi files and compose it as
> > you describe then it is completely fine but forcing others to use this
> > structure and write dts by hand will be big pain for a lot of users.
> 
> Using a composed model in the upstream kernel doesn't force anything
> upon the existing users of your generator.  They can still use whatever
> gets spit out of your generator (assuming it generates nodes with
> appropriate bindings).  Unless I'm missing something here.
> 
> > Also in design tools you can setup if you use qspi,nor,nand flash
> > memory interface.
> > memory interface, baudrates, dma, ports to PL logic, connections, etc.
> > and from my point of view is very complicated to describe it by
> >
> > There are a lot of combination which you can have on one reference
> > board.  You can't enable all hard IPs at one time and use all of them
> > that's why you shouldn't list all of them in the kernel.
> 
> I disagree with this.  In my opinion, all of the "hard IPs" should be
> described in the zynq-7000.dtsi, and those nodes which aren't available
> explicitly disabled in the board-specific file.

It looks like most everyone is doing this in their device trees.

> 
> > From my point of view make sense to have one DTS file in the kernel
> > and one defconfig for the most popular zynq board where will be
> > exactly written that this DTS is connected to this reference hw
> > design. If you want to get more reference design go to this page and
> > download it.  Adding all DTSes for zynq boards to the kernel is
> > overkill.  If you want to use your hw design you can use this
> > generator and generate it or write it by hand.
> 
> All I'm asking for is for there to be a common zynq-7000.dtsi that
> describes all of the static PS logic ("hard IPs") in the upstream kernel
> source that I can include in my own (hand maintained) board
> descriptions.  It would be nice if there was an example of its use, like
> with a zc702 board file also upstream, but it is not really important to
> me.
> 

Yes I understand and see the benefits too. 

> I do not want a dependency on the EDK.
> 

Understood and it makes sense what you're saying.

> My request does not sound unreasonable to me and is what other platforms
> are doing.
> 

Agreed.  We'll wrestle with this some more within Xilinx.

Thanks,
John

>   Josh

[PATCH v4 2/5] zynq: use pl310 device tree bindings

[Josh Cartwright]

The Zynq has a PL310 L2 cache controller.  Convert in-tree uses to using
the device tree.

Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
Cc: John Linn <john.linn@xilinx.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/boot/dts/zynq-ep107.dts           | 9 +++++++++
 arch/arm/mach-zynq/common.c                | 9 +--------
 arch/arm/mach-zynq/include/mach/zynq_soc.h | 4 ----
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq-ep107.dts
index 7bfff4a..87204d7 100644
--- a/arch/arm/boot/dts/zynq-ep107.dts
+++ b/arch/arm/boot/dts/zynq-ep107.dts
@@ -44,6 +44,15 @@
 			      <0xF8F00100 0x100>;
 		};
 
+		L2: cache-controller {
+			compatible = "arm,pl310-cache";
+			reg = <0xF8F02000 0x1000>;
+			arm,data-latency = <2 3 2>;
+			arm,tag-latency = <2 3 2>;
+			cache-unified;
+			cache-level = <2>;
+		};
+
 		uart0: uart@e0000000 {
 			compatible = "xlnx,xuartps";
 			reg = <0xE0000000 0x1000>;
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index d73963b..056091a 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -45,12 +45,10 @@ static struct of_device_id zynq_of_bus_ids[] __initdata = {
  */
 static void __init xilinx_init_machine(void)
 {
-#ifdef CONFIG_CACHE_L2X0
 	/*
 	 * 64KB way size, 8-way associativity, parity disabled
 	 */
-	l2x0_init(PL310_L2CC_BASE, 0x02060000, 0xF0F0FFFF);
-#endif
+	l2x0_of_init(0x02060000, 0xF0F0FFFF);
 
 	of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
 }
@@ -83,11 +81,6 @@ static struct map_desc io_desc[] __initdata = {
 		.pfn		= __phys_to_pfn(SCU_PERIPH_PHYS),
 		.length		= SZ_8K,
 		.type		= MT_DEVICE,
-	}, {
-		.virtual	= PL310_L2CC_VIRT,
-		.pfn		= __phys_to_pfn(PL310_L2CC_PHYS),
-		.length		= SZ_4K,
-		.type		= MT_DEVICE,
 	},
 
 #ifdef CONFIG_DEBUG_LL
diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
index 3d1c6a6..218283a 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -25,9 +25,6 @@
 #define TTC0_PHYS			0xF8001000
 #define TTC0_VIRT			TTC0_PHYS
 
-#define PL310_L2CC_PHYS			0xF8F02000
-#define PL310_L2CC_VIRT			PL310_L2CC_PHYS
-
 #define SCU_PERIPH_PHYS			0xF8F00000
 #define SCU_PERIPH_VIRT			SCU_PERIPH_PHYS
 
@@ -35,7 +32,6 @@
 
 #define TTC0_BASE			IOMEM(TTC0_VIRT)
 #define SCU_PERIPH_BASE			IOMEM(SCU_PERIPH_VIRT)
-#define PL310_L2CC_BASE			IOMEM(PL310_L2CC_VIRT)
 
 /*
  * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
-- 
1.8.0

RE: [PATCH v4 2/5] zynq: use pl310 device tree bindings

[Michal Simek]

> -----Original Message-----
> From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> Sent: Wednesday, October 24, 2012 10:04 PM
> To: arm@kernel.org; Arnd Bergmann
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; John
> Linn; Nick Bowler; Michal Simek
> Subject: [PATCH v4 2/5] zynq: use pl310 device tree bindings
>
> The Zynq has a PL310 L2 cache controller.  Convert in-tree uses to using the
> device tree.
>
> Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
> Cc: John Linn <john.linn@xilinx.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/boot/dts/zynq-ep107.dts           | 9 +++++++++
>  arch/arm/mach-zynq/common.c                | 9 +--------
>  arch/arm/mach-zynq/include/mach/zynq_soc.h | 4 ----
>  3 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq-
> ep107.dts
> index 7bfff4a..87204d7 100644
> --- a/arch/arm/boot/dts/zynq-ep107.dts
> +++ b/arch/arm/boot/dts/zynq-ep107.dts
> @@ -44,6 +44,15 @@
>                             <0xF8F00100 0x100>;
>               };
>
> +             L2: cache-controller {
> +                     compatible = "arm,pl310-cache";
> +                     reg = <0xF8F02000 0x1000>;
> +                     arm,data-latency = <2 3 2>;
> +                     arm,tag-latency = <2 3 2>;
> +                     cache-unified;
> +                     cache-level = <2>;
> +             };
> +
>               uart0: uart@e0000000 {
>                       compatible = "xlnx,xuartps";
>                       reg = <0xE0000000 0x1000>;
> diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> index d73963b..056091a 100644
> --- a/arch/arm/mach-zynq/common.c
> +++ b/arch/arm/mach-zynq/common.c
> @@ -45,12 +45,10 @@ static struct of_device_id zynq_of_bus_ids[] __initdata = {
>   */
>  static void __init xilinx_init_machine(void)  { -#ifdef CONFIG_CACHE_L2X0
>       /*
>        * 64KB way size, 8-way associativity, parity disabled
>        */
> -     l2x0_init(PL310_L2CC_BASE, 0x02060000, 0xF0F0FFFF);
> -#endif
> +     l2x0_of_init(0x02060000, 0xF0F0FFFF);
>
>       of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);  } @@ -83,11
> +81,6 @@ static struct map_desc io_desc[] __initdata = {
>               .pfn            = __phys_to_pfn(SCU_PERIPH_PHYS),
>               .length         = SZ_8K,
>               .type           = MT_DEVICE,
> -     }, {
> -             .virtual        = PL310_L2CC_VIRT,
> -             .pfn            = __phys_to_pfn(PL310_L2CC_PHYS),
> -             .length         = SZ_4K,
> -             .type           = MT_DEVICE,
>       },
>
>  #ifdef CONFIG_DEBUG_LL
> diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-
> zynq/include/mach/zynq_soc.h
> index 3d1c6a6..218283a 100644
> --- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
> +++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
> @@ -25,9 +25,6 @@
>  #define TTC0_PHYS                    0xF8001000
>  #define TTC0_VIRT                    TTC0_PHYS
>
> -#define PL310_L2CC_PHYS                      0xF8F02000
> -#define PL310_L2CC_VIRT                      PL310_L2CC_PHYS
> -
>  #define SCU_PERIPH_PHYS                      0xF8F00000
>  #define SCU_PERIPH_VIRT                      SCU_PERIPH_PHYS
>
> @@ -35,7 +32,6 @@
>
>  #define TTC0_BASE                    IOMEM(TTC0_VIRT)
>  #define SCU_PERIPH_BASE                      IOMEM(SCU_PERIPH_VIRT)
> -#define PL310_L2CC_BASE                      IOMEM(PL310_L2CC_VIRT)
>
>  /*
>   * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
> --
> 1.8.0
>

This is ok. No changes are necessary.
Acked-by: Michal Simek <michal.simek@xilinx.com>

Please add my acked-by line to this patch to the v5 series.
When all patches are ready I will apply it to zynq next branch at git.xilnx.com.

Thanks,
Michal



This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH v4 3/5] zynq: remove use of CLKDEV_LOOKUP

[Josh Cartwright]

The Zynq support in mainline does not (yet) make use of any of the
generic clk or clk lookup functionality.  Remove what is upstream for
now, until the out-of-tree implementation is in suitable form for
merging.

An important side effect of this patch is that it allows the building of
a Zynq kernel without running into unresolved symbol problems:

   drivers/built-in.o: In function `amba_get_enable_pclk':
   clkdev.c:(.text+0x444): undefined reference to `clk_enable'
   drivers/built-in.o: In function `amba_remove':
   clkdev.c:(.text+0x488): undefined reference to `clk_disable'
   drivers/built-in.o: In function `amba_probe':
   clkdev.c:(.text+0x540): undefined reference to `clk_disable'
   drivers/built-in.o: In function `amba_device_add':
   clkdev.c:(.text+0x77c): undefined reference to `clk_disable'
   drivers/built-in.o: In function `enable_clock':
   clkdev.c:(.text+0x29738): undefined reference to `clk_enable'
   drivers/built-in.o: In function `disable_clock':
   clkdev.c:(.text+0x29778): undefined reference to `clk_disable'
   drivers/built-in.o: In function `__pm_clk_remove':
   clkdev.c:(.text+0x297f8): undefined reference to `clk_disable'
   drivers/built-in.o: In function `pm_clk_suspend':
   clkdev.c:(.text+0x29bc8): undefined reference to `clk_disable'
   drivers/built-in.o: In function `pm_clk_resume':
   clkdev.c:(.text+0x29c28): undefined reference to `clk_enable'
   make[2]: *** [vmlinux] Error 1
   make[1]: *** [sub-make] Error 2
   make: *** [all] Error 2

In addition, eliminate Zynq's "use" of the versatile platform, as it is
no longer needed.  As Nick Bowler points out:

   For the record, I think this was introduced by commit 56a34b03ff427
   ("ARM: versatile: Make plat-versatile clock optional") which forgot to
   select PLAT_VERSATILE_CLOCK on Zynq.  This is not all that surprising,
   because the fact that Zynq "uses" PLAT_VERSATILE is secretly hidden in
   the Makefile.

   Nevertheless, the only feature from versatile that Zynq needed was the
   clock support, so this patch should *also* delete the secret use of
   plat-versatile by removing this line from arch/arm/Makefile:

      plat-$(CONFIG_ARCH_ZYNQ)      += versatile

Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
Cc: John Linn <john.linn@xilinx.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/Kconfig                         |  1 -
 arch/arm/Makefile                        |  1 -
 arch/arm/mach-zynq/common.c              |  1 -
 arch/arm/mach-zynq/include/mach/clkdev.h | 32 --------------------------------
 4 files changed, 35 deletions(-)
 delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index cce4f8d..de70d99 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -946,7 +946,6 @@ config ARCH_ZYNQ
 	bool "Xilinx Zynq ARM Cortex A9 Platform"
 	select ARM_AMBA
 	select ARM_GIC
-	select CLKDEV_LOOKUP
 	select CPU_V7
 	select GENERIC_CLOCKEVENTS
 	select ICST
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 451757d..8dbab2d 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -197,7 +197,6 @@ machine-$(CONFIG_ARCH_ZYNQ)		+= zynq
 # by CONFIG_* macro name.
 plat-$(CONFIG_ARCH_OMAP)	+= omap
 plat-$(CONFIG_ARCH_S3C64XX)	+= samsung
-plat-$(CONFIG_ARCH_ZYNQ)	+= versatile
 plat-$(CONFIG_PLAT_IOP)		+= iop
 plat-$(CONFIG_PLAT_NOMADIK)	+= nomadik
 plat-$(CONFIG_PLAT_ORION)	+= orion
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 056091a..ba48f06 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -31,7 +31,6 @@
 #include <asm/hardware/cache-l2x0.h>
 
 #include <mach/zynq_soc.h>
-#include <mach/clkdev.h>
 #include "common.h"
 
 static struct of_device_id zynq_of_bus_ids[] __initdata = {
diff --git a/arch/arm/mach-zynq/include/mach/clkdev.h b/arch/arm/mach-zynq/include/mach/clkdev.h
deleted file mode 100644
index c6e73d8..0000000
--- a/arch/arm/mach-zynq/include/mach/clkdev.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/*
- * arch/arm/mach-zynq/include/mach/clkdev.h
- *
- *  Copyright (C) 2011 Xilinx, Inc.
- *
- * 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.
- *
- */
-
-#ifndef __MACH_CLKDEV_H__
-#define __MACH_CLKDEV_H__
-
-#include <plat/clock.h>
-
-struct clk {
-	unsigned long		rate;
-	const struct clk_ops	*ops;
-	const struct icst_params *params;
-	void __iomem		*vcoreg;
-};
-
-#define __clk_get(clk) ({ 1; })
-#define __clk_put(clk) do { } while (0)
-
-#endif
-- 
1.8.0

RE: [PATCH v4 3/5] zynq: remove use of CLKDEV_LOOKUP

[Michal Simek]

> -----Original Message-----
> From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> Sent: Wednesday, October 24, 2012 10:04 PM
> To: arm@kernel.org; Arnd Bergmann
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; John
> Linn; Nick Bowler; Michal Simek
> Subject: [PATCH v4 3/5] zynq: remove use of CLKDEV_LOOKUP
>
> The Zynq support in mainline does not (yet) make use of any of the generic clk
> or clk lookup functionality.  Remove what is upstream for now, until the out-of-
> tree implementation is in suitable form for merging.
>
> An important side effect of this patch is that it allows the building of a Zynq
> kernel without running into unresolved symbol problems:
>
>    drivers/built-in.o: In function `amba_get_enable_pclk':
>    clkdev.c:(.text+0x444): undefined reference to `clk_enable'
>    drivers/built-in.o: In function `amba_remove':
>    clkdev.c:(.text+0x488): undefined reference to `clk_disable'
>    drivers/built-in.o: In function `amba_probe':
>    clkdev.c:(.text+0x540): undefined reference to `clk_disable'
>    drivers/built-in.o: In function `amba_device_add':
>    clkdev.c:(.text+0x77c): undefined reference to `clk_disable'
>    drivers/built-in.o: In function `enable_clock':
>    clkdev.c:(.text+0x29738): undefined reference to `clk_enable'
>    drivers/built-in.o: In function `disable_clock':
>    clkdev.c:(.text+0x29778): undefined reference to `clk_disable'
>    drivers/built-in.o: In function `__pm_clk_remove':
>    clkdev.c:(.text+0x297f8): undefined reference to `clk_disable'
>    drivers/built-in.o: In function `pm_clk_suspend':
>    clkdev.c:(.text+0x29bc8): undefined reference to `clk_disable'
>    drivers/built-in.o: In function `pm_clk_resume':
>    clkdev.c:(.text+0x29c28): undefined reference to `clk_enable'
>    make[2]: *** [vmlinux] Error 1
>    make[1]: *** [sub-make] Error 2
>    make: *** [all] Error 2
>
> In addition, eliminate Zynq's "use" of the versatile platform, as it is no longer
> needed.  As Nick Bowler points out:
>
>    For the record, I think this was introduced by commit 56a34b03ff427
>    ("ARM: versatile: Make plat-versatile clock optional") which forgot to
>    select PLAT_VERSATILE_CLOCK on Zynq.  This is not all that surprising,
>    because the fact that Zynq "uses" PLAT_VERSATILE is secretly hidden in
>    the Makefile.
>
>    Nevertheless, the only feature from versatile that Zynq needed was the
>    clock support, so this patch should *also* delete the secret use of
>    plat-versatile by removing this line from arch/arm/Makefile:
>
>       plat-$(CONFIG_ARCH_ZYNQ)      += versatile
>
> Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
> Cc: John Linn <john.linn@xilinx.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/Kconfig                         |  1 -
>  arch/arm/Makefile                        |  1 -
>  arch/arm/mach-zynq/common.c              |  1 -
>  arch/arm/mach-zynq/include/mach/clkdev.h | 32 --------------------------------
>  4 files changed, 35 deletions(-)
>  delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index cce4f8d..de70d99 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -946,7 +946,6 @@ config ARCH_ZYNQ
>       bool "Xilinx Zynq ARM Cortex A9 Platform"
>       select ARM_AMBA
>       select ARM_GIC
> -     select CLKDEV_LOOKUP
>       select CPU_V7
>       select GENERIC_CLOCKEVENTS
>       select ICST
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile index 451757d..8dbab2d
> 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -197,7 +197,6 @@ machine-$(CONFIG_ARCH_ZYNQ)               += zynq
>  # by CONFIG_* macro name.
>  plat-$(CONFIG_ARCH_OMAP)     += omap
>  plat-$(CONFIG_ARCH_S3C64XX)  += samsung
> -plat-$(CONFIG_ARCH_ZYNQ)     += versatile
>  plat-$(CONFIG_PLAT_IOP)              += iop
>  plat-$(CONFIG_PLAT_NOMADIK)  += nomadik
>  plat-$(CONFIG_PLAT_ORION)    += orion
> diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
> index 056091a..ba48f06 100644
> --- a/arch/arm/mach-zynq/common.c
> +++ b/arch/arm/mach-zynq/common.c
> @@ -31,7 +31,6 @@
>  #include <asm/hardware/cache-l2x0.h>
>
>  #include <mach/zynq_soc.h>
> -#include <mach/clkdev.h>
>  #include "common.h"
>
>  static struct of_device_id zynq_of_bus_ids[] __initdata = { diff --git
> a/arch/arm/mach-zynq/include/mach/clkdev.h b/arch/arm/mach-
> zynq/include/mach/clkdev.h
> deleted file mode 100644
> index c6e73d8..0000000
> --- a/arch/arm/mach-zynq/include/mach/clkdev.h
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/*
> - * arch/arm/mach-zynq/include/mach/clkdev.h
> - *
> - *  Copyright (C) 2011 Xilinx, Inc.
> - *
> - * 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.
> - *
> - */
> -
> -#ifndef __MACH_CLKDEV_H__
> -#define __MACH_CLKDEV_H__
> -
> -#include <plat/clock.h>
> -
> -struct clk {
> -     unsigned long           rate;
> -     const struct clk_ops    *ops;
> -     const struct icst_params *params;
> -     void __iomem            *vcoreg;
> -};
> -
> -#define __clk_get(clk) ({ 1; })
> -#define __clk_put(clk) do { } while (0)
> -
> -#endif

Tested-by: Michal Simek <michal.simek@xilinx.com>

Thanks
Michal


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC

[Josh Cartwright]

This makes the definition of VMALLOC_END suitable for use within
assembly code.  This is necessary to allow the use of VMALLOC_END in
defining where the early uart is mapped for use with DEBUG_LL.

Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/include/asm/pgtable.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 08c1231..72904a2 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -40,7 +40,7 @@
  */
 #define VMALLOC_OFFSET		(8*1024*1024)
 #define VMALLOC_START		(((unsigned long)high_memory + VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1))
-#define VMALLOC_END		0xff000000UL
+#define VMALLOC_END		_AC(0xff000000,UL)
 
 #define LIBRARY_TEXT_START	0x0c000000
 
-- 
1.8.0

RE: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC

[Michal Simek]

> -----Original Message-----
> From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> Sent: Wednesday, October 24, 2012 10:05 PM
> To: arm@kernel.org; Arnd Bergmann
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; John
> Linn; Nick Bowler; Michal Simek
> Subject: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC
> 
> This makes the definition of VMALLOC_END suitable for use within assembly
> code.  This is necessary to allow the use of VMALLOC_END in defining where the
> early uart is mapped for use with DEBUG_LL.
> 
> Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/include/asm/pgtable.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> index 08c1231..72904a2 100644
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -40,7 +40,7 @@
>   */
>  #define VMALLOC_OFFSET		(8*1024*1024)
>  #define VMALLOC_START		(((unsigned long)high_memory +
> VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1))
> -#define VMALLOC_END		0xff000000UL
> +#define VMALLOC_END		_AC(0xff000000,UL)

This shouldn't be the part of this series but should go to mainline through different tree.
Arnd, Olof: Can you take this patch to your arm-soc tree? 

I don't think it is a good workstyle to propose it to mainline through zynq soc tree.
What do you think?

Thanks,
Michal

Re: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC

[Arnd Bergmann]

On Saturday 27 October 2012, Michal Simek wrote:
> > diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> > index 08c1231..72904a2 100644
> > --- a/arch/arm/include/asm/pgtable.h
> > +++ b/arch/arm/include/asm/pgtable.h
> > @@ -40,7 +40,7 @@
> >   */
> >  #define VMALLOC_OFFSET               (8*1024*1024)
> >  #define VMALLOC_START                (((unsigned long)high_memory +
> > VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1))
> > -#define VMALLOC_END          0xff000000UL
> > +#define VMALLOC_END          _AC(0xff000000,UL)
> 
> This shouldn't be the part of this series but should go to mainline through different tree.
> Arnd, Olof: Can you take this patch to your arm-soc tree? 
> 
> I don't think it is a good workstyle to propose it to mainline through zynq soc tree.
> What do you think?

The arm-soc tree is not the right place either, this is architecture code which is
in Russell's domain. I would suggest getting an Ack from Russell if he's ok with
it and then merging it together with your other changes into arm-soc.

	Arnd

RE: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC

[Michal Simek]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Tuesday, October 30, 2012 11:22 PM
> To: Michal Simek
> Cc: Josh Cartwright; arm@kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; John Linn; Nick Bowler; Russell King - ARM Linux
> Subject: Re: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC
>
> On Saturday 27 October 2012, Michal Simek wrote:
> > > diff --git a/arch/arm/include/asm/pgtable.h
> > > b/arch/arm/include/asm/pgtable.h index 08c1231..72904a2 100644
> > > --- a/arch/arm/include/asm/pgtable.h
> > > +++ b/arch/arm/include/asm/pgtable.h
> > > @@ -40,7 +40,7 @@
> > >   */
> > >  #define VMALLOC_OFFSET               (8*1024*1024)
> > >  #define VMALLOC_START                (((unsigned long)high_memory +
> > > VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1))
> > > -#define VMALLOC_END          0xff000000UL
> > > +#define VMALLOC_END          _AC(0xff000000,UL)
> >
> > This shouldn't be the part of this series but should go to mainline through
> different tree.
> > Arnd, Olof: Can you take this patch to your arm-soc tree?
> >
> > I don't think it is a good workstyle to propose it to mainline through zynq soc
> tree.
> > What do you think?
>
> The arm-soc tree is not the right place either, this is architecture code which is in
> Russell's domain. I would suggest getting an Ack from Russell if he's ok with it
> and then merging it together with your other changes into arm-soc.

That's what I thought too.
Not sure if Josh wants to push this to mainline.

Thanks,
Michal




This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC

[Josh Cartwright]

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

On Wed, Oct 31, 2012 at 08:43:35AM +0000, Michal Simek wrote:
> 
> 
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > Sent: Tuesday, October 30, 2012 11:22 PM
> > To: Michal Simek
> > Cc: Josh Cartwright; arm@kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; John Linn; Nick Bowler; Russell King - ARM Linux
> > Subject: Re: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC
> >
> > On Saturday 27 October 2012, Michal Simek wrote:
> > > > diff --git a/arch/arm/include/asm/pgtable.h
> > > > b/arch/arm/include/asm/pgtable.h index 08c1231..72904a2 100644
> > > > --- a/arch/arm/include/asm/pgtable.h
> > > > +++ b/arch/arm/include/asm/pgtable.h
> > > > @@ -40,7 +40,7 @@
> > > >   */
> > > >  #define VMALLOC_OFFSET               (8*1024*1024)
> > > >  #define VMALLOC_START                (((unsigned long)high_memory +
> > > > VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1))
> > > > -#define VMALLOC_END          0xff000000UL
> > > > +#define VMALLOC_END          _AC(0xff000000,UL)
> > >
> > > This shouldn't be the part of this series but should go to mainline through
> > different tree.
> > > Arnd, Olof: Can you take this patch to your arm-soc tree?
> > >
> > > I don't think it is a good workstyle to propose it to mainline through zynq soc
> > tree.
> > > What do you think?
> >
> > The arm-soc tree is not the right place either, this is architecture code which is in
> > Russell's domain. I would suggest getting an Ack from Russell if he's ok with it
> > and then merging it together with your other changes into arm-soc.
> 
> That's what I thought too.
> Not sure if Josh wants to push this to mainline.

The patch is not relevant anymore.  It was needed when the virtual
address of the early uart mapping was defined via VMALLOC_END.  That was
causing us problems, so we ended up choosing a fixed, known working
address within the vmalloc region to map the uart.

Thanks,

  Josh

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH v4 5/5] zynq: move static peripheral mappings

[Josh Cartwright]

Shifting them up into the vmalloc region prevents the following warning,
when booting a zynq qemu target with more than 512mb of RAM:

  BUG: mapping for 0xe0000000 at 0xe0000000 out of vmalloc space

In addition, it allows for reuse of these mappings when the proper
drivers issue requests via ioremap().

Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
Cc: John Linn <john.linn@xilinx.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-zynq/common.c                |  6 +++---
 arch/arm/mach-zynq/include/mach/zynq_soc.h | 23 +++++++++++++----------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index ba48f06..ba8d14f 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -73,12 +73,12 @@ static struct map_desc io_desc[] __initdata = {
 	{
 		.virtual	= TTC0_VIRT,
 		.pfn		= __phys_to_pfn(TTC0_PHYS),
-		.length		= SZ_4K,
+		.length		= TTC0_SIZE,
 		.type		= MT_DEVICE,
 	}, {
 		.virtual	= SCU_PERIPH_VIRT,
 		.pfn		= __phys_to_pfn(SCU_PERIPH_PHYS),
-		.length		= SZ_8K,
+		.length		= SCU_PERIPH_SIZE,
 		.type		= MT_DEVICE,
 	},
 
@@ -86,7 +86,7 @@ static struct map_desc io_desc[] __initdata = {
 	{
 		.virtual	= UART0_VIRT,
 		.pfn		= __phys_to_pfn(UART0_PHYS),
-		.length		= SZ_4K,
+		.length		= UART0_SIZE,
 		.type		= MT_DEVICE,
 	},
 #endif
diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
index 218283a..c6b9b67 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -15,27 +15,30 @@
 #ifndef __MACH_XILINX_SOC_H__
 #define __MACH_XILINX_SOC_H__
 
+#include <asm/pgtable.h>
+
 #define PERIPHERAL_CLOCK_RATE		2500000
 
-/* For now, all mappings are flat (physical = virtual)
+/* Static peripheral mappings are mapped at the top of the
+ * vmalloc region
  */
-#define UART0_PHYS			0xE0000000
-#define UART0_VIRT			UART0_PHYS
+#define UART0_PHYS		0xE0000000
+#define UART0_SIZE		SZ_4K
+#define UART0_VIRT		(VMALLOC_END - UART0_SIZE)
 
-#define TTC0_PHYS			0xF8001000
-#define TTC0_VIRT			TTC0_PHYS
+#define TTC0_PHYS		0xF8001000
+#define TTC0_SIZE		SZ_4K
+#define TTC0_VIRT		(UART0_VIRT - TTC0_SIZE)
 
-#define SCU_PERIPH_PHYS			0xF8F00000
-#define SCU_PERIPH_VIRT			SCU_PERIPH_PHYS
+#define SCU_PERIPH_PHYS		0xF8F00000
+#define SCU_PERIPH_SIZE		SZ_8K
+#define SCU_PERIPH_VIRT		(TTC0_VIRT - SCU_PERIPH_SIZE)
 
 /* The following are intended for the devices that are mapped early */
 
 #define TTC0_BASE			IOMEM(TTC0_VIRT)
 #define SCU_PERIPH_BASE			IOMEM(SCU_PERIPH_VIRT)
 
-/*
- * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
- */
 #define LL_UART_PADDR	UART0_PHYS
 #define LL_UART_VADDR	UART0_VIRT
 
-- 
1.8.0

Re: [PATCH v4 5/5] zynq: move static peripheral mappings

[Nick Bowler]

Hi Josh,

On 2012-10-24 15:04 -0500, Josh Cartwright wrote:
> Shifting them up into the vmalloc region prevents the following warning,
> when booting a zynq qemu target with more than 512mb of RAM:
[...]
> -/* For now, all mappings are flat (physical = virtual)
> +/* Static peripheral mappings are mapped at the top of the
> + * vmalloc region
>   */
> -#define UART0_PHYS			0xE0000000
> -#define UART0_VIRT			UART0_PHYS
> +#define UART0_PHYS		0xE0000000
> +#define UART0_SIZE		SZ_4K
> +#define UART0_VIRT		(VMALLOC_END - UART0_SIZE)

Did you test this on any real hardware?  I can't get the ZC702 to work
with the UART mapped at this address (this ends up being mapped at
0xFEFFF000), although I can't for the life of me figure out why the
virtual address even matters.  Note that for the ZC702, the physical
address of the "main" UART is 0xE0001000.

All I end up seeing is "Uncompressing Linux... done, booting the
kernel." with no further messages.  With the UART mapped at
0xF0001000, all printouts make it to the console.  I tried a couple
different virtual addresses and I'm surprised at the results, since
the behaviour seems to vary wildly.  I saw three behaviours depending
only on the virtual address of the static mapping; all results are 100%
reproducible:

   "Works":     all printouts make it to the console
   "Fails":     no printouts make it to the console after decompression
   "Truncated": the first few lines of output do not make it to the
                console, but after that it "Works".  The first line
		successfully printed is always
		  "Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 260096"

And here are the addresses I tested:

  Address       Result
  -----------------------
  0xf0000000    Truncated
  0xf0001000    Works
  0xf0007000    Truncated
  0xf0008000    Fails
  0xf0009000    Fails
  0xf000e000    Truncated
  0xf000f000    Fails
  0xf8000000    Truncated
  0xf8001000    Works
  0xfef00000    Truncated
  0xfef01000    Works
  0xfef08000    Fails
  0xfef0f000    Fails
  0xfeff0000    Fails
  0xfeff1000    Fails
  0xfeffe000    Fails
  0xfefff000    Fails

Judging by the list, the console seems to only work properly if the
defined virtual address is Fxxx1000 and xxx is not too big...

Confused,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Re: [PATCH v4 5/5] zynq: move static peripheral mappings

[Josh Cartwright]

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

On Thu, Oct 25, 2012 at 04:17:01PM -0400, Nick Bowler wrote:
> Hi Josh,
>
> On 2012-10-24 15:04 -0500, Josh Cartwright wrote:
> > Shifting them up into the vmalloc region prevents the following warning,
> > when booting a zynq qemu target with more than 512mb of RAM:
> [...]
> > -/* For now, all mappings are flat (physical = virtual)
> > +/* Static peripheral mappings are mapped at the top of the
> > + * vmalloc region
> >   */
> > -#define UART0_PHYS			0xE0000000
> > -#define UART0_VIRT			UART0_PHYS
> > +#define UART0_PHYS		0xE0000000
> > +#define UART0_SIZE		SZ_4K
> > +#define UART0_VIRT		(VMALLOC_END - UART0_SIZE)
>
> Did you test this on any real hardware?  I can't get the ZC702 to work
> with the UART mapped at this address (this ends up being mapped at
> 0xFEFFF000), although I can't for the life of me figure out why the
> virtual address even matters.  Note that for the ZC702, the physical
> address of the "main" UART is 0xE0001000.

Ugh, not yet;  My testing has been on a qemu model.  I also
unfortunately neglected to mention I am carrying a qemu patch that
forces RX_EN/TX_EN of the uarts out of reset.  There is an (incomplete)
thread on qemu-devel discussing whose responsibility it really is to
enable the uarts:

   http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg03779.html

Clearly, though, if you are seeing the "Uncompressing Linux..."
messages, then the uart is enabled, so I don't think that's the problem.

> All I end up seeing is "Uncompressing Linux... done, booting the
> kernel." with no further messages.  With the UART mapped at
> 0xF0001000, all printouts make it to the console.  I tried a couple
> different virtual addresses and I'm surprised at the results, since
> the behaviour seems to vary wildly.  I saw three behaviours depending
> only on the virtual address of the static mapping; all results are 100%
> reproducible:
>
>    "Works":     all printouts make it to the console
>    "Fails":     no printouts make it to the console after decompression
>    "Truncated": the first few lines of output do not make it to the
>                 console, but after that it "Works".  The first line
> 		successfully printed is always
> 		  "Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 260096"

Odd, I'm wondering the uart gets into a weird state, and some bits get
knocked loose at console_initcall() time, when the console driver comes
up (Assuming CONFIG_SERIAL_XILINX_PS_UART)?

> And here are the addresses I tested:
> 
>   Address       Result
>   -----------------------
>   0xf0000000    Truncated
>   0xf0001000    Works
>   0xf0007000    Truncated
>   0xf0008000    Fails
>   0xf0009000    Fails
>   0xf000e000    Truncated
>   0xf000f000    Fails
>   0xf8000000    Truncated
>   0xf8001000    Works
>   0xfef00000    Truncated
>   0xfef01000    Works
>   0xfef08000    Fails
>   0xfef0f000    Fails
>   0xfeff0000    Fails
>   0xfeff1000    Fails
>   0xfeffe000    Fails
>   0xfefff000    Fails
> 
> Judging by the list, the console seems to only work properly if the
> defined virtual address is Fxxx1000 and xxx is not too big...

Very odd.  Do you mind sending out your patch allowing the selection of
the secondary uart for DEBUG_LL?

Thanks,

  Josh

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v4 5/5] zynq: move static peripheral mappings

[Nick Bowler]

On 2012-10-25 16:29 -0500, Josh Cartwright wrote:
> On Thu, Oct 25, 2012 at 04:17:01PM -0400, Nick Bowler wrote:
> > Did you test this on any real hardware?  I can't get the ZC702 to work
> > with the UART mapped at this address (this ends up being mapped at
> > 0xFEFFF000), although I can't for the life of me figure out why the
> > virtual address even matters.  Note that for the ZC702, the physical
> > address of the "main" UART is 0xE0001000.
> 
> Ugh, not yet;  My testing has been on a qemu model.  I also
> unfortunately neglected to mention I am carrying a qemu patch that
> forces RX_EN/TX_EN of the uarts out of reset.  There is an (incomplete)
> thread on qemu-devel discussing whose responsibility it really is to
> enable the uarts:
> 
>    http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg03779.html
> 
> Clearly, though, if you are seeing the "Uncompressing Linux..."
> messages, then the uart is enabled, so I don't think that's the problem.

Yes, the uart is presumably enabled by u-boot.

> >    "Works":     all printouts make it to the console
> >    "Fails":     no printouts make it to the console after decompression
> >    "Truncated": the first few lines of output do not make it to the
> >                 console, but after that it "Works".  The first line
> > 		successfully printed is always
> > 		  "Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 260096"
> 
> Odd, I'm wondering the uart gets into a weird state, and some bits get
> knocked loose at console_initcall() time, when the console driver comes
> up (Assuming CONFIG_SERIAL_XILINX_PS_UART)?

While I am using that driver, it is not initialized until relatively
late in the boot process.  If I were to guess, I would guess that,
except for when it "Works", the really really early printk stuff isn't
actually hitting the uart at all.  The "Fails" case would then be due to
the stray writes crashing the board, and the "Truncated" case due to the
stray writes being (ostensibly) benign.

But I really have no way right now to test this hypothesis, since I
can't print anything in the failing case.

> > And here are the addresses I tested:
> > 
> >   Address       Result
> >   -----------------------
> >   0xf0000000    Truncated
> >   0xf0001000    Works
[...]
> >   0xfefff000    Fails
> > 
> > Judging by the list, the console seems to only work properly if the
> > defined virtual address is Fxxx1000 and xxx is not too big...
> 
> Very odd.  Do you mind sending out your patch allowing the selection of
> the secondary uart for DEBUG_LL?

I will follow up with the version that applies on top of your series in
a moment.  I'm confident that the UART works on the ZC702 when mapped at
0xf0001000, since I've been running with that since I first got my hands
on one of these boards.

But you don't need any patch to do the same tests I was doing above:
you can just change UART0_PHYS to 0xe0001000 and then set UART0_VIRT
accordingly (you may need to move the TTC/SCU mappings depending where
you put the UART, of course).

Cheers,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

[PATCH] ARM: zynq: Allow UART1 to be used as DEBUG_LL console.

[Nick Bowler]

The main UART on the Xilinx ZC702 board is UART1, located at address
e0001000.  Add a Kconfig option to select this device as the low-level
debugging port.  This allows the really early boot printouts to reach
the USB serial adaptor on this board.

For consistency's sake, add a choice entry for UART0 even though it is
the the default if UART1 is not selected.

As there are currently known issues related to the UART virtual
mappings, this is KNOWN BROKEN, not to be merged yet!

Not-Yet-Signed-off-by: Nick Bowler <nbowler@elliptictech.com>
---
 arch/arm/Kconfig.debug                     |   17 +++++++++++++++++
 arch/arm/mach-zynq/common.c                |    6 +++---
 arch/arm/mach-zynq/include/mach/zynq_soc.h |   18 ++++++++++++------
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index b0f3857b3a4c..7754d51f2b19 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -132,6 +132,23 @@ choice
 		  their output to UART1 serial port on DaVinci TNETV107X
 		  devices.
 
+	config DEBUG_ZYNQ_UART0
+		bool "Kernel low-level debugging on Xilinx Zynq using UART0"
+		depends on ARCH_ZYNQ
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to UART0 on the Zynq platform.
+
+	config DEBUG_ZYNQ_UART1
+		bool "Kernel low-level debugging on Xilinx Zynq using UART1"
+		depends on ARCH_ZYNQ
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to UART1 on the Zynq platform.
+
+		  If you have a ZC702 board and want early boot messages to
+		  appear on the USB serial adaptor, select this option.
+
 	config DEBUG_DC21285_PORT
 		bool "Kernel low-level debugging messages via footbridge serial port"
 		depends on FOOTBRIDGE
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index ba8d14f78d4d..93b91059faab 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -84,9 +84,9 @@ static struct map_desc io_desc[] __initdata = {
 
 #ifdef CONFIG_DEBUG_LL
 	{
-		.virtual	= UART0_VIRT,
-		.pfn		= __phys_to_pfn(UART0_PHYS),
-		.length		= UART0_SIZE,
+		.virtual	= LL_UART_VADDR,
+		.pfn		= __phys_to_pfn(LL_UART_PADDR),
+		.length		= UART_SIZE,
 		.type		= MT_DEVICE,
 	},
 #endif
diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
index c6b9b67bf7c7..cab72bfd183c 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -23,23 +23,29 @@
  * vmalloc region
  */
 #define UART0_PHYS		0xE0000000
-#define UART0_SIZE		SZ_4K
-#define UART0_VIRT		(VMALLOC_END - UART0_SIZE)
+#define UART1_PHYS		0xE0001000
+#define UART_SIZE		SZ_4K
+#define UART_VIRT		(VMALLOC_END - UART_SIZE)
 
 #define TTC0_PHYS		0xF8001000
 #define TTC0_SIZE		SZ_4K
-#define TTC0_VIRT		(UART0_VIRT - TTC0_SIZE)
+#define TTC0_VIRT		(UART_VIRT - TTC0_SIZE)
 
 #define SCU_PERIPH_PHYS		0xF8F00000
 #define SCU_PERIPH_SIZE		SZ_8K
 #define SCU_PERIPH_VIRT		(TTC0_VIRT - SCU_PERIPH_SIZE)
 
+#if IS_ENABLED(CONFIG_DEBUG_ZYNQ_UART1)
+#	define LL_UART_PADDR	UART1_PHYS
+#	define LL_UART_VADDR	UART_VIRT
+#else
+#	define LL_UART_PADDR	UART0_PHYS
+#	define LL_UART_VADDR	UART_VIRT
+#endif
+
 /* The following are intended for the devices that are mapped early */
 
 #define TTC0_BASE			IOMEM(TTC0_VIRT)
 #define SCU_PERIPH_BASE			IOMEM(SCU_PERIPH_VIRT)
 
-#define LL_UART_PADDR	UART0_PHYS
-#define LL_UART_VADDR	UART0_VIRT
-
 #endif
-- 
1.7.8.6

Re: [PATCH] ARM: zynq: Allow UART1 to be used as DEBUG_LL console.

[Josh Cartwright]

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

On Thu, Oct 25, 2012 at 06:47:34PM -0400, Nick Bowler wrote:
> The main UART on the Xilinx ZC702 board is UART1, located at address
> e0001000.  Add a Kconfig option to select this device as the low-level
> debugging port.  This allows the really early boot printouts to reach
> the USB serial adaptor on this board.
> 
> For consistency's sake, add a choice entry for UART0 even though it is
> the the default if UART1 is not selected.
> 
> As there are currently known issues related to the UART virtual
> mappings, this is KNOWN BROKEN, not to be merged yet!
> 
> Not-Yet-Signed-off-by: Nick Bowler <nbowler@elliptictech.com>

Tested-by: Josh Cartwright <josh.cartwright@ni.com>

Now that v5 of the initial zynq cleanup patchset is queued up for
merging (with a workaround for the uart mapping problem), what would it
take for you to sign off on this patch?

There is some trivial merging that has to be done to get it to apply
cleanly on v5.  See a rebased version below.

Thanks,
  Josh

diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index b0f3857..7754d51 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -132,6 +132,23 @@ choice
 		  their output to UART1 serial port on DaVinci TNETV107X
 		  devices.
 
+	config DEBUG_ZYNQ_UART0
+		bool "Kernel low-level debugging on Xilinx Zynq using UART0"
+		depends on ARCH_ZYNQ
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to UART0 on the Zynq platform.
+
+	config DEBUG_ZYNQ_UART1
+		bool "Kernel low-level debugging on Xilinx Zynq using UART1"
+		depends on ARCH_ZYNQ
+		help
+		  Say Y here if you want the debug print routines to direct
+		  their output to UART1 on the Zynq platform.
+
+		  If you have a ZC702 board and want early boot messages to
+		  appear on the USB serial adaptor, select this option.
+
 	config DEBUG_DC21285_PORT
 		bool "Kernel low-level debugging messages via footbridge serial port"
 		depends on FOOTBRIDGE
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index ba8d14f..93b9105 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -84,9 +84,9 @@ static struct map_desc io_desc[] __initdata = {
 
 #ifdef CONFIG_DEBUG_LL
 	{
-		.virtual	= UART0_VIRT,
-		.pfn		= __phys_to_pfn(UART0_PHYS),
-		.length		= UART0_SIZE,
+		.virtual	= LL_UART_VADDR,
+		.pfn		= __phys_to_pfn(LL_UART_PADDR),
+		.length		= UART_SIZE,
 		.type		= MT_DEVICE,
 	},
 #endif
diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
index 1b8bf0e..7f4f38b 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -25,8 +25,9 @@
  * address that is known to work.
  */
 #define UART0_PHYS		0xE0000000
-#define UART0_SIZE		SZ_4K
-#define UART0_VIRT		0xF0001000
+#define UART1_PHYS		0xE0001000
+#define UART_SIZE		SZ_4K
+#define UART_VIRT		0xF0001000
 
 #define TTC0_PHYS		0xF8001000
 #define TTC0_SIZE		SZ_4K
@@ -36,12 +37,17 @@
 #define SCU_PERIPH_SIZE		SZ_8K
 #define SCU_PERIPH_VIRT		(TTC0_VIRT - SCU_PERIPH_SIZE)
 
+#if IS_ENABLED(CONFIG_DEBUG_ZYNQ_UART1)
+#	define LL_UART_PADDR	UART1_PHYS
+#	define LL_UART_VADDR	UART_VIRT
+#else
+#	define LL_UART_PADDR	UART0_PHYS
+#	define LL_UART_VADDR	UART_VIRT
+#endif
+
 /* The following are intended for the devices that are mapped early */
 
 #define TTC0_BASE			IOMEM(TTC0_VIRT)
 #define SCU_PERIPH_BASE			IOMEM(SCU_PERIPH_VIRT)
 
-#define LL_UART_PADDR	UART0_PHYS
-#define LL_UART_VADDR	UART0_VIRT
-
 #endif

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] ARM: zynq: Allow UART1 to be used as DEBUG_LL console.

[Nick Bowler]

On 2012-10-29 10:56 -0600, Josh Cartwright wrote:
> On Thu, Oct 25, 2012 at 06:47:34PM -0400, Nick Bowler wrote:
> > The main UART on the Xilinx ZC702 board is UART1, located at address
> > e0001000.  Add a Kconfig option to select this device as the low-level
> > debugging port.  This allows the really early boot printouts to reach
> > the USB serial adaptor on this board.
> > 
> > For consistency's sake, add a choice entry for UART0 even though it is
> > the the default if UART1 is not selected.
> > 
> > As there are currently known issues related to the UART virtual
> > mappings, this is KNOWN BROKEN, not to be merged yet!
> > 
> > Not-Yet-Signed-off-by: Nick Bowler <nbowler@elliptictech.com>
> 
> Tested-by: Josh Cartwright <josh.cartwright@ni.com>
> 
> Now that v5 of the initial zynq cleanup patchset is queued up for
> merging (with a workaround for the uart mapping problem), what would it
> take for you to sign off on this patch?

Great, I've tested this on top of the other 4 and the boot console is
working now.  I will resend the patch with my signoff.

(I wonder if UART0 has similar address problems on the ZC702 if it is
selected as the boot console... but this is harder to test as AFAIK it's
not connected to anything on this board by default, but it could in
principle be connected to something in the PL.  We can cross that bridge
if and when we get to it, I guess).

> There is some trivial merging that has to be done to get it to apply
> cleanly on v5.  See a rebased version below.

Yup, looks good.

Cheers,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Re: [PATCH v4 5/5] zynq: move static peripheral mappings

[Josh Cartwright]

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

On Thu, Oct 25, 2012 at 06:41:08PM -0400, Nick Bowler wrote:
> On 2012-10-25 16:29 -0500, Josh Cartwright wrote:
> > On Thu, Oct 25, 2012 at 04:17:01PM -0400, Nick Bowler wrote:
> > > Did you test this on any real hardware?  I can't get the ZC702 to work
> > > with the UART mapped at this address (this ends up being mapped at
> > > 0xFEFFF000), although I can't for the life of me figure out why the
> > > virtual address even matters.  Note that for the ZC702, the physical
> > > address of the "main" UART is 0xE0001000.

Good news is you're not crazy; I was able to duplicate the problem here.

> If I were to guess, I would guess that, except for when it "Works",
> the really really early printk stuff isn't actually hitting the uart
> at all.  The "Fails" case would then be due to the stray writes
> crashing the board, and the "Truncated" case due to the stray writes
> being (ostensibly) benign.

If I'm not mistaken, this hypothesis is predicated on the early bootup
code establishing a (linear?) mapping for addresses > VMALLOC_START;
before the mdesc->map_io() is even handled.  That seems odd to me.

> But I really have no way right now to test this hypothesis, since I
> can't print anything in the failing case.

Not sure if I'll be able to get anything meaningful out of it yet (I've
not historically had good luck with Xilinx's debugging tools), but I did
finally get a JTAG debugger hooked up to the zc702.  I'll see if I can
get any useful information tomorrow.

Thanks,

  Josh

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH v4 5/5] zynq: move static peripheral mappings

[Michal Simek]

HI Josh and Nick,

look below.

> -----Original Message-----
> From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> Sent: Friday, October 26, 2012 3:03 AM
> To: Nick Bowler
> Cc: arm@kernel.org; Arnd Bergmann; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; John Linn; Michal Simek
> Subject: Re: [PATCH v4 5/5] zynq: move static peripheral mappings
> 
> On Thu, Oct 25, 2012 at 06:41:08PM -0400, Nick Bowler wrote:
> > On 2012-10-25 16:29 -0500, Josh Cartwright wrote:
> > > On Thu, Oct 25, 2012 at 04:17:01PM -0400, Nick Bowler wrote:
> > > > Did you test this on any real hardware?  I can't get the ZC702 to
> > > > work with the UART mapped at this address (this ends up being
> > > > mapped at 0xFEFFF000), although I can't for the life of me figure
> > > > out why the virtual address even matters.  Note that for the
> > > > ZC702, the physical address of the "main" UART is 0xE0001000.
> 
> Good news is you're not crazy; I was able to duplicate the problem here.
> 
> > If I were to guess, I would guess that, except for when it "Works",
> > the really really early printk stuff isn't actually hitting the uart
> > at all.  The "Fails" case would then be due to the stray writes
> > crashing the board, and the "Truncated" case due to the stray writes
> > being (ostensibly) benign.
> 
> If I'm not mistaken, this hypothesis is predicated on the early bootup code
> establishing a (linear?) mapping for addresses > VMALLOC_START; before the
> mdesc->map_io() is even handled.  That seems odd to me.
> 
> > But I really have no way right now to test this hypothesis, since I
> > can't print anything in the failing case.
> 
> Not sure if I'll be able to get anything meaningful out of it yet (I've not
> historically had good luck with Xilinx's debugging tools), but I did finally get a
> JTAG debugger hooked up to the zc702.  I'll see if I can get any useful
> information tomorrow.

I have seen the same problem on zc702. I will debug it.
Josh: the best will be if you can send v5 for patches 1-3 (1 with small changes in dts - uart)  which I will apply
to arm-next. 

4/5 should go out of zynq subtree, it means directly to arm-soc or via Russel's tree. 

5/5 + Nick patch should be tested. 

Thanks,
Michal

[PATCH v4 0/5] zynq subarch cleanups

[Josh Cartwright]

Michal-

Here is a v5 of the zynq cleanup patchset that addresses your feedback.  I've
intentionally left patches 4 and 5 in the set until we figure out the
appropriate way to get them in tree (feel free to just apply 1-3)

I've also moved the uart mapping in patch 5 to a known working address, until
we can work out what is happening there.  This should allow this patchset to be
applied and have the zc702 boot.

You had suggested removing/renaming the zynq-ep107.dts; it wasn't clear whether
you had wanted that in this patchset or not.  I'm going to assume not.  I'll
follow up with this, after this patchset is applied, if that works for you.

Thanks,
   Josh

---
Things have been relatively quiet on the Zynq front lately.  This patchset does
a bit of cleanup of the Zynq subarchitecture.  It was the necessary set of
things I had to do to get a zynq target booting with the upstream qemu model.

Patches 1 and 2 move zynq to use the GIC and pl310 L2 cache controller device
tree bindings respectively.

Patch 3 removes unused clock infrastructure.  The plan is to rework the
out-of-tree Xilinx generic clk support into something suitable for merging.
What's in tree now just isn't used at all, and can be removed.

Patch 4 and 5 move around the static peripheral mappings into the vmalloc area.

---
Changes since v4:
  - Fixed uart interrupt spec in zynq-ep107.dtb (patch 1)
  - Moved early uart mapping to a known working address (patch 5)

Changes since v3:
  - Patch 3 also removes the zynq "use" of versatile

Changes since v2:
  - Reordered patchset to prevent remapping peripherals that were subsequently
    removed from the static map
  - Use DT bindings for the L2 cache controller

Changes since v1:
  - Make sure arm@kernel.org was included
  - Rebased on arm-soc/for-next
  - Added a cover letter
  - Elaborated a bit on why I removed CLKDEV_LOOKUP

---
Josh Cartwright (5):
  zynq: use GIC device tree bindings
  zynq: use pl310 device tree bindings
  zynq: remove use of CLKDEV_LOOKUP
  ARM: annotate VMALLOC_END definition with _AC
  zynq: move static peripheral mappings

 arch/arm/Kconfig                           |  1 -
 arch/arm/Makefile                          |  1 -
 arch/arm/boot/dts/zynq-ep107.dts           | 19 ++++++++++++++----
 arch/arm/include/asm/pgtable.h             |  2 +-
 arch/arm/mach-zynq/common.c                | 23 ++++++++++-----------
 arch/arm/mach-zynq/include/mach/clkdev.h   | 32 ------------------------------
 arch/arm/mach-zynq/include/mach/zynq_soc.h | 31 ++++++++++++++---------------
 7 files changed, 41 insertions(+), 68 deletions(-)
 delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h

-- 
1.8.0

[PATCH v4 1/5] zynq: use GIC device tree bindings

[Josh Cartwright]

The Zynq uses the cortex-a9-gic.  This eliminates the need to hardcode
register addresses.

Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
Cc: John Linn <john.linn@xilinx.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/boot/dts/zynq-ep107.dts           | 10 ++++++----
 arch/arm/mach-zynq/common.c                |  7 ++++++-
 arch/arm/mach-zynq/include/mach/zynq_soc.h |  2 --
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq-ep107.dts
index 37ca192..f914090 100644
--- a/arch/arm/boot/dts/zynq-ep107.dts
+++ b/arch/arm/boot/dts/zynq-ep107.dts
@@ -36,16 +36,18 @@
 		ranges;
 
 		intc: interrupt-controller@f8f01000 {
+			compatible = "arm,cortex-a9-gic";
+			#interrupt-cells = <3>;
+			#address-cells = <1>;
 			interrupt-controller;
-			compatible = "arm,gic";
-			reg = <0xF8F01000 0x1000>;
-			#interrupt-cells = <2>;
+			reg = <0xF8F01000 0x1000>,
+			      <0xF8F00100 0x100>;
 		};
 
 		uart0: uart@e0000000 {
 			compatible = "xlnx,xuartps";
 			reg = <0xE0000000 0x1000>;
-			interrupts = <59 0>;
+			interrupts = <0 27 4>;
 			clock = <50000000>;
 		};
 	};
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index ab5cfdd..d73963b 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -55,12 +55,17 @@ static void __init xilinx_init_machine(void)
 	of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
 }
 
+static struct of_device_id irq_match[] __initdata = {
+	{ .compatible = "arm,cortex-a9-gic", .data = gic_of_init, },
+	{ }
+};
+
 /**
  * xilinx_irq_init() - Interrupt controller initialization for the GIC.
  */
 static void __init xilinx_irq_init(void)
 {
-	gic_init(0, 29, SCU_GIC_DIST_BASE, SCU_GIC_CPU_BASE);
+	of_irq_init(irq_match);
 }
 
 /* The minimum devices needed to be mapped before the VM system is up and
diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
index d0d3f8f..3d1c6a6 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -35,8 +35,6 @@
 
 #define TTC0_BASE			IOMEM(TTC0_VIRT)
 #define SCU_PERIPH_BASE			IOMEM(SCU_PERIPH_VIRT)
-#define SCU_GIC_CPU_BASE		(SCU_PERIPH_BASE + 0x100)
-#define SCU_GIC_DIST_BASE		(SCU_PERIPH_BASE + 0x1000)
 #define PL310_L2CC_BASE			IOMEM(PL310_L2CC_VIRT)
 
 /*
-- 
1.8.0

RE: [PATCH v4 1/5] zynq: use GIC device tree bindings

[Michal Simek]

> -----Original Message-----
> From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> Sent: Thursday, October 18, 2012 2:47 AM
> To: arm@kernel.org; Arnd Bergmann
> Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; John
> Linn; Nick Bowler; Michal Simek
> Subject: [PATCH v4 1/5] zynq: use GIC device tree bindings
> 
> The Zynq uses the cortex-a9-gic.  This eliminates the need to hardcode register
> addresses.
> 
> Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
> Cc: John Linn <john.linn@xilinx.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/boot/dts/zynq-ep107.dts           | 10 ++++++----
>  arch/arm/mach-zynq/common.c                |  7 ++++++-
>  arch/arm/mach-zynq/include/mach/zynq_soc.h |  2 --
>  3 files changed, 12 insertions(+), 7 deletions(-)
> 

Applied to http://git.xilinx.com/?p=linux-xlnx.git;a=shortlog;h=refs/heads/arm-next

Thanks,
Michal