libata: cleanup SAT error translation

- Remove duplicate Medium Error Entry. - Fix translations to match SAT2 translation table. - Remove warning messages when translation is not found when decoding error or status register. - Goes through status register decoding when only ABRT bit is set in error register. Tested: When a disk fails, it sets Status = 0x71 [DRDY DF ERR] , Error = 0x4 [ABRT] This patch will make the sense key HARDWARE_ERROR instead. When there is a simple command syntax error: Status = 0x51 [DRDY ERR] , Error = 0x4 [ABRT] The sense key remains ABORTED_COMMAND. tj: Some updates to the description and comments. Signed-off-by: Gwendal Grignou <gwendal@google.com> Signed-off-by: Tejun Heo <tj@kernel.org>

libata: cleanup SAT error translation
- Remove duplicate Medium Error Entry. - Fix translations to match SAT2 translation table. - Remove warning messages when translation is not found when decoding error or status register. - Goes through status register decoding when only ABRT bit is set in error register. Tested: When a disk fails, it sets Status = 0x71 [DRDY DF ERR] , Error = 0x4 [ABRT] This patch will make the sense key HARDWARE_ERROR instead. When there is a simple command syntax error: Status = 0x51 [DRDY ERR] , Error = 0x4 [ABRT] The sense key remains ABORTED_COMMAND. tj: Some updates to the description and comments. Signed-off-by: Gwendal Grignou <gwendal@google.com> Signed-off-by: Tejun Heo <tj@kernel.org>
Gwendal Grignou · Tejun Heo
1 parent 1e8f5f761c
Showing 1 changed file with 17 additions and 20 deletions Side-by-side Diff
drivers/ata/libata-scsi.c
@@ -849,25 +849,24 @@
 		/*  Bad address mark */
 		{0x01, 		MEDIUM_ERROR, 0x13, 0x00}, 	// Address mark not found       Address mark not found for data field
 		/* TRK0 */
-		{0x02, 		HARDWARE_ERROR, 0x00, 0x00}, 	// Track 0 not found		  Hardware error
-		/* Abort & !ICRC */
-		{0x04, 		ABORTED_COMMAND, 0x00, 0x00}, 	// Aborted command              Aborted command
+		{0x02, 		HARDWARE_ERROR, 0x00, 0x00}, 	// Track 0 not found		Hardware error
+		/* Abort: 0x04 is not translated here, see below */
 		/* Media change request */
 		{0x08, 		NOT_READY, 0x04, 0x00}, 	// Media change request	  FIXME: faking offline
-		/* SRV */
-		{0x10, 		ABORTED_COMMAND, 0x14, 0x00}, 	// ID not found                 Recorded entity not found
-		/* Media change */
-		{0x08,  	NOT_READY, 0x04, 0x00}, 	// Media change		  FIXME: faking offline
+		/* SRV/IDNF */
+		{0x10, 		ILLEGAL_REQUEST, 0x21, 0x00}, 	// ID not found                 Logical address out of range
+		/* MC */
+		{0x20, 		UNIT_ATTENTION, 0x28, 0x00}, 	// Media Changed		Not ready to ready change, medium may have changed
 		/* ECC */
 		{0x40, 		MEDIUM_ERROR, 0x11, 0x04}, 	// Uncorrectable ECC error      Unrecovered read error
 		/* BBD - block marked bad */
-		{0x80, 		MEDIUM_ERROR, 0x11, 0x04}, 	// Block marked bad		  Medium error, unrecovered read error
+		{0x80, 		MEDIUM_ERROR, 0x11, 0x04}, 	// Block marked bad		Medium error, unrecovered read error
 		{0xFF, 0xFF, 0xFF, 0xFF}, // END mark
 	};
 	static const unsigned char stat_table[][4] = {
 		/* Must be first because BUSY means no other bits valid */
 		{0x80, 		ABORTED_COMMAND, 0x47, 0x00},	// Busy, fake parity for now
-		{0x20, 		HARDWARE_ERROR,  0x00, 0x00}, 	// Device fault
+		{0x20, 		HARDWARE_ERROR,  0x44, 0x00}, 	// Device fault, internal target failure
 		{0x08, 		ABORTED_COMMAND, 0x47, 0x00},	// Timed out in xfer, fake parity for now
 		{0x04, 		RECOVERED_ERROR, 0x11, 0x00},	// Recovered ECC error	  Medium error, recovered
 		{0xFF, 0xFF, 0xFF, 0xFF}, // END mark
  
@@ -892,13 +891,13 @@
 				goto translate_done;
 			}
 		}
-		/* No immediate match */
-		if (verbose)
-			printk(KERN_WARNING "ata%u: no sense translation for "
-			       "error 0x%02x\n", id, drv_err);
 	}
  
-	/* Fall back to interpreting status bits */
+	/*
+	 * Fall back to interpreting status bits.  Note that if the drv_err
+	 * has only the ABRT bit set, we decode drv_stat.  ABRT by itself
+	 * is not descriptive enough.
+	 */
 	for (i = 0; stat_table[i][0] != 0xFF; i++) {
 		if (stat_table[i][0] & drv_stat) {
 			*sk = stat_table[i][1];
  
@@ -907,13 +906,11 @@
 			goto translate_done;
 		}
 	}
-	/* No error?  Undecoded? */
-	if (verbose)
-		printk(KERN_WARNING "ata%u: no sense translation for "
-		       "status: 0x%02x\n", id, drv_stat);
  
-	/* We need a sensible error return here, which is tricky, and one
-	   that won't cause people to do things like return a disk wrongly */
+	/*
+	 * We need a sensible error return here, which is tricky, and one
+	 * that won't cause people to do things like return a disk wrongly.
+	 */
 	*sk = ABORTED_COMMAND;
 	*asc = 0x00;
 	*ascq = 0x00;
...	...	@@ -849,25 +849,24 @@
849	849	/* Bad address mark */
850	850	{0x01, MEDIUM_ERROR, 0x13, 0x00}, // Address mark not found Address mark not found for data field
851	851	/* TRK0 */
852		- {0x02, HARDWARE_ERROR, 0x00, 0x00}, // Track 0 not found Hardware error
853		- /* Abort & !ICRC */
854		- {0x04, ABORTED_COMMAND, 0x00, 0x00}, // Aborted command Aborted command
	852	+ {0x02, HARDWARE_ERROR, 0x00, 0x00}, // Track 0 not found Hardware error
	853	+ /* Abort: 0x04 is not translated here, see below */
855	854	/* Media change request */
856	855	{0x08, NOT_READY, 0x04, 0x00}, // Media change request FIXME: faking offline
857		- /* SRV */
858		- {0x10, ABORTED_COMMAND, 0x14, 0x00}, // ID not found Recorded entity not found
859		- /* Media change */
860		- {0x08, NOT_READY, 0x04, 0x00}, // Media change FIXME: faking offline
	856	+ /* SRV/IDNF */
	857	+ {0x10, ILLEGAL_REQUEST, 0x21, 0x00}, // ID not found Logical address out of range
	858	+ /* MC */
	859	+ {0x20, UNIT_ATTENTION, 0x28, 0x00}, // Media Changed Not ready to ready change, medium may have changed
861	860	/* ECC */
862	861	{0x40, MEDIUM_ERROR, 0x11, 0x04}, // Uncorrectable ECC error Unrecovered read error
863	862	/* BBD - block marked bad */
864		- {0x80, MEDIUM_ERROR, 0x11, 0x04}, // Block marked bad Medium error, unrecovered read error
	863	+ {0x80, MEDIUM_ERROR, 0x11, 0x04}, // Block marked bad Medium error, unrecovered read error
865	864	{0xFF, 0xFF, 0xFF, 0xFF}, // END mark
866	865	};
867	866	static const unsigned char stat_table[][4] = {
868	867	/* Must be first because BUSY means no other bits valid */
869	868	{0x80, ABORTED_COMMAND, 0x47, 0x00}, // Busy, fake parity for now
870		- {0x20, HARDWARE_ERROR, 0x00, 0x00}, // Device fault
	869	+ {0x20, HARDWARE_ERROR, 0x44, 0x00}, // Device fault, internal target failure
871	870	{0x08, ABORTED_COMMAND, 0x47, 0x00}, // Timed out in xfer, fake parity for now
872	871	{0x04, RECOVERED_ERROR, 0x11, 0x00}, // Recovered ECC error Medium error, recovered
873	872	{0xFF, 0xFF, 0xFF, 0xFF}, // END mark
874	873
...	...	@@ -892,13 +891,13 @@
892	891	goto translate_done;
893	892	}
894	893	}
895		- /* No immediate match */
896		- if (verbose)
897		- printk(KERN_WARNING "ata%u: no sense translation for "
898		- "error 0x%02x\n", id, drv_err);
899	894	}
900	895
901		- /* Fall back to interpreting status bits */
	896	+ /*
	897	+ * Fall back to interpreting status bits. Note that if the drv_err
	898	+ * has only the ABRT bit set, we decode drv_stat. ABRT by itself
	899	+ * is not descriptive enough.
	900	+ */
902	901	for (i = 0; stat_table[i][0] != 0xFF; i++) {
903	902	if (stat_table[i][0] & drv_stat) {
904	903	*sk = stat_table[i][1];
905	904
...	...	@@ -907,13 +906,11 @@
907	906	goto translate_done;
908	907	}
909	908	}
910		- /* No error? Undecoded? */
911		- if (verbose)
912		- printk(KERN_WARNING "ata%u: no sense translation for "
913		- "status: 0x%02x\n", id, drv_stat);
914	909
915		- /* We need a sensible error return here, which is tricky, and one
916		- that won't cause people to do things like return a disk wrongly */
	910	+ /*
	911	+ * We need a sensible error return here, which is tricky, and one
	912	+ * that won't cause people to do things like return a disk wrongly.
	913	+ */
917	914	*sk = ABORTED_COMMAND;
918	915	*asc = 0x00;
919	916	*ascq = 0x00;