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