| Message ID | 4E32B88E.6030508@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Fri, Jul 29, 2011 at 07:11:34PM +0530, Shubhrajyoti wrote: > On Friday 29 July 2011 06:07 PM, Felipe Balbi wrote: > >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 ? > I wasn't aware of the conflict resolution part. Actually came across this > piece of code as per the discussion on the reset implementation patch > will update > the changelogs. > How about, > > From: Shubhrajyoti D<shubhrajyoti@ti.com> > > Currently for OMAP4 the I2C_WE is not programmed. > This patch enables the programming for OMAP4. > > Fixes a conflict resolution of Andy's patches. I think you need to be a bit more verbose here ;-) Describe what happened and point to the commit number and mailing list archives for references. Imagine someone else reads this commit half a year from now, will s/he have enough information to understand the background of this patch ?
Hi, On Fri, Jul 29, 2011 at 07:33:39PM +0530, Datta, Shubhrajyoti wrote: > On Fri, Jul 29, 2011 at 7:11 PM, Shubhrajyoti <[1]shubhrajyoti@ti.com> > wrote: > > On Friday 29 July 2011 06:07 PM, Felipe Balbi wrote: > > 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] > [2]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: > > [3]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 ? > > I wasn't aware of the conflict resolution part. Actually came across > this > piece of code as per the discussion on the reset implementation patch > will update > the changelogs. > How about, > > Earlier mail got corrupted resending this is much worse. What mail client are you using ? Maybe there are some tips on Documentation/email-clients.txt
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);