Re: [PATCH v3 2/3] tty: serial: msm: Remove duplicate operations on clocks in startup/shutdown
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. -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/3] tty: serial: msm: Remove duplicate operations on clocks in startup/shutdown
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. -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/3] tty: serial: msm: Remove duplicate operations on clocks in startup/shutdown
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 majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- The Qualcomm Innovation Center, Inc. is a member of the 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/3] tty: serial: msm: Remove duplicate operations on clocks in startup/shutdown
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- The Qualcomm Innovation Center, Inc. is a member of the 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/