| Message ID | 1311940122-10681-1-git-send-email-shubhrajyoti@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Fri, Jul 29, 2011 at 05:18:42PM +0530, Shubhrajyoti D wrote: > Currently for OMAP4 the I2C_WE is not programmed. > This patch enables the programming for OMAP4. > > Reported-by: Santosh Shilimkar <santosh.shilimkar@ti.com> > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > --- > TODO: > Currently all the wakeup sources are enabled. > There is scope of optimising the same. Will revisit it. > Rebased on Kevin's wip/i2c branch > Tested on OMAP4430. > > drivers/i2c/busses/i2c-omap.c | 5 ++--- > 1 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index d05efe7..18cc0af 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -313,9 +313,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) > * REVISIT: Some wakeup sources might not be needed. > */ > dev->westate = OMAP_I2C_WE_ALL; > - if (dev->rev < OMAP_I2C_REV_ON_3530_4430) > - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, > - dev->westate); > + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, > + dev->westate); this is also enabling for 3530, have you tested there too ?? On the other hand, this looks like it's fixing a bogus change on commit a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4 (I2C: OMAP2+: address confused probed version naming) specifically on the hunk below [1]: @@ -379,7 +379,9 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wkup sources might not be needed. */ dev->westate = OMAP_I2C_WE_ALL; - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev->westate); + if (dev->rev < OMAP_I2C_REV_ON_3530_4430) + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev->westate); } } omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); if that's the case, you should either update your commit log stating that this is a fix to a bug introduced by that commit, or fold this patch in the same. You should also Cc the patch author to clarify why the dev->rev check was added. Andy, can you clarify why you added the revision check which didn't exist before ? [1] http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commitdiff;h=a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4
On 07/29/2011 01:07 PM, Somebody in the thread at some point said: Hi - > - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev->westate); > + if (dev->rev< OMAP_I2C_REV_ON_3530_4430) > + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, > + dev->westate); > Andy, can you clarify why you added the revision check which didn't > exist before ? > > [1] http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commitdiff;h=a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4 > At the time I wrote the patches back in March, the code there was different: there was a pre-extant test avoiding that line on 4430, and the patch is simply converting it to the new scheme. You can see it here: http://permalink.gmane.org/gmane.linux.ports.arm.omap/54940 @@ -379,7 +379,7 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wkup sources might not be needed. */ dev->westate = OMAP_I2C_WE_ALL; - if (dev->rev < OMAP_I2C_REV_ON_4430) + if (dev->rev < OMAP_I2C_REV_ON_3530_4430) omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev->westate); } I guess since March and before this got committed for 3.1, someone got a patch in first removing the test, so when my patchset was uplevelled for commit against 3.1-rc this conflict was dealt with by re-introducing the test. Long story short, it's there from me as a mechanical 1:1 renaming action as part of the fix that 3530 and 4430 (different) IPs return the same rev number. Despite how it now looks I didn't add it, so if Shubhrajyoti has reasons to think it should be gone again I have nothing against that at all. -Andy -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Jul 29, 2011 at 01:28:12PM +0100, "Andy Green (???)" wrote: > On 07/29/2011 01:07 PM, Somebody in the thread at some point said: > > Hi - > > >- omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev->westate); > >+ if (dev->rev< OMAP_I2C_REV_ON_3530_4430) > >+ omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, > >+ dev->westate); > > >Andy, can you clarify why you added the revision check which didn't > >exist before ? > > > >[1] http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commitdiff;h=a3a7acbcc3df4e9ecc12aa1fc435534d74ebbdf4 > > > > At the time I wrote the patches back in March, the code there was > different: there was a pre-extant test avoiding that line on 4430, > and the patch is simply converting it to the new scheme. You can see > it here: > > http://permalink.gmane.org/gmane.linux.ports.arm.omap/54940 > > @@ -379,7 +379,7 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) > * REVISIT: Some wkup sources might not be needed. > */ > dev->westate = OMAP_I2C_WE_ALL; > - if (dev->rev < OMAP_I2C_REV_ON_4430) > + if (dev->rev < OMAP_I2C_REV_ON_3530_4430) > omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, > dev->westate); > } > > I guess since March and before this got committed for 3.1, someone > got a patch in first removing the test, so when my patchset was > uplevelled for commit against 3.1-rc this conflict was dealt with by > re-introducing the test. > > Long story short, it's there from me as a mechanical 1:1 renaming > action as part of the fix that 3530 and 4430 (different) IPs return > the same rev number. Despite how it now looks I didn't add it, so if > Shubhrajyoti has reasons to think it should be gone again I have > nothing against that at all. yeah, looks like a bad conflict resolution. Shubhrajyoti, care to respin the patch and update commit log stating that it is fixing a bad conflict resolution or something ?
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index d05efe7..18cc0af 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -313,9 +313,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) * REVISIT: Some wakeup sources might not be needed. */ dev->westate = OMAP_I2C_WE_ALL; - if (dev->rev < OMAP_I2C_REV_ON_3530_4430) - omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, - dev->westate); + omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, + dev->westate); } omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0);
Currently for OMAP4 the I2C_WE is not programmed. This patch enables the programming for OMAP4. Reported-by: Santosh Shilimkar <santosh.shilimkar@ti.com> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> --- TODO: Currently all the wakeup sources are enabled. There is scope of optimising the same. Will revisit it. Rebased on Kevin's wip/i2c branch Tested on OMAP4430. drivers/i2c/busses/i2c-omap.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-)