| Message ID | 20190221214332.4246-9-simon.k.r.goldschmidt@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | arm: socfpga: implement proper peripheral reset handling | expand |
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@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); > - > debug("Unfreezing/Thaw all I/O banks\n"); > /* unfreeze / thaw all IO banks */ > sys_mgr_frzctrl_thaw_req(); > Nice!
On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut <marex@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@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 > > - > > debug("Unfreezing/Thaw all I/O banks\n"); > > /* unfreeze / thaw all IO banks */ > > sys_mgr_frzctrl_thaw_req(); > > > > Nice! > > -- > Best regards, > Marek Vasut
On 2/22/19 7:35 AM, Simon Goldschmidt wrote: > On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut <marex@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@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 ?
Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@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@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@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 >
On 2/22/19 8:10 PM, Simon Goldschmidt wrote: > > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de > <mailto:marex@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@denx.de > <mailto:marex@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@gmail.com > <mailto:simon.k.r.goldschmidt@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 ?
Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut <marex@denx.de> geschrieben: > On 2/22/19 8:10 PM, Simon Goldschmidt wrote: > > > > > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de > > <mailto:marex@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@denx.de > > <mailto:marex@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@gmail.com > > <mailto:simon.k.r.goldschmidt@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. But that again would require more time than I currently have... :-( Regards, Simon
On 2/22/19 8:42 PM, Simon Goldschmidt wrote: > > > Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut <marex@denx.de > <mailto:marex@denx.de>> geschrieben: > > On 2/22/19 8:10 PM, Simon Goldschmidt wrote: > > > > > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de > <mailto:marex@denx.de> > > <mailto:marex@denx.de <mailto:marex@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@denx.de > <mailto:marex@denx.de> > > <mailto:marex@denx.de <mailto:marex@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@gmail.com > <mailto:simon.k.r.goldschmidt@gmail.com> > > <mailto:simon.k.r.goldschmidt@gmail.com > <mailto:simon.k.r.goldschmidt@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 ... > 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.
Am Fr., 22. Feb. 2019, 20:47 hat Marek Vasut <marex@denx.de> geschrieben: > On 2/22/19 8:42 PM, Simon Goldschmidt wrote: > > > > > > Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut <marex@denx.de > > <mailto:marex@denx.de>> geschrieben: > > > > On 2/22/19 8:10 PM, Simon Goldschmidt wrote: > > > > > > > > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de > > <mailto:marex@denx.de> > > > <mailto:marex@denx.de <mailto:marex@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@denx.de > > <mailto:marex@denx.de> > > > <mailto:marex@denx.de <mailto:marex@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@gmail.com > > <mailto:simon.k.r.goldschmidt@gmail.com> > > > <mailto:simon.k.r.goldschmidt@gmail.com > > <mailto:simon.k.r.goldschmidt@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
On 2/22/19 8:55 PM, Simon Goldschmidt wrote: > > > Am Fr., 22. Feb. 2019, 20:47 hat Marek Vasut <marex@denx.de > <mailto:marex@denx.de>> geschrieben: > > On 2/22/19 8:42 PM, Simon Goldschmidt wrote: > > > > > > Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut <marex@denx.de > <mailto:marex@denx.de> > > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: > > > > On 2/22/19 8:10 PM, Simon Goldschmidt wrote: > > > > > > > > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de > <mailto:marex@denx.de> > > <mailto:marex@denx.de <mailto:marex@denx.de>> > > > <mailto:marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@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@denx.de <mailto:marex@denx.de> > > <mailto:marex@denx.de <mailto:marex@denx.de>> > > > <mailto:marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@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@gmail.com > <mailto:simon.k.r.goldschmidt@gmail.com> > > <mailto:simon.k.r.goldschmidt@gmail.com > <mailto:simon.k.r.goldschmidt@gmail.com>> > > > <mailto:simon.k.r.goldschmidt@gmail.com > <mailto:simon.k.r.goldschmidt@gmail.com> > > <mailto:simon.k.r.goldschmidt@gmail.com > <mailto:simon.k.r.goldschmidt@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. > 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. > > 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 ?
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@denx.de >> <mailto:marex@denx.de>> geschrieben: >> >> On 2/22/19 8:42 PM, Simon Goldschmidt wrote: >> > >> > >> > Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut <marex@denx.de >> <mailto:marex@denx.de> >> > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: >> > >> > On 2/22/19 8:10 PM, Simon Goldschmidt wrote: >> > > >> > > >> > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de >> <mailto:marex@denx.de> >> > <mailto:marex@denx.de <mailto:marex@denx.de>> >> > > <mailto:marex@denx.de <mailto:marex@denx.de> >> <mailto:marex@denx.de <mailto:marex@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@denx.de <mailto:marex@denx.de> >> > <mailto:marex@denx.de <mailto:marex@denx.de>> >> > > <mailto:marex@denx.de <mailto:marex@denx.de> >> <mailto:marex@denx.de <mailto:marex@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@gmail.com >> <mailto:simon.k.r.goldschmidt@gmail.com> >> > <mailto:simon.k.r.goldschmidt@gmail.com >> <mailto:simon.k.r.goldschmidt@gmail.com>> >> > > <mailto:simon.k.r.goldschmidt@gmail.com >> <mailto:simon.k.r.goldschmidt@gmail.com> >> > <mailto:simon.k.r.goldschmidt@gmail.com >> <mailto:simon.k.r.goldschmidt@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
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); - debug("Unfreezing/Thaw all I/O banks\n"); /* unfreeze / thaw all IO banks */ sys_mgr_frzctrl_thaw_req();
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@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(-)