[PATCH 0/2] i2c: omap: revision register updates

[PATCH 0/2] i2c: omap: revision register updates

[Shubhrajyoti D]

Does the followiing
- Make the revision a 32- bit consisting of rev_lo amd rev_hi each
of 16 bits.

- Also use the revision register for the erratum i207.

Also more cleanup is possible will check on that subsequently.


Tested on OMAP4430sdp  ,4460 ,omap3630 and 3430.

todo: omap2 testing.

Shubhrajyoti D (2):
  i2c: omap: use revision check for OMAP_I2C_FLAG_APPLY_ERRATA_I207
  i2c: omap: Fix the revision register read

 drivers/i2c/busses/i2c-omap.c |   62 +++++++++++++++++++++++++++++++---------
 1 files changed, 48 insertions(+), 14 deletions(-)

-- 
1.7.5.4

[PATCH 1/2] i2c: omap: Fix the revision register read

[Shubhrajyoti D]

The revision register on OMAP4 is a 16-bit lo and a 16-bit
hi. Currently the driver reads only the lower 8-bits.
Fix the same by preventing the truncating of the rev register
for OMAP4.

Also use the scheme bit ie bit-14 of the hi register to know if it
is OMAP_I2C_IP_VERSION_2.

On platforms previous to OMAP4 the offset 0x04 is IE register whose
bit-14 reset value is 0, the code uses the same to its advantage.

Also since the omap_i2c_read_reg uses reg_map_ip_* a raw_readw is done
to fetch the revision register.

The dev->regs is populated after reading the rev_hi. A NULL check
has been added in the resume handler to prevent the access before
the setting of the regs.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
 drivers/i2c/busses/i2c-omap.c |   59 ++++++++++++++++++++++++++++++++---------
 1 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index db31eae..d8e7709 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -49,9 +49,10 @@
 #define OMAP_I2C_OMAP1_REV_2		0x20
 
 /* I2C controller revisions present on specific hardware */
-#define OMAP_I2C_REV_ON_2430		0x36
-#define OMAP_I2C_REV_ON_3430_3530	0x3C
-#define OMAP_I2C_REV_ON_3630_4430	0x40
+#define OMAP_I2C_REV_ON_2430		0x00000036
+#define OMAP_I2C_REV_ON_3430_3530	0x0000003C
+#define OMAP_I2C_REV_ON_3630		0x00000040
+#define OMAP_I2C_REV_ON_4430_PLUS	0x50400002
 
 /* timeout waiting for the controller to respond */
 #define OMAP_I2C_TIMEOUT (msecs_to_jiffies(1000))
@@ -202,7 +203,7 @@ struct omap_i2c_dev {
 						 * fifo_size==0 implies no fifo
 						 * if set, should be trsh+1
 						 */
-	u8			rev;
+	u32			rev;
 	unsigned		b_hw:1;		/* bad h/w fixes */
 	unsigned		receiver:1;	/* true when we're in receiver mode */
 	u16			iestate;	/* Saved interrupt register */
@@ -490,7 +491,7 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *dev, u8 size, bool is_rx)
 
 	omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf);
 
-	if (dev->rev < OMAP_I2C_REV_ON_3630_4430)
+	if (dev->rev < OMAP_I2C_REV_ON_3630)
 		dev->b_hw = 1; /* Enable hardware fixes */
 
 	/* calculate wakeup latency constraint for MPU */
@@ -1052,6 +1053,14 @@ static const struct of_device_id omap_i2c_of_match[] = {
 MODULE_DEVICE_TABLE(of, omap_i2c_of_match);
 #endif
 
+#define OMAP_I2C_SCHEME(rev)		(rev & 0xc000) >> 14
+
+#define OMAP_I2C_REV_SCHEME_0_MAJOR(rev) (rev >> 4)
+#define OMAP_I2C_REV_SCHEME_0_MINOR(rev) (rev & 0xf)
+
+#define OMAP_I2C_REV_SCHEME_1_MAJOR(rev) ((rev & 0x0700) >> 7)
+#define OMAP_I2C_REV_SCHEME_1_MINOR(rev) (rev & 0x1f)
+
 static int __devinit
 omap_i2c_probe(struct platform_device *pdev)
 {
@@ -1064,6 +1073,8 @@ omap_i2c_probe(struct platform_device *pdev)
 	const struct of_device_id *match;
 	int irq;
 	int r;
+	u32 rev;
+	u16 minor, major;
 
 	/* NOTE: driver uses the static register mapping */
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1117,11 +1128,6 @@ omap_i2c_probe(struct platform_device *pdev)
 
 	dev->reg_shift = (dev->flags >> OMAP_I2C_FLAG_BUS_SHIFT__SHIFT) & 3;
 
-	if (dev->dtrev == OMAP_I2C_IP_VERSION_2)
-		dev->regs = (u8 *)reg_map_ip_v2;
-	else
-		dev->regs = (u8 *)reg_map_ip_v1;
-
 	pm_runtime_enable(dev->dev);
 	pm_runtime_set_autosuspend_delay(dev->dev, OMAP_I2C_PM_TIMEOUT);
 	pm_runtime_use_autosuspend(dev->dev);
@@ -1130,7 +1136,31 @@ omap_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR_VALUE(r))
 		goto err_free_mem;
 
-	dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff;
+	/*
+	 * Read the Rev hi bit-[15:14] ie scheme this is 1 indicates ver2.
+	 * On omap3 Offset 4 is IE Reg the bit [15:14] is XDR_IE which is 0
+	 * at reset. Also since the omap_i2c_read_reg uses reg_map_ip_* a
+	 * raw_readw is done.
+	 */
+	rev = __raw_readw(dev->base + 0x04);
+
+	switch (OMAP_I2C_SCHEME(rev)) {
+	case 0:
+		dev->regs = (u8 *)reg_map_ip_v1;
+		dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff;
+		minor = OMAP_I2C_REV_SCHEME_0_MAJOR(dev->rev);
+		major = OMAP_I2C_REV_SCHEME_0_MAJOR(dev->rev);
+	break;
+	case 1:
+		/* FALLTHROUGH */
+	default:
+		dev->regs = (u8 *)reg_map_ip_v2;
+		rev = (rev << 16) |
+			omap_i2c_read_reg(dev, OMAP_I2C_IP_V2_REVNB_LO);
+		minor = OMAP_I2C_REV_SCHEME_1_MINOR(rev);
+		major = OMAP_I2C_REV_SCHEME_1_MAJOR(rev);
+		dev->rev = rev;
+	}
 
 	dev->errata = 0;
 
@@ -1155,7 +1185,7 @@ omap_i2c_probe(struct platform_device *pdev)
 
 		dev->fifo_size = (dev->fifo_size / 2);
 
-		if (dev->rev < OMAP_I2C_REV_ON_3630_4430)
+		if (dev->rev < OMAP_I2C_REV_ON_3630)
 			dev->b_hw = 1; /* Enable hardware fixes */
 
 		/* calculate wakeup latency constraint for MPU */
@@ -1198,7 +1228,7 @@ omap_i2c_probe(struct platform_device *pdev)
 	}
 
 	dev_info(dev->dev, "bus %d rev%d.%d.%d at %d kHz\n", adap->nr,
-		 dev->dtrev, dev->rev >> 4, dev->rev & 0xf, dev->speed);
+		 dev->dtrev, major, minor, dev->speed);
 
 	of_i2c_register_devices(adap);
 
@@ -1264,6 +1294,9 @@ static int omap_i2c_runtime_resume(struct device *dev)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);
 
+	if (!_dev->regs)
+		return 0;
+
 	if (_dev->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) {
 		omap_i2c_write_reg(_dev, OMAP_I2C_CON_REG, 0);
 		omap_i2c_write_reg(_dev, OMAP_I2C_PSC_REG, _dev->pscstate);
-- 
1.7.5.4

[PATCH 2/2] i2c: omap: use revision check for OMAP_I2C_FLAG_APPLY_ERRATA_I207

[Shubhrajyoti D]

The errata i207 is enabled for 2430 and 3xxx. Use the revision check
to enable the erratum instead.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
 drivers/i2c/busses/i2c-omap.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index d8e7709..44245d4 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1164,7 +1164,8 @@ omap_i2c_probe(struct platform_device *pdev)
 
 	dev->errata = 0;
 
-	if (dev->flags & OMAP_I2C_FLAG_APPLY_ERRATA_I207)
+	if (dev->rev >= OMAP_I2C_REV_ON_2430 &&
+			dev->rev < OMAP_I2C_REV_ON_4430_PLUS)
 		dev->errata |= I2C_OMAP_ERRATA_I207;
 
 	if (dev->rev <= OMAP_I2C_REV_ON_3430_3530)
-- 
1.7.5.4

Re: [PATCH 1/2] i2c: omap: Fix the revision register read

[Felipe Balbi]

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

Hi,

On Fri, Nov 02, 2012 at 04:09:44PM +0530, Shubhrajyoti D wrote:
> The revision register on OMAP4 is a 16-bit lo and a 16-bit
> hi. Currently the driver reads only the lower 8-bits.
> Fix the same by preventing the truncating of the rev register
> for OMAP4.
> 
> Also use the scheme bit ie bit-14 of the hi register to know if it
> is OMAP_I2C_IP_VERSION_2.
> 
> On platforms previous to OMAP4 the offset 0x04 is IE register whose
> bit-14 reset value is 0, the code uses the same to its advantage.
> 
> Also since the omap_i2c_read_reg uses reg_map_ip_* a raw_readw is done
> to fetch the revision register.
> 
> The dev->regs is populated after reading the rev_hi. A NULL check
> has been added in the resume handler to prevent the access before
> the setting of the regs.

tested on which platforms ?

> Signed-off-by: Shubhrajyoti D <shubhrajyoti-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-omap.c |   59 ++++++++++++++++++++++++++++++++---------
>  1 files changed, 46 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index db31eae..d8e7709 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -49,9 +49,10 @@
>  #define OMAP_I2C_OMAP1_REV_2		0x20
>  
>  /* I2C controller revisions present on specific hardware */
> -#define OMAP_I2C_REV_ON_2430		0x36
> -#define OMAP_I2C_REV_ON_3430_3530	0x3C
> -#define OMAP_I2C_REV_ON_3630_4430	0x40
> +#define OMAP_I2C_REV_ON_2430		0x00000036

are you sure this are the contents on 2430 ? Have you actually ran this
code on that platform ?

> +#define OMAP_I2C_REV_ON_3430_3530	0x0000003C
> +#define OMAP_I2C_REV_ON_3630		0x00000040

likewise for these two.

> +#define OMAP_I2C_REV_ON_4430_PLUS	0x50400002

what about 4460, 4470, 3530, etc etc etc ?

> @@ -490,7 +491,7 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *dev, u8 size, bool is_rx)
>  
>  	omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf);
>  
> -	if (dev->rev < OMAP_I2C_REV_ON_3630_4430)
> +	if (dev->rev < OMAP_I2C_REV_ON_3630)
>  		dev->b_hw = 1; /* Enable hardware fixes */
>  
>  	/* calculate wakeup latency constraint for MPU */
> @@ -1052,6 +1053,14 @@ static const struct of_device_id omap_i2c_of_match[] = {
>  MODULE_DEVICE_TABLE(of, omap_i2c_of_match);
>  #endif
>  
> +#define OMAP_I2C_SCHEME(rev)		(rev & 0xc000) >> 14

you miss () there to make sure no other operations will take higher
precedence when using this macro.

> @@ -1130,7 +1136,31 @@ omap_i2c_probe(struct platform_device *pdev)
>  	if (IS_ERR_VALUE(r))
>  		goto err_free_mem;
>  
> -	dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff;
> +	/*
> +	 * Read the Rev hi bit-[15:14] ie scheme this is 1 indicates ver2.
> +	 * On omap3 Offset 4 is IE Reg the bit [15:14] is XDR_IE which is 0
> +	 * at reset. Also since the omap_i2c_read_reg uses reg_map_ip_* a
> +	 * raw_readw is done.
> +	 */
> +	rev = __raw_readw(dev->base + 0x04);
> +
> +	switch (OMAP_I2C_SCHEME(rev)) {
> +	case 0:

define macros for these two cases.

> +		dev->regs = (u8 *)reg_map_ip_v1;
> +		dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff;
> +		minor = OMAP_I2C_REV_SCHEME_0_MAJOR(dev->rev);
> +		major = OMAP_I2C_REV_SCHEME_0_MAJOR(dev->rev);
> +	break;

wrong indentation.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] i2c: omap: use revision check for OMAP_I2C_FLAG_APPLY_ERRATA_I207

[Felipe Balbi]

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

On Fri, Nov 02, 2012 at 04:09:45PM +0530, Shubhrajyoti D wrote:
> The errata i207 is enabled for 2430 and 3xxx. Use the revision check
> to enable the erratum instead.
> 
> Signed-off-by: Shubhrajyoti D <shubhrajyoti-l0cyMroinI0@public.gmane.org>

very good. I haven't read the errata but from a code standpoint, it
looks good:

Reviewed-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>

> ---
>  drivers/i2c/busses/i2c-omap.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index d8e7709..44245d4 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -1164,7 +1164,8 @@ omap_i2c_probe(struct platform_device *pdev)
>  
>  	dev->errata = 0;
>  
> -	if (dev->flags & OMAP_I2C_FLAG_APPLY_ERRATA_I207)
> +	if (dev->rev >= OMAP_I2C_REV_ON_2430 &&
> +			dev->rev < OMAP_I2C_REV_ON_4430_PLUS)
>  		dev->errata |= I2C_OMAP_ERRATA_I207;
>  
>  	if (dev->rev <= OMAP_I2C_REV_ON_3430_3530)
> -- 
> 1.7.5.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] i2c: omap: use revision check for OMAP_I2C_FLAG_APPLY_ERRATA_I207

[Shubhrajyoti Datta]

On Fri, Nov 2, 2012 at 4:37 PM, Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> wrote:
> On Fri, Nov 02, 2012 at 04:09:45PM +0530, Shubhrajyoti D wrote:
>> The errata i207 is enabled for 2430 and 3xxx. Use the revision check
>> to enable the erratum instead.
>>
>> Signed-off-by: Shubhrajyoti D <shubhrajyoti-l0cyMroinI0@public.gmane.org>
>
> very good. I haven't read the errata but from a code standpoint, it
> looks good:
>
> Reviewed-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
>

Also I post to this the flag may be deleted.


From: Shubhrajyoti D <shubhrajyoti-l0cyMroinI0@public.gmane.org>
Date: Fri, 2 Nov 2012 16:34:08 +0530
Subject: [PATCH] ARM: i2c: omap: Remove the i207 errata flag

The commit [i2c: omap: use revision check for OMAP_I2C_FLAG_APPLY_ERRATA_I207]
uses the revision id instead of the flag. So the flag can be safely removed.

Signed-off-by: Shubhrajyoti D <shubhrajyoti-l0cyMroinI0@public.gmane.org>
---
 arch/arm/mach-omap2/omap_hwmod_2430_data.c |    3 +--
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    9 +++------
 drivers/i2c/busses/i2c-omap.c              |    3 +--
 include/linux/i2c-omap.h                   |    1 -
 4 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
index c455e41..b79ccf6 100644
--- a/arch/arm/mach-omap2/omap_hwmod_2430_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_2430_data.c
@@ -76,8 +76,7 @@ static struct omap_hwmod_class i2c_class = {

 static struct omap_i2c_dev_attr i2c_dev_attr = {
 	.fifo_depth	= 8, /* bytes */
-	.flags		= OMAP_I2C_FLAG_APPLY_ERRATA_I207 |
-			  OMAP_I2C_FLAG_BUS_SHIFT_2 |
+	.flags		= OMAP_I2C_FLAG_BUS_SHIFT_2 |
 			  OMAP_I2C_FLAG_FORCE_19200_INT_CLK,
 };

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index f67b7ee..943222c4 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -791,8 +791,7 @@ static struct omap_hwmod omap3xxx_dss_venc_hwmod = {
 /* I2C1 */
 static struct omap_i2c_dev_attr i2c1_dev_attr = {
 	.fifo_depth	= 8, /* bytes */
-	.flags		= OMAP_I2C_FLAG_APPLY_ERRATA_I207 |
-			  OMAP_I2C_FLAG_RESET_REGS_POSTIDLE |
+	.flags		= OMAP_I2C_FLAG_RESET_REGS_POSTIDLE |
 			  OMAP_I2C_FLAG_BUS_SHIFT_2,
 };

@@ -818,8 +817,7 @@ static struct omap_hwmod omap3xxx_i2c1_hwmod = {
 /* I2C2 */
 static struct omap_i2c_dev_attr i2c2_dev_attr = {
 	.fifo_depth	= 8, /* bytes */
-	.flags = OMAP_I2C_FLAG_APPLY_ERRATA_I207 |
-		 OMAP_I2C_FLAG_RESET_REGS_POSTIDLE |
+	.flags = OMAP_I2C_FLAG_RESET_REGS_POSTIDLE |
 		 OMAP_I2C_FLAG_BUS_SHIFT_2,
 };

@@ -845,8 +843,7 @@ static struct omap_hwmod omap3xxx_i2c2_hwmod = {
 /* I2C3 */
 static struct omap_i2c_dev_attr i2c3_dev_attr = {
 	.fifo_depth	= 64, /* bytes */
-	.flags = OMAP_I2C_FLAG_APPLY_ERRATA_I207 |
-		 OMAP_I2C_FLAG_RESET_REGS_POSTIDLE |
+	.flags = OMAP_I2C_FLAG_RESET_REGS_POSTIDLE |
 		 OMAP_I2C_FLAG_BUS_SHIFT_2,
 };

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index dd97c14..87970fa 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1029,8 +1029,7 @@ static const struct i2c_algorithm omap_i2c_algo = {
 #ifdef CONFIG_OF
 static struct omap_i2c_bus_platform_data omap3_pdata = {
 	.rev = OMAP_I2C_IP_VERSION_1,
-	.flags = OMAP_I2C_FLAG_APPLY_ERRATA_I207 |
-		 OMAP_I2C_FLAG_RESET_REGS_POSTIDLE |
+	.flags = OMAP_I2C_FLAG_RESET_REGS_POSTIDLE |
 		 OMAP_I2C_FLAG_BUS_SHIFT_2,
 };

diff --git a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h
index df804ba..5c88187 100644
--- a/include/linux/i2c-omap.h
+++ b/include/linux/i2c-omap.h
@@ -21,7 +21,6 @@
 #define OMAP_I2C_FLAG_SIMPLE_CLOCK		BIT(1)
 #define OMAP_I2C_FLAG_16BIT_DATA_REG		BIT(2)
 #define OMAP_I2C_FLAG_RESET_REGS_POSTIDLE	BIT(3)
-#define OMAP_I2C_FLAG_APPLY_ERRATA_I207	BIT(4)
 #define OMAP_I2C_FLAG_ALWAYS_ARMXOR_CLK	BIT(5)
 #define OMAP_I2C_FLAG_FORCE_19200_INT_CLK	BIT(6)
 /* how the CPU address bus must be translated for I2C unit access */
-- 
1.7.5.4

Re: [PATCH 1/2] i2c: omap: Fix the revision register read

[Shubhrajyoti Datta]

On Fri, Nov 2, 2012 at 4:36 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Fri, Nov 02, 2012 at 04:09:44PM +0530, Shubhrajyoti D wrote:
>> The revision register on OMAP4 is a 16-bit lo and a 16-bit
>> hi. Currently the driver reads only the lower 8-bits.
>> Fix the same by preventing the truncating of the rev register
>> for OMAP4.
>>
>> Also use the scheme bit ie bit-14 of the hi register to know if it
>> is OMAP_I2C_IP_VERSION_2.
>>
>> On platforms previous to OMAP4 the offset 0x04 is IE register whose
>> bit-14 reset value is 0, the code uses the same to its advantage.
>>
>> Also since the omap_i2c_read_reg uses reg_map_ip_* a raw_readw is done
>> to fetch the revision register.
>>
>> The dev->regs is populated after reading the rev_hi. A NULL check
>> has been added in the resume handler to prevent the access before
>> the setting of the regs.
>
> tested on which platforms ?

omap4430 , 4460 and omap3.
>
>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>> ---
>>  drivers/i2c/busses/i2c-omap.c |   59 ++++++++++++++++++++++++++++++++---------
>>  1 files changed, 46 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
>> index db31eae..d8e7709 100644
>> --- a/drivers/i2c/busses/i2c-omap.c
>> +++ b/drivers/i2c/busses/i2c-omap.c
>> @@ -49,9 +49,10 @@
>>  #define OMAP_I2C_OMAP1_REV_2         0x20
>>
>>  /* I2C controller revisions present on specific hardware */
>> -#define OMAP_I2C_REV_ON_2430         0x36
>> -#define OMAP_I2C_REV_ON_3430_3530    0x3C
>> -#define OMAP_I2C_REV_ON_3630_4430    0x40
>> +#define OMAP_I2C_REV_ON_2430         0x00000036
>
> are you sure this are the contents on 2430 ? Have you actually ran this
> code on that platform ?

I did not run on 2430. Will try to get a platform and run.

However the current code already has and uses the same value so
 I feel it should be fine.:-)


>
>> +#define OMAP_I2C_REV_ON_3430_3530    0x0000003C
>> +#define OMAP_I2C_REV_ON_3630         0x00000040
>
> likewise for these two.

I verified that the 3630 returns 0x40 on my beaglexM.

>
>> +#define OMAP_I2C_REV_ON_4430_PLUS    0x50400002
>
> what about 4460, 4470, 3530, etc etc etc ?
>
>> @@ -490,7 +491,7 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *dev, u8 size, bool is_rx)
>>
>>       omap_i2c_write_reg(dev, OMAP_I2C_BUF_REG, buf);
>>
>> -     if (dev->rev < OMAP_I2C_REV_ON_3630_4430)
>> +     if (dev->rev < OMAP_I2C_REV_ON_3630)
>>               dev->b_hw = 1; /* Enable hardware fixes */
>>
>>       /* calculate wakeup latency constraint for MPU */
>> @@ -1052,6 +1053,14 @@ static const struct of_device_id omap_i2c_of_match[] = {
>>  MODULE_DEVICE_TABLE(of, omap_i2c_of_match);
>>  #endif
>>
>> +#define OMAP_I2C_SCHEME(rev)         (rev & 0xc000) >> 14
>
> you miss () there to make sure no other operations will take higher
> precedence when using this macro.

Indeed. Thanks.
>
>> @@ -1130,7 +1136,31 @@ omap_i2c_probe(struct platform_device *pdev)
>>       if (IS_ERR_VALUE(r))
>>               goto err_free_mem;
>>
>> -     dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff;
>> +     /*
>> +      * Read the Rev hi bit-[15:14] ie scheme this is 1 indicates ver2.
>> +      * On omap3 Offset 4 is IE Reg the bit [15:14] is XDR_IE which is 0
>> +      * at reset. Also since the omap_i2c_read_reg uses reg_map_ip_* a
>> +      * raw_readw is done.
>> +      */
>> +     rev = __raw_readw(dev->base + 0x04);
>> +
>> +     switch (OMAP_I2C_SCHEME(rev)) {
>> +     case 0:
>
> define macros for these two cases.

OK
>
>> +             dev->regs = (u8 *)reg_map_ip_v1;
>> +             dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff;
>> +             minor = OMAP_I2C_REV_SCHEME_0_MAJOR(dev->rev);
>> +             major = OMAP_I2C_REV_SCHEME_0_MAJOR(dev->rev);
>> +     break;
>
> wrong indentation.

Yes will fix.
>
> --
> balbi