On Mon, Jan 28, 2019 at 8:25 PM Marek Vasut <ma...@denx.de> 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 <ma...@denx.de> wrote: > >>>> > >>>> On 1/28/19 12:49 PM, Simon Goldschmidt wrote: > >>>>> On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <ma...@denx.de> 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 <simon.k.r.goldschm...@gmail.com> > >>>>>>>>> --- > >>>>>>>>> > >>>>>>>>> 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). 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? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
