RE: [PATCH] sata_fsl: add workaround for data length mismatch on freescale V2 controller
> -Original Message-
> From: David Laight [mailto:david.lai...@aculab.com]
> Sent: Tuesday, September 04, 2012 10:51 PM
> To: Xie Shaohui-B21989; jgar...@pobox.com; linux-...@vger.kernel.org
> Cc: linuxppc-...@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
>
> > + /* 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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.
I also wonder it this is worthy of an actual quirk?
Might be worth doing anyway.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] sata_fsl: add workaround for data length mismatch on freescale V2 controller
> The freescale V2 SATA controller checks > if the received data length matches > the programmed length 'ttl', if not, > it assumes that this is an error. ... Can you tell us exactly what "The freescale V2 SATA controller" is, and what versions of what devices contain it? Thanks, Clive -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] sata_fsl: add workaround for data length mismatch on freescale V2 controller
The freescale V2 SATA controller checks if the received data length matches the programmed length 'ttl', if not, it assumes that this is an error. ... Can you tell us exactly what The freescale V2 SATA controller is, and what versions of what devices contain it? Thanks, Clive -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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.
I also wonder it this is worthy of an actual quirk?
Might be worth doing anyway.
David
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] sata_fsl: add workaround for data length mismatch on freescale V2 controller
-Original Message-
From: David Laight [mailto:david.lai...@aculab.com]
Sent: Tuesday, September 04, 2012 10:51 PM
To: Xie Shaohui-B21989; jgar...@pobox.com; linux-...@vger.kernel.org
Cc: linuxppc-...@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
+ /* 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
