Patchwork [U-Boot] sntp: fix problems at setting RTC at multi-bus I2C systems

login
register
mail settings
Submitter Trübenbach, Ralf
Date May 2, 2011, 2:57 p.m.
Message ID <B578782695EC1545A6E9231C72F8ED2B09331A@MEN-EX2.intra.men.de>
Download mbox | patch
Permalink /patch/93672/
State Rejected
Headers show

Comments

Trübenbach, Ralf - May 2, 2011, 2:57 p.m.
Setting the RTC via SNTP did not work if the current I2C bus no. is
--
Best Regards/Mit freundlichen Gruessen
Ralf Trübenbach
------------------------------------------------------------------------
Ralf Trübenbach, Software Design
MEN Mikro Elektronik GmbH
Neuwieder Straße 5-7
90411 Nürnberg, Germany
Phone +49-911-99 33 5-0
Fax +49-911-99 33 5-910
Ralf.Truebenbach@men.de
www.men.de
MEN Mikro Elektronik GmbH - Manfred Schmitz (CTO), Udo Fuchs (CFO) 
- Handelsregister/Trade Register AG Nürnberg HRB 5540
Mike Frysinger - May 2, 2011, 7:01 p.m.
On Monday, May 02, 2011 10:57:11 Trübenbach, Ralf wrote:
> Setting the RTC via SNTP did not work if the current I2C bus no. is
> different from the bus no. the RTC is connected to.
> 
> Signed-off-by: Ralf Trübenbach <ralf.truebenbach@men.de>
> ---
> I don't like that I have to include the I2C things into sntp.c.
> But changing the rtc_set() function in 20 RTC drivers I don't
> like, too.

that would probably be the correct answer though.  not all rtc devices are 
i2c, and binding i2c logic to the `date` command is wrong.

looks like when Stefan added this logic to cmd_date.c, he did it incorrectly
-mike
Wolfgang Denk - May 2, 2011, 7:08 p.m.
Dear =?iso-8859-1?Q?Tr=FCbenbach=2C_Ralf?=,

In message <B578782695EC1545A6E9231C72F8ED2B09331A@MEN-EX2.intra.men.de> you wrote:
> Setting the RTC via SNTP did not work if the current I2C bus no. is
> different from the bus no. the RTC is connected to.
> 
> Signed-off-by: Ralf Tr=FCbenbach <ralf.truebenbach@men.de>
> ---
> I don't like that I have to include the I2C things into sntp.c.

Right, this is totally the wrong place.

> But changing the rtc_set() function in 20 RTC drivers I don't 
> like, too.

But that's where such code belongs - probably not into all of them,
but into a single central locatioon.

> index 76c10ec..7eee0db 100644
> --- a/net/sntp.c
> +++ b/net/sntp.c
> @@ -9,6 +9,9 @@
>  #include <command.h>
>  #include <net.h>
>  #include <rtc.h>
> +#if defined(CONFIG_CMD_DATE)
> +#include <i2c.h>
> +#endif

This also makes little sense - what if the RTC is not I2C, but bus
attached?

> -	 * As the RTC's used in U-Boot sepport second resolution only
> +	 * As the RTC's used in U-Boot support second resolution only

Unrelated changes must be submitted separately.

>  #if defined(CONFIG_CMD_DATE)
> +	/* switch to correct I2C bus */
> +	old_bus = I2C_GET_BUS();
> +	I2C_SET_BUS(CONFIG_SYS_RTC_BUS_NUM);
> +
>  	rtc_set (&tm);
> +
> +	/* switch back to original I2C bus */
> +	I2C_SET_BUS(old_bus);
>  #endif

Totally wrong: wrong place, wrong dependeny.

NAK.

Best regards,

Wolfgang Denk

Patch

different from the bus no. the RTC is connected to.

Signed-off-by: Ralf Trübenbach <ralf.truebenbach@men.de>
---
I don't like that I have to include the I2C things into sntp.c. 
But changing the rtc_set() function in 20 RTC drivers I don't 
like, too.

diff --git a/net/sntp.c b/net/sntp.c
index 76c10ec..7eee0db 100644
--- a/net/sntp.c
+++ b/net/sntp.c
@@ -9,6 +9,9 @@ 
 #include <command.h>
 #include <net.h>
 #include <rtc.h>
+#if defined(CONFIG_CMD_DATE)
+#include <i2c.h>
+#endif
 
 #include "sntp.h"
 
@@ -53,20 +56,30 @@ 
 	struct sntp_pkt_t *rpktp = (struct sntp_pkt_t *)pkt;
 	struct rtc_time tm;
 	ulong seconds;
+#if defined(CONFIG_CMD_DATE)
+	int old_bus;
+#endif
 
 	debug("%s\n", __func__);
 
 	if (dest != SntpOurPort) return;
 
 	/*
-	 * As the RTC's used in U-Boot sepport second resolution only
+	 * As the RTC's used in U-Boot support second resolution only
 	 * we simply ignore the sub-second field.
 	 */
 	memcpy (&seconds, &rpktp->transmit_timestamp, sizeof(ulong));
 
 	to_tm(ntohl(seconds) - 2208988800UL + NetTimeOffset, &tm);
 #if defined(CONFIG_CMD_DATE)
+	/* switch to correct I2C bus */
+	old_bus = I2C_GET_BUS();
+	I2C_SET_BUS(CONFIG_SYS_RTC_BUS_NUM);
+
 	rtc_set (&tm);
+
+	/* switch back to original I2C bus */
+	I2C_SET_BUS(old_bus);
 #endif
 	printf ("Date: %4d-%02d-%02d Time: %2d:%02d:%02d\n",
 		tm.tm_year, tm.tm_mon, tm.tm_mday,