diff mbox

N900 sleep mode (in 4.5-rc0, if that matters)

Message ID 20160130221509.GA28644@amd (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Machek Jan. 30, 2016, 10:15 p.m. UTC
Hi!

> > > > ffdffe8d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200072
> > > > 0000000d 48004a28 (fa004a28) cm_idlest3_core
> > > > 
> > > > cm_idlest1_core changes periodicall often, to 00218072. The rest seems
> > > > constant.
> > > 
> > > For cm_idlest1_core 42 is the answer.. Here you have bits 4 and 5
> > > blocking which is for OTG and it's PHY. That's a known issue with
> > > musb and setting pm_runtime_irq_safe() on the MUSB parent.
> > > 
> > > If you do rmmod omap2430 and phy-twl4030usb chances are the LEDs will
> > > start going off assuming the McSPI bit goes low with WLAN idling.
> > 
> > Ok, so I tried to compile kernel without omap2430/phy-twl4030usb
> > . That did not help. So I thought, ok, maybe rmmod is needed to
> > trigger some powersaving? But that is not exactly easy to do:
> > 
> > pavel@n900:/my/tui/ofone$ sudo insmod /my/modules/omap2430.ko
> > pavel@n900:/my/tui/ofone$ sudo insmod /my/modules/phy-twl4030-usb.ko
> > pavel@n900:/my/tui/ofone$ sudo rmmod phy-twl4030-usb.ko
> > Error: Module phy_twl4030_usb is in use
> > pavel@n900:/my/tui/ofone$
> > 
> > Any ideas what jumps to use the modules? Charger code?
> 
> I tried a kernel without charger code, and no luck, rmmod fails the
> same way. dmesg says:
> [  111.093078] wlan0: authenticated
> [  111.097442] wlan0: associate with 06:27:22:f9:10:6a (try 1/3)
> [  111.104553] wlan0: RX AssocResp from 06:27:22:f9:10:6a (capab=0x421
> status=0 aid=2)
> [  111.104705] wlan0: AP has invalid WMM params (AIFSN=1 for ACI 2),
> will use 2
> [  111.104736] wlan0: AP has invalid WMM params (AIFSN=1 for ACI 3),
> will use 2
> [  111.256652] wlan0: associated
> [  184.681427] HS USB OTG: no transceiver configured
> [  184.681488] musb-hdrc musb-hdrc.0.auto: musb_init_controller failed
> with status -517
> [  184.681976] HS USB OTG: no transceiver configured
> [  184.682006] musb-hdrc musb-hdrc.0.auto: musb_init_controller failed
> with status -517
> [  187.690338] twl4030_usb 48070000.i2c:twl@48:twl4030-usb:
> Initialized TWL4030 USB module
> [  187.698303] musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk
> combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn)
> [  187.698333] musb-hdrc: MHDRC RTL version 1.400
> [  187.698333] musb-hdrc: setup fifo_mode 4
> [  187.698394] musb-hdrc: 28/31 max ep, 16384/16384 memory
> pavel@n900:/my/tui/ofone$

I added following hack to phy-twl4030-usb.c so that I could avoid
modules and module unloading problem. But still could not get it to
sleep :-(.

									Pavel

Comments

Tony Lindgren Feb. 1, 2016, 6:13 p.m. UTC | #1
* Pavel Machek <pavel@ucw.cz> [160130 14:16]:
> Hi!
> 
> > > > > ffdffe8d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200072
> > > > > 0000000d 48004a28 (fa004a28) cm_idlest3_core
> > > > > 
> > > > > cm_idlest1_core changes periodicall often, to 00218072. The rest seems
> > > > > constant.
> > > > 
> > > > For cm_idlest1_core 42 is the answer.. Here you have bits 4 and 5
> > > > blocking which is for OTG and it's PHY. That's a known issue with
> > > > musb and setting pm_runtime_irq_safe() on the MUSB parent.
> > > > 
> > > > If you do rmmod omap2430 and phy-twl4030usb chances are the LEDs will
> > > > start going off assuming the McSPI bit goes low with WLAN idling.
> > > 
> > > Ok, so I tried to compile kernel without omap2430/phy-twl4030usb
> > > . That did not help. So I thought, ok, maybe rmmod is needed to
> > > trigger some powersaving? But that is not exactly easy to do:
> > > 
> > > pavel@n900:/my/tui/ofone$ sudo insmod /my/modules/omap2430.ko
> > > pavel@n900:/my/tui/ofone$ sudo insmod /my/modules/phy-twl4030-usb.ko
> > > pavel@n900:/my/tui/ofone$ sudo rmmod phy-twl4030-usb.ko
> > > Error: Module phy_twl4030_usb is in use
> > > pavel@n900:/my/tui/ofone$
> > > 
> > > Any ideas what jumps to use the modules? Charger code?
> > 
> > I tried a kernel without charger code, and no luck, rmmod fails the
> > same way. dmesg says:
> > [  111.093078] wlan0: authenticated
> > [  111.097442] wlan0: associate with 06:27:22:f9:10:6a (try 1/3)
> > [  111.104553] wlan0: RX AssocResp from 06:27:22:f9:10:6a (capab=0x421
> > status=0 aid=2)
> > [  111.104705] wlan0: AP has invalid WMM params (AIFSN=1 for ACI 2),
> > will use 2
> > [  111.104736] wlan0: AP has invalid WMM params (AIFSN=1 for ACI 3),
> > will use 2
> > [  111.256652] wlan0: associated
> > [  184.681427] HS USB OTG: no transceiver configured
> > [  184.681488] musb-hdrc musb-hdrc.0.auto: musb_init_controller failed
> > with status -517
> > [  184.681976] HS USB OTG: no transceiver configured
> > [  184.682006] musb-hdrc musb-hdrc.0.auto: musb_init_controller failed
> > with status -517
> > [  187.690338] twl4030_usb 48070000.i2c:twl@48:twl4030-usb:
> > Initialized TWL4030 USB module
> > [  187.698303] musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk
> > combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn)
> > [  187.698333] musb-hdrc: MHDRC RTL version 1.400
> > [  187.698333] musb-hdrc: setup fifo_mode 4
> > [  187.698394] musb-hdrc: 28/31 max ep, 16384/16384 memory
> > pavel@n900:/my/tui/ofone$
> 
> I added following hack to phy-twl4030-usb.c so that I could avoid
> modules and module unloading problem. But still could not get it to
> sleep :-(.

Are you sure you're using v4.5-rc? I recently patched away some
issues where rmmod of the musb related modules did not work.

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
Pavel Machek Feb. 1, 2016, 9:17 p.m. UTC | #2
Hi!

> > > > Any ideas what jumps to use the modules? Charger code?
> > > 
> > > I tried a kernel without charger code, and no luck, rmmod fails the
> > > same way. dmesg says:
> > > [  111.093078] wlan0: authenticated
> > > [  111.097442] wlan0: associate with 06:27:22:f9:10:6a (try 1/3)
> > > [  111.104553] wlan0: RX AssocResp from 06:27:22:f9:10:6a (capab=0x421
> > > status=0 aid=2)
> > > [  111.104705] wlan0: AP has invalid WMM params (AIFSN=1 for ACI 2),
> > > will use 2
> > > [  111.104736] wlan0: AP has invalid WMM params (AIFSN=1 for ACI 3),
> > > will use 2
> > > [  111.256652] wlan0: associated
> > > [  184.681427] HS USB OTG: no transceiver configured
> > > [  184.681488] musb-hdrc musb-hdrc.0.auto: musb_init_controller failed
> > > with status -517
> > > [  184.681976] HS USB OTG: no transceiver configured
> > > [  184.682006] musb-hdrc musb-hdrc.0.auto: musb_init_controller failed
> > > with status -517
> > > [  187.690338] twl4030_usb 48070000.i2c:twl@48:twl4030-usb:
> > > Initialized TWL4030 USB module
> > > [  187.698303] musb-hdrc: ConfigData=0xde (UTMI-8, dyn FIFOs, bulk
> > > combine, bulk split, HB-ISO Rx, HB-ISO Tx, SoftConn)
> > > [  187.698333] musb-hdrc: MHDRC RTL version 1.400
> > > [  187.698333] musb-hdrc: setup fifo_mode 4
> > > [  187.698394] musb-hdrc: 28/31 max ep, 16384/16384 memory
> > > pavel@n900:/my/tui/ofone$
> > 
> > I added following hack to phy-twl4030-usb.c so that I could avoid
> > modules and module unloading problem. But still could not get it to
> > sleep :-(.
> 
> Are you sure you're using v4.5-rc? I recently patched away some
> issues where rmmod of the musb related modules did not work.

No, sorry, that was with 4.4. As you hit "PM regression with commit
5de85b9d57ab PM runtime re-init in v4.5-rc1", I thought I'd avoid
v4.5-rc. 

(I assume I have to insmod and rmmod, right? Because powersave is not
entered if I simply compile-out usb).

Would you have commit ids for those rmmod fixes? It might be good to
push them into stable, and I should try again with them applied...

Thanks and best regards,
								Pavel
Tony Lindgren Feb. 1, 2016, 10:11 p.m. UTC | #3
* Pavel Machek <pavel@ucw.cz> [160201 13:18]:
> 
> No, sorry, that was with 4.4. As you hit "PM regression with commit
> 5de85b9d57ab PM runtime re-init in v4.5-rc1", I thought I'd avoid
> v4.5-rc. 

OK

> (I assume I have to insmod and rmmod, right? Because powersave is not
> entered if I simply compile-out usb).

Depending on what the bootloader does and probably also if
USB was used during the booting.. So yeah you may need to modprobe
and then rmmod.

> Would you have commit ids for those rmmod fixes? It might be good to
> push them into stable, and I should try again with them applied...

055555fc459 ("usb: musb: core: Fix handling of the phy notifications")
03e43528ab68 ("usb: musb: Fix unbalanced pm_runtime_enable")

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
Tony Lindgren Feb. 4, 2016, 5:35 a.m. UTC | #4
* Tony Lindgren <tony@atomide.com> [160201 14:12]:
> * Pavel Machek <pavel@ucw.cz> [160201 13:18]:
> > 
> > No, sorry, that was with 4.4. As you hit "PM regression with commit
> > 5de85b9d57ab PM runtime re-init in v4.5-rc1", I thought I'd avoid
> > v4.5-rc. 
> 
> OK
> 
> > (I assume I have to insmod and rmmod, right? Because powersave is not
> > entered if I simply compile-out usb).
> 
> Depending on what the bootloader does and probably also if
> USB was used during the booting.. So yeah you may need to modprobe
> and then rmmod.
> 
> > Would you have commit ids for those rmmod fixes? It might be good to
> > push them into stable, and I should try again with them applied...
> 
> 055555fc459 ("usb: musb: core: Fix handling of the phy notifications")
> 03e43528ab68 ("usb: musb: Fix unbalanced pm_runtime_enable")

Oh and looks like these two phy-twl4030-phy fixes never got
merged from thread "[PATCH 0/2] Two phy-twl4030-usb fixes for
unloading the module":

phy: twl4030-usb: Relase usb phy on unload
phy: twl4030-usb: Fix unbalanced pm_runtime_enable on module reload

I'll resend those. But without those again deeper idle states
are blocked.. And it could be that n900 needs similar additional
patches for it's USB PHY as I've tested things so far only with
phy-twl4030-usb PHY based systems.

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
Pavel Machek Feb. 7, 2016, 9:23 p.m. UTC | #5
Hi!

> > (I assume I have to insmod and rmmod, right? Because powersave is not
> > entered if I simply compile-out usb).
> 
> Depending on what the bootloader does and probably also if
> USB was used during the booting.. So yeah you may need to modprobe
> and then rmmod.
> 
> > Would you have commit ids for those rmmod fixes? It might be good to
> > push them into stable, and I should try again with them applied...
> 
> 055555fc459 ("usb: musb: core: Fix handling of the phy notifications")
> 03e43528ab68 ("usb: musb: Fix unbalanced pm_runtime_enable")

Ok, with that, I can insmod and rmmod. But I still get:

00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
0000000d 48004a28 (fa004a28) cm_idlest3_core

Best regards,
									Pavel
Pavel Machek Feb. 7, 2016, 9:37 p.m. UTC | #6
Hi!

> > > No, sorry, that was with 4.4. As you hit "PM regression with commit
> > > 5de85b9d57ab PM runtime re-init in v4.5-rc1", I thought I'd avoid
> > > v4.5-rc. 
> > 
> > OK
> > 
> > > (I assume I have to insmod and rmmod, right? Because powersave is not
> > > entered if I simply compile-out usb).
> > 
> > Depending on what the bootloader does and probably also if
> > USB was used during the booting.. So yeah you may need to modprobe
> > and then rmmod.
> > 
> > > Would you have commit ids for those rmmod fixes? It might be good to
> > > push them into stable, and I should try again with them applied...
> > 
> > 055555fc459 ("usb: musb: core: Fix handling of the phy notifications")
> > 03e43528ab68 ("usb: musb: Fix unbalanced pm_runtime_enable")
> 
> Oh and looks like these two phy-twl4030-phy fixes never got
> merged from thread "[PATCH 0/2] Two phy-twl4030-usb fixes for
> unloading the module":
> 
> phy: twl4030-usb: Relase usb phy on unload
> phy: twl4030-usb: Fix unbalanced pm_runtime_enable on module reload
> 
> I'll resend those. But without those again deeper idle states
> are blocked.. And it could be that n900 needs similar additional
> patches for it's USB PHY as I've tested things so far only with
> phy-twl4030-usb PHY based systems.

I don't get it. I was using

+CONFIG_USB_MUSB_OMAP2PLUS=m
+CONFIG_TWL4030_USB=m

on the n900. Are you suggesting I should be using something else on
the N900? If so, what?

Pali, do you have deeper sleep states working on 4.4 kernel? (Or
anything 4.X?)
									
									Pavel
Pali Rohár Feb. 8, 2016, 8:51 a.m. UTC | #7
On Sunday 07 February 2016 22:37:20 Pavel Machek wrote:
> Pali, do you have deeper sleep states working on 4.4 kernel? (Or
> anything 4.X?)

Do not know, at least I did not play or test this part of power
management yet.
Tony Lindgren Feb. 9, 2016, 5:24 p.m. UTC | #8
* Pavel Machek <pavel@ucw.cz> [160207 13:24]:
> Hi!
> 
> > > (I assume I have to insmod and rmmod, right? Because powersave is not
> > > entered if I simply compile-out usb).
> > 
> > Depending on what the bootloader does and probably also if
> > USB was used during the booting.. So yeah you may need to modprobe
> > and then rmmod.
> > 
> > > Would you have commit ids for those rmmod fixes? It might be good to
> > > push them into stable, and I should try again with them applied...
> > 
> > 055555fc459 ("usb: musb: core: Fix handling of the phy notifications")
> > 03e43528ab68 ("usb: musb: Fix unbalanced pm_runtime_enable")
> 
> Ok, with that, I can insmod and rmmod. But I still get:
> 
> 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000

I think the cm_idlest_per is fine.

> ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
> 0000000d 48004a28 (fa004a28) cm_idlest3_core

Bit 21 in cm_idlest1_core is for MCSPI4 so WLAN. Does that go
down if do sleep 5; cat /sys/kernel/debug/pm_debug/count ?

If not, the're PM runtime missing or broken somewhere.

FYI, below is my omap3 usb test script that I use to start and
stop USB, it also works on n900. And after stopping it n900
continues hitting deeper idle states just fine.

That is with these two phy patches applied on v4.5-rc3:

"phy: twl4030-usb: Fix unbalanced pm_runtime_enable on module reload")
"phy: twl4030-usb: Relase usb phy on unload")

Maybe I gave you wrong info on the USB patches needed.. But these
two above are needed on v4.5-rc3. And looks like Kishon has now
applied them.

Oh and I also have the MMC PM runtime regression fix here that I
have not yet posted. Will post today. But I guess you're on
v4.4 right now still.

Regards,

Tony

8< --------
#!/bin/bash

# Change for your UDC controller
phy_module=phy_twl4030_usb
udc_glue=omap2430
udc_module=musb_hdrc
udc_name=musb-hdrc.0.auto

start_usb_gadgets() {
	vendor=0x1d6b
	product=0x0106
	file=$1
  
	mount -t configfs none /sys/kernel/config
	mkdir /sys/kernel/config/usb_gadget/g1
	old_pwd=$(pwd)
	cd /sys/kernel/config/usb_gadget/g1
	
	echo $product > idProduct
	echo $vendor > idVendor
	mkdir strings/0x409
	echo 123456789 > strings/0x409/serialnumber
	echo foo > strings/0x409/manufacturer
	echo "Multi Gadget" > strings/0x409/product
	
	mkdir configs/c.1
	echo 100 > configs/c.1/MaxPower
	mkdir configs/c.1/strings/0x409
	echo "Config 100mA" > configs/c.1/strings/0x409/configuration
	
	mkdir configs/c.5
	echo 500 > configs/c.5/MaxPower
	mkdir configs/c.5/strings/0x409
	echo "Config 500mA" > configs/c.5/strings/0x409/configuration

	mkdir functions/mass_storage.0
	echo $file > functions/mass_storage.0/lun.0/file
	ln -s functions/mass_storage.0 configs/c.1
	ln -s functions/mass_storage.0 configs/c.5

	mkdir functions/acm.0
	ln -s functions/acm.0 configs/c.1
	ln -s functions/acm.0 configs/c.5

	mkdir functions/ecm.0
	ln -s functions/ecm.0 configs/c.1
	ln -s functions/ecm.0 configs/c.5

	# Adding rndis seems to cause alignment trap or some
	# random oops on reboot after rmmod $udc_glue

	echo $udc_name > /sys/kernel/config/usb_gadget/g1/UDC
	cd $old_pwd
}

stop_usb_gadgets() {
	old_pwd=$(pwd)
	cd /sys/kernel/config/usb_gadget/g1

	echo "" > /sys/kernel/config/usb_gadget/g1/UDC

	rm configs/c.1/ecm.0
	rm configs/c.5/ecm.0
	rmdir functions/ecm.0

	rm configs/c.1/acm.0
	rm configs/c.5/acm.0
	rmdir functions/acm.0

	echo "" > functions/mass_storage.0/lun.0/file
	rm configs/c.1/mass_storage.0
	rm configs/c.5/mass_storage.0
	rmdir functions/mass_storage.0

	rmmod $udc_glue
	rmmod $phy_module
	rmmod $udc_module

	cd $old_pwd
}

case $1 in
	start)
	modprobe libcomposite
	modprobe $phy_module
	modprobe $udc_glue
	start_usb_gadgets ""
        ;;
	stop)
	stop_usb_gadgets
	;;
        *)
        ;;
esac
--
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
Tony Lindgren Feb. 9, 2016, 5:38 p.m. UTC | #9
* Tony Lindgren <tony@atomide.com> [160209 09:26]:
> * Pavel Machek <pavel@ucw.cz> [160207 13:24]:
> > Hi!
> > 
> > > > (I assume I have to insmod and rmmod, right? Because powersave is not
> > > > entered if I simply compile-out usb).
> > > 
> > > Depending on what the bootloader does and probably also if
> > > USB was used during the booting.. So yeah you may need to modprobe
> > > and then rmmod.
> > > 
> > > > Would you have commit ids for those rmmod fixes? It might be good to
> > > > push them into stable, and I should try again with them applied...
> > > 
> > > 055555fc459 ("usb: musb: core: Fix handling of the phy notifications")
> > > 03e43528ab68 ("usb: musb: Fix unbalanced pm_runtime_enable")
> > 
> > Ok, with that, I can insmod and rmmod. But I still get:
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> 
> I think the cm_idlest_per is fine.
> 
> > ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
> > 0000000d 48004a28 (fa004a28) cm_idlest3_core
> 
> Bit 21 in cm_idlest1_core is for MCSPI4 so WLAN. Does that go
> down if do sleep 5; cat /sys/kernel/debug/pm_debug/count ?
> 
> If not, the're PM runtime missing or broken somewhere.
> 
> FYI, below is my omap3 usb test script that I use to start and
> stop USB, it also works on n900. And after stopping it n900
> continues hitting deeper idle states just fine.
> 
> That is with these two phy patches applied on v4.5-rc3:
> 
> "phy: twl4030-usb: Fix unbalanced pm_runtime_enable on module reload")
> "phy: twl4030-usb: Relase usb phy on unload")
> 
> Maybe I gave you wrong info on the USB patches needed.. But these
> two above are needed on v4.5-rc3. And looks like Kishon has now
> applied them.
> 
> Oh and I also have the MMC PM runtime regression fix here that I
> have not yet posted. Will post today. But I guess you're on
> v4.4 right now still.

And looks like the USB RNDIS gadget has some extra issues
breaking reboot. So I'd stick to ECM if possible and leave out
RNDIS completely.

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
Tony Lindgren Feb. 11, 2016, 1:08 a.m. UTC | #10
* Tony Lindgren <tony@atomide.com> [160209 09:26]:
> * Pavel Machek <pavel@ucw.cz> [160207 13:24]:
> 
> > ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
> > 0000000d 48004a28 (fa004a28) cm_idlest3_core
> 
> Bit 21 in cm_idlest1_core is for MCSPI4 so WLAN. Does that go
> down if do sleep 5; cat /sys/kernel/debug/pm_debug/count ?
> 
> If not, the're PM runtime missing or broken somewhere.

Just tested n900 wlan here and it's working for me with Emil's
patch from the "" thread and the PM runtime regression fix for
hsmmc I posted earlier today. Looks like n900 is hitting off
mode just fine with wlan0 configured.

It does not wake up to pings from outside from off mode though
until the system wakes up.

I'll take a look and send a patch for that separately.

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
Pavel Machek March 20, 2016, 8:33 a.m. UTC | #11
Hi!

First, sorry for the delay. Busy with other projects, then tried to
keep USB in 4.5 from regressing.

I'm now back on 4.4 branch.

> > > > (I assume I have to insmod and rmmod, right? Because powersave is not
> > > > entered if I simply compile-out usb).
> > > 
> > > Depending on what the bootloader does and probably also if
> > > USB was used during the booting.. So yeah you may need to modprobe
> > > and then rmmod.
> > > 
> > > > Would you have commit ids for those rmmod fixes? It might be good to
> > > > push them into stable, and I should try again with them applied...
> > > 
> > > 055555fc459 ("usb: musb: core: Fix handling of the phy notifications")
> > > 03e43528ab68 ("usb: musb: Fix unbalanced pm_runtime_enable")
> > 
> > Ok, with that, I can insmod and rmmod. But I still get:
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> 
> I think the cm_idlest_per is fine.
> 
> > ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
> > 0000000d 48004a28 (fa004a28) cm_idlest3_core
> 
> Bit 21 in cm_idlest1_core is for MCSPI4 so WLAN. Does that go
> down if do sleep 5; cat /sys/kernel/debug/pm_debug/count ?

I'm using

sudo iw dev wlan0 set power_save on ; not sure if it matters.

I now have:

f7de3e8d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 0021c072

So that bit seems to stay here. Aha... and it went off now:

f7fe3e8d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 0001c072

Good. I still have some work in USB disabling.

> If not, the're PM runtime missing or broken somewhere.
> 
> FYI, below is my omap3 usb test script that I use to start and
> stop USB, it also works on n900. And after stopping it n900
> continues hitting deeper idle states just fine.
> 
> That is with these two phy patches applied on v4.5-rc3:
> 
> "phy: twl4030-usb: Fix unbalanced pm_runtime_enable on module reload")
> "phy: twl4030-usb: Relase usb phy on unload")
> 
> Maybe I gave you wrong info on the USB patches needed.. But these
> two above are needed on v4.5-rc3. And looks like Kishon has now
> applied them.

Lets see... yes, I seem to have those. Messing with modules is hard in
my environment, and I push kernels over usb from the "NOLO" IIRC.

> Oh and I also have the MMC PM runtime regression fix here that I
> have not yet posted. Will post today. But I guess you're on
> v4.4 right now still.

Yes. I can now go to 4.5 (or even 4.6-rc0 if needed), but I guess
staying on 4.4 is easiest for now.

Thanks for the script.

> 	rmmod $udc_glue
> 	rmmod $phy_module
> 	rmmod $udc_module

Aha, you are removing one more module; I was not working with
musb_hdrc.

Best regards,
									Pavel
Pavel Machek March 20, 2016, 8:38 a.m. UTC | #12
Hi!

> > Oh and I also have the MMC PM runtime regression fix here that I
> > have not yet posted. Will post today. But I guess you're on
> > v4.4 right now still.
> 
> And looks like the USB RNDIS gadget has some extra issues
> breaking reboot. So I'd stick to ECM if possible and leave out
> RNDIS completely.

Thanks for the information. I should be using ECM

CONFIG_USB_F_ECM=y
# CONFIG_USB_ETH_RNDIS is not set

Best regards,
									Pavel
Pavel Machek March 23, 2016, 12:38 p.m. UTC | #13
Hi!

> > > 055555fc459 ("usb: musb: core: Fix handling of the phy notifications")
> > > 03e43528ab68 ("usb: musb: Fix unbalanced pm_runtime_enable")
> > 
> > Ok, with that, I can insmod and rmmod. But I still get:
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> 
> I think the cm_idlest_per is fine.
> 
> > ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
> > 0000000d 48004a28 (fa004a28) cm_idlest3_core
> 
> Bit 21 in cm_idlest1_core is for MCSPI4 so WLAN. Does that go
> down if do sleep 5; cat /sys/kernel/debug/pm_debug/count ?
> 
> If not, the're PM runtime missing or broken somewhere.
> 
> FYI, below is my omap3 usb test script that I use to start and
> stop USB, it also works on n900. And after stopping it n900
> continues hitting deeper idle states just fine.

You rmmod musb_hdrc, so I checked, and found:

550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1093)
static void musb_shutdown(struct platform_de
vice *pdev)
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1094) {
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1095)
struct musb     *musb = dev_to_musb(&pdev
->dev);
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1096)
unsigned long   flags;
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1097)
4f9edd2d7 (Hema HK                   2011-03-22 16:02:12 +0530 1098)
pm_runtime_get_sync(musb->controller);
24307caef (Grazvydas Ignotas         2012-01-12 15:22:45 +0200 1099)
2cc65feab (Daniel Mack               2013-04-10 21:55:47 +0200 1100)
musb_host_cleanup(musb);
24307caef (Grazvydas Ignotas         2012-01-12 15:22:45 +0200 1101)
musb_gadget_cleanup(musb);
24307caef (Grazvydas Ignotas         2012-01-12 15:22:45 +0200 1102)
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1103)
spin_lock_irqsave(&musb->lock, flags);
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1104)
musb_platform_disable(musb);
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1105)
musb_generic_disable(musb);
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1106)
spin_unlock_irqrestore(&musb->lock, flags
);
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1107)
120d074c5 (Grazvydas Ignotas         2010-10-10 13:52:22 -0500 1108)
musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
120d074c5 (Grazvydas Ignotas         2010-10-10 13:52:22 -0500 1109)
musb_platform_exit(musb);
120d074c5 (Grazvydas Ignotas         2010-10-10 13:52:22 -0500 1110)
4f9edd2d7 (Hema HK                   2011-03-22 16:02:12 +0530 1111)
pm_runtime_put(musb->controller);
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1112)
/* FIXME power down */
550a7375f (Felipe Balbi              2008-07-24 12:27:36 +0300 1113) }

I thought i was responsible for the FIXME, but apparently not... If
you happen to have some changes there, that would be useful to
know....

Ok, another attempt at shutting USB down:

00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
f7dffe9d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200062
0000000d 48004a28 (fa004a28) cm_idlest3_core

Tried again today:

00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
f7dffe9d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200062
0000000d 48004a28 (fa004a28) cm_idlest3_core
pavel@n900:/my/tui/ofone$ sleep test

00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
ffde7e9d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218062
0000000d 48004a28 (fa004a28) cm_idlest3_core

00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
fedffe9d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 01200062
0000000d 48004a28 (fa004a28) cm_idlest3_core

00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
ffde7e9d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218062
0000000d 48004a28 (fa004a28) cm_idlest3_core

Is there documentation for the cm_idlest1_ bits?

How idle system do I need to have? Screen is blanked and machine
should be mostly idle, but there's X running on another vt with Mate
desktop, and some python scripts... GSM modem should be online.

Are you booting over USB from NOLO?

Thanks and best regards,

								Pavel
Pavel Machek March 23, 2016, 2:37 p.m. UTC | #14
On Wed 2016-02-10 17:08:18, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [160209 09:26]:
> > * Pavel Machek <pavel@ucw.cz> [160207 13:24]:
> > 
> > > ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
> > > 0000000d 48004a28 (fa004a28) cm_idlest3_core
> > 
> > Bit 21 in cm_idlest1_core is for MCSPI4 so WLAN. Does that go
> > down if do sleep 5; cat /sys/kernel/debug/pm_debug/count ?
> > 
> > If not, the're PM runtime missing or broken somewhere.
> 
> Just tested n900 wlan here and it's working for me with Emil's
> patch from the "" thread and the PM runtime regression fix for
> hsmmc I posted earlier today. Looks like n900 is hitting off
> mode just fine with wlan0 configured.

Emil's patch, would that be:

commit fb724ed5c6175a2407b256d506c9e703c6bb62d4
Author: Emil Goode <emil.fsw@goode.io>
Date:   Wed Feb 10 02:22:16 2016 +0100

    wlcore: Fix regression in wlcore_set_partition()

    The commit 3719c17e1816 ("wlcore/wl18xx: fw logger over sdio")
    introduced a
        regression causing the wlcore to time out and go into
    recovery. Reverting the
        changes regarding write of the last partition size brings the
    module back to
        it's functional state.

    Fixes: 3719c17e1816 ("wlcore/wl18xx: fw logger over sdio")

?

Hsmmc regression, that's

Date: Wed, 10 Feb 2016 15:02:44 -0800
From: Tony Lindgren <tony@atomide.com>
To: linux-omap@vger.kernel.org
Subject: [PATCH 1/7] mmc: omap_hsmmc: Fix PM regression with deferred probe for pm_runtime_reinit

? Aha, but the patch this regression fixes does not seem to be in 4.4,
so I should be fine unless I go to 4.5.

> It does not wake up to pings from outside from off mode though
> until the system wakes up.

Ok, I guess I'm trying to get it to sleep, first, then we can talk
about wakeup ;-).

Best regards,
									Pavel
Tony Lindgren March 30, 2016, 7:12 p.m. UTC | #15
* Pavel Machek <pavel@ucw.cz> [160323 05:38]:
> 
> Ok, another attempt at shutting USB down:
> 
> 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> f7dffe9d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200062
> 0000000d 48004a28 (fa004a28) cm_idlest3_core
> 
> Tried again today:
> 
> 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> f7dffe9d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200062
> 0000000d 48004a28 (fa004a28) cm_idlest3_core
> pavel@n900:/my/tui/ofone$ sleep test
> 
> 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> ffde7e9d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218062
> 0000000d 48004a28 (fa004a28) cm_idlest3_core
> 
> 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> fedffe9d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 01200062
> 0000000d 48004a28 (fa004a28) cm_idlest3_core
> 
> 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> ffde7e9d 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218062
> 0000000d 48004a28 (fa004a28) cm_idlest3_core
> 
> Is there documentation for the cm_idlest1_ bits?

Yes, see the TRM.

> How idle system do I need to have? Screen is blanked and machine
> should be mostly idle, but there's X running on another vt with Mate
> desktop, and some python scripts... GSM modem should be online.

Well I think it's the USB only you have blocking deeper idle states.

Are you sure you rmmod:ed all the USB related modules like in my
test script?

MUSB currently has an unresolved issue where it blocks idle states
if loaded.

> Are you booting over USB from NOLO?

I'm booting over smc91x using u-boot.

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
Pali Rohár April 4, 2016, 11:09 a.m. UTC | #16
On Wednesday 30 March 2016 12:12:09 Tony Lindgren wrote:
> > How idle system do I need to have? Screen is blanked and machine
> > should be mostly idle, but there's X running on another vt with Mate
> > desktop, and some python scripts... GSM modem should be online.
> 
> Well I think it's the USB only you have blocking deeper idle states.
> 
> Are you sure you rmmod:ed all the USB related modules like in my
> test script?
> 
> MUSB currently has an unresolved issue where it blocks idle states
> if loaded.

Is somebody working on this musb issue? Is there any progress?
Pavel Machek April 4, 2016, 9:30 p.m. UTC | #17
Hi!

> > How idle system do I need to have? Screen is blanked and machine
> > should be mostly idle, but there's X running on another vt with Mate
> > desktop, and some python scripts... GSM modem should be online.
> 
> Well I think it's the USB only you have blocking deeper idle states.
> 
> Are you sure you rmmod:ed all the USB related modules like in my
> test script?

I tested with rmmoding two, and then modified kernel to do equivalent
operations.

Let me try again:

phy_module=phy_twl4030_usb
udc_glue=omap2430
udc_module=musb_hdrc

Ok, I did "start" with your script. After some fiddling it seems to
work, and PC detected the values:

[25379.533127] usb 1-5: USB disconnect, device number 12
[27161.884132] usb 1-5: new high-speed USB device number 13 using
ehci-pci
[27161.996037] usb 1-5: device descriptor read/64, error -32
[27162.420048] usb 4-1: new full-speed USB device number 2 using
uhci_hcd
[27162.563098] usb 4-1: not running at top speed; connect to a high
speed hub
[27162.601106] usb 4-1: New USB device found, idVendor=1d6b,
idProduct=0106
[27162.601112] usb 4-1: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
[27162.601115] usb 4-1: Product: Multi Gadget
[27162.601118] usb 4-1: Manufacturer: foo
[27162.601121] usb 4-1: SerialNumber: 123456789
[27162.614766] cdc_ether 4-1:1.0 usb0: register 'cdc_ether' at
usb-0000:00:1d.2-1, CDC Ethernet Device, c6:8b:83:74:f2:f3
[27163.195272] cdc_ether 4-1:1.0 usb0: kevent 12 may have been dropped

Stop seemed to work, too:

root@n900:/my/tui/ofone# ./gadgets.sh stop
rm: cannot remove `configs/c.1/acm.0': No such file or directory
rm: cannot remove `configs/c.5/acm.0': No such file or directory
rmdir: failed to remove `functions/acm.0': No such file or directory
./gadgets.sh: line 74: functions/mass_storage.0/lun.0/file: No such
file or directory
rm: cannot remove `configs/c.1/mass_storage.0': No such file or
directory
rm: cannot remove `configs/c.5/mass_storage.0': No such file or
directory
rmdir: failed to remove `functions/mass_storage.0': No such file or
directory
root@n900:/my/tui/ofone# lsmod
Module                  Size  Used by
root@n900:/my/tui/ofone#

Display off, on wifi:

00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
f7de7ebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218042
0000000d 48004a28 (fa004a28) cm_idlest3_core

00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
ffde7ebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218042

00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042

..so I believe gadget bits are still set.

> I'm booting over smc91x using u-boot.

I'm booting directly from NOLO over usb. Machine is running X, but
they are not active -- chvt. Screen is off.

Best regards,


									Pavel
Tony Lindgren April 4, 2016, 10:07 p.m. UTC | #18
* Pavel Machek <pavel@ucw.cz> [160404 14:31]:
> 
> Display off, on wifi:
> 
> 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> f7de7ebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218042
> 0000000d 48004a28 (fa004a28) cm_idlest3_core
> 
> 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> ffde7ebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218042
> 
> 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
> 
> ..so I believe gadget bits are still set.

Nope, you got PM working now for the USB as it's now ending with 0x42
instead of 0x62 :) You still have bit 21 blocking, sorry don't
remember what that one is, but that's in the TRM for idlest1
register.

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
Tony Lindgren April 4, 2016, 10:31 p.m. UTC | #19
* Pali Rohár <pali.rohar@gmail.com> [160404 04:10]:
> On Wednesday 30 March 2016 12:12:09 Tony Lindgren wrote:
> > > How idle system do I need to have? Screen is blanked and machine
> > > should be mostly idle, but there's X running on another vt with Mate
> > > desktop, and some python scripts... GSM modem should be online.
> > 
> > Well I think it's the USB only you have blocking deeper idle states.
> > 
> > Are you sure you rmmod:ed all the USB related modules like in my
> > test script?
> > 
> > MUSB currently has an unresolved issue where it blocks idle states
> > if loaded.
> 
> Is somebody working on this musb issue? Is there any progress?

No idea when I might get to it.. Please take a look if you have  a
chance to work on it, I think all we have to do:

1. Change musb_gadget_pullup() to run as tasklet/delayed_work to
   avoid trying to access I2C PHY's from atomic context. See
   3e43a0725637 ("usb: musb: core: add pm_runtime_irq_safe()")
   for more info. Not sure if this should be done in a generic
   way or for MUSB only. It could be that I remember wrong what
   I thought needs to be done. But we want to get rid of the
   pm_runtime_irq_save() as that currently forever blocks PM
   for the MUSB hardware specific wrapper driver.

2. Remove pm_runtime_irq_safe() added in commit 3e43a0725637

Adding Bin to Cc, maybe he has some better fix in mind.

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
Pavel Machek April 5, 2016, 10:09 a.m. UTC | #20
On Mon 2016-04-04 15:07:49, Tony Lindgren wrote:
> * Pavel Machek <pavel@ucw.cz> [160404 14:31]:
> > 
> > Display off, on wifi:
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> > f7de7ebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218042
> > 0000000d 48004a28 (fa004a28) cm_idlest3_core
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> > ffde7ebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218042
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> > ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
> > 
> > ..so I believe gadget bits are still set.
> 
> Nope, you got PM working now for the USB as it's now ending with 0x42
> instead of 0x62 :) You still have bit 21 blocking, sorry don't
> remember what that one is, but that's in the TRM for idlest1
> register.

Aha... I thought the "0x42" was the problem, not "0x30". That was
going on and off for me for a long while. Actually, I don't think I
did anything special with USB gadget in this boot, and it is still
0x42.

Bit 21 is wifi, AFAICT. With right sleeps between reading the debug
file and talking to wifi, I now get:

Battery 4.16V 4.18V 4.14V 95% 93% 100% 1569/1569 mAh Charging
49/650/1800 mA
fffffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00000042
Battery 4.16V 4.18V 4.14V 95% 93% 100% 1569/1569 mAh Charging
48/650/1800 mA
fffffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00000042

0x02 seems to be SDRC.
0x40 seems to be OMAPCTRL

I get

0x18000 randomly, too. That should be I2C1, and I2C2. But even when
idlest1_blocking_bits are 0x42, the left and right backlight on
keyboard is lit constantly -- not blinking -- indicating that it is
still not sleeping.

Full debug is:

Battery 4.15V 4.18V 4.13V 94% 93% 100% 1569/1569 mAh Charging
29/650/1800 mA
usbhost_pwrdm
(ON),OFF:12183,RET:277831,INA:0,ON:290015,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
sgx_pwrdm
(OFF),OFF:1,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
core_pwrdm
(ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0
per_pwrdm
(ON),OFF:12183,RET:0,INA:0,ON:12184,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
dss_pwrdm
(ON),OFF:12183,RET:267009,INA:0,ON:279193,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
cam_pwrdm
(ON),OFF:12183,RET:277826,INA:2,ON:290012,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
neon_pwrdm (ON),OFF:12183,RET:277832,INA:0,ON:290016,RET-LOGIC-OFF:0
mpu_pwrdm
(ON),OFF:12183,RET:277830,INA:0,ON:290014,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
iva2_pwrdm
(OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RET-MEMBANK4-OFF:0
usbhost_clkdm->usbhost_pwrdm (1)
sgx_clkdm->sgx_pwrdm (0)
per_clkdm->per_pwrdm (19)
cam_clkdm->cam_pwrdm (1)
dss_clkdm->dss_pwrdm (1)
d2d_clkdm->core_pwrdm (0)
iva2_clkdm->iva2_pwrdm (0)
mpu_clkdm->mpu_pwrdm (0)
core_l4_clkdm->core_pwrdm (19)
core_l3_clkdm->core_pwrdm (1)
neon_clkdm->neon_pwrdm (0)
00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
fffffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00000042
0000000d 48004a28 (fa004a28) cm_idlest3_core
^CTraceback (most recent call last):
  File "./sleepmond", line 24, in <module>
      time.sleep(5)
      KeyboardInterrupt


Anything else I should look at?

Thanks and best regards,
									Pavel
Pavel Machek April 5, 2016, 1:17 p.m. UTC | #21
Hi!

> > 
> > Display off, on wifi:
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> > f7de7ebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218042
> > 0000000d 48004a28 (fa004a28) cm_idlest3_core
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> > ffde7ebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218042
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> > ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
> > 
> > ..so I believe gadget bits are still set.
> 
> Nope, you got PM working now for the USB as it's now ending with 0x42
> instead of 0x62 :) You still have bit 21 blocking, sorry don't
> remember what that one is, but that's in the TRM for idlest1
> register.

Looking more through the registers... GPIO banks seem to be listed in
cm_idlest_per. You mentioned that they should be automatically
controlled? Does that still work when they are exported to the
userspace?

pavel@n900:~$ ls /sys/class/gpio/
export	       gpio177  gpio73  gpio75     gpiochip128  gpiochip32
gpiochip64  unexport
gpio157  gpio70   gpio74  gpiochip0  gpiochip160  gpiochip494
gpiochip96

Best regards,
									Pavel
Pavel Machek April 5, 2016, 2:22 p.m. UTC | #22
Hi!

> * Pavel Machek <pavel@ucw.cz> [160404 14:31]:
> > 
> > Display off, on wifi:
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> > f7de7ebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218042
> > 0000000d 48004a28 (fa004a28) cm_idlest3_core
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> > ffde7ebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00218042
> > 
> > 00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
> > ffdffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00200042
> > 
> > ..so I believe gadget bits are still set.
> 
> Nope, you got PM working now for the USB as it's now ending with 0x42
> instead of 0x62 :) You still have bit 21 blocking, sorry don't
> remember what that one is, but that's in the TRM for idlest1
> register.

Ok, I realized that _something_ set up my keyboard on console, too, so
I'm able to do "init 1" and still interact with the console.

cm_idlest1_core blocking bits are now 0x42 pretty consistently. But
they stay 0x42, even when screen is on (and screen on prevents low
power, right?) so I guess they are not the whole story.

I'm able to get down to 50mA power consumption with screen off. I was
getting 90mA with X, wifi in powersave and screen off.

Head proximity sensor is still on in this configuration. But I guess
that should not keep the system busy...?

Ok, wait a moment, I'm getting "Camera Focus", "Camera Capture" and
"Lock button" interrupts ... at something like 20/second. Without
touching anything. Hmm. Also "pm_wkup", but that might be
expected. Plus, 49052000.gpio and 49054000.gpio are rather
active. Might be related to the above. Also "gp_timer" and
48070000.i2c are active, perhaps also related. I don't think I have
the camera working...

"Lock button" interrupt stops increasing as long as I hold the lock
button in the "unlock" position. "Camera Focus" and "Camera Capture"
will stop increasing when I hold down the Camera button.

Best regards,
									Pavel
Tony Lindgren April 5, 2016, 3:29 p.m. UTC | #23
* Pavel Machek <pavel@ucw.cz> [160405 07:23]:
>
> Ok, I realized that _something_ set up my keyboard on console, too, so
> I'm able to do "init 1" and still interact with the console.
> 
> cm_idlest1_core blocking bits are now 0x42 pretty consistently. But
> they stay 0x42, even when screen is on (and screen on prevents low
> power, right?) so I guess they are not the whole story.

Yes they stay at 0x42. And it seems you've configured the UART
idle timeouts too.

> I'm able to get down to 50mA power consumption with screen off. I was
> getting 90mA with X, wifi in powersave and screen off.
> 
> Head proximity sensor is still on in this configuration. But I guess
> that should not keep the system busy...?
> 
> Ok, wait a moment, I'm getting "Camera Focus", "Camera Capture" and
> "Lock button" interrupts ... at something like 20/second. Without
> touching anything. Hmm. Also "pm_wkup", but that might be
> expected. Plus, 49052000.gpio and 49054000.gpio are rather
> active. Might be related to the above. Also "gp_timer" and
> 48070000.i2c are active, perhaps also related. I don't think I have
> the camera working...

Hmm that does not sound right at all for the GPIOs. The pm_wkup
interrupt triggers every time you hit wfi pretty much, so that
should be OK.

> "Lock button" interrupt stops increasing as long as I hold the lock
> button in the "unlock" position. "Camera Focus" and "Camera Capture"
> will stop increasing when I hold down the Camera button.

I have not seen that issue last time I checked. Sorry won't
be able to test it until Thursday, maybe there's some GPIO
regression now.

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
Pavel Machek April 5, 2016, 8:51 p.m. UTC | #24
Hi!

> > Ok, I realized that _something_ set up my keyboard on console, too, so
> > I'm able to do "init 1" and still interact with the console.
> > 
> > cm_idlest1_core blocking bits are now 0x42 pretty consistently. But
> > they stay 0x42, even when screen is on (and screen on prevents low
> > power, right?) so I guess they are not the whole story.
> 
> Yes they stay at 0x42. And it seems you've configured the UART
> idle timeouts too.

Yes, thanks for the script ;-).

> > I'm able to get down to 50mA power consumption with screen off. I was
> > getting 90mA with X, wifi in powersave and screen off.
> > 
> > Head proximity sensor is still on in this configuration. But I guess
> > that should not keep the system busy...?
> > 
> > Ok, wait a moment, I'm getting "Camera Focus", "Camera Capture" and
> > "Lock button" interrupts ... at something like 20/second. Without
> > touching anything. Hmm. Also "pm_wkup", but that might be
> > expected. Plus, 49052000.gpio and 49054000.gpio are rather
> > active. Might be related to the above. Also "gp_timer" and
> > 48070000.i2c are active, perhaps also related. I don't think I have
> > the camera working...
> 
> Hmm that does not sound right at all for the GPIOs. The pm_wkup
> interrupt triggers every time you hit wfi pretty much, so that
> should be OK.

Wifi was down at that point.

Some more testing: after boot, interrupt counts stay low, as
expected. But when I attempt to enable power management, they start
rising. Sometimes 60/second, sometimes less.

Things are fine as long as I don't enable the off mode; when I enable
the off mode, "echo 1 > /sys/kernel/debug/pm_debug/enable_off_mode"
interrupt counts start rising immediately. "echo 0 >
/sys/kernel/debug/pm_debug/enable_off_mode" stops that.

But no matter what configuration, activity LEDs still indicate it is
busy, and idle power consumption is cca 53mA.

> > "Lock button" interrupt stops increasing as long as I hold the lock
> > button in the "unlock" position. "Camera Focus" and "Camera Capture"
> > will stop increasing when I hold down the Camera button.
> 
> I have not seen that issue last time I checked. Sorry won't
> be able to test it until Thursday, maybe there's some GPIO
> regression now.

No problem, thanks for all the help.

Best regards,
									Pavel
Tony Lindgren April 7, 2016, 5:40 p.m. UTC | #25
Hi,

* Pavel Machek <pavel@ucw.cz> [160405 13:52]:
> Hi!
> 
> > > Ok, I realized that _something_ set up my keyboard on console, too, so
> > > I'm able to do "init 1" and still interact with the console.
> > > 
> > > cm_idlest1_core blocking bits are now 0x42 pretty consistently. But
> > > they stay 0x42, even when screen is on (and screen on prevents low
> > > power, right?) so I guess they are not the whole story.
> > 
> > Yes they stay at 0x42. And it seems you've configured the UART
> > idle timeouts too.
> 
> Yes, thanks for the script ;-).
> 
> > > I'm able to get down to 50mA power consumption with screen off. I was
> > > getting 90mA with X, wifi in powersave and screen off.
> > > 
> > > Head proximity sensor is still on in this configuration. But I guess
> > > that should not keep the system busy...?
> > > 
> > > Ok, wait a moment, I'm getting "Camera Focus", "Camera Capture" and
> > > "Lock button" interrupts ... at something like 20/second. Without
> > > touching anything. Hmm. Also "pm_wkup", but that might be
> > > expected. Plus, 49052000.gpio and 49054000.gpio are rather
> > > active. Might be related to the above. Also "gp_timer" and
> > > 48070000.i2c are active, perhaps also related. I don't think I have
> > > the camera working...
> > 
> > Hmm that does not sound right at all for the GPIOs. The pm_wkup
> > interrupt triggers every time you hit wfi pretty much, so that
> > should be OK.
> 
> Wifi was down at that point.
> 
> Some more testing: after boot, interrupt counts stay low, as
> expected. But when I attempt to enable power management, they start
> rising. Sometimes 60/second, sometimes less.
> 
> Things are fine as long as I don't enable the off mode; when I enable
> the off mode, "echo 1 > /sys/kernel/debug/pm_debug/enable_off_mode"
> interrupt counts start rising immediately. "echo 0 >
> /sys/kernel/debug/pm_debug/enable_off_mode" stops that.
> 
> But no matter what configuration, activity LEDs still indicate it is
> busy, and idle power consumption is cca 53mA.

Not seeing that here at all. My n900 with v4.6-rc2 and omap2plus_defconfig
booted with u-boot keeps hitting off mode with both LEDs going off just
fine. It's waking up about once a second or a litte bit more often.

Care to post your .config somewhere, I could give that a try?

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
Pavel Machek April 7, 2016, 7:48 p.m. UTC | #26
Hi!

> > > > Ok, I realized that _something_ set up my keyboard on console, too, so
> > > > I'm able to do "init 1" and still interact with the console.
> > > > 
> > > > cm_idlest1_core blocking bits are now 0x42 pretty consistently. But
> > > > they stay 0x42, even when screen is on (and screen on prevents low
> > > > power, right?) so I guess they are not the whole story.
> > > 
> > > Yes they stay at 0x42. And it seems you've configured the UART
> > > idle timeouts too.
> > 
> > Yes, thanks for the script ;-).
> > 
> > > > I'm able to get down to 50mA power consumption with screen off. I was
> > > > getting 90mA with X, wifi in powersave and screen off.
> > > > 
> > > > Head proximity sensor is still on in this configuration. But I guess
> > > > that should not keep the system busy...?
> > > > 
> > > > Ok, wait a moment, I'm getting "Camera Focus", "Camera Capture" and
> > > > "Lock button" interrupts ... at something like 20/second. Without
> > > > touching anything. Hmm. Also "pm_wkup", but that might be
> > > > expected. Plus, 49052000.gpio and 49054000.gpio are rather
> > > > active. Might be related to the above. Also "gp_timer" and
> > > > 48070000.i2c are active, perhaps also related. I don't think I have
> > > > the camera working...
> > > 
> > > Hmm that does not sound right at all for the GPIOs. The pm_wkup
> > > interrupt triggers every time you hit wfi pretty much, so that
> > > should be OK.
> > 
> > Wifi was down at that point.
> > 
> > Some more testing: after boot, interrupt counts stay low, as
> > expected. But when I attempt to enable power management, they start
> > rising. Sometimes 60/second, sometimes less.
> > 
> > Things are fine as long as I don't enable the off mode; when I enable
> > the off mode, "echo 1 > /sys/kernel/debug/pm_debug/enable_off_mode"
> > interrupt counts start rising immediately. "echo 0 >
> > /sys/kernel/debug/pm_debug/enable_off_mode" stops that.
> > 
> > But no matter what configuration, activity LEDs still indicate it is
> > busy, and idle power consumption is cca 53mA.
> 
> Not seeing that here at all. My n900 with v4.6-rc2 and omap2plus_defconfig
> booted with u-boot keeps hitting off mode with both LEDs going off just
> fine. It's waking up about once a second or a litte bit more often.

Ok, that was 4.4 (with patches that should not be related).

I now checked with 4.6-rc2+mini_changes, and I'm getting similar results:

Battery 4.05V 4.07V 4.11V 85% 91% 98% 1546/1569 mAh Full -131/550/100
mA
00001fff 48005020 (fa005020) cm_idlest_per blocking bits: 0007e000
f7fffebd 48004a20 (fa004a20) cm_idlest1_core blocking bits: 00000042
0000000d 48004a28 (fa004a28) cm_idlest3_core

I'm booting from NOLO, and I'm _not_ loading/removing gadget modules,
as blocking bits are 0x42, so gadget is not a problem. (Let me know if
I'm wrong).

Let me retry with 4.6-rc2 (9735a22799b9214d17d3c231fe377fc852f042e9),
no modifications. Aha, but vanilla 4.6-rc2 does not have
cm_idlest1_core display in debugfs :-(. 

And yes, I see the same effect in /proc/interrupts:

180:        280  49052000.gpio   4 Edge      Camera Focus
181:        280  49052000.gpio   5 Edge      Camera Capture
(few seconds after that)
180:        738  49052000.gpio   4 Edge      Camera Focus
181:        738  49052000.gpio   5 Edge      Camera Capture

after I do echo 1 > /sys/kernel/debug/pm_debug/enable_off_mode .

> Care to post your .config somewhere, I could give that a try?

gzipped config is attached.

Note that I'm still using NOLO. I enabled the sleep, then went to
runlevel 1. LEDs still stay on, 55mA power consumption. That was with
1 in off_mode.

Best regards,
									Pavel
Tony Lindgren April 7, 2016, 9:32 p.m. UTC | #27
* Pavel Machek <pavel@ucw.cz> [160407 12:49]:
> 
> gzipped config is attached.
> 
> Note that I'm still using NOLO. I enabled the sleep, then went to
> runlevel 1. LEDs still stay on, 55mA power consumption. That was with
> 1 in off_mode.

Nothing idling for me with your .config.. And it seems slower to boot
compared to omap2plus_defconfig? Maybe because of the extra GPIO
interrupts?

Looks like you have 117 additional entries in .config enabled compared
to omap2plus_defconfig. Maybe go back to omap2plus_defconfig with minimal
changes and verify it idles properly first?

I'm suspecting it's some driver(s) you have enabled causing the
issue.

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
Pavel Machek April 7, 2016, 11:01 p.m. UTC | #28
Hi!

> > gzipped config is attached.
> > 
> > Note that I'm still using NOLO. I enabled the sleep, then went to
> > runlevel 1. LEDs still stay on, 55mA power consumption. That was with
> > 1 in off_mode.
> 
> Nothing idling for me with your .config.. And it seems slower to boot
> compared to omap2plus_defconfig? Maybe because of the extra GPIO
> interrupts?

Extra interrupts only happen when enable_off_mode is 1, so that should
not be an issue during boot.

> Looks like you have 117 additional entries in .config enabled compared
> to omap2plus_defconfig. Maybe go back to omap2plus_defconfig with minimal
> changes and verify it idles properly first?
> 
> I'm suspecting it's some driver(s) you have enabled causing the
> issue.

I guess so. Do you (or anyone else) have minimum non-modular config
for N900 that boots with video? Could I get lsmod from your system?
(Yes, I still have nightmares from getting .config that works).

Thanks and best regards,
									Pavel
Tony Lindgren April 7, 2016, 11:41 p.m. UTC | #29
* Pavel Machek <pavel@ucw.cz> [160407 16:02]:
> Hi!
> 
> > > gzipped config is attached.
> > > 
> > > Note that I'm still using NOLO. I enabled the sleep, then went to
> > > runlevel 1. LEDs still stay on, 55mA power consumption. That was with
> > > 1 in off_mode.
> > 
> > Nothing idling for me with your .config.. And it seems slower to boot
> > compared to omap2plus_defconfig? Maybe because of the extra GPIO
> > interrupts?
> 
> Extra interrupts only happen when enable_off_mode is 1, so that should
> not be an issue during boot.

OK maybe it's just the extra driver probe time then.

> > Looks like you have 117 additional entries in .config enabled compared
> > to omap2plus_defconfig. Maybe go back to omap2plus_defconfig with minimal
> > changes and verify it idles properly first?
> > 
> > I'm suspecting it's some driver(s) you have enabled causing the
> > issue.
> 
> I guess so. Do you (or anyone else) have minimum non-modular config
> for N900 that boots with video? Could I get lsmod from your system?
> (Yes, I still have nightmares from getting .config that works).

Well I've been just using omap2plus_defconfig with:

# modprobe tsc2005
# modprobe gpio_backlight
# modprobe panel_sony_acx565akm
# modprobe omapfb

# echo 255 > /sys/class/backlight/acx565akm/brightness

And then the following to blank for idle:

# echo 1 > /sys/devices/platform/omapfb/graphics/fb0/blank

But in the world of eternal regressions, I'm not seeing anything
on the framebuffer with v4.6-rc2 :( No idea what broke it or when
as my n900 is in the rack.

Anyways, below is also my lsmod output.

Regards,

Tony


Module                  Size  Used by
panel_sharp_ls037v7dw01     4148  0
ads7846                12959  0
hwmon                   4213  1 ads7846
gpio_keys               9053  0
twl4030_keypad          3896  0
matrix_keymap           2801  1 twl4030_keypad
omapfb                 39255  1
cfbfillrect             3614  1 omapfb
cfbimgblt               2416  1 omapfb
cfbcopyarea             3187  1 omapfb
panel_sony_acx565akm     7895  1
omapdss               269684  4 panel_sharp_ls037v7dw01,omapfb,panel_sony_acx565akm
gpio_backlight          2804  0
tsc2005                 1782  0
tsc200x_core            7337  1 tsc2005
ledtrig_default_on      1119  0
leds_gpio               3530  0
led_class               5418  1 leds_gpio
rtc_twl                 6234  0
twl4030_wdt             2711  0
--
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
Pavel Machek April 8, 2016, 9:19 a.m. UTC | #30
Hi!

> > > Looks like you have 117 additional entries in .config enabled compared
> > > to omap2plus_defconfig. Maybe go back to omap2plus_defconfig with minimal
> > > changes and verify it idles properly first?
> > > 
> > > I'm suspecting it's some driver(s) you have enabled causing the
> > > issue.
> > 
> > I guess so. Do you (or anyone else) have minimum non-modular config
> > for N900 that boots with video? Could I get lsmod from your system?
> > (Yes, I still have nightmares from getting .config that works).
> 
> Well I've been just using omap2plus_defconfig with:
> 
> # modprobe tsc2005
> # modprobe gpio_backlight
> # modprobe panel_sony_acx565akm
> # modprobe omapfb
> 
> # echo 255 > /sys/class/backlight/acx565akm/brightness
> 
> And then the following to blank for idle:
> 
> # echo 1 > /sys/devices/platform/omapfb/graphics/fb0/blank
> 
> But in the world of eternal regressions, I'm not seeing anything
> on the framebuffer with v4.6-rc2 :( No idea what broke it or when
> as my n900 is in the rack.

Well, for the record, framebuffer works in my config with v4.6-rc2. So
that is another config issue. Hopefully it is not the same issue ;-).

Best regards,
									Pavel
Pavel Machek April 11, 2016, 9:30 a.m. UTC | #31
Hi!

> > > > gzipped config is attached.
> > > > 
> > > > Note that I'm still using NOLO. I enabled the sleep, then went to
> > > > runlevel 1. LEDs still stay on, 55mA power consumption. That was with
> > > > 1 in off_mode.
> > > 
> > > Nothing idling for me with your .config.. And it seems slower to boot
> > > compared to omap2plus_defconfig? Maybe because of the extra GPIO
> > > interrupts?
> > 
> > Extra interrupts only happen when enable_off_mode is 1, so that should
> > not be an issue during boot.
> 
> OK maybe it's just the extra driver probe time then.

Ok, I did try to minimize config differences. I'm still booting from
NOLO. Now I'm doing the tests from init=... boot. idlest1_core
blocking bits says 0042. But the LEDs still don't blink.

I made config with pretty minimal differences from defconfig (making
required drivers =y, lockdep off so that video works).

Do you think you could try with my config?

> # echo 255 > /sys/class/backlight/acx565akm/brightness
> 
> And then the following to blank for idle:
> 
> # echo 1 > /sys/devices/platform/omapfb/graphics/fb0/blank

Yes, I'm doing this, on console.

Thanks and best regards,
								Pavel
Pavel Machek April 11, 2016, 9:41 a.m. UTC | #32
Hi!

> > > > > gzipped config is attached.
> > > > > 
> > > > > Note that I'm still using NOLO. I enabled the sleep, then went to
> > > > > runlevel 1. LEDs still stay on, 55mA power consumption. That was with
> > > > > 1 in off_mode.
> > > > 
> > > > Nothing idling for me with your .config.. And it seems slower to boot
> > > > compared to omap2plus_defconfig? Maybe because of the extra GPIO
> > > > interrupts?
> > > 
> > > Extra interrupts only happen when enable_off_mode is 1, so that should
> > > not be an issue during boot.
> > 
> > OK maybe it's just the extra driver probe time then.
> 
> Ok, I did try to minimize config differences. I'm still booting from
> NOLO. Now I'm doing the tests from init=... boot. idlest1_core
> blocking bits says 0042. But the LEDs still don't blink.
> 
> I made config with pretty minimal differences from defconfig (making
> required drivers =y, lockdep off so that video works).
> 
> Do you think you could try with my config?

Ok, it works now. I was doing tests in daylight so it was poorly
visible. The right part of keyboard stays lit (but that's expected
AFAICT), but the left part blinks.

Thanks!
								Pavel
diff mbox

Patch

diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 3a707dd..ac3761b 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -532,6 +532,43 @@  static ssize_t twl4030_usb_vbus_show(struct device *dev,
 }
 static DEVICE_ATTR(vbus, 0444, twl4030_usb_vbus_show, NULL);
 
+static ssize_t twl4030_test_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct twl4030_usb *twl = dev_get_drvdata(dev);
+	int ret = -EINVAL;
+
+	mutex_lock(&twl->lock);
+	ret = sprintf(buf, "%s\n", "hello, world");
+	mutex_unlock(&twl->lock);
+
+	return ret;
+}
+
+static int twl4030_shutdown(struct twl4030_usb *twl);
+
+static ssize_t twl4030_test_store(struct device *dev,
+		 struct device_attribute *attr, const char *buf, size_t count)
+{
+	unsigned long tmp;
+
+	struct twl4030_usb *twl = dev_get_drvdata(dev);
+
+	mutex_lock(&twl->lock);
+	sscanf(buf, "%lX", &tmp);
+	printk("TWL HACK: tmp = 0x%lX\n", tmp);
+	mutex_unlock(&twl->lock);
+
+	if (tmp == 0xdead) {
+		printk("TWL HACK: killing hardware\n");
+		printk("TWL HACK: killing hardware = %d\n", twl4030_shutdown(twl));
+	}
+	
+	return strnlen(buf, count);
+}
+
+static DEVICE_ATTR(test, 0664, twl4030_test_show, twl4030_test_store);
+
 static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
 {
 	struct twl4030_usb *twl = _twl;
@@ -710,6 +747,9 @@  static int twl4030_usb_probe(struct platform_device *pdev)
 	if (device_create_file(&pdev->dev, &dev_attr_vbus))
 		dev_warn(&pdev->dev, "could not create sysfs file\n");
 
+	if (device_create_file(&pdev->dev, &dev_attr_test))
+		dev_warn(&pdev->dev, "could not create sysfs file #2\n");
+	
 	ATOMIC_INIT_NOTIFIER_HEAD(&twl->phy.notifier);
 
 	pm_runtime_use_autosuspend(&pdev->dev);
@@ -745,14 +785,12 @@  static int twl4030_usb_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int twl4030_usb_remove(struct platform_device *pdev)
+static int twl4030_shutdown(struct twl4030_usb *twl)
 {
-	struct twl4030_usb *twl = platform_get_drvdata(pdev);
 	int val;
 
 	pm_runtime_get_sync(twl->dev);
 	cancel_delayed_work(&twl->id_workaround_work);
-	device_remove_file(twl->dev, &dev_attr_vbus);
 
 	/* set transceiver mode to power on defaults */
 	twl4030_usb_set_mode(twl, -1);
@@ -779,6 +817,17 @@  static int twl4030_usb_remove(struct platform_device *pdev)
 	return 0;
 }
 
+
+static int twl4030_usb_remove(struct platform_device *pdev)
+{
+	struct twl4030_usb *twl = platform_get_drvdata(pdev);
+
+	device_remove_file(twl->dev, &dev_attr_vbus);
+	device_remove_file(twl->dev, &dev_attr_test);
+	
+	return twl4030_shutdown(twl);
+}
+
 #ifdef CONFIG_OF
 static const struct of_device_id twl4030_usb_id_table[] = {
 	{ .compatible = "ti,twl4030-usb" },