On 23:20-20240119, Apurva Nandan wrote:
[...]
> +void k3_spl_init(void)
> +{
> + struct udevice *dev;
> + int ret;
> +
> + /*
> + * 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_CPU_V7R)) {
> + /*
> + * 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
Nit pick - there is no SYSFW image anymore - it is either TIFS or DM.
> + * 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), NULL, NULL);
> +
> + 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();
> +}
> +
> +void k3_mem_init(void)
> +{
> + struct udevice *dev;
> + int ret, ctr = 1;
> +
> + if (IS_ENABLED(CONFIG_K3_J721E_DDRSS)) {
> + ret = uclass_get_device(UCLASS_RAM, 0, &dev);
> + if (ret)
> + panic("DRAM 0 init failed: %d\n", ret);
> +
> + while (dev) {
why loop on dev? is it possible to have ret != 0 and dev = 0?
> + ret = uclass_next_device_err(&dev);
> + if (ret) {
> + printf("Initialized %d DRAM controllers\n",
> ctr);
> + break;
> + }
> + ctr++;
What is the use of ctr++ ?? please do a limit check for instances.
[...]
Next time, please respond to the review comment questions so that I
know that you have considered and decided something is not necessary
or something was missed in the new version - for example what happened
to mmc_stop/restart?
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5
849D 1736 249D
