Message ID | 1352118223-3796-1-git-send-email-shubhrajyoti@ti.com |
---|---|
State | New |
Headers | show |
On Mon, Nov 05, 2012 at 05:53:36PM +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. > > 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. > > Also since the omap_i2c_read_reg uses reg_map_ip_* a raw_readw is done > to fetch the revision register. > > 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. > Reviewed-by: Felipe Balbi <balbi@ti.com> > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > --- > v3: Fix the comments. > > drivers/i2c/busses/i2c-omap.c | 61 ++++++++++++++++++++++++++++++++--------- > 1 files changed, 48 insertions(+), 13 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index db31eae..5c6f538 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -49,9 +49,10 @@ > #define OMAP_I2C_OMAP1_REV_2 0x20 > > /* 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_2430 0x00000036 > +#define OMAP_I2C_REV_ON_3430_3530 0x0000003C > +#define OMAP_I2C_REV_ON_3630 0x00000040 > +#define OMAP_I2C_REV_ON_4430_PLUS 0x50400002 > > /* 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; > + u32 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 */ > @@ -1052,6 +1053,16 @@ static const struct of_device_id omap_i2c_of_match[] = { > MODULE_DEVICE_TABLE(of, omap_i2c_of_match); > #endif > > +#define OMAP_I2C_SCHEME(rev) ((rev & 0xc000) >> 14) > + > +#define OMAP_I2C_REV_SCHEME_0_MAJOR(rev) (rev >> 4) > +#define OMAP_I2C_REV_SCHEME_0_MINOR(rev) (rev & 0xf) > + > +#define OMAP_I2C_REV_SCHEME_1_MAJOR(rev) ((rev & 0x0700) >> 7) > +#define OMAP_I2C_REV_SCHEME_1_MINOR(rev) (rev & 0x1f) > +#define OMAP_I2C_SCHEME_0 0 > +#define OMAP_I2C_SCHEME_1 1 > + > static int __devinit > omap_i2c_probe(struct platform_device *pdev) > { > @@ -1064,6 +1075,8 @@ omap_i2c_probe(struct platform_device *pdev) > const struct of_device_id *match; > int irq; > int r; > + u32 rev; > + u16 minor, major; > > /* NOTE: driver uses the static register mapping */ > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > @@ -1117,11 +1130,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 +1138,31 @@ 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-[15:14] ie scheme this is 1 indicates ver2. > + * On omap1/3/2 Offset 4 is IE Reg the bit [15:14] is 0 at reset. > + * Also since the omap_i2c_read_reg uses reg_map_ip_* a > + * raw_readw is done. > + */ > + rev = __raw_readw(dev->base + 0x04); > + > + switch (OMAP_I2C_SCHEME(rev)) { > + case OMAP_I2C_SCHEME_0: > + dev->regs = (u8 *)reg_map_ip_v1; > + dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG); > + minor = OMAP_I2C_REV_SCHEME_0_MAJOR(dev->rev); > + major = OMAP_I2C_REV_SCHEME_0_MAJOR(dev->rev); > + break; > + case OMAP_I2C_SCHEME_1: > + /* FALLTHROUGH */ > + default: > + dev->regs = (u8 *)reg_map_ip_v2; > + rev = (rev << 16) | > + omap_i2c_read_reg(dev, OMAP_I2C_IP_V2_REVNB_LO); > + minor = OMAP_I2C_REV_SCHEME_1_MINOR(rev); > + major = OMAP_I2C_REV_SCHEME_1_MAJOR(rev); > + dev->rev = rev; > + } > > dev->errata = 0; > > @@ -1155,7 +1187,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 */ > @@ -1198,7 +1230,7 @@ omap_i2c_probe(struct platform_device *pdev) > } > > dev_info(dev->dev, "bus %d rev%d.%d.%d at %d kHz\n", adap->nr, > - dev->dtrev, dev->rev >> 4, dev->rev & 0xf, dev->speed); > + dev->dtrev, major, minor, dev->speed); > > of_i2c_register_devices(adap); > > @@ -1264,6 +1296,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); > -- > 1.7.5.4 >
On Mon, Nov 05, 2012 at 05:53:38PM +0530, Shubhrajyoti D wrote: > The dtrev is used only for the comments. Remove the same and use > the scheme instead to know if it is version2. > > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> Reviewed-by: Felipe Balbi <balbi@ti.com> > --- > v3: remove the scheme from the commments. > todo: remove the dtrev from hwmod etc. > > drivers/i2c/busses/i2c-omap.c | 12 +++++------- > 1 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index 737d843..5f0c06c 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -191,7 +191,6 @@ struct omap_i2c_dev { > u32 latency; /* maximum MPU wkup latency */ > struct pm_qos_request pm_qos_request; > u32 speed; /* Speed of bus in kHz */ > - u32 dtrev; /* extra revision from DT */ > u32 flags; > u16 cmd_err; > u8 *buf; > @@ -1076,7 +1075,7 @@ omap_i2c_probe(struct platform_device *pdev) > int irq; > int r; > u32 rev; > - u16 minor, major; > + u16 minor, major, scheme; > > /* NOTE: driver uses the static register mapping */ > mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > @@ -1108,7 +1107,6 @@ omap_i2c_probe(struct platform_device *pdev) > u32 freq = 100000; /* default to 100000 Hz */ > > pdata = match->data; > - dev->dtrev = pdata->rev; > dev->flags = pdata->flags; > > of_property_read_u32(node, "clock-frequency", &freq); > @@ -1117,7 +1115,6 @@ omap_i2c_probe(struct platform_device *pdev) > } else if (pdata != NULL) { > dev->speed = pdata->clkrate; > dev->flags = pdata->flags; > - dev->dtrev = pdata->rev; > } > > dev->dev = &pdev->dev; > @@ -1146,7 +1143,8 @@ omap_i2c_probe(struct platform_device *pdev) > */ > rev = __raw_readw(dev->base + 0x04); > > - switch (OMAP_I2C_SCHEME(rev)) { > + scheme = OMAP_I2C_SCHEME(rev); > + switch (scheme) { > case OMAP_I2C_SCHEME_0: > dev->regs = (u8 *)reg_map_ip_v1; > dev->rev = omap_i2c_read_reg(dev, OMAP_I2C_REV_REG); > @@ -1230,8 +1228,8 @@ omap_i2c_probe(struct platform_device *pdev) > goto err_unuse_clocks; > } > > - dev_info(dev->dev, "bus %d rev%d.%d.%d at %d kHz\n", adap->nr, > - dev->dtrev, major, minor, dev->speed); > + dev_info(dev->dev, "bus %d rev%d.%d at %d kHz\n", adap->nr, > + major, minor, dev->speed); > > of_i2c_register_devices(adap); > > -- > 1.7.5.4 >
Hi, On Mon, Nov 05, 2012 at 05:53:43PM +0530, Shubhrajyoti D wrote: > Currently after the reset the sysc is written with hardcoded values. > The patch reads the sysc register and writes back the same value > after reset. > > - Some unnecessary rev checks can be optimised. > - Also due to whatever reason the hwmod flags are changed > we will not reset the values. > - In some of the cases the minor values of the 2430 register > is different(0x37) in that case the autoidle setting may be missed. > > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > --- > drivers/i2c/busses/i2c-omap.c | 20 +++++--------------- > 1 files changed, 5 insertions(+), 15 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index 25f1564..a09acdc 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -302,7 +302,11 @@ static void __omap_i2c_init(struct omap_i2c_dev *dev) > static int omap_i2c_reset(struct omap_i2c_dev *dev) > { > unsigned long timeout; > + u16 sysc; > + > if (dev->rev >= OMAP_I2C_OMAP1_REV_2) { > + sysc = omap_i2c_read_reg(dev, OMAP_I2C_SYSC_REG); > + > /* Disable I2C controller before soft reset */ > omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, > omap_i2c_read_reg(dev, OMAP_I2C_CON_REG) & > @@ -324,22 +328,8 @@ static int omap_i2c_reset(struct omap_i2c_dev *dev) > } > > /* SYSC register is cleared by the reset; rewrite it */ > - if (dev->rev == OMAP_I2C_REV_ON_2430) { > - > - omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, > - SYSC_AUTOIDLE_MASK); > + omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, sysc); > > - } 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 << > - __ffs(SYSC_SIDLEMODE_MASK)); > - dev->syscstate |= (SYSC_CLOCKACTIVITY_FCLK << > - __ffs(SYSC_CLOCKACTIVITY_MASK)); > - > - omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, > - dev->syscstate); > - } not sure if this will work. What about the first time you call reset() ? won't SYSC just contain the reset values ?
On Monday 05 November 2012 07:44 PM, Felipe Balbi wrote: >> - dev->syscstate); >> > - } > not sure if this will work. What about the first time you call reset() ? > won't SYSC just contain the reset values ? No actually the hwmod sets the value.
Hi, On Mon, Nov 05, 2012 at 07:53:45PM +0530, Shubhrajyoti wrote: > On Monday 05 November 2012 07:44 PM, Felipe Balbi wrote: > >> - dev->syscstate); > >> > - } > > not sure if this will work. What about the first time you call reset() ? > > won't SYSC just contain the reset values ? > No actually the hwmod sets the value. ahaaa, ok good. Let's get an Ack from Benoit, then.
On 11/5/2012 3:25 PM, Felipe Balbi wrote: > Hi, > > On Mon, Nov 05, 2012 at 07:53:45PM +0530, Shubhrajyoti wrote: >> On Monday 05 November 2012 07:44 PM, Felipe Balbi wrote: >>>> - dev->syscstate); >>>>> - } >>> not sure if this will work. What about the first time you call reset() ? >>> won't SYSC just contain the reset values ? >> No actually the hwmod sets the value. > > ahaaa, ok good. Let's get an Ack from Benoit, then. Well, in fact, I'm a little bit surprised that we still have to hack the SYSC directly without using an omap_device API. I know that we have to do some weird stuff for reseting that IP, but didn't we already exposed something to allow that? Regards, Benoit
On 11/5/12, Cousson, Benoit <b-cousson@ti.com> wrote: > On 11/5/2012 3:25 PM, Felipe Balbi wrote: >> Hi, >> >> On Mon, Nov 05, 2012 at 07:53:45PM +0530, Shubhrajyoti wrote: >>> On Monday 05 November 2012 07:44 PM, Felipe Balbi wrote: >>>>> - dev->syscstate); >>>>>> - } >>>> not sure if this will work. What about the first time you call reset() >>>> ? >>>> won't SYSC just contain the reset values ? >>> No actually the hwmod sets the value. >> >> ahaaa, ok good. Let's get an Ack from Benoit, then. > > Well, in fact, I'm a little bit surprised that we still have to hack the there was an attempt [1] the pdata stuff may have issues with dt having to deal with more pdata [2] > SYSC directly without using an omap_device API. Paul was not happy with omap device api [3] As far as the patch is concerned it is only getting rid of the hard coded flags and the rev check. > I know that we have to > do some weird stuff for reseting that IP, but didn't we already exposed > something to allow that? > > Regards, > Benoit [1] http://www.spinics.net/lists/linux-i2c/msg06810.html [2] http://www.spinics.net/lists/linux-i2c/msg06937.html [3] http://www.spinics.net/lists/linux-i2c/msg06943.html
On Mon, Nov 5, 2012 at 5:53 PM, Shubhrajyoti D <shubhrajyoti@ti.com> wrote: > > Does the followiing > - Make the revision a 32- bit consisting of rev_lo amd rev_hi each > of 16 bits. > > - Also use the revision register for the erratum i207. > - Refactor the i2c_omap_init code. > > Adds a patch to remove the hardcoding sysc register. Instead > read register ,reset and then writeback the read value. > > Also more cleanup is possible will check on that subsequently. > > Previous discussions can be found > http://www.spinics.net/lists/linux-omap/msg81265.html > > > Tested on OMAP4430sdp ,4460 ,omap3630 ,3430 and omap2430. > > For omap2 testing the below patch was used > [PATCH] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set > If there are no further comments can this be considered for next. Thanks and Regards,
On Mon, Nov 05, 2012 at 05:53:35PM +0530, Shubhrajyoti D wrote: > Shubhrajyoti D (8): > i2c: omap: Fix the revision register read > i2c: omap: use revision check for OMAP_I2C_FLAG_APPLY_ERRATA_I207 > i2c: omap: remove the dtrev > ARM: i2c: omap: Remove the i207 errata flag > i2c: omap: re-factor omap_i2c_init function > i2c: omap: make reset a seperate function > i2c: omap: Restore i2c context always > i2c: omap: cleanup the sysc write Pushed the series to for-next, after fixing a trivial merge conflict caused by reverting the QoS patch.
On Wednesday 14 November 2012 05:34 PM, Wolfram Sang wrote: > On Mon, Nov 05, 2012 at 05:53:35PM +0530, Shubhrajyoti D wrote: > >> Shubhrajyoti D (8): >> i2c: omap: Fix the revision register read >> i2c: omap: use revision check for OMAP_I2C_FLAG_APPLY_ERRATA_I207 >> i2c: omap: remove the dtrev >> ARM: i2c: omap: Remove the i207 errata flag >> i2c: omap: re-factor omap_i2c_init function >> i2c: omap: make reset a seperate function >> i2c: omap: Restore i2c context always >> i2c: omap: cleanup the sysc write > Pushed the series to for-next, after fixing a trivial merge conflict > caused by reverting the QoS patch. Thanks. >