[PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives (Added SAS Transport APIs)

[PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives (Added SAS Transport APIs)

[Kashyap, Desai]

Added tlr_enabled bit in sas_device structure, which will have boolean
value 0 for disabled and 1 for enabled.

As suggested by James B, I have added sas_tlr_supported(),sas_enable_tlr(),
sas_disable_tlr() functions at SAS transport layer so that any other
Low layer driver can use those APIs.

SAS transport API sas_tlr_supported() will send vpd page 0x90,
to check the TLR support. If TLR is supported for end device, MPT2SAS driver
will enable the TLR bit in the SCSI_IO for every request. If there is a
response with MPI2_SCSITASKMGMT_RSP_INVALID_FRAME, the driver will turn off
the TLR logic.

Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxx>
Reviewed-by: Eric Moore <Eric.moore@xxxxxxx>
---
diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
index b748893..d9492a9 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
@@ -1309,7 +1309,6 @@ _scsih_slave_alloc(struct scsi_device *sdev)
 	struct MPT2SAS_DEVICE *sas_device_priv_data;
 	struct scsi_target *starget;
 	struct _raid_device *raid_device;
-	struct _sas_device *sas_device;
 	unsigned long flags;
 
 	sas_device_priv_data = kzalloc(sizeof(struct scsi_device), GFP_KERNEL);
@@ -1336,21 +1335,8 @@ _scsih_slave_alloc(struct scsi_device *sdev)
 		if (raid_device)
 			raid_device->sdev = sdev; /* raid is single lun */
 		spin_unlock_irqrestore(&ioc->raid_device_lock, flags);
-	} else {
-		/* set TLR bit for SSP devices */
-		if (!(ioc->facts.IOCCapabilities &
-		     MPI2_IOCFACTS_CAPABILITY_TLR))
-			goto out;
-		spin_lock_irqsave(&ioc->sas_device_lock, flags);
-		sas_device = mpt2sas_scsih_sas_device_find_by_sas_address(ioc,
-		   sas_device_priv_data->sas_target->sas_address);
-		spin_unlock_irqrestore(&ioc->sas_device_lock, flags);
-		if (sas_device && sas_device->device_info &
-		    MPI2_SAS_DEVICE_INFO_SSP_TARGET)
-			sas_device_priv_data->flags |= MPT_DEVICE_TLR_ON;
 	}
 
- out:
 	return 0;
 }
 
@@ -1617,6 +1603,32 @@ _scsih_get_volume_capabilities(struct MPT2SAS_ADAPTER *ioc,
 }
 
 /**
+ * _scsih_enable_tlr - setting TLR flags
+ * @ioc: per adapter object
+ * @sdev: scsi device struct
+ *
+ * Enabling Transaction Layer Retries for tape devices when
+ * vpd page 0x90 is present
+ *
+ */
+static void
+_scsih_enable_tlr(struct MPT2SAS_ADAPTER *ioc, struct scsi_device *sdev)
+{
+	/* only for TAPE */
+	if (sdev->type != TYPE_TAPE)
+		return;
+
+	if (!(ioc->facts.IOCCapabilities & MPI2_IOCFACTS_CAPABILITY_TLR))
+		return;
+
+	sas_enable_tlr(sdev);
+	sdev_printk(KERN_INFO, sdev, "TLR %s\n",
+	    sdev->tlr_enabled ? "Enabled" : "Disabled");
+	return;
+
+}
+
+/**
  * _scsih_slave_configure - device configure routine.
  * @sdev: scsi device struct
  *
@@ -1761,8 +1773,10 @@ _scsih_slave_configure(struct scsi_device *sdev)
 
 	_scsih_change_queue_depth(sdev, qdepth, SCSI_QDEPTH_DEFAULT);
 
-	if (ssp_target)
+	if (ssp_target) {
 		sas_read_port_mode_page(sdev);
+		_scsih_enable_tlr(ioc, sdev);
+	}
 	return 0;
 }
 
@@ -3049,7 +3063,7 @@ _scsih_qcmd(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
 	} else
 		mpi_control |= MPI2_SCSIIO_CONTROL_SIMPLEQ;
 
-	if ((sas_device_priv_data->flags & MPT_DEVICE_TLR_ON))
+	if (scmd->device->tlr_enabled)
 		mpi_control |= MPI2_SCSIIO_CONTROL_TLR_ON;
 
 	smid = mpt2sas_base_get_smid_scsiio(ioc, ioc->scsi_io_cb_idx, scmd);
@@ -3438,10 +3452,11 @@ _scsih_io_done(struct MPT2SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
 		    le32_to_cpu(mpi_reply->ResponseInfo) & 0xFF;
 	if (!sas_device_priv_data->tlr_snoop_check) {
 		sas_device_priv_data->tlr_snoop_check++;
-		if ((sas_device_priv_data->flags & MPT_DEVICE_TLR_ON) &&
-		    response_code == MPI2_SCSITASKMGMT_RSP_INVALID_FRAME)
-			sas_device_priv_data->flags &=
-			    ~MPT_DEVICE_TLR_ON;
+		if ((scmd->device->tlr_enabled) &&
+		    response_code == MPI2_SCSITASKMGMT_RSP_INVALID_FRAME) {
+			sas_disable_tlr(scmd->device);
+			sdev_printk(KERN_INFO, scmd->device, "TLR disabled\n");
+		}
 	}
 
 	xfer_cnt = le32_to_cpu(mpi_reply->TransferCount);
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index fd47cb1..41b8eab 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -358,6 +358,61 @@ void sas_remove_host(struct Scsi_Host *shost)
 }
 EXPORT_SYMBOL(sas_remove_host);
 
+/**
+ * sas_tlr_supported - checking TLR bit in vpd 0x90
+ * @sdev: scsi device struct
+ *
+ * Check Transport Layer Retries are supported or not.
+ * If vpd page 0x90 is present, TRL is supported.
+ *
+ */
+unsigned int
+sas_tlr_supported(struct scsi_device *sdev)
+{
+	char *buffer;
+
+	buffer = scsi_get_vpd_page(sdev, 0x90);
+	if (buffer == NULL)
+		return 0;
+
+	kfree(buffer);
+	return 1;
+
+}
+EXPORT_SYMBOL(sas_tlr_supported);
+
+/**
+ * sas_disable_tlr - setting TLR flags
+ * @sdev: scsi device struct
+ *
+ * Seting tlr_enabled flag to 0.
+ *
+ */
+void
+sas_disable_tlr(struct scsi_device *sdev)
+{
+	sdev->tlr_enabled = 0;
+}
+EXPORT_SYMBOL(sas_disable_tlr);
+
+/**
+ * sas_enable_tlr - setting TLR flags
+ * @sdev: scsi device struct
+ *
+ * Seting tlr_enabled flag 1.
+ *
+ */
+void sas_enable_tlr(struct scsi_device *sdev)
+{
+	unsigned int tlr_supported = 0;
+	tlr_supported  = sas_tlr_supported(sdev);
+
+	if (tlr_supported)
+		sdev->tlr_enabled = 1;
+
+	return;
+}
+EXPORT_SYMBOL(sas_enable_tlr);
 
 /*
  * SAS Phy attributes
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 68d185c..7c1361b 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -112,6 +112,7 @@ struct scsi_device {
 				 * scsi_devinfo.[hc]. For now used only to
 				 * pass settings from slave_alloc to scsi
 				 * core. */
+	unsigned tlr_enabled:1;	/* set tlr flags */
 	unsigned writeable:1;
 	unsigned removable:1;
 	unsigned changed:1;	/* Data invalid due to media change */
diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
index 61ad359..2fb099f 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -181,6 +181,10 @@ extern int sas_phy_add(struct sas_phy *);
 extern void sas_phy_delete(struct sas_phy *);
 extern int scsi_is_sas_phy(const struct device *);
 
+unsigned int sas_tlr_supported(struct scsi_device *);
+void sas_disable_tlr(struct scsi_device *);
+void sas_enable_tlr(struct scsi_device *);
+
 extern struct sas_rphy *sas_end_device_alloc(struct sas_port *);
 extern struct sas_rphy *sas_expander_alloc(struct sas_port *, enum sas_device_type);
 void sas_rphy_free(struct sas_rphy *);

Re: [PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives (Added SAS Transport APIs)

[James Bottomley]

On Wed, 2009-12-16 at 18:56 +0530, Kashyap, Desai wrote:
> Added tlr_enabled bit in sas_device structure, which will have boolean
> value 0 for disabled and 1 for enabled.
> 
> As suggested by James B, I have added sas_tlr_supported(),sas_enable_tlr(),
> sas_disable_tlr() functions at SAS transport layer so that any other
> Low layer driver can use those APIs.
> 
> SAS transport API sas_tlr_supported() will send vpd page 0x90,
> to check the TLR support. If TLR is supported for end device, MPT2SAS driver
> will enable the TLR bit in the SCSI_IO for every request. If there is a
> response with MPI2_SCSITASKMGMT_RSP_INVALID_FRAME, the driver will turn off
> the TLR logic.
> 
> Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxx>
> Reviewed-by: Eric Moore <Eric.moore@xxxxxxx>

What's with the xxxx?  I've got better things to do than fix up
changelog signoffs.

> diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
> index fd47cb1..41b8eab 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -358,6 +358,61 @@ void sas_remove_host(struct Scsi_Host *shost)
>  }
>  EXPORT_SYMBOL(sas_remove_host);
>  
> +/**
> + * sas_tlr_supported - checking TLR bit in vpd 0x90
> + * @sdev: scsi device struct
> + *
> + * Check Transport Layer Retries are supported or not.
> + * If vpd page 0x90 is present, TRL is supported.
> + *
> + */
> +unsigned int
> +sas_tlr_supported(struct scsi_device *sdev)
> +{
> +	char *buffer;
> +
> +	buffer = scsi_get_vpd_page(sdev, 0x90);
> +	if (buffer == NULL)
> +		return 0;
> +
> +	kfree(buffer);
> +	return 1;

This can't be right.  TLR supported is the lowest bit of the 8th byte of
the port entry ... you can't condition this on whether the protocol page
exists or not.

> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 68d185c..7c1361b 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -112,6 +112,7 @@ struct scsi_device {
>  				 * scsi_devinfo.[hc]. For now used only to
>  				 * pass settings from slave_alloc to scsi
>  				 * core. */
> +	unsigned tlr_enabled:1;	/* set tlr flags */

And this is wrong:  the tlr is SAS specific, it belongs in the end
device flags like all the other SAS specific entries.

We also need to expose it to the user in case they want to change the
flag.

How about separating the two patches and doing this for the transport
class?  If it looks OK, I'll add it to the patch sequence and alter this
commit.

James

---
diff --git a/drivers/scsi/scsi_sas_internal.h b/drivers/scsi/scsi_sas_internal.h
index 998cb5b..6266a5d 100644
--- a/drivers/scsi/scsi_sas_internal.h
+++ b/drivers/scsi/scsi_sas_internal.h
@@ -5,7 +5,7 @@
 #define SAS_PHY_ATTRS		17
 #define SAS_PORT_ATTRS		1
 #define SAS_RPORT_ATTRS		7
-#define SAS_END_DEV_ATTRS	3
+#define SAS_END_DEV_ATTRS	5
 #define SAS_EXPANDER_ATTRS	7
 
 struct sas_internal {
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index f27e52d..8b30675 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -155,6 +155,17 @@ static struct {
 sas_bitfield_name_search(linkspeed, sas_linkspeed_names)
 sas_bitfield_name_set(linkspeed, sas_linkspeed_names)
 
+static struct sas_end_device *sas_sdev_to_rdev(struct scsi_device *sdev)
+{
+	struct sas_rphy *rphy = target_to_rphy(sdev->sdev_target);
+	struct sas_end_device *rdev;
+
+	BUG_ON(rphy->identify.device_type != SAS_END_DEVICE);
+
+	rdev = rphy_to_end_device(rphy);
+	return rdev;
+}
+
 static void sas_smp_request(struct request_queue *q, struct Scsi_Host *shost,
 			    struct sas_rphy *rphy)
 {
@@ -358,6 +369,78 @@ void sas_remove_host(struct Scsi_Host *shost)
 }
 EXPORT_SYMBOL(sas_remove_host);
 
+/**
+ * sas_tlr_supported - checking TLR bit in vpd 0x90
+ * @sdev: scsi device struct
+ *
+ * Check Transport Layer Retries are supported or not.
+ * If vpd page 0x90 is present, TRL is supported.
+ *
+ */
+unsigned int
+sas_tlr_supported(struct scsi_device *sdev)
+{
+	const int vpd_len = 32;
+	struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
+	char *buffer = kzalloc(vpd_len, GFP_KERNEL);
+	int ret = 0;
+
+	if (scsi_get_vpd_page(sdev, 0x90, buffer, vpd_len))
+		goto out;
+
+	/*
+	 * Magic numbers: the VPD Protocol page (0x90)
+	 * has a 4 byte header and then one entry per device port
+	 * the TLR bit is at offset 8 on each port entry
+	 * if we take the first port, that's at total offset 12
+	 */
+	ret = buffer[12] & 0x01;
+
+ out:
+	kfree(buffer);
+	rdev->tlr_supported = ret;
+	return ret;
+
+}
+EXPORT_SYMBOL(sas_tlr_supported);
+
+/**
+ * sas_disable_tlr - setting TLR flags
+ * @sdev: scsi device struct
+ *
+ * Seting tlr_enabled flag to 0.
+ *
+ */
+void
+sas_disable_tlr(struct scsi_device *sdev)
+{
+	struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
+
+	rdev->tlr_enabled = 0;
+}
+EXPORT_SYMBOL(sas_disable_tlr);
+
+/**
+ * sas_enable_tlr - setting TLR flags
+ * @sdev: scsi device struct
+ *
+ * Seting tlr_enabled flag 1.
+ *
+ */
+void sas_enable_tlr(struct scsi_device *sdev)
+{
+	unsigned int tlr_supported = 0;
+	tlr_supported  = sas_tlr_supported(sdev);
+
+	if (tlr_supported) {
+		struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
+
+		rdev->tlr_enabled = 1;
+	}
+
+	return;
+}
+EXPORT_SYMBOL(sas_enable_tlr);
 
 /*
  * SAS Phy attributes
@@ -1146,15 +1229,10 @@ sas_rphy_simple_attr(identify.phy_identifier, phy_identifier, "%d\n", u8);
 int sas_read_port_mode_page(struct scsi_device *sdev)
 {
 	char *buffer = kzalloc(BUF_SIZE, GFP_KERNEL), *msdata;
-	struct sas_rphy *rphy = target_to_rphy(sdev->sdev_target);
-	struct sas_end_device *rdev;
+	struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
 	struct scsi_mode_data mode_data;
 	int res, error;
 
-	BUG_ON(rphy->identify.device_type != SAS_END_DEVICE);
-
-	rdev = rphy_to_end_device(rphy);
-
 	if (!buffer)
 		return -ENOMEM;
 
@@ -1207,6 +1285,10 @@ sas_end_dev_simple_attr(I_T_nexus_loss_timeout, I_T_nexus_loss_timeout,
 			"%d\n", int);
 sas_end_dev_simple_attr(initiator_response_timeout, initiator_response_timeout,
 			"%d\n", int);
+sas_end_dev_simple_attr(tlr_supported, tlr_supported,
+			"%d\n", int);
+sas_end_dev_simple_attr(tlr_enabled, tlr_enabled,
+			"%d\n", int);
 
 static DECLARE_TRANSPORT_CLASS(sas_expander_class,
 			       "sas_expander", NULL, NULL, NULL);
@@ -1733,6 +1815,8 @@ sas_attach_transport(struct sas_function_template *ft)
 	SETUP_END_DEV_ATTRIBUTE(end_dev_ready_led_meaning);
 	SETUP_END_DEV_ATTRIBUTE(end_dev_I_T_nexus_loss_timeout);
 	SETUP_END_DEV_ATTRIBUTE(end_dev_initiator_response_timeout);
+	SETUP_END_DEV_ATTRIBUTE(end_dev_tlr_supported);
+	SETUP_END_DEV_ATTRIBUTE(end_dev_tlr_enabled);
 	i->end_dev_attrs[count] = NULL;
 
 	count = 0;
diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h
index 61ad359..2988fcf 100644
--- a/include/scsi/scsi_transport_sas.h
+++ b/include/scsi/scsi_transport_sas.h
@@ -107,6 +107,8 @@ struct sas_end_device {
 	struct sas_rphy		rphy;
 	/* flags */
 	unsigned		ready_led_meaning:1;
+	unsigned		tlr_supported:1;
+	unsigned		tlr_enabled:1;
 	/* parameters */
 	u16			I_T_nexus_loss_timeout;
 	u16			initiator_response_timeout;
@@ -181,6 +183,10 @@ extern int sas_phy_add(struct sas_phy *);
 extern void sas_phy_delete(struct sas_phy *);
 extern int scsi_is_sas_phy(const struct device *);
 
+unsigned int sas_tlr_supported(struct scsi_device *);
+void sas_disable_tlr(struct scsi_device *);
+void sas_enable_tlr(struct scsi_device *);
+
 extern struct sas_rphy *sas_end_device_alloc(struct sas_port *);
 extern struct sas_rphy *sas_expander_alloc(struct sas_port *, enum sas_device_type);
 void sas_rphy_free(struct sas_rphy *);

RE: [PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives (Added SAS Transport APIs)

[Desai, Kashyap]

> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@suse.de]
> Sent: Monday, January 18, 2010 9:45 PM
> To: Desai, Kashyap
> Cc: linux-scsi@vger.kernel.org; Moore, Eric; Prakash, Sathya
> Subject: Re: [PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives
> (Added SAS Transport APIs)
> 
> On Wed, 2009-12-16 at 18:56 +0530, Kashyap, Desai wrote:
> > Added tlr_enabled bit in sas_device structure, which will have
> boolean
> > value 0 for disabled and 1 for enabled.
> >
> > As suggested by James B, I have added
> sas_tlr_supported(),sas_enable_tlr(),
> > sas_disable_tlr() functions at SAS transport layer so that any other
> > Low layer driver can use those APIs.
> >
> > SAS transport API sas_tlr_supported() will send vpd page 0x90,
> > to check the TLR support. If TLR is supported for end device, MPT2SAS
> driver
> > will enable the TLR bit in the SCSI_IO for every request. If there is
> a
> > response with MPI2_SCSITASKMGMT_RSP_INVALID_FRAME, the driver will
> turn off
> > the TLR logic.
> >
> > Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxx>
> > Reviewed-by: Eric Moore <Eric.moore@xxxxxxx>
> 
> What's with the xxxx?  I've got better things to do than fix up
> changelog signoffs.

Really Sorry about this gaffe. 
> 
> > diff --git a/drivers/scsi/scsi_transport_sas.c
> b/drivers/scsi/scsi_transport_sas.c
> > index fd47cb1..41b8eab 100644
> > --- a/drivers/scsi/scsi_transport_sas.c
> > +++ b/drivers/scsi/scsi_transport_sas.c
> > @@ -358,6 +358,61 @@ void sas_remove_host(struct Scsi_Host *shost)
> >  }
> >  EXPORT_SYMBOL(sas_remove_host);
> >
> > +/**
> > + * sas_tlr_supported - checking TLR bit in vpd 0x90
> > + * @sdev: scsi device struct
> > + *
> > + * Check Transport Layer Retries are supported or not.
> > + * If vpd page 0x90 is present, TRL is supported.
> > + *
> > + */
> > +unsigned int
> > +sas_tlr_supported(struct scsi_device *sdev)
> > +{
> > +	char *buffer;
> > +
> > +	buffer = scsi_get_vpd_page(sdev, 0x90);
> > +	if (buffer == NULL)
> > +		return 0;
> > +
> > +	kfree(buffer);
> > +	return 1;
> 
> This can't be right.  TLR supported is the lowest bit of the 8th byte
> of
> the port entry ... you can't condition this on whether the protocol
> page
> exists or not.
Got this point. I will change accordingly.
> 
> > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> > index 68d185c..7c1361b 100644
> > --- a/include/scsi/scsi_device.h
> > +++ b/include/scsi/scsi_device.h
> > @@ -112,6 +112,7 @@ struct scsi_device {
> >  				 * scsi_devinfo.[hc]. For now used only to
> >  				 * pass settings from slave_alloc to scsi
> >  				 * core. */
> > +	unsigned tlr_enabled:1;	/* set tlr flags */
> 
> And this is wrong:  the tlr is SAS specific, it belongs in the end
> device flags like all the other SAS specific entries.
I got it. I will move this into scsi_transport_sas.h in sas_end_device  as flags.
> 
> We also need to expose it to the user in case they want to change the
> flag.
> 
I am little confused on this point. Hope you don’t mind simplifying it.

> How about separating the two patches and doing this for the transport
> class?  If it looks OK, I'll add it to the patch sequence and alter
> this
> commit.
James, What does this mean ?
> 
> James
> 
> ---
> diff --git a/drivers/scsi/scsi_sas_internal.h
> b/drivers/scsi/scsi_sas_internal.h
> index 998cb5b..6266a5d 100644
> --- a/drivers/scsi/scsi_sas_internal.h
> +++ b/drivers/scsi/scsi_sas_internal.h
> @@ -5,7 +5,7 @@
>  #define SAS_PHY_ATTRS		17
>  #define SAS_PORT_ATTRS		1
>  #define SAS_RPORT_ATTRS		7
> -#define SAS_END_DEV_ATTRS	3
> +#define SAS_END_DEV_ATTRS	5
>  #define SAS_EXPANDER_ATTRS	7
> 
>  struct sas_internal {
> diff --git a/drivers/scsi/scsi_transport_sas.c
> b/drivers/scsi/scsi_transport_sas.c
> index f27e52d..8b30675 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -155,6 +155,17 @@ static struct {
>  sas_bitfield_name_search(linkspeed, sas_linkspeed_names)
>  sas_bitfield_name_set(linkspeed, sas_linkspeed_names)
> 
> +static struct sas_end_device *sas_sdev_to_rdev(struct scsi_device
> *sdev)
> +{
> +	struct sas_rphy *rphy = target_to_rphy(sdev->sdev_target);
> +	struct sas_end_device *rdev;
> +
> +	BUG_ON(rphy->identify.device_type != SAS_END_DEVICE);
> +
> +	rdev = rphy_to_end_device(rphy);
> +	return rdev;
> +}
> +
>  static void sas_smp_request(struct request_queue *q, struct Scsi_Host
> *shost,
>  			    struct sas_rphy *rphy)
>  {
> @@ -358,6 +369,78 @@ void sas_remove_host(struct Scsi_Host *shost)
>  }
>  EXPORT_SYMBOL(sas_remove_host);
> 
> +/**
> + * sas_tlr_supported - checking TLR bit in vpd 0x90
> + * @sdev: scsi device struct
> + *
> + * Check Transport Layer Retries are supported or not.
> + * If vpd page 0x90 is present, TRL is supported.
> + *
> + */
> +unsigned int
> +sas_tlr_supported(struct scsi_device *sdev)
> +{
> +	const int vpd_len = 32;
> +	struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
> +	char *buffer = kzalloc(vpd_len, GFP_KERNEL);
> +	int ret = 0;
> +
> +	if (scsi_get_vpd_page(sdev, 0x90, buffer, vpd_len))
> +		goto out;
> +
> +	/*
> +	 * Magic numbers: the VPD Protocol page (0x90)
> +	 * has a 4 byte header and then one entry per device port
> +	 * the TLR bit is at offset 8 on each port entry
> +	 * if we take the first port, that's at total offset 12
> +	 */
> +	ret = buffer[12] & 0x01;
> +
> + out:
> +	kfree(buffer);
> +	rdev->tlr_supported = ret;
> +	return ret;
> +
> +}
> +EXPORT_SYMBOL(sas_tlr_supported);
> +
> +/**
> + * sas_disable_tlr - setting TLR flags
> + * @sdev: scsi device struct
> + *
> + * Seting tlr_enabled flag to 0.
> + *
> + */
> +void
> +sas_disable_tlr(struct scsi_device *sdev)
> +{
> +	struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
> +
> +	rdev->tlr_enabled = 0;
> +}
> +EXPORT_SYMBOL(sas_disable_tlr);
> +
> +/**
> + * sas_enable_tlr - setting TLR flags
> + * @sdev: scsi device struct
> + *
> + * Seting tlr_enabled flag 1.
> + *
> + */
> +void sas_enable_tlr(struct scsi_device *sdev)
> +{
> +	unsigned int tlr_supported = 0;
> +	tlr_supported  = sas_tlr_supported(sdev);
> +
> +	if (tlr_supported) {
> +		struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
> +
> +		rdev->tlr_enabled = 1;
> +	}
> +
> +	return;
> +}
> +EXPORT_SYMBOL(sas_enable_tlr);
> 
>  /*
>   * SAS Phy attributes
> @@ -1146,15 +1229,10 @@ sas_rphy_simple_attr(identify.phy_identifier,
> phy_identifier, "%d\n", u8);
>  int sas_read_port_mode_page(struct scsi_device *sdev)
>  {
>  	char *buffer = kzalloc(BUF_SIZE, GFP_KERNEL), *msdata;
> -	struct sas_rphy *rphy = target_to_rphy(sdev->sdev_target);
> -	struct sas_end_device *rdev;
> +	struct sas_end_device *rdev = sas_sdev_to_rdev(sdev);
>  	struct scsi_mode_data mode_data;
>  	int res, error;
> 
> -	BUG_ON(rphy->identify.device_type != SAS_END_DEVICE);
> -
> -	rdev = rphy_to_end_device(rphy);
> -
>  	if (!buffer)
>  		return -ENOMEM;
> 
> @@ -1207,6 +1285,10 @@ sas_end_dev_simple_attr(I_T_nexus_loss_timeout,
> I_T_nexus_loss_timeout,
>  			"%d\n", int);
>  sas_end_dev_simple_attr(initiator_response_timeout,
> initiator_response_timeout,
>  			"%d\n", int);
> +sas_end_dev_simple_attr(tlr_supported, tlr_supported,
> +			"%d\n", int);
> +sas_end_dev_simple_attr(tlr_enabled, tlr_enabled,
> +			"%d\n", int);
> 
>  static DECLARE_TRANSPORT_CLASS(sas_expander_class,
>  			       "sas_expander", NULL, NULL, NULL);
> @@ -1733,6 +1815,8 @@ sas_attach_transport(struct sas_function_template
> *ft)
>  	SETUP_END_DEV_ATTRIBUTE(end_dev_ready_led_meaning);
>  	SETUP_END_DEV_ATTRIBUTE(end_dev_I_T_nexus_loss_timeout);
>  	SETUP_END_DEV_ATTRIBUTE(end_dev_initiator_response_timeout);
> +	SETUP_END_DEV_ATTRIBUTE(end_dev_tlr_supported);
> +	SETUP_END_DEV_ATTRIBUTE(end_dev_tlr_enabled);
>  	i->end_dev_attrs[count] = NULL;
> 
>  	count = 0;
> diff --git a/include/scsi/scsi_transport_sas.h
> b/include/scsi/scsi_transport_sas.h
> index 61ad359..2988fcf 100644
> --- a/include/scsi/scsi_transport_sas.h
> +++ b/include/scsi/scsi_transport_sas.h
> @@ -107,6 +107,8 @@ struct sas_end_device {
>  	struct sas_rphy		rphy;
>  	/* flags */
>  	unsigned		ready_led_meaning:1;
> +	unsigned		tlr_supported:1;
> +	unsigned		tlr_enabled:1;
>  	/* parameters */
>  	u16			I_T_nexus_loss_timeout;
>  	u16			initiator_response_timeout;
> @@ -181,6 +183,10 @@ extern int sas_phy_add(struct sas_phy *);
>  extern void sas_phy_delete(struct sas_phy *);
>  extern int scsi_is_sas_phy(const struct device *);
> 
> +unsigned int sas_tlr_supported(struct scsi_device *);
> +void sas_disable_tlr(struct scsi_device *);
> +void sas_enable_tlr(struct scsi_device *);
> +
>  extern struct sas_rphy *sas_end_device_alloc(struct sas_port *);
>  extern struct sas_rphy *sas_expander_alloc(struct sas_port *, enum
> sas_device_type);
>  void sas_rphy_free(struct sas_rphy *);
> 


Thanks,
Kashyap

RE: [PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives (Added SAS Transport APIs)

[James Bottomley]

On Tue, 2010-01-19 at 23:57 +0530, Desai, Kashyap wrote:
> 
> > -----Original Message-----
> > From: James Bottomley [mailto:James.Bottomley@suse.de]
> > Sent: Monday, January 18, 2010 9:45 PM
> > To: Desai, Kashyap
> > Cc: linux-scsi@vger.kernel.org; Moore, Eric; Prakash, Sathya
> > Subject: Re: [PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives
> > (Added SAS Transport APIs)
> > 
> > On Wed, 2009-12-16 at 18:56 +0530, Kashyap, Desai wrote:
> > > Added tlr_enabled bit in sas_device structure, which will have
> > boolean
> > > value 0 for disabled and 1 for enabled.
> > >
> > > As suggested by James B, I have added
> > sas_tlr_supported(),sas_enable_tlr(),
> > > sas_disable_tlr() functions at SAS transport layer so that any other
> > > Low layer driver can use those APIs.
> > >
> > > SAS transport API sas_tlr_supported() will send vpd page 0x90,
> > > to check the TLR support. If TLR is supported for end device, MPT2SAS
> > driver
> > > will enable the TLR bit in the SCSI_IO for every request. If there is
> > a
> > > response with MPI2_SCSITASKMGMT_RSP_INVALID_FRAME, the driver will
> > turn off
> > > the TLR logic.
> > >
> > > Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxx>
> > > Reviewed-by: Eric Moore <Eric.moore@xxxxxxx>
> > 
> > What's with the xxxx?  I've got better things to do than fix up
> > changelog signoffs.
> 
> Really Sorry about this gaffe. 
> > 
> > > diff --git a/drivers/scsi/scsi_transport_sas.c
> > b/drivers/scsi/scsi_transport_sas.c
> > > index fd47cb1..41b8eab 100644
> > > --- a/drivers/scsi/scsi_transport_sas.c
> > > +++ b/drivers/scsi/scsi_transport_sas.c
> > > @@ -358,6 +358,61 @@ void sas_remove_host(struct Scsi_Host *shost)
> > >  }
> > >  EXPORT_SYMBOL(sas_remove_host);
> > >
> > > +/**
> > > + * sas_tlr_supported - checking TLR bit in vpd 0x90
> > > + * @sdev: scsi device struct
> > > + *
> > > + * Check Transport Layer Retries are supported or not.
> > > + * If vpd page 0x90 is present, TRL is supported.
> > > + *
> > > + */
> > > +unsigned int
> > > +sas_tlr_supported(struct scsi_device *sdev)
> > > +{
> > > +	char *buffer;
> > > +
> > > +	buffer = scsi_get_vpd_page(sdev, 0x90);
> > > +	if (buffer == NULL)
> > > +		return 0;
> > > +
> > > +	kfree(buffer);
> > > +	return 1;
> > 
> > This can't be right.  TLR supported is the lowest bit of the 8th byte
> > of
> > the port entry ... you can't condition this on whether the protocol
> > page
> > exists or not.
> Got this point. I will change accordingly.
> > 
> > > diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> > > index 68d185c..7c1361b 100644
> > > --- a/include/scsi/scsi_device.h
> > > +++ b/include/scsi/scsi_device.h
> > > @@ -112,6 +112,7 @@ struct scsi_device {
> > >  				 * scsi_devinfo.[hc]. For now used only to
> > >  				 * pass settings from slave_alloc to scsi
> > >  				 * core. */
> > > +	unsigned tlr_enabled:1;	/* set tlr flags */
> > 
> > And this is wrong:  the tlr is SAS specific, it belongs in the end
> > device flags like all the other SAS specific entries.
> I got it. I will move this into scsi_transport_sas.h in sas_end_device  as flags.
> > 
> > We also need to expose it to the user in case they want to change the
> > flag.
> > 
> I am little confused on this point. Hope you don’t mind simplifying it.
> 
> > How about separating the two patches and doing this for the transport
> > class?  If it looks OK, I'll add it to the patch sequence and alter
> > this
> > commit.
> James, What does this mean ?

It means does the patch look OK to you?  As in storing the data in the
transport class and separating it into supported and enabled.  The
actual API you use for mpt2sas is unchanged.

James


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

RE: [PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives (Added SAS Transport APIs)

[Desai, Kashyap]

> -----Original Message-----
> From: James Bottomley [mailto:James.Bottomley@suse.de]
> Sent: Wednesday, January 20, 2010 12:48 AM
> To: Desai, Kashyap
> Cc: linux-scsi@vger.kernel.org; Moore, Eric; Prakash, Sathya
> Subject: RE: [PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives
> (Added SAS Transport APIs)
> 
> On Tue, 2010-01-19 at 23:57 +0530, Desai, Kashyap wrote:
> >
> > > -----Original Message-----
> > > From: James Bottomley [mailto:James.Bottomley@suse.de]
> > > Sent: Monday, January 18, 2010 9:45 PM
> > > To: Desai, Kashyap
> > > Cc: linux-scsi@vger.kernel.org; Moore, Eric; Prakash, Sathya
> > > Subject: Re: [PATCH 9/10] mpt2sas: Enable TLR for SSP TAPE drives
> > > (Added SAS Transport APIs)
> > >
> > > On Wed, 2009-12-16 at 18:56 +0530, Kashyap, Desai wrote:
> > > > Added tlr_enabled bit in sas_device structure, which will have
> > > boolean
> > > > value 0 for disabled and 1 for enabled.
> > > >
> > > > As suggested by James B, I have added
> > > sas_tlr_supported(),sas_enable_tlr(),
> > > > sas_disable_tlr() functions at SAS transport layer so that any
> other
> > > > Low layer driver can use those APIs.
> > > >
> > > > SAS transport API sas_tlr_supported() will send vpd page 0x90,
> > > > to check the TLR support. If TLR is supported for end device,
> MPT2SAS
> > > driver
> > > > will enable the TLR bit in the SCSI_IO for every request. If
> there is
> > > a
> > > > response with MPI2_SCSITASKMGMT_RSP_INVALID_FRAME, the driver
> will
> > > turn off
> > > > the TLR logic.
> > > >
> > > > Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxx>
> > > > Reviewed-by: Eric Moore <Eric.moore@xxxxxxx>
> > >
> > > What's with the xxxx?  I've got better things to do than fix up
> > > changelog signoffs.
> >
> > Really Sorry about this gaffe.
> > >
> > > > diff --git a/drivers/scsi/scsi_transport_sas.c
> > > b/drivers/scsi/scsi_transport_sas.c
> > > > index fd47cb1..41b8eab 100644
> > > > --- a/drivers/scsi/scsi_transport_sas.c
> > > > +++ b/drivers/scsi/scsi_transport_sas.c
> > > > @@ -358,6 +358,61 @@ void sas_remove_host(struct Scsi_Host
> *shost)
> > > >  }
> > > >  EXPORT_SYMBOL(sas_remove_host);
> > > >
> > > > +/**
> > > > + * sas_tlr_supported - checking TLR bit in vpd 0x90
> > > > + * @sdev: scsi device struct
> > > > + *
> > > > + * Check Transport Layer Retries are supported or not.
> > > > + * If vpd page 0x90 is present, TRL is supported.
> > > > + *
> > > > + */
> > > > +unsigned int
> > > > +sas_tlr_supported(struct scsi_device *sdev)
> > > > +{
> > > > +	char *buffer;
> > > > +
> > > > +	buffer = scsi_get_vpd_page(sdev, 0x90);
> > > > +	if (buffer == NULL)
> > > > +		return 0;
> > > > +
> > > > +	kfree(buffer);
> > > > +	return 1;
> > >
> > > This can't be right.  TLR supported is the lowest bit of the 8th
> byte
> > > of
> > > the port entry ... you can't condition this on whether the protocol
> > > page
> > > exists or not.
> > Got this point. I will change accordingly.
> > >
> > > > diff --git a/include/scsi/scsi_device.h
> b/include/scsi/scsi_device.h
> > > > index 68d185c..7c1361b 100644
> > > > --- a/include/scsi/scsi_device.h
> > > > +++ b/include/scsi/scsi_device.h
> > > > @@ -112,6 +112,7 @@ struct scsi_device {
> > > >  				 * scsi_devinfo.[hc]. For now used only
> to
> > > >  				 * pass settings from slave_alloc to scsi
> > > >  				 * core. */
> > > > +	unsigned tlr_enabled:1;	/* set tlr flags */
> > >
> > > And this is wrong:  the tlr is SAS specific, it belongs in the end
> > > device flags like all the other SAS specific entries.
> > I got it. I will move this into scsi_transport_sas.h in
> sas_end_device  as flags.
> > >
> > > We also need to expose it to the user in case they want to change
> the
> > > flag.
> > >
> > I am little confused on this point. Hope you don’t mind simplifying
> it.
> >
> > > How about separating the two patches and doing this for the
> transport
> > > class?  If it looks OK, I'll add it to the patch sequence and alter
> > > this
> > > commit.
> > James, What does this mean ?
> 
> It means does the patch look OK to you?  As in storing the data in the
> transport class and separating it into supported and enabled.  The
Thanks James. Now I understood your change.
This looks OK to me. 

- Kashyap

> actual API you use for mpt2sas is unchanged.
> 
> James
>