[PATCH] libsas: add SMP READ/WRTIE GPIO support

[PATCH] libsas: add SMP READ/WRTIE GPIO support

[Artur Wojcik]

This patch enables the SMP READ/WRITE GPIO function interface in libsas SMP
host module. The interface is used to control the SGPIO initiator in the SAS
initiator. The implementation is SFF-8485 and SAS-2 compliant.

There are two functions in transport class only responsible for reading and
writting GPIO registers. I decided to leave the decission about what type of
registers are supported to lldd.

Now the user space application may issue SMP READ/WRITE GPIO frame to HBA in
order to read/write GPIO registers.

Signed-off-by: Artur Wojcik <artur.wojcik@intel.com>
---
 drivers/scsi/libsas/sas_host_smp.c |   48 +++++++++++++++++++++++++++++++++++-
 include/scsi/libsas.h              |   14 +++++++++++
 include/scsi/sas.h                 |    4 +++
 3 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_host_smp.c b/drivers/scsi/libsas/sas_host_smp.c
index 04ad8dd..2d34cb6 100644
--- a/drivers/scsi/libsas/sas_host_smp.c
+++ b/drivers/scsi/libsas/sas_host_smp.c
@@ -51,6 +51,40 @@ static void sas_host_smp_discover(struct sas_ha_struct *sas_ha, u8 *resp_data,
 	resp_data[15] = rphy->identify.target_port_protocols;
 }
 
+static void sas_host_smp_read_gpio(struct sas_ha_struct *sas_ha, u8 *resp_data,
+	u8 reg_type, u8 reg_index, u8 reg_count)
+{
+	struct sas_internal *i =
+		to_sas_internal(sas_ha->core.shost->transportt);
+
+	if (i->dft->lldd_read_gpio == NULL) {
+		resp_data[2] = SMP_RESP_FUNC_UNK;
+	} else {
+		if (i->dft->lldd_read_gpio(sas_ha, reg_type, reg_index,
+					   reg_count, &resp_data[4]) == 0)
+			resp_data[2] = SMP_RESP_FUNC_ACC;
+		else
+			resp_data[2] = SMP_RESP_FUNC_FAILED;
+	}
+}
+
+static void sas_host_smp_write_gpio(struct sas_ha_struct *sas_ha, u8 *resp_data,
+	u8 reg_type, u8 reg_index, u8 reg_count, u8 *write_data)
+{
+	struct sas_internal *i =
+		to_sas_internal(sas_ha->core.shost->transportt);
+
+	if (i->dft->lldd_write_gpio == NULL) {
+		resp_data[2] = SMP_RESP_FUNC_UNK;
+	} else {
+		if (i->dft->lldd_write_gpio(sas_ha, reg_type, reg_index,
+					    reg_count, &resp_data[8]) == 0)
+			resp_data[2] = SMP_RESP_FUNC_ACC;
+		else
+			resp_data[2] = SMP_RESP_FUNC_FAILED;
+	}
+}
+
 static void sas_report_phy_sata(struct sas_ha_struct *sas_ha, u8 *resp_data,
 				u8 phy_id)
 {
@@ -195,8 +229,13 @@ int sas_smp_host_handler(struct Scsi_Host *shost, struct request *req,
 		       SAS_EXPANDER_PRODUCT_ID_LEN);
 		break;
 
+	case SMP_READ_GPIO_REG_ENHANCED:
 	case SMP_READ_GPIO_REG:
-		/* FIXME: need GPIO support in the transport class */
+		if (req->resid_len != 12)
+			resp_data[2] = SMP_RESP_INV_FRM_LEN;
+		else
+			sas_host_smp_read_gpio(sas_ha, resp_data, req_data[2],
+				req_data[3], req_data[4]);
 		break;
 
 	case SMP_DISCOVER:
@@ -230,8 +269,13 @@ int sas_smp_host_handler(struct Scsi_Host *shost, struct request *req,
 		/* Can't implement; hosts have no routes */
 		break;
 
+	case SMP_WRITE_GPIO_REG_ENHANCED:
 	case SMP_WRITE_GPIO_REG:
-		/* FIXME: need GPIO support in the transport class */
+		if (req->resid_len < (req_data[4] * 4) + 8)
+			resp_data[2] = SMP_RESP_INV_FRM_LEN;
+		else
+			sas_host_smp_write_gpio(sas_ha, resp_data, req_data[2],
+				req_data[3], req_data[4], &req_data[5]);
 		break;
 
 	case SMP_CONF_ROUTE_INFO:
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 8f6bb9c..6d876ca 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -56,6 +56,14 @@ enum sas_phy_type {
         PHY_TYPE_VIRTUAL
 };
 
+enum gpio_reg_type {
+	GPIO_REG_CFG = 0,
+	GPIO_REG_RX = 1,
+	GPIO_REG_RX_GP = 2,
+	GPIO_REG_TX = 3,
+	GPIO_REG_TX_GP = 4,
+};
+
 /* The events are mnemonically described in sas_dump.c
  * so when updating/adding events here, please also
  * update the other file too.
@@ -613,6 +621,12 @@ struct sas_domain_function_template {
 
 	/* Phy management */
 	int (*lldd_control_phy)(struct asd_sas_phy *, enum phy_func, void *);
+
+	/* GPIO support */
+	int (*lldd_read_gpio)(struct sas_ha_struct *, u8 reg_type,
+			      u8 reg_index, u8 reg_count, u8 *read_data);
+	int (*lldd_write_gpio)(struct sas_ha_struct *, u8 reg_type,
+			       u8 reg_index, u8 reg_count, u8 *write_data);
 };
 
 extern int sas_register_ha(struct sas_ha_struct *);
diff --git a/include/scsi/sas.h b/include/scsi/sas.h
index e9fd022..253094d 100644
--- a/include/scsi/sas.h
+++ b/include/scsi/sas.h
@@ -54,6 +54,10 @@
 #define SMP_PHY_CONTROL          0x91
 #define SMP_PHY_TEST_FUNCTION    0x92
 
+/* SAS-2 specific */
+#define SMP_READ_GPIO_REG_ENHANCED    0x07
+#define SMP_WRITE_GPIO_REG_ENHANCED   0x83
+
 #define SMP_RESP_FUNC_ACC        0x00
 #define SMP_RESP_FUNC_UNK        0x01
 #define SMP_RESP_FUNC_FAILED     0x02

Re: [PATCH] libsas: add SMP READ/WRTIE GPIO support

[Douglas Gilbert]

On 11-06-01 10:02 AM, Artur Wojcik wrote:
> This patch enables the SMP READ/WRITE GPIO function interface in libsas SMP
> host module. The interface is used to control the SGPIO initiator in the SAS
> initiator. The implementation is SFF-8485 and SAS-2 compliant.
>
> There are two functions in transport class only responsible for reading and
> writting GPIO registers. I decided to leave the decission about what type of
> registers are supported to lldd.
>
> Now the user space application may issue SMP READ/WRITE GPIO frame to HBA in
> order to read/write GPIO registers.

Your code also accepts the somewhat mysterious READ GPIO
REGISTER ENHANCED SMP function and its WRITE equivalent.
Reference is made in sas2r16.pdf to SFF-8485 for those
functions. However the most recent version of SFF-8485
on the Seagate site is 0.7 dated February 2006, and it does
not define those ENHANCED functions.

Could you tell us what those ENHANCED versions do?

Doug Gilbert

Re: [PATCH] libsas: add SMP READ/WRTIE GPIO support

[Artur Wojcik]

On 06/01/2011 04:32 PM, Douglas Gilbert wrote:
> On 11-06-01 10:02 AM, Artur Wojcik wrote:
>> This patch enables the SMP READ/WRITE GPIO function interface in 
>> libsas SMP
>> host module. The interface is used to control the SGPIO initiator in 
>> the SAS
>> initiator. The implementation is SFF-8485 and SAS-2 compliant.
>>
>> There are two functions in transport class only responsible for 
>> reading and
>> writting GPIO registers. I decided to leave the decission about what 
>> type of
>> registers are supported to lldd.
>>
>> Now the user space application may issue SMP READ/WRITE GPIO frame to 
>> HBA in
>> order to read/write GPIO registers.
>
> Your code also accepts the somewhat mysterious READ GPIO
> REGISTER ENHANCED SMP function and its WRITE equivalent.
> Reference is made in sas2r16.pdf to SFF-8485 for those
> functions. However the most recent version of SFF-8485
> on the Seagate site is 0.7 dated February 2006, and it does
> not define those ENHANCED functions.
>
> Could you tell us what those ENHANCED versions do?
>
> Doug Gilbert
I have not found the description of ENHANCED frames in any publicly 
available specification related to SGPIO. I took SAS-2 as the major 
reference and it says SMP functions 02h (READ) and 82h (WRITE) are 
obsolete. My understanding is that for the moment the new functions are 
replacing the deprecated functions, but the frame format and registers 
set stay the same.

Artur Wojcik

Re: [PATCH] libsas: add SMP READ/WRTIE GPIO support

[Dan Williams]

On Wed, Jun 1, 2011 at 7:02 AM, Artur Wojcik <artur.wojcik@intel.com> wrote:
> This patch enables the SMP READ/WRITE GPIO function interface in libsas SMP
> host module. The interface is used to control the SGPIO initiator in the SAS
> initiator. The implementation is SFF-8485 and SAS-2 compliant.
>
> There are two functions in transport class only responsible for reading and
> writting GPIO registers. I decided to leave the decission about what type of
> registers are supported to lldd.
>
> Now the user space application may issue SMP READ/WRITE GPIO frame to HBA in
> order to read/write GPIO registers.
>
> Signed-off-by: Artur Wojcik <artur.wojcik@intel.com>
> ---
>  drivers/scsi/libsas/sas_host_smp.c |   48 +++++++++++++++++++++++++++++++++++-
>  include/scsi/libsas.h              |   14 +++++++++++
>  include/scsi/sas.h                 |    4 +++
>  3 files changed, 64 insertions(+), 2 deletions(-)
>

James, thoughts?

The thing that struck me most about this patch was the following
subtle deletion:

> -               /* FIXME: need GPIO support in the transport class */

Transport class support in my mind implies sysfs representation of the
sgpio targets, this patch is just providing an interface for tickling
sgpio-initiator registers in the hba.  This is likely fine to get the
conversation started, but it punts on the bigger question on
whether/how to describe the sgpio initiator topology to userspace.
This patch also enforces that the interface to the lldd is raw sgpio
protocol, which seems like a passable idea to me.  If a vendor does
something non-standard they can internally translate sgpio to
vendor-specific.
--
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] libsas: add SMP READ/WRTIE GPIO support

[James Bottomley]

On Wed, 2011-06-01 at 19:02 +0200, Artur Wojcik wrote:
> On 06/01/2011 04:32 PM, Douglas Gilbert wrote:
> > On 11-06-01 10:02 AM, Artur Wojcik wrote:
> >> This patch enables the SMP READ/WRITE GPIO function interface in 
> >> libsas SMP
> >> host module. The interface is used to control the SGPIO initiator in 
> >> the SAS
> >> initiator. The implementation is SFF-8485 and SAS-2 compliant.
> >>
> >> There are two functions in transport class only responsible for 
> >> reading and
> >> writting GPIO registers. I decided to leave the decission about what 
> >> type of
> >> registers are supported to lldd.
> >>
> >> Now the user space application may issue SMP READ/WRITE GPIO frame to 
> >> HBA in
> >> order to read/write GPIO registers.
> >
> > Your code also accepts the somewhat mysterious READ GPIO
> > REGISTER ENHANCED SMP function and its WRITE equivalent.
> > Reference is made in sas2r16.pdf to SFF-8485 for those
> > functions. However the most recent version of SFF-8485
> > on the Seagate site is 0.7 dated February 2006, and it does
> > not define those ENHANCED functions.
> >
> > Could you tell us what those ENHANCED versions do?
> >
> > Doug Gilbert
> I have not found the description of ENHANCED frames in any publicly 
> available specification related to SGPIO. I took SAS-2 as the major 
> reference and it says SMP functions 02h (READ) and 82h (WRITE) are 
> obsolete. My understanding is that for the moment the new functions are 
> replacing the deprecated functions, but the frame format and registers 
> set stay the same.

Then why change the function numbers?  I mean if you're keeping the call
and return compatible, you can just reuse the now obsoleted functions.
Usually you only change functions if you have to break compatibility
somehow (like order or length of parameters).   Since the current
implementation critically depends on this (because you break out the
fields from the frame) I'd be very wary of putting this in only to have
it broken again by a later revision of SFF-8485

James

Re: [PATCH] libsas: add SMP READ/WRTIE GPIO support

[James Bottomley]

On Wed, 2011-08-03 at 15:02 -0500, James Bottomley wrote:
> On Wed, 2011-06-01 at 19:02 +0200, Artur Wojcik wrote:
> > On 06/01/2011 04:32 PM, Douglas Gilbert wrote:
> > > On 11-06-01 10:02 AM, Artur Wojcik wrote:
> > >> This patch enables the SMP READ/WRITE GPIO function interface in 
> > >> libsas SMP
> > >> host module. The interface is used to control the SGPIO initiator in 
> > >> the SAS
> > >> initiator. The implementation is SFF-8485 and SAS-2 compliant.
> > >>
> > >> There are two functions in transport class only responsible for 
> > >> reading and
> > >> writting GPIO registers. I decided to leave the decission about what 
> > >> type of
> > >> registers are supported to lldd.
> > >>
> > >> Now the user space application may issue SMP READ/WRITE GPIO frame to 
> > >> HBA in
> > >> order to read/write GPIO registers.
> > >
> > > Your code also accepts the somewhat mysterious READ GPIO
> > > REGISTER ENHANCED SMP function and its WRITE equivalent.
> > > Reference is made in sas2r16.pdf to SFF-8485 for those
> > > functions. However the most recent version of SFF-8485
> > > on the Seagate site is 0.7 dated February 2006, and it does
> > > not define those ENHANCED functions.
> > >
> > > Could you tell us what those ENHANCED versions do?
> > >
> > > Doug Gilbert
> > I have not found the description of ENHANCED frames in any publicly 
> > available specification related to SGPIO. I took SAS-2 as the major 
> > reference and it says SMP functions 02h (READ) and 82h (WRITE) are 
> > obsolete. My understanding is that for the moment the new functions are 
> > replacing the deprecated functions, but the frame format and registers 
> > set stay the same.
> 
> Then why change the function numbers?  I mean if you're keeping the call
> and return compatible, you can just reuse the now obsoleted functions.
> Usually you only change functions if you have to break compatibility
> somehow (like order or length of parameters).   Since the current
> implementation critically depends on this (because you break out the
> fields from the frame) I'd be very wary of putting this in only to have
> it broken again by a later revision of SFF-8485

OK, I got the answer.  It's buried deeply in the 14h ballot comment
resolutions:

http://www.t10.org/cgi-bin/ac.pl?t=d&f=08-212r8.pdf

SFF-8485 v 0.7 actually has an incorrect format for the frame because it
doesn't take into account the required request length/response length
fields which have to occupy bytes 2 and 3, so they're going to rev
SFF-8485 to conform to the SMP frame requirements.  That means all the
fields will shift and your patch will be wrong. We can guess, based on
this that the register fields will begin at offset 4.
 
James