[U-Boot] [PATCH] ARM: socfpga: Remove socfpga_sdram_apply_static_cfg()

[Marek Vasut]

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 [1]. 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 [2], where the order
>> becomes the exact opposite of what commit message [1] 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
>> bridge.
>>
>> 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
>> controller.
>>
>> 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.
>>
>> [1] https://github.com/altera-opensource/u-boot-socfpga/commit/75905816ec95b0ccd515700b922628d7aa9036f8
>> [2] 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.

-- 
Best regards,
Marek Vasut