From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steffen Trumtrar Subject: Re: [PATCHv5 1/3] dts: socfpga: Add bindings for Altera SoC SDRAM controller Date: Tue, 20 May 2014 16:44:47 +0200 Message-ID: <20140520144446.GJ949@pengutronix.de> References: <1400169891-29546-1-git-send-email-tthayer@altera.com> <1400169891-29546-2-git-send-email-tthayer@altera.com> <20140516075344.GF949@pengutronix.de> <20140519191206.GH949@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-doc-owner@vger.kernel.org To: Alan Tull Cc: Thor Thayer , Thor Thayer , Rob Herring , pawel.moll@arm.com, Mark Rutland , ijc+devicetree@hellion.org.uk, Kumar Gala , Rob Landley , linux@arm.linux.org.uk, Dinh Nguyen , dougthompson@xmission.com, Grant Likely , Borislav Petkov , "devicetree@vger.kernel.org" , "linux-doc@vger.kernel.org" , linux-kernel , linux-arm-kernel@lists.infradead.org, linux-edac@vger.kernel.org, Alan Tull List-Id: devicetree@vger.kernel.org Hi! On Tue, May 20, 2014 at 09:31:06AM -0500, Alan Tull wrote: > On Mon, May 19, 2014 at 2:37 PM, Thor Thayer wrote: > > >>> >> diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-sdram.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-sdram.txt > >>> >> new file mode 100644 > >>> >> index 0000000..8f8746b > >>> >> --- /dev/null > >>> >> +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-sdram.txt > >>> >> @@ -0,0 +1,11 @@ > >>> >> +Altera SOCFPGA SDRAM Controller > >>> >> + > >>> >> +Required properties: > >>> >> +- compatible : "altr,sdr-ctl"; > >>> >> +- reg : Should contain 1 register ranges(address and length) > >>> >> + > >>> >> +Example: > >>> >> + sdrctl@ffc25000 { > >>> >> + compatible = "altr,sdr-ctl"; > >>> >> + reg = <0xffc25000 0x1000>; > >>> >> + }; > >>> >> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi > >>> >> index df43702..6ce912e 100644 > >>> >> --- a/arch/arm/boot/dts/socfpga.dtsi > >>> >> +++ b/arch/arm/boot/dts/socfpga.dtsi > >>> >> @@ -676,6 +676,11 @@ > >>> >> clocks = <&l4_sp_clk>; > >>> >> }; > >>> >> > >>> >> + sdrctl@ffc25000 { > >>> >> + compatible = "altr,sdr-ctl", "syscon"; > >>> > ^^^^^^^^^^ > >>> > > >>> > Get rid of that, too, please. > >>> > >>> Hi Steffan, > >>> > >>> I believe I need to keep the "syscon". The same register (ctrlcfg) > >>> that has the ECC enable bitfield also includes the DRAM configuration > >>> bitfields that other drivers will want to access (specifically the > >>> FPGA bridge needs this information). Since this register will be > >>> shared between drivers, syscon seems like the best solution. > >>> > >> > >> Hm, from looking at the documentation of the ctrlcfg I can't really > >> understand which bits you would need for the FPGA brigde and why. > > Hi Steffen, > > Offset 0x80 in the sdr-ctl is the "fpgaportrst" register. 14 bits > wide, defaults to 0. When appropriate bits set to 1 in that reg, it > allows an FPGA port to come out of reset (enables that port). Has no > other effect on SDRAM configuration. > > >> That all sounds like stuff you would want to set for the specific > >> RAM you are dealing with on a specific board. > >> What bridge are you talking about? The SDRAM bridge? > > Yes, the port allows the FPGA a direct path to the SDRAM. This one > register the only register in the sdr that the bridge driver needs. > So, what I suggested down ... > >> > >> I can see the problem with the ECC enable, though. > >> > >> Regards, > >> Steffen > >> > > > >>> > sdrctl@ffc25000 { > >>> > compatible = "altr,sdr-ctl"; > >>> > reg = <0xffc25000 0x1000>; > >>> > ranges; > >>> > > >>> > edac@ffc2502c { > >>> > compatible = "altr,sdram-edac"; > >>> > reg = <0xffc2502c 0x50>; > >>> > interrupts = <0 39 4>; > >>> > }; > >>> > }; > >>> > > >>> > Then we can later add: > >>> > > >>> > sdr-ports: ports@ffc2507c { > >>> > #reset-cells = <1>; > >>> > compatible = "altr,sdr-ports"; > >>> > reg = <0xffc2507c 0x10>; > >>> > clocks = <&ddr_dqs_clk>; > >>> > ... > >>> > }; > ... here should work. No?! Just a simple driver that registers with the reset-framework. I would add 0x7c to that driver and than that driver could "configure" the port and let it out of reset. I have done the same thing for the other 3 bridges, but am not finished yet. Especially the GPV-stuff needs to at least be able to be added later if not now. Regard, Steffen -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |