On 11/12/2018 18:10, Stephen Warren wrote:
On 12/11/18 2:44 AM, Jean-Jacques Hiblot wrote:
On 11/12/2018 05:41, Heiko Schocher wrote:
Hello Stephen,
Am 10.12.2018 um 19:23 schrieb Stephen Warren:
The following commit:
dm: i2c: Make i2c_get_chip_for_busnum() fail if the chip is not
detected
i2c_get_chip_for_busnum() really should check the presence of
the chip on
the bus. Most of the users of this function assume that this
is done.
... causes a boot failure on NVIDIA Jetson TX2:
:-(
Thanks for detecting so fast!
U-Boot 2019.01-rc1-00220-g7ff485c68b7e (Dec 10 2018 - 11:20:41 -0700)
TEGRA186
Model: NVIDIA P2771-0000-500
DRAM: 7.8 GiB
tegra_ivc_read_get_next_frame() timed out (-12)
tegra_board_init: Cannot find MAX77620 I2C chip
initcall sequence 00000000fffa95a8 failed at call 0000000080083480
(err=-110)
### ERROR ### Please RESET the board ###
This may be due to the fact the bus in question is implemented by
RPC to a separate CPU, and that mechanism hasn't been used with
probing before. In general though, there's not guarantee that
probing will work even on a local/native I2C bus, since different
chips don't support all probe methods (see i2c-detect in Linux,
which supports various different probing methods due to this), so
I'm rather surprised this change was implemented. Is it really
necessary? I believe we should revert it.
The probe method is not the same in u-boot as in i2c-detect. In
u-boot there is no data transfer, the probe only sends the address on
the bus and fails if the device does not respond with a ACK (or if
something else goes wrong). Every I2C device supports this kind of
probe by design.
Errors could happen though:
- device not present, or not powered up or in reset state
- bus not ready (in your case, maybe the CPU doing the actual work is
not ready)
- bus speed too high.
In all those cases this could be fixed in the board specific code.
While I agree that a commit should not break platforms, I'm not
convinced that reverting the commit is the right solution: in
tegra_board_init() the call to i2c_get_chip_for_busnum() is followed
by a call to dm_i2c_write(). Assuming that we remove the offending
commit, i2c_get_chip_for_busnum() would not fail anymore, but in this
case the following call to dm_i2c_write() should fail. If it doesn't
then I suspect that there is something wrong in the tegra I2C bus
driver that makes it unable to transfer only the address word.
Yes, I imagine that our other CPU doesn't support zero-length
transfers. However, that's not going to change. Our only choice is not
to do this unnecessary probing.
Even if we were going to modify the Tegra I2C bus driver to solve or
work around this, we would still need to:
a) Revert the change.
b) Develop the fix.
c) Re-apply the original change.
... to reduce the time window where the code is broken. Right now
everyone working on Tegra U-Boot is rather swamped so spending time on
fixing this regression is rather annoying...
How about implementing the probe_chip() callback.
diff --git a/drivers/i2c/tegra186_bpmp_i2c.c
b/drivers/i2c/tegra186_bpmp_i2c.c
index b4fff43..6256d27 100644
--- a/drivers/i2c/tegra186_bpmp_i2c.c
+++ b/drivers/i2c/tegra186_bpmp_i2c.c
@@ -85,6 +85,11 @@ static int tegra186_bpmp_i2c_xfer(struct udevice
*dev, struct i2c_msg *msg,
return 0;
}
+static tegra186_bpmp_probe_chip(struct udevice *bus, uint chip_addr,
uint chip_flags)
+{
+ return 0;
+}
+
static int tegra186_bpmp_i2c_probe(struct udevice *dev)
{
struct tegra186_bpmp_i2c *priv = dev_get_priv(dev);
@@ -101,6 +106,7 @@ static int tegra186_bpmp_i2c_probe(struct udevice *dev)
static const struct dm_i2c_ops tegra186_bpmp_i2c_ops = {
.xfer = tegra186_bpmp_i2c_xfer,
+ .probe_chip = tegra186_bpmp_probe_chip,
};
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
