From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grant Likely Subject: Re: [PATCH v2] powerpc: i2c-mpc: make I2C bus speed configurable Date: Wed, 1 Apr 2009 07:47:21 -0600 Message-ID: References: <49D36885.9070806@grandegger.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@ozlabs.org Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@ozlabs.org To: Wolfgang Grandegger Cc: linuxppc-dev , devicetree-discuss , linux-i2c@vger.kernel.org, Ben Dooks List-Id: devicetree@vger.kernel.org On Wed, Apr 1, 2009 at 7:44 AM, Grant Likely wr= ote: > On Wed, Apr 1, 2009 at 7:13 AM, Wolfgang Grandegger w= rote: >> 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. =A0However, 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. =A0More comments below. Oh, and you should look in MAINTAINERS and cc this patch to Ben Dooks and the linux-i2c@vger.kernel.org mailing list. g. > >> -static void mpc_i2c_setclock(struct mpc_i2c *i2c) >> -{ >> - =A0 =A0 =A0 /* Set clock and filters */ >> - =A0 =A0 =A0 if (i2c->flags & FSL_I2C_DEV_SEPARATE_DFSRR) { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 writeb(0x31, i2c->base + MPC_I2C_FDR); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 writeb(0x10, i2c->base + MPC_I2C_DFSRR); >> - =A0 =A0 =A0 } else if (i2c->flags & FSL_I2C_DEV_CLOCK_5200) >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 writeb(0x3f, i2c->base + MPC_I2C_FDR); >> - =A0 =A0 =A0 else >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 writel(0x1031, i2c->base + MPC_I2C_FDR); >> +static void mpc_i2c_setclock_52xx(struct device_node *node, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct= mpc_i2c *i2c, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 u32 cl= ock, u32 prescaler) >> +{ >> + =A0 =A0 =A0 int fdr =3D mpc52xx_i2c_get_fdr(node, clock, prescaler); >> + >> + =A0 =A0 =A0 if (fdr < 0) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 fdr =3D 0x3f; /* backward compatibility */ >> + =A0 =A0 =A0 writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR); >> + =A0 =A0 =A0 dev_info(i2c->dev, "clock %d Hz (fdr=3D%d)\n", clock, fdr); >> +} >> + >> +static void mpc_i2c_setclock_8xxx(struct device_node *node, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct= mpc_i2c *i2c, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 u32 cl= ock, u32 prescaler) >> +{ >> + =A0 =A0 =A0 int fdr =3D fsl_i2c_get_fdr(node, clock, prescaler); >> + >> + =A0 =A0 =A0 if (fdr < 0) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 fdr =3D 0x1031; /* backward compatibility = */ >> + =A0 =A0 =A0 writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR); >> + =A0 =A0 =A0 writeb((fdr >> 8) & 0xff, i2c->base + MPC_I2C_DFSRR); >> + =A0 =A0 =A0 dev_info(i2c->dev, "clock %d Hz (dfsrr=3D%d fdr=3D%d)\n", >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0clock, fdr >> 8, fdr & 0xff); >> =A0} > > Very neat and tidy indeed. =A0I like this. > >> Index: linux-2.6/arch/powerpc/sysdev/fsl_soc.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- linux-2.6.orig/arch/powerpc/sysdev/fsl_soc.c =A0 =A0 =A0 =A02009-03-= 31 21:27:52.000000000 +0200 >> +++ linux-2.6/arch/powerpc/sysdev/fsl_soc.c =A0 =A0 2009-04-01 12:09:18.= 796719327 +0200 >> @@ -39,7 +39,7 @@ >> =A0#include >> =A0#include >> =A0#include >> - >> +#define DEBUG > =A0^^^^^^^^^^^^^^^^^^^^^^^ =A0oops? > > g. > > -- > Grant Likely, B.Sc., P.Eng. > Secret Lab Technologies Ltd. > -- = Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.