[U-Boot] [PATCH v3 8/9] tegra: i2c: Enable new CONFIG_SYS_I2C framework
Heiko Schocher
hs at denx.de
Thu Aug 1 06:32:11 CEST 2013
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:
>
> 1) 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.
>
> 2) 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
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
More information about the U-Boot
mailing list