| Message ID | 1366198467-6757-4-git-send-email-sourav.poddar@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Wed, Apr 17, 2013 at 05:04:24PM +0530, Sourav Poddar wrote: > @@ -1632,6 +1650,8 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = { > SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume) > SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend, > serial_omap_runtime_resume, NULL) > + .prepare = serial_omap_prepare, > + .complete = serial_omap_complete, if CONFIG_PM_SLEEP isn't defined, this will break compilation.
Hi Felipe, On Thursday 18 April 2013 09:28 AM, Felipe Balbi wrote: > Hi, > > On Wed, Apr 17, 2013 at 05:04:24PM +0530, Sourav Poddar wrote: >> @@ -1632,6 +1650,8 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = { >> SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume) >> SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend, >> serial_omap_runtime_resume, NULL) >> + .prepare = serial_omap_prepare, >> + .complete = serial_omap_complete, > if CONFIG_PM_SLEEP isn't defined, this will break compilation. > True. Then, will it not be a better idea to add a similar macro[1] in include/linux/pm.h for prepare/complete callback as it is present for suspend/resume ?. [1]: #ifdef CONFIG_PM_SLEEP #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ .suspend = suspend_fn, \ .resume = resume_fn, \ .freeze = suspend_fn, \ .thaw = resume_fn, \ .poweroff = suspend_fn, \ .restore = resume_fn, #else #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) #endif ~Sourav -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Thu, Apr 18, 2013 at 05:37:48PM +0530, Sourav Poddar wrote: > Hi Felipe, > On Thursday 18 April 2013 09:28 AM, Felipe Balbi wrote: > >Hi, > > > >On Wed, Apr 17, 2013 at 05:04:24PM +0530, Sourav Poddar wrote: > >>@@ -1632,6 +1650,8 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = { > >> SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume) > >> SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend, > >> serial_omap_runtime_resume, NULL) > >>+ .prepare = serial_omap_prepare, > >>+ .complete = serial_omap_complete, > >if CONFIG_PM_SLEEP isn't defined, this will break compilation. > > > True. > > Then, will it not be a better idea to add a similar macro[1] in > include/linux/pm.h for > prepare/complete callback as it is present for suspend/resume ?. > > [1]: > #ifdef CONFIG_PM_SLEEP > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \ > .suspend = suspend_fn, \ > .resume = resume_fn, \ > .freeze = suspend_fn, \ > .thaw = resume_fn, \ > .poweroff = suspend_fn, \ > .restore = resume_fn, > #else > #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) > #endif might be :-)
Sourav Poddar <sourav.poddar@ti.com> writes: > The patch adapt the serial core/driver to take care of the case when "no_console_suspend" > is used in the bootargs. The patch will remove dependency to set od->flags to > "OMAP_DEVICE_NO_IDLE_ON_SUSPEND" in serial.c(non dt case) and omap_device.c(dt case). > > Prepare and complete callbacks will ensure that clocks remain active for the console > uart when "no_console_suspend" is used in the bootargs. > > Signed-off-by: Sourav Poddar <sourav.poddar@ti.com> This changelog needs a rework. The driver itself was not aware of od->flags and omap_device stuff in general, so it's not really relevant. The driver is also not directly managing clocks, int's only doing runtime PM callbacks. What you want to say in the changelog is that the driver manages "no_console_suspend" by preventing runtime PM during the suspend path, which forces the console UART to stay awake. > --- > drivers/tty/serial/omap-serial.c | 20 ++++++++++++++++++++ > 1 files changed, 20 insertions(+), 0 deletions(-) > > diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c > index 08332f3..9ef80cf 100644 > --- a/drivers/tty/serial/omap-serial.c > +++ b/drivers/tty/serial/omap-serial.c > @@ -1278,6 +1278,24 @@ static struct uart_driver serial_omap_reg = { > }; > > #ifdef CONFIG_PM_SLEEP > +static int serial_omap_prepare(struct device *dev) > +{ > + struct uart_omap_port *up = dev_get_drvdata(dev); > + > + if (!console_suspend_enabled && uart_console(&up->port)) > + pm_runtime_disable(dev); > + > + return 0; > +} > + > +static void serial_omap_complete(struct device *dev) > +{ > + struct uart_omap_port *up = dev_get_drvdata(dev); > + > + if (!console_suspend_enabled && uart_console(&up->port)) > + pm_runtime_enable(dev); > +} > + For compilation with !CONFIG_PM_SLEEP, you'll also need: #else #define serial_omap_prepare NULL #define serial_omap_prepare NULL #endif /* CONFIG_PM_SLEEP */ > static int serial_omap_suspend(struct device *dev) > { > struct uart_omap_port *up = dev_get_drvdata(dev); > @@ -1632,6 +1650,8 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = { > SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume) > SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend, > serial_omap_runtime_resume, NULL) > + .prepare = serial_omap_prepare, > + .complete = serial_omap_complete, > }; > > #if defined(CONFIG_OF) Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Kevin, On Thursday 18 April 2013 11:26 PM, Kevin Hilman wrote: > Sourav Poddar<sourav.poddar@ti.com> writes: > >> The patch adapt the serial core/driver to take care of the case when "no_console_suspend" >> is used in the bootargs. The patch will remove dependency to set od->flags to >> "OMAP_DEVICE_NO_IDLE_ON_SUSPEND" in serial.c(non dt case) and omap_device.c(dt case). >> >> Prepare and complete callbacks will ensure that clocks remain active for the console >> uart when "no_console_suspend" is used in the bootargs. >> >> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com> > This changelog needs a rework. The driver itself was not aware of > od->flags and omap_device stuff in general, so it's not really > relevant. The driver is also not directly managing clocks, int's only > doing runtime PM callbacks. > > What you want to say in the changelog is that the driver manages > "no_console_suspend" by preventing runtime PM during the suspend path, > which forces the console UART to stay awake. > Yes, looks to the point. Will update the changelog in the next version. >> --- >> drivers/tty/serial/omap-serial.c | 20 ++++++++++++++++++++ >> 1 files changed, 20 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c >> index 08332f3..9ef80cf 100644 >> --- a/drivers/tty/serial/omap-serial.c >> +++ b/drivers/tty/serial/omap-serial.c >> @@ -1278,6 +1278,24 @@ static struct uart_driver serial_omap_reg = { >> }; >> >> #ifdef CONFIG_PM_SLEEP >> +static int serial_omap_prepare(struct device *dev) >> +{ >> + struct uart_omap_port *up = dev_get_drvdata(dev); >> + >> + if (!console_suspend_enabled&& uart_console(&up->port)) >> + pm_runtime_disable(dev); >> + >> + return 0; >> +} >> + >> +static void serial_omap_complete(struct device *dev) >> +{ >> + struct uart_omap_port *up = dev_get_drvdata(dev); >> + >> + if (!console_suspend_enabled&& uart_console(&up->port)) >> + pm_runtime_enable(dev); >> +} >> + > For compilation with !CONFIG_PM_SLEEP, you'll also need: > > #else > #define serial_omap_prepare NULL > #define serial_omap_prepare NULL > #endif /* CONFIG_PM_SLEEP */ > Ok. Will change this. Though, just a query/proposal on this, will it be correct if we try to create a macro[1] in include/linux/pm.h for prepare/complete as it is done for suspend/resume. ? [1]: #ifdef CONFIG_PM_SLEEP #define SET_SYSTEM_SLEEP_PM_PREP_COMP_OPS(prepare_fn, complete_fn) \ .prepare = prepare_fn, \ .complete = complete_fn, \ #else #define SET_SYSTEM_SLEEP_PM_PREP_COMP_OPS(prepare_fn, complete_fn) #endif ~Sourav >> static int serial_omap_suspend(struct device *dev) >> { >> struct uart_omap_port *up = dev_get_drvdata(dev); >> @@ -1632,6 +1650,8 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = { >> SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume) >> SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend, >> serial_omap_runtime_resume, NULL) >> + .prepare = serial_omap_prepare, >> + .complete = serial_omap_complete, >> }; >> >> #if defined(CONFIG_OF) > Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sourav Poddar <sourav.poddar@ti.com> writes: > Hi Kevin, > On Thursday 18 April 2013 11:26 PM, Kevin Hilman wrote: >> Sourav Poddar<sourav.poddar@ti.com> writes: >> >>> The patch adapt the serial core/driver to take care of the case when "no_console_suspend" >>> is used in the bootargs. The patch will remove dependency to set od->flags to >>> "OMAP_DEVICE_NO_IDLE_ON_SUSPEND" in serial.c(non dt case) and omap_device.c(dt case). >>> >>> Prepare and complete callbacks will ensure that clocks remain active for the console >>> uart when "no_console_suspend" is used in the bootargs. >>> >>> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com> >> This changelog needs a rework. The driver itself was not aware of >> od->flags and omap_device stuff in general, so it's not really >> relevant. The driver is also not directly managing clocks, int's only >> doing runtime PM callbacks. >> >> What you want to say in the changelog is that the driver manages >> "no_console_suspend" by preventing runtime PM during the suspend path, >> which forces the console UART to stay awake. >> > Yes, looks to the point. Will update the changelog in the next version. >>> --- >>> drivers/tty/serial/omap-serial.c | 20 ++++++++++++++++++++ >>> 1 files changed, 20 insertions(+), 0 deletions(-) >>> >>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c >>> index 08332f3..9ef80cf 100644 >>> --- a/drivers/tty/serial/omap-serial.c >>> +++ b/drivers/tty/serial/omap-serial.c >>> @@ -1278,6 +1278,24 @@ static struct uart_driver serial_omap_reg = { >>> }; >>> >>> #ifdef CONFIG_PM_SLEEP >>> +static int serial_omap_prepare(struct device *dev) >>> +{ >>> + struct uart_omap_port *up = dev_get_drvdata(dev); >>> + >>> + if (!console_suspend_enabled&& uart_console(&up->port)) >>> + pm_runtime_disable(dev); >>> + >>> + return 0; >>> +} >>> + >>> +static void serial_omap_complete(struct device *dev) >>> +{ >>> + struct uart_omap_port *up = dev_get_drvdata(dev); >>> + >>> + if (!console_suspend_enabled&& uart_console(&up->port)) >>> + pm_runtime_enable(dev); >>> +} >>> + >> For compilation with !CONFIG_PM_SLEEP, you'll also need: >> >> #else >> #define serial_omap_prepare NULL >> #define serial_omap_prepare NULL >> #endif /* CONFIG_PM_SLEEP */ >> > Ok. Will change this. > Though, just a query/proposal on this, will it be correct if we try to > create a macro[1] > in include/linux/pm.h for prepare/complete as it is done for > suspend/resume. ? Sure, you could do that, but personally I don't think it's as broadly useful (otherwise, it would be there already.) Kevin > [1]: > #ifdef CONFIG_PM_SLEEP > #define SET_SYSTEM_SLEEP_PM_PREP_COMP_OPS(prepare_fn, complete_fn) \ > .prepare = prepare_fn, \ > .complete = complete_fn, \ > #else > #define SET_SYSTEM_SLEEP_PM_PREP_COMP_OPS(prepare_fn, complete_fn) > #endif > > ~Sourav >>> static int serial_omap_suspend(struct device *dev) >>> { >>> struct uart_omap_port *up = dev_get_drvdata(dev); >>> @@ -1632,6 +1650,8 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = { >>> SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume) >>> SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend, >>> serial_omap_runtime_resume, NULL) >>> + .prepare = serial_omap_prepare, >>> + .complete = serial_omap_complete, >>> }; >>> >>> #if defined(CONFIG_OF) >> Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c index 08332f3..9ef80cf 100644 --- a/drivers/tty/serial/omap-serial.c +++ b/drivers/tty/serial/omap-serial.c @@ -1278,6 +1278,24 @@ static struct uart_driver serial_omap_reg = { }; #ifdef CONFIG_PM_SLEEP +static int serial_omap_prepare(struct device *dev) +{ + struct uart_omap_port *up = dev_get_drvdata(dev); + + if (!console_suspend_enabled && uart_console(&up->port)) + pm_runtime_disable(dev); + + return 0; +} + +static void serial_omap_complete(struct device *dev) +{ + struct uart_omap_port *up = dev_get_drvdata(dev); + + if (!console_suspend_enabled && uart_console(&up->port)) + pm_runtime_enable(dev); +} + static int serial_omap_suspend(struct device *dev) { struct uart_omap_port *up = dev_get_drvdata(dev); @@ -1632,6 +1650,8 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(serial_omap_suspend, serial_omap_resume) SET_RUNTIME_PM_OPS(serial_omap_runtime_suspend, serial_omap_runtime_resume, NULL) + .prepare = serial_omap_prepare, + .complete = serial_omap_complete, }; #if defined(CONFIG_OF)
The patch adapt the serial core/driver to take care of the case when "no_console_suspend" is used in the bootargs. The patch will remove dependency to set od->flags to "OMAP_DEVICE_NO_IDLE_ON_SUSPEND" in serial.c(non dt case) and omap_device.c(dt case). Prepare and complete callbacks will ensure that clocks remain active for the console uart when "no_console_suspend" is used in the bootargs. Signed-off-by: Sourav Poddar <sourav.poddar@ti.com> --- drivers/tty/serial/omap-serial.c | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-)