[PATCH v6 03/17] arm: mach-k3: Add basic support for J784S4 SoC definition

[Bryan Brattlof]

On December  7, 2023 thus sayeth Neha Malcom Francis:
> Hi Apurva,
> 
> On 06/12/23 18:07, Apurva Nandan wrote:
> > Add J784S4 initialization files for initial SPL boot.
> > 
> > Signed-off-by: Hari Nagalla <hnagalla at ti.com>
> > [ add firewall configurations and change the R5 MCU scratchpad ]
> > Signed-off-by: Manorit Chawdhry <m-chawdhry at ti.com>
> > Signed-off-by: Dasnavis Sabiya <sabiya.d at ti.com>
> > Signed-off-by: Apurva Nandan <a-nandan at ti.com>
> > ---
> >   arch/arm/mach-k3/Kconfig                      |  16 +-
> >   arch/arm/mach-k3/Makefile                     |   2 +
> >   arch/arm/mach-k3/arm64-mmu.c                  |  52 +++
> >   arch/arm/mach-k3/include/mach/hardware.h      |   4 +
> >   .../mach-k3/include/mach/j784s4_hardware.h    |  60 ++++
> >   arch/arm/mach-k3/include/mach/j784s4_spl.h    |  47 +++
> >   arch/arm/mach-k3/include/mach/spl.h           |   4 +
> >   arch/arm/mach-k3/j784s4_fdt.c                 |  15 +
> >   arch/arm/mach-k3/j784s4_init.c                | 335 ++++++++++++++++++
> >   arch/arm/mach-k3/r5/Makefile                  |   1 +
> >   10 files changed, 529 insertions(+), 7 deletions(-)
> >   create mode 100644 arch/arm/mach-k3/include/mach/j784s4_hardware.h
> >   create mode 100644 arch/arm/mach-k3/include/mach/j784s4_spl.h
> >   create mode 100644 arch/arm/mach-k3/j784s4_fdt.c
> >   create mode 100644 arch/arm/mach-k3/j784s4_init.c
> > 

...

> > +
> > +void k3_mmc_stop_clock(void)
> > +{
> > +	if (IS_ENABLED(CONFIG_K3_LOAD_SYSFW)) {
> > +		if (spl_boot_device() == BOOT_DEVICE_MMC1) {
> > +			struct mmc *mmc = find_mmc_device(0);
> > +
> > +			if (!mmc)
> > +				return;
> > +
> > +			mmc->saved_clock = mmc->clock;
> > +			mmc_set_clock(mmc, 0, true);
> > +		}
> > +	}
> > +}
> > +
> > +void k3_mmc_restart_clock(void)
> > +{
> > +	if (IS_ENABLED(CONFIG_K3_LOAD_SYSFW)) {
> > +		if (spl_boot_device() == BOOT_DEVICE_MMC1) {
> > +			struct mmc *mmc = find_mmc_device(0);
> > +
> > +			if (!mmc)
> > +				return;
> > +
> > +			mmc_set_clock(mmc, mmc->saved_clock, false);
> > +		}
> > +	}
> > +}

...

> > +void board_init_f(ulong dummy)
> > +{
> > +	struct udevice *dev;
> > +	int ret, ctr = 1;
> > +
> > +	/*
> > +	 * Cannot delay this further as there is a chance that
> > +	 * K3_BOOT_PARAM_TABLE_INDEX can be over written by SPL MALLOC section.
> > +	 */
> > +	store_boot_info_from_rom();
> > +
> > +	/* Make all control module registers accessible */
> > +	ctrl_mmr_unlock();
> > +
> > +	if (IS_ENABLED(CONFIG_CPU_V7R)) {
> > +		disable_linefill_optimization();
> > +		setup_k3_mpu_regions();
> > +	}
> > +
> > +	/* Init DM early */
> > +	ret = spl_early_init();
> > +
> > +	/* Prepare console output */
> > +	preloader_console_init();
> > +
> > +	if (IS_ENABLED(CONFIG_K3_LOAD_SYSFW)) {
> > +		/*
> > +		 * Process pinctrl for the serial0 a.k.a. WKUP_UART0 module and continue
> > +		 * regardless of the result of pinctrl. Do this without probing the
> > +		 * device, but instead by searching the device that would request the
> > +		 * given sequence number if probed. The UART will be used by the system
> > +		 * firmware (SYSFW) image for various purposes and SYSFW depends on us
> > +		 * to initialize its pin settings.
> > +		 */
> > +		ret = uclass_find_device_by_seq(UCLASS_SERIAL, 0, &dev);
> > +		if (!ret)
> > +			pinctrl_select_state(dev, "default");
> > +
> > +		/*
> > +		 * Load, start up, and configure system controller firmware. Provide
> > +		 * the U-Boot console init function to the SYSFW post-PM configuration
> > +		 * callback hook, effectively switching on (or over) the console
> > +		 * output.
> > +		 */
> > +		k3_sysfw_loader(is_rom_loaded_sysfw(&bootdata),
> > +				k3_mmc_stop_clock, k3_mmc_restart_clock);
> > +
> 
> All these K3 specific code, should we push it out to a separate function
> like J721S2? (commit fca27ee8b993bd1a5752fc2156b1e86358fa8041 "arch:
> mach-k3: Update board specific API name to K3 generic API name")
> 
> Else we can take this up in a separate clean up series.
>

Do we still have a need to support the split binary (sysfw.itb) boot
anymore now that binman is building tiboot3 for us? I don't think we 
will ever be in a situation where ROM hasn't loaded the firmware for us.

> > +#ifdef CONFIG_SPL_OF_LIST
> > +		if (IS_ENABLED(CONFIG_TI_I2C_BOARD_DETECT))
> > +			do_board_detect();
> > +#endif
> > +

Should we tie CONFIG_SPL_OF_LIST and CONFIG_TI_I2C_BOARD_DETECT together 
in Kconfig to avoid this?

> > +		if (IS_ENABLED(CONFIG_SPL_CLK_K3)) {
> > +			/*
> > +			 * Force probe of clk_k3 driver here to ensure basic default clock
> > +			 * configuration is always done for enabling PM services.
> > +			 */
> > +			ret = uclass_get_device_by_driver(UCLASS_CLK,
> > +							  DM_DRIVER_GET(ti_clk),
> > +							  &dev);
> > +			if (ret)
> > +				panic("Failed to initialize clk-k3!\n");
> > +		}
> > +
> > +		remove_fwl_configs(cbass_hc_cfg0_fwls, ARRAY_SIZE(cbass_hc_cfg0_fwls));
> > +		remove_fwl_configs(cbass_hc2_fwls, ARRAY_SIZE(cbass_hc2_fwls));
> > +		remove_fwl_configs(cbass_rc_cfg0_fwls, ARRAY_SIZE(cbass_rc_cfg0_fwls));
> > +		remove_fwl_configs(infra_cbass0_fwls, ARRAY_SIZE(infra_cbass0_fwls));
> > +		remove_fwl_configs(mcu_cbass0_fwls, ARRAY_SIZE(mcu_cbass0_fwls));
> > +		remove_fwl_configs(wkup_cbass0_fwls, ARRAY_SIZE(wkup_cbass0_fwls));
> > +		remove_fwl_configs(navss_cbass0_fwls, ARRAY_SIZE(navss_cbass0_fwls));
> > +	}
> > +
> > +	/* Output System Firmware version info */
> > +	k3_sysfw_print_ver();
> > +
> > +	if (IS_ENABLED(CONFIG_TARGET_J784S4_R5_EVM)) {
> > +		ret = uclass_get_device_by_name(UCLASS_MISC, "msmc", &dev);
> > +		if (ret)
> > +			panic("Probe of msmc failed: %d\n", ret);
> > +
> > +		ret = uclass_get_device(UCLASS_RAM, 0, &dev);
> > +		if (ret)
> > +			panic("DRAM 0 init failed: %d\n", ret);
> > +
> > +		while (dev) {
> > +			ret = uclass_next_device_err(&dev);
> > +			if (ret) {
> > +				printf("Initialized %d DRAM controllers\n", ctr);
> > +				break;
> > +			}
> > +			ctr++;

Nice! However I'm worried we miss events with controllers that we wanted 
and failed to probe. Should we invert this check?

The uclass_foreach_dev() macro could help here too, but I don't see that 
used very much. There may be a better idea i'm missing

> 
> This looks fine. With the above comment addressed or not:
> Reviewed-by: Neha Malcom Francis <n-francis at ti.com>
>

~Bryan