| Message ID | 1484935719-20624-1-git-send-email-aford173@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* aford173@gmail.com <aford173@gmail.com> [170120 10:10]: > From: Adam Ford <aford173@gmail.com> > > This fixes commit ab8dd3aed011 ("ARM: DTS: Add minimal Support for > Logic PD DM3730 SOM-LV") where the system may not successfully > start after a reboot. Hmm so what goes wrong here? And which compatible do you end up using that works? Regards, Tony > Signed-off-by: Adam Ford <aford173@gmail.com> > > diff --git a/arch/arm/boot/dts/logicpd-som-lv.dtsi b/arch/arm/boot/dts/logicpd-som-lv.dtsi > index 26cce4d..fe19e7d 100644 > --- a/arch/arm/boot/dts/logicpd-som-lv.dtsi > +++ b/arch/arm/boot/dts/logicpd-som-lv.dtsi > @@ -261,7 +261,7 @@ > > &twl { > twl_power: power { > - compatible = "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle"; > + compatible = "ti,twl4030-power","ti,twl4030-power-reset", "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle"; > ti,use_poweroff; > }; > }; > -- > 2.7.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 23, 2017 at 12:58 PM, Tony Lindgren <tony@atomide.com> wrote: > * aford173@gmail.com <aford173@gmail.com> [170120 10:10]: >> From: Adam Ford <aford173@gmail.com> >> >> This fixes commit ab8dd3aed011 ("ARM: DTS: Add minimal Support for >> Logic PD DM3730 SOM-LV") where the system may not successfully >> start after a reboot. > > Hmm so what goes wrong here? And which compatible do you end up > using that works? The issue was the bootloader (MLO) would load, but it would throw an error unable to load anything (like U-boot) from the MMC. After reading the binding, the twl4030-power-reset seemed like a good option, so I added it. I didn't think the compatible options were mutually exclusive, because the original device tree you did the for the Torpedo had both "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle" enabled, so I added the "ti,twl4030-power-reset" option. Looking at binding, I thought I should have the more generic compatible = "ti,twl4030-power" added, but it doesn't appear to change the behavior, so I'm ok with removing that. Either way, it appears as if II need the "ti,twl4030-power-reset" to address the bootloader hanging. Does that mean I have to remove the other "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle" flags? adam > > Regards, > > Tony > >> Signed-off-by: Adam Ford <aford173@gmail.com> >> >> diff --git a/arch/arm/boot/dts/logicpd-som-lv.dtsi b/arch/arm/boot/dts/logicpd-som-lv.dtsi >> index 26cce4d..fe19e7d 100644 >> --- a/arch/arm/boot/dts/logicpd-som-lv.dtsi >> +++ b/arch/arm/boot/dts/logicpd-som-lv.dtsi >> @@ -261,7 +261,7 @@ >> >> &twl { >> twl_power: power { >> - compatible = "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle"; >> + compatible = "ti,twl4030-power","ti,twl4030-power-reset", "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle"; >> ti,use_poweroff; >> }; >> }; >> -- >> 2.7.4 >> -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Adam Ford <aford173@gmail.com> [170123 11:51]: > On Mon, Jan 23, 2017 at 12:58 PM, Tony Lindgren <tony@atomide.com> wrote: > > * aford173@gmail.com <aford173@gmail.com> [170120 10:10]: > >> From: Adam Ford <aford173@gmail.com> > >> > >> This fixes commit ab8dd3aed011 ("ARM: DTS: Add minimal Support for > >> Logic PD DM3730 SOM-LV") where the system may not successfully > >> start after a reboot. > > > > Hmm so what goes wrong here? And which compatible do you end up > > using that works? > > The issue was the bootloader (MLO) would load, but it would throw an > error unable to load anything (like U-boot) from the MMC. After > reading the binding, the twl4030-power-reset seemed like a good > option, so I added it. OK.. > I didn't think the compatible options were mutually exclusive, because > the original device tree you did the for the Torpedo had both > "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle" enabled, so I > added the "ti,twl4030-power-reset" option. > > Looking at binding, I thought I should have the more generic > compatible = "ti,twl4030-power" added, but it doesn't appear to change > the behavior, so I'm ok with removing that. > > Either way, it appears as if II need the "ti,twl4030-power-reset" to > address the bootloader hanging. Does that mean I have to remove the > other "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle" flags? Hmm I think we may have a bug somewhere. The "ti,twl4030-power-reset" should be a subset of "ti,twl4030-power-idle" which is a subset of "ti,twl4030-power-idle-osc-off". Maybe try adding some printks to the driver to see if the power-reset features are configured with power-idle-osc-off? From PM point of view, you really want to use all features with "ti,twl4030-power-idle-osc-off" if your hardware allows cutting off the oscillator during idle :) Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 23, 2017 at 3:01 PM, Tony Lindgren <tony@atomide.com> wrote: > * Adam Ford <aford173@gmail.com> [170123 11:51]: >> On Mon, Jan 23, 2017 at 12:58 PM, Tony Lindgren <tony@atomide.com> wrote: >> > * aford173@gmail.com <aford173@gmail.com> [170120 10:10]: >> >> From: Adam Ford <aford173@gmail.com> >> >> >> >> This fixes commit ab8dd3aed011 ("ARM: DTS: Add minimal Support for >> >> Logic PD DM3730 SOM-LV") where the system may not successfully >> >> start after a reboot. >> > >> > Hmm so what goes wrong here? And which compatible do you end up >> > using that works? >> >> The issue was the bootloader (MLO) would load, but it would throw an >> error unable to load anything (like U-boot) from the MMC. After >> reading the binding, the twl4030-power-reset seemed like a good >> option, so I added it. > > OK.. > >> I didn't think the compatible options were mutually exclusive, because >> the original device tree you did the for the Torpedo had both >> "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle" enabled, so I >> added the "ti,twl4030-power-reset" option. >> >> Looking at binding, I thought I should have the more generic >> compatible = "ti,twl4030-power" added, but it doesn't appear to change >> the behavior, so I'm ok with removing that. >> >> Either way, it appears as if II need the "ti,twl4030-power-reset" to >> address the bootloader hanging. Does that mean I have to remove the >> other "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle" flags? > > Hmm I think we may have a bug somewhere. The "ti,twl4030-power-reset" > should be a subset of "ti,twl4030-power-idle" which is a subset of > "ti,twl4030-power-idle-osc-off". > > Maybe try adding some printks to the driver to see if the power-reset > features are configured with power-idle-osc-off? I did some testing with the stable 4.9.y branch. I set Using only the power-idle-osc-off, I was able to observe the following: [ 2.425842] TWL4030_WAKEUP12_SCRIPT [ 2.434448] TWL4030_WAKEUP3_SCRIPT [ 2.453979] TWL4030_WRST_SCRIPT [ 2.460754] TWL4030_SLEEP_SCRIPT Reboot was not successful. The "ti,twl4030-power-reset" flag only showed [ 2.439392] TWL4030_WRST_SCRIPT Reboot was successful. and with just the "ti,twl4030-power-idle" flag, I received: [ 2.426177] TWL4030_WAKEUP12_SCRIPT [ 2.434783] TWL4030_WAKEUP3_SCRIPT [ 2.454315] TWL4030_WRST_SCRIPT [ 2.461090] TWL4030_SLEEP_SCRIPT Reboot was not successful. > > From PM point of view, you really want to use all features with > "ti,twl4030-power-idle-osc-off" if your hardware allows cutting off > the oscillator during idle :) I haven't observed any issues with "ti,twl4030-power-idle-osc-off" during normal operation or during sleep, but if the system cannot reboot, we can't use it. Do you have any suggestions? If not, I may have to switch the compatible flat to "ti,twl4030-power-reset" for compatibility reason. I have a couple customers inquiring with the same reboot issue, and this fixes their issue in both instances. > Regards, > > Tony adam -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Adam Ford <aford173@gmail.com> [170124 05:49]: > On Mon, Jan 23, 2017 at 3:01 PM, Tony Lindgren <tony@atomide.com> wrote: > > * Adam Ford <aford173@gmail.com> [170123 11:51]: > >> On Mon, Jan 23, 2017 at 12:58 PM, Tony Lindgren <tony@atomide.com> wrote: > >> > * aford173@gmail.com <aford173@gmail.com> [170120 10:10]: > >> >> From: Adam Ford <aford173@gmail.com> > >> >> > >> >> This fixes commit ab8dd3aed011 ("ARM: DTS: Add minimal Support for > >> >> Logic PD DM3730 SOM-LV") where the system may not successfully > >> >> start after a reboot. > >> > > >> > Hmm so what goes wrong here? And which compatible do you end up > >> > using that works? > >> > >> The issue was the bootloader (MLO) would load, but it would throw an > >> error unable to load anything (like U-boot) from the MMC. After > >> reading the binding, the twl4030-power-reset seemed like a good > >> option, so I added it. > > > > OK.. > > > >> I didn't think the compatible options were mutually exclusive, because > >> the original device tree you did the for the Torpedo had both > >> "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle" enabled, so I > >> added the "ti,twl4030-power-reset" option. > >> > >> Looking at binding, I thought I should have the more generic > >> compatible = "ti,twl4030-power" added, but it doesn't appear to change > >> the behavior, so I'm ok with removing that. > >> > >> Either way, it appears as if II need the "ti,twl4030-power-reset" to > >> address the bootloader hanging. Does that mean I have to remove the > >> other "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle" flags? > > > > Hmm I think we may have a bug somewhere. The "ti,twl4030-power-reset" > > should be a subset of "ti,twl4030-power-idle" which is a subset of > > "ti,twl4030-power-idle-osc-off". > > > > Maybe try adding some printks to the driver to see if the power-reset > > features are configured with power-idle-osc-off? > > I did some testing with the stable 4.9.y branch. I set > > > Using only the power-idle-osc-off, I was able to observe the following: > > [ 2.425842] TWL4030_WAKEUP12_SCRIPT > [ 2.434448] TWL4030_WAKEUP3_SCRIPT > [ 2.453979] TWL4030_WRST_SCRIPT > [ 2.460754] TWL4030_SLEEP_SCRIPT > > Reboot was not successful. > > The "ti,twl4030-power-reset" flag only showed > [ 2.439392] TWL4030_WRST_SCRIPT > > Reboot was successful. > > > and with just the "ti,twl4030-power-idle" flag, I received: > > [ 2.426177] TWL4030_WAKEUP12_SCRIPT > [ 2.434783] TWL4030_WAKEUP3_SCRIPT > [ 2.454315] TWL4030_WRST_SCRIPT > [ 2.461090] TWL4030_SLEEP_SCRIPT > > Reboot was not successful. OK so the TWL4030_WRST_SCRIPT is configured in all cases. So most likely it's one of the regulators that either can't idle or can't be shut off during idle. > > From PM point of view, you really want to use all features with > > "ti,twl4030-power-idle-osc-off" if your hardware allows cutting off > > the oscillator during idle :) > > I haven't observed any issues with "ti,twl4030-power-idle-osc-off" > during normal operation or during sleep, but if the system cannot > reboot, we can't use it. > > Do you have any suggestions? If not, I may have to switch the > compatible flat to "ti,twl4030-power-reset" for compatibility reason. > I have a couple customers inquiring with the same reboot issue, and > this fixes their issue in both instances. Yes comment out one line at a time in omap3_idle_rconfig and see which regulator needs to stay on for you for reboot to work. Also see if adding or leaving out "ti,system-power-controller" or "ti,use_poweroff" makes a difference. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 24, 2017 at 8:59 AM, Tony Lindgren <tony@atomide.com> wrote: > * Adam Ford <aford173@gmail.com> [170124 05:49]: >> On Mon, Jan 23, 2017 at 3:01 PM, Tony Lindgren <tony@atomide.com> wrote: >> > * Adam Ford <aford173@gmail.com> [170123 11:51]: >> >> On Mon, Jan 23, 2017 at 12:58 PM, Tony Lindgren <tony@atomide.com> wrote: >> >> > * aford173@gmail.com <aford173@gmail.com> [170120 10:10]: >> >> >> From: Adam Ford <aford173@gmail.com> >> >> >> >> >> >> This fixes commit ab8dd3aed011 ("ARM: DTS: Add minimal Support for >> >> >> Logic PD DM3730 SOM-LV") where the system may not successfully >> >> >> start after a reboot. >> >> > >> >> > Hmm so what goes wrong here? And which compatible do you end up >> >> > using that works? >> >> >> >> The issue was the bootloader (MLO) would load, but it would throw an >> >> error unable to load anything (like U-boot) from the MMC. After >> >> reading the binding, the twl4030-power-reset seemed like a good >> >> option, so I added it. >> > >> > OK.. >> > >> >> I didn't think the compatible options were mutually exclusive, because >> >> the original device tree you did the for the Torpedo had both >> >> "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle" enabled, so I >> >> added the "ti,twl4030-power-reset" option. >> >> >> >> Looking at binding, I thought I should have the more generic >> >> compatible = "ti,twl4030-power" added, but it doesn't appear to change >> >> the behavior, so I'm ok with removing that. >> >> >> >> Either way, it appears as if II need the "ti,twl4030-power-reset" to >> >> address the bootloader hanging. Does that mean I have to remove the >> >> other "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle" flags? >> > >> > Hmm I think we may have a bug somewhere. The "ti,twl4030-power-reset" >> > should be a subset of "ti,twl4030-power-idle" which is a subset of >> > "ti,twl4030-power-idle-osc-off". >> > >> > Maybe try adding some printks to the driver to see if the power-reset >> > features are configured with power-idle-osc-off? >> >> I did some testing with the stable 4.9.y branch. I set >> >> >> Using only the power-idle-osc-off, I was able to observe the following: >> >> [ 2.425842] TWL4030_WAKEUP12_SCRIPT >> [ 2.434448] TWL4030_WAKEUP3_SCRIPT >> [ 2.453979] TWL4030_WRST_SCRIPT >> [ 2.460754] TWL4030_SLEEP_SCRIPT >> >> Reboot was not successful. >> >> The "ti,twl4030-power-reset" flag only showed >> [ 2.439392] TWL4030_WRST_SCRIPT >> >> Reboot was successful. >> >> >> and with just the "ti,twl4030-power-idle" flag, I received: >> >> [ 2.426177] TWL4030_WAKEUP12_SCRIPT >> [ 2.434783] TWL4030_WAKEUP3_SCRIPT >> [ 2.454315] TWL4030_WRST_SCRIPT >> [ 2.461090] TWL4030_SLEEP_SCRIPT >> >> Reboot was not successful. > > OK so the TWL4030_WRST_SCRIPT is configured in all cases. So most > likely it's one of the regulators that either can't idle or can't be > shut off during idle. > >> > From PM point of view, you really want to use all features with >> > "ti,twl4030-power-idle-osc-off" if your hardware allows cutting off >> > the oscillator during idle :) >> >> I haven't observed any issues with "ti,twl4030-power-idle-osc-off" >> during normal operation or during sleep, but if the system cannot >> reboot, we can't use it. >> >> Do you have any suggestions? If not, I may have to switch the >> compatible flat to "ti,twl4030-power-reset" for compatibility reason. >> I have a couple customers inquiring with the same reboot issue, and >> this fixes their issue in both instances. > > Yes comment out one line at a time in omap3_idle_rconfig and see > which regulator needs to stay on for you for reboot to work. I appreciate you help on this. I commented out every line and the issue never resolved itself. If anything commenting out TWL_REMAP_OFF(RES_VPLL1, DEV_GRP_P1, 3, 1), made the problem worse, because MLO didn't even load. > Also see if adding or leaving out "ti,system-power-controller" or > "ti,use_poweroff" makes a difference. I tried removing that before I did anything else, and it didn't appear to make any difference. If you have any other ideas, I'm willing to try stuff. I'll continue to dig around in here. > Regards, > > Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Adam Ford <aford173@gmail.com> [170124 10:24]: > I appreciate you help on this. I commented out every line and the > issue never resolved itself. If anything commenting out > TWL_REMAP_OFF(RES_VPLL1, DEV_GRP_P1, 3, 1), made the > problem worse, because MLO didn't even load. Hmm. Maybe it's the order of the configurations? See the notes about bad order in load_twl4030_script(). Maybe try adding the reset configuration first? > > Also see if adding or leaving out "ti,system-power-controller" or > > "ti,use_poweroff" makes a difference. > > I tried removing that before I did anything else, and it didn't appear > to make any difference. If you have any other ideas, I'm willing to try stuff. > I'll continue to dig around in here. OK. I would try adding one configuration at a time and see if the order or something in the configuration affects it. Note that you may need to power cycle the device for some time to clear any state in twl4030 if something goes wrong. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 24, 2017 at 12:39 PM, Tony Lindgren <tony@atomide.com> wrote: > * Adam Ford <aford173@gmail.com> [170124 10:24]: >> I appreciate you help on this. I commented out every line and the >> issue never resolved itself. If anything commenting out >> TWL_REMAP_OFF(RES_VPLL1, DEV_GRP_P1, 3, 1), made the >> problem worse, because MLO didn't even load. > > Hmm. Maybe it's the order of the configurations? See the notes about > bad order in load_twl4030_script(). > > Maybe try adding the reset configuration first? > >> > Also see if adding or leaving out "ti,system-power-controller" or >> > "ti,use_poweroff" makes a difference. >> >> I tried removing that before I did anything else, and it didn't appear >> to make any difference. If you have any other ideas, I'm willing to try stuff. >> I'll continue to dig around in here. > > OK. I would try adding one configuration at a time and see if the > order or something in the configuration affects it. It appears as if the first 'compatible' flag is the only one accepted. Any subsequent ones are ignored. I played around with a few different options including reordering omap3_idle_scripts so omap3_wrst_script or first and neither did the trick. I added a bunch of debug code to see if we were getting any errors. (sidenote: the error handing in twl4030-power.c is very inconsistent. in some places we just exit on error with no message, and other places we display an error and other places we display what failed. I am not complaining because I didn't hit any of the error traps, but this may be hard for others to troubleshoot) I know this doesn't say much, but the one line of code that made the system work was to disable: /* Set ACTIVE to SLEEP SEQ address in T2 memory*/ err = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, address, R_SEQ_ADD_A2S); I am not an expert on the twl4030, but I am guessing that enables sleep. Can you tell me what's different about the reboot sequence between the the different power modes? It seems to operate just fine in the different power modes and the power savings is very nice. From what I can tell, we only have one reset sequence, yet with the low power modes, it fails to access the MMC1. Is it possible that something in the PBIAS registers do or do not reset correctly? > Note that you may need to power cycle the device for some time to > clear any state in twl4030 if something goes wrong. Of course. I have been doing that with the backup battery disconnected. > Regards, > > Tony adam -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> /* Set ACTIVE to SLEEP SEQ address in T2 memory*/ > err = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, address, > R_SEQ_ADD_A2S); > > I am not an expert on the twl4030, but I am guessing that enables > sleep. This PMIC at COLD reset uses internal OTP mask rom defaults for any sequencing needs. After power up SW can programming in new sequences into PMIC internal SRAM. This flexibility is needed as the PMIC can control several different optional board level hook ups. To really program the PMIC correctly you have to know something about the boards requirements. The above is telling the PMIC the address of the sequence to execute for an omap3 soc-level sleep event. The sequence is made up of internal write and delay commands. If someone cares to understand, the PMIC memory can be dumped and the sequences examined. But as I said some of these sequences 'should' have a board-to-pmic hook up dependency. Not all sequences will make sense for every board design. Generally to get OFF modes done correctly and optimized this level of detail needs to be handled. Using trial and error maybe you can arrive with something which doesn't crash... but it might not really be doing the right thing. Regards, Richard W.
diff --git a/arch/arm/boot/dts/logicpd-som-lv.dtsi b/arch/arm/boot/dts/logicpd-som-lv.dtsi index 26cce4d..fe19e7d 100644 --- a/arch/arm/boot/dts/logicpd-som-lv.dtsi +++ b/arch/arm/boot/dts/logicpd-som-lv.dtsi @@ -261,7 +261,7 @@ &twl { twl_power: power { - compatible = "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle"; + compatible = "ti,twl4030-power","ti,twl4030-power-reset", "ti,twl4030-power-idle-osc-off", "ti,twl4030-power-idle"; ti,use_poweroff; }; };