Re: [PATCH v2 4/4] ata: add ata port runtime PM callbacks

[Alan Stern]

On Mon, 14 Nov 2011, Lin Ming wrote:

> Current patches has a bug that system suspend fails if ata port was
> runtime suspended.
> 
> disk suspend issues sync cache and stop device commands that obviously
> need ata port to be active. So we need to runtime resume ata port first.

This is wrong.  If the port is already suspended then so are all the 
drives below the port.  Hence there is no need to sync the cache or 
stop the device.

> Alan, Tejun
> 
> How about below fix?
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index fa3a591..ebb87fbf 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -50,6 +50,7 @@
>  #include <linux/string_helpers.h>
>  #include <linux/async.h>
>  #include <linux/slab.h>
> +#include <linux/pm_runtime.h>
>  #include <asm/uaccess.h>
>  #include <asm/unaligned.h>
>  
> @@ -2762,6 +2763,14 @@ static int sd_suspend(struct device *dev, pm_message_t mesg)
>  	if (!sdkp)
>  		return 0;	/* this can happen */
>  
> +	/*
> +	 * Resume parent device to handle sync cache and
> +	 * stop device commands
> +	 */
> +	ret = pm_runtime_get_sync(dev->parent);
> +	if (ret)
> +		goto exit;
> +
>  	if (sdkp->WCE) {
>  		sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n");
>  		ret = sd_sync_cache(sdkp);

This is not the right approach.  You should look instead at 
scsi_dev_type_suspend() in scsi_pm.c.  If the device is already runtime 
suspended then the routine should return immediately.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/