On Sun, Sep 06, 2015 at 06:11:37PM +0200, Maxime Ripard wrote:
> On Fri, Sep 04, 2015 at 01:20:44PM -0400, Tom Rini wrote:
> > On Mon, Aug 31, 2015 at 04:46:06PM +0200, Maxime Ripard wrote:
> >
> > > The functions and a few define to generate a fastboot message to be sent
> > > back to the host were so far duplicated among the users.
> > >
> > > Move them all to a common place.
> > [snip]
> > > diff --git a/common/aboot.c b/common/aboot.c
> > > index 18ff30ee6d11..37ad50efc50a 100644
> > > --- a/common/aboot.c
> > > +++ b/common/aboot.c
> > > @@ -277,7 +277,7 @@ void store_sparse_image(sparse_storage_t *storage,
> > >
> > > sparse_header = sparse_parse_header(&data);
> > > if (!sparse_header) {
> > > - fastboot_fail("sparse header issue\n");
> > > + printf("sparse header issue\n");
> > > return;
> > > }
> > >
> > > @@ -288,7 +288,6 @@ void store_sparse_image(sparse_storage_t *storage,
> > > if (sparse_header->blk_sz % storage->block_sz) {
> > > printf("%s: Sparse image block size issue [%u]\n",
> > > __func__, sparse_header->blk_sz);
> > > - fastboot_fail("sparse image block size issue");
> > > return;
> > > }
> > >
> > > @@ -299,7 +298,7 @@ void store_sparse_image(sparse_storage_t *storage,
> > > for (chunk = 0; chunk < sparse_header->total_chunks; chunk++) {
> > > chunk_header = sparse_parse_chunk(sparse_header, &data);
> > > if (!chunk_header) {
> > > - fastboot_fail("Unknown chunk type");
> > > + printf("Unknown chunk type");
> > > return;
> > > }
> > >
> > > @@ -314,7 +313,6 @@ void store_sparse_image(sparse_storage_t *storage,
> > > if (blk + blkcnt > storage->start + storage->size) {
> > > printf("%s: Request would exceed partition size!\n",
> > > __func__);
> > > - fastboot_fail("Request would exceed partition size!");
> > > return;
> > > }
> > >
> > > @@ -331,7 +329,6 @@ void store_sparse_image(sparse_storage_t *storage,
> > > if (buffer_blks != buffer_blk_cnt) {
> > > printf("%s: Write %d failed %d\n",
> > > __func__, i, buffer_blks);
> > > - fastboot_fail("flash write failure");
> > > return;
> > > }
> > >
> > > @@ -348,9 +345,10 @@ void store_sparse_image(sparse_storage_t *storage,
> > > printf("........ wrote %u bytes to '%s'\n", bytes_written,
> > > storage->name);
> > >
> > > - if (total_blocks != sparse_header->total_blks)
> > > - fastboot_fail("sparse image write failure");
> > > + if (total_blocks != sparse_header->total_blks) {
> > > + printf("sparse image write failure");
> > > + return;
> > > + }
> > >
> > > - fastboot_okay("");
> > > return;
> > > }
> >
> > Why in the case of this image do we not need to do fastboot_fail/okay
> > now? It's not obvious from the rest of the changes to me, thanks!
>
> I took that shortcut while refactoring and forgot to fix it...
>
> The issue is that we don't have access to the response buffer in those
> functions. We could pass it as an argument, but that would:
>
> - Prevent the calling function (that "owns" the buffer pointer) to
> use fastboot_fail / fastboot_okay, which feels a bit wrong.
>
> - On a more theorical point, the sparse image format should be
> decoupled from the fastboot protocol, hence not really rely of
> these functions.
>
> What we can do though, is simply return an error code, and the storage
> layers could use fastboot_okay / fastboot_fail themselves. Does that
> sound good?Error code and letting the higher level deal with it sounds good to me! -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
