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?
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
[...]
@@ -302,18 +304,48 @@ static int tegra_i2c_read_data(u32 addr, u8 *data, u32
len)
#error "Please enable device tree support to use this driver"
#endif
+/**
+ * Check that a bus number is valid and return a pointer to it
+ *
+ * @param bus_num Bus number to check / return
+ * @return pointer to bus, if valid, else NULL
+ */
+static struct i2c_bus *tegra_i2c_get_bus(unsigned int bus_num)
+{
+ struct i2c_bus *bus;
+
+ if (bus_num>= TEGRA_I2C_NUM_CONTROLLERS) {
+ debug("%s: Invalid bus number %u\n", __func__, bus_num);
+ return NULL;
+ }
+ bus =&i2c_controllers[bus_num];
+ if (!bus->inited) {
+ debug("%s: Bus %u not available\n", __func__, bus_num);
+ return NULL;
+ }
+
+ return bus;
+}
+
I added, as Stephen and you suggested, in the "struct i2c_adapter"
to the driver callbacks a new parameter "struct i2c_adapter *adap":
struct i2c_adapter {
void (*init)(struct i2c_adapter *adap, int speed,
int slaveaddr);
int (*probe)(struct i2c_adapter *adap, uint8_t chip);
int (*read)(struct i2c_adapter *adap, uint8_t chip,
uint addr, int alen, uint8_t *buffer,
int len);
int (*write)(struct i2c_adapter *adap, uint8_t chip,
uint addr, int alen, uint8_t *buffer,
int len);
uint (*set_bus_speed)(struct i2c_adapter *adap,
uint speed);
[...]
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)
> +{
> + 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?
unsigned int i2c_get_bus_speed(void)
This function is not needed for the new framework!
[...]
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!
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
