On Thu, Jul 03, 2014 at 09:47:17AM -0400, Tejun Heo wrote:
> On Thu, Jul 03, 2014 at 08:09:41PM +0800, Kevin Hao wrote:
> > The sata on fsl mpc8315e is broken after the commit 8a4aeec8d2d6
> > ("libata/ahci: accommodate tag ordered controllers"). The reason is
> > that the ata controller on this SoC only implement a queue depth of
> > 16. When issuing the commands in tag order, all the commands in tag
> > 16 ~ 17 are mapped to tag 0 unconditionally and then causes the sata
> > malfunction. It makes no senses to use a 32 queue in software while
> > the hardware has less queue depth. This patch provides the function
> > for libata to adjust the queue depth for a host controller.
>
> ATA_TAG_INTERNAL is an internal way of marking an EH command and
> doesn't actually get used as a command tag. After all, EH doesn't
> issue any tagged command. We can use tag 32 for it but have been
> using 31 for it for historical reasons, so the only necessary part is
> updating the tag allocator to wrap according to the max number of tags
> supported by the controller.
Sorry I didn't quite follow. In some cases, we still need to iterate all
the commands in the queue (including the internal command). For example,
in drivers/ata/sata_fsl.c:
static void sata_fsl_host_intr(struct ata_port *ap)
/* Workaround for data length mismatch errata */
if (unlikely(hstatus & INT_ON_DATA_LENGTH_MISMATCH)) {
for (tag = 0; tag < ATA_MAX_QUEUE; tag++) {
Since the fsl sata only support a 16 command queue, if we put the internal
command to ata_port->qcmd[16 - 1], we can easily change the above code to:
static void sata_fsl_host_intr(struct ata_port *ap)
/* Workaround for data length mismatch errata */
if (unlikely(hstatus & INT_ON_DATA_LENGTH_MISMATCH)) {
for (tag = 0; tag < 16; tag++) {
But if we still put the internal command to ata_port->qcmd[31], it will
make it more complicated to iterate all the commands.
There are also several iterations of the command queue just like the above
codes in drivers/ata/libata-eh.c. They should suffer the same issue.
Did I miss something?
Thanks,
Kevin
Attachment:
pgpQ6cg81mF6w.pgp
Description: PGP signature
