Skip to contentSkip to conversation

RFC: tegra: Avoid using I2C prior to relocation

PATCH New Simon Glass 4 participants 11 messages Aug 7, 2013 – Aug 14, 2013

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

Tegra recently moved to the new I2C framework, which sets up I2C prior to relocation, and prior to calling i2c_init_board(). This causes a crash on Tegra boards.

note:

There are many ways to fix this. I believe this is one. It disables i2c_init() until relocation is complete. I have been unable to test it so far due to problems getting my Seaboard to work. I will try another Tegra board, but send this for comment in the meantime.

---
 drivers/i2c/tegra_i2c.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/i2c/tegra_i2c.c +4 -0
index 9ac3969..9847cf1 100644
@@ -453,6 +453,10 @@ void i2c_init_board(void)
 
 static void tegra_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr)
 {
+	/* No i2c support prior to relocation */
+	if (!(gd->flags & GD_FLG_RELOC))
+		return;
+
 	/* This will override the speed selected in the fdt for that port */
 	debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
 	i2c_set_bus_speed(speed);
Signature · 1 line
1.8.3
#1 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

Quoted 10 lines · Simon Glass

Tegra recently moved to the new I2C framework, which sets up I2C prior to relocation, and prior to calling i2c_init_board(). This causes a crash on Tegra boards.

note:

There are many ways to fix this. I believe this is one. It disables i2c_init() until relocation is complete. I have been unable to test it so far due to problems getting my Seaboard to work. I will try another Tegra board, but send this for comment in the meantime.

(On Beaver and Dalmore, tested booting to U-Boot command prompt followed by "i2c dev 0; i2c probe")

Note: I believe this is an enormous hack that hacks around the problem of dynamic device initialization just not being well thought out relative to the restrictions of U-Boot's various boot stages. I'd still prefer an outright revert of the broken code.

In other words, tegra_i2c_init() simply shouldn't be called at the wrong time; it shouldn't have to handle being called at the wrong time and null itself out when that happens.

However, if this is what it takes to get U-Boot working again, then let's apply it ASAP.

#2 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

Hi Stephen,

Quoted 23 lines · Stephen Warren
Quoted 7 lines · Simon Glass

Tegra recently moved to the new I2C framework, which sets up I2C prior to relocation, and prior to calling i2c_init_board(). This causes a crash on Tegra boards.

note:

There are many ways to fix this. I believe this is one. It disables

i2c_init()

Quoted 3 lines

until relocation is complete. I have been unable to test it so far due to problems getting my Seaboard to work. I will try another Tegra board, but send this for comment in the meantime.

(On Beaver and Dalmore, tested booting to U-Boot command prompt followed by "i2c dev 0; i2c probe")

Note: I believe this is an enormous hack that hacks around the problem of dynamic device initialization just not being well thought out relative to the restrictions of U-Boot's various boot stages. I'd still prefer an outright revert of the broken code.

Quoted 7 lines

In other words, tegra_i2c_init() simply shouldn't be called at the wrong time; it shouldn't have to handle being called at the wrong time and null itself out when that happens.

However, if this is what it takes to get U-Boot working again, then let's apply it ASAP.

Fair enough.

Restricting my comments to I2C only, I think that we could have (at least until driver model happens) an I2C init for each driver (in this case it is i2c_init_board() that is called by the I2C subsystem twice during init (once from board_init_f() and once from board_init_r()). The i2c_init() call is odd because it does an init of a single I2C bus, which isn't really necessary for most hardware. But I'm not sure it is worth worrying about this until we have driver model.

Regards,
Simon

#3 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

In reply to Stephen Warren
Quoted 22 lines · Stephen Warren
Quoted 10 lines · Simon Glass

Tegra recently moved to the new I2C framework, which sets up I2C prior to relocation, and prior to calling i2c_init_board(). This causes a crash on Tegra boards.

note:

There are many ways to fix this. I believe this is one. It disables i2c_init() until relocation is complete. I have been unable to test it so far due to problems getting my Seaboard to work. I will try another Tegra board, but send this for comment in the meantime.

(On Beaver and Dalmore, tested booting to U-Boot command prompt followed by "i2c dev 0; i2c probe")

Note: I believe this is an enormous hack that hacks around the problem of dynamic device initialization just not being well thought out relative to the restrictions of U-Boot's various boot stages. I'd still prefer an outright revert of the broken code.

Quoted 6 lines

In other words, tegra_i2c_init() simply shouldn't be called at the wrong time; it shouldn't have to handle being called at the wrong time and null itself out when that happens.

However, if this is what it takes to get U-Boot working again, then let's apply it ASAP.

This doesn't seem to have been applied yet. Are you expecting this to go through the main U-boot Tree, I2C tree, or Tegra tree? I just noticed that you only CC'd the Tegra maintainer...

#4 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

+Tom Rini

Hi Stephen,

Quoted 32 lines · Stephen Warren
Quoted 2 lines · Stephen Warren
Quoted 1 line · Simon Glass

Tegra recently moved to the new I2C framework, which sets up I2C prior

to

Quoted 1 line
Quoted 1 line

relocation, and prior to calling i2c_init_board(). This causes a crash

on

Quoted 5 lines
Quoted 5 lines

Tegra boards.

note:

There are many ways to fix this. I believe this is one. It disables

i2c_init()

Quoted 1 line
Quoted 1 line

until relocation is complete. I have been unable to test it so far due

to

Quoted 1 line
Quoted 1 line

problems getting my Seaboard to work. I will try another Tegra board,

but

Quoted 16 lines
Quoted 1 line

send this for comment in the meantime.

(On Beaver and Dalmore, tested booting to U-Boot command prompt followed by "i2c dev 0; i2c probe")

Note: I believe this is an enormous hack that hacks around the problem of dynamic device initialization just not being well thought out relative to the restrictions of U-Boot's various boot stages. I'd still prefer an outright revert of the broken code.

In other words, tegra_i2c_init() simply shouldn't be called at the wrong time; it shouldn't have to handle being called at the wrong time and null itself out when that happens.

Quoted 8 lines
Quoted 2 lines

However, if this is what it takes to get U-Boot working again, then let's apply it ASAP.

This doesn't seem to have been applied yet. Are you expecting this to go through the main U-boot Tree, I2C tree, or Tegra tree? I just noticed that you only CC'd the Tegra maintainer...

I put tegra: on the front expecting it to go that way, but it doesn't matter. Also your comments did not exactly represent a glowing recommendation.

Tom (Rini) are you willing to pick this up as a bug fix please?

Regards,
Simon

#5 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

Simon,

From: [email protected] [mailto:sjg at google.com] On Behalf Of Simon Glass
Sent: Friday, August 09, 2013 9:04 PM
To: Stephen Warren
Cc: U-Boot Mailing List; Tom Warren; Stephen Warren; trini at ti.com
Subject: Re: [PATCH] RFC: tegra: Avoid using I2C prior to relocation

+Tom Rini

Hi Stephen,

On Fri, Aug 9, 2013 at 5:17 PM, Stephen Warren <swarren at wwwdotorg.org<mailto:[email protected]>> wrote:

Quoted 22 lines · Stephen Warren
Quoted 10 lines · Simon Glass

Tegra recently moved to the new I2C framework, which sets up I2C prior to relocation, and prior to calling i2c_init_board(). This causes a crash on Tegra boards.

note:

There are many ways to fix this. I believe this is one. It disables i2c_init() until relocation is complete. I have been unable to test it so far due to problems getting my Seaboard to work. I will try another Tegra board, but send this for comment in the meantime.

Tested-by Stephen Warren <swarren at nvidia.com<mailto:[email protected]>>

(On Beaver and Dalmore, tested booting to U-Boot command prompt followed by "i2c dev 0; i2c probe")

Note: I believe this is an enormous hack that hacks around the problem of dynamic device initialization just not being well thought out relative to the restrictions of U-Boot's various boot stages. I'd still prefer an outright revert of the broken code.

Quoted 6 lines

In other words, tegra_i2c_init() simply shouldn't be called at the wrong time; it shouldn't have to handle being called at the wrong time and null itself out when that happens.

However, if this is what it takes to get U-Boot working again, then let's apply it ASAP.

This doesn't seem to have been applied yet. Are you expecting this to go through the main U-boot Tree, I2C tree, or Tegra tree? I just noticed that you only CC'd the Tegra maintainer...

I put tegra: on the front expecting it to go that way, but it doesn't matter. Also your comments did not exactly represent a glowing recommendation. [Tom] It's still marked RFC - doesn't that have to go away before anyone can pick it up / apply it?

Tom (Rini) are you willing to pick this up as a bug fix please?

Regards,
Simon

--
nvpublic

#6 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

Hi Tom,

Quoted 59 lines · Tom Warren

Simon,

From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
Sent: Friday, August 09, 2013 9:04 PM
To: Stephen Warren
Cc: U-Boot Mailing List; Tom Warren; Stephen Warren; trini at ti.com
Subject: Re: [PATCH] RFC: tegra: Avoid using I2C prior to relocation

+Tom Rini

Hi Stephen,

On Fri, Aug 9, 2013 at 5:17 PM, Stephen Warren <[email protected]> wrote:

Quoted 22 lines · Stephen Warren
Quoted 10 lines · Simon Glass

Tegra recently moved to the new I2C framework, which sets up I2C prior to relocation, and prior to calling i2c_init_board(). This causes a crash on Tegra boards.

note:

There are many ways to fix this. I believe this is one. It disables i2c_init() until relocation is complete. I have been unable to test it so far due to problems getting my Seaboard to work. I will try another Tegra board, but send this for comment in the meantime.

(On Beaver and Dalmore, tested booting to U-Boot command prompt followed by "i2c dev 0; i2c probe")

Note: I believe this is an enormous hack that hacks around the problem of dynamic device initialization just not being well thought out relative to the restrictions of U-Boot's various boot stages. I'd still prefer an outright revert of the broken code.

Quoted 6 lines

In other words, tegra_i2c_init() simply shouldn't be called at the wrong time; it shouldn't have to handle being called at the wrong time and null itself out when that happens.

However, if this is what it takes to get U-Boot working again, then let's apply it ASAP.

This doesn't seem to have been applied yet. Are you expecting this to go through the main U-boot Tree, I2C tree, or Tegra tree? I just noticed that you only CC'd the Tegra maintainer...

Quoted 3 lines

I put tegra: on the front expecting it to go that way, but it doesn't matter. Also your comments did not exactly represent a glowing recommendation.

[Tom] It?s still marked RFC ? doesn?t that have to go away before anyone can pick it up / apply it?

Sorry, I missed this in first reading due to the quoting. I will re-issue without RFC.

Regards,
Simon

#7 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

Quoted 7 lines · Simon Glass

Hi Tom,

Quoted 4 lines · Tom Warren

Simon,

From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass

...

Quoted 6 lines
Quoted 3 lines

I put tegra: on the front expecting it to go that way, but it doesn't matter. Also your comments did not exactly represent a glowing recommendation.

[Tom] It?s still marked RFC ? doesn?t that have to go away before anyone can pick it up / apply it?

Sorry, I missed this in first reading due to the quoting. I will re-issue without RFC.

If that's the only change, you hardly need to repost it to edit the patch subject; there are many ways of fixing that when applying the patch...

#8 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

In reply to Stephen Warren
Quoted 5 lines · Stephen Warren
Quoted 4 lines · Simon Glass

Tegra recently moved to the new I2C framework, which sets up I2C prior to relocation, and prior to calling i2c_init_board(). This causes a crash on Tegra boards.

Quoted 8 lines
Quoted 6 lines

note:

There are many ways to fix this. I believe this is one. It disables i2c_init() until relocation is complete. I have been unable to test it so far due to problems getting my Seaboard to work. I will try another Tegra board, but send this for comment in the meantime.

With a hand-tweaked commit message, applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130813/b9c19a05/attachment.pgp>
#9 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

Quoted 14 lines · Tom Rini
Quoted 13 lines · Stephen Warren
Quoted 11 lines · Simon Glass

Tegra recently moved to the new I2C framework, which sets up I2C prior to relocation, and prior to calling i2c_init_board(). This causes a crash on Tegra boards.

note:

There are many ways to fix this. I believe this is one. It disables i2c_init() until relocation is complete. I have been unable to test it so far due to problems getting my Seaboard to work. I will try another Tegra board, but send this for comment in the meantime.

Quoted 4 lines
Quoted 1 line

With a hand-tweaked commit message, applied to u-boot/master, thanks!

Thanks!

For the record, I tested u-boot.git/master commit:

cdce889 tegra: Avoid using I2C prior to relocation

on Beaver, and booted a kernel both from SD card and over USB Ethernet, and both cases worked fine. So, we have a known-good commit for any future bisects:-)

#10 lore raw

PATCH 1/1 RFC: tegra: Avoid using I2C prior to relocation

Hi Stephen,

Quoted 22 lines · Stephen Warren
Quoted 14 lines · Tom Rini
Quoted 13 lines · Stephen Warren
Quoted 11 lines · Simon Glass

Tegra recently moved to the new I2C framework, which sets up I2C prior to relocation, and prior to calling i2c_init_board(). This causes a crash on Tegra boards.

note:

There are many ways to fix this. I believe this is one. It disables i2c_init() until relocation is complete. I have been unable to test it so far due to problems getting my Seaboard to work. I will try another Tegra board, but send this for comment in the meantime.

Quoted 4 lines
Quoted 1 line

With a hand-tweaked commit message, applied to u-boot/master, thanks!

Thanks!

Quoted 7 lines

For the record, I tested u-boot.git/master commit:

cdce889 tegra: Avoid using I2C prior to relocation

on Beaver, and booted a kernel both from SD card and over USB Ethernet, and both cases worked fine. So, we have a known-good commit for any future bisects:-)

OK that's good, thanks.

Regards,
Simon