Re: [PATCH v2 1/1] scsi: Synchronize request queue PM status only on successful resume

2019-01-03 Thread Mika Westerberg
Hi,

On Thu, Jan 03, 2019 at 02:53:15PM +0800, stanley@mediatek.com wrote:
> From: Stanley Chu 
> 
> The commit 356fd2663cff ("scsi: Set request queue runtime PM status
> back to active on resume") fixed up the inconsistent RPM status between
> request queue and device. However changing request queue RPM status
> shall be done only on successful resume, otherwise status may be still
> inconsistent as below,
> 
> Request queue: RPM_ACTIVE
> Device: RPM_SUSPENDED
> 
> This ends up soft lockup because requests can be submitted to
> underlying devices but those devices and their required resource
> are not resumed.

It would be good to add some example of the soft lockup you are seeing
here.

> Fixes: 356fd2663cff ("scsi: Set request queue runtime PM status
> back to active on resume")

You don't need to wrap this.

The change itself looks fine.


Re: [PATCH v2 3/3] scsi: allow scsi devices to use direct complete

2016-02-29 Thread Mika Westerberg
On Sat, Feb 27, 2016 at 01:07:06AM -0800, dbasehore . wrote:
> That's an interesting question. Part of direct complete is to leave
> the device runtime suspended even after the system resumes if
> possible. The comments in pm_complete_with_resume_check indicate that
> the firmware may resume a device. I could see this happening with some
> kind of SCSI device.
> 
> If this is possible, are we able to put the device back into a
> consistent state (runtime suspended) by calling suspend for the scsi
> device? If not, we might need to use pm_complete_with_resume_check for
> the complete callback.

To me the latter option sounds safer.

I tried this series (as I'm interested in AHCI host controller runtime
PM) and noticed possible issue.

I have following two additional patches from linux-next applied to get
system resume working (otherwise resume is never finished):

  d07ab6d11477 block: Add blk_set_runtime_active()
  356fd2663cff scsi: Set request queue runtime PM status back to active on 
resume

Then I do this:

  # echo auto > /sys/block/sda/device/power/control
  # echo 5000 > /sys/block/sda/device/power/autosuspend_delay_ms

  # echo auto > /sys/bus/pci/devices/\:00\:12.0/ata1/power/control

The last line unblocks runtime PM from the parent device.

After 5 seconds of idle I see that the disk gets runtime suspended:

  [  566.858971] sd 0:0:0:0: [sda] Synchronizing SCSI cache
  [  567.049288] sd 0:0:0:0: [sda] Stopping disk

Now suspend the machine:

  # rtcwake -s10 -mmem

Once the system resumes I get this:

  [  668.879149] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
  [  668.927319] ata1.00: configured for UDMA/133
  [  669.392246] ata1: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
  [  669.440427] ata1.00: configured for UDMA/133
  [  669.445352] sd 0:0:0:0: [sda] Starting disk
  [  669.464647] sda: detected capacity change from 0 to 240057409536
  [  669.482596] sda: detected capacity change from 0 to 240057409536

It looks like we get resumed two times. If I have this patch reverted I
can see it happening only once. Also with the patch applied and if the
parent device has runtime PM blocked, it happens only once.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 3/3] scsi: allow scsi devices to use direct complete

2016-02-27 Thread Mika Westerberg
On Sat, Feb 27, 2016 at 12:10:03AM -0800, dbasehore . wrote:
> A device is not able to use direct complete if its children do not
> also use direct complete. Even though the SCSI layer leaves devices
> runtime suspended, the way it does it still prevents its parent from
> using direct complete.

Okay.

Do you need to provide ->complete() hook that then resumes the device
when system is resumed (since the device resume hook is never called
when direct_complete is set)? Along the lines of
pm_complete_with_resume_check().
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 3/3] scsi: allow scsi devices to use direct complete

2016-02-26 Thread Mika Westerberg
On Wed, Feb 24, 2016 at 04:22:28PM -0800, Derek Basehore wrote:
> This allows scsi devices to remain runtime suspended for system
> suspend. Since runtime suspend is stricter than system suspend
> callbacks, this is just returning a positive number for the prepare
> callback.

AFAICT SCSI layer already leaves devices runtime suspended during system
suspend (see scsi_bus_suspend_common()). What's the benefit using
direct_complete over the current implementation?

> Signed-off-by: Derek Basehore 
> Reviewed-by: Eric Caruso 
> ---
>  drivers/scsi/scsi_pm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
> index b44c1bb..7af76ad 100644
> --- a/drivers/scsi/scsi_pm.c
> +++ b/drivers/scsi/scsi_pm.c
> @@ -178,7 +178,7 @@ static int scsi_bus_prepare(struct device *dev)
>   /* Wait until async scanning is finished */
>   scsi_complete_async_scans();
>   }
> - return 0;
> + return 1;
>  }
>  
>  static int scsi_bus_suspend(struct device *dev)
> -- 
> 2.7.0.rc3.207.g0ac5344
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/7] Runtime PM support for AHCI host controller driver

2016-02-19 Thread Mika Westerberg
On Thu, Feb 18, 2016 at 11:40:29AM -0500, Tejun Heo wrote:
> Hello,
> 
> Patchset looks good to me.  Once Jens acks the first patch, I'll apply
> the whole series to libata/for-4.6 w/ the field initialization
> updated.

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/7] scsi: Drop runtime PM usage count after host is added

2016-02-19 Thread Mika Westerberg
On Fri, Feb 19, 2016 at 09:50:50AM +1100, Julian Calaby wrote:
> Hi Mika,
> 
> On Thu, Feb 18, 2016 at 7:54 PM, Mika Westerberg
>  wrote:
> > Runtime PM of the SCSI host is already handled by calls to
> > scsi_autopm_get_host() and scsi_autopm_put_host() from appropriate places
> > whenever the host needs to be powered on. This works fine when there is
> > device connected to the host as once it runtime suspends the host will too.
> >
> > However, if there is no device connected the host is never runtime
> > suspended (the usage counter is always 0).
> >
> > Allow runtime suspend of host even if it has no devices connected by
> > calling scsi_autopm_put_host() at the end of scsi_add_host_with_dma(). We
> > temporarily increase runtime PM usage counter first so call to
> > scsi_autopm_put_host() will result idle request to be scheduled for the
> > device.
> >
> > Signed-off-by: Mika Westerberg 
> > ---
> >  drivers/scsi/hosts.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> > index 82ac1cd818ac..e46bf4d152a0 100644
> > --- a/drivers/scsi/hosts.c
> > +++ b/drivers/scsi/hosts.c
> > @@ -250,6 +250,12 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, 
> > struct device *dev,
> > if (error)
> > goto out_destroy_freelist;
> >
> > +   /*
> > +* Increase usage count temporarily here so that calling
> > +* scsi_autopm_put_host() will trigger runtime idle if there is
> > +* nothing else preventing suspending the device.
> > +*/
> > +   pm_runtime_get_noresume(&shost->shost_gendev);
> > pm_runtime_set_active(&shost->shost_gendev);
> > pm_runtime_enable(&shost->shost_gendev);
> > device_enable_async_suspend(&shost->shost_gendev);
> > @@ -290,6 +296,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, 
> > struct device *dev,
> > goto out_destroy_host;
> >
> > scsi_proc_host_add(shost);
> > +   scsi_autopm_put_host(shost);
> 
> Would it be cleaner to export the code that runs when the usage
> counter decrements and call it here?

There actually is no code to run. This just ensures that the device
runtime_idle gets called which allows the parent device to runtime
suspend (as it returns 0). Alternative way would be just to call
pm_request_idle() directly here.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/7] Runtime PM support for AHCI host controller driver

2016-02-18 Thread Mika Westerberg
Hi,

Linux already supports runtime PM of disks (drivers/scsi/sd.c) so that
after certain amount of idle time the disk is suspended automatically. This
series extends the support to AHCI host controllers. Whenever SATA ports
are determined to be idle (all children are runtime suspended) the host
controller is also suspended.

On recent Intel CPUs like Broxton this allows the CPU to go low power idle
states like S0ix runtime (given that all necessary blocks are also in their
correesponding low power states).

Patches [1-2/7] fix a lockup where disk is runtime suspended and the system
is put to sleep. They are independent of the rest of the series.

Patch [3/7] makes it possible for SATA ports to be runtime suspended when
there is not disk connected. For example on Lenovo Yoga 900 there are two
SATA ports which only one of them has disk connected. This patch allows the
host controller to runtime suspend whenever the disk is idle.

Rest of the patches bring runtime PM support for the AHCI driver. By
default runtime PM is blocked and needs to be unblocked from userspace
(following what other PCI drivers do). I've used following script to
unblock runtime PM for the whole stack (with 15 seconds of idle time):

--8<--8<--8<--8<--8<--8<--8<--
#!/bin/sh

TIMEOUT=${1:-15}
HOST=$(lspci -D | grep "SATA controller" | cut -f 1 -d ' ')
DISK=sda

# Enable runtime PM for all SATA ports
for port in /sys/bus/pci/devices/$HOST/ata*; do
echo auto > $port/power/control
done
# Then for the host controller
echo auto > /sys/bus/pci/devices/$HOST/power/control

# And last for the disk
echo auto > /sys/block/$DISK/device/power/control
echo $(($TIMEOUT * 1000)) > /sys/block/$DISK/device/power/autosuspend_delay_ms
--8<--8<--8<--8<--8<--8<--8<--

Mika Westerberg (7):
  block: Add blk_set_runtime_active()
  scsi: Set request queue runtime PM status back to active on resume
  scsi: Drop runtime PM usage count after host is added
  ahci: Cache host controller version
  ahci: Convert driver to use modern PM hooks
  ahci: Add functions to manage runtime PM of AHCI ports
  ahci: Add runtime PM support for the host controller

 block/blk-core.c   |  24 
 drivers/ata/ahci.c | 102 +++--
 drivers/ata/ahci.h |   1 +
 drivers/ata/libahci.c  |  55 +++---
 drivers/scsi/hosts.c   |   7 
 drivers/scsi/scsi_pm.c |  10 +
 include/linux/blkdev.h |   2 +
 7 files changed, 167 insertions(+), 34 deletions(-)

-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/7] scsi: Drop runtime PM usage count after host is added

2016-02-18 Thread Mika Westerberg
Runtime PM of the SCSI host is already handled by calls to
scsi_autopm_get_host() and scsi_autopm_put_host() from appropriate places
whenever the host needs to be powered on. This works fine when there is
device connected to the host as once it runtime suspends the host will too.

However, if there is no device connected the host is never runtime
suspended (the usage counter is always 0).

Allow runtime suspend of host even if it has no devices connected by
calling scsi_autopm_put_host() at the end of scsi_add_host_with_dma(). We
temporarily increase runtime PM usage counter first so call to
scsi_autopm_put_host() will result idle request to be scheduled for the
device.

Signed-off-by: Mika Westerberg 
---
 drivers/scsi/hosts.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 82ac1cd818ac..e46bf4d152a0 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -250,6 +250,12 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct 
device *dev,
if (error)
goto out_destroy_freelist;
 
+   /*
+* Increase usage count temporarily here so that calling
+* scsi_autopm_put_host() will trigger runtime idle if there is
+* nothing else preventing suspending the device.
+*/
+   pm_runtime_get_noresume(&shost->shost_gendev);
pm_runtime_set_active(&shost->shost_gendev);
pm_runtime_enable(&shost->shost_gendev);
device_enable_async_suspend(&shost->shost_gendev);
@@ -290,6 +296,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct 
device *dev,
goto out_destroy_host;
 
scsi_proc_host_add(shost);
+   scsi_autopm_put_host(shost);
return error;
 
  out_destroy_host:
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/7] block: Add blk_set_runtime_active()

2016-02-18 Thread Mika Westerberg
If block device is left runtime suspended during system suspend, resume
hook of the driver typically corrects runtime PM status of the device back
to "active" after it is resumed. However, this is not enough as queue's
runtime PM status is still "suspended". As long as it is in this state
blk_pm_peek_request() returns NULL and thus prevents new requests to be
processed.

Add new function blk_set_runtime_active() that can be used to force the
queue status back to "active" as needed.

Signed-off-by: Mika Westerberg 
---
 block/blk-core.c   | 24 
 include/linux/blkdev.h |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/block/blk-core.c b/block/blk-core.c
index b83d29755b5a..d0c6f4c92cb6 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -3529,6 +3529,30 @@ void blk_post_runtime_resume(struct request_queue *q, 
int err)
spin_unlock_irq(q->queue_lock);
 }
 EXPORT_SYMBOL(blk_post_runtime_resume);
+
+/**
+ * blk_set_runtime_active - Force runtime status of the queue to be active
+ * @q: the queue of the device
+ *
+ * If the device is left runtime suspended during system suspend the resume
+ * hook typically resumes the device and corrects runtime status
+ * accordingly. However, that does not affect the queue runtime PM status
+ * which is still "suspended". This prevents processing requests from the
+ * queue.
+ *
+ * This function can be used in driver's resume hook to correct queue
+ * runtime PM status and re-enable peeking requests from the queue. It
+ * should be called before first request is added to the queue.
+ */
+void blk_set_runtime_active(struct request_queue *q)
+{
+   spin_lock_irq(q->queue_lock);
+   q->rpm_status = RPM_ACTIVE;
+   pm_runtime_mark_last_busy(q->dev);
+   pm_request_autosuspend(q->dev);
+   spin_unlock_irq(q->queue_lock);
+}
+EXPORT_SYMBOL(blk_set_runtime_active);
 #endif
 
 int __init blk_dev_init(void)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4571ef1a12a9..40c0241a6f28 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1029,6 +1029,7 @@ extern int blk_pre_runtime_suspend(struct request_queue 
*q);
 extern void blk_post_runtime_suspend(struct request_queue *q, int err);
 extern void blk_pre_runtime_resume(struct request_queue *q);
 extern void blk_post_runtime_resume(struct request_queue *q, int err);
+extern void blk_set_runtime_active(struct request_queue *q);
 #else
 static inline void blk_pm_runtime_init(struct request_queue *q,
struct device *dev) {}
@@ -1039,6 +1040,7 @@ static inline int blk_pre_runtime_suspend(struct 
request_queue *q)
 static inline void blk_post_runtime_suspend(struct request_queue *q, int err) 
{}
 static inline void blk_pre_runtime_resume(struct request_queue *q) {}
 static inline void blk_post_runtime_resume(struct request_queue *q, int err) {}
+extern inline void blk_set_runtime_active(struct request_queue *q) {}
 #endif
 
 /*
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 7/7] ahci: Add runtime PM support for the host controller

2016-02-18 Thread Mika Westerberg
This patch adds runtime PM support for the AHCI host controller driver so
that the host controller is powered down when all SATA ports are runtime
suspended. Powering down the AHCI host controller can reduce power
consumption and possibly allow the CPU to enter lower power idle states
(S0ix) during runtime.

Runtime PM is blocked by default and needs to be unblocked from userspace
as needed (via power/* sysfs nodes).

Signed-off-by: Mika Westerberg 
---
 drivers/ata/ahci.c | 73 +-
 1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 44f9d1c09bbe..fb0c23ae3ce9 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -85,6 +85,7 @@ enum board_ids {
 };
 
 static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id 
*ent);
+static void ahci_remove_one(struct pci_dev *dev);
 static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
 unsigned long deadline);
 static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class,
@@ -93,10 +94,14 @@ static void ahci_mcp89_apple_enable(struct pci_dev *pdev);
 static bool is_mcp89_apple(struct pci_dev *pdev);
 static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
unsigned long deadline);
+#ifdef CONFIG_PM
+static int ahci_pci_device_runtime_suspend(struct device *dev);
+static int ahci_pci_device_runtime_resume(struct device *dev);
 #ifdef CONFIG_PM_SLEEP
 static int ahci_pci_device_suspend(struct device *dev);
 static int ahci_pci_device_resume(struct device *dev);
 #endif
+#endif /* CONFIG_PM */
 
 static struct scsi_host_template ahci_sht = {
AHCI_SHT("ahci"),
@@ -559,13 +564,15 @@ static const struct pci_device_id ahci_pci_tbl[] = {
 
 static const struct dev_pm_ops ahci_pci_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(ahci_pci_device_suspend, ahci_pci_device_resume)
+   SET_RUNTIME_PM_OPS(ahci_pci_device_runtime_suspend,
+  ahci_pci_device_runtime_resume, NULL)
 };
 
 static struct pci_driver ahci_pci_driver = {
.name   = DRV_NAME,
.id_table   = ahci_pci_tbl,
.probe  = ahci_init_one,
-   .remove = ata_pci_remove_one,
+   .remove = ahci_remove_one,
.driver.pm  = &ahci_pci_pm_ops,
 };
 
@@ -794,21 +801,13 @@ static int ahci_avn_hardreset(struct ata_link *link, 
unsigned int *class,
 }
 
 
-#ifdef CONFIG_PM_SLEEP
-static int ahci_pci_device_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static void ahci_pci_disable_interrupts(struct ata_host *host)
 {
-   struct pci_dev *pdev = to_pci_dev(dev);
-   struct ata_host *host = pci_get_drvdata(pdev);
struct ahci_host_priv *hpriv = host->private_data;
void __iomem *mmio = hpriv->mmio;
u32 ctl;
 
-   if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
-   dev_err(&pdev->dev,
-   "BIOS update required for suspend/resume\n");
-   return -EIO;
-   }
-
/* AHCI spec rev1.1 section 8.3.3:
 * Software must disable interrupts prior to requesting a
 * transition of the HBA to D3 state.
@@ -817,7 +816,44 @@ static int ahci_pci_device_suspend(struct device *dev)
ctl &= ~HOST_IRQ_EN;
writel(ctl, mmio + HOST_CTL);
readl(mmio + HOST_CTL); /* flush */
+}
+
+static int ahci_pci_device_runtime_suspend(struct device *dev)
+{
+   struct pci_dev *pdev = to_pci_dev(dev);
+   struct ata_host *host = pci_get_drvdata(pdev);
 
+   ahci_pci_disable_interrupts(host);
+   return 0;
+}
+
+static int ahci_pci_device_runtime_resume(struct device *dev)
+{
+   struct pci_dev *pdev = to_pci_dev(dev);
+   struct ata_host *host = pci_get_drvdata(pdev);
+   int rc;
+
+   rc = ahci_pci_reset_controller(host);
+   if (rc)
+   return rc;
+   ahci_pci_init_controller(host);
+   return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ahci_pci_device_suspend(struct device *dev)
+{
+   struct pci_dev *pdev = to_pci_dev(dev);
+   struct ata_host *host = pci_get_drvdata(pdev);
+   struct ahci_host_priv *hpriv = host->private_data;
+
+   if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
+   dev_err(&pdev->dev,
+   "BIOS update required for suspend/resume\n");
+   return -EIO;
+   }
+
+   ahci_pci_disable_interrupts(host);
return ata_host_suspend(host, PMSG_SUSPEND);
 }
 
@@ -845,6 +881,8 @@ static int ahci_pci_device_resume(struct device *dev)
 }
 #endif
 
+#endif /* CONFIG_PM */
+
 static int ahci_configure_dma_masks(struct pci_dev *pdev, int using_dac)
 {
int rc;
@@ -1664,7 +1702,18 @@ static int ahci_init_one(struct pci_dev *pdev, const 

[PATCH 5/7] ahci: Convert driver to use modern PM hooks

2016-02-18 Thread Mika Westerberg
In order to add support for runtime PM to the ahci driver we first need to
convert the driver to use modern non-legacy system suspend hooks. There
should be no functional changes.

Signed-off-by: Mika Westerberg 
---
 drivers/ata/ahci.c | 49 ++---
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 546a3692774f..44f9d1c09bbe 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -93,9 +93,9 @@ static void ahci_mcp89_apple_enable(struct pci_dev *pdev);
 static bool is_mcp89_apple(struct pci_dev *pdev);
 static int ahci_p5wdh_hardreset(struct ata_link *link, unsigned int *class,
unsigned long deadline);
-#ifdef CONFIG_PM
-static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
-static int ahci_pci_device_resume(struct pci_dev *pdev);
+#ifdef CONFIG_PM_SLEEP
+static int ahci_pci_device_suspend(struct device *dev);
+static int ahci_pci_device_resume(struct device *dev);
 #endif
 
 static struct scsi_host_template ahci_sht = {
@@ -557,16 +557,16 @@ static const struct pci_device_id ahci_pci_tbl[] = {
{ } /* terminate list */
 };
 
+static const struct dev_pm_ops ahci_pci_pm_ops = {
+   SET_SYSTEM_SLEEP_PM_OPS(ahci_pci_device_suspend, ahci_pci_device_resume)
+};
 
 static struct pci_driver ahci_pci_driver = {
.name   = DRV_NAME,
.id_table   = ahci_pci_tbl,
.probe  = ahci_init_one,
.remove = ata_pci_remove_one,
-#ifdef CONFIG_PM
-   .suspend= ahci_pci_device_suspend,
-   .resume = ahci_pci_device_resume,
-#endif
+   .driver.pm  = &ahci_pci_pm_ops,
 };
 
 #if defined(CONFIG_PATA_MARVELL) || defined(CONFIG_PATA_MARVELL_MODULE)
@@ -794,44 +794,39 @@ static int ahci_avn_hardreset(struct ata_link *link, 
unsigned int *class,
 }
 
 
-#ifdef CONFIG_PM
-static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
+#ifdef CONFIG_PM_SLEEP
+static int ahci_pci_device_suspend(struct device *dev)
 {
+   struct pci_dev *pdev = to_pci_dev(dev);
struct ata_host *host = pci_get_drvdata(pdev);
struct ahci_host_priv *hpriv = host->private_data;
void __iomem *mmio = hpriv->mmio;
u32 ctl;
 
-   if (mesg.event & PM_EVENT_SUSPEND &&
-   hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
+   if (hpriv->flags & AHCI_HFLAG_NO_SUSPEND) {
dev_err(&pdev->dev,
"BIOS update required for suspend/resume\n");
return -EIO;
}
 
-   if (mesg.event & PM_EVENT_SLEEP) {
-   /* AHCI spec rev1.1 section 8.3.3:
-* Software must disable interrupts prior to requesting a
-* transition of the HBA to D3 state.
-*/
-   ctl = readl(mmio + HOST_CTL);
-   ctl &= ~HOST_IRQ_EN;
-   writel(ctl, mmio + HOST_CTL);
-   readl(mmio + HOST_CTL); /* flush */
-   }
+   /* AHCI spec rev1.1 section 8.3.3:
+* Software must disable interrupts prior to requesting a
+* transition of the HBA to D3 state.
+*/
+   ctl = readl(mmio + HOST_CTL);
+   ctl &= ~HOST_IRQ_EN;
+   writel(ctl, mmio + HOST_CTL);
+   readl(mmio + HOST_CTL); /* flush */
 
-   return ata_pci_device_suspend(pdev, mesg);
+   return ata_host_suspend(host, PMSG_SUSPEND);
 }
 
-static int ahci_pci_device_resume(struct pci_dev *pdev)
+static int ahci_pci_device_resume(struct device *dev)
 {
+   struct pci_dev *pdev = to_pci_dev(dev);
struct ata_host *host = pci_get_drvdata(pdev);
int rc;
 
-   rc = ata_pci_device_do_resume(pdev);
-   if (rc)
-   return rc;
-
/* Apple BIOS helpfully mangles the registers on resume */
if (is_mcp89_apple(pdev))
ahci_mcp89_apple_enable(pdev);
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/7] ahci: Add functions to manage runtime PM of AHCI ports

2016-02-18 Thread Mika Westerberg
Add new functions ahci_rpm_get_port()/ahci_rpm_put_port() that change
runtime PM status of AHCI ports. Depending if the AHCI host has runtime PM
enabled or disabled calling these may trigger runtime suspend/resume of the
host controller.

We also call these functions in appropriate places to make sure host
controller registers are available before using them.

Signed-off-by: Mika Westerberg 
---
 drivers/ata/libahci.c | 48 +++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 2ce4c638714e..a5dc80810a5e 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -224,6 +224,31 @@ static void ahci_enable_ahci(void __iomem *mmio)
WARN_ON(1);
 }
 
+/**
+ * ahci_rpm_get_port - Make sure the port is powered on
+ * @ap: Port to power on
+ *
+ * Whenever there is need to access the AHCI host registers outside of
+ * normal execution paths, call this function to make sure the host is
+ * actually powered on.
+ */
+static int ahci_rpm_get_port(struct ata_port *ap)
+{
+   return pm_runtime_get_sync(ap->dev);
+}
+
+/**
+ * ahci_rpm_put_port - Undoes ahci_rpm_get_port()
+ * @ap: Port to power down
+ *
+ * Undoes ahci_rpm_get_port() and possibly powers down the AHCI host
+ * if it has no more active users.
+ */
+static void ahci_rpm_put_port(struct ata_port *ap)
+{
+   pm_runtime_put(ap->dev);
+}
+
 static ssize_t ahci_show_host_caps(struct device *dev,
   struct device_attribute *attr, char *buf)
 {
@@ -260,8 +285,13 @@ static ssize_t ahci_show_port_cmd(struct device *dev,
struct Scsi_Host *shost = class_to_shost(dev);
struct ata_port *ap = ata_shost_to_port(shost);
void __iomem *port_mmio = ahci_port_base(ap);
+   ssize_t ret;
 
-   return sprintf(buf, "%x\n", readl(port_mmio + PORT_CMD));
+   ahci_rpm_get_port(ap);
+   ret = sprintf(buf, "%x\n", readl(port_mmio + PORT_CMD));
+   ahci_rpm_put_port(ap);
+
+   return ret;
 }
 
 static ssize_t ahci_read_em_buffer(struct device *dev,
@@ -277,17 +307,20 @@ static ssize_t ahci_read_em_buffer(struct device *dev,
size_t count;
int i;
 
+   ahci_rpm_get_port(ap);
spin_lock_irqsave(ap->lock, flags);
 
em_ctl = readl(mmio + HOST_EM_CTL);
if (!(ap->flags & ATA_FLAG_EM) || em_ctl & EM_CTL_XMT ||
!(hpriv->em_msg_type & EM_MSG_TYPE_SGPIO)) {
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
return -EINVAL;
}
 
if (!(em_ctl & EM_CTL_MR)) {
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
return -EAGAIN;
}
 
@@ -315,6 +348,7 @@ static ssize_t ahci_read_em_buffer(struct device *dev,
}
 
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
 
return i;
 }
@@ -339,11 +373,13 @@ static ssize_t ahci_store_em_buffer(struct device *dev,
size % 4 || size > hpriv->em_buf_sz)
return -EINVAL;
 
+   ahci_rpm_get_port(ap);
spin_lock_irqsave(ap->lock, flags);
 
em_ctl = readl(mmio + HOST_EM_CTL);
if (em_ctl & EM_CTL_TM) {
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
return -EBUSY;
}
 
@@ -356,6 +392,7 @@ static ssize_t ahci_store_em_buffer(struct device *dev,
writel(em_ctl | EM_CTL_TM, mmio + HOST_EM_CTL);
 
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
 
return size;
 }
@@ -369,7 +406,9 @@ static ssize_t ahci_show_em_supported(struct device *dev,
void __iomem *mmio = hpriv->mmio;
u32 em_ctl;
 
+   ahci_rpm_get_port(ap);
em_ctl = readl(mmio + HOST_EM_CTL);
+   ahci_rpm_put_port(ap);
 
return sprintf(buf, "%s%s%s%s\n",
   em_ctl & EM_CTL_LED ? "led " : "",
@@ -1010,6 +1049,7 @@ static ssize_t ahci_transmit_led_message(struct ata_port 
*ap, u32 state,
else
return -EINVAL;
 
+   ahci_rpm_get_port(ap);
spin_lock_irqsave(ap->lock, flags);
 
/*
@@ -1019,6 +1059,7 @@ static ssize_t ahci_transmit_led_message(struct ata_port 
*ap, u32 state,
em_ctl = readl(mmio + HOST_EM_CTL);
if (em_ctl & EM_CTL_TM) {
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
return -EBUSY;
}
 
@@ -1046,6 +1087,8 @@ static ssize_t ahci_transmit_led_message(struct ata_port 
*ap, u32 state,
emp->led_state = state;
 
spin_unlock_irqrestore(ap->lock, flags);
+   ahci_rpm_put_port(ap);
+
return size;
 }
 
@@ -2248,6 +229

[PATCH 4/7] ahci: Cache host controller version

2016-02-18 Thread Mika Westerberg
This allows sysfs nodes to read the cached value directly instead of
powering up possibly runtime suspended controller.

Signed-off-by: Mika Westerberg 
---
 drivers/ata/ahci.h| 1 +
 drivers/ata/libahci.c | 7 +++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index a44c75d4c284..8138bc967eda 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -336,6 +336,7 @@ struct ahci_host_priv {
void __iomem *  mmio;   /* bus-independent mem map */
u32 cap;/* cap to use */
u32 cap2;   /* cap2 to use */
+   u32 version;/* cached version */
u32 port_map;   /* port map to use */
u32 saved_cap;  /* saved initial cap */
u32 saved_cap2; /* saved initial cap2 */
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 402967902cbe..2ce4c638714e 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -250,9 +250,8 @@ static ssize_t ahci_show_host_version(struct device *dev,
struct Scsi_Host *shost = class_to_shost(dev);
struct ata_port *ap = ata_shost_to_port(shost);
struct ahci_host_priv *hpriv = ap->host->private_data;
-   void __iomem *mmio = hpriv->mmio;
 
-   return sprintf(buf, "%x\n", readl(mmio + HOST_VERSION));
+   return sprintf(buf, "%x\n", hpriv->version);
 }
 
 static ssize_t ahci_show_port_cmd(struct device *dev,
@@ -508,6 +507,7 @@ void ahci_save_initial_config(struct device *dev, struct 
ahci_host_priv *hpriv)
/* record values to use during operation */
hpriv->cap = cap;
hpriv->cap2 = cap2;
+   hpriv->version = readl(mmio + HOST_VERSION);
hpriv->port_map = port_map;
 
if (!hpriv->start_engine)
@@ -2389,11 +2389,10 @@ static void ahci_port_stop(struct ata_port *ap)
 void ahci_print_info(struct ata_host *host, const char *scc_s)
 {
struct ahci_host_priv *hpriv = host->private_data;
-   void __iomem *mmio = hpriv->mmio;
u32 vers, cap, cap2, impl, speed;
const char *speed_s;
 
-   vers = readl(mmio + HOST_VERSION);
+   vers = hpriv->version;
cap = hpriv->cap;
cap2 = hpriv->cap2;
impl = hpriv->port_map;
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/7] scsi: Set request queue runtime PM status back to active on resume

2016-02-18 Thread Mika Westerberg
We treat system suspend of SCSI devices pretty much the same as runtime
suspend. If the device is already runtime suspended we leave it to that
state during system suspend. On resume from system sleep we then resume the
device and correct the runtime PM status back to "active".

There is a problem with this because runtime PM status of the request queue
in question is not changed (it will be in "suspended" state). When SCSI
disk driver (sd.c) resumes the disk it sends START message to the device
and because the request queue is still in "suspended" state
blk_pm_peek_request() returns NULL preventing resume of the disk.

The issue can be reproduced with following commands:

  # echo auto > /sys/block/sda/device/power/control
  # echo 15000 > /sys/block/sda/device/power/autosuspend_delay_ms
  [   57.191706] sd 0:0:0:0: [sda] Synchronizing SCSI cache
  [   57.380015] sd 0:0:0:0: [sda] Stopping disk

Now suspend the machine:

  # rtcwake -s10 -mmem

This ends up in soft lockup because resume is not proceeding accordingly
and userspace is never restarted. Also there is nothing printed to the
console.

Fix this by forcing request queue status to "active" before the disk is
resumed.

Signed-off-by: Mika Westerberg 
---
 drivers/scsi/scsi_pm.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 459abe1dcc87..b44c1bb687a2 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -139,6 +139,16 @@ static int scsi_bus_resume_common(struct device *dev,
else
fn = NULL;
 
+   /*
+* Forcibly set runtime PM status of request queue to "active" to
+* make sure we can again get requests from the queue (see also
+* blk_pm_peek_request()).
+*
+* The resume hook will correct runtime PM status of the disk.
+*/
+   if (scsi_is_sdev_device(dev) && pm_runtime_suspended(dev))
+   blk_set_runtime_active(to_scsi_device(dev)->request_queue);
+
if (fn) {
async_schedule_domain(fn, dev, &scsi_sd_pm_domain);
 
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] SCSI: Add Marvell Console to VPD blacklist

2016-01-27 Thread Mika Westerberg
I have a Marvell 88SE9230 SATA Controller that has some sort of
integrated console SCSI device attached to one of the ports.

  ata14: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
  ata14.00: ATAPI: MARVELL VIRTUALL, 1.09, max UDMA/66
  ata14.00: configured for UDMA/66
  scsi 13:0:0:0: Processor Marvell  Console 1.01 PQ: 0 ANSI: 5

Sending it VPD INQUIRY command seem to always fail with following error:

  ata14.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
  ata14.00: irq_stat 0x4001
  ata14.00: cmd a0/01:00:00:00:01/00:00:00:00:00/a0 tag 2 dma 16640 in
Inquiry 12 01 00 00 ff 00res 00/00:00:00:00:00/00:00:00:00:00/00 
Emask 0x3 (HSM violation)
  ata14: hard resetting link

This has been minor annoyance (only error printed on dmesg) until commit
09e2b0b14690 ("scsi: rescan VPD attributes") added call to scsi_attach_vpd()
in scsi_rescan_device(). The commit causes the system to splat out
following errors continuously without ever reaching the UI:

  ata14.00: configured for UDMA/66
  ata14: EH complete
  ata14.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
  ata14.00: irq_stat 0x4001
  ata14.00: cmd a0/01:00:00:00:01/00:00:00:00:00/a0 tag 6 dma 16640 in
Inquiry 12 01 00 00 ff 00res 00/00:00:00:00:00/00:00:00:00:00/00 
Emask 0x3 (HSM violation)
  ata14: hard resetting link
  ata14: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
  ata14.00: configured for UDMA/66
  ata14: EH complete
  ata14.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
  ata14.00: irq_stat 0x4001
  ata14.00: cmd a0/01:00:00:00:01/00:00:00:00:00/a0 tag 7 dma 16640 in
Inquiry 12 01 00 00 ff 00res 00/00:00:00:00:00/00:00:00:00:00/00 
Emask 0x3 (HSM violation)

Without in-depth understanding of SCSI layer and the Marvell controller, I
suspect this happens because when the link goes down (because of an error)
we schedule scsi_rescan_device() which again fails to read VPD data... ad
infinitum.

Since VPD data cannot be read from the device anyway we prevent the SCSI
layer from even trying by blacklisting the device. This gets away the error
and the system starts up normally.

Signed-off-by: Mika Westerberg 
Cc: Hannes Reinecke 
---
I'm not sure if this is most ideal fix. Comment on top of
scsi_static_device_list[] says that the list is going away eventually and
suggest me to use command line instead. I can do that but then everyone
else having similar device will need to do the same to get rid of the boot
failure.

 drivers/scsi/scsi_devinfo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 47b9d13..350cbed 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -205,6 +205,7 @@ static struct {
{"Intel", "Multi-Flex", NULL, BLIST_NO_RSOC},
{"iRiver", "iFP Mass Driver", NULL, BLIST_NOT_LOCKABLE | 
BLIST_INQUIRY_36},
{"LASOUND", "CDX7405", "3.10", BLIST_MAX5LUN | BLIST_SINGLELUN},
+   {"Marvell", "Console", "1.01", BLIST_SKIP_VPD_PAGES},
{"MATSHITA", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
{"MATSHITA", "DMC-LC5", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
{"MATSHITA", "DMC-LC40", NULL, BLIST_NOT_LOCKABLE | BLIST_INQUIRY_36},
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html