| Message ID | 1351673959-5918-1-git-send-email-shubhrajyoti@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 31, 2012 at 2:29 PM, Shubhrajyoti D <shubhrajyoti@ti.com> 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. > > 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 omap4sdp, omap3630 based beagle , omap3430sdp.
Hi, On Wed, Oct 31, 2012 at 02:29:19PM +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. very good, but you need to test this with OMAP2/3/4 (5 ??). How was this tested ? > 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. > > 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. this could get some rephrasing, I guess. At least to me it's difficult to understand what you mean :-s > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > --- > todo: some of the flag checks can be removed in favour of revision check. > > drivers/i2c/busses/i2c-omap.c | 35 +++++++++++++++++++++++++---------- > 1 files changed, 25 insertions(+), 10 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index db31eae..651a7f7 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -51,7 +51,8 @@ > /* 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_3630 0x40 > +#define OMAP_I2C_REV_ON_4430_PLUS 0x5040 I would rather see a proper decoding of the revision, meaning that you would: For omap2/3: rev major = rev >> 8; rev minor = rev & 0xff; For OMAP4/5: well, that's a lot more complex, but you have that data ;-) > /* 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; > + u16 rev; IMHO this should be u32, so you don't need rev_lo and rev_hi below. > 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 */ > @@ -1064,6 +1065,8 @@ omap_i2c_probe(struct platform_device *pdev) > const struct of_device_id *match; > int irq; > int r; > + u16 rev_lo; > + u16 rev_hi; > > /* NOTE: driver uses the static register mapping */ > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > @@ -1117,11 +1120,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 +1128,21 @@ 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-14 ie scheme this is 1 indicates ver2 or > + * highlander. the "scheme" in highlander is a 2 bit value. In order to make this future proof, you need to read both bits (31:30). > + * On omap3 Offset 4 is IE Reg the bit 14 is XDR_IE which is 0 at reset. > + */ please align the * characters. > + rev_hi = __raw_readw(dev->base + 0x04); you should make omap_i2c_read_reg() work fine for this case too. > + > + if (rev_hi & 0x4000) {/* If scheme 1*/ you should add a symbolic define for scheme, something like: switch (OMAP_I2C_SCHEME(rev)) { case OMAP_I2C_SCHEME_1: foo(); break; case OMAP_I2C_SCHEME_2: /* FALLTHROUGH */ default: bar(); } note that this will also default to highest known scheme if another scheme is added. You need to make the driver behave like that to decrease amount of rework to support newest OMAPs. > + dev->regs = (u8 *)reg_map_ip_v2; > + dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_IP_V2_REVNB_HI); > + rev_lo = omap_i2c_read_reg(dev, OMAP_I2C_IP_V2_REVNB_LO); > + dev_info(dev->dev, "the low rev %x\n", rev_lo); > + } else { > + dev->regs = (u8 *)reg_map_ip_v1; > + dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff; > + } > > dev->errata = 0; > > @@ -1155,7 +1167,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 */ > @@ -1264,6 +1276,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; this is wrong, you need to make sure dev->regs is set early enough.
Hi, On Wed, Oct 31, 2012 at 03:02:57PM +0530, Shubhrajyoti Datta wrote: > On Wed, Oct 31, 2012 at 2:29 PM, Shubhrajyoti D <shubhrajyoti@ti.com> 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. > > > > 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 omap4sdp, omap3630 based beagle , omap3430sdp. oh, now I see. You need to test on OMAP2 which has different revision layout.
On Wednesday 31 October 2012 03:42 PM, Felipe Balbi wrote: > Hi, > > On Wed, Oct 31, 2012 at 02:29:19PM +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. > very good, but you need to test this with OMAP2/3/4 (5 ??). How was this > tested ? > >> 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. >> >> 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. > this could get some rephrasing, I guess. At least to me it's difficult > to understand what you mean :-s > >> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> >> --- >> todo: some of the flag checks can be removed in favour of revision check. >> >> drivers/i2c/busses/i2c-omap.c | 35 +++++++++++++++++++++++++---------- >> 1 files changed, 25 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c >> index db31eae..651a7f7 100644 >> --- a/drivers/i2c/busses/i2c-omap.c >> +++ b/drivers/i2c/busses/i2c-omap.c >> @@ -51,7 +51,8 @@ >> /* 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_3630 0x40 >> +#define OMAP_I2C_REV_ON_4430_PLUS 0x5040 > I would rather see a proper decoding of the revision, meaning that you > would: > > For omap2/3: > > rev major = rev >> 8; > rev minor = rev & 0xff; you mean rev major = rev >> 4; rev minor = rev & 0xf; thats doable too. However that currently that is read together currently. > > For OMAP4/5: > > well, that's a lot more complex, but you have that data ;-) > >> /* 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; >> + u16 rev; > IMHO this should be u32, so you don't need rev_lo and rev_hi below. > >> 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 */ >> @@ -1064,6 +1065,8 @@ omap_i2c_probe(struct platform_device *pdev) >> const struct of_device_id *match; >> int irq; >> int r; >> + u16 rev_lo; >> + u16 rev_hi; >> >> /* NOTE: driver uses the static register mapping */ >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> @@ -1117,11 +1120,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 +1128,21 @@ 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-14 ie scheme this is 1 indicates ver2 or >> + * highlander. > the "scheme" in highlander is a 2 bit value. In order to make this > future proof, you need to read both bits (31:30). Good point will fix it. > >> + * On omap3 Offset 4 is IE Reg the bit 14 is XDR_IE which is 0 at reset. >> + */ > please align the * characters. yes will repost > >> + rev_hi = __raw_readw(dev->base + 0x04); > you should make omap_i2c_read_reg() work fine for this case too. Just felt it is more readlable this way also I didnt want to use the reg_shift etc. which also may get cleaned up sometime. > >> + >> + if (rev_hi & 0x4000) {/* If scheme 1*/ > you should add a symbolic define for scheme, something like: > > switch (OMAP_I2C_SCHEME(rev)) { > case OMAP_I2C_SCHEME_1: > foo(); > break; > case OMAP_I2C_SCHEME_2: > /* FALLTHROUGH */ > default: > bar(); > } > > note that this will also default to highest known scheme if another > scheme is added. You need to make the driver behave like that to > decrease amount of rework to support newest OMAPs. OK. > >> + dev->regs = (u8 *)reg_map_ip_v2; >> + dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_IP_V2_REVNB_HI); >> + rev_lo = omap_i2c_read_reg(dev, OMAP_I2C_IP_V2_REVNB_LO); >> + dev_info(dev->dev, "the low rev %x\n", rev_lo); >> + } else { >> + dev->regs = (u8 *)reg_map_ip_v1; >> + dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff; >> + } >> >> dev->errata = 0; >> >> @@ -1155,7 +1167,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 */ >> @@ -1264,6 +1276,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; > this is wrong, you need to make sure dev->regs is set early enough.
On Wednesday 31 October 2012 03:42 PM, Felipe Balbi wrote: >> >> > + if (!_dev->regs) >> > + return 0; > this is wrong, you need to make sure dev->regs is set early enough. to set the dev->regs I use the value read from revision register. to read the revision register I do a get_sync first. This in turn leads to the call to resume handler. At this point the regs is NULL and so the check. After reading the register and checking the scheme bit. the reg map is populated. How can this issue be solved?
Hi, On Wed, Oct 31, 2012 at 04:18:15PM +0530, Shubhrajyoti wrote: > On Wednesday 31 October 2012 03:42 PM, Felipe Balbi wrote: > > Hi, > > > > On Wed, Oct 31, 2012 at 02:29:19PM +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. > > very good, but you need to test this with OMAP2/3/4 (5 ??). How was this > > tested ? > > > >> 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. > >> > >> 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. > > this could get some rephrasing, I guess. At least to me it's difficult > > to understand what you mean :-s > > > >> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > >> --- > >> todo: some of the flag checks can be removed in favour of revision check. > >> > >> drivers/i2c/busses/i2c-omap.c | 35 +++++++++++++++++++++++++---------- > >> 1 files changed, 25 insertions(+), 10 deletions(-) > >> > >> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > >> index db31eae..651a7f7 100644 > >> --- a/drivers/i2c/busses/i2c-omap.c > >> +++ b/drivers/i2c/busses/i2c-omap.c > >> @@ -51,7 +51,8 @@ > >> /* 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_3630 0x40 > >> +#define OMAP_I2C_REV_ON_4430_PLUS 0x5040 > > I would rather see a proper decoding of the revision, meaning that you > > would: > > > > For omap2/3: > > > > rev major = rev >> 8; > > rev minor = rev & 0xff; > you mean > > rev major = rev >> 4; > rev minor = rev & 0xf; might be, I didn't look at the TRM to make sure, my bad :-) > thats doable too. However that currently that is read together > currently. and that's what's wrong IMHO. What's current in driver is only valid for OMAP1 IIRC. > > For OMAP4/5: > > > > well, that's a lot more complex, but you have that data ;-) > > > >> /* 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; > >> + u16 rev; > > IMHO this should be u32, so you don't need rev_lo and rev_hi below. > > > >> 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 */ > >> @@ -1064,6 +1065,8 @@ omap_i2c_probe(struct platform_device *pdev) > >> const struct of_device_id *match; > >> int irq; > >> int r; > >> + u16 rev_lo; > >> + u16 rev_hi; > >> > >> /* NOTE: driver uses the static register mapping */ > >> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > >> @@ -1117,11 +1120,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 +1128,21 @@ 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-14 ie scheme this is 1 indicates ver2 or > >> + * highlander. > > the "scheme" in highlander is a 2 bit value. In order to make this > > future proof, you need to read both bits (31:30). > Good point will fix it. > > > >> + * On omap3 Offset 4 is IE Reg the bit 14 is XDR_IE which is 0 at reset. > >> + */ > > please align the * characters. > yes will repost > > > >> + rev_hi = __raw_readw(dev->base + 0x04); > > you should make omap_i2c_read_reg() work fine for this case too. > > Just felt it is more readlable this way also I didnt want to use the > reg_shift etc. > > which also may get cleaned up sometime. correct, but I think for now using omap_i2c_read_reg() is fine. Maybe defining (for now) REV_HI and REV_LO on both reg_maps would do it? Just for the time being until we can get rid of reg_shift and omap_i2c_ip_version ?? I don't know, but to me using __raw_readl() with a hardcoded offset is a bit odd.
Hi, On Wed, Oct 31, 2012 at 04:28:38PM +0530, Shubhrajyoti wrote: > On Wednesday 31 October 2012 03:42 PM, Felipe Balbi wrote: > >> > >> > + if (!_dev->regs) > >> > + return 0; > > this is wrong, you need to make sure dev->regs is set early enough. > > > to set the dev->regs I use the value read from revision register. > > to read the revision register I do a get_sync first. This in turn leads > to the call to resume handler. > At this point the regs is NULL and so the check. > > After reading the register and checking the scheme bit. > the reg map is populated. > > How can this issue be solved? aaa good point, missed that. Nevermind that comment ;-) cheers
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index db31eae..651a7f7 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -51,7 +51,8 @@ /* 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_3630 0x40 +#define OMAP_I2C_REV_ON_4430_PLUS 0x5040 /* 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; + u16 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 */ @@ -1064,6 +1065,8 @@ omap_i2c_probe(struct platform_device *pdev) const struct of_device_id *match; int irq; int r; + u16 rev_lo; + u16 rev_hi; /* NOTE: driver uses the static register mapping */ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -1117,11 +1120,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 +1128,21 @@ 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-14 ie scheme this is 1 indicates ver2 or + * highlander. + * On omap3 Offset 4 is IE Reg the bit 14 is XDR_IE which is 0 at reset. + */ + rev_hi = __raw_readw(dev->base + 0x04); + + if (rev_hi & 0x4000) {/* If scheme 1*/ + dev->regs = (u8 *)reg_map_ip_v2; + dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_IP_V2_REVNB_HI); + rev_lo = omap_i2c_read_reg(dev, OMAP_I2C_IP_V2_REVNB_LO); + dev_info(dev->dev, "the low rev %x\n", rev_lo); + } else { + dev->regs = (u8 *)reg_map_ip_v1; + dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG) & 0xff; + } dev->errata = 0; @@ -1155,7 +1167,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 */ @@ -1264,6 +1276,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);
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. 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> --- todo: some of the flag checks can be removed in favour of revision check. drivers/i2c/busses/i2c-omap.c | 35 +++++++++++++++++++++++++---------- 1 files changed, 25 insertions(+), 10 deletions(-)