[U-Boot] [PATCH v2 5/6] TFTP: net/tftp.c: add server mode receive

[Detlev Zundel]

Hi Luca,

> Signed-off-by: Luca Ceresoli <luca.ceresoli at comelit.it>
> Cc: Wolfgang Denk <wd at denx.de>
> ---
> Changes in v2: none.

Only one comment below

>
>  net/tftp.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  net/tftp.h |    6 +++++
>  2 files changed, 74 insertions(+), 4 deletions(-)
>
> diff --git a/net/tftp.c b/net/tftp.c
> index e166a63..87eb0b8 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -2,6 +2,8 @@
>   *	Copyright 1994, 1995, 2000 Neil Russell.
>   *	(See License)
>   *	Copyright 2000, 2001 DENX Software Engineering, Wolfgang Denk, wd at denx.de
> + *	Copyright 2011 Comelit Group SpA,
> + *	               Luca Ceresoli <luca.ceresoli at comelit.it>
>   */
>  
>  #include <common.h>
> @@ -74,6 +76,7 @@ static short	TftpNumchars;	/* The number of hashes we printed      */
>  #define STATE_TOO_LARGE	3
>  #define STATE_BAD_MAGIC	4
>  #define STATE_OACK	5
> +#define STATE_RECV_WRQ	6
>  
>  #define TFTP_BLOCK_SIZE		512		    /* default TFTP block size	*/
>  #define TFTP_SEQUENCE_SIZE	((ulong)(1<<16))    /* sequence number is 16 bit */
> @@ -241,6 +244,10 @@ TftpSend (void)
>  			TftpBlock=ext2_find_next_zero_bit(Bitmap,(Mapsize*8),0);
>  		/*..falling..*/
>  #endif
> +
> +#ifdef CONFIG_CMD_TFTPSRV
> +	case STATE_RECV_WRQ:
> +#endif
>  	case STATE_DATA:
>  		xp = pkt;
>  		s = (ushort *)pkt;
> @@ -293,7 +300,11 @@ TftpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
>  #endif
>  		return;
>  	}
> -	if (TftpState != STATE_SEND_RRQ && src != TftpRemotePort)
> +	if (TftpState != STATE_SEND_RRQ &&
> +#ifdef CONFIG_CMD_TFTPSRV
> +	    TftpState != STATE_RECV_WRQ &&
> +#endif
> +	    src != TftpRemotePort)
>  		return;

Hm, I have to admit that I do not really like adding so many ifdefs into
the tftp code and even into the state machine code.  Can you give me a
number on how much larger u-boot gets on your platform with and without
the server support?  If this is not too much, maybe we should include
support always.  If it is too much, maybe we can at least keep the state
machine without ifdefs - if I see it correctly, this will only add at
maximum a few bytes and STATE_RECW_WRQ will simply never be entered,
correct?

Cheers
  Detlev

-- 
Zivilisation ist der Zaubertrick, der uns unsere wahre Natur verbirgt.
                                            -- Salman Rushdie
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de