pxa/hx4700: Fix basic suspend/resume
Subsystems (6)
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Basic suspend/resume is fixed by ensuring that the PGSR registers are set correctly before sleep mode is entered. In particular four of the active low resets need to be driven high while in sleep mode, otherwise the unit resets itself instead of suspending. Another problem was that the PCFR_GPROD bit is apparently set by the HTC bootloader; this caused GPIO reset (i.e. the reset button) to fail immediately after returning from sleep mode.
Driver-specific bugs still remain. In particular the mmc driver does not resume properly after returning from sleep mode, thus preventing sleep mode from being entered a second time.
---
arch/arm/mach-pxa/hx4700.c +12 -1
@@ -120,7 +120,11 @@ static unsigned long hx4700_pin_config[]GPIO19_SSP2_SCLK ,GPIO86_SSP2_RXD ,GPIO87_SSP2_TXD , -GPIO88_GPIO , +MFP_CFG_OUT (GPIO88 ,AF0 ,KEEP_OUTPUT ),/* TSC2046_CS */ + +/* BQ24022 Regulator */ +MFP_CFG_OUT (GPIO72 ,AF0 ,KEEP_OUTPUT ),/* BQ24022_nCHARGE_EN */ +MFP_CFG_OUT (GPIO96 ,AF0 ,KEEP_OUTPUT ),/* BQ24022_ISET2 */ /* HX4700 specific input GPIOs */ GPIO12_GPIO, /* ASIC3_IRQ */
@@ -133,6 +137,11 @@ static unsigned long hx4700_pin_config[]GPIO108_GPIO ,/* GSM_READY */ GPIO58_GPIO ,/* TSC2046_nPENIRQ */ GPIO66_GPIO ,/* nSDIO_IRQ */ + +MFP_CFG_OUT (GPIO61 ,AF0 ,DRIVE_HIGH ),/* W3220_nRESET */ +MFP_CFG_OUT (GPIO71 ,AF0 ,DRIVE_HIGH ),/* ASIC3_nRESET */ +MFP_CFG_OUT (GPIO81 ,AF0 ,DRIVE_HIGH ),/* CPU_GP_nRESET */ + MFP_CFG_OUT(GPIO116, AF0, DRIVE_HIGH), /* CPU_HW_nRESET */ };#define HX4700_GPIO_IN (num, _desc) \
@@ -847,6 +872,8 @@ static struct gpio_ress global_gpios[] =static void __init hx4700_init (void ) { +PCFR &= ~PCFR_GPROD ;/* Else GPIO reset fails after sleep mode */ +pxa2xx_mfp_config (ARRAY_AND_SIZE (hx4700_pin_config));hx4700_gpio_request (ARRAY_AND_SIZE (global_gpios));
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Quoted 2 lines · Paul Parsons
Driver-specific bugs still remain. In particular the mmc driver does not resume properly after returning from sleep mode, thus preventing sleep mode from being entered a second time.
Sorry, no comments about your patch, just wanted to say about mmc driver. Some time ago when I tried to look at suspend on hx4700, this patch helped me to survive suspend-wakeup cycle with root on MMC:
drivers/mfd/asic3.c +2 -0
@@ -895,6 +895,8 @@ static struct mfd_cell asic3_cell_mmc = { .name = "tmio-mmc", .enable = asic3_mmc_enable, .disable = asic3_mmc_disable, + .suspend = asic3_mmc_disable, + .resume = asic3_mmc_enable, .driver_data = &asic3_mmc_data, .num_resources = ARRAY_SIZE(asic3_mmc_resources), .resources = asic3_mmc_resources,
That was around 2.6.37. Not sure if it still helps and I have no possibility to check - tmio_mmc on ASIC3 seems to be totally borken in 2.6.39-rcX - it doesn't even detect card on my hx4700 :( And that's pity, as I mainly use SD for rootfs.
Signature · 2 lines
Best regards, Dmitry "MAD" Artamonow
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Hi Dmitry,
That patch found its way into 2.6.39-rc7 and earlier so I'm already using it. Thanks anyway.
As for the tmio_mmc driver, it more or less works for me in 2.6.39-rc7, though it does produce a lot of "tmio_mmc: Spurious irq" messages. My SD card is detected OK and reading / writing it works fine. I'll try to look into both the spurious IRQ problem and the suspend / resume bug(s).
mmc0 at 0xc489c800 irq 130
...
mmc0: new SD card at address 1234
mmcblk0: mmc0:1234 SD01G 972 MiBtmio_mmc: Spurious irq, disabling! 0x00800780 0x833f0304 0x00000000
mmcblk0: p1tmio_mmc: Spurious irq, disabling! 0x00800780 0x833f0304 0x00000000
...
Regards,
Paul
Quoted 54 lines · Dmitry Artamonow
From: Dmitry Artamonow <[email protected]>
Subject: Re: [PATCH] pxa/hx4700: Fix basic suspend/resume
To: "Paul Parsons" <[email protected]>
Cc: eric.y.miao at gmail.com, philipp.zabel at gmail.com, linux-arm-kernel at lists.infradead.org
Date: Friday, 13 May, 2011, 11:32Quoted 2 lines · Paul Parsons
Driver-specific bugs still remain. In particular the
mmc driver does not resume properly after returning from sleep mode, thus preventing sleep mode from being entered a second time.
Sorry, no comments about your patch, just wanted to say about mmc
driver. Some time ago when I tried to look at suspend on hx4700,
this patch helped me to survive suspend-wakeup cycle with root on MMC:drivers/mfd/asic3.c +0
-0@@ -895,6 +895,8 @@ static struct mfd_cell asic3_cell_mmc = { ??? .name? ? ? ? ? = "tmio-mmc", ??? .enable? ? ? ? = asic3_mmc_enable, ??? .disable? ????=
asic3_mmc_disable,
+??? .suspend??? =asic3_mmc_disable,
+??? .resume???
??? =asic3_mmc_enable,
??? .driver_data???=&asic3_mmc_data,
??? .num_resources =ARRAY_SIZE(asic3_mmc_resources),
??? .resources? ???=asic3_mmc_resources,That was around 2.6.37. Not sure if it still helps and I have no possibility
to check -tmio_mmcon ASIC3 seems to be totally borken in 2.6.39-rcX - it
doesn't even detect card on my hx4700 :( And that's pity, as I mainly use
SD for rootfs.Signature · 2 lines
Best regards, Dmitry "MAD" Artamonow
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Quoted 4 lines · Paul Parsons
Hi Dmitry,
That patch found its way into 2.6.39-rc7 and earlier so I'm already using it. Thanks anyway.
Erm, do you have some special 2.6.39-rc7? :) I can't see in mine any .suspend/.resume callbacks defined in asic3.c for tmio_mmc cell. Or have you meant some other changes? BTW, I quickly looked at tmio_mmc.c and it seems very suspicious that it calls cell->disable(dev) on suspend, but cell->resume(dev) on resume. As I said earlier, .resume isn't defined in asic3.c for tmio_mmc cell - so probably that's the culprit.
Quoted 4 lines
As for the
tmio_mmcdriver, it more or less works for me in 2.6.39-rc7, though it does produce a lot of "tmio_mmc: Spurious irq" messages. My SD card is detected OK and reading / writing it works fine. I'll try to look into both the spurious IRQ problem and the suspend / resume bug(s).
Ah, so mmc works for you in 2.6.39-rc7? Nice to hear! Wonder why it doesn't work for me. I suspect this may be connected to the fact that I boot from WinCE using HaRET - probably Windows leaves controller in some inconsistent state and driver doesn't reset hardware properly.
As for "Spurious irq, disabling!" messages, I'm seeing them for a long time - since pr_debug was turned into pr_warning in commit 311f3ac768 ("mmc: add DMA support to tmio_mmc driver, when used on SuperH") So I suspect this issue (if it's issue at all) has always been there, just unseen before that commit. Not sure what causes those spurious interrupts - maybe it's some specific of TMIO hardware in ASIC3?
Signature · 2 lines
Best regards, Dmitry "MAD" Artamonow
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Hi Dmitry,
Quoted 4 lines
Erm, do you have some special 2.6.39-rc7? :) I can't see in mine
any .suspend/.resume callbacks defined inasic3.cfortmio_mmccell.
Now neither can I. Strange. Maybe I noticed that the functions asic3_mmc_enable and asic3_mmc_disable were already in the table and looked no further. Or maybe I applied the patch and forgot, or thought it had failed. Whatever the reason, I just applied it for real and it works; mmc now behaves itself after suspend/resume. Someone needs to submit that patch.
Quoted 8 lines
Ah, so mmc works for you in 2.6.39-rc7?? Nice to hear! Wonder why it
doesn't work for me. I suspect this may be connected to the fact that
I boot from WinCE using HaRET - probably Windows leaves controller in
some inconsistent state and driver doesn't reset hardware properly.
Maybe. I'm using the SDG bootloader v1.2.5 and have long since dumped WinCE. In fact I patched the bootloader to change the flash partitions and free up the 0.5Mb of flash (between 0.5Mb and 1.0Mb) used for the WinCE registry or something. Thus I can no longer use WinCE even if I wanted to.
Quoted 11 lines
As for "Spurious irq, disabling!" messages, I'm seeing them for a long
time - sincepr_debugwas turned intopr_warningin commit311f3ac768
("mmc: add DMA support totmio_mmcdriver, when used on SuperH")
So I suspect this issue (if it's issue at all) has always been there, just
unseen before that commit. Not sure what causes those spurious interrupts -
maybe it's some specific of TMIO hardware in ASIC3?
Today I found what causes the spurious interrupts. It's a race condition caused by the interrupt handler looping in a while loop.
At the end of a multiple read operation the handler clears the final RXRDY status bit in one iteration and then clears the DATAEND status bit in the next iteration. However the DATAEND interrupt is still queued in the system somewhere and can't be delivered until the handler has returned. So whether the handler detects spurious interrupts depends on how quickly the DATAEND follows the final RXRDY. Similarly for multiple writes.
Try this quick hack:
drivers/mmc/host/tmio_mmc_pio.c +1 -1
@@ -603,7 +603,7 @@ static irqreturn_t tmio_mmc_irq(int irq, goto out; } -while (ireg) { +if (ireg) {/* Card insert / remove attempts */ if (ireg & (TMIO_STAT_CARD_INSERT |TMIO_STAT_CARD_REMOVE )) { tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_CARD_INSERT |
Regards,
Paul
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Quoted 10 lines · Paul Parsons
Today I found what causes the spurious interrupts. It's a race condition caused by the interrupt handler looping in a while loop.
At the end of a multiple read operation the handler clears the final RXRDY status bit in one iteration and then clears the DATAEND status bit in the next iteration. However the DATAEND interrupt is still queued in the system somewhere and can't be delivered until the handler has returned. So whether the handler detects spurious interrupts depends on how quickly the DATAEND follows the final RXRDY. Similarly for multiple writes.
Try this quick hack:
drivers/mmc/host/tmio_mmc_pio.c static irqreturn_t tmio_mmc_irq(int irq, · line 609
Quoted 8 lines
drivers/mmc/host/tmio_mmc_pio.c +1
-1@@ -603,7 +603,7 @@ static irqreturn_t tmio_mmc_irq(int irq, goto out; } -while (ireg) { +if (ireg) {/* Card insert / remove attempts */ if (ireg & (TMIO_STAT_CARD_INSERT |TMIO_STAT_CARD_REMOVE )) { tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_CARD_INSERT |
Well, it really seems to make all those "Spurious IRQ" messages disappear. (and btw, MMC magically started to work for me in -rc7. Probably that were some earlier -rc's that didn't work and issue was fixed in -rc7. Or maybe I'm just lucky today)
I think it's worth to submit it to tmio_mmc maintainers along with the description of this situation on ASIC3. Even if it won't be accepted, it'll make them aware of the problem and maybe some clean solution could be worked out then.
Signature · 2 lines
Best regards, Dmitry "MAD" Artamonow
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Hi Dmitry,
Quoted 7 lines
Well, it really seems to make all those "Spurious IRQ" messages disappear.
(and btw, MMC magically started to work for me in -rc7. Probably that were
some earlier -rc's that didn't work and issue was fixed in -rc7. Or maybe
I'm just lucky today)
That's good news.
Quoted 7 lines
I think it's worth to submit it to
tmio_mmcmaintainers along with the
description of this situation on ASIC3. Even if it won't be accepted,
it'll make them aware of the problem and maybe some clean solution could
be worked out then.
I have just now submitted a patch to linux-mmc.
Regards,
Paul
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
In reply to Paul ParsonsAm Mittwoch, den 11.05.2011, 19:49 +0000 schrieb Paul Parsons:
Quoted 9 lines
Basic suspend/resume is fixed by ensuring that the PGSR registers are set correctly before sleep mode is entered. In particular four of the active low resets need to be driven high while in sleep mode, otherwise the unit resets itself instead of suspending. Another problem was that the
PCFR_GPRODbit is apparently set by the HTC bootloader; this caused GPIO reset (i.e. the reset button) to fail immediately after returning from sleep mode.Driver-specific bugs still remain. In particular the mmc driver does not resume properly after returning from sleep mode, thus preventing sleep mode from being entered a second time.
Signed-off-by Paul Parsons <[email protected]>---
arch/arm/mach-pxa/hx4700.c static unsigned long hx4700_pin_config[] · line 123
Quoted 5 lines
arch/arm/mach-pxa/hx4700.c +1
-1@@ -120,7 +120,11 @@ static unsigned long hx4700_pin_config[]GPIO19_SSP2_SCLK ,GPIO86_SSP2_RXD ,GPIO87_SSP2_TXD , - GPIO88_GPIO, + MFP_CFG_OUT(GPIO88, AF0, KEEP_OUTPUT), /* TSC2046_CS */
What you want here is to keep the SPI chip select deasserted (high) during suspend, so I think that should be DRIVE_HIGH instead.
Otherwise,
regards
Philipp
Haojian Zhuang
maintainerPATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Quoted 19 lines · Philipp Zabel
Am Mittwoch, den 11.05.2011, 19:49 +0000 schrieb Paul Parsons:
Quoted 9 lines
Basic suspend/resume is fixed by ensuring that the PGSR registers are set correctly before sleep mode is entered. In particular four of the active low resets need to be driven high while in sleep mode, otherwise the unit resets itself instead of suspending. Another problem was that the
PCFR_GPRODbit is apparently set by the HTC bootloader; this caused GPIO reset (i.e. the reset button) to fail immediately after returning from sleep mode.Driver-specific bugs still remain. In particular the mmc driver does not resume properly after returning from sleep mode, thus preventing sleep mode from being entered a second time.
Signed-off-by Paul Parsons <[email protected]>---
arch/arm/mach-pxa/hx4700.c ? ? ? 2011-05-11 20:21:54.026109594 +0100 static unsigned long hx4700_pin_config[] · line 123
Quoted 5 lines
arch/arm/mach-pxa/hx4700.c ? ? ? 2011-05-11 20:21:54.026109594 +0100 renamed from arch/arm/mach-pxa/hx4700.c ? ? ? 2011-05-11 00:54:10.787247169 +0100 +1
-1@@ -120,7 +120,11 @@ static unsigned long hx4700_pin_config[] ? ? ? GPIO19_SSP2_SCLK, ? ? ? GPIO86_SSP2_RXD, ? ? ? GPIO87_SSP2_TXD, - ? ? GPIO88_GPIO, + ? ? MFP_CFG_OUT(GPIO88, AF0, KEEP_OUTPUT), ?/* TSC2046_CS */What you want here is to keep the SPI chip select deasserted (high) during suspend, so I think that should be
DRIVE_HIGHinstead.
Quoted 3 lines
Otherwise,
Acked-by Philipp Zabel <[email protected]>
This patch keeps a lot of devices in power-on mode (not reset mode). Is it necessary?
Thanks
Haojian
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Hello Haojian,
Quoted 11 lines
Quoted 1 line
What you want here is to keep the SPI chip select
deasserted (high)
Quoted 1 line
during suspend, so I think that should be
DRIVE_HIGHinstead.
Quoted 4 lines
Otherwise,
Acked-by Philipp Zabel <[email protected]>This patch keeps a lot of devices in power-on mode (not reset mode).
Is it necessary?
No, of course not. This patch is just a first step to get hx4700 suspend mode working; currently it does not work at all.
The suspend mode disposition of devices and GPIOs still needs to be reviewed at some point. For example, someone else discovered that the two Bluetooth outputs (GPIO43, GPIO45) account for approximately one third of the suspend mode power consumption! Correcting that will be my next patch.
Regards,
Paul
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Quoted 14 lines · Paul Parsons
Hello Haojian,
Quoted 11 lines
Quoted 1 line
What you want here is to keep the SPI chip select
deasserted (high)
Quoted 1 line
during suspend, so I think that should be
DRIVE_HIGHinstead.
Quoted 4 lines
Otherwise,
Acked-by Philipp Zabel <[email protected]>This patch keeps a lot of devices in power-on mode (not reset mode).
Is it necessary?
Quoted 8 lines
No, of course not. This patch is just a first step to get hx4700 suspend mode working; currently it does not work at all.
The suspend mode disposition of devices and GPIOs still needs to be reviewed at some point. For example, someone else discovered that the two Bluetooth outputs (GPIO43, GPIO45) account for approximately one third of the suspend mode power consumption! Correcting that will be my next patch.
Applied.
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
In reply to Philipp ZabelQuoted 5 lines · Philipp Zabel
Am Mittwoch, den 11.05.2011, 19:49 +0000 schrieb Paul Parsons:
Quoted 4 lines
Basic suspend/resume is fixed by ensuring that the PGSR registers are set correctly before sleep mode is entered. In particular four of the active low resets need to be driven high while in sleep mode, otherwise the unit resets itself instead of suspending. Another problem was that the
PCFR_GPRODbit is apparently set by the HTC bootloader; this caused GPIO reset (i.e. the reset button) to fail immediately after returning from sleep mode.Driver-specific bugs still remain. In particular the mmc driver does not resume properly after returning from sleep mode, thus preventing sleep mode from being entered a second time.
Quoted 13 lines
arch/arm/mach-pxa/hx4700.c static unsigned long hx4700_pin_config[] · line 123
Quoted 10 lines
Signed-off-by Paul Parsons <[email protected]>---
arch/arm/mach-pxa/hx4700.c +1
-1@@ -120,7 +120,11 @@ static unsigned long hx4700_pin_config[]GPIO19_SSP2_SCLK ,GPIO86_SSP2_RXD ,GPIO87_SSP2_TXD , - GPIO88_GPIO, + MFP_CFG_OUT(GPIO88, AF0, KEEP_OUTPUT), /* TSC2046_CS */What you want here is to keep the SPI chip select deasserted (high) during suspend, so I think that should be
DRIVE_HIGHinstead.
And I don't think we allowed folk to use MFP_CFG_xxx() macros in their board support code. Is there something wrong with:
GPIO88_GPIO | MFP_LPM_KEEP_OUTPUT
?
Haojian Zhuang
maintainerPATCH 1/1 pxa/hx4700: Fix basic suspend/resume
Quoted 20 lines · Russell King - ARM Linux
Quoted 5 lines · Philipp Zabel
Am Mittwoch, den 11.05.2011, 19:49 +0000 schrieb Paul Parsons:
Quoted 4 lines
Basic suspend/resume is fixed by ensuring that the PGSR registers are set correctly before sleep mode is entered. In particular four of the active low resets need to be driven high while in sleep mode, otherwise the unit resets itself instead of suspending. Another problem was that the
PCFR_GPRODbit is apparently set by the HTC bootloader; this caused GPIO reset (i.e. the reset button) to fail immediately after returning from sleep mode.Driver-specific bugs still remain. In particular the mmc driver does not resume properly after returning from sleep mode, thus preventing sleep mode from being entered a second time.
Quoted 13 lines
arch/arm/mach-pxa/hx4700.c ? ? 2011-05-11 20:21:54.026109594 +0100 static unsigned long hx4700_pin_config[] · line 123
Quoted 10 lines
Signed-off-by Paul Parsons <[email protected]>---
arch/arm/mach-pxa/hx4700.c ? ? 2011-05-11 20:21:54.026109594 +0100 renamed from arch/arm/mach-pxa/hx4700.c ? ? 2011-05-11 00:54:10.787247169 +0100 +1
-1@@ -120,7 +120,11 @@ static unsigned long hx4700_pin_config[] ? ? GPIO19_SSP2_SCLK, ? ? GPIO86_SSP2_RXD, ? ? GPIO87_SSP2_TXD, - ? GPIO88_GPIO, + ? MFP_CFG_OUT(GPIO88, AF0, KEEP_OUTPUT), ?/* TSC2046_CS */What you want here is to keep the SPI chip select deasserted (high) during suspend, so I think that should be
DRIVE_HIGHinstead.
Quoted 5 lines
And I don't think we allowed folk to use
MFP_CFG_xxx()macros in their board support code. ?Is there something wrong with:? ? ? ?GPIO88_GPIO |
MFP_LPM_KEEP_OUTPUT
Yes, it should be MFPR_LPM_DRIVE_HIGH or MFPR_LPM_PULL_HIGH. Thanks for your reminder.
Paul,
Could you help to format the patch again?
Best Regards
Haojian
PATCH 1/1 pxa/hx4700: Fix basic suspend/resume
In reply to Russell King - ARM LinuxHello Russell,
Quoted 19 lines
clean-2.6.39-rc7/arch/arm/mach-pxa/hx4700.c???2011-05-11 00:54:10.787247169 +0100Quoted 1 line
Quoted 1 line
+++
linux-2.6.39-rc7/arch/arm/mach-pxa/hx4700.c???2011-05-11 20:21:54.026109594 +0100Quoted 1 line
Review static unsigned long
Quoted 1 line
(quoted diff) +0
-0@@ -120,7 +120,11 @@ static unsigned longhx4700_pin_config[]
Quoted 5 lines
Review static unsigned long · line 123
Quoted 5 lines
(quoted diff) +1
-1? ??? GPIO19_SSP2_SCLK, ? ??? GPIO86_SSP2_RXD, ? ??? GPIO87_SSP2_TXD, -??? GPIO88_GPIO, +??? MFP_CFG_OUT(GPIO88, AF0,
KEEP_OUTPUT),??? /*TSC2046_CS*/Quoted 2 lines
What you want here is to keep the SPI chip select
deasserted (high)
Quoted 1 line
during suspend, so I think that should be
DRIVE_HIGHinstead.
Quoted 7 lines
And I don't think we allowed folk to use
MFP_CFG_xxx()macros in their
board support code.? Is there something wrong with:???
GPIO88_GPIO|MFP_LPM_KEEP_OUTPUT?
Yes there is; the reason for using MFP_CFG_OUT() was to set the direction of GPIO88 to output.
Currently, all of the GPIO88 definitions in <mach/mfp-pxa27x.h> are inputs, whereas the hx4700 uses it as an output. I could have added a new GPIO88 definition to <mach/mfp-pxa27x.h> to support the hx4700 only, but - based on an earlier experience - I thought that would have been frowned on.
Presumably if MFP_CFG_OUT() is not allowed to be used in board support code then neither is MFP_DIR_OUT, which is defined in the same file.
Is there another compile-time solution? Or should I just add an hx4700-specific definition of GPIO88 to <mach/mfp-pxa27x.h> ?
Regards,
Paul