On 1/29/19 9:23 AM, Simon Goldschmidt wrote: > On Mon, Jan 28, 2019 at 8:25 PM Marek Vasut <[email protected]> wrote: >> >> On 1/28/19 8:17 PM, Simon Goldschmidt wrote: >>> Am 28.01.2019 um 20:02 schrieb Marek Vasut: >>>> On 1/28/19 1:38 PM, Simon Goldschmidt wrote: >>>>> On Mon, Jan 28, 2019 at 12:59 PM Marek Vasut <[email protected]> wrote: >>>>>> >>>>>> On 1/28/19 12:49 PM, Simon Goldschmidt wrote: >>>>>>> On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <[email protected]> wrote: >>>>>>>> >>>>>>>> On 1/27/19 9:47 AM, Simon Goldschmidt wrote: >>>>>>>>> Am 26.01.2019 um 09:58 schrieb Marek Vasut: >>>>>>>>>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote: >>>>>>>>>>> To clean up reset handling for socfpga gen5, let's move the >>>>>>>>>>> code snippet >>>>>>>>>>> taking the DDR controller out of reset from SPL to the DDR driver. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Simon Goldschmidt <[email protected]> >>>>>>>>>>> --- >>>>>>>>>>> >>>>>>>>>>> arch/arm/mach-socfpga/spl_gen5.c | 1 - >>>>>>>>>>> drivers/ddr/altera/sdram_gen5.c | 4 ++++ >>>>>>>>>>> 2 files changed, 4 insertions(+), 1 deletion(-) >>>>>>>>>>> >>>>>>>>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c >>>>>>>>>>> b/arch/arm/mach-socfpga/spl_gen5.c >>>>>>>>>>> index ccdc661d05..f9bea892b1 100644 >>>>>>>>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c >>>>>>>>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c >>>>>>>>>>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy) >>>>>>>>>>> socfpga_bridges_reset(1); >>>>>>>>>>> } >>>>>>>>>>> - socfpga_per_reset(SOCFPGA_RESET(SDR), 0); >>>>>>>>>>> socfpga_per_reset(SOCFPGA_RESET(UART0), 0); >>>>>>>>>>> socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0); >>>>>>>>>>> diff --git a/drivers/ddr/altera/sdram_gen5.c >>>>>>>>>>> b/drivers/ddr/altera/sdram_gen5.c >>>>>>>>>>> index 821060459c..bd54c420f8 100644 >>>>>>>>>>> --- a/drivers/ddr/altera/sdram_gen5.c >>>>>>>>>>> +++ b/drivers/ddr/altera/sdram_gen5.c >>>>>>>>>>> @@ -7,6 +7,7 @@ >>>>>>>>>>> #include <div64.h> >>>>>>>>>>> #include <watchdog.h> >>>>>>>>>>> #include <asm/arch/fpga_manager.h> >>>>>>>>>>> +#include <asm/arch/reset_manager.h> >>>>>>>>>>> #include <asm/arch/sdram.h> >>>>>>>>>>> #include <asm/arch/system_manager.h> >>>>>>>>>>> #include <asm/io.h> >>>>>>>>>>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int >>>>>>>>>>> sdr_phy_reg) >>>>>>>>>>> SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB; >>>>>>>>>>> int ret; >>>>>>>>>>> + /* release DDR from reset */ >>>>>>>>>>> + socfpga_per_reset(SOCFPGA_RESET(SDR), 0); >>>>>>>>>>> + >>>>>>>>>> >>>>>>>>>> Can the reset framework do this ? >>>>>>>>> >>>>>>>>> Hmm, it probably could, but I see that as a diferent patch. The >>>>>>>>> altera >>>>>>>>> DDR driver currently does not work with devicetree, so using the >>>>>>>>> reset >>>>>>>>> framework here would be a bigger patch, I think. >>>>>>>>> >>>>>>>>> Can we do that later and clean up this by just moving the code? >>>>>>>> >>>>>>>> How much effort is it to switch this driver over to DM ? >>>>>>> >>>>>>> I really don't know. Searching through drivers/ddr does not seem to >>>>>>> give me >>>>>>> an example of a DTS-enabled ddr driver. Given that, I'd rather just >>>>>>> push this >>>>>>> patch now. While it's true that it doesn't clean up everything, it's >>>>>>> not as if it >>>>>>> would make things worse. >>>>>> >>>>>> That's a valid point. >>>>>> >>>>>> I guess you can add DRAM uclass and just probe the driver, which should >>>>>> be all that's needed. >>>>> >>>>> Hmm, there *is* a UCLASS_RAM, but its drivers are in 'drivers/ram'. >>>>> Is there >>>>> any reason those are separated from 'drivers/ddr'? >>>> >>>> I don't think so, seems like these two directories should be merged. >>> >>> Yes, I think so too by now. >>> >>> I'll see if I can change this patch to use UCLASS_RAM. A patch/series to >>> merge drivers/ddr wih drivers/ram should be separate. >> >> Sounds good. > > It kind of works with UCLASS_RAM, but there's a problem with the devicetree: > it describes the SDR controller starting at 0xffc25000 where the official > memory > map from Intel says it starts at 0xffc20000, but describes its > registers starting > from offset 0x5000. However, in U-Boot, the file > 'drivers/ddr/altera/sequencer.c' > uses those lower addresses. > > So now I can either change the dts to let SDR registers start at 0xffc20000 > instead of 0xffc25000 (and in consequence adapt Linux, too) or add a new > driver > for the sequencer that occupies this lower range (and make sdram_gen5.c use > it somehow).
Fix the DT, I think the DT is buggy. The DRAM controller probably starts at 0xffc2_0000 and the various 4k chunks above 0xffc2_0000 are various subcomponents of the DDR controller. > Before I spin another round, what would be the preferred way to take? > > Anyway, I failed to find public documentation for this sequencer thing. Do you > happen to know why it's done like this? I don't have one, but I _think_ it's a HW instance of the altera DDR3 controller which you can also put into the FPGA, and that one is in quartus. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
