[U-Boot] [PATCH] xyzModem.c Packet buffer dynamic allocate

Hello.
It changes reduce bss usage. Ymodem receive buffer dinamic allocation.
Signed-off-by: Yoshinori Sato ysato@users.sourceforge.jp --- common/cmd_load.c | 1 + common/xyzModem.c | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/common/cmd_load.c b/common/cmd_load.c index dad0303..f144bf4 100644 --- a/common/cmd_load.c +++ b/common/cmd_load.c @@ -992,6 +992,7 @@ static ulong load_serial_ymodem (ulong offset) store_addr, res); if (rc != 0) { flash_perror (rc); + xyzModem_stream_close (&err); return (~0); } } else diff --git a/common/xyzModem.c b/common/xyzModem.c index 7a46805..5af995d 100644 --- a/common/xyzModem.c +++ b/common/xyzModem.c @@ -58,6 +58,7 @@ #include <xyzModem.h> #include <stdarg.h> #include <crc.h> +#include <malloc.h>
/* Assumption - run xyzModem protocol over the console port */
@@ -81,7 +82,7 @@ static struct #else int *__chan; #endif - unsigned char pkt[1024], *bufp; + unsigned char *pkt, *bufp; unsigned char blk, cblk, crc1, crc2; unsigned char next_blk; /* Expected block */ int len, mode, total_retries; @@ -353,6 +354,7 @@ xyzModem_get_hdr (void) bool hdr_found = false; int i, can_total, hdr_chars; unsigned short cksum; + char *pktp;
ZM_DEBUG (zm_new ()); /* Find the start of a header */ @@ -433,13 +435,14 @@ xyzModem_get_hdr (void) } xyz.len = (c == SOH) ? 128 : 1024; xyz.bufp = xyz.pkt; + pktp = xyz.pkt; for (i = 0; i < xyz.len; i++) { res = CYGACC_COMM_IF_GETC_TIMEOUT (*xyz.__chan, &c); ZM_DEBUG (zm_save (c)); if (res) { - xyz.pkt[i] = c; + *pktp++ = c; } else { @@ -489,9 +492,10 @@ xyzModem_get_hdr (void) else { cksum = 0; + pktp = xyz.pkt; for (i = 0; i < xyz.len; i++) { - cksum += xyz.pkt[i]; + cksum += *pktp++; } if (xyz.crc1 != (cksum & 0xFF)) { @@ -560,6 +564,11 @@ xyzModem_stream_open (connection_info_t * info, int *err) xyz.read_length = 0; xyz.file_length = 0; #endif + if (xyz.pkt == NULL) { + xyz.pkt = malloc(1024); + if (xyz.pkt == NULL) + return -1; + }
CYGACC_COMM_IF_PUTC (*xyz.__chan, (xyz.crc_mode ? 'C' : NAK));
@@ -743,6 +752,8 @@ xyzModem_stream_close (int *err) xyz.crc_mode ? "CRC" : "Cksum", xyz.total_SOH, xyz.total_STX, xyz.total_CAN, xyz.total_retries); ZM_DEBUG (zm_flush ()); + free(xyz.pkt); + xyz.pkt = NULL; }
/* Need to be able to clean out the input buffer, so have to take the */

Dear Yoshinori Sato,
In message 87sk0q1mnf.wl%ysato@users.sourceforge.jp you wrote:
It changes reduce bss usage.
... and increases the load on the malloc arean.
Ymodem receive buffer dinamic allocation.
As far as I can see we increase the code size, and shift a fixed sized buffer from bss to the maloc arene.
I don't consider this an improvement, but I may be overlooking something.
Which advantages do you see with the changed code?
Best regards,
Wolfgang Denk

At Fri, 01 Oct 2010 09:57:49 +0200, Wolfgang Denk wrote:
Dear Yoshinori Sato,
In message 87sk0q1mnf.wl%ysato@users.sourceforge.jp you wrote:
It changes reduce bss usage.
... and increases the load on the malloc arean.
Ymodem receive buffer dinamic allocation.
As far as I can see we increase the code size, and shift a fixed sized buffer from bss to the maloc arene.
I don't consider this an improvement, but I may be overlooking something.
Which advantages do you see with the changed code?
My target have too small size RAM. I want reduced to data and bss.
It is limited to one usage in a static allocation. But dynamic allocation can use generically.
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de If the facts don't fit the theory, change the facts. -- Albert Einstein

On Fri, Oct 1, 2010 at 11:02 AM, Yoshinori Sato wrote:
At Fri, 01 Oct 2010 09:57:49 +0200, Wolfgang Denk wrote:
Yoshinori Sato wrote:
It changes reduce bss usage.
... and increases the load on the malloc arean.
Ymodem receive buffer dinamic allocation.
As far as I can see we increase the code size, and shift a fixed sized buffer from bss to the maloc arene.
I don't consider this an improvement, but I may be overlooking something.
Which advantages do you see with the changed code?
My target have too small size RAM. I want reduced to data and bss.
It is limited to one usage in a static allocation. But dynamic allocation can use generically.
i feel like this is a much more uncommon usage scenario and is more harmful than good to boards, and one which isnt specific to xyzModem. perhaps we need a general tune config which says "prefer malloc over bss" and this creates a set of helper macros which expand either into malloc calls or nothing (since the storage is already in bss). -mike
participants (3)
-
Mike Frysinger
-
Wolfgang Denk
-
Yoshinori Sato