[PATCH v2] env: ti: ti_common.env: Fix get_overlaystring for FIT Image

[Manorit Chawdhry]

Hi Andrew,

On 12:03-20230921, Andrew Davis wrote:
> On 9/21/23 5:49 AM, Manorit Chawdhry wrote:
> > After the refactor with conf- nodes in fitImage, overlaystring wasn't
> > didn't handle the new conf- nodes in FIT Booting. Fix get_overlaystring
> > to handle conf- nodes.
> > 
> > Fixes: 837833a724b7 ("environment: ti: Add get_fit_config command to get FIT config string")
> > Reported-by: Aniket Limaye <a-limaye at ti.com>
> > Signed-off-by: Manorit Chawdhry <m-chawdhry at ti.com>
> > ---
> > Test Logs:
> > => setenv name_overlays ti/k3-dt.dtbo ti/k3-dt1.dtbo
> > => run get_overlaystring
> > => printenv overlaystring
> > overlaystring=#conf-ti_k3-dt.dtbo#conf-ti_k3-dt1.dtbo
> > ---
> > Changes in v2:
> > - Fix tabs
> > - Link to v1: https://lore.kernel.org/r/20230919-b4-upstream-overlaystring-v1-1-4c56e8d66173@ti.com
> > ---
> >   include/env/ti/ti_common.env | 11 ++++++++---
> >   1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/env/ti/ti_common.env b/include/env/ti/ti_common.env
> > index e87a41a6590b..bc554fe7ca6d 100644
> > --- a/include/env/ti/ti_common.env
> > +++ b/include/env/ti/ti_common.env
> > @@ -16,9 +16,14 @@ addr_fit=0x90000000
> >   name_fit=fitImage
> >   update_to_fit=setenv loadaddr ${addr_fit}; setenv bootfile ${name_fit}
> >   get_overlaystring=
> > -	for overlay in $name_overlays;
> > -	do;
> > -	setenv overlaystring ${overlaystring}'#'${overlay};
> > +	for overlay in $name_overlays; do;
> > +		if test ${boot_fit} -eq 1;
> > +			then
> 
> Why is the `then` tabbed out here, you can make it on the end of
> the above line, same as you changed for `do`. Then untab all this
> by one.
> 
> > +				setexpr name_fit_overlay gsub / _ conf-${overlay};
> > +				setenv overlaystring ${overlaystring}'#'${name_fit_overlay};
> > +			else
> > +				setenv overlaystring ${overlaystring}'#'${overlay};
> 
> If you set `overlay` using setexpr, then this line is common and doesn't need to
> be out here. Also do we use this overlaystring for anything other than the
> FIT case? And if not, why do we need this boot_fit == 1 check at all?

Ah, I see this is only for FIT usecase. I'll refactor the name too to
get_fit_overlaystring as that makes more sense then.

Regards,
Manorit

> 
> Andrew
> 
> > +		fi;
> >   	done;
> >   get_fit_config=setexpr name_fit_config gsub / _ conf-${fdtfile}
> >   run_fit=run get_fit_config; bootm ${addr_fit}#${name_fit_config}${overlaystring}
> > 
> > ---
> > base-commit: 2fe4b54556ea6271237b35de68dc458bfceab94c
> > change-id: 20230915-b4-upstream-overlaystring-207e28b8c5fb
> > 
> > Best regards,