Re: [PATCH] OMAP4: I2C: Enable the wakeup in I2C_WE

[Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>]

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

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-l0cyMroinI0@public.gmane.org>
> 
> 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 ?

-- 
balbi

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