Re: [PATCH v3] libata: support the ata host which implements a queue depth less than 32

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

On 11-07-2014 10:10, 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 ~ 31 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. So consider the queue depth
implemented by the hardware when requesting a command tag.

Fixes: 8a4aeec8d2d6 ("libata/ahci: accommodate tag ordered controllers")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Kevin Hao <haokexin@xxxxxxxxx>
---
v3: Use ap->scsi_host->can_queue for the queue depth implemented by hardware.
     Patch 2 in v2 is also dropped due to this change.

v2: Remove the changes for the ata tag helper functions

Hi Tejun,

I didn't get explicit objection for the codes at http://marc.info/?l=linux-ide&m=140478830920334&w=2
So I assume that you are OK wit it. The code in this patch is the same as that,
just add the commit log.

  drivers/ata/libata-core.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 8f3043165048..4792fea79acf 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4728,14 +4728,17 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words)
  static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap)
  {
  	struct ata_queued_cmd *qc = NULL;
-	unsigned int i, tag;
+	unsigned int i, tag, max_queue;
+
+	max_queue = ap->scsi_host->can_queue;
+	WARN_ON_ONCE(max_queue > ATA_MAX_QUEUE);

  	/* no command while frozen */
  	if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
  		return NULL;

-	for (i = 0; i < ATA_MAX_QUEUE; i++) {
-		tag = (i + ap->last_tag + 1) % ATA_MAX_QUEUE;
+	for (i = 0, tag = ap->last_tag + 1; i < max_queue; i++, tag++) {
+		tag = tag < max_queue ? tag : 0;

   Assigning 'tag' back to 'tag' is quite stupid, don't you think? Why not:

		if (tag >= max_queue)
			tag = 0;

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux