RE: [PATCH] net/imx_fec: Updating the IMX_FEC IP to support loopback mode.

[Wasim, Bilal]

Thanks for the pointers philippe.. Is the patch okay to be merged without it or 
do I need to do a re-submission with the updated username ?

- Bilal 

-----Original Message-----
From: Philippe Mathieu-Daudé [mailto:address@hidden] 
Sent: Friday, November 29, 2019 8:38 PM
To: address@hidden; address@hidden
Cc: address@hidden; address@hidden; address@hidden; address@hidden; Wasim, 
Bilal <address@hidden>; address@hidden; Jason Wang <address@hidden>
Subject: Re: [PATCH] net/imx_fec: Updating the IMX_FEC IP to support loopback 
mode.

Hi Bilal,

Cc'ing Jason, the maintainer of network devices.

On 11/29/19 4:05 PM, address@hidden wrote:
> From: bwasim <address@hidden>

Your git setup misses your 'user.name', you could fix it running:

   git config user.name "Bilal Wasim"

(eventually with the --global option).

The patch looks good otherwise.

Thanks!

> Loopback mode only works when specific conditions (as dictated by the 
> IP guide) are met, i.e. the MII_MODE is set and the RMII_MODE is 
> cleared. If not, we simply send the packet on the output queue (for TX 
> to the host network). Tested by running a custom RTOS and TXing a ton 
> of packets. The same packets were received on the RX side..
> 
> Signed-off-by: Bilal Wasim <address@hidden>
> ---
>   hw/net/imx_fec.c | 27 +++++++++++++++++++++++++--
>   1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c index 
> bd99236864..c51e7f7363 100644
> --- a/hw/net/imx_fec.c
> +++ b/hw/net/imx_fec.c
> @@ -256,6 +256,29 @@ static const VMStateDescription vmstate_imx_eth = 
> {
>   
>   static void imx_eth_update(IMXFECState *s);
>   
> +/*
> + * Function to check if the MAC is configured to run in loopback mode.
> + * If so, invoke the "receive" routine.
> + * Else write to the output.
> + * */
> +static void send_pkt(IMXFECState *s, uint8_t *frame, int frame_size) 
> +{
> +    NetClientState *nc = qemu_get_queue(s->nic);
> +
> +    /*
> +     * Loopback or Normal mode ?
> +     * Per the FEC Manual: If loopback is enabled, the MII_MODE
> +     * should be SET and the RMII_MODE should be cleared. Loopback
> +     * will only work if this criterion is met. If not met,
> +     * we will send the frame on the output queue. */
> +    if ((s->regs[ENET_RCR] & ENET_RCR_LOOP) && (s->regs[ENET_RCR] & 
> ENET_RCR_MII_MODE)
> +            && !(s->regs[ENET_RCR] & ENET_RCR_RMII_MODE)) {
> +        nc->info->receive(nc, frame, frame_size);
> +    } else {
> +        qemu_send_packet(nc, frame, frame_size);
> +    }
> +}
> +
>   /*
>    * The MII phy could raise a GPIO to the processor which in turn
>    * could be handled as an interrpt by the OS.
> @@ -488,7 +511,7 @@ static void imx_fec_do_tx(IMXFECState *s)
>           frame_size += len;
>           if (bd.flags & ENET_BD_L) {
>               /* Last buffer in frame.  */
> -            qemu_send_packet(qemu_get_queue(s->nic), s->frame, frame_size);
> +            send_pkt(s, (uint8_t *)&s->frame, frame_size);
>               ptr = s->frame;
>               frame_size = 0;
>               s->regs[ENET_EIR] |= ENET_INT_TXF; @@ -586,7 +609,7 @@ 
> static void imx_enet_do_tx(IMXFECState *s, uint32_t index)
>               }
>               /* Last buffer in frame.  */
>   
> -            qemu_send_packet(qemu_get_queue(s->nic), s->frame, frame_size);
> +            send_pkt(s, (uint8_t *)&s->frame, frame_size);
>               ptr = s->frame;
>   
>               frame_size = 0;
>