[U-Boot] [PATCH v2] arm: Kirkwood: add SYSRSTn Duration Counter Support

[Prafulla Wadaskar]

> -----Original Message-----
> From: Jean-Christophe PLAGNIOL-VILLARD [mailto:plagnioj at jcrosoft.com] 
> Sent: Tuesday, August 18, 2009 4:24 AM
> To: Wolfgang Denk
> Cc: Prafulla Wadaskar; u-boot at lists.denx.de; Ashish Karkare; 
> Prabhanjan Sarnaik
> Subject: Re: [U-Boot] [PATCH v2] arm: Kirkwood: add SYSRSTn 
> Duration Counter Support
> 
> On 00:36 Tue 18 Aug     , Wolfgang Denk wrote:
> > Dear Jean-Christophe PLAGNIOL-VILLARD,
> > 
> > In message <20090817221152.GM23695 at game.jcrosoft.org> you wrote:
> > >
> > > > +	printf("Starting %s process...\n", __FUNCTION__);
> > > > +	sprintf(cmd, "run ");
> > > > +	sprintf(img, "sysrstcmd");
> > > > +	argv[0] = cmd;
> > > > +	argv[1] = img;
> > > > +	if ((do_run(NULL, 0, 2, argv)) != 0x0) {
> > > > +		printf("Error.. %s failed\n", __FUNCTION__);
> > > > +	} else {
> > > > +		printf("%s process finished\n", __FUNCTION__);
> > > > +	}
> > > 
> > > > +#else	/* CONFIG_CMD_RUN */
I think better approach will be report it at build time
i.e. #error "Error.. CONFIG_CMD_RUN needed for sysrstcmd support"

> > > > +	printf("Error.. %s needs run command 
> support\n", __FUNCTION__);
> > > > +#endif	/* CONFIG_CMD_RUN */
> > > why not replace this by
> > > 
> > > 	char *s = getenv("sysrstcmd");
> > > 
> > > 	if (!s) {
> > > 		printf("Error.. %s failed, check sysrstcmd\n",
> > > 			__FUNCTION__);
> > > 		return;
> > > 	}
> > > 
> > > 	printf("Starting %s process...\n", __FUNCTION__);
> > > #if !defined(CONFIG_SYS_HUSH_PARSER)
> > > 	ret = run_command (s, 0);
> > > #else
> > > 	ret = parse_string_outer(s, FLAG_PARSE_SEMICOLON
> > > 				  | FLAG_EXIT_FROM_LOOP);
I had checked this before,
this is already done in the context of do_run
to avoid code duplication I have used do_run.


> > > #endif
> > 
> > Maybe because the original code does not need an #ifdef ?
> but the size is bigger on the current implementation
> and as we have this is other part of U-Boot we can add a 
> inline to have the
> ifdef at only one place
As suggested above, we can report it at build time and reduce some on binary size too.

Regards..
Prafulla . .

> 
> Best Regards,
> J.
>