From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dalon L Westergreen Date: Mon, 16 Mar 2020 12:55:48 -0700 Subject: [U-Boot] [PATCH] ARM: socfpga: Remove socfpga_sdram_apply_static_cfg() In-Reply-To: References: <5a684d24-0e60-bed3-a7c0-d53fcd54b7e4@denx.de> <7e87eb65-6fcc-d9a5-e36b-193f5bba9e3d@denx.de> <061dc254-934d-53d5-1446-cdcf16a85e5e@denx.de> <515836a3-d276-da15-616b-679ad87408ff@denx.de> <31d45968-9187-bbfd-7592-c45dd2a64e8b@denx.de> <093bd6c18dc96cd2d2cbf7d19c20edac98f2a7ec.camel@linux.intel.com> <2c711e5d-bad7-9c72-043b-ec2abc9f0f4d@gmail.com> Message-ID: <540289ba53eb7127f5242cfb7e94a1a3a2879c7f.camel@linux.intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Mon, 2020-03-16 at 20:06 +0100, Marek Vasut wrote: > On 3/16/20 8:04 PM, Simon Goldschmidt wrote: > > Am 16.03.2020 um 19:04 schrieb Dalon L Westergreen: > > > > > > On Thu, 2020-03-12 at 16:57 +0100, Marek Vasut wrote: > > > > On 3/12/20 4:54 PM, Dalon L Westergreen wrote: > > > > [...] > > > > > > > > (thanks for fixing your mailer :)) > > > > > > > > > > > > > > The problem was that this was causing weird sporadic hangs > > > > > > > > > > on > > > > > > > > > > Gen5, > > > > > > > > > > which is why it was removed. So until there is an > > > > > > > > > > explanation for > > > > > > > > > > those > > > > > > > > > > hangs, I'm not really OK with this. > > > > > > > > > > > > > > > > > > These sporadic hangs you saw were on devices without an FPGA > > > > > > > > > image > > > > > > > > > directly > > > > > > > > > accessing the SDRAM ports, right? > > > > > > > > > > > > > > > > Yes > > > > > > > > > > > > > > > > > > Maybe the application of static config should happen in SPL, > > > > > > > > > > before > > > > > > > > > > the > > > > > > > > > > DRAM is running, or something like that ? > > > > > > > > > > > > > > > > > > Thinking this further, limiting it to applying in SPL is not a > > > > > > > > > good > > > > > > > > > idea > > > > > > > > > since > > > > > > > > > that prevents us from implementing different FPGA > > > > > > > > > images/configs by > > > > > > > > > loading a > > > > > > > > > config later in the boot (i.e. in U-Boot from a FIT-image). > > > > > > > > > > > > > > > > Well, but later we have SDRAM running and we cannot make it go > > > > > > > > idle, > > > > > > > > can > > > > > > > > we? > > > > > > > > > > > > > > > > > > Unfortunately the sdram cfg apply must occur AFTER the fpga is > > > > > configured. This > > > > > is because the FPGA drives configuration bits, around the interfaces > > > > > datawidth > > > > > for example, that are used in setting up the dram interface. I > > > > > believe the > > > > > intent is for the command to only run when the ridge enable function > > > > > is > > > > > called. > > > > > > > > So that's one part of the fix -- only run this apply_static_cfg if the > > > > bitstream uses the F2S bridge. > > > > > > actually, the restriction is to apply this only if the FPGA is configured, > > > whether the bridge is used is irrelevant. you can further restrict this > > > to only when the bridge is used, but to me that means devicetree entries > > > or something to determine whether it is used. > > > > In my opinion, we need an FPGA-specific devicetree (or something > > similar) to describe the FPGA image, anyway. > > Like a DTO ? DTOs are how i suggest solving this in linux, so i would assume a dto would be best here too. > > > Today, too much > > configuration is applied at compile time (or when programming SPL to > > flash at latest) - there's currently no way to switch peripherals to > > FPGA for one image but not for another, for example. > > Yes > > > Worse: if you enable bridges but there's no slave attached, the CPU can > > be stuck. That would need to be described with the FPGA image as well. > > Can you propose a patch ? >