[patch 6/8] mptfusion: fc transport

[patch 6/8] mptfusion: fc transport

[Moore, Eric Dean]

[-- Attachment #1: Type: text/plain, Size: 133 bytes --]

This provides fc_transport support, exporting
port_id, port_name, node_name.

Signed-off-by: Eric Moore <Eric.Moore@lsil.com>








[-- Attachment #2: mptfusion-6.patch --]
[-- Type: application/octet-stream, Size: 7252 bytes --]

diff -uarN b/drivers/message/fusion/Kconfig a/drivers/message/fusion/Kconfig
--- b/drivers/message/fusion/Kconfig	2004-11-17 16:24:38.000000000 -0700
+++ a/drivers/message/fusion/Kconfig	2004-11-18 12:28:57.000000000 -0700
@@ -4,6 +4,7 @@
 config FUSION
 	tristate "Fusion MPT (base + ScsiHost) drivers"
 	depends on PCI && SCSI
+        select SCSI_FC_ATTRS
 	---help---
 	  LSI Logic Fusion(TM) Message Passing Technology (MPT) device support
 	  provides high performance SCSI host initiator, and LAN [1] interface
diff -uarN b/drivers/message/fusion/Makefile a/drivers/message/fusion/Makefile
--- b/drivers/message/fusion/Makefile	2004-11-18 12:18:18.000000000 -0700
+++ a/drivers/message/fusion/Makefile	2004-11-18 12:24:53.000000000 -0700
@@ -33,6 +33,7 @@
 #CFLAGS_mptscsih.o += -DMPT_DEBUG_SCANDV
 #CFLAGS_mptscsih.o += -DMPT_DEBUG_RESET
 #CFLAGS_mptscsih.o += -DMPT_DEBUG_NEH
+CFLAGS_mptscsih.o += -DMPTSCSIH_ENABLE_FC_TRANSPORT
 #
 #  For mptctl:
 #CFLAGS_mptctl.o += -DMPT_DEBUG_IOCTL
diff -uarN b/drivers/message/fusion/mptscsih.c a/drivers/message/fusion/mptscsih.c
--- b/drivers/message/fusion/mptscsih.c	2004-11-18 14:54:27.000000000 -0700
+++ a/drivers/message/fusion/mptscsih.c	2004-11-18 14:54:45.000000000 -0700
@@ -83,6 +83,8 @@
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_tcq.h>
+#include <scsi/scsi_transport.h>
+#include <scsi/scsi_transport_fc.h>
 
 #include "mptbase.h"
 #include "mptscsih.h"
@@ -259,6 +261,10 @@
 static struct mptscsih_driver_setup driver_setup;
 static struct scsi_host_template driver_template;
 
+#if defined(MPTSCSIH_ENABLE_FC_TRANSPORT)
+static struct scsi_transport_template *mptscsih_transport_template = NULL;
+#endif
+
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /**
  *	mptscsih_add_sge - Place a simple SGE at address pAddr.
@@ -1129,7 +1135,13 @@
 	sh->max_lun = MPT_LAST_LUN + 1;
 	sh->max_channel = 0;
 	sh->this_id = ioc->pfacts[0].PortSCSIID;
-		
+	
+#if defined(MPTSCSIH_ENABLE_FC_TRANSPORT)
+	if (ioc->bus_type == FC) {
+		BUG_ON(mptscsih_transport_template == NULL);
+		sh->transportt = mptscsih_transport_template;
+	}
+#endif
 	/* Required entry.
 	 */
 	sh->unique_id = ioc->id;
@@ -1301,6 +1313,145 @@
 
 }
 
+#if defined(MPTSCSIH_ENABLE_FC_TRANSPORT)
+/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+/*
+ *	GetFcDevicePage0 - returns FC Device Page 0 data
+ *	@ioc: Pointer to MPT_ADAPTER structure
+ *	@bus: bus id
+ *	@targetId: target id
+ *	@fcDevicePage: FC Device Page 0 data
+ *
+ *	Returns count of number bytes copied into @fcDevicePage
+ *
+ */
+
+static int
+GetFcDevicePage0(MPT_ADAPTER *ioc, u8 bus, u8 targetId, pFCDevicePage0_t fcDevicePage)
+{
+	ConfigPageHeader_t	 hdr;
+	CONFIGPARMS		 cfg;
+	pFCDevicePage0_t	 ppage0_alloc;
+	dma_addr_t		 page0_dma;
+	int			 data_sz;
+	int			 copy_sz=0;
+	int			 rc;
+
+	/* Get FCPort Page 0 header */
+	hdr.PageVersion = 0;
+	hdr.PageLength = 0;
+	hdr.PageNumber = 0;
+	hdr.PageType = MPI_CONFIG_PAGETYPE_FC_DEVICE;
+	cfg.hdr = &hdr;
+	cfg.physAddr = -1;
+	cfg.action = MPI_CONFIG_ACTION_PAGE_HEADER;
+	cfg.dir = 0;
+
+	cfg.pageAddr = (bus << 8) + targetId + MPI_FC_DEVICE_PGAD_FORM_BUS_TID;
+	cfg.timeout = 0;
+
+	if ((rc = mpt_config(ioc, &cfg)) != 0)
+		return 0;
+
+	if (hdr.PageLength == 0)
+		return 0;
+
+	data_sz = hdr.PageLength * 4;
+	rc = -ENOMEM;
+	ppage0_alloc = (pFCDevicePage0_t ) pci_alloc_consistent(ioc->pcidev,
+	    data_sz, &page0_dma);
+	if (ppage0_alloc) {
+		memset((u8 *)ppage0_alloc, 0, data_sz);
+		cfg.physAddr = page0_dma;
+		cfg.action = MPI_CONFIG_ACTION_PAGE_READ_CURRENT;
+
+		if ((rc = mpt_config(ioc, &cfg)) == 0) {
+			/* save the data */
+			copy_sz = min_t(int, sizeof(FCDevicePage0_t), data_sz);
+			memcpy(fcDevicePage, ppage0_alloc, copy_sz);
+		}
+
+		pci_free_consistent(ioc->pcidev, data_sz, (u8 *) ppage0_alloc,
+		    page0_dma);
+	}
+
+	return copy_sz;
+}
+
+static void
+mptscsih_get_port_id(struct scsi_target *starget)
+{
+	struct Scsi_Host *host = dev_to_shost(starget->dev.parent);
+	MPT_SCSI_HOST	*hd;
+	MPT_ADAPTER 	*ioc;
+	FCDevicePage0_t fcDevicePage;
+	int rc;
+
+	fc_starget_port_id(starget) = -1;
+	hd = (MPT_SCSI_HOST *)host->hostdata;
+	if (hd==NULL)
+		return;
+	ioc = hd->ioc;
+	rc = GetFcDevicePage0(ioc, starget->channel, starget->id, &fcDevicePage);
+	if (rc > offsetof(FCDevicePage0_t,Protocol))
+		fc_starget_port_id(starget) = le64_to_cpu(fcDevicePage.PortIdentifier);
+}
+
+static void
+mptscsih_get_port_name(struct scsi_target *starget)
+{
+	struct Scsi_Host *host = dev_to_shost(starget->dev.parent);
+	MPT_SCSI_HOST	*hd;
+	MPT_ADAPTER 	*ioc;
+	FCDevicePage0_t fcDevicePage;
+	int rc;
+
+	fc_starget_port_name(starget) = -1;
+	hd = (MPT_SCSI_HOST *)host->hostdata;
+	if (hd==NULL)
+		return;
+	ioc = hd->ioc;
+	rc = GetFcDevicePage0(ioc, starget->channel, starget->id, &fcDevicePage);
+	if (rc > offsetof(FCDevicePage0_t,PortIdentifier)) {
+		memcpy(&fc_starget_port_name(starget),&fcDevicePage.WWPN,
+		    sizeof(fcDevicePage.WWPN));
+		le64_to_cpus(&fc_starget_port_name(starget));
+	}
+}
+
+static void
+mptscsih_get_node_name(struct scsi_target *starget)
+{
+	struct Scsi_Host *host = dev_to_shost(starget->dev.parent);
+	MPT_SCSI_HOST	*hd;
+	MPT_ADAPTER 	*ioc;
+	FCDevicePage0_t fcDevicePage;
+	int rc;
+
+	fc_starget_node_name(starget) = -1;
+	hd = (MPT_SCSI_HOST *)host->hostdata;
+	if (hd==NULL)
+		return;
+	ioc = hd->ioc;
+	rc = GetFcDevicePage0(ioc, starget->channel, starget->id, &fcDevicePage);
+	if (rc > offsetof(FCDevicePage0_t,WWPN)) {
+		memcpy(&fc_starget_node_name(starget),&fcDevicePage.WWNN,
+		    sizeof(fcDevicePage.WWNN));
+		le64_to_cpus(&fc_starget_node_name(starget));
+	}
+}
+
+static struct fc_function_template mptscsih_transport_functions = {
+	.get_starget_port_id = mptscsih_get_port_id,
+	.show_starget_port_id = 1,
+	.get_starget_port_name = mptscsih_get_port_name,
+	.show_starget_port_name = 1,
+	.get_starget_node_name = mptscsih_get_node_name,
+	.show_starget_node_name = 1,
+};
+#endif
+
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /*
  *	mptscsih_remove - Removed scsi devices
@@ -1481,6 +1632,13 @@
 
 	show_mptmod_ver(my_NAME, my_VERSION);
 
+#if defined(MPTSCSIH_ENABLE_FC_TRANSPORT)
+	mptscsih_transport_template = 
+	    fc_attach_transport(&mptscsih_transport_functions);
+	if (!mptscsih_transport_template)
+		return -ENODEV;
+#endif
+	
 	ScsiDoneCtx = mpt_register(mptscsih_io_done, MPTSCSIH_DRIVER);
 	ScsiTaskCtx = mpt_register(mptscsih_taskmgmt_complete, MPTSCSIH_DRIVER);
 	ScsiScanDvCtx = mpt_register(mptscsih_scandv_complete, MPTSCSIH_DRIVER);
@@ -1528,6 +1686,9 @@
 {
 	mpt_device_driver_deregister(MPTSCSIH_DRIVER);
 
+#if defined(MPTSCSIH_ENABLE_FC_TRANSPORT)
+	fc_release_transport(mptscsih_transport_template);
+#endif
 	mpt_reset_deregister(ScsiDoneCtx);
 	dprintk((KERN_INFO MYNAM
 	  ": Deregistered for IOC reset notifications\n"));

[patch 6/8] mptfusion: fc transport

[Moore, Eric Dean]

[-- Attachment #1: Type: text/plain, Size: 132 bytes --]

This provides fc_transport support, exporting
port_id, port_name, node_name.

Signed-off-by: Eric Moore <Eric.Moore@lsil.com>







[-- Attachment #2: mptfusion-6.patch --]
[-- Type: application/octet-stream, Size: 7252 bytes --]

diff -uarN b/drivers/message/fusion/Kconfig a/drivers/message/fusion/Kconfig
--- b/drivers/message/fusion/Kconfig	2004-11-17 16:24:38.000000000 -0700
+++ a/drivers/message/fusion/Kconfig	2004-11-18 12:28:57.000000000 -0700
@@ -4,6 +4,7 @@
 config FUSION
 	tristate "Fusion MPT (base + ScsiHost) drivers"
 	depends on PCI && SCSI
+        select SCSI_FC_ATTRS
 	---help---
 	  LSI Logic Fusion(TM) Message Passing Technology (MPT) device support
 	  provides high performance SCSI host initiator, and LAN [1] interface
diff -uarN b/drivers/message/fusion/Makefile a/drivers/message/fusion/Makefile
--- b/drivers/message/fusion/Makefile	2004-11-18 12:18:18.000000000 -0700
+++ a/drivers/message/fusion/Makefile	2004-11-18 12:24:53.000000000 -0700
@@ -33,6 +33,7 @@
 #CFLAGS_mptscsih.o += -DMPT_DEBUG_SCANDV
 #CFLAGS_mptscsih.o += -DMPT_DEBUG_RESET
 #CFLAGS_mptscsih.o += -DMPT_DEBUG_NEH
+CFLAGS_mptscsih.o += -DMPTSCSIH_ENABLE_FC_TRANSPORT
 #
 #  For mptctl:
 #CFLAGS_mptctl.o += -DMPT_DEBUG_IOCTL
diff -uarN b/drivers/message/fusion/mptscsih.c a/drivers/message/fusion/mptscsih.c
--- b/drivers/message/fusion/mptscsih.c	2004-11-18 14:54:27.000000000 -0700
+++ a/drivers/message/fusion/mptscsih.c	2004-11-18 14:54:45.000000000 -0700
@@ -83,6 +83,8 @@
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_tcq.h>
+#include <scsi/scsi_transport.h>
+#include <scsi/scsi_transport_fc.h>
 
 #include "mptbase.h"
 #include "mptscsih.h"
@@ -259,6 +261,10 @@
 static struct mptscsih_driver_setup driver_setup;
 static struct scsi_host_template driver_template;
 
+#if defined(MPTSCSIH_ENABLE_FC_TRANSPORT)
+static struct scsi_transport_template *mptscsih_transport_template = NULL;
+#endif
+
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /**
  *	mptscsih_add_sge - Place a simple SGE at address pAddr.
@@ -1129,7 +1135,13 @@
 	sh->max_lun = MPT_LAST_LUN + 1;
 	sh->max_channel = 0;
 	sh->this_id = ioc->pfacts[0].PortSCSIID;
-		
+	
+#if defined(MPTSCSIH_ENABLE_FC_TRANSPORT)
+	if (ioc->bus_type == FC) {
+		BUG_ON(mptscsih_transport_template == NULL);
+		sh->transportt = mptscsih_transport_template;
+	}
+#endif
 	/* Required entry.
 	 */
 	sh->unique_id = ioc->id;
@@ -1301,6 +1313,145 @@
 
 }
 
+#if defined(MPTSCSIH_ENABLE_FC_TRANSPORT)
+/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+/*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
+/*
+ *	GetFcDevicePage0 - returns FC Device Page 0 data
+ *	@ioc: Pointer to MPT_ADAPTER structure
+ *	@bus: bus id
+ *	@targetId: target id
+ *	@fcDevicePage: FC Device Page 0 data
+ *
+ *	Returns count of number bytes copied into @fcDevicePage
+ *
+ */
+
+static int
+GetFcDevicePage0(MPT_ADAPTER *ioc, u8 bus, u8 targetId, pFCDevicePage0_t fcDevicePage)
+{
+	ConfigPageHeader_t	 hdr;
+	CONFIGPARMS		 cfg;
+	pFCDevicePage0_t	 ppage0_alloc;
+	dma_addr_t		 page0_dma;
+	int			 data_sz;
+	int			 copy_sz=0;
+	int			 rc;
+
+	/* Get FCPort Page 0 header */
+	hdr.PageVersion = 0;
+	hdr.PageLength = 0;
+	hdr.PageNumber = 0;
+	hdr.PageType = MPI_CONFIG_PAGETYPE_FC_DEVICE;
+	cfg.hdr = &hdr;
+	cfg.physAddr = -1;
+	cfg.action = MPI_CONFIG_ACTION_PAGE_HEADER;
+	cfg.dir = 0;
+
+	cfg.pageAddr = (bus << 8) + targetId + MPI_FC_DEVICE_PGAD_FORM_BUS_TID;
+	cfg.timeout = 0;
+
+	if ((rc = mpt_config(ioc, &cfg)) != 0)
+		return 0;
+
+	if (hdr.PageLength == 0)
+		return 0;
+
+	data_sz = hdr.PageLength * 4;
+	rc = -ENOMEM;
+	ppage0_alloc = (pFCDevicePage0_t ) pci_alloc_consistent(ioc->pcidev,
+	    data_sz, &page0_dma);
+	if (ppage0_alloc) {
+		memset((u8 *)ppage0_alloc, 0, data_sz);
+		cfg.physAddr = page0_dma;
+		cfg.action = MPI_CONFIG_ACTION_PAGE_READ_CURRENT;
+
+		if ((rc = mpt_config(ioc, &cfg)) == 0) {
+			/* save the data */
+			copy_sz = min_t(int, sizeof(FCDevicePage0_t), data_sz);
+			memcpy(fcDevicePage, ppage0_alloc, copy_sz);
+		}
+
+		pci_free_consistent(ioc->pcidev, data_sz, (u8 *) ppage0_alloc,
+		    page0_dma);
+	}
+
+	return copy_sz;
+}
+
+static void
+mptscsih_get_port_id(struct scsi_target *starget)
+{
+	struct Scsi_Host *host = dev_to_shost(starget->dev.parent);
+	MPT_SCSI_HOST	*hd;
+	MPT_ADAPTER 	*ioc;
+	FCDevicePage0_t fcDevicePage;
+	int rc;
+
+	fc_starget_port_id(starget) = -1;
+	hd = (MPT_SCSI_HOST *)host->hostdata;
+	if (hd==NULL)
+		return;
+	ioc = hd->ioc;
+	rc = GetFcDevicePage0(ioc, starget->channel, starget->id, &fcDevicePage);
+	if (rc > offsetof(FCDevicePage0_t,Protocol))
+		fc_starget_port_id(starget) = le64_to_cpu(fcDevicePage.PortIdentifier);
+}
+
+static void
+mptscsih_get_port_name(struct scsi_target *starget)
+{
+	struct Scsi_Host *host = dev_to_shost(starget->dev.parent);
+	MPT_SCSI_HOST	*hd;
+	MPT_ADAPTER 	*ioc;
+	FCDevicePage0_t fcDevicePage;
+	int rc;
+
+	fc_starget_port_name(starget) = -1;
+	hd = (MPT_SCSI_HOST *)host->hostdata;
+	if (hd==NULL)
+		return;
+	ioc = hd->ioc;
+	rc = GetFcDevicePage0(ioc, starget->channel, starget->id, &fcDevicePage);
+	if (rc > offsetof(FCDevicePage0_t,PortIdentifier)) {
+		memcpy(&fc_starget_port_name(starget),&fcDevicePage.WWPN,
+		    sizeof(fcDevicePage.WWPN));
+		le64_to_cpus(&fc_starget_port_name(starget));
+	}
+}
+
+static void
+mptscsih_get_node_name(struct scsi_target *starget)
+{
+	struct Scsi_Host *host = dev_to_shost(starget->dev.parent);
+	MPT_SCSI_HOST	*hd;
+	MPT_ADAPTER 	*ioc;
+	FCDevicePage0_t fcDevicePage;
+	int rc;
+
+	fc_starget_node_name(starget) = -1;
+	hd = (MPT_SCSI_HOST *)host->hostdata;
+	if (hd==NULL)
+		return;
+	ioc = hd->ioc;
+	rc = GetFcDevicePage0(ioc, starget->channel, starget->id, &fcDevicePage);
+	if (rc > offsetof(FCDevicePage0_t,WWPN)) {
+		memcpy(&fc_starget_node_name(starget),&fcDevicePage.WWNN,
+		    sizeof(fcDevicePage.WWNN));
+		le64_to_cpus(&fc_starget_node_name(starget));
+	}
+}
+
+static struct fc_function_template mptscsih_transport_functions = {
+	.get_starget_port_id = mptscsih_get_port_id,
+	.show_starget_port_id = 1,
+	.get_starget_port_name = mptscsih_get_port_name,
+	.show_starget_port_name = 1,
+	.get_starget_node_name = mptscsih_get_node_name,
+	.show_starget_node_name = 1,
+};
+#endif
+
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /*
  *	mptscsih_remove - Removed scsi devices
@@ -1481,6 +1632,13 @@
 
 	show_mptmod_ver(my_NAME, my_VERSION);
 
+#if defined(MPTSCSIH_ENABLE_FC_TRANSPORT)
+	mptscsih_transport_template = 
+	    fc_attach_transport(&mptscsih_transport_functions);
+	if (!mptscsih_transport_template)
+		return -ENODEV;
+#endif
+	
 	ScsiDoneCtx = mpt_register(mptscsih_io_done, MPTSCSIH_DRIVER);
 	ScsiTaskCtx = mpt_register(mptscsih_taskmgmt_complete, MPTSCSIH_DRIVER);
 	ScsiScanDvCtx = mpt_register(mptscsih_scandv_complete, MPTSCSIH_DRIVER);
@@ -1528,6 +1686,9 @@
 {
 	mpt_device_driver_deregister(MPTSCSIH_DRIVER);
 
+#if defined(MPTSCSIH_ENABLE_FC_TRANSPORT)
+	fc_release_transport(mptscsih_transport_template);
+#endif
 	mpt_reset_deregister(ScsiDoneCtx);
 	dprintk((KERN_INFO MYNAM
 	  ": Deregistered for IOC reset notifications\n"));

Re: [patch 6/8] mptfusion: fc transport

[James Bottomley]

On Fri, 2004-11-19 at 09:39, Moore, Eric Dean wrote:
> This provides fc_transport support, exporting
> port_id, port_name, node_name.

This:

@@ -4,6 +4,7 @@
 config FUSION
        tristate "Fusion MPT (base + ScsiHost) drivers"
        depends on PCI && SCSI
+        select SCSI_FC_ATTRS
        ---help---

Doesn't look correct.  It will force the FC attrs to be built even if
you're only building for a purely SPI device.  Worse, module
dependencies will force the FC transport to load even if you're only
attaching to a SCSI device.  This isn't really optimal, especially as
the FC transport class is growing in size now.

How difficult would it be to separate out the scsi and FC pieces of the
fusion driver, so we could allow more fined grained control of this?

James

RE: [patch 6/8] mptfusion: fc transport

[Moore, Eric Dean]

On Thursday, November 25, 2004 1:55 PM, James Bottomley wrote:
> On Fri, 2004-11-19 at 09:39, Moore, Eric Dean wrote:
> > This provides fc_transport support, exporting
> > port_id, port_name, node_name.
> 
> This:
> 
> @@ -4,6 +4,7 @@
>  config FUSION
>         tristate "Fusion MPT (base + ScsiHost) drivers"
>         depends on PCI && SCSI
> +        select SCSI_FC_ATTRS
>         ---help---
> 
> Doesn't look correct.  It will force the FC attrs to be built even if
> you're only building for a purely SPI device.  Worse, module
> dependencies will force the FC transport to load even if you're only
> attaching to a SCSI device.  This isn't really optimal, especially as
> the FC transport class is growing in size now.
> 
> How difficult would it be to separate out the scsi and FC 
> pieces of the
> fusion driver, so we could allow more fined grained control of this?
> 

 
This driver is four separate driver modules { mptbase, mptscsih, mptlan,
mptctl },
with FC and SCSI spread through out, and SAS waiting in the wings.
In this current design, separating FC is not going to be an easy task,
as well as dealing with SAS because I'm sure people would want that
separated as well.  
Device recognization done from mptbase, as well as unique
configuration/initialization
for respective bus type, and messaging interface from the other drivers
(mptscsih,
mptlan, and mptctl) to the firmware.  The scsi interface layer is in
mptscsih, 
and is where this fc transports and our spi domain validation is done
currently. 

What do you have in mind in seperating this code for each (3) different bus
classes from 
each these (4) drivers?  

Eric

RE: [patch 6/8] mptfusion: fc transport

[James Bottomley]

On Wed, 2004-12-01 at 14:46, Moore, Eric Dean wrote:
> On Thursday, November 25, 2004 1:55 PM, James Bottomley wrote:
> > How difficult would it be to separate out the scsi and FC 
> > pieces of the
> > fusion driver, so we could allow more fined grained control of this?
>  
> This driver is four separate driver modules { mptbase, mptscsih, mptlan,
> mptctl },
> with FC and SCSI spread through out, and SAS waiting in the wings.
> In this current design, separating FC is not going to be an easy task,
> as well as dealing with SAS because I'm sure people would want that
> separated as well.  
> Device recognization done from mptbase, as well as unique
> configuration/initialization
> for respective bus type, and messaging interface from the other drivers
> (mptscsih,
> mptlan, and mptctl) to the firmware.  The scsi interface layer is in
> mptscsih, 
> and is where this fc transports and our spi domain validation is done
> currently. 
> 
> What do you have in mind in seperating this code for each (3) different bus
> classes from 
> each these (4) drivers?  

Nothing complex.  How about just a simple module wrapper for the PCI
attachment, so you have the FC devices in one, the SPI in another etc. 
Then you could attach and initialise the FC transport only if the user
modprobes the FC driver.

I just don't see it being tenable to drag in the whole of the fc
transport class in a SCSI only configuration for fusion.

James

RE: [patch 6/8] mptfusion: fc transport

[Moore, Eric Dean]

On Wednesday, December 01, 2004 1:50 PM, James Bottomley wrote:
> Nothing complex.  How about just a simple module wrapper for the PCI
> attachment, so you have the FC devices in one, the SPI in 
> another etc. 
> Then you could attach and initialise the FC transport only if the user
> modprobes the FC driver.
> 
> I just don't see it being tenable to drag in the whole of the fc
> transport class in a SCSI only configuration for fusion.
> 

This sounds to me that your asking for a separate drivers for SCSI, FC and
SAS. Right?
Meaning in addition to mptscsih.ko, we would have mptfc.ko, mptsas.ko. Each
having its unique module_init, module_exit, and probe routines.  
If so, code will triple because now we will be replicating same source
across drivers 
that handle scsi queued commands, interrupt handling, error recovery,
firmware
asyn event handling, etc.  

Eric Moore

RE: [patch 6/8] mptfusion: fc transport

[Moore, Eric Dean]

Why does spi_attach_transport or fc_attach_transport have to be
done from the module_init routines?  Meaning there is an assumption
that spi and FC are separate drivers.  Why can't the xxx_attach_transport be
done
later from the probe routines when we know which pci devices we are
attaching?

The problem is the mpt fusion drivers use the same host to firmware 
interface several bus types, e.g. U320 SCSI, 2gb and 4gb FC, and SAS
chipsets 
are using the same message passing protocol.  Bottom line, we are not going
to 
create separate drivers for SCSI, FC, and SAS, having redundant code through
out.
I'm sure other vendors having unified drivers for several bus types in the
kernel source
tree.  I would assume this will be the case for other vendors when SAS
starts getting added.

Also another problem. How about creating driver update disks?  It appears to
me this process is now
broken as driver linking xxx_attach_transport will not load the LLDs.
Now we will need to include scsi_transport_fc.ko and scsi_transport_spi.ko 
on the floppy, and hacking required so these transport ko's are loaded prior
to the LLDs.

Eric

On Wednesday, December 01, 2004 4:41 PM, Moore, Eric Dean wrote:
> On Wednesday, December 01, 2004 1:50 PM, James Bottomley wrote:
> > Nothing complex.  How about just a simple module wrapper for the PCI
> > attachment, so you have the FC devices in one, the SPI in 
> > another etc. 
> > Then you could attach and initialise the FC transport only 
> if the user
> > modprobes the FC driver.
> > 
> > I just don't see it being tenable to drag in the whole of the fc
> > transport class in a SCSI only configuration for fusion.
> > 
> 
> This sounds to me that your asking for a separate drivers for 
> SCSI, FC and SAS. Right?
> Meaning in addition to mptscsih.ko, we would have mptfc.ko, 
> mptsas.ko. Each
> having its unique module_init, module_exit, and probe routines.  
> If so, code will triple because now we will be replicating 
> same source across drivers 
> that handle scsi queued commands, interrupt handling, error 
> recovery, firmware
> asyn event handling, etc.  
> 
> Eric Moore
> 

Re: [patch 6/8] mptfusion: fc transport

[Christoph Hellwig]

On Fri, Dec 03, 2004 at 10:33:17AM -0700, Moore, Eric Dean wrote:
> Why does spi_attach_transport or fc_attach_transport have to be
> done from the module_init routines?  Meaning there is an assumption
> that spi and FC are separate drivers.  Why can't the xxx_attach_transport be
> done
> later from the probe routines when we know which pci devices we are
> attaching?

That's fine.  But as soon as a module uses symbols from another module that
second module needs to be loaded.

But I think splitting doesn't make a lot of sense here, we can introduce
config options to disable SPI/FC/SAS support so people building custom
kernels don't waste memory.

RE: [patch 6/8] mptfusion: fc transport

[James Bottomley]

On Fri, 2004-12-03 at 11:33, Moore, Eric Dean wrote:
> Why does spi_attach_transport or fc_attach_transport have to be
> done from the module_init routines?  Meaning there is an assumption
> that spi and FC are separate drivers.  Why can't the xxx_attach_transport be
> done
> later from the probe routines when we know which pci devices we are
> attaching?

Can we take a step back from this and look at the actual problem you're
trying to solve with the fc class?

At the moment, its only real use is to display certain device
parameters, which is a passive feature (although it now has the capacity
to control cable pull blocking, which will be an active facility).

Your requirement came from SGI, who cares about the parameters.  What
I'm proposing is that you move the mptbase_pci_table out of mptbase.c
and into *two* upper modules (you could even do it into mptscsih.c and a
new mptfc.c, which would depend on mptscsih).  Now you have the FC
devices registered separately, you can put all of the fc transport
display into mptfc.c.  This involves moving no code other than the
actual PCI table.  Linux will automatically only modprobe mptfc if the
user has a FC device PCI ID and everyone will be happy.

Why is this such a hard thing to do?

James

RE: [patch 6/8] mptfusion: fc transport

[Moore, Eric Dean]

Your wanting a separate module mptfc.ko for fc pci ids, right? 
Which means current fc pci ids that are supported in mptscshi.ko, 
are now moved to new driver mptfc.ko. LSI Management told me NO 
to this solution.
  
Our concern is the impact to linux distributions installers and
end users that upgrade to these newer drivers.  
Example: Someone having older driver using boot device attached
to our controller.  Then they download a newer kernel from Kernel.org with
newer
drivers having mptfc.  They mkinitrd, or mk_initrd (without modifing
linuxrc),
reboot then what?  Can they see the boot partition?   

Can we change the time when LLDs register themselves to the 
transport layer?  Like from the probe routine.


Eric Moore

On Monday, December 06, 2004 8:18 AM, James Bottomley wrote:
> 
> On Fri, 2004-12-03 at 11:33, Moore, Eric Dean wrote:
> > Why does spi_attach_transport or fc_attach_transport have to be
> > done from the module_init routines?  Meaning there is an assumption
> > that spi and FC are separate drivers.  Why can't the 
> xxx_attach_transport be
> > done
> > later from the probe routines when we know which pci devices we are
> > attaching?
> 
> Can we take a step back from this and look at the actual 
> problem you're
> trying to solve with the fc class?
> 
> At the moment, its only real use is to display certain device
> parameters, which is a passive feature (although it now has 
> the capacity
> to control cable pull blocking, which will be an active facility).
> 
> Your requirement came from SGI, who cares about the parameters.  What
> I'm proposing is that you move the mptbase_pci_table out of mptbase.c
> and into *two* upper modules (you could even do it into 
> mptscsih.c and a
> new mptfc.c, which would depend on mptscsih).  Now you have the FC
> devices registered separately, you can put all of the fc transport
> display into mptfc.c.  This involves moving no code other than the
> actual PCI table.  Linux will automatically only modprobe mptfc if the
> user has a FC device PCI ID and everyone will be happy.
> 
> Why is this such a hard thing to do?
> 
> James
> 
> 

RE: [patch 6/8] mptfusion: fc transport

[James Bottomley]

On Mon, 2004-12-06 at 11:12, Moore, Eric Dean wrote:
> Our concern is the impact to linux distributions installers and
> end users that upgrade to these newer drivers.  
> Example: Someone having older driver using boot device attached
> to our controller.  Then they download a newer kernel from Kernel.org with
> newer
> drivers having mptfc.  They mkinitrd, or mk_initrd (without modifing
> linuxrc),
> reboot then what?  Can they see the boot partition?   

That's a concern only for people who boot from your fibre adapters.  How
many such users are there?

> Can we change the time when LLDs register themselves to the 
> transport layer?  Like from the probe routine.

Well, not really, unless you have a proposal to get around the
fundamental problem Christoph already pointed out:  any reference to the
fc_ transport methods will automatically cause modprobe to load it
regardless of whether it is used.

James

RE: [patch 6/8] mptfusion: fc transport

[Moore, Eric Dean]

Monday, December 06, 2004 10:21 AM, James Bottomley wrote:
> 
> On Mon, 2004-12-06 at 11:12, Moore, Eric Dean wrote:
> > Our concern is the impact to linux distributions installers and
> > end users that upgrade to these newer drivers.  
> > Example: Someone having older driver using boot device attached
> > to our controller.  Then they download a newer kernel from 
> Kernel.org with
> > newer
> > drivers having mptfc.  They mkinitrd, or mk_initrd (without modifing
> > linuxrc),
> > reboot then what?  Can they see the boot partition?   
> 
> That's a concern only for people who boot from your fibre 
> adapters.  How
> many such users are there?

I'm checking into this, and let you know. I'm pretty certain
that there are people who have blade severs connected to a SAN using
our LSI FC controller, and then booting to devices in FC RAID storage 
array(from other vendors) populated out on the SAN.

> 
> > Can we change the time when LLDs register themselves to the 
> > transport layer?  Like from the probe routine.
> 
> Well, not really, unless you have a proposal to get around the
> fundamental problem Christoph already pointed out:  any 
> reference to the
> fc_ transport methods will automatically cause modprobe to load it
> regardless of whether it is used.
> 

Also, I forgot to mention other concerns in previous email.

(1) If mptfc depends on mptscsih, then what is going to be said
when we implement the domain validation using the spi transport layer?
When someone loads mptfc.ko, then we need mptscsih.ko and
scsi_transport_spi.ko
loaded, even though there is no U320 adapter present.  The same complaint
can
be said here.  In my current kernel, scsi_transport_spi.ko = 35KB,
scsi_transport_fc.ko = 27KB.

(2) Also, I think there is more involved than just 
moving mptbase_pci_table from mptbase.c to mptscsih.c.
mptbase_probe is enumerating the bus's when pci_module_init(_)
 is called.  Would mptscsih.c be calling pci_module_init instead?  
If so, will the current code for mptbase_probe still 
work as is, or will it need moving to mptscsih?  If its moved, 
then mptlan.ko and mptctl.ko would not be able to work as it does today?  
They currently depend only on mptbase.ko.

Eric

    

RE: [patch 6/8] mptfusion: fc transport

[James Bottomley]

On Mon, 2004-12-06 at 17:29 -0700, Moore, Eric Dean wrote:
> Monday, December 06, 2004 10:21 AM, James Bottomley wrote:
> > On Mon, 2004-12-06 at 11:12, Moore, Eric Dean wrote:
> > > Our concern is the impact to linux distributions installers and
> > > end users that upgrade to these newer drivers.  
> > > Example: Someone having older driver using boot device attached
> > > to our controller.  Then they download a newer kernel from 
> > Kernel.org with
> > > newer
> > > drivers having mptfc.  They mkinitrd, or mk_initrd (without modifing
> > > linuxrc),
> > > reboot then what?  Can they see the boot partition?   
> > 
> > That's a concern only for people who boot from your fibre 
> > adapters.  How
> > many such users are there?
> 
> I'm checking into this, and let you know. I'm pretty certain
> that there are people who have blade severs connected to a SAN using
> our LSI FC controller, and then booting to devices in FC RAID storage 
> array(from other vendors) populated out on the SAN.

Well, I talked to the people in charge of the Vendor Kernel's at Red Hat
and SuSE.  They have agreed make the installer changes needed to help
people upgrading from the current monolithic fusion driver to a more
modular one.  (Sorry for the delay, it took a while to get everyone's
buy in for this).

> Also, I forgot to mention other concerns in previous email.
> 
> (1) If mptfc depends on mptscsih, then what is going to be said
> when we implement the domain validation using the spi transport layer?
> When someone loads mptfc.ko, then we need mptscsih.ko and
> scsi_transport_spi.ko
> loaded, even though there is no U320 adapter present.  The same complaint
> can
> be said here.  In my current kernel, scsi_transport_spi.ko = 35KB,
> scsi_transport_fc.ko = 27KB.

Actually, I think I'd like this to be a clean modularisation, so the SPI
functions go in one piece, the FC functions in another and the SAS
functions in yet a third.  Then everything could depend on the core
fusion engine for the interface with firmware.  That would allow:

- the transport classes to attach correctly,
- the transport dependent functions (which currently litter the driver
as if (spi) else if(fc) to be contained individually
- and the PCI ID's which will drive the installer and upgrade processes
to be stored in the transport dependent pieces.

> (2) Also, I think there is more involved than just 
> moving mptbase_pci_table from mptbase.c to mptscsih.c.
> mptbase_probe is enumerating the bus's when pci_module_init(_)
>  is called.  Would mptscsih.c be calling pci_module_init instead?  
> If so, will the current code for mptbase_probe still 
> work as is, or will it need moving to mptscsih?  If its moved, 
> then mptlan.ko and mptctl.ko would not be able to work as it does today?  
> They currently depend only on mptbase.ko.

Well, I think the true way it should happen is to make mptbase house
only the functions necessary to implement the fusion firmware API
(although generic functions could be part of it as well).  Then a true
fusion driver would have an mpt transport module, the base module and
optionally the mptlan module.  If I read mptctl right, it only
implements the RAID ioctl features, so it could likewise be optional.

James

RE: [patch 6/8] mptfusion: fc transport

[Moore, Eric Dean]

On Saturday, January 08, 2005 2:33 PM, James Bottomley wrote:
> Actually, I think I'd like this to be a clean modularisation, 
> so the SPI
> functions go in one piece, the FC functions in another and the SAS
> functions in yet a third.  Then everything could depend on the core
> fusion engine for the interface with firmware.  That would allow:
> 
> - the transport classes to attach correctly,
> - the transport dependent functions (which currently litter the driver
> as if (spi) else if(fc) to be contained individually
> - and the PCI ID's which will drive the installer and upgrade 
> processes
> to be stored in the transport dependent pieces.
> 

I agree, and I begun getting others on the same page here.
I will be investigating this design soon. Thanks.
Although, we soon will have some other updates I would like to get
out ahead of this, one of them requested by hch@.

> optionally the mptlan module.  If I read mptctl right, it only
> implements the RAID ioctl features, so it could likewise be optional.
> 

mptctl is raid ioctl's and more.  
We provide our fw(firmware) passthru interface to applications, as
well as fw wrappers.

Eric