cmd_date.c error or itention?

Hi,

cmd_data.c codes the following:

	case 2:			/* set date & time */
		if (strcmp(argv[1],"reset") == 0) {
			puts ("Reset RTC...\n");
			rtc_reset ();
		} else {
			/* initialize tm with current time */
			rcode = rtc_get (&tm);

			if(!rcode) {
				/* insert new date & time */
				if (mk_date (argv[1], &tm) != 0) {
					puts ("## Bad date format\n");
					break;
				}
				/* and write to RTC */
				rcode = rtc_set (&tm);
				if(rcode)
					puts("## Set date failed\n");
			} else {
				puts("## Get date failed\n");
			}
		}
		/* FALL TROUGH */

Now I have implemented rtc_get() such that it returns an error code when the time is corrupt/has never been set.

However the "if(!rcode)" then prevents the time to be set!

Is that intentional and rtc_get() should never return an error ?? Or is it an oversight, and a patch would be welcome ?

Reinhard

Dear "Reinhard Meyer (-VC)",

...

...

It is considered kind of "normal" that a RTC may return "corrupt" data. This is not an error, as it will be fixed when setting the date. An error condition is something that really makes the RTC unusable, like non-functioning communication on the I2C bus (if your RTC is connected to that), or reading a Low Voltage error status from the RTC status register, etc.

Well, if you cannot eliably communicate with the RTC when reading, there is little sense trying to write to it - on contrary, this might even be dangerous.

The behaviour is intentional, but rtc_get() can of course return error codes - only your expectation when this is the case is different from mine.

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: wd at denx.de
Misquotation is, in fact, the pride and privilege of the  learned.  A
widely-read  man  never  quotes  accurately,  for  the rather obvious
reason that he has read too widely.
                - Hesketh Pearson _Common Misquotations_ introduction

Dear Wolfgang,

of course that depends on what is considered an error.

As well as there is a difference between read error and file not found, there well might be a difference in clock nonfunctional and time invalid...

But alas, I will make rtc_get not return an error and zero out the tm structure instead when the driver KNOWS the date is not correct.

And btw. a low voltage status error from the clock does not necessaryly mean the clock cannot be set again, that status (and thats exactly what I intended to return) just could mean that the backup voltage was too low during a system unpowered time to guarantee a proper date but since (in our case) the backup power comes from a GoldCap a new set of the clock would heal that status.

And if the clock really would be defective
/* and write to RTC */ rcode = rtc_set (&tm); if(rcode)
puts("## Set date failed\n");
would still give a proper error message!

Reinhard
-------------- next part --------------
A non-text attachment was scrubbed...
Name: reinhard_meyer.vcf
Type: text/x-vcard
Size: 370 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100705/fc2c372e/attachment.vcf

Dear Reinhard Meyer,

Indeed.

Why would you do that? This prevents anybody trying to track down problems from seeing what is really going on. When you retrun the real (incorrect) data, I can see if the attempt to set the date shows any affect at all - with your method I don't see anything at all.

Did you read my argumentation why I rejected anatolij's patch to "fix" unaligned bus accesses on the 5200/512x in the "md" command? That's the same here.

Do not hush up errors. Let the user see what is really going on.

If I can see a bogus date but repeated calls show increments in the seconds register this is much, much more useful than seing zero values.

This may be the case with your RTC and on your board. Other hardware may behave differently. I just tried to come up with an example.

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: wd at denx.de
If I can have honesty, it's easier to overlook mistakes.
	-- Kirk, "Space Seed", stardate 3141.9

So far no AT91SAM9xxx board has a date command in u-boot. The kernel as it is will not set the system time when the offset register is zero. If the register is non-zero the time will be used.

I'm just trying to have the same behaviour in u-boot.

Besides your argumentation is flawed: why try to READ the clock when I am going to set it anyway? This reading and the following if just increase the code size :)

Now setting the clock still gives the warning (puts() in the driver) that the time is invalid (because the time is read before overwritten).

I think that is more irritating. So your suggestions is then to have the driver not say anything at all, just return OK and a bogus value...

Reinhard
-------------- next part --------------
A non-text attachment was scrubbed...
Name: reinhard_meyer.vcf
Type: text/x-vcard
Size: 370 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100705/613c82f4/attachment.vcf