Message ID | 1364901613-25080-10-git-send-email-josephl@nvidia.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 04/02/2013 05:20 AM, Joseph Lo wrote: > Adding the PM configuration of PMC when the platform support suspend > function. For this patch, some more boards could be supported, but I think we'll have to rely on other people (Lucas, Thierry) to send patches for them, since they know the details of the board. It's fine to leave suspend disabled on those boards until later. Aside from the 1 other comment I made on this series, I think it's good to go once your previous 3-long series is fixed and reposted per my comments on it.
On Tue, Apr 02, 2013 at 02:00:55PM -0600, Stephen Warren wrote: > On 04/02/2013 05:20 AM, Joseph Lo wrote: > > Adding the PM configuration of PMC when the platform support suspend > > function. > > For this patch, some more boards could be supported, but I think we'll > have to rely on other people (Lucas, Thierry) to send patches for them, > since they know the details of the board. It's fine to leave suspend > disabled on those boards until later. I'm not sure what would be the best way to get the Tamonten devices out of suspend again. For Plutux and TEC we have a power button that could possibly be reused. There's a WAKEUP pin on the Tamonten connector which goes to GPIO PV2, but the mechanism to trigger it is carrier-board dependent. For the Medcom-Wide we don't have a power button, but it could possibly use the WAKEUP pin if that's connected. It seems like in all designs it goes to the CPLD, but I need to check with engineering if there's a way to manipulate it. So yes, I think for now we can leave suspend disabled on Tamonten and I can send patches to enable it once I've clarified with engineering. Thierry
On Wed, 2013-04-03 at 13:59 +0800, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Tue, Apr 02, 2013 at 02:00:55PM -0600, Stephen Warren wrote: > > On 04/02/2013 05:20 AM, Joseph Lo wrote: > > > Adding the PM configuration of PMC when the platform support suspend > > > function. > > > > For this patch, some more boards could be supported, but I think we'll > > have to rely on other people (Lucas, Thierry) to send patches for them, > > since they know the details of the board. It's fine to leave suspend > > disabled on those boards until later. > > I'm not sure what would be the best way to get the Tamonten devices out > of suspend again. For Plutux and TEC we have a power button that could > possibly be reused. There's a WAKEUP pin on the Tamonten connector which > goes to GPIO PV2, but the mechanism to trigger it is carrier-board > dependent. > > For the Medcom-Wide we don't have a power button, but it could possibly > use the WAKEUP pin if that's connected. It seems like in all designs it > goes to the CPLD, but I need to check with engineering if there's a way > to manipulate it. > > So yes, I think for now we can leave suspend disabled on Tamonten and I > can send patches to enable it once I've clarified with engineering. > Hi Thierry, I just recall I still need at least "nvidia,cpu-pwr-good-time" and "nvidia,cpu-pwr-off-time" two properties to make CPU idle power-down mode (same with LP2) work normally after this series. So can you provide me that or point me out where is your downstream kernel source? There data would be come from DT after this series. Thanks, Joseph
On Wed, Apr 03, 2013 at 02:34:25PM +0800, Joseph Lo wrote: > On Wed, 2013-04-03 at 13:59 +0800, Thierry Reding wrote: > > * PGP Signed by an unknown key > > > > On Tue, Apr 02, 2013 at 02:00:55PM -0600, Stephen Warren wrote: > > > On 04/02/2013 05:20 AM, Joseph Lo wrote: > > > > Adding the PM configuration of PMC when the platform support suspend > > > > function. > > > > > > For this patch, some more boards could be supported, but I think we'll > > > have to rely on other people (Lucas, Thierry) to send patches for them, > > > since they know the details of the board. It's fine to leave suspend > > > disabled on those boards until later. > > > > I'm not sure what would be the best way to get the Tamonten devices out > > of suspend again. For Plutux and TEC we have a power button that could > > possibly be reused. There's a WAKEUP pin on the Tamonten connector which > > goes to GPIO PV2, but the mechanism to trigger it is carrier-board > > dependent. > > > > For the Medcom-Wide we don't have a power button, but it could possibly > > use the WAKEUP pin if that's connected. It seems like in all designs it > > goes to the CPLD, but I need to check with engineering if there's a way > > to manipulate it. > > > > So yes, I think for now we can leave suspend disabled on Tamonten and I > > can send patches to enable it once I've clarified with engineering. > > > Hi Thierry, > > I just recall I still need at least "nvidia,cpu-pwr-good-time" and > "nvidia,cpu-pwr-off-time" two properties to make CPU idle power-down > mode (same with LP2) work normally after this series. > > So can you provide me that or point me out where is your downstream > kernel source? There data would be come from DT after this series. We don't have suspend support in downstream kernels either. However since the Tamonten was derived from the Harmony reference design I suppose the same values can be reused. That said, the binding document says that those values are only required if nvidia,suspend-mode is set. So if we don't add it for the Tamonten boards why would you need the cpu-pwr-good-time and cpu-pwr-off-time values. Or maybe the binding document is wrong? Thierry
On Wed, 2013-04-03 at 15:04 +0800, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Wed, Apr 03, 2013 at 02:34:25PM +0800, Joseph Lo wrote: > > On Wed, 2013-04-03 at 13:59 +0800, Thierry Reding wrote: > > > > Old Signed by an unknown key > > > > > > On Tue, Apr 02, 2013 at 02:00:55PM -0600, Stephen Warren wrote: > > > > On 04/02/2013 05:20 AM, Joseph Lo wrote: > > > > > Adding the PM configuration of PMC when the platform support suspend > > > > > function. > > > > > > > > For this patch, some more boards could be supported, but I think we'll > > > > have to rely on other people (Lucas, Thierry) to send patches for them, > > > > since they know the details of the board. It's fine to leave suspend > > > > disabled on those boards until later. > > > > > > I'm not sure what would be the best way to get the Tamonten devices out > > > of suspend again. For Plutux and TEC we have a power button that could > > > possibly be reused. There's a WAKEUP pin on the Tamonten connector which > > > goes to GPIO PV2, but the mechanism to trigger it is carrier-board > > > dependent. > > > > > > For the Medcom-Wide we don't have a power button, but it could possibly > > > use the WAKEUP pin if that's connected. It seems like in all designs it > > > goes to the CPLD, but I need to check with engineering if there's a way > > > to manipulate it. > > > > > > So yes, I think for now we can leave suspend disabled on Tamonten and I > > > can send patches to enable it once I've clarified with engineering. > > > > > Hi Thierry, > > > > I just recall I still need at least "nvidia,cpu-pwr-good-time" and > > "nvidia,cpu-pwr-off-time" two properties to make CPU idle power-down > > mode (same with LP2) work normally after this series. > > > > So can you provide me that or point me out where is your downstream > > kernel source? There data would be come from DT after this series. > > We don't have suspend support in downstream kernels either. However > since the Tamonten was derived from the Harmony reference design I > suppose the same values can be reused. > OK. That's enough. > That said, the binding document says that those values are only required > if nvidia,suspend-mode is set. So if we don't add it for the Tamonten > boards why would you need the cpu-pwr-good-time and cpu-pwr-off-time > values. Or maybe the binding document is wrong? Actually, when you support CPU idle power-down mode (LP2), you already support suspend function (at least LP2 mode). Just no wake up source define in DT for your boards. May I add these properties to your boards (same as Harmony)? Then you can figure out your wake up source later, at least the RTC can wake up your device I believe. Thanks, Joseph
On Wed, Apr 03, 2013 at 03:11:17PM +0800, Joseph Lo wrote: > On Wed, 2013-04-03 at 15:04 +0800, Thierry Reding wrote: > > * PGP Signed by an unknown key > > > > On Wed, Apr 03, 2013 at 02:34:25PM +0800, Joseph Lo wrote: > > > On Wed, 2013-04-03 at 13:59 +0800, Thierry Reding wrote: > > > > > Old Signed by an unknown key > > > > > > > > On Tue, Apr 02, 2013 at 02:00:55PM -0600, Stephen Warren wrote: > > > > > On 04/02/2013 05:20 AM, Joseph Lo wrote: > > > > > > Adding the PM configuration of PMC when the platform support suspend > > > > > > function. > > > > > > > > > > For this patch, some more boards could be supported, but I think we'll > > > > > have to rely on other people (Lucas, Thierry) to send patches for them, > > > > > since they know the details of the board. It's fine to leave suspend > > > > > disabled on those boards until later. > > > > > > > > I'm not sure what would be the best way to get the Tamonten devices out > > > > of suspend again. For Plutux and TEC we have a power button that could > > > > possibly be reused. There's a WAKEUP pin on the Tamonten connector which > > > > goes to GPIO PV2, but the mechanism to trigger it is carrier-board > > > > dependent. > > > > > > > > For the Medcom-Wide we don't have a power button, but it could possibly > > > > use the WAKEUP pin if that's connected. It seems like in all designs it > > > > goes to the CPLD, but I need to check with engineering if there's a way > > > > to manipulate it. > > > > > > > > So yes, I think for now we can leave suspend disabled on Tamonten and I > > > > can send patches to enable it once I've clarified with engineering. > > > > > > > Hi Thierry, > > > > > > I just recall I still need at least "nvidia,cpu-pwr-good-time" and > > > "nvidia,cpu-pwr-off-time" two properties to make CPU idle power-down > > > mode (same with LP2) work normally after this series. > > > > > > So can you provide me that or point me out where is your downstream > > > kernel source? There data would be come from DT after this series. > > > > We don't have suspend support in downstream kernels either. However > > since the Tamonten was derived from the Harmony reference design I > > suppose the same values can be reused. > > > OK. That's enough. > > > That said, the binding document says that those values are only required > > if nvidia,suspend-mode is set. So if we don't add it for the Tamonten > > boards why would you need the cpu-pwr-good-time and cpu-pwr-off-time > > values. Or maybe the binding document is wrong? > > Actually, when you support CPU idle power-down mode (LP2), you already > support suspend function (at least LP2 mode). Just no wake up source > define in DT for your boards. Okay, but for CPU idle LP2 we don't need a wakeup source, right? I only need the wakeup source if I explicitly suspend the device. > May I add these properties to your boards (same as Harmony)? Then you > can figure out your wake up source later, at least the RTC can wake up > your device I believe. From what I understand there should be no changes in behaviour after this patch, so it should be fine. Thierry
On Wed, 2013-04-03 at 15:19 +0800, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Wed, Apr 03, 2013 at 03:11:17PM +0800, Joseph Lo wrote: > > On Wed, 2013-04-03 at 15:04 +0800, Thierry Reding wrote: > > > > Old Signed by an unknown key > > > > > > On Wed, Apr 03, 2013 at 02:34:25PM +0800, Joseph Lo wrote: > > > > On Wed, 2013-04-03 at 13:59 +0800, Thierry Reding wrote: > > > > > > Old Signed by an unknown key > > > > > > > > > > On Tue, Apr 02, 2013 at 02:00:55PM -0600, Stephen Warren wrote: > > > > > > On 04/02/2013 05:20 AM, Joseph Lo wrote: > > > > > > > Adding the PM configuration of PMC when the platform support suspend > > > > > > > function. > > > > > > > > > > > > For this patch, some more boards could be supported, but I think we'll > > > > > > have to rely on other people (Lucas, Thierry) to send patches for them, > > > > > > since they know the details of the board. It's fine to leave suspend > > > > > > disabled on those boards until later. > > > > > > > > > > I'm not sure what would be the best way to get the Tamonten devices out > > > > > of suspend again. For Plutux and TEC we have a power button that could > > > > > possibly be reused. There's a WAKEUP pin on the Tamonten connector which > > > > > goes to GPIO PV2, but the mechanism to trigger it is carrier-board > > > > > dependent. > > > > > > > > > > For the Medcom-Wide we don't have a power button, but it could possibly > > > > > use the WAKEUP pin if that's connected. It seems like in all designs it > > > > > goes to the CPLD, but I need to check with engineering if there's a way > > > > > to manipulate it. > > > > > > > > > > So yes, I think for now we can leave suspend disabled on Tamonten and I > > > > > can send patches to enable it once I've clarified with engineering. > > > > > > > > > Hi Thierry, > > > > > > > > I just recall I still need at least "nvidia,cpu-pwr-good-time" and > > > > "nvidia,cpu-pwr-off-time" two properties to make CPU idle power-down > > > > mode (same with LP2) work normally after this series. > > > > > > > > So can you provide me that or point me out where is your downstream > > > > kernel source? There data would be come from DT after this series. > > > > > > We don't have suspend support in downstream kernels either. However > > > since the Tamonten was derived from the Harmony reference design I > > > suppose the same values can be reused. > > > > > OK. That's enough. > > > > > That said, the binding document says that those values are only required > > > if nvidia,suspend-mode is set. So if we don't add it for the Tamonten > > > boards why would you need the cpu-pwr-good-time and cpu-pwr-off-time > > > values. Or maybe the binding document is wrong? > > > > Actually, when you support CPU idle power-down mode (LP2), you already > > support suspend function (at least LP2 mode). Just no wake up source > > define in DT for your boards. > > Okay, but for CPU idle LP2 we don't need a wakeup source, right? I only > need the wakeup source if I explicitly suspend the device. > Yes. The difference between CPU idle and suspend LP2 is wake up from any interrupt or specific interrupt source (wake up source for suspend). My approach is adding CPU idle LP2 support first and make it support suspend later. Then reorganize them into PM related code and get the configurations from DT. > > May I add these properties to your boards (same as Harmony)? Then you > > can figure out your wake up source later, at least the RTC can wake up > > your device I believe. > > From what I understand there should be no changes in behaviour after > this patch, so it should be fine. > OK, thanks.
diff --git a/arch/arm/boot/dts/tegra20-harmony.dts b/arch/arm/boot/dts/tegra20-harmony.dts index 4f08521..7904eb4 100644 --- a/arch/arm/boot/dts/tegra20-harmony.dts +++ b/arch/arm/boot/dts/tegra20-harmony.dts @@ -429,6 +429,12 @@ pmc { nvidia,invert-interrupt; + nvidia,suspend-mode = <2>; + nvidia,cpu-pwr-good-time = <5000>; + nvidia,cpu-pwr-off-time = <5000>; + nvidia,core-pwr-good-time = <3845 3845>; + nvidia,core-pwr-off-time = <3875>; + nvidia,sys-clock-req-active-high; }; usb@c5000000 { diff --git a/arch/arm/boot/dts/tegra20-paz00.dts b/arch/arm/boot/dts/tegra20-paz00.dts index c55e062..680eeeb 100644 --- a/arch/arm/boot/dts/tegra20-paz00.dts +++ b/arch/arm/boot/dts/tegra20-paz00.dts @@ -428,6 +428,12 @@ pmc { nvidia,invert-interrupt; + nvidia,suspend-mode = <2>; + nvidia,cpu-pwr-good-time = <2000>; + nvidia,cpu-pwr-off-time = <0>; + nvidia,core-pwr-good-time = <3845 3845>; + nvidia,core-pwr-off-time = <0>; + nvidia,sys-clock-req-active-high; }; usb@c5000000 { diff --git a/arch/arm/boot/dts/tegra20-seaboard.dts b/arch/arm/boot/dts/tegra20-seaboard.dts index 5572ee7..2ff92c9 100644 --- a/arch/arm/boot/dts/tegra20-seaboard.dts +++ b/arch/arm/boot/dts/tegra20-seaboard.dts @@ -530,6 +530,12 @@ pmc { nvidia,invert-interrupt; + nvidia,suspend-mode = <2>; + nvidia,cpu-pwr-good-time = <5000>; + nvidia,cpu-pwr-off-time = <5000>; + nvidia,core-pwr-good-time = <3845 3845>; + nvidia,core-pwr-off-time = <3875>; + nvidia,sys-clock-req-active-high; }; memory-controller@7000f400 { diff --git a/arch/arm/boot/dts/tegra20-trimslice.dts b/arch/arm/boot/dts/tegra20-trimslice.dts index 8e8d4f6..7760b67 100644 --- a/arch/arm/boot/dts/tegra20-trimslice.dts +++ b/arch/arm/boot/dts/tegra20-trimslice.dts @@ -313,6 +313,15 @@ }; }; + pmc { + nvidia,suspend-mode = <2>; + nvidia,cpu-pwr-good-time = <5000>; + nvidia,cpu-pwr-off-time = <5000>; + nvidia,core-pwr-good-time = <3845 3845>; + nvidia,core-pwr-off-time = <3875>; + nvidia,sys-clock-req-active-high; + }; + usb@c5000000 { status = "okay"; nvidia,vbus-gpio = <&gpio 170 0>; /* gpio PV2 */ diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts index 4363764..98a1eae 100644 --- a/arch/arm/boot/dts/tegra20-ventana.dts +++ b/arch/arm/boot/dts/tegra20-ventana.dts @@ -506,6 +506,12 @@ pmc { nvidia,invert-interrupt; + nvidia,suspend-mode = <2>; + nvidia,cpu-pwr-good-time = <2000>; + nvidia,cpu-pwr-off-time = <100>; + nvidia,core-pwr-good-time = <3845 3845>; + nvidia,core-pwr-off-time = <458>; + nvidia,sys-clock-req-active-high; }; usb@c5000000 { diff --git a/arch/arm/boot/dts/tegra20-whistler.dts b/arch/arm/boot/dts/tegra20-whistler.dts index 878790e..ff74be7 100644 --- a/arch/arm/boot/dts/tegra20-whistler.dts +++ b/arch/arm/boot/dts/tegra20-whistler.dts @@ -509,6 +509,14 @@ pmc { nvidia,invert-interrupt; + nvidia,suspend-mode = <2>; + nvidia,cpu-pwr-good-time = <2000>; + nvidia,cpu-pwr-off-time = <1000>; + nvidia,core-pwr-good-time = <0 3845>; + nvidia,core-pwr-off-time = <93727>; + nvidia,core-power-req-active-high; + nvidia,sys-clock-req-active-high; + nvidia,combined-power-req; }; usb@c5000000 { diff --git a/arch/arm/boot/dts/tegra30-beaver.dts b/arch/arm/boot/dts/tegra30-beaver.dts index 772e6cf..f17deb0 100644 --- a/arch/arm/boot/dts/tegra30-beaver.dts +++ b/arch/arm/boot/dts/tegra30-beaver.dts @@ -266,6 +266,13 @@ pmc { status = "okay"; nvidia,invert-interrupt; + nvidia,suspend-mode = <2>; + nvidia,cpu-pwr-good-time = <2000>; + nvidia,cpu-pwr-off-time = <200>; + nvidia,core-pwr-good-time = <3845 3845>; + nvidia,core-pwr-off-time = <0>; + nvidia,core-power-req-active-high; + nvidia,sys-clock-req-active-high; }; sdhci@78000000 { diff --git a/arch/arm/boot/dts/tegra30-cardhu.dtsi b/arch/arm/boot/dts/tegra30-cardhu.dtsi index 053913f..9171834 100644 --- a/arch/arm/boot/dts/tegra30-cardhu.dtsi +++ b/arch/arm/boot/dts/tegra30-cardhu.dtsi @@ -320,6 +320,13 @@ pmc { status = "okay"; nvidia,invert-interrupt; + nvidia,suspend-mode = <2>; + nvidia,cpu-pwr-good-time = <2000>; + nvidia,cpu-pwr-off-time = <200>; + nvidia,core-pwr-good-time = <3845 3845>; + nvidia,core-pwr-off-time = <0>; + nvidia,core-power-req-active-high; + nvidia,sys-clock-req-active-high; }; sdhci@78000000 {
Adding the PM configuration of PMC when the platform support suspend function. Signed-off-by: Joseph Lo <josephl@nvidia.com> --- V5: * no change V4: * no change V3: * fix the configuration for paz00 board V2: * no change --- arch/arm/boot/dts/tegra20-harmony.dts | 6 ++++++ arch/arm/boot/dts/tegra20-paz00.dts | 6 ++++++ arch/arm/boot/dts/tegra20-seaboard.dts | 6 ++++++ arch/arm/boot/dts/tegra20-trimslice.dts | 9 +++++++++ arch/arm/boot/dts/tegra20-ventana.dts | 6 ++++++ arch/arm/boot/dts/tegra20-whistler.dts | 8 ++++++++ arch/arm/boot/dts/tegra30-beaver.dts | 7 +++++++ arch/arm/boot/dts/tegra30-cardhu.dtsi | 7 +++++++ 8 files changed, 55 insertions(+)