Re: [Qemu-arm] [PATCH v2 05/15] imx_fec: Use ENET_FTRL to determine truncation length

[Philippe Mathieu-Daudé]

Hi Andrey,

On 12/14/2017 11:52 AM, Andrey Smirnov wrote:
> Frame truncation length, TRUNC_FL, is determined by the contents of
> ENET_FTRL register, so convert the code to use it instead of a
> hardcoded constant.
> 
> To avoid the case where TRUNC_FL is greater that ENET_MAX_FRAME_SIZE,
> increase the value of the latter to its theoretical maximum of 16K.
> 
> Cc: Peter Maydell <address@hidden>
> Cc: Jason Wang <address@hidden>
> Cc: Philippe Mathieu-Daudé <address@hidden>
> Cc: address@hidden
> Cc: address@hidden
> Cc: address@hidden
> Signed-off-by: Andrey Smirnov <address@hidden>
> ---
>  hw/net/imx_fec.c         | 4 ++--
>  include/hw/net/imx_fec.h | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
> index 56cb72273c..50da91bf9e 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -1052,8 +1052,8 @@ static ssize_t imx_enet_receive(NetClientState *nc, 
> const uint8_t *buf,
>      crc_ptr = (uint8_t *) &crc;
>  
>      /* Huge frames are truncted.  */
> -    if (size > ENET_MAX_FRAME_SIZE) {
> -        size = ENET_MAX_FRAME_SIZE;
> +    if (size > s->regs[ENET_FTRL]) {

Shouldn't this be:

       if (size > s->regs[ENET_FTRL] + 1) {

> +        size = s->regs[ENET_FTRL];

and:

           size = s->regs[ENET_FTRL] + 1;

>          flags |= ENET_BD_TR | ENET_BD_LG;
>      }
>  
> diff --git a/include/hw/net/imx_fec.h b/include/hw/net/imx_fec.h
> index 67993870a2..a390d704a6 100644
> --- a/include/hw/net/imx_fec.h
> +++ b/include/hw/net/imx_fec.h
> @@ -86,7 +86,6 @@
>  #define ENET_TCCR3             393
>  #define ENET_MAX               400
>  
> -#define ENET_MAX_FRAME_SIZE    2032
>  
>  /* EIR and EIMR */
>  #define ENET_INT_HB            (1 << 31)
> @@ -155,6 +154,8 @@
>  #define ENET_RCR_NLC           (1 << 30)
>  #define ENET_RCR_GRS           (1 << 31)
>  
> +#define ENET_MAX_FRAME_SIZE    (1 << ENET_RCR_MAX_FL_LENGTH)
> +
>  /* TCR */
>  #define ENET_TCR_GTS           (1 << 0)
>  #define ENET_TCR_FDEN          (1 << 2)
>