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 york...@freescale.com 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 york...@freescale.com
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 york...@freescale.com
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?
I tend to agree, but I have a vague recollection that there is at least one
SPL
that uses net. Sorry I don't
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 york...@freescale.com 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 york...@freescale.com 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 york...@freescale.com
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?
I tend to agree, but I have a vague recollection that there is at
least one SPL that uses net. Sorry I don't recall what board it was
that I'm thinking about.
-Joe
___
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/17/2015 07:37 AM, Joe Hershberger wrote:
On Fri, Aug 14, 2015 at 12:59 PM, York Sun york...@freescale.com 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 york...@freescale.com
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 york...@freescale.com
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?
I tend to agree, but I have a vague recollection that there is at
least one SPL that uses net. Sorry I don't recall what board it was
that I'm thinking about.
That's exactly what I was worry about. In this case, can we settle with adding
eth_validate_ethaddr_str() without changing the code in env_flag.c? It is a
little duplication but seems easy and clean.
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 york...@freescale.com 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 york...@freescale.com
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 york...@freescale.com 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 york...@freescale.com
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
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 york...@freescale.com 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 york...@freescale.com 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 york...@freescale.com 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 york...@freescale.com 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 york...@freescale.com 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 york...@freescale.com 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
+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 york...@freescale.com 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 york...@freescale.com 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
Too much top-posting.
On Mon, Aug 10, 2015 at 2:41 PM, York Sun york...@freescale.com 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 codrin.ciubota...@freescale.com
---
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 common.h
+#include linux/ctype.h
#include command.h
#include dm.h
#include environment.h
@@ -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 york...@freescale.com 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 codrin.ciubota...@freescale.com
---
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 common.h
+#include linux/ctype.h
#include command.h
#include dm.h
#include environment.h
@@ -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
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 codrin.ciubota...@freescale.com
---
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 common.h
+#include linux/ctype.h
#include command.h
#include dm.h
#include environment.h
@@ -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,
On Mon, Aug 10, 2015 at 3:03 PM, York Sun york...@freescale.com 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 york...@freescale.com 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 codrin.ciubota...@freescale.com
---
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 common.h
+#include linux/ctype.h
#include command.h
#include dm.h
#include environment.h
@@ -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 01:05 PM, Joe Hershberger wrote: Hi York, On Mon, Aug 10, 2015 at 3:03 PM, York Sun york...@freescale.com 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 york...@freescale.com 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
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 codrin.ciubota...@freescale.com
---
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 common.h
+#include linux/ctype.h
#include command.h
#include dm.h
#include environment.h
@@ -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 Codrin, On Fri, Jul 24, 2015 at 8:55 AM, Codrin Ciubotariu codrin.ciubota...@freescale.com 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 codrin.ciubota...@freescale.com --- 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 joe.hershber...@ni.com ___ 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 codrin.ciubota...@freescale.com
---
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 common.h
+#include linux/ctype.h
#include command.h
#include dm.h
#include environment.h
@@ -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] [PATCH v3 11/16] net/eth.c: Add function to validate a MAC address
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 codrin.ciubota...@freescale.com
---
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 common.h
+#include linux/ctype.h
#include command.h
#include dm.h
#include environment.h
@@ -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;
--
1.9.3
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
