[U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver

[Marek Vasut]

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 <marex at denx.de> wrote:
>>>>
>>>> On 1/28/19 12:49 PM, Simon Goldschmidt wrote:
>>>>> On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex at 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.goldschmidt at 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.

-- 
Best regards,
Marek Vasut