Re: [PATCH] enclosure: add support for enclosure services
On Tue, 12 Feb 2008 13:28:15 -0600 James Bottomley [EMAIL PROTECTED] wrote: On Tue, 2008-02-12 at 11:07 -0800, Kristen Carlson Accardi wrote: I understand what you are trying to do - I guess I just doubt the value you've added by doing this. I think that there's going to be so much customization that system vendors will want to add, that they are going to wind up adding a custom library regardless, so standardising those few things won't buy us anything. It depends ... if you actually have a use for the customisations, yes. If you just want the basics of who (what's in the enclousure), what (activity) and where (locate) then I think it solves your problem almost entirely. So, entirely as a straw horse, tell me what else your enclosures provide that I haven't listed in the four points. The SES standards too provide a huge range of things that no-one ever seems to implement (temperature, power, fan speeds etc). I think the users of enclosures fall int these categories 85% just want to know where their device actually is (i.e. that sdc is in enclosure slot 5) 50% like watching the activity lights 30% want to be able to have a visual locate function 20% want a visual failure indication (the other 80% rely on some OS notification instead) When you add up the overlapping needs, you get about 90% of people happy with the basics that the enclosure services provide. Could there be more ... sure; should there be more ... I don't think so ... that's what value add the user libraries can provide. James I don't think I'm arguing whether or not your solution may work, what I am arguing is really a more philosophical point. Not can we do it this way, but should we do it way. I am of the opinion that management belongs in userspace. I also am of the opinion that if you can successfully accomplish something in user space, you should. I also believe that even if you provide this basic interface, all system vendors are going to provide libraries on top of that to customize it, so you've not added much value to just a simple message passing interface. So, I'm happy to defer to Jeff's judgement call here - I just want to do what's right for our customers and get an enclosure management interface for SATA exposed, preferrably in time for the 2.6.26 merge window. If he prefers your design, I'll disagree, but commit to his decision and try to get this to work for SATA. If he'd rather see something along the lines of what I proposed, then since it is 100% self contained in the SATA subsystem, it shouldn't impact whatever you want to do in the SCSI subsystem. Jeff? - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] enclosure: add support for enclosure services
On Tue, 12 Feb 2008 12:45:35 -0600 James Bottomley [EMAIL PROTECTED] wrote: On Tue, 2008-02-12 at 10:22 -0800, Kristen Carlson Accardi wrote: I apologize for taking so long to review this patch. I obviously agree wholeheartedly with Luben. The problem I ran into while trying to design an enclosure management interface for the SATA devices is that there is all this vendor defined stuff. For example, for the AHCI LED protocol, the only defined LED is 'activity'. For LED2 and LED3 it is up to hardware vendors to define these. For SGPIO there's all kinds of ways for hw vendors to customize. I felt that it was going to be a maintainance nightmare to have to keep track of various vendors enclosure implementations in the ahci driver, and that it'd be better to just have user space libraries take care of that. Plus, that way a vendor doesn't have to get a patch into the kernel to get their new spiffy wizzy bang blinky lights working (think of how long it takes something to even get into a vendor kernel, which is what these guys care about...). So I'm still not sold on having an enclosure abstraction in the kernel - at least for the SATA controllers. Correct me if I'm wrong, but didn't the original AHCI enclosure patch expose activity LEDs via sysfs? You are sort of wrong. we exposed a sysfs entry to enable sofware controlled activity LED, then the driver was responsible for turning it on and off. (blech, I know, but some vendors want this feature). I'm not saying there aren't a lot of non standard pieces that need to be activated by direct commands or other user activated protocol. I am saying there are a lot of standard pieces that we could do with showing in a uniform manner. The pieces I think are absolutely standard are 1. Actual enclosure presence (is this device in an enclosure) 2. Activity LED, this seems to be a feature of every enclosure. I also think the following are reasonably standard (based on the fact that most enclosure standards recommend but don't require this): 3. Locate LED (for locating the device). Even if you only have an activity LED, this is usually done by flashing the activity LED in a well defined pattern. 4. Fault. this is the least standardised of the lot, but does seem to be present in about every enclosure implementation. All I've done is standardise these four pieces ... the services actually take into account that it might not be possible to do certain of these (like fault). James I understand what you are trying to do - I guess I just doubt the value you've added by doing this. I think that there's going to be so much customization that system vendors will want to add, that they are going to wind up adding a custom library regardless, so standardising those few things won't buy us anything. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] enclosure: add support for enclosure services
On Mon, 4 Feb 2008 18:01:36 -0800 (PST) Luben Tuikov [EMAIL PROTECTED] wrote: --- On Mon, 2/4/08, James Bottomley [EMAIL PROTECTED] wrote: The enclosure misc device is really just a library providing sysfs support for physical enclosure devices and their components. Who is the target audience/user of those facilities? a) The kernel itself needing to read/write SES pages? That depends on the enclosure integration, but right at the moment, it doesn't Yes, I didn't suspect so. b) A user space application using sysfs to read/write SES pages? Not an application so much as a user. The idea of sysfs is to allow users to get and set the information in addition to applications. Exactly the same argument stands for a user-space application with a user-space library. This is the classical case of where it is better to do this in user-space as opposed to the kernel. The kernel provides capability to access the SES device. The user space application and library provide interpretation and control. Thus if the enclosure were upgraded, one doesn't need to upgrade their kernel in order to utilize the new capabilities of the SES device. Plus upgrading a user-space application is a lot easier than the kernel (and no reboot necessary). Consider another thing: vendors would really like unprecedented access to the SES device in the enclosure so as your ses/enclosure code keeps state it would get out of sync when vendor user-space enclosure applications access (and modify) the SES device's pages. You can test this yourself: submit a patch that removes SES /dev/sgX support; advertise your ses/class solution and watch the fun. At the moment SES device management is done via an application (user-space) and a user-space library used by the application and /dev/sgX to send SCSI commands to the SES device. I must have missed that when I was looking for implementations; what's the URL? I'm not aware of any GPLed ones. That doesn't necessarily mean that the best course of action is to bloat the kernel. You can move your ses/enclosure stuff to a user space application library and thus start a GPLed one. But, if we have non-scsi enclosures to integrate, that makes it harder for a user application because it has to know all the implementations. So does the kernel. And as I pointed out above, it is a lot easier to upgrade a user-space application and library than it is to upgrade a new kernel and having to reboot the computer to run the new kernel. A sysfs framework on the other hand is a universal known thing for the user applications. So would a user-space ses library, a la libses.so. One could have a very good argument to not bloat the kernel with this but leave it to a user-space application and a library to do all this and communicate with the SES device via the kernel's /dev/sgX. The same thing goes for other esoteric SCSI infrastructure pieces like cd changers. On the whole, given that ATA is asking for enclosure management in kernel, it makes sense to consolidate the infrastructure and a ses ULD is a very good test bed. What is wrong with exporting the SES device as /dev/sgX and having a user-space application and library to do all this? Luben Hi, I apologize for taking so long to review this patch. I obviously agree wholeheartedly with Luben. The problem I ran into while trying to design an enclosure management interface for the SATA devices is that there is all this vendor defined stuff. For example, for the AHCI LED protocol, the only defined LED is 'activity'. For LED2 and LED3 it is up to hardware vendors to define these. For SGPIO there's all kinds of ways for hw vendors to customize. I felt that it was going to be a maintainance nightmare to have to keep track of various vendors enclosure implementations in the ahci driver, and that it'd be better to just have user space libraries take care of that. Plus, that way a vendor doesn't have to get a patch into the kernel to get their new spiffy wizzy bang blinky lights working (think of how long it takes something to even get into a vendor kernel, which is what these guys care about...). So I'm still not sold on having an enclosure abstraction in the kernel - at least for the SATA controllers. Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: use notifier chain for Asynchronous Event Notification
Add a notifier chain for SCSI asynchronous events. Add a notifier block for events which should be sent to user space, and add support for the MEDIA_CHANGE event, which would be used by a driver when new media has been inserted. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] --- This patch is against the latest -mm and is on top of what has already been put in for AN support. It just moves the event notification thread out of scsi_lib.c and into it's own file, as well as adding notifier chain support. Index: 2.6-mm/drivers/scsi/scsi.c === --- 2.6-mm.orig/drivers/scsi/scsi.c +++ 2.6-mm/drivers/scsi/scsi.c @@ -1045,6 +1045,8 @@ static int __init init_scsi(void) if (error) goto cleanup_sysctl; + scsi_aen_init(); + scsi_netlink_init(); printk(KERN_NOTICE SCSI subsystem initialized\n); Index: 2.6-mm/drivers/scsi/scsi_aen.c === --- /dev/null +++ 2.6-mm/drivers/scsi/scsi_aen.c @@ -0,0 +1,168 @@ +/* + * SCSI Asynchronous Event Notification + * + * Copyright (c) 2007, Intel Corporation + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Send Feedback to: Kristen Carlson Accardi [EMAIL PROTECTED] + */ +#include linux/notifier.h + +#include scsi/scsi.h +#include scsi/scsi_cmnd.h +#include scsi/scsi_dbg.h +#include scsi/scsi_device.h +#include scsi/scsi_driver.h +#include scsi/scsi_eh.h +#include scsi/scsi_host.h + +static int scsi_aen_uevent_notifier(struct notifier_block *nb, + unsigned long action, void *sdev); + +BLOCKING_NOTIFIER_HEAD(scsi_aen_chain); + +static struct notifier_block scsi_aen_nb = { + .notifier_call = scsi_aen_uevent_notifier, +}; + +/* must match scsi_device_event enum in scsi_device.h */ +static char * scsi_device_event_strings[] = { + MEDIA_CHANGE=1, +}; + +/**scsi_aen_notifier_register - register to receive aen events + * @nb: callers notifier_block + * + * Add a callers notifier_block to the AEN notify chain. + */ +int scsi_aen_notifier_register(struct notifier_block *nb) +{ + return blocking_notifier_chain_register(scsi_aen_chain, nb); +} +EXPORT_SYMBOL_GPL(scsi_aen_notifier_register); + +/**scsi_aen_notifier_register - register to receive aen events + * @nb: callers notifier_block + * + * remove a callers notifier_block from the AEN notify chain. + */ +int scsi_aen_notifier_unregister(struct notifier_block *nb) +{ + return blocking_notifier_chain_unregister(scsi_aen_chain, nb); +} +EXPORT_SYMBOL_GPL(scsi_aen_notifier_unregister); + +/** + * scsi_device_set_event - Add a new Async event to the event list + * @sdev: scsi_device event occurred on + * @event: the scsi device event + * + * Add a new scsi_device_event_info struct to the scsi_device_event_list + * queue. This may be called from interrupt context. + * + * Returns 0 if successful, -ENOMEM otherwise. + */ +static int +scsi_device_set_event(struct scsi_device *sdev, enum scsi_device_event event) +{ + struct scsi_device_event_info *scsi_event; + unsigned long flags; + + scsi_event = kzalloc(sizeof(*scsi_event), GFP_ATOMIC); + if (!scsi_event) + return -ENOMEM; + INIT_LIST_HEAD(scsi_event-link); + scsi_event-event = event; + spin_lock_irqsave(sdev-list_lock, flags); + list_add_tail(scsi_event-link, sdev-sdev_event_list); + spin_unlock_irqrestore(sdev-list_lock, flags); + return 0; +} + +/** + * scsi_device_event_notify_thread - send a uevent for each scsi event + * @work: work struct for scsi_device + * + * Traverse the queue of scsi device events, dequeue each event and + * send a uevent. Frees event. May not be called from interrupt. + */ +static void scsi_event_notify_thread(struct work_struct *work) +{ + struct scsi_device *sdev; + struct list_head *tmp; + struct list_head *next; + struct scsi_device_event_info *sdev_event; + unsigned long flags; + + sdev = container_of(work, struct scsi_device, ew.work); + list_for_each_safe(tmp, next, sdev-sdev_event_list) { + sdev_event = list_entry(tmp, struct
Re: [patch 1/4] libata: check for AN support
On Wed, 15 Aug 2007 03:59:10 -0400 Jeff Garzik [EMAIL PROTECTED] wrote: Since I wanted to get in Tejun's ata_link changes before this, this will require a rediff. But more importantly, as I was going to apply it I noticed another problem: we need to verify that SiS and NVIDIA both support AN support turning this on, since this patch affects not only Intel but also those vendors' AHCI hardware as well. It's a bit annoying but this is a cross-vendor driver after all... Jeff Got a link to their public docs? I'm happy to look in their specs to confirm. Otherwise, maybe you can forward to people there to have them check, or find someone under NDA with them who can. Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] scsi: expose AN support to user space
On Wed, 15 Aug 2007 09:01:49 -0500 James Bottomley [EMAIL PROTECTED] wrote: On Wed, 2007-08-15 at 04:13 -0400, Jeff Garzik wrote: Kristen Carlson Accardi wrote: If a scsi_device supports async notification for media change, then let user space know this capability exists by creating a new sysfs entry media_change_notify, which will be 1 if it is supported, and 0 if not supported. Create a routine which allows scsi devices to send a uevent when media change events occur. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] if James is happy with this patch, I'm happy with patch #2 and #3 Actually, we just got a second potential consumer ... although I'll reprod to have the reporter send it to the list. It's a device that needs notice of report luns data changing. The proposed mechanism looks a bit narrow now (too tied to media change). I'll see if I can propose a more generic update. James Too narrow because it's tied to scsi_device? Obviously it'd be easy to expand the scsi_device_event enum to include LUNS_DATA_CHANGE or something, and even the scsi_device_event_info struct can easily be expanded if you need more info attached to the event. Let me know if there's something specific I can help with. Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/4] Updated AN patches, now without gendisk
On Sat, 11 Aug 2007 16:00:53 +0200 Kay Sievers [EMAIL PROTECTED] wrote: On 8/8/07, Kristen Carlson Accardi [EMAIL PROTECTED] wrote: Here is an updated set of patches that implement Asynchronous Notification support for ATAPI devices. In this version I no longer export the AN capability through genhd, and the uevent is sent by the scsi_device instead of gendisk. I added a generic SCSI device event mechanism so that it can be expanded in the future with other types of events. Please let me know what you think. Does that mean: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=8ce7ad7b2d11fae2c3d285a6a0caea9322c0b8fc will go away? Yes - this method was rejected by James, so I had to change away from using the genhd to transmit events. Then we will need to come up with an idea to propagate that to the blockdev, it gets a bit nasty for userspace to find out which blockdevice it should update. It's certainly possible, but just not as easy as it is now, because userspace just doesn't really care in most cases what kind of bus device a block device is coming from. Is it possible to use the linkage that is in the scsi_device directory of the disk that is sending the event to find the relevant block devices? For example, /sys/class/scsi_device/0:0:0:0/device/block:sda on my system tells me which block devices belong to this device. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] Expose Power Management Policy option to users
On Wed, 1 Aug 2007 14:16:51 -0700 Kristen Carlson Accardi [EMAIL PROTECTED] wrote: I was able to duplicate Tejun's problem on an ATAPI device I had here. this updated patch fixed my problem. These devices are just setting PhyReady (N) and CommWake (W) in Serror on power state changes, ignoring them seems to be fine, and fixed the problem for me. Hi Tejun, Were you able to test out this patch and see if it solved your ATAPI device/ALPM issues? Thanks, Kristen Enable Aggressive Link Power management for AHCI controllers. This patch will set the correct bits to turn on Aggressive Link Power Management (ALPM) for the ahci driver. This will cause the controller and disk to negotiate a lower power state for the link when there is no activity (see the AHCI 1.x spec for details). This feature is mutually exclusive with Hot Plug, so when ALPM is enabled, Hot Plug is disabled. ALPM will be enabled by default, but it is settable via the scsi host syfs interface. Possible settings for this feature are: Setting Effect -- min_power ALPM is enabled, and link set to enter lowest power state (SLUMBER) when idle Hot plug not allowed. max_performance ALPM is disabled, Hot Plug is allowed medium_power ALPM is enabled, and link set to enter second lowest power state (PARTIAL) when idle. Hot plug not allowed. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -48,6 +48,9 @@ #define DRV_NAME ahci #define DRV_VERSION 2.3 +static int ahci_enable_alpm(struct ata_port *ap, + enum link_pm policy); +static int ahci_disable_alpm(struct ata_port *ap); enum { AHCI_PCI_BAR= 5, @@ -98,6 +101,7 @@ enum { /* HOST_CAP bits */ HOST_CAP_SSC= (1 14), /* Slumber capable */ HOST_CAP_CLO= (1 24), /* Command List Override support */ + HOST_CAP_ALPM = (1 26), /* Aggressive Link PM support */ HOST_CAP_SSS= (1 27), /* Staggered Spin-up */ HOST_CAP_SNTF = (1 29), /* SNotification register */ HOST_CAP_NCQ= (1 30), /* Native Command Queueing */ @@ -153,6 +157,8 @@ enum { PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS, /* PORT_CMD bits */ + PORT_CMD_ASP= (1 27), /* Aggressive Slumber/Partial */ + PORT_CMD_ALPE = (1 26), /* Aggressive Link PM enable */ PORT_CMD_ATAPI = (1 24), /* Device is ATAPI */ PORT_CMD_LIST_ON= (1 15), /* cmd list DMA engine running */ PORT_CMD_FIS_ON = (1 14), /* FIS DMA engine running */ @@ -244,6 +250,11 @@ static int ahci_pci_device_suspend(struc static int ahci_pci_device_resume(struct pci_dev *pdev); #endif +static struct class_device_attribute *ahci_shost_attrs[] = { + class_device_attr_link_power_management_policy, + NULL +}; + static struct scsi_host_template ahci_sht = { .module = THIS_MODULE, .name = DRV_NAME, @@ -261,6 +272,7 @@ static struct scsi_host_template ahci_sh .slave_configure= ata_scsi_slave_config, .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, + .shost_attrs= ahci_shost_attrs, }; static const struct ata_port_operations ahci_ops = { @@ -292,6 +304,8 @@ static const struct ata_port_operations .port_suspend = ahci_port_suspend, .port_resume= ahci_port_resume, #endif + .enable_pm = ahci_enable_alpm, + .disable_pm = ahci_disable_alpm, .port_start = ahci_port_start, .port_stop = ahci_port_stop, @@ -778,6 +792,156 @@ static void ahci_power_up(struct ata_por writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD); } +static int ahci_disable_alpm(struct ata_port *ap) +{ + void __iomem *port_mmio = ahci_port_base(ap); + u32 cmd, scontrol; + struct ahci_port_priv *pp = ap-private_data; + + /* + * disable Interface Power Management State Transitions + * This is accomplished by setting bits 8:11 of the + * SATA Control register + */ + scontrol = readl(port_mmio + PORT_SCR_CTL); + scontrol |= (0x3 8); + writel(scontrol, port_mmio + PORT_SCR_CTL); + + /* get the existing command bits */ + cmd = readl(port_mmio + PORT_CMD); + + /* disable ALPM and ASP */ + cmd = ~PORT_CMD_ASP; + cmd = ~PORT_CMD_ALPE; + + /* force the interface back to active */ + cmd |= PORT_CMD_ICC_ACTIVE
[patch 0/4] Updated AN patches, now without gendisk
Hello, Here is an updated set of patches that implement Asynchronous Notification support for ATAPI devices. In this version I no longer export the AN capability through genhd, and the uevent is sent by the scsi_device instead of gendisk. I added a generic SCSI device event mechanism so that it can be expanded in the future with other types of events. Please let me know what you think. Kristen -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/4] libata: check for AN support
Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-core.c === --- 2.6-git.orig/drivers/ata/libata-core.c +++ 2.6-git/drivers/ata/libata-core.c @@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long static unsigned int ata_dev_init_params(struct ata_device *dev, u16 heads, u16 sectors); static unsigned int ata_dev_set_xfermode(struct ata_device *dev); +static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable); static void ata_dev_xfermask(struct ata_device *dev); static unsigned long ata_dev_blacklisted(const struct ata_device *dev); @@ -1974,6 +1975,22 @@ int ata_dev_configure(struct ata_device } dev-cdb_len = (unsigned int) rc; + /* +* check to see if this ATAPI device supports +* Asynchronous Notification +*/ + if ((ap-flags ATA_FLAG_AN) ata_id_has_AN(id)) { + int err; + /* issue SET feature command to turn this on */ + err = ata_dev_set_AN(dev, SETFEATURES_SATA_ENABLE); + if (err) + ata_dev_printk(dev, KERN_ERR, + unable to set AN, err %x\n, + err); + else + dev-flags |= ATA_DFLAG_AN; + } + if (ata_id_cdb_intr(dev-id)) { dev-flags |= ATA_DFLAG_CDB_INTR; cdb_intr_string = , CDB intr; @@ -3948,6 +3965,42 @@ static unsigned int ata_dev_set_xfermode } /** + * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES + * @dev: Device to which command will be sent + * @enable: Whether to enable or disable the feature + * + * Issue SET FEATURES - SATA FEATURES command to device @dev + * on port @ap with sector count set to indicate Asynchronous + * Notification feature + * + * LOCKING: + * PCI/etc. bus probe sem. + * + * RETURNS: + * 0 on success, AC_ERR_* mask otherwise. + */ +static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable) +{ + struct ata_taskfile tf; + unsigned int err_mask; + + /* set up set-features taskfile */ + DPRINTK(set features - SATA features\n); + + ata_tf_init(dev, tf); + tf.command = ATA_CMD_SET_FEATURES; + tf.feature = enable; + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf.protocol = ATA_PROT_NODATA; + tf.nsect = SATA_AN; + + err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + + DPRINTK(EXIT, err_mask=%x\n, err_mask); + return err_mask; +} + +/** * ata_dev_init_params - Issue INIT DEV PARAMS command * @dev: Device to which command will be sent * @heads: Number of heads (taskfile parameter) Index: 2.6-git/include/linux/ata.h === --- 2.6-git.orig/include/linux/ata.h +++ 2.6-git/include/linux/ata.h @@ -217,6 +217,12 @@ enum { SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ + SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */ + SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */ + + /* SETFEATURE Sector counts for SATA features */ + SATA_AN = 0x05, /* Asynchronous Notification */ + /* ATAPI stuff */ ATAPI_PKT_DMA = (1 0), ATAPI_DMADIR= (1 2), /* ATAPI data dir: @@ -344,6 +350,9 @@ struct ata_taskfile { #define ata_id_queue_depth(id) (((id)[75] 0x1f) + 1) #define ata_id_removeable(id) ((id)[0] (1 7)) #define ata_id_has_dword_io(id)((id)[50] (1 0)) +#define ata_id_has_AN(id) \ + ( (((id)[76] != 0x) ((id)[76] != 0x)) \ + ((id)[78] (1 5)) ) #define ata_id_iordy_disable(id) ((id)[49] (1 10)) #define ata_id_has_iordy(id) ((id)[49] (1 9)) #define ata_id_u32(id,n) \ Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -139,7 +139,8 @@ enum { ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ ATA_DFLAG_ACPI_PENDING = (1 5), /* ACPI resume action pending */ ATA_DFLAG_ACPI_FAILED = (1 6), /* ACPI on devcfg has failed */ - ATA_DFLAG_CFG_MASK = (1 8) - 1, + ATA_DFLAG_AN= (1 7), /* device supports AN */ + ATA_DFLAG_CFG_MASK = (1 12) - 1, ATA_DFLAG_PIO = (1 8), /* device limited to PIO mode */ ATA_DFLAG_NCQ_OFF = (1 9), /* device
[patch 2/4] scsi: expose AN support to user space
If a scsi_device supports async notification for media change, then let user space know this capability exists by creating a new sysfs entry media_change_notify, which will be 1 if it is supported, and 0 if not supported. Create a routine which allows scsi devices to send a uevent when media change events occur. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/include/scsi/scsi_device.h === --- 2.6-git.orig/include/scsi/scsi_device.h +++ 2.6-git/include/scsi/scsi_device.h @@ -46,6 +46,16 @@ enum scsi_device_state { * to the scsi lld. */ }; +/* must match scsi_device_event_strings in scsi_lib.c */ +enum scsi_device_event { + SDEV_MEDIA_CHANGE = 1, /* media has changed */ +}; + +struct scsi_device_event_info { + enum scsi_device_event event; + struct list_head link; +}; + struct scsi_device { struct Scsi_Host *host; struct request_queue *request_queue; @@ -126,7 +136,7 @@ struct scsi_device { unsigned fix_capacity:1;/* READ_CAPACITY is too high by 1 */ unsigned guess_capacity:1; /* READ_CAPACITY might be too high by 1 */ unsigned retry_hwerror:1; /* Retry HARDWARE_ERROR */ - + unsigned media_change_notify:1; /* dev supports async media notify */ unsigned int device_blocked;/* Device returned QUEUE_FULL. */ unsigned int max_device_blocked; /* what device_blocked counts down from */ @@ -144,6 +154,7 @@ struct scsi_device { struct execute_work ew; /* used to get process context on put */ enum scsi_device_state sdev_state; + struct list_headsdev_event_list; unsigned long sdev_data[0]; } __attribute__((aligned(sizeof(unsigned long; #defineto_scsi_device(d) \ @@ -275,6 +286,8 @@ extern int scsi_test_unit_ready(struct s int retries); extern int scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state); +extern int scsi_device_event_notify(struct scsi_device *sdev, + enum scsi_device_event event); extern int scsi_device_quiesce(struct scsi_device *sdev); extern void scsi_device_resume(struct scsi_device *sdev); extern void scsi_target_quiesce(struct scsi_target *); Index: 2.6-git/drivers/scsi/scsi_lib.c === --- 2.6-git.orig/drivers/scsi/scsi_lib.c +++ 2.6-git/drivers/scsi/scsi_lib.c @@ -64,6 +64,11 @@ static struct scsi_host_sg_pool scsi_sg_ }; #undef SP +/* must match scsi_device_event enum in scsi_device.h */ +static char * scsi_device_event_strings[] = { + MEDIA_CHANGE=1, +}; + static void scsi_run_queue(struct request_queue *q); /* @@ -2007,6 +2012,84 @@ scsi_device_set_state(struct scsi_device EXPORT_SYMBOL(scsi_device_set_state); /** + * scsi_device_set_event - Add a new Async event to the event list + * @sdev: scsi_device event occurred on + * @event: the scsi device event + * + * Add a new scsi_device_event_info struct to the scsi_device_event_list + * queue. This may be called from interrupt context. + * + * Returns 0 if successful, -ENOMEM otherwise. + */ +static int +scsi_device_set_event(struct scsi_device *sdev, enum scsi_device_event event) +{ + struct scsi_device_event_info *scsi_event; + unsigned long flags; + + scsi_event = kzalloc(sizeof(*scsi_event), GFP_ATOMIC); + if (!scsi_event) + return -ENOMEM; + INIT_LIST_HEAD(scsi_event-link); + scsi_event-event = event; + spin_lock_irqsave(sdev-list_lock, flags); + list_add_tail(scsi_event-link, sdev-sdev_event_list); + spin_unlock_irqrestore(sdev-list_lock, flags); + return 0; +} + +/** + * scsi_device_event_notify_thread - send a uevent for each scsi event + * @work: work struct for scsi_device + * + * Traverse the queue of scsi device events, dequeue each event and + * send a uevent. Frees event. May not be called from interrupt. + */ +static void scsi_event_notify_thread(struct work_struct *work) +{ + struct scsi_device *sdev; + char *envp[] = { NULL, NULL }; + struct list_head *tmp; + struct list_head *next; + struct scsi_device_event_info *sdev_event; + unsigned long flags; + + sdev = container_of(work, struct scsi_device, ew.work); + list_for_each_safe(tmp, next, sdev-sdev_event_list) { + sdev_event = list_entry(tmp, struct scsi_device_event_info, + link); + spin_lock_irqsave(sdev-list_lock, flags); + list_del(sdev_event-link); + spin_unlock_irqrestore(sdev-list_lock, flags); + envp[0] = scsi_device_event_strings[sdev_event-event-1]; + kobject_uevent_env(sdev
[patch 3/4] libata: expose AN to user space
If Asynchronous Notification of media change events is supported, pass that information up to the SCSI layer. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -771,6 +771,9 @@ static void ata_scsi_dev_config(struct s blk_queue_max_hw_segments(q, q-max_hw_segments - 1); } + if (dev-flags ATA_DFLAG_AN) + sdev-media_change_notify = 1; + if (dev-flags ATA_DFLAG_NCQ) { int depth; -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 4/4] libata: send event when AN received
When we get an SDB FIS with the 'N' bit set, we should send an event to user space to indicate that there has been a media change. This will be done via the scsi device. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -1359,6 +1359,28 @@ static void ahci_port_intr(struct ata_po return; } + if (status PORT_IRQ_SDB_FIS) { + /* +* if this is an ATAPI device with AN turned on, +* then we should interrogate the device to +* determine the cause of the interrupt +* +* for AN - this we should check the SDB FIS +* and find the I and N bits set +*/ + const u32 *f = pp-rx_fis + RX_FIS_SDB; + + /* check the 'N' bit in word 0 of the FIS */ + if (f[0] (1 15)) { + int port_addr = ((f[0] 0x0f00) 8); + struct ata_device *adev; + if (port_addr ATA_MAX_DEVICES) { + adev = ap-device[port_addr]; + if (adev-flags ATA_DFLAG_AN) + ata_scsi_media_change_notify(adev); + } + } + } if (ap-sactive) qc_active = readl(port_mmio + PORT_SCR_ACT); else Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -737,6 +737,7 @@ extern void ata_host_init(struct ata_hos extern int ata_scsi_detect(struct scsi_host_template *sht); extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg); extern int ata_scsi_queuecmd(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)); +extern void ata_scsi_media_change_notify(struct ata_device *atadev); extern void ata_sas_port_destroy(struct ata_port *); extern struct ata_port *ata_sas_port_alloc(struct ata_host *, struct ata_port_info *, struct Scsi_Host *); Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -3099,6 +3099,22 @@ static void ata_scsi_remove_dev(struct a } /** + * ata_scsi_media_change_notify - send media change event + * @atadev: Pointer to the disk device with media change event + * + * Tell the block layer to send a media change notification + * event. + * + * LOCKING: + * interrupt context, may not sleep. + */ +void ata_scsi_media_change_notify(struct ata_device *atadev) +{ + scsi_device_event_notify(atadev-sdev, SDEV_MEDIA_CHANGE); +} +EXPORT_SYMBOL_GPL(ata_scsi_media_change_notify); + +/** * ata_scsi_hotplug - SCSI part of hotplug * @work: Pointer to ATA port to perform SCSI hotplug on * -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] Expose Power Management Policy option to users
On Wed, 01 Aug 2007 12:24:44 +0900 Tejun Heo [EMAIL PROTECTED] wrote: It would be *really* great if we can find more about how they do it. How and when it's enabled and on which systems. Is it possible to find this out? No - it's really not a good idea for us to go and ask other OS's how they implement things in this level of detail. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] Expose Power Management Policy option to users
On Wed, 01 Aug 2007 18:23:21 +0900 Tejun Heo [EMAIL PROTECTED] wrote: Tejun Heo wrote: Arjan van de Ven wrote: They were hardware problems. I don't think any amount of proper implementation can fix them. I have one DVD RAM somewhere in my pile of hardware which locks up solidly if any link PS mode is used and had a and the AHCI ALPM code decides to use power savings on this device? if so, please give us the idents so that we can add it to the blacklist as the first entry... (or can buy it to check it in detail first) Yeap, lemme check. It's TSSTcorpCD/DVDW SH-S183A with firmware revision SB01. Wanna check ID capability bits but 'hdparm -I /dev/sr0' is still broken and it's already past 3 am here. I'll report back tomorrow. Oops, that was the wrong one. Locking up one was HL-DT-STDVD-RAM GSA-H30N and it correctly reports that it doesn't support IPM. Here are some test results. Controller is ICH9. 00:1f.2 SATA controller [Class 0106]: Intel Corporation Unknown device [8086:2922] (rev 02) 1. HL-DT-STDVD-RAM GSA-H30N ATAPI CD-ROM, with removable media Model Number: HL-DT-STDVD-RAM GSA-H30N Serial Number: Firmware Revision: 1.00 Standards: Likely used CD-ROM ATAPI-1 Configuration: DRQ response: 50us. Packet size: 12 bytes Capabilities: LBA, IORDY(can be disabled) DMA: sdma0 sdma1 sdma2 mdma0 mdma1 mdma2 udma0 udma1 udma2 udma3 udma4 *udma5 Cycle time: min=120ns recommended=120ns PIO: pio0 pio1 pio2 pio3 pio4 Cycle time: no flow control=120ns IORDY flow control=120 This device doesn't claim to support HIPM nor DIPM. Are you sure you are using the latest version of the patch? This seems like a bug, if the device doesn't support HIPM or DIPM it shouldn't attempt to use ALPM. There was a check for this put into the patch on about the second or third version - maybe I'm not doing it right. (from the patch located here: http://www.kernel.org/pub/linux/kernel/people/kristen/patches/SATA/alpm/libata-enable-pm if (ata_id_has_hipm(dev-id) || ata_id_has_dipm(dev-id)) + dev-flags |= ATA_DFLAG_IPM; + snip 2. TSSTcorpCD/DVDW SH-S183A ATAPI CD-ROM, with removable media Model Number: TSSTcorpCD/DVDW SH-S183A Serial Number: Firmware Revision: SB01 Standards: Likely used CD-ROM ATAPI-1 Configuration: DRQ response: 50us. Packet size: 12 bytes Capabilities: LBA, IORDY(can be disabled) DMA: mdma0 mdma1 mdma2 udma0 udma1 *udma2 Cycle time: min=120ns recommended=120ns PIO: pio0 pio1 pio2 pio3 pio4 Cycle time: no flow control=120ns IORDY flow control=120ns Commands/features: Enabled Supported: *SATA-I signaling speed (1.5Gb/s) *Host-initiated interface power management *Phy event counters Device-initiated interface power management unknown 78[5] *Software settings preservation This one claims to support HIPS. # echo min_power link_power_management_policy [ 1301.917248] ata1.00: exception Emask 0x10 SAct 0x0 SErr 0x5 action 0x2 [ 1301.938338] ata1.00: irq_stat 0x4001 [ 1301.956955] ata1.00: cmd a0/00:00:00:00:20/00:00:00:00:00/a0 tag 0 cdb 0x43 data 12 in [ 1301.956959] res 51/20:03:00:00:00/00:00:00:00:00/a0 Emask 0x10 (ATA bus error) [ 1304.189565] ata1: soft resetting port [ 1304.207745] ata1: SATA link down (SStatus 611 SControl 300) [ 1304.228076] ata1: failed to recover some devices, retrying in 5 secs [ 1309.245599] ata1: hard resetting port [ 1314.773227] ata1: port is slow to respond, please be patient (Status 0x80) [ 1319.269677] ata1: COMRESET failed (errno=-16) [ 1319.289639] ata1: hard resetting port [ 1319.781285] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300) [ 1320.115569] ata1.00: configured for UDMA/33 [ 1320.134780] ata1: EH complete And hotplug works fine after EH is done with itself. Dunno why. It works because after EH you've reset the port, and when you reset the port you turn off ipm (note the value of SControl). I left this the way it was without attempting to renable link pm after a reset because I figured if there was something bad happening and we needed to reset we should leave ipm off. As far as the failure you are seeing goes - this may not actually be a hardware problem but just a case of us not understanding which bits we need to clear out of the Interrupt status register once we enable ALPM. The value of SErr indicates that this device has gotten PhyRdy - which is normal for power state changes, and the interrupt status indicates that a d2h FIS was received which is also normal - the TFES bit seems to be set - it may be that we
Re: [patch 3/4] Enable link power management for ata drivers
On Wed, 01 Aug 2007 17:27:39 +0900 Tejun Heo [EMAIL PROTECTED] wrote: Kristen Carlson Accardi wrote: snippy Is it safe to use ALPM on a device which only claims to support DIPM? Yes - I doubled checked this with the AHCI people - and of course you have Edvin's testing to prove it does fine. I think this should be ATA_HORKAGE_IPM. OK - I changed it. @@ -321,6 +321,8 @@ struct ata_taskfile { ((u64) (id)[(n) + 0]) ) #define ata_id_cdb_intr(id)(((id)[0] 0x60) == 0x20) +#define ata_id_has_hipm(id)((id)[76] (1 9)) +#define ata_id_has_dipm(id)((id)[78] (1 3)) We probably need !0x test here. Thanks, I fixed that too. As far as moving the enable/disable_pm calls to EH - can you take a look at the other patch I sent which implements the shost_attrs to see if I still need to do this? I really don't know much about the EH stuff - can you explain why we need to use it to set the link pm? thanks for your review, Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] Expose Power Management Policy option to users
I was able to duplicate Tejun's problem on an ATAPI device I had here. this updated patch fixed my problem. These devices are just setting PhyReady (N) and CommWake (W) in Serror on power state changes, ignoring them seems to be fine, and fixed the problem for me. Enable Aggressive Link Power management for AHCI controllers. This patch will set the correct bits to turn on Aggressive Link Power Management (ALPM) for the ahci driver. This will cause the controller and disk to negotiate a lower power state for the link when there is no activity (see the AHCI 1.x spec for details). This feature is mutually exclusive with Hot Plug, so when ALPM is enabled, Hot Plug is disabled. ALPM will be enabled by default, but it is settable via the scsi host syfs interface. Possible settings for this feature are: Setting Effect -- min_power ALPM is enabled, and link set to enter lowest power state (SLUMBER) when idle Hot plug not allowed. max_performance ALPM is disabled, Hot Plug is allowed medium_powerALPM is enabled, and link set to enter second lowest power state (PARTIAL) when idle. Hot plug not allowed. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -48,6 +48,9 @@ #define DRV_NAME ahci #define DRV_VERSION2.3 +static int ahci_enable_alpm(struct ata_port *ap, + enum link_pm policy); +static int ahci_disable_alpm(struct ata_port *ap); enum { AHCI_PCI_BAR= 5, @@ -98,6 +101,7 @@ enum { /* HOST_CAP bits */ HOST_CAP_SSC= (1 14), /* Slumber capable */ HOST_CAP_CLO= (1 24), /* Command List Override support */ + HOST_CAP_ALPM = (1 26), /* Aggressive Link PM support */ HOST_CAP_SSS= (1 27), /* Staggered Spin-up */ HOST_CAP_SNTF = (1 29), /* SNotification register */ HOST_CAP_NCQ= (1 30), /* Native Command Queueing */ @@ -153,6 +157,8 @@ enum { PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS, /* PORT_CMD bits */ + PORT_CMD_ASP= (1 27), /* Aggressive Slumber/Partial */ + PORT_CMD_ALPE = (1 26), /* Aggressive Link PM enable */ PORT_CMD_ATAPI = (1 24), /* Device is ATAPI */ PORT_CMD_LIST_ON= (1 15), /* cmd list DMA engine running */ PORT_CMD_FIS_ON = (1 14), /* FIS DMA engine running */ @@ -244,6 +250,11 @@ static int ahci_pci_device_suspend(struc static int ahci_pci_device_resume(struct pci_dev *pdev); #endif +static struct class_device_attribute *ahci_shost_attrs[] = { + class_device_attr_link_power_management_policy, + NULL +}; + static struct scsi_host_template ahci_sht = { .module = THIS_MODULE, .name = DRV_NAME, @@ -261,6 +272,7 @@ static struct scsi_host_template ahci_sh .slave_configure= ata_scsi_slave_config, .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, + .shost_attrs= ahci_shost_attrs, }; static const struct ata_port_operations ahci_ops = { @@ -292,6 +304,8 @@ static const struct ata_port_operations .port_suspend = ahci_port_suspend, .port_resume= ahci_port_resume, #endif + .enable_pm = ahci_enable_alpm, + .disable_pm = ahci_disable_alpm, .port_start = ahci_port_start, .port_stop = ahci_port_stop, @@ -778,6 +792,156 @@ static void ahci_power_up(struct ata_por writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD); } +static int ahci_disable_alpm(struct ata_port *ap) +{ + void __iomem *port_mmio = ahci_port_base(ap); + u32 cmd, scontrol; + struct ahci_port_priv *pp = ap-private_data; + + /* +* disable Interface Power Management State Transitions +* This is accomplished by setting bits 8:11 of the +* SATA Control register +*/ + scontrol = readl(port_mmio + PORT_SCR_CTL); + scontrol |= (0x3 8); + writel(scontrol, port_mmio + PORT_SCR_CTL); + + /* get the existing command bits */ + cmd = readl(port_mmio + PORT_CMD); + + /* disable ALPM and ASP */ + cmd = ~PORT_CMD_ASP; + cmd = ~PORT_CMD_ALPE; + + /* force the interface back to active */ + cmd |= PORT_CMD_ICC_ACTIVE; + + /* write out new cmd value */ + writel(cmd, port_mmio + PORT_CMD); + cmd = readl(port_mmio + PORT_CMD); + + /* wait 10ms to be sure we've come out of any low power state */ + msleep(10
Re: [patch 1/4] Store interrupt value
Store interrupt value Use a stored value for which interrupts to enable. Changing this allows us to selectively turn off certain interrupts later and have them stay off. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -175,6 +175,7 @@ enum { AHCI_FLAG_32BIT_ONLY= (1 28), /* force 32bit */ AHCI_FLAG_MV_PATA = (1 29), /* PATA port */ AHCI_FLAG_NO_MSI= (1 30), /* no PCI MSI */ + AHCI_FLAG_NO_HOTPLUG= (1 31), /* ignore PxSERR.DIAG.N */ AHCI_FLAG_COMMON= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY | ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA | @@ -215,6 +216,7 @@ struct ahci_port_priv { unsigned intncq_saw_d2h:1; unsigned intncq_saw_dmas:1; unsigned intncq_saw_sdb:1; + u32 intr_mask; /* interrupts to enable */ }; static int ahci_scr_read(struct ata_port *ap, unsigned int sc_reg, u32 *val); @@ -1518,6 +1520,7 @@ static void ahci_thaw(struct ata_port *a void __iomem *mmio = ap-host-iomap[AHCI_PCI_BAR]; void __iomem *port_mmio = ahci_port_base(ap); u32 tmp; + struct ahci_port_priv *pp = ap-private_data; /* clear IRQ */ tmp = readl(port_mmio + PORT_IRQ_STAT); @@ -1525,7 +1528,7 @@ static void ahci_thaw(struct ata_port *a writel(1 ap-port_no, mmio + HOST_IRQ_STAT); /* turn IRQ back on */ - writel(DEF_PORT_IRQ, port_mmio + PORT_IRQ_MASK); + writel(pp-intr_mask, port_mmio + PORT_IRQ_MASK); } static void ahci_error_handler(struct ata_port *ap) @@ -1679,6 +1682,12 @@ static int ahci_port_start(struct ata_po pp-cmd_tbl = mem; pp-cmd_tbl_dma = mem_dma; + /* +* Save off initial list of interrupts to be enabled. +* This could be changed later +*/ + pp-intr_mask = DEF_PORT_IRQ; + ap-private_data = pp; /* engage engines, captain */ - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] Expose Power Management Policy option to users
On Wed, 01 Aug 2007 12:24:44 +0900 Tejun Heo [EMAIL PROTECTED] wrote: Kristen Carlson Accardi wrote: I don't think the interface you're suggesting is a good one. Do you? I think if it's applicable to SCSI at all it is fine. If it is not, then I think we need to make do with the interface we are given. I do not think we should hold up a feature for libata sysfs integration. Well, I guess we'll have to agree to disagree here and leave the decision to James and Jeff. A compromise to avoiding having this in SCSI is to use the shost_attrs array in the scsi host template. This will not change the way the interface will look, it will just remove any link power management stuff from the scsi subsystem directly and have it be an ATA only attribute. I did go ahead and leave the Documentation file in the scsi directory because that is where the attribute winds up being visible to the user. Here's the patch, and this series is also available at http://www.kernel.org/pub/linux/kernel/people/kristen/patches/SATA/alpm: Enable link power management for ata drivers libata drivers can define a function (enable_pm) that will perform hardware specific actions to enable whatever power management policy the user set up from the scsi sysfs interface if the driver supports it. This power management policy will be activated after all disks have been enumerated and intialized. Drivers should also define disable_pm, which will turn off link power management, but not change link power management policy. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -110,6 +110,78 @@ static struct scsi_transport_template at .user_scan = ata_scsi_user_scan, }; +static const struct { + enum link_pmvalue; + char*name; +} link_pm_policy[] = { + { NOT_AVAILABLE, max_performance }, + { MIN_POWER, min_power }, + { MAX_PERFORMANCE, max_performance }, + { MEDIUM_POWER, medium_power }, +}; + +const char *ata_scsi_link_pm_policy(enum link_pm policy) +{ + int i; + char *name = NULL; + + for (i = 0; i ARRAY_SIZE(link_pm_policy); i++) { + if (link_pm_policy[i].value == policy) { + name = link_pm_policy[i].name; + break; + } + } + return name; +} + +static ssize_t store_link_pm_policy(struct class_device *class_dev, + const char *buf, size_t count) +{ + struct Scsi_Host *shost = class_to_shost(class_dev); + struct ata_port *ap = ata_shost_to_port(shost); + enum link_pm policy = 0; + int i; + + /* +* we are skipping array location 0 on purpose - this +* is because a value of NOT_AVAILABLE is displayed +* to the user as max_performance, but when the user +* writes max_performance, they actually want the +* value to match MAX_PERFORMANCE. +*/ + for (i = 1; i ARRAY_SIZE(link_pm_policy); i++) { + const int len = strlen(link_pm_policy[i].name); + if (strncmp(link_pm_policy[i].name, buf, len) == 0 + buf[len] == '\n') { + policy = link_pm_policy[i].value; + break; + } + } + if (!policy) + return -EINVAL; + + if (ata_scsi_set_link_pm_policy(ap, policy)) + return -EINVAL; + return count; +} + +static ssize_t +show_link_pm_policy(struct class_device *class_dev, char *buf) +{ + struct Scsi_Host *shost = class_to_shost(class_dev); + struct ata_port *ap = ata_shost_to_port(shost); + const char *policy = + ata_scsi_link_pm_policy(ap-pm_policy); + + if (!policy) + return -EINVAL; + + return snprintf(buf, 23, %s\n, policy); +} +CLASS_DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR, + show_link_pm_policy, store_link_pm_policy); +EXPORT_SYMBOL_GPL(class_device_attr_link_power_management_policy); + static void ata_scsi_invalid_field(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)) { @@ -2904,6 +2976,47 @@ void ata_scsi_simulate(struct ata_device } } +int ata_scsi_set_link_pm_policy(struct ata_port *ap, + enum link_pm policy) +{ + int rc = -EINVAL; + int i; + + /* +* make sure no broken devices are on this port, +* and that all devices support interface power +* management +*/ + for (i = 0; i ATA_MAX_DEVICES; i++) { + struct ata_device *dev = ap-device[i]; + + /* only check drives which exist */ + if (!ata_dev_enabled(dev)) + continue
[patch 0/3] Updated ALPM patches
Hi, This is an updated series of ALPM patches. I moved all link power management stuff out of scsi sysfs and just use the shost_addr pointer in the host template to create the sysfs files. This provides the same interface as before, without requiring any scsi changes. I also fixed a bug in the handling of the interrupts for some devices (mostly ATAPI it seems) which will need some bits out of Serror cleared on power state changes, and I also modified the code slightly based on feedback from tejun. please review, Thanks, Kristen -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/3] Store interrupt value
Use a stored value for which interrupts to enable. Changing this allows us to selectively turn off certain interrupts later and have them stay off. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -175,6 +175,7 @@ enum { AHCI_FLAG_32BIT_ONLY= (1 28), /* force 32bit */ AHCI_FLAG_MV_PATA = (1 29), /* PATA port */ AHCI_FLAG_NO_MSI= (1 30), /* no PCI MSI */ + AHCI_FLAG_NO_HOTPLUG= (1 31), /* ignore PxSERR.DIAG.N */ AHCI_FLAG_COMMON= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY | ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA | @@ -215,6 +216,7 @@ struct ahci_port_priv { unsigned intncq_saw_d2h:1; unsigned intncq_saw_dmas:1; unsigned intncq_saw_sdb:1; + u32 intr_mask; /* interrupts to enable */ }; static int ahci_scr_read(struct ata_port *ap, unsigned int sc_reg, u32 *val); @@ -1518,6 +1520,7 @@ static void ahci_thaw(struct ata_port *a void __iomem *mmio = ap-host-iomap[AHCI_PCI_BAR]; void __iomem *port_mmio = ahci_port_base(ap); u32 tmp; + struct ahci_port_priv *pp = ap-private_data; /* clear IRQ */ tmp = readl(port_mmio + PORT_IRQ_STAT); @@ -1525,7 +1528,7 @@ static void ahci_thaw(struct ata_port *a writel(1 ap-port_no, mmio + HOST_IRQ_STAT); /* turn IRQ back on */ - writel(DEF_PORT_IRQ, port_mmio + PORT_IRQ_MASK); + writel(pp-intr_mask, port_mmio + PORT_IRQ_MASK); } static void ahci_error_handler(struct ata_port *ap) @@ -1679,6 +1682,12 @@ static int ahci_port_start(struct ata_po pp-cmd_tbl = mem; pp-cmd_tbl_dma = mem_dma; + /* +* Save off initial list of interrupts to be enabled. +* This could be changed later +*/ + pp-intr_mask = DEF_PORT_IRQ; + ap-private_data = pp; /* engage engines, captain */ -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/3] Enable link power management for ata drivers
libata drivers can define a function (enable_pm) that will perform hardware specific actions to enable whatever power management policy the user set up from the scsi sysfs interface if the driver supports it. This power management policy will be activated after all disks have been enumerated and intialized. Drivers should also define disable_pm, which will turn off link power management, but not change link power management policy. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -110,6 +110,78 @@ static struct scsi_transport_template at .user_scan = ata_scsi_user_scan, }; +static const struct { + enum link_pmvalue; + char*name; +} link_pm_policy[] = { + { NOT_AVAILABLE, max_performance }, + { MIN_POWER, min_power }, + { MAX_PERFORMANCE, max_performance }, + { MEDIUM_POWER, medium_power }, +}; + +const char *ata_scsi_link_pm_policy(enum link_pm policy) +{ + int i; + char *name = NULL; + + for (i = 0; i ARRAY_SIZE(link_pm_policy); i++) { + if (link_pm_policy[i].value == policy) { + name = link_pm_policy[i].name; + break; + } + } + return name; +} + +static ssize_t store_link_pm_policy(struct class_device *class_dev, + const char *buf, size_t count) +{ + struct Scsi_Host *shost = class_to_shost(class_dev); + struct ata_port *ap = ata_shost_to_port(shost); + enum link_pm policy = 0; + int i; + + /* +* we are skipping array location 0 on purpose - this +* is because a value of NOT_AVAILABLE is displayed +* to the user as max_performance, but when the user +* writes max_performance, they actually want the +* value to match MAX_PERFORMANCE. +*/ + for (i = 1; i ARRAY_SIZE(link_pm_policy); i++) { + const int len = strlen(link_pm_policy[i].name); + if (strncmp(link_pm_policy[i].name, buf, len) == 0 + buf[len] == '\n') { + policy = link_pm_policy[i].value; + break; + } + } + if (!policy) + return -EINVAL; + + if (ata_scsi_set_link_pm_policy(ap, policy)) + return -EINVAL; + return count; +} + +static ssize_t +show_link_pm_policy(struct class_device *class_dev, char *buf) +{ + struct Scsi_Host *shost = class_to_shost(class_dev); + struct ata_port *ap = ata_shost_to_port(shost); + const char *policy = + ata_scsi_link_pm_policy(ap-pm_policy); + + if (!policy) + return -EINVAL; + + return snprintf(buf, 23, %s\n, policy); +} +CLASS_DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR, + show_link_pm_policy, store_link_pm_policy); +EXPORT_SYMBOL_GPL(class_device_attr_link_power_management_policy); + static void ata_scsi_invalid_field(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)) { @@ -2904,6 +2976,47 @@ void ata_scsi_simulate(struct ata_device } } +int ata_scsi_set_link_pm_policy(struct ata_port *ap, + enum link_pm policy) +{ + int rc = -EINVAL; + int i; + + /* +* make sure no broken devices are on this port, +* and that all devices support interface power +* management +*/ + for (i = 0; i ATA_MAX_DEVICES; i++) { + struct ata_device *dev = ap-device[i]; + + /* only check drives which exist */ + if (!ata_dev_enabled(dev)) + continue; + + /* +* do we need to handle the case where we've hotplugged +* a broken drive (since hotplug and ALPM are mutually +* exclusive) ? +* +* If so, if we detect a broken drive on a port with +* alpm already enabled, then we should reset the policy +* to off for the entire port. +*/ + if ((dev-horkage ATA_HORKAGE_IPM) || + !(dev-flags ATA_DFLAG_IPM)) { + ata_dev_printk(dev, KERN_ERR, + Unable to set Link PM policy\n); + ap-pm_policy = MAX_PERFORMANCE; + } + } + + if (ap-ops-enable_pm) + rc = ap-ops-enable_pm(ap, policy); + return rc; +} +EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy); + int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht) { int i, rc; Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig
[patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
This patch will set the correct bits to turn on Aggressive Link Power Management (ALPM) for the ahci driver. This will cause the controller and disk to negotiate a lower power state for the link when there is no activity (see the AHCI 1.x spec for details). This feature is mutually exclusive with Hot Plug, so when ALPM is enabled, Hot Plug is disabled. ALPM will be enabled by default, but it is settable via the scsi host syfs interface. Possible settings for this feature are: Setting Effect -- min_power ALPM is enabled, and link set to enter lowest power state (SLUMBER) when idle Hot plug not allowed. max_performance ALPM is disabled, Hot Plug is allowed medium_powerALPM is enabled, and link set to enter second lowest power state (PARTIAL) when idle. Hot plug not allowed. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -48,6 +48,9 @@ #define DRV_NAME ahci #define DRV_VERSION2.3 +static int ahci_enable_alpm(struct ata_port *ap, + enum link_pm policy); +static int ahci_disable_alpm(struct ata_port *ap); enum { AHCI_PCI_BAR= 5, @@ -98,6 +101,7 @@ enum { /* HOST_CAP bits */ HOST_CAP_SSC= (1 14), /* Slumber capable */ HOST_CAP_CLO= (1 24), /* Command List Override support */ + HOST_CAP_ALPM = (1 26), /* Aggressive Link PM support */ HOST_CAP_SSS= (1 27), /* Staggered Spin-up */ HOST_CAP_SNTF = (1 29), /* SNotification register */ HOST_CAP_NCQ= (1 30), /* Native Command Queueing */ @@ -153,6 +157,8 @@ enum { PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS, /* PORT_CMD bits */ + PORT_CMD_ASP= (1 27), /* Aggressive Slumber/Partial */ + PORT_CMD_ALPE = (1 26), /* Aggressive Link PM enable */ PORT_CMD_ATAPI = (1 24), /* Device is ATAPI */ PORT_CMD_LIST_ON= (1 15), /* cmd list DMA engine running */ PORT_CMD_FIS_ON = (1 14), /* FIS DMA engine running */ @@ -244,6 +250,11 @@ static int ahci_pci_device_suspend(struc static int ahci_pci_device_resume(struct pci_dev *pdev); #endif +static struct class_device_attribute *ahci_shost_attrs[] = { + class_device_attr_link_power_management_policy, + NULL +}; + static struct scsi_host_template ahci_sht = { .module = THIS_MODULE, .name = DRV_NAME, @@ -261,6 +272,7 @@ static struct scsi_host_template ahci_sh .slave_configure= ata_scsi_slave_config, .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, + .shost_attrs= ahci_shost_attrs, }; static const struct ata_port_operations ahci_ops = { @@ -292,6 +304,8 @@ static const struct ata_port_operations .port_suspend = ahci_port_suspend, .port_resume= ahci_port_resume, #endif + .enable_pm = ahci_enable_alpm, + .disable_pm = ahci_disable_alpm, .port_start = ahci_port_start, .port_stop = ahci_port_stop, @@ -778,6 +792,156 @@ static void ahci_power_up(struct ata_por writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD); } +static int ahci_disable_alpm(struct ata_port *ap) +{ + void __iomem *port_mmio = ahci_port_base(ap); + u32 cmd, scontrol; + struct ahci_port_priv *pp = ap-private_data; + + /* +* disable Interface Power Management State Transitions +* This is accomplished by setting bits 8:11 of the +* SATA Control register +*/ + scontrol = readl(port_mmio + PORT_SCR_CTL); + scontrol |= (0x3 8); + writel(scontrol, port_mmio + PORT_SCR_CTL); + + /* get the existing command bits */ + cmd = readl(port_mmio + PORT_CMD); + + /* disable ALPM and ASP */ + cmd = ~PORT_CMD_ASP; + cmd = ~PORT_CMD_ALPE; + + /* force the interface back to active */ + cmd |= PORT_CMD_ICC_ACTIVE; + + /* write out new cmd value */ + writel(cmd, port_mmio + PORT_CMD); + cmd = readl(port_mmio + PORT_CMD); + + /* wait 10ms to be sure we've come out of any low power state */ + msleep(10); + + /* clear out any PhyRdy stuff from interrupt status */ + writel(PORT_IRQ_PHYRDY, port_mmio + PORT_IRQ_STAT); + + /* go ahead and clean out PhyRdy Change from Serror too */ + ahci_scr_write(ap, SCR_ERROR, ((1 16) | (1 18))); + + /* +* Clear flag to indicate that we should ignore all
Re: [patch 2/4] Expose Power Management Policy option to users
On Tue, 31 Jul 2007 23:45:25 +0900 Tejun Heo [EMAIL PROTECTED] wrote: Anyways, I don't really think this attribute belongs to SCSI sysfs hierarchy. There currently isn't any alternative but sysfs is part of userland visible interface and putting something into SCSI sysfs hierarchy just because libata doesn't have one doesn't look like a good idea. sysfs isn't far from being detached from kobject and driver model. I think it would be best to wait a bit and build proper libata sysfs hierarchy which won't have to be changed later when libata departs from SCSI. Well, it isn't really a good way but IMHO it's better than sticking ATA power saving node into SCSI sysfs hierarchy. Wait a bit could be a very long time. Who is working on building this new libata sysfs support now? If the answer is no one, which I think it may be, do you want to hold up a feature that actually helps many people for possibly 6 months or more just because we have to go through scsi right now for our sysfs interface? on top of that, the last mail I got from James on this subject indicated that if we kept our granularity large with the power savings levels, SCSI can actually take advantage of this as well. Sure, we may have to tweak things around later, but isn't this what we do routinely? Holding up valuable features from the kernel because things aren't perfect yet seems really broken. As far as your complaints about broken hardware go, keep in mind that the patch set does provide a method of adding these disks to a blacklist, so I don't see that as a problem. And, the default for this feature is off, and user space would have to explicitly enable it. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] Expose Power Management Policy option to users
On Tue, 31 Jul 2007 15:27:34 +0900 Tejun Heo [EMAIL PROTECTED] wrote: Jeff Garzik wrote: Any chance the SCSI peeps could ACK this, and then let me include it in the ALPM patchset in the libata tree? ATA link PS is pretty complex with HIPM, DIPM and AHCI ALPM. I'm not sure whether this three level knob would be sufficient. It might be good enough if we're gonna develop extensive in-kernel black/white list specifying which method works on which combination but my gut tells me that it's best left to userland (probably in the form of per-notebook PS profile). I think what you are saying is that you'd like a way to use your HIPM and DIPM without ALPM on the AHCI driver. Fine - it's really easy to add these levels later - if they don't make sense at the sysfs interface we can add module params to specify the definition of min_power as being performed via HIPM and DIPM instead of ALPM - although as of yet we have no evidence what so ever that this method actually adds value over ALPM. Adding to the fun, there are quite a few broken devices out there which act weirdly when link PS actions are taken. OK - this is why I added the blacklist for this feature. Also, I generally don't think AHCI ALPM is a good idea. It doesn't have 'cool down' period before entering PS state which unnecessarily hampers performance and might increase chance of device malfunction. might increase? How about some actual examples of where you've shown this to be a problem? I can assert that I think ALPM is a good idea, because I've never had a report of it causing problems. Windows has been using this feature for a very long time - and you have to admit that they have a pretty large market share. Nobody is complaining about ALPM increasing device malfunction, so unless you have proof it seems insane to nak due to this. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] Expose Power Management Policy option to users
On Wed, 01 Aug 2007 03:02:55 +0900 Tejun Heo [EMAIL PROTECTED] wrote: Kristen Carlson Accardi wrote: I think what you are saying is that you'd like a way to use your HIPM and DIPM without ALPM on the AHCI driver. Fine - it's really easy to add these levels later - if they don't make sense at the sysfs interface we can add module params to specify the definition of min_power as being performed via HIPM and DIPM instead of ALPM - although as of yet we have no evidence what so ever that this method actually adds value over ALPM. I don't really care whose PS implementation goes in. Believe me. I try to stay away from that. I don't even like my previous implementation. ALPM has unnecessary performance penalty is not applicable to non-ahci controller. Have you tested ALPM on non-intel ahcis? There are a lot out there these days. I have not personally, however there has been a lot of testing of this hardware feature both on other OS and for this particular implementation by the powertop community, which is composed of community members running all sorts of hardware. So far I've not received any bug reports wrt non-intel AHCI. As I mentioned several times, the default for ALPM is to be off anyway. I don't think the interface you're suggesting is a good one. Do you? I think if it's applicable to SCSI at all it is fine. If it is not, then I think we need to make do with the interface we are given. I do not think we should hold up a feature for libata sysfs integration. Also, I generally don't think AHCI ALPM is a good idea. It doesn't have 'cool down' period before entering PS state which unnecessarily hampers performance and might increase chance of device malfunction. might increase? How about some actual examples of where you've shown this to be a problem? I wouldn't have used might if I had actual examples. Well, feel free to disregard anything following the might. I just feel uneasy about jumping back and forth between PS and active states between consecutive commands. I want us to be careful about spreading a lot of unease without data to back it up. I can assert that I think ALPM is a good idea, because I've never had a report of it causing problems. Windows has been using this feature for a very long time - and you have to admit that they have a pretty large market share. Nobody is complaining about ALPM increasing device malfunction, so unless you have proof it seems insane to nak due to this. Is ALPM enabled by default? How do they deal with the performance degradation? I believe so, but I'm obviously not privvy to their implementation details. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2/4] Expose Power Management Policy option to users
On Wed, 01 Aug 2007 02:48:42 +0900 Tejun Heo [EMAIL PROTECTED] wrote: Hello, Kristen. Kristen Carlson Accardi wrote: On Tue, 31 Jul 2007 23:45:25 +0900 Tejun Heo [EMAIL PROTECTED] wrote: Anyways, I don't really think this attribute belongs to SCSI sysfs hierarchy. There currently isn't any alternative but sysfs is part of userland visible interface and putting something into SCSI sysfs hierarchy just because libata doesn't have one doesn't look like a good idea. sysfs isn't far from being detached from kobject and driver model. I think it would be best to wait a bit and build proper libata sysfs hierarchy which won't have to be changed later when libata departs from SCSI. Well, it isn't really a good way but IMHO it's better than sticking ATA power saving node into SCSI sysfs hierarchy. Wait a bit could be a very long time. Who is working on building this new libata sysfs support now? I happen to be working on sysfs these days and guess why I started working on sysfs. :-) Disassociating sysfs from driver model is probably one or two patchsets away. It could have happened earlier but I wanted to pace things a bit so that the changes receive some testing through release cycles. Check how many sysfs related changes went through .23-rc1 merge window and expect about the same influx during the next cycle; with some luck, it can be complete before .24-rc1 window. So at current rate of development and kernel release schedule, the best possible scenario is still 6 months away - whereas this patchset is already tested and ready for merging now. If the answer is no one, which I think it may be, do you want to hold up a feature that actually helps many people for possibly 6 months or more just because we have to go through scsi right now for our sysfs interface? I don't necessarily want to but delaying it might be the right thing to do. Anyways, if we're going to do an interim solution, I don't think mainline SCSI sysfs hierarchy is the right place. Do it with module parameter which carries large to be deprecated sign or maintain out of tree patches. Out of tree patches don't work. Nobody tests them. A module parameter will not work - we need to be able to expose the sysfs interface so that users may chose to turn the feature off and on at will - mainly according to their usage. For example, at boot time - you want ALPM off to maximize performance. Lets say when you are plugged into the wall socket, you leave it off, but then when you go on battery power you would want to enable it. on top of that, the last mail I got from James on this subject indicated that if we kept our granularity large with the power savings levels, SCSI can actually take advantage of this as well. Sure, we may have to tweak things around later, but isn't this what we do routinely? Holding up valuable features from the kernel because things aren't perfect yet seems really broken. As far as your complaints about broken hardware go, keep in mind that the patch set does provide a method of adding these disks to a blacklist, so I don't see that as a problem. And, the default for this feature is off, and user space would have to explicitly enable it. This is gonna be a bit long. Please be patient. Due to the generosity of the ATA committee, we have, by default, at least two ways to achieve link PS - HIPS and DIPS. I dunno why but someone thought we needed two. And then, ahci people thought automatic They thought we needed two because sometimes the device knows when it will be idle faster than the host can. this is why ALPM can determine idleness faster than any software algorithm on the host cpu can. You can use ALPM without having HIPM. You can also use it without having DIPM. HIPS would be nice, which I fully agree, and added ALPM. Unfortunately, they mandated ALPM to kick in the moment command engine becomes idle, so most current implementations suffer unnecessary performance hit when ALPM is active. unnecessary is subjective and at the moment, untrue. You have to make performance/power tradeoffs with anything, whether it's CPU or your AHCI controller. It's the current cost of getting out of these sleep states even if you aren't using ALPM but just doing HIPM or DIPM manually. But, this is why it's so critical to allow the user to control when ALPM is enabled dynamically - which this patchset does. The medium power setting is provided for users who wish to not go to SLUMBER and get the higher latency cost but still have some power savings. We have this crazy number of combinations. HIPS, DIPS, not-so-hot-looking ALPM and probably some number of broken devices which may be happy with some method but not with others. Some might be happy with PARTIAL but vomit on SLUMBER. I might be being too pessimistic but my last two years in IDE/ATA land taught me to be pessimistic about hardware quality
Re: [patch 2/4] Expose Power Management Policy option to users
On Mon, 9 Jul 2007 19:36:04 + Pavel Machek [EMAIL PROTECTED] wrote: Hi! This patch will modify the scsi subsystem to allow users to set a power management policy for the link. The scsi subsystem will create a new sysfs file for each host in /sys/class/scsi_host called link_power_management_policy. This file can have 3 possible values: Value Meaning --- min_power User wishes the link to conserve power as much as possible, even at the cost of some performance max_performance User wants priority to be on performance, not power savings medium_powerUser wants power savings, with less performance cost than min_power (but less power savings as well). Has that anything to do with HIPM vs. DIPM? Pavel Hi Pavel, I'm not sure I'm understanding your question, but if you mean the different levels of power savings you would get, no. With ALPM you have the option of instructing the link to either go into slumber or partial mode when it determines the link is quiet. Slumber uses less power, but has more latency to come out of. So, for a SATA device, setting the link_power_managment file to medium_power will set up the link to only go into Partial mode, which has less power savings (about half by my informal measurement), but less latency, and therefore less of a performance hit. Hopefully this answers your question. Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 0/4] SATA power savings patches (ALPM)
Hi Jeff, Here's the most recent patches for ALPM. These are also located at: http://www.kernel.org/pub/linux/kernel/people/kristen/patches/SATA/alpm These patches implement Aggressive Link Power management for AHCI controllers. This feature is described in detail in the AHCI 1.x spec. It provides power savings of anywhere from .5-1.5 Watts depending on the system when it is enabled. (a graph demonstrating power savings of this patch along with other power patches is located at http://www.linuxpowertop.org/results.php for your enjoyment). This patch is changed from the previous version in that it incorporates all the feedback I've gotten on the patches to date. It changes the default of the ahci driver to be max_performance rather than min_power, so userspace will need to explicitly enable alpm via the sysfs interface when it deems it an appropriate time to do this. In addition, the patch was changed to allow either HIPM or DIPM as a prerequisite for enabling ALPM, rather than just using HIPM. Thanks, Kristen -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/4] Store interrupt value
Use a stored value for which interrupts to enable. Changing this allows us to selectively turn off certain interrupts later and have them stay off. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -211,6 +211,7 @@ struct ahci_port_priv { unsigned intncq_saw_d2h:1; unsigned intncq_saw_dmas:1; unsigned intncq_saw_sdb:1; + u32 intr_mask; /* interrupts to enable */ }; static u32 ahci_scr_read (struct ata_port *ap, unsigned int sc_reg); @@ -1408,6 +1409,7 @@ static void ahci_thaw(struct ata_port *a void __iomem *mmio = ap-host-iomap[AHCI_PCI_BAR]; void __iomem *port_mmio = ahci_port_base(ap); u32 tmp; + struct ahci_port_priv *pp = ap-private_data; /* clear IRQ */ tmp = readl(port_mmio + PORT_IRQ_STAT); @@ -1415,7 +1417,7 @@ static void ahci_thaw(struct ata_port *a writel(1 ap-port_no, mmio + HOST_IRQ_STAT); /* turn IRQ back on */ - writel(DEF_PORT_IRQ, port_mmio + PORT_IRQ_MASK); + writel(pp-intr_mask, port_mmio + PORT_IRQ_MASK); } static void ahci_error_handler(struct ata_port *ap) @@ -1571,6 +1573,12 @@ static int ahci_port_start(struct ata_po pp-cmd_tbl = mem; pp-cmd_tbl_dma = mem_dma; + /* +* Save off initial list of interrupts to be enabled. +* This could be changed later +*/ + pp-intr_mask = DEF_PORT_IRQ; + ap-private_data = pp; /* power up port */ -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/4] Expose Power Management Policy option to users
This patch will modify the scsi subsystem to allow users to set a power management policy for the link. The scsi subsystem will create a new sysfs file for each host in /sys/class/scsi_host called link_power_management_policy. This file can have 3 possible values: Value Meaning --- min_power User wishes the link to conserve power as much as possible, even at the cost of some performance max_performance User wants priority to be on performance, not power savings medium_powerUser wants power savings, with less performance cost than min_power (but less power savings as well). Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/hosts.c === --- 2.6-git.orig/drivers/scsi/hosts.c +++ 2.6-git/drivers/scsi/hosts.c @@ -54,6 +54,25 @@ static struct class shost_class = { }; /** + * scsi_host_set_link_pm - Change the link power management policy + * @shost: scsi host to change the policy of. + * @policy:policy to change to. + * + * Returns zero if successful or an error if the requested + * transition is illegal. + **/ +int scsi_host_set_link_pm(struct Scsi_Host *shost, + enum scsi_host_link_pm policy) +{ + struct scsi_host_template *sht = shost-hostt; + if (sht-set_link_pm_policy) + sht-set_link_pm_policy(shost, policy); + + return 0; +} +EXPORT_SYMBOL_GPL(scsi_host_set_link_pm); + +/** * scsi_host_set_state - Take the given host through the host * state model. * @shost: scsi host to change the state of. Index: 2.6-git/drivers/scsi/scsi_sysfs.c === --- 2.6-git.orig/drivers/scsi/scsi_sysfs.c +++ 2.6-git/drivers/scsi/scsi_sysfs.c @@ -189,6 +189,74 @@ show_shost_state(struct class_device *cl static CLASS_DEVICE_ATTR(state, S_IRUGO | S_IWUSR, show_shost_state, store_shost_state); +static const struct { + enum scsi_host_link_pm value; + char*name; +} shost_link_pm_policy[] = { + { SHOST_NOT_AVAILABLE, max_performance }, + { SHOST_MIN_POWER, min_power }, + { SHOST_MAX_PERFORMANCE, max_performance }, + { SHOST_MEDIUM_POWER, medium_power }, +}; + +const char *scsi_host_link_pm_policy(enum scsi_host_link_pm policy) +{ + int i; + char *name = NULL; + + for (i = 0; i ARRAY_SIZE(shost_link_pm_policy); i++) { + if (shost_link_pm_policy[i].value == policy) { + name = shost_link_pm_policy[i].name; + break; + } + } + return name; +} + +static ssize_t store_link_pm_policy(struct class_device *class_dev, + const char *buf, size_t count) +{ + struct Scsi_Host *shost = class_to_shost(class_dev); + enum scsi_host_link_pm policy = 0; + int i; + + /* +* we are skipping array location 0 on purpose - this +* is because a value of SHOST_NOT_AVAILABLE is displayed +* to the user as max_performance, but when the user +* writes max_performance, they actually want the +* value to match SHOST_MAX_PERFORMANCE. +*/ + for (i = 1; i ARRAY_SIZE(shost_link_pm_policy); i++) { + const int len = strlen(shost_link_pm_policy[i].name); + if (strncmp(shost_link_pm_policy[i].name, buf, len) == 0 + buf[len] == '\n') { + policy = shost_link_pm_policy[i].value; + break; + } + } + if (!policy) + return -EINVAL; + + if (scsi_host_set_link_pm(shost, policy)) + return -EINVAL; + return count; +} +static ssize_t +show_link_pm_policy(struct class_device *class_dev, char *buf) +{ + struct Scsi_Host *shost = class_to_shost(class_dev); + const char *policy = + scsi_host_link_pm_policy(shost-shost_link_pm_policy); + + if (!policy) + return -EINVAL; + + return snprintf(buf, 23, %s\n, policy); +} +static CLASS_DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR, + show_link_pm_policy, store_link_pm_policy); + shost_rd_attr(unique_id, %u\n); shost_rd_attr(host_busy, %hu\n); shost_rd_attr(cmd_per_lun, %hd\n); @@ -207,6 +275,7 @@ static struct class_device_attribute *sc class_device_attr_proc_name, class_device_attr_scan, class_device_attr_state, + class_device_attr_link_power_management_policy, NULL }; Index: 2.6-git/include/scsi/scsi_host.h === --- 2.6-git.orig/include/scsi/scsi_host.h +++ 2.6-git/include/scsi/scsi_host.h @@ -42,6 +42,16 @@ enum scsi_eh_timer_return { EH_RESET_TIMER
[patch 3/4] Enable link power management for ata drivers
libata drivers can define a function (enable_pm) that will perform hardware specific actions to enable whatever power management policy the user set up from the scsi sysfs interface if the driver supports it. This power management policy will be activated after all disks have been enumerated and intialized. Drivers should also define disable_pm, which will turn off link power management, but not change link power management policy. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -2905,6 +2905,51 @@ void ata_scsi_simulate(struct ata_device } } +int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost, + enum scsi_host_link_pm policy) +{ + struct ata_port *ap = ata_shost_to_port(shost); + int rc = -EINVAL; + int i; + + /* +* make sure no broken devices are on this port, +* and that all devices support interface power +* management +*/ + for (i = 0; i ATA_MAX_DEVICES; i++) { + struct ata_device *dev = ap-device[i]; + + /* only check drives which exist */ + if (!ata_dev_enabled(dev)) + continue; + + /* +* do we need to handle the case where we've hotplugged +* a broken drive (since hotplug and ALPM are mutually +* exclusive) ? +* +* If so, if we detect a broken drive on a port with +* alpm already enabled, then we should reset the policy +* to off for the entire port. +*/ + if ((dev-horkage ATA_HORKAGE_ALPM) || + !(dev-flags ATA_DFLAG_IPM)) { + ata_dev_printk(dev, KERN_ERR, + Unable to set Link PM policy\n); + ap-pm_policy = SHOST_MAX_PERFORMANCE; + } + } + + if (ap-ops-enable_pm) + rc = ap-ops-enable_pm(ap, policy); + + if (!rc) + shost-shost_link_pm_policy = ap-pm_policy; + return rc; +} +EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy); + int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht) { int i, rc; @@ -2927,7 +2972,7 @@ int ata_scsi_add_hosts(struct ata_host * shost-max_lun = 1; shost-max_channel = 1; shost-max_cmd_len = 16; - + shost-shost_link_pm_policy = ap-pm_policy; rc = scsi_add_host(ap-scsi_host, ap-host-dev); if (rc) goto err_add; Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_IPM = (1 6), /* device supports interface PM */ ATA_DFLAG_CFG_MASK = (1 8) - 1, ATA_DFLAG_PIO = (1 8), /* device limited to PIO mode */ @@ -298,6 +299,7 @@ enum { ATA_HORKAGE_NODMA = (1 1), /* DMA problems */ ATA_HORKAGE_NONCQ = (1 2), /* Don't use NCQ */ ATA_HORKAGE_MAX_SEC_128 = (1 3), /* Limit max sects to 128 */ + ATA_HORKAGE_ALPM= (1 4), /* ALPM problems */ }; enum hsm_task_states { @@ -546,6 +548,7 @@ struct ata_port { pm_message_tpm_mesg; int *pm_result; + enum scsi_host_link_pm pm_policy; void*private_data; @@ -605,7 +608,8 @@ struct ata_port_operations { int (*port_suspend) (struct ata_port *ap, pm_message_t mesg); int (*port_resume) (struct ata_port *ap); - + int (*enable_pm) (struct ata_port *ap, enum scsi_host_link_pm policy); + int (*disable_pm) (struct ata_port *ap); int (*port_start) (struct ata_port *ap); void (*port_stop) (struct ata_port *ap); @@ -811,7 +815,7 @@ extern int ata_cable_40wire(struct ata_p extern int ata_cable_80wire(struct ata_port *ap); extern int ata_cable_sata(struct ata_port *ap); extern int ata_cable_unknown(struct ata_port *ap); - +extern int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost, enum scsi_host_link_pm); /* * Timing helpers */ Index: 2.6-git/drivers/ata/libata-core.c === --- 2.6-git.orig/drivers/ata/libata-core.c +++ 2.6-git/drivers/ata/libata-core.c @@ -2021,6 +2021,9 @@ int ata_dev_configure(struct ata_device
[patch 4/4] Enable Aggressive Link Power management for AHCI controllers.
This patch will set the correct bits to turn on Aggressive Link Power Management (ALPM) for the ahci driver. This will cause the controller and disk to negotiate a lower power state for the link when there is no activity (see the AHCI 1.x spec for details). This feature is mutually exclusive with Hot Plug, so when ALPM is enabled, Hot Plug is disabled. ALPM will be enabled by default, but it is settable via the scsi host syfs interface. Possible settings for this feature are: Setting Effect -- min_power ALPM is enabled, and link set to enter lowest power state (SLUMBER) when idle Hot plug not allowed. max_performance ALPM is disabled, Hot Plug is allowed medium_powerALPM is enabled, and link set to enter second lowest power state (PARTIAL) when idle. Hot plug not allowed. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -48,6 +48,9 @@ #define DRV_NAME ahci #define DRV_VERSION2.2 +static int ahci_enable_alpm(struct ata_port *ap, + enum scsi_host_link_pm policy); +static int ahci_disable_alpm(struct ata_port *ap); enum { AHCI_PCI_BAR= 5, @@ -97,6 +100,7 @@ enum { /* HOST_CAP bits */ HOST_CAP_SSC= (1 14), /* Slumber capable */ HOST_CAP_CLO= (1 24), /* Command List Override support */ + HOST_CAP_ALPM = (1 26), /* Aggressive Link PM support */ HOST_CAP_SSS= (1 27), /* Staggered Spin-up */ HOST_CAP_NCQ= (1 30), /* Native Command Queueing */ HOST_CAP_64 = (1 31), /* PCI DAC (64-bit DMA) support */ @@ -151,6 +155,8 @@ enum { PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS, /* PORT_CMD bits */ + PORT_CMD_ASP= (1 27), /* Aggressive Slumber/Partial */ + PORT_CMD_ALPE = (1 26), /* Aggressive Link PM enable */ PORT_CMD_ATAPI = (1 24), /* Device is ATAPI */ PORT_CMD_LIST_ON= (1 15), /* cmd list DMA engine running */ PORT_CMD_FIS_ON = (1 14), /* FIS DMA engine running */ @@ -171,6 +177,7 @@ enum { AHCI_FLAG_HONOR_PI = (1 26), /* honor PORTS_IMPL */ AHCI_FLAG_IGN_SERR_INTERNAL = (1 27), /* ignore SERR_INTERNAL */ AHCI_FLAG_32BIT_ONLY= (1 28), /* force 32bit */ + AHCI_FLAG_NO_HOTPLUG= (1 29), /* ignore PxSERR.DIAG.N */ AHCI_FLAG_COMMON= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY | ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA | @@ -253,6 +260,7 @@ static struct scsi_host_template ahci_sh .slave_configure= ata_scsi_slave_config, .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, + .set_link_pm_policy = ata_scsi_set_link_pm_policy, }; static const struct ata_port_operations ahci_ops = { @@ -284,6 +292,8 @@ static const struct ata_port_operations .port_suspend = ahci_port_suspend, .port_resume= ahci_port_resume, #endif + .enable_pm = ahci_enable_alpm, + .disable_pm = ahci_disable_alpm, .port_start = ahci_port_start, .port_stop = ahci_port_stop, @@ -719,6 +729,156 @@ static void ahci_power_up(struct ata_por writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD); } +static int ahci_disable_alpm(struct ata_port *ap) +{ + void __iomem *port_mmio = ahci_port_base(ap); + u32 cmd, scontrol; + struct ahci_port_priv *pp = ap-private_data; + + /* +* disable Interface Power Management State Transitions +* This is accomplished by setting bits 8:11 of the +* SATA Control register +*/ + scontrol = readl(port_mmio + PORT_SCR_CTL); + scontrol |= (0x3 8); + writel(scontrol, port_mmio + PORT_SCR_CTL); + + /* get the existing command bits */ + cmd = readl(port_mmio + PORT_CMD); + + /* disable ALPM and ASP */ + cmd = ~PORT_CMD_ASP; + cmd = ~PORT_CMD_ALPE; + + /* force the interface back to active */ + cmd |= PORT_CMD_ICC_ACTIVE; + + /* write out new cmd value */ + writel(cmd, port_mmio + PORT_CMD); + cmd = readl(port_mmio + PORT_CMD); + + /* wait 10ms to be sure we've come out of any low power state */ + msleep(10); + + /* clear out any PhyRdy stuff from interrupt status */ + writel(PORT_IRQ_PHYRDY, port_mmio + PORT_IRQ_STAT); + + /* go ahead and clean out PhyRdy Change from Serror too */ + ahci_scr_write(ap
[patch 0/3] clean gendisk out of scsi ULD structs
Since gendisk will now become part of struct scsi_device, we don't need to store this value in any private data structs where they already store scsi_device. This series cleans up a few drivers which did this. Kristen -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/3] SCSI: osst: remove gendisk from private data struct
Since gendisk is now part of scsi_device, don't store this struct in private data struct Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/osst.c === --- 2.6-git.orig/drivers/scsi/osst.c +++ 2.6-git/drivers/scsi/osst.c @@ -191,7 +191,7 @@ static int osst_write_error_recovery(str static inline char *tape_name(struct osst_tape *tape) { - return tape-drive-disk_name; + return tape-device-disk-disk_name; } /* Routines that handle the interaction with mid-layer SCSI routines */ @@ -5803,7 +5803,6 @@ static int osst_probe(struct device *dev drive-private_data = tpnt-driver; sprintf(drive-disk_name, osst%d, dev_num); tpnt-driver = osst_template; - tpnt-drive = drive; tpnt-in_use = 0; tpnt-capacity = 0xf; tpnt-dirty = 0; @@ -5885,6 +5884,8 @@ static int osst_probe(struct device *dev goto out_free_sysfs1; } + SDp-disk = drive; + sdev_printk(KERN_INFO, SDp, osst :I: Attached OnStream %.5s tape as %s\n, SDp-model, tape_name(tpnt)); @@ -5915,7 +5916,7 @@ static int osst_remove(struct device *de osst_sysfs_destroy(MKDEV(OSST_MAJOR, i)); osst_sysfs_destroy(MKDEV(OSST_MAJOR, i+128)); tpnt-device = NULL; - put_disk(tpnt-drive); + put_disk(tpnt-device-disk); os_scsi_tapes[i] = NULL; osst_nr_dev--; write_unlock(os_scsi_tapes_lock); @@ -5988,7 +5989,7 @@ static void __exit exit_osst (void) normalize_buffer(STp-buffer); kfree(STp-buffer); } - put_disk(STp-drive); + put_disk(STp-device-disk); kfree(STp); } kfree(os_scsi_tapes); Index: 2.6-git/drivers/scsi/osst.h === --- 2.6-git.orig/drivers/scsi/osst.h +++ 2.6-git/drivers/scsi/osst.h @@ -623,7 +623,6 @@ struct osst_tape { unsigned char last_cmnd[6]; unsigned char last_sense[16]; #endif - struct gendisk *drive; } ; /* scsi tape command */ -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/3] SCSI: sr: remove gendisk from private data struct
Since gendisk is now part of scsi_device, don't store this struct in private data struct Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/sr.c === --- 2.6-git.orig/drivers/scsi/sr.c +++ 2.6-git/drivers/scsi/sr.c @@ -265,9 +265,9 @@ static void rw_intr(struct scsi_cmnd * S * last 75 2K sectors, we decrease the saved size * value. */ - if (error_sector get_capacity(cd-disk) + if (error_sector get_capacity(cd-device-disk) cd-capacity - error_sector 4 * 75) - set_capacity(cd-disk, error_sector); + set_capacity(cd-device-disk, error_sector); break; case RECOVERED_ERROR: @@ -302,7 +302,7 @@ static int sr_init_command(struct scsi_c struct scsi_cd *cd = scsi_cd(SCpnt-request-rq_disk); SCSI_LOG_HLQUEUE(1, printk(Doing sr request, dev = %s, block = %d\n, - cd-disk-disk_name, block)); + cd-device-disk-disk_name, block)); if (!cd-device || !scsi_device_online(cd-device)) { SCSI_LOG_HLQUEUE(2, printk(Finishing %ld sectors\n, @@ -571,9 +571,7 @@ static int sr_probe(struct device *dev) disk-flags = GENHD_FL_CD; cd-device = sdev; - cd-disk = disk; cd-driver = sr_template; - cd-disk = disk; cd-capacity = 0x1f; cd-device-changed = 1;/* force recheck CD type */ cd-use = 1; @@ -603,7 +601,10 @@ static int sr_probe(struct device *dev) dev_set_drvdata(dev, cd); disk-flags |= GENHD_FL_REMOVABLE; + if (sdev-media_change_notify) + disk-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; add_disk(disk); + sdev-disk = disk; sdev_printk(KERN_DEBUG, sdev, Attached scsi CD-ROM %s\n, cd-cdi.name); @@ -688,7 +689,7 @@ static void get_sectorsize(struct scsi_c * Add this so that we have the ability to correctly gauge * what the device is capable of. */ - set_capacity(cd-disk, cd-capacity); + set_capacity(cd-device-disk, cd-capacity); } queue = cd-device-request_queue; @@ -850,7 +851,7 @@ static int sr_packet(struct cdrom_device static void sr_kref_release(struct kref *kref) { struct scsi_cd *cd = container_of(kref, struct scsi_cd, kref); - struct gendisk *disk = cd-disk; + struct gendisk *disk = cd-device-disk; spin_lock(sr_index_lock); clear_bit(disk-first_minor, sr_index_bits); @@ -869,7 +870,7 @@ static int sr_remove(struct device *dev) { struct scsi_cd *cd = dev_get_drvdata(dev); - del_gendisk(cd-disk); + del_gendisk(cd-device-disk); mutex_lock(sr_ref_mutex); kref_put(cd-kref, sr_kref_release); Index: 2.6-git/drivers/scsi/sr.h === --- 2.6-git.orig/drivers/scsi/sr.h +++ 2.6-git/drivers/scsi/sr.h @@ -38,10 +38,9 @@ typedef struct scsi_cd { unsigned readcd_known:1;/* drive supports READ_CD (0xbe) */ unsigned readcd_cdda:1; /* reading audio data using READ_CD */ struct cdrom_device_info cdi; - /* We hold gendisk and scsi_device references on probe and use + /* We hold scsi_device references on probe and use * the refs on this kref to decide when to release them */ struct kref kref; - struct gendisk *disk; } Scsi_CD; int sr_do_ioctl(Scsi_CD *, struct packet_command *); -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Fri, 22 Jun 2007 21:00:00 +0200 Jens Axboe [EMAIL PROTECTED] wrote: Exactly, it needs to be handled by some power management daemon anyway and be integrated with power savings in general. You could use io load to determine when to enable/disable alpm, for instance. Acked-by: Kristen Carlson Accardi [EMAIL PROTECTED] Will you integrate it into the next posting? yes, I will do that. Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2b/3] Expose Power Management Policy option to users
Enable link power management for ata drivers libata drivers can define a function (enable_pm) that will perform hardware specific actions to enable whatever power management policy the user set up from the scsi sysfs interface if the driver supports it. This power management policy will be activated after all disks have been enumerated and initialized. Drivers should also define disable_pm, which will turn off link power management, but not change link power management policy. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] --- I've updated this patch to fix a problem with suspend breaking when alpm is enabled. To fix this, I've changed the patch to allow drivers to have a disable_pm function which can be used by the host controller to turn off link power managment before requesting an PM activity. We will turn it back on after Resume. This whole series can be found at: http://www.kernel.org/pub/linux/kernel/people/kristen/patches/SATA/alpm/ Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -2909,6 +2909,51 @@ void ata_scsi_simulate(struct ata_device } } +int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost, + enum scsi_host_link_pm policy) +{ + struct ata_port *ap = ata_shost_to_port(shost); + int rc = -EINVAL; + int i; + + /* +* make sure no broken devices are on this port, +* and that all devices support interface power +* management +*/ + for (i = 0; i ATA_MAX_DEVICES; i++) { + struct ata_device *dev = ap-device[i]; + + /* only check drives which exist */ + if (!ata_dev_enabled(dev)) + continue; + + /* +* do we need to handle the case where we've hotplugged +* a broken drive (since hotplug and ALPM are mutually +* exclusive) ? +* +* If so, if we detect a broken drive on a port with +* alpm already enabled, then we should reset the policy +* to off for the entire port. +*/ + if ((dev-horkage ATA_HORKAGE_ALPM) || + !(dev-flags ATA_DFLAG_IPM)) { + ata_dev_printk(dev, KERN_ERR, + Unable to set Link PM policy\n); + ap-pm_policy = SHOST_MAX_PERFORMANCE; + } + } + + if (ap-ops-enable_pm) + rc = ap-ops-enable_pm(ap, policy); + + if (!rc) + shost-shost_link_pm_policy = ap-pm_policy; + return rc; +} +EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy); + int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht) { int i, rc; @@ -2931,7 +2976,7 @@ int ata_scsi_add_hosts(struct ata_host * shost-max_lun = 1; shost-max_channel = 1; shost-max_cmd_len = 16; - + shost-shost_link_pm_policy = ap-pm_policy; rc = scsi_add_host(ap-scsi_host, ap-host-dev); if (rc) goto err_add; Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_IPM = (1 6), /* device supports interface PM */ ATA_DFLAG_CFG_MASK = (1 8) - 1, ATA_DFLAG_PIO = (1 8), /* device limited to PIO mode */ @@ -299,6 +300,7 @@ enum { ATA_HORKAGE_NONCQ = (1 2), /* Don't use NCQ */ ATA_HORKAGE_MAX_SEC_128 = (1 3), /* Limit max sects to 128 */ ATA_HORKAGE_DMA_RW_ONLY = (1 4), /* ATAPI DMA for RW only */ + ATA_HORKAGE_ALPM= (1 5), /* ALPM problems */ }; enum hsm_task_states { @@ -547,6 +549,7 @@ struct ata_port { pm_message_tpm_mesg; int *pm_result; + enum scsi_host_link_pm pm_policy; void*private_data; @@ -606,7 +609,8 @@ struct ata_port_operations { int (*port_suspend) (struct ata_port *ap, pm_message_t mesg); int (*port_resume) (struct ata_port *ap); - + int (*enable_pm) (struct ata_port *ap, enum scsi_host_link_pm policy); + int (*disable_pm) (struct ata_port *ap); int (*port_start) (struct ata_port *ap); void (*port_stop) (struct ata_port *ap); @@ -812,7 +816,7 @@ extern int ata_cable_40wire(struct ata_p extern int ata_cable_80wire(struct ata_port *ap
[patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
Enable Aggressive Link Power management for AHCI controllers. This patch will set the correct bits to turn on Aggressive Link Power Management (ALPM) for the ahci driver. This will cause the controller and disk to negotiate a lower power state for the link when there is no activity (see the AHCI 1.x spec for details). This feature is mutually exclusive with Hot Plug, so when ALPM is enabled, Hot Plug is disabled. ALPM will be enabled by default, but it is settable via the scsi host syfs interface. Possible settings for this feature are: Setting Effect -- min_power ALPM is enabled, and link set to enter lowest power state (SLUMBER) when idle Hot plug not allowed. max_performance ALPM is disabled, Hot Plug is allowed medium_powerALPM is enabled, and link set to enter second lowest power state (PARTIAL) when idle. Hot plug not allowed. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] --- I've changed this patch to define disable_pm, and to change the behavior of ahci_disable_alpm() to not change the link pm policy, and just turn off alpm. this whole patch series can be found here: http://www.kernel.org/pub/linux/kernel/people/kristen/patches/SATA/alpm/ Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -48,6 +48,9 @@ #define DRV_NAME ahci #define DRV_VERSION2.2 +static int ahci_enable_alpm(struct ata_port *ap, + enum scsi_host_link_pm policy); +static int ahci_disable_alpm(struct ata_port *ap); enum { AHCI_PCI_BAR= 5, @@ -97,6 +100,7 @@ enum { /* HOST_CAP bits */ HOST_CAP_SSC= (1 14), /* Slumber capable */ HOST_CAP_CLO= (1 24), /* Command List Override support */ + HOST_CAP_ALPM = (1 26), /* Aggressive Link PM support */ HOST_CAP_SSS= (1 27), /* Staggered Spin-up */ HOST_CAP_NCQ= (1 30), /* Native Command Queueing */ HOST_CAP_64 = (1 31), /* PCI DAC (64-bit DMA) support */ @@ -151,6 +155,8 @@ enum { PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS, /* PORT_CMD bits */ + PORT_CMD_ASP= (1 27), /* Aggressive Slumber/Partial */ + PORT_CMD_ALPE = (1 26), /* Aggressive Link PM enable */ PORT_CMD_ATAPI = (1 24), /* Device is ATAPI */ PORT_CMD_LIST_ON= (1 15), /* cmd list DMA engine running */ PORT_CMD_FIS_ON = (1 14), /* FIS DMA engine running */ @@ -171,6 +177,7 @@ enum { AHCI_FLAG_HONOR_PI = (1 26), /* honor PORTS_IMPL */ AHCI_FLAG_IGN_SERR_INTERNAL = (1 27), /* ignore SERR_INTERNAL */ AHCI_FLAG_32BIT_ONLY= (1 28), /* force 32bit */ + AHCI_FLAG_NO_HOTPLUG= (1 29), /* ignore PxSERR.DIAG.N */ AHCI_FLAG_COMMON= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY | ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA | @@ -253,6 +260,7 @@ static struct scsi_host_template ahci_sh .slave_configure= ata_scsi_slave_config, .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, + .set_link_pm_policy = ata_scsi_set_link_pm_policy, }; static const struct ata_port_operations ahci_ops = { @@ -284,6 +292,8 @@ static const struct ata_port_operations .port_suspend = ahci_port_suspend, .port_resume= ahci_port_resume, #endif + .enable_pm = ahci_enable_alpm, + .disable_pm = ahci_disable_alpm, .port_start = ahci_port_start, .port_stop = ahci_port_stop, @@ -719,6 +729,158 @@ static void ahci_power_up(struct ata_por writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD); } +static int ahci_disable_alpm(struct ata_port *ap) +{ + void __iomem *port_mmio = ahci_port_base(ap); + u32 cmd, scontrol; + struct ahci_port_priv *pp = ap-private_data; + + /* +* disable Interface Power Management State Transitions +* This is accomplished by setting bits 8:11 of the +* SATA Control register +*/ + scontrol = readl(port_mmio + PORT_SCR_CTL); + scontrol |= (0x3 8); + writel(scontrol, port_mmio + PORT_SCR_CTL); + + /* get the existing command bits */ + cmd = readl(port_mmio + PORT_CMD); + + /* disable ALPM and ASP */ + cmd = ~PORT_CMD_ASP; + cmd = ~PORT_CMD_ALPE; + + /* force the interface back to active */ + cmd |= PORT_CMD_ICC_ACTIVE; + + /* write out new cmd value */ + writel(cmd, port_mmio + PORT_CMD); + cmd = readl
Re: [patch 2a/3] Expose Power Management Policy option to users
On Wed, 13 Jun 2007 08:26:48 -0700 James Bottomley [EMAIL PROTECTED] wrote: On Tue, 2007-06-12 at 10:46 -0700, Kristen Carlson Accardi wrote: Expose Power Management Policy option to users This patch will modify the scsi subsystem to allow users to set a power management policy for the link. The scsi subsystem will create a new sysfs file for each host in /sys/class/scsi_host called link_power_management_policy. This file can have 3 possible values: I'm afraid the host isn't really the right place to put the link power management policy (assuming you want to manage the individual links separately) because there isn't a one to one correspondence between links and hosts. To take the model I understand: SAS; the links are managed at the phy level, so the power policy should be set there and thus should probably be a property of the phy object, which doesn't even exist in the SCSI model, it only exists in the transport class. It strikes me that even for ATA, the same thing is probably true. Now, I can see that the power management models of all the transports might share some similarities (particularly at this three stage granular level); if so, it might make sense to export helpers from the mid-layer for the transport classes to use for this. Ok - sorry for my ignorance about SCSI - but my sources (i.e. Arjan) tell me that the problem is that Link in ATA land means something different than Link in SCSI land, and that what I really need to do is leave this code under the Host class, but rename it to something that more accurately reflects what it means under SCSI. So, is the word segment more appropriate? Should I rename the file to segment_power_management_policy? Thanks, kristen Value Meaning --- min_power User wishes the link to conserve power as much as possible, even at the cost of some performance max_performance User wants priority to be on performance, not power savings medium_powerUser wants power savings, with less performance cost than min_power (but less power savings as well). These seem like nicely sane and generic values. James - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/3] AHCI Link Power Management
On Wed, 13 Jun 2007 11:04:30 +0200 Pavel Machek [EMAIL PROTECTED] wrote: Hi! I'm not sure about this. We need better PM framework to support powersaving in other controllers and some ahcis don't save much when only link power management is used, do you have data to support this? Yeah, it was some Lenovo notebook. Pavel is more familiar with the hardware. Pavel, what was the notebook which didn't save much power with standard SATA power save but needed port to be completely turned off? Thinkpad x60. Some one Kristen probably used while developing the patch :-). Yes - that confirms my conclusion that the first patch just wasn't done correctly - cause when I measure the power savings with a power meter on the X60 with my patches I see ~ 1W. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/3] AHCI Link Power Management
On Tue, 12 Jun 2007 13:40:15 +0900 Tejun Heo [EMAIL PROTECTED] wrote: I don't think the end result will vary in any significant way. My biggest argument for sw implementation is it can be used for other controllers. What I had in mind when I created the new port operation enable_pm was that other controllers (besides the ahci controller) could define their own method of enabling power management. Maybe for non-ahci controllers this is a software based solution which uses generic SATA dipm/hipm stuff and polling. See patch 2/3 of this series for the implementation of this. Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
On Tue, 12 Jun 2007 11:46:56 -0400 Jeff Garzik [EMAIL PROTECTED] wrote: Matthew Garrett wrote: On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh wrote: On Tue, 12 Jun 2007, Matthew Garrett wrote: Laptop bays are designed to deal with hotplugging PATA - I don't think this is too much of an issue :) The new SATA ones use the SATA hardware hotplug ;-) Just like the pci-e cards use usb2.0 and pci-e hotplug... Yes, but they'll also send an ACPI interrupt even if the SATA host controller doesn't - it's part of the spec for bays. Regardless, having a laptop does not imply having a docking bay. Jeff For bay devices, we can use ACPI just like we do now. For non-bay devices, we can implement hotplug via polling when ALPM is enabled. In my experience most laptop vendors implement extra drive as either PATA in a dock station, USB in a dock station, or a bay device either on the dock station, or on the laptop itself. Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 2a/3] Expose Power Management Policy option to users
Expose Power Management Policy option to users This patch will modify the scsi subsystem to allow users to set a power management policy for the link. The scsi subsystem will create a new sysfs file for each host in /sys/class/scsi_host called link_power_management_policy. This file can have 3 possible values: Value Meaning --- min_power User wishes the link to conserve power as much as possible, even at the cost of some performance max_performance User wants priority to be on performance, not power savings medium_powerUser wants power savings, with less performance cost than min_power (but less power savings as well). Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/hosts.c === --- 2.6-git.orig/drivers/scsi/hosts.c +++ 2.6-git/drivers/scsi/hosts.c @@ -54,6 +54,25 @@ static struct class shost_class = { }; /** + * scsi_host_set_link_pm - Change the link power management policy + * @shost: scsi host to change the policy of. + * @policy:policy to change to. + * + * Returns zero if successful or an error if the requested + * transition is illegal. + **/ +int scsi_host_set_link_pm(struct Scsi_Host *shost, + enum scsi_host_link_pm policy) +{ + struct scsi_host_template *sht = shost-hostt; + if (sht-set_link_pm_policy) + sht-set_link_pm_policy(shost, policy); + + return 0; +} +EXPORT_SYMBOL_GPL(scsi_host_set_link_pm); + +/** * scsi_host_set_state - Take the given host through the host * state model. * @shost: scsi host to change the state of. Index: 2.6-git/drivers/scsi/scsi_sysfs.c === --- 2.6-git.orig/drivers/scsi/scsi_sysfs.c +++ 2.6-git/drivers/scsi/scsi_sysfs.c @@ -189,6 +189,74 @@ show_shost_state(struct class_device *cl static CLASS_DEVICE_ATTR(state, S_IRUGO | S_IWUSR, show_shost_state, store_shost_state); +static const struct { + enum scsi_host_link_pm value; + char*name; +} shost_link_pm_policy[] = { + { SHOST_NOT_AVAILABLE, max_performance }, + { SHOST_MIN_POWER, min_power }, + { SHOST_MAX_PERFORMANCE, max_performance }, + { SHOST_MEDIUM_POWER, medium_power }, +}; + +const char *scsi_host_link_pm_policy(enum scsi_host_link_pm policy) +{ + int i; + char *name = NULL; + + for (i = 0; i ARRAY_SIZE(shost_link_pm_policy); i++) { + if (shost_link_pm_policy[i].value == policy) { + name = shost_link_pm_policy[i].name; + break; + } + } + return name; +} + +static ssize_t store_link_pm_policy(struct class_device *class_dev, + const char *buf, size_t count) +{ + struct Scsi_Host *shost = class_to_shost(class_dev); + enum scsi_host_link_pm policy = 0; + int i; + + /* +* we are skipping array location 0 on purpose - this +* is because a value of SHOST_NOT_AVAILABLE is displayed +* to the user as max_performance, but when the user +* writes max_performance, they actually want the +* value to match SHOST_MAX_PERFORMANCE. +*/ + for (i = 1; i ARRAY_SIZE(shost_link_pm_policy); i++) { + const int len = strlen(shost_link_pm_policy[i].name); + if (strncmp(shost_link_pm_policy[i].name, buf, len) == 0 + buf[len] == '\n') { + policy = shost_link_pm_policy[i].value; + break; + } + } + if (!policy) + return -EINVAL; + + if (scsi_host_set_link_pm(shost, policy)) + return -EINVAL; + return count; +} +static ssize_t +show_link_pm_policy(struct class_device *class_dev, char *buf) +{ + struct Scsi_Host *shost = class_to_shost(class_dev); + const char *policy = + scsi_host_link_pm_policy(shost-shost_link_pm_policy); + + if (!policy) + return -EINVAL; + + return snprintf(buf, 23, %s\n, policy); +} +static CLASS_DEVICE_ATTR(link_power_management_policy, S_IRUGO | S_IWUSR, + show_link_pm_policy, store_link_pm_policy); + shost_rd_attr(unique_id, %u\n); shost_rd_attr(host_busy, %hu\n); shost_rd_attr(cmd_per_lun, %hd\n); @@ -207,6 +275,7 @@ static struct class_device_attribute *sc class_device_attr_proc_name, class_device_attr_scan, class_device_attr_state, + class_device_attr_link_power_management_policy, NULL }; Index: 2.6-git/include/scsi/scsi_host.h === --- 2.6-git.orig/include/scsi/scsi_host.h +++ 2.6-git/include/scsi/scsi_host.h @@ -42,6 +42,16 @@ enum
Re: [patch 2b/3] Expose Power Management Policy option to users
Enable link power management for ata drivers libata drivers can define a function (enable_pm) that will perform hardware specific actions to enable whatever power management policy the user set up from the scsi sysfs interface if the driver supports it. This power management policy will be activated after all disks have been enumerated and intialized. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -2890,6 +2890,51 @@ void ata_scsi_simulate(struct ata_device } } +int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost, + enum scsi_host_link_pm policy) +{ + struct ata_port *ap = ata_shost_to_port(shost); + int rc = -EINVAL; + int i; + + /* +* make sure no broken devices are on this port, +* and that all devices support interface power +* management +*/ + for (i = 0; i ATA_MAX_DEVICES; i++) { + struct ata_device *dev = ap-device[i]; + + /* only check drives which exist */ + if (!ata_dev_enabled(dev)) + continue; + + /* +* do we need to handle the case where we've hotplugged +* a broken drive (since hotplug and ALPM are mutually +* exclusive) ? +* +* If so, if we detect a broken drive on a port with +* alpm already enabled, then we should reset the policy +* to off for the entire port. +*/ + if ((dev-horkage ATA_HORKAGE_ALPM) || + !(dev-flags ATA_DFLAG_IPM)) { + ata_dev_printk(dev, KERN_ERR, + Unable to set Link PM policy\n); + ap-pm_policy = SHOST_MAX_PERFORMANCE; + } + } + + if (ap-ops-enable_pm) + rc = ap-ops-enable_pm(ap, policy); + + if (!rc) + shost-shost_link_pm_policy = ap-pm_policy; + return rc; +} +EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy); + int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht) { int i, rc; @@ -2912,7 +2957,7 @@ int ata_scsi_add_hosts(struct ata_host * shost-max_lun = 1; shost-max_channel = 1; shost-max_cmd_len = 16; - + shost-shost_link_pm_policy = ap-pm_policy; rc = scsi_add_host(ap-scsi_host, ap-host-dev); if (rc) goto err_add; Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_IPM = (1 6), /* device supports interface PM */ ATA_DFLAG_CFG_MASK = (1 8) - 1, ATA_DFLAG_PIO = (1 8), /* device limited to PIO mode */ @@ -300,6 +301,7 @@ enum { ATA_HORKAGE_NONCQ = (1 2), /* Don't use NCQ */ ATA_HORKAGE_MAX_SEC_128 = (1 3), /* Limit max sects to 128 */ ATA_HORKAGE_DMA_RW_ONLY = (1 4), /* ATAPI DMA for RW only */ + ATA_HORKAGE_ALPM= (1 5), /* ALPM problems */ }; enum hsm_task_states { @@ -548,6 +550,7 @@ struct ata_port { pm_message_tpm_mesg; int *pm_result; + enum scsi_host_link_pm pm_policy; void*private_data; @@ -607,7 +610,7 @@ struct ata_port_operations { int (*port_suspend) (struct ata_port *ap, pm_message_t mesg); int (*port_resume) (struct ata_port *ap); - + int (*enable_pm) (struct ata_port *ap, enum scsi_host_link_pm policy); int (*port_start) (struct ata_port *ap); void (*port_stop) (struct ata_port *ap); @@ -812,7 +815,7 @@ extern int ata_cable_40wire(struct ata_p extern int ata_cable_80wire(struct ata_port *ap); extern int ata_cable_sata(struct ata_port *ap); extern int ata_cable_unknown(struct ata_port *ap); - +extern int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost, enum scsi_host_link_pm); /* * Timing helpers */ Index: 2.6-git/drivers/ata/libata-core.c === --- 2.6-git.orig/drivers/ata/libata-core.c +++ 2.6-git/drivers/ata/libata-core.c @@ -2019,6 +2019,9 @@ int ata_dev_configure(struct ata_device if (dev-flags ATA_DFLAG_LBA48) dev-max_sectors = ATA_MAX_SECTORS_LBA48; + if (ata_id_has_hipm(dev-id
[patch 0/3] AHCI Link Power Management
Hi, This series of patches enables Aggressive Link Power Management for AHCI devices, as documented in the AHCI spec. On my laptop (a Lenovo X60), this saves me a full watt of power. On other systems, reported power savings range from .5-1.5 Watts. It has been tested by the kind folks at #powertop with similar results. Please give it a try and let me know what you think. thanks, Kristen -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/3] Store interrupt value
Use a stored value for which interrupts to enable. Changing this allows us to selectively turn off certain interrupts later and have them stay off. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -211,6 +211,7 @@ struct ahci_port_priv { unsigned intncq_saw_d2h:1; unsigned intncq_saw_dmas:1; unsigned intncq_saw_sdb:1; + u32 intr_mask; /* interrupts to enable */ }; static u32 ahci_scr_read (struct ata_port *ap, unsigned int sc_reg); @@ -1384,6 +1385,7 @@ static void ahci_thaw(struct ata_port *a void __iomem *mmio = ap-host-iomap[AHCI_PCI_BAR]; void __iomem *port_mmio = ahci_port_base(ap); u32 tmp; + struct ahci_port_priv *pp = ap-private_data; /* clear IRQ */ tmp = readl(port_mmio + PORT_IRQ_STAT); @@ -1391,7 +1393,7 @@ static void ahci_thaw(struct ata_port *a writel(1 ap-port_no, mmio + HOST_IRQ_STAT); /* turn IRQ back on */ - writel(DEF_PORT_IRQ, port_mmio + PORT_IRQ_MASK); + writel(pp-intr_mask, port_mmio + PORT_IRQ_MASK); } static void ahci_error_handler(struct ata_port *ap) @@ -1547,6 +1549,12 @@ static int ahci_port_start(struct ata_po pp-cmd_tbl = mem; pp-cmd_tbl_dma = mem_dma; + /* +* Save off initial list of interrupts to be enabled. +* This could be changed later +*/ + pp-intr_mask = DEF_PORT_IRQ; + ap-private_data = pp; /* power up port */ -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 2/3] Expose Power Management Policy option to users
This patch will modify the scsi and ata subsystem to allow users to set a power management policy for the link. libata drivers can define a function (enable_pm) that will perform hardware specific actions to enable whatever power management policy the user sets up if the driver supports it. This power management policy will be activated after all disks have been enumerated and intialized. The scsi subsystem will create a new sysfs file for each host in /sys/class/scsi_host called link_power_management_policy. This file can have 3 possible values: Value Meaning --- min_power User wishes the link to conserve power as much as possible, even at the cost of some performance max_performance User wants priority to be on performance, not power savings medium_powerUser wants power savings, with less performance cost than min_power (but less power savings as well). Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -2890,6 +2890,51 @@ void ata_scsi_simulate(struct ata_device } } +int ata_scsi_set_link_pm_policy(struct Scsi_Host *shost, + enum scsi_host_link_pm policy) +{ + struct ata_port *ap = ata_shost_to_port(shost); + int rc = -EINVAL; + int i; + + /* +* make sure no broken devices are on this port, +* and that all devices support interface power +* management +*/ + for (i = 0; i ATA_MAX_DEVICES; i++) { + struct ata_device *dev = ap-device[i]; + + /* only check drives which exist */ + if (!ata_dev_enabled(dev)) + continue; + + /* +* do we need to handle the case where we've hotplugged +* a broken drive (since hotplug and ALPM are mutually +* exclusive) ? +* +* If so, if we detect a broken drive on a port with +* alpm already enabled, then we should reset the policy +* to off for the entire port. +*/ + if ((dev-horkage ATA_HORKAGE_ALPM) || + !(dev-flags ATA_DFLAG_IPM)) { + ata_dev_printk(dev, KERN_ERR, + Unable to set Link PM policy\n); + ap-pm_policy = SHOST_MAX_PERFORMANCE; + } + } + + if (ap-ops-enable_pm) + rc = ap-ops-enable_pm(ap, policy); + + if (!rc) + shost-shost_link_pm_policy = ap-pm_policy; + return rc; +} +EXPORT_SYMBOL_GPL(ata_scsi_set_link_pm_policy); + int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht) { int i, rc; @@ -2912,7 +2957,7 @@ int ata_scsi_add_hosts(struct ata_host * shost-max_lun = 1; shost-max_channel = 1; shost-max_cmd_len = 16; - + shost-shost_link_pm_policy = ap-pm_policy; rc = scsi_add_host(ap-scsi_host, ap-host-dev); if (rc) goto err_add; Index: 2.6-git/drivers/scsi/hosts.c === --- 2.6-git.orig/drivers/scsi/hosts.c +++ 2.6-git/drivers/scsi/hosts.c @@ -54,6 +54,25 @@ static struct class shost_class = { }; /** + * scsi_host_set_link_pm - Change the link power management policy + * @shost: scsi host to change the policy of. + * @policy:policy to change to. + * + * Returns zero if successful or an error if the requested + * transition is illegal. + **/ +int scsi_host_set_link_pm(struct Scsi_Host *shost, + enum scsi_host_link_pm policy) +{ + struct scsi_host_template *sht = shost-hostt; + if (sht-set_link_pm_policy) + sht-set_link_pm_policy(shost, policy); + + return 0; +} +EXPORT_SYMBOL_GPL(scsi_host_set_link_pm); + +/** * scsi_host_set_state - Take the given host through the host * state model. * @shost: scsi host to change the state of. Index: 2.6-git/drivers/scsi/scsi_sysfs.c === --- 2.6-git.orig/drivers/scsi/scsi_sysfs.c +++ 2.6-git/drivers/scsi/scsi_sysfs.c @@ -189,6 +189,74 @@ show_shost_state(struct class_device *cl static CLASS_DEVICE_ATTR(state, S_IRUGO | S_IWUSR, show_shost_state, store_shost_state); +static const struct { + enum scsi_host_link_pm value; + char*name; +} shost_link_pm_policy[] = { + { SHOST_NOT_AVAILABLE, max_performance }, + { SHOST_MIN_POWER, min_power }, + { SHOST_MAX_PERFORMANCE, max_performance
[patch 3/3] Enable Aggressive Link Power management for AHCI controllers.
This patch will set the correct bits to turn on Aggressive Link Power Management (ALPM) for the ahci driver. This will cause the controller and disk to negotiate a lower power state for the link when there is no activity (see the AHCI 1.x spec for details). This feature is mutually exclusive with Hot Plug, so when ALPM is enabled, Hot Plug is disabled. ALPM will be enabled by default, but it is settable via the scsi host syfs interface. Possible settings for this feature are: Setting Effect -- min_power ALPM is enabled, and link set to enter lowest power state (SLUMBER) when idle Hot plug not allowed. max_performance ALPM is disabled, Hot Plug is allowed medium_powerALPM is enabled, and link set to enter second lowest power state (PARTIAL) when idle. Hot plug not allowed. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -48,6 +48,9 @@ #define DRV_NAME ahci #define DRV_VERSION2.2 +static int ahci_enable_alpm(struct ata_port *ap, + enum scsi_host_link_pm policy); +static int ahci_disable_alpm(struct ata_port *ap); enum { AHCI_PCI_BAR= 5, @@ -97,6 +100,7 @@ enum { /* HOST_CAP bits */ HOST_CAP_SSC= (1 14), /* Slumber capable */ HOST_CAP_CLO= (1 24), /* Command List Override support */ + HOST_CAP_ALPM = (1 26), /* Aggressive Link PM support */ HOST_CAP_SSS= (1 27), /* Staggered Spin-up */ HOST_CAP_NCQ= (1 30), /* Native Command Queueing */ HOST_CAP_64 = (1 31), /* PCI DAC (64-bit DMA) support */ @@ -151,6 +155,8 @@ enum { PORT_IRQ_PIOS_FIS | PORT_IRQ_D2H_REG_FIS, /* PORT_CMD bits */ + PORT_CMD_ASP= (1 27), /* Aggressive Slumber/Partial */ + PORT_CMD_ALPE = (1 26), /* Aggressive Link PM enable */ PORT_CMD_ATAPI = (1 24), /* Device is ATAPI */ PORT_CMD_LIST_ON= (1 15), /* cmd list DMA engine running */ PORT_CMD_FIS_ON = (1 14), /* FIS DMA engine running */ @@ -171,6 +177,7 @@ enum { AHCI_FLAG_HONOR_PI = (1 26), /* honor PORTS_IMPL */ AHCI_FLAG_IGN_SERR_INTERNAL = (1 27), /* ignore SERR_INTERNAL */ AHCI_FLAG_32BIT_ONLY= (1 28), /* force 32bit */ + AHCI_FLAG_NO_HOTPLUG= (1 29), /* ignore PxSERR.DIAG.N */ AHCI_FLAG_COMMON= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY | ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA | @@ -253,6 +260,7 @@ static struct scsi_host_template ahci_sh .slave_configure= ata_scsi_slave_config, .slave_destroy = ata_scsi_slave_destroy, .bios_param = ata_std_bios_param, + .set_link_pm_policy = ata_scsi_set_link_pm_policy, }; static const struct ata_port_operations ahci_ops = { @@ -284,6 +292,7 @@ static const struct ata_port_operations .port_suspend = ahci_port_suspend, .port_resume= ahci_port_resume, #endif + .enable_pm = ahci_enable_alpm, .port_start = ahci_port_start, .port_stop = ahci_port_stop, @@ -695,6 +704,152 @@ static void ahci_power_up(struct ata_por writel(cmd | PORT_CMD_ICC_ACTIVE, port_mmio + PORT_CMD); } +static int ahci_disable_alpm(struct ata_port *ap) +{ + void __iomem *port_mmio = ahci_port_base(ap); + u32 cmd, scontrol; + struct ahci_port_priv *pp = ap-private_data; + + /* +* disable Interface Power Management State Transitions +* This is accomplished by setting bits 8:11 of the +* SATA Control register +*/ + scontrol = readl(port_mmio + PORT_SCR_CTL); + scontrol |= (0x3 8); + writel(scontrol, port_mmio + PORT_SCR_CTL); + + /* get the existing command bits */ + cmd = readl(port_mmio + PORT_CMD); + + /* disable ALPM and ASP */ + cmd = ~PORT_CMD_ASP; + cmd = ~PORT_CMD_ALPE; + + /* force the interface back to active */ + cmd |= PORT_CMD_ICC_ACTIVE; + + /* write out new cmd value */ + writel(cmd, port_mmio + PORT_CMD); + cmd = readl(port_mmio + PORT_CMD); + + /* wait 10ms to be sure we've come out of any low power state */ + msleep(10); + + /* clear out any PhyRdy stuff from interrupt status */ + writel(PORT_IRQ_PHYRDY, port_mmio + PORT_IRQ_STAT); + + /* go ahead and clean out PhyRdy Change from Serror too */ + ahci_scr_write(ap, SCR_ERROR, (1 16)); + + /* +* Clear flag
Re: [patch 1/7] libata: check for AN support
On Thu, 24 May 2007 23:15:56 -0400 Jeff Garzik [EMAIL PROTECTED] wrote: Kristen Carlson Accardi wrote: Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] --- Andrew, I cleaned up the function header to properly comply with kernel doc requirements. Other than that, this patch is the same. I would ask for a simple revision: update ata_dev_set_AN() such that it takes a second argument 'enable'. This boolean indicates to the function whether SETFEATURES_SATA_ENABLE or SETFEATURES_SATA_DISABLE should be passed to the device. Otherwise than that, it's ready to merge I would say. Jeff - can you fold this into the original patch, or would you like me to resubmit the whole thing? Kristen Modify ata_dev_set_AN to take a second argument 'enable'. This boolean indicates to the function whether SETFEATURES_SATA_ENABLE or SETFEATURES_SATA_DISABLE should be passed to the device. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-core.c === --- 2.6-git.orig/drivers/ata/libata-core.c +++ 2.6-git/drivers/ata/libata-core.c @@ -70,7 +70,7 @@ const unsigned long sata_deb_timing_long static unsigned int ata_dev_init_params(struct ata_device *dev, u16 heads, u16 sectors); static unsigned int ata_dev_set_xfermode(struct ata_device *dev); -static unsigned int ata_dev_set_AN(struct ata_device *dev); +static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable); static void ata_dev_xfermask(struct ata_device *dev); unsigned int ata_print_id = 1; @@ -2010,7 +2010,7 @@ int ata_dev_configure(struct ata_device if ((ap-flags ATA_FLAG_AN) ata_id_has_AN(id)) { int err; /* issue SET feature command to turn this on */ - err = ata_dev_set_AN(dev); + err = ata_dev_set_AN(dev, SETFEATURES_SATA_ENABLE); if (err) ata_dev_printk(dev, KERN_ERR, unable to set AN, err %x\n, @@ -3966,6 +3966,7 @@ static unsigned int ata_dev_set_xfermode /** * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES * @dev: Device to which command will be sent + * @enable: Whether to enable or disable the feature * * Issue SET FEATURES - SATA FEATURES command to device @dev * on port @ap with sector count set to indicate Asynchronous @@ -3977,7 +3978,7 @@ static unsigned int ata_dev_set_xfermode * RETURNS: * 0 on success, AC_ERR_* mask otherwise. */ -static unsigned int ata_dev_set_AN(struct ata_device *dev) +static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable) { struct ata_taskfile tf; unsigned int err_mask; @@ -3987,7 +3988,7 @@ static unsigned int ata_dev_set_AN(struc ata_tf_init(dev, tf); tf.command = ATA_CMD_SET_FEATURES; - tf.feature = SETFEATURES_SATA_ENABLE; + tf.feature = enable; tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; tf.protocol = ATA_PROT_NODATA; tf.nsect = SATA_AN; - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 5/7] genhd: send async notification on media change
On Tue, 22 May 2007 14:12:11 -0700 Andrew Morton [EMAIL PROTECTED] wrote: On Wed, 9 May 2007 16:38:35 -0700 Kristen Carlson Accardi [EMAIL PROTECTED] wrote: Send an uevent to user space to indicate that a media change event has occurred. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/block/genhd.c === --- 2.6-git.orig/block/genhd.c +++ 2.6-git/block/genhd.c @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = { .show = diskstats_show }; +static void media_change_notify_thread(struct work_struct *work) +{ + struct gendisk *gd = container_of(work, struct gendisk, async_notify); + char event[] = MEDIA_CHANGE=1; + char *envp[] = { event, NULL }; + + /* +* set enviroment vars to indicate which event this is for +* so that user space will know to go check the media status. +*/ + kobject_uevent_env(gd-kobj, KOBJ_CHANGE, envp); + put_device(gd-driverfs_dev); +} + +void genhd_media_change_notify(struct gendisk *disk) +{ + get_device(disk-driverfs_dev); + schedule_work(disk-async_notify); +} +EXPORT_SYMBOL_GPL(genhd_media_change_notify); + struct gendisk *alloc_disk(int minors) { return alloc_disk_node(minors, -1); @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino kobj_set_kset_s(disk,block_subsys); kobject_init(disk-kobj); rand_initialize_disk(disk); + INIT_WORK(disk-async_notify, + media_change_notify_thread); } return disk; Why does this do a schedule_work() rather than calling kobject_uevent_env() directly? Because it is called at Interrupt time. Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 5/7] genhd: send async notification on media change
On Wed, 23 May 2007 12:03:55 -0500 James Bottomley [EMAIL PROTECTED] wrote: On Wed, 2007-05-23 at 09:31 -0700, Kristen Carlson Accardi wrote: On Tue, 22 May 2007 14:12:11 -0700 Andrew Morton [EMAIL PROTECTED] wrote: On Wed, 9 May 2007 16:38:35 -0700 Kristen Carlson Accardi [EMAIL PROTECTED] wrote: Send an uevent to user space to indicate that a media change event has occurred. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/block/genhd.c === --- 2.6-git.orig/block/genhd.c +++ 2.6-git/block/genhd.c @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = { .show = diskstats_show }; +static void media_change_notify_thread(struct work_struct *work) +{ + struct gendisk *gd = container_of(work, struct gendisk, async_notify); + char event[] = MEDIA_CHANGE=1; + char *envp[] = { event, NULL }; + + /* +* set enviroment vars to indicate which event this is for +* so that user space will know to go check the media status. +*/ + kobject_uevent_env(gd-kobj, KOBJ_CHANGE, envp); + put_device(gd-driverfs_dev); +} + +void genhd_media_change_notify(struct gendisk *disk) +{ + get_device(disk-driverfs_dev); + schedule_work(disk-async_notify); +} +EXPORT_SYMBOL_GPL(genhd_media_change_notify); + struct gendisk *alloc_disk(int minors) { return alloc_disk_node(minors, -1); @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino kobj_set_kset_s(disk,block_subsys); kobject_init(disk-kobj); rand_initialize_disk(disk); + INIT_WORK(disk-async_notify, + media_change_notify_thread); } return disk; Why does this do a schedule_work() rather than calling kobject_uevent_env() directly? Because it is called at Interrupt time. It better not be ... there's two GFP_KERNEL allocations just above this line in the file. James Where? We are talking about the call to genhd_media_change_notify(). the calling path is this: ahci_host_intr()-ata_scsi_media_change_notify()-genhd_media_change_notify(). I don't see the allocations - are they hidden somewhere? thanks, Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 5/7] genhd: send async notification on media change
On Wed, 23 May 2007 13:51:51 -0500 James Bottomley [EMAIL PROTECTED] wrote: On Wed, 2007-05-23 at 11:26 -0700, Kristen Carlson Accardi wrote: On Wed, 23 May 2007 12:03:55 -0500 James Bottomley [EMAIL PROTECTED] wrote: On Wed, 2007-05-23 at 09:31 -0700, Kristen Carlson Accardi wrote: On Tue, 22 May 2007 14:12:11 -0700 Andrew Morton [EMAIL PROTECTED] wrote: On Wed, 9 May 2007 16:38:35 -0700 Kristen Carlson Accardi [EMAIL PROTECTED] wrote: Send an uevent to user space to indicate that a media change event has occurred. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/block/genhd.c === --- 2.6-git.orig/block/genhd.c +++ 2.6-git/block/genhd.c @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = { .show = diskstats_show }; +static void media_change_notify_thread(struct work_struct *work) +{ + struct gendisk *gd = container_of(work, struct gendisk, async_notify); + char event[] = MEDIA_CHANGE=1; + char *envp[] = { event, NULL }; + + /* +* set enviroment vars to indicate which event this is for +* so that user space will know to go check the media status. +*/ + kobject_uevent_env(gd-kobj, KOBJ_CHANGE, envp); + put_device(gd-driverfs_dev); +} + +void genhd_media_change_notify(struct gendisk *disk) +{ + get_device(disk-driverfs_dev); + schedule_work(disk-async_notify); +} +EXPORT_SYMBOL_GPL(genhd_media_change_notify); + struct gendisk *alloc_disk(int minors) { return alloc_disk_node(minors, -1); @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino kobj_set_kset_s(disk,block_subsys); kobject_init(disk-kobj); rand_initialize_disk(disk); + INIT_WORK(disk-async_notify, + media_change_notify_thread); } return disk; Why does this do a schedule_work() rather than calling kobject_uevent_env() directly? Because it is called at Interrupt time. It better not be ... there's two GFP_KERNEL allocations just above this line in the file. James Where? We are talking about the call to genhd_media_change_notify(). the calling path is this: ahci_host_intr()-ata_scsi_media_change_notify()-genhd_media_change_notify(). I don't see the allocations - are they hidden somewhere? Sorry, I thought you were saying alloc_disk_node() could be called from interrupt context. gets back on ball If you just want to invoke guaranteed user context from a place in the code which *may* be called from interrupt, then execute_in_process_context() might be a better way of doing it ... at least it avoids setting up a workqueue where none is needed. James That is good to know - although in this particular case we are guaranteed to always be called from interrupt context since our uevent needs to be sent in response to an Interrupt received from the disk, so it wouldn't buy us anything. Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/7] libata: check for AN support
Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] --- Andrew, I cleaned up the function header to properly comply with kernel doc requirements. Other than that, this patch is the same. Index: 2.6-mm/drivers/ata/libata-core.c === --- 2.6-mm.orig/drivers/ata/libata-core.c +++ 2.6-mm/drivers/ata/libata-core.c @@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long static unsigned int ata_dev_init_params(struct ata_device *dev, u16 heads, u16 sectors); static unsigned int ata_dev_set_xfermode(struct ata_device *dev); +static unsigned int ata_dev_set_AN(struct ata_device *dev); static void ata_dev_xfermask(struct ata_device *dev); unsigned int ata_print_id = 1; @@ -1983,6 +1984,22 @@ int ata_dev_configure(struct ata_device } dev-cdb_len = (unsigned int) rc; + /* +* check to see if this ATAPI device supports +* Asynchronous Notification +*/ + if ((ap-flags ATA_FLAG_AN) ata_id_has_AN(id)) { + int err; + /* issue SET feature command to turn this on */ + err = ata_dev_set_AN(dev); + if (err) + ata_dev_printk(dev, KERN_ERR, + unable to set AN, err %x\n, + err); + else + dev-flags |= ATA_DFLAG_AN; + } + if (ata_id_cdb_intr(dev-id)) { dev-flags |= ATA_DFLAG_CDB_INTR; cdb_intr_string = , CDB intr; @@ -3910,6 +3927,41 @@ static unsigned int ata_dev_set_xfermode } /** + * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES + * @dev: Device to which command will be sent + * + * Issue SET FEATURES - SATA FEATURES command to device @dev + * on port @ap with sector count set to indicate Asynchronous + * Notification feature + * + * LOCKING: + * PCI/etc. bus probe sem. + * + * RETURNS: + * 0 on success, AC_ERR_* mask otherwise. + */ +static unsigned int ata_dev_set_AN(struct ata_device *dev) +{ + struct ata_taskfile tf; + unsigned int err_mask; + + /* set up set-features taskfile */ + DPRINTK(set features - SATA features\n); + + ata_tf_init(dev, tf); + tf.command = ATA_CMD_SET_FEATURES; + tf.feature = SETFEATURES_SATA_ENABLE; + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf.protocol = ATA_PROT_NODATA; + tf.nsect = SATA_AN; + + err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + + DPRINTK(EXIT, err_mask=%x\n, err_mask); + return err_mask; +} + +/** * ata_dev_init_params - Issue INIT DEV PARAMS command * @dev: Device to which command will be sent * @heads: Number of heads (taskfile parameter) Index: 2.6-mm/include/linux/ata.h === --- 2.6-mm.orig/include/linux/ata.h +++ 2.6-mm/include/linux/ata.h @@ -204,6 +204,12 @@ enum { SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ + SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */ + SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */ + + /* SETFEATURE Sector counts for SATA features */ + SATA_AN = 0x05, /* Asynchronous Notification */ + /* ATAPI stuff */ ATAPI_PKT_DMA = (1 0), ATAPI_DMADIR= (1 2), /* ATAPI data dir: @@ -309,6 +315,9 @@ struct ata_taskfile { #define ata_id_queue_depth(id) (((id)[75] 0x1f) + 1) #define ata_id_removeable(id) ((id)[0] (1 7)) #define ata_id_has_dword_io(id)((id)[50] (1 0)) +#define ata_id_has_AN(id) \ + ( (((id)[76] != 0x) ((id)[76] != 0x)) \ + ((id)[78] (1 5)) ) #define ata_id_iordy_disable(id) ((id)[49] (1 10)) #define ata_id_has_iordy(id) ((id)[49] (1 9)) #define ata_id_u32(id,n) \ Index: 2.6-mm/include/linux/libata.h === --- 2.6-mm.orig/include/linux/libata.h +++ 2.6-mm/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_AN= (1 5), /* device supports Async notification */ ATA_DFLAG_CFG_MASK = (1 8) - 1, ATA_DFLAG_PIO = (1 8), /* device limited to PIO mode */ @@ -174,6 +175,7 @@ enum { ATA_FLAG_SETXFER_POLLING= (1 14), /* use
Re: [patch 6/7] SCSI: save disk in scsi_device - resend
On Mon, 7 May 2007 08:29:28 -0700 Kristen Carlson Accardi [EMAIL PROTECTED] wrote: On Fri, 04 May 2007 15:30:48 -0500 James Bottomley [EMAIL PROTECTED] wrote: On Fri, 2007-05-04 at 11:17 -0700, Kristen Carlson Accardi wrote: Give anyone who has access to scsi_device access to the genhd struct as well. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/sd.c === --- 2.6-git.orig/drivers/scsi/sd.c +++ 2.6-git/drivers/scsi/sd.c @@ -1711,6 +1711,7 @@ static int sd_probe(struct device *dev) dev_set_drvdata(dev, sdkp); add_disk(gd); + sdp-disk = gd; sdev_printk(KERN_NOTICE, sdp, Attached scsi %sdisk %s\n, sdp-removable ? removable : , gd-disk_name); Index: 2.6-git/drivers/scsi/sr.c === --- 2.6-git.orig/drivers/scsi/sr.c +++ 2.6-git/drivers/scsi/sr.c @@ -604,6 +604,7 @@ static int sr_probe(struct device *dev) if (sdev-media_change_notify) disk-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; add_disk(disk); + sdev-disk = disk; sdev_printk(KERN_DEBUG, sdev, Attached scsi CD-ROM %s\n, cd-cdi.name); Index: 2.6-git/include/scsi/scsi_device.h === --- 2.6-git.orig/include/scsi/scsi_device.h +++ 2.6-git/include/scsi/scsi_device.h @@ -138,7 +138,7 @@ struct scsi_device { struct device sdev_gendev; struct class_device sdev_classdev; - + struct gendisk *disk; struct execute_work ew; /* used to get process context on put */ enum scsi_device_state sdev_state; If you're going to do this, you need to take on board removing the struct gendisk from all the ULD structures (since it's now become generic). James Ok - I can send that as a separate patch series, or I can include it in this one. Which is preferred? I interpret the lack of response to this question to mean that I can just decide for myself - in which case I prefer to send this as a separate patchset after this one is integrated. Thanks! Kristen - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 0/7] Asynchronous Notification for ATAPI devices (resend)
Hi Jeff, Here are the AN patches again, they have not changed with the exception of patch #1, which does set the host flag in board_ahci and board_ahci_pi now (thanks Tejun). This patch series implements Asynchronous Notification (AN) for SATA ATAPI devices as defined in SATA 2.5 and AHCI 1.1 and higher. Drives which support this feature will send a notification when new media is inserted and removed, preventing the need for user space to poll for new media. This support is exposed to user space via a flag that will be set in /sys/block/sr*/capability_flags. If the flag is set, user space can disable polling for the new media, and the genhd driver will send a KOBJ_CHANGE event with the envp set to MEDIA_CHANGE_EVENT=1. Note that this patch only implements support for directly attached drives - AN with drives attached to a port multiplier requires additional changes. Thanks! Kristen -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/7] libata: check for AN support
Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-core.c === --- 2.6-git.orig/drivers/ata/libata-core.c +++ 2.6-git/drivers/ata/libata-core.c @@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long static unsigned int ata_dev_init_params(struct ata_device *dev, u16 heads, u16 sectors); static unsigned int ata_dev_set_xfermode(struct ata_device *dev); +static unsigned int ata_dev_set_AN(struct ata_device *dev); static void ata_dev_xfermask(struct ata_device *dev); unsigned int ata_print_id = 1; @@ -1981,6 +1982,22 @@ int ata_dev_configure(struct ata_device } dev-cdb_len = (unsigned int) rc; + /* +* check to see if this ATAPI device supports +* Asynchronous Notification +*/ + if ((ap-flags ATA_FLAG_AN) ata_id_has_AN(id)) { + int err; + /* issue SET feature command to turn this on */ + err = ata_dev_set_AN(dev); + if (err) + ata_dev_printk(dev, KERN_ERR, + unable to set AN, err %x\n, + err); + else + dev-flags |= ATA_DFLAG_AN; + } + if (ata_id_cdb_intr(dev-id)) { dev-flags |= ATA_DFLAG_CDB_INTR; cdb_intr_string = , CDB intr; @@ -3908,6 +3925,42 @@ static unsigned int ata_dev_set_xfermode } /** + * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES + * with sector count set to indicate + * Asynchronous Notification feature + * @dev: Device to which command will be sent + * + * Issue SET FEATURES - SATA FEATURES command to device @dev + * on port @ap. + * + * LOCKING: + * PCI/etc. bus probe sem. + * + * RETURNS: + * 0 on success, AC_ERR_* mask otherwise. + */ +static unsigned int ata_dev_set_AN(struct ata_device *dev) +{ + struct ata_taskfile tf; + unsigned int err_mask; + + /* set up set-features taskfile */ + DPRINTK(set features - SATA features\n); + + ata_tf_init(dev, tf); + tf.command = ATA_CMD_SET_FEATURES; + tf.feature = SETFEATURES_SATA_ENABLE; + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf.protocol = ATA_PROT_NODATA; + tf.nsect = SATA_AN; + + err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + + DPRINTK(EXIT, err_mask=%x\n, err_mask); + return err_mask; +} + +/** * ata_dev_init_params - Issue INIT DEV PARAMS command * @dev: Device to which command will be sent * @heads: Number of heads (taskfile parameter) Index: 2.6-git/include/linux/ata.h === --- 2.6-git.orig/include/linux/ata.h +++ 2.6-git/include/linux/ata.h @@ -204,6 +204,12 @@ enum { SETFEATURES_SPINUP = 0x07, /* Spin-up drive */ + SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */ + SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */ + + /* SETFEATURE Sector counts for SATA features */ + SATA_AN = 0x05, /* Asynchronous Notification */ + /* ATAPI stuff */ ATAPI_PKT_DMA = (1 0), ATAPI_DMADIR= (1 2), /* ATAPI data dir: @@ -309,6 +315,9 @@ struct ata_taskfile { #define ata_id_queue_depth(id) (((id)[75] 0x1f) + 1) #define ata_id_removeable(id) ((id)[0] (1 7)) #define ata_id_has_dword_io(id)((id)[50] (1 0)) +#define ata_id_has_AN(id) \ + ( (((id)[76] != 0x) ((id)[76] != 0x)) \ + ((id)[78] (1 5)) ) #define ata_id_iordy_disable(id) ((id)[49] (1 10)) #define ata_id_has_iordy(id) ((id)[49] (1 9)) #define ata_id_u32(id,n) \ Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_AN= (1 5), /* device supports Async notification */ ATA_DFLAG_CFG_MASK = (1 8) - 1, ATA_DFLAG_PIO = (1 8), /* device limited to PIO mode */ @@ -174,6 +175,7 @@ enum { ATA_FLAG_SETXFER_POLLING= (1 14), /* use polling for SETXFER */ ATA_FLAG_IGN_SIMPLEX= (1 15), /* ignore SIMPLEX
[patch 2/7] genhd: expose AN to user space
Allow user space to determine if a disk supports Asynchronous Notification of media changes. This is done by adding a new sysfs file capability_flags, which is documented in (insert file name). This sysfs file will export all disk capabilities flags to user space. We also define a new flag to define the media change notification capability. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/block/genhd.c === --- 2.6-git.orig/block/genhd.c +++ 2.6-git/block/genhd.c @@ -370,7 +370,10 @@ static ssize_t disk_size_read(struct gen { return sprintf(page, %llu\n, (unsigned long long)get_capacity(disk)); } - +static ssize_t disk_capability_read(struct gendisk *disk, char *page) +{ + return sprintf(page, %x\n, disk-flags); +} static ssize_t disk_stats_read(struct gendisk * disk, char *page) { preempt_disable(); @@ -413,6 +416,10 @@ static struct disk_attribute disk_attr_s .attr = {.name = size, .mode = S_IRUGO }, .show = disk_size_read }; +static struct disk_attribute disk_attr_capability = { + .attr = {.name = capability, .mode = S_IRUGO }, + .show = disk_capability_read +}; static struct disk_attribute disk_attr_stat = { .attr = {.name = stat, .mode = S_IRUGO }, .show = disk_stats_read @@ -453,6 +460,7 @@ static struct attribute * default_attrs[ disk_attr_removable.attr, disk_attr_size.attr, disk_attr_stat.attr, + disk_attr_capability.attr, #ifdef CONFIG_FAIL_MAKE_REQUEST disk_attr_fail.attr, #endif Index: 2.6-git/include/linux/genhd.h === --- 2.6-git.orig/include/linux/genhd.h +++ 2.6-git/include/linux/genhd.h @@ -94,6 +94,7 @@ struct hd_struct { #define GENHD_FL_REMOVABLE 1 #define GENHD_FL_DRIVERFS 2 +#define GENHD_FL_MEDIA_CHANGE_NOTIFY 4 #define GENHD_FL_CD8 #define GENHD_FL_UP16 #define GENHD_FL_SUPPRESS_PARTITION_INFO 32 Index: 2.6-git/Documentation/block/capability.txt === --- /dev/null +++ 2.6-git/Documentation/block/capability.txt @@ -0,0 +1,15 @@ +Generic Block Device Capability +=== +This file documents the sysfs file block/disk/capability + +capability is a hex word indicating which capabilities a specific disk +supports. For more information on bits not listed here, see +include/linux/genhd.h + +Capability Value +--- +GENHD_FL_MEDIA_CHANGE_NOTIFY 4 + When this bit is set, the disk supports Asynchronous Notification + of media change events. These events will be broadcast to user + space via kernel uevent. + -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 3/7] scsi: expose AN to user space
Get media change notification capability from disk and pass this information to genhd by setting appropriate flag. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/sr.c === --- 2.6-git.orig/drivers/scsi/sr.c +++ 2.6-git/drivers/scsi/sr.c @@ -603,6 +603,8 @@ static int sr_probe(struct device *dev) dev_set_drvdata(dev, cd); disk-flags |= GENHD_FL_REMOVABLE; + if (sdev-media_change_notify) + disk-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; add_disk(disk); sdev_printk(KERN_DEBUG, sdev, Index: 2.6-git/include/scsi/scsi_device.h === --- 2.6-git.orig/include/scsi/scsi_device.h +++ 2.6-git/include/scsi/scsi_device.h @@ -126,7 +126,7 @@ struct scsi_device { unsigned fix_capacity:1;/* READ_CAPACITY is too high by 1 */ unsigned guess_capacity:1; /* READ_CAPACITY might be too high by 1 */ unsigned retry_hwerror:1; /* Retry HARDWARE_ERROR */ - + unsigned media_change_notify:1; /* dev supports async media notify */ unsigned int device_blocked;/* Device returned QUEUE_FULL. */ unsigned int max_device_blocked; /* what device_blocked counts down from */ Index: 2.6-git/drivers/scsi/sd.c === --- 2.6-git.orig/drivers/scsi/sd.c +++ 2.6-git/drivers/scsi/sd.c @@ -1668,6 +1668,9 @@ static int sd_probe(struct device *dev) if (sdp-removable) gd-flags |= GENHD_FL_REMOVABLE; + if (sdp-media_change_notify) + gd-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; + dev_set_drvdata(dev, sdkp); add_disk(gd); -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 5/7] genhd: send async notification on media change
Send an uevent to user space to indicate that a media change event has occurred. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/block/genhd.c === --- 2.6-git.orig/block/genhd.c +++ 2.6-git/block/genhd.c @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = { .show = diskstats_show }; +static void media_change_notify_thread(struct work_struct *work) +{ + struct gendisk *gd = container_of(work, struct gendisk, async_notify); + char event[] = MEDIA_CHANGE=1; + char *envp[] = { event, NULL }; + + /* +* set enviroment vars to indicate which event this is for +* so that user space will know to go check the media status. +*/ + kobject_uevent_env(gd-kobj, KOBJ_CHANGE, envp); + put_device(gd-driverfs_dev); +} + +void genhd_media_change_notify(struct gendisk *disk) +{ + get_device(disk-driverfs_dev); + schedule_work(disk-async_notify); +} +EXPORT_SYMBOL_GPL(genhd_media_change_notify); + struct gendisk *alloc_disk(int minors) { return alloc_disk_node(minors, -1); @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino kobj_set_kset_s(disk,block_subsys); kobject_init(disk-kobj); rand_initialize_disk(disk); + INIT_WORK(disk-async_notify, + media_change_notify_thread); } return disk; } Index: 2.6-git/include/linux/genhd.h === --- 2.6-git.orig/include/linux/genhd.h +++ 2.6-git/include/linux/genhd.h @@ -66,6 +66,7 @@ struct partition { #include linux/smp.h #include linux/string.h #include linux/fs.h +#include linux/workqueue.h struct partition { unsigned char boot_ind; /* 0x80 - active */ @@ -139,6 +140,7 @@ struct gendisk { #else struct disk_stats dkstats; #endif + struct work_struct async_notify; }; /* Structure for sysfs attributes on block devices */ @@ -419,7 +421,7 @@ extern struct gendisk *alloc_disk_node(i extern struct gendisk *alloc_disk(int minors); extern struct kobject *get_disk(struct gendisk *disk); extern void put_disk(struct gendisk *disk); - +extern void genhd_media_change_notify(struct gendisk *disk); extern void blk_register_region(dev_t dev, unsigned long range, struct module *module, struct kobject *(*probe)(dev_t, int *, void *), -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 4/7] libata: expose AN to user space
If Asynchronous Notification of media change events is supported, pass that information up to the SCSI layer. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -899,6 +899,9 @@ static void ata_scsi_dev_config(struct s blk_queue_max_hw_segments(q, q-max_hw_segments - 1); } + if (dev-flags ATA_DFLAG_AN) + sdev-media_change_notify = 1; + if (dev-flags ATA_DFLAG_NCQ) { int depth; -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 6/7] SCSI: save disk in scsi_device
Give anyone who has access to scsi_device access to the genhd struct as well. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/sd.c === --- 2.6-git.orig/drivers/scsi/sd.c +++ 2.6-git/drivers/scsi/sd.c @@ -1673,6 +1673,7 @@ static int sd_probe(struct device *dev) dev_set_drvdata(dev, sdkp); add_disk(gd); + sdp-disk = gd; sd_printk(KERN_NOTICE, sdkp, Attached SCSI %sdisk\n, sdp-removable ? removable : ); Index: 2.6-git/drivers/scsi/sr.c === --- 2.6-git.orig/drivers/scsi/sr.c +++ 2.6-git/drivers/scsi/sr.c @@ -606,6 +606,7 @@ static int sr_probe(struct device *dev) if (sdev-media_change_notify) disk-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; add_disk(disk); + sdev-disk = disk; sdev_printk(KERN_DEBUG, sdev, Attached scsi CD-ROM %s\n, cd-cdi.name); Index: 2.6-git/include/scsi/scsi_device.h === --- 2.6-git.orig/include/scsi/scsi_device.h +++ 2.6-git/include/scsi/scsi_device.h @@ -140,7 +140,7 @@ struct scsi_device { struct device sdev_gendev; struct class_device sdev_classdev; - + struct gendisk *disk; struct execute_work ew; /* used to get process context on put */ enum scsi_device_state sdev_state; -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 7/7] libata: send event when AN received
When we get an SDB FIS with the 'N' bit set, we should send an event to user space to indicate that there has been a media change. This will be done via the block device. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -1218,6 +1218,28 @@ static void ahci_host_intr(struct ata_po return; } + if (status PORT_IRQ_SDB_FIS) { + /* +* if this is an ATAPI device with AN turned on, +* then we should interrogate the device to +* determine the cause of the interrupt +* +* for AN - this we should check the SDB FIS +* and find the I and N bits set +*/ + const u32 *f = pp-rx_fis + RX_FIS_SDB; + + /* check the 'N' bit in word 0 of the FIS */ + if (f[0] (1 15)) { + int port_addr = ((f[0] 0x0f00) 8); + struct ata_device *adev; + if (port_addr ATA_MAX_DEVICES) { + adev = ap-device[port_addr]; + if (adev-flags ATA_DFLAG_AN) + ata_scsi_media_change_notify(adev); + } + } + } if (ap-sactive) qc_active = readl(port_mmio + PORT_SCR_ACT); else Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -721,6 +721,7 @@ extern void ata_host_init(struct ata_hos extern int ata_scsi_detect(struct scsi_host_template *sht); extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg); extern int ata_scsi_queuecmd(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)); +extern void ata_scsi_media_change_notify(struct ata_device *atadev); extern void ata_sas_port_destroy(struct ata_port *); extern struct ata_port *ata_sas_port_alloc(struct ata_host *, struct ata_port_info *, struct Scsi_Host *); Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -3112,6 +3112,22 @@ static void ata_scsi_remove_dev(struct a } /** + * ata_scsi_media_change_notify - send media change event + * @atadev: Pointer to the disk device with media change event + * + * Tell the block layer to send a media change notification + * event. + * + * LOCKING: + * interrupt context, may not sleep. + */ +void ata_scsi_media_change_notify(struct ata_device *atadev) +{ + genhd_media_change_notify(atadev-sdev-disk); +} +EXPORT_SYMBOL_GPL(ata_scsi_media_change_notify); + +/** * ata_scsi_hotplug - SCSI part of hotplug * @work: Pointer to ATA port to perform SCSI hotplug on * -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 6/7] SCSI: save disk in scsi_device - resend
On Fri, 04 May 2007 15:30:48 -0500 James Bottomley [EMAIL PROTECTED] wrote: On Fri, 2007-05-04 at 11:17 -0700, Kristen Carlson Accardi wrote: Give anyone who has access to scsi_device access to the genhd struct as well. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/sd.c === --- 2.6-git.orig/drivers/scsi/sd.c +++ 2.6-git/drivers/scsi/sd.c @@ -1711,6 +1711,7 @@ static int sd_probe(struct device *dev) dev_set_drvdata(dev, sdkp); add_disk(gd); + sdp-disk = gd; sdev_printk(KERN_NOTICE, sdp, Attached scsi %sdisk %s\n, sdp-removable ? removable : , gd-disk_name); Index: 2.6-git/drivers/scsi/sr.c === --- 2.6-git.orig/drivers/scsi/sr.c +++ 2.6-git/drivers/scsi/sr.c @@ -604,6 +604,7 @@ static int sr_probe(struct device *dev) if (sdev-media_change_notify) disk-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; add_disk(disk); + sdev-disk = disk; sdev_printk(KERN_DEBUG, sdev, Attached scsi CD-ROM %s\n, cd-cdi.name); Index: 2.6-git/include/scsi/scsi_device.h === --- 2.6-git.orig/include/scsi/scsi_device.h +++ 2.6-git/include/scsi/scsi_device.h @@ -138,7 +138,7 @@ struct scsi_device { struct device sdev_gendev; struct class_device sdev_classdev; - + struct gendisk *disk; struct execute_work ew; /* used to get process context on put */ enum scsi_device_state sdev_state; If you're going to do this, you need to take on board removing the struct gendisk from all the ULD structures (since it's now become generic). James Ok - I can send that as a separate patch series, or I can include it in this one. Which is preferred? - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 0/7] Asynchronous Notification for ATAPI devices (v2) - resend
This patch series implements Asynchronous Notification (AN) for SATA ATAPI devices as defined in SATA 2.5 and AHCI 1.1 and higher. Drives which support this feature will send a notification when new media is inserted and removed, preventing the need for user space to poll for new media. This support is exposed to user space via a flag that will be set in /sys/block/sr*/capability_flags. If the flag is set, user space can disable polling for the new media, and the genhd driver will send a KOBJ_CHANGE event with the envp set to MEDIA_CHANGE_EVENT=1. Note that this patch only implements support for directly attached drives - AN with drives attached to a port multiplier requires additional changes. Thanks! Kristen -- -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/7] libata: check for AN support - resend
Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. Changes from last version: * use parens around id in ata.h Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-core.c === --- 2.6-git.orig/drivers/ata/libata-core.c +++ 2.6-git/drivers/ata/libata-core.c @@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long static unsigned int ata_dev_init_params(struct ata_device *dev, u16 heads, u16 sectors); static unsigned int ata_dev_set_xfermode(struct ata_device *dev); +static unsigned int ata_dev_set_AN(struct ata_device *dev); static void ata_dev_xfermask(struct ata_device *dev); static unsigned int ata_print_id = 1; @@ -1744,6 +1745,22 @@ int ata_dev_configure(struct ata_device } dev-cdb_len = (unsigned int) rc; + /* +* check to see if this ATAPI device supports +* Asynchronous Notification +*/ + if ((ap-flags ATA_FLAG_AN) ata_id_has_AN(id)) { + int err; + /* issue SET feature command to turn this on */ + err = ata_dev_set_AN(dev); + if (err) + ata_dev_printk(dev, KERN_ERR, + unable to set AN, err %x\n, + err); + else + dev-flags |= ATA_DFLAG_AN; + } + if (ata_id_cdb_intr(dev-id)) { dev-flags |= ATA_DFLAG_CDB_INTR; cdb_intr_string = , CDB intr; @@ -3525,6 +3542,42 @@ static unsigned int ata_dev_set_xfermode } /** + * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES + * with sector count set to indicate + * Asynchronous Notification feature + * @dev: Device to which command will be sent + * + * Issue SET FEATURES - SATA FEATURES command to device @dev + * on port @ap. + * + * LOCKING: + * PCI/etc. bus probe sem. + * + * RETURNS: + * 0 on success, AC_ERR_* mask otherwise. + */ +static unsigned int ata_dev_set_AN(struct ata_device *dev) +{ + struct ata_taskfile tf; + unsigned int err_mask; + + /* set up set-features taskfile */ + DPRINTK(set features - SATA features\n); + + ata_tf_init(dev, tf); + tf.command = ATA_CMD_SET_FEATURES; + tf.feature = SETFEATURES_SATA_ENABLE; + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf.protocol = ATA_PROT_NODATA; + tf.nsect = SATA_AN; + + err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + + DPRINTK(EXIT, err_mask=%x\n, err_mask); + return err_mask; +} + +/** * ata_dev_init_params - Issue INIT DEV PARAMS command * @dev: Device to which command will be sent * @heads: Number of heads (taskfile parameter) Index: 2.6-git/include/linux/ata.h === --- 2.6-git.orig/include/linux/ata.h +++ 2.6-git/include/linux/ata.h @@ -194,6 +194,12 @@ enum { SETFEATURES_WC_ON = 0x02, /* Enable write cache */ SETFEATURES_WC_OFF = 0x82, /* Disable write cache */ + SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */ + SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */ + + /* SETFEATURE Sector counts for SATA features */ + SATA_AN = 0x05, /* Asynchronous Notification */ + /* ATAPI stuff */ ATAPI_PKT_DMA = (1 0), ATAPI_DMADIR= (1 2), /* ATAPI data dir: @@ -299,6 +305,9 @@ struct ata_taskfile { #define ata_id_queue_depth(id) (((id)[75] 0x1f) + 1) #define ata_id_removeable(id) ((id)[0] (1 7)) #define ata_id_has_dword_io(id)((id)[50] (1 0)) +#define ata_id_has_AN(id) \ + ( (((id)[76] != 0x) ((id)[76] != 0x)) \ + ((id)[78] (1 5)) ) #define ata_id_iordy_disable(id) ((id)[49] (1 10)) #define ata_id_has_iordy(id) ((id)[49] (1 9)) #define ata_id_u32(id,n) \ Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_AN= (1 5), /* device supports Async notification */ ATA_DFLAG_CFG_MASK = (1 8) - 1, ATA_DFLAG_PIO = (1 8), /* device limited to PIO mode */ @@ -174,6 +175,7 @@ enum
[patch 2/7] genhd: expose AN to user space - resend
Allow user space to determine if a disk supports Asynchronous Notification of media changes. This is done by adding a new sysfs file capability_flags, which is documented in (insert file name). This sysfs file will export all disk capabilities flags to user space. We also define a new flag to define the media change notification capability. Changed from last version: * changed sysfs filename to capability from capability_flags Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/block/genhd.c === --- 2.6-git.orig/block/genhd.c +++ 2.6-git/block/genhd.c @@ -370,7 +370,10 @@ static ssize_t disk_size_read(struct gen { return sprintf(page, %llu\n, (unsigned long long)get_capacity(disk)); } - +static ssize_t disk_capability_read(struct gendisk *disk, char *page) +{ + return sprintf(page, %x\n, disk-flags); +} static ssize_t disk_stats_read(struct gendisk * disk, char *page) { preempt_disable(); @@ -413,6 +416,10 @@ static struct disk_attribute disk_attr_s .attr = {.name = size, .mode = S_IRUGO }, .show = disk_size_read }; +static struct disk_attribute disk_attr_capability = { + .attr = {.name = capability, .mode = S_IRUGO }, + .show = disk_capability_read +}; static struct disk_attribute disk_attr_stat = { .attr = {.name = stat, .mode = S_IRUGO }, .show = disk_stats_read @@ -453,6 +460,7 @@ static struct attribute * default_attrs[ disk_attr_removable.attr, disk_attr_size.attr, disk_attr_stat.attr, + disk_attr_capability.attr, #ifdef CONFIG_FAIL_MAKE_REQUEST disk_attr_fail.attr, #endif Index: 2.6-git/include/linux/genhd.h === --- 2.6-git.orig/include/linux/genhd.h +++ 2.6-git/include/linux/genhd.h @@ -94,6 +94,7 @@ struct hd_struct { #define GENHD_FL_REMOVABLE 1 #define GENHD_FL_DRIVERFS 2 +#define GENHD_FL_MEDIA_CHANGE_NOTIFY 4 #define GENHD_FL_CD8 #define GENHD_FL_UP16 #define GENHD_FL_SUPPRESS_PARTITION_INFO 32 Index: 2.6-git/Documentation/block/capability.txt === --- /dev/null +++ 2.6-git/Documentation/block/capability.txt @@ -0,0 +1,15 @@ +Generic Block Device Capability +=== +This file documents the sysfs file block/disk/capability + +capability is a hex word indicating which capabilities a specific disk +supports. For more information on bits not listed here, see +include/linux/genhd.h + +Capability Value +--- +GENHD_FL_MEDIA_CHANGE_NOTIFY 4 + When this bit is set, the disk supports Asynchronous Notification + of media change events. These events will be broadcast to user + space via kernel uevent. + - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 3/7] scsi: expose AN to user space - resend
Get media change notification capability from disk and pass this information to genhd by setting appropriate flag. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/sr.c === --- 2.6-git.orig/drivers/scsi/sr.c +++ 2.6-git/drivers/scsi/sr.c @@ -601,6 +601,8 @@ static int sr_probe(struct device *dev) dev_set_drvdata(dev, cd); disk-flags |= GENHD_FL_REMOVABLE; + if (sdev-media_change_notify) + disk-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; add_disk(disk); sdev_printk(KERN_DEBUG, sdev, Index: 2.6-git/include/scsi/scsi_device.h === --- 2.6-git.orig/include/scsi/scsi_device.h +++ 2.6-git/include/scsi/scsi_device.h @@ -124,7 +124,7 @@ struct scsi_device { unsigned fix_capacity:1;/* READ_CAPACITY is too high by 1 */ unsigned guess_capacity:1; /* READ_CAPACITY might be too high by 1 */ unsigned retry_hwerror:1; /* Retry HARDWARE_ERROR */ - + unsigned media_change_notify:1; /* dev supports async media notify */ unsigned int device_blocked;/* Device returned QUEUE_FULL. */ unsigned int max_device_blocked; /* what device_blocked counts down from */ Index: 2.6-git/drivers/scsi/sd.c === --- 2.6-git.orig/drivers/scsi/sd.c +++ 2.6-git/drivers/scsi/sd.c @@ -1706,6 +1706,9 @@ static int sd_probe(struct device *dev) if (sdp-removable) gd-flags |= GENHD_FL_REMOVABLE; + if (sdp-media_change_notify) + gd-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; + dev_set_drvdata(dev, sdkp); add_disk(gd); -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 5/7] genhd: send async notification on media change
Send an uevent to user space to indicate that a media change event has occurred. Changes from last version: * use get/put_device to increment reference count on the device struct Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/block/genhd.c === --- 2.6-git.orig/block/genhd.c +++ 2.6-git/block/genhd.c @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = { .show = diskstats_show }; +static void media_change_notify_thread(struct work_struct *work) +{ + struct gendisk *gd = container_of(work, struct gendisk, async_notify); + char event[] = MEDIA_CHANGE=1; + char *envp[] = { event, NULL }; + + /* +* set enviroment vars to indicate which event this is for +* so that user space will know to go check the media status. +*/ + kobject_uevent_env(gd-kobj, KOBJ_CHANGE, envp); + put_device(gd-driverfs_dev); +} + +void genhd_media_change_notify(struct gendisk *disk) +{ + get_device(disk-driverfs_dev); + schedule_work(disk-async_notify); +} +EXPORT_SYMBOL_GPL(genhd_media_change_notify); + struct gendisk *alloc_disk(int minors) { return alloc_disk_node(minors, -1); @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino kobj_set_kset_s(disk,block_subsys); kobject_init(disk-kobj); rand_initialize_disk(disk); + INIT_WORK(disk-async_notify, + media_change_notify_thread); } return disk; } Index: 2.6-git/include/linux/genhd.h === --- 2.6-git.orig/include/linux/genhd.h +++ 2.6-git/include/linux/genhd.h @@ -66,6 +66,7 @@ struct partition { #include linux/smp.h #include linux/string.h #include linux/fs.h +#include linux/workqueue.h struct partition { unsigned char boot_ind; /* 0x80 - active */ @@ -139,6 +140,7 @@ struct gendisk { #else struct disk_stats dkstats; #endif + struct work_struct async_notify; }; /* Structure for sysfs attributes on block devices */ @@ -419,7 +421,7 @@ extern struct gendisk *alloc_disk_node(i extern struct gendisk *alloc_disk(int minors); extern struct kobject *get_disk(struct gendisk *disk); extern void put_disk(struct gendisk *disk); - +extern void genhd_media_change_notify(struct gendisk *disk); extern void blk_register_region(dev_t dev, unsigned long range, struct module *module, struct kobject *(*probe)(dev_t, int *, void *), - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 7/7] libata: send event when AN received - resend
When we get an SDB FIS with the 'N' bit set, we should send an event to user space to indicate that there has been a media change. This will be done via the block device. changed from last version: * Make sure that port_addr is within ATA_MAX_DEVICES Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -1147,6 +1147,28 @@ static void ahci_host_intr(struct ata_po return; } + if (status PORT_IRQ_SDB_FIS) { + /* +* if this is an ATAPI device with AN turned on, +* then we should interrogate the device to +* determine the cause of the interrupt +* +* for AN - this we should check the SDB FIS +* and find the I and N bits set +*/ + const u32 *f = pp-rx_fis + RX_FIS_SDB; + + /* check the 'N' bit in word 0 of the FIS */ + if (f[0] (1 15)) { + int port_addr = ((f[0] 0x0f00) 8); + struct ata_device *adev; + if (port_addr ATA_MAX_DEVICES) { + adev = ap-device[port_addr]; + if (adev-flags ATA_DFLAG_AN) + ata_scsi_media_change_notify(adev); + } + } + } if (ap-sactive) qc_active = readl(port_mmio + PORT_SCR_ACT); else Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -737,6 +737,7 @@ extern void ata_host_init(struct ata_hos extern int ata_scsi_detect(struct scsi_host_template *sht); extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg); extern int ata_scsi_queuecmd(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)); +extern void ata_scsi_media_change_notify(struct ata_device *atadev); extern void ata_sas_port_destroy(struct ata_port *); extern struct ata_port *ata_sas_port_alloc(struct ata_host *, struct ata_port_info *, struct Scsi_Host *); Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -3057,6 +3057,22 @@ static void ata_scsi_remove_dev(struct a } /** + * ata_scsi_media_change_notify - send media change event + * @atadev: Pointer to the disk device with media change event + * + * Tell the block layer to send a media change notification + * event. + * + * LOCKING: + * interrupt context, may not sleep. + */ +void ata_scsi_media_change_notify(struct ata_device *atadev) +{ + genhd_media_change_notify(atadev-sdev-disk); +} +EXPORT_SYMBOL_GPL(ata_scsi_media_change_notify); + +/** * ata_scsi_hotplug - SCSI part of hotplug * @work: Pointer to ATA port to perform SCSI hotplug on * - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 0/7] Asynchronous Notification for ATAPI devices (v2) - resend
On Fri, 04 May 2007 15:30:03 -0500 James Bottomley [EMAIL PROTECTED] wrote: On Fri, 2007-05-04 at 11:12 -0700, Kristen Carlson Accardi wrote: This patch series implements Asynchronous Notification (AN) for SATA ATAPI devices as defined in SATA 2.5 and AHCI 1.1 and higher. Drives which support this feature will send a notification when new media is inserted and removed, preventing the need for user space to poll for new media. This support is exposed to user space via a flag that will be set in /sys/block/sr*/capability_flags. If the flag is set, user space can disable polling for the new media, and the genhd driver will send a KOBJ_CHANGE event with the envp set to MEDIA_CHANGE_EVENT=1. Note that this patch only implements support for directly attached drives - AN with drives attached to a port multiplier requires additional changes. I seem to remember when this came up before, the observation was made that AENs occur for many more things than simple media change notices, so we should probably have a more generic AEN handling mechanism. Is there any plan to move in this direction James In this implementation, we use kobject_uevent_env (vs. just kobject_uevent in the original implementation) to send a KOBJ_CHANGE with the enviroment value of MEDIA_CHANGE=1. For other asynchronous events, you could still use KOBJ_CHANGE with a different env value. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/7] libata: check for AN support
On Wed, 25 Apr 2007 02:49:46 +0200 Olivier Galibert [EMAIL PROTECTED] wrote: On Tue, Apr 24, 2007 at 01:53:27PM -0700, Kristen Carlson Accardi wrote: Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. changes from last version: * fix typo in ata_id_has_AN and make word 76 test more clear * If we fail to set the AN feature, just print a warning and continue Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] @@ -299,6 +305,8 @@ struct ata_taskfile { #define ata_id_queue_depth(id) (((id)[75] 0x1f) + 1) #define ata_id_removeable(id) ((id)[0] (1 7)) #define ata_id_has_dword_io(id)((id)[50] (1 0)) +#define ata_id_has_AN(id) \ + (((id[76] != 0x) (id[76] != 0x)) ((id)[78] (1 5))) (id)[76] I guess ? Sorry for being a pain :/ OG. Ok - I'll fix that. Thank you for being a pain :), I really appreciate the time you are taking to review my patches. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/7] libata: check for AN support
Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. Changes from last version: * use parens around id in ata.h Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-core.c === --- 2.6-git.orig/drivers/ata/libata-core.c +++ 2.6-git/drivers/ata/libata-core.c @@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long static unsigned int ata_dev_init_params(struct ata_device *dev, u16 heads, u16 sectors); static unsigned int ata_dev_set_xfermode(struct ata_device *dev); +static unsigned int ata_dev_set_AN(struct ata_device *dev); static void ata_dev_xfermask(struct ata_device *dev); static unsigned int ata_print_id = 1; @@ -1744,6 +1745,22 @@ int ata_dev_configure(struct ata_device } dev-cdb_len = (unsigned int) rc; + /* +* check to see if this ATAPI device supports +* Asynchronous Notification +*/ + if ((ap-flags ATA_FLAG_AN) ata_id_has_AN(id)) { + int err; + /* issue SET feature command to turn this on */ + err = ata_dev_set_AN(dev); + if (err) + ata_dev_printk(dev, KERN_ERR, + unable to set AN, err %x\n, + err); + else + dev-flags |= ATA_DFLAG_AN; + } + if (ata_id_cdb_intr(dev-id)) { dev-flags |= ATA_DFLAG_CDB_INTR; cdb_intr_string = , CDB intr; @@ -3525,6 +3542,42 @@ static unsigned int ata_dev_set_xfermode } /** + * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES + * with sector count set to indicate + * Asynchronous Notification feature + * @dev: Device to which command will be sent + * + * Issue SET FEATURES - SATA FEATURES command to device @dev + * on port @ap. + * + * LOCKING: + * PCI/etc. bus probe sem. + * + * RETURNS: + * 0 on success, AC_ERR_* mask otherwise. + */ +static unsigned int ata_dev_set_AN(struct ata_device *dev) +{ + struct ata_taskfile tf; + unsigned int err_mask; + + /* set up set-features taskfile */ + DPRINTK(set features - SATA features\n); + + ata_tf_init(dev, tf); + tf.command = ATA_CMD_SET_FEATURES; + tf.feature = SETFEATURES_SATA_ENABLE; + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf.protocol = ATA_PROT_NODATA; + tf.nsect = SATA_AN; + + err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + + DPRINTK(EXIT, err_mask=%x\n, err_mask); + return err_mask; +} + +/** * ata_dev_init_params - Issue INIT DEV PARAMS command * @dev: Device to which command will be sent * @heads: Number of heads (taskfile parameter) Index: 2.6-git/include/linux/ata.h === --- 2.6-git.orig/include/linux/ata.h +++ 2.6-git/include/linux/ata.h @@ -194,6 +194,12 @@ enum { SETFEATURES_WC_ON = 0x02, /* Enable write cache */ SETFEATURES_WC_OFF = 0x82, /* Disable write cache */ + SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */ + SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */ + + /* SETFEATURE Sector counts for SATA features */ + SATA_AN = 0x05, /* Asynchronous Notification */ + /* ATAPI stuff */ ATAPI_PKT_DMA = (1 0), ATAPI_DMADIR= (1 2), /* ATAPI data dir: @@ -299,6 +305,9 @@ struct ata_taskfile { #define ata_id_queue_depth(id) (((id)[75] 0x1f) + 1) #define ata_id_removeable(id) ((id)[0] (1 7)) #define ata_id_has_dword_io(id)((id)[50] (1 0)) +#define ata_id_has_AN(id) \ + ( (((id)[76] != 0x) ((id)[76] != 0x)) \ + ((id)[78] (1 5)) ) #define ata_id_iordy_disable(id) ((id)[49] (1 10)) #define ata_id_has_iordy(id) ((id)[49] (1 9)) #define ata_id_u32(id,n) \ Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_AN= (1 5), /* device supports Async notification */ ATA_DFLAG_CFG_MASK = (1 8) - 1, ATA_DFLAG_PIO = (1 8), /* device limited to PIO mode */ @@ -174,6 +175,7 @@ enum
Re: [patch 1/7] libata: check for AN support
On Wed, 25 Apr 2007 20:16:51 +0100 Matt Sealey [EMAIL PROTECTED] wrote: +#define ata_id_has_AN(id) \ + ( (((id)[76] != 0x) ((id)[76] != 0x)) \ + ((id)[78] (1 5)) ) ?? --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_AN= (1 5), /* device supports Async notification */ ATA_DFLAG_CFG_MASK = (1 8) - 1, Why don't the macros use the enums? It makes the code hard to read without painful cross-reference doesn't it? Surely (id)[76] (ATA_DFLAG_AN) is a lot more readable than 1 5 - even if the flag is obviously that, a lot of values and registers can have 1 5 as a flag and mean a lot of different things. It's really just a coincidence that the ATA_DFLAG_AN bit is the same as the bit in the identify device word, so this would not be appropriate. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/7] libata: check for AN support
On Tue, 24 Apr 2007 12:23:04 +0200 Olivier Galibert [EMAIL PROTECTED] wrote: Sorry for replying to Alan's reply, I missed the original mail. +#define ata_id_has_AN(id)\ + ((id[76] (~id[76])) ((id)[78] (1 5))) (a ~a) (b 32) I don't think that does what you think it does, because at that point it's a funny way to write 0 ((0 or 1) binary-and (0 or 32)). I'm not even sure what it is you want. If for the first part you wanted (id[76] != 0x00 id[76] != 0xff), please write just that, thanks :-) OG. From the serial ata spec, we have: 13.2.1.18Word 78: Serial ATA features supported If Word 76 is not h or h, Word 78 reports the optional features supported by the device. Support for this word is optional and if not supported the word shall be zero indicating the device has no support for new Serial ATA capabilities. so, basically yes, I'm really testing to make sure that word 76 isn't 0 or all one then using that value with value of bit in work 78 to determine AN support - if you think this is really obfuscated, I've got no problem changing it - there's obviously many ways to mess around with bits. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/7] libata: check for AN support
On Tue, 24 Apr 2007 17:03:29 +0900 Tejun Heo [EMAIL PROTECTED] wrote: Hello, Kristen Carlson Accardi wrote: static unsigned int ata_print_id = 1; @@ -1744,6 +1745,23 @@ int ata_dev_configure(struct ata_device } dev-cdb_len = (unsigned int) rc; + /* +* check to see if this ATAPI device supports +* Asynchronous Notification +*/ + if ((ap-flags ATA_FLAG_AN) ata_id_has_AN(id)) + { + /* issue SET feature command to turn this on */ + rc = ata_dev_set_AN(dev); Please don't store err_mask into int rc. Please store it to a separate err_mask variable and report it when printing error message. + if (rc) { + ata_dev_printk(dev, KERN_ERR, + unable to set AN\n); + rc = -EINVAL; Wouldn't -EIO be more appropriate? I think Alan is right - and being unable to turn on AN should not be fatal. I'll just change all this code to just print the err and keep going. + goto err_out_nosup; + } + dev-flags |= ATA_DFLAG_AN; + } + Not NACKing. Just notes for future improvements. We need to be more careful here. ATA/ATAPI world is filled with braindamaged devices and I bet there are devices which advertises it can do AN but chokes when AN is enabled. This should be handled similarly to ACPI failure. Currently ACPI does the following. 1. try once, if fail, record that ACPI failed. return error to trigger retry. 2. try again, if fail again, ignore error if possible (!FROZEN) and turn off ACPI. This fallback mechanism for optional features can probably be generalized and used for both ACPI and AN. Ok - meanwhile I think it's appropriate here to just do try-once-fail-give-up. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/7] libata: check for AN support
On Tue, 24 Apr 2007 20:05:52 +0200 Olivier Galibert [EMAIL PROTECTED] wrote: On Tue, Apr 24, 2007 at 08:49:04AM -0700, Kristen Carlson Accardi wrote: On Tue, 24 Apr 2007 12:23:04 +0200 Olivier Galibert [EMAIL PROTECTED] wrote: Sorry for replying to Alan's reply, I missed the original mail. +#define ata_id_has_AN(id)\ + ((id[76] (~id[76])) ((id)[78] (1 5))) (a ~a) (b 32) I don't think that does what you think it does, because at that point it's a funny way to write 0 ((0 or 1) binary-and (0 or 32)). I'm not even sure what it is you want. If for the first part you wanted (id[76] != 0x00 id[76] != 0xff), please write just that, thanks :-) OG. From the serial ata spec, we have: 13.2.1.18Word 78: Serial ATA features supported If Word 76 is not h or h, Word 78 reports the optional features supported by the device. Support for this word is optional and if not supported the word shall be zero indicating the device has no support for new Serial ATA capabilities. so, basically yes, I'm really testing to make sure that word 76 isn't 0 or all one then using that value with value of bit in work 78 to determine AN support - if you think this is really obfuscated, I've got no problem changing it - there's obviously many ways to mess around with bits. is not , so right now it's really incorrect. 1 32 is 0. ah - ok, gotcha, thanks. ((id)[76] != 0x (id)[76] != 0x ((id)[78] (1 5))) The implicit typing of id looks dangerous to me, but you're not the one who has started it. OG. - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/7] libata: check for AN support
Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. changes from last version: * fix typo in ata_id_has_AN and make word 76 test more clear * If we fail to set the AN feature, just print a warning and continue Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-core.c === --- 2.6-git.orig/drivers/ata/libata-core.c +++ 2.6-git/drivers/ata/libata-core.c @@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long static unsigned int ata_dev_init_params(struct ata_device *dev, u16 heads, u16 sectors); static unsigned int ata_dev_set_xfermode(struct ata_device *dev); +static unsigned int ata_dev_set_AN(struct ata_device *dev); static void ata_dev_xfermask(struct ata_device *dev); static unsigned int ata_print_id = 1; @@ -1744,6 +1745,22 @@ int ata_dev_configure(struct ata_device } dev-cdb_len = (unsigned int) rc; + /* +* check to see if this ATAPI device supports +* Asynchronous Notification +*/ + if ((ap-flags ATA_FLAG_AN) ata_id_has_AN(id)) { + int err; + /* issue SET feature command to turn this on */ + err = ata_dev_set_AN(dev); + if (err) + ata_dev_printk(dev, KERN_ERR, + unable to set AN, err %x\n, + err); + else + dev-flags |= ATA_DFLAG_AN; + } + if (ata_id_cdb_intr(dev-id)) { dev-flags |= ATA_DFLAG_CDB_INTR; cdb_intr_string = , CDB intr; @@ -3525,6 +3542,42 @@ static unsigned int ata_dev_set_xfermode } /** + * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES + * with sector count set to indicate + * Asynchronous Notification feature + * @dev: Device to which command will be sent + * + * Issue SET FEATURES - SATA FEATURES command to device @dev + * on port @ap. + * + * LOCKING: + * PCI/etc. bus probe sem. + * + * RETURNS: + * 0 on success, AC_ERR_* mask otherwise. + */ +static unsigned int ata_dev_set_AN(struct ata_device *dev) +{ + struct ata_taskfile tf; + unsigned int err_mask; + + /* set up set-features taskfile */ + DPRINTK(set features - SATA features\n); + + ata_tf_init(dev, tf); + tf.command = ATA_CMD_SET_FEATURES; + tf.feature = SETFEATURES_SATA_ENABLE; + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf.protocol = ATA_PROT_NODATA; + tf.nsect = SATA_AN; + + err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + + DPRINTK(EXIT, err_mask=%x\n, err_mask); + return err_mask; +} + +/** * ata_dev_init_params - Issue INIT DEV PARAMS command * @dev: Device to which command will be sent * @heads: Number of heads (taskfile parameter) Index: 2.6-git/include/linux/ata.h === --- 2.6-git.orig/include/linux/ata.h +++ 2.6-git/include/linux/ata.h @@ -194,6 +194,12 @@ enum { SETFEATURES_WC_ON = 0x02, /* Enable write cache */ SETFEATURES_WC_OFF = 0x82, /* Disable write cache */ + SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */ + SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */ + + /* SETFEATURE Sector counts for SATA features */ + SATA_AN = 0x05, /* Asynchronous Notification */ + /* ATAPI stuff */ ATAPI_PKT_DMA = (1 0), ATAPI_DMADIR= (1 2), /* ATAPI data dir: @@ -299,6 +305,8 @@ struct ata_taskfile { #define ata_id_queue_depth(id) (((id)[75] 0x1f) + 1) #define ata_id_removeable(id) ((id)[0] (1 7)) #define ata_id_has_dword_io(id)((id)[50] (1 0)) +#define ata_id_has_AN(id) \ + (((id[76] != 0x) (id[76] != 0x)) ((id)[78] (1 5))) #define ata_id_iordy_disable(id) ((id)[49] (1 10)) #define ata_id_has_iordy(id) ((id)[49] (1 9)) #define ata_id_u32(id,n) \ Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_AN= (1 5), /* device supports Async notification */ ATA_DFLAG_CFG_MASK = (1 8) - 1
Re: [patch 2/7] genhd: expose AN to user space
Allow user space to determine if a disk supports Asynchronous Notification of media changes. This is done by adding a new sysfs file capability_flags, which is documented in (insert file name). This sysfs file will export all disk capabilities flags to user space. We also define a new flag to define the media change notification capability. Changed from last version: * changed sysfs filename to capability from capability_flags Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/block/genhd.c === --- 2.6-git.orig/block/genhd.c +++ 2.6-git/block/genhd.c @@ -370,7 +370,10 @@ static ssize_t disk_size_read(struct gen { return sprintf(page, %llu\n, (unsigned long long)get_capacity(disk)); } - +static ssize_t disk_capability_read(struct gendisk *disk, char *page) +{ + return sprintf(page, %x\n, disk-flags); +} static ssize_t disk_stats_read(struct gendisk * disk, char *page) { preempt_disable(); @@ -413,6 +416,10 @@ static struct disk_attribute disk_attr_s .attr = {.name = size, .mode = S_IRUGO }, .show = disk_size_read }; +static struct disk_attribute disk_attr_capability = { + .attr = {.name = capability, .mode = S_IRUGO }, + .show = disk_capability_read +}; static struct disk_attribute disk_attr_stat = { .attr = {.name = stat, .mode = S_IRUGO }, .show = disk_stats_read @@ -453,6 +460,7 @@ static struct attribute * default_attrs[ disk_attr_removable.attr, disk_attr_size.attr, disk_attr_stat.attr, + disk_attr_capability.attr, #ifdef CONFIG_FAIL_MAKE_REQUEST disk_attr_fail.attr, #endif Index: 2.6-git/include/linux/genhd.h === --- 2.6-git.orig/include/linux/genhd.h +++ 2.6-git/include/linux/genhd.h @@ -94,6 +94,7 @@ struct hd_struct { #define GENHD_FL_REMOVABLE 1 #define GENHD_FL_DRIVERFS 2 +#define GENHD_FL_MEDIA_CHANGE_NOTIFY 4 #define GENHD_FL_CD8 #define GENHD_FL_UP16 #define GENHD_FL_SUPPRESS_PARTITION_INFO 32 Index: 2.6-git/Documentation/block/capability.txt === --- /dev/null +++ 2.6-git/Documentation/block/capability.txt @@ -0,0 +1,15 @@ +Generic Block Device Capability +=== +This file documents the sysfs file block/disk/capability + +capability is a hex word indicating which capabilities a specific disk +supports. For more information on bits not listed here, see +include/linux/genhd.h + +Capability Value +--- +GENHD_FL_MEDIA_CHANGE_NOTIFY 4 + When this bit is set, the disk supports Asynchronous Notification + of media change events. These events will be broadcast to user + space via kernel uevent. + - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 5/7] genhd: send async notification on media change
Send an uevent to user space to indicate that a media change event has occurred. Changes from last version: * use get/put_device to increment reference count on the device struct Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/block/genhd.c === --- 2.6-git.orig/block/genhd.c +++ 2.6-git/block/genhd.c @@ -643,6 +643,27 @@ struct seq_operations diskstats_op = { .show = diskstats_show }; +static void media_change_notify_thread(struct work_struct *work) +{ + struct gendisk *gd = container_of(work, struct gendisk, async_notify); + char event[] = MEDIA_CHANGE=1; + char *envp[] = { event, NULL }; + + /* +* set enviroment vars to indicate which event this is for +* so that user space will know to go check the media status. +*/ + kobject_uevent_env(gd-kobj, KOBJ_CHANGE, envp); + put_device(gd-driverfs_dev); +} + +void genhd_media_change_notify(struct gendisk *disk) +{ + get_device(disk-driverfs_dev); + schedule_work(disk-async_notify); +} +EXPORT_SYMBOL_GPL(genhd_media_change_notify); + struct gendisk *alloc_disk(int minors) { return alloc_disk_node(minors, -1); @@ -672,6 +693,8 @@ struct gendisk *alloc_disk_node(int mino kobj_set_kset_s(disk,block_subsys); kobject_init(disk-kobj); rand_initialize_disk(disk); + INIT_WORK(disk-async_notify, + media_change_notify_thread); } return disk; } Index: 2.6-git/include/linux/genhd.h === --- 2.6-git.orig/include/linux/genhd.h +++ 2.6-git/include/linux/genhd.h @@ -66,6 +66,7 @@ struct partition { #include linux/smp.h #include linux/string.h #include linux/fs.h +#include linux/workqueue.h struct partition { unsigned char boot_ind; /* 0x80 - active */ @@ -139,6 +140,7 @@ struct gendisk { #else struct disk_stats dkstats; #endif + struct work_struct async_notify; }; /* Structure for sysfs attributes on block devices */ @@ -419,7 +421,7 @@ extern struct gendisk *alloc_disk_node(i extern struct gendisk *alloc_disk(int minors); extern struct kobject *get_disk(struct gendisk *disk); extern void put_disk(struct gendisk *disk); - +extern void genhd_media_change_notify(struct gendisk *disk); extern void blk_register_region(dev_t dev, unsigned long range, struct module *module, struct kobject *(*probe)(dev_t, int *, void *), - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 7/7] libata: send event when AN received
When we get an SDB FIS with the 'N' bit set, we should send an event to user space to indicate that there has been a media change. This will be done via the block device. changed from last version: * Make sure that port_addr is within ATA_MAX_DEVICES Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -1147,6 +1147,28 @@ static void ahci_host_intr(struct ata_po return; } + if (status PORT_IRQ_SDB_FIS) { + /* +* if this is an ATAPI device with AN turned on, +* then we should interrogate the device to +* determine the cause of the interrupt +* +* for AN - this we should check the SDB FIS +* and find the I and N bits set +*/ + const u32 *f = pp-rx_fis + RX_FIS_SDB; + + /* check the 'N' bit in word 0 of the FIS */ + if (f[0] (1 15)) { + int port_addr = ((f[0] 0x0f00) 8); + struct ata_device *adev; + if (port_addr ATA_MAX_DEVICES) { + adev = ap-device[port_addr]; + if (adev-flags ATA_DFLAG_AN) + ata_scsi_media_change_notify(adev); + } + } + } if (ap-sactive) qc_active = readl(port_mmio + PORT_SCR_ACT); else Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -737,6 +737,7 @@ extern void ata_host_init(struct ata_hos extern int ata_scsi_detect(struct scsi_host_template *sht); extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg); extern int ata_scsi_queuecmd(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)); +extern void ata_scsi_media_change_notify(struct ata_device *atadev); extern void ata_sas_port_destroy(struct ata_port *); extern struct ata_port *ata_sas_port_alloc(struct ata_host *, struct ata_port_info *, struct Scsi_Host *); Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -3057,6 +3057,22 @@ static void ata_scsi_remove_dev(struct a } /** + * ata_scsi_media_change_notify - send media change event + * @atadev: Pointer to the disk device with media change event + * + * Tell the block layer to send a media change notification + * event. + * + * LOCKING: + * interrupt context, may not sleep. + */ +void ata_scsi_media_change_notify(struct ata_device *atadev) +{ + genhd_media_change_notify(atadev-sdev-disk); +} +EXPORT_SYMBOL_GPL(ata_scsi_media_change_notify); + +/** * ata_scsi_hotplug - SCSI part of hotplug * @work: Pointer to ATA port to perform SCSI hotplug on * - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 1/7] libata: check for AN support
Check to see if an ATAPI device supports Asynchronous Notification. If so, enable it. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/libata-core.c === --- 2.6-git.orig/drivers/ata/libata-core.c +++ 2.6-git/drivers/ata/libata-core.c @@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long static unsigned int ata_dev_init_params(struct ata_device *dev, u16 heads, u16 sectors); static unsigned int ata_dev_set_xfermode(struct ata_device *dev); +static unsigned int ata_dev_set_AN(struct ata_device *dev); static void ata_dev_xfermask(struct ata_device *dev); static unsigned int ata_print_id = 1; @@ -1744,6 +1745,23 @@ int ata_dev_configure(struct ata_device } dev-cdb_len = (unsigned int) rc; + /* +* check to see if this ATAPI device supports +* Asynchronous Notification +*/ + if ((ap-flags ATA_FLAG_AN) ata_id_has_AN(id)) + { + /* issue SET feature command to turn this on */ + rc = ata_dev_set_AN(dev); + if (rc) { + ata_dev_printk(dev, KERN_ERR, + unable to set AN\n); + rc = -EINVAL; + goto err_out_nosup; + } + dev-flags |= ATA_DFLAG_AN; + } + if (ata_id_cdb_intr(dev-id)) { dev-flags |= ATA_DFLAG_CDB_INTR; cdb_intr_string = , CDB intr; @@ -3525,6 +3543,42 @@ static unsigned int ata_dev_set_xfermode } /** + * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES + * with sector count set to indicate + * Asynchronous Notification feature + * @dev: Device to which command will be sent + * + * Issue SET FEATURES - SATA FEATURES command to device @dev + * on port @ap. + * + * LOCKING: + * PCI/etc. bus probe sem. + * + * RETURNS: + * 0 on success, AC_ERR_* mask otherwise. + */ +static unsigned int ata_dev_set_AN(struct ata_device *dev) +{ + struct ata_taskfile tf; + unsigned int err_mask; + + /* set up set-features taskfile */ + DPRINTK(set features - SATA features\n); + + ata_tf_init(dev, tf); + tf.command = ATA_CMD_SET_FEATURES; + tf.feature = SETFEATURES_SATA_ENABLE; + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + tf.protocol = ATA_PROT_NODATA; + tf.nsect = SATA_AN; + + err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0); + + DPRINTK(EXIT, err_mask=%x\n, err_mask); + return err_mask; +} + +/** * ata_dev_init_params - Issue INIT DEV PARAMS command * @dev: Device to which command will be sent * @heads: Number of heads (taskfile parameter) Index: 2.6-git/include/linux/ata.h === --- 2.6-git.orig/include/linux/ata.h +++ 2.6-git/include/linux/ata.h @@ -194,6 +194,12 @@ enum { SETFEATURES_WC_ON = 0x02, /* Enable write cache */ SETFEATURES_WC_OFF = 0x82, /* Disable write cache */ + SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */ + SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */ + + /* SETFEATURE Sector counts for SATA features */ + SATA_AN = 0x05, /* Asynchronous Notification */ + /* ATAPI stuff */ ATAPI_PKT_DMA = (1 0), ATAPI_DMADIR= (1 2), /* ATAPI data dir: @@ -299,6 +305,8 @@ struct ata_taskfile { #define ata_id_queue_depth(id) (((id)[75] 0x1f) + 1) #define ata_id_removeable(id) ((id)[0] (1 7)) #define ata_id_has_dword_io(id)((id)[50] (1 0)) +#define ata_id_has_AN(id) \ + ((id[76] (~id[76])) ((id)[78] (1 5))) #define ata_id_iordy_disable(id) ((id)[49] (1 10)) #define ata_id_has_iordy(id) ((id)[49] (1 9)) #define ata_id_u32(id,n) \ Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -136,6 +136,7 @@ enum { ATA_DFLAG_CDB_INTR = (1 2), /* device asserts INTRQ when ready for CDB */ ATA_DFLAG_NCQ = (1 3), /* device supports NCQ */ ATA_DFLAG_FLUSH_EXT = (1 4), /* do FLUSH_EXT instead of FLUSH */ + ATA_DFLAG_AN= (1 5), /* device supports Async notification */ ATA_DFLAG_CFG_MASK = (1 8) - 1, ATA_DFLAG_PIO = (1 8), /* device limited to PIO mode */ @@ -174,6 +175,7 @@ enum { ATA_FLAG_SETXFER_POLLING= (1 14), /* use polling for SETXFER
[patch 0/7] Asynchronous Notification for ATAPI devices (v2)
This patch series implements Asynchronous Notification (AN) for SATA ATAPI devices as defined in SATA 2.5 and AHCI 1.1 and higher. Drives which support this feature will send a notification when new media is inserted and removed, preventing the need for user space to poll for new media. This support is exposed to user space via a flag that will be set in /sys/block/sr*/capability_flags. If the flag is set, user space can disable polling for the new media, and the genhd driver will send a KOBJ_CHANGE event with the envp set to MEDIA_CHANGE_EVENT=1. Note that this patch only implements support for directly attached drives - AN with drives attached to a port multiplier requires additional changes. Thanks! Kristen -- -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 3/7] scsi: expose AN to user space
Get media change notification capability from disk and pass this information to genhd by setting appropriate flag. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/sr.c === --- 2.6-git.orig/drivers/scsi/sr.c +++ 2.6-git/drivers/scsi/sr.c @@ -601,6 +601,8 @@ static int sr_probe(struct device *dev) dev_set_drvdata(dev, cd); disk-flags |= GENHD_FL_REMOVABLE; + if (sdev-media_change_notify) + disk-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; add_disk(disk); sdev_printk(KERN_DEBUG, sdev, Index: 2.6-git/include/scsi/scsi_device.h === --- 2.6-git.orig/include/scsi/scsi_device.h +++ 2.6-git/include/scsi/scsi_device.h @@ -124,7 +124,7 @@ struct scsi_device { unsigned fix_capacity:1;/* READ_CAPACITY is too high by 1 */ unsigned guess_capacity:1; /* READ_CAPACITY might be too high by 1 */ unsigned retry_hwerror:1; /* Retry HARDWARE_ERROR */ - + unsigned media_change_notify:1; /* dev supports async media notify */ unsigned int device_blocked;/* Device returned QUEUE_FULL. */ unsigned int max_device_blocked; /* what device_blocked counts down from */ Index: 2.6-git/drivers/scsi/sd.c === --- 2.6-git.orig/drivers/scsi/sd.c +++ 2.6-git/drivers/scsi/sd.c @@ -1706,6 +1706,9 @@ static int sd_probe(struct device *dev) if (sdp-removable) gd-flags |= GENHD_FL_REMOVABLE; + if (sdp-media_change_notify) + gd-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; + dev_set_drvdata(dev, sdkp); add_disk(gd); -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 5/7] genhd: send async notification on media change
Send an uevent to user space to indicate that a media change event has occurred. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/block/genhd.c === --- 2.6-git.orig/block/genhd.c +++ 2.6-git/block/genhd.c @@ -643,6 +643,25 @@ struct seq_operations diskstats_op = { .show = diskstats_show }; +static void media_change_notify_thread(struct work_struct *work) +{ + struct gendisk *gd = container_of(work, struct gendisk, async_notify); + char event[] = MEDIA_CHANGE=1; + char *envp[] = { event, NULL }; + + /* +* set enviroment vars to indicate which event this is for +* so that user space will know to go check the media status. +*/ + kobject_uevent_env(gd-kobj, KOBJ_CHANGE, envp); +} + +void genhd_media_change_notify(struct gendisk *disk) +{ + schedule_work(disk-async_notify); +} +EXPORT_SYMBOL_GPL(genhd_media_change_notify); + struct gendisk *alloc_disk(int minors) { return alloc_disk_node(minors, -1); @@ -672,6 +691,8 @@ struct gendisk *alloc_disk_node(int mino kobj_set_kset_s(disk,block_subsys); kobject_init(disk-kobj); rand_initialize_disk(disk); + INIT_WORK(disk-async_notify, + media_change_notify_thread); } return disk; } Index: 2.6-git/include/linux/genhd.h === --- 2.6-git.orig/include/linux/genhd.h +++ 2.6-git/include/linux/genhd.h @@ -66,6 +66,7 @@ struct partition { #include linux/smp.h #include linux/string.h #include linux/fs.h +#include linux/workqueue.h struct partition { unsigned char boot_ind; /* 0x80 - active */ @@ -139,6 +140,7 @@ struct gendisk { #else struct disk_stats dkstats; #endif + struct work_struct async_notify; }; /* Structure for sysfs attributes on block devices */ @@ -419,7 +421,7 @@ extern struct gendisk *alloc_disk_node(i extern struct gendisk *alloc_disk(int minors); extern struct kobject *get_disk(struct gendisk *disk); extern void put_disk(struct gendisk *disk); - +extern void genhd_media_change_notify(struct gendisk *disk); extern void blk_register_region(dev_t dev, unsigned long range, struct module *module, struct kobject *(*probe)(dev_t, int *, void *), -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 6/7] SCSI: save disk in scsi_device
Give anyone who has access to scsi_device access to the genhd struct as well. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/scsi/sd.c === --- 2.6-git.orig/drivers/scsi/sd.c +++ 2.6-git/drivers/scsi/sd.c @@ -1711,6 +1711,7 @@ static int sd_probe(struct device *dev) dev_set_drvdata(dev, sdkp); add_disk(gd); + sdp-disk = gd; sdev_printk(KERN_NOTICE, sdp, Attached scsi %sdisk %s\n, sdp-removable ? removable : , gd-disk_name); Index: 2.6-git/drivers/scsi/sr.c === --- 2.6-git.orig/drivers/scsi/sr.c +++ 2.6-git/drivers/scsi/sr.c @@ -604,6 +604,7 @@ static int sr_probe(struct device *dev) if (sdev-media_change_notify) disk-flags |= GENHD_FL_MEDIA_CHANGE_NOTIFY; add_disk(disk); + sdev-disk = disk; sdev_printk(KERN_DEBUG, sdev, Attached scsi CD-ROM %s\n, cd-cdi.name); Index: 2.6-git/include/scsi/scsi_device.h === --- 2.6-git.orig/include/scsi/scsi_device.h +++ 2.6-git/include/scsi/scsi_device.h @@ -138,7 +138,7 @@ struct scsi_device { struct device sdev_gendev; struct class_device sdev_classdev; - + struct gendisk *disk; struct execute_work ew; /* used to get process context on put */ enum scsi_device_state sdev_state; -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch 7/7] libata: send event when AN received
When we get an SDB FIS with the 'N' bit set, we should send an event to user space to indicate that there has been a media change. This will be done via the block device. Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED] Index: 2.6-git/drivers/ata/ahci.c === --- 2.6-git.orig/drivers/ata/ahci.c +++ 2.6-git/drivers/ata/ahci.c @@ -1147,6 +1147,25 @@ static void ahci_host_intr(struct ata_po return; } + if (status PORT_IRQ_SDB_FIS) { + /* +* if this is an ATAPI device with AN turned on, +* then we should interrogate the device to +* determine the cause of the interrupt +* +* for AN - this we should check the SDB FIS +* and find the I and N bits set +*/ + const u32 *f = pp-rx_fis + RX_FIS_SDB; + + /* check the 'N' bit in word 0 of the FIS */ + if (f[0] (1 15)) { + int port_addr = ((f[0] 0x0f00) 8); + struct ata_device *adev = ap-device[port_addr]; + if (adev-flags ATA_DFLAG_AN) + ata_scsi_media_change_notify(adev); + } + } if (ap-sactive) qc_active = readl(port_mmio + PORT_SCR_ACT); else Index: 2.6-git/include/linux/libata.h === --- 2.6-git.orig/include/linux/libata.h +++ 2.6-git/include/linux/libata.h @@ -737,6 +737,7 @@ extern void ata_host_init(struct ata_hos extern int ata_scsi_detect(struct scsi_host_template *sht); extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg); extern int ata_scsi_queuecmd(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)); +extern void ata_scsi_media_change_notify(struct ata_device *atadev); extern void ata_sas_port_destroy(struct ata_port *); extern struct ata_port *ata_sas_port_alloc(struct ata_host *, struct ata_port_info *, struct Scsi_Host *); Index: 2.6-git/drivers/ata/libata-scsi.c === --- 2.6-git.orig/drivers/ata/libata-scsi.c +++ 2.6-git/drivers/ata/libata-scsi.c @@ -3057,6 +3057,22 @@ static void ata_scsi_remove_dev(struct a } /** + * ata_scsi_media_change_notify - send media change event + * @atadev: Pointer to the disk device with media change event + * + * Tell the block layer to send a media change notification + * event. + * + * LOCKING: + * interrupt context, may not sleep. + */ +void ata_scsi_media_change_notify(struct ata_device *atadev) +{ + genhd_media_change_notify(atadev-sdev-disk); +} +EXPORT_SYMBOL_GPL(ata_scsi_media_change_notify); + +/** * ata_scsi_hotplug - SCSI part of hotplug * @work: Pointer to ATA port to perform SCSI hotplug on * -- - To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html