ZPODD Patches
Subsystems (4)
v12 cover ZPODD Patches
v12:
Suggestions by Tejun Heo:
Fold patch 2 into patch 3;
Do not use bitfields in zpodd structure;
Call zpodd_exit in ata_scsi_remove_dev;
Remove run_atapi_cmd, use ata_exec_internal directly; Use enum to return ODD loading mechanism type information; Added some comments.
Typo fix:
Fixed some mis-spellings as pointed out by Sergei Shtylyov.
Several function name changes:
Rename zpodd_pre_poweroff to zpodd_enable_run_wake, zpodd_pre_poweron to zpodd_disable_run_wake. This feels clearer and people can get what the two functions are supposed to do by their names;
Rename zpodd_post_resume to zpodd_post_poweron, since this function is only useful when the ODD is powered off during suspend.
I decided to rename these functions when I'm adding comments for them as suggested by Tejun Heo, and I felt I didn't name them well...
QOS no_poweroff flag:
Only export qos flag for ZPODD devices, disks that can be runtime powered off should also export this interface, but it doesn't belong to this patchset so will be done in other patches.
A new patch:
Added a patch to remove a no more useful scsi_device flag can_power_off.
A git repo for it:
https://github.com/aaronlu/linux.git zpodd_v12
v11:
Introduce event_driven flag in scsi_device to silence the media event poll after ODD is powered off;
Removed ata layer PM QOS control, instead, simply limit ACPI state to D3_HOT when choosing state;
Make the power off delay a module param named zpodd_poweroff_delay, defaults to 30 seconds.
v10:
Introduce PM_QOS_NO_POLL flag to skip calling disk's events_check callback;
Do not use zero power ready hint information from event poll; Check attached device in port's runtime idle callback to decide if suspend is desired;
Address various comments from Tejun Heo.
v9:
Build ZPODD as part of libata instead of another standalone module as it is tightly related to other libata files.
Identify and init ZPODD during probe time instead of after SCSI device is created as suggested by Tejun Heo.
Make use of pm qos flag to give ACPI hint when choosing ACPI state. Expose qos flag to give user control of whether power off is allowed.
This patchset used Rafael's pm-qos work:git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-qos
v8:
This version is a redesign, it doesn't have much to do with previous versions. The ZPODD implementation is done almost entirely in ATA layer now, except 2 helper functions from SCSI sr driver to block disk events.
The basic idea is that, when ata port is runtime suspended, it will check if the ODD is ready to be powered off. And if yes, events is blocked and power omitted; if not, ODD's power supply remains unchanged by keeping ACPI state at D0.
Some background knowledge about ZPODD is added below v1 history log.
v7:
Re work of runtime pm of sr driver, based on ideas of Alan Stern and Oliver Neukum.
Jeff, due to the ready_to_power_off flag added, there is a small change in [PATCH v7 6/6] libata: acpi: respect may_power_off flag, please check if I can still get your ack, thanks.
v6:
When user changes may_power_off flag through sysfs entry and if device is already runtime suspended, resume resume it so that it can respect this flag next time it is runtime suspended as suggested by Alan Stern. Call scsi_autopm_get/put_device once in sr_check_events as suggested by Alan Stern.
v5:
Add may_power_off flag to scsi device.
Alan Stern suggested that I should not mess runtime suspend with runtime power off, but the current zpodd implementation made it not easy to seperate. So I re-wrote the zpodd implementation, the end result is, normal ODD can also enter runtime suspended state, but their power won't be removed.
v4:
Rebase on top of Linus' tree, due to this, the problem of a missing flag in v3 is gone;
Add a new function scsi_autopm_put_device_autosuspend to first mark last busy for the device and then put autosuspend it as suggested by Oliver Neukum.
Typo fix as pointed by Sergei Shtylyov.
Check can_power_off flag before any runtime pm operations in sr.
v3:
Rebase on top of scsi-misc tree;
Add the sr related patches previously in Jeff's libata tree; Re-organize the sr patches.
A problem for now: for patch
scsi: sr: support zero power ODD(ZPODD)
I can't set a flag in libata-acpi.c since a related function is missing in scsi-misc tree. Will fix this when 3.6-rc1 released.
v2:
Bug fix for v1;
Use scsi_autopm_* in sr driver instead of pm_runtime_*;
v1:
Here are some patches to make ZPODD easier to use for end users and a fix for using ZPODD with system suspend.
Some background knowledge about ZPODD:
ODD means Optical Disc Drive.
ZPODD means Zero Power ODD, it is a mechanism to place the ODD into zero power state when the system is running at S0 system state without user's awareness.
It achieved this by ACPI and SATA device attention pin. For power off, normal ACPI control method is used to place the device into D3 cold ACPI device state, aka. device power supply omitted. For power on, when user press the eject button of a drawer type ODD or when user inserts an ODD into a slot type ODD, the device attention pin will trigger. In the current x86 implementation, this pin will connect to a GPE, and the GPE will trigger an ACPI interrupt. With our pre-registered ACPI notification code, the device can be runtime resumed, and we place the device back to full power state by setting its ACPI state to D0. The whole process is transparent to the end user.
Aaron Lu (9):
scsi: sr: support runtime pm
libata: identify and init ZPODD devices
libata: move acpi notification code to zpodd
libata: check zero power ready status for ZPODD
libata: handle power transition of ODD
libata: expose pm qos flags for ata device
libata: scsi: no poll when ODD is powered off
libata: do not suspend port if normal ODD is attached
scsi: remove can_power_off flag from scsi_device
drivers/ata/Kconfig | 13 ++ drivers/ata/Makefile | 1 + drivers/ata/libata-acpi.c | 109 +++++------------ drivers/ata/libata-core.c | 23 +++- drivers/ata/libata-eh.c | 16 ++- drivers/ata/libata-scsi.c | 2 + drivers/ata/libata-zpodd.c | 299 +++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/libata.h | 27 ++++ drivers/scsi/sr.c | 36 +++++- include/linux/libata.h | 3 + include/scsi/scsi_device.h | 4 +- include/uapi/linux/cdrom.h | 34 ++++++ 12 files changed, 476 insertions(+), 91 deletions(-) create mode 100644 drivers/ata/libata-zpodd.c
Signature · 1 line
1.7.11.7
PATCH v12 1/9 scsi: sr: support runtime pm
This patch adds runtime pm support for sr.
It did this by increasing the runtime usage_count of the device when:
- its block device is opened;
- the events checking is to run.
And decreasing the runtime usage_count of the device when:
- its block device is closed;
- After the events checking is done.
The idea is discussed in this mail thread:
http://thread.gmane.org/gmane.linux.acpi.devel/55243/focus=52703
--- drivers/scsi/sr.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
drivers/scsi/sr.c +25 -5
@@ -45,6 +45,7 @@#include <linux/blkdev.h> #include <linux/mutex.h> #include <linux/slab.h> +#include <linux/pm_runtime.h> #include <asm/uaccess.h> #include <scsi/scsi.h>
@@ -146,7 +147,8 @@ static inline struct scsi_cd *scsi_cd_get(struct gendisk *disk)kref_get (&cd->kref);if (scsi_device_get (cd->device)) goto out_put; - goto out; +if (!scsi_autopm_get_device (cd->device)) + goto out; out_put:kref_put (&cd->kref, sr_kref_release);
@@ -162,6 +164,7 @@ static void scsi_cd_put(struct scsi_cd *cd)mutex_lock (&sr_ref_mutex);kref_put (&cd->kref, sr_kref_release); +scsi_autopm_put_device (sdev);scsi_device_put (sdev);mutex_unlock (&sr_ref_mutex); }
@@ -211,7 +214,7 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi, unsigned int clearing, int slot) {struct scsi_cd *cd = cdi->handle; -bool last_present; +bool last_present = cd->media_present;struct scsi_sense_hdr sshdr;unsigned int events;int ret;
@@ -220,6 +223,8 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi,if (CDSL_CURRENT != slot)return 0 ; +scsi_autopm_get_device (cd->device); + events =sr_get_events (cd->device); cd->get_event_changed |= events &DISK_EVENT_MEDIA_CHANGE ;
@@ -246,10 +251,9 @@ static unsigned int sr_check_events(struct cdrom_device_info *cdi, }if (!(clearing &DISK_EVENT_MEDIA_CHANGE )) -return events; + goto out; do_tur:/* let's see whether the media is there with TUR */ - last_present = cd->media_present; ret =scsi_test_unit_ready (cd->device,SR_TIMEOUT ,MAX_RETRIES , &sshdr);/*
@@ -270,7 +274,7 @@ do_tur: }if (cd->ignore_get_event) -return events; + goto out;/* check whether GET_EVENT is reporting spurious MEDIA_CHANGE */ if (!cd->tur_changed) {
@@ -287,6 +291,18 @@ do_tur: cd->tur_changed = false; cd->get_event_changed = false; +out: +/* +* If there is no medium detected or the medium has been there +* since last poll, try to suspend the device. Otherwise, keep +* it active for one more poll interval so that if user space +* application opens the block device, we can avoid a runtime +* status change. +*/ +pm_runtime_put_noidle (&cd->device->sdev_gendev); +if (!cd->media_present || last_present) +pm_runtime_suspend (&cd->device->sdev_gendev); +return events; }
@@ -718,6 +734,8 @@ static int sr_probe(struct device *dev)sdev_printk (KERN_DEBUG , sdev,"Attached scsi CD-ROM %s\n" , cd->cdi.name); +scsi_autopm_put_device (cd->device); +return 0 ; fail_put:
@@ -965,6 +983,8 @@ static int sr_remove(struct device *dev) {struct scsi_cd *cd =dev_get_drvdata (dev); +scsi_autopm_get_device (cd->device); +blk_queue_prep_rq (cd->device->request_queue, scsi_prep_fn);del_gendisk (cd->disk);
Signature · 1 line
1.7.11.7
PATCH v12 2/9 libata: identify and init ZPODD devices
In reply to Aaron LuThe ODD can be enabled for ZPODD if the following three conditions are satisfied:
1 The ODD supports device attention;
2 The platform can runtime power off the ODD through ACPI;
3 The ODD is either slot type or drawer type.
For such ODDs, zpodd_init is called and a new structure is allocated for it to store ZPODD related stuffs.
And the zpodd_dev_enabled function is used to test if ZPODD is currently enabled for this ODD.
A new config CONFIG_SATA_ZPODD is added to selectively build ZPODD code.
--- drivers/ata/Kconfig | 13 ++++++ drivers/ata/Makefile | 1 + drivers/ata/libata-core.c | 4 +- drivers/ata/libata-scsi.c | 2 + drivers/ata/libata-zpodd.c | 100 +++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/libata.h | 14 +++++++ include/linux/libata.h | 3 ++ include/uapi/linux/cdrom.h | 34 +++++++++++++++ 8 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 drivers/ata/libata-zpodd.c
drivers/ata/Kconfig +13 -0
@@ -58,6 +58,19 @@ config ATA_ACPI You can disable this at kernel boot time by using the option libata.noacpi=1 +config SATA_ZPODD + bool "SATA Zero Power ODD Support" + depends on ATA_ACPI + default n + help + This option adds support for SATA ZPODD. It requires both + ODD and the platform support, and if enabled, will automatically + power on/off the ODD when certain condition is satisfied. This + does not impact user's experience of the ODD, only power is saved + when ODD is not in use(i.e. no disc inside). + + If unsure, say N. + config SATA_PMP bool "SATA Port Multiplier support" default y
drivers/ata/Makefile +1 -0
@@ -107,3 +107,4 @@ libata-y := libata-core.o libata-scsi.o libata-eh.o libata-transport.o libata-$(CONFIG_ATA_SFF) += libata-sff.o libata-$(CONFIG_SATA_PMP) += libata-pmp.o libata-$(CONFIG_ATA_ACPI) += libata-acpi.o +libata-$(CONFIG_SATA_ZPODD) += libata-zpodd.o
drivers/ata/libata-core.c +3 -1
@@ -2396,8 +2396,10 @@ int ata_dev_configure(struct ata_device *dev) dma_dir_string =", DMADIR" ; } -if (ata_id_has_da (dev->id)) +if (ata_id_has_da (dev->id)) { dev->flags |=ATA_DFLAG_DA ; +zpodd_init (dev); + }/* print device info to dmesg */ if (ata_msg_drv (ap) && print_info)
drivers/ata/libata-scsi.c +2 -0
@@ -3755,6 +3755,8 @@ static void ata_scsi_remove_dev(struct ata_device *dev)mutex_lock (&ap->scsi_host->scan_mutex);spin_lock_irqsave (ap->lock, flags); +if (zpodd_dev_enabled (dev)) +zpodd_exit (dev);ata_acpi_unbind (dev);/* clearing dev->sdev is protected by host lock */
drivers/ata/libata-zpodd.c new file +100 -0
100 lines not on this page · raw message
drivers/ata/libata.h +14 -0
@@ -230,4 +230,18 @@ static inline void ata_sff_exit(void) { }#endif /* CONFIG_ATA_SFF */ +/* libata-zpodd.c */ +#ifdef CONFIG_SATA_ZPODD +void zpodd_init (struct ata_device *dev); +void zpodd_exit (struct ata_device *dev); +static inline bool zpodd_dev_enabled (struct ata_device *dev) +{ +return dev->zpodd !=NULL ; +} +#else /* CONFIG_SATA_ZPODD */ +static inline void zpodd_init (struct ata_device *dev) {} +static inline void zpodd_exit (struct ata_device *dev) {} +static inline bool zpodd_dev_enabled (struct ata_device *dev) {return false; } +#endif /* CONFIG_SATA_ZPODD */ +#endif /* __LIBATA_H__ */
include/linux/libata.h +3 -0
@@ -620,6 +620,9 @@ struct ata_device {union acpi_object *gtf_cache;unsigned int gtf_filter;#endif +#ifdef CONFIG_SATA_ZPODD +void *zpodd; +#endif struct device tdev;/* n_sector is CLEAR_BEGIN, read comment above CLEAR_BEGIN */ u64 n_sectors;/* size of device, if ATA */
include/uapi/linux/cdrom.h +34 -0
@@ -908,5 +908,39 @@ struct mode_page_header {__be16 desc_length; }; +/* 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; +};#endif /* _UAPI_LINUX_CDROM_H */
Signature · 1 line
1.7.11.7
Re: [PATCH v12 2/9] libata: identify and init ZPODD devices
Quoted 8 lines · Aaron Lu
The ODD can be enabled for ZPODD if the following three conditions are satisfied:
1 The ODD supports device attention;
2 The platform can runtime power off the ODD through ACPI;
3 The ODD is either slot type or drawer type.
For such ODDs,zpodd_initis called and a new structure is allocated for it to store ZPODD related stuffs.
Quoted 6 lines
And the
zpodd_dev_enabledfunction is used to test if ZPODD is currently enabled for this ODD.A new config
CONFIG_SATA_ZPODDis added to selectively build ZPODD code.Signed-off-by Aaron Lu <[email protected]>
Thanks.
Signature · 1 line
tejun
PATCH v12 3/9 libata: move acpi notification code to zpodd
In reply to Aaron LuSince the ata acpi notification code introduced in commit 3bd46600a7a7e938c54df8cdbac9910668c7dfb0 is solely for ZPODD, and we now have a dedicated place for it, move these code there.
And the ata_acpi_add_pm_notifier code is changed a little bit in that it is now invoked when scsi device is not bound with ACPI yet, so the way to get the acpi handle is different with the previous version. And the ata_acpi_add/remove_pm_notifier is also simplified a little bit in that it doesn't check if the acpi_device for the handle exists or not as the odd_can_poweroff function already checked that.
--- drivers/ata/libata-acpi.c | 71 ---------------------------------------------- drivers/ata/libata-zpodd.c | 34 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 71 deletions(-)
drivers/ata/libata-acpi.c +0 -71
@@ -974,57 +974,6 @@ 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_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); - } -} - -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_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); - } -} -static void ata_acpi_register_power_resource (struct ata_device *dev) {struct scsi_device *sdev = dev->sdev;
@@ -1057,13 +1006,11 @@ static void ata_acpi_unregister_power_resource(struct ata_device *dev)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); }
@@ -1105,9 +1052,6 @@ static int ata_acpi_bind_device(struct ata_port *ap, struct scsi_device *sdev, acpi_handle *handle) {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 ) {if (!sata_pmp_attached (ap))
@@ -1124,21 +1068,6 @@ static int ata_acpi_bind_device(struct ata_port *ap, struct scsi_device *sdev,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 ; }
drivers/ata/libata-zpodd.c +34 -0
@@ -1,5 +1,7 @@#include <linux/libata.h> #include <linux/cdrom.h> +#include <linux/pm_runtime.h> +#include <scsi/scsi_device.h> #include "libata.h"
@@ -12,6 +14,10 @@ enum odd_mech_type {struct zpodd {enum odd_mech_type mech_type;/* init during probe, RO afterwards */ struct ata_device *dev; + +/* The following fields are synchronized by PM core. */ +bool from_notify;/* resumed as a result of +* acpi wake notification */ };/* Per the spec, only slot type and drawer type ODD can be supported */
@@ -68,6 +74,32 @@ static bool odd_can_poweroff(struct ata_device *ata_dev)return acpi_device_can_poweroff (acpi_dev); } +static void zpodd_wake_dev (acpi_handle handle,u32 event,void *context) +{ +struct ata_device *ata_dev = context; +struct zpodd *zpodd = ata_dev->zpodd; +struct device *dev = &ata_dev->sdev->sdev_gendev; + +if (event ==ACPI_NOTIFY_DEVICE_WAKE && ata_dev && +pm_runtime_suspended (dev)) { + zpodd->from_notify = true; +pm_runtime_resume (dev); + } +} + +static void ata_acpi_add_pm_notifier (struct ata_device *dev) +{ +acpi_handle handle =ata_dev_acpi_handle (dev); +acpi_install_notify_handler (handle,ACPI_SYSTEM_NOTIFY , + zpodd_wake_dev, dev); +} + +static void ata_acpi_remove_pm_notifier (struct ata_device *dev) +{ +acpi_handle handle =DEVICE_ACPI_HANDLE (&dev->sdev->sdev_gendev); +acpi_remove_notify_handler (handle,ACPI_SYSTEM_NOTIFY , zpodd_wake_dev); +} +void zpodd_init (struct ata_device *dev) {enum odd_mech_type mech_type;
@@ -89,12 +121,14 @@ void zpodd_init(struct ata_device *dev) zpodd->mech_type = mech_type; +ata_acpi_add_pm_notifier (dev); zpodd->dev = dev; dev->zpodd = zpodd; }void zpodd_exit (struct ata_device *dev) { +ata_acpi_remove_pm_notifier (dev);kfree (dev->zpodd); dev->zpodd =NULL ; }
Signature · 1 line
1.7.11.7
Re: [PATCH v12 3/9] libata: move acpi notification code to zpodd
Quoted 4 lines · Aaron Lu
Since the ata acpi notification code introduced in commit
3bd46600a7a7e938c54df8cdbac9910668c7dfb0is solely for ZPODD, and we now have a dedicated place for it, move these code there.
Quoted 8 lines
And the
ata_acpi_add_pm_notifiercode is changed a little bit in that it is now invoked when scsi device is not bound with ACPI yet, so the way to get the acpi handle is different with the previous version. And the ata_acpi_add/remove_pm_notifier is also simplified a little bit in that it doesn't check if theacpi_devicefor the handle exists or not as theodd_can_powerofffunction already checked that.Signed-off-by Aaron Lu <[email protected]>
Thanks.
Signature · 1 line
tejun
PATCH v12 4/9 libata: check zero power ready status for ZPODD
In reply to Aaron LuPer the Mount Fuji spec, the ODD is considered zero power ready when:
- For slot type ODD, no media inside;
- For tray type ODD, no media inside and tray closed.
The information can be retrieved by either the returned information of command GET_EVENT_STATUS_NOTIFICATION(the command is used to poll for media event) or sense code.
The information provided by the media status byte is not accurate, it is possible that after a new disc is just inserted, the status byte still returns media not present. So this information can not be used as the deciding factor, we use sense code to decide if zpready status is true.
When we first sensed the ODD in the zero power ready state, the zp_sampled will be set and timestamp will be recoreded. And after ODD stayed in this state for some pre-defined period, the ODD is considered as power off ready and the zp_ready flag will be set. The zp_ready flag serves as the deciding factor other code will use to see if power off is OK for the ODD.
The Mount Fuji spec suggests a delay should be used here, to avoid the case user ejects the ODD and then instantly inserts a new one again, so that we can avoid a power transition. And some ODDs may be slow to place its head to the home position after disc is ejected, so a delay here is generally a good idea. And the delay time can be changed via the module param zpodd_poweroff_delay.
The zero power ready status check is performed in the ata port's runtime suspend code path, when port is not frozen yet, as we need to issue some IOs to the ODD.
--- drivers/ata/libata-eh.c | 14 +++++++-- drivers/ata/libata-zpodd.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/libata.h | 5 ++++ 3 files changed, 92 insertions(+), 2 deletions(-)
drivers/ata/libata-eh.c +12 -2
@@ -1591,7 +1591,7 @@ static int ata_eh_read_log_10h(struct ata_device *dev,* RETURNS: * 0 on success, AC_ERR_* mask on failure. */ -static unsigned int atapi_eh_tur (struct ata_device *dev,u8 *r_sense_key) +unsigned int atapi_eh_tur (struct ata_device *dev,u8 *r_sense_key) {u8 cdb[ATAPI_CDB_LEN ] = {TEST_UNIT_READY ,0 ,0 ,0 ,0 ,0 };struct ata_taskfile tf;
@@ -1624,7 +1624,7 @@ static unsigned int atapi_eh_tur(struct ata_device *dev, u8 *r_sense_key)* RETURNS: * 0 on success, AC_ERR_* mask on failure */ -static unsigned int atapi_eh_request_sense (struct ata_device *dev, +unsigned int atapi_eh_request_sense (struct ata_device *dev,u8 *sense_buf,u8 dfl_sense_key) { u8 cdb[ATAPI_CDB_LEN] =
@@ -4022,6 +4022,7 @@ static void ata_eh_handle_port_suspend(struct ata_port *ap) {unsigned long flags;int rc =0 ; +struct ata_device *dev;/* are we suspending? */ spin_lock_irqsave (ap->lock, flags);
@@ -4034,6 +4035,15 @@ static void ata_eh_handle_port_suspend(struct ata_port *ap)WARN_ON (ap->pflags &ATA_PFLAG_SUSPENDED ); +/* +* If we have a ZPODD attached, check its zero +* power ready status before the port is frozen. +*/ +ata_for_each_dev (dev, &ap->link,ENABLED ) { +if (zpodd_dev_enabled (dev)) +zpodd_on_suspend (dev); + } +/* tell ACPI we're suspending */ rc =ata_acpi_on_suspend (ap);if (rc)
drivers/ata/libata-zpodd.c +75 -0
97 lines not on this page · raw message
drivers/ata/libata.h +5 -0
@@ -182,6 +182,9 @@ extern void ata_eh_finish(struct ata_port *ap);extern int ata_ering_map (struct ata_ering *ering,int (*map_fn)(struct ata_ering_entry *,void *),void *arg); +extern unsigned int atapi_eh_tur (struct ata_device *dev,u8 *r_sense_key); +extern unsigned int atapi_eh_request_sense (struct ata_device *dev, +u8 *sense_buf,u8 dfl_sense_key);/* libata-pmp.c */ #ifdef CONFIG_SATA_PMP
@@ -238,10 +241,12 @@ static inline bool zpodd_dev_enabled(struct ata_device *dev) {return dev->zpodd !=NULL ; } +void zpodd_on_suspend (struct ata_device *dev);#else /* CONFIG_SATA_ZPODD */ static inline void zpodd_init (struct ata_device *dev) {}static inline void zpodd_exit (struct ata_device *dev) {}static inline bool zpodd_dev_enabled (struct ata_device *dev) {return false; } +static inline void zpodd_on_suspend (struct ata_device *dev) {}#endif /* CONFIG_SATA_ZPODD */ #endif /* __LIBATA_H__ */
Signature · 1 line
1.7.11.7
Re: [PATCH v12 4/9] libata: check zero power ready status for ZPODD
Hello, Aaron.
Quoted 10 lines · Aaron Lu
drivers/ata/libata-zpodd.c +10
-0+/* +* Update the zpodd->zp_ready field. This field will only be set +* if the ODD has stayed in ZP ready state for zpodd_poweroff_delay +* time, and will be used to decide if power off is allowed. If it +* is set, it will be cleared during resume from powered off state. +*/ +void zpodd_on_suspend (struct ata_device *dev) +{ +struct zpodd *zpodd = dev->zpodd; +unsigned long expires;
drivers/ata/libata-zpodd.c static bool odd_can_poweroff(struct ata_device *ata_dev) · line 141
Quoted 10 lines
drivers/ata/libata-zpodd.c +10
-0+ +if (!zpready (dev)) { + zpodd->zp_sampled = false; +return ; + } + +if (!zpodd->zp_sampled) { + zpodd->zp_sampled = true; + zpodd->last_ready = jiffies; +return ;
Is the above return necessary? Can't we just fall through and always check the actual condition?
drivers/ata/libata-zpodd.c static bool odd_can_poweroff(struct ata_device *ata_dev) · line 150
Quoted 9 lines
drivers/ata/libata-zpodd.c +9
-0+ } + + expires = zpodd->last_ready + +msecs_to_jiffies (zpodd_poweroff_delay *1000 ); +if (time_before (jiffies, expires)) +return ; + + zpodd->zp_ready = true; +}
Thanks.
Signature · 1 line
tejun
Re: [PATCH v12 4/9] libata: check zero power ready status for ZPODD
Hi Tejun,
Quoted 18 lines · Tejun Heo
Hello, Aaron.
Quoted 15 lines · Aaron Lu
drivers/ata/libata-zpodd.c +15
-0+/* +* Update the zpodd->zp_ready field. This field will only be set +* if the ODD has stayed in ZP ready state for zpodd_poweroff_delay +* time, and will be used to decide if power off is allowed. If it +* is set, it will be cleared during resume from powered off state. +*/ +void zpodd_on_suspend (struct ata_device *dev) +{ +struct zpodd *zpodd = dev->zpodd; +unsigned long expires; + +if (!zpready (dev)) { + zpodd->zp_sampled = false; +return ; + }
Quoted 8 lines
drivers/ata/libata-zpodd.c static bool odd_can_poweroff(struct ata_device *ata_dev) · line 141
Quoted 5 lines
drivers/ata/libata-zpodd.c +5
-0+ +if (!zpodd->zp_sampled) { + zpodd->zp_sampled = true; + zpodd->last_ready = jiffies; +return ;Is the above return necessary? Can't we just fall through and always check the actual condition?
The zpready(dev) function will check the actual condition by issuing a TUR and check the returned sense code. And the zp_sampled serves as an indication if this is the first time we sensed the ODD in ZP ready status.
The zpodd_on_suspend works like this:
check ZP ready status by calling zpready(dev)
if not, clear zp_sampled and zp_ready
return
ODD is ZP ready
check if this is the first time we sensed it is ZP ready
if not, set the zp_sampled flag and record timestamp return
This is not the first time we sensed ZP ready for the ODD
check if the ODD has been in this state long enough
if not, return
The ODD has been in ZP ready state long enough
set the zp_ready flag, the zpodd_zpready will use that flag
Done.
Does this work flow look OK?
BTW, I just realized I mistakenly removed the clear of zp_ready in v12 when zpready(dev) returns false, I thought it is not necessary since each time we set that flag, the ODD will be powered off and the flag will always be cleared during resume time in post_poweron. But when qos_no_poweroff is set, this is not the case and can cause problem when ODD is not in ZP ready state and user cleared the qos_no_poweroff flag. I'll add a comment for this in next version to avoid making this mistake again in the future...
Thanks,
Aaron
Quoted 15 lines
drivers/ata/libata-zpodd.c static bool odd_can_poweroff(struct ata_device *ata_dev) · line 150
Quoted 10 lines
drivers/ata/libata-zpodd.c +9
-0+ } + + expires = zpodd->last_ready + +msecs_to_jiffies (zpodd_poweroff_delay *1000 ); +if (time_before (jiffies, expires)) +return ; + + zpodd->zp_ready = true; +}Thanks.
Signature · 1 line
tejun
PATCH v12 5/9 libata: handle power transition of ODD
In reply to Aaron LuWhen ata port is runtime suspended, it will check if the ODD attched to it is a zero power(ZP) capable ODD and if the ZP capable ODD is in zero power ready state. And if this is not the case, the highest acpi state will be limited to ACPI_STATE_D3_HOT to avoid powering off the ODD. And if the ODD can be powered off, runtime wake capability needs to be enabled and powered_off flag will be set to let resume code knows that the ODD was in powered off state.
And on resume, before it is powered on, if it was powered off during suspend, runtime wake capability needs to be disabled. After it is recovered, the ODD is considered functional, post power on processing like eject tray if the ODD is drawer type is done, and several ZPODD related fields will also be reset.
--- drivers/ata/libata-acpi.c | 35 +++++++++++++------ drivers/ata/libata-eh.c | 2 ++ drivers/ata/libata-zpodd.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/libata.h | 8 +++++ 4 files changed, 118 insertions(+), 10 deletions(-)
drivers/ata/libata-acpi.c +25 -10
@@ -835,6 +835,22 @@ void ata_acpi_on_resume(struct ata_port *ap) } } +static int ata_acpi_choose_suspend_state (struct ata_device *dev) +{ +int d_max_in =ACPI_STATE_D3_COLD ; + +/* +* For ATAPI, runtime D3 cold is only allowed +* for ZPODD in zero power ready state +*/ +if (dev->class ==ATA_DEV_ATAPI && + !(zpodd_dev_enabled (dev) &&zpodd_zpready (dev))) + d_max_in =ACPI_STATE_D3_HOT ; + +return acpi_pm_device_sleep_state (&dev->sdev->sdev_gendev, +NULL , d_max_in); +} +/** * ata_acpi_set_state - set the port power state * @ap: target ATA port
@@ -861,17 +877,16 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t state)continue ;if (state.event !=PM_EVENT_ON ) { - acpi_state =acpi_pm_device_sleep_state ( - &dev->sdev->sdev_gendev,NULL ,ACPI_STATE_D3 ); -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); + acpi_state =ata_acpi_choose_suspend_state (dev); +if (acpi_state ==ACPI_STATE_D0 ) +continue ; +if (zpodd_dev_enabled (dev) && + acpi_state ==ACPI_STATE_D3_COLD ) +zpodd_enable_run_wake (dev); +acpi_bus_set_power (handle, acpi_state); }else { -/* Ditto */ -acpi_pm_device_run_wake ( - &dev->sdev->sdev_gendev, false); +if (zpodd_dev_enabled (dev)) +zpodd_disable_run_wake (dev);acpi_bus_set_power (handle,ACPI_STATE_D0 ); } }
drivers/ata/libata-eh.c +2 -0
@@ -3857,6 +3857,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, rc =atapi_eh_clear_ua (dev);if (rc) goto rest_fail; +if (zpodd_dev_enabled (dev)) +zpodd_post_poweron (dev); } }
drivers/ata/libata-zpodd.c +83 -0
97 lines not on this page · raw message
drivers/ata/libata.h +8 -0
@@ -242,11 +242,19 @@ static inline bool zpodd_dev_enabled(struct ata_device *dev)return dev->zpodd !=NULL ; }void zpodd_on_suspend (struct ata_device *dev); +bool zpodd_zpready (struct ata_device *dev); +void zpodd_enable_run_wake (struct ata_device *dev); +void zpodd_disable_run_wake (struct ata_device *dev); +void zpodd_post_poweron (struct ata_device *dev);#else /* CONFIG_SATA_ZPODD */ static inline void zpodd_init (struct ata_device *dev) {}static inline void zpodd_exit (struct ata_device *dev) {}static inline bool zpodd_dev_enabled (struct ata_device *dev) {return false; }static inline void zpodd_on_suspend (struct ata_device *dev) {} +static inline bool zpodd_zpready (struct ata_device *dev) {return false; } +static inline void zpodd_enable_run_wake (struct ata_device *dev) {} +static inline void zpodd_disable_run_wake (struct ata_device *dev) {} +static inline void zpodd_post_poweron (struct ata_device *dev) {}#endif /* CONFIG_SATA_ZPODD */ #endif /* __LIBATA_H__ */
Signature · 1 line
1.7.11.7
Re: [PATCH v12 5/9] libata: handle power transition of ODD
Quoted 8 lines · Aaron Lu
When ata port is runtime suspended, it will check if the ODD attched to it is a zero power(ZP) capable ODD and if the ZP capable ODD is in zero power ready state. And if this is not the case, the highest acpi state will be limited to
ACPI_STATE_D3_HOTto avoid powering off the ODD. And if the ODD can be powered off, runtime wake capability needs to be enabled andpowered_offflag will be set to let resume code knows that the ODD was in powered off state.
Quoted 7 lines
And on resume, before it is powered on, if it was powered off during suspend, runtime wake capability needs to be disabled. After it is recovered, the ODD is considered functional, post power on processing like eject tray if the ODD is drawer type is done, and several ZPODD related fields will also be reset.
Signed-off-by Aaron Lu <[email protected]>
Signature · 1 line
tejun
PATCH v12 6/9 libata: expose pm qos flags for ata device
In reply to Aaron LuExpose pm qos flags to user space so that user has a chance to disable ZPODD feature, if he/she has a broken platform or devices or simply does not like this feature.
This flag is exposed to user space only for ZPODD devices.
--- drivers/ata/libata-acpi.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/ata/libata-acpi.c +3 -0
@@ -17,6 +17,7 @@#include <linux/pci.h> #include <linux/slab.h> #include <linux/pm_runtime.h> +#include <linux/pm_qos.h> #include <scsi/scsi_device.h> #include "libata.h"
@@ -1022,6 +1023,8 @@ static void ata_acpi_unregister_power_resource(struct ata_device *dev)void ata_acpi_bind (struct ata_device *dev) {ata_acpi_register_power_resource (dev); +if (zpodd_dev_enabled (dev)) +dev_pm_qos_expose_flags (&dev->sdev->sdev_gendev,0 ); }void ata_acpi_unbind (struct ata_device *dev)
Signature · 1 line
1.7.11.7
Re: [PATCH v12 6/9] libata: expose pm qos flags for ata device
Quoted 7 lines · Aaron Lu
Expose pm qos flags to user space so that user has a chance to disable ZPODD feature, if he/she has a broken platform or devices or simply does not like this feature.
This flag is exposed to user space only for ZPODD devices.
Signed-off-by Aaron Lu <[email protected]>
Thanks.
Signature · 1 line
tejun
PATCH v12 7/9 libata: scsi: no poll when ODD is powered off
In reply to Aaron LuWhen the ODD is powered off, any action the user did to the ODD that would generate a media event will trigger an ACPI interrupt, so the poll for media event is no longer necessary. And the poll will also cause a runtime status change, which will stop the ODD from staying in powered off state, so the poll should better be stopped.
But since we don't have access to the gendisk structure in LLDs, here comes the disable_disk_events flag for scsi device. This flag serves as a capability of the device, conveyed by the LLDs to upper layer. It is set when LLDs know that this device will no longer generate any media related events, so that the check_events can simply return 0 without bothering the device, effectively silence the poll.
--- drivers/ata/libata-zpodd.c | 8 ++++++++ drivers/scsi/sr.c | 6 +++++- include/scsi/scsi_device.h | 3 +++ 3 files changed, 16 insertions(+), 1 deletion(-)
drivers/ata/libata-zpodd.c +8 -0
@@ -182,6 +182,13 @@ void zpodd_enable_run_wake(struct ata_device *dev) {struct zpodd *zpodd = dev->zpodd; +/* +* Silence the media change poll, as we will be notified when +* user wants to use the ODD so there is no meaning to poll +* it when it is powered off +*/ + dev->sdev->disable_disk_events = true; + zpodd->powered_off = true;device_set_run_wake (&dev->sdev->sdev_gendev, true);acpi_pm_device_run_wake (&dev->sdev->sdev_gendev, true);
@@ -230,6 +237,7 @@ void zpodd_post_poweron(struct ata_device *dev) zpodd->zp_sampled = false; zpodd->zp_ready = false; + dev->sdev->disable_disk_events = false; }static void zpodd_wake_dev (acpi_handle handle,u32 event,void *context)
drivers/scsi/sr.c +5 -1
@@ -594,7 +594,11 @@ static unsigned int sr_block_check_events(struct gendisk *disk, unsigned int clearing) {struct scsi_cd *cd =scsi_cd (disk); -return cdrom_check_events (&cd->cdi, clearing); + +if (!cd->device->disable_disk_events) +return cdrom_check_events (&cd->cdi, clearing); +else +return 0 ; }static int sr_block_revalidate_disk (struct gendisk *disk)
include/scsi/scsi_device.h +3 -0
@@ -161,6 +161,9 @@ struct scsi_device { unsigned wce_default_on:1; /* Cache is ON by default */ unsigned no_dif:1; /* T10 PI (DIF) should be disabled */ +bool disable_disk_events;/* disable poll for disk events, used in +* ATA layer, sychronized by PM core */ +DECLARE_BITMAP (supported_events,SDEV_EVT_MAXBITS );/* supported events */ struct list_head event_list;/* asserted events */ struct work_struct event_work;
Signature · 1 line
1.7.11.7
Re: [PATCH v12 7/9] libata: scsi: no poll when ODD is powered off
drivers/ata/libata-zpodd.c void zpodd_enable_run_wake(struct ata_device *dev) · line 190
Quoted 10 lines · Aaron Lu
drivers/ata/libata-zpodd.c +6
-0@@ -182,6 +182,13 @@ void zpodd_enable_run_wake(struct ata_device *dev) {struct zpodd *zpodd = dev->zpodd; +/* +* Silence the media change poll, as we will be notified when +* user wants to use the ODD so there is no meaning to poll +* it when it is powered off +*/ + dev->sdev->disable_disk_events = true;
What's the synchronization rule for this field?
Thanks.
Signature · 1 line
tejun
Re: [PATCH v12 7/9] libata: scsi: no poll when ODD is powered off
Quoted 5 lines · Tejun Heo
Quoted 4 lines · Aaron Lu
drivers/ata/libata-zpodd.c +0
-0@@ -182,6 +182,13 @@ void zpodd_enable_run_wake(struct ata_device *dev) {struct zpodd *zpodd = dev->zpodd;
Quoted 8 lines
drivers/ata/libata-zpodd.c void zpodd_enable_run_wake(struct ata_device *dev) · line 190
Quoted 6 lines
drivers/ata/libata-zpodd.c +6
-0+/* +* Silence the media change poll, as we will be notified when +* user wants to use the ODD so there is no meaning to poll +* it when it is powered off +*/ + dev->sdev->disable_disk_events = true;What's the synchronization rule for this field?
I documented the rule in include/scsi/scsi_device.h.
This field is modified in the ata port's runtime suspend and resume callback, and is read accessed in the check_events callback of the sr block driver. The runtime PM callback is synchronized by PM core, in that the two callbacks will never run concurrently. So I guess saying synchronized by PM core is enough for this field?
This is what I've added in v12 for scsi_device structure:
+ bool disable_disk_events; /* disable poll for disk events, used in
+ * ATA layer, sychronized by PM core */
+
Or do you mean I should add a comment explaining the sync rule when it is modifed, like in the above code?
Thanks,
Aaron
Quoted 5 lines
Thanks.
Signature · 1 line
tejun
Re: [PATCH v12 7/9] libata: scsi: no poll when ODD is powered off
Hello, Aaron.
Quoted 16 lines · Aaron Lu
Quoted 1 line
What's the synchronization rule for this field?
I documented the rule in
include/scsi/scsi_device.h.This field is modified in the ata port's runtime suspend and resume callback, and is read accessed in the
check_eventscallback of the sr block driver. The runtime PM callback is synchronized by PM core, in that the two callbacks will never run concurrently. So I guess saying synchronized by PM core is enough for this field?This is what I've added in v12 for
scsi_devicestructure:include/scsi/scsi_device.h +3
-0+bool disable_disk_events;/* disable poll for disk events, used in +* ATA layer, sychronized by PM core */ +
Quoted 2 lines
Or do you mean I should add a comment explaining the sync rule when it is modifed, like in the above code?
The thing is that disabling disk events doesn't necessarily have anything to do with PM, so tying synchronization to PM subsystem is a bit unexpected. How about making it an atomic_t? That way, disabling can stack and synchronization dependency to PM is removed.
Thanks.
Signature · 1 line
tejun
Re: [PATCH v12 7/9] libata: scsi: no poll when ODD is powered off
Hi Tejun,
Quoted 19 lines · Tejun Heo
Hello, Aaron.
Quoted 16 lines · Aaron Lu
Quoted 1 line
What's the synchronization rule for this field?
I documented the rule in
include/scsi/scsi_device.h.This field is modified in the ata port's runtime suspend and resume callback, and is read accessed in the
check_eventscallback of the sr block driver. The runtime PM callback is synchronized by PM core, in that the two callbacks will never run concurrently. So I guess saying synchronized by PM core is enough for this field?This is what I've added in v12 for
scsi_devicestructure:include/scsi/scsi_device.h +3
-0+bool disable_disk_events;/* disable poll for disk events, used in +* ATA layer, sychronized by PM core */ +
Quoted 7 lines
Quoted 2 lines
Or do you mean I should add a comment explaining the sync rule when it is modifed, like in the above code?
The thing is that disabling disk events doesn't necessarily have anything to do with PM, so tying synchronization to PM subsystem is a bit unexpected. How about making it an
atomic_t? That way, disabling can stack and synchronization dependency to PM is removed.
OK, will make it atomic in next version, thanks for the advice.
Perhaps I can add two scsi helper functions in scsi_lib.c like:
void sdev_disable_disk_events(struct scsi_device *sdev)
{
atomic_inc(&sdev->disk_events_disable_depth);
}void sdev_enable_disk_events(struct scsi_device *sdev)
{
if (WARN_ON_ONCE(atomic_read(&sdev->disk_events_disable_depth) <= 0))
return;
atomic_dec(&sdev->disk_events_disable_depth);
}And call them in ATA layer. Do you like this?
Thanks,
Aaron
Re: [PATCH v12 7/9] libata: scsi: no poll when ODD is powered off
Hello,
Quoted 9 lines · Aaron Lu
OK, will make it atomic in next version, thanks for the advice.
Perhaps I can add two scsi helper functions in
scsi_lib.clike:void
sdev_disable_disk_events(struct scsi_device *sdev){ atomic_inc(&sdev->disk_events_disable_depth); }
Quoted 8 lines
void
sdev_enable_disk_events(struct scsi_device *sdev){ if (WARN_ON_ONCE(atomic_read(&sdev->disk_events_disable_depth) <= 0)) return; atomic_dec(&sdev->disk_events_disable_depth); }And call them in ATA layer. Do you like this?
Sounds good to me. James, how does the series look to you?
Thanks!
Signature · 1 line
tejun
Re: [PATCH v12 7/9] libata: scsi: no poll when ODD is powered off
Quoted 19 lines · Tejun Heo
Hello,
Quoted 16 lines · Aaron Lu
OK, will make it atomic in next version, thanks for the advice.
Perhaps I can add two scsi helper functions in
scsi_lib.clike:void
sdev_disable_disk_events(struct scsi_device *sdev){ atomic_inc(&sdev->disk_events_disable_depth); }void
sdev_enable_disk_events(struct scsi_device *sdev){ if (WARN_ON_ONCE(atomic_read(&sdev->disk_events_disable_depth) <= 0)) return; atomic_dec(&sdev->disk_events_disable_depth); }
Quoted 3 lines
Quoted 1 line
And call them in ATA layer. Do you like this?
Sounds good to me. James, how does the series look to you?
Indeed. Want James' Acked-by for patch #1.
I think it's ready. It can go into libata-dev.git #upstream, and be reverted prior to Linus push if James NAKs.
Jeff
Re: [PATCH v12 7/9] libata: scsi: no poll when ODD is powered off
Quoted 20 lines · Jeff Garzik
Quoted 19 lines · Tejun Heo
Hello,
Quoted 16 lines · Aaron Lu
OK, will make it atomic in next version, thanks for the advice.
Perhaps I can add two scsi helper functions in
scsi_lib.clike:void
sdev_disable_disk_events(struct scsi_device *sdev){ atomic_inc(&sdev->disk_events_disable_depth); }void
sdev_enable_disk_events(struct scsi_device *sdev){ if (WARN_ON_ONCE(atomic_read(&sdev->disk_events_disable_depth) <= 0)) return; atomic_dec(&sdev->disk_events_disable_depth); }
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
And call them in ATA layer. Do you like this?
Sounds good to me. James, how does the series look to you?
Indeed. Want James' Acked-by for patch #1.
I think it's ready. It can go into libata-dev.git #upstream, and be reverted prior to Linus push if James NAKs.
Great! Thanks.
I'll incorporate Tejun's suggestions into v13 and send them out soon.
-Aaron
PATCH v12 8/9 libata: do not suspend port if normal ODD is attached
In reply to Aaron LuFor ODDs, the upper layer will poll for media change every few seconds, which will make it enter and leave suspend state very often. And as each suspend will also cause a hard/soft reset, the gain of runtime suspend is very little while the ODD may malfunction after constantly being reset. So the idle callback here will not proceed to suspend if a non-ZPODD capable ODD is attached to the port.
--- drivers/ata/libata-core.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
drivers/ata/libata-core.c +19 -0
@@ -5408,8 +5408,27 @@ static int ata_port_resume(struct device *dev)return rc; } +/* +* For ODDs, the upper layer will poll for media change every few seconds, +* which will make it enter and leave suspend state every few seconds. And +* as each suspend will cause a hard/soft reset, the gain of runtime suspend +* is very little and the ODD may malfunction after constantly being reset. +* So the idle callback here will not proceed to suspend if a non-ZPODD capable +* ODD is attached to the port. +*/ static int ata_port_runtime_idle (struct device *dev) { +struct ata_port *ap =to_ata_port (dev); +struct ata_link *link; +struct ata_device *adev; + +ata_for_each_link (link, ap,HOST_FIRST ) { +ata_for_each_dev (adev, link,ENABLED ) +if (adev->class ==ATA_DEV_ATAPI && + !zpodd_dev_enabled (adev)) +return -EBUSY ; + } +return pm_runtime_suspend (dev); }
Signature · 1 line
1.7.11.7
Re: [PATCH v12 8/9] libata: do not suspend port if normal ODD is attached
Quoted 9 lines · Aaron Lu
For ODDs, the upper layer will poll for media change every few seconds, which will make it enter and leave suspend state very often. And as each suspend will also cause a hard/soft reset, the gain of runtime suspend is very little while the ODD may malfunction after constantly being reset. So the idle callback here will not proceed to suspend if a non-ZPODD capable ODD is attached to the port.
Signed-off-by Aaron Lu <[email protected]>
Thanks.
Signature · 1 line
tejun
PATCH v12 9/9 scsi: remove can_power_off flag from scsi_device
In reply to Aaron LuCommit 166a2967b45ede2e2e56f3ede3cd32053dc17812 "libata: tell scsi layer device supports runtime power off" introduced the can_power_off flag for scsi_device and is used to support ZPODD implementation in SCSI layer. Since ZPODD is now implemented in ATA layer, that flag is no longer needed, so remove it.
--- include/scsi/scsi_device.h | 1 - 1 file changed, 1 deletion(-)
include/scsi/scsi_device.h +0 -1
@@ -157,7 +157,6 @@ struct scsi_device { unsigned no_read_capacity_16:1; /* Avoid READ_CAPACITY_16 cmds */ unsigned try_rc_10_first:1; /* Try READ_CAPACACITY_10 first */ unsigned is_visible:1; /* is the device visible in sysfs */ - unsigned can_power_off:1; /* Device supports runtime power off */ unsigned wce_default_on:1; /* Cache is ON by default */ unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
Signature · 1 line
1.7.11.7