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

Simon Goldschmidt simon.k.r.goldschmidt at gmail.com
Fri Feb 22 19:55:43 UTC 2019


Am Fr., 22. Feb. 2019, 20:47 hat Marek Vasut <marex at denx.de> geschrieben:

> On 2/22/19 8:42 PM, Simon Goldschmidt wrote:
> >
> >
> > Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut <marex at denx.de
> > <mailto:marex at denx.de>> geschrieben:
> >
> >     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>
> >     > <mailto: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>
> >     >     <mailto: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>
> >     >     <mailto: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 ?
> >
> >
> > That would be great: I do have a use case where the pin description
> > should be changeable after SPL.
>
> Well, we don't have the pin control documentation, so unless we
> "observe" what quartus generates real hard ...
>

Sadly, yes. However, I would think it would still be better to embed those
u32 arrays we have now into the device tree. That would give us the
possibility to apply them from SPL, U-Boot or even Linux.

My use case is that the pin settings change depending on the FPGA image
(e.g. hps pins routed to FPGA etc.). With the current code, I have to
update SPL to match the FPGA...


> > But that again would require more time than I currently have... :-(
>
> Right, I'm not asking you to do it to get this series in.
>

I didn't mean to say that. But I'd like to have that better sooner than
later. Only I can't find the time to do it ...

Regards,
Simon


More information about the U-Boot mailing list