Skip to contentSkip to conversation

assortment of fixes/enhancements

PATCH New Marcel Ziswiler 5 participants 28 messages Jul 1, 2015 – Jul 8, 2015

Subsystems (7)
Networking VFS USB Library code +3 more subsystems
lore
Share & tools
lore
#0 lore raw

cover assortment of fixes/enhancements

From: Marcel Ziswiler <[email protected]>

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
Signature · 1 line
1.9.3
#1 lore raw

PATCH 1/8 fs/fs.c: read up to EOF when len would read past EOF

From: Max Krummenacher <[email protected]>

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.

---
 fs/fs.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
fs/fs.c +2 -4
index ac0897d..827b143 100644
@@ -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;
Signature · 1 line
1.9.3
#2 lore raw

PATCH 1/8 fs/fs.c: read up to EOF when len would read past EOF

Quoted 16 lines · Marcel Ziswiler

From: Max Krummenacher <[email protected]>

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

Quoted 5 lines

The patch changes the behaviour to printing an informational message and returning the actual read number of bytes.

So this behaves like read(2) now, right ?

Best regards,
Marek Vasut

#3 lore raw

PATCH 1/8 fs/fs.c: read up to EOF when len would read past EOF

Quoted 1 line · Marek Vasut

So this behaves like read(2) now, right ?

Exactly for convenient use in U-Boot scripts.

Signature · 1 line
Sent from my Android device with K-9 Mail. Please excuse my brevity.
#4 lore raw

PATCH 2/8 net: asix: fix operation without eeprom

In reply to Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

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.

---
 drivers/usb/eth/asix.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)
drivers/usb/eth/asix.c +26 -1
index c8697ae..3926d50 100644
@@ -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);
Signature · 1 line
1.9.3
#5 lore raw

PATCH 2/8 net: asix: fix operation without eeprom

On Thursday, July 02, 2015 at 01:04:47 AM, Marcel Ziswiler wrote:

Hi!

[...]

drivers/usb/eth/asix.c line 70

Quoted 5 lines
drivers/usb/eth/asix.c +1 -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

drivers/usb/eth/asix.c line 72

Quoted 2 lines
drivers/usb/eth/asix.c +2 -0
+						   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
*/

Review line 73

Quoted 1 line
(quoted diff) +1 -0
+#define AX_RX_CTL_RH1M			0x0100	/* Enable RX-Header mode 

0 */

Quoted 8 lines
drivers/usb/eth/asix.c +2 -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)

drivers/usb/eth/asix.c static int asix_init(struct eth_device *eth, bd_t *bd) · line 439

Quoted 6 lines
drivers/usb/eth/asix.c +2 -1
@@ -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.

Quoted 10 lines
drivers/usb/eth/asix.c +7 -0
+		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 {

[...]

#6 lore raw

PATCH 2/8 net: asix: fix operation without eeprom

Quoted 12 lines · Marek Vasut

On Thursday, July 02, 2015 at 01:04:47 AM, Marcel Ziswiler wrote:

Hi!

[...]

drivers/usb/eth/asix.c line 70

Quoted 5 lines
drivers/usb/eth/asix.c +1 -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

Quoted 6 lines

drivers/usb/eth/asix.c line 72

Quoted 2 lines
drivers/usb/eth/asix.c +2 -0
+						   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!

Quoted 9 lines

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.

Quoted 14 lines

Review line 73

Quoted 1 line
(quoted diff) +1 -0
+#define AX_RX_CTL_RH1M			0x0100	/* Enable RX-Header mode 

0 */

drivers/usb/eth/asix.c static int asix_init(struct eth_device *eth,

Quoted 8 lines
drivers/usb/eth/asix.c +2 -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)

Quoted 3 lines
drivers/usb/eth/asix.c +0 -0
 	debug("** %s()\n", __func__);
 
Quoted 8 lines

drivers/usb/eth/asix.c static int asix_init(struct eth_device *eth, bd_t *bd) · line 439

Quoted 3 lines
drivers/usb/eth/asix.c +2 -1
-	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.

Quoted 3 lines

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.

Quoted 2 lines

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.

Quoted 12 lines
Quoted 10 lines
drivers/usb/eth/asix.c +7 -0
+		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 {

[...]

#7 lore raw

PATCH 2/8 net: asix: fix operation without eeprom

Quoted 4 lines · Marcel Ziswiler
Quoted 3 lines · Marek Vasut

On Thursday, July 02, 2015 at 01:04:47 AM, Marcel Ziswiler wrote:

Hi!

Hi!

Quoted 19 lines
Quoted 12 lines

[...]

drivers/usb/eth/asix.c line 72

Quoted 7 lines
drivers/usb/eth/asix.c +1 -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

Quoted 6 lines

drivers/usb/eth/asix.c line 72

Quoted 2 lines
drivers/usb/eth/asix.c +2 -0
+						   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.

Quoted 2 lines

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 :)

Quoted 6 lines
Quoted 6 lines

The multiline comments should be like this according to kernel
coding style (to my knowledge):

/*

Quoted 7 lines
Quoted 4 lines

* 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.

Quoted 29 lines
Quoted 20 lines

Review line 75

Quoted 1 line
(quoted diff) +1 -0
+#define AX_RX_CTL_RH1M			0x0100	/* Enable RX-Header mode

0 */

drivers/usb/eth/asix.c static int asix_init(struct eth_device *eth,

Quoted 11 lines
drivers/usb/eth/asix.c +2 -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 \

drivers/usb/eth/asix.c +0 -0
 	(AX_RX_CTL_SO | AX_RX_CTL_AB)
@@ -426,7 +435,15 @@ static int asix_init(struct eth_device *eth,

bd_t *bd)

Quoted 2 lines
drivers/usb/eth/asix.c +0 -0
 	debug("** %s()\n", __func__);
 
Quoted 8 lines

drivers/usb/eth/asix.c static int asix_init(struct eth_device *eth, bd_t *bd) · line 439

Quoted 3 lines
drivers/usb/eth/asix.c +2 -1
-	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?

Quoted 3 lines

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.

Quoted 10 lines
Quoted 3 lines

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.

Quoted 2 lines

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! :-)

[...]

#8 lore raw

PATCH 2/8 net: asix: fix operation without eeprom

In reply to Marcel Ziswiler

Hi Marcel,

Quoted 26 lines · Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

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.

---
 drivers/usb/eth/asix.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)
drivers/usb/eth/asix.c +3 -0
index c8697ae..3926d50 100644
@@ -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+
  */

drivers/usb/eth/asix.c line 72

Quoted 6 lines
drivers/usb/eth/asix.c +3 -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.

drivers/usb/eth/asix.c line 77

Quoted 5 lines
drivers/usb/eth/asix.c +3 -0
+#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.

drivers/usb/eth/asix.c static int asix_init(struct eth_device *eth, bd_t *bd) · line 439

Quoted 10 lines
drivers/usb/eth/asix.c +2 -1
 #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.

Quoted 11 lines
drivers/usb/eth/asix.c +7 -0
+               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 {

drivers/usb/eth/asix.c static int asix_init(struct eth_device *eth, bd_t *bd) · line 469

Quoted 6 lines
drivers/usb/eth/asix.c +3 -0
@@ -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?

drivers/usb/eth/asix.c static int asix_recv(struct eth_device *eth) · line 558

Quoted 10 lines
drivers/usb/eth/asix.c +3 -0
+
        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.

drivers/usb/eth/asix.c static int asix_recv(struct eth_device *eth) · line 563

Quoted 5 lines
drivers/usb/eth/asix.c +2 -0
+                       buf_ptr += 2;
+
                /* Notify net stack */
                net_process_received_packet(buf_ptr + sizeof(packet_len),
                                            packet_len);

Thanks,
-Joe

#9 lore raw

PATCH 2/8 net: asix: fix operation without eeprom

Hi Joe

Quoted 1 line · Joe Hershberger

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.

Quoted 1 line

I recommend patman.

So far I have not used that one yet but thanks for the tip.

Quoted 2 lines

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.

Quoted 1 line

Please don't use magic numbers.

I already got rid of this in v2.

Quoted 1 line

Also, ideally don't create PID hacks.

Understood, unfortunately I don't know what the effect of this on other ASIX variants is.

Quoted 1 line

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

#10 lore raw

PATCH 3/8 generic-board: allow showing custom board info

In reply to Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

Allow showing custom board info from a checkboard() function being implemented if CONFIG_CUSTOM_BOARDINFO is specified.

---
 common/board_info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
common/board_info.c +1 -1
index 42d0641..a692f74 100644
@@ -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;
 
Signature · 1 line
1.9.3
#11 lore raw

PATCH 3/8 generic-board: allow showing custom board info

Hi Marcel,

Quoted 5 lines · Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

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.

Quoted 19 lines
---
 common/board_info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
common/board_info.c +1 -2
index 42d0641..a692f74 100644
@@ -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;
 
--
Quoted 2 lines

1.9.3

Regards,
Simon

#12 lore raw

PATCH 4/8 logos: add Toradex logo

In reply to Marcel Ziswiler

From: Stefan Agner <[email protected]>

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
---
 tools/logos/toradex.bmp | Bin 0 -> 24982 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tools/logos/toradex.bmp
tools/logos/toradex.bmp new file binary +0 -0
new file mode 100644
index 0000000000000000000000000000000000000000..3e2dcf23358dd46fc7b1bb0dae70d3ba985606ee
GIT binary patch

z)o?ge$H&KN5I<3e=V$8d>X=FOvUf4RF-9GwOXm?&!4O0&+n+q7w at T?zki_KefM4U z{>E%7cX9 at 4?p}+ee}^sYW=T|)$^Ax)NlX&5B0nM{8Rnmznm5KKVp_st_Yv(G+L zFJHb?UwrX}tr*!)mLAArM~|9YxTH->7fD{Z at VV-FNEy@4r_+{P2VN at y8$4Pe1*n ze*XDq7JPr*Bd|wckH8**|6c?$-BY$WuXH)>+4$GltT&d^DwHM*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 at 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~~<OiomlLh 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 at 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<y960gsFET__#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{3E1wSgXK;f9G3NNJg|BhJ$V&%C8XMV z5yY12fL>_nce%}xkrI(bJujQSb{aBV0~vme{n2hSa*5*<k?fC0MP7vz}YlXXD_cJ zFydwrm~GgPqq*BDWh79O*iIzw*ceaomteE&$s2sVf}SgG9i;!-UHn2#mE=6fev< zkw?34wneOO$9roR?Wz<c$)x6xJ>k3x3Z=X%$Oek)4R(1N?Q9!%p}udsM_Y at 9)Udq zdj$3f>=D=_ut#7g1dfhR;S1NZ)A5s?x@~D3CDUxNSX at uy!IqYx>XUQ^oo}Y;rl8-k z7=~y_Sua}B_%8Q{<7bywk}ggk??@{yFtzrODuu13q}y2?kiiZ1p)-cw+LQpOIj5Z z04l;B4L0~ifuwSd2hyRrbVrW3!=C<Z1=h5ns&vulJU9VXzr&++q%^1QyNVZ7x);W zmR56rXErFYa#5q>Jq+MoTP at vA;FTe!r(UZj3!T>|jGMM;$+WZJo<&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#aH6tJ>lvbX5vjUl#jSe*F-lMxW~s~D6g(|(9AZLL(!;pQh<#RuBvzw*_+( zmZK}&L?U5^rnV}U}>R*PnL*<;6HbRgItRiH(OT><{=Aezw*Gorfj*<m(f?!=t9D zvTu}-=Vj$%z3(RCt at 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>>Fnv1EhZig%1j=OJr8Ig5d*S!G at Lo}1uI7bOB7DJvd(J7|2Ewtfqz;KLs zQWD{yQL=%xKOUIVm7d3ohijsI+3^?1JH>jW7hP>K*ZQN!ZISw53pdZkLrMX*ro7 zHz?Mqi}b<ffVmiBT5GoD;WUJxD?7K{2fpP~^(Xus1(RJBY;aFSQ?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$vhFOYl} zmgJyI8Ov-ct0V+h~*|0M4%!#0!IQw!d{$BH$g4dNCfP&-kf)5Y^b1i_f0FB1;RO zY!foMMT#bpty!xOmTt;@IcWMfVo4FAH&0$~R(QmcCp63gevBf*nXM5PgcH&-$pmcB zF02L8E_CmOCHWq!{)DB~($cy1{#erJHeyLL_nNS@SbFk?MD%yB(MVw0T(E1xE|Bu z5xI7Wg&vW*+Eq>GCcL~rI6{QGg|H0Iu3O_AE_y_a1f*7F&|<lj&8yw5P=9>1nReJ zw7PJ_a$iWnvawt9Nq6PzVY(b-n1Iuq5si7VbNH%Pe6k{w?X9pnmK?1GMLHxG&s$%j z(*})f$l9EU)rDs^82W8s!E?k|vOUV at fy86-!pOU<adqrRmwfp9NMiR(RLF<pOU9JX zBKZ?prG^ZGKc#gnISvdiQRiDQtV{C(J*k_B&c#^Y*w_LqF^G(f_FrWC9f!7Qad^9 zq1@KY^F2mqY|}oOqcKF|E}fs9d5pTDX%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(GdQ8DhfQZpKk_uO zg=GJu^#+y$4(Ho&@X%xgjEy at tG(z%BxMMSsoedf<akH}z8~_=2W2UGdEtneG*ujri zcqHy<L74Zsi-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$vap19m4< z)_YSg@t1d;mJ;^m``BVy5=K|-0;gN9>?e@!8b5_ztv4LK;BVm3h38(ALUu at b4cC z3*$J5vBlEpuLfEa{|9IpLSn{^?F{%e;oN=i0F#g1y1V%Z7kt8dnQ%IVv)<)lDtWk z&a&44MDoR=SeeaKWV8^D0I*mVt~PJGOmE9#<+EV64omZv35q*tnIPTyZuWZ5)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 at w83nXkzX3_A8 zHL<%Nj>fWqygWbM5y|ixM_$oHzUW)u&fB58h?8WuMhmOl+nmPNO9K at AHaUbOvk-H- zE*B{p0Wza?ECHRF&|1}KMT;Vb)I5kuun;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 at db_ZcO^xrQ1NkjBvKKI~(CF>L(rs!2 z2-+e!9eV{h?$qTi!_w>wENsEd9?O0z;Wo at c7^!gG3~>69zH_U$O)phkc7Ir6YWyAU RodMc0QTRs=7z!{{x@?rvLx|

literal 0
HcmV?d00001

Signature · 1 line
1.9.3
#13 lore raw

PATCH 5/8 colibri_vf: remove spurious new line

In reply to Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

Remove spurious new line in configuration file.

---
 include/configs/colibri_vf.h | 1 -
 1 file changed, 1 deletion(-)
include/configs/colibri_vf.h +0 -1
index aff2810..2523f32 100644
@@ -69,7 +69,6 @@
 				"512k(u-boot-env),"		\
 				"-(ubi)"
 
-
 #define CONFIG_MMC
 #define CONFIG_FSL_ESDHC
 #define CONFIG_SYS_FSL_ESDHC_ADDR	0
Signature · 1 line
1.9.3
#15 lore raw

PATCH 6/8 image-fdt.c: store returned error value

In reply to Marcel Ziswiler

From: Max Krummenacher <[email protected]>

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]

---
 common/image-fdt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
common/image-fdt.c +2 -1
index 80e3e63..d9fce17 100644
@@ -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;
Signature · 1 line
1.9.3
#16 lore raw

PATCH 6/8 image-fdt.c: store returned error value

Quoted 8 lines · Marcel Ziswiler

From: Max Krummenacher <[email protected]>

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]

Best regards,
Marek Vasut

#17 lore raw

PATCH 6/8 image-fdt.c: store returned error value

Quoted 11 lines · Marek Vasut
Quoted 8 lines · Marcel Ziswiler

From: Max Krummenacher <[email protected]>

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]

#18 lore raw

PATCH 7/8 mtd/nand/ubi: assortment of alignment fixes

In reply to Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

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

---
 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(-)
common/cmd_ubi.c +1 -1
index cbc10c5..30a1259 100644
@@ -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;
drivers/mtd/nand/nand_util.c +1 -1
index ee2c24d..395ba2d 100644
@@ -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;
fs/ubifs/super.c +2 -2
index 10f8fff..ac0d13f 100644
@@ -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));
 
 	/*
fs/ubifs/ubifs.c +2 -2
index 6dd6174..03f9f79 100644
@@ -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__);
lib/gzip.c +1 -1
index ff37d4f..2a74411 100644
@@ -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);
 }
Signature · 1 line
1.9.3
#19 lore raw

PATCH 7/8 mtd/nand/ubi: assortment of alignment fixes

Quoted 9 lines · Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

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

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

#20 lore raw

PATCH 7/8 mtd/nand/ubi: assortment of alignment fixes

Quoted 6 lines · Marek Vasut
Quoted 5 lines · Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer alignment into account which led to failures of the following form:

Quoted 8 lines
Quoted 4 lines

ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108
ERROR: v7_dcache_inval_range - stop address is not aligned - 0x1f7f1108

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

#21 lore raw

PATCH 7/8 mtd/nand/ubi: assortment of alignment fixes

Quoted 11 lines · Scott Wood
Quoted 10 lines · Marek Vasut
Quoted 9 lines · Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

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

Quoted 7 lines
Quoted 5 lines
Quoted 1 line

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 ?

Quoted 1 line

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

#22 lore raw

PATCH 7/8 mtd/nand/ubi: assortment of alignment fixes

Quoted 15 lines · Marek Vasut
Quoted 14 lines · Scott Wood
Quoted 13 lines · Marek Vasut
Quoted 11 lines · Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

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

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

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.

Quoted 5 lines
Quoted 2 lines

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?

Quoted 11 lines

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).

Quoted 4 lines

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

#23 lore raw

PATCH 7/8 mtd/nand/ubi: assortment of alignment fixes

Quoted 23 lines · Scott Wood
Quoted 22 lines · Marek Vasut
Quoted 20 lines · Scott Wood
Quoted 11 lines · Marek Vasut
Quoted 10 lines · Marcel Ziswiler

From: Marcel Ziswiler <[email protected]>

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

Quoted 5 lines
Quoted 1 line

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.

Quoted 6 lines
Quoted 1 line

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.

Quoted 9 lines
Quoted 5 lines
Quoted 2 lines

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.

Quoted 12 lines
Quoted 12 lines

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).

Quoted 8 lines
Quoted 4 lines

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

#24 lore raw

PATCH 7/8 mtd/nand/ubi: assortment of alignment fixes

Quoted 20 lines · Marek Vasut
Quoted 19 lines · Scott Wood
Quoted 17 lines · Marek Vasut

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.
"

Quoted 5 lines
Quoted 3 lines

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

#25 lore raw

PATCH 7/8 mtd/nand/ubi: assortment of alignment fixes

Quoted 20 lines · Scott Wood
Quoted 19 lines · Marek Vasut
Quoted 18 lines · Scott Wood
Quoted 16 lines · Marek Vasut

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.
"

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

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

#26 lore raw

PATCH 8/8 tftp.c: fix CONFIG_TFTP_TSIZE for small files

In reply to Marcel Ziswiler

From: Max Krummenacher <[email protected]>

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.

---
 net/tftp.c | 2 ++
 1 file changed, 2 insertions(+)
net/tftp.c +2 -0
index 3e99e73..1853fe7 100644
@@ -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('#');
Signature · 1 line
1.9.3
#27 lore raw

PATCH 8/8 tftp.c: fix CONFIG_TFTP_TSIZE for small files

Quoted 23 lines · Marcel Ziswiler

From: Max Krummenacher <[email protected]>

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.

---
 net/tftp.c | 2 ++
 1 file changed, 2 insertions(+)
net/tftp.c +0 -0
index 3e99e73..1853fe7 100644

net/tftp.c static void show_block_marker(void) · line 252

Quoted 4 lines
net/tftp.c +1 -0
@@ -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](...

Best regards,
Marek Vasut