Hi Sascha,
On Thu, Mar 26, 2020 at 07:03:11AM +0100, Sascha Hauer wrote:
> Hi Jules,
>
> >
> > @@ -132,6 +134,10 @@ struct file_list_entry *dfu_file_entry;
> > static int dfufd = -EINVAL;
> > static struct file_list *dfu_files;
> > static int dfudetach;
> > +static struct mtd_info_user dfu_mtdinfo;
> > +static loff_t dfu_written;
> > +static loff_t dfu_erased;
> > +static int prog_erase;
>
> These are not initialized, this works for the first file updated via
> DFU, but what about the following ones?
Good point, this initialization is be required between to file.
I don't think cleaning this in unbind will be sufficient, I will
try to update multiple files.
>
> >
> > /* USB DFU functional descriptor */
> > static struct usb_dfu_func_descriptor usb_dfu_func = {
> > @@ -327,8 +333,16 @@ static void dfu_cleanup(struct f_dfu *dfu)
> > static void dn_complete(struct usb_ep *ep, struct usb_request *req)
> > {
> > struct f_dfu *dfu = req->context;
> > + loff_t size;
> > int ret;
> >
> > + if (prog_erase && (dfu_written + req->length) > dfu_erased) {
> > + size = roundup(req->length, dfu_mtdinfo.erasesize);
> > + erase(dfufd, size, dfu_erased);
>
> You should check the return value here.
Right
> > + dfu_erased += size;
> > + }
> > +
> > + dfu_written += req->length;
> > ret = write(dfufd, req->buf, req->length);
> > if (ret < (int)req->length) {
> > perror("write");
> > @@ -493,7 +507,12 @@ static int dfu_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
> > }
> >
> > if (!(dfu_file_entry->flags & FILE_LIST_FLAG_SAFE)) {
> > - ret = erase(dfufd, ERASE_SIZE_ALL, 0);
> > + ret = ioctl(dfufd, MEMGETINFO, &dfu_mtdinfo);
> > + if (ret) /* not a mtd */
> > + ret = erase(dfufd, ERASE_SIZE_ALL, 0);
> > + else
> > + prog_erase = 1;
>
> I am not aware of any non mtd devices that need erase. I think you can
> drop the full erase here.
OK
Thanks for the review,
Jules
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
