[PATCH 01/12] sas_ata: Require CONFIG_ATA in Kconfig

[PATCH 01/12] sas_ata: Require CONFIG_ATA in Kconfig

[Darrick J. Wong]

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/Kconfig b/drivers/scsi/libsas/Kconfig
index aafdc92..b64e391 100644
--- a/drivers/scsi/libsas/Kconfig
+++ b/drivers/scsi/libsas/Kconfig
@@ -24,7 +24,7 @@ #
 
 config SCSI_SAS_LIBSAS
 	tristate "SAS Domain Transport Attributes"
-	depends on SCSI
+	depends on SCSI && ATA
 	select SCSI_SAS_ATTRS
 	help
 	  This provides transport specific helpers for SAS drivers which

[PATCH 02/12] sas_ata: Satisfy libata qc function locking requirements

[Darrick J. Wong]

ata_qc_complete and ata_sas_queuecmd require that the port lock be held
when they are called.  sas_ata doesn't do this, leading to BUG messages
about qc tags newly allocated qc tags already being in use.  This patch
fixes the locking, which should clean up the rest of those messages.

So far I've tested this against an IBM x206m with two SATA disks with no
BUG messages and no other signs of things going wrong, and the machine
finally passed the pounder stress test.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/sas_ata.c       |    4 ++++
 drivers/scsi/libsas/sas_scsi_host.c |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index de42b5b..0bb1a14 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -92,7 +92,9 @@ static void sas_ata_task_done(struct sas
 	struct task_status_struct *stat = &task->task_status;
 	struct ata_task_resp *resp = (struct ata_task_resp *)stat->buf;
 	enum ata_completion_errors ac;
+	unsigned long flags;
 
+	spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
 	if (stat->stat == SAS_PROTO_RESPONSE) {
 		ata_tf_from_fis(resp->ending_fis, &dev->sata_dev.tf);
 		qc->err_mask |= ac_err_mask(dev->sata_dev.tf.command);
@@ -113,6 +115,8 @@ static void sas_ata_task_done(struct sas
 	}
 
 	ata_qc_complete(qc);
+	spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);
+
 	list_del_init(&task->list);
 	sas_free_task(task);
 }
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 2cd478a..fee9c10 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -213,8 +213,12 @@ int sas_queuecommand(struct scsi_cmnd *c
 		struct sas_task *task;
 
 		if (dev_is_sata(dev)) {
+			unsigned long flags;
+
+			spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
 			res = ata_sas_queuecmd(cmd, scsi_done,
 					       dev->sata_dev.ap);
+			spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);
 			goto out;
 		}
 

[PATCH 03/12] sas_ata: sas_ata_qc_issue should return AC_ERR_*

[Darrick J. Wong]

The sas_ata_qc_issue function was incorrectly written to return error
codes such as -ENOMEM.  Since libata OR's qc->err_mask with the
return value, It is necessary to make my code return one of the
AC_ERR_ codes instead.  For now, use AC_ERR_SYSTEM because an error
here means that the OS couldn't send the command to the controller.

If anybody has a suggestion for a better AC_ERR_ code to use, please
suggest it.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/sas_ata.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 0bb1a14..46e1dbe 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -123,7 +123,7 @@ static void sas_ata_task_done(struct sas
 
 static unsigned int sas_ata_qc_issue(struct ata_queued_cmd *qc)
 {
-	int res = -ENOMEM;
+	int res;
 	struct sas_task *task;
 	struct domain_device *dev = qc->ap->private_data;
 	struct sas_ha_struct *sas_ha = dev->port->ha;
@@ -135,7 +135,7 @@ static unsigned int sas_ata_qc_issue(str
 
 	task = sas_alloc_task(GFP_ATOMIC);
 	if (!task)
-		goto out;
+		return AC_ERR_SYSTEM;
 	task->dev = dev;
 	task->task_proto = SAS_PROTOCOL_STP;
 	task->task_done = sas_ata_task_done;
@@ -187,12 +187,10 @@ static unsigned int sas_ata_qc_issue(str
 		SAS_DPRINTK("lldd_execute_task returned: %d\n", res);
 
 		sas_free_task(task);
-		if (res == -SAS_QUEUE_FULL)
-			return -ENOMEM;
+		return AC_ERR_SYSTEM;
 	}
 
-out:
-	return res;
+	return 0;
 }
 
 static u8 sas_ata_check_status(struct ata_port *ap)

[PATCH 04/12] sas_ata: ata_post_internal should abort the sas_task

[Darrick J. Wong]

This patch adds a new field, lldd_task, to ata_queued_cmd so that libata
users such as libsas can associate some data with a qc.  The particular
ambition with this patch is to associate a sas_task with a qc; that way,
if libata decides to timeout a command, we can come back (in
sas_ata_post_internal) and abort the sas task.

One question remains: Is it necessary to reset the phy on error, or will
the libata error handler take care of it?  (Assuming that one is written,
of course.)  This patch, as it is today, works well enough to clean
things up when an ATA device probe attempt fails halfway through the probe,
though I'm not sure this is always the right thing to do.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/sas_ata.c |   30 +++++++++++++++++++++++++++---
 include/linux/libata.h        |    1 +
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 46e1dbe..c8af884 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -88,12 +88,17 @@ static enum ata_completion_errors sas_to
 static void sas_ata_task_done(struct sas_task *task)
 {
 	struct ata_queued_cmd *qc = task->uldd_task;
-	struct domain_device *dev = qc->ap->private_data;
+	struct domain_device *dev;
 	struct task_status_struct *stat = &task->task_status;
 	struct ata_task_resp *resp = (struct ata_task_resp *)stat->buf;
 	enum ata_completion_errors ac;
 	unsigned long flags;
 
+	if (!qc)
+		goto qc_already_gone;
+
+	dev = qc->ap->private_data;
+
 	spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
 	if (stat->stat == SAS_PROTO_RESPONSE) {
 		ata_tf_from_fis(resp->ending_fis, &dev->sata_dev.tf);
@@ -114,9 +119,11 @@ static void sas_ata_task_done(struct sas
 		}
 	}
 
+	qc->lldd_task = NULL;
 	ata_qc_complete(qc);
 	spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);
 
+qc_already_gone:
 	list_del_init(&task->list);
 	sas_free_task(task);
 }
@@ -166,6 +173,7 @@ static unsigned int sas_ata_qc_issue(str
 	task->scatter = qc->__sg;
 	task->ata_task.retry_count = 1;
 	task->task_state_flags = SAS_TASK_STATE_PENDING;
+	qc->lldd_task = task;
 
 	switch (qc->tf.protocol) {
 	case ATA_PROT_NCQ:
@@ -237,8 +245,24 @@ static void sas_ata_post_internal(struct
 	if (qc->flags & ATA_QCFLAG_FAILED)
 		qc->err_mask |= AC_ERR_OTHER;
 
-	if (qc->err_mask)
-		SAS_DPRINTK("%s: Failure; reset phy!\n", __FUNCTION__);
+	if (qc->err_mask) {
+		/*
+		 * Find the sas_task and kill it.  By this point,
+		 * libata has decided to kill the qc, so we needn't
+		 * bother with sas_ata_task_done.  But we still
+		 * ought to abort the task.
+		 */
+		struct sas_task *task = qc->lldd_task;
+		struct domain_device *dev = qc->ap->private_data;
+
+		qc->lldd_task = NULL;
+		if (task) {
+			task->uldd_task = NULL;
+			__sas_task_abort(task);
+		}
+
+		sas_phy_reset(dev->port->phy, 1);
+	}
 }
 
 static void sas_ata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 22aa69e..fe98957 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -452,6 +452,7 @@ struct ata_queued_cmd {
 	ata_qc_cb_t		complete_fn;
 
 	void			*private_data;
+	void			*lldd_task;
 };
 
 struct ata_port_stats {

Re: [PATCH 04/12] sas_ata: ata_post_internal should abort the sas_task

[Tejun Heo]

Darrick J. Wong wrote:
> This patch adds a new field, lldd_task, to ata_queued_cmd so that libata
> users such as libsas can associate some data with a qc.  The particular
> ambition with this patch is to associate a sas_task with a qc; that way,
> if libata decides to timeout a command, we can come back (in
> sas_ata_post_internal) and abort the sas task.

Can you rename ->lldd_task to something more generic, say, 
->low/driver_private_data and add a comment to explain who owns it?

> One question remains: Is it necessary to reset the phy on error, or will
> the libata error handler take care of it?  (Assuming that one is written,
> of course.)  This patch, as it is today, works well enough to clean
> things up when an ATA device probe attempt fails halfway through the probe,
> though I'm not sure this is always the right thing to do.

->post_internal_cmd is only responsible for putting the controller into 
a sane state and perform minimal diagnosis of the error if command has 
failed (setting AC_ERR_*).  Sane states are...

1. the controller is ready for further commands, if possible.

2. the controller is not processing the failed command, if #1 is not 
possible.  IOW, DMA engine is stopped (or any other activity which pokes 
memory) and the port is frozen (IRQ muted).

libata EH will take care of resetting, revalidating / reconfiguring 
attached ATA device using the provided reset methods, so no need for 
phy_reset in post_internal_cmd.

-- 
tejun

[PATCH 1/2] sas_ata: Rename ata_queued_cmd->lldd_task to driver_data

[Darrick J. Wong]

Per Tejun's request, rename the lldd_task field and add comments about it.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/sas_ata.c |    8 ++++----
 include/linux/libata.h        |    4 +++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 2db2589..c92f4b6 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -122,7 +122,7 @@ static void sas_ata_task_done(struct sas
 		}
 	}
 
-	qc->lldd_task = NULL;
+	qc->driver_data = NULL;
 	if (qc->scsicmd)
 		ASSIGN_SAS_TASK(qc->scsicmd, NULL);
 	ata_qc_complete(qc);
@@ -192,7 +192,7 @@ static unsigned int sas_ata_qc_issue(str
 	task->scatter = qc->__sg;
 	task->ata_task.retry_count = 1;
 	task->task_state_flags = SAS_TASK_STATE_PENDING;
-	qc->lldd_task = task;
+	qc->driver_data = task;
 
 	switch (qc->tf.protocol) {
 	case ATA_PROT_NCQ:
@@ -276,10 +276,10 @@ static void sas_ata_post_internal(struct
 		 * bother with sas_ata_task_done.  But we still
 		 * ought to abort the task.
 		 */
-		struct sas_task *task = qc->lldd_task;
+		struct sas_task *task = qc->driver_data;
 		unsigned long flags;
 
-		qc->lldd_task = NULL;
+		qc->driver_data = NULL;
 		if (task) {
 			/* Should this be a AT(API) device reset? */
 			spin_lock_irqsave(&task->task_state_lock, flags);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index a20646c..a8eafc7 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -445,7 +445,9 @@ struct ata_queued_cmd {
 	ata_qc_cb_t		complete_fn;
 
 	void			*private_data;
-	void			*lldd_task;
+
+	/* This is owned by a low level libata client */
+	void			*driver_data;
 };
 
 struct ata_port_stats {

Re: [PATCH 1/2] sas_ata: Rename ata_queued_cmd->lldd_task to driver_data

[Jeff Garzik]

Darrick J. Wong wrote:
> Per Tejun's request, rename the lldd_task field and add comments about it.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
> 
>  drivers/scsi/libsas/sas_ata.c |    8 ++++----
>  include/linux/libata.h        |    4 +++-
>  2 files changed, 7 insertions(+), 5 deletions(-)

looks good to me

though please CC linux-ide on all libata patches...

	Jeff

[PATCH 2/2]: sas_ata: Don't reset the phy in post_internal_command

[Darrick J. Wong]

We don't need to reset the SAS phy in sas_ata_post_internal; all
that is necessary is to clear out the task from the SAS HA.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/sas_ata.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index c92f4b6..d91c5ba 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -281,11 +281,6 @@ static void sas_ata_post_internal(struct
 
 		qc->driver_data = NULL;
 		if (task) {
-			/* Should this be a AT(API) device reset? */
-			spin_lock_irqsave(&task->task_state_lock, flags);
-			task->task_state_flags |= SAS_TASK_NEED_DEV_RESET;
-			spin_unlock_irqrestore(&task->task_state_lock, flags);
-
 			task->uldd_task = NULL;
 			__sas_task_abort(task);
 		}

[PATCH 05/12] sas_ata: Don't copy aic94xx's sactive to ata_port

[Darrick J. Wong]

Since the aic94xx sequencer assigns its own NCQ tags to ATA commands, it
no longer makes any sense to copy the sactive field in the STP response
to ata_port->sactive, as that will confuse libata.  Also, libata seems
to be capable of managing sactive on its own.

The attached patch gets rid of one of the causes of the BUG messages in
ata_qc_new, and seems to work without problems on an IBM x206m.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/sas_ata.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index c8af884..16c3e5a 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -106,7 +106,6 @@ static void sas_ata_task_done(struct sas
 		dev->sata_dev.sstatus = resp->sstatus;
 		dev->sata_dev.serror = resp->serror;
 		dev->sata_dev.scontrol = resp->scontrol;
-		dev->sata_dev.ap->sactive = resp->sactive;
 	} else if (stat->stat != SAM_STAT_GOOD) {
 		ac = sas_to_ata_err(stat);
 		if (ac) {

[PATCH 06/12] sas_ata: Implement SATA PHY control

[Darrick J. Wong]

This patch requires "libsas: Add a sysfs knob to enable/disable a phy"
to be applied.  It hooks the SControl write function to provide basic
SATA phy control for phy enable/disable and speed limits.  Power
management is still broken, though it is unclear that libata actually
uses those SControl bits anyway.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/sas_ata.c       |   42 ++++++++++++++++++++++++++++++++++-
 drivers/scsi/libsas/sas_scsi_host.c |    1 +
 2 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 16c3e5a..2bb619e 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -270,6 +270,46 @@ static void sas_ata_tf_read(struct ata_p
 	memcpy(tf, &dev->sata_dev.tf, sizeof (*tf));
 }
 
+static void sas_ata_scontrol_write(struct domain_device *dev, u32 val)
+{
+	u32 tmp = dev->sata_dev.scontrol;
+	struct sas_phy *phy = dev->port->phy;
+
+	val &= 0x0FF; /* only set max spd and dev ctrl */
+	val |= 0x300; /* disallow host pm */
+	val |= tmp & 0xFFFFF000; /* preserve upper bits */
+
+	/* disable phy */
+	if ((val & 0x4) && !(tmp & 0x4))
+		sas_phy_enable(phy, 0);
+
+	/* enable phy */
+	if (!(val & 0x4) && (tmp & 0x4))
+		sas_phy_enable(phy, 1);
+
+	/* reset phy */
+	if ((val & 0x1) && !(tmp & 0x1))
+		sas_phy_reset(phy, 0);
+
+	/* speed limit */
+	if ((val & 0xF0) != (tmp & 0xF0)) {
+		struct sas_phy_linkrates rates = {0};
+
+		switch ((val & 0xF0) >> 4) {
+		case 0:
+		case 2:
+			rates.maximum_linkrate = SAS_LINK_RATE_3_0_GBPS;
+			break;
+		case 1:
+			rates.maximum_linkrate = SAS_LINK_RATE_1_5_GBPS;
+			break;
+		}
+		sas_set_phy_speed(phy, &rates);
+	}
+
+	dev->sata_dev.scontrol = val;
+}
+
 static void sas_ata_scr_write(struct ata_port *ap, unsigned int sc_reg_in,
 			      u32 val)
 {
@@ -281,7 +321,7 @@ static void sas_ata_scr_write(struct ata
 			dev->sata_dev.sstatus = val;
 			break;
 		case SCR_CONTROL:
-			dev->sata_dev.scontrol = val;
+			sas_ata_scontrol_write(dev, val);
 			break;
 		case SCR_ERROR:
 			dev->sata_dev.serror = val;
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index fee9c10..5b0c471 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -1040,3 +1040,4 @@ EXPORT_SYMBOL_GPL(sas_eh_device_reset_ha
 EXPORT_SYMBOL_GPL(sas_slave_alloc);
 EXPORT_SYMBOL_GPL(sas_target_destroy);
 EXPORT_SYMBOL_GPL(sas_ioctl);
+EXPORT_SYMBOL_GPL(sas_set_phy_speed);

Re: [PATCH 06/12] sas_ata: Implement SATA PHY control

[Tejun Heo]

Darrick J. Wong wrote:
> This patch requires "libsas: Add a sysfs knob to enable/disable a phy"
> to be applied.  It hooks the SControl write function to provide basic
> SATA phy control for phy enable/disable and speed limits.  Power
> management is still broken, though it is unclear that libata actually
> uses those SControl bits anyway.

I don't really think exporting SControl as-is a good idea.  Not all 
devices would act as expected if you change speed limit in SControl on 
the fly.  Also, SATA phy speed is managed by libata core layer and used 
during EH.  Please take a look at sata_down_spd_limit() and its usage.

-- 
tejun

[PATCH 07/12] libsas: Accept SAM_GOOD for ATAPI devices in sas_ata_task_done

[Darrick J. Wong]

A sas_task sent to an ATAPI devices returns SAM_GOOD if successful.
Therefore, we should treat this the same way we treat ATA commands
that succeed.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/sas_ata.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 2bb619e..20f3a5e 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -100,7 +100,7 @@ static void sas_ata_task_done(struct sas
 	dev = qc->ap->private_data;
 
 	spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
-	if (stat->stat == SAS_PROTO_RESPONSE) {
+	if (stat->stat == SAS_PROTO_RESPONSE || stat->stat == SAM_GOOD) {
 		ata_tf_from_fis(resp->ending_fis, &dev->sata_dev.tf);
 		qc->err_mask |= ac_err_mask(dev->sata_dev.tf.command);
 		dev->sata_dev.sstatus = resp->sstatus;

[PATCH 08/12] libsas: Unknown STP devices should be reported to libata as unknown.

[Darrick J. Wong]

When libsas encounters a STP device whose protocol isn't recognized (i.e.
not ATA or ATAPI), we should set the ata_device's class to ATA_DEV_UNKNOWN
instead of ATA_DEV_ATA.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/sas_ata.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 20f3a5e..7ebda69 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -232,7 +232,7 @@ static void sas_ata_phy_reset(struct ata
 			SAS_DPRINTK("%s: Unknown SATA command set: %d.\n",
 				    __FUNCTION__,
 				    dev->sata_dev.command_set);
-			ap->device[0].class = ATA_DEV_ATA;
+			ap->device[0].class = ATA_DEV_UNKNOWN;
 			break;
 	}
 

Re: [PATCH 08/12] libsas: Unknown STP devices should be reported to libata as unknown.

[Tejun Heo]

Darrick J. Wong wrote:
> When libsas encounters a STP device whose protocol isn't recognized (i.e.
> not ATA or ATAPI), we should set the ata_device's class to ATA_DEV_UNKNOWN
> instead of ATA_DEV_ATA.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
> 
>  drivers/scsi/libsas/sas_ata.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
> index 20f3a5e..7ebda69 100644
> --- a/drivers/scsi/libsas/sas_ata.c
> +++ b/drivers/scsi/libsas/sas_ata.c
> @@ -232,7 +232,7 @@ static void sas_ata_phy_reset(struct ata
>  			SAS_DPRINTK("%s: Unknown SATA command set: %d.\n",
>  				    __FUNCTION__,
>  				    dev->sata_dev.command_set);
> -			ap->device[0].class = ATA_DEV_ATA;
> +			ap->device[0].class = ATA_DEV_UNKNOWN;

ATA_DEV_UNKNOWN currently indicates 'unclassified' state and leaving 
device as unclassified after successful reset is considered failure, so 
this is a bit confusing but you need to return ATA_DEV_NONE here. 
Hmmm... maybe libata should use ATA_DEV_UNCLASSIFIED instead.

-- 
tejun

[PATCH 09/12] sas_ata: Assign sas_task to scsi_cmnd to enable EH for ATA devices

[Darrick J. Wong]

The SATL should connect the scsi_cmnd to the sas_task (despite the presence
of libata) so that requests to abort scsi_cmnds headed to the ATA device
can be processed by the EH and aborted correctly.  The abort status should
still be propagated from sas -> ata -> scsi.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/sas_ata.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 7ebda69..8111222 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -119,6 +119,8 @@ static void sas_ata_task_done(struct sas
 	}
 
 	qc->lldd_task = NULL;
+	if (qc->scsicmd)
+		ASSIGN_SAS_TASK(qc->scsicmd, NULL);
 	ata_qc_complete(qc);
 	spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);
 
@@ -184,6 +186,9 @@ static unsigned int sas_ata_qc_issue(str
 		break;
 	}
 
+	if (qc->scsicmd)
+		ASSIGN_SAS_TASK(qc->scsicmd, task);
+
 	if (sas_ha->lldd_max_execute_num < 2)
 		res = i->dft->lldd_execute_task(task, 1, GFP_ATOMIC);
 	else
@@ -193,6 +198,8 @@ static unsigned int sas_ata_qc_issue(str
 	if (res) {
 		SAS_DPRINTK("lldd_execute_task returned: %d\n", res);
 
+		if (qc->scsicmd)
+			ASSIGN_SAS_TASK(qc->scsicmd, NULL);
 		sas_free_task(task);
 		return AC_ERR_SYSTEM;
 	}

[PATCH 10/12] sas_ata: Implement sas_task_abort for ATA devices

[Darrick J. Wong]

ATA devices need special handling for sas_task_abort.  If the ATA command
came from SCSI, then we merely need to tell SCSI to abort the scsi_cmnd.
However, internal commands require a bit more work--we need to fill the qc
with the appropriate error status and complete the command, and eventually
post_internal will issue the actual ABORT TASK.
---

 drivers/scsi/libsas/sas_ata.c       |   47 +++++++++++++++++++++++++++++++++--
 drivers/scsi/libsas/sas_internal.h  |    3 ++
 drivers/scsi/libsas/sas_scsi_host.c |    8 ++++--
 include/scsi/sas_ata.h              |    2 +
 4 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 8111222..1b7221c 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -30,6 +30,8 @@ #include <scsi/scsi.h>
 #include <scsi/scsi_transport.h>
 #include <scsi/scsi_transport_sas.h>
 #include "../scsi_sas_internal.h"
+#include "../scsi_transport_api.h"
+#include <scsi/scsi_eh.h>
 
 static enum ata_completion_errors sas_to_ata_err(struct task_status_struct *ts)
 {
@@ -91,6 +93,7 @@ static void sas_ata_task_done(struct sas
 	struct domain_device *dev;
 	struct task_status_struct *stat = &task->task_status;
 	struct ata_task_resp *resp = (struct ata_task_resp *)stat->buf;
+	struct sas_ha_struct *sas_ha;
 	enum ata_completion_errors ac;
 	unsigned long flags;
 
@@ -98,6 +101,7 @@ static void sas_ata_task_done(struct sas
 		goto qc_already_gone;
 
 	dev = qc->ap->private_data;
+	sas_ha = dev->port->ha;
 
 	spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
 	if (stat->stat == SAS_PROTO_RESPONSE || stat->stat == SAM_GOOD) {
@@ -124,6 +128,20 @@ static void sas_ata_task_done(struct sas
 	ata_qc_complete(qc);
 	spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);
 
+	/*
+	 * If the sas_task has an ata qc, a scsi_cmnd and the aborted
+	 * flag is set, then we must have come in via the libsas EH
+	 * functions.  When we exit this function, we need to put the
+	 * scsi_cmnd on the list of finished errors.  The ata_qc_complete
+	 * call cleans up the libata side of things but we're protected
+	 * from the scsi_cmnd going away because the scsi_cmnd is owned
+	 * by the EH, making libata's call to scsi_done a NOP.
+	 */
+	spin_lock_irqsave(&task->task_state_lock, flags);
+	if (qc->scsicmd && task->task_state_flags & SAS_TASK_STATE_ABORTED)
+		scsi_eh_finish_cmd(qc->scsicmd, &sas_ha->eh_done_q);
+	spin_unlock_irqrestore(&task->task_state_lock, flags);
+
 qc_already_gone:
 	list_del_init(&task->list);
 	sas_free_task(task);
@@ -259,15 +277,18 @@ static void sas_ata_post_internal(struct
 		 * ought to abort the task.
 		 */
 		struct sas_task *task = qc->lldd_task;
-		struct domain_device *dev = qc->ap->private_data;
+		unsigned long flags;
 
 		qc->lldd_task = NULL;
 		if (task) {
+			/* Should this be a AT(API) device reset? */
+			spin_lock_irqsave(&task->task_state_lock, flags);
+			task->task_state_flags |= SAS_TASK_NEED_DEV_RESET;
+			spin_unlock_irqrestore(&task->task_state_lock, flags);
+
 			task->uldd_task = NULL;
 			__sas_task_abort(task);
 		}
-
-		sas_phy_reset(dev->port->phy, 1);
 	}
 }
 
@@ -409,3 +430,23 @@ int sas_ata_init_host_and_port(struct do
 
 	return 0;
 }
+
+void sas_ata_task_abort(struct sas_task *task)
+{
+	struct ata_queued_cmd *qc = task->uldd_task;
+	struct completion *waiting;
+
+	/* Bounce SCSI-initiated commands to the SCSI EH */
+	if (qc->scsicmd) {
+		scsi_req_abort_cmd(qc->scsicmd);
+		scsi_schedule_eh(qc->scsicmd->device->host);
+		return;
+	}
+
+	/* Internal command, fake a timeout and complete. */
+	qc->flags &= ~ATA_QCFLAG_ACTIVE;
+	qc->flags |= ATA_QCFLAG_FAILED;
+	qc->err_mask |= AC_ERR_TIMEOUT;
+	waiting = qc->private_data;
+	complete(waiting);
+}
diff --git a/drivers/scsi/libsas/sas_internal.h b/drivers/scsi/libsas/sas_internal.h
index a78638d..2b8213b 100644
--- a/drivers/scsi/libsas/sas_internal.h
+++ b/drivers/scsi/libsas/sas_internal.h
@@ -39,6 +39,9 @@ #else
 #define SAS_DPRINTK(fmt, ...)
 #endif
 
+#define TO_SAS_TASK(_scsi_cmd)  ((void *)(_scsi_cmd)->host_scribble)
+#define ASSIGN_SAS_TASK(_sc, _t) do { (_sc)->host_scribble = (void *) _t; } while (0)
+
 void sas_scsi_recover_host(struct Scsi_Host *shost);
 
 int sas_show_class(enum sas_class class, char *buf);
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 5b0c471..a30c0b7 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -44,9 +44,6 @@ #include <linux/libata.h>
 
 /* ---------- SCSI Host glue ---------- */
 
-#define TO_SAS_TASK(_scsi_cmd)  ((void *)(_scsi_cmd)->host_scribble)
-#define ASSIGN_SAS_TASK(_sc, _t) do { (_sc)->host_scribble = (void *) _t; } while (0)
-
 static void sas_scsi_task_done(struct sas_task *task)
 {
 	struct task_status_struct *ts = &task->task_status;
@@ -998,6 +995,11 @@ void sas_task_abort(struct sas_task *tas
 		return;
 	}
 
+	if (dev_is_sata(task->dev)) {
+		sas_ata_task_abort(task);
+		return;
+	}
+
 	scsi_req_abort_cmd(sc);
 	scsi_schedule_eh(sc->device->host);
 }
diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
index 72a1904..3407c81 100644
--- a/include/scsi/sas_ata.h
+++ b/include/scsi/sas_ata.h
@@ -36,4 +36,6 @@ static inline int dev_is_sata(struct dom
 int sas_ata_init_host_and_port(struct domain_device *found_dev,
 			       struct scsi_target *starget);
 
+void sas_ata_task_abort(struct sas_task *task);
+
 #endif /* _SAS_ATA_H_ */

[PATCH 11/12] libsas: Provide a generic SATL registration function

[Darrick J. Wong]

Decouple libsas and sas_ata so that the latter can be provided as a
plug-in module for the former.  Any module wishing to provide SATL
services registers itself with libsas; when SATA devices are
discovered, libsas will module_get/put as necessary to ensure that
the module cannot go away accidentally.  At this time, we cannot
start a SAS HBA without a SATL, load a SATL later, and then
rerun device discovery; that may be addressed in a later patch.
A phy reset will do the job quite nicely.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/Kconfig         |   11 +++
 drivers/scsi/libsas/sas_discover.c  |    6 --
 drivers/scsi/libsas/sas_scsi_host.c |  137 ++++++++++++++++++++++++++++++++---
 include/scsi/libsas.h               |   30 +-------
 include/scsi/sas_ata.h              |   38 +++++++++-
 5 files changed, 176 insertions(+), 46 deletions(-)

diff --git a/drivers/scsi/libsas/Kconfig b/drivers/scsi/libsas/Kconfig
index b64e391..9c06eec 100644
--- a/drivers/scsi/libsas/Kconfig
+++ b/drivers/scsi/libsas/Kconfig
@@ -24,12 +24,21 @@ #
 
 config SCSI_SAS_LIBSAS
 	tristate "SAS Domain Transport Attributes"
-	depends on SCSI && ATA
+	depends on SCSI
 	select SCSI_SAS_ATTRS
 	help
 	  This provides transport specific helpers for SAS drivers which
 	  use the domain device construct (like the aic94xxx).
 
+config SCSI_SAS_SATL
+	tristate "Serial ATA Translation Layer (SATL) on SAS controllers"
+	depends on SCSI_SAS_LIBSAS && ATA
+	default y
+	help
+	  This provides an ATA translation layer between libsas and
+	  libata to load SATA devices that are connected to SAS
+	  controllers.
+
 config SCSI_SAS_LIBSAS_DEBUG
 	bool "Compile the SAS Domain Transport Attributes in debug mode"
 	default y
diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c
index a18c0f6..56cc8da 100644
--- a/drivers/scsi/libsas/sas_discover.c
+++ b/drivers/scsi/libsas/sas_discover.c
@@ -476,12 +476,6 @@ cont1:
 	if (!dev->parent)
 		sas_sata_propagate_sas_addr(dev);
 
-	/* XXX Hint: register this SATA device with SATL.
-	   When this returns, dev->sata_dev->lu is alive and
-	   present.
-	sas_satl_register_dev(dev);
-	*/
-
 	sas_fill_in_rphy(dev, dev->rphy);
 
 	return 0;
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index a30c0b7..073b6a7 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -44,6 +44,10 @@ #include <linux/libata.h>
 
 /* ---------- SCSI Host glue ---------- */
 
+
+static DEFINE_SPINLOCK(satl_ops_lock);
+static struct satl_operations *satl_ops;
+
 static void sas_scsi_task_done(struct sas_task *task)
 {
 	struct task_status_struct *ts = &task->task_status;
@@ -213,8 +217,8 @@ int sas_queuecommand(struct scsi_cmnd *c
 			unsigned long flags;
 
 			spin_lock_irqsave(dev->sata_dev.ap->lock, flags);
-			res = ata_sas_queuecmd(cmd, scsi_done,
-					       dev->sata_dev.ap);
+			res = satl_ops->queuecommand(cmd, scsi_done,
+						     dev->sata_dev.ap);
 			spin_unlock_irqrestore(dev->sata_dev.ap->lock, flags);
 			goto out;
 		}
@@ -663,8 +667,9 @@ int sas_ioctl(struct scsi_device *sdev, 
 {
 	struct domain_device *dev = sdev_to_domain_dev(sdev);
 
-	if (dev_is_sata(dev))
-		return ata_scsi_ioctl(sdev, cmd, arg);
+	if (dev_is_sata(dev)) {
+		return satl_ops->ioctl(sdev, cmd, arg);
+	}
 
 	return -EINVAL;
 }
@@ -705,6 +710,29 @@ static inline struct domain_device *sas_
 	return sas_find_dev_by_rphy(rphy);
 }
 
+static int sas_target_alloc_sata(struct domain_device *dev,
+				 struct scsi_target *starget)
+{
+	int res = -ENODEV;
+
+	/* Do we have a SATL available? */
+	if (!get_satl())
+		goto satl_found;
+
+	request_module("sas_ata");
+	if (!get_satl())
+		goto satl_found;
+
+	SAS_DPRINTK("sas_ata not loaded, ignoring SATA devices\n");
+	goto no_satl;
+
+satl_found:
+	res = satl_ops->init_target(dev, starget);
+
+no_satl:
+	return res;
+}
+
 int sas_target_alloc(struct scsi_target *starget)
 {
 	struct domain_device *found_dev = sas_find_target(starget);
@@ -714,7 +742,7 @@ int sas_target_alloc(struct scsi_target 
 		return -ENODEV;
 
 	if (dev_is_sata(found_dev)) {
-		res = sas_ata_init_host_and_port(found_dev, starget);
+		res = sas_target_alloc_sata(found_dev, starget);
 		if (res)
 			return res;
 	}
@@ -734,7 +762,7 @@ int sas_slave_configure(struct scsi_devi
 	BUG_ON(dev->rphy->identify.device_type != SAS_END_DEVICE);
 
 	if (dev_is_sata(dev)) {
-		ata_sas_slave_configure(scsi_dev, dev->sata_dev.ap);
+		satl_ops->configure_port(scsi_dev, dev->sata_dev.ap);
 		return 0;
 	}
 
@@ -764,7 +792,7 @@ void sas_slave_destroy(struct scsi_devic
 	struct domain_device *dev = sdev_to_domain_dev(scsi_dev);
 
 	if (dev_is_sata(dev))
-		ata_port_disable(dev->sata_dev.ap);
+		satl_ops->deactivate_port(dev->sata_dev.ap);
 }
 
 int sas_change_queue_depth(struct scsi_device *scsi_dev, int new_depth)
@@ -996,7 +1024,7 @@ void sas_task_abort(struct sas_task *tas
 	}
 
 	if (dev_is_sata(task->dev)) {
-		sas_ata_task_abort(task);
+		satl_ops->task_abort(task);
 		return;
 	}
 
@@ -1009,7 +1037,7 @@ int sas_slave_alloc(struct scsi_device *
 	struct domain_device *dev = sdev_to_domain_dev(scsi_dev);
 
 	if (dev_is_sata(dev))
-		return ata_sas_port_init(dev->sata_dev.ap);
+		return satl_ops->init_port(dev->sata_dev.ap);
 
 	return 0;
 }
@@ -1021,12 +1049,94 @@ void sas_target_destroy(struct scsi_targ
 	if (!found_dev)
 		return;
 
-	if (dev_is_sata(found_dev))
-		ata_sas_port_destroy(found_dev->sata_dev.ap);
+	if (dev_is_sata(found_dev)) {
+		if (found_dev->sata_dev.ap)
+			satl_ops->destroy_port(found_dev->sata_dev.ap);
+		put_satl();
+	}
 
 	return;
 }
 
+struct sas_task *sas_alloc_task(gfp_t flags)
+{
+	extern struct kmem_cache *sas_task_cache;
+	struct sas_task *task = kmem_cache_alloc(sas_task_cache, flags);
+
+	if (task) {
+		memset(task, 0, sizeof(*task));
+		INIT_LIST_HEAD(&task->list);
+		spin_lock_init(&task->task_state_lock);
+		task->task_state_flags = SAS_TASK_STATE_PENDING;
+		init_timer(&task->timer);
+		init_completion(&task->completion);
+	}
+
+	return task;
+}
+
+void sas_free_task(struct sas_task *task)
+{
+	if (task) {
+		extern struct kmem_cache *sas_task_cache;
+		BUG_ON(!list_empty(&task->list));
+		kmem_cache_free(sas_task_cache, task);
+	}
+}
+
+/* Jump table to ATA translation layer functions */
+int sas_register_satl(struct satl_operations *ops)
+{
+	int res = -ENODEV;
+
+	spin_lock(&satl_ops_lock);
+	if (satl_ops)
+		goto out;
+	satl_ops = ops;
+	res = 0;
+out:
+	spin_unlock(&satl_ops_lock);
+	return res;
+}
+
+int sas_unregister_satl(struct satl_operations *ops)
+{
+	int res = -ENODEV;
+
+	spin_lock(&satl_ops_lock);
+	if (satl_ops != ops)
+		goto out;
+	satl_ops = NULL;
+	res = 0;
+out:
+	spin_unlock(&satl_ops_lock);
+	return res;
+}
+
+int get_satl(void)
+{
+	int res = -ENODEV;
+
+	spin_lock(&satl_ops_lock);
+
+	if (!satl_ops)
+		goto out;
+	if (!try_module_get(satl_ops->owner))
+		goto out;
+	res = 0;
+
+out:
+	spin_unlock(&satl_ops_lock);
+	return res;
+}
+
+void put_satl(void)
+{
+	spin_lock(&satl_ops_lock);
+	module_put(satl_ops->owner);
+	spin_unlock(&satl_ops_lock);
+}
+
 EXPORT_SYMBOL_GPL(sas_queuecommand);
 EXPORT_SYMBOL_GPL(sas_target_alloc);
 EXPORT_SYMBOL_GPL(sas_slave_configure);
@@ -1043,3 +1153,8 @@ EXPORT_SYMBOL_GPL(sas_slave_alloc);
 EXPORT_SYMBOL_GPL(sas_target_destroy);
 EXPORT_SYMBOL_GPL(sas_ioctl);
 EXPORT_SYMBOL_GPL(sas_set_phy_speed);
+EXPORT_SYMBOL_GPL(sas_register_satl);
+EXPORT_SYMBOL_GPL(sas_unregister_satl);
+EXPORT_SYMBOL_GPL(sas_queue_up);
+EXPORT_SYMBOL_GPL(sas_alloc_task);
+EXPORT_SYMBOL_GPL(sas_free_task);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 1400c3f..b97cca3 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -572,31 +572,8 @@ #define SAS_TASK_STATE_ABORTED      4
 #define SAS_TASK_NEED_DEV_RESET     8
 #define SAS_TASK_AT_INITIATOR       16
 
-static inline struct sas_task *sas_alloc_task(gfp_t flags)
-{
-	extern struct kmem_cache *sas_task_cache;
-	struct sas_task *task = kmem_cache_alloc(sas_task_cache, flags);
-
-	if (task) {
-		memset(task, 0, sizeof(*task));
-		INIT_LIST_HEAD(&task->list);
-		spin_lock_init(&task->task_state_lock);
-		task->task_state_flags = SAS_TASK_STATE_PENDING;
-		init_timer(&task->timer);
-		init_completion(&task->completion);
-	}
-
-	return task;
-}
-
-static inline void sas_free_task(struct sas_task *task)
-{
-	if (task) {
-		extern struct kmem_cache *sas_task_cache;
-		BUG_ON(!list_empty(&task->list));
-		kmem_cache_free(sas_task_cache, task);
-	}
-}
+struct sas_task *sas_alloc_task(gfp_t flags);
+void sas_free_task(struct sas_task *task);
 
 struct sas_domain_function_template {
 	/* The class calls these to notify the LLDD of an event. */
@@ -675,4 +652,7 @@ extern void sas_target_destroy(struct sc
 extern int sas_slave_alloc(struct scsi_device *);
 extern int sas_ioctl(struct scsi_device *sdev, int cmd, void __user *arg);
 
+int get_satl(void);
+void put_satl(void);
+
 #endif /* _SASLIB_H_ */
diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
index 3407c81..f1d90f7 100644
--- a/include/scsi/sas_ata.h
+++ b/include/scsi/sas_ata.h
@@ -28,14 +28,46 @@ #define _SAS_ATA_H_
 #include <linux/libata.h>
 #include <scsi/libsas.h>
 
+struct satl_operations {
+	struct module *owner;
+
+	int (*init_target) (struct domain_device *found_dev,
+			    struct scsi_target *starget);
+	int  (*queuecommand) (struct scsi_cmnd *cmd,
+			      void (*done)(struct scsi_cmnd *),
+			      struct ata_port *ap);
+	int  (*ioctl) (struct scsi_device *dev, int cmd,
+		       void __user *arg);
+	int  (*configure_port) (struct scsi_device *,
+				struct ata_port *);
+	void (*deactivate_port) (struct ata_port *);
+	void (*destroy_port) (struct ata_port *);
+	int  (*init_port) (struct ata_port *);
+	void (*task_abort) (struct sas_task *);
+};
+
+int sas_register_satl(struct satl_operations *satl_ops);
+int sas_unregister_satl(struct satl_operations *satl_ops);
+
+#ifdef CONFIG_SCSI_SAS_SATL_MODULE
+# define SAS_ATA_AVAILABLE
+#endif
+
+#ifdef CONFIG_SCSI_SAS_SATL
+# define SAS_ATA_AVAILABLE
+#endif
+
+#ifdef SAS_ATA_AVAILABLE
+
 static inline int dev_is_sata(struct domain_device *dev)
 {
 	return (dev->rphy->identify.target_port_protocols & SAS_PROTOCOL_SATA);
 }
 
-int sas_ata_init_host_and_port(struct domain_device *found_dev,
-			       struct scsi_target *starget);
+#else
+
+#define dev_is_sata(x) (0)
 
-void sas_ata_task_abort(struct sas_task *task);
+#endif /* SAS_ATA_AVAILABLE */
 
 #endif /* _SAS_ATA_H_ */

[PATCH 12/12] sas_ata: Make this a module separate from libsas

[Darrick J. Wong]

Break out sas_ata as a free-standing module that provides a SATA
Translation Layer (SATL) for libsas.  This patch requires the libsas
SATL registration patch; the changes to sas_ata itself are rather
minor.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/scsi/libsas/Makefile  |    5 +++--
 drivers/scsi/libsas/sas_ata.c |   37 ++++++++++++++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/libsas/Makefile b/drivers/scsi/libsas/Makefile
index 6383eb5..5e95902 100644
--- a/drivers/scsi/libsas/Makefile
+++ b/drivers/scsi/libsas/Makefile
@@ -33,5 +33,6 @@ libsas-y +=  sas_init.o     \
 		sas_dump.o     \
 		sas_discover.o \
 		sas_expander.o \
-		sas_scsi_host.o \
-		sas_ata.o
+		sas_scsi_host.o
+
+obj-$(CONFIG_SCSI_SAS_SATL) += sas_ata.o
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 1b7221c..f75fa59 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -404,8 +404,8 @@ static struct ata_port_info sata_port_in
 	.port_ops = &sas_sata_ops
 };
 
-int sas_ata_init_host_and_port(struct domain_device *found_dev,
-			       struct scsi_target *starget)
+static int sas_ata_init_host_and_port(struct domain_device *found_dev,
+				      struct scsi_target *starget)
 {
 	struct Scsi_Host *shost = dev_to_shost(&starget->dev);
 	struct sas_ha_struct *ha = SHOST_TO_SAS_HA(shost);
@@ -431,7 +431,7 @@ int sas_ata_init_host_and_port(struct do
 	return 0;
 }
 
-void sas_ata_task_abort(struct sas_task *task)
+static void sas_ata_task_abort(struct sas_task *task)
 {
 	struct ata_queued_cmd *qc = task->uldd_task;
 	struct completion *waiting;
@@ -450,3 +450,34 @@ void sas_ata_task_abort(struct sas_task 
 	waiting = qc->private_data;
 	complete(waiting);
 }
+
+/* Module initialization */
+static struct satl_operations sas_ata_ops = {
+	.owner			= THIS_MODULE,
+	.init_target		= sas_ata_init_host_and_port,
+	.queuecommand		= ata_sas_queuecmd,
+	.ioctl			= ata_scsi_ioctl,
+	.configure_port		= ata_sas_slave_configure,
+	.deactivate_port	= ata_port_disable,
+	.destroy_port		= ata_sas_port_destroy,
+	.init_port		= ata_sas_port_init,
+	.task_abort		= sas_ata_task_abort
+};
+
+static int __init sas_ata_init(void)
+{
+	return sas_register_satl(&sas_ata_ops);
+}
+
+static void __exit sas_ata_exit(void)
+{
+	sas_unregister_satl(&sas_ata_ops);
+}
+
+module_init(sas_ata_init);
+module_exit(sas_ata_exit);
+
+MODULE_AUTHOR("Darrick Wong <djwong@us.ibm.com>");
+MODULE_DESCRIPTION("libata SATL for SAS");
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("1.0");

Re: [PATCH 12/12] sas_ata: Make this a module separate from libsas

[James Bottomley]

On Tue, 2007-01-30 at 01:19 -0800, Darrick J. Wong wrote:
> Break out sas_ata as a free-standing module that provides a SATA
> Translation Layer (SATL) for libsas.  This patch requires the libsas
> SATL registration patch; the changes to sas_ata itself are rather
> minor.

Right at the moment, this doesn't work.  The dependency of ATA_AVAILABLE
on SCSI_SAS_SATL forces libsas to require sas_ata if you select it as a
module (i.e. they're not truly independent).

How about this solution to untangle them?

James

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 009fd2b..73a737f 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -46,7 +46,16 @@
 
 
 static DEFINE_SPINLOCK(satl_ops_lock);
-static struct satl_operations *satl_ops;
+static struct satl_operations *satl_ops = NULL;
+
+
+static inline int dev_is_sata(struct domain_device *dev)
+{
+	return (satl_ops
+		&& (dev->rphy->identify.target_port_protocols
+		    & SAS_PROTOCOL_SATA));
+}
+
 
 static void sas_scsi_task_done(struct sas_task *task)
 {
diff --git a/include/scsi/sas_ata.h b/include/scsi/sas_ata.h
index f1d90f7..06a9be5 100644
--- a/include/scsi/sas_ata.h
+++ b/include/scsi/sas_ata.h
@@ -49,25 +49,4 @@ struct satl_operations {
 int sas_register_satl(struct satl_operations *satl_ops);
 int sas_unregister_satl(struct satl_operations *satl_ops);
 
-#ifdef CONFIG_SCSI_SAS_SATL_MODULE
-# define SAS_ATA_AVAILABLE
-#endif
-
-#ifdef CONFIG_SCSI_SAS_SATL
-# define SAS_ATA_AVAILABLE
-#endif
-
-#ifdef SAS_ATA_AVAILABLE
-
-static inline int dev_is_sata(struct domain_device *dev)
-{
-	return (dev->rphy->identify.target_port_protocols & SAS_PROTOCOL_SATA);
-}
-
-#else
-
-#define dev_is_sata(x) (0)
-
-#endif /* SAS_ATA_AVAILABLE */
-
 #endif /* _SAS_ATA_H_ */

Re: [PATCH 12/12] sas_ata: Make this a module separate from libsas

[Darrick J. Wong]

James Bottomley wrote:
> On Tue, 2007-01-30 at 01:19 -0800, Darrick J. Wong wrote:
>> Break out sas_ata as a free-standing module that provides a SATA
>> Translation Layer (SATL) for libsas.  This patch requires the libsas
>> SATL registration patch; the changes to sas_ata itself are rather
>> minor.
> 
> Right at the moment, this doesn't work.  The dependency of ATA_AVAILABLE
> on SCSI_SAS_SATL forces libsas to require sas_ata if you select it as a
> module (i.e. they're not truly independent).
> 
> How about this solution to untangle them?

ACK.

--D