Re: [PATCH 3/3] qla4xxx: iospace_config move defn

Re: [PATCH 3/3] qla4xxx: iospace_config move defn

[Mike Christie]

Doug Maxey wrote:
> From: Doug Maxey <dwm@enoyolf.org>
> 
> Move the defn before the caller, and remove the decl.
> 

I think not doing the defn before the caller is nice, but I honestly
could care less either way as far as merging things goes (it seems like
Christoph hates then but people like Jeff let the driver writer do what
they want). Qlogic seems to love doing it, so are they ok with all these
patches?


And one comment...

> Signed-off-by: Doug Maxey <dwm@enoyolf.org>
> ---
> 
> drivers/scsi/qla4xxx/ql4_os.c |  126
> ++++++++++++++++++++---------------------
> 1 files changed, 62 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index f656fb3..30f245a 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -46,8 +46,6 @@ MODULE_PARM_DESC(extended_error_logging,
> 
> void qla4xxx_config_dma_addressing(struct scsi_qla_host *ha);
> 
> -static int qla4xxx_iospace_config(struct scsi_qla_host *ha);
> -
> /*
>  * iSCSI template entry points
>  */
> @@ -1138,6 +1136,68 @@ static void qla4xxx_free_adapter(struct
> }
> 
> +/***
> + * qla4xxx_iospace_config - maps registers
> + * @ha: pointer to adapter structure
> + *
> + * This routines maps HBA's registers from the pci address space
> + * into the kernel virtual address space for memory mapped i/o.
> + **/
> +static int qla4xxx_iospace_config(struct scsi_qla_host *ha)
> +{
> +    unsigned long pio, pio_len, pio_flags;
> +    unsigned long mmio, mmio_len, mmio_flags;
> +
> +    pio = pci_resource_start(ha->pdev, 0);
> +    pio_len = pci_resource_len(ha->pdev, 0);
> +    pio_flags = pci_resource_flags(ha->pdev, 0);
> +    if (pio_flags & IORESOURCE_IO) {
> +        if (pio_len < MIN_IOBASE_LEN) {
> +            dev_warn(&ha->pdev->dev,
> +                   "Invalid PCI I/O region size\n");
> +            pio = 0;
> +        }
> +    } else {
> +        dev_warn(&ha->pdev->dev, "region #0 not a PIO resource\n");
> +        pio = 0;
> +    }
> +

Is my mailer messing up the tabs or are you (maybe the original code was
messed up and did not get fixed in the move)?

Re: [PATCH 3/3] qla4xxx: iospace_config move defn

[Mike Christie]

Mike Christie wrote:
> 
> Is my mailer messing up the tabs or are you (maybe the original code was
> messed up and did not get fixed in the move)?
> 

Nevermind, I switch mailers and it looks ok now. Weird, some patches
looked ok and some did not before.

[PATCHSET] qla4xxx: more qla4xxx cleanup

[Doug Maxey]

Howdy!

The following 3 patch series implements some more qla4xxx cleanup,
which should the the last of these in the one file.

Shortlog:
Doug Maxey:
      qla4xxx: clarify doc for probe_adapter
      qla4xxx: really move free_adapter
      qla4xxx: iospace_config move defn

Diffstats:
 drivers/scsi/qla4xxx/ql4_os.c |  203 ++++++++++++++++++++---------------------
 1 files changed, 99 insertions(+), 104 deletions(-)

---

I really do believe this is the last of the code churn in ql4_os.c.  I
lose track of where I am sometimes, and changes get left behind.  :)

Oh, and hopefully I learned enough about stgit to have the following
patches be replies to this one.  Eike pointed out that last set was
not a reply to the cover letter.  Apparently you cannot save a cover,
edit it, and then send it and have the patches be replies.  At least
not the way I did it.  The message-id generated and used in the patch
mail was not the one in the saved cover letter, so the replies were to
a non-existant (at least on the recipient) message-id.

--

The next series will implement the iscsi specific printk changes
requested by Mike.

--
++doug

[PATCH 1/3] qla4xxx: clarify doc for probe_adapter

[Doug Maxey]

From: Doug Maxey <dwm@enoyolf.org>

Clarify the statments in the kerneldoc header.

Signed-off-by: Doug Maxey <dwm@enoyolf.org>
---

 drivers/scsi/qla4xxx/ql4_os.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index f39eb5c..b14e63a 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -1110,11 +1110,9 @@ static void qla4xxx_do_dpc(void *data)
  * @pdev: pointer to pci_dev structure
  * @pci_device_id: pointer to pci_device entry
  *
- * This routine will probe for Qlogic 4010 iSCSI host adapters.
- * It returns the number of host adapters of a particular
- * type that were found.  It also initializes all data necessary for
- * the driver.	It is passed-in the host number, so that it
- * knows where its first entry is in the scsi_hosts[] array.
+ * This routine will probe for Qlogic 40xx iSCSI host adapters.  It
+ * returns zero if the device successfully configured.  It also
+ * initializes all data necessary for the driver.
  **/
 static int __devinit qla4xxx_probe_adapter(struct pci_dev *pdev,
 					   const struct pci_device_id *ent)

[PATCH 2/3] qla4xxx: really move free_adapter

[Doug Maxey]

From: Doug Maxey <dwm@enoyolf.org>

Ok.  This one got away.  Really move defn to before callers.

Signed-off-by: Doug Maxey <dwm@enoyolf.org>
---

 drivers/scsi/qla4xxx/ql4_os.c |   69 ++++++++++++++++++++---------------------
 1 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index b14e63a..f656fb3 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -47,7 +47,6 @@ MODULE_PARM_DESC(extended_error_logging,
 void qla4xxx_config_dma_addressing(struct scsi_qla_host *ha);
 
 static int qla4xxx_iospace_config(struct scsi_qla_host *ha);
-static void qla4xxx_free_adapter(struct scsi_qla_host *ha);
 
 /*
  * iSCSI template entry points
@@ -1106,6 +1105,40 @@ static void qla4xxx_do_dpc(void *data)
 }
 
 /**
+ * qla4xxx_free_adapter - release the adapter
+ * @ha: pointer to adapter structure
+ **/
+static void qla4xxx_free_adapter(struct scsi_qla_host *ha)
+{
+
+	if (test_bit(AF_INTERRUPTS_ON, &ha->flags)) {
+		/* Turn-off interrupts on the card. */
+		qla4xxx_disable_intrs(ha);
+	}
+
+	/* Kill the kernel thread for this host */
+	if (ha->dpc_thread)
+		destroy_workqueue(ha->dpc_thread);
+
+	/* Issue Soft Reset to put firmware in unknown state */
+	qla4xxx_soft_reset(ha);
+
+	/* Remove timer thread, if present */
+	if (ha->timer_active)
+		qla4xxx_stop_timer(ha);
+
+	/* free extra memory */
+	qla4xxx_mem_free(ha);
+
+	/* Detach interrupts */
+	if (test_and_clear_bit(AF_IRQ_ATTACHED, &ha->flags))
+		free_irq(ha->pdev->irq, ha);
+
+	pci_disable_device(ha->pdev);
+
+}
+
+/**
  * qla4xxx_probe_adapter - callback function to probe HBA
  * @pdev: pointer to pci_dev structure
  * @pci_device_id: pointer to pci_device entry
@@ -1288,40 +1321,6 @@ static void __devexit qla4xxx_remove_ada
 	pci_set_drvdata(pdev, NULL);
 }
 
-/**
- * qla4xxx_free_adapter - release the adapter
- * @ha: pointer to adapter structure
- **/
-static void qla4xxx_free_adapter(struct scsi_qla_host *ha)
-{
-
-	if (test_bit(AF_INTERRUPTS_ON, &ha->flags)) {
-		/* Turn-off interrupts on the card. */
-		qla4xxx_disable_intrs(ha);
-	}
-
-	/* Kill the kernel thread for this host */
-	if (ha->dpc_thread)
-		destroy_workqueue(ha->dpc_thread);
-
-	/* Issue Soft Reset to put firmware in unknown state */
-	qla4xxx_soft_reset(ha);
-
-	/* Remove timer thread, if present */
-	if (ha->timer_active)
-		qla4xxx_stop_timer(ha);
-
-	/* free extra memory */
-	qla4xxx_mem_free(ha);
-
-	/* Detach interrupts */
-	if (test_and_clear_bit(AF_IRQ_ATTACHED, &ha->flags))
-		free_irq(ha->pdev->irq, ha);
-
-	pci_disable_device(ha->pdev);
-
-}
-
 /***
  * qla4xxx_iospace_config - maps registers
  * @ha: pointer to adapter structure

[PATCH 3/3] qla4xxx: iospace_config move defn

[Doug Maxey]

From: Doug Maxey <dwm@enoyolf.org>

Move the defn before the caller, and remove the decl.

Signed-off-by: Doug Maxey <dwm@enoyolf.org>
---

 drivers/scsi/qla4xxx/ql4_os.c |  126 ++++++++++++++++++++---------------------
 1 files changed, 62 insertions(+), 64 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index f656fb3..30f245a 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -46,8 +46,6 @@ MODULE_PARM_DESC(extended_error_logging,
 
 void qla4xxx_config_dma_addressing(struct scsi_qla_host *ha);
 
-static int qla4xxx_iospace_config(struct scsi_qla_host *ha);
-
 /*
  * iSCSI template entry points
  */
@@ -1138,6 +1136,68 @@ static void qla4xxx_free_adapter(struct 
 
 }
 
+/***
+ * qla4xxx_iospace_config - maps registers
+ * @ha: pointer to adapter structure
+ *
+ * This routines maps HBA's registers from the pci address space
+ * into the kernel virtual address space for memory mapped i/o.
+ **/
+static int qla4xxx_iospace_config(struct scsi_qla_host *ha)
+{
+	unsigned long pio, pio_len, pio_flags;
+	unsigned long mmio, mmio_len, mmio_flags;
+
+	pio = pci_resource_start(ha->pdev, 0);
+	pio_len = pci_resource_len(ha->pdev, 0);
+	pio_flags = pci_resource_flags(ha->pdev, 0);
+	if (pio_flags & IORESOURCE_IO) {
+		if (pio_len < MIN_IOBASE_LEN) {
+			dev_warn(&ha->pdev->dev,
+				   "Invalid PCI I/O region size\n");
+			pio = 0;
+		}
+	} else {
+		dev_warn(&ha->pdev->dev, "region #0 not a PIO resource\n");
+		pio = 0;
+	}
+
+	/* Use MMIO operations for all accesses. */
+	mmio = pci_resource_start(ha->pdev, 1);
+	mmio_len = pci_resource_len(ha->pdev, 1);
+	mmio_flags = pci_resource_flags(ha->pdev, 1);
+
+	if (!(mmio_flags & IORESOURCE_MEM)) {
+		dev_err(&ha->pdev->dev,
+			   "region #0 not an MMIO resource, aborting\n");
+		goto iospace_error_exit;
+	}
+	if (mmio_len < MIN_IOBASE_LEN) {
+		dev_err(&ha->pdev->dev,
+			   "Invalid PCI mem region size, aborting\n");
+		goto iospace_error_exit;
+	}
+
+	if (pci_request_regions(ha->pdev, DRIVER_NAME)) {
+		dev_warn(&ha->pdev->dev,
+			 "Failed to reserve PIO/MMIO regions\n");
+		goto iospace_error_exit;
+	}
+
+	ha->pio_address = pio;
+	ha->pio_length = pio_len;
+	ha->reg = ioremap(mmio, MIN_IOBASE_LEN);
+	if (!ha->reg) {
+		dev_err(&ha->pdev->dev, "cannot remap MMIO, aborting\n");
+		goto iospace_error_exit;
+	}
+
+	return 0;
+
+iospace_error_exit:
+	return -ENOMEM;
+}
+
 /**
  * qla4xxx_probe_adapter - callback function to probe HBA
  * @pdev: pointer to pci_dev structure
@@ -1321,68 +1381,6 @@ static void __devexit qla4xxx_remove_ada
 	pci_set_drvdata(pdev, NULL);
 }
 
-/***
- * qla4xxx_iospace_config - maps registers
- * @ha: pointer to adapter structure
- *
- * This routines maps HBA's registers from the pci address space
- * into the kernel virtual address space for memory mapped i/o.
- **/
-static int qla4xxx_iospace_config(struct scsi_qla_host *ha)
-{
-	unsigned long pio, pio_len, pio_flags;
-	unsigned long mmio, mmio_len, mmio_flags;
-
-	pio = pci_resource_start(ha->pdev, 0);
-	pio_len = pci_resource_len(ha->pdev, 0);
-	pio_flags = pci_resource_flags(ha->pdev, 0);
-	if (pio_flags & IORESOURCE_IO) {
-		if (pio_len < MIN_IOBASE_LEN) {
-			dev_warn(&ha->pdev->dev,
-				   "Invalid PCI I/O region size\n");
-			pio = 0;
-		}
-	} else {
-		dev_warn(&ha->pdev->dev, "region #0 not a PIO resource\n");
-		pio = 0;
-	}
-
-	/* Use MMIO operations for all accesses. */
-	mmio = pci_resource_start(ha->pdev, 1);
-	mmio_len = pci_resource_len(ha->pdev, 1);
-	mmio_flags = pci_resource_flags(ha->pdev, 1);
-
-	if (!(mmio_flags & IORESOURCE_MEM)) {
-		dev_err(&ha->pdev->dev,
-			   "region #0 not an MMIO resource, aborting\n");
-		goto iospace_error_exit;
-	}
-	if (mmio_len < MIN_IOBASE_LEN) {
-		dev_err(&ha->pdev->dev,
-			   "Invalid PCI mem region size, aborting\n");
-		goto iospace_error_exit;
-	}
-
-	if (pci_request_regions(ha->pdev, DRIVER_NAME)) {
-		dev_warn(&ha->pdev->dev,
-			 "Failed to reserve PIO/MMIO regions\n");
-		goto iospace_error_exit;
-	}
-
-	ha->pio_address = pio;
-	ha->pio_length = pio_len;
-	ha->reg = ioremap(mmio, MIN_IOBASE_LEN);
-	if (!ha->reg) {
-		dev_err(&ha->pdev->dev, "cannot remap MMIO, aborting\n");
-		goto iospace_error_exit;
-	}
-
-	return 0;
-
-iospace_error_exit:
-	return -ENOMEM;
-}
-
 /**
  * qla4xxx_config_dma_addressing() - Configure OS DMA addressing method.
  * @ha: HA context

Re: [PATCH 3/3] qla4xxx: iospace_config move defn

[David C Somayajulu]

On Wed, 2006-08-09 at 13:56 -0400, Mike Christie wrote:
> Doug Maxey wrote:
> > From: Doug Maxey <dwm@enoyolf.org>
> > 
> > Move the defn before the caller, and remove the decl.
> > 
> 
> I think not doing the defn before the caller is nice, but I honestly
> could care less either way as far as merging things goes (it seems like
> Christoph hates then but people like Jeff let the driver writer do what
> they want). Qlogic seems to love doing it, so are they ok with all these
> patches?
We have no problem with these patches since it makes the code cleaner
and makes everyone happy.
-david S.

Re: [PATCH 3/3] qla4xxx: iospace_config move defn

[Andrew Vasquez]

On Wed, 09 Aug 2006, David Somayajulu wrote:

> On Wed, 2006-08-09 at 13:56 -0400, Mike Christie wrote:
> > Doug Maxey wrote:
> > > From: Doug Maxey <dwm@enoyolf.org>
> > > 
> > > Move the defn before the caller, and remove the decl.
> > > 
> > 
> > I think not doing the defn before the caller is nice, but I honestly
> > could care less either way as far as merging things goes (it seems like
> > Christoph hates then but people like Jeff let the driver writer do what
> > they want). Qlogic seems to love doing it, so are they ok with all these
> > patches?
> We have no problem with these patches since it makes the code cleaner
> and makes everyone happy.

So, would you folks (Mike/James) like the qla4xxx patches respun with
Doug's changes?  Or will Dave S's original submission be added to
scsi-misc-2.6.git with the fixup/cleanups on top?

Thanks,
AV

Re: [PATCH 3/3] qla4xxx: iospace_config move defn

[James Bottomley]

On Wed, 2006-08-09 at 13:15 -0700, Andrew Vasquez wrote:
> So, would you folks (Mike/James) like the qla4xxx patches respun with
> Doug's changes?  Or will Dave S's original submission be added to
> scsi-misc-2.6.git with the fixup/cleanups on top?

I'd rather someone provided me with a clean set of agreed upon diffs to
incorporate.

James

Re: [PATCH 3/3] qla4xxx: iospace_config move defn

[Doug Maxey]

On Wed, 09 Aug 2006 14:03:24 EDT, Mike Christie wrote:
> Mike Christie wrote:
> > 
> > Is my mailer messing up the tabs or are you (maybe the original code was
> > messed up and did not get fixed in the move)?
> > 

Wellll..

Have not been commenting on the whitespace changes.  It appears that 
some frobnication was happening, I have been silently editing leading 
spaces to tabs and stripping trailing whitespace.

Are you seeing tabs changed to spaces?  Or the other way around?  It is
possible that emacs is getting involved some, but I can easily see the
trailing whitespace. (nee '(show-trailing-whitespace t)).

There is also a vi macro to do the same.  My vi experience is too
ancient to comment about the placement in the rc of the following,
but does work for me when I have to fall back to the ubiquitous editor. 
YMMV.

~/.vimrc
...
" Switch syntax highlighting on, when the terminal has colors
" Also switch on highlighting the last used search pattern.
if &t_Co > 2 || has("gui_running")
  syntax on
  set hlsearch
  highlight WhitespaceEOL ctermbg=red guibg=red
  match WhitespaceEOL /\s\+$/
endif
...

> 
> Nevermind, I switch mailers and it looks ok now. Weird, some patches
> looked ok and some did not before.
> 

Re: [PATCH 3/3] qla4xxx: iospace_config move defn

[David C Somayajulu]

On Wed, 2006-08-09 at 15:21 -0500, James Bottomley wrote:
> On Wed, 2006-08-09 at 13:15 -0700, Andrew Vasquez wrote:
> > So, would you folks (Mike/James) like the qla4xxx patches respun with
> > Doug's changes?  Or will Dave S's original submission be added to
> > scsi-misc-2.6.git with the fixup/cleanups on top?
> 
> I'd rather someone provided me with a clean set of agreed upon diffs to
> incorporate.
> 
> James
> 
> 
Excellent. I am working on a comprehensive set of patches and will try
to get it out by the end of the day.
-david Somayajulu
qLogic Corporation.