2012-06-18 06:26:28

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 0/13] SATA ZPODD support

Hi all,

This is the v5 patches to add SATA ZPODD support, to try it:
git pull git://git.kernel.org/pub/scm/linux/kernel/git/mlin/linux.git zpodd

Holger and Mathtew,
Patch 1 to Patch 4 are the libata acpi binding patches from you.
I removed dock related code from your original version.
Would you help to review?

v5:
- rebase to v3.5-rc3
- fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu)
- rename is_pci_ata to compat_pci_ata (Alan Cox)

v4:
https://lkml.org/lkml/2012/5/28/11
- Includes libata acpi binding patches from Holger Macht and Matthew Garrett.
- tell scsi layer device supports runtime power off
- check support for device busy class events

v3:
https://lkml.org/lkml/2012/3/28/20
- Split the ACPI D3Cold state support patches out
- Adds "Device Attention" bit check

v2:
https://lkml.org/lkml/2012/3/1/61
- _PR3 indicates D3Cold support
- move can_power_off flag to pm_subsys_data
- allow all combinations of power resource and device
- split patches into smaller ones to make review easy

v1:
https://lkml.org/lkml/2012/2/13/86


Aaron Lu (4):
libata: tell scsi layer device supports runtime power off
[SCSI] sr: check support for device busy class events
[SCSI] sr: support zero power ODD
[SCSI] sr: make sure ODD is in resumed state in block ioctl

Holger Macht (2):
[SCSI]: add wrapper to access and set scsi_bus_type in struct acpi_bus_type
libata: use correct PCI devices

Lin Ming (4):
libata-acpi: set acpi state for SATA port
libata-acpi: add ata port runtime D3Cold support
libata-acpi: register/unregister device to/from power resource
libata: detect Device Attention support

Matthew Garrett (2):
libata: bind the Linux device tree to the ACPI device tree
libata: migrate ACPI code over to new bindings


drivers/acpi/glue.c | 4 +-
drivers/acpi/power.c | 2 +
drivers/ata/libata-acpi.c | 389 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------
drivers/ata/libata-core.c | 9 ++-
drivers/ata/libata-pmp.c | 4 -
drivers/ata/libata-scsi.c | 3 +
drivers/ata/libata.h | 13 +--
drivers/ata/pata_acpi.c | 4 +-
drivers/scsi/scsi_lib.c | 17 ++++
drivers/scsi/sr.c | 160 +++++++++++++++++++++++++++++++++++-
drivers/scsi/sr.h | 3 +
include/linux/ata.h | 1 +
include/linux/cdrom.h | 43 ++++++++++
include/linux/libata.h | 9 +--
include/scsi/scsi.h | 10 +++
include/scsi/scsi_device.h | 2 +
16 files changed, 534 insertions(+), 139 deletions(-)


2012-06-18 06:26:36

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 03/12] libata: migrate ACPI code over to new bindings

From: Matthew Garrett <[email protected]>

Now that we have the ability to directly glue the ACPI namespace to the
driver model in libata, we don't need the custom code to handle the same
thing. Remove it and migrate the functions over to the new code.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: Holger Macht <[email protected]>
Signed-off-by: Lin Ming <[email protected]>
---
drivers/ata/libata-acpi.c | 161 ++++++++++-----------------------------------
drivers/ata/libata-core.c | 3 -
drivers/ata/libata-pmp.c | 4 --
drivers/ata/libata.h | 5 --
drivers/ata/pata_acpi.c | 4 +-
include/linux/libata.h | 7 +-
6 files changed, 40 insertions(+), 144 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index ae93593..adc6336 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -47,7 +47,14 @@ static void ata_acpi_clear_gtf(struct ata_device *dev)
dev->gtf_cache = NULL;
}

-static acpi_handle ap_acpi_handle(struct ata_port *ap)
+/**
+ * ata_ap_acpi_handle - provide the acpi_handle for an ata_port
+ * @ap: the acpi_handle returned will correspond to this port
+ *
+ * Returns the acpi_handle for the ACPI namespace object corresponding to
+ * the ata_port passed into the function, or NULL if no such object exists
+ */
+acpi_handle ata_ap_acpi_handle(struct ata_port *ap)
{
if (ap->flags & ATA_FLAG_ACPI_SATA)
return NULL;
@@ -64,8 +71,16 @@ static acpi_handle ap_acpi_handle(struct ata_port *ap)
return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev),
ap->port_no);
}
+EXPORT_SYMBOL(ata_ap_acpi_handle);

-static acpi_handle dev_acpi_handle(struct ata_device *dev)
+/**
+ * ata_dev_acpi_handle - provide the acpi_handle for an ata_device
+ * @dev: the acpi_device returned will correspond to this port
+ *
+ * Returns the acpi_handle for the ACPI namespace object corresponding to
+ * the ata_device passed into the function, or NULL if no such object exists
+ */
+acpi_handle ata_dev_acpi_handle(struct ata_device *dev)
{
acpi_integer adr;
struct ata_port *ap = dev->link->ap;
@@ -77,66 +92,9 @@ static acpi_handle dev_acpi_handle(struct ata_device *dev)
adr = SATA_ADR(ap->port_no, dev->link->pmp);
return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev), adr);
} else
- return acpi_get_child(ap_acpi_handle(ap), dev->devno);
-}
-
-/**
- * ata_acpi_associate_sata_port - associate SATA port with ACPI objects
- * @ap: target SATA port
- *
- * Look up ACPI objects associated with @ap and initialize acpi_handle
- * fields of @ap, the port and devices accordingly.
- *
- * LOCKING:
- * EH context.
- *
- * RETURNS:
- * 0 on success, -errno on failure.
- */
-void ata_acpi_associate_sata_port(struct ata_port *ap)
-{
- WARN_ON(!(ap->flags & ATA_FLAG_ACPI_SATA));
-
- if (!sata_pmp_attached(ap)) {
- u64 adr = SATA_ADR(ap->port_no, NO_PORT_MULT);
-
- ap->link.device->acpi_handle =
- acpi_get_child(ap->host->acpi_handle, adr);
- } else {
- struct ata_link *link;
-
- ap->link.device->acpi_handle = NULL;
-
- ata_for_each_link(link, ap, EDGE) {
- u64 adr = SATA_ADR(ap->port_no, link->pmp);
-
- link->device->acpi_handle =
- acpi_get_child(ap->host->acpi_handle, adr);
- }
- }
-}
-
-static void ata_acpi_associate_ide_port(struct ata_port *ap)
-{
- int max_devices, i;
-
- ap->acpi_handle = acpi_get_child(ap->host->acpi_handle, ap->port_no);
- if (!ap->acpi_handle)
- return;
-
- max_devices = 1;
- if (ap->flags & ATA_FLAG_SLAVE_POSS)
- max_devices++;
-
- for (i = 0; i < max_devices; i++) {
- struct ata_device *dev = &ap->link.device[i];
-
- dev->acpi_handle = acpi_get_child(ap->acpi_handle, i);
- }
-
- if (ata_acpi_gtm(ap, &ap->__acpi_init_gtm) == 0)
- ap->pflags |= ATA_PFLAG_INIT_GTM_VALID;
+ return acpi_get_child(ata_ap_acpi_handle(ap), dev->devno);
}
+EXPORT_SYMBOL(ata_dev_acpi_handle);

/* @ap and @dev are the same as ata_acpi_handle_hotplug() */
static void ata_acpi_detach_device(struct ata_port *ap, struct ata_device *dev)
@@ -262,56 +220,6 @@ static const struct acpi_dock_ops ata_acpi_ap_dock_ops = {
};

/**
- * ata_acpi_associate - associate ATA host with ACPI objects
- * @host: target ATA host
- *
- * Look up ACPI objects associated with @host and initialize
- * acpi_handle fields of @host, its ports and devices accordingly.
- *
- * LOCKING:
- * EH context.
- *
- * RETURNS:
- * 0 on success, -errno on failure.
- */
-void ata_acpi_associate(struct ata_host *host)
-{
- int i, j;
-
- if (!is_pci_dev(host->dev) || libata_noacpi)
- return;
-
- host->acpi_handle = DEVICE_ACPI_HANDLE(host->dev);
- if (!host->acpi_handle)
- return;
-
- for (i = 0; i < host->n_ports; i++) {
- struct ata_port *ap = host->ports[i];
-
- if (host->ports[0]->flags & ATA_FLAG_ACPI_SATA)
- ata_acpi_associate_sata_port(ap);
- else
- ata_acpi_associate_ide_port(ap);
-
- if (ap->acpi_handle) {
- /* we might be on a docking station */
- register_hotplug_dock_device(ap->acpi_handle,
- &ata_acpi_ap_dock_ops, ap);
- }
-
- for (j = 0; j < ata_link_max_devices(&ap->link); j++) {
- struct ata_device *dev = &ap->link.device[j];
-
- if (dev->acpi_handle) {
- /* we might be on a docking station */
- register_hotplug_dock_device(dev->acpi_handle,
- &ata_acpi_dev_dock_ops, dev);
- }
- }
- }
-}
-
-/**
* ata_acpi_dissociate - dissociate ATA host from ACPI objects
* @host: target ATA host
*
@@ -332,7 +240,7 @@ void ata_acpi_dissociate(struct ata_host *host)
struct ata_port *ap = host->ports[i];
const struct ata_acpi_gtm *gtm = ata_acpi_init_gtm(ap);

- if (ap->acpi_handle && gtm)
+ if (ata_ap_acpi_handle(ap) && gtm)
ata_acpi_stm(ap, gtm);
}
}
@@ -357,7 +265,8 @@ int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *gtm)
acpi_status status;
int rc = 0;

- status = acpi_evaluate_object(ap->acpi_handle, "_GTM", NULL, &output);
+ status = acpi_evaluate_object(ata_ap_acpi_handle(ap), "_GTM", NULL,
+ &output);

rc = -ENOENT;
if (status == AE_NOT_FOUND)
@@ -427,7 +336,8 @@ int ata_acpi_stm(struct ata_port *ap, const struct ata_acpi_gtm *stm)
input.count = 3;
input.pointer = in_params;

- status = acpi_evaluate_object(ap->acpi_handle, "_STM", &input, NULL);
+ status = acpi_evaluate_object(ata_ap_acpi_handle(ap), "_STM", &input,
+ NULL);

if (status == AE_NOT_FOUND)
return -ENOENT;
@@ -484,7 +394,8 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf)
__func__, ap->port_no);

/* _GTF has no input parameters */
- status = acpi_evaluate_object(dev->acpi_handle, "_GTF", NULL, &output);
+ status = acpi_evaluate_object(ata_dev_acpi_handle(dev), "_GTF", NULL,
+ &output);
out_obj = dev->gtf_cache = output.pointer;

if (ACPI_FAILURE(status)) {
@@ -850,7 +761,8 @@ static int ata_acpi_push_id(struct ata_device *dev)

/* It's OK for _SDD to be missing too. */
swap_buf_le16(dev->id, ATA_ID_WORDS);
- status = acpi_evaluate_object(dev->acpi_handle, "_SDD", &input, NULL);
+ status = acpi_evaluate_object(ata_dev_acpi_handle(dev), "_SDD", &input,
+ NULL);
swap_buf_le16(dev->id, ATA_ID_WORDS);

if (status == AE_NOT_FOUND)
@@ -900,7 +812,7 @@ void ata_acpi_on_resume(struct ata_port *ap)
const struct ata_acpi_gtm *gtm = ata_acpi_init_gtm(ap);
struct ata_device *dev;

- if (ap->acpi_handle && gtm) {
+ if (ata_ap_acpi_handle(ap) && gtm) {
/* _GTM valid */

/* restore timing parameters */
@@ -941,22 +853,22 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
{
struct ata_device *dev;

- if (!ap->acpi_handle || (ap->flags & ATA_FLAG_ACPI_SATA))
+ if (!ata_ap_acpi_handle(ap) || (ap->flags & ATA_FLAG_ACPI_SATA))
return;

/* channel first and then drives for power on and vica versa
for power off */
if (state.event == PM_EVENT_ON)
- acpi_bus_set_power(ap->acpi_handle, ACPI_STATE_D0);
+ acpi_bus_set_power(ata_ap_acpi_handle(ap), ACPI_STATE_D0);

ata_for_each_dev(dev, &ap->link, ENABLED) {
- if (dev->acpi_handle)
- acpi_bus_set_power(dev->acpi_handle,
+ if (ata_dev_acpi_handle(dev))
+ acpi_bus_set_power(ata_dev_acpi_handle(dev),
state.event == PM_EVENT_ON ?
ACPI_STATE_D0 : ACPI_STATE_D3);
}
if (state.event != PM_EVENT_ON)
- acpi_bus_set_power(ap->acpi_handle, ACPI_STATE_D3);
+ acpi_bus_set_power(ata_ap_acpi_handle(ap), ACPI_STATE_D3);
}

/**
@@ -981,7 +893,7 @@ int ata_acpi_on_devcfg(struct ata_device *dev)
int nr_executed = 0;
int rc;

- if (!dev->acpi_handle)
+ if (!ata_dev_acpi_handle(dev))
return 0;

/* do we need to do _GTF? */
@@ -1027,7 +939,6 @@ int ata_acpi_on_devcfg(struct ata_device *dev)
}

ata_dev_warn(dev, "ACPI: failed the second time, disabled\n");
- dev->acpi_handle = NULL;

/* We can safely continue if no _GTF command has been executed
* and port is not frozen.
@@ -1097,7 +1008,7 @@ static int ata_acpi_bind_device(struct device *dev, int channel, int id,
else
ata_dev = &ap->link.device[id];

- *handle = dev_acpi_handle(ata_dev);
+ *handle = ata_dev_acpi_handle(ata_dev);

if (!*handle)
return -ENODEV;
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 5628bf6..cebf18e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6051,9 +6051,6 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht)
if (rc)
goto err_tadd;

- /* associate with ACPI nodes */
- ata_acpi_associate(host);
-
/* set cable, sata_spd_limit and report */
for (i = 0; i < host->n_ports; i++) {
struct ata_port *ap = host->ports[i];
diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c
index 21b80c5..61c59ee 100644
--- a/drivers/ata/libata-pmp.c
+++ b/drivers/ata/libata-pmp.c
@@ -529,8 +529,6 @@ int sata_pmp_attach(struct ata_device *dev)
ata_for_each_link(tlink, ap, EDGE)
sata_link_init_spd(tlink);

- ata_acpi_associate_sata_port(ap);
-
return 0;

fail:
@@ -570,8 +568,6 @@ static void sata_pmp_detach(struct ata_device *dev)
ap->nr_pmp_links = 0;
link->pmp = 0;
spin_unlock_irqrestore(ap->lock, flags);
-
- ata_acpi_associate_sata_port(ap);
}

/**
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 3df3362..6aba606 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -110,9 +110,6 @@ extern void __ata_port_probe(struct ata_port *ap);
/* libata-acpi.c */
#ifdef CONFIG_ATA_ACPI
extern unsigned int ata_acpi_gtf_filter;
-
-extern void ata_acpi_associate_sata_port(struct ata_port *ap);
-extern void ata_acpi_associate(struct ata_host *host);
extern void ata_acpi_dissociate(struct ata_host *host);
extern int ata_acpi_on_suspend(struct ata_port *ap);
extern void ata_acpi_on_resume(struct ata_port *ap);
@@ -122,8 +119,6 @@ extern void ata_acpi_set_state(struct ata_port *ap, pm_message_t state);
extern int ata_acpi_register(void);
extern void ata_acpi_unregister(void);
#else
-static inline void ata_acpi_associate_sata_port(struct ata_port *ap) { }
-static inline void ata_acpi_associate(struct ata_host *host) { }
static inline void ata_acpi_dissociate(struct ata_host *host) { }
static inline int ata_acpi_on_suspend(struct ata_port *ap) { return 0; }
static inline void ata_acpi_on_resume(struct ata_port *ap) { }
diff --git a/drivers/ata/pata_acpi.c b/drivers/ata/pata_acpi.c
index 54145ed..b63ca3b 100644
--- a/drivers/ata/pata_acpi.c
+++ b/drivers/ata/pata_acpi.c
@@ -39,7 +39,7 @@ static int pacpi_pre_reset(struct ata_link *link, unsigned long deadline)
{
struct ata_port *ap = link->ap;
struct pata_acpi *acpi = ap->private_data;
- if (ap->acpi_handle == NULL || ata_acpi_gtm(ap, &acpi->gtm) < 0)
+ if (ata_ap_acpi_handle(ap) == NULL || ata_acpi_gtm(ap, &acpi->gtm) < 0)
return -ENODEV;

return ata_sff_prereset(link, deadline);
@@ -195,7 +195,7 @@ static int pacpi_port_start(struct ata_port *ap)
struct pci_dev *pdev = to_pci_dev(ap->host->dev);
struct pata_acpi *acpi;

- if (ap->acpi_handle == NULL)
+ if (ata_ap_acpi_handle(ap) == NULL)
return -ENODEV;

acpi = ap->private_data = devm_kzalloc(&pdev->dev, sizeof(struct pata_acpi), GFP_KERNEL);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 6e887c7..888feef 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -545,9 +545,6 @@ struct ata_host {
struct mutex eh_mutex;
struct task_struct *eh_owner;

-#ifdef CONFIG_ATA_ACPI
- acpi_handle acpi_handle;
-#endif
struct ata_port *simplex_claimed; /* channel owning the DMA */
struct ata_port *ports[0];
};
@@ -615,7 +612,6 @@ struct ata_device {
struct scsi_device *sdev; /* attached SCSI device */
void *private_data;
#ifdef CONFIG_ATA_ACPI
- acpi_handle acpi_handle;
union acpi_object *gtf_cache;
unsigned int gtf_filter;
#endif
@@ -797,7 +793,6 @@ struct ata_port {
void *private_data;

#ifdef CONFIG_ATA_ACPI
- acpi_handle acpi_handle;
struct ata_acpi_gtm __acpi_init_gtm; /* use ata_acpi_init_gtm() */
#endif
/* owned by EH */
@@ -1114,6 +1109,8 @@ int ata_acpi_stm(struct ata_port *ap, const struct ata_acpi_gtm *stm);
int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *stm);
unsigned long ata_acpi_gtm_xfermask(struct ata_device *dev,
const struct ata_acpi_gtm *gtm);
+acpi_handle ata_ap_acpi_handle(struct ata_port *ap);
+acpi_handle ata_dev_acpi_handle(struct ata_device *dev);
int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm);
#else
static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap)
--
1.7.10

2012-06-18 06:26:41

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 06/12] libata-acpi: add ata port runtime D3Cold support

ATA port may support runtime D3Cold state, for example, Zero-power ODD case.
This patch adds wakeup notifier and enable/disable run_wake during
supend/resume.

Signed-off-by: Lin Ming <[email protected]>
---
drivers/ata/libata-acpi.c | 77 ++++++++++++++++++++++++++++++++++++++++++---
drivers/ata/libata-scsi.c | 3 ++
drivers/ata/libata.h | 4 +++
3 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 2887eb0..3f7d376 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -16,6 +16,7 @@
#include <linux/libata.h>
#include <linux/pci.h>
#include <linux/slab.h>
+#include <linux/pm_runtime.h>
#include <scsi/scsi_device.h>
#include "libata.h"

@@ -863,10 +864,23 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)

ata_for_each_dev(dev, &ap->link, ENABLED) {
handle = ata_dev_acpi_handle(dev);
- if (handle)
- acpi_bus_set_power(handle,
- state.event == PM_EVENT_ON ?
- ACPI_STATE_D0 : ACPI_STATE_D3);
+ if (!handle)
+ continue;
+
+ if (state.event != PM_EVENT_ON) {
+ acpi_state = acpi_pm_device_sleep_state(
+ &dev->sdev->sdev_gendev, NULL);
+ if (acpi_state > 0)
+ acpi_bus_set_power(handle, acpi_state);
+ /* TBD: need to check if it's runtime pm request */
+ acpi_pm_device_run_wake(
+ &dev->sdev->sdev_gendev, true);
+ } else {
+ /* Ditto */
+ acpi_pm_device_run_wake(
+ &dev->sdev->sdev_gendev, false);
+ acpi_bus_set_power(handle, ACPI_STATE_D0);
+ }
}

handle = ata_ap_acpi_handle(ap);
@@ -966,6 +980,61 @@ void ata_acpi_on_disable(struct ata_device *dev)
ata_acpi_clear_gtf(dev);
}

+static void ata_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
+{
+ struct ata_device *ata_dev = context;
+
+ if (event == ACPI_NOTIFY_DEVICE_WAKE && ata_dev &&
+ pm_runtime_suspended(&ata_dev->sdev->sdev_gendev))
+ scsi_autopm_get_device(ata_dev->sdev);
+}
+
+static void ata_acpi_add_pm_notifier(struct ata_device *dev)
+{
+ struct acpi_device *acpi_dev;
+ acpi_handle handle;
+ acpi_status status;
+
+ handle = ata_dev_acpi_handle(dev);
+ if (!handle)
+ return;
+
+ status = acpi_bus_get_device(handle, &acpi_dev);
+ if (ACPI_SUCCESS(status)) {
+ acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+ ata_acpi_wake_dev, dev);
+ device_set_run_wake(&dev->sdev->sdev_gendev, true);
+ }
+}
+
+static void ata_acpi_remove_pm_notifier(struct ata_device *dev)
+{
+ struct acpi_device *acpi_dev;
+ acpi_handle handle;
+ acpi_status status;
+
+ handle = ata_dev_acpi_handle(dev);
+ if (!handle)
+ return;
+
+ status = acpi_bus_get_device(handle, &acpi_dev);
+ if (ACPI_SUCCESS(status)) {
+ device_set_run_wake(&dev->sdev->sdev_gendev, false);
+ acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+ ata_acpi_wake_dev);
+ }
+}
+
+void ata_acpi_bind(struct ata_device *dev)
+{
+ ata_acpi_add_pm_notifier(dev);
+}
+
+void ata_acpi_unbind(struct ata_device *dev)
+{
+ ata_acpi_remove_pm_notifier(dev);
+}
+
static int compat_pci_ata(struct device *dev)
{
struct pci_dev *pdev;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2222635..8ec81ca 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3445,6 +3445,7 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
if (!IS_ERR(sdev)) {
dev->sdev = sdev;
scsi_device_put(sdev);
+ ata_acpi_bind(dev);
} else {
dev->sdev = NULL;
}
@@ -3541,6 +3542,8 @@ static void ata_scsi_remove_dev(struct ata_device *dev)
mutex_lock(&ap->scsi_host->scan_mutex);
spin_lock_irqsave(ap->lock, flags);

+ ata_acpi_unbind(dev);
+
/* clearing dev->sdev is protected by host lock */
sdev = dev->sdev;
dev->sdev = NULL;
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 6aba606..b0ba924 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -118,6 +118,8 @@ extern void ata_acpi_on_disable(struct ata_device *dev);
extern void ata_acpi_set_state(struct ata_port *ap, pm_message_t state);
extern int ata_acpi_register(void);
extern void ata_acpi_unregister(void);
+extern void ata_acpi_bind(struct ata_device *dev);
+extern void ata_acpi_unbind(struct ata_device *dev);
#else
static inline void ata_acpi_dissociate(struct ata_host *host) { }
static inline int ata_acpi_on_suspend(struct ata_port *ap) { return 0; }
@@ -128,6 +130,8 @@ static inline void ata_acpi_set_state(struct ata_port *ap,
pm_message_t state) { }
static inline int ata_acpi_register(void) { return 0; }
static void ata_acpi_unregister(void) { }
+static void ata_acpi_bind(struct ata_device *dev) { }
+static void ata_acpi_unbind(struct ata_device *dev) { }
#endif

/* libata-scsi.c */
--
1.7.10

2012-06-18 06:26:46

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 08/12] libata: detect Device Attention support

Add a new flag ATA_DFLAG_DA to indicate that device supports "Device
Attention".

Acked-by: Aaron Lu <[email protected]>
Signed-off-by: Lin Ming <[email protected]>
---
drivers/ata/libata-core.c | 3 +++
include/linux/ata.h | 1 +
include/linux/libata.h | 2 ++
3 files changed, 6 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index cebf18e..ba169c4 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2374,6 +2374,9 @@ int ata_dev_configure(struct ata_device *dev)
dma_dir_string = ", DMADIR";
}

+ if (ata_id_has_da(dev->id))
+ dev->flags |= ATA_DFLAG_DA;
+
/* print device info to dmesg */
if (ata_msg_drv(ap) && print_info)
ata_dev_info(dev,
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 32df2b6..5713d3a 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -578,6 +578,7 @@ static inline int ata_is_data(u8 prot)
((u64) (id)[(n) + 0]) )

#define ata_id_cdb_intr(id) (((id)[ATA_ID_CONFIG] & 0x60) == 0x20)
+#define ata_id_has_da(id) ((id)[77] & (1 << 4))

static inline bool ata_id_has_hipm(const u16 *id)
{
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 888feef..cc22b943 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -161,6 +161,8 @@ enum {
ATA_DFLAG_DETACH = (1 << 24),
ATA_DFLAG_DETACHED = (1 << 25),

+ ATA_DFLAG_DA = (1 << 26), /* device supports Device Attention */
+
ATA_DEV_UNKNOWN = 0, /* unknown device */
ATA_DEV_ATA = 1, /* ATA device */
ATA_DEV_ATA_UNSUP = 2, /* ATA device (unsupported) */
--
1.7.10

2012-06-18 06:26:50

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 11/12] [SCSI] sr: support zero power ODD

From: Aaron Lu <[email protected]>

If there is no media inside the ODD and the ODD's tray is closed, it's
safe to omit power. When user ejects the tray by pressing the button or
inserts a disc into the slot, the ODD will gets resumed from acpi
notifier handler.

Signed-off-by: Aaron Lu <[email protected]>
Signed-off-by: Lin Ming <[email protected]>
---
drivers/ata/libata-acpi.c | 4 +-
drivers/scsi/sr.c | 128 +++++++++++++++++++++++++++++++++++++++++++-
drivers/scsi/sr.h | 2 +
include/scsi/scsi_device.h | 1 +
4 files changed, 133 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 0c54d1e..08edebf 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -985,8 +985,10 @@ static void ata_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
struct ata_device *ata_dev = context;

if (event == ACPI_NOTIFY_DEVICE_WAKE && ata_dev &&
- pm_runtime_suspended(&ata_dev->sdev->sdev_gendev))
+ pm_runtime_suspended(&ata_dev->sdev->sdev_gendev)) {
+ ata_dev->sdev->wakeup_by_user = 1;
scsi_autopm_get_device(ata_dev->sdev);
+ }
}

static void ata_acpi_add_pm_notifier(struct ata_device *dev)
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index abfefab..72488c2 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -46,6 +46,7 @@
#include <linux/mutex.h>
#include <linux/slab.h>
#include <asm/uaccess.h>
+#include <linux/pm_runtime.h>

#include <scsi/scsi.h>
#include <scsi/scsi_dbg.h>
@@ -79,6 +80,8 @@ static DEFINE_MUTEX(sr_mutex);
static int sr_probe(struct device *);
static int sr_remove(struct device *);
static int sr_done(struct scsi_cmnd *);
+static int sr_suspend(struct device *dev, pm_message_t msg);
+static int sr_resume(struct device *dev);

static struct scsi_driver sr_template = {
.owner = THIS_MODULE,
@@ -86,6 +89,8 @@ static struct scsi_driver sr_template = {
.name = "sr",
.probe = sr_probe,
.remove = sr_remove,
+ .suspend = sr_suspend,
+ .resume = sr_resume,
},
.done = sr_done,
};
@@ -167,6 +172,58 @@ static void scsi_cd_put(struct scsi_cd *cd)
mutex_unlock(&sr_ref_mutex);
}

+static int sr_suspend(struct device *dev, pm_message_t msg)
+{
+ int poweroff;
+ struct scsi_sense_hdr sshdr;
+ struct scsi_cd *cd = dev_get_drvdata(dev);
+
+ /* no action for system suspend */
+ if (msg.event == PM_EVENT_SUSPEND)
+ return 0;
+
+ /* do another TUR to see if the ODD is still ready to be powered off */
+ scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr);
+
+ if (cd->cdi.mask & CDC_CLOSE_TRAY)
+ /* no media for caddy/slot type ODD */
+ poweroff = scsi_sense_valid(&sshdr) && sshdr.asc == 0x3a;
+ else
+ /* no media and door closed for tray type ODD */
+ poweroff = scsi_sense_valid(&sshdr) && sshdr.asc == 0x3a &&
+ sshdr.ascq == 0x01;
+
+ if (!poweroff) {
+ pm_runtime_get_noresume(dev);
+ atomic_set(&cd->suspend_count, 1);
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+static int sr_resume(struct device *dev)
+{
+ struct scsi_cd *cd;
+ struct scsi_sense_hdr sshdr;
+
+ cd = dev_get_drvdata(dev);
+
+ /* get the disk ready */
+ scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr);
+
+ /* if user wakes up the ODD, eject the tray */
+ if (cd->device->wakeup_by_user) {
+ cd->device->wakeup_by_user = 0;
+ if (!(cd->cdi.mask & CDC_CLOSE_TRAY))
+ sr_tray_move(&cd->cdi, 1);
+ }
+
+ atomic_set(&cd->suspend_count, 1);
+
+ return 0;
+}
+
static unsigned int sr_get_events(struct scsi_device *sdev)
{
u8 buf[8];
@@ -201,6 +258,37 @@ static unsigned int sr_get_events(struct scsi_device *sdev)
return 0;
}

+static int sr_unit_load_done(struct scsi_cd *cd)
+{
+ u8 buf[8];
+ u8 cmd[] = { GET_EVENT_STATUS_NOTIFICATION,
+ 1, /* polled */
+ 0, 0, /* reserved */
+ 1 << 6, /* notification class: Device Busy */
+ 0, 0, /* reserved */
+ 0, sizeof(buf), /* allocation length */
+ 0, /* control */
+ };
+ struct event_header *eh = (void *)buf;
+ struct device_busy_event_desc *desc = (void *)(buf + 4);
+ struct scsi_sense_hdr sshdr;
+ int result;
+
+ result = scsi_execute_req(cd->device, cmd, DMA_FROM_DEVICE, buf,
+ sizeof(buf), &sshdr, SR_TIMEOUT, MAX_RETRIES, NULL);
+
+ if (result || be16_to_cpu(eh->data_len) < sizeof(*desc))
+ return 0;
+
+ if (eh->nea || eh->notification_class != 0x6)
+ return 0;
+
+ if (desc->device_busy_event == 2 && desc->device_busy_status == 0)
+ return 1;
+ else
+ return 0;
+}
+
/*
* This function checks to see if the media has been changed or eject
* button has been pressed. It is possible that we have already
@@ -215,12 +303,21 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
bool last_present;
struct scsi_sense_hdr sshdr;
unsigned int events;
- int ret;
+ int ret, poweroff;

/* no changer support */
if (CDSL_CURRENT != slot)
return 0;

+ if (pm_runtime_suspended(&cd->device->sdev_gendev))
+ return 0;
+
+ /* if the logical unit just finished loading/unloading, do a TUR */
+ if (cd->device->can_power_off && cd->dbml && sr_unit_load_done(cd)) {
+ events = 0;
+ goto do_tur;
+ }
+
events = sr_get_events(cd->device);
cd->get_event_changed |= events & DISK_EVENT_MEDIA_CHANGE;

@@ -270,6 +367,22 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,
cd->tur_changed = true;
}

+ if (cd->device->can_power_off && !cd->media_present) {
+ if (cd->cdi.mask & CDC_CLOSE_TRAY)
+ poweroff = 1;
+ else
+ poweroff = sshdr.ascq == 0x01;
+ /*
+ * This function might be called concurrently by a kernel
+ * thread (in-kernel polling) and old versions of udisks,
+ * to avoid put the device twice, an atomic operation is used.
+ */
+ if (poweroff && atomic_add_unless(&cd->suspend_count, -1, 0)) {
+ pm_runtime_mark_last_busy(&cd->device->sdev_gendev);
+ pm_runtime_put_autosuspend(&cd->device->sdev_gendev);
+ }
+ }
+
if (cd->ignore_get_event)
return events;

@@ -704,6 +817,15 @@ static int sr_probe(struct device *dev)
blk_queue_prep_rq(sdev->request_queue, sr_prep_fn);
sr_vendor_init(cd);

+ /* zero power support */
+ if (sdev->can_power_off) {
+ check_dbml(cd);
+ /* default delay time is 3 minutes */
+ pm_runtime_set_autosuspend_delay(dev, 180 * 1000);
+ pm_runtime_use_autosuspend(dev);
+ atomic_set(&cd->suspend_count, 1);
+ }
+
disk->driverfs_dev = &sdev->sdev_gendev;
set_capacity(disk, cd->capacity);
disk->private_data = &cd->driver;
@@ -988,6 +1110,10 @@ static int sr_remove(struct device *dev)
{
struct scsi_cd *cd = dev_get_drvdata(dev);

+ /* disable runtime pm and possibly resume the device */
+ if (!atomic_dec_and_test(&cd->suspend_count))
+ pm_runtime_get_sync(dev);
+
blk_queue_prep_rq(cd->device->request_queue, scsi_prep_fn);
del_gendisk(cd->disk);

diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 7cc40ad..fd5c550 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -49,6 +49,8 @@ typedef struct scsi_cd {
bool get_event_changed:1; /* changed according to GET_EVENT */
bool ignore_get_event:1; /* GET_EVENT is unreliable, use TUR */

+ atomic_t suspend_count; /* we should request autosuspend only once */
+
struct cdrom_device_info cdi;
/* We hold gendisk and scsi_device references on probe and use
* the refs on this kref to decide when to release them */
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 1237fac..65b9732 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -153,6 +153,7 @@ struct scsi_device {
unsigned no_read_capacity_16:1; /* Avoid READ_CAPACITY_16 cmds */
unsigned is_visible:1; /* is the device visible in sysfs */
unsigned can_power_off:1; /* Device supports runtime power off */
+ unsigned wakeup_by_user:1; /* user wakes up the ODD */

DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
struct list_head event_list; /* asserted events */
--
1.7.10

2012-06-18 06:27:13

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 10/12] [SCSI] sr: check support for device busy class events

From: Aaron Lu <[email protected]>

Signed-off-by: Aaron Lu <[email protected]>
---
drivers/scsi/sr.c | 23 +++++++++++++++++++++++
drivers/scsi/sr.h | 1 +
include/linux/cdrom.h | 43 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5fc97d2..abfefab 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -101,6 +101,7 @@ static DEFINE_MUTEX(sr_ref_mutex);
static int sr_open(struct cdrom_device_info *, int);
static void sr_release(struct cdrom_device_info *);

+static void check_dbml(struct scsi_cd *);
static void get_sectorsize(struct scsi_cd *);
static void get_capabilities(struct scsi_cd *);

@@ -728,6 +729,28 @@ static int sr_probe(struct device *dev)
return error;
}

+static void check_dbml(struct scsi_cd *cd)
+{
+ struct packet_command cgc;
+ unsigned char buffer[16];
+ struct rm_feature_desc *rfd;
+
+ init_cdrom_command(&cgc, buffer, sizeof(buffer), CGC_DATA_READ);
+ cgc.cmd[0] = GPCMD_GET_CONFIGURATION;
+ cgc.cmd[3] = CDF_RM;
+ cgc.cmd[8] = sizeof(buffer);
+ cgc.quiet = 1;
+
+ if (cd->cdi.ops->generic_packet(&cd->cdi, &cgc))
+ return;
+
+ rfd = (struct rm_feature_desc *)&buffer[sizeof(struct feature_header)];
+ if (be16_to_cpu(rfd->feature_code) != CDF_RM)
+ return;
+
+ if (rfd->dbml)
+ cd->dbml = 1;
+}

static void get_sectorsize(struct scsi_cd *cd)
{
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 37c8f6b..7cc40ad 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -41,6 +41,7 @@ typedef struct scsi_cd {
unsigned readcd_known:1; /* drive supports READ_CD (0xbe) */
unsigned readcd_cdda:1; /* reading audio data using READ_CD */
unsigned media_present:1; /* media is present */
+ unsigned dbml:1; /* generates device busy class events */

/* GET_EVENT spurious event handling, blk layer guarantees exclusion */
int tur_mismatch; /* nr of get_event TUR mismatches */
diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
index dfd7f18..25f305c 100644
--- a/include/linux/cdrom.h
+++ b/include/linux/cdrom.h
@@ -727,6 +727,7 @@ struct request_sense {
/*
* feature profile
*/
+#define CDF_RM 0x0003 /* "Removeable Medium" */
#define CDF_RWRT 0x0020 /* "Random Writable" */
#define CDF_HWDM 0x0024 /* "Hardware Defect Management" */
#define CDF_MRW 0x0028
@@ -739,6 +740,48 @@ struct request_sense {
#define CDM_MRW_BGFORMAT_ACTIVE 2
#define CDM_MRW_BGFORMAT_COMPLETE 3

+/* Removable medium feature descriptor */
+struct rm_feature_desc {
+ __be16 feature_code;
+#if defined(__BIG_ENDIAN_BITFIELD)
+ __u8 reserved1 : 2;
+ __u8 feature_version : 4;
+ __u8 persistent : 1;
+ __u8 curr : 1;
+#elif defined(__LITTLE_ENDIAN_BITFIELD)
+ __u8 curr : 1;
+ __u8 persistent : 1;
+ __u8 feature_version : 4;
+ __u8 reserved1 : 2;
+#endif
+ __u8 add_len;
+#if defined(__BIG_ENDIAN_BITFIELD)
+ __u8 mech_type : 3;
+ __u8 load : 1;
+ __u8 eject : 1;
+ __u8 pvnt_jmpr : 1;
+ __u8 dbml : 1;
+ __u8 lock : 1;
+#elif defined(__LITTLE_ENDIAN_BITFIELD)
+ __u8 lock : 1;
+ __u8 dbml : 1;
+ __u8 pvnt_jmpr : 1;
+ __u8 eject : 1;
+ __u8 load : 1;
+ __u8 mech_type : 3;
+#endif
+ __u8 reserved2;
+ __u8 reserved3;
+ __u8 reserved4;
+};
+
+struct device_busy_event_desc {
+ __u8 device_busy_event : 4;
+ __u8 reserved1 : 4;
+ __u8 device_busy_status;
+ __u8 time;
+};
+
/*
* mrw address spaces
*/
--
1.7.10

2012-06-18 06:27:10

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 12/12] [SCSI] sr: make sure ODD is in resumed state in block ioctl

From: Aaron Lu <[email protected]>

When application tries to access the ODD's block device by ioctl,
make sure ODD is in an active state.

Signed-off-by: Aaron Lu <[email protected]>
Signed-off-by: Lin Ming <[email protected]>
---
drivers/scsi/sr.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 72488c2..3da0879 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -654,6 +654,13 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
void __user *argp = (void __user *)arg;
int ret;

+ /* Make sure the ODD is not suspended */
+ ret = pm_runtime_get_sync(&sdev->sdev_gendev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(&sdev->sdev_gendev);
+ return -EACCES;
+ }
+
mutex_lock(&sr_mutex);

/*
@@ -685,6 +692,8 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,

out:
mutex_unlock(&sr_mutex);
+ pm_runtime_mark_last_busy(&cd->device->sdev_gendev);
+ pm_runtime_put_autosuspend(&cd->device->sdev_gendev);
return ret;
}

--
1.7.10

2012-06-18 06:28:09

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 09/12] libata: tell scsi layer device supports runtime power off

From: Aaron Lu <[email protected]>

If ATA device supports "Device Attention", then tell scsi layer that
the device supports runtime power off.

Signed-off-by: Aaron Lu <[email protected]>
Signed-off-by: Lin Ming <[email protected]>
---
drivers/ata/libata-acpi.c | 32 ++++++++++++++++++++++++++++----
include/scsi/scsi_device.h | 1 +
2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 9287593..0c54d1e 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -1000,7 +1000,10 @@ static void ata_acpi_add_pm_notifier(struct ata_device *dev)
return;

status = acpi_bus_get_device(handle, &acpi_dev);
- if (ACPI_SUCCESS(status)) {
+ if (ACPI_FAILURE(status))
+ return;
+
+ if (dev->sdev->can_power_off) {
acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
ata_acpi_wake_dev, dev);
device_set_run_wake(&dev->sdev->sdev_gendev, true);
@@ -1018,7 +1021,10 @@ static void ata_acpi_remove_pm_notifier(struct ata_device *dev)
return;

status = acpi_bus_get_device(handle, &acpi_dev);
- if (ACPI_SUCCESS(status)) {
+ if (ACPI_FAILURE(status))
+ return;
+
+ if (dev->sdev->can_power_off) {
device_set_run_wake(&dev->sdev->sdev_gendev, false);
acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
ata_acpi_wake_dev);
@@ -1102,10 +1108,13 @@ static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle)
static int ata_acpi_bind_device(struct device *dev, int channel, int id,
acpi_handle *handle)
{
- struct device *host = dev->parent->parent;
- struct Scsi_Host *shost = dev_to_shost(host);
+ struct scsi_device *sdev = to_scsi_device(dev);
+ struct Scsi_Host *shost = sdev->host;
struct ata_port *ap = ata_shost_to_port(shost);
struct ata_device *ata_dev;
+ acpi_status status;
+ struct acpi_device *acpi_dev;
+ struct acpi_device_power_state *states;

if (ap->flags & ATA_FLAG_ACPI_SATA)
ata_dev = &ap->link.device[channel];
@@ -1117,6 +1126,21 @@ static int ata_acpi_bind_device(struct device *dev, int channel, int id,
if (!*handle)
return -ENODEV;

+ status = acpi_bus_get_device(*handle, &acpi_dev);
+ if (ACPI_FAILURE(status))
+ return 0;
+
+ /*
+ * If firmware has _PS3 or _PR3 for this device,
+ * and this ata ODD device support device attention,
+ * it means this device can be powered off
+ */
+ states = acpi_dev->power.states;
+ if ((states[ACPI_STATE_D3_HOT].flags.valid ||
+ states[ACPI_STATE_D3_COLD].flags.explicit_set) &&
+ ata_dev->flags & ATA_DFLAG_DA)
+ sdev->can_power_off = 1;
+
return 0;
}

diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 6efb2e1..1237fac 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -152,6 +152,7 @@ struct scsi_device {
unsigned no_read_disc_info:1; /* Avoid READ_DISC_INFO cmds */
unsigned no_read_capacity_16:1; /* Avoid READ_CAPACITY_16 cmds */
unsigned is_visible:1; /* is the device visible in sysfs */
+ unsigned can_power_off:1; /* Device supports runtime power off */

DECLARE_BITMAP(supported_events, SDEV_EVT_MAXBITS); /* supported events */
struct list_head event_list; /* asserted events */
--
1.7.10

2012-06-18 06:28:53

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 07/12] libata-acpi: register/unregister device to/from power resource

Signed-off-by: Lin Ming <[email protected]>
---
drivers/acpi/power.c | 2 ++
drivers/ata/libata-acpi.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+)

diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index dd6d6a3..eb640874 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -390,6 +390,7 @@ void acpi_power_resource_unregister_device(struct device *dev, acpi_handle handl
__acpi_power_resource_unregister_device(dev,
list->handles[i]);
}
+EXPORT_SYMBOL_GPL(acpi_power_resource_unregister_device);

static int __acpi_power_resource_register_device(
struct acpi_power_managed_device *powered_device, acpi_handle handle)
@@ -460,6 +461,7 @@ int acpi_power_resource_register_device(struct device *dev, acpi_handle handle)
printk(KERN_WARNING PREFIX "Invalid Power Resource to register!");
return -ENODEV;
}
+EXPORT_SYMBOL_GPL(acpi_power_resource_register_device);

/**
* acpi_device_sleep_wake - execute _DSW (Device Sleep Wake) or (deprecated in
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 3f7d376..9287593 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -1025,14 +1025,46 @@ static void ata_acpi_remove_pm_notifier(struct ata_device *dev)
}
}

+static void ata_acpi_register_power_resource(struct ata_device *dev)
+{
+ struct scsi_device *sdev = dev->sdev;
+ acpi_handle handle;
+ struct device *device;
+
+ handle = ata_dev_acpi_handle(dev);
+ if (!handle)
+ return;
+
+ device = &sdev->sdev_gendev;
+
+ acpi_power_resource_register_device(device, handle);
+}
+
+static void ata_acpi_unregister_power_resource(struct ata_device *dev)
+{
+ struct scsi_device *sdev = dev->sdev;
+ acpi_handle handle;
+ struct device *device;
+
+ handle = ata_dev_acpi_handle(dev);
+ if (!handle)
+ return;
+
+ device = &sdev->sdev_gendev;
+
+ acpi_power_resource_unregister_device(device, handle);
+}
+
void ata_acpi_bind(struct ata_device *dev)
{
ata_acpi_add_pm_notifier(dev);
+ ata_acpi_register_power_resource(dev);
}

void ata_acpi_unbind(struct ata_device *dev)
{
ata_acpi_remove_pm_notifier(dev);
+ ata_acpi_unregister_power_resource(dev);
}

static int compat_pci_ata(struct device *dev)
--
1.7.10

2012-06-18 06:29:14

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 05/12] libata-acpi: set acpi state for SATA port

Currently, ata_acpi_set_state() only sets acpi sate for IDE port.
Remove this limitation.

Acked-by: Aaron Lu <[email protected]>
Signed-off-by: Lin Ming <[email protected]>
---
drivers/ata/libata-acpi.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 98826e96..2887eb0 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -852,23 +852,26 @@ void ata_acpi_on_resume(struct ata_port *ap)
void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)
{
struct ata_device *dev;
-
- if (!ata_ap_acpi_handle(ap) || (ap->flags & ATA_FLAG_ACPI_SATA))
- return;
+ acpi_handle handle;
+ int acpi_state;

/* channel first and then drives for power on and vica versa
for power off */
- if (state.event == PM_EVENT_ON)
- acpi_bus_set_power(ata_ap_acpi_handle(ap), ACPI_STATE_D0);
+ handle = ata_ap_acpi_handle(ap);
+ if (handle && state.event == PM_EVENT_ON)
+ acpi_bus_set_power(handle, ACPI_STATE_D0);

ata_for_each_dev(dev, &ap->link, ENABLED) {
- if (ata_dev_acpi_handle(dev))
- acpi_bus_set_power(ata_dev_acpi_handle(dev),
+ handle = ata_dev_acpi_handle(dev);
+ if (handle)
+ acpi_bus_set_power(handle,
state.event == PM_EVENT_ON ?
ACPI_STATE_D0 : ACPI_STATE_D3);
}
- if (state.event != PM_EVENT_ON)
- acpi_bus_set_power(ata_ap_acpi_handle(ap), ACPI_STATE_D3);
+
+ handle = ata_ap_acpi_handle(ap);
+ if (handle && state.event != PM_EVENT_ON)
+ acpi_bus_set_power(handle, ACPI_STATE_D3);
}

/**
--
1.7.10

2012-06-18 06:26:35

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 02/12] libata: bind the Linux device tree to the ACPI device tree

From: Matthew Garrett <[email protected]>

Associate the ACPI device tree and libata devices.
This patch uses the generic ACPI glue framework to do so.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: Holger Macht <[email protected]>
Signed-off-by: Lin Ming <[email protected]>
---

Changelog:

- fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu)
- rename is_pci_ata to compat_pci_ata (Alan Cox)

drivers/acpi/glue.c | 4 +-
drivers/ata/libata-acpi.c | 125 +++++++++++++++++++++++++++++++++++++++++++++
drivers/ata/libata-core.c | 3 ++
drivers/ata/libata.h | 4 ++
4 files changed, 134 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 1564e09..18d6812 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -39,6 +39,7 @@ int register_acpi_bus_type(struct acpi_bus_type *type)
}
return -ENODEV;
}
+EXPORT_SYMBOL(register_acpi_bus_type);

int unregister_acpi_bus_type(struct acpi_bus_type *type)
{
@@ -54,6 +55,7 @@ int unregister_acpi_bus_type(struct acpi_bus_type *type)
}
return -ENODEV;
}
+EXPORT_SYMBOL(unregister_acpi_bus_type);

static struct acpi_bus_type *acpi_get_bus_type(struct bus_type *type)
{
@@ -69,7 +71,6 @@ static struct acpi_bus_type *acpi_get_bus_type(struct bus_type *type)
up_read(&bus_type_sem);
return ret;
}
-EXPORT_SYMBOL_GPL(register_acpi_bus_type);

static int acpi_find_bridge_device(struct device *dev, acpi_handle * handle)
{
@@ -86,7 +87,6 @@ static int acpi_find_bridge_device(struct device *dev, acpi_handle * handle)
up_read(&bus_type_sem);
return ret;
}
-EXPORT_SYMBOL_GPL(unregister_acpi_bus_type);

/* Get device's handler per its address under its parent */
struct acpi_find_child {
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index bb7c5f1..ae93593 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -47,6 +47,39 @@ static void ata_acpi_clear_gtf(struct ata_device *dev)
dev->gtf_cache = NULL;
}

+static acpi_handle ap_acpi_handle(struct ata_port *ap)
+{
+ if (ap->flags & ATA_FLAG_ACPI_SATA)
+ return NULL;
+
+ /*
+ * If acpi bind operation has already happened, we can get the handle
+ * for the port by checking the corresponding scsi_host device's
+ * firmware node, otherwise we will need to find out the handle from
+ * its parent's acpi node.
+ */
+ if (ap->scsi_host)
+ return DEVICE_ACPI_HANDLE(&ap->scsi_host->shost_gendev);
+ else
+ return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev),
+ ap->port_no);
+}
+
+static acpi_handle dev_acpi_handle(struct ata_device *dev)
+{
+ acpi_integer adr;
+ struct ata_port *ap = dev->link->ap;
+
+ if (ap->flags & ATA_FLAG_ACPI_SATA) {
+ if (!sata_pmp_attached(ap))
+ adr = SATA_ADR(ap->port_no, NO_PORT_MULT);
+ else
+ adr = SATA_ADR(ap->port_no, dev->link->pmp);
+ return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev), adr);
+ } else
+ return acpi_get_child(ap_acpi_handle(ap), dev->devno);
+}
+
/**
* ata_acpi_associate_sata_port - associate SATA port with ACPI objects
* @ap: target SATA port
@@ -1018,3 +1051,95 @@ void ata_acpi_on_disable(struct ata_device *dev)
{
ata_acpi_clear_gtf(dev);
}
+
+static int compat_pci_ata(struct device *dev)
+{
+ struct pci_dev *pdev;
+
+ if (!is_pci_dev(dev))
+ return 0;
+
+ pdev = to_pci_dev(dev);
+
+ if ((pdev->class >> 8) != PCI_CLASS_STORAGE_SATA &&
+ (pdev->class >> 8) != PCI_CLASS_STORAGE_IDE)
+ return 0;
+
+ return 1;
+}
+
+static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle)
+{
+ struct Scsi_Host *shost = dev_to_shost(dev);
+ struct ata_port *ap = ata_shost_to_port(shost);
+
+ if (ap->flags & ATA_FLAG_ACPI_SATA)
+ return -ENODEV;
+
+ *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), ap->port_no);
+
+ if (!*handle)
+ return -ENODEV;
+
+ return 0;
+}
+
+static int ata_acpi_bind_device(struct device *dev, int channel, int id,
+ acpi_handle *handle)
+{
+ struct device *host = dev->parent->parent;
+ struct Scsi_Host *shost = dev_to_shost(host);
+ struct ata_port *ap = ata_shost_to_port(shost);
+ struct ata_device *ata_dev;
+
+ if (ap->flags & ATA_FLAG_ACPI_SATA)
+ ata_dev = &ap->link.device[channel];
+ else
+ ata_dev = &ap->link.device[id];
+
+ *handle = dev_acpi_handle(ata_dev);
+
+ if (!*handle)
+ return -ENODEV;
+
+ return 0;
+}
+
+static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
+{
+ unsigned int host, channel, id, lun;
+
+ if (sscanf(dev_name(dev), "host%u", &host) == 1) {
+ if (!compat_pci_ata(dev->parent))
+ return -ENODEV;
+
+ return ata_acpi_bind_host(dev, host, handle);
+ } else if (sscanf(dev_name(dev), "%d:%d:%d:%d",
+ &host, &channel, &id, &lun) == 4) {
+ if (!compat_pci_ata(dev->parent->parent->parent))
+ return -ENODEV;
+
+ return ata_acpi_bind_device(dev, channel, id, handle);
+ } else
+ return -ENODEV;
+}
+
+static int ata_acpi_find_dummy(struct device *dev, acpi_handle *handle)
+{
+ return -ENODEV;
+}
+
+static struct acpi_bus_type ata_acpi_bus = {
+ .find_bridge = ata_acpi_find_dummy,
+ .find_device = ata_acpi_find_device,
+};
+
+int ata_acpi_register(void)
+{
+ return scsi_register_acpi_bus_type(&ata_acpi_bus);
+}
+
+void ata_acpi_unregister(void)
+{
+ scsi_unregister_acpi_bus_type(&ata_acpi_bus);
+}
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index cece3a4..5628bf6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6513,6 +6513,8 @@ static int __init ata_init(void)

ata_parse_force_param();

+ ata_acpi_register();
+
rc = ata_sff_init();
if (rc) {
kfree(ata_force_tbl);
@@ -6539,6 +6541,7 @@ static void __exit ata_exit(void)
ata_release_transport(ata_scsi_transport_template);
libata_transport_exit();
ata_sff_exit();
+ ata_acpi_unregister();
kfree(ata_force_tbl);
}

diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 9d0fd0b..3df3362 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -119,6 +119,8 @@ extern void ata_acpi_on_resume(struct ata_port *ap);
extern int ata_acpi_on_devcfg(struct ata_device *dev);
extern void ata_acpi_on_disable(struct ata_device *dev);
extern void ata_acpi_set_state(struct ata_port *ap, pm_message_t state);
+extern int ata_acpi_register(void);
+extern void ata_acpi_unregister(void);
#else
static inline void ata_acpi_associate_sata_port(struct ata_port *ap) { }
static inline void ata_acpi_associate(struct ata_host *host) { }
@@ -129,6 +131,8 @@ static inline int ata_acpi_on_devcfg(struct ata_device *dev) { return 0; }
static inline void ata_acpi_on_disable(struct ata_device *dev) { }
static inline void ata_acpi_set_state(struct ata_port *ap,
pm_message_t state) { }
+static inline int ata_acpi_register(void) { return 0; }
+static void ata_acpi_unregister(void) { }
#endif

/* libata-scsi.c */
--
1.7.10

2012-06-18 06:29:50

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 04/12] libata: use correct PCI devices

From: Holger Macht <[email protected]>

Commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a made ata ports parent
devices of scsi hosts, so we need to go yet another level up to be able
to use the correct PCI devices.

Signed-off-by: Holger Macht <[email protected]>
Signed-off-by: Lin Ming <[email protected]>
---
drivers/ata/libata-acpi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index adc6336..98826e96 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -987,7 +987,7 @@ static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle)
if (ap->flags & ATA_FLAG_ACPI_SATA)
return -ENODEV;

- *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), ap->port_no);
+ *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent->parent), ap->port_no);

if (!*handle)
return -ENODEV;
@@ -1021,13 +1021,13 @@ static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
unsigned int host, channel, id, lun;

if (sscanf(dev_name(dev), "host%u", &host) == 1) {
- if (!compat_pci_ata(dev->parent))
+ if (!compat_pci_ata(dev->parent->parent))
return -ENODEV;

return ata_acpi_bind_host(dev, host, handle);
} else if (sscanf(dev_name(dev), "%d:%d:%d:%d",
&host, &channel, &id, &lun) == 4) {
- if (!compat_pci_ata(dev->parent->parent->parent))
+ if (!compat_pci_ata(dev->parent->parent->parent->parent))
return -ENODEV;

return ata_acpi_bind_device(dev, channel, id, handle);
--
1.7.10

2012-06-18 06:30:19

by Lin Ming

[permalink] [raw]
Subject: [PATCH v5 01/12] [SCSI]: add wrapper to access and set scsi_bus_type in struct acpi_bus_type

From: Holger Macht <[email protected]>

For being able to bind ata devices against acpi devices, scsi_bus_type
needs to be set as bus in struct acpi_bus_type. So add wrapper to
scsi_lib to accomplish that.

Signed-off-by: Holger Macht <[email protected]>
Signed-off-by: Lin Ming <[email protected]>
---
drivers/scsi/scsi_lib.c | 17 +++++++++++++++++
include/scsi/scsi.h | 10 ++++++++++
2 files changed, 27 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6dfb978..08f1e29 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -68,6 +68,23 @@ static struct scsi_host_sg_pool scsi_sg_pools[] = {

struct kmem_cache *scsi_sdb_cache;

+#ifdef CONFIG_ACPI
+#include <acpi/acpi_bus.h>
+
+int scsi_register_acpi_bus_type(struct acpi_bus_type *bus)
+{
+ bus->bus = &scsi_bus_type;
+ return register_acpi_bus_type(bus);
+}
+EXPORT_SYMBOL_GPL(scsi_register_acpi_bus_type);
+
+void scsi_unregister_acpi_bus_type(struct acpi_bus_type *bus)
+{
+ unregister_acpi_bus_type(bus);
+}
+EXPORT_SYMBOL_GPL(scsi_unregister_acpi_bus_type);
+#endif
+
/*
* When to reinvoke queueing after a resource shortage. It's 3 msecs to
* not change behaviour from the previous unplug mechanism, experimentation
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index f34a5a8..4527b3a 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -214,6 +214,16 @@ scsi_command_size(const unsigned char *cmnd)
scsi_varlen_cdb_length(cmnd) : COMMAND_SIZE(cmnd[0]);
}

+#ifdef CONFIG_ACPI
+struct acpi_bus_type;
+
+extern int
+scsi_register_acpi_bus_type(struct acpi_bus_type *bus);
+
+extern void
+scsi_unregister_acpi_bus_type(struct acpi_bus_type *bus);
+#endif
+
/*
* SCSI Architecture Model (SAM) Status codes. Taken from SAM-3 draft
* T10/1561-D Revision 4 Draft dated 7th November 2002.
--
1.7.10

2012-06-18 06:33:09

by Lin Ming

[permalink] [raw]
Subject: Re: [PATCH v5 00/12] SATA ZPODD support

(The subject should be: [PATCH v5 00/12] SATA ZPODD support)

On Mon, Jun 18, 2012 at 2:25 PM, Lin Ming <[email protected]> wrote:
> Hi all,
>
> This is the v5 patches to add SATA ZPODD support, to try it:
> git pull git://git.kernel.org/pub/scm/linux/kernel/git/mlin/linux.git zpodd
>
> Holger and Mathtew,
> Patch 1 to Patch 4 are the libata acpi binding patches from you.
> I removed dock related code from your original version.
> Would you help to review?
>
> v5:
> - rebase to v3.5-rc3
> - fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu)
> - rename is_pci_ata to compat_pci_ata (Alan Cox)
>
> v4:
> https://lkml.org/lkml/2012/5/28/11
> - Includes libata acpi binding patches from Holger Macht and Matthew Garrett.
> - tell scsi layer device supports runtime power off
> - check support for device busy class events
>
> v3:
> https://lkml.org/lkml/2012/3/28/20
> - Split the ACPI D3Cold state support patches out
> - Adds "Device Attention" bit check
>
> v2:
> https://lkml.org/lkml/2012/3/1/61
> - _PR3 indicates D3Cold support
> - move can_power_off flag to pm_subsys_data
> - allow all combinations of power resource and device
> - split patches into smaller ones to make review easy
>
> v1:
> https://lkml.org/lkml/2012/2/13/86
>
>
> Aaron Lu (4):
> ? ? ?libata: tell scsi layer device supports runtime power off
> ? ? ?[SCSI] sr: check support for device busy class events
> ? ? ?[SCSI] sr: support zero power ODD
> ? ? ?[SCSI] sr: make sure ODD is in resumed state in block ioctl
>
> Holger Macht (2):
> ? ? ?[SCSI]: add wrapper to access and set scsi_bus_type in struct acpi_bus_type
> ? ? ?libata: use correct PCI devices
>
> Lin Ming (4):
> ? ? ?libata-acpi: set acpi state for SATA port
> ? ? ?libata-acpi: add ata port runtime D3Cold support
> ? ? ?libata-acpi: register/unregister device to/from power resource
> ? ? ?libata: detect Device Attention support
>
> Matthew Garrett (2):
> ? ? ?libata: bind the Linux device tree to the ACPI device tree
> ? ? ?libata: migrate ACPI code over to new bindings
>
>
> ?drivers/acpi/glue.c ? ? ? ?| ? ?4 +-
> ?drivers/acpi/power.c ? ? ? | ? ?2 +
> ?drivers/ata/libata-acpi.c ?| ?389 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------
> ?drivers/ata/libata-core.c ?| ? ?9 ++-
> ?drivers/ata/libata-pmp.c ? | ? ?4 -
> ?drivers/ata/libata-scsi.c ?| ? ?3 +
> ?drivers/ata/libata.h ? ? ? | ? 13 +--
> ?drivers/ata/pata_acpi.c ? ?| ? ?4 +-
> ?drivers/scsi/scsi_lib.c ? ?| ? 17 ++++
> ?drivers/scsi/sr.c ? ? ? ? ?| ?160 +++++++++++++++++++++++++++++++++++-
> ?drivers/scsi/sr.h ? ? ? ? ?| ? ?3 +
> ?include/linux/ata.h ? ? ? ?| ? ?1 +
> ?include/linux/cdrom.h ? ? ?| ? 43 ++++++++++
> ?include/linux/libata.h ? ? | ? ?9 +--
> ?include/scsi/scsi.h ? ? ? ?| ? 10 +++
> ?include/scsi/scsi_device.h | ? ?2 +
> ?16 files changed, 534 insertions(+), 139 deletions(-)

2012-06-18 11:38:07

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH v5 04/12] libata: use correct PCI devices

On 18-06-2012 10:25, Lin Ming wrote:

> From: Holger Macht <[email protected]>

> Commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a

Please also specify that commit's summary in parens.

> made ata ports parent
> devices of scsi hosts, so we need to go yet another level up to be able
> to use the correct PCI devices.

> Signed-off-by: Holger Macht <[email protected]>
> Signed-off-by: Lin Ming <[email protected]>

MBR, Sergei

2012-06-18 12:03:50

by Lin Ming

[permalink] [raw]
Subject: Re: [PATCH v5 04/12] libata: use correct PCI devices

On Mon, Jun 18, 2012 at 7:37 PM, Sergei Shtylyov <[email protected]> wrote:
> On 18-06-2012 10:25, Lin Ming wrote:
>
>> From: Holger Macht <[email protected]>
>
>
>> Commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a
>
>
> ? Please also specify that commit's summary in parens.

OK.

Thanks,
Lin Ming

>
>
>> made ata ports parent
>> devices of scsi hosts, so we need to go yet another level up to be able
>> to use the correct PCI devices.
>
>
>> Signed-off-by: Holger Macht <[email protected]>
>> Signed-off-by: Lin Ming <[email protected]>
>
>
> MBR, Sergei

2012-06-20 23:50:54

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH v5 02/12] libata: bind the Linux device tree to the ACPI device tree

On Sun, Jun 17, 2012 at 11:25 PM, Lin Ming <[email protected]> wrote:
> From: Matthew Garrett <[email protected]>
>
> Associate the ACPI device tree and libata devices.
> This patch uses the generic ACPI glue framework to do so.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> Signed-off-by: Holger Macht <[email protected]>
> Signed-off-by: Lin Ming <[email protected]>
> ---
>
> Changelog:
>
> - fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu)
> - rename is_pci_ata to compat_pci_ata (Alan Cox)
>
> ?drivers/acpi/glue.c ? ? ? | ? ?4 +-
> ?drivers/ata/libata-acpi.c | ?125 +++++++++++++++++++++++++++++++++++++++++++++
> ?drivers/ata/libata-core.c | ? ?3 ++
> ?drivers/ata/libata.h ? ? ?| ? ?4 ++
> ?4 files changed, 134 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
> index 1564e09..18d6812 100644
> --- a/drivers/acpi/glue.c
> +++ b/drivers/acpi/glue.c
[..]
> +static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle)
> +{
> + ? ? ? struct Scsi_Host *shost = dev_to_shost(dev);
> + ? ? ? struct ata_port *ap = ata_shost_to_port(shost);
> +
> + ? ? ? if (ap->flags & ATA_FLAG_ACPI_SATA)
> + ? ? ? ? ? ? ? return -ENODEV;
> +
> + ? ? ? *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), ap->port_no);
> +
> + ? ? ? if (!*handle)
> + ? ? ? ? ? ? ? return -ENODEV;
> +
> + ? ? ? return 0;
> +}

I think this patch should be squashed with patch 4, right? That is if
we keep the hard-coded ->parent chasing...

> +static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
> +{
> + ? ? ? unsigned int host, channel, id, lun;
> +
> + ? ? ? if (sscanf(dev_name(dev), "host%u", &host) == 1) {
> + ? ? ? ? ? ? ? if (!compat_pci_ata(dev->parent))
> + ? ? ? ? ? ? ? ? ? ? ? return -ENODEV;
> +
> + ? ? ? ? ? ? ? return ata_acpi_bind_host(dev, host, handle);
> + ? ? ? } else if (sscanf(dev_name(dev), "%d:%d:%d:%d",
> + ? ? ? ? ? ? ? ? ? ? ? &host, &channel, &id, &lun) == 4) {
> + ? ? ? ? ? ? ? if (!compat_pci_ata(dev->parent->parent->parent))
> + ? ? ? ? ? ? ? ? ? ? ? return -ENODEV;

...this looks like it should be using a dev_to_ata_port() helper which
like dev_to_shost() skips the need to remember just how many parents
until we get back to our ata_port, scsi_host, etc.

--
Dan

2012-06-21 01:26:50

by Lin Ming

[permalink] [raw]
Subject: Re: [PATCH v5 02/12] libata: bind the Linux device tree to the ACPI device tree

On Wed, 2012-06-20 at 16:50 -0700, Dan Williams wrote:
> On Sun, Jun 17, 2012 at 11:25 PM, Lin Ming <[email protected]> wrote:
> > From: Matthew Garrett <[email protected]>
> >
> > Associate the ACPI device tree and libata devices.
> > This patch uses the generic ACPI glue framework to do so.
> >
> > Signed-off-by: Matthew Garrett <[email protected]>
> > Signed-off-by: Holger Macht <[email protected]>
> > Signed-off-by: Lin Ming <[email protected]>
> > ---
> >
> > Changelog:
> >
> > - fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu)
> > - rename is_pci_ata to compat_pci_ata (Alan Cox)
> >
> > drivers/acpi/glue.c | 4 +-
> > drivers/ata/libata-acpi.c | 125 +++++++++++++++++++++++++++++++++++++++++++++
> > drivers/ata/libata-core.c | 3 ++
> > drivers/ata/libata.h | 4 ++
> > 4 files changed, 134 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
> > index 1564e09..18d6812 100644
> > --- a/drivers/acpi/glue.c
> > +++ b/drivers/acpi/glue.c
> [..]
> > +static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle)
> > +{
> > + struct Scsi_Host *shost = dev_to_shost(dev);
> > + struct ata_port *ap = ata_shost_to_port(shost);
> > +
> > + if (ap->flags & ATA_FLAG_ACPI_SATA)
> > + return -ENODEV;
> > +
> > + *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), ap->port_no);
> > +
> > + if (!*handle)
> > + return -ENODEV;
> > +
> > + return 0;
> > +}
>
> I think this patch should be squashed with patch 4, right? That is if
> we keep the hard-coded ->parent chasing...

Right. Will merge patch 4.

>
> > +static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
> > +{
> > + unsigned int host, channel, id, lun;
> > +
> > + if (sscanf(dev_name(dev), "host%u", &host) == 1) {
> > + if (!compat_pci_ata(dev->parent))
> > + return -ENODEV;
> > +
> > + return ata_acpi_bind_host(dev, host, handle);
> > + } else if (sscanf(dev_name(dev), "%d:%d:%d:%d",
> > + &host, &channel, &id, &lun) == 4) {
> > + if (!compat_pci_ata(dev->parent->parent->parent))
> > + return -ENODEV;
>
> ...this looks like it should be using a dev_to_ata_port() helper which
> like dev_to_shost() skips the need to remember just how many parents
> until we get back to our ata_port, scsi_host, etc.

Oh, no.

compat_pci_ata checks whether the controller(rather than shost or ata
port) is pci SATA/IDE controller.

As in patch 4:
compat_pci_ata(dev->parent->parent->parent->parent)

The "dev" is ata port, and dev->parent->parent->parent->parent points to
the controller dev.

This looks ugly. How about add two helpers?
shost_to_controller(dev) and ata_port_to_controller(dev)

Thanks,
Lin Ming

>
> --
> Dan

2012-06-21 03:59:25

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH v5 02/12] libata: bind the Linux device tree to the ACPI device tree

On Wed, Jun 20, 2012 at 6:26 PM, Lin Ming <[email protected]> wrote:
> On Wed, 2012-06-20 at 16:50 -0700, Dan Williams wrote:
>> On Sun, Jun 17, 2012 at 11:25 PM, Lin Ming <[email protected]> wrote:
>> > From: Matthew Garrett <[email protected]>
>> >
>> > Associate the ACPI device tree and libata devices.
>> > This patch uses the generic ACPI glue framework to do so.
>> >
>> > Signed-off-by: Matthew Garrett <[email protected]>
>> > Signed-off-by: Holger Macht <[email protected]>
>> > Signed-off-by: Lin Ming <[email protected]>
>> > ---
>> >
>> > Changelog:
>> >
>> > - fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu)
>> > - rename is_pci_ata to compat_pci_ata (Alan Cox)
>> >
>> > ?drivers/acpi/glue.c ? ? ? | ? ?4 +-
>> > ?drivers/ata/libata-acpi.c | ?125 +++++++++++++++++++++++++++++++++++++++++++++
>> > ?drivers/ata/libata-core.c | ? ?3 ++
>> > ?drivers/ata/libata.h ? ? ?| ? ?4 ++
>> > ?4 files changed, 134 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
>> > index 1564e09..18d6812 100644
>> > --- a/drivers/acpi/glue.c
>> > +++ b/drivers/acpi/glue.c
>> [..]
>> > +static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle)
>> > +{
>> > + ? ? ? struct Scsi_Host *shost = dev_to_shost(dev);
>> > + ? ? ? struct ata_port *ap = ata_shost_to_port(shost);
>> > +
>> > + ? ? ? if (ap->flags & ATA_FLAG_ACPI_SATA)
>> > + ? ? ? ? ? ? ? return -ENODEV;
>> > +
>> > + ? ? ? *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), ap->port_no);
>> > +
>> > + ? ? ? if (!*handle)
>> > + ? ? ? ? ? ? ? return -ENODEV;
>> > +
>> > + ? ? ? return 0;
>> > +}
>>
>> I think this patch should be squashed with patch 4, right? ?That is if
>> we keep the hard-coded ->parent chasing...
>
> Right. Will merge patch 4.
>
>>
>> > +static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
>> > +{
>> > + ? ? ? unsigned int host, channel, id, lun;
>> > +
>> > + ? ? ? if (sscanf(dev_name(dev), "host%u", &host) == 1) {
>> > + ? ? ? ? ? ? ? if (!compat_pci_ata(dev->parent))
>> > + ? ? ? ? ? ? ? ? ? ? ? return -ENODEV;
>> > +
>> > + ? ? ? ? ? ? ? return ata_acpi_bind_host(dev, host, handle);
>> > + ? ? ? } else if (sscanf(dev_name(dev), "%d:%d:%d:%d",
>> > + ? ? ? ? ? ? ? ? ? ? ? &host, &channel, &id, &lun) == 4) {
>> > + ? ? ? ? ? ? ? if (!compat_pci_ata(dev->parent->parent->parent))
>> > + ? ? ? ? ? ? ? ? ? ? ? return -ENODEV;
>>
>> ...this looks like it should be using a dev_to_ata_port() helper which
>> like dev_to_shost() skips the need to remember just how many parents
>> until we get back to our ata_port, scsi_host, etc.
>
> Oh, no.
>
> compat_pci_ata checks whether the controller(rather than shost or ata
> port) is pci SATA/IDE controller.
>
> As in patch 4:
> compat_pci_ata(dev->parent->parent->parent->parent)
>
> The "dev" is ata port, and dev->parent->parent->parent->parent points to
> the controller dev.
>
> This looks ugly. How about add two helpers?
> shost_to_controller(dev) and ata_port_to_controller(dev)
>

What about something like the following (untested)? Makes it mostly
independent of the topology, gives some type safety, and drops some
redundant work finding the ata_port.

static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
{
struct ata_port *ap = dev_to_ata_port(dev);

if (!ap)
return -ENODEV;

if (!compat_pci_ata(ap))
return -ENODEV;

if (scsi_is_host_device(dev))
return ata_acpi_bind_host(ap, handle);
else if (scsi_is_sdev_device(dev)) {
struct scsi_device *sdev = to_scsi_device(dev);

return ata_acpi_bind_device(ap, sdev->channel,
sdev->id, handle);
} else
return -ENODEV;
}

2012-06-21 06:37:16

by Lin Ming

[permalink] [raw]
Subject: Re: [PATCH v5 02/12] libata: bind the Linux device tree to the ACPI device tree

On Wed, 2012-06-20 at 20:59 -0700, Dan Williams wrote:
> On Wed, Jun 20, 2012 at 6:26 PM, Lin Ming <[email protected]> wrote:
> > On Wed, 2012-06-20 at 16:50 -0700, Dan Williams wrote:
> >> On Sun, Jun 17, 2012 at 11:25 PM, Lin Ming <[email protected]> wrote:
> >> > From: Matthew Garrett <[email protected]>
> >> >
> >> > Associate the ACPI device tree and libata devices.
> >> > This patch uses the generic ACPI glue framework to do so.
> >> >
> >> > Signed-off-by: Matthew Garrett <[email protected]>
> >> > Signed-off-by: Holger Macht <[email protected]>
> >> > Signed-off-by: Lin Ming <[email protected]>
> >> > ---
> >> >
> >> > Changelog:
> >> >
> >> > - fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu)
> >> > - rename is_pci_ata to compat_pci_ata (Alan Cox)
> >> >
> >> > drivers/acpi/glue.c | 4 +-
> >> > drivers/ata/libata-acpi.c | 125 +++++++++++++++++++++++++++++++++++++++++++++
> >> > drivers/ata/libata-core.c | 3 ++
> >> > drivers/ata/libata.h | 4 ++
> >> > 4 files changed, 134 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
> >> > index 1564e09..18d6812 100644
> >> > --- a/drivers/acpi/glue.c
> >> > +++ b/drivers/acpi/glue.c
> >> [..]
> >> > +static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle)
> >> > +{
> >> > + struct Scsi_Host *shost = dev_to_shost(dev);
> >> > + struct ata_port *ap = ata_shost_to_port(shost);
> >> > +
> >> > + if (ap->flags & ATA_FLAG_ACPI_SATA)
> >> > + return -ENODEV;
> >> > +
> >> > + *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), ap->port_no);
> >> > +
> >> > + if (!*handle)
> >> > + return -ENODEV;
> >> > +
> >> > + return 0;
> >> > +}
> >>
> >> I think this patch should be squashed with patch 4, right? That is if
> >> we keep the hard-coded ->parent chasing...
> >
> > Right. Will merge patch 4.
> >
> >>
> >> > +static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
> >> > +{
> >> > + unsigned int host, channel, id, lun;
> >> > +
> >> > + if (sscanf(dev_name(dev), "host%u", &host) == 1) {
> >> > + if (!compat_pci_ata(dev->parent))
> >> > + return -ENODEV;
> >> > +
> >> > + return ata_acpi_bind_host(dev, host, handle);
> >> > + } else if (sscanf(dev_name(dev), "%d:%d:%d:%d",
> >> > + &host, &channel, &id, &lun) == 4) {
> >> > + if (!compat_pci_ata(dev->parent->parent->parent))
> >> > + return -ENODEV;
> >>
> >> ...this looks like it should be using a dev_to_ata_port() helper which
> >> like dev_to_shost() skips the need to remember just how many parents
> >> until we get back to our ata_port, scsi_host, etc.
> >
> > Oh, no.
> >
> > compat_pci_ata checks whether the controller(rather than shost or ata
> > port) is pci SATA/IDE controller.
> >
> > As in patch 4:
> > compat_pci_ata(dev->parent->parent->parent->parent)
> >
> > The "dev" is ata port, and dev->parent->parent->parent->parent points to
> > the controller dev.
> >
> > This looks ugly. How about add two helpers?
> > shost_to_controller(dev) and ata_port_to_controller(dev)
> >
>
> What about something like the following (untested)? Makes it mostly
> independent of the topology, gives some type safety, and drops some
> redundant work finding the ata_port.
>
> static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
> {
> struct ata_port *ap = dev_to_ata_port(dev);
>
> if (!ap)
> return -ENODEV;
>
> if (!compat_pci_ata(ap))
> return -ENODEV;
>
> if (scsi_is_host_device(dev))
> return ata_acpi_bind_host(ap, handle);
> else if (scsi_is_sdev_device(dev)) {
> struct scsi_device *sdev = to_scsi_device(dev);
>
> return ata_acpi_bind_device(ap, sdev->channel,
> sdev->id, handle);
> } else
> return -ENODEV;
> }

Good idea.

Below is the incremental patch on top of this series.

drivers/ata/libata-acpi.c | 56 +++++++++++++++++++++++++++------------------
drivers/ata/libata-core.c | 2 --
drivers/ata/libata.h | 2 ++
3 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 08edebf..4112eaa 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -1075,8 +1075,9 @@ void ata_acpi_unbind(struct ata_device *dev)
ata_acpi_unregister_power_resource(dev);
}

-static int compat_pci_ata(struct device *dev)
+static int compat_pci_ata(struct ata_port *ap)
{
+ struct device *dev = ap->tdev.parent;
struct pci_dev *pdev;

if (!is_pci_dev(dev))
@@ -1091,15 +1092,13 @@ static int compat_pci_ata(struct device *dev)
return 1;
}

-static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle)
+static int ata_acpi_bind_host(struct ata_port *ap, acpi_handle *handle)
{
- struct Scsi_Host *shost = dev_to_shost(dev);
- struct ata_port *ap = ata_shost_to_port(shost);
-
if (ap->flags & ATA_FLAG_ACPI_SATA)
return -ENODEV;

- *handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent->parent), ap->port_no);
+ *handle = acpi_get_child(DEVICE_ACPI_HANDLE(ap->tdev.parent),
+ ap->port_no);

if (!*handle)
return -ENODEV;
@@ -1107,21 +1106,18 @@ static int ata_acpi_bind_host(struct device *dev, int host, acpi_handle *handle)
return 0;
}

-static int ata_acpi_bind_device(struct device *dev, int channel, int id,
+static int ata_acpi_bind_device(struct ata_port *ap, struct scsi_device *sdev,
acpi_handle *handle)
{
- struct scsi_device *sdev = to_scsi_device(dev);
- struct Scsi_Host *shost = sdev->host;
- struct ata_port *ap = ata_shost_to_port(shost);
struct ata_device *ata_dev;
acpi_status status;
struct acpi_device *acpi_dev;
struct acpi_device_power_state *states;

if (ap->flags & ATA_FLAG_ACPI_SATA)
- ata_dev = &ap->link.device[channel];
+ ata_dev = &ap->link.device[sdev->channel];
else
- ata_dev = &ap->link.device[id];
+ ata_dev = &ap->link.device[sdev->id];

*handle = ata_dev_acpi_handle(ata_dev);

@@ -1146,21 +1142,37 @@ static int ata_acpi_bind_device(struct device *dev, int channel, int id,
return 0;
}

+static int is_ata_port(const struct device *dev)
+{
+ return dev->type == &ata_port_type;
+}
+
+static struct ata_port *dev_to_ata_port(struct device *dev)
+{
+ while (!is_ata_port(dev)) {
+ if (!dev->parent)
+ return NULL;
+ dev = dev->parent;
+ }
+ return to_ata_port(dev);
+}
+
static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
{
- unsigned int host, channel, id, lun;
+ struct ata_port *ap = dev_to_ata_port(dev);

- if (sscanf(dev_name(dev), "host%u", &host) == 1) {
- if (!compat_pci_ata(dev->parent->parent))
- return -ENODEV;
+ if (!ap)
+ return -ENODEV;
+
+ if (!compat_pci_ata(ap))
+ return -ENODEV;

- return ata_acpi_bind_host(dev, host, handle);
- } else if (sscanf(dev_name(dev), "%d:%d:%d:%d",
- &host, &channel, &id, &lun) == 4) {
- if (!compat_pci_ata(dev->parent->parent->parent->parent))
- return -ENODEV;
+ if (scsi_is_host_device(dev))
+ return ata_acpi_bind_host(ap, handle);
+ else if (scsi_is_sdev_device(dev)) {
+ struct scsi_device *sdev = to_scsi_device(dev);

- return ata_acpi_bind_device(dev, channel, id, handle);
+ return ata_acpi_bind_device(ap, sdev, handle);
} else
return -ENODEV;
}
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index ba169c4..c14f88c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5291,8 +5291,6 @@ static int ata_port_request_pm(struct ata_port *ap, pm_message_t mesg,
return rc;
}

-#define to_ata_port(d) container_of(d, struct ata_port, tdev)
-
static int ata_port_suspend_common(struct device *dev, pm_message_t mesg)
{
struct ata_port *ap = to_ata_port(dev);
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index b0ba924..44a7939 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -107,6 +107,8 @@ extern const char *sata_spd_string(unsigned int spd);
extern int ata_port_probe(struct ata_port *ap);
extern void __ata_port_probe(struct ata_port *ap);

+#define to_ata_port(d) container_of(d, struct ata_port, tdev)
+
/* libata-acpi.c */
#ifdef CONFIG_ATA_ACPI
extern unsigned int ata_acpi_gtf_filter;


2012-06-21 15:40:42

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH v5 02/12] libata: bind the Linux device tree to the ACPI device tree

On Wed, Jun 20, 2012 at 11:37 PM, Lin Ming <[email protected]> wrote:
> Good idea.
>
> Below is the incremental patch on top of this series.
>

Looks good, I like your additional change to pass a sdev to
ata_acpi_bind_device.

--
Dan

2012-06-22 18:41:37

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH v5 0/13] SATA ZPODD support

On 06/18/2012 02:25 AM, Lin Ming wrote:
> Hi all,
>
> This is the v5 patches to add SATA ZPODD support, to try it:
> git pull git://git.kernel.org/pub/scm/linux/kernel/git/mlin/linux.git zpodd
>
> Holger and Mathtew,
> Patch 1 to Patch 4 are the libata acpi binding patches from you.
> I removed dock related code from your original version.
> Would you help to review?
>
> v5:
> - rebase to v3.5-rc3
> - fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu)
> - rename is_pci_ata to compat_pci_ata (Alan Cox)
>
> v4:
> https://lkml.org/lkml/2012/5/28/11
> - Includes libata acpi binding patches from Holger Macht and Matthew Garrett.
> - tell scsi layer device supports runtime power off
> - check support for device busy class events

In the case of this patchset, the main worry are the crashes that were
introduced by the Macht/Garrett patchset. Finally with v5 that
longstanding problem has been resolved, so we can move forward and get
this into linux-next (libata-dev.git#upstream).

Jeff



2012-06-23 01:06:18

by Lin Ming

[permalink] [raw]
Subject: Re: [PATCH v5 0/13] SATA ZPODD support

On Fri, 2012-06-22 at 14:41 -0400, Jeff Garzik wrote:
> On 06/18/2012 02:25 AM, Lin Ming wrote:
> > Hi all,
> >
> > This is the v5 patches to add SATA ZPODD support, to try it:
> > git pull git://git.kernel.org/pub/scm/linux/kernel/git/mlin/linux.git zpodd
> >
> > Holger and Mathtew,
> > Patch 1 to Patch 4 are the libata acpi binding patches from you.
> > I removed dock related code from your original version.
> > Would you help to review?
> >
> > v5:
> > - rebase to v3.5-rc3
> > - fixed crash reported at https://lkml.org/lkml/2012/2/24/2 (Aaron Lu)
> > - rename is_pci_ata to compat_pci_ata (Alan Cox)
> >
> > v4:
> > https://lkml.org/lkml/2012/5/28/11
> > - Includes libata acpi binding patches from Holger Macht and Matthew Garrett.
> > - tell scsi layer device supports runtime power off
> > - check support for device busy class events
>
> In the case of this patchset, the main worry are the crashes that were
> introduced by the Macht/Garrett patchset. Finally with v5 that
> longstanding problem has been resolved, so we can move forward and get
> this into linux-next (libata-dev.git#upstream).

I'm so glad to move forward.
I have some update suggested by Dan Williams, at
https://lkml.org/lkml/2012/6/21/26

I'll send out v6 for you to apply next Monday.

Thanks,
Lin Ming

>
> Jeff