Re: [PATCH] enclosure: add support for enclosure services

2008-02-13 Thread Kristen Carlson Accardi
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

2008-02-12 Thread Kristen Carlson Accardi
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

2008-02-12 Thread Kristen Carlson Accardi
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

2007-09-12 Thread Kristen Carlson Accardi
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

2007-08-15 Thread Kristen Carlson Accardi
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

2007-08-15 Thread Kristen Carlson Accardi
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

2007-08-13 Thread Kristen Carlson Accardi
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

2007-08-09 Thread Kristen Carlson Accardi
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

2007-08-08 Thread Kristen Carlson Accardi
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

2007-08-08 Thread Kristen Carlson Accardi
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

2007-08-08 Thread Kristen Carlson Accardi
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

2007-08-08 Thread Kristen Carlson Accardi
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

2007-08-08 Thread Kristen Carlson Accardi
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

2007-08-01 Thread Kristen Carlson Accardi
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

2007-08-01 Thread Kristen Carlson Accardi
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

2007-08-01 Thread Kristen Carlson Accardi
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

2007-08-01 Thread Kristen Carlson Accardi
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

2007-08-01 Thread Kristen Carlson Accardi
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

2007-08-01 Thread Kristen Carlson Accardi
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

2007-08-01 Thread Kristen Carlson Accardi
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

2007-08-01 Thread Kristen Carlson Accardi
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

2007-08-01 Thread Kristen Carlson Accardi
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.

2007-08-01 Thread Kristen Carlson Accardi
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

2007-07-31 Thread Kristen Carlson Accardi
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

2007-07-31 Thread Kristen Carlson Accardi
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

2007-07-31 Thread Kristen Carlson Accardi
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

2007-07-31 Thread Kristen Carlson Accardi
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

2007-07-11 Thread Kristen Carlson Accardi
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)

2007-07-05 Thread Kristen Carlson Accardi
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

2007-07-05 Thread Kristen Carlson Accardi
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

2007-07-05 Thread Kristen Carlson Accardi
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

2007-07-05 Thread Kristen Carlson Accardi
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.

2007-07-05 Thread Kristen Carlson Accardi
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

2007-07-05 Thread Kristen Carlson Accardi
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

2007-07-05 Thread Kristen Carlson Accardi
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

2007-07-05 Thread Kristen Carlson Accardi
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.

2007-06-26 Thread Kristen Carlson Accardi
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

2007-06-20 Thread Kristen Carlson Accardi
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.

2007-06-20 Thread Kristen Carlson Accardi
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

2007-06-14 Thread Kristen Carlson Accardi
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

2007-06-13 Thread Kristen Carlson Accardi
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

2007-06-12 Thread Kristen Carlson Accardi
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.

2007-06-12 Thread Kristen Carlson Accardi
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

2007-06-12 Thread Kristen Carlson Accardi
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

2007-06-12 Thread Kristen Carlson Accardi
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

2007-06-11 Thread Kristen Carlson Accardi
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

2007-06-11 Thread Kristen Carlson Accardi
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

2007-06-11 Thread Kristen Carlson Accardi
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.

2007-06-11 Thread Kristen Carlson Accardi
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

2007-06-11 Thread Kristen Carlson Accardi
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

2007-05-23 Thread Kristen Carlson Accardi
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

2007-05-23 Thread Kristen Carlson Accardi
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

2007-05-23 Thread Kristen Carlson Accardi
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

2007-05-10 Thread Kristen Carlson Accardi
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

2007-05-09 Thread Kristen Carlson Accardi
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)

2007-05-09 Thread Kristen Carlson Accardi
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

2007-05-09 Thread Kristen Carlson Accardi
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

2007-05-09 Thread Kristen Carlson Accardi
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

2007-05-09 Thread Kristen Carlson Accardi
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

2007-05-09 Thread Kristen Carlson Accardi
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

2007-05-09 Thread Kristen Carlson Accardi
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

2007-05-09 Thread Kristen Carlson Accardi
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

2007-05-09 Thread Kristen Carlson Accardi
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

2007-05-07 Thread Kristen Carlson Accardi
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

2007-05-04 Thread Kristen Carlson Accardi
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

2007-05-04 Thread Kristen Carlson Accardi
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

2007-05-04 Thread Kristen Carlson Accardi
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

2007-05-04 Thread Kristen Carlson Accardi
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

2007-05-04 Thread Kristen Carlson Accardi
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

2007-05-04 Thread Kristen Carlson Accardi
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

2007-05-04 Thread Kristen Carlson Accardi
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

2007-04-25 Thread Kristen Carlson Accardi
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

2007-04-25 Thread Kristen Carlson Accardi
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

2007-04-25 Thread Kristen Carlson Accardi
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

2007-04-24 Thread Kristen Carlson Accardi
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

2007-04-24 Thread Kristen Carlson Accardi
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

2007-04-24 Thread Kristen Carlson Accardi
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

2007-04-24 Thread Kristen Carlson Accardi
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

2007-04-24 Thread Kristen Carlson Accardi
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

2007-04-24 Thread Kristen Carlson Accardi
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

2007-04-24 Thread Kristen Carlson Accardi
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

2007-04-23 Thread Kristen Carlson Accardi
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)

2007-04-23 Thread Kristen Carlson Accardi
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

2007-04-23 Thread Kristen Carlson Accardi
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

2007-04-23 Thread Kristen Carlson Accardi
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

2007-04-23 Thread Kristen Carlson Accardi
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

2007-04-23 Thread Kristen Carlson Accardi
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