Hello Stephen,
Am 31.07.2013 21:31, schrieb Stephen Warren:
On 07/30/2013 11:46 PM, Heiko Schocher wrote: ...
Hmm.. each i2c adapter has its own init function ... why the tegra driver do not use it? And do the necessary inits in it? So we initialize an adapater only if we use it, which is also a rule for u-boot ...
I have no hw, but it should be possible to add to process_nodes() a parameter "controller_number" and call process_nodes(controller_number, ...) from the i2c drivers init function ...
There are two steps to initializing I2C on a Tegra system:
- Parsing the DT to determine which/how-many I2C adapters exist in the
SoC. This will yield information such as the register address of the I2C adapters, which clock/reset signal they rely on, perhaps the max bus clock rate, etc.
This is a single global operation that needs to happen one single time before anything else.
Why?
This stage initializes the i2c_controllers[] array, since that's what stores all the data parsed from DT.
- Actually initializing or using the I2C HW. That could certainly be
part of the per-I2C-controller init() function you mention.
For that purpose is the i2c_init function.
And why we could not do step 1 when we do step 2? Why doing step 1 for hw we later do not use?
saying something like this (just as an example, should be tested):
diff --git a/drivers/i2c/tegra_i2c.c b/drivers/i2c/tegra_i2c.c index 9ac3969..7bbd3c7 100644 --- a/drivers/i2c/tegra_i2c.c +++ b/drivers/i2c/tegra_i2c.c @@ -378,81 +378,91 @@ static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus) * @param is_scs 1 if this HW uses a single clock source (T114+) * @return 0 if ok, -1 on error */ -static int process_nodes(const void *blob, int node_list[], int count, +static int process_nodes(const void *blob, int node_list[], + struct i2c_adapter *adap, int count, int is_dvc, int is_scs) { struct i2c_bus *i2c_bus; - int i; - - /* build the i2c_controllers[] for each controller */ - for (i = 0; i < count; i++) { - int node = node_list[i]; + int node = node_list[i];
- if (node <= 0) - continue; + bus = &i2c_controllers[adap->hwadapnr]; + i2c_bus->id = adap->hwadapnr;
- i2c_bus = &i2c_controllers[i]; - i2c_bus->id = i; + if (node <= 0) + continue;
- if (i2c_get_config(blob, node, i2c_bus)) { - printf("i2c_init_board: failed to decode bus %d\n", i); - return -1; - } + if (i2c_get_config(blob, node, i2c_bus)) { + printf("i2c_init_board: failed to decode bus %d\n", i); + return -1; + }
- i2c_bus->is_scs = is_scs; + i2c_bus->is_scs = is_scs;
- i2c_bus->is_dvc = is_dvc; - if (is_dvc) { - i2c_bus->control = - &((struct dvc_ctlr *)i2c_bus->regs)->control; - } else { - i2c_bus->control = &i2c_bus->regs->control; - } - debug("%s: controller bus %d at %p, periph_id %d, speed %d: ", - is_dvc ? "dvc" : "i2c", i, i2c_bus->regs, - i2c_bus->periph_id, i2c_bus->speed); - i2c_init_controller(i2c_bus); - debug("ok\n"); - i2c_bus->inited = 1; - - /* Mark position as used */ - node_list[i] = -1; + i2c_bus->is_dvc = is_dvc; + if (is_dvc) { + i2c_bus->control = + &((struct dvc_ctlr *)i2c_bus->regs)->control; + } else { + i2c_bus->control = &i2c_bus->regs->control; } + debug("%s: controller bus %d at %p, periph_id %d, speed %d: ", + is_dvc ? "dvc" : "i2c", i, i2c_bus->regs, + i2c_bus->periph_id, i2c_bus->speed); + i2c_init_controller(i2c_bus); + debug("ok\n"); + i2c_bus->inited = 1; + + /* Mark position as used */ + node_list[i] = -1;
return 0; }
/* Sadly there is no error return from this function */ -void i2c_init_board(void) +static int tegra_i2c_init_board(struct i2c_adapter *adap) { + struct i2c_bus *i2c_bus; int node_list[TEGRA_I2C_NUM_CONTROLLERS]; const void *blob = gd->fdt_blob; int count;
+ bus = tegra_i2c_get_bus(adap); + if (bus) + return 0; + /* First check for newer (T114+) I2C ports */ count = fdtdec_find_aliases_for_id(blob, "i2c", COMPAT_NVIDIA_TEGRA114_I2C, node_list, TEGRA_I2C_NUM_CONTROLLERS); - if (process_nodes(blob, node_list, count, 0, 1)) - return; + if (process_nodes(blob, node_list, adap, count, 0, 1)) + return -1;
/* Now get the older (T20/T30) normal I2C ports */ count = fdtdec_find_aliases_for_id(blob, "i2c", COMPAT_NVIDIA_TEGRA20_I2C, node_list, TEGRA_I2C_NUM_CONTROLLERS); - if (process_nodes(blob, node_list, count, 0, 0)) - return; + if (process_nodes(blob, node_list, adap, count, 0, 0)) + return -1;
/* Now look for dvc ports */ count = fdtdec_add_aliases_for_id(blob, "i2c", COMPAT_NVIDIA_TEGRA20_DVC, node_list, TEGRA_I2C_NUM_CONTROLLERS); - if (process_nodes(blob, node_list, count, 1, 0)) - return; + if (process_nodes(blob, node_list, adap, count, 1, 0)) + return -1; + + return 0; }
static void tegra_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr) { + int ret; + + ret = tegra_i2c_init_board(adap); + if (ret) { + printf("Could not decode bus: %d\n", adap->hwadapnr); + 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);
Which will call process_nodes() from the i2c_init function, which only is called, when i2c bus get used ...
Now, I think the big disconnect here is that historically, the U-Boot binary has hard-coded all the details that step (1) above parses out of DT, so that step (1) did not need to exist.
However, Simon has been pushing Tegra towards not hard-coding any of this information, but instead having a single binary that can work on arbitrary Tegra boards and even across different Tegra SoCs which have a different number of I2C controllers at different register addresses. This is quite fundamentally different to how U-Boot has worked in the past, and evidently has some problems fitting into the typical U-Boot initialization sequence.
Yes ... but again, if we parse the DT in the moment we need to init a hw, it would fit again (at least for the dt case). But we have a problem, if we need to write (like the i2c_controllers[] array) before relocation. So I2C and DT usage is not possible before relocation. (And not only i2c in conjunction with dt I think)
bye, Heiko