* [U-Boot] [PATCH 01/13] mtd: uboot: Add meaningful error message
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-04 17:20 ` Tom Rini
2015-08-31 14:46 ` [U-Boot] [PATCH 02/13] sparse: Move main header parsing to a function of its own Maxime Ripard
` (11 subsequent siblings)
12 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
The current error message in get_part if CONFIG_MTDPARTS is disabled is
"offset is not a number" which is confusing and doesn't help at all.
Change that for something that might give a hint on what's going on.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
drivers/mtd/mtd_uboot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index c517b9c65d68..21386951efd5 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -37,7 +37,7 @@ static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
return 0;
#else
- puts("offset is not a number\n");
+ puts("mtdparts support missing.\n");
return -1;
#endif
}
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 02/13] sparse: Move main header parsing to a function of its own
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
2015-08-31 14:46 ` [U-Boot] [PATCH 01/13] mtd: uboot: Add meaningful error message Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-04 17:20 ` Tom Rini
2015-08-31 14:46 ` [U-Boot] [PATCH 03/13] sparse: Refactor chunk parsing function Maxime Ripard
` (10 subsequent siblings)
12 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
The current sparse image format parser is quite tangled, with a lot of
code duplication.
Start refactoring it by moving the header parsing function to a function
of its own.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
common/aboot.c | 55 +++++++++++++++++++++++++++++--------------------------
1 file changed, 29 insertions(+), 26 deletions(-)
diff --git a/common/aboot.c b/common/aboot.c
index fba8e3e683e7..7f12677d13a1 100644
--- a/common/aboot.c
+++ b/common/aboot.c
@@ -41,35 +41,12 @@
#include <part.h>
#include <sparse_format.h>
-void write_sparse_image(block_dev_desc_t *dev_desc,
- disk_partition_t *info, const char *part_name,
- void *data, unsigned sz)
+static sparse_header_t *sparse_parse_header(void **data)
{
- lbaint_t blk;
- lbaint_t blkcnt;
- lbaint_t blks;
- uint32_t bytes_written = 0;
- unsigned int chunk;
- unsigned int chunk_data_sz;
- uint32_t *fill_buf = NULL;
- uint32_t fill_val;
- sparse_header_t *sparse_header;
- chunk_header_t *chunk_header;
- uint32_t total_blocks = 0;
- int i;
-
/* Read and skip over sparse image header */
- sparse_header = (sparse_header_t *) data;
+ sparse_header_t *sparse_header = (sparse_header_t *) *data;
- data += sparse_header->file_hdr_sz;
- if (sparse_header->file_hdr_sz > sizeof(sparse_header_t))
- {
- /*
- * Skip the remaining bytes in a header that is longer than
- * we expected.
- */
- data += (sparse_header->file_hdr_sz - sizeof(sparse_header_t));
- }
+ *data += sparse_header->file_hdr_sz;
debug("=== Sparse Image Header ===\n");
debug("magic: 0x%x\n", sparse_header->magic);
@@ -81,6 +58,32 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
debug("total_blks: %d\n", sparse_header->total_blks);
debug("total_chunks: %d\n", sparse_header->total_chunks);
+ return sparse_header;
+}
+
+void write_sparse_image(block_dev_desc_t *dev_desc,
+ disk_partition_t *info, const char *part_name,
+ void *data, unsigned sz)
+{
+ lbaint_t blk;
+ lbaint_t blkcnt;
+ lbaint_t blks;
+ uint32_t bytes_written = 0;
+ unsigned int chunk;
+ unsigned int chunk_data_sz;
+ uint32_t *fill_buf = NULL;
+ uint32_t fill_val;
+ sparse_header_t *sparse_header;
+ chunk_header_t *chunk_header;
+ uint32_t total_blocks = 0;
+ int i;
+
+ sparse_header = sparse_parse_header(&data);
+ if (!sparse_header) {
+ fastboot_fail("sparse header issue\n");
+ return;
+ }
+
/* verify sparse_header->blk_sz is an exact multiple of info->blksz */
if (sparse_header->blk_sz !=
(sparse_header->blk_sz & ~(info->blksz - 1))) {
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 03/13] sparse: Refactor chunk parsing function
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
2015-08-31 14:46 ` [U-Boot] [PATCH 01/13] mtd: uboot: Add meaningful error message Maxime Ripard
2015-08-31 14:46 ` [U-Boot] [PATCH 02/13] sparse: Move main header parsing to a function of its own Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-04 17:20 ` Tom Rini
2015-08-31 14:46 ` [U-Boot] [PATCH 04/13] sparse: Simplify multiple logic Maxime Ripard
` (9 subsequent siblings)
12 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
The chunk parsing code was duplicating a lot of code among the various
chunk types, while all of them could be covered by generic and simple
functions.
Refactor the current code to reuse as much code as possible and hopefully
make the chunk parsing loop more readable and concise.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
common/aboot.c | 351 ++++++++++++++++++++++++++++++++++++---------------------
1 file changed, 221 insertions(+), 130 deletions(-)
diff --git a/common/aboot.c b/common/aboot.c
index 7f12677d13a1..65e633acfcb9 100644
--- a/common/aboot.c
+++ b/common/aboot.c
@@ -37,10 +37,29 @@
#include <config.h>
#include <common.h>
#include <aboot.h>
+#include <errno.h>
#include <malloc.h>
#include <part.h>
#include <sparse_format.h>
+static unsigned int sparse_get_chunk_data_size(sparse_header_t *sparse,
+ chunk_header_t *chunk)
+{
+ return chunk->total_sz - sparse->chunk_hdr_sz;
+}
+
+static bool sparse_chunk_has_buffer(chunk_header_t *chunk)
+{
+ switch (chunk->chunk_type) {
+ case CHUNK_TYPE_RAW:
+ case CHUNK_TYPE_FILL:
+ return true;
+
+ default:
+ return false;
+ }
+}
+
static sparse_header_t *sparse_parse_header(void **data)
{
/* Read and skip over sparse image header */
@@ -61,6 +80,173 @@ static sparse_header_t *sparse_parse_header(void **data)
return sparse_header;
}
+static int sparse_parse_fill_chunk(sparse_header_t *sparse,
+ chunk_header_t *chunk)
+{
+ unsigned int chunk_data_sz = sparse_get_chunk_data_size(sparse, chunk);
+
+ if (chunk_data_sz != sizeof(uint32_t))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int sparse_parse_raw_chunk(sparse_header_t *sparse,
+ chunk_header_t *chunk)
+{
+ unsigned int chunk_data_sz = sparse_get_chunk_data_size(sparse, chunk);
+
+ /* Check if the data size is a multiple of the main block size */
+ if (chunk_data_sz % sparse->blk_sz)
+ return -EINVAL;
+
+ /* Check that the chunk size is consistent */
+ if ((chunk_data_sz / sparse->blk_sz) != chunk->chunk_sz)
+ return -EINVAL;
+
+ return 0;
+}
+
+static chunk_header_t *sparse_parse_chunk(sparse_header_t *sparse,
+ void **image)
+{
+ chunk_header_t *chunk = (chunk_header_t *) *image;
+ int ret;
+
+ debug("=== Chunk Header ===\n");
+ debug("chunk_type: 0x%x\n", chunk->chunk_type);
+ debug("chunk_data_sz: 0x%x\n", chunk->chunk_sz);
+ debug("total_size: 0x%x\n", chunk->total_sz);
+
+ switch (chunk->chunk_type) {
+ case CHUNK_TYPE_RAW:
+ ret = sparse_parse_raw_chunk(sparse, chunk);
+ if (ret)
+ return NULL;
+ break;
+
+ case CHUNK_TYPE_FILL:
+ ret = sparse_parse_fill_chunk(sparse, chunk);
+ if (ret)
+ return NULL;
+ break;
+
+ case CHUNK_TYPE_DONT_CARE:
+ case CHUNK_TYPE_CRC32:
+ debug("Ignoring chunk\n");
+ break;
+
+ default:
+ printf("%s: Unknown chunk type: %x\n", __func__,
+ chunk->chunk_type);
+ return NULL;
+ }
+
+ *image += sparse->chunk_hdr_sz;
+
+ return chunk;
+}
+
+static int sparse_get_fill_buffer(sparse_header_t *sparse,
+ chunk_header_t *chunk,
+ sparse_buffer_t *buffer,
+ unsigned int blk_sz,
+ void *data)
+{
+ int i;
+
+ buffer->type = CHUNK_TYPE_FILL;
+
+ /*
+ * We create a buffer of one block, and ask it to be
+ * repeated as many times as needed.
+ */
+ buffer->length = blk_sz;
+ buffer->repeat = (chunk->chunk_sz * sparse->blk_sz) / blk_sz;
+
+ buffer->data = memalign(ARCH_DMA_MINALIGN,
+ ROUNDUP(blk_sz,
+ ARCH_DMA_MINALIGN));
+ if (!buffer->data)
+ return -ENOMEM;
+
+ for (i = 0; i < (buffer->length / sizeof(uint32_t)); i++)
+ ((uint32_t *)buffer->data)[i] = *(uint32_t *)(data);
+
+ return 0;
+}
+
+static int sparse_get_raw_buffer(sparse_header_t *sparse,
+ chunk_header_t *chunk,
+ sparse_buffer_t *buffer,
+ unsigned int blk_sz,
+ void *data)
+{
+ unsigned int chunk_data_sz = sparse_get_chunk_data_size(sparse, chunk);
+
+ buffer->type = CHUNK_TYPE_RAW;
+ buffer->length = chunk_data_sz;
+ buffer->data = data;
+ buffer->repeat = 1;
+
+ return 0;
+}
+
+static sparse_buffer_t *sparse_get_data_buffer(sparse_header_t *sparse,
+ chunk_header_t *chunk,
+ unsigned int blk_sz,
+ void **image)
+{
+ unsigned int chunk_data_sz = sparse_get_chunk_data_size(sparse, chunk);
+ sparse_buffer_t *buffer;
+ void *data = *image;
+ int ret;
+
+ *image += chunk_data_sz;
+
+ if (!sparse_chunk_has_buffer(chunk))
+ return NULL;
+
+ buffer = calloc(sizeof(sparse_buffer_t), 1);
+ if (!buffer)
+ return NULL;
+
+ switch(chunk->chunk_type) {
+ case CHUNK_TYPE_RAW:
+ ret = sparse_get_raw_buffer(sparse, chunk, buffer, blk_sz,
+ data);
+ if (ret)
+ return NULL;
+ break;
+
+ case CHUNK_TYPE_FILL:
+ ret = sparse_get_fill_buffer(sparse, chunk, buffer, blk_sz,
+ data);
+ if (ret)
+ return NULL;
+ break;
+
+ default:
+ return NULL;
+ }
+
+ debug("=== Buffer ===\n");
+ debug("length: 0x%x\n", buffer->length);
+ debug("repeat: 0x%x\n", buffer->repeat);
+ debug("type: 0x%x\n", buffer->type);
+ debug("data: 0x%p\n", buffer->data);
+
+ return buffer;
+}
+
+static void sparse_put_data_buffer(sparse_buffer_t *buffer)
+{
+ if (buffer->type == CHUNK_TYPE_FILL)
+ free(buffer->data);
+
+ free(buffer);
+}
+
void write_sparse_image(block_dev_desc_t *dev_desc,
disk_partition_t *info, const char *part_name,
void *data, unsigned sz)
@@ -69,12 +255,10 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
lbaint_t blkcnt;
lbaint_t blks;
uint32_t bytes_written = 0;
- unsigned int chunk;
- unsigned int chunk_data_sz;
- uint32_t *fill_buf = NULL;
- uint32_t fill_val;
+ unsigned int chunk;
sparse_header_t *sparse_header;
- chunk_header_t *chunk_header;
+ chunk_header_t *chunk_header;
+ sparse_buffer_t *buffer;
uint32_t total_blocks = 0;
int i;
@@ -97,142 +281,49 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
/* Start processing chunks */
blk = info->start;
- for (chunk=0; chunk<sparse_header->total_chunks; chunk++)
- {
- /* Read and skip over chunk header */
- chunk_header = (chunk_header_t *) data;
- data += sizeof(chunk_header_t);
-
- if (chunk_header->chunk_type != CHUNK_TYPE_RAW) {
- 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);
+ 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");
+ return;
}
- if (sparse_header->chunk_hdr_sz > sizeof(chunk_header_t))
- {
- /*
- * Skip the remaining bytes in a header that is longer
- * than we expected.
- */
- data += (sparse_header->chunk_hdr_sz -
- sizeof(chunk_header_t));
- }
+ /* Retrieve the buffer we're going to write */
+ buffer = sparse_get_data_buffer(sparse_header, chunk_header,
+ info->blksz, &data);
+ if (!buffer)
+ continue;
- chunk_data_sz = sparse_header->blk_sz * chunk_header->chunk_sz;
- blkcnt = chunk_data_sz / info->blksz;
- switch (chunk_header->chunk_type)
- {
- case CHUNK_TYPE_RAW:
- if (chunk_header->total_sz !=
- (sparse_header->chunk_hdr_sz + chunk_data_sz))
- {
- fastboot_fail(
- "Bogus chunk size for chunk type Raw");
- return;
- }
+ blkcnt = (buffer->length / info->blksz) * buffer->repeat;
- if (blk + blkcnt > info->start + info->size) {
- printf(
- "%s: Request would exceed partition size!\n",
- __func__);
- fastboot_fail(
- "Request would exceed partition size!");
- return;
- }
-
- blks = dev_desc->block_write(dev_desc->dev, blk, blkcnt,
- data);
- if (blks != blkcnt) {
- printf("%s: Write failed " LBAFU "\n",
- __func__, blks);
- fastboot_fail("flash write failure");
- return;
- }
- blk += blkcnt;
- bytes_written += blkcnt * info->blksz;
- total_blocks += chunk_header->chunk_sz;
- data += chunk_data_sz;
- break;
-
- case CHUNK_TYPE_FILL:
- if (chunk_header->total_sz !=
- (sparse_header->chunk_hdr_sz + sizeof(uint32_t)))
- {
- fastboot_fail(
- "Bogus chunk size for chunk type FILL");
- return;
- }
-
- fill_buf = (uint32_t *)
- memalign(ARCH_DMA_MINALIGN,
- ROUNDUP(info->blksz,
- ARCH_DMA_MINALIGN));
- if (!fill_buf)
- {
- fastboot_fail(
- "Malloc failed for: CHUNK_TYPE_FILL");
- return;
- }
-
- fill_val = *(uint32_t *)data;
- data = (char *) data + sizeof(uint32_t);
+ if (blk + blkcnt > info->start + info->size) {
+ printf("%s: Request would exceed partition size!\n",
+ __func__);
+ fastboot_fail("Request would exceed partition size!");
+ return;
+ }
- for (i = 0; i < (info->blksz / sizeof(fill_val)); i++)
- fill_buf[i] = fill_val;
+ for (i = 0; i < buffer->repeat; i++) {
+ unsigned int buffer_blk_cnt;
+ unsigned int buffer_blks;
- if (blk + blkcnt > info->start + info->size) {
- printf(
- "%s: Request would exceed partition size!\n",
- __func__);
- fastboot_fail(
- "Request would exceed partition size!");
- return;
- }
+ buffer_blk_cnt = buffer->length / storage->block_sz;
- for (i = 0; i < blkcnt; i++) {
- blks = dev_desc->block_write(dev_desc->dev,
- blk, 1, fill_buf);
- if (blks != 1) {
- printf(
- "%s: Write failed, block # " LBAFU "\n",
- __func__, blkcnt);
- fastboot_fail("flash write failure");
- free(fill_buf);
- return;
- }
- blk++;
- }
- bytes_written += blkcnt * info->blksz;
- total_blocks += chunk_data_sz / sparse_header->blk_sz;
-
- free(fill_buf);
- break;
-
- case CHUNK_TYPE_DONT_CARE:
- blk += blkcnt;
- total_blocks += chunk_header->chunk_sz;
- break;
-
- case CHUNK_TYPE_CRC32:
- if (chunk_header->total_sz !=
- sparse_header->chunk_hdr_sz)
- {
- fastboot_fail(
- "Bogus chunk size for chunk type Dont Care");
+ buffer_blks = dev_desc->block_write(dev_desc->dev, blk,
+ buffer_blk_cnt, buffer->data);
+ if (buffer_blks != buffer_blk_cnt) {
+ printf("%s: Write %d failed " LBAFU "\n",
+ __func__, i, buffer_blks);
+ fastboot_fail("flash write failure");
return;
}
- total_blocks += chunk_header->chunk_sz;
- data += chunk_data_sz;
- break;
- default:
- printf("%s: Unknown chunk type: %x\n", __func__,
- chunk_header->chunk_type);
- fastboot_fail("Unknown chunk type");
- return;
+ blk += buffer_blk_cnt;
+ bytes_written += buffer->length;
+ total_blocks += buffer_blk_cnt;
}
+
+ sparse_put_data_buffer(buffer);
}
debug("Wrote %d blocks, expected to write %d blocks\n",
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 04/13] sparse: Simplify multiple logic
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
` (2 preceding siblings ...)
2015-08-31 14:46 ` [U-Boot] [PATCH 03/13] sparse: Refactor chunk parsing function Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-04 17:20 ` Tom Rini
2015-08-31 14:46 ` [U-Boot] [PATCH 05/13] sparse: Implement storage abstraction Maxime Ripard
` (8 subsequent siblings)
12 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
To check the alignment of the image blocks to the storage blocks, the
current code uses a convoluted syntax, while a simple mod also does the
work.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
common/aboot.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/common/aboot.c b/common/aboot.c
index 65e633acfcb9..c8556d9b23f4 100644
--- a/common/aboot.c
+++ b/common/aboot.c
@@ -269,8 +269,7 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
}
/* verify sparse_header->blk_sz is an exact multiple of info->blksz */
- if (sparse_header->blk_sz !=
- (sparse_header->blk_sz & ~(info->blksz - 1))) {
+ if (sparse_header->blk_sz % info->blksz) {
printf("%s: Sparse image block size issue [%u]\n",
__func__, sparse_header->blk_sz);
fastboot_fail("sparse image block size issue");
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 04/13] sparse: Simplify multiple logic
2015-08-31 14:46 ` [U-Boot] [PATCH 04/13] sparse: Simplify multiple logic Maxime Ripard
@ 2015-09-04 17:20 ` Tom Rini
2015-09-06 11:27 ` Maxime Ripard
0 siblings, 1 reply; 65+ messages in thread
From: Tom Rini @ 2015-09-04 17:20 UTC (permalink / raw)
To: u-boot
On Mon, Aug 31, 2015 at 04:46:04PM +0200, Maxime Ripard wrote:
> To check the alignment of the image blocks to the storage blocks, the
> current code uses a convoluted syntax, while a simple mod also does the
> work.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> common/aboot.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/common/aboot.c b/common/aboot.c
> index 65e633acfcb9..c8556d9b23f4 100644
> --- a/common/aboot.c
> +++ b/common/aboot.c
> @@ -269,8 +269,7 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
> }
>
> /* verify sparse_header->blk_sz is an exact multiple of info->blksz */
> - if (sparse_header->blk_sz !=
> - (sparse_header->blk_sz & ~(info->blksz - 1))) {
> + if (sparse_header->blk_sz % info->blksz) {
So, sometimes we have convoluted syntax like this to avoid what ends up
as floating point math on 32bit platforms. Maybe this needs to be one
of the helpers in include/linux/math64.h ? Or is this really just
convoluted syntax? Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150904/9a9b9bcf/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 04/13] sparse: Simplify multiple logic
2015-09-04 17:20 ` Tom Rini
@ 2015-09-06 11:27 ` Maxime Ripard
2015-09-06 19:28 ` Tom Rini
0 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-09-06 11:27 UTC (permalink / raw)
To: u-boot
On Fri, Sep 04, 2015 at 01:20:38PM -0400, Tom Rini wrote:
> On Mon, Aug 31, 2015 at 04:46:04PM +0200, Maxime Ripard wrote:
>
> > To check the alignment of the image blocks to the storage blocks, the
> > current code uses a convoluted syntax, while a simple mod also does the
> > work.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> > common/aboot.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/common/aboot.c b/common/aboot.c
> > index 65e633acfcb9..c8556d9b23f4 100644
> > --- a/common/aboot.c
> > +++ b/common/aboot.c
> > @@ -269,8 +269,7 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
> > }
> >
> > /* verify sparse_header->blk_sz is an exact multiple of info->blksz */
> > - if (sparse_header->blk_sz !=
> > - (sparse_header->blk_sz & ~(info->blksz - 1))) {
> > + if (sparse_header->blk_sz % info->blksz) {
>
> So, sometimes we have convoluted syntax like this to avoid what ends up
> as floating point math on 32bit platforms.
Now that you speak of this, we did have some compilers on some
platforms that ended up generating floating point related failures on
our branch, but I didn't have the time to look into it.
However, I don't really know how we can end up with floating point
math with a mod, is this some optimisations done by gcc?
> Maybe this needs to be one of the helpers in include/linux/math64.h?
Probably then, yes.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150906/d5389adc/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 04/13] sparse: Simplify multiple logic
2015-09-06 11:27 ` Maxime Ripard
@ 2015-09-06 19:28 ` Tom Rini
2015-09-13 17:08 ` Maxime Ripard
0 siblings, 1 reply; 65+ messages in thread
From: Tom Rini @ 2015-09-06 19:28 UTC (permalink / raw)
To: u-boot
On Sun, Sep 06, 2015 at 01:27:20PM +0200, Maxime Ripard wrote:
> On Fri, Sep 04, 2015 at 01:20:38PM -0400, Tom Rini wrote:
> > On Mon, Aug 31, 2015 at 04:46:04PM +0200, Maxime Ripard wrote:
> >
> > > To check the alignment of the image blocks to the storage blocks, the
> > > current code uses a convoluted syntax, while a simple mod also does the
> > > work.
> > >
> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > ---
> > > common/aboot.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/common/aboot.c b/common/aboot.c
> > > index 65e633acfcb9..c8556d9b23f4 100644
> > > --- a/common/aboot.c
> > > +++ b/common/aboot.c
> > > @@ -269,8 +269,7 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
> > > }
> > >
> > > /* verify sparse_header->blk_sz is an exact multiple of info->blksz */
> > > - if (sparse_header->blk_sz !=
> > > - (sparse_header->blk_sz & ~(info->blksz - 1))) {
> > > + if (sparse_header->blk_sz % info->blksz) {
> >
> > So, sometimes we have convoluted syntax like this to avoid what ends up
> > as floating point math on 32bit platforms.
>
> Now that you speak of this, we did have some compilers on some
> platforms that ended up generating floating point related failures on
> our branch, but I didn't have the time to look into it.
>
> However, I don't really know how we can end up with floating point
> math with a mod, is this some optimisations done by gcc?
My fuzz recollection is that GCC on 32bit ARM when it defaults to a
hardfloat ABI "knows" that it is "safe" to use the floating point
registers to do 64bit division rather than shifting and so forth.
> > Maybe this needs to be one of the helpers in include/linux/math64.h?
>
> Probably then, yes.
Yeah, this is it. My first guess is that if you look at
9e374e7b729dc9f68be89cd3e3b1d4d48c768ecf you'll find an example that
matches the problem here and what the right helper/code chagnes are.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150906/18a1602a/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 04/13] sparse: Simplify multiple logic
2015-09-06 19:28 ` Tom Rini
@ 2015-09-13 17:08 ` Maxime Ripard
0 siblings, 0 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-09-13 17:08 UTC (permalink / raw)
To: u-boot
On Sun, Sep 06, 2015 at 03:28:34PM -0400, Tom Rini wrote:
> On Sun, Sep 06, 2015 at 01:27:20PM +0200, Maxime Ripard wrote:
> > On Fri, Sep 04, 2015 at 01:20:38PM -0400, Tom Rini wrote:
> > > On Mon, Aug 31, 2015 at 04:46:04PM +0200, Maxime Ripard wrote:
> > >
> > > > To check the alignment of the image blocks to the storage blocks, the
> > > > current code uses a convoluted syntax, while a simple mod also does the
> > > > work.
> > > >
> > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > > ---
> > > > common/aboot.c | 3 +--
> > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/common/aboot.c b/common/aboot.c
> > > > index 65e633acfcb9..c8556d9b23f4 100644
> > > > --- a/common/aboot.c
> > > > +++ b/common/aboot.c
> > > > @@ -269,8 +269,7 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
> > > > }
> > > >
> > > > /* verify sparse_header->blk_sz is an exact multiple of info->blksz */
> > > > - if (sparse_header->blk_sz !=
> > > > - (sparse_header->blk_sz & ~(info->blksz - 1))) {
> > > > + if (sparse_header->blk_sz % info->blksz) {
> > >
> > > So, sometimes we have convoluted syntax like this to avoid what ends up
> > > as floating point math on 32bit platforms.
> >
> > Now that you speak of this, we did have some compilers on some
> > platforms that ended up generating floating point related failures on
> > our branch, but I didn't have the time to look into it.
> >
> > However, I don't really know how we can end up with floating point
> > math with a mod, is this some optimisations done by gcc?
>
> My fuzz recollection is that GCC on 32bit ARM when it defaults to a
> hardfloat ABI "knows" that it is "safe" to use the floating point
> registers to do 64bit division rather than shifting and so forth.
Hmm, ok, I see.
> > > Maybe this needs to be one of the helpers in include/linux/math64.h?
> >
> > Probably then, yes.
>
> Yeah, this is it. My first guess is that if you look at
> 9e374e7b729dc9f68be89cd3e3b1d4d48c768ecf you'll find an example that
> matches the problem here and what the right helper/code chagnes are.
I'll look at this and use it then.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150913/96c1e172/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 05/13] sparse: Implement storage abstraction
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
` (3 preceding siblings ...)
2015-08-31 14:46 ` [U-Boot] [PATCH 04/13] sparse: Simplify multiple logic Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-04 17:20 ` Tom Rini
2015-08-31 14:46 ` [U-Boot] [PATCH 06/13] fastboot: Move fastboot response functions to fastboot core Maxime Ripard
` (7 subsequent siblings)
12 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
The current sparse image parser relies heavily on the MMC layer, and
doesn't allow any other kind of storage medium to be used.
Rework the parser to support any kind of storage medium, as long as there
is an implementation for it.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
common/aboot.c | 51 +++++++++++++++++++++++++++++++++++----------------
common/fb_mmc.c | 39 +++++++++++++++++++++++++++++++++++----
include/aboot.h | 14 ++++++++++++++
3 files changed, 84 insertions(+), 20 deletions(-)
diff --git a/common/aboot.c b/common/aboot.c
index c8556d9b23f4..18ff30ee6d11 100644
--- a/common/aboot.c
+++ b/common/aboot.c
@@ -42,6 +42,13 @@
#include <part.h>
#include <sparse_format.h>
+typedef struct sparse_buffer {
+ void *data;
+ u32 length;
+ u32 repeat;
+ u16 type;
+} sparse_buffer_t;
+
static unsigned int sparse_get_chunk_data_size(sparse_header_t *sparse,
chunk_header_t *chunk)
{
@@ -247,13 +254,11 @@ static void sparse_put_data_buffer(sparse_buffer_t *buffer)
free(buffer);
}
-void write_sparse_image(block_dev_desc_t *dev_desc,
- disk_partition_t *info, const char *part_name,
- void *data, unsigned sz)
+void store_sparse_image(sparse_storage_t *storage,
+ void *storage_priv, void *data)
{
- lbaint_t blk;
- lbaint_t blkcnt;
- lbaint_t blks;
+ uint32_t blk;
+ uint32_t blkcnt;
uint32_t bytes_written = 0;
unsigned int chunk;
sparse_header_t *sparse_header;
@@ -262,14 +267,25 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
uint32_t total_blocks = 0;
int i;
+ debug("=== Storage ===\n");
+ debug("name: %s\n", storage->name);
+ debug("block_size: 0x%x\n", storage->block_sz);
+ debug("start: 0x%x\n", storage->start);
+ debug("size: 0x%x\n", storage->size);
+ debug("write: 0x%p\n", storage->write);
+ debug("priv: 0x%p\n", storage_priv);
+
sparse_header = sparse_parse_header(&data);
if (!sparse_header) {
fastboot_fail("sparse header issue\n");
return;
}
- /* verify sparse_header->blk_sz is an exact multiple of info->blksz */
- if (sparse_header->blk_sz % info->blksz) {
+ /*
+ * Verify that the sparse block size is a multiple of our
+ * storage backend block size
+ */
+ 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");
@@ -279,7 +295,7 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
puts("Flashing Sparse Image\n");
/* Start processing chunks */
- blk = info->start;
+ blk = storage->start;
for (chunk = 0; chunk < sparse_header->total_chunks; chunk++) {
chunk_header = sparse_parse_chunk(sparse_header, &data);
if (!chunk_header) {
@@ -289,13 +305,13 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
/* Retrieve the buffer we're going to write */
buffer = sparse_get_data_buffer(sparse_header, chunk_header,
- info->blksz, &data);
+ storage->block_sz, &data);
if (!buffer)
continue;
- blkcnt = (buffer->length / info->blksz) * buffer->repeat;
+ blkcnt = (buffer->length / storage->block_sz) * buffer->repeat;
- if (blk + blkcnt > info->start + info->size) {
+ if (blk + blkcnt > storage->start + storage->size) {
printf("%s: Request would exceed partition size!\n",
__func__);
fastboot_fail("Request would exceed partition size!");
@@ -308,10 +324,12 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
buffer_blk_cnt = buffer->length / storage->block_sz;
- buffer_blks = dev_desc->block_write(dev_desc->dev, blk,
- buffer_blk_cnt, buffer->data);
+ buffer_blks = storage->write(storage,
+ storage_priv,
+ blk, buffer_blk_cnt,
+ buffer->data);
if (buffer_blks != buffer_blk_cnt) {
- printf("%s: Write %d failed " LBAFU "\n",
+ printf("%s: Write %d failed %d\n",
__func__, i, buffer_blks);
fastboot_fail("flash write failure");
return;
@@ -327,7 +345,8 @@ void write_sparse_image(block_dev_desc_t *dev_desc,
debug("Wrote %d blocks, expected to write %d blocks\n",
total_blocks, sparse_header->total_blks);
- printf("........ wrote %u bytes to '%s'\n", bytes_written, part_name);
+ printf("........ wrote %u bytes to '%s'\n", bytes_written,
+ storage->name);
if (total_blocks != sparse_header->total_blks)
fastboot_fail("sparse image write failure");
diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index 0c48cf929f8f..a44627c00060 100644
--- a/common/fb_mmc.c
+++ b/common/fb_mmc.c
@@ -33,6 +33,10 @@ void fastboot_okay(const char *s)
strncat(response_str, s, RESPONSE_LEN - 4 - 1);
}
+struct fb_mmc_sparse {
+ block_dev_desc_t *dev_desc;
+};
+
static int get_partition_info_efi_by_name_or_alias(block_dev_desc_t *dev_desc,
const char *name, disk_partition_t *info)
{
@@ -55,6 +59,19 @@ static int get_partition_info_efi_by_name_or_alias(block_dev_desc_t *dev_desc,
return ret;
}
+
+static unsigned int fb_mmc_sparse_write(struct sparse_storage *storage,
+ void *priv,
+ unsigned int offset,
+ unsigned int size,
+ char *data)
+{
+ struct fb_mmc_sparse *sparse = priv;
+ block_dev_desc_t *dev_desc = sparse->dev_desc;
+
+ return dev_desc->block_write(dev_desc->dev, offset, size, data);
+}
+
static void write_raw_image(block_dev_desc_t *dev_desc, disk_partition_t *info,
const char *part_name, void *buffer,
unsigned int download_bytes)
@@ -126,12 +143,26 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
return;
}
- if (is_sparse_image(download_buffer))
- write_sparse_image(dev_desc, &info, cmd, download_buffer,
- download_bytes);
- else
+ if (is_sparse_image(download_buffer)) {
+ struct fb_mmc_sparse sparse_priv;
+ sparse_storage_t sparse;
+
+ sparse_priv.dev_desc = dev_desc;
+
+ sparse.block_sz = info.blksz;
+ sparse.start = info.start;
+ sparse.size = info.size;
+ sparse.name = cmd;
+ sparse.write = fb_mmc_sparse_write;
+
+ printf("Flashing sparse image at offset " LBAFU "\n",
+ info.start);
+
+ store_sparse_image(&sparse, &sparse_priv, download_buffer);
+ } else {
write_raw_image(dev_desc, &info, cmd, download_buffer,
download_bytes);
+ }
}
void fb_mmc_erase(const char *cmd, char *response)
diff --git a/include/aboot.h b/include/aboot.h
index 30e4d36df8ba..162d81da6d9d 100644
--- a/include/aboot.h
+++ b/include/aboot.h
@@ -9,6 +9,17 @@
#define ROUNDUP(x, y) (((x) + ((y) - 1)) & ~((y) - 1))
+typedef struct sparse_storage {
+ unsigned int block_sz;
+ unsigned int start;
+ unsigned int size;
+ const char *name;
+
+ unsigned int (*write)(struct sparse_storage *storage, void *priv,
+ unsigned int offset, unsigned int size,
+ char *data);
+} sparse_storage_t;
+
void fastboot_fail(const char *s);
void fastboot_okay(const char *s);
@@ -26,3 +37,6 @@ static inline int is_sparse_image(void *buf)
void write_sparse_image(block_dev_desc_t *dev_desc,
disk_partition_t *info, const char *part_name,
void *data, unsigned sz);
+
+void store_sparse_image(sparse_storage_t *storage, void *storage_priv,
+ void *data);
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 06/13] fastboot: Move fastboot response functions to fastboot core
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
` (4 preceding siblings ...)
2015-08-31 14:46 ` [U-Boot] [PATCH 05/13] sparse: Implement storage abstraction Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-04 17:20 ` Tom Rini
2015-08-31 14:46 ` [U-Boot] [PATCH 07/13] fastboot: Implement NAND backend Maxime Ripard
` (6 subsequent siblings)
12 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
common/aboot.c | 14 ++++++--------
common/fb_mmc.c | 43 ++++++++++++++---------------------------
drivers/usb/gadget/f_fastboot.c | 27 ++++++++++++++++++--------
include/aboot.h | 3 ---
include/fastboot.h | 22 +++++++++++++++++++++
5 files changed, 62 insertions(+), 47 deletions(-)
create mode 100644 include/fastboot.h
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;
}
diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index a44627c00060..0d6bcbf1f78b 100644
--- a/common/fb_mmc.c
+++ b/common/fb_mmc.c
@@ -6,6 +6,7 @@
#include <config.h>
#include <common.h>
+#include <fastboot.h>
#include <fb_mmc.h>
#include <part.h>
#include <aboot.h>
@@ -16,23 +17,8 @@
#define CONFIG_FASTBOOT_GPT_NAME GPT_ENTRY_NAME
#endif
-/* The 64 defined bytes plus the '\0' */
-#define RESPONSE_LEN (64 + 1)
-
static char *response_str;
-void fastboot_fail(const char *s)
-{
- strncpy(response_str, "FAIL\0", 5);
- strncat(response_str, s, RESPONSE_LEN - 4 - 1);
-}
-
-void fastboot_okay(const char *s)
-{
- strncpy(response_str, "OKAY\0", 5);
- strncat(response_str, s, RESPONSE_LEN - 4 - 1);
-}
-
struct fb_mmc_sparse {
block_dev_desc_t *dev_desc;
};
@@ -85,7 +71,7 @@ static void write_raw_image(block_dev_desc_t *dev_desc, disk_partition_t *info,
if (blkcnt > info->size) {
error("too large for partition: '%s'\n", part_name);
- fastboot_fail("too large for partition");
+ fastboot_fail(response_str, "too large for partition");
return;
}
@@ -95,13 +81,13 @@ static void write_raw_image(block_dev_desc_t *dev_desc, disk_partition_t *info,
buffer);
if (blks != blkcnt) {
error("failed writing to device %d\n", dev_desc->dev);
- fastboot_fail("failed writing to device");
+ fastboot_fail(response_str, "failed writing to device");
return;
}
printf("........ wrote " LBAFU " bytes to '%s'\n", blkcnt * info->blksz,
part_name);
- fastboot_okay("");
+ fastboot_okay(response_str, "");
}
void fb_mmc_flash_write(const char *cmd, void *download_buffer,
@@ -116,7 +102,7 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
dev_desc = get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
error("invalid mmc device\n");
- fastboot_fail("invalid mmc device");
+ fastboot_fail(response_str, "invalid mmc device");
return;
}
@@ -126,20 +112,21 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
if (is_valid_gpt_buf(dev_desc, download_buffer)) {
printf("%s: invalid GPT - refusing to write to flash\n",
__func__);
- fastboot_fail("invalid GPT partition");
+ fastboot_fail(response_str, "invalid GPT partition");
return;
}
if (write_mbr_and_gpt_partitions(dev_desc, download_buffer)) {
printf("%s: writing GPT partitions failed\n", __func__);
- fastboot_fail("writing GPT partitions failed");
+ fastboot_fail(response_str,
+ "writing GPT partitions failed");
return;
}
printf("........ success\n");
- fastboot_okay("");
+ fastboot_okay(response_str, "");
return;
} else if (get_partition_info_efi_by_name_or_alias(dev_desc, cmd, &info)) {
error("cannot find partition: '%s'\n", cmd);
- fastboot_fail("cannot find partition");
+ fastboot_fail(response_str, "cannot find partition");
return;
}
@@ -175,7 +162,7 @@ void fb_mmc_erase(const char *cmd, char *response)
if (mmc == NULL) {
error("invalid mmc device");
- fastboot_fail("invalid mmc device");
+ fastboot_fail(response_str, "invalid mmc device");
return;
}
@@ -185,14 +172,14 @@ void fb_mmc_erase(const char *cmd, char *response)
dev_desc = get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) {
error("invalid mmc device");
- fastboot_fail("invalid mmc device");
+ fastboot_fail(response_str, "invalid mmc device");
return;
}
ret = get_partition_info_efi_by_name_or_alias(dev_desc, cmd, &info);
if (ret) {
error("cannot find partition: '%s'", cmd);
- fastboot_fail("cannot find partition");
+ fastboot_fail(response_str, "cannot find partition");
return;
}
@@ -211,11 +198,11 @@ void fb_mmc_erase(const char *cmd, char *response)
blks = dev_desc->block_erase(dev_desc->dev, blks_start, blks_size);
if (blks != blks_size) {
error("failed erasing from device %d", dev_desc->dev);
- fastboot_fail("failed erasing from device");
+ fastboot_fail(response_str, "failed erasing from device");
return;
}
printf("........ erased " LBAFU " bytes from '%s'\n",
blks_size * info.blksz, cmd);
- fastboot_okay("");
+ fastboot_okay(response_str, "");
}
diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index ca01a018b5d1..5703decfd360 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -13,6 +13,7 @@
#include <config.h>
#include <common.h>
#include <errno.h>
+#include <fastboot.h>
#include <malloc.h>
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
@@ -34,9 +35,6 @@
#define RX_ENDPOINT_MAXIMUM_PACKET_SIZE_1_1 (0x0040)
#define TX_ENDPOINT_MAXIMUM_PACKET_SIZE (0x0040)
-/* The 64 defined bytes plus \0 */
-#define RESPONSE_LEN (64 + 1)
-
#define EP_BUFFER_SIZE 4096
struct f_fastboot {
@@ -125,6 +123,19 @@ static struct usb_gadget_strings *fastboot_strings[] = {
static void rx_handler_command(struct usb_ep *ep, struct usb_request *req);
static int strcmp_l1(const char *s1, const char *s2);
+
+void fastboot_fail(char *response, const char *reason)
+{
+ strncpy(response, "FAIL\0", 5);
+ strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
+}
+
+void fastboot_okay(char *response, const char *reason)
+{
+ strncpy(response, "OKAY\0", 5);
+ strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1);
+}
+
static void fastboot_complete(struct usb_ep *ep, struct usb_request *req)
{
int status = req->status;
@@ -358,7 +369,7 @@ static int strcmp_l1(const char *s1, const char *s2)
static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
{
char *cmd = req->buf;
- char response[RESPONSE_LEN];
+ char response[FASTBOOT_RESPONSE_LEN];
const char *s;
size_t chars_left;
@@ -415,7 +426,7 @@ static unsigned int rx_bytes_expected(unsigned int maxpacket)
#define BYTES_PER_DOT 0x20000
static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
{
- char response[RESPONSE_LEN];
+ char response[FASTBOOT_RESPONSE_LEN];
unsigned int transfer_size = download_size - download_bytes;
const unsigned char *buffer = req->buf;
unsigned int buffer_size = req->actual;
@@ -472,7 +483,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
static void cb_download(struct usb_ep *ep, struct usb_request *req)
{
char *cmd = req->buf;
- char response[RESPONSE_LEN];
+ char response[FASTBOOT_RESPONSE_LEN];
unsigned int max;
strsep(&cmd, ":");
@@ -533,7 +544,7 @@ static void cb_continue(struct usb_ep *ep, struct usb_request *req)
static void cb_flash(struct usb_ep *ep, struct usb_request *req)
{
char *cmd = req->buf;
- char response[RESPONSE_LEN];
+ char response[FASTBOOT_RESPONSE_LEN];
strsep(&cmd, ":");
if (!cmd) {
@@ -577,7 +588,7 @@ static void cb_oem(struct usb_ep *ep, struct usb_request *req)
static void cb_erase(struct usb_ep *ep, struct usb_request *req)
{
char *cmd = req->buf;
- char response[RESPONSE_LEN];
+ char response[FASTBOOT_RESPONSE_LEN];
strsep(&cmd, ":");
if (!cmd) {
diff --git a/include/aboot.h b/include/aboot.h
index 162d81da6d9d..55da8336f20d 100644
--- a/include/aboot.h
+++ b/include/aboot.h
@@ -20,9 +20,6 @@ typedef struct sparse_storage {
char *data);
} sparse_storage_t;
-void fastboot_fail(const char *s);
-void fastboot_okay(const char *s);
-
static inline int is_sparse_image(void *buf)
{
sparse_header_t *s_header = (sparse_header_t *)buf;
diff --git a/include/fastboot.h b/include/fastboot.h
new file mode 100644
index 000000000000..db826d20bfef
--- /dev/null
+++ b/include/fastboot.h
@@ -0,0 +1,22 @@
+/*
+ * (C) Copyright 2008 - 2009
+ * Windriver, <www.windriver.com>
+ * Tom Rix <Tom.Rix@windriver.com>
+ *
+ * Copyright 2011 Sebastian Andrzej Siewior <bigeasy@linutronix.de>
+ *
+ * Copyright 2014 Linaro, Ltd.
+ * Rob Herring <robh@kernel.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+#ifndef _FASTBOOT_H_
+#define _FASTBOOT_H_
+
+/* The 64 defined bytes plus \0 */
+#define FASTBOOT_RESPONSE_LEN (64 + 1)
+
+void fastboot_fail(char *response, const char *reason);
+void fastboot_okay(char *response, const char *reason);
+
+#endif /* _FASTBOOT_H_ */
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 06/13] fastboot: Move fastboot response functions to fastboot core
2015-08-31 14:46 ` [U-Boot] [PATCH 06/13] fastboot: Move fastboot response functions to fastboot core Maxime Ripard
@ 2015-09-04 17:20 ` Tom Rini
2015-09-06 16:11 ` Maxime Ripard
0 siblings, 1 reply; 65+ messages in thread
From: Tom Rini @ 2015-09-04 17:20 UTC (permalink / raw)
To: u-boot
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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150904/f4427325/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 06/13] fastboot: Move fastboot response functions to fastboot core
2015-09-04 17:20 ` Tom Rini
@ 2015-09-06 16:11 ` Maxime Ripard
2015-09-06 19:43 ` Tom Rini
0 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-09-06 16:11 UTC (permalink / raw)
To: u-boot
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?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150906/3f9b2ab8/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 06/13] fastboot: Move fastboot response functions to fastboot core
2015-09-06 16:11 ` Maxime Ripard
@ 2015-09-06 19:43 ` Tom Rini
0 siblings, 0 replies; 65+ messages in thread
From: Tom Rini @ 2015-09-06 19:43 UTC (permalink / raw)
To: u-boot
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150906/799efb0f/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 07/13] fastboot: Implement NAND backend
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
` (5 preceding siblings ...)
2015-08-31 14:46 ` [U-Boot] [PATCH 06/13] fastboot: Move fastboot response functions to fastboot core Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-04 17:20 ` Tom Rini
2015-09-10 7:41 ` Boris Brezillon
2015-08-31 14:46 ` [U-Boot] [PATCH 08/13] fastboot: nand: Add pre erase and write hooks Maxime Ripard
` (5 subsequent siblings)
12 siblings, 2 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
So far the fastboot code was only supporting MMC-backed devices for its
flashing operations (flash and erase).
Add a storage backend for NAND-backed devices.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
common/Makefile | 7 +-
common/fb_nand.c | 198 ++++++++++++++++++++++++++++++++++++++++
drivers/usb/gadget/f_fastboot.c | 12 ++-
include/fb_nand.h | 10 ++
4 files changed, 225 insertions(+), 2 deletions(-)
create mode 100644 common/fb_nand.c
create mode 100644 include/fb_nand.h
diff --git a/common/Makefile b/common/Makefile
index dc82433e90de..20e8027a8913 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -271,10 +271,15 @@ obj-y += memsize.o
obj-y += stdio.o
# This option is not just y/n - it can have a numeric value
-ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
+ifdef CONFIG_FASTBOOT_FLASH
obj-y += aboot.o
+ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
obj-y += fb_mmc.o
endif
+ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
+obj-y += fb_nand.o
+endif
+endif
obj-$(CONFIG_CMD_BLOB) += cmd_blob.o
diff --git a/common/fb_nand.c b/common/fb_nand.c
new file mode 100644
index 000000000000..0e1d0603dbcf
--- /dev/null
+++ b/common/fb_nand.c
@@ -0,0 +1,198 @@
+/*
+ * Copyright 2014 Broadcom Corporation.
+ * Copyright 2015 Free Electrons.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <config.h>
+#include <common.h>
+
+#include <aboot.h>
+#include <fastboot.h>
+#include <sparse_format.h>
+
+#include <linux/mtd/mtd.h>
+#include <jffs2/jffs2.h>
+#include <nand.h>
+
+static char *response_str;
+
+struct fb_nand_sparse {
+ nand_info_t *nand;
+ struct part_info *part;
+};
+
+static int fb_nand_lookup(const char *partname, char *response,
+ nand_info_t **nand,
+ struct part_info **part)
+{
+ struct mtd_device *dev;
+ int ret;
+ u8 pnum;
+
+ ret = mtdparts_init();
+ if (ret) {
+ error("Cannot initialize MTD partitions\n");
+ fastboot_fail(response_str, "cannot init mtdparts");
+ return ret;
+ }
+
+ ret = find_dev_and_part(partname, &dev, &pnum, part);
+ if (ret) {
+ error("cannot find partition: '%s'", partname);
+ fastboot_fail(response_str, "cannot find partition");
+ return ret;
+ }
+
+ if (dev->id->type != MTD_DEV_TYPE_NAND) {
+ error("partition '%s' is not stored on a NAND device",
+ partname);
+ fastboot_fail(response_str, "not a NAND device");
+ return -EINVAL;
+ }
+
+ *nand = &nand_info[dev->id->num];
+
+ return 0;
+}
+
+static int _fb_nand_erase(nand_info_t *nand, struct part_info *part)
+{
+ nand_erase_options_t opts;
+ int ret;
+
+ memset(&opts, 0, sizeof(opts));
+ opts.offset = part->offset;
+ opts.length = part->size;
+ opts.quiet = 1;
+
+ printf("Erasing blocks 0x%llx to 0x%llx\n",
+ part->offset, part->offset + part->size);
+
+ ret = nand_erase_opts(nand, &opts);
+ if (ret)
+ return ret;
+
+ printf("........ erased 0x%llx bytes from '%s'\n",
+ part->size, part->name);
+
+ return 0;
+}
+
+static int _fb_nand_write(nand_info_t *nand, struct part_info *part,
+ void *buffer, unsigned int offset,
+ unsigned int length)
+{
+ int flags = WITH_WR_VERIFY;
+ int ret;
+
+#ifdef CONFIG_FASTBOOT_FLASH_NAND_TRIMFFS
+ flags |= WITH_DROP_FFS;
+#endif
+ ret = nand_write_skip_bad(nand, offset, &length, NULL,
+ part->size - (offset - part->offset),
+ buffer, flags);
+
+ if (ret)
+ return ret;
+
+ return ret;
+}
+
+static unsigned int fb_nand_sparse_write(struct sparse_storage *storage,
+ void *priv,
+ unsigned int offset,
+ unsigned int size,
+ char *data)
+{
+ struct fb_nand_sparse *sparse = priv;
+
+ _fb_nand_write(sparse->nand, sparse->part, data,
+ offset * storage->block_sz,
+ size * storage->block_sz);
+
+ return size;
+}
+
+void fb_nand_flash_write(const char *partname, void *download_buffer,
+ unsigned int download_bytes, char *response)
+{
+ struct part_info *part;
+ nand_info_t *nand = NULL;
+ int ret;
+
+ /* initialize the response buffer */
+ response_str = response;
+
+ ret = fb_nand_lookup(partname, response, &nand, &part);
+ if (ret) {
+ error("invalid NAND device");
+ fastboot_fail(response_str, "invalid NAND device");
+ return;
+ }
+
+
+ ret = _fb_nand_erase(nand, part);
+ if (ret) {
+ error("failed erasing from device %s", nand->name);
+ fastboot_fail(response_str, "failed erasing from device");
+ return;
+ }
+
+ if (is_sparse_image(download_buffer)) {
+ struct fb_nand_sparse sparse_priv;
+ sparse_storage_t sparse;
+
+ sparse_priv.nand = nand;
+ sparse_priv.part = part;
+
+ sparse.block_sz = nand->writesize;
+ sparse.start = part->offset / sparse.block_sz;
+ sparse.size = part->size / sparse.block_sz;
+ sparse.name = part->name;
+ sparse.write = fb_nand_sparse_write;
+
+ printf("Flashing sparse image at offset 0x%llx\n",
+ part->offset);
+
+ store_sparse_image(&sparse, &sparse_priv, download_buffer);
+ } else {
+ printf("Flashing raw image at offset 0x%llx\n",
+ part->offset);
+
+ _fb_nand_write(nand, part, download_buffer, part->offset,
+ download_bytes);
+
+ printf("........ wrote %u bytes to '%s'\n",
+ download_bytes, part->name);
+ }
+
+ fastboot_okay("");
+}
+
+void fb_nand_erase(const char *partname, char *response)
+{
+ struct part_info *part;
+ nand_info_t *nand = NULL;
+ int ret;
+
+ /* initialize the response buffer */
+ response_str = response;
+
+ ret = fb_nand_lookup(partname, response, &nand, &part);
+ if (ret) {
+ error("invalid NAND device");
+ fastboot_fail(response_str, "invalid NAND device");
+ return;
+ }
+
+ ret = _fb_nand_erase(nand, part);
+ if (ret) {
+ error("failed erasing from device %s", nand->name);
+ fastboot_fail(response_str, "failed erasing from device");
+ return;
+ }
+
+ fastboot_okay("");
+}
diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index 5703decfd360..866d426732fa 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -24,6 +24,9 @@
#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
#include <fb_mmc.h>
#endif
+#ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
+#include <fb_nand.h>
+#endif
#define FASTBOOT_VERSION "0.4"
@@ -558,6 +561,10 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req)
fb_mmc_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR,
download_bytes, response);
#endif
+#ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
+ fb_nand_flash_write(cmd, (void *)CONFIG_USB_FASTBOOT_BUF_ADDR,
+ download_bytes, response);
+#endif
fastboot_tx_write_str(response);
}
#endif
@@ -565,7 +572,7 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req)
static void cb_oem(struct usb_ep *ep, struct usb_request *req)
{
char *cmd = req->buf;
-#ifdef CONFIG_FASTBOOT_FLASH
+#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
if (strncmp("format", cmd + 4, 6) == 0) {
char cmdbuf[32];
sprintf(cmdbuf, "gpt write mmc %x $partitions",
@@ -602,6 +609,9 @@ static void cb_erase(struct usb_ep *ep, struct usb_request *req)
#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
fb_mmc_erase(cmd, response);
#endif
+#ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV
+ fb_nand_erase(cmd, response);
+#endif
fastboot_tx_write_str(response);
}
#endif
diff --git a/include/fb_nand.h b/include/fb_nand.h
new file mode 100644
index 000000000000..88bdf3690de9
--- /dev/null
+++ b/include/fb_nand.h
@@ -0,0 +1,10 @@
+/*
+ * Copyright 2014 Broadcom Corporation.
+ * Copyright 2015 Free Electrons.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+void fb_nand_flash_write(const char *cmd, void *download_buffer,
+ unsigned int download_bytes, char *response);
+void fb_nand_erase(const char *cmd, char *response);
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 07/13] fastboot: Implement NAND backend
2015-08-31 14:46 ` [U-Boot] [PATCH 07/13] fastboot: Implement NAND backend Maxime Ripard
@ 2015-09-04 17:20 ` Tom Rini
2015-09-06 15:57 ` Maxime Ripard
2015-09-10 7:41 ` Boris Brezillon
1 sibling, 1 reply; 65+ messages in thread
From: Tom Rini @ 2015-09-04 17:20 UTC (permalink / raw)
To: u-boot
On Mon, Aug 31, 2015 at 04:46:07PM +0200, Maxime Ripard wrote:
> So far the fastboot code was only supporting MMC-backed devices for its
> flashing operations (flash and erase).
>
> Add a storage backend for NAND-backed devices.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
In general:
Reviewed-by: Tom Rini <trini@konsulko.com>
But would it be possible to do this similar to how DFU is where we can
have NAND and MMC and whatever else built-in and then use the right one
as instructed by the user? ie change the command from 'fastboot
<USB_controller>' to 'fastboot <USB_controller> [interface]' (or maybe
have to make interface required, not sure). Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150904/c0a5219f/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 07/13] fastboot: Implement NAND backend
2015-09-04 17:20 ` Tom Rini
@ 2015-09-06 15:57 ` Maxime Ripard
2015-09-06 19:41 ` Tom Rini
0 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-09-06 15:57 UTC (permalink / raw)
To: u-boot
On Fri, Sep 04, 2015 at 01:20:49PM -0400, Tom Rini wrote:
> On Mon, Aug 31, 2015 at 04:46:07PM +0200, Maxime Ripard wrote:
>
> > So far the fastboot code was only supporting MMC-backed devices for its
> > flashing operations (flash and erase).
> >
> > Add a storage backend for NAND-backed devices.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> In general:
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> But would it be possible to do this similar to how DFU is where we can
> have NAND and MMC and whatever else built-in and then use the right one
> as instructed by the user? ie change the command from 'fastboot
> <USB_controller>' to 'fastboot <USB_controller> [interface]' (or maybe
> have to make interface required, not sure). Thanks!
As in something like "fastboot 0 mmc 0" ?
Sounds good, however, since fastboot only refers to partitions by
name, we could simply allow mix and matching them. What do you think?
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150906/021bb85f/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 07/13] fastboot: Implement NAND backend
2015-09-06 15:57 ` Maxime Ripard
@ 2015-09-06 19:41 ` Tom Rini
0 siblings, 0 replies; 65+ messages in thread
From: Tom Rini @ 2015-09-06 19:41 UTC (permalink / raw)
To: u-boot
On Sun, Sep 06, 2015 at 05:57:25PM +0200, Maxime Ripard wrote:
> On Fri, Sep 04, 2015 at 01:20:49PM -0400, Tom Rini wrote:
> > On Mon, Aug 31, 2015 at 04:46:07PM +0200, Maxime Ripard wrote:
> >
> > > So far the fastboot code was only supporting MMC-backed devices for its
> > > flashing operations (flash and erase).
> > >
> > > Add a storage backend for NAND-backed devices.
> > >
> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >
> > In general:
> >
> > Reviewed-by: Tom Rini <trini@konsulko.com>
> >
> > But would it be possible to do this similar to how DFU is where we can
> > have NAND and MMC and whatever else built-in and then use the right one
> > as instructed by the user? ie change the command from 'fastboot
> > <USB_controller>' to 'fastboot <USB_controller> [interface]' (or maybe
> > have to make interface required, not sure). Thanks!
>
> As in something like "fastboot 0 mmc 0" ?
>
> Sounds good, however, since fastboot only refers to partitions by
> name, we could simply allow mix and matching them. What do you think?
Sure, sounds good, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150906/f4beeffb/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 07/13] fastboot: Implement NAND backend
2015-08-31 14:46 ` [U-Boot] [PATCH 07/13] fastboot: Implement NAND backend Maxime Ripard
2015-09-04 17:20 ` Tom Rini
@ 2015-09-10 7:41 ` Boris Brezillon
1 sibling, 0 replies; 65+ messages in thread
From: Boris Brezillon @ 2015-09-10 7:41 UTC (permalink / raw)
To: u-boot
Hi Maxime,
On Mon, 31 Aug 2015 16:46:07 +0200
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> So far the fastboot code was only supporting MMC-backed devices for its
> flashing operations (flash and erase).
>
> Add a storage backend for NAND-backed devices.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
[...]
> +
> +static int _fb_nand_write(nand_info_t *nand, struct part_info *part,
> + void *buffer, unsigned int offset,
> + unsigned int length)
> +{
> + int flags = WITH_WR_VERIFY;
> + int ret;
> +
> +#ifdef CONFIG_FASTBOOT_FLASH_NAND_TRIMFFS
> + flags |= WITH_DROP_FFS;
> +#endif
> + ret = nand_write_skip_bad(nand, offset, &length, NULL,
> + part->size - (offset - part->offset),
> + buffer, flags);
Hm, you seem to ignore skipped blocks here (the 'actual' parameter is
NULL), which means you won't adapt the offset accordingly and will
likely override some data if you appear to have bad blocks in the
section you're writing with the sparse method.
> +
> + if (ret)
> + return ret;
> +
> + return ret;
> +}
> +
> +static unsigned int fb_nand_sparse_write(struct sparse_storage *storage,
> + void *priv,
> + unsigned int offset,
Maybe you should pass a pointer here, so that you can properly update it
with the number of skipped blocks (the same goes for the _fb_nand_write
function).
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 08/13] fastboot: nand: Add pre erase and write hooks
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
` (6 preceding siblings ...)
2015-08-31 14:46 ` [U-Boot] [PATCH 07/13] fastboot: Implement NAND backend Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-04 17:20 ` Tom Rini
2015-08-31 14:46 ` [U-Boot] [PATCH 09/13] sparse: Rename the file and header Maxime Ripard
` (4 subsequent siblings)
12 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
Some devices might need to do some per-partition initialization
(ECC/Randomizer settings change for example) before actually accessing it.
Add some hooks before the write and erase operations to let the boards
define what they need to do if needed.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
common/fb_nand.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/common/fb_nand.c b/common/fb_nand.c
index 0e1d0603dbcf..1aab1e88f9c0 100644
--- a/common/fb_nand.c
+++ b/common/fb_nand.c
@@ -23,6 +23,16 @@ struct fb_nand_sparse {
struct part_info *part;
};
+__weak int board_fastboot_erase_partition_setup(char *name)
+{
+ return 0;
+}
+
+__weak int board_fastboot_write_partition_setup(char *name)
+{
+ return 0;
+}
+
static int fb_nand_lookup(const char *partname, char *response,
nand_info_t **nand,
struct part_info **part)
@@ -90,10 +100,10 @@ static int _fb_nand_write(nand_info_t *nand, struct part_info *part,
#ifdef CONFIG_FASTBOOT_FLASH_NAND_TRIMFFS
flags |= WITH_DROP_FFS;
#endif
+
ret = nand_write_skip_bad(nand, offset, &length, NULL,
part->size - (offset - part->offset),
buffer, flags);
-
if (ret)
return ret;
@@ -132,6 +142,9 @@ void fb_nand_flash_write(const char *partname, void *download_buffer,
return;
}
+ ret = board_fastboot_erase_partition_setup(part->name);
+ if (ret)
+ return;
ret = _fb_nand_erase(nand, part);
if (ret) {
@@ -140,6 +153,10 @@ void fb_nand_flash_write(const char *partname, void *download_buffer,
return;
}
+ ret = board_fastboot_write_partition_setup(part->name);
+ if (ret)
+ return;
+
if (is_sparse_image(download_buffer)) {
struct fb_nand_sparse sparse_priv;
sparse_storage_t sparse;
@@ -187,6 +204,10 @@ void fb_nand_erase(const char *partname, char *response)
return;
}
+ ret = board_fastboot_erase_partition_setup(part->name);
+ if (ret)
+ return;
+
ret = _fb_nand_erase(nand, part);
if (ret) {
error("failed erasing from device %s", nand->name);
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 09/13] sparse: Rename the file and header
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
` (7 preceding siblings ...)
2015-08-31 14:46 ` [U-Boot] [PATCH 08/13] fastboot: nand: Add pre erase and write hooks Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-04 17:21 ` Tom Rini
2015-08-31 14:46 ` [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger Maxime Ripard
` (3 subsequent siblings)
12 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
The Android sparse image format is currently supported through a file
called aboot, which isn't really such a great name, since the sparse image
format is only used for transferring data with fastboot.
Rename the file and header to a file called "sparse", which also makes it
consistent with the header defining the image structures.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
common/Makefile | 2 +-
common/fb_mmc.c | 2 +-
common/fb_nand.c | 2 +-
common/{aboot.c => sparse.c} | 2 +-
include/{aboot.h => sparse.h} | 0
5 files changed, 4 insertions(+), 4 deletions(-)
rename common/{aboot.c => sparse.c} (99%)
rename include/{aboot.h => sparse.h} (100%)
diff --git a/common/Makefile b/common/Makefile
index 20e8027a8913..f4c4cb623fc2 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -272,7 +272,7 @@ obj-y += stdio.o
# This option is not just y/n - it can have a numeric value
ifdef CONFIG_FASTBOOT_FLASH
-obj-y += aboot.o
+obj-y += sparse.o
ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV
obj-y += fb_mmc.o
endif
diff --git a/common/fb_mmc.c b/common/fb_mmc.c
index 0d6bcbf1f78b..eceedf1f1803 100644
--- a/common/fb_mmc.c
+++ b/common/fb_mmc.c
@@ -9,7 +9,7 @@
#include <fastboot.h>
#include <fb_mmc.h>
#include <part.h>
-#include <aboot.h>
+#include <sparse.h>
#include <sparse_format.h>
#include <mmc.h>
diff --git a/common/fb_nand.c b/common/fb_nand.c
index 1aab1e88f9c0..eb7ce4a35a43 100644
--- a/common/fb_nand.c
+++ b/common/fb_nand.c
@@ -8,8 +8,8 @@
#include <config.h>
#include <common.h>
-#include <aboot.h>
#include <fastboot.h>
+#include <sparse.h>
#include <sparse_format.h>
#include <linux/mtd/mtd.h>
diff --git a/common/aboot.c b/common/sparse.c
similarity index 99%
rename from common/aboot.c
rename to common/sparse.c
index 37ad50efc50a..74178f764b18 100644
--- a/common/aboot.c
+++ b/common/sparse.c
@@ -36,10 +36,10 @@
#include <config.h>
#include <common.h>
-#include <aboot.h>
#include <errno.h>
#include <malloc.h>
#include <part.h>
+#include <sparse.h>
#include <sparse_format.h>
typedef struct sparse_buffer {
diff --git a/include/aboot.h b/include/sparse.h
similarity index 100%
rename from include/aboot.h
rename to include/sparse.h
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 09/13] sparse: Rename the file and header
2015-08-31 14:46 ` [U-Boot] [PATCH 09/13] sparse: Rename the file and header Maxime Ripard
@ 2015-09-04 17:21 ` Tom Rini
2015-09-06 11:28 ` Maxime Ripard
0 siblings, 1 reply; 65+ messages in thread
From: Tom Rini @ 2015-09-04 17:21 UTC (permalink / raw)
To: u-boot
On Mon, Aug 31, 2015 at 04:46:09PM +0200, Maxime Ripard wrote:
> The Android sparse image format is currently supported through a file
> called aboot, which isn't really such a great name, since the sparse image
> format is only used for transferring data with fastboot.
>
> Rename the file and header to a file called "sparse", which also makes it
> consistent with the header defining the image structures.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
In general, yes this is better so:
Reviewed-by: Tom Rini <trini@konsulko.com>
But I honestly thought you were messing with the Linux sparse stuff at
first. So maybe common/image-spare.[ch] ?
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150904/11ce9164/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 09/13] sparse: Rename the file and header
2015-09-04 17:21 ` Tom Rini
@ 2015-09-06 11:28 ` Maxime Ripard
0 siblings, 0 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-09-06 11:28 UTC (permalink / raw)
To: u-boot
On Fri, Sep 04, 2015 at 01:21:00PM -0400, Tom Rini wrote:
> On Mon, Aug 31, 2015 at 04:46:09PM +0200, Maxime Ripard wrote:
>
> > The Android sparse image format is currently supported through a file
> > called aboot, which isn't really such a great name, since the sparse image
> > format is only used for transferring data with fastboot.
> >
> > Rename the file and header to a file called "sparse", which also makes it
> > consistent with the header defining the image structures.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>
> In general, yes this is better so:
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> But I honestly thought you were messing with the Linux sparse stuff at
> first. So maybe common/image-spare.[ch] ?
Ah, good point. Yes, I'll rename it :)
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150906/b1c6caef/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
` (8 preceding siblings ...)
2015-08-31 14:46 ` [U-Boot] [PATCH 09/13] sparse: Rename the file and header Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-08-31 15:01 ` Hans de Goede
2015-09-01 7:22 ` Ian Campbell
2015-08-31 14:46 ` [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image Maxime Ripard
` (2 subsequent siblings)
12 siblings, 2 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
When using fastboot and flashing a larger image such as the main partition
of a system, the current 32MB limit for the buffer is quite small.
Increase it to something that looks decent for such a use case.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
include/configs/sunxi-common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 1abf73c31179..710521c617f5 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
#ifdef CONFIG_USB_FUNCTION_FASTBOOT
#define CONFIG_CMD_FASTBOOT
#define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
-#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
+#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
#define CONFIG_FASTBOOT_FLASH
#define CONFIG_FASTBOOT_FLASH_MMC_DEV 0
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-08-31 14:46 ` [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger Maxime Ripard
@ 2015-08-31 15:01 ` Hans de Goede
2015-08-31 19:17 ` Rob Herring
` (2 more replies)
2015-09-01 7:22 ` Ian Campbell
1 sibling, 3 replies; 65+ messages in thread
From: Hans de Goede @ 2015-08-31 15:01 UTC (permalink / raw)
To: u-boot
Hi,
On 31-08-15 16:46, Maxime Ripard wrote:
> When using fastboot and flashing a larger image such as the main partition
> of a system, the current 32MB limit for the buffer is quite small.
>
> Increase it to something that looks decent for such a use case.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> include/configs/sunxi-common.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index 1abf73c31179..710521c617f5 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
> #ifdef CONFIG_USB_FUNCTION_FASTBOOT
> #define CONFIG_CMD_FASTBOOT
> #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
> -#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
> +#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
Hmm, where / how does this get allocated? On some boards we only
have 256M RAM, so this is not going to fit ... also if this comes
out of the heap, the current heap is only 4M and the wip sunxi
nand patches boost it to 64 (I still need to verify this works on
a 256M board, this may need a tweak to bootm_size to make sure
the bootm code does not try to put the kernel where it conflicts
with the heap ...).
Regards,
Hans
^ permalink raw reply [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-08-31 15:01 ` Hans de Goede
@ 2015-08-31 19:17 ` Rob Herring
2015-09-01 7:14 ` Maxime Ripard
2015-09-01 7:05 ` Maxime Ripard
2015-09-01 8:02 ` Siarhei Siamashka
2 siblings, 1 reply; 65+ messages in thread
From: Rob Herring @ 2015-08-31 19:17 UTC (permalink / raw)
To: u-boot
On Mon, Aug 31, 2015 at 10:01 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 31-08-15 16:46, Maxime Ripard wrote:
>>
>> When using fastboot and flashing a larger image such as the main partition
>> of a system, the current 32MB limit for the buffer is quite small.
>>
>> Increase it to something that looks decent for such a use case.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>> include/configs/sunxi-common.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/configs/sunxi-common.h
>> b/include/configs/sunxi-common.h
>> index 1abf73c31179..710521c617f5 100644
>> --- a/include/configs/sunxi-common.h
>> +++ b/include/configs/sunxi-common.h
>> @@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
>> #ifdef CONFIG_USB_FUNCTION_FASTBOOT
>> #define CONFIG_CMD_FASTBOOT
>> #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
>> -#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
>> +#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
>
>
> Hmm, where / how does this get allocated? On some boards we only
> have 256M RAM, so this is not going to fit ... also if this comes
> out of the heap, the current heap is only 4M and the wip sunxi
> nand patches boost it to 64 (I still need to verify this works on
> a 256M board, this may need a tweak to bootm_size to make sure
> the bootm code does not try to put the kernel where it conflicts
> with the heap ...).
I don't think this needs to be so big with current fastboot tool. It
will break up the files if needed. However, IIRC this only works for
sparse images, so I think this needs to be sized large enough for your
biggest bootimage.
Rob
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-08-31 19:17 ` Rob Herring
@ 2015-09-01 7:14 ` Maxime Ripard
2015-09-08 13:00 ` Rob Herring
0 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-09-01 7:14 UTC (permalink / raw)
To: u-boot
On Mon, Aug 31, 2015 at 02:17:50PM -0500, Rob Herring wrote:
> On Mon, Aug 31, 2015 at 10:01 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> > Hi,
> >
> > On 31-08-15 16:46, Maxime Ripard wrote:
> >>
> >> When using fastboot and flashing a larger image such as the main partition
> >> of a system, the current 32MB limit for the buffer is quite small.
> >>
> >> Increase it to something that looks decent for such a use case.
> >>
> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> ---
> >> include/configs/sunxi-common.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/configs/sunxi-common.h
> >> b/include/configs/sunxi-common.h
> >> index 1abf73c31179..710521c617f5 100644
> >> --- a/include/configs/sunxi-common.h
> >> +++ b/include/configs/sunxi-common.h
> >> @@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
> >> #ifdef CONFIG_USB_FUNCTION_FASTBOOT
> >> #define CONFIG_CMD_FASTBOOT
> >> #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
> >> -#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
> >> +#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
> >
> >
> > Hmm, where / how does this get allocated? On some boards we only
> > have 256M RAM, so this is not going to fit ... also if this comes
> > out of the heap, the current heap is only 4M and the wip sunxi
> > nand patches boost it to 64 (I still need to verify this works on
> > a 256M board, this may need a tweak to bootm_size to make sure
> > the bootm code does not try to put the kernel where it conflicts
> > with the heap ...).
>
> I don't think this needs to be so big with current fastboot tool. It
> will break up the files if needed. However, IIRC this only works for
> sparse images, so I think this needs to be sized large enough for your
> biggest bootimage.
Hmm, interesting.
Do you know how it works exactly ? Are we expected to just go on with
writing data at the offset we previously stopped in such a case? I
don't think we support that currently.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/d4ac449b/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-01 7:14 ` Maxime Ripard
@ 2015-09-08 13:00 ` Rob Herring
2015-09-08 15:44 ` Maxime Ripard
0 siblings, 1 reply; 65+ messages in thread
From: Rob Herring @ 2015-09-08 13:00 UTC (permalink / raw)
To: u-boot
On Tue, Sep 1, 2015 at 2:14 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, Aug 31, 2015 at 02:17:50PM -0500, Rob Herring wrote:
>> On Mon, Aug 31, 2015 at 10:01 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> > Hi,
>> >
>> > On 31-08-15 16:46, Maxime Ripard wrote:
>> >>
>> >> When using fastboot and flashing a larger image such as the main partition
>> >> of a system, the current 32MB limit for the buffer is quite small.
>> >>
>> >> Increase it to something that looks decent for such a use case.
>> >>
>> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> >> ---
>> >> include/configs/sunxi-common.h | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/include/configs/sunxi-common.h
>> >> b/include/configs/sunxi-common.h
>> >> index 1abf73c31179..710521c617f5 100644
>> >> --- a/include/configs/sunxi-common.h
>> >> +++ b/include/configs/sunxi-common.h
>> >> @@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
>> >> #ifdef CONFIG_USB_FUNCTION_FASTBOOT
>> >> #define CONFIG_CMD_FASTBOOT
>> >> #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
>> >> -#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
>> >> +#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
>> >
>> >
>> > Hmm, where / how does this get allocated? On some boards we only
>> > have 256M RAM, so this is not going to fit ... also if this comes
>> > out of the heap, the current heap is only 4M and the wip sunxi
>> > nand patches boost it to 64 (I still need to verify this works on
>> > a 256M board, this may need a tweak to bootm_size to make sure
>> > the bootm code does not try to put the kernel where it conflicts
>> > with the heap ...).
>>
>> I don't think this needs to be so big with current fastboot tool. It
>> will break up the files if needed. However, IIRC this only works for
>> sparse images, so I think this needs to be sized large enough for your
>> biggest bootimage.
>
> Hmm, interesting.
>
> Do you know how it works exactly ? Are we expected to just go on with
> writing data at the offset we previously stopped in such a case? I
> don't think we support that currently.
The hard work is on the client side. The client will retrieve the
maxdownloadsize variable and then split the sparse image into smaller
hunks if needed. So the u-boot side doesn't have to do anything
special if 2 chunks happen to be contiguous.
Rob
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-08 13:00 ` Rob Herring
@ 2015-09-08 15:44 ` Maxime Ripard
0 siblings, 0 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-09-08 15:44 UTC (permalink / raw)
To: u-boot
On Tue, Sep 08, 2015 at 08:00:50AM -0500, Rob Herring wrote:
> On Tue, Sep 1, 2015 at 2:14 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Mon, Aug 31, 2015 at 02:17:50PM -0500, Rob Herring wrote:
> >> On Mon, Aug 31, 2015 at 10:01 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> >> > Hi,
> >> >
> >> > On 31-08-15 16:46, Maxime Ripard wrote:
> >> >>
> >> >> When using fastboot and flashing a larger image such as the main partition
> >> >> of a system, the current 32MB limit for the buffer is quite small.
> >> >>
> >> >> Increase it to something that looks decent for such a use case.
> >> >>
> >> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >> >> ---
> >> >> include/configs/sunxi-common.h | 2 +-
> >> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/include/configs/sunxi-common.h
> >> >> b/include/configs/sunxi-common.h
> >> >> index 1abf73c31179..710521c617f5 100644
> >> >> --- a/include/configs/sunxi-common.h
> >> >> +++ b/include/configs/sunxi-common.h
> >> >> @@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
> >> >> #ifdef CONFIG_USB_FUNCTION_FASTBOOT
> >> >> #define CONFIG_CMD_FASTBOOT
> >> >> #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
> >> >> -#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
> >> >> +#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
> >> >
> >> >
> >> > Hmm, where / how does this get allocated? On some boards we only
> >> > have 256M RAM, so this is not going to fit ... also if this comes
> >> > out of the heap, the current heap is only 4M and the wip sunxi
> >> > nand patches boost it to 64 (I still need to verify this works on
> >> > a 256M board, this may need a tweak to bootm_size to make sure
> >> > the bootm code does not try to put the kernel where it conflicts
> >> > with the heap ...).
> >>
> >> I don't think this needs to be so big with current fastboot tool. It
> >> will break up the files if needed. However, IIRC this only works for
> >> sparse images, so I think this needs to be sized large enough for your
> >> biggest bootimage.
> >
> > Hmm, interesting.
> >
> > Do you know how it works exactly ? Are we expected to just go on with
> > writing data at the offset we previously stopped in such a case? I
> > don't think we support that currently.
>
> The hard work is on the client side. The client will retrieve the
> maxdownloadsize variable and then split the sparse image into smaller
> hunks if needed. So the u-boot side doesn't have to do anything
> special if 2 chunks happen to be contiguous.
Well, it still has to make sure it keeps the offset of the current
session instead of restarting from the partition base offset, and I'm
not sure I've seen anything like it for now.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150908/85230286/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-08-31 15:01 ` Hans de Goede
2015-08-31 19:17 ` Rob Herring
@ 2015-09-01 7:05 ` Maxime Ripard
2015-09-01 8:59 ` Hans de Goede
2015-09-01 8:02 ` Siarhei Siamashka
2 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-09-01 7:05 UTC (permalink / raw)
To: u-boot
Hi,
On Mon, Aug 31, 2015 at 05:01:42PM +0200, Hans de Goede wrote:
> On 31-08-15 16:46, Maxime Ripard wrote:
> >When using fastboot and flashing a larger image such as the main partition
> >of a system, the current 32MB limit for the buffer is quite small.
> >
> >Increase it to something that looks decent for such a use case.
> >
> >Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >---
> > include/configs/sunxi-common.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> >index 1abf73c31179..710521c617f5 100644
> >--- a/include/configs/sunxi-common.h
> >+++ b/include/configs/sunxi-common.h
> >@@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
> > #ifdef CONFIG_USB_FUNCTION_FASTBOOT
> > #define CONFIG_CMD_FASTBOOT
> > #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
> >-#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
> >+#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
>
> Hmm, where / how does this get allocated? On some boards we only
> have 256M RAM, so this is not going to fit ... also if this comes
> out of the heap, the current heap is only 4M and the wip sunxi
> nand patches boost it to 64 (I still need to verify this works on
> a 256M board, this may need a tweak to bootm_size to make sure
> the bootm code does not try to put the kernel where it conflicts
> with the heap ...).
It's not allocated, it just uses the RAM directly, starting at the
offset CONFIG_FASTBOOT_BUF_ADDR (0x42000000 in our case), just like
any *load function for example.
The only thing we have to make sure is that we won't overwrite U-boot
itself, which will be an issue on those 256MB boards...
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/33bda824/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-01 7:05 ` Maxime Ripard
@ 2015-09-01 8:59 ` Hans de Goede
2015-09-03 21:43 ` Maxime Ripard
0 siblings, 1 reply; 65+ messages in thread
From: Hans de Goede @ 2015-09-01 8:59 UTC (permalink / raw)
To: u-boot
Hi,
On 01-09-15 09:05, Maxime Ripard wrote:
> Hi,
>
> On Mon, Aug 31, 2015 at 05:01:42PM +0200, Hans de Goede wrote:
>> On 31-08-15 16:46, Maxime Ripard wrote:
>>> When using fastboot and flashing a larger image such as the main partition
>>> of a system, the current 32MB limit for the buffer is quite small.
>>>
>>> Increase it to something that looks decent for such a use case.
>>>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> ---
>>> include/configs/sunxi-common.h | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
>>> index 1abf73c31179..710521c617f5 100644
>>> --- a/include/configs/sunxi-common.h
>>> +++ b/include/configs/sunxi-common.h
>>> @@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
>>> #ifdef CONFIG_USB_FUNCTION_FASTBOOT
>>> #define CONFIG_CMD_FASTBOOT
>>> #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
>>> -#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
>>> +#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
>>
>> Hmm, where / how does this get allocated? On some boards we only
>> have 256M RAM, so this is not going to fit ... also if this comes
>> out of the heap, the current heap is only 4M and the wip sunxi
>> nand patches boost it to 64 (I still need to verify this works on
>> a 256M board, this may need a tweak to bootm_size to make sure
>> the bootm code does not try to put the kernel where it conflicts
>> with the heap ...).
>
> It's not allocated, it just uses the RAM directly, starting at the
> offset CONFIG_FASTBOOT_BUF_ADDR (0x42000000 in our case), just like
> any *load function for example.
>
> The only thing we have to make sure is that we won't overwrite U-boot
> itself, which will be an issue on those 256MB boards...
Well the only 256M board is the A13-OLinuXino-MICRO, all other boards
have at least 512M and the A13-OLinuXino-MICRO does not have nand,
so I guess we do not really need to worry about this.
Regards,
Hans
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-01 8:59 ` Hans de Goede
@ 2015-09-03 21:43 ` Maxime Ripard
0 siblings, 0 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-09-03 21:43 UTC (permalink / raw)
To: u-boot
On Tue, Sep 01, 2015 at 10:59:52AM +0200, Hans de Goede wrote:
> Hi,
>
> On 01-09-15 09:05, Maxime Ripard wrote:
> >Hi,
> >
> >On Mon, Aug 31, 2015 at 05:01:42PM +0200, Hans de Goede wrote:
> >>On 31-08-15 16:46, Maxime Ripard wrote:
> >>>When using fastboot and flashing a larger image such as the main partition
> >>>of a system, the current 32MB limit for the buffer is quite small.
> >>>
> >>>Increase it to something that looks decent for such a use case.
> >>>
> >>>Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>>---
> >>> include/configs/sunxi-common.h | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> >>>index 1abf73c31179..710521c617f5 100644
> >>>--- a/include/configs/sunxi-common.h
> >>>+++ b/include/configs/sunxi-common.h
> >>>@@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
> >>> #ifdef CONFIG_USB_FUNCTION_FASTBOOT
> >>> #define CONFIG_CMD_FASTBOOT
> >>> #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
> >>>-#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
> >>>+#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
> >>
> >>Hmm, where / how does this get allocated? On some boards we only
> >>have 256M RAM, so this is not going to fit ... also if this comes
> >>out of the heap, the current heap is only 4M and the wip sunxi
> >>nand patches boost it to 64 (I still need to verify this works on
> >>a 256M board, this may need a tweak to bootm_size to make sure
> >>the bootm code does not try to put the kernel where it conflicts
> >>with the heap ...).
> >
> >It's not allocated, it just uses the RAM directly, starting at the
> >offset CONFIG_FASTBOOT_BUF_ADDR (0x42000000 in our case), just like
> >any *load function for example.
> >
> >The only thing we have to make sure is that we won't overwrite U-boot
> >itself, which will be an issue on those 256MB boards...
>
> Well the only 256M board is the A13-OLinuXino-MICRO, all other boards
> have at least 512M and the A13-OLinuXino-MICRO does not have nand,
> so I guess we do not really need to worry about this.
You could use fastboot to flash something on the MMC, so I guess it is
something to worry about :)
Rob was kind of saying that such a huge value wasn't needed, so I
guess we can simply drop that patch.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150903/7431cab1/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-08-31 15:01 ` Hans de Goede
2015-08-31 19:17 ` Rob Herring
2015-09-01 7:05 ` Maxime Ripard
@ 2015-09-01 8:02 ` Siarhei Siamashka
2 siblings, 0 replies; 65+ messages in thread
From: Siarhei Siamashka @ 2015-09-01 8:02 UTC (permalink / raw)
To: u-boot
On Mon, 31 Aug 2015 17:01:42 +0200
Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 31-08-15 16:46, Maxime Ripard wrote:
> > When using fastboot and flashing a larger image such as the main partition
> > of a system, the current 32MB limit for the buffer is quite small.
> >
> > Increase it to something that looks decent for such a use case.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> > include/configs/sunxi-common.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> > index 1abf73c31179..710521c617f5 100644
> > --- a/include/configs/sunxi-common.h
> > +++ b/include/configs/sunxi-common.h
> > @@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
> > #ifdef CONFIG_USB_FUNCTION_FASTBOOT
> > #define CONFIG_CMD_FASTBOOT
> > #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
> > -#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
> > +#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
>
> Hmm, where / how does this get allocated? On some boards we only
> have 256M RAM, so this is not going to fit ... also if this comes
> out of the heap, the current heap is only 4M and the wip sunxi
> nand patches boost it to 64 (I still need to verify this works on
> a 256M board, this may need a tweak to bootm_size to make sure
> the bootm code does not try to put the kernel where it conflicts
> with the heap ...).
Can this be eventually improved to become a dynamic limit (depending
on the RAM size available on the device) instead of the "one size fits
all" hardcoded define approach?
--
Best regards,
Siarhei Siamashka
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-08-31 14:46 ` [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger Maxime Ripard
2015-08-31 15:01 ` Hans de Goede
@ 2015-09-01 7:22 ` Ian Campbell
2015-09-01 7:44 ` Siarhei Siamashka
2015-09-01 7:57 ` Maxime Ripard
1 sibling, 2 replies; 65+ messages in thread
From: Ian Campbell @ 2015-09-01 7:22 UTC (permalink / raw)
To: u-boot
On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> When using fastboot and flashing a larger image such as the main partition
> of a system, the current 32MB limit for the buffer is quite small.
(Apart from rooting/rescuing the odd phone I'm completely unfamiliar
with fastboot, so sorry if this is all obvious).
The main partition of a system these days is measured in GB, I think.
So why does going from 32MB to 256MB for the buffer make a useful
difference?
Is there some enormous per-buffer overhead which needs to be amortised?
Or is something else going on?
IOW what is the practical impact of this change?
> Increase it to something that looks decent for such a use case.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> include/configs/sunxi-common.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi
> -common.h
> index 1abf73c31179..710521c617f5 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -363,7 +363,7 @@ extern int soft_i2c_gpio_scl;
> #ifdef CONFIG_USB_FUNCTION_FASTBOOT
> #define CONFIG_CMD_FASTBOOT
> #define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
> -#define CONFIG_FASTBOOT_BUF_SIZE 0x2000000
> +#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
>
> #define CONFIG_FASTBOOT_FLASH
> #define CONFIG_FASTBOOT_FLASH_MMC_DEV 0
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-01 7:22 ` Ian Campbell
@ 2015-09-01 7:44 ` Siarhei Siamashka
2015-09-01 8:11 ` Maxime Ripard
2015-09-04 17:02 ` Tom Rini
2015-09-01 7:57 ` Maxime Ripard
1 sibling, 2 replies; 65+ messages in thread
From: Siarhei Siamashka @ 2015-09-01 7:44 UTC (permalink / raw)
To: u-boot
On Tue, 01 Sep 2015 08:22:04 +0100
Ian Campbell <ijc@hellion.org.uk> wrote:
> On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > When using fastboot and flashing a larger image such as the main partition
> > of a system, the current 32MB limit for the buffer is quite small.
>
> (Apart from rooting/rescuing the odd phone I'm completely unfamiliar
> with fastboot, so sorry if this is all obvious).
>
> The main partition of a system these days is measured in GB, I think.
> So why does going from 32MB to 256MB for the buffer make a useful
> difference?
>
> Is there some enormous per-buffer overhead which needs to be amortised?
> Or is something else going on?
>
> IOW what is the practical impact of this change?
I don't know what are Maxime's plans. But if fastboot is fast and
can load the kernel and initrd to the device over USB, then it
becomes a useful alternative to using FEL for loading kernel.
FEL is implemented by BROM, but it is not particularly fast and
the transfer speed varies for different SoC variants:
http://linux-sunxi.org/FEL/USBBoot#SoC_support_status
Somebody might want to evaluate the use of fastboot for loading kernel
with initrd and document it at http://linux-sunxi.org/FEL/USBBoot
FEL is still needed for loading U-Boot over USB on sunxi hardware, but
U-Boot size is relatively small compared to the kernel, and especially
initrd.
--
Best regards,
Siarhei Siamashka
^ permalink raw reply [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-01 7:44 ` Siarhei Siamashka
@ 2015-09-01 8:11 ` Maxime Ripard
2015-09-04 17:02 ` Tom Rini
1 sibling, 0 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-09-01 8:11 UTC (permalink / raw)
To: u-boot
On Tue, Sep 01, 2015 at 10:44:58AM +0300, Siarhei Siamashka wrote:
> On Tue, 01 Sep 2015 08:22:04 +0100
> Ian Campbell <ijc@hellion.org.uk> wrote:
>
> > On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > > When using fastboot and flashing a larger image such as the main partition
> > > of a system, the current 32MB limit for the buffer is quite small.
> >
> > (Apart from rooting/rescuing the odd phone I'm completely unfamiliar
> > with fastboot, so sorry if this is all obvious).
> >
> > The main partition of a system these days is measured in GB, I think.
> > So why does going from 32MB to 256MB for the buffer make a useful
> > difference?
> >
> > Is there some enormous per-buffer overhead which needs to be amortised?
> > Or is something else going on?
> >
> > IOW what is the practical impact of this change?
>
> I don't know what are Maxime's plans. But if fastboot is fast and
> can load the kernel and initrd to the device over USB, then it
> becomes a useful alternative to using FEL for loading kernel.
This serie is mostly to use fastboot to flash partition images to the
storage device. We're using it to program the flash the first time.
It's indeed significantly faster than using fel to do so (by a
guesstimate, something around 30-50%), and is more pratical in the
sense that you can later on reflash only a single partition, all of
that through USB.
However, you can also use it to simply boot a kernel, without touching
any of the device storage. The main drawback with that is that the DT
has to be appended, which prevents the use of simplefb or psci in our
case...
I tried to amend the android boot image format that fastboot sends
when doing so to add a section for the DT, but gave up when having to
deal with the image parser in u-boot.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/ef279425/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-01 7:44 ` Siarhei Siamashka
2015-09-01 8:11 ` Maxime Ripard
@ 2015-09-04 17:02 ` Tom Rini
2015-09-06 11:23 ` Maxime Ripard
1 sibling, 1 reply; 65+ messages in thread
From: Tom Rini @ 2015-09-04 17:02 UTC (permalink / raw)
To: u-boot
On Tue, Sep 01, 2015 at 10:44:58AM +0300, Siarhei Siamashka wrote:
> On Tue, 01 Sep 2015 08:22:04 +0100
> Ian Campbell <ijc@hellion.org.uk> wrote:
>
> > On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > > When using fastboot and flashing a larger image such as the main partition
> > > of a system, the current 32MB limit for the buffer is quite small.
> >
> > (Apart from rooting/rescuing the odd phone I'm completely unfamiliar
> > with fastboot, so sorry if this is all obvious).
> >
> > The main partition of a system these days is measured in GB, I think.
> > So why does going from 32MB to 256MB for the buffer make a useful
> > difference?
> >
> > Is there some enormous per-buffer overhead which needs to be amortised?
> > Or is something else going on?
> >
> > IOW what is the practical impact of this change?
>
> I don't know what are Maxime's plans. But if fastboot is fast and
> can load the kernel and initrd to the device over USB, then it
> becomes a useful alternative to using FEL for loading kernel.
This is in fact a common use case for development. Other platforms /
SoCs do use fastboot in non-Android development as how to pass in a new
testing kernel / initrd / device tree combo.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150904/da438a75/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-04 17:02 ` Tom Rini
@ 2015-09-06 11:23 ` Maxime Ripard
0 siblings, 0 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-09-06 11:23 UTC (permalink / raw)
To: u-boot
On Fri, Sep 04, 2015 at 01:02:49PM -0400, Tom Rini wrote:
> On Tue, Sep 01, 2015 at 10:44:58AM +0300, Siarhei Siamashka wrote:
>
> > On Tue, 01 Sep 2015 08:22:04 +0100
> > Ian Campbell <ijc@hellion.org.uk> wrote:
> >
> > > On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > > > When using fastboot and flashing a larger image such as the main partition
> > > > of a system, the current 32MB limit for the buffer is quite small.
> > >
> > > (Apart from rooting/rescuing the odd phone I'm completely unfamiliar
> > > with fastboot, so sorry if this is all obvious).
> > >
> > > The main partition of a system these days is measured in GB, I think.
> > > So why does going from 32MB to 256MB for the buffer make a useful
> > > difference?
> > >
> > > Is there some enormous per-buffer overhead which needs to be amortised?
> > > Or is something else going on?
> > >
> > > IOW what is the practical impact of this change?
> >
> > I don't know what are Maxime's plans. But if fastboot is fast and
> > can load the kernel and initrd to the device over USB, then it
> > becomes a useful alternative to using FEL for loading kernel.
>
> This is in fact a common use case for development. Other platforms /
> SoCs do use fastboot in non-Android development as how to pass in a new
> testing kernel / initrd / device tree combo.
AFAIK, there's no support in fastboot for a dt yet. You can only give
it a kernel with an appended DT.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150906/9c30a9f9/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-01 7:22 ` Ian Campbell
2015-09-01 7:44 ` Siarhei Siamashka
@ 2015-09-01 7:57 ` Maxime Ripard
2015-09-04 16:59 ` Tom Rini
1 sibling, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-09-01 7:57 UTC (permalink / raw)
To: u-boot
Hi Ian,
On Tue, Sep 01, 2015 at 08:22:04AM +0100, Ian Campbell wrote:
> On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > When using fastboot and flashing a larger image such as the main partition
> > of a system, the current 32MB limit for the buffer is quite small.
>
> (Apart from rooting/rescuing the odd phone I'm completely unfamiliar
> with fastboot, so sorry if this is all obvious).
>
> The main partition of a system these days is measured in GB, I think.
> So why does going from 32MB to 256MB for the buffer make a useful
> difference?
>
> Is there some enormous per-buffer overhead which needs to be amortised?
> Or is something else going on?
>
> IOW what is the practical impact of this change?
It really depends on the image content you're trying to flash, but
there's two things to consider:
- On a NAND device, you will just flash the actual content, but not
the extra empty space in the partition, and the FS will
automatically grow the first time you mount it.
- If there's some blocks that are filled with the same content, the
sparse image format I was refactoring earlier in that patchset
will be able to compress it to 16 bytes (iirc), instead of a full
block (512 bytes or a page, depending on whether you use an MMC or
a NAND). On NAND images, there's often a lot of pages filled with
0xff, which fall into that category, I'd expect it to be similar
on block partitions for the extra space.
The current use case we have for example is to flash a 4GB partition
with a debian system, in a UBI partition. Even though it's a decent
system, storage size-wise, the file actually transfered and stored in
the memory is around 210MB once compressed, and 270MB if not
compressed.
Of course, this will all depend on the ratio between the empty space
and the files themselves, and what files you actually have there, but
while 32MB is definitely useless, 256MB is already a decent size.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/579d3bde/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-01 7:57 ` Maxime Ripard
@ 2015-09-04 16:59 ` Tom Rini
2015-09-06 11:22 ` Maxime Ripard
0 siblings, 1 reply; 65+ messages in thread
From: Tom Rini @ 2015-09-04 16:59 UTC (permalink / raw)
To: u-boot
On Tue, Sep 01, 2015 at 09:57:16AM +0200, Maxime Ripard wrote:
[snip]
> Of course, this will all depend on the ratio between the empty space
> and the files themselves, and what files you actually have there, but
> while 32MB is definitely useless, 256MB is already a decent size.
So there's not any automatic chunking and writing of the image? Maybe
I'm confusing this with DFU and/or my wishlist/imagination...
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150904/754cae25/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-04 16:59 ` Tom Rini
@ 2015-09-06 11:22 ` Maxime Ripard
2015-09-07 9:07 ` Ian Campbell
0 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-09-06 11:22 UTC (permalink / raw)
To: u-boot
On Fri, Sep 04, 2015 at 12:59:24PM -0400, Tom Rini wrote:
> On Tue, Sep 01, 2015 at 09:57:16AM +0200, Maxime Ripard wrote:
>
> [snip]
> > Of course, this will all depend on the ratio between the empty space
> > and the files themselves, and what files you actually have there, but
> > while 32MB is definitely useless, 256MB is already a decent size.
>
> So there's not any automatic chunking and writing of the image? Maybe
> I'm confusing this with DFU and/or my wishlist/imagination...
Apparently, Rob was saying there was some, but I'm not sure we support
that yet, I'd need to look into it.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150906/40f01934/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger
2015-09-06 11:22 ` Maxime Ripard
@ 2015-09-07 9:07 ` Ian Campbell
0 siblings, 0 replies; 65+ messages in thread
From: Ian Campbell @ 2015-09-07 9:07 UTC (permalink / raw)
To: u-boot
On Sun, 2015-09-06 at 13:22 +0200, Maxime Ripard wrote:
> On Fri, Sep 04, 2015 at 12:59:24PM -0400, Tom Rini wrote:
> > On Tue, Sep 01, 2015 at 09:57:16AM +0200, Maxime Ripard wrote:
> >
> > [snip]
> > > Of course, this will all depend on the ratio between the empty space
> > > and the files themselves, and what files you actually have there, but
> > > while 32MB is definitely useless, 256MB is already a decent size.
> >
> > So there's not any automatic chunking and writing of the image? Maybe
> > I'm confusing this with DFU and/or my wishlist/imagination...
>
> Apparently, Rob was saying there was some, but I'm not sure we support
> that yet, I'd need to look into it.
It seems this is pretty critical, given that storage (both capacity and
actual required space within the fs) are growing and 256MB is only barely
big enough for the 4GB image you have today (210MB after the various
compression strategies are applied).
Ian.
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
` (9 preceding siblings ...)
2015-08-31 14:46 ` [U-Boot] [PATCH 10/13] sunxi: Make the fastboot buffer larger Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-01 7:08 ` Ian Campbell
2015-08-31 14:46 ` [U-Boot] [PATCH 12/13] sunxi: A13-Olinuxino: Enable the USB OTG controller Maxime Ripard
2015-08-31 14:46 ` [U-Boot] [PATCH 13/13] sunxi: cubietruck: " Maxime Ripard
12 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
When using the fastboot boot command, the image sent to U-Boot will be an
Android boot image. If the support is missing, that won't obviously work,
so we need it in our configuration.
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
include/configs/sunxi-common.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index 710521c617f5..ee81fa9a1142 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -364,6 +364,7 @@ extern int soft_i2c_gpio_scl;
#define CONFIG_CMD_FASTBOOT
#define CONFIG_FASTBOOT_BUF_ADDR CONFIG_SYS_LOAD_ADDR
#define CONFIG_FASTBOOT_BUF_SIZE (256 << 20)
+#define CONFIG_ANDROID_BOOT_IMAGE
#define CONFIG_FASTBOOT_FLASH
#define CONFIG_FASTBOOT_FLASH_MMC_DEV 0
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image
2015-08-31 14:46 ` [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image Maxime Ripard
@ 2015-09-01 7:08 ` Ian Campbell
2015-09-01 7:15 ` Maxime Ripard
` (2 more replies)
0 siblings, 3 replies; 65+ messages in thread
From: Ian Campbell @ 2015-09-01 7:08 UTC (permalink / raw)
To: u-boot
On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> When using the fastboot boot command, the image sent to U-Boot will be an
> Android boot image. If the support is missing, that won't obviously work,
> so we need it in our configuration.
Dumb question: Is it possible to boot anything _other_ than an android
boot image via fastboot?
If not then one of the two config options really ought to imply the
other.
If it is possible to boot something else via fastboot then:
Acked-by: Ian Campbell <ijc@hellion.org.uk>
Ian.
^ permalink raw reply [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image
2015-09-01 7:08 ` Ian Campbell
@ 2015-09-01 7:15 ` Maxime Ripard
2015-09-01 9:00 ` Paul Kocialkowski
2015-09-08 13:12 ` Rob Herring
2 siblings, 0 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-09-01 7:15 UTC (permalink / raw)
To: u-boot
On Tue, Sep 01, 2015 at 08:08:58AM +0100, Ian Campbell wrote:
> On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > When using the fastboot boot command, the image sent to U-Boot will be an
> > Android boot image. If the support is missing, that won't obviously work,
> > so we need it in our configuration.
>
> Dumb question: Is it possible to boot anything _other_ than an android
> boot image via fastboot?
AFAIK, the fastboot tool will automatically generate an android boot
image when given any image, and will send that. I don't think there's
any way to circumvent that behaviour.
> If not then one of the two config options really ought to imply the
> other.
Yeah, probably.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/6c1ae8dd/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image
2015-09-01 7:08 ` Ian Campbell
2015-09-01 7:15 ` Maxime Ripard
@ 2015-09-01 9:00 ` Paul Kocialkowski
2015-09-01 10:46 ` Ian Campbell
2015-09-01 11:28 ` Maxime Ripard
2015-09-08 13:12 ` Rob Herring
2 siblings, 2 replies; 65+ messages in thread
From: Paul Kocialkowski @ 2015-09-01 9:00 UTC (permalink / raw)
To: u-boot
Le mardi 01 septembre 2015 ? 08:08 +0100, Ian Campbell a ?crit :
> On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > When using the fastboot boot command, the image sent to U-Boot will be an
> > Android boot image. If the support is missing, that won't obviously work,
> > so we need it in our configuration.
>
> Dumb question: Is it possible to boot anything _other_ than an android
> boot image via fastboot?
>
> If not then one of the two config options really ought to imply the
> other.
Well, those options have not been moved to Kconfig yet, but when they
do, there should indeed be such a dependency.
Otherwise, I believe it still makes sense to have two separate options
because one might want Android boot image support without fastboot.
> If it is possible to boot something else via fastboot then:
> Acked-by: Ian Campbell <ijc@hellion.org.uk>
>
> Ian.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/19d9b17c/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image
2015-09-01 9:00 ` Paul Kocialkowski
@ 2015-09-01 10:46 ` Ian Campbell
2015-09-01 11:29 ` Maxime Ripard
2015-09-01 11:28 ` Maxime Ripard
1 sibling, 1 reply; 65+ messages in thread
From: Ian Campbell @ 2015-09-01 10:46 UTC (permalink / raw)
To: u-boot
On Tue, 2015-09-01 at 11:00 +0200, Paul Kocialkowski wrote:
> Le mardi 01 septembre 2015 ? 08:08 +0100, Ian Campbell a ?crit :
> > On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > > When using the fastboot boot command, the image sent to U-Boot will
> > > be an
> > > Android boot image. If the support is missing, that won't obviously
> > > work,
> > > so we need it in our configuration.
> >
> > Dumb question: Is it possible to boot anything _other_ than an android
> > boot image via fastboot?
> >
> > If not then one of the two config options really ought to imply the
> > other.
>
> Well, those options have not been moved to Kconfig yet, but when they
> do, there should indeed be such a dependency.
>
> Otherwise, I believe it still makes sense to have two separate options
> because one might want Android boot image support without fastboot.
Then fastboot should depend on android boot image support but not vice
-versa.
>
> > If it is possible to boot something else via fastboot then:
> > Acked-by: Ian Campbell <ijc@hellion.org.uk>
> >
> > Ian.
>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image
2015-09-01 10:46 ` Ian Campbell
@ 2015-09-01 11:29 ` Maxime Ripard
2015-09-01 13:54 ` Ian Campbell
0 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-09-01 11:29 UTC (permalink / raw)
To: u-boot
On Tue, Sep 01, 2015 at 11:46:05AM +0100, Ian Campbell wrote:
> On Tue, 2015-09-01 at 11:00 +0200, Paul Kocialkowski wrote:
> > Le mardi 01 septembre 2015 ? 08:08 +0100, Ian Campbell a ?crit :
> > > On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > > > When using the fastboot boot command, the image sent to U-Boot will
> > > > be an
> > > > Android boot image. If the support is missing, that won't obviously
> > > > work,
> > > > so we need it in our configuration.
> > >
> > > Dumb question: Is it possible to boot anything _other_ than an android
> > > boot image via fastboot?
> > >
> > > If not then one of the two config options really ought to imply the
> > > other.
> >
> > Well, those options have not been moved to Kconfig yet, but when they
> > do, there should indeed be such a dependency.
> >
> > Otherwise, I believe it still makes sense to have two separate options
> > because one might want Android boot image support without fastboot.
>
> Then fastboot should depend on android boot image support but not vice
> -versa.
Indeed, but it's not yet a Kconfig option.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/fbcd214a/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image
2015-09-01 11:29 ` Maxime Ripard
@ 2015-09-01 13:54 ` Ian Campbell
2015-09-01 13:59 ` Paul Kocialkowski
0 siblings, 1 reply; 65+ messages in thread
From: Ian Campbell @ 2015-09-01 13:54 UTC (permalink / raw)
To: u-boot
On Tue, 2015-09-01 at 13:29 +0200, Maxime Ripard wrote:
> On Tue, Sep 01, 2015 at 11:46:05AM +0100, Ian Campbell wrote:
> > On Tue, 2015-09-01 at 11:00 +0200, Paul Kocialkowski wrote:
> > > Le mardi 01 septembre 2015 ? 08:08 +0100, Ian Campbell a ?crit :
> > > > On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > > > > When using the fastboot boot command, the image sent to U-Boot
> > > > > will
> > > > > be an
> > > > > Android boot image. If the support is missing, that won't
> > > > > obviously
> > > > > work,
> > > > > so we need it in our configuration.
> > > >
> > > > Dumb question: Is it possible to boot anything _other_ than an
> > > > android
> > > > boot image via fastboot?
> > > >
> > > > If not then one of the two config options really ought to imply the
> > > > other.
> > >
> > > Well, those options have not been moved to Kconfig yet, but when they
> > > do, there should indeed be such a dependency.
> > >
> > > Otherwise, I believe it still makes sense to have two separate
> > > options
> > > because one might want Android boot image support without fastboot.
> >
> > Then fastboot should depend on android boot image support but not vice
> > -versa.
>
> Indeed, but it's not yet a Kconfig option.
I'm not sure that's a necessary prerequisite, in some common place do:
#if defined(CONFIG_CMD_FASTBOOT) && !defined(CONFIG_ANDROID_BOOT_IMAGE)
#define CONFIG_ANDROID_BOOT_IMAGE
#endif
Ian.
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image
2015-09-01 13:54 ` Ian Campbell
@ 2015-09-01 13:59 ` Paul Kocialkowski
0 siblings, 0 replies; 65+ messages in thread
From: Paul Kocialkowski @ 2015-09-01 13:59 UTC (permalink / raw)
To: u-boot
Le mardi 01 septembre 2015 ? 14:54 +0100, Ian Campbell a ?crit :
> On Tue, 2015-09-01 at 13:29 +0200, Maxime Ripard wrote:
> > On Tue, Sep 01, 2015 at 11:46:05AM +0100, Ian Campbell wrote:
> > > On Tue, 2015-09-01 at 11:00 +0200, Paul Kocialkowski wrote:
> > > > Le mardi 01 septembre 2015 ? 08:08 +0100, Ian Campbell a ?crit :
> > > > > On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > > > > > When using the fastboot boot command, the image sent to U-Boot
> > > > > > will
> > > > > > be an
> > > > > > Android boot image. If the support is missing, that won't
> > > > > > obviously
> > > > > > work,
> > > > > > so we need it in our configuration.
> > > > >
> > > > > Dumb question: Is it possible to boot anything _other_ than an
> > > > > android
> > > > > boot image via fastboot?
> > > > >
> > > > > If not then one of the two config options really ought to imply the
> > > > > other.
> > > >
> > > > Well, those options have not been moved to Kconfig yet, but when they
> > > > do, there should indeed be such a dependency.
> > > >
> > > > Otherwise, I believe it still makes sense to have two separate
> > > > options
> > > > because one might want Android boot image support without fastboot.
> > >
> > > Then fastboot should depend on android boot image support but not vice
> > > -versa.
> >
> > Indeed, but it's not yet a Kconfig option.
>
> I'm not sure that's a necessary prerequisite, in some common place do:
>
> #if defined(CONFIG_CMD_FASTBOOT) && !defined(CONFIG_ANDROID_BOOT_IMAGE)
> #define CONFIG_ANDROID_BOOT_IMAGE
> #endif
Well, I'm not sure adding Kconfig-style logic really is a good idea as
it removes some incentive to do things right and move everything over to
Kconfig (which is out of the scope of this patch set, I believe).
In addition, I don't see what a relevant place to but this in would be.
Putting that in sunxi-common.h is just adding two useless extra lines
(having the define is sufficient).
Overall, I don't think this is a good idea when we have Kconfig around
the corner to do things right?
--
Paul Kocialkowski, Replicant developer
Replicant is a fully free Android distribution running on several
devices, a free software mobile operating system putting the emphasis on
freedom and privacy/security.
Website: http://www.replicant.us/
Blog: http://blog.replicant.us/
Wiki/tracker/forums: http://redmine.replicant.us/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/d506c28d/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image
2015-09-01 9:00 ` Paul Kocialkowski
2015-09-01 10:46 ` Ian Campbell
@ 2015-09-01 11:28 ` Maxime Ripard
1 sibling, 0 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-09-01 11:28 UTC (permalink / raw)
To: u-boot
On Tue, Sep 01, 2015 at 11:00:53AM +0200, Paul Kocialkowski wrote:
> Le mardi 01 septembre 2015 ? 08:08 +0100, Ian Campbell a ?crit :
> > On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
> > > When using the fastboot boot command, the image sent to U-Boot will be an
> > > Android boot image. If the support is missing, that won't obviously work,
> > > so we need it in our configuration.
> >
> > Dumb question: Is it possible to boot anything _other_ than an android
> > boot image via fastboot?
> >
> > If not then one of the two config options really ought to imply the
> > other.
>
> Well, those options have not been moved to Kconfig yet, but when they
> do, there should indeed be such a dependency.
>
> Otherwise, I believe it still makes sense to have two separate options
> because one might want Android boot image support without fastboot.
Ah, good point, Android stores its kernel/initrd in that format. So
even if you don't want fastboot, you might want the support for the
image itself.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150901/a6e7dccc/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image
2015-09-01 7:08 ` Ian Campbell
2015-09-01 7:15 ` Maxime Ripard
2015-09-01 9:00 ` Paul Kocialkowski
@ 2015-09-08 13:12 ` Rob Herring
2 siblings, 0 replies; 65+ messages in thread
From: Rob Herring @ 2015-09-08 13:12 UTC (permalink / raw)
To: u-boot
On Tue, Sep 1, 2015 at 2:08 AM, Ian Campbell <ijc@hellion.org.uk> wrote:
> On Mon, 2015-08-31 at 16:46 +0200, Maxime Ripard wrote:
>> When using the fastboot boot command, the image sent to U-Boot will be an
>> Android boot image. If the support is missing, that won't obviously work,
>> so we need it in our configuration.
>
> Dumb question: Is it possible to boot anything _other_ than an android
> boot image via fastboot?
Yes, fastboot just writes either raw block data or sparse images to
partitions. While the client tool can create the boot image for you,
that is purely a client side feature. What you can boot is determined
by your boot scripts. You can also boot an Android boot image with
bootm and without any fastboot support.
Rob
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 12/13] sunxi: A13-Olinuxino: Enable the USB OTG controller
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
` (10 preceding siblings ...)
2015-08-31 14:46 ` [U-Boot] [PATCH 11/13] sunxi: Add support for android boot image Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
2015-09-01 9:01 ` Hans de Goede
2015-08-31 14:46 ` [U-Boot] [PATCH 13/13] sunxi: cubietruck: " Maxime Ripard
12 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
The A13-Olinuxino has a mini-USB connector that can be used to power up
the board and as an OTG connector.
Since we have already some USB host-only ports right beside this one,
enable it in gadget mode
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
configs/A13-OLinuXino_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/configs/A13-OLinuXino_defconfig b/configs/A13-OLinuXino_defconfig
index 4b4337223ca5..70aa194b91c4 100644
--- a/configs/A13-OLinuXino_defconfig
+++ b/configs/A13-OLinuXino_defconfig
@@ -3,6 +3,7 @@ CONFIG_ARCH_SUNXI=y
CONFIG_MACH_SUN5I=y
CONFIG_DRAM_CLK=408
CONFIG_DRAM_EMR1=0
+CONFIG_USB0_VBUS_DET="PG1"
CONFIG_USB1_VBUS_PIN="PG11"
CONFIG_AXP_GPIO=y
# CONFIG_VIDEO_HDMI is not set
@@ -11,6 +12,7 @@ CONFIG_VIDEO_VGA_VIA_LCD_FORCE_SYNC_ACTIVE_HIGH=y
CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:3,vmode:0"
CONFIG_VIDEO_LCD_POWER="AXP0-0"
CONFIG_VIDEO_LCD_BL_PWM="PB2"
+CONFIG_USB_MUSB_SUNXI=y
CONFIG_DEFAULT_DEVICE_TREE="sun5i-a13-olinuxino"
# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
CONFIG_SPL=y
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 12/13] sunxi: A13-Olinuxino: Enable the USB OTG controller
2015-08-31 14:46 ` [U-Boot] [PATCH 12/13] sunxi: A13-Olinuxino: Enable the USB OTG controller Maxime Ripard
@ 2015-09-01 9:01 ` Hans de Goede
2015-09-03 21:41 ` Maxime Ripard
0 siblings, 1 reply; 65+ messages in thread
From: Hans de Goede @ 2015-09-01 9:01 UTC (permalink / raw)
To: u-boot
Hi,
On 31-08-15 16:46, Maxime Ripard wrote:
> The A13-Olinuxino has a mini-USB connector that can be used to power up
> the board and as an OTG connector.
>
> Since we have already some USB host-only ports right beside this one,
> enable it in gadget mode
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> configs/A13-OLinuXino_defconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/configs/A13-OLinuXino_defconfig b/configs/A13-OLinuXino_defconfig
> index 4b4337223ca5..70aa194b91c4 100644
> --- a/configs/A13-OLinuXino_defconfig
> +++ b/configs/A13-OLinuXino_defconfig
> @@ -3,6 +3,7 @@ CONFIG_ARCH_SUNXI=y
> CONFIG_MACH_SUN5I=y
> CONFIG_DRAM_CLK=408
> CONFIG_DRAM_EMR1=0
> +CONFIG_USB0_VBUS_DET="PG1"
> CONFIG_USB1_VBUS_PIN="PG11"
> CONFIG_AXP_GPIO=y
> # CONFIG_VIDEO_HDMI is not set
> @@ -11,6 +12,7 @@ CONFIG_VIDEO_VGA_VIA_LCD_FORCE_SYNC_ACTIVE_HIGH=y
> CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:3,vmode:0"
> CONFIG_VIDEO_LCD_POWER="AXP0-0"
> CONFIG_VIDEO_LCD_BL_PWM="PB2"
> +CONFIG_USB_MUSB_SUNXI=y
> CONFIG_DEFAULT_DEVICE_TREE="sun5i-a13-olinuxino"
> # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> CONFIG_SPL=y
The CONFIG_xxx defines for using MUSB in either host or gadget mode have
changed in v2015.10, looks like you need to rebase this series and fix
this.
Regards,
Hans
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 12/13] sunxi: A13-Olinuxino: Enable the USB OTG controller
2015-09-01 9:01 ` Hans de Goede
@ 2015-09-03 21:41 ` Maxime Ripard
2015-09-10 18:47 ` Hans de Goede
0 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-09-03 21:41 UTC (permalink / raw)
To: u-boot
Hi Hans,
On Tue, Sep 01, 2015 at 11:01:06AM +0200, Hans de Goede wrote:
> Hi,
>
> On 31-08-15 16:46, Maxime Ripard wrote:
> >The A13-Olinuxino has a mini-USB connector that can be used to power up
> >the board and as an OTG connector.
> >
> >Since we have already some USB host-only ports right beside this one,
> >enable it in gadget mode
> >
> >Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >---
> > configs/A13-OLinuXino_defconfig | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> >diff --git a/configs/A13-OLinuXino_defconfig b/configs/A13-OLinuXino_defconfig
> >index 4b4337223ca5..70aa194b91c4 100644
> >--- a/configs/A13-OLinuXino_defconfig
> >+++ b/configs/A13-OLinuXino_defconfig
> >@@ -3,6 +3,7 @@ CONFIG_ARCH_SUNXI=y
> > CONFIG_MACH_SUN5I=y
> > CONFIG_DRAM_CLK=408
> > CONFIG_DRAM_EMR1=0
> >+CONFIG_USB0_VBUS_DET="PG1"
> > CONFIG_USB1_VBUS_PIN="PG11"
> > CONFIG_AXP_GPIO=y
> > # CONFIG_VIDEO_HDMI is not set
> >@@ -11,6 +12,7 @@ CONFIG_VIDEO_VGA_VIA_LCD_FORCE_SYNC_ACTIVE_HIGH=y
> > CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:3,vmode:0"
> > CONFIG_VIDEO_LCD_POWER="AXP0-0"
> > CONFIG_VIDEO_LCD_BL_PWM="PB2"
> >+CONFIG_USB_MUSB_SUNXI=y
> > CONFIG_DEFAULT_DEVICE_TREE="sun5i-a13-olinuxino"
> > # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> > CONFIG_SPL=y
>
> The CONFIG_xxx defines for using MUSB in either host or gadget mode have
> changed in v2015.10, looks like you need to rebase this series and fix
> this.
This was based on 2015.10-rc2. I enabled it as a gadget through
menuconfig. What is the policy on using gadget vs host? I guess it
would make more sense to enable all the OTG connectors to gadget, but
maybe that's just me.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150903/f3d60092/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 12/13] sunxi: A13-Olinuxino: Enable the USB OTG controller
2015-09-03 21:41 ` Maxime Ripard
@ 2015-09-10 18:47 ` Hans de Goede
2015-09-13 17:13 ` Maxime Ripard
0 siblings, 1 reply; 65+ messages in thread
From: Hans de Goede @ 2015-09-10 18:47 UTC (permalink / raw)
To: u-boot
Hi,
On 03-09-15 23:41, Maxime Ripard wrote:
> Hi Hans,
>
> On Tue, Sep 01, 2015 at 11:01:06AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 31-08-15 16:46, Maxime Ripard wrote:
>>> The A13-Olinuxino has a mini-USB connector that can be used to power up
>>> the board and as an OTG connector.
>>>
>>> Since we have already some USB host-only ports right beside this one,
>>> enable it in gadget mode
>>>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> ---
>>> configs/A13-OLinuXino_defconfig | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/configs/A13-OLinuXino_defconfig b/configs/A13-OLinuXino_defconfig
>>> index 4b4337223ca5..70aa194b91c4 100644
>>> --- a/configs/A13-OLinuXino_defconfig
>>> +++ b/configs/A13-OLinuXino_defconfig
>>> @@ -3,6 +3,7 @@ CONFIG_ARCH_SUNXI=y
>>> CONFIG_MACH_SUN5I=y
>>> CONFIG_DRAM_CLK=408
>>> CONFIG_DRAM_EMR1=0
>>> +CONFIG_USB0_VBUS_DET="PG1"
>>> CONFIG_USB1_VBUS_PIN="PG11"
>>> CONFIG_AXP_GPIO=y
>>> # CONFIG_VIDEO_HDMI is not set
>>> @@ -11,6 +12,7 @@ CONFIG_VIDEO_VGA_VIA_LCD_FORCE_SYNC_ACTIVE_HIGH=y
>>> CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:3,vmode:0"
>>> CONFIG_VIDEO_LCD_POWER="AXP0-0"
>>> CONFIG_VIDEO_LCD_BL_PWM="PB2"
>>> +CONFIG_USB_MUSB_SUNXI=y
>>> CONFIG_DEFAULT_DEVICE_TREE="sun5i-a13-olinuxino"
>>> # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
>>> CONFIG_SPL=y
>>
>> The CONFIG_xxx defines for using MUSB in either host or gadget mode have
>> changed in v2015.10, looks like you need to rebase this series and fix
>> this.
>
> This was based on 2015.10-rc2.
Are you sure?
drivers/usb/musb-new/Kconfig has:
config USB_MUSB_HOST
bool "MUSB host mode support"
help
Enables the MUSB USB dual-role controller in host mode.
config USB_MUSB_GADGET
bool "MUSB gadget mode support"
help
Enables the MUSB USB dual-role controller in gadget mode.
if USB_MUSB_HOST || USB_MUSB_GADGET
config USB_MUSB_SUNXI
bool "Enable sunxi OTG / DRC USB controller"
depends on ARCH_SUNXI
default y
---help---
Say y here to enable support for the sunxi OTG / DRC USB controller
used on almost all sunxi boards.
endif
And you are not setting USB_MUSB_HOST nor USB_MUSB_GADGET so you should
never get a chance to set USB_MUSB_SUNXI. Also note that USB_MUSB_SUNXI
gets selected automatically as soon as you select either USB_MUSB_HOST
or USB_MUSB_GADGET...
> I enabled it as a gadget through
> menuconfig. What is the policy on using gadget vs host? I guess it
> would make more sense to enable all the OTG connectors to gadget, but
> maybe that's just me.
The policy I'm thinking of (and which we are currently sorta following
wrt enabling USB_MUSB_HOST) is that on devices with no normal usb
ports we use USB_MUSB_HOST, so that people can interact with u-boot
without needing a serial console (which often requires soldering).
On boards which do have a normal usb port I fully agree that
USB_MUSB_GADGET is a good default.
In practice this will likely boil down to use USB_MUSB_HOST on tablets
and USB_MUSB_GADGET everywhere else.
Regards,
Hans
^ permalink raw reply [flat|nested] 65+ messages in thread* [U-Boot] [PATCH 12/13] sunxi: A13-Olinuxino: Enable the USB OTG controller
2015-09-10 18:47 ` Hans de Goede
@ 2015-09-13 17:13 ` Maxime Ripard
2015-09-13 17:38 ` Hans de Goede
0 siblings, 1 reply; 65+ messages in thread
From: Maxime Ripard @ 2015-09-13 17:13 UTC (permalink / raw)
To: u-boot
On Thu, Sep 10, 2015 at 08:47:03PM +0200, Hans de Goede wrote:
> Hi,
>
> On 03-09-15 23:41, Maxime Ripard wrote:
> >Hi Hans,
> >
> >On Tue, Sep 01, 2015 at 11:01:06AM +0200, Hans de Goede wrote:
> >>Hi,
> >>
> >>On 31-08-15 16:46, Maxime Ripard wrote:
> >>>The A13-Olinuxino has a mini-USB connector that can be used to power up
> >>>the board and as an OTG connector.
> >>>
> >>>Since we have already some USB host-only ports right beside this one,
> >>>enable it in gadget mode
> >>>
> >>>Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >>>---
> >>> configs/A13-OLinuXino_defconfig | 2 ++
> >>> 1 file changed, 2 insertions(+)
> >>>
> >>>diff --git a/configs/A13-OLinuXino_defconfig b/configs/A13-OLinuXino_defconfig
> >>>index 4b4337223ca5..70aa194b91c4 100644
> >>>--- a/configs/A13-OLinuXino_defconfig
> >>>+++ b/configs/A13-OLinuXino_defconfig
> >>>@@ -3,6 +3,7 @@ CONFIG_ARCH_SUNXI=y
> >>> CONFIG_MACH_SUN5I=y
> >>> CONFIG_DRAM_CLK=408
> >>> CONFIG_DRAM_EMR1=0
> >>>+CONFIG_USB0_VBUS_DET="PG1"
> >>> CONFIG_USB1_VBUS_PIN="PG11"
> >>> CONFIG_AXP_GPIO=y
> >>> # CONFIG_VIDEO_HDMI is not set
> >>>@@ -11,6 +12,7 @@ CONFIG_VIDEO_VGA_VIA_LCD_FORCE_SYNC_ACTIVE_HIGH=y
> >>> CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:3,vmode:0"
> >>> CONFIG_VIDEO_LCD_POWER="AXP0-0"
> >>> CONFIG_VIDEO_LCD_BL_PWM="PB2"
> >>>+CONFIG_USB_MUSB_SUNXI=y
> >>> CONFIG_DEFAULT_DEVICE_TREE="sun5i-a13-olinuxino"
> >>> # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
> >>> CONFIG_SPL=y
> >>
> >>The CONFIG_xxx defines for using MUSB in either host or gadget mode have
> >>changed in v2015.10, looks like you need to rebase this series and fix
> >>this.
> >
> >This was based on 2015.10-rc2.
>
> Are you sure?
>
> drivers/usb/musb-new/Kconfig has:
>
> config USB_MUSB_HOST
> bool "MUSB host mode support"
> help
> Enables the MUSB USB dual-role controller in host mode.
>
> config USB_MUSB_GADGET
> bool "MUSB gadget mode support"
> help
> Enables the MUSB USB dual-role controller in gadget mode.
>
> if USB_MUSB_HOST || USB_MUSB_GADGET
>
> config USB_MUSB_SUNXI
> bool "Enable sunxi OTG / DRC USB controller"
> depends on ARCH_SUNXI
> default y
> ---help---
> Say y here to enable support for the sunxi OTG / DRC USB controller
> used on almost all sunxi boards.
>
> endif
>
> And you are not setting USB_MUSB_HOST nor USB_MUSB_GADGET so you should
> never get a chance to set USB_MUSB_SUNXI. Also note that USB_MUSB_SUNXI
> gets selected automatically as soon as you select either USB_MUSB_HOST
> or USB_MUSB_GADGET...
Hmmm, there's something fishy then. I maybe used menuconfig and forgot
to fix the defconfig or something. I don't see any other
explanation. Anyway, I'll update and fix it in v2, obviously :)
> > I enabled it as a gadget through menuconfig. What is the policy on
> >using gadget vs host? I guess it would make more sense to enable
> >all the OTG connectors to gadget, but maybe that's just me.
>
> The policy I'm thinking of (and which we are currently sorta following
> wrt enabling USB_MUSB_HOST) is that on devices with no normal usb
> ports we use USB_MUSB_HOST, so that people can interact with u-boot
> without needing a serial console (which often requires soldering).
>
> On boards which do have a normal usb port I fully agree that
> USB_MUSB_GADGET is a good default.
>
> In practice this will likely boil down to use USB_MUSB_HOST on tablets
> and USB_MUSB_GADGET everywhere else.
Sounds good!
If there's a UART gadget, we could also always select USB_MUSB_GADGET,
and export a UART there for the tablets, but I haven't seen such a
gadget in the code, so I guess your idea makes more sense :)
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150913/d622c906/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 12/13] sunxi: A13-Olinuxino: Enable the USB OTG controller
2015-09-13 17:13 ` Maxime Ripard
@ 2015-09-13 17:38 ` Hans de Goede
2015-09-14 21:19 ` Maxime Ripard
0 siblings, 1 reply; 65+ messages in thread
From: Hans de Goede @ 2015-09-13 17:38 UTC (permalink / raw)
To: u-boot
Hi,
On 13-09-15 19:13, Maxime Ripard wrote:
> On Thu, Sep 10, 2015 at 08:47:03PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 03-09-15 23:41, Maxime Ripard wrote:
>>> Hi Hans,
>>>
>>> On Tue, Sep 01, 2015 at 11:01:06AM +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 31-08-15 16:46, Maxime Ripard wrote:
>>>>> The A13-Olinuxino has a mini-USB connector that can be used to power up
>>>>> the board and as an OTG connector.
>>>>>
>>>>> Since we have already some USB host-only ports right beside this one,
>>>>> enable it in gadget mode
>>>>>
>>>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>>>> ---
>>>>> configs/A13-OLinuXino_defconfig | 2 ++
>>>>> 1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/configs/A13-OLinuXino_defconfig b/configs/A13-OLinuXino_defconfig
>>>>> index 4b4337223ca5..70aa194b91c4 100644
>>>>> --- a/configs/A13-OLinuXino_defconfig
>>>>> +++ b/configs/A13-OLinuXino_defconfig
>>>>> @@ -3,6 +3,7 @@ CONFIG_ARCH_SUNXI=y
>>>>> CONFIG_MACH_SUN5I=y
>>>>> CONFIG_DRAM_CLK=408
>>>>> CONFIG_DRAM_EMR1=0
>>>>> +CONFIG_USB0_VBUS_DET="PG1"
>>>>> CONFIG_USB1_VBUS_PIN="PG11"
>>>>> CONFIG_AXP_GPIO=y
>>>>> # CONFIG_VIDEO_HDMI is not set
>>>>> @@ -11,6 +12,7 @@ CONFIG_VIDEO_VGA_VIA_LCD_FORCE_SYNC_ACTIVE_HIGH=y
>>>>> CONFIG_VIDEO_LCD_MODE="x:800,y:480,depth:18,pclk_khz:33000,le:16,ri:209,up:22,lo:22,hs:30,vs:1,sync:3,vmode:0"
>>>>> CONFIG_VIDEO_LCD_POWER="AXP0-0"
>>>>> CONFIG_VIDEO_LCD_BL_PWM="PB2"
>>>>> +CONFIG_USB_MUSB_SUNXI=y
>>>>> CONFIG_DEFAULT_DEVICE_TREE="sun5i-a13-olinuxino"
>>>>> # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
>>>>> CONFIG_SPL=y
>>>>
>>>> The CONFIG_xxx defines for using MUSB in either host or gadget mode have
>>>> changed in v2015.10, looks like you need to rebase this series and fix
>>>> this.
>>>
>>> This was based on 2015.10-rc2.
>>
>> Are you sure?
>>
>> drivers/usb/musb-new/Kconfig has:
>>
>> config USB_MUSB_HOST
>> bool "MUSB host mode support"
>> help
>> Enables the MUSB USB dual-role controller in host mode.
>>
>> config USB_MUSB_GADGET
>> bool "MUSB gadget mode support"
>> help
>> Enables the MUSB USB dual-role controller in gadget mode.
>>
>> if USB_MUSB_HOST || USB_MUSB_GADGET
>>
>> config USB_MUSB_SUNXI
>> bool "Enable sunxi OTG / DRC USB controller"
>> depends on ARCH_SUNXI
>> default y
>> ---help---
>> Say y here to enable support for the sunxi OTG / DRC USB controller
>> used on almost all sunxi boards.
>>
>> endif
>>
>> And you are not setting USB_MUSB_HOST nor USB_MUSB_GADGET so you should
>> never get a chance to set USB_MUSB_SUNXI. Also note that USB_MUSB_SUNXI
>> gets selected automatically as soon as you select either USB_MUSB_HOST
>> or USB_MUSB_GADGET...
>
> Hmmm, there's something fishy then. I maybe used menuconfig and forgot
> to fix the defconfig or something. I don't see any other
> explanation. Anyway, I'll update and fix it in v2, obviously :)
>
>>> I enabled it as a gadget through menuconfig. What is the policy on
>>> using gadget vs host? I guess it would make more sense to enable
>>> all the OTG connectors to gadget, but maybe that's just me.
>>
>> The policy I'm thinking of (and which we are currently sorta following
>> wrt enabling USB_MUSB_HOST) is that on devices with no normal usb
>> ports we use USB_MUSB_HOST, so that people can interact with u-boot
>> without needing a serial console (which often requires soldering).
>>
>> On boards which do have a normal usb port I fully agree that
>> USB_MUSB_GADGET is a good default.
>>
>> In practice this will likely boil down to use USB_MUSB_HOST on tablets
>> and USB_MUSB_GADGET everywhere else.
>
> Sounds good!
>
> If there's a UART gadget, we could also always select USB_MUSB_GADGET,
> and export a UART there for the tablets, but I haven't seen such a
> gadget in the code, so I guess your idea makes more sense :)
Ack. AFAIK u-boot currently lacks a serial gadget, once it gets one
things get somewhat more complicated, for debugging a serial gadget
is fine. But for now (no usable tablet ui) I sorta expect people to
want to have hooked up a usb-keyb to a tablet (which is not running
android) anyways, at which point the host option makes more sense.
Ideally we would build in both, and decide which one to use based
on the id-pin. The sunxi musb-host code already checks the id-pin
and does not touch the controller if it is not pulled down (iow
if no host cable is inserted).
Regards,
Hans
>
> Maxime
>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 12/13] sunxi: A13-Olinuxino: Enable the USB OTG controller
2015-09-13 17:38 ` Hans de Goede
@ 2015-09-14 21:19 ` Maxime Ripard
0 siblings, 0 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-09-14 21:19 UTC (permalink / raw)
To: u-boot
On Sun, Sep 13, 2015 at 07:38:27PM +0200, Hans de Goede wrote:
> >>The policy I'm thinking of (and which we are currently sorta following
> >>wrt enabling USB_MUSB_HOST) is that on devices with no normal usb
> >>ports we use USB_MUSB_HOST, so that people can interact with u-boot
> >>without needing a serial console (which often requires soldering).
> >>
> >>On boards which do have a normal usb port I fully agree that
> >>USB_MUSB_GADGET is a good default.
> >>
> >>In practice this will likely boil down to use USB_MUSB_HOST on tablets
> >>and USB_MUSB_GADGET everywhere else.
> >
> >Sounds good!
> >
> >If there's a UART gadget, we could also always select USB_MUSB_GADGET,
> >and export a UART there for the tablets, but I haven't seen such a
> >gadget in the code, so I guess your idea makes more sense :)
>
> Ack. AFAIK u-boot currently lacks a serial gadget, once it gets one
> things get somewhat more complicated, for debugging a serial gadget
> is fine. But for now (no usable tablet ui) I sorta expect people to
> want to have hooked up a usb-keyb to a tablet (which is not running
> android) anyways, at which point the host option makes more sense.
>
> Ideally we would build in both, and decide which one to use based
> on the id-pin. The sunxi musb-host code already checks the id-pin
> and does not touch the controller if it is not pulled down (iow
> if no host cable is inserted).
Looks like a good plan. I'll update my patch and resend it.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150914/b99091b2/attachment.sig>
^ permalink raw reply [flat|nested] 65+ messages in thread
* [U-Boot] [PATCH 13/13] sunxi: cubietruck: Enable the USB OTG controller
2015-08-31 14:46 [U-Boot] [PATCH 00/13] Implement fastboot over NAND Maxime Ripard
` (11 preceding siblings ...)
2015-08-31 14:46 ` [U-Boot] [PATCH 12/13] sunxi: A13-Olinuxino: Enable the USB OTG controller Maxime Ripard
@ 2015-08-31 14:46 ` Maxime Ripard
12 siblings, 0 replies; 65+ messages in thread
From: Maxime Ripard @ 2015-08-31 14:46 UTC (permalink / raw)
To: u-boot
The Cubietruck has a mini-USB connector that can be used to power up the
board and as an OTG connector.
Since we have already some USB host-only ports right beside this one,
enable it in gadget mode
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
configs/Cubietruck_defconfig | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/configs/Cubietruck_defconfig b/configs/Cubietruck_defconfig
index b8809c806b3f..59e8cb6ea339 100644
--- a/configs/Cubietruck_defconfig
+++ b/configs/Cubietruck_defconfig
@@ -13,3 +13,8 @@ CONFIG_SYS_EXTRA_OPTIONS="AXP209_POWER,SUNXI_GMAC,RGMII,AHCI,SATAPWR=SUNXI_GPH(1
# CONFIG_CMD_FPGA is not set
CONFIG_ETH_DESIGNWARE=y
CONFIG_USB_EHCI_HCD=y
+CONFIG_USB0_ID_DET="PH19"
+CONFIG_USB0_VBUS_DET="PH22"
+CONFIG_USB0_VBUS_PIN="PH17"
+CONFIG_USB_MUSB_SUNXI=y
+CONFIG_USB_MUSB_GADGET=y
--
2.5.0
^ permalink raw reply related [flat|nested] 65+ messages in thread