| Message ID | 1409763954-5494-4-git-send-email-srae@broadcom.com |
|---|---|
| State | Accepted |
| Commit | 1c39d856db8b3e32fe3d5b820fbfc8ba57ab8dd7 |
| Headers | show |
Dear Steve Rae, In message <1409763954-5494-4-git-send-email-srae@broadcom.com> you wrote: > - port dprintf() to debug() > - update formatting > > Signed-off-by: Steve Rae <srae@broadcom.com> > --- > > Changes in v3: > - use original license text > > Changes in v2: > - use BSD-3-Clause > > common/aboot.c | 97 +++++++++++++++++++++++++++++++++------------------------- > 1 file changed, 56 insertions(+), 41 deletions(-) > > diff --git a/common/aboot.c b/common/aboot.c > index a302c92..3611feb 100644 > --- a/common/aboot.c > +++ b/common/aboot.c > @@ -28,6 +28,9 @@ > * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF > * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > * > + * NOTE: > + * Although it is very similar, this license text is not identical > + * to the "BSD-3-Clause", therefore, DO NOT MODIFY THIS LICENSE TEXT! > */ I understand your intention of starting with the pristine file, and then adaptng it to U-Boot, but I don't like adding a broken file in patch 1/4 only to fix it later in patch 3/4. I think it would be better to squash these patches. Second, as already mentioned, we need to assign a SPDX ID for this. Did you check with SPDX if there a matching ID? Best regards, Wolfgang Denk
On Thu, Sep 04, 2014 at 07:28:04AM +0200, Wolfgang Denk wrote: > Dear Steve Rae, > > In message <1409763954-5494-4-git-send-email-srae@broadcom.com> you wrote: > > - port dprintf() to debug() > > - update formatting > > > > Signed-off-by: Steve Rae <srae@broadcom.com> > > --- > > > > Changes in v3: > > - use original license text > > > > Changes in v2: > > - use BSD-3-Clause > > > > common/aboot.c | 97 +++++++++++++++++++++++++++++++++------------------------- > > 1 file changed, 56 insertions(+), 41 deletions(-) > > > > diff --git a/common/aboot.c b/common/aboot.c > > index a302c92..3611feb 100644 > > --- a/common/aboot.c > > +++ b/common/aboot.c > > @@ -28,6 +28,9 @@ > > * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF > > * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > * > > + * NOTE: > > + * Although it is very similar, this license text is not identical > > + * to the "BSD-3-Clause", therefore, DO NOT MODIFY THIS LICENSE TEXT! > > */ > > I understand your intention of starting with the pristine file, and > then adaptng it to U-Boot, but I don't like adding a broken file in > patch 1/4 only to fix it later in patch 3/4. I think it would be > better to squash these patches. But it would make tracking things a bit harder. We could squash 2/4 and 3/4 into one easy enough tho. > Second, as already mentioned, we need to assign a SPDX ID for this. > > Did you check with SPDX if there a matching ID? So, we've gone round-and-round on this, and Steve is doing what I asked him to here. In sum, this is _not_ BSD-3, it's a one-off from it with some interesting wording changes that mean we can't just call it BSD-3. Since there's nothing else going to use this (and frankly I'm mildly puzzled by how hard it is to dig up an aboot.c with sparse image support that doesn't have this change but also does come from a google domain) I didn't want to add a new license file for this non-standard license.
Hi Tom, > On Thu, Sep 04, 2014 at 07:28:04AM +0200, Wolfgang Denk wrote: > > Dear Steve Rae, > > > > In message <1409763954-5494-4-git-send-email-srae@broadcom.com> you > > wrote: > > > - port dprintf() to debug() > > > - update formatting > > > > > > Signed-off-by: Steve Rae <srae@broadcom.com> > > > --- > > > > > > Changes in v3: > > > - use original license text > > > > > > Changes in v2: > > > - use BSD-3-Clause > > > > > > common/aboot.c | 97 > > > +++++++++++++++++++++++++++++++++------------------------- 1 file > > > changed, 56 insertions(+), 41 deletions(-) > > > > > > diff --git a/common/aboot.c b/common/aboot.c > > > index a302c92..3611feb 100644 > > > --- a/common/aboot.c > > > +++ b/common/aboot.c > > > @@ -28,6 +28,9 @@ > > > * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS > > > SOFTWARE, EVEN IF > > > * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > > * > > > + * NOTE: > > > + * Although it is very similar, this license text is not > > > identical > > > + * to the "BSD-3-Clause", therefore, DO NOT MODIFY THIS > > > LICENSE TEXT! */ > > > > I understand your intention of starting with the pristine file, and > > then adaptng it to U-Boot, but I don't like adding a broken file in > > patch 1/4 only to fix it later in patch 3/4. I think it would be > > better to squash these patches. > > But it would make tracking things a bit harder. We could squash 2/4 > and 3/4 into one easy enough tho. > > > Second, as already mentioned, we need to assign a SPDX ID for this. > > > > Did you check with SPDX if there a matching ID? > > So, we've gone round-and-round on this, and Steve is doing what I > asked him to here. In sum, this is _not_ BSD-3, it's a one-off from > it with some interesting wording changes that mean we can't just call > it BSD-3. Since there's nothing else going to use this (and frankly > I'm mildly puzzled by how hard it is to dig up an aboot.c with sparse > image support that doesn't have this change but also does come from a > google domain) I didn't want to add a new license file for this > non-standard license. > Do we have the agreement here and therefore there aren't any obstacles to include those patches to u-boot?
On Mon, Sep 08, 2014 at 12:49:56PM +0200, Lukasz Majewski wrote: > Hi Tom, > > > On Thu, Sep 04, 2014 at 07:28:04AM +0200, Wolfgang Denk wrote: > > > Dear Steve Rae, > > > > > > In message <1409763954-5494-4-git-send-email-srae@broadcom.com> you > > > wrote: > > > > - port dprintf() to debug() > > > > - update formatting > > > > > > > > Signed-off-by: Steve Rae <srae@broadcom.com> > > > > --- > > > > > > > > Changes in v3: > > > > - use original license text > > > > > > > > Changes in v2: > > > > - use BSD-3-Clause > > > > > > > > common/aboot.c | 97 > > > > +++++++++++++++++++++++++++++++++------------------------- 1 file > > > > changed, 56 insertions(+), 41 deletions(-) > > > > > > > > diff --git a/common/aboot.c b/common/aboot.c > > > > index a302c92..3611feb 100644 > > > > --- a/common/aboot.c > > > > +++ b/common/aboot.c > > > > @@ -28,6 +28,9 @@ > > > > * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS > > > > SOFTWARE, EVEN IF > > > > * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > > > * > > > > + * NOTE: > > > > + * Although it is very similar, this license text is not > > > > identical > > > > + * to the "BSD-3-Clause", therefore, DO NOT MODIFY THIS > > > > LICENSE TEXT! */ > > > > > > I understand your intention of starting with the pristine file, and > > > then adaptng it to U-Boot, but I don't like adding a broken file in > > > patch 1/4 only to fix it later in patch 3/4. I think it would be > > > better to squash these patches. > > > > But it would make tracking things a bit harder. We could squash 2/4 > > and 3/4 into one easy enough tho. > > > > > Second, as already mentioned, we need to assign a SPDX ID for this. > > > > > > Did you check with SPDX if there a matching ID? > > > > So, we've gone round-and-round on this, and Steve is doing what I > > asked him to here. In sum, this is _not_ BSD-3, it's a one-off from > > it with some interesting wording changes that mean we can't just call > > it BSD-3. Since there's nothing else going to use this (and frankly > > I'm mildly puzzled by how hard it is to dig up an aboot.c with sparse > > image support that doesn't have this change but also does come from a > > google domain) I didn't want to add a new license file for this > > non-standard license. > > > > Do we have the agreement here and therefore there aren't any obstacles > to include those patches to u-boot? Yes. I plan on grabbing them shortly.
On Wed, Sep 03, 2014 at 10:05:53AM -0700, Steve Rae wrote: > - port dprintf() to debug() > - update formatting > > Signed-off-by: Steve Rae <srae@broadcom.com> Applied to u-boot/master, thanks!
diff --git a/common/aboot.c b/common/aboot.c index a302c92..3611feb 100644 --- a/common/aboot.c +++ b/common/aboot.c @@ -28,6 +28,9 @@ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * + * NOTE: + * Although it is very similar, this license text is not identical + * to the "BSD-3-Clause", therefore, DO NOT MODIFY THIS LICENSE TEXT! */ void cmd_flash_mmc_sparse_img(const char *arg, void *data, unsigned sz) @@ -70,23 +73,24 @@ void cmd_flash_mmc_sparse_img(const char *arg, void *data, unsigned sz) } data += sparse_header->file_hdr_sz; - if(sparse_header->file_hdr_sz > sizeof(sparse_header_t)) + if (sparse_header->file_hdr_sz > sizeof(sparse_header_t)) { - /* Skip the remaining bytes in a header that is longer than + /* + * Skip the remaining bytes in a header that is longer than * we expected. */ data += (sparse_header->file_hdr_sz - sizeof(sparse_header_t)); } - dprintf (SPEW, "=== Sparse Image Header ===\n"); - dprintf (SPEW, "magic: 0x%x\n", sparse_header->magic); - dprintf (SPEW, "major_version: 0x%x\n", sparse_header->major_version); - dprintf (SPEW, "minor_version: 0x%x\n", sparse_header->minor_version); - dprintf (SPEW, "file_hdr_sz: %d\n", sparse_header->file_hdr_sz); - dprintf (SPEW, "chunk_hdr_sz: %d\n", sparse_header->chunk_hdr_sz); - dprintf (SPEW, "blk_sz: %d\n", sparse_header->blk_sz); - dprintf (SPEW, "total_blks: %d\n", sparse_header->total_blks); - dprintf (SPEW, "total_chunks: %d\n", sparse_header->total_chunks); + debug("=== Sparse Image Header ===\n"); + debug("magic: 0x%x\n", sparse_header->magic); + debug("major_version: 0x%x\n", sparse_header->major_version); + debug("minor_version: 0x%x\n", sparse_header->minor_version); + debug("file_hdr_sz: %d\n", sparse_header->file_hdr_sz); + debug("chunk_hdr_sz: %d\n", sparse_header->chunk_hdr_sz); + debug("blk_sz: %d\n", sparse_header->blk_sz); + debug("total_blks: %d\n", sparse_header->total_blks); + debug("total_chunks: %d\n", sparse_header->total_chunks); /* Start processing chunks */ for (chunk=0; chunk<sparse_header->total_chunks; chunk++) @@ -95,33 +99,37 @@ void cmd_flash_mmc_sparse_img(const char *arg, void *data, unsigned sz) chunk_header = (chunk_header_t *) data; data += sizeof(chunk_header_t); - dprintf (SPEW, "=== Chunk Header ===\n"); - dprintf (SPEW, "chunk_type: 0x%x\n", chunk_header->chunk_type); - dprintf (SPEW, "chunk_data_sz: 0x%x\n", chunk_header->chunk_sz); - dprintf (SPEW, "total_size: 0x%x\n", chunk_header->total_sz); + debug("=== Chunk Header ===\n"); + debug("chunk_type: 0x%x\n", chunk_header->chunk_type); + debug("chunk_data_sz: 0x%x\n", chunk_header->chunk_sz); + debug("total_size: 0x%x\n", chunk_header->total_sz); - if(sparse_header->chunk_hdr_sz > sizeof(chunk_header_t)) + if (sparse_header->chunk_hdr_sz > sizeof(chunk_header_t)) { - /* Skip the remaining bytes in a header that is longer than - * we expected. + /* + * Skip the remaining bytes in a header that is longer + * than we expected. */ - data += (sparse_header->chunk_hdr_sz - sizeof(chunk_header_t)); + data += (sparse_header->chunk_hdr_sz - + sizeof(chunk_header_t)); } chunk_data_sz = sparse_header->blk_sz * chunk_header->chunk_sz; switch (chunk_header->chunk_type) { case CHUNK_TYPE_RAW: - if(chunk_header->total_sz != (sparse_header->chunk_hdr_sz + - chunk_data_sz)) + if (chunk_header->total_sz != + (sparse_header->chunk_hdr_sz + chunk_data_sz)) { - fastboot_fail("Bogus chunk size for chunk type Raw"); + fastboot_fail( + "Bogus chunk size for chunk type Raw"); return; } - if(mmc_write(ptn + ((uint64_t)total_blocks*sparse_header->blk_sz), - chunk_data_sz, - (unsigned int*)data)) + if (mmc_write(ptn + + ((uint64_t)total_blocks * + sparse_header->blk_sz), + chunk_data_sz, (unsigned int *)data)) { fastboot_fail("flash write failure"); return; @@ -131,17 +139,22 @@ void cmd_flash_mmc_sparse_img(const char *arg, void *data, unsigned sz) break; case CHUNK_TYPE_FILL: - if(chunk_header->total_sz != (sparse_header->chunk_hdr_sz + - sizeof(uint32_t))) + if (chunk_header->total_sz != + (sparse_header->chunk_hdr_sz + sizeof(uint32_t))) { - fastboot_fail("Bogus chunk size for chunk type FILL"); + fastboot_fail( + "Bogus chunk size for chunk type FILL"); return; } - fill_buf = (uint32_t *)memalign(CACHE_LINE, ROUNDUP(sparse_header->blk_sz, CACHE_LINE)); + fill_buf = (uint32_t *) + memalign(CACHE_LINE, + ROUNDUP(sparse_header->blk_sz, + CACHE_LINE)); if (!fill_buf) { - fastboot_fail("Malloc failed for: CHUNK_TYPE_FILL"); + fastboot_fail( + "Malloc failed for: CHUNK_TYPE_FILL"); return; } @@ -156,9 +169,10 @@ void cmd_flash_mmc_sparse_img(const char *arg, void *data, unsigned sz) for (i = 0; i < chunk_blk_cnt; i++) { - if(mmc_write(ptn + ((uint64_t)total_blocks*sparse_header->blk_sz), - sparse_header->blk_sz, - fill_buf)) + if (mmc_write(ptn + + ((uint64_t)total_blocks * + sparse_header->blk_sz), + sparse_header->blk_sz, fill_buf)) { fastboot_fail("flash write failure"); free(fill_buf); @@ -176,9 +190,11 @@ void cmd_flash_mmc_sparse_img(const char *arg, void *data, unsigned sz) break; case CHUNK_TYPE_CRC: - if(chunk_header->total_sz != sparse_header->chunk_hdr_sz) + if (chunk_header->total_sz != + sparse_header->chunk_hdr_sz) { - fastboot_fail("Bogus chunk size for chunk type Dont Care"); + fastboot_fail( + "Bogus chunk size for chunk type Dont Care"); return; } total_blocks += chunk_header->chunk_sz; @@ -186,19 +202,18 @@ void cmd_flash_mmc_sparse_img(const char *arg, void *data, unsigned sz) break; default: - dprintf(CRITICAL, "Unkown chunk type: %x\n",chunk_header->chunk_type); + debug("Unkown chunk type: %x\n", + chunk_header->chunk_type); fastboot_fail("Unknown chunk type"); return; } } - dprintf(INFO, "Wrote %d blocks, expected to write %d blocks\n", - total_blocks, sparse_header->total_blks); + debug("Wrote %d blocks, expected to write %d blocks\n", + total_blocks, sparse_header->total_blks); - if(total_blocks != sparse_header->total_blks) - { + if (total_blocks != sparse_header->total_blks) fastboot_fail("sparse image write failure"); - } fastboot_okay(""); return;
- port dprintf() to debug() - update formatting Signed-off-by: Steve Rae <srae@broadcom.com> --- Changes in v3: - use original license text Changes in v2: - use BSD-3-Clause common/aboot.c | 97 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 56 insertions(+), 41 deletions(-)