Re: [U-Boot] [PATCH 4/8] net: Move environment functions to the common file
Hi Joe, On Sat, Jan 23, 2016 at 6:32 AM, Joe Hershberger wrote: > Hi Bin, > > On Sun, Jan 17, 2016 at 10:41 PM, Bin Meng wrote: >> Hi Simon, >> >> On Mon, Jan 18, 2016 at 5:51 AM, Simon Glass wrote: >>> Move the functions which set ethernet environment variables to the common >>> file. >>> >>> Signed-off-by: Simon Glass >>> --- >>> >>> net/eth.c | 43 --- >>> net/eth_common.c | 43 +++ >>> net/eth_internal.h | 16 >>> 3 files changed, 59 insertions(+), 43 deletions(-) >>> > > > >>> diff --git a/net/eth_internal.h b/net/eth_internal.h >>> index e65d898..38d8420 100644 >>> --- a/net/eth_internal.h >>> +++ b/net/eth_internal.h >>> @@ -12,4 +12,20 @@ >>> /* Do init that is common to driver model and legacy networking */ >>> void eth_common_init(void); >>> >>> +/** >>> + * eth_setenv_enetaddr_by_index() - set the MAC address envrionment >>> variable >>> + * >>> + * This sets up an environment variable with the given MAC address >>> (@enetaddr). >>> + * The environment variable to be set is defined by >>> <@base_name><@index>addr. >>> + * If @index is 0 it is omitted. For common Ethernet this means ethaddr, >>> + * eth1addr, etc. >>> + * >>> + * @base_name: Base name for variable, typically "eth" >>> + * @index: Index of interface being updated (>=0) >>> + * @enetaddr: Pointer to MAC address to put into the variable >>> + * @return 0 if OK, other value on error >>> + */ >>> +int eth_setenv_enetaddr_by_index(const char *base_name, int index, >>> +uchar *enetaddr); >>> + >>> #endif >>> -- >> >> Could you add some comments about the other routines >> (eth_parse_enetaddr, eth_getenv_enetaddr, eth_setenv_enetaddr, >> eth_getenv_enetaddr_by_index)? > > That would be great, but those are already defined in include/net.h > with poor / incomplete comments. I don't think fixing that *needs* to > be part of this patch, but it would be welcomed. > Simon added comments for eth_setenv_enetaddr_by_index() which did not have any comments before, so I'd assume we should add comments for others in the same patch. Otherwise it looks odd to me. Regards, Bin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/8] net: Move environment functions to the common file
Hi Bin, On Sun, Jan 17, 2016 at 10:41 PM, Bin Meng wrote: > Hi Simon, > > On Mon, Jan 18, 2016 at 5:51 AM, Simon Glass wrote: >> Move the functions which set ethernet environment variables to the common >> file. >> >> Signed-off-by: Simon Glass >> --- >> >> net/eth.c | 43 --- >> net/eth_common.c | 43 +++ >> net/eth_internal.h | 16 >> 3 files changed, 59 insertions(+), 43 deletions(-) >> >> diff --git a/net/eth_internal.h b/net/eth_internal.h >> index e65d898..38d8420 100644 >> --- a/net/eth_internal.h >> +++ b/net/eth_internal.h >> @@ -12,4 +12,20 @@ >> /* Do init that is common to driver model and legacy networking */ >> void eth_common_init(void); >> >> +/** >> + * eth_setenv_enetaddr_by_index() - set the MAC address envrionment variable >> + * >> + * This sets up an environment variable with the given MAC address >> (@enetaddr). >> + * The environment variable to be set is defined by >> <@base_name><@index>addr. >> + * If @index is 0 it is omitted. For common Ethernet this means ethaddr, >> + * eth1addr, etc. >> + * >> + * @base_name: Base name for variable, typically "eth" >> + * @index: Index of interface being updated (>=0) >> + * @enetaddr: Pointer to MAC address to put into the variable >> + * @return 0 if OK, other value on error >> + */ >> +int eth_setenv_enetaddr_by_index(const char *base_name, int index, >> +uchar *enetaddr); >> + >> #endif >> -- > > Could you add some comments about the other routines > (eth_parse_enetaddr, eth_getenv_enetaddr, eth_setenv_enetaddr, > eth_getenv_enetaddr_by_index)? That would be great, but those are already defined in include/net.h with poor / incomplete comments. I don't think fixing that *needs* to be part of this patch, but it would be welcomed. > > Regards, > Bin > ___ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/8] net: Move environment functions to the common file
On Sun, Jan 17, 2016 at 3:51 PM, Simon Glass wrote: > Move the functions which set ethernet environment variables to the common > file. > > Signed-off-by: Simon Glass Acked-by: Joe Hershberger ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/8] net: Move environment functions to the common file
Hi Simon,
On Mon, Jan 18, 2016 at 5:51 AM, Simon Glass wrote:
> Move the functions which set ethernet environment variables to the common
> file.
>
> Signed-off-by: Simon Glass
> ---
>
> net/eth.c | 43 ---
> net/eth_common.c | 43 +++
> net/eth_internal.h | 16
> 3 files changed, 59 insertions(+), 43 deletions(-)
>
> diff --git a/net/eth.c b/net/eth.c
> index 602925d..af8fcae 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -20,49 +20,6 @@
>
> DECLARE_GLOBAL_DATA_PTR;
>
> -void eth_parse_enetaddr(const char *addr, uchar *enetaddr)
> -{
> - char *end;
> - int i;
> -
> - for (i = 0; i < 6; ++i) {
> - enetaddr[i] = addr ? simple_strtoul(addr, &end, 16) : 0;
> - if (addr)
> - addr = (*end) ? end + 1 : end;
> - }
> -}
> -
> -int eth_getenv_enetaddr(const char *name, uchar *enetaddr)
> -{
> - eth_parse_enetaddr(getenv(name), enetaddr);
> - return is_valid_ethaddr(enetaddr);
> -}
> -
> -int eth_setenv_enetaddr(const char *name, const uchar *enetaddr)
> -{
> - char buf[20];
> -
> - sprintf(buf, "%pM", enetaddr);
> -
> - return setenv(name, buf);
> -}
> -
> -int eth_getenv_enetaddr_by_index(const char *base_name, int index,
> -uchar *enetaddr)
> -{
> - char enetvar[32];
> - sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
> - return eth_getenv_enetaddr(enetvar, enetaddr);
> -}
> -
> -static inline int eth_setenv_enetaddr_by_index(const char *base_name, int
> index,
> -uchar *enetaddr)
> -{
> - char enetvar[32];
> - sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
> - return eth_setenv_enetaddr(enetvar, enetaddr);
> -}
> -
> static int eth_mac_skip(int index)
> {
> char enetvar[15];
> diff --git a/net/eth_common.c b/net/eth_common.c
> index ee0b6df..3fa6d83 100644
> --- a/net/eth_common.c
> +++ b/net/eth_common.c
> @@ -10,6 +10,49 @@
> #include
> #include "eth_internal.h"
>
> +void eth_parse_enetaddr(const char *addr, uchar *enetaddr)
> +{
> + char *end;
> + int i;
> +
> + for (i = 0; i < 6; ++i) {
> + enetaddr[i] = addr ? simple_strtoul(addr, &end, 16) : 0;
> + if (addr)
> + addr = (*end) ? end + 1 : end;
> + }
> +}
> +
> +int eth_getenv_enetaddr(const char *name, uchar *enetaddr)
> +{
> + eth_parse_enetaddr(getenv(name), enetaddr);
> + return is_valid_ethaddr(enetaddr);
> +}
> +
> +int eth_setenv_enetaddr(const char *name, const uchar *enetaddr)
> +{
> + char buf[20];
> +
> + sprintf(buf, "%pM", enetaddr);
> +
> + return setenv(name, buf);
> +}
> +
> +int eth_getenv_enetaddr_by_index(const char *base_name, int index,
> +uchar *enetaddr)
> +{
> + char enetvar[32];
> + sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
> + return eth_getenv_enetaddr(enetvar, enetaddr);
> +}
> +
> +int eth_setenv_enetaddr_by_index(const char *base_name, int index,
> +uchar *enetaddr)
> +{
> + char enetvar[32];
> + sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
> + return eth_setenv_enetaddr(enetvar, enetaddr);
> +}
> +
> void eth_common_init(void)
> {
> bootstage_mark(BOOTSTAGE_ID_NET_ETH_START);
> diff --git a/net/eth_internal.h b/net/eth_internal.h
> index e65d898..38d8420 100644
> --- a/net/eth_internal.h
> +++ b/net/eth_internal.h
> @@ -12,4 +12,20 @@
> /* Do init that is common to driver model and legacy networking */
> void eth_common_init(void);
>
> +/**
> + * eth_setenv_enetaddr_by_index() - set the MAC address envrionment variable
> + *
> + * This sets up an environment variable with the given MAC address
> (@enetaddr).
> + * The environment variable to be set is defined by <@base_name><@index>addr.
> + * If @index is 0 it is omitted. For common Ethernet this means ethaddr,
> + * eth1addr, etc.
> + *
> + * @base_name: Base name for variable, typically "eth"
> + * @index: Index of interface being updated (>=0)
> + * @enetaddr: Pointer to MAC address to put into the variable
> + * @return 0 if OK, other value on error
> + */
> +int eth_setenv_enetaddr_by_index(const char *base_name, int index,
> +uchar *enetaddr);
> +
> #endif
> --
Could you add some comments about the other routines
(eth_parse_enetaddr, eth_getenv_enetaddr, eth_setenv_enetaddr,
eth_getenv_enetaddr_by_index)?
Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 4/8] net: Move environment functions to the common file
Move the functions which set ethernet environment variables to the common
file.
Signed-off-by: Simon Glass
---
net/eth.c | 43 ---
net/eth_common.c | 43 +++
net/eth_internal.h | 16
3 files changed, 59 insertions(+), 43 deletions(-)
diff --git a/net/eth.c b/net/eth.c
index 602925d..af8fcae 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -20,49 +20,6 @@
DECLARE_GLOBAL_DATA_PTR;
-void eth_parse_enetaddr(const char *addr, uchar *enetaddr)
-{
- char *end;
- int i;
-
- for (i = 0; i < 6; ++i) {
- enetaddr[i] = addr ? simple_strtoul(addr, &end, 16) : 0;
- if (addr)
- addr = (*end) ? end + 1 : end;
- }
-}
-
-int eth_getenv_enetaddr(const char *name, uchar *enetaddr)
-{
- eth_parse_enetaddr(getenv(name), enetaddr);
- return is_valid_ethaddr(enetaddr);
-}
-
-int eth_setenv_enetaddr(const char *name, const uchar *enetaddr)
-{
- char buf[20];
-
- sprintf(buf, "%pM", enetaddr);
-
- return setenv(name, buf);
-}
-
-int eth_getenv_enetaddr_by_index(const char *base_name, int index,
-uchar *enetaddr)
-{
- char enetvar[32];
- sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
- return eth_getenv_enetaddr(enetvar, enetaddr);
-}
-
-static inline int eth_setenv_enetaddr_by_index(const char *base_name, int
index,
-uchar *enetaddr)
-{
- char enetvar[32];
- sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
- return eth_setenv_enetaddr(enetvar, enetaddr);
-}
-
static int eth_mac_skip(int index)
{
char enetvar[15];
diff --git a/net/eth_common.c b/net/eth_common.c
index ee0b6df..3fa6d83 100644
--- a/net/eth_common.c
+++ b/net/eth_common.c
@@ -10,6 +10,49 @@
#include
#include "eth_internal.h"
+void eth_parse_enetaddr(const char *addr, uchar *enetaddr)
+{
+ char *end;
+ int i;
+
+ for (i = 0; i < 6; ++i) {
+ enetaddr[i] = addr ? simple_strtoul(addr, &end, 16) : 0;
+ if (addr)
+ addr = (*end) ? end + 1 : end;
+ }
+}
+
+int eth_getenv_enetaddr(const char *name, uchar *enetaddr)
+{
+ eth_parse_enetaddr(getenv(name), enetaddr);
+ return is_valid_ethaddr(enetaddr);
+}
+
+int eth_setenv_enetaddr(const char *name, const uchar *enetaddr)
+{
+ char buf[20];
+
+ sprintf(buf, "%pM", enetaddr);
+
+ return setenv(name, buf);
+}
+
+int eth_getenv_enetaddr_by_index(const char *base_name, int index,
+uchar *enetaddr)
+{
+ char enetvar[32];
+ sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
+ return eth_getenv_enetaddr(enetvar, enetaddr);
+}
+
+int eth_setenv_enetaddr_by_index(const char *base_name, int index,
+uchar *enetaddr)
+{
+ char enetvar[32];
+ sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
+ return eth_setenv_enetaddr(enetvar, enetaddr);
+}
+
void eth_common_init(void)
{
bootstage_mark(BOOTSTAGE_ID_NET_ETH_START);
diff --git a/net/eth_internal.h b/net/eth_internal.h
index e65d898..38d8420 100644
--- a/net/eth_internal.h
+++ b/net/eth_internal.h
@@ -12,4 +12,20 @@
/* Do init that is common to driver model and legacy networking */
void eth_common_init(void);
+/**
+ * eth_setenv_enetaddr_by_index() - set the MAC address envrionment variable
+ *
+ * This sets up an environment variable with the given MAC address (@enetaddr).
+ * The environment variable to be set is defined by <@base_name><@index>addr.
+ * If @index is 0 it is omitted. For common Ethernet this means ethaddr,
+ * eth1addr, etc.
+ *
+ * @base_name: Base name for variable, typically "eth"
+ * @index: Index of interface being updated (>=0)
+ * @enetaddr: Pointer to MAC address to put into the variable
+ * @return 0 if OK, other value on error
+ */
+int eth_setenv_enetaddr_by_index(const char *base_name, int index,
+uchar *enetaddr);
+
#endif
--
2.6.0.rc2.230.g3dd15c0
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
