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 <[email protected]>
> > > > ---
> > > >
> >
> > [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;
> 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.
Regards,
Siddharth.
