[U-Boot] [PATCH] I2C: Add support for multiple I2C busses for SNTP (effect to RTC)

[U-Boot] [PATCH] I2C: Add support for multiple I2C busses for SNTP (effect to RTC)

[Stephan Linz]

This patch switches to the desired I2C bus when the SNTP
network service is called. This can be configured using the
CONFIG_SYS_RTC_BUS_NUM define.

In my eyes this is a bad and quick hack, but the same as
was making for the date and dtt commands (commit: 0dc018e).
The right way would be to move all the hardware specific i2c
code down to the rtc driver layer.

Signed-off-by: Stephan Linz <linz@li-pro.net>
---
 net/sntp.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/net/sntp.c b/net/sntp.c
index 76c10ec..a4399a8 100644
--- a/net/sntp.c
+++ b/net/sntp.c
@@ -10,6 +10,12 @@
 #include <net.h>
 #include <rtc.h>
 
+#if defined(CONFIG_CMD_DATE)
+/* FIXME: The i2c dependency should move into the RTC drivers itself. This higher
+ * network layer must not know about hardware specifics!!!!!!! */
+#include <i2c.h>
+#endif
+
 #include "sntp.h"
 
 #define SNTP_TIMEOUT 10000UL
@@ -53,6 +59,9 @@ SntpHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
 	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__);
 
@@ -66,7 +75,14 @@ SntpHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
 
 	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,
-- 
1.6.3.3

[U-Boot] [PATCH] I2C: Add support for multiple I2C busses for SNTP (effect to RTC)

[Wolfgang Denk]

Dear Stephan Linz,

In message <1282310377-9302-1-git-send-email-linz@li-pro.net> you wrote:
> This patch switches to the desired I2C bus when the SNTP
> network service is called. This can be configured using the
> CONFIG_SYS_RTC_BUS_NUM define.
...
> +#if defined(CONFIG_CMD_DATE)
> +/* FIXME: The i2c dependency should move into the RTC drivers itself. This higher
> + * network layer must not know about hardware specifics!!!!!!! */

Incorrect multiline comment style.  And line too long.


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
Extreme feminine beauty is always disturbing.
	-- Spock, "The Cloud Minders", stardate 5818.4

[U-Boot] [PATCH] I2C: Add support for multiple I2C busses for SNTP (effect to RTC)

[Stephan Linz]

This patch switches to the desired I2C bus when the SNTP
network service is called. This can be configured using the
CONFIG_SYS_RTC_BUS_NUM define.

In my eyes this is a bad and quick hack, but the same as
was making for the date and dtt commands (commit: 0dc018e).
The right way would be to move all the hardware specific i2c
code down to the rtc driver layer.

Signed-off-by: Stephan Linz <linz@li-pro.net>
---
 net/sntp.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/net/sntp.c b/net/sntp.c
index 76c10ec..5544570 100644
--- a/net/sntp.c
+++ b/net/sntp.c
@@ -10,6 +10,14 @@
 #include <net.h>
 #include <rtc.h>
 
+/*
+ * FIXME: The i2c dependency should move into the RTC drivers itself.
+ * This higher network layer must not know about hardware specifics!
+ */
+#if defined(CONFIG_CMD_DATE)
+#include <i2c.h>
+#endif
+
 #include "sntp.h"
 
 #define SNTP_TIMEOUT 10000UL
@@ -53,6 +61,9 @@ SntpHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
 	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__);
 
@@ -66,7 +77,14 @@ SntpHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
 
 	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,
-- 
1.6.0.4

[U-Boot] [PATCH] I2C: Add support for multiple I2C busses for SNTP (effect to RTC)

[Heiko Schocher]

Hello Stephan,

added Ben Warren to cc: because this is a change in net,
which also falls in his area of responsibility ...

Stephan Linz wrote:
> This patch switches to the desired I2C bus when the SNTP
> network service is called. This can be configured using the
> CONFIG_SYS_RTC_BUS_NUM define.
> 
> In my eyes this is a bad and quick hack, but the same as
> was making for the date and dtt commands (commit: 0dc018e).
> The right way would be to move all the hardware specific i2c
> code down to the rtc driver layer.

Yes, thats would the best way, but why you don;t go this
way? Beside of that, this patch look good to me ...

> Signed-off-by: Stephan Linz <linz@li-pro.net>
> ---
>  net/sntp.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/net/sntp.c b/net/sntp.c
> index 76c10ec..5544570 100644
> --- a/net/sntp.c
> +++ b/net/sntp.c
> @@ -10,6 +10,14 @@
>  #include <net.h>
>  #include <rtc.h>
>  
> +/*
> + * FIXME: The i2c dependency should move into the RTC drivers itself.
> + * This higher network layer must not know about hardware specifics!
> + */
> +#if defined(CONFIG_CMD_DATE)
> +#include <i2c.h>
> +#endif
> +
>  #include "sntp.h"
>  
>  #define SNTP_TIMEOUT 10000UL
> @@ -53,6 +61,9 @@ SntpHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
>  	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__);
>  
> @@ -66,7 +77,14 @@ SntpHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
>  
>  	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,

bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] I2C: Add support for multiple I2C busses for SNTP (effect to RTC)

[Stephan Linz]

Hello Heiko,
Hello Ben,


> Hello Stephan,
> 
> added Ben Warren to cc: because this is a change in net,
> which also falls in his area of responsibility ...

OK, fine ...

> 
> Stephan Linz wrote:
> > This patch switches to the desired I2C bus when the SNTP
> > network service is called. This can be configured using the
> > CONFIG_SYS_RTC_BUS_NUM define.
> > 
> > In my eyes this is a bad and quick hack, but the same as
> > was making for the date and dtt commands (commit: 0dc018e).
> > The right way would be to move all the hardware specific i2c
> > code down to the rtc driver layer.
> 
> Yes, thats would the best way, but why you don;t go this
> way?

It's easy: time .... Joking apart, this patch brings the network layer in harmony to the command layer regardless of how beautiful it is. In one of the next steps I (we) can create a new patch to resolve this unsightly condition and move the hardware specifics into the rtc layer. Even such a patch will grow up and change more than a dozen of files.

I'm a friend of more small steps. So I've changed this small piece. I offer to make the big change in the next few weeks ... ?

> Beside of that, this patch look good to me ...

OK.
Ben, what do you mean?

> 
> > Signed-off-by: Stephan Linz <linz@li-pro.net>
> > ---
> >  net/sntp.c |   18 ++++++++++++++++++
> >  1 files changed, 18 insertions(+), 0 deletions(-)
> > 
> > diff --git a/net/sntp.c b/net/sntp.c
> > index 76c10ec..5544570 100644
> > --- a/net/sntp.c
> > +++ b/net/sntp.c
> > @@ -10,6 +10,14 @@
> >  #include <net.h>
> >  #include <rtc.h>
> >  
> > +/*
> > + * FIXME: The i2c dependency should move into the RTC drivers itself.
> > + * This higher network layer must not know about hardware specifics!
> > + */
> > +#if defined(CONFIG_CMD_DATE)
> > +#include <i2c.h>
> > +#endif
> > +
> >  #include "sntp.h"
> >  
> >  #define SNTP_TIMEOUT 10000UL
> > @@ -53,6 +61,9 @@ SntpHandler (uchar *pkt, unsigned dest, unsigned src,
> unsigned len)
> >  	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__);
> >  
> > @@ -66,7 +77,14 @@ SntpHandler (uchar *pkt, unsigned dest, unsigned src,
> unsigned len)
> >  
> >  	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,
> 
> bye
> Heiko
> -- 
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> 

--- original Nachricht Ende ----