[PATCH] board: ti: j721e,j7200: fix do_main_cpsw0_qsgmii_phyinit

[Anshul Dalal]

On Tue Feb 3, 2026 at 12:06 PM IST, Siddharth Vadapalli wrote:
> On Tue, 2026-02-03 at 11:38 +0530, Anshul Dalal wrote:
>> On Tue Feb 3, 2026 at 11:11 AM IST, Siddharth Vadapalli wrote:
>> > On Tue, 2026-02-03 at 10:51 +0530, Anshul Dalal wrote:
>> > > On Mon Feb 2, 2026 at 7:40 PM IST, Siddharth Vadapalli wrote:
>> > > > Since commit 27cc5951c862 ("include: env: ti: add default for
>> > > > do_main_cpsw0_qsgmii_phyinit"), the value of the environment variable
>> > > > do_main_cpsw0_qsgmii_phyinit happened to remain '0' and couldn't be
>> > > > changed without user intervention. This behavior is due to the following
>> > > > cyclic dependency:
>> > > > A) ti_common.env sets do_main_cpsw0_qsgmii_phyinit to '0' and its value
>> > > >    can only be updated automatically by main_cpsw0_qsgmii_phyinit.
>> > > > B) main_cpsw0_qsgmii_phyinit is defined in j721e.env and it can run only
>> > > >    if 'do_main_cpsw0_qsgmii_phyinit' is already '1' which isn't possible
>> > > >    unless the user manually assigns the value.
>> > > > 
>> > > > Fix the aforementioned cyclic dependency by using board_late_init() to
>> > > > detect the QSGMII Daughtercard and set do_main_cpsw0_qsgmii_phyinit.
>> > > > 
>> > > > Additionally, to address the issue of do_main_cpsw0_qsgmii_phyinit being
>> > > > 'undefined' for other platforms, replace:
>> > > > 	if test ${do_main_cpsw0_qsgmii_phyinit} -eq 1;
>> > > > with:
>> > > > 	if env exists do_main_cpsw0_qsgmii_phyinit;
>> > > > in ti_common.env.
>> > > > 
>> > > > Fixes: 27cc5951c862 ("include: env: ti: add default for do_main_cpsw0_qsgmii_phyinit")
>> > > > Signed-off-by: Siddharth Vadapalli <s-vadapalli at ti.com>
>> > > > ---
>> > > > 
>> > 
>> > [TRIMMED]
>> > 
>> > > > diff --git a/include/env/ti/ti_common.env b/include/env/ti/ti_common.env
>> > > > index 03e3267ef8a..a0ed83f52ac 100644
>> > > > --- a/include/env/ti/ti_common.env
>> > > > +++ b/include/env/ti/ti_common.env
>> > > > @@ -22,11 +22,10 @@ get_fit_overlaystring=
>> > > >  	done;
>> > > >  get_fit_config=setexpr name_fit_config gsub / _ conf-${fdtfile}
>> > > >  run_fit=run get_fit_config; bootm ${addr_fit}#${name_fit_config}${overlaystring}
>> > > > -do_main_cpsw0_qsgmii_phyinit=0
>> > > >  bootcmd_ti_mmc=
>> > > >  	run init_${boot};
>> > > >  #if CONFIG_CMD_REMOTEPROC
>> > > > -	if test ${do_main_cpsw0_qsgmii_phyinit} -eq 1;
>> > > > +	if env exists do_main_cpsw0_qsgmii_phyinit;
>> > > 
>> > > The patch looks good to me however I wonder if we should factor out
>> > > cpsw0_qsgmii related envs out of ti_common.env since as far as I can
>> > > tell we only seem to be using it for the j721e/j7200 devices.
>> > 
>> > Please refer to the following commit from 22 July 2023:
>> > https://github.com/u-boot/u-boot/commit/4ae1a2470ce7895b747c85a140aaf8b647ae6a32
>> > The commit added:
>> > 	run main_cpsw0_qsgmii_phyinit
>> > in include/environment/ti/ti_armv7_common.env and ti_armv7_common.env
>> > eventually got renamed to ti_common.env by:
>> > https://github.com/u-boot/u-boot/commit/bf9c61acb6caed97114029d2dc1e91b148cd9b8a
>> > 
>> > Although I agree with your feedback in that it will make more sense for
>> > do_main_cpsw0_qsgmii_phyinit to be present in the board specific env, given
>> > the background pointed out above, it seems to me that we are simply going
>> > in circles by undoing the changes that were previously made. Nevertheless,
>> > if you think that it should be moved to the board environment, I will do so
>> > in a future cleanup patch.
>> 
>> The patch that added main_cpsw0_qsgmii_phyinit was actually based on the
>> commit 14439cd71c1a ("configs: k3: make consistent bootcmd across all k3
>> socs") where main_cpsw0_qsgmii_phyinit was factored out form the
>> j721e/j7200 defconfigs to the common env. I think this could be better
>> handled by having a board level hook such as follows:
>> 
>> 	--- a/include/env/ti/ti_common.env
>> 	+++ b/include/env/ti/ti_common.env
>> 	@@ -22,14 +22,10 @@ get_fit_overlaystring=
>> 		done;
>> 	 get_fit_config=setexpr name_fit_config gsub / _ conf-${fdtfile}
>> 	 run_fit=run get_fit_config; bootm ${addr_fit}#${name_fit_config}${overlaystring}
>> 	+init_board=
>> 	 bootcmd_ti_mmc=
>> 		run init_${boot};
>> 	-#if CONFIG_CMD_REMOTEPROC
>> 	-       if env exists do_main_cpsw0_qsgmii_phyinit;
>> 	-               then run main_cpsw0_qsgmii_phyinit;
>> 	-       fi;
>> 	-       run boot_rprocs;
>> 	-#endif
>> 	+       run init_board;
>
> This assumes that it is defined for all boards and we will run into the
> following error for those boards which don't have it in their env:
> 	## Error: "init_board" not defined
> So it needs to be updated to:
> 	if env exists init_board;
> 		then run init_board;
> 	fi;
>

I thought setting init_board to empty string above would've taken care
of that but I can incorporate your suggestion as well, thanks :)

>> 		if test ${boot_fit} -eq 1;
>> 			then run get_fit_${boot}; run get_fit_overlaystring; run run_fit;
>> 		else;
>> 
>> with init_board being used as required in the board's env as follows:
>> 
>> 	--- a/board/ti/j7200/j7200.env
>> 	+++ b/board/ti/j7200/j7200.env
>> 	@@ -34,6 +34,13 @@ main_cpsw0_qsgmii_phyinit=
>> 		fi;
>> 	 #endif
>> 
>> 	+#if CONFIG_CMD_REMOTEPROC
>> 	+init_board=
>> 	+       if env exists do_main_cpsw0_qsgmii_phyinit;
>> 	+               then run main_cpsw0_qsgmii_phyinit;
>> 	+       fi;
>> 	+       run boot_rprocs;
>> 	+#endif
>> 	 #if CONFIG_TARGET_J7200_A72_EVM
>> 	 rproc_fw_binaries= 1 /lib/firmware/j7200-mcu-r5f0_1-fw 2 /lib/firmware/j7200-main-r5f0_0-fw 3 /lib/firmware/j7200-main-r5f0_1-fw
>> 	 #endif
>
> With the minor comment above, this diff looks good to me. Please let me
> know if you plan on posting the patch for it.
>
> Additionally, since this is unrelated to the current patch, please
> acknowledge that you have no concerns with the current patch.
>

Yeah, I can post the refactor once your patch is merged to which I have
no objections to.

Reviewed-by: Anshul Dalal <anshuld at ti.com>