On Mon, Mar 10, 2025 at 07:52:17PM +0100, Marco Felsch wrote: > On 25-02-28, Sascha Hauer wrote: > > Add missing error handling in the image loading code. Properly check for > > errors and panic when mandatory binaries are missing instead of blindly > > continuing. > > > > Signed-off-by: Sascha Hauer <[email protected]> > > --- > > arch/arm/mach-k3/r5.c | 57 > > ++++++++++++++++++++++++++++----------------------- > > 1 file changed, 31 insertions(+), 26 deletions(-) > > > > diff --git a/arch/arm/mach-k3/r5.c b/arch/arm/mach-k3/r5.c > > index 2418e9ae73..e12c888afa 100644 > > --- a/arch/arm/mach-k3/r5.c > > +++ b/arch/arm/mach-k3/r5.c > > @@ -283,7 +283,7 @@ static int load_fip(const char *filename, off_t offset) > > return 0; > > } > > > > -static void do_dfu(void) > > +static int do_dfu(void) > > { > > struct usbgadget_funcs funcs = {}; > > int ret; > > @@ -293,37 +293,24 @@ static void do_dfu(void) > > funcs.dfu_opts = "/fip.img(fip)cs"; > > > > ret = usbgadget_prepare_register(&funcs); > > - if (ret) > > - goto err; > > + if (ret) { > > + pr_err("DFU failed with: %pe\n", ERR_PTR(ret)); > > + return ret; > > + } > > > > while (1) { > > ret = stat("/fip.img", &s); > > if (!ret) { > > printf("Downloaded FIP image, DFU done\n"); > > - load_fip("/fip.img", 0); > > - break; > > + ret = load_fip("/fip.img", 0); > > + if (!ret) > > + return 0; > > } > > > > command_slice_release(); > > mdelay(50); > > command_slice_acquire(); > > }; > > - > > - return; > > - > > -err: > > - pr_err("DFU failed with: %pe\n", ERR_PTR(ret)); > > -} > > - > > -static int load_images(void) > > -{ > > - int err; > > - > > - err = load_fip("/boot/k3.fip", 0); > > - if (!err) > > - return 0; > > - > > - return 0; > > } > > Removing load_images should go into patch-3.
I deliberately did it in this patch because load_images already does error checking, but up to now the return value of load_images() wasn't checked. By doing it like you suggest I would either end up calling load_fip() without checking the error, or I would introduce an incomplete error checking in k3_r5_start_image() where the return value of load_fip() is checked, but not the return values of do_dfu and load_fip_emmc(). > > static int load_fip_emmc(void) > > @@ -332,6 +319,7 @@ static int load_fip_emmc(void) > > struct mci *mci; > > char *fname; > > const char *mmcdev = "mmc0"; > > + int ret; > > Nit: Sometimes you do use ret, sometimes err. I would like to align it > if possible. Changed to 'ret' consistently. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
