> -----Original Message----- > From: Wolfgang Denk [mailto:w...@denx.de] > Sent: Thursday, August 20, 2009 3:08 PM > To: Prafulla Wadaskar > Cc: u-boot@lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik; > Ronen Shitrit > Subject: Re: [U-Boot] [PATCH v2][repost] arm: Kirkwood: add > SYSRSTn Duration Counter Support > > Dear Prafulla Wadaskar, > > In message > <73173d32e9439e4abb5151606c3e19e202e3915...@sc-vexch1.marvell. > com> you wrote: > > > > > > + if (!s) { > > > > + printf("Error.. %s failed, check sysrstcmd\n", > > > > + __FUNCTION__); > > > > + return; > > > > > > Why is this considered an error? I think it is perfectly > legal to not > > > define this environment variable. For example, it is also > no error to > > > set "bootdelay" and not define "bootcmd". I think we > should implement > > > consistent behaviour. > > It is similar with one difference- sysrstcmd is > additionally gated with h/w trigger, > > Um... yes... agreed, but that's not actually so special. Consider for > example the use of "altbootcmd" in connection with the boot > count limit > feature, or the "failbootcmd" which gets run in case of critical POST > errors. None of these produce any such error messages. For consistency > I recommend to remove this message here, too. > > > Secondly it is not as known as bootcmd, so it is always > better to throw some error message. > > This save some of developer's time and email exchanges :-) > > Well, for developers it may be useful during test - but it should not > be present for regular users of the production version. Maybe you > change it into a debug() ? Agreed I will do this.
> > ... > > > > + sysrst_cnt = (0x1fffffff & readl(KW_REG_SYSRST_CNT)); > > > > + printf("H/w Rst hold time: %d.%d secs\n", > > > > + sysrst_cnt / SYSRST_CNT_1SEC_VAL, > > > > + sysrst_cnt % SYSRST_CNT_1SEC_VAL); > > > > > > This should be debvug(), too ? > > Does it harm if we keep this info? > > Well, yes, it does. It adds output, which makes the boot process more > noisy and addds to the boot time. And normally none of the end users > will actually ever look at this information. That's understood but only in case sysrstdelay is defined which is not default case :-) > > > It is just like "cpu name, speed etc". > > Well, this _is_ information which the end users regularly check and > pay attention to. > > > SysRST is a feature provided by h/w that we are supporting, > > It may help users who are willing to use this feature > > Any way it is gated by "sysrstdelay" > > So I think we must keep this print alive > > Really? What is the advantage for the enduser to know if he pressed > the button for 5.1 or 5.3 seconds? No, I mean it is useful in case of 4.9 or 5.1 :-) > > Please make it a debug(). Should I? even though by default it will not show up :-) Regards.. Prafulla . . > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de > "...this does not mean that some of us should not want, in a rather > dispassionate sort of way, to put a bullet through csh's head." > - Larry Wall in <1992aug6.221512.5...@netlabs.com> > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot