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