[PATCH 1/4] bbu: Add barebox_update search by device

[PATCH 1/4] bbu: Add barebox_update search by device

[Markus Pargmann]

bbu_data includes a devicefile information. Add the possibility to make
an update based on the given devicefile. This is in addition to the
normal search for a barebox update handler by its name.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 common/bbu.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/common/bbu.c b/common/bbu.c
index 4d71fa4a87cb..bf3790d13fff 100644
--- a/common/bbu.c
+++ b/common/bbu.c
@@ -97,6 +97,20 @@ static struct bbu_handler *bbu_find_handler(const char *name)
 	return NULL;
 }
 
+static struct bbu_handler *bbu_find_handler_by_device(const char *devicepath)
+{
+	struct bbu_handler *handler;
+
+	if (!devicepath)
+		return NULL;
+
+	list_for_each_entry(handler, &bbu_image_handlers, list)
+		if (!strcmp(handler->devicefile, devicepath))
+			return handler;
+
+	return NULL;
+}
+
 /*
  * do a barebox update with data from *data
  */
@@ -105,7 +119,11 @@ int barebox_update(struct bbu_data *data)
 	struct bbu_handler *handler;
 	int ret;
 
-	handler = bbu_find_handler(data->handler_name);
+	handler = bbu_find_handler_by_device(data->devicefile);
+
+	if (!handler)
+		handler = bbu_find_handler(data->handler_name);
+
 	if (!handler)
 		return -ENODEV;
 
-- 
2.7.0.rc3


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

[PATCH 2/4] bbu: Add function to check if an update handler exists

[Markus Pargmann]

This adds a function to check for the existence of an update handler
based on the supplied bbu_data.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 common/bbu.c  | 14 ++++++++++++++
 include/bbu.h |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/common/bbu.c b/common/bbu.c
index bf3790d13fff..a7f6b4b851ee 100644
--- a/common/bbu.c
+++ b/common/bbu.c
@@ -111,6 +111,20 @@ static struct bbu_handler *bbu_find_handler_by_device(const char *devicepath)
 	return NULL;
 }
 
+bool barebox_update_handler_exists(struct bbu_data *data)
+{
+	struct bbu_handler *handler;
+
+	handler = bbu_find_handler_by_device(data->devicefile);
+	if (handler)
+		return true;
+
+	if (!data->handler_name)
+		return false;
+
+	return !bbu_find_handler(data->handler_name);
+}
+
 /*
  * do a barebox update with data from *data
  */
diff --git a/include/bbu.h b/include/bbu.h
index 727791171858..0fe7a1a9bcdb 100644
--- a/include/bbu.h
+++ b/include/bbu.h
@@ -36,6 +36,8 @@ int bbu_confirm(struct bbu_data *);
 
 int barebox_update(struct bbu_data *);
 
+bool barebox_update_handler_exists(struct bbu_data *);
+
 void bbu_handlers_list(void);
 
 #ifdef CONFIG_BAREBOX_UPDATE
-- 
2.7.0.rc3


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

[PATCH 3/4] fastboot: Fix usage of ubiformat for UBI image transfers

[Markus Pargmann]

Currently all fastboot flash commands with UBI images are handled by a
final call to 'ubiformat'. This only makes sense for flash commands
where the target file is a mtd device. If we just want to transfer a UBI
image we would expect a simple copy to the correct location.

This patch checks if the destination file is a MTD device by opening it
and calling an ioctl MEMGETINFO. Only for MTD devices, ubiformat is
called.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 drivers/usb/gadget/f_fastboot.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index bf28f7c22aaa..192c0d6eb7f4 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -686,7 +686,21 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req, const char *cmd
 	}
 
 	if (filetype == filetype_ubi) {
-		char *cmd = asprintf("ubiformat -y -f %s %s", FASTBOOT_TMPFILE, filename);
+		char *cmd;
+		int fd;
+		struct mtd_info_user meminfo;
+
+		fd = open(filename, O_RDONLY);
+		if (!fd)
+			goto copy;
+
+		ret = ioctl(fd, MEMGETINFO, &meminfo);
+		close(fd);
+		/* Not a MTD device, ubiformat is not a valid operation */
+		if (ret)
+			goto copy;
+
+		cmd = asprintf("ubiformat -y -f %s %s", FASTBOOT_TMPFILE, filename);
 
 		fastboot_tx_print(f_fb, "INFOThis is an UBI image...");
 
@@ -702,6 +716,7 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req, const char *cmd
 		goto out;
 	}
 
+copy:
 	ret = copy_file(FASTBOOT_TMPFILE, filename, 1);
 	if (ret) {
 		fastboot_tx_print(f_fb, "FAILwrite partition: %s", strerror(-ret));
-- 
2.7.0.rc3


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

Re: [PATCH 3/4] fastboot: Fix usage of ubiformat for UBI image transfers

[Sascha Hauer]

On Tue, Feb 16, 2016 at 07:54:47PM +0100, Markus Pargmann wrote:
> Currently all fastboot flash commands with UBI images are handled by a
> final call to 'ubiformat'. This only makes sense for flash commands
> where the target file is a mtd device. If we just want to transfer a UBI
> image we would expect a simple copy to the correct location.
> 
> This patch checks if the destination file is a MTD device by opening it
> and calling an ioctl MEMGETINFO. Only for MTD devices, ubiformat is
> called.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  drivers/usb/gadget/f_fastboot.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
> index bf28f7c22aaa..192c0d6eb7f4 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -686,7 +686,21 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req, const char *cmd
>  	}
>  
>  	if (filetype == filetype_ubi) {
> -		char *cmd = asprintf("ubiformat -y -f %s %s", FASTBOOT_TMPFILE, filename);
> +		char *cmd;
> +		int fd;
> +		struct mtd_info_user meminfo;
> +
> +		fd = open(filename, O_RDONLY);
> +		if (!fd)
> +			goto copy;

0 is a valid file descriptor, though one that will never be returned by
open(). You want to check for fd < 0 here.

> +
> +		ret = ioctl(fd, MEMGETINFO, &meminfo);
> +		close(fd);
> +		/* Not a MTD device, ubiformat is not a valid operation */
> +		if (ret)
> +			goto copy;
> +
> +		cmd = asprintf("ubiformat -y -f %s %s", FASTBOOT_TMPFILE, filename);

Hm, ubiformat should get a C api.

Sascha

-- 
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 |

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

Re: [PATCH 3/4] fastboot: Fix usage of ubiformat for UBI image transfers

[Markus Pargmann]

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

Hi,

On Wednesday, February 17, 2016 08:29:14 AM Sascha Hauer wrote:
> On Tue, Feb 16, 2016 at 07:54:47PM +0100, Markus Pargmann wrote:
> > Currently all fastboot flash commands with UBI images are handled by a
> > final call to 'ubiformat'. This only makes sense for flash commands
> > where the target file is a mtd device. If we just want to transfer a UBI
> > image we would expect a simple copy to the correct location.
> > 
> > This patch checks if the destination file is a MTD device by opening it
> > and calling an ioctl MEMGETINFO. Only for MTD devices, ubiformat is
> > called.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> >  drivers/usb/gadget/f_fastboot.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
> > index bf28f7c22aaa..192c0d6eb7f4 100644
> > --- a/drivers/usb/gadget/f_fastboot.c
> > +++ b/drivers/usb/gadget/f_fastboot.c
> > @@ -686,7 +686,21 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req, const char *cmd
> >  	}
> >  
> >  	if (filetype == filetype_ubi) {
> > -		char *cmd = asprintf("ubiformat -y -f %s %s", FASTBOOT_TMPFILE, filename);
> > +		char *cmd;
> > +		int fd;
> > +		struct mtd_info_user meminfo;
> > +
> > +		fd = open(filename, O_RDONLY);
> > +		if (!fd)
> > +			goto copy;
> 
> 0 is a valid file descriptor, though one that will never be returned by
> open(). You want to check for fd < 0 here.

Right, thanks, will fix that.

> 
> > +
> > +		ret = ioctl(fd, MEMGETINFO, &meminfo);
> > +		close(fd);
> > +		/* Not a MTD device, ubiformat is not a valid operation */
> > +		if (ret)
> > +			goto copy;
> > +
> > +		cmd = asprintf("ubiformat -y -f %s %s", FASTBOOT_TMPFILE, filename);
> 
> Hm, ubiformat should get a C api.

Yes, probably.

Best Regards,

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

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

[Markus Pargmann]

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) {
+		struct bbu_data data = {
+			.devicefile = filename,
+			.imagefile = FASTBOOT_TMPFILE,
+			.flags = BBU_FLAG_YES,
+		};
+
+		if (!barebox_update_handler_exists(&data))
+			goto copy;
+
+		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");
+			return;
+		}
+
+		ret = barebox_update(&data);
+
+		if (ret) {
+			fastboot_tx_print(f_fb, "FAILupdate barebox: %s", strerror(-ret));
+			return;
+		}
+
+		goto out;
+	}
+
 copy:
 	ret = copy_file(FASTBOOT_TMPFILE, filename, 1);
 	if (ret) {
-- 
2.7.0.rc3


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

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

[Sascha Hauer]

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.

> +
> +		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/

Sascha

-- 
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 |

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

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

[Markus Pargmann]

[-- 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