implement PMP support, take 6
PATCH 2/2 implement PMP support, take 6
Hello, all.
his is the sixth take of libata-pmp patchset. This patchset contains 10 patches and implements PMP support.
Changes from the last take[L] are.
* Better AN handling. All AHCI now can do hotplug on downstream ports whether it has SNTF or not - patch #6 and #8 updated.
* Blacklisting of controllers which falsely claim PMP support - new patches #9 and #10.
This patchset is against
libata-dev#upstream (b56feca2ede0dd11a8409743ff14cf14cbcf5062)
+ [1] sata_sil24-fix-IRQ-clearing-race-when-PCIX_IRQ_WOC-is-used
+ [2] libata-pmp patchset, take #6
Thanks.
--
tejun
[L] http://thread.gmane.org/gmane.linux.ide/20870
[1] http://article.gmane.org/gmane.linux.ide/22918
[2] http://thread.gmane.org/gmane.linux.ide/22921
PATCH 1/10 libata-pmp: update ata_eh_reset() for PMP
PMP always requires SRST to be enabled. Also, hardreset reports classification code from the first device when PMP is attached, not from the PMP. Update ata_eh_reset() such that followup softreset is performed if the controller is PMP capable and the host link is being reset.
--- drivers/ata/libata-eh.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/ata/libata-eh.c +2 -0
@@ -2003,6 +2003,8 @@ static int ata_eh_followup_srst_needed(struct ata_link *link,return 1 ;if (rc !=0 )return 0 ; +if ((link->ap->flags &ATA_FLAG_PMP ) &&ata_is_host_link (link)) +return 1 ;if (classify && !(link->flags &ATA_LFLAG_ASSUME_CLASS ) && classes[0 ] ==ATA_DEV_UNKNOWN )return 1 ;
Signature · 1 line
1.5.0.3
PATCH 2/10 libata-pmp: implement Port Multiplier support
In reply to Tejun HeoImplement Port Multiplier support. To support PMP, a LLDD has to supply ops->pmp_read() and pmp_write(). If non-null, ->pmp_attach and ->pmp_detach are called on PMP attach and detach, respectively.
->pmp_read/write() can be called while the port is frozen, so they must be implemented by polling. This patch supplies several helpers to ease ->pmp_read/write() implementation.
Also, irq_handler and error_handler must be PMP aware. Most of PMP aware EH can be done by calling ata_pmp_do_eh() with appropriate methods. PMP EH uses separate set of reset methods and this patch implements standard prereset, hardreset and postreset methods.
This patch only implements PMP support. The next patch will integrate PMP into the reset of libata and thus enable PMP support.
--- drivers/ata/Makefile | 3 +- drivers/ata/libata-core.c | 11 + drivers/ata/libata-pmp.c | 1182 +++++++++++++++++++++++++++++++++++++++++++++ drivers/ata/libata.h | 5 + include/linux/libata.h | 19 + 5 files changed, 1219 insertions(+), 1 deletions(-) create mode 100644 drivers/ata/libata-pmp.c
drivers/ata/Makefile +2 -1
@@ -70,5 +70,6 @@ obj-$(CONFIG_ATA_GENERIC) += ata_generic.o # Should be last libata driver obj-$(CONFIG_PATA_LEGACY) += pata_legacy.o -libata-objs := libata-core.o libata-scsi.o libata-sff.o libata-eh.o +libata-objs := libata-core.o libata-scsi.o libata-sff.o libata-eh.o \ + libata-pmp.o libata-$(CONFIG_ATA_ACPI) += libata-acpi.o
drivers/ata/libata-core.c +11 -0
@@ -3863,6 +3863,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = { { "IOMEGA ZIP 250 ATAPI", NULL, ATA_HORKAGE_NODMA }, /* temporary fix */ { "IOMEGA ZIP 250 ATAPI Floppy", NULL, ATA_HORKAGE_NODMA }, +/* Odd clown on sil3726/4726 PMPs */ + { "Config Disk", NULL, ATA_HORKAGE_NODMA | + ATA_HORKAGE_SKIP_PM },/* Weird ATAPI devices */ { "TORiSAN DVD-ROM DRD-N216", NULL, ATA_HORKAGE_MAX_SEC_128 },
@@ -7242,6 +7245,14 @@ EXPORT_SYMBOL_GPL(ata_pci_default_filter);EXPORT_SYMBOL_GPL (ata_pci_clear_simplex);#endif /* CONFIG_PCI */ +EXPORT_SYMBOL_GPL (sata_pmp_read_init_tf); +EXPORT_SYMBOL_GPL (sata_pmp_read_val); +EXPORT_SYMBOL_GPL (sata_pmp_write_init_tf); +EXPORT_SYMBOL_GPL (sata_pmp_std_prereset); +EXPORT_SYMBOL_GPL (sata_pmp_std_hardreset); +EXPORT_SYMBOL_GPL (sata_pmp_std_postreset); +EXPORT_SYMBOL_GPL (sata_pmp_do_eh); +EXPORT_SYMBOL_GPL (__ata_ehi_push_desc);EXPORT_SYMBOL_GPL (ata_ehi_push_desc);EXPORT_SYMBOL_GPL (ata_ehi_clear_desc);
drivers/ata/libata-pmp.c new file +1182 -0
1,182 lines not on this page · raw message
drivers/ata/libata.h +5 -0
@@ -152,6 +152,11 @@ extern void ata_schedule_scsi_eh(struct Scsi_Host *shost);extern void ata_scsi_dev_rescan (struct work_struct *work);extern int ata_bus_probe (struct ata_port *ap); +/* libata-pmp.c */ +extern int sata_pmp_scr_read (struct ata_link *link,int reg,u32 *val); +extern int sata_pmp_scr_write (struct ata_link *link,int reg,u32 val); +extern int sata_pmp_attach (struct ata_device *dev); +/* libata-eh.c */ extern enum scsi_eh_timer_return ata_scsi_timed_out (struct scsi_cmnd *cmd);extern void ata_scsi_error (struct Scsi_Host *host);
include/linux/libata.h +19 -0
@@ -944,6 +944,25 @@ extern unsigned long ata_pci_default_filter(struct ata_device *, unsigned long);#endif /* CONFIG_PCI */ /* +* PMP +*/ +extern void sata_pmp_read_init_tf (struct ata_taskfile *tf, +struct ata_device *dev,int pmp,int reg); +extern u32 sata_pmp_read_val (const struct ata_taskfile *tf); +extern void sata_pmp_write_init_tf (struct ata_taskfile *tf, +struct ata_device *dev, +int pmp,int reg,u32 val); +extern int sata_pmp_std_prereset (struct ata_link *link,unsigned long deadline); +extern int sata_pmp_std_hardreset (struct ata_link *link,unsigned int *class, +unsigned long deadline); +extern void sata_pmp_std_postreset (struct ata_link *link,unsigned int *class); +extern void sata_pmp_do_eh (struct ata_port *ap, +ata_prereset_fn_t prereset,ata_reset_fn_t softreset, +ata_reset_fn_t hardreset,ata_postreset_fn_t postreset, +ata_prereset_fn_t pmp_prereset,ata_reset_fn_t pmp_softreset, +ata_reset_fn_t pmp_hardreset,ata_postreset_fn_t pmp_postreset); + +/* * EH */ extern void ata_eng_timeout (struct ata_port *ap);
Signature · 1 line
1.5.0.3
PATCH 3/10 libata-pmp: hook PMP support and enable it
In reply to Tejun HeoHook PMP support into libata and enable it. Connect SCR and probing functions, and update ata_dev_classify() to detect PMP.
--- drivers/ata/libata-core.c | 113 ++++++++++++++++++++++++++++++++------------- drivers/ata/libata-eh.c | 18 ++++++- 2 files changed, 96 insertions(+), 35 deletions(-)
drivers/ata/libata-core.c +81 -32
@@ -59,7 +59,7 @@#include "libata.h" -#define DRV_VERSION "2.21"/* must be exactly four chars */ +#define DRV_VERSION "3.00"/* must be exactly four chars */ /* debounce timing parameters in msecs { interval, duration, timeout } */
@@ -672,29 +672,49 @@ static unsigned int ata_devchk(struct ata_port *ap, unsigned int device)* None. * * RETURNS: -* Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, or %ATA_DEV_UNKNOWN -* the event of failure. +* Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or +* %ATA_DEV_UNKNOWN the event of failure. */ -unsigned int ata_dev_classify (const struct ata_taskfile *tf) {/* Apple's open source Darwin code hints that some devices only * put a proper signature into the LBA mid/high registers, * So, we only check those. It's sufficient for uniqueness. +* +* ATA/ATAPI-7 (d1532v1r1: Feb. 19, 2003) specified separate +* signatures for ATA and ATAPI devices attached on SerialATA, +* 0x3c/0xc3 and 0x69/0x96 respectively. However, SerialATA +* spec has never mentioned about using different signatures +* for ATA/ATAPI devices. Then, Serial ATA II: Port +* Multiplier specification began to use 0x69/0x96 to identify +* port multpliers and 0x3c/0xc3 to identify SEMB device. +* ATA/ATAPI-7 dropped descriptions about 0x3c/0xc3 and +* 0x69/0x96 shortly and described them as reserved for +* SerialATA. +* +* We follow the current spec and consider that 0x69/0x96 +* identifies a port multiplier and 0x3c/0xc3 a SEMB device. */ - -if (((tf->lbam ==0 ) && (tf->lbah ==0 )) || - ((tf->lbam ==0x3c ) && (tf->lbah ==0xc3 ))) { +if ((tf->lbam ==0 ) && (tf->lbah ==0 )) {DPRINTK ("found ATA device by sig\n" );return ATA_DEV_ATA ; } -if (((tf->lbam ==0x14 ) && (tf->lbah ==0xeb )) || - ((tf->lbam ==0x69 ) && (tf->lbah ==0x96 ))) { +if ((tf->lbam ==0x14 ) && (tf->lbah ==0xeb )) {DPRINTK ("found ATAPI device by sig\n" );return ATA_DEV_ATAPI ; } +if ((tf->lbam ==0x69 ) && (tf->lbah ==0x96 )) { +DPRINTK ("found PMP device by sig\n" ); +return ATA_DEV_PMP ; + } + +if ((tf->lbam ==0x3c ) && (tf->lbah ==0xc3 )) { +printk ("ata: SEMB device ignored\n" ); +return ATA_DEV_SEMB_UNSUP ;/* not yet */ + } +DPRINTK ("unknown device\n" );return ATA_DEV_UNKNOWN ; }
@@ -3424,6 +3444,12 @@ int ata_std_prereset(struct ata_link *link, unsigned long deadline) (link->flags & ATA_LFLAG_HRST_TO_RESUME)) ehc->i.action |= ATA_EH_HARDRESET; +/* Some PMPs don't work with only SRST, force hardreset if PMP +* is supported. +*/ +if (ap->flags &ATA_FLAG_PMP ) + ehc->i.action |=ATA_EH_HARDRESET ; +/* if we're about to do hardreset, nothing more to do */ if (ehc->i.action &ATA_EH_HARDRESET )return 0 ;
@@ -3614,6 +3640,16 @@ int sata_std_hardreset(struct ata_link *link, unsigned int *class,/* wait a while before checking status, see SRST for more info */ msleep (150 ); +/* If PMP is supported, we have to do follow-up SRST. Note +* that some PMPs don't send D2H Reg FIS after hardreset at +* all if the first port is empty. Wait for it just for a +* second and request follow-up SRST. +*/ +if (ap->flags &ATA_FLAG_PMP ) { +ata_wait_ready (ap, jiffies +HZ ); +return -EAGAIN ; + } + rc =ata_wait_ready (ap, deadline);/* link occupied, -ENODEV too is an error */ if (rc) {
@@ -5957,22 +5993,26 @@ int sata_scr_valid(struct ata_link *link)* @val: Place to store read value * * Read SCR register @reg of @link into *@val. This function is -* guaranteed to succeed if the cable type of the port is SATA -* and the port implements ->scr_read. +* guaranteed to succeed if @link is ap->link, the cable type of +* the port is SATA and the port implements ->scr_read. * * LOCKING: -* None. +* None if @link is ap->link. Kernel thread context otherwise. * * RETURNS: * 0 on success, negative errno on failure. */ int sata_scr_read (struct ata_link *link,int reg,u32 *val) { -struct ata_port *ap = link->ap; +if (ata_is_host_link (link)) { +struct ata_port *ap = link->ap; -if (sata_scr_valid (link)) -return ap->ops->scr_read (ap, reg, val); -return -EOPNOTSUPP ; +if (sata_scr_valid (link)) +return ap->ops->scr_read (ap, reg, val); +return -EOPNOTSUPP ; + } + +return sata_pmp_scr_read (link, reg, val); }/**
@@ -5982,22 +6022,26 @@ int sata_scr_read(struct ata_link *link, int reg, u32 *val)* @val: value to write * * Write @val to SCR register @reg of @link. This function is -* guaranteed to succeed if the cable type of the port is SATA -* and the port implements ->scr_read. +* guaranteed to succeed if @link is ap->link, the cable type of +* the port is SATA and the port implements ->scr_read. * * LOCKING: -* None. +* None if @link is ap->link. Kernel thread context otherwise. * * RETURNS: * 0 on success, negative errno on failure. */ int sata_scr_write (struct ata_link *link,int reg,u32 val) { -struct ata_port *ap = link->ap; +if (ata_is_host_link (link)) { +struct ata_port *ap = link->ap; -if (sata_scr_valid (link)) -return ap->ops->scr_write (ap, reg, val); -return -EOPNOTSUPP ; +if (sata_scr_valid (link)) +return ap->ops->scr_write (ap, reg, val); +return -EOPNOTSUPP ; + } + +return sata_pmp_scr_write (link, reg, val); }/**
@@ -6010,23 +6054,27 @@ int sata_scr_write(struct ata_link *link, int reg, u32 val)* function performs flush after writing to the register. * * LOCKING: -* None. +* None if @link is ap->link. Kernel thread context otherwise. * * RETURNS: * 0 on success, negative errno on failure. */ int sata_scr_write_flush (struct ata_link *link,int reg,u32 val) { -struct ata_port *ap = link->ap; -int rc; +if (ata_is_host_link (link)) { +struct ata_port *ap = link->ap; +int rc; -if (sata_scr_valid (link)) { - rc = ap->ops->scr_write (ap, reg, val); -if (rc ==0 ) - rc = ap->ops->scr_read (ap, reg, &val); -return rc; +if (sata_scr_valid (link)) { + rc = ap->ops->scr_write (ap, reg, val); +if (rc ==0 ) + rc = ap->ops->scr_read (ap, reg, &val); +return rc; + } +return -EOPNOTSUPP ; } -return -EOPNOTSUPP ; + +return sata_pmp_scr_write (link, reg, val); }/**
@@ -6415,6 +6463,7 @@ static void ata_host_release(struct device *gendev, void *res)if (ap->scsi_host)scsi_host_put (ap->scsi_host); +kfree (ap->pmp_link);kfree (ap); host->ports[i] =NULL ; }
drivers/ata/libata-eh.c +15 -3
@@ -2209,6 +2209,8 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link, readid_flags |=ATA_READID_POSTRESET ;if ((action &ATA_EH_REVALIDATE ) &&ata_dev_enabled (dev)) { +WARN_ON (dev->class ==ATA_DEV_PMP ); +if (ata_link_offline (link)) { rc = -EIO ; goto err;
@@ -2234,8 +2236,11 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link, ata_class_enabled(ehc->classes[dev->devno])) { dev->class = ehc->classes[dev->devno]; - rc =ata_dev_read_id (dev, &dev->class, readid_flags, - dev->id); +if (dev->class ==ATA_DEV_PMP ) + rc =sata_pmp_attach (dev); +else + rc =ata_dev_read_id (dev, &dev->class, + readid_flags, dev->id);switch (rc) {case 0 : new_mask |=1 << dev->devno;
@@ -2264,7 +2269,8 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,* device detection messages backwards. */ ata_link_for_each_dev (dev, link) { -if (!(new_mask & (1 << dev->devno))) +if (!(new_mask & (1 << dev->devno)) || + dev->class ==ATA_DEV_PMP )continue ; ehc->i.flags |=ATA_EHI_PRINTINFO ;
@@ -2521,6 +2527,12 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,if (rc) goto dev_fail; +/* if PMP got attached, return, pmp EH will take care of it */ +if (link->device->class ==ATA_DEV_PMP ) { + ehc->i.action =0 ; +return 0 ; + } +/* configure transfer mode if necessary */ if (ehc->i.flags &ATA_EHI_SETMODE ) { rc =ata_set_mode (link, &dev);
Signature · 1 line
1.5.0.3
PATCH 4/10 libata-pmp: extend ACPI support to cover PMP
In reply to Tejun HeoExtend ata_acpi_associate_sata_port() such that it can handle PMP and call it when PMP is attached and detached.
Build breakage when !CONFIG_ATA_ACPI was spotted and fixed by Petr Vandrovec.
--- drivers/ata/libata-acpi.c | 36 ++++++++++++++++++++++++++++++++---- drivers/ata/libata-pmp.c | 4 ++++ drivers/ata/libata.h | 2 ++ 3 files changed, 38 insertions(+), 4 deletions(-)
drivers/ata/libata-acpi.c +32 -4
@@ -40,12 +40,40 @@ static int is_pci_dev(struct device *dev)return (dev->bus == &pci_bus_type); } -static void ata_acpi_associate_sata_port (struct ata_port *ap) +/** +* ata_acpi_associate_sata_port - associate SATA port with ACPI objects +* @ap: target SATA port +* +* Look up ACPI objects associated with @ap and initialize acpi_handle +* fields of @ap, the port and devices accordingly. +* +* LOCKING: +* EH context. +* +* RETURNS: +* 0 on success, -errno on failure. +*/ +void ata_acpi_associate_sata_port (struct ata_port *ap) { -acpi_integer adr =SATA_ADR (ap->port_no,NO_PORT_MULT ); +WARN_ON (!(ap->flags &ATA_FLAG_ACPI_SATA )); + +if (!ap->nr_pmp_links) { +acpi_integer adr =SATA_ADR (ap->port_no,NO_PORT_MULT ); + + ap->link.device->acpi_handle = +acpi_get_child (ap->host->acpi_handle, adr); + }else { +struct ata_link *link; + + ap->link.device->acpi_handle =NULL ; - ap->link.device->acpi_handle = -acpi_get_child (ap->host->acpi_handle, adr); +ata_port_for_each_link (link, ap) { +acpi_integer adr =SATA_ADR (ap->port_no, link->pmp); + + link->device->acpi_handle = +acpi_get_child (ap->host->acpi_handle, adr); + } + } }static void ata_acpi_associate_ide_port (struct ata_port *ap)
drivers/ata/libata-pmp.c +4 -0
@@ -607,6 +607,8 @@ int sata_pmp_attach(struct ata_device *dev)ata_port_for_each_link (tlink, ap)sata_link_init_spd (tlink); +ata_acpi_associate_sata_port (ap); +return 0 ; fail:
@@ -646,6 +648,8 @@ static void sata_pmp_detach(struct ata_device *dev) ap->nr_pmp_links =0 ; link->pmp =0 ;spin_unlock_irqrestore (ap->lock, flags); + +ata_acpi_associate_sata_port (ap); }/**
drivers/ata/libata.h +2 -0
@@ -102,11 +102,13 @@ extern struct ata_port *ata_port_alloc(struct ata_host *host);/* libata-acpi.c */ #ifdef CONFIG_ATA_ACPI +extern void ata_acpi_associate_sata_port (struct ata_port *ap);extern void ata_acpi_associate (struct ata_host *host);extern int ata_acpi_on_suspend (struct ata_port *ap);extern void ata_acpi_on_resume (struct ata_port *ap);extern int ata_acpi_on_devcfg (struct ata_device *adev);#else +static inline void ata_acpi_associate_sata_port (struct ata_port *ap) { }static inline void ata_acpi_associate (struct ata_host *host) { }static inline int ata_acpi_on_suspend (struct ata_port *ap) {return 0 ; }static inline void ata_acpi_on_resume (struct ata_port *ap) { }
Signature · 1 line
1.5.0.3
PATCH 5/10 libata-pmp: implement qc_defer for command switching PMP support
In reply to Tejun HeoImplement sata_pmp_qc_defer_cmd_switch() - standard qc_defer for command switching PMP support.
--- drivers/ata/libata-core.c | 1 + drivers/ata/libata-pmp.c | 30 ++++++++++++++++++++++++++++++ include/linux/libata.h | 1 + 3 files changed, 32 insertions(+), 0 deletions(-)
drivers/ata/libata-core.c +1 -0
@@ -7294,6 +7294,7 @@ EXPORT_SYMBOL_GPL(ata_pci_default_filter);EXPORT_SYMBOL_GPL (ata_pci_clear_simplex);#endif /* CONFIG_PCI */ +EXPORT_SYMBOL_GPL (sata_pmp_qc_defer_cmd_switch);EXPORT_SYMBOL_GPL (sata_pmp_read_init_tf);EXPORT_SYMBOL_GPL (sata_pmp_read_val);EXPORT_SYMBOL_GPL (sata_pmp_write_init_tf);
drivers/ata/libata-pmp.c +30 -0
@@ -70,6 +70,36 @@ static int sata_pmp_write(struct ata_link *link, int reg, u32 val) }/** +* sata_pmp_qc_defer_cmd_switch - qc_defer for command switching PMP +* @qc: ATA command in question +* +* A host which has command switching PMP support cannot issue +* commands to multiple links simultaneously. +* +* LOCKING: +* spin_lock_irqsave(host lock) +* +* RETURNS: +* ATA_DEFER_* if deferring is needed, 0 otherwise. +*/ +int sata_pmp_qc_defer_cmd_switch (struct ata_queued_cmd *qc) +{ +struct ata_link *link = qc->dev->link; +struct ata_port *ap = link->ap; + +if (ap->excl_link ==NULL || ap->excl_link == link) { +if (ap->nr_active_links ==0 ||ata_link_active (link)) { + qc->flags |=ATA_QCFLAG_CLEAR_EXCL ; +return ata_std_qc_defer (qc); + } + + ap->excl_link = link; + } + +return ATA_DEFER_PORT ; +} + +/** * sata_pmp_read_init_tf - initialize TF for PMP read * @tf: taskfile to initialize * @dev: PMP dev
include/linux/libata.h +1 -0
@@ -946,6 +946,7 @@ extern unsigned long ata_pci_default_filter(struct ata_device *, unsigned long);/* * PMP */ +extern int sata_pmp_qc_defer_cmd_switch (struct ata_queued_cmd *qc);extern void sata_pmp_read_init_tf (struct ata_taskfile *tf,struct ata_device *dev,int pmp,int reg);extern u32 sata_pmp_read_val (const struct ata_taskfile *tf);
Signature · 1 line
1.5.0.3
PATCH 6/10 sata_sil24: implement PMP support
In reply to Tejun HeoImplement PMP support. sil24 supports full FIS-switching. However, it has a PMP DMA CS errata which requires port-wide resetting if commands are outstanding to three or more devices when an error occurs on one of them.
ATAPI commands often result in CHECK SENSE and it's crucial to not reset them before fetching sense data. Unfortunately, ATAPI CHECK SENSE causes a lot of problem if command is outstanding to any other device usually resulting in port-wide reset. So, sata_sil24 implements sil24_qc_defer() which guarantees ATAPI command is run by itself.
--- drivers/ata/sata_sil24.c | 227 +++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 206 insertions(+), 21 deletions(-)
drivers/ata/sata_sil24.c +206 -21
@@ -30,7 +30,7 @@#include <linux/libata.h> #define DRV_NAME "sata_sil24" -#define DRV_VERSION "1.0" +#define DRV_VERSION "1.1"/* * Port request block (PRB) 32 bytes
@@ -238,7 +238,7 @@ enum {SIL24_COMMON_FLAGS =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY |ATA_FLAG_MMIO |ATA_FLAG_PIO_DMA |ATA_FLAG_NCQ |ATA_FLAG_ACPI_SATA | -ATA_FLAG_AN , +ATA_FLAG_AN |ATA_FLAG_PMP ,SIL24_COMMON_LFLAGS =ATA_LFLAG_SKIP_D2H_BSY , SIL24_FLAG_PCIX_IRQ_WOC = (1 << 24), /* IRQ loss errata on PCI-X */
@@ -330,9 +330,14 @@ static u8 sil24_check_status(struct ata_port *ap);static int sil24_scr_read (struct ata_port *ap,unsigned sc_reg,u32 *val);static int sil24_scr_write (struct ata_port *ap,unsigned sc_reg,u32 val);static void sil24_tf_read (struct ata_port *ap,struct ata_taskfile *tf); +static int sil24_qc_defer (struct ata_queued_cmd *qc);static void sil24_qc_prep (struct ata_queued_cmd *qc);static unsigned int sil24_qc_issue (struct ata_queued_cmd *qc);static void sil24_irq_clear (struct ata_port *ap); +static void sil24_pmp_attach (struct ata_port *ap); +static void sil24_pmp_detach (struct ata_port *ap); +static int sil24_pmp_read (struct ata_device *dev,int pmp,int reg,u32 *r_val); +static int sil24_pmp_write (struct ata_device *dev,int pmp,int reg,u32 val);static void sil24_freeze (struct ata_port *ap);static void sil24_thaw (struct ata_port *ap);static void sil24_error_handler (struct ata_port *ap);
@@ -341,6 +346,7 @@ static int sil24_port_start(struct ata_port *ap);static int sil24_init_one (struct pci_dev *pdev,const struct pci_device_id *ent);#ifdef CONFIG_PM static int sil24_pci_device_resume (struct pci_dev *pdev); +static int sil24_port_resume (struct ata_port *ap);#endif static const struct pci_device_id sil24_pci_tbl[] = {
@@ -393,7 +399,7 @@ static const struct ata_port_operations sil24_ops = { .tf_read = sil24_tf_read, - .qc_defer = ata_std_qc_defer, + .qc_defer = sil24_qc_defer, .qc_prep = sil24_qc_prep, .qc_issue = sil24_qc_issue,
@@ -402,12 +408,21 @@ static const struct ata_port_operations sil24_ops = { .scr_read = sil24_scr_read, .scr_write = sil24_scr_write, + .pmp_attach = sil24_pmp_attach, + .pmp_detach = sil24_pmp_detach, + .pmp_read = sil24_pmp_read, + .pmp_write = sil24_pmp_write, + .freeze = sil24_freeze, .thaw = sil24_thaw, .error_handler = sil24_error_handler, .post_internal_cmd = sil24_post_internal_cmd, .port_start = sil24_port_start, + +#ifdef CONFIG_PM + .port_resume = sil24_port_resume, +#endif };/*
@@ -521,11 +536,40 @@ static void sil24_tf_read(struct ata_port *ap, struct ata_taskfile *tf) *tf = pp->tf; } +static void sil24_config_pmp (struct ata_port *ap,int attached) +{ +void __iomem *port = ap->ioaddr.cmd_addr; + +if (attached) +writel (PORT_CS_PMP_EN , port +PORT_CTRL_STAT ); +else +writel (PORT_CS_PMP_EN , port +PORT_CTRL_CLR ); +} + +static void sil24_clear_pmp (struct ata_port *ap) +{ +void __iomem *port = ap->ioaddr.cmd_addr; +int i; + +writel (PORT_CS_PMP_RESUME , port +PORT_CTRL_CLR ); + +for (i =0 ; i <SATA_PMP_MAX_PORTS ; i++) { +void __iomem *pmp_base = port +PORT_PMP + i *PORT_PMP_SIZE ; + +writel (0 , pmp_base +PORT_PMP_STATUS ); +writel (0 , pmp_base +PORT_PMP_QACTIVE ); + } +} +static int sil24_init_port (struct ata_port *ap) {void __iomem *port = ap->ioaddr.cmd_addr;u32 tmp; +/* clear PMP error status */ +if (ap->nr_pmp_links) +sil24_clear_pmp (ap); +writel (PORT_CS_INIT , port +PORT_CTRL_STAT );ata_wait_register (port +PORT_CTRL_STAT ,PORT_CS_INIT ,PORT_CS_INIT ,10 ,100 );
@@ -640,7 +684,7 @@ static int sil24_do_softreset(struct ata_link *link, unsigned int *class,static int sil24_softreset (struct ata_link *link,unsigned int *class,unsigned long deadline) { -return sil24_do_softreset (link, class,0 , deadline); +return sil24_do_softreset (link, class,SATA_PMP_CTRL_PORT , deadline); } static int sil24_hardreset(struct ata_link *link, unsigned int *class,
@@ -708,6 +752,38 @@ static inline void sil24_fill_sg(struct ata_queued_cmd *qc, } } +static int sil24_qc_defer (struct ata_queued_cmd *qc) +{ +struct ata_link *link = qc->dev->link; +struct ata_port *ap = link->ap; +u8 prot = qc->tf.protocol; +int is_atapi = (prot ==ATA_PROT_ATAPI || + prot ==ATA_PROT_ATAPI_NODATA || + prot ==ATA_PROT_ATAPI_DMA ); + +/* ATAPI commands completing with CHECK_SENSE cause various +* weird problems if other commands are active. PMP DMA CS +* errata doesn't cover all and HSM violation occurs even with +* only one other device active. Always run an ATAPI command +* by itself. +*/ +if (unlikely (ap->excl_link)) { +if (link == ap->excl_link) { +if (ap->nr_active_links) +return ATA_DEFER_PORT ; + qc->flags |=ATA_QCFLAG_CLEAR_EXCL ; + }else +return ATA_DEFER_PORT ; + }else if (unlikely (is_atapi)) { + ap->excl_link = link; +if (ap->nr_active_links) +return ATA_DEFER_PORT ; + qc->flags |=ATA_QCFLAG_CLEAR_EXCL ; + } + +return ata_std_qc_defer (qc); +} +static void sil24_qc_prep (struct ata_queued_cmd *qc) {struct ata_port *ap = qc->ap;
@@ -751,7 +827,7 @@ static void sil24_qc_prep(struct ata_queued_cmd *qc) } prb->ctrl =cpu_to_le16 (ctrl); -ata_tf_to_fis (&qc->tf,0 ,1 , prb->fis); +ata_tf_to_fis (&qc->tf, qc->dev->link->pmp,1 , prb->fis);if (qc->flags &ATA_QCFLAG_DMAMAP )sil24_fill_sg (qc, sge);
@@ -780,6 +856,65 @@ static void sil24_irq_clear(struct ata_port *ap)/* unused */ } +static void sil24_pmp_attach (struct ata_port *ap) +{ +sil24_config_pmp (ap,1 ); +sil24_init_port (ap); +} + +static void sil24_pmp_detach (struct ata_port *ap) +{ +sil24_init_port (ap); +sil24_config_pmp (ap,0 ); +} + +static int sil24_pmp_read (struct ata_device *dev,int pmp,int reg,u32 *r_val) +{ +struct ata_port *ap = dev->link->ap; +struct ata_taskfile tf; +int rc; + +sata_pmp_read_init_tf (&tf, dev, pmp, reg); + rc =sil24_exec_polled_cmd (ap,SATA_PMP_CTRL_PORT , &tf,1 ,0 , +SATA_PMP_SCR_TIMEOUT ); +if (rc ==0 ) { +sil24_read_tf (ap,0 , &tf); + *r_val =sata_pmp_read_val (&tf); + } +return rc; +} + +static int sil24_pmp_write (struct ata_device *dev,int pmp,int reg,u32 val) +{ +struct ata_port *ap = dev->link->ap; +struct ata_taskfile tf; + +sata_pmp_write_init_tf (&tf, dev, pmp, reg, val); +return sil24_exec_polled_cmd (ap,SATA_PMP_CTRL_PORT , &tf,1 ,0 , +SATA_PMP_SCR_TIMEOUT ); +} + +static int sil24_pmp_softreset (struct ata_link *link,unsigned int *class, +unsigned long deadline) +{ +return sil24_do_softreset (link, class, link->pmp, deadline); +} + +static int sil24_pmp_hardreset (struct ata_link *link,unsigned int *class, +unsigned long deadline) +{ +int rc; + + rc =sil24_init_port (link->ap); +if (rc) { +ata_link_printk (link,KERN_ERR , +"hardreset failed (port not ready)\n" ); +return rc; + } + +return sata_pmp_std_hardreset (link, class, deadline); +} +static void sil24_freeze (struct ata_port *ap) {void __iomem *port = ap->ioaddr.cmd_addr;
@@ -807,8 +942,10 @@ static void sil24_error_intr(struct ata_port *ap) {void __iomem *port = ap->ioaddr.cmd_addr;struct sil24_port_priv *pp = ap->private_data; -struct ata_eh_info *ehi = &ap->link.eh_info; -int freeze =0 ; +struct ata_queued_cmd *qc =NULL ; +struct ata_link *link; +struct ata_eh_info *ehi; +int abort =0 , freeze =0 ;u32 irq_stat;/* on error, we need to clear IRQ explicitly */
@@ -816,6 +953,8 @@ static void sil24_error_intr(struct ata_port *ap)writel (irq_stat, port +PORT_IRQ_STAT );/* first, analyze and record host port events */ + link = &ap->link; + ehi = &link->eh_info;ata_ehi_clear_desc (ehi);ata_ehi_push_desc (ehi,"irq_stat 0x%08x" , irq_stat);
@@ -844,8 +983,43 @@ static void sil24_error_intr(struct ata_port *ap)if (irq_stat &PORT_IRQ_ERROR ) {struct sil24_cerr_info *ci =NULL ;unsigned int err_mask =0 , action =0 ; -struct ata_queued_cmd *qc; -u32 cerr; +u32 context, cerr; +int pmp; + + abort =1 ; + +/* DMA Context Switch Failure in Port Multiplier Mode +* errata. If we have active commands to 3 or more +* devices, any error condition on active devices can +* corrupt DMA context switching. +*/ +if (ap->nr_active_links >=3 ) { + ehi->err_mask |=AC_ERR_OTHER ; + ehi->action |=ATA_EH_HARDRESET ; +ata_ehi_push_desc (ehi,"PMP DMA CS errata" ); + freeze =1 ; + } + +/* find out the offending link and qc */ +if (ap->nr_pmp_links) { + context =readl (port +PORT_CONTEXT ); + pmp = (context >>5 ) &0xf ; + +if (pmp < ap->nr_pmp_links) { + link = &ap->pmp_link[pmp]; + ehi = &link->eh_info; + qc =ata_qc_from_tag (ap, link->active_tag); + +ata_ehi_clear_desc (ehi); +ata_ehi_push_desc (ehi,"irq_stat 0x%08x" , + irq_stat); + }else { + err_mask |=AC_ERR_HSM ; + action |=ATA_EH_HARDRESET ; + freeze =1 ; + } + }else + qc =ata_qc_from_tag (ap, link->active_tag);/* analyze CMD_ERR */ cerr =readl (port +PORT_CMD_ERR );
@@ -864,7 +1038,6 @@ static void sil24_error_intr(struct ata_port *ap) }/* record error info */ - qc =ata_qc_from_tag (ap, ap->link.active_tag);if (qc) {sil24_read_tf (ap, qc->tag, &pp->tf); qc->err_mask |= err_mask;
@@ -872,13 +1045,21 @@ static void sil24_error_intr(struct ata_port *ap) ehi->err_mask |= err_mask; ehi->action |= action; + +/* if PMP, resume */ +if (ap->nr_pmp_links) +writel (PORT_CS_PMP_RESUME , port +PORT_CTRL_STAT ); }/* freeze or abort */ if (freeze)ata_port_freeze (ap); -else -ata_port_abort (ap); +else if (abort) { +if (qc) +ata_link_abort (qc->dev->link); +else +ata_port_abort (ap); + } }static void sil24_finish_qc (struct ata_queued_cmd *qc)
@@ -971,16 +1152,14 @@ static irqreturn_t sil24_interrupt(int irq, void *dev_instance)static void sil24_error_handler (struct ata_port *ap) { -struct ata_eh_context *ehc = &ap->link.eh_context; - -if (sil24_init_port (ap)) { +if (sil24_init_port (ap))ata_eh_freeze_port (ap); - ehc->i.action |=ATA_EH_HARDRESET ; - }/* perform recovery */ -ata_do_eh (ap, ata_std_prereset, sil24_softreset, sil24_hardreset, - ata_std_postreset); +sata_pmp_do_eh (ap, ata_std_prereset, sil24_softreset, sil24_hardreset, + ata_std_postreset, sata_pmp_std_prereset, + sil24_pmp_softreset, sil24_pmp_hardreset, + sata_pmp_std_postreset); }static void sil24_post_internal_cmd (struct ata_queued_cmd *qc)
@@ -988,8 +1167,8 @@ static void sil24_post_internal_cmd(struct ata_queued_cmd *qc)struct ata_port *ap = qc->ap;/* make DMA engine forget about the failed command */ -if (qc->flags &ATA_QCFLAG_FAILED ) -sil24_init_port (ap); +if ((qc->flags &ATA_QCFLAG_FAILED ) &&sil24_init_port (ap)) +ata_eh_freeze_port (ap); }static int sil24_port_start (struct ata_port *ap)
@@ -1190,6 +1369,12 @@ static int sil24_pci_device_resume(struct pci_dev *pdev) return 0; } + +static int sil24_port_resume (struct ata_port *ap) +{ +sil24_config_pmp (ap, ap->nr_pmp_links); +return 0 ; +}#endif static int __init sil24_init (void )
Signature · 1 line
1.5.0.3
PATCH 7/10 sata_sil24: implement PORT_RST
In reply to Tejun HeoAs DEV_RST (hardreset) sometimes fail to recover the controller (especially after PMP DMA CS errata). In such cases, perform PORT_RST prior to DEV_RST.
--- drivers/ata/sata_sil24.c | 93 ++++++++++++++++++++++++++++++++++----------- 1 files changed, 70 insertions(+), 23 deletions(-)
drivers/ata/sata_sil24.c +70 -23
@@ -323,6 +323,7 @@ struct sil24_port_priv {union sil24_cmd_block *cmd_block;/* 32 cmd blocks */ dma_addr_t cmd_block_dma;/* DMA base addr for them */ struct ata_taskfile tf;/* Cached taskfile registers */ +int do_port_rst; };static void sil24_dev_config (struct ata_device *dev);
@@ -536,6 +537,31 @@ static void sil24_tf_read(struct ata_port *ap, struct ata_taskfile *tf) *tf = pp->tf; } +static void sil24_config_port (struct ata_port *ap) +{ +void __iomem *port = ap->ioaddr.cmd_addr; + +/* configure IRQ WoC */ +if (ap->flags &SIL24_FLAG_PCIX_IRQ_WOC ) +writel (PORT_CS_IRQ_WOC , port +PORT_CTRL_STAT ); +else +writel (PORT_CS_IRQ_WOC , port +PORT_CTRL_CLR ); + +/* zero error counters. */ +writel (0x8000 , port +PORT_DECODE_ERR_THRESH ); +writel (0x8000 , port +PORT_CRC_ERR_THRESH ); +writel (0x8000 , port +PORT_HSHK_ERR_THRESH ); +writel (0x0000 , port +PORT_DECODE_ERR_CNT ); +writel (0x0000 , port +PORT_CRC_ERR_CNT ); +writel (0x0000 , port +PORT_HSHK_ERR_CNT ); + +/* always use 64bit activation */ +writel (PORT_CS_32BIT_ACTV , port +PORT_CTRL_CLR ); + +/* clear port multiplier enable and resume bits */ +writel (PORT_CS_PMP_EN |PORT_CS_PMP_RESUME , port +PORT_CTRL_CLR ); +} +static void sil24_config_pmp (struct ata_port *ap,int attached) {void __iomem *port = ap->ioaddr.cmd_addr;
@@ -564,6 +590,7 @@ static void sil24_clear_pmp(struct ata_port *ap)static int sil24_init_port (struct ata_port *ap) {void __iomem *port = ap->ioaddr.cmd_addr; +struct sil24_port_priv *pp = ap->private_data;u32 tmp;/* clear PMP error status */
@@ -576,8 +603,12 @@ static int sil24_init_port(struct ata_port *ap) tmp =ata_wait_register (port +PORT_CTRL_STAT ,PORT_CS_RDY ,0 ,10 ,100 ); -if ((tmp & (PORT_CS_INIT |PORT_CS_RDY )) !=PORT_CS_RDY ) +if ((tmp & (PORT_CS_INIT |PORT_CS_RDY )) !=PORT_CS_RDY ) { + pp->do_port_rst =1 ; + ap->link.eh_context.i.action |=ATA_EH_HARDRESET ;return -EIO ; + } +return 0 ; }
@@ -692,10 +723,34 @@ static int sil24_hardreset(struct ata_link *link, unsigned int *class, {struct ata_port *ap = link->ap;void __iomem *port = ap->ioaddr.cmd_addr; +struct sil24_port_priv *pp = ap->private_data; +int did_port_rst =0 ;const char *reason;int tout_msec, rc;u32 tmp; + retry: +/* Sometimes, DEV_RST is not enough to recover the controller. +* This happens often after PM DMA CS errata. +*/ +if (pp->do_port_rst) { +ata_port_printk (ap,KERN_WARNING ,"controller in dubious " +"state, performing PORT_RST\n" ); + +writel (PORT_CS_PORT_RST , port +PORT_CTRL_STAT ); +msleep (10 ); +writel (PORT_CS_PORT_RST , port +PORT_CTRL_CLR ); +ata_wait_register (port +PORT_CTRL_STAT ,PORT_CS_RDY ,0 , +10 ,5000 ); + +/* restore port configuration */ +sil24_config_port (ap); +sil24_config_pmp (ap, ap->nr_pmp_links); + + pp->do_port_rst =0 ; + did_port_rst =1 ; + } +/* sil24 does the right thing(tm) without any protection */ sata_set_spd (link);
@@ -732,6 +787,11 @@ static int sil24_hardreset(struct ata_link *link, unsigned int *class,return -EAGAIN ; err: +if (!did_port_rst) { + pp->do_port_rst =1 ; + goto retry; + } +ata_link_printk (link,KERN_ERR ,"hardreset failed (%s)\n" , reason);return -EIO ; }
@@ -997,6 +1057,7 @@ static void sil24_error_intr(struct ata_port *ap) ehi->err_mask |=AC_ERR_OTHER ; ehi->action |=ATA_EH_HARDRESET ;ata_ehi_push_desc (ehi,"PMP DMA CS errata" ); + pp->do_port_rst =1 ; freeze =1 ; }
@@ -1152,6 +1213,8 @@ static irqreturn_t sil24_interrupt(int irq, void *dev_instance)static void sil24_error_handler (struct ata_port *ap) { +struct sil24_port_priv *pp = ap->private_data; +if (sil24_init_port (ap))ata_eh_freeze_port (ap);
@@ -1160,6 +1223,8 @@ static void sil24_error_handler(struct ata_port *ap) ata_std_postreset, sata_pmp_std_prereset, sil24_pmp_softreset, sil24_pmp_hardreset, sata_pmp_std_postreset); + + pp->do_port_rst =0 ; }static void sil24_post_internal_cmd (struct ata_queued_cmd *qc)
@@ -1206,7 +1271,6 @@ static int sil24_port_start(struct ata_port *ap)static void sil24_init_controller (struct ata_host *host) {void __iomem *host_base = host->iomap[SIL24_HOST_BAR ]; -void __iomem *port_base = host->iomap[SIL24_PORT_BAR ];u32 tmp;int i;
@@ -1218,7 +1282,8 @@ static void sil24_init_controller(struct ata_host *host)/* init ports */ for (i =0 ; i < host->n_ports; i++) { -void __iomem *port = port_base + i *PORT_REGS_SIZE ; +struct ata_port *ap = host->ports[i]; +void __iomem *port = ap->ioaddr.cmd_addr;/* Initial PHY setting */ writel (0x20c , port +PORT_PHY_CFG );
@@ -1235,26 +1300,8 @@ static void sil24_init_controller(struct ata_host *host) "failed to clear port RST\n"); } -/* Configure IRQ WoC */ -if (host->ports[0 ]->flags &SIL24_FLAG_PCIX_IRQ_WOC ) -writel (PORT_CS_IRQ_WOC , port +PORT_CTRL_STAT ); -else -writel (PORT_CS_IRQ_WOC , port +PORT_CTRL_CLR ); - -/* Zero error counters. */ -writel (0x8000 , port +PORT_DECODE_ERR_THRESH ); -writel (0x8000 , port +PORT_CRC_ERR_THRESH ); -writel (0x8000 , port +PORT_HSHK_ERR_THRESH ); -writel (0x0000 , port +PORT_DECODE_ERR_CNT ); -writel (0x0000 , port +PORT_CRC_ERR_CNT ); -writel (0x0000 , port +PORT_HSHK_ERR_CNT ); - -/* Always use 64bit activation */ -writel (PORT_CS_32BIT_ACTV , port +PORT_CTRL_CLR ); - -/* Clear port multiplier enable and resume bits */ -writel (PORT_CS_PMP_EN |PORT_CS_PMP_RESUME , - port +PORT_CTRL_CLR ); +/* configure port */ +sil24_config_port (ap); }/* Turn on interrupts */
Signature · 1 line
1.5.0.3
PATCH 8/10 ahci: implement PMP support
In reply to Tejun HeoImplement AHCI PMP support. ahci only supports command based switching. Also, for some reason, NCQ over PMP doesn't work now. Other than that, everything works.
Tested on ICH9R, JMB360/363 + SIMG3726, 4726 and 5744.
--- drivers/ata/ahci.c | 220 +++++++++++++++++++++++++++++++++++++++++----------- 1 files changed, 174 insertions(+), 46 deletions(-)
drivers/ata/ahci.c +174 -46
@@ -46,7 +46,7 @@#include <linux/libata.h> #define DRV_NAME "ahci" -#define DRV_VERSION "2.3" +#define DRV_VERSION "3.0"enum {
@@ -96,6 +96,7 @@ enum {/* HOST_CAP bits */ HOST_CAP_SSC = (1 <<14 ),/* Slumber capable */ +HOST_CAP_PMP = (1 <<17 ),/* Port Multiplier support */ HOST_CAP_CLO = (1 <<24 ),/* Command List Override support */ HOST_CAP_SSS = (1 <<27 ),/* Staggered Spin-up */ HOST_CAP_SNTF = (1 << 29), /* SNotification register */
@@ -143,7 +144,8 @@ enum {PORT_IRQ_IF_ERR |PORT_IRQ_CONNECT |PORT_IRQ_PHYRDY | -PORT_IRQ_UNK_FIS , +PORT_IRQ_UNK_FIS | +PORT_IRQ_BAD_PMP ,PORT_IRQ_ERROR =PORT_IRQ_FREEZE |PORT_IRQ_TF_ERR | PORT_IRQ_HBUS_DATA_ERR,
@@ -153,6 +155,7 @@ enum {/* PORT_CMD bits */ PORT_CMD_ATAPI = (1 <<24 ),/* Device is ATAPI */ +PORT_CMD_PMP = (1 <<17 ),/* PMP attached */ PORT_CMD_LIST_ON = (1 <<15 ),/* cmd list DMA engine running */ PORT_CMD_FIS_ON = (1 <<14 ),/* FIS DMA engine running */ PORT_CMD_FIS_RX = (1 << 4), /* Enable FIS receive DMA engine */
@@ -204,6 +207,7 @@ struct ahci_host_priv { };struct ahci_port_priv { +struct ata_link *active_link;struct ahci_cmd_hdr *cmd_slot;dma_addr_t cmd_slot_dma;void *cmd_tbl;
@@ -229,6 +233,10 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc);static u8 ahci_check_status (struct ata_port *ap);static void ahci_freeze (struct ata_port *ap);static void ahci_thaw (struct ata_port *ap); +static void ahci_pmp_attach (struct ata_port *ap); +static void ahci_pmp_detach (struct ata_port *ap); +static int ahci_pmp_read (struct ata_device *dev,int pmp,int reg,u32 *r_val); +static int ahci_pmp_write (struct ata_device *dev,int pmp,int reg,u32 val);static void ahci_error_handler (struct ata_port *ap);static void ahci_vt8251_error_handler (struct ata_port *ap);static void ahci_post_internal_cmd (struct ata_queued_cmd *qc);
@@ -268,7 +276,7 @@ static const struct ata_port_operations ahci_ops = { .tf_read = ahci_tf_read, - .qc_defer = ata_std_qc_defer, + .qc_defer = sata_pmp_qc_defer_cmd_switch, .qc_prep = ahci_qc_prep, .qc_issue = ahci_qc_issue,
@@ -283,6 +291,11 @@ static const struct ata_port_operations ahci_ops = { .error_handler = ahci_error_handler, .post_internal_cmd = ahci_post_internal_cmd, + .pmp_attach = ahci_pmp_attach, + .pmp_detach = ahci_pmp_detach, + .pmp_read = ahci_pmp_read, + .pmp_write = ahci_pmp_write, + #ifdef CONFIG_PM .port_suspend = ahci_port_suspend, .port_resume = ahci_port_resume,
@@ -299,7 +312,7 @@ static const struct ata_port_operations ahci_vt8251_ops = { .tf_read = ahci_tf_read, - .qc_defer = ata_std_qc_defer, + .qc_defer = sata_pmp_qc_defer_cmd_switch, .qc_prep = ahci_qc_prep, .qc_issue = ahci_qc_issue,
@@ -314,6 +327,11 @@ static const struct ata_port_operations ahci_vt8251_ops = { .error_handler = ahci_vt8251_error_handler, .post_internal_cmd = ahci_post_internal_cmd, + .pmp_attach = ahci_pmp_attach, + .pmp_detach = ahci_pmp_detach, + .pmp_read = ahci_pmp_read, + .pmp_write = ahci_pmp_write, + #ifdef CONFIG_PM .port_suspend = ahci_port_suspend, .port_resume = ahci_port_resume,
@@ -1108,7 +1126,12 @@ static int ahci_do_softreset(struct ata_link *link, unsigned int *class,static int ahci_softreset (struct ata_link *link,unsigned int *class,unsigned long deadline) { -return ahci_do_softreset (link, class,0 , deadline); +int pmp =0 ; + +if (link->ap->flags &ATA_FLAG_PMP ) + pmp =SATA_PMP_CTRL_PORT ; + +return ahci_do_softreset (link, class, pmp, deadline); } static int ahci_hardreset(struct ata_link *link, unsigned int *class,
@@ -1135,7 +1158,7 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,if (rc ==0 &&ata_link_online (link)) *class =ahci_dev_classify (ap); -if (*class ==ATA_DEV_UNKNOWN ) +if (rc != -EAGAIN && *class ==ATA_DEV_UNKNOWN ) *class =ATA_DEV_NONE ;DPRINTK ("EXIT, rc=%d, class=%u\n" , rc, *class);
@@ -1190,6 +1213,12 @@ static void ahci_postreset(struct ata_link *link, unsigned int *class) } } +static int ahci_pmp_softreset (struct ata_link *link,unsigned int *class, +unsigned long deadline) +{ +return ahci_do_softreset (link, class, link->pmp, deadline); +} +static u8 ahci_check_status (struct ata_port *ap) {void __iomem *mmio = ap->ioaddr.cmd_addr;
@@ -1248,7 +1277,7 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc)*/ cmd_tbl = pp->cmd_tbl + qc->tag *AHCI_CMD_TBL_SZ ; -ata_tf_to_fis (&qc->tf,0 ,1 , cmd_tbl); +ata_tf_to_fis (&qc->tf, qc->dev->link->pmp,1 , cmd_tbl);if (is_atapi) {memset (cmd_tbl +AHCI_CMD_TBL_CDB ,0 ,32 );memcpy (cmd_tbl +AHCI_CMD_TBL_CDB , qc->cdb, qc->dev->cdb_len);
@@ -1261,7 +1290,7 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc)/* * Fill in command slot information. */ - opts = cmd_fis_len | n_elem <<16 ; + opts = cmd_fis_len | n_elem <<16 | (qc->dev->link->pmp <<12 );if (qc->tf.flags &ATA_TFLAG_WRITE ) opts |=AHCI_CMD_WRITE ;if (is_atapi)
@@ -1273,65 +1302,85 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc)static void ahci_error_intr (struct ata_port *ap,u32 irq_stat) {struct ahci_port_priv *pp = ap->private_data; -struct ata_eh_info *ehi = &ap->link.eh_info; -unsigned int err_mask =0 , action =0 ; -struct ata_queued_cmd *qc; +struct ata_eh_info *host_ehi = &ap->link.eh_info; +struct ata_link *link =NULL ; +struct ata_queued_cmd *active_qc; +struct ata_eh_info *active_ehi;u32 serror; -ata_ehi_clear_desc (ehi); +/* determine active link */ +ata_port_for_each_link (link, ap) +if (ata_link_active (link)) +break ; +if (!link) + link = &ap->link; + + active_qc =ata_qc_from_tag (ap, link->active_tag); + active_ehi = &link->eh_info; + +/* record irq stat */ +ata_ehi_clear_desc (host_ehi); +ata_ehi_push_desc (host_ehi,"irq_stat 0x%08x" , irq_stat);/* AHCI needs SError cleared; otherwise, it might lock up */ ahci_scr_read (ap,SCR_ERROR , &serror);ahci_scr_write (ap,SCR_ERROR , serror); - -/* analyze @irq_stat */ -ata_ehi_push_desc (ehi,"irq_stat 0x%08x" , irq_stat); + host_ehi->serror |= serror;/* some controllers set IRQ_IF_ERR on device errors, ignore it */ if (ap->flags &AHCI_FLAG_IGN_IRQ_IF_ERR ) irq_stat &= ~PORT_IRQ_IF_ERR ;if (irq_stat &PORT_IRQ_TF_ERR ) { - err_mask |=AC_ERR_DEV ; +/* If qc is active, charge it; otherwise, the active +* link. There's no active qc on NCQ errors. It will +* be determined by EH by reading log page 10h. +*/ +if (active_qc) + active_qc->err_mask |=AC_ERR_DEV ; +else + active_ehi->err_mask |=AC_ERR_DEV ; +if (ap->flags &AHCI_FLAG_IGN_SERR_INTERNAL ) - serror &= ~SERR_INTERNAL ; + host_ehi->serror &= ~SERR_INTERNAL ; + } + +if (irq_stat &PORT_IRQ_UNK_FIS ) { +u32 *unk = (u32 *)(pp->rx_fis +RX_FIS_UNK ); + + active_ehi->err_mask |=AC_ERR_HSM ; + active_ehi->action |=ATA_EH_SOFTRESET ; +ata_ehi_push_desc (active_ehi, +"unknown FIS %08x %08x %08x %08x" , + unk[0 ], unk[1 ], unk[2 ], unk[3 ]); + } + +if (ap->nr_pmp_links && (irq_stat &PORT_IRQ_BAD_PMP )) { + active_ehi->err_mask |=AC_ERR_HSM ; + active_ehi->action |=ATA_EH_SOFTRESET ; +ata_ehi_push_desc (active_ehi,"incorrect PMP" ); }if (irq_stat & (PORT_IRQ_HBUS_ERR |PORT_IRQ_HBUS_DATA_ERR )) { - err_mask |=AC_ERR_HOST_BUS ; - action |=ATA_EH_SOFTRESET ; + host_ehi->err_mask |=AC_ERR_HOST_BUS ; + host_ehi->action |=ATA_EH_SOFTRESET ; +ata_ehi_push_desc (host_ehi,"host bus error" ); }if (irq_stat &PORT_IRQ_IF_ERR ) { - err_mask |=AC_ERR_ATA_BUS ; - action |=ATA_EH_SOFTRESET ; -ata_ehi_push_desc (ehi,"interface fatal error" ); + host_ehi->err_mask |=AC_ERR_ATA_BUS ; + host_ehi->action |=ATA_EH_SOFTRESET ; +ata_ehi_push_desc (host_ehi,"interface fatal error" ); }if (irq_stat & (PORT_IRQ_CONNECT |PORT_IRQ_PHYRDY )) { -ata_ehi_hotplugged (ehi); -ata_ehi_push_desc (ehi,"%s" , irq_stat &PORT_IRQ_CONNECT ? +ata_ehi_hotplugged (host_ehi); +ata_ehi_push_desc (host_ehi,"%s" , + irq_stat &PORT_IRQ_CONNECT ?"connection status changed" :"PHY RDY changed" ); } -if (irq_stat &PORT_IRQ_UNK_FIS ) { -u32 *unk = (u32 *)(pp->rx_fis +RX_FIS_UNK ); - - err_mask |=AC_ERR_HSM ; - action |=ATA_EH_SOFTRESET ; -ata_ehi_push_desc (ehi,"unknown FIS %08x %08x %08x %08x" , - unk[0 ], unk[1 ], unk[2 ], unk[3 ]); - } -/* okay, let's hand over to EH */ - ehi->serror |= serror; - ehi->action |= action; - - qc =ata_qc_from_tag (ap, ap->link.active_tag); -if (qc) - qc->err_mask |= err_mask; -else - ehi->err_mask |= err_mask;if (irq_stat &PORT_IRQ_FREEZE )ata_port_freeze (ap);
@@ -1369,7 +1418,8 @@ static void ahci_port_intr(struct ata_port *ap)sata_async_notification (ap); } -if (ap->link.sactive) +/* pp->active_link is valid iff any command is in flight */ +if (ap->qc_active && pp->active_link->sactive) qc_active =readl (port_mmio +PORT_SCR_ACT );else qc_active =readl (port_mmio +PORT_CMD_ISSUE );
@@ -1507,6 +1557,13 @@ static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc) {struct ata_port *ap = qc->ap;void __iomem *port_mmio =ahci_port_base (ap); +struct ahci_port_priv *pp = ap->private_data; + +/* Keep track of the currently active link. It will be used +* in completion path to determine whether NCQ phase is in +* progress. +*/ + pp->active_link = qc->dev->link;if (qc->tf.protocol ==ATA_PROT_NCQ )writel (1 << qc->tag, port_mmio +PORT_SCR_ACT );
@@ -1536,8 +1593,11 @@ static void ahci_thaw(struct ata_port *ap)writel (tmp, port_mmio +PORT_IRQ_STAT );writel (1 << ap->port_no, mmio +HOST_IRQ_STAT ); -/* turn IRQ back on */ -writel (pp->intr_mask, port_mmio +PORT_IRQ_MASK ); +/* turn IRQ back on, ignore BAD_PMP if PMP isn't attached */ + tmp = pp->intr_mask; +if (!ap->nr_pmp_links) + tmp &= ~PORT_IRQ_BAD_PMP ; +writel (tmp, port_mmio +PORT_IRQ_MASK ); }static void ahci_error_handler (struct ata_port *ap)
@@ -1549,8 +1609,10 @@ static void ahci_error_handler(struct ata_port *ap) }/* perform recovery */ -ata_do_eh (ap, ata_std_prereset, ahci_softreset, ahci_hardreset, - ahci_postreset); +sata_pmp_do_eh (ap, ata_std_prereset, ahci_softreset, + ahci_hardreset, ahci_postreset, + sata_pmp_std_prereset, ahci_pmp_softreset, + sata_pmp_std_hardreset, sata_pmp_std_postreset); }static void ahci_vt8251_error_handler (struct ata_port *ap)
@@ -1575,11 +1637,74 @@ static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)ahci_kick_engine (ap,1 ); } +static void ahci_pmp_attach (struct ata_port *ap) +{ +void __iomem *port_mmio =ahci_port_base (ap); +u32 cmd; + + cmd =readl (port_mmio +PORT_CMD ); + cmd |=PORT_CMD_PMP ; +writel (cmd, port_mmio +PORT_CMD ); +} + +static void ahci_pmp_detach (struct ata_port *ap) +{ +void __iomem *port_mmio =ahci_port_base (ap); +struct ahci_host_priv *hpriv = ap->host->private_data; +unsigned long flags; +u32 cmd; + + cmd =readl (port_mmio +PORT_CMD ); + cmd &= ~PORT_CMD_PMP ; +writel (cmd, port_mmio +PORT_CMD ); + +if (hpriv->cap &HOST_CAP_NCQ ) { +spin_lock_irqsave (ap->lock, flags); + ap->flags |=ATA_FLAG_NCQ ; +spin_unlock_irqrestore (ap->lock, flags); + } +} + +static int ahci_pmp_read (struct ata_device *dev,int pmp,int reg,u32 *r_val) +{ +struct ata_port *ap = dev->link->ap; +struct ata_taskfile tf; +int rc; + +ahci_kick_engine (ap,0 ); + +sata_pmp_read_init_tf (&tf, dev, pmp, reg); + rc =ahci_exec_polled_cmd (ap,SATA_PMP_CTRL_PORT , &tf,1 ,0 , +SATA_PMP_SCR_TIMEOUT ); +if (rc ==0 ) { +ahci_tf_read (ap, &tf); + *r_val =sata_pmp_read_val (&tf); + } +return rc; +} + +static int ahci_pmp_write (struct ata_device *dev,int pmp,int reg,u32 val) +{ +struct ata_port *ap = dev->link->ap; +struct ata_taskfile tf; + +ahci_kick_engine (ap,0 ); + +sata_pmp_write_init_tf (&tf, dev, pmp, reg, val); +return ahci_exec_polled_cmd (ap,SATA_PMP_CTRL_PORT , &tf,1 ,0 , +SATA_PMP_SCR_TIMEOUT ); +} +static int ahci_port_resume (struct ata_port *ap) {ahci_power_up (ap);ahci_start_port (ap); +if (ap->nr_pmp_links) +ahci_pmp_attach (ap); +else +ahci_pmp_detach (ap); +return 0 ; }
@@ -1860,6 +1985,9 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)if (hpriv->cap &HOST_CAP_NCQ ) pi.flags |=ATA_FLAG_NCQ ; +if (hpriv->cap &HOST_CAP_PMP ) + pi.flags |=ATA_FLAG_PMP ; + host =ata_host_alloc_pinfo (&pdev->dev, ppi,fls (hpriv->port_map));if (!host)return -ENOMEM ;
Signature · 1 line
1.5.0.3
PATCH 9/10 ahci: move host flags over to pi.private_data
In reply to Tejun HeoPrivate pi.flags area is full and we need more private flags. Move host private flags over to pi.private_data. During initialization, these flags are copied to hpriv->flags.
--- drivers/ata/ahci.c | 63 ++++++++++++++++++++++++++++----------------------- 1 files changed, 35 insertions(+), 28 deletions(-)
drivers/ata/ahci.c +35 -28
@@ -169,14 +169,16 @@ enum {PORT_CMD_ICC_PARTIAL = (0x2 <<28 ),/* Put i/f in partial state */ PORT_CMD_ICC_SLUMBER = (0x6 <<28 ),/* Put i/f in slumber state */ +/* hpriv->flags bits */ +AHCI_HFLAG_NO_NCQ = (1 <<0 ), +AHCI_HFLAG_IGN_IRQ_IF_ERR = (1 <<1 ),/* ignore IRQ_IF_ERR */ +AHCI_HFLAG_IGN_SERR_INTERNAL = (1 <<2 ),/* ignore SERR_INTERNAL */ +AHCI_HFLAG_32BIT_ONLY = (1 <<3 ),/* force 32bit */ +AHCI_HFLAG_MV_PATA = (1 <<4 ),/* PATA port */ +AHCI_HFLAG_NO_MSI = (1 <<5 ),/* no PCI MSI */ +/* ap->flags bits */ -AHCI_FLAG_NO_NCQ = (1 <<24 ), -AHCI_FLAG_IGN_IRQ_IF_ERR = (1 <<25 ),/* ignore IRQ_IF_ERR */ -AHCI_FLAG_IGN_SERR_INTERNAL = (1 <<27 ),/* ignore SERR_INTERNAL */ -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_NO_HOTPLUG = (1 <<24 ),/* ignore PxSERR.DIAG.N */ AHCI_FLAG_COMMON =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY | ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
@@ -200,6 +202,7 @@ struct ahci_sg { };struct ahci_host_priv { +unsigned int flags;/* AHCI_HFLAG_* */ u32 cap;/* cap to use */ u32 port_map;/* port map to use */ u32 saved_cap;/* saved initial cap */
@@ -341,6 +344,8 @@ static const struct ata_port_operations ahci_vt8251_ops = { .port_stop = ahci_port_stop, }; +#define AHCI_HFLAGS (flags) .private_data = (void *)(flags) +static const struct ata_port_info ahci_port_info[] = {/* board_ahci */ {
@@ -352,7 +357,8 @@ static const struct ata_port_info ahci_port_info[] = { },/* board_ahci_vt8251 */ { - .flags = AHCI_FLAG_COMMON | AHCI_FLAG_NO_NCQ, + AHCI_HFLAGS (AHCI_HFLAG_NO_NCQ), + .flags = AHCI_FLAG_COMMON, .link_flags = AHCI_LFLAG_COMMON | ATA_LFLAG_HRST_TO_RESUME, .pio_mask = 0x1f, /* pio0-4 */ .udma_mask = ATA_UDMA6,
@@ -360,7 +366,8 @@ static const struct ata_port_info ahci_port_info[] = { },/* board_ahci_ign_iferr */ { - .flags = AHCI_FLAG_COMMON | AHCI_FLAG_IGN_IRQ_IF_ERR, + AHCI_HFLAGS (AHCI_HFLAG_IGN_IRQ_IF_ERR), + .flags = AHCI_FLAG_COMMON, .link_flags = AHCI_LFLAG_COMMON, .pio_mask = 0x1f, /* pio0-4 */ .udma_mask = ATA_UDMA6,
@@ -368,9 +375,9 @@ static const struct ata_port_info ahci_port_info[] = { },/* board_ahci_sb600 */ { - .flags = AHCI_FLAG_COMMON | -AHCI_FLAG_IGN_SERR_INTERNAL | - AHCI_FLAG_32BIT_ONLY, +AHCI_HFLAGS (AHCI_HFLAG_IGN_SERR_INTERNAL | + AHCI_HFLAG_32BIT_ONLY), + .flags = AHCI_FLAG_COMMON, .link_flags = AHCI_LFLAG_COMMON, .pio_mask = 0x1f, /* pio0-4 */ .udma_mask = ATA_UDMA6,
@@ -378,11 +385,10 @@ static const struct ata_port_info ahci_port_info[] = { },/* board_ahci_mv */ { - .sht = &ahci_sht, +AHCI_HFLAGS (AHCI_HFLAG_NO_NCQ |AHCI_HFLAG_NO_MSI | + AHCI_HFLAG_MV_PATA), .flags =ATA_FLAG_SATA |ATA_FLAG_NO_LEGACY | -ATA_FLAG_MMIO |ATA_FLAG_PIO_DMA | -AHCI_FLAG_NO_NCQ |AHCI_FLAG_NO_MSI | - AHCI_FLAG_MV_PATA, + ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA, .link_flags = AHCI_LFLAG_COMMON, .pio_mask = 0x1f, /* pio0-4 */ .udma_mask = ATA_UDMA6,
@@ -534,7 +540,6 @@ static inline void __iomem *ahci_port_base(struct ata_port *ap)/** * ahci_save_initial_config - Save and fixup initial config values * @pdev: target PCI device -* @pi: associated ATA port info * @hpriv: host private area to store config values * * Some registers containing configuration info might be setup by
@@ -548,7 +553,6 @@ static inline void __iomem *ahci_port_base(struct ata_port *ap)* None. */ static void ahci_save_initial_config (struct pci_dev *pdev, -const struct ata_port_info *pi,struct ahci_host_priv *hpriv) {void __iomem *mmio =pcim_iomap_table (pdev)[AHCI_PCI_BAR ];
@@ -562,13 +566,13 @@ static void ahci_save_initial_config(struct pci_dev *pdev, hpriv->saved_port_map = port_map =readl (mmio +HOST_PORTS_IMPL );/* some chips have errata preventing 64bit use */ -if ((cap &HOST_CAP_64 ) && (pi->flags &AHCI_FLAG_32BIT_ONLY )) { +if ((cap &HOST_CAP_64 ) && (hpriv->flags &AHCI_HFLAG_32BIT_ONLY )) {dev_printk (KERN_INFO , &pdev->dev,"controller can't do 64bit DMA, forcing 32bit\n" ); cap &= ~HOST_CAP_64 ; } -if ((cap &HOST_CAP_NCQ ) && (pi->flags &AHCI_FLAG_NO_NCQ )) { +if ((cap &HOST_CAP_NCQ ) && (hpriv->flags &AHCI_HFLAG_NO_NCQ )) {dev_printk (KERN_INFO , &pdev->dev,"controller can't do NCQ, turning off CAP_NCQ\n" ); cap &= ~HOST_CAP_NCQ ;
@@ -579,7 +583,7 @@ static void ahci_save_initial_config(struct pci_dev *pdev,* is asserted through the standard AHCI port * presence register, as bit 4 (counting from 0) */ -if (pi->flags &AHCI_FLAG_MV_PATA ) { +if (hpriv->flags &AHCI_HFLAG_MV_PATA ) { dev_printk(KERN_ERR, &pdev->dev, "MV_AHCI HACK: port_map %x -> %x\n", hpriv->port_map,
@@ -913,13 +917,14 @@ static void ahci_port_init(struct pci_dev *pdev, struct ata_port *ap,static void ahci_init_controller (struct ata_host *host) { +struct ahci_host_priv *hpriv = host->private_data;struct pci_dev *pdev =to_pci_dev (host->dev);void __iomem *mmio = host->iomap[AHCI_PCI_BAR ];int i;void __iomem *port_mmio;u32 tmp; -if (host->ports[0 ]->flags &AHCI_FLAG_MV_PATA ) { +if (hpriv->flags &AHCI_HFLAG_MV_PATA ) { port_mmio =__ahci_port_base (host,4 );writel (0 , port_mmio +PORT_IRQ_MASK );
@@ -1301,6 +1306,7 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc)static void ahci_error_intr (struct ata_port *ap,u32 irq_stat) { +struct ahci_host_priv *hpriv = ap->host->private_data;struct ahci_port_priv *pp = ap->private_data;struct ata_eh_info *host_ehi = &ap->link.eh_info;struct ata_link *link =NULL ;
@@ -1328,7 +1334,7 @@ static void ahci_error_intr(struct ata_port *ap, u32 irq_stat) host_ehi->serror |= serror;/* some controllers set IRQ_IF_ERR on device errors, ignore it */ -if (ap->flags &AHCI_FLAG_IGN_IRQ_IF_ERR ) +if (hpriv->flags &AHCI_HFLAG_IGN_IRQ_IF_ERR ) irq_stat &= ~PORT_IRQ_IF_ERR ;if (irq_stat &PORT_IRQ_TF_ERR ) {
@@ -1341,7 +1347,7 @@ static void ahci_error_intr(struct ata_port *ap, u32 irq_stat)else active_ehi->err_mask |=AC_ERR_DEV ; -if (ap->flags &AHCI_FLAG_IGN_SERR_INTERNAL ) +if (hpriv->flags &AHCI_HFLAG_IGN_SERR_INTERNAL ) host_ehi->serror &= ~SERR_INTERNAL ; }
@@ -1971,15 +1977,16 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)if (rc)return rc; -if ((pi.flags &AHCI_FLAG_NO_MSI ) ||pci_enable_msi (pdev)) -pci_intx (pdev,1 ); - hpriv =devm_kzalloc (dev,sizeof (*hpriv),GFP_KERNEL );if (!hpriv)return -ENOMEM ; + hpriv->flags |= (unsigned long )pi.private_data; + +if ((hpriv->flags &AHCI_HFLAG_NO_MSI ) ||pci_enable_msi (pdev)) +pci_intx (pdev,1 );/* save initial config */ -ahci_save_initial_config (pdev, &pi, hpriv); +ahci_save_initial_config (pdev, hpriv);/* prepare host */ if (hpriv->cap &HOST_CAP_NCQ )
Signature · 1 line
1.5.0.3
PATCH 10/10 ahci: implement AHCI_HFLAG_NO_PMP
In reply to Tejun HeoOf course some controllers lie about PMP support. Black list them.
--- drivers/ata/ahci.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)
drivers/ata/ahci.c +9 -2
@@ -176,6 +176,7 @@ enum {AHCI_HFLAG_32BIT_ONLY = (1 <<3 ),/* force 32bit */ AHCI_HFLAG_MV_PATA = (1 <<4 ),/* PATA port */ AHCI_HFLAG_NO_MSI = (1 <<5 ),/* no PCI MSI */ +AHCI_HFLAG_NO_PMP = (1 <<6 ),/* no PMP */ /* ap->flags bits */ AHCI_FLAG_NO_HOTPLUG = (1 << 24), /* ignore PxSERR.DIAG.N */
@@ -357,7 +358,7 @@ static const struct ata_port_info ahci_port_info[] = { },/* board_ahci_vt8251 */ { - AHCI_HFLAGS (AHCI_HFLAG_NO_NCQ), + AHCI_HFLAGS (AHCI_HFLAG_NO_NCQ | AHCI_HFLAG_NO_PMP), .flags = AHCI_FLAG_COMMON, .link_flags = AHCI_LFLAG_COMMON | ATA_LFLAG_HRST_TO_RESUME, .pio_mask = 0x1f, /* pio0-4 */
@@ -376,7 +377,7 @@ static const struct ata_port_info ahci_port_info[] = {/* board_ahci_sb600 */ {AHCI_HFLAGS (AHCI_HFLAG_IGN_SERR_INTERNAL | - AHCI_HFLAG_32BIT_ONLY), + AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_PMP), .flags = AHCI_FLAG_COMMON, .link_flags = AHCI_LFLAG_COMMON, .pio_mask = 0x1f, /* pio0-4 */
@@ -578,6 +579,12 @@ static void ahci_save_initial_config(struct pci_dev *pdev, cap &= ~HOST_CAP_NCQ ; } +if ((cap &&HOST_CAP_PMP ) && (hpriv->flags &AHCI_HFLAG_NO_PMP )) { +dev_printk (KERN_INFO , &pdev->dev, +"controller can't do PMP, turning off CAP_PMP\n" ); + cap &= ~HOST_CAP_PMP ; + } +/* * Temporary Marvell 6145 hack: PATA port presence * is asserted through the standard AHCI port
Signature · 1 line
1.5.0.3
Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Tejun HeoHere's one thing that jumps out at me, reviewing the PMP patchset:
PMP reads and writes require polling, which is not something we should impose upon the design. Conversations with a port multiplier are fundamentally packetized, and modern controllers treat these just like the bazillion other types of packets they must deal with: just another entry on a DMA ring, with an interrupt to signal completion/reception.
For situations like this, libata EH needs to the use normal, natural delivery method: initiate an action, and wait asynchronously for completion.
Certainly the flow of control might be synchronous inside libata EH; that requirement is normal and usable with an issue+wait setup.
More and more I am convinced this "mission creep", the slow expansion of polling even into modern controllers, is going in the wrong direction.
Consider the mvsas driver (Marvell 6440 SAS/SATA), whose rough draft I posted yesterday: in order to support SAS wide ports -- multiple phys aggregated at runtime into a single "SAS port" -- and remote SATA device attachment behind expanders then PMPs, their hardware design became
- single command queue, for all ports/phys
- single response queue, for all ports/phys
- events from all ports, SAS and SATA, aggregate
onto those queues
- a single MSI interrupt signals "go look for new work on
DMA ring"
There, even the concept of "port" is fluid, and the libata EH model of freezing and thawing a port (with the desired irq-off result) just doesn't fit the hardware.
The model under SAS+SATA is even closer to that of a network driver: you just have a very dumb driver, that sends and receives frames, as signalled via interrupt.
As such, polling is simply an outmoded concept. It does not make sense on new hardware, and forcing design decisions down that path only lead to a cascade of similar design decisions -- pmp_read polling being just one example of such a result.
Just like the Linux kernel MM platform API presents 3 levels of page table entries, even when the hardware may only have 2, libata high level API _must_ be implemented as 100% asynchronous event driven API.
If the default implementation chooses to use polling -- i.e. all SFF controllers -- that's fine. But in the new SAS/SATA world its clear that we have far too many polling-related assumptions as it is.
Polling just flat out doesn't make sense on modern SAS/SATA -- and even a couple modern SATA controllers. On such controllers, we are notified immediately via interrupt even in the event of errors.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 1 line · Jeff Garzik
Just like the Linux kernel MM platform API presents 3 levels of page table entries, even when the hardware may only have 2, libata high level API _must_ be implemented as 100% asynchronous event driven API.
To be more clear, non-taskfile packets/commands should be delivered via ->qc_issue, just like taskfiles.
This is similar to sas_task in libsas, which can be a SATA packet, a SAS packet, a management packet, ...
In the long run, we should probably look at integrating ata_queued_cmd and sas_task. Once ata_queued_cmd becomes multi-protocol (tf, pmp, sgpio, ...), it becomes more generic. sas_task is also multi-protocol.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Jeff GarzikJeff Garzik wrote:
[--snip--]
Quoted 3 lines
There, even the concept of "port" is fluid, and the libata EH model of freezing and thawing a port (with the desired irq-off result) just doesn't fit the hardware.
Well, the current model was developed while struggling with the first generation PMP hardware which had a lot of quirks. More focus was on making the system run and keeping it that way. Anyways, now that the quirks are better understood, I think we can make PMP register access irq-driven safely.
[--snip--]
Quoted 4 lines
As such, polling is simply an outmoded concept. It does not make sense on new hardware, and forcing design decisions down that path only lead to a cascade of similar design decisions --
pmp_readpolling being just one example of such a result.
For pmp read/write, I agree that IRQ driven access would be better but even for fairly advanced controllers like ahci or sil24, I think resetting-by-polling is a good idea.
Quoted 3 lines
Just like the Linux kernel MM platform API presents 3 levels of page table entries, even when the hardware may only have 2, libata high level API _must_ be implemented as 100% asynchronous event driven API.
Or allow both?
Quoted 7 lines
If the default implementation chooses to use polling -- i.e. all SFF controllers -- that's fine. But in the new SAS/SATA world its clear that we have far too many polling-related assumptions as it is.
Polling just flat out doesn't make sense on modern SAS/SATA -- and even a couple modern SATA controllers. On such controllers, we are notified immediately via interrupt even in the event of errors.
For SAS, I don't have any strong opinion. For SATA, I think we definitely need to allow or even prefer polling for host port resets.
Is this NACK on the patchset or can we update PMP access later?
Signature · 1 line
tejun
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 1 line · Tejun Heo
Is this NACK on the patchset or can we update PMP access later?
Ping?
Signature · 1 line
tejun
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Tejun HeoQuoted 13 lines · Tejun Heo
Jeff Garzik wrote:
[--snip--]Quoted 3 lines
There, even the concept of "port" is fluid, and the libata EH model of freezing and thawing a port (with the desired irq-off result) just doesn't fit the hardware.
Well, the current model was developed while struggling with the first generation PMP hardware which had a lot of quirks. More focus was on making the system run and keeping it that way. Anyways, now that the quirks are better understood, I think we can make PMP register access irq-driven safely.
[--snip--]
Quoted 8 lines
Quoted 4 lines
As such, polling is simply an outmoded concept. It does not make sense on new hardware, and forcing design decisions down that path only lead to a cascade of similar design decisions --
pmp_readpolling being just one example of such a result.For pmp read/write, I agree that IRQ driven access would be better but even for fairly advanced controllers like ahci or sil24, I think resetting-by-polling is a good idea.
Not for SAS/SATA controllers. These SAS+SATA controllers allow you set a bit initiating phy reset, and then receive an interrupt when something interesting happens.
The reason why I (we?) am just finding out about this is: both drivers/scsi/libsas and drivers/scsi/ipr are still using the old-EH :(
But overall, anything sufficiently "high level" is not friendly to polling -- if a polling model is even possible.
Quoted 7 lines
Quoted 3 lines
Just like the Linux kernel MM platform API presents 3 levels of page table entries, even when the hardware may only have 2, libata high level API _must_ be implemented as 100% asynchronous event driven API.
Or allow both?
Quoted 1 line
If the default implementation chooses to use polling -- i.e. all SFF
Note the line quoted above, immediately following your response... such a setup does permit both: an asynchronous submit/complete API can be implementing under the hood via polling or irq, as the driver chooses.
The main point is to not /require/ polling.
Quoted 11 lines
Quoted 6 lines
controllers -- that's fine. But in the new SAS/SATA world its clear that we have far too many polling-related assumptions as it is.
Polling just flat out doesn't make sense on modern SAS/SATA -- and even a couple modern SATA controllers. On such controllers, we are notified immediately via interrupt even in the event of errors.
For SAS, I don't have any strong opinion. For SATA, I think we definitely need to allow or even prefer polling for host port resets.
Is this NACK on the patchset or can we update PMP access later?
Sorry, yes, it is a NAK: polling should not be requirement.
I considered making multi-protocol ->qc_issue() a requirement too, but that seemed like it might delay things too much. But consider this a strong to-do item... ->pmp_read and ->pmp_write hooks should be folded into ->qc_issue and ata_qc_complete(), because quite often a PMP read/write packet can be delivered just like any other packet. We want a single "submit packet to hardware" interface, not multiple ones.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 10 lines · Jeff Garzik
Quoted 1 line
Is this NACK on the patchset or can we update PMP access later?
Sorry, yes, it is a NAK: polling should not be requirement.
I considered making multi-protocol ->qc_issue() a requirement too, but that seemed like it might delay things too much. But consider this a strong to-do item... ->pmp_read and ->pmp_write hooks should be folded into ->qc_issue and
ata_qc_complete(), because quite often a PMP read/write packet can be delivered just like any other packet. We want a single "submit packet to hardware" interface, not multiple ones.
Aieee... Another merge delay. I wish the review process proceeded a bit
swifter. The patchset has been around literally for years now and
submitted for review six times if I have the take number right. :-(
Signature · 1 line
tejun
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 3 lines
Aieee... Another merge delay. I wish the review process proceeded a bit
swifter. The patchset has been around literally for years now and
submitted for review six times if I have the take number right. :-(
Agreed - thats one reason I stick everything in -mm. Its taking several months for a known fix to get from me to the outside. I'm not sure its entirely a Jeff problem however.
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 7 lines · Alan Cox
Quoted 3 lines
Aieee... Another merge delay. I wish the review process proceeded a bit
swifter. The patchset has been around literally for years now and
submitted for review six times if I have the take number right. :-(Agreed - thats one reason I stick everything in -mm. Its taking several months for a known fix to get from me to the outside. I'm not sure its entirely a Jeff problem however.
This doesn't affect Tejun or anyone else really, but going through -mm tends to gum up the works. Your patches wind up not applying to libata#upstream, in which case they get dropped and I assume Andrew or someone will resend usable versions.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 4 lines
This doesn't affect Tejun or anyone else really, but going through -mm tends to gum up the works. Your patches wind up not applying to libata#upstream, in which case they get dropped and I assume Andrew or someone will resend usable versions.
Its the only way to get testing done and get them into distributions in time.
Alan
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 4 lines · Alan Cox
Quoted 1 line
This doesn't affect Tejun or anyone else really, but going through -mm tends to gum up the works. Your patches wind up not applying to libata#upstream, in which case they get dropped and I assume Andrew or someone will resend usable versions.
Its the only way to get testing done and get them into distributions in time.
Not true... libata gets auto-propagated into -mm, and Andrew pulls regularly.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 9 lines · Jeff Garzik
Quoted 6 lines · Alan Cox
Quoted 3 lines
This doesn't affect Tejun or anyone else really, but going through
-mm tends to gum up the works. Your patches wind up not applying to
libata#upstream, in which case they get dropped and I assume Andrew or someone will resend usable versions.Its the only way to get testing done and get them into distributions in time.
Not true... libata gets auto-propagated into -mm, and Andrew pulls regularly.
I believe the point was that getting things into libata is glacial compared with getting them into -mm. Witness the PMP patchset.
Cheers
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 6 lines · Mark Lord
Quoted 5 lines · Jeff Garzik
Quoted 4 lines · Alan Cox
Quoted 3 lines
This doesn't affect Tejun or anyone else really, but going through
-mm tends to gum up the works. Your patches wind up not applying to
libata#upstream, in which case they get dropped and I assume Andrew or someone will resend usable versions.
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Its the only way to get testing done and get them into distributions in time.
Not true... libata gets auto-propagated into -mm, and Andrew pulls regularly.
I believe the point was that getting things into libata is glacial compared with getting them into -mm. Witness the PMP patchset.
I don't think PMP was ever in -mm?
But regardless, people can send me "for test only" or WIP stuff that I can auto-propagate to -mm via libata-dev.git. I keep many libata-dev.git branches alive with testing and WIP code, and it doesn't consume much time at all.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Mark Lord(my last response only addressed -mm)
Quoted 1 line · Mark Lord
I believe the point was that getting things into libata is glacial
IMHO would say that there are two causes of that:
1) I am sometimes slow in merging, part of which is my own fault, and I can only promise to try and do better there. Part of which is a result of stuff being dependent on -mm (which requires plenty of time hand-merging) rather than libata-dev.git#upstream.
2) I have been intentionally staging major libata behavior changes between Linux releases. Luckily most of these are behind us, but, in several cases with things like ACPI on/off (hopefully 'on' in 2.6.24), probing changes (switchover to new EH for probing via hotplug, etc.), interrupt handling changes.
I dislike getting "too much" into a single release, because of the difficulty of getting large scale feedback without a major kernel release.
So far I think the kernel releases have been pretty darn successful in "not breaking everybody" but that clearly conflicts with desired development speed, given the glacial pace of each kernel release. If each kernel release were 1-2 months apart, we would have many more testing points, and I think you guys and I would both be happy. But that's not the reality today, with 3+ month kernel release cycles (ugh!!).
I'm very much interested in hearing suggestions and comments.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 20 lines · On Fri, 28 Sep 2007 23:29:44 -0400 Jeff Garzik <[email protected]> wrote:
(my last response only addressed -mm)
Quoted 1 line · Mark Lord
I believe the point was that getting things into libata is glacial
IMHO would say that there are two causes of that:
1) I am sometimes slow in merging, part of which is my own fault, and I can only promise to try and do better there. Part of which is a result of stuff being dependent on -mm (which requires plenty of time hand-merging) rather than libata-dev.git#upstream.2) I have been intentionally staging major libata behavior changes between Linux releases. Luckily most of these are behind us, but, in several cases with things like ACPI on/off (hopefully 'on' in 2.6.24), probing changes (switchover to new EH for probing via hotplug, etc.), interrupt handling changes.
I dislike getting "too much" into a single release, because of the difficulty of getting large scale feedback without a major kernel release.
Quoted 9 lines
So far I think the kernel releases have been pretty darn successful in "not breaking everybody" but that clearly conflicts with desired development speed, given the glacial pace of each kernel release. If each kernel release were 1-2 months apart, we would have many more testing points, and I think you guys and I would both be happy. But that's not the reality today, with 3+ month kernel release cycles (ugh!!).
I'm very much interested in hearing suggestions and comments.
There's an easy fix...
The releases are slow because a) there's so much stuff in them and b) it takes so long to stabilise it all.
If all developers were to be more careful in their work, and take more time to review and test others' work, both problems get fixed: less code, higher quality.
We don't know how to make this happen. We haven't even tried.
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 20 lines · Andrew Morton
Quoted 7 lines · On Fri, 28 Sep 2007 23:29:44 -0400 Jeff Garzik <[email protected]> wrote:
(my last response only addressed -mm)
Quoted 1 line · Mark Lord
I believe the point was that getting things into libata is glacial
IMHO would say that there are two causes of that:
1) I am sometimes slow in merging, part of which is my own fault, and I can only promise to try and do better there. Part of which is a result of stuff being dependent on -mm (which requires plenty of time hand-merging) rather than libata-dev.git#upstream.Quoted 8 lines
2) I have been intentionally staging major libata behavior changes between Linux releases. Luckily most of these are behind us, but, in several cases with things like ACPI on/off (hopefully 'on' in 2.6.24), probing changes (switchover to new EH for probing via hotplug, etc.), interrupt handling changes.
I dislike getting "too much" into a single release, because of the difficulty of getting large scale feedback without a major kernel release.
So far I think the kernel releases have been pretty darn successful in "not breaking everybody" but that clearly conflicts with desired development speed, given the glacial pace of each kernel release. If each kernel release were 1-2 months apart, we would have many more testing points, and I think you guys and I would both be happy. But that's not the reality today, with 3+ month kernel release cycles (ugh!!).
I'm very much interested in hearing suggestions and comments.
There's an easy fix...
Quoted 8 lines
The releases are slow because a) there's so much stuff in them and b) it takes so long to stabilise it all.
If all developers were to be more careful in their work, and take more time to review and test others' work, both problems get fixed: less code, higher quality.
We don't know how to make this happen. We haven't even tried.
I think you missed part of my point, with regards to libata:
Even though stuff gets tested in -mm and in mainline -rc releases, we simply do not have the test audience that a major kernel release does, when it comes to determining whether new libata probe behavior will break millions of boxes (or not).
Unlike CPUs and other hardware, our attached devices (disks, cd-roms) are entirely black boxes of [mis]behavior, with plenty of libata behavior based _entirely_ on observations in the field, not stuff documented in a specification somewhere.
3-4 true "trial and error" periods per year (kernel releases) give libata a similar number of test points per years. Not a whole lot of room when you consider that we must actively (but carefully!) experimental with new kernel features in the each kernel release.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Jeff GarzikQuoted 27 lines · Jeff Garzik
(my last response only addressed -mm)
Quoted 1 line · Mark Lord
I believe the point was that getting things into libata is glacial
IMHO would say that there are two causes of that:
1) I am sometimes slow in merging, part of which is my own fault, and I can only promise to try and do better there. Part of which is a result of stuff being dependent on -mm (which requires plenty of time hand-merging) rather than libata-dev.git#upstream.2) I have been intentionally staging major libata behavior changes between Linux releases. Luckily most of these are behind us, but, in several cases with things like ACPI on/off (hopefully 'on' in 2.6.24), probing changes (switchover to new EH for probing via hotplug, etc.), interrupt handling changes.
I dislike getting "too much" into a single release, because of the difficulty of getting large scale feedback without a major kernel release.
So far I think the kernel releases have been pretty darn successful in "not breaking everybody" but that clearly conflicts with desired development speed, given the glacial pace of each kernel release. If each kernel release were 1-2 months apart, we would have many more testing points, and I think you guys and I would both be happy. But that's not the reality today, with 3+ month kernel release cycles (ugh!!).
Quoted 3 lines
I'm very much interested in hearing suggestions and comments.
Jeff
Perhaps you could more fully leverage the various distro alpha/beta/rcs/release kernels.
Specific to the PMP patches, they went into the openSUSE beta/rc kernels (2.6.23 based) in early Aug. I think. So they have already been through a significant set of Novell internal and community testing. Likely similar testing to what it would get in -mm.
I believe they are part of the OpenSUSE 10.3 (2.6.23 based) full release that is set for Thursday (Oct. 4) and thus will see a huge amount of public testing.
Greg
Signature · 7 lines
Greg Freemyer Litigation Triage Solutions Specialist http://www.linkedin.com/in/gregfreemyer The Norcross Group The Intersection of Evidence & Technology http://www.norcrossgroup.com
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Jeff Garzikty today, with 3+ month kernel release cycles (ugh!!).
Quoted 2 lines
I'm very much interested in hearing suggestions and comments.
I think PMP should go in for 2.6.24 and then get revised over time to not poll and to go via qc_issue. Which seems to be the consensus of everyone but you on this one.
In general I have no big problems with the way its going - need to get stuff through -mm into libata faster but thats something I need to sort out with you specifically and I can accept your view its more my problem.
But PMP is a definite candidate for in ASAP
Alan
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 6 lines · Alan Cox
ty today, with 3+ month kernel release cycles (ugh!!).
Quoted 1 line
I'm very much interested in hearing suggestions and comments.
I think PMP should go in for 2.6.24 and then get revised over time to not poll and to go via
qc_issue. Which seems to be the consensus of everyone but you on this one.
It's not an "over time" issue. SAS drivers that will be ready for 2.6.24 (broadsas, mvsas) have both been coded to support PMP transparently -- but the current libata PMP code requires that PMP support in SAS be TURNED OFF, because it is fundamentally incompatible by its design.
This is an issue that has been present and known for many, many months -- as long as drivers/scsi/libsas/sas_ata.c has been in the tree.
The more we walk down the polling path, the more incompatible we become with SATA-capable controllers that are in users' hands today.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Tejun HeoQuoted 11 lines · Tejun Heo
Quoted 9 lines · Jeff Garzik
Quoted 1 line
Is this NACK on the patchset or can we update PMP access later?
Sorry, yes, it is a NAK: polling should not be requirement.
I considered making multi-protocol ->qc_issue() a requirement too, but that seemed like it might delay things too much. But consider this a strong to-do item... ->pmp_read and ->pmp_write hooks should be folded into ->qc_issue and
ata_qc_complete(), because quite often a PMP read/write packet can be delivered just like any other packet. We want a single "submit packet to hardware" interface, not multiple ones.
Quoted 3 lines
Aieee... Another merge delay. I wish the review process proceeded a bit
swifter. The patchset has been around literally for years now and
submitted for review six times if I have the take number right. :-(
Well the vast majority of the patches are in, what five out of six original patchsets?
Sorry I didn't catch the polling requirement beforehand, it was not really clear from a quick read.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 6 lines · Jeff Garzik
Quoted 3 lines
Aieee... Another merge delay. I wish the review process proceeded a bit
swifter. The patchset has been around literally for years now and
submitted for review six times if I have the take number right. :-(Well the vast majority of the patches are in, what five out of six original patchsets?
Yeah, I'm frustrated mainly because I've been telling people that mainline will probably have PMP support when 2.6.24 comes out and it seems we'll miss the merge window again. Oh, well...
Quoted 2 lines
Sorry I didn't catch the polling requirement beforehand, it was not really clear from a quick read.
->pmp_read/write stuff is something which I've been meaning to change anyway. When developing the PMP code, PMP register access while frozen seemed necessary but now I think we can be just as safe without it. I was thinking about changing it after merge because the current code received a lot of testing and I didn't want to destabilize it right before merging.
I'll be back home mid next week. I'll try to re-test and re-submit the changes ASAP.
Thanks.
Signature · 1 line
tejun
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 21 lines · Tejun Heo
Quoted 5 lines · Jeff Garzik
Quoted 3 lines
Aieee... Another merge delay. I wish the review process proceeded a bit
swifter. The patchset has been around literally for years now and
submitted for review six times if I have the take number right. :-(Well the vast majority of the patches are in, what five out of six original patchsets?
Yeah, I'm frustrated mainly because I've been telling people that mainline will probably have PMP support when 2.6.24 comes out and it seems we'll miss the merge window again. Oh, well...
Quoted 2 lines
Sorry I didn't catch the polling requirement beforehand, it was not really clear from a quick read.
->pmp_read/write stuff is something which I've been meaning to change anyway. When developing the PMP code, PMP register access while frozen seemed necessary but now I think we can be just as safe without it. I was thinking about changing it after merge because the current code received a lot of testing and I didn't want to destabilize it right before merging.
Quoted 2 lines
I'll be back home mid next week. I'll try to re-test and re-submit the changes ASAP.
Jeff, seeing as Tejun's commitment is never in doubt here, I really believe we should go with the existing PMP patchset for 2.6.24 (unless the respin happens quickly enough).
This functionality is way overdue, and we shouldn't be impeding it as long as we have been.
Tejun will definitely continue to rework the changes you've asked for in time for the next release, but let's not hold things up unreasonably here.
Cheers
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 15 lines · Mark Lord
Quoted 14 lines · Tejun Heo
Quoted 5 lines · Jeff Garzik
Quoted 3 lines
Aieee... Another merge delay. I wish the review process proceeded a bit
swifter. The patchset has been around literally for years now and
submitted for review six times if I have the take number right. :-(Well the vast majority of the patches are in, what five out of six original patchsets?
Yeah, I'm frustrated mainly because I've been telling people that mainline will probably have PMP support when 2.6.24 comes out and it seems we'll miss the merge window again. Oh, well...
Quoted 2 lines
Sorry I didn't catch the polling requirement beforehand, it was not really clear from a quick read.
Quoted 6 lines
Quoted 6 lines
->pmp_read/write stuff is something which I've been meaning to change anyway. When developing the PMP code, PMP register access while frozen seemed necessary but now I think we can be just as safe without it. I was thinking about changing it after merge because the current code received a lot of testing and I didn't want to destabilize it right before merging.
This is an excellent point for merging the PMP code as it is currently and doing revamp later. PMP patchset in the current form has got quite a lot of testing in -mm and "last minute" changes have a tendency to bring up some nasty surprises.
Quoted 9 lines
Quoted 2 lines
I'll be back home mid next week. I'll try to re-test and re-submit the changes ASAP.
Jeff, seeing as Tejun's commitment is never in doubt here, I really believe we should go with the existing PMP patchset for 2.6.24 (unless the respin happens quickly enough).
This functionality is way overdue, and we shouldn't be impeding it as long as we have been.
It is way, way overdue...
Quoted 2 lines
Tejun will definitely continue to rework the changes you've asked for in time for the next release, but let's not hold things up unreasonably here.
Seconded.
Bart
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Mark LordQuoted 6 lines
Jeff, seeing as Tejun's commitment is never in doubt here, I really believe we should go with the existing PMP patchset for 2.6.24 (unless the respin happens quickly enough).
This functionality is way overdue, and we shouldn't be impeding it as long as we have been.
I would second this. Its far too important to not get this stuff upstream and usable NOW. Yes the model will have to change a bit but the entire libata today has almost no resemblence to the one a year ago.
It can evolve and then the old one can die off just as we did with EH (except for libsas anyway)
Alan
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 7 lines · Alan Cox
Quoted 6 lines
Jeff, seeing as Tejun's commitment is never in doubt here, I really believe we should go with the existing PMP patchset for 2.6.24 (unless the respin happens quickly enough).
This functionality is way overdue, and we shouldn't be impeding it as long as we have been.
Quoted 6 lines
I would second this. Its far too important to not get this stuff upstream and usable NOW. Yes the model will have to change a bit but the entire libata today has almost no resemblence to the one a year ago.
It can evolve and then the old one can die off just as we did with EH (except for libsas anyway)
I certainly deserve plenty of blame for not catching this fact earlier, much to my chagrin. But there are real technical issues at hand:
Polling ALREADY makes the job of fixing SAS/SATA exception handling difficult. Expanding polling to something SAS/SATA controllers treat as fundamentally irq-driven and integrated with the rest of the command flow is moving in the wrong direction.
To re-re-re-summarize, polling in PMP is fundamentally broken for an ENTIRE CLASS OF HARDWARE that we actively support today. And jgarzik/misc-2.6.git#sas is adding two more controllers to that list.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 8 lines · Jeff Garzik
Polling ALREADY makes the job of fixing SAS/SATA exception handling difficult. Expanding polling to something SAS/SATA controllers treat as fundamentally irq-driven and integrated with the rest of the command flow is moving in the wrong direction.
To re-re-re-summarize, polling in PMP is fundamentally broken for an ENTIRE CLASS OF HARDWARE that we actively support today. And jgarzik/misc-2.6.git#sas is adding two more controllers to that list.
As an interim solution, it doesn't make anything worse tho. Those drivers don't support PMP anyway. After rc1 merge, polling PMP access can be replaced with new qc_issue (probably ata_exec_internal) based code.
The question here is whether it's worth to include PMP support with polling PMP register access as an interim solution for 2.6.24. I think it will be beneficial for both user convenience and testing as long as the said change is made soon after -rc1.
My vote is yes but this kind of decision ultimately falls on the subsystem maintainer, so it's your call, Jeff.
Thanks.
Signature · 1 line
tejun
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 10 lines · Tejun Heo
Quoted 8 lines · Jeff Garzik
Polling ALREADY makes the job of fixing SAS/SATA exception handling difficult. Expanding polling to something SAS/SATA controllers treat as fundamentally irq-driven and integrated with the rest of the command flow is moving in the wrong direction.
To re-re-re-summarize, polling in PMP is fundamentally broken for an ENTIRE CLASS OF HARDWARE that we actively support today. And jgarzik/misc-2.6.git#sas is adding two more controllers to that list.
Quoted 8 lines
As an interim solution, it doesn't make anything worse tho. Those drivers don't support PMP anyway. After rc1 merge, polling PMP access can be replaced with new
qc_issue(probablyata_exec_internal) based code.The question here is whether it's worth to include PMP support with polling PMP register access as an interim solution for 2.6.24. I think it will be beneficial for both user convenience and testing as long as the said change is made soon after -rc1.
Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS driver releases out of PMP.
I pulled your last PMP patchset, and will now endeavor to fix the API prior to 2.6.24 merge window opening.
Linux high level message-submit / message-complete APIs should never _require_ polling, even if its 100% polling under the hood. There are far too many cases in the field where you don't have direct access to hardware registers to poll. Or such polling would interfere with the operation of other ports. Or any of a myriad of other reasons.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 2 lines · Jeff Garzik
Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS driver releases out of PMP.
Not merging PMP has about the same effect for SAS, right?
Quoted 2 lines
I pulled your last PMP patchset, and will now endeavor to fix the API prior to 2.6.24 merge window opening.
I have a preliminary patch. The patch is pretty simple too. I'll submit after some testing but I'm not sure whether doing this right before the merge window is a good idea. I'd prefer not to support PMP on SAS for 2.6.24.
Quoted 5 lines
Linux high level message-submit / message-complete APIs should never _require_ polling, even if its 100% polling under the hood. There are far too many cases in the field where you don't have direct access to hardware registers to poll. Or such polling would interfere with the operation of other ports. Or any of a myriad of other reasons.
I don't think that's true for actual ATA controllers but, for SAS, probably true. Is the frozen reset mechanism a headache too for SAS?
Thanks.
Signature · 1 line
tejun
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Jeff GarzikQuoted 2 lines
Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS driver releases out of PMP.
No PMP in 2.6.24 screws PMP users, who outnumber SAS users by a few thousand to one I suspect.
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 4 lines · Alan Cox
Quoted 1 line
Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS driver releases out of PMP.
No PMP in 2.6.24 screws PMP users, who outnumber SAS users by a few thousand to one I suspect.
no-PMP-in-2.6.24 is not happening, so that's rather irrelevant.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Jeff GarzikQuoted 4 lines · Jeff Garzik
I certainly deserve plenty of blame for not catching this fact earlier, much to my chagrin. But there are real technical issues at hand:
Polling ALREADY makes the job of fixing SAS/SATA exception handling difficult. Expanding polling to something SAS/SATA controllers treat as fundamentally irq-driven and integrated with the rest of the command flow is moving in the wrong direction.
Fine. But the exising PMP patchset has already taken close to a year to become mature and safe enough to pass "Jeff review". Tons of us are awaiting it in mainline, and to rework it to meet your latest concerns will delay it until 2.6.25 at least --> mid 2008. Not Acceptable.
It doesn't *break* anything (we know of).
It doesn't affect existing SAS functionality.
It *does* add working, critical functionality to libata.
And.. Tejun isn't just going to lay down and be happy
with it in 2.6.24. He's fully committed to the rework you're demanding, but in a more timely and correct fashion.
Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it.
The current implementation is certainly kernel-worthy, and will be reworked over time as is the rest of libata.
The barrier to entry for new libata functionality is way too high. Yes, we want readable, *reliable* code, because screwing people's filesystems is not an option. Fine.
Tejun's current code is more than "good enough",
and "perfect" code doesn't exist.
But we're all striving for it anyway.
Cheers
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 2 lines · Mark Lord
Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it.
It's not about enhancing and tuning.
It's about me (and/or James B) having to __undo__ the current code, just to get things working on an entire class of SATA-capable controllers out in the field.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 9 lines · Jeff Garzik
Quoted 2 lines · Mark Lord
Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it.
It's not about enhancing and tuning.
It's about me (and/or James B) having to
__undo__the current code, just to get things working on an entire class of SATA-capable controllers out in the field.
Hmmm... Simpy not setting ATA_FLAG_PMP isn't enough?
Signature · 1 line
tejun
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 11 lines · Tejun Heo
Quoted 8 lines · Jeff Garzik
Quoted 2 lines · Mark Lord
Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it.
It's not about enhancing and tuning.
It's about me (and/or James B) having to
__undo__the current code, just to get things working on an entire class of SATA-capable controllers out in the field.Hmmm... Simpy not setting
ATA_FLAG_PMPisn't enough?
The point was that I was going to turn on PMP support for SAS controllers in 2.6.24 to coincide with the merge, but now cannot without fixing things.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 7 lines · Jeff Garzik
Quoted 6 lines · Tejun Heo
Quoted 5 lines · Jeff Garzik
Quoted 2 lines · Mark Lord
Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it.
It's not about enhancing and tuning.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
It's about me (and/or James B) having to
__undo__the current code, just to get things working on an entire class of SATA-capable controllers out in the field.Hmmm... Simpy not setting
ATA_FLAG_PMPisn't enough?The point was that I was going to turn on PMP support for SAS controllers in 2.6.24 to coincide with the merge, but now cannot without fixing things.
So, don't.
Just a few days ago PMP was slated to not be enabled for *any* controllers until 2.6.25 or so. Now we have it a release early, for all but SAS.
Cheers
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 12 lines · Mark Lord
Quoted 11 lines · Jeff Garzik
Quoted 10 lines · Tejun Heo
Quoted 8 lines · Jeff Garzik
Quoted 2 lines · Mark Lord
Linux kernel development is supposed to happen incrementally nowadays. Get a nice working solution in place, and then enhance/tune it.
It's not about enhancing and tuning.
It's about me (and/or James B) having to
__undo__the current code, just to get things working on an entire class of SATA-capable controllers out in the field.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Hmmm... Simpy not setting
ATA_FLAG_PMPisn't enough?The point was that I was going to turn on PMP support for SAS controllers in 2.6.24 to coincide with the merge, but now cannot without fixing things.
So, don't.
Just a few days ago PMP was slated to not be enabled for *any* controllers until 2.6.25 or so. Now we have it a release early, for all but SAS.
A few days before that, both PMP and SAS /were/ slated for 2.6.24, and after I fix the design problems, they will be again.
One way or another, upstream will /not/ be doing polling PMP in 2.6.24.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 4 lines · Jeff Garzik
A few days before that, both PMP and SAS /were/ slated for 2.6.24, and after I fix the design problems, they will be again.
One way or another, upstream will /not/ be doing polling PMP in 2.6.24.
Just an update to let you know that I've been working on it. sata_sil24 works okay but ahci still craps itself after resetting downstream ports.
Thanks.
Signature · 1 line
tejun
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 8 lines · Tejun Heo
Quoted 4 lines · Jeff Garzik
A few days before that, both PMP and SAS /were/ slated for 2.6.24, and after I fix the design problems, they will be again.
One way or another, upstream will /not/ be doing polling PMP in 2.6.24.
Just an update to let you know that I've been working on it.
sata_sil24works okay but ahci still craps itself after resetting downstream ports.
Thanks for your patience and perseverance.
It looks like it would be too difficult to get SAS PMP working for 2.6.24 merge window open, so I think it is only fair to rescind my assertion of "polling PMP not in 2.6.24 release."
Removing the polling remains a design requirement for SAS, but the more I look at old-EH-encrusted libsas, the more work I feel it needs before its ready for PMP. Sorry.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 9 lines · Jeff Garzik
Quoted 8 lines · Tejun Heo
Quoted 4 lines · Jeff Garzik
A few days before that, both PMP and SAS /were/ slated for 2.6.24, and after I fix the design problems, they will be again.
One way or another, upstream will /not/ be doing polling PMP in 2.6.24.
Just an update to let you know that I've been working on it.
sata_sil24works okay but ahci still craps itself after resetting downstream ports.
This turns out to be a different issue. SIMG 5723/5744 doesn't like being driven by ahci controllers (both ICH9 and JMB) and the problem isn't related to whether polling is used or not. It's very weird because SATA tracer doesn't show much difference on the host side wire between being driven by ahci and sil24/32, but on the fan-out side, things are seriously broken (repeated COMRESET/COMWAKE/COMINIT and no FIS successfully being relayed from the host side).
Other than that, I've tested things on various combinations and am fairly confident with it. I think we can go for 2.6.24 merge. Even things turn out to be bad, we at least have pretty good bisection point.
Quoted 1 line
Thanks for your patience and perseverance.
Thanks. :-)
Quoted 7 lines
It looks like it would be too difficult to get SAS PMP working for 2.6.24 merge window open, so I think it is only fair to rescind my assertion of "polling PMP not in 2.6.24 release."
Removing the polling remains a design requirement for SAS, but the more I look at old-EH-encrusted libsas, the more work I feel it needs before its ready for PMP. Sorry.
I wish libata EH was easier to deal with from SAS side. I think being able to present SAS end point as an independent ATA host to libata EH would probably make life easier for both sides, but I don't have any actual experience with SAS (yet).
Thanks.
Signature · 1 line
tejun
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
In reply to Jeff GarzikJeff Garzik wrote:
..
Quoted 1 line
As such, polling is simply an outmoded concept. It does not make sense on new hardware, and forcing design decisions down that path only lead to a cascade of similar design decisions --
pmp_readpolling being just one example of such a result.
A related tangent here, but I wonder if someday we might consider something like NAPI for libata. Our I/O rates can far exceed those of the network stack, and there polling (NAPI) has helped quite a bit.
But our data behaviour is mostly synchronous, whereas network stuff is more
asynchronous. But still, with multiple drives all using NCQ on a port,
a polled interface option might give higher throughput and lower CPU loading.
Just like with the networking code.
Not that this specifically relates to concerns with the current PMP patches, but I'm curious where Jeff sees this all going, as he works on both sides.
Cheers
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 7 lines · Mark Lord
Jeff Garzik wrote:
..Quoted 1 line
As such, polling is simply an outmoded concept. It does not make sense on new hardware, and forcing design decisions down that path only lead to a cascade of similar design decisions --
pmp_readpolling being just one example of such a result.A related tangent here, but I wonder if someday we might consider something like NAPI for libata. Our I/O rates can far exceed those of the network stack, and there polling (NAPI) has helped quite a bit.
I certainly think about these issues each time I write a new storage driver, given that most new hardware has interrupt mitigation/coalescing capability built into it -- in AHCI's case, at my urging.
Storage is a different profile, with quite a bit few interrupts.
Storage interrupt rates are way, way below that of networking. Ethernet is basically forced to deal with an interrupt event every 1K. By design, storage gathers up as much I/O as possible, such that each interrupt could be completing 128K, 512K, 1MB or more.
Finally, for storage I/O workloads with lots of smaller I/Os, those workloads are often seek-heavy, where any interrupt overhead is few and far between, while you're waiting on your slow disk to seek. You /don't/ want to be polling for long milliseconds, waiting for a seek. That's a good way to increase your CPU usage, rather than decrease it.
So, there just hasn't been a real need so far. If interrupt overhead becomes an issue, my first step would be to start programming the interrupt coalescing hardware that comes in modern SATA controllers.
PCI MSI, found on all new controllers, tends to mitigate interrupt overhead even further.
Quoted 3 lines
But our data behaviour is mostly synchronous, whereas network stuff is more
asynchronous. But still, with multiple drives all using NCQ on a port,
a polled interface option might give higher throughput and lower CPU loading.
There are definitely options worth exploring. And with full ports all doing NCQ, you certainly have a lot of events to deal with.
Jeff
Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
Quoted 3 lines
So, there just hasn't been a real need so far. If interrupt overhead becomes an issue, my first step would be to start programming the interrupt coalescing hardware that comes in modern SATA controllers.
Another factor is queue sizes. You don't get 1024 entry queues on your typical disk controller and that makes NAPI type stuff less viable, and sensible IRQ colaescing on the card more sane.
Alan
PS: Jeff - if you want stuff from -mm and it's got cross dependancies, clashes just ask me for "patch xyz versus libata head" and I'll sort it out