linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] libata: cosmetic clean up in ata_eh_reset()
@ 2007-10-24  6:21 Tejun Heo
  2007-10-25  6:02 ` Jeff Garzik
  0 siblings, 1 reply; 2+ messages in thread
From: Tejun Heo @ 2007-10-24  6:21 UTC (permalink / raw)
  To: Jeff Garzik, linux-ide

Local variable @action usage in ata_eh_reset() is a bit confusing.
It's used only to cache ehc->i.action to test reset masks after
clearing it; however, due to the generic name "action", it's easy to
misinterpret the local variable as containing the selected reset
method later.  Also, the reason for caching the original value is easy
to miss.

This patch renames @action to @tmp_action and make it buffer newly
selected value instead to improve readability.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-eh.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 93e2b54..8cb35bb 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2071,7 +2071,7 @@ int ata_eh_reset(struct ata_link *link, int classify,
 	int try = 0;
 	struct ata_device *dev;
 	unsigned long deadline;
-	unsigned int action;
+	unsigned int tmp_action;
 	ata_reset_fn_t reset;
 	unsigned long flags;
 	int rc;
@@ -2086,14 +2086,14 @@ int ata_eh_reset(struct ata_link *link, int classify,
 	/* Determine which reset to use and record in ehc->i.action.
 	 * prereset() may examine and modify it.
 	 */
-	action = ehc->i.action;
-	ehc->i.action &= ~ATA_EH_RESET_MASK;
 	if (softreset && (!hardreset || (!(link->flags & ATA_LFLAG_NO_SRST) &&
 					 !sata_set_spd_needed(link) &&
-					 !(action & ATA_EH_HARDRESET))))
-		ehc->i.action |= ATA_EH_SOFTRESET;
+					 !(ehc->i.action & ATA_EH_HARDRESET))))
+		tmp_action = ATA_EH_SOFTRESET;
 	else
-		ehc->i.action |= ATA_EH_HARDRESET;
+		tmp_action = ATA_EH_HARDRESET;
+
+	ehc->i.action = (ehc->i.action & ~ATA_EH_RESET_MASK) | tmp_action;
 
 	if (prereset) {
 		rc = prereset(link, jiffies + ATA_EH_PRERESET_TIMEOUT);

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] libata: cosmetic clean up in ata_eh_reset()
  2007-10-24  6:21 [PATCH] libata: cosmetic clean up in ata_eh_reset() Tejun Heo
@ 2007-10-25  6:02 ` Jeff Garzik
  0 siblings, 0 replies; 2+ messages in thread
From: Jeff Garzik @ 2007-10-25  6:02 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide

Tejun Heo wrote:
> Local variable @action usage in ata_eh_reset() is a bit confusing.
> It's used only to cache ehc->i.action to test reset masks after
> clearing it; however, due to the generic name "action", it's easy to
> misinterpret the local variable as containing the selected reset
> method later.  Also, the reason for caching the original value is easy
> to miss.
> 
> This patch renames @action to @tmp_action and make it buffer newly
> selected value instead to improve readability.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  drivers/ata/libata-eh.c |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

applied



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-10-25  6:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-24  6:21 [PATCH] libata: cosmetic clean up in ata_eh_reset() Tejun Heo
2007-10-25  6:02 ` Jeff Garzik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).