[U-Boot] ARM SOCFPGA: add resetmgr command so reset can be deasserted in bootcmd (for example on peripheral dma interfaces after fpga has been programmed).

---
 arch/arm/mach-socfpga/reset_manager_gen5.c | 31 +++++++++++++++++++++++++++++
+
 1 file changed, 31 insertions(+)

Hi Frank,

Thanks for the patch. Just a few notes:

Please reformat your patch to a commit header and commit message. For
example, this patch should be like this:

arm: socfpga: add resetmgr command

Add resetmgr command so reset can be deasserted in bootcmd (for example
on peripheral dma interfaces after fpga has been programmed).

Now on to the patch itself. I don't think this patch is needed and is
probably not the approach you want. It should be up to each driver to
de-assert the resets that it needs. Do it manually is probably not the
approach you want.

Dinh

On 12/14/2017 03:49 PM, Frank Mori Hess wrote:
> ---
>  arch/arm/mach-socfpga/reset_manager_gen5.c | 31 +++++++++++++++++++++++++++++
> +
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/arm/mach-socfpga/reset_manager_gen5.c b/arch/arm/mach-
> socfpga/reset_manager_gen5.c
> index aa88adb414..6ad5d2a362 100644
> --- a/arch/arm/mach-socfpga/reset_manager_gen5.c
> +++ b/arch/arm/mach-socfpga/reset_manager_gen5.c
> @@ -114,3 +114,34 @@ void socfpga_bridges_reset(int enable)
>  	return;
>  }
>  #endif
> +
> +int resetmgr_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	unsigned long bank;
> +	unsigned long offset;
> +	unsigned long assert;
> +
> +	if (argc != 4)
> +		return CMD_RET_USAGE;
> +
> +	bank = simple_strtoul(argv[1], NULL, 0);
> +	offset = simple_strtoul(argv[2], NULL, 0);
> +	assert = simple_strtoul(argv[3], NULL, 0);
> +	socfpga_per_reset(RSTMGR_DEFINE(bank, offset), assert);
> +	return 0;
> +}
> +
> +U_BOOT_CMD(
> +	resetmgr, 4, 1, resetmgr_cmd,
> +	"SoCFPGA HPS reset manager control",
> +	"resetmgr bank offset assert\n"
> +	"    bank - Bank of reset to assert/deassert.\n"
> +	"        0 ... mpumodrst\n"
> +	"        1 ... permodrst\n"
> +	"        2 ... per2modrst\n"
> +	"        3 ... brgmodrst\n"
> +	"        4 ... miscmodrst\n"
> +	"    offset - Offset of reset to assert/deassert.\n"
> +	"    assert - 1 to assert reset, 0 to deassert.\n"
> +	""
> +);
>

Marek Vasut wrote:
> Please always CC the list. Do NOT top-post.

You do realize I was replying to an email you sent to my personal address and 
you didn't even send to the list?
 
> What is your goal here ?

To put things in context, my larger goal is to update u-boot from the old 
version altera integrates into Quartus to a reasonably recent version of 
mainline u-boot being provided by the distro we are using.  My naive hope was 
the new version of u-boot would work at least as well as the old altera one.  
Experience so far:  infinite reboot loop due to broken cadence driver:

https://lists.denx.de/pipermail/u-boot/2017-December/313470.html

No response except from author responsible for breaking the driver insisting 
his changes be kept.

And now: DMA peripheral requests for FPGA are non-functional due to mainline 
u-boot ignoring the reset_config.h in the handoff files generated by Quartus.  
Apparently, the mainline uboot position is that it is inappropriate to provide 
any more support for initializing the resets than providing the ability to 
write to memory addresses with "mw".
 
> 
> Going back to my initial question -- what is your usecase and your aim
> here ? Usually you use FPGA manager in Linux to load the FPGA.
> >> 
> >> But you can really just do mw to the correct address or create a U-Boot
> >> script , so this command is not really needed, is it ?
> 

Ok, I am running Linux on the board.  I don't see how it would help to load 
the FPGA from Linux rather than u-boot.  The dma peripheral requests would 
still be just as disabled.  The only difference would be that I would be 
forced to deassert the resets from Linux rather than u-boot, which I guess 
would make it not your problem?  In principle, the fpga manager provides a 
write_complete hook that the socfpga fpga manager could use to deassert resets 
(perhaps based on device tree settings), but looking at my 4.1.33 fpga/
socfpga.c it doesn't seem to.

On 12/16/2017 06:03 PM, Frank Mori Hess wrote:
> Marek Vasut wrote:
>> Please always CC the list. Do NOT top-post.
> 
> You do realize I was replying to an email you sent to my personal address and 
> you didn't even send to the list?

The reply was To: to the u-boot ML, CC others, check the headers.

>> What is your goal here ?
> 
> To put things in context, my larger goal is to update u-boot from the old 
> version altera integrates into Quartus to a reasonably recent version of 
> mainline u-boot being provided by the distro we are using.  My naive hope was 
> the new version of u-boot would work at least as well as the old altera one.  
> Experience so far:  infinite reboot loop due to broken cadence driver:
> 
> https://lists.denx.de/pipermail/u-boot/2017-December/313470.html
> 
> No response except from author responsible for breaking the driver insisting 
> his changes be kept.

You need to figure out how to make it work for both platforms. Propose
some sort of patch to Vignesh and ask him to test it, I think he'd be
more than willing to ... CC me if you get stuck.

> And now: DMA peripheral requests for FPGA are non-functional due to mainline 
> u-boot ignoring the reset_config.h in the handoff files generated by Quartus.  
> Apparently, the mainline uboot position is that it is inappropriate to provide 
> any more support for initializing the resets than providing the ability to 
> write to memory addresses with "mw".

So far, it seems to be that mw is really all you need.
Do you have an argument for why mw is not sufficient ?

>> Going back to my initial question -- what is your usecase and your aim
>> here ? Usually you use FPGA manager in Linux to load the FPGA.
>>>>
>>>> But you can really just do mw to the correct address or create a U-Boot
>>>> script , so this command is not really needed, is it ?
>>
> 
> Ok, I am running Linux on the board.  I don't see how it would help to load 
> the FPGA from Linux rather than u-boot.  The dma peripheral requests would 
> still be just as disabled.  The only difference would be that I would be 
> forced to deassert the resets from Linux rather than u-boot, which I guess 
> would make it not your problem?

You might want to tone down this hostile attitude, it could be that this
is the same thing which triggered Vignesh in the QSPI NOR thread ...

> In principle, the fpga manager provides a 
> write_complete hook that the socfpga fpga manager could use to deassert resets 
> (perhaps based on device tree settings), but looking at my 4.1.33 fpga/
> socfpga.c it doesn't seem to.  

Oh, so you're using some ancient hacked-up vendor kernel, not mainline
Linux. If you model your hardware correctly in DT or whatever you use
and your drivers (Linux or U-Boot) are correct, then the DMA resets are
also toggled correctly and you won't need to start hacking in all these
special commands. That's how things should be done ...