| Message ID | 1428668396-6955-2-git-send-email-gpramod@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | Andy Gross |
| Headers | show |
On 04/10/15 05:19, Pramod Gurav wrote: > @@ -683,8 +679,7 @@ static void msm_power(struct uart_port *port, unsigned int state, > > switch (state) { > case 0: > - clk_prepare_enable(msm_port->clk); > - clk_prepare_enable(msm_port->pclk); > + msm_init_clock(port); Hm... now we would call msm_serial_set_mnd_regs() whenever we power on the port? Presumably we only need to do that once when we probe (or when we resume from a sleep state that resets the registers, i.e. hibernation) but I guess we're getting saved by the fact that the if/else if pair in msm_serial_set_mnd_regs_from_uartclk would never be true after the first time we call it? > break; > case 3: > clk_disable_unprepare(msm_port->clk);
Thanks Stephen for review. On Fri, April 10, 2015 11:33 pm, Stephen Boyd wrote: > On 04/10/15 05:19, Pramod Gurav wrote: >> @@ -683,8 +679,7 @@ static void msm_power(struct uart_port *port, >> unsigned int state, >> >> switch (state) { >> case 0: >> - clk_prepare_enable(msm_port->clk); >> - clk_prepare_enable(msm_port->pclk); >> + msm_init_clock(port); > > Hm... now we would call msm_serial_set_mnd_regs() whenever we power on > the port? Presumably we only need to do that once when we probe (or when > we resume from a sleep state that resets the registers, i.e. > hibernation) but I guess we're getting saved by the fact that the > if/else if pair in msm_serial_set_mnd_regs_from_uartclk would never be > true after the first time we call it? I tried replacing msm_init_clock() call with msm_serial_set_mnd_regs() in msm_startup() as msm_startup gets called just after msm_power() so that clk_prepare_enable() is followed by mnd settings. But it does not get the kernel booted for some reason. So, can I get a acked-by for this patch or you still think it can be done in a better way? > >> break; >> case 3: >> clk_disable_unprepare(msm_port->clk); > > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > > -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 04/29/15 08:45, Pramod Gurav wrote: > Thanks Stephen for review. > > On Fri, April 10, 2015 11:33 pm, Stephen Boyd wrote: >> On 04/10/15 05:19, Pramod Gurav wrote: >>> @@ -683,8 +679,7 @@ static void msm_power(struct uart_port *port, >>> unsigned int state, >>> >>> switch (state) { >>> case 0: >>> - clk_prepare_enable(msm_port->clk); >>> - clk_prepare_enable(msm_port->pclk); >>> + msm_init_clock(port); >> Hm... now we would call msm_serial_set_mnd_regs() whenever we power on >> the port? Presumably we only need to do that once when we probe (or when >> we resume from a sleep state that resets the registers, i.e. >> hibernation) but I guess we're getting saved by the fact that the >> if/else if pair in msm_serial_set_mnd_regs_from_uartclk would never be >> true after the first time we call it? > I tried replacing msm_init_clock() call with msm_serial_set_mnd_regs() in > msm_startup() as msm_startup gets called just after msm_power() so that > clk_prepare_enable() is followed by mnd settings. But it does not get the > kernel booted for some reason. That's concerning. Did you also drop the call to msm_init_clock() from msm_power() as is done in this patch? If that's done then it seems that nothing would be different besides the removal of clk_prepare_enable() in msm_startup(). > > So, can I get a acked-by for this patch or you still think it can be done > in a better way? Using msm_init_clock() in the msm_power() doesn't look symmetrical so if we can avoid it I would prefer that.
diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c index 99aba04..c32b088 100644 --- a/drivers/tty/serial/msm_serial.c +++ b/drivers/tty/serial/msm_serial.c @@ -494,8 +494,6 @@ static int msm_startup(struct uart_port *port) if (unlikely(ret)) return ret; - msm_init_clock(port); - if (likely(port->fifosize > 12)) rfr_level = port->fifosize - 12; else @@ -525,8 +523,6 @@ static void msm_shutdown(struct uart_port *port) msm_port->imr = 0; msm_write(port, 0, UART_IMR); /* disable interrupts */ - clk_disable_unprepare(msm_port->clk); - free_irq(port->irq, port); } @@ -683,8 +679,7 @@ static void msm_power(struct uart_port *port, unsigned int state, switch (state) { case 0: - clk_prepare_enable(msm_port->clk); - clk_prepare_enable(msm_port->pclk); + msm_init_clock(port); break; case 3: clk_disable_unprepare(msm_port->clk);
clk_prepare_enable/clk_disable_unprepare is already done in .pm function of uart_ops hence get rid of them from msm_startup and msm_shutdown. Signed-off-by: Pramod Gurav <gpramod@codeaurora.org> --- Changes since last version: - Decided to drop calls to clk operations drivers/tty/serial/msm_serial.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)