Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi Codrin, On Fri, Jul 24, 2015 at 8:55 AM, Codrin Ciubotariu wrote: > The code from common/env_flags.c that checks if a > string has the format of a MAC address has been moved > in net/eth.c as a separate function called > eth_validate_ethaddr_str(). > > Signed-off-by: Codrin Ciubotariu > --- > > Changes for v3: > - none, new patch; > > common/env_flags.c | 15 ++- > include/net.h | 1 + > net/eth.c | 30 ++ > 3 files changed, 33 insertions(+), 13 deletions(-) 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 v3 11/16] net/eth.c: Add function to validate a MAC address
On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote: > The code from common/env_flags.c that checks if a > string has the format of a MAC address has been moved > in net/eth.c as a separate function called > eth_validate_ethaddr_str(). > > Signed-off-by: Codrin Ciubotariu > --- > > Changes for v3: > - none, new patch; > > common/env_flags.c | 15 ++- > include/net.h | 1 + > net/eth.c | 30 ++ > 3 files changed, 33 insertions(+), 13 deletions(-) > > diff --git a/common/env_flags.c b/common/env_flags.c > index 5189f5b..3e39fd1 100644 > --- a/common/env_flags.c > +++ b/common/env_flags.c > @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value, > } > break; > case env_flags_vartype_macaddr: > - cur = value; > - for (i = 0; i < 6; i++) { > - skip_num(1, cur, &end, 2); > - if (cur == end) > - return -1; > - if (cur + 2 == end && is_hex_prefix(cur)) > - return -1; > - if (i != 5 && *end != ':') > - return -1; > - if (i == 5 && *end != '\0') > - return -1; > - cur = end + 1; > - } > + if (eth_validate_ethaddr_str(value)) > + return -1; > break; > #endif > case env_flags_vartype_end: > diff --git a/include/net.h b/include/net.h > index d17173d..c487aa7 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -218,6 +218,7 @@ void eth_try_another(int first_restart); /* Change the > device */ > void eth_set_current(void); /* set nterface to ethcur var */ > > int eth_get_dev_index(void); /* get the device index */ > +int eth_validate_ethaddr_str(const char *addr); > void eth_parse_enetaddr(const char *addr, uchar *enetaddr); > int eth_getenv_enetaddr(char *name, uchar *enetaddr); > int eth_setenv_enetaddr(char *name, const uchar *enetaddr); > diff --git a/net/eth.c b/net/eth.c > index 953b731..a6fdf1b 100644 > --- a/net/eth.c > +++ b/net/eth.c > @@ -7,6 +7,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -19,6 +20,35 @@ > > DECLARE_GLOBAL_DATA_PTR; > > +int eth_validate_ethaddr_str(const char *addr) > +{ > + unsigned long val; > + int i; > + const char *cur; > + char *end; > + > + if (!addr) > + return -1; > + > + cur = addr; > + for (i = 0; i < 6; i++) { > + val = simple_strtoul(cur, &end, 16); > + if (cur + 1 != end && cur + 2 != end) > + return -1; > + if (val > 0xff) > + return -1; > + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x') > + return -1; > + if (i != 5 && *end != ':') > + return -1; > + if (i == 5 && *end != '\0') > + return -1; > + cur = end + 1; > + } > + > + return 0; > +} > + > void eth_parse_enetaddr(const char *addr, uchar *enetaddr) > { > char *end; > Codrin, This patch breaks most SPL targets. Please reconsider the location of eth_validate_ethaddr_str(). York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi York, I didn't know that SPL uses net/eth.c . Could you please suggest a place for eth_validate_ethaddr_str()? Thanks and best regards, Codrin > -Original Message- > From: Sun York-R58495 > Sent: Saturday, August 08, 2015 3:31 AM > To: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de > Cc: joe.hershber...@ni.com; Kushwaha Prabhakar-B32579 > Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC > address > > On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote: > > The code from common/env_flags.c that checks if a string has the > > format of a MAC address has been moved in net/eth.c as a separate > > function called eth_validate_ethaddr_str(). > > > > Signed-off-by: Codrin Ciubotariu > > --- > > > > Changes for v3: > > - none, new patch; > > > > common/env_flags.c | 15 ++- > > include/net.h | 1 + > > net/eth.c | 30 ++ > > 3 files changed, 33 insertions(+), 13 deletions(-) > > > > diff --git a/common/env_flags.c b/common/env_flags.c index > > 5189f5b..3e39fd1 100644 > > --- a/common/env_flags.c > > +++ b/common/env_flags.c > > @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value, > > } > > break; > > case env_flags_vartype_macaddr: > > - cur = value; > > - for (i = 0; i < 6; i++) { > > - skip_num(1, cur, &end, 2); > > - if (cur == end) > > - return -1; > > - if (cur + 2 == end && is_hex_prefix(cur)) > > - return -1; > > - if (i != 5 && *end != ':') > > - return -1; > > - if (i == 5 && *end != '\0') > > - return -1; > > - cur = end + 1; > > - } > > + if (eth_validate_ethaddr_str(value)) > > + return -1; > > break; > > #endif > > case env_flags_vartype_end: > > diff --git a/include/net.h b/include/net.h index d17173d..c487aa7 > > 100644 > > --- a/include/net.h > > +++ b/include/net.h > > @@ -218,6 +218,7 @@ void eth_try_another(int first_restart);/* > > Change the > device */ > > void eth_set_current(void);/* set nterface to ethcur var */ > > > > int eth_get_dev_index(void); /* get the device index */ > > +int eth_validate_ethaddr_str(const char *addr); > > void eth_parse_enetaddr(const char *addr, uchar *enetaddr); int > > eth_getenv_enetaddr(char *name, uchar *enetaddr); int > > eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git > > a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644 > > --- a/net/eth.c > > +++ b/net/eth.c > > @@ -7,6 +7,7 @@ > > */ > > > > #include > > +#include > > #include > > #include > > #include > > @@ -19,6 +20,35 @@ > > > > DECLARE_GLOBAL_DATA_PTR; > > > > +int eth_validate_ethaddr_str(const char *addr) { > > + unsigned long val; > > + int i; > > + const char *cur; > > + char *end; > > + > > + if (!addr) > > + return -1; > > + > > + cur = addr; > > + for (i = 0; i < 6; i++) { > > + val = simple_strtoul(cur, &end, 16); > > + if (cur + 1 != end && cur + 2 != end) > > + return -1; > > + if (val > 0xff) > > + return -1; > > + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x') > > + return -1; > > + if (i != 5 && *end != ':') > > + return -1; > > + if (i == 5 && *end != '\0') > > + return -1; > > + cur = end + 1; > > + } > > + > > + return 0; > > +} > > + > > void eth_parse_enetaddr(const char *addr, uchar *enetaddr) { > > char *end; > > > > Codrin, > > This patch breaks most SPL targets. Please reconsider the location of > eth_validate_ethaddr_str(). > > York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
SPL doesn't use net/eth.c. You add a call in env_flags.c. I think you can put it in header file and use static inline, or keep it in the same file where it is called. Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion. York On 08/10/2015 01:44 AM, Ciubotariu Codrin Constantin-B43658 wrote: > Hi York, > > I didn't know that SPL uses net/eth.c . Could you please suggest a place for > eth_validate_ethaddr_str()? > > Thanks and best regards, > Codrin > >> -Original Message- >> From: Sun York-R58495 >> Sent: Saturday, August 08, 2015 3:31 AM >> To: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de >> Cc: joe.hershber...@ni.com; Kushwaha Prabhakar-B32579 >> Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC >> address >> >> On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote: >>> The code from common/env_flags.c that checks if a string has the >>> format of a MAC address has been moved in net/eth.c as a separate >>> function called eth_validate_ethaddr_str(). >>> >>> Signed-off-by: Codrin Ciubotariu >>> --- >>> >>> Changes for v3: >>> - none, new patch; >>> >>> common/env_flags.c | 15 ++- >>> include/net.h | 1 + >>> net/eth.c | 30 ++ >>> 3 files changed, 33 insertions(+), 13 deletions(-) >>> >>> diff --git a/common/env_flags.c b/common/env_flags.c index >>> 5189f5b..3e39fd1 100644 >>> --- a/common/env_flags.c >>> +++ b/common/env_flags.c >>> @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value, >>> } >>> break; >>> case env_flags_vartype_macaddr: >>> - cur = value; >>> - for (i = 0; i < 6; i++) { >>> - skip_num(1, cur, &end, 2); >>> - if (cur == end) >>> - return -1; >>> - if (cur + 2 == end && is_hex_prefix(cur)) >>> - return -1; >>> - if (i != 5 && *end != ':') >>> - return -1; >>> - if (i == 5 && *end != '\0') >>> - return -1; >>> - cur = end + 1; >>> - } >>> + if (eth_validate_ethaddr_str(value)) >>> + return -1; >>> break; >>> #endif >>> case env_flags_vartype_end: >>> diff --git a/include/net.h b/include/net.h index d17173d..c487aa7 >>> 100644 >>> --- a/include/net.h >>> +++ b/include/net.h >>> @@ -218,6 +218,7 @@ void eth_try_another(int first_restart);/* >>> Change the >> device */ >>> void eth_set_current(void);/* set nterface to ethcur var */ >>> >>> int eth_get_dev_index(void); /* get the device index */ >>> +int eth_validate_ethaddr_str(const char *addr); >>> void eth_parse_enetaddr(const char *addr, uchar *enetaddr); int >>> eth_getenv_enetaddr(char *name, uchar *enetaddr); int >>> eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git >>> a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644 >>> --- a/net/eth.c >>> +++ b/net/eth.c >>> @@ -7,6 +7,7 @@ >>> */ >>> >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -19,6 +20,35 @@ >>> >>> DECLARE_GLOBAL_DATA_PTR; >>> >>> +int eth_validate_ethaddr_str(const char *addr) { >>> + unsigned long val; >>> + int i; >>> + const char *cur; >>> + char *end; >>> + >>> + if (!addr) >>> + return -1; >>> + >>> + cur = addr; >>> + for (i = 0; i < 6; i++) { >>> + val = simple_strtoul(cur, &end, 16); >>> + if (cur + 1 != end && cur + 2 != end) >>> + return -1; >>> + if (val > 0xff) >>> + return -1; >>> + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x') >>> + return -1; >>> + if (i != 5 && *end != ':') >>> + return -1; >>> + if (i == 5 && *end != '\0') >>> + return -1; >>> + cur = end + 1; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> void eth_parse_enetaddr(const char *addr, uchar *enetaddr) { >>> char *end; >>> >> >> Codrin, >> >> This patch breaks most SPL targets. Please reconsider the location of >> eth_validate_ethaddr_str(). >> >> York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Too much top-posting. On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: > SPL doesn't use net/eth.c. You add a call in env_flags.c. > > I think you can put it in header file and use static inline, or keep it in the > same file where it is called. That is probably fine. > Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in > Kconfig. Joe may have some good suggestion. I don't think this is the reason. The problem is that net is *not* build for SPL, but env is. > On 08/10/2015 01:44 AM, Ciubotariu Codrin Constantin-B43658 wrote: >> Hi York, >> >> I didn't know that SPL uses net/eth.c . Could you please suggest a place for >> eth_validate_ethaddr_str()? >> >> Thanks and best regards, >> Codrin >> >>> -Original Message- >>> From: Sun York-R58495 >>> Sent: Saturday, August 08, 2015 3:31 AM >>> To: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de >>> Cc: joe.hershber...@ni.com; Kushwaha Prabhakar-B32579 >>> Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC >>> address >>> >>> On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote: The code from common/env_flags.c that checks if a string has the format of a MAC address has been moved in net/eth.c as a separate function called eth_validate_ethaddr_str(). Signed-off-by: Codrin Ciubotariu --- Changes for v3: - none, new patch; common/env_flags.c | 15 ++- include/net.h | 1 + net/eth.c | 30 ++ 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/common/env_flags.c b/common/env_flags.c index 5189f5b..3e39fd1 100644 --- a/common/env_flags.c +++ b/common/env_flags.c @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char *value, } break; case env_flags_vartype_macaddr: - cur = value; - for (i = 0; i < 6; i++) { - skip_num(1, cur, &end, 2); - if (cur == end) - return -1; - if (cur + 2 == end && is_hex_prefix(cur)) - return -1; - if (i != 5 && *end != ':') - return -1; - if (i == 5 && *end != '\0') - return -1; - cur = end + 1; - } + if (eth_validate_ethaddr_str(value)) + return -1; break; #endif case env_flags_vartype_end: diff --git a/include/net.h b/include/net.h index d17173d..c487aa7 100644 --- a/include/net.h +++ b/include/net.h @@ -218,6 +218,7 @@ void eth_try_another(int first_restart);/* Change the >>> device */ void eth_set_current(void);/* set nterface to ethcur var */ int eth_get_dev_index(void); /* get the device index */ +int eth_validate_ethaddr_str(const char *addr); void eth_parse_enetaddr(const char *addr, uchar *enetaddr); int eth_getenv_enetaddr(char *name, uchar *enetaddr); int eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644 --- a/net/eth.c +++ b/net/eth.c @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -19,6 +20,35 @@ DECLARE_GLOBAL_DATA_PTR; +int eth_validate_ethaddr_str(const char *addr) { + unsigned long val; + int i; + const char *cur; + char *end; + + if (!addr) + return -1; + + cur = addr; + for (i = 0; i < 6; i++) { + val = simple_strtoul(cur, &end, 16); + if (cur + 1 != end && cur + 2 != end) + return -1; + if (val > 0xff) + return -1; + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x') + return -1; + if (i != 5 && *end != ':') + return -1; + if (i == 5 && *end != '\0') + return -1; + cur = end + 1; + } + + return 0; +} + void eth_parse_enetaddr(const char *addr, uchar *enetaddr) { char *end; >>> >>> Codrin, >>> >>> This patch breaks most SPL targets. Please reconsider the location of >>> eth_validate_ethaddr_str(). >>> >>> York > ___ > 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 v3 11/16] net/eth.c: Add function to validate a MAC address
On 08/10/2015 12:57 PM, Joe Hershberger wrote: > Too much top-posting. > > On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: >> SPL doesn't use net/eth.c. You add a call in env_flags.c. >> >> I think you can put it in header file and use static inline, or keep it in >> the >> same file where it is called. > > That is probably fine. > >> Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in >> Kconfig. Joe may have some good suggestion. > > I don't think this is the reason. The problem is that net is *not* > build for SPL, but env is. Yes, env is built. The offending lines in common/env_flags.c are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another way. York > >> On 08/10/2015 01:44 AM, Ciubotariu Codrin Constantin-B43658 wrote: >>> Hi York, >>> >>> I didn't know that SPL uses net/eth.c . Could you please suggest a place >>> for eth_validate_ethaddr_str()? >>> >>> Thanks and best regards, >>> Codrin >>> -Original Message- From: Sun York-R58495 Sent: Saturday, August 08, 2015 3:31 AM To: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de Cc: joe.hershber...@ni.com; Kushwaha Prabhakar-B32579 Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote: > The code from common/env_flags.c that checks if a string has the > format of a MAC address has been moved in net/eth.c as a separate > function called eth_validate_ethaddr_str(). > > Signed-off-by: Codrin Ciubotariu > --- > > Changes for v3: > - none, new patch; > > common/env_flags.c | 15 ++- > include/net.h | 1 + > net/eth.c | 30 ++ > 3 files changed, 33 insertions(+), 13 deletions(-) > > diff --git a/common/env_flags.c b/common/env_flags.c index > 5189f5b..3e39fd1 100644 > --- a/common/env_flags.c > +++ b/common/env_flags.c > @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char > *value, > } > break; > case env_flags_vartype_macaddr: > - cur = value; > - for (i = 0; i < 6; i++) { > - skip_num(1, cur, &end, 2); > - if (cur == end) > - return -1; > - if (cur + 2 == end && is_hex_prefix(cur)) > - return -1; > - if (i != 5 && *end != ':') > - return -1; > - if (i == 5 && *end != '\0') > - return -1; > - cur = end + 1; > - } > + if (eth_validate_ethaddr_str(value)) > + return -1; > break; > #endif > case env_flags_vartype_end: > diff --git a/include/net.h b/include/net.h index d17173d..c487aa7 > 100644 > --- a/include/net.h > +++ b/include/net.h > @@ -218,6 +218,7 @@ void eth_try_another(int first_restart);/* > Change the device */ > void eth_set_current(void);/* set nterface to ethcur var > */ > > int eth_get_dev_index(void); /* get the device index */ > +int eth_validate_ethaddr_str(const char *addr); > void eth_parse_enetaddr(const char *addr, uchar *enetaddr); int > eth_getenv_enetaddr(char *name, uchar *enetaddr); int > eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git > a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644 > --- a/net/eth.c > +++ b/net/eth.c > @@ -7,6 +7,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -19,6 +20,35 @@ > > DECLARE_GLOBAL_DATA_PTR; > > +int eth_validate_ethaddr_str(const char *addr) { > + unsigned long val; > + int i; > + const char *cur; > + char *end; > + > + if (!addr) > + return -1; > + > + cur = addr; > + for (i = 0; i < 6; i++) { > + val = simple_strtoul(cur, &end, 16); > + if (cur + 1 != end && cur + 2 != end) > + return -1; > + if (val > 0xff) > + return -1; > + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x') > + return -1; > + if (i != 5 && *end != ':') > + return -1; > + if (i == 5 && *end != '\0') > + return -1; > + cur = end + 1; > + } > + > + return 0; > +} > + > void eth_parse_enetaddr(const char *addr, uchar *enetaddr) { > char *end; > Codrin, This patch breaks most SPL targets. Please reconsider the location of eth_validat
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi York, On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: > > > On 08/10/2015 12:57 PM, Joe Hershberger wrote: >> Too much top-posting. >> >> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: >>> SPL doesn't use net/eth.c. You add a call in env_flags.c. >>> >>> I think you can put it in header file and use static inline, or keep it in >>> the >>> same file where it is called. >> >> That is probably fine. >> >>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in >>> Kconfig. Joe may have some good suggestion. >> >> I don't think this is the reason. The problem is that net is *not* >> build for SPL, but env is. > > Yes, env is built. The offending lines in common/env_flags.c are gated by > "#ifdef CONFIG_CMD_NET". That's why I say it could be another way. OK, sure... but that breaks intended behavior, I think. >> >>> On 08/10/2015 01:44 AM, Ciubotariu Codrin Constantin-B43658 wrote: Hi York, I didn't know that SPL uses net/eth.c . Could you please suggest a place for eth_validate_ethaddr_str()? Thanks and best regards, Codrin > -Original Message- > From: Sun York-R58495 > Sent: Saturday, August 08, 2015 3:31 AM > To: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de > Cc: joe.hershber...@ni.com; Kushwaha Prabhakar-B32579 > Subject: Re: [PATCH v3 11/16] net/eth.c: Add function to validate a MAC > address > > On 07/24/2015 06:55 AM, Codrin Ciubotariu wrote: >> The code from common/env_flags.c that checks if a string has the >> format of a MAC address has been moved in net/eth.c as a separate >> function called eth_validate_ethaddr_str(). >> >> Signed-off-by: Codrin Ciubotariu >> --- >> >> Changes for v3: >> - none, new patch; >> >> common/env_flags.c | 15 ++- >> include/net.h | 1 + >> net/eth.c | 30 ++ >> 3 files changed, 33 insertions(+), 13 deletions(-) >> >> diff --git a/common/env_flags.c b/common/env_flags.c index >> 5189f5b..3e39fd1 100644 >> --- a/common/env_flags.c >> +++ b/common/env_flags.c >> @@ -239,19 +239,8 @@ static int _env_flags_validate_type(const char >> *value, >> } >> break; >> case env_flags_vartype_macaddr: >> - cur = value; >> - for (i = 0; i < 6; i++) { >> - skip_num(1, cur, &end, 2); >> - if (cur == end) >> - return -1; >> - if (cur + 2 == end && is_hex_prefix(cur)) >> - return -1; >> - if (i != 5 && *end != ':') >> - return -1; >> - if (i == 5 && *end != '\0') >> - return -1; >> - cur = end + 1; >> - } >> + if (eth_validate_ethaddr_str(value)) >> + return -1; >> break; >> #endif >> case env_flags_vartype_end: >> diff --git a/include/net.h b/include/net.h index d17173d..c487aa7 >> 100644 >> --- a/include/net.h >> +++ b/include/net.h >> @@ -218,6 +218,7 @@ void eth_try_another(int first_restart);/* >> Change the > device */ >> void eth_set_current(void);/* set nterface to ethcur >> var */ >> >> int eth_get_dev_index(void); /* get the device index */ >> +int eth_validate_ethaddr_str(const char *addr); >> void eth_parse_enetaddr(const char *addr, uchar *enetaddr); int >> eth_getenv_enetaddr(char *name, uchar *enetaddr); int >> eth_setenv_enetaddr(char *name, const uchar *enetaddr); diff --git >> a/net/eth.c b/net/eth.c index 953b731..a6fdf1b 100644 >> --- a/net/eth.c >> +++ b/net/eth.c >> @@ -7,6 +7,7 @@ >> */ >> >> #include >> +#include >> #include >> #include >> #include >> @@ -19,6 +20,35 @@ >> >> DECLARE_GLOBAL_DATA_PTR; >> >> +int eth_validate_ethaddr_str(const char *addr) { >> + unsigned long val; >> + int i; >> + const char *cur; >> + char *end; >> + >> + if (!addr) >> + return -1; >> + >> + cur = addr; >> + for (i = 0; i < 6; i++) { >> + val = simple_strtoul(cur, &end, 16); >> + if (cur + 1 != end && cur + 2 != end) >> + return -1; >> + if (val > 0xff) >> + return -1; >> + if (cur + 2 >= end && tolower(*(cur + 1)) == 'x') >> + return -1; >> + if (i != 5 && *end != ':') >> + return -1; >> + if (i == 5 && *end != '\0') >> + return -1; >> + cur = end + 1; >> + } >> + >
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On 08/10/2015 01:05 PM, Joe Hershberger wrote: > Hi York, > > On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: >> >> >> On 08/10/2015 12:57 PM, Joe Hershberger wrote: >>> Too much top-posting. >>> >>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: SPL doesn't use net/eth.c. You add a call in env_flags.c. I think you can put it in header file and use static inline, or keep it in the same file where it is called. >>> >>> That is probably fine. >>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' in Kconfig. Joe may have some good suggestion. >>> >>> I don't think this is the reason. The problem is that net is *not* >>> build for SPL, but env is. >> >> Yes, env is built. The offending lines in common/env_flags.c are gated by >> "#ifdef CONFIG_CMD_NET". That's why I say it could be another way. > > OK, sure... but that breaks intended behavior, I think. > I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess the fix can be either to put the common function in header file after making it really simple to reduce dependency, or to keep the original code in env_flag.c. York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
+Codrin Somehow I dropped Codrin in last reply. On 08/10/2015 01:45 PM, York Sun wrote: > > > On 08/10/2015 01:05 PM, Joe Hershberger wrote: >> Hi York, >> >> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: >>> >>> >>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: Too much top-posting. On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: > SPL doesn't use net/eth.c. You add a call in env_flags.c. > > I think you can put it in header file and use static inline, or keep it > in the > same file where it is called. That is probably fine. > Another way is to undef CONFIG_CMD_NET for SPL part. It is default to 'y' > in > Kconfig. Joe may have some good suggestion. I don't think this is the reason. The problem is that net is *not* build for SPL, but env is. >>> >>> Yes, env is built. The offending lines in common/env_flags.c are gated by >>> "#ifdef CONFIG_CMD_NET". That's why I say it could be another way. >> >> OK, sure... but that breaks intended behavior, I think. >> > > I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So I guess > the fix can be either to put the common function in header file after making > it > really simple to reduce dependency, or to keep the original code in > env_flag.c. > Codrin, Can you prepare a new patch? You don't have to send the whole set. All but one have been acked by Joe. York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi York, Sure, I will make v4 this week for this change only. I will try to shrink the function and inline it in the header file. Thanks and best regards, Codrin > -Original Message- > From: Sun York-R58495 > Sent: Wednesday, August 12, 2015 10:59 PM > To: Ciubotariu Codrin Constantin-B43658 > Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com > Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a > MAC > address > > +Codrin > > Somehow I dropped Codrin in last reply. > > On 08/10/2015 01:45 PM, York Sun wrote: > > > > > > On 08/10/2015 01:05 PM, Joe Hershberger wrote: > >> Hi York, > >> > >> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: > >>> > >>> > >>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: > >>>> Too much top-posting. > >>>> > >>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: > >>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. > >>>>> > >>>>> I think you can put it in header file and use static inline, or > >>>>> keep it in the same file where it is called. > >>>> > >>>> That is probably fine. > >>>> > >>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default > >>>>> to 'y' in Kconfig. Joe may have some good suggestion. > >>>> > >>>> I don't think this is the reason. The problem is that net is *not* > >>>> build for SPL, but env is. > >>> > >>> Yes, env is built. The offending lines in common/env_flags.c are > >>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another > >>> way. > >> > >> OK, sure... but that breaks intended behavior, I think. > >> > > > > I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So > > I guess the fix can be either to put the common function in header > > file after making it really simple to reduce dependency, or to keep the > original code in env_flag.c. > > > > Codrin, > > Can you prepare a new patch? You don't have to send the whole set. All but one > have been acked by Joe. > > York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi Joe, York, > -Original Message- > From: Sun York-R58495 > Sent: Wednesday, August 12, 2015 10:59 PM > To: Ciubotariu Codrin Constantin-B43658 > Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com > Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a > MAC > address > > +Codrin > > Somehow I dropped Codrin in last reply. > > On 08/10/2015 01:45 PM, York Sun wrote: > > > > > > On 08/10/2015 01:05 PM, Joe Hershberger wrote: > >> Hi York, > >> > >> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: > >>> > >>> > >>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: > >>>> Too much top-posting. > >>>> > >>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: > >>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. > >>>>> > >>>>> I think you can put it in header file and use static inline, or > >>>>> keep it in the same file where it is called. > >>>> > >>>> That is probably fine. > >>>> > >>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default > >>>>> to 'y' in Kconfig. Joe may have some good suggestion. > >>>> > >>>> I don't think this is the reason. The problem is that net is *not* > >>>> build for SPL, but env is. > >>> > >>> Yes, env is built. The offending lines in common/env_flags.c are > >>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another > >>> way. > >> > >> OK, sure... but that breaks intended behavior, I think. > >> > > > > I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So > > I guess the fix can be either to put the common function in header > > file after making it really simple to reduce dependency, or to keep the > original code in env_flag.c. > > > > Codrin, > > Can you prepare a new patch? You don't have to send the whole set. All but one > have been acked by Joe. > > York I can't inline eth_validate_ethaddr_str in eth.h since it depends on simple_strtoul and tolower. Also, I can't let it in common/env_flags.c because I need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c file that is also used by SPL targets. Could you please suggest such a file? SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is built, but net/net.c or net/eth.c not. Thanks and best regards, Codrin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote: > Hi Joe, York, > >> -Original Message- >> From: Sun York-R58495 >> Sent: Wednesday, August 12, 2015 10:59 PM >> To: Ciubotariu Codrin Constantin-B43658 >> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a >> MAC >> address >> >> +Codrin >> >> Somehow I dropped Codrin in last reply. >> >> On 08/10/2015 01:45 PM, York Sun wrote: >>> >>> >>> On 08/10/2015 01:05 PM, Joe Hershberger wrote: >>>> Hi York, >>>> >>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: >>>>> >>>>> >>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: >>>>>> Too much top-posting. >>>>>> >>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun wrote: >>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. >>>>>>> >>>>>>> I think you can put it in header file and use static inline, or >>>>>>> keep it in the same file where it is called. >>>>>> >>>>>> That is probably fine. >>>>>> >>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is default >>>>>>> to 'y' in Kconfig. Joe may have some good suggestion. >>>>>> >>>>>> I don't think this is the reason. The problem is that net is *not* >>>>>> build for SPL, but env is. >>>>> >>>>> Yes, env is built. The offending lines in common/env_flags.c are >>>>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another >>>>> way. >>>> >>>> OK, sure... but that breaks intended behavior, I think. >>>> >>> >>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. So >>> I guess the fix can be either to put the common function in header >>> file after making it really simple to reduce dependency, or to keep the >> original code in env_flag.c. >>> >> >> Codrin, >> >> Can you prepare a new patch? You don't have to send the whole set. All but >> one >> have been acked by Joe. >> >> York > > I can't inline eth_validate_ethaddr_str in eth.h since it depends on > simple_strtoul and tolower. Also, I can't let it in common/env_flags.c > because I need to access if from drivers/net/vsc9953.c . I guess it has to be > in a .c file that is also used by SPL targets. Could you please suggest such > a file? > > SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is > built, but net/net.c or net/eth.c not. > I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET for SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If you can experiment it, you can try to gate the code in env_flags.c with CONFIG_SPL_BUILD. It sounds reasonable to me. York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
Hi York, > -Original Message- > From: Sun York-R58495 > Sent: Thursday, August 13, 2015 6:55 PM > To: Ciubotariu Codrin Constantin-B43658 > Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com > Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a > MAC > address > > > > On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote: > > Hi Joe, York, > > > >> -Original Message- > >> From: Sun York-R58495 > >> Sent: Wednesday, August 12, 2015 10:59 PM > >> To: Ciubotariu Codrin Constantin-B43658 > >> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com > >> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to > >> validate a MAC address > >> > >> +Codrin > >> > >> Somehow I dropped Codrin in last reply. > >> > >> On 08/10/2015 01:45 PM, York Sun wrote: > >>> > >>> > >>> On 08/10/2015 01:05 PM, Joe Hershberger wrote: > >>>> Hi York, > >>>> > >>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: > >>>>> > >>>>> > >>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: > >>>>>> Too much top-posting. > >>>>>> > >>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun > >>>>>> wrote: > >>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. > >>>>>>> > >>>>>>> I think you can put it in header file and use static inline, or > >>>>>>> keep it in the same file where it is called. > >>>>>> > >>>>>> That is probably fine. > >>>>>> > >>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is > >>>>>>> default to 'y' in Kconfig. Joe may have some good suggestion. > >>>>>> > >>>>>> I don't think this is the reason. The problem is that net is > >>>>>> *not* build for SPL, but env is. > >>>>> > >>>>> Yes, env is built. The offending lines in common/env_flags.c are > >>>>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another > way. > >>>> > >>>> OK, sure... but that breaks intended behavior, I think. > >>>> > >>> > >>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. > >>> So I guess the fix can be either to put the common function in > >>> header file after making it really simple to reduce dependency, or > >>> to keep the > >> original code in env_flag.c. > >>> > >> > >> Codrin, > >> > >> Can you prepare a new patch? You don't have to send the whole set. > >> All but one have been acked by Joe. > >> > >> York > > > > I can't inline eth_validate_ethaddr_str in eth.h since it depends on > simple_strtoul and tolower. Also, I can't let it in common/env_flags.c > because I > need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c > file > that is also used by SPL targets. Could you please suggest such a file? > > > > SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is > built, but net/net.c or net/eth.c not. > > > > I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET for > SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If > you can experiment it, you can try to gate the code in env_flags.c with > CONFIG_SPL_BUILD. It sounds reasonable to me. > > York Something like #if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD) case env_flags_vartype_ipaddr: cur = value; for (i = 0; i < 4; i++) { skip_num(0, cur, &end, 3); if (cur == end) return -1; if (i != 3 && *end != '.') return -1; if (i == 3 && *end != '\0') return -1; cur = end + 1; } break; case env_flags_vartype_macaddr: if (eth_validate_ethaddr_str(value)) return -1; break; #endif ? I get two warnings on this: ../common/env_flags.c: In function '_env_flags_validate_type': ../common/env_flags.c:203:2: warning: enumeration value 'env_flags_vartype_ipaddr' not handled in switch [-Wswitch] switch (type) { ^ ../common/env_flags.c:203:2: warning: enumeration value 'env_flags_vartype_macaddr' not handled in switch [-Wswitch] Unless I guard these values in env_flags.h: #if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD) env_flags_vartype_ipaddr, env_flags_vartype_macaddr, #endif Best regards, Codrin ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote: > Hi York, > >> -Original Message- >> From: Sun York-R58495 >> Sent: Thursday, August 13, 2015 6:55 PM >> To: Ciubotariu Codrin Constantin-B43658 >> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a >> MAC >> address >> >> >> >> On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote: >>> Hi Joe, York, >>> >>>> -Original Message- >>>> From: Sun York-R58495 >>>> Sent: Wednesday, August 12, 2015 10:59 PM >>>> To: Ciubotariu Codrin Constantin-B43658 >>>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >>>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to >>>> validate a MAC address >>>> >>>> +Codrin >>>> >>>> Somehow I dropped Codrin in last reply. >>>> >>>> On 08/10/2015 01:45 PM, York Sun wrote: >>>>> >>>>> >>>>> On 08/10/2015 01:05 PM, Joe Hershberger wrote: >>>>>> Hi York, >>>>>> >>>>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: >>>>>>> >>>>>>> >>>>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: >>>>>>>> Too much top-posting. >>>>>>>> >>>>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun >>>>>>>> wrote: >>>>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. >>>>>>>>> >>>>>>>>> I think you can put it in header file and use static inline, or >>>>>>>>> keep it in the same file where it is called. >>>>>>>> >>>>>>>> That is probably fine. >>>>>>>> >>>>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is >>>>>>>>> default to 'y' in Kconfig. Joe may have some good suggestion. >>>>>>>> >>>>>>>> I don't think this is the reason. The problem is that net is >>>>>>>> *not* build for SPL, but env is. >>>>>>> >>>>>>> Yes, env is built. The offending lines in common/env_flags.c are >>>>>>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another >> way. >>>>>> >>>>>> OK, sure... but that breaks intended behavior, I think. >>>>>> >>>>> >>>>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. >>>>> So I guess the fix can be either to put the common function in >>>>> header file after making it really simple to reduce dependency, or >>>>> to keep the >>>> original code in env_flag.c. >>>>> >>>> >>>> Codrin, >>>> >>>> Can you prepare a new patch? You don't have to send the whole set. >>>> All but one have been acked by Joe. >>>> >>>> York >>> >>> I can't inline eth_validate_ethaddr_str in eth.h since it depends on >> simple_strtoul and tolower. Also, I can't let it in common/env_flags.c >> because I >> need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c >> file >> that is also used by SPL targets. Could you please suggest such a file? >>> >>> SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is >> built, but net/net.c or net/eth.c not. >>> >> >> I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET >> for >> SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. If >> you can experiment it, you can try to gate the code in env_flags.c with >> CONFIG_SPL_BUILD. It sounds reasonable to me. >> >> York > > Something like > > #if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD) > case env_flags_vartype_ipaddr: > cur = value; > for (i = 0; i < 4; i++) { > skip_num(0, cur, &end, 3); > if (cur == end) > return -1; > if (i != 3 && *end != '.') > return -1; > if (i == 3 && *end != '\0') > return -1; > cur = end + 1; > } > break; > case env_flags_vartype_macaddr: > if (eth_validate_ethaddr_str(value)) > return -1; > break; > #endif > > ? > > I get two warnings on this: > ../common/env_flags.c: In function '_env_flags_validate_type': > ../common/env_flags.c:203:2: warning: enumeration value > 'env_flags_vartype_ipaddr' not handled in switch [-Wswitch] > switch (type) { > ^ > ../common/env_flags.c:203:2: warning: enumeration value > 'env_flags_vartype_macaddr' not handled in switch [-Wswitch] > > Unless I guard these values in env_flags.h: > #if defined(CONFIG_CMD_NET) && !defined(CONFIG_SPL_BUILD) > env_flags_vartype_ipaddr, > env_flags_vartype_macaddr, > #endif > It makes sense to me to take out these two for SPL build. The whole purpose of SPL is to load the final image. I don't see a chance users would be able to type any command. Joe, do you agree the CMD_NET shouldn't be used for SPL? York ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On Fri, Aug 14, 2015 at 12:59 PM, York Sun wrote: > > > On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote: >> Hi York, >> >>> -Original Message- >>> From: Sun York-R58495 >>> Sent: Thursday, August 13, 2015 6:55 PM >>> To: Ciubotariu Codrin Constantin-B43658 >>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate >>> a MAC >>> address >>> >>> >>> >>> On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote: >>>> Hi Joe, York, >>>> >>>>> -Original Message- >>>>> From: Sun York-R58495 >>>>> Sent: Wednesday, August 12, 2015 10:59 PM >>>>> To: Ciubotariu Codrin Constantin-B43658 >>>>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >>>>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to >>>>> validate a MAC address >>>>> >>>>> +Codrin >>>>> >>>>> Somehow I dropped Codrin in last reply. >>>>> >>>>> On 08/10/2015 01:45 PM, York Sun wrote: >>>>>> >>>>>> >>>>>> On 08/10/2015 01:05 PM, Joe Hershberger wrote: >>>>>>> Hi York, >>>>>>> >>>>>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: >>>>>>>>> Too much top-posting. >>>>>>>>> >>>>>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun >>>>>>>>> wrote: >>>>>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. >>>>>>>>>> >>>>>>>>>> I think you can put it in header file and use static inline, or >>>>>>>>>> keep it in the same file where it is called. >>>>>>>>> >>>>>>>>> That is probably fine. >>>>>>>>> >>>>>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is >>>>>>>>>> default to 'y' in Kconfig. Joe may have some good suggestion. >>>>>>>>> >>>>>>>>> I don't think this is the reason. The problem is that net is >>>>>>>>> *not* build for SPL, but env is. >>>>>>>> >>>>>>>> Yes, env is built. The offending lines in common/env_flags.c are >>>>>>>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another >>> way. >>>>>>> >>>>>>> OK, sure... but that breaks intended behavior, I think. >>>>>>> >>>>>> >>>>>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. >>>>>> So I guess the fix can be either to put the common function in >>>>>> header file after making it really simple to reduce dependency, or >>>>>> to keep the >>>>> original code in env_flag.c. >>>>>> >>>>> >>>>> Codrin, >>>>> >>>>> Can you prepare a new patch? You don't have to send the whole set. >>>>> All but one have been acked by Joe. >>>>> >>>>> York >>>> >>>> I can't inline eth_validate_ethaddr_str in eth.h since it depends on >>> simple_strtoul and tolower. Also, I can't let it in common/env_flags.c >>> because I >>> need to access if from drivers/net/vsc9953.c . I guess it has to be in a .c >>> file >>> that is also used by SPL targets. Could you please suggest such a file? >>>> >>>> SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is >>> built, but net/net.c or net/eth.c not. >>>> >>> >>> I was discussing with Joe about the possibility to deselect CONFIG_CMD_NET >>> for >>> SPL build. The issue here is Kconfig is not re-evaluated for the SPL part. >>> If >>> you can experiment it, you can try to gate the code in env_flags.c with >>> CONFIG_SPL_BUILD. It sounds reasonable to me. >>> >>> York >> >> Somethi
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
On 08/17/2015 07:37 AM, Joe Hershberger wrote: > On Fri, Aug 14, 2015 at 12:59 PM, York Sun wrote: >> >> >> On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote: >>> Hi York, >>> >>>> -Original Message- >>>> From: Sun York-R58495 >>>> Sent: Thursday, August 13, 2015 6:55 PM >>>> To: Ciubotariu Codrin Constantin-B43658 >>>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >>>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate >>>> a MAC >>>> address >>>> >>>> >>>> >>>> On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote: >>>>> Hi Joe, York, >>>>> >>>>>> -Original Message----- >>>>>> From: Sun York-R58495 >>>>>> Sent: Wednesday, August 12, 2015 10:59 PM >>>>>> To: Ciubotariu Codrin Constantin-B43658 >>>>>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com >>>>>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to >>>>>> validate a MAC address >>>>>> >>>>>> +Codrin >>>>>> >>>>>> Somehow I dropped Codrin in last reply. >>>>>> >>>>>> On 08/10/2015 01:45 PM, York Sun wrote: >>>>>>> >>>>>>> >>>>>>> On 08/10/2015 01:05 PM, Joe Hershberger wrote: >>>>>>>> Hi York, >>>>>>>> >>>>>>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: >>>>>>>>>> Too much top-posting. >>>>>>>>>> >>>>>>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun >>>>>>>>>> wrote: >>>>>>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. >>>>>>>>>>> >>>>>>>>>>> I think you can put it in header file and use static inline, or >>>>>>>>>>> keep it in the same file where it is called. >>>>>>>>>> >>>>>>>>>> That is probably fine. >>>>>>>>>> >>>>>>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is >>>>>>>>>>> default to 'y' in Kconfig. Joe may have some good suggestion. >>>>>>>>>> >>>>>>>>>> I don't think this is the reason. The problem is that net is >>>>>>>>>> *not* build for SPL, but env is. >>>>>>>>> >>>>>>>>> Yes, env is built. The offending lines in common/env_flags.c are >>>>>>>>> gated by "#ifdef CONFIG_CMD_NET". That's why I say it could be another >>>> way. >>>>>>>> >>>>>>>> OK, sure... but that breaks intended behavior, I think. >>>>>>>> >>>>>>> >>>>>>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. >>>>>>> So I guess the fix can be either to put the common function in >>>>>>> header file after making it really simple to reduce dependency, or >>>>>>> to keep the >>>>>> original code in env_flag.c. >>>>>>> >>>>>> >>>>>> Codrin, >>>>>> >>>>>> Can you prepare a new patch? You don't have to send the whole set. >>>>>> All but one have been acked by Joe. >>>>>> >>>>>> York >>>>> >>>>> I can't inline eth_validate_ethaddr_str in eth.h since it depends on >>>> simple_strtoul and tolower. Also, I can't let it in common/env_flags.c >>>> because I >>>> need to access if from drivers/net/vsc9953.c . I guess it has to be in a >>>> .c file >>>> that is also used by SPL targets. Could you please suggest such a file? >>>>> >>>>> SPL targets make use of CONFIG_CMD_NET? It seems strange that ETH env is >>>> built, but net/net.c or net/eth.c not
Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
> -Original Message- > From: Joe Hershberger [mailto:joe.hershber...@gmail.com] > Sent: Monday, August 17, 2015 5:38 PM > To: Sun York-R58495 > Cc: Ciubotariu Codrin Constantin-B43658; u-boot@lists.denx.de; > joe.hershber...@ni.com > Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to validate a > MAC > address > > On Fri, Aug 14, 2015 at 12:59 PM, York Sun wrote: > > > > > > On 08/14/2015 01:28 AM, Ciubotariu Codrin Constantin-B43658 wrote: > >> Hi York, > >> > >>> -Original Message- > >>> From: Sun York-R58495 > >>> Sent: Thursday, August 13, 2015 6:55 PM > >>> To: Ciubotariu Codrin Constantin-B43658 > >>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com > >>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to > >>> validate a MAC address > >>> > >>> > >>> > >>> On 08/13/2015 08:42 AM, Ciubotariu Codrin Constantin-B43658 wrote: > >>>> Hi Joe, York, > >>>> > >>>>> -----Original Message----- > >>>>> From: Sun York-R58495 > >>>>> Sent: Wednesday, August 12, 2015 10:59 PM > >>>>> To: Ciubotariu Codrin Constantin-B43658 > >>>>> Cc: Joe Hershberger; u-boot@lists.denx.de; joe.hershber...@ni.com > >>>>> Subject: Re: [U-Boot] [PATCH v3 11/16] net/eth.c: Add function to > >>>>> validate a MAC address > >>>>> > >>>>> +Codrin > >>>>> > >>>>> Somehow I dropped Codrin in last reply. > >>>>> > >>>>> On 08/10/2015 01:45 PM, York Sun wrote: > >>>>>> > >>>>>> > >>>>>> On 08/10/2015 01:05 PM, Joe Hershberger wrote: > >>>>>>> Hi York, > >>>>>>> > >>>>>>> On Mon, Aug 10, 2015 at 3:03 PM, York Sun > >>>>>>> wrote: > >>>>>>>> > >>>>>>>> > >>>>>>>> On 08/10/2015 12:57 PM, Joe Hershberger wrote: > >>>>>>>>> Too much top-posting. > >>>>>>>>> > >>>>>>>>> On Mon, Aug 10, 2015 at 2:41 PM, York Sun > wrote: > >>>>>>>>>> SPL doesn't use net/eth.c. You add a call in env_flags.c. > >>>>>>>>>> > >>>>>>>>>> I think you can put it in header file and use static inline, > >>>>>>>>>> or keep it in the same file where it is called. > >>>>>>>>> > >>>>>>>>> That is probably fine. > >>>>>>>>> > >>>>>>>>>> Another way is to undef CONFIG_CMD_NET for SPL part. It is > >>>>>>>>>> default to 'y' in Kconfig. Joe may have some good suggestion. > >>>>>>>>> > >>>>>>>>> I don't think this is the reason. The problem is that net is > >>>>>>>>> *not* build for SPL, but env is. > >>>>>>>> > >>>>>>>> Yes, env is built. The offending lines in common/env_flags.c > >>>>>>>> are gated by "#ifdef CONFIG_CMD_NET". That's why I say it could > >>>>>>>> be another > >>> way. > >>>>>>> > >>>>>>> OK, sure... but that breaks intended behavior, I think. > >>>>>>> > >>>>>> > >>>>>> I see. The CONFIG_CMD_NET is not evaluated separated for SPL build. > >>>>>> So I guess the fix can be either to put the common function in > >>>>>> header file after making it really simple to reduce dependency, > >>>>>> or to keep the > >>>>> original code in env_flag.c. > >>>>>> > >>>>> > >>>>> Codrin, > >>>>> > >>>>> Can you prepare a new patch? You don't have to send the whole set. > >>>>> All but one have been acked by Joe. > >>>>> > >>>>> York > >>>> > >>>> I can't inline eth_validate_ethaddr_str in eth.h since it depends > >>>> on > >>> simple_strtoul and tolower. Also, I can't let it in > >>&g