Re: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Vaibhav, "Bedia, Vaibhav" writes: [...] >> So, my proposal is that Sourav remove that flag from the AM33xx hwmod >> when he removes this feature. > > Apologies for the delayed response. I was out of office for a couple of > days. > > I don't recall the exact kernel version in which I ended up adding > this flag to keep the clock running but yes after the change mentioned > below this flag is not required. I just did a sanity check by removing > this flag on v3.8 kernel and things work fine across suspend. Great, thanks for checking. That leaves only the UART driver, so after Sourav's changes, we will drop the flag completely. Kevin -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Kevin, On Thursday 11 April 2013 07:45 PM, Kevin Hilman wrote: Kevin Hilman writes: "Bedia, Vaibhav" writes: Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string "ti,am3352-ocmcram". ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. OK, but *today*, in *mainline*, where in the linux kernel (not the M3 firmware) is the OCMRAM clock cut during suspend? From what I can see, there is no driver for this device, so there are no system PM calls being done for that device, and thus no omap_device calls being done for that device, so the no_idle_on_suspend has no effect. OK, I think I confused things here, sorry. I was thinking runtime PM here, but wrote system PM. The no_idle_on_suspend feature only affects system PM, and the omap_device calls will still be called during system PM, even without a driver. That being said, the commit below[1], added in v3.6 should prevent the any automaic clock gating for devices without drivers bound. Since there is no driver for the OCM RAM block, you shouldn't be affected by the automatic idle on suspend anyways. So, my proposal is that Sourav remove that flag from the AM33xx hwmod when he removes this feature. Kevin Thanks a lot for your inputs and helping in bringing this thread to a logical conclusion. I will post a v4 for this patch along with other fixes/cleanups required as recommended by you and russell. Thanks, Sourav [1] commit 72bb6f9b51c82c820ddef892455a85b115460904 Author: Kevin Hilman Date: Tue Jul 10 15:29:04 2012 -0700 ARM: OMAP: omap_device: don't attempt late suspend if no driver bound Currently, the omap_device PM domain layer uses the late suspend and early resume callbacks to ensure devices are in their low power states. However, this is attempted even in cases where a driver probe has failed. If a driver's ->probe() method fails, the driver is likely in a state where it is not expecting its runtime PM callbacks to be called, yet currently the omap_device PM domain code attempts to call the drivers callbacks. To fix, use the omap_device driver_status field to check whether a driver is bound to the omap_device before attempting to trigger driver callbacks. Reviewed-by: Paul Walmsley Signed-off-by: Kevin Hilman -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Kevin, On Thu, Apr 11, 2013 at 19:45:33, Kevin Hilman wrote: > Kevin Hilman writes: > > > "Bedia, Vaibhav" writes: > > > >> Hi Sourav, > >> > >> On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: > >> [...] > >>> Yes, had a look at that and found your situation similar to UART. > >>> > >>> But how exactly this gets used, I mean I don't see any drivers/ in > >>> mainline > >>> making use of this compatible string "ti,am3352-ocmcram". ? > >> > >> OCMC clock is enabled during bootup (not sure whether that's the h/w > >> default or ROM does it) since the initial bootloader runs from there. > >> By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the > >> clock running. Right now the sram code under arch/arm/plat-omap/ is what > >> manages the OCMC. I guess this needs to move somewhere under drivers/ > >> and start managing the clocks. Even then we'll need a mechanism > >> to leave the clocks running as part of the kernel suspend process > >> since the assembly code which runs at the fag end of the suspend > >> process runs out of OCMC and hence we can't cut its clock. > >> > >> On AM335x, the OCMC clock is cut to have PER power domain transition > >> but that's done in the WKUP-M3 firmware when going down. During the > >> wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the > >> kernel resumes the h/w state is same. > > > > OK, but *today*, in *mainline*, where in the linux kernel (not the M3 > > firmware) is the OCMRAM clock cut during suspend? > > > > From what I can see, there is no driver for this device, so there are no > > system PM calls being done for that device, and thus no omap_device > > calls being done for that device, so the no_idle_on_suspend has no > > effect. > > OK, I think I confused things here, sorry. I was thinking runtime PM > here, but wrote system PM. The no_idle_on_suspend feature only affects > system PM, and the omap_device calls will still be called during system > PM, even without a driver. > > That being said, the commit below[1], added in v3.6 should prevent the > any automaic clock gating for devices without drivers bound. Since > there is no driver for the OCM RAM block, you shouldn't be affected by > the automatic idle on suspend anyways. > > So, my proposal is that Sourav remove that flag from the AM33xx hwmod > when he removes this feature. > Apologies for the delayed response. I was out of office for a couple of days. I don't recall the exact kernel version in which I ended up adding this flag to keep the clock running but yes after the change mentioned below this flag is not required. I just did a sanity check by removing this flag on v3.8 kernel and things work fine across suspend. Regards, Vaibhav -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Kevin, On Thu, Apr 11, 2013 at 19:45:33, Kevin Hilman wrote: Kevin Hilman khil...@linaro.org writes: Bedia, Vaibhav vaibhav.be...@ti.com writes: Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string ti,am3352-ocmcram. ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. OK, but *today*, in *mainline*, where in the linux kernel (not the M3 firmware) is the OCMRAM clock cut during suspend? From what I can see, there is no driver for this device, so there are no system PM calls being done for that device, and thus no omap_device calls being done for that device, so the no_idle_on_suspend has no effect. OK, I think I confused things here, sorry. I was thinking runtime PM here, but wrote system PM. The no_idle_on_suspend feature only affects system PM, and the omap_device calls will still be called during system PM, even without a driver. That being said, the commit below[1], added in v3.6 should prevent the any automaic clock gating for devices without drivers bound. Since there is no driver for the OCM RAM block, you shouldn't be affected by the automatic idle on suspend anyways. So, my proposal is that Sourav remove that flag from the AM33xx hwmod when he removes this feature. Apologies for the delayed response. I was out of office for a couple of days. I don't recall the exact kernel version in which I ended up adding this flag to keep the clock running but yes after the change mentioned below this flag is not required. I just did a sanity check by removing this flag on v3.8 kernel and things work fine across suspend. Regards, Vaibhav -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Kevin, On Thursday 11 April 2013 07:45 PM, Kevin Hilman wrote: Kevin Hilmankhil...@linaro.org writes: Bedia, Vaibhavvaibhav.be...@ti.com writes: Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string ti,am3352-ocmcram. ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. OK, but *today*, in *mainline*, where in the linux kernel (not the M3 firmware) is the OCMRAM clock cut during suspend? From what I can see, there is no driver for this device, so there are no system PM calls being done for that device, and thus no omap_device calls being done for that device, so the no_idle_on_suspend has no effect. OK, I think I confused things here, sorry. I was thinking runtime PM here, but wrote system PM. The no_idle_on_suspend feature only affects system PM, and the omap_device calls will still be called during system PM, even without a driver. That being said, the commit below[1], added in v3.6 should prevent the any automaic clock gating for devices without drivers bound. Since there is no driver for the OCM RAM block, you shouldn't be affected by the automatic idle on suspend anyways. So, my proposal is that Sourav remove that flag from the AM33xx hwmod when he removes this feature. Kevin Thanks a lot for your inputs and helping in bringing this thread to a logical conclusion. I will post a v4 for this patch along with other fixes/cleanups required as recommended by you and russell. Thanks, Sourav [1] commit 72bb6f9b51c82c820ddef892455a85b115460904 Author: Kevin Hilmankhil...@ti.com Date: Tue Jul 10 15:29:04 2012 -0700 ARM: OMAP: omap_device: don't attempt late suspend if no driver bound Currently, the omap_device PM domain layer uses the late suspend and early resume callbacks to ensure devices are in their low power states. However, this is attempted even in cases where a driver probe has failed. If a driver's -probe() method fails, the driver is likely in a state where it is not expecting its runtime PM callbacks to be called, yet currently the omap_device PM domain code attempts to call the drivers callbacks. To fix, use the omap_device driver_status field to check whether a driver is bound to the omap_device before attempting to trigger driver callbacks. Reviewed-by: Paul Walmsleyp...@pwsan.com Signed-off-by: Kevin Hilmankhil...@ti.com -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Vaibhav, Bedia, Vaibhav vaibhav.be...@ti.com writes: [...] So, my proposal is that Sourav remove that flag from the AM33xx hwmod when he removes this feature. Apologies for the delayed response. I was out of office for a couple of days. I don't recall the exact kernel version in which I ended up adding this flag to keep the clock running but yes after the change mentioned below this flag is not required. I just did a sanity check by removing this flag on v3.8 kernel and things work fine across suspend. Great, thanks for checking. That leaves only the UART driver, so after Sourav's changes, we will drop the flag completely. Kevin -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Kevin Hilman writes: > "Bedia, Vaibhav" writes: > >> Hi Sourav, >> >> On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: >> [...] >>> Yes, had a look at that and found your situation similar to UART. >>> >>> But how exactly this gets used, I mean I don't see any drivers/ in mainline >>> making use of this compatible string "ti,am3352-ocmcram". ? >> >> OCMC clock is enabled during bootup (not sure whether that's the h/w >> default or ROM does it) since the initial bootloader runs from there. >> By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the >> clock running. Right now the sram code under arch/arm/plat-omap/ is what >> manages the OCMC. I guess this needs to move somewhere under drivers/ >> and start managing the clocks. Even then we'll need a mechanism >> to leave the clocks running as part of the kernel suspend process >> since the assembly code which runs at the fag end of the suspend >> process runs out of OCMC and hence we can't cut its clock. >> >> On AM335x, the OCMC clock is cut to have PER power domain transition >> but that's done in the WKUP-M3 firmware when going down. During the >> wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the >> kernel resumes the h/w state is same. > > OK, but *today*, in *mainline*, where in the linux kernel (not the M3 > firmware) is the OCMRAM clock cut during suspend? > > From what I can see, there is no driver for this device, so there are no > system PM calls being done for that device, and thus no omap_device > calls being done for that device, so the no_idle_on_suspend has no > effect. OK, I think I confused things here, sorry. I was thinking runtime PM here, but wrote system PM. The no_idle_on_suspend feature only affects system PM, and the omap_device calls will still be called during system PM, even without a driver. That being said, the commit below[1], added in v3.6 should prevent the any automaic clock gating for devices without drivers bound. Since there is no driver for the OCM RAM block, you shouldn't be affected by the automatic idle on suspend anyways. So, my proposal is that Sourav remove that flag from the AM33xx hwmod when he removes this feature. Kevin [1] commit 72bb6f9b51c82c820ddef892455a85b115460904 Author: Kevin Hilman Date: Tue Jul 10 15:29:04 2012 -0700 ARM: OMAP: omap_device: don't attempt late suspend if no driver bound Currently, the omap_device PM domain layer uses the late suspend and early resume callbacks to ensure devices are in their low power states. However, this is attempted even in cases where a driver probe has failed. If a driver's ->probe() method fails, the driver is likely in a state where it is not expecting its runtime PM callbacks to be called, yet currently the omap_device PM domain code attempts to call the drivers callbacks. To fix, use the omap_device driver_status field to check whether a driver is bound to the omap_device before attempting to trigger driver callbacks. Reviewed-by: Paul Walmsley Signed-off-by: Kevin Hilman -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Kevin Hilman khil...@linaro.org writes: Bedia, Vaibhav vaibhav.be...@ti.com writes: Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string ti,am3352-ocmcram. ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. OK, but *today*, in *mainline*, where in the linux kernel (not the M3 firmware) is the OCMRAM clock cut during suspend? From what I can see, there is no driver for this device, so there are no system PM calls being done for that device, and thus no omap_device calls being done for that device, so the no_idle_on_suspend has no effect. OK, I think I confused things here, sorry. I was thinking runtime PM here, but wrote system PM. The no_idle_on_suspend feature only affects system PM, and the omap_device calls will still be called during system PM, even without a driver. That being said, the commit below[1], added in v3.6 should prevent the any automaic clock gating for devices without drivers bound. Since there is no driver for the OCM RAM block, you shouldn't be affected by the automatic idle on suspend anyways. So, my proposal is that Sourav remove that flag from the AM33xx hwmod when he removes this feature. Kevin [1] commit 72bb6f9b51c82c820ddef892455a85b115460904 Author: Kevin Hilman khil...@ti.com Date: Tue Jul 10 15:29:04 2012 -0700 ARM: OMAP: omap_device: don't attempt late suspend if no driver bound Currently, the omap_device PM domain layer uses the late suspend and early resume callbacks to ensure devices are in their low power states. However, this is attempted even in cases where a driver probe has failed. If a driver's -probe() method fails, the driver is likely in a state where it is not expecting its runtime PM callbacks to be called, yet currently the omap_device PM domain code attempts to call the drivers callbacks. To fix, use the omap_device driver_status field to check whether a driver is bound to the omap_device before attempting to trigger driver callbacks. Reviewed-by: Paul Walmsley p...@pwsan.com Signed-off-by: Kevin Hilman khil...@ti.com -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
"Bedia, Vaibhav" writes: > Hi Sourav, > > On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: > [...] >> Yes, had a look at that and found your situation similar to UART. >> >> But how exactly this gets used, I mean I don't see any drivers/ in mainline >> making use of this compatible string "ti,am3352-ocmcram". ? > > OCMC clock is enabled during bootup (not sure whether that's the h/w > default or ROM does it) since the initial bootloader runs from there. > By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the > clock running. Right now the sram code under arch/arm/plat-omap/ is what > manages the OCMC. I guess this needs to move somewhere under drivers/ > and start managing the clocks. Even then we'll need a mechanism > to leave the clocks running as part of the kernel suspend process > since the assembly code which runs at the fag end of the suspend > process runs out of OCMC and hence we can't cut its clock. > > On AM335x, the OCMC clock is cut to have PER power domain transition > but that's done in the WKUP-M3 firmware when going down. During the > wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the > kernel resumes the h/w state is same. OK, but *today*, in *mainline*, where in the linux kernel (not the M3 firmware) is the OCMRAM clock cut during suspend? >From what I can see, there is no driver for this device, so there are no system PM calls being done for that device, and thus no omap_device calls being done for that device, so the no_idle_on_suspend has no effect. Kevin -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] > Yes, had a look at that and found your situation similar to UART. > > But how exactly this gets used, I mean I don't see any drivers/ in mainline > making use of this compatible string "ti,am3352-ocmcram". ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. Regards, Vaibhav -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Vaibhav, On Wednesday 10 April 2013 11:49 AM, Bedia, Vaibhav wrote: Hi Sourav, Kevin, On Wed, Apr 10, 2013 at 11:37:28, Poddar, Sourav wrote: Hi, On Wednesday 10 April 2013 12:37 AM, Kevin Hilman wrote: Sourav Poddar writes: Hi Kevin, On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote: Sourav Poddarwrites: With dt boot, uart wakeup after suspend is non functional while using "no_console_suspend" in the bootargs. With "no_console_suspend" used, we should prevent the runtime suspend of the uart port which is getting used as an console. Cc: Santosh Shilimkar Cc: Felipe Balbi Cc: Rajendra nayak Tested on omap5430evm, omap4430sdp. Signed-off-by: Sourav Poddar Rather than make these special checks inside the driver's runtime PM callbacks, you should just disable runtime PM (pm_runtime_disable()) Then, this should be broken into 2 patches. 1) serial core: add the '->is_console' flag. (nit on naming: don't call it port_is_console, since the struct is already a uart_port) 2) In the OMAP UART driver's ->prepare callback, check the is_console flag and pm_runtime_disable() accordingly (then pm_runtime_enable() in the drivers's ->complete callback. Kevin I was working on your above suggestions, but realised there is not only console uart which has the requirement of keeping the clocks enabled while going on suspend. If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has "no_idle_on_suspend" property used. Can you please ask the AM33xx folks how (and why) this is being used? I don't see/find a driver for this device in mainline, so without a driver this flag will not be used. Looping in Vaibhav Bedia for ocmcram.. [Vaibhav]: There is a discussion going on about a cleaner way of handling ti, no_idle_on_suspend" part (as this is a sort of hack). We got a way around for UART ($subject) by making serial core/driver handle this for us. But with this, we will delete codes around "no_idle_on_suspend" flag in omap_device file. But, we realised that its not only UART which requires the clocks to be active whie going for suspend. There is a dts entry for ocmcram also. As Kevin also pointed out, we don't see a driver for this device in mainline, It would be great if you can explain how its getting used? You can find the complete discussion on v3 here: https://lkml.org/lkml/2013/4/5/239 The flag in question is used to ensure that the clock to OCMC RAM is not disabled by the PM code. From the changelog which added this flag: "Note: OCMC RAM is part of the PER power domain and supports retention. The assembly code for low power entry/exit will run from OCMC RAM. To ensure that the OMAP PM code does not attempt to disable the clock to OCMC RAM as part of the suspend process add the no_idle_on_suspend flag." We had discussed about the usage of this flag in the RFC version of the patch [1]. Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string "ti,am3352-ocmcram". ? Regards, Vaibhav [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129510.html -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Sourav, Kevin, On Wed, Apr 10, 2013 at 11:37:28, Poddar, Sourav wrote: > Hi, > On Wednesday 10 April 2013 12:37 AM, Kevin Hilman wrote: > > Sourav Poddar writes: > > > >> Hi Kevin, > >> On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote: > >>> Sourav Poddar writes: > >>> > With dt boot, uart wakeup after suspend is non functional while using > "no_console_suspend" in the bootargs. With "no_console_suspend" used, we > should prevent the runtime suspend of the uart port which is getting used > as an console. > > Cc: Santosh Shilimkar > Cc: Felipe Balbi > Cc: Rajendra nayak > Tested on omap5430evm, omap4430sdp. > > Signed-off-by: Sourav Poddar > >>> Rather than make these special checks inside the driver's runtime PM > >>> callbacks, you should just disable runtime PM (pm_runtime_disable()) > >>> > >>> Then, this should be broken into 2 patches. > >>> > >>> 1) serial core: add the '->is_console' flag. (nit on naming: don't call > >>> it port_is_console, since the struct is already a uart_port) > >>> > >>> 2) In the OMAP UART driver's ->prepare callback, check the is_console flag > >>> and pm_runtime_disable() accordingly (then pm_runtime_enable() in > >>> the drivers's ->complete callback. > >>> > >>> Kevin > >> I was working on your above suggestions, but realised there is not > >> only console > >> uart which has the requirement of keeping the clocks enabled while going on > >> suspend. > >> > >> If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has > >> "no_idle_on_suspend" property used. > > Can you please ask the AM33xx folks how (and why) this is being used? > > > > I don't see/find a driver for this device in mainline, so without a > > driver this flag will not be used. > > > Looping in Vaibhav Bedia for ocmcram.. > > [Vaibhav]: >There is a discussion going on about a cleaner way of handling >ti, no_idle_on_suspend" part (as this is a sort of hack). We got a way >around for UART ($subject) by making serial core/driver handle this > for us. >But with this, we will delete codes around "no_idle_on_suspend" flag in >omap_device file. > >But, we realised that its not only UART which requires the clocks to > be active >whie going for suspend. There is a dts entry for ocmcram also. > >As Kevin also pointed out, we don't see a driver for this device in > mainline, It would be >great if you can explain how its getting used? > >You can find the complete discussion on v3 here: > https://lkml.org/lkml/2013/4/5/239 > The flag in question is used to ensure that the clock to OCMC RAM is not disabled by the PM code. >From the changelog which added this flag: "Note: OCMC RAM is part of the PER power domain and supports retention. The assembly code for low power entry/exit will run from OCMC RAM. To ensure that the OMAP PM code does not attempt to disable the clock to OCMC RAM as part of the suspend process add the no_idle_on_suspend flag." We had discussed about the usage of this flag in the RFC version of the patch [1]. Regards, Vaibhav [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129510.html -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi,
On Wednesday 10 April 2013 12:37 AM, Kevin Hilman wrote:
Sourav Poddar writes:
Hi Kevin,
On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote:
Sourav Poddar writes:
With dt boot, uart wakeup after suspend is non functional while using
"no_console_suspend" in the bootargs. With "no_console_suspend" used, we
should prevent the runtime suspend of the uart port which is getting used
as an console.
Cc: Santosh Shilimkar
Cc: Felipe Balbi
Cc: Rajendra nayak
Tested on omap5430evm, omap4430sdp.
Signed-off-by: Sourav Poddar
Rather than make these special checks inside the driver's runtime PM
callbacks, you should just disable runtime PM (pm_runtime_disable())
Then, this should be broken into 2 patches.
1) serial core: add the '->is_console' flag. (nit on naming: don't call
it port_is_console, since the struct is already a uart_port)
2) In the OMAP UART driver's ->prepare callback, check the is_console flag
and pm_runtime_disable() accordingly (then pm_runtime_enable() in
the drivers's ->complete callback.
Kevin
I was working on your above suggestions, but realised there is not
only console
uart which has the requirement of keeping the clocks enabled while going on
suspend.
If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has
"no_idle_on_suspend" property used.
Can you please ask the AM33xx folks how (and why) this is being used?
I don't see/find a driver for this device in mainline, so without a
driver this flag will not be used.
Looping in Vaibhav Bedia for ocmcram..
[Vaibhav]:
There is a discussion going on about a cleaner way of handling
ti, no_idle_on_suspend" part (as this is a sort of hack). We got a way
around for UART ($subject) by making serial core/driver handle this
for us.
But with this, we will delete codes around "no_idle_on_suspend" flag in
omap_device file.
But, we realised that its not only UART which requires the clocks to
be active
whie going for suspend. There is a dts entry for ocmcram also.
As Kevin also pointed out, we don't see a driver for this device in
mainline, It would be
great if you can explain how its getting used?
You can find the complete discussion on v3 here:
https://lkml.org/lkml/2013/4/5/239
ocmcram: ocmcram@4030 {
compatible = "ti,am3352-ocmcram";
reg =<0x4030 0x1>;
ti,hwmods = "ocmcram";
ti,no_idle_on_suspend;
};
This property gets checked in omap_device file and correspondingly
od->flags is set.
Based on your above inputs, the patches which I cooked up is
inlined[1]. Though, the below
patches works fine for uart case. The patches will effect ocmcram case
and I am inling them
"just for discussion".
Could you also have a look at Russell's suggestion for getting rid of
the 'is_console' flag.
[Kevin]: Yes, will do that.
Thanks,
Kevin
~Sourav
--
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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi,
On Wednesday 10 April 2013 12:37 AM, Kevin Hilman wrote:
Sourav Poddarsourav.pod...@ti.com writes:
Hi Kevin,
On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote:
Sourav Poddarsourav.pod...@ti.com writes:
With dt boot, uart wakeup after suspend is non functional while using
no_console_suspend in the bootargs. With no_console_suspend used, we
should prevent the runtime suspend of the uart port which is getting used
as an console.
Cc: Santosh Shilimkarsantosh.shilim...@ti.com
Cc: Felipe Balbiba...@ti.com
Cc: Rajendra nayakrna...@ti.com
Tested on omap5430evm, omap4430sdp.
Signed-off-by: Sourav Poddarsourav.pod...@ti.com
Rather than make these special checks inside the driver's runtime PM
callbacks, you should just disable runtime PM (pm_runtime_disable())
Then, this should be broken into 2 patches.
1) serial core: add the '-is_console' flag. (nit on naming: don't call
it port_is_console, since the struct is already a uart_port)
2) In the OMAP UART driver's -prepare callback, check the is_console flag
and pm_runtime_disable() accordingly (then pm_runtime_enable() in
the drivers's -complete callback.
Kevin
I was working on your above suggestions, but realised there is not
only console
uart which has the requirement of keeping the clocks enabled while going on
suspend.
If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has
no_idle_on_suspend property used.
Can you please ask the AM33xx folks how (and why) this is being used?
I don't see/find a driver for this device in mainline, so without a
driver this flag will not be used.
Looping in Vaibhav Bedia for ocmcram..
[Vaibhav]:
There is a discussion going on about a cleaner way of handling
ti, no_idle_on_suspend part (as this is a sort of hack). We got a way
around for UART ($subject) by making serial core/driver handle this
for us.
But with this, we will delete codes around no_idle_on_suspend flag in
omap_device file.
But, we realised that its not only UART which requires the clocks to
be active
whie going for suspend. There is a dts entry for ocmcram also.
As Kevin also pointed out, we don't see a driver for this device in
mainline, It would be
great if you can explain how its getting used?
You can find the complete discussion on v3 here:
https://lkml.org/lkml/2013/4/5/239
ocmcram: ocmcram@4030 {
compatible = ti,am3352-ocmcram;
reg =0x4030 0x1;
ti,hwmods = ocmcram;
ti,no_idle_on_suspend;
};
This property gets checked in omap_device file and correspondingly
od-flags is set.
Based on your above inputs, the patches which I cooked up is
inlined[1]. Though, the below
patches works fine for uart case. The patches will effect ocmcram case
and I am inling them
just for discussion.
Could you also have a look at Russell's suggestion for getting rid of
the 'is_console' flag.
[Kevin]: Yes, will do that.
Thanks,
Kevin
~Sourav
--
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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Sourav, Kevin, On Wed, Apr 10, 2013 at 11:37:28, Poddar, Sourav wrote: Hi, On Wednesday 10 April 2013 12:37 AM, Kevin Hilman wrote: Sourav Poddarsourav.pod...@ti.com writes: Hi Kevin, On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote: Sourav Poddarsourav.pod...@ti.com writes: With dt boot, uart wakeup after suspend is non functional while using no_console_suspend in the bootargs. With no_console_suspend used, we should prevent the runtime suspend of the uart port which is getting used as an console. Cc: Santosh Shilimkarsantosh.shilim...@ti.com Cc: Felipe Balbiba...@ti.com Cc: Rajendra nayakrna...@ti.com Tested on omap5430evm, omap4430sdp. Signed-off-by: Sourav Poddarsourav.pod...@ti.com Rather than make these special checks inside the driver's runtime PM callbacks, you should just disable runtime PM (pm_runtime_disable()) Then, this should be broken into 2 patches. 1) serial core: add the '-is_console' flag. (nit on naming: don't call it port_is_console, since the struct is already a uart_port) 2) In the OMAP UART driver's -prepare callback, check the is_console flag and pm_runtime_disable() accordingly (then pm_runtime_enable() in the drivers's -complete callback. Kevin I was working on your above suggestions, but realised there is not only console uart which has the requirement of keeping the clocks enabled while going on suspend. If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has no_idle_on_suspend property used. Can you please ask the AM33xx folks how (and why) this is being used? I don't see/find a driver for this device in mainline, so without a driver this flag will not be used. Looping in Vaibhav Bedia for ocmcram.. [Vaibhav]: There is a discussion going on about a cleaner way of handling ti, no_idle_on_suspend part (as this is a sort of hack). We got a way around for UART ($subject) by making serial core/driver handle this for us. But with this, we will delete codes around no_idle_on_suspend flag in omap_device file. But, we realised that its not only UART which requires the clocks to be active whie going for suspend. There is a dts entry for ocmcram also. As Kevin also pointed out, we don't see a driver for this device in mainline, It would be great if you can explain how its getting used? You can find the complete discussion on v3 here: https://lkml.org/lkml/2013/4/5/239 The flag in question is used to ensure that the clock to OCMC RAM is not disabled by the PM code. From the changelog which added this flag: Note: OCMC RAM is part of the PER power domain and supports retention. The assembly code for low power entry/exit will run from OCMC RAM. To ensure that the OMAP PM code does not attempt to disable the clock to OCMC RAM as part of the suspend process add the no_idle_on_suspend flag. We had discussed about the usage of this flag in the RFC version of the patch [1]. Regards, Vaibhav [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129510.html -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Vaibhav, On Wednesday 10 April 2013 11:49 AM, Bedia, Vaibhav wrote: Hi Sourav, Kevin, On Wed, Apr 10, 2013 at 11:37:28, Poddar, Sourav wrote: Hi, On Wednesday 10 April 2013 12:37 AM, Kevin Hilman wrote: Sourav Poddarsourav.pod...@ti.com writes: Hi Kevin, On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote: Sourav Poddarsourav.pod...@ti.comwrites: With dt boot, uart wakeup after suspend is non functional while using no_console_suspend in the bootargs. With no_console_suspend used, we should prevent the runtime suspend of the uart port which is getting used as an console. Cc: Santosh Shilimkarsantosh.shilim...@ti.com Cc: Felipe Balbiba...@ti.com Cc: Rajendra nayakrna...@ti.com Tested on omap5430evm, omap4430sdp. Signed-off-by: Sourav Poddarsourav.pod...@ti.com Rather than make these special checks inside the driver's runtime PM callbacks, you should just disable runtime PM (pm_runtime_disable()) Then, this should be broken into 2 patches. 1) serial core: add the '-is_console' flag. (nit on naming: don't call it port_is_console, since the struct is already a uart_port) 2) In the OMAP UART driver's -prepare callback, check the is_console flag and pm_runtime_disable() accordingly (then pm_runtime_enable() in the drivers's -complete callback. Kevin I was working on your above suggestions, but realised there is not only console uart which has the requirement of keeping the clocks enabled while going on suspend. If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has no_idle_on_suspend property used. Can you please ask the AM33xx folks how (and why) this is being used? I don't see/find a driver for this device in mainline, so without a driver this flag will not be used. Looping in Vaibhav Bedia for ocmcram.. [Vaibhav]: There is a discussion going on about a cleaner way of handling ti, no_idle_on_suspend part (as this is a sort of hack). We got a way around for UART ($subject) by making serial core/driver handle this for us. But with this, we will delete codes around no_idle_on_suspend flag in omap_device file. But, we realised that its not only UART which requires the clocks to be active whie going for suspend. There is a dts entry for ocmcram also. As Kevin also pointed out, we don't see a driver for this device in mainline, It would be great if you can explain how its getting used? You can find the complete discussion on v3 here: https://lkml.org/lkml/2013/4/5/239 The flag in question is used to ensure that the clock to OCMC RAM is not disabled by the PM code. From the changelog which added this flag: Note: OCMC RAM is part of the PER power domain and supports retention. The assembly code for low power entry/exit will run from OCMC RAM. To ensure that the OMAP PM code does not attempt to disable the clock to OCMC RAM as part of the suspend process add the no_idle_on_suspend flag. We had discussed about the usage of this flag in the RFC version of the patch [1]. Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string ti,am3352-ocmcram. ? Regards, Vaibhav [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129510.html -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string ti,am3352-ocmcram. ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. Regards, Vaibhav -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Bedia, Vaibhav vaibhav.be...@ti.com writes: Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string ti,am3352-ocmcram. ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. OK, but *today*, in *mainline*, where in the linux kernel (not the M3 firmware) is the OCMRAM clock cut during suspend? From what I can see, there is no driver for this device, so there are no system PM calls being done for that device, and thus no omap_device calls being done for that device, so the no_idle_on_suspend has no effect. Kevin -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Russell, On Monday 08 April 2013 10:44 PM, Russell King - ARM Linux wrote: On Fri, Apr 05, 2013 at 06:45:33PM +0530, Sourav Poddar wrote: With dt boot, uart wakeup after suspend is non functional while using "no_console_suspend" in the bootargs. With "no_console_suspend" used, we should prevent the runtime suspend of the uart port which is getting used as an console. Cc: Santosh Shilimkar Cc: Felipe Balbi Cc: Rajendra nayak Tested on omap5430evm, omap4430sdp. Signed-off-by: Sourav Poddar --- v2->v3 Based on Kevin Hilman and Santosh Shilimkar comments, modified serial core/driver layer to bypass runtime suspend for console uart while using "no_console_suspend". This patch is based on Santosh Shilimkar serial patch[1] Rather than introducing this "port_is_console" thing, please move uart_console() into the serial_core.h header file, making it an inline function, and use that in omap-serial.c. Remember to fix drivers/tty/serial/mpc52xx_uart.c as well for that change. Thanks for the pointer. Will take care of your suggestions in the next version. ~Sourav -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Sourav Poddar writes:
> Hi Kevin,
> On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote:
>> Sourav Poddar writes:
>>
>>> With dt boot, uart wakeup after suspend is non functional while using
>>> "no_console_suspend" in the bootargs. With "no_console_suspend" used, we
>>> should prevent the runtime suspend of the uart port which is getting used
>>> as an console.
>>>
>>> Cc: Santosh Shilimkar
>>> Cc: Felipe Balbi
>>> Cc: Rajendra nayak
>>> Tested on omap5430evm, omap4430sdp.
>>>
>>> Signed-off-by: Sourav Poddar
>> Rather than make these special checks inside the driver's runtime PM
>> callbacks, you should just disable runtime PM (pm_runtime_disable())
>>
>> Then, this should be broken into 2 patches.
>>
>> 1) serial core: add the '->is_console' flag. (nit on naming: don't call
>> it port_is_console, since the struct is already a uart_port)
>>
>> 2) In the OMAP UART driver's ->prepare callback, check the is_console flag
>> and pm_runtime_disable() accordingly (then pm_runtime_enable() in
>> the drivers's ->complete callback.
>>
>> Kevin
>
> I was working on your above suggestions, but realised there is not
> only console
> uart which has the requirement of keeping the clocks enabled while going on
> suspend.
>
> If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has
> "no_idle_on_suspend" property used.
Can you please ask the AM33xx folks how (and why) this is being used?
I don't see/find a driver for this device in mainline, so without a
driver this flag will not be used.
> ocmcram: ocmcram@4030 {
> compatible = "ti,am3352-ocmcram";
> reg = <0x4030 0x1>;
> ti,hwmods = "ocmcram";
> ti,no_idle_on_suspend;
> };
> This property gets checked in omap_device file and correspondingly
> od->flags is set.
>
> Based on your above inputs, the patches which I cooked up is
> inlined[1]. Though, the below
> patches works fine for uart case. The patches will effect ocmcram case
> and I am inling them
> "just for discussion".
Could you also have a look at Russell's suggestion for getting rid of
the 'is_console' flag.
Thanks,
Kevin
--
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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Kevin,
On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote:
Sourav Poddar writes:
With dt boot, uart wakeup after suspend is non functional while using
"no_console_suspend" in the bootargs. With "no_console_suspend" used, we
should prevent the runtime suspend of the uart port which is getting used
as an console.
Cc: Santosh Shilimkar
Cc: Felipe Balbi
Cc: Rajendra nayak
Tested on omap5430evm, omap4430sdp.
Signed-off-by: Sourav Poddar
Rather than make these special checks inside the driver's runtime PM
callbacks, you should just disable runtime PM (pm_runtime_disable())
Then, this should be broken into 2 patches.
1) serial core: add the '->is_console' flag. (nit on naming: don't call
it port_is_console, since the struct is already a uart_port)
2) In the OMAP UART driver's ->prepare callback, check the is_console flag
and pm_runtime_disable() accordingly (then pm_runtime_enable() in
the drivers's ->complete callback.
Kevin
I was working on your above suggestions, but realised there is not only
console
uart which has the requirement of keeping the clocks enabled while going on
suspend.
If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has
"no_idle_on_suspend" property used.
ocmcram: ocmcram@4030 {
compatible = "ti,am3352-ocmcram";
reg = <0x4030 0x1>;
ti,hwmods = "ocmcram";
ti,no_idle_on_suspend;
};
This property gets checked in omap_device file and correspondingly
od->flags is set.
Based on your above inputs, the patches which I cooked up is inlined[1].
Though, the below
patches works fine for uart case. The patches will effect ocmcram case
and I am inling them
"just for discussion".
I am not sure, if there is any other cleaner way of getting around this
"no_idle_on_suspend" flag
as they are getting used for ocmcram also. ?
Thanks,
Sourav
[1]:
From: Sourav Poddar
Date: Wed, 13 Mar 2013 20:32:36 +0530
Subject: [PATCH 1/2] arm: mach-omap2: remove
"OMAP_DEVICE_NO_IDLE_ON_SUSPEND" check
Remove the "OMAP_DEVICE_NO_IDLE_ON_SUSPEND" check, since serial core and
driver
takes care of the case when "no_console_suspend" is used in the bootargs and
you need to keep the clock enable for console even while suspend.
Tested on omap5430evm, omap4430sdp.
Signed-off-by: Sourav Poddar
---
arch/arm/mach-omap2/omap_device.c |7 +--
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_device.c
b/arch/arm/mach-omap2/omap_device.c
index 381be7a..d6dce8f 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -620,11 +620,8 @@ static int _od_suspend_noirq(struct device *dev)
ret = pm_generic_suspend_noirq(dev);
if (!ret && !pm_runtime_status_suspended(dev)) {
- if (pm_generic_runtime_suspend(dev) == 0) {
- if (!(od->flags & OMAP_DEVICE_NO_IDLE_ON_SUSPEND))
- omap_device_idle(pdev);
+ if (pm_generic_runtime_suspend(dev) == 0)
od->flags |= OMAP_DEVICE_SUSPENDED;
- }
}
return ret;
@@ -638,8 +635,6 @@ static int _od_resume_noirq(struct device *dev)
if ((od->flags & OMAP_DEVICE_SUSPENDED) &&
!pm_runtime_status_suspended(dev)) {
od->flags &= ~OMAP_DEVICE_SUSPENDED;
- if (!(od->flags & OMAP_DEVICE_NO_IDLE_ON_SUSPEND))
- omap_device_enable(pdev);
pm_generic_runtime_resume(dev);
}
--
From: Sourav Poddar
Date: Wed, 13 Mar 2013 20:32:36 +0530
Subject: [PATCH 2/2] driver: serial: omap: add prepare/complete callback
for "no_console_suspend" case
This patch adapt the serial core/driver to take care of the case when
"no_console_suspend"
is used in the bootargs. This 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.
Tested on omap5430evm, omap4430sdp.
Signed-off-by: Sourav Poddar
---
drivers/tty/serial/omap-serial.c | 20
drivers/tty/serial/serial_core.c |3 +++
include/linux/serial_core.h |1 +
3 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 08332f3..b726b2b 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 && up->port.is_console)
+ pm_runtime_disable(dev);
+
+ return 0;
+}
+
Re: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Kevin,
On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote:
Sourav Poddarsourav.pod...@ti.com writes:
With dt boot, uart wakeup after suspend is non functional while using
no_console_suspend in the bootargs. With no_console_suspend used, we
should prevent the runtime suspend of the uart port which is getting used
as an console.
Cc: Santosh Shilimkarsantosh.shilim...@ti.com
Cc: Felipe Balbiba...@ti.com
Cc: Rajendra nayakrna...@ti.com
Tested on omap5430evm, omap4430sdp.
Signed-off-by: Sourav Poddarsourav.pod...@ti.com
Rather than make these special checks inside the driver's runtime PM
callbacks, you should just disable runtime PM (pm_runtime_disable())
Then, this should be broken into 2 patches.
1) serial core: add the '-is_console' flag. (nit on naming: don't call
it port_is_console, since the struct is already a uart_port)
2) In the OMAP UART driver's -prepare callback, check the is_console flag
and pm_runtime_disable() accordingly (then pm_runtime_enable() in
the drivers's -complete callback.
Kevin
I was working on your above suggestions, but realised there is not only
console
uart which has the requirement of keeping the clocks enabled while going on
suspend.
If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has
no_idle_on_suspend property used.
ocmcram: ocmcram@4030 {
compatible = ti,am3352-ocmcram;
reg = 0x4030 0x1;
ti,hwmods = ocmcram;
ti,no_idle_on_suspend;
};
This property gets checked in omap_device file and correspondingly
od-flags is set.
Based on your above inputs, the patches which I cooked up is inlined[1].
Though, the below
patches works fine for uart case. The patches will effect ocmcram case
and I am inling them
just for discussion.
I am not sure, if there is any other cleaner way of getting around this
no_idle_on_suspend flag
as they are getting used for ocmcram also. ?
Thanks,
Sourav
[1]:
From: Sourav Poddar sourav.pod...@ti.com
Date: Wed, 13 Mar 2013 20:32:36 +0530
Subject: [PATCH 1/2] arm: mach-omap2: remove
OMAP_DEVICE_NO_IDLE_ON_SUSPEND check
Remove the OMAP_DEVICE_NO_IDLE_ON_SUSPEND check, since serial core and
driver
takes care of the case when no_console_suspend is used in the bootargs and
you need to keep the clock enable for console even while suspend.
Tested on omap5430evm, omap4430sdp.
Signed-off-by: Sourav Poddar sourav.pod...@ti.com
---
arch/arm/mach-omap2/omap_device.c |7 +--
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_device.c
b/arch/arm/mach-omap2/omap_device.c
index 381be7a..d6dce8f 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -620,11 +620,8 @@ static int _od_suspend_noirq(struct device *dev)
ret = pm_generic_suspend_noirq(dev);
if (!ret !pm_runtime_status_suspended(dev)) {
- if (pm_generic_runtime_suspend(dev) == 0) {
- if (!(od-flags OMAP_DEVICE_NO_IDLE_ON_SUSPEND))
- omap_device_idle(pdev);
+ if (pm_generic_runtime_suspend(dev) == 0)
od-flags |= OMAP_DEVICE_SUSPENDED;
- }
}
return ret;
@@ -638,8 +635,6 @@ static int _od_resume_noirq(struct device *dev)
if ((od-flags OMAP_DEVICE_SUSPENDED)
!pm_runtime_status_suspended(dev)) {
od-flags = ~OMAP_DEVICE_SUSPENDED;
- if (!(od-flags OMAP_DEVICE_NO_IDLE_ON_SUSPEND))
- omap_device_enable(pdev);
pm_generic_runtime_resume(dev);
}
--
From: Sourav Poddar sourav.pod...@ti.com
Date: Wed, 13 Mar 2013 20:32:36 +0530
Subject: [PATCH 2/2] driver: serial: omap: add prepare/complete callback
for no_console_suspend case
This patch adapt the serial core/driver to take care of the case when
no_console_suspend
is used in the bootargs. This 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.
Tested on omap5430evm, omap4430sdp.
Signed-off-by: Sourav Poddar sourav.pod...@ti.com
---
drivers/tty/serial/omap-serial.c | 20
drivers/tty/serial/serial_core.c |3 +++
include/linux/serial_core.h |1 +
3 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 08332f3..b726b2b 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);
+
+
Re: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Sourav Poddar sourav.pod...@ti.com writes:
Hi Kevin,
On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote:
Sourav Poddarsourav.pod...@ti.com writes:
With dt boot, uart wakeup after suspend is non functional while using
no_console_suspend in the bootargs. With no_console_suspend used, we
should prevent the runtime suspend of the uart port which is getting used
as an console.
Cc: Santosh Shilimkarsantosh.shilim...@ti.com
Cc: Felipe Balbiba...@ti.com
Cc: Rajendra nayakrna...@ti.com
Tested on omap5430evm, omap4430sdp.
Signed-off-by: Sourav Poddarsourav.pod...@ti.com
Rather than make these special checks inside the driver's runtime PM
callbacks, you should just disable runtime PM (pm_runtime_disable())
Then, this should be broken into 2 patches.
1) serial core: add the '-is_console' flag. (nit on naming: don't call
it port_is_console, since the struct is already a uart_port)
2) In the OMAP UART driver's -prepare callback, check the is_console flag
and pm_runtime_disable() accordingly (then pm_runtime_enable() in
the drivers's -complete callback.
Kevin
I was working on your above suggestions, but realised there is not
only console
uart which has the requirement of keeping the clocks enabled while going on
suspend.
If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has
no_idle_on_suspend property used.
Can you please ask the AM33xx folks how (and why) this is being used?
I don't see/find a driver for this device in mainline, so without a
driver this flag will not be used.
ocmcram: ocmcram@4030 {
compatible = ti,am3352-ocmcram;
reg = 0x4030 0x1;
ti,hwmods = ocmcram;
ti,no_idle_on_suspend;
};
This property gets checked in omap_device file and correspondingly
od-flags is set.
Based on your above inputs, the patches which I cooked up is
inlined[1]. Though, the below
patches works fine for uart case. The patches will effect ocmcram case
and I am inling them
just for discussion.
Could you also have a look at Russell's suggestion for getting rid of
the 'is_console' flag.
Thanks,
Kevin
--
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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Russell, On Monday 08 April 2013 10:44 PM, Russell King - ARM Linux wrote: On Fri, Apr 05, 2013 at 06:45:33PM +0530, Sourav Poddar wrote: With dt boot, uart wakeup after suspend is non functional while using no_console_suspend in the bootargs. With no_console_suspend used, we should prevent the runtime suspend of the uart port which is getting used as an console. Cc: Santosh Shilimkarsantosh.shilim...@ti.com Cc: Felipe Balbiba...@ti.com Cc: Rajendra nayakrna...@ti.com Tested on omap5430evm, omap4430sdp. Signed-off-by: Sourav Poddarsourav.pod...@ti.com --- v2-v3 Based on Kevin Hilman and Santosh Shilimkar comments, modified serial core/driver layer to bypass runtime suspend for console uart while using no_console_suspend. This patch is based on Santosh Shilimkar serial patch[1] Rather than introducing this port_is_console thing, please move uart_console() into the serial_core.h header file, making it an inline function, and use that in omap-serial.c. Remember to fix drivers/tty/serial/mpc52xx_uart.c as well for that change. Thanks for the pointer. Will take care of your suggestions in the next version. ~Sourav -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
On Fri, Apr 05, 2013 at 06:45:33PM +0530, Sourav Poddar wrote: > With dt boot, uart wakeup after suspend is non functional while using > "no_console_suspend" in the bootargs. With "no_console_suspend" used, we > should prevent the runtime suspend of the uart port which is getting used > as an console. > > Cc: Santosh Shilimkar > Cc: Felipe Balbi > Cc: Rajendra nayak > Tested on omap5430evm, omap4430sdp. > > Signed-off-by: Sourav Poddar > --- > v2->v3 > Based on Kevin Hilman and Santosh Shilimkar comments, modified > serial core/driver layer to bypass runtime suspend > for console uart while using "no_console_suspend". > > This patch is based on Santosh Shilimkar serial patch[1] Rather than introducing this "port_is_console" thing, please move uart_console() into the serial_core.h header file, making it an inline function, and use that in omap-serial.c. Remember to fix drivers/tty/serial/mpc52xx_uart.c as well for that change. -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
On Fri, Apr 05, 2013 at 06:45:33PM +0530, Sourav Poddar wrote: With dt boot, uart wakeup after suspend is non functional while using no_console_suspend in the bootargs. With no_console_suspend used, we should prevent the runtime suspend of the uart port which is getting used as an console. Cc: Santosh Shilimkarsantosh.shilim...@ti.com Cc: Felipe Balbiba...@ti.com Cc: Rajendra nayakrna...@ti.com Tested on omap5430evm, omap4430sdp. Signed-off-by: Sourav Poddar sourav.pod...@ti.com --- v2-v3 Based on Kevin Hilman and Santosh Shilimkar comments, modified serial core/driver layer to bypass runtime suspend for console uart while using no_console_suspend. This patch is based on Santosh Shilimkar serial patch[1] Rather than introducing this port_is_console thing, please move uart_console() into the serial_core.h header file, making it an inline function, and use that in omap-serial.c. Remember to fix drivers/tty/serial/mpc52xx_uart.c as well for that change. -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Sourav Poddar writes: > With dt boot, uart wakeup after suspend is non functional while using > "no_console_suspend" in the bootargs. With "no_console_suspend" used, we > should prevent the runtime suspend of the uart port which is getting used > as an console. > > Cc: Santosh Shilimkar > Cc: Felipe Balbi > Cc: Rajendra nayak > Tested on omap5430evm, omap4430sdp. > > Signed-off-by: Sourav Poddar Rather than make these special checks inside the driver's runtime PM callbacks, you should just disable runtime PM (pm_runtime_disable()) Then, this should be broken into 2 patches. 1) serial core: add the '->is_console' flag. (nit on naming: don't call it port_is_console, since the struct is already a uart_port) 2) In the OMAP UART driver's ->prepare callback, check the is_console flag and pm_runtime_disable() accordingly (then pm_runtime_enable() in the drivers's ->complete callback. Kevin -- 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: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Sourav Poddar sourav.pod...@ti.com writes: With dt boot, uart wakeup after suspend is non functional while using no_console_suspend in the bootargs. With no_console_suspend used, we should prevent the runtime suspend of the uart port which is getting used as an console. Cc: Santosh Shilimkarsantosh.shilim...@ti.com Cc: Felipe Balbiba...@ti.com Cc: Rajendra nayakrna...@ti.com Tested on omap5430evm, omap4430sdp. Signed-off-by: Sourav Poddar sourav.pod...@ti.com Rather than make these special checks inside the driver's runtime PM callbacks, you should just disable runtime PM (pm_runtime_disable()) Then, this should be broken into 2 patches. 1) serial core: add the '-is_console' flag. (nit on naming: don't call it port_is_console, since the struct is already a uart_port) 2) In the OMAP UART driver's -prepare callback, check the is_console flag and pm_runtime_disable() accordingly (then pm_runtime_enable() in the drivers's -complete callback. Kevin -- 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/
