[U-Boot] [PATCH V2] net: fec_mxc: allow use with cache enabled

[Marek Vasut <marex@denx.de>]

Dear Eric Nelson,

> On 03/06/2012 12:49 PM, Marek Vasut wrote:
> > Dear Eric Nelson,
> > 
> >> On 03/06/2012 10:06 AM, Eric Nelson wrote:
> >>> On 03/05/2012 01:06 PM, Marek Vasut wrote:
> >>>> Dear Eric Nelson,
> >>>> 
> >>   >>  <snip>
> >>>>> 
> >>>>> + if (!fec->rbd_base) {
> >>>>> + ret = -ENOMEM;
> >>>>> + goto err2;
> >>>>> + }
> >>>>> + memset(fec->rbd_base, 0, size);
> >>>>> + }
> >>>> 
> >>>> We might want to flush the descriptors to memory after they have been
> >>>> inited?
> >> 
> >> We're also missing a flush after the call to fec_rbd_init().
> > 
> > I think we need only one flush in the whole allocation sequence. But
> > you're probably right here.
> 
> Yeah. There's no point in doing memset() and flush() just to write them
> again and not flush the proper values.
> 
> >> I'm inclined to move that call to right after the memset and before
> >> a newly-added flush and do the same with the call to tbd_init().
> > 
> > You mean into fec_rbd_init() ?
> 
> fec_tbd_init(). See below.
> 
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index 0db5ca9..d5d0d5e 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -544,6 +544,7 @@ static int fec_init(struct eth_device *dev, bd_t* bd)
>                          goto err1;
>                  }
>                  memset(fec->tbd_base, 0, size);
> +               fec_tbd_init(fec);
>                  flush_dcache_range((unsigned)fec->tbd_base, size);
>          }
> 
> @@ -560,6 +561,13 @@ static int fec_init(struct eth_device *dev, bd_t* bd)
>                          goto err2;
>                  }
>                  memset(fec->rbd_base, 0, size);
> +               /*
> +                * Initialize RxBD ring
> +                */
> +               if (fec_rbd_init(fec, FEC_RBD_NUM, FEC_MAX_PKT_SIZE) < 0) {
> +                       ret = -ENOMEM;
> +                       goto err3;
> +               }
>                  flush_dcache_range((unsigned)fec->rbd_base,
>                                     (unsigned)fec->rbd_base + size);
>          }
> @@ -619,16 +627,6 @@ static int fec_init(struct eth_device *dev, bd_t* bd)
>          writel((uint32_t)fec->tbd_base, &fec->eth->etdsr);
>          writel((uint32_t)fec->rbd_base, &fec->eth->erdsr);
> 
> -       /*
> -        * Initialize RxBD/TxBD rings
> -        */
> -       if (fec_rbd_init(fec, FEC_RBD_NUM, FEC_MAX_PKT_SIZE) < 0) {
> -               ret = -ENOMEM;
> -               goto err3;
> -       }
> -       fec_tbd_init(fec);
> -
> -

I see, makes sense.

Best regards,
Marek Vasut