* [U-Boot] [PATCH 0/3] Three mpc5200 patches from Pengutronix.de
@ 2009-03-21 13:38 Jon
2009-03-21 13:38 ` [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init() Jon
` (2 more replies)
0 siblings, 3 replies; 29+ messages in thread
From: Jon @ 2009-03-21 13:38 UTC (permalink / raw)
To: u-boot
Three generic mpc5200 patches from Pengutronix.de that missed being submitted to mainline.
---
Sascha Hauer (3):
mpc5200: make i2c faster
mpc52xx phy: initialize only when needed
mpc5200: do not use printf in i2c_init()
cpu/mpc5xxx/i2c.c | 16 +++-------------
drivers/net/mpc5xxx_fec.c | 11 +++++++++--
2 files changed, 12 insertions(+), 15 deletions(-)
--
Jon Smirl
jonsmirl at gmail.com
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-21 13:38 [U-Boot] [PATCH 0/3] Three mpc5200 patches from Pengutronix.de Jon
@ 2009-03-21 13:38 ` Jon
2009-03-21 19:47 ` Wolfgang Denk
2009-03-21 13:38 ` [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed Jon
2009-03-21 13:38 ` [U-Boot] [PATCH 3/3] mpc5200: make i2c faster Jon
2 siblings, 1 reply; 29+ messages in thread
From: Jon @ 2009-03-21 13:38 UTC (permalink / raw)
To: u-boot
From: Sascha Hauer <s.hauer@pengutronix.de>
On boards which have the environment in eeprom, i2c_init()
is called before the console and ram is initialized. Do
not printf here.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
cpu/mpc5xxx/i2c.c | 10 ----------
1 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
index 7d76274..843af9c 100644
--- a/cpu/mpc5xxx/i2c.c
+++ b/cpu/mpc5xxx/i2c.c
@@ -269,7 +269,6 @@ static int mpc_get_fdr(int speed)
if (gd->flags & GD_FLG_RELOC) {
fdr = divider;
} else {
- printf("%ld kHz, ", best_speed / 1000);
return divider;
}
}
@@ -310,29 +309,24 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
xaddr[3] = addr & 0xFF;
if (wait_for_bb()) {
- printf("i2c_read: bus is busy\n");
goto Done;
}
mpc_reg_out(®s->mcr, I2C_STA, I2C_STA);
if (do_address(chip, 0)) {
- printf("i2c_read: failed to address chip\n");
goto Done;
}
if (send_bytes(chip, &xaddr[4-alen], alen)) {
- printf("i2c_read: send_bytes failed\n");
goto Done;
}
mpc_reg_out(®s->mcr, I2C_RSTA, I2C_RSTA);
if (do_address(chip, 1)) {
- printf("i2c_read: failed to address chip\n");
goto Done;
}
if (receive_bytes(chip, (char *)buf, len)) {
- printf("i2c_read: receive_bytes failed\n");
goto Done;
}
@@ -354,23 +348,19 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
xaddr[3] = addr & 0xFF;
if (wait_for_bb()) {
- printf("i2c_write: bus is busy\n");
goto Done;
}
mpc_reg_out(®s->mcr, I2C_STA, I2C_STA);
if (do_address(chip, 0)) {
- printf("i2c_write: failed to address chip\n");
goto Done;
}
if (send_bytes(chip, &xaddr[4-alen], alen)) {
- printf("i2c_write: send_bytes failed\n");
goto Done;
}
if (send_bytes(chip, (char *)buf, len)) {
- printf("i2c_write: send_bytes failed\n");
goto Done;
}
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed
2009-03-21 13:38 [U-Boot] [PATCH 0/3] Three mpc5200 patches from Pengutronix.de Jon
2009-03-21 13:38 ` [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init() Jon
@ 2009-03-21 13:38 ` Jon
2009-03-25 19:12 ` Jon Smirl
2009-04-04 20:37 ` Wolfgang Denk
2009-03-21 13:38 ` [U-Boot] [PATCH 3/3] mpc5200: make i2c faster Jon
2 siblings, 2 replies; 29+ messages in thread
From: Jon @ 2009-03-21 13:38 UTC (permalink / raw)
To: u-boot
From: Sascha Hauer <s.hauer@pengutronix.de>
Do not initialize phy on startup, instead initialize it
when we actually need it.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/mpc5xxx_fec.c | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/net/mpc5xxx_fec.c b/drivers/net/mpc5xxx_fec.c
index 0f1d1af..1876b76 100644
--- a/drivers/net/mpc5xxx_fec.c
+++ b/drivers/net/mpc5xxx_fec.c
@@ -42,6 +42,8 @@ typedef struct {
int fec5xxx_miiphy_read(char *devname, uint8 phyAddr, uint8 regAddr, uint16 * retVal);
int fec5xxx_miiphy_write(char *devname, uint8 phyAddr, uint8 regAddr, uint16 data);
+static int mpc5xxx_fec_init_phy(struct eth_device *dev, bd_t * bis);
+
/********************************************************************/
#if (DEBUG & 0x2)
static void mpc5xxx_fec_phydump (char *devname)
@@ -249,6 +251,8 @@ static int mpc5xxx_fec_init(struct eth_device *dev, bd_t * bis)
printf ("mpc5xxx_fec_init... Begin\n");
#endif
+ mpc5xxx_fec_init_phy(dev, bis);
+
/*
* Initialize RxBD/TxBD rings
*/
@@ -387,6 +391,11 @@ static int mpc5xxx_fec_init_phy(struct eth_device *dev, bd_t * bis)
{
mpc5xxx_fec_priv *fec = (mpc5xxx_fec_priv *)dev->priv;
const uint8 phyAddr = CONFIG_PHY_ADDR; /* Only one PHY */
+ static int initialized = 0;
+
+ if(initialized)
+ return 0;
+ initialized = 1;
#if (DEBUG & 0x1)
printf ("mpc5xxx_fec_init_phy... Begin\n");
@@ -937,8 +946,6 @@ int mpc5xxx_fec_initialize(bd_t * bis)
mpc5xxx_fec_set_hwaddr(fec, env_enetaddr);
}
- mpc5xxx_fec_init_phy(dev, bis);
-
return 1;
}
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 3/3] mpc5200: make i2c faster
2009-03-21 13:38 [U-Boot] [PATCH 0/3] Three mpc5200 patches from Pengutronix.de Jon
2009-03-21 13:38 ` [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init() Jon
2009-03-21 13:38 ` [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed Jon
@ 2009-03-21 13:38 ` Jon
2009-03-21 19:48 ` Wolfgang Denk
2009-03-22 23:16 ` Sascha Hauer
2 siblings, 2 replies; 29+ messages in thread
From: Jon @ 2009-03-21 13:38 UTC (permalink / raw)
To: u-boot
From: Sascha Hauer <s.hauer@pengutronix.de>
The mpc5xxx i2c driver has great delays while waiting for the chip status.
make the delays smaller and the driver faster.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
cpu/mpc5xxx/i2c.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
index 843af9c..41feb1d 100644
--- a/cpu/mpc5xxx/i2c.c
+++ b/cpu/mpc5xxx/i2c.c
@@ -38,7 +38,7 @@ DECLARE_GLOBAL_DATA_PTR;
#error CONFIG_SYS_I2C_MODULE is not properly configured
#endif
-#define I2C_TIMEOUT 100
+#define I2C_TIMEOUT 10000
#define I2C_RETRIES 3
struct mpc5xxx_i2c_tap {
@@ -94,7 +94,7 @@ static int wait_for_bb(void)
mpc_reg_out(®s->mcr, 0, 0);
mpc_reg_out(®s->mcr, I2C_EN, 0);
#endif
- udelay(1000);
+ udelay(1);
status = mpc_reg_in(®s->msr);
}
@@ -109,7 +109,7 @@ static int wait_for_pin(int *status)
*status = mpc_reg_in(®s->msr);
while (timeout-- && !(*status & I2C_IF)) {
- udelay(1000);
+ udelay(1);
*status = mpc_reg_in(®s->msr);
}
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-21 13:38 ` [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init() Jon
@ 2009-03-21 19:47 ` Wolfgang Denk
2009-03-21 20:39 ` Jon Smirl
0 siblings, 1 reply; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-21 19:47 UTC (permalink / raw)
To: u-boot
Dear Jon,
In message <20090321133844.11905.61201.stgit@localhost> you wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
>
> On boards which have the environment in eeprom, i2c_init()
> is called before the console and ram is initialized. Do
> not printf here.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> cpu/mpc5xxx/i2c.c | 10 ----------
> 1 files changed, 0 insertions(+), 10 deletions(-)
Rather than doing this unconsitionally, this change should only be
implemented on boards which actually hold the envrionment in I2C
EEPROM (alternatively, one should consider fixing these boards to use
more appropriate storage; in addition to being not reliable, I2C
EEPROM is slow - see
http://www.denx.de/wiki/DULG/AN2004_11_BootTimeOptimization )
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
"Bureaucracy is the enemy of innovation."
- Mark Shepherd, former President and CEO of Texas Instruments
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 3/3] mpc5200: make i2c faster
2009-03-21 13:38 ` [U-Boot] [PATCH 3/3] mpc5200: make i2c faster Jon
@ 2009-03-21 19:48 ` Wolfgang Denk
2009-03-22 14:20 ` Jon Smirl
2009-03-22 23:16 ` Sascha Hauer
1 sibling, 1 reply; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-21 19:48 UTC (permalink / raw)
To: u-boot
Dear Jon,
In message <20090321133848.11905.59350.stgit@localhost> you wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
>
> The mpc5xxx i2c driver has great delays while waiting for the chip status.
> make the delays smaller and the driver faster.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> cpu/mpc5xxx/i2c.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
Are you absolutely sure that reducing these delays from 1 millisecond
to 1 microsecond will work on all boards with all currently supported
devices?
And how much do we make I2C faster this way? Where do we save time,
and how much?
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
It's certainly convenient the way the crime (or condition) of
stupidity carries with it its own punishment, automatically
admisistered without remorse, pity, or prejudice. :-)
-- Tom Christiansen in <559seq$ag1$1@csnews.cs.colorado.edu>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-21 19:47 ` Wolfgang Denk
@ 2009-03-21 20:39 ` Jon Smirl
2009-03-21 20:54 ` Wolfgang Denk
2009-03-21 20:56 ` Wolfgang Denk
0 siblings, 2 replies; 29+ messages in thread
From: Jon Smirl @ 2009-03-21 20:39 UTC (permalink / raw)
To: u-boot
On Sat, Mar 21, 2009 at 3:47 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Jon,
>
> In message <20090321133844.11905.61201.stgit@localhost> you wrote:
>> From: Sascha Hauer <s.hauer@pengutronix.de>
>>
>> On boards which have the environment in eeprom, i2c_init()
>> is called before the console and ram is initialized. Do
>> not printf here.
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>> ?cpu/mpc5xxx/i2c.c | ? 10 ----------
>> ?1 files changed, 0 insertions(+), 10 deletions(-)
>
> Rather than doing this unconditionally, this change should only be
> implemented on boards which actually hold the envrionment in I2C
> EEPROM (alternatively, one should consider fixing these boards to use
> more appropriate storage; in addition to being not reliable, I2C
> EEPROM is slow - see
> http://www.denx.de/wiki/DULG/AN2004_11_BootTimeOptimization )
>
How about this instead? Is there a variable that indicates when the
console is safe to use, if so I can adjust the macro.
From: Jon Smirl <jonsmirl@gmail.com>
On boards which have the environment in eeprom, i2c_init()
is called before the console and ram is initialized. Do
not printf here.
Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
---
cpu/mpc5xxx/i2c.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
index 7d76274..40f1682 100644
--- a/cpu/mpc5xxx/i2c.c
+++ b/cpu/mpc5xxx/i2c.c
@@ -41,6 +41,13 @@ DECLARE_GLOBAL_DATA_PTR;
#define I2C_TIMEOUT 100
#define I2C_RETRIES 3
+#ifdef CONFIG_ENV_IS_IN_EEPROM
+/* On boards which have the environment in eeprom, i2c_init()
+ * is called before the console and ram is initialized. Do
+ * not printf here. */
+#define printf(format, arg...) do {} while (0)
+#endif
+
struct mpc5xxx_i2c_tap {
int scl2tap;
int tap2tap;
--
Jon Smirl
jonsmirl at gmail.com
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-21 20:39 ` Jon Smirl
@ 2009-03-21 20:54 ` Wolfgang Denk
2009-03-21 20:56 ` Wolfgang Denk
1 sibling, 0 replies; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-21 20:54 UTC (permalink / raw)
To: u-boot
Dear Jon Smirl,
In message <9e4733910903211339u60f5531cp7a9574f2d1ba264f@mail.gmail.com> you wrote:
>
> How about this instead? Is there a variable that indicates when the
> console is safe to use, if so I can adjust the macro.
gd->have_console
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
Testing can show the presense of bugs, but not their absence.
-- Edsger Dijkstra
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-21 20:39 ` Jon Smirl
2009-03-21 20:54 ` Wolfgang Denk
@ 2009-03-21 20:56 ` Wolfgang Denk
2009-03-22 14:01 ` Jon Smirl
1 sibling, 1 reply; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-21 20:56 UTC (permalink / raw)
To: u-boot
Dear Jon Smirl,
In message <9e4733910903211339u60f5531cp7a9574f2d1ba264f@mail.gmail.com> you wrote:
>
> How about this instead? Is there a variable that indicates when the
> console is safe to use, if so I can adjust the macro.
BTW: you might want to compare with cpu/mpc8xx/i2c.c or cpu/ppc4xx/i2c.c
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
Where people stand is not as important as which way they face.
- Terry Pratchett & Stephen Briggs, _The Discworld Companion_
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-21 20:56 ` Wolfgang Denk
@ 2009-03-22 14:01 ` Jon Smirl
2009-03-22 14:21 ` Wolfgang Denk
0 siblings, 1 reply; 29+ messages in thread
From: Jon Smirl @ 2009-03-22 14:01 UTC (permalink / raw)
To: u-boot
On Sat, Mar 21, 2009 at 4:56 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Jon Smirl,
>
> In message <9e4733910903211339u60f5531cp7a9574f2d1ba264f@mail.gmail.com> you wrote:
>>
>> How about this instead? ?Is there a variable that indicates when the
>> console is safe to use, if so I can adjust the macro.
>
> BTW: you might want to compare with cpu/mpc8xx/i2c.c or cpu/ppc4xx/i2c.c
I've test this version with the eeprom and it works correctly.
-----------------------------------------------
mpc5200 with eeprom, suppress printf until console initialized
From: Jon Smirl <jonsmirl@gmail.com>
On boards which have the environment in eeprom, i2c_init()
is called before the console and ram is initialized. Suppress
printfs until the console is initialized.
Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
---
cpu/mpc5xxx/i2c.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
index 7d76274..a5478b2 100644
--- a/cpu/mpc5xxx/i2c.c
+++ b/cpu/mpc5xxx/i2c.c
@@ -41,6 +41,13 @@ DECLARE_GLOBAL_DATA_PTR;
#define I2C_TIMEOUT 100
#define I2C_RETRIES 3
+#ifdef CONFIG_ENV_IS_IN_EEPROM
+/* On boards which have the environment in eeprom, i2c_init()
+ * is called before the console and ram is initialized. Suppress
+ * printf until the console is initialized. */
+#define printf(format, arg...) if (gd->have_console) printf(format, ## arg)
+#endif
+
struct mpc5xxx_i2c_tap {
int scl2tap;
int tap2tap;
--
Jon Smirl
jonsmirl at gmail.com
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 3/3] mpc5200: make i2c faster
2009-03-21 19:48 ` Wolfgang Denk
@ 2009-03-22 14:20 ` Jon Smirl
2009-03-22 15:21 ` Wolfgang Denk
0 siblings, 1 reply; 29+ messages in thread
From: Jon Smirl @ 2009-03-22 14:20 UTC (permalink / raw)
To: u-boot
On Sat, Mar 21, 2009 at 3:48 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Jon,
>
> In message <20090321133848.11905.59350.stgit@localhost> you wrote:
>> From: Sascha Hauer <s.hauer@pengutronix.de>
>>
>> The mpc5xxx i2c driver has great delays while waiting for the chip status.
>> make the delays smaller and the driver faster.
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>> ?cpu/mpc5xxx/i2c.c | ? ?6 +++---
>> ?1 files changed, 3 insertions(+), 3 deletions(-)
>
> Are you absolutely sure that reducing these delays from 1 millisecond
> to 1 microsecond will work on all boards with all currently supported
> devices?
>
> And how much do we make I2C faster this way? Where do we save time,
> and how much?
I measured the delay. It is between 40us and 52us on my hardware so I
adjusted the loop to 60us. The previous 1ms delay was 20x bigger than
necessary. If the hardware is slower, it will just loop and add more
delay.
----------------------------------------------------------------------
mpc5200: reduce delays in i2c
The mpc5xxx i2c driver has great delays while waiting for the chip status.
Make the delays smaller and the driver faster. The measured delay is
55us at 100Khz. Set the delay to 15us which should work for 400Khz. 100Khz
will loop four times and get a 60us delay.
Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
---
cpu/mpc5xxx/i2c.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
index a5478b2..dcb184c 100644
--- a/cpu/mpc5xxx/i2c.c
+++ b/cpu/mpc5xxx/i2c.c
@@ -38,7 +38,7 @@ DECLARE_GLOBAL_DATA_PTR;
#error CONFIG_SYS_I2C_MODULE is not properly configured
#endif
-#define I2C_TIMEOUT 100
+#define I2C_TIMEOUT 1000
#define I2C_RETRIES 3
#ifdef CONFIG_ENV_IS_IN_EEPROM
@@ -101,7 +101,7 @@ static int wait_for_bb(void)
mpc_reg_out(®s->mcr, 0, 0);
mpc_reg_out(®s->mcr, I2C_EN, 0);
#endif
- udelay(1000);
+ udelay(15);
status = mpc_reg_in(®s->msr);
}
@@ -116,7 +116,7 @@ static int wait_for_pin(int *status)
*status = mpc_reg_in(®s->msr);
while (timeout-- && !(*status & I2C_IF)) {
- udelay(1000);
+ udelay(15);
*status = mpc_reg_in(®s->msr);
}
>
> 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
> It's certainly ?convenient ?the ?way ?the ?crime ?(or ?condition) ?of
> stupidity ? carries ? with ? it ?its ?own ?punishment, ?automatically
> admisistered without remorse, pity, or prejudice. :-)
> ? ? ? ? -- Tom Christiansen in <559seq$ag1$1@csnews.cs.colorado.edu>
>
--
Jon Smirl
jonsmirl at gmail.com
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-22 14:01 ` Jon Smirl
@ 2009-03-22 14:21 ` Wolfgang Denk
2009-03-22 14:23 ` Jon Smirl
0 siblings, 1 reply; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-22 14:21 UTC (permalink / raw)
To: u-boot
Dear Jon,
In message <9e4733910903220701h104932cdu6b226e5c05e9b3d4@mail.gmail.com> you wrote:
>
> I've test this version with the eeprom and it works correctly.
It may work, but such a macro definition violates some rules.
> +#define printf(format, arg...) if (gd->have_console) printf(format, ## arg)
There are only a few places in cpu/mpc5xxx/i2c.c where printf() is
used that we should explicitly code this, there.
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: wd at denx.de
Your own mileage may vary.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-22 14:21 ` Wolfgang Denk
@ 2009-03-22 14:23 ` Jon Smirl
2009-03-22 15:23 ` Wolfgang Denk
0 siblings, 1 reply; 29+ messages in thread
From: Jon Smirl @ 2009-03-22 14:23 UTC (permalink / raw)
To: u-boot
On Sun, Mar 22, 2009 at 10:21 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Jon,
>
> In message <9e4733910903220701h104932cdu6b226e5c05e9b3d4@mail.gmail.com> you wrote:
>>
>> I've test this version with the eeprom and it works correctly.
>
> It may work, but such a macro definition violates some rules.
>
>> +#define printf(format, arg...) if (gd->have_console) printf(format, ## arg)
>
> There are only a few places in cpu/mpc5xxx/i2c.c where printf() is
> used that we should explicitly code this, there.
Would it be better to put the check for (gd->have_console) into printf itself?
--
Jon Smirl
jonsmirl at gmail.com
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 3/3] mpc5200: make i2c faster
2009-03-22 14:20 ` Jon Smirl
@ 2009-03-22 15:21 ` Wolfgang Denk
2009-03-23 2:59 ` Jon Smirl
0 siblings, 1 reply; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-22 15:21 UTC (permalink / raw)
To: u-boot
Dear Jon,
In message <9e4733910903220720l723de65fm59a7e6a7fed1eda7@mail.gmail.com> you wrote:
>
> > And how much do we make I2C faster this way? Where do we save time,
> > and how much?
>
> I measured the delay. It is between 40us and 52us on my hardware so I
> adjusted the loop to 60us. The previous 1ms delay was 20x bigger than
> necessary. If the hardware is slower, it will just loop and add more
> delay.
You did not answer my question in which way this actually makes I2C
faster? Where do we save time, and how much?
Which operation of I2C on the board gets acelerated, and how much?
> The mpc5xxx i2c driver has great delays while waiting for the chip status.
> Make the delays smaller and the driver faster. The measured delay is
> 55us at 100Khz. Set the delay to 15us which should work for 400Khz. 100Khz
> will loop four times and get a 60us delay.
>
> Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
> ---
> cpu/mpc5xxx/i2c.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
> index a5478b2..dcb184c 100644
> --- a/cpu/mpc5xxx/i2c.c
> +++ b/cpu/mpc5xxx/i2c.c
> @@ -38,7 +38,7 @@ DECLARE_GLOBAL_DATA_PTR;
> #error CONFIG_SYS_I2C_MODULE is not properly configured
> #endif
>
> -#define I2C_TIMEOUT 100
> +#define I2C_TIMEOUT 1000
> #define I2C_RETRIES 3
>
> #ifdef CONFIG_ENV_IS_IN_EEPROM
> @@ -101,7 +101,7 @@ static int wait_for_bb(void)
> mpc_reg_out(®s->mcr, 0, 0);
> mpc_reg_out(®s->mcr, I2C_EN, 0);
> #endif
> - udelay(1000);
> + udelay(15);
> status = mpc_reg_in(®s->msr);
> }
Hm.. You increase the I2C_TIMEOUT by a factor of 10, but reduce the
per-loop delay by a factor of > 66; instead of the old total timeout
of 100 * 1000 us = 100 milliseconds we now have a timeout of 1000 *
15 us = 15 milliseconds. That is much, much less.
Is this change intentional? If yes, are you sure it is correct with
all existing boards and I2C devices out there?
> @@ -116,7 +116,7 @@ static int wait_for_pin(int *status)
> *status = mpc_reg_in(®s->msr);
>
> while (timeout-- && !(*status & I2C_IF)) {
> - udelay(1000);
> + udelay(15);
> *status = mpc_reg_in(®s->msr);
Ditto here.
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
The best things in life are for a fee.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-22 14:23 ` Jon Smirl
@ 2009-03-22 15:23 ` Wolfgang Denk
2009-03-23 2:55 ` Jon Smirl
0 siblings, 1 reply; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-22 15:23 UTC (permalink / raw)
To: u-boot
Dear Jon Smirl,
In message <9e4733910903220723u31546286q233c9b24b7a5d53@mail.gmail.com> you wrote:
>
> Would it be better to put the check for (gd->have_console) into printf itself?
No, as you could also use puts() or putc(). There are only a few
isolated places where we need this, so let's make this clearly
visible there.
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
The use of COBOL cripples the mind; its teaching should, therefore,
be regarded as a criminal offence.
-- Edsger W. Dijkstra, SIGPLAN Notices, Volume 17, Number 5
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 3/3] mpc5200: make i2c faster
2009-03-21 13:38 ` [U-Boot] [PATCH 3/3] mpc5200: make i2c faster Jon
2009-03-21 19:48 ` Wolfgang Denk
@ 2009-03-22 23:16 ` Sascha Hauer
2009-03-23 3:00 ` Grant Likely
1 sibling, 1 reply; 29+ messages in thread
From: Sascha Hauer @ 2009-03-22 23:16 UTC (permalink / raw)
To: u-boot
Hi,
On Sat, Mar 21, 2009 at 09:38:48AM -0400, Jon wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
>
> The mpc5xxx i2c driver has great delays while waiting for the chip status.
> make the delays smaller and the driver faster.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> cpu/mpc5xxx/i2c.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
> index 843af9c..41feb1d 100644
> --- a/cpu/mpc5xxx/i2c.c
> +++ b/cpu/mpc5xxx/i2c.c
> @@ -38,7 +38,7 @@ DECLARE_GLOBAL_DATA_PTR;
> #error CONFIG_SYS_I2C_MODULE is not properly configured
> #endif
>
> -#define I2C_TIMEOUT 100
> +#define I2C_TIMEOUT 10000
> #define I2C_RETRIES 3
>
> struct mpc5xxx_i2c_tap {
> @@ -94,7 +94,7 @@ static int wait_for_bb(void)
> mpc_reg_out(®s->mcr, 0, 0);
> mpc_reg_out(®s->mcr, I2C_EN, 0);
> #endif
> - udelay(1000);
> + udelay(1);
> status = mpc_reg_in(®s->msr);
> }
>
The actual condition we wait for takes only a few useconds to become
true, but we wait for at least 1ms. As the I2C EEPROM environment
routine reads nearly the complete EEPROM several times during startup
the speed gain is enormous.
I can't recall if it was intentional to reduce the overall timeout with
this patch, but I can imagine that the i2c detect function will be
faster aswell.
> @@ -109,7 +109,7 @@ static int wait_for_pin(int *status)
> *status = mpc_reg_in(®s->msr);
>
> while (timeout-- && !(*status & I2C_IF)) {
> - udelay(1000);
> + udelay(1);
> *status = mpc_reg_in(®s->msr);
Same here.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-22 15:23 ` Wolfgang Denk
@ 2009-03-23 2:55 ` Jon Smirl
2009-03-24 8:38 ` Heiko Schocher
2009-03-25 21:51 ` Wolfgang Denk
0 siblings, 2 replies; 29+ messages in thread
From: Jon Smirl @ 2009-03-23 2:55 UTC (permalink / raw)
To: u-boot
On Sun, Mar 22, 2009 at 11:23 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Jon Smirl,
>
> In message <9e4733910903220723u31546286q233c9b24b7a5d53@mail.gmail.com> you wrote:
>>
>> Would it be better to put the check for (gd->have_console) ?into printf itself?
>
> No, as you could also use puts() or putc(). ?There ?are ?only ?a ?few
> isolated ?places ?where ?we ?need ?this, ?so ?let's make this clearly
> visible there.
mpc5200 with eeprom, suppress printf until console initialized
From: Jon Smirl <jonsmirl@gmail.com>
On boards which have the environment in eeprom, i2c_init()
is called before the console and ram is initialized. Suppress
printfs until the console is initialized.
Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
---
cpu/mpc5xxx/i2c.c | 30 ++++++++++++++++++++----------
1 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
index 7d76274..0860ecf 100644
--- a/cpu/mpc5xxx/i2c.c
+++ b/cpu/mpc5xxx/i2c.c
@@ -269,7 +269,8 @@ static int mpc_get_fdr(int speed)
if (gd->flags & GD_FLG_RELOC) {
fdr = divider;
} else {
- printf("%ld kHz, ", best_speed / 1000);
+ if (gd->have_console)
+ printf("%ld kHz, ", best_speed / 1000);
return divider;
}
}
@@ -310,29 +311,34 @@ int i2c_read(uchar chip, uint addr, int alen,
uchar *buf, int len)
xaddr[3] = addr & 0xFF;
if (wait_for_bb()) {
- printf("i2c_read: bus is busy\n");
+ if (gd->have_console)
+ printf("i2c_read: bus is busy\n");
goto Done;
}
mpc_reg_out(®s->mcr, I2C_STA, I2C_STA);
if (do_address(chip, 0)) {
- printf("i2c_read: failed to address chip\n");
+ if (gd->have_console)
+ printf("i2c_read: failed to address chip\n");
goto Done;
}
if (send_bytes(chip, &xaddr[4-alen], alen)) {
- printf("i2c_read: send_bytes failed\n");
+ if (gd->have_console)
+ printf("i2c_read: send_bytes failed\n");
goto Done;
}
mpc_reg_out(®s->mcr, I2C_RSTA, I2C_RSTA);
if (do_address(chip, 1)) {
- printf("i2c_read: failed to address chip\n");
+ if (gd->have_console)
+ printf("i2c_read: failed to address chip\n");
goto Done;
}
if (receive_bytes(chip, (char *)buf, len)) {
- printf("i2c_read: receive_bytes failed\n");
+ if (gd->have_console)
+ printf("i2c_read: receive_bytes failed\n");
goto Done;
}
@@ -354,23 +360,27 @@ int i2c_write(uchar chip, uint addr, int alen,
uchar *buf, int len)
xaddr[3] = addr & 0xFF;
if (wait_for_bb()) {
- printf("i2c_write: bus is busy\n");
+ if (gd->have_console)
+ printf("i2c_write: bus is busy\n");
goto Done;
}
mpc_reg_out(®s->mcr, I2C_STA, I2C_STA);
if (do_address(chip, 0)) {
- printf("i2c_write: failed to address chip\n");
+ if (gd->have_console)
+ printf("i2c_write: failed to address chip\n");
goto Done;
}
if (send_bytes(chip, &xaddr[4-alen], alen)) {
- printf("i2c_write: send_bytes failed\n");
+ if (gd->have_console)
+ printf("i2c_write: send_bytes failed\n");
goto Done;
}
if (send_bytes(chip, (char *)buf, len)) {
- printf("i2c_write: send_bytes failed\n");
+ if (gd->have_console)
+ printf("i2c_write: send_bytes failed\n");
goto Done;
}
--
Jon Smirl
jonsmirl at gmail.com
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 3/3] mpc5200: make i2c faster
2009-03-22 15:21 ` Wolfgang Denk
@ 2009-03-23 2:59 ` Jon Smirl
2009-03-23 8:24 ` Wolfgang Denk
0 siblings, 1 reply; 29+ messages in thread
From: Jon Smirl @ 2009-03-23 2:59 UTC (permalink / raw)
To: u-boot
On Sun, Mar 22, 2009 at 11:21 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Jon,
>
> In message <9e4733910903220720l723de65fm59a7e6a7fed1eda7@mail.gmail.com> you wrote:
>>
>> > And how much do we make I2C faster this way? Where do we save time,
>> > and how much?
>>
>> I measured the delay. It is between 40us and 52us on my hardware so I
>> adjusted the loop to 60us. The previous 1ms delay was 20x bigger than
>> necessary. ?If the hardware is slower, it will ?just loop and add more
>> delay.
>
> You did not answer my question in which way this actually makes I2C
> faster? Where do we save time, and how much?
I increased the retry loop to 10,000. This definitely makes my system
faster. On my bus the actual i2c delay is 40-55us. The original code
always delayed 1,000us so for me it a gain of 940us on each i2c
operation. This is visible during things like probe.
mpc5200: reduce delays in i2c
From: Sascha Hauer <s.hauer@pengutronix.de>
The mpc5xxx i2c driver has great delays while waiting for the chip status.
make the delays smaller and the driver faster. The measured delay is
55us at 100Khz. Set the delay to 15us which should work for 400Khz. 100Khz
will loop four times and get a 60us delay.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
cpu/mpc5xxx/i2c.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cpu/mpc5xxx/i2c.c b/cpu/mpc5xxx/i2c.c
index 0860ecf..58470d1 100644
--- a/cpu/mpc5xxx/i2c.c
+++ b/cpu/mpc5xxx/i2c.c
@@ -38,7 +38,7 @@ DECLARE_GLOBAL_DATA_PTR;
#error CONFIG_SYS_I2C_MODULE is not properly configured
#endif
-#define I2C_TIMEOUT 100
+#define I2C_TIMEOUT 10000
#define I2C_RETRIES 3
struct mpc5xxx_i2c_tap {
@@ -94,7 +94,7 @@ static int wait_for_bb(void)
mpc_reg_out(®s->mcr, 0, 0);
mpc_reg_out(®s->mcr, I2C_EN, 0);
#endif
- udelay(1000);
+ udelay(15);
status = mpc_reg_in(®s->msr);
}
@@ -109,7 +109,7 @@ static int wait_for_pin(int *status)
*status = mpc_reg_in(®s->msr);
while (timeout-- && !(*status & I2C_IF)) {
- udelay(1000);
+ udelay(15);
*status = mpc_reg_in(®s->msr);
}
--
Jon Smirl
jonsmirl at gmail.com
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 3/3] mpc5200: make i2c faster
2009-03-22 23:16 ` Sascha Hauer
@ 2009-03-23 3:00 ` Grant Likely
2009-03-23 6:24 ` Wolfgang Denk
0 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2009-03-23 3:00 UTC (permalink / raw)
To: u-boot
On Sun, Mar 22, 2009 at 5:16 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Sat, Mar 21, 2009 at 09:38:48AM -0400, Jon wrote:
>> ?struct mpc5xxx_i2c_tap {
>> @@ -94,7 +94,7 @@ static int wait_for_bb(void)
>> ? ? ? ? ? ? ? mpc_reg_out(®s->mcr, 0, 0);
>> ? ? ? ? ? ? ? mpc_reg_out(®s->mcr, I2C_EN, 0);
>> ?#endif
>> - ? ? ? ? ? ? udelay(1000);
>> + ? ? ? ? ? ? udelay(1);
>> ? ? ? ? ? ? ? status = mpc_reg_in(®s->msr);
>> ? ? ? }
>>
>
> The actual condition we wait for takes only a few useconds to become
> true, but we wait for at least 1ms. As the I2C EEPROM environment
> routine reads nearly the complete EEPROM several times during startup
> the speed gain is enormous.
> I can't recall if it was intentional to reduce the overall timeout with
> this patch, but I can imagine that the i2c detect function will be
> faster aswell.
Even better, since this is entirely a busywait, the udelay(1) could be
dropped entirely and the exit condition be based on the value of
timebase. That would give the absolute maximum resolution to the
busyloop. However, the incremental improvement probably isn't a great
deal better than this patch.
g.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 3/3] mpc5200: make i2c faster
2009-03-23 3:00 ` Grant Likely
@ 2009-03-23 6:24 ` Wolfgang Denk
0 siblings, 0 replies; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-23 6:24 UTC (permalink / raw)
To: u-boot
Dear Grant Likely,
In message <fa686aa40903222000l13369f61gf93b26b4c804e383@mail.gmail.com> you wrote:
>
> Even better, since this is entirely a busywait, the udelay(1) could be
> dropped entirely and the exit condition be based on the value of
> timebase. That would give the absolute maximum resolution to the
> busyloop. However, the incremental improvement probably isn't a great
> deal better than this patch.
I disagree. Keep in mind that on systems with hardware watchdog
enabled udelay() implicitly takes care of triggering the watchdog, so
using it in busy-wait loops is always a good idea.
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
Build a system that even a fool can use and only a fool will want to
use it.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 3/3] mpc5200: make i2c faster
2009-03-23 2:59 ` Jon Smirl
@ 2009-03-23 8:24 ` Wolfgang Denk
0 siblings, 0 replies; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-23 8:24 UTC (permalink / raw)
To: u-boot
Dear Jon,
In message <9e4733910903221959n452a067kd46b313b9b453ec3@mail.gmail.com> you wrote:
>
> > You did not answer my question in which way this actually makes I2C
> > faster? Where do we save time, and how much?
>
> I increased the retry loop to 10,000. This definitely makes my system
> faster. On my bus the actual i2c delay is 40-55us. The original code
> always delayed 1,000us so for me it a gain of 940us on each i2c
> operation. This is visible during things like probe.
Hm... you don't really convince me. Being "visible" is nice, but is
it a reason to change the code? I mean, if you said something like:
"without that patch operation FOO takes 20 seconds, and with the
patch it takes less than 3 seconds", then everybody can understand
why this is a good thing to do.
And: the I2C probe operation itself is just a debug thing, nothing
you do in normal operation, so it doesn't look that time critical to
me.
I agree that the change itself looks harmless, and I'm willing to
accept it, but at least make sure not to reduce the total timeout
values - that would require re-testing on some boards.
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
"Deliver yesterday, code today, think tomorrow."
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-23 2:55 ` Jon Smirl
@ 2009-03-24 8:38 ` Heiko Schocher
2009-03-25 21:51 ` Wolfgang Denk
1 sibling, 0 replies; 29+ messages in thread
From: Heiko Schocher @ 2009-03-24 8:38 UTC (permalink / raw)
To: u-boot
Hello Jon,
Jon Smirl wrote:
> On Sun, Mar 22, 2009 at 11:23 AM, Wolfgang Denk <wd@denx.de> wrote:
>
>> Dear Jon Smirl,
>>
>> In message <9e4733910903220723u31546286q233c9b24b7a5d53@mail.gmail.com> you wrote:
>>
>>> Would it be better to put the check for (gd->have_console) into printf itself?
>>>
>> No, as you could also use puts() or putc(). There are only a few
>> isolated places where we need this, so let's make this clearly
>> visible there.
>>
>
> mpc5200 with eeprom, suppress printf until console initialized
>
> From: Jon Smirl <jonsmirl@gmail.com>
>
> On boards which have the environment in eeprom, i2c_init()
> is called before the console and ram is initialized. Suppress
> printfs until the console is initialized.
>
> Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
> ---
> cpu/mpc5xxx/i2c.c | 30 ++++++++++++++++++++----------
> 1 files changed, 20 insertions(+), 10 deletions(-)
>
Acked-by: Heiko Schocher <hs@denx.de>
bye
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed
2009-03-21 13:38 ` [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed Jon
@ 2009-03-25 19:12 ` Jon Smirl
2009-03-25 21:53 ` Wolfgang Denk
2009-04-04 20:37 ` Wolfgang Denk
1 sibling, 1 reply; 29+ messages in thread
From: Jon Smirl @ 2009-03-25 19:12 UTC (permalink / raw)
To: u-boot
Is this patch ok for inclusion?
On Sat, Mar 21, 2009 at 9:38 AM, Jon <jonsmirl@gmail.com> wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
>
> Do not initialize phy on startup, instead initialize it
> when we actually need it.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> ?drivers/net/mpc5xxx_fec.c | ? 11 +++++++++--
> ?1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/mpc5xxx_fec.c b/drivers/net/mpc5xxx_fec.c
> index 0f1d1af..1876b76 100644
> --- a/drivers/net/mpc5xxx_fec.c
> +++ b/drivers/net/mpc5xxx_fec.c
> @@ -42,6 +42,8 @@ typedef struct {
> ?int fec5xxx_miiphy_read(char *devname, uint8 phyAddr, uint8 regAddr, uint16 * retVal);
> ?int fec5xxx_miiphy_write(char *devname, uint8 phyAddr, uint8 regAddr, uint16 data);
>
> +static int mpc5xxx_fec_init_phy(struct eth_device *dev, bd_t * bis);
> +
> ?/********************************************************************/
> ?#if (DEBUG & 0x2)
> ?static void mpc5xxx_fec_phydump (char *devname)
> @@ -249,6 +251,8 @@ static int mpc5xxx_fec_init(struct eth_device *dev, bd_t * bis)
> ? ? ? ?printf ("mpc5xxx_fec_init... Begin\n");
> ?#endif
>
> + ? ? ? mpc5xxx_fec_init_phy(dev, bis);
> +
> ? ? ? ?/*
> ? ? ? ? * Initialize RxBD/TxBD rings
> ? ? ? ? */
> @@ -387,6 +391,11 @@ static int mpc5xxx_fec_init_phy(struct eth_device *dev, bd_t * bis)
> ?{
> ? ? ? ?mpc5xxx_fec_priv *fec = (mpc5xxx_fec_priv *)dev->priv;
> ? ? ? ?const uint8 phyAddr = CONFIG_PHY_ADDR; ?/* Only one PHY */
> + ? ? ? static int initialized = 0;
> +
> + ? ? ? if(initialized)
> + ? ? ? ? ? ? ? return 0;
> + ? ? ? initialized = 1;
>
> ?#if (DEBUG & 0x1)
> ? ? ? ?printf ("mpc5xxx_fec_init_phy... Begin\n");
> @@ -937,8 +946,6 @@ int mpc5xxx_fec_initialize(bd_t * bis)
> ? ? ? ? ? ? ? ?mpc5xxx_fec_set_hwaddr(fec, env_enetaddr);
> ? ? ? ?}
>
> - ? ? ? mpc5xxx_fec_init_phy(dev, bis);
> -
> ? ? ? ?return 1;
> ?}
>
>
>
--
Jon Smirl
jonsmirl at gmail.com
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-23 2:55 ` Jon Smirl
2009-03-24 8:38 ` Heiko Schocher
@ 2009-03-25 21:51 ` Wolfgang Denk
2009-03-25 22:31 ` Jon Smirl
1 sibling, 1 reply; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-25 21:51 UTC (permalink / raw)
To: u-boot
Dear Jon Smirl,
In message <9e4733910903221955h4a6b3c24t34cce6989911be21@mail.gmail.com> you wrote:
> On Sun, Mar 22, 2009 at 11:23 AM, Wolfgang Denk <wd@denx.de> wrote:
> > Dear Jon Smirl,
> >
> > In message <9e4733910903220723u31546286q233c9b24b7a5d53@mail.gmail.com> y=
> ou wrote:
> >>
> >> Would it be better to put the check for (gd->have_console) =A0into print=
> f itself?
> >
> > No, as you could also use puts() or putc(). =A0There =A0are =A0only =A0a =
> =A0few
> > isolated =A0places =A0where =A0we =A0need =A0this, =A0so =A0let's make th=
> is clearly
> > visible there.
>
> mpc5200 with eeprom, suppress printf until console initialized
>
> From: Jon Smirl <jonsmirl@gmail.com>
Argh.... Such comments have to go *below* the '---' line, otherwise
they become part of the commit message.
And your patch is line-wrapped and thus corrupted by your mailer.
Please FIX your mailer (or even better use git-send-email to submit
patches).
> On boards which have the environment in eeprom, i2c_init()
> is called before the console and ram is initialized. Suppress
> printfs until the console is initialized.
>
> Signed-off-by: Jon Smirl <jonsmirl@gmail.com>
> ---
> cpu/mpc5xxx/i2c.c | 30 ++++++++++++++++++++----------
> 1 files changed, 20 insertions(+), 10 deletions(-)
Fixed manually and applied. 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: wd at denx.de
"In Christianity neither morality nor religion come into contact with
reality at any point." - Friedrich Nietzsche
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed
2009-03-25 19:12 ` Jon Smirl
@ 2009-03-25 21:53 ` Wolfgang Denk
2009-03-31 1:04 ` Jon Smirl
0 siblings, 1 reply; 29+ messages in thread
From: Wolfgang Denk @ 2009-03-25 21:53 UTC (permalink / raw)
To: u-boot
Dear Jon Smirl,
In message <9e4733910903251212j4436f1afu817456c7f6e9c970@mail.gmail.com> you wrote:
> Is this patch ok for inclusion?
>
> On Sat, Mar 21, 2009 at 9:38 AM, Jon <jonsmirl@gmail.com> wrote:
> > From: Sascha Hauer <s.hauer@pengutronix.de>
> >
> > Do not initialize phy on startup, instead initialize it
> > when we actually need it.
> >
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > ?drivers/net/mpc5xxx_fec.c | ? 11 +++++++++--
> > ?1 files changed, 9 insertions(+), 2 deletions(-)
This is more for Ben to review, I think.
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
Intuition, however illogical, is recognized as a command prerogative.
-- Kirk, "Obsession", stardate 3620.7
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init()
2009-03-25 21:51 ` Wolfgang Denk
@ 2009-03-25 22:31 ` Jon Smirl
0 siblings, 0 replies; 29+ messages in thread
From: Jon Smirl @ 2009-03-25 22:31 UTC (permalink / raw)
To: u-boot
On Wed, Mar 25, 2009 at 5:51 PM, Wolfgang Denk <wd@denx.de> wrote:
> Fixed manually and applied. Thanks.
I used stgit to smtp.gmail.com::587 for the initial patch.
I wasn't thinking and used cut/paste to paste the last patch into a reply.
These patches are at git.digispeaker.com if that is easier.
--
Jon Smirl
jonsmirl at gmail.com
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed
2009-03-25 21:53 ` Wolfgang Denk
@ 2009-03-31 1:04 ` Jon Smirl
2009-03-31 6:13 ` Ben Warren
0 siblings, 1 reply; 29+ messages in thread
From: Jon Smirl @ 2009-03-31 1:04 UTC (permalink / raw)
To: u-boot
On Wed, Mar 25, 2009 at 5:53 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Jon Smirl,
>
> In message <9e4733910903251212j4436f1afu817456c7f6e9c970@mail.gmail.com> you wrote:
>> Is this patch ok for inclusion?
Ping? Any answer on this one?
>>
>> On Sat, Mar 21, 2009 at 9:38 AM, Jon <jonsmirl@gmail.com> wrote:
>> > From: Sascha Hauer <s.hauer@pengutronix.de>
>> >
>> > Do not initialize phy on startup, instead initialize it
>> > when we actually need it.
>> >
>> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> > ---
>> > ?drivers/net/mpc5xxx_fec.c | ? 11 +++++++++--
>> > ?1 files changed, 9 insertions(+), 2 deletions(-)
>
> This is more for Ben to review, I think.
>
> 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
> Intuition, however illogical, is recognized as a command prerogative.
> ? ? ? ?-- Kirk, "Obsession", stardate 3620.7
>
--
Jon Smirl
jonsmirl at gmail.com
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed
2009-03-31 1:04 ` Jon Smirl
@ 2009-03-31 6:13 ` Ben Warren
0 siblings, 0 replies; 29+ messages in thread
From: Ben Warren @ 2009-03-31 6:13 UTC (permalink / raw)
To: u-boot
On Mon, Mar 30, 2009 at 6:04 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
> On Wed, Mar 25, 2009 at 5:53 PM, Wolfgang Denk <wd@denx.de> wrote:
> > Dear Jon Smirl,
> >
> > In message <9e4733910903251212j4436f1afu817456c7f6e9c970@mail.gmail.com>
> you wrote:
> >> Is this patch ok for inclusion?
>
> Ping? Any answer on this one?
>
Sorry, I have some catching up to do. Will respond within the next couple
of days.
regards,
Ben
^ permalink raw reply [flat|nested] 29+ messages in thread
* [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed
2009-03-21 13:38 ` [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed Jon
2009-03-25 19:12 ` Jon Smirl
@ 2009-04-04 20:37 ` Wolfgang Denk
1 sibling, 0 replies; 29+ messages in thread
From: Wolfgang Denk @ 2009-04-04 20:37 UTC (permalink / raw)
To: u-boot
Dear Jon,
In message <20090321133846.11905.81764.stgit@localhost> you wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
>
> Do not initialize phy on startup, instead initialize it
> when we actually need it.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/net/mpc5xxx_fec.c | 11 +++++++++--
> 1 files changed, 9 insertions(+), 2 deletions(-)
Applied, 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: wd at denx.de
The average woman would rather have beauty than brains, because the
average man can see better than he can think.
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2009-04-04 20:37 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-21 13:38 [U-Boot] [PATCH 0/3] Three mpc5200 patches from Pengutronix.de Jon
2009-03-21 13:38 ` [U-Boot] [PATCH 1/3] mpc5200: do not use printf in i2c_init() Jon
2009-03-21 19:47 ` Wolfgang Denk
2009-03-21 20:39 ` Jon Smirl
2009-03-21 20:54 ` Wolfgang Denk
2009-03-21 20:56 ` Wolfgang Denk
2009-03-22 14:01 ` Jon Smirl
2009-03-22 14:21 ` Wolfgang Denk
2009-03-22 14:23 ` Jon Smirl
2009-03-22 15:23 ` Wolfgang Denk
2009-03-23 2:55 ` Jon Smirl
2009-03-24 8:38 ` Heiko Schocher
2009-03-25 21:51 ` Wolfgang Denk
2009-03-25 22:31 ` Jon Smirl
2009-03-21 13:38 ` [U-Boot] [PATCH 2/3] mpc52xx phy: initialize only when needed Jon
2009-03-25 19:12 ` Jon Smirl
2009-03-25 21:53 ` Wolfgang Denk
2009-03-31 1:04 ` Jon Smirl
2009-03-31 6:13 ` Ben Warren
2009-04-04 20:37 ` Wolfgang Denk
2009-03-21 13:38 ` [U-Boot] [PATCH 3/3] mpc5200: make i2c faster Jon
2009-03-21 19:48 ` Wolfgang Denk
2009-03-22 14:20 ` Jon Smirl
2009-03-22 15:21 ` Wolfgang Denk
2009-03-23 2:59 ` Jon Smirl
2009-03-23 8:24 ` Wolfgang Denk
2009-03-22 23:16 ` Sascha Hauer
2009-03-23 3:00 ` Grant Likely
2009-03-23 6:24 ` Wolfgang Denk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox