Hi Stephen, On Mon, Oct 29, 2012 at 8:34 AM, Stephen Warren <swar...@wwwdotorg.org> wrote: > On 10/29/2012 03:47 AM, Heiko Schocher wrote: >> Hello Stephen, >> >> On 26.10.2012 18:07, Stephen Warren wrote: >>> On 10/25/2012 11:48 PM, Heiko Schocher wrote: >>>> Hello Simon, >>>> >>>> On 25.10.2012 23:37, Simon Glass wrote: >>>>> On Mon, Oct 22, 2012 at 10:40 AM, Heiko Schocher<h...@denx.de> wrote: >>>>>> rebased/reworked the I2C multibus patches from Simon Glass found >>>>>> here: >>>>>> >>>>>> http://www.mail-archive.com/u-boot@lists.denx.de/msg75530.html >>>>>> >>>>>> It seems the timing is coming, to bring this in mainline and >>>>>> move boards over to the new i2c framework. As an example I >>>>>> converted the soft-i2c driver (and all boards using it) to >>>>>> the new framework, so this patchseries has to be tested >>>>>> intensively, as I can check compile only ... >>>>> >>>>> I am very happy to see this, thank you. >>>> >>>> I am too ;-) and Sorry that I am only now ready ... >>>> >>>>> I have brought this in and tried to get it running for Tegra. A few >>>>> points: >>>>> >>>>> 1. The methods in struct i2c_adapter should IMO be passed a struct >>>>> i2c_adapter *, so they can determine which adapter is being accessed. >>>>> Otherwise I can't see how they would know. >>>> >>>> They can get the current used adapter through the defines in >>>> include/i2c.h: >>>> [...] >>>> #define I2C_ADAP_NR(bus) i2c_adap[i2c_bus[bus].adapter] >>>> #define I2C_BUS ((struct i2c_bus_hose *)gd->cur_i2c_bus) >>>> #define I2C_ADAP i2c_adap[I2C_BUS->adapter] >>>> #define I2C_ADAP_HWNR (I2C_ADAP->hwadapnr) >>>> >>>> preparing just the fsl i2c driver and there I do for example: >>>> drivers/i2c/fsl_i2c.c >>>> [...] >>>> static const struct fsl_i2c *i2c_dev[2] = { >>>> (struct fsl_i2c *) (CONFIG_SYS_IMMR + >>>> CONFIG_SYS_FSL_I2C_OFFSET), >>>> #ifdef CONFIG_SYS_FSL_I2C2_OFFSET >>>> (struct fsl_i2c *) (CONFIG_SYS_IMMR + >>>> CONFIG_SYS_FSL_I2C2_OFFSET) >>>> #endif >>>> }; >>>> [...] >>>> static int fsl_i2c_probe(uchar chip) >>>> { >>>> struct fsl_i2c *dev = (struct fsl_i2c *)i2c_dev[I2C_ADAP_HWNR]; >>>> [...] >>>> >>>> but of course, we still can change the "struct i2c_adapter" if >>>> needed ... but we have one more parameter ... Ok, not really a bad >>>> problem. >>> >>> That rather relies on their being a concept of a "current" I2C adapter. >>> It seems a little limiting to require that. What if the "current" >>> adapter is the user-selected adapter for commands to operate on, but >>> e.g. some power-management driver wants to use I2C to communicate with a >>> PMIC during the internals of some other command. Sure, you could save >>> and later restore the I2C core's idea of "current" adapter, but it'd >>> surely be cleaner to just pass around the I2C adapter ID or struct >>> pointer everywhere to avoid the need for save/restore. >> >> Yes, you are right, but just the same problem with current code! >> You mixed here two things! > > I think you're reading more into what I was saying than what I actually > said. > > If there are e.g. 4 I2C controllers in an SoC, the driver needs to know > which one is in use. Passing that information directly to the driver > functions is much simple than requiring the SoC I2C driver to go grovel > in some I2C core global variables to find out the same information.
I think Heiko agreed with this, just that he wants to take things a step at a time. > > This is all unrelated to I2C bus muxes; they shouldn't be implemented as > part of an SoC I2C driver anyway, so the driver shouldn't know about bus > muxes before or after this patch - the I2C core should manage that. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
