From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Raja, Govindraj" Subject: Re: [PATCH v2 04/12] Serial: OMAP: Add runtime pm support for omap-serial driver Date: Thu, 5 May 2011 11:18:46 +0530 Message-ID: References: <1304080796-625-1-git-send-email-govindraj.raja@ti.com> <1304080796-625-5-git-send-email-govindraj.raja@ti.com> <87hb9aqk53.fsf@ti.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0237288326==" Return-path: In-Reply-To: <87hb9aqk53.fsf@ti.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Kevin Hilman Cc: Paul Walmsley , Benoit Cousson , Tony Lindgren , Rajendra Nayak , linux-serial@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-Id: linux-serial@vger.kernel.org --===============0237288326== Content-Type: multipart/alternative; boundary=bcaec517a88a67247304a280ee66 --bcaec517a88a67247304a280ee66 Content-Type: text/plain; charset=ISO-8859-1 On Thu, May 5, 2011 at 2:05 AM, Kevin Hilman wrote: > "Govindraj.R" writes: > > > Adapts omap-serial driver to use pm_runtime api's. > > > > 1.) Populate reg values to uart port which can be used for context > restore. > > 2.) Moving context_restore func to driver from serial.c > > 3.) Adding port_enable/disable func to enable/disable given uart port. > > enable port using get_sync and disable using autosuspend. > > 4.) using runtime irq safe api to make get_sync be called from irq > context. > > > > Signed-off-by: Govindraj.R > > --- > > arch/arm/mach-omap2/serial.c | 16 ++ > > arch/arm/plat-omap/include/plat/omap-serial.h | 2 + > > drivers/tty/serial/omap-serial.c | 211 > ++++++++++++++++++++++--- > > 3 files changed, 203 insertions(+), 26 deletions(-) > > > > diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c > > index 8c1a4c7..314d82f 100644 > > --- a/arch/arm/mach-omap2/serial.c > > +++ b/arch/arm/mach-omap2/serial.c > > @@ -189,6 +189,21 @@ static void omap_serial_fill_default_pads(struct > omap_board_data *bdata) > > } > > } > > > > +static void omap_uart_wakeup_enable(struct platform_device *pdev, bool > enable) > > +{ > > + struct omap_uart_port_info *up = pdev->dev.platform_data; > > + > > + /* Set or clear wake-enable bit */ > > + if (up->wk_en && up->wk_mask) { > > + u32 v = __raw_readl(up->wk_en); > > + if (enable) > > + v |= up->wk_mask; > > + else > > + v &= ~up->wk_mask; > > + __raw_writel(v, up->wk_en); > > + } > > +} > > Rather than having the driver do this via the PRCM, can you do an > experiment? Can you try to leave the module-level wakeup enabled all > the time in the PRCM, and the driver can then enable/disable > module-level wakeups simply by toggling ENAWAKEUP in the module's > SYSCONFIG? If that works, than you can just use > omap_hwmod_enable_wakeup() for this and not need the above function. > If we leave PRCM uart module level bit enabled all time and we disable wakeup using sysfs echo disabled > /sys/devices/platform/omap/omap_uart.0/power/wakeup it wakes up after clock disable, however unsetting the bit ensures it doesn't wakeup after uart clocks are cut. As Paul suggested, How about adding omap_device/omap_hwmod function to set and unset the bit? Using .module_bit = OMAP3430_EN_UART1_SHIFT, from hwmod data? > > > static void omap_uart_idle_init(struct omap_uart_port_info *uart, > > unsigned short num) > > { > > @@ -332,6 +347,7 @@ void __init omap_serial_init_port(struct > omap_board_data *bdata) > > > > pdata->uartclk = OMAP24XX_BASE_BAUD * 16; > > pdata->flags = UPF_BOOT_AUTOCONF; > > + pdata->enable_wakeup = omap_uart_wakeup_enable; > > if (bdata->id == omap_uart_con_id) > > pdata->console_uart = true; > > > > <> > > + pm_runtime_use_autosuspend(&pdev->dev); > > + pm_runtime_set_autosuspend_delay(&pdev->dev, > > + OMAP_UART_AUTOSUSPEND_DELAY); > > + > > + pm_runtime_enable(&pdev->dev); > > + pm_runtime_irq_safe(&pdev->dev); > > + > > + if (omap_up_info->console_uart) { > > + od = to_omap_device(up->pdev); > > + omap_hwmod_idle(od->hwmods[0]); > > Driver should have know knowlege of the underlying hwmod. > > I assume this is here because of the usage of HWMOD_INIT_NO_IDLE and > HWMOD_INIT_NO_RESET, right? Yes correct. > With proper runtime PM, we should be able to > remove the need for using those flags. > For omap-uart used for earlyprintk's the early_console driver will use printch from debug macro to print through uart. During omap-hwmod is getting initialized --> _setup --> _idle ---> _disable_clocks at this point of time while printch is happening and hwmod disables uart clocks I dont have uart-console driver available which can enable clocks back. So I have to use those flags when earlyprintk is enabled. -- Thanks, Govindraj.R > > Kevin > > > + serial_omap_port_enable(up); > > + serial_omap_port_disable(up); > > + } > > + > > ui[pdev->id] = up; > > serial_omap_add_console_port(up); > > > > ret = uart_add_one_port(&serial_omap_reg, &up->port); > > if (ret != 0) > > - goto do_release_region; > > + goto err1; > > > > + dev_set_drvdata(&pdev->dev, up); > > platform_set_drvdata(pdev, up); > > + > > return 0; > > err: > > dev_err(&pdev->dev, "[UART%d]: failure [%s]: %d\n", > > pdev->id, __func__, ret); > > +err1: > > + kfree(up); > > do_release_region: > > release_mem_region(mem->start, (mem->end - mem->start) + 1); > > return ret; > > @@ -1318,20 +1421,76 @@ static int serial_omap_remove(struct > platform_device *dev) > > > > platform_set_drvdata(dev, NULL); > > if (up) { > > + pm_runtime_disable(&up->pdev->dev); > > uart_remove_one_port(&serial_omap_reg, &up->port); > > kfree(up); > > } > > return 0; > > } > > > > +static void omap_uart_restore_context(struct uart_omap_port *up) > > +{ > > + u16 efr = 0; > > + > > + serial_out(up, UART_OMAP_MDR1, up->mdr1); > > + serial_out(up, UART_LCR, 0xBF); /* Config B mode */ > > + efr = serial_in(up, UART_EFR); > > + serial_out(up, UART_EFR, UART_EFR_ECB); > > + serial_out(up, UART_LCR, 0x0); /* Operational mode */ > > + serial_out(up, UART_IER, 0x0); > > + serial_out(up, UART_LCR, 0xBF); /* Config B mode */ > > + serial_out(up, UART_DLL, up->dll); > > + serial_out(up, UART_DLM, up->dlh); > > + serial_out(up, UART_LCR, 0x0); /* Operational mode */ > > + serial_out(up, UART_IER, up->ier); > > + serial_out(up, UART_FCR, up->fcr); > > + serial_out(up, UART_LCR, 0x80); > > + serial_out(up, UART_MCR, up->mcr); > > + serial_out(up, UART_LCR, 0xBF); /* Config B mode */ > > + serial_out(up, UART_EFR, efr); > > + serial_out(up, UART_LCR, UART_LCR_WLEN8); > > + /* UART 16x mode */ > > + serial_out(up, UART_OMAP_MDR1, up->mdr1); > > +} > > + > > +static int omap_serial_runtime_suspend(struct device *dev) > > +{ > > + struct uart_omap_port *up = dev_get_drvdata(dev); > > + > > + if (!up) > > + goto done; > > + > > + if (device_may_wakeup(dev)) > > + up->enable_wakeup(up->pdev, true); > > + else > > + up->enable_wakeup(up->pdev, false); > > +done: > > + return 0; > > +} > > + > > +static int omap_serial_runtime_resume(struct device *dev) > > +{ > > + struct uart_omap_port *up = dev_get_drvdata(dev); > > + > > + if (up) > > + omap_uart_restore_context(up); > > + > > + return 0; > > +} > > + > > +static const struct dev_pm_ops omap_serial_dev_pm_ops = { > > + .suspend = serial_omap_suspend, > > + .resume = serial_omap_resume, > > + .runtime_suspend = omap_serial_runtime_suspend, > > + .runtime_resume = omap_serial_runtime_resume, > > +}; > > + > > static struct platform_driver serial_omap_driver = { > > .probe = serial_omap_probe, > > .remove = serial_omap_remove, > > - > > - .suspend = serial_omap_suspend, > > - .resume = serial_omap_resume, > > .driver = { > > .name = DRIVER_NAME, > > + .pm = &omap_serial_dev_pm_ops, > > }, > > }; > --bcaec517a88a67247304a280ee66 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable

On Thu, May 5, 2011 at 2:05 AM, Kevin Hi= lman <khilman@ti.com= > wrote:
"Govindraj.R" <govindraj.raja@ti.com> writes:

> Adapts omap-serial driver to use pm_runtime api's.
>
> 1.) Populate reg values to uart port which can be used for context res= tore.
> 2.) Moving context_restore func to driver from serial.c
> 3.) Adding port_enable/disable func to enable/disable given uart port.=
> =A0 =A0 enable port using get_sync and disable using autosuspend.
> 4.) using runtime irq safe api to make get_sync be called from irq con= text.
>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
> =A0arch/arm/mach-omap2/serial.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| = =A0 16 ++
> =A0arch/arm/plat-omap/include/plat/omap-serial.h | =A0 =A02 +
> =A0drivers/tty/serial/omap-serial.c =A0 =A0 =A0 =A0 =A0 =A0 =A0| =A021= 1 ++++++++++++++++++++++---
> =A03 files changed, 203 insertions(+), 26 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial= .c
> index 8c1a4c7..314d82f 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -189,6 +189,21 @@ static void omap_serial_fill_default_pads(struct = omap_board_data *bdata)
> =A0 =A0 =A0 }
> =A0}
>
> +static void omap_uart_wakeup_enable(struct platform_device *pdev, boo= l enable)
> +{
> + =A0 =A0 struct omap_uart_port_info *up =3D pdev->dev.platform_dat= a;
> +
> + =A0 =A0 /* Set or clear wake-enable bit */
> + =A0 =A0 if (up->wk_en && up->wk_mask) {
> + =A0 =A0 =A0 =A0 =A0 =A0 u32 v =3D __raw_readl(up->wk_en);
> + =A0 =A0 =A0 =A0 =A0 =A0 if (enable)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 v |=3D up->wk_mask;
> + =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 v &=3D ~up->wk_mask;<= br> > + =A0 =A0 =A0 =A0 =A0 =A0 __raw_writel(v, up->wk_en);
> + =A0 =A0 }
> +}

Rather than having the driver do this via the PRCM, can you do = an
experiment? =A0Can you try to leave the module-level wakeup enabled all
the time in the PRCM, and the driver can then enable/disable
module-level wakeups simply by toggling ENAWAKEUP in the module's
SYSCONFIG? =A0If that works, than you can just use
omap_hwmod_enable_wakeup() for this and not need the above function.


If we leave PRCM uart module l= evel bit enabled all time and we disable
wakeup using sysfs
=A0
echo=A0disabled=A0> /sys/devices/platform/omap/omap_u= art.0/power/wakeup

it wakes up after clock=A0disab= le, however unsetting the bit ensures it=A0doesn't
wakeup aft= er uart clocks are cut.

As Paul suggested, How about adding=A0o= map_device/omap_hwmod function
to set and unset the bit?

Using

.module_bit =3D OMAP3430= _EN_UART1_SHIFT,

f= rom hwmod data?

=A0

> =A0static void omap_uart_idle_init(struct omap_uart_port_info *uart, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 unsigned s= hort num)
> =A0{
> @@ -332,6 +347,7 @@ void __init omap_serial_init_port(struct omap_boar= d_data *bdata)
>
> =A0 =A0 =A0 pdata->uartclk =3D OMAP24XX_BASE_BAUD * 16;
> =A0 =A0 =A0 pdata->flags =3D UPF_BOOT_AUTOCONF;
> + =A0 =A0 pdata->enable_wakeup =3D omap_uart_wakeup_enable;
> =A0 =A0 =A0 if (bdata->id =3D=3D omap_uart_con_id)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 pdata->console_uart =3D true;
>


<<SNIP>&g= t;
=A0
> + =A0 =A0 pm_runtime_use_autosuspend(&pdev->dev);
> + =A0 =A0 pm_runtime_set_autosuspend_delay(&pdev->dev,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 OMAP_UART_AUTOSUSPEND_DELAY)= ;
> +
> + =A0 =A0 pm_runtime_enable(&pdev->dev);
> + =A0 =A0 pm_runtime_irq_safe(&pdev->dev);
> +
> + =A0 =A0 if (omap_up_info->console_uart) {
> + =A0 =A0 =A0 =A0 =A0 =A0 od =3D to_omap_device(up->pdev);
> + =A0 =A0 =A0 =A0 =A0 =A0 omap_hwmod_idle(od->hwmods[0]);

Driver should have know knowlege of the underlying hwmod.

I assume this is here because of the usage of HWMOD_INIT_NO_IDLE and
HWMOD_INIT_NO_RESET, right?

=A0=A0 Yes cor= rect.

=A0
=A0= With proper runtime PM, we should be able to
remove the need for using those flags.

=
For omap-uart use= d for earlyprintk's the early_console driver will
use printch from debug = macro to print through uart.

<= span class=3D"Apple-style-span" style=3D"border-collapse: collapse; font-fa= mily: arial, sans-serif; font-size: 13px; ">During omap-hwmod is getting in= itialized

--> _setup
=A0 =A0--> _idle
=A0 =A0 ---> _disable_clocks=

at this point of time while printch is happening and
hwmod disab= les uart clocks I dont have uart-console
driver available which can enab= le clocks back.

So I have to use those flags when earlyprintk=A0is enabled.

--
Thanks,
Govindraj.R
<= br>

=A0

Kevin

> + =A0 =A0 =A0 =A0 =A0 =A0 serial_omap_port_enable(up);
> + =A0 =A0 =A0 =A0 =A0 =A0 serial_omap_port_disable(up);
> + =A0 =A0 }
> +
> =A0 =A0 =A0 ui[pdev->id] =3D up;
> =A0 =A0 =A0 serial_omap_add_console_port(up);
>
> =A0 =A0 =A0 ret =3D uart_add_one_port(&serial_omap_reg, &up-&g= t;port);
> =A0 =A0 =A0 if (ret !=3D 0)
> - =A0 =A0 =A0 =A0 =A0 =A0 goto do_release_region;
> + =A0 =A0 =A0 =A0 =A0 =A0 goto err1;
>
> + =A0 =A0 dev_set_drvdata(&pdev->dev, up);
> =A0 =A0 =A0 platform_set_drvdata(pdev, up);
> +
> =A0 =A0 =A0 return 0;
> =A0err:
> =A0 =A0 =A0 dev_err(&pdev->dev, "[UART%d]: failure [%s]: %= d\n",
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pdev->i= d, __func__, ret);
> +err1:
> + =A0 =A0 kfree(up);
> =A0do_release_region:
> =A0 =A0 =A0 release_mem_region(mem->start, (mem->end - mem->s= tart) + 1);
> =A0 =A0 =A0 return ret;
> @@ -1318,20 +1421,76 @@ static int serial_omap_remove(struct platform_= device *dev)
>
> =A0 =A0 =A0 platform_set_drvdata(dev, NULL);
> =A0 =A0 =A0 if (up) {
> + =A0 =A0 =A0 =A0 =A0 =A0 pm_runtime_disable(&up->pdev->dev)= ;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 uart_remove_one_port(&serial_omap_reg,= &up->port);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 kfree(up);
> =A0 =A0 =A0 }
> =A0 =A0 =A0 return 0;
> =A0}
>
> +static void omap_uart_restore_context(struct uart_omap_port *up)
> +{
> + =A0 =A0 u16 efr =3D 0;
> +
> + =A0 =A0 serial_out(up, UART_OMAP_MDR1, up->mdr1);
> + =A0 =A0 serial_out(up, UART_LCR, 0xBF); /* Config B mode */
> + =A0 =A0 efr =3D serial_in(up, UART_EFR);
> + =A0 =A0 serial_out(up, UART_EFR, UART_EFR_ECB);
> + =A0 =A0 serial_out(up, UART_LCR, 0x0); /* Operational mode */
> + =A0 =A0 serial_out(up, UART_IER, 0x0);
> + =A0 =A0 serial_out(up, UART_LCR, 0xBF); /* Config B mode */
> + =A0 =A0 serial_out(up, UART_DLL, up->dll);
> + =A0 =A0 serial_out(up, UART_DLM, up->dlh);
> + =A0 =A0 serial_out(up, UART_LCR, 0x0); /* Operational mode */
> + =A0 =A0 serial_out(up, UART_IER, up->ier);
> + =A0 =A0 serial_out(up, UART_FCR, up->fcr);
> + =A0 =A0 serial_out(up, UART_LCR, 0x80);
> + =A0 =A0 serial_out(up, UART_MCR, up->mcr);
> + =A0 =A0 serial_out(up, UART_LCR, 0xBF); /* Config B mode */
> + =A0 =A0 serial_out(up, UART_EFR, efr);
> + =A0 =A0 serial_out(up, UART_LCR, UART_LCR_WLEN8);
> + =A0 =A0 /* UART 16x mode */
> + =A0 =A0 serial_out(up, UART_OMAP_MDR1, up->mdr1);
> +}
> +
> +static int omap_serial_runtime_suspend(struct device *dev)
> +{
> + =A0 =A0 struct uart_omap_port *up =3D dev_get_drvdata(dev);
> +
> + =A0 =A0 if (!up)
> + =A0 =A0 =A0 =A0 =A0 =A0 goto done;
> +
> + =A0 =A0 if (device_may_wakeup(dev))
> + =A0 =A0 =A0 =A0 =A0 =A0 up->enable_wakeup(up->pdev, true);
> + =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 up->enable_wakeup(up->pdev, false); > +done:
> + =A0 =A0 return 0;
> +}
> +
> +static int omap_serial_runtime_resume(struct device *dev)
> +{
> + =A0 =A0 struct uart_omap_port *up =3D dev_get_drvdata(dev);
> +
> + =A0 =A0 if (up)
> + =A0 =A0 =A0 =A0 =A0 =A0 omap_uart_restore_context(up);
> +
> + =A0 =A0 return 0;
> +}
> +
> +static const struct dev_pm_ops omap_serial_dev_pm_ops =3D {
> + =A0 =A0 .suspend =3D serial_omap_suspend,
> + =A0 =A0 .resume =3D serial_omap_resume,
> + =A0 =A0 .runtime_suspend =3D omap_serial_runtime_suspend,
> + =A0 =A0 .runtime_resume =3D omap_serial_runtime_resume,
> +};
> +
> =A0static struct platform_driver serial_omap_driver =3D {
> =A0 =A0 =A0 .probe =A0 =A0 =A0 =A0 =A0=3D serial_omap_probe,
> =A0 =A0 =A0 .remove =A0 =A0 =A0 =A0 =3D serial_omap_remove,
> -
> - =A0 =A0 .suspend =A0 =A0 =A0 =A0=3D serial_omap_suspend,
> - =A0 =A0 .resume =A0 =A0 =A0 =A0 =3D serial_omap_resume,
> =A0 =A0 =A0 .driver =A0 =A0 =A0 =A0 =3D {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 .name =A0 =3D DRIVER_NAME,
> + =A0 =A0 =A0 =A0 =A0 =A0 .pm =3D &omap_serial_dev_pm_ops,
> =A0 =A0 =A0 },
> =A0};

--bcaec517a88a67247304a280ee66-- --===============0237288326== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============0237288326==--