> -----Original Message----- > From: Wolfgang Denk [mailto:w...@denx.de] > Sent: Wednesday, August 19, 2009 12:50 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 > <1250679240-17557-1-git-send-email-prafu...@marvell.com> you wrote: > > I am sorry for previous post v2, pls ignore it, this is the > right patch for the same > > This comment does not belong to the commit message. Please move below > the "---" line. Okay I will add this in my next post
> > > This feature can be used to trigger special command > "sysrstcmd" using > > reset key long press event and environment variable > "sysrstdelay" is set > > (useful for reset to factory or manufacturing mode execution) > > > > Kirkwood SoC implements a hardware-based SYSRSTn duration counter. > > When SYSRSTn is asserted low, a SYSRSTn duration counter is running. > > The counter value is stored in the SYSRSTn Length Counter Register > > The counter is based on the 25-MHz reference clock (40ns) > > It is a 29-bit counter, yielding a maximum counting duration of > > 2^29/25 MHz (21.4 seconds). When the counter reach its > maximum value, > > it remains at this value until counter reset is triggered by setting > > bit 31 of KW_REG_SYSRST_CNT > > > > Implementation: > > Upon long reset assertion (> ${sysrstdleay} in secs) > sysrstcmd will be > > That's a typo, it's "sysrstdelay", right? Please fix while we are at > it. Thanks.. I will take care > > > +static void kw_sysrst_action(void) > > +{ > > + int ret; > > + char *s = getenv("sysrstcmd"); > > + > > + 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, 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 :-) > > > + } > > + > > + printf("Starting %s process...\n", __FUNCTION__); > > This should be a debug(), I think. Don't produce too much output. > > > + if (ret < 0) > > + printf("Error.. %s failed\n", __FUNCTION__); > > + else > > + printf("%s process finished\n", __FUNCTION__); > > Ditto - please turn into debug(). Okay no issues..I will do it > > > + > > +static void kw_sysrst_check(void) > > +{ > > + u32 sysrst_cnt, sysrst_dly; > > + char *s; > > + > > + /* > > + * no action if sysrstdelay environment variable is not defined > > + */ > > + s = getenv("sysrstdelay"); > > + if (s == NULL) > > + return; > > + > > + /* read sysrstdelay value */ > > + sysrst_dly = (u32) simple_strtoul(s, NULL, 10); > > + > > + /* read SysRst Length counter register (bits 28:0) */ > > + 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? It is just like "cpu name, speed etc". 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 Regards.. Prafulla . . > > > Thanks. > > 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 > If a person (a) is poorly, (b) receives treatment intended to make > him better, and (c) gets better, then no power of reasoning known to > medical science can convince him that it may not have been the > treatment that restored his health. > - Sir Peter Medawar, The Art of the Soluble > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot