[U-Boot] [PATCH] tsec: Configure the buffer descriptor bases to always include all of the descriptors
Andy Fleming
afleming at freescale.com
Wed Aug 10 21:49:13 CEST 2011
On Aug 10, 2011, at 2:24 PM, Detlev Zundel wrote:
> Hi Joe,
>
>> On Wed, Aug 10, 2011 at 7:29 AM, Detlev Zundel <dzu at denx.de> wrote:
>>>> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
>>>> index 78ffc95..1805ca0 100644
>>>> --- a/drivers/net/tsec.c
>>>> +++ b/drivers/net/tsec.c
>>>> @@ -250,8 +250,8 @@ static void startup_tsec(struct eth_device *dev)
>>>> txIdx = 0;
>>>>
>>>> /* Point to the buffer descriptors */
>>>> - out_be32(®s->tbase, (unsigned int)(&rtx.txbd[txIdx]));
>>>> - out_be32(®s->rbase, (unsigned int)(&rtx.rxbd[rxIdx]));
>>>> + out_be32(®s->tbase, (unsigned int)(&rtx.txbd[0]));
>>>> + out_be32(®s->rbase, (unsigned int)(&rtx.rxbd[0]));
>>>>
>>>> /* Initialize the Rx Buffer descriptors */
>>>> for (i = 0; i < PKTBUFSRX; i++) {
>>>
>>> I see these two lines just before the code you change (one is even in
>>> the context of your patch):
>>>
>>> /* reset the indices to zero */
>>> rxIdx = 0;
>>> txIdx = 0;
>>>
>>> So can you tell me, what your change actually does? I cannot remember
>>> that we have concurrency issues here, or do we?
>>
>> My apologies... I ported this patch from my work in u-boot 2009.11 and
>> did not notice that change above. I think explicitly using 0 when
>> assigning the base address pointers is clearer, though.
>>
>> It seems the resetting of the indexes to 0 was added by Andy Fleming
>> in 063c12633d5ad74d52152d9c358e715475e17629, though the log doesn't
>> discuss it..
>
> Yes, I see - it even slipped my review :( For the patch as such I don't
> have a preference - looking at the code both ways really read the same
> for me.
Well, it wasn't added in that patch, exactly. What really happened is I accidentally applied two patches, and then had to break them up again. That part accidentally got put in the second patch. A careful review of the patch history indicates that the indices have always been zeroed out beforehand (though sometimes in separate functions).
All the same, it looks like this patch is a good idea, to me.
Andy
More information about the U-Boot
mailing list