Skip to content

Commit

Permalink
libata: kill ATA_FLAG_DISABLED
Browse files Browse the repository at this point in the history
ATA_FLAG_DISABLED is only used by drivers which don't use
->error_handler framework and is largely broken.  Its only meaningful
function is to make irq handlers skip processing if the flag is set,
which is largely useless and even harmful as it makes those ports more
likely to cause IRQ storms.

Kill ATA_FLAG_DISABLED and makes the callers disable attached devices
instead.  ata_port_probe() and ata_port_disable() which manipulate the
flag are also killed.

This simplifies condition check in IRQ handlers.  While updating IRQ
handlers, remove ap NULL check as libata guarantees consecutive port
allocation (unoccupied ports are initialized with dummies) and
long-obsolete ATA_QCFLAG_ACTIVE check (checked by ata_qc_from_tag()).

Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
  • Loading branch information
Tejun Heo authored and Jeff Garzik committed May 18, 2010
1 parent c7a8209 commit 3e4ec34
Show file tree
Hide file tree
Showing 18 changed files with 209 additions and 358 deletions.
66 changes: 1 addition & 65 deletions drivers/ata/libata-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1907,22 +1907,6 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
ap->qc_active = preempted_qc_active;
ap->nr_active_links = preempted_nr_active_links;

/* XXX - Some LLDDs (sata_mv) disable port on command failure.
* Until those drivers are fixed, we detect the condition
* here, fail the command with AC_ERR_SYSTEM and reenable the
* port.
*
* Note that this doesn't change any behavior as internal
* command failure results in disabling the device in the
* higher layer for LLDDs without new reset/EH callbacks.
*
* Kill the following code as soon as those drivers are fixed.
*/
if (ap->flags & ATA_FLAG_DISABLED) {
err_mask |= AC_ERR_SYSTEM;
ata_port_probe(ap);
}

spin_unlock_irqrestore(ap->lock, flags);

if ((err_mask & AC_ERR_TIMEOUT) && auto_timeout)
Expand Down Expand Up @@ -2768,8 +2752,6 @@ int ata_bus_probe(struct ata_port *ap)
int rc;
struct ata_device *dev;

ata_port_probe(ap);

ata_for_each_dev(dev, &ap->link, ALL)
tries[dev->devno] = ATA_PROBE_MAX_TRIES;

Expand Down Expand Up @@ -2797,17 +2779,14 @@ int ata_bus_probe(struct ata_port *ap)
ap->ops->phy_reset(ap);

ata_for_each_dev(dev, &ap->link, ALL) {
if (!(ap->flags & ATA_FLAG_DISABLED) &&
dev->class != ATA_DEV_UNKNOWN)
if (dev->class != ATA_DEV_UNKNOWN)
classes[dev->devno] = dev->class;
else
classes[dev->devno] = ATA_DEV_NONE;

dev->class = ATA_DEV_UNKNOWN;
}

ata_port_probe(ap);

/* read IDENTIFY page and configure devices. We have to do the identify
specific sequence bass-ackwards so that PDIAG- is released by
the slave device */
Expand Down Expand Up @@ -2857,8 +2836,6 @@ int ata_bus_probe(struct ata_port *ap)
ata_for_each_dev(dev, &ap->link, ENABLED)
return 0;

/* no device present, disable port */
ata_port_disable(ap);
return -ENODEV;

fail:
Expand Down Expand Up @@ -2889,22 +2866,6 @@ int ata_bus_probe(struct ata_port *ap)
goto retry;
}

/**
* ata_port_probe - Mark port as enabled
* @ap: Port for which we indicate enablement
*
* Modify @ap data structure such that the system
* thinks that the entire port is enabled.
*
* LOCKING: host lock, or some other form of
* serialization.
*/

void ata_port_probe(struct ata_port *ap)
{
ap->flags &= ~ATA_FLAG_DISABLED;
}

/**
* sata_print_link_status - Print SATA link status
* @link: SATA link to printk link status about
Expand Down Expand Up @@ -2951,26 +2912,6 @@ struct ata_device *ata_dev_pair(struct ata_device *adev)
return pair;
}

/**
* ata_port_disable - Disable port.
* @ap: Port to be disabled.
*
* Modify @ap data structure such that the system
* thinks that the entire port is disabled, and should
* never attempt to probe or communicate with devices
* on this port.
*
* LOCKING: host lock, or some other form of
* serialization.
*/

void ata_port_disable(struct ata_port *ap)
{
ap->link.device[0].class = ATA_DEV_NONE;
ap->link.device[1].class = ATA_DEV_NONE;
ap->flags |= ATA_FLAG_DISABLED;
}

/**
* sata_down_spd_limit - adjust SATA spd limit downward
* @link: Link to adjust SATA spd limit for
Expand Down Expand Up @@ -5716,7 +5657,6 @@ struct ata_port *ata_port_alloc(struct ata_host *host)

ap->pflags |= ATA_PFLAG_INITIALIZING;
ap->lock = &host->lock;
ap->flags = ATA_FLAG_DISABLED;
ap->print_id = -1;
ap->ctl = ATA_DEVCTL_OBS;
ap->host = host;
Expand Down Expand Up @@ -6145,8 +6085,6 @@ static void async_port_probe(void *data, async_cookie_t cookie)
struct ata_eh_info *ehi = &ap->link.eh_info;
unsigned long flags;

ata_port_probe(ap);

/* kick EH for boot probing */
spin_lock_irqsave(ap->lock, flags);

Expand Down Expand Up @@ -6823,7 +6761,6 @@ EXPORT_SYMBOL_GPL(ata_port_start);
EXPORT_SYMBOL_GPL(ata_do_set_mode);
EXPORT_SYMBOL_GPL(ata_std_qc_defer);
EXPORT_SYMBOL_GPL(ata_noop_qc_prep);
EXPORT_SYMBOL_GPL(ata_port_probe);
EXPORT_SYMBOL_GPL(ata_dev_disable);
EXPORT_SYMBOL_GPL(sata_set_spd);
EXPORT_SYMBOL_GPL(ata_wait_after_reset);
Expand All @@ -6835,7 +6772,6 @@ EXPORT_SYMBOL_GPL(sata_std_hardreset);
EXPORT_SYMBOL_GPL(ata_std_postreset);
EXPORT_SYMBOL_GPL(ata_dev_classify);
EXPORT_SYMBOL_GPL(ata_dev_pair);
EXPORT_SYMBOL_GPL(ata_port_disable);
EXPORT_SYMBOL_GPL(ata_ratelimit);
EXPORT_SYMBOL_GPL(ata_wait_register);
EXPORT_SYMBOL_GPL(ata_scsi_queuecmd);
Expand Down
3 changes: 0 additions & 3 deletions drivers/ata/libata-scsi.c
Original file line number Diff line number Diff line change
Expand Up @@ -3345,9 +3345,6 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
struct ata_link *link;
struct ata_device *dev;

if (ap->flags & ATA_FLAG_DISABLED)
return;

repeat:
ata_for_each_link(link, ap, EDGE) {
ata_for_each_dev(dev, link, ENABLED) {
Expand Down
10 changes: 2 additions & 8 deletions drivers/ata/libata-sff.c
Original file line number Diff line number Diff line change
Expand Up @@ -1807,9 +1807,6 @@ irqreturn_t ata_sff_interrupt(int irq, void *dev_instance)
struct ata_port *ap = host->ports[i];
struct ata_queued_cmd *qc;

if (unlikely(ap->flags & ATA_FLAG_DISABLED))
continue;

qc = ata_qc_from_tag(ap, ap->link.active_tag);
if (qc) {
if (!(qc->tf.flags & ATA_TFLAG_POLLING))
Expand Down Expand Up @@ -1884,11 +1881,8 @@ void ata_sff_lost_interrupt(struct ata_port *ap)

/* Only one outstanding command per SFF channel */
qc = ata_qc_from_tag(ap, ap->link.active_tag);
/* Check we have a live one.. */
if (qc == NULL || !(qc->flags & ATA_QCFLAG_ACTIVE))
return;
/* We cannot lose an interrupt on a polled command */
if (qc->tf.flags & ATA_TFLAG_POLLING)
/* We cannot lose an interrupt on a non-existent or polled command */
if (!qc || qc->tf.flags & ATA_TFLAG_POLLING)
return;
/* See if the controller thinks it is still busy - if so the command
isn't a lost IRQ but is still in progress */
Expand Down
16 changes: 5 additions & 11 deletions drivers/ata/pata_bf54x.c
Original file line number Diff line number Diff line change
Expand Up @@ -1400,18 +1400,12 @@ static irqreturn_t bfin_ata_interrupt(int irq, void *dev_instance)
spin_lock_irqsave(&host->lock, flags);

for (i = 0; i < host->n_ports; i++) {
struct ata_port *ap;
struct ata_port *ap = host->ports[i];
struct ata_queued_cmd *qc;

ap = host->ports[i];
if (ap &&
!(ap->flags & ATA_FLAG_DISABLED)) {
struct ata_queued_cmd *qc;

qc = ata_qc_from_tag(ap, ap->link.active_tag);
if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING)) &&
(qc->flags & ATA_QCFLAG_ACTIVE))
handled |= bfin_ata_host_intr(ap, qc);
}
qc = ata_qc_from_tag(ap, ap->link.active_tag);
if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING)))
handled |= bfin_ata_host_intr(ap, qc);
}

spin_unlock_irqrestore(&host->lock, flags);
Expand Down
9 changes: 2 additions & 7 deletions drivers/ata/pata_octeon_cf.c
Original file line number Diff line number Diff line change
Expand Up @@ -654,9 +654,6 @@ static irqreturn_t octeon_cf_interrupt(int irq, void *dev_instance)
ap = host->ports[i];
ocd = ap->dev->platform_data;

if (ap->flags & ATA_FLAG_DISABLED)
continue;

ocd = ap->dev->platform_data;
cf_port = ap->private_data;
dma_int.u64 =
Expand All @@ -666,8 +663,7 @@ static irqreturn_t octeon_cf_interrupt(int irq, void *dev_instance)

qc = ata_qc_from_tag(ap, ap->link.active_tag);

if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING)) &&
(qc->flags & ATA_QCFLAG_ACTIVE)) {
if (qc && !(qc->tf.flags & ATA_TFLAG_POLLING)) {
if (dma_int.s.done && !dma_cfg.s.en) {
if (!sg_is_last(qc->cursg)) {
qc->cursg = sg_next(qc->cursg);
Expand Down Expand Up @@ -737,8 +733,7 @@ static void octeon_cf_delayed_finish(struct work_struct *work)
goto out;
}
qc = ata_qc_from_tag(ap, ap->link.active_tag);
if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING)) &&
(qc->flags & ATA_QCFLAG_ACTIVE))
if (qc && !(qc->tf.flags & ATA_TFLAG_POLLING))
octeon_cf_dma_finished(ap, qc);
out:
spin_unlock_irqrestore(&host->lock, flags);
Expand Down
66 changes: 30 additions & 36 deletions drivers/ata/pdc_adma.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,8 +442,6 @@ static inline unsigned int adma_intr_pkt(struct ata_host *host)
continue;
handled = 1;
adma_enter_reg_mode(ap);
if (ap->flags & ATA_FLAG_DISABLED)
continue;
pp = ap->private_data;
if (!pp || pp->state != adma_state_pkt)
continue;
Expand Down Expand Up @@ -484,42 +482,38 @@ static inline unsigned int adma_intr_mmio(struct ata_host *host)
unsigned int handled = 0, port_no;

for (port_no = 0; port_no < host->n_ports; ++port_no) {
struct ata_port *ap;
ap = host->ports[port_no];
if (ap && (!(ap->flags & ATA_FLAG_DISABLED))) {
struct ata_queued_cmd *qc;
struct adma_port_priv *pp = ap->private_data;
if (!pp || pp->state != adma_state_mmio)
struct ata_port *ap = host->ports[port_no];
struct adma_port_priv *pp = ap->private_data;
struct ata_queued_cmd *qc;

if (!pp || pp->state != adma_state_mmio)
continue;
qc = ata_qc_from_tag(ap, ap->link.active_tag);
if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING))) {

/* check main status, clearing INTRQ */
u8 status = ata_sff_check_status(ap);
if ((status & ATA_BUSY))
continue;
qc = ata_qc_from_tag(ap, ap->link.active_tag);
if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING))) {

/* check main status, clearing INTRQ */
u8 status = ata_sff_check_status(ap);
if ((status & ATA_BUSY))
continue;
DPRINTK("ata%u: protocol %d (dev_stat 0x%X)\n",
ap->print_id, qc->tf.protocol, status);

/* complete taskfile transaction */
pp->state = adma_state_idle;
qc->err_mask |= ac_err_mask(status);
if (!qc->err_mask)
ata_qc_complete(qc);
else {
struct ata_eh_info *ehi =
&ap->link.eh_info;
ata_ehi_clear_desc(ehi);
ata_ehi_push_desc(ehi,
"status 0x%02X", status);

if (qc->err_mask == AC_ERR_DEV)
ata_port_abort(ap);
else
ata_port_freeze(ap);
}
handled = 1;
DPRINTK("ata%u: protocol %d (dev_stat 0x%X)\n",
ap->print_id, qc->tf.protocol, status);

/* complete taskfile transaction */
pp->state = adma_state_idle;
qc->err_mask |= ac_err_mask(status);
if (!qc->err_mask)
ata_qc_complete(qc);
else {
struct ata_eh_info *ehi = &ap->link.eh_info;
ata_ehi_clear_desc(ehi);
ata_ehi_push_desc(ehi, "status 0x%02X", status);

if (qc->err_mask == AC_ERR_DEV)
ata_port_abort(ap);
else
ata_port_freeze(ap);
}
handled = 1;
}
}
return handled;
Expand Down
17 changes: 3 additions & 14 deletions drivers/ata/sata_inic162x.c
Original file line number Diff line number Diff line change
Expand Up @@ -415,22 +415,11 @@ static irqreturn_t inic_interrupt(int irq, void *dev_instance)

spin_lock(&host->lock);

for (i = 0; i < NR_PORTS; i++) {
struct ata_port *ap = host->ports[i];

if (!(host_irq_stat & (HIRQ_PORT0 << i)))
continue;

if (likely(ap && !(ap->flags & ATA_FLAG_DISABLED))) {
inic_host_intr(ap);
for (i = 0; i < NR_PORTS; i++)
if (host_irq_stat & (HIRQ_PORT0 << i)) {
inic_host_intr(host->ports[i]);
handled++;
} else {
if (ata_ratelimit())
dev_printk(KERN_ERR, host->dev, "interrupt "
"from disabled port %d (0x%x)\n",
i, host_irq_stat);
}
}

spin_unlock(&host->lock);

Expand Down
18 changes: 4 additions & 14 deletions drivers/ata/sata_mv.c
Original file line number Diff line number Diff line change
Expand Up @@ -2355,13 +2355,9 @@ static struct ata_queued_cmd *mv_get_active_qc(struct ata_port *ap)
if (pp->pp_flags & MV_PP_FLAG_NCQ_EN)
return NULL;
qc = ata_qc_from_tag(ap, ap->link.active_tag);
if (qc) {
if (qc->tf.flags & ATA_TFLAG_POLLING)
qc = NULL;
else if (!(qc->flags & ATA_QCFLAG_ACTIVE))
qc = NULL;
}
return qc;
if (qc && !(qc->tf.flags & ATA_TFLAG_POLLING))
return qc;
return NULL;
}

static void mv_pmp_error_handler(struct ata_port *ap)
Expand Down Expand Up @@ -2546,9 +2542,7 @@ static void mv_unexpected_intr(struct ata_port *ap, int edma_was_enabled)
char *when = "idle";

ata_ehi_clear_desc(ehi);
if (ap->flags & ATA_FLAG_DISABLED) {
when = "disabled";
} else if (edma_was_enabled) {
if (edma_was_enabled) {
when = "EDMA enabled";
} else {
struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->link.active_tag);
Expand Down Expand Up @@ -2782,10 +2776,6 @@ static void mv_port_intr(struct ata_port *ap, u32 port_cause)
struct mv_port_priv *pp;
int edma_was_enabled;

if (ap->flags & ATA_FLAG_DISABLED) {
mv_unexpected_intr(ap, 0);
return;
}
/*
* Grab a snapshot of the EDMA_EN flag setting,
* so that we have a consistent view for this port,
Expand Down
Loading

0 comments on commit 3e4ec34

Please sign in to comment.