Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751620AbbDIFgS (ORCPT ); Thu, 9 Apr 2015 01:36:18 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:43846 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751258AbbDIFgP (ORCPT ); Thu, 9 Apr 2015 01:36:15 -0400 Message-ID: <28dae70b3f6435d2e0178c31154af9f5.squirrel@www.codeaurora.org> In-Reply-To: <5525BF1D.3050408@codeaurora.org> References: <1428499733-21963-1-git-send-email-gpramod@codeaurora.org> <1428499733-21963-2-git-send-email-gpramod@codeaurora.org> <5525BF1D.3050408@codeaurora.org> Date: Thu, 9 Apr 2015 11:06:14 +0530 Subject: Re: [PATCH v2 2/2] tty: serial: msm: Disable pclk when port is closed From: "Pramod Gurav" To: "Stephen Boyd" Cc: "Pramod Gurav" , linux-arm-msm@vger.kernel.org, linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org, bryanh@codeaurora.org, jslaby@suse.cz User-Agent: SquirrelMail/1.4.22-4.el6 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT X-Priority: 3 (Normal) Importance: Normal Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2676 Lines: 75 On Thu, April 9, 2015 5:21 am, Stephen Boyd wrote: > On 04/08/15 06:28, Pramod Gurav wrote: >> Disable the pclk when tty port is closed by user space. >> >> Signed-off-by: Pramod Gurav >> --- >> drivers/tty/serial/msm_serial.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/tty/serial/msm_serial.c >> b/drivers/tty/serial/msm_serial.c >> index 4c1e9ea..f38565c 100644 >> --- a/drivers/tty/serial/msm_serial.c >> +++ b/drivers/tty/serial/msm_serial.c >> @@ -523,6 +523,7 @@ static void msm_shutdown(struct uart_port *port) >> msm_write(port, 0, UART_IMR); /* disable interrupts */ >> >> clk_disable_unprepare(msm_port->clk); >> + clk_disable_unprepare(msm_port->pclk); >> >> free_irq(port->irq, port); >> } > > It's not clear to me at all when this clock is enabled and when it's > disabled during the lifetime of this driver. For example, why do we have > a .pm op to turn clocks on and off? Shouldn't they already be on? Can > you please explain when the clocks are turned on and off and what > userspace actions cause that to happen? Looking at drivers like > amba-pl010.c I don't see any .pm op, just a > clk_prepare_enable/clk_disable_unprepare pair in the startup and > shutdown ops. When a userspce opens a serial port the uart_startup (in serial_core) function is executed which changes the uart_pm state to UART_PM_STATE_ON. So when this port is release/closed by the application uart_close(serial_core) changes the uart_pm state to UART_PM_STATE_OFF if its not console. But it is not done in uart_shutdown function. So ideally clk_prepare_enable/clk_disable_unprepare must be called in .pm only. So, we can get rid of these operations from msm_startup function as these will be called anyway using .pm ops. About .pm in uart_ops, there are few drivers which have it for an example, atmel_serial, sh-sci etc. That is where they do clk_prepare_enable/clk_disable_unprepare. And moreover when there is suspend across system these function will handled through .pm. > > Minus my confusion of why our clocking is complicated, it looks correct > to me to do this, so > > Reviewed-by: Stephen Boyd > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > > Pramod -- 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/