Hello Simon,

On 05.11.2012 21:43, Simon Glass wrote:
Hi Heiko,

On Tue, Oct 30, 2012 at 10:26 PM, Heiko Schocher<h...@denx.de>  wrote:
Hello Simon,


On 30.10.2012 18:28, Simon Glass wrote:

Rather than using a variable in various places, add a single function,
tegra_i2c_get_bus(), which returns a pointer to information about a
bus.

This will make it easier to move to the new i2c framework.

Signed-off-by: Simon Glass<s...@chromium.org>
---
   drivers/i2c/tegra_i2c.c |   78
++++++++++++++++++++++++++++++++++++----------
   1 files changed, 61 insertions(+), 17 deletions(-)


First question, did you tried the new i2c framework on some HW? Works it
for you?

Yes, it works fine. Tried on a seaboard.

Great, thanks for testing!

diff --git a/drivers/i2c/tegra_i2c.c b/drivers/i2c/tegra_i2c.c
index efc77fa..3e157f4 100644
--- a/drivers/i2c/tegra_i2c.c
+++ b/drivers/i2c/tegra_i2c.c
[...]
so the probe/read/write and set_bus_speed() functions gets now a
"struct i2c_adapter" pointer ... i2c driver internally, we need only
the "struct i2c_adapter", which the i2c core passes to the i2c driver.
So this function would look like now:

+static struct i2c_bus *tegra_i2c_get_bus(struct i2c_adapter *adap)

Sounds good. Are you going to send a new patch?

Yes, want to do some more test ... and waiting for the result of the
vote, which direction we should go, see previous EMail ...

+{
+     struct i2c_bus *bus;
         ^^^^^^^^^^^^^^
         maybe a rename to "i2c_tegra_driver" would be good?
+     bus =&i2c_controllers[adap->hwadapnr];

+     if (!bus->inited) {
+             debug("%s: Bus %u not available\n", __func__, bus_num);
+             return NULL;
+     }
+
+     return bus;
+}

With this, we can get rid of i2c_bus_num in this driver.

Also the probe/read/write and set_bus_speed() functions needs to
be adapted. I can do this for you, and add this patchset to my
v2 post, if it is ok for you?

Of course.

Ok, do this ...

   unsigned int i2c_get_bus_speed(void)


This function is not needed for the new framework!
[...]

Yes, I thought I removed it in 2/2, perhaps not.


   int i2c_set_bus_speed(unsigned int speed)
   {
-       struct i2c_bus *i2c_bus;
+       struct i2c_bus *bus;

-       i2c_bus =&i2c_controllers[i2c_bus_num];

-       i2c_bus->speed = speed;
-       i2c_init_controller(i2c_bus);
+       bus = tegra_i2c_get_bus(i2c_bus_num);
+       if (!bus)
+               return 0;
+       bus->speed = speed;
+       i2c_init_controller(bus);

         return 0;
   }
@@ -426,7 +458,7 @@ void i2c_init(int speed, int slaveaddr)

[...]

Rest looks good to me.

Thanks!

Will await your further patch, and I am ready to test. Thanks.

Thanks!

bye,
Heiko
--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to