| Message ID | 1334142776-10583-1-git-send-email-shubhrajyoti@ti.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 11, 2012 at 04:42:39PM +0530, Shubhrajyoti D wrote: > The functions omap_i2c_unidle/idle are called from omap_i2c_runtime_resume > and omap_i2c_runtime_suspend which is compiled for CONFIG_PM_RUNTIME. > This patch removes the omap_i2c_unidle/idle functions and folds them > into the runtime callbacks. > > This fixes the below warn when CONFIG_PM_RUNTIME is not defined > > CC arch/arm/mach-omap2/board-ti8168evm.o > drivers/i2c/busses/i2c-omap.c:272: warning: 'omap_i2c_unidle' defined but not used > drivers/i2c/busses/i2c-omap.c:293: warning: 'omap_i2c_idle' defined but not used > CC net/ipv4/ip_forward.o > > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> how about just moving them into the #ifdef ? /me thinks it's easier to read with the function. Matter of taste though, it's not that much code.
On Wed, Apr 11, 2012 at 04:42:49PM +0530, Shubhrajyoti D wrote: > The various devm_ functions allocate memory that is released when a driver > detaches. This patch uses devm_kzalloc, devm_request_mem_region and > devm_ioremap for data that is allocated in the probe function of a platform > device and is only freed in the remove function. > > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > --- > drivers/i2c/busses/i2c-omap.c | 29 +++++++++-------------------- > 1 files changed, 9 insertions(+), 20 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index 121c52e..86be475 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -995,17 +995,17 @@ omap_i2c_probe(struct platform_device *pdev) > return -ENODEV; > } > > - ioarea = request_mem_region(mem->start, resource_size(mem), > - pdev->name); > + ioarea = devm_request_mem_region(&pdev->dev, mem->start, > + resource_size(mem), pdev->name); you could use devm_request_and_ioremap()
On Wed, Apr 11, 2012 at 04:42:52PM +0530, Shubhrajyoti D wrote: > Use SET_RUNTIME_PM_OPS macro to set runtime functions. > > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > --- > drivers/i2c/busses/i2c-omap.c | 11 ++++------- > 1 files changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index dd65416..28de1d2 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -1217,15 +1217,12 @@ static int omap_i2c_runtime_resume(struct device *dev) > > return 0; > } > +#endif > > static struct dev_pm_ops omap_i2c_pm_ops = { > - .runtime_suspend = omap_i2c_runtime_suspend, > - .runtime_resume = omap_i2c_runtime_resume, > + SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend, > + omap_i2c_runtime_resume, NULL) > }; > -#define OMAP_I2C_PM_OPS (&omap_i2c_pm_ops) > -#else > -#define OMAP_I2C_PM_OPS NULL > -#endif I think you should keep this define, otherwise the pm pointer will always be valid.
On Wednesday 11 April 2012 05:04 PM, Felipe Balbi wrote: > On Wed, Apr 11, 2012 at 04:42:49PM +0530, Shubhrajyoti D wrote: >> The various devm_ functions allocate memory that is released when a driver >> detaches. This patch uses devm_kzalloc, devm_request_mem_region and >> devm_ioremap for data that is allocated in the probe function of a platform >> device and is only freed in the remove function. >> >> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> >> --- >> drivers/i2c/busses/i2c-omap.c | 29 +++++++++-------------------- >> 1 files changed, 9 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c >> index 121c52e..86be475 100644 >> --- a/drivers/i2c/busses/i2c-omap.c >> +++ b/drivers/i2c/busses/i2c-omap.c >> @@ -995,17 +995,17 @@ omap_i2c_probe(struct platform_device *pdev) >> return -ENODEV; >> } >> >> - ioarea = request_mem_region(mem->start, resource_size(mem), >> - pdev->name); >> + ioarea = devm_request_mem_region(&pdev->dev, mem->start, >> + resource_size(mem), pdev->name); > you could use devm_request_and_ioremap() OK will do that. thanks,
Hi Felipe, On Wed, Apr 11, 2012 at 5:02 PM, Felipe Balbi <balbi@ti.com> wrote: > On Wed, Apr 11, 2012 at 04:42:39PM +0530, Shubhrajyoti D wrote: >> The functions omap_i2c_unidle/idle are called from omap_i2c_runtime_resume >> and omap_i2c_runtime_suspend which is compiled for CONFIG_PM_RUNTIME. >> This patch removes the omap_i2c_unidle/idle functions and folds them >> into the runtime callbacks. >> >> This fixes the below warn when CONFIG_PM_RUNTIME is not defined >> >> CC arch/arm/mach-omap2/board-ti8168evm.o >> drivers/i2c/busses/i2c-omap.c:272: warning: 'omap_i2c_unidle' defined but not used >> drivers/i2c/busses/i2c-omap.c:293: warning: 'omap_i2c_idle' defined but not used >> CC net/ipv4/ip_forward.o >> >> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > > how about just moving them into the #ifdef ? /me thinks it's easier to > read with the function. Matter of taste though, it's not that much code. Kevin preferred folding into the runtime functions. http://www.spinics.net/lists/linux-omap/msg62764.html Don't feel strongly though. > > -- > balbi
On Wed, Apr 11, 2012 at 05:29:07PM +0530, Shubhrajyoti Datta wrote: > Hi Felipe, > > On Wed, Apr 11, 2012 at 5:02 PM, Felipe Balbi <balbi@ti.com> wrote: > > On Wed, Apr 11, 2012 at 04:42:39PM +0530, Shubhrajyoti D wrote: > >> The functions omap_i2c_unidle/idle are called from omap_i2c_runtime_resume > >> and omap_i2c_runtime_suspend which is compiled for CONFIG_PM_RUNTIME. > >> This patch removes the omap_i2c_unidle/idle functions and folds them > >> into the runtime callbacks. > >> > >> This fixes the below warn when CONFIG_PM_RUNTIME is not defined > >> > >> CC arch/arm/mach-omap2/board-ti8168evm.o > >> drivers/i2c/busses/i2c-omap.c:272: warning: 'omap_i2c_unidle' defined but not used > >> drivers/i2c/busses/i2c-omap.c:293: warning: 'omap_i2c_idle' defined but not used > >> CC net/ipv4/ip_forward.o > >> > >> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > > > > how about just moving them into the #ifdef ? /me thinks it's easier to > > read with the function. Matter of taste though, it's not that much code. > > Kevin preferred folding into the runtime functions. > > http://www.spinics.net/lists/linux-omap/msg62764.html > > Don't feel strongly though. ok, I stand corrected ;-) No need to change your patch again ;-)
Bablpi, On Wed, Apr 11, 2012 at 17:05:38, Balbi, Felipe wrote: > On Wed, Apr 11, 2012 at 04:42:52PM +0530, Shubhrajyoti D wrote: > > Use SET_RUNTIME_PM_OPS macro to set runtime functions. > > > > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > > --- > > drivers/i2c/busses/i2c-omap.c | 11 ++++------- > > 1 files changed, 4 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > > index dd65416..28de1d2 100644 > > --- a/drivers/i2c/busses/i2c-omap.c > > +++ b/drivers/i2c/busses/i2c-omap.c > > @@ -1217,15 +1217,12 @@ static int omap_i2c_runtime_resume(struct device *dev) > > > > return 0; > > } > > +#endif > > > > static struct dev_pm_ops omap_i2c_pm_ops = { > > - .runtime_suspend = omap_i2c_runtime_suspend, > > - .runtime_resume = omap_i2c_runtime_resume, > > + SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend, > > + omap_i2c_runtime_resume, NULL) > > }; > > -#define OMAP_I2C_PM_OPS (&omap_i2c_pm_ops) > > -#else > > -#define OMAP_I2C_PM_OPS NULL > > -#endif > > I think you should keep this define, otherwise the pm pointer will > always be valid. No. using SET_RUNTIME_PM_OPS will make it NULL when !CONFIG_PM_RUNTIME Kindly correct me if I am wrong. > > -- > balbi > Regards, Gururaja
On Wed, Apr 11, 2012 at 12:09:22PM +0000, Hebbar, Gururaja wrote: > Bablpi, > > On Wed, Apr 11, 2012 at 17:05:38, Balbi, Felipe wrote: > > On Wed, Apr 11, 2012 at 04:42:52PM +0530, Shubhrajyoti D wrote: > > > Use SET_RUNTIME_PM_OPS macro to set runtime functions. > > > > > > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > > > --- > > > drivers/i2c/busses/i2c-omap.c | 11 ++++------- > > > 1 files changed, 4 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > > > index dd65416..28de1d2 100644 > > > --- a/drivers/i2c/busses/i2c-omap.c > > > +++ b/drivers/i2c/busses/i2c-omap.c > > > @@ -1217,15 +1217,12 @@ static int omap_i2c_runtime_resume(struct device *dev) > > > > > > return 0; > > > } > > > +#endif > > > > > > static struct dev_pm_ops omap_i2c_pm_ops = { > > > - .runtime_suspend = omap_i2c_runtime_suspend, > > > - .runtime_resume = omap_i2c_runtime_resume, > > > + SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend, > > > + omap_i2c_runtime_resume, NULL) > > > }; > > > -#define OMAP_I2C_PM_OPS (&omap_i2c_pm_ops) > > > -#else > > > -#define OMAP_I2C_PM_OPS NULL > > > -#endif > > > > I think you should keep this define, otherwise the pm pointer will > > always be valid. > > No. using SET_RUNTIME_PM_OPS will make it NULL when !CONFIG_PM_RUNTIME > Kindly correct me if I am wrong. you will have a defined structure with NULL members, but the structure itself is still valid.
On Wed, Apr 11, 2012 at 17:40:58, Balbi, Felipe wrote: > On Wed, Apr 11, 2012 at 12:09:22PM +0000, Hebbar, Gururaja wrote: > > Bablpi, > > > > On Wed, Apr 11, 2012 at 17:05:38, Balbi, Felipe wrote: > > > On Wed, Apr 11, 2012 at 04:42:52PM +0530, Shubhrajyoti D wrote: > > > > Use SET_RUNTIME_PM_OPS macro to set runtime functions. > > > > > > > > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> > > > > --- > > > > drivers/i2c/busses/i2c-omap.c | 11 ++++------- > > > > 1 files changed, 4 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > > > > index dd65416..28de1d2 100644 > > > > --- a/drivers/i2c/busses/i2c-omap.c > > > > +++ b/drivers/i2c/busses/i2c-omap.c > > > > @@ -1217,15 +1217,12 @@ static int omap_i2c_runtime_resume(struct device *dev) > > > > > > > > return 0; > > > > } > > > > +#endif > > > > > > > > static struct dev_pm_ops omap_i2c_pm_ops = { > > > > - .runtime_suspend = omap_i2c_runtime_suspend, > > > > - .runtime_resume = omap_i2c_runtime_resume, > > > > + SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend, > > > > + omap_i2c_runtime_resume, NULL) > > > > }; > > > > -#define OMAP_I2C_PM_OPS (&omap_i2c_pm_ops) > > > > -#else > > > > -#define OMAP_I2C_PM_OPS NULL > > > > -#endif > > > > > > I think you should keep this define, otherwise the pm pointer will > > > always be valid. > > > > No. using SET_RUNTIME_PM_OPS will make it NULL when !CONFIG_PM_RUNTIME > > Kindly correct me if I am wrong. > > you will have a defined structure with NULL members, but the structure > itself is still valid. Oops. You are correct. I stand corrected. > > -- > balbi > Regards, Gururaja
Hi Felipe, On Wed, Apr 11, 2012 at 5:05 PM, Felipe Balbi <balbi@ti.com> wrote: > On Wed, Apr 11, 2012 at 04:42:52PM +0530, Shubhrajyoti D wrote: >> Use SET_RUNTIME_PM_OPS macro to set runtime functions. >> >> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> >> --- >> drivers/i2c/busses/i2c-omap.c | 11 ++++------- >> 1 files changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c >> index dd65416..28de1d2 100644 >> --- a/drivers/i2c/busses/i2c-omap.c >> +++ b/drivers/i2c/busses/i2c-omap.c >> @@ -1217,15 +1217,12 @@ static int omap_i2c_runtime_resume(struct device *dev) >> >> return 0; >> } >> +#endif >> >> static struct dev_pm_ops omap_i2c_pm_ops = { >> - .runtime_suspend = omap_i2c_runtime_suspend, >> - .runtime_resume = omap_i2c_runtime_resume, >> + SET_RUNTIME_PM_OPS(omap_i2c_runtime_suspend, >> + omap_i2c_runtime_resume, NULL) >> }; >> -#define OMAP_I2C_PM_OPS (&omap_i2c_pm_ops) >> -#else >> -#define OMAP_I2C_PM_OPS NULL >> -#endif > > I think you should keep this define, otherwise the pm pointer will > always be valid. Will correct it . Thanks, > > -- > balbi
The patch series does the following - Warn fixes if CONFIG_PM_RUNTIME is not selected. - I2C register restore only if context if the context is lost - Bus busy recovery mechanism. - the reset is not done in init. - Adds a patch to use devm_* functions - Also checks the return type of the get_sync and in case of errors prevents register access, also print the cause of failure in case of errors. - In case of i2c remove register access was done without any get_sync fix the same. - Adds a pdata function pointer to do context save restore - Split the omap_i2c_isr to increase readability - Make the i2c use SET_RUNTIME_PM_OPS - Folds a patch from Tasslehoff to prevent any merge conflicts. - Prevents the XDUF flag to be set if the underflow condition is not met. - As per discussion in [1] .Adds a patch to rename the 1p153 errata and use the unique id instead as the section number in the recent errata docs has changed. - As discussed in [2] Paul has queued the flag for context restore patch, removing it from the series. [1] http://www.spinics.net/lists/linux-i2c/msg07607.html [2] http://www.spinics.net/lists/linux-i2c/msg07685.html Tested on omap4sdp and omap3sdp. The following changes since commit 258f742635360175564e9470eb060ff4d4b984e7: modpost: Fix modpost license checking of vmlinux.o (2012-04-09 20:52:56 -0700) are available in the git repository at: git://gitorious.org/linus-tree/linus-tree.git i2c_omap-next Jon Hunter (1): I2C: OMAP: Correct I2C revision for OMAP3 Shubhrajyoti D (15): I2C: OMAP: make omap_i2c_unidle/idle functions depend on CONFIG_PM_RUNTIME I2C: OMAP: Remove reset at init I2C: OMAP: I2C register restore only if context is lost I2C: OMAP: Fix the interrupt clearing in OMAP4 I2C: OMAP: Fix the mismatch of pm_runtime enable and disable I2C: OMAP: Optimise the remove code I2C: OMAP: Fix the error handling I2C: OMAP: Don't check if wait_for_completion_timeout() returns less than zero I2C: OMAP: use devm_* functions I2C: OMAP: Fix the crash in i2c remove I2C: OMAP: Handle error check for pm runtime I2C: OMAP: Use SET_RUNTIME_PM_OPS I2C: OMAP: make the read ready processing a separate function I2C: OMAP: Do not set the XUDF if the underflow is not reached I2C: OMAP: Rename the 1p153 to the erratum id i462 Tasslehoff Kjappfot (1): I2C: OMAP: fix missing handling of errata I2C_OMAP3_1P153 Vikram Pandita (1): I2C: OMAP: Recover from Bus Busy condition arch/arm/plat-omap/i2c.c | 3 + drivers/i2c/busses/i2c-omap.c | 348 +++++++++++++++++++++++------------------ include/linux/i2c-omap.h | 1 + 3 files changed, 198 insertions(+), 154 deletions(-)