[U-Boot] [PATCH v2 8/8] arm: socfpga: implement proper peripheral reset

[Marek Vasut]

On 2/22/19 8:10 PM, Simon Goldschmidt wrote:
> 
> 
> Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex at denx.de
> <mailto:marex at denx.de>> geschrieben:
> 
>     On 2/22/19 7:35 AM, Simon Goldschmidt wrote:
>     > On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut <marex at denx.de
>     <mailto:marex at denx.de>> wrote:
>     >>
>     >> On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
>     >>> This commit removes ad-hoc reset handling for peripheral resets
>     from SPL
>     >>> for socfpga gen5.
>     >>>
>     >>> This is done because as U-Boot drivers support reset handling by
>     now.
>     >>>
>     >>> Signed-off-by: Simon Goldschmidt
>     <simon.k.r.goldschmidt at gmail.com
>     <mailto:simon.k.r.goldschmidt at gmail.com>>
>     >>> ---
>     >>>
>     >>> Changes in v2:
>     >>> - removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since
>     compatibility
>     >>>   now uses an environment variable
>     >>>
>     >>>  arch/arm/mach-socfpga/misc_gen5.c | 10 ----------
>     >>>  arch/arm/mach-socfpga/spl_gen5.c  | 10 ----------
>     >>>  2 files changed, 20 deletions(-)
>     >>>
>     >>> diff --git a/arch/arm/mach-socfpga/misc_gen5.c
>     b/arch/arm/mach-socfpga/misc_gen5.c
>     >>> index 6e11ba6cb2..9865f5b5b1 100644
>     >>> --- a/arch/arm/mach-socfpga/misc_gen5.c
>     >>> +++ b/arch/arm/mach-socfpga/misc_gen5.c
>     >>> @@ -201,16 +201,6 @@ int arch_early_init_r(void)
>     >>>       /* Add device descriptor to FPGA device table */
>     >>>       socfpga_fpga_add(&altera_fpga[0]);
>     >>>
>     >>> -#ifdef CONFIG_DESIGNWARE_SPI
>     >>> -     /* Get Designware SPI controller out of reset */
>     >>> -     socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0);
>     >>> -     socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0);
>     >>> -#endif
>     >>> -
>     >>> -#ifdef CONFIG_NAND_DENALI
>     >>> -     socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
>     >>> -#endif
>     >>> -
>     >>>       return 0;
>     >>>  }
>     >>>
>     >>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>     b/arch/arm/mach-socfpga/spl_gen5.c
>     >>> index 1bff8cbfcf..e1e65261eb 100644
>     >>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>     >>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>     >>> @@ -36,16 +36,12 @@ u32 spl_boot_device(void)
>     >>>               return BOOT_DEVICE_RAM;
>     >>>       case 0x2:       /* NAND Flash (1.8V) */
>     >>>       case 0x3:       /* NAND Flash (3.0V) */
>     >>> -             socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
>     >>>               return BOOT_DEVICE_NAND;
>     >>>       case 0x4:       /* SD/MMC External Transceiver (1.8V) */
>     >>>       case 0x5:       /* SD/MMC Internal Transceiver (3.0V) */
>     >>> -             socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0);
>     >>> -             socfpga_per_reset(SOCFPGA_RESET(DMA), 0);
>     >>>               return BOOT_DEVICE_MMC1;
>     >>>       case 0x6:       /* QSPI Flash (1.8V) */
>     >>>       case 0x7:       /* QSPI Flash (3.0V) */
>     >>> -             socfpga_per_reset(SOCFPGA_RESET(QSPI), 0);
>     >>>               return BOOT_DEVICE_SPI;
>     >>>       default:
>     >>>               printf("Invalid boot device (bsel=%08x)!\n", bsel);
>     >>> @@ -99,9 +95,7 @@ void board_init_f(ulong dummy)
>     >>>               socfpga_bridges_reset(1);
>     >>>       }
>     >>>
>     >>> -     socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
>     >>>       socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>     >>> -
>     >>>       timer_init();
>     >>>
>     >>>       debug("Reconfigure Clock Manager\n");
>     >>> @@ -123,10 +117,6 @@ void board_init_f(ulong dummy)
>     >>>       sysmgr_pinmux_init();
>     >>>       sysmgr_config_warmrstcfgio(0);
>     >>>
>     >>> -     /* De-assert reset for peripherals and bridges based on
>     handoff */
>     >>> -     reset_deassert_peripherals_handoff();
>     >>> -     socfpga_bridges_reset(0);
>     >
>     > I just saw that this line might have unwanted side effects in that
>     it does
>     > not write the enabled bridges bitmask to the 'iswgrp_handoff'
>     registers.
>     > So the 'bridge enable' command might not work.
>     >
>     > This whole handoff thing looks somewhat broken to me anyway: the
>     > original Altera U-Boot did it because it obeyed Quartus output,
>     while we
>     > now just always write a constant bitmask here (see
>     socfpga_bridges_reset()).
>     >
>     > Regards,
>     > Simon
> 
>     Might make sense to finally clean it up ?
> 
> 
> Yeah, well the problem is that for some things, handoff from Quartus is
> required while for other things, devicetree information like it is now
> is enough.

Might be just about the right time to convert quartus handoff files to DT ?

-- 
Best regards,
Marek Vasut