| Message ID | 20121024200310.GB6713@beefymiracle.amer.corp.natinst.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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
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
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
> -----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
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>; ... }; }; ð0 { phy@7 { compatible = "marvell,888e1116r"; ... reg = <0x7>; }; }; Thoughts? Thanks, Josh
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>; > ... > }; > }; > ð0 { > 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
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
> -----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
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) /*