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

[Simon Goldschmidt]

Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <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> 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>
> >>> ---
> >>>
> >>> 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.

I'll set that in my list *g*

Regards,
Simon

>