Hello Stephen, Stephen Warren wrote: > Simon Glass wrote ednesday, January 11, 2012 9:18 PM: >> On Mon, Jan 9, 2012 at 2:07 PM, Stephen Warren <swar...@nvidia.com> wrote: >>> On 12/26/2011 11:11 AM, Simon Glass wrote: >>>> From: Yen Lin <ye...@nvidia.com> >>>> >>>> Add basic i2c driver for Tegra2 with 8- and 16-bit address support. >>>> The driver supports building both with and without CONFIG_OF_CONTROL. >>>> >>>> Without CONFIG_OF_CONTROL a number of CONFIG options must be supplied >>>> in the board config header file: >>>> >>>> I2CSPEED_KHZ - speed to run I2C bus at (typically 100000) >>>> CONFIG_I2Cx_PIN_MUX - pin mux setting for each port (P, 1, 2, 3) >>>> (typically this will be 0 to bring the port out the common >>>> pins) > ... > >>> ... >>>> diff --git a/drivers/i2c/tegra2_i2c.c b/drivers/i2c/tegra2_i2c.c >>> ... >>>> +struct i2c_bus i2c_controllers[CONFIG_SYS_MAX_I2C_BUS]; >>> What if there are I2C bus extenders/muxes/... such that there are more >>> I2C buses in the system than Tegra I2C controllers? I'd rather see an >>> explicit TEGRA_I2C_NUM_CONTROLLERS define used throughout this patch. >> We don't actually support CONFIG_I2C_MUX, so I can't see how that >> could happen. Can you please explain a bit more? > > We may not support it now, but I see no reason we won't in the future. > If we confuse the two defines now, it'll make it harder to allow muxes > in the future. The fix is simply using the correct define name within > the I2C driver isn't it; pretty simple.
If we really have this case, we *must* get the multibus/multiadapter branch from here: http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2_20111112 to uboot mainline! With that approach we could handle this case in a clean way ... This branch is not in sync with current TOT, and it has to be tested again, as I didn't found time for this in the last year :-( Also, as its base is >2 years old, there are a lot of checkpatch errors in this patchserie, which have to be cleaned up ... hope I get some time for it ... help is welcome. >>>> +int i2c_init_board(void) >>>> +{ [...] >>> Surely that function needs to actually do something, at least set up the >>> clocks so that the (user?) requested rate is honored, or print an error >>> message if you're only allowed to use the hard-coded bus rate. >> See my other message. I suppose we could reinit, but we really don't >> want to honour the speed, since the fdt speed setting is then lost and >> irrecoverable. For now it feels like we should ignore it. > > Hmm. I suspect the answer here is roughly to override the following in > cmd_i2c.c: > > /* TODO: Implement architecture-specific get/set functions */ > unsigned int __def_i2c_get_bus_speed(void) > { > return CONFIG_SYS_I2C_SPEED; > } > unsigned int i2c_get_bus_speed(void) > __attribute__((weak, alias("__def_i2c_get_bus_speed"))); > > int __def_i2c_set_bus_speed(unsigned int speed) > { > if (speed != CONFIG_SYS_I2C_SPEED) > return -1; > > return 0; > } > int i2c_set_bus_speed(unsigned int) > __attribute__((weak, alias("__def_i2c_set_bus_speed"))); > > To actually read/write the rate in use by the driver. Yep, for this reason this functions are weak, and you can define a driver specific function which returns the current used settings in the driver. Code should compile, if tegra2 not define CONFIG_SYS_I2C_SPEED, as a "default" value for CONFIG_SYS_I2C_SPEED is defined in include/i2c.h ... > Then, fix do_i2c_reset() to use i2c_get_bus_speed() so it interacts > correctly with those functions. Yep, I think, you are right here, that should be fixed. As the default function for i2c_get_bus_speed returns CONFIG_SYS_I2C_SPEED, that should be OK. > There may be other places that need to be updates to use those function > instead of hard-coding CONFIG_SYS_I2C_SPEED too. Perhaps we could even > get away without defining CONFIG_SYS_I2C_SPEED for Tegra since it's > meaningless. Instead, ifdef those default function definitions above > based on whether CONFIG_SYS_I2C_SPEED is defined or not. > 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
