On Wed, Jun 20, 2012 at 7:44 PM, Tony Lindgren <tony@xxxxxxxxxxx> wrote: > * Shubhrajyoti <shubhrajyoti@xxxxxx> [120620 06:06]: >> On Wednesday 20 June 2012 04:02 PM, Tony Lindgren wrote: >> > * Shubhrajyoti D <shubhrajyoti@xxxxxx> [120618 07:35]: >> >> From: Jon Hunter <jon-hunter@xxxxxx> >> >> >> >> The OMAP3530 is based upon the same silicon as the OMAP3430 and so the I2C >> >> revision is the same for 3430 and 3530. However, the OMAP3630 device has the >> >> same I2C revision as OMAP4. Correct the revision definition to reflect this. >> >> >> >> This patch is based on work done by Jon Hunter <jon-hunter@xxxxxx> >> >> Changes from his patch >> >> - Update OMAP_I2C_REV_ON_3430 also to reflect that it is same as 3530 >> > ... >> >> /* timeout waiting for the controller to respond */ >> >> #define OMAP_I2C_TIMEOUT (msecs_to_jiffies(1000)) >> >> @@ -298,7 +298,7 @@ static int omap_i2c_reset(struct omap_i2c_dev *dev) >> >> omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, >> >> SYSC_AUTOIDLE_MASK); >> >> >> >> - } else if (dev->rev >= OMAP_I2C_REV_ON_3430) { >> >> + } else if (dev->rev >= OMAP_I2C_REV_ON_3430_3530) { >> >> dev->syscstate = SYSC_AUTOIDLE_MASK; >> >> dev->syscstate |= SYSC_ENAWAKEUP_MASK; >> >> dev->syscstate |= (SYSC_IDLEMODE_SMART << >> > Having to patch all over the place for these revision defines leads >> > into unmaintainable code as new SoCs get added. >> > >> > Please instead just check the revision once during init, then set >> > up some feature bits like I2C_OMAP_NEEDS_XYZ_RESET that the runtime >> > code can check. >> Even now at probe >> >> dev->rev is set by reading the rev register. >> >> The (macro)name used to identify is only changed. >> >> Also all the ips need reset the difference is that 2430 has less bits eg only autoidle and reset. >> So a flag needs reset may not be meaningful. > > Hmm OK so this is a cosmetic/readability fix.. Yes > > ..but your earlier patch now spreads the checking of revision > to the new non-init function omap_i2c_reset. > > Why don't you just do this cosmetic/readability rename fix > before your 03/13 patch? OK thats doable.I can reorder the patch. > Then set the already existing > OMAP_I2C_FLAG_RESET_REGS_POSTIDLE during init for some > revisions and use that instead of the rev check in > omap_i2c_reset? OMAP_I2C_FLAG_RESET_REGS_POSTIDLE is a hwmod flag whose intention is to find know if it can lose context after idle. The rev check that we have is because post reset( triggered by driver) to know if we have to restore only autoidle or other bits like clockactivity sidle , enable wakeup . > > Tony > > -- > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html