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

[Simon Glass]

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?

>
>>> @@ -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.

>
> Best regards,
> -Joe

Regards,
Simon