[U-Boot] [PATCH] ARM: socfpga: Remove socfpga_sdram_apply_static_cfg()
marex at denx.de
Mon Apr 29 08:34:38 UTC 2019
On 4/26/19 8:39 AM, Simon Goldschmidt wrote:
> On Tue, Apr 23, 2019 at 6:14 PM Marek Vasut <marex at denx.de> wrote:
>> The usage of socfpga_sdram_apply_static_cfg() seems rather dubious and
>> is confirmed to lead to a rare system hang when enabling bridges. This
>> patch removes the socfpga_sdram_apply_static_cfg() altogether, because
>> it's use seems unjustified and problematic.
>> The socfpga_sdram_apply_static_cfg() triggers write to SDRAM staticcfg
>> register to set the applycfg bit, which according to old vendor U-Boot
>> sources can only be written when there is no traffic between the SDRAM
>> controller and the rest of the system. Empirical measurements confirm
>> this, setting the applycfg bit when there is traffic between the SDRAM
>> controller and CPU leads to the SDRAM controller accesses being blocked
>> shortly after.
>> Altera originally solved this by moving the entire code which sets the
>> staticcfg register to OCRAM . The commit message claims that the
>> applycfg bit needs to be set after write to fpgaportrst register. This
>> is however inverted by Altera shortly after in , where the order
>> becomes the exact opposite of what commit message  claims to be the
>> required order. The explanation points to a possible problem in AMP
>> use-case, where the FPGA might be sending transactions through the F2S
>> However, the AMP is only the tip of the iceberg here. Any of the other
>> L2, L3 or L4 masters can trigger transactions to the SDRAM. It becomes
>> rather non-trivial to guarantee there are no transactions to the SDRAM
>> The SoCFPGA SDRAM driver always writes the applycfg bit in SPL. Thus,
>> writing the applycfg again in bridge enable code seems redundant and
>> can presumably be dropped.
>>  https://github.com/altera-opensource/u-boot-socfpga/commit/75905816ec95b0ccd515700b922628d7aa9036f8
>>  https://github.com/altera-opensource/u-boot-socfpga/commit/8ba6986b04a91d23c7adf529186b34c8d2967ad5
>> Signed-off-by: Marek Vasut <marex at denx.de>
>> Cc: Chin Liang See <chin.liang.see at intel.com>
>> Cc: Dinh Nguyen <dinguyen at kernel.org>
>> Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
>> Cc: Tien Fong Chee <tien.fong.chee at intel.com>
> Good catch!
> Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
I am still hoping that Chin might jump in and explain the discrepancy
between those two patches in Altera U-Boot fork I linked above.
More information about the U-Boot