ata: libata-sff: refactor ata_sff_set_devctl()

Commit 41dec29bcb05 ("libata: introduce sff_set_devctl() method") left some
clumsy checks surrounding calls to ata_sff_set_devctl() which Jeff Garzik
suggested to factor out...  and I never followed up. :-(

At last, refactor ata_sff_set_devctl() to include the repetitive checks and
return a 'bool' result indicating if the device control register exists or
not.

While at it, further update the 'kernel-doc' comment -- the device control
register has never been a part of the taskfile, despite what Jeff and co.
think! :-)

Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index f5e4cd046055e7374cb7fccb6aceb1695040240f..aefe8af5cdacf17d65c4956269b00c5ab47e2d91 100644 (file)
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -265,20 +265,26 @@ EXPORT_SYMBOL_GPL(ata_sff_wait_ready);
  *     @ap: port where the device is
  *     @ctl: value to write
  *
- *     Writes ATA taskfile device control register.
+ *     Writes ATA device control register.
  *
- *     Note: may NOT be used as the sff_set_devctl() entry in
- *     ata_port_operations.
+ *     RETURN:
+ *     true if the register exists, false if not.
  *
  *     LOCKING:
  *     Inherited from caller.
  */
-static void ata_sff_set_devctl(struct ata_port *ap, u8 ctl)
+static bool ata_sff_set_devctl(struct ata_port *ap, u8 ctl)
 {
-       if (ap->ops->sff_set_devctl)
+       if (ap->ops->sff_set_devctl) {
                ap->ops->sff_set_devctl(ap, ctl);
-       else
+               return true;
+       }
+       if (ap->ioaddr.ctl_addr) {
                iowrite8(ctl, ap->ioaddr.ctl_addr);
+               return true;
+       }
+
+       return false;
 }
 
 /**
@@ -357,8 +363,6 @@ static void ata_dev_select(struct ata_port *ap, unsigned int device,
  */
 void ata_sff_irq_on(struct ata_port *ap)
 {
-       struct ata_ioports *ioaddr = &ap->ioaddr;
-
        if (ap->ops->sff_irq_on) {
                ap->ops->sff_irq_on(ap);
                return;
@@ -367,8 +371,7 @@ void ata_sff_irq_on(struct ata_port *ap)
        ap->ctl &= ~ATA_NIEN;
        ap->last_ctl = ap->ctl;
 
-       if (ap->ops->sff_set_devctl || ioaddr->ctl_addr)
-               ata_sff_set_devctl(ap, ap->ctl);
+       ata_sff_set_devctl(ap, ap->ctl);
        ata_wait_idle(ap);
 
        if (ap->ops->sff_irq_clear)
@@ -1662,8 +1665,7 @@ void ata_sff_freeze(struct ata_port *ap)
        ap->ctl |= ATA_NIEN;
        ap->last_ctl = ap->ctl;
 
-       if (ap->ops->sff_set_devctl || ap->ioaddr.ctl_addr)
-               ata_sff_set_devctl(ap, ap->ctl);
+       ata_sff_set_devctl(ap, ap->ctl);
 
        /* Under certain circumstances, some controllers raise IRQ on
         * ATA_NIEN manipulation.  Also, many controllers fail to mask
@@ -2061,10 +2063,8 @@ void ata_sff_postreset(struct ata_link *link, unsigned int *classes)
                return;
 
        /* set up device control */
-       if (ap->ops->sff_set_devctl || ap->ioaddr.ctl_addr) {
-               ata_sff_set_devctl(ap, ap->ctl);
+       if (ata_sff_set_devctl(ap, ap->ctl))
                ap->last_ctl = ap->ctl;
-       }
 }
 EXPORT_SYMBOL_GPL(ata_sff_postreset);