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

[Xie Shaohui-B21989]

> -----Original Message-----
> From: David Laight [mailto:David.Laight at ACULAB.COM]
> Sent: Tuesday, September 04, 2012 10:51 PM
> To: Xie Shaohui-B21989; jgarzik at pobox.com; linux-ide at vger.kernel.org
> Cc: linuxppc-dev at lists.ozlabs.org; linux-kernel at vger.kernel.org; Bhartiya
> Anju-B07263
> Subject: RE: [PATCH] sata_fsl: add workaround for data length mismatch on
> freescale V2 controller
> 
> > +	/* Read command completed register */
> > +	done_mask = ioread32(hcr_base + CC);
> > +
> > +	if (host_priv->quirks & SATA_FSL_QUIRK_V2_ERRATA) {
> > +		if (unlikely(hstatus & INT_ON_DATA_LENGTH_MISMATCH)) {
> > +			for (tag = 0; tag < ATA_MAX_QUEUE; tag++) {
> > +				qc = ata_qc_from_tag(ap, tag);
> > +				if (qc && ata_is_atapi(qc->tf.protocol))
> {
> > +					atapi_flag = 1;
> > +					break;
> > +				}
> > +			}
> > +		}
> > +	}
> > +
> > +	/* Workaround for data length mismatch errata */
> > +	if (atapi_flag) {
> 
> Seems to me like the conditionals for this code are all in the wrong
> order - adding code to the normal path.
> 
> 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 = ioread32(hcr_base + CC);" before these codes,
 because these codes will clean command completed register.

> 
> 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 code only handles this errata.

Best Regards, 
Shaohui Xie