RE: [PATCH] sata_fsl: add workaround for data length mismatch on freescale V2 controller

-----Original Message-----
From: David Laight [mailto:David.Laight@ACULAB.COM]
Sent: Tuesday, September 04, 2012 10:51 PM
To: Xie Shaohui-B21989; jgarzik@pobox.com; linux-ide@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Bhartiya
Anju-B07263
Subject: RE: [PATCH] sata_fsl: add workaround for data length mismatch on
freescale V2 controller
=20

quoted

+	/* Read command completed register */
+	done_mask =3D ioread32(hcr_base + CC);
+
+	if (host_priv->quirks & SATA_FSL_QUIRK_V2_ERRATA) {
+		if (unlikely(hstatus & INT_ON_DATA_LENGTH_MISMATCH)) {
+			for (tag =3D 0; tag < ATA_MAX_QUEUE; tag++) {
+				qc =3D ata_qc_from_tag(ap, tag);
+				if (qc && ata_is_atapi(qc->tf.protocol))

{

quoted

+					atapi_flag =3D 1;
+					break;
+				}
+			}
+		}
+	}
+
+	/* Workaround for data length mismatch errata */
+	if (atapi_flag) {

=20
Seems to me like the conditionals for this code are all in the wrong
order - adding code to the normal path.
=20
The whole lot should probably be inside:
	if (unlikely(hstatus & INT_ON_DATA_LENGTH_MISMATCH)) { and the
'atapi_flag' boolean removed.

[S.H] OK. But I need to move the "done_mask =3D ioread32(hcr_base + CC);" b=
efore these codes,
 because these codes will clean command completed register.

=20
I also wonder it this is worthy of an actual quirk?
Might be worth doing anyway.

[S.H] The quirk is useful for our internal use(there is another errata but =
got fixed by silicon upgrade),
but you are right it's worth doing anyway in upstream, since the upstream c=
ode only handles this errata.

Best Regards,=20
Shaohui Xie