From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Thu, 01 Aug 2013 06:32:11 +0200 Subject: [U-Boot] [PATCH v3 8/9] tegra: i2c: Enable new CONFIG_SYS_I2C framework In-Reply-To: <51F9662D.1070602@wwwdotorg.org> References: <1367668903-29653-1-git-send-email-hs@denx.de> <1367668903-29653-9-git-send-email-hs@denx.de> <51F6946F.8010500@wwwdotorg.org> <51F740FD.4080505@denx.de> <51F81261.7020008@wwwdotorg.org> <51F81B51.6010305@wwwdotorg.org> <51F83104.5090508@wwwdotorg.org> <51F83570.1010909@wwwdotorg.org> <51F8A4CA.7070702@denx.de> <51F9662D.1070602@wwwdotorg.org> Message-ID: <51F9E4CB.9040004@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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