RE: [PATCH 16/24] omap4: Add i2c support on omap4 platform

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Tony Lindgren [mailto:tony@xxxxxxxxxxx]
> Sent: Wednesday, February 17, 2010 12:21 AM
> To: Shilimkar, Santosh
> Cc: linux-omap@xxxxxxxxxxxxxxx; ben@xxxxxxxxxxxx; sameo@xxxxxxxxxxxxxxx; lrg@xxxxxxxxxxxxxxx;
> paul@xxxxxxxxx; Syed, Rafiuddin; Ben Dooks
> Subject: Re: [PATCH 16/24] omap4: Add i2c support on omap4 platform
> 
> * Santosh Shilimkar <santosh.shilimkar@xxxxxx> [100216 07:55]:
> > This patch is rebased version of earlier post to add I2C
> > driver support to OMAP4 platform. On OMAP4, all
> > I2C register address offsets are changed from OMAP1/2/3 I2C.
> > In order to not have #ifdef's at various places in code,
> > as well as to support multi-OMAP build, an array is created
> > to hold the register addresses with it's offset.
> >
> > This patch was submitted, reviewed and acked on mailing list
> > already. For more details refer below link
> > http://www.mail-archive.com/linux-i2c@xxxxxxxxxxxxxxx/msg02281.html
> >
> > This version updated on top of 16 bit support added as part of
> > the commit "8bb209278e555a626f9637e844fe4c1b90e849f6"
> >
> > Signed-off-by: Syed Rafiuddin <rafiuddin.syed@xxxxxx>
> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@xxxxxx>
> > Acked-by: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
> > CC: Tony Lindgren <tony@xxxxxxxxxxx>
> > CC: Ben Dooks <ben-linux@xxxxxxxxx>
> > ---
> >  arch/arm/plat-omap/i2c.c      |   17 ++++-
> >  drivers/i2c/busses/i2c-omap.c |  148 ++++++++++++++++++++++++++++++++---------
> >  2 files changed, 131 insertions(+), 34 deletions(-)
> >
> > diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c
> > index 96d2781..f18d757 100644
> > --- a/arch/arm/plat-omap/i2c.c
> > +++ b/arch/arm/plat-omap/i2c.c
> > @@ -53,9 +53,15 @@ static struct resource i2c_resources[][2] = {
> >  #if	defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> >  	{ I2C_RESOURCE_BUILDER(OMAP2_I2C_BASE2, INT_24XX_I2C2_IRQ) },
> >  #endif
> > +#if	defined(CONFIG_ARCH_OMAP4)
> > +	{ I2C_RESOURCE_BUILDER(OMAP2_I2C_BASE2, INT_44XX_I2C2_IRQ) },
> > +#endif
> >  #if	defined(CONFIG_ARCH_OMAP3)
> >  	{ I2C_RESOURCE_BUILDER(OMAP2_I2C_BASE3, INT_34XX_I2C3_IRQ) },
> >  #endif
> > +#if	defined(CONFIG_ARCH_OMAP4)
> > +	{ I2C_RESOURCE_BUILDER(OMAP2_I2C_BASE3, INT_44XX_I2C3_IRQ) },
> > +#endif
> >  };
> 
> This is OK, but..
> 
> >  #define I2C_DEV_BUILDER(bus_id, res, data)		\
> > @@ -72,10 +78,11 @@ static struct resource i2c_resources[][2] = {
> >  static u32 i2c_rate[ARRAY_SIZE(i2c_resources)];
> >  static struct platform_device omap_i2c_devices[] = {
> >  	I2C_DEV_BUILDER(1, i2c_resources[0], &i2c_rate[0]),
> > -#if	defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> > +#if	defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3) || \
> > +	defined(CONFIG_ARCH_OMAP4)
> >  	I2C_DEV_BUILDER(2, i2c_resources[1], &i2c_rate[1]),
> >  #endif
> > -#if	defined(CONFIG_ARCH_OMAP3)
> > +#if	defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4)
> >  	I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_rate[2]),
> >  #endif
> >  };
> 
> .. the ifdefs above can be simplified to ifdef CONFIG_ARCH_OMAP2PLUS.
> That is, with the patches already queued into omap for-next.
YEP
> 
> > @@ -370,7 +434,11 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
> >  			internal_clk = 9600;
> >  		else
> >  			internal_clk = 4000;
> > -		fclk_rate = clk_get_rate(dev->fclk) / 1000;
> > +		/* FIXME: Remove this once clock framework is available*/
> > +		if (dev->rev >= OMAP_I2C_REV_ON_4430)
> > +			fclk_rate = 96000;
> > +		else
> > +			fclk_rate = clk_get_rate(dev->fclk) / 1000;
> >
> >  		/* Compute prescaler divisor */
> >  		psc = fclk_rate / internal_clk;
> 
> Sounds like this part should no longer be needed?
> 
Yes. I missed this
> >  					*dev->buf++ = w;
> >  					dev->buf_len--;
> > -					/* Data reg from 2430 is 8 bit wide */
> > +					/* Data reg in 2430, omap3 and
> > +					 * omap4 is 8 bit wide
> > +					 */
> >  					if (!cpu_is_omap2430() &&
> > -							!cpu_is_omap34xx()) {
> > +							!cpu_is_omap34xx() &&
> > +							!cpu_is_omap44xx()) {
> >  						if (dev->buf_len) {
> >  							*dev->buf++ = w >> 8;
> >  							dev->buf_len--;
> 
> How about change this to if (cpu_class_is_omap1() || cpu_is_omap2420())?
> That way it does not need to be patched for new omaps.
> 
> > @@ -786,9 +857,12 @@ complete:
> >  				if (dev->buf_len) {
> >  					w = *dev->buf++;
> >  					dev->buf_len--;
> > -					/* Data reg from  2430 is 8 bit wide */
> > +					/* Data reg in 2430, omap3 and
> > +					 * omap4 is 8 bit wide
> > +					 */
> >  					if (!cpu_is_omap2430() &&
> > -							!cpu_is_omap34xx()) {
> > +							!cpu_is_omap34xx() &&
> > +							!cpu_is_omap44xx()) {
> >  						if (dev->buf_len) {
> >  							w |= *dev->buf++ << 8;
> >  							dev->buf_len--;
> 
> Here too.
> 
> >  	dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff;
> >
> > -	if (cpu_is_omap2430() || cpu_is_omap34xx()) {
> > +	if (cpu_is_omap2430() || cpu_is_omap34xx() || cpu_is_omap44xx()) {
> >  		u16 s;
> >
> >  		/* Set up the fifo size - Get total size */
> 
> And here too.
> 
> Regards,
> 
> Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux