From patchwork Tue Nov 29 16:02:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olliver Schinagl X-Patchwork-Id: 700614 X-Patchwork-Delegate: joe.hershberger@gmail.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 3tSpHP64Z7z9t2T for ; Wed, 30 Nov 2016 03:03:13 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=schinagl.nl header.i=@schinagl.nl header.b="RdhT/Z7J"; dkim-atps=neutral Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 41079B3861; Tue, 29 Nov 2016 17:03:11 +0100 (CET) Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id B-BKrNWer65w; Tue, 29 Nov 2016 17:03:11 +0100 (CET) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 9567CB3841; Tue, 29 Nov 2016 17:03:10 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 6CC07B383F for ; Tue, 29 Nov 2016 17:03:05 +0100 (CET) Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BaZWtL6vBdN5 for ; Tue, 29 Nov 2016 17:03:05 +0100 (CET) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from 7of9.schinagl.nl (7of9.schinagl.nl [88.159.158.68]) by theia.denx.de (Postfix) with ESMTPS id 311EDB383E for ; Tue, 29 Nov 2016 17:03:01 +0100 (CET) Received: from [10.180.0.193] (static-98-101-100-159.thenetworkfactory.nl [159.100.101.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by 7of9.schinagl.nl (Postfix) with ESMTPSA id 09BD1567EB; Tue, 29 Nov 2016 17:02:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=schinagl.nl; s=7of9; t=1480435379; bh=MKONk19l3b5sv+etpoLmXWq1z9r26PUH0HLHamYzuR8=; h=Subject:To:References:Cc:From:Date:In-Reply-To; b=RdhT/Z7J5K8Pr8MP1hN55CGZOqnwdi6tIqCcKawxee4rX+MZykE7S4WQEBE2ruRh+ K3Ro6Of3bWUUj/FcAtlM1tViLl2d7vUecPIFpDWtW4r/wIQK7d3P9TV0mjDuRJZejM FpmepLxI0ytKSemLx8A9vP/lGyv2LJNX+ybx1dL4= To: Joe Hershberger , Simon Glass , Hans de Goede References: <[PATCHv3] Retrieve MAC address from EEPROM> <20161125153032.14617-1-oliver@schinagl.nl> <20161125153032.14617-2-oliver@schinagl.nl> From: Olliver Schinagl Message-ID: <2cbd7fc0-1340-3c53-37d8-b920936852d8@schinagl.nl> Date: Tue, 29 Nov 2016 17:02:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161125153032.14617-2-oliver@schinagl.nl> Cc: Fabio Estevam , Stefan Roese , Stephen Warren , Lev Iserovich , dev@linux-sunxi.org, u-boot@lists.denx.de, Alexey Brodkin , Michal Simek , Jeffy Chen Subject: Re: [U-Boot] [PATCH 01/14] net: cosmetic: Do not use magic values for ARP_HLEN X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.15 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" Just a note to myself mostly, I forgot to this one in the patch series, son in v2 I'll add that as well. On 25-11-16 16:30, Olliver Schinagl wrote: > Commit 674bb249825a ("net: cosmetic: Replace magic numbers in arp.c with > constants") introduced a nice define to replace the magic value 6 for > the ethernet hardware address. Replace more hardcoded instances of 6 > which really reference the ARP_HLEN (iow the MAC/Hardware/Ethernet > address). > > Signed-off-by: Olliver Schinagl > --- > common/fdt_support.c | 2 +- > include/net.h | 52 +++++++++++++++++++++++++++------------------------- > net/eth-uclass.c | 10 +++++----- > net/eth_legacy.c | 10 +++++----- > 4 files changed, 38 insertions(+), 36 deletions(-) > > diff --git a/common/fdt_support.c b/common/fdt_support.c > index 0609470..b082662 100644 > --- a/common/fdt_support.c > +++ b/common/fdt_support.c > @@ -471,7 +471,7 @@ void fdt_fixup_ethernet(void *fdt) > char *tmp, *end; > char mac[16]; > const char *path; > - unsigned char mac_addr[6]; > + unsigned char mac_addr[ARP_HLEN]; > int offset; > > if (fdt_path_offset(fdt, "/aliases") < 0) > diff --git a/include/net.h b/include/net.h > index 06320c6..8137cf3 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -38,6 +38,9 @@ > > #define PKTALIGN ARCH_DMA_MINALIGN > > +/* ARP hardware address length */ > +#define ARP_HLEN 6 > + > /* IPv4 addresses are always 32 bits in size */ > struct in_addr { > __be32 s_addr; > @@ -90,7 +93,7 @@ enum eth_state_t { > */ > struct eth_pdata { > phys_addr_t iobase; > - unsigned char enetaddr[6]; > + unsigned char enetaddr[ARP_HLEN]; > int phy_interface; > int max_speed; > }; > @@ -161,7 +164,7 @@ void eth_halt_state_only(void); /* Set passive state */ > #ifndef CONFIG_DM_ETH > struct eth_device { > char name[16]; > - unsigned char enetaddr[6]; > + unsigned char enetaddr[ARP_HLEN]; > phys_addr_t iobase; > int state; > > @@ -293,9 +296,9 @@ u32 ether_crc(size_t len, unsigned char const *p); > */ > > struct ethernet_hdr { > - u8 et_dest[6]; /* Destination node */ > - u8 et_src[6]; /* Source node */ > - u16 et_protlen; /* Protocol or length */ > + u8 et_dest[ARP_HLEN]; /* Destination node */ > + u8 et_src[ARP_HLEN]; /* Source node */ > + u16 et_protlen; /* Protocol or length */ > }; > > /* Ethernet header size */ > @@ -304,16 +307,16 @@ struct ethernet_hdr { > #define ETH_FCS_LEN 4 /* Octets in the FCS */ > > struct e802_hdr { > - u8 et_dest[6]; /* Destination node */ > - u8 et_src[6]; /* Source node */ > - u16 et_protlen; /* Protocol or length */ > - u8 et_dsap; /* 802 DSAP */ > - u8 et_ssap; /* 802 SSAP */ > - u8 et_ctl; /* 802 control */ > - u8 et_snap1; /* SNAP */ > + u8 et_dest[ARP_HLEN]; /* Destination node */ > + u8 et_src[ARP_HLEN]; /* Source node */ > + u16 et_protlen; /* Protocol or length */ > + u8 et_dsap; /* 802 DSAP */ > + u8 et_ssap; /* 802 SSAP */ > + u8 et_ctl; /* 802 control */ > + u8 et_snap1; /* SNAP */ > u8 et_snap2; > u8 et_snap3; > - u16 et_prot; /* 802 protocol */ > + u16 et_prot; /* 802 protocol */ > }; > > /* 802 + SNAP + ethernet header size */ > @@ -323,11 +326,11 @@ struct e802_hdr { > * Virtual LAN Ethernet header > */ > struct vlan_ethernet_hdr { > - u8 vet_dest[6]; /* Destination node */ > - u8 vet_src[6]; /* Source node */ > - u16 vet_vlan_type; /* PROT_VLAN */ > - u16 vet_tag; /* TAG of VLAN */ > - u16 vet_type; /* protocol type */ > + u8 vet_dest[ARP_HLEN]; /* Destination node */ > + u8 vet_src[ARP_HLEN]; /* Source node */ > + u16 vet_vlan_type; /* PROT_VLAN */ > + u16 vet_tag; /* TAG of VLAN */ > + u16 vet_type; /* protocol type */ > }; > > /* VLAN Ethernet header size */ > @@ -398,7 +401,6 @@ struct arp_hdr { > # define ARP_ETHER 1 /* Ethernet hardware address */ > u16 ar_pro; /* Format of protocol address */ > u8 ar_hln; /* Length of hardware address */ > -# define ARP_HLEN 6 > u8 ar_pln; /* Length of protocol address */ > # define ARP_PLEN 4 > u16 ar_op; /* Operation */ > @@ -507,16 +509,16 @@ extern char net_nis_domain[32]; /* Our IS domain */ > extern char net_hostname[32]; /* Our hostname */ > extern char net_root_path[64]; /* Our root path */ > /** END OF BOOTP EXTENTIONS **/ > -extern u8 net_ethaddr[6]; /* Our ethernet address */ > -extern u8 net_server_ethaddr[6]; /* Boot server enet address */ > +extern u8 net_ethaddr[ARP_HLEN]; /* Our ethernet address */ > +extern u8 net_server_ethaddr[ARP_HLEN]; /* Boot server enet address */ > extern struct in_addr net_ip; /* Our IP addr (0 = unknown) */ > extern struct in_addr net_server_ip; /* Server IP addr (0 = unknown) */ > extern uchar *net_tx_packet; /* THE transmit packet */ > extern uchar *net_rx_packets[PKTBUFSRX]; /* Receive packets */ > extern uchar *net_rx_packet; /* Current receive packet */ > extern int net_rx_packet_len; /* Current rx packet length */ > -extern const u8 net_bcast_ethaddr[6]; /* Ethernet broadcast address */ > -extern const u8 net_null_ethaddr[6]; > +extern const u8 net_bcast_ethaddr[ARP_HLEN]; /* Ethernet broadcast address */ > +extern const u8 net_null_ethaddr[ARP_HLEN]; > > #define VLAN_NONE 4095 /* untagged */ > #define VLAN_IDMASK 0x0fff /* mask of valid vlan id */ > @@ -555,9 +557,9 @@ extern ushort cdp_appliance_vlan; /* CDP returned appliance VLAN */ > */ > static inline int is_cdp_packet(const uchar *ethaddr) > { > - extern const u8 net_cdp_ethaddr[6]; > + extern const u8 net_cdp_ethaddr[ARP_HLEN]; > > - return memcmp(ethaddr, net_cdp_ethaddr, 6) == 0; > + return memcmp(ethaddr, net_cdp_ethaddr, ARP_HLEN) == 0; > } > #endif > > diff --git a/net/eth-uclass.c b/net/eth-uclass.c > index a32961e..a9fc6bb 100644 > --- a/net/eth-uclass.c > +++ b/net/eth-uclass.c > @@ -230,7 +230,7 @@ static int on_ethaddr(const char *name, const char *value, enum env_op op, > eth_write_hwaddr(dev); > break; > case env_op_delete: > - memset(pdata->enetaddr, 0, 6); > + memset(pdata->enetaddr, 0, ARP_HLEN); > } > } > > @@ -458,7 +458,7 @@ static int eth_post_probe(struct udevice *dev) > { > struct eth_device_priv *priv = dev->uclass_priv; > struct eth_pdata *pdata = dev->platdata; > - unsigned char env_enetaddr[6]; > + unsigned char env_enetaddr[ARP_HLEN]; > > #if defined(CONFIG_NEEDS_MANUAL_RELOC) > struct eth_ops *ops = eth_get_ops(dev); > @@ -497,7 +497,7 @@ static int eth_post_probe(struct udevice *dev) > eth_getenv_enetaddr_by_index("eth", dev->seq, env_enetaddr); > if (!is_zero_ethaddr(env_enetaddr)) { > if (!is_zero_ethaddr(pdata->enetaddr) && > - memcmp(pdata->enetaddr, env_enetaddr, 6)) { > + memcmp(pdata->enetaddr, env_enetaddr, ARP_HLEN)) { > printf("\nWarning: %s MAC addresses don't match:\n", > dev->name); > printf("Address in SROM is %pM\n", > @@ -507,7 +507,7 @@ static int eth_post_probe(struct udevice *dev) > } > > /* Override the ROM MAC address */ > - memcpy(pdata->enetaddr, env_enetaddr, 6); > + memcpy(pdata->enetaddr, env_enetaddr, ARP_HLEN); > } else if (is_valid_ethaddr(pdata->enetaddr)) { > eth_setenv_enetaddr_by_index("eth", dev->seq, pdata->enetaddr); > printf("\nWarning: %s using MAC address from ROM\n", > @@ -534,7 +534,7 @@ static int eth_pre_remove(struct udevice *dev) > eth_get_ops(dev)->stop(dev); > > /* clear the MAC address */ > - memset(pdata->enetaddr, 0, 6); > + memset(pdata->enetaddr, 0, ARP_HLEN); > > return 0; > } > diff --git a/net/eth_legacy.c b/net/eth_legacy.c > index d6d7cee..2b2c2de 100644 > --- a/net/eth_legacy.c > +++ b/net/eth_legacy.c > @@ -120,7 +120,7 @@ static int on_ethaddr(const char *name, const char *value, enum env_op op, > eth_parse_enetaddr(value, dev->enetaddr); > break; > case env_op_delete: > - memset(dev->enetaddr, 0, 6); > + memset(dev->enetaddr, 0, ARP_HLEN); > } > } > dev = dev->next; > @@ -133,14 +133,14 @@ U_BOOT_ENV_CALLBACK(ethaddr, on_ethaddr); > int eth_write_hwaddr(struct eth_device *dev, const char *base_name, > int eth_number) > { > - unsigned char env_enetaddr[6]; > + unsigned char env_enetaddr[ARP_HLEN]; > int ret = 0; > > eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr); > > if (!is_zero_ethaddr(env_enetaddr)) { > if (!is_zero_ethaddr(dev->enetaddr) && > - memcmp(dev->enetaddr, env_enetaddr, 6)) { > + memcmp(dev->enetaddr, env_enetaddr, ARP_HLEN)) { > printf("\nWarning: %s MAC addresses don't match:\n", > dev->name); > printf("Address in SROM is %pM\n", > @@ -149,7 +149,7 @@ int eth_write_hwaddr(struct eth_device *dev, const char *base_name, > env_enetaddr); > } > > - memcpy(dev->enetaddr, env_enetaddr, 6); > + memcpy(dev->enetaddr, env_enetaddr, ARP_HLEN); > } else if (is_valid_ethaddr(dev->enetaddr)) { > eth_setenv_enetaddr_by_index(base_name, eth_number, > dev->enetaddr); > @@ -298,7 +298,7 @@ int eth_initialize(void) > */ > int eth_mcast_join(struct in_addr mcast_ip, int join) > { > - u8 mcast_mac[6]; > + u8 mcast_mac[ARP_HLEN]; > if (!eth_current || !eth_current->mcast) > return -1; > mcast_mac[5] = htonl(mcast_ip.s_addr) & 0xff; diff --git a/net/eth_common.c b/net/eth_common.c index e0d8b62..57ef821 100644 --- a/net/eth_common.c +++ b/net/eth_common.c @@ -51,7 +51,7 @@ void eth_parse_enetaddr(const char *addr, uchar *enetaddr) char *end; int i; - for (i = 0; i < 6; ++i) { + for (i = 0; i < ARP_HLEN; ++i) { enetaddr[i] = addr ? simple_strtoul(addr, &end, 16) : 0; if (addr) addr = (*end) ? end + 1 : end;