The driver's calls to ata_sff_altstatus() are mostly surrounded by some clumsy checks. Refactor ata_sff_altstatus() to include the repetitive checks and return a 'bool' result indicating if the alternate status register exists or not. While at it, further update the 'kernel-doc' comment -- the alternate status register has never been a part of the taskfile, despite what Jeff and co. think! :-) Signed-off-by: Sergey Shtylyov <s.shtylyov@xxxxxx> --- drivers/ata/libata-sff.c | 56 ++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c index 3fb5bd4de50c..fc8915964919 100644 --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -70,22 +70,35 @@ EXPORT_SYMBOL_GPL(ata_sff_check_status); /** * ata_sff_altstatus - Read device alternate status reg * @ap: port where the device is + * @status: pointer to a status value * - * Reads ATA taskfile alternate status register for - * currently-selected device and return its value. + * Reads ATA alternate status register for currently-selected device + * and return its value. * - * Note: may NOT be used as the check_altstatus() entry in - * ata_port_operations. + * RETURN: + * true if the register exists, false if not. * * LOCKING: * Inherited from caller. */ -static u8 ata_sff_altstatus(struct ata_port *ap) +static bool ata_sff_altstatus(struct ata_port *ap, u8 *status) { - if (ap->ops->sff_check_altstatus) - return ap->ops->sff_check_altstatus(ap); + u8 tmp; - return ioread8(ap->ioaddr.altstatus_addr); + if (ap->ops->sff_check_altstatus) { + tmp = ap->ops->sff_check_altstatus(ap); + goto read; + } + if (ap->ioaddr.altstatus_addr) { + tmp = ioread8(ap->ioaddr.altstatus_addr); + goto read; + } + return false; + +read: + if (status) + *status = tmp; + return true; } /** @@ -104,12 +117,9 @@ static u8 ata_sff_irq_status(struct ata_port *ap) { u8 status; - if (ap->ops->sff_check_altstatus || ap->ioaddr.altstatus_addr) { - status = ata_sff_altstatus(ap); - /* Not us: We are busy */ - if (status & ATA_BUSY) - return status; - } + /* Not us: We are busy */ + if (ata_sff_altstatus(ap, &status) && (status & ATA_BUSY)) + return status; /* Clear INTRQ latch */ status = ap->ops->sff_check_status(ap); return status; @@ -129,10 +139,7 @@ static u8 ata_sff_irq_status(struct ata_port *ap) static void ata_sff_sync(struct ata_port *ap) { - if (ap->ops->sff_check_altstatus) - ap->ops->sff_check_altstatus(ap); - else if (ap->ioaddr.altstatus_addr) - ioread8(ap->ioaddr.altstatus_addr); + ata_sff_altstatus(ap, NULL); } /** @@ -164,12 +171,12 @@ EXPORT_SYMBOL_GPL(ata_sff_pause); void ata_sff_dma_pause(struct ata_port *ap) { - if (ap->ops->sff_check_altstatus || ap->ioaddr.altstatus_addr) { - /* An altstatus read will cause the needed delay without - messing up the IRQ status */ - ata_sff_altstatus(ap); + /* + * An altstatus read will cause the needed delay without + * messing up the IRQ status + */ + if (ata_sff_altstatus(ap, NULL)) return; - } /* There are no DMA controllers without ctl. BUG here to ensure we never violate the HDMA1:0 transition timing and risk corruption. */ @@ -1637,8 +1644,7 @@ void ata_sff_lost_interrupt(struct ata_port *ap) return; /* See if the controller thinks it is still busy - if so the command isn't a lost IRQ but is still in progress */ - status = ata_sff_altstatus(ap); - if (status & ATA_BUSY) + if (ata_sff_altstatus(ap, &status) && (status & ATA_BUSY)) return; /* There was a command running, we are no longer busy and we have -- 2.26.3