Grant Likely wrote: > On Wed, Apr 1, 2009 at 7:44 AM, Grant Likely <grant.likely@xxxxxxxxxxxx> wrote: >> On Wed, Apr 1, 2009 at 7:13 AM, Wolfgang Grandegger <wg@xxxxxxxxxxxxxx> wrote: >>> This patch makes the I2C bus speed configurable by using the I2C node >>> property "clock-frequency". If the property is not defined, the old >>> fixed clock settings will be used for backward compatibility. >> I mostly like this patch. However, as I mentioned in my other reply, >> I'm going to NAK it because the divisor tables are not common code and >> do not belong in common code files. >> >> Other than that, I like this version of the patch and I think it is >> almost finished. More comments below. Looks good now, indeed. > Oh, and you should look in MAINTAINERS and cc this patch to Ben Dooks > and the linux-i2c@xxxxxxxxxxxxxxx mailing list. Right, and I need to separate I2C and PowerPC related patches. And it will require another round but some acked-by lines will help. >>> -static void mpc_i2c_setclock(struct mpc_i2c *i2c) >>> -{ >>> - /* Set clock and filters */ >>> - if (i2c->flags & FSL_I2C_DEV_SEPARATE_DFSRR) { >>> - writeb(0x31, i2c->base + MPC_I2C_FDR); >>> - writeb(0x10, i2c->base + MPC_I2C_DFSRR); >>> - } else if (i2c->flags & FSL_I2C_DEV_CLOCK_5200) >>> - writeb(0x3f, i2c->base + MPC_I2C_FDR); >>> - else >>> - writel(0x1031, i2c->base + MPC_I2C_FDR); >>> +static void mpc_i2c_setclock_52xx(struct device_node *node, >>> + struct mpc_i2c *i2c, >>> + u32 clock, u32 prescaler) >>> +{ >>> + int fdr = mpc52xx_i2c_get_fdr(node, clock, prescaler); >>> + >>> + if (fdr < 0) >>> + fdr = 0x3f; /* backward compatibility */ >>> + writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR); >>> + dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr); >>> +} >>> + >>> +static void mpc_i2c_setclock_8xxx(struct device_node *node, >>> + struct mpc_i2c *i2c, >>> + u32 clock, u32 prescaler) >>> +{ >>> + int fdr = fsl_i2c_get_fdr(node, clock, prescaler); >>> + >>> + if (fdr < 0) >>> + fdr = 0x1031; /* backward compatibility */ >>> + writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR); >>> + writeb((fdr >> 8) & 0xff, i2c->base + MPC_I2C_DFSRR); >>> + dev_info(i2c->dev, "clock %d Hz (dfsrr=%d fdr=%d)\n", >>> + clock, fdr >> 8, fdr & 0xff); >>> } >> Very neat and tidy indeed. I like this. Yep. >>> Index: linux-2.6/arch/powerpc/sysdev/fsl_soc.c >>> =================================================================== >>> --- linux-2.6.orig/arch/powerpc/sysdev/fsl_soc.c 2009-03-31 21:27:52.000000000 +0200 >>> +++ linux-2.6/arch/powerpc/sysdev/fsl_soc.c 2009-04-01 12:09:18.796719327 +0200 >>> @@ -39,7 +39,7 @@ >>> #include <sysdev/fsl_soc.h> >>> #include <mm/mmu_decl.h> >>> #include <asm/cpm2.h> >>> - >>> +#define DEBUG >> ^^^^^^^^^^^^^^^^^^^^^^^ oops? Will fix. Wolfgang. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html