[U-Boot] [PATCH v2 8/8] arm: socfpga: implement proper peripheral reset
Simon Goldschmidt
simon.k.r.goldschmidt at gmail.com
Tue Feb 26 19:53:04 UTC 2019
On 22.02.2019 22:14, Marek Vasut wrote:
> On 2/22/19 8:55 PM, Simon Goldschmidt wrote:
>>
>> Am Fr., 22. Feb. 2019, 20:47 hat Marek Vasut <marex at denx.de
>> <mailto: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>
>> > <mailto: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>>
>> > > <mailto: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>>
>> > > <mailto: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>>
>> > > <mailto: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.
> Well, it's SPL only, it's highly un-recommended to fiddle with the pin
> configuration at runtime. Although, I suspect that might be just altera
> being cautious.
Yeah, I would have guesses as long as we change the pin settings without
accessing the pins in between, we should probably be good...
>
>> 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...
> Right, having it in DT, we could have one SPL for multiple systems.
> Would be nice.
I'll try if I can test that. That would probably result in a pinctrl
driver that only supports one preset pin set...
>> > 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 ...
> Right, that's what I meant.
>
> btw you gonna be at EW next week ?
No, unfortunately not.
Regards,
Simon
More information about the U-Boot
mailing list