diff mbox

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

Message ID 4F567FE5.3050805@boundarydevices.com
State Superseded
Headers show

Commit Message

Eric Nelson March 6, 2012, 9:21 p.m. UTC 
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.

U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Marek Vasut March 6, 2012, 9:38 p.m. UTC | #1 
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
diff mbox

Patch

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);
-
-

_______________________________________________
U-Boot mailing list