[U-Boot] FEC and EFI Simple Network

[Fabio Estevam]

Adding Joe in case he has some ideas.

On Tue, Mar 27, 2018 at 9:12 AM, Patrick Wildt <patrick at blueri.se> wrote:
> Hi,
>
> I have been debugging network issues when running an EFI Application
> that uses the EFI Simple Network protocol on an i.MX6 machine (FEC).
>
> The symptom is that u-boot's (FEC) internal RX ring index is reset to 0,
> while the controller is still at idx 3 (or something else).  This is
> caused by the following circumstances:
>
> The Simple Network protocol offers methods like Start(), Stop(),
> Initialize(), Shutdown(), Reset().  Also the protocol has a state.  The
> states are Stopped, Started, Initialized.  The transition is as follows:
>
> Stopped ---Start()---> Started ---Initialize()--> Initialized
>
> Start() does some initialization, Initialize() allocates the TX/RX
> descriptors and actually kicks off the network engine.
>
> So far, only Initialize() is implemented in our u-boot interface, and it
> calls eth_init() which in the end calls fec_init().  Our network state
> is _always_ Started.  This means that EFI Applications see that that the
> state is Started and then call Initialize() to start the actual network
> traffic.  There is no call to Stop()/Shutdown()/Reset() as it's supposed
> to be in a sane state.
>
> In my case the FEC is already initialized since I booted using network
> and the RX desc index is already non-zero.  Now the EFI Application sees
> that the state is Started, calls Initialize() which makes u-boot call
> eth_init() which then calls fec_init().
>
> fec_init() does not reset the controller so that the controller-internal
> RX desc index is not reset to zero.  fec_init() calls fec_open() which
> then resets the driver-internal RX desc index to zero.  Now they are out
> of sync, boom.
>
> This means that fec_init() without a previous fec_halt() breaks the
> whole network if it was already running.  The Designware driver as used
> by some sunxi platforms does a reset of the controller in the init
> function.  Maybe calling fec_halt() at the start of fec_init() could be
> a possible solution?
>
> Patrick
>
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index ff7ad91116..ba8bd9920d 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -522,6 +522,12 @@ static int fec_open(struct eth_device *edev)
>         return 0;
>  }
>
> +#ifdef CONFIG_DM_ETH
> +static void fecmxc_halt(struct udevice *dev);
> +#else
> +static void fec_halt(struct eth_device *dev);
> +#endif
> +
>  #ifdef CONFIG_DM_ETH
>  static int fecmxc_init(struct udevice *dev)
>  #else
> @@ -537,6 +543,15 @@ static int fec_init(struct eth_device *dev, bd_t *bd)
>         u8 *i;
>         ulong addr;
>
> +#ifdef CONFIG_DM_ETH
> +       fecmxc_halt(dev);
> +#else
> +       fec_halt(dev);
> +#endif
> +
> +       writel(~FEC_TCNTRL_GTS & readl(&fec->eth->x_cntrl),
> +              &fec->eth->x_cntrl);
> +
>         /* Initialize MAC address */
>  #ifdef CONFIG_DM_ETH
>         fecmxc_set_hwaddr(dev);
> @@ -825,19 +840,12 @@ static int fec_recv(struct eth_device *dev)
>         }
>         if (ievent & FEC_IEVENT_HBERR) {
>                 /* Heartbeat error */
> -               writel(0x00000001 | readl(&fec->eth->x_cntrl),
> +               writel(FEC_TCNTRL_GTS | readl(&fec->eth->x_cntrl),
>                        &fec->eth->x_cntrl);
>         }
>         if (ievent & FEC_IEVENT_GRA) {
>                 /* Graceful stop complete */
> -               if (readl(&fec->eth->x_cntrl) & 0x00000001) {
> -#ifdef CONFIG_DM_ETH
> -                       fecmxc_halt(dev);
> -#else
> -                       fec_halt(dev);
> -#endif
> -                       writel(~0x00000001 & readl(&fec->eth->x_cntrl),
> -                              &fec->eth->x_cntrl);
> +               if (readl(&fec->eth->x_cntrl) & FEC_TCNTRL_GTS) {
>  #ifdef CONFIG_DM_ETH
>                         fecmxc_init(dev);
>  #else
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot