On Jum, 2017-07-28 at 09:52 +0200, Marek Vasut wrote:
On 07/28/2017 07:16 AM, Chee, Tien Fong wrote:
On Kha, 2017-07-27 at 11:37 +0200, Marek Vasut wrote:
On 07/27/2017 11:21 AM, Chee, Tien Fong wrote:
On Kha, 2017-07-27 at 10:21 +0200, Marek Vasut wrote:
On 07/27/2017 06:36 AM, tien.fong.chee@intel.com wrote:
From: Tien Fong Chee tien.fong.chee@intel.com
Remove unused FPGA feature for saving some space in gen5 SPL.
I really dislike the ifdefery. Why is this patch even needed? I thought we had no space problems in the Gen5 SPL?
I can remove those codes within ifdefery because they are no longer required.
Why ?
because those functions are testing on FPGA when the bridge is enabled in SPL.
That's what those functions do, it doesn't answer my question why they're no longer needed.
It is because fpga is disabled in SPL, hence those fpga checking routine can be removed. For example init sdram mmr running before calling command "enable fpga", then we can safely remove pgamgr_test_fpga_ready(). But, drivers are shared between SPL and U-boot, we still need minimal ifdefery in socfpga_bridges_reset.
But, i will keep minimal ifdefery on socfpga_bridges_reset to indicate the fpga bridges should not be released in SPL.
I keep them here just for one day we can use if we need to. Do you remember we have consent to clean up those useless codes for SPL, all fpga related drivers will be moved into one driver location.
No, sorry.
Are you disagree on keeping the ifdefery codes, or disagree on moving all FPGA related functions into drivers/fpga/... ?
I dislike the ifdeffery.
DO you have any better idea on this common function shared between SPL and U-boot?
Signed-off-by: Tien Fong Chee tien.fong.chee@intel.com
arch/arm/mach-socfpga/reset_manager_gen5.c | 9 +++++++++ arch/arm/mach-socfpga/system_manager_gen5.c | 6 ------ drivers/ddr/altera/sdram.c | 8 +++++++- 3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-socfpga/reset_manager_gen5.c b/arch/arm/mach-socfpga/reset_manager_gen5.c index aa88adb..c954063 100644 --- a/arch/arm/mach-socfpga/reset_manager_gen5.c +++ b/arch/arm/mach-socfpga/reset_manager_gen5.c @@ -97,6 +97,14 @@ void socfpga_bridges_reset(int enable) writel(0, &sysmgr_regs- >iswgrp_handoff[0]); writel(l3mask, &sysmgr_regs- > > iswgrp_handoff[1]); +/*
- FPGA feature is not required in SPL, so FPGA bridges
release from reset
- should not be supported in SPL, but some FPGA releated
setting can be stored
- in handoff registers as SPL to U-boot/OS handoff
information. These
- handoff information can be used in later phase such as
feeding handoff
- information to bridge command when user want to enable
FPGA feature in U-boot
- */
+#ifndef CONFIG_SPL_BUILD /* Check signal from FPGA. */ if (!fpgamgr_test_fpga_ready()) { /* FPGA not ready, do nothing. We allow system to boot @@ -110,6 +118,7 @@ void socfpga_bridges_reset(int enable) /* Remap the bridges into memory map */ writel(l3mask, SOCFPGA_L3REGS_ADDRESS);
I believe the L3REGS needs to be programmed on Gen5.
Yes, this is done when calling command "bridge enable". iswgrp_handoff[1] contains l3mask, which will be written into SOCFPGA_L3REGS_ADDRESS.
I think this needs to be done earlier, otherwise the first 1 MiB or so of DRAM isn't accessible.
Yeah, you are right....this is what i missing, the OCRAM should be remap to lowest memory region 1 MiB. So i propose just to remap OCRAM, and FPGA related bridges can be remaped in U-boot by calling the "enable bridge" command.
So basically the user would need to run this random command to fix his obscure memory issues with the first 1 MiB ? Unacceptable, U-Boot does this and this behavior will be retained. There's no way such a change of behavior can be let in.
The only remap first 1 MiB required to L3 is OCRAM. Hence, i propose we only keep OCRAM remap at here. Any FPGA related bridges visibility to L3 master can be set visible on L3 memory map when we require accessing to FPGA or calling "enable bridge".
Chin Liang, Could you help to advice on these?
[...]