[U-Boot] [PATCH 0/8] assortment of fixes/enhancements

From: Marcel Ziswiler marcel.ziswiler@toradex.com
This patch set is an assortment of fixes/enhancements distilled straight from our downstream integration work.
Marcel Ziswiler (4): net: asix: fix operation without eeprom generic-board: allow showing custom board info colibri_vf: remove spurious new line mtd/nand/ubi: assortment of alignment fixes
Max Krummenacher (3): fs/fs.c: read up to EOF when len would read past EOF image-fdt.c: store returned error value tftp.c: fix CONFIG_TFTP_TSIZE for small files
Stefan Agner (1): logos: add Toradex logo
common/board_info.c | 2 +- common/cmd_ubi.c | 2 +- common/image-fdt.c | 3 ++- drivers/mtd/nand/nand_util.c | 2 +- drivers/usb/eth/asix.c | 27 ++++++++++++++++++++++++++- fs/fs.c | 6 ++---- fs/ubifs/super.c | 4 ++-- fs/ubifs/ubifs.c | 4 ++-- include/configs/colibri_vf.h | 1 - lib/gzip.c | 2 +- net/tftp.c | 2 ++ tools/logos/toradex.bmp | Bin 0 -> 24982 bytes 12 files changed, 40 insertions(+), 15 deletions(-) create mode 100644 tools/logos/toradex.bmp

From: Max Krummenacher max.krummenacher@toradex.com
http://lists.denx.de/pipermail/u-boot/2012-September/134347.html allows for reading files in chunks from the shell.
When this feature is used to read past the end of a file an error was returned instead of returning the bytes read up to the end of file. Thus the following fails in the shell:
offset = 0 len = chunksize do read file, offset, len write data until bytes_read < len
The patch changes the behaviour to printing an informational message and returning the actual read number of bytes.
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- fs/fs.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/fs.c b/fs/fs.c index ac0897d..827b143 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -301,10 +301,8 @@ int fs_read(const char *filename, ulong addr, loff_t offset, loff_t len, unmap_sysmem(buf);
/* If we requested a specific number of bytes, check we got it */ - if (ret == 0 && len && *actread != len) { - printf("** Unable to read file %s **\n", filename); - ret = -1; - } + if (ret == 0 && len && *actread != len) + printf("** %s shorter than offset + len **\n", filename); fs_close();
return ret;

On Thursday, July 02, 2015 at 01:04:46 AM, Marcel Ziswiler wrote:
From: Max Krummenacher max.krummenacher@toradex.com
http://lists.denx.de/pipermail/u-boot/2012-September/134347.html allows for reading files in chunks from the shell.
When this feature is used to read past the end of a file an error was returned instead of returning the bytes read up to the end of file. Thus the following fails in the shell:
offset = 0 len = chunksize do read file, offset, len write data until bytes_read < len
The patch changes the behaviour to printing an informational message and returning the actual read number of bytes.
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
So this behaves like read(2) now, right ?
Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut

On 2 July 2015 07:46:19 CEST, Marek Vasut marex@denx.de wrote:
So this behaves like read(2) now, right ?
Exactly for convenient use in U-Boot scripts.

From: Marcel Ziswiler marcel.ziswiler@toradex.com
This patch fixes operation of our on-board AX88772B chip without EEPROM but with a ethaddr coming from the regular U-Boot environment. This is a forward port of some remaining parts initially implemented by Antmicro.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- drivers/usb/eth/asix.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c index c8697ae..3926d50 100644 --- a/drivers/usb/eth/asix.c +++ b/drivers/usb/eth/asix.c @@ -1,5 +1,8 @@ /* * Copyright (c) 2011 The Chromium OS Authors. + * Copyright (c) 2012-2014 Toradex, Inc. + * + * Patched for AX88772B by Antmicro Ltd <www.antmicro.com> * * SPDX-License-Identifier: GPL-2.0+ */ @@ -64,8 +67,14 @@ AX_MEDIUM_AC | AX_MEDIUM_RE)
/* AX88772 & AX88178 RX_CTL values */ +#define AX_RX_CTL_RH2M 0x0200 /* Enable IP header in receive + buffer aligned on 32-bit + boundary */ +#define AX_RX_CTL_RH1M 0x0100 /* Enable RX-Header mode 0 */ #define AX_RX_CTL_SO 0x0080 #define AX_RX_CTL_AB 0x0008 +#define AX_RX_HEADER_DEFAULT (AX_RX_CTL_RH1M | \ + AX_RX_CTL_RH2M)
#define AX_DEFAULT_RX_CTL \ (AX_RX_CTL_SO | AX_RX_CTL_AB) @@ -426,7 +435,15 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
debug("** %s()\n", __func__);
- if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0) + if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) && + (dev->pusb_dev->descriptor.idProduct == 0x772b)) { + if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL | + AX_RX_HEADER_DEFAULT) < 0) + goto out_err; + } else if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0) + goto out_err; + + if (asix_write_hwaddr(eth) < 0) goto out_err;
do { @@ -447,6 +464,10 @@ static int asix_init(struct eth_device *eth, bd_t *bd) goto out_err; }
+ /* Wait some more to avoid timeout on first transfer + (e.g. EHCI timed out on TD - token=0x8008d80) */ + udelay(25000); + return 0; out_err: return -1; @@ -533,6 +554,10 @@ static int asix_recv(struct eth_device *eth) return -1; }
+ if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) && + (dev->pusb_dev->descriptor.idProduct == 0x772b)) + buf_ptr += 2; + /* Notify net stack */ net_process_received_packet(buf_ptr + sizeof(packet_len), packet_len);

On Thursday, July 02, 2015 at 01:04:47 AM, Marcel Ziswiler wrote:
Hi!
[...]
@@ -64,8 +67,14 @@ AX_MEDIUM_AC | AX_MEDIUM_RE)
/* AX88772 & AX88178 RX_CTL values */ +#define AX_RX_CTL_RH2M 0x0200 /* Enable IP header in
receive
buffer aligned on 32-bit
boundary */
The comments need a bit of polishing, though it is not the main problem I have with this patch. The multiline comments should be like this according to kernel coding style (to my knowledge):
/* * foo * bar * baz */
+#define AX_RX_CTL_RH1M 0x0100 /* Enable RX-Header mode
0 */
#define AX_RX_CTL_SO 0x0080 #define AX_RX_CTL_AB 0x0008 +#define AX_RX_HEADER_DEFAULT (AX_RX_CTL_RH1M | \
AX_RX_CTL_RH2M)
#define AX_DEFAULT_RX_CTL \ (AX_RX_CTL_SO | AX_RX_CTL_AB) @@ -426,7 +435,15 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
debug("** %s()\n", __func__);
- if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0)
- if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) &&
(dev->pusb_dev->descriptor.idProduct == 0x772b)) {
I don't like hardcoding these constants here (and further down). I understand that those are AX88792B chips (or whatever the number is, there's a B at the end and they're not exactly compatible with the original AX88792), but what about making this a bit more generic?
What I expect is that when AX88792C comes, we'd just add another if (idVendor == ... ) into this code here with another magic number and it will become an unmaintainable horror.
Maybe add a function which handles quirks of each revision (B, C, ...) of the ASIX chip and definitelly define those magic numbers as macros.
if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL |
AX_RX_HEADER_DEFAULT) < 0)
goto out_err;
} else if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0)
goto out_err;
if (asix_write_hwaddr(eth) < 0) goto out_err;
do {
[...]

On 2 July 2015 07:50:59 CEST, Marek Vasut marex@denx.de wrote:
On Thursday, July 02, 2015 at 01:04:47 AM, Marcel Ziswiler wrote:
Hi!
[...]
@@ -64,8 +67,14 @@ AX_MEDIUM_AC | AX_MEDIUM_RE)
/* AX88772 & AX88178 RX_CTL values */ +#define AX_RX_CTL_RH2M 0x0200 /* Enable IP header in
receive
buffer aligned on 32-bit
boundary */
The comments need a bit of polishing, though it is not the main problem I have with this patch.
I was hesitant at first but then decided to submit it anyway to get some feedback on the thematic. So thank you very much!
The multiline comments should be like this according to kernel coding style (to my knowledge):
/*
- foo
- bar
- baz
*/
Yeah, sorry. My bad. I since got educated in doing this but stumble over it at times on older patches.
+#define AX_RX_CTL_RH1M 0x0100 /* Enable RX-Header mode
0 */
#define AX_RX_CTL_SO 0x0080 #define AX_RX_CTL_AB 0x0008 +#define AX_RX_HEADER_DEFAULT (AX_RX_CTL_RH1M | \
AX_RX_CTL_RH2M)
#define AX_DEFAULT_RX_CTL \ (AX_RX_CTL_SO | AX_RX_CTL_AB) @@ -426,7 +435,15 @@ static int asix_init(struct eth_device *eth,
bd_t *bd)
debug("** %s()\n", __func__);
- if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0)
- if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) &&
(dev->pusb_dev->descriptor.idProduct == 0x772b)) {
I don't like hardcoding these constants here (and further down). I understand that those are AX88792B chips (or whatever the number is, there's a B at the end and they're not exactly compatible with the original AX88792), but what about making this a bit more generic?
AX88772B actually and yes there seem to be C variants of that same chip out now as well but we haven't gotten our hands on any such yet. I just do remember that ASIX does not take backwards compatibility too serious.
What I expect is that when AX88792C comes, we'd just add another if (idVendor == ... ) into this code here with another magic number and it will become an unmaintainable horror.
Understood.
Maybe add a function which handles quirks of each revision (B, C, ...) of the ASIX chip and definitelly define those magic numbers as macros.
Agreed.
if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL |
AX_RX_HEADER_DEFAULT) < 0)
goto out_err;
} else if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0)
goto out_err;
if (asix_write_hwaddr(eth) < 0) goto out_err;
do {
[...]

On Thursday, July 02, 2015 at 08:12:32 AM, Marcel Ziswiler wrote:
On 2 July 2015 07:50:59 CEST, Marek Vasut marex@denx.de wrote:
On Thursday, July 02, 2015 at 01:04:47 AM, Marcel Ziswiler wrote:
Hi!
Hi!
[...]
@@ -64,8 +67,14 @@
AX_MEDIUM_AC | AX_MEDIUM_RE)
/* AX88772 & AX88178 RX_CTL values */
+#define AX_RX_CTL_RH2M 0x0200 /* Enable IP header in
receive
buffer aligned on 32-bit
boundary */
The comments need a bit of polishing, though it is not the main problem I have with this patch.
I was hesitant at first but then decided to submit it anyway to get some feedback on the thematic. So thank you very much!
No worries :)
The multiline comments should be like this according to kernel coding style (to my knowledge):
/*
- foo
- bar
- baz
*/
Yeah, sorry. My bad. I since got educated in doing this but stumble over it at times on older patches.
Yeah, the code in certain areas of U-Boot isn't the pinacle of coding style excellence for sure.
+#define AX_RX_CTL_RH1M 0x0100 /* Enable RX-Header mode
0 */
#define AX_RX_CTL_SO 0x0080 #define AX_RX_CTL_AB 0x0008
+#define AX_RX_HEADER_DEFAULT (AX_RX_CTL_RH1M | \
AX_RX_CTL_RH2M)
#define AX_DEFAULT_RX_CTL \
(AX_RX_CTL_SO | AX_RX_CTL_AB)
@@ -426,7 +435,15 @@ static int asix_init(struct eth_device *eth,
bd_t *bd)
debug("** %s()\n", __func__);
- if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0)
- if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) &&
(dev->pusb_dev->descriptor.idProduct == 0x772b)) {
I don't like hardcoding these constants here (and further down). I understand that those are AX88792B chips (or whatever the number is, there's a B at the end and they're not exactly compatible with the original AX88792), but what about making this a bit more generic?
AX88772B actually and yes there seem to be C variants of that same chip out now as well but we haven't gotten our hands on any such yet. I just do remember that ASIX does not take backwards compatibility too serious.
Yeah, that I can confirm this.
What I expect is that when AX88792C comes, we'd just add another if (idVendor == ... ) into this code here with another magic number and it will become an unmaintainable horror.
Understood.
Maybe add a function which handles quirks of each revision (B, C, ...) of the ASIX chip and definitelly define those magic numbers as macros.
Agreed.
OK, thanks! :-)
[...]

Hi Marcel,
On Wed, Jul 1, 2015 at 6:04 PM, Marcel Ziswiler marcel@ziswiler.com wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
This patch fixes operation of our on-board AX88772B chip without EEPROM but with a ethaddr coming from the regular U-Boot environment. This is a forward port of some remaining parts initially implemented by Antmicro.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
drivers/usb/eth/asix.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c index c8697ae..3926d50 100644 --- a/drivers/usb/eth/asix.c +++ b/drivers/usb/eth/asix.c @@ -1,5 +1,8 @@ /*
- Copyright (c) 2011 The Chromium OS Authors.
- Copyright (c) 2012-2014 Toradex, Inc.
*/
- Patched for AX88772B by Antmicro Ltd <www.antmicro.com>
- SPDX-License-Identifier: GPL-2.0+
@@ -64,8 +67,14 @@ AX_MEDIUM_AC | AX_MEDIUM_RE)
/* AX88772 & AX88178 RX_CTL values */ +#define AX_RX_CTL_RH2M 0x0200 /* Enable IP header in receive
buffer aligned on 32-bit
boundary */
Definitely use checkpatch.pl on this series. I recommend patman.
+#define AX_RX_CTL_RH1M 0x0100 /* Enable RX-Header mode 0 */ #define AX_RX_CTL_SO 0x0080 #define AX_RX_CTL_AB 0x0008 +#define AX_RX_HEADER_DEFAULT (AX_RX_CTL_RH1M | \
AX_RX_CTL_RH2M)
I'm guessing this is ported from the ASIX Linux driver. Perhaps mention in the log where it came from.
#define AX_DEFAULT_RX_CTL \ (AX_RX_CTL_SO | AX_RX_CTL_AB) @@ -426,7 +435,15 @@ static int asix_init(struct eth_device *eth, bd_t *bd)
debug("** %s()\n", __func__);
if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0)
if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) &&
(dev->pusb_dev->descriptor.idProduct == 0x772b)) {
Please don't use magic numbers. Also, ideally don't create PID hacks.
if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL |
AX_RX_HEADER_DEFAULT) < 0)
goto out_err;
} else if (asix_write_rx_ctl(dev, AX_DEFAULT_RX_CTL) < 0)
goto out_err;
if (asix_write_hwaddr(eth) < 0) goto out_err; do {
@@ -447,6 +464,10 @@ static int asix_init(struct eth_device *eth, bd_t *bd) goto out_err; }
/* Wait some more to avoid timeout on first transfer
(e.g. EHCI timed out on TD - token=0x8008d80) */
udelay(25000);
Is this always needed or is it only helpful to your device?
return 0;
out_err: return -1; @@ -533,6 +554,10 @@ static int asix_recv(struct eth_device *eth) return -1; }
if ((dev->pusb_dev->descriptor.idVendor == 0x0b95) &&
(dev->pusb_dev->descriptor.idProduct == 0x772b))
Again, Avoid PID hacks and at least magic numbers.
buf_ptr += 2;
/* Notify net stack */ net_process_received_packet(buf_ptr + sizeof(packet_len), packet_len);
Thanks, -Joe

Hi Joe
On 8 July 2015 05:55:28 CEST, Joe Hershberger joe.hershberger@gmail.com wrote:
Definitely use checkpatch.pl on this series.
Yeah, it's been late and I somehow forgot about it. I already run v2 about to be posted through it now.
I recommend patman.
So far I have not used that one yet but thanks for the tip.
I'm guessing this is ported from the ASIX Linux driver. Perhaps mention in the log where it came from.
As mentioned in the log it actually came from Antmicro who did an initial port way back 2011.
Please don't use magic numbers.
I already got rid of this in v2.
Also, ideally don't create PID hacks.
Understood, unfortunately I don't know what the effect of this on other ASIX variants is.
Is this always needed or is it only helpful to your device?
Good question which posting it here I hoped to get some more feedback about.
Cheers
Marcel

From: Marcel Ziswiler marcel.ziswiler@toradex.com
Allow showing custom board info from a checkboard() function being implemented if CONFIG_CUSTOM_BOARDINFO is specified.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- common/board_info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_info.c b/common/board_info.c index 42d0641..a692f74 100644 --- a/common/board_info.c +++ b/common/board_info.c @@ -19,7 +19,7 @@ int __weak checkboard(void) */ int show_board_info(void) { -#ifdef CONFIG_OF_CONTROL +#if defined(CONFIG_OF_CONTROL) && !defined(CONFIG_CUSTOM_BOARDINFO) DECLARE_GLOBAL_DATA_PTR; const char *model;

Hi Marcel,
On 1 July 2015 at 17:04, Marcel Ziswiler marcel@ziswiler.com wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
Allow showing custom board info from a checkboard() function being implemented if CONFIG_CUSTOM_BOARDINFO is specified.
Can you reword this? It seems confusing. From what I can tell you are turning off the device tree model display if CONFIG_CUSTOM_BOARDINFO is enabled.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
common/board_info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/board_info.c b/common/board_info.c index 42d0641..a692f74 100644 --- a/common/board_info.c +++ b/common/board_info.c @@ -19,7 +19,7 @@ int __weak checkboard(void) */ int show_board_info(void) { -#ifdef CONFIG_OF_CONTROL +#if defined(CONFIG_OF_CONTROL) && !defined(CONFIG_CUSTOM_BOARDINFO) DECLARE_GLOBAL_DATA_PTR; const char *model;
-- 1.9.3
Regards, Simon

From: Stefan Agner stefan.agner@toradex.com
Use the boot loader splash screen from WinCE which matches our wallpapers position wise. Although the logo is an 8-bit indexed BMP as well colours looked odd at first in U-Boot. After converting to full RGB palette and converting back to an indexed BMP using imagemagick the Logo showed up properly.
$ convert tools/logos/toradex-rgb.bmp -type Palette -colors 256 \ -compress none -verbose BMP3:tools/logos/toradex.bmp
Signed-off-by: Stefan Agner stefan.agner@toradex.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- tools/logos/toradex.bmp | Bin 0 -> 24982 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tools/logos/toradex.bmp
diff --git a/tools/logos/toradex.bmp b/tools/logos/toradex.bmp new file mode 100644 index 0000000000000000000000000000000000000000..3e2dcf23358dd46fc7b1bb0dae70d3ba985606ee GIT binary patch literal 24982 zcmeHPy>H~k6(3!0Ppji;#g)jt)k#_{wd9K2#Q*^=NL&Xlg21U<8%UaK+}lmcz@3r8 zfPpKV!nJ|?0|wHjaPQK8fh$)A3?uJj-pr63?s8A3a}M4CL=NZUy*Iyk^WK{wSAYD= z=YLa_e*aLZ5_0byDD@|}AE+Uf|Mdf<P9y$<Lad;4aB!dw9z0MF;P2q!Z`5EgP!Em{ z)o?ge$H&KN5I<3e=V$8d>`X=FOvUf4RF-9GwOXm?&!4O0&+n+q7w@T?zki_KefM4U z{`>E%7cX9@4?p}+ee}^sYW=T|)$^Ax)NlX&5B0nM{8Rnmzn`m5KKVp_`st_Yv(G+L zFJHb?UwrX}`tr*!)mLAArM~|9YxT`H->7fD{Z@VV-FNEy@4r_+{P2VN@y8$4Pe1*n ze*XDq7JPr*Bd|wckH8**|6c?$-BY$WuXH)>+4$GltT&d^Dw`HM`*ot!Z`8f993j0- z)2$!rUtO!-SQ0X^-mmg*71<5T=*=whD64cerzMc+B?zC=6_$mH3O3tvSK1o<LQ{fn zB&EG%WnAcLMi@*>T@|n=0S?u4y2i3urHi;E<0ZADLN604a@3Vdz+-S{=(3z(U(17X znYN6Qr7kB^4V;ov=iHE?rY_KKnQ&(-KGM5~6zSz0hL2#*Gz!F5#Bx;SV790BUc(Z0 zTv-Oqn?tM$W9kcC%w|iVo9ld5RfuJ#p|YwFOR0^@Y6e(ik%uM7P-H~~<O`ioml`Lh zi_r+?4k#c_Knq%8Lx*KlVfCBAQpF%dg7c}COP$KqNCS@*)L<ptlZ5j)M9DRW$Uw!Q z-k5_FV`hXv#kzp!44-Z7RD;q@sjZ_v)g>(yr6y$P3UnRR#$JdDh!I0)kSeB$piwli zoMCm70SR_rmkN-I^b~gactsR=W&uNkn%r4}ZY+6frZ7y<DoivOB15BQY;7$~)rcVC z=5Pg7GEf>S2_kA?G+<#9tYAS&ASQ!Xr<WlHmeajtaEYWk$(JBrlH-XCOA}>0*K!x; z-B@Ca2s%}0dokq%Dw5+IL1Ak-Q^#^bYnnQ9HPUNgiS<~)g3>jFWrb^jJ7eblQ;MrE z)6qmnQw<9XREmU~Rys%ZlDn0p6HB%TK#rXgjE<WkInxP8Y-?%RH-?(yvgE|o3U36I zy=0cr7A!*yg!+BllM1CTm$3el0<U1hP{gTT;*zy@$Ga0t=w<Y>oIrxH=TwAO&Jie; z)Q0qODds^L?T!Li5>?ehFKK<y960gsFE`T__#84?)eHj~bgPJEg^L}M-1!Nw9222k z#pP!%)Qj!qOy#IJBd}*P@DFTu*<Mn7VrvU*M(zpjCY6j6KCxX*aM5=$>d;GEPKY4c z%LW6(=k?zrh`(^O6LjJbQQ{RaBrJ{1v)0XWhaD~?jQcR00xyjh7%-bxT%;)~_XILD z4)U`x!I9$#ZS55Ku~W9hJ-tlHbmll6VMZ|t%HkSRf%G%<R<zp*F|eFocV@;W5!i!g zoKlE6g)Cw@gLp6pq2%sZduzNZXG>k+1?)ry5jMn^Xzu~;8BGGmm#Aww#h0|+q!!~# zh|6L4E_wD6s9=nX)w<58B1W(%tX?KylBO^-gk@Fbvl2(-1+b38edRD5!c^it(YUM= z&?yoTHYX)2pWGqK$-{CAf%pV=G}I6#1ou6-z#`n_PzLusJYp>30La$jzNfJSJwFaq zKm^)M>pG)S7+EYcbZEv&!Tt+&L*&AU0c{3E`1`wSgXK;f9G3NNJg|BhJ$V&%C8XMV z5yY1`2fL>_nce%}xkrI(bJujQSb{aBV0~vme{n2hSa*5*<k?fC0MP7vz}YlXXD_cJ zFydwrm~GgPqq*BDWh79O*iIzw*ceaomteE&$s2sVf}S`gG9i;!-`UHn2#mE=6fev< zkw?34wneOO$9roR?Wz<c$)x6xJ>k3x3Z=X%$Oek)4R(1N?Q9!%p}udsM_`Y@9)Udq zdj$3f>=D=_ut#7g1dfhR;S1NZ)A5s?x@~D3CDUxNSX@uy!IqYx>XUQ^oo}Y;rl8-k z7=~y_Sua}B_%8Q{<7bywk}ggk??@{yFtzrODuu13q}y2?kiiZ1p`)-cw+LQpOIj5Z z04l;B4L0~ifuwSd2hyR`rbVrW3!=C<Z1=h5ns&vulJU9VXzr&++q%^1QyNVZ7x);W zmR56rXErFYa#5q>Jq+MoTP@vA;FTe!r(UZj3!T>|jGMM;$+WZJ`o<&4SvVQNwDa-T z<7W*d;ql_QMSep;2+PAci{oD_Pqx7=*0e{#Y~UESmGXFnWe&^1b*qhrGv#Iq(lXQf zlqs~-fQucgCe4oLCl_rfp=8|AJX9RO5?%qd(+Xdqn6weglu&CDsMc6w2RIC|(E3_9 zn0wk#aH6`tJ>lvbX5vjUl#jSe*F-lMxW~s~D6g(|(9AZLL(!`;pQh<#RuBvzw*_+( zmZ`K}&L?U5^rnV}U}>R*PnL*<;6HbRgItRiH(OT><{=Aezw*Gorfj*<m(`f?!=t9D zv`Tu}-=Vj$%z3(RCt@N`<^~@zRU5E8F{tF$m83E?KvuA9u^7!vKRaJAM79fBTUv*y zQRiD0ZB$XO!t+1$)`!pK1Bi7OomuQ!nTto=-Rd<hERE^T{F#NX@i2)7bYm&1BB&ED zL(ySrEM1MoiGkeCfjT6sOfT=9$c9$VVOq?S>`qM$<r;g}xjT2<LeMKX?e15vX<|8I z1!=s081cA_>+Z&qhgvy6h#B*+ScX{~va&91Fxx?ENy%`S;1p|o7f&f+Y<=Z6HCt{0 z7w*8k>>Fn`v1EhZig%1j=OJ`r8Ig5d*S!G@Lo}1uI7bOB7DJvd(J7|2Ewtfqz;KLs z`QWD{yQL=%xKOUIVm7d3ohijsI+3^?1JH>jW7hP>K*ZQN!ZISw53pdZk`LrMX*ro7 zHz?Mqi}b<ffVmiBT5GoD;WUJxD?7K{2fpP~^(Xus1(RJBY;a`FSQ?Nad&Y0BJF)cI z_+9B^Q)EL}^12i^<hRRdiJIlxNz3m68!8SI3NQ_<qpOC2sW%Tiji349(8cNTv2#TD zbb4`R7I}PAm2U4g%5JNoHjD<AG_}EZ{R{yu-!3digvC4{FV6&Hxj<BdELbslUQdWt zK*gKXjkmuxT7e%Z?UWpFsEB9ev?Xpor#<B33l7-Rl|Mds8IH*}D0M7{-Md2uihNQR zmQ1T<EpcePVsRD+B!e%49r__wt+|S6rBP?Hy||^m=CSoa-}c1J*ezwqN&LfpU7XGn zGMFqlNXYtR;tuQgfL0w#2F$K_pDP1M1MR|+09%E&)$olHCd860a*zd?)ioPT2EXb~ zZpzI5hd3;~SksrjGhdI<t(T$VGwDTp9}_dsM@#t8l}TdXp=}*Y__z;Jc$v`hFOYl} zmgJyI8`Ov-ct0V+h~*|0M4%!#0!IQw!d{$BH$g4dNCfP&-kf)5Y^b1i_f0F`B1;RO zY!foMMT#bpty!xOmTt;@IcWMfVo4FAH&0$~R(QmcCp63gevBf*nXM5PgcH&-$pmcB zF02L8E_CmOCHWq!{)DB~($cy1{#erJHeyLL_nNS@S`bFk?MD%yB(MVw0T(E1xE|Bu z5xI7Wg&vW*+Eq>GCcL~rI6{QGg|H0Iu3O_AE_y_a1f*7F&|<lj&8yw5P=9>1n`ReJ zw7PJ_a$iWnvawt9Nq6PzVY(b-n1Iuq5si7VbNH%Pe6k{w?X9pnmK?1GMLHxG&s$%j z(*})f$l9EU)rDs^82W8s!E?k|vOUV@fy86-!pOU<adqrRmwfp9NMiR(RLF<pOU9JX zBKZ?prG^ZGKc#gnISvdiQ`RiDQtV{C(J*k_B&c#^Y*w_LqF^G(f_FrWC9f!7Qad^9 zq1@KY^F2mqY|}oOqcKF|E}fs9d5`pTDX%J$*>&p7V=}$AXKBQ!fhEZqR4v|Bu+lMT ztx#@AAzpj!!z6k&OsF*xAPwsxW6A2fjTQ~Tclocdfdm@$Ak*P$ayB+1cHNf-maGn~ za|c<P<`jp6A|A9)nr(<@L8Q`!>Jk(MSjZt8Yg{KSM%=k^<5A(GdQ8DhfQZp`Kk_uO zg=GJu^#+y$4(Ho&@X%xgjEy@tG(z%BxMMSsoedf<akH}z8~_=2W2UGdEtneG*ujri zcqHyL74Zsi-8OPd37c3d@G_mPR7^p{(uB%3u$7%!xwFG_qTm+isjW<YKW2%(4|$ zGk6kT%GpwrmN^U(TJWP2tBz(af;P4xQS))5V)lOH)5Ov!=52i^$WVHFPcB$D!DK_) zcn217+-)bqoWL#X4j+C=$Vp7cCR)-)5NF!V$Gi<{!nOH%!@+uAYuVgG0z>o0#(%*I zKhw37FCv*>Ywp|{)fn9<Je(3f<Mjf<0E?lECoN8WXmOi0dorV=JHI$X$vap1`9m4< z)_YSg@t1d;mJ`;^m``BVy5=K|-0;gN9>?e@!8b5_ztv4LK;BVm3h38(A`LUu@b4cC z3*$J5vBlEpuLfEa{|9IpLSn{^?F{%e;oN=i0F#g1y1V%`Z7kt8dnQ%IVv)<)lDtWk z&a&44MDoR=SeeaKWV8^D0I*mVt~PJGOmE9#<+EV64omZv35q*tnIPTyZuW`Z5)f&7 z-0QnZ<Au)qzUx5-0W9t78Nrn9@`m3^6|>cMjrbfb_@#o*NK0+sFpH85hr<%SLUPny zE|XoV@k7oK<8Ll8?R>UQf{oje5g!dZu{Y;JSgM3SvvA_W*F$cfAp}RVe5D<G(7}wO z6e)$fZ+uN}v7%n9Vnk}?VW|S2%6PQQi!(p$=S@M~9ErSJ??<%_(ACwm{+}5T?I4zF z2tNb$lY7fi6UhIL0Gf+Ijq4z<vU*U!B!GoE-C;)X0T){rA4>=yyJ@w83nXkzX3_A8 zHL<%Nj>fWqygWbM5y|ixM_$oHzUW)u&fB58h?8WuMhmOl+nmPNO9K@AHaUbOvk-H- zE*B{p0Wza?ECHRF&|1}KMT;Vb)I5kuu`n;XMHbOJ?nlSt)6?<s<Nog35(?`jRYx&= z5R@ieP#6+}Q39>fxM>5Rcrs}hTgJj-f-H;$3}Oild(o;>{w1C^VEN|7>B92OOY%w_ zdFLhp+AEQGeVyJeER(d6*cUDur4n_#KA3t@db_ZcO^xrQ1NkjBvKKI~(CF>L(rs!2 z2-+e!9eV{h?$qTi!_w>wENsEd9?O0z;Wo@c7^!gG3~>69zH_U$O)phkc7Ir6YWyAU RodMc`0Q`TRs=7z!{{x@?rvLx|
literal 0 HcmV?d00001

From: Marcel Ziswiler marcel.ziswiler@toradex.com
Remove spurious new line in configuration file.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- include/configs/colibri_vf.h | 1 - 1 file changed, 1 deletion(-)
diff --git a/include/configs/colibri_vf.h b/include/configs/colibri_vf.h index aff2810..2523f32 100644 --- a/include/configs/colibri_vf.h +++ b/include/configs/colibri_vf.h @@ -69,7 +69,6 @@ "512k(u-boot-env)," \ "-(ubi)"
- #define CONFIG_MMC #define CONFIG_FSL_ESDHC #define CONFIG_SYS_FSL_ESDHC_ADDR 0

On Thursday, July 02, 2015 at 01:04:50 AM, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
Remove spurious new line in configuration file.
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
This doesn't break anything, so:
Tested-by: Marek Vasut marex@denx.de Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut

From: Max Krummenacher max.krummenacher@toradex.com
This fixes the following warning (and the runtime error reporting): ../common/image-fdt.c:491:4: warning: 'fdt_ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- common/image-fdt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/image-fdt.c b/common/image-fdt.c index 80e3e63..d9fce17 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -492,7 +492,8 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, } } if (IMAGE_OF_SYSTEM_SETUP) { - if (ft_system_setup(blob, gd->bd)) { + fdt_ret = ft_system_setup(blob, gd->bd); + if (fdt_ret) { printf("ERROR: system-specific fdt fixup failed: %s\n", fdt_strerror(fdt_ret)); goto err;

On Thursday, July 02, 2015 at 01:04:51 AM, Marcel Ziswiler wrote:
From: Max Krummenacher max.krummenacher@toradex.com
This fixes the following warning (and the runtime error reporting): ../common/image-fdt.c:491:4: warning: 'fdt_ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
Acked-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut

On 1 July 2015 at 23:52, Marek Vasut marex@denx.de wrote:
On Thursday, July 02, 2015 at 01:04:51 AM, Marcel Ziswiler wrote:
From: Max Krummenacher max.krummenacher@toradex.com
This fixes the following warning (and the runtime error reporting): ../common/image-fdt.c:491:4: warning: 'fdt_ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
Acked-by: Marek Vasut marex@denx.de
Acked-by: Simon Glass sjg@chromium.org

From: Marcel Ziswiler marcel.ziswiler@toradex.com
Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer alignment into account which led to failures of the following form:
ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108 ERROR: v7_dcache_inval_range - stop address is not aligned - 0x1f7f1108
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- common/cmd_ubi.c | 2 +- drivers/mtd/nand/nand_util.c | 2 +- fs/ubifs/super.c | 4 ++-- fs/ubifs/ubifs.c | 4 ++-- lib/gzip.c | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c index cbc10c5..30a1259 100644 --- a/common/cmd_ubi.c +++ b/common/cmd_ubi.c @@ -363,7 +363,7 @@ int ubi_volume_read(char *volume, char *buf, size_t size) tbuf_size = vol->usable_leb_size; if (size < tbuf_size) tbuf_size = ALIGN(size, ubi->min_io_size); - tbuf = malloc(tbuf_size); + tbuf = memalign(ARCH_DMA_MINALIGN, tbuf_size); if (!tbuf) { printf("NO MEM\n"); return ENOMEM; diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index ee2c24d..395ba2d 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -839,7 +839,7 @@ int nand_torture(nand_info_t *nand, loff_t offset)
patt_count = ARRAY_SIZE(patterns);
- buf = malloc(nand->erasesize); + buf = memalign(ARCH_DMA_MINALIGN, nand->erasesize); if (buf == NULL) { puts("Out of memory for erase block buffer\n"); return -ENOMEM; diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 10f8fff..ac0d13f 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -57,7 +57,7 @@ struct inode *iget_locked(struct super_block *sb, unsigned long ino) { struct inode *inode;
- inode = (struct inode *)malloc(sizeof(struct ubifs_inode)); + inode = (struct inode *)memalign(ARCH_DMA_MINALIGN, sizeof(struct ubifs_inode)); if (inode) { inode->i_ino = ino; inode->i_sb = sb; @@ -104,7 +104,7 @@ void iput(struct inode *inode) /* * Allocate and use new inode */ - ino = (struct inode *)malloc(sizeof(struct ubifs_inode)); + ino = (struct inode *)memalign(ARCH_DMA_MINALIGN, sizeof(struct ubifs_inode)); memcpy(ino, inode, sizeof(struct ubifs_inode));
/* diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 6dd6174..03f9f79 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -108,7 +108,7 @@ static inline struct crypto_comp *crypto_alloc_comp(const char *alg_name, struct crypto_comp *ptr; int i = 0;
- ptr = malloc(sizeof(struct crypto_comp)); + ptr = memalign(ARCH_DMA_MINALIGN, sizeof(struct crypto_comp)); while (i < UBIFS_COMPR_TYPES_CNT) { comp = ubifs_compressors[i]; if (!comp) { @@ -723,7 +723,7 @@ static int do_readpage(struct ubifs_info *c, struct inode *inode, * destination area to a multiple of * UBIFS_BLOCK_SIZE. */ - buff = malloc(UBIFS_BLOCK_SIZE); + buff = memalign(ARCH_DMA_MINALIGN, UBIFS_BLOCK_SIZE); if (!buff) { printf("%s: Error, malloc fails!\n", __func__); diff --git a/lib/gzip.c b/lib/gzip.c index ff37d4f..2a74411 100644 --- a/lib/gzip.c +++ b/lib/gzip.c @@ -25,7 +25,7 @@ static void *zalloc(void *x, unsigned items, unsigned size) size *= items; size = (size + ZALLOC_ALIGNMENT - 1) & ~(ZALLOC_ALIGNMENT - 1);
- p = malloc (size); + p = memalign(ARCH_DMA_MINALIGN, size);
return (p); }

On Thursday, July 02, 2015 at 01:04:52 AM, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer alignment into account which led to failures of the following form:
ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108 ERROR: v7_dcache_inval_range - stop address is not aligned - 0x1f7f1108
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
What about using ALLOC_CACHE_ALIGN_BUFFER() and friends instead ? See include/common.h for their definition, this is what those functions are exactly for.
Best regards, Marek Vasut

On Thu, 2015-07-02 at 07:53 +0200, Marek Vasut wrote:
On Thursday, July 02, 2015 at 01:04:52 AM, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer alignment into account which led to failures of the following form:
ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108 ERROR: v7_dcache_inval_range - stop address is not aligned - 0x1f7f1108
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
What about using ALLOC_CACHE_ALIGN_BUFFER() and friends instead ? See include/common.h for their definition, this is what those functions are exactly for.
ALLOC_CACHE_ALIGN_BUFFER() is for statically allocating an aligned buffer. Dynamically allocating an aligned buffer is exactly what memalign() is for.
-Scott

On Thursday, July 02, 2015 at 11:35:19 PM, Scott Wood wrote:
On Thu, 2015-07-02 at 07:53 +0200, Marek Vasut wrote:
On Thursday, July 02, 2015 at 01:04:52 AM, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer alignment into account which led to failures of the following form:
ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108 ERROR: v7_dcache_inval_range - stop address is not aligned
- 0x1f7f1108
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
What about using ALLOC_CACHE_ALIGN_BUFFER() and friends instead ? See include/common.h for their definition, this is what those functions are exactly for.
ALLOC_CACHE_ALIGN_BUFFER() is for statically allocating an aligned buffer.
You're confusing this with DEFINE_ALIGN_BUFFER, no ?
Dynamically allocating an aligned buffer is exactly what memalign() is for.
Isn't memalign()ed memory aligned only to the start address, while the end address (and thus the length) is not aligned ? This is what memalign(3) has to say:
" The function posix_memalign() allocates size bytes and places the address of the allocated memory in *memptr. The address of the allo‐ cated memory will be a multiple of alignment, which must be a power of two and a multiple of sizeof(void *). If size is 0, then the value placed in *memptr is either NULL, or a unique pointer value that can later be successfully passed to free(3).
The obsolete function memalign() allocates size bytes and returns a pointer to the allocated memory. The memory address will be a mul‐ tiple of alignment, which must be a power of two. "
Best regards, Marek Vasut

On Fri, 2015-07-03 at 15:44 +0200, Marek Vasut wrote:
On Thursday, July 02, 2015 at 11:35:19 PM, Scott Wood wrote:
On Thu, 2015-07-02 at 07:53 +0200, Marek Vasut wrote:
On Thursday, July 02, 2015 at 01:04:52 AM, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer alignment into account which led to failures of the following form:
ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108 ERROR: v7_dcache_inval_range - stop address is not aligned
- 0x1f7f1108
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
What about using ALLOC_CACHE_ALIGN_BUFFER() and friends instead ? See include/common.h for their definition, this is what those functions are exactly for.
ALLOC_CACHE_ALIGN_BUFFER() is for statically allocating an aligned buffer.
You're confusing this with DEFINE_ALIGN_BUFFER, no ?
OK, not "statically", but on the stack. It is not appropriate to turn dynamic allocations into stack allocations without considering how large the allocation can be. It'd also be more intrusive a change than necessary, even if the sizes were small enough.
Dynamically allocating an aligned buffer is exactly what memalign() is for.
Isn't memalign()ed memory aligned only to the start address, while the end address (and thus the length) is not aligned ?
The end address is aligned if the size passed to memalign is aligned. Maybe add a wrapper that calls memalign() with the size rounded up to ARCH_DMA_MINALIGN?
This is what memalign(3) has to say:
" The function posix_memalign() allocates size bytes and places the address of the allocated memory in *memptr. The address of the allo‐ cated memory will be a multiple of alignment, which must be a power of two and a multiple of sizeof(void *). If size is 0, then the value placed in *memptr is either NULL, or a unique pointer value that can later be successfully passed to free(3).
The obsolete function memalign() allocates size bytes and returns a pointer to the allocated memory. The memory address will be a mul‐ tiple of alignment, which must be a power of two. "
posix_memalign() does not exist in U-Boot, and it's not clear to me why memalign() should be considered obsolete. Is the difference just the ability to return -EINVAL?
-Scott

On Monday, July 06, 2015 at 11:25:35 PM, Scott Wood wrote:
On Fri, 2015-07-03 at 15:44 +0200, Marek Vasut wrote:
On Thursday, July 02, 2015 at 11:35:19 PM, Scott Wood wrote:
On Thu, 2015-07-02 at 07:53 +0200, Marek Vasut wrote:
On Thursday, July 02, 2015 at 01:04:52 AM, Marcel Ziswiler wrote:
From: Marcel Ziswiler marcel.ziswiler@toradex.com
Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer alignment into account which led to failures of the following form:
ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108 ERROR: v7_dcache_inval_range - stop address is not aligned - 0x1f7f1108
Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
What about using ALLOC_CACHE_ALIGN_BUFFER() and friends instead ? See include/common.h for their definition, this is what those functions are exactly for.
ALLOC_CACHE_ALIGN_BUFFER() is for statically allocating an aligned buffer.
You're confusing this with DEFINE_ALIGN_BUFFER, no ?
OK, not "statically", but on the stack. It is not appropriate to turn dynamic allocations into stack allocations without considering how large the allocation can be. It'd also be more intrusive a change than necessary, even if the sizes were small enough.
Fine.
Dynamically allocating an aligned buffer is exactly what memalign() is for.
Isn't memalign()ed memory aligned only to the start address, while the end address (and thus the length) is not aligned ?
The end address is aligned if the size passed to memalign is aligned. Maybe add a wrapper that calls memalign() with the size rounded up to ARCH_DMA_MINALIGN?
I agree.
This is what memalign(3) has
to say:
" The function posix_memalign() allocates size bytes and places the address of the allocated memory in *memptr. The address of the allo‐ cated memory will be a multiple of alignment, which must be a power of two and a multiple of sizeof(void *). If size is 0, then the value placed in *memptr is either NULL, or a unique pointer value that can later be successfully passed to free(3).
The obsolete function memalign() allocates size bytes and returns a pointer to the allocated memory. The memory address will be a mul‐ tiple of alignment, which must be a power of two. "
posix_memalign() does not exist in U-Boot, and it's not clear to me why memalign() should be considered obsolete. Is the difference just the ability to return -EINVAL?
The args are also totally different.
Best regards, Marek Vasut

On Mon, 2015-07-06 at 23:45 +0200, Marek Vasut wrote:
On Monday, July 06, 2015 at 11:25:35 PM, Scott Wood wrote:
On Fri, 2015-07-03 at 15:44 +0200, Marek Vasut wrote:
This is what memalign(3) has
to say:
" The function posix_memalign() allocates size bytes and places the address of the allocated memory in *memptr. The address of the allo‐ cated memory will be a multiple of alignment, which must be a power of two and a multiple of sizeof(void *). If size is 0, then the value placed in *memptr is either NULL, or a unique pointer value that can later be successfully passed to free(3).
The obsolete function memalign() allocates size bytes and returns a pointer to the allocated memory. The memory address will be a mul‐ tiple of alignment, which must be a power of two. "
posix_memalign() does not exist in U-Boot, and it's not clear to me why memalign() should be considered obsolete. Is the difference just the ability to return -EINVAL?
The args are also totally different.
...in order to accommodate returning an error value.
-Scott

On Monday, July 06, 2015 at 11:50:39 PM, Scott Wood wrote:
On Mon, 2015-07-06 at 23:45 +0200, Marek Vasut wrote:
On Monday, July 06, 2015 at 11:25:35 PM, Scott Wood wrote:
On Fri, 2015-07-03 at 15:44 +0200, Marek Vasut wrote:
This is what memalign(3) has
to say:
" The function posix_memalign() allocates size bytes and places the address of the allocated memory in *memptr. The address of the allo‐ cated memory will be a multiple of alignment, which must be a power of two and a multiple of sizeof(void *). If size is 0, then the value placed in *memptr is either NULL, or a unique pointer value that can later be successfully passed to free(3).
The obsolete function memalign() allocates size bytes and returns a pointer to the allocated memory. The memory address will be a mul‐ tiple of alignment, which must be a power of two. "
posix_memalign() does not exist in U-Boot, and it's not clear to me why memalign() should be considered obsolete. Is the difference just the ability to return -EINVAL?
The args are also totally different.
...in order to accommodate returning an error value.
... of course.
Best regards, Marek Vasut

From: Max Krummenacher max.krummenacher@toradex.com
CONFIG_TFTP_TSIZE should limit a tftp downloads progress to 50 '#' chars. Make this work also for small files.
If the file size is small, i.e. smaller than 2 tftp block sizes the number of '#' can get much larger. i.e. with a 1 byte file 65000 characters are printed, with a 512 byte file around 500.
When using CONFIG TFTP BLOCKSIZE together with CONFIG_IP_DEFRAG the issue is more notable.
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com --- net/tftp.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/net/tftp.c b/net/tftp.c index 3e99e73..1853fe7 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -249,6 +249,8 @@ static void show_block_marker(void) if (tftp_tsize) { ulong pos = tftp_cur_block * tftp_block_size + tftp_block_wrap_offset; + if(pos > tftp_tsize) + pos = tftp_tsize;
while (tftp_tsize_num_hash < pos * 50 / tftp_tsize) { putc('#');

On Thursday, July 02, 2015 at 01:04:53 AM, Marcel Ziswiler wrote:
From: Max Krummenacher max.krummenacher@toradex.com
CONFIG_TFTP_TSIZE should limit a tftp downloads progress to 50 '#' chars. Make this work also for small files.
If the file size is small, i.e. smaller than 2 tftp block sizes the number of '#' can get much larger. i.e. with a 1 byte file 65000 characters are printed, with a 512 byte file around 500.
When using CONFIG TFTP BLOCKSIZE together with CONFIG_IP_DEFRAG the issue is more notable.
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com Signed-off-by: Marcel Ziswiler marcel.ziswiler@toradex.com
net/tftp.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/net/tftp.c b/net/tftp.c index 3e99e73..1853fe7 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -249,6 +249,8 @@ static void show_block_marker(void) if (tftp_tsize) { ulong pos = tftp_cur_block * tftp_block_size + tftp_block_wrap_offset;
if(pos > tftp_tsize)
Nitpick:
if[SPACE](...
Reviewed-by: Marek Vasut marex@denx.de
Best regards, Marek Vasut
participants (5)
-
Joe Hershberger
-
Marcel Ziswiler
-
Marek Vasut
-
Scott Wood
-
Simon Glass