[U-Boot] [PATCH 01/28] net: Remove volatile from all of net except the eth driver interface

[Joe Hershberger]

Hi Simon,

On Tue, Jan 24, 2012 at 12:09 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Joe,
>
> On Fri, Jan 20, 2012 at 12:15 PM, Joe Hershberger
> <joe.hershberger at gmail.com> wrote:
>> Hi Simon,
>>
>> On Fri, Jan 20, 2012 at 10:22 AM, Simon Glass <sjg at chromium.org> wrote:
>>> Hi Joe,
>>>
>>> On Thu, Jan 19, 2012 at 4:53 PM, Joe Hershberger <joe.hershberger at ni.com> wrote:
>>>> The mv_eth driver should not redefine the net function definition
>>>>
>>>> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
>>>> Cc: Joe Hershberger <joe.hershberger at gmail.com>
>>>> Cc: Wolfgang Denk <wd at denx.de>
>>>> ---
>>>>  <snip>
>>>> @@ -1454,9 +1454,9 @@ NetReceive(volatile uchar *inpkt, int len)
>>>>
>>>>        debug("packet received\n");
>>>>
>>>> -       NetRxPacket = inpkt;
>>>> +       NetRxPacket = (uchar *)inpkt;
>>>>        NetRxPacketLen = len;
>>>> -       et = (Ethernet_t *)inpkt;
>>>> +       et = (Ethernet_t *)NetRxPacket;
>>>
>>> Why change this?
>>>
>>>>
>>>>        /* too small packet? */
>>>>        if (len < ETHER_HDR_SIZE)
>>>> @@ -1491,11 +1491,11 @@ NetReceive(volatile uchar *inpkt, int len)
>>>>                 */
>>>>                x = ntohs(et->et_prot);
>>>>
>>>> -               ip = (IP_t *)(inpkt + E802_HDR_SIZE);
>>>> +               ip = (IP_t *)(NetRxPacket + E802_HDR_SIZE);
>>>
>>> and these? You are using a global instead of the passed-in local.
>>>
>>>>                len -= E802_HDR_SIZE;
>>>>
>>>>        } else if (x != PROT_VLAN) {    /* normal packet */
>>>> -               ip = (IP_t *)(inpkt + ETHER_HDR_SIZE);
>>>> +               ip = (IP_t *)(NetRxPacket + ETHER_HDR_SIZE);
>>>
>>>
>>>>                len -= ETHER_HDR_SIZE;
>>>>
>>>>        } else {                        /* VLAN packet */
>>>> @@ -1519,7 +1519,7 @@ NetReceive(volatile uchar *inpkt, int len)
>>>>                vlanid = cti & VLAN_IDMASK;
>>>>                x = ntohs(vet->vet_type);
>>>>
>>>> -               ip = (IP_t *)(inpkt + VLAN_ETHER_HDR_SIZE);
>>>> +               ip = (IP_t *)(NetRxPacket + VLAN_ETHER_HDR_SIZE);
>>>
>>>
>>>>                len -= VLAN_ETHER_HDR_SIZE;
>>>>        }
>>>>
>>
>> This patch removes volatile from the NetRxPacket and all but 1 of the
>> other places that inpkt was assigned.  You will notice that the first
>> assignment of inpkt to NetRxPacket casts away the volatile:
>
> Yes - I wonder why NetReceive needs to remain volatile?

It is called by all the Ethernet drivers with volatile buffer
pointers.  At this point, I decided to limit the changes to not
include every Ethernet driver (have to split it somewhere).

>>>> @@ -1454,9 +1454,9 @@ NetReceive(volatile uchar *inpkt, int len)
>>>> <snip>
>>>> -       NetRxPacket = inpkt;
>>>> +       NetRxPacket = (uchar *)inpkt;
>>
>> All the assignments that need a non-volatile pointer now use
>> NetRxPacket instead of inpkt, since it is already assigned and and the
>> same type minus volatile.
>
> Yes, I am only sensitive to this because it is a global and there is
> enough use of globals in the net code already.

I chose to not add a local variable to hold the non-volatile pointer
since there was already a global.  Since u-boot is single threaded,
this should not be a problem and could be easily changed later when
the Ethernet driver interface is cleaned up.

Best regards,
-Joe