public inbox for linux-ide@vger.kernel.org
 help / color / mirror / Atom feed
From: Sergey Shtylyov <s.shtylyov@omp.ru>
To: Damien Le Moal <damien.lemoal@opensource.wdc.com>,
	<linux-ide@vger.kernel.org>
Subject: [PATCH v2 2/2] ata: libata-sff: refactor ata_sff_altstatus()
Date: Sun, 13 Feb 2022 18:10:32 +0300	[thread overview]
Message-ID: <20220213151032.4694-3-s.shtylyov@omp.ru> (raw)
In-Reply-To: <20220213151032.4694-1-s.shtylyov@omp.ru>

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@omp.ru>
---
 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


  parent reply	other threads:[~2022-02-13 15:10 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-13 15:10 [PATCH 0/2] Refactor the accessors for the ATA device control and alternate status registers Sergey Shtylyov
2022-02-13 15:10 ` [PATCH v2 1/2] ata: libata-sff: refactor ata_sff_set_devctl() Sergey Shtylyov
2022-02-13 23:25   ` Damien Le Moal
2022-02-14  8:37     ` Sergey Shtylyov
2022-02-13 15:10 ` Sergey Shtylyov [this message]
2022-02-17 12:12   ` [PATCH v2 2/2] ata: libata-sff: refactor ata_sff_altstatus() Dan Carpenter
2022-02-17 16:02     ` Sergey Shtylyov
2022-02-18 22:58       ` Damien Le Moal
2022-02-18 22:59         ` Damien Le Moal
2022-02-16  7:20 ` [PATCH 0/2] Refactor the accessors for the ATA device control and alternate status registers Damien Le Moal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220213151032.4694-3-s.shtylyov@omp.ru \
    --to=s.shtylyov@omp.ru \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=linux-ide@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox