diff mbox

[OpenWrt-Devel,RFT] kernel: mtdsplit_uimage: debug buf/header sizes

Message ID 1423741634-19137-1-git-send-email-zajec5@gmail.com
State RFC
Delegated to: Rafał Miłecki
Headers show

Commit Message

Rafał Miłecki Feb. 12, 2015, 11:47 a.m. UTC 
---
John can you give it a try, please?
---
 .../generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c  | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

John Crispin Feb. 12, 2015, 7:25 p.m. UTC | #1 
yep, fixes the problem. please push it :)

On 12/02/2015 12:47, Rafał Miłecki wrote:
> ---
> John can you give it a try, please?
> ---
>  .../generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c  | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
> index 2bb5e9a..4abc4be 100644
> --- a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
> +++ b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
> @@ -20,6 +20,8 @@
>  
>  #include "mtdsplit.h"
>  
> +static int first_try = 1;
> +
>  /*
>   * uimage_header itself is only 64B, but it may be prepended with another data.
>   * Currently the biggest size is for Edimax devices: 20B + 64B
> @@ -60,6 +62,8 @@ read_uimage_header(struct mtd_info *mtd, size_t offset, u_char *buf,
>  	size_t retlen;
>  	int ret;
>  
> +	if (first_try)
> +		pr_info("[%s] buf:%p header_len:%zu\n", __FUNCTION__, buf, header_len);
>  	ret = mtd_read(mtd, offset, header_len, &retlen, buf);
>  	if (ret) {
>  		pr_debug("read error in \"%s\"\n", mtd->name);
> @@ -106,11 +110,15 @@ static int __mtdsplit_parse_uimage(struct mtd_info *master,
>  		ret = -ENOMEM;
>  		goto err_free_parts;
>  	}
> +	if (first_try)
> +		pr_info("[%s] buf:%p sizeof(*buf):%u\n", __FUNCTION__, buf, sizeof(*buf));
>  
>  	/* find uImage on erase block boundaries */
>  	for (offset = 0; offset < master->size; offset += master->erasesize) {
>  		struct uimage_header *header;
>  
> +		if (first_try)
> +			pr_info("[%s] sizeof(*header):%u\n", __FUNCTION__, sizeof(*header));
>  		uimage_size = 0;
>  
>  		ret = read_uimage_header(master, offset, buf, sizeof(*buf));
> @@ -308,8 +316,13 @@ static ssize_t uimage_find_edimax(u_char *buf, size_t len)
>  {
>  	struct uimage_header *header;
>  
> +	if (first_try)
> +		pr_info("[%s] buf:%p len:%zu FW_EDIMAX_OFFSET + sizeof(*header):%u\n", __FUNCTION__, buf, len, FW_EDIMAX_OFFSET + sizeof(*header));
> +
>  	if (len < FW_EDIMAX_OFFSET + sizeof(*header)) {
> -		pr_err("Buffer too small for checking Edimax header\n");
> +		if (first_try)
> +			pr_err("Buffer too small for checking Edimax header\n");
> +		first_try = 0;
>  		return -ENOSPC;
>  	}
>  
>
Rafał Miłecki Feb. 12, 2015, 7:26 p.m. UTC | #2 
On 12 February 2015 at 20:25, John Crispin <blogic@openwrt.org> wrote:
> yep, fixes the problem. please push it :)

It just hides the problem, there is still something wrong with sizes.
This patch it printing messages to help track the bug. Please provide
an output
John Crispin Feb. 12, 2015, 8:04 p.m. UTC | #3 
[    0.564000] Creating 4 MTD partitions on "spi32766.0":
[    0.568000] 0x000000000000-0x000000010000 : "uboot"
[    0.572000] 0x000000010000-0x000000020000 : "uboot-env"
[    0.580000] 0x000000020000-0x000000030000 : "calibration"
[    0.584000] 0x000000050000-0x000000ff0000 : "firmware"
[    0.596000] mtdsplit_uimage: [__mtdsplit_parse_uimage] buf:c0049000
sizeof(*buf):1
[    0.600000] mtdsplit_uimage: [__mtdsplit_parse_uimage] sizeof(*header):64
[    0.608000] mtdsplit_uimage: [read_uimage_header] buf:c0049000
header_len:1
[    0.616000] mtdsplit_uimage: [uimage_find_edimax] buf:c0049000 len:1
FW_EDIMAX_OFFSET + sizeof(*header):84
[    0.624000] mtdsplit_uimage: Buffer too small for checking Edimax header
[    0.784000] Dedicated partitioner didn't split firmware partition,
please fill a bug report!
[    0.788000] 0x000000172108-0x000000ff0000 : "rootfs"
[    0.796000] mtd: partition "rootfs" must either start or end on erase
block boundary or be smaller than an erase block -- forcing read-only
[    0.808000] mtd: device 4 (rootfs) set to be root filesystem
[    0.812000] 1 squashfs-split partitions found on MTD device rootfs
[    0.820000] 0x000000340000-0x000000ff0000 : "rootfs_data"
[    1.916000] Realtek RTL8366RB ethernet switch driver version 0.2.4
[    1.924000] rtl8366rb rtl8366rb: using GPIO pins 491 (SDA) and 493 (SCK)
[    1.928000] rtl8366rb rtl8366rb: RTL5937 ver. 3 chip found





On 12/02/2015 12:47, Rafał Miłecki wrote:
> ---
> John can you give it a try, please?
> ---
>  .../generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c  | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
> index 2bb5e9a..4abc4be 100644
> --- a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
> +++ b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
> @@ -20,6 +20,8 @@
>  
>  #include "mtdsplit.h"
>  
> +static int first_try = 1;
> +
>  /*
>   * uimage_header itself is only 64B, but it may be prepended with another data.
>   * Currently the biggest size is for Edimax devices: 20B + 64B
> @@ -60,6 +62,8 @@ read_uimage_header(struct mtd_info *mtd, size_t offset, u_char *buf,
>  	size_t retlen;
>  	int ret;
>  
> +	if (first_try)
> +		pr_info("[%s] buf:%p header_len:%zu\n", __FUNCTION__, buf, header_len);
>  	ret = mtd_read(mtd, offset, header_len, &retlen, buf);
>  	if (ret) {
>  		pr_debug("read error in \"%s\"\n", mtd->name);
> @@ -106,11 +110,15 @@ static int __mtdsplit_parse_uimage(struct mtd_info *master,
>  		ret = -ENOMEM;
>  		goto err_free_parts;
>  	}
> +	if (first_try)
> +		pr_info("[%s] buf:%p sizeof(*buf):%u\n", __FUNCTION__, buf, sizeof(*buf));
>  
>  	/* find uImage on erase block boundaries */
>  	for (offset = 0; offset < master->size; offset += master->erasesize) {
>  		struct uimage_header *header;
>  
> +		if (first_try)
> +			pr_info("[%s] sizeof(*header):%u\n", __FUNCTION__, sizeof(*header));
>  		uimage_size = 0;
>  
>  		ret = read_uimage_header(master, offset, buf, sizeof(*buf));
> @@ -308,8 +316,13 @@ static ssize_t uimage_find_edimax(u_char *buf, size_t len)
>  {
>  	struct uimage_header *header;
>  
> +	if (first_try)
> +		pr_info("[%s] buf:%p len:%zu FW_EDIMAX_OFFSET + sizeof(*header):%u\n", __FUNCTION__, buf, len, FW_EDIMAX_OFFSET + sizeof(*header));
> +
>  	if (len < FW_EDIMAX_OFFSET + sizeof(*header)) {
> -		pr_err("Buffer too small for checking Edimax header\n");
> +		if (first_try)
> +			pr_err("Buffer too small for checking Edimax header\n");
> +		first_try = 0;
>  		return -ENOSPC;
>  	}
>  
>
Rafał Miłecki Feb. 12, 2015, 8:18 p.m. UTC | #4 
On 12 February 2015 at 21:04, John Crispin <blogic@openwrt.org> wrote:
> [    0.564000] Creating 4 MTD partitions on "spi32766.0":
> [    0.568000] 0x000000000000-0x000000010000 : "uboot"
> [    0.572000] 0x000000010000-0x000000020000 : "uboot-env"
> [    0.580000] 0x000000020000-0x000000030000 : "calibration"
> [    0.584000] 0x000000050000-0x000000ff0000 : "firmware"
> [    0.596000] mtdsplit_uimage: [__mtdsplit_parse_uimage] buf:c0049000
> sizeof(*buf):1
> [    0.600000] mtdsplit_uimage: [__mtdsplit_parse_uimage] sizeof(*header):64
> [    0.608000] mtdsplit_uimage: [read_uimage_header] buf:c0049000
> header_len:1
> [    0.616000] mtdsplit_uimage: [uimage_find_edimax] buf:c0049000 len:1
> FW_EDIMAX_OFFSET + sizeof(*header):84
> [    0.624000] mtdsplit_uimage: Buffer too small for checking Edimax header
> [    0.784000] Dedicated partitioner didn't split firmware partition,
> please fill a bug report!
> [    0.788000] 0x000000172108-0x000000ff0000 : "rootfs"
> [    0.796000] mtd: partition "rootfs" must either start or end on erase
> block boundary or be smaller than an erase block -- forcing read-only
> [    0.808000] mtd: device 4 (rootfs) set to be root filesystem
> [    0.812000] 1 squashfs-split partitions found on MTD device rootfs
> [    0.820000] 0x000000340000-0x000000ff0000 : "rootfs_data"
> [    1.916000] Realtek RTL8366RB ethernet switch driver version 0.2.4
> [    1.924000] rtl8366rb rtl8366rb: using GPIO pins 491 (SDA) and 493 (SCK)
> [    1.928000] rtl8366rb rtl8366rb: RTL5937 ver. 3 chip found

Thank you! My obvious bug was fixed in
https://dev.openwrt.org/changeset/44424/

Please note that if you still get
> Dedicated partitioner didn't split firmware partition, please fill a bug report!
with 44424, it means that there is something wrong in
target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
and we/you should look for a working reference code in
405-mtd-old-firmware-uimage-splitter.patch
John Crispin Feb. 12, 2015, 8:20 p.m. UTC | #5 
On 12/02/2015 21:18, Rafał Miłecki wrote:
> On 12 February 2015 at 21:04, John Crispin <blogic@openwrt.org> wrote:
>> [    0.564000] Creating 4 MTD partitions on "spi32766.0":
>> [    0.568000] 0x000000000000-0x000000010000 : "uboot"
>> [    0.572000] 0x000000010000-0x000000020000 : "uboot-env"
>> [    0.580000] 0x000000020000-0x000000030000 : "calibration"
>> [    0.584000] 0x000000050000-0x000000ff0000 : "firmware"
>> [    0.596000] mtdsplit_uimage: [__mtdsplit_parse_uimage] buf:c0049000
>> sizeof(*buf):1
>> [    0.600000] mtdsplit_uimage: [__mtdsplit_parse_uimage] sizeof(*header):64
>> [    0.608000] mtdsplit_uimage: [read_uimage_header] buf:c0049000
>> header_len:1
>> [    0.616000] mtdsplit_uimage: [uimage_find_edimax] buf:c0049000 len:1
>> FW_EDIMAX_OFFSET + sizeof(*header):84
>> [    0.624000] mtdsplit_uimage: Buffer too small for checking Edimax header
>> [    0.784000] Dedicated partitioner didn't split firmware partition,
>> please fill a bug report!
>> [    0.788000] 0x000000172108-0x000000ff0000 : "rootfs"
>> [    0.796000] mtd: partition "rootfs" must either start or end on erase
>> block boundary or be smaller than an erase block -- forcing read-only
>> [    0.808000] mtd: device 4 (rootfs) set to be root filesystem
>> [    0.812000] 1 squashfs-split partitions found on MTD device rootfs
>> [    0.820000] 0x000000340000-0x000000ff0000 : "rootfs_data"
>> [    1.916000] Realtek RTL8366RB ethernet switch driver version 0.2.4
>> [    1.924000] rtl8366rb rtl8366rb: using GPIO pins 491 (SDA) and 493 (SCK)
>> [    1.928000] rtl8366rb rtl8366rb: RTL5937 ver. 3 chip found
> 
> Thank you! My obvious bug was fixed in
> https://dev.openwrt.org/changeset/44424/
> 

they tend to be the hardest to find ;)

> Please note that if you still get
>> Dedicated partitioner didn't split firmware partition, please fill a bug report!
> with 44424, it means that there is something wrong in
> target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
> and we/you should look for a working reference code in
> 405-mtd-old-firmware-uimage-splitter.patch
>
diff mbox

Patch

diff --git a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
index 2bb5e9a..4abc4be 100644
--- a/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
+++ b/target/linux/generic/files/drivers/mtd/mtdsplit/mtdsplit_uimage.c
@@ -20,6 +20,8 @@ 
 
 #include "mtdsplit.h"
 
+static int first_try = 1;
+
 /*
  * uimage_header itself is only 64B, but it may be prepended with another data.
  * Currently the biggest size is for Edimax devices: 20B + 64B
@@ -60,6 +62,8 @@  read_uimage_header(struct mtd_info *mtd, size_t offset, u_char *buf,
 	size_t retlen;
 	int ret;
 
+	if (first_try)
+		pr_info("[%s] buf:%p header_len:%zu\n", __FUNCTION__, buf, header_len);
 	ret = mtd_read(mtd, offset, header_len, &retlen, buf);
 	if (ret) {
 		pr_debug("read error in \"%s\"\n", mtd->name);
@@ -106,11 +110,15 @@  static int __mtdsplit_parse_uimage(struct mtd_info *master,
 		ret = -ENOMEM;
 		goto err_free_parts;
 	}
+	if (first_try)
+		pr_info("[%s] buf:%p sizeof(*buf):%u\n", __FUNCTION__, buf, sizeof(*buf));
 
 	/* find uImage on erase block boundaries */
 	for (offset = 0; offset < master->size; offset += master->erasesize) {
 		struct uimage_header *header;
 
+		if (first_try)
+			pr_info("[%s] sizeof(*header):%u\n", __FUNCTION__, sizeof(*header));
 		uimage_size = 0;
 
 		ret = read_uimage_header(master, offset, buf, sizeof(*buf));
@@ -308,8 +316,13 @@  static ssize_t uimage_find_edimax(u_char *buf, size_t len)
 {
 	struct uimage_header *header;
 
+	if (first_try)
+		pr_info("[%s] buf:%p len:%zu FW_EDIMAX_OFFSET + sizeof(*header):%u\n", __FUNCTION__, buf, len, FW_EDIMAX_OFFSET + sizeof(*header));
+
 	if (len < FW_EDIMAX_OFFSET + sizeof(*header)) {
-		pr_err("Buffer too small for checking Edimax header\n");
+		if (first_try)
+			pr_err("Buffer too small for checking Edimax header\n");
+		first_try = 0;
 		return -ENOSPC;
 	}