Patchwork libata-sff: Reenable Port Multiplier after libata-sff remodeling.

login
register
mail settings
Submitter Gwendal Grignou
Date Aug. 29, 2010, 12:31 a.m.
Message ID <1283041897-4378-1-git-send-email-gwendal@google.com>
Download mbox | patch
Permalink /patch/62929/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Gwendal Grignou - Aug. 29, 2010, 12:31 a.m.
Re-introduce port_task_data to hold the link on the which the current
request is in progress. It allows support of links behind port multiplier.

Not all libata-sff is PMP compliant. Code for native BMDMA controller
does not take in accound PMP.

Tested on Marvell 7042 and Sil7526.

Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 drivers/ata/libata-sff.c |   25 ++++++++++++++++---------
 drivers/ata/sata_mv.c    |    2 +-
 include/linux/libata.h   |    4 +++-
 3 files changed, 20 insertions(+), 11 deletions(-)
Tejun Heo - Aug. 29, 2010, 9:05 a.m.
Hello,

On 08/29/2010 02:31 AM, Gwendal Grignou wrote:
> Re-introduce port_task_data to hold the link on the which the current
> request is in progress. It allows support of links behind port multiplier.
> 
> Not all libata-sff is PMP compliant. Code for native BMDMA controller
> does not take in accound PMP.
> 
> Tested on Marvell 7042 and Sil7526.

So sata_mv is using SFF state machine for PMP devices too.  Didn't see
that coming.  Thanks for fixing this.  Can you please consider the
followings?

* Make ata_sff_queue_pio_task() take a @qc (or @link if @qc can't be
  used for whatever reason) and @delay, instead of taking @ap, @link
  and @delay.

* Change ata_port->port_task_data to something more specific.  ie.
  sff_pio_task_qc or sff_pio_task_link.

Thanks.
Gwendal Grignou - Aug. 30, 2010, 5:19 p.m.
On Sun, Aug 29, 2010 at 2:05 AM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On 08/29/2010 02:31 AM, Gwendal Grignou wrote:
>> Re-introduce port_task_data to hold the link on the which the current
>> request is in progress. It allows support of links behind port multiplier.
>>
>> Not all libata-sff is PMP compliant. Code for native BMDMA controller
>> does not take in accound PMP.
>>
>> Tested on Marvell 7042 and Sil7526.
>
> So sata_mv is using SFF state machine for PMP devices too.  Didn't see
> that coming.  Thanks for fixing this.  Can you please consider the
> followings?
>
> * Make ata_sff_queue_pio_task() take a @qc (or @link if @qc can't be
>  used for whatever reason) and @delay, instead of taking @ap, @link
>  and @delay.
Done. I keep link, because qc could be reused if the command timeout
while the task is delayed.
>
> * Change ata_port->port_task_data to something more specific.  ie.
>  sff_pio_task_qc or sff_pio_task_link.
Done.

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

Patch

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 3b82d8e..70c0e16 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1042,7 +1042,8 @@  static void ata_hsm_qc_complete(struct ata_queued_cmd *qc, int in_wq)
 int ata_sff_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,
 		     u8 status, int in_wq)
 {
-	struct ata_eh_info *ehi = &ap->link.eh_info;
+	struct ata_link *link = qc->dev->link;
+	struct ata_eh_info *ehi = &link->eh_info;
 	unsigned long flags = 0;
 	int poll_next;
 
@@ -1298,8 +1299,11 @@  fsm_start:
 }
 EXPORT_SYMBOL_GPL(ata_sff_hsm_move);
 
-void ata_sff_queue_pio_task(struct ata_port *ap, unsigned long delay)
+void ata_sff_queue_pio_task(struct ata_port *ap, void *data,
+			    unsigned long delay)
 {
+	ap->port_task_data = data;
+
 	/* may fail if ata_sff_flush_pio_task() in progress */
 	queue_delayed_work(ata_sff_wq, &ap->sff_pio_task,
 			   msecs_to_jiffies(delay));
@@ -1321,12 +1325,13 @@  static void ata_sff_pio_task(struct work_struct *work)
 {
 	struct ata_port *ap =
 		container_of(work, struct ata_port, sff_pio_task.work);
+	struct ata_link *link = ap->port_task_data;
 	struct ata_queued_cmd *qc;
 	u8 status;
 	int poll_next;
 
 	/* qc can be NULL if timeout occurred */
-	qc = ata_qc_from_tag(ap, ap->link.active_tag);
+	qc = ata_qc_from_tag(ap, link->active_tag);
 	if (!qc)
 		return;
 
@@ -1345,7 +1350,7 @@  fsm_start:
 		msleep(2);
 		status = ata_sff_busy_wait(ap, ATA_BUSY, 10);
 		if (status & ATA_BUSY) {
-			ata_sff_queue_pio_task(ap, ATA_SHORT_PAUSE);
+			ata_sff_queue_pio_task(ap, link, ATA_SHORT_PAUSE);
 			return;
 		}
 	}
@@ -1376,6 +1381,7 @@  fsm_start:
 unsigned int ata_sff_qc_issue(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
+	struct ata_link *link = qc->dev->link;
 
 	/* Use polling pio if the LLD doesn't handle
 	 * interrupt driven pio and atapi CDB interrupt.
@@ -1396,7 +1402,7 @@  unsigned int ata_sff_qc_issue(struct ata_queued_cmd *qc)
 		ap->hsm_task_state = HSM_ST_LAST;
 
 		if (qc->tf.flags & ATA_TFLAG_POLLING)
-			ata_sff_queue_pio_task(ap, 0);
+			ata_sff_queue_pio_task(ap, link, 0);
 
 		break;
 
@@ -1409,7 +1415,7 @@  unsigned int ata_sff_qc_issue(struct ata_queued_cmd *qc)
 		if (qc->tf.flags & ATA_TFLAG_WRITE) {
 			/* PIO data out protocol */
 			ap->hsm_task_state = HSM_ST_FIRST;
-			ata_sff_queue_pio_task(ap, 0);
+			ata_sff_queue_pio_task(ap, link, 0);
 
 			/* always send first data block using the
 			 * ata_sff_pio_task() codepath.
@@ -1419,7 +1425,7 @@  unsigned int ata_sff_qc_issue(struct ata_queued_cmd *qc)
 			ap->hsm_task_state = HSM_ST;
 
 			if (qc->tf.flags & ATA_TFLAG_POLLING)
-				ata_sff_queue_pio_task(ap, 0);
+				ata_sff_queue_pio_task(ap, link, 0);
 
 			/* if polling, ata_sff_pio_task() handles the
 			 * rest.  otherwise, interrupt handler takes
@@ -1441,7 +1447,7 @@  unsigned int ata_sff_qc_issue(struct ata_queued_cmd *qc)
 		/* send cdb by polling if no cdb interrupt */
 		if ((!(qc->dev->flags & ATA_DFLAG_CDB_INTR)) ||
 		    (qc->tf.flags & ATA_TFLAG_POLLING))
-			ata_sff_queue_pio_task(ap, 0);
+			ata_sff_queue_pio_task(ap, link, 0);
 		break;
 
 	default:
@@ -2734,6 +2740,7 @@  EXPORT_SYMBOL_GPL(ata_bmdma_dumb_qc_prep);
 unsigned int ata_bmdma_qc_issue(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
+	struct ata_link *link = qc->dev->link;
 
 	/* defer PIO handling to sff_qc_issue */
 	if (!ata_is_dma(qc->tf.protocol))
@@ -2762,7 +2769,7 @@  unsigned int ata_bmdma_qc_issue(struct ata_queued_cmd *qc)
 
 		/* send cdb by polling if no cdb interrupt */
 		if (!(qc->dev->flags & ATA_DFLAG_CDB_INTR))
-			ata_sff_queue_pio_task(ap, 0);
+			ata_sff_queue_pio_task(ap, link, 0);
 		break;
 
 	default:
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index 8198259..1538d5b 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -2284,7 +2284,7 @@  static unsigned int mv_qc_issue_fis(struct ata_queued_cmd *qc)
 	}
 
 	if (qc->tf.flags & ATA_TFLAG_POLLING)
-		ata_sff_queue_pio_task(ap, 0);
+		ata_sff_queue_pio_task(ap, link, 0);
 	return 0;
 }
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index f010f18..1939d92 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -723,6 +723,7 @@  struct ata_port {
 	struct ata_ioports	ioaddr;	/* ATA cmd/ctl/dma register blocks */
 	u8			ctl;	/* cache of ATA control register */
 	u8			last_ctl;	/* Cache last written value */
+	void*			port_task_data;
 	struct delayed_work	sff_pio_task;
 #ifdef CONFIG_ATA_BMDMA
 	struct ata_bmdma_prd	*bmdma_prd;	/* BMDMA SG list */
@@ -1594,7 +1595,8 @@  extern void ata_sff_irq_on(struct ata_port *ap);
 extern void ata_sff_irq_clear(struct ata_port *ap);
 extern int ata_sff_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,
 			    u8 status, int in_wq);
-extern void ata_sff_queue_pio_task(struct ata_port *ap, unsigned long delay);
+extern void ata_sff_queue_pio_task(struct ata_port *ap, void* data,
+				   unsigned long delay);
 extern unsigned int ata_sff_qc_issue(struct ata_queued_cmd *qc);
 extern bool ata_sff_qc_fill_rtf(struct ata_queued_cmd *qc);
 extern unsigned int ata_sff_port_intr(struct ata_port *ap,