Re: [PATCH 4/4] fastboot: Add a ARM Barebox filetype handler

[Markus Pargmann <mpa@pengutronix.de>]

[-- Attachment #1.1: Type: text/plain, Size: 2638 bytes --]

On Wednesday, February 17, 2016 08:37:49 AM Sascha Hauer wrote:
> On Tue, Feb 16, 2016 at 07:54:48PM +0100, Markus Pargmann wrote:
> > This will automatically call barebox_update for the transfered file if
> > it is an ARM Barebox image and the destination file is defined by some
> > update handler.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> >  drivers/usb/gadget/f_fastboot.c | 29 +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
> > index 192c0d6eb7f4..ae18d068e93f 100644
> > --- a/drivers/usb/gadget/f_fastboot.c
> > +++ b/drivers/usb/gadget/f_fastboot.c
> > @@ -23,6 +23,7 @@
> >  #include <clock.h>
> >  #include <ioctl.h>
> >  #include <libbb.h>
> > +#include <bbu.h>
> >  #include <boot.h>
> >  #include <dma.h>
> >  #include <fs.h>
> > @@ -716,6 +717,34 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req, const char *cmd
> >  		goto out;
> >  	}
> >  
> > +	if (filetype == filetype_arm_barebox) {
> 
> There are some other barebox image types, like filetype_mips_barebox and
> filetype_ch_image, so maybe we should check for them aswell? But then
> again, ...
> 
> > +		struct bbu_data data = {
> > +			.devicefile = filename,
> > +			.imagefile = FASTBOOT_TMPFILE,
> > +			.flags = BBU_FLAG_YES,
> > +		};
> > +
> > +		if (!barebox_update_handler_exists(&data))
> > +			goto copy;
> 
> ... Do we need the filetype check at all? Isn't this check enough? With
> existing update handler for a device using barebox_update is the right
> way to go, no matter what the filetype check says.

There may be someone out there who wishes to write some arbitrary data
to the same device where the bootloader is. As far as I know the barebox
update handler is able to write barebox images to the given device. I
don't know what happens with data that is not a barebox image. So I
would prefer to use the filetype checks first.

> 
> > +
> > +		fastboot_tx_print(f_fb, "INFOThis is an ARM Barebox image...");
> > +
> > +		data.image = read_file(data.imagefile, &data.len);
> > +		if (!data.image) {
> > +			fastboot_tx_print(f_fb, "FAILreading bareboxs");
> 
> s/bareboxs/barebox/

Will fix.

Thanks,

Markus

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox