Hi Simon, On Tue, Jan 24, 2012 at 12:09 AM, Simon Glass <s...@chromium.org> wrote: > Hi Joe, > > On Fri, Jan 20, 2012 at 12:15 PM, Joe Hershberger > <joe.hershber...@gmail.com> wrote: >> Hi Simon, >> >> On Fri, Jan 20, 2012 at 10:22 AM, Simon Glass <s...@chromium.org> wrote: >>> Hi Joe, >>> >>> On Thu, Jan 19, 2012 at 4:53 PM, Joe Hershberger <joe.hershber...@ni.com> >>> wrote: >>>> The mv_eth driver should not redefine the net function definition >>>> >>>> Signed-off-by: Joe Hershberger <joe.hershber...@ni.com> >>>> Cc: Joe Hershberger <joe.hershber...@gmail.com> >>>> Cc: Wolfgang Denk <w...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot