[PATCH] mmc: sdhci-s3c: Remove old and misprototyped suspend operations

[PATCH] mmc: sdhci-s3c: Remove old and misprototyped suspend operations

[Mark Brown]

Now that the driver is using dev_pm_ops the suspend operations in the
platform_driver structure won't get called so don't need to be there,
and certainly shouldn't be the same function as dev_pm_ops since the
signatures are different.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/mmc/host/sdhci-s3c.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 4cb272c..9a20d1f 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -647,8 +647,6 @@ static const struct dev_pm_ops sdhci_s3c_pmops = {
 static struct platform_driver sdhci_s3c_driver = {
 	.probe		= sdhci_s3c_probe,
 	.remove		= __devexit_p(sdhci_s3c_remove),
-	.suspend	= sdhci_s3c_suspend,
-	.resume	        = sdhci_s3c_resume,
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "s3c-sdhci",
-- 
1.7.7.3

Re: [PATCH] mmc: sdhci-s3c: Remove old and misprototyped suspend operations

[Jaehoon Chung]

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

On 11/22/2011 03:00 AM, Mark Brown wrote:

> Now that the driver is using dev_pm_ops the suspend operations in the
> platform_driver structure won't get called so don't need to be there,
> and certainly shouldn't be the same function as dev_pm_ops since the
> signatures are different.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/mmc/host/sdhci-s3c.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index 4cb272c..9a20d1f 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -647,8 +647,6 @@ static const struct dev_pm_ops sdhci_s3c_pmops = {
>  static struct platform_driver sdhci_s3c_driver = {
>  	.probe		= sdhci_s3c_probe,
>  	.remove		= __devexit_p(sdhci_s3c_remove),
> -	.suspend	= sdhci_s3c_suspend,
> -	.resume	        = sdhci_s3c_resume,
>  	.driver		= {
>  		.owner	= THIS_MODULE,
>  		.name	= "s3c-sdhci",

Re: [PATCH] mmc: sdhci-s3c: Remove old and misprototyped suspend operations

[Chris Ball]

Hi,

On Tue, Nov 29 2011, Jaehoon Chung wrote:
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>
> On 11/22/2011 03:00 AM, Mark Brown wrote:
>
>> Now that the driver is using dev_pm_ops the suspend operations in the
>> platform_driver structure won't get called so don't need to be there,
>> and certainly shouldn't be the same function as dev_pm_ops since the
>> signatures are different.
>> 
>> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>> ---
>>  drivers/mmc/host/sdhci-s3c.c |    2 --
>>  1 files changed, 0 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
>> index 4cb272c..9a20d1f 100644
>> --- a/drivers/mmc/host/sdhci-s3c.c
>> +++ b/drivers/mmc/host/sdhci-s3c.c
>> @@ -647,8 +647,6 @@ static const struct dev_pm_ops sdhci_s3c_pmops = {
>>  static struct platform_driver sdhci_s3c_driver = {
>>  	.probe		= sdhci_s3c_probe,
>>  	.remove		= __devexit_p(sdhci_s3c_remove),
>> -	.suspend	= sdhci_s3c_suspend,
>> -	.resume	        = sdhci_s3c_resume,
>>  	.driver		= {
>>  		.owner	= THIS_MODULE,
>>  		.name	= "s3c-sdhci",

Thanks, pushed to mmc-next for 3.2 with Jaehoon's ACK.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH] mmc: sdhci-s3c: Remove old and misprototyped suspend operations

[Tushar Behera]

Hi,

Chris Ball <cjb <at> laptop.org> writes:

> 
> Hi,
> 
> On Tue, Nov 29 2011, Jaehoon Chung wrote:
> > Acked-by: Jaehoon Chung <jh80.chung <at> samsung.com>
> >
> > On 11/22/2011 03:00 AM, Mark Brown wrote:
> >
> >> Now that the driver is using dev_pm_ops the suspend operations in the
> >> platform_driver structure won't get called so don't need to be there,
> >> and certainly shouldn't be the same function as dev_pm_ops since the
> >> signatures are different.
> >> 
> >> Signed-off-by: Mark Brown <broonie <at> opensource.wolfsonmicro.com>

On Origen board (based on EXYNOS4210), the primary filesystem is on a
SD/MMC card. When tested with v3.2-rc6 kernel, the system doesn't resume
properly.

After resume, it keeps printing following message and the filesystem never
comes up.

mmc0: Timeout waiting for hardware interrupt.

If this patch is reverted, the system is able to mount the filesystem 
successfully.    

Am I missing something?

Re: [PATCH] mmc: sdhci-s3c: Remove old and misprototyped suspend operations

[Chris Ball]

Hi,

On Mon, Dec 19 2011, Tushar Behera wrote:
>> >> Now that the driver is using dev_pm_ops the suspend operations in the
>> >> platform_driver structure won't get called so don't need to be there,
>> >> and certainly shouldn't be the same function as dev_pm_ops since the
>> >> signatures are different.
>> >> 
>> >> Signed-off-by: Mark Brown <broonie <at> opensource.wolfsonmicro.com>
>
> On Origen board (based on EXYNOS4210), the primary filesystem is on a
> SD/MMC card. When tested with v3.2-rc6 kernel, the system doesn't resume
> properly.
>
> After resume, it keeps printing following message and the filesystem never
> comes up.
>
> mmc0: Timeout waiting for hardware interrupt.
>
> If this patch is reverted, the system is able to mount the filesystem 
> successfully.    
>
> Am I missing something?

Mark/Jaehoon?  This looks very bad.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH] mmc: sdhci-s3c: Remove old and misprototyped suspend operations

[Jaehoon Chung]

Hi Tushar.

I also tested this patch with Samsung-SoC.
But i didn't find the below message 
"mmc0: Timeout waiting for hardware interrupt."

I didn't understand what is that differ with previously code?
(just using dev_pm_ops..)

Thanks,
Jaehoon Chung

On 12/25/2011 11:29 AM, Chris Ball wrote:

> Hi,
> 
> On Mon, Dec 19 2011, Tushar Behera wrote:
>>>>> Now that the driver is using dev_pm_ops the suspend operations in the
>>>>> platform_driver structure won't get called so don't need to be there,
>>>>> and certainly shouldn't be the same function as dev_pm_ops since the
>>>>> signatures are different.
>>>>>
>>>>> Signed-off-by: Mark Brown <broonie <at> opensource.wolfsonmicro.com>
>>
>> On Origen board (based on EXYNOS4210), the primary filesystem is on a
>> SD/MMC card. When tested with v3.2-rc6 kernel, the system doesn't resume
>> properly.
>>
>> After resume, it keeps printing following message and the filesystem never
>> comes up.
>>
>> mmc0: Timeout waiting for hardware interrupt.
>>
>> If this patch is reverted, the system is able to mount the filesystem 
>> successfully.    
>>
>> Am I missing something?
> 
> Mark/Jaehoon?  This looks very bad.

> 
> - Chris.

Re: [PATCH] mmc: sdhci-s3c: Remove old and misprototyped suspend operations

[Tushar Behera]

Hi Jaehoon,

On 12/26/2011 07:24 AM, Jaehoon Chung wrote:
> Hi Tushar.
>
> I also tested this patch with Samsung-SoC.
> But i didn't find the below message
> "mmc0: Timeout waiting for hardware interrupt."
>
When the primary filesystem is on SD/MMC card, I need to select 
following config options for the system to go to sleep. Is it in anyway 
the reason of conflict?

CONFIG_MMC_UNSAFE_RESUME=y

I modified my test-case to use a ramdisk instead and use a script to 
mount/unmount the filesystem on MMC card. In that case also I get the 
above error for some time - around 30s. After that, it behaves normally 
(mounting/unmounting etc.)

$ echo mem > /sys/power/state
[   18.825000] PM: Syncing filesystems ... done.
[   18.830000] Freezing user space processes ... (elapsed 0.00 seconds) 
done.
[   18.835000] Freezing remaining freezable tasks ... (elapsed 0.01 
seconds) done.
[   18.855000] Suspending console(s) (use no_console_suspend to debug)
[   18.865000] wake enabled for irq 292
[   18.865000] wake enabled for irq 301
[   18.865000] wake enabled for irq 302
[   18.865000] wake enabled for irq 303
[   18.865000] wake enabled for irq 304
[   18.865000] wake enabled for irq 305
[   18.890000] PM: suspend of devices complete after 26.491 msecs
[   18.890000] PM: late suspend of devices complete after 0.829 msecs
[   18.890000] Disabling non-boot CPUs ...
[   18.895000] IRQ80 no longer affine to CPU1
[   18.895000] CPU1: shutdown
[   18.895000] Enabling non-boot CPUs ...
[   18.905000] CPU1: Booted secondary processor
[   18.905000] Calibrating delay loop (skipped) already calibrated this CPU
[   18.905000] CPU1: Unknown IPI message 0x1
[   18.905000] CPU1 is up
[   18.905000] PM: early resume of devices complete after 0.745 msecs
[   18.910000] s3c-i2c s3c2440-i2c.0: slave address 0x10
[   18.910000] s3c-i2c s3c2440-i2c.0: bus frequency set to 97 KHz
[   18.910000] usb usb1: root hub lost power or was reset
[   18.935000] wake disabled for irq 301
[   18.935000] wake disabled for irq 302
[   18.935000] wake disabled for irq 303
[   18.935000] wake disabled for irq 304
[   18.935000] wake disabled for irq 305
[   18.935000] wake disabled for irq 292
[   19.070000] PM: resume of devices complete after 160.084 msecs
[   19.180000] Restarting tasks ... done.
/ $ [   29.200000] mmc0: Timeout waiting for hardware interrupt.
[   39.220000] mmc0: Timeout waiting for hardware interrupt.
[   49.240000] mmc0: Timeout waiting for hardware interrupt.
[   59.260000] mmc0: Timeout waiting for hardware interrupt.
[   59.260000] mmc0: card e624 removed
[   59.330000] mmc0: new SDHC card at address e624
[   59.335000] mmcblk0: mmc0:e624 SD04G 3.69 GiB
[   59.340000]  mmcblk0: p1

> I didn't understand what is that differ with previously code?
> (just using dev_pm_ops..)
>

I could not find any dev_pm_ops related implementation in sdhci-s3c.c. 
Are there some other patches that I am missing on v3.2-rc6? Or any 
specific config settings that I need for this to be working?

> Thanks,
> Jaehoon Chung
>
> On 12/25/2011 11:29 AM, Chris Ball wrote:
>
>> Hi,
>>
>> On Mon, Dec 19 2011, Tushar Behera wrote:
>>>>>> Now that the driver is using dev_pm_ops the suspend operations in the
>>>>>> platform_driver structure won't get called so don't need to be there,
>>>>>> and certainly shouldn't be the same function as dev_pm_ops since the
>>>>>> signatures are different.
>>>>>>
>>>>>> Signed-off-by: Mark Brown<broonie<at>  opensource.wolfsonmicro.com>
>>>
>>> On Origen board (based on EXYNOS4210), the primary filesystem is on a
>>> SD/MMC card. When tested with v3.2-rc6 kernel, the system doesn't resume
>>> properly.
>>>
>>> After resume, it keeps printing following message and the filesystem never
>>> comes up.
>>>
>>> mmc0: Timeout waiting for hardware interrupt.
>>>
>>> If this patch is reverted, the system is able to mount the filesystem
>>> successfully.
>>>
>>> Am I missing something?
>>
>> Mark/Jaehoon?  This looks very bad.
>
>>
>> - Chris.
>
>


-- 
Tushar Behera

Re: [PATCH] mmc: sdhci-s3c: Remove old and misprototyped suspend operations

[Jaehoon Chung]

Hi Tushar.

Did you test with linux-3.2-rc6?
I checked the linux-3.2-rc6..that version may be missed the dev_pm_ops patch..
(Manuel's patch [PATCH v4] mmc: sdhci: remove "state" argument from sdhci_suspend_host)

I didn't understand this condition.

That patch is include in linux-3.2-rc7.

To Chris.
If adopt "sdhci-s3c: remove Old and misprototyped suspend operation"
I think that Manuel's patch must be include in liux-3.2-rc6 

Thanks,
Jaehoon Chung

On 12/26/2011 02:05 PM, Tushar Behera wrote:

> Hi Jaehoon,
> 
> On 12/26/2011 07:24 AM, Jaehoon Chung wrote:
>> Hi Tushar.
>>
>> I also tested this patch with Samsung-SoC.
>> But i didn't find the below message
>> "mmc0: Timeout waiting for hardware interrupt."
>>
> When the primary filesystem is on SD/MMC card, I need to select following config options for the system to go to sleep. Is it in anyway the reason of conflict?
> 
> CONFIG_MMC_UNSAFE_RESUME=y
> 
> I modified my test-case to use a ramdisk instead and use a script to mount/unmount the filesystem on MMC card. In that case also I get the above error for some time - around 30s. After that, it behaves normally (mounting/unmounting etc.)
> 
> $ echo mem > /sys/power/state
> [   18.825000] PM: Syncing filesystems ... done.
> [   18.830000] Freezing user space processes ... (elapsed 0.00 seconds) done.
> [   18.835000] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
> [   18.855000] Suspending console(s) (use no_console_suspend to debug)
> [   18.865000] wake enabled for irq 292
> [   18.865000] wake enabled for irq 301
> [   18.865000] wake enabled for irq 302
> [   18.865000] wake enabled for irq 303
> [   18.865000] wake enabled for irq 304
> [   18.865000] wake enabled for irq 305
> [   18.890000] PM: suspend of devices complete after 26.491 msecs
> [   18.890000] PM: late suspend of devices complete after 0.829 msecs
> [   18.890000] Disabling non-boot CPUs ...
> [   18.895000] IRQ80 no longer affine to CPU1
> [   18.895000] CPU1: shutdown
> [   18.895000] Enabling non-boot CPUs ...
> [   18.905000] CPU1: Booted secondary processor
> [   18.905000] Calibrating delay loop (skipped) already calibrated this CPU
> [   18.905000] CPU1: Unknown IPI message 0x1
> [   18.905000] CPU1 is up
> [   18.905000] PM: early resume of devices complete after 0.745 msecs
> [   18.910000] s3c-i2c s3c2440-i2c.0: slave address 0x10
> [   18.910000] s3c-i2c s3c2440-i2c.0: bus frequency set to 97 KHz
> [   18.910000] usb usb1: root hub lost power or was reset
> [   18.935000] wake disabled for irq 301
> [   18.935000] wake disabled for irq 302
> [   18.935000] wake disabled for irq 303
> [   18.935000] wake disabled for irq 304
> [   18.935000] wake disabled for irq 305
> [   18.935000] wake disabled for irq 292
> [   19.070000] PM: resume of devices complete after 160.084 msecs
> [   19.180000] Restarting tasks ... done.
> / $ [   29.200000] mmc0: Timeout waiting for hardware interrupt.
> [   39.220000] mmc0: Timeout waiting for hardware interrupt.
> [   49.240000] mmc0: Timeout waiting for hardware interrupt.
> [   59.260000] mmc0: Timeout waiting for hardware interrupt.
> [   59.260000] mmc0: card e624 removed
> [   59.330000] mmc0: new SDHC card at address e624
> [   59.335000] mmcblk0: mmc0:e624 SD04G 3.69 GiB
> [   59.340000]  mmcblk0: p1
> 
>> I didn't understand what is that differ with previously code?
>> (just using dev_pm_ops..)
>>
> 
> I could not find any dev_pm_ops related implementation in sdhci-s3c.c. Are there some other patches that I am missing on v3.2-rc6? Or any specific config settings that I need for this to be working?
> 
>> Thanks,
>> Jaehoon Chung
>>
>> On 12/25/2011 11:29 AM, Chris Ball wrote:
>>
>>> Hi,
>>>
>>> On Mon, Dec 19 2011, Tushar Behera wrote:
>>>>>>> Now that the driver is using dev_pm_ops the suspend operations in the
>>>>>>> platform_driver structure won't get called so don't need to be there,
>>>>>>> and certainly shouldn't be the same function as dev_pm_ops since the
>>>>>>> signatures are different.
>>>>>>>
>>>>>>> Signed-off-by: Mark Brown<broonie<at>  opensource.wolfsonmicro.com>
>>>>
>>>> On Origen board (based on EXYNOS4210), the primary filesystem is on a
>>>> SD/MMC card. When tested with v3.2-rc6 kernel, the system doesn't resume
>>>> properly.
>>>>
>>>> After resume, it keeps printing following message and the filesystem never
>>>> comes up.
>>>>
>>>> mmc0: Timeout waiting for hardware interrupt.
>>>>
>>>> If this patch is reverted, the system is able to mount the filesystem
>>>> successfully.
>>>>
>>>> Am I missing something?
>>>
>>> Mark/Jaehoon?  This looks very bad.
>>
>>>
>>> - Chris.
>>
>>
> 
> 

Re: [PATCH] mmc: sdhci-s3c: Remove old and misprototyped suspend operations

[Tushar Behera]

Hi Jaehoon,

On 12/26/2011 10:53 AM, Jaehoon Chung wrote:
> Hi Tushar.
>
> Did you test with linux-3.2-rc6?
> I checked the linux-3.2-rc6..that version may be missed the dev_pm_ops patch..
> (Manuel's patch [PATCH v4] mmc: sdhci: remove "state" argument from sdhci_suspend_host)
>
> I didn't understand this condition.
>
> That patch is include in linux-3.2-rc7.
>

Thanks for the information. I ran the tests with linux-3.2-rc7 and 
everything seems great.

> To Chris.
> If adopt "sdhci-s3c: remove Old and misprototyped suspend operation"
> I think that Manuel's patch must be include in liux-3.2-rc6
>
> Thanks,
> Jaehoon Chung
>

-- 
Tushar Behera