[RFC-PATCH] Move i2c adaptor speed from a module parameter to platform device resource

[RFC-PATCH] Move i2c adaptor speed from a module parameter to platform device resource

[nishanth menon]

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

Hi All,

Please find attached patch to move the i2c speed parameter from module
to bus specific resource.

To Discuss:
* Is IORESOURCE_IO the best way of defining speed for i2c bus?

This, however, requires the following follow up patches:
* Move i2c platform from plat-omap to respective board files.
*    Reasoning: I2C speed at 100Khz is not always optimal for I2C1 by
*      default (e.g. 3430).
*    Impact: all board files will need to have it...but it will ensure
*    that the i2c init is done in the proper sequence - 1 followed by 2.
* Support FIFO mode of data transfer - this is supported on newer I2C h/w
*     revs
*    Options: support all in a single driver/split them up? have i2c-omap.c
*     and i2c-omap2.c?
* Support High speed transfers at a bus level
*    Will depend of FIFO implementation as polled/interrupt mode is too
*     slow and causes underrun.
* Support all known Erratas for i2c module.. make features
*    i2c_module_hardware_revision based.
* Support proper bus speed support
*    Multiple devices on the same bus can have varied speed requirements.
*    Infact speed could be a parameter for i2c messages?
*    Impact: I2c corelevel changes required.i2c list disc.

Regards,
Nishanth Menon

[-- Attachment #2: 0001-Move-i2c-adaptor-speed-from-a-module-parameter-to-pl.patch --]
[-- Type: application/octet-stream, Size: 5657 bytes --]

From 31f046247db8d68214812afae3bb457b3dea89fc Mon Sep 17 00:00:00 2001
From: Nishanth Menon <menon.nishanth@gmail.com>
Date: Mon, 2 Apr 2007 19:07:58 -0500
Subject: [PATCH] Move i2c adaptor speed from a module parameter to platform device resource
To Discuss:
* Is IORESOURCE_IO the best way of defining speed for i2c bus?

This, however, requires the following follow up patches:
* Move i2c platform from plat-omap to respective board files.
*    Reasoning: I2C speed at 100Khz is not always optimal for I2C1 by
*      default (e.g. 3430).
*    Impact: all board files will need to have it...but it will ensure
*    that the i2c init is done in the proper sequence - 1 followed by 2.
* Support FIFO mode of data transfer - this is supported on newer I2C h/w
*     revs
*    Options: support all in a single driver/split them up? have i2c-omap.c
*     and i2c-omap2.c?
* Support High speed transfers at a bus level
*    Will depend of FIFO implementation as polled/interrupt mode is too
*     slow and causes underrun.
* Support all known Erratas for i2c module.. make features
*    i2c_module_hardware_revision based.
* Support proper bus speed support
*    Multiple devices on the same bus can have varied speed requirements.
*    Infact speed could be a parameter for i2c messages?
*    Impact: I2c corelevel changes required.i2c list disc.
---
 arch/arm/mach-omap2/devices.c |    6 +++++-
 arch/arm/plat-omap/devices.c  |    4 ++++
 drivers/i2c/busses/i2c-omap.c |   34 ++++++++++++++++++----------------
 3 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 767140c..0c0c2e8 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -37,6 +37,10 @@ static struct resource i2c_resources2[] = {
 		.flags		= IORESOURCE_MEM,
 	},
 	{
+		.end		= 3400,
+		.flags		= IORESOURCE_IO,
+	},
+	{
 		.start		= OMAP2_I2C_INT2,
 		.flags		= IORESOURCE_IRQ,
 	},
@@ -60,7 +64,7 @@ static void omap_init_i2c(void)
 		omap_cfg_reg(J15_24XX_I2C2_SCL);
 		omap_cfg_reg(H19_24XX_I2C2_SDA);
 	}
-	(void) platform_device_register(&omap_i2c_device2);
+	(void)platform_device_register(&omap_i2c_device2);
 }
 
 #else
diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
index 27b5353..771ca6f 100644
--- a/arch/arm/plat-omap/devices.c
+++ b/arch/arm/plat-omap/devices.c
@@ -107,6 +107,10 @@ static struct resource i2c_resources1[] = {
 		.start		= 0,
 		.flags		= IORESOURCE_IRQ,
 	},
+	{
+		.end		= 100,
+		.flags		= IORESOURCE_IO,
+	},
 };
 
 /* DMA not used; works around erratum writing to non-empty i2c fifo */
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 5308d67..b20509e 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -113,16 +113,11 @@
 /* I2C System Configuration Register (OMAP_I2C_SYSC): */
 #define OMAP_I2C_SYSC_SRST		(1 << 1)	/* Soft Reset */
 
-/* REVISIT: Use platform_data instead of module parameters */
-/* Fast Mode = 400 kHz, Standard = 100 kHz */
-static int clock = 100; /* Default: 100 kHz */
-module_param(clock, int, 0);
-MODULE_PARM_DESC(clock, "Set I2C clock in kHz: 400=fast mode (default == 100)");
-
 struct omap_i2c_dev {
 	struct device		*dev;
 	void __iomem		*base;		/* virtual */
 	int			irq;
+	int 			speed;		/* Default: 100 kHz */
 	struct clk		*iclk;		/* Interface clock */
 	struct clk		*fclk;		/* Functional clock */
 	struct completion	cmd_complete;
@@ -248,9 +243,9 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
 		psc = 2;
 
 	omap_i2c_write_reg(dev, OMAP_I2C_SCLL_REG,
-			   fclk_rate / (clock * 2) - 7 + psc);
+			   fclk_rate / (dev->speed * 2) - 7 + psc);
 	omap_i2c_write_reg(dev, OMAP_I2C_SCLH_REG,
-			   fclk_rate / (clock * 2) - 7 + psc);
+			   fclk_rate / (dev->speed * 2) - 7 + psc);
 
 	/* Take the I2C module out of reset: */
 	omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN);
@@ -579,7 +574,7 @@ omap_i2c_probe(struct platform_device *pdev)
 {
 	struct omap_i2c_dev	*dev;
 	struct i2c_adapter	*adap;
-	struct resource		*mem, *irq, *ioarea;
+	struct resource		*mem, *irq, *ioarea, *speed;
 	int r;
 
 	/* NOTE: driver uses the static register mapping */
@@ -593,6 +588,8 @@ omap_i2c_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "no irq resource?\n");
 		return -ENODEV;
 	}
+	speed = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	/* It is ok to have a NULL speed -we will use defaults */
 
 	ioarea = request_mem_region(mem->start, (mem->end - mem->start) + 1,
 			pdev->name);
@@ -601,17 +598,22 @@ omap_i2c_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	if (clock > 200)
-		clock = 400;	/* Fast mode */
-	else
-		clock = 100;	/* Standard mode */
-
 	dev = kzalloc(sizeof(struct omap_i2c_dev), GFP_KERNEL);
 	if (!dev) {
 		r = -ENOMEM;
 		goto err_release_region;
 	}
 
+	if (!speed) {
+		dev->speed = 100; /* use standard */
+	} else {
+		dev->speed = speed->end;
+	}
+	/* TODO: Add support for Hghspeed */
+	if (dev->speed > 200)
+		dev->speed = 400;	/* Fast mode */
+	else
+		dev->speed = 100;	/* Standard mode */
 	dev->dev = &pdev->dev;
 	dev->irq = irq->start;
 	dev->base = (void __iomem *) IO_ADDRESS(mem->start);
@@ -636,8 +638,8 @@ omap_i2c_probe(struct platform_device *pdev)
 		goto err_unuse_clocks;
 	}
 	r = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff;
-	dev_info(dev->dev, "bus %d rev%d.%d at %d kHz\n",
-		 pdev->id, r >> 4, r & 0xf, clock);
+	dev_info(dev->dev, "bus %d rev%d.%d at %d kHz(requested %d kHz)\n",
+		 pdev->id, r >> 4, r & 0xf, dev->speed, (speed)? speed->end:0);
 
 	adap = &dev->adapter;
 	i2c_set_adapdata(adap, dev);
-- 
1.5.0.5


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

RE: [RFC-PATCH] Move i2c adaptor speed from a module parameter toplatform device resource

[Syed Mohammed, Khasim]

Hi Nishanth,

>-----Original Message-----
>From: linux-omap-open-source-bounces@linux.omap.com [mailto:linux-omap-
>open-source-bounces@linux.omap.com] On Behalf Of nishanth menon
>Sent: Monday, April 02, 2007 7:14 PM
>To: linux-omap-open-source@linux.omap.com
>Subject: [RFC-PATCH] Move i2c adaptor speed from a module parameter
>toplatform device resource
>
>Hi All,
>
>Please find attached patch to move the i2c speed parameter from module
>to bus specific resource.
>
>To Discuss:
>* Is IORESOURCE_IO the best way of defining speed for i2c bus?
>
I think we will be giving a different meaning to IORESOURCE_IO by doing
this. Other drivers mainly use this for IO space handling purposes. 

I thought some I2C config structure would have been better, like one
available for SPI/MMC. I don't know if this gets formally approved.

The other option I thought was, 

static int omap2_i2c2_clkrate = 400;

static struct platform_device omap_i2c_device2 = {
        .name           = "i2c_omap",
        .id             = 2,
        .num_resources  = ARRAY_SIZE(i2c_resources2),
        .resource       = i2c_resources2,
        .dev            = {
                .platform_data = &omap2_i2c2_clkrate,
        },
};

:), 

>This, however, requires the following follow up patches:
>* Move i2c platform from plat-omap to respective board files.
>*    Reasoning: I2C speed at 100Khz is not always optimal for I2C1 by
>*      default (e.g. 3430).
>*    Impact: all board files will need to have it...but it will ensure
>*    that the i2c init is done in the proper sequence - 1 followed by
2.

We had discussed this before, but Tony was not "for" this approach.
http://linux.omap.com/pipermail/linux-omap-open-source/2006-November/008
398.html

>* Support FIFO mode of data transfer - this is supported on newer I2C
h/w
>*     revs
>*    Options: support all in a single driver/split them up? have
i2c-omap.c
>*     and i2c-omap2.c?

We can add all new functionality to existing i2c-omap.c.

>* Support High speed transfers at a bus level
>*    Will depend of FIFO implementation as polled/interrupt mode is too
>*     slow and causes underrun.


We can...

>* Support all known Erratas for i2c module.. make features
>*    i2c_module_hardware_revision based.
>* Support proper bus speed support
>*    Multiple devices on the same bus can have varied speed
requirements.
>*    Infact speed could be a parameter for i2c messages?
>*    Impact: I2c corelevel changes required.i2c list disc.

No specific comments as of now. This activity can be handled once we are
done with all of the above

>Regards,
>Nishanth Menon

Regards,
Khasim

Re: [RFC-PATCH] Move i2c adaptor speed from a module parameter toplatform device resource

[Nishanth Menon]

Syed Mohammed, Khasim stated on 4/6/2007 5:00 PM:
>> This, however, requires the following follow up patches:
>> * Move i2c platform from plat-omap to respective board files.
>> *    Reasoning: I2C speed at 100Khz is not always optimal for I2C1 by
>> *      default (e.g. 3430).
>> *    Impact: all board files will need to have it...but it will ensure
>> *    that the i2c init is done in the proper sequence - 1 followed by
>>     
> 2.
>
> We had discussed this before, but Tony was not "for" this approach.
> http://linux.omap.com/pipermail/linux-omap-open-source/2006-November/008
>
>   
The argument then was that we could have a common definition for all
platform. if we look at our code, if have enough amount of #ifdefs that
we have been trying to avoid..
at the very least, it should move out to mach-omapx/devices.c
Regards,
Nishanth Menon